[FIX] serveResources: Improve cache invalidation (#688)

Co-authored-by: Merlin Beutlberger <m.beutlberger@sap.com>

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.slice(0, -1) + `-${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()));
 			}
 

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");
+});

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 = {