Preserve connectivity when pasting

Fixes #2584

index.html

@@ -151,7 +151,7 @@
         <script src='js/id/actions/change_tags.js'></script>
         <script src='js/id/actions/circularize.js'></script>
         <script src='js/id/actions/connect.js'></script>
-        <script src='js/id/actions/copy_entity.js'></script>
+        <script src='js/id/actions/copy_entities.js'></script>
         <script src='js/id/actions/delete_member.js'></script>
         <script src='js/id/actions/delete_multiple.js'></script>
         <script src='js/id/actions/delete_node.js'></script>

js/id/actions/copy_entities.js

@@ -0,0 +1,21 @@
+iD.actions.CopyEntities = function(ids, fromGraph) {
+    var copies = {};
+
+    var action = function(graph) {
+        ids.forEach(function(id) {
+            fromGraph.entity(id).copy(fromGraph, copies);
+        });
+
+        for (var id in copies) {
+            graph = graph.replace(copies[id]);
+        }
+
+        return graph;
+    };
+
+    action.copies = function() {
+        return copies;
+    };
+
+    return action;
+};

js/id/behavior/paste.js

@@ -31,30 +31,21 @@ iD.behavior.Paste = function(context) {
         var extent = iD.geo.Extent(),
             oldIDs = context.copyIDs(),
             oldGraph = context.copyGraph(),
-            newIDs = [],
-            i, j;
+            newIDs = [];
 
         if (!oldIDs.length) return;
 
-        for (i = 0; i < oldIDs.length; i++) {
-            var oldEntity = oldGraph.entity(oldIDs[i]),
-                action = iD.actions.CopyEntity(oldEntity.id, oldGraph, true),
-                newEntities;
+        var action = iD.actions.CopyEntities(oldIDs, oldGraph);
+        context.perform(action);
 
-            extent._extend(oldEntity.extent(oldGraph));
-            context.perform(action);
-
-            // First element in `newEntities` contains the copied Entity,
-            // Subsequent array elements contain any descendants..
-            newEntities = action.newEntities();
-            newIDs.push(newEntities[0].id);
+        var copies = action.copies();
+        for (var id in copies) {
+            var oldEntity = oldGraph.entity(id),
+                newEntity = copies[id];
 
-            for (j = 0; j < newEntities.length; j++) {
-                var newEntity = newEntities[j],
-                    tags = _.omit(newEntity.tags, omitTag);
-
-                context.perform(iD.actions.ChangeTags(newEntity.id, tags));
-            }
+            extent._extend(oldEntity.extent(oldGraph));
+            newIDs.push(newEntity.id);
+            context.perform(iD.actions.ChangeTags(newEntity.id, _.omit(newEntity.tags, omitTag)));
         }
 
         // Put pasted objects where mouse pointer is..

js/id/core/entity.js

@@ -72,10 +72,14 @@ iD.Entity.prototype = {
         return this;
     },
 
-    copy: function() {
-        // Returns an array so that we can support deep copying ways and relations.
-        // The first array element will contain this.copy, followed by any descendants.
-        return [iD.Entity(this, {id: undefined, user: undefined, version: undefined})];
+    copy: function(resolver, copies) {
+        if (copies[this.id])
+            return copies[this.id];
+
+        var copy = iD.Entity(this, {id: undefined, user: undefined, version: undefined});
+        copies[this.id] = copy;
+
+        return copy;
     },
 
     osmId: function() {

js/id/core/relation.js

@@ -20,31 +20,19 @@ _.extend(iD.Relation.prototype, {
     type: 'relation',
     members: [],
 
-    copy: function(deep, resolver, replacements) {
-        var copy = iD.Entity.prototype.copy.call(this);
-        if (!deep || !resolver || !this.isComplete(resolver)) {
-            return copy;
-        }
+    copy: function(resolver, copies) {
+        if (copies[this.id])
+            return copies[this.id];
 
-        var members = [],
-            i, oldmember, oldid, newid, children;
+        var copy = iD.Entity.prototype.copy.call(this, resolver, copies);
 
-        replacements = replacements || {};
-        replacements[this.id] = copy[0].id;
+        var members = this.members.map(function(member) {
+            return _.extend({}, member, {id: resolver.entity(member.id).copy(resolver, copies).id});
+        });
 
-        for (i = 0; i < this.members.length; i++) {
-            oldmember = this.members[i];
-            oldid = oldmember.id;
-            newid = replacements[oldid];
-            if (!newid) {
-                children = resolver.entity(oldid).copy(true, resolver, replacements);
-                newid = replacements[oldid] = children[0].id;
-                copy = copy.concat(children);
-            }
-            members.push({id: newid, type: oldmember.type, role: oldmember.role});
-        }
+        copy = copy.update({members: members});
+        copies[this.id] = copy;
 
-        copy[0] = copy[0].update({members: members});
         return copy;
     },
 

js/id/core/way.js

@@ -12,29 +12,19 @@ _.extend(iD.Way.prototype, {
     type: 'way',
     nodes: [],
 
-    copy: function(deep, resolver) {
-        var copy = iD.Entity.prototype.copy.call(this);
+    copy: function(resolver, copies) {
+        if (copies[this.id])
+            return copies[this.id];
 
-        if (!deep || !resolver) {
-            return copy;
-        }
+        var copy = iD.Entity.prototype.copy.call(this, resolver, copies);
 
-        var nodes = [],
-            replacements = {},
-            i, oldid, newid, child;
-
-        for (i = 0; i < this.nodes.length; i++) {
-            oldid = this.nodes[i];
-            newid = replacements[oldid];
-            if (!newid) {
-                child = resolver.entity(oldid).copy();
-                newid = replacements[oldid] = child[0].id;
-                copy = copy.concat(child);
-            }
-            nodes.push(newid);
-        }
+        var nodes = this.nodes.map(function(id) {
+            return resolver.entity(id).copy(resolver, copies).id;
+        });
+
+        copy = copy.update({nodes: nodes});
+        copies[this.id] = copy;
 
-        copy[0] = copy[0].update({nodes: nodes});
         return copy;
     },
 

test/index.html

@@ -136,7 +136,7 @@
     <script src='../js/id/actions/change_tags.js'></script>
     <script src='../js/id/actions/circularize.js'></script>
     <script src='../js/id/actions/connect.js'></script>
-    <script src='../js/id/actions/copy_entity.js'></script>
+    <script src='../js/id/actions/copy_entities.js'></script>
     <script src='../js/id/actions/delete_member.js'></script>
     <script src='../js/id/actions/delete_multiple.js'></script>
     <script src='../js/id/actions/delete_node.js'></script>
@@ -246,7 +246,7 @@
     <script src='spec/actions/orthogonalize.js'></script>
     <script src='spec/actions/straighten.js'></script>
     <script src='spec/actions/connect.js'></script>
-    <script src="spec/actions/copy_entity.js"></script>
+    <script src="spec/actions/copy_entities.js"></script>
     <script src='spec/actions/delete_member.js'></script>
     <script src="spec/actions/delete_multiple.js"></script>
     <script src="spec/actions/delete_node.js"></script>

test/index_packaged.html

@@ -42,7 +42,7 @@
     <script src="spec/actions/change_tags.js"></script>
     <script src='spec/actions/circularize.js'></script>
     <script src='spec/actions/connect.js'></script>
-    <script src="spec/actions/copy_entity.js"></script>
+    <script src="spec/actions/copy_entities.js"></script>
     <script src='spec/actions/delete_member.js'></script>
     <script src="spec/actions/delete_multiple.js"></script>
     <script src="spec/actions/delete_node.js"></script>

test/spec/actions/copy_entities.js

@@ -0,0 +1,71 @@
+describe("iD.actions.CopyEntities", function () {
+    it("copies a node", function () {
+        var a = iD.Node({id: 'a'}),
+            base = iD.Graph([a]),
+            head = iD.actions.CopyEntities(['a'], base)(base),
+            diff = iD.Difference(base, head),
+            created = diff.created();
+
+        expect(head.hasEntity('a')).to.be.ok;
+        expect(created).to.have.length(1);
+    });
+
+    it("copies a way", function () {
+        var a = iD.Node({id: 'a'}),
+            b = iD.Node({id: 'b'}),
+            w = iD.Way({id: 'w', nodes: ['a', 'b']}),
+            base = iD.Graph([a, b, w]),
+            action = iD.actions.CopyEntities(['w'], base),
+            head = action(base),
+            diff = iD.Difference(base, head),
+            created = diff.created();
+
+        expect(head.hasEntity('w')).to.be.ok;
+        expect(created).to.have.length(3);
+    });
+
+    it("copies multiple nodes", function () {
+        var base = iD.Graph([
+                iD.Node({id: 'a'}),
+                iD.Node({id: 'b'})
+            ]),
+            action = iD.actions.CopyEntities(['a', 'b'], base),
+            head = action(base),
+            diff = iD.Difference(base, head),
+            created = diff.created();
+
+        expect(head.hasEntity('a')).to.be.ok;
+        expect(head.hasEntity('b')).to.be.ok;
+        expect(created).to.have.length(2);
+    });
+
+    it("copies multiple ways, keeping the same connections", function () {
+        var base = iD.Graph([
+                iD.Node({id: 'a'}),
+                iD.Node({id: 'b'}),
+                iD.Node({id: 'c'}),
+                iD.Way({id: 'w1', nodes: ['a', 'b']}),
+                iD.Way({id: 'w2', nodes: ['b', 'c']})
+            ]),
+            action = iD.actions.CopyEntities(['w1', 'w2'], base),
+            head = action(base),
+            diff = iD.Difference(base, head),
+            created = diff.created();
+
+        expect(created).to.have.length(5);
+        expect(action.copies().w1.nodes[1]).to.eql(action.copies().w2.nodes[0]);
+    });
+
+    it("obtains source entities from an alternate graph", function () {
+        var a = iD.Node({id: 'a'}),
+            old = iD.Graph([a]),
+            base = iD.Graph(),
+            action = iD.actions.CopyEntities(['a'], old),
+            head = action(base),
+            diff = iD.Difference(base, head),
+            created = diff.created();
+
+        expect(head.hasEntity('a')).not.to.be.ok;
+        expect(Object.keys(action.copies())).to.have.length(1);
+    });
+});