From ffd1aa0ca46dea2ef7f4a02d370d2f7186bc64a0 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Fri, 10 Mar 2017 11:45:03 +0000 Subject: [PATCH 1/2] just get doc_id for quick check to see if it exists --- services/docstore/app/coffee/DocManager.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/docstore/app/coffee/DocManager.coffee b/services/docstore/app/coffee/DocManager.coffee index 15e14d5428..a794864e45 100644 --- a/services/docstore/app/coffee/DocManager.coffee +++ b/services/docstore/app/coffee/DocManager.coffee @@ -101,7 +101,7 @@ module.exports = DocManager = callback null, modified, rev deleteDoc: (project_id, doc_id, callback = (error) ->) -> - DocManager.getDoc project_id, doc_id, { version: false }, (error, doc) -> + DocManager.getDoc project_id, doc_id, { _id: true }, (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 From c0e23df82b55f14ec37a6e0948cb1595c94e8b61 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 15 Mar 2017 22:09:56 +0000 Subject: [PATCH 2/2] break getDoc into multiple helpers, stop passing filters though from other calls --- .../docstore/app/coffee/DocManager.coffee | 36 +++++- .../docstore/app/coffee/HttpController.coffee | 4 +- .../acceptance/coffee/ArchiveDocsTests.coffee | 6 +- .../test/unit/coffee/DocManagerTests.coffee | 120 ++++++++++++++---- .../unit/coffee/HttpControllerTests.coffee | 16 +-- 5 files changed, 139 insertions(+), 43 deletions(-) diff --git a/services/docstore/app/coffee/DocManager.coffee b/services/docstore/app/coffee/DocManager.coffee index a794864e45..d9f0bd6747 100644 --- a/services/docstore/app/coffee/DocManager.coffee +++ b/services/docstore/app/coffee/DocManager.coffee @@ -6,13 +6,16 @@ DocArchive = require "./DocArchiveManager" RangeManager = require "./RangeManager" module.exports = DocManager = + + # TODO: For historical reasons, the doc version is currently stored in the docOps # collection (which is all that this collection contains). In future, we should # migrate this version property to be part of the docs collection, to guarantee # consitency between lines and version when writing/reading, and for a simpler schema. - getDoc: (project_id, doc_id, filter = { version: false, inS3:true}, callback = (error, doc) ->) -> - if filter? and !filter.inS3? - filter.inS3 = true + _getDoc: (project_id, doc_id, filter = {}, callback = (error, doc) ->) -> + if filter.inS3 != true + return callback("must include inS3 when getting doc") + MongoManager.findDoc project_id, doc_id, filter, (err, doc)-> if err? return callback(err) @@ -23,7 +26,7 @@ module.exports = DocManager = if err? logger.err err:err, project_id:project_id, doc_id:doc_id, "error unarchiving doc" return callback(err) - DocManager.getDoc project_id, doc_id, filter, callback + DocManager._getDoc project_id, doc_id, filter, callback else if filter.version MongoManager.getDocVersion doc_id, (error, version) -> @@ -33,6 +36,25 @@ module.exports = DocManager = else callback err, doc + checkDocExists: (project_id, doc_id, callback = (err, exists)->)-> + DocManager._getDoc project_id, doc_id, {_id:1, inS3:true}, (err, doc)-> + if err? + return callback(err) + callback(err, doc?) + + getFullDoc: (project_id, doc_id, callback = (err, doc)->)-> + DocManager._getDoc project_id, doc_id, {lines: true, rev: true, deleted: true, version: true, ranges: true, inS3:true}, (err, doc)-> + if err? + return callback(err) + callback(err, doc) + + + getDocLines: (project_id, doc_id, callback = (err, doc)->)-> + DocManager._getDoc project_id, doc_id, {lines:true, inS3:true}, (err, doc)-> + if err? + return callback(err) + callback(err, doc) + getAllNonDeletedDocs: (project_id, filter, callback = (error, docs) ->) -> DocArchive.unArchiveAllDocs project_id, (error) -> if error? @@ -49,7 +71,7 @@ module.exports = DocManager = if !lines? or !version? or !ranges? return callback(new Error("no lines, version or ranges provided")) - DocManager.getDoc project_id, doc_id, {version: true, rev: true, lines: true, version: true, ranges: true}, (err, doc)-> + DocManager._getDoc project_id, doc_id, {version: true, rev: true, lines: true, version: true, ranges: true, inS3:true}, (err, doc)-> 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) @@ -101,8 +123,8 @@ module.exports = DocManager = callback null, modified, rev deleteDoc: (project_id, doc_id, callback = (error) ->) -> - DocManager.getDoc project_id, doc_id, { _id: true }, (error, doc) -> + DocManager.checkDocExists project_id, doc_id, (error, exists) -> return callback(error) if error? - return callback new Errors.NotFoundError("No such project/doc to delete: #{project_id}/#{doc_id}") if !doc? + return callback new Errors.NotFoundError("No such project/doc to delete: #{project_id}/#{doc_id}") if !exists MongoManager.markDocAsDeleted project_id, doc_id, callback diff --git a/services/docstore/app/coffee/HttpController.coffee b/services/docstore/app/coffee/HttpController.coffee index 7c8dd0aecf..75298580b3 100644 --- a/services/docstore/app/coffee/HttpController.coffee +++ b/services/docstore/app/coffee/HttpController.coffee @@ -10,7 +10,7 @@ module.exports = HttpController = doc_id = req.params.doc_id include_deleted = req.query?.include_deleted == "true" logger.log project_id: project_id, doc_id: doc_id, "getting doc" - DocManager.getDoc project_id, doc_id, {lines: true, rev: true, deleted: true, version: true, ranges: true}, (error, doc) -> + DocManager.getFullDoc project_id, doc_id, (error, doc) -> return next(error) if error? logger.log doc: doc, "got doc" if !doc? @@ -24,7 +24,7 @@ module.exports = HttpController = project_id = req.params.project_id doc_id = req.params.doc_id logger.log project_id: project_id, doc_id: doc_id, "getting raw doc" - DocManager.getDoc project_id, doc_id, {lines: true}, (error, doc) -> + DocManager.getDocLines project_id, doc_id, (error, doc) -> return next(error) if error? if !doc? res.send 404 diff --git a/services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee b/services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee index a7616fcd86..26c627c085 100644 --- a/services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee @@ -16,15 +16,17 @@ describe "Archiving", -> @docs = [{ _id: ObjectId() lines: ["one", "two", "three"] + version:@version rev: 2 }, { _id: ObjectId() lines: ["aaa", "bbb", "ccc"] rev: 4 + version:93 }, { _id: ObjectId() 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 ॣ ॣ冗" ] - version:@version + version:313 }] @ranges = [] jobs = for doc in @docs @@ -152,7 +154,7 @@ describe "Archiving", -> done() describe "Unarchiving automatically", -> - it "should unarchive the docs ", (done) -> + it "should unarchive the docs", (done) -> DocstoreClient.archiveAllDoc @project_id, (error, res) => DocstoreClient.getDoc @project_id, @docs[0]._id, {}, (error, res, doc) => doc.lines.should.deep.equal @docs[0].lines diff --git a/services/docstore/test/unit/coffee/DocManagerTests.coffee b/services/docstore/test/unit/coffee/DocManagerTests.coffee index c07e6a4e4a..082badd0f3 100644 --- a/services/docstore/test/unit/coffee/DocManagerTests.coffee +++ b/services/docstore/test/unit/coffee/DocManagerTests.coffee @@ -1,6 +1,7 @@ SandboxedModule = require('sandboxed-module') sinon = require('sinon') chai = require('chai') +assert = require("chai").assert chai.should() expect = chai.expect modulePath = require('path').join __dirname, '../../../app/js/DocManager' @@ -26,6 +27,72 @@ describe "DocManager", -> @callback = sinon.stub() @stubbedError = new Error("blew up") + + describe "checkDocExists", -> + beforeEach -> + @DocManager._getDoc = sinon.stub() + + it "should call get doc with a quick filter", (done)-> + @DocManager._getDoc.callsArgWith(3, null, {_id:@doc_id}) + @DocManager.checkDocExists @project_id, @doc_id, (err, exist)=> + exist.should.equal true + @DocManager._getDoc.calledWith(@project_id, @doc_id, {_id:1, inS3:true}).should.equal true + done() + + it "should return false when doc is not there", (done)-> + @DocManager._getDoc.callsArgWith(3, null) + @DocManager.checkDocExists @project_id, @doc_id, (err, exist)=> + exist.should.equal false + done() + + it "should return error when get doc errors", (done)-> + @DocManager._getDoc.callsArgWith(3, "error") + @DocManager.checkDocExists @project_id, @doc_id, (err, exist)=> + err.should.equal "error" + done() + + describe "getFullDoc", -> + beforeEach -> + @DocManager._getDoc = sinon.stub() + @doc = + _id: @doc_id + lines:["2134"] + + it "should call get doc with a quick filter", (done)-> + @DocManager._getDoc.callsArgWith(3, null, @doc) + @DocManager.getFullDoc @project_id, @doc_id, (err, doc)=> + doc.should.equal @doc + @DocManager._getDoc.calledWith(@project_id, @doc_id, {lines: true, rev: true, deleted: true, version: true, ranges: true, inS3:true}).should.equal true + done() + + it "should return error when get doc errors", (done)-> + @DocManager._getDoc.callsArgWith(3, "error") + @DocManager.getFullDoc @project_id, @doc_id, (err, exist)=> + err.should.equal "error" + done() + + describe "getRawDoc", -> + + beforeEach -> + @DocManager._getDoc = sinon.stub() + @doc = + lines:["2134"] + + it "should call get doc with a quick filter", (done)-> + @DocManager._getDoc.callsArgWith(3, null, @doc) + @DocManager.getDocLines @project_id, @doc_id, (err, doc)=> + doc.should.equal @doc + @DocManager._getDoc.calledWith(@project_id, @doc_id, {lines: true, inS3:true}).should.equal true + done() + + it "should return error when get doc errors", (done)-> + @DocManager._getDoc.callsArgWith(3, "error") + @DocManager.getDocLines @project_id, @doc_id, (err, exist)=> + err.should.equal "error" + done() + + + describe "getDoc", -> beforeEach -> @project = { name: "mock-project" } @@ -38,20 +105,26 @@ describe "DocManager", -> beforeEach -> @MongoManager.findDoc.yields(null, @doc) - it "should always get inS3 even when filter is passed", (done)-> - @DocManager.getDoc @project_id, @doc_id, {version: true}, => - @MongoManager.findDoc.args[0][2].inS3.should.equal true + it "should error if inS3 is not set to true", (done)-> + @DocManager._getDoc @project_id, @doc_id, {inS3: false}, (err)-> + expect(err).to.exist done() it "should always get inS3 even when no filter is passed", (done)-> - @DocManager.getDoc @project_id, @doc_id, undefined, => - @MongoManager.findDoc.args[0][2].inS3.should.equal true + @DocManager._getDoc @project_id, @doc_id, undefined, (err)=> + @MongoManager.findDoc.called.should.equal false + expect(err).to.exist + done() + + it "should not error if inS3 is set to true", (done)-> + @DocManager._getDoc @project_id, @doc_id, {inS3: true}, (err)-> + expect(err).to.not.exist done() describe "when the doc is in the doc collection", -> beforeEach -> @MongoManager.findDoc.yields(null, @doc) - @DocManager.getDoc @project_id, @doc_id, {version: true}, @callback + @DocManager._getDoc @project_id, @doc_id, {version: true, inS3:true}, @callback it "should get the doc from the doc collection", -> @MongoManager.findDoc @@ -72,7 +145,7 @@ describe "DocManager", -> describe "without the version filter", -> beforeEach -> @MongoManager.findDoc.yields(null, @doc) - @DocManager.getDoc @project_id, @doc_id, {version: false}, @callback + @DocManager._getDoc @project_id, @doc_id, {version: false, inS3:true}, @callback it "should not get the doc version from the docOps collection", -> @MongoManager.getDocVersion.called.should.equal false @@ -80,7 +153,7 @@ describe "DocManager", -> describe "when MongoManager.findDoc errors", -> beforeEach -> @MongoManager.findDoc.yields(@stubbedError) - @DocManager.getDoc @project_id, @doc_id, {version: true}, @callback + @DocManager._getDoc @project_id, @doc_id, {version: true, inS3:true}, @callback it "should return the error", -> @callback.calledWith(@stubbedError).should.equal true @@ -93,7 +166,7 @@ describe "DocManager", -> @doc.inS3 = false callback() sinon.spy @DocArchiveManager, "unarchiveDoc" - @DocManager.getDoc @project_id, @doc_id, {version: true}, @callback + @DocManager._getDoc @project_id, @doc_id, {version: true, inS3:true}, @callback it "should call the DocArchive to unarchive the doc", -> @DocArchiveManager.unarchiveDoc.calledWith(@project_id, @doc_id).should.equal true @@ -107,7 +180,7 @@ describe "DocManager", -> describe "when the doc does not exist in the docs collection", -> beforeEach -> @MongoManager.findDoc = sinon.stub().yields(null, null) - @DocManager.getDoc @project_id, @doc_id, {version: true}, @callback + @DocManager._getDoc @project_id, @doc_id, {version: true, inS3:true}, @callback it "should return a NotFoundError", -> @callback @@ -147,12 +220,12 @@ describe "DocManager", -> beforeEach -> @lines = ["mock", "doc", "lines"] @rev = 77 - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, {lines: @lines, rev:@rev}) + @DocManager.checkDocExists = sinon.stub().callsArgWith(2, null, true) @MongoManager.markDocAsDeleted = sinon.stub().callsArg(2) @DocManager.deleteDoc @project_id, @doc_id, @callback it "should get the doc", -> - @DocManager.getDoc + @DocManager.checkDocExists .calledWith(@project_id, @doc_id) .should.equal true @@ -166,10 +239,9 @@ describe "DocManager", -> describe "when the doc does not exist", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, null) + @DocManager.checkDocExists = sinon.stub().callsArgWith(2, null, false) @DocManager.deleteDoc @project_id, @doc_id, @callback - it "should return a NotFoundError", -> @callback .calledWith(new Errors.NotFoundError("No such doc: #{@doc_id}")) @@ -202,16 +274,16 @@ describe "DocManager", -> @MongoManager.upsertIntoDocCollection = sinon.stub().callsArg(3) @MongoManager.setDocVersion = sinon.stub().yields() - @DocManager.getDoc = sinon.stub() + @DocManager._getDoc = sinon.stub() describe "when only the doc lines have changed", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) + @DocManager._getDoc = sinon.stub().callsArgWith(3, null, @doc) @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @originalRanges, @callback it "should get the existing doc", -> - @DocManager.getDoc - .calledWith(@project_id, @doc_id, {version: true, rev: true, lines: true, version: true, ranges: true}) + @DocManager._getDoc + .calledWith(@project_id, @doc_id, {version: true, rev: true, lines: true, version: true, ranges: true, inS3:true}) .should.equal true it "should upsert the document to the doc collection", -> @@ -227,7 +299,7 @@ describe "DocManager", -> describe "when the doc ranges have changed", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) + @DocManager._getDoc = sinon.stub().callsArgWith(3, null, @doc) @RangeManager.shouldUpdateRanges.returns true @DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version, @newRanges, @callback @@ -244,7 +316,7 @@ describe "DocManager", -> describe "when only the version has changed", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) + @DocManager._getDoc = sinon.stub().callsArgWith(3, null, @doc) @DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version + 1, @originalRanges, @callback it "should not change the lines or ranges", -> @@ -260,7 +332,7 @@ describe "DocManager", -> describe "when the doc has not changed at all", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) + @DocManager._getDoc = sinon.stub().callsArgWith(3, null, @doc) @DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version, @originalRanges, @callback it "should not update the ranges or lines", -> @@ -296,7 +368,7 @@ describe "DocManager", -> describe "when there is a generic error getting the doc", -> beforeEach -> @error = new Error("doc could not be found") - @DocManager.getDoc = sinon.stub().callsArgWith(3, @error, null, null) + @DocManager._getDoc = sinon.stub().callsArgWith(3, @error, null, null) @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @originalRanges, @callback it "should not upsert the document to the doc collection", -> @@ -307,7 +379,7 @@ describe "DocManager", -> describe "when the doc lines have not changed", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) + @DocManager._getDoc = sinon.stub().callsArgWith(3, null, @doc) @DocManager.updateDoc @project_id, @doc_id, @oldDocLines.slice(), @version, @originalRanges, @callback it "should not update the doc", -> @@ -318,7 +390,7 @@ describe "DocManager", -> describe "when the doc does not exist", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, null, null) + @DocManager._getDoc = sinon.stub().callsArgWith(3, null, null, null) @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @originalRanges, @callback it "should upsert the document to the doc collection", -> diff --git a/services/docstore/test/unit/coffee/HttpControllerTests.coffee b/services/docstore/test/unit/coffee/HttpControllerTests.coffee index 1a0752d850..0f32f4b9e3 100644 --- a/services/docstore/test/unit/coffee/HttpControllerTests.coffee +++ b/services/docstore/test/unit/coffee/HttpControllerTests.coffee @@ -40,12 +40,12 @@ describe "HttpController", -> @req.params = project_id: @project_id doc_id: @doc_id - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) + @DocManager.getFullDoc = sinon.stub().callsArgWith(2, null, @doc) @HttpController.getDoc @req, @res, @next it "should get the document with the version (including deleted)", -> - @DocManager.getDoc - .calledWith(@project_id, @doc_id, {lines: true, rev: true, deleted: true, version: true, ranges: true}) + @DocManager.getFullDoc + .calledWith(@project_id, @doc_id) .should.equal true it "should return the doc as JSON", -> @@ -63,11 +63,11 @@ describe "HttpController", -> @req.params = project_id: @project_id doc_id: @doc_id - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @deletedDoc) + @DocManager.getFullDoc = sinon.stub().callsArgWith(2, null, @deletedDoc) it "should get the doc from the doc manager", -> @HttpController.getDoc @req, @res, @next - @DocManager.getDoc.calledWith(@project_id, @doc_id, {lines: true, rev: true, deleted: true, version: true, ranges: true}).should.equal true + @DocManager.getFullDoc.calledWith(@project_id, @doc_id).should.equal true it "should return 404 if the query string delete is not set ", -> @HttpController.getDoc @req, @res, @next @@ -91,12 +91,12 @@ describe "HttpController", -> @req.params = project_id: @project_id doc_id: @doc_id - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) + @DocManager.getDocLines = sinon.stub().callsArgWith(2, null, @doc) @HttpController.getRawDoc @req, @res, @next it "should get the document without the version", -> - @DocManager.getDoc - .calledWith(@project_id, @doc_id, {lines: true}) + @DocManager.getDocLines + .calledWith(@project_id, @doc_id) .should.equal true it "should set the content type header", ->