From 7c918ba16162fbdb55a8fadffc15dffad941332c Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 18 Jan 2018 16:52:23 -0500 Subject: [PATCH] Allow Relation.replaceMember to optionally preserve duplicates (closes #4696) --- modules/actions/add_member.js | 17 +- modules/osm/relation.js | 8 +- test/spec/actions/join.js | 432 +++++++++++++++++++--------------- test/spec/osm/relation.js | 23 +- 4 files changed, 262 insertions(+), 218 deletions(-) diff --git a/modules/actions/add_member.js b/modules/actions/add_member.js index a9dfa7ec64..0491753290 100644 --- a/modules/actions/add_member.js +++ b/modules/actions/add_member.js @@ -39,7 +39,7 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) { tempWay = osmWay({ id: 'wTemp', nodes: insertPair.nodes }); graph = graph.replace(tempWay); var tempMember = { id: tempWay.id, type: 'way', role: member.role }; - var tempRelation = replaceMemberAll(relation, insertPair.originalID, tempMember); + var tempRelation = relation.replaceMember({id: insertPair.originalID}, tempMember, true); groups = _groupBy(tempRelation.members, function(m) { return m.type; }); groups.way = groups.way || []; @@ -160,21 +160,6 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) { } - // Relation.replaceMember() removes duplicates, and we don't want that.. #4696 - function replaceMemberAll(relation, needleID, replacement) { - var members = []; - for (var i = 0; i < relation.members.length; i++) { - var member = relation.members[i]; - if (member.id !== needleID) { - members.push(member); - } else { - members.push({id: replacement.id, type: replacement.type, role: member.role}); - } - } - return relation.update({members: members}); - } - - // This is the same as `Relation.indexedMembers`, // Except we don't want to index all the members, only the ways function withIndex(arr) { diff --git a/modules/osm/relation.js b/modules/osm/relation.js index 0526278600..b44dbd4200 100644 --- a/modules/osm/relation.js +++ b/modules/osm/relation.js @@ -161,9 +161,9 @@ _extend(osmRelation.prototype, { // Wherever a member appears with id `needle.id`, replace it with a member // with id `replacement.id`, type `replacement.type`, and the original role, - // unless a member already exists with that id and role. Return an updated - // relation. - replaceMember: function(needle, replacement) { + // By default, adding a duplicate member (by id and role) is prevented. + // Return an updated relation. + replaceMember: function(needle, replacement, keepDuplicates) { if (!this.memberById(needle.id)) return this; @@ -173,7 +173,7 @@ _extend(osmRelation.prototype, { var member = this.members[i]; if (member.id !== needle.id) { members.push(member); - } else if (!this.memberByIdAndRole(replacement.id, member.role)) { + } else if (keepDuplicates || !this.memberByIdAndRole(replacement.id, member.role)) { members.push({id: replacement.id, type: replacement.type, role: member.role}); } } diff --git a/test/spec/actions/join.js b/test/spec/actions/join.js index 8b13781874..e7de473f35 100644 --- a/test/spec/actions/join.js +++ b/test/spec/actions/join.js @@ -2,67 +2,67 @@ describe('iD.actionJoin', function () { describe('#disabled', function () { it('returns falsy for ways that share an end/start node', function () { // a --> b ==> c - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['b', 'c']}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['b', 'c']}) + ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).not.to.be.ok; }); it('returns falsy for ways that share a start/end node', function () { // a <-- b <== c - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['b', 'a']}), - iD.Way({id: '=', nodes: ['c', 'b']}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['b', 'a']}), + iD.osmWay({id: '=', nodes: ['c', 'b']}) + ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).not.to.be.ok; }); it('returns falsy for ways that share a start/start node', function () { // a <-- b ==> c - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['b', 'a']}), - iD.Way({id: '=', nodes: ['b', 'c']}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['b', 'a']}), + iD.osmWay({id: '=', nodes: ['b', 'c']}) + ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).not.to.be.ok; }); it('returns falsy for ways that share an end/end node', function () { // a --> b <== c - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['c', 'b']}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['c', 'b']}) + ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).not.to.be.ok; }); it('returns falsy for more than two ways when connected, regardless of order', function () { // a --> b ==> c ~~> d - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'd'}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['b', 'c']}), - iD.Way({id: '~', nodes: ['c', 'd']}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['b', 'c']}), + iD.osmWay({id: '~', nodes: ['c', 'd']}) + ]); expect(iD.actionJoin(['-', '=', '~']).disabled(graph)).not.to.be.ok; expect(iD.actionJoin(['-', '~', '=']).disabled(graph)).not.to.be.ok; @@ -73,9 +73,9 @@ describe('iD.actionJoin', function () { }); it('returns \'not_eligible\' for non-line geometries', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}) + ]); expect(iD.actionJoin(['a']).disabled(graph)).to.equal('not_eligible'); }); @@ -84,14 +84,14 @@ describe('iD.actionJoin', function () { // a -- b -- c // | // d - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'd'}), - 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: 'd'}), + iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), + iD.osmWay({id: '=', nodes: ['b', 'd']}) + ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).to.equal('not_adjacent'); }); @@ -101,18 +101,18 @@ describe('iD.actionJoin', function () { // from: - // to: = // via: b - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['b', 'c']}), - iD.Relation({id: 'r', tags: {type: 'restriction'}, members: [ - {type: 'way', id: '-', role: 'from'}, - {type: 'way', id: '=', role: 'to'}, - {type: 'node', id: 'b', role: 'via'} - ]}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['b', 'c']}), + iD.osmRelation({id: 'r', tags: {type: 'restriction'}, members: [ + {type: 'way', id: '-', role: 'from'}, + {type: 'way', id: '=', role: 'to'}, + {type: 'node', id: 'b', role: 'via'} + ]}) + ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).to.equal('restriction'); }); @@ -124,20 +124,20 @@ describe('iD.actionJoin', function () { // from: - // to: | // via: b - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'd'}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['b', 'c']}), - iD.Way({id: '|', nodes: ['b', 'd']}), - iD.Relation({id: 'r', tags: {type: 'restriction'}, members: [ - {type: 'way', id: '-', role: 'from'}, - {type: 'way', id: '|', role: 'to'}, - {type: 'node', id: 'b', role: 'via'} - ]}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['b', 'c']}), + iD.osmWay({id: '|', nodes: ['b', 'd']}), + iD.osmRelation({id: 'r', tags: {type: 'restriction'}, members: [ + {type: 'way', id: '-', role: 'from'}, + {type: 'way', id: '|', role: 'to'}, + {type: 'node', id: 'b', role: 'via'} + ]}) + ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).to.equal('restriction'); }); @@ -149,20 +149,20 @@ describe('iD.actionJoin', function () { // from: - // to: | // via: a - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'd'}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['b', 'c']}), - iD.Way({id: '|', nodes: ['a', 'd']}), - iD.Relation({id: 'r', tags: {type: 'restriction'}, members: [ - {type: 'way', id: '-', role: 'from'}, - {type: 'way', id: '|', role: 'to'}, - {type: 'node', id: 'a', role: 'via'} - ]}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['b', 'c']}), + iD.osmWay({id: '|', nodes: ['a', 'd']}), + iD.osmRelation({id: 'r', tags: {type: 'restriction'}, members: [ + {type: 'way', id: '-', role: 'from'}, + {type: 'way', id: '|', role: 'to'}, + {type: 'node', id: 'a', role: 'via'} + ]}) + ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).not.to.be.ok; }); @@ -176,68 +176,68 @@ describe('iD.actionJoin', function () { // from: | // to: \ // via: b - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'd'}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['b', 'c']}), - iD.Way({id: '|', nodes: ['d', 'b']}), - iD.Way({id: '\\', nodes: ['b', 'e']}), - iD.Relation({id: 'r', tags: {type: 'restriction'}, members: [ - {type: 'way', id: '|', role: 'from'}, - {type: 'way', id: '\\', role: 'to'}, - {type: 'node', id: 'b', role: 'via'} - ]}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['b', 'c']}), + iD.osmWay({id: '|', nodes: ['d', 'b']}), + iD.osmWay({id: '\\', nodes: ['b', 'e']}), + iD.osmRelation({id: 'r', tags: {type: 'restriction'}, members: [ + {type: 'way', id: '|', role: 'from'}, + {type: 'way', id: '\\', role: 'to'}, + {type: 'node', id: 'b', role: 'via'} + ]}) + ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).not.to.be.ok; }); it('returns \'conflicting_tags\' for two entities that have conflicting tags', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b'], tags: {highway: 'primary'}}), - iD.Way({id: '=', nodes: ['b', 'c'], tags: {highway: 'secondary'}}) + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b'], tags: {highway: 'primary'}}), + iD.osmWay({id: '=', nodes: ['b', 'c'], tags: {highway: 'secondary'}}) ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).to.equal('conflicting_tags'); }); it('takes tag reversals into account when calculating conflicts', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b'], tags: {'oneway': 'yes'}}), - iD.Way({id: '=', nodes: ['c', 'b'], tags: {'oneway': '-1'}}) + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b'], tags: {'oneway': 'yes'}}), + iD.osmWay({id: '=', nodes: ['c', 'b'], tags: {'oneway': '-1'}}) ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).not.to.be.ok; }); it('returns falsy for exceptions to tag conflicts: missing tag', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b'], tags: {highway: 'primary'}}), - iD.Way({id: '=', nodes: ['b', 'c'], tags: {}}) + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b'], tags: {highway: 'primary'}}), + iD.osmWay({id: '=', nodes: ['b', 'c'], tags: {}}) ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).not.to.be.ok; }); it('returns falsy for exceptions to tag conflicts: uninteresting tag', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b'], tags: {'tiger:cfcc': 'A41'}}), - iD.Way({id: '=', nodes: ['b', 'c'], tags: {'tiger:cfcc': 'A42'}}) + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b'], tags: {'tiger:cfcc': 'A41'}}), + iD.osmWay({id: '=', nodes: ['b', 'c'], tags: {'tiger:cfcc': 'A42'}}) ]); expect(iD.actionJoin(['-', '=']).disabled(graph)).not.to.be.ok; @@ -247,13 +247,13 @@ describe('iD.actionJoin', function () { it('joins a --> b ==> c', function () { // Expected result: // a --> b --> c - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['b', 'c']}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['b', 'c']}) + ]); graph = iD.actionJoin(['-', '='])(graph); @@ -264,13 +264,13 @@ describe('iD.actionJoin', function () { it('joins a <-- b <== c', function () { // Expected result: // a <-- b <-- c - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['b', 'a']}), - iD.Way({id: '=', nodes: ['c', 'b']}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['b', 'a']}), + iD.osmWay({id: '=', nodes: ['c', 'b']}) + ]); graph = iD.actionJoin(['-', '='])(graph); @@ -282,13 +282,13 @@ describe('iD.actionJoin', function () { // Expected result: // a --> b --> c // tags on --- reversed - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['b', 'a'], tags: {'lanes:forward': 2}}), - iD.Way({id: '=', nodes: ['b', 'c']}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['b', 'a'], tags: {'lanes:forward': 2}}), + iD.osmWay({id: '=', nodes: ['b', 'c']}) + ]); graph = iD.actionJoin(['-', '='])(graph); @@ -301,13 +301,13 @@ describe('iD.actionJoin', function () { // Expected result: // a --> b --> c // tags on === reversed - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['c', 'b'], tags: {'lanes:forward': 2}}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['c', 'b'], tags: {'lanes:forward': 2}}) + ]); graph = iD.actionJoin(['-', '='])(graph); @@ -320,17 +320,17 @@ describe('iD.actionJoin', function () { // Expected result: // a --> b --> c --> d --> e // tags on === reversed - 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']}), - iD.Way({id: '=', nodes: ['c', 'b'], tags: {'lanes:forward': 2}}), - iD.Way({id: '+', nodes: ['d', 'c']}), - iD.Way({id: '*', nodes: ['d', 'e'], tags: {'lanes:backward': 2}}) - ]); + 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']}), + iD.osmWay({id: '=', nodes: ['c', 'b'], tags: {'lanes:forward': 2}}), + iD.osmWay({id: '+', nodes: ['d', 'c']}), + iD.osmWay({id: '*', nodes: ['d', 'e'], tags: {'lanes:backward': 2}}) + ]); graph = iD.actionJoin(['-', '=', '+', '*'])(graph); @@ -346,15 +346,15 @@ describe('iD.actionJoin', function () { // --- is new, === is existing, +++ is new // Expected result: // a ==> b ==> c ==> d - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'd'}), - iD.Way({id: 'w-1', nodes: ['a', 'b']}), - iD.Way({id: 'w1', nodes: ['b', 'c']}), - iD.Way({id: 'w-2', nodes: ['c', 'd']}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmWay({id: 'w-1', nodes: ['a', 'b']}), + iD.osmWay({id: 'w1', nodes: ['b', 'c']}), + iD.osmWay({id: 'w-2', nodes: ['c', 'd']}) + ]); graph = iD.actionJoin(['w-1', 'w1', 'w-2'])(graph); @@ -364,15 +364,15 @@ describe('iD.actionJoin', function () { }); it('merges tags', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'd'}), - iD.Way({id: '-', nodes: ['a', 'b'], tags: {a: 'a', b: '-', c: 'c'}}), - iD.Way({id: '=', nodes: ['b', 'c'], tags: {a: 'a', b: '=', d: 'd'}}), - iD.Way({id: '+', nodes: ['c', 'd'], tags: {a: 'a', b: '=', e: 'e'}}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmWay({id: '-', nodes: ['a', 'b'], tags: {a: 'a', b: '-', c: 'c'}}), + iD.osmWay({id: '=', nodes: ['b', 'c'], tags: {a: 'a', b: '=', d: 'd'}}), + iD.osmWay({id: '+', nodes: ['c', 'd'], tags: {a: 'a', b: '=', e: 'e'}}) + ]); graph = iD.actionJoin(['-', '=', '+'])(graph); @@ -380,19 +380,65 @@ describe('iD.actionJoin', function () { }); it('merges relations', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['b', 'c']}), - iD.Relation({id: 'r1', members: [{id: '=', role: 'r1', type: 'way'}]}), - iD.Relation({id: 'r2', members: [{id: '=', role: 'r2', type: 'way'}, {id: '-', role: 'r2', type: 'way'}]}) - ]); + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['b', 'c']}), + iD.osmRelation({id: 'r1', members: [ + {id: '=', role: 'r1', type: 'way'} + ]}), + iD.osmRelation({id: 'r2', members: [ + {id: '=', role: 'r2', type: 'way'}, + {id: '-', role: 'r2', type: 'way'} + ]}) + ]); graph = iD.actionJoin(['-', '='])(graph); expect(graph.entity('r1').members).to.eql([{id: '-', role: 'r1', type: 'way'}]); expect(graph.entity('r2').members).to.eql([{id: '-', role: 'r2', type: 'way'}]); }); + + it('preserves duplicate route segments in relations', function () { + // + // Situation: + // a ---> b ===> c ~~~~> d join '-' and '=' + // Relation: ['-', '=', '~', '~', '=', '-'] + // + // Expected result: + // a ---> b ---> c ~~~~> d + // Relation: ['-', '~', '~', '-'] + // + var graph = iD.coreGraph([ + iD.osmNode({ id: 'a', loc: [0, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmNode({ id: 'c', loc: [2, 0] }), + iD.osmNode({ id: 'd', loc: [3, 0] }), + iD.osmWay({ id: '-', nodes: ['a', 'b'] }), + iD.osmWay({ id: '=', nodes: ['b', 'c'] }), + iD.osmWay({ id: '~', nodes: ['c', 'd'] }), + iD.osmRelation({id: 'r', members: [ + {id: '-', role: 'forward', type: 'way'}, + {id: '=', role: 'forward', type: 'way'}, + {id: '~', role: 'forward', type: 'way'}, + {id: '~', role: 'forward', type: 'way'}, + {id: '=', role: 'forward', type: 'way'}, + {id: '-', role: 'forward', type: 'way'} + ]}) + ]); + + graph = iD.actionJoin(['-', '='])(graph); + + expect(graph.entity('-').nodes).to.eql(['a', 'b', 'c']); + expect(graph.entity('~').nodes).to.eql(['c', 'd']); + expect(graph.entity('r').members).to.eql([ + {id: '-', role: 'forward', type: 'way'}, + {id: '~', role: 'forward', type: 'way'}, + {id: '~', role: 'forward', type: 'way'}, + {id: '-', role: 'forward', type: 'way'} + ]); + }); + }); diff --git a/test/spec/osm/relation.js b/test/spec/osm/relation.js index c5135ab259..4d449d2265 100644 --- a/test/spec/osm/relation.js +++ b/test/spec/osm/relation.js @@ -258,24 +258,37 @@ describe('iD.osmRelation', function () { it('replaces a member which doesn\'t already exist', function () { var r = iD.Relation({members: [{id: 'a', role: 'a'}]}); - expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'node'}).members).to.eql([{id: 'b', role: 'a', type: 'node'}]); + expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'node'}).members) + .to.eql([{id: 'b', role: 'a', type: 'node'}]); }); it('preserves the existing role', function () { var r = iD.Relation({members: [{id: 'a', role: 'a', type: 'node'}]}); - expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'node'}).members).to.eql([{id: 'b', role: 'a', type: 'node'}]); + expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'node'}).members) + .to.eql([{id: 'b', role: 'a', type: 'node'}]); }); it('uses the replacement type', function () { var r = iD.Relation({members: [{id: 'a', role: 'a', type: 'node'}]}); - expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'way'}).members).to.eql([{id: 'b', role: 'a', type: 'way'}]); + expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'way'}).members) + .to.eql([{id: 'b', role: 'a', type: 'way'}]); }); it('removes members if replacing them would produce duplicates', function () { var r = iD.Relation({members: [ {id: 'a', role: 'b', type: 'node'}, - {id: 'b', role: 'b', type: 'node'}]}); - expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'node'}).members).to.eql([{id: 'b', role: 'b', type: 'node'}]); + {id: 'b', role: 'b', type: 'node'} + ]}); + expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'node'}).members) + .to.eql([{id: 'b', role: 'b', type: 'node'}]); + }); + it('keeps duplicate members if `keepDuplicates = true`', function () { + var r = iD.Relation({members: [ + {id: 'a', role: 'b', type: 'node'}, + {id: 'b', role: 'b', type: 'node'} + ]}); + expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'node'}, true).members) + .to.eql([{id: 'b', role: 'b', type: 'node'}, {id: 'b', role: 'b', type: 'node'}]); }); });