-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
feat(assets): Use entity-tags to revalidate cached remote images #12426
Merged
ematipico
merged 12 commits into
withastro:main
from
oliverlynch:remote-assets-use-etag
Dec 18, 2024
+144
−26
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
2e2fc9d
feat(assets): Store etag to refresh cached images without a full down…
oliverlynch e844a93
Merge branch 'withastro:main' into remote-assets-use-etag
oliverlynch 6955f3f
Seperate loading and revalidating functions
oliverlynch 9981da8
Add changeset
oliverlynch 83d007d
Merge branch 'main' into remote-assets-use-etag
oliverlynch c3e60f3
Merge branch 'withastro:main' into remote-assets-use-etag
oliverlynch f7a614a
Updates based on requested changes
oliverlynch 2ebfd63
Wording changes, use stale cache on failure to revalidate
oliverlynch a806f5c
Merge branch 'withastro:main' into remote-assets-use-etag
oliverlynch c63f48d
Add If-Modified-Since as cache revalidation method
oliverlynch 093dbdb
Merge branch 'main' into remote-assets-use-etag
ascorbic c3675ac
Update .changeset/red-poems-pay.md
oliverlynch File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
'astro': minor | ||
--- | ||
|
||
Improves asset caching of remote images | ||
|
||
Astro will now store [entity tags](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag) and the [Last-Modified](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Last-Modified) date for cached remote images and use them to revalidate the cache when it goes stale. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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'), | ||
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 { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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
.json