fix(files): resolve metadata version from the record's own storage key

Author: waleedlatif1

apps/sim/lib/uploads/contexts/workspace/__integration__/file-versions.integration.ts

@@ -38,6 +38,7 @@ import {
 import { WORKSPACE_FILE_STORAGE_CLEANUP_OUTBOX_EVENT } from '@/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox'
 import {
   getCurrentWorkspaceFileVersion,
+  getWorkspaceFileVersionNumberForRecord,
   queryWorkspaceFileVersions,
   releaseWorkspaceFileVersionsForPurgeInTx,
 } from '@/lib/uploads/contexts/workspace/workspace-file-versions'
@@ -227,6 +228,8 @@ describe('workspace file version history in PostgreSQL', () => {
     const after = await getWorkspaceFile(fixture.workspaceId, fixture.fileId)
     if (!after) throw new Error('file missing')
     expect((await getCurrentWorkspaceFileVersion(after)).version).toBe(2)
+    expect(await getWorkspaceFileVersionNumberForRecord(before)).toBe(1)
+    expect(await getWorkspaceFileVersionNumberForRecord(after)).toBe(2)
   })
 
   it('does not keep an empty shell as a version of its own', async () => {

apps/sim/lib/uploads/contexts/workspace/workspace-file-versions.ts

@@ -497,6 +497,24 @@ export async function getCurrentWorkspaceFileVersion(
   return head ? toVersionRecord(head) : implicitFirstVersion(file)
 }
 
+/**
+ * The version number of the content a file record describes. Matched by the record's storage key, so
+ * it stays exact even if a write committed after the record was read: that record's key still names
+ * its own version row. A key with no row is either a file with no history (implicit version 1) or
+ * bytes a collaborative write replaced within the same version, which keeps the head's number.
+ */
+export async function getWorkspaceFileVersionNumberForRecord(
+  file: Pick<WorkspaceFileVersionSubject, 'id' | 'key'>
+): Promise<number> {
+  const [row] = await db
+    .select({ version: workspaceFileVersion.version })
+    .from(workspaceFileVersion)
+    .where(and(eq(workspaceFileVersion.fileId, file.id), eq(workspaceFileVersion.key, file.key)))
+    .limit(1)
+  if (row) return row.version
+  return (await loadWorkspaceFileVersionHead(file.id))?.version ?? 1
+}
+
 /** One version of a file, or null when it never existed or retention removed it. */
 export async function getWorkspaceFileVersion(
   file: WorkspaceFileVersionSubject,

apps/sim/lib/workspace-files/application/read-workspace-file-metadata.test.ts

@@ -8,11 +8,11 @@ const mocks = vi.hoisted(() => ({
   getWorkspaceFile: vi.fn(),
   getShareForResource: vi.fn(),
   resolvePermission: vi.fn(),
-  getCurrentVersion: vi.fn(),
+  getVersionNumberForRecord: vi.fn(),
 }))
 
 vi.mock('@/lib/uploads/contexts/workspace/workspace-file-versions', () => ({
-  getCurrentWorkspaceFileVersion: mocks.getCurrentVersion,
+  getWorkspaceFileVersionNumberForRecord: mocks.getVersionNumberForRecord,
 }))
 
 vi.mock('@sim/platform-authz/workspace', () => ({
@@ -177,19 +177,16 @@ describe('readWorkspaceFileMetadataWithVersion', () => {
     mocks.resolvePermission.mockResolvedValue('admin')
   })
 
-  it('pairs the version number with the stored object the returned record describes', async () => {
-    const rewritten = { ...file, key: 'workspace/ws/data-2.csv', size: 50 }
-    mocks.getWorkspaceFile.mockResolvedValueOnce(file).mockResolvedValueOnce(rewritten)
-    mocks.getCurrentVersion
-      .mockResolvedValueOnce({ version: 2, key: rewritten.key })
-      .mockResolvedValueOnce({ version: 2, key: rewritten.key })
+  it('reports the version of the stored object the returned record describes', async () => {
+    mocks.getWorkspaceFile.mockResolvedValueOnce(file)
+    mocks.getVersionNumberForRecord.mockResolvedValueOnce(4)
 
     await expect(
       readWorkspaceFileMetadataWithVersion.execute({
         principal,
         input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' },
       })
-    ).resolves.toEqual({ file: rewritten, share, currentVersion: 2 })
-    expect(mocks.getWorkspaceFile).toHaveBeenCalledTimes(2)
+    ).resolves.toEqual({ file, share, currentVersion: 4 })
+    expect(mocks.getVersionNumberForRecord).toHaveBeenCalledWith(file)
   })
 })

apps/sim/lib/workspace-files/application/read-workspace-file-metadata.ts

@@ -7,7 +7,7 @@ import {
   getWorkspaceFile,
   type WorkspaceFileRecord,
 } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
-import { getCurrentWorkspaceFileVersion } from '@/lib/uploads/contexts/workspace/workspace-file-versions'
+import { getWorkspaceFileVersionNumberForRecord } from '@/lib/uploads/contexts/workspace/workspace-file-versions'
 import { defineAuthorizedWorkspaceFileUseCase } from '@/lib/workspace-files/application/authorized-workspace-file-use-case'
 import { fileOperations } from '@/lib/workspace-files/application/operations'
 import { resolveActiveWorkspaceFileContext } from '@/lib/workspace-files/application/workspace-file-context'
@@ -59,28 +59,18 @@ export const readWorkspaceFileMetadata = defineAuthorizedWorkspaceFileUseCase({
   execute: executeReadWorkspaceFileMetadata,
 })
 
-/** Attempts to read a file record and version head that describe the same stored object. */
-const CURRENT_VERSION_READ_ATTEMPTS = 3
-
 /**
  * The same read plus the current version number, for the public metadata surface. Kept separate so
- * the many internal callers of {@link readWorkspaceFileMetadata} pay no extra query.
- *
- * The file row and the version head are separate reads, so a content write committing between them
- * would pair the old key and size with the new number; the read repeats until both describe the
- * same stored object.
+ * the many internal callers of {@link readWorkspaceFileMetadata} pay no extra query. The number is
+ * resolved from the returned record's own storage key, so it always identifies the content that
+ * record describes.
  */
 export const readWorkspaceFileMetadataWithVersion = defineAuthorizedWorkspaceFileUseCase({
   operation: fileOperations.readMetadata,
   resolveContext: ({ input }: { input: ReadWorkspaceFileMetadataInput }) =>
     resolveActiveWorkspaceFileContext(input),
   async execute(args): Promise<ReadWorkspaceFileMetadataWithVersionResult> {
-    for (let attempt = 1; ; attempt++) {
-      const result = await executeReadWorkspaceFileMetadata(args)
-      const current = await getCurrentWorkspaceFileVersion(result.file)
-      if (current.key === result.file.key || attempt === CURRENT_VERSION_READ_ATTEMPTS) {
-        return { ...result, currentVersion: current.version }
-      }
-    }
+    const result = await executeReadWorkspaceFileMetadata(args)
+    return { ...result, currentVersion: await getWorkspaceFileVersionNumberForRecord(result.file) }
   },
 })