Merge pull request #3653 from overleaf/jpa-merge-deleted-docs-sources

[EditorHttpController] fetch deletedDocs from docstore, merge w/ project

GitOrigin-RevId: 5cf46aa7ded034285051ddae21b5c80d8c806693
This commit is contained in:
Jakob Ackermann
2021-04-06 11:15:16 +02:00
committed by Copybot
parent 92194202d7
commit 3bdf7afbbf
7 changed files with 141 additions and 5 deletions

View File

@@ -85,6 +85,28 @@ const DocstoreManager = {
)
},
getAllDeletedDocs(project_id, callback) {
const url = `${settings.apis.docstore.url}/project/${project_id}/doc-deleted`
request.get({ url, timeout: TIMEOUT, json: true }, function(
error,
res,
docs
) {
if (error) {
callback(OError.tag(error, 'could not get deleted docs from docstore'))
} else if (res.statusCode === 200) {
callback(null, docs)
} else {
callback(
new OError(
`docstore api responded with non-success code: ${res.statusCode}`,
{ project_id }
)
)
}
})
},
getAllRanges(project_id, callback) {
if (callback == null) {
callback = function(error) {}

View File

@@ -13,6 +13,7 @@ const AuthenticationController = require('../Authentication/AuthenticationContro
const Errors = require('../Errors/Errors')
const HttpErrorHandler = require('../Errors/HttpErrorHandler')
const ProjectEntityUpdateHandler = require('../Project/ProjectEntityUpdateHandler')
const DocstoreManager = require('../Docstore/DocstoreManager')
const { expressify } = require('../../util/promises')
module.exports = {
@@ -99,6 +100,9 @@ async function _buildJoinProjectView(req, projectId, userId) {
if (project == null) {
throw new Errors.NotFoundError('project not found')
}
const deletedDocsFromDocstore = await DocstoreManager.promises.getAllDeletedDocs(
projectId
)
const members = await CollaboratorsGetter.promises.getInvitedMembersWithPrivilegeLevels(
projectId
)
@@ -127,7 +131,8 @@ async function _buildJoinProjectView(req, projectId, userId) {
project: ProjectEditorHandler.buildProjectModelView(
project,
members,
invites
invites,
deletedDocsFromDocstore
),
privilegeLevel,
isRestrictedUser

View File

@@ -15,10 +15,15 @@ let ProjectEditorHandler
const _ = require('underscore')
const Path = require('path')
function mergeDeletedDocs(a, b) {
const docIdsInA = new Set(a.map(doc => doc._id.toString()))
return a.concat(b.filter(doc => !docIdsInA.has(doc._id.toString())))
}
module.exports = ProjectEditorHandler = {
trackChangesAvailable: false,
buildProjectModelView(project, members, invites) {
buildProjectModelView(project, members, invites, deletedDocsFromDocstore) {
let owner, ownerFeatures
if (!Array.isArray(project.deletedDocs)) {
project.deletedDocs = []
@@ -38,7 +43,10 @@ module.exports = ProjectEditorHandler = {
description: project.description,
spellCheckLanguage: project.spellCheckLanguage,
deletedByExternalDataSource: project.deletedByExternalDataSource || false,
deletedDocs: project.deletedDocs,
deletedDocs: mergeDeletedDocs(
project.deletedDocs,
deletedDocsFromDocstore
),
members: [],
invites,
tokens: project.tokens,

View File

@@ -40,6 +40,10 @@ class MockDocstoreApi extends AbstractMockApi {
res.json(Object.values(this.docs[req.params.projectId] || {}))
})
this.app.get('/project/:projectId/doc-deleted', (req, res) => {
res.json(this.getDeletedDocs(req.params.projectId))
})
this.app.get('/project/:projectId/doc/:docId', (req, res) => {
const { projectId, docId } = req.params
const doc = this.docs[projectId][docId]

View File

@@ -393,6 +393,72 @@ describe('DocstoreManager', function() {
})
})
describe('getAllDeletedDocs', function() {
describe('with a successful response code', function() {
beforeEach(function(done) {
this.callback.callsFake(done)
this.docs = [{ _id: 'mock-doc-id', name: 'foo.tex' }]
this.request.get = sinon
.stub()
.callsArgWith(1, null, { statusCode: 200 }, this.docs)
this.DocstoreManager.getAllDeletedDocs(this.project_id, this.callback)
})
it('should get all the project docs in the docstore api', function() {
this.request.get.should.have.been.calledWith({
url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc-deleted`,
timeout: 30 * 1000,
json: true
})
})
it('should call the callback with the docs', function() {
this.callback.should.have.been.calledWith(null, this.docs)
})
})
describe('with an error', function() {
beforeEach(function(done) {
this.callback.callsFake(() => done())
this.request.get = sinon
.stub()
.callsArgWith(1, new Error('connect failed'))
this.DocstoreManager.getAllDocs(this.project_id, this.callback)
})
it('should call the callback with an error', function() {
this.callback.should.have.been.calledWith(
sinon.match
.instanceOf(Error)
.and(sinon.match.has('message', 'connect failed'))
)
})
})
describe('with a failed response code', function() {
beforeEach(function(done) {
this.callback.callsFake(() => done())
this.request.get = sinon
.stub()
.callsArgWith(1, null, { statusCode: 500 })
this.DocstoreManager.getAllDocs(this.project_id, this.callback)
})
it('should call the callback with an error', function() {
this.callback.should.have.been.calledWith(
sinon.match
.instanceOf(Error)
.and(
sinon.match.has(
'message',
'docstore api responded with non-success code: 500'
)
)
)
})
})
})
describe('getAllRanges', function() {
describe('with a successful response code', function() {
beforeEach(function() {

View File

@@ -119,6 +119,11 @@ describe('EditorHttpController', function() {
convertDocToFile: sinon.stub().resolves(this.file)
}
}
this.DocstoreManager = {
promises: {
getAllDeletedDocs: sinon.stub().resolves([])
}
}
this.HttpErrorHandler = {
notFound: sinon.stub(),
unprocessableEntity: sinon.stub()
@@ -141,6 +146,7 @@ describe('EditorHttpController', function() {
'../../infrastructure/FileWriter': this.FileWriter,
'../Project/ProjectEntityUpdateHandler': this
.ProjectEntityUpdateHandler,
'../Docstore/DocstoreManager': this.DocstoreManager,
'../Errors/HttpErrorHandler': this.HttpErrorHandler
}
})

View File

@@ -106,6 +106,9 @@ describe('ProjectEditorHandler', function() {
token: 'my-secret-token2'
}
]
this.deletedDocsFromDocstore = [
{ _id: 'deleted-doc-id-from-docstore', name: 'docstore.tex' }
]
return (this.handler = SandboxedModule.require(modulePath))
})
@@ -115,7 +118,8 @@ describe('ProjectEditorHandler', function() {
return (this.result = this.handler.buildProjectModelView(
this.project,
this.members,
this.invites
this.invites,
this.deletedDocsFromDocstore
))
})
@@ -155,7 +159,8 @@ describe('ProjectEditorHandler', function() {
// omit deletedAt field
_id: this.project.deletedDocs[0]._id,
name: this.project.deletedDocs[0].name
}
},
this.deletedDocsFromDocstore[0]
])
})
@@ -235,6 +240,26 @@ describe('ProjectEditorHandler', function() {
})
})
describe('when docstore sends a deleted doc that is also present in the project', function() {
beforeEach(function() {
this.deletedDocsFromDocstore.push(this.project.deletedDocs[0])
this.result = this.handler.buildProjectModelView(
this.project,
this.members,
this.invites,
this.deletedDocsFromDocstore
)
})
it('should not send any duplicate', function() {
should.exist(this.result.deletedDocs)
this.result.deletedDocs.should.deep.equal([
this.project.deletedDocs[0],
this.deletedDocsFromDocstore[0]
])
})
})
describe('deletedByExternalDataSource', function() {
it('should set the deletedByExternalDataSource flag to false when it is not there', function() {
delete this.project.deletedByExternalDataSource