From 90147b23dbf314f975c2cae56b97b9f8508984ac Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@7thposition.com>
Date: Sun, 4 Jan 2015 23:15:47 -0500
Subject: [PATCH] Make copy handle circular and other degenerate Relations

---
 js/id/core/relation.js     |  9 +++---
 test/spec/core/relation.js | 58 ++++++++++++++++++++++++++++++++++----
 2 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/js/id/core/relation.js b/js/id/core/relation.js
index 174904cad..761db2a65 100644
--- a/js/id/core/relation.js
+++ b/js/id/core/relation.js
@@ -20,23 +20,24 @@ _.extend(iD.Relation.prototype, {
     type: 'relation',
     members: [],
 
-    copy: function(deep, resolver) {
+    copy: function(deep, resolver, replacements) {
         var copy = iD.Entity.prototype.copy.call(this);
-
         if (!deep || !resolver || !this.isComplete(resolver)) {
             return copy;
         }
 
         var members = [],
-            replacements = {},
             i, oldmember, oldid, newid, children;
 
+        replacements = replacements || {};
+        replacements[this.id] = copy[0].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);
+                children = resolver.entity(oldid).copy(true, resolver, replacements);
                 newid = replacements[oldid] = children[0].id;
                 copy = copy.concat(children);
             }
diff --git a/test/spec/core/relation.js b/test/spec/core/relation.js
index cd13d2dd4..8588f3781 100644
--- a/test/spec/core/relation.js
+++ b/test/spec/core/relation.js
@@ -45,10 +45,10 @@ describe('iD.Relation', function () {
                 r1 = iD.Relation({id: 'r1', members: [{id: 'w1', role: 'outer'}]}),
                 graph = iD.Graph([a, b, c, w1, r1]),
                 result = r1.copy(),
-                r2 = result[0];
+                r1_copy = result[0];
 
             expect(result).to.have.length(1);
-            expect(r1.members).to.deep.equal(r2.members);
+            expect(r1.members).to.deep.equal(r1_copy.members);
         });
 
         it("makes new members when deep = true", function () {
@@ -59,7 +59,7 @@ describe('iD.Relation', function () {
                 r1 = iD.Relation({id: 'r1', members: [{id: 'w1', role: 'outer'}]}),
                 graph = iD.Graph([a, b, c, w1, r1]),
                 result = r1.copy(true, graph),
-                r2 = result[0];
+                r1_copy = result[0];
 
             expect(result).to.have.length(5);
             expect(result[0]).to.be.an.instanceof(iD.Relation);
@@ -68,9 +68,57 @@ describe('iD.Relation', function () {
             expect(result[3]).to.be.an.instanceof(iD.Node);
             expect(result[4]).to.be.an.instanceof(iD.Node);
 
-            expect(r2.members[0].id).not.to.equal(r1.members[0].id);
-            expect(r2.members[0].role).to.equal(r1.members[0].role);
+            expect(r1_copy.members[0].id).not.to.equal(r1.members[0].id);
+            expect(r1_copy.members[0].role).to.equal(r1.members[0].role);
         });
+
+        it("deep copies non-tree relation graphs without duplicating children", function () {
+            var w = iD.Way({id: 'w'}),
+                r1 = iD.Relation({id: 'r1', members: [{id: 'r2'}, {id: 'w'}]}),
+                r2 = iD.Relation({id: 'r2', members: [{id: 'w'}]}),
+                graph = iD.Graph([w, r1, r2]),
+                result = r1.copy(true, graph),
+                r1_copy = result[0],
+                r2_copy = result[1],
+                w_copy = result[2];
+
+            expect(result).to.have.length(3);
+            expect(r1_copy).to.be.an.instanceof(iD.Relation);
+            expect(r2_copy).to.be.an.instanceof(iD.Relation);
+            expect(w_copy).to.be.an.instanceof(iD.Way);
+
+            expect(r1_copy.members[0].id).to.equal(r2_copy.id);
+            expect(r1_copy.members[1].id).to.equal(r2_copy.members[0].id);
+        });
+
+        // it("deep copies cyclical relation graphs without issue", function () {
+        //     var r1 = iD.Relation({id: 'r1', members: [{id: 'r2'}]}),
+        //         r2 = iD.Relation({id: 'r2', members: [{id: 'r1'}]}),
+        //         graph = iD.Graph([r1, r2]),
+        //         result = r1.copy(true, graph),
+        //         r1_copy = result[0],
+        //         r2_copy = result[1];
+
+        //     expect(result).to.have.length(2);
+        //     expect(r1_copy).to.be.an.instanceof(iD.Relation);
+        //     expect(r2_copy).to.be.an.instanceof(iD.Relation);
+
+        //     var msg = 'r1_copy = ' + JSON.stringify(r1_copy) +
+        //               'r2_copy = ' + JSON.stringify(r2_copy);
+        //     expect(r1_copy.members[0].id).to.equal(r2_copy.id, msg);
+        //     expect(r2_copy.members[0].id).to.equal(r1_copy.id, msg);
+        // });
+
+        // it("deep copies self-refrencing relations without issue", function () {
+        //     var r1 = iD.Relation({id: 'r1', members: [{id: 'r1'}]}),
+        //         graph = iD.Graph([r1]),
+        //         result = r1.copy(true, graph),
+        //         r1_copy = result[0];
+
+        //     expect(result).to.have.length(1);
+        //     expect(r1_copy).to.be.an.instanceof(iD.Relation);
+        //     expect(r1_copy.members[0].id).to.equal(r1_copy.id);
+        // });
     });
 
     describe("#extent", function () {