From 748abdb9502a12b366145e78942d4a0800992074 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 9 Jan 2018 11:41:54 -0500 Subject: [PATCH 01/15] Code formatting --- test/spec/actions/split.js | 1044 +++++++++++++++++---------------- test/spec/osm/multipolygon.js | 196 ++++--- 2 files changed, 642 insertions(+), 598 deletions(-) diff --git a/test/spec/actions/split.js b/test/spec/actions/split.js index e722929fd..d09148499 100644 --- a/test/spec/actions/split.js +++ b/test/spec/actions/split.js @@ -1,581 +1,615 @@ describe('iD.actionSplit', function () { beforeEach(function () { - iD.areaKeys = iD.Context().presets().areaKeys(); + iD.areaKeys = iD.coreContext().presets().areaKeys(); }); + describe('#disabled', function () { it('returns falsy for a non-end node of a single way', function () { - 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']}) + ]); expect(iD.actionSplit('b').disabled(graph)).not.to.be.ok; }); it('returns falsy for an intersection of two ways', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'c'}), - iD.Node({id: '*'}), - iD.Way({id: '-', nodes: ['a', '*', 'b']}), - 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: 'c'}), + iD.osmNode({id: '*'}), + iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), + iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) + ]); expect(iD.actionSplit('*').disabled(graph)).not.to.be.ok; }); it('returns falsy for an intersection of two ways with parent way specified', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'c'}), - iD.Node({id: '*'}), - iD.Way({id: '-', nodes: ['a', '*', 'b']}), - 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: 'c'}), + iD.osmNode({id: '*'}), + iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), + iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) + ]); expect(iD.actionSplit('*').limitWays(['-']).disabled(graph)).not.to.be.ok; }); it('returns falsy for a self-intersection', 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', 'c', 'a', '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', 'a', 'd']}) + ]); expect(iD.actionSplit('a').disabled(graph)).not.to.be.ok; }); it('returns \'not_eligible\' for the first node of a single way', function () { - 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']}) + ]); expect(iD.actionSplit('a').disabled(graph)).to.equal('not_eligible'); }); it('returns \'not_eligible\' for the last node of a single way', function () { - 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']}) + ]); expect(iD.actionSplit('b').disabled(graph)).to.equal('not_eligible'); }); it('returns \'not_eligible\' for an intersection of two ways with non-parent way specified', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'c'}), - iD.Node({id: '*'}), - iD.Way({id: '-', nodes: ['a', '*', 'b']}), - 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: 'c'}), + iD.osmNode({id: '*'}), + iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), + iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) + ]); expect(iD.actionSplit('*').limitWays(['-', '=']).disabled(graph)).to.equal('not_eligible'); }); }); - it('creates a new way with the appropriate nodes', function () { - // Situation: - // a ---- b ---- c - // - // Split at b. - // - // 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', 'c']}) - ]); - graph = iD.actionSplit('b', ['='])(graph); + describe('ways', function () { - expect(graph.entity('-').nodes).to.eql(['a', 'b']); - expect(graph.entity('=').nodes).to.eql(['b', 'c']); - }); - - it('copies tags to the new way', function () { - var tags = {highway: 'residential'}, - graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b', 'c'], tags: tags}) - ]); - - graph = iD.actionSplit('b', ['='])(graph); - - // Immutable tags => should be shared by identity. - expect(graph.entity('-').tags).to.equal(tags); - expect(graph.entity('=').tags).to.equal(tags); - }); - - it('splits a way at a T-junction', function () { - // Situation: - // a ---- b ---- c - // | - // d - // - // Split at b. - // - // 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: '-', nodes: ['a', 'b', 'c']}), - iD.Way({id: '|', nodes: ['d', 'b']}) - ]); - - graph = iD.actionSplit('b', ['='])(graph); - - expect(graph.entity('-').nodes).to.eql(['a', 'b']); - expect(graph.entity('=').nodes).to.eql(['b', 'c']); - expect(graph.entity('|').nodes).to.eql(['d', 'b']); - }); - - it('splits multiple ways at an intersection', function () { - // Situation: - // c - // | - // a ---- * ---- b - // ¦ - // d - // - // Split at b. - // - // Expected result: - // c - // | - // a ---- * ==== b - // ¦ - // d - // - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'c'}), - iD.Node({id: '*'}), - iD.Way({id: '-', nodes: ['a', '*', 'b']}), - iD.Way({id: '|', nodes: ['c', '*', 'd']}) - ]); - - graph = iD.actionSplit('*', ['=', '¦'])(graph); - - expect(graph.entity('-').nodes).to.eql(['a', '*']); - expect(graph.entity('=').nodes).to.eql(['*', 'b']); - expect(graph.entity('|').nodes).to.eql(['c', '*']); - expect(graph.entity('¦').nodes).to.eql(['*', 'd']); - }); - - it('splits the specified ways at an intersection', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'c'}), - iD.Node({id: '*'}), - iD.Way({id: '-', nodes: ['a', '*', 'b']}), - iD.Way({id: '|', nodes: ['c', '*', 'd']}) - ]); - - var g1 = iD.actionSplit('*', ['=']).limitWays(['-'])(graph); - expect(g1.entity('-').nodes).to.eql(['a', '*']); - expect(g1.entity('=').nodes).to.eql(['*', 'b']); - expect(g1.entity('|').nodes).to.eql(['c', '*', 'd']); - - var g2 = iD.actionSplit('*', ['¦']).limitWays(['|'])(graph); - expect(g2.entity('-').nodes).to.eql(['a', '*', 'b']); - expect(g2.entity('|').nodes).to.eql(['c', '*']); - expect(g2.entity('¦').nodes).to.eql(['*', 'd']); - - var g3 = iD.actionSplit('*', ['=', '¦']).limitWays(['-', '|'])(graph); - expect(g3.entity('-').nodes).to.eql(['a', '*']); - expect(g3.entity('=').nodes).to.eql(['*', 'b']); - expect(g3.entity('|').nodes).to.eql(['c', '*']); - expect(g3.entity('¦').nodes).to.eql(['*', 'd']); - }); - - it('splits self-intersecting ways', function () { - // Situation: - // b - // / | - // / | - // c - a -- d - // - // Split at a. - // - // Expected result: - // b - // / | - // / | - // c - a == 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', 'a', 'd']}) - ]); - - graph = iD.actionSplit('a', ['='])(graph); - - expect(graph.entity('-').nodes).to.eql(['a', 'b', 'c', 'a']); - expect(graph.entity('=').nodes).to.eql(['a', 'd']); - }); - - it('splits a closed way at the given point and its antipode', function () { - // Situation: - // a ---- b - // | | - // d ---- c - // - // Split at a. - // - // Expected result: - // a ---- b - // || | - // d ==== c - // - var graph = iD.Graph([ - iD.Node({id: 'a', loc: [0,1]}), - iD.Node({id: 'b', loc: [1,1]}), - iD.Node({id: 'c', loc: [1,0]}), - iD.Node({id: 'd', loc: [0,0]}), - iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}) - ]); - - var g1 = iD.actionSplit('a', ['='])(graph); - expect(g1.entity('-').nodes).to.eql(['a', 'b', 'c']); - expect(g1.entity('=').nodes).to.eql(['c', 'd', 'a']); - - var g2 = iD.actionSplit('b', ['='])(graph); - expect(g2.entity('-').nodes).to.eql(['b', 'c', 'd']); - expect(g2.entity('=').nodes).to.eql(['d', 'a', 'b']); - - var g3 = iD.actionSplit('c', ['='])(graph); - expect(g3.entity('-').nodes).to.eql(['c', 'd', 'a']); - expect(g3.entity('=').nodes).to.eql(['a', 'b', 'c']); - - var g4 = iD.actionSplit('d', ['='])(graph); - expect(g4.entity('-').nodes).to.eql(['d', 'a', 'b']); - expect(g4.entity('=').nodes).to.eql(['b', 'c', 'd']); - }); - - it('splits an area by converting it to a multipolygon', function () { - var graph = iD.Graph([ - iD.Node({id: 'a', loc: [0,1]}), - iD.Node({id: 'b', loc: [1,1]}), - iD.Node({id: 'c', loc: [1,0]}), - iD.Node({id: 'd', loc: [0,0]}), - iD.Way({id: '-', tags: {building: 'yes'}, nodes: ['a', 'b', 'c', 'd', 'a']}) - ]); - - graph = iD.actionSplit('a', ['='])(graph); - expect(graph.entity('-').tags).to.eql({}); - expect(graph.entity('=').tags).to.eql({}); - expect(graph.parentRelations(graph.entity('-'))).to.have.length(1); - - var relation = graph.parentRelations(graph.entity('-'))[0]; - expect(relation.tags).to.eql({type: 'multipolygon', building: 'yes'}); - expect(relation.members).to.eql([ - {id: '-', role: 'outer', type: 'way'}, - {id: '=', role: 'outer', type: 'way'} - ]); - }); - - it('splits only the line of a node shared by a line and an area', function () { - var graph = iD.Graph([ - iD.Node({id: 'a', loc: [0,1]}), - iD.Node({id: 'b', loc: [1,1]}), - iD.Node({id: 'c', loc: [1,0]}), - iD.Way({id: '-', nodes: ['a', 'b', 'c']}), - iD.Way({id: '=', nodes: ['a', 'b', 'c', 'a'], tags: {area: 'yes'}}) - ]); - - graph = iD.actionSplit('b', ['~'])(graph); - - expect(graph.entity('-').nodes).to.eql(['a', 'b']); - expect(graph.entity('~').nodes).to.eql(['b', 'c']); - expect(graph.entity('=').nodes).to.eql(['a', 'b', 'c', 'a']); - expect(graph.parentRelations(graph.entity('='))).to.have.length(0); - }); - - it('adds the new way to parent relations (no connections)', function () { - // Situation: - // a ---- b ---- c - // Relation: [----] - // - // Split at b. - // - // Expected result: - // a ---- b ==== c - // Relation: [----, ====] - // - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b', 'c']}), - iD.Relation({id: 'r', members: [{id: '-', type: 'way', role: 'forward'}]}) - ]); - - graph = iD.actionSplit('b', ['='])(graph); - - expect(graph.entity('r').members).to.eql([ - {id: '-', type: 'way', role: 'forward'}, - {id: '=', type: 'way', role: 'forward'} - ]); - }); - - it('adds the new way to parent relations (forward order)', function () { - // Situation: - // a ---- b ---- c ~~~~ d - // Relation: [----, ~~~~] - // - // Split at b. - // - // Expected result: - // a ---- b ==== c ~~~~ d - // Relation: [----, ====, ~~~~] - // - 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: ['c', 'd']}), - iD.Relation({id: 'r', members: [{id: '-', type: 'way'}, {id: '~', type: 'way'}]}) - ]); - - graph = iD.actionSplit('b', ['='])(graph); - - var ids = graph.entity('r').members.map(function(m) { return m.id; }); - expect(ids).to.have.ordered.members(['-', '=', '~']); - }); - - it('adds the new way to parent relations (reverse order)', function () { - // Situation: - // a ---- b ---- c ~~~~ d - // Relation: [~~~~, ----] - // - // Split at b. - // - // Expected result: - // a ---- b ==== c ~~~~ d - // Relation: [~~~~, ====, ----] - // - 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: ['c', 'd']}), - iD.Relation({id: 'r', members: [{id: '~', type: 'way'}, {id: '-', type: 'way'}]}) - ]); - - graph = iD.actionSplit('b', ['='])(graph); - - var ids = graph.entity('r').members.map(function(m) { return m.id; }); - expect(ids).to.have.ordered.members(['~', '=', '-']); - }); - - it('handles incomplete relations', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({id: '-', nodes: ['a', 'b', 'c']}), - iD.Relation({id: 'r', members: [{id: '~', type: 'way'}, {id: '-', type: 'way'}]}) - ]); - - graph = iD.actionSplit('b', ['='])(graph); - - var ids = graph.entity('r').members.map(function(m) { return m.id; }); - expect(ids).to.have.ordered.members(['~', '-', '=']); - }); - - it('converts simple multipolygon to a proper multipolygon', function () { - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Way({'id': '-', nodes: ['a', 'b', 'c'], tags: {natural: 'water'}}), - iD.Relation({id: 'r', members: [{id: '-', type: 'way', role: 'outer'}], tags: {type: 'multipolygon'}}) - ]); - - graph = iD.actionSplit('b', ['='])(graph); - - expect(graph.entity('-').tags).to.eql({}); - expect(graph.entity('r').tags).to.eql({type: 'multipolygon', natural: 'water'}); - var ids = graph.entity('r').members.map(function(m) { return m.id; }); - expect(ids).to.have.ordered.members(['-', '=']); - }); - - ['restriction', 'restriction:bus'].forEach(function (type) { - it('updates a restriction\'s \'from\' role', function () { + it('creates a new way with the appropriate nodes', function () { // Situation: - // a ----> b ----> c ~~~~ d - // A restriction from ---- to ~~~~ via c. + // a ---- b ---- c // // Split at b. // // Expected result: - // a ----> b ====> c ~~~~ d - // A restriction from ==== to ~~~~ via c. + // a ---- b ==== c // - 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: ['c', 'd']}), - iD.Relation({id: 'r', tags: {type: type}, members: [ - {id: '-', role: 'from', type: 'way'}, - {id: '~', role: 'to', type: 'way'}, - {id: 'c', role: 'via', type: 'node'} - ]}) - ]); + 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.actionSplit('b', ['='])(graph); - expect(graph.entity('r').members).to.eql([ - {id: '=', role: 'from', type: 'way'}, - {id: '~', role: 'to', type: 'way'}, - {id: 'c', role: 'via', type: 'node'} - ]); + expect(graph.entity('-').nodes).to.eql(['a', 'b']); + expect(graph.entity('=').nodes).to.eql(['b', 'c']); }); - it('updates a restriction\'s \'to\' role', function () { + it('copies tags to the new way', function () { + var tags = {highway: 'residential'}; + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b', 'c'], tags: tags}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + // Immutable tags => should be shared by identity. + expect(graph.entity('-').tags).to.equal(tags); + expect(graph.entity('=').tags).to.equal(tags); + }); + + it('splits a way at a T-junction', function () { // Situation: - // a ----> b ----> c ~~~~ d - // A restriction from ~~~~ to ---- via c. + // a ---- b ---- c + // | + // d // // Split at b. // // Expected result: - // a ----> b ====> c ~~~~ d - // A restriction from ~~~~ to ==== via c. + // 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: ['c', 'd']}), - iD.Relation({id: 'r', tags: {type: type}, members: [ + 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: ['d', 'b']}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['a', 'b']); + expect(graph.entity('=').nodes).to.eql(['b', 'c']); + expect(graph.entity('|').nodes).to.eql(['d', 'b']); + }); + + it('splits multiple ways at an intersection', function () { + // Situation: + // c + // | + // a ---- * ---- b + // ¦ + // d + // + // Split at b. + // + // Expected result: + // c + // | + // a ---- * ==== b + // ¦ + // d + // + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: '*'}), + iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), + iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) + ]); + + graph = iD.actionSplit('*', ['=', '¦'])(graph); + + expect(graph.entity('-').nodes).to.eql(['a', '*']); + expect(graph.entity('=').nodes).to.eql(['*', 'b']); + expect(graph.entity('|').nodes).to.eql(['c', '*']); + expect(graph.entity('¦').nodes).to.eql(['*', 'd']); + }); + + it('splits the specified ways at an intersection', function () { + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: '*'}), + iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), + iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) + ]); + + var g1 = iD.actionSplit('*', ['=']).limitWays(['-'])(graph); + expect(g1.entity('-').nodes).to.eql(['a', '*']); + expect(g1.entity('=').nodes).to.eql(['*', 'b']); + expect(g1.entity('|').nodes).to.eql(['c', '*', 'd']); + + var g2 = iD.actionSplit('*', ['¦']).limitWays(['|'])(graph); + expect(g2.entity('-').nodes).to.eql(['a', '*', 'b']); + expect(g2.entity('|').nodes).to.eql(['c', '*']); + expect(g2.entity('¦').nodes).to.eql(['*', 'd']); + + var g3 = iD.actionSplit('*', ['=', '¦']).limitWays(['-', '|'])(graph); + expect(g3.entity('-').nodes).to.eql(['a', '*']); + expect(g3.entity('=').nodes).to.eql(['*', 'b']); + expect(g3.entity('|').nodes).to.eql(['c', '*']); + expect(g3.entity('¦').nodes).to.eql(['*', 'd']); + }); + + it('splits self-intersecting ways', function () { + // Situation: + // b + // / | + // / | + // c - a -- d + // + // Split at a. + // + // Expected result: + // b + // / | + // / | + // c - a == 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', 'a', 'd']}) + ]); + + graph = iD.actionSplit('a', ['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['a', 'b', 'c', 'a']); + expect(graph.entity('=').nodes).to.eql(['a', 'd']); + }); + + it('splits a closed way at the given point and its antipode', function () { + // Situation: + // a ---- b + // | | + // d ---- c + // + // Split at a. + // + // Expected result: + // a ---- b + // || | + // d ==== c + // + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0,1]}), + iD.osmNode({id: 'b', loc: [1,1]}), + iD.osmNode({id: 'c', loc: [1,0]}), + iD.osmNode({id: 'd', loc: [0,0]}), + iD.osmWay({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}) + ]); + + var g1 = iD.actionSplit('a', ['='])(graph); + expect(g1.entity('-').nodes).to.eql(['a', 'b', 'c']); + expect(g1.entity('=').nodes).to.eql(['c', 'd', 'a']); + + var g2 = iD.actionSplit('b', ['='])(graph); + expect(g2.entity('-').nodes).to.eql(['b', 'c', 'd']); + expect(g2.entity('=').nodes).to.eql(['d', 'a', 'b']); + + var g3 = iD.actionSplit('c', ['='])(graph); + expect(g3.entity('-').nodes).to.eql(['c', 'd', 'a']); + expect(g3.entity('=').nodes).to.eql(['a', 'b', 'c']); + + var g4 = iD.actionSplit('d', ['='])(graph); + expect(g4.entity('-').nodes).to.eql(['d', 'a', 'b']); + expect(g4.entity('=').nodes).to.eql(['b', 'c', 'd']); + }); + }); + + + describe('relations', function () { + + it('handles incomplete relations', function () { + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), + iD.osmRelation({id: 'r', members: [{id: '~', type: 'way'}, {id: '-', type: 'way'}]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + var ids = graph.entity('r').members.map(function(m) { return m.id; }); + expect(ids).to.have.ordered.members(['~', '-', '=']); + }); + + + describe('member ordering', function () { + + it('adds the new way to parent relations (no connections)', function () { + // Situation: + // a ---- b ---- c + // Relation: [----] + // + // Split at b. + // + // Expected result: + // a ---- b ==== c + // Relation: [----, ====] + // + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), + iD.osmRelation({id: 'r', members: [{id: '-', type: 'way', role: 'forward'}]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + expect(graph.entity('r').members).to.eql([ + {id: '-', type: 'way', role: 'forward'}, + {id: '=', type: 'way', role: 'forward'} + ]); + }); + + it('adds the new way to parent relations (forward order)', function () { + // Situation: + // a ---- b ---- c ~~~~ d + // Relation: [----, ~~~~] + // + // Split at b. + // + // Expected result: + // a ---- b ==== c ~~~~ d + // Relation: [----, ====, ~~~~] + // + 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: ['c', 'd']}), + iD.osmRelation({id: 'r', members: [{id: '-', type: 'way'}, {id: '~', type: 'way'}]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + var ids = graph.entity('r').members.map(function(m) { return m.id; }); + expect(ids).to.have.ordered.members(['-', '=', '~']); + }); + + it('adds the new way to parent relations (reverse order)', function () { + // Situation: + // a ---- b ---- c ~~~~ d + // Relation: [~~~~, ----] + // + // Split at b. + // + // Expected result: + // a ---- b ==== c ~~~~ d + // Relation: [~~~~, ====, ----] + // + 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: ['c', 'd']}), + iD.osmRelation({id: 'r', members: [{id: '~', type: 'way'}, {id: '-', type: 'way'}]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + var ids = graph.entity('r').members.map(function(m) { return m.id; }); + expect(ids).to.have.ordered.members(['~', '=', '-']); + }); + }); + + + describe('type = multipolygon', function () { + + it('splits an area by converting it to a multipolygon', function () { + // Situation: + // a ---- b + // | | + // d ---- c + // + // Split at a. + // + // Expected result: + // a ---- b + // || | + // d ==== c + // + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0,1]}), + iD.osmNode({id: 'b', loc: [1,1]}), + iD.osmNode({id: 'c', loc: [1,0]}), + iD.osmNode({id: 'd', loc: [0,0]}), + iD.osmWay({id: '-', tags: {building: 'yes'}, nodes: ['a', 'b', 'c', 'd', 'a']}) + ]); + + graph = iD.actionSplit('a', ['='])(graph); + expect(graph.entity('-').tags).to.eql({}); + expect(graph.entity('=').tags).to.eql({}); + expect(graph.parentRelations(graph.entity('-'))).to.have.length(1); + + var relation = graph.parentRelations(graph.entity('-'))[0]; + expect(relation.tags).to.eql({type: 'multipolygon', building: 'yes'}); + expect(relation.members).to.eql([ + {id: '-', role: 'outer', type: 'way'}, + {id: '=', role: 'outer', type: 'way'} + ]); + }); + + it('splits only the line of a node shared by a line and an area', function () { + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0,1]}), + iD.osmNode({id: 'b', loc: [1,1]}), + iD.osmNode({id: 'c', loc: [1,0]}), + iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), + iD.osmWay({id: '=', nodes: ['a', 'b', 'c', 'a'], tags: {area: 'yes'}}) + ]); + + graph = iD.actionSplit('b', ['~'])(graph); + + expect(graph.entity('-').nodes).to.eql(['a', 'b']); + expect(graph.entity('~').nodes).to.eql(['b', 'c']); + expect(graph.entity('=').nodes).to.eql(['a', 'b', 'c', 'a']); + expect(graph.parentRelations(graph.entity('='))).to.have.length(0); + }); + + it('converts simple multipolygon to a proper multipolygon', function () { + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmWay({'id': '-', nodes: ['a', 'b', 'c'], tags: {natural: 'water'}}), + iD.osmRelation({id: 'r', members: [{id: '-', type: 'way', role: 'outer'}], tags: {type: 'multipolygon'}}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + expect(graph.entity('-').tags).to.eql({}); + expect(graph.entity('r').tags).to.eql({type: 'multipolygon', natural: 'water'}); + var ids = graph.entity('r').members.map(function(m) { return m.id; }); + expect(ids).to.have.ordered.members(['-', '=']); + }); + }); + + + ['restriction', 'restriction:bus'].forEach(function (type) { + describe('type = ' + type, function () { + + it('updates a restriction\'s \'from\' role', function () { + // Situation: + // a ----> b ----> c ~~~~ d + // A restriction from ---- to ~~~~ via c. + // + // Split at b. + // + // Expected result: + // a ----> b ====> c ~~~~ d + // A restriction from ==== to ~~~~ via c. + // + 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: ['c', 'd']}), + iD.osmRelation({id: 'r', tags: {type: type}, members: [ + {id: '-', role: 'from', type: 'way'}, + {id: '~', role: 'to', type: 'way'}, + {id: 'c', role: 'via', type: 'node'} + ]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + expect(graph.entity('r').members).to.eql([ + {id: '=', role: 'from', type: 'way'}, + {id: '~', role: 'to', type: 'way'}, + {id: 'c', role: 'via', type: 'node'} + ]); + }); + + it('updates a restriction\'s \'to\' role', function () { + // Situation: + // a ----> b ----> c ~~~~ d + // A restriction from ~~~~ to ---- via c. + // + // Split at b. + // + // Expected result: + // a ----> b ====> c ~~~~ d + // A restriction from ~~~~ to ==== via c. + // + 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: ['c', 'd']}), + iD.osmRelation({id: 'r', tags: {type: type}, members: [ + {id: '~', role: 'from', type: 'way'}, + {id: '-', role: 'to', type: 'way'}, + {id: 'c', role: 'via', type: 'node'} + ]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + expect(graph.entity('r').members).to.eql([ {id: '~', role: 'from', type: 'way'}, - {id: '-', role: 'to', type: 'way'}, + {id: '=', role: 'to', type: 'way'}, {id: 'c', role: 'via', type: 'node'} - ]}) - ]); - - graph = iD.actionSplit('b', ['='])(graph); - - expect(graph.entity('r').members).to.eql([ - {id: '~', role: 'from', type: 'way'}, - {id: '=', role: 'to', type: 'way'}, - {id: 'c', role: 'via', type: 'node'} - ]); - }); + ]); + }); - it('updates both \'to\' and \'from\' roles for u-turn restrictions', function () { - // Situation: - // a ----> b ----> c ~~~~ d - // A restriction from ---- to ---- via c. - // - // Split at b. - // - // Expected result: - // a ----> b ====> c ~~~~ d - // A restriction from ==== to ==== via c. - // - 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: ['c', 'd']}), - iD.Relation({id: 'r', tags: {type: type}, members: [ - {id: '-', role: 'from', type: 'way'}, - {id: '-', role: 'to', type: 'way'}, + it('updates both \'to\' and \'from\' roles for u-turn restrictions', function () { + // Situation: + // a ----> b ----> c ~~~~ d + // A restriction from ---- to ---- via c. + // + // Split at b. + // + // Expected result: + // a ----> b ====> c ~~~~ d + // A restriction from ==== to ==== via c. + // + 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: ['c', 'd']}), + iD.osmRelation({id: 'r', tags: {type: type}, members: [ + {id: '-', role: 'from', type: 'way'}, + {id: '-', role: 'to', type: 'way'}, + {id: 'c', role: 'via', type: 'node'} + ]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + expect(graph.entity('r').members).to.eql([ + {id: '=', role: 'from', type: 'way'}, + {id: '=', role: 'to', type: 'way'}, {id: 'c', role: 'via', type: 'node'} - ]}) - ]); + ]); + }); - graph = iD.actionSplit('b', ['='])(graph); + it('leaves unaffected restrictions unchanged', function () { + // Situation: + // a <---- b <---- c ~~~~ d + // A restriction from ---- to ~~~~ via c. + // + // Split at b. + // + // Expected result: + // a <==== b <---- c ~~~~ d + // A restriction from ---- to ~~~~ via c. + // + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmWay({id: '-', nodes: ['c', 'b', 'a']}), + iD.osmWay({id: '~', nodes: ['c', 'd']}), + iD.osmRelation({id: 'r', tags: {type: type}, members: [ + {id: '-', role: 'from', type: 'way'}, + {id: '~', role: 'to', type: 'way'}, + {id: 'c', role: 'via', type: 'node'} + ]}) + ]); - expect(graph.entity('r').members).to.eql([ - {id: '=', role: 'from', type: 'way'}, - {id: '=', role: 'to', type: 'way'}, - {id: 'c', role: 'via', type: 'node'} - ]); - }); + graph = iD.actionSplit('b', ['='])(graph); - it('leaves unaffected restrictions unchanged', function () { - // Situation: - // a <---- b <---- c ~~~~ d - // A restriction from ---- to ~~~~ via c. - // - // Split at b. - // - // Expected result: - // a <==== b <---- c ~~~~ d - // A restriction from ---- to ~~~~ via c. - // - var graph = iD.Graph([ - iD.Node({id: 'a'}), - iD.Node({id: 'b'}), - iD.Node({id: 'c'}), - iD.Node({id: 'd'}), - iD.Way({id: '-', nodes: ['c', 'b', 'a']}), - iD.Way({id: '~', nodes: ['c', 'd']}), - iD.Relation({id: 'r', tags: {type: type}, members: [ + expect(graph.entity('r').members).to.eql([ {id: '-', role: 'from', type: 'way'}, {id: '~', role: 'to', type: 'way'}, {id: 'c', role: 'via', type: 'node'} - ]}) - ]); + ]); + }); + }); - graph = iD.actionSplit('b', ['='])(graph); - - expect(graph.entity('r').members).to.eql([ - {id: '-', role: 'from', type: 'way'}, - {id: '~', role: 'to', type: 'way'}, - {id: 'c', role: 'via', type: 'node'} - ]); }); }); }); diff --git a/test/spec/osm/multipolygon.js b/test/spec/osm/multipolygon.js index 6d0c9fb0b..aea1d8a68 100644 --- a/test/spec/osm/multipolygon.js +++ b/test/spec/osm/multipolygon.js @@ -1,91 +1,101 @@ describe('iD.osmIsSimpleMultipolygonOuterMember', function() { it('returns the parent relation of a simple multipolygon outer', function() { - var outer = iD.Way({tags: {'natural':'wood'}}), - relation = iD.Relation({tags: {type: 'multipolygon'}, - members: [{id: outer.id, role: 'outer'}]}), - graph = iD.Graph([outer, relation]); + var outer = iD.osmWay({tags: {'natural':'wood'}}); + var relation = iD.osmRelation( + {tags: {type: 'multipolygon'}, members: [{id: outer.id, role: 'outer'}]} + ); + var graph = iD.coreGraph([outer, relation]); expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.equal(relation); }); it('returns the parent relation of a simple multipolygon outer, assuming role outer if unspecified', function() { - var outer = iD.Way({tags: {'natural':'wood'}}), - relation = iD.Relation({tags: {type: 'multipolygon'}, - members: [{id: outer.id}]}), - graph = iD.Graph([outer, relation]); + var outer = iD.osmWay({tags: {'natural':'wood'}}); + var relation = iD.osmRelation( + {tags: {type: 'multipolygon'}, members: [{id: outer.id}]} + ); + var graph = iD.coreGraph([outer, relation]); expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.equal(relation); }); it('returns false if entity is not a way', function() { - var outer = iD.Node({tags: {'natural':'wood'}}), - relation = iD.Relation({tags: {type: 'multipolygon'}, - members: [{id: outer.id, role: 'outer'}]}), - graph = iD.Graph([outer, relation]); + var outer = iD.osmNode({tags: {'natural':'wood'}}); + var relation = iD.osmRelation( + {tags: {type: 'multipolygon'}, members: [{id: outer.id, role: 'outer'}]} + ); + var graph = iD.coreGraph([outer, relation]); expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false; }); it('returns false if entity does not have interesting tags', function() { - var outer = iD.Way({tags: {'tiger:reviewed':'no'}}), - relation = iD.Relation({tags: {type: 'multipolygon'}, - members: [{id: outer.id, role: 'outer'}]}), - graph = iD.Graph([outer, relation]); + var outer = iD.osmWay({tags: {'tiger:reviewed':'no'}}); + var relation = iD.osmRelation( + {tags: {type: 'multipolygon'}, members: [{id: outer.id, role: 'outer'}]} + ); + var graph = iD.coreGraph([outer, relation]); expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false; }); it('returns false if entity does not have a parent relation', function() { - var outer = iD.Way({tags: {'natural':'wood'}}), - graph = iD.Graph([outer]); + var outer = iD.osmWay({tags: {'natural':'wood'}}); + var graph = iD.coreGraph([outer]); expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false; }); it('returns false if the parent is not a multipolygon', function() { - var outer = iD.Way({tags: {'natural':'wood'}}), - relation = iD.Relation({tags: {type: 'route'}, - members: [{id: outer.id, role: 'outer'}]}), - graph = iD.Graph([outer, relation]); + var outer = iD.osmWay({tags: {'natural':'wood'}}); + var relation = iD.osmRelation( + {tags: {type: 'route'}, members: [{id: outer.id, role: 'outer'}]} + ); + var graph = iD.coreGraph([outer, relation]); expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false; }); it('returns false if the parent has interesting tags', function() { - var outer = iD.Way({tags: {'natural':'wood'}}), - relation = iD.Relation({tags: {natural: 'wood', type: 'multipolygon'}, - members: [{id: outer.id, role: 'outer'}]}), - graph = iD.Graph([outer, relation]); + var outer = iD.osmWay({tags: {'natural':'wood'}}); + var relation = iD.osmRelation( + {tags: {natural: 'wood', type: 'multipolygon'}, members: [{id: outer.id, role: 'outer'}]} + ); + var graph = iD.coreGraph([outer, relation]); expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false; }); it('returns the parent relation of a simple multipolygon outer, ignoring uninteresting parent tags', function() { - var outer = iD.Way({tags: {'natural':'wood'}}), - relation = iD.Relation({tags: {'tiger:reviewed':'no', type: 'multipolygon'}, - members: [{id: outer.id, role: 'outer'}]}), - graph = iD.Graph([outer, relation]); + var outer = iD.osmWay({tags: {'natural':'wood'}}); + var relation = iD.osmRelation( + {tags: {'tiger:reviewed':'no', type: 'multipolygon'}, members: [{id: outer.id, role: 'outer'}]} + ); + var graph = iD.coreGraph([outer, relation]); expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.equal(relation); }); it('returns false if the parent has multiple outer ways', function() { - var outer1 = iD.Way({tags: {'natural':'wood'}}), - outer2 = iD.Way({tags: {'natural':'wood'}}), - relation = iD.Relation({tags: {type: 'multipolygon'}, - members: [{id: outer1.id, role: 'outer'}, {id: outer2.id, role: 'outer'}]}), - graph = iD.Graph([outer1, outer2, relation]); + var outer1 = iD.osmWay({tags: {'natural':'wood'}}); + var outer2 = iD.osmWay({tags: {'natural':'wood'}}); + var relation = iD.osmRelation( + {tags: {type: 'multipolygon'}, members: [{id: outer1.id, role: 'outer'}, {id: outer2.id, role: 'outer'}]} + ); + var graph = iD.coreGraph([outer1, outer2, relation]); expect(iD.osmIsSimpleMultipolygonOuterMember(outer1, graph)).to.be.false; expect(iD.osmIsSimpleMultipolygonOuterMember(outer2, graph)).to.be.false; }); it('returns false if the parent has multiple outer ways, assuming role outer if unspecified', function() { - var outer1 = iD.Way({tags: {'natural':'wood'}}), - outer2 = iD.Way({tags: {'natural':'wood'}}), - relation = iD.Relation({tags: {type: 'multipolygon'}, - members: [{id: outer1.id}, {id: outer2.id}]}), - graph = iD.Graph([outer1, outer2, relation]); + var outer1 = iD.osmWay({tags: {'natural':'wood'}}); + var outer2 = iD.osmWay({tags: {'natural':'wood'}}); + var relation = iD.osmRelation( + {tags: {type: 'multipolygon'}, members: [{id: outer1.id}, {id: outer2.id}]} + ); + var graph = iD.coreGraph([outer1, outer2, relation]); expect(iD.osmIsSimpleMultipolygonOuterMember(outer1, graph)).to.be.false; expect(iD.osmIsSimpleMultipolygonOuterMember(outer2, graph)).to.be.false; }); it('returns false if the entity is not an outer', function() { - var inner = iD.Way({tags: {'natural':'wood'}}), - relation = iD.Relation({tags: {type: 'multipolygon'}, - members: [{id: inner.id, role: 'inner'}]}), - graph = iD.Graph([inner, relation]); + var inner = iD.osmWay({tags: {'natural':'wood'}}); + var relation = iD.osmRelation( + {tags: {type: 'multipolygon'}, members: [{id: inner.id, role: 'inner'}]} + ); + var graph = iD.coreGraph([inner, relation]); expect(iD.osmIsSimpleMultipolygonOuterMember(inner, graph)).to.be.false; }); }); @@ -93,28 +103,28 @@ describe('iD.osmIsSimpleMultipolygonOuterMember', function() { describe('iD.osmSimpleMultipolygonOuterMember', function() { it('returns the outer member of a simple multipolygon', function() { - var inner = iD.Way(), - outer = iD.Way({tags: {'natural':'wood'}}), - relation = iD.Relation({tags: {type: 'multipolygon'}, members: [ - {id: outer.id, role: 'outer'}, - {id: inner.id, role: 'inner'}] - }), - graph = iD.Graph([inner, outer, relation]); + var inner = iD.osmWay(); + var outer = iD.osmWay({tags: {'natural':'wood'}}); + var relation = iD.osmRelation({tags: {type: 'multipolygon'}, members: [ + {id: outer.id, role: 'outer'}, + {id: inner.id, role: 'inner'}] + }); + var graph = iD.coreGraph([inner, outer, relation]); expect(iD.osmSimpleMultipolygonOuterMember(inner, graph)).to.equal(outer); expect(iD.osmSimpleMultipolygonOuterMember(outer, graph)).to.equal(outer); }); it('returns falsy for a complex multipolygon', function() { - var inner = iD.Way(), - outer1 = iD.Way({tags: {'natural':'wood'}}), - outer2 = iD.Way({tags: {'natural':'wood'}}), - relation = iD.Relation({tags: {type: 'multipolygon'}, members: [ - {id: outer1.id, role: 'outer'}, - {id: outer2.id, role: 'outer'}, - {id: inner.id, role: 'inner'}] - }), - graph = iD.Graph([inner, outer1, outer2, relation]); + var inner = iD.osmWay(); + var outer1 = iD.osmWay({tags: {'natural':'wood'}}); + var outer2 = iD.osmWay({tags: {'natural':'wood'}}); + var relation = iD.osmRelation({tags: {type: 'multipolygon'}, members: [ + {id: outer1.id, role: 'outer'}, + {id: outer2.id, role: 'outer'}, + {id: inner.id, role: 'inner'}] + }); + var graph = iD.coreGraph([inner, outer1, outer2, relation]); expect(iD.osmSimpleMultipolygonOuterMember(inner, graph)).not.to.be.ok; expect(iD.osmSimpleMultipolygonOuterMember(outer1, graph)).not.to.be.ok; @@ -122,12 +132,12 @@ describe('iD.osmSimpleMultipolygonOuterMember', function() { }); it('handles incomplete relations', function() { - var way = iD.Way({id: 'w'}), - relation = iD.Relation({id: 'r', tags: {type: 'multipolygon'}, members: [ - {id: 'o', role: 'outer'}, - {id: 'w', role: 'inner'}] - }), - graph = iD.Graph([way, relation]); + var way = iD.osmWay({id: 'w'}); + var relation = iD.osmRelation({id: 'r', tags: {type: 'multipolygon'}, members: [ + {id: 'o', role: 'outer'}, + {id: 'w', role: 'inner'}] + }); + var graph = iD.coreGraph([way, relation]); expect(iD.osmSimpleMultipolygonOuterMember(way, graph)).not.to.be.ok; }); @@ -136,11 +146,11 @@ describe('iD.osmSimpleMultipolygonOuterMember', function() { describe('iD.osmJoinWays', function() { it('returns an array of members with nodes properties', function() { - var node = iD.Node({loc: [0, 0]}), - way = iD.Way({nodes: [node.id]}), - member = {id: way.id, type: 'way'}, - graph = iD.Graph([node, way]), - result = iD.osmJoinWays([member], graph); + var node = iD.osmNode({loc: [0, 0]}); + var way = iD.osmWay({nodes: [node.id]}); + var member = {id: way.id, type: 'way'}; + var graph = iD.coreGraph([node, way]); + var result = iD.osmJoinWays([member], graph); expect(result.length).to.equal(1); expect(result[0].nodes.length).to.equal(1); @@ -150,16 +160,16 @@ describe('iD.osmJoinWays', function() { }); it('returns the members in the correct order', function() { - // a<===b--->c~~~>d - var graph = iD.Graph([ - iD.Node({id: 'a', loc: [0, 0]}), - iD.Node({id: 'b', loc: [0, 0]}), - iD.Node({id: 'c', loc: [0, 0]}), - iD.Node({id: 'd', loc: [0, 0]}), - iD.Way({id: '=', nodes: ['b', 'a']}), - iD.Way({id: '-', nodes: ['b', 'c']}), - iD.Way({id: '~', nodes: ['c', 'd']}), - iD.Relation({id: 'r', members: [ + // a <=== b ---> c ~~~> d + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0, 0]}), + iD.osmNode({id: 'b', loc: [0, 0]}), + iD.osmNode({id: 'c', loc: [0, 0]}), + iD.osmNode({id: 'd', loc: [0, 0]}), + iD.osmWay({id: '=', nodes: ['b', 'a']}), + iD.osmWay({id: '-', nodes: ['b', 'c']}), + iD.osmWay({id: '~', nodes: ['c', 'd']}), + iD.osmRelation({id: 'r', members: [ {id: '-', type: 'way'}, {id: '~', type: 'way'}, {id: '=', type: 'way'} @@ -176,28 +186,28 @@ describe('iD.osmJoinWays', 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: {'oneway': 'yes', '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: {'oneway': 'yes', 'lanes:forward': 2}}) + ]); var result = iD.osmJoinWays([graph.entity('-'), graph.entity('=')], graph); expect(result[0][1].tags).to.eql({'oneway': '-1', 'lanes:backward': 2}); }); it('ignores non-way members', function() { - var node = iD.Node({loc: [0, 0]}), - member = {id: 'n', type: 'node'}, - graph = iD.Graph([node]); + var node = iD.osmNode({loc: [0, 0]}); + var member = {id: 'n', type: 'node'}; + var graph = iD.coreGraph([node]); expect(iD.osmJoinWays([member], graph)).to.eql([]); }); it('ignores incomplete members', function() { - var member = {id: 'w', type: 'way'}, - graph = iD.Graph(); + var member = {id: 'w', type: 'way'}; + var graph = iD.coreGraph(); expect(iD.osmJoinWays([member], graph)).to.eql([]); }); }); From 07262fa7118c8cf4e111ebe173bafbeafaf9928a Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 9 Jan 2018 23:57:44 -0500 Subject: [PATCH 02/15] Add tests for #4589 --- test/spec/actions/add_member.js | 158 ++++++++++++++++++++++---------- test/spec/actions/split.js | 62 +++++++++++++ test/spec/osm/multipolygon.js | 32 +++++++ 3 files changed, 203 insertions(+), 49 deletions(-) diff --git a/test/spec/actions/add_member.js b/test/spec/actions/add_member.js index e5a5801c6..0d220b59a 100644 --- a/test/spec/actions/add_member.js +++ b/test/spec/actions/add_member.js @@ -1,7 +1,7 @@ describe('iD.actionAddMember', function() { it('adds an member to a relation at the specified index', function() { - var r = iD.Relation({members: [{id: '1'}, {id: '3'}]}), - g = iD.actionAddMember(r.id, {id: '2'}, 1)(iD.Graph([r])); + var r = iD.osmRelation({members: [{id: '1'}, {id: '3'}]}); + var g = iD.actionAddMember(r.id, {id: '2'}, 1)(iD.coreGraph([r])); expect(g.entity(r.id).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]); }); @@ -10,81 +10,141 @@ describe('iD.actionAddMember', function() { return graph.entity('r').members.map(function (m) { return m.id; }); } - specify('no members', function() { - var graph = iD.Graph([ - iD.Node({id: 'a', loc: [0, 0]}), - iD.Node({id: 'b', loc: [0, 0]}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Relation({id: 'r'}) + it('adds the member to a relation with no members', function() { + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0, 0]}), + iD.osmNode({id: 'b', loc: [0, 0]}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmRelation({id: 'r'}) ]); graph = iD.actionAddMember('r', {id: '-', type: 'way'})(graph); expect(members(graph)).to.eql(['-']); }); - specify('not connecting', function() { - // a--->b c===>d - var graph = iD.Graph([ - iD.Node({id: 'a', loc: [0, 0]}), - iD.Node({id: 'b', loc: [0, 0]}), - iD.Node({id: 'c', loc: [0, 0]}), - iD.Node({id: 'd', loc: [0, 0]}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['c', 'd']}), - iD.Relation({id: 'r', members: [{id: '-', type: 'way'}]}) + it('appends the member if the ways are not connecting', function() { + // Before: ---> + // After: ---> ... ===> + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0, 0]}), + iD.osmNode({id: 'b', loc: [0, 0]}), + iD.osmNode({id: 'c', loc: [0, 0]}), + iD.osmNode({id: 'd', loc: [0, 0]}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['c', 'd']}), + iD.osmRelation({id: 'r', members: [ + {id: '-', type: 'way'} + ]}) ]); graph = iD.actionAddMember('r', {id: '=', type: 'way'})(graph); expect(members(graph)).to.eql(['-', '=']); }); - specify('connecting at end', function() { - // a--->b===>c - var graph = iD.Graph([ - iD.Node({id: 'a', loc: [0, 0]}), - iD.Node({id: 'b', loc: [0, 0]}), - iD.Node({id: 'c', loc: [0, 0]}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['b', 'c']}), - iD.Relation({id: 'r', members: [{id: '-', type: 'way'}]}) + it('appends the member if the way connects at end', function() { + // Before: ---> + // After: ---> ===> + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0, 0]}), + iD.osmNode({id: 'b', loc: [0, 0]}), + iD.osmNode({id: 'c', loc: [0, 0]}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['b', 'c']}), + iD.osmRelation({id: 'r', members: [ + {id: '-', type: 'way'} + ]}) ]); graph = iD.actionAddMember('r', {id: '=', type: 'way'})(graph); expect(members(graph)).to.eql(['-', '=']); }); - specify('connecting at beginning', function() { - // a===>b--->c~~~>d - var graph = iD.Graph([ - iD.Node({id: 'a', loc: [0, 0]}), - iD.Node({id: 'b', loc: [0, 0]}), - iD.Node({id: 'c', loc: [0, 0]}), - iD.Node({id: 'd', loc: [0, 0]}), - iD.Way({id: '=', nodes: ['a', 'b']}), - iD.Way({id: '-', nodes: ['b', 'c']}), - iD.Way({id: '~', nodes: ['c', 'd']}), - iD.Relation({id: 'r', members: [{id: '-', type: 'way'}, {id: '~', type: 'way'}]}) + it('inserts the member if the way connects at beginning', function() { + // Before: ---> ~~~> + // After: ===> ---> ~~~> + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0, 0]}), + iD.osmNode({id: 'b', loc: [0, 0]}), + iD.osmNode({id: 'c', loc: [0, 0]}), + iD.osmNode({id: 'd', loc: [0, 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: '-', type: 'way'}, + {id: '~', type: 'way'} + ]}) ]); graph = iD.actionAddMember('r', {id: '=', type: 'way'})(graph); expect(members(graph)).to.eql(['=', '-', '~']); }); - specify('connecting in middle', function() { - // a--->b===>c~~~>d - var graph = iD.Graph([ - iD.Node({id: 'a', loc: [0, 0]}), - iD.Node({id: 'b', loc: [0, 0]}), - iD.Node({id: 'c', loc: [0, 0]}), - iD.Node({id: 'd', loc: [0, 0]}), - iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['b', 'c']}), - iD.Way({id: '~', nodes: ['c', 'd']}), - iD.Relation({id: 'r', members: [{id: '-', type: 'way'}, {id: '~', type: 'way'}]}) + it('inserts the member if the way connects in middle', function() { + // Before: ---> ~~~> + // After: ---> ===> ~~~> + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0, 0]}), + iD.osmNode({id: 'b', loc: [0, 0]}), + iD.osmNode({id: 'c', loc: [0, 0]}), + iD.osmNode({id: 'd', loc: [0, 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: '-', type: 'way'}, + {id: '~', type: 'way'} + ]}) ]); graph = iD.actionAddMember('r', {id: '=', type: 'way'})(graph); expect(members(graph)).to.eql(['-', '=', '~']); }); + + it('inserts the member multiple times if the way exists multiple times (middle)', function() { + // Before: ---> ~~~> ---> + // After: ---> ===> ~~~> ===> ---> + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0, 0]}), + iD.osmNode({id: 'b', loc: [0, 0]}), + iD.osmNode({id: 'c', loc: [0, 0]}), + iD.osmNode({id: 'd', loc: [0, 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: '-', type: 'way'}, + {id: '~', type: 'way'}, + {id: '-', type: 'way'} + ]}) + ]); + + graph = iD.actionAddMember('r', {id: '=', type: 'way'})(graph); + expect(members(graph)).to.eql(['-', '=', '~', '=', '-']); + }); + + it('inserts the member multiple times if the way exists multiple times (beginning/end)', function() { + // Before: ===> ~~~> ===> + // After: ---> ===> ~~~> ===> ---> + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0, 0]}), + iD.osmNode({id: 'b', loc: [0, 0]}), + iD.osmNode({id: 'c', loc: [0, 0]}), + iD.osmNode({id: 'd', loc: [0, 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: '=', type: 'way'}, + {id: '~', type: 'way'}, + {id: '=', type: 'way'} + ]}) + ]); + + graph = iD.actionAddMember('r', {id: '-', type: 'way'})(graph); + expect(members(graph)).to.eql(['-', '=', '~', '=', '-']); + }); + + }); }); diff --git a/test/spec/actions/split.js b/test/spec/actions/split.js index d09148499..cc5fd31c4 100644 --- a/test/spec/actions/split.js +++ b/test/spec/actions/split.js @@ -394,6 +394,68 @@ describe('iD.actionSplit', function () { var ids = graph.entity('r').members.map(function(m) { return m.id; }); expect(ids).to.have.ordered.members(['~', '=', '-']); }); + + it('adds the new way to parent relations (unsplit way belongs multiple times)', function () { + // Situation: + // a ---- b ---- c ~~~~ d + // Relation: [~~~~, ----, ~~~~] + // + // Split at b. + // + // Expected result: + // a ---- b ==== c ~~~~ d + // Relation: [~~~~, ====, ----, ====, ~~~~] + // + 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: ['c', 'd']}), + iD.osmRelation({id: 'r', members: [ + {id: '~', type: 'way'}, + {id: '-', type: 'way'}, + {id: '~', type: 'way'} + ]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + var ids = graph.entity('r').members.map(function(m) { return m.id; }); + expect(ids).to.have.ordered.members(['~', '=', '-', '=', '~']); + }); + + it('adds the new way to parent relations (split way belongs multiple times)', function () { + // Situation: + // a ---- b ---- c ~~~~ d + // Relation: [----, ~~~~, ----] + // + // Split at b. + // + // Expected result: + // a ---- b ==== c ~~~~ d + // Relation: [----, ====, ~~~~, ====, ----] + // + 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: ['c', 'd']}), + iD.osmRelation({id: 'r', members: [ + {id: '-', type: 'way'}, + {id: '~', type: 'way'}, + {id: '-', type: 'way'} + ]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + var ids = graph.entity('r').members.map(function(m) { return m.id; }); + expect(ids).to.have.ordered.members(['-', '=', '~', '=', '-']); + }); }); diff --git a/test/spec/osm/multipolygon.js b/test/spec/osm/multipolygon.js index aea1d8a68..ea0fd9c3d 100644 --- a/test/spec/osm/multipolygon.js +++ b/test/spec/osm/multipolygon.js @@ -210,4 +210,36 @@ describe('iD.osmJoinWays', function() { var graph = iD.coreGraph(); expect(iD.osmJoinWays([member], graph)).to.eql([]); }); + + it('understands doubled-back relation members', function() { + // e + // / \ + // a <=== b ---> c ~~~> d + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0, 0]}), + iD.osmNode({id: 'b', loc: [0, 0]}), + iD.osmNode({id: 'c', loc: [0, 0]}), + iD.osmNode({id: 'd', loc: [0, 0]}), + iD.osmNode({id: 'e', loc: [0, 0]}), + iD.osmWay({id: '=', nodes: ['b', 'a']}), + iD.osmWay({id: '-', nodes: ['b', 'c']}), + iD.osmWay({id: '~', nodes: ['c', 'd']}), + iD.osmWay({id: '\\', nodes: ['d', 'e']}), + iD.osmWay({id: '/', nodes: ['c', 'e']}), + iD.osmRelation({id: 'r', members: [ + {id: '=', type: 'way'}, + {id: '-', type: 'way'}, + {id: '~', type: 'way'}, + {id: '\\', type: 'way'}, + {id: '/', type: 'way'}, + {id: '-', type: 'way'}, + {id: '=', type: 'way'} + ]}) + ]); + + var result = iD.osmJoinWays(graph.entity('r').members, graph); + var ids = result[0].map(function (w) { return w.id; }); + expect(ids).to.have.ordered.members(['=', '-', '~', '\\', '/', '-', '=']); + }); + }); From 8f6cb207fcb624afe9a194a71ed6178c410da41a Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 11 Jan 2018 21:42:29 -0500 Subject: [PATCH 03/15] Much expanded tests for osmJoinWays --- test/spec/actions/add_member.js | 30 ++-- test/spec/osm/multipolygon.js | 289 +++++++++++++++++++++++++------- 2 files changed, 240 insertions(+), 79 deletions(-) diff --git a/test/spec/actions/add_member.js b/test/spec/actions/add_member.js index 0d220b59a..95e9d98a6 100644 --- a/test/spec/actions/add_member.js +++ b/test/spec/actions/add_member.js @@ -23,8 +23,8 @@ describe('iD.actionAddMember', function() { }); it('appends the member if the ways are not connecting', function() { - // Before: ---> - // After: ---> ... ===> + // Before: a ---> b + // After: a ---> b .. c ===> d var graph = iD.coreGraph([ iD.osmNode({id: 'a', loc: [0, 0]}), iD.osmNode({id: 'b', loc: [0, 0]}), @@ -42,8 +42,8 @@ describe('iD.actionAddMember', function() { }); it('appends the member if the way connects at end', function() { - // Before: ---> - // After: ---> ===> + // Before: a ---> b + // After: a ---> b ===> c var graph = iD.coreGraph([ iD.osmNode({id: 'a', loc: [0, 0]}), iD.osmNode({id: 'b', loc: [0, 0]}), @@ -60,8 +60,8 @@ describe('iD.actionAddMember', function() { }); it('inserts the member if the way connects at beginning', function() { - // Before: ---> ~~~> - // After: ===> ---> ~~~> + // Before: b ---> c ~~~> d + // After: a ===> b ---> c ~~~> d var graph = iD.coreGraph([ iD.osmNode({id: 'a', loc: [0, 0]}), iD.osmNode({id: 'b', loc: [0, 0]}), @@ -81,8 +81,8 @@ describe('iD.actionAddMember', function() { }); it('inserts the member if the way connects in middle', function() { - // Before: ---> ~~~> - // After: ---> ===> ~~~> + // Before: a ---> b .. c ~~~> d + // After: a ---> b ===> c ~~~> d var graph = iD.coreGraph([ iD.osmNode({id: 'a', loc: [0, 0]}), iD.osmNode({id: 'b', loc: [0, 0]}), @@ -102,8 +102,8 @@ describe('iD.actionAddMember', function() { }); it('inserts the member multiple times if the way exists multiple times (middle)', function() { - // Before: ---> ~~~> ---> - // After: ---> ===> ~~~> ===> ---> + // Before: a ---> b .. c ~~~> d <~~~ c .. b <--- a + // After: a ---> b ===> c ~~~> d <~~~ c <=== b <--- a var graph = iD.coreGraph([ iD.osmNode({id: 'a', loc: [0, 0]}), iD.osmNode({id: 'b', loc: [0, 0]}), @@ -115,17 +115,18 @@ describe('iD.actionAddMember', function() { iD.osmRelation({id: 'r', members: [ {id: '-', type: 'way'}, {id: '~', type: 'way'}, + {id: '~', type: 'way'}, {id: '-', type: 'way'} ]}) ]); graph = iD.actionAddMember('r', {id: '=', type: 'way'})(graph); - expect(members(graph)).to.eql(['-', '=', '~', '=', '-']); + expect(members(graph)).to.eql(['-', '=', '~', '~', '=', '-']); }); it('inserts the member multiple times if the way exists multiple times (beginning/end)', function() { - // Before: ===> ~~~> ===> - // After: ---> ===> ~~~> ===> ---> + // Before: b ===> c ~~~> d <~~~ c <=== b + // After: a ---> b ===> c ~~~> d <~~~ c <=== b <--- a var graph = iD.coreGraph([ iD.osmNode({id: 'a', loc: [0, 0]}), iD.osmNode({id: 'b', loc: [0, 0]}), @@ -137,12 +138,13 @@ describe('iD.actionAddMember', function() { iD.osmRelation({id: 'r', members: [ {id: '=', type: 'way'}, {id: '~', type: 'way'}, + {id: '~', type: 'way'}, {id: '=', type: 'way'} ]}) ]); graph = iD.actionAddMember('r', {id: '-', type: 'way'})(graph); - expect(members(graph)).to.eql(['-', '=', '~', '=', '-']); + expect(members(graph)).to.eql(['-', '=', '~', '~', '=', '-']); }); diff --git a/test/spec/osm/multipolygon.js b/test/spec/osm/multipolygon.js index ea0fd9c3d..b71249c74 100644 --- a/test/spec/osm/multipolygon.js +++ b/test/spec/osm/multipolygon.js @@ -145,56 +145,122 @@ describe('iD.osmSimpleMultipolygonOuterMember', function() { describe('iD.osmJoinWays', function() { + function getIDs(objects) { + return objects.map(function(node) { return node.id; }); + } + it('returns an array of members with nodes properties', function() { - var node = iD.osmNode({loc: [0, 0]}); - var way = iD.osmWay({nodes: [node.id]}); - var member = {id: way.id, type: 'way'}; + var node = iD.osmNode({id: 'a', loc: [0, 0]}); + var way = iD.osmWay({id: '-', nodes: ['a']}); + var member = {id: '-', type: 'way'}; var graph = iD.coreGraph([node, way]); + var result = iD.osmJoinWays([member], graph); expect(result.length).to.equal(1); - expect(result[0].nodes.length).to.equal(1); - expect(result[0].nodes[0]).to.equal(node); + expect(getIDs(result[0].nodes)).to.eql(['a']); expect(result[0].length).to.equal(1); - expect(result[0][0]).to.equal(member); + expect(result[0][0]).to.have.own.property('id', '-'); + expect(result[0][0]).to.have.own.property('type', 'way'); }); - it('returns the members in the correct order', function() { - // a <=== b ---> c ~~~> d - var graph = iD.coreGraph([ - iD.osmNode({id: 'a', loc: [0, 0]}), - iD.osmNode({id: 'b', loc: [0, 0]}), - iD.osmNode({id: 'c', loc: [0, 0]}), - iD.osmNode({id: 'd', loc: [0, 0]}), - iD.osmWay({id: '=', nodes: ['b', 'a']}), - iD.osmWay({id: '-', nodes: ['b', 'c']}), - iD.osmWay({id: '~', nodes: ['c', 'd']}), - iD.osmRelation({id: 'r', members: [ - {id: '-', type: 'way'}, - {id: '~', type: 'way'}, - {id: '=', type: 'way'} - ]}) - ]); + it('joins ways', function() { + // + // a ---> b ===> c + // + var a = iD.osmNode({id: 'a', loc: [0, 0]}); + var b = iD.osmNode({id: 'b', loc: [1, 0]}); + var c = iD.osmNode({id: 'c', loc: [2, 0]}); + var w1 = iD.osmWay({id: '-', nodes: ['a', 'b']}); + var w2 = iD.osmWay({id: '=', nodes: ['b', 'c']}); + var graph = iD.coreGraph([a, b, c, w1, w2]); - var result = iD.osmJoinWays(graph.entity('r').members, graph); - var ids = result[0].map(function (w) { return w.id; }); - expect(ids).to.have.ordered.members(['=', '-', '~']); + var result = iD.osmJoinWays([w1, w2], graph); + expect(result.length).to.equal(1); + expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); + expect(result[0].length).to.equal(2); + expect(result[0][0]).to.eql(w1); + expect(result[0][1]).to.eql(w2); + }); + + it('joins relation members', function() { + // + // a ---> b ===> c + // r: ['-', '='] + // + var a = iD.osmNode({id: 'a', loc: [0, 0]}); + var b = iD.osmNode({id: 'b', loc: [1, 0]}); + var c = iD.osmNode({id: 'c', loc: [2, 0]}); + var w1 = iD.osmWay({id: '-', nodes: ['a', 'b']}); + var w2 = iD.osmWay({id: '=', nodes: ['b', 'c']}); + var r = iD.osmRelation({id: 'r', members: [ + {id: '-', type: 'way'}, + {id: '=', type: 'way'} + ]}); + var graph = iD.coreGraph([a, b, c, w1, w2, r]); + + var result = iD.osmJoinWays(r.members, graph); + expect(result.length).to.equal(1); + expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); + expect(result[0].length).to.equal(2); + expect(result[0][0]).to.have.own.property('id', '-'); + expect(result[0][0]).to.have.own.property('type', 'way'); + expect(result[0][1]).to.have.own.property('id', '='); + expect(result[0][1]).to.have.own.property('type', 'way'); + }); + + it('returns joined members in the correct order', function() { + // + // a <=== b ---> c ~~~> d + // r: ['-', '~', '='] + // + var a = iD.osmNode({id: 'a', loc: [0, 0]}); + var b = iD.osmNode({id: 'b', loc: [1, 0]}); + var c = iD.osmNode({id: 'c', loc: [2, 0]}); + var d = iD.osmNode({id: 'd', loc: [3, 0]}); + var w1 = iD.osmWay({id: '-', nodes: ['b', 'c']}); + var w2 = iD.osmWay({id: '=', nodes: ['b', 'a']}); + var w3 = iD.osmWay({id: '~', nodes: ['c', 'd']}); + var r = iD.osmRelation({id: 'r', members: [ + {id: '-', type: 'way'}, + {id: '~', type: 'way'}, + {id: '=', type: 'way'} + ]}); + var graph = iD.coreGraph([a, b, c, d, w1, w2, w3, r]); + + var result = iD.osmJoinWays(r.members, graph); + expect(result.length).to.equal(1); + expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c', 'd']); + expect(result[0].length).to.equal(3); + expect(result[0][0]).to.have.own.property('id', '='); + expect(result[0][0]).to.have.own.property('type', 'way'); + expect(result[0][1]).to.have.own.property('id', '-'); + expect(result[0][1]).to.have.own.property('type', 'way'); + expect(result[0][2]).to.have.own.property('id', '~'); + expect(result[0][2]).to.have.own.property('type', 'way'); }); it('reverses member tags of reversed segements', function() { - // a --> b <== c - // Expected result: - // a --> b --> c - // tags on === reversed - 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: {'oneway': 'yes', 'lanes:forward': 2}}) - ]); + // + // Source: + // a ---> b <=== c + // Result: + // a ---> b ===> c (and b === c reversed) + // + var a = iD.osmNode({id: 'a', loc: [0, 0]}); + var b = iD.osmNode({id: 'b', loc: [1, 0]}); + var c = iD.osmNode({id: 'c', loc: [2, 0]}); + var w1 = iD.osmWay({id: '-', nodes: ['a', 'b']}); + var w2 = iD.osmWay({id: '=', nodes: ['c', 'b'], tags: {'oneway': 'yes', 'lanes:forward': 2}}); + var graph = iD.coreGraph([a, b, c, w1, w2]); - var result = iD.osmJoinWays([graph.entity('-'), graph.entity('=')], graph); + var result = iD.osmJoinWays([w1, w2], graph); + expect(result.length).to.equal(1); + expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); + expect(result[0].length).to.equal(2); + expect(result[0][0]).to.eql(w1); + expect(result[0][1]).to.be.an.instanceof(iD.osmWay); + expect(result[0][1].nodes).to.eql(['b', 'c']); expect(result[0][1].tags).to.eql({'oneway': '-1', 'lanes:backward': 2}); }); @@ -211,35 +277,128 @@ describe('iD.osmJoinWays', function() { expect(iD.osmJoinWays([member], graph)).to.eql([]); }); - it('understands doubled-back relation members', function() { - // e - // / \ - // a <=== b ---> c ~~~> d - var graph = iD.coreGraph([ - iD.osmNode({id: 'a', loc: [0, 0]}), - iD.osmNode({id: 'b', loc: [0, 0]}), - iD.osmNode({id: 'c', loc: [0, 0]}), - iD.osmNode({id: 'd', loc: [0, 0]}), - iD.osmNode({id: 'e', loc: [0, 0]}), - iD.osmWay({id: '=', nodes: ['b', 'a']}), - iD.osmWay({id: '-', nodes: ['b', 'c']}), - iD.osmWay({id: '~', nodes: ['c', 'd']}), - iD.osmWay({id: '\\', nodes: ['d', 'e']}), - iD.osmWay({id: '/', nodes: ['c', 'e']}), - iD.osmRelation({id: 'r', members: [ - {id: '=', type: 'way'}, - {id: '-', type: 'way'}, - {id: '~', type: 'way'}, - {id: '\\', type: 'way'}, - {id: '/', type: 'way'}, - {id: '-', type: 'way'}, - {id: '=', type: 'way'} - ]}) - ]); + it('returns multiple arrays for disjoint ways', function() { + // + // b + // / \ d ---> e ===> f + // a c + // + var a = iD.osmNode({id: 'a', loc: [0, -1]}); + var b = iD.osmNode({id: 'b', loc: [1, 1]}); + var c = iD.osmNode({id: 'c', loc: [2, -1]}); + var d = iD.osmNode({id: 'd', loc: [5, 0]}); + var e = iD.osmNode({id: 'e', loc: [6, 0]}); + var f = iD.osmNode({id: 'f', loc: [7, 0]}); + var w1 = iD.osmWay({id: '/', nodes: ['a', 'b']}); + var w2 = iD.osmWay({id: '\\', nodes: ['b', 'c']}); + var w3 = iD.osmWay({id: '-', nodes: ['d', 'e']}); + var w4 = iD.osmWay({id: '=', nodes: ['e', 'f']}); + var graph = iD.coreGraph([a, b, c, d, e, f, w1, w2, w3, w4]); - var result = iD.osmJoinWays(graph.entity('r').members, graph); - var ids = result[0].map(function (w) { return w.id; }); - expect(ids).to.have.ordered.members(['=', '-', '~', '\\', '/', '-', '=']); + var result = iD.osmJoinWays([w1, w2, w3, w4], graph); + + expect(result.length).to.equal(2); + expect(result[0].length).to.equal(2); + expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); + expect(result[0][0]).to.eql(w1); + expect(result[0][1]).to.eql(w2); + + expect(result[1].length).to.equal(2); + expect(getIDs(result[1].nodes)).to.eql(['d', 'e', 'f']); + expect(result[1][0]).to.eql(w3); + expect(result[1][1]).to.eql(w4); + }); + + it('returns multiple arrays for disjoint relations', function() { + // + // b + // / \ + // a c d ---> e ===> f + // + // r: ['/', '\', '-', '='] + // + var a = iD.osmNode({id: 'a', loc: [0, 0]}); + var b = iD.osmNode({id: 'b', loc: [1, 1]}); + var c = iD.osmNode({id: 'c', loc: [2, 0]}); + var d = iD.osmNode({id: 'd', loc: [5, 0]}); + var e = iD.osmNode({id: 'e', loc: [6, 0]}); + var f = iD.osmNode({id: 'f', loc: [7, 0]}); + var w1 = iD.osmWay({id: '/', nodes: ['a', 'b']}); + var w2 = iD.osmWay({id: '\\', nodes: ['b', 'c']}); + var w3 = iD.osmWay({id: '-', nodes: ['d', 'e']}); + var w4 = iD.osmWay({id: '=', nodes: ['e', 'f']}); + var r = iD.osmRelation({id: 'r', members: [ + {id: '/', type: 'way'}, + {id: '\\', type: 'way'}, + {id: '-', type: 'way'}, + {id: '=', type: 'way'} + ]}); + var graph = iD.coreGraph([a, b, c, d, e, f, w1, w2, w3, w4, r]); + var result = iD.osmJoinWays(r.members, graph); + + expect(result.length).to.equal(2); + expect(result[0].length).to.equal(2); + expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); + expect(result[0][0]).to.have.own.property('id', '/'); + expect(result[0][0]).to.have.own.property('type', 'way'); + expect(result[0][1]).to.have.own.property('id', '\\'); + expect(result[0][1]).to.have.own.property('type', 'way'); + + expect(result[1].length).to.equal(2); + expect(getIDs(result[1].nodes)).to.eql(['d', 'e', 'f']); + expect(result[1][0]).to.have.own.property('id', '-'); + expect(result[1][0]).to.have.own.property('type', 'way'); + expect(result[1][1]).to.have.own.property('id', '='); + expect(result[1][1]).to.have.own.property('type', 'way'); + }); + + it('understands doubled-back relation members', function() { + // + // e + // / \ + // a <=== b ---> c ~~~> d + // + // r: ['=', '-', '~', '\', '/', '-', '='] + // + var a = iD.osmNode({id: 'a', loc: [0, 0]}); + var b = iD.osmNode({id: 'b', loc: [1, 0]}); + var c = iD.osmNode({id: 'c', loc: [2, 0]}); + var d = iD.osmNode({id: 'd', loc: [4, 0]}); + var e = iD.osmNode({id: 'e', loc: [3, 1]}); + var w1 = iD.osmWay({id: '=', nodes: ['b', 'a']}); + var w2 = iD.osmWay({id: '-', nodes: ['b', 'c']}); + var w3 = iD.osmWay({id: '~', nodes: ['c', 'd']}); + var w4 = iD.osmWay({id: '\\', nodes: ['d', 'e']}); + var w5 = iD.osmWay({id: '/', nodes: ['c', 'e']}); + var r = iD.osmRelation({id: 'r', members: [ + {id: '=', type: 'way'}, + {id: '-', type: 'way'}, + {id: '~', type: 'way'}, + {id: '\\', type: 'way'}, + {id: '/', type: 'way'}, + {id: '-', type: 'way'}, + {id: '=', type: 'way'} + ]}); + var graph = iD.coreGraph([a, b, c, d, e, w1, w2, w3, w4, w5, r]); + + var result = iD.osmJoinWays(r.members, graph); + expect(result.length).to.equal(1); + expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c', 'd', 'e', 'c', 'b', 'a']); + expect(result[0].length).to.equal(7); + expect(result[0][0]).to.have.own.property('id', '='); + expect(result[0][0]).to.have.own.property('type', 'way'); + expect(result[0][1]).to.have.own.property('id', '-'); + expect(result[0][1]).to.have.own.property('type', 'way'); + expect(result[0][2]).to.have.own.property('id', '~'); + expect(result[0][2]).to.have.own.property('type', 'way'); + expect(result[0][3]).to.have.own.property('id', '\\'); + expect(result[0][3]).to.have.own.property('type', 'way'); + expect(result[0][4]).to.have.own.property('id', '/'); + expect(result[0][4]).to.have.own.property('type', 'way'); + expect(result[0][5]).to.have.own.property('id', '-'); + expect(result[0][5]).to.have.own.property('type', 'way'); + expect(result[0][6]).to.have.own.property('id', '='); + expect(result[0][6]).to.have.own.property('type', 'way'); }); }); From 0fd801d750733e01764dab2fe477e69323451213 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 12 Jan 2018 17:23:56 -0500 Subject: [PATCH 04/15] Prefer to join member ways in a way that preserves their order (re #4589) Strongly prefer to generate a forward path that preserves the order of the members array. For multipolygons and most relations, member order does not matter - but for routes, it does. If we started this sequence backwards (i.e. next member way attaches to the start node and not the end node), reverse the initial way before continuing. --- modules/osm/multipolygon.js | 161 ++++++++++++++++++++++++++-------- test/spec/osm/multipolygon.js | 88 +++++++++++-------- 2 files changed, 171 insertions(+), 78 deletions(-) diff --git a/modules/osm/multipolygon.js b/modules/osm/multipolygon.js index 05f37bf95..92ed38a22 100644 --- a/modules/osm/multipolygon.js +++ b/modules/osm/multipolygon.js @@ -82,13 +82,7 @@ export function osmSimpleMultipolygonOuterMember(entity, graph) { // Incomplete members (those for which `graph.hasEntity(element.id)` returns // false) and non-way members are ignored. // -export function osmJoinWays(array, graph) { - var joined = [], member, current, nodes, first, last, i, how, what; - - array = array.filter(function(member) { - return member.type === 'way' && graph.hasEntity(member.id); - }); - +export function osmJoinWays(toJoin, graph) { function resolve(member) { return graph.childNodes(graph.entity(member.id)); } @@ -97,52 +91,141 @@ export function osmJoinWays(array, graph) { return member.tags ? actionReverse(member.id, { reverseOneway: true })(graph).entity(member.id) : member; } - while (array.length) { - member = array.shift(); - current = [member]; - current.nodes = nodes = resolve(member).slice(); - joined.push(current); - while (array.length && nodes[0] !== nodes[nodes.length - 1]) { - first = nodes[0]; - last = nodes[nodes.length - 1]; + // make a copy containing only the ways to join + toJoin = toJoin.filter(function(member) { + return member.type === 'way' && graph.hasEntity(member.id); + }); - for (i = 0; i < array.length; i++) { - member = array[i]; - what = resolve(member); + var sequences = []; - if (last === what[0]) { - how = nodes.push; - what = what.slice(1); + while (toJoin.length) { + // start a new sequence + var way = toJoin.shift(); + var currWays = [way]; + var currNodes = resolve(way).slice(); + var doneSequence = false; + + // add to it + while (toJoin.length && !doneSequence) { + var start = currNodes[0]; + var end = currNodes[currNodes.length - 1]; + var fn = null; + var nodes = null; + var i; + + // find the next way + for (i = 0; i < toJoin.length; i++) { + way = toJoin[i]; + nodes = resolve(way); + + // Strongly prefer to generate a forward path that preserves the order + // of the members array. For multipolygons and most relations, member + // order does not matter - but for routes, it does. If we started this + // sequence backwards (i.e. next member way attaches to the start node + // and not the end node), reverse the initial way before continuing. + if (currWays.length === 1 && nodes[0] !== end && nodes[nodes.length - 1] !== end && + (nodes[nodes.length - 1] === start || nodes[0] === start) + ) { + currWays[0] = reverse(currWays[0]); + currNodes.reverse(); + start = currNodes[0]; + end = currNodes[currNodes.length - 1]; + } + + if (nodes[0] === end) { + fn = currNodes.push; // join to end + nodes = nodes.slice(1); break; - } else if (last === what[what.length - 1]) { - how = nodes.push; - what = what.slice(0, -1).reverse(); - member = reverse(member); + } else if (nodes[nodes.length - 1] === end) { + fn = currNodes.push; // join to end + nodes = nodes.slice(0, -1).reverse(); + way = reverse(way); break; - } else if (first === what[what.length - 1]) { - how = nodes.unshift; - what = what.slice(0, -1); + } else if (nodes[nodes.length - 1] === start) { + fn = currNodes.unshift; // join to beginning + nodes = nodes.slice(0, -1); break; - } else if (first === what[0]) { - how = nodes.unshift; - what = what.slice(1).reverse(); - member = reverse(member); + } else if (nodes[0] === start) { + fn = currNodes.unshift; // join to beginning + nodes = nodes.slice(1).reverse(); + way = reverse(way); break; } else { - what = how = null; + fn = nodes = null; } } - if (!what) - break; // No more joinable ways. + if (!nodes) { + doneSequence = true; // couldn't find a joinable way + break; + } - how.apply(current, [member]); - how.apply(nodes, what); + fn.apply(currWays, [way]); + fn.apply(currNodes, nodes); - array.splice(i, 1); + toJoin.splice(i, 1); } + + + currWays.nodes = currNodes; + sequences.push(currWays); } - return joined; + return sequences; + + + // var joined = []; + + // while (array.length) { + // var member = array.shift(); + // var current = [member]; + // var nodes = resolve(member).slice(); + + // current.nodes = nodes; + // joined.push(current); + + // while (array.length && nodes[0] !== nodes[nodes.length - 1]) { + // var first = nodes[0]; + // var last = nodes[nodes.length - 1]; + // var how, what, i; + + // for (i = 0; i < array.length; i++) { + // member = array[i]; + // what = resolve(member); + + // if (last === what[0]) { + // how = nodes.push; + // what = what.slice(1); + // break; + // } else if (last === what[what.length - 1]) { + // how = nodes.push; + // what = what.slice(0, -1).reverse(); + // member = reverse(member); + // break; + // } else if (first === what[what.length - 1]) { + // how = nodes.unshift; + // what = what.slice(0, -1); + // break; + // } else if (first === what[0]) { + // how = nodes.unshift; + // what = what.slice(1).reverse(); + // member = reverse(member); + // break; + // } else { + // what = how = null; + // } + // } + + // if (!what) + // break; // No more joinable ways. + + // how.apply(current, [member]); + // how.apply(nodes, what); + + // array.splice(i, 1); + // } + // } + + // return joined; } diff --git a/test/spec/osm/multipolygon.js b/test/spec/osm/multipolygon.js index b71249c74..6f5211ab7 100644 --- a/test/spec/osm/multipolygon.js +++ b/test/spec/osm/multipolygon.js @@ -160,8 +160,7 @@ describe('iD.osmJoinWays', function() { expect(result.length).to.equal(1); expect(getIDs(result[0].nodes)).to.eql(['a']); expect(result[0].length).to.equal(1); - expect(result[0][0]).to.have.own.property('id', '-'); - expect(result[0][0]).to.have.own.property('type', 'way'); + expect(result[0][0]).to.eql(member); }); it('joins ways', function() { @@ -203,10 +202,8 @@ describe('iD.osmJoinWays', function() { expect(result.length).to.equal(1); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); expect(result[0].length).to.equal(2); - expect(result[0][0]).to.have.own.property('id', '-'); - expect(result[0][0]).to.have.own.property('type', 'way'); - expect(result[0][1]).to.have.own.property('id', '='); - expect(result[0][1]).to.have.own.property('type', 'way'); + expect(result[0][0]).to.eql({id: '-', type: 'way'}); + expect(result[0][1]).to.eql({id: '=', type: 'way'}); }); it('returns joined members in the correct order', function() { @@ -232,12 +229,9 @@ describe('iD.osmJoinWays', function() { expect(result.length).to.equal(1); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c', 'd']); expect(result[0].length).to.equal(3); - expect(result[0][0]).to.have.own.property('id', '='); - expect(result[0][0]).to.have.own.property('type', 'way'); - expect(result[0][1]).to.have.own.property('id', '-'); - expect(result[0][1]).to.have.own.property('type', 'way'); - expect(result[0][2]).to.have.own.property('id', '~'); - expect(result[0][2]).to.have.own.property('type', 'way'); + expect(result[0][0]).to.eql({id: '=', type: 'way'}); + expect(result[0][1]).to.eql({id: '-', type: 'way'}); + expect(result[0][2]).to.eql({id: '~', type: 'way'}); }); it('reverses member tags of reversed segements', function() { @@ -245,7 +239,7 @@ describe('iD.osmJoinWays', function() { // Source: // a ---> b <=== c // Result: - // a ---> b ===> c (and b === c reversed) + // a ---> b ===> c (and === reversed) // var a = iD.osmNode({id: 'a', loc: [0, 0]}); var b = iD.osmNode({id: 'b', loc: [1, 0]}); @@ -264,6 +258,31 @@ describe('iD.osmJoinWays', function() { expect(result[0][1].tags).to.eql({'oneway': '-1', 'lanes:backward': 2}); }); + it('reverses the initial segment to preserve member order', function() { + // + // Source: + // a <--- b ===> c + // Result: + // a ---> b ===> c (and --- reversed) + // + var a = iD.osmNode({id: 'a', loc: [0, 0]}); + var b = iD.osmNode({id: 'b', loc: [1, 0]}); + var c = iD.osmNode({id: 'c', loc: [2, 0]}); + var w1 = iD.osmWay({id: '-', nodes: ['b', 'a'], tags: {'oneway': 'yes', 'lanes:forward': 2}}); + var w2 = iD.osmWay({id: '=', nodes: ['b', 'c']}); + var graph = iD.coreGraph([a, b, c, w1, w2]); + + var result = iD.osmJoinWays([w1, w2], graph); + expect(result.length).to.equal(1); + expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); + expect(result[0].length).to.equal(2); + expect(result[0][0]).to.be.an.instanceof(iD.osmWay); + expect(result[0][0].nodes).to.eql(['a', 'b']); + expect(result[0][0].tags).to.eql({'oneway': '-1', 'lanes:backward': 2}); + expect(result[0][1]).to.eql(w2); + }); + + it('ignores non-way members', function() { var node = iD.osmNode({loc: [0, 0]}); var member = {id: 'n', type: 'node'}; @@ -280,12 +299,12 @@ describe('iD.osmJoinWays', function() { it('returns multiple arrays for disjoint ways', function() { // // b - // / \ d ---> e ===> f - // a c + // / \ + // a c d ---> e ===> f // - var a = iD.osmNode({id: 'a', loc: [0, -1]}); + var a = iD.osmNode({id: 'a', loc: [0, 0]}); var b = iD.osmNode({id: 'b', loc: [1, 1]}); - var c = iD.osmNode({id: 'c', loc: [2, -1]}); + var c = iD.osmNode({id: 'c', loc: [2, 0]}); var d = iD.osmNode({id: 'd', loc: [5, 0]}); var e = iD.osmNode({id: 'e', loc: [6, 0]}); var f = iD.osmNode({id: 'f', loc: [7, 0]}); @@ -298,6 +317,7 @@ describe('iD.osmJoinWays', function() { var result = iD.osmJoinWays([w1, w2, w3, w4], graph); expect(result.length).to.equal(2); + expect(result[0].length).to.equal(2); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); expect(result[0][0]).to.eql(w1); @@ -337,19 +357,16 @@ describe('iD.osmJoinWays', function() { var result = iD.osmJoinWays(r.members, graph); expect(result.length).to.equal(2); + expect(result[0].length).to.equal(2); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); - expect(result[0][0]).to.have.own.property('id', '/'); - expect(result[0][0]).to.have.own.property('type', 'way'); - expect(result[0][1]).to.have.own.property('id', '\\'); - expect(result[0][1]).to.have.own.property('type', 'way'); + expect(result[0][0]).to.eql({id: '/', type: 'way'}); + expect(result[0][1]).to.eql({id: '\\', type: 'way'}); expect(result[1].length).to.equal(2); expect(getIDs(result[1].nodes)).to.eql(['d', 'e', 'f']); - expect(result[1][0]).to.have.own.property('id', '-'); - expect(result[1][0]).to.have.own.property('type', 'way'); - expect(result[1][1]).to.have.own.property('id', '='); - expect(result[1][1]).to.have.own.property('type', 'way'); + expect(result[1][0]).to.eql({id: '-', type: 'way'}); + expect(result[1][1]).to.eql({id: '=', type: 'way'}); }); it('understands doubled-back relation members', function() { @@ -385,20 +402,13 @@ describe('iD.osmJoinWays', function() { expect(result.length).to.equal(1); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c', 'd', 'e', 'c', 'b', 'a']); expect(result[0].length).to.equal(7); - expect(result[0][0]).to.have.own.property('id', '='); - expect(result[0][0]).to.have.own.property('type', 'way'); - expect(result[0][1]).to.have.own.property('id', '-'); - expect(result[0][1]).to.have.own.property('type', 'way'); - expect(result[0][2]).to.have.own.property('id', '~'); - expect(result[0][2]).to.have.own.property('type', 'way'); - expect(result[0][3]).to.have.own.property('id', '\\'); - expect(result[0][3]).to.have.own.property('type', 'way'); - expect(result[0][4]).to.have.own.property('id', '/'); - expect(result[0][4]).to.have.own.property('type', 'way'); - expect(result[0][5]).to.have.own.property('id', '-'); - expect(result[0][5]).to.have.own.property('type', 'way'); - expect(result[0][6]).to.have.own.property('id', '='); - expect(result[0][6]).to.have.own.property('type', 'way'); + expect(result[0][0]).to.eql({id: '=', type: 'way'}); + expect(result[0][1]).to.eql({id: '-', type: 'way'}); + expect(result[0][2]).to.eql({id: '~', type: 'way'}); + expect(result[0][3]).to.eql({id: '\\', type: 'way'}); + expect(result[0][4]).to.eql({id: '/', type: 'way'}); + expect(result[0][5]).to.eql({id: '-', type: 'way'}); + expect(result[0][6]).to.eql({id: '=', type: 'way'}); }); }); From 8dbb6eb20c022a185182e31d85b34dd3d186eb14 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 13 Jan 2018 01:45:46 -0500 Subject: [PATCH 05/15] Return reversal actions performed by osmJoinWays (see #4688) --- modules/osm/multipolygon.js | 63 ++++------------------------------- test/spec/osm/multipolygon.js | 12 ++++++- 2 files changed, 17 insertions(+), 58 deletions(-) diff --git a/modules/osm/multipolygon.js b/modules/osm/multipolygon.js index 92ed38a22..2292f65f3 100644 --- a/modules/osm/multipolygon.js +++ b/modules/osm/multipolygon.js @@ -1,5 +1,6 @@ import { actionReverse } from '../actions/reverse'; import { osmIsInterestingTag } from './tags'; +import { osmWay } from './way'; // For fixing up rendering of multipolygons with tags on the outer member. @@ -87,8 +88,10 @@ export function osmJoinWays(toJoin, graph) { return graph.childNodes(graph.entity(member.id)); } - function reverse(member) { - return member.tags ? actionReverse(member.id, { reverseOneway: true })(graph).entity(member.id) : member; + function reverse(which) { + var action = actionReverse(which.id, { reverseOneway: true }); + sequences.actions.push(action); + return (which instanceof osmWay) ? action(graph).entity(which.id) : which; } @@ -98,6 +101,7 @@ export function osmJoinWays(toJoin, graph) { }); var sequences = []; + sequences.actions = []; while (toJoin.length) { // start a new sequence @@ -173,59 +177,4 @@ export function osmJoinWays(toJoin, graph) { } return sequences; - - - // var joined = []; - - // while (array.length) { - // var member = array.shift(); - // var current = [member]; - // var nodes = resolve(member).slice(); - - // current.nodes = nodes; - // joined.push(current); - - // while (array.length && nodes[0] !== nodes[nodes.length - 1]) { - // var first = nodes[0]; - // var last = nodes[nodes.length - 1]; - // var how, what, i; - - // for (i = 0; i < array.length; i++) { - // member = array[i]; - // what = resolve(member); - - // if (last === what[0]) { - // how = nodes.push; - // what = what.slice(1); - // break; - // } else if (last === what[what.length - 1]) { - // how = nodes.push; - // what = what.slice(0, -1).reverse(); - // member = reverse(member); - // break; - // } else if (first === what[what.length - 1]) { - // how = nodes.unshift; - // what = what.slice(0, -1); - // break; - // } else if (first === what[0]) { - // how = nodes.unshift; - // what = what.slice(1).reverse(); - // member = reverse(member); - // break; - // } else { - // what = how = null; - // } - // } - - // if (!what) - // break; // No more joinable ways. - - // how.apply(current, [member]); - // how.apply(nodes, what); - - // array.splice(i, 1); - // } - // } - - // return joined; } diff --git a/test/spec/osm/multipolygon.js b/test/spec/osm/multipolygon.js index 6f5211ab7..1cb467d72 100644 --- a/test/spec/osm/multipolygon.js +++ b/test/spec/osm/multipolygon.js @@ -158,6 +158,7 @@ describe('iD.osmJoinWays', function() { var result = iD.osmJoinWays([member], graph); expect(result.length).to.equal(1); + expect(result.actions).to.eql([]); expect(getIDs(result[0].nodes)).to.eql(['a']); expect(result[0].length).to.equal(1); expect(result[0][0]).to.eql(member); @@ -176,6 +177,7 @@ describe('iD.osmJoinWays', function() { var result = iD.osmJoinWays([w1, w2], graph); expect(result.length).to.equal(1); + expect(result.actions).to.eql([]); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); expect(result[0].length).to.equal(2); expect(result[0][0]).to.eql(w1); @@ -200,6 +202,7 @@ describe('iD.osmJoinWays', function() { var result = iD.osmJoinWays(r.members, graph); expect(result.length).to.equal(1); + expect(result.actions).to.eql([]); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); expect(result[0].length).to.equal(2); expect(result[0][0]).to.eql({id: '-', type: 'way'}); @@ -227,6 +230,7 @@ describe('iD.osmJoinWays', function() { var result = iD.osmJoinWays(r.members, graph); expect(result.length).to.equal(1); + expect(result.actions.length).to.equal(1); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c', 'd']); expect(result[0].length).to.equal(3); expect(result[0][0]).to.eql({id: '=', type: 'way'}); @@ -250,6 +254,7 @@ describe('iD.osmJoinWays', function() { var result = iD.osmJoinWays([w1, w2], graph); expect(result.length).to.equal(1); + expect(result.actions.length).to.equal(1); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); expect(result[0].length).to.equal(2); expect(result[0][0]).to.eql(w1); @@ -274,6 +279,7 @@ describe('iD.osmJoinWays', function() { var result = iD.osmJoinWays([w1, w2], graph); expect(result.length).to.equal(1); + expect(result.actions.length).to.equal(1); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); expect(result[0].length).to.equal(2); expect(result[0][0]).to.be.an.instanceof(iD.osmWay); @@ -317,6 +323,7 @@ describe('iD.osmJoinWays', function() { var result = iD.osmJoinWays([w1, w2, w3, w4], graph); expect(result.length).to.equal(2); + expect(result.actions).to.eql([]); expect(result[0].length).to.equal(2); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); @@ -357,6 +364,7 @@ describe('iD.osmJoinWays', function() { var result = iD.osmJoinWays(r.members, graph); expect(result.length).to.equal(2); + expect(result.actions).to.eql([]); expect(result[0].length).to.equal(2); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c']); @@ -399,7 +407,9 @@ describe('iD.osmJoinWays', function() { var graph = iD.coreGraph([a, b, c, d, e, w1, w2, w3, w4, w5, r]); var result = iD.osmJoinWays(r.members, graph); - expect(result.length).to.equal(1); + expect(result.length).to.equal(3); + expect(result.actions.length).to.equal(1); + expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c', 'd', 'e', 'c', 'b', 'a']); expect(result[0].length).to.equal(7); expect(result[0][0]).to.eql({id: '=', type: 'way'}); From 075b85c81d50fceb4fa247d173e991678586c833 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sun, 14 Jan 2018 14:49:57 -0500 Subject: [PATCH 06/15] Apply reversal actions in `actionJoin` (closes #4688) --- modules/actions/join.js | 34 +++++++++++++++++----------------- test/spec/actions/join.js | 12 ++++++------ test/spec/osm/multipolygon.js | 4 ++-- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/modules/actions/join.js b/modules/actions/join.js index 5749ffbba..421cda3ce 100644 --- a/modules/actions/join.js +++ b/modules/actions/join.js @@ -1,13 +1,8 @@ import _extend from 'lodash-es/extend'; import _groupBy from 'lodash-es/groupBy'; -import _map from 'lodash-es/map'; import { actionDeleteWay } from './delete_way'; - -import { - osmIsInterestingTag, - osmJoinWays -} from '../osm'; +import { osmIsInterestingTag, osmJoinWays } from '../osm'; // Join ways at the end node they share. @@ -27,25 +22,30 @@ export function actionJoin(ids) { var action = function(graph) { - var ways = ids.map(graph.entity, graph), - survivor = ways[0]; + var ways = ids.map(graph.entity, graph); + var survivorID = ways[0].id; // Prefer to keep an existing way. for (var i = 0; i < ways.length; i++) { if (!ways[i].isNew()) { - survivor = ways[i]; + survivorID = ways[i].id; break; } } - var joined = osmJoinWays(ways, graph)[0]; + var sequences = osmJoinWays(ways, graph); + var joined = sequences[0]; - survivor = survivor.update({nodes: _map(joined.nodes, 'id')}); + // We might need to reverse some of these ways before joining them. #4688 + // `joined.actions` property will contain any actions we need to apply. + graph = sequences.actions.reduce(function(g, action) { return action(g); }, graph); + + var survivor = graph.entity(survivorID); + survivor = survivor.update({ nodes: joined.nodes.map(function(n) { return n.id; }) }); graph = graph.replace(survivor); joined.forEach(function(way) { - if (way.id === survivor.id) - return; + if (way.id === survivorID) return; graph.parentRelations(way).forEach(function(parent) { graph = graph.replace(parent.replaceMember(way, survivor)); @@ -70,10 +70,10 @@ export function actionJoin(ids) { if (joined.length > 1) return 'not_adjacent'; - var nodeIds = _map(joined[0].nodes, 'id').slice(1, -1), - relation, - tags = {}, - conflicting = false; + var nodeIds = joined[0].nodes.map(function(n) { return n.id; }).slice(1, -1); + var relation; + var tags = {}; + var conflicting = false; joined[0].forEach(function(way) { var parents = graph.parentRelations(way); diff --git a/test/spec/actions/join.js b/test/spec/actions/join.js index d591c4d55..8b1378187 100644 --- a/test/spec/actions/join.js +++ b/test/spec/actions/join.js @@ -274,25 +274,25 @@ describe('iD.actionJoin', function () { graph = iD.actionJoin(['-', '='])(graph); - expect(graph.entity('-').nodes).to.eql(['c', 'b', 'a']); + expect(graph.entity('-').nodes).to.eql(['a', 'b', 'c']); expect(graph.hasEntity('=')).to.be.undefined; }); it('joins a <-- b ==> c', function () { // Expected result: - // a <-- b <-- c - // tags on === reversed + // 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']}), - iD.Way({id: '=', nodes: ['b', 'c'], tags: {'lanes:forward': 2}}) + iD.Way({id: '-', nodes: ['b', 'a'], tags: {'lanes:forward': 2}}), + iD.Way({id: '=', nodes: ['b', 'c']}) ]); graph = iD.actionJoin(['-', '='])(graph); - expect(graph.entity('-').nodes).to.eql(['c', 'b', 'a']); + expect(graph.entity('-').nodes).to.eql(['a', 'b', 'c']); expect(graph.hasEntity('=')).to.be.undefined; expect(graph.entity('-').tags).to.eql({'lanes:backward': 2}); }); diff --git a/test/spec/osm/multipolygon.js b/test/spec/osm/multipolygon.js index 1cb467d72..f3bba303b 100644 --- a/test/spec/osm/multipolygon.js +++ b/test/spec/osm/multipolygon.js @@ -407,8 +407,8 @@ describe('iD.osmJoinWays', function() { var graph = iD.coreGraph([a, b, c, d, e, w1, w2, w3, w4, w5, r]); var result = iD.osmJoinWays(r.members, graph); - expect(result.length).to.equal(3); - expect(result.actions.length).to.equal(1); + expect(result.length).to.equal(1); + expect(result.actions.length).to.equal(3); expect(getIDs(result[0].nodes)).to.eql(['a', 'b', 'c', 'd', 'e', 'c', 'b', 'a']); expect(result[0].length).to.equal(7); From 0382ce7d4b61897ae6e9b848d8e8902a7f706db1 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sun, 14 Jan 2018 21:45:25 -0500 Subject: [PATCH 07/15] Updated function doc --- modules/osm/multipolygon.js | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/modules/osm/multipolygon.js b/modules/osm/multipolygon.js index 2292f65f3..322626df8 100644 --- a/modules/osm/multipolygon.js +++ b/modules/osm/multipolygon.js @@ -63,21 +63,26 @@ export function osmSimpleMultipolygonOuterMember(entity, graph) { } -// Join `array` into sequences of connecting ways. -// +// Join `toJoin` array into sequences of connecting ways. + // Segments which share identical start/end nodes will, as much as possible, // be connected with each other. // // The return value is a nested array. Each constituent array contains elements -// of `array` which have been determined to connect. Each consitituent array -// also has a `nodes` property whose value is an ordered array of member nodes, -// with appropriate order reversal and start/end coordinate de-duplication. +// of `toJoin` which have been determined to connect. // -// Members of `array` must have, at minimum, `type` and `id` properties. -// Thus either an array of `osmWay`s or a relation member array may be -// used. +// Each consitituent array also has a `nodes` property whose value is an +// ordered array of member nodes, with appropriate order reversal and +// start/end coordinate de-duplication. // -// If an member has a `tags` property, its tags will be reversed via +// The returned sequences array also has an `actions` array property, containing +// any reversal actions that should be applied to the graph, should the calling +// code attempt to actually join the given ways. +// +// Members of `toJoin` must have, at minimum, `type` and `id` properties. +// Thus either an array of `osmWay`s or a relation member array may be used. +// +// If an member is an `osmWay`, its tags and childnodes will be reversed via // `actionReverse` in the output. // // Incomplete members (those for which `graph.hasEntity(element.id)` returns @@ -94,7 +99,6 @@ export function osmJoinWays(toJoin, graph) { return (which instanceof osmWay) ? action(graph).entity(which.id) : which; } - // make a copy containing only the ways to join toJoin = toJoin.filter(function(member) { return member.type === 'way' && graph.hasEntity(member.id); From 03fa6e7be915156bbe2a6bb5544e288a0c376df0 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 15 Jan 2018 22:02:43 -0500 Subject: [PATCH 08/15] Add tryInsert option to osmJoinWays --- modules/actions/add_member.js | 13 +++++- modules/actions/split.js | 56 +++++++++++++----------- modules/osm/multipolygon.js | 81 ++++++++++++++++++++++------------- test/spec/actions/split.js | 61 +++++++++++++++++++------- 4 files changed, 142 insertions(+), 69 deletions(-) diff --git a/modules/actions/add_member.js b/modules/actions/add_member.js index 195dd7df9..3d7654d36 100644 --- a/modules/actions/add_member.js +++ b/modules/actions/add_member.js @@ -4,12 +4,16 @@ import { osmJoinWays } from '../osm'; export function actionAddMember(relationId, member, memberIndex) { return function(graph) { var relation = graph.entity(relationId); + var numAdded = 0; + // If we weren't passed a memberIndex, + // try to perform sensible inserts based on how the ways join together if (isNaN(memberIndex) && member.type === 'way') { var members = relation.indexedMembers(); members.push(member); var joined = osmJoinWays(members, graph); + for (var i = 0; i < joined.length; i++) { var segment = joined[i]; for (var j = 0; j < segment.length && segment.length >= 2; j++) { @@ -23,10 +27,17 @@ export function actionAddMember(relationId, member, memberIndex) { } else { memberIndex = Math.min(segment[j - 1].index + 1, segment[j + 1].index + 1); } + + relation = relation.addMember(member, memberIndex + (numAdded++)); } } } - return graph.replace(relation.addMember(member, memberIndex)); + // By default, add at index (or append to end if index undefined) + if (!numAdded) { + relation = relation.addMember(member, memberIndex); + } + + return graph.replace(relation); }; } diff --git a/modules/actions/split.js b/modules/actions/split.js index 6fd73f0bc..e0bd8b20f 100644 --- a/modules/actions/split.js +++ b/modules/actions/split.js @@ -29,7 +29,7 @@ import { utilWrap } from '../util'; // https://github.com/systemed/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/SplitWayAction.as // export function actionSplit(nodeId, newWayIds) { - var wayIds; + var _wayIDs; // if the way is closed, we need to search for a partner node // to split the way at. @@ -42,11 +42,11 @@ export function actionSplit(nodeId, newWayIds) { // For example: bone-shaped areas get split across their waist // line, circles across the diameter. function splitArea(nodes, idxA, graph) { - var lengths = new Array(nodes.length), - length, - i, - best = 0, - idxB; + var lengths = new Array(nodes.length); + var length; + var i; + var best = 0; + var idxB; function wrap(index) { return utilWrap(index, nodes.length); @@ -84,16 +84,16 @@ export function actionSplit(nodeId, newWayIds) { function split(graph, wayA, newWayId) { - var wayB = osmWay({id: newWayId, tags: wayA.tags}), - nodesA, - nodesB, - isArea = wayA.isArea(), - isOuter = osmIsSimpleMultipolygonOuterMember(wayA, graph); + var wayB = osmWay({id: newWayId, tags: wayA.tags}); + var nodesA; + var nodesB; + var isArea = wayA.isArea(); + var isOuter = osmIsSimpleMultipolygonOuterMember(wayA, graph); if (wayA.isClosed()) { - var nodes = wayA.nodes.slice(0, -1), - idxA = _indexOf(nodes, nodeId), - idxB = splitArea(nodes, idxA, graph); + var nodes = wayA.nodes.slice(0, -1); + var idxA = _indexOf(nodes, nodeId); + var idxB = splitArea(nodes, idxA, graph); if (idxB < idxA) { nodesA = nodes.slice(idxA).concat(nodes.slice(0, idxB + 1)); @@ -134,7 +134,14 @@ export function actionSplit(nodeId, newWayIds) { role: relation.memberById(wayA.id).role }; - graph = actionAddMember(relation.id, member)(graph); + // how many times should this member be inserted? + // var matches = relation.members.filter(function(member) { + // return member.id === wayA.id; + // }); + + // matches.forEach(function() { + graph = actionAddMember(relation.id, member)(graph); + // }); } }); @@ -144,7 +151,8 @@ export function actionSplit(nodeId, newWayIds) { members: [ {id: wayA.id, role: 'outer', type: 'way'}, {id: wayB.id, role: 'outer', type: 'way'} - ]}); + ] + }); graph = graph.replace(multipolygon); graph = graph.replace(wayA.update({tags: {}})); @@ -165,15 +173,15 @@ export function actionSplit(nodeId, newWayIds) { action.ways = function(graph) { - var node = graph.entity(nodeId), - parents = graph.parentWays(node), - hasLines = _some(parents, function(parent) { return parent.geometry(graph) === 'line'; }); + var node = graph.entity(nodeId); + var parents = graph.parentWays(node); + var hasLines = _some(parents, function(parent) { return parent.geometry(graph) === 'line'; }); return parents.filter(function(parent) { - if (wayIds && wayIds.indexOf(parent.id) === -1) + if (_wayIDs && _wayIDs.indexOf(parent.id) === -1) return false; - if (!wayIds && hasLines && parent.geometry(graph) !== 'line') + if (!_wayIDs && hasLines && parent.geometry(graph) !== 'line') return false; if (parent.isClosed()) { @@ -193,14 +201,14 @@ export function actionSplit(nodeId, newWayIds) { action.disabled = function(graph) { var candidates = action.ways(graph); - if (candidates.length === 0 || (wayIds && wayIds.length !== candidates.length)) + if (candidates.length === 0 || (_wayIDs && _wayIDs.length !== candidates.length)) return 'not_eligible'; }; action.limitWays = function(_) { - if (!arguments.length) return wayIds; - wayIds = _; + if (!arguments.length) return _wayIDs; + _wayIDs = _; return action; }; diff --git a/modules/osm/multipolygon.js b/modules/osm/multipolygon.js index 322626df8..c8412fecf 100644 --- a/modules/osm/multipolygon.js +++ b/modules/osm/multipolygon.js @@ -75,31 +75,36 @@ export function osmSimpleMultipolygonOuterMember(entity, graph) { // ordered array of member nodes, with appropriate order reversal and // start/end coordinate de-duplication. // +// Members of `toJoin` must have, at minimum, `type` and `id` properties. +// Thus either an array of `osmWay`s or a relation member array may be used. +// +// If an member is an `osmWay`, its tags and childnodes may be reversed via +// `actionReverse` in the output. +// // The returned sequences array also has an `actions` array property, containing // any reversal actions that should be applied to the graph, should the calling // code attempt to actually join the given ways. // -// Members of `toJoin` must have, at minimum, `type` and `id` properties. -// Thus either an array of `osmWay`s or a relation member array may be used. -// -// If an member is an `osmWay`, its tags and childnodes will be reversed via -// `actionReverse` in the output. -// // Incomplete members (those for which `graph.hasEntity(element.id)` returns // false) and non-way members are ignored. // -export function osmJoinWays(toJoin, graph) { +// `tryInsert` is an optional object. +// If supplied, insert the given way/member after an existing way/member: +// `{ item: wayOrMember, afterID: id }` +// (This feature is used by `actionSplit`) +// +export function osmJoinWays(toJoin, graph, tryInsert) { function resolve(member) { return graph.childNodes(graph.entity(member.id)); } - function reverse(which) { - var action = actionReverse(which.id, { reverseOneway: true }); + function reverse(item) { + var action = actionReverse(item.id, { reverseOneway: true }); sequences.actions.push(action); - return (which instanceof osmWay) ? action(graph).entity(which.id) : which; + return (item instanceof osmWay) ? action(graph).entity(item.id) : item; } - // make a copy containing only the ways to join + // make a copy containing only the items to join toJoin = toJoin.filter(function(member) { return member.type === 'way' && graph.hasEntity(member.id); }); @@ -109,10 +114,11 @@ export function osmJoinWays(toJoin, graph) { while (toJoin.length) { // start a new sequence - var way = toJoin.shift(); - var currWays = [way]; - var currNodes = resolve(way).slice(); + var item = toJoin.shift(); + var currWays = [item]; + var currNodes = resolve(item).slice(); var doneSequence = false; + var isInserting = false; // add to it while (toJoin.length && !doneSequence) { @@ -122,10 +128,21 @@ export function osmJoinWays(toJoin, graph) { var nodes = null; var i; - // find the next way - for (i = 0; i < toJoin.length; i++) { - way = toJoin[i]; - nodes = resolve(way); + // Find the next way/member to join. + // If it is time to attempt an insert, try that item first. + // Otherwise, search for a next item in `toJoin` + var toCheck; + if (!isInserting && tryInsert && tryInsert.afterID === currWays[currWays.length - 1].id) { + toCheck = [tryInsert.item]; + isInserting = true; + } else { + toCheck = toJoin.slice(); + isInserting = false; + } + + for (i = 0; i < toCheck.length; i++) { + item = toCheck[i]; + nodes = resolve(item); // Strongly prefer to generate a forward path that preserves the order // of the members array. For multipolygons and most relations, member @@ -148,7 +165,7 @@ export function osmJoinWays(toJoin, graph) { } else if (nodes[nodes.length - 1] === end) { fn = currNodes.push; // join to end nodes = nodes.slice(0, -1).reverse(); - way = reverse(way); + item = reverse(item); break; } else if (nodes[nodes.length - 1] === start) { fn = currNodes.unshift; // join to beginning @@ -157,25 +174,31 @@ export function osmJoinWays(toJoin, graph) { } else if (nodes[0] === start) { fn = currNodes.unshift; // join to beginning nodes = nodes.slice(1).reverse(); - way = reverse(way); + item = reverse(item); break; } else { fn = nodes = null; } } - if (!nodes) { - doneSequence = true; // couldn't find a joinable way - break; + if (nodes) { // we found something to join + fn.apply(currWays, [item]); + fn.apply(currNodes, nodes); + + if (!isInserting) { + toJoin.splice(i, 1); + } + + } else { // couldn't find a joinable way/member + // if inserting, restart the loop and look in `toJoin` next time. + // if not inserting, there is nowhere else to look, mark as done. + if (!isInserting) { + doneSequence = true; + break; + } } - - fn.apply(currWays, [way]); - fn.apply(currNodes, nodes); - - toJoin.splice(i, 1); } - currWays.nodes = currNodes; sequences.push(currWays); } diff --git a/test/spec/actions/split.js b/test/spec/actions/split.js index cc5fd31c4..1ebbcf89c 100644 --- a/test/spec/actions/split.js +++ b/test/spec/actions/split.js @@ -183,7 +183,7 @@ describe('iD.actionSplit', function () { iD.osmNode({id: 'a'}), iD.osmNode({id: 'b'}), iD.osmNode({id: 'c'}), - iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), iD.osmNode({id: '*'}), iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) @@ -202,7 +202,7 @@ describe('iD.actionSplit', function () { iD.osmNode({id: 'a'}), iD.osmNode({id: 'b'}), iD.osmNode({id: 'c'}), - iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), iD.osmNode({id: '*'}), iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) @@ -244,7 +244,7 @@ describe('iD.actionSplit', function () { iD.osmNode({id: 'a'}), iD.osmNode({id: 'b'}), iD.osmNode({id: 'c'}), - iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), iD.osmWay({id: '-', nodes: ['a', 'b', 'c', 'a', 'd']}) ]); @@ -314,15 +314,15 @@ describe('iD.actionSplit', function () { describe('member ordering', function () { - it('adds the new way to parent relations (no connections)', function () { + it('adds the new way to parent relations (simple)', function () { // Situation: - // a ---- b ---- c + // a ----> b ----> c // Relation: [----] // // Split at b. // // Expected result: - // a ---- b ==== c + // a ----> b ====> c // Relation: [----, ====] // var graph = iD.coreGraph([ @@ -343,13 +343,13 @@ describe('iD.actionSplit', function () { it('adds the new way to parent relations (forward order)', function () { // Situation: - // a ---- b ---- c ~~~~ d + // a ----> b ----> c ~~~~> d // Relation: [----, ~~~~] // // Split at b. // // Expected result: - // a ---- b ==== c ~~~~ d + // a ----> b ====> c ~~~~> d // Relation: [----, ====, ~~~~] // var graph = iD.coreGraph([ @@ -370,13 +370,13 @@ describe('iD.actionSplit', function () { it('adds the new way to parent relations (reverse order)', function () { // Situation: - // a ---- b ---- c ~~~~ d + // a ----> b ----> c ~~~~> d // Relation: [~~~~, ----] // // Split at b. // // Expected result: - // a ---- b ==== c ~~~~ d + // a ----> b ====> c ~~~~> d // Relation: [~~~~, ====, ----] // var graph = iD.coreGraph([ @@ -397,13 +397,13 @@ describe('iD.actionSplit', function () { it('adds the new way to parent relations (unsplit way belongs multiple times)', function () { // Situation: - // a ---- b ---- c ~~~~ d + // a ----> b ----> c ~~~~> d // Relation: [~~~~, ----, ~~~~] // // Split at b. // // Expected result: - // a ---- b ==== c ~~~~ d + // a ----> b ====> c ~~~~> d // Relation: [~~~~, ====, ----, ====, ~~~~] // var graph = iD.coreGraph([ @@ -426,15 +426,15 @@ describe('iD.actionSplit', function () { expect(ids).to.have.ordered.members(['~', '=', '-', '=', '~']); }); - it('adds the new way to parent relations (split way belongs multiple times)', function () { + it('adds the new way to parent relations (forward split way belongs multiple times)', function () { // Situation: - // a ---- b ---- c ~~~~ d + // a ----> b ----> c ~~~~> d // Relation: [----, ~~~~, ----] // // Split at b. // // Expected result: - // a ---- b ==== c ~~~~ d + // a ----> b ====> c ~~~~> d // Relation: [----, ====, ~~~~, ====, ----] // var graph = iD.coreGraph([ @@ -456,6 +456,37 @@ describe('iD.actionSplit', function () { var ids = graph.entity('r').members.map(function(m) { return m.id; }); expect(ids).to.have.ordered.members(['-', '=', '~', '=', '-']); }); + + it('adds the new way to parent relations (reverse split way belongs multiple times)', function () { + // Situation: + // a <---- b <---- c ~~~~> d + // Relation: [----, ~~~~, ----] + // + // Split at b. + // + // Expected result: + // a <==== b <---- c ~~~~> d + // Relation: [====, ----, ~~~~, ----, ====] + // + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmWay({id: '-', nodes: ['c', 'b', 'a']}), + iD.osmWay({id: '~', nodes: ['c', 'd']}), + iD.osmRelation({id: 'r', members: [ + {id: '-', type: 'way'}, + {id: '~', type: 'way'}, + {id: '-', type: 'way'} + ]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + var ids = graph.entity('r').members.map(function(m) { return m.id; }); + expect(ids).to.have.ordered.members(['=', '-', '~', '-', '=']); + }); }); From 221158e9184731be58eb5408b3e3f431154d3be3 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 15 Jan 2018 23:13:59 -0500 Subject: [PATCH 09/15] WIP: Add insertHint to actionAddMember, actionSplit --- modules/actions/add_member.js | 16 +++++++++++----- modules/actions/split.js | 12 +++++------- modules/osm/multipolygon.js | 12 ++++++------ test/spec/actions/add_member.js | 12 ++++++++---- 4 files changed, 30 insertions(+), 22 deletions(-) diff --git a/modules/actions/add_member.js b/modules/actions/add_member.js index 3d7654d36..d5f5b26b7 100644 --- a/modules/actions/add_member.js +++ b/modules/actions/add_member.js @@ -1,18 +1,21 @@ import { osmJoinWays } from '../osm'; -export function actionAddMember(relationId, member, memberIndex) { - return function(graph) { +export function actionAddMember(relationId, member, memberIndex, insertHint) { + + var action = function(graph) { var relation = graph.entity(relationId); var numAdded = 0; // If we weren't passed a memberIndex, // try to perform sensible inserts based on how the ways join together - if (isNaN(memberIndex) && member.type === 'way') { + if ((isNaN(memberIndex) || insertHint) && member.type === 'way') { var members = relation.indexedMembers(); - members.push(member); + if (!insertHint) { + members.push(member); // just push and let osmJoinWays sort it out + } - var joined = osmJoinWays(members, graph); + var joined = osmJoinWays(members, graph, insertHint); for (var i = 0; i < joined.length; i++) { var segment = joined[i]; @@ -40,4 +43,7 @@ export function actionAddMember(relationId, member, memberIndex) { return graph.replace(relation); }; + + + return action; } diff --git a/modules/actions/split.js b/modules/actions/split.js index e0bd8b20f..a9d0bd77f 100644 --- a/modules/actions/split.js +++ b/modules/actions/split.js @@ -134,14 +134,12 @@ export function actionSplit(nodeId, newWayIds) { role: relation.memberById(wayA.id).role }; - // how many times should this member be inserted? - // var matches = relation.members.filter(function(member) { - // return member.id === wayA.id; - // }); + var insertHint = { + item: member, + nextTo: wayA.id + }; - // matches.forEach(function() { - graph = actionAddMember(relation.id, member)(graph); - // }); + graph = actionAddMember(relation.id, member, undefined, insertHint)(graph); } }); diff --git a/modules/osm/multipolygon.js b/modules/osm/multipolygon.js index c8412fecf..0990e3e30 100644 --- a/modules/osm/multipolygon.js +++ b/modules/osm/multipolygon.js @@ -88,12 +88,12 @@ export function osmSimpleMultipolygonOuterMember(entity, graph) { // Incomplete members (those for which `graph.hasEntity(element.id)` returns // false) and non-way members are ignored. // -// `tryInsert` is an optional object. -// If supplied, insert the given way/member after an existing way/member: -// `{ item: wayOrMember, afterID: id }` +// `insertHint` is an optional object. +// If supplied, insert the given way/member next to an existing way/member: +// `{ item: wayOrMember, nextTo: id }` // (This feature is used by `actionSplit`) // -export function osmJoinWays(toJoin, graph, tryInsert) { +export function osmJoinWays(toJoin, graph, insertHint) { function resolve(member) { return graph.childNodes(graph.entity(member.id)); } @@ -132,8 +132,8 @@ export function osmJoinWays(toJoin, graph, tryInsert) { // If it is time to attempt an insert, try that item first. // Otherwise, search for a next item in `toJoin` var toCheck; - if (!isInserting && tryInsert && tryInsert.afterID === currWays[currWays.length - 1].id) { - toCheck = [tryInsert.item]; + if (!isInserting && insertHint && insertHint.nextTo === currWays[currWays.length - 1].id) { + toCheck = [insertHint.item]; isInserting = true; } else { toCheck = toJoin.slice(); diff --git a/test/spec/actions/add_member.js b/test/spec/actions/add_member.js index 95e9d98a6..863045849 100644 --- a/test/spec/actions/add_member.js +++ b/test/spec/actions/add_member.js @@ -101,7 +101,7 @@ describe('iD.actionAddMember', function() { expect(members(graph)).to.eql(['-', '=', '~']); }); - it('inserts the member multiple times if the way exists multiple times (middle)', function() { + it('inserts the member multiple times if hint provided (middle)', function() { // Before: a ---> b .. c ~~~> d <~~~ c .. b <--- a // After: a ---> b ===> c ~~~> d <~~~ c <=== b <--- a var graph = iD.coreGraph([ @@ -120,11 +120,13 @@ describe('iD.actionAddMember', function() { ]}) ]); - graph = iD.actionAddMember('r', {id: '=', type: 'way'})(graph); + var member = { id: '=', type: 'way' }; + var hint = { item: member, nextTo: '-' }; + graph = iD.actionAddMember('r', member, undefined, hint)(graph); expect(members(graph)).to.eql(['-', '=', '~', '~', '=', '-']); }); - it('inserts the member multiple times if the way exists multiple times (beginning/end)', function() { + it('inserts the member multiple times if hint provided (beginning/end)', function() { // Before: b ===> c ~~~> d <~~~ c <=== b // After: a ---> b ===> c ~~~> d <~~~ c <=== b <--- a var graph = iD.coreGraph([ @@ -143,7 +145,9 @@ describe('iD.actionAddMember', function() { ]}) ]); - graph = iD.actionAddMember('r', {id: '-', type: 'way'})(graph); + var member = { id: '-', type: 'way' }; + var hint = { item: member, nextTo: '=' }; + graph = iD.actionAddMember('r', member, undefined, hint)(graph); expect(members(graph)).to.eql(['-', '=', '~', '~', '=', '-']); }); From be46e85ec09fa6a4f7b86225d082c80b96f68001 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 16 Jan 2018 17:41:14 -0500 Subject: [PATCH 10/15] Move insert way pairing code from osmJoinWays to actionAddMember (tests for actionAddMember now passing!) --- modules/actions/add_member.js | 128 +++++++++++++++++++++++--------- modules/actions/split.js | 10 ++- modules/osm/multipolygon.js | 47 +++--------- test/spec/actions/add_member.js | 28 ++++--- 4 files changed, 128 insertions(+), 85 deletions(-) diff --git a/modules/actions/add_member.js b/modules/actions/add_member.js index d5f5b26b7..ab15f343c 100644 --- a/modules/actions/add_member.js +++ b/modules/actions/add_member.js @@ -1,49 +1,105 @@ -import { osmJoinWays } from '../osm'; +import _groupBy from 'lodash-es/groupBy'; + +import { osmJoinWays, osmWay } from '../osm'; -export function actionAddMember(relationId, member, memberIndex, insertHint) { +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}); + } + var action = function(graph) { var relation = graph.entity(relationId); - var numAdded = 0; - // If we weren't passed a memberIndex, - // try to perform sensible inserts based on how the ways join together - if ((isNaN(memberIndex) || insertHint) && member.type === 'way') { - var members = relation.indexedMembers(); - if (!insertHint) { - members.push(member); // just push and let osmJoinWays sort it out - } - - var joined = osmJoinWays(members, graph, insertHint); - - for (var i = 0; i < joined.length; i++) { - var segment = joined[i]; - for (var j = 0; j < segment.length && segment.length >= 2; j++) { - if (segment[j] !== member) - continue; - - if (j === 0) { - memberIndex = segment[j + 1].index; - } else if (j === segment.length - 1) { - memberIndex = segment[j - 1].index + 1; - } else { - memberIndex = Math.min(segment[j - 1].index + 1, segment[j + 1].index + 1); - } - - relation = relation.addMember(member, memberIndex + (numAdded++)); - } - } + if ((isNaN(memberIndex) || insertPair) && member.type === 'way') { + // Try to perform sensible inserts based on how the ways join together + graph = addWayMember(relation, graph); + } else { + graph = graph.replace(relation.addMember(member, memberIndex)); } - // By default, add at index (or append to end if index undefined) - if (!numAdded) { - relation = relation.addMember(member, memberIndex); - } - - return graph.replace(relation); + return graph; }; + // Add a way member into the relation "wherever it makes sense". + // In this situation we were not supplied a memberIndex. + function addWayMember(relation, graph) { + var groups; + var tempWay; + var i, j; + + if (insertPair) { + // We're adding a member that must stay paired with an existing member. + // (This feature is used by `actionSplit`) + // + // This is tricky because the members may exist multiple times in the + // member list, and with different A-B/B-A ordering and different roles. + // (e.g. a bus route that loops out and back - #4589). + // + // Replace the existing member with a temporary way, + // so that `osmJoinWays` can treat the pair like a single way. + tempWay = osmWay({ id: 'wTemp', nodes: insertPair.nodes }); + graph = graph.replace(tempWay); + var tempMember = { id: tempWay.id, type: 'way', role: '' }; + var tempRelation = replaceMemberAll(relation, insertPair.originalID, tempMember); + groups = _groupBy(tempRelation.members, function(m) { return m.type; }); + groups.way = groups.way || []; + + } else { + // Add the member anywhere.. Just push and let `osmJoinWays` decide where to put it. + groups = _groupBy(relation.members, function(m) { return m.type; }); + groups.way = groups.way || []; + groups.way.push(member); + } + + var joined = osmJoinWays(groups.way, graph); + + var newWayMembers = []; + for (i = 0; i < joined.length; i++) { + var segment = joined[i]; + var nodes = segment.nodes.slice(); + + for (j = 0; j < segment.length; j++) { + var way = graph.entity(segment[j].id); + if (tempWay && segment[j].id === tempWay.id) { + if (nodes[0].id === insertPair.nodes[0]) { + newWayMembers.push({ id: insertPair.originalID, type: 'way', role: segment[j].role }); + newWayMembers.push({ id: insertPair.insertedID, type: 'way', role: segment[j].role }); + } else { + newWayMembers.push({ id: insertPair.insertedID, type: 'way', role: segment[j].role }); + newWayMembers.push({ id: insertPair.originalID, type: 'way', role: segment[j].role }); + } + } else { + newWayMembers.push(segment[j]); + } + nodes.splice(0, way.nodes.length - 1); + } + } + + if (tempWay) { + graph = graph.remove(tempWay); + } + + // Write members in the order: nodes, ways, relations + // This is reccomended for Public Transport routes: + // see https://wiki.openstreetmap.org/wiki/Public_transport#Service_routes + var newMembers = (groups.node || []).concat(newWayMembers, (groups.relation || [])); + return graph.replace(relation.update({members: newMembers})); + } + + return action; } diff --git a/modules/actions/split.js b/modules/actions/split.js index a9d0bd77f..1763274ac 100644 --- a/modules/actions/split.js +++ b/modules/actions/split.js @@ -85,6 +85,7 @@ export function actionSplit(nodeId, newWayIds) { function split(graph, wayA, newWayId) { var wayB = osmWay({id: newWayId, tags: wayA.tags}); + var origNodes = wayA.nodes.slice(); var nodesA; var nodesB; var isArea = wayA.isArea(); @@ -134,12 +135,13 @@ export function actionSplit(nodeId, newWayIds) { role: relation.memberById(wayA.id).role }; - var insertHint = { - item: member, - nextTo: wayA.id + var insertPair = { + originalID: wayA.id, + insertedID: wayB.id, + nodes: origNodes }; - graph = actionAddMember(relation.id, member, undefined, insertHint)(graph); + graph = actionAddMember(relation.id, member, undefined, insertPair)(graph); } }); diff --git a/modules/osm/multipolygon.js b/modules/osm/multipolygon.js index 0990e3e30..41e61a5ae 100644 --- a/modules/osm/multipolygon.js +++ b/modules/osm/multipolygon.js @@ -88,12 +88,7 @@ export function osmSimpleMultipolygonOuterMember(entity, graph) { // Incomplete members (those for which `graph.hasEntity(element.id)` returns // false) and non-way members are ignored. // -// `insertHint` is an optional object. -// If supplied, insert the given way/member next to an existing way/member: -// `{ item: wayOrMember, nextTo: id }` -// (This feature is used by `actionSplit`) -// -export function osmJoinWays(toJoin, graph, insertHint) { +export function osmJoinWays(toJoin, graph) { function resolve(member) { return graph.childNodes(graph.entity(member.id)); } @@ -109,6 +104,7 @@ export function osmJoinWays(toJoin, graph, insertHint) { return member.type === 'way' && graph.hasEntity(member.id); }); + var sequences = []; sequences.actions = []; @@ -118,7 +114,6 @@ export function osmJoinWays(toJoin, graph, insertHint) { var currWays = [item]; var currNodes = resolve(item).slice(); var doneSequence = false; - var isInserting = false; // add to it while (toJoin.length && !doneSequence) { @@ -129,19 +124,8 @@ export function osmJoinWays(toJoin, graph, insertHint) { var i; // Find the next way/member to join. - // If it is time to attempt an insert, try that item first. - // Otherwise, search for a next item in `toJoin` - var toCheck; - if (!isInserting && insertHint && insertHint.nextTo === currWays[currWays.length - 1].id) { - toCheck = [insertHint.item]; - isInserting = true; - } else { - toCheck = toJoin.slice(); - isInserting = false; - } - - for (i = 0; i < toCheck.length; i++) { - item = toCheck[i]; + for (i = 0; i < toJoin.length; i++) { + item = toJoin[i]; nodes = resolve(item); // Strongly prefer to generate a forward path that preserves the order @@ -181,22 +165,15 @@ export function osmJoinWays(toJoin, graph, insertHint) { } } - if (nodes) { // we found something to join - fn.apply(currWays, [item]); - fn.apply(currNodes, nodes); - - if (!isInserting) { - toJoin.splice(i, 1); - } - - } else { // couldn't find a joinable way/member - // if inserting, restart the loop and look in `toJoin` next time. - // if not inserting, there is nowhere else to look, mark as done. - if (!isInserting) { - doneSequence = true; - break; - } + if (!nodes) { // couldn't find a joinable way/member + doneSequence = true; + break; } + + fn.apply(currWays, [item]); + fn.apply(currNodes, nodes); + + toJoin.splice(i, 1); } currWays.nodes = currNodes; diff --git a/test/spec/actions/add_member.js b/test/spec/actions/add_member.js index 863045849..3f7895097 100644 --- a/test/spec/actions/add_member.js +++ b/test/spec/actions/add_member.js @@ -101,7 +101,7 @@ describe('iD.actionAddMember', function() { expect(members(graph)).to.eql(['-', '=', '~']); }); - it('inserts the member multiple times if hint provided (middle)', function() { + it('inserts the member multiple times if insertPair provided (middle)', function() { // Before: a ---> b .. c ~~~> d <~~~ c .. b <--- a // After: a ---> b ===> c ~~~> d <~~~ c <=== b <--- a var graph = iD.coreGraph([ @@ -121,21 +121,25 @@ describe('iD.actionAddMember', function() { ]); var member = { id: '=', type: 'way' }; - var hint = { item: member, nextTo: '-' }; - graph = iD.actionAddMember('r', member, undefined, hint)(graph); + var insertPair = { + originalID: '-', + insertedID: '=', + nodes: ['a','b','c'] + }; + graph = iD.actionAddMember('r', member, undefined, insertPair)(graph); expect(members(graph)).to.eql(['-', '=', '~', '~', '=', '-']); }); - it('inserts the member multiple times if hint provided (beginning/end)', function() { - // Before: b ===> c ~~~> d <~~~ c <=== b - // After: a ---> b ===> c ~~~> d <~~~ c <=== b <--- a + it('inserts the member multiple times if insertPair provided (beginning/end)', function() { + // Before: b <=== c ~~~> d <~~~ c ===> b + // After: a <--- b <=== c ~~~> d <~~~ c ===> b ---> a var graph = iD.coreGraph([ iD.osmNode({id: 'a', loc: [0, 0]}), iD.osmNode({id: 'b', loc: [0, 0]}), iD.osmNode({id: 'c', loc: [0, 0]}), iD.osmNode({id: 'd', loc: [0, 0]}), - iD.osmWay({id: '-', nodes: ['a', 'b']}), - iD.osmWay({id: '=', nodes: ['b', 'c']}), + iD.osmWay({id: '-', nodes: ['b', 'a']}), + iD.osmWay({id: '=', nodes: ['c', 'b']}), iD.osmWay({id: '~', nodes: ['c', 'd']}), iD.osmRelation({id: 'r', members: [ {id: '=', type: 'way'}, @@ -146,8 +150,12 @@ describe('iD.actionAddMember', function() { ]); var member = { id: '-', type: 'way' }; - var hint = { item: member, nextTo: '=' }; - graph = iD.actionAddMember('r', member, undefined, hint)(graph); + var insertPair = { + originalID: '=', + insertedID: '-', + nodes: ['c','b','a'] + }; + graph = iD.actionAddMember('r', member, undefined, insertPair)(graph); expect(members(graph)).to.eql(['-', '=', '~', '~', '=', '-']); }); From 3be577d8db034024bcc6fb7d3a7034da58987986 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 16 Jan 2018 21:37:43 -0500 Subject: [PATCH 11/15] However we fix actionAddMember, it needs to work for incomplete relations --- test/spec/actions/add_member.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/spec/actions/add_member.js b/test/spec/actions/add_member.js index 3f7895097..40b9a63c2 100644 --- a/test/spec/actions/add_member.js +++ b/test/spec/actions/add_member.js @@ -10,6 +10,26 @@ describe('iD.actionAddMember', function() { return graph.entity('r').members.map(function (m) { return m.id; }); } + it('handles incomplete relations', function () { + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0, 0]}), + iD.osmNode({id: 'b', loc: [0, 0]}), + iD.osmNode({id: 'c', loc: [0, 0]}), + iD.osmNode({id: 'd', loc: [0, 0]}), + iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), + iD.osmWay({id: '=', nodes: ['c','d']}), + iD.osmRelation({id: 'r', members: [ + {id: '~', type: 'way'}, + {id: '-', type: 'way'} + ]}) + ]); + + graph = iD.actionAddMember('r', {id: '=', type: 'way'})(graph); + + var ids = graph.entity('r').members.map(function(m) { return m.id; }); + expect(members(graph)).to.eql(['~', '-', '=']); + }); + it('adds the member to a relation with no members', function() { var graph = iD.coreGraph([ iD.osmNode({id: 'a', loc: [0, 0]}), From 8f9a46b75a8ac9a9030a9e11cbd4be589867374f Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Wed, 17 Jan 2018 22:59:55 -0500 Subject: [PATCH 12/15] Change actionAddMember to rearrange indexed members in place This allows it to work around issues where a relation may not be completely downloaded --- modules/actions/add_member.js | 159 ++++++++++++++++++++++++++-------- 1 file changed, 122 insertions(+), 37 deletions(-) diff --git a/modules/actions/add_member.js b/modules/actions/add_member.js index ab15f343c..67f1d8b04 100644 --- a/modules/actions/add_member.js +++ b/modules/actions/add_member.js @@ -1,26 +1,13 @@ +import _clone from 'lodash-es/clone'; import _groupBy from 'lodash-es/groupBy'; +import _omit from 'lodash-es/omit'; import { osmJoinWays, osmWay } from '../osm'; 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}); - } - - - var action = function(graph) { + return function action(graph) { var relation = graph.entity(relationId); if ((isNaN(memberIndex) || insertPair) && member.type === 'way') { @@ -37,9 +24,7 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) { // Add a way member into the relation "wherever it makes sense". // In this situation we were not supplied a memberIndex. function addWayMember(relation, graph) { - var groups; - var tempWay; - var i, j; + var groups, tempWay, item, i, j, k; if (insertPair) { // We're adding a member that must stay paired with an existing member. @@ -59,32 +44,56 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) { groups.way = groups.way || []; } else { - // Add the member anywhere.. Just push and let `osmJoinWays` decide where to put it. + // Add the member anywhere, one time. Just push and let `osmJoinWays` decide where to put it. groups = _groupBy(relation.members, function(m) { return m.type; }); groups.way = groups.way || []; groups.way.push(member); } - var joined = osmJoinWays(groups.way, graph); + var members = withIndex(groups.way); + var joined = osmJoinWays(members, graph); - var newWayMembers = []; + // `joined` might not contain all of the way members, + // But will contain only the completed (downloaded) members for (i = 0; i < joined.length; i++) { var segment = joined[i]; var nodes = segment.nodes.slice(); + var startIndex = segment[0].index; - for (j = 0; j < segment.length; j++) { - var way = graph.entity(segment[j].id); - if (tempWay && segment[j].id === tempWay.id) { - if (nodes[0].id === insertPair.nodes[0]) { - newWayMembers.push({ id: insertPair.originalID, type: 'way', role: segment[j].role }); - newWayMembers.push({ id: insertPair.insertedID, type: 'way', role: segment[j].role }); - } else { - newWayMembers.push({ id: insertPair.insertedID, type: 'way', role: segment[j].role }); - newWayMembers.push({ id: insertPair.originalID, type: 'way', role: segment[j].role }); - } - } else { - newWayMembers.push(segment[j]); + // j = array index in `members` where this segment starts + for (j = 0; j < members.length; j++) { + if (members[j].index === startIndex) { + break; } + } + + // k = each member in segment + for (k = 0; k < segment.length; k++) { + item = segment[k]; + var way = graph.entity(item.id); + + // If this is a paired item, generate members in correct order and role + if (tempWay && item.id === tempWay.id) { + if (nodes[0].id === insertPair.nodes[0]) { + item.pair = [ + { id: insertPair.originalID, type: 'way', role: item.role }, + { id: insertPair.insertedID, type: 'way', role: item.role } + ]; + } else { + item.pair = [ + { id: insertPair.insertedID, type: 'way', role: item.role }, + { id: insertPair.originalID, type: 'way', role: item.role } + ]; + } + } + + // reorder `members` if necessary + if (k > 0) { + if (j+k >= members.length || item.index !== members[j+k].index) { + moveMember(members, item.index, j+k); + } + } + nodes.splice(0, way.nodes.length - 1); } } @@ -93,13 +102,89 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) { graph = graph.remove(tempWay); } + // Final pass: skip dead items, split pairs, remove index properties + var wayMembers = []; + for (i = 0; i < members.length; i++) { + item = members[i]; + if (item.index === -1) continue; + + if (item.pair) { + wayMembers.push(item.pair[0]); + wayMembers.push(item.pair[1]); + } else { + wayMembers.push(_omit(item, 'index')); + } + } + // Write members in the order: nodes, ways, relations // This is reccomended for Public Transport routes: // see https://wiki.openstreetmap.org/wiki/Public_transport#Service_routes - var newMembers = (groups.node || []).concat(newWayMembers, (groups.relation || [])); + var newMembers = (groups.node || []).concat(wayMembers, (groups.relation || [])); + return graph.replace(relation.update({members: newMembers})); + + + // `moveMember()` changes the `members` array in place by splicing + // the item with `.index = findIndex` to where it belongs, + // and marking the old position as "dead" with `.index = -1` + // + // j=5, k=0 jk + // segment 5 4 7 6 + // members 0 1 2 3 4 5 6 7 8 9 keep 5 in j+k + // + // j=5, k=1 j k + // segment 5 4 7 6 + // members 0 1 2 3 4 5 6 7 8 9 move 4 to j+k + // members 0 1 2 3 x 5 4 6 7 8 9 moved + // + // j=5, k=2 j k + // segment 5 4 7 6 + // members 0 1 2 3 x 5 4 6 7 8 9 move 7 to j+k + // members 0 1 2 3 x 5 4 7 6 x 8 9 moved + // + // j=5, k=3 j k + // segment 5 4 7 6 + // members 0 1 2 3 x 5 4 7 6 x 8 9 keep 6 in j+k + // + function moveMember(arr, findIndex, toIndex) { + for (var i = 0; i < arr.length; i++) { + if (arr[i].index === findIndex) { + break; + } + } + + var item = _clone(arr[i]); + arr[i].index = -1; // mark as dead + item.index = toIndex; + arr.splice(toIndex, 0, item); + } + + + // 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) { + var result = new Array(arr.length); + for (var i = 0; i < arr.length; i++) { + result[i] = arr[i]; + result[i].index = i; + } + return result; + } } - - return action; } From d6afd399fcedd98f26b34180e6a941c0006e0387 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 18 Jan 2018 14:34:34 -0500 Subject: [PATCH 13/15] Revised and expanded actionSplit tests to cover route splitting --- test/spec/actions/add_member.js | 2 - test/spec/actions/split.js | 763 ++++++++++++++++++++++---------- 2 files changed, 538 insertions(+), 227 deletions(-) diff --git a/test/spec/actions/add_member.js b/test/spec/actions/add_member.js index 40b9a63c2..b2a8486ec 100644 --- a/test/spec/actions/add_member.js +++ b/test/spec/actions/add_member.js @@ -25,8 +25,6 @@ describe('iD.actionAddMember', function() { ]); graph = iD.actionAddMember('r', {id: '=', type: 'way'})(graph); - - var ids = graph.entity('r').members.map(function(m) { return m.id; }); expect(members(graph)).to.eql(['~', '-', '=']); }); diff --git a/test/spec/actions/split.js b/test/spec/actions/split.js index 1ebbcf89c..febf47d72 100644 --- a/test/spec/actions/split.js +++ b/test/spec/actions/split.js @@ -7,85 +7,122 @@ describe('iD.actionSplit', function () { describe('#disabled', function () { it('returns falsy for a non-end node of a single way', function () { + // + // a ---> b ---> c split at 'b' not disabled + // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmNode({id: 'c'}), - iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}) + iD.osmNode({ id: 'a', loc: [0, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmNode({ id: 'c', loc: [2, 0] }), + iD.osmWay({ id: '-', nodes: ['a', 'b', 'c'] }) ]); expect(iD.actionSplit('b').disabled(graph)).not.to.be.ok; }); it('returns falsy for an intersection of two ways', function () { + // + // c + // | + // a ---> * ---> b split at '*' not disabled + // | + // d + // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmNode({id: 'c'}), - iD.osmNode({id: 'c'}), - iD.osmNode({id: '*'}), - iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), - iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) + iD.osmNode({ id: 'a', loc: [-1, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmNode({ id: 'c', loc: [0, 1] }), + iD.osmNode({ id: 'd', loc: [0, -1] }), + iD.osmNode({ id: '*', loc: [0, 0] }), + iD.osmWay({ id: '-', nodes: ['a', '*', 'b'] }), + iD.osmWay({ id: '|', nodes: ['c', '*', 'd'] }) ]); expect(iD.actionSplit('*').disabled(graph)).not.to.be.ok; }); it('returns falsy for an intersection of two ways with parent way specified', function () { + // + // c + // | + // a ---> * ---> b split '-' at '*' not disabled + // | + // d + // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmNode({id: 'c'}), - iD.osmNode({id: 'c'}), - iD.osmNode({id: '*'}), - iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), - iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) + iD.osmNode({ id: 'a', loc: [-1, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmNode({ id: 'c', loc: [0, 1] }), + iD.osmNode({ id: 'd', loc: [0, -1] }), + iD.osmNode({ id: '*', loc: [0, 0] }), + iD.osmWay({ id: '-', nodes: ['a', '*', 'b'] }), + iD.osmWay({ id: '|', nodes: ['c', '*', 'd'] }) ]); expect(iD.actionSplit('*').limitWays(['-']).disabled(graph)).not.to.be.ok; }); it('returns falsy for a self-intersection', function () { + // + // b -- c + // | / + // | / split '-' at 'a' not disabled + // | / + // a -- b + // 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', 'a', 'd']}) + iD.osmNode({ id: 'a', loc: [0, 0] }), + iD.osmNode({ id: 'b', loc: [0, 2] }), + iD.osmNode({ id: 'c', loc: [1, 2] }), + iD.osmNode({ id: 'd', loc: [1, 0] }), + iD.osmWay({ id: '-', nodes: ['a', 'b', 'c', 'a', 'd'] }) ]); expect(iD.actionSplit('a').disabled(graph)).not.to.be.ok; }); it('returns \'not_eligible\' for the first node of a single way', function () { + // + // a ---> b split at 'a' disabled - 'not eligible' + // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmWay({id: '-', nodes: ['a', 'b']}) + iD.osmNode({ id: 'a', loc: [0, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmWay({ id: '-', nodes: ['a', 'b'] }) ]); expect(iD.actionSplit('a').disabled(graph)).to.equal('not_eligible'); }); it('returns \'not_eligible\' for the last node of a single way', function () { + // + // a ---> b split at 'b' disabled - 'not eligible' + // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmWay({id: '-', nodes: ['a', 'b']}) + iD.osmNode({ id: 'a', loc: [0, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmWay({ id: '-', nodes: ['a', 'b'] }) ]); expect(iD.actionSplit('b').disabled(graph)).to.equal('not_eligible'); }); it('returns \'not_eligible\' for an intersection of two ways with non-parent way specified', function () { + // + // c + // | + // a ---> * ---> b split '-' and '=' at '*' disabled - 'not eligible' + // | (there is no '=' here) + // d + // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmNode({id: 'c'}), - iD.osmNode({id: 'c'}), - iD.osmNode({id: '*'}), - iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), - iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) + iD.osmNode({ id: 'a', loc: [-1, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmNode({ id: 'c', loc: [0, 1] }), + iD.osmNode({ id: 'd', loc: [0, -1] }), + iD.osmNode({ id: '*', loc: [0, 0] }), + iD.osmWay({ id: '-', nodes: ['a', '*', 'b'] }), + iD.osmWay({ id: '|', nodes: ['c', '*', 'd'] }) ]); expect(iD.actionSplit('*').limitWays(['-', '=']).disabled(graph)).to.equal('not_eligible'); @@ -96,19 +133,18 @@ describe('iD.actionSplit', function () { describe('ways', function () { it('creates a new way with the appropriate nodes', function () { - // Situation: - // a ---- b ---- c // - // Split at b. + // Situation: + // a ---> b ---> c split at 'b' // // Expected result: - // a ---- b ==== c + // 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']}) + iD.osmNode({ id: 'a', loc: [0, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmNode({ id: 'c', loc: [2, 0] }), + iD.osmWay({ id: '-', nodes: ['a', 'b', 'c'] }) ]); graph = iD.actionSplit('b', ['='])(graph); @@ -118,12 +154,12 @@ describe('iD.actionSplit', function () { }); it('copies tags to the new way', function () { - var tags = {highway: 'residential'}; + var tags = { highway: 'residential' }; var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmNode({id: 'c'}), - iD.osmWay({id: '-', nodes: ['a', 'b', 'c'], tags: tags}) + iD.osmNode({ id: 'a', loc: [0, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmNode({ id: 'c', loc: [2, 0] }), + iD.osmWay({ id: '-', nodes: ['a', 'b', 'c'], tags: tags }) ]); graph = iD.actionSplit('b', ['='])(graph); @@ -134,23 +170,22 @@ describe('iD.actionSplit', function () { }); it('splits a way at a T-junction', function () { + // // Situation: - // a ---- b ---- c + // a ---- b ---- c split at 'b' // | // d // - // Split at b. - // // Expected result: // a ---- b ==== c // | // d // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmNode({id: 'c'}), - iD.osmNode({id: 'd'}), + iD.osmNode({ id: 'a', loc: [-1, 0] }), + iD.osmNode({ id: 'b', loc: [0, 0] }), + iD.osmNode({ id: 'c', loc: [1, 0] }), + iD.osmNode({ id: 'd', loc: [0, -1] }), iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), iD.osmWay({id: '|', nodes: ['d', 'b']}) ]); @@ -163,30 +198,29 @@ describe('iD.actionSplit', function () { }); it('splits multiple ways at an intersection', function () { - // Situation: - // c - // | - // a ---- * ---- b - // ¦ - // d // - // Split at b. + // Situation: + // c + // | + // a ---- * ---- b split at '*' + // | + // d // // Expected result: - // c - // | - // a ---- * ==== b - // ¦ - // d + // c + // | + // a ---- * ==== b + // ¦ + // d // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmNode({id: 'c'}), - iD.osmNode({id: 'd'}), - iD.osmNode({id: '*'}), - iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), - iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) + iD.osmNode({ id: 'a', loc: [-1, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmNode({ id: 'c', loc: [0, 1] }), + iD.osmNode({ id: 'd', loc: [0, -1] }), + iD.osmNode({ id: '*', loc: [0, 0] }), + iD.osmWay({ id: '-', nodes: ['a', '*', 'b'] }), + iD.osmWay({ id: '|', nodes: ['c', '*', 'd'] }) ]); graph = iD.actionSplit('*', ['=', '¦'])(graph); @@ -198,14 +232,21 @@ describe('iD.actionSplit', function () { }); it('splits the specified ways at an intersection', function () { + // + // c + // | + // a ---- * ---- b split at '*' + // | + // d + // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmNode({id: 'c'}), - iD.osmNode({id: 'd'}), - iD.osmNode({id: '*'}), - iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), - iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) + iD.osmNode({ id: 'a', loc: [-1, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmNode({ id: 'c', loc: [0, 1] }), + iD.osmNode({ id: 'd', loc: [0, -1] }), + iD.osmNode({ id: '*', loc: [0, 0] }), + iD.osmWay({ id: '-', nodes: ['a', '*', 'b'] }), + iD.osmWay({ id: '|', nodes: ['c', '*', 'd'] }) ]); var g1 = iD.actionSplit('*', ['=']).limitWays(['-'])(graph); @@ -226,26 +267,27 @@ describe('iD.actionSplit', function () { }); it('splits self-intersecting ways', function () { + // // Situation: - // b + // b + // /| // / | // / | - // c - a -- d - // - // Split at a. + // c - a -- d split at 'a' // // Expected result: - // b + // b + // /| // / | // / | // c - a == 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', 'a', 'd']}) + iD.osmNode({ id: 'a', loc: [0, 0] }), + iD.osmNode({ id: 'b', loc: [0, 2] }), + iD.osmNode({ id: 'c', loc: [-1, 0] }), + iD.osmNode({ id: 'd', loc: [1, 0] }), + iD.osmWay({ id: '-', nodes: ['a', 'b', 'c', 'a', 'd'] }) ]); graph = iD.actionSplit('a', ['='])(graph); @@ -255,24 +297,18 @@ describe('iD.actionSplit', function () { }); it('splits a closed way at the given point and its antipode', function () { + // // Situation: // a ---- b // | | // d ---- c // - // Split at a. - // - // Expected result: - // a ---- b - // || | - // d ==== c - // var graph = iD.coreGraph([ - iD.osmNode({id: 'a', loc: [0,1]}), - iD.osmNode({id: 'b', loc: [1,1]}), - iD.osmNode({id: 'c', loc: [1,0]}), - iD.osmNode({id: 'd', loc: [0,0]}), - iD.osmWay({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}) + iD.osmNode({ id: 'a', loc: [0, 1] }), + iD.osmNode({ id: 'b', loc: [1, 1] }), + iD.osmNode({ id: 'c', loc: [1, 0] }), + iD.osmNode({ id: 'd', loc: [0, 0] }), + iD.osmWay({ id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}) ]); var g1 = iD.actionSplit('a', ['='])(graph); @@ -296,197 +332,474 @@ describe('iD.actionSplit', function () { describe('relations', function () { + function members(graph) { + return graph.entity('r').members.map(function (m) { return m.id; }); + } + + it('handles incomplete relations', function () { + // + // Situation: + // a ---> b ---> c split at 'b' + // Relation: ['~', '-'] + // + // Expected result: + // a ---> b ===> c + // Relation: ['~', '-', '='] + // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmNode({id: 'c'}), - iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), - iD.osmRelation({id: 'r', members: [{id: '~', type: 'way'}, {id: '-', type: 'way'}]}) + iD.osmNode({ id: 'a', loc: [0, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmNode({ id: 'c', loc: [2, 0] }), + iD.osmWay({ id: '-', nodes: ['a', 'b', 'c'] }), + iD.osmRelation({id: 'r', members: [ + { id: '~', type: 'way' }, + { id: '-', type: 'way' } + ]}) ]); graph = iD.actionSplit('b', ['='])(graph); - - var ids = graph.entity('r').members.map(function(m) { return m.id; }); - expect(ids).to.have.ordered.members(['~', '-', '=']); + expect(members(graph)).to.eql(['~', '-', '=']); }); describe('member ordering', function () { it('adds the new way to parent relations (simple)', function () { - // Situation: - // a ----> b ----> c - // Relation: [----] // - // Split at b. + // Situation: + // a ---> b ---> c split at 'b' + // Relation: ['-'] // // Expected result: - // a ----> b ====> c - // Relation: [----, ====] + // a ---> b ===> c + // Relation: ['-', '='] // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmNode({id: 'c'}), - iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), - iD.osmRelation({id: 'r', members: [{id: '-', type: 'way', role: 'forward'}]}) + iD.osmNode({ id: 'a', loc: [0, 0] }), + iD.osmNode({ id: 'b', loc: [1, 0] }), + iD.osmNode({ id: 'c', loc: [2, 0] }), + iD.osmWay({ id: '-', nodes: ['a', 'b', 'c'] }), + iD.osmRelation({id: 'r', members: [ + { id: '-', type: 'way', role: 'forward' } + ]}) ]); graph = iD.actionSplit('b', ['='])(graph); expect(graph.entity('r').members).to.eql([ - {id: '-', type: 'way', role: 'forward'}, - {id: '=', type: 'way', role: 'forward'} + { id: '-', type: 'way', role: 'forward' }, + { id: '=', type: 'way', role: 'forward' } ]); }); it('adds the new way to parent relations (forward order)', function () { - // Situation: - // a ----> b ----> c ~~~~> d - // Relation: [----, ~~~~] // - // Split at b. + // Situation: + // a ---> b ---> c ~~~> d split at 'b' + // Relation: ['-', '~'] // // Expected result: - // a ----> b ====> c ~~~~> d - // Relation: [----, ====, ~~~~] + // a ---> b ===> c ~~~> d + // Relation: ['-', '=', '~'] // 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: ['c', 'd']}), - iD.osmRelation({id: 'r', members: [{id: '-', type: 'way'}, {id: '~', type: 'way'}]}) - ]); - - graph = iD.actionSplit('b', ['='])(graph); - - var ids = graph.entity('r').members.map(function(m) { return m.id; }); - expect(ids).to.have.ordered.members(['-', '=', '~']); - }); - - it('adds the new way to parent relations (reverse order)', function () { - // Situation: - // a ----> b ----> c ~~~~> d - // Relation: [~~~~, ----] - // - // Split at b. - // - // Expected result: - // a ----> b ====> c ~~~~> d - // Relation: [~~~~, ====, ----] - // - 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: ['c', 'd']}), - iD.osmRelation({id: 'r', members: [{id: '~', type: 'way'}, {id: '-', type: 'way'}]}) - ]); - - graph = iD.actionSplit('b', ['='])(graph); - - var ids = graph.entity('r').members.map(function(m) { return m.id; }); - expect(ids).to.have.ordered.members(['~', '=', '-']); - }); - - it('adds the new way to parent relations (unsplit way belongs multiple times)', function () { - // Situation: - // a ----> b ----> c ~~~~> d - // Relation: [~~~~, ----, ~~~~] - // - // Split at b. - // - // Expected result: - // a ----> b ====> c ~~~~> d - // Relation: [~~~~, ====, ----, ====, ~~~~] - // - 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: ['c', 'd']}), + 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', 'c'] }), + iD.osmWay({ id: '~', nodes: ['c', 'd'] }), iD.osmRelation({id: 'r', members: [ - {id: '~', type: 'way'}, - {id: '-', type: 'way'}, - {id: '~', type: 'way'} + { id: '-', type: 'way' }, + { id: '~', type: 'way' } ]}) ]); graph = iD.actionSplit('b', ['='])(graph); - - var ids = graph.entity('r').members.map(function(m) { return m.id; }); - expect(ids).to.have.ordered.members(['~', '=', '-', '=', '~']); + expect(members(graph)).to.eql(['-', '=', '~']); }); - it('adds the new way to parent relations (forward split way belongs multiple times)', function () { - // Situation: - // a ----> b ----> c ~~~~> d - // Relation: [----, ~~~~, ----] + it('adds the new way to parent relations (reverse order)', function () { // - // Split at b. + // Situation: + // a ---> b ---> c ~~~> d split at 'b' + // Relation: ['~', '-'] // // Expected result: - // a ----> b ====> c ~~~~> d - // Relation: [----, ====, ~~~~, ====, ----] + // a ---> b ===> c ~~~> d + // Relation: ['~', '=', '-'] // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmNode({id: 'c'}), - iD.osmNode({id: 'd'}), + 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', 'c'] }), + iD.osmWay({ id: '~', nodes: ['c', 'd'] }), + iD.osmRelation({id: 'r', members: [ + { id: '~', type: 'way' }, + { id: '-', type: 'way' } + ]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + expect(members(graph)).to.eql(['~', '=', '-']); + }); + }); + + describe('splitting out-and-back routes', function () { + var a = iD.osmNode({ id: 'a', loc: [0, 0] }); + var b = iD.osmNode({ id: 'b', loc: [0, 1] }); + var c = iD.osmNode({ id: 'c', loc: [0, 2] }); + var d = iD.osmNode({ id: 'd', loc: [0, 3] }); + + it('splits out-and-back1 route at b', function () { + // + // Situation: + // a ---> b ---> c ~~~> d split at 'b' + // Relation: ['-', '~', '~', '-'] + // + // Expected result: + // a ---> b ===> c ~~~> d + // Relation: ['-', '=', '~', '~', '=', '-'] + // + var graph = iD.coreGraph([ + a, b, c, d, iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), iD.osmWay({id: '~', nodes: ['c', 'd']}), iD.osmRelation({id: 'r', members: [ {id: '-', type: 'way'}, {id: '~', type: 'way'}, + {id: '~', type: 'way'}, {id: '-', type: 'way'} ]}) ]); - graph = iD.actionSplit('b', ['='])(graph); - var ids = graph.entity('r').members.map(function(m) { return m.id; }); - expect(ids).to.have.ordered.members(['-', '=', '~', '=', '-']); + expect(graph.entity('-').nodes).to.eql(['a', 'b']); + expect(graph.entity('=').nodes).to.eql(['b', 'c']); + expect(graph.entity('~').nodes).to.eql(['c', 'd']); + expect(members(graph)).to.eql(['-', '=', '~', '~', '=', '-']); }); - it('adds the new way to parent relations (reverse split way belongs multiple times)', function () { - // Situation: - // a <---- b <---- c ~~~~> d - // Relation: [----, ~~~~, ----] + it('splits out-and-back2 route at b', function () { // - // Split at b. + // Situation: + // a <--- b <--- c ~~~> d split at 'b' + // Relation: ['-', '~', '~', '-'] // // Expected result: - // a <==== b <---- c ~~~~> d - // Relation: [====, ----, ~~~~, ----, ====] + // a <=== b <--- c ~~~> d + // Relation: ['=', '-', '~', '~', '-', '='] // var graph = iD.coreGraph([ - iD.osmNode({id: 'a'}), - iD.osmNode({id: 'b'}), - iD.osmNode({id: 'c'}), - iD.osmNode({id: 'd'}), + a, b, c, d, iD.osmWay({id: '-', nodes: ['c', 'b', 'a']}), iD.osmWay({id: '~', nodes: ['c', 'd']}), iD.osmRelation({id: 'r', members: [ {id: '-', type: 'way'}, {id: '~', type: 'way'}, + {id: '~', type: 'way'}, {id: '-', type: 'way'} ]}) ]); - graph = iD.actionSplit('b', ['='])(graph); - var ids = graph.entity('r').members.map(function(m) { return m.id; }); - expect(ids).to.have.ordered.members(['=', '-', '~', '-', '=']); + expect(graph.entity('-').nodes).to.eql(['c', 'b']); + expect(graph.entity('=').nodes).to.eql(['b', 'a']); + expect(graph.entity('~').nodes).to.eql(['c', 'd']); + expect(members(graph)).to.eql(['=', '-', '~', '~', '-', '=']); }); + + it('splits out-and-back3 route at b', function () { + // + // Situation: + // a ---> b ---> c <~~~ d split at 'b' + // Relation: ['-', '~', '~', '-'] + // + // Expected result: + // a ---> b ===> c <~~~ d + // Relation: ['-', '=', '~', '~', '=', '-'] + // + var graph = iD.coreGraph([ + a, b, c, d, + iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), + iD.osmWay({id: '~', nodes: ['d', 'c']}), + iD.osmRelation({id: 'r', members: [ + {id: '-', type: 'way'}, + {id: '~', type: 'way'}, + {id: '~', type: 'way'}, + {id: '-', type: 'way'} + ]}) + ]); + graph = iD.actionSplit('b', ['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['a', 'b']); + expect(graph.entity('=').nodes).to.eql(['b', 'c']); + expect(graph.entity('~').nodes).to.eql(['d', 'c']); + expect(members(graph)).to.eql(['-', '=', '~', '~', '=', '-']); + }); + + it('splits out-and-back4 route at b', function () { + // + // Situation: + // a <--- b <--- c <~~~ d split at 'b' + // Relation: ['-', '~', '~', '-'] + // + // Expected result: + // a <=== b <--- c <~~~ d + // Relation: ['=', '-', '~', '~', '-', '='] + // + var graph = iD.coreGraph([ + a, b, c, d, + iD.osmWay({id: '-', nodes: ['c', 'b', 'a']}), + iD.osmWay({id: '~', nodes: ['d', 'c']}), + iD.osmRelation({id: 'r', members: [ + {id: '-', type: 'way'}, + {id: '~', type: 'way'}, + {id: '~', type: 'way'}, + {id: '-', type: 'way'} + ]}) + ]); + graph = iD.actionSplit('b', ['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['c', 'b']); + expect(graph.entity('=').nodes).to.eql(['b', 'a']); + expect(graph.entity('~').nodes).to.eql(['d', 'c']); + expect(members(graph)).to.eql(['=', '-', '~', '~', '-', '=']); + }); + }); + + + describe('splitting spoon routes', function () { + var a = iD.osmNode({ id: 'a', loc: [0, 0] }); + var b = iD.osmNode({ id: 'b', loc: [0, 1] }); + var c = iD.osmNode({ id: 'c', loc: [1, 1] }); + var d = iD.osmNode({ id: 'd', loc: [1, 0] }); + var e = iD.osmNode({ id: 'e', loc: [2, 0] }); + var f = iD.osmNode({ id: 'f', loc: [3, 0] }); + + // + // Situation: + // b --> c + // | | + // a <-- d ~~~> e ~~~> f + // + // Relation: ['~', '-', '~'] + // + var spoon1 = iD.coreGraph([ + a, b, c, d, e, f, + iD.osmWay({id: '-', nodes: ['d', 'a', 'b', 'c', 'd']}), + iD.osmWay({id: '~', nodes: ['d', 'e', 'f']}), + iD.osmRelation({id: 'r', members: [ + {id: '~', type: 'way'}, + {id: '-', type: 'way'}, + {id: '~', type: 'way'} + ]}) + ]); + + // + // Situation: + // b <-- c + // | | + // a --> d ~~~> e ~~~> f + // + // Relation: ['~', '-', '~'] + // + var spoon2 = iD.coreGraph([ + a, b, c, d, e, f, + iD.osmWay({id: '-', nodes: ['d', 'c', 'b', 'a', 'd']}), + iD.osmWay({id: '~', nodes: ['d', 'e', 'f']}), + iD.osmRelation({id: 'r', members: [ + {id: '~', type: 'way'}, + {id: '-', type: 'way'}, + {id: '~', type: 'way'} + ]}) + ]); + + // + // Situation: + // b --> c + // | | + // a <-- d <~~~ e <~~~ f + // + // Relation: ['~', '-', '~'] + // + var spoon3 = iD.coreGraph([ + a, b, c, d, e, f, + iD.osmWay({id: '-', nodes: ['d', 'a', 'b', 'c', 'd']}), + iD.osmWay({id: '~', nodes: ['f', 'e', 'd']}), + iD.osmRelation({id: 'r', members: [ + {id: '~', type: 'way'}, + {id: '-', type: 'way'}, + {id: '~', type: 'way'} + ]}) + ]); + + // + // Situation: + // b <-- c + // | | + // a --> d <~~~ e <~~~ f + // + // Relation: ['~', '-', '~'] + // + var spoon4 = iD.coreGraph([ + a, b, c, d, e, f, + iD.osmWay({id: '-', nodes: ['d', 'c', 'b', 'a', 'd']}), + iD.osmWay({id: '~', nodes: ['f', 'e', 'd']}), + iD.osmRelation({id: 'r', members: [ + {id: '~', type: 'way'}, + {id: '-', type: 'way'}, + {id: '~', type: 'way'} + ]}) + ]); + + it('splits spoon1 route at d', function () { + // + // Expected result: + // b ==> c + // | ‖ + // a <-- d ~~~> e ~~~> f + // + // Relation: ['~', '-', '=', '~'] + // + var graph = spoon1; + graph = iD.actionSplit('d', ['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['d', 'a', 'b']); + expect(graph.entity('=').nodes).to.eql(['b', 'c', 'd']); + expect(graph.entity('~').nodes).to.eql(['d', 'e', 'f']); + expect(members(graph)).to.eql(['~', '-', '=', '~']); + }); + + it('splits spoon2 route at d', function () { + // + // Expected result: + // b <-- c + // ‖ | + // a ==> d ~~~> e ~~~> f + // + // Relation: ['~', '-', '=', '~'] + // + var graph = spoon2; + graph = iD.actionSplit('d', ['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['d', 'c', 'b']); + expect(graph.entity('=').nodes).to.eql(['b', 'a', 'd']); + expect(graph.entity('~').nodes).to.eql(['d', 'e', 'f']); + expect(members(graph)).to.eql(['~', '-', '=', '~']); + }); + + it('splits spoon3 route at d', function () { + // + // Expected result: + // b ==> c + // | ‖ + // a <-- d <~~~ e <~~~ f + // + // Relation: ['~', '-', '=', '~'] + // + var graph = spoon3; + graph = iD.actionSplit('d', ['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['d', 'a', 'b']); + expect(graph.entity('=').nodes).to.eql(['b', 'c', 'd']); + expect(graph.entity('~').nodes).to.eql(['f', 'e', 'd']); + expect(members(graph)).to.eql(['~', '-', '=', '~']); + }); + + it('splits spoon4 route at d', function () { + // + // Expected result: + // b <-- c + // ‖ | + // a ==> d <~~~ e <~~~ f + // + // Relation: ['~', '-', '=', '~'] + // + var graph = spoon4; + graph = iD.actionSplit('d', ['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['d', 'c', 'b']); + expect(graph.entity('=').nodes).to.eql(['b', 'a', 'd']); + expect(graph.entity('~').nodes).to.eql(['f', 'e', 'd']); + expect(members(graph)).to.eql(['~', '-', '=', '~']); + }); + + it('splits spoon1 route at e', function () { + // + // Expected result: + // b --> c + // | | + // a <-- d ~~~> e ===> f + // + // Relation: ['=', '~', '-', '~', '='] + // + var graph = spoon1; + graph = iD.actionSplit('e', ['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['d', 'a', 'b', 'c', 'd']); + expect(graph.entity('~').nodes).to.eql(['d', 'e']); + expect(graph.entity('=').nodes).to.eql(['e', 'f']); + expect(members(graph)).to.eql(['=', '~', '-', '~', '=']); + }); + + it('splits spoon2 route at e', function () { + // + // Expected result: + // b <-- c + // | | + // a --> d ~~~> e ===> f + // + // Relation: ['=', '~', '-', '~', '='] + // + var graph = spoon2; + graph = iD.actionSplit('e', ['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['d', 'c', 'b', 'a', 'd']); + expect(graph.entity('~').nodes).to.eql(['d', 'e']); + expect(graph.entity('=').nodes).to.eql(['e', 'f']); + expect(members(graph)).to.eql(['=', '~', '-', '~', '=']); + }); + + it('splits spoon3 route at e', function () { + // + // Expected result: + // b --> c + // | | + // a <-- d <=== e <~~~ f + // + // Relation: ['~', '=', '-', '=', '~'] + // + var graph = spoon3; + graph = iD.actionSplit('e', ['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['d', 'a', 'b', 'c', 'd']); + expect(graph.entity('~').nodes).to.eql(['f', 'e']); + expect(graph.entity('=').nodes).to.eql(['e', 'd']); + expect(members(graph)).to.eql(['~', '=', '-', '=', '~']); + }); + + it('splits spoon4 route at e', function () { + // + // Expected result: + // b <-- c + // | | + // a --> d <=== e <~~~ f + // + // Relation: ['~', '=', '-', '=', '~'] + // + var graph = spoon4; + graph = iD.actionSplit('e', ['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['d', 'c', 'b', 'a', 'd']); + expect(graph.entity('~').nodes).to.eql(['f', 'e']); + expect(graph.entity('=').nodes).to.eql(['e', 'd']); + expect(members(graph)).to.eql(['~', '=', '-', '=', '~']); + }); + }); From be9bbd927133aeee7254add81b541f2d9b25eb4d Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 18 Jan 2018 15:21:38 -0500 Subject: [PATCH 14/15] Add tests for member ordering: node, way, relation --- modules/actions/add_member.js | 2 +- test/spec/actions/add_member.js | 24 ++++++++++++++++++++++++ test/spec/actions/split.js | 25 +++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/modules/actions/add_member.js b/modules/actions/add_member.js index 67f1d8b04..a9dfa7ec6 100644 --- a/modules/actions/add_member.js +++ b/modules/actions/add_member.js @@ -38,7 +38,7 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) { // so that `osmJoinWays` can treat the pair like a single way. tempWay = osmWay({ id: 'wTemp', nodes: insertPair.nodes }); graph = graph.replace(tempWay); - var tempMember = { id: tempWay.id, type: 'way', role: '' }; + var tempMember = { id: tempWay.id, type: 'way', role: member.role }; var tempRelation = replaceMemberAll(relation, insertPair.originalID, tempMember); groups = _groupBy(tempRelation.members, function(m) { return m.type; }); groups.way = groups.way || []; diff --git a/test/spec/actions/add_member.js b/test/spec/actions/add_member.js index b2a8486ec..e27ef07e7 100644 --- a/test/spec/actions/add_member.js +++ b/test/spec/actions/add_member.js @@ -177,6 +177,30 @@ describe('iD.actionAddMember', function() { expect(members(graph)).to.eql(['-', '=', '~', '~', '=', '-']); }); + it('reorders members as node, way, relation (for Public Transport routing)', function() { + var graph = iD.coreGraph([ + iD.osmNode({id: 'a', loc: [0, 0]}), + iD.osmNode({id: 'b', loc: [0, 0]}), + iD.osmNode({id: 'c', loc: [0, 0]}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['b', 'c']}), + iD.osmRelation({id: 'r', members: [ + { id: 'n1', type: 'node', role: 'forward' }, + { id: '-', type: 'way', role: 'forward' }, + { id: 'r1', type: 'relation', role: 'forward' }, + { id: 'n2', type: 'node', role: 'forward' } + ]}) + ]); + + graph = iD.actionAddMember('r', { id: '=', type: 'way', role: 'forward' })(graph); + expect(graph.entity('r').members).to.eql([ + { id: 'n1', type: 'node', role: 'forward' }, + { id: 'n2', type: 'node', role: 'forward' }, + { id: '-', type: 'way', role: 'forward' }, + { id: '=', type: 'way', role: 'forward' }, + { id: 'r1', type: 'relation', role: 'forward' } + ]); + }); }); }); diff --git a/test/spec/actions/split.js b/test/spec/actions/split.js index febf47d72..8e536a754 100644 --- a/test/spec/actions/split.js +++ b/test/spec/actions/split.js @@ -446,6 +446,31 @@ describe('iD.actionSplit', function () { graph = iD.actionSplit('b', ['='])(graph); expect(members(graph)).to.eql(['~', '=', '-']); }); + + it('reorders members as node, way, relation (for Public Transport routing)', function () { + 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.osmWay({ id: '-', nodes: ['a', 'b', 'c'] }), + iD.osmRelation({id: 'r', members: [ + { id: 'n1', type: 'node', role: 'forward' }, + { id: '-', type: 'way', role: 'forward' }, + { id: 'r1', type: 'relation', role: 'forward' }, + { id: 'n2', type: 'node', role: 'forward' } + ]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + expect(graph.entity('r').members).to.eql([ + { id: 'n1', type: 'node', role: 'forward' }, + { id: 'n2', type: 'node', role: 'forward' }, + { id: '-', type: 'way', role: 'forward' }, + { id: '=', type: 'way', role: 'forward' }, + { id: 'r1', type: 'relation', role: 'forward'} + ]); + }); }); describe('splitting out-and-back routes', function () { From 7c918ba16162fbdb55a8fadffc15dffad941332c Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 18 Jan 2018 16:52:23 -0500 Subject: [PATCH 15/15] 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 a9dfa7ec6..049175329 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 052627860..b44dbd420 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 8b1378187..e7de473f3 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 c5135ab25..4d449d226 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'}]); }); });