From 6408d150d51fa8c10625224ac71548e00c1f12d7 Mon Sep 17 00:00:00 2001 From: M Fahru Date: Wed, 18 Oct 2023 09:24:29 -0700 Subject: [PATCH] Merge pull request #15242 from overleaf/mf-index-all-references-after-bib-is-modified [web] If a particular bib is modified, index all the existing bib files instead of only indexing the modified bib file GitOrigin-RevId: 2d0f965746c45142f0927d06a23759aa5ff52c1a --- .../References/ReferencesController.js | 27 -- .../Features/References/ReferencesHandler.js | 33 --- services/web/app/src/router.js | 10 - .../js/ide/references/ReferencesManager.js | 15 +- .../References/ReferencesControllerTests.js | 150 ----------- .../src/References/ReferencesHandlerTests.js | 240 ------------------ 6 files changed, 1 insertion(+), 474 deletions(-) diff --git a/services/web/app/src/Features/References/ReferencesController.js b/services/web/app/src/Features/References/ReferencesController.js index 04a3a243a1..5b85c74346 100644 --- a/services/web/app/src/Features/References/ReferencesController.js +++ b/services/web/app/src/Features/References/ReferencesController.js @@ -18,33 +18,6 @@ const EditorRealTimeController = require('../Editor/EditorRealTimeController') const { OError } = require('../Errors/Errors') module.exports = ReferencesController = { - index(req, res, next) { - const projectId = req.params.Project_id - const { shouldBroadcast } = req.body - const { docIds } = req.body - if (!docIds || !(docIds instanceof Array)) { - logger.err( - { projectId, docIds }, - "docIds is not valid, should be either Array or String 'ALL'" - ) - return res.sendStatus(400) - } - return ReferencesHandler.index(projectId, docIds, function (error, data) { - if (error) { - OError.tag(error, 'failed to index references', { projectId }) - return next(error) - } - return ReferencesController._handleIndexResponse( - req, - res, - projectId, - shouldBroadcast, - false, - data - ) - }) - }, - indexAll(req, res, next) { const projectId = req.params.Project_id const { shouldBroadcast } = req.body diff --git a/services/web/app/src/Features/References/ReferencesHandler.js b/services/web/app/src/Features/References/ReferencesHandler.js index 38b7f99275..30713fb604 100644 --- a/services/web/app/src/Features/References/ReferencesHandler.js +++ b/services/web/app/src/Features/References/ReferencesHandler.js @@ -25,9 +25,6 @@ const _ = require('underscore') const Async = require('async') const Errors = require('../Errors/Errors') -const oneMinInMs = 60 * 1000 -const fiveMinsInMs = oneMinInMs * 5 - if (!Features.hasFeature('references')) { logger.debug('references search not enabled') } @@ -131,36 +128,6 @@ module.exports = ReferencesHandler = { ) }, - index(projectId, docIds, callback) { - if (callback == null) { - callback = function () {} - } - return ProjectGetter.getProject( - projectId, - { rootFolder: true, owner_ref: 1 }, - function (err, project) { - if (err) { - OError.tag(err, 'error finding project', { - projectId, - }) - return callback(err) - } - if (!project) { - return callback( - new Errors.NotFoundError(`project does not exist: ${projectId}`) - ) - } - return ReferencesHandler._doIndexOperation( - projectId, - project, - docIds, - [], - callback - ) - } - ) - }, - _doIndexOperation(projectId, project, docIds, fileIds, callback) { if (!Features.hasFeature('references')) { return callback() diff --git a/services/web/app/src/router.js b/services/web/app/src/router.js index 38a449db07..d817f225b5 100644 --- a/services/web/app/src/router.js +++ b/services/web/app/src/router.js @@ -143,10 +143,6 @@ const rateLimiters = { points: 30, duration: 60, }), - indexProjectReferences: new RateLimiter('index-project-references', { - points: 30, - duration: 60, - }), miscOutputDownload: new RateLimiter('misc-output-download', { points: 1000, duration: 60 * 60, @@ -1073,12 +1069,6 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) { ChatController.sendMessage ) - webRouter.post( - '/project/:Project_id/references/index', - AuthorizationMiddleware.ensureUserCanReadProject, - RateLimiterMiddleware.rateLimit(rateLimiters.indexProjectReferences), - ReferencesController.index - ) webRouter.post( '/project/:Project_id/references/indexAll', AuthorizationMiddleware.ensureUserCanReadProject, diff --git a/services/web/frontend/js/ide/references/ReferencesManager.js b/services/web/frontend/js/ide/references/ReferencesManager.js index 408d46bbce..3ad2cde92a 100644 --- a/services/web/frontend/js/ide/references/ReferencesManager.js +++ b/services/web/frontend/js/ide/references/ReferencesManager.js @@ -85,24 +85,11 @@ export default ReferencesManager = class ReferencesManager { cacheEntry.timestamp > now - CACHE_LIFETIME && cacheEntry.hash === sha1 if (!isCached) { - this.indexReferences([docId], shouldBroadcast) + this.indexAllReferences(shouldBroadcast) this.existingIndexHash[docId] = { hash: sha1, timestamp: now } } } - indexReferences(docIds, shouldBroadcast) { - const opts = { - docIds, - shouldBroadcast, - _csrf: window.csrfToken, - } - return this.ide.$http - .post(`/project/${this.$scope.project_id}/references/index`, opts) - .then(response => { - return this._storeReferencesKeys(response.data.keys, false) - }) - } - indexAllReferences(shouldBroadcast) { const opts = { shouldBroadcast, diff --git a/services/web/test/unit/src/References/ReferencesControllerTests.js b/services/web/test/unit/src/References/ReferencesControllerTests.js index 0be49cff2e..6d9c711e95 100644 --- a/services/web/test/unit/src/References/ReferencesControllerTests.js +++ b/services/web/test/unit/src/References/ReferencesControllerTests.js @@ -198,154 +198,4 @@ describe('ReferencesController', function () { }) }) }) - - describe('index', function () { - beforeEach(function () { - return (this.call = callback => { - this.controller.index(this.req, this.res, this.next) - return callback() - }) - }) - - describe('with docIds as an array and shouldBroadcast as false', function () { - beforeEach(function () { - return this.ReferencesHandler.index.callsArgWith( - 2, - null, - this.fakeResponseData - ) - }) - - it('should call ReferencesHandler.index', function (done) { - return this.call(() => { - this.ReferencesHandler.index.callCount.should.equal(1) - this.ReferencesHandler.index - .calledWith(this.projectId, this.docIds) - .should.equal(true) - return done() - }) - }) - - it('should return data', function (done) { - return this.call(() => { - this.res.json.callCount.should.equal(1) - this.res.json.calledWith(this.fakeResponseData).should.equal(true) - return done() - }) - }) - - it('should not produce an error', function (done) { - return this.call(() => { - this.res.sendStatus.callCount.should.equal(0) - this.res.sendStatus.calledWith(500).should.equal(false) - this.res.sendStatus.calledWith(400).should.equal(false) - return done() - }) - }) - - it('should not call EditorRealTimController.emitToRoom', function (done) { - return this.call(() => { - this.EditorRealTimeController.emitToRoom.callCount.should.equal(0) - return done() - }) - }) - - describe('when ReferencesHandler.index produces an error', function () { - beforeEach(function () { - return this.ReferencesHandler.index.callsArgWith( - 2, - new Error('woops'), - null - ) - }) - - it('should produce an error response', function (done) { - return this.call(() => { - this.next.callCount.should.equal(1) - this.next - .calledWith(sinon.match.instanceOf(Error)) - .should.equal(true) - return done() - }) - }) - }) - }) - - describe('when shouldBroadcast is true', function () { - beforeEach(function () { - this.ReferencesHandler.index.callsArgWith( - 2, - null, - this.fakeResponseData - ) - return (this.req.body.shouldBroadcast = true) - }) - - it('should call EditorRealTimeController.emitToRoom', function (done) { - return this.call(() => { - this.EditorRealTimeController.emitToRoom.callCount.should.equal(1) - return done() - }) - }) - - it('should not produce an error', function (done) { - return this.call(() => { - this.res.sendStatus.callCount.should.equal(0) - this.res.sendStatus.calledWith(500).should.equal(false) - this.res.sendStatus.calledWith(400).should.equal(false) - return done() - }) - }) - - it('should still return data', function (done) { - return this.call(() => { - this.res.json.callCount.should.equal(1) - this.res.json.calledWith(this.fakeResponseData).should.equal(true) - return done() - }) - }) - }) - - describe('with missing docIds', function () { - beforeEach(function () { - return delete this.req.body.docIds - }) - - it('should produce an error response', function (done) { - return this.call(() => { - this.res.sendStatus.callCount.should.equal(1) - this.res.sendStatus.calledWith(400).should.equal(true) - return done() - }) - }) - - it('should not call ReferencesHandler.index', function (done) { - return this.call(() => { - this.ReferencesHandler.index.callCount.should.equal(0) - return done() - }) - }) - }) - - describe('with invalid docIds', function () { - beforeEach(function () { - return (this.req.body.docIds = 42) - }) - - it('should produce an error response', function (done) { - return this.call(() => { - this.res.sendStatus.callCount.should.equal(1) - this.res.sendStatus.calledWith(400).should.equal(true) - return done() - }) - }) - - it('should not call ReferencesHandler.index', function (done) { - return this.call(() => { - this.ReferencesHandler.index.callCount.should.equal(0) - return done() - }) - }) - }) - }) }) diff --git a/services/web/test/unit/src/References/ReferencesHandlerTests.js b/services/web/test/unit/src/References/ReferencesHandlerTests.js index 7542a0efc2..c73954471a 100644 --- a/services/web/test/unit/src/References/ReferencesHandlerTests.js +++ b/services/web/test/unit/src/References/ReferencesHandlerTests.js @@ -80,246 +80,6 @@ describe('ReferencesHandler', function () { }) }) - describe('index', function () { - beforeEach(function () { - sinon.stub(this.handler, '_findBibDocIds') - sinon.stub(this.handler, '_findBibFileIds') - sinon.stub(this.handler, '_isFullIndex').callsArgWith(1, null, true) - this.request.post.callsArgWith( - 1, - null, - { statusCode: 200 }, - this.fakeResponseData - ) - return (this.call = callback => { - return this.handler.index(this.projectId, this.docIds, callback) - }) - }) - - describe('when references feature is disabled', function () { - beforeEach(function () { - this.Features.hasFeature.withArgs('references').returns(false) - }) - - it('should not try to retrieve any user information', function (done) { - this.call(() => { - this.UserGetter.getUser.callCount.should.equal(0) - done() - }) - }) - - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.equal(undefined) - return done() - }) - }) - }) - - describe('with docIds as an array', function () { - beforeEach(function () { - return (this.docIds = ['aaa', 'ccc']) - }) - - it('should not call _findBibDocIds', function (done) { - return this.call((err, data) => { - this.handler._findBibDocIds.callCount.should.equal(0) - return done() - }) - }) - - it('should call ProjectGetter.getProject', function (done) { - return this.call((err, data) => { - this.ProjectGetter.getProject.callCount.should.equal(1) - this.ProjectGetter.getProject - .calledWith(this.projectId) - .should.equal(true) - return done() - }) - }) - - it('should not call _findBibDocIds', function (done) { - return this.call((err, data) => { - this.handler._findBibDocIds.callCount.should.equal(0) - return done() - }) - }) - - it('should call DocumentUpdaterHandler.flushDocToMongo', function (done) { - return this.call((err, data) => { - this.DocumentUpdaterHandler.flushDocToMongo.callCount.should.equal(2) - this.docIds.forEach(docId => { - return this.DocumentUpdaterHandler.flushDocToMongo - .calledWith(this.projectId, docId) - .should.equal(true) - }) - return done() - }) - }) - - it('should make a request to references service', function (done) { - return this.call((err, data) => { - this.request.post.callCount.should.equal(1) - const arg = this.request.post.firstCall.args[0] - expect(arg.json).to.have.all.keys('docUrls', 'fullIndex') - expect(arg.json.docUrls.length).to.equal(2) - expect(arg.json.fullIndex).to.equal(true) - return done() - }) - }) - - it('should not produce an error', function (done) { - return this.call((err, data) => { - expect(err).to.equal(null) - return done() - }) - }) - - it('should return data', function (done) { - return this.call((err, data) => { - expect(data).to.not.equal(null) - expect(data).to.not.equal(undefined) - expect(data).to.equal(this.fakeResponseData) - return done() - }) - }) - }) - - describe('when ProjectGetter.getProject produces an error', function () { - beforeEach(function () { - return this.ProjectGetter.getProject.callsArgWith(2, new Error('woops')) - }) - - it('should produce an error', function (done) { - return this.call((err, data) => { - expect(err).to.not.equal(null) - expect(err).to.be.instanceof(Error) - expect(data).to.equal(undefined) - return done() - }) - }) - - it('should not send request', function (done) { - return this.call((err, data) => { - this.request.post.callCount.should.equal(0) - return done() - }) - }) - }) - - describe('when ProjectGetter.getProject returns null', function () { - beforeEach(function () { - return this.ProjectGetter.getProject.callsArgWith(2, null) - }) - - it('should produce an error', function (done) { - return this.call((err, data) => { - expect(err).to.not.equal(null) - expect(err).to.be.instanceof(Errors.NotFoundError) - expect(data).to.equal(undefined) - return done() - }) - }) - - it('should not send request', function (done) { - return this.call((err, data) => { - this.request.post.callCount.should.equal(0) - return done() - }) - }) - }) - - describe('when _isFullIndex produces an error', function () { - beforeEach(function () { - this.ProjectGetter.getProject.callsArgWith(2, null, this.fakeProject) - return this.handler._isFullIndex.callsArgWith(1, new Error('woops')) - }) - - it('should produce an error', function (done) { - return this.call((err, data) => { - expect(err).to.not.equal(null) - expect(err).to.be.instanceof(Error) - expect(data).to.equal(undefined) - return done() - }) - }) - - it('should not send request', function (done) { - return this.call((err, data) => { - this.request.post.callCount.should.equal(0) - return done() - }) - }) - }) - - describe('when flushDocToMongo produces an error', function () { - beforeEach(function () { - this.ProjectGetter.getProject.callsArgWith(2, null, this.fakeProject) - this.handler._isFullIndex.callsArgWith(1, false) - return this.DocumentUpdaterHandler.flushDocToMongo.callsArgWith( - 2, - new Error('woops') - ) - }) - - it('should produce an error', function (done) { - return this.call((err, data) => { - expect(err).to.not.equal(null) - expect(err).to.be.instanceof(Error) - expect(data).to.equal(undefined) - return done() - }) - }) - - it('should not send request', function (done) { - return this.call((err, data) => { - this.request.post.callCount.should.equal(0) - return done() - }) - }) - }) - - describe('when request produces an error', function () { - beforeEach(function () { - this.ProjectGetter.getProject.callsArgWith(2, null, this.fakeProject) - this.handler._isFullIndex.callsArgWith(1, null, false) - this.DocumentUpdaterHandler.flushDocToMongo.callsArgWith(2, null) - return this.request.post.callsArgWith(1, new Error('woops')) - }) - - it('should produce an error', function (done) { - return this.call((err, data) => { - expect(err).to.not.equal(null) - expect(err).to.be.instanceof(Error) - expect(data).to.equal(undefined) - return done() - }) - }) - }) - - describe('when request responds with error status', function () { - beforeEach(function () { - this.ProjectGetter.getProject.callsArgWith(2, null, this.fakeProject) - this.handler._isFullIndex.callsArgWith(1, null, false) - return this.request.post.callsArgWith( - 1, - null, - { statusCode: 500 }, - null - ) - }) - - it('should produce an error', function (done) { - return this.call((err, data) => { - expect(err).to.not.equal(null) - expect(err).to.be.instanceof(Error) - expect(data).to.equal(undefined) - return done() - }) - }) - }) - }) - describe('indexAll', function () { beforeEach(function () { sinon.stub(this.handler, '_findBibDocIds').returns(['aaa', 'ccc'])