fix: LastModified Date (#211)

* copying over mtime changes

* initial test changes

* update fetch-handler test

* added utime func

* update fetch-handler test

* test change

* adding utime for local testing

* small test update

---------

Co-authored-by: Rob Stanford <[email protected]>

Author: taty2010

src/build/content/prerendered.ts

@@ -53,34 +53,43 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise<void>
       Object.entries(manifest.routes).map(async ([route, meta]): Promise<void> => {
         const key = routeToFilePath(route)
         let value: CacheValue
+        let path: string
         switch (true) {
           // Parallel route default layout has no prerendered page
           case meta.dataRoute?.endsWith('/default.rsc') &&
             !existsSync(join(ctx.publishDir, 'server/app', `${key}.html`)):
             return
           case meta.dataRoute?.endsWith('.json'):
-            value = await buildPagesCacheValue(join(ctx.publishDir, 'server/pages', key))
+            path = join(ctx.publishDir, 'server/pages', key)
+            value = await buildPagesCacheValue(path)
             break
           case meta.dataRoute?.endsWith('.rsc'):
-            value = await buildAppCacheValue(join(ctx.publishDir, 'server/app', key))
+            path = join(ctx.publishDir, 'server/app', key)
+            value = await buildAppCacheValue(path)
             break
           case meta.dataRoute === null:
-            value = await buildRouteCacheValue(join(ctx.publishDir, 'server/app', key))
+            path = join(ctx.publishDir, 'server/app', key)
+            value = await buildRouteCacheValue(path)
             break
           default:
             throw new Error(`Unrecognized content: ${route}`)
         }
 
-        await ctx.writeCacheEntry(key, value)
+        await ctx.writeCacheEntry(
+          key,
+          value,
+          meta.dataRoute === null ? `${path}.body` : `${path}.html`,
+        )
       }),
     )
 
     // app router 404 pages are not in the prerender manifest
     // so we need to check for them manually
     if (existsSync(join(ctx.publishDir, `server/app/_not-found.html`))) {
       const key = '/404'
-      const value = await buildAppCacheValue(join(ctx.publishDir, 'server/app/_not-found'))
-      await ctx.writeCacheEntry(key, value)
+      const path = join(ctx.publishDir, 'server/app/_not-found')
+      const value = await buildAppCacheValue(path)
+      await ctx.writeCacheEntry(key, value, `${path}.html`)
     }
   } catch (error) {
     ctx.failBuild('Failed assembling prerendered content for upload', error)
@@ -99,8 +108,9 @@ export const copyFetchContent = async (ctx: PluginContext): Promise<void> => {
 
     await Promise.all(
       paths.map(async (key): Promise<void> => {
-        const value = await buildFetchCacheValue(join(ctx.publishDir, 'cache/fetch-cache', key))
-        await ctx.writeCacheEntry(key, value)
+        const path = join(ctx.publishDir, 'cache/fetch-cache', key)
+        const value = await buildFetchCacheValue(path)
+        await ctx.writeCacheEntry(key, value, path)
       }),
     )
   } catch (error) {

src/build/plugin-context.ts

@@ -1,5 +1,5 @@
 import { readFileSync } from 'node:fs'
-import { mkdir, readFile, writeFile } from 'node:fs/promises'
+import { mkdir, readFile, writeFile, stat } from 'node:fs/promises'
 // Here we need to actually import `resolve` from node:path as we want to resolve the paths
 // eslint-disable-next-line no-restricted-imports
 import { dirname, join, resolve } from 'node:path'
@@ -188,10 +188,12 @@ export class PluginContext {
   /**
    * Write a cache entry to the blob upload directory.
    */
-  async writeCacheEntry(route: string, value: CacheValue): Promise<void> {
+  async writeCacheEntry(route: string, value: CacheValue, filePath: string): Promise<void> {
+    // Getting modified file date from prerendered content
+    const { mtime } = await stat(filePath)
     const path = join(this.blobDir, await encodeBlobKey(route))
     const entry = JSON.stringify({
-      lastModified: Date.now(),
+      lastModified: mtime.getTime(),
       value,
     } satisfies CacheEntry)
 

tests/integration/cache-handler.test.ts

@@ -7,14 +7,19 @@ import {
   invokeFunction,
   runPlugin,
   type FixtureTestContext,
+  runPluginStep,
 } from '../utils/fixture.js'
 import {
+  changeMDate,
   decodeBlobKey,
   encodeBlobKey,
   generateRandomObjectID,
   getBlobEntries,
   startMockBlobStore,
 } from '../utils/helpers.js'
+import { join } from 'path'
+import { existsSync } from 'node:fs'
+import { minify } from 'next/dist/build/swc/index.js'
 
 // Disable the verbose logging of the lambda-local runtime
 getLogger().level = 'alert'
@@ -36,6 +41,22 @@ describe('page router', () => {
   test<FixtureTestContext>('page router with static revalidate', async (ctx) => {
     await createFixture('page-router', ctx)
     console.time('runPlugin')
+    const {
+      constants: { PUBLISH_DIR },
+    } = await runPluginStep(ctx, 'onPreBuild')
+    const filePaths = [
+      'server/pages/static/revalidate-automatic.html',
+      'server/pages/static/revalidate-automatic.json',
+      'server/pages/static/revalidate-slow.html',
+      'server/pages/static/revalidate-slow.json',
+    ]
+
+    filePaths.forEach(async (filePath) => {
+      if (existsSync(filePath)) {
+        // Changing the fetch files modified date to a past date since the test files are copied and dont preserve the mtime locally
+        await changeMDate(join(PUBLISH_DIR, filePath), 1674690060000)
+      }
+    })
     await runPlugin(ctx)
     console.timeEnd('runPlugin')
     // check if the blob entries where successful set on the build plugin
@@ -55,19 +76,20 @@ describe('page router', () => {
     const call1Date = load(call1.body)('[data-testid="date-now"]').text()
     expect(call1.statusCode).toBe(200)
     expect(load(call1.body)('h1').text()).toBe('Show #71')
-    expect(call1.headers, 'a cache hit on the first invocation of a prerendered page').toEqual(
+    // Because we're using mtime instead of Date.now() first invocation will actually be a cache miss.
+    expect(call1.headers, 'a cache miss on the first invocation of a prerendered page').toEqual(
       expect.objectContaining({
-        'cache-status': expect.stringMatching(/"Next.js"; hit/),
+        'cache-status': expect.stringMatching(/"Next.js"; miss/),
         'netlify-cdn-cache-control': 's-maxage=5, stale-while-revalidate=31536000',
       }),
     )
 
+    // wait to have a stale page
+    await new Promise<void>((resolve) => setTimeout(resolve, 9_000))
+
     // Ping this now so we can wait in parallel
     const callLater = await invokeFunction(ctx, { url: 'static/revalidate-slow' })
 
-    // wait to have a stale page
-    await new Promise<void>((resolve) => setTimeout(resolve, 3_000))
-
     // now it should be a cache miss
     const call2 = await invokeFunction(ctx, { url: 'static/revalidate-automatic' })
     const call2Date = load(call2.body)('[data-testid="date-now"]').text()
@@ -131,9 +153,10 @@ describe('app router', () => {
     const post1Date = load(post1.body)('[data-testid="date-now"]').text()
     expect(post1.statusCode).toBe(200)
     expect(load(post1.body)('h1').text()).toBe('Revalidate Fetch')
-    expect(post1.headers, 'a cache hit on the first invocation of a prerendered page').toEqual(
+    expect(post1.headers, 'a cache miss on the first invocation of a prerendered page').toEqual(
+      // It will be stale/miss instead of hit
       expect.objectContaining({
-        'cache-status': expect.stringMatching(/"Next.js"; hit/),
+        'cache-status': expect.stringMatching(/"Next.js"; miss/),
         'netlify-cdn-cache-control': 's-maxage=5, stale-while-revalidate=31536000',
       }),
     )
@@ -228,9 +251,9 @@ describe('route', () => {
         name: 'Under the Dome',
       }),
     })
-    expect(call1.headers, 'a cache hit on the first invocation of a prerendered route').toEqual(
+    expect(call1.headers, 'a cache miss on the first invocation').toEqual(
       expect.objectContaining({
-        'cache-status': expect.stringMatching(/"Next.js"; hit/),
+        'cache-status': expect.stringMatching(/"Next.js"; miss/),
       }),
     )
     // wait to have a stale route

tests/integration/fetch-handler.test.ts

@@ -9,6 +9,7 @@ import {
   invokeFunction,
   runPlugin,
   type FixtureTestContext,
+  runPluginStep,
 } from '../utils/fixture.js'
 import {
   decodeBlobKey,
@@ -17,7 +18,10 @@ import {
   getBlobEntries,
   getFetchCacheKey,
   startMockBlobStore,
+  changeMDate,
 } from '../utils/helpers.js'
+import { existsSync } from 'node:fs'
+import { join } from 'node:path'
 
 // Disable the verbose logging of the lambda-local runtime
 getLogger().level = 'alert'
@@ -66,11 +70,20 @@ afterEach(async () => {
 test<FixtureTestContext>('if the fetch call is cached correctly', async (ctx) => {
   await createFixture('revalidate-fetch', ctx)
   console.time('TimeUntilStale')
+  const {
+    constants: { PUBLISH_DIR },
+  } = await runPluginStep(ctx, 'onPreBuild')
+  const originalKey = '460ed46cd9a194efa197be9f2571e51b729a039d1cff9834297f416dce5ada29'
+
+  const filePath = join(PUBLISH_DIR, 'cache/fetch-cache', originalKey)
+  if (existsSync(filePath)) {
+    // Changing the fetch files modified date to a past date since the test files are copied and dont preserve the mtime locally
+    await changeMDate(filePath, 1674690060000)
+  }
   await runPlugin(ctx)
 
   // replace the build time fetch cache with our mocked hash
   const cacheKey = await getFetchCacheKey(new URL('/1', apiBase).href)
-  const originalKey = '460ed46cd9a194efa197be9f2571e51b729a039d1cff9834297f416dce5ada29'
   const fakeKey = cacheKey
   const fetchEntry = await ctx.blobStore.get(encodeBlobKey(originalKey), { type: 'json' })
 
@@ -104,10 +117,10 @@ test<FixtureTestContext>('if the fetch call is cached correctly', async (ctx) =>
   console.timeEnd('TimeUntilStale')
 
   const post1Name = load(post1.body)('[data-testid="name"]').text()
-  // should still get the old value
-  expect(handlerCalled, 'should not call the API as the request should be cached').toBe(0)
+  // Will still end up calling he API on initial request with us using mtime for lastModified
+  expect(handlerCalled, 'should not call the API as the request should be cached').toBe(1)
   expect(post1.statusCode).toBe(200)
-  expect(post1Name).toBe('Under the Dome')
+  expect(post1Name).toBe('Fake response')
   expect(post1.headers, 'the page should be a miss').toEqual(
     expect.objectContaining({
       'cache-status': expect.stringMatching(/"Next.js"; miss/),
@@ -127,5 +140,5 @@ test<FixtureTestContext>('if the fetch call is cached correctly', async (ctx) =>
   const post2Name = load(post2.body)('[data-testid="name"]').text()
   expect(post2.statusCode).toBe(200)
   expect.soft(post2Name).toBe('Fake response')
-  expect(handlerCalled).toBe(1)
+  expect(handlerCalled).toBe(2)
 })

tests/utils/helpers.ts

@@ -5,10 +5,11 @@ import { BlobsServer, getDeployStore } from '@netlify/blobs'
 import type { NetlifyPluginUtils } from '@netlify/build'
 import IncrementalCache from 'next/dist/server/lib/incremental-cache/index.js'
 import { Buffer } from 'node:buffer'
-import { mkdtemp } from 'node:fs/promises'
+import { mkdtemp, stat } from 'node:fs/promises'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { assert, vi } from 'vitest'
+import { utimes } from 'node:fs'
 
 /**
  * Uses next.js incremental cache to compute the same cache key for a URL that is automatically generated
@@ -115,3 +116,17 @@ export const mockBuildUtils = {
     assert.fail(`${message}: ${options?.error || ''}`)
   },
 } as unknown as NetlifyPluginUtils
+
+export const changeMDate = async (filePath: string, newTime: number) => {
+  const prevStat = await stat(filePath)
+
+  console.log('Previous Modified time', {
+    mtime: prevStat.mtime.getTime(),
+  })
+
+  utimes(filePath, new Date(newTime), new Date(newTime), () => {
+    console.log('New Modified time', {
+      mtime: newTime,
+    })
+  })
+}