From c8d77bc2dbe168a7e7b0a505950d1088ea38d0c8 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 30 Nov 2015 15:26:00 -0800 Subject: [PATCH 1/6] [Entanglement] Allow cross-space linking Allow links across spaces; only disallow move or copy. Addresses WTD-1587 --- platform/entanglement/src/policies/CrossSpacePolicy.js | 4 +--- platform/entanglement/test/policies/CrossSpacePolicySpec.js | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/platform/entanglement/src/policies/CrossSpacePolicy.js b/platform/entanglement/src/policies/CrossSpacePolicy.js index a113972815..c47e653f08 100644 --- a/platform/entanglement/src/policies/CrossSpacePolicy.js +++ b/platform/entanglement/src/policies/CrossSpacePolicy.js @@ -28,9 +28,7 @@ define( var DISALLOWED_ACTIONS = [ "move", - "copy", - "link", - "compose" + "copy" ]; /** diff --git a/platform/entanglement/test/policies/CrossSpacePolicySpec.js b/platform/entanglement/test/policies/CrossSpacePolicySpec.js index 214efc1cc3..a0b030f04f 100644 --- a/platform/entanglement/test/policies/CrossSpacePolicySpec.js +++ b/platform/entanglement/test/policies/CrossSpacePolicySpec.js @@ -72,7 +72,7 @@ define( policy = new CrossSpacePolicy(); }); - ['move', 'copy', 'link', 'compose'].forEach(function (key) { + ['move', 'copy'].forEach(function (key) { describe("for " + key + " actions", function () { beforeEach(function () { testActionMetadata.key = key; From fe600de0f711fa17920384335f19ca405c3d97b5 Mon Sep 17 00:00:00 2001 From: Henry Date: Wed, 2 Dec 2015 11:28:49 -0800 Subject: [PATCH 2/6] [Copy] #338 added CreationPolicy and appropriate tests, amended CreateActionProvider, and updated existing tests --- platform/commonUI/browse/bundle.json | 6 +++ .../src/creation/CreateActionProvider.js | 2 +- .../browse/src/creation/CreationPolicy.js | 49 +++++++++++++++++ .../test/creation/CreateActionProviderSpec.js | 29 ++++++++-- .../test/creation/CreationPolicySpec.js | 54 +++++++++++++++++++ platform/commonUI/browse/test/suite.json | 1 + 6 files changed, 136 insertions(+), 5 deletions(-) create mode 100644 platform/commonUI/browse/src/creation/CreationPolicy.js create mode 100644 platform/commonUI/browse/test/creation/CreationPolicySpec.js diff --git a/platform/commonUI/browse/bundle.json b/platform/commonUI/browse/bundle.json index 0d4c1eedd6..6954bba12c 100644 --- a/platform/commonUI/browse/bundle.json +++ b/platform/commonUI/browse/bundle.json @@ -102,6 +102,12 @@ "implementation": "navigation/NavigationService.js" } ], + "policies": [ + { + "implementation": "CreationPolicy.js", + "category": "creation" + } + ], "actions": [ { "key": "navigate", diff --git a/platform/commonUI/browse/src/creation/CreateActionProvider.js b/platform/commonUI/browse/src/creation/CreateActionProvider.js index 4ca2bce59f..4152d9a27f 100644 --- a/platform/commonUI/browse/src/creation/CreateActionProvider.js +++ b/platform/commonUI/browse/src/creation/CreateActionProvider.js @@ -69,7 +69,7 @@ define( // Introduce one create action per type return this.typeService.listTypes().filter(function (type) { - return type.hasFeature("creation"); + return self.policyService.allow("creation", type); }).map(function (type) { return new CreateAction( type, diff --git a/platform/commonUI/browse/src/creation/CreationPolicy.js b/platform/commonUI/browse/src/creation/CreationPolicy.js new file mode 100644 index 0000000000..a30a78ab8b --- /dev/null +++ b/platform/commonUI/browse/src/creation/CreationPolicy.js @@ -0,0 +1,49 @@ +/***************************************************************************** + * Open MCT Web, Copyright (c) 2014-2015, United States Government + * as represented by the Administrator of the National Aeronautics and Space + * Administration. All rights reserved. + * + * Open MCT Web is licensed under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0. + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + * Open MCT Web includes source code licensed under additional open source + * licenses. See the Open Source Licenses file (LICENSES.md) included with + * this source code distribution or the Licensing information page available + * at runtime from the About dialog for additional information. + *****************************************************************************/ +/*global define*/ + +define( + [], + function () { + "use strict"; + + /** + * A policy for determining whether objects of a certain type can be + * created. + * @returns {{allow: Function}} + * @constructor + */ + function CreationPolicy() { + return { + /** + * Only allow creation of object types that have the + * Creation capability + */ + allow: function (action, type) { + return type.hasFeature("creation"); + } + }; + } + + return CreationPolicy; + } +); \ No newline at end of file diff --git a/platform/commonUI/browse/test/creation/CreateActionProviderSpec.js b/platform/commonUI/browse/test/creation/CreateActionProviderSpec.js index 431f6101c2..fdba091623 100644 --- a/platform/commonUI/browse/test/creation/CreateActionProviderSpec.js +++ b/platform/commonUI/browse/test/creation/CreateActionProviderSpec.js @@ -33,6 +33,9 @@ define( var mockTypeService, mockDialogService, mockCreationService, + mockPolicyService, + mockCreationPolicy, + mockPolicyMap = {}, mockTypes, provider; @@ -67,14 +70,32 @@ define( "creationService", [ "createObject" ] ); + mockPolicyService = jasmine.createSpyObj( + "policyService", + [ "allow" ] + ); + mockTypes = [ "A", "B", "C" ].map(createMockType); + mockTypes.forEach(function(type){ + mockPolicyMap[type.getName()] = true; + }); + + mockCreationPolicy = function(type){ + return mockPolicyMap[type.getName()]; + }; + + mockPolicyService.allow.andCallFake(function(category, type){ + return category === "creation" && mockCreationPolicy(type) ? true : false; + }); + mockTypeService.listTypes.andReturn(mockTypes); provider = new CreateActionProvider( mockTypeService, mockDialogService, - mockCreationService + mockCreationService, + mockPolicyService ); }); @@ -94,15 +115,15 @@ define( it("does not expose non-creatable types", function () { // One of the types won't have the creation feature... - mockTypes[1].hasFeature.andReturn(false); + mockPolicyMap[mockTypes[0].getName()] = false; // ...so it should have been filtered out. expect(provider.getActions({ key: "create", domainObject: {} }).length).toEqual(2); // Make sure it was creation which was used to check - expect(mockTypes[1].hasFeature) - .toHaveBeenCalledWith("creation"); + expect(mockPolicyService.allow) + .toHaveBeenCalledWith("creation", mockTypes[0]); }); }); } diff --git a/platform/commonUI/browse/test/creation/CreationPolicySpec.js b/platform/commonUI/browse/test/creation/CreationPolicySpec.js new file mode 100644 index 0000000000..41afee0875 --- /dev/null +++ b/platform/commonUI/browse/test/creation/CreationPolicySpec.js @@ -0,0 +1,54 @@ +/***************************************************************************** + * Open MCT Web, Copyright (c) 2014-2015, United States Government + * as represented by the Administrator of the National Aeronautics and Space + * Administration. All rights reserved. + * + * Open MCT Web is licensed under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0. + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + * Open MCT Web includes source code licensed under additional open source + * licenses. See the Open Source Licenses file (LICENSES.md) included with + * this source code distribution or the Licensing information page available + * at runtime from the About dialog for additional information. + *****************************************************************************/ +/*global define,describe,it,expect,beforeEach,jasmine*/ + +define( + ["../../src/creation/CreationPolicy"], + function (CreationPolicy) { + "use strict"; + + describe("The creation policy", function () { + var mockDomainObject, + mockType, + policy; + + beforeEach(function () { + mockType = jasmine.createSpyObj( + 'type', + ['hasFeature'] + ); + + policy = new CreationPolicy(); + }); + + it("allows creation of types with the creation feature", function () { + mockType.hasFeature.andReturn(true); + expect(policy.allow({}, mockType)).toBeTruthy(); + }); + + it("disallows creation of types without the creation feature", function () { + mockType.hasFeature.andReturn(false); + expect(policy.allow({}, mockType)).toBeFalsy(); + }); + }); + } +); \ No newline at end of file diff --git a/platform/commonUI/browse/test/suite.json b/platform/commonUI/browse/test/suite.json index b9292b6ef1..70e62bcfa6 100644 --- a/platform/commonUI/browse/test/suite.json +++ b/platform/commonUI/browse/test/suite.json @@ -8,6 +8,7 @@ "creation/CreateMenuController", "creation/CreateWizard", "creation/CreationService", + "creation/CreationPolicy", "creation/LocatorController", "navigation/NavigateAction", "navigation/NavigationService", From 9953e16415740b52a704aea63f6e744d1920493e Mon Sep 17 00:00:00 2001 From: Henry Date: Wed, 2 Dec 2015 14:06:22 -0800 Subject: [PATCH 3/6] #338 Copy now using target persistence space --- platform/entanglement/src/services/CopyTask.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/platform/entanglement/src/services/CopyTask.js b/platform/entanglement/src/services/CopyTask.js index f484856448..719785e17b 100644 --- a/platform/entanglement/src/services/CopyTask.js +++ b/platform/entanglement/src/services/CopyTask.js @@ -44,6 +44,7 @@ define( this.$q = $q; this.deferred = undefined; this.persistenceService = persistenceService; + this.persistenceSpace = parent.getCapability("persistence") && parent.getCapability("persistence").getSpace(); this.persisted = 0; this.now = now; this.clones = []; @@ -52,6 +53,10 @@ define( function composeChild(child, parent) { //Once copied, associate each cloned // composee with its parent clone + + //Could check islink here, and not set the location if it is a + // link? Object should have been contextualized during + // composition, so isLink should work. child.model.location = parent.id; parent.model.composition = parent.model.composition || []; return parent.model.composition.push(child.id); @@ -76,7 +81,7 @@ define( return self.$q.all(self.clones.map(function(clone){ clone.model.persisted = self.now(); - return self.persistenceService.createObject(clone.persistenceSpace, clone.id, clone.model) + return self.persistenceService.createObject(self.persistenceSpace, clone.id, clone.model) .then(function(){ self.deferred.notify({phase: "copying", totalObjects: self.clones.length, processed: ++self.persisted}); }); @@ -96,7 +101,7 @@ define( } return self.persistenceService - .updateObject(parentClone.persistenceSpace, parentClone.id, parentClone.model) + .updateObject(self.persistenceSpace, parentClone.id, parentClone.model) .then(function(){return self.parent.getCapability("composition").add(parentClone.id);}) .then(function(){return self.parent.getCapability("persistence").persist();}) .then(function(){return parentClone;}); @@ -138,15 +143,16 @@ define( CopyTask.prototype.copy = function(originalObject, originalParent) { var self = this, modelClone = { - id: uuid(), - model: cloneObjectModel(originalObject.getModel()), - persistenceSpace: originalParent.hasCapability('persistence') && originalParent.getCapability('persistence').getSpace() - }; + id: uuid(), + model: cloneObjectModel(originalObject.getModel()) + }, + clone; return this.$q.when(originalObject.useCapability('composition')).then(function(composees){ self.deferred.notify({phase: "preparing"}); //Duplicate the object's children, and their children, and // so on down to the leaf nodes of the tree. + //If it is a link, don't both with children return self.copyComposees(composees, modelClone, originalObject).then(function (){ //Add the clone to the list of clones that will //be returned by this function From 11d8daf3edc291bb4cad809e9a0fb35b870f9401 Mon Sep 17 00:00:00 2001 From: Henry Date: Wed, 2 Dec 2015 14:07:08 -0800 Subject: [PATCH 4/6] #338 Fixed incorrect specification of CreationPolicy in bundle.json --- platform/commonUI/browse/bundle.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/commonUI/browse/bundle.json b/platform/commonUI/browse/bundle.json index 6954bba12c..e290f296ff 100644 --- a/platform/commonUI/browse/bundle.json +++ b/platform/commonUI/browse/bundle.json @@ -104,7 +104,7 @@ ], "policies": [ { - "implementation": "CreationPolicy.js", + "implementation": "creation/CreationPolicy.js", "category": "creation" } ], From 8e3c5db3bfa07a0618a3e4550863b5862b540d10 Mon Sep 17 00:00:00 2001 From: Henry Date: Wed, 2 Dec 2015 14:14:04 -0800 Subject: [PATCH 5/6] #338 fixed incorrect 'allow' function specification in CreationPolicy --- platform/commonUI/browse/src/creation/CreationPolicy.js | 2 +- .../commonUI/browse/test/creation/CreationPolicySpec.js | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/platform/commonUI/browse/src/creation/CreationPolicy.js b/platform/commonUI/browse/src/creation/CreationPolicy.js index a30a78ab8b..0ccf82d907 100644 --- a/platform/commonUI/browse/src/creation/CreationPolicy.js +++ b/platform/commonUI/browse/src/creation/CreationPolicy.js @@ -38,7 +38,7 @@ define( * Only allow creation of object types that have the * Creation capability */ - allow: function (action, type) { + allow: function (type) { return type.hasFeature("creation"); } }; diff --git a/platform/commonUI/browse/test/creation/CreationPolicySpec.js b/platform/commonUI/browse/test/creation/CreationPolicySpec.js index 41afee0875..1f88c1b149 100644 --- a/platform/commonUI/browse/test/creation/CreationPolicySpec.js +++ b/platform/commonUI/browse/test/creation/CreationPolicySpec.js @@ -27,8 +27,7 @@ define( "use strict"; describe("The creation policy", function () { - var mockDomainObject, - mockType, + var mockType, policy; beforeEach(function () { @@ -42,12 +41,12 @@ define( it("allows creation of types with the creation feature", function () { mockType.hasFeature.andReturn(true); - expect(policy.allow({}, mockType)).toBeTruthy(); + expect(policy.allow(mockType)).toBeTruthy(); }); it("disallows creation of types without the creation feature", function () { mockType.hasFeature.andReturn(false); - expect(policy.allow({}, mockType)).toBeFalsy(); + expect(policy.allow(mockType)).toBeFalsy(); }); }); } From cee0ecf0efd445a9e6225e0fe132cd14e3f75b0a Mon Sep 17 00:00:00 2001 From: Henry Date: Wed, 2 Dec 2015 19:10:10 -0800 Subject: [PATCH 6/6] Removed use of composition and mutation because they trigger the search indexer too early and it tries to retrieve objects that have not been persisted yet --- .../entanglement/src/services/CopyService.js | 2 +- .../entanglement/src/services/CopyTask.js | 101 +++++++++++------- 2 files changed, 63 insertions(+), 40 deletions(-) diff --git a/platform/entanglement/src/services/CopyService.js b/platform/entanglement/src/services/CopyService.js index 29c6be4d10..ad0617526f 100644 --- a/platform/entanglement/src/services/CopyService.js +++ b/platform/entanglement/src/services/CopyService.js @@ -71,7 +71,7 @@ define( */ CopyService.prototype.perform = function (domainObject, parent) { var $q = this.$q, - copyTask = new CopyTask(domainObject, parent, this.persistenceService, this.$q, this.now); + copyTask = new CopyTask(domainObject, parent, this.persistenceService, this.policyService, this.$q, this.now); if (this.validate(domainObject, parent)) { return copyTask.perform(); } else { diff --git a/platform/entanglement/src/services/CopyTask.js b/platform/entanglement/src/services/CopyTask.js index 719785e17b..1f2db40f3b 100644 --- a/platform/entanglement/src/services/CopyTask.js +++ b/platform/entanglement/src/services/CopyTask.js @@ -38,12 +38,13 @@ define( * @param now * @constructor */ - function CopyTask (domainObject, parent, persistenceService, $q, now){ + function CopyTask (domainObject, parent, persistenceService, policyService, $q, now){ this.domainObject = domainObject; this.parent = parent; this.$q = $q; this.deferred = undefined; this.persistenceService = persistenceService; + this.policyService = policyService; this.persistenceSpace = parent.getCapability("persistence") && parent.getCapability("persistence").getSpace(); this.persisted = 0; this.now = now; @@ -54,20 +55,30 @@ define( //Once copied, associate each cloned // composee with its parent clone - //Could check islink here, and not set the location if it is a - // link? Object should have been contextualized during - // composition, so isLink should work. - child.model.location = parent.id; - parent.model.composition = parent.model.composition || []; - return parent.model.composition.push(child.id); + parent.getModel().composition.push(child.getId()); + + //Check if the object being composed is a link + if (!child.getCapability("location").isLink()) { + child.getModel().location = parent.getId(); + } } function cloneObjectModel(objectModel) { var clone = JSON.parse(JSON.stringify(objectModel)); - delete clone.composition; + /** + * Reset certain fields. + */ + //If has a composition, set it to an empty array. Will be + // recomposed later with the ids of its cloned children. + if (clone.composition) { + //Important to set it to an empty array here, otherwise + // hasCapability("composition") returns false; + clone.composition = []; + } delete clone.persisted; delete clone.modified; + delete clone.location; return clone; } @@ -78,13 +89,10 @@ define( * result in automatic request batching by the browser. */ function persistObjects(self) { - return self.$q.all(self.clones.map(function(clone){ - clone.model.persisted = self.now(); - return self.persistenceService.createObject(self.persistenceSpace, clone.id, clone.model) - .then(function(){ - self.deferred.notify({phase: "copying", totalObjects: self.clones.length, processed: ++self.persisted}); - }); + return clone.getCapability("persistence").persist().then(function(){ + self.deferred.notify({phase: "copying", totalObjects: self.clones.length, processed: ++self.persisted}); + }); })).then(function(){ return self; }); @@ -96,15 +104,13 @@ define( function addClonesToParent(self) { var parentClone = self.clones[self.clones.length-1]; - if (!self.parent.hasCapability('composition')){ - return self.$q.reject(); - } - - return self.persistenceService - .updateObject(self.persistenceSpace, parentClone.id, parentClone.model) - .then(function(){return self.parent.getCapability("composition").add(parentClone.id);}) - .then(function(){return self.parent.getCapability("persistence").persist();}) - .then(function(){return parentClone;}); + //self.persistenceService + // .updateObject(self.persistenceSpace, + // parentClone.id, parentClone.model) + return parentClone.getCapability("persistence").persist() + .then(function(){self.parent.getCapability("composition").add(parentClone.getId())}) + .then(function(){return self.parent.getCapability("persistence").persist();}) + .then(function(){return parentClone;}); // Ensure the clone of the original domainObject is returned } @@ -123,7 +129,7 @@ define( return promise.then(function(){ // ...to recursively copy it (and its children) return self.copy(composee, originalParent).then(function(composee){ - composeChild(composee, clonedParent); + return composeChild(composee, clonedParent); }); });}, self.$q.when(undefined) ); @@ -142,24 +148,37 @@ define( */ CopyTask.prototype.copy = function(originalObject, originalParent) { var self = this, - modelClone = { - id: uuid(), - model: cloneObjectModel(originalObject.getModel()) - }, clone; - return this.$q.when(originalObject.useCapability('composition')).then(function(composees){ - self.deferred.notify({phase: "preparing"}); - //Duplicate the object's children, and their children, and - // so on down to the leaf nodes of the tree. - //If it is a link, don't both with children - return self.copyComposees(composees, modelClone, originalObject).then(function (){ - //Add the clone to the list of clones that will - //be returned by this function - self.clones.push(modelClone); - return modelClone; + //Check if the type of the object being copied allows for + // creation of new instances. If it does not, then a link to the + // original will be created instead. + if (this.policyService.allow("creation", originalObject.getCapability("type"))){ + //create a new clone of the original object. Use the + // creation capability of the targetParent to create the + // new clone. This will ensure that the correct persistence + // space is used. + clone = this.parent.hasCapability("instantiation") && originalParent.useCapability("instantiation", cloneObjectModel(originalObject.getModel())); + + //Iterate through child tree + return this.$q.when(originalObject.useCapability('composition')).then(function(composees){ + self.deferred.notify({phase: "preparing"}); + //Duplicate the object's children, and their children, and + // so on down to the leaf nodes of the tree. + //If it is a link, don't both with children + return self.copyComposees(composees, clone, originalObject).then(function (){ + //Add the clone to the list of clones that will + //be returned by this function + self.clones.push(clone); + return clone; + }); }); - }); + } else { + //Creating a link, no need to iterate children + return $q.when(originalObject); + } + + }; /** @@ -191,6 +210,10 @@ define( CopyTask.prototype.perform = function(){ this.deferred = this.$q.defer(); + if (!this.parent.hasCapability('composition')){ + return self.$q.reject(); + } + this.buildCopyPlan() .then(persistObjects) .then(addClonesToParent)