perf(twenty-server): cache BuiltFrontComponent and PublicAsset responses (#22523)
Follow-up to #22510. Closes #22515 — extends `Cache-Control` to the two remaining app-asset folders that #22510 left `immutable: false` because they're path-addressed. Each now gets the directive that matches **how it is addressed**. ## BuiltFrontComponent → immutable I was wrong in the #22515 write-up to call this "stable URL, mutable bytes." The browser **already content-addresses it**: `FrontComponentRenderer` fetches `/rest/front-components/:id?checksum=${builtComponentChecksum}` (`getFrontComponentUrl`), so a rebuild changes the checksum → changes the URL → busts the cache. That makes `immutable` safe — no stale-code window — and needs no new versioning machinery. Wired the header into `FrontComponentController.getBuiltJs` (which passed no folder) and the front-component presign path. ## PublicAsset → bounded public cache Genuinely path-addressed and overwritten in place on every app (re)install/redeploy (upsert on `['path','workspaceId','applicationId']`), so it **cannot** be `immutable`. Instead: - **`public`** — the `/public-assets/...` endpoint is unauthenticated (`PublicEndpointGuard`), so the bytes are already world-readable; marking the response `public` lets a CDN (e.g. Cloudflare in front of the server) serve app/marketplace logos from the edge instead of hitting the origin on every render. Today these responses carry no `Cache-Control` at all. - **`max-age=3600`, not `immutable`** — a bounded window so an asset overwrite recovers within an hour. This one hour is the single judgement call here; tune it (or add `stale-while-revalidate`) to taste. ## Mechanism Generalized `FileFolderConfig.immutable` (boolean) into `cacheControl` (`string | null`) so a folder can carry its own directive instead of only opting into one hardcoded string. `setFileResponseHeaders` and the presign paths now read `cacheControl` directly. The immutable-folder set is unchanged; only BuiltFrontComponent (→ immutable) and PublicAsset (→ bounded public) move. ## Tests `setFileResponseHeaders` spec updated: BuiltFrontComponent now asserts immutable, PublicAsset asserts `public, max-age=3600`, and the remaining path-addressed folders (`AppTarball`, `Source`, `BuiltLogicFunction`, `Dependencies`) assert no `Cache-Control`. _Note: I bundled both folders into one PR since they share the config generalization — happy to split BuiltFrontComponent (safe/immutable) from PublicAsset (the `max-age` judgement call) if you'd rather review them separately._ https://claude.ai/code/session_01AKwhTxYFDhWhCZ4b7sf35W --- _Generated by [Claude Code](https://claude.ai/code/session_01AKwhTxYFDhWhCZ4b7sf35W)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22523?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
@@ -92,7 +92,7 @@ export class FileController {
|
||||
return res.redirect(fileResponse.presignedUrl);
|
||||
}
|
||||
|
||||
setFileResponseHeaders(res, fileResponse.mimeType);
|
||||
setFileResponseHeaders(res, fileResponse.mimeType, FileFolder.PublicAsset);
|
||||
|
||||
try {
|
||||
await pipeline(fileResponse.stream, res);
|
||||
|
||||
+16
-14
@@ -8,62 +8,64 @@ registerEnumType(FileFolder, {
|
||||
|
||||
export type FileFolderConfig = {
|
||||
ignoreExpirationToken: boolean;
|
||||
immutable: boolean;
|
||||
cacheControl: string | null;
|
||||
};
|
||||
|
||||
export const IMMUTABLE_FILE_CACHE_CONTROL = 'private, max-age=86400, immutable';
|
||||
|
||||
export const PUBLIC_ASSET_CACHE_CONTROL = 'public, max-age=3600';
|
||||
|
||||
export const fileFolderConfigs: Record<FileFolder, FileFolderConfig> = {
|
||||
[FileFolder.CorePicture]: {
|
||||
ignoreExpirationToken: true,
|
||||
immutable: true,
|
||||
cacheControl: IMMUTABLE_FILE_CACHE_CONTROL,
|
||||
},
|
||||
[FileFolder.AgentChat]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: true,
|
||||
cacheControl: IMMUTABLE_FILE_CACHE_CONTROL,
|
||||
},
|
||||
[FileFolder.BuiltLogicFunction]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
cacheControl: null,
|
||||
},
|
||||
[FileFolder.BuiltFrontComponent]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
cacheControl: IMMUTABLE_FILE_CACHE_CONTROL,
|
||||
},
|
||||
[FileFolder.PublicAsset]: {
|
||||
ignoreExpirationToken: true,
|
||||
immutable: false,
|
||||
cacheControl: PUBLIC_ASSET_CACHE_CONTROL,
|
||||
},
|
||||
[FileFolder.Source]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
cacheControl: null,
|
||||
},
|
||||
[FileFolder.FilesField]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: true,
|
||||
cacheControl: IMMUTABLE_FILE_CACHE_CONTROL,
|
||||
},
|
||||
[FileFolder.Dependencies]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
cacheControl: null,
|
||||
},
|
||||
[FileFolder.Workflow]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: true,
|
||||
cacheControl: IMMUTABLE_FILE_CACHE_CONTROL,
|
||||
},
|
||||
[FileFolder.EmailAttachment]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: true,
|
||||
cacheControl: IMMUTABLE_FILE_CACHE_CONTROL,
|
||||
},
|
||||
[FileFolder.AppTarball]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
cacheControl: null,
|
||||
},
|
||||
[FileFolder.GeneratedSdkClient]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: false,
|
||||
cacheControl: null,
|
||||
},
|
||||
[FileFolder.Dpa]: {
|
||||
ignoreExpirationToken: false,
|
||||
immutable: true,
|
||||
cacheControl: IMMUTABLE_FILE_CACHE_CONTROL,
|
||||
},
|
||||
};
|
||||
|
||||
@@ -13,10 +13,7 @@ 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 { fileFolderConfigs } from 'src/engine/core-modules/file/interfaces/file-folder.interface';
|
||||
import { type FileResponse } from 'src/engine/core-modules/file/types/file-response.type';
|
||||
import { FILE_STATUS } from 'src/engine/core-modules/file/types/file-status.types';
|
||||
import { getContentDisposition } from 'src/engine/core-modules/file/utils/get-content-disposition.utils';
|
||||
@@ -217,9 +214,8 @@ export class FileService {
|
||||
),
|
||||
responseContentType: mimeType,
|
||||
responseContentDisposition: getContentDisposition(mimeType),
|
||||
responseCacheControl: fileFolderConfigs[fileFolder].immutable
|
||||
? IMMUTABLE_FILE_CACHE_CONTROL
|
||||
: undefined,
|
||||
responseCacheControl:
|
||||
fileFolderConfigs[fileFolder].cacheControl ?? undefined,
|
||||
});
|
||||
|
||||
if (presignedUrl) {
|
||||
|
||||
+25
-9
@@ -93,6 +93,7 @@ describe('setFileResponseHeaders', () => {
|
||||
FileFolder.AgentChat,
|
||||
FileFolder.EmailAttachment,
|
||||
FileFolder.Dpa,
|
||||
FileFolder.BuiltFrontComponent,
|
||||
])(
|
||||
'should set an immutable Cache-Control for immutable folder %s',
|
||||
(fileFolder) => {
|
||||
@@ -107,21 +108,36 @@ describe('setFileResponseHeaders', () => {
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
FileFolder.PublicAsset,
|
||||
FileFolder.AppTarball,
|
||||
FileFolder.Source,
|
||||
FileFolder.BuiltFrontComponent,
|
||||
])('should not set Cache-Control for mutable folder %s', (fileFolder) => {
|
||||
it('should set a bounded public Cache-Control for the PublicAsset folder', () => {
|
||||
const res = createMockResponse();
|
||||
|
||||
setFileResponseHeaders(res as any, 'image/png', fileFolder);
|
||||
setFileResponseHeaders(res as any, 'image/png', FileFolder.PublicAsset);
|
||||
|
||||
expect(res.setHeader).not.toHaveBeenCalledWith(
|
||||
expect(res.setHeader).toHaveBeenCalledWith(
|
||||
'Cache-Control',
|
||||
expect.anything(),
|
||||
'public, max-age=3600',
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
FileFolder.AppTarball,
|
||||
FileFolder.Source,
|
||||
FileFolder.BuiltLogicFunction,
|
||||
FileFolder.Dependencies,
|
||||
FileFolder.GeneratedSdkClient,
|
||||
])(
|
||||
'should not set Cache-Control for non-cacheable folder %s',
|
||||
(fileFolder) => {
|
||||
const res = createMockResponse();
|
||||
|
||||
setFileResponseHeaders(res as any, 'image/png', fileFolder);
|
||||
|
||||
expect(res.setHeader).not.toHaveBeenCalledWith(
|
||||
'Cache-Control',
|
||||
expect.anything(),
|
||||
);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe('getContentDisposition', () => {
|
||||
|
||||
+7
-6
@@ -3,10 +3,7 @@ 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 { fileFolderConfigs } from 'src/engine/core-modules/file/interfaces/file-folder.interface';
|
||||
import { getContentDisposition } from 'src/engine/core-modules/file/utils/get-content-disposition.utils';
|
||||
|
||||
export const setFileResponseHeaders = (
|
||||
@@ -20,7 +17,11 @@ export const setFileResponseHeaders = (
|
||||
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||
res.setHeader('Content-Disposition', getContentDisposition(contentType));
|
||||
|
||||
if (isDefined(fileFolder) && fileFolderConfigs[fileFolder].immutable) {
|
||||
res.setHeader('Cache-Control', IMMUTABLE_FILE_CACHE_CONTROL);
|
||||
const cacheControl = isDefined(fileFolder)
|
||||
? fileFolderConfigs[fileFolder].cacheControl
|
||||
: null;
|
||||
|
||||
if (isDefined(cacheControl)) {
|
||||
res.setHeader('Cache-Control', cacheControl);
|
||||
}
|
||||
};
|
||||
|
||||
+6
-1
@@ -11,6 +11,7 @@ import {
|
||||
import { pipeline } from 'stream/promises';
|
||||
|
||||
import { Response } from 'express';
|
||||
import { FileFolder } from 'twenty-shared/types';
|
||||
|
||||
import {
|
||||
FileStorageException,
|
||||
@@ -87,7 +88,11 @@ export class FrontComponentController {
|
||||
return res.redirect(fileResponse.presignedUrl);
|
||||
}
|
||||
|
||||
setFileResponseHeaders(res, fileResponse.mimeType);
|
||||
setFileResponseHeaders(
|
||||
res,
|
||||
fileResponse.mimeType,
|
||||
FileFolder.BuiltFrontComponent,
|
||||
);
|
||||
|
||||
try {
|
||||
await pipeline(fileResponse.stream, res);
|
||||
|
||||
+4
@@ -6,6 +6,7 @@ import { isDefined } from 'twenty-shared/utils';
|
||||
import { ApplicationService } from 'src/engine/core-modules/application/application.service';
|
||||
import { type FlatApplication } from 'src/engine/core-modules/application/types/flat-application.type';
|
||||
import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service';
|
||||
import { fileFolderConfigs } 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 { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
|
||||
@@ -332,6 +333,9 @@ export class FrontComponentService {
|
||||
),
|
||||
responseContentType: mimeType,
|
||||
responseContentDisposition: getContentDisposition(mimeType),
|
||||
responseCacheControl:
|
||||
fileFolderConfigs[FileFolder.BuiltFrontComponent].cacheControl ??
|
||||
undefined,
|
||||
});
|
||||
|
||||
if (presignedUrl) {
|
||||
|
||||
Reference in New Issue
Block a user