From 3138919cb75e5bc57afef1878ff02e6f6f593d2c Mon Sep 17 00:00:00 2001 From: Simon Detheridge Date: Mon, 3 Dec 2018 11:06:05 +0000 Subject: [PATCH] Merge pull request #1184 from sharelatex/spd-zip-project-name-from-tex-content zip upload: Read project name from title in zip contents GitOrigin-RevId: 27122674a0374f86a10c04485d787f4caaf21f5b --- .../Features/Documents/DocumentHelper.coffee | 24 +++- .../Project/ProjectRootDocManager.coffee | 93 ++++++++++++--- .../Templates/TemplatesManager.coffee | 2 +- .../Uploads/ProjectUploadManager.coffee | 46 +++++++- .../coffee/ProjectStructureTests.coffee | 27 ++++- .../files/test_project_with_name.zip | Bin 0 -> 3996 bytes .../Documents/DocumentHelperTests.coffee | 17 +++ .../Project/ProjectRootDocManagerTests.coffee | 109 ++++++++++++++++++ .../Templates/TemplatesManagerTests.coffee | 4 +- .../Uploads/ProjectUploadManagerTests.coffee | 101 +++++++++++++--- 10 files changed, 376 insertions(+), 47 deletions(-) create mode 100644 services/web/test/acceptance/files/test_project_with_name.zip diff --git a/services/web/app/coffee/Features/Documents/DocumentHelper.coffee b/services/web/app/coffee/Features/Documents/DocumentHelper.coffee index 6c6ddb492d..26c1d8d7a9 100644 --- a/services/web/app/coffee/Features/Documents/DocumentHelper.coffee +++ b/services/web/app/coffee/Features/Documents/DocumentHelper.coffee @@ -3,11 +3,23 @@ module.exports = DocumentHelper = TITLE_WITH_CURLY_BRACES = /\\[tT]itle\*?\s*{([^}]+)}/ TITLE_WITH_SQUARE_BRACES = /\\[tT]itle\s*\[([^\]]+)\]/ ESCAPED_BRACES = /\\([{}\[\]])/g - content = content.substring(0, maxContentToScan).split("\n") if typeof content is 'string' - title = null - for line in content + + for line in DocumentHelper._getLinesFromContent(content, maxContentToScan) match = line.match(TITLE_WITH_SQUARE_BRACES) || line.match(TITLE_WITH_CURLY_BRACES) if match? - title = match[1].replace(ESCAPED_BRACES, (br)->br[1]) - break - return title + return match[1].replace(ESCAPED_BRACES, (br)->br[1]) + + return null + + contentHasDocumentclass: (content, maxContentToScan = 30000) -> + for line in DocumentHelper._getLinesFromContent(content, maxContentToScan) + # We've had problems with this regex locking up CPU. + # Previously /.*\\documentclass/ would totally lock up on lines of 500kb (data text files :() + # This regex will only look from the start of the line, including whitespace so will return quickly + # regardless of line length. + return true if line.match /^\s*\\documentclass/ + + return false + + _getLinesFromContent: (content, maxContentToScan) -> + return if typeof content is 'string' then content.substring(0, maxContentToScan).split("\n") else content diff --git a/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee b/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee index 201ffca56a..edc39f7436 100644 --- a/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee +++ b/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee @@ -1,31 +1,24 @@ ProjectEntityHandler = require "./ProjectEntityHandler" ProjectEntityUpdateHandler = require "./ProjectEntityUpdateHandler" ProjectGetter = require "./ProjectGetter" +DocumentHelper = require "../Documents/DocumentHelper" Path = require "path" +fs = require("fs") async = require("async") +globby = require("globby") _ = require("underscore") module.exports = ProjectRootDocManager = setRootDocAutomatically: (project_id, callback = (error) ->) -> - ProjectEntityHandler.getAllDocs project_id, (error, docs) -> return callback(error) if error? - - root_doc_id = null jobs = _.map docs, (doc, path)-> return (cb)-> - rootDocId = null - for line in doc.lines || [] - # We've had problems with this regex locking up CPU. - # Previously /.*\\documentclass/ would totally lock up on lines of 500kb (data text files :() - # This regex will only look from the start of the line, including whitespace so will return quickly - # regardless of line length. - match = /^\s*\\documentclass/.test(line) - isRootDoc = /\.R?tex$/.test(Path.extname(path)) and match - if isRootDoc - rootDocId = doc?._id - cb(rootDocId) + if /\.R?tex$/.test(Path.extname(path)) && DocumentHelper.contentHasDocumentclass(doc.lines) + cb(doc._id) + else + cb(null) async.series jobs, (root_doc_id)-> if root_doc_id? @@ -33,6 +26,48 @@ module.exports = ProjectRootDocManager = else callback() + findRootDocFileFromDirectory: (directoryPath, callback = (error, path, content) ->) -> + filePathsPromise = globby([ + '**/*.{tex,Rtex}' + ], { + cwd: directoryPath, + followSymlinkedDirectories: false, + onlyFiles: true, + case: false + } + ) + + # the search order is such that we prefer files closer to the project root, then + # we go by file size in ascending order, because people often have a main + # file that just includes a bunch of other files; then we go by name, in + # order to be deterministic + filePathsPromise.then( + (unsortedFiles) -> + ProjectRootDocManager._sortFileList unsortedFiles, directoryPath, (err, files) -> + return callback(err) if err? + doc = null + + async.until( + -> + return doc? || files.length == 0 + (cb) -> + file = files.shift() + fs.readFile Path.join(directoryPath, file), 'utf8', (error, content) -> + return cb(error) if error? + content = (content || '').replace(/\r/g, '') + if DocumentHelper.contentHasDocumentclass(content) + doc = {path: file, content: content} + cb(null) + (err) -> + callback(err, doc?.path, doc?.content) + ) + (err) -> + callback(err) + ) + + # coffeescript's implicit-return mechanism returns filePathsPromise from this method, which confuses mocha + return null + setRootDocFromName: (project_id, rootDocName, callback = (error) ->) -> ProjectEntityHandler.getAllDocPathsFromProjectById project_id, (error, docPaths) -> return callback(error) if error? @@ -88,3 +123,33 @@ module.exports = ProjectRootDocManager = ProjectRootDocManager.setRootDocAutomatically project_id, callback else ProjectRootDocManager.setRootDocAutomatically project_id, callback + + _sortFileList: (listToSort, rootDirectory, callback = (error, result)->) -> + async.mapLimit( + listToSort + 5 + (filePath, cb) -> + fs.stat Path.join(rootDirectory, filePath), (err, stat) -> + return cb(err) if err? + cb(null, + size: stat.size + path: filePath + elements: filePath.split(Path.sep).length + name: Path.basename(filePath) + ) + (err, files) -> + return callback(err) if err? + + callback(null, _.map files.sort(ProjectRootDocManager._rootDocSort), (file)-> return file.path) + ) + + _rootDocSort: (a, b) -> + # sort first by folder depth + return a.elements - b.elements if a.elements != b.elements + # ensure main.tex is at the start of each folder + return -1 if (a.name == 'main.tex' && b.name != 'main.tex') + return 1 if (a.name != 'main.tex' && b.name == 'main.tex') + # prefer smaller files + return a.size - b.size if a.size != b.size + # otherwise, use the full path name + return a.path.localeCompare(b.path) diff --git a/services/web/app/coffee/Features/Templates/TemplatesManager.coffee b/services/web/app/coffee/Features/Templates/TemplatesManager.coffee index 086b744fda..15aa302d5a 100644 --- a/services/web/app/coffee/Features/Templates/TemplatesManager.coffee +++ b/services/web/app/coffee/Features/Templates/TemplatesManager.coffee @@ -28,7 +28,7 @@ module.exports = TemplatesManager = if zipReq.response.statusCode != 200 logger.err { uri: zipUrl, statusCode: zipReq.response.statusCode }, "non-success code getting zip from template API" return callback new Error("get zip failed") - ProjectUploadManager.createProjectFromZipArchive user_id, projectName, dumpPath, (err, project) -> + ProjectUploadManager.createProjectFromZipArchiveWithName user_id, projectName, dumpPath, (err, project) -> if err? logger.err { err, zipReq }, "problem building project from zip" return callback err diff --git a/services/web/app/coffee/Features/Uploads/ProjectUploadManager.coffee b/services/web/app/coffee/Features/Uploads/ProjectUploadManager.coffee index 4a1dee2d9b..5d412005ea 100644 --- a/services/web/app/coffee/Features/Uploads/ProjectUploadManager.coffee +++ b/services/web/app/coffee/Features/Uploads/ProjectUploadManager.coffee @@ -1,13 +1,47 @@ path = require "path" rimraf = require "rimraf" +async = require "async" ArchiveManager = require "./ArchiveManager" FileSystemImportManager = require "./FileSystemImportManager" ProjectCreationHandler = require "../Project/ProjectCreationHandler" ProjectRootDocManager = require "../Project/ProjectRootDocManager" ProjectDetailsHandler = require "../Project/ProjectDetailsHandler" +DocumentHelper = require "../Documents/DocumentHelper" module.exports = ProjectUploadHandler = - createProjectFromZipArchive: (owner_id, proposedName, zipPath, callback = (error, project) ->) -> + createProjectFromZipArchive: (owner_id, defaultName, zipPath, callback = (error, project) ->) -> + destination = @_getDestinationDirectory zipPath + docPath = null + project = null + + async.waterfall([ + (cb) -> + ArchiveManager.extractZipArchive zipPath, destination, cb + (cb) -> + ProjectRootDocManager.findRootDocFileFromDirectory destination, (error, _docPath, docContents) -> + cb(error, _docPath, docContents) + (_docPath, docContents, cb) -> + docPath = _docPath + proposedName = DocumentHelper.getTitleFromTexContent(docContents || '') || defaultName + ProjectDetailsHandler.generateUniqueName owner_id, proposedName, (error, name) -> + cb(error, name) + (name, cb) -> + ProjectCreationHandler.createBlankProject owner_id, name, (error, _project) -> + cb(error, _project) + (_project, cb) => + project = _project + @_insertZipContentsIntoFolder owner_id, project._id, project.rootFolder[0]._id, destination, cb + (cb) -> + if docPath? + ProjectRootDocManager.setRootDocFromName project._id, docPath, (error) -> + cb(error) + else + cb(null) + (cb) -> + cb(null, project) + ], callback) + + createProjectFromZipArchiveWithName: (owner_id, proposedName, zipPath, callback = (error, project) ->) -> ProjectDetailsHandler.generateUniqueName owner_id, proposedName, (error, name) => return callback(error) if error? ProjectCreationHandler.createBlankProject owner_id, name, (error, project) => @@ -18,10 +52,14 @@ module.exports = ProjectUploadHandler = return callback(error) if error? callback(error, project) - insertZipArchiveIntoFolder: (owner_id, project_id, folder_id, path, callback = (error) ->) -> - destination = @_getDestinationDirectory path - ArchiveManager.extractZipArchive path, destination, (error) -> + insertZipArchiveIntoFolder: (owner_id, project_id, folder_id, zipPath, callback = (error) ->) -> + destination = @_getDestinationDirectory zipPath + ArchiveManager.extractZipArchive zipPath, destination, (error) => return callback(error) if error? + + @_insertZipContentsIntoFolder owner_id, project_id, folder_id, destination, callback + + _insertZipContentsIntoFolder: (owner_id, project_id, folder_id, destination, callback = (error) ->) -> ArchiveManager.findTopLevelDirectory destination, (error, topLevelDestination) -> return callback(error) if error? FileSystemImportManager.addFolderContents owner_id, project_id, folder_id, topLevelDestination, false, (error) -> diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index 30fbccfb8d..3ae879c229 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -125,6 +125,7 @@ describe "ProjectStructureChanges", -> MockDocUpdaterApi.clearProjectStructureUpdates() zip_file = fs.createReadStream(Path.resolve(__dirname + '/../files/test_project.zip')) + @test_project_name = 'wombat' req = @owner.request.post { uri: "project/new/upload", @@ -137,7 +138,7 @@ describe "ProjectStructureChanges", -> @uploaded_project_id = JSON.parse(body).project_id done() - it "should version the dosc created", -> + it "should version the docs created", -> {docUpdates: updates, version} = MockDocUpdaterApi.getProjectStructureUpdates(@uploaded_project_id) expect(updates.length).to.equal(1) update = updates[0] @@ -155,6 +156,30 @@ describe "ProjectStructureChanges", -> expect(update.url).to.be.a('string'); expect(version).to.equal(2) + describe "uploading a project with a name", -> + before (done) -> + MockDocUpdaterApi.clearProjectStructureUpdates() + + zip_file = fs.createReadStream(Path.resolve(__dirname + '/../files/test_project_with_name.zip')) + @test_project_name = 'wombat' + + req = @owner.request.post { + uri: "project/new/upload", + formData: + qqfile: zip_file + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to upload project #{res.statusCode}") + @uploaded_project_id = JSON.parse(body).project_id + done() + + it "should set the project name from the zip contents", (done) -> + ProjectGetter.getProject @uploaded_project_id, (error, project) => + expect(error).not.to.exist + expect(project.name).to.equal @test_project_name + done() + describe "uploading a file", -> beforeEach (done) -> MockDocUpdaterApi.clearProjectStructureUpdates() diff --git a/services/web/test/acceptance/files/test_project_with_name.zip b/services/web/test/acceptance/files/test_project_with_name.zip new file mode 100644 index 0000000000000000000000000000000000000000..90a2d78dd81ca2c561a36e0ec17aa87f4f4fdd4b GIT binary patch literal 3996 zcmZ{ncQ71`y2e*;Az}3zB}xbpJ$gxWS)z;87SWcds}sHVvZ8!swIHGkcC`pW^xnei zy~irc&3DesJ!j56ciwsHyz|WS?+>JjhffUv0EhuDu0Gm{Pat~%e>UVlF3bQID+j2c zr!CAtp9Fx1B*K_=F)*8Y!N>qO_*-}Y06yf;pFyH9Mc2~@_}6Xk=^iS+=oQiXCO9;- zl51@t5I`FZ3{E0Y>Y8-4Z!{KhMR7xpDK+3an*xR_f@_>8qSs`0NBsD?$;yaYW3T0> zzd!fypVVvQRy@4cd5lO^$Ydq0s(#5#XpIegtDwfQW1~r7$|CP88*nN~zjo=0!~^}W zdwy4cO#pAngAH;3z>xv~AO$c3gxwrqw$6fXQ2T%Q|J(b2@$X{fD@pA<&E_;nU&~W0 zOdY|X$!B9Pke2$y48q1eaL*=wai&SYN~LyyRd+UPPrqbOQ)v*yKAOgxkae-`UvPMZ zxO*=Sub&2q%BUFjO+UG$dzwKJBuR+K14$Ypl~{eCcH! z)4@m^WFr@32DNczNUcX-X>OfWM>mK2WrwKxGQ}R-Xu2G6i8-BpTIJN)9g&1+Bk0)@6jwRMPn*X1=7xR7=go}8!D^a8SrhM$tx>uoDI)4(K-^`R^=kvtv>OYf0w=jZ1|8w94T z4%dzMd!?^G$8b>;4y3YFT5=b)Hy)49?rIQy);N1|AuV*opO!y*Ik!445|F zlP+i(Hch zv<*ZPLdJHbrH9<{({gp2b`W_S;;*u_OIm6uIc-GipcWsW?14BW+vE0@{oP>_uY=3U zeq*5?_-^Z+urhu+br%l_@%WCdqc>?Dn;0!aJeI_#wz2D3b~^PT9N`vV3r=mc;-Hv; zGH4TdedNrL2gNEEe^jkRE2nrTCZ_~Qs04GRWjI|(eaC*W24TokrXv@h*WUMI#D3%| zy4Tv%(rNmhO>)&(kCkE9@MDwW^PSqyE6M~97hUZmEtRmlUBj)Z4-XVQy1w-V>Y}@0 zK7hB0KqNKGnKcyY#=~o;KIF7AwB-JDc&+P`lCH^6Uu1qUOrsTqdNT<}FItib8@mmn z%-Xc>mv%emZP)9F#YxwlJwJs^ctqj63r?zg3>VYo6d6(VIQd<=wXQDV;kxmvG7?!5 zEvYc}{7X5W)K@Bz-buTbjK2zfDqny@l@1B?mFWos=q=9MbJq#)VZSWSo*w=nc3A`a zhP(J?Q#89nXc_q5|BXp>&N1;^OfHrZEE$Vk!a?lZDl3i157>bp&Slx2;vxwaW!&O7SEJ+S8* z?|H4w98G7*Y@|K5UmefPRV4di;~ndhec-$3qCVhAhUAf+ewuL^itAOIBz|!)&Zkl7 zGV~xcHJLppt))P-!;`mqX zK~u5P(ZmGM_#AHWu5GC@9SZL$U`Fal4z;Oa$l`i(s}HeoB@=0z=mD)(+IV#WUXz%T zRZ3?wUAx6GQ}>Ab1zhMNH(U#MS08@mL-N>yHCe3HEh8e`C!-e)TRm=o#b6e|DUZ}- z##A_@19k4gn$efkbxw6xb%ew}Gb;*)BNf2KKth_l?ts4x8J`|sZ_Dew9&;K$8MiJE z5T(c=Z}>2^7FL>b-v!ca->6iI5adjV)5#uEPlRAlo^Ac;BAv97^3l|}xw(_M{DV^h z&pqXf5Pz-t_Y;OWEhJxYUA#9=d-^LgMV(v>^jp27y^s6Mo7(KvoG6Ful0>kqdx(SZ z*QC&dmqGQH*|GI4Kpj?_oAL7GQfHtG*~wvKIv zJ;2EWW!ye#yCQ>+B;w8b?^-g=(Ym$Z-&fp&5Y*F|7Af&J2SV&(6goWKJTbQ0)^S_z zy^i^*^^9r#S4>IH=AuwjZF=xC9?=EgGZQGT_vgDJ&WceTcY>ffVL_^x@jp_{pMhI=A9mI;jD#$+FF;6h8r+d;_7yd4X0RTm=n}MLuiKWcl{}#F_a0s_3yN z5kbm89L&amk6c3Dh(n|0p$`KY(aiB}$@vgL23~y^rSjK!nSxQ4$5uX|T%oDs%@u=$Y!nBHU zn739?+pP<>Pn?{Afo)jYOZ!D&MeIUQooCmV$8S&8A=8kvwIfK?rc)AsETo6;l@=Zn zXAQn;C6R`OIw9Q`ck{YhzDZ*|_D;i!dy*a$Z2xSkfX10jl)$S7&+x~~ge@7OyY^zu z6H59(Lo@lcvyCgxv!#odN6X3s9TyD8w)#vOCTRch(v6l2@~ZHSQjcwqgrDC+l0skU z^BcUtl_R<=qr#g&4FtDe*?8|?Z?$GMm9Zn+=U!9&!rv_Qfmo@}z?_|ekc*2E9(grq z3VsB63ij4BRvjV?wI@#7P^eE<7((_ zPWOvd>b#0#^k%v1@iWE?da%IiYQCkVZ?_FY5tOd=Jg-WRNlO^sKj z{^qo}dUh2~)xA6{sHREK6zgAjbTOzJ8+&rXpv+&aK>!4Pd4@kv`ySN0IJA?zk+6?2 z%7aVd(a+}O`pTbqXJuUQOg99SgHq;a&(GP{^85Q^mSUn~243GziZmA6ZM;l-g>AIA zA)XAcb2=5f{OQ?iH;+_zBZQGS4_NdCo6}bsi?lOYJ~w-;mCqAA{MC~I^@WX;1C`T! zRC01rwMh;Kw--F}g6{1li+2E9fTHV?>n=t8@u1zNz-FsWOxEc3hbK;i>IB6HDm!!$ zn?HsNh|F(v&C8Kfn#3fJ=V!9PE>%6Azegod?YC0>^=fm0$3F#S>tz)(9oxB0OR#a8 z#t=El{#HadPMR3^wYZbJAIIuGtfR+V;-d86*TECDCS6PKXISKF>hoeU#8w;$d#XyYQYiYd7`-;x0xO0Nu^Coe{< zse{F$y-?D`Pkb$AZrQEiZ~DLHv?(dBQ%n%fl~F|o4~kbi=ycM}n5yDZ8ybp36MQL` zst2R4)B`^1HylvfwD;97yse#|ni*RdXHCo_9w=fG8d;+H^h>M}N}ohWR4z9N)aooC zx#j?t(Lj_W%zB)8*P8poN0fEi`CiZ8qa@E{YMK{f`Z-+VKT=W~96ULc<`7qoq%~o( zG1|4z5;FI_xm|=9y)oLe!^$+Xpo|oJFoi~$wV#&gsG^&jOO8l>@kkP~HI5%-%PULq zae%5 zYWPQ#30MZrJv)YnN0x4OI5RhR`gxCxV#2frjfn0Rxo{Ln7)L1SfK2lpoiyUza-zdu zwsPC#ggVI$Ouj!(CiA8aZT;>^TQJT_SpCL&?|v}po@5}u4B(;LZ#8-lH`)RR5a}Eb zFRorccOvGgDaN9y#+o_6ouH`@vQyZ$FI$b3Tarox(&Bal?e( z{Y=`>2E`x!e$}iX*eTcrDRfjI#xFvz#=p#d=yiu+%hxZ2qEM3f%sO0DRw@MaQ@u2T zr3eImRz}pozyLD`<@a{S*0r`qm$U1pxjBg@vCS literal 0 HcmV?d00001 diff --git a/services/web/test/unit/coffee/Documents/DocumentHelperTests.coffee b/services/web/test/unit/coffee/Documents/DocumentHelperTests.coffee index 929669251c..16a9f2bb2e 100644 --- a/services/web/test/unit/coffee/Documents/DocumentHelperTests.coffee +++ b/services/web/test/unit/coffee/Documents/DocumentHelperTests.coffee @@ -26,3 +26,20 @@ describe "DocumentHelper", -> it "should accept an array", -> document = ["\\begin{document}","\\title{foo}","\\end{document}"] expect(@DocumentHelper.getTitleFromTexContent(document)).to.equal "foo" + + describe "contentHasDocumentclass", -> + it "should return true if the content has a documentclass", -> + document = ["% line", "% line", "% line", "\\documentclass"] + expect(@DocumentHelper.contentHasDocumentclass(document)).to.equal true + + it "should allow whitespace before the documentclass", -> + document = ["% line", "% line", "% line", " \\documentclass"] + expect(@DocumentHelper.contentHasDocumentclass(document)).to.equal true + + it "should not allow non-whitespace before the documentclass", -> + document = ["% line", "% line", "% line", " asdf \\documentclass"] + expect(@DocumentHelper.contentHasDocumentclass(document)).to.equal false + + it "should return false when there is no documentclass", -> + document = ["% line", "% line", "% line"] + expect(@DocumentHelper.contentHasDocumentclass(document)).to.equal false diff --git a/services/web/test/unit/coffee/Project/ProjectRootDocManagerTests.coffee b/services/web/test/unit/coffee/Project/ProjectRootDocManagerTests.coffee index fc8fdec7bc..0de8dce9c4 100644 --- a/services/web/test/unit/coffee/Project/ProjectRootDocManagerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectRootDocManagerTests.coffee @@ -1,5 +1,6 @@ chai = require('chai') should = chai.should() +expect = chai.expect sinon = require("sinon") modulePath = "../../../../app/js/Features/Project/ProjectRootDocManager.js" SandboxedModule = require('sandboxed-module') @@ -14,10 +15,18 @@ describe 'ProjectRootDocManager', -> "doc-id-4": "/nested/chapter1b.tex" @sl_req_id = "sl-req-id-123" @callback = sinon.stub() + @globby = sinon.stub().returns(new Promise (resolve) -> + resolve(['a.tex', 'b.tex', 'main.tex']) + ) + @fs = + readFile: sinon.stub().callsArgWith(2, new Error('file not found')) + stat: sinon.stub().callsArgWith(1, null, {size: 100}) @ProjectRootDocManager = SandboxedModule.require modulePath, requires: "./ProjectEntityHandler" : @ProjectEntityHandler = {} "./ProjectEntityUpdateHandler" : @ProjectEntityUpdateHandler = {} "./ProjectGetter" : @ProjectGetter = {} + "globby" : @globby + "fs" : @fs describe "setRootDocAutomatically", -> describe "when there is a suitable root doc", -> @@ -81,6 +90,106 @@ describe 'ProjectRootDocManager', -> it "should not set the root doc to the doc containing a documentclass", -> @ProjectEntityUpdateHandler.setRootDoc.called.should.equal false + describe "findRootDocFileFromDirectory", -> + beforeEach -> + @fs.readFile.withArgs('/foo/a.tex').callsArgWith(2, null, 'Hello World!') + @fs.readFile.withArgs('/foo/b.tex').callsArgWith(2, null, "I'm a little teapot, get me out of here.") + @fs.readFile.withArgs('/foo/main.tex').callsArgWith(2, null, "Help, I'm trapped in a unit testing factory") + @fs.readFile.withArgs('/foo/c.tex').callsArgWith(2, null, 'Tomato, tomahto.') + @fs.readFile.withArgs('/foo/a/a.tex').callsArgWith(2, null, 'Potato? Potahto. Potootee!') + @documentclassContent = "% test\n\\documentclass\n\% test" + + describe "when there is a file in a subfolder", -> + @globby = sinon.stub().returns(new Promise (resolve) -> + resolve(['c.tex', 'a.tex', 'a/a.tex', 'b.tex']) + ) + + it "processes the root folder files first, and then the subfolder, in alphabetical order", -> + @ProjectRootDocManager.findRootDocFileFromDirectory '/foo', => + expect(error).not.to.exist + expect(path).to.equal null + sinon.assert.callOrder( + @fs.readFile.withArgs('/foo/a.tex') + @fs.readFile.withArgs('/foo/b.tex') + @fs.readFile.withArgs('/foo/c.tex') + @fs.readFile.withArgs('/foo/a/a.tex') + ) + done() + + it "processes smaller files first", -> + @fs.stat.withArgs('/foo/c.tex').callsArgWith(1, null, {size: 1}) + @ProjectRootDocManager.findRootDocFileFromDirectory '/foo', => + expect(error).not.to.exist + expect(path).to.equal null + sinon.assert.callOrder( + @fs.readFile.withArgs('/foo/c.tex') + @fs.readFile.withArgs('/foo/a.tex') + @fs.readFile.withArgs('/foo/b.tex') + @fs.readFile.withArgs('/foo/a/a.tex') + ) + done() + + describe "when main.tex contains a documentclass", -> + beforeEach -> + @fs.readFile.withArgs('/foo/main.tex').callsArgWith(2, null, @documentclassContent) + + it "returns main.tex", (done) -> + @ProjectRootDocManager.findRootDocFileFromDirectory '/foo', (error, path, content) => + expect(error).not.to.exist + expect(path).to.equal 'main.tex' + expect(content).to.equal @documentclassContent + done() + + it "processes main.text first and stops processing when it finds the content", (done) -> + @ProjectRootDocManager.findRootDocFileFromDirectory '/foo', => + expect(@fs.readFile).to.be.calledWith('/foo/main.tex') + expect(@fs.readFile).not.to.be.calledWith('/foo/a.tex') + done() + + describe "when a.tex contains a documentclass", -> + beforeEach -> + @fs.readFile.withArgs('/foo/a.tex').callsArgWith(2, null, @documentclassContent) + + it "returns a.tex", (done) -> + @ProjectRootDocManager.findRootDocFileFromDirectory '/foo', (error, path, content) => + expect(error).not.to.exist + expect(path).to.equal 'a.tex' + expect(content).to.equal @documentclassContent + done() + + it "processes main.text first and stops processing when it finds the content", (done) -> + @ProjectRootDocManager.findRootDocFileFromDirectory '/foo', => + expect(@fs.readFile).to.be.calledWith('/foo/main.tex') + expect(@fs.readFile).to.be.calledWith('/foo/a.tex') + expect(@fs.readFile).not.to.be.calledWith('/foo/b.tex') + done() + + describe "when there is no documentclass", -> + it "returns null with no error", (done) -> + @ProjectRootDocManager.findRootDocFileFromDirectory '/foo', (error, path, content) => + expect(error).not.to.exist + expect(path).not.to.exist + expect(content).not.to.exist + done() + + it "processes all the files", (done) -> + @ProjectRootDocManager.findRootDocFileFromDirectory '/foo', => + expect(@fs.readFile).to.be.calledWith('/foo/main.tex') + expect(@fs.readFile).to.be.calledWith('/foo/a.tex') + expect(@fs.readFile).to.be.calledWith('/foo/b.tex') + done() + + describe "when there is an error reading a file", -> + beforeEach -> + @fs.readFile.withArgs('/foo/a.tex').callsArgWith(2, new Error('something went wrong')) + + it "returns an error", (done) -> + @ProjectRootDocManager.findRootDocFileFromDirectory '/foo', (error, path, content) => + expect(error).to.exist + expect(path).not.to.exist + expect(content).not.to.exist + done() + describe "setRootDocFromName", -> describe "when there is a suitable root doc", -> beforeEach (done)-> diff --git a/services/web/test/unit/coffee/Templates/TemplatesManagerTests.coffee b/services/web/test/unit/coffee/Templates/TemplatesManagerTests.coffee index c7d329181a..d182cfd3e4 100644 --- a/services/web/test/unit/coffee/Templates/TemplatesManagerTests.coffee +++ b/services/web/test/unit/coffee/Templates/TemplatesManagerTests.coffee @@ -31,7 +31,7 @@ describe 'TemplatesManager', -> unlink : sinon.stub() createWriteStream : sinon.stub().returns(on: sinon.stub().yields()) } - @ProjectUploadManager = {createProjectFromZipArchive : sinon.stub().callsArgWith(3, null, {_id:@project_id})} + @ProjectUploadManager = {createProjectFromZipArchiveWithName : sinon.stub().callsArgWith(3, null, {_id:@project_id})} @dumpFolder = "dump/path" @ProjectOptionsHandler = { setCompiler:sinon.stub().callsArgWith(2) @@ -87,7 +87,7 @@ describe 'TemplatesManager', -> @fs.createWriteStream.should.have.been.calledWith @dumpPath it "should create project", -> - @ProjectUploadManager.createProjectFromZipArchive.should.have.been.calledWithMatch @user_id, @templateName, @dumpPath + @ProjectUploadManager.createProjectFromZipArchiveWithName.should.have.been.calledWithMatch @user_id, @templateName, @dumpPath it "should unlink file", -> @fs.unlink.should.have.been.calledWith @dumpPath diff --git a/services/web/test/unit/coffee/Uploads/ProjectUploadManagerTests.coffee b/services/web/test/unit/coffee/Uploads/ProjectUploadManagerTests.coffee index 8edf700252..6fad3eb214 100644 --- a/services/web/test/unit/coffee/Uploads/ProjectUploadManagerTests.coffee +++ b/services/web/test/unit/coffee/Uploads/ProjectUploadManagerTests.coffee @@ -10,28 +10,95 @@ describe "ProjectUploadManager", -> @folder_id = "folder-id-123" @owner_id = "onwer-id-123" @callback = sinon.stub() + @source = "/path/to/zip/file-name.zip" + @destination = "/path/to/zile/file-extracted" + @root_folder_id = @folder_id + @owner_id = "owner-id-123" + @name = "Project name" + @othername = "Other name" + @project = + _id: @project_id + rootFolder: [ _id: @root_folder_id ] @ProjectUploadManager = SandboxedModule.require modulePath, requires: "./FileSystemImportManager" : @FileSystemImportManager = {} "./ArchiveManager" : @ArchiveManager = {} "../Project/ProjectCreationHandler" : @ProjectCreationHandler = {} "../Project/ProjectRootDocManager" : @ProjectRootDocManager = {} "../Project/ProjectDetailsHandler" : @ProjectDetailsHandler = {} + "../Documents/DocumentHelper" : @DocumentHelper = {} "rimraf" : @rimraf = sinon.stub().callsArg(1) + @ArchiveManager.extractZipArchive = sinon.stub().callsArg(2) + @ArchiveManager.findTopLevelDirectory = sinon.stub().callsArgWith(1, null, @topLevelDestination = "/path/to/zip/file-extracted/nested") + @ProjectCreationHandler.createBlankProject = sinon.stub().callsArgWith(2, null, @project) + @ProjectRootDocManager.setRootDocAutomatically = sinon.stub().callsArg(1) + @FileSystemImportManager.addFolderContents = sinon.stub().callsArg(5) + @ProjectRootDocManager.findRootDocFileFromDirectory = sinon.stub().callsArgWith(1, null, 'main.tex', @othername) + @ProjectRootDocManager.setRootDocFromName = sinon.stub().callsArg(2) + @DocumentHelper.getTitleFromTexContent = sinon.stub().returns(@othername) + describe "createProjectFromZipArchive", -> - beforeEach -> - @source = "/path/to/zip/file-name.zip" - @root_folder_id = @folder_id - @owner_id = "owner-id-123" - @name = "Project name" - @project = - _id: @project_id - rootFolder: [ _id: @root_folder_id ] + describe "when the title can be read from the root document", -> + beforeEach (done) -> + @ProjectUploadManager._getDestinationDirectory = sinon.stub().returns @destination + @ProjectDetailsHandler.generateUniqueName = sinon.stub().callsArgWith(2, null, @othername) + @ProjectUploadManager.createProjectFromZipArchive @owner_id, @name, @source, (err, project) => + @callback(err, project) + done() + + it "should set up the directory to extract the archive to", -> + @ProjectUploadManager._getDestinationDirectory.calledWith(@source).should.equal true + + it "should extract the archive", -> + @ArchiveManager.extractZipArchive.calledWith(@source, @destination).should.equal true + + it "should find the top level directory", -> + @ArchiveManager.findTopLevelDirectory.calledWith(@destination).should.equal true + + it "should insert the extracted archive into the folder", -> + @FileSystemImportManager.addFolderContents.calledWith(@owner_id, @project_id, @folder_id, @topLevelDestination, false) + .should.equal true + + it "should create a project owned by the owner_id", -> + @ProjectCreationHandler + .createBlankProject + .calledWith(@owner_id) + .should.equal true + + it "should create a project with the correct name", -> + @ProjectCreationHandler + .createBlankProject + .calledWith(sinon.match.any, @othername) + .should.equal true + + it "should read the title from the tex contents", -> + @DocumentHelper.getTitleFromTexContent.called.should.equal true + + it "should set the root document", -> + @ProjectRootDocManager.setRootDocFromName.calledWith(@project_id, 'main.tex').should.equal true + + it "should call the callback", -> + @callback.calledWith(sinon.match.falsy, @project).should.equal true + + describe "when the root document can't be determined", -> + beforeEach (done) -> + @ProjectRootDocManager.findRootDocFileFromDirectory = sinon.stub().callsArg(1) + @ProjectUploadManager._getDestinationDirectory = sinon.stub().returns @destination + @ProjectDetailsHandler.generateUniqueName = sinon.stub().callsArgWith(2, null, @name) + @ProjectUploadManager.createProjectFromZipArchive @owner_id, @name, @source, (err, project) => + @callback(err, project) + done() + + it "should not try to set the root doc", -> + @ProjectRootDocManager.setRootDocFromName.called.should.equal false + + describe "createProjectFromZipArchiveWithName", -> + beforeEach (done) -> @ProjectDetailsHandler.generateUniqueName = sinon.stub().callsArgWith(2, null, @name) - @ProjectCreationHandler.createBlankProject = sinon.stub().callsArgWith(2, null, @project) @ProjectUploadManager.insertZipArchiveIntoFolder = sinon.stub().callsArg(4) - @ProjectRootDocManager.setRootDocAutomatically = sinon.stub().callsArg(1) - @ProjectUploadManager.createProjectFromZipArchive @owner_id, @name, @source, @callback + @ProjectUploadManager.createProjectFromZipArchiveWithName @owner_id, @name, @source, (err, project) => + @callback(err, project) + done() it "should create a project owned by the owner_id", -> @ProjectCreationHandler @@ -61,15 +128,11 @@ describe "ProjectUploadManager", -> @callback.calledWith(sinon.match.falsy, @project).should.equal true describe "insertZipArchiveIntoFolder", -> - beforeEach -> - @source = "/path/to/zile/file.zip" - @destination = "/path/to/zile/file-extracted" + beforeEach (done) -> @ProjectUploadManager._getDestinationDirectory = sinon.stub().returns @destination - @ArchiveManager.extractZipArchive = sinon.stub().callsArg(2) - @ArchiveManager.findTopLevelDirectory = sinon.stub().callsArgWith(1, null, @topLevelDestination = "/path/to/zip/file-extracted/nested") - @FileSystemImportManager.addFolderContents = sinon.stub().callsArg(5) - - @ProjectUploadManager.insertZipArchiveIntoFolder @owner_id, @project_id, @folder_id, @source, @callback + @ProjectUploadManager.insertZipArchiveIntoFolder @owner_id, @project_id, @folder_id, @source, (err) => + @callback(err) + done() it "should set up the directory to extract the archive to", -> @ProjectUploadManager._getDestinationDirectory.calledWith(@source).should.equal true