Merge branch 'master' of https://github.com/nasa/openmctweb into open431

platform/entanglement/src/services/CopyTask.js

@@ -58,6 +58,8 @@ define(
             if (setLocation && child.getModel().location === undefined) {
                 child.getModel().location = parent.getId();
             }
+
+            return child;
         }
 
         function cloneObjectModel(objectModel) {
@@ -111,9 +113,7 @@ define(
          * copying.
          * @private
          */
-        CopyTask.prototype.rewriteIdentifiers = function (obj) {
+        CopyTask.prototype.rewriteIdentifiers = function (obj, idMap) {
-            var idMap = this.idMap;
-
             if (Array.isArray(obj)) {
                 obj.forEach(function (value, index) {
                     obj[index] = idMap[value] || value;
@@ -127,18 +127,11 @@ define(
                         delete obj[key];
                         obj[idMap[key]] = value;
                     }
-                    this.rewriteIdentifiers(value);
+                    this.rewriteIdentifiers(value, idMap);
                 }, this);
             }
         };
 
-        function rewriteIdentifiersInClones(self) {
-            self.clones.forEach(function (clone) {
-                self.rewriteIdentifiers(clone.getModel());
-            });
-            return self;
-        }
-
         /**
          * Given an array of objects composed by a parent, clone them, then
          * add them to the parent.
@@ -146,7 +139,8 @@ define(
          * @returns {*}
          */
         CopyTask.prototype.copyComposees = function(composees, clonedParent, originalParent){
-            var self = this;
+            var self = this,
+                idMap = {};
 
             return (composees || []).reduce(function(promise, originalComposee){
                 //If the composee is composed of other
@@ -154,13 +148,28 @@ define(
                 return promise.then(function(){
                     // ...to recursively copy it (and its children)
                     return self.copy(originalComposee, originalParent).then(function(clonedComposee){
+                        //Map the original composee's ID to that of its
+                        // clone so that we can replace any references to it
+                        // in the parent
+                        idMap[originalComposee.getId()] = clonedComposee.getId();
+
                         //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)
-            );
+            ).then(function(){
+                    //Replace any references in the cloned parent to
+                    // contained objects that have been composed with the
+                    // Ids of the clones
+                    self.rewriteIdentifiers(clonedParent.getModel(), idMap);
+
+                    //Add the clone to the list of clones that will
+                    //be returned by this function
+                    self.clones.push(clonedParent);
+                    return clonedParent;
+            });
         };
 
         /**
@@ -188,21 +197,13 @@ define(
                 // space is used.
                 clone = this.parent.useCapability("instantiation", cloneObjectModel(originalObject.getModel()));
 
-                // Record ID mappings so we can rewrite properties later
-                self.idMap[originalObject.getId()] = clone.getId();
-
                 //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 (){
+                    return self.copyComposees(composees, clone, originalObject);
-                        //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
@@ -245,7 +246,6 @@ define(
             this.deferred = this.$q.defer();
 
             this.buildCopyPlan()
-                .then(rewriteIdentifiersInClones)
                 .then(persistObjects)
                 .then(addClonesToParent)
                 .then(this.deferred.resolve, this.deferred.reject);

platform/entanglement/test/services/CopyTaskSpec.js

@@ -143,12 +143,7 @@ define(
                     mockDeferred.promise = synchronousPromise(value);
                 });
 
-                task = new CopyTask(
+
-                    mockDomainObject,
-                    mockParentObject,
-                    mockPolicyService,
-                    mockQ
-                );
             });
 
 
@@ -156,6 +151,13 @@ define(
                 var model;
 
                 beforeEach(function () {
+                    task = new CopyTask(
+                        mockDomainObject,
+                        mockParentObject,
+                        mockPolicyService,
+                        mockQ
+                    );
+
                     task.perform().then(function (clone) {
                         model = clone.getModel();
                     });
@@ -180,6 +182,68 @@ define(
                 });
             });
 
+            describe("copies object trees with multiple references to the" +
+                " same object", function () {
+                var model,
+                    mockDomainObjectB,
+                    mockComposingObject,
+                    composingObjectModel,
+                    domainObjectClone,
+                    domainObjectBClone;
+
+                beforeEach(function () {
+                    mockDomainObjectB = domainObjectFactory({
+                        capabilities: makeMockCapabilities(testModel.composition),
+                        model: testModel
+                    });
+                    composingObjectModel = {
+                        name: 'mockComposingObject',
+                        composition: [mockDomainObject.getId(), mockDomainObjectB.getId()]
+                    };
+                    mockComposingObject = domainObjectFactory({
+                        capabilities: makeMockCapabilities(composingObjectModel.composition),
+                        model: composingObjectModel
+                    });
+
+                    mockComposingObject.capabilities.composition.invoke.andReturn([mockDomainObject, mockDomainObjectB]);
+                    task = new CopyTask(
+                        mockComposingObject,
+                        mockParentObject,
+                        mockPolicyService,
+                        mockQ
+                    );
+
+                    task.perform();
+                    domainObjectClone = task.clones[2];
+                    domainObjectBClone = task.clones[5];
+                });
+
+                /**
+                 * mockDomainObject and mockDomainObjectB have the same
+                 * model with references to children ID_A and ID_B. Expect
+                 * that after duplication the references should differ
+                 * because they are each now referencing different child
+                 * objects. This tests the issue reported in #428
+                 */
+                it(" and correctly updates child identifiers in models ", function () {
+                    var childA_ID = task.clones[0].getId(),
+                        childB_ID = task.clones[1].getId(),
+                        childC_ID = task.clones[3].getId(),
+                        childD_ID = task.clones[4].getId();
+
+                    expect(domainObjectClone.model.someArr[0]).toNotBe(domainObjectBClone.model.someArr[0]);
+                    expect(domainObjectClone.model.someArr[0]).toBe(childA_ID);
+                    expect(domainObjectBClone.model.someArr[0]).toBe(childC_ID);
+                    expect(domainObjectClone.model.someArr[1]).toNotBe(domainObjectBClone.model.someArr[1]);
+                    expect(domainObjectClone.model.someArr[1]).toBe(childB_ID);
+                    expect(domainObjectBClone.model.someArr[1]).toBe(childD_ID);
+                    expect(domainObjectClone.model.someObj.someProperty).toNotBe(domainObjectBClone.model.someObj.someProperty);
+                    expect(domainObjectClone.model.someObj.someProperty).toBe(childB_ID);
+                    expect(domainObjectBClone.model.someObj.someProperty).toBe(childD_ID);
+
+                });
+            });
+
         });