Skip to content

Commit

Permalink
Add validation rule to flag impossible oneway highways and waterways (c…
Browse files Browse the repository at this point in the history
…lose #6216)
  • Loading branch information
quincylvania committed Apr 26, 2019
1 parent 7d29a98 commit ecc217f
Show file tree
Hide file tree
Showing 11 changed files with 274 additions and 26 deletions.
19 changes: 19 additions & 0 deletions data/core.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,23 @@ en:
unknown_road:
message: "{feature} has no classification"
reference: "Roads without a specific type may not appear in maps or routing."
impossible_oneway:
title: Impossible One-Ways
tip: "Find route issues with one-way features"
waterway:
connected:
start:
message: "{feature} flows away from a connected waterway"
end:
message: "{feature} flows against a connected waterway"
reference: "Waterway segements should all flow in the same direction."
highway:
start:
message: "{feature} is unreachable"
reference: "One-way roads must be accessible via other roads."
end:
message: "{feature} has no outlet"
reference: "One-way roads must lead to other roads."
unsquare_way:
title: Unsquare Buildings
message: "{feature} isn't quite square"
Expand Down Expand Up @@ -1444,6 +1461,8 @@ en:
title: Remove the tags
reposition_features:
title: Reposition the features
reverse_feature:
title: Reverse this feature
select_preset:
title: Select a feature type
select_road_type:
Expand Down
14 changes: 14 additions & 0 deletions data/deprecated.json
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,20 @@
"old": {"office": "real_estate"},
"replace": {"office": "estate_agent"}
},
{
"old": {"oneway": "1"},
"replace": {"oneway": "yes"}
},
{
"old": {"oneway": "alternate"},
"replace": {"oneway": "alternating"}
},
{
"old": {"oneway": "no;yes"}
},
{
"old": {"oneway": "unknown"}
},
{
"old": {"place_name": "*"},
"replace": {"name": "$1"}
Expand Down
4 changes: 4 additions & 0 deletions data/taginfo.json
Original file line number Diff line number Diff line change
Expand Up @@ -1801,6 +1801,10 @@
{"key": "natural", "value": "marsh", "description": "🄳 ➜ natural=wetland + wetland=marsh"},
{"key": "natural", "value": "waterfall", "description": "🄳 ➜ waterway=waterfall"},
{"key": "office", "value": "real_estate", "description": "🄳 ➜ office=estate_agent"},
{"key": "oneway", "value": "1", "description": "🄳 ➜ oneway=yes"},
{"key": "oneway", "value": "alternate", "description": "🄳 ➜ oneway=alternating"},
{"key": "oneway", "value": "no;yes", "description": "🄳"},
{"key": "oneway", "value": "unknown", "description": "🄳"},
{"key": "place_name", "description": "🄳 ➜ name=*"},
{"key": "pole", "value": "transition", "description": "🄳 ➜ location:transition=yes"},
{"key": "postcode", "description": "🄳 ➜ addr:postcode=*"},
Expand Down
28 changes: 28 additions & 0 deletions dist/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1737,6 +1737,31 @@
"message": "{feature} has no classification",
"reference": "Roads without a specific type may not appear in maps or routing."
},
"impossible_oneway": {
"title": "Impossible One-Ways",
"tip": "Find route issues with one-way features",
"waterway": {
"connected": {
"start": {
"message": "{feature} flows away from a connected waterway"
},
"end": {
"message": "{feature} flows against a connected waterway"
},
"reference": "Waterway segements should all flow in the same direction."
}
},
"highway": {
"start": {
"message": "{feature} is unreachable",
"reference": "One-way roads must be accessible via other roads."
},
"end": {
"message": "{feature} has no outlet",
"reference": "One-way roads must lead to other roads."
}
}
},
"unsquare_way": {
"title": "Unsquare Buildings",
"message": "{feature} isn't quite square",
Expand Down Expand Up @@ -1795,6 +1820,9 @@
"reposition_features": {
"title": "Reposition the features"
},
"reverse_feature": {
"title": "Reverse this feature"
},
"select_preset": {
"title": "Select a feature type"
},
Expand Down
9 changes: 5 additions & 4 deletions modules/core/validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,13 @@ export function coreValidator(context) {

if (entity.type === 'way') {
runValidation('crossing_ways');
runValidation('almost_junction');

// only check for disconnected way if no almost junctions
if (runValidation('almost_junction')) {
runValidation('disconnected_way');
// only check impossible_oneway if no disconnected_way issues
if (runValidation('disconnected_way')) {
runValidation('impossible_oneway');
} else {
ran.disconnected_way = true;
ran.impossible_oneway = true;
}

runValidation('tag_suggests_area');
Expand Down
1 change: 1 addition & 0 deletions modules/operations/reverse.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export function operationReverse(selectedIDs, context) {

var operation = function() {
context.perform(actionReverse(entityID), operation.annotation());
context.validator().validate();
};


Expand Down
20 changes: 20 additions & 0 deletions modules/osm/tags.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,23 @@ export var osmRightSideIsInsideTags = {
'embankment': true
}
};

// "highway" tag values for pedestrian or vehicle right-of-ways that make up the routable network
export var osmRoutableHighwayTagValues = {
motorway: true, trunk: true, primary: true, secondary: true, tertiary: true, residential: true,
motorway_link: true, trunk_link: true, primary_link: true, secondary_link: true, tertiary_link: true,
unclassified: true, road: true, service: true, track: true, living_street: true, raceway: true, bus_guideway: true,
path: true, footway: true, cycleway: true, bridleway: true, pedestrian: true, corridor: true, steps: true
};

// "railway" tag values representing existing railroad tracks (purposely does not include 'abandoned')
export var osmRailwayTrackTagValues = {
rail: true, light_rail: true, tram: true, subway: true,
monorail: true, funicular: true, miniature: true, narrow_gauge: true,
disused: true, preserved: true
};

// "waterway" tag values for line features representing water flow
export var osmFlowingWaterwayTagValues = {
canal: true, ditch: true, drain: true, river: true, stream: true
};
5 changes: 2 additions & 3 deletions modules/validations/almost_junction.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { actionChangeTags } from '../actions/change_tags';
import { actionMergeNodes } from '../actions/merge_nodes';
import { t } from '../util/locale';
import { utilDisplayLabel } from '../util';
import { osmRoutableHighwayTagValues } from '../osm/tags';
import { validationIssue, validationIssueFix } from '../core/validation';


Expand All @@ -21,9 +22,7 @@ export function validationAlmostJunction() {

function isHighway(entity) {
return entity.type === 'way' &&
entity.tags.highway &&
entity.tags.highway !== 'no' &&
entity.tags.highway !== 'proposed';
osmRoutableHighwayTagValues[entity.tags.highway];
}

function isNoexit(node) {
Expand Down
23 changes: 4 additions & 19 deletions modules/validations/crossing_ways.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { actionChangeTags } from '../actions/change_tags';
import { actionMergeNodes } from '../actions/merge_nodes';
import { geoExtent, geoLineIntersection, geoSphericalClosestNode } from '../geo';
import { osmNode } from '../osm/node';
import { osmFlowingWaterwayTagValues, osmRailwayTrackTagValues, osmRoutableHighwayTagValues } from '../osm/tags';
import { t } from '../util/locale';
import { utilDisplayLabel } from '../util';
import { validationIssue, validationIssueFix } from '../core/validation';
Expand Down Expand Up @@ -67,22 +68,6 @@ export function validationCrossingWays() {
return getFeatureTypeForTags(tags);
}

// whitelists
var waterways = {
canal: true, ditch: true, drain: true, river: true, stream: true
};
var railways = {
rail: true, disused: true, tram: true, subway: true, narrow_gauge: true,
light_rail: true, preserved: true, miniature: true, monorail: true, funicular: true
};
var highways = {
residential: true, service: true, track: true, unclassified: true, footway: true,
path: true, tertiary: true, secondary: true, primary: true, living_street: true,
cycleway: true, trunk: true, steps: true, motorway: true, motorway_link: true,
pedestrian: true, trunk_link: true, primary_link: true, secondary_link: true,
road: true, tertiary_link: true, bridleway: true, raceway: true, corridor: true,
bus_guideway: true
};
// blacklist
var ignoredBuildings = {
demolished: true, dismantled: true, proposed: true, razed: true
Expand All @@ -95,9 +80,9 @@ export function validationCrossingWays() {
// don't check non-building areas
if (hasTag(tags, 'area')) return null;

if (hasTag(tags, 'highway') && highways[tags.highway]) return 'highway';
if (hasTag(tags, 'railway') && railways[tags.railway]) return 'railway';
if (hasTag(tags, 'waterway') && waterways[tags.waterway]) return 'waterway';
if (hasTag(tags, 'highway') && osmRoutableHighwayTagValues[tags.highway]) return 'highway';
if (hasTag(tags, 'railway') && osmRailwayTrackTagValues[tags.railway]) return 'railway';
if (hasTag(tags, 'waterway') && osmFlowingWaterwayTagValues[tags.waterway]) return 'waterway';

return null;
}
Expand Down
176 changes: 176 additions & 0 deletions modules/validations/impossible_oneway.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
import { t } from '../util/locale';
import { modeDrawLine } from '../modes/draw_line';
import { actionReverse } from '../actions/reverse';
import { utilDisplayLabel } from '../util';
import { osmFlowingWaterwayTagValues, osmOneWayTags, osmRoutableHighwayTagValues } from '../osm/tags';
import { validationIssue, validationIssueFix } from '../core/validation';


export function validationImpossibleOneway() {
var type = 'impossible_oneway';

function typeForWay(way) {
if (osmRoutableHighwayTagValues[way.tags.highway]) return 'highway';
if (osmFlowingWaterwayTagValues[way.tags.waterway]) return 'waterway';
return null;
}

function isOneway(way) {
if (way.tags.oneway === 'yes') return true;
if (way.tags.oneway) return false;

for (var key in way.tags) {
if (osmOneWayTags[key] && osmOneWayTags[key][way.tags[key]]) {
return true;
}
}
return false;
}

function continueDrawing(way, vertex, context) {
// make sure the vertex is actually visible and editable
var map = context.map();
if (!map.editable() || !map.trimmedExtent().contains(vertex.loc)) {
map.zoomToEase(vertex);
}

context.enter(
modeDrawLine(context, way.id, context.graph(), context.graph(), 'line', way.affix(vertex.id), true)
);
}

function nodeOccursMoreThanOnce(way, nodeID) {
var occurences = 0;
for (var index in way.nodes) {
if (way.nodes[index] === nodeID) {
occurences += 1;
if (occurences > 1) return true;
}
}
return false;
}

function issuesForNode(context, way, nodeID) {

var isFirst = nodeID === way.first();

var wayType = typeForWay(way);
var isWaterway = wayType === 'waterway';

// ignore if this way is self-connected at this node
if (nodeOccursMoreThanOnce(way, nodeID)) return [];

var osm = context.connection();
if (!osm) return [];

var node = context.hasEntity(nodeID);

// ignore if this node or its tile are unloaded
if (!node || !osm.isDataLoaded(node.loc)) return [];

var attachedWaysOfSameType = context.graph().parentWays(node).filter(function(parentWay) {
if (parentWay.id === way.id) return false;
return typeForWay(parentWay) === wayType;
});

// assume it's okay for waterways to start or end disconnected for now
if (isWaterway && attachedWaysOfSameType.length === 0) return [];

var attachedOneways = attachedWaysOfSameType.filter(function(attachedWay) {
return isOneway(attachedWay);
});

// ignore if the way is connected to some non-oneway features
if (attachedOneways.length < attachedWaysOfSameType.length) return [];

if (attachedOneways.length) {
var connectedEndpointsOkay = attachedOneways.some(function(attachedOneway) {
return (isFirst ? attachedOneway.first() : attachedOneway.last()) !== nodeID;
});
if (connectedEndpointsOkay) return [];
}

var fixes = [];

if (attachedOneways.length) {
fixes.push(new validationIssueFix({
icon: 'iD-operation-reverse',
title: t('issues.fix.reverse_feature.title'),
entityIds: [way.id],
onClick: function() {
var id = this.issue.entities[0].id;
context.perform(actionReverse(id), t('operations.reverse.annotation'));
}
}));
}
if (node.tags.noexit !== 'yes') {
fixes.push(new validationIssueFix({
icon: 'iD-operation-continue' + (isFirst ? '-left' : ''),
title: t('issues.fix.continue_from_' + (isFirst ? 'start' : 'end') + '.title'),
onClick: function() {
var entityID = this.issue.entities[0].id;
var vertexID = this.issue.entities[1].id;
var way = context.entity(entityID);
var vertex = context.entity(vertexID);
continueDrawing(way, vertex, context);
}
}));
}

var placement = isFirst ? 'start' : 'end',
messageID = wayType + '.',
referenceID = wayType + '.';

if (isWaterway) {
messageID += 'connected.' + placement;
referenceID += 'connected';
} else {
messageID += placement;
referenceID += placement;
}

return [new validationIssue({
type: type,
subtype: wayType,
severity: 'warning',
message: t('issues.impossible_oneway.' + messageID + '.message', {
feature: utilDisplayLabel(way, context)
}),
reference: getReference(referenceID),
entities: [way, node],
fixes: fixes
})];

function getReference(referenceID) {
return function showReference(selection) {
selection.selectAll('.issue-reference')
.data([0])
.enter()
.append('div')
.attr('class', 'issue-reference')
.text(t('issues.impossible_oneway.' + referenceID + '.reference'));
};
}
}

var validation = function checkDisconnectedWay(entity, context) {

if (entity.type !== 'way' || entity.geometry(context.graph()) !== 'line') return [];

if (entity.isClosed()) return [];

if (!typeForWay(entity)) return [];

if (!isOneway(entity)) return [];

var firstIssues = issuesForNode(context, entity, entity.first());
var lastIssues = issuesForNode(context, entity, entity.last());

return firstIssues.concat(lastIssues);
};


validation.type = type;

return validation;
}
Loading

0 comments on commit ecc217f

Please sign in to comment.