From f326d632ab74223bd666a9cb793e6b8b4b8bea06 Mon Sep 17 00:00:00 2001 From: Ersun Warncke Date: Wed, 5 Feb 2020 08:53:31 -0400 Subject: [PATCH] Revert "Revert "optimize rootDoc_id validation"" This reverts commit 13e4b22daa99b096cf2a9625212a855be59b1fdc. GitOrigin-RevId: f015bc2bb54e98d1271bc2417647638aa80ba843 --- .../app/src/Features/Compile/ClsiManager.js | 158 +++++++++++------- .../test/unit/src/Compile/ClsiManagerTests.js | 4 + .../unit/src/Compile/CompileManagerTests.js | 4 +- 3 files changed, 102 insertions(+), 64 deletions(-) diff --git a/services/web/app/src/Features/Compile/ClsiManager.js b/services/web/app/src/Features/Compile/ClsiManager.js index 1a2e856b64..6161d3073d 100644 --- a/services/web/app/src/Features/Compile/ClsiManager.js +++ b/services/web/app/src/Features/Compile/ClsiManager.js @@ -3,6 +3,7 @@ const Settings = require('settings-sharelatex') const request = require('request') const ProjectGetter = require('../Project/ProjectGetter') const ProjectEntityHandler = require('../Project/ProjectEntityHandler') +const ProjectRootDocManager = require('../Project/ProjectRootDocManager') const logger = require('logger-sharelatex') const Url = require('url') const OError = require('@overleaf/o-error') @@ -465,15 +466,35 @@ const ClsiManager = { return outputFiles }, + _ensureRootDocumentIsValid(project, callback) { + // if root doc is set and id is contained somewhere in directory tree then accept it + try { + if ( + project.rootDoc_id && + project.rootFolder && + JSON.stringify(project.rootFolder).includes(project.rootDoc_id) + ) { + return callback() + } + } catch (err) { + // ignore errors here, which are very unlikely, and just attempt to set the root doc again + } + ProjectRootDocManager.setRootDocAutomatically(project._id, callback) + }, + _buildRequest(projectId, options, callback) { if (options == null) { options = {} } ProjectGetter.getProject( projectId, - { compiler: 1, rootDoc_id: 1, imageName: 1, rootFolder: 1, rootDoc_id: 1 }, + { + compiler: 1, + rootDoc_id: 1, + imageName: 1, + rootFolder: 1 + }, (err, project) => { - console.log("GGGG", project) if (err != null) { return callback( new OError({ @@ -491,71 +512,84 @@ const ClsiManager = { project.compiler = 'pdflatex' } - if (options.incrementalCompilesEnabled || options.syncType != null) { - // new way, either incremental or full - const timer = new Metrics.Timer('editor.compile-getdocs-redis') - ClsiManager.getContentFromDocUpdaterIfMatch( - projectId, - project, - options, - (err, projectStateHash, docUpdaterDocs) => { + ClsiManager._ensureRootDocumentIsValid(project, err => { + if (err != null) { + return callback( + new OError({ + message: 'error setting rootDoc_id', + info: { projectId } + }).withCause(err) + ) + } + if (options.incrementalCompilesEnabled || options.syncType != null) { + // new way, either incremental or full + const timer = new Metrics.Timer('editor.compile-getdocs-redis') + ClsiManager.getContentFromDocUpdaterIfMatch( + projectId, + project, + options, + (err, projectStateHash, docUpdaterDocs) => { + timer.done() + if (err != null) { + logger.error( + { err, projectId }, + 'error checking project state' + ) + // note: we don't bail out when there's an error getting + // incremental files from the docupdater, we just fall back + // to a normal compile below + } + // see if we can send an incremental update to the CLSI + if ( + docUpdaterDocs != null && + options.syncType !== 'full' && + err == null + ) { + Metrics.inc('compile-from-redis') + ClsiManager._buildRequestFromDocupdater( + projectId, + options, + project, + projectStateHash, + docUpdaterDocs, + callback + ) + } else { + Metrics.inc('compile-from-mongo') + ClsiManager._buildRequestFromMongo( + projectId, + options, + project, + projectStateHash, + callback + ) + } + } + ) + } else { + // old way, always from mongo + const timer = new Metrics.Timer('editor.compile-getdocs-mongo') + ClsiManager._getContentFromMongo(projectId, (err, docs, files) => { timer.done() if (err != null) { - logger.error({ err, projectId }, 'error checking project state') - // note: we don't bail out when there's an error getting - // incremental files from the docupdater, we just fall back - // to a normal compile below - } - // see if we can send an incremental update to the CLSI - if ( - docUpdaterDocs != null && - options.syncType !== 'full' && - err == null - ) { - Metrics.inc('compile-from-redis') - ClsiManager._buildRequestFromDocupdater( - projectId, - options, - project, - projectStateHash, - docUpdaterDocs, - callback - ) - } else { - Metrics.inc('compile-from-mongo') - ClsiManager._buildRequestFromMongo( - projectId, - options, - project, - projectStateHash, - callback + return callback( + new OError({ + message: 'failed to get contents from Mongo', + info: { projectId } + }).withCause(err) ) } - } - ) - } else { - // old way, always from mongo - const timer = new Metrics.Timer('editor.compile-getdocs-mongo') - ClsiManager._getContentFromMongo(projectId, (err, docs, files) => { - timer.done() - if (err != null) { - return callback( - new OError({ - message: 'failed to get contents from Mongo', - info: { projectId } - }).withCause(err) + ClsiManager._finaliseRequest( + projectId, + options, + project, + docs, + files, + callback ) - } - ClsiManager._finaliseRequest( - projectId, - options, - project, - docs, - files, - callback - ) - }) - } + }) + } + }) } ) }, diff --git a/services/web/test/unit/src/Compile/ClsiManagerTests.js b/services/web/test/unit/src/Compile/ClsiManagerTests.js index fc9b79da07..845d140128 100644 --- a/services/web/test/unit/src/Compile/ClsiManagerTests.js +++ b/services/web/test/unit/src/Compile/ClsiManagerTests.js @@ -24,6 +24,9 @@ describe('ClsiManager', function() { this.DocumentUpdaterHandler = { getProjectDocsIfMatch: sinon.stub().callsArgWith(2, null, null) } + this.ProjectRootDocManager = { + setRootDocAutomatically: sinon.stub().yields() + } this.logger = { log: sinon.stub(), error: sinon.stub(), @@ -63,6 +66,7 @@ describe('ClsiManager', function() { }, '../Project/ProjectEntityHandler': this.ProjectEntityHandler, '../Project/ProjectGetter': this.ProjectGetter, + '../Project/ProjectRootDocManager': this.ProjectRootDocManager, '../DocumentUpdater/DocumentUpdaterHandler': this .DocumentUpdaterHandler, './ClsiCookieManager': () => this.ClsiCookieManager, diff --git a/services/web/test/unit/src/Compile/CompileManagerTests.js b/services/web/test/unit/src/Compile/CompileManagerTests.js index d9e0007e5e..afadd137bb 100644 --- a/services/web/test/unit/src/Compile/CompileManagerTests.js +++ b/services/web/test/unit/src/Compile/CompileManagerTests.js @@ -76,7 +76,7 @@ describe('CompileManager', function() { this.CompileManager._checkIfRecentlyCompiled = sinon .stub() .callsArgWith(2, null, false) - this.ProjectRootDocManager.ensureRootDocumentIsValid = sinon + this.ProjectRootDocManager.ensureRootDocumentIsSet = sinon .stub() .callsArgWith(1, null) this.CompileManager.getProjectCompileLimits = sinon @@ -115,7 +115,7 @@ describe('CompileManager', function() { }) it('should ensure that the root document is set', function() { - return this.ProjectRootDocManager.ensureRootDocumentIsValid + return this.ProjectRootDocManager.ensureRootDocumentIsSet .calledWith(this.project_id) .should.equal(true) })