diff --git a/src/storage/version/list.js b/src/storage/version/list.js index 3d6196b7..342157d3 100644 --- a/src/storage/version/list.js +++ b/src/storage/version/list.js @@ -24,14 +24,8 @@ function orgListFromEnv(env, name) { return new Set(raw.split(',').map((s) => s.trim()).filter(Boolean)); } -/** Org uses audit.txt as the version list source (new feature). */ -function orgUsesAuditFileList(env, org) { - return orgListFromEnv(env, 'VERSIONS_AUDIT_FILE_ORGS').has(org); -} - /** - * With audit-file feature: skip reading org/.da-versions (after migration). - * Only applies when org is also in VERSIONS_AUDIT_FILE_ORGS. + * Skip reading org/.da-versions (after migration is complete for this org). */ function orgSkipsLegacy(env, org) { return orgListFromEnv(env, 'VERSIONS_AUDIT_SKIP_LEGACY_ORGS').has(org); @@ -127,56 +121,45 @@ function mergeLegacyAndNewResult(legacyResult, newResult) { export async function listObjectVersions(env, { bucket, org, key }) { const current = await getObject(env, { bucket, org, key }, true); - if (current.status === 404 || !current.metadata.id) { + const repo = key.includes('/') ? key.split('/')[0] : ''; + + if (current.status === 404 || !current.metadata.id || !repo) { return 404; } const fileId = current.metadata.id; - const repo = key.includes('/') ? key.split('/')[0] : ''; - - if (repo && orgUsesAuditFileList(env, org)) { - let auditLines = []; - try { - auditLines = await readAuditLines(env, { bucket, org }, repo, fileId); - } catch { - // no audit - } - const ext = fileExt(key); - const auditEntries = buildEntriesFromAudit(auditLines, repo, org, fileId, ext); - auditEntries.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); - auditEntries.splice(MAX_VERSIONS); - const auditResult = { - status: 200, - contentType: 'application/json', - body: JSON.stringify(auditEntries), - }; - if (orgSkipsLegacy(env, org)) { - versionListModeLog({ - mode: 'audit_file', - org, - key, - fileId, - legacy: 'skipped', - }); - return auditResult; - } - const legacyResult = await listFromLegacyStructure(env, { bucket, org, key }, fileId); + let auditLines = []; + try { + auditLines = await readAuditLines(env, { bucket, org }, repo, fileId); + } catch { + // no audit + } + const ext = fileExt(key); + const auditEntries = buildEntriesFromAudit(auditLines, repo, org, fileId, ext); + auditEntries.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); + auditEntries.splice(MAX_VERSIONS); + const auditResult = { + status: 200, + contentType: 'application/json', + body: JSON.stringify(auditEntries), + }; + if (orgSkipsLegacy(env, org)) { versionListModeLog({ mode: 'audit_file', org, key, fileId, - legacy: 'merged', + legacy: 'skipped', }); - return mergeLegacyAndNewResult(legacyResult, auditResult); + return auditResult; } - + const legacyResult = await listFromLegacyStructure(env, { bucket, org, key }, fileId); versionListModeLog({ - mode: 'legacy', + mode: 'audit_file', org, key, fileId, - detail: 'org_root_da_versions_only', + legacy: 'merged', }); - return listFromLegacyStructure(env, { bucket, org, key }, fileId); + return mergeLegacyAndNewResult(legacyResult, auditResult); } diff --git a/src/storage/version/put.js b/src/storage/version/put.js index d4786d70..13b4ae32 100644 --- a/src/storage/version/put.js +++ b/src/storage/version/put.js @@ -211,10 +211,6 @@ export async function putObjectWithVersion( const Preparsingstore = storeBody ? Timestamp : pps; - const usesAuditFile = new Set( - (env?.VERSIONS_AUDIT_FILE_ORGS || '').split(',').map((s) => s.trim()).filter(Boolean), - ).has(daCtx.org); - // Only create version for explicit label (POST /versionsource) or Restore Point. No Collab Parse. const shouldCreateVersionObject = createVersion && (update.label != null || Label === 'Restore Point'); @@ -223,7 +219,7 @@ export async function putObjectWithVersion( const versionResp = await putVersion(config, { Bucket: input.Bucket, Org: daCtx.org, - Repo: usesAuditFile ? (daCtx.site || undefined) : undefined, + Repo: daCtx.site || undefined, Body: (body || storeBody ? current.body : ''), ContentLength: (body || storeBody ? current.contentLength : undefined), ContentType: current.contentType, @@ -247,50 +243,29 @@ export async function putObjectWithVersion( // Audit: one entry per versionable PUT; versionLabel + versionId when labelled version created. // Store path without repo prefix and versionId without extension for readability. if (createVersion) { - if (usesAuditFile) { - const versionId = versionCreated ? Version : undefined; - const versionLabel = versionCreated ? (Label ?? '') : undefined; - const pathForAudit = (daCtx.site && Path.startsWith(`${daCtx.site}/`)) - ? Path.slice(daCtx.site.length) - : Path; - let auditErr; - for (let i = 0; i < AUDIT_WRITE_RETRIES; i += 1) { - try { - // eslint-disable-next-line no-await-in-loop - await writeAuditEntry(env, { bucket: input.Bucket, org: daCtx.org }, daCtx.site, ID, { - timestamp: Timestamp, - users: Users, - path: pathForAudit, - versionLabel, - versionId, - }); - auditErr = null; - break; - } catch (e) { auditErr = e; } - } - if (auditErr) { - // eslint-disable-next-line no-console - console.error(`Failed to write audit entry after ${AUDIT_WRITE_RETRIES} retries`, auditErr); - } - } else if (!shouldCreateVersionObject) { - // Legacy path: write an empty version object so listFromLegacyStructure can find it. - // Only needed when no snapshot was created — the snapshot itself serves as the marker. - await putVersion(config, { - Bucket: input.Bucket, - Org: daCtx.org, - Body: '', - ContentLength: 0, - ContentType: current.contentType, - ID, - Version, - Ext: daCtx.ext, - Metadata: { - Users, - Timestamp, - Path, - Label: Label ?? '', - }, - }, false); + const versionId = versionCreated ? Version : undefined; + const versionLabel = versionCreated ? (Label ?? '') : undefined; + const pathForAudit = (daCtx.site && Path.startsWith(`${daCtx.site}/`)) + ? Path.slice(daCtx.site.length) + : Path; + let auditErr; + for (let i = 0; i < AUDIT_WRITE_RETRIES; i += 1) { + try { + // eslint-disable-next-line no-await-in-loop + await writeAuditEntry(env, { bucket: input.Bucket, org: daCtx.org }, daCtx.site, ID, { + timestamp: Timestamp, + users: Users, + path: pathForAudit, + versionLabel, + versionId, + }); + auditErr = null; + break; + } catch (e) { auditErr = e; } + } + if (auditErr) { + // eslint-disable-next-line no-console + console.error(`Failed to write audit entry after ${AUDIT_WRITE_RETRIES} retries`, auditErr); } } diff --git a/test/storage/object/conditionals.test.js b/test/storage/object/conditionals.test.js index 23f5e58f..c9223a62 100644 --- a/test/storage/object/conditionals.test.js +++ b/test/storage/object/conditionals.test.js @@ -252,7 +252,7 @@ describe('Conditional Headers', () => { const clientConditionals = { ifMatch: '"wrongetag"' }; const resp = await putObjectWithVersion( - { VERSIONS_AUDIT_FILE_ORGS: ORG }, + {}, daCtx, update, false, diff --git a/test/storage/version/list.test.js b/test/storage/version/list.test.js index 44a9cbad..833b9cf1 100644 --- a/test/storage/version/list.test.js +++ b/test/storage/version/list.test.js @@ -15,64 +15,48 @@ import esmock from 'esmock'; describe('Version List', () => { it('should return 404 when current object does not exist', async () => { - const mockGetObject = async () => ({ - status: 404, - metadata: {}, - }); - + const mockGetObject = async () => ({ status: 404, metadata: {} }); const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { - default: mockGetObject, - }, + '../../../src/storage/object/get.js': { default: mockGetObject }, }); - - const result = await listObjectVersions({}, { bucket: 'test', org: 'org', key: 'file.html' }); + const result = await listObjectVersions({}, { bucket: 'test', org: 'org', key: 'repo/file.html' }); assert.strictEqual(result, 404); }); it('should return 404 when current object has no id', async () => { - const mockGetObject = async () => ({ - status: 200, - metadata: {}, + const mockGetObject = async () => ({ status: 200, metadata: {} }); + const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { + '../../../src/storage/object/get.js': { default: mockGetObject }, }); + const result = await listObjectVersions({}, { bucket: 'test', org: 'org', key: 'repo/file.html' }); + assert.strictEqual(result, 404); + }); + it('should return 404 when key has no repo prefix', async () => { + const mockGetObject = async () => ({ status: 200, metadata: { id: 'test-id' } }); const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { - default: mockGetObject, - }, + '../../../src/storage/object/get.js': { default: mockGetObject }, }); - const result = await listObjectVersions({}, { bucket: 'test', org: 'org', key: 'file.html' }); assert.strictEqual(result, 404); }); - it('should return error when list objects fails', async () => { - const mockGetObject = async () => ({ - status: 200, - metadata: { id: 'test-id-123' }, - }); - - const mockListObjects = async () => ({ - status: 500, - body: '[]', - }); - + it('should degrade gracefully when list objects fails (legacy merge returns audit result)', async () => { + const mockGetObject = async () => ({ status: 200, metadata: { id: 'test-id-123' } }); + const mockListObjects = async () => ({ status: 500, body: '[]' }); const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { - default: mockGetObject, - }, - '../../../src/storage/object/list.js': { - default: mockListObjects, - }, + '../../../src/storage/object/get.js': { default: mockGetObject }, + '../../../src/storage/object/list.js': { default: mockListObjects }, + '../../../src/storage/version/audit.js': { readAuditLines: async () => [] }, }); - - const result = await listObjectVersions({}, { bucket: 'test', org: 'org', key: 'file.html' }); - assert.strictEqual(result.status, 500); + const result = await listObjectVersions({}, { bucket: 'test', org: 'org', key: 'repo/file.html' }); + assert.strictEqual(result.status, 200); + assert.strictEqual(result.body, '[]'); }); it('should list versions with basic metadata', async () => { const mockGetObject = async (env, { key }, metadataOnly) => { - if (key === 'file.html') { + if (key === 'repo/file.html') { return { status: 200, metadata: { id: 'test-id-123' }, @@ -84,7 +68,7 @@ describe('Version List', () => { metadata: { timestamp: '1234567890', users: '[{"email":"user@example.com"}]', - path: 'file.html', + path: 'repo/file.html', }, contentLength: 0, }; @@ -98,28 +82,25 @@ describe('Version List', () => { }); const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { - default: mockGetObject, - }, - '../../../src/storage/object/list.js': { - default: mockListObjects, - }, + '../../../src/storage/object/get.js': { default: mockGetObject }, + '../../../src/storage/object/list.js': { default: mockListObjects }, + '../../../src/storage/version/audit.js': { readAuditLines: async () => [] }, }); - const result = await listObjectVersions({}, { bucket: 'test', org: 'org', key: 'file.html' }); + const result = await listObjectVersions({}, { bucket: 'test', org: 'org', key: 'repo/file.html' }); assert.strictEqual(result.status, 200); const versions = JSON.parse(result.body); assert.strictEqual(versions.length, 1); assert.deepStrictEqual(versions[0].users, [{ email: 'user@example.com' }]); assert.strictEqual(versions[0].timestamp, 1234567890); - assert.strictEqual(versions[0].path, 'file.html'); + assert.strictEqual(versions[0].path, 'repo/file.html'); assert.strictEqual(versions[0].url, undefined); // No URL when contentLength is 0 }); it('should include URL when version has content', async () => { const mockGetObject = async (env, { key }, metadataOnly) => { - if (key === 'file.html') { + if (key === 'repo/file.html') { return { status: 200, metadata: { id: 'test-id-456' }, @@ -131,7 +112,7 @@ describe('Version List', () => { metadata: { timestamp: '1234567890', users: '[{"email":"user@example.com"}]', - path: 'file.html', + path: 'repo/file.html', label: 'Important Version', }, contentLength: 100, @@ -146,15 +127,12 @@ describe('Version List', () => { }); const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { - default: mockGetObject, - }, - '../../../src/storage/object/list.js': { - default: mockListObjects, - }, + '../../../src/storage/object/get.js': { default: mockGetObject }, + '../../../src/storage/object/list.js': { default: mockListObjects }, + '../../../src/storage/version/audit.js': { readAuditLines: async () => [] }, }); - const result = await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'file.html' }); + const result = await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'repo/file.html' }); assert.strictEqual(result.status, 200); const versions = JSON.parse(result.body); @@ -166,7 +144,7 @@ describe('Version List', () => { it('should filter out failed version requests', async () => { let callCount = 0; const mockGetObject = async (env, { key }, metadataOnly) => { - if (key === 'file.html') { + if (key === 'repo/file.html') { return { status: 200, metadata: { id: 'test-id-789' }, @@ -186,7 +164,7 @@ describe('Version List', () => { metadata: { timestamp: `123456789${callCount}`, users: '[{"email":"user@example.com"}]', - path: 'file.html', + path: 'repo/file.html', }, contentLength: 10, }; @@ -202,15 +180,12 @@ describe('Version List', () => { }); const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { - default: mockGetObject, - }, - '../../../src/storage/object/list.js': { - default: mockListObjects, - }, + '../../../src/storage/object/get.js': { default: mockGetObject }, + '../../../src/storage/object/list.js': { default: mockListObjects }, + '../../../src/storage/version/audit.js': { readAuditLines: async () => [] }, }); - const result = await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'file.html' }); + const result = await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'repo/file.html' }); assert.strictEqual(result.status, 200); const versions = JSON.parse(result.body); @@ -221,7 +196,7 @@ describe('Version List', () => { it('should handle batch processing for many versions', async () => { const getObjectCalls = []; const mockGetObject = async (env, { key }, metadataOnly) => { - if (key === 'file.html') { + if (key === 'repo/file.html') { return { status: 200, metadata: { id: 'test-id-batch' }, @@ -233,7 +208,7 @@ describe('Version List', () => { metadata: { timestamp: '1234567890', users: '[{"email":"user@example.com"}]', - path: 'file.html', + path: 'repo/file.html', }, contentLength: 10, }; @@ -251,15 +226,12 @@ describe('Version List', () => { }); const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { - default: mockGetObject, - }, - '../../../src/storage/object/list.js': { - default: mockListObjects, - }, + '../../../src/storage/object/get.js': { default: mockGetObject }, + '../../../src/storage/object/list.js': { default: mockListObjects }, + '../../../src/storage/version/audit.js': { readAuditLines: async () => [] }, }); - const result = await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'file.html' }); + const result = await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'repo/file.html' }); assert.strictEqual(result.status, 200); const resultVersions = JSON.parse(result.body); @@ -271,7 +243,7 @@ describe('Version List', () => { it('should handle versions missing metadata fields gracefully', async () => { const mockGetObject = async (env, { key }, metadataOnly) => { - if (key === 'file.html') { + if (key === 'repo/file.html') { return { status: 200, metadata: { id: 'test-id-missing' }, @@ -297,15 +269,12 @@ describe('Version List', () => { }); const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { - default: mockGetObject, - }, - '../../../src/storage/object/list.js': { - default: mockListObjects, - }, + '../../../src/storage/object/get.js': { default: mockGetObject }, + '../../../src/storage/object/list.js': { default: mockListObjects }, + '../../../src/storage/version/audit.js': { readAuditLines: async () => [] }, }); - const result = await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'file.html' }); + const result = await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'repo/file.html' }); assert.strictEqual(result.status, 200); const versions = JSON.parse(result.body); @@ -328,15 +297,12 @@ describe('Version List', () => { }); const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { - default: mockGetObject, - }, - '../../../src/storage/object/list.js': { - default: mockListObjects, - }, + '../../../src/storage/object/get.js': { default: mockGetObject }, + '../../../src/storage/object/list.js': { default: mockListObjects }, + '../../../src/storage/version/audit.js': { readAuditLines: async () => [] }, }); - const result = await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'file.html' }); + const result = await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'repo/file.html' }); assert.strictEqual(result.status, 200); const versions = JSON.parse(result.body); @@ -345,7 +311,7 @@ describe('Version List', () => { it('should handle all versions failing', async () => { const mockGetObject = async (env, { key }, metadataOnly) => { - if (key === 'file.html') { + if (key === 'repo/file.html') { return { status: 200, metadata: { id: 'test-id-allfail' }, @@ -367,15 +333,12 @@ describe('Version List', () => { }); const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { - default: mockGetObject, - }, - '../../../src/storage/object/list.js': { - default: mockListObjects, - }, + '../../../src/storage/object/get.js': { default: mockGetObject }, + '../../../src/storage/object/list.js': { default: mockListObjects }, + '../../../src/storage/version/audit.js': { readAuditLines: async () => [] }, }); - const result = await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'file.html' }); + const result = await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'repo/file.html' }); assert.strictEqual(result.status, 200); const versions = JSON.parse(result.body); @@ -399,26 +362,21 @@ describe('Version List', () => { }; const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { - default: mockGetObject, - }, - '../../../src/storage/object/list.js': { - default: mockListObjects, - }, + '../../../src/storage/object/get.js': { default: mockGetObject }, + '../../../src/storage/object/list.js': { default: mockListObjects }, + '../../../src/storage/version/audit.js': { readAuditLines: async () => [] }, }); - await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'file.html' }); + await listObjectVersions({}, { bucket: 'test', org: 'testorg', key: 'repo/file.html' }); // Verify MAX_VERSIONS (500) is passed to listObjects assert.strictEqual(maxVersionsParam, 500); }); - describe('backward compat: not migrated but new audit entries in new path', () => { - it('when new path has only audit.txt (no snapshots), legacy mode shows legacy snapshots only', async () => { + describe('audit file mode with legacy merge', () => { + it('merges audit entries and legacy snapshots for a repo key', async () => { const listObjectCalls = []; - const getObjectCalls = []; const mockGetObject = async (env, { key }) => { - getObjectCalls.push(key); if (key === 'myrepo/docs/file.html') { return { status: 200, metadata: { id: 'file-id-bcompat' } }; } @@ -448,10 +406,9 @@ describe('Version List', () => { return { status: 404, body: '[]' }; }; - const newAuditLines = [ + const mockReadAuditLines = async () => [ { timestamp: 5000, users: [{ email: 'new@example.com' }], path: 'myrepo/docs/file.html' }, ]; - const mockReadAuditLines = async () => newAuditLines; const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { '../../../src/storage/object/get.js': { default: mockGetObject }, @@ -466,13 +423,13 @@ describe('Version List', () => { assert.strictEqual(result.status, 200); const versions = JSON.parse(result.body); - assert.strictEqual(versions.length, 1, 'audit.txt not listed without VERSIONS_AUDIT_FILE_ORGS'); - assert.ok(versions[0].url); - assert.strictEqual(versions[0].timestamp, 1000); + assert.strictEqual(versions.length, 2, 'audit entry and legacy snapshot must both be returned'); + assert.strictEqual(versions[0].timestamp, 5000, 'most recent (audit) entry first'); + assert.strictEqual(versions[1].timestamp, 1000, 'legacy entry second'); assert.ok(listObjectCalls.some((k) => k.startsWith('.da-versions/')), 'legacy prefix listed for merge'); }); - it('when new path list returns 404, uses legacy only', async () => { + it('empty audit falls back to legacy entries only', async () => { const mockGetObject = async (env, { key }) => { if (key === 'repo/path.html') { return { status: 200, metadata: { id: 'id-404' } }; @@ -488,9 +445,6 @@ describe('Version List', () => { }; const mockListObjects = async (env, { key }) => { - if (key === 'repo/.da-versions/id-404') { - return { status: 404, body: '[]' }; - } if (key === '.da-versions/id-404') { return { status: 200, @@ -503,6 +457,7 @@ describe('Version List', () => { const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { '../../../src/storage/object/get.js': { default: mockGetObject }, '../../../src/storage/object/list.js': { default: mockListObjects }, + '../../../src/storage/version/audit.js': { readAuditLines: async () => [] }, }); const result = await listObjectVersions( @@ -516,7 +471,7 @@ describe('Version List', () => { assert.ok(versions[0].url); }); - it('without VERSIONS_AUDIT_FILE_ORGS, repo/.da-versions snapshots are not listed', async () => { + it('empty audit and empty legacy returns empty list', async () => { const listKeys = []; const mockGetObject = async (env, { key }) => { if (key === 'repo/doc.html') { @@ -536,6 +491,7 @@ describe('Version List', () => { const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { '../../../src/storage/object/get.js': { default: mockGetObject }, '../../../src/storage/object/list.js': { default: mockListObjects }, + '../../../src/storage/version/audit.js': { readAuditLines: async () => [] }, }); const result = await listObjectVersions( @@ -546,11 +502,10 @@ describe('Version List', () => { assert.strictEqual(result.status, 200); const versions = JSON.parse(result.body); assert.strictEqual(versions.length, 0); - assert.ok(!listKeys.some((k) => k.includes('repo/.da-versions'))); }); }); - describe('VERSIONS_AUDIT_FILE_ORGS (new mode)', () => { + describe('audit file mode', () => { it('audit file + SKIP_LEGACY: audit only, no org/.da-versions list', async () => { const listObjectCalls = []; const mockGetObject = async (env, { key }) => { @@ -577,10 +532,7 @@ describe('Version List', () => { }); const result = await listObjectVersions( - { - VERSIONS_AUDIT_FILE_ORGS: 'testorg', - VERSIONS_AUDIT_SKIP_LEGACY_ORGS: 'testorg', - }, + { VERSIONS_AUDIT_SKIP_LEGACY_ORGS: 'testorg' }, { bucket: 'bkt', org: 'testorg', key: 'myrepo/docs/file.html' }, ); @@ -609,10 +561,7 @@ describe('Version List', () => { }); const result = await listObjectVersions( - { - VERSIONS_AUDIT_FILE_ORGS: 'testorg', - VERSIONS_AUDIT_SKIP_LEGACY_ORGS: 'testorg', - }, + { VERSIONS_AUDIT_SKIP_LEGACY_ORGS: 'testorg' }, { bucket: 'bkt', org: 'testorg', key: 'repo/path.html' }, ); @@ -620,7 +569,7 @@ describe('Version List', () => { assert.strictEqual(result.body, '[]'); }); - it('audit file without skip: merges org/.da-versions with audit entries', async () => { + it('merges org/.da-versions with audit entries when skip-legacy not set', async () => { const listObjectCalls = []; const mockGetObject = async (env, { key }) => { if (key === 'myrepo/docs/file.html') { @@ -659,7 +608,7 @@ describe('Version List', () => { }); const result = await listObjectVersions( - { VERSIONS_AUDIT_FILE_ORGS: 'testorg' }, + {}, { bucket: 'bkt', org: 'testorg', key: 'myrepo/docs/file.html' }, ); @@ -668,7 +617,7 @@ describe('Version List', () => { assert.strictEqual(versions.length, 2); }); - it('audit file without skip: deduplicates entries with same timestamp as audit', async () => { + it('deduplicates entries with same timestamp as audit', async () => { const mockGetObject = async (env, { key }) => { if (key === 'myrepo/docs/file.html') { return { status: 200, metadata: { id: 'fid-dedup' } }; @@ -708,7 +657,7 @@ describe('Version List', () => { }); const result = await listObjectVersions( - { VERSIONS_AUDIT_FILE_ORGS: 'testorg' }, + {}, { bucket: 'bkt', org: 'testorg', key: 'myrepo/docs/file.html' }, ); @@ -717,58 +666,6 @@ describe('Version List', () => { assert.strictEqual(versions[0].timestamp, 1000); }); - it('org not in new mode: merges legacy with new when new has no snapshots', async () => { - const listObjectCalls = []; - const mockGetObject = async (env, { key }) => { - if (key === 'myrepo/docs/file.html') { - return { status: 200, metadata: { id: 'file-id-true' } }; - } - if (key === '.da-versions/file-id-true/snap1.html') { - return { - status: 200, - metadata: { - timestamp: '1000', - users: '[{"email":"legacy@example.com"}]', - path: 'myrepo/docs/file.html', - }, - contentLength: 100, - }; - } - return { status: 404 }; - }; - - const mockListObjects = async (env, { key }) => { - listObjectCalls.push(key); - if (key === 'myrepo/.da-versions/file-id-true') { - return { status: 200, body: JSON.stringify([{ name: 'audit', ext: 'txt' }]) }; - } - if (key === '.da-versions/file-id-true') { - return { status: 200, body: JSON.stringify([{ name: 'snap1', ext: 'html' }]) }; - } - return { status: 404, body: '[]' }; - }; - - const mockReadAuditLines = async () => [ - { timestamp: 5000, users: [{ email: 'new@example.com' }], path: 'myrepo/docs/file.html' }, - ]; - - const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { default: mockGetObject }, - '../../../src/storage/object/list.js': { default: mockListObjects }, - '../../../src/storage/version/audit.js': { readAuditLines: mockReadAuditLines }, - }); - - const result = await listObjectVersions( - {}, - { bucket: 'bkt', org: 'testorg', key: 'myrepo/docs/file.html' }, - ); - - assert.strictEqual(result.status, 200); - const versions = JSON.parse(result.body); - assert.strictEqual(versions.length, 1, 'new path has no snapshot files; legacy only'); - assert.ok(listObjectCalls.some((k) => k.startsWith('.da-versions/'))); - }); - it('readAuditLines throws: falls back to empty audit entries and proceeds', async () => { const mockGetObject = async (env, { key }) => { if (key === 'repo/doc.html') { @@ -789,7 +686,7 @@ describe('Version List', () => { }); const result = await listObjectVersions( - { VERSIONS_AUDIT_FILE_ORGS: 'testorg' }, + {}, { bucket: 'b', org: 'testorg', key: 'repo/doc.html' }, ); @@ -827,10 +724,7 @@ describe('Version List', () => { }); const result = await listObjectVersions( - { - VERSIONS_AUDIT_FILE_ORGS: 'acme', - VERSIONS_AUDIT_SKIP_LEGACY_ORGS: 'acme', - }, + { VERSIONS_AUDIT_SKIP_LEGACY_ORGS: 'acme' }, { bucket: 'b', org: 'acme', key: 'r/doc.html' }, ); @@ -864,7 +758,7 @@ describe('Version List', () => { }); const result = await listObjectVersions( - { VERSIONS_AUDIT_FILE_ORGS: 'testorg', VERSIONS_AUDIT_SKIP_LEGACY_ORGS: 'testorg' }, + { VERSIONS_AUDIT_SKIP_LEGACY_ORGS: 'testorg' }, { bucket: 'b', org: 'testorg', key: 'myrepo/doc.html' }, ); @@ -873,49 +767,5 @@ describe('Version List', () => { assert.strictEqual(versions[0].timestamp, 600, 'most recent entry first'); assert.strictEqual(versions[499].timestamp, 101, 'oldest included entry'); }); - - it('when org not in VERSIONS_AUDIT_FILE_ORGS, only org/.da-versions (not repo path)', async () => { - const listKeys = []; - const mockGetObject = async (env, { key }) => { - if (key === 'r/doc.html') { - return { status: 200, metadata: { id: 'fid2' } }; - } - if (key === '.da-versions/fid2/leg.html') { - return { - status: 200, - metadata: { - timestamp: '50', - users: '[{"email":"s@b.com"}]', - path: 'r/doc.html', - }, - contentLength: 10, - }; - } - return { status: 404 }; - }; - const mockListObjects = async (env, { key }) => { - listKeys.push(key); - if (key === '.da-versions/fid2') { - return { status: 200, body: JSON.stringify([{ name: 'leg', ext: 'html' }]) }; - } - return { status: 404 }; - }; - - const { listObjectVersions } = await esmock('../../../src/storage/version/list.js', { - '../../../src/storage/object/get.js': { default: mockGetObject }, - '../../../src/storage/object/list.js': { default: mockListObjects }, - }); - - const result = await listObjectVersions( - { VERSIONS_AUDIT_FILE_ORGS: 'other-org' }, - { bucket: 'b', org: 'acme', key: 'r/doc.html' }, - ); - - assert.ok(listKeys.some((k) => k === '.da-versions/fid2')); - assert.ok(!listKeys.some((k) => k.startsWith('r/'))); - const versions = JSON.parse(result.body); - assert.strictEqual(versions.length, 1); - assert.strictEqual(versions[0].url, '/versionsource/acme/fid2/leg.html'); - }); }); }); diff --git a/test/storage/version/put.test.js b/test/storage/version/put.test.js index 723ca1dc..e04beffe 100644 --- a/test/storage/version/put.test.js +++ b/test/storage/version/put.test.js @@ -2361,7 +2361,7 @@ describe('Version Put', () => { type: 'text/html', }; - const resp = await putObjectWithVersion({ VERSIONS_AUDIT_FILE_ORGS: 'myorg' }, daCtx, update, true); + const resp = await putObjectWithVersion({}, daCtx, update, true); assert.strictEqual(resp.status, 200); assert.strictEqual(auditCalls.length, 1, 'writeAuditEntry must be called once'); @@ -2410,7 +2410,7 @@ describe('Version Put', () => { label: 'My version', }; - const resp = await putObjectWithVersion({ VERSIONS_AUDIT_FILE_ORGS: 'o' }, daCtx, update, true); + const resp = await putObjectWithVersion({}, daCtx, update, true); assert.strictEqual(resp.status, 200); assert.strictEqual(resp.versionCreated, true); @@ -2462,7 +2462,7 @@ describe('Version Put', () => { label: 'Release 1', }; - await putObjectWithVersion({ VERSIONS_AUDIT_FILE_ORGS: 'o' }, daCtx, update, true); + await putObjectWithVersion({}, daCtx, update, true); assert.strictEqual(auditCalls.length, 1); assert.strictEqual( @@ -2519,7 +2519,7 @@ describe('Version Put', () => { type: 'text/html', }; - await putObjectWithVersion({ VERSIONS_AUDIT_FILE_ORGS: 'o' }, daCtx, update, true); + await putObjectWithVersion({}, daCtx, update, true); assert.strictEqual(auditCalls.length, 1); assert.strictEqual( @@ -2559,7 +2559,7 @@ describe('Version Put', () => { }); const resp = await putObjectWithVersion( - { VERSIONS_AUDIT_FILE_ORGS: 'o' }, + {}, { org: 'o', ext: 'html', site: 'repo', users: [], }, @@ -2603,7 +2603,7 @@ describe('Version Put', () => { let resp; try { resp = await putObjectWithVersion( - { VERSIONS_AUDIT_FILE_ORGS: 'o' }, + {}, { org: 'o', ext: 'html', site: 'repo', users: [], }, @@ -2624,126 +2624,6 @@ describe('Version Put', () => { ); }); - it('writes legacy empty version object when org is not in VERSIONS_AUDIT_FILE_ORGS', async () => { - const auditCalls = []; - const legacyPutCalls = []; - - const mockGetObject = async () => ({ - body: 'content', - contentType: 'text/html', - contentLength: 7, - metadata: { id: 'fid', version: 'v1' }, - status: 200, - }); - - const mockS3Client = { send: async () => ({ $metadata: { httpStatusCode: 200 } }) }; - // S3Client is used directly by putVersion(noneMatch=false) for the legacy audit write - function MockS3Client() { - this.send = async (cmd) => { - if (cmd instanceof PutObjectCommand) legacyPutCalls.push(cmd.input); - return { $metadata: { httpStatusCode: 200 } }; - }; - } - - const { putObjectWithVersion } = await esmock('../../../src/storage/version/put.js', { - '@aws-sdk/client-s3': { S3Client: MockS3Client, PutObjectCommand }, - '../../../src/storage/object/get.js': { default: mockGetObject }, - '../../../src/storage/utils/version.js': { - ifNoneMatch: () => mockS3Client, - ifMatch: () => mockS3Client, - }, - '../../../src/storage/version/audit.js': { - writeAuditEntry: async () => { auditCalls.push(1); }, - }, - }); - - const daCtx = { - org: 'legacyorg', - ext: 'html', - site: 'myrepo', - users: [{ email: 'u@x.com' }], - }; - const update = { - bucket: 'bkt', - org: 'legacyorg', - key: 'myrepo/doc.html', - body: 'updated', - type: 'text/html', - }; - - // env has no VERSIONS_AUDIT_FILE_ORGS → legacy path - const resp = await putObjectWithVersion({}, daCtx, update, true); - - assert.strictEqual(resp.status, 200); - assert.strictEqual(auditCalls.length, 0, 'writeAuditEntry must NOT be called for legacy org'); - assert.strictEqual(legacyPutCalls.length, 1, 'legacy empty version PUT must happen'); - assert.ok(legacyPutCalls[0].Key.includes('.da-versions/'), 'must use legacy .da-versions/ path'); - assert.strictEqual(legacyPutCalls[0].ContentLength, 0, 'legacy version body must be empty'); - assert.strictEqual(legacyPutCalls[0].Metadata?.Path, 'myrepo/doc.html'); - }); - - it('stores labeled snapshot under legacy .da-versions/ path when org is not in VERSIONS_AUDIT_FILE_ORGS', async () => { - const snapshotPutCalls = []; - const mockGetObject = async () => ({ - body: 'content', - contentType: 'text/html', - contentLength: 7, - metadata: { id: 'fid', version: 'v1' }, - status: 200, - }); - - const emptyCalls = []; - const mockIfNoneMatch = () => ({ - send: async (cmd) => { - if (cmd instanceof PutObjectCommand) snapshotPutCalls.push(cmd.input); - return { $metadata: { httpStatusCode: 200 } }; - }, - }); - const mockS3Client = { send: async () => ({ $metadata: { httpStatusCode: 200 } }) }; - function MockS3Client() { - this.send = async (cmd) => { - if (cmd instanceof PutObjectCommand) emptyCalls.push(cmd.input); - return { $metadata: { httpStatusCode: 200 } }; - }; - } - - const { putObjectWithVersion } = await esmock('../../../src/storage/version/put.js', { - '@aws-sdk/client-s3': { S3Client: MockS3Client, PutObjectCommand }, - '../../../src/storage/object/get.js': { default: mockGetObject }, - '../../../src/storage/utils/version.js': { - ifNoneMatch: mockIfNoneMatch, - ifMatch: () => mockS3Client, - }, - '../../../src/storage/version/audit.js': { writeAuditEntry: async () => {} }, - }); - - const daCtx = { - org: 'legacyorg', - ext: 'html', - site: 'myrepo', - users: [{ email: 'u@x.com' }], - }; - const update = { - bucket: 'bkt', - org: 'legacyorg', - key: 'myrepo/doc.html', - body: 'updated', - type: 'text/html', - label: 'My snapshot', - }; - - // env has no VERSIONS_AUDIT_FILE_ORGS → legacy path - await putObjectWithVersion({}, daCtx, update, true); - - const snapshotPut = snapshotPutCalls.find((p) => p.Key?.includes('.da-versions/')); - assert.ok(snapshotPut, 'snapshot must be PUT under .da-versions/'); - assert.ok(!snapshotPut.Key.includes('myrepo/.da-versions'), 'snapshot must NOT use repo-scoped new path'); - assert.ok(snapshotPut.Key.startsWith('legacyorg/.da-versions/'), 'snapshot must use legacy org-root path'); - // The snapshot itself is the version marker — no separate empty write must clobber it - const emptyVersionWrites = emptyCalls.filter((p) => p.Key?.includes('.da-versions/') && p.ContentLength === 0); - assert.strictEqual(emptyVersionWrites.length, 0, 'empty write must NOT overwrite the labeled snapshot'); - }); - it('does not write audit for non-versionable type (e.g. PDF)', async () => { const auditCalls = []; const mockWriteAuditEntry = async () => { @@ -2815,7 +2695,6 @@ describe('Version Put', () => { '../../../src/storage/version/audit.js': { writeAuditEntry: async () => {} }, }); - // No VERSIONS_AUDIT_FILE_ORGS → Mode A await putObjectWithVersion( {}, { @@ -2831,8 +2710,7 @@ describe('Version Put', () => { assert.strictEqual(daVersionWrites.length, 0, 'binary files must never write to .da-versions/'); }); - // Mode B (audit-file): plain edit — audit entry written, no snapshot object written - it('Mode B: plain edit writes audit entry but no snapshot to .da-versions/', async () => { + it('plain edit writes audit entry but no snapshot to .da-versions/', async () => { const auditCalls = []; const snapshotWrites = []; @@ -2871,9 +2749,8 @@ describe('Version Put', () => { }, }); - // VERSIONS_AUDIT_FILE_ORGS contains org → Mode B; no label → plain edit await putObjectWithVersion( - { VERSIONS_AUDIT_FILE_ORGS: 'myorg' }, + {}, { org: 'myorg', ext: 'html', site: 'myrepo', users: [{ email: 'u@x.com' }], }, @@ -2888,8 +2765,7 @@ describe('Version Put', () => { assert.strictEqual(snapshotToDaVersions.length, 0, 'plain edit must not write a snapshot object'); }); - // Mode B (audit-file): labeled version — snapshot stored under repo-scoped new path - it('Mode B: labeled version snapshot stored under repo-scoped new path', async () => { + it('labeled version snapshot stored under repo-scoped new path', async () => { const snapshotWrites = []; const mockGetObject = async () => ({ @@ -2923,9 +2799,8 @@ describe('Version Put', () => { '../../../src/storage/version/audit.js': { writeAuditEntry: async () => {} }, }); - // VERSIONS_AUDIT_FILE_ORGS contains org → Mode B; label provided → labeled version await putObjectWithVersion( - { VERSIONS_AUDIT_FILE_ORGS: 'myorg' }, + {}, { org: 'myorg', ext: 'html', site: 'myrepo', users: [{ email: 'u@x.com' }], }, @@ -3023,7 +2898,7 @@ describe('Version Put', () => { org: 'o', ext: 'html', site: 'repo', users: [{ email: 'u@x.com' }], }; const update = { org: 'o', key: 'repo/doc.html', type: 'text/html' }; - const resp = await putObjectWithVersion({ VERSIONS_AUDIT_FILE_ORGS: 'o' }, daCtx, update, true); + const resp = await putObjectWithVersion({}, daCtx, update, true); // audit failure must not bubble up; main put succeeded assert.strictEqual(resp.status, 200);