From f72f88adfaab5a1e2b59d79cb939a747a9924de7 Mon Sep 17 00:00:00 2001 From: Pete Richards Date: Thu, 6 Aug 2015 14:41:32 -0700 Subject: [PATCH] [Location] Use parent id as location Use the parent id as the location for a model. This greatly reduces the recursive work that must be done during move operations to keep the location accurate. Additionally, the locationService now implements a method `persistLocation` which can be used to persist the current object location as it's original location. --- .../browse/src/creation/CreationService.js | 19 +-- .../test/creation/CreationServiceSpec.js | 17 +- .../src/capabilities/LocationCapability.js | 41 +++-- .../entanglement/src/services/MoveService.js | 158 ++--------------- .../capabilities/LocationCapabilitySpec.js | 78 ++++++--- .../test/services/MoveServiceSpec.js | 159 +++--------------- 6 files changed, 112 insertions(+), 360 deletions(-) diff --git a/platform/commonUI/browse/src/creation/CreationService.js b/platform/commonUI/browse/src/creation/CreationService.js index 78c0cbebbd..f89ac0e909 100644 --- a/platform/commonUI/browse/src/creation/CreationService.js +++ b/platform/commonUI/browse/src/creation/CreationService.js @@ -121,25 +121,8 @@ define( // Store the location of an object relative to it's parent. function addLocationToModel(modelId, model, parent) { - var context = parent.getCapability("context"), - pathObjects, - pathIds; + model.location = parent.getId(); - if (!context) { - $log.warn('No parent context, location will not be set.'); - return model; - } - - pathObjects = context.getPath(); - if (!pathObjects || !pathObjects.length) { - pathObjects = []; - } - pathIds = pathObjects.map(function (object) { - return object.getId(); - }); - pathIds.push(modelId); - - model.location = pathIds.join('/'); return model; } diff --git a/platform/commonUI/browse/test/creation/CreationServiceSpec.js b/platform/commonUI/browse/test/creation/CreationServiceSpec.js index 85868974b0..bb26b1ccd9 100644 --- a/platform/commonUI/browse/test/creation/CreationServiceSpec.js +++ b/platform/commonUI/browse/test/creation/CreationServiceSpec.js @@ -103,21 +103,13 @@ define( mockPromise(true) ); - mockContextCapability.getPath.andReturn([ - { - getId: function () { return 'root'; } - }, - { - getId: function () { return 'parent'; } - } - ]); - mockParentObject.getCapability.andCallFake(function (key) { return mockCapabilities[key]; }); mockParentObject.useCapability.andCallFake(function (key, value) { return mockCapabilities[key].invoke(value); }); + mockParentObject.getId.andReturn('parentId'); mockPersistenceCapability.persist.andReturn( mockPromise(true) @@ -209,16 +201,15 @@ define( expect(mockLog.error).toHaveBeenCalled(); }); - it("stores location on new domainObjects", function() { + it("stores location on new domainObjects", function () { var model = { name: "my model" }; var objectPromise = creationService.createObject( model, mockParentObject ); - expect(model.location).toBeDefined(); - expect(model.location.indexOf('root/parent')).toBe(0); + expect(model.location).toBe('parentId'); }); }); } -); \ No newline at end of file +); diff --git a/platform/entanglement/src/capabilities/LocationCapability.js b/platform/entanglement/src/capabilities/LocationCapability.js index be55509791..66593c7130 100644 --- a/platform/entanglement/src/capabilities/LocationCapability.js +++ b/platform/entanglement/src/capabilities/LocationCapability.js @@ -9,6 +9,23 @@ define( return this; } + /** + * Persist the current location of the current domain object as it's + * primary location. Returns a promise. + */ + LocationCapability.prototype.persistLocation = function () { + return this.domainObject.useCapability( + 'mutation', + function (model) { + model.location = this.getLocation(); + }.bind(this) + ).then(function () { + return this.domainObject + .getCapability('persistence') + .persist(); + }.bind(this)); + }; + /** * Return the current location of the current domain object. Only * valid for domain objects that have a context capability. @@ -19,19 +36,10 @@ define( pathIds; if (!context) { - return this.domainObject.getId(); + return ''; } - pathObjects = context.getPath(); - if (!pathObjects) { - pathObjects = []; - } - - pathIds = pathObjects.map(function (object) { - return object.getId(); - }); - - return pathIds.join('/'); + return context.getParent().getId(); }; /** @@ -64,17 +72,6 @@ define( return new LocationCapability(domainObject); } - /** - * Return true if the LocationCapability can apply to a given - * domainObject, otherwise return false. - */ - createLocationCapability.appliesTo = function (domainObject) { - // if (!domainObject.hasCapability) { - // return false; - // } - // return domainObject.hasCapability('context'); - }; - return createLocationCapability; } ); diff --git a/platform/entanglement/src/services/MoveService.js b/platform/entanglement/src/services/MoveService.js index d35e6a3a4f..7ee5876e4a 100644 --- a/platform/entanglement/src/services/MoveService.js +++ b/platform/entanglement/src/services/MoveService.js @@ -32,153 +32,6 @@ define( */ function MoveService(policyService, linkService, $q) { - - /** - * Returns a promise for a childSelector for a given object which - * returns all original children in that given object. - */ - function getOriginalsInObject(object) { - var originalChildren = {}; - return getOriginalChildren(object) - .then(function recurseChildren(children) { - var promises = []; - children.children.forEach(function(child) { - promises.push(getOriginalsInObject(child)); - originalChildren[child.getId()] = {}; - }); - return $q.all(promises); - }).then(function prepareResponse(childResponses) { - childResponses.forEach(function(childResp) { - Object.keys(childResp).forEach(function(childId) { - originalChildren[childId] = childResp[childId]; - }); - }); - var childSelector = {}; - childSelector[object.getId()] = originalChildren; - return childSelector; - }); - } - - /** - * Get the original children in an object. Returns a promise for an object - * with a single key (the object's key), the value of which is an array - * of original children in that object. - */ - function getOriginalChildren(object) { - if (!object.hasCapability('composition')) { - return $q.when({ - objectId: object.getId(), - children: [] - }); - } - return object.useCapability('composition') - .then(function filterToOriginal(children) { - return children.filter(function(child) { - if (child.hasCapability('location')) { - return child.getCapability('location') - .isOriginal(); - } - return false; - }); - }) - .then(function returnAsObject(children) { - return { - objectId: object.getId(), - children: children - }; - }); - } - - /** - * Persist an instance of an object as the original location of - * that object. - */ - function saveAsOriginal(newOriginal) { - return newOriginal.useCapability( - 'mutation', - function (model) { - model.location = newOriginal - .getCapability('location') - .getLocation(); - } - ).then(function() { - return newOriginal - .getCapability('persistence') - .persist(); - }); - } - - /** - * childrenSelector is an object where keys are childIds and values - * are childSelector objects. Returns all children in the object - * which have a key in the childSelector, plus all children from - * recursive childSelection. - */ - function getMatchingChildrenFromObject(object, childrenSelector) { - var selected = []; - if (!object.hasCapability('composition')) { - return $q.when(selected); - } - return object.useCapability('composition') - .then(function selectChildren(children) { - var promises = []; - children.forEach(function (child) { - if (!childrenSelector[child.getId()]) { - return; - } - selected.push(child); - promises.push(getMatchingChildrenFromObject( - child, - childrenSelector[child.getId()] - )); - }); - - return $q.all(promises) - .then(function reduceResults (results) { - return results.reduce(function(memo, result) { - return memo.concat(result); - }, selected); - }); - }); - } - - - /** - * Uses the original object to determine which objects are originals, - * and then returns a function which, when given the same object in the - * new context, will update the location of all originals in the leaf - * to match the new context. - */ - function updateOriginalLocation(object) { - var oldLocationCapability = object.getCapability('location'); - if (!oldLocationCapability.isOriginal()) { - return function (objectInNewContext) { - return objectInNewContext; - }; - } - return function setOriginalLocation(objectInNewContext) { - return saveAsOriginal(objectInNewContext) - .then(function () { - if (!object.hasCapability('composition')) { - return objectInNewContext; - } - getOriginalsInObject(object) - .then(function(childSelector) { - return getMatchingChildrenFromObject( - objectInNewContext, - childSelector[object.getId()] - ); - }).then(function(originalChildren) { - return $q.all( - originalChildren.map(saveAsOriginal) - ); - }).then(function() { - return objectInNewContext; - }); - }); - }; - } - return { /** * Returns `true` if `object` can be moved into @@ -216,7 +69,16 @@ define( perform: function (object, parentObject) { return linkService .perform(object, parentObject) - .then(updateOriginalLocation(object)) + .then(function (objectInNewContext) { + if (!object.hasCapability('location')) { + return; + } + if (object.getCapability('location').isOriginal()) { + return objectInNewContext + .getCapability('location') + .persistLocation(); + } + }) .then(function () { return object .getCapability('action') diff --git a/platform/entanglement/test/capabilities/LocationCapabilitySpec.js b/platform/entanglement/test/capabilities/LocationCapabilitySpec.js index cec26520e5..e707a34590 100644 --- a/platform/entanglement/test/capabilities/LocationCapabilitySpec.js +++ b/platform/entanglement/test/capabilities/LocationCapabilitySpec.js @@ -3,66 +3,90 @@ define( [ '../../src/capabilities/LocationCapability', - '../DomainObjectFactory' + '../DomainObjectFactory', + '../ControlledPromise' ], - function (LocationCapability, domainObjectFactory) { + function (LocationCapability, domainObjectFactory, ControlledPromise) { describe("LocationCapability", function () { - - // xit("applies to objects with a context capability", function () { - // var domainObject = domainObjectFactory({ - // capabilities: { - // context: true - // } - // }); - // expect(LocationCapability.appliesTo(domainObject)).toBe(true); - // }); - // - // xit("does not apply to objects without context capability", function () { - // var domainObject = domainObjectFactory(); - // expect(LocationCapability.appliesTo(domainObject)).toBe(false); - // }); - describe("instantiated with domain object", function () { var locationCapability, + persistencePromise, + mutationPromise, domainObject; beforeEach(function () { domainObject = domainObjectFactory({ capabilities: { context: { - getPath: function() { - return [ - domainObjectFactory({id: 'root'}), - domainObjectFactory({id: 'parent'}), - domainObjectFactory({id: 'me'}) - ]; + getParent: function() { + return domainObjectFactory({id: 'root'}); } - } + }, + persistence: jasmine.createSpyObj( + 'persistenceCapability', + ['persist'] + ), + mutation: jasmine.createSpyObj( + 'mutationCapability', + ['invoke'] + ) } }); + persistencePromise = new ControlledPromise(); + domainObject.capabilities.persistence.persist.andReturn( + persistencePromise + ); + + mutationPromise = new ControlledPromise(); + domainObject.capabilities.mutation.invoke.andCallFake( + function (mutator) { + return mutationPromise.then(function () { + mutator(domainObject.model); + }); + } + ); + locationCapability = new LocationCapability(domainObject); }); it("returns location", function () { expect(locationCapability.getLocation()) - .toBe('root/parent/me'); + .toBe('root'); }); it("knows when the object is an original", function () { - domainObject.model.location = 'root/parent/me'; + domainObject.model.location = 'root'; expect(locationCapability.isOriginal()).toBe(true); expect(locationCapability.isLink()).toBe(false); }); it("knows when the object is a link.", function () { - domainObject.model.location = 'root/another/location/me'; + domainObject.model.location = 'different-root'; expect(locationCapability.isLink()).toBe(true); expect(locationCapability.isOriginal()).toBe(false); }); + it("can persist location", function () { + var persistResult = locationCapability.persistLocation(), + whenComplete = jasmine.createSpy('whenComplete'); + + persistResult.then(whenComplete); + + expect(domainObject.model.location).not.toBeDefined(); + mutationPromise.resolve(); + expect(domainObject.model.location).toBe('root'); + + expect(whenComplete).not.toHaveBeenCalled(); + expect(domainObject.capabilities.persistence.persist) + .toHaveBeenCalled(); + + persistencePromise.resolve(); + expect(whenComplete).toHaveBeenCalled(); + }); + }); }); } diff --git a/platform/entanglement/test/services/MoveServiceSpec.js b/platform/entanglement/test/services/MoveServiceSpec.js index bc3d7f08ad..b48dd4e66e 100644 --- a/platform/entanglement/test/services/MoveServiceSpec.js +++ b/platform/entanglement/test/services/MoveServiceSpec.js @@ -149,9 +149,7 @@ define( newParent, actionCapability, locationCapability, - persistenceCapability, - persistencePromise, - mutationPromise, + locationPromise, moveResult; beforeEach(function () { @@ -164,35 +162,18 @@ define( 'locationCapability', [ 'isOriginal', - 'getLocation' + 'persistLocation' ] ); - persistenceCapability = jasmine.createSpyObj( - 'persistenceCapability', - ['persist'] - ); - - persistencePromise = new ControlledPromise(); - persistenceCapability.persist.andReturn(persistencePromise); - - mutationPromise = new ControlledPromise(); + locationPromise = new ControlledPromise(); + locationCapability.persistLocation.andReturn(locationPromise); object = domainObjectFactory({ name: 'object', capabilities: { action: actionCapability, - mutation: { - invoke: function (mutator) { - mutator(object.model); - return mutationPromise; - } - }, - persistence: persistenceCapability, location: locationCapability - }, - model: { - location: 'otherThing' } }); @@ -211,134 +192,48 @@ define( .toHaveBeenCalledWith(jasmine.any(Function)); }); - it("removes object when link is completed", function () { - linkService.perform.mostRecentCall.promise.resolve(); - expect(object.getCapability) - .toHaveBeenCalledWith('action'); - expect(actionCapability.perform) - .toHaveBeenCalledWith('remove'); - }); - - describe("when moving an original", function() { + describe("when moving an original", function () { beforeEach(function () { locationCapability.isOriginal.andReturn(true); - locationCapability.getLocation.andReturn('newParent'); linkService.perform.mostRecentCall.promise.resolve(); }); - it("updates location", function() { - expect(object.model.location).toBe('newParent'); - }); - it("persists new model when mutation completes", function() { - mutationPromise.resolve(); - expect(persistenceCapability.persist) + it("updates location", function () { + expect(locationCapability.persistLocation) .toHaveBeenCalled(); }); + + describe("after location update", function () { + beforeEach(function () { + locationPromise.resolve(); + }); + + it("removes object from parent", function () { + expect(actionCapability.perform) + .toHaveBeenCalledWith('remove'); + }); + }); + }); - describe("when moving a link", function() { + describe("when moving a link", function () { beforeEach(function () { locationCapability.isOriginal.andReturn(false); - locationCapability.getLocation.andReturn('newParent'); linkService.perform.mostRecentCall.promise.resolve(); }); - it("does not modify location", function() { - expect(object.model.location).toBe('otherThing'); - }); - it("does not call persistence", function() { - expect(persistenceCapability.persist) + + it("does not update location", function () { + expect(locationCapability.persistLocation) .not .toHaveBeenCalled(); }); - }); - describe("when moving an object with children", function() { - - var children; - - beforeEach(function () { - - var instantMutator = function (index) { - return { - invoke: function (mutator) { - mutator(children[index].model); - return { - then: function(callback) { - callback(); - } - }; - } - }; - }; - - children = [ - domainObjectFactory({ - id: 'childa', - capabilities: { - location: jasmine.createSpyObj( - 'childalocation', - ['isOriginal', 'getLocation'] - ), - mutation: instantMutator(0) - }, - model: { - location: 'childa-old-location' - } - }), - domainObjectFactory({ - id: 'childb', - capabilities: { - location: jasmine.createSpyObj( - 'childblocation', - ['isOriginal', 'getLocation'] - ), - mutation: instantMutator(1) - }, - model: { - location: 'childb-old-location' - } - }), - domainObjectFactory({ - id: 'childc', - capabilities: { - location: jasmine.createSpyObj( - 'childclocation', - ['isOriginal', 'getLocation'] - ), - mutation: instantMutator(2) - }, - model: { - location: 'childc-old-location' - } - }) - ]; - - children[0].capabilities.location.isOriginal.andReturn(true); - children[0].capabilities.location.getLocation.andReturn('childalocation'); - children[1].capabilities.location.isOriginal.andReturn(true); - children[1].capabilities.location.getLocation.andReturn('childblocation'); - children[2].capabilities.location.isOriginal.andReturn(false); - children[2].capabilities.location.getLocation.andReturn('childclocation'); - - object.capabilities.composition = { - invoke: function () { - return { - then: function (callback) { - callback(children); - } - }; - } - }; - linkService.perform.mostRecentCall.promise.resolve(); - }); - - - it("recursively updates the location of originals", function () { - expect(children[0].model.location).toBe('childalocation'); - expect(children[1].model.location).toBe('childblocation'); - expect(children[2].model.location).toBe('childc-old-location'); + it("removes object from parent", function () { + expect(actionCapability.perform) + .toHaveBeenCalledWith('remove'); }); }); + }); }); }