Rename simpleMultipolygon functions to oldMultipolygon

Return old_multipolygon validation issues for the relations as well as the outer ways
Remove console logging
Don't run missing_tag validation for relations with old_multipolygon issues
This commit is contained in:
Quincy Morgan
2019-02-01 15:19:28 -05:00
parent 973697b8ba
commit 9785021f44
8 changed files with 106 additions and 53 deletions
+2 -2
View File
@@ -7,7 +7,7 @@ import { actionAddMember } from './add_member';
import { geoSphericalDistance } from '../geo';
import {
osmIsSimpleMultipolygonOuterMember,
osmIsOldMultipolygonOuterMember,
osmRelation,
osmWay
} from '../osm';
@@ -93,7 +93,7 @@ export function actionSplit(nodeId, newWayIds) {
var nodesA;
var nodesB;
var isArea = wayA.isArea();
var isOuter = osmIsSimpleMultipolygonOuterMember(wayA, graph);
var isOuter = osmIsOldMultipolygonOuterMember(wayA, graph);
if (wayA.isClosed()) {
var nodes = wayA.nodes.slice(0, -1);
+18 -16
View File
@@ -1,6 +1,5 @@
import { dispatch as d3_dispatch } from 'd3-dispatch';
import _isObject from 'lodash-es/isObject';
import _isFunction from 'lodash-es/isFunction';
import _map from 'lodash-es/map';
import _filter from 'lodash-es/filter';
@@ -21,9 +20,6 @@ export function coreValidator(context) {
var validations = _filter(Validations, _isFunction).reduce(function(obj, validation) {
var func = validation();
if (!func.type) {
console.log('Validation rule not found for: ' + validation);
}
obj[func.type] = func;
return obj;
}, {});
@@ -84,12 +80,23 @@ export function coreValidator(context) {
var ranValidations = new Set([]);
// runs validation and appends resulting issues, returning true if validation passed
function runValidation(type) {
var fn = validations[type];
var typeIssues = fn(entity, context);
issues = issues.concat(typeIssues);
ranValidations.add(type);
return typeIssues.length === 0;
if (!ranValidations.has(type)) {
var fn = validations[type];
var typeIssues = fn(entity, context);
issues = issues.concat(typeIssues);
ranValidations.add(type);
return typeIssues.length === 0;
}
return true;
}
if (entity.type === 'relation') {
if (!runValidation('old_multipolygon')) {
// don't flag missing tags if they are on the outer way
ranValidations.add('missing_tag');
}
}
// other validations require feature to be tagged
if (!runValidation('missing_tag')) return issues;
if (entity.type === 'way') {
@@ -103,10 +110,8 @@ export function coreValidator(context) {
runValidation('tag_suggests_area');
}
// run all validations not yet run manually
entityValidationIds.forEach(function(ruleId) {
if (!ranValidations.has(ruleId)) {
runValidation(ruleId);
}
entityValidationIds.forEach(function(id) {
runValidation(id);
});
return issues;
}
@@ -184,9 +189,6 @@ export function validationIssue(attrs) {
return id;
};
if (!_isObject(attrs)) console.log('Input attrs is not an object');
if (!attrs.message) console.log('attrs.message is empty');
this.type = attrs.type;
this.severity = attrs.severity;
this.message = attrs.message;
+3 -2
View File
@@ -17,8 +17,9 @@ export {
} from './lanes';
export {
osmIsSimpleMultipolygonOuterMember,
osmSimpleMultipolygonOuterMember,
osmOldMultipolygonOuterMemberOfRelation,
osmIsOldMultipolygonOuterMember,
osmOldMultipolygonOuterMember,
osmJoinWays
} from './multipolygon';
+37 -2
View File
@@ -3,9 +3,44 @@ import { osmIsInterestingTag } from './tags';
import { osmWay } from './way';
// "Old" multipolyons, previously known as "simple" multipolygons, are as follows:
//
// 1. Relation tagged with `type=multipolygon` and no interesting tags.
// 2. One and only one member with the `outer` role. Must be a way with interesting tags.
// 3. No members without a role.
//
// Old multipolygons are no longer recommended but are still rendered as areas by iD.
export function osmOldMultipolygonOuterMemberOfRelation(entity, graph) {
if (entity.type !== 'relation' ||
!entity.isMultipolygon()
|| Object.keys(entity.tags).filter(osmIsInterestingTag).length > 1) {
return false;
}
var outerMember;
for (var memberIndex in entity.members) {
var member = entity.members[memberIndex];
if (!member.role) return false;
if (member.role === 'outer') {
if (outerMember) return false;
if (member.type !== 'way') return false;
if (!graph.hasEntity(member.id)) return false;
outerMember = graph.entity(member.id);
if (Object.keys(outerMember.tags).filter(osmIsInterestingTag).length === 0) {
return false;
}
}
}
return outerMember;
}
// For fixing up rendering of multipolygons with tags on the outer member.
// https://github.com/openstreetmap/iD/issues/613
export function osmIsSimpleMultipolygonOuterMember(entity, graph) {
export function osmIsOldMultipolygonOuterMember(entity, graph) {
if (entity.type !== 'way' || Object.keys(entity.tags).filter(osmIsInterestingTag).length === 0)
return false;
@@ -30,7 +65,7 @@ export function osmIsSimpleMultipolygonOuterMember(entity, graph) {
}
export function osmSimpleMultipolygonOuterMember(entity, graph) {
export function osmOldMultipolygonOuterMember(entity, graph) {
if (entity.type !== 'way')
return false;
+2 -2
View File
@@ -3,7 +3,7 @@ import _values from 'lodash-es/values';
import { bisector as d3_bisector } from 'd3-array';
import { osmEntity, osmIsSimpleMultipolygonOuterMember } from '../osm';
import { osmEntity, osmIsOldMultipolygonOuterMember } from '../osm';
import { svgPath, svgSegmentWay, svgTagClasses } from './index';
@@ -200,7 +200,7 @@ export function svgAreas(projection, context) {
var entity = entities[i];
if (entity.geometry(graph) !== 'area') continue;
multipolygon = osmIsSimpleMultipolygonOuterMember(entity, graph);
multipolygon = osmIsOldMultipolygonOuterMember(entity, graph);
if (multipolygon) {
areas[multipolygon.id] = {
entity: multipolygon.mergeTags(entity.tags),
+2 -2
View File
@@ -14,7 +14,7 @@ import {
svgTagClasses
} from './index';
import { osmEntity, osmSimpleMultipolygonOuterMember } from '../osm';
import { osmEntity, osmOldMultipolygonOuterMember } from '../osm';
import { utilDetect } from '../util/detect';
@@ -191,7 +191,7 @@ export function svgLines(projection, context) {
for (var i = 0; i < entities.length; i++) {
var entity = entities[i];
var outer = osmSimpleMultipolygonOuterMember(entity, graph);
var outer = osmOldMultipolygonOuterMember(entity, graph);
if (outer) {
ways.push(entity.mergeTags(outer.tags));
oldMultiPolygonOuters[outer.id] = true;
+21 -6
View File
@@ -1,5 +1,8 @@
import { t } from '../util/locale';
import { osmIsSimpleMultipolygonOuterMember } from '../osm';
import {
osmIsOldMultipolygonOuterMember,
osmOldMultipolygonOuterMemberOfRelation
} from '../osm';
import { utilDisplayLabel } from '../util';
import {
validationIssue,
@@ -12,17 +15,29 @@ import {
export function validationOldMultipolygon() {
var validation = function(entity, context) {
var issues = [];
var graph = context.graph();
var mistaggedMultipolygon = osmIsSimpleMultipolygonOuterMember(entity, graph);
if (mistaggedMultipolygon) {
var multipolygonLabel = utilDisplayLabel(mistaggedMultipolygon, context);
var multipolygon, outerWay;
if (entity.type === 'relation') {
outerWay = osmOldMultipolygonOuterMemberOfRelation(entity, graph);
multipolygon = entity;
} else if (entity.type === 'way') {
multipolygon = osmIsOldMultipolygonOuterMember(entity, graph);
outerWay = entity;
} else {
return issues;
}
if (multipolygon && outerWay) {
var multipolygonLabel = utilDisplayLabel(multipolygon, context);
issues.push(new validationIssue({
type: 'old_multipolygon',
severity: 'warning',
message: t('issues.old_multipolygon.message', {multipolygon: multipolygonLabel}),
message: t('issues.old_multipolygon.message', { multipolygon: multipolygonLabel }),
tooltip: t('issues.old_multipolygon.tip'),
entities: [entity, mistaggedMultipolygon],
entities: [outerWay, multipolygon],
fixes: [
new validationIssueFix({
title: t('issues.fix.move_tags.title'),
+21 -21
View File
@@ -1,11 +1,11 @@
describe('iD.osmIsSimpleMultipolygonOuterMember', function() {
describe('iD.osmIsOldMultipolygonOuterMember', function() {
it('returns the parent relation of a simple multipolygon outer', function() {
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);
expect(iD.osmIsOldMultipolygonOuterMember(outer, graph)).to.equal(relation);
});
it('returns the parent relation of a simple multipolygon outer, assuming role outer if unspecified', function() {
@@ -14,7 +14,7 @@ describe('iD.osmIsSimpleMultipolygonOuterMember', function() {
{tags: {type: 'multipolygon'}, members: [{id: outer.id}]}
);
var graph = iD.coreGraph([outer, relation]);
expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.equal(relation);
expect(iD.osmIsOldMultipolygonOuterMember(outer, graph)).to.equal(relation);
});
it('returns false if entity is not a way', function() {
@@ -23,7 +23,7 @@ describe('iD.osmIsSimpleMultipolygonOuterMember', function() {
{tags: {type: 'multipolygon'}, members: [{id: outer.id, role: 'outer'}]}
);
var graph = iD.coreGraph([outer, relation]);
expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false;
expect(iD.osmIsOldMultipolygonOuterMember(outer, graph)).to.be.false;
});
it('returns false if entity does not have interesting tags', function() {
@@ -32,13 +32,13 @@ describe('iD.osmIsSimpleMultipolygonOuterMember', function() {
{tags: {type: 'multipolygon'}, members: [{id: outer.id, role: 'outer'}]}
);
var graph = iD.coreGraph([outer, relation]);
expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false;
expect(iD.osmIsOldMultipolygonOuterMember(outer, graph)).to.be.false;
});
it('returns false if entity does not have a parent relation', function() {
var outer = iD.osmWay({tags: {'natural':'wood'}});
var graph = iD.coreGraph([outer]);
expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false;
expect(iD.osmIsOldMultipolygonOuterMember(outer, graph)).to.be.false;
});
it('returns false if the parent is not a multipolygon', function() {
@@ -47,7 +47,7 @@ describe('iD.osmIsSimpleMultipolygonOuterMember', function() {
{tags: {type: 'route'}, members: [{id: outer.id, role: 'outer'}]}
);
var graph = iD.coreGraph([outer, relation]);
expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false;
expect(iD.osmIsOldMultipolygonOuterMember(outer, graph)).to.be.false;
});
it('returns false if the parent has interesting tags', function() {
@@ -56,7 +56,7 @@ describe('iD.osmIsSimpleMultipolygonOuterMember', function() {
{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;
expect(iD.osmIsOldMultipolygonOuterMember(outer, graph)).to.be.false;
});
it('returns the parent relation of a simple multipolygon outer, ignoring uninteresting parent tags', function() {
@@ -65,7 +65,7 @@ describe('iD.osmIsSimpleMultipolygonOuterMember', function() {
{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);
expect(iD.osmIsOldMultipolygonOuterMember(outer, graph)).to.equal(relation);
});
it('returns false if the parent has multiple outer ways', function() {
@@ -75,8 +75,8 @@ describe('iD.osmIsSimpleMultipolygonOuterMember', function() {
{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;
expect(iD.osmIsOldMultipolygonOuterMember(outer1, graph)).to.be.false;
expect(iD.osmIsOldMultipolygonOuterMember(outer2, graph)).to.be.false;
});
it('returns false if the parent has multiple outer ways, assuming role outer if unspecified', function() {
@@ -86,8 +86,8 @@ describe('iD.osmIsSimpleMultipolygonOuterMember', function() {
{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;
expect(iD.osmIsOldMultipolygonOuterMember(outer1, graph)).to.be.false;
expect(iD.osmIsOldMultipolygonOuterMember(outer2, graph)).to.be.false;
});
it('returns false if the entity is not an outer', function() {
@@ -96,12 +96,12 @@ describe('iD.osmIsSimpleMultipolygonOuterMember', function() {
{tags: {type: 'multipolygon'}, members: [{id: inner.id, role: 'inner'}]}
);
var graph = iD.coreGraph([inner, relation]);
expect(iD.osmIsSimpleMultipolygonOuterMember(inner, graph)).to.be.false;
expect(iD.osmIsOldMultipolygonOuterMember(inner, graph)).to.be.false;
});
});
describe('iD.osmSimpleMultipolygonOuterMember', function() {
describe('iD.osmOldMultipolygonOuterMember', function() {
it('returns the outer member of a simple multipolygon', function() {
var inner = iD.osmWay();
var outer = iD.osmWay({tags: {'natural':'wood'}});
@@ -111,8 +111,8 @@ describe('iD.osmSimpleMultipolygonOuterMember', function() {
});
var graph = iD.coreGraph([inner, outer, relation]);
expect(iD.osmSimpleMultipolygonOuterMember(inner, graph)).to.equal(outer);
expect(iD.osmSimpleMultipolygonOuterMember(outer, graph)).to.equal(outer);
expect(iD.osmOldMultipolygonOuterMember(inner, graph)).to.equal(outer);
expect(iD.osmOldMultipolygonOuterMember(outer, graph)).to.equal(outer);
});
it('returns falsy for a complex multipolygon', function() {
@@ -126,9 +126,9 @@ describe('iD.osmSimpleMultipolygonOuterMember', function() {
});
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;
expect(iD.osmSimpleMultipolygonOuterMember(outer2, graph)).not.to.be.ok;
expect(iD.osmOldMultipolygonOuterMember(inner, graph)).not.to.be.ok;
expect(iD.osmOldMultipolygonOuterMember(outer1, graph)).not.to.be.ok;
expect(iD.osmOldMultipolygonOuterMember(outer2, graph)).not.to.be.ok;
});
it('handles incomplete relations', function() {
@@ -139,7 +139,7 @@ describe('iD.osmSimpleMultipolygonOuterMember', function() {
});
var graph = iD.coreGraph([way, relation]);
expect(iD.osmSimpleMultipolygonOuterMember(way, graph)).not.to.be.ok;
expect(iD.osmOldMultipolygonOuterMember(way, graph)).not.to.be.ok;
});
});