From 0917fe10ca8ab0b5a36d2f1d4a3ae410892f74c8 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 19 Sep 2016 14:32:58 +0100 Subject: [PATCH] Return type when finding element by path so that we don't need a heuristic --- .../Features/Project/ProjectLocator.coffee | 25 +++++++++++----- .../ThirdPartyDataStore/UpdateMerger.coffee | 9 ++---- .../coffee/Project/ProjectLocatorTests.coffee | 30 ++++++++++++------- .../UpdateMergerTests.coffee | 20 +++---------- 4 files changed, 44 insertions(+), 40 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectLocator.coffee b/services/web/app/coffee/Features/Project/ProjectLocator.coffee index 9cd80d7c72..6c5ce708fc 100644 --- a/services/web/app/coffee/Features/Project/ProjectLocator.coffee +++ b/services/web/app/coffee/Features/Project/ProjectLocator.coffee @@ -81,7 +81,7 @@ module.exports = ProjectLocator = else getRootDoc project - findElementByPath: (project_or_id, needlePath, callback = (err, foundEntity)->)-> + findElementByPath: (project_or_id, needlePath, callback = (err, foundEntity, type)->)-> getParentFolder = (haystackFolder, foldersList, level, cb)-> if foldersList.length == 0 @@ -100,12 +100,23 @@ module.exports = ProjectLocator = getEntity = (folder, entityName, cb)-> if !entityName? - return cb null, folder - enteties = _.union folder.fileRefs, folder.docs, folder.folders - result = _.find enteties, (entity)-> - entity?.name.toLowerCase() == entityName.toLowerCase() + return cb null, folder, "folder" + + for file in folder.fileRefs + if file?.name.toLowerCase() == entityName.toLowerCase() + result = file + type = "file" + for doc in folder.docs + if doc?.name.toLowerCase() == entityName.toLowerCase() + result = doc + type = "doc" + for childFolder in folder.folders + if childFolder?.name.toLowerCase() == entityName.toLowerCase() + result = childFolder + type = "folder" + if result? - cb null, result + cb null, result, type else cb("not found project_or_id: #{project_or_id} search path: #{needlePath}, entity #{entityName} could not be found") @@ -117,7 +128,7 @@ module.exports = ProjectLocator = if !project? return callback("project could not be found for finding a element #{project_or_id}") if needlePath == '' || needlePath == '/' - return callback(null, project.rootFolder[0]) + return callback(null, project.rootFolder[0], "folder") if needlePath.indexOf('/') == 0 needlePath = needlePath.substring(1) diff --git a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee index 212c251029..49a07ff7ab 100644 --- a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee +++ b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee @@ -33,16 +33,11 @@ module.exports = self.p.processDoc project_id, elementId, user_id, fsPath, path, source, callback deleteUpdate: (project_id, path, source, callback)-> - projectLocator.findElementByPath project_id, path, (err, element)-> - type = 'file' + projectLocator.findElementByPath project_id, path, (err, element, type)-> if err? || !element? logger.log element:element, project_id:project_id, path:path, "could not find entity for deleting, assuming it was already deleted" return callback() - if element.lines? - type = 'doc' - else if element.folders? - type = 'folder' - logger.log project_id:project_id, updateType:path, updateType:type, element:element, "processing update to delete entity from tpds" + logger.log project_id:project_id, path:path, type:type, element:element, "processing update to delete entity from tpds" editorController.deleteEntity project_id, element._id, type, source, (err)-> logger.log project_id:project_id, path:path, "finished processing update to delete entity from tpds" callback() diff --git a/services/web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee index 437c32b122..cc56e9f583 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee @@ -181,61 +181,71 @@ describe 'ProjectLocator', -> it 'should take a doc path and return the element for a root level document', (done)-> path = "#{doc1.name}" - @locator.findElementByPath project._id, path, (err, element)-> + @locator.findElementByPath project._id, path, (err, element, type)-> element.should.deep.equal doc1 + expect(type).to.equal "doc" done() it 'should take a doc path and return the element for a root level document with a starting slash', (done)-> path = "/#{doc1.name}" - @locator.findElementByPath project._id, path, (err, element)-> + @locator.findElementByPath project._id, path, (err, element, type)-> element.should.deep.equal doc1 + expect(type).to.equal "doc" done() it 'should take a doc path and return the element for a nested document', (done)-> path = "#{subFolder.name}/#{secondSubFolder.name}/#{subSubDoc.name}" - @locator.findElementByPath project._id, path, (err, element)-> + @locator.findElementByPath project._id, path, (err, element, type)-> element.should.deep.equal subSubDoc + expect(type).to.equal "doc" done() it 'should take a file path and return the element for a root level document', (done)-> path = "#{file1.name}" - @locator.findElementByPath project._id, path, (err, element)-> + @locator.findElementByPath project._id, path, (err, element, type)-> element.should.deep.equal file1 + expect(type).to.equal "file" done() it 'should take a file path and return the element for a nested document', (done)-> path = "#{subFolder.name}/#{secondSubFolder.name}/#{subSubFile.name}" - @locator.findElementByPath project._id, path, (err, element)-> + @locator.findElementByPath project._id, path, (err, element, type)-> element.should.deep.equal subSubFile + expect(type).to.equal "file" done() it 'should take a file path and return the element for a nested document case insenstive', (done)-> path = "#{subFolder.name.toUpperCase()}/#{secondSubFolder.name.toUpperCase()}/#{subSubFile.name.toUpperCase()}" - @locator.findElementByPath project._id, path, (err, element)-> + @locator.findElementByPath project._id, path, (err, element, type)-> element.should.deep.equal subSubFile + expect(type).to.equal "file" done() it 'should take a file path and return the element for a nested folder', (done)-> path = "#{subFolder.name}/#{secondSubFolder.name}" - @locator.findElementByPath project._id, path, (err, element)-> + @locator.findElementByPath project._id, path, (err, element, type)-> element.should.deep.equal secondSubFolder + expect(type).to.equal "folder" done() it 'should take a file path and return the root folder', (done)-> - @locator.findElementByPath project._id, "/", (err, element)-> + @locator.findElementByPath project._id, "/", (err, element, type)-> element.should.deep.equal rootFolder + expect(type).to.equal "folder" done() it 'should return an error if the file can not be found inside know folder', (done)-> - @locator.findElementByPath project._id, "#{subFolder.name}/#{secondSubFolder.name}/exist.txt", (err, element)-> + @locator.findElementByPath project._id, "#{subFolder.name}/#{secondSubFolder.name}/exist.txt", (err, element, type)-> err.should.not.equal undefined assert.equal element, undefined + expect(type).to.be.undefined done() it 'should return an error if the file can not be found inside unknown folder', (done)-> - @locator.findElementByPath project._id, "this/does/not/exist.txt", (err, element)-> + @locator.findElementByPath project._id, "this/does/not/exist.txt", (err, element, type)-> err.should.not.equal undefined assert.equal element, undefined + expect(type).to.be.undefined done() diff --git a/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee b/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee index f4cd157115..f1fced4625 100644 --- a/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee +++ b/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee @@ -137,32 +137,20 @@ describe 'UpdateMerger :', -> beforeEach -> @path = "folder/doc1" + @type = "mock-type" @editorController.deleteEntity = -> @entity_id = "entity_id_here" @entity = _id:@entity_id - @projectLocator.findElementByPath = (project_id, path, cb)=> cb(null, @entity, @path) + @projectLocator.findElementByPath = (project_id, path, cb)=> cb(null, @entity, @type) it 'should get the element id', -> @projectLocator.findElementByPath = sinon.spy() @updateMerger.deleteUpdate @project_id, @path, @source, -> @projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true - it 'should delete the entity in the editor controller with type doc when entity has docLines array', (done)-> + it 'should delete the entity in the editor controller with the correct type', (done)-> @entity.lines = [] - mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, "doc", @source).callsArg(4) - @updateMerger.deleteUpdate @project_id, @path, @source, -> - mock.verify() - done() - - it 'should delete the entity in the editor controller with type folder when entity has folders array', (done)-> - @entity.folders = [] - mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, "folder", @source).callsArg(4) - @updateMerger.deleteUpdate @project_id, @path, @source, -> - mock.verify() - done() - - it 'should delete the entity in the editor controller with type file when entity has no interesting properties', (done)-> - mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, "file", @source).callsArg(4) + mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, @type, @source).callsArg(4) @updateMerger.deleteUpdate @project_id, @path, @source, -> mock.verify() done()