From 06a5e923b5a906d3fd05ae698405362e8ed1507c Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Wed, 1 Jul 2026 18:30:58 +0200 Subject: [PATCH 01/17] Extend recorder repositories for version listing --- src/archivist/recorder/repositories/git/git.js | 3 ++- src/archivist/recorder/repositories/git/index.js | 8 ++++---- src/archivist/recorder/repositories/git/index.test.js | 6 ++++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/archivist/recorder/repositories/git/git.js b/src/archivist/recorder/repositories/git/git.js index 1b25871e1..5b2466cc3 100644 --- a/src/archivist/recorder/repositories/git/git.js +++ b/src/archivist/recorder/repositories/git/git.js @@ -106,7 +106,8 @@ export default class Git { return commits; } catch (error) { - if (/unknown revision or path not in the working tree|does not have any commits yet/.test(error.message)) { + // `bad object` is raised for a well-formed but absent object ID (e.g. a snapshot referenced by a version but missing from this repository); like an unknown revision, it means "no match" rather than a hard failure + if (/unknown revision or path not in the working tree|does not have any commits yet|bad object/.test(error.message)) { return []; } diff --git a/src/archivist/recorder/repositories/git/index.js b/src/archivist/recorder/repositories/git/index.js index 1e54fb4bd..0b4510f6d 100644 --- a/src/archivist/recorder/repositories/git/index.js +++ b/src/archivist/recorder/repositories/git/index.js @@ -92,14 +92,14 @@ export default class GitRepository extends RepositoryInterface { return Promise.all((await this.#getCommits({ limit, offset, includeTechnicalUpgrades })).map(commit => this.#toDomain(commit, { deferContentLoading: true }))); } - async findByService(serviceId, { limit, offset, includeTechnicalUpgrades = true } = {}) { - const pathPattern = DataMapper.generateFilePath(serviceId); + async findByServiceAndTermsType(serviceId, termsType, { limit, offset, includeTechnicalUpgrades = true } = {}) { + const pathPattern = DataMapper.generateFilePath(serviceId, termsType); return Promise.all((await this.#getCommits({ pathFilter: pathPattern, limit, offset, includeTechnicalUpgrades })).map(commit => this.#toDomain(commit, { deferContentLoading: true }))); } - async findByServiceAndTermsType(serviceId, termsType, { limit, offset, includeTechnicalUpgrades = true } = {}) { - const pathPattern = DataMapper.generateFilePath(serviceId, termsType); + async findByService(serviceId, { limit, offset, includeTechnicalUpgrades = true } = {}) { + const pathPattern = DataMapper.generateFilePath(serviceId); return Promise.all((await this.#getCommits({ pathFilter: pathPattern, limit, offset, includeTechnicalUpgrades })).map(commit => this.#toDomain(commit, { deferContentLoading: true }))); } diff --git a/src/archivist/recorder/repositories/git/index.test.js b/src/archivist/recorder/repositories/git/index.test.js index 2ae2dfdae..e597e170f 100644 --- a/src/archivist/recorder/repositories/git/index.test.js +++ b/src/archivist/recorder/repositories/git/index.test.js @@ -400,6 +400,12 @@ describe('GitRepository', () => { expect(await subject.findById('inexistantID')).to.equal(null); }); }); + + context('when the requested ID is well formed but absent from the repository', () => { + it('returns null rather than throwing a "bad object" error', async () => { + expect(await subject.findById('ecd9407eb26b1bf0613186175ee80edbdeedd47f')).to.equal(null); + }); + }); }); describe('#findByDate', () => { From 326b91c2f811f0858fa2f844f2a716e6c62726ac Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Wed, 1 Jul 2026 18:57:46 +0200 Subject: [PATCH 02/17] Add findMetadataById to the recorder repositories --- src/archivist/recorder/repositories/git/index.js | 6 ++++++ src/archivist/recorder/repositories/interface.js | 9 +++++++++ src/archivist/recorder/repositories/mongo/index.js | 13 +++++++++++++ 3 files changed, 28 insertions(+) diff --git a/src/archivist/recorder/repositories/git/index.js b/src/archivist/recorder/repositories/git/index.js index 0b4510f6d..b15cc4a61 100644 --- a/src/archivist/recorder/repositories/git/index.js +++ b/src/archivist/recorder/repositories/git/index.js @@ -88,6 +88,12 @@ export default class GitRepository extends RepositoryInterface { return this.#toDomain(commit); } + async findMetadataById(recordId) { + const commit = await this.git.getCommit([recordId]); + + return this.#toDomain(commit, { deferContentLoading: true }); + } + async findAll({ limit, offset, includeTechnicalUpgrades = true } = {}) { return Promise.all((await this.#getCommits({ limit, offset, includeTechnicalUpgrades })).map(commit => this.#toDomain(commit, { deferContentLoading: true }))); } diff --git a/src/archivist/recorder/repositories/interface.js b/src/archivist/recorder/repositories/interface.js index 00e8bfce5..1f99d896b 100644 --- a/src/archivist/recorder/repositories/interface.js +++ b/src/archivist/recorder/repositories/interface.js @@ -69,6 +69,15 @@ class RepositoryInterface { throw new Error(`#findById method is not implemented in ${this.constructor.name}`); } + /** + * Find the metadata of the record that matches the given record ID, without loading its content + * @param {string} recordId - Record ID of the record to find + * @returns {Promise} Promise that will be resolved with the found record (without content) or null if none match the given ID + */ + async findMetadataById(recordId) { + throw new Error(`#findMetadataById method is not implemented in ${this.constructor.name}`); + } + /** * Find all records, in descending chronological order (newest first; opposite of #iterate) * For performance reasons, the content of the records will not be loaded by default. Use #loadRecordContent to load the content of individual records diff --git a/src/archivist/recorder/repositories/mongo/index.js b/src/archivist/recorder/repositories/mongo/index.js index 649b22981..e9d9168a1 100644 --- a/src/archivist/recorder/repositories/mongo/index.js +++ b/src/archivist/recorder/repositories/mongo/index.js @@ -88,6 +88,19 @@ export default class MongoRepository extends RepositoryInterface { return this.#toDomain(mongoDocument); } + async findMetadataById(recordId) { + if (!ObjectId.isValid(recordId)) { + return null; + } + + const document = await this.collection.findOne( + { _id: ObjectId.createFromHexString(recordId) }, + { projection: { content: 0 } }, + ); + + return document ? this.#toDomain(document, { deferContentLoading: true }) : null; + } + async findAll({ limit, offset, includeTechnicalUpgrades = true } = {}) { const filter = includeTechnicalUpgrades ? {} : { isTechnicalUpgrade: { $ne: true } }; let query = this.collection.find(filter).project({ content: 0 }).sort({ fetchDate: -1 }); From 9cba957962981cbf1cdff6f92d80e4910dff0a3b Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Wed, 1 Jul 2026 19:30:57 +0200 Subject: [PATCH 03/17] Add version nav to the recorder repositories --- .../recorder/repositories/git/dataMapper.js | 4 + .../recorder/repositories/git/git.js | 26 +++ .../recorder/repositories/git/index.js | 26 +++ .../recorder/repositories/git/index.test.js | 155 ++++++++++++++++++ .../recorder/repositories/interface.js | 14 ++ .../recorder/repositories/mongo/index.js | 40 +++++ .../recorder/repositories/mongo/index.test.js | 150 +++++++++++++++++ 7 files changed, 415 insertions(+) diff --git a/src/archivist/recorder/repositories/git/dataMapper.js b/src/archivist/recorder/repositories/git/dataMapper.js index 9f2757f45..2d79a5378 100644 --- a/src/archivist/recorder/repositories/git/dataMapper.js +++ b/src/archivist/recorder/repositories/git/dataMapper.js @@ -30,6 +30,10 @@ const MULTIPLE_SOURCE_DOCUMENTS_PREFIX = 'This version was recorded after extrac export const COMMIT_MESSAGE_PREFIXES_REGEXP = new RegExp(`^(${Object.values(COMMIT_MESSAGE_PREFIXES).join('|')})`); +export function isTechnicalUpgrade(message) { + return message.startsWith(COMMIT_MESSAGE_PREFIXES.technicalUpgrade) || message.startsWith(COMMIT_MESSAGE_PREFIXES.deprecated_refilter); +} + export function toPersistence(record, snapshotIdentiferTemplate) { const { serviceId, termsType, documentId, snapshotIds = [], mimeType, metadata } = record; diff --git a/src/archivist/recorder/repositories/git/git.js b/src/archivist/recorder/repositories/git/git.js index 5b2466cc3..2f11e935f 100644 --- a/src/archivist/recorder/repositories/git/git.js +++ b/src/archivist/recorder/repositories/git/git.js @@ -165,4 +165,30 @@ export default class Git { async updateCommitGraph() { await this.git.raw([ 'commit-graph', 'write', '--reachable', '--changed-paths', '--append' ]); } + + async listPathRevisions(pathFilter) { + let output; + + try { + // Lean log: only the data needed to order versions (hash, author timestamp, subject), no diff or message body. + // Ordering and technical-upgrade filtering are done by the caller in memory, so `--author-date-order`/`--grep`/`--name-only` are deliberately omitted. + output = await this.git.raw([ 'log', '--no-merges', '--format=%H%x09%at%x09%s', '--', pathFilter ]); + } catch (error) { + if (/unknown revision or path not in the working tree|does not have any commits yet/.test(error.message)) { + return []; + } + + throw error; + } + + if (!output) { + return []; + } + + return output.trim().split('\n').filter(Boolean).map(line => { + const [ hash, timestamp, ...subjectParts ] = line.split('\t'); + + return { hash, timestamp: parseInt(timestamp, 10), subject: subjectParts.join('\t') }; + }); + } } diff --git a/src/archivist/recorder/repositories/git/index.js b/src/archivist/recorder/repositories/git/index.js index b15cc4a61..e9de61ad3 100644 --- a/src/archivist/recorder/repositories/git/index.js +++ b/src/archivist/recorder/repositories/git/index.js @@ -110,6 +110,32 @@ export default class GitRepository extends RepositoryInterface { return Promise.all((await this.#getCommits({ pathFilter: pathPattern, limit, offset, includeTechnicalUpgrades })).map(commit => this.#toDomain(commit, { deferContentLoading: true }))); } + async getNavigationIds(serviceId, termsType, versionId, { includeTechnicalUpgrades = true } = {}) { + const pathPattern = DataMapper.generateFilePath(serviceId, termsType); + let revisions = await this.git.listPathRevisions(pathPattern); // single lean walk of the terms history + + if (!includeTechnicalUpgrades) { + revisions = revisions.filter(revision => !DataMapper.isTechnicalUpgrade(revision.subject)); + } + + // Deterministic total order: most recent first, commit SHA as a stable tiebreaker for versions sharing the same fetch date (git stores second precision). + // prev/next are then adjacent entries in this single order, so navigation always round-trips, unlike the previous chronological/topological mix. + revisions.sort((a, b) => b.timestamp - a.timestamp || (a.hash < b.hash ? -1 : 1)); + + const index = revisions.findIndex(revision => revision.hash === versionId); + + if (index === -1) { + return { first: null, prev: null, next: null, last: null }; + } + + return { + last: revisions[0].hash, // newest version of these terms + first: revisions[revisions.length - 1].hash, // oldest version of these terms + next: index > 0 ? revisions[index - 1].hash : null, // the version recorded just after this one + prev: index < revisions.length - 1 ? revisions[index + 1].hash : null, // the version recorded just before this one + }; + } + async count(serviceId, termsType) { const grepOptions = Object.values(DataMapper.COMMIT_MESSAGE_PREFIXES).map(prefix => `--grep=${prefix}`); const pathOptions = []; diff --git a/src/archivist/recorder/repositories/git/index.test.js b/src/archivist/recorder/repositories/git/index.test.js index e597e170f..079a17d0c 100644 --- a/src/archivist/recorder/repositories/git/index.test.js +++ b/src/archivist/recorder/repositories/git/index.test.js @@ -866,6 +866,161 @@ describe('GitRepository', () => { }); }); + describe('#getNavigationIds', () => { + let firstVersion; + let middleVersion; + let lastVersion; + + before(async function () { + this.timeout(5000); + + firstVersion = await subject.save(new Version({ + serviceId: SERVICE_PROVIDER_ID, + termsType: TERMS_TYPE, + content: 'first content', + fetchDate: FETCH_DATE_EARLIER, + snapshotIds: [SNAPSHOT_ID], + })); + + middleVersion = await subject.save(new Version({ + serviceId: SERVICE_PROVIDER_ID, + termsType: TERMS_TYPE, + content: 'middle content', + fetchDate: FETCH_DATE, + snapshotIds: [SNAPSHOT_ID], + })); + + lastVersion = await subject.save(new Version({ + serviceId: SERVICE_PROVIDER_ID, + termsType: TERMS_TYPE, + content: 'last content', + fetchDate: FETCH_DATE_LATER, + snapshotIds: [SNAPSHOT_ID], + })); + }); + + after(() => subject.removeAll()); + + context('for the oldest version', () => { + let navigationIds; + + before(async () => { navigationIds = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, firstVersion.id); }); + + it('has no previous version', () => expect(navigationIds.prev).to.be.null); + it('points to the next version', () => expect(navigationIds.next).to.equal(middleVersion.id)); + it('points to itself as first', () => expect(navigationIds.first).to.equal(firstVersion.id)); + it('points to the newest version as last', () => expect(navigationIds.last).to.equal(lastVersion.id)); + }); + + context('for a middle version', () => { + let navigationIds; + + before(async () => { navigationIds = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, middleVersion.id); }); + + it('points to the previous version', () => expect(navigationIds.prev).to.equal(firstVersion.id)); + it('points to the next version', () => expect(navigationIds.next).to.equal(lastVersion.id)); + it('points to the oldest version as first', () => expect(navigationIds.first).to.equal(firstVersion.id)); + it('points to the newest version as last', () => expect(navigationIds.last).to.equal(lastVersion.id)); + }); + + context('for the newest version', () => { + let navigationIds; + + before(async () => { navigationIds = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, lastVersion.id); }); + + it('points to the previous version', () => expect(navigationIds.prev).to.equal(middleVersion.id)); + it('has no next version', () => expect(navigationIds.next).to.be.null); + it('points to itself as last', () => expect(navigationIds.last).to.equal(lastVersion.id)); + }); + + context('when the version does not exist', () => { + it('returns only null IDs', async () => { + expect(await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, 'ffffffffffffffffffffffffffffffffffffffff')).to.deep.equal({ first: null, prev: null, next: null, last: null }); + }); + }); + + context('when a technical upgrade is recorded out of chronological order', () => { + // A technical upgrade re-renders an old snapshot: it carries an OLD fetch date but is committed last (topologically recent). + // The previous implementation mixed chronological (findPrevious) and topological (findNext) order, so prev/next disagreed here. + // The navigation IDs are now derived from a single chronological order, so they stay exact inverses. + const TECHNICAL_UPGRADE_DATE = new Date('2000-01-01T09:00:00.000Z'); // between firstVersion (06:00) and middleVersion (12:00) + let technicalUpgrade; + + before(async () => { + technicalUpgrade = await subject.save(new Version({ + serviceId: SERVICE_PROVIDER_ID, + termsType: TERMS_TYPE, + content: 'technical upgrade content', + fetchDate: TECHNICAL_UPGRADE_DATE, + isTechnicalUpgrade: true, + snapshotIds: [SNAPSHOT_ID], + })); + }); + + it('orders it by its fetch date, not its commit position', async () => { + const navigationIds = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, technicalUpgrade.id); + + expect(navigationIds.prev).to.equal(firstVersion.id); + expect(navigationIds.next).to.equal(middleVersion.id); + }); + + it('keeps prev and next as exact inverses (round-trips)', async () => { + const fromFirst = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, firstVersion.id); + + expect(fromFirst.next).to.equal(technicalUpgrade.id); + + const backFromUpgrade = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, fromFirst.next); + + expect(backFromUpgrade.prev).to.equal(firstVersion.id); + }); + + it('can exclude technical upgrades from the sequence', async () => { + const navigationIds = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, firstVersion.id, { includeTechnicalUpgrades: false }); + + expect(navigationIds.next).to.equal(middleVersion.id); + }); + }); + }); + + describe('#getNavigationIds with versions sharing the same fetch date', () => { + // Git stores commit dates at second precision, so distinct versions can share a fetch date. + // The record ID tiebreaker must keep the order deterministic so prev/next still round-trip. + let ids; + + before(async function () { + this.timeout(5000); + ids = []; + for (const content of [ 'tie A', 'tie B', 'tie C' ]) { + ids.push((await subject.save(new Version({ serviceId: SERVICE_PROVIDER_ID, termsType: TERMS_TYPE, content, fetchDate: FETCH_DATE, snapshotIds: [SNAPSHOT_ID] }))).id); + } + }); + + after(() => subject.removeAll()); + + it('round-trips next then prev for every version', async () => { + for (const id of ids) { + const navigationIds = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, id); + + if (navigationIds.next) { + expect((await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, navigationIds.next)).prev).to.equal(id); + } + } + }); + + it('exposes all three versions as a single ordered chain', async () => { + const head = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, ids[0]); + const walked = new Set([head.first]); + let cursor = head.first; + + while (cursor) { + walked.add(cursor); + cursor = (await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, cursor)).next; + } + + expect(walked).to.have.lengthOf(ids.length); + }); + }); + describe('#findLatest', () => { context('when there are records for the given service', () => { let lastSnapshotId; diff --git a/src/archivist/recorder/repositories/interface.js b/src/archivist/recorder/repositories/interface.js index 1f99d896b..fd3632411 100644 --- a/src/archivist/recorder/repositories/interface.js +++ b/src/archivist/recorder/repositories/interface.js @@ -108,6 +108,20 @@ class RepositoryInterface { throw new Error(`#findByService method is not implemented in ${this.constructor.name}`); } + /** + * Get the IDs locating a version within the history of its terms: the first (oldest) and last (newest) versions, as well as the immediately previous (older) and next (newer) ones + * These IDs are computed from a single deterministic chronological order (by fetch date, with the record ID as a stable tiebreaker), so navigating prev/next always round-trips + * @param {string} serviceId - Service ID of the version + * @param {string} termsType - Terms type of the version + * @param {string} versionId - ID of the version to locate within its terms history + * @param {object} [options] - Query options + * @param {boolean} [options.includeTechnicalUpgrades] - When false, exclude technical upgrade records from the sequence. Default: true + * @returns {Promise<{first: ?string, prev: ?string, next: ?string, last: ?string}>} Promise resolved with the related version IDs, each null when there is none + */ + async getNavigationIds(serviceId, termsType, versionId, options = {}) { + throw new Error(`#getNavigationIds method is not implemented in ${this.constructor.name}`); + } + /** * Find all records for a specific service and terms type, in descending chronological order * For performance reasons, the content of the records will not be loaded by default. Use #loadRecordContent to load the content of individual records diff --git a/src/archivist/recorder/repositories/mongo/index.js b/src/archivist/recorder/repositories/mongo/index.js index e9d9168a1..5e6758c6d 100644 --- a/src/archivist/recorder/repositories/mongo/index.js +++ b/src/archivist/recorder/repositories/mongo/index.js @@ -159,6 +159,46 @@ export default class MongoRepository extends RepositoryInterface { .map(mongoDocument => this.#toDomain(mongoDocument, { deferContentLoading: true }))); } + async getNavigationIds(serviceId, termsType, versionId, { includeTechnicalUpgrades = true } = {}) { + const empty = { first: null, prev: null, next: null, last: null }; + + if (!ObjectId.isValid(versionId)) { + return empty; + } + + const _id = ObjectId.createFromHexString(versionId); + const filter = { serviceId, termsType }; + + if (!includeTechnicalUpgrades) { + filter.isTechnicalUpgrade = { $ne: true }; + } + + const current = await this.collection.findOne({ ...filter, _id }, { projection: { fetchDate: 1 } }); + + if (!current) { + return empty; + } + + const { fetchDate } = current; + const idOnly = { projection: { _id: 1 } }; + + // Deterministic total order (fetchDate, then _id) ascending; prev is the greatest record strictly before the current one, next the least strictly after. + // Using _id as a tiebreaker makes prev and next well-defined even for versions sharing the same fetch date, so navigation always round-trips. + const [ oldest, newest, previous, next ] = await Promise.all([ + this.collection.find(filter, idOnly).sort({ fetchDate: 1, _id: 1 }).limit(1).next(), + this.collection.find(filter, idOnly).sort({ fetchDate: -1, _id: -1 }).limit(1).next(), + this.collection.find({ ...filter, $or: [{ fetchDate: { $lt: fetchDate } }, { fetchDate, _id: { $lt: _id } }] }, idOnly).sort({ fetchDate: -1, _id: -1 }).limit(1).next(), + this.collection.find({ ...filter, $or: [{ fetchDate: { $gt: fetchDate } }, { fetchDate, _id: { $gt: _id } }] }, idOnly).sort({ fetchDate: 1, _id: 1 }).limit(1).next(), + ]); + + return { + first: oldest?._id?.toString() || null, + last: newest?._id?.toString() || null, + prev: previous?._id?.toString() || null, + next: next?._id?.toString() || null, + }; + } + count(serviceId, termsType) { const filter = {}; diff --git a/src/archivist/recorder/repositories/mongo/index.test.js b/src/archivist/recorder/repositories/mongo/index.test.js index 72b3f3b3d..1e8d420b5 100644 --- a/src/archivist/recorder/repositories/mongo/index.test.js +++ b/src/archivist/recorder/repositories/mongo/index.test.js @@ -988,6 +988,156 @@ describe('MongoRepository', () => { }); }); + describe('#getNavigationIds', () => { + let firstVersion; + let middleVersion; + let lastVersion; + + before(async () => { + firstVersion = await subject.save(new Version({ + serviceId: SERVICE_PROVIDER_ID, + termsType: TERMS_TYPE, + content: 'first content', + fetchDate: FETCH_DATE_EARLIER, + snapshotIds: [SNAPSHOT_ID], + })); + + middleVersion = await subject.save(new Version({ + serviceId: SERVICE_PROVIDER_ID, + termsType: TERMS_TYPE, + content: 'middle content', + fetchDate: FETCH_DATE, + snapshotIds: [SNAPSHOT_ID], + })); + + lastVersion = await subject.save(new Version({ + serviceId: SERVICE_PROVIDER_ID, + termsType: TERMS_TYPE, + content: 'last content', + fetchDate: FETCH_DATE_LATER, + snapshotIds: [SNAPSHOT_ID], + })); + }); + + after(() => subject.removeAll()); + + context('for the oldest version', () => { + let navigationIds; + + before(async () => { navigationIds = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, firstVersion.id); }); + + it('has no previous version', () => expect(navigationIds.prev).to.be.null); + it('points to the next version', () => expect(navigationIds.next).to.equal(middleVersion.id)); + it('points to itself as first', () => expect(navigationIds.first).to.equal(firstVersion.id)); + it('points to the newest version as last', () => expect(navigationIds.last).to.equal(lastVersion.id)); + }); + + context('for a middle version', () => { + let navigationIds; + + before(async () => { navigationIds = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, middleVersion.id); }); + + it('points to the previous version', () => expect(navigationIds.prev).to.equal(firstVersion.id)); + it('points to the next version', () => expect(navigationIds.next).to.equal(lastVersion.id)); + it('points to the oldest version as first', () => expect(navigationIds.first).to.equal(firstVersion.id)); + it('points to the newest version as last', () => expect(navigationIds.last).to.equal(lastVersion.id)); + }); + + context('for the newest version', () => { + let navigationIds; + + before(async () => { navigationIds = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, lastVersion.id); }); + + it('points to the previous version', () => expect(navigationIds.prev).to.equal(middleVersion.id)); + it('has no next version', () => expect(navigationIds.next).to.be.null); + it('points to itself as last', () => expect(navigationIds.last).to.equal(lastVersion.id)); + }); + + context('when the version does not exist', () => { + it('returns only null IDs', async () => { + expect(await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, 'ffffffffffffffffffffffff')).to.deep.equal({ first: null, prev: null, next: null, last: null }); + }); + }); + + context('when a technical upgrade is recorded out of chronological order', () => { + // A technical upgrade re-renders an old snapshot: it carries an OLD fetch date but is inserted last. + // The navigation IDs are derived from a single chronological order, so prev/next stay exact inverses. + const TECHNICAL_UPGRADE_DATE = new Date('2000-01-01T09:00:00.000Z'); // between firstVersion (06:00) and middleVersion (12:00) + let technicalUpgrade; + + before(async () => { + technicalUpgrade = await subject.save(new Version({ + serviceId: SERVICE_PROVIDER_ID, + termsType: TERMS_TYPE, + content: 'technical upgrade content', + fetchDate: TECHNICAL_UPGRADE_DATE, + isTechnicalUpgrade: true, + snapshotIds: [SNAPSHOT_ID], + })); + }); + + it('orders it by its fetch date, not its insertion order', async () => { + const navigationIds = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, technicalUpgrade.id); + + expect(navigationIds.prev).to.equal(firstVersion.id); + expect(navigationIds.next).to.equal(middleVersion.id); + }); + + it('keeps prev and next as exact inverses (round-trips)', async () => { + const fromFirst = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, firstVersion.id); + + expect(fromFirst.next).to.equal(technicalUpgrade.id); + + const backFromUpgrade = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, fromFirst.next); + + expect(backFromUpgrade.prev).to.equal(firstVersion.id); + }); + + it('can exclude technical upgrades from the sequence', async () => { + const navigationIds = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, firstVersion.id, { includeTechnicalUpgrades: false }); + + expect(navigationIds.next).to.equal(middleVersion.id); + }); + }); + }); + + describe('#getNavigationIds with versions sharing the same fetch date', () => { + // Distinct versions can share a fetch date; the _id tiebreaker must keep the order deterministic so prev/next still round-trip. + let ids; + + before(async () => { + ids = []; + for (const content of [ 'tie A', 'tie B', 'tie C' ]) { + ids.push((await subject.save(new Version({ serviceId: SERVICE_PROVIDER_ID, termsType: TERMS_TYPE, content, fetchDate: FETCH_DATE, snapshotIds: [SNAPSHOT_ID] }))).id); + } + }); + + after(() => subject.removeAll()); + + it('round-trips next then prev for every version', async () => { + for (const id of ids) { + const navigationIds = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, id); + + if (navigationIds.next) { + expect((await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, navigationIds.next)).prev).to.equal(id); + } + } + }); + + it('exposes all three versions as a single ordered chain', async () => { + const head = await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, ids[0]); + const walked = new Set([head.first]); + let cursor = head.first; + + while (cursor) { + walked.add(cursor); + cursor = (await subject.getNavigationIds(SERVICE_PROVIDER_ID, TERMS_TYPE, cursor)).next; + } + + expect(walked).to.have.lengthOf(ids.length); + }); + }); + describe('#findLatest', () => { context('when there are records for the given service', () => { let lastSnapshotId; From 59b0361fbdad22fc4add5dfae1795e76fd16c1a5 Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Wed, 1 Jul 2026 19:37:48 +0200 Subject: [PATCH 04/17] Add version diff stat to the recorder repositories --- .../recorder/repositories/git/git.js | 21 +++++++++++++++++++ .../recorder/repositories/git/index.js | 4 ++++ .../recorder/repositories/interface.js | 9 ++++++++ .../recorder/repositories/mongo/index.js | 5 +++++ 4 files changed, 39 insertions(+) diff --git a/src/archivist/recorder/repositories/git/git.js b/src/archivist/recorder/repositories/git/git.js index 2f11e935f..4902f2958 100644 --- a/src/archivist/recorder/repositories/git/git.js +++ b/src/archivist/recorder/repositories/git/git.js @@ -191,4 +191,25 @@ export default class Git { return { hash, timestamp: parseInt(timestamp, 10), subject: subjectParts.join('\t') }; }); } + + async getDiffStats(commitHash) { + const output = await this.git.raw([ 'show', '--numstat', '--format=', commitHash ]); + + let additions = 0; + let deletions = 0; + + for (const line of output.trim().split('\n')) { + if (!line) { + continue; + } + + const [ added, deleted ] = line.split('\t'); + + // Binary files show '-' for additions/deletions + if (added !== '-') { additions += parseInt(added, 10); } + if (deleted !== '-') { deletions += parseInt(deleted, 10); } + } + + return { additions, deletions }; + } } diff --git a/src/archivist/recorder/repositories/git/index.js b/src/archivist/recorder/repositories/git/index.js index e9de61ad3..8e89a1e56 100644 --- a/src/archivist/recorder/repositories/git/index.js +++ b/src/archivist/recorder/repositories/git/index.js @@ -190,6 +190,10 @@ export default class GitRepository extends RepositoryInterface { record.content = pdfBuffer; } + getDiffStats(recordId) { + return this.git.getDiffStats(recordId); + } + async #getCommits({ pathFilter, reverse = false, limit, offset, includeTechnicalUpgrades = true } = {}) { const prefixes = includeTechnicalUpgrades ? DataMapper.COMMIT_MESSAGE_PREFIXES diff --git a/src/archivist/recorder/repositories/interface.js b/src/archivist/recorder/repositories/interface.js index fd3632411..834b12962 100644 --- a/src/archivist/recorder/repositories/interface.js +++ b/src/archivist/recorder/repositories/interface.js @@ -175,6 +175,15 @@ class RepositoryInterface { async loadRecordContent(record) { throw new Error(`#loadRecordContent method is not implemented in ${this.constructor.name}`); } + + /** + * Get diff statistics for a specific record + * @param {string} recordId - Record ID to get diff stats for + * @returns {Promise<{additions: number, deletions: number}>} Promise that will be resolved with the diff statistics + */ + async getDiffStats(recordId) { + throw new Error(`#getDiffStats method is not implemented in ${this.constructor.name}`); + } } export default RepositoryInterface; diff --git a/src/archivist/recorder/repositories/mongo/index.js b/src/archivist/recorder/repositories/mongo/index.js index 5e6758c6d..b515e5709 100644 --- a/src/archivist/recorder/repositories/mongo/index.js +++ b/src/archivist/recorder/repositories/mongo/index.js @@ -233,6 +233,11 @@ export default class MongoRepository extends RepositoryInterface { record.content = content instanceof Binary ? content.buffer : content; } + // eslint-disable-next-line class-methods-use-this, no-unused-vars + getDiffStats(recordId) { + return { additions: null, deletions: null }; // Diff stats are not available for MongoDB storage + } + async #toDomain(mongoDocument, { deferContentLoading } = {}) { if (!mongoDocument) { return null; From 997e65ed50d7f1c4ef91a5d47d642b5b11bf2ef7 Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Wed, 1 Jul 2026 19:58:50 +0200 Subject: [PATCH 05/17] Add version endpoints to the Collection API --- src/collection-api/routes/versions.js | 327 ++++++++++++++++++++++++++ 1 file changed, 327 insertions(+) diff --git a/src/collection-api/routes/versions.js b/src/collection-api/routes/versions.js index 176ba0c55..a258bcbc7 100644 --- a/src/collection-api/routes/versions.js +++ b/src/collection-api/routes/versions.js @@ -11,6 +11,36 @@ import { toISODateWithoutMilliseconds } from '../../archivist/utils/date.js'; * name: Versions * description: Versions API * components: + * parameters: + * LimitParam: + * in: query + * name: limit + * description: | + * The maximum number of versions to return. + * + * **Note for Git storage**: Pagination uses Git's `--skip` and `--max-count` options, + * which work in topological order rather than strictly chronological order. + * This means paginated results may not be in perfect chronological sequence, + * but this is an acceptable performance trade-off. + * schema: + * type: integer + * minimum: 1 + * maximum: 500 + * default: 100 + * required: false + * OffsetParam: + * in: query + * name: offset + * description: | + * The number of versions to skip before returning results. + * + * **Note for Git storage**: Pagination uses Git's `--skip` and `--max-count` options, + * which work in topological order rather than strictly chronological order. + * schema: + * type: integer + * minimum: 0 + * default: 0 + * required: false * schemas: * Version: * type: object @@ -26,10 +56,307 @@ import { toISODateWithoutMilliseconds } from '../../archivist/utils/date.js'; * content: * type: string * description: The JSON-escaped Markdown content of the version + * VersionListItem: + * type: object + * properties: + * id: + * type: string + * description: The ID of the version. + * serviceId: + * type: string + * description: The ID of the service. + * termsType: + * type: string + * description: The type of terms. + * fetchDate: + * type: string + * format: date-time + * description: The ISO 8601 datetime string when the version was recorded. + * isFirstRecord: + * type: boolean + * description: Whether this version is the first one recorded for this service and terms type. + * isTechnicalUpgrade: + * type: boolean + * description: Whether this version is a technical upgrade (a re-render of an existing snapshot) rather than a content change at the source. + * VersionListItemWithStats: + * allOf: + * - $ref: '#/components/schemas/VersionListItem' + * - type: object + * properties: + * additions: + * type: integer + * nullable: true + * description: The number of lines added in this version, or null if not available. + * deletions: + * type: integer + * nullable: true + * description: The number of lines deleted in this version, or null if not available. + * PaginatedVersionsResponse: + * type: object + * properties: + * data: + * type: array + * description: The list of versions. + * items: + * $ref: '#/components/schemas/VersionListItem' + * count: + * type: integer + * description: The total number of versions found. + * limit: + * type: integer + * description: The maximum number of versions returned in this response. + * offset: + * type: integer + * description: The number of versions skipped before returning results. + * PaginatedVersionsWithStatsResponse: + * type: object + * properties: + * data: + * type: array + * description: The list of versions with diff statistics. + * items: + * $ref: '#/components/schemas/VersionListItemWithStats' + * count: + * type: integer + * description: The total number of versions found. + * limit: + * type: integer + * description: The maximum number of versions returned in this response. + * offset: + * type: integer + * description: The number of versions skipped before returning results. + * ErrorResponse: + * type: object + * properties: + * error: + * type: string + * description: Error message. + * responses: + * BadRequestError: + * description: Invalid pagination parameters. + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/ErrorResponse' + * NotFoundError: + * description: Resource not found. + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/ErrorResponse' */ export default function versionsRouter(versionsRepository) { const router = express.Router(); + function parsePaginationParams(query) { + const limit = query.limit ? parseInt(query.limit, 10) : 100; + const offset = query.offset ? parseInt(query.offset, 10) : 0; + + return { limit, offset }; + } + + function validatePaginationParams(limit, offset) { + if (Number.isNaN(limit) || limit < 1) { + return { error: 'Invalid limit parameter. Must be a positive integer.' }; + } + + if (limit > 500) { + return { error: 'Invalid limit parameter. Must not exceed 500.' }; + } + + if (Number.isNaN(offset) || offset < 0) { + return { error: 'Invalid offset parameter. Must be a non-negative integer.' }; + } + + return null; + } + + function mapVersionToListItem(version) { + return { + id: version.id, + serviceId: version.serviceId, + termsType: version.termsType, + fetchDate: toISODateWithoutMilliseconds(version.fetchDate), + isFirstRecord: version.isFirstRecord, + isTechnicalUpgrade: version.isTechnicalUpgrade, + }; + } + + /** + * @private + * @swagger + * /versions: + * get: + * summary: Get all versions. + * tags: [Versions] + * produces: + * - application/json + * parameters: + * - $ref: '#/components/parameters/LimitParam' + * - $ref: '#/components/parameters/OffsetParam' + * responses: + * 200: + * description: A JSON object containing the list of all versions and metadata. + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/PaginatedVersionsResponse' + * 400: + * $ref: '#/components/responses/BadRequestError' + */ + router.get('/versions', async (req, res) => { + const { limit, offset } = parsePaginationParams(req.query); + const validationError = validatePaginationParams(limit, offset); + + if (validationError) { + return res.status(400).json(validationError); + } + + const paginatedVersions = await versionsRepository.findAll({ limit, offset }); + + const versionsList = paginatedVersions.map(mapVersionToListItem); + + const response = { + data: versionsList, + count: await versionsRepository.count(), + limit, + offset, + }; + + return res.status(200).json(response); + }); + + /** + * @private + * @swagger + * /versions/{serviceId}: + * get: + * summary: Get all versions for a specific service. + * tags: [Versions] + * produces: + * - application/json + * parameters: + * - in: path + * name: serviceId + * description: The ID of the service whose versions will be returned. + * schema: + * type: string + * required: true + * - $ref: '#/components/parameters/LimitParam' + * - $ref: '#/components/parameters/OffsetParam' + * responses: + * 200: + * description: A JSON object containing the list of versions and metadata. + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/PaginatedVersionsResponse' + * 400: + * $ref: '#/components/responses/BadRequestError' + * 404: + * $ref: '#/components/responses/NotFoundError' + */ + router.get('/versions/:serviceId', async (req, res) => { + const { serviceId } = req.params; + const { limit, offset } = parsePaginationParams(req.query); + const validationError = validatePaginationParams(limit, offset); + + if (validationError) { + return res.status(400).json(validationError); + } + + const totalCount = await versionsRepository.count(serviceId); + + if (totalCount === 0) { + return res.status(404).json({ error: `No versions found for service "${serviceId}"` }); + } + + const paginatedVersions = await versionsRepository.findByService(serviceId, { limit, offset }); + + const versionsList = paginatedVersions.map(mapVersionToListItem); + + const response = { + data: versionsList, + count: totalCount, + limit, + offset, + }; + + return res.status(200).json(response); + }); + + /** + * @private + * @swagger + * /versions/{serviceId}/{termsType}: + * get: + * summary: Get all versions of some terms for a specific service. + * tags: [Versions] + * produces: + * - application/json + * parameters: + * - in: path + * name: serviceId + * description: The ID of the service whose versions will be returned. + * schema: + * type: string + * required: true + * - in: path + * name: termsType + * description: The type of terms whose versions will be returned. + * schema: + * type: string + * required: true + * - $ref: '#/components/parameters/LimitParam' + * - $ref: '#/components/parameters/OffsetParam' + * responses: + * 200: + * description: A JSON object containing the list of versions with diff statistics and metadata. + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/PaginatedVersionsWithStatsResponse' + * 400: + * $ref: '#/components/responses/BadRequestError' + * 404: + * $ref: '#/components/responses/NotFoundError' + */ + router.get('/versions/:serviceId/:termsType', async (req, res) => { + const { serviceId, termsType } = req.params; + const { limit, offset } = parsePaginationParams(req.query); + const validationError = validatePaginationParams(limit, offset); + + if (validationError) { + return res.status(400).json(validationError); + } + + const totalCount = await versionsRepository.count(serviceId, termsType); + + if (totalCount === 0) { + return res.status(404).json({ error: `No versions found for service "${serviceId}" and terms type "${termsType}"` }); + } + + const paginatedVersions = await versionsRepository.findByServiceAndTermsType(serviceId, termsType, { limit, offset }); + + const versionsList = await Promise.all(paginatedVersions.map(async version => { + const stats = await versionsRepository.getDiffStats(version.id); + + return { + ...mapVersionToListItem(version), + ...stats, + }; + })); + + const response = { + data: versionsList, + count: totalCount, + limit, + offset, + }; + + return res.status(200).json(response); + }); + /** * @private * @swagger From 254bd820ad784f123810be154180ea835d3691dd Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Wed, 1 Jul 2026 19:59:30 +0200 Subject: [PATCH 06/17] Add single-version endpoints to the Collection API --- src/collection-api/routes/index.js | 3 +- src/collection-api/routes/versions.js | 223 +++++++- src/collection-api/routes/versions.test.js | 620 ++++++++++++++++++++- 3 files changed, 792 insertions(+), 54 deletions(-) diff --git a/src/collection-api/routes/index.js b/src/collection-api/routes/index.js index c4ae5ec33..85f446086 100644 --- a/src/collection-api/routes/index.js +++ b/src/collection-api/routes/index.js @@ -38,6 +38,7 @@ export default async function apiRouter(basePath) { const collection = await getCollection(); const versionsStorageConfig = config.get('@opentermsarchive/engine.recorder.versions.storage'); const versionsRepository = await RepositoryFactory.create(versionsStorageConfig).initialize(); + const snapshotsRepository = await RepositoryFactory.create(config.get('@opentermsarchive/engine.recorder.snapshots.storage')).initialize(); const feedConfig = config.get('@opentermsarchive/engine.collection-api.feed'); if (!collection.metadata?.id) { @@ -50,7 +51,7 @@ export default async function apiRouter(basePath) { router.use(await metadataRouter(collection, services)); router.use(servicesRouter(services)); - router.use(versionsRouter(versionsRepository)); + router.use(versionsRouter(versionsRepository, snapshotsRepository)); router.use(feedRouter(services, versionsRepository, versionsStorageConfig.type, feedConfig.limit, feedConfig.versionUrlTemplate)); return router; diff --git a/src/collection-api/routes/versions.js b/src/collection-api/routes/versions.js index a258bcbc7..8df032544 100644 --- a/src/collection-api/routes/versions.js +++ b/src/collection-api/routes/versions.js @@ -1,10 +1,12 @@ import express from 'express'; import { toISODateWithoutMilliseconds } from '../../archivist/utils/date.js'; +import logger from '../logger.js'; /** - * @param {object} versionsRepository The versions repository instance - * @returns {express.Router} The router instance + * @param {object} versionsRepository The versions repository instance + * @param {object} snapshotsRepository The snapshots repository instance + * @returns {express.Router} The router instance * @private * @swagger * tags: @@ -46,13 +48,25 @@ import { toISODateWithoutMilliseconds } from '../../archivist/utils/date.js'; * type: object * description: Version content and metadata * properties: + * id: + * type: string + * description: The ID of the version. + * serviceId: + * type: string + * description: The ID of the service. + * termsType: + * type: string + * description: The type of terms. * fetchDate: * type: string * format: date-time * description: The ISO 8601 datetime string when the version was recorded. - * id: - * type: string - * description: The ID of the version. + * isFirstRecord: + * type: boolean + * description: Whether this version is the first one recorded for this service and terms type. + * isTechnicalUpgrade: + * type: boolean + * description: Whether this version is a technical upgrade (a re-render of an existing snapshot) rather than a content change at the source. * content: * type: string * description: The JSON-escaped Markdown content of the version @@ -125,6 +139,45 @@ import { toISODateWithoutMilliseconds } from '../../archivist/utils/date.js'; * offset: * type: integer * description: The number of versions skipped before returning results. + * VersionWithLinks: + * allOf: + * - $ref: '#/components/schemas/Version' + * - type: object + * properties: + * additions: + * type: integer + * nullable: true + * description: The number of lines added in this version, or null if not available. + * deletions: + * type: integer + * nullable: true + * description: The number of lines deleted in this version, or null if not available. + * fetchUrls: + * type: array + * description: The URLs of the source documents that were fetched to produce this version. + * items: + * type: string + * format: uri + * links: + * type: object + * description: Navigation links to related versions. + * properties: + * first: + * type: string + * description: The ID of the first version for this service and terms type. + * nullable: true + * prev: + * type: string + * description: The ID of the previous version, or null if this is the first. + * nullable: true + * next: + * type: string + * description: The ID of the next version, or null if this is the last. + * nullable: true + * last: + * type: string + * description: The ID of the last version for this service and terms type. + * nullable: true * ErrorResponse: * type: object * properties: @@ -145,7 +198,7 @@ import { toISODateWithoutMilliseconds } from '../../archivist/utils/date.js'; * schema: * $ref: '#/components/schemas/ErrorResponse' */ -export default function versionsRouter(versionsRepository) { +export default function versionsRouter(versionsRepository, snapshotsRepository) { const router = express.Router(); function parsePaginationParams(query) { @@ -182,6 +235,54 @@ export default function versionsRouter(versionsRepository) { }; } + async function getFetchUrls(snapshotIds) { + if (!snapshotIds?.length) { + return []; + } + + const snapshots = await Promise.all(snapshotIds.map(async id => { + const snapshot = await snapshotsRepository.findMetadataById(id); + + if (!snapshot) { + logger.warn(`Could not resolve source snapshot ${id}; its fetch URL will be missing from the version. The snapshots repository is likely missing or out of sync with the versions repository.`); + } + + return snapshot; + })); + + return snapshots + .filter(Boolean) + .map(snapshot => snapshot.metadata?.['x-source-document-location']) + .filter(Boolean); + } + + // Builds the full detail response shared by every single-version endpoint, so they cannot drift apart + async function buildVersionDetail(version) { + const [ navigationIds, stats, fetchUrls ] = await Promise.all([ + versionsRepository.getNavigationIds(version.serviceId, version.termsType, version.id), + versionsRepository.getDiffStats(version.id), + getFetchUrls(version.snapshotIds), + ]); + + return { + id: version.id, + serviceId: version.serviceId, + termsType: version.termsType, + fetchDate: toISODateWithoutMilliseconds(version.fetchDate), + content: version.content, + isFirstRecord: version.isFirstRecord, + isTechnicalUpgrade: version.isTechnicalUpgrade, + fetchUrls, + links: { + first: navigationIds.first, + prev: navigationIds.prev, + next: navigationIds.next, + last: navigationIds.last, + }, + ...stats, + }; + } + /** * @private * @swagger @@ -357,6 +458,88 @@ export default function versionsRouter(versionsRepository) { return res.status(200).json(response); }); + /** + * @private + * @swagger + * /version/{versionId}: + * get: + * summary: Get a specific version by its ID. + * tags: [Versions] + * produces: + * - application/json + * parameters: + * - in: path + * name: versionId + * description: The ID of the version to retrieve. + * schema: + * type: string + * required: true + * responses: + * 200: + * description: A JSON object containing the version content, metadata, and navigation links. + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/VersionWithLinks' + * 404: + * $ref: '#/components/responses/NotFoundError' + */ + router.get('/version/:versionId', async (req, res) => { + const { versionId } = req.params; + + const version = await versionsRepository.findById(versionId); + + if (!version) { + return res.status(404).json({ error: `No version found with ID "${versionId}"` }); + } + + return res.status(200).json(await buildVersionDetail(version)); + }); + + /** + * @private + * @swagger + * /version/{serviceId}/{termsType}/latest: + * get: + * summary: Get the latest version of some terms for a service. + * tags: [Versions] + * produces: + * - application/json + * parameters: + * - in: path + * name: serviceId + * description: The ID of the service whose version will be returned. + * schema: + * type: string + * required: true + * - in: path + * name: termsType + * description: The type of terms whose version will be returned. + * schema: + * type: string + * required: true + * responses: + * 200: + * description: A JSON object containing the version content, metadata, and navigation links. + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/VersionWithLinks' + * 404: + * $ref: '#/components/responses/NotFoundError' + */ + router.get('/version/:serviceId/:termsType/latest', async (req, res) => { + const { serviceId, termsType } = req.params; + + const version = await versionsRepository.findLatest(serviceId, termsType); + + if (!version) { + return res.status(404).json({ error: `No version found for service "${serviceId}" and terms type "${termsType}"` }); + } + + return res.status(200).json(await buildVersionDetail(version)); + }); + /** * @private * @swagger @@ -388,31 +571,19 @@ export default function versionsRouter(versionsRepository) { * required: true * responses: * 200: - * description: A JSON object containing the version content and metadata. + * description: A JSON object containing the version content, metadata, and navigation links. * content: * application/json: * schema: - * $ref: '#/components/schemas/Version' + * $ref: '#/components/schemas/VersionWithLinks' * 404: - * description: No version found for the specified combination of service ID, terms type and date. - * content: - * application/json: - * schema: - * type: object - * properties: - * error: - * type: string - * description: Error message indicating that no version is found. + * $ref: '#/components/responses/NotFoundError' * 416: * description: The requested date is in the future. * content: * application/json: * schema: - * type: object - * properties: - * error: - * type: string - * description: Error message indicating that the requested date is in the future. + * $ref: '#/components/schemas/ErrorResponse' */ router.get('/version/:serviceId/:termsType/:date', async (req, res) => { const { serviceId, termsType, date } = req.params; @@ -425,14 +596,10 @@ export default function versionsRouter(versionsRepository) { const version = await versionsRepository.findByDate(serviceId, termsType, requestedDate); if (!version) { - return res.status(404).json({ error: `No version found for date ${date}` }); + return res.status(404).json({ error: `No version found for service "${serviceId}" and terms type "${termsType}" at date ${date}` }); } - return res.status(200).json({ - id: version.id, - fetchDate: toISODateWithoutMilliseconds(version.fetchDate), - content: version.content, - }); + return res.status(200).json(await buildVersionDetail(version)); }); return router; diff --git a/src/collection-api/routes/versions.test.js b/src/collection-api/routes/versions.test.js index bfdff4e15..432a71eb3 100644 --- a/src/collection-api/routes/versions.test.js +++ b/src/collection-api/routes/versions.test.js @@ -12,57 +12,622 @@ const basePath = config.get('@opentermsarchive/engine.collection-api.basePath'); const request = supertest(app); describe('Versions API', () => { - describe('GET /version/:serviceId/:termsType/:date', () => { - let expectedResult; - let versionsRepository; - const FETCH_DATE = new Date('2023-01-01T12:00:00Z'); - const VERSION_COMMON_ATTRIBUTES = { - serviceId: 'service·A', - termsType: 'Terms of Service', - snapshotId: ['snapshot_id'], - }; + let versionsRepository; + const FETCH_DATE = new Date('2023-01-01T12:00:00Z'); + const VERSION_COMMON_ATTRIBUTES = { + serviceId: 'service-1', + termsType: 'Terms of Service', + snapshotId: ['snapshot_id'], + }; - before(async () => { - versionsRepository = RepositoryFactory.create(config.get('@opentermsarchive/engine.recorder.versions.storage')); + before(async () => { + versionsRepository = RepositoryFactory.create(config.get('@opentermsarchive/engine.recorder.versions.storage')); + await versionsRepository.initialize(); + }); + + after(() => versionsRepository.removeAll()); - await versionsRepository.initialize(); + describe('GET /versions/:serviceId/:termsType', () => { + let version1; + let version2; + let version3; + before(async () => { const ONE_HOUR = 60 * 60 * 1000; - await versionsRepository.save(new Version({ + version1 = new Version({ ...VERSION_COMMON_ATTRIBUTES, content: 'initial content', fetchDate: new Date(new Date(FETCH_DATE).getTime() - ONE_HOUR), - })); + }); + await versionsRepository.save(version1); - const version = new Version({ + version2 = new Version({ ...VERSION_COMMON_ATTRIBUTES, content: 'updated content', fetchDate: FETCH_DATE, }); + await versionsRepository.save(version2); - await versionsRepository.save(version); + version3 = new Version({ + ...VERSION_COMMON_ATTRIBUTES, + content: 'latest content', + fetchDate: new Date(new Date(FETCH_DATE).getTime() + ONE_HOUR), + }); + await versionsRepository.save(version3); await versionsRepository.save(new Version({ + serviceId: 'service-2', + termsType: 'Privacy Policy', + snapshotId: ['snapshot_id'], + content: 'other service content', + fetchDate: FETCH_DATE, + })); + }); + + let response; + + context('when versions are found', () => { + before(async () => { + response = await request.get(`${basePath}/v1/versions/service-1/Terms%20of%20Service`); + }); + + it('responds with 200 status code', () => { + expect(response.status).to.equal(200); + }); + + it('responds with Content-Type application/json', () => { + expect(response.type).to.equal('application/json'); + }); + + it('returns response with metadata structure', () => { + expect(response.body).to.have.all.keys('data', 'count', 'limit', 'offset'); + }); + + it('returns all versions for the service and terms type', () => { + expect(response.body.data).to.be.an('array').with.lengthOf(3); + }); + + it('returns correct count', () => { + expect(response.body.count).to.equal(3); + }); + + it('returns versions with id, serviceId, termsType, fetchDate, isFirstRecord, isTechnicalUpgrade, additions and deletions', () => { + response.body.data.forEach(version => { + expect(version).to.have.all.keys('id', 'serviceId', 'termsType', 'fetchDate', 'isFirstRecord', 'isTechnicalUpgrade', 'additions', 'deletions'); + expect(version).to.not.have.property('content'); + }); + }); + + it('returns versions with correct serviceId and termsType', () => { + response.body.data.forEach(version => { + expect(version.serviceId).to.equal('service-1'); + expect(version.termsType).to.equal('Terms of Service'); + }); + }); + + it('returns versions in reverse chronological order', () => { + expect(response.body.data[0].id).to.equal(version3.id); + expect(response.body.data[1].id).to.equal(version2.id); + expect(response.body.data[2].id).to.equal(version1.id); + }); + + it('returns versions with correct fetchDates', () => { + expect(response.body.data[0].fetchDate).to.equal(toISODateWithoutMilliseconds(version3.fetchDate)); + expect(response.body.data[1].fetchDate).to.equal(toISODateWithoutMilliseconds(version2.fetchDate)); + expect(response.body.data[2].fetchDate).to.equal(toISODateWithoutMilliseconds(version1.fetchDate)); + }); + }); + + context('with pagination', () => { + context('with default limit (no query parameters)', () => { + before(async () => { + response = await request.get(`${basePath}/v1/versions/service-1/Terms%20of%20Service`); + }); + + it('responds with 200 status code', () => { + expect(response.status).to.equal(200); + }); + + it('returns all versions when total is less than default limit', () => { + expect(response.body.data).to.be.an('array').with.lengthOf(3); + }); + + it('includes default pagination metadata', () => { + expect(response.body).to.have.property('limit', 100); + expect(response.body).to.have.property('offset', 0); + }); + }); + + context('with limit parameter', () => { + before(async () => { + response = await request.get(`${basePath}/v1/versions/service-1/Terms%20of%20Service?limit=2`); + }); + + it('responds with 200 status code', () => { + expect(response.status).to.equal(200); + }); + + it('returns limited number of versions', () => { + expect(response.body.data).to.be.an('array').with.lengthOf(2); + }); + + it('returns correct total count', () => { + expect(response.body.count).to.equal(3); + }); + + it('includes pagination metadata', () => { + expect(response.body).to.have.property('limit', 2); + expect(response.body).to.have.property('offset', 0); + }); + + it('returns first two versions in reverse chronological order', () => { + expect(response.body.data[0].id).to.equal(version3.id); + expect(response.body.data[1].id).to.equal(version2.id); + }); + }); + + context('with limit and offset parameters', () => { + before(async () => { + response = await request.get(`${basePath}/v1/versions/service-1/Terms%20of%20Service?limit=1&offset=1`); + }); + + it('responds with 200 status code', () => { + expect(response.status).to.equal(200); + }); + + it('returns limited number of versions starting from offset', () => { + expect(response.body.data).to.be.an('array').with.lengthOf(1); + }); + + it('returns correct total count', () => { + expect(response.body.count).to.equal(3); + }); + + it('includes pagination metadata', () => { + expect(response.body).to.have.property('limit', 1); + expect(response.body).to.have.property('offset', 1); + }); + + it('returns second version', () => { + expect(response.body.data[0].id).to.equal(version2.id); + }); + }); + + context('with only offset parameter', () => { + before(async () => { + response = await request.get(`${basePath}/v1/versions/service-1/Terms%20of%20Service?offset=1`); + }); + + it('responds with 200 status code', () => { + expect(response.status).to.equal(200); + }); + + it('returns all versions starting from offset', () => { + expect(response.body.data).to.be.an('array').with.lengthOf(2); + }); + + it('returns correct total count', () => { + expect(response.body.count).to.equal(3); + }); + + it('includes offset in metadata', () => { + expect(response.body).to.have.property('offset', 1); + }); + + it('returns last two versions', () => { + expect(response.body.data[0].id).to.equal(version2.id); + expect(response.body.data[1].id).to.equal(version1.id); + }); + }); + + context('with invalid limit parameter (too small)', () => { + before(async () => { + response = await request.get(`${basePath}/v1/versions/service-1/Terms%20of%20Service?limit=0`); + }); + + it('responds with 400 status code', () => { + expect(response.status).to.equal(400); + }); + + it('returns error message', () => { + expect(response.body).to.have.property('error'); + expect(response.body.error).to.include('Invalid limit parameter'); + }); + }); + + context('with invalid limit parameter (exceeds maximum)', () => { + before(async () => { + response = await request.get(`${basePath}/v1/versions/service-1/Terms%20of%20Service?limit=501`); + }); + + it('responds with 400 status code', () => { + expect(response.status).to.equal(400); + }); + + it('returns error message', () => { + expect(response.body).to.have.property('error'); + expect(response.body.error).to.include('Invalid limit parameter'); + expect(response.body.error).to.include('500'); + }); + }); + + context('with invalid offset parameter', () => { + before(async () => { + response = await request.get(`${basePath}/v1/versions/service-1/Terms%20of%20Service?offset=-1`); + }); + + it('responds with 400 status code', () => { + expect(response.status).to.equal(400); + }); + + it('returns error message', () => { + expect(response.body).to.have.property('error'); + expect(response.body.error).to.include('Invalid offset parameter'); + }); + }); + }); + + context('when no versions are found', () => { + before(async () => { + response = await request.get(`${basePath}/v1/versions/non-existent-service/Terms%20of%20Service`); + }); + + it('responds with 404 status code', () => { + expect(response.status).to.equal(404); + }); + + it('responds with Content-Type application/json', () => { + expect(response.type).to.equal('application/json'); + }); + + it('returns an error message', () => { + expect(response.body.error).to.contain('No versions found').and.to.contain('non-existent-service').and.to.contain('Terms of Service'); + }); + }); + }); + + describe('GET /version/:versionId', () => { + const ONE_HOUR = 60 * 60 * 1000; + let firstVersion; + let middleVersion; + let lastVersion; + + before(async () => { + await versionsRepository.removeAll(); + + firstVersion = new Version({ + ...VERSION_COMMON_ATTRIBUTES, + content: 'first content', + fetchDate: new Date(FETCH_DATE.getTime() - ONE_HOUR), + }); + await versionsRepository.save(firstVersion); + + middleVersion = new Version({ + ...VERSION_COMMON_ATTRIBUTES, + content: 'middle content', + fetchDate: FETCH_DATE, + }); + await versionsRepository.save(middleVersion); + + lastVersion = new Version({ + ...VERSION_COMMON_ATTRIBUTES, + content: 'last content', + fetchDate: new Date(FETCH_DATE.getTime() + ONE_HOUR), + }); + await versionsRepository.save(lastVersion); + }); + + let response; + + context('when requesting the first version', () => { + before(async () => { + response = await request.get(`${basePath}/v1/version/${firstVersion.id}`); + }); + + it('responds with 200 status code', () => { + expect(response.status).to.equal(200); + }); + + it('responds with Content-Type application/json', () => { + expect(response.type).to.equal('application/json'); + }); + + it('returns the version content and metadata', () => { + expect(response.body.id).to.equal(firstVersion.id); + expect(response.body.serviceId).to.equal(firstVersion.serviceId); + expect(response.body.termsType).to.equal(firstVersion.termsType); + expect(response.body.fetchDate).to.equal(toISODateWithoutMilliseconds(firstVersion.fetchDate)); + expect(response.body.content).to.equal(firstVersion.content); + }); + + it('returns fetchUrls as an empty array when no matching snapshots exist', () => { + expect(response.body.fetchUrls).to.deep.equal([]); + }); + + it('returns links object', () => { + expect(response.body.links).to.be.an('object'); + }); + + it('returns first link pointing to itself', () => { + expect(response.body.links.first).to.equal(firstVersion.id); + }); + + it('returns null for prev', () => { + expect(response.body.links.prev).to.be.null; + }); + + it('returns next link', () => { + expect(response.body.links.next).to.equal(middleVersion.id); + }); + + it('returns last link', () => { + expect(response.body.links.last).to.equal(lastVersion.id); + }); + }); + + context('when requesting a middle version', () => { + before(async () => { + response = await request.get(`${basePath}/v1/version/${middleVersion.id}`); + }); + + it('responds with 200 status code', () => { + expect(response.status).to.equal(200); + }); + + it('returns the version content and metadata', () => { + expect(response.body.id).to.equal(middleVersion.id); + expect(response.body.serviceId).to.equal(middleVersion.serviceId); + expect(response.body.termsType).to.equal(middleVersion.termsType); + expect(response.body.content).to.equal(middleVersion.content); + }); + + it('returns first link', () => { + expect(response.body.links.first).to.equal(firstVersion.id); + }); + + it('returns prev link', () => { + expect(response.body.links.prev).to.equal(firstVersion.id); + }); + + it('returns next link', () => { + expect(response.body.links.next).to.equal(lastVersion.id); + }); + + it('returns last link', () => { + expect(response.body.links.last).to.equal(lastVersion.id); + }); + }); + + context('when requesting the last version', () => { + before(async () => { + response = await request.get(`${basePath}/v1/version/${lastVersion.id}`); + }); + + it('responds with 200 status code', () => { + expect(response.status).to.equal(200); + }); + + it('returns the version content and metadata', () => { + expect(response.body.id).to.equal(lastVersion.id); + expect(response.body.serviceId).to.equal(lastVersion.serviceId); + expect(response.body.termsType).to.equal(lastVersion.termsType); + expect(response.body.content).to.equal(lastVersion.content); + }); + + it('returns first link', () => { + expect(response.body.links.first).to.equal(firstVersion.id); + }); + + it('returns prev link', () => { + expect(response.body.links.prev).to.equal(middleVersion.id); + }); + + it('returns null for next', () => { + expect(response.body.links.next).to.be.null; + }); + + it('returns last link pointing to itself', () => { + expect(response.body.links.last).to.equal(lastVersion.id); + }); + }); + + context('when the version does not exist', () => { + before(async () => { + response = await request.get(`${basePath}/v1/version/non-existent-id`); + }); + + it('responds with 404 status code', () => { + expect(response.status).to.equal(404); + }); + + it('responds with Content-Type application/json', () => { + expect(response.type).to.equal('application/json'); + }); + + it('returns an error message', () => { + expect(response.body.error).to.contain('No version found').and.to.contain('non-existent-id'); + }); + }); + + context('when a referenced snapshot is absent from the repository', () => { + let versionWithMissingSnapshot; + + before(async () => { + versionWithMissingSnapshot = new Version({ + serviceId: 'service-missing-snapshot', + termsType: 'Terms of Service', + content: 'content referencing a snapshot that is missing from the repository', + fetchDate: FETCH_DATE, + snapshotIds: ['ecd9407eb26b1bf0613186175ee80edbdeedd47f'], // well-formed ID that is absent from the snapshots repository + }); + await versionsRepository.save(versionWithMissingSnapshot); + response = await request.get(`${basePath}/v1/version/${versionWithMissingSnapshot.id}`); + }); + + it('responds with 200 rather than failing on the unresolved snapshot', () => { + expect(response.status).to.equal(200); + }); + + it('degrades fetchUrls to an empty array', () => { + expect(response.body.fetchUrls).to.deep.equal([]); + }); + }); + }); + + describe('GET /version/:serviceId/:termsType/latest', () => { + let firstVersion; + let middleVersion; + let lastVersion; + + before(async () => { + await versionsRepository.removeAll(); + + const ONE_HOUR = 60 * 60 * 1000; + + firstVersion = await versionsRepository.save(new Version({ + ...VERSION_COMMON_ATTRIBUTES, + content: 'first content', + fetchDate: new Date(FETCH_DATE.getTime() - ONE_HOUR), + })); + + middleVersion = await versionsRepository.save(new Version({ + ...VERSION_COMMON_ATTRIBUTES, + content: 'middle content', + fetchDate: FETCH_DATE, + })); + + lastVersion = await versionsRepository.save(new Version({ + ...VERSION_COMMON_ATTRIBUTES, + content: 'last content', + fetchDate: new Date(FETCH_DATE.getTime() + ONE_HOUR), + })); + }); + + let response; + + context('when versions exist', () => { + before(async () => { + response = await request.get(`${basePath}/v1/version/service-1/Terms%20of%20Service/latest`); + }); + + it('responds with 200 status code', () => { + expect(response.status).to.equal(200); + }); + + it('responds with Content-Type application/json', () => { + expect(response.type).to.equal('application/json'); + }); + + it('returns the latest version', () => { + expect(response.body.id).to.equal(lastVersion.id); + expect(response.body.serviceId).to.equal(lastVersion.serviceId); + expect(response.body.termsType).to.equal(lastVersion.termsType); + expect(response.body.content).to.equal(lastVersion.content); + expect(response.body.fetchDate).to.equal(toISODateWithoutMilliseconds(lastVersion.fetchDate)); + }); + + it('returns fetchUrls as an empty array when no matching snapshots exist', () => { + expect(response.body.fetchUrls).to.deep.equal([]); + }); + + it('returns diff stats (additions and deletions)', () => { + expect(response.body).to.have.property('additions'); + expect(response.body).to.have.property('deletions'); + }); + + it('returns links object', () => { + expect(response.body.links).to.be.an('object'); + }); + + it('returns first link', () => { + expect(response.body.links.first).to.equal(firstVersion.id); + }); + + it('returns prev link', () => { + expect(response.body.links.prev).to.equal(middleVersion.id); + }); + + it('returns null for next', () => { + expect(response.body.links.next).to.be.null; + }); + + it('returns last link pointing to itself', () => { + expect(response.body.links.last).to.equal(lastVersion.id); + }); + }); + + context('when no versions exist', () => { + before(async () => { + response = await request.get(`${basePath}/v1/version/non-existent-service/Non%20Existent%20Terms/latest`); + }); + + it('responds with 404 status code', () => { + expect(response.status).to.equal(404); + }); + + it('responds with Content-Type application/json', () => { + expect(response.type).to.equal('application/json'); + }); + + it('returns an error message', () => { + expect(response.body.error).to.contain('No version found').and.to.contain('non-existent-service'); + }); + }); + }); + + describe('GET /version/:serviceId/:termsType/:date', () => { + let expectedResult; + let firstVersion; + let middleVersion; + let lastVersion; + + before(async () => { + await versionsRepository.removeAll(); + + const ONE_HOUR = 60 * 60 * 1000; + + firstVersion = await versionsRepository.save(new Version({ + ...VERSION_COMMON_ATTRIBUTES, + content: 'initial content', + fetchDate: new Date(new Date(FETCH_DATE).getTime() - ONE_HOUR), + })); + + middleVersion = await versionsRepository.save(new Version({ + ...VERSION_COMMON_ATTRIBUTES, + content: 'updated content', + fetchDate: FETCH_DATE, + })); + + lastVersion = await versionsRepository.save(new Version({ ...VERSION_COMMON_ATTRIBUTES, content: 'latest content', fetchDate: new Date(new Date(FETCH_DATE).getTime() + ONE_HOUR), })); expectedResult = { - id: version.id, - fetchDate: toISODateWithoutMilliseconds(version.fetchDate), - content: version.content, + id: middleVersion.id, + serviceId: middleVersion.serviceId, + termsType: middleVersion.termsType, + fetchDate: toISODateWithoutMilliseconds(middleVersion.fetchDate), + content: middleVersion.content, + isFirstRecord: false, + isTechnicalUpgrade: false, + fetchUrls: [], + links: { + first: firstVersion.id, + prev: firstVersion.id, + next: lastVersion.id, + last: lastVersion.id, + }, }; }); - after(() => versionsRepository.removeAll()); - let response; context('when a version is found', () => { before(async () => { - response = await request.get(`${basePath}/v1/version/service·A/Terms%20of%20Service/${encodeURIComponent(toISODateWithoutMilliseconds(FETCH_DATE))}`); + response = await request.get(`${basePath}/v1/version/service-1/Terms%20of%20Service/${encodeURIComponent(toISODateWithoutMilliseconds(FETCH_DATE))}`); }); it('responds with 200 status code', () => { @@ -74,13 +639,18 @@ describe('Versions API', () => { }); it('returns the expected version', () => { - expect(response.body).to.deep.equal(expectedResult); + expect(response.body).to.deep.include(expectedResult); + }); + + it('returns diff stats (additions and deletions)', () => { + expect(response.body).to.have.property('additions'); + expect(response.body).to.have.property('deletions'); }); }); context('when the requested date is anterior to the first available version', () => { before(async () => { - response = await request.get(`${basePath}/v1/version/service·A/Terms%20of%20Service/2000-01-01T12:00:00Z`); + response = await request.get(`${basePath}/v1/version/service-1/Terms%20of%20Service/2000-01-01T12:00:00Z`); }); it('responds with 404 status code', () => { @@ -100,7 +670,7 @@ describe('Versions API', () => { before(async () => { const dateInTheFuture = new Date(Date.now() + 60000); // 1 minute in the future - response = await request.get(`${basePath}/v1/version/service·A/Terms%20of%20Service/${encodeURIComponent(toISODateWithoutMilliseconds(dateInTheFuture))}`); + response = await request.get(`${basePath}/v1/version/service-1/Terms%20of%20Service/${encodeURIComponent(toISODateWithoutMilliseconds(dateInTheFuture))}`); }); it('responds with 416 status code', () => { From 7773844da96bbcf936fbf0c144ad697598fed3a2 Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Wed, 1 Jul 2026 19:59:46 +0200 Subject: [PATCH 07/17] Return JSON errors from the services endpoints --- src/collection-api/routes/services.js | 55 +++++++++++++++++---------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/src/collection-api/routes/services.js b/src/collection-api/routes/services.js index 9906b7152..3bbd79f27 100644 --- a/src/collection-api/routes/services.js +++ b/src/collection-api/routes/services.js @@ -9,6 +9,24 @@ import express from 'express'; * description: Services API * components: * schemas: + * ServiceListItem: + * type: object + * properties: + * id: + * type: string + * description: The ID of the service. + * name: + * type: string + * description: The name of the service. + * terms: + * type: array + * description: The declared terms types for this service. + * items: + * type: object + * properties: + * type: + * type: string + * description: The type of terms. * Service: * type: object * description: Definition of a service and the agreements its provider sets forth. While the information is the same, the format differs from the JSON declaration files that are designed for readability by contributors. @@ -51,6 +69,19 @@ import express from 'express'; * description: The names of filters to apply to the content. * items: * type: string + * ErrorResponse: + * type: object + * properties: + * error: + * type: string + * description: Error message. + * responses: + * NotFoundError: + * description: Resource not found. + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/ErrorResponse' */ export default function servicesRouter(services) { const router = express.Router(); @@ -71,23 +102,7 @@ export default function servicesRouter(services) { * schema: * type: array * items: - * type: object - * properties: - * id: - * type: string - * description: The ID of the service. - * name: - * type: string - * description: The name of the service. - * terms: - * type: array - * description: The declared terms types for this service. - * items: - * type: object - * properties: - * type: - * type: string - * description: The type of terms. + * $ref: '#/components/schemas/ServiceListItem' */ router.get('/services', (req, res) => { res.status(200).json(Object.values(services).map(service => ({ @@ -127,15 +142,13 @@ export default function servicesRouter(services) { * schema: * $ref: '#/components/schemas/Service' * 404: - * description: No service matching the provided ID is found. + * $ref: '#/components/responses/NotFoundError' */ router.get('/service/:serviceId', (req, res) => { const service = Object.hasOwn(services, req.params.serviceId) ? services[req.params.serviceId] : null; if (!service) { - res.status(404).send('Service not found'); - - return; + return res.status(404).json({ error: 'Service not found' }); } res.status(200).json({ From ea29a0e80a452ddeda80e6810915b3b9f9baf589 Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Wed, 1 Jul 2026 20:42:24 +0200 Subject: [PATCH 08/17] Add changelog entry --- CHANGELOG.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ab80cae2..7e6a996ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ All changes that impact users of this module are documented in this file, in the [Common Changelog](https://common-changelog.org) format with some additional specifications defined in the CONTRIBUTING file. This codebase adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased [major] + +> Development of this release was supported by the [NGI0 Commons Fund](https://nlnet.nl/project/Modular-OTA/), a fund established by [NLnet](https://nlnet.nl/) with financial support from the European Commission's [Next Generation Internet](https://www.ngi.eu) programme, under the aegis of DG CNECT under grant agreement N°101069594. + +### Added + +- Add paginated `GET /versions`, `GET /versions/{serviceId}` and `GET /versions/{serviceId}/{termsType}` endpoints to the Collection API to list versions +- Add `GET /version/{versionId}` and `GET /version/{serviceId}/{termsType}/latest` endpoints to the Collection API to retrieve a single version +- Expose `serviceId`, `termsType`, `isFirstRecord`, `isTechnicalUpgrade`, source `fetchUrls`, navigation `links` and diff statistics (`additions` and `deletions`) in Collection API version responses + +### Changed + +- **Breaking:** Return the `GET /service/{serviceId}` not-found response as JSON instead of plain text + ## 14.1.0 - 2026-06-29 > Development of this release was supported by [the Research Chair in Content Moderation](https://regulation-tech.cnam.fr/) at the Conservatoire National des Arts et Métiers. From d0c79b1cb409f2863ee07f404d9b1a177d435a01 Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Mon, 6 Jul 2026 11:14:27 +0200 Subject: [PATCH 09/17] Prevent Git argument injection via record IDs The single-version endpoints pass the URL `versionId` straight to `findById`, which placed it in a `git log` revision position with no separator. A value such as `--output=/path` was therefore parsed by git as an option, letting an unauthenticated caller create or overwrite an arbitrary file with `git log` output. Reject any record ID that is not a hexadecimal commit SHA before it reaches git, mirroring the `ObjectId.isValid` guard already used by the MongoDB backend, and pass validated IDs after `--end-of-options` as a second line of defence. --- .../recorder/repositories/git/index.js | 14 +++++- .../recorder/repositories/git/index.test.js | 47 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/archivist/recorder/repositories/git/index.js b/src/archivist/recorder/repositories/git/index.js index 8e89a1e56..e38aad9ad 100644 --- a/src/archivist/recorder/repositories/git/index.js +++ b/src/archivist/recorder/repositories/git/index.js @@ -15,6 +15,8 @@ import Git from './git.js'; const fs = fsApi.promises; +const RECORD_ID_REGEXP = /^[0-9a-f]{7,40}$/i; // A record ID is a Git commit SHA: 7 (abbreviated) to 40 (full) hexadecimal characters. Anything else cannot be a record and is rejected before reaching git, so a value such as `--output=…` can never be parsed as a command-line option + export default class GitRepository extends RepositoryInterface { constructor({ path, author, publish, snapshotIdentiferTemplate }) { super(); @@ -83,13 +85,21 @@ export default class GitRepository extends RepositoryInterface { } async findById(recordId) { - const commit = await this.git.getCommit([recordId]); + if (!RECORD_ID_REGEXP.test(recordId)) { + return null; + } + + const commit = await this.git.getCommit([ '--end-of-options', recordId ]); // `--end-of-options` forces git to treat `recordId` as a revision, never as an option: a second line of defence that keeps the lookup safe from argument injection even if the format guard above is ever relaxed return this.#toDomain(commit); } async findMetadataById(recordId) { - const commit = await this.git.getCommit([recordId]); + if (!RECORD_ID_REGEXP.test(recordId)) { + return null; + } + + const commit = await this.git.getCommit([ '--end-of-options', recordId ]); // `--end-of-options` forces git to treat `recordId` as a revision, never as an option: a second line of defence that keeps the lookup safe from argument injection even if the format guard above is ever relaxed return this.#toDomain(commit, { deferContentLoading: true }); } diff --git a/src/archivist/recorder/repositories/git/index.test.js b/src/archivist/recorder/repositories/git/index.test.js index 079a17d0c..6ace179ca 100644 --- a/src/archivist/recorder/repositories/git/index.test.js +++ b/src/archivist/recorder/repositories/git/index.test.js @@ -406,6 +406,53 @@ describe('GitRepository', () => { expect(await subject.findById('ecd9407eb26b1bf0613186175ee80edbdeedd47f')).to.equal(null); }); }); + + context('when the requested ID could be interpreted as a git option', () => { + const INJECTION_PROOF_FILE_PATH = path.resolve(__dirname, 'findById-argument-injection-proof.txt'); + + after(() => fs.rmSync(INJECTION_PROOF_FILE_PATH, { force: true })); + + it('returns null without letting the ID reach git as an argument', async () => { + expect(await subject.findById(`--output=${INJECTION_PROOF_FILE_PATH}`)).to.equal(null); + expect(fs.existsSync(INJECTION_PROOF_FILE_PATH), 'a version ID must never be interpreted as a git option').to.be.false; + }); + }); + }); + + describe('#findMetadataById', () => { + let id; + + before(async () => { + ({ id } = await subject.save(new Version({ + serviceId: SERVICE_PROVIDER_ID, + termsType: TERMS_TYPE, + content: CONTENT, + fetchDate: FETCH_DATE, + snapshotIds: [SNAPSHOT_ID], + mimeType: HTML_MIME_TYPE, + metadata: METADATA, + }))); + }); + + after(() => subject.removeAll()); + + it('returns the record', async () => { + const record = await subject.findMetadataById(id); + + expect(record).to.be.an.instanceof(Version); + expect(record.id).to.include(id); + }); + + context('when the requested ID could be interpreted as a git option', () => { + const INJECTION_PROOF_FILE_PATH = path.resolve(__dirname, 'findMetadataById-argument-injection-proof.txt'); + + after(() => fs.rmSync(INJECTION_PROOF_FILE_PATH, { force: true })); + + it('returns null without letting the ID reach git as an argument', async () => { + expect(await subject.findMetadataById(`--output=${INJECTION_PROOF_FILE_PATH}`)).to.equal(null); + expect(fs.existsSync(INJECTION_PROOF_FILE_PATH), 'a version ID must never be interpreted as a git option').to.be.false; + }); + }); }); describe('#findByDate', () => { From 1c0b885c36cd1fb5a36a58394cd6232cad025453 Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Mon, 6 Jul 2026 11:17:35 +0200 Subject: [PATCH 10/17] Separate paths from options in Git lookups --- .../recorder/repositories/git/git.js | 2 +- .../recorder/repositories/git/index.js | 2 +- .../recorder/repositories/git/index.test.js | 43 +++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/archivist/recorder/repositories/git/git.js b/src/archivist/recorder/repositories/git/git.js index 4902f2958..f479f4c6a 100644 --- a/src/archivist/recorder/repositories/git/git.js +++ b/src/archivist/recorder/repositories/git/git.js @@ -155,7 +155,7 @@ export default class Git { } async listFiles(path) { - return (await this.git.raw([ 'ls-files', path ])).split('\n'); + return (await this.git.raw([ 'ls-files', '--', path ])).split('\n'); } async writeCommitGraph() { diff --git a/src/archivist/recorder/repositories/git/index.js b/src/archivist/recorder/repositories/git/index.js index e38aad9ad..73ffa17bd 100644 --- a/src/archivist/recorder/repositories/git/index.js +++ b/src/archivist/recorder/repositories/git/index.js @@ -79,7 +79,7 @@ export default class GitRepository extends RepositoryInterface { async findByDate(serviceId, termsType, date, documentId) { const filePath = DataMapper.generateFilePath(serviceId, termsType, documentId); - const commit = await this.git.getCommit([ `--until=${date?.toISOString()}`, filePath ]); + const commit = await this.git.getCommit([ `--until=${date?.toISOString()}`, '--', filePath ]); return this.#toDomain(commit); } diff --git a/src/archivist/recorder/repositories/git/index.test.js b/src/archivist/recorder/repositories/git/index.test.js index 6ace179ca..bf4c199b8 100644 --- a/src/archivist/recorder/repositories/git/index.test.js +++ b/src/archivist/recorder/repositories/git/index.test.js @@ -538,6 +538,31 @@ describe('GitRepository', () => { expect(record.metadata).to.deep.equal(METADATA); }); }); + + context('when the service ID is a git argument injection attempt', () => { + const INJECTION_PROOF_FILE_PATH = path.resolve(__dirname, 'findByDate-argument-injection-proof.*'); + + before(async () => { + await subject.save(new Version({ + serviceId: SERVICE_PROVIDER_ID, + termsType: TERMS_TYPE, + content: CONTENT, + fetchDate: FETCH_DATE, + snapshotIds: [SNAPSHOT_ID], + })); + }); + + after(async () => { + fs.rmSync(INJECTION_PROOF_FILE_PATH, { force: true }); + await subject.removeAll(); + }); + + it('treats the service ID as a path so it cannot reach git as an option', async () => { + await subject.findByDate(`--output=${__dirname}`, 'findByDate-argument-injection-proof', FETCH_DATE_LATER); + + expect(fs.existsSync(INJECTION_PROOF_FILE_PATH), 'a service ID must never be interpreted as a git option').to.be.false; + }); + }); }); describe('#findAll', () => { @@ -1128,6 +1153,24 @@ describe('GitRepository', () => { expect(latestRecord).to.equal(null); }); }); + + context('when the service ID could be interpreted as a git option', () => { + before(async () => { + await subject.save(new Version({ + serviceId: SERVICE_PROVIDER_ID, + termsType: TERMS_TYPE, + content: CONTENT, + fetchDate: FETCH_DATE, + snapshotIds: [SNAPSHOT_ID], + })); + }); + + after(() => subject.removeAll()); + + it('treats the service ID as a path and returns null instead of erroring', async () => { + expect(await subject.findLatest('--not-a-git-option', TERMS_TYPE)).to.equal(null); + }); + }); }); describe('#iterate', () => { From 4894aeb255a96ead984fa3638ed4de98c2a88444 Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Mon, 6 Jul 2026 15:40:34 +0200 Subject: [PATCH 11/17] Reject version lookups that cannot match a path The version lookups compose a service ID, terms type or document ID into a Git pathspec. A value with a path separator or a `..` segment resolved outside the repository, and the resulting fatal git error exposed the repository filesystem location. Reject such values before they reach git, where they cannot match a record anyway, so the lookups return empty and the endpoints answer 404 instead of 500. --- CHANGELOG.md | 4 ++ .../recorder/repositories/git/index.js | 42 +++++++++++++ .../recorder/repositories/git/index.test.js | 37 ++++++++++++ src/collection-api/routes/versions.test.js | 60 +++++++++++++++++++ 4 files changed, 143 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e6a996ea..7036d49a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ All changes that impact users of this module are documented in this file, in the - **Breaking:** Return the `GET /service/{serviceId}` not-found response as JSON instead of plain text +### Fixed + +- Return `404` instead of a server error when a version lookup contains path separators or relative path segments; the error previously exposed the repository filesystem location + ## 14.1.0 - 2026-06-29 > Development of this release was supported by [the Research Chair in Content Moderation](https://regulation-tech.cnam.fr/) at the Conservatoire National des Arts et Métiers. diff --git a/src/archivist/recorder/repositories/git/index.js b/src/archivist/recorder/repositories/git/index.js index 73ffa17bd..166c530c0 100644 --- a/src/archivist/recorder/repositories/git/index.js +++ b/src/archivist/recorder/repositories/git/index.js @@ -17,6 +17,24 @@ const fs = fsApi.promises; const RECORD_ID_REGEXP = /^[0-9a-f]{7,40}$/i; // A record ID is a Git commit SHA: 7 (abbreviated) to 40 (full) hexadecimal characters. Anything else cannot be a record and is rejected before reaching git, so a value such as `--output=…` can never be parsed as a command-line option +const CONTROL_CHARACTERS_REGEXP = /\p{Cc}/u; // Matches any Unicode "control" character (general category Cc): the C0 range (U+0000 to U+001F), DEL (U+007F) and the C1 range (U+0080 to U+009F), i.e. 65 non-printable characters including NUL. The `u` flag is required for the `\p{...}` property escape to be recognised, otherwise the pattern would match the literal text `p{Cc}`. Legitimate service IDs, terms types and document IDs never contain these, and NUL in particular can truncate a value once it reaches git or the filesystem, so any segment holding one is rejected. + +// A service ID, terms type or document ID forms a single segment of a record file path (see DataMapper.generateFilePath): it may not be empty, be a relative segment (`.` or `..`), or contain a path separator or a control character. +// Rejecting anything else keeps hostile values from reaching git, where a pathspec that resolves outside the repository (such as `../foo/*`) aborts with an error that exposes the repository location. +function isPlainPathSegment(segment) { + return segment.length > 0 + && segment !== '.' + && segment !== '..' + && !segment.includes('/') + && !segment.includes('\\') + && !CONTROL_CHARACTERS_REGEXP.test(segment); +} + +function canMatchRecordFilePath(...pathSegments) { + // A non-string segment means "not provided" (`undefined`, or `false` for an absent document ID) and constrains nothing + return pathSegments.every(segment => typeof segment !== 'string' || isPlainPathSegment(segment)); +} + export default class GitRepository extends RepositoryInterface { constructor({ path, author, publish, snapshotIdentiferTemplate }) { super(); @@ -66,6 +84,10 @@ export default class GitRepository extends RepositoryInterface { } async findLatest(serviceId, termsType, documentId) { + if (!canMatchRecordFilePath(serviceId, termsType, documentId)) { + return null; + } + const matchingFilesPaths = await this.git.listFiles(DataMapper.generateFilePath(serviceId, termsType, documentId)); if (!matchingFilesPaths.length) { @@ -78,6 +100,10 @@ export default class GitRepository extends RepositoryInterface { } async findByDate(serviceId, termsType, date, documentId) { + if (!canMatchRecordFilePath(serviceId, termsType, documentId)) { + return null; + } + const filePath = DataMapper.generateFilePath(serviceId, termsType, documentId); const commit = await this.git.getCommit([ `--until=${date?.toISOString()}`, '--', filePath ]); @@ -109,18 +135,30 @@ export default class GitRepository extends RepositoryInterface { } async findByServiceAndTermsType(serviceId, termsType, { limit, offset, includeTechnicalUpgrades = true } = {}) { + if (!canMatchRecordFilePath(serviceId, termsType)) { + return []; + } + const pathPattern = DataMapper.generateFilePath(serviceId, termsType); return Promise.all((await this.#getCommits({ pathFilter: pathPattern, limit, offset, includeTechnicalUpgrades })).map(commit => this.#toDomain(commit, { deferContentLoading: true }))); } async findByService(serviceId, { limit, offset, includeTechnicalUpgrades = true } = {}) { + if (!canMatchRecordFilePath(serviceId)) { + return []; + } + const pathPattern = DataMapper.generateFilePath(serviceId); return Promise.all((await this.#getCommits({ pathFilter: pathPattern, limit, offset, includeTechnicalUpgrades })).map(commit => this.#toDomain(commit, { deferContentLoading: true }))); } async getNavigationIds(serviceId, termsType, versionId, { includeTechnicalUpgrades = true } = {}) { + if (!canMatchRecordFilePath(serviceId, termsType)) { + return { first: null, prev: null, next: null, last: null }; + } + const pathPattern = DataMapper.generateFilePath(serviceId, termsType); let revisions = await this.git.listPathRevisions(pathPattern); // single lean walk of the terms history @@ -147,6 +185,10 @@ export default class GitRepository extends RepositoryInterface { } async count(serviceId, termsType) { + if (!canMatchRecordFilePath(serviceId, termsType)) { + return 0; + } + const grepOptions = Object.values(DataMapper.COMMIT_MESSAGE_PREFIXES).map(prefix => `--grep=${prefix}`); const pathOptions = []; diff --git a/src/archivist/recorder/repositories/git/index.test.js b/src/archivist/recorder/repositories/git/index.test.js index bf4c199b8..2ca102c8e 100644 --- a/src/archivist/recorder/repositories/git/index.test.js +++ b/src/archivist/recorder/repositories/git/index.test.js @@ -563,6 +563,12 @@ describe('GitRepository', () => { expect(fs.existsSync(INJECTION_PROOF_FILE_PATH), 'a service ID must never be interpreted as a git option').to.be.false; }); }); + + context('when the service ID is a path traversal attempt', () => { + it('returns null instead of erroring', async () => { + expect(await subject.findByDate('../../outside', TERMS_TYPE, FETCH_DATE)).to.equal(null); + }); + }); }); describe('#findAll', () => { @@ -723,6 +729,13 @@ describe('GitRepository', () => { }); }); + context('when the service ID or terms type is a path traversal attempt', () => { + it('returns an empty array instead of erroring', async () => { + expect(await subject.findByServiceAndTermsType('../../outside', TERMS_TYPE)).to.be.an('array').that.is.empty; + expect(await subject.findByServiceAndTermsType(SERVICE_PROVIDER_ID, '../../outside')).to.be.an('array').that.is.empty; + }); + }); + context('with includeTechnicalUpgrades: false', () => { let filteredRecords; let technicalUpgradeId; @@ -839,6 +852,12 @@ describe('GitRepository', () => { }); }); + context('when the service ID is a path traversal attempt', () => { + it('returns an empty array instead of erroring', async () => { + expect(await subject.findByService('../../outside')).to.be.an('array').that.is.empty; + }); + }); + context('with includeTechnicalUpgrades: false', () => { let filteredRecords; let technicalUpgradeId; @@ -920,6 +939,12 @@ describe('GitRepository', () => { }); }); + context('when the service ID is a path traversal attempt', () => { + it('returns zero instead of erroring', async () => { + expect(await subject.count('../../outside', TERMS_TYPE)).to.equal(0); + }); + }); + context('with only serviceId filter', () => { it('returns count for all terms types of a service', async () => { // Add a version with different terms type @@ -1011,6 +1036,12 @@ describe('GitRepository', () => { }); }); + context('when the service ID is a path traversal attempt', () => { + it('returns only null IDs instead of erroring', async () => { + expect(await subject.getNavigationIds('../../outside', TERMS_TYPE, firstVersion.id)).to.deep.equal({ first: null, prev: null, next: null, last: null }); + }); + }); + context('when a technical upgrade is recorded out of chronological order', () => { // A technical upgrade re-renders an old snapshot: it carries an OLD fetch date but is committed last (topologically recent). // The previous implementation mixed chronological (findPrevious) and topological (findNext) order, so prev/next disagreed here. @@ -1171,6 +1202,12 @@ describe('GitRepository', () => { expect(await subject.findLatest('--not-a-git-option', TERMS_TYPE)).to.equal(null); }); }); + + context('when the service ID is a path traversal attempt', () => { + it('returns null instead of erroring', async () => { + expect(await subject.findLatest('../../outside', TERMS_TYPE)).to.equal(null); + }); + }); }); describe('#iterate', () => { diff --git a/src/collection-api/routes/versions.test.js b/src/collection-api/routes/versions.test.js index 432a71eb3..d79717a08 100644 --- a/src/collection-api/routes/versions.test.js +++ b/src/collection-api/routes/versions.test.js @@ -285,6 +285,38 @@ describe('Versions API', () => { expect(response.body.error).to.contain('No versions found').and.to.contain('non-existent-service').and.to.contain('Terms of Service'); }); }); + + context('when the service ID is a path traversal attempt', () => { + before(async () => { + response = await request.get(`${basePath}/v1/versions/..%2F..%2Foutside/Terms%20of%20Service`); + }); + + it('responds with 404 status code', () => { + expect(response.status).to.equal(404); + }); + + it('does not expose the repository location', () => { + expect(response.body.error).to.contain('No versions found').and.to.not.contain('outside repository'); + }); + }); + }); + + describe('GET /versions/:serviceId', () => { + let response; + + context('when the service ID is a path traversal attempt', () => { + before(async () => { + response = await request.get(`${basePath}/v1/versions/..%2F..%2Foutside`); + }); + + it('responds with 404 status code', () => { + expect(response.status).to.equal(404); + }); + + it('does not expose the repository location', () => { + expect(response.body.error).to.contain('No versions found').and.to.not.contain('outside repository'); + }); + }); }); describe('GET /version/:versionId', () => { @@ -574,6 +606,20 @@ describe('Versions API', () => { expect(response.body.error).to.contain('No version found').and.to.contain('non-existent-service'); }); }); + + context('when the service ID is a path traversal attempt', () => { + before(async () => { + response = await request.get(`${basePath}/v1/version/..%2F..%2Foutside/Terms%20of%20Service/latest`); + }); + + it('responds with 404 status code', () => { + expect(response.status).to.equal(404); + }); + + it('does not expose the repository location', () => { + expect(response.body.error).to.contain('No version found').and.to.not.contain('outside repository'); + }); + }); }); describe('GET /version/:serviceId/:termsType/:date', () => { @@ -685,5 +731,19 @@ describe('Versions API', () => { expect(response.body.error).to.equal('Requested version is in the future'); }); }); + + context('when the service ID is a path traversal attempt', () => { + before(async () => { + response = await request.get(`${basePath}/v1/version/..%2F..%2Foutside/Terms%20of%20Service/2023-01-01T12:00:00Z`); + }); + + it('responds with 404 status code', () => { + expect(response.status).to.equal(404); + }); + + it('does not expose the repository location', () => { + expect(response.body.error).to.contain('No version found').and.to.not.contain('outside repository'); + }); + }); }); }); From a06c7b3f30a44f780181dd28fc0d651b1a3ea48f Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Mon, 6 Jul 2026 15:40:45 +0200 Subject: [PATCH 12/17] Return generic Collection API error responses --- CHANGELOG.md | 1 + src/collection-api/middlewares/errors.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7036d49a0..cad672dcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ All changes that impact users of this module are documented in this file, in the ### Fixed - Return `404` instead of a server error when a version lookup contains path separators or relative path segments; the error previously exposed the repository filesystem location +- Return a generic message instead of internal error details in Collection API `500` responses ## 14.1.0 - 2026-06-29 diff --git a/src/collection-api/middlewares/errors.js b/src/collection-api/middlewares/errors.js index 7a98e51e7..7aca0f0a4 100644 --- a/src/collection-api/middlewares/errors.js +++ b/src/collection-api/middlewares/errors.js @@ -2,5 +2,5 @@ import logger from '../logger.js'; export default function errorsMiddleware(err, req, res, next) { logger.error(err.stack); - res.status(500).json({ error: err.message }); + res.status(500).json({ error: 'Internal Server Error' }); // Never echo internal error details: they can expose server internals such as filesystem paths } From 0680139ffd287afc6dbf72f2edbcbe52635c4880 Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Mon, 6 Jul 2026 15:41:02 +0200 Subject: [PATCH 13/17] Make diff statistics an optional capability Diff statistics are only meaningful for the Git backend, which retains diffs; the MongoDB backend stores full snapshots and cannot provide them. The interface declared the method as returning numbers while MongoDB stubbed it with nulls. Move the null default into the repository interface so snapshot-based backends inherit it, drop the MongoDB stub, and document the statistics as an optional, nullable capability. --- .../recorder/repositories/interface.js | 14 +++++++------ .../recorder/repositories/mongo/index.js | 5 ----- .../recorder/repositories/mongo/index.test.js | 21 +++++++++++++++++++ 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/archivist/recorder/repositories/interface.js b/src/archivist/recorder/repositories/interface.js index 834b12962..d234eaad0 100644 --- a/src/archivist/recorder/repositories/interface.js +++ b/src/archivist/recorder/repositories/interface.js @@ -177,12 +177,14 @@ class RepositoryInterface { } /** - * Get diff statistics for a specific record - * @param {string} recordId - Record ID to get diff stats for - * @returns {Promise<{additions: number, deletions: number}>} Promise that will be resolved with the diff statistics - */ - async getDiffStats(recordId) { - throw new Error(`#getDiffStats method is not implemented in ${this.constructor.name}`); + * Get the number of lines added and deleted by a record, relative to the previous state of the same terms + * This is an optional capability: backends that retain diffs (Git) override it, while backends that store full snapshots (MongoDB) inherit this default and report the statistics as unavailable + * @param {string} recordId - Record ID to get diff stats for + * @returns {Promise<{additions: ?number, deletions: ?number}>} Promise resolved with the number of added and deleted lines, each null when the backend cannot provide them + */ + // eslint-disable-next-line class-methods-use-this, no-unused-vars + getDiffStats(recordId) { + return { additions: null, deletions: null }; } } diff --git a/src/archivist/recorder/repositories/mongo/index.js b/src/archivist/recorder/repositories/mongo/index.js index b515e5709..5e6758c6d 100644 --- a/src/archivist/recorder/repositories/mongo/index.js +++ b/src/archivist/recorder/repositories/mongo/index.js @@ -233,11 +233,6 @@ export default class MongoRepository extends RepositoryInterface { record.content = content instanceof Binary ? content.buffer : content; } - // eslint-disable-next-line class-methods-use-this, no-unused-vars - getDiffStats(recordId) { - return { additions: null, deletions: null }; // Diff stats are not available for MongoDB storage - } - async #toDomain(mongoDocument, { deferContentLoading } = {}) { if (!mongoDocument) { return null; diff --git a/src/archivist/recorder/repositories/mongo/index.test.js b/src/archivist/recorder/repositories/mongo/index.test.js index 1e8d420b5..b4d3a008f 100644 --- a/src/archivist/recorder/repositories/mongo/index.test.js +++ b/src/archivist/recorder/repositories/mongo/index.test.js @@ -1138,6 +1138,27 @@ describe('MongoRepository', () => { }); }); + describe('#getDiffStats', () => { + // Diff statistics are an optional repository capability; MongoDB stores full snapshots rather than diffs, so it inherits the interface default that reports them as unavailable. + let versionId; + + before(async () => { + ({ id: versionId } = await subject.save(new Version({ + serviceId: SERVICE_PROVIDER_ID, + termsType: TERMS_TYPE, + content: CONTENT, + fetchDate: FETCH_DATE, + snapshotIds: [SNAPSHOT_ID], + }))); + }); + + after(() => subject.removeAll()); + + it('reports additions and deletions as unavailable', async () => { + expect(await subject.getDiffStats(versionId)).to.deep.equal({ additions: null, deletions: null }); + }); + }); + describe('#findLatest', () => { context('when there are records for the given service', () => { let lastSnapshotId; From ee100dcc77a7860ead51a82ef9e76a39b8e821b5 Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Mon, 6 Jul 2026 15:41:02 +0200 Subject: [PATCH 14/17] Document the options of Git commands --- .../recorder/repositories/git/git.js | 54 ++++++++++++++----- 1 file changed, 42 insertions(+), 12 deletions(-) diff --git a/src/archivist/recorder/repositories/git/git.js b/src/archivist/recorder/repositories/git/git.js index f479f4c6a..884ae8011 100644 --- a/src/archivist/recorder/repositories/git/git.js +++ b/src/archivist/recorder/repositories/git/git.js @@ -22,7 +22,7 @@ export default class Git { this.git = simpleGit(this.path, { trimmed: true, - maxConcurrentProcesses: 1, + maxConcurrentProcesses: 1, // Serialise git processes: concurrent runs on the same repository race the index and the commit-graph and can corrupt them }); await this.git.init(); @@ -131,17 +131,26 @@ export default class Git { async cleanUp() { await fs.rm(path.join(this.path, '.git', 'objects', 'info', 'commit-graph.lock'), { force: true }); // Remove a leftover commit-graph lock from a previous `commit-graph write` that was killed mid-write (e.g. the process was terminated during a deploy or restart). The commit-graph is a disposable cache rebuilt by `writeCommitGraph`, so clearing a stale lock is safe and prevents every subsequent run from failing. - await this.git.reset('hard'); + await this.git.reset('hard'); // Discard staged and unstaged changes to tracked files - return this.git.clean('f', '-d'); + return this.git.clean('f', '-d'); // Force-remove untracked files (`f`) and untracked directories (`-d`) } getFullHash(shortHash) { - return this.git.show([ shortHash, '--pretty=%H', '-s' ]); + return this.git.show([ + shortHash, + '--pretty=%H', // Print the full 40-character commit hash + '-s', // Suppress the diff output, only the formatted hash is wanted + ]); } restore(path, commit) { - return this.git.raw([ 'restore', '-s', commit, '--', path ]); + return this.git.raw([ + 'restore', + '-s', commit, // Take the file contents from this specific commit rather than from the index + '--', // Everything after is a pathspec, never a revision or an option + path, + ]); } async destroyHistory() { @@ -155,24 +164,40 @@ export default class Git { } async listFiles(path) { - return (await this.git.raw([ 'ls-files', '--', path ])).split('\n'); + return (await this.git.raw([ 'ls-files', '--', path ])).split('\n'); // "--" tells Git that everything following is a file path, not a revision or option. } async writeCommitGraph() { - await this.git.raw([ 'commit-graph', 'write', '--reachable', '--changed-paths' ]); + await this.git.raw([ + 'commit-graph', + 'write', + '--reachable', // Cover every commit reachable from the refs, so the whole history is indexed + '--changed-paths', // Also store changed-path Bloom filters, which speed up the path-limited log/diff behind version lookups + ]); } async updateCommitGraph() { - await this.git.raw([ 'commit-graph', 'write', '--reachable', '--changed-paths', '--append' ]); + await this.git.raw([ + 'commit-graph', + 'write', + '--reachable', // Cover every commit reachable from the refs + '--changed-paths', // Also store the changed-path Bloom filters that speed up path-limited log/diff + '--append', // Extend the existing commit-graph instead of rewriting it in full + ]); } async listPathRevisions(pathFilter) { let output; try { - // Lean log: only the data needed to order versions (hash, author timestamp, subject), no diff or message body. - // Ordering and technical-upgrade filtering are done by the caller in memory, so `--author-date-order`/`--grep`/`--name-only` are deliberately omitted. - output = await this.git.raw([ 'log', '--no-merges', '--format=%H%x09%at%x09%s', '--', pathFilter ]); + // Ordering and technical-upgrade filtering are done by the caller in memory, so `--author-date-order`/`--grep`/`--name-only` are deliberately omitted to keep this walk lean. + output = await this.git.raw([ + 'log', + '--no-merges', // Records are stored as regular commits, never as merges + '--format=%H%x09%at%x09%s', // Tab-separated hash, author date (epoch seconds) and subject: the minimum needed to order versions and detect technical upgrades, with no diff or message body loaded + '--', // Everything after is a pathspec, never a revision or an option + pathFilter, + ]); } catch (error) { if (/unknown revision or path not in the working tree|does not have any commits yet/.test(error.message)) { return []; @@ -193,7 +218,12 @@ export default class Git { } async getDiffStats(commitHash) { - const output = await this.git.raw([ 'show', '--numstat', '--format=', commitHash ]); + const output = await this.git.raw([ + 'show', + '--numstat', // Report added/deleted line counts per file as tab-separated numbers, instead of a textual diff + '--format=', // Drop the commit header so the output holds only the numstat lines + commitHash, + ]); let additions = 0; let deletions = 0; From f76e3c2dd412e3c37011fcd127589ee2519c67b4 Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Mon, 6 Jul 2026 17:34:53 +0200 Subject: [PATCH 15/17] Update changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cad672dcc..6fdd1891d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ All changes that impact users of this module are documented in this file, in the ### Fixed +- Reject version lookups whose service ID, terms type or record ID would be parsed as a Git option, which previously let an unauthenticated caller overwrite arbitrary files through the Collection API - Return `404` instead of a server error when a version lookup contains path separators or relative path segments; the error previously exposed the repository filesystem location - Return a generic message instead of internal error details in Collection API `500` responses From 9b5a6bc712a666e0b3e1f4bb7b690cb289985e1a Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Tue, 7 Jul 2026 10:43:32 +0200 Subject: [PATCH 16/17] Simplify comments --- .../recorder/repositories/git/git.js | 30 +++++++++++-------- .../recorder/repositories/git/index.js | 15 +++++----- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/archivist/recorder/repositories/git/git.js b/src/archivist/recorder/repositories/git/git.js index 884ae8011..7cba202fc 100644 --- a/src/archivist/recorder/repositories/git/git.js +++ b/src/archivist/recorder/repositories/git/git.js @@ -22,7 +22,7 @@ export default class Git { this.git = simpleGit(this.path, { trimmed: true, - maxConcurrentProcesses: 1, // Serialise git processes: concurrent runs on the same repository race the index and the commit-graph and can corrupt them + maxConcurrentProcesses: 1, // Concurrent runs on the same repository race the index and the commit-graph and can corrupt them }); await this.git.init(); @@ -106,7 +106,7 @@ export default class Git { return commits; } catch (error) { - // `bad object` is raised for a well-formed but absent object ID (e.g. a snapshot referenced by a version but missing from this repository); like an unknown revision, it means "no match" rather than a hard failure + // `bad object` is raised for a well-formed but absent object ID; like an unknown revision, it means "no match" rather than a hard failure if (/unknown revision or path not in the working tree|does not have any commits yet|bad object/.test(error.message)) { return []; } @@ -131,7 +131,7 @@ export default class Git { async cleanUp() { await fs.rm(path.join(this.path, '.git', 'objects', 'info', 'commit-graph.lock'), { force: true }); // Remove a leftover commit-graph lock from a previous `commit-graph write` that was killed mid-write (e.g. the process was terminated during a deploy or restart). The commit-graph is a disposable cache rebuilt by `writeCommitGraph`, so clearing a stale lock is safe and prevents every subsequent run from failing. - await this.git.reset('hard'); // Discard staged and unstaged changes to tracked files + await this.git.reset('hard'); return this.git.clean('f', '-d'); // Force-remove untracked files (`f`) and untracked directories (`-d`) } @@ -148,7 +148,7 @@ export default class Git { return this.git.raw([ 'restore', '-s', commit, // Take the file contents from this specific commit rather than from the index - '--', // Everything after is a pathspec, never a revision or an option + '--', // Everything after is a pathspec, not a revision or option path, ]); } @@ -164,7 +164,7 @@ export default class Git { } async listFiles(path) { - return (await this.git.raw([ 'ls-files', '--', path ])).split('\n'); // "--" tells Git that everything following is a file path, not a revision or option. + return (await this.git.raw([ 'ls-files', '--', path ])).split('\n'); // Everything after "--" is a pathspec, not a revision or option } async writeCommitGraph() { @@ -172,7 +172,7 @@ export default class Git { 'commit-graph', 'write', '--reachable', // Cover every commit reachable from the refs, so the whole history is indexed - '--changed-paths', // Also store changed-path Bloom filters, which speed up the path-limited log/diff behind version lookups + '--changed-paths', // Also store the changed-path Bloom filters that speed up path-limited log/diff ]); } @@ -180,8 +180,8 @@ export default class Git { await this.git.raw([ 'commit-graph', 'write', - '--reachable', // Cover every commit reachable from the refs - '--changed-paths', // Also store the changed-path Bloom filters that speed up path-limited log/diff + '--reachable', + '--changed-paths', '--append', // Extend the existing commit-graph instead of rewriting it in full ]); } @@ -190,11 +190,11 @@ export default class Git { let output; try { - // Ordering and technical-upgrade filtering are done by the caller in memory, so `--author-date-order`/`--grep`/`--name-only` are deliberately omitted to keep this walk lean. + // Ordering and technical-upgrade filtering are done by the caller in memory, so `--author-date-order`/`--grep`/`--name-only` are deliberately omitted to keep this walk lean output = await this.git.raw([ 'log', - '--no-merges', // Records are stored as regular commits, never as merges - '--format=%H%x09%at%x09%s', // Tab-separated hash, author date (epoch seconds) and subject: the minimum needed to order versions and detect technical upgrades, with no diff or message body loaded + '--no-merges', + '--format=%H%x09%at%x09%s', // Tab-separated hash, author date and subject: the minimum needed to order versions and detect technical upgrades, with no diff or message body loaded '--', // Everything after is a pathspec, never a revision or an option pathFilter, ]); @@ -236,8 +236,12 @@ export default class Git { const [ added, deleted ] = line.split('\t'); // Binary files show '-' for additions/deletions - if (added !== '-') { additions += parseInt(added, 10); } - if (deleted !== '-') { deletions += parseInt(deleted, 10); } + if (added !== '-') { + additions += parseInt(added, 10); + } + if (deleted !== '-') { + deletions += parseInt(deleted, 10); + } } return { additions, deletions }; diff --git a/src/archivist/recorder/repositories/git/index.js b/src/archivist/recorder/repositories/git/index.js index 166c530c0..a50a7f2ae 100644 --- a/src/archivist/recorder/repositories/git/index.js +++ b/src/archivist/recorder/repositories/git/index.js @@ -15,12 +15,11 @@ import Git from './git.js'; const fs = fsApi.promises; -const RECORD_ID_REGEXP = /^[0-9a-f]{7,40}$/i; // A record ID is a Git commit SHA: 7 (abbreviated) to 40 (full) hexadecimal characters. Anything else cannot be a record and is rejected before reaching git, so a value such as `--output=…` can never be parsed as a command-line option +const RECORD_ID_REGEXP = /^[0-9a-f]{7,40}$/i; // Git commit SHA 7 (abbreviated) to 40 (full) hexadecimal characters. Prevent value such as `--output=…` to be parsed as a command-line option -const CONTROL_CHARACTERS_REGEXP = /\p{Cc}/u; // Matches any Unicode "control" character (general category Cc): the C0 range (U+0000 to U+001F), DEL (U+007F) and the C1 range (U+0080 to U+009F), i.e. 65 non-printable characters including NUL. The `u` flag is required for the `\p{...}` property escape to be recognised, otherwise the pattern would match the literal text `p{Cc}`. Legitimate service IDs, terms types and document IDs never contain these, and NUL in particular can truncate a value once it reaches git or the filesystem, so any segment holding one is rejected. +const CONTROL_CHARACTERS_REGEXP = /\p{Cc}/u; // Matches any Unicode "control" character: the C0 range (U+0000 to U+001F), DEL (U+007F) and the C1 range (U+0080 to U+009F), i.e. 65 non-printable characters including NUL. The `u` flag is required for the `\p{...}` property escape to be recognised, otherwise the pattern would match the literal text `p{Cc}`. Legitimate service IDs, terms types and document IDs never contain these, and NUL in particular can truncate a value once it reaches git or the filesystem, so any segment holding one is rejected. -// A service ID, terms type or document ID forms a single segment of a record file path (see DataMapper.generateFilePath): it may not be empty, be a relative segment (`.` or `..`), or contain a path separator or a control character. -// Rejecting anything else keeps hostile values from reaching git, where a pathspec that resolves outside the repository (such as `../foo/*`) aborts with an error that exposes the repository location. +// Keeps hostile values from reaching git, where a pathspec that resolves outside the repository (such as `../foo/*`) aborts with an error that exposes the repository location. function isPlainPathSegment(segment) { return segment.length > 0 && segment !== '.' @@ -177,10 +176,10 @@ export default class GitRepository extends RepositoryInterface { } return { - last: revisions[0].hash, // newest version of these terms - first: revisions[revisions.length - 1].hash, // oldest version of these terms - next: index > 0 ? revisions[index - 1].hash : null, // the version recorded just after this one - prev: index < revisions.length - 1 ? revisions[index + 1].hash : null, // the version recorded just before this one + last: revisions[0].hash, + first: revisions[revisions.length - 1].hash, + next: index > 0 ? revisions[index - 1].hash : null, + prev: index < revisions.length - 1 ? revisions[index + 1].hash : null, }; } From 15e5898ee037dea9e50c9f3f5eabf006d346b5f6 Mon Sep 17 00:00:00 2001 From: Nicolas Dupont Date: Tue, 7 Jul 2026 10:58:53 +0200 Subject: [PATCH 17/17] Lint --- src/archivist/recorder/repositories/git/git.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/archivist/recorder/repositories/git/git.js b/src/archivist/recorder/repositories/git/git.js index 7cba202fc..4a80eda4a 100644 --- a/src/archivist/recorder/repositories/git/git.js +++ b/src/archivist/recorder/repositories/git/git.js @@ -236,11 +236,11 @@ export default class Git { const [ added, deleted ] = line.split('\t'); // Binary files show '-' for additions/deletions - if (added !== '-') { - additions += parseInt(added, 10); + if (added !== '-') { + additions += parseInt(added, 10); } - if (deleted !== '-') { - deletions += parseInt(deleted, 10); + if (deleted !== '-') { + deletions += parseInt(deleted, 10); } }