Skip to content

Commit ac604b7

Browse files
authored
fix(files): share one served-bytes predicate between the serve route and large downloads (#9548)
* fix(files): share one served-bytes predicate between the serve route and large downloads * fix(files): judge an attachment's document name by its file record, state the stream helper's caller precondition
1 parent e407f28 commit ac604b7

21 files changed

Lines changed: 142 additions & 61 deletions

File tree

‎apps/sim/app/api/files/serve/[...path]/route.test.ts‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -571,17 +571,53 @@ describe('File Serve API Route', () => {
571571
describe('outside the workspace-file model', () => {
572572
const key = 'workspace/ws/1790000000000-0123456789abcdef-clip.mp4'
573573

574-
function serveAttachment(headers: Record<string, string> = {}) {
574+
function serveAttachment(headers: Record<string, string> = {}, objectKey = key) {
575575
mockIsUsingCloudStorage.mockReturnValue(true)
576576
storageServiceMockFns.mockDownloadFileStream.mockImplementation(async () =>
577577
Readable.from([Buffer.from('attachment')])
578578
)
579579
return GET(
580-
new NextRequest(`http://localhost:3000/api/files/serve/${key}`, { headers }),
581-
createRouteContext({ path: key.split('/') })
580+
new NextRequest(`http://localhost:3000/api/files/serve/${objectKey}`, { headers }),
581+
createRouteContext({ path: objectKey.split('/') })
582582
)
583583
}
584584

585+
describe('a document name', () => {
586+
const documentKey = 'workspace/ws/1790000000000-0123456789abcdef-scan.pdf'
587+
588+
beforeEach(() => {
589+
storageServiceMockFns.mockHeadObject.mockResolvedValue({ size: 10, version: '"v1"' })
590+
storageServiceMockFns.mockDownloadFile.mockResolvedValue(Buffer.from('rendered'))
591+
})
592+
593+
it('streams when its file record says it is a finished upload', async () => {
594+
uploadsMetadataMockFns.mockGetFileMetadataByKey.mockResolvedValue({
595+
key: documentKey,
596+
contentType: 'application/pdf',
597+
})
598+
599+
const response = await serveAttachment({}, documentKey)
600+
601+
expect(response.status).toBe(200)
602+
expect(storageServiceMockFns.mockDownloadFileStream).toHaveBeenCalled()
603+
expect(storageServiceMockFns.mockDownloadFile).not.toHaveBeenCalled()
604+
})
605+
606+
it.each([
607+
['its record says it is a generation source', { contentType: 'text/x-pdflibjs' }],
608+
['it has no file record', null],
609+
])('stays buffered when %s', async (_label, record) => {
610+
uploadsMetadataMockFns.mockGetFileMetadataByKey.mockResolvedValue(
611+
record && { key: documentKey, ...record }
612+
)
613+
614+
await serveAttachment({}, documentKey)
615+
616+
expect(storageServiceMockFns.mockDownloadFileStream).not.toHaveBeenCalled()
617+
expect(storageServiceMockFns.mockDownloadFile).toHaveBeenCalled()
618+
})
619+
})
620+
585621
it('streams a range of an attachment sized by storage, never buffering it', async () => {
586622
storageServiceMockFns.mockHeadObject.mockResolvedValue({
587623
size: 500_000_000,

‎apps/sim/app/api/files/serve/[...path]/route.ts‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,22 @@ import {
2828
workspaceFileCacheControl,
2929
} from '@/lib/uploads/server/delivery'
3030
import { resolveServableImageBytes } from '@/lib/uploads/server/image-derivative'
31-
import { resolveStoredFileContext } from '@/lib/uploads/server/metadata'
31+
import { getFileMetadataByKey, resolveStoredFileContext } from '@/lib/uploads/server/metadata'
3232
import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types'
3333
import {
3434
inferContextFromKey,
35-
needsRenderedArtifact,
35+
isRenderableDocumentName,
3636
resolveEffectiveMimeType,
3737
} from '@/lib/uploads/utils/file'
3838
import { internalWorkspaceFileServeAuth } from '@/lib/workspace-files/api'
3939
import { fetchAuthorizedServableWorkspaceFileBuffer } from '@/lib/workspace-files/application/fetch-servable-workspace-file-buffer'
4040
import { finishFileDelivery } from '@/lib/workspace-files/application/finish-file-delivery'
4141
import { readWorkspaceFileRecordByKey } from '@/lib/workspace-files/application/read-workspace-file-record-by-key'
4242
import { streamAuthorizedWorkspaceFile } from '@/lib/workspace-files/application/stream-authorized-workspace-file'
43-
import { isSimPageSource, SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-compile'
43+
import { isSimPageSource } from '@/lib/workspace-files/page-compile'
44+
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-content-type'
4445
import { renderSimPageDocumentWithAssets } from '@/lib/workspace-files/page-document.server'
46+
import { servesStoredBytes } from '@/lib/workspace-files/stored-bytes'
4547
import { type KnowledgeFileAccess, verifyFileAccess } from '@/app/api/files/authorization'
4648
import {
4749
createConditionalFileResponse,
@@ -203,10 +205,10 @@ async function resolveTransformedBytes(params: {
203205
* Whether a request is answered with the stored object byte-for-byte, so its bytes can stream from
204206
* storage instead of being read whole and capped at the transfer ceiling.
205207
*
206-
* Every other answer is produced from the bytes and stays buffered: a Sim page renders, a generated
207-
* document compiles, and a `preview` may be a HEIC transcode — sniffed from the content, so no name
208-
* or type rules it out. What streams is exactly what {@link resolveServableBytes} and the
209-
* workspace resolver would have passed through unchanged.
208+
* Every other answer is produced from the bytes and stays buffered: see {@link servesStoredBytes},
209+
* plus a `preview`, which may be a HEIC transcode — sniffed from the content, so no name or type
210+
* rules it out. What streams is exactly what {@link resolveServableBytes} and the workspace
211+
* resolver would have passed through unchanged.
210212
*/
211213
function servesStoredObject(
212214
filename: string,
@@ -215,12 +217,7 @@ function servesStoredObject(
215217
): boolean {
216218
if (options.raw) return true
217219
if (options.preview) return false
218-
if (recordedType === SIM_PAGE_CONTENT_TYPE || filename.toLowerCase().endsWith('.html')) {
219-
return false
220-
}
221-
// A recorded type tells a finished upload from a generation source; without one, every
222-
// document name may be a source.
223-
return !needsRenderedArtifact(recordedType, filename)
220+
return servesStoredBytes(recordedType, filename)
224221
}
225222

226223
/**
@@ -231,11 +228,26 @@ function storedObjectContentType(filename: string, options: ServeOptions): strin
231228
return options.raw ? getContentType(filename) : resolveEffectiveMimeType(undefined, filename)
232229
}
233230

231+
/**
232+
* The type recorded for an object outside the workspace-file model, read only where it decides the
233+
* answer: a document name may be a finished upload or a generation source, and the object's file
234+
* record, where it has one, says which. An object with no record is judged by its name.
235+
*/
236+
async function recordedDocumentType(
237+
key: string,
238+
context: StorageContext,
239+
filename: string,
240+
options: ServeOptions
241+
): Promise<string | undefined> {
242+
if (options.raw || options.preview || !isRenderableDocumentName(filename)) return undefined
243+
return (await getFileMetadataByKey(key, context))?.contentType
244+
}
245+
234246
/**
235247
* Streams a stored object outside the workspace-file model (execution outputs, chat and copilot
236248
* attachments, knowledge documents) when the request is answered with it as-is, or returns null
237-
* for the buffered path. The object is its only record, so storage is asked for its length and
238-
* version, and the version is what proves a revalidation still holds.
249+
* for the buffered path. Storage is asked for the object's length and version, and the version is
250+
* what proves a revalidation still holds.
239251
*/
240252
async function streamStoredObject(params: {
241253
key: string
@@ -245,7 +257,8 @@ async function streamStoredObject(params: {
245257
request: NextRequest
246258
}): Promise<NextResponse | null> {
247259
const { key, context, filename, options } = params
248-
if (!servesStoredObject(filename, undefined, options)) return null
260+
const recordedType = await recordedDocumentType(key, context, filename, options)
261+
if (!servesStoredObject(filename, recordedType, options)) return null
249262
const stored = await headObject(key, context)
250263
if (!stored) throw new FileNotFoundError(`File not found: ${key}`)
251264

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/text-editor.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ import {
2727
import type { FileDownloadSource } from '@/lib/uploads/client/download'
2828
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
2929
import { getFileExtension } from '@/lib/uploads/utils/file'
30-
import { isSimPageSource, SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-compile'
30+
import { isSimPageSource } from '@/lib/workspace-files/page-compile'
31+
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-content-type'
3132
import { EditorContextMenu } from '@/app/workspace/[workspaceId]/files/components/file-viewer/editor-context-menu'
3233
import { FileSaveConflict } from '@/app/workspace/[workspaceId]/files/components/file-viewer/file-save-conflict'
3334
import type { PreviewMode } from '@/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer'

‎apps/sim/app/workspace/[workspaceId]/files/files.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ import {
4848
isFileBrowserSize,
4949
isFileBrowserType,
5050
} from '@/lib/workspace-files/browser'
51-
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-compile'
51+
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-content-type'
5252
import { FindBar } from '@/app/workspace/[workspaceId]/components/find-bar/find-bar'
5353
import { useFindShortcut } from '@/app/workspace/[workspaceId]/components/find-bar/use-find-shortcut'
5454
import type { MoveOptionNode } from '@/app/workspace/[workspaceId]/components/folders'

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/mothership-view.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
} from '@/lib/mothership/resources/types'
1212
import type { FileDownloadSource } from '@/lib/uploads/client/download'
1313
import { getFileExtension } from '@/lib/uploads/utils/file'
14-
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-compile'
14+
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-content-type'
1515
import type { PreviewMode } from '@/app/workspace/[workspaceId]/files/components/file-viewer'
1616
import {
1717
isCsvStreamOnly,

‎apps/sim/lib/mothership/tools/server/files/edit-content.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ import {
2121
HAND_WRITTEN_PAGE_MESSAGE,
2222
isHandWrittenCompiledPage,
2323
isSimPageSource,
24-
SIM_PAGE_CONTENT_TYPE,
2524
} from '@/lib/workspace-files/page-compile'
25+
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-content-type'
2626
import { buildEmbeddedImageRefWarning } from './embedded-image-refs'
2727
import { waitForLatestFileIntent } from './intent-store'
2828
import { compileDocForWrite, getDocumentFormatInfo, inferContentType } from './workspace-file'

‎apps/sim/lib/projects/files/application/artifacts.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,8 @@ import {
5656
textExtractionUnsupportedError,
5757
} from '@/lib/workspace-files/application/read-workspace-file-text'
5858
import { fileOwnerCondition } from '@/lib/workspace-files/ownership-query'
59-
import {
60-
isSimPageSource,
61-
ProjectPageForeignEmbedError,
62-
SIM_PAGE_CONTENT_TYPE,
63-
} from '@/lib/workspace-files/page-compile'
59+
import { isSimPageSource, ProjectPageForeignEmbedError } from '@/lib/workspace-files/page-compile'
60+
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-content-type'
6461
import { renderSimPageDocument } from '@/lib/workspace-files/page-document'
6562
import {
6663
collectSimPageFileReferences,

‎apps/sim/lib/projects/files/application/content.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import {
4343
import { parseWorkspaceFileRevision } from '@/lib/workspace-files/application/file-revision'
4444
import { resolveWorkspaceFileVersionWrite } from '@/lib/workspace-files/application/file-version-write'
4545
import { MAX_WORKSPACE_FILE_CONTENT_BYTES } from '@/lib/workspace-files/orchestration'
46-
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-compile'
46+
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-content-type'
4747
import { restoreSimPageSourceBuffer } from '@/lib/workspace-files/page-source-embed'
4848

4949
interface ContentUseCaseArgs<I, P = undefined> {

‎apps/sim/lib/projects/files/application/downloads.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ import {
5959
} from '@/lib/workspace-files/download-selection'
6060
import { MAX_ZIP_DOWNLOAD_BYTES, MAX_ZIP_DOWNLOAD_FILES } from '@/lib/workspace-files/limits'
6161
import { fileOwnerCondition } from '@/lib/workspace-files/ownership-query'
62-
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-compile'
62+
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-content-type'
6363
import {
6464
createFileReadReceipt,
6565
type FileReadReceipt,

‎apps/sim/lib/projects/files/application/uploads.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ import {
5656
type UploadSessionRecord,
5757
} from '@/lib/uploads/upload-session/service'
5858
import type { WorkspaceFileUploadSource } from '@/lib/uploads/upload-session/workspace-file-provenance'
59-
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-compile'
59+
import { SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-content-type'
6060
import {
6161
MAX_SIM_PAGE_UPLOAD_SNIFF_BYTES,
6262
restoreSimPageSourceBuffer,

0 commit comments

Comments
 (0)