Merge pull request #29 from sharelatex/ja-support-undo-flag

Support an incoming undo flag for history restores
This commit is contained in:
James Allen 2017-03-21 15:15:21 +00:00 committed by GitHub
commit 51daffbc36
6 changed files with 91 additions and 19 deletions

View file

@ -43,7 +43,7 @@ module.exports = DocumentManager =
return callback(error) if error?
callback null, lines, version, ops, ranges
setDoc: (project_id, doc_id, newLines, source, user_id, _callback = (error) ->) ->
setDoc: (project_id, doc_id, newLines, source, user_id, undoing, _callback = (error) ->) ->
timer = new Metrics.Timer("docManager.setDoc")
callback = (args...) ->
timer.done()
@ -63,6 +63,9 @@ module.exports = DocumentManager =
logger.log doc_id: doc_id, project_id: project_id, oldLines: oldLines, newLines: newLines, "setting a document via http"
DiffCodec.diffAsShareJsOp oldLines, newLines, (error, op) ->
return callback(error) if error?
if undoing
for o in op or []
o.u = true # Turn on undo flag for each op for track changes
update =
doc: doc_id
op: op
@ -161,9 +164,9 @@ module.exports = DocumentManager =
UpdateManager = require "./UpdateManager"
UpdateManager.lockUpdatesAndDo DocumentManager.getDocAndRecentOps, project_id, doc_id, fromVersion, callback
setDocWithLock: (project_id, doc_id, lines, source, user_id, callback = (error) ->) ->
setDocWithLock: (project_id, doc_id, lines, source, user_id, undoing, callback = (error) ->) ->
UpdateManager = require "./UpdateManager"
UpdateManager.lockUpdatesAndDo DocumentManager.setDoc, project_id, doc_id, lines, source, user_id, callback
UpdateManager.lockUpdatesAndDo DocumentManager.setDoc, project_id, doc_id, lines, source, user_id, undoing, callback
flushDocIfLoadedWithLock: (project_id, doc_id, callback = (error) ->) ->
UpdateManager = require "./UpdateManager"

View file

@ -40,16 +40,14 @@ module.exports = HttpController =
setDoc: (req, res, next = (error) ->) ->
doc_id = req.params.doc_id
project_id = req.params.project_id
lines = req.body.lines
source = req.body.source
user_id = req.body.user_id
{lines, source, user_id, undoing} = req.body
lineSize = HttpController._getTotalSizeOfLines(lines)
if lineSize > TWO_MEGABYTES
logger.log {project_id, doc_id, source, lineSize, user_id}, "document too large, returning 406 response"
return res.send 406
logger.log project_id: project_id, doc_id: doc_id, lines: lines, source: source, user_id: user_id, "setting doc via http"
logger.log {project_id, doc_id, lines, source, user_id, undoing}, "setting doc via http"
timer = new Metrics.Timer("http.setDoc")
DocumentManager.setDocWithLock project_id, doc_id, lines, source, user_id, (error) ->
DocumentManager.setDocWithLock project_id, doc_id, lines, source, user_id, undoing, (error) ->
timer.done()
return next(error) if error?
logger.log project_id: project_id, doc_id: doc_id, "set doc via http"

View file

@ -41,7 +41,7 @@ describe "Setting a document", ->
DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) =>
throw error if error?
setTimeout () =>
DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, (error, res, body) =>
DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, false, (error, res, body) =>
@statusCode = res.statusCode
done()
, 200
@ -74,7 +74,7 @@ describe "Setting a document", ->
before (done) ->
[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version}
DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, (error, res, body) =>
DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, false, (error, res, body) =>
@statusCode = res.statusCode
setTimeout done, 200
@ -94,3 +94,60 @@ describe "Setting a document", ->
throw error if error?
expect(lines).to.not.exist
done()
describe "with track changes", ->
before ->
@lines = ["one", "one and a half", "two", "three"]
@id_seed = "587357bd35e64f6157"
@update =
doc: @doc_id
op: [{
d: "one and a half\n"
p: 4
}]
meta:
tc: @id_seed
user_id: @user_id
v: @version
describe "with the undo flag", ->
before (done) ->
[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version}
DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) =>
throw error if error?
DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) =>
throw error if error?
# Go back to old lines, with undo flag
DocUpdaterClient.setDocLines @project_id, @doc_id, @lines, @source, @user_id, true, (error, res, body) =>
@statusCode = res.statusCode
setTimeout done, 200
it "should undo the tracked changes", (done) ->
DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, data) =>
throw error if error?
ranges = data.ranges
expect(ranges.changes).to.be.undefined
done()
describe "without the undo flag", ->
before (done) ->
[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version}
DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) =>
throw error if error?
DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) =>
throw error if error?
# Go back to old lines, without undo flag
DocUpdaterClient.setDocLines @project_id, @doc_id, @lines, @source, @user_id, false, (error, res, body) =>
@statusCode = res.statusCode
setTimeout done, 200
it "should not undo the tracked changes", (done) ->
DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, data) =>
throw error if error?
ranges = data.ranges
expect(ranges.changes.length).to.equal 1
done()

View file

@ -53,13 +53,14 @@ module.exports = DocUpdaterClient =
request.post "http://localhost:3003/project/#{project_id}/doc/#{doc_id}/flush", (error, res, body) ->
callback error, res, body
setDocLines: (project_id, doc_id, lines, source, user_id, callback = (error) ->) ->
setDocLines: (project_id, doc_id, lines, source, user_id, undoing, callback = (error) ->) ->
request.post {
url: "http://localhost:3003/project/#{project_id}/doc/#{doc_id}"
json:
lines: lines
source: source
user_id: user_id
undoing: undoing
}, (error, res, body) ->
callback error, res, body

View file

@ -194,6 +194,7 @@ describe "DocumentManager", ->
beforeEach ->
@beforeLines = ["before", "lines"]
@afterLines = ["after", "lines"]
@ops = [{ i: "foo", p: 4 }, { d: "bar", p: 42 }]
@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @ranges, true)
@DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops)
@UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null)
@ -202,7 +203,7 @@ describe "DocumentManager", ->
describe "when already loaded", ->
beforeEach ->
@DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, @callback
@DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, false, @callback
it "should get the current doc lines", ->
@DocumentManager.getDoc
@ -246,7 +247,7 @@ describe "DocumentManager", ->
describe "when not already loaded", ->
beforeEach ->
@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, false)
@DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, @callback
@DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, false, @callback
it "should flush and delete the doc from the doc updater", ->
@DocumentManager.flushAndDeleteDoc
@ -255,7 +256,7 @@ describe "DocumentManager", ->
describe "without new lines", ->
beforeEach ->
@DocumentManager.setDoc @project_id, @doc_id, null, @source, @user_id, @callback
@DocumentManager.setDoc @project_id, @doc_id, null, @source, @user_id, false, @callback
it "should return the callback with an error", ->
@callback.calledWith(new Error("No lines were passed to setDoc"))
@ -263,6 +264,17 @@ describe "DocumentManager", ->
it "should not try to get the doc lines", ->
@DocumentManager.getDoc.called.should.equal false
describe "with the undoing flag", ->
beforeEach ->
# Copy ops so we don't interfere with other tests
@ops = [{ i: "foo", p: 4 }, { d: "bar", p: 42 }]
@DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops)
@DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, true, @callback
it "should set the undo flag on each op", ->
for op in @ops
op.u.should.equal true
describe "acceptChanges", ->
beforeEach ->
@change_id = "mock-change-id"

View file

@ -125,15 +125,16 @@ describe "HttpController", ->
lines: @lines
source: @source
user_id: @user_id
undoing: @undoing = true
describe "successfully", ->
beforeEach ->
@DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5)
@DocumentManager.setDocWithLock = sinon.stub().callsArgWith(6)
@HttpController.setDoc(@req, @res, @next)
it "should set the doc", ->
@DocumentManager.setDocWithLock
.calledWith(@project_id, @doc_id, @lines, @source, @user_id)
.calledWith(@project_id, @doc_id, @lines, @source, @user_id, @undoing)
.should.equal true
it "should return a successful No Content response", ->
@ -143,7 +144,7 @@ describe "HttpController", ->
it "should log the request", ->
@logger.log
.calledWith(doc_id: @doc_id, project_id: @project_id, lines: @lines, source: @source, user_id: @user_id, "setting doc via http")
.calledWith(doc_id: @doc_id, project_id: @project_id, lines: @lines, source: @source, user_id: @user_id, undoing: @undoing, "setting doc via http")
.should.equal true
it "should time the request", ->
@ -151,7 +152,7 @@ describe "HttpController", ->
describe "when an errors occurs", ->
beforeEach ->
@DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5, new Error("oops"))
@DocumentManager.setDocWithLock = sinon.stub().callsArgWith(6, new Error("oops"))
@HttpController.setDoc(@req, @res, @next)
it "should call next with the error", ->
@ -165,7 +166,7 @@ describe "HttpController", ->
for _ in [0..200000]
lines.push "test test test"
@req.body.lines = lines
@DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5)
@DocumentManager.setDocWithLock = sinon.stub().callsArgWith(6)
@HttpController.setDoc(@req, @res, @next)
it 'should send back a 406 response', ->