-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(assets): Use entity-tags to revalidate cached remote images #12426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 11 commits
2e2fc9d
e844a93
6955f3f
9981da8
83d007d
c3e60f3
f7a614a
2ebfd63
a806f5c
c63f48d
093dbdb
c3675ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'astro': minor | ||
--- | ||
|
||
Improves the asset caching by allowing stale assets to be revalidated [using entity tags](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag) or the [Last-Modified](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Last-Modified) date. | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,18 +15,18 @@ import { getConfiguredImageService } from '../internal.js'; | |
import type { LocalImageService } from '../services/service.js'; | ||
import type { AssetsGlobalStaticImagesList, ImageMetadata, ImageTransform } from '../types.js'; | ||
import { isESMImportedImage } from '../utils/imageKind.js'; | ||
import { type RemoteCacheEntry, loadRemoteImage } from './remote.js'; | ||
import { type RemoteCacheEntry, loadRemoteImage, revalidateRemoteImage } from './remote.js'; | ||
|
||
interface GenerationDataUncached { | ||
cached: false; | ||
cached: 'miss'; | ||
weight: { | ||
before: number; | ||
after: number; | ||
}; | ||
} | ||
|
||
interface GenerationDataCached { | ||
cached: true; | ||
cached: 'revalidated' | 'hit'; | ||
} | ||
|
||
type GenerationData = GenerationDataUncached | GenerationDataCached; | ||
|
@@ -43,7 +43,12 @@ type AssetEnv = { | |
assetsFolder: AstroConfig['build']['assets']; | ||
}; | ||
|
||
type ImageData = { data: Uint8Array; expires: number }; | ||
type ImageData = { | ||
data: Uint8Array; | ||
expires: number; | ||
etag?: string; | ||
lastModified?: string; | ||
}; | ||
|
||
export async function prepareAssetsGenerationEnv( | ||
pipeline: BuildPipeline, | ||
|
@@ -135,9 +140,12 @@ export async function generateImagesForPath( | |
const timeEnd = performance.now(); | ||
const timeChange = getTimeStat(timeStart, timeEnd); | ||
const timeIncrease = `(+${timeChange})`; | ||
const statsText = generationData.cached | ||
? `(reused cache entry)` | ||
: `(before: ${generationData.weight.before}kB, after: ${generationData.weight.after}kB)`; | ||
const statsText = | ||
generationData.cached !== 'miss' | ||
? generationData.cached === 'hit' | ||
? `(reused cache entry)` | ||
: `(revalidated cache entry)` | ||
: `(before: ${generationData.weight.before}kB, after: ${generationData.weight.after}kB)`; | ||
const count = `(${env.count.current}/${env.count.total})`; | ||
env.logger.info( | ||
null, | ||
|
@@ -156,7 +164,7 @@ export async function generateImagesForPath( | |
const finalFolderURL = new URL('./', finalFileURL); | ||
await fs.promises.mkdir(finalFolderURL, { recursive: true }); | ||
|
||
// For remote images, instead of saving the image directly, we save a JSON file with the image data and expiration date from the server | ||
// For remote images, instead of saving the image directly, we save a JSON file with the image data, expiration date, etag and last-modified date from the server | ||
const cacheFile = basename(filepath) + (isLocalImage ? '' : '.json'); | ||
const cachedFileURL = new URL(cacheFile, env.assetsCacheDir); | ||
|
||
|
@@ -166,7 +174,7 @@ export async function generateImagesForPath( | |
await fs.promises.copyFile(cachedFileURL, finalFileURL, fs.constants.COPYFILE_FICLONE); | ||
|
||
return { | ||
cached: true, | ||
cached: 'hit', | ||
}; | ||
} else { | ||
const JSONData = JSON.parse(readFileSync(cachedFileURL, 'utf-8')) as RemoteCacheEntry; | ||
|
@@ -184,11 +192,43 @@ export async function generateImagesForPath( | |
await fs.promises.writeFile(finalFileURL, Buffer.from(JSONData.data, 'base64')); | ||
|
||
return { | ||
cached: true, | ||
cached: 'hit', | ||
}; | ||
} else { | ||
await fs.promises.unlink(cachedFileURL); | ||
} | ||
|
||
// Try to revalidate the cache | ||
if (JSONData.etag || JSONData.lastModified) { | ||
try { | ||
const revalidatedData = await revalidateRemoteImage(options.src as string, { | ||
etag: JSONData.etag, | ||
lastModified: JSONData.lastModified, | ||
}); | ||
|
||
if (revalidatedData.data.length) { | ||
// Image cache was stale, update original image to avoid redownload | ||
originalImage = revalidatedData; | ||
} else { | ||
revalidatedData.data = Buffer.from(JSONData.data, 'base64'); | ||
|
||
// Freshen cache on disk | ||
await writeRemoteCacheFile(cachedFileURL, revalidatedData, env); | ||
|
||
await fs.promises.writeFile(finalFileURL, revalidatedData.data); | ||
return { cached: 'revalidated' }; | ||
} | ||
} catch (e) { | ||
// Reuse stale cache if revalidation fails | ||
env.logger.warn( | ||
null, | ||
`An error was encountered while revalidating a cached remote asset. Proceeding with stale cache. ${e}`, | ||
); | ||
|
||
await fs.promises.writeFile(finalFileURL, Buffer.from(JSONData.data, 'base64')); | ||
return { cached: 'hit' }; | ||
} | ||
} | ||
|
||
await fs.promises.unlink(cachedFileURL); | ||
} | ||
} catch (e: any) { | ||
if (e.code !== 'ENOENT') { | ||
|
@@ -209,6 +249,8 @@ export async function generateImagesForPath( | |
let resultData: Partial<ImageData> = { | ||
data: undefined, | ||
expires: originalImage.expires, | ||
etag: originalImage.etag, | ||
lastModified: originalImage.lastModified, | ||
}; | ||
|
||
const imageService = (await getConfiguredImageService()) as LocalImageService; | ||
|
@@ -239,13 +281,7 @@ export async function generateImagesForPath( | |
if (isLocalImage) { | ||
await fs.promises.writeFile(cachedFileURL, resultData.data); | ||
} else { | ||
await fs.promises.writeFile( | ||
cachedFileURL, | ||
JSON.stringify({ | ||
data: Buffer.from(resultData.data).toString('base64'), | ||
expires: resultData.expires, | ||
}), | ||
); | ||
await writeRemoteCacheFile(cachedFileURL, resultData as ImageData, env); | ||
} | ||
} | ||
} catch (e) { | ||
|
@@ -259,7 +295,7 @@ export async function generateImagesForPath( | |
} | ||
|
||
return { | ||
cached: false, | ||
cached: 'miss', | ||
weight: { | ||
// Divide by 1024 to get size in kilobytes | ||
before: Math.trunc(originalImage.data.byteLength / 1024), | ||
|
@@ -269,6 +305,25 @@ export async function generateImagesForPath( | |
} | ||
} | ||
|
||
async function writeRemoteCacheFile(cachedFileURL: URL, resultData: ImageData, env: AssetEnv) { | ||
try { | ||
return await fs.promises.writeFile( | ||
cachedFileURL, | ||
JSON.stringify({ | ||
data: Buffer.from(resultData.data).toString('base64'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realise that this is what we were already doing, but it does seem a bit wasteful to be saving images as JSON-encoded base64 strings, and this could be a good time to fix it. I think it would be better to store the image in a binary, and then use a separate JSON file for metadata, probably with the same filename alongside it but with something like an appended |
||
expires: resultData.expires, | ||
etag: resultData.etag, | ||
lastModified: resultData.lastModified, | ||
}), | ||
); | ||
} catch (e) { | ||
env.logger.warn( | ||
null, | ||
`An error was encountered while writing the cache file for a remote asset. Proceeding without caching this asset. Error: ${e}`, | ||
); | ||
} | ||
} | ||
|
||
export function getStaticImageList(): AssetsGlobalStaticImagesList { | ||
if (!globalThis?.astroAsset?.staticImages) { | ||
return new Map(); | ||
|
@@ -279,11 +334,7 @@ export function getStaticImageList(): AssetsGlobalStaticImagesList { | |
|
||
async function loadImage(path: string, env: AssetEnv): Promise<ImageData> { | ||
if (isRemotePath(path)) { | ||
const remoteImage = await loadRemoteImage(path); | ||
return { | ||
data: remoteImage.data, | ||
expires: remoteImage.expires, | ||
}; | ||
Comment on lines
-282
to
-286
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no idea what happened here, but thank you for cleaning it, ha |
||
return await loadRemoteImage(path); | ||
} | ||
|
||
return { | ||
|
Uh oh!
There was an error while loading. Please reload this page.