From 194bc41322aaa85eb2a5f23e98d459b074c37060 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Tue, 1 Sep 2015 17:03:54 -0700 Subject: [PATCH 01/23] [Composition] Add an add method Add a method to add new objects to the composition of another domain object. nasa/openmctweb#97 --- .../src/capabilities/CompositionCapability.js | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/platform/core/src/capabilities/CompositionCapability.js b/platform/core/src/capabilities/CompositionCapability.js index f1b2532040..a2a269e0b8 100644 --- a/platform/core/src/capabilities/CompositionCapability.js +++ b/platform/core/src/capabilities/CompositionCapability.js @@ -50,6 +50,49 @@ define( this.domainObject = domainObject; } + /** + * Add a domain object to the composition of the field. + * This mutates but does not persist the modified object. + * + * If no index is given, this is added to the end of the composition. + * + * @param {DomainObject|string} domainObject the domain object to add, + * or simply its identifier + * @param {number} [index] the index at which to add the object + * @returns {Promise.<boolean>} the mutation result + */ + CompositionCapability.prototype.add = function (domainObject, index) { + var id = typeof domainObject === 'string' ? + domainObject : domainObject.getId(); + + function addIdToModel(model) { + var composition = model.composition, + oldIndex = composition.indexOf(id); + + // If no index has been specified already and the id is already + // present, nothing to do. If the id is already at that index, + // also nothing to do. + if ((isNaN(index) && oldIndex !== -1) || (index === oldIndex) { + return; + } + + // Pick a specific index if needed. + index = isNaN(index) ? composition.length : index; + // Also, don't put past the end of the array + index = Math.min(composition.length, index); + + // Remove the existing instance of the id + if (oldIndex !== -1) { + model.composition.splice(oldIndex, 1); + } + + // ...and add it back at the appropriate index. + model.composition.splice(index, 0, id); + } + + return this.domainObject.useCapability('mutation', addIdToModel); + }; + /** * Request the composition of this object. * @returns {Promise.<DomainObject[]>} a list of all domain From 8e995eba8f5af52a0aabd34c39ade61fdc539749 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Tue, 1 Sep 2015 17:11:09 -0700 Subject: [PATCH 02/23] [Composition] Use composition.add during creation --- .../browse/src/creation/CreationService.js | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/platform/commonUI/browse/src/creation/CreationService.js b/platform/commonUI/browse/src/creation/CreationService.js index 3dd96d289e..188c384c1a 100644 --- a/platform/commonUI/browse/src/creation/CreationService.js +++ b/platform/commonUI/browse/src/creation/CreationService.js @@ -86,18 +86,9 @@ define( // composition, so that it will subsequently appear // as a child contained by that parent. function addToComposition(id, parent, parentPersistence) { - var mutatationResult = parent.useCapability("mutation", function (model) { - if (Array.isArray(model.composition)) { - // Don't add if the id is already there - if (model.composition.indexOf(id) === -1) { - model.composition.push(id); - } - } else { - // This is abnormal; composition should be an array - self.$log.warn(NO_COMPOSITION_WARNING + parent.getId()); - return false; // Cancel mutation - } - }); + var compositionCapability = parent.getCapability('composition'), + mutationResult = compositionCapability && + compositionCapability.add(id); return self.$q.when(mutatationResult).then(function (result) { if (!result) { From b9d8b124ffc5dafba1cce20aa525937a683f9bd1 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Wed, 2 Sep 2015 09:08:47 -0700 Subject: [PATCH 03/23] [Composition] Use composition.add from link action --- .../commonUI/edit/src/actions/LinkAction.js | 17 +++++------------ .../src/capabilities/CompositionCapability.js | 7 ++++--- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/platform/commonUI/edit/src/actions/LinkAction.js b/platform/commonUI/edit/src/actions/LinkAction.js index 74abd2a93c..95ed9a8082 100644 --- a/platform/commonUI/edit/src/actions/LinkAction.js +++ b/platform/commonUI/edit/src/actions/LinkAction.js @@ -36,20 +36,11 @@ define( function LinkAction(context) { this.domainObject = (context || {}).domainObject; this.selectedObject = (context || {}).selectedObject; - this.selectedId = this.selectedObject && this.selectedObject.getId(); } LinkAction.prototype.perform = function () { var self = this; - // Add this domain object's identifier - function addId(model) { - if (Array.isArray(model.composition) && - model.composition.indexOf(self.selectedId) < 0) { - model.composition.push(self.selectedId); - } - } - // Persist changes to the domain object function doPersist() { var persistence = @@ -59,11 +50,13 @@ define( // Link these objects function doLink() { - return self.domainObject.useCapability("mutation", addId) - .then(doPersist); + var composition = self.domainObject && + self.domainObject.getCapability('composition'); + return composition && composition.add(self.selectedObject) + .then(doPersist); } - return this.selectedId && doLink(); + return this.selectedObject && doLink(); }; return LinkAction; diff --git a/platform/core/src/capabilities/CompositionCapability.js b/platform/core/src/capabilities/CompositionCapability.js index a2a269e0b8..c96a060986 100644 --- a/platform/core/src/capabilities/CompositionCapability.js +++ b/platform/core/src/capabilities/CompositionCapability.js @@ -63,7 +63,8 @@ define( */ CompositionCapability.prototype.add = function (domainObject, index) { var id = typeof domainObject === 'string' ? - domainObject : domainObject.getId(); + domainObject : domainObject.getId(), + $q = this.$q; function addIdToModel(model) { var composition = model.composition, @@ -71,9 +72,9 @@ define( // If no index has been specified already and the id is already // present, nothing to do. If the id is already at that index, - // also nothing to do. + // also nothing to do, so cancel mutation. if ((isNaN(index) && oldIndex !== -1) || (index === oldIndex) { - return; + return false; } // Pick a specific index if needed. From d3d94d67ea1ae974f3c6cd4b6d5540c1bf4ec45b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Wed, 2 Sep 2015 09:11:46 -0700 Subject: [PATCH 04/23] [Composition] Use composition.add from LinkService --- .../src/capabilities/CompositionCapability.js | 5 ++--- platform/entanglement/src/services/LinkService.js | 15 ++++++--------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/platform/core/src/capabilities/CompositionCapability.js b/platform/core/src/capabilities/CompositionCapability.js index c96a060986..1bf7f790f1 100644 --- a/platform/core/src/capabilities/CompositionCapability.js +++ b/platform/core/src/capabilities/CompositionCapability.js @@ -63,8 +63,7 @@ define( */ CompositionCapability.prototype.add = function (domainObject, index) { var id = typeof domainObject === 'string' ? - domainObject : domainObject.getId(), - $q = this.$q; + domainObject : domainObject.getId(); function addIdToModel(model) { var composition = model.composition, @@ -73,7 +72,7 @@ define( // If no index has been specified already and the id is already // present, nothing to do. If the id is already at that index, // also nothing to do, so cancel mutation. - if ((isNaN(index) && oldIndex !== -1) || (index === oldIndex) { + if ((isNaN(index) && oldIndex !== -1) || (index === oldIndex)) { return false; } diff --git a/platform/entanglement/src/services/LinkService.js b/platform/entanglement/src/services/LinkService.js index 9fb38dd273..ae02675eeb 100644 --- a/platform/entanglement/src/services/LinkService.js +++ b/platform/entanglement/src/services/LinkService.js @@ -52,10 +52,13 @@ define( "composition", parentCandidate.getCapability('type'), object.getCapability('type') - ); + ) && parentCandidate.hasCapability('composition'); }; LinkService.prototype.perform = function (object, parentObject) { + // Note that this was checked-for explicitly during validate step + var composition = parentObject.getCapability('composition'); + function findChild(children) { var i; for (i = 0; i < children.length; i += 1) { @@ -65,16 +68,10 @@ define( } } - return parentObject.useCapability('mutation', function (model) { - if (model.composition.indexOf(object.getId()) === -1) { - model.composition.push(object.getId()); - } - }).then(function () { + return composition.add(object).then(function () { return parentObject.getCapability('persistence').persist(); }).then(function getObjectWithNewContext() { - return parentObject - .useCapability('composition') - .then(findChild); + return composition.invoke().then(findChild); }); }; From 8759fdbd95d675565cf384e1b0b6618f468c5ba4 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Wed, 2 Sep 2015 09:37:26 -0700 Subject: [PATCH 05/23] [Composition] Update specs Update specs to reflect usage of the add method in the composition capability. --- .../browse/src/creation/CreationService.js | 6 +-- .../test/creation/CreationServiceSpec.js | 47 +++++++++-------- .../edit/test/actions/LinkActionSpec.js | 36 ++++--------- .../entanglement/src/services/LinkService.js | 2 +- .../test/services/LinkServiceSpec.js | 52 +++++++++++++------ 5 files changed, 75 insertions(+), 68 deletions(-) diff --git a/platform/commonUI/browse/src/creation/CreationService.js b/platform/commonUI/browse/src/creation/CreationService.js index 188c384c1a..984a3dbe85 100644 --- a/platform/commonUI/browse/src/creation/CreationService.js +++ b/platform/commonUI/browse/src/creation/CreationService.js @@ -87,12 +87,12 @@ define( // as a child contained by that parent. function addToComposition(id, parent, parentPersistence) { var compositionCapability = parent.getCapability('composition'), - mutationResult = compositionCapability && + addResult = compositionCapability && compositionCapability.add(id); - return self.$q.when(mutatationResult).then(function (result) { + return self.$q.when(addResult).then(function (result) { if (!result) { - self.$log.error("Could not mutate " + parent.getId()); + self.$log.error("Could not modify " + parent.getId()); return undefined; } diff --git a/platform/commonUI/browse/test/creation/CreationServiceSpec.js b/platform/commonUI/browse/test/creation/CreationServiceSpec.js index bdcd752c6c..d492b51f8f 100644 --- a/platform/commonUI/browse/test/creation/CreationServiceSpec.js +++ b/platform/commonUI/browse/test/creation/CreationServiceSpec.js @@ -86,7 +86,7 @@ define( ); mockCompositionCapability = jasmine.createSpyObj( "composition", - ["invoke"] + ["invoke", "add"] ); mockContextCapability = jasmine.createSpyObj( "context", @@ -120,6 +120,7 @@ define( mockCompositionCapability.invoke.andReturn( mockPromise([mockNewObject]) ); + mockCompositionCapability.add.andReturn(mockPromise(true)); creationService = new CreationService( mockPersistenceService, @@ -143,33 +144,34 @@ define( parentModel = { composition: ["notAnyUUID"] }; creationService.createObject(model, mockParentObject); - // Invoke the mutation callback - expect(mockMutationCapability.invoke).toHaveBeenCalled(); - mockMutationCapability.invoke.mostRecentCall.args[0](parentModel); - - // Should have a longer composition now, with the new UUID - expect(parentModel.composition.length).toEqual(2); + // Verify that a new ID was added + expect(mockCompositionCapability.add) + .toHaveBeenCalledWith(jasmine.any(String)); }); - it("warns if parent has no composition", function () { - var model = { someKey: "some value" }, - parentModel = { }; - creationService.createObject(model, mockParentObject); + it("provides the newly-created object", function () { + var mockDomainObject = jasmine.createSpyObj( + 'newDomainObject', + ['getId', 'getModel', 'getCapability'] + ), + mockCallback = jasmine.createSpy('callback'); - // Verify precondition; no prior warnings - expect(mockLog.warn).not.toHaveBeenCalled(); + // Act as if the object had been created + mockCompositionCapability.add.andCallFake(function (id) { + mockDomainObject.getId.andReturn(id); + mockCompositionCapability.invoke + .andReturn(mockPromise([mockDomainObject])); + return mockPromise(true); + }); - // Invoke the mutation callback - expect(mockMutationCapability.invoke).toHaveBeenCalled(); - mockMutationCapability.invoke.mostRecentCall.args[0](parentModel); + // Should find it in the composition + creationService.createObject({}, mockParentObject) + .then(mockCallback); + + expect(mockCallback).toHaveBeenCalledWith(mockDomainObject); - // Should have a longer composition now, with the new UUID - expect(mockLog.warn).toHaveBeenCalled(); - // Composition should still be undefined - expect(parentModel.composition).toBeUndefined(); }); - it("warns if parent has no persistence capability", function () { // Callbacks var success = jasmine.createSpy("success"), @@ -185,7 +187,6 @@ define( expect(mockLog.warn).toHaveBeenCalled(); expect(success).not.toHaveBeenCalled(); expect(failure).toHaveBeenCalled(); - }); it("logs an error when mutaton fails", function () { @@ -194,7 +195,7 @@ define( var model = { someKey: "some value" }, parentModel = { composition: ["notAnyUUID"] }; - mockMutationCapability.invoke.andReturn(mockPromise(false)); + mockCompositionCapability.add.andReturn(mockPromise(false)); creationService.createObject(model, mockParentObject); diff --git a/platform/commonUI/edit/test/actions/LinkActionSpec.js b/platform/commonUI/edit/test/actions/LinkActionSpec.js index 835f93740a..96ea30e2b3 100644 --- a/platform/commonUI/edit/test/actions/LinkActionSpec.js +++ b/platform/commonUI/edit/test/actions/LinkActionSpec.js @@ -31,7 +31,7 @@ define( mockDomainObject, mockParent, mockContext, - mockMutation, + mockComposition, mockPersistence, mockType, actionContext, @@ -67,7 +67,7 @@ define( } }; mockContext = jasmine.createSpyObj("context", [ "getParent" ]); - mockMutation = jasmine.createSpyObj("mutation", [ "invoke" ]); + mockComposition = jasmine.createSpyObj("composition", [ "invoke", "add" ]); mockPersistence = jasmine.createSpyObj("persistence", [ "persist" ]); mockType = jasmine.createSpyObj("type", [ "hasFeature" ]); @@ -75,11 +75,11 @@ define( mockDomainObject.getCapability.andReturn(mockContext); mockContext.getParent.andReturn(mockParent); mockType.hasFeature.andReturn(true); - mockMutation.invoke.andReturn(mockPromise(true)); - + mockComposition.invoke.andReturn(mockPromise(true)); + mockComposition.add.andReturn(mockPromise(true)); capabilities = { - mutation: mockMutation, + composition: mockComposition, persistence: mockPersistence, type: mockType }; @@ -96,33 +96,17 @@ define( }); - it("mutates the parent when performed", function () { + it("adds to the parent's composition when performed", function () { action.perform(); - expect(mockMutation.invoke) - .toHaveBeenCalledWith(jasmine.any(Function)); + expect(mockComposition.add) + .toHaveBeenCalledWith(mockDomainObject); }); - it("changes composition from its mutation function", function () { - var mutator, result; + it("persists changes afterward", function () { action.perform(); - mutator = mockMutation.invoke.mostRecentCall.args[0]; - result = mutator(model); - - // Should not have cancelled the mutation - expect(result).not.toBe(false); - - // Simulate mutate's behavior (remove can either return a - // new model or modify this one in-place) - result = result || model; - - // Should have removed "test" - that was our - // mock domain object's id. - expect(result.composition).toEqual(["a", "b", "c", "test"]); - - // Finally, should have persisted expect(mockPersistence.persist).toHaveBeenCalled(); }); }); } -); \ No newline at end of file +); diff --git a/platform/entanglement/src/services/LinkService.js b/platform/entanglement/src/services/LinkService.js index ae02675eeb..ab648611e1 100644 --- a/platform/entanglement/src/services/LinkService.js +++ b/platform/entanglement/src/services/LinkService.js @@ -45,7 +45,7 @@ define( if (parentCandidate.getId() === object.getId()) { return false; } - if (parentCandidate.getModel().composition.indexOf(object.getId()) !== -1) { + if ((parentCandidate.getModel().composition || []).indexOf(object.getId()) !== -1) { return false; } return this.policyService.allow( diff --git a/platform/entanglement/test/services/LinkServiceSpec.js b/platform/entanglement/test/services/LinkServiceSpec.js index b9bbe62c58..7f430320a2 100644 --- a/platform/entanglement/test/services/LinkServiceSpec.js +++ b/platform/entanglement/test/services/LinkServiceSpec.js @@ -55,11 +55,18 @@ define( name: 'object' }); parentCandidate = domainObjectFactory({ - name: 'parentCandidate' + name: 'parentCandidate', + capabilities: { + composition: jasmine.createSpyObj( + 'composition', + ['invoke', 'add'] + ) + } }); validate = function () { return linkService.validate(object, parentCandidate); }; + mockPolicyService.allow.andReturn(true); }); it("does not allow invalid parentCandidate", function () { @@ -81,6 +88,23 @@ define( expect(validate()).toBe(false); }); + it("does not allow parents that contains object", function () { + object.id = 'abc'; + parentCandidate.id = 'xyz'; + parentCandidate.model.composition = ['abc']; + expect(validate()).toBe(false); + }); + + it("does not allow parents without composition", function () { + parentCandidate = domainObjectFactory({ + name: 'parentCandidate' + }); + object.id = 'abc'; + parentCandidate.id = 'xyz'; + parentCandidate.model.composition = undefined; + expect(validate()).toBe(false); + }); + describe("defers to policyService", function () { beforeEach(function () { object.id = 'abc'; @@ -121,16 +145,16 @@ define( linkedObject, parentModel, parentObject, - mutationPromise, compositionPromise, persistencePromise, + addPromise, compositionCapability, persistenceCapability; beforeEach(function () { - mutationPromise = new ControlledPromise(); compositionPromise = new ControlledPromise(); persistencePromise = new ControlledPromise(); + addPromise = new ControlledPromise(); persistenceCapability = jasmine.createSpyObj( 'persistenceCapability', ['persist'] @@ -138,9 +162,10 @@ define( persistenceCapability.persist.andReturn(persistencePromise); compositionCapability = jasmine.createSpyObj( 'compositionCapability', - ['invoke'] + ['invoke', 'add'] ); compositionCapability.invoke.andReturn(compositionPromise); + compositionCapability.add.andReturn(addPromise); parentModel = { composition: [] }; @@ -151,7 +176,7 @@ define( mutation: { invoke: function (mutator) { mutator(parentModel); - return mutationPromise; + return new ControlledPromise(); } }, persistence: persistenceCapability, @@ -172,20 +197,17 @@ define( }); - it("modifies parent model composition", function () { - expect(parentModel.composition.length).toBe(0); + it("adds to the parent's composition", function () { + expect(compositionCapability.add).not.toHaveBeenCalled(); linkService.perform(object, parentObject); - expect(parentObject.useCapability).toHaveBeenCalledWith( - 'mutation', - jasmine.any(Function) - ); - expect(parentModel.composition).toContain('xyz'); + expect(compositionCapability.add) + .toHaveBeenCalledWith(object); }); it("persists parent", function () { linkService.perform(object, parentObject); - expect(mutationPromise.then).toHaveBeenCalled(); - mutationPromise.resolve(); + expect(addPromise.then).toHaveBeenCalled(); + addPromise.resolve(); expect(parentObject.getCapability) .toHaveBeenCalledWith('persistence'); expect(persistenceCapability.persist).toHaveBeenCalled(); @@ -197,7 +219,7 @@ define( whenComplete = jasmine.createSpy('whenComplete'); returnPromise.then(whenComplete); - mutationPromise.resolve(); + addPromise.resolve(); persistencePromise.resolve(); compositionPromise.resolve([linkedObject]); expect(whenComplete).toHaveBeenCalledWith(linkedObject); From ba6e542d08c1acabb3cd1869e97cb1a80c77fa73 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Wed, 2 Sep 2015 11:16:19 -0700 Subject: [PATCH 06/23] [Persistence] Load models from multiple spaces --- platform/core/bundle.json | 18 +++++++++++- .../core/src/models/PersistedModelProvider.js | 28 ++++++++++++++----- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/platform/core/bundle.json b/platform/core/bundle.json index 4b33f48f35..35b76c8a32 100644 --- a/platform/core/bundle.json +++ b/platform/core/bundle.json @@ -63,7 +63,12 @@ "provides": "modelService", "type": "provider", "implementation": "models/PersistedModelProvider.js", - "depends": [ "persistenceService", "$q", "PERSISTENCE_SPACE" ] + "depends": [ + "persistenceService", + "$q", + "PERSISTENCE_SPACE", + "ADDITIONAL_PERSISTENCE_SPACES" + ] }, { "provides": "modelService", @@ -215,6 +220,17 @@ "composition": [] } } + ], + "constants": [ + { + "key": "PERSISTENCE_SPACE", + "value": "mct" + }, + { + "key": "ADDITIONAL_PERSISTENCE_SPACES", + "value": [], + "description": "An array of additional persistence spaces to load models from." + } ] } } diff --git a/platform/core/src/models/PersistedModelProvider.js b/platform/core/src/models/PersistedModelProvider.js index 59ab020b14..3acd5a6ae6 100644 --- a/platform/core/src/models/PersistedModelProvider.js +++ b/platform/core/src/models/PersistedModelProvider.js @@ -39,23 +39,37 @@ define( * @param {PersistenceService} persistenceService the service in which * domain object models are persisted. * @param $q Angular's $q service, for working with promises - * @param {string} SPACE the name of the persistence space from which - * models should be retrieved. + * @param {string} space the name of the persistence space(s) + * from which models should be retrieved. + * @param {string} spaces additional persistence spaces to use */ - function PersistedModelProvider(persistenceService, $q, space) { + function PersistedModelProvider(persistenceService, $q, space, spaces) { this.persistenceService = persistenceService; this.$q = $q; - this.space = space; + this.spaces = [space].concat(spaces || []); + } + + // Take the most recently modified model, for cases where + // multiple persistence spaces return models. + function takeMostRecent(modelA, modelB) { + return (!modelA || modelA.modified === undefined) ? modelB : + (!modelB || modelB.modified === undefined) ? modelA : + modelA.modified > modelB.modified ? modelA : + modelB; } PersistedModelProvider.prototype.getModels = function (ids) { var persistenceService = this.persistenceService, $q = this.$q, - space = this.space; + spaces = this.spaces; - // Load a single object model from persistence + // Load a single object model from any persistence spaces function loadModel(id) { - return persistenceService.readObject(space, id); + return $q.all(spaces.map(function (space) { + return persistenceService.readObject(space, id); + })).then(function (models) { + return models.reduce(takeMostRecent); + }); } // Package the result as id->model From b1238b0c96272788f991488e2a2c8f7ff9a1ff2b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Wed, 2 Sep 2015 11:37:47 -0700 Subject: [PATCH 07/23] [Composition] Return newly-contextualized object After adding to composition, return the newly-contextualized object; this is regularly used by other services. --- .../browse/src/creation/CreationService.js | 12 +-------- .../src/capabilities/CompositionCapability.js | 25 ++++++++++++++++--- .../entanglement/src/services/LinkService.js | 8 +++--- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/platform/commonUI/browse/src/creation/CreationService.js b/platform/commonUI/browse/src/creation/CreationService.js index 984a3dbe85..baac8488c3 100644 --- a/platform/commonUI/browse/src/creation/CreationService.js +++ b/platform/commonUI/browse/src/creation/CreationService.js @@ -97,17 +97,7 @@ define( } return parentPersistence.persist().then(function () { - // Locate and return new Object in context of parent. - return parent - .useCapability('composition') - .then(function (children) { - var i; - for (i = 0; i < children.length; i += 1) { - if (children[i].getId() === id) { - return children[i]; - } - } - }); + return result; }); }); } diff --git a/platform/core/src/capabilities/CompositionCapability.js b/platform/core/src/capabilities/CompositionCapability.js index 1bf7f790f1..8fcca62174 100644 --- a/platform/core/src/capabilities/CompositionCapability.js +++ b/platform/core/src/capabilities/CompositionCapability.js @@ -59,11 +59,27 @@ define( * @param {DomainObject|string} domainObject the domain object to add, * or simply its identifier * @param {number} [index] the index at which to add the object - * @returns {Promise.<boolean>} the mutation result + * @returns {Promise.<DomainObject>} a promise for the added object + * in its new context */ CompositionCapability.prototype.add = function (domainObject, index) { - var id = typeof domainObject === 'string' ? - domainObject : domainObject.getId(); + var self = this, + id = typeof domainObject === 'string' ? + domainObject : domainObject.getId(); + + // Find the object with the above id, used to contextualize + function findObject(objects) { + var i; + for (i = 0; i < objects.length; i += 1) { + if (objects[i].getId() === id) { + return objects[i]; + } + } + } + + function contextualize(mutationResult) { + return mutationResult && self.invoke().then(findObject); + } function addIdToModel(model) { var composition = model.composition, @@ -90,7 +106,8 @@ define( model.composition.splice(index, 0, id); } - return this.domainObject.useCapability('mutation', addIdToModel); + return this.domainObject.useCapability('mutation', addIdToModel) + .then(contextualize); }; /** diff --git a/platform/entanglement/src/services/LinkService.js b/platform/entanglement/src/services/LinkService.js index ab648611e1..bfebddcbd9 100644 --- a/platform/entanglement/src/services/LinkService.js +++ b/platform/entanglement/src/services/LinkService.js @@ -68,10 +68,10 @@ define( } } - return composition.add(object).then(function () { - return parentObject.getCapability('persistence').persist(); - }).then(function getObjectWithNewContext() { - return composition.invoke().then(findChild); + return composition.add(object).then(function (result) { + return parentObject.getCapability('persistence') + .persist() + .then(function () { return result; }); }); }; From b2649de64916ddfaa9d12d5beaaf2f7cb6e4c975 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Wed, 2 Sep 2015 11:45:52 -0700 Subject: [PATCH 08/23] [Composition] Update specs Update specs for changes to the return value of composition.add, nasa/openmctweb#97 --- .../test/creation/CreationServiceSpec.js | 2 +- .../core/src/models/PersistedModelProvider.js | 8 ++--- .../test/models/PersistedModelProviderSpec.js | 31 ++++++++++++++++--- .../entanglement/src/services/LinkService.js | 9 ------ .../test/services/LinkServiceSpec.js | 4 +-- 5 files changed, 34 insertions(+), 20 deletions(-) diff --git a/platform/commonUI/browse/test/creation/CreationServiceSpec.js b/platform/commonUI/browse/test/creation/CreationServiceSpec.js index d492b51f8f..bc26c8be23 100644 --- a/platform/commonUI/browse/test/creation/CreationServiceSpec.js +++ b/platform/commonUI/browse/test/creation/CreationServiceSpec.js @@ -161,7 +161,7 @@ define( mockDomainObject.getId.andReturn(id); mockCompositionCapability.invoke .andReturn(mockPromise([mockDomainObject])); - return mockPromise(true); + return mockPromise(mockDomainObject); }); // Should find it in the composition diff --git a/platform/core/src/models/PersistedModelProvider.js b/platform/core/src/models/PersistedModelProvider.js index 3acd5a6ae6..a10f818179 100644 --- a/platform/core/src/models/PersistedModelProvider.js +++ b/platform/core/src/models/PersistedModelProvider.js @@ -52,10 +52,10 @@ define( // Take the most recently modified model, for cases where // multiple persistence spaces return models. function takeMostRecent(modelA, modelB) { - return (!modelA || modelA.modified === undefined) ? modelB : - (!modelB || modelB.modified === undefined) ? modelA : - modelA.modified > modelB.modified ? modelA : - modelB; + return (!modelB || modelB.modified === undefined) ? modelA : + (!modelA || modelA.modified === undefined) ? modelB : + modelB.modified > modelA.modified ? modelB : + modelA; } PersistedModelProvider.prototype.getModels = function (ids) { diff --git a/platform/core/test/models/PersistedModelProviderSpec.js b/platform/core/test/models/PersistedModelProviderSpec.js index 20f0e89649..8dcb58a400 100644 --- a/platform/core/test/models/PersistedModelProviderSpec.js +++ b/platform/core/test/models/PersistedModelProviderSpec.js @@ -32,7 +32,9 @@ define( describe("The persisted model provider", function () { var mockQ, mockPersistenceService, - SPACE = "some space", + SPACE = "space0", + spaces = [ "space1" ], + modTimes, provider; function mockPromise(value) { @@ -51,12 +53,14 @@ define( } beforeEach(function () { + modTimes = {}; mockQ = { when: mockPromise, all: mockAll }; mockPersistenceService = { readObject: function (space, id) { return mockPromise({ space: space, - id: id + id: id, + modified: (modTimes[space] || {})[id] }); } }; @@ -64,7 +68,8 @@ define( provider = new PersistedModelProvider( mockPersistenceService, mockQ, - SPACE + SPACE, + spaces ); }); @@ -82,6 +87,24 @@ define( }); }); + it("reads object models from multiple spaces", function () { + var models; + + modTimes.space1 = { + 'x': 12321 + }; + + provider.getModels(["a", "x", "zz"]).then(function (m) { + models = m; + }); + + expect(models).toEqual({ + a: { space: SPACE, id: "a" }, + x: { space: 'space1', id: "x", modified: 12321 }, + zz: { space: SPACE, id: "zz" } + }); + }); + }); } -); \ No newline at end of file +); diff --git a/platform/entanglement/src/services/LinkService.js b/platform/entanglement/src/services/LinkService.js index bfebddcbd9..8a0613d9b5 100644 --- a/platform/entanglement/src/services/LinkService.js +++ b/platform/entanglement/src/services/LinkService.js @@ -59,15 +59,6 @@ define( // Note that this was checked-for explicitly during validate step var composition = parentObject.getCapability('composition'); - function findChild(children) { - var i; - for (i = 0; i < children.length; i += 1) { - if (children[i].getId() === object.getId()) { - return children[i]; - } - } - } - return composition.add(object).then(function (result) { return parentObject.getCapability('persistence') .persist() diff --git a/platform/entanglement/test/services/LinkServiceSpec.js b/platform/entanglement/test/services/LinkServiceSpec.js index 7f430320a2..32bb4c0a7a 100644 --- a/platform/entanglement/test/services/LinkServiceSpec.js +++ b/platform/entanglement/test/services/LinkServiceSpec.js @@ -207,7 +207,7 @@ define( it("persists parent", function () { linkService.perform(object, parentObject); expect(addPromise.then).toHaveBeenCalled(); - addPromise.resolve(); + addPromise.resolve(linkedObject); expect(parentObject.getCapability) .toHaveBeenCalledWith('persistence'); expect(persistenceCapability.persist).toHaveBeenCalled(); @@ -219,7 +219,7 @@ define( whenComplete = jasmine.createSpy('whenComplete'); returnPromise.then(whenComplete); - addPromise.resolve(); + addPromise.resolve(linkedObject); persistencePromise.resolve(); compositionPromise.resolve([linkedObject]); expect(whenComplete).toHaveBeenCalledWith(linkedObject); From 17e9e87a2b592e10feaf6cc98ff25cfae1cdd4d8 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Wed, 2 Sep 2015 11:56:37 -0700 Subject: [PATCH 09/23] [Composition] Test composition.add Add test case to verify the behavior of the add method of the composition capability. --- .../src/capabilities/CompositionCapability.js | 22 ++--- .../capabilities/CompositionCapabilitySpec.js | 96 ++++++++++++++++++- 2 files changed, 105 insertions(+), 13 deletions(-) diff --git a/platform/core/src/capabilities/CompositionCapability.js b/platform/core/src/capabilities/CompositionCapability.js index 8fcca62174..925e27ba79 100644 --- a/platform/core/src/capabilities/CompositionCapability.js +++ b/platform/core/src/capabilities/CompositionCapability.js @@ -65,7 +65,10 @@ define( CompositionCapability.prototype.add = function (domainObject, index) { var self = this, id = typeof domainObject === 'string' ? - domainObject : domainObject.getId(); + domainObject : domainObject.getId(), + model = self.domainObject.getModel(), + composition = model.composition, + oldIndex = composition.indexOf(id); // Find the object with the above id, used to contextualize function findObject(objects) { @@ -82,16 +85,6 @@ define( } function addIdToModel(model) { - var composition = model.composition, - oldIndex = composition.indexOf(id); - - // If no index has been specified already and the id is already - // present, nothing to do. If the id is already at that index, - // also nothing to do, so cancel mutation. - if ((isNaN(index) && oldIndex !== -1) || (index === oldIndex)) { - return false; - } - // Pick a specific index if needed. index = isNaN(index) ? composition.length : index; // Also, don't put past the end of the array @@ -106,6 +99,13 @@ define( model.composition.splice(index, 0, id); } + // If no index has been specified already and the id is already + // present, nothing to do. If the id is already at that index, + // also nothing to do, so cancel mutation. + if ((isNaN(index) && oldIndex !== -1) || (index === oldIndex)) { + return contextualize(true); + } + return this.domainObject.useCapability('mutation', addIdToModel) .then(contextualize); }; diff --git a/platform/core/test/capabilities/CompositionCapabilitySpec.js b/platform/core/test/capabilities/CompositionCapabilitySpec.js index 518cb2e795..08b135b172 100644 --- a/platform/core/test/capabilities/CompositionCapabilitySpec.js +++ b/platform/core/test/capabilities/CompositionCapabilitySpec.js @@ -47,7 +47,7 @@ define( // so support that, but don't introduce complication of // native promises. function mockPromise(value) { - return { + return (value || {}).then ? value : { then: function (callback) { return mockPromise(callback(value)); } @@ -111,6 +111,98 @@ define( }); + it("allows domain objects to be added", function () { + var result, + testModel = { composition: [] }, + mockChild = jasmine.createSpyObj("child", DOMAIN_OBJECT_METHODS); + + mockDomainObject.getModel.andReturn(testModel); + mockObjectService.getObjects.andReturn(mockPromise({a: mockChild})); + mockChild.getCapability.andReturn(undefined); + mockChild.getId.andReturn('a'); + + mockDomainObject.useCapability.andCallFake(function (key, mutator) { + if (key === 'mutation') { + mutator(testModel); + return mockPromise(true); + } + }); + + composition.add(mockChild).then(function (domainObject) { + result = domainObject; + }); + + expect(testModel.composition).toEqual(['a']); + + // Should have returned the added object in its new context + expect(result.getId()).toEqual('a'); + expect(result.getCapability('context')).toBeDefined(); + expect(result.getCapability('context').getParent()) + .toEqual(mockDomainObject); + }); + + it("does not re-add IDs which are already present", function () { + var result, + testModel = { composition: [ 'a' ] }, + mockChild = jasmine.createSpyObj("child", DOMAIN_OBJECT_METHODS); + + mockDomainObject.getModel.andReturn(testModel); + mockObjectService.getObjects.andReturn(mockPromise({a: mockChild})); + mockChild.getCapability.andReturn(undefined); + mockChild.getId.andReturn('a'); + + mockDomainObject.useCapability.andCallFake(function (key, mutator) { + if (key === 'mutation') { + mutator(testModel); + return mockPromise(true); + } + }); + + composition.add(mockChild).then(function (domainObject) { + result = domainObject; + }); + + // Still just 'a' + expect(testModel.composition).toEqual(['a']); + + // Should have returned the added object in its new context + expect(result.getId()).toEqual('a'); + expect(result.getCapability('context')).toBeDefined(); + expect(result.getCapability('context').getParent()) + .toEqual(mockDomainObject); + }); + + it("can add objects at a specified index", function () { + var result, + testModel = { composition: [ 'a', 'b', 'c' ] }, + mockChild = jasmine.createSpyObj("child", DOMAIN_OBJECT_METHODS); + + mockDomainObject.getModel.andReturn(testModel); + mockObjectService.getObjects.andReturn(mockPromise({a: mockChild})); + mockChild.getCapability.andReturn(undefined); + mockChild.getId.andReturn('a'); + + mockDomainObject.useCapability.andCallFake(function (key, mutator) { + if (key === 'mutation') { + mutator(testModel); + return mockPromise(true); + } + }); + + composition.add(mockChild, 1).then(function (domainObject) { + result = domainObject; + }); + + // Still just 'a' + expect(testModel.composition).toEqual(['b', 'a', 'c']); + + // Should have returned the added object in its new context + expect(result.getId()).toEqual('a'); + expect(result.getCapability('context')).toBeDefined(); + expect(result.getCapability('context').getParent()) + .toEqual(mockDomainObject); + }); + }); } -); \ No newline at end of file +); From 3310016264c653dbca14736e0f04e33381bcea78 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Wed, 2 Sep 2015 12:01:43 -0700 Subject: [PATCH 10/23] [Properties] Hide rows without controls Hide rows for domain object properties that do not have associated controls from the Edit Properties dialog; follow up for nasa/openmctweb#92 --- platform/commonUI/edit/src/actions/PropertiesDialog.js | 3 +++ platform/commonUI/edit/test/actions/PropertiesDialogSpec.js | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/platform/commonUI/edit/src/actions/PropertiesDialog.js b/platform/commonUI/edit/src/actions/PropertiesDialog.js index 97ee1f5c0a..f461c0b8e1 100644 --- a/platform/commonUI/edit/src/actions/PropertiesDialog.js +++ b/platform/commonUI/edit/src/actions/PropertiesDialog.js @@ -54,6 +54,9 @@ define( var row = Object.create(property.getDefinition()); row.key = index; return row; + }).filter(function (row) { + // Only show properties which are editable + return row.control; }) }] }; diff --git a/platform/commonUI/edit/test/actions/PropertiesDialogSpec.js b/platform/commonUI/edit/test/actions/PropertiesDialogSpec.js index 7269ae64a3..a9077e8ec6 100644 --- a/platform/commonUI/edit/test/actions/PropertiesDialogSpec.js +++ b/platform/commonUI/edit/test/actions/PropertiesDialogSpec.js @@ -39,7 +39,7 @@ define( return { getValue: function (model) { return model[k]; }, setValue: function (model, v) { model[k] = v; }, - getDefinition: function () { return {}; } + getDefinition: function () { return { control: 'textfield '}; } }; }); From c17269ba8bc7cff3ee2481d1b2c9b14bbba7c34f Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Tue, 22 Sep 2015 13:37:56 -0700 Subject: [PATCH 11/23] [LinkService] Reorder/refactor validation Reoder validation to consider the composition capability first; refactor to express as a single expression to clarify validation logic (explicitly say 'and' instead of implying it with control flow.) Based on feedback from nasa/openmctweb#98 --- .../entanglement/src/services/LinkService.js | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/platform/entanglement/src/services/LinkService.js b/platform/entanglement/src/services/LinkService.js index 8a0613d9b5..69085bbfe4 100644 --- a/platform/entanglement/src/services/LinkService.js +++ b/platform/entanglement/src/services/LinkService.js @@ -39,20 +39,17 @@ define( } LinkService.prototype.validate = function (object, parentCandidate) { - if (!parentCandidate || !parentCandidate.getId) { - return false; - } - if (parentCandidate.getId() === object.getId()) { - return false; - } - if ((parentCandidate.getModel().composition || []).indexOf(object.getId()) !== -1) { - return false; - } - return this.policyService.allow( - "composition", - parentCandidate.getCapability('type'), - object.getCapability('type') - ) && parentCandidate.hasCapability('composition'); + var objectId = object.getId(); + return !!parentCandidate && + !!parentCandidate.getId && + parentCandidate.getId() !== objectId && + parentCandidate.hasCapability("composition") && + parentCandidate.getModel().composition.indexOf(objectId) === -1 && + this.policyService.allow( + "composition", + parentCandidate.getCapability('type'), + object.getCapability('type') + ); }; LinkService.prototype.perform = function (object, parentObject) { From 55362f6b268482c4079a6dcafc3d68d5818572e1 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Tue, 22 Sep 2015 13:48:11 -0700 Subject: [PATCH 12/23] [LinkService] Clarify comment Clarify comment, based on feedback from nasa/openmctweb#98 --- platform/entanglement/src/services/LinkService.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/platform/entanglement/src/services/LinkService.js b/platform/entanglement/src/services/LinkService.js index 69085bbfe4..221bd042c0 100644 --- a/platform/entanglement/src/services/LinkService.js +++ b/platform/entanglement/src/services/LinkService.js @@ -53,7 +53,8 @@ define( }; LinkService.prototype.perform = function (object, parentObject) { - // Note that this was checked-for explicitly during validate step + // It is assumed here that validate has been called, and therefore + // that parentObject.hasCapability('composition'). var composition = parentObject.getCapability('composition'); return composition.add(object).then(function (result) { From b4a2bfd727b828caf52705e2b27fbcca10e71603 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Tue, 22 Sep 2015 13:50:10 -0700 Subject: [PATCH 13/23] [LinkService] Change variable name ...based on feedback from nasa/openmctweb#98 --- platform/entanglement/src/services/LinkService.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/platform/entanglement/src/services/LinkService.js b/platform/entanglement/src/services/LinkService.js index 221bd042c0..da4a89e33c 100644 --- a/platform/entanglement/src/services/LinkService.js +++ b/platform/entanglement/src/services/LinkService.js @@ -57,10 +57,10 @@ define( // that parentObject.hasCapability('composition'). var composition = parentObject.getCapability('composition'); - return composition.add(object).then(function (result) { + return composition.add(object).then(function (objectInNewContext) { return parentObject.getCapability('persistence') .persist() - .then(function () { return result; }); + .then(function () { return objectInNewContext; }); }); }; From 597b18af1cfc76591c3b0207c55726815d6ad438 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Tue, 22 Sep 2015 14:07:46 -0700 Subject: [PATCH 14/23] [LinkService] Amend test cases Remove redundant test case; modify composition-checking test case to expose correct capability information. In response to feedback from nasa/openmctweb#98 --- platform/entanglement/test/services/LinkServiceSpec.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/platform/entanglement/test/services/LinkServiceSpec.js b/platform/entanglement/test/services/LinkServiceSpec.js index 32bb4c0a7a..69c6ebd48c 100644 --- a/platform/entanglement/test/services/LinkServiceSpec.js +++ b/platform/entanglement/test/services/LinkServiceSpec.js @@ -88,19 +88,15 @@ define( expect(validate()).toBe(false); }); - it("does not allow parents that contains object", function () { - object.id = 'abc'; - parentCandidate.id = 'xyz'; - parentCandidate.model.composition = ['abc']; - expect(validate()).toBe(false); - }); - it("does not allow parents without composition", function () { parentCandidate = domainObjectFactory({ name: 'parentCandidate' }); object.id = 'abc'; parentCandidate.id = 'xyz'; + parentCandidate.hasCapability.andCallFake(function (c) { + return c !== 'composition'; + }); parentCandidate.model.composition = undefined; expect(validate()).toBe(false); }); From ff24f064758f82f92ecffc2d766afb4790d8e9c2 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Tue, 22 Sep 2015 14:10:06 -0700 Subject: [PATCH 15/23] [Entanglement] Add JSDoc ...per feedback from nasa/openmctweb#98 --- platform/entanglement/src/actions/AbstractComposeAction.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/platform/entanglement/src/actions/AbstractComposeAction.js b/platform/entanglement/src/actions/AbstractComposeAction.js index cc78eb1888..6d747bc764 100644 --- a/platform/entanglement/src/actions/AbstractComposeAction.js +++ b/platform/entanglement/src/actions/AbstractComposeAction.js @@ -32,7 +32,8 @@ define( * @private */ /** - * Change the composition of the specified objects. + * Change the composition of the specified objects. Note that this + * should only be invoked after successfully validating. * * @param {DomainObject} domainObject the domain object to * move, copy, or link. From 119403e71cd8d35078a03036793835f8fedcb1e0 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Tue, 22 Sep 2015 14:39:15 -0700 Subject: [PATCH 16/23] [Entanglement] Throw errors if validation is skipped ...per feedback from nasa/openmctweb#98 --- .../entanglement/src/services/CopyService.js | 6 ++++++ .../entanglement/src/services/LinkService.js | 19 +++++++++++-------- .../entanglement/src/services/MoveService.js | 6 ++++++ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/platform/entanglement/src/services/CopyService.js b/platform/entanglement/src/services/CopyService.js index d62eb2a0ed..48ba3e7ce5 100644 --- a/platform/entanglement/src/services/CopyService.js +++ b/platform/entanglement/src/services/CopyService.js @@ -64,6 +64,12 @@ define( return self.perform(domainObject, parent); } + if (!this.validate(domainObject, parent)) { + throw new Error( + "Tried to copy objects without validating first." + ); + } + if (domainObject.hasCapability('composition')) { model.composition = []; } diff --git a/platform/entanglement/src/services/LinkService.js b/platform/entanglement/src/services/LinkService.js index da4a89e33c..465a07f3de 100644 --- a/platform/entanglement/src/services/LinkService.js +++ b/platform/entanglement/src/services/LinkService.js @@ -53,15 +53,18 @@ define( }; LinkService.prototype.perform = function (object, parentObject) { - // It is assumed here that validate has been called, and therefore - // that parentObject.hasCapability('composition'). - var composition = parentObject.getCapability('composition'); + if (!this.validate(object, parentObject)) { + throw new Error( + "Tried to link objects without validating first." + ); + } - return composition.add(object).then(function (objectInNewContext) { - return parentObject.getCapability('persistence') - .persist() - .then(function () { return objectInNewContext; }); - }); + return parentObject.getCapability('composition').add(object) + .then(function (objectInNewContext) { + return parentObject.getCapability('persistence') + .persist() + .then(function () { return objectInNewContext; }); + }); }; return LinkService; diff --git a/platform/entanglement/src/services/MoveService.js b/platform/entanglement/src/services/MoveService.js index 30c341ef22..608163310f 100644 --- a/platform/entanglement/src/services/MoveService.js +++ b/platform/entanglement/src/services/MoveService.js @@ -82,6 +82,12 @@ define( } } + if (!this.validate(object, parentObject)) { + throw new Error( + "Tried to move objects without validating first." + ); + } + return this.linkService .perform(object, parentObject) .then(relocate) From 6996883b85d8ac4fd270e7c1f6485f4bb1a02c77 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Tue, 22 Sep 2015 15:42:39 -0700 Subject: [PATCH 17/23] [Entanglement] Update specs Update specs for move/copy/link services to account for re-validation at time an action is performed. nasa/openmctweb#98 --- .../src/actions/AbstractComposeAction.js | 3 +- .../test/services/CopyServiceSpec.js | 39 ++++++--- .../test/services/LinkServiceSpec.js | 2 +- .../test/services/MoveServiceSpec.js | 86 ++++++++++--------- 4 files changed, 76 insertions(+), 54 deletions(-) diff --git a/platform/entanglement/src/actions/AbstractComposeAction.js b/platform/entanglement/src/actions/AbstractComposeAction.js index 6d747bc764..f68391adc9 100644 --- a/platform/entanglement/src/actions/AbstractComposeAction.js +++ b/platform/entanglement/src/actions/AbstractComposeAction.js @@ -44,7 +44,8 @@ define( * @method platform/entanglement.AbstractComposeService#perform */ /** - * Check if one object can be composed into another. + * Check if this composition change is valid for these objects. + * * @param {DomainObject} domainObject the domain object to * move, copy, or link. * @param {DomainObject} parent the domain object whose composition diff --git a/platform/entanglement/test/services/CopyServiceSpec.js b/platform/entanglement/test/services/CopyServiceSpec.js index 6d7b1d5069..31c8dbccf4 100644 --- a/platform/entanglement/test/services/CopyServiceSpec.js +++ b/platform/entanglement/test/services/CopyServiceSpec.js @@ -41,19 +41,24 @@ define( } describe("CopyService", function () { + var policyService; + + beforeEach(function () { + policyService = jasmine.createSpyObj( + 'policyService', + ['allow'] + ); + policyService.allow.andReturn(true); + }); + describe("validate", function () { - var policyService, - copyService, + var copyService, object, parentCandidate, validate; beforeEach(function () { - policyService = jasmine.createSpyObj( - 'policyService', - ['allow'] - ); copyService = new CopyService( null, null, @@ -148,7 +153,7 @@ define( ); createObjectPromise = synchronousPromise(undefined); creationService.createObject.andReturn(createObjectPromise); - copyService = new CopyService(null, creationService); + copyService = new CopyService(null, creationService, policyService); copyResult = copyService.perform(object, newParent); copyFinished = jasmine.createSpy('copyFinished'); copyResult.then(copyFinished); @@ -180,7 +185,8 @@ define( }); describe("on domainObject with composition", function () { - var childObject, + var newObject, + childObject, compositionCapability, compositionPromise; @@ -216,6 +222,17 @@ define( composition: compositionCapability } }); + newObject = domainObjectFactory({ + name: 'object', + id: 'abc2', + model: { + name: 'some object', + composition: [] + }, + capabilities: { + composition: compositionCapability + } + }); newParent = domainObjectFactory({ name: 'newParent', id: '456', @@ -227,9 +244,11 @@ define( 'creationService', ['createObject'] ); - createObjectPromise = synchronousPromise(undefined); + policyService.allow.andReturn(true); + + createObjectPromise = synchronousPromise(newObject); creationService.createObject.andReturn(createObjectPromise); - copyService = new CopyService(mockQ, creationService); + copyService = new CopyService(mockQ, creationService, policyService); copyResult = copyService.perform(object, newParent); copyFinished = jasmine.createSpy('copyFinished'); copyResult.then(copyFinished); diff --git a/platform/entanglement/test/services/LinkServiceSpec.js b/platform/entanglement/test/services/LinkServiceSpec.js index 69c6ebd48c..5a92119e81 100644 --- a/platform/entanglement/test/services/LinkServiceSpec.js +++ b/platform/entanglement/test/services/LinkServiceSpec.js @@ -41,6 +41,7 @@ define( 'policyService', ['allow'] ); + mockPolicyService.allow.andReturn(true); linkService = new LinkService(mockPolicyService); }); @@ -66,7 +67,6 @@ define( validate = function () { return linkService.validate(object, parentCandidate); }; - mockPolicyService.allow.andReturn(true); }); it("does not allow invalid parentCandidate", function () { diff --git a/platform/entanglement/test/services/MoveServiceSpec.js b/platform/entanglement/test/services/MoveServiceSpec.js index 02494a0afb..16eaa9dcbf 100644 --- a/platform/entanglement/test/services/MoveServiceSpec.js +++ b/platform/entanglement/test/services/MoveServiceSpec.js @@ -40,58 +40,57 @@ define( var moveService, policyService, + object, + objectContextCapability, + currentParent, + parentCandidate, linkService; beforeEach(function () { + objectContextCapability = jasmine.createSpyObj( + 'objectContextCapability', + [ + 'getParent' + ] + ); + + object = domainObjectFactory({ + name: 'object', + id: 'a', + capabilities: { + context: objectContextCapability, + type: { type: 'object' } + } + }); + + currentParent = domainObjectFactory({ + name: 'currentParent', + id: 'b' + }); + + objectContextCapability.getParent.andReturn(currentParent); + + parentCandidate = domainObjectFactory({ + name: 'parentCandidate', + model: { composition: [] }, + id: 'c', + capabilities: { + type: { type: 'parentCandidate' } + } + }); policyService = jasmine.createSpyObj( 'policyService', ['allow'] ); linkService = new MockLinkService(); + policyService.allow.andReturn(true); moveService = new MoveService(policyService, linkService); }); describe("validate", function () { - var object, - objectContextCapability, - currentParent, - parentCandidate, - validate; + var validate; beforeEach(function () { - - objectContextCapability = jasmine.createSpyObj( - 'objectContextCapability', - [ - 'getParent' - ] - ); - - object = domainObjectFactory({ - name: 'object', - id: 'a', - capabilities: { - context: objectContextCapability, - type: { type: 'object' } - } - }); - - currentParent = domainObjectFactory({ - name: 'currentParent', - id: 'b' - }); - - objectContextCapability.getParent.andReturn(currentParent); - - parentCandidate = domainObjectFactory({ - name: 'parentCandidate', - model: { composition: [] }, - id: 'c', - capabilities: { - type: { type: 'parentCandidate' } - } - }); - validate = function () { return moveService.validate(object, parentCandidate); }; @@ -145,14 +144,15 @@ define( describe("perform", function () { - var object, - newParent, - actionCapability, + var actionCapability, locationCapability, locationPromise, + newParent, moveResult; beforeEach(function () { + newParent = parentCandidate; + actionCapability = jasmine.createSpyObj( 'actionCapability', ['perform'] @@ -175,7 +175,9 @@ define( name: 'object', capabilities: { action: actionCapability, - location: locationCapability + location: locationCapability, + context: objectContextCapability, + type: { type: 'object' } } }); From 4743833f7c9c0e19a8b3734abcc04a054e6c4697 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Tue, 22 Sep 2015 15:49:22 -0700 Subject: [PATCH 18/23] [Entanglement] Verify revalidation Verify that move/copy/link revalidate when performed and throw errors when moving/copying/linking would be invalid. nasa/openmctweb#98 --- .../entanglement/test/services/CopyServiceSpec.js | 14 ++++++++++++++ .../entanglement/test/services/LinkServiceSpec.js | 11 +++++++++++ .../entanglement/test/services/MoveServiceSpec.js | 11 +++++++++++ 3 files changed, 36 insertions(+) diff --git a/platform/entanglement/test/services/CopyServiceSpec.js b/platform/entanglement/test/services/CopyServiceSpec.js index 31c8dbccf4..0df6663bd7 100644 --- a/platform/entanglement/test/services/CopyServiceSpec.js +++ b/platform/entanglement/test/services/CopyServiceSpec.js @@ -285,6 +285,20 @@ define( }); }); + it("throws an expection when performed on invalid inputs", function () { + var copyService = + new CopyService(mockQ, creationService, policyService); + + function perform() { + copyService.perform(object, newParent); + } + + policyService.allow.andReturn(true); + expect(perform).not.toThrow(); + policyService.allow.andReturn(false); // Cause validate to fail + expect(perform).toThrow(); + }); + }); }); } diff --git a/platform/entanglement/test/services/LinkServiceSpec.js b/platform/entanglement/test/services/LinkServiceSpec.js index 5a92119e81..37e58c97d8 100644 --- a/platform/entanglement/test/services/LinkServiceSpec.js +++ b/platform/entanglement/test/services/LinkServiceSpec.js @@ -220,6 +220,17 @@ define( compositionPromise.resolve([linkedObject]); expect(whenComplete).toHaveBeenCalledWith(linkedObject); }); + + it("throws an expection when performed on invalid inputs", function () { + function perform() { + linkService.perform(object, parentObject); + } + + mockPolicyService.allow.andReturn(true); + expect(perform).not.toThrow(); + mockPolicyService.allow.andReturn(false); // Cause validate to fail + expect(perform).toThrow(); + }); }); }); } diff --git a/platform/entanglement/test/services/MoveServiceSpec.js b/platform/entanglement/test/services/MoveServiceSpec.js index 16eaa9dcbf..7bf7d0372f 100644 --- a/platform/entanglement/test/services/MoveServiceSpec.js +++ b/platform/entanglement/test/services/MoveServiceSpec.js @@ -196,6 +196,17 @@ define( .toHaveBeenCalledWith(jasmine.any(Function)); }); + it("throws an expection when performed on invalid inputs", function () { + function perform() { + moveService.perform(object, newParent); + } + + policyService.allow.andReturn(true); + expect(perform).not.toThrow(); + policyService.allow.andReturn(false); // Cause validate to fail + expect(perform).toThrow(); + }); + describe("when moving an original", function () { beforeEach(function () { locationCapability.getContextualLocation From 8db334e45bcb81aaa8e9c5fd79bd362fd95685b7 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Tue, 22 Sep 2015 15:51:15 -0700 Subject: [PATCH 19/23] Revert "[LinkService] Reorder/refactor validation" This reverts commit c17269ba8bc7cff3ee2481d1b2c9b14bbba7c34f. --- .../entanglement/src/services/LinkService.js | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/platform/entanglement/src/services/LinkService.js b/platform/entanglement/src/services/LinkService.js index 465a07f3de..40d809ebcb 100644 --- a/platform/entanglement/src/services/LinkService.js +++ b/platform/entanglement/src/services/LinkService.js @@ -39,17 +39,20 @@ define( } LinkService.prototype.validate = function (object, parentCandidate) { - var objectId = object.getId(); - return !!parentCandidate && - !!parentCandidate.getId && - parentCandidate.getId() !== objectId && - parentCandidate.hasCapability("composition") && - parentCandidate.getModel().composition.indexOf(objectId) === -1 && - this.policyService.allow( - "composition", - parentCandidate.getCapability('type'), - object.getCapability('type') - ); + if (!parentCandidate || !parentCandidate.getId) { + return false; + } + if (parentCandidate.getId() === object.getId()) { + return false; + } + if ((parentCandidate.getModel().composition || []).indexOf(object.getId()) !== -1) { + return false; + } + return this.policyService.allow( + "composition", + parentCandidate.getCapability('type'), + object.getCapability('type') + ) && parentCandidate.hasCapability('composition'); }; LinkService.prototype.perform = function (object, parentObject) { From 97892869ae7f48e7bbfaaf7d47f0932ecc979902 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Tue, 22 Sep 2015 15:53:27 -0700 Subject: [PATCH 20/23] [Entanglement] Move composition check up Move check for composition up higher in the sequence of if-blocks in the validation step of linking; remove fallback to [] for undefined composition, which is no longer necessary as a consequence of this. nasa/openmctweb#98 --- platform/entanglement/src/services/LinkService.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/platform/entanglement/src/services/LinkService.js b/platform/entanglement/src/services/LinkService.js index 40d809ebcb..5989c3cf06 100644 --- a/platform/entanglement/src/services/LinkService.js +++ b/platform/entanglement/src/services/LinkService.js @@ -45,14 +45,17 @@ define( if (parentCandidate.getId() === object.getId()) { return false; } - if ((parentCandidate.getModel().composition || []).indexOf(object.getId()) !== -1) { + if (!parentCandidate.hasCapability('composition')) { + return false; + } + if (parentCandidate.getModel().composition.indexOf(object.getId()) !== -1) { return false; } return this.policyService.allow( "composition", parentCandidate.getCapability('type'), object.getCapability('type') - ) && parentCandidate.hasCapability('composition'); + ); }; LinkService.prototype.perform = function (object, parentObject) { From 6dbccd50009e4431a0e6d334ca0919b013315173 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Wed, 23 Sep 2015 12:43:03 -0700 Subject: [PATCH 21/23] [Entanglement] Update test cases around error-throwing ...per code review feedback, nasa/openmctweb#97 --- .../test/services/CopyServiceSpec.js | 57 ++++++++++++------- .../test/services/LinkServiceSpec.js | 9 +-- .../test/services/MoveServiceSpec.js | 9 +-- 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/platform/entanglement/test/services/CopyServiceSpec.js b/platform/entanglement/test/services/CopyServiceSpec.js index 0df6663bd7..b32eda40b6 100644 --- a/platform/entanglement/test/services/CopyServiceSpec.js +++ b/platform/entanglement/test/services/CopyServiceSpec.js @@ -131,6 +131,15 @@ define( copyResult, copyFinished; + beforeEach(function () { + creationService = jasmine.createSpyObj( + 'creationService', + ['createObject'] + ); + createObjectPromise = synchronousPromise(undefined); + creationService.createObject.andReturn(createObjectPromise); + }); + describe("on domain object without composition", function () { beforeEach(function () { object = domainObjectFactory({ @@ -147,12 +156,6 @@ define( composition: [] } }); - creationService = jasmine.createSpyObj( - 'creationService', - ['createObject'] - ); - createObjectPromise = synchronousPromise(undefined); - creationService.createObject.andReturn(createObjectPromise); copyService = new CopyService(null, creationService, policyService); copyResult = copyService.perform(object, newParent); copyFinished = jasmine.createSpy('copyFinished'); @@ -240,10 +243,6 @@ define( composition: [] } }); - creationService = jasmine.createSpyObj( - 'creationService', - ['createObject'] - ); policyService.allow.andReturn(true); createObjectPromise = synchronousPromise(newObject); @@ -285,18 +284,36 @@ define( }); }); - it("throws an expection when performed on invalid inputs", function () { - var copyService = - new CopyService(mockQ, creationService, policyService); + describe("on invalid inputs", function () { + beforeEach(function () { + object = domainObjectFactory({ + name: 'object', + capabilities: { + type: { type: 'object' } + } + }); + newParent = domainObjectFactory({ + name: 'parentCandidate', + capabilities: { + type: { type: 'parentCandidate' } + } + }); + }); - function perform() { - copyService.perform(object, newParent); - } + it("throws an error", function () { + var copyService = + new CopyService(mockQ, creationService, policyService); - policyService.allow.andReturn(true); - expect(perform).not.toThrow(); - policyService.allow.andReturn(false); // Cause validate to fail - expect(perform).toThrow(); + function perform() { + copyService.perform(object, newParent); + } + + spyOn(copyService, "validate"); + copyService.validate.andReturn(true); + expect(perform).not.toThrow(); + copyService.validate.andReturn(false); + expect(perform).toThrow(); + }); }); }); diff --git a/platform/entanglement/test/services/LinkServiceSpec.js b/platform/entanglement/test/services/LinkServiceSpec.js index 37e58c97d8..24ecf7a801 100644 --- a/platform/entanglement/test/services/LinkServiceSpec.js +++ b/platform/entanglement/test/services/LinkServiceSpec.js @@ -20,7 +20,7 @@ * at runtime from the About dialog for additional information. *****************************************************************************/ -/*global define,describe,beforeEach,it,jasmine,expect */ +/*global define,describe,beforeEach,it,jasmine,expect,spyOn */ define( [ @@ -221,14 +221,15 @@ define( expect(whenComplete).toHaveBeenCalledWith(linkedObject); }); - it("throws an expection when performed on invalid inputs", function () { + it("throws an error when performed on invalid inputs", function () { function perform() { linkService.perform(object, parentObject); } - mockPolicyService.allow.andReturn(true); + spyOn(linkService, 'validate'); + linkService.validate.andReturn(true); expect(perform).not.toThrow(); - mockPolicyService.allow.andReturn(false); // Cause validate to fail + linkService.validate.andReturn(false); expect(perform).toThrow(); }); }); diff --git a/platform/entanglement/test/services/MoveServiceSpec.js b/platform/entanglement/test/services/MoveServiceSpec.js index 7bf7d0372f..0eb9943dd8 100644 --- a/platform/entanglement/test/services/MoveServiceSpec.js +++ b/platform/entanglement/test/services/MoveServiceSpec.js @@ -20,7 +20,7 @@ * at runtime from the About dialog for additional information. *****************************************************************************/ -/*global define,describe,beforeEach,it,jasmine,expect */ +/*global define,describe,beforeEach,it,jasmine,expect,spyOn */ define( [ '../../src/services/MoveService', @@ -196,14 +196,15 @@ define( .toHaveBeenCalledWith(jasmine.any(Function)); }); - it("throws an expection when performed on invalid inputs", function () { + it("throws an error when performed on invalid inputs", function () { function perform() { moveService.perform(object, newParent); } - policyService.allow.andReturn(true); + spyOn(moveService, "validate"); + moveService.validate.andReturn(true); expect(perform).not.toThrow(); - policyService.allow.andReturn(false); // Cause validate to fail + moveService.validate.andReturn(false); expect(perform).toThrow(); }); From 082122ddeccc4fedd2634864b94382d74fd492dc Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Wed, 23 Sep 2015 12:46:23 -0700 Subject: [PATCH 22/23] [Entanglement] Move policyService.allow.andReturn ...to locations where the specified return value can be more closely correlated to the relevant test cases. nasa/openmctweb#98 --- platform/entanglement/test/services/CopyServiceSpec.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/platform/entanglement/test/services/CopyServiceSpec.js b/platform/entanglement/test/services/CopyServiceSpec.js index b32eda40b6..2788fcefa8 100644 --- a/platform/entanglement/test/services/CopyServiceSpec.js +++ b/platform/entanglement/test/services/CopyServiceSpec.js @@ -48,7 +48,6 @@ define( 'policyService', ['allow'] ); - policyService.allow.andReturn(true); }); describe("validate", function () { @@ -138,6 +137,7 @@ define( ); createObjectPromise = synchronousPromise(undefined); creationService.createObject.andReturn(createObjectPromise); + policyService.allow.andReturn(true); }); describe("on domain object without composition", function () { @@ -243,7 +243,6 @@ define( composition: [] } }); - policyService.allow.andReturn(true); createObjectPromise = synchronousPromise(newObject); creationService.createObject.andReturn(createObjectPromise); From fa60d620396822a4e61c1f2f3c9a56134f5da8d1 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen <victor.woeltjen@nasa.gov> Date: Wed, 23 Sep 2015 12:49:04 -0700 Subject: [PATCH 23/23] [Entanglement] Simplify test case ...per review feedback from nasa/openmctweb#98 --- platform/entanglement/test/services/LinkServiceSpec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/platform/entanglement/test/services/LinkServiceSpec.js b/platform/entanglement/test/services/LinkServiceSpec.js index 24ecf7a801..3d63c2c030 100644 --- a/platform/entanglement/test/services/LinkServiceSpec.js +++ b/platform/entanglement/test/services/LinkServiceSpec.js @@ -97,7 +97,6 @@ define( parentCandidate.hasCapability.andCallFake(function (c) { return c !== 'composition'; }); - parentCandidate.model.composition = undefined; expect(validate()).toBe(false); });