Restore deleted nodes that are discovered to have extant parents

This bug has existed since 1.0!

Fixes #2085
This commit is contained in:
John Firebaugh
2014-01-06 16:23:15 -08:00
parent 354ef9df44
commit 4bf21cbc8a
4 changed files with 82 additions and 55 deletions

View File

@@ -12,7 +12,7 @@ iD.Graph = function(other, mutable) {
this.entities = Object.create({});
this._parentWays = Object.create({});
this._parentRels = Object.create({});
this.rebase(other || []);
this.rebase(other || [], [this]);
}
this.transients = {};
@@ -95,22 +95,44 @@ iD.Graph.prototype = {
// is used only during the history operation that merges newly downloaded
// data into each state. To external consumers, it should appear as if the
// graph always contained the newly downloaded data.
rebase: function(entities) {
rebase: function(entities, stack) {
var base = this.base(),
i, k, child, id, keys;
i, j, k, id;
// Merging of data only needed if graph is the base graph
if (!this.inherited) {
for (i = 0; i < entities.length; i++) {
var entity = entities[i];
if (!base.entities[entity.id]) {
base.entities[entity.id] = entity;
this._updateCalculated(undefined, entity,
base.parentWays, base.parentRels);
for (i = 0; i < entities.length; i++) {
var entity = entities[i];
if (base.entities[entity.id])
continue;
// Merging data into the base graph
base.entities[entity.id] = entity;
this._updateCalculated(undefined, entity,
base.parentWays, base.parentRels);
// Restore provisionally-deleted nodes that are discovered to have an extant parent
if (entity.type === 'way') {
for (j = 0; j < entity.nodes.length; j++) {
id = entity.nodes[j];
for (k = 1; k < stack.length; k++) {
var ents = stack[k].entities;
if (ents.hasOwnProperty(id) && ents[id] === undefined) {
delete ents[id];
}
}
}
}
}
for (i = 0; i < stack.length; i++) {
stack[i]._updateRebased();
}
},
_updateRebased: function() {
var base = this.base(),
i, k, child, id, keys;
keys = Object.keys(this._parentWays);
for (i = 0; i < keys.length; i++) {
child = keys[i];
@@ -229,9 +251,7 @@ iD.Graph.prototype = {
freeze: function() {
this.frozen = true;
if (iD.debug) {
Object.freeze(this.entities);
}
// No longer freezing entities here due to in-place updates needed in rebase.
return this;
},

View File

@@ -42,10 +42,7 @@ iD.History = function(context) {
},
merge: function(entities, extent) {
for (var i = 0; i < stack.length; i++) {
stack[i].graph.rebase(entities);
}
stack[0].graph.rebase(entities, _.pluck(stack, 'graph'));
tree.rebase(entities);
dispatch.change(undefined, extent);

View File

@@ -62,26 +62,24 @@ describe('iD.Graph', function() {
it("sets the frozen flag", function () {
expect(iD.Graph([], true).freeze().frozen).to.be.true;
});
if (iD.debug) {
it("freezes entities", function () {
expect(Object.isFrozen(iD.Graph().entities)).to.be.true;
});
}
});
describe("#rebase", function () {
it("preserves existing entities", function () {
var node = iD.Node({id: 'n'}),
graph = iD.Graph([node]);
graph.rebase([]);
graph.rebase([], [graph]);
expect(graph.entity('n')).to.equal(node);
});
it("includes new entities", function () {
var node = iD.Node({id: 'n'}),
graph = iD.Graph();
graph.rebase([node]);
graph.rebase([node], [graph]);
expect(graph.entity('n')).to.equal(node);
});
@@ -89,13 +87,17 @@ describe('iD.Graph', function() {
var a = iD.Node({id: 'n'}),
b = iD.Node({id: 'n'}),
graph = iD.Graph([a]);
graph.rebase([b]);
graph.rebase([b], [graph]);
expect(graph.entity('n')).to.equal(a);
});
it("inherits entities from base prototypally", function () {
var graph = iD.Graph();
graph.rebase([iD.Node()]);
graph.rebase([iD.Node()], [graph]);
expect(graph.entities).not.to.have.ownProperty('n');
});
@@ -105,7 +107,8 @@ describe('iD.Graph', function() {
w2 = iD.Way({id: 'w2', nodes: ['n']}),
graph = iD.Graph([n, w1]);
graph.rebase([w2]);
graph.rebase([w2], [graph]);
expect(graph.parentWays(n)).to.eql([w1, w2]);
expect(graph._parentWays.hasOwnProperty('n')).to.be.false;
});
@@ -114,7 +117,9 @@ describe('iD.Graph', function() {
var n = iD.Node({id: 'n'}),
w1 = iD.Way({id: 'w1', nodes: ['n']}),
graph = iD.Graph([n, w1]);
graph.rebase([w1]);
graph.rebase([w1], [graph]);
expect(graph.parentWays(n)).to.eql([w1]);
});
@@ -126,8 +131,7 @@ describe('iD.Graph', function() {
graph = iD.Graph([n, w1]),
graph2 = graph.replace(w2);
graph.rebase([w3]);
graph2.rebase([w3]);
graph.rebase([w3], [graph, graph2]);
expect(graph2.parentWays(n)).to.eql([w1, w2, w3]);
});
@@ -140,8 +144,7 @@ describe('iD.Graph', function() {
graph = iD.Graph([n1, n2, w1]),
graph2 = graph.replace(w2);
graph.rebase([w1]);
graph2.rebase([w1]);
graph.rebase([w1], [graph, graph2]);
expect(graph2.parentWays(n2)).to.eql([]);
});
@@ -152,19 +155,30 @@ describe('iD.Graph', function() {
graph = iD.Graph([n, w1]),
graph2 = graph.remove(w1);
graph.rebase([w1]);
graph2.rebase([w1]);
graph.rebase([w1], [graph, graph2]);
expect(graph2.parentWays(n)).to.eql([]);
});
it("re-adds a deleted node that is discovered to have another parent", function() {
var n = iD.Node({id: 'n'}),
w1 = iD.Way({id: 'w1', nodes: ['n']}),
w2 = iD.Way({id: 'w2', nodes: ['n']}),
graph = iD.Graph([n, w1]),
graph2 = graph.remove(n);
graph.rebase([n, w2], [graph, graph2]);
expect(graph2.entity('n')).to.eql(n);
});
it("updates parentRelations", function () {
var n = iD.Node({id: 'n'}),
r1 = iD.Relation({id: 'r1', members: [{id: 'n'}]}),
r2 = iD.Relation({id: 'r2', members: [{id: 'n'}]}),
graph = iD.Graph([n, r1]);
graph.rebase([r2]);
graph.rebase([r2], [graph]);
expect(graph.parentRelations(n)).to.eql([r1, r2]);
expect(graph._parentRels.hasOwnProperty('n')).to.be.false;
@@ -177,8 +191,7 @@ describe('iD.Graph', function() {
graph = iD.Graph([n, r1]),
graph2 = graph.replace(r2);
graph.rebase([r1]);
graph2.rebase([r1]);
graph.rebase([r1], [graph, graph2]);
expect(graph2.parentRelations(n)).to.eql([]);
});
@@ -189,8 +202,7 @@ describe('iD.Graph', function() {
graph = iD.Graph([n, r1]),
graph2 = graph.remove(r1);
graph.rebase([r1]);
graph2.rebase([r1]);
graph.rebase([r1], [graph, graph2]);
expect(graph2.parentRelations(n)).to.eql([]);
});
@@ -203,8 +215,8 @@ describe('iD.Graph', function() {
graph = iD.Graph([n, r1]),
graph2 = graph.replace(r2);
graph.rebase([r3]);
graph2.rebase([r3]);
graph.rebase([r3], [graph, graph2]);
expect(graph2.parentRelations(n)).to.eql([r1, r2, r3]);
});
@@ -221,7 +233,7 @@ describe('iD.Graph', function() {
}
expect(numParents(n)).to.equal(1);
graph.rebase([w2]);
graph.rebase([w2], [graph]);
expect(numParents(n)).to.equal(2);
});
});

View File

@@ -5,7 +5,7 @@ describe("iD.Tree", function() {
tree = iD.Tree(graph),
node = iD.Node({id: 'n', loc: [1, 1]});
graph.rebase([node]);
graph.rebase([node], [graph]);
tree.rebase([node]);
expect(tree.intersects(iD.geo.Extent([0, 0], [2, 2]), graph)).to.eql([node]);
@@ -17,11 +17,11 @@ describe("iD.Tree", function() {
node = iD.Node({id: 'n', loc: [1, 1]}),
extent = iD.geo.Extent([0, 0], [2, 2]);
graph.rebase([node]);
graph.rebase([node], [graph]);
tree.rebase([node]);
expect(tree.intersects(extent, graph)).to.eql([node]);
graph.rebase([node]);
graph.rebase([node], [graph]);
tree.rebase([node]);
expect(tree.intersects(extent, graph)).to.eql([node]);
});
@@ -35,7 +35,7 @@ describe("iD.Tree", function() {
expect(tree.intersects(iD.geo.Extent([9, 9], [11, 11]), g)).to.eql([node_]);
graph.rebase({n: node});
graph.rebase([node], [graph]);
tree.rebase([node]);
expect(tree.intersects(iD.geo.Extent([0, 0], [2, 2]), g)).to.eql([]);
@@ -63,11 +63,11 @@ describe("iD.Tree", function() {
relation = iD.Relation({id: 'r', members: [{id: 'n1'}, {id: 'n2'}]}),
extent = iD.geo.Extent([0.5, 0.5], [1.5, 1.5]);
graph.rebase([relation, n1]);
graph.rebase([relation, n1], [graph]);
tree.rebase([relation, n1]);
expect(tree.intersects(extent, graph)).to.eql([]);
graph.rebase([n2]);
graph.rebase([n2], [graph]);
tree.rebase([n2]);
expect(tree.intersects(extent, graph)).to.eql([n2, relation]);
});
@@ -83,8 +83,7 @@ describe("iD.Tree", function() {
expect(tree.intersects(extent, graph)).to.eql([]);
base.rebase([node]);
graph.rebase([node]);
base.rebase([node], [base, graph]);
tree.rebase([node]);
expect(tree.intersects(extent, graph)).to.eql([node, way]);
});
@@ -160,7 +159,7 @@ describe("iD.Tree", function() {
var graph = base.replace(node).remove(node);
expect(tree.intersects(extent, graph)).to.eql([]);
base.rebase([node]);
base.rebase([node], [base]);
tree.rebase([node]);
expect(tree.intersects(extent, graph)).to.eql([]);
});
@@ -176,8 +175,7 @@ describe("iD.Tree", function() {
var graph = base.replace(r1).replace(r2);
expect(tree.intersects(extent, graph)).to.eql([]);
base.rebase([node]);
graph.rebase([node]);
base.rebase([node], [base, graph]);
tree.rebase([node]);
expect(tree.intersects(extent, graph)).to.eql([node, r1, r2]);
});