Keep the oldest interesting ID alive when merging nodes

This commit is contained in:
Thomas Petillon
2020-04-11 16:26:17 +02:00
parent e9c7436289
commit 87ca2b09cc
3 changed files with 158 additions and 23 deletions

View File

@@ -1,12 +1,13 @@
import { actionDeleteNode } from './delete_node';
import { actionDeleteWay } from './delete_way';
import { utilArrayUniq } from '../util';
import { osmEntity } from '../osm';
import { utilArrayUniq, utilOldestID } from '../util';
// Connect the ways at the given nodes.
//
// First choose a node to be the survivor, with preference given
// to an existing (not new) node.
// to the oldest existing (not new) and "interesting" node.
//
// Tags and relation memberships of of non-surviving nodes are merged
// to the survivor.
@@ -24,11 +25,21 @@ export function actionConnect(nodeIDs) {
var parents;
var i, j;
// Choose a survivor node, prefer an existing (not new) node - #4974
// Select the node with the ID passed as parameter if it is in the list,
// otherwise select the node with the oldest ID as the survivor, or the
// last one if there are only new nodes.
nodeIDs.reverse();
var interestingIDs = [];
for (i = 0; i < nodeIDs.length; i++) {
survivor = graph.entity(nodeIDs[i]);
if (survivor.version) break; // found one
node = graph.entity(nodeIDs[i]);
if (node.hasInterestingTags()) {
if (!node.isNew()) {
interestingIDs.push(node.id);
}
}
}
survivor = graph.entity(utilOldestID(interestingIDs.length > 0 ? interestingIDs : nodeIDs));
// Replace all non-surviving nodes with the survivor and merge tags.
for (i = 0; i < nodeIDs.length; i++) {
@@ -71,11 +82,8 @@ export function actionConnect(nodeIDs) {
var relations, relation, role;
var i, j, k;
// 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
}
// Select the node with the oldest ID as the survivor.
survivor = graph.entity(utilOldestID(nodeIDs));
// 1. disable if the nodes being connected have conflicting relation roles
for (i = 0; i < nodeIDs.length; i++) {

View File

@@ -1,28 +1,85 @@
describe('iD.actionConnect', function() {
it('chooses the first non-new node as the survivor', function() {
it('merges tags', function() {
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b', version: '1'}),
iD.osmNode({id: 'c', version: '1'})
iD.osmNode({id: 'a', tags: { highway: 'traffic_signals' }}),
iD.osmNode({id: 'b', tags: { crossing: 'marked' }}),
]);
graph = iD.actionConnect(['a', 'b', 'c'])(graph);
graph = iD.actionConnect(['a', 'b'])(graph);
expect(graph.hasEntity('a')).not.to.be.ok;
expect(graph.hasEntity('b')).to.be.ok;
expect(graph.hasEntity('c')).not.to.be.ok;
var survivor = graph.hasEntity('b');
expect(survivor).to.be.an.instanceof(iD.osmNode);
expect(survivor.tags).to.eql({ highway: 'traffic_signals', crossing: 'marked' }, 'merge all tags');
});
it('chooses the oldest node as the survivor', function() {
var graph = iD.coreGraph([
iD.osmNode({id: 'n3'}),
iD.osmNode({id: 'n-1'}),
iD.osmNode({id: 'n2'}),
iD.osmNode({id: 'n4'})
]);
graph = iD.actionConnect(['n3', 'n-1', 'n2', 'n4'])(graph);
expect(graph.hasEntity('n3')).not.to.be.ok;
expect(graph.hasEntity('n-1')).not.to.be.ok;
expect(graph.hasEntity('n2')).to.be.ok;
expect(graph.hasEntity('n4')).not.to.be.ok;
});
it('chooses the oldest interesting node as the survivor', function() {
var graph = iD.coreGraph([
iD.osmNode({id: 'n3'}),
iD.osmNode({id: 'n1'}),
iD.osmNode({id: 'n2', tags: { highway: 'traffic_signals' }}),
iD.osmNode({id: 'n4', tags: { crossing: 'marked' }})
]);
graph = iD.actionConnect(['n3', 'n1', 'n2', 'n4'])(graph);
expect(graph.hasEntity('n3')).not.to.be.ok;
expect(graph.hasEntity('n1')).not.to.be.ok;
expect(graph.hasEntity('n4')).not.to.be.ok;
var survivor = graph.hasEntity('n2');
expect(survivor).to.be.an.instanceof(iD.osmNode);
expect(survivor.tags).to.eql({ highway: 'traffic_signals', crossing: 'marked' }, 'merge all tags');
});
it('chooses an existing node as the survivor', function() {
var graph = iD.coreGraph([
iD.osmNode({id: 'n3'}),
iD.osmNode({id: 'n-1'}),
iD.osmNode({id: 'n-2', tags: { highway: 'traffic_signals' }}),
iD.osmNode({id: 'n-4', tags: { crossing: 'marked' }})
]);
graph = iD.actionConnect(['n3', 'n-1', 'n-2', 'n-4'])(graph);
expect(graph.hasEntity('n-1')).not.to.be.ok;
expect(graph.hasEntity('n-2')).not.to.be.ok;
expect(graph.hasEntity('n-4')).not.to.be.ok;
var survivor = graph.hasEntity('n3');
expect(survivor).to.be.an.instanceof(iD.osmNode);
expect(survivor.tags).to.eql({ highway: 'traffic_signals', crossing: 'marked' }, 'merge all tags');
});
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: 'a', tags: { highway: 'traffic_signals' }}),
iD.osmNode({id: 'b', tags: { crossing: 'marked' }}),
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;
var survivor = graph.hasEntity('c');
expect(survivor).to.be.an.instanceof(iD.osmNode);
expect(survivor.tags).to.eql({ highway: 'traffic_signals', crossing: 'marked' }, 'merge all tags');
});

View File

@@ -72,9 +72,79 @@ describe('iD.actionMergeNodes', function () {
});
it('keeps the id of the interesting node', function() {
var graph = iD.coreGraph([
iD.osmNode({ id: 'n1', loc: [0, 0] }),
iD.osmNode({ id: 'n2', loc: [4, 4], tags: { highway: 'traffic_signals' }})
]);
graph = iD.actionMergeNodes(['n1', 'n2'])(graph);
expect(graph.hasEntity('n1')).to.be.undefined;
var survivor = graph.hasEntity('n2');
expect(survivor).to.be.an.instanceof(iD.osmNode);
expect(survivor.tags).to.eql({ highway: 'traffic_signals' }, 'merge all tags');
expect(survivor.loc).to.eql([4, 4], 'use loc of interesting node');
});
it('keeps the id of the existing node', function() {
var graph = iD.coreGraph([
iD.osmNode({ id: 'n1', loc: [0, 0] }),
iD.osmNode({ id: 'b', loc: [4, 4], tags: { highway: 'traffic_signals' }})
]);
graph = iD.actionMergeNodes(['n1', 'b'])(graph);
expect(graph.hasEntity('b')).to.be.undefined;
var survivor = graph.hasEntity('n1');
expect(survivor).to.be.an.instanceof(iD.osmNode);
expect(survivor.tags).to.eql({ highway: 'traffic_signals' }, 'merge all tags');
expect(survivor.loc).to.eql([4, 4], 'use loc of interesting node');
});
it('keeps the id of the oldest node', function() {
var graph = iD.coreGraph([
iD.osmNode({ id: 'n2', loc: [0, 0] }),
iD.osmNode({ id: 'n1', loc: [2, 2] }),
iD.osmNode({ id: 'n3', loc: [4, 4] })
]);
graph = iD.actionMergeNodes(['n2', 'n1', 'n3'])(graph);
expect(graph.hasEntity('n2')).to.be.undefined;
expect(graph.hasEntity('n3')).to.be.undefined;
var survivor = graph.hasEntity('n1');
expect(survivor).to.be.an.instanceof(iD.osmNode);
});
it('keeps the id of the oldest interesting node', function() {
var graph = iD.coreGraph([
iD.osmNode({ id: 'n3', loc: [0, 0] }),
iD.osmNode({ id: 'n1', loc: [2, 2] }),
iD.osmNode({ id: 'n2', loc: [4, 4], tags: { highway: 'traffic_signals' }}),
iD.osmNode({ id: 'n4', loc: [8, 8], tags: { crossing: 'marked' }})
]);
graph = iD.actionMergeNodes(['n2', 'n1', 'n3', 'n4'])(graph);
expect(graph.hasEntity('n1')).to.be.undefined;
expect(graph.hasEntity('n3')).to.be.undefined;
expect(graph.hasEntity('n4')).to.be.undefined;
var survivor = graph.hasEntity('n2');
expect(survivor).to.be.an.instanceof(iD.osmNode);
});
it('merges two nodes along a single way', function() {
//
// scenerio: merge b,c:
// scenario: merge b,c:
//
// a -- b -- c a ---- c
//
@@ -98,7 +168,7 @@ describe('iD.actionMergeNodes', function () {
it('merges two nodes from two ways', function() {
//
// scenerio: merge b,d:
// scenario: merge b,d:
//
// a -- b -- c a -_ _- c
// d
@@ -129,7 +199,7 @@ describe('iD.actionMergeNodes', function () {
it('merges three nodes from three ways', function () {
//
// scenerio: merge b,d:
// scenario: merge b,d,e:
//
// c c
// | |