From fe600de0f711fa17920384335f19ca405c3d97b5 Mon Sep 17 00:00:00 2001 From: Henry Date: Wed, 2 Dec 2015 11:28:49 -0800 Subject: [PATCH 01/15] [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 02/15] #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 03/15] #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 04/15] #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 05/15] 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) From 3b427c31a29c71f4e406a46c415f4a57ef765059 Mon Sep 17 00:00:00 2001 From: Henry Date: Wed, 2 Dec 2015 19:18:50 -0800 Subject: [PATCH 06/15] Fixed error with not properly referenced --- platform/entanglement/src/services/CopyTask.js | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/platform/entanglement/src/services/CopyTask.js b/platform/entanglement/src/services/CopyTask.js index 1f2db40f3b..1a47d8d5c7 100644 --- a/platform/entanglement/src/services/CopyTask.js +++ b/platform/entanglement/src/services/CopyTask.js @@ -104,14 +104,10 @@ define( function addClonesToParent(self) { var parentClone = self.clones[self.clones.length-1]; - //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 + 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;}); } /** @@ -175,7 +171,7 @@ define( }); } else { //Creating a link, no need to iterate children - return $q.when(originalObject); + return self.$q.when(originalObject); } From 734e979c9427fc7cc1afd3e917bbb41ec1013819 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 3 Dec 2015 12:49:54 -0800 Subject: [PATCH 07/15] #338 Fixed failing tests after refactor --- platform/entanglement/bundle.json | 3 +- .../entanglement/src/services/CopyService.js | 7 +- .../entanglement/src/services/CopyTask.js | 13 +- .../test/services/CopyServiceSpec.js | 220 ++++++++++-------- 4 files changed, 135 insertions(+), 108 deletions(-) diff --git a/platform/entanglement/bundle.json b/platform/entanglement/bundle.json index fc17ef9ef2..9679aec4db 100644 --- a/platform/entanglement/bundle.json +++ b/platform/entanglement/bundle.json @@ -89,8 +89,7 @@ "name": "Copy Service", "description": "Provides a service for copying objects", "implementation": "services/CopyService.js", - "depends": ["$q", "creationService", "policyService", - "persistenceService", "now"] + "depends": ["$q", "policyService", "now"] }, { "key": "locationService", diff --git a/platform/entanglement/src/services/CopyService.js b/platform/entanglement/src/services/CopyService.js index ad0617526f..03f32b76ce 100644 --- a/platform/entanglement/src/services/CopyService.js +++ b/platform/entanglement/src/services/CopyService.js @@ -38,12 +38,9 @@ define( * @memberof platform/entanglement * @implements {platform/entanglement.AbstractComposeService} */ - function CopyService($q, creationService, policyService, persistenceService, now) { + function CopyService($q, policyService) { this.$q = $q; - this.creationService = creationService; this.policyService = policyService; - this.persistenceService = persistenceService; - this.now = now; } CopyService.prototype.validate = function (object, parentCandidate) { @@ -71,7 +68,7 @@ define( */ CopyService.prototype.perform = function (domainObject, parent) { var $q = this.$q, - copyTask = new CopyTask(domainObject, parent, this.persistenceService, this.policyService, this.$q, this.now); + copyTask = new CopyTask(domainObject, parent, this.policyService, this.$q); 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 1a47d8d5c7..d3790bf433 100644 --- a/platform/entanglement/src/services/CopyTask.js +++ b/platform/entanglement/src/services/CopyTask.js @@ -33,21 +33,16 @@ define( * * @param domainObject The object to copy * @param parent The new location of the cloned object tree - * @param persistenceService * @param $q - * @param now * @constructor */ - function CopyTask (domainObject, parent, persistenceService, policyService, $q, now){ + function CopyTask (domainObject, parent, policyService, $q){ 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; this.clones = []; } @@ -154,7 +149,7 @@ define( // 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())); + clone = this.parent.hasCapability("instantiation") && this.parent.useCapability("instantiation", cloneObjectModel(originalObject.getModel())); //Iterate through child tree return this.$q.when(originalObject.useCapability('composition')).then(function(composees){ @@ -193,7 +188,7 @@ define( var self = this; return this.copy(self.domainObject, self.parent).then(function(domainObjectClone){ - domainObjectClone.model.location = self.parent.getId(); + domainObjectClone.getModel().location = self.parent.getId(); return self; }); }; @@ -207,7 +202,7 @@ define( this.deferred = this.$q.defer(); if (!this.parent.hasCapability('composition')){ - return self.$q.reject(); + return this.$q.reject(); } this.buildCopyPlan() diff --git a/platform/entanglement/test/services/CopyServiceSpec.js b/platform/entanglement/test/services/CopyServiceSpec.js index 391d90913c..c10efb1522 100644 --- a/platform/entanglement/test/services/CopyServiceSpec.js +++ b/platform/entanglement/test/services/CopyServiceSpec.js @@ -63,7 +63,6 @@ define( beforeEach(function () { copyService = new CopyService( - null, null, policyService ); @@ -130,48 +129,52 @@ define( creationService, createObjectPromise, copyService, - mockPersistenceService, mockNow, object, newParent, copyResult, copyFinished, persistObjectPromise, - parentPersistenceCapability, + persistenceCapability, + instantiationCapability, + compositionCapability, + locationCapability, resolvedValue; beforeEach(function () { - creationService = jasmine.createSpyObj( - 'creationService', - ['createObject'] - ); createObjectPromise = synchronousPromise(undefined); - creationService.createObject.andReturn(createObjectPromise); policyService.allow.andReturn(true); - - mockPersistenceService = jasmine.createSpyObj( - 'persistenceService', - ['createObject', 'updateObject'] - ); + persistObjectPromise = synchronousPromise(undefined); - mockPersistenceService.createObject.andReturn(persistObjectPromise); - mockPersistenceService.updateObject.andReturn(persistObjectPromise); - - parentPersistenceCapability = jasmine.createSpyObj( - "persistence", + + instantiationCapability = jasmine.createSpyObj( + "instantiation", + [ "invoke" ] + ); + + persistenceCapability = jasmine.createSpyObj( + "persistenceCapability", [ "persist", "getSpace" ] ); + persistenceCapability.persist.andReturn(persistObjectPromise); - parentPersistenceCapability.persist.andReturn(persistObjectPromise); - parentPersistenceCapability.getSpace.andReturn("testSpace"); + compositionCapability = jasmine.createSpyObj( + 'compositionCapability', + ['invoke', 'add'] + ); - mockNow = jasmine.createSpyObj("mockNow", ["now"]); - mockNow.now.andCallFake(function(){ - return 1234; - }); + locationCapability = jasmine.createSpyObj( + 'locationCapability', + ['isLink'] + ); + locationCapability.isLink.andReturn(false); - mockDeferred = jasmine.createSpyObj('mockDeferred', ['notify', 'resolve']); + mockDeferred = jasmine.createSpyObj( + 'mockDeferred', + ['notify', 'resolve', 'reject'] + ); mockDeferred.notify.andCallFake(function(notification){}); + mockDeferred.reject.andCallFake(function(){}); mockDeferred.resolve.andCallFake(function(value){resolvedValue = value;}); mockDeferred.promise = { then: function(callback){ @@ -179,7 +182,11 @@ define( } }; - mockQ = jasmine.createSpyObj('mockQ', ['when', 'all', 'reject', 'defer']); + mockQ = jasmine.createSpyObj( + 'mockQ', + ['when', 'all', 'reject', 'defer'] + ); + mockQ.reject.andReturn(synchronousPromise(undefined)); mockQ.when.andCallFake(synchronousPromise); mockQ.all.andCallFake(function (promises) { var result = {}; @@ -194,6 +201,8 @@ define( describe("on domain object without composition", function () { beforeEach(function () { + var objectCopy; + newParent = domainObjectFactory({ name: 'newParent', id: '456', @@ -201,7 +210,9 @@ define( composition: [] }, capabilities: { - persistence: parentPersistenceCapability + instantiation: instantiationCapability, + persistence: persistenceCapability, + composition: compositionCapability } }); @@ -210,31 +221,46 @@ define( id: 'abc', model: { name: 'some object', - location: newParent.id, - persisted: mockNow.now() + location: '456', + someOtherAttribute: 'some other value', + embeddedObjectAttribute: { + name: 'Some embedded object' + } + }, + capabilities: { + persistence: persistenceCapability } }); - - copyService = new CopyService(mockQ, creationService, policyService, mockPersistenceService, mockNow.now); + + objectCopy = domainObjectFactory({ + name: 'object', + id: 'abc.copy.fdgdfgdf', + capabilities: { + persistence: persistenceCapability, + location: locationCapability + } + }); + + instantiationCapability.invoke.andCallFake( + function(model){ + objectCopy.model = model; + return objectCopy; + } + ); + + copyService = new CopyService(mockQ, policyService); copyResult = copyService.perform(object, newParent); copyFinished = jasmine.createSpy('copyFinished'); copyResult.then(copyFinished); }); - it("uses persistence service", function () { - expect(mockPersistenceService.createObject) - .toHaveBeenCalledWith(parentPersistenceCapability.getSpace(), jasmine.any(String), object.getModel()); - - expect(persistObjectPromise.then) - .toHaveBeenCalledWith(jasmine.any(Function)); - }); + it("uses persistence capability", function () { + expect(persistenceCapability.persist) + .toHaveBeenCalled(); + }); it("deep clones object model", function () { - //var newModel = creationService - var newModel = mockPersistenceService - .createObject - .mostRecentCall - .args[2]; + var newModel = copyFinished.calls[0].args[0].getModel(); expect(newModel).toEqual(object.model); expect(newModel).not.toBe(object.model); }); @@ -249,27 +275,57 @@ define( describe("on domainObject with composition", function () { var newObject, childObject, - compositionCapability, - locationCapability, + objectClone, + childObjectClone, compositionPromise; beforeEach(function () { + var invocationCount = 0, + objectClones; + instantiationCapability.invoke.andCallFake( + function(model){ + var cloneToReturn = objectClones[invocationCount++]; + cloneToReturn.model = model; + return cloneToReturn; + } + ); - locationCapability = jasmine.createSpyObj('locationCapability', ['isLink']); - locationCapability.isLink.andReturn(true); + newParent = domainObjectFactory({ + name: 'newParent', + id: '456', + model: { + composition: [] + }, + capabilities: { + instantiation: instantiationCapability, + persistence: persistenceCapability, + composition: compositionCapability + } + }); childObject = domainObjectFactory({ name: 'childObject', id: 'def', model: { - name: 'a child object' + name: 'a child object', + location: 'abc' + }, + capabilities: { + persistence: persistenceCapability, + location: locationCapability } }); - compositionCapability = jasmine.createSpyObj( - 'compositionCapability', - ['invoke', 'add'] - ); + + childObjectClone = domainObjectFactory({ + name: 'childObject', + id: 'def.clone', + capabilities: { + persistence: persistenceCapability, + location: locationCapability + } + }); + compositionPromise = jasmine.createSpyObj( 'compositionPromise', ['then'] @@ -280,7 +336,7 @@ define( .andReturn(synchronousPromise([childObject])); object = domainObjectFactory({ - name: 'object', + name: 'some object', id: 'abc', model: { name: 'some object', @@ -288,36 +344,27 @@ define( location: 'testLocation' }, capabilities: { + instantiation: instantiationCapability, composition: compositionCapability, - location: locationCapability - } - }); - newObject = domainObjectFactory({ - name: 'object', - id: 'abc2', - model: { - name: 'some object', - composition: [] - }, - capabilities: { - composition: compositionCapability - } - }); - newParent = domainObjectFactory({ - name: 'newParent', - id: '456', - model: { - composition: [] - }, - capabilities: { - composition: compositionCapability, - persistence: parentPersistenceCapability + location: locationCapability, + persistence: persistenceCapability } }); - createObjectPromise = synchronousPromise(newObject); - creationService.createObject.andReturn(createObjectPromise); - copyService = new CopyService(mockQ, creationService, policyService, mockPersistenceService, mockNow.now); + objectClone = domainObjectFactory({ + name: 'some object', + id: 'abc.clone', + capabilities: { + instantiation: instantiationCapability, + composition: compositionCapability, + location: locationCapability, + persistence: persistenceCapability + } + }); + + objectClones = [objectClone, childObjectClone]; + + copyService = new CopyService(mockQ, policyService); }); describe("the cloning process", function(){ @@ -327,24 +374,13 @@ define( copyResult.then(copyFinished); }); - it("copies object and children in a bottom-up" + - " fashion", function () { - expect(mockPersistenceService.createObject.calls[0].args[2].name).toEqual(childObject.model.name); - expect(mockPersistenceService.createObject.calls[1].args[2].name).toEqual(object.model.name); - }); - it("returns a promise", function () { expect(copyResult.then).toBeDefined(); expect(copyFinished).toHaveBeenCalled(); }); - it("clears modified and sets persisted", function () { - expect(copyFinished.mostRecentCall.args[0].model.modified).toBeUndefined(); - expect(copyFinished.mostRecentCall.args[0].model.persisted).toBe(mockNow.now()); - }); - it ("correctly locates cloned objects", function() { - expect(mockPersistenceService.createObject.calls[0].args[2].location).toEqual(mockPersistenceService.createObject.calls[1].args[1]); + expect(childObjectClone.getModel().location).toEqual(objectClone.getId()); }); }); @@ -368,7 +404,7 @@ define( it("throws an error", function () { var copyService = - new CopyService(mockQ, creationService, policyService, mockPersistenceService, mockNow.now); + new CopyService(mockQ, policyService); function perform() { copyService.perform(object, newParent); From 6aab9f4e34a7b1399f9594f2ee655f33466c9320 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 3 Dec 2015 14:47:42 -0800 Subject: [PATCH 08/15] Added test for linking --- .../test/services/CopyServiceSpec.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/platform/entanglement/test/services/CopyServiceSpec.js b/platform/entanglement/test/services/CopyServiceSpec.js index c10efb1522..a6815a314a 100644 --- a/platform/entanglement/test/services/CopyServiceSpec.js +++ b/platform/entanglement/test/services/CopyServiceSpec.js @@ -379,10 +379,28 @@ define( expect(copyFinished).toHaveBeenCalled(); }); + it("returns a promise", function () { + expect(copyResult.then).toBeDefined(); + expect(copyFinished).toHaveBeenCalled(); + }); + it ("correctly locates cloned objects", function() { expect(childObjectClone.getModel().location).toEqual(objectClone.getId()); }); + }); + describe("when cloning non-creatable objects", function() { + beforeEach(function () { + policyService.allow.callFake(function(category, object){ + return category === 'creation'; + }); + copyResult = copyService.perform(object, newParent); + copyFinished = jasmine.createSpy('copyFinished'); + copyResult.then(copyFinished); + }); + it ("creates links", function() { + expect(childObjectClone.getModel().location).toEqual(objectClone.getId()); + }); }); }); From baec0f971909092edcfe1e952cd1d3582de2262c Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 3 Dec 2015 15:28:07 -0800 Subject: [PATCH 09/15] #338 added new test case for creation of links when object type is not createable --- platform/entanglement/src/services/CopyTask.js | 14 ++++++++------ .../entanglement/test/services/CopyServiceSpec.js | 12 ++++++++---- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/platform/entanglement/src/services/CopyTask.js b/platform/entanglement/src/services/CopyTask.js index d3790bf433..a12bdf2f11 100644 --- a/platform/entanglement/src/services/CopyTask.js +++ b/platform/entanglement/src/services/CopyTask.js @@ -39,6 +39,7 @@ define( function CopyTask (domainObject, parent, policyService, $q){ this.domainObject = domainObject; this.parent = parent; + this.firstClone = undefined; this.$q = $q; this.deferred = undefined; this.policyService = policyService; @@ -97,12 +98,10 @@ define( * Will add a list of clones to the specified parent's composition */ function addClonesToParent(self) { - var parentClone = self.clones[self.clones.length-1]; - - return parentClone.getCapability("persistence").persist() - .then(function(){self.parent.getCapability("composition").add(parentClone.getId())}) + return self.firstClone.getCapability("persistence").persist() + .then(function(){self.parent.getCapability("composition").add(self.firstClone.getId())}) .then(function(){return self.parent.getCapability("persistence").persist();}) - .then(function(){return parentClone;}); + .then(function(){return self.firstClone;}); } /** @@ -188,7 +187,10 @@ define( var self = this; return this.copy(self.domainObject, self.parent).then(function(domainObjectClone){ - domainObjectClone.getModel().location = self.parent.getId(); + if (domainObjectClone !== self.domainObject) { + domainObjectClone.getModel().location = self.parent.getId(); + } + self.firstClone = domainObjectClone; return self; }); }; diff --git a/platform/entanglement/test/services/CopyServiceSpec.js b/platform/entanglement/test/services/CopyServiceSpec.js index a6815a314a..d4de5aa2aa 100644 --- a/platform/entanglement/test/services/CopyServiceSpec.js +++ b/platform/entanglement/test/services/CopyServiceSpec.js @@ -390,16 +390,20 @@ define( }); describe("when cloning non-creatable objects", function() { beforeEach(function () { - policyService.allow.callFake(function(category, object){ - return category === 'creation'; + policyService.allow.andCallFake(function(category){ + //Return false for 'creation' policy + return category !== 'creation'; }); copyResult = copyService.perform(object, newParent); copyFinished = jasmine.createSpy('copyFinished'); copyResult.then(copyFinished); }); - it ("creates links", function() { - expect(childObjectClone.getModel().location).toEqual(objectClone.getId()); + it ("creates link instead of clone", function() { + var copiedObject = copyFinished.calls[0].args[0]; + expect(copiedObject).toBe(object); + expect(compositionCapability.add).toHaveBeenCalledWith(copiedObject.getId()); + //expect(newParent.getModel().composition).toContain(copiedObject.getId()); }); }); }); From 6e391098a309dbdf7956b439cf063aba8c513b2b Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 3 Dec 2015 15:32:26 -0800 Subject: [PATCH 10/15] Fixed jslint error --- platform/entanglement/src/services/CopyTask.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/entanglement/src/services/CopyTask.js b/platform/entanglement/src/services/CopyTask.js index a12bdf2f11..93b7bc6180 100644 --- a/platform/entanglement/src/services/CopyTask.js +++ b/platform/entanglement/src/services/CopyTask.js @@ -99,7 +99,7 @@ define( */ function addClonesToParent(self) { return self.firstClone.getCapability("persistence").persist() - .then(function(){self.parent.getCapability("composition").add(self.firstClone.getId())}) + .then(function(){self.parent.getCapability("composition").add(self.firstClone.getId());}) .then(function(){return self.parent.getCapability("persistence").persist();}) .then(function(){return self.firstClone;}); } From 96249e6bcc2ad16ea3ee670cf9d615808651bfdc Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 3 Dec 2015 15:51:27 -0800 Subject: [PATCH 11/15] Removed redundant line in test spec --- platform/entanglement/test/services/CopyServiceSpec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/platform/entanglement/test/services/CopyServiceSpec.js b/platform/entanglement/test/services/CopyServiceSpec.js index d4de5aa2aa..f3530d37ef 100644 --- a/platform/entanglement/test/services/CopyServiceSpec.js +++ b/platform/entanglement/test/services/CopyServiceSpec.js @@ -174,7 +174,6 @@ define( ['notify', 'resolve', 'reject'] ); mockDeferred.notify.andCallFake(function(notification){}); - mockDeferred.reject.andCallFake(function(){}); mockDeferred.resolve.andCallFake(function(value){resolvedValue = value;}); mockDeferred.promise = { then: function(callback){ From f8099550bd81511cd41a0ce3e4cccb11e5770a69 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 3 Dec 2015 16:22:45 -0800 Subject: [PATCH 12/15] [Copy] #338 updated code style in CreationPolicy --- .../browse/src/creation/CreationPolicy.js | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/platform/commonUI/browse/src/creation/CreationPolicy.js b/platform/commonUI/browse/src/creation/CreationPolicy.js index 0ccf82d907..28749e711f 100644 --- a/platform/commonUI/browse/src/creation/CreationPolicy.js +++ b/platform/commonUI/browse/src/creation/CreationPolicy.js @@ -27,23 +27,19 @@ define( "use strict"; /** - * A policy for determining whether objects of a certain type can be + * A policy for determining whether objects of a given type can be * created. - * @returns {{allow: Function}} * @constructor + * @implements {Policy} + * @memberof platform/commonUI/browse */ function CreationPolicy() { - return { - /** - * Only allow creation of object types that have the - * Creation capability - */ - allow: function (type) { - return type.hasFeature("creation"); - } - }; } + CreationPolicy.prototype.allow = function (type) { + return type.hasFeature("creation"); + }; + return CreationPolicy; } ); \ No newline at end of file From 6aa77ff4687318622eb832c569acadac6608aa0b Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 3 Dec 2015 16:41:06 -0800 Subject: [PATCH 13/15] #338 fixed failing test --- platform/entanglement/src/services/CopyTask.js | 4 ---- .../entanglement/test/services/CopyServiceSpec.js | 12 ++++++++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/platform/entanglement/src/services/CopyTask.js b/platform/entanglement/src/services/CopyTask.js index 93b7bc6180..43ec1960b4 100644 --- a/platform/entanglement/src/services/CopyTask.js +++ b/platform/entanglement/src/services/CopyTask.js @@ -203,10 +203,6 @@ define( CopyTask.prototype.perform = function(){ this.deferred = this.$q.defer(); - if (!this.parent.hasCapability('composition')){ - return this.$q.reject(); - } - this.buildCopyPlan() .then(persistObjects) .then(addClonesToParent) diff --git a/platform/entanglement/test/services/CopyServiceSpec.js b/platform/entanglement/test/services/CopyServiceSpec.js index f3530d37ef..3d4ebf147e 100644 --- a/platform/entanglement/test/services/CopyServiceSpec.js +++ b/platform/entanglement/test/services/CopyServiceSpec.js @@ -412,15 +412,23 @@ define( object = domainObjectFactory({ name: 'object', capabilities: { - type: { type: 'object' } + type: { type: 'object' }, + location: locationCapability, + persistence: persistenceCapability } }); + newParent = domainObjectFactory({ name: 'parentCandidate', capabilities: { - type: { type: 'parentCandidate' } + type: { type: 'parentCandidate' }, + instantiation: instantiationCapability, + composition: compositionCapability, + persistence: persistenceCapability } }); + + instantiationCapability.invoke.andReturn(object); }); it("throws an error", function () { From 57efe4e0d13b639d3a53adbdf6a669c2c63b4868 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 3 Dec 2015 17:10:28 -0800 Subject: [PATCH 14/15] Removed UUID reference --- platform/entanglement/src/services/CopyTask.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/platform/entanglement/src/services/CopyTask.js b/platform/entanglement/src/services/CopyTask.js index 43ec1960b4..6ac6bcc6a8 100644 --- a/platform/entanglement/src/services/CopyTask.js +++ b/platform/entanglement/src/services/CopyTask.js @@ -23,8 +23,8 @@ /*global define */ define( - ["uuid"], - function (uuid) { + [], + function () { "use strict"; /** @@ -148,7 +148,7 @@ define( // 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") && this.parent.useCapability("instantiation", cloneObjectModel(originalObject.getModel())); + clone = this.parent.useCapability("instantiation", cloneObjectModel(originalObject.getModel())); //Iterate through child tree return this.$q.when(originalObject.useCapability('composition')).then(function(composees){ From b388c76e457bae87d0b94207a34433f0d4ce54fc Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 3 Dec 2015 18:24:38 -0800 Subject: [PATCH 15/15] [copy] #338 Modified check to set object location, added setLocation flag, Removed unused parameter --- .../entanglement/src/services/CopyTask.js | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/platform/entanglement/src/services/CopyTask.js b/platform/entanglement/src/services/CopyTask.js index 6ac6bcc6a8..d20c233b42 100644 --- a/platform/entanglement/src/services/CopyTask.js +++ b/platform/entanglement/src/services/CopyTask.js @@ -47,14 +47,14 @@ define( this.clones = []; } - function composeChild(child, parent) { + function composeChild(child, parent, setLocation) { //Once copied, associate each cloned // composee with its parent clone parent.getModel().composition.push(child.getId()); - //Check if the object being composed is a link - if (!child.getCapability("location").isLink()) { + //If a location is not specified, set it. + if (setLocation && child.getModel().location === undefined) { child.getModel().location = parent.getId(); } } @@ -113,13 +113,16 @@ define( CopyTask.prototype.copyComposees = function(composees, clonedParent, originalParent){ var self = this; - return (composees || []).reduce(function(promise, composee){ + return (composees || []).reduce(function(promise, originalComposee){ //If the composee is composed of other // objects, chain a promise.. return promise.then(function(){ // ...to recursively copy it (and its children) - return self.copy(composee, originalParent).then(function(composee){ - return composeChild(composee, clonedParent); + return self.copy(originalComposee, originalParent).then(function(clonedComposee){ + //Compose the child within its parent. Cloned + // objects will need to also have their location + // set, however linked objects will not. + return composeChild(clonedComposee, clonedParent, clonedComposee !== originalComposee); }); });}, self.$q.when(undefined) ); @@ -132,11 +135,11 @@ define( * cloning objects, and composing them with their child clones * as it goes * @private - * @param originalObject - * @param originalParent - * @returns {*} + * @returns {DomainObject} If the type of the original object allows for + * duplication, then a duplicate of the object, otherwise the object + * itself (to allow linking to non duplicatable objects). */ - CopyTask.prototype.copy = function(originalObject, originalParent) { + CopyTask.prototype.copy = function(originalObject) { var self = this, clone;