github/overleaf
1
0
Fork 0
mirror of https://github.com/overleaf/overleaf.git synced 2024-11-21 20:47:08 -05:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #123 from overleaf/em-doc-hard-delete

Browse source 
Add ignore_flush_errors option to the doc delete endpoint
This commit is contained in:
Eric Mc Sween 2020-03-10 10:11:00 -04:00 committed by GitHub
commit ff32104fe6
7 changed files with 66 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
services/document-updater/app.coffee
View file

@ -54,7 +54,7 @@ app.post '/project/:project_id/get_and_flush_if_old', HttpCont
app.post '/project/:project_id/clearState', HttpController.clearProjectState app.post '/project/:project_id/clearState', HttpController.clearProjectState
app.post '/project/:project_id/doc/:doc_id', HttpController.setDoc app.post '/project/:project_id/doc/:doc_id', HttpController.setDoc
app.post '/project/:project_id/doc/:doc_id/flush', HttpController.flushDocIfLoaded app.post '/project/:project_id/doc/:doc_id/flush', HttpController.flushDocIfLoaded
app.delete '/project/:project_id/doc/:doc_id', HttpController.flushAndDeleteDoc app.delete '/project/:project_id/doc/:doc_id', HttpController.deleteDoc
app.delete '/project/:project_id', HttpController.deleteProject app.delete '/project/:project_id', HttpController.deleteProject
app.delete '/project', HttpController.deleteMultipleProjects app.delete '/project', HttpController.deleteMultipleProjects
app.post '/project/:project_id', HttpController.updateProject app.post '/project/:project_id', HttpController.updateProject

14
services/document-updater/app/coffee/DocumentManager.coffee
View file

@ -91,7 +91,7 @@ module.exports = DocumentManager =
return callback(error) if error? return callback(error) if error?
callback null callback null
else else
DocumentManager.flushAndDeleteDoc project_id, doc_id, (error) -> DocumentManager.flushAndDeleteDoc project_id, doc_id, {}, (error) ->
# There is no harm in flushing project history if the previous # There is no harm in flushing project history if the previous
# call failed and sometimes it is required # call failed and sometimes it is required
HistoryManager.flushProjectChangesAsync project_id HistoryManager.flushProjectChangesAsync project_id
@ -115,14 +115,18 @@ module.exports = DocumentManager =
return callback(error) if error? return callback(error) if error?
RedisManager.clearUnflushedTime doc_id, callback RedisManager.clearUnflushedTime doc_id, callback
flushAndDeleteDoc: (project_id, doc_id, _callback = (error) ->) -> flushAndDeleteDoc: (project_id, doc_id, options, _callback) ->
timer = new Metrics.Timer("docManager.flushAndDeleteDoc") timer = new Metrics.Timer("docManager.flushAndDeleteDoc")
callback = (args...) -> callback = (args...) ->
timer.done() timer.done()
_callback(args...) _callback(args...)
DocumentManager.flushDocIfLoaded project_id, doc_id, (error) -> DocumentManager.flushDocIfLoaded project_id, doc_id, (error) ->
return callback(error) if error? if error?
if options.ignoreFlushErrors
logger.warn {project_id: project_id, doc_id: doc_id, err: error}, "ignoring flush error while deleting document"
else
return callback(error)
# Flush in the background since it requires a http request # Flush in the background since it requires a http request
HistoryManager.flushDocChangesAsync project_id, doc_id HistoryManager.flushDocChangesAsync project_id, doc_id
@ -218,9 +222,9 @@ module.exports = DocumentManager =
UpdateManager = require "./UpdateManager" UpdateManager = require "./UpdateManager"
UpdateManager.lockUpdatesAndDo DocumentManager.flushDocIfLoaded, project_id, doc_id, callback UpdateManager.lockUpdatesAndDo DocumentManager.flushDocIfLoaded, project_id, doc_id, callback
flushAndDeleteDocWithLock: (project_id, doc_id, callback = (error) ->) -> flushAndDeleteDocWithLock: (project_id, doc_id, options, callback) ->
UpdateManager = require "./UpdateManager" UpdateManager = require "./UpdateManager"
UpdateManager.lockUpdatesAndDo DocumentManager.flushAndDeleteDoc, project_id, doc_id, callback UpdateManager.lockUpdatesAndDo DocumentManager.flushAndDeleteDoc, project_id, doc_id, options, callback
acceptChangesWithLock: (project_id, doc_id, change_ids, callback = (error) ->) -> acceptChangesWithLock: (project_id, doc_id, change_ids, callback = (error) ->) ->
UpdateManager = require "./UpdateManager" UpdateManager = require "./UpdateManager"

7
services/document-updater/app/coffee/HttpController.coffee
View file

@ -103,12 +103,13 @@ module.exports = HttpController =
logger.log project_id: project_id, doc_id: doc_id, "flushed doc via http" logger.log project_id: project_id, doc_id: doc_id, "flushed doc via http"
res.send 204 # No Content res.send 204 # No Content
flushAndDeleteDoc: (req, res, next = (error) ->) -> deleteDoc: (req, res, next = (error) ->) ->
doc_id = req.params.doc_id doc_id = req.params.doc_id
project_id = req.params.project_id project_id = req.params.project_id
logger.log project_id: project_id, doc_id: doc_id, "deleting doc via http" ignoreFlushErrors = req.query.ignore_flush_errors == 'true'
timer = new Metrics.Timer("http.deleteDoc") timer = new Metrics.Timer("http.deleteDoc")
DocumentManager.flushAndDeleteDocWithLock project_id, doc_id, (error) -> logger.log project_id: project_id, doc_id: doc_id, "deleting doc via http"
DocumentManager.flushAndDeleteDocWithLock project_id, doc_id, { ignoreFlushErrors: ignoreFlushErrors }, (error) ->
timer.done() timer.done()
# There is no harm in flushing project history if the previous call # There is no harm in flushing project history if the previous call
# failed and sometimes it is required # failed and sometimes it is required

2
services/document-updater/app/coffee/ProjectManager.coffee
View file

@ -52,7 +52,7 @@ module.exports = ProjectManager =
for doc_id in (doc_ids or []) for doc_id in (doc_ids or [])
do (doc_id) -> do (doc_id) ->
jobs.push (callback) -> jobs.push (callback) ->
DocumentManager.flushAndDeleteDocWithLock project_id, doc_id, (error) -> DocumentManager.flushAndDeleteDocWithLock project_id, doc_id, {}, (error) ->
if error? if error?
logger.error err: error, project_id: project_id, doc_id: doc_id, "error deleting doc" logger.error err: error, project_id: project_id, doc_id: doc_id, "error deleting doc"
errors.push(error) errors.push(error)

27
services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee
View file

@ -16,7 +16,7 @@ describe "DocumentManager", ->
"./HistoryManager": @HistoryManager = "./HistoryManager": @HistoryManager =
flushDocChangesAsync: sinon.stub() flushDocChangesAsync: sinon.stub()
flushProjectChangesAsync: sinon.stub() flushProjectChangesAsync: sinon.stub()
"logger-sharelatex": @logger = {log: sinon.stub()} "logger-sharelatex": @logger = {log: sinon.stub(), warn: sinon.stub()}
"./DocOpsManager": @DocOpsManager = {} "./DocOpsManager": @DocOpsManager = {}
"./Metrics": @Metrics = "./Metrics": @Metrics =
Timer: class Timer Timer: class Timer
@ -47,7 +47,7 @@ describe "DocumentManager", ->
beforeEach -> beforeEach ->
@RedisManager.removeDocFromMemory = sinon.stub().callsArg(2) @RedisManager.removeDocFromMemory = sinon.stub().callsArg(2)
@DocumentManager.flushDocIfLoaded = sinon.stub().callsArgWith(2) @DocumentManager.flushDocIfLoaded = sinon.stub().callsArgWith(2)
@DocumentManager.flushAndDeleteDoc @project_id, @doc_id, @callback @DocumentManager.flushAndDeleteDoc @project_id, @doc_id, {}, @callback
it "should flush the doc", -> it "should flush the doc", ->
@DocumentManager.flushDocIfLoaded @DocumentManager.flushDocIfLoaded
@ -70,6 +70,25 @@ describe "DocumentManager", ->
.calledWithExactly(@project_id, @doc_id) .calledWithExactly(@project_id, @doc_id)
.should.equal true .should.equal true
describe "when a flush error occurs", ->
beforeEach ->
@DocumentManager.flushDocIfLoaded = sinon.stub().callsArgWith(2, new Error("boom!"))
@RedisManager.removeDocFromMemory = sinon.stub().callsArg(2)
it "should not remove the doc from redis", (done) ->
@DocumentManager.flushAndDeleteDoc @project_id, @doc_id, {}, (error) =>
error.should.exist
@RedisManager.removeDocFromMemory.called.should.equal false
done()
describe "when ignoring flush errors", ->
it "should remove the doc from redis", (done) ->
@DocumentManager.flushAndDeleteDoc @project_id, @doc_id, { ignoreFlushErrors: true }, (error) =>
if error?
return done(error)
@RedisManager.removeDocFromMemory.called.should.equal true
done()
describe "flushDocIfLoaded", -> describe "flushDocIfLoaded", ->
describe "when the doc is in Redis", -> describe "when the doc is in Redis", ->
beforeEach -> beforeEach ->
@ -220,7 +239,7 @@ describe "DocumentManager", ->
@DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops) @DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops)
@UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null) @UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null)
@DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2) @DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2)
@DocumentManager.flushAndDeleteDoc = sinon.stub().callsArg(2) @DocumentManager.flushAndDeleteDoc = sinon.stub().callsArg(3)
describe "when already loaded", -> describe "when already loaded", ->
beforeEach -> beforeEach ->
@ -276,7 +295,7 @@ describe "DocumentManager", ->
it "should flush and delete the doc from the doc updater", -> it "should flush and delete the doc from the doc updater", ->
@DocumentManager.flushAndDeleteDoc @DocumentManager.flushAndDeleteDoc
.calledWith(@project_id, @doc_id) .calledWith(@project_id, @doc_id, {})
.should.equal true .should.equal true
it "should not flush the project history", -> it "should not flush the project history", ->

27
services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee
View file

@ -265,21 +265,22 @@ describe "HttpController", ->
.calledWith(new Error("oops")) .calledWith(new Error("oops"))
.should.equal true .should.equal true
describe "flushAndDeleteDoc", -> describe "deleteDoc", ->
beforeEach -> beforeEach ->
@req = @req =
params: params:
project_id: @project_id project_id: @project_id
doc_id: @doc_id doc_id: @doc_id
query: {}
describe "successfully", -> describe "successfully", ->
beforeEach -> beforeEach ->
@DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArgWith(2) @DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArgWith(3)
@HttpController.flushAndDeleteDoc(@req, @res, @next) @HttpController.deleteDoc(@req, @res, @next)
it "should flush and delete the doc", -> it "should flush and delete the doc", ->
@DocumentManager.flushAndDeleteDocWithLock @DocumentManager.flushAndDeleteDocWithLock
.calledWith(@project_id, @doc_id) .calledWith(@project_id, @doc_id, { ignoreFlushErrors: false })
.should.equal true .should.equal true
it "should flush project history", -> it "should flush project history", ->
@ -300,10 +301,24 @@ describe "HttpController", ->
it "should time the request", -> it "should time the request", ->
@Metrics.Timer::done.called.should.equal true @Metrics.Timer::done.called.should.equal true
describe "ignoring errors", ->
beforeEach ->
@req.query.ignore_flush_errors = 'true'
@DocumentManager.flushAndDeleteDocWithLock = sinon.stub().yields()
@HttpController.deleteDoc(@req, @res, @next)
it "should delete the doc", ->
@DocumentManager.flushAndDeleteDocWithLock
.calledWith(@project_id, @doc_id, { ignoreFlushErrors: true })
.should.equal true
it "should return a successful No Content response", ->
@res.send.calledWith(204).should.equal true
describe "when an errors occurs", -> describe "when an errors occurs", ->
beforeEach -> beforeEach ->
@DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArgWith(2, new Error("oops")) @DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArgWith(3, new Error("oops"))
@HttpController.flushAndDeleteDoc(@req, @res, @next) @HttpController.deleteDoc(@req, @res, @next)
it "should flush project history", -> it "should flush project history", ->
@HistoryManager.flushProjectChangesAsync @HistoryManager.flushProjectChangesAsync

8
services/document-updater/test/unit/coffee/ProjectManager/flushAndDeleteProjectTests.coffee
View file

@ -23,7 +23,7 @@ describe "ProjectManager - flushAndDeleteProject", ->
beforeEach (done) -> beforeEach (done) ->
@doc_ids = ["doc-id-1", "doc-id-2", "doc-id-3"] @doc_ids = ["doc-id-1", "doc-id-2", "doc-id-3"]
@RedisManager.getDocIdsInProject = sinon.stub().callsArgWith(1, null, @doc_ids) @RedisManager.getDocIdsInProject = sinon.stub().callsArgWith(1, null, @doc_ids)
@DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArg(2) @DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArg(3)
@ProjectManager.flushAndDeleteProjectWithLocks @project_id, {}, (error) => @ProjectManager.flushAndDeleteProjectWithLocks @project_id, {}, (error) =>
@callback(error) @callback(error)
done() done()
@ -36,7 +36,7 @@ describe "ProjectManager - flushAndDeleteProject", ->
it "should delete each doc in the project", -> it "should delete each doc in the project", ->
for doc_id in @doc_ids for doc_id in @doc_ids
@DocumentManager.flushAndDeleteDocWithLock @DocumentManager.flushAndDeleteDocWithLock
.calledWith(@project_id, doc_id) .calledWith(@project_id, doc_id, {})
.should.equal true .should.equal true
it "should flush project history", -> it "should flush project history", ->
@ -54,7 +54,7 @@ describe "ProjectManager - flushAndDeleteProject", ->
beforeEach (done) -> beforeEach (done) ->
@doc_ids = ["doc-id-1", "doc-id-2", "doc-id-3"] @doc_ids = ["doc-id-1", "doc-id-2", "doc-id-3"]
@RedisManager.getDocIdsInProject = sinon.stub().callsArgWith(1, null, @doc_ids) @RedisManager.getDocIdsInProject = sinon.stub().callsArgWith(1, null, @doc_ids)
@DocumentManager.flushAndDeleteDocWithLock = sinon.spy (project_id, doc_id, callback = (error) ->) => @DocumentManager.flushAndDeleteDocWithLock = sinon.spy (project_id, doc_id, options, callback) =>
if doc_id == "doc-id-1" if doc_id == "doc-id-1"
callback(@error = new Error("oops, something went wrong")) callback(@error = new Error("oops, something went wrong"))
else else
@ -66,7 +66,7 @@ describe "ProjectManager - flushAndDeleteProject", ->
it "should still flush each doc in the project", -> it "should still flush each doc in the project", ->
for doc_id in @doc_ids for doc_id in @doc_ids
@DocumentManager.flushAndDeleteDocWithLock @DocumentManager.flushAndDeleteDocWithLock
.calledWith(@project_id, doc_id) .calledWith(@project_id, doc_id, {})
.should.equal true .should.equal true
it "should still flush project history", -> it "should still flush project history", ->