From 5e66307500d250c42c4ea0b98a1b32fc09afd339 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 13 Feb 2013 16:01:13 -0800 Subject: [PATCH] Fix incorrect parentWays after reloading a split way When recalculating parent ways/relations during rebase, a graph should not add modified or deleted entities as parents. Such entities will already be correctly marked as parents or not. Graph had the correct behavior for deleted entities, but not for modified entities. This had the effect that if you split a way that was partially off screen, and then panned so that the way was re-retrieved, Graph#rebase would mistakenly add back the original way as a parent of all the nodes that were split into the new section, making them appear as shared. Fixes #751. --- js/id/core/graph.js | 5 +++-- test/spec/core/graph.js | 42 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/js/id/core/graph.js b/js/id/core/graph.js index a3fb8e823..a595f0df6 100644 --- a/js/id/core/graph.js +++ b/js/id/core/graph.js @@ -93,6 +93,7 @@ iD.Graph.prototype = { rebase: function(entities) { var base = this.base(), i, k, child, id, keys; + // Merging of data only needed if graph is the base graph if (!this.inherited) { for (i in entities) { @@ -110,7 +111,7 @@ iD.Graph.prototype = { if (base.parentWays[child]) { for (k = 0; k < base.parentWays[child].length; k++) { id = base.parentWays[child][k]; - if (this.entity(id) && !_.contains(this._parentWays[child], id)) { + if (!this.entities.hasOwnProperty(id) && !_.contains(this._parentWays[child], id)) { this._parentWays[child].push(id); } } @@ -123,7 +124,7 @@ iD.Graph.prototype = { if (base.parentRels[child]) { for (k = 0; k < base.parentRels[child].length; k++) { id = base.parentRels[child][k]; - if (this.entity(id) && !_.contains(this._parentRels[child], id)) { + if (!this.entities.hasOwnProperty(id) && !_.contains(this._parentRels[child], id)) { this._parentRels[child].push(id); } } diff --git a/test/spec/core/graph.js b/test/spec/core/graph.js index a099d8b61..ef6741bb9 100644 --- a/test/spec/core/graph.js +++ b/test/spec/core/graph.js @@ -107,19 +107,36 @@ describe('iD.Graph', function() { w3 = iD.Way({id: 'w3', nodes: ['n']}), graph = iD.Graph([n, w1]), graph2 = graph.replace(w2); + graph.rebase({ 'w3': w3 }); graph2.rebase({ 'w3': w3 }); expect(graph2.parentWays(n)).to.eql([w1, w2, w3]); }); - it("avoids re-adding removed parentWays", function() { + it("avoids re-adding a modified way as a parent way", function() { + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n2'}), + w1 = iD.Way({id: 'w1', nodes: ['n1', 'n2']}), + w2 = w1.removeNode('n2'), + graph = iD.Graph([n1, n2, w1]), + graph2 = graph.replace(w2); + + graph.rebase({ 'w1': w1 }); + graph2.rebase({ 'w1': w1 }); + + expect(graph2.parentWays(n2)).to.eql([]); + }); + + it("avoids re-adding a deleted way as a parent way", function() { var n = iD.Node({id: 'n'}), w1 = iD.Way({id: 'w1', nodes: ['n']}), graph = iD.Graph([n, w1]), graph2 = graph.remove(w1); + graph.rebase({ 'w1': w1 }); graph2.rebase({ 'w1': w1 }); + expect(graph2.parentWays(n)).to.eql([]); }); @@ -135,14 +152,29 @@ describe('iD.Graph', function() { expect(graph._parentRels.hasOwnProperty('n')).to.be.false; }); - it("avoids re-adding removed parentRels", function() { + it("avoids re-adding a modified relation as a parent relation", function() { + var n = iD.Node({id: 'n'}), + r1 = iD.Relation({id: 'r1', members: [{id: 'n'}]}), + r2 = r1.removeMember('n'), + graph = iD.Graph([n, r1]), + graph2 = graph.replace(r2); + + graph.rebase({ 'r1': r1 }); + graph2.rebase({ 'r1': r1 }); + + expect(graph2.parentRelations(n)).to.eql([]); + }); + + it("avoids re-adding a deleted relation as a parent relation", function() { var n = iD.Node({id: 'n'}), r1 = iD.Relation({id: 'r1', members: [{id: 'n'}]}), graph = iD.Graph([n, r1]), graph2 = graph.remove(r1); - graph.rebase({ 'w1': r1 }); - graph2.rebase({ 'w1': r1 }); - expect(graph2.parentWays(n)).to.eql([]); + + graph.rebase({ 'r1': r1 }); + graph2.rebase({ 'r1': r1 }); + + expect(graph2.parentRelations(n)).to.eql([]); }); it("updates parentRels for nodes with modified parentWays", function () {