From 0a65a5eabf564308765affe91eccb9d705368199 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 5 Dec 2016 14:21:49 +0000 Subject: [PATCH] Set and get ranges along with lines --- .../docstore/app/coffee/DocManager.coffee | 68 ++++++--- .../docstore/app/coffee/HttpController.coffee | 5 +- .../docstore/app/coffee/RangeManager.coffee | 35 +++++ .../acceptance/coffee/ArchiveDocsTests.coffee | 22 ++- .../coffee/DeletingDocsTests.coffee | 3 +- .../coffee/GettingAllDocsTests.coffee | 6 +- .../acceptance/coffee/GettingDocsTests.coffee | 20 ++- .../coffee/UpdatingDocsTests.coffee | 130 +++++++++++------ .../coffee/helpers/DocstoreClient.coffee | 18 +-- .../test/unit/coffee/DocManagerTests.coffee | 131 ++++++++++++------ .../unit/coffee/HttpControllerTests.coffee | 3 +- 11 files changed, 294 insertions(+), 147 deletions(-) create mode 100644 services/docstore/app/coffee/RangeManager.coffee diff --git a/services/docstore/app/coffee/DocManager.coffee b/services/docstore/app/coffee/DocManager.coffee index 09218ed5b8..df63383bb1 100644 --- a/services/docstore/app/coffee/DocManager.coffee +++ b/services/docstore/app/coffee/DocManager.coffee @@ -3,6 +3,7 @@ Errors = require "./Errors" logger = require "logger-sharelatex" _ = require "underscore" DocArchive = require "./DocArchiveManager" +RangeManager = require "./RangeManager" module.exports = DocManager = # TODO: For historical reasons, the doc version is currently stored in the docOps @@ -40,7 +41,7 @@ module.exports = DocManager = else return callback(null, docs) - updateDoc: (project_id, doc_id, lines, version, callback = (error, modified, rev) ->) -> + updateDoc: (project_id, doc_id, lines, version, ranges, callback = (error, modified, rev) ->) -> if !lines? or !version? return callback(new Error("no lines or version provided")) @@ -48,33 +49,56 @@ module.exports = DocManager = if err? and !(err instanceof Errors.NotFoundError) logger.err project_id: project_id, doc_id: doc_id, err:err, "error getting document for update" return callback(err) + + ranges = RangeManager.jsonRangesToMongo(ranges) - isNewDoc = lines.length == 0 - linesAreSame = _.isEqual(doc?.lines, lines) - versionsAreSame = (doc?.version == version) - - if linesAreSame and versionsAreSame and !isNewDoc - logger.log project_id: project_id, doc_id: doc_id, rev: doc?.rev, "doc lines have not changed - not updating" - return callback null, false, doc?.rev + if !doc? + # If the document doesn't exist, we'll make sure to create/update all parts of it. + updateLines = true + updateVersion = true + updateRanges = true else - oldRev = doc?.rev || 0 - logger.log { - project_id: project_id - doc_id: doc_id, - oldDocLines: doc?.lines - newDocLines: lines - rev: oldRev - oldVersion: doc?.version - newVersion: version - }, "updating doc lines" - MongoManager.upsertIntoDocCollection project_id, doc_id, {lines}, (error)-> + updateLines = not _.isEqual(doc.lines, lines) + updateVersion = (doc.version != version) + updateRanges = RangeManager.shouldUpdateRanges(doc.ranges, ranges) + + modified = false + rev = doc?.rev || 0 + + updateLinesAndRangesIfNeeded = (cb) -> + if updateLines or updateRanges + update = {} + if updateLines + update.lines = lines + if updateRanges + update.ranges = ranges + logger.log { project_id, doc_id, oldDoc: doc, update: update }, "updating doc lines and ranges" + + modified = true + rev += 1 # rev will be incremented in mongo by MongoManager.upsertIntoDocCollection + MongoManager.upsertIntoDocCollection project_id, doc_id, update, cb + else + logger.log { project_id, doc_id, }, "doc lines have not changed - not updating" + cb() + + updateVersionIfNeeded = (cb) -> + if updateVersion + logger.log { project_id, doc_id, oldVersion: doc?.version, newVersion: version }, "updating doc version" + modified = true + MongoManager.setDocVersion doc_id, version, cb + else + logger.log { project_id, doc_id, version }, "doc version has not changed - not updating" + cb() + + updateLinesAndRangesIfNeeded (error) -> + return callback(error) if error? + updateVersionIfNeeded (error) -> return callback(callback) if error? - MongoManager.setDocVersion doc_id, version, (error) -> - return callback(error) if error? - callback null, true, oldRev + 1 # rev will have been incremented in mongo by MongoManager.updateDoc + callback null, modified, rev deleteDoc: (project_id, doc_id, callback = (error) ->) -> DocManager.getDoc project_id, doc_id, { version: false }, (error, doc) -> return callback(error) if error? return callback new Errors.NotFoundError("No such project/doc to delete: #{project_id}/#{doc_id}") if !doc? MongoManager.markDocAsDeleted project_id, doc_id, callback + diff --git a/services/docstore/app/coffee/HttpController.coffee b/services/docstore/app/coffee/HttpController.coffee index 5425b78c13..cae8fedb53 100644 --- a/services/docstore/app/coffee/HttpController.coffee +++ b/services/docstore/app/coffee/HttpController.coffee @@ -50,6 +50,7 @@ module.exports = HttpController = doc_id = req.params.doc_id lines = req.body?.lines version = req.body?.version + ranges = req.body?.ranges if !lines? or lines not instanceof Array logger.error project_id: project_id, doc_id: doc_id, "no doc lines provided" @@ -62,7 +63,7 @@ module.exports = HttpController = return logger.log project_id: project_id, doc_id: doc_id, "got http request to update doc" - DocManager.updateDoc project_id, doc_id, lines, version, (error, modified, rev) -> + DocManager.updateDoc project_id, doc_id, lines, version, ranges, (error, modified, rev) -> return next(error) if error? res.json { modified: modified @@ -86,6 +87,8 @@ module.exports = HttpController = } if doc.version? doc_view.version = doc.version + if doc.ranges? + doc_view.ranges = doc.ranges return doc_view _buildRawDocView: (doc)-> diff --git a/services/docstore/app/coffee/RangeManager.coffee b/services/docstore/app/coffee/RangeManager.coffee new file mode 100644 index 0000000000..9cc65677d4 --- /dev/null +++ b/services/docstore/app/coffee/RangeManager.coffee @@ -0,0 +1,35 @@ +_ = require "underscore" +{ObjectId} = require("./mongojs") + +module.exports = RangeManager = + shouldUpdateRanges: (doc_ranges, incoming_ranges) -> + # TODO: If we have no incoming_ranges, just ignore for now while + # we're rolling this out, but eventually this should be a mandatory + # field and this will become an error condition + return false if !incoming_ranges? + + # If the ranges are empty, we don't store them in the DB, so set + # doc_ranges to an empty object as default, since this is was the + # incoming_ranges will be for an empty range set. + if !doc_ranges? + doc_ranges = {} + + return not _.isEqual(doc_ranges, incoming_ranges) + + jsonRangesToMongo: (ranges) -> + return null if !ranges? + for change in ranges.changes or [] + change.id = @_safeObjectId(change.id) + if change.metadata?.ts? + change.metadata.ts = new Date(change.metadata.ts) + for comment in ranges.comments or [] + comment.id = @_safeObjectId(comment.id) + if comment.metadata?.ts? + comment.metadata.ts = new Date(comment.metadata.ts) + return ranges + + _safeObjectId: (data) -> + try + return ObjectId(data) + catch + return data \ No newline at end of file diff --git a/services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee b/services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee index 0d602b49cb..80204da731 100644 --- a/services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee @@ -8,7 +8,7 @@ Settings = require("settings-sharelatex") DocstoreClient = require "./helpers/DocstoreClient" -describe "Archiving all docs", -> +describe "Archiving", -> beforeEach (done) -> @callback = sinon.stub() @project_id = ObjectId() @@ -25,18 +25,19 @@ describe "Archiving all docs", -> lines: [ "", "undefined", "undef", "null", "NULL", "(null)", "nil", "NIL", "true", "false", "True", "False", "None", "\\", "\\\\", "0", "1", "1.00", "$1.00", "1/2", "1E2", "1E02", "1E+02", "-1", "-1.00", "-$1.00", "-1/2", "-1E2", "-1E02", "-1E+02", "1/0", "0/0", "-2147483648/-1", "-9223372036854775808/-1", "0.00", "0..0", ".", "0.0.0", "0,00", "0,,0", ",", "0,0,0", "0.0/0", "1.0/0.0", "0.0/0.0", "1,0/0,0", "0,0/0,0", "--1", "-", "-.", "-,", "999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999", "NaN", "Infinity", "-Infinity", "0x0", "0xffffffff", "0xffffffffffffffff", "0xabad1dea", "123456789012345678901234567890123456789", "1,000.00", "1 000.00", "1'000.00", "1,000,000.00", "1 000 000.00", "1'000'000.00", "1.000,00", "1 000,00", "1'000,00", "1.000.000,00", "1 000i̳̞v̢͇ḙ͎͟-҉̭̩̼͔m̤̭̫i͕͇̝̦n̗͙ḍ̟ ̯̲͕͞ǫ̟̯̰̲͙̻̝f ̪̰̰̗̖̭̘͘c̦͍̲̞͍̩̙ḥ͚a̮͎̟̙͜ơ̩̹͎s̤.̝̝ ҉Z̡̖̜͖̰̣͉̜a͖̰͙̬͡l̲̫̳͍̩g̡̟̼̱͚̞̬ͅo̗͜.̟", "̦H̬̤̗̤͝e͜ ̜̥̝̻͍̟́w̕h̖̯͓o̝͙̖͎̱̮ ҉̺̙̞̟͈W̷̼̭a̺̪͍į͈͕̭͙̯̜t̶̼̮s̘͙͖̕ ̠̫̠B̻͍͙͉̳ͅe̵h̵̬͇̫͙i̹͓̳̳̮͎̫̕n͟d̴̪̜̖ ̰͉̩͇͙̲͞ͅT͖̼͓̪͢h͏͓̮̻e̬̝̟ͅ ̤̹̝W͙̞̝͔͇͝ͅa͏͓͔̹̼̣l̴͔̰̤̟͔ḽ̫.͕", "Z̮̞̠͙͔ͅḀ̗̞͈̻̗Ḷ͙͎̯̹̞͓G̻O̭̗̮", "˙ɐnbᴉlɐ ɐuƃɐɯ ǝɹolop ʇǝ ǝɹoqɐl ʇn ʇunpᴉpᴉɔuᴉ ɹodɯǝʇ poɯsnᴉǝ op pǝs 'ʇᴉlǝ ƃuᴉɔsᴉdᴉpɐ ɹnʇǝʇɔǝsuoɔ 'ʇǝɯɐ ʇᴉs ɹolop ɯnsdᴉ ɯǝɹo˥", "00˙Ɩ$-", "Ｔｈｅ ｑｕｉｃｋ ｂｒｏｗｎ ｆｏｘ ｊｕｍｐｓ ｏｖｅｒ ｔｈｅ ｌａｚｙ ｄｏｇ", "𝐓𝐡𝐞 𝐪𝐮𝐢𝐜𝐤 𝐛𝐫𝐨𝐰𝐧 𝐟𝐨𝐱 𝐣𝐮𝐦𝐩𝐬 𝐨𝐯𝐞𝐫 𝐭𝐡𝐞 𝐥𝐚𝐳𝐲 𝐝𝐨𝐠", "𝕿𝖍𝖊 𝖖𝖚𝖎𝖈𝖐 𝖇𝖗𝖔𝖜𝖓 𝖋𝖔𝖝 𝖏𝖚𝖒𝖕𝖘 𝖔𝖛𝖊𝖗 𝖙𝖍𝖊 𝖑𝖆𝖟𝖞 𝖉𝖔𝖌", "𝑻𝒉𝒆 𝒒𝒖𝒊𝒄𝒌 𝒃𝒓𝒐𝒘𝒏 𝒇𝒐𝒙 𝒋𝒖𝒎𝒑𝒔 𝒐𝒗𝒆𝒓 𝒕𝒉𝒆 𝒍𝒂𝒛𝒚 𝒅𝒐𝒈", "𝓣𝓱𝓮 𝓺𝓾𝓲𝓬𝓴 𝓫𝓻𝓸𝔀𝓷 𝓯𝓸𝔁 𝓳𝓾𝓶𝓹𝓼 𝓸𝓿𝓮𝓻 𝓽𝓱𝓮 𝓵𝓪𝔃𝔂 𝓭𝓸𝓰", "𝕋𝕙𝕖 𝕢𝕦𝕚𝕔𝕜 𝕓𝕣𝕠𝕨𝕟 𝕗𝕠𝕩 𝕛𝕦𝕞𝕡𝕤 𝕠𝕧𝕖𝕣 𝕥𝕙𝕖 𝕝𝕒𝕫𝕪 𝕕𝕠𝕘", "𝚃𝚑𝚎 𝚚𝚞𝚒𝚌𝚔 𝚋𝚛𝚘𝚠𝚗 𝚏𝚘𝚡 𝚓𝚞𝚖𝚙𝚜 𝚘𝚟𝚎𝚛 𝚝𝚑𝚎 𝚕𝚊𝚣𝚢 𝚍𝚘𝚐", "⒯⒣⒠ ⒬⒰⒤⒞⒦ ⒝⒭⒪⒲⒩ ⒡⒪⒳ ⒥⒰⒨⒫⒮ ⒪⒱⒠⒭ ⒯⒣⒠ ⒧⒜⒵⒴ ⒟⒪⒢", "", "<script>alert('123');</script>", "", " ", "\">", "'>", ">", "", "< / script >< script >alert(123)< / script >", " onfocus=JaVaSCript:alert(123) autofocus ", "\" onfocus=JaVaSCript:alert(123) autofocus ", "' onfocus=JaVaSCript:alert(123) autofocus ", "＜script＞alert(123)＜/script＞", "ript>alert(123)ript>", "-->", "\";alert(123);t=\"", "';alert(123);t='", "JavaSCript:alert(123)", ";alert(123);", "src=JaVaSCript:prompt(132)", "\"><\\x3Cscript>javascript:alert(1) ", "'`\"><\\x00script>javascript:alert(1)", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "ABC
DEF", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "test", "`\"'>", "`\"'>", "`\"'>", "`\"'>", "`\"'>", "`\"'>", "`\"'>", "`\"'>", "`\"'>", "`\"'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "\"`'>", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "XXX", "javascript:alert(1)\"` `>", "", "", "<a href=http://foo.bar/#x=`y></a><img alt=\"`><img src=x:x onerror=javascript:alert(1)></a>\">", "<!--[if]><script>javascript:alert(1)</script -->", "<!--[if<img src=x onerror=javascript:alert(1)//]> -->", "<script src=\"/\\%(jscript)s\"></script>", "<script src=\"\\\\%(jscript)s\"></script>", "<IMG \"\"\"><SCRIPT>alert(\"XSS\")</SCRIPT>\">", "<IMG SRC=javascript:alert(String.fromCharCode(88,83,83))>", "<IMG SRC=# onmouseover=\"alert('xxs')\">", "<IMG SRC= onmouseover=\"alert('xxs')\">", "<IMG onmouseover=\"alert('xxs')\">", "<IMG SRC=javascript:alert('XSS')>", "<IMG SRC=javascript:alert('XSS')>", "<IMG SRC=javascript:alert('XSS')>", "<IMG SRC=\"jav ascript:alert('XSS');\">", "<IMG SRC=\"jav ascript:alert('XSS');\">", "<IMG SRC=\"jav ascript:alert('XSS');\">", "<IMG SRC=\"jav ascript:alert('XSS');\">", "perl -e 'print \"<IMG SRC=java\\0script:alert(\\\"XSS\\\")>\";' > out", "<IMG SRC=\"  javascript:alert('XSS');\">", "<SCRIPT/XSS SRC=\"http://ha.ckers.org/xss.js\"></SCRIPT>", "<BODY onload!#$%&()*~+-_.,:;?@[/|\\]^`=alert(\"XSS\")>", "<SCRIPT/SRC=\"http://ha.ckers.org/xss.js\"></SCRIPT>", "<<SCRIPT>alert(\"XSS\");//<</SCRIPT>", "<SCRIPT SRC=http://ha.ckers.org/xss.js?< B >", "<SCRIPT SRC=//ha.ckers.org/.j>", "<IMG SRC=\"javascript:alert('XSS')\"", "<iframe src=http://ha.ckers.org/scriptlet.html <", "\\\";alert('XSS');//", "<plaintext>", "1;DROP TABLE users", "1'; DROP TABLE users-- 1", "' OR 1=1 -- 1", "' OR '1'='1", "-", "--", "--version", "--help", "$USER", "/dev/null; touch /tmp/blns.fail ; echo", "`touch /tmp/blns.fail`", "$(touch /tmp/blns.fail)", "@{[system \"touch /tmp/blns.fail\"]}", "eval(\"puts 'hello world'\")", "System(\"ls -al /\")", "`ls -al /`", "Kernel.exec(\"ls -al /\")", "Kernel.exit(1)", "%x('ls -al /')", "<?xml version=\"1.0\" encoding=\"ISO-8859-1\"?><!DOCTYPE foo [ <!ELEMENT foo ANY ><!ENTITY xxe SYSTEM \"file:///etc/passwd\" >]><foo>&xxe;</foo>", "$HOME", "$ENV{'HOME'}", "%d", "%s", "%*.*s", "../../../../../../../../../../../etc/passwd%00", "../../../../../../../../../../../etc/hosts", "() { 0; }; touch /tmp/blns.shellshock1.fail;", "() { _; } >_[$($())] { touch /tmp/blns.shellshock2.fail; }", "CON", "PRN", "AUX", "CLOCK$", "NUL", "A:", "ZZ:", "COM1", "LPT1", "LPT2", "LPT3", "COM2", "COM3", "COM4", "Scunthorpe General Hospital", "Penistone Community Church", "Lightwater Country Park", "Jimmy Clitheroe", "Horniman Museum", "shitake mushrooms", "RomansInSussex.co.uk", "http://www.cum.qc.ca/", "Craig Cockburn, Software Specialist", "Linda Callahan", "Dr. Herman I. Libshitz", "magna cum laude", "Super Bowl XXX", "medieval erection of parapets", "evaluate", "mocha", "expression", "Arsenal canal", "classic", "Tyson Gay", "If you're reading this, you've been in a coma for almost 20 years now. We're trying a new technique. We don't know where this message will end up in your dream, but we hope it works. Please wake up, we miss you.", "Roses are \u001b[0;31mred\u001b[0m, violets are \u001b[0;34mblue. Hope you enjoy terminal hue", "But now...\u001b[20Cfor my greatest trick...\u001b[8m", "The quic\b\b\b\b\b\bk brown fo\u0007\u0007\u0007\u0007\u0007\u0007\u0007\u0007\u0007\u0007\u0007x... [Beeeep]", "Powerلُلُصّبُلُلصّبُررً ॣ ॣh ॣ ॣ冗" ] rev: 6 }] + @version = 42 + @ranges = [] jobs = for doc in @docs do (doc) => (callback) => - DocstoreClient.createDoc @project_id, doc._id, doc.lines, (err)=> + DocstoreClient.createDoc @project_id, doc._id, doc.lines, @version, @ranges, (err)=> doc.lines[0] = doc.lines[0]+" added" - DocstoreClient.updateDoc @project_id, doc._id, doc.lines, 42, callback + DocstoreClient.updateDoc @project_id, doc._id, doc.lines, @version, @ranges, callback async.series jobs, done afterEach (done) -> db.docs.remove({project_id: @project_id}, done) - describe "Archiving all docs", -> beforeEach (done) -> @@ -101,18 +102,17 @@ describe "Archiving all docs", -> callback() async.series jobs, done - - - describe "archiving massive document", (done)-> + describe "Archiving a large document", (done)-> beforeEach (done)-> @timeout 1000 * 30 quarterMegInBytes = 250000 lines = require("crypto").randomBytes(quarterMegInBytes).toString("hex") @docs[1].lines = [lines,lines,lines,lines] - DocstoreClient.updateDoc @project_id, @docs[1]._id, @docs[1].lines, 42, => + @version = 42 + @ranges = [] + DocstoreClient.updateDoc @project_id, @docs[1]._id, @docs[1].lines, @version, @ranges, () => DocstoreClient.archiveAllDoc @project_id, (error, @res) => done() - it "should archive all the docs", (done) -> @res.statusCode.should.equal 204 @@ -129,7 +129,6 @@ describe "Archiving all docs", -> async.series jobs, done it "should be able get the same docs back", (done) -> - jobs = for archiveDoc in @docs do (archiveDoc) => (callback) => @@ -138,8 +137,7 @@ describe "Archiving all docs", -> callback() async.series jobs, done - describe "Unarchiving all docs", -> - + describe "Unarchiving", -> it "should unarchive all the docs", (done) -> DocstoreClient.archiveAllDoc @project_id, (error, res) => DocstoreClient.getAllDocs @project_id, (error, res, docs) => diff --git a/services/docstore/test/acceptance/coffee/DeletingDocsTests.coffee b/services/docstore/test/acceptance/coffee/DeletingDocsTests.coffee index d2d5a05727..82dbfb4c66 100644 --- a/services/docstore/test/acceptance/coffee/DeletingDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/DeletingDocsTests.coffee @@ -11,7 +11,8 @@ describe "Deleting a doc", -> @doc_id = ObjectId() @lines = ["original", "lines"] @version = 42 - DocstoreClient.createDoc @project_id, @doc_id, @lines, (error) => + @ranges = [] + DocstoreClient.createDoc @project_id, @doc_id, @lines, @version, @ranges, (error) => throw error if error? done() diff --git a/services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee b/services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee index 6a9c164001..33128c5da2 100644 --- a/services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee @@ -22,12 +22,14 @@ describe "Getting all docs", -> lines: ["111", "222", "333"] rev: 6 }] + version = 42 + ranges = 42 jobs = for doc in @docs do (doc) => (callback) => - DocstoreClient.createDoc @project_id, doc._id, doc.lines, (err)=> + DocstoreClient.createDoc @project_id, doc._id, doc.lines, version, ranges, (err)=> doc.lines[0] = doc.lines[0]+" added" - DocstoreClient.updateDoc @project_id, doc._id, doc.lines, 42, callback + DocstoreClient.updateDoc @project_id, doc._id, doc.lines, version, ranges, callback async.series jobs, done it "should return all the docs", (done) -> diff --git a/services/docstore/test/acceptance/coffee/GettingDocsTests.coffee b/services/docstore/test/acceptance/coffee/GettingDocsTests.coffee index 345f29f89f..b17a294f24 100644 --- a/services/docstore/test/acceptance/coffee/GettingDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/GettingDocsTests.coffee @@ -10,7 +10,17 @@ describe "Getting a doc", -> @project_id = ObjectId() @doc_id = ObjectId() @lines = ["original", "lines"] - DocstoreClient.createDoc @project_id, @doc_id, @lines, (error) => + @version = 42 + @ranges = { + changes: [{ + id: ObjectId().toString() + op: { i: "foo", p: 3 } + meta: + user_id: ObjectId().toString() + ts: new Date().toString() + }] + } + DocstoreClient.createDoc @project_id, @doc_id, @lines, @version, @ranges, (error) => throw error if error? done() @@ -18,6 +28,8 @@ describe "Getting a doc", -> it "should get the doc lines and version", (done) -> DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) => doc.lines.should.deep.equal @lines + doc.version.should.equal @version + doc.ranges.should.deep.equal @ranges done() describe "when the doc does not exist", -> @@ -30,11 +42,15 @@ describe "Getting a doc", -> describe "when the doc is a deleted doc", -> beforeEach (done) -> @deleted_doc_id = ObjectId() - DocstoreClient.createDeletedDoc @project_id, @deleted_doc_id, @lines, done + DocstoreClient.createDoc @project_id, @deleted_doc_id, @lines, @version, @ranges, (error) => + throw error if error? + DocstoreClient.deleteDoc @project_id, @deleted_doc_id, done it "should return the doc", (done) -> DocstoreClient.getDoc @project_id, @deleted_doc_id, {include_deleted:true},(error, res, doc) => doc.lines.should.deep.equal @lines + doc.version.should.equal @version + doc.ranges.should.deep.equal @ranges doc.deleted.should.equal true done() diff --git a/services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee b/services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee index 0fdf3ae0b2..0dd60baa05 100644 --- a/services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee @@ -11,33 +11,32 @@ describe "Applying updates to a doc", -> @doc_id = ObjectId() @originalLines = ["original", "lines"] @newLines = ["new", "lines"] + @originalRanges = { + changes: [{ + id: ObjectId().toString() + op: { i: "foo", p: 3 } + meta: + user_id: ObjectId().toString() + ts: new Date().toString() + }] + } + @newRanges = { + changes: [{ + id: ObjectId().toString() + op: { i: "bar", p: 6 } + meta: + user_id: ObjectId().toString() + ts: new Date().toString() + }] + } @version = 42 - DocstoreClient.createDoc @project_id, @doc_id, @originalLines, (error) => + DocstoreClient.createDoc @project_id, @doc_id, @originalLines, @version, @originalRanges, (error) => throw error if error? - DocstoreClient.setDocVersion @doc_id, @version, (error) => - throw error if error? - done() + done() - describe "when the lines have changed", -> + describe "when nothing has been updated", -> beforeEach (done) -> - DocstoreClient.updateDoc @project_id, @doc_id, @newLines, @version, (error, res, @body) => - done() - - it "should return modified = true", -> - @body.modified.should.equal true - - it "should return the rev", -> - @body.rev.should.equal 2 - - it "should update the doc in the API but not change the version", (done) -> - DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) => - doc.lines.should.deep.equal @newLines - doc.version.should.equal @version - done() - - describe "when the lines and version have not been updated", -> - beforeEach (done) -> - DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, @version, (error, res, @body) => + DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, @version, @originalRanges, (error, res, @body) => done() it "should return modified = false", -> @@ -46,12 +45,68 @@ describe "Applying updates to a doc", -> it "should not update the doc in the API", (done) -> DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) => doc.lines.should.deep.equal @originalLines + doc.version.should.equal @version + doc.ranges.should.deep.equal @originalRanges + done() + + describe "when the lines have changed", -> + beforeEach (done) -> + DocstoreClient.updateDoc @project_id, @doc_id, @newLines, @version, @originalRanges, (error, res, @body) => + done() + + it "should return modified = true", -> + @body.modified.should.equal true + + it "should return the rev", -> + @body.rev.should.equal 2 + + it "should update the doc in the API", (done) -> + DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) => + doc.lines.should.deep.equal @newLines + doc.version.should.equal @version + doc.ranges.should.deep.equal @originalRanges + done() + + describe "when the version has changed", -> + beforeEach (done) -> + DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, @version + 1, @originalRanges, (error, res, @body) => + done() + + it "should return modified = true", -> + @body.modified.should.equal true + + it "should return the rev", -> + @body.rev.should.equal 2 + + it "should update the doc in the API", (done) -> + DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) => + doc.lines.should.deep.equal @originalLines + doc.version.should.equal @version + 1 + doc.ranges.should.deep.equal @originalRanges + done() + + describe "when the ranges have changed", -> + beforeEach (done) -> + DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, @version, @newRanges, (error, res, @body) => + done() + + it "should return modified = true", -> + @body.modified.should.equal true + + it "should return the rev", -> + @body.rev.should.equal 2 + + it "should update the doc in the API", (done) -> + DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) => + doc.lines.should.deep.equal @originalLines + doc.version.should.equal @version + doc.ranges.should.deep.equal @newRanges done() describe "when the doc does not exist", -> beforeEach (done) -> @missing_doc_id = ObjectId() - DocstoreClient.updateDoc @project_id, @missing_doc_id, @originalLines, 0, (error, @res, @body) => + DocstoreClient.updateDoc @project_id, @missing_doc_id, @originalLines, 0, @originalRanges, (error, @res, @body) => done() it "should create the doc", -> @@ -61,12 +116,13 @@ describe "Applying updates to a doc", -> DocstoreClient.getDoc @project_id, @missing_doc_id, {}, (error, res, doc) => doc.lines.should.deep.equal @originalLines doc.version.should.equal 0 + doc.ranges.should.deep.equal @originalRanges done() describe "when malformed doc lines are provided", -> describe "when the lines are not an array", -> beforeEach (done) -> - DocstoreClient.updateDoc @project_id, @doc_id, { foo: "bar" }, @version, (error, @res, @body) => + DocstoreClient.updateDoc @project_id, @doc_id, { foo: "bar" }, @version, @originalRanges, (error, @res, @body) => done() it "should return 400", -> @@ -79,7 +135,7 @@ describe "Applying updates to a doc", -> describe "when the lines are not present", -> beforeEach (done) -> - DocstoreClient.updateDoc @project_id, @doc_id, null, @version, (error, @res, @body) => + DocstoreClient.updateDoc @project_id, @doc_id, null, @version, @originalRanges, (error, @res, @body) => done() it "should return 400", -> @@ -92,7 +148,7 @@ describe "Applying updates to a doc", -> describe "when no version is provided", -> beforeEach (done) -> - DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, null, (error, @res, @body) => + DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, null, @originalRanges, (error, @res, @body) => done() it "should return 400", -> @@ -108,7 +164,7 @@ describe "Applying updates to a doc", -> beforeEach (done) -> line = new Array(1025).join("x") # 1kb @largeLines = Array.apply(null, Array(1024)).map(() -> line) # 1mb - DocstoreClient.updateDoc @project_id, @doc_id, @largeLines, @version, (error, res, @body) => + DocstoreClient.updateDoc @project_id, @doc_id, @largeLines, @version, @originalRanges, (error, res, @body) => done() it "should return modified = true", -> @@ -118,21 +174,3 @@ describe "Applying updates to a doc", -> DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) => doc.lines.should.deep.equal @largeLines done() - - describe "when the version has changed", -> - beforeEach (done) -> - DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, @version + 1, (error, res, @body) => - done() - - it "should return modified = true", -> - @body.modified.should.equal true - - it "should return the rev", -> - @body.rev.should.equal 2 - - it "should update the doc in the API", (done) -> - DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) => - doc.lines.should.deep.equal @originalLines - doc.version.should.equal @version + 1 - done() - diff --git a/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee b/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee index 6c12628826..b4ccdcb418 100644 --- a/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee +++ b/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee @@ -5,19 +5,8 @@ DocArchiveManager = require("../../../../app/js/DocArchiveManager.js") module.exports = DocstoreClient = - createDoc: (project_id, doc_id, lines, callback = (error) ->) -> - db.docs.save({_id: doc_id, project_id:project_id, lines: lines, rev:1}, callback) - - setDocVersion: (doc_id, version, callback = (error) ->) -> - db.docOps.save({doc_id: doc_id, version: version}, callback) - - createDeletedDoc: (project_id, doc_id, lines, callback = (error) ->) -> - db.docs.insert { - _id: doc_id - project_id: project_id - lines: lines - deleted: true - }, callback + createDoc: (project_id, doc_id, lines, version, ranges, callback = (error) ->) -> + DocstoreClient.updateDoc project_id, doc_id, lines, version, ranges, callback getDoc: (project_id, doc_id, qs, callback = (error, res, body) ->) -> request.get { @@ -32,12 +21,13 @@ module.exports = DocstoreClient = json: true }, callback - updateDoc: (project_id, doc_id, lines, version, callback = (error, res, body) ->) -> + updateDoc: (project_id, doc_id, lines, version, ranges, callback = (error, res, body) ->) -> request.post { url: "http://localhost:#{settings.internal.docstore.port}/project/#{project_id}/doc/#{doc_id}" json: lines: lines version: version + ranges: ranges }, callback deleteDoc: (project_id, doc_id, callback = (error, res, body) ->) -> diff --git a/services/docstore/test/unit/coffee/DocManagerTests.coffee b/services/docstore/test/unit/coffee/DocManagerTests.coffee index 95cda2b95c..f88ca914b4 100644 --- a/services/docstore/test/unit/coffee/DocManagerTests.coffee +++ b/services/docstore/test/unit/coffee/DocManagerTests.coffee @@ -12,6 +12,10 @@ describe "DocManager", -> @DocManager = SandboxedModule.require modulePath, requires: "./MongoManager": @MongoManager = {} "./DocArchiveManager": @DocArchiveManager = {} + "./RangeManager": @RangeManager = { + jsonRangesToMongo: (r) -> r + shouldUpdateRanges: sinon.stub().returns false + } "logger-sharelatex": @logger = log: sinon.stub() warn:-> @@ -160,17 +164,35 @@ describe "DocManager", -> beforeEach -> @oldDocLines = ["old", "doc", "lines"] @newDocLines = ["new", "doc", "lines"] + @originalRanges = { + changes: [{ + id: ObjectId().toString() + op: { i: "foo", p: 3 } + meta: + user_id: ObjectId().toString() + ts: new Date().toString() + }] + } + @newRanges = { + changes: [{ + id: ObjectId().toString() + op: { i: "bar", p: 6 } + meta: + user_id: ObjectId().toString() + ts: new Date().toString() + }] + } @version = 42 - @doc = { _id: @doc_id, project_id: @project_id, lines: @oldDocLines, rev: @rev = 5, version: @version } + @doc = { _id: @doc_id, project_id: @project_id, lines: @oldDocLines, rev: @rev = 5, version: @version, ranges: @originalRanges } @MongoManager.upsertIntoDocCollection = sinon.stub().callsArg(3) @MongoManager.setDocVersion = sinon.stub().yields() @DocManager.getDoc = sinon.stub() - describe "when the doc lines have changed", -> + describe "when only the doc lines have changed", -> beforeEach -> @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) - @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback + @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @originalRanges, @callback it "should get the existing doc", -> @DocManager.getDoc @@ -182,61 +204,84 @@ describe "DocManager", -> .calledWith(@project_id, @doc_id, {lines: @newDocLines}) .should.equal true - it "should update the version", -> - @MongoManager.setDocVersion - .calledWith(@doc_id, @version) - .should.equal true - - it "should log out the old and new doc lines", -> - @logger.log - .calledWith( - project_id: @project_id - doc_id: @doc_id - oldDocLines: @oldDocLines - newDocLines: @newDocLines - rev: @doc.rev - oldVersion: @version - newVersion: @version - "updating doc lines" - ) - .should.equal true + it "should not update the version", -> + @MongoManager.setDocVersion.called.should.equal false it "should return the callback with the new rev", -> @callback.calledWith(null, true, @rev + 1).should.equal true - describe "when the version has changed", -> + describe "when the doc ranges have changed", -> beforeEach -> @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) - @DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version + 1, @callback + @RangeManager.shouldUpdateRanges.returns true + @DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version, @newRanges, @callback + + it "should get the existing doc", -> + @DocManager.getDoc + .calledWith(@project_id, @doc_id) + .should.equal true + + it "should upsert the ranges", -> + @MongoManager.upsertIntoDocCollection + .calledWith(@project_id, @doc_id, {ranges: @newRanges}) + .should.equal true + + it "should not update the version", -> + @MongoManager.setDocVersion.called.should.equal false + + it "should return the callback with the new rev", -> + @callback.calledWith(null, true, @rev + 1).should.equal true + + describe "when only the version has changed", -> + beforeEach -> + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) + @DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version + 1, @originalRanges, @callback it "should get the existing doc with the version", -> @DocManager.getDoc .calledWith(@project_id, @doc_id, {version: true}) .should.equal true - it "should upsert the document to the doc collection", -> - @MongoManager.upsertIntoDocCollection - .calledWith(@project_id, @doc_id, {lines: @oldDocLines}) - .should.equal true + it "should not change the lines or ranges", -> + @MongoManager.upsertIntoDocCollection.called.should.equal false it "should update the version", -> @MongoManager.setDocVersion .calledWith(@doc_id, @version + 1) .should.equal true - it "should return the callback with the new rev", -> - @callback.calledWith(null, true, @rev + 1).should.equal true + it "should return the callback with the old rev", -> + @callback.calledWith(null, true, @rev).should.equal true + + describe "when the doc has not changed at all", -> + beforeEach -> + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) + @DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version, @originalRanges, @callback + + it "should get the existing doc", -> + @DocManager.getDoc + .calledWith(@project_id, @doc_id) + .should.equal true + + it "should not update the ranges or lines", -> + @MongoManager.upsertIntoDocCollection.called.should.equal false + + it "should not update the version", -> + @MongoManager.setDocVersion.called.should.equal false + + it "should return the callback with the old rev and modified == false", -> + @callback.calledWith(null, false, @rev).should.equal true describe "when the version is null", -> beforeEach -> - @DocManager.updateDoc @project_id, @doc_id, @newDocLines, null, @callback + @DocManager.updateDoc @project_id, @doc_id, @newDocLines, null, @originalRanges, @callback it "should return an error", -> @callback.calledWith(new Error("no lines or version provided")).should.equal true describe "when the lines are null", -> beforeEach -> - @DocManager.updateDoc @project_id, @doc_id, null, @version, @callback + @DocManager.updateDoc @project_id, @doc_id, null, @version, @originalRanges, @callback it "should return an error", -> @callback.calledWith(new Error("no lines or version provided")).should.equal true @@ -245,7 +290,7 @@ describe "DocManager", -> beforeEach -> @error = new Error("doc could not be found") @DocManager.getDoc = sinon.stub().callsArgWith(3, @error, null, null) - @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback + @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @originalRanges, @callback it "should not upsert the document to the doc collection", -> @MongoManager.upsertIntoDocCollection.called.should.equal false @@ -256,7 +301,7 @@ describe "DocManager", -> describe "when the doc lines have not changed", -> beforeEach -> @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) - @DocManager.updateDoc @project_id, @doc_id, @oldDocLines.slice(), @version, @callback + @DocManager.updateDoc @project_id, @doc_id, @oldDocLines.slice(), @version, @originalRanges, @callback it "should not update the doc", -> @MongoManager.upsertIntoDocCollection.called.should.equal false @@ -264,25 +309,19 @@ describe "DocManager", -> it "should return the callback with the existing rev", -> @callback.calledWith(null, false, @rev).should.equal true - describe "when the doc lines are an empty array", -> - beforeEach -> - @doc.lines = [] - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) - @DocManager.updateDoc @project_id, @doc_id, @doc.lines, @callback - - it "should upsert the document to the doc collection", -> - @MongoManager.upsertIntoDocCollection - .calledWith(@project_id, @doc_id, {lines: @doc.lines}) - .should.equal true - describe "when the doc does not exist", -> beforeEach -> @DocManager.getDoc = sinon.stub().callsArgWith(3, null, null, null) - @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback + @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @originalRanges, @callback it "should upsert the document to the doc collection", -> @MongoManager.upsertIntoDocCollection - .calledWith(@project_id, @doc_id, {lines: @newDocLines}) + .calledWith(@project_id, @doc_id, {lines: @newDocLines, ranges: @originalRanges}) + .should.equal true + + it "should set the version", -> + @MongoManager.setDocVersion + .calledWith(@doc_id, @version) .should.equal true it "should return the callback with the new rev", -> diff --git a/services/docstore/test/unit/coffee/HttpControllerTests.coffee b/services/docstore/test/unit/coffee/HttpControllerTests.coffee index 8293eb7b40..839e960bca 100644 --- a/services/docstore/test/unit/coffee/HttpControllerTests.coffee +++ b/services/docstore/test/unit/coffee/HttpControllerTests.coffee @@ -196,12 +196,13 @@ describe "HttpController", -> @req.body = lines: @lines = ["hello", "world"] version: @version = 42 + ranges: @ranges = { changes: "mock" } @DocManager.updateDoc = sinon.stub().yields(null, true, @rev = 5) @HttpController.updateDoc @req, @res, @next it "should update the document", -> @DocManager.updateDoc - .calledWith(@project_id, @doc_id, @lines, @version) + .calledWith(@project_id, @doc_id, @lines, @version, @ranges) .should.equal true it "should return a modified status", ->