From f2e3eb5fb73358c8fa93fc9677d4b8d2965f8e71 Mon Sep 17 00:00:00 2001 From: Clive F Date: Fri, 3 Jul 2026 10:43:36 +0100 Subject: [PATCH] perf(twenty-server): browser-cache picture file responses (avatars/logos) (#22166) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #22163. ## Why Picture file responses (`GET /file/:fileFolder/:id`, `FileController.getFileById`) set no `Cache-Control` header, so the browser re-fetches the same avatar/picture on every render. When one member's avatar appears many times on a page (e.g. a record table or Kanban where that member owns many rows), this fires dozens of parallel GETs for the identical image; the browser cancels the redundant in-flight ones, and the server logs each client-aborted stream as `Error streaming file from storage`. Picture files are content-addressed by an immutable file id — changing an avatar or logo mints a new file id (and therefore a new URL) — so the bytes at any given URL never change and can be cached aggressively. ## What - `setFileResponseHeaders` now adds `Cache-Control: private, max-age=86400, immutable` for the picture folders (`CorePicture`, `ProfilePicture`, `WorkspaceLogo`, `PersonPicture`); `getFileById` passes the `fileFolder` through. - Scoped to picture folders so non-image files (attachments, tarballs, source, …) are not cached past a permission/visibility change. - `private` because files are served behind a per-workspace file token; `immutable` + the content-addressed id gives automatic cache-busting when the picture changes. ## Tests - Unit tests for `setFileResponseHeaders`: header is set for each picture folder, and not set for non-picture folders or when no folder is provided. - Controller test asserts the header on a `CorePicture` stream response. Review in cubic --------- Co-authored-by: Charles Bochet Co-authored-by: Félix Malfait --- .../file/controllers/file.controller.spec.ts | 4 ++ .../file/controllers/file.controller.ts | 2 +- .../file/file-url/file-url.service.ts | 39 ++++++++++++++-- .../set-file-response-headers.utils.spec.ts | 46 +++++++++++++++++++ .../utils/set-file-response-headers.utils.ts | 20 +++++++- 5 files changed, 105 insertions(+), 6 deletions(-) diff --git a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts index a114bfa3ab..afbb6a5f98 100644 --- a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts @@ -151,6 +151,10 @@ describe('FileController', () => { 'Content-Disposition', 'inline', ); + expect(mockResponse.setHeader).toHaveBeenCalledWith( + 'Cache-Control', + 'private, max-age=86400, immutable', + ); expect(mockPipeline).toHaveBeenCalledWith(mockStream, mockResponse); }); diff --git a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts index 1bab85e0b8..24e549cc86 100644 --- a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts @@ -152,7 +152,7 @@ export class FileController { return res.redirect(fileResponse.presignedUrl); } - setFileResponseHeaders(res, fileResponse.mimeType); + setFileResponseHeaders(res, fileResponse.mimeType, fileFolder); try { await pipeline(fileResponse.stream, res); diff --git a/packages/twenty-server/src/engine/core-modules/file/file-url/file-url.service.ts b/packages/twenty-server/src/engine/core-modules/file/file-url/file-url.service.ts index de7dee6455..b084619afe 100644 --- a/packages/twenty-server/src/engine/core-modules/file/file-url/file-url.service.ts +++ b/packages/twenty-server/src/engine/core-modules/file/file-url/file-url.service.ts @@ -11,6 +11,8 @@ import { type WorkspaceEntity } from 'src/engine/core-modules/workspace/workspac @Injectable() export class FileUrlService { + private readonly inflightFileUrlSignings = new Map>(); + constructor( private readonly jwtWrapperService: JwtWrapperService, private readonly twentyConfigService: TwentyConfigService, @@ -39,10 +41,39 @@ export class FileUrlService { workspaceId: string; fileFolder: FileFolder; }): Promise { - const fileTokenExpiresIn = this.twentyConfigService.get( - 'FILE_TOKEN_EXPIRES_IN', - ); + const signingCacheKey = `${workspaceId}:${fileFolder}:${fileId}`; + const inflightSigning = this.inflightFileUrlSignings.get(signingCacheKey); + if (isDefined(inflightSigning)) { + return inflightSigning; + } + + const signing = (async () => { + try { + return await this.buildSignedFileUrl({ + fileId, + workspaceId, + fileFolder, + }); + } finally { + this.inflightFileUrlSignings.delete(signingCacheKey); + } + })(); + + this.inflightFileUrlSignings.set(signingCacheKey, signing); + + return signing; + } + + private async buildSignedFileUrl({ + fileId, + workspaceId, + fileFolder, + }: { + fileId: string; + workspaceId: string; + fileFolder: FileFolder; + }): Promise { const payload: FileTokenJwtPayload = { workspaceId, fileId, @@ -51,7 +82,7 @@ export class FileUrlService { }; const token = await this.jwtWrapperService.signAsyncOrThrow(payload, { - expiresIn: fileTokenExpiresIn, + expiresIn: this.twentyConfigService.get('FILE_TOKEN_EXPIRES_IN'), }); const serverUrl = this.twentyConfigService.get('SERVER_URL'); diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/set-file-response-headers.utils.spec.ts b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/set-file-response-headers.utils.spec.ts index d601f86fcd..7d9ceef396 100644 --- a/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/set-file-response-headers.utils.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/set-file-response-headers.utils.spec.ts @@ -1,3 +1,5 @@ +import { FileFolder } from 'twenty-shared/types'; + import { getContentDisposition } from 'src/engine/core-modules/file/utils/get-content-disposition.utils'; import { setFileResponseHeaders } from 'src/engine/core-modules/file/utils/set-file-response-headers.utils'; @@ -72,6 +74,50 @@ describe('setFileResponseHeaders', () => { ); }, ); + + it('should not set Cache-Control when no fileFolder is provided', () => { + const res = createMockResponse(); + + setFileResponseHeaders(res as any, 'image/png'); + + expect(res.setHeader).not.toHaveBeenCalledWith( + 'Cache-Control', + expect.anything(), + ); + }); + + 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.FilesField, + FileFolder.Attachment, + FileFolder.Workflow, + FileFolder.PublicAsset, + ])( + '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', () => { diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/set-file-response-headers.utils.ts b/packages/twenty-server/src/engine/core-modules/file/utils/set-file-response-headers.utils.ts index 026340fa49..124af59316 100644 --- a/packages/twenty-server/src/engine/core-modules/file/utils/set-file-response-headers.utils.ts +++ b/packages/twenty-server/src/engine/core-modules/file/utils/set-file-response-headers.utils.ts @@ -1,11 +1,29 @@ import { type Response } from 'express'; +import { FileFolder } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; + import { getContentDisposition } from 'src/engine/core-modules/file/utils/get-content-disposition.utils'; -export const setFileResponseHeaders = (res: Response, mimeType: string) => { +const CACHEABLE_PICTURE_FILE_FOLDERS: FileFolder[] = [FileFolder.CorePicture]; + +const PICTURE_CACHE_CONTROL = 'private, max-age=86400, immutable'; + +export const setFileResponseHeaders = ( + res: Response, + mimeType: string, + fileFolder?: FileFolder, +) => { const contentType = mimeType || 'application/octet-stream'; res.setHeader('Content-Type', contentType); 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); + } };