From 3aa579f2322665a96269d54201a44e14350eef2e Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Thu, 10 Apr 2025 07:40:32 -0400 Subject: [PATCH] Merge pull request #24736 from overleaf/em-history-get-changes Endpoint for getting history changes GitOrigin-RevId: b96afed0492522d62df9c24390f76e5490afbb44 --- .../history-v1/api/controllers/projects.js | 40 ++++ .../history-v1/api/swagger/project_import.js | 41 +++- .../test/acceptance/js/api/projects.test.js | 192 +++++++++++++++--- services/history-v1/test/setup.js | 1 + 4 files changed, 241 insertions(+), 33 deletions(-) diff --git a/services/history-v1/api/controllers/projects.js b/services/history-v1/api/controllers/projects.js index 7dbeb53fae..15ed076e30 100644 --- a/services/history-v1/api/controllers/projects.js +++ b/services/history-v1/api/controllers/projects.js @@ -138,6 +138,45 @@ async function getHistoryBefore(req, res, next) { } } +/** + * Get all changes since the beginning of history or since a given version + */ +async function getChanges(req, res, next) { + const projectId = req.swagger.params.project_id.value + const since = req.swagger.params.since.value ?? 0 + + if (since < 0) { + // Negative values would cause an infinite loop + return res.status(400).json({ + error: `Version out of bounds: ${since}`, + }) + } + + const changes = [] + let chunk = await chunkStore.loadLatest(projectId) + + if (since > chunk.getEndVersion()) { + return res.status(400).json({ + error: `Version out of bounds: ${since}`, + }) + } + + // Fetch all chunks that come after the chunk that contains the start version + while (chunk.getStartVersion() > since) { + const changesInChunk = chunk.getChanges() + changes.unshift(...changesInChunk) + chunk = await chunkStore.loadAtVersion(projectId, chunk.getStartVersion()) + } + + // Extract the relevant changes from the chunk that contains the start version + const changesInChunk = chunk + .getChanges() + .slice(since - chunk.getStartVersion()) + changes.unshift(...changesInChunk) + + res.json(changes) +} + async function getZip(req, res, next) { const projectId = req.swagger.params.project_id.value const version = req.swagger.params.version.value @@ -337,6 +376,7 @@ module.exports = { getLatestHistoryRaw: expressify(getLatestHistoryRaw), getHistory: expressify(getHistory), getHistoryBefore: expressify(getHistoryBefore), + getChanges: expressify(getChanges), getZip: expressify(getZip), createZip: expressify(createZip), deleteProject: expressify(deleteProject), diff --git a/services/history-v1/api/swagger/project_import.js b/services/history-v1/api/swagger/project_import.js index 60eb47fce4..a93f42d27e 100644 --- a/services/history-v1/api/swagger/project_import.js +++ b/services/history-v1/api/swagger/project_import.js @@ -100,9 +100,48 @@ const importChanges = { ], } +const getChanges = { + 'x-swagger-router-controller': 'projects', + operationId: 'getChanges', + tags: ['Project'], + description: 'Get changes applied to a project', + parameters: [ + { + name: 'project_id', + in: 'path', + description: 'project id', + required: true, + type: 'string', + }, + { + name: 'since', + in: 'query', + description: 'start version', + required: false, + type: 'number', + }, + ], + responses: { + 200: { + description: 'Success', + schema: { + type: 'array', + items: { + $ref: '#/definitions/Change', + }, + }, + }, + }, + security: [ + { + basic: [], + }, + ], +} + exports.paths = { '/projects/{project_id}/import': { post: importSnapshot }, '/projects/{project_id}/legacy_import': { post: importSnapshot }, - '/projects/{project_id}/changes': { post: importChanges }, + '/projects/{project_id}/changes': { get: getChanges, post: importChanges }, '/projects/{project_id}/legacy_changes': { post: importChanges }, } diff --git a/services/history-v1/test/acceptance/js/api/projects.test.js b/services/history-v1/test/acceptance/js/api/projects.test.js index 7af8b0172f..7470a98c4f 100644 --- a/services/history-v1/test/acceptance/js/api/projects.test.js +++ b/services/history-v1/test/acceptance/js/api/projects.test.js @@ -21,6 +21,8 @@ const { Snapshot, Change, AddFileOperation, + EditFileOperation, + TextOperation, } = require('overleaf-editor-core') const testProjects = require('./support/test_projects') @@ -103,56 +105,182 @@ describe('project controller', function () { // https://github.com/overleaf/write_latex/pull/5120#discussion_r244291862 }) - describe('getLatestHashedContent', function () { - let limitsToPersistImmediately + describe('project with changes', function () { + let projectId - before(function () { + beforeEach(async function () { // used to provide a limit which forces us to persist all of the changes. const farFuture = new Date() farFuture.setTime(farFuture.getTime() + 7 * 24 * 3600 * 1000) - limitsToPersistImmediately = { + const limits = { minChangeTimestamp: farFuture, maxChangeTimestamp: farFuture, } - }) - - it('returns a snaphot', async function () { const changes = [ new Change( [new AddFileOperation('test.tex', File.fromString('ab'))], new Date(), [] ), + new Change( + [new AddFileOperation('other.tex', File.fromString('hello'))], + new Date(), + [] + ), ] - const projectId = await createEmptyProject() - await persistChanges(projectId, changes, limitsToPersistImmediately, 0) - const response = - await testServer.basicAuthClient.apis.Project.getLatestHashedContent({ - project_id: projectId, - }) - expect(response.status).to.equal(HTTPStatus.OK) - const snapshot = Snapshot.fromRaw(response.obj) - expect(snapshot.countFiles()).to.equal(1) - expect(snapshot.getFile('test.tex').getHash()).to.equal( - testFiles.STRING_AB_HASH - ) + projectId = await createEmptyProject() + await persistChanges(projectId, changes, limits, 0) }) - describe('getLatestHistoryRaw', function () { - it('should handles read', async function () { - const projectId = fixtures.docs.initializedProject.id + + describe('getLatestHashedContent', function () { + it('returns a snapshot', async function () { const response = - await testServer.pseudoJwtBasicAuthClient.apis.Project.getLatestHistoryRaw( - { - project_id: projectId, - readOnly: 'true', - } + await testServer.basicAuthClient.apis.Project.getLatestHashedContent({ + project_id: projectId, + }) + expect(response.status).to.equal(HTTPStatus.OK) + const snapshot = Snapshot.fromRaw(response.obj) + expect(snapshot.countFiles()).to.equal(2) + expect(snapshot.getFile('test.tex').getHash()).to.equal( + testFiles.STRING_AB_HASH + ) + }) + }) + + describe('getChanges', function () { + it('returns all changes when not given a limit', async function () { + const response = + await testServer.basicAuthClient.apis.Project.getChanges({ + project_id: projectId, + }) + expect(response.status).to.equal(HTTPStatus.OK) + const changes = response.obj + expect(changes.length).to.equal(2) + const filenames = changes + .flatMap(change => change.operations) + .map(operation => operation.pathname) + expect(filenames).to.deep.equal(['test.tex', 'other.tex']) + }) + + it('returns only requested changes', async function () { + const response = + await testServer.basicAuthClient.apis.Project.getChanges({ + project_id: projectId, + since: 1, + }) + expect(response.status).to.equal(HTTPStatus.OK) + const changes = response.obj + expect(changes.length).to.equal(1) + const filenames = changes + .flatMap(change => change.operations) + .map(operation => operation.pathname) + expect(filenames).to.deep.equal(['other.tex']) + }) + + it('rejects negative versions', async function () { + await expect( + testServer.basicAuthClient.apis.Project.getChanges({ + project_id: projectId, + since: -1, + }) + ).to.be.rejectedWith('Bad Request') + }) + + it('rejects out of bounds versions', async function () { + await expect( + testServer.basicAuthClient.apis.Project.getChanges({ + project_id: projectId, + since: 20, + }) + ).to.be.rejectedWith('Bad Request') + }) + }) + }) + + describe('project with many chunks', function () { + let projectId + + beforeEach(async function () { + // used to provide a limit which forces us to persist all of the changes. + const farFuture = new Date() + farFuture.setTime(farFuture.getTime() + 7 * 24 * 3600 * 1000) + const limits = { + minChangeTimestamp: farFuture, + maxChangeTimestamp: farFuture, + maxChunkChanges: 5, + } + const changes = [ + new Change( + [new AddFileOperation('test.tex', File.fromString(''))], + new Date(), + [] + ), + ] + + for (let i = 0; i < 20; i++) { + const textOperation = new TextOperation() + textOperation.retain(i) + textOperation.insert('x') + changes.push( + new Change( + [new EditFileOperation('test.tex', textOperation)], + new Date(), + [] ) - expect(response.body).to.deep.equal({ - startVersion: 0, - endVersion: 1, - endTimestamp: '2032-01-01T00:00:00.000Z', - }) + ) + } + + projectId = await createEmptyProject() + await persistChanges(projectId, changes, limits, 0) + }) + + it('returns all changes when not given a limit', async function () { + const response = await testServer.basicAuthClient.apis.Project.getChanges( + { + project_id: projectId, + } + ) + expect(response.status).to.equal(HTTPStatus.OK) + const changes = response.obj + expect(changes.length).to.equal(21) + expect(changes[10].operations[0].operation.textOperation).to.deep.equal([ + 9, + 'x', + ]) + }) + + it('returns only requested changes', async function () { + const response = await testServer.basicAuthClient.apis.Project.getChanges( + { + project_id: projectId, + since: 10, + } + ) + expect(response.status).to.equal(HTTPStatus.OK) + const changes = response.obj + expect(changes.length).to.equal(11) + expect(changes[2].operations[0].operation.textOperation).to.deep.equal([ + 11, + 'x', + ]) + }) + }) + + describe('getLatestHistoryRaw', function () { + it('should handles read', async function () { + const projectId = fixtures.docs.initializedProject.id + const response = + await testServer.pseudoJwtBasicAuthClient.apis.Project.getLatestHistoryRaw( + { + project_id: projectId, + readOnly: 'true', + } + ) + expect(response.body).to.deep.equal({ + startVersion: 0, + endVersion: 1, + endTimestamp: '2032-01-01T00:00:00.000Z', }) }) }) diff --git a/services/history-v1/test/setup.js b/services/history-v1/test/setup.js index 38c1b283ad..b38753b280 100644 --- a/services/history-v1/test/setup.js +++ b/services/history-v1/test/setup.js @@ -8,6 +8,7 @@ const { knex, mongodb } = require('../storage') require('mongodb').ObjectId.cacheHexString = true chai.use(chaiAsPromised) +chai.config.truncateThreshold = 0 async function setupPostgresDatabase() { this.timeout(60_000)