From 416ae66f7121c6293ad3d196ac90782a0f9d9d34 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 5 Jul 2018 14:47:56 -0400 Subject: [PATCH] Support more direction tags for reversal in Reverse action (closes #5121) This commit also refactors some of the logic to make it simpler and more efficient (removes lodash _transform too) and updates the comment that was out of date. --- modules/actions/reverse.js | 175 ++++---- test/spec/actions/reverse.js | 752 ++++++++++++++++++++--------------- 2 files changed, 518 insertions(+), 409 deletions(-) diff --git a/modules/actions/reverse.js b/modules/actions/reverse.js index 3f92fcd91c..d1181c5fce 100644 --- a/modules/actions/reverse.js +++ b/modules/actions/reverse.js @@ -1,67 +1,61 @@ -import _transform from 'lodash-es/transform'; - - /* - Order the nodes of a way in reverse order and reverse any direction dependent tags - other than `oneway`. (We assume that correcting a backwards oneway is the primary - reason for reversing a way.) - - The following transforms are performed: - - Keys: - *:right=* ⟺ *:left=* - *:forward=* ⟺ *:backward=* - direction=up ⟺ direction=down - incline=up ⟺ incline=down - *=right ⟺ *=left - - Relation members: - role=forward ⟺ role=backward - role=north ⟺ role=south - role=east ⟺ role=west - - In addition, numeric-valued `incline` tags are negated. - - The JOSM implementation was used as a guide, but transformations that were of unclear benefit - or adjusted tags that don't seem to be used in practice were omitted. - - Also, each node on the way is examined for its own tags and the following transformations are performed - in order to ensure associated nodes (eg a Stop Sign) is also reversed - - Node Keys: - *direction=forward ⟺ *direction=backward - *direction=left ⟺ *direction=right - *:forward=* ⟺ *:backward=* - *:left=* ⟺ *:right=* - - References: - http://wiki.openstreetmap.org/wiki/Forward_%26_backward,_left_%26_right - http://wiki.openstreetmap.org/wiki/Key:direction#Steps - http://wiki.openstreetmap.org/wiki/Key:incline - http://wiki.openstreetmap.org/wiki/Route#Members - http://josm.openstreetmap.de/browser/josm/trunk/src/org/openstreetmap/josm/corrector/ReverseWayTagCorrector.java - http://wiki.openstreetmap.org/wiki/Tag:highway%3Dstop - http://wiki.openstreetmap.org/wiki/Key:traffic_sign#On_a_way_or_area - */ -export function actionReverse(wayId, options) { - var replacements = [ - [/:right$/, ':left'], [/:left$/, ':right'], - [/:forward$/, ':backward'], [/:backward$/, ':forward'] - ], - numeric = /^([+\-]?)(?=[\d.])/, - roleReversals = { - forward: 'backward', - backward: 'forward', - north: 'south', - south: 'north', - east: 'west', - west: 'east' - }; +Order the nodes of a way in reverse order and reverse any direction dependent tags +other than `oneway`. (We assume that correcting a backwards oneway is the primary +reason for reversing a way.) + +In addition, numeric-valued `incline` tags are negated. + +The JOSM implementation was used as a guide, but transformations that were of unclear benefit +or adjusted tags that don't seem to be used in practice were omitted. + +References: + http://wiki.openstreetmap.org/wiki/Forward_%26_backward,_left_%26_right + http://wiki.openstreetmap.org/wiki/Key:direction#Steps + http://wiki.openstreetmap.org/wiki/Key:incline + http://wiki.openstreetmap.org/wiki/Route#Members + http://josm.openstreetmap.de/browser/josm/trunk/src/org/openstreetmap/josm/corrector/ReverseWayTagCorrector.java + http://wiki.openstreetmap.org/wiki/Tag:highway%3Dstop + http://wiki.openstreetmap.org/wiki/Key:traffic_sign#On_a_way_or_area +*/ +export function actionReverse(wayID, options) { + var ignoreKey = /^.*(_|:)?(description|name|note|website|ref|source|comment|watch|attribution)(_|:)?/; + var numeric = /^([+\-]?)(?=[\d.])/; + var keyReplacements = [ + [/:right$/, ':left'], + [/:left$/, ':right'], + [/:forward$/, ':backward'], + [/:backward$/, ':forward'] + ]; + var valueReplacements = { + left: 'right', + right: 'left', + up: 'down', + down: 'up', + forward: 'backward', + backward: 'forward', + forwards: 'backward', + backwards: 'forward', + }; + var roleReplacements = { + forward: 'backward', + backward: 'forward', + forwards: 'backward', + backwards: 'forward', + north: 'south', + south: 'north', + east: 'west', + west: 'east' + }; + var onewayReplacements = { + yes: '-1', + '1': '-1', + '-1': 'yes' + }; function reverseKey(key) { - for (var i = 0; i < replacements.length; ++i) { - var replacement = replacements[i]; + for (var i = 0; i < keyReplacements.length; ++i) { + var replacement = keyReplacements[i]; if (replacement[0].test(key)) { return key.replace(replacement[0], replacement[1]); } @@ -71,61 +65,47 @@ export function actionReverse(wayId, options) { function reverseValue(key, value) { + if (ignoreKey.test(key)) return value; + if (key === 'incline' && numeric.test(value)) { return value.replace(numeric, function(_, sign) { return sign === '-' ? '' : '-'; }); - } else if (key === 'incline' || key === 'direction') { - return {up: 'down', down: 'up'}[value] || value; } else if (options && options.reverseOneway && key === 'oneway') { - return {yes: '-1', '1': '-1', '-1': 'yes'}[value] || value; + return onewayReplacements[value] || value; } else { - return {left: 'right', right: 'left'}[value] || value; + return valueReplacements[value] || value; } } - function reverseDirectionTags(node) { - // Update the direction based tags as appropriate then return an updated node - return node.update({tags: _transform(node.tags, function(acc, tagValue, tagKey) { - // See if this is a direction tag and reverse (or use existing value if not recognised) - var re = /direction$/; - if (re.test(tagKey)) { - acc[tagKey] = {forward: 'backward', backward: 'forward', left: 'right', right: 'left'}[tagValue] || tagValue; - } else { - // Use the reverseKey method to cater for situations such as traffic_sign:forward=stop - // This will pass through other tags unchanged - acc[reverseKey(tagKey)] = tagValue; - } - return acc; - }, {})}); - } - + // Reverse the direction of tags attached to the nodes - #3076 + function reverseNodeTags(graph, nodeIDs) { + for (var i = 0; i < nodeIDs.length; i++) { + var node = graph.hasEntity(nodeIDs[i]); + if (!node || !Object.keys(node.tags).length) continue; - function reverseTagsOnNodes(graph, nodeIds) { - // Reverse the direction of appropriate tags attached to the nodes (#3076) - return nodeIds - // Get each node from the graph - .map(function(nodeId) { return graph.entity(nodeId);}) - // Check tags on the node, if there aren't any, we can skip - .filter(function(existingNode) { return existingNode.tags !== undefined;}) - // Get a new version of each node with the appropriate tags reversed - .map(function(existingNode) { return reverseDirectionTags(existingNode);}) - // Chain together consecutive updates to the graph for each updated node and return - .reduce(function (accGraph, value) { return accGraph.replace(value); }, graph); + var tags = {}; + for (var key in node.tags) { + tags[reverseKey(key)] = reverseValue(key, node.tags[key]); + } + graph = graph.replace(node.update({tags: tags})); + } + return graph; } return function(graph) { - var way = graph.entity(wayId), - nodes = way.nodes.slice().reverse(), - tags = {}, key, role; + var way = graph.entity(wayID); + var nodes = way.nodes.slice().reverse(); + var tags = {}; + var role; - for (key in way.tags) { + for (var key in way.tags) { tags[reverseKey(key)] = reverseValue(key, way.tags[key]); } graph.parentRelations(way).forEach(function(relation) { relation.members.forEach(function(member, index) { - if (member.id === way.id && (role = roleReversals[member.role])) { + if (member.id === way.id && (role = roleReplacements[member.role])) { relation = relation.updateMember({role: role}, index); graph = graph.replace(relation); } @@ -134,6 +114,7 @@ export function actionReverse(wayId, options) { // Reverse any associated directions on nodes on the way and then replace // the way itself with the reversed node ids and updated way tags - return reverseTagsOnNodes(graph, nodes).replace(way.update({nodes: nodes, tags: tags})); + return reverseNodeTags(graph, nodes) + .replace(way.update({nodes: nodes, tags: tags})); }; } diff --git a/test/spec/actions/reverse.js b/test/spec/actions/reverse.js index 7e99b9a440..3481790052 100644 --- a/test/spec/actions/reverse.js +++ b/test/spec/actions/reverse.js @@ -1,375 +1,503 @@ describe('iD.actionReverse', function () { it('reverses the order of nodes in the way', function () { - var node1 = iD.Node(), - node2 = iD.Node(), - way = iD.Way({nodes: [node1.id, node2.id]}), - graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, way])); + var node1 = iD.osmNode(); + var node2 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, way])); expect(graph.entity(way.id).nodes).to.eql([node2.id, node1.id]); }); it('preserves non-directional tags', function () { - var way = iD.Way({tags: {'highway': 'residential'}}), - graph = iD.Graph([way]); + var way = iD.osmWay({tags: {'highway': 'residential'}}); + var graph = iD.coreGraph([way]); graph = iD.actionReverse(way.id)(graph); expect(graph.entity(way.id).tags).to.eql({'highway': 'residential'}); }); - it('preserves oneway tags', function () { - var way = iD.Way({tags: {'oneway': 'yes'}}), - graph = iD.Graph([way]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'oneway': 'yes'}); - }); + describe('reverses oneway', function () { + it('preserves oneway tags', function () { + var way = iD.osmWay({tags: {'oneway': 'yes'}}); + var graph = iD.coreGraph([way]); - it('reverses oneway tags if reverseOneway: true is provided', function () { - var graph = iD.Graph([ - iD.Way({id: 'yes', tags: {oneway: 'yes'}}), - iD.Way({id: 'no', tags: {oneway: 'no'}}), - iD.Way({id: '1', tags: {oneway: '1'}}), - iD.Way({id: '-1', tags: {oneway: '-1'}}) - ]); - - expect(iD.actionReverse('yes', {reverseOneway: true})(graph) - .entity('yes').tags).to.eql({oneway: '-1'}); - expect(iD.actionReverse('no', {reverseOneway: true})(graph) - .entity('no').tags).to.eql({oneway: 'no'}); - expect(iD.actionReverse('1', {reverseOneway: true})(graph) - .entity('1').tags).to.eql({oneway: '-1'}); - expect(iD.actionReverse('-1', {reverseOneway: true})(graph) - .entity('-1').tags).to.eql({oneway: 'yes'}); - }); + graph = iD.actionReverse(way.id)(graph); + expect(graph.entity(way.id).tags).to.eql({'oneway': 'yes'}); + }); - it('transforms *:right=* ⟺ *:left=*', function () { - var way = iD.Way({tags: {'cycleway:right': 'lane'}}), - graph = iD.Graph([way]); + it('reverses oneway tags if reverseOneway: true is provided', function () { + var graph = iD.coreGraph([ + iD.osmWay({id: 'yes', tags: {oneway: 'yes'}}), + iD.osmWay({id: 'no', tags: {oneway: 'no'}}), + iD.osmWay({id: '1', tags: {oneway: '1'}}), + iD.osmWay({id: '-1', tags: {oneway: '-1'}}) + ]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'cycleway:left': 'lane'}); + expect(iD.actionReverse('yes', {reverseOneway: true})(graph) + .entity('yes').tags).to.eql({oneway: '-1'}, 'yes'); + expect(iD.actionReverse('no', {reverseOneway: true})(graph) + .entity('no').tags).to.eql({oneway: 'no'}, 'no'); + expect(iD.actionReverse('1', {reverseOneway: true})(graph) + .entity('1').tags).to.eql({oneway: '-1'}, '1'); + expect(iD.actionReverse('-1', {reverseOneway: true})(graph) + .entity('-1').tags).to.eql({oneway: 'yes'}, '-1'); + }); + + it('ignores other oneway tags', function () { + var graph = iD.coreGraph([ + iD.osmWay({id: 'alternating', tags: {oneway: 'alternating'}}), + iD.osmWay({id: 'reversible', tags: {oneway: 'reversible'}}), + iD.osmWay({id: 'dummy', tags: {oneway: 'dummy'}}) + ]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'cycleway:right': 'lane'}); + expect(iD.actionReverse('alternating', {reverseOneway: true})(graph) + .entity('alternating').tags).to.eql({oneway: 'alternating'}, 'alternating'); + expect(iD.actionReverse('reversible', {reverseOneway: true})(graph) + .entity('reversible').tags).to.eql({oneway: 'reversible'}, 'reversible'); + expect(iD.actionReverse('dummy', {reverseOneway: true})(graph) + .entity('dummy').tags).to.eql({oneway: 'dummy'}, 'dummy'); + }); }); - it('transforms *:forward=* ⟺ *:backward=*', function () { - var way = iD.Way({tags: {'maxspeed:forward': '25'}}), - graph = iD.Graph([way]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'maxspeed:backward': '25'}); + describe('reverses incline', function () { + it('transforms incline=up ⟺ incline=down', function () { + var way = iD.osmWay({tags: {'incline': 'up'}}); + var graph = iD.coreGraph([way]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'maxspeed:forward': '25'}); - }); + graph = iD.actionReverse(way.id)(graph); + expect(graph.entity(way.id).tags).to.eql({'incline': 'down'}); - it('transforms direction=up ⟺ direction=down', function () { - var way = iD.Way({tags: {'incline': 'up'}}), - graph = iD.Graph([way]); + graph = iD.actionReverse(way.id)(graph); + expect(graph.entity(way.id).tags).to.eql({'incline': 'up'}); + }); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'incline': 'down'}); + it('negates numeric-valued incline tags', function () { + var way = iD.osmWay({tags: {'incline': '5%'}}); + var graph = iD.coreGraph([way]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'incline': 'up'}); - }); + graph = iD.actionReverse(way.id)(graph); + expect(graph.entity(way.id).tags).to.eql({'incline': '-5%'}); - it('transforms incline=up ⟺ incline=down', function () { - var way = iD.Way({tags: {'incline': 'up'}}), - graph = iD.Graph([way]); + graph = iD.actionReverse(way.id)(graph); + expect(graph.entity(way.id).tags).to.eql({'incline': '5%'}); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'incline': 'down'}); + way = iD.osmWay({tags: {'incline': '.8°'}}); + graph = iD.coreGraph([way]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'incline': 'up'}); + graph = iD.actionReverse(way.id)(graph); + expect(graph.entity(way.id).tags).to.eql({'incline': '-.8°'}); + }); }); - it('negates numeric-valued incline tags', function () { - var way = iD.Way({tags: {'incline': '5%'}}), - graph = iD.Graph([way]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'incline': '-5%'}); - - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'incline': '5%'}); - - way = iD.Way({tags: {'incline': '.8°'}}); - graph = iD.Graph([way]); - - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'incline': '-.8°'}); - }); - - it('transforms *=right ⟺ *=left', function () { - var way = iD.Way({tags: {'sidewalk': 'right'}}), - graph = iD.Graph([way]); + describe('reverses directional keys on ways', function () { + it('transforms *:right=* ⟺ *:left=*', function () { + var way = iD.osmWay({tags: {'cycleway:right': 'lane'}}); + var graph = iD.coreGraph([way]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'sidewalk': 'left'}); + graph = iD.actionReverse(way.id)(graph); + expect(graph.entity(way.id).tags).to.eql({'cycleway:left': 'lane'}); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'sidewalk': 'right'}); - }); + graph = iD.actionReverse(way.id)(graph); + expect(graph.entity(way.id).tags).to.eql({'cycleway:right': 'lane'}); + }); - it('transforms multiple directional tags', function () { - var way = iD.Way({tags: {'maxspeed:forward': '25', 'maxspeed:backward': '30'}}), - graph = iD.Graph([way]); + it('transforms *:forward=* ⟺ *:backward=*', function () { + var way = iD.osmWay({tags: {'maxspeed:forward': '25'}}); + var graph = iD.coreGraph([way]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(way.id).tags).to.eql({'maxspeed:backward': '25', 'maxspeed:forward': '30'}); - }); + graph = iD.actionReverse(way.id)(graph); + expect(graph.entity(way.id).tags).to.eql({'maxspeed:backward': '25'}); - it('transforms role=forward ⟺ role=backward in member relations', function () { - var way = iD.Way({tags: {highway: 'residential'}}), - relation = iD.Relation({members: [{type: 'way', id: way.id, role: 'forward'}]}), - graph = iD.Graph([way, relation]); + graph = iD.actionReverse(way.id)(graph); + expect(graph.entity(way.id).tags).to.eql({'maxspeed:forward': '25'}); + }); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(relation.id).members[0].role).to.eql('backward'); + it('transforms multiple directional tags', function () { + var way = iD.osmWay({tags: {'maxspeed:forward': '25', 'maxspeed:backward': '30'}}); + var graph = iD.coreGraph([way]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(relation.id).members[0].role).to.eql('forward'); + graph = iD.actionReverse(way.id)(graph); + expect(graph.entity(way.id).tags).to.eql({'maxspeed:backward': '25', 'maxspeed:forward': '30'}); + }); }); - it('transforms role=north ⟺ role=south in member relations', function () { - var way = iD.Way({tags: {highway: 'residential'}}), - relation = iD.Relation({members: [{type: 'way', id: way.id, role: 'north'}]}), - graph = iD.Graph([way, relation]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(relation.id).members[0].role).to.eql('south'); - - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(relation.id).members[0].role).to.eql('north'); - }); - - it('transforms role=east ⟺ role=west in member relations', function () { - var way = iD.Way({tags: {highway: 'residential'}}), - relation = iD.Relation({members: [{type: 'way', id: way.id, role: 'east'}]}), - graph = iD.Graph([way, relation]); + describe('reverses directional values on ways', function () { + it('transforms *=up ⟺ *=down', function () { + var graph = iD.coreGraph([ + iD.osmWay({id: 'inclineU', tags: {incline: 'up'}}), + iD.osmWay({id: 'directionU', tags: {direction: 'up'}}), + iD.osmWay({id: 'inclineD', tags: {incline: 'down'}}), + iD.osmWay({id: 'directionD', tags: {direction: 'down'}}) + ]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(relation.id).members[0].role).to.eql('west'); + expect(iD.actionReverse('inclineU')(graph) + .entity('inclineU').tags).to.eql({incline: 'down'}, 'inclineU'); + expect(iD.actionReverse('directionU')(graph) + .entity('directionU').tags).to.eql({direction: 'down'}, 'directionU'); + + expect(iD.actionReverse('inclineD')(graph) + .entity('inclineD').tags).to.eql({incline: 'up'}, 'inclineD'); + expect(iD.actionReverse('directionD')(graph) + .entity('directionD').tags).to.eql({direction: 'up'}, 'directionD'); + }); + + it('skips *=up ⟺ *=down for ignored tags', function () { + var graph = iD.coreGraph([ + iD.osmWay({id: 'name', tags: {name: 'up'}}), + iD.osmWay({id: 'note', tags: {note: 'up'}}), + iD.osmWay({id: 'ref', tags: {ref: 'down'}}), + iD.osmWay({id: 'description', tags: {description: 'down'}}) + ]); - graph = iD.actionReverse(way.id)(graph); - expect(graph.entity(relation.id).members[0].role).to.eql('east'); - }); + expect(iD.actionReverse('name')(graph) + .entity('name').tags).to.eql({name: 'up'}, 'name'); + expect(iD.actionReverse('note')(graph) + .entity('note').tags).to.eql({note: 'up'}, 'note'); + expect(iD.actionReverse('ref')(graph) + .entity('ref').tags).to.eql({ref: 'down'}, 'ref'); + expect(iD.actionReverse('description')(graph) + .entity('description').tags).to.eql({description: 'down'}, 'description'); + }); + + it('transforms *=forward ⟺ *=backward', function () { + var graph = iD.coreGraph([ + iD.osmWay({id: 'conveyingF', tags: {conveying: 'forward'}}), + iD.osmWay({id: 'directionF', tags: {direction: 'forward'}}), + iD.osmWay({id: 'priorityF', tags: {priority: 'forward'}}), + iD.osmWay({id: 'trolley_wireF', tags: {trolley_wire: 'forward'}}), + iD.osmWay({id: 'conveyingB', tags: {conveying: 'backward'}}), + iD.osmWay({id: 'directionB', tags: {direction: 'backward'}}), + iD.osmWay({id: 'priorityB', tags: {priority: 'backward'}}), + iD.osmWay({id: 'trolley_wireB', tags: {trolley_wire: 'backward'}}) + ]); - // For issue #3076 - it('reverses the direction of a forward facing stop sign on the way', function () { - var node1 = iD.Node(); - var node2 = iD.Node(); - var node3 = iD.Node(); - // Attach a forward facing stop sign to node 2 - node2.tags = { 'direction': 'forward', 'highway': 'stop' }; - // Create our way - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - // Act - reverse the way - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - // Assert - confirm that the stop sign on node 2 has changed direction - var target = graph.entity(node2.id); - expect(target.tags.direction).to.eql('backward'); - }); + expect(iD.actionReverse('conveyingF')(graph) + .entity('conveyingF').tags).to.eql({conveying: 'backward'}, 'conveyingF'); + expect(iD.actionReverse('directionF')(graph) + .entity('directionF').tags).to.eql({direction: 'backward'}, 'directionF'); + expect(iD.actionReverse('priorityF')(graph) + .entity('priorityF').tags).to.eql({priority: 'backward'}, 'priorityF'); + expect(iD.actionReverse('trolley_wireF')(graph) + .entity('trolley_wireF').tags).to.eql({trolley_wire: 'backward'}, 'trolley_wireF'); + + expect(iD.actionReverse('conveyingB')(graph) + .entity('conveyingB').tags).to.eql({conveying: 'forward'}, 'conveyingB'); + expect(iD.actionReverse('directionB')(graph) + .entity('directionB').tags).to.eql({direction: 'forward'}, 'directionB'); + expect(iD.actionReverse('priorityB')(graph) + .entity('priorityB').tags).to.eql({priority: 'forward'}, 'priorityB'); + expect(iD.actionReverse('trolley_wireB')(graph) + .entity('trolley_wireB').tags).to.eql({trolley_wire: 'forward'}, 'trolley_wireB'); + }); + + it('drops "s" from forwards/backwards when reversing', function () { + var graph = iD.coreGraph([ + iD.osmWay({id: 'conveyingF', tags: {conveying: 'forwards'}}), + iD.osmWay({id: 'conveyingB', tags: {conveying: 'backwards'}}) + ]); - it('reverses the direction of a backward facing stop sign on the way', function () { - var node1 = iD.Node(); - var node2 = iD.Node(); - var node3 = iD.Node(); - // Attach a backward facing stop sign to node 2 - node2.tags = { 'direction': 'backward', 'highway': 'stop' }; - // Create our way - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - // Act - reverse the way - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - // Assert - confirm that the stop sign on node 2 has changed direction - var target = graph.entity(node2.id); - expect(target.tags.direction).to.eql('forward'); - }); + expect(iD.actionReverse('conveyingF')(graph) + .entity('conveyingF').tags).to.eql({conveying: 'backward'}, 'conveyingF'); + expect(iD.actionReverse('conveyingB')(graph) + .entity('conveyingB').tags).to.eql({conveying: 'forward'}, 'conveyingB'); + }); + + it('skips *=forward ⟺ *=backward for ignored tags', function () { + var graph = iD.coreGraph([ + iD.osmWay({id: 'name', tags: {name: 'forward'}}), + iD.osmWay({id: 'note', tags: {note: 'forwards'}}), + iD.osmWay({id: 'ref', tags: {ref: 'backward'}}), + iD.osmWay({id: 'description', tags: {description: 'backwards'}}) + ]); - it('reverses the direction of a left facing stop sign on the way', function () { - var node1 = iD.Node(); - var node2 = iD.Node(); - var node3 = iD.Node(); - // Attach a left facing stop sign to node 2 (not sure this is a real situation, - // but allows us to test) - node2.tags = { 'direction': 'left', 'highway': 'stop' }; - // Create our way - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - // Act - reverse the way - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - // Assert - confirm that the stop sign on node 2 has changed direction - var target = graph.entity(node2.id); - expect(target.tags.direction).to.eql('right'); - }); + expect(iD.actionReverse('name')(graph) + .entity('name').tags).to.eql({name: 'forward'}, 'name'); + expect(iD.actionReverse('note')(graph) + .entity('note').tags).to.eql({note: 'forwards'}, 'note'); + expect(iD.actionReverse('ref')(graph) + .entity('ref').tags).to.eql({ref: 'backward'}, 'ref'); + expect(iD.actionReverse('description')(graph) + .entity('description').tags).to.eql({description: 'backwards'}, 'description'); + }); + + it('transforms *=right ⟺ *=left', function () { + var graph = iD.coreGraph([ + iD.osmWay({id: 'sidewalkR', tags: {sidewalk: 'right'}}), + iD.osmWay({id: 'sidewalkL', tags: {sidewalk: 'left'}}) + ]); - it('reverses the direction of a right facing stop sign on the way', function () { - var node1 = iD.Node(); - var node2 = iD.Node(); - var node3 = iD.Node(); - // Attach a right facing stop sign to node 2 (not sure this is a real situation, - // but allows us to test) - node2.tags = { 'direction': 'right', 'highway': 'stop' }; - // Create our way - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - // Act - reverse the way - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - // Assert - confirm that the stop sign on node 2 has changed direction - var target = graph.entity(node2.id); - expect(target.tags.direction).to.eql('left'); - }); + expect(iD.actionReverse('sidewalkR')(graph) + .entity('sidewalkR').tags).to.eql({sidewalk: 'left'}, 'sidewalkR'); + expect(iD.actionReverse('sidewalkL')(graph) + .entity('sidewalkL').tags).to.eql({sidewalk: 'right'}, 'sidewalkL'); + }); + + it('skips *=right ⟺ *=left for ignored tags', function () { + var graph = iD.coreGraph([ + iD.osmWay({id: 'name', tags: {name: 'right'}}), + iD.osmWay({id: 'note', tags: {note: 'right'}}), + iD.osmWay({id: 'ref', tags: {ref: 'left'}}), + iD.osmWay({id: 'description', tags: {description: 'left'}}) + ]); - it('does not assign a direction to a directionless stop sign on the way during a reverse', function () { - var node1 = iD.Node(); - var node2 = iD.Node(); - var node3 = iD.Node(); - // Attach a stop sign to node 2 with no direction specified - node2.tags = { 'highway': 'stop' }; - // Create our way - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - // Act - reverse the way - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - // Assert - confirm that the stop sign on node 2 has not gained a direction tag - var target = graph.entity(node2.id); - expect(target.tags.direction).to.be.undefined; + expect(iD.actionReverse('name')(graph) + .entity('name').tags).to.eql({name: 'right'}, 'name'); + expect(iD.actionReverse('note')(graph) + .entity('note').tags).to.eql({note: 'right'}, 'note'); + expect(iD.actionReverse('ref')(graph) + .entity('ref').tags).to.eql({ref: 'left'}, 'ref'); + expect(iD.actionReverse('description')(graph) + .entity('description').tags).to.eql({description: 'left'}, 'description'); + }); }); - it('ignores directions other than forward or backward on attached stop sign during a reverse', function () { - var node1 = iD.Node(); - var node2 = iD.Node(); - var node3 = iD.Node(); - // Attach a stop sign to node 2 with a non-standard direction - node2.tags = { 'direction': 'empty', 'highway': 'stop' }; - // Create our way - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - // Act - reverse the way - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - // Assert - confirm that the stop sign on node 2 has not had its direction tag altered - var target = graph.entity(node2.id); - expect(target.tags.direction).to.eql('empty'); - }); - it('reverses the direction of a forward facing traffic sign on the way', function () { - var node1 = iD.Node(); - var node2 = iD.Node(); - var node3 = iD.Node(); - // Attach a forward facing stop sign to node 2 using the traffic_sign approach - node2.tags = { 'traffic_sign:forward': 'stop' }; - // Create our way - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - // Act - reverse the way - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - // Assert - confirm that the stop sign on node 2 has changed direction - var target = graph.entity(node2.id); - expect(target.tags['traffic_sign:backward']).to.eql('stop'); - }); + describe('reverses relation roles', function () { + it('transforms role=forward ⟺ role=backward in member relations', function () { + var graph = iD.coreGraph([ + iD.osmNode({id: 'n1'}), + iD.osmNode({id: 'n2'}), + iD.osmWay({id: 'w1', nodes: ['n1', 'n2'], tags: {highway: 'residential'}}), + iD.osmRelation({id: 'forward', members: [{type: 'way', id: 'w1', role: 'forward'}]}), + iD.osmRelation({id: 'backward', members: [{type: 'way', id: 'w1', role: 'backward'}]}) + ]); - it('reverses the direction of a backward facing stop sign on the way', function () { - var node1 = iD.Node(); - var node2 = iD.Node(); - var node3 = iD.Node(); - // Attach a backward facing stop sign to node 2 using the traffic_sign approach - node2.tags = { 'traffic_sign:backward': 'stop' }; - // Create our way - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - // Act - reverse the way - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - // Assert - confirm that the stop sign on node 2 has changed direction - var target = graph.entity(node2.id); - expect(target.tags['traffic_sign:forward']).to.eql('stop'); - }); + expect(iD.actionReverse('w1')(graph) + .entity('forward').members[0].role).to.eql('backward', 'forward'); + expect(iD.actionReverse('w1')(graph) + .entity('backward').members[0].role).to.eql('forward', 'backward'); + }); + + it('drops "s" from forwards/backwards when reversing', function () { + var graph = iD.coreGraph([ + iD.osmNode({id: 'n1'}), + iD.osmNode({id: 'n2'}), + iD.osmWay({id: 'w1', nodes: ['n1', 'n2'], tags: {highway: 'residential'}}), + iD.osmRelation({id: 'forwards', members: [{type: 'way', id: 'w1', role: 'forwards'}]}), + iD.osmRelation({id: 'backwards', members: [{type: 'way', id: 'w1', role: 'backwards'}]}) + ]); - it('reverses the direction of a left facing traffic sign on the way', function () { - var node1 = iD.Node(); - var node2 = iD.Node(); - var node3 = iD.Node(); - // Attach a left facing stop sign to node 2 using the traffic_sign approach - node2.tags = { 'traffic_sign:left': 'stop' }; - // Create our way - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - // Act - reverse the way - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - // Assert - confirm that the stop sign on node 2 has changed direction - var target = graph.entity(node2.id); - expect(target.tags['traffic_sign:right']).to.eql('stop'); - }); + expect(iD.actionReverse('w1')(graph) + .entity('forwards').members[0].role).to.eql('backward', 'forwards'); + expect(iD.actionReverse('w1')(graph) + .entity('backwards').members[0].role).to.eql('forward', 'backwards'); + }); + + it('transforms role=north ⟺ role=south in member relations', function () { + var graph = iD.coreGraph([ + iD.osmNode({id: 'n1'}), + iD.osmNode({id: 'n2'}), + iD.osmWay({id: 'w1', nodes: ['n1', 'n2'], tags: {highway: 'residential'}}), + iD.osmRelation({id: 'north', members: [{type: 'way', id: 'w1', role: 'north'}]}), + iD.osmRelation({id: 'south', members: [{type: 'way', id: 'w1', role: 'south'}]}) + ]); - it('reverses the direction of a right facing stop sign on the way', function () { - var node1 = iD.Node(); - var node2 = iD.Node(); - var node3 = iD.Node(); - // Attach a right facing stop sign to node 2 using the traffic_sign approach - node2.tags = { 'traffic_sign:right': 'stop' }; - // Create our way - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - // Act - reverse the way - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - // Assert - confirm that the stop sign on node 2 has changed direction - var target = graph.entity(node2.id); - expect(target.tags['traffic_sign:left']).to.eql('stop'); - }); + expect(iD.actionReverse('w1')(graph) + .entity('north').members[0].role).to.eql('south', 'north'); + expect(iD.actionReverse('w1')(graph) + .entity('south').members[0].role).to.eql('north', 'south'); + }); + + it('transforms role=east ⟺ role=west in member relations', function () { + var graph = iD.coreGraph([ + iD.osmNode({id: 'n1'}), + iD.osmNode({id: 'n2'}), + iD.osmWay({id: 'w1', nodes: ['n1', 'n2'], tags: {highway: 'residential'}}), + iD.osmRelation({id: 'east', members: [{type: 'way', id: 'w1', role: 'east'}]}), + iD.osmRelation({id: 'west', members: [{type: 'way', id: 'w1', role: 'west'}]}) + ]); - // For issue #4595 - it('reverses the direction of a forward facing traffic_signals on the way', function () { - var node1 = iD.Node(); - var node2 = iD.Node({tags: { 'traffic_signals:direction': 'forward', 'highway': 'traffic_signals' }}); - var node3 = iD.Node(); - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - var target = graph.entity(node2.id); - expect(target.tags['traffic_signals:direction']).to.eql('backward'); - }); + expect(iD.actionReverse('w1')(graph) + .entity('east').members[0].role).to.eql('west', 'east'); + expect(iD.actionReverse('w1')(graph) + .entity('west').members[0].role).to.eql('east', 'west'); + }); + + it('ignores directionless roles in member relations', function () { + var graph = iD.coreGraph([ + iD.osmNode({id: 'n1'}), + iD.osmNode({id: 'n2'}), + iD.osmWay({id: 'w1', nodes: ['n1', 'n2'], tags: {highway: 'residential'}}), + iD.osmRelation({id: 'ignore', members: [{type: 'way', id: 'w1', role: 'ignore'}]}), + iD.osmRelation({id: 'empty', members: [{type: 'way', id: 'w1', role: ''}]}) + ]); - it('reverses the direction of a backward facing traffic_signals on the way', function () { - var node1 = iD.Node(); - var node2 = iD.Node({tags: { 'traffic_signals:direction': 'backward', 'highway': 'traffic_signals' }}); - var node3 = iD.Node(); - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - var target = graph.entity(node2.id); - expect(target.tags['traffic_signals:direction']).to.eql('forward'); + expect(iD.actionReverse('w1')(graph) + .entity('ignore').members[0].role).to.eql('ignore', 'ignore'); + expect(iD.actionReverse('w1')(graph) + .entity('empty').members[0].role).to.eql('', 'empty'); + }); }); - it('reverses the direction of a left facing traffic_signals on the way', function () { - var node1 = iD.Node(); - var node2 = iD.Node({tags: { 'traffic_signals:direction': 'left', 'highway': 'traffic_signals' }}); - var node3 = iD.Node(); - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - var target = graph.entity(node2.id); - expect(target.tags['traffic_signals:direction']).to.eql('right'); - }); - it('reverses the direction of a right facing traffic_signals on the way', function () { - var node1 = iD.Node(); - var node2 = iD.Node({tags: { 'traffic_signals:direction': 'right', 'highway': 'traffic_signals' }}); - var node3 = iD.Node(); - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - var target = graph.entity(node2.id); - expect(target.tags['traffic_signals:direction']).to.eql('left'); + describe('reverses directional values on childnodes', function () { + // For issue #3076 + it('reverses the direction of a forward facing stop sign on the way', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: {'direction': 'forward', 'highway': 'stop'}}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags.direction).to.eql('backward'); + }); + + it('reverses the direction of a backward facing stop sign on the way', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: {'direction': 'backward', 'highway': 'stop'}}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags.direction).to.eql('forward'); + }); + + it('reverses the direction of a left facing stop sign on the way', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: {'direction': 'left', 'highway': 'stop'}}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags.direction).to.eql('right'); + }); + + it('reverses the direction of a right facing stop sign on the way', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: {'direction': 'right', 'highway': 'stop'}}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags.direction).to.eql('left'); + }); + + it('does not assign a direction to a directionless stop sign on the way during a reverse', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: {'highway': 'stop'}}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags.direction).to.be.undefined; + }); + + it('ignores directions other than forward or backward on attached stop sign during a reverse', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: {'direction': 'empty', 'highway': 'stop'}}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags.direction).to.eql('empty'); + }); }); - it('does not assign a direction to a directionless traffic_signals on the way during a reverse', function () { - var node1 = iD.Node(); - var node2 = iD.Node({tags: { 'highway': 'traffic_signals' }}); - var node3 = iD.Node(); - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - var target = graph.entity(node2.id); - expect(target.tags['traffic_signals:direction']).to.be.undefined; - }); - it('ignores directions other than forward or backward on attached traffic_signals during a reverse', function () { - var node1 = iD.Node(); - var node2 = iD.Node({tags: { 'traffic_signals:direction': 'empty', 'highway': 'traffic_signals' }}); - var node3 = iD.Node(); - var way = iD.Way({nodes: [node1.id, node2.id, node3.id]}); - var graph = iD.actionReverse(way.id)(iD.Graph([node1, node2, node3, way])); - var target = graph.entity(node2.id); - expect(target.tags['traffic_signals:direction']).to.eql('empty'); + describe('reverses directional keys on childnodes', function () { + it('reverses the direction of a forward facing traffic sign on the way', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: {'traffic_sign:forward': 'stop'}}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags['traffic_sign:backward']).to.eql('stop'); + }); + + it('reverses the direction of a backward facing stop sign on the way', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: {'traffic_sign:backward': 'stop'}}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags['traffic_sign:forward']).to.eql('stop'); + }); + + it('reverses the direction of a left facing traffic sign on the way', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: {'traffic_sign:left': 'stop'}}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags['traffic_sign:right']).to.eql('stop'); + }); + + it('reverses the direction of a right facing stop sign on the way', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: {'traffic_sign:right': 'stop'}}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags['traffic_sign:left']).to.eql('stop'); + }); + + // For issue #4595 + it('reverses the direction of a forward facing traffic_signals on the way', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: { 'traffic_signals:direction': 'forward', 'highway': 'traffic_signals' }}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags['traffic_signals:direction']).to.eql('backward'); + }); + + it('reverses the direction of a backward facing traffic_signals on the way', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: { 'traffic_signals:direction': 'backward', 'highway': 'traffic_signals' }}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags['traffic_signals:direction']).to.eql('forward'); + }); + + it('reverses the direction of a left facing traffic_signals on the way', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: { 'traffic_signals:direction': 'left', 'highway': 'traffic_signals' }}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags['traffic_signals:direction']).to.eql('right'); + }); + + it('reverses the direction of a right facing traffic_signals on the way', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: { 'traffic_signals:direction': 'right', 'highway': 'traffic_signals' }}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags['traffic_signals:direction']).to.eql('left'); + }); + + it('does not assign a direction to a directionless traffic_signals on the way during a reverse', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: { 'highway': 'traffic_signals' }}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags['traffic_signals:direction']).to.be.undefined; + }); + + it('ignores directions other than forward or backward on attached traffic_signals during a reverse', function () { + var node1 = iD.osmNode(); + var node2 = iD.osmNode({tags: { 'traffic_signals:direction': 'empty', 'highway': 'traffic_signals' }}); + var node3 = iD.osmNode(); + var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]}); + var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way])); + var target = graph.entity(node2.id); + expect(target.tags['traffic_signals:direction']).to.eql('empty'); + }); }); - });