Cache bust image when generating previews (#23639)

* Refactor read method to allow for cache busting

* update tests

* fix formatting

* add changeset

* use file.modified_on as version instead of cache busting

* run linter

---------

Co-authored-by: Brainslug <br41nslug@users.noreply.github.com>
This commit is contained in:
Nitwel
2024-10-09 14:52:20 +02:00
committed by GitHub
parent 737b364eae
commit 26ef287f4b
16 changed files with 100 additions and 65 deletions

View File

@@ -0,0 +1,12 @@
---
'@directus/storage-driver-cloudinary': patch
'@directus/storage-driver-supabase': patch
'@directus/storage-driver-azure': patch
'@directus/storage-driver-local': patch
'@directus/storage-driver-gcs': patch
'@directus/storage-driver-s3': patch
'@directus/storage': patch
'@directus/api': patch
---
Allow for cachebusting when reading images from cloudinary to use the latest image for preview generation.

View File

@@ -138,7 +138,7 @@ export class AssetsService {
if (exists) {
return {
stream: await storage.location(file.storage).read(assetFilename, range),
stream: await storage.location(file.storage).read(assetFilename, { range }),
file,
stat: await storage.location(file.storage).stat(assetFilename),
};
@@ -168,7 +168,9 @@ export class AssetsService {
});
}
const readStream = await storage.location(file.storage).read(file.filename_disk, range);
const version = file.modified_on !== undefined ? String(new Date(file.modified_on).getTime() / 1000) : undefined;
const readStream = await storage.location(file.storage).read(file.filename_disk, { range, version });
const transformer = getSharpInstance();
@@ -202,12 +204,12 @@ export class AssetsService {
}
return {
stream: await storage.location(file.storage).read(assetFilename, range),
stream: await storage.location(file.storage).read(assetFilename, { range }),
stat: await storage.location(file.storage).stat(assetFilename),
file,
};
} else {
const readStream = await storage.location(file.storage).read(file.filename_disk, range);
const readStream = await storage.location(file.storage).read(file.filename_disk, { range });
const stat = await storage.location(file.storage).stat(file.filename_disk);
return { stream: readStream, file, stat };
}

View File

@@ -227,19 +227,19 @@ describe('#read', () => {
});
test('Calls download with offset if start range is provided', async () => {
await driver.read(sample.path.input, { start: sample.range.start });
await driver.read(sample.path.input, { range: { start: sample.range.start } });
expect(mockDownload).toHaveBeenCalledWith(sample.range.start, undefined);
});
test('Calls download with count if end range is provided', async () => {
await driver.read(sample.path.input, { end: sample.range.end });
await driver.read(sample.path.input, { range: { end: sample.range.end } });
expect(mockDownload).toHaveBeenCalledWith(undefined, sample.range.end + 1);
});
test('Calls download with offset and count if start and end ranges are provided', async () => {
await driver.read(sample.path.input, sample.range);
await driver.read(sample.path.input, { range: sample.range });
expect(mockDownload).toHaveBeenCalledWith(sample.range.start, sample.range.end - sample.range.start + 1);
});

View File

@@ -1,5 +1,5 @@
import { BlobServiceClient, ContainerClient, StorageSharedKeyCredential } from '@azure/storage-blob';
import type { Driver, Range } from '@directus/storage';
import type { Driver, ReadOptions } from '@directus/storage';
import { normalizePath } from '@directus/utils';
import { join } from 'node:path';
import type { Readable } from 'node:stream';
@@ -33,7 +33,9 @@ export class DriverAzure implements Driver {
return normalizePath(join(this.root, filepath));
}
async read(filepath: string, range?: Range) {
async read(filepath: string, options?: ReadOptions) {
const { range } = options || {};
const { readableStreamBody } = await this.containerClient
.getBlobClient(this.fullPath(filepath))
.download(range?.start, range?.end ? range.end - (range.start || 0) + 1 : undefined);

View File

@@ -562,7 +562,7 @@ describe('#read', () => {
});
test('Adds optional Range header for start', async () => {
await driver.read(sample.path.input, { start: sample.range.start, end: undefined });
await driver.read(sample.path.input, { range: { start: sample.range.start, end: undefined } });
expect(fetch).toHaveBeenCalledWith(
`https://res.cloudinary.com/${sample.config.cloudName}/${sample.resourceType}/upload/${sample.parameterSignature}/${sample.path.inputFull}`,
@@ -571,7 +571,7 @@ describe('#read', () => {
});
test('Adds optional Range header for end', async () => {
await driver.read(sample.path.input, { start: undefined, end: sample.range.end });
await driver.read(sample.path.input, { range: { start: undefined, end: sample.range.end } });
expect(fetch).toHaveBeenCalledWith(
`https://res.cloudinary.com/${sample.config.cloudName}/${sample.resourceType}/upload/${sample.parameterSignature}/${sample.path.inputFull}`,
@@ -580,7 +580,7 @@ describe('#read', () => {
});
test('Adds optional Range header for start and end', async () => {
await driver.read(sample.path.input, sample.range);
await driver.read(sample.path.input, { range: sample.range });
expect(fetch).toHaveBeenCalledWith(
`https://res.cloudinary.com/${sample.config.cloudName}/${sample.resourceType}/upload/${sample.parameterSignature}/${sample.path.inputFull}`,

View File

@@ -1,4 +1,4 @@
import type { Driver, Range } from '@directus/storage';
import type { Driver, ReadOptions } from '@directus/storage';
import { normalizePath } from '@directus/utils';
import { Blob, Buffer } from 'node:buffer';
import { createHash } from 'node:crypto';
@@ -119,10 +119,12 @@ export class DriverCloudinary implements Driver {
return `Basic ${base64}`;
}
async read(filepath: string, range?: Range) {
async read(filepath: string, options?: ReadOptions) {
const { range, version } = options ?? {};
const resourceType = this.getResourceType(filepath);
const fullPath = this.fullPath(filepath);
const signature = this.getParameterSignature(fullPath);
const signature = version !== undefined ? `v${version}` : this.getParameterSignature(fullPath);
const url = `https://res.cloudinary.com/${this.cloudName}/${resourceType}/upload/${signature}/${fullPath}`;
const requestInit: RequestInit = { method: 'GET' };

View File

@@ -198,13 +198,13 @@ describe('#read', () => {
});
test('Passes optional range to createReadStream', async () => {
await driver.read('/path/to/file', { start: sample.range.start, end: undefined });
await driver.read('/path/to/file', { range: { start: sample.range.start, end: undefined } });
expect(mockFile.createReadStream).toHaveBeenCalledWith({ start: sample.range.start, end: undefined });
await driver.read('/path/to/file', sample.range);
await driver.read('/path/to/file', { range: sample.range });
expect(mockFile.createReadStream).toHaveBeenCalledWith(sample.range);
await driver.read('/path/to/file', { start: undefined, end: sample.range.end });
await driver.read('/path/to/file', { range: { start: undefined, end: sample.range.end } });
expect(mockFile.createReadStream).toHaveBeenCalledWith({ start: undefined, end: sample.range.end });
});
});

View File

@@ -1,4 +1,4 @@
import type { Driver, Range } from '@directus/storage';
import type { Driver, ReadOptions } from '@directus/storage';
import { normalizePath } from '@directus/utils';
import type { Bucket, CreateReadStreamOptions, GetFilesOptions } from '@google-cloud/storage';
import { Storage } from '@google-cloud/storage';
@@ -33,13 +33,15 @@ export class DriverGCS implements Driver {
return this.bucket.file(filepath);
}
async read(filepath: string, range?: Range) {
const options: CreateReadStreamOptions = {};
async read(filepath: string, options?: ReadOptions) {
const { range } = options || {};
if (range?.start) options.start = range.start;
if (range?.end) options.end = range.end;
const stream_options: CreateReadStreamOptions = {};
return this.file(this.fullPath(filepath)).createReadStream(options);
if (range?.start) stream_options.start = range.start;
if (range?.end) stream_options.end = range.end;
return this.file(this.fullPath(filepath)).createReadStream(stream_options);
}
async write(filepath: string, content: Readable) {

View File

@@ -145,19 +145,19 @@ describe('#read', () => {
});
test('Calls createReadStream with optional start range', async () => {
await driver.read(sample.path.input, { start: sample.range.start });
await driver.read(sample.path.input, { range: { start: sample.range.start } });
expect(createReadStream).toHaveBeenCalledWith(sample.path.inputFull, { start: sample.range.start });
});
test('Calls createReadStream with optional end range', async () => {
await driver.read(sample.path.input, { end: sample.range.end });
await driver.read(sample.path.input, { range: { end: sample.range.end } });
expect(createReadStream).toHaveBeenCalledWith(sample.path.inputFull, { end: sample.range.end });
});
test('Calls createReadStream with optional start and end range', async () => {
await driver.read(sample.path.input, sample.range);
await driver.read(sample.path.input, { range: sample.range });
expect(createReadStream).toHaveBeenCalledWith(sample.path.inputFull, sample.range);
});

View File

@@ -1,4 +1,4 @@
import type { TusDriver, ChunkedUploadContext, Range } from '@directus/storage';
import type { TusDriver, ChunkedUploadContext, ReadOptions } from '@directus/storage';
import fsProm from 'fs/promises';
import { createReadStream, createWriteStream } from 'node:fs';
import { access, copyFile, mkdir, opendir, rename, stat, unlink } from 'node:fs/promises';
@@ -28,18 +28,20 @@ export class DriverLocal implements TusDriver {
await mkdir(dirpath, { recursive: true });
}
async read(filepath: string, range?: Range) {
const options: Parameters<typeof createReadStream>[1] = {};
async read(filepath: string, options?: ReadOptions) {
const { range } = options || {};
const stream_options: Parameters<typeof createReadStream>[1] = {};
if (range?.start) {
options.start = range.start;
stream_options.start = range.start;
}
if (range?.end) {
options.end = range.end;
stream_options.end = range.end;
}
return createReadStream(this.fullPath(filepath), options);
return createReadStream(this.fullPath(filepath), stream_options);
}
async stat(filepath: string) {

View File

@@ -328,7 +328,7 @@ describe('#read', () => {
});
test('Optionally allows setting start range offset', async () => {
await driver.read(sample.path.input, { start: sample.range.start });
await driver.read(sample.path.input, { range: { start: sample.range.start } });
expect(GetObjectCommand).toHaveBeenCalledWith({
Key: sample.path.inputFull,
@@ -338,7 +338,7 @@ describe('#read', () => {
});
test('Optionally allows setting end range offset', async () => {
await driver.read(sample.path.input, { end: sample.range.end });
await driver.read(sample.path.input, { range: { end: sample.range.end } });
expect(GetObjectCommand).toHaveBeenCalledWith({
Key: sample.path.inputFull,
@@ -348,7 +348,7 @@ describe('#read', () => {
});
test('Optionally allows setting start and end range offset', async () => {
await driver.read(sample.path.input, sample.range);
await driver.read(sample.path.input, { range: sample.range });
expect(GetObjectCommand).toHaveBeenCalledWith({
Key: sample.path.inputFull,
@@ -361,7 +361,7 @@ describe('#read', () => {
vi.mocked(driver['client'].send).mockReturnValue({ Body: undefined } as unknown as void);
try {
await driver.read(sample.path.input, sample.range);
await driver.read(sample.path.input, { range: sample.range });
} catch (err: any) {
expect(err).toBeInstanceOf(Error);
expect(err.message).toBe(`No stream returned for file "${sample.path.input}"`);
@@ -371,7 +371,7 @@ describe('#read', () => {
test('Throws an error when returned stream is not a readable stream', async () => {
vi.mocked(isReadableStream).mockReturnValue(false);
expect(driver.read(sample.path.input, sample.range)).rejects.toThrowError(
expect(driver.read(sample.path.input, { range: sample.range })).rejects.toThrowError(
new Error(`No stream returned for file "${sample.path.input}"`),
);
});
@@ -382,7 +382,7 @@ describe('#read', () => {
vi.mocked(driver['client'].send).mockReturnValue({ Body: sample.stream } as unknown as void);
vi.mocked(GetObjectCommand).mockReturnValue(mockGetObjectCommand);
const stream = await driver.read(sample.path.input, sample.range);
const stream = await driver.read(sample.path.input, { range: sample.range });
expect(driver['client'].send).toHaveBeenCalledWith(mockGetObjectCommand);
expect(stream).toBe(sample.stream);

View File

@@ -24,7 +24,7 @@ import {
UploadPartCommand,
} from '@aws-sdk/client-s3';
import { Upload } from '@aws-sdk/lib-storage';
import type { ChunkedUploadContext, Range, TusDriver } from '@directus/storage';
import type { ChunkedUploadContext, ReadOptions, TusDriver } from '@directus/storage';
import { normalizePath } from '@directus/utils';
import { isReadableStream } from '@directus/utils/node';
import { Permit, Semaphore } from '@shopify/semaphore';
@@ -130,7 +130,9 @@ export class DriverS3 implements TusDriver {
return normalizePath(join(this.root, filepath));
}
async read(filepath: string, range?: Range): Promise<Readable> {
async read(filepath: string, options?: ReadOptions): Promise<Readable> {
const { range } = options ?? {};
const commandInput: GetObjectCommandInput = {
Key: this.fullPath(filepath),
Bucket: this.config.bucket,

View File

@@ -264,7 +264,7 @@ describe('#read', () => {
});
test('Optionally allows setting start range offset', async () => {
await driver.read(sample.path.input, { start: sample.range.start } as any);
await driver.read(sample.path.input, { range: { start: sample.range.start } } as any);
expect(fetch).toHaveBeenCalledWith(endpoint, {
headers: {
@@ -276,7 +276,7 @@ describe('#read', () => {
});
test('Optionally allows setting end range offset', async () => {
await driver.read(sample.path.input, { end: sample.range.end } as any);
await driver.read(sample.path.input, { range: { end: sample.range.end } } as any);
expect(fetch).toHaveBeenCalledWith(endpoint, {
headers: {
@@ -288,7 +288,7 @@ describe('#read', () => {
});
test('Optionally allows setting start and end range offset', async () => {
await driver.read(sample.path.input, sample.range);
await driver.read(sample.path.input, { range: sample.range });
expect(fetch).toHaveBeenCalledWith(endpoint, {
headers: {
@@ -303,7 +303,7 @@ describe('#read', () => {
vi.mocked(fetch).mockReturnValue({ status: 400, body: new ReadableStream() } as unknown as Promise<Response>);
try {
await driver.read(sample.path.input, sample.range);
await driver.read(sample.path.input, { range: sample.range });
} catch (err: any) {
expect(err).toBeInstanceOf(Error);
expect(err.message).toBe(`No stream returned for file "${sample.path.input}"`);
@@ -313,13 +313,13 @@ describe('#read', () => {
test('Throws an error when returned stream is not a readable stream', async () => {
vi.mocked(fetch).mockReturnValue({ status: 200, body: undefined } as unknown as Promise<Response>);
expect(driver.read(sample.path.input, sample.range)).rejects.toThrowError(
expect(driver.read(sample.path.input, { range: sample.range })).rejects.toThrowError(
new Error(`No stream returned for file "${sample.path.input}"`),
);
});
test('Returns stream', async () => {
const stream = await driver.read(sample.path.input, sample.range);
const stream = await driver.read(sample.path.input, { range: sample.range });
expect(fetch).toHaveBeenCalledWith(endpoint, {
headers: {

View File

@@ -1,4 +1,4 @@
import type { Driver, Range } from '@directus/storage';
import type { Driver, ReadOptions } from '@directus/storage';
import { normalizePath } from '@directus/utils';
import { StorageClient } from '@supabase/storage-js';
import { join } from 'node:path';
@@ -68,7 +68,9 @@ export class DriverSupabase implements Driver {
return `${this.endpoint}/${join('object/authenticated', this.config.bucket, this.fullPath(filepath))}`;
}
async read(filepath: string, range?: Range) {
async read(filepath: string, options?: ReadOptions) {
const { range } = options || {};
const requestInit: RequestInit = { method: 'GET' };
requestInit.headers = {

View File

@@ -41,6 +41,11 @@ export type Stat = {
modified: Date;
};
export type ReadOptions = {
range?: Range | undefined;
version?: string | undefined;
};
export type ChunkedUploadContext = {
size?: number | undefined;
metadata: Record<string, string | null> | undefined;
@@ -49,7 +54,7 @@ export type ChunkedUploadContext = {
export declare class Driver {
constructor(config: Record<string, unknown>);
read(filepath: string, range?: Range): Promise<Readable>;
read(filepath: string, options?: ReadOptions): Promise<Readable>;
write(filepath: string, content: Readable, type?: string): Promise<void>;
delete(filepath: string): Promise<void>;
stat(filepath: string): Promise<Stat>;

34
pnpm-lock.yaml generated
View File

@@ -15804,7 +15804,7 @@ snapshots:
'@tus/file-store@1.3.3':
dependencies:
'@tus/utils': 0.2.0
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
optionalDependencies:
'@redis/client': 1.6.0
transitivePeerDependencies:
@@ -15813,7 +15813,7 @@ snapshots:
'@tus/server@1.6.0':
dependencies:
'@tus/utils': 0.2.0
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
lodash.throttle: 4.1.1
optionalDependencies:
'@redis/client': 1.6.0
@@ -16310,7 +16310,7 @@ snapshots:
dependencies:
'@ampproject/remapping': 2.3.0
'@bcoe/v8-coverage': 0.2.3
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
istanbul-lib-coverage: 3.2.2
istanbul-lib-report: 3.0.1
istanbul-lib-source-maps: 5.0.6
@@ -16688,13 +16688,13 @@ snapshots:
agent-base@6.0.2:
dependencies:
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
transitivePeerDependencies:
- supports-color
agent-base@7.1.1:
dependencies:
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
transitivePeerDependencies:
- supports-color
@@ -17691,6 +17691,10 @@ snapshots:
dependencies:
ms: 2.1.2
debug@4.3.6:
dependencies:
ms: 2.1.2
debug@4.3.6(supports-color@5.5.0):
dependencies:
ms: 2.1.2
@@ -19027,7 +19031,7 @@ snapshots:
dependencies:
'@tootallnate/once': 1.1.2
agent-base: 6.0.2
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
transitivePeerDependencies:
- supports-color
optional: true
@@ -19036,14 +19040,14 @@ snapshots:
dependencies:
'@tootallnate/once': 2.0.0
agent-base: 6.0.2
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
transitivePeerDependencies:
- supports-color
http-proxy-agent@7.0.2:
dependencies:
agent-base: 7.1.1
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
transitivePeerDependencies:
- supports-color
@@ -19055,14 +19059,14 @@ snapshots:
https-proxy-agent@5.0.1:
dependencies:
agent-base: 6.0.2
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
transitivePeerDependencies:
- supports-color
https-proxy-agent@7.0.5:
dependencies:
agent-base: 7.1.1
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
transitivePeerDependencies:
- supports-color
@@ -19160,7 +19164,7 @@ snapshots:
dependencies:
'@ioredis/commands': 1.2.0
cluster-key-slot: 1.1.2
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
denque: 2.1.0
lodash.defaults: 4.2.0
lodash.isarguments: 3.1.0
@@ -19338,7 +19342,7 @@ snapshots:
istanbul-lib-source-maps@5.0.6:
dependencies:
'@jridgewell/trace-mapping': 0.3.25
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
istanbul-lib-coverage: 3.2.2
transitivePeerDependencies:
- supports-color
@@ -22284,7 +22288,7 @@ snapshots:
socks-proxy-agent@6.2.1:
dependencies:
agent-base: 6.0.2
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
socks: 2.8.3
transitivePeerDependencies:
- supports-color
@@ -23408,7 +23412,7 @@ snapshots:
vite-node@1.6.0(@types/node@18.19.50)(sass@1.78.0)(terser@5.31.6):
dependencies:
cac: 6.7.14
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
pathe: 1.1.2
picocolors: 1.1.0
vite: 5.2.11(@types/node@18.19.50)(sass@1.78.0)(terser@5.31.6)
@@ -23546,7 +23550,7 @@ snapshots:
'@vitest/utils': 1.6.0
acorn-walk: 8.3.3
chai: 4.5.0
debug: 4.3.6(supports-color@5.5.0)
debug: 4.3.6
execa: 8.0.1
local-pkg: 0.5.0
magic-string: 0.30.11