diff --git a/services/document-updater/app/js/HistoryManager.js b/services/document-updater/app/js/HistoryManager.js index 3a91b29cb8..3963431925 100644 --- a/services/document-updater/app/js/HistoryManager.js +++ b/services/document-updater/app/js/HistoryManager.js @@ -106,10 +106,12 @@ const HistoryManager = { projectHistoryId, docs, files, + opts, function (error) { if (error) { return callback(error) } + if (opts.resyncProjectStructureOnly) return callback() const DocumentManager = require('./DocumentManager') const resyncDoc = (doc, cb) => { DocumentManager.resyncDocContentsWithLock( diff --git a/services/document-updater/app/js/HttpController.js b/services/document-updater/app/js/HttpController.js index d8f2d0c5d3..4a1539b07c 100644 --- a/services/document-updater/app/js/HttpController.js +++ b/services/document-updater/app/js/HttpController.js @@ -433,7 +433,13 @@ function updateProject(req, res, next) { function resyncProjectHistory(req, res, next) { const projectId = req.params.project_id - const { projectHistoryId, docs, files, historyRangesMigration } = req.body + const { + projectHistoryId, + docs, + files, + historyRangesMigration, + resyncProjectStructureOnly, + } = req.body logger.debug( { projectId, docs, files }, @@ -444,6 +450,9 @@ function resyncProjectHistory(req, res, next) { if (historyRangesMigration) { opts.historyRangesMigration = historyRangesMigration } + if (resyncProjectStructureOnly) { + opts.resyncProjectStructureOnly = resyncProjectStructureOnly + } HistoryManager.resyncProjectHistory( projectId, diff --git a/services/document-updater/app/js/ProjectHistoryRedisManager.js b/services/document-updater/app/js/ProjectHistoryRedisManager.js index 888948af09..9a9985d99a 100644 --- a/services/document-updater/app/js/ProjectHistoryRedisManager.js +++ b/services/document-updater/app/js/ProjectHistoryRedisManager.js @@ -152,7 +152,13 @@ const ProjectHistoryRedisManager = { return await ProjectHistoryRedisManager.queueOps(projectId, jsonUpdate) }, - async queueResyncProjectStructure(projectId, projectHistoryId, docs, files) { + async queueResyncProjectStructure( + projectId, + projectHistoryId, + docs, + files, + opts + ) { logger.debug({ projectId, docs, files }, 'queue project structure resync') const projectUpdate = { resyncProjectStructure: { docs, files }, @@ -161,6 +167,9 @@ const ProjectHistoryRedisManager = { ts: new Date(), }, } + if (opts.resyncProjectStructureOnly) { + projectUpdate.resyncProjectStructureOnly = opts.resyncProjectStructureOnly + } const jsonUpdate = JSON.stringify(projectUpdate) return await ProjectHistoryRedisManager.queueOps(projectId, jsonUpdate) }, diff --git a/services/document-updater/test/unit/js/HistoryManager/HistoryManagerTests.js b/services/document-updater/test/unit/js/HistoryManager/HistoryManagerTests.js index d0ac6cb9e0..2fd019d4c2 100644 --- a/services/document-updater/test/unit/js/HistoryManager/HistoryManagerTests.js +++ b/services/document-updater/test/unit/js/HistoryManager/HistoryManagerTests.js @@ -217,34 +217,75 @@ describe('HistoryManager', function () { .stub() .yields() this.DocumentManager.resyncDocContentsWithLock = sinon.stub().yields() - this.HistoryManager.resyncProjectHistory( - this.project_id, - this.projectHistoryId, - this.docs, - this.files, - this.callback - ) }) - it('should queue a project structure reync', function () { - this.ProjectHistoryRedisManager.queueResyncProjectStructure - .calledWith( + describe('full sync', function () { + beforeEach(function () { + this.HistoryManager.resyncProjectHistory( this.project_id, this.projectHistoryId, this.docs, - this.files + this.files, + {}, + this.callback ) - .should.equal(true) + }) + + it('should queue a project structure reync', function () { + this.ProjectHistoryRedisManager.queueResyncProjectStructure + .calledWith( + this.project_id, + this.projectHistoryId, + this.docs, + this.files + ) + .should.equal(true) + }) + + it('should queue doc content reyncs', function () { + this.DocumentManager.resyncDocContentsWithLock + .calledWith(this.project_id, this.docs[0].doc, this.docs[0].path) + .should.equal(true) + }) + + it('should call the callback', function () { + this.callback.called.should.equal(true) + }) }) - it('should queue doc content reyncs', function () { - this.DocumentManager.resyncDocContentsWithLock - .calledWith(this.project_id, this.docs[0].doc, this.docs[0].path) - .should.equal(true) - }) + describe('resyncProjectStructureOnly=true', function () { + beforeEach(function () { + this.HistoryManager.resyncProjectHistory( + this.project_id, + this.projectHistoryId, + this.docs, + this.files, + { resyncProjectStructureOnly: true }, + this.callback + ) + }) - it('should call the callback', function () { - this.callback.called.should.equal(true) + it('should queue a project structure reync', function () { + this.ProjectHistoryRedisManager.queueResyncProjectStructure + .calledWith( + this.project_id, + this.projectHistoryId, + this.docs, + this.files, + { resyncProjectStructureOnly: true } + ) + .should.equal(true) + }) + + it('should not queue doc content reyncs', function () { + this.DocumentManager.resyncDocContentsWithLock.called.should.equal( + false + ) + }) + + it('should call the callback', function () { + this.callback.called.should.equal(true) + }) }) }) }) diff --git a/services/project-history/app/js/Errors.js b/services/project-history/app/js/Errors.js index 54aab952a9..0b8d24b0d2 100644 --- a/services/project-history/app/js/Errors.js +++ b/services/project-history/app/js/Errors.js @@ -8,3 +8,4 @@ export class InconsistentChunkError extends OError {} export class UpdateWithUnknownFormatError extends OError {} export class UnexpectedOpTypeError extends OError {} export class TooManyRequestsError extends OError {} +export class NeedFullProjectStructureResyncError extends OError {} diff --git a/services/project-history/app/js/RedisManager.js b/services/project-history/app/js/RedisManager.js index ac3197daf2..fe17508452 100644 --- a/services/project-history/app/js/RedisManager.js +++ b/services/project-history/app/js/RedisManager.js @@ -136,6 +136,9 @@ async function getUpdatesInBatches(projectId, batchSize, runner) { moreBatches = true break } + if (update.resyncProjectStructureOnly) { + update._raw = rawUpdate + } rawUpdates.push(rawUpdate) updates.push(update) @@ -151,6 +154,26 @@ async function getUpdatesInBatches(projectId, batchSize, runner) { } } +/** + * @param {string} projectId + * @param {ResyncProjectStructureUpdate} update + * @return {Promise} + */ +async function deleteAppliedDocUpdate(projectId, update) { + const raw = update._raw + // Delete the first occurrence of the update with LREM KEY COUNT + // VALUE by setting COUNT to 1 which 'removes COUNT elements equal to + // value moving from head to tail.' + // + // If COUNT is 0 the entire list would be searched which would block + // redis since it would be an O(N) operation where N is the length of + // the queue, in a multi of the batch size. + metrics.summary('redis.projectHistoryOps', raw.length, { + status: 'lrem', + }) + await rclient.lrem(Keys.projectHistoryOps({ project_id: projectId }), 1, raw) +} + async function deleteAppliedDocUpdates(projectId, updates) { const multi = rclient.multi() // Delete all the updates which have been applied (exact match) @@ -160,7 +183,7 @@ async function deleteAppliedDocUpdates(projectId, updates) { // value moving from head to tail.' // // If COUNT is 0 the entire list would be searched which would block - // redis snce it would be an O(N) operation where N is the length of + // redis since it would be an O(N) operation where N is the length of // the queue, in a multi of the batch size. metrics.summary('redis.projectHistoryOps', update.length, { status: 'lrem', @@ -383,6 +406,7 @@ export const promises = { countUnprocessedUpdates, getRawUpdatesBatch, deleteAppliedDocUpdates, + deleteAppliedDocUpdate, destroyDocUpdatesQueue, getUpdatesInBatches, getProjectIdsWithHistoryOps, diff --git a/services/project-history/app/js/SyncManager.js b/services/project-history/app/js/SyncManager.js index 3865dd8bf9..67e89cc85a 100644 --- a/services/project-history/app/js/SyncManager.js +++ b/services/project-history/app/js/SyncManager.js @@ -8,7 +8,7 @@ import logger from '@overleaf/logger' import Metrics from '@overleaf/metrics' import OError from '@overleaf/o-error' import { File, Range } from 'overleaf-editor-core' -import { SyncError } from './Errors.js' +import { NeedFullProjectStructureResyncError, SyncError } from './Errors.js' import { db, ObjectId } from './mongodb.js' import * as SnapshotManager from './SnapshotManager.js' import * as LockManager from './LockManager.js' @@ -100,6 +100,9 @@ async function _startResyncWithoutLock(projectId, options) { if (options.historyRangesMigration) { webOpts.historyRangesMigration = options.historyRangesMigration } + if (options.resyncProjectStructureOnly) { + webOpts.resyncProjectStructureOnly = options.resyncProjectStructureOnly + } await WebApiManager.promises.requestResync(projectId, webOpts) await setResyncState(projectId, syncState) } @@ -281,8 +284,10 @@ class SyncState { }) } - for (const doc of update.resyncProjectStructure.docs) { - this.startDocContentSync(doc.path) + if (!update.resyncProjectStructureOnly) { + for (const doc of update.resyncProjectStructure.docs) { + this.startDocContentSync(doc.path) + } } this.stopProjectStructureSync() @@ -475,6 +480,28 @@ class SyncUpdateExpander { persistedBinaryFiles ) this.queueSetMetadataOpsForLinkedFiles(update) + + if (update.resyncProjectStructureOnly) { + const docPaths = new Set() + for (const entity of update.resyncProjectStructure.docs) { + const path = UpdateTranslator._convertPathname(entity.path) + docPaths.add(path) + } + for (const expandedUpdate of this.expandedUpdates) { + if (docPaths.has(expandedUpdate.pathname)) { + // Clear the resync state and queue entry, we need to start over. + this.expandedUpdates = [] + await clearResyncState(this.projectId) + await RedisManager.promises.deleteAppliedDocUpdate( + this.projectId, + update + ) + throw new NeedFullProjectStructureResyncError( + 'aborting partial resync: touched doc' + ) + } + } + } } else if ('resyncDocContent' in update) { logger.debug( { projectId: this.projectId, update }, diff --git a/services/project-history/app/js/WebApiManager.js b/services/project-history/app/js/WebApiManager.js index dc1c366892..2697db29c7 100644 --- a/services/project-history/app/js/WebApiManager.js +++ b/services/project-history/app/js/WebApiManager.js @@ -39,6 +39,9 @@ async function requestResync(projectId, opts = {}) { if (opts.historyRangesMigration) { body.historyRangesMigration = opts.historyRangesMigration } + if (opts.resyncProjectStructureOnly) { + body.resyncProjectStructureOnly = opts.resyncProjectStructureOnly + } await fetchNothing( `${Settings.apis.web.url}/project/${projectId}/history/resync`, { diff --git a/services/project-history/app/js/types.ts b/services/project-history/app/js/types.ts index 37cd50f9a9..206ccfdcd5 100644 --- a/services/project-history/app/js/types.ts +++ b/services/project-history/app/js/types.ts @@ -94,6 +94,9 @@ export type ResyncProjectStructureUpdate = { meta: { ts: string } + // optional fields for resyncProjectStructureOnly=true + resyncProjectStructureOnly?: boolean + _raw: string } export type ResyncDocContentUpdate = { diff --git a/services/project-history/test/acceptance/js/SyncTests.js b/services/project-history/test/acceptance/js/SyncTests.js index 085462c7e2..89e002d4dd 100644 --- a/services/project-history/test/acceptance/js/SyncTests.js +++ b/services/project-history/test/acceptance/js/SyncTests.js @@ -9,6 +9,7 @@ import Settings from '@overleaf/settings' import * as ProjectHistoryClient from './helpers/ProjectHistoryClient.js' import * as ProjectHistoryApp from './helpers/ProjectHistoryApp.js' import sinon from 'sinon' +import { getFailure } from './helpers/ProjectHistoryClient.js' const { ObjectId } = mongodb const EMPTY_FILE_HASH = 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391' @@ -1315,6 +1316,228 @@ describe('Syncing with web and doc-updater', function () { ) }) }) + + describe('resyncProjectStructureOnly', function () { + it('should handle structure only updates', function (done) { + const fileHash = 'aed2973e4b8a7ff1b30ff5c4751e5a2b38989e74' + + MockHistoryStore() + .get(`/api/projects/${historyId}/latest/history`) + .reply(200, { + chunk: { + history: { + snapshot: { + files: { + 'main.tex': { + hash: '0a207c060e61f3b88eaee0a8cd0696f46fb155eb', + stringLength: 3, + }, + }, + }, + changes: [], + }, + startVersion: 0, + }, + }) + + const docContentRequest = MockHistoryStore() + .get( + `/api/projects/${historyId}/blobs/0a207c060e61f3b88eaee0a8cd0696f46fb155eb` + ) + .reply(200, 'a\nb') + MockHistoryStore() + .head(`/api/projects/${historyId}/blobs/${fileHash}`) + .reply(200) + const addFile = MockHistoryStore() + .post(`/api/projects/${historyId}/legacy_changes`, body => { + expect(body).to.deep.equal([ + { + v2Authors: [], + authors: [], + timestamp: this.timestamp.toJSON(), + operations: [ + { + pathname: 'test.png', + file: { + hash: fileHash, + }, + }, + ], + origin: { kind: 'test-origin' }, + }, + ]) + return true + }) + .query({ end_version: 0 }) + .reply(204) + + // allow a 2nd resync + MockWeb() + .post(`/project/${this.project_id}/history/resync`) + .reply(204) + + async.series( + [ + cb => { + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + cb => { + const update = { + projectHistoryId: historyId, + resyncProjectStructureOnly: true, + resyncProjectStructure: { + docs: [{ path: '/main.tex' }], + files: [ + { + file: this.file_id, + path: '/test.png', + _hash: fileHash, + createdBlob: true, + }, + ], + }, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + ProjectHistoryClient.flushProject(this.project_id, cb) + }, + cb => { + // fails when previous resync did not finish + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + ], + error => { + if (error) { + throw error + } + assert( + addFile.isDone(), + `/api/projects/${historyId}/changes should have been called` + ) + assert( + !docContentRequest.isDone(), + 'should not have requested doc content' + ) + done() + } + ) + }) + it('should reject partial resync on docs', function (done) { + const fileHash = 'aed2973e4b8a7ff1b30ff5c4751e5a2b38989e74' + + MockHistoryStore() + .get(`/api/projects/${historyId}/latest/history`) + .reply(200, { + chunk: { + history: { + snapshot: { + files: { + 'main.tex': { + hash: '0a207c060e61f3b88eaee0a8cd0696f46fb155eb', + stringLength: 3, + }, + }, + }, + changes: [], + }, + startVersion: 0, + }, + }) + + const docContentRequest = MockHistoryStore() + .get( + `/api/projects/${historyId}/blobs/0a207c060e61f3b88eaee0a8cd0696f46fb155eb` + ) + .reply(200, 'a\nb') + MockHistoryStore() + .head(`/api/projects/${historyId}/blobs/${fileHash}`) + .reply(200) + const addFile = MockHistoryStore() + .post(`/api/projects/${historyId}/legacy_changes`) + .query({ end_version: 0 }) + .reply(204) + + // allow a 2nd resync + MockWeb() + .post(`/project/${this.project_id}/history/resync`) + .reply(204) + + async.series( + [ + cb => { + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + cb => { + const update = { + projectHistoryId: historyId, + resyncProjectStructureOnly: true, + resyncProjectStructure: { + docs: [{ path: '/main-renamed.tex' }], + files: [ + { + file: this.file_id, + path: '/test.png', + _hash: fileHash, + createdBlob: true, + }, + ], + }, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + ProjectHistoryClient.flushProject( + this.project_id, + { allowErrors: true }, + (err, res) => { + if (err) return cb(err) + expect(res.statusCode).to.equal(500) + expect(loggerError).to.have.been.calledWith( + sinon.match({ + err: { + name: 'NeedFullProjectStructureResyncError', + message: 'aborting partial resync: touched doc', + }, + }) + ) + + getFailure(this.project_id, (err, failure) => { + if (err) return cb(err) + expect(failure).to.include({ + error: + 'NeedFullProjectStructureResyncError: aborting partial resync: touched doc', + }) + cb() + }) + } + ) + }, + cb => { + // fails when previous resync did not finish + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + ], + error => { + if (error) { + throw error + } + assert(!addFile.isDone(), 'should not have persisted changes') + assert( + !docContentRequest.isDone(), + 'should not have requested doc content' + ) + done() + } + ) + }) + }) }) }) }) diff --git a/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js b/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js index cadc7969a7..7a30b27740 100644 --- a/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js +++ b/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js @@ -330,6 +330,10 @@ export function setFailure(failureEntry, callback) { ) } +export function getFailure(projectId, callback) { + db.projectHistoryFailures.findOne({ project_id: projectId }, callback) +} + export function transferLabelOwnership(fromUser, toUser, callback) { request.post( { diff --git a/services/project-history/test/unit/js/SyncManager/SyncManagerTests.js b/services/project-history/test/unit/js/SyncManager/SyncManagerTests.js index 8d7f8f1496..1004ffd78d 100644 --- a/services/project-history/test/unit/js/SyncManager/SyncManagerTests.js +++ b/services/project-history/test/unit/js/SyncManager/SyncManagerTests.js @@ -411,6 +411,39 @@ describe('SyncManager', function () { }) }) + it('records docs to resync when resyncProjectStructureOnly=true is not set', async function () { + const updates = [this.projectStructureSyncUpdate] + const { updates: filteredUpdates, syncState } = + await this.SyncManager.promises.skipUpdatesDuringSync( + this.projectId, + updates + ) + + expect(filteredUpdates).to.deep.equal([this.projectStructureSyncUpdate]) + expect(syncState.toRaw()).to.deep.equal({ + resyncProjectStructure: false, + resyncDocContents: ['new.tex'], + origin: { kind: 'history-resync' }, + }) + }) + + it('records no docs to resync with resyncProjectStructureOnly=true', async function () { + this.projectStructureSyncUpdate.resyncProjectStructureOnly = true + const updates = [this.projectStructureSyncUpdate] + const { updates: filteredUpdates, syncState } = + await this.SyncManager.promises.skipUpdatesDuringSync( + this.projectId, + updates + ) + + expect(filteredUpdates).to.deep.equal([this.projectStructureSyncUpdate]) + expect(syncState.toRaw()).to.deep.equal({ + resyncProjectStructure: false, + resyncDocContents: [], + origin: { kind: 'history-resync' }, + }) + }) + it('allow project structure updates after project structure sync update', async function () { const updates = [this.projectStructureSyncUpdate, this.renameUpdate] const { updates: filteredUpdates, syncState } = diff --git a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js index b20a61d883..0aedff8853 100644 --- a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js +++ b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js @@ -302,6 +302,9 @@ function resyncProjectHistory( if (opts.historyRangesMigration) { body.historyRangesMigration = opts.historyRangesMigration } + if (opts.resyncProjectStructureOnly) { + body.resyncProjectStructureOnly = opts.resyncProjectStructureOnly + } _makeRequest( { path: `/project/${projectId}/history/resync`, diff --git a/services/web/app/src/Features/History/HistoryController.js b/services/web/app/src/Features/History/HistoryController.js index d7f01b2fd2..f6ca483dce 100644 --- a/services/web/app/src/Features/History/HistoryController.js +++ b/services/web/app/src/Features/History/HistoryController.js @@ -148,6 +148,9 @@ module.exports = HistoryController = { if (historyRangesMigration) { opts.historyRangesMigration = historyRangesMigration } + if (req.body.resyncProjectStructureOnly) { + opts.resyncProjectStructureOnly = req.body.resyncProjectStructureOnly + } ProjectEntityUpdateHandler.resyncProjectHistory( projectId, opts,