From 5083060fbbbeff77ffacb702f5cdf9451023dd25 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 1 Dec 2022 07:33:03 -0500 Subject: [PATCH] Merge pull request #10677 from overleaf/em-history-id-string Represent history ids as strings instead of integers GitOrigin-RevId: 18977195b65492836e570c571ec7e8c7e088612b --- .../document-updater/app/js/PersistenceManager.js | 2 +- services/document-updater/app/js/RedisManager.js | 4 ---- .../unit/js/RedisManager/RedisManagerTests.js | 2 +- .../app/src/Features/History/HistoryManager.js | 8 ++++---- .../Features/Project/ProjectCreationHandler.js | 8 +++++--- .../src/Features/Project/ProjectHistoryHandler.js | 6 +++--- .../web/scripts/history/HistoryUpgradeHelper.js | 15 +++++---------- ...de_none_without_conversion_if_no_sl_history.js | 10 +++++----- .../test/unit/src/History/HistoryManagerTests.js | 2 +- .../src/Project/ProjectHistoryHandlerTests.js | 2 +- 10 files changed, 26 insertions(+), 33 deletions(-) diff --git a/services/document-updater/app/js/PersistenceManager.js b/services/document-updater/app/js/PersistenceManager.js index bdfa0dadb3..74ca5635c2 100644 --- a/services/document-updater/app/js/PersistenceManager.js +++ b/services/document-updater/app/js/PersistenceManager.js @@ -99,7 +99,7 @@ function getDoc(projectId, docId, options = {}, _callback) { body.version, body.ranges, body.pathname, - body.projectHistoryId, + body.projectHistoryId?.toString(), body.projectHistoryType ) } else if (res.statusCode === 404) { diff --git a/services/document-updater/app/js/RedisManager.js b/services/document-updater/app/js/RedisManager.js index d00a47f270..60128a8923 100644 --- a/services/document-updater/app/js/RedisManager.js +++ b/services/document-updater/app/js/RedisManager.js @@ -244,10 +244,6 @@ module.exports = RedisManager = { return callback(new Errors.NotFoundError('document not found')) } - if (projectHistoryId != null) { - projectHistoryId = parseInt(projectHistoryId) - } - if (docLines && version && !pathname) { metrics.inc('pathname', 1, { path: 'RedisManager.getDoc', diff --git a/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js b/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js index d983ab01cd..30ced07946 100644 --- a/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js +++ b/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js @@ -106,7 +106,7 @@ describe('RedisManager', function () { this.docId = 'doc-id-123' this.project_id = 'project-id-123' - this.projectHistoryId = 123 + this.projectHistoryId = '123' this.callback = sinon.stub() }) diff --git a/services/web/app/src/Features/History/HistoryManager.js b/services/web/app/src/Features/History/HistoryManager.js index e6da673afa..83282356b9 100644 --- a/services/web/app/src/Features/History/HistoryManager.js +++ b/services/web/app/src/Features/History/HistoryManager.js @@ -11,7 +11,7 @@ async function initializeProject() { settings.apis.project_history.initializeHistoryForNewProjects ) ) { - return + return null } const response = await fetch(`${settings.apis.project_history.url}/project`, { method: 'POST', @@ -25,11 +25,11 @@ async function initializeProject() { }) } const body = await response.json() - const overleafId = body && body.project && body.project.id - if (!overleafId) { + const historyId = body && body.project && body.project.id + if (!historyId) { throw new OError('project-history did not provide an id', { body }) } - return { overleaf_id: overleafId } + return historyId } async function flushProject(projectId) { diff --git a/services/web/app/src/Features/Project/ProjectCreationHandler.js b/services/web/app/src/Features/Project/ProjectCreationHandler.js index bae235ffe4..07cfaea0d4 100644 --- a/services/web/app/src/Features/Project/ProjectCreationHandler.js +++ b/services/web/app/src/Features/Project/ProjectCreationHandler.js @@ -153,9 +153,11 @@ async function _createBlankProject( await ProjectDetailsHandler.promises.validateProjectName(projectName) if (!attributes.overleaf) { - const history = await HistoryManager.promises.initializeProject() - attributes.overleaf = { - history: { id: history ? history.overleaf_id : undefined }, + const historyId = await HistoryManager.promises.initializeProject() + if (historyId != null) { + attributes.overleaf = { + history: { id: historyId }, + } } } diff --git a/services/web/app/src/Features/Project/ProjectHistoryHandler.js b/services/web/app/src/Features/Project/ProjectHistoryHandler.js index 3ae185c668..fcea9eaf73 100644 --- a/services/web/app/src/Features/Project/ProjectHistoryHandler.js +++ b/services/web/app/src/Features/Project/ProjectHistoryHandler.js @@ -154,16 +154,16 @@ const ProjectHistoryHandler = { if (history_id != null) { return callback() } // history already exists, success - return HistoryManager.initializeProject(function (err, history) { + return HistoryManager.initializeProject(function (err, historyId) { if (err != null) { return callback(err) } - if (!(history != null ? history.overleaf_id : undefined)) { + if (historyId == null) { return callback(new Error('failed to initialize history id')) } return ProjectHistoryHandler.setHistoryId( project_id, - history.overleaf_id, + historyId, function (err) { if (err != null) { return callback(err) diff --git a/services/web/scripts/history/HistoryUpgradeHelper.js b/services/web/scripts/history/HistoryUpgradeHelper.js index 84baa900a2..30d48d93e9 100644 --- a/services/web/scripts/history/HistoryUpgradeHelper.js +++ b/services/web/scripts/history/HistoryUpgradeHelper.js @@ -174,16 +174,11 @@ async function doUpgradeForNoneWithoutConversion(project) { // Logic originally from ProjectHistoryHandler.ensureHistoryExistsForProject // However sends a force resync project to project history instead // of a resync request to doc-updater - const historyId = await ProjectHistoryHandler.promises.getHistoryId( - projectId - ) - if (!historyId) { - const history = await HistoryManager.promises.initializeProject() - if (history && history.overleaf_id) { - await ProjectHistoryHandler.promises.setHistoryId( - projectId, - history.overleaf_id - ) + let historyId = await ProjectHistoryHandler.promises.getHistoryId(projectId) + if (historyId == null) { + historyId = await HistoryManager.promises.initializeProject() + if (historyId != null) { + await ProjectHistoryHandler.promises.setHistoryId(projectId, historyId) } } await HistoryManager.promises.resyncProject(projectId, { diff --git a/services/web/scripts/history/upgrade_none_without_conversion_if_no_sl_history.js b/services/web/scripts/history/upgrade_none_without_conversion_if_no_sl_history.js index 18df1a0882..2567adb1a5 100644 --- a/services/web/scripts/history/upgrade_none_without_conversion_if_no_sl_history.js +++ b/services/web/scripts/history/upgrade_none_without_conversion_if_no_sl_history.js @@ -117,15 +117,15 @@ async function doUpgradeForNoneWithoutConversion(project) { // Logic originally from ProjectHistoryHandler.ensureHistoryExistsForProject // However sends a force resync project to project history instead // of a resync request to doc-updater - const historyId = await ProjectHistoryHandler.promises.getHistoryId( + let historyId = await ProjectHistoryHandler.promises.getHistoryId( projectId ) - if (!historyId) { - const history = await HistoryManager.promises.initializeProject() - if (history && history.overleaf_id) { + if (historyId == null) { + historyId = await HistoryManager.promises.initializeProject() + if (historyId != null) { await ProjectHistoryHandler.promises.setHistoryId( projectId, - history.overleaf_id + historyId ) } } diff --git a/services/web/test/unit/src/History/HistoryManagerTests.js b/services/web/test/unit/src/History/HistoryManagerTests.js index 831137f0f6..0e4f81956a 100644 --- a/services/web/test/unit/src/History/HistoryManagerTests.js +++ b/services/web/test/unit/src/History/HistoryManagerTests.js @@ -74,7 +74,7 @@ describe('HistoryManager', function () { }) it('should return the overleaf id', function () { - expect(this.result).to.deep.equal({ overleaf_id: this.overleaf_id }) + expect(this.result).to.deep.equal(this.overleaf_id) }) }) diff --git a/services/web/test/unit/src/Project/ProjectHistoryHandlerTests.js b/services/web/test/unit/src/Project/ProjectHistoryHandlerTests.js index 3a8451e282..56e4f44919 100644 --- a/services/web/test/unit/src/Project/ProjectHistoryHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectHistoryHandlerTests.js @@ -60,7 +60,7 @@ describe('ProjectHistoryHandler', function () { this.newHistoryId = 123456789 this.HistoryManager.initializeProject = sinon .stub() - .callsArgWith(0, null, { overleaf_id: this.newHistoryId }) + .callsArgWith(0, null, this.newHistoryId) this.HistoryManager.flushProject = sinon.stub().callsArg(1) return (this.ProjectEntityUpdateHandler.resyncProjectHistory = sinon .stub()