From 609798a9729ee42744acdfe508ba800f98bc1099 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 29 Jul 2024 11:10:54 +0200 Subject: [PATCH 1/3] [FIX] serveResources: Improve cache invalidation --- lib/middleware/serveResources.js | 14 ++- test/lib/server/caching.js | 99 ++++++++++++++++++++ test/lib/server/middleware/serveResources.js | 9 +- 3 files changed, 116 insertions(+), 6 deletions(-) create mode 100644 test/lib/server/caching.js diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index bfa1becd..768d172a 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -95,10 +95,18 @@ function createMiddleware({resources, middlewareUtil}) { // Enable ETag caching const statInfo = resource.getStatInfo(); - if (statInfo?.size !== undefined) { - res.setHeader("ETag", etag(statInfo)); + if (statInfo?.size !== undefined && !resource.isModified()) { + let etagHeader = etag(statInfo); + if (resource.getProject()) { + // Add project version to ETag to invalidate cache when project version changes. + // This is necessary to invalidate files with ${version} placeholders. + etagHeader = etagHeader.replace(/"$/g, `-${resource.getProject().getVersion()}"`); + } + res.setHeader("ETag", etagHeader); } else { - // Fallback to buffer if stats are not available or insufficient + // Fallback to buffer if stats are not available or insufficient or resource is modified. + // Modified resources must use the buffer for cache invalidation so that UI5 Tooling changes + // invalidate the cache even when the original resource is not modified. res.setHeader("ETag", etag(await resource.getBuffer())); } diff --git a/test/lib/server/caching.js b/test/lib/server/caching.js new file mode 100644 index 00000000..8cabe730 --- /dev/null +++ b/test/lib/server/caching.js @@ -0,0 +1,99 @@ +import test from "ava"; +import sinonGlobal from "sinon"; +import esmock from "esmock"; +import supertest from "supertest"; +import {graphFromPackageDependencies} from "@ui5/project/graph"; + +let request; +let server; + +// Start server before running tests +test.before(async (t) => { + const sinon = t.context.sinon = sinonGlobal.createSandbox(); + + t.context.manifestEnhancer = sinon.stub(); + + const {serve} = await esmock.p("../../../lib/server.js", {}, { + "@ui5/builder/processors/manifestEnhancer": t.context.manifestEnhancer, + }); + + const graph = await graphFromPackageDependencies({ + cwd: "./test/fixtures/application.a" + }); + + t.context.applicationProject = graph.getProject("application.a"); + + server = await serve(graph, { + port: 3334 + }); + request = supertest("http://localhost:3334"); +}); + +test.after.always(() => { + return new Promise((resolve, reject) => { + server.close((error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }); + }); +}); + +test("serveResources: manifestEnhancer cache invalidation", async (t) => { + const {manifestEnhancer} = t.context; + + manifestEnhancer.callsFake(async ({resources}) => { + for (const resource of resources) { + resource.setString(JSON.stringify({"mockedResponse": "v1"})); + } + }); + + const response = await request.get("/manifest.json"); + if (response.error) { + throw new Error(response.error); + } + t.is(response.statusCode, 200, "Correct HTTP status code"); + t.is(response.text, JSON.stringify({ + "mockedResponse": "v1" + }), "Correct response"); + + const cachedResponse = await request.get("/manifest.json").set({"If-None-Match": response.headers.etag}); + t.is(cachedResponse.statusCode, 304, "Correct HTTP status code"); + + // Changes to the response content should invalidate the cache + manifestEnhancer.callsFake(async ({resources}) => { + for (const resource of resources) { + resource.setString(JSON.stringify({"mockedResponse": "v2"})); + } + }); + + const newResponse = await request.get("/manifest.json").set({"If-None-Match": response.headers.etag}); + t.is(newResponse.statusCode, 200, "Correct HTTP status code"); + t.is(newResponse.text, JSON.stringify({ + "mockedResponse": "v2" + }), "Correct response"); +}); + +test("serveResources: version placeholder cache invalidation", async (t) => { + const {applicationProject} = t.context; + + const response = await request.get("/versionTest.js"); + if (response.error) { + throw new Error(response.error); + } + t.is(response.statusCode, 200, "Correct HTTP status code"); + t.is(response.text, "console.log(`1.0.0`);\n", "Correct response"); + + const cachedResponse = await request.get("/versionTest.js").set({"If-None-Match": response.headers.etag}); + t.is(cachedResponse.statusCode, 304, "Correct HTTP status code"); + + // Changes to the project version should invalidate the cache + applicationProject._version = "1.0.1-SNAPSHOT"; + + const newResponse = await request.get("/versionTest.js").set({"If-None-Match": response.headers.etag}); + t.is(newResponse.statusCode, 200, "Correct HTTP status code"); + t.is(newResponse.text, "console.log(`1.0.1-SNAPSHOT`);\n", "Correct response"); + t.regex(newResponse.headers.etag, /1\.0\.1-SNAPSHOT/, "Correct updated ETag"); +}); diff --git a/test/lib/server/middleware/serveResources.js b/test/lib/server/middleware/serveResources.js index ac9515ef..cad028f5 100644 --- a/test/lib/server/middleware/serveResources.js +++ b/test/lib/server/middleware/serveResources.js @@ -337,7 +337,8 @@ test.serial("Check verbose logging", async (t) => { return { getVersion: () => "1.0.0" }; - } + }, + isModified: () => false }; const resources = { @@ -404,7 +405,8 @@ test.serial("Check if version replacement is done", (t) => { getVersion: () => "1.0.0" }; }, - getPathTree: () => "" + getPathTree: () => "", + isModified: () => false }; const resources = { @@ -483,7 +485,8 @@ test.serial("Check if utf8 characters are correctly processed in version replace getVersion: () => "1.0.0" }; }, - getPathTree: () => "" + getPathTree: () => "", + isModified: () => false }; const resources = { From 4fd49878a9bbd4de3eed1a93d976236a8b178bf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20O=C3=9Fwald?= Date: Mon, 29 Jul 2024 15:46:26 +0200 Subject: [PATCH 2/3] Update lib/middleware/serveResources.js Co-authored-by: Merlin Beutlberger --- lib/middleware/serveResources.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index 768d172a..c4ee61e6 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -100,7 +100,7 @@ function createMiddleware({resources, middlewareUtil}) { if (resource.getProject()) { // Add project version to ETag to invalidate cache when project version changes. // This is necessary to invalidate files with ${version} placeholders. - etagHeader = etagHeader.replace(/"$/g, `-${resource.getProject().getVersion()}"`); + etagHeader = etagHeader.slice(0, -1) + `-${resource.getProject().getVersion()}"`); } res.setHeader("ETag", etagHeader); } else { From 01fa88f463ce40f7d9fc06ea26d6076b6ce6ac42 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 29 Jul 2024 15:50:41 +0200 Subject: [PATCH 3/3] Fix syntax error --- lib/middleware/serveResources.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index c4ee61e6..4e709599 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -100,7 +100,7 @@ function createMiddleware({resources, middlewareUtil}) { if (resource.getProject()) { // Add project version to ETag to invalidate cache when project version changes. // This is necessary to invalidate files with ${version} placeholders. - etagHeader = etagHeader.slice(0, -1) + `-${resource.getProject().getVersion()}"`); + etagHeader = etagHeader.slice(0, -1) + `-${resource.getProject().getVersion()}"`; } res.setHeader("ETag", etagHeader); } else {