Check if-none-match before fulfilling preservation cache

.changeset/fuzzy-nails-invent.md

@@ -0,0 +1,5 @@
+---
+"@cloudflare/pages-shared": minor
+---
+
+feat: Return a 304 Not Modified response when matching an asset preservation cache request if appropriate

packages/pages-shared/__tests__/asset-server/handler.test.ts

@@ -572,38 +572,52 @@ describe("asset-server handler", () => {
 				'"asset-key-foo.html"'
 			);
 
-			// Delete the asset from the manifest and ensure it's served from preservation cache
+			// Delete the asset from the manifest and ensure it's served from preservation cache with a 304 when if-none-match is present
 			findAssetEntryForPath = async (_path: string) => {
 				return null;
 			};
 			const { response: response2 } = await getTestResponse({
+				request: new Request("https://example.com/foo", {
+					headers: { "if-none-match": expectedHeaders.etag },
+				}),
+				metadata,
+				findAssetEntryForPath,
+				caches,
+				fetchAsset: () =>
+					Promise.resolve(Object.assign(new Response("hello world!"))),
+			});
+			expect(response2.status).toBe(304);
+			expect(await response2.text()).toMatchInlineSnapshot('""');
+
+			// Ensure the asset is served from preservation cache with a 200 if if-none-match is not present
+			const { response: response3 } = await getTestResponse({
 				request: new Request("https://example.com/foo"),
 				metadata,
 				findAssetEntryForPath,
 				caches,
 				fetchAsset: () =>
 					Promise.resolve(Object.assign(new Response("hello world!"))),
 			});
-			expect(response2.status).toBe(200);
-			expect(await response2.text()).toMatchInlineSnapshot('"hello world!"');
+			expect(response3.status).toBe(200);
+			expect(await response3.text()).toMatchInlineSnapshot('"hello world!"');
 			// Cached responses have the same headers with a few changes/additions:
-			expect(Object.fromEntries(response2.headers)).toStrictEqual({
+			expect(Object.fromEntries(response3.headers)).toStrictEqual({
 				...expectedHeaders,
 				"cache-control": "public, s-maxage=604800",
 				"x-robots-tag": "noindex",
 				"cf-cache-status": "HIT", // new header
 			});
 
 			// Serve with a fresh cache and ensure we don't get a response
-			const { response: response3 } = await getTestResponse({
+			const { response: response4 } = await getTestResponse({
 				request: new Request("https://example.com/foo"),
 				metadata,
 				findAssetEntryForPath,
 				fetchAsset: () =>
 					Promise.resolve(Object.assign(new Response("hello world!"))),
 			});
-			expect(response3.status).toBe(404);
-			expect(Object.fromEntries(response3.headers)).toMatchInlineSnapshot(`
+			expect(response4.status).toBe(404);
+			expect(Object.fromEntries(response4.headers)).toMatchInlineSnapshot(`
 				{
 				  "access-control-allow-origin": "*",
 				  "cache-control": "no-store",

packages/pages-shared/asset-server/handler.ts

@@ -68,6 +68,10 @@ export function normaliseHeaders(
 	}
 }
 
+type FindAssetEntryForPath<AssetEntry> = (
+	path: string
+) => Promise<null | AssetEntry>;
+
 function generateETagHeader(assetKey: string) {
 	// https://support.cloudflare.com/hc/en-us/articles/218505467-Using-ETag-Headers-with-Cloudflare
 	// We sometimes remove etags unless they are wrapped in quotes
@@ -95,7 +99,10 @@ type ServeAsset<AssetEntry> = (
 type CacheStatus = "hit" | "miss";
 type CacheResult<A extends string> = `${A}-${CacheStatus}`;
 export type HandlerMetrics = {
-	preservationCacheResult?: CacheResult<"checked"> | "disabled";
+	preservationCacheResult?:
+		| CacheResult<"checked">
+		| "not-modified"
+		| "disabled";
 	earlyHintsResult?: CacheResult<"used" | "notused"> | "disabled";
 };
 
@@ -663,6 +670,19 @@ export async function generateHandler<
 						return new Response(null, preservedResponse);
 					}
 					if (assetKey) {
+						const { strongETag, weakETag } = generateETagHeader(assetKey);
+						const isIfNoneMatch = checkIfNoneMatch(
+							request,
+							strongETag,
+							weakETag
+						);
+						if (isIfNoneMatch) {
+							if (setMetrics) {
+								setMetrics({ preservationCacheResult: "not-modified" });
+							}
+							return new NotModifiedResponse();
+						}
+
 						const asset = await fetchAsset(assetKey);
 
 						if (asset) {