perf(twenty-server): drive immutable file caching from fileFolderConfigs on both serving paths (#22510)
Follow-up to #22166 (now merged). Single commit, rebased onto main. ## Why #22166 introduced a `Cache-Control` header for avatar responses, gated on a hardcoded `CACHEABLE_PICTURE_FILE_FOLDERS = [CorePicture]` list. Two limitations: - The list is an ad-hoc second classification of `FileFolder`, maintained separately from the central `fileFolderConfigs`. - The header is only set on the stream branch of `getFileById`. On S3 deployments with presigned URLs enabled, the controller 302-redirects before `setFileResponseHeaders` runs and the presigned S3 response carries no `Cache-Control` at all — so the header never fires where it matters most. Whether a folder's bytes are cacheable-forever is a property of how the folder is written, and the codebase already has a per-folder source of truth: `fileFolderConfigs`. ## What - Add `immutable: boolean` to `FileFolderConfig`. `true` for folders whose write paths mint a fresh `v4()` file id embedded in the resource path on every upload — so the bytes behind a given URL can never change: `CorePicture`, `FilesField`, `Workflow`, `AgentChat`, `EmailAttachment`, `Dpa`. `false` everywhere else, notably: - `PublicAsset` — path-addressed, overwritten in place on app (re)install (including the new manifest logo import) - `AppTarball` — reuses `tarballFileId` and a stable `${registrationId}/app.tar.gz` path across version bumps - `setFileResponseHeaders` reads the flag instead of the ad-hoc list (list deleted). - Thread `responseCacheControl` through `FileStorageService.getPresignedUrl` → `StorageDriver` → `S3Driver`, which passes it as `ResponseCacheControl` on the `GetObjectCommand`, so presigned S3 responses return the same `Cache-Control: private, max-age=86400, immutable` on the redirect path. `private` is kept because responses are gated by a per-workspace file token; `immutable` is safe because a changed file always gets a new id and URL. ## Tests - `setFileResponseHeaders` spec: header set for each immutable folder, not set for mutable folders (`PublicAsset`, `AppTarball`, deprecated picture folders) or when no folder is provided. - `S3Driver.getPresignedUrl` spec: asserts `ResponseCacheControl` is forwarded onto the `GetObjectCommand`. https://claude.ai/code/session_01AKwhTxYFDhWhCZ4b7sf35W
This commit is contained in:
+7
@@ -51,6 +51,7 @@ describe('S3Driver.getPresignedUrl', () => {
|
||||
filePath: 'file.png',
|
||||
responseContentType: 'image/png',
|
||||
responseContentDisposition: 'inline',
|
||||
responseCacheControl: 'private, max-age=86400, immutable',
|
||||
});
|
||||
|
||||
expect(result).toBe(
|
||||
@@ -61,6 +62,12 @@ describe('S3Driver.getPresignedUrl', () => {
|
||||
expect.any(GetObjectCommand),
|
||||
{ expiresIn: 900 },
|
||||
);
|
||||
|
||||
const command = (getSignedUrl as jest.Mock).mock.calls[0][1];
|
||||
|
||||
expect(command.input.ResponseCacheControl).toBe(
|
||||
'private, max-age=86400, immutable',
|
||||
);
|
||||
});
|
||||
|
||||
it('should presign with a separate client when endpoint override is provided', async () => {
|
||||
|
||||
+1
@@ -40,5 +40,6 @@ export interface StorageDriver {
|
||||
expiresInSeconds?: number;
|
||||
responseContentType?: string;
|
||||
responseContentDisposition?: string;
|
||||
responseCacheControl?: string;
|
||||
}): Promise<string | null>;
|
||||
}
|
||||
|
||||
@@ -385,6 +385,7 @@ export class S3Driver implements StorageDriver {
|
||||
expiresInSeconds?: number;
|
||||
responseContentType?: string;
|
||||
responseContentDisposition?: string;
|
||||
responseCacheControl?: string;
|
||||
}): Promise<string | null> {
|
||||
if (!this.presignClient) {
|
||||
return null;
|
||||
@@ -395,6 +396,7 @@ export class S3Driver implements StorageDriver {
|
||||
Key: params.filePath,
|
||||
ResponseContentType: params.responseContentType,
|
||||
ResponseContentDisposition: params.responseContentDisposition,
|
||||
ResponseCacheControl: params.responseCacheControl,
|
||||
});
|
||||
|
||||
return getSignedUrl(this.presignClient, command, {
|
||||
|
||||
+1
@@ -104,6 +104,7 @@ export class ValidatedStorageDriver implements StorageDriver {
|
||||
expiresInSeconds?: number;
|
||||
responseContentType?: string;
|
||||
responseContentDisposition?: string;
|
||||
responseCacheControl?: string;
|
||||
}): Promise<string | null> {
|
||||
assertStoragePathIsSafe(params.filePath);
|
||||
|
||||
|
||||
@@ -192,6 +192,7 @@ export class FileStorageService {
|
||||
expiresInSeconds?: number;
|
||||
responseContentType?: string;
|
||||
responseContentDisposition?: string;
|
||||
responseCacheControl?: string;
|
||||
},
|
||||
): Promise<string | null> {
|
||||
const driver = this.fileStorageDriverFactory.getCurrentDriver();
|
||||
@@ -203,6 +204,7 @@ export class FileStorageService {
|
||||
expiresInSeconds: params.expiresInSeconds,
|
||||
responseContentType: params.responseContentType,
|
||||
responseContentDisposition: params.responseContentDisposition,
|
||||
responseCacheControl: params.responseCacheControl,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
+21
@@ -9,62 +9,83 @@ registerEnumType(FileFolder, {
|
||||
|
||||
export type FileFolderConfig = {
|
||||
ignoreExpirationToken: boolean;
|
||||
immutable: boolean;
|
||||
};
|
||||
|
||||
export const IMMUTABLE_FILE_CACHE_CONTROL = 'private, max-age=86400, immutable';
|
||||
|
||||
export const fileFolderConfigs: Record<FileFolder, FileFolderConfig> = {
|
||||
[FileFolder.ProfilePicture]: {
|
||||
ignoreExpirationToken: true,
|
||||
immutable: false,
|
||||
},
|
||||
[FileFolder.WorkspaceLogo]: {
|
||||
ignoreExpirationToken: true,
|
||||
immutable: false,
|
||||
},
|
||||
[FileFolder.Attachment]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
},
|
||||
[FileFolder.PersonPicture]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
},
|
||||
[FileFolder.CorePicture]: {
|
||||
ignoreExpirationToken: true,
|
||||
immutable: true,
|
||||
},
|
||||
[FileFolder.File]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
},
|
||||
[FileFolder.AgentChat]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: true,
|
||||
},
|
||||
[FileFolder.BuiltLogicFunction]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
},
|
||||
[FileFolder.BuiltFrontComponent]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
},
|
||||
[FileFolder.PublicAsset]: {
|
||||
ignoreExpirationToken: true,
|
||||
immutable: false,
|
||||
},
|
||||
[FileFolder.Source]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
},
|
||||
[FileFolder.FilesField]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: true,
|
||||
},
|
||||
[FileFolder.Dependencies]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
},
|
||||
[FileFolder.Workflow]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: true,
|
||||
},
|
||||
[FileFolder.EmailAttachment]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: true,
|
||||
},
|
||||
[FileFolder.AppTarball]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
},
|
||||
[FileFolder.GeneratedSdkClient]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
},
|
||||
[FileFolder.Dpa]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: true,
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -13,6 +13,10 @@ import {
|
||||
FileStorageExceptionCode,
|
||||
} from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception';
|
||||
import { FileEntity } from 'src/engine/core-modules/file/entities/file.entity';
|
||||
import {
|
||||
fileFolderConfigs,
|
||||
IMMUTABLE_FILE_CACHE_CONTROL,
|
||||
} from 'src/engine/core-modules/file/interfaces/file-folder.interface';
|
||||
import { type FileResponse } from 'src/engine/core-modules/file/types/file-response.type';
|
||||
import { getContentDisposition } from 'src/engine/core-modules/file/utils/get-content-disposition.utils';
|
||||
import { removeFileFolderFromFileEntityPath } from 'src/engine/core-modules/file/utils/remove-file-folder-from-file-entity-path.utils';
|
||||
@@ -209,6 +213,9 @@ export class FileService {
|
||||
),
|
||||
responseContentType: mimeType,
|
||||
responseContentDisposition: getContentDisposition(mimeType),
|
||||
responseCacheControl: fileFolderConfigs[fileFolder].immutable
|
||||
? IMMUTABLE_FILE_CACHE_CONTROL
|
||||
: undefined,
|
||||
});
|
||||
|
||||
if (presignedUrl) {
|
||||
|
||||
+25
-19
@@ -86,38 +86,44 @@ describe('setFileResponseHeaders', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('should set an immutable Cache-Control for the CorePicture folder', () => {
|
||||
const res = createMockResponse();
|
||||
|
||||
setFileResponseHeaders(res as any, 'image/png', FileFolder.CorePicture);
|
||||
|
||||
expect(res.setHeader).toHaveBeenCalledWith(
|
||||
'Cache-Control',
|
||||
'private, max-age=86400, immutable',
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
FileFolder.ProfilePicture,
|
||||
FileFolder.WorkspaceLogo,
|
||||
FileFolder.PersonPicture,
|
||||
FileFolder.CorePicture,
|
||||
FileFolder.FilesField,
|
||||
FileFolder.Attachment,
|
||||
FileFolder.Workflow,
|
||||
FileFolder.PublicAsset,
|
||||
FileFolder.AgentChat,
|
||||
FileFolder.EmailAttachment,
|
||||
FileFolder.Dpa,
|
||||
])(
|
||||
'should not set Cache-Control for non-cacheable folder %s',
|
||||
'should set an immutable Cache-Control for immutable folder %s',
|
||||
(fileFolder) => {
|
||||
const res = createMockResponse();
|
||||
|
||||
setFileResponseHeaders(res as any, 'image/png', fileFolder);
|
||||
|
||||
expect(res.setHeader).not.toHaveBeenCalledWith(
|
||||
expect(res.setHeader).toHaveBeenCalledWith(
|
||||
'Cache-Control',
|
||||
expect.anything(),
|
||||
'private, max-age=86400, immutable',
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
FileFolder.ProfilePicture,
|
||||
FileFolder.WorkspaceLogo,
|
||||
FileFolder.PersonPicture,
|
||||
FileFolder.Attachment,
|
||||
FileFolder.PublicAsset,
|
||||
FileFolder.AppTarball,
|
||||
])('should not set Cache-Control for mutable folder %s', (fileFolder) => {
|
||||
const res = createMockResponse();
|
||||
|
||||
setFileResponseHeaders(res as any, 'image/png', fileFolder);
|
||||
|
||||
expect(res.setHeader).not.toHaveBeenCalledWith(
|
||||
'Cache-Control',
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getContentDisposition', () => {
|
||||
|
||||
+6
-9
@@ -3,12 +3,12 @@ import { type Response } from 'express';
|
||||
import { FileFolder } from 'twenty-shared/types';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import {
|
||||
fileFolderConfigs,
|
||||
IMMUTABLE_FILE_CACHE_CONTROL,
|
||||
} from 'src/engine/core-modules/file/interfaces/file-folder.interface';
|
||||
import { getContentDisposition } from 'src/engine/core-modules/file/utils/get-content-disposition.utils';
|
||||
|
||||
const CACHEABLE_PICTURE_FILE_FOLDERS: FileFolder[] = [FileFolder.CorePicture];
|
||||
|
||||
const PICTURE_CACHE_CONTROL = 'private, max-age=86400, immutable';
|
||||
|
||||
export const setFileResponseHeaders = (
|
||||
res: Response,
|
||||
mimeType: string,
|
||||
@@ -20,10 +20,7 @@ export const setFileResponseHeaders = (
|
||||
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||
res.setHeader('Content-Disposition', getContentDisposition(contentType));
|
||||
|
||||
if (
|
||||
isDefined(fileFolder) &&
|
||||
CACHEABLE_PICTURE_FILE_FOLDERS.includes(fileFolder)
|
||||
) {
|
||||
res.setHeader('Cache-Control', PICTURE_CACHE_CONTROL);
|
||||
if (isDefined(fileFolder) && fileFolderConfigs[fileFolder].immutable) {
|
||||
res.setHeader('Cache-Control', IMMUTABLE_FILE_CACHE_CONTROL);
|
||||
}
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user