When connecting nodes, prefer to keep an existing (not new) node

(closes #4974, closes #4674)
This commit is contained in:
Bryan Housel
2018-04-09 14:20:40 -04:00
parent c42556ec96
commit b09c712fc5
2 changed files with 90 additions and 75 deletions

View File

@@ -3,8 +3,8 @@ import { actionDeleteNode } from './delete_node';
// Connect the ways at the given nodes.
//
// The last node will survive. All other nodes will be replaced with
// the surviving node in parent ways, and then removed.
// First choose a node to be the survivor, with preference given
// to an existing (not new) node.
//
// Tags and relation memberships of of non-surviving nodes are merged
// to the survivor.
@@ -17,11 +17,20 @@ import { actionDeleteNode } from './delete_node';
//
export function actionConnect(nodeIds) {
return function(graph) {
var last = nodeIds[nodeIds.length - 1];
var survivor = graph.entity(last);
var survivor;
var node;
var i;
for (var i = 0; i < nodeIds.length - 1; i++) {
var node = graph.entity(nodeIds[i]);
// Choose a survivor node, prefer an existing (not new) node - #4974
for (i = 0; i < nodeIds.length; i++) {
survivor = graph.entity(nodeIds[i]);
if (survivor.version) break; // found one
}
// Replace all non-surviving nodes with the survivor and merge tags.
for (i = 0; i < nodeIds.length; i++) {
node = graph.entity(nodeIds[i]);
if (node.id === survivor.id) continue;
/* eslint-disable no-loop-func */
graph.parentWays(node).forEach(function(parent) {

View File

@@ -1,18 +1,31 @@
describe('iD.actionConnect', function() {
it('removes all but the final node', function() {
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Node({id: 'c'})
]);
it('chooses the first non-new node as the survivor', function() {
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b', version: '1'}),
iD.osmNode({id: 'c', version: '1'})
]);
graph = iD.actionConnect(['a', 'b', 'c'])(graph);
expect(graph.hasEntity('a')).to.be.undefined;
expect(graph.hasEntity('b')).to.be.undefined;
expect(graph.entity('c')).not.to.be.undefined;
expect(graph.hasEntity('a')).not.to.be.ok;
expect(graph.hasEntity('b')).to.be.ok;
expect(graph.hasEntity('c')).not.to.be.ok;
});
it('chooses the last node as the survivor when all are new', function() {
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmNode({id: 'c'})
]);
graph = iD.actionConnect(['a', 'b', 'c'])(graph);
expect(graph.hasEntity('a')).not.to.be.ok;
expect(graph.hasEntity('b')).not.to.be.ok;
expect(graph.hasEntity('c')).to.be.ok;
});
it('replaces non-surviving nodes in parent ways', function() {
// a --- b --- c
//
@@ -28,18 +41,17 @@ describe('iD.actionConnect', function() {
// |
// d
//
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Node({id: 'c'}),
iD.Node({id: 'd'}),
iD.Node({id: 'e'}),
iD.Way({id: '-', nodes: ['a', 'b', 'c']}),
iD.Way({id: '|', nodes: ['d', 'e']})
]);
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmNode({id: 'c'}),
iD.osmNode({id: 'd'}),
iD.osmNode({id: 'e'}),
iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}),
iD.osmWay({id: '|', nodes: ['d', 'e']})
]);
graph = iD.actionConnect(['e', 'b'])(graph);
expect(graph.entity('-').nodes).to.eql(['a', 'b', 'c']);
expect(graph.entity('|').nodes).to.eql(['d', 'b']);
});
@@ -53,18 +65,17 @@ describe('iD.actionConnect', function() {
//
// Connect [a, d].
//
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Node({id: 'c'}),
iD.Node({id: 'd'}),
iD.Node({id: 'e'}),
iD.Way({id: '-', nodes: ['a', 'b', 'c', 'a']}),
iD.Way({id: '=', nodes: ['d', 'e']})
]);
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmNode({id: 'c'}),
iD.osmNode({id: 'd'}),
iD.osmNode({id: 'e'}),
iD.osmWay({id: '-', nodes: ['a', 'b', 'c', 'a']}),
iD.osmWay({id: '=', nodes: ['d', 'e']})
]);
graph = iD.actionConnect(['a', 'd'])(graph);
expect(graph.entity('-').nodes).to.eql(['d', 'b', 'c', 'd']);
});
@@ -77,15 +88,14 @@ describe('iD.actionConnect', function() {
//
// a --- c
//
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Node({id: 'c'}),
iD.Way({id: '-', nodes: ['a', 'b', 'c']})
]);
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmNode({id: 'c'}),
iD.osmWay({id: '-', nodes: ['a', 'b', 'c']})
]);
graph = iD.actionConnect(['b', 'c'])(graph);
expect(graph.entity('-').nodes).to.eql(['a', 'c']);
expect(graph.hasEntity('b')).to.be.undefined;
});
@@ -103,17 +113,16 @@ describe('iD.actionConnect', function() {
// |
// d
//
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Node({id: 'c'}),
iD.Node({id: 'c'}),
iD.Way({id: '-', nodes: ['a', 'b', 'c']}),
iD.Way({id: '|', nodes: ['b', 'd']})
]);
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmNode({id: 'c'}),
iD.osmNode({id: 'c'}),
iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}),
iD.osmWay({id: '|', nodes: ['b', 'd']})
]);
graph = iD.actionConnect(['b', 'c'])(graph);
expect(graph.entity('-').nodes).to.eql(['a', 'c']);
expect(graph.entity('|').nodes).to.eql(['c', 'd']);
expect(graph.hasEntity('b')).to.be.undefined;
@@ -124,44 +133,41 @@ describe('iD.actionConnect', function() {
//
// Connect [a, b]
//
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Way({id: '-', nodes: ['a', 'b']})
]);
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmWay({id: '-', nodes: ['a', 'b']})
]);
graph = iD.actionConnect(['a', 'b'])(graph);
expect(graph.hasEntity('a')).to.be.undefined;
expect(graph.hasEntity('-')).to.be.undefined;
});
it('merges tags to the surviving node', function() {
var graph = iD.Graph([
iD.Node({id: 'a', tags: {a: 'a'}}),
iD.Node({id: 'b', tags: {b: 'b'}}),
iD.Node({id: 'c', tags: {c: 'c'}})
]);
var graph = iD.coreGraph([
iD.osmNode({id: 'a', tags: {a: 'a'}}),
iD.osmNode({id: 'b', tags: {b: 'b'}}),
iD.osmNode({id: 'c', tags: {c: 'c'}})
]);
graph = iD.actionConnect(['a', 'b', 'c'])(graph);
expect(graph.entity('c').tags).to.eql({a: 'a', b: 'b', c: 'c'});
});
it('merges memberships to the surviving node', function() {
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Node({id: 'c'}),
iD.Node({id: 'c'}),
iD.Way({id: '-', nodes: ['a', 'b']}),
iD.Way({id: '=', nodes: ['c', 'd']}),
iD.Relation({id: 'r1', members: [{id: 'b', role: 'r1', type: 'node'}]}),
iD.Relation({id: 'r2', members: [{id: 'b', role: 'r2', type: 'node'}, {id: 'c', role: 'r2', type: 'node'}]})
]);
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmNode({id: 'c'}),
iD.osmNode({id: 'c'}),
iD.osmWay({id: '-', nodes: ['a', 'b']}),
iD.osmWay({id: '=', nodes: ['c', 'd']}),
iD.osmRelation({id: 'r1', members: [{id: 'b', role: 'r1', type: 'node'}]}),
iD.osmRelation({id: 'r2', members: [{id: 'b', role: 'r2', type: 'node'}, {id: 'c', role: 'r2', type: 'node'}]})
]);
graph = iD.actionConnect(['b', 'c'])(graph);
expect(graph.entity('r1').members).to.eql([{id: 'c', role: 'r1', type: 'node'}]);
expect(graph.entity('r2').members).to.eql([{id: 'c', role: 'r2', type: 'node'}]);
});