From 46f3cea33aff2a7a5e8c0b9686dbd0d3d23900ef Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Fri, 1 Feb 2019 11:20:51 -0500 Subject: [PATCH] Reorganize validation code Rename IssueManager to coreValidator Rename disconnected highway to disconnected way Rename highway almost junction to almost junction Rename mapcss checks to maprules Rename deprecated tags to deprecated tag --- data/core.yaml | 14 +- dist/locales/en.json | 16 +- modules/core/context.js | 14 +- modules/core/index.js | 1 + modules/core/validator.js | 236 ++++++++++++++++++ modules/index.js | 2 +- modules/services/maprules.js | 4 +- modules/ui/commit.js | 2 +- modules/ui/commit_warnings.js | 2 +- modules/ui/entity_issues.js | 4 +- modules/ui/issues.js | 10 +- ..._almost_junction.js => almost_junction.js} | 16 +- modules/validations/crossing_ways.js | 4 +- modules/validations/deprecated_tag.js | 4 +- ...nnected_highway.js => disconnected_way.js} | 12 +- modules/validations/generic_name.js | 2 +- modules/validations/index.js | 9 +- modules/validations/issue_manager.js | 129 ---------- modules/validations/many_deletions.js | 4 +- .../{mapcss_checks.js => maprules.js} | 6 +- modules/validations/missing_tag.js | 2 +- modules/validations/old_multipolygon.js | 2 +- modules/validations/tag_suggests_area.js | 3 +- modules/validations/validation_issue.js | 101 -------- test/spec/validations/issue_manager.js | 14 +- 25 files changed, 312 insertions(+), 301 deletions(-) create mode 100644 modules/core/validator.js rename modules/validations/{highway_almost_junction.js => almost_junction.js} (92%) rename modules/validations/{disconnected_highway.js => disconnected_way.js} (89%) delete mode 100644 modules/validations/issue_manager.js rename modules/validations/{mapcss_checks.js => maprules.js} (78%) delete mode 100644 modules/validations/validation_issue.js diff --git a/data/core.yaml b/data/core.yaml index 2ebc5ea43..1fb092e67 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1151,9 +1151,10 @@ en: severity: error: error warning: warning - disconnected_highway: - message: "{highway} is disconnected from other highways." - tip: Roads should be connected to other roads or building entrances. + disconnected_way: + highway: + message: "{highway} is disconnected from other roads and paths." + tip: Highways should connect to other highways or building entrances. old_multipolygon: message: "{multipolygon} has misplaced tags." tip: Multipolygons should be tagged on their relation, not their outer way. @@ -1202,9 +1203,10 @@ en: tip: Crossing bridges should use different layers. bridge-bridge_connectable: tip: Crossing bridges should be connected or use different layers. - highway_almost_junction: - message: "{highway} is very close but not connected to {highway2}." - tip: Intersecting highways should share a junction vertex. + almost_junction: + message: "{feature} is very close but not connected to {feature2}." + highway-highway: + tip: Intersecting highways should share a junction vertex. fix: add_connection_vertex: title: Connect the features diff --git a/dist/locales/en.json b/dist/locales/en.json index 9c24bb8af..87697cf5e 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1395,9 +1395,11 @@ "error": "error", "warning": "warning" }, - "disconnected_highway": { - "message": "{highway} is disconnected from other highways.", - "tip": "Roads should be connected to other roads or building entrances." + "disconnected_way": { + "highway": { + "message": "{highway} is disconnected from other roads and paths.", + "tip": "Highways should connect to other highways or building entrances." + } }, "old_multipolygon": { "message": "{multipolygon} has misplaced tags.", @@ -1468,9 +1470,11 @@ "tip": "Crossing bridges should be connected or use different layers." } }, - "highway_almost_junction": { - "message": "{highway} is very close but not connected to {highway2}.", - "tip": "Intersecting highways should share a junction vertex." + "almost_junction": { + "message": "{feature} is very close but not connected to {feature2}.", + "highway-highway": { + "tip": "Intersecting highways should share a junction vertex." + } }, "fix": { "add_connection_vertex": { diff --git a/modules/core/context.js b/modules/core/context.js index 3842d50db..6bc3ae849 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -13,7 +13,7 @@ import { select as d3_select } from 'd3-selection'; import { t, currentLocale, addTranslation, setLocale } from '../util/locale'; import { coreHistory } from './history'; -import { IssueManager } from '../validations/issue_manager'; +import { coreValidator } from './validator'; import { dataLocales, dataEn } from '../../data'; import { geoRawMercator } from '../geo/raw_mercator'; import { modeSelect } from '../modes/select'; @@ -96,10 +96,10 @@ export function coreContext() { /* Straight accessors. Avoid using these if you can. */ - var connection, history, issueManager; + var connection, history, validator; context.connection = function() { return connection; }; context.history = function() { return history; }; - context.issueManager = function() { return issueManager; }; + context.validator = function() { return validator; }; /* Connection */ context.preauth = function(options) { @@ -457,22 +457,22 @@ export function coreContext() { context.changes = history.changes; context.intersects = history.intersects; - issueManager = IssueManager(context); + validator = coreValidator(context); // run validation upon restoring from page reload history.on('restore', function() { - issueManager.validate(); + validator.validate(); }); // re-run validation upon a significant graph change history.on('annotatedChange', function(difference) { if (difference) { - issueManager.validate(); + validator.validate(); } }); // re-run validation upon merging fetched data history.on('merge', function(entities) { if (entities && entities.length > 0) { - issueManager.validate(); + validator.validate(); } }); diff --git a/modules/core/index.js b/modules/core/index.js index da7f872d8..cc0061242 100644 --- a/modules/core/index.js +++ b/modules/core/index.js @@ -3,3 +3,4 @@ export { coreDifference } from './difference'; export { coreGraph } from './graph'; export { coreHistory } from './history'; export { coreTree } from './tree'; +export { coreValidator } from './validator'; diff --git a/modules/core/validator.js b/modules/core/validator.js new file mode 100644 index 000000000..7cb6b5a58 --- /dev/null +++ b/modules/core/validator.js @@ -0,0 +1,236 @@ +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'; +import _flatten from 'lodash-es/flatten'; +import _flattenDeep from 'lodash-es/flattenDeep'; +import _uniq from 'lodash-es/uniq'; +import _uniqWith from 'lodash-es/uniqWith'; +import { osmEntity } from '../osm'; + +import { utilRebind } from '../util/rebind'; +import * as Validations from '../validations/index'; + +export var ValidationIssueType = { + deprecated_tag: 'deprecated_tag', + disconnected_way: 'disconnected_way', + many_deletions: 'many_deletions', + missing_tag: 'missing_tag', + old_multipolygon: 'old_multipolygon', + tag_suggests_area: 'tag_suggests_area', + maprules: 'maprules', + crossing_ways: 'crossing_ways', + almost_junction: 'almost_junction', + generic_name: 'generic_name' +}; + +export var ValidationIssueSeverity = { + warning: 'warning', + error: 'error', +}; + +export function coreValidator(context) { + var dispatch = d3_dispatch('reload'), + self = {}, + issues = [], + issuesByEntityId = {}; + + var validations = _filter(Validations, _isFunction).reduce(function(obj, validation) { + var func = validation(); + if (!func.type) { + throw new Error('Validation type not found: ' + validation); + } + obj[func.type] = func; + return obj; + }, {}); + + self.featureApplicabilityOptions = ['edited', 'all']; + + var featureApplicability = context.storage('issue-features') || 'edited'; + + self.getFeatureApplicability = function() { + return featureApplicability; + }; + + self.setFeatureApplicability = function(applicability) { + featureApplicability = applicability; + context.storage('issue-features', applicability); + }; + + self.getIssues = function() { + return issues; + }; + + self.getWarnings = function() { + return issues.filter(function(issue) { + return issue.severity === 'warning'; + }); + }; + self.getErrors = function() { + return issues.filter(function(issue) { + return issue.severity === 'error'; + }); + }; + + self.getIssuesForEntityWithID = function(entityID) { + if (!context.hasEntity(entityID)) { + return []; + } + if (!issuesByEntityId[entityID]) { + var entity = context.entity(entityID); + issuesByEntityId[entityID] = validateEntity(entity); + } + return issuesByEntityId[entityID]; + }; + + var genericEntityValidations = [ + ValidationIssueType.deprecated_tag, + ValidationIssueType.generic_name, + ValidationIssueType.maprules, + ValidationIssueType.old_multipolygon + ]; + + function validateEntity(entity) { + var issues = []; + // 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); + return typeIssues.length === 0; + } + // other validations require feature to be tagged + if (!runValidation(ValidationIssueType.missing_tag)) return issues; + if (entity.type === 'way') { + if (runValidation(ValidationIssueType.almost_junction)) { + // only check for disconnected highway if no almost junctions + runValidation(ValidationIssueType.disconnected_way); + } + runValidation(ValidationIssueType.crossing_ways); + runValidation(ValidationIssueType.tag_suggests_area); + } + genericEntityValidations.forEach(function(fn) { + runValidation(fn); + }) + return issues; + } + + self.validate = function() { + // clear cached issues + issuesByEntityId = {}; + issues = []; + + var history = context.history(); + var changes = history.changes(); + var entitiesToCheck = changes.created.concat(changes.modified); + var graph = history.graph(); + + issues = issues.concat(validations.many_deletions(changes, context)); + + entitiesToCheck = _uniq(_flattenDeep(_map(entitiesToCheck, function(entity) { + var entities = [entity]; + if (entity.type === 'node') { + // validate ways if their nodes have changed + entities = entities.concat(graph.parentWays(entity)); + } + entities = _map(entities, function(entity) { + if (entity.type !== 'relation') { + // validate relations if their geometries have changed + return [entity].concat(graph.parentRelations(entity)); + } + return entity; + }); + return entities; + }))); + + for (var entityIndex in entitiesToCheck) { + var entity = entitiesToCheck[entityIndex]; + var entityIssues = validateEntity(entity); + issuesByEntityId[entity.id] = entityIssues; + issues = issues.concat(entityIssues); + } + issues = _uniqWith(issues, function(issue1, issue2) { + return issue1.id() === issue2.id(); + }); + dispatch.call('reload', self, issues); + }; + + return utilRebind(self, dispatch, 'on'); +} + +export function validationIssue(attrs) { + + // A unique, deterministic string hash. + // Issues with identical id values are considered identical. + this.id = function () { + var id = this.type; + + if (this.hash) { + // subclasses can pass in their own differentiator + id += this.hash; + } + + // issue subclasses set the entity order but it must be deterministic + var entityKeys = _map(this.entities, function(entity) { + // use the key since it factors in the entity's local version + return osmEntity.key(entity); + }); + // factor in the entities this issue is for + id += entityKeys.join(); + if (this.coordinates) { + // factor in coordinates since two separate issues can have an + // idential type and entities, e.g. in crossing_ways + id += this.coordinates.join(); + } + return id; + }; + + if (!_isObject(attrs)) throw new Error('Input attrs is not an object'); + if (!attrs.type || !ValidationIssueType.hasOwnProperty(attrs.type)) { + throw new Error('Invalid attrs.type: ' + attrs.type); + } + if (!attrs.severity || !ValidationIssueSeverity.hasOwnProperty(attrs.severity)) { + throw new Error('Invalid attrs.severity: ' + attrs.severity); + } + if (!attrs.message) throw new Error('attrs.message is empty'); + + this.type = attrs.type; + this.severity = attrs.severity; + this.message = attrs.message; + this.tooltip = attrs.tooltip; + this.entities = attrs.entities; // expect an array of entities + this.coordinates = attrs.coordinates; // expect a [lon, lat] array + this.info = attrs.info; // an object containing arbitrary extra information + this.fixes = attrs.fixes; // expect an array of functions for possible fixes + + this.hash = attrs.hash; // an optional string to further differentiate the issue + + this.loc = function() { + if (this.coordinates && Array.isArray(this.coordinates) && this.coordinates.length === 2) { + return this.coordinates; + } + if (this.entities && this.entities.length > 0) { + if (this.entities[0].loc) { + return this.entities[0].loc; + } + } + }; + + if (this.fixes) { + for (var i=0; i 0) { return t('commit.outstanding_errors_message', { count: errorCount }); diff --git a/modules/ui/commit_warnings.js b/modules/ui/commit_warnings.js index a022e3b6c..d7ac748a2 100644 --- a/modules/ui/commit_warnings.js +++ b/modules/ui/commit_warnings.js @@ -13,7 +13,7 @@ export function uiCommitWarnings(context) { function commitWarnings(selection) { - var issues = context.issueManager().getIssues(); + var issues = context.validator().getIssues(); issues = _reduce(issues, function(issues, val) { var severity = val.severity; diff --git a/modules/ui/entity_issues.js b/modules/ui/entity_issues.js index 737fe0ad0..31b0bb990 100644 --- a/modules/ui/entity_issues.js +++ b/modules/ui/entity_issues.js @@ -16,7 +16,7 @@ export function uiEntityIssues(context) { var dispatch = d3_dispatch('change'); var _entityID; - context.issueManager().on('reload.entity_issues', update); + context.validator().on('reload.entity_issues', update); function update() { var selection = d3_select('.entity-issues .disclosure-wrap'); @@ -33,7 +33,7 @@ export function uiEntityIssues(context) { function render(selection) { - var issues = context.issueManager().getIssuesForEntityWithID(_entityID); + var issues = context.validator().getIssuesForEntityWithID(_entityID); if (issues.length > 0) { d3_select('.entity-issues') diff --git a/modules/ui/issues.js b/modules/ui/issues.js index ae6efa7a6..30d98e9ca 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -21,7 +21,7 @@ export function uiIssues(context) { var pane = d3_select(null); var _shown = false; - context.issueManager().on('reload.issues_pane', update); + context.validator().on('reload.issues_pane', update); function renderIssuesOptions(selection) { var container = selection.selectAll('.issues-options-container') @@ -100,7 +100,7 @@ export function uiIssues(context) { function drawIssuesList(selection) { - var issues = context.issueManager().getIssues(); + var issues = context.validator().getIssues(); /*validations = _reduce(issues, function(validations, val) { var severity = val.severity; @@ -175,11 +175,11 @@ export function uiIssues(context) { } function showsFeatureApplicability(d) { - return context.issueManager().getFeatureApplicability() === d; + return context.validator().getFeatureApplicability() === d; } function setFeatureApplicability(d) { - context.issueManager().setFeatureApplicability(d); + context.validator().setFeatureApplicability(d); update(); } @@ -187,7 +187,7 @@ export function uiIssues(context) { _featureApplicabilityList .call( drawListItems, - context.issueManager().featureApplicabilityOptions, + context.validator().featureApplicabilityOptions, 'radio', 'features_to_validate', setFeatureApplicability, diff --git a/modules/validations/highway_almost_junction.js b/modules/validations/almost_junction.js similarity index 92% rename from modules/validations/highway_almost_junction.js rename to modules/validations/almost_junction.js index df05e0f8e..1f6cb7aed 100644 --- a/modules/validations/highway_almost_junction.js +++ b/modules/validations/almost_junction.js @@ -19,13 +19,13 @@ import { ValidationIssueSeverity, validationIssue, validationIssueFix -} from './validation_issue'; +} from '../core/validator'; /** * Look for roads that can be connected to other roads with a short extension */ -export function validationHighwayAlmostJunction() { +export function validationAlmostJunction() { function isHighway(entity) { return entity.type === 'way' && entity.tags.highway && entity.tags.highway !== 'no'; @@ -146,13 +146,13 @@ export function validationHighwayAlmostJunction() { })); } issues.push(new validationIssue({ - type: ValidationIssueType.highway_almost_junction, + type: ValidationIssueType.almost_junction, severity: ValidationIssueSeverity.warning, - message: t('issues.highway_almost_junction.message', { - highway: utilDisplayLabel(endHighway, context), - highway2: utilDisplayLabel(edgeHighway, context) + message: t('issues.almost_junction.message', { + feature: utilDisplayLabel(endHighway, context), + feature2: utilDisplayLabel(edgeHighway, context) }), - tooltip: t('issues.highway_almost_junction.tip'), + tooltip: t('issues.almost_junction.highway-highway.tip'), entities: [endHighway, node, edgeHighway], coordinates: extendableNodes[j].node.loc, info: { @@ -166,7 +166,7 @@ export function validationHighwayAlmostJunction() { return issues; }; - validation.type = ValidationIssueType.highway_almost_junction; + validation.type = ValidationIssueType.almost_junction; return validation; } diff --git a/modules/validations/crossing_ways.js b/modules/validations/crossing_ways.js index 339d7813e..b258df00a 100644 --- a/modules/validations/crossing_ways.js +++ b/modules/validations/crossing_ways.js @@ -10,13 +10,13 @@ import { ValidationIssueSeverity, validationIssue, validationIssueFix -} from './validation_issue'; +} from '../core/validator'; import { osmNode } from '../osm'; import { actionAddMidpoint } from '../actions'; import { geoChooseEdge } from '../geo'; -export function validationHighwayCrossingOtherWays() { +export function validationCrossingWays() { // Check if the edge going from n1 to n2 crosses (without a connection node) // any edge on way. Return the corss point if so. function findEdgeToWayCrossCoords(n1, n2, way, graph, edgePairsVisited) { diff --git a/modules/validations/deprecated_tag.js b/modules/validations/deprecated_tag.js index 5a2664fe1..9affc6db5 100644 --- a/modules/validations/deprecated_tag.js +++ b/modules/validations/deprecated_tag.js @@ -9,7 +9,7 @@ import { ValidationIssueSeverity, validationIssue, validationIssueFix -} from './validation_issue'; +} from '../core/validator'; import { actionChangeTags } from '../actions'; @@ -82,7 +82,7 @@ export function validationDeprecatedTag() { return issues; }; - validation.type = ValidationIssueType.deprecated_tags; + validation.type = ValidationIssueType.deprecated_tag; return validation; } diff --git a/modules/validations/disconnected_highway.js b/modules/validations/disconnected_way.js similarity index 89% rename from modules/validations/disconnected_highway.js rename to modules/validations/disconnected_way.js index 3d32fd29e..ef68af26c 100644 --- a/modules/validations/disconnected_highway.js +++ b/modules/validations/disconnected_way.js @@ -7,11 +7,11 @@ import { ValidationIssueSeverity, validationIssue, validationIssueFix -} from './validation_issue'; +} from '../core/validator'; import { operationDelete } from '../operations/index'; import { modeDrawLine } from '../modes'; -export function validationDisconnectedHighway() { +export function validationDisconnectedWay() { function isDisconnectedHighway(entity, graph) { if (!entity.tags.highway) return false; @@ -39,10 +39,10 @@ export function validationDisconnectedHighway() { var entityLabel = utilDisplayLabel(entity, context); issues.push(new validationIssue({ - type: ValidationIssueType.disconnected_highway, + type: ValidationIssueType.disconnected_way, severity: ValidationIssueSeverity.warning, - message: t('issues.disconnected_highway.message', {highway: entityLabel}), - tooltip: t('issues.disconnected_highway.tip'), + message: t('issues.disconnected_way.highway.message', { highway: entityLabel }), + tooltip: t('issues.disconnected_way.highway.tip'), entities: [entity], fixes: [ new validationIssueFix({ @@ -81,7 +81,7 @@ export function validationDisconnectedHighway() { return issues; }; - validation.type = ValidationIssueType.disconnected_highway; + validation.type = ValidationIssueType.disconnected_way; return validation; } diff --git a/modules/validations/generic_name.js b/modules/validations/generic_name.js index ff57024eb..365c8f60e 100644 --- a/modules/validations/generic_name.js +++ b/modules/validations/generic_name.js @@ -8,7 +8,7 @@ import { ValidationIssueSeverity, validationIssue, validationIssueFix -} from './validation_issue'; +} from '../core/validator'; import { actionChangeTags } from '../actions'; diff --git a/modules/validations/index.js b/modules/validations/index.js index c8e8a0b91..6dca7c307 100644 --- a/modules/validations/index.js +++ b/modules/validations/index.js @@ -1,11 +1,10 @@ export { validationDeprecatedTag } from './deprecated_tag'; -export { validationDisconnectedHighway } from './disconnected_highway'; -export { validationHighwayCrossingOtherWays } from './crossing_ways'; -export { validationHighwayAlmostJunction } from './highway_almost_junction'; -export { ValidationIssueType, ValidationIssueSeverity } from './validation_issue'; +export { validationDisconnectedWay } from './disconnected_way'; +export { validationCrossingWays } from './crossing_ways'; +export { validationAlmostJunction } from './almost_junction'; export { validationGenericName } from './generic_name.js'; export { validationManyDeletions } from './many_deletions'; -export { validationMapCSSChecks } from './mapcss_checks'; +export { validationMaprules } from './maprules'; export { validationMissingTag } from './missing_tag'; export { validationOldMultipolygon } from './old_multipolygon'; export { validationTagSuggestsArea } from './tag_suggests_area'; diff --git a/modules/validations/issue_manager.js b/modules/validations/issue_manager.js deleted file mode 100644 index 1251cc2a5..000000000 --- a/modules/validations/issue_manager.js +++ /dev/null @@ -1,129 +0,0 @@ -import { dispatch as d3_dispatch } from 'd3-dispatch'; - -import _map from 'lodash-es/map'; -import _flatten from 'lodash-es/flatten'; -import _flattenDeep from 'lodash-es/flattenDeep'; -import _uniq from 'lodash-es/uniq'; -import _uniqWith from 'lodash-es/uniqWith'; - -import { utilRebind } from '../util/rebind'; -import * as validations from '../validations/index'; - -export function IssueManager(context) { - var dispatch = d3_dispatch('reload'), - self = {}, - issues = [], - issuesByEntityId = {}; - - self.featureApplicabilityOptions = ['edited', 'all']; - - var featureApplicability = context.storage('issue-features') || 'edited'; - - self.getFeatureApplicability = function() { - return featureApplicability; - }; - - self.setFeatureApplicability = function(applicability) { - featureApplicability = applicability; - context.storage('issue-features', applicability); - }; - - self.getIssues = function() { - return issues; - }; - - self.getWarnings = function() { - return issues.filter(function(issue) { - return issue.severity === 'warning'; - }); - }; - self.getErrors = function() { - return issues.filter(function(issue) { - return issue.severity === 'error'; - }); - }; - - self.getIssuesForEntityWithID = function(entityID) { - if (!context.hasEntity(entityID)) { - return []; - } - if (!issuesByEntityId[entityID]) { - var entity = context.entity(entityID); - issuesByEntityId[entityID] = validateEntity(entity); - } - return issuesByEntityId[entityID]; - }; - - var genericEntityValidations = [ - validations.validationDeprecatedTag(), - validations.validationGenericName(), - validations.validationMapCSSChecks(), - validations.validationOldMultipolygon() - ]; - - function validateEntity(entity) { - var issues = []; - // runs validation and appends resulting issues, returning true if validation passed - function runValidation(fn) { - var typeIssues = fn(entity, context); - issues = issues.concat(typeIssues); - return typeIssues.length === 0; - } - // other validations require feature to be tagged - if (!runValidation(validations.validationMissingTag())) return issues; - if (entity.type === 'way') { - if (runValidation(validations.validationHighwayAlmostJunction())) { - // only check for disconnected highway if no almost junctions - runValidation(validations.validationDisconnectedHighway()); - } - runValidation(validations.validationHighwayCrossingOtherWays()); - runValidation(validations.validationTagSuggestsArea()); - } - genericEntityValidations.forEach(function(fn) { - runValidation(fn); - }) - return issues; - } - - self.validate = function() { - // clear cached issues - issuesByEntityId = {}; - issues = []; - - var history = context.history(); - var changes = history.changes(); - var entitiesToCheck = changes.created.concat(changes.modified); - var graph = history.graph(); - - issues = issues.concat(validations.validationManyDeletions()(changes, context)); - - entitiesToCheck = _uniq(_flattenDeep(_map(entitiesToCheck, function(entity) { - var entities = [entity]; - if (entity.type === 'node') { - // validate ways if their nodes have changed - entities = entities.concat(graph.parentWays(entity)); - } - entities = _map(entities, function(entity) { - if (entity.type !== 'relation') { - // validate relations if their geometries have changed - return [entity].concat(graph.parentRelations(entity)); - } - return entity; - }); - return entities; - }))); - - for (var entityIndex in entitiesToCheck) { - var entity = entitiesToCheck[entityIndex]; - var entityIssues = validateEntity(entity); - issuesByEntityId[entity.id] = entityIssues; - issues = issues.concat(entityIssues); - } - issues = _uniqWith(issues, function(issue1, issue2) { - return issue1.id() === issue2.id(); - }); - dispatch.call('reload', self, issues); - }; - - return utilRebind(self, dispatch, 'on'); -} diff --git a/modules/validations/many_deletions.js b/modules/validations/many_deletions.js index e570c6e10..fe32c1c95 100644 --- a/modules/validations/many_deletions.js +++ b/modules/validations/many_deletions.js @@ -3,7 +3,7 @@ import { ValidationIssueType, ValidationIssueSeverity, validationIssue, -} from './validation_issue'; +} from '../core/validator'; export function validationManyDeletions() { @@ -35,7 +35,7 @@ export function validationManyDeletions() { return issues; }; - validation.type = ValidationIssueType.map_rule_issue; + validation.type = ValidationIssueType.many_deletions; return validation; } diff --git a/modules/validations/mapcss_checks.js b/modules/validations/maprules.js similarity index 78% rename from modules/validations/mapcss_checks.js rename to modules/validations/maprules.js index e8df9d3ea..b5320a38a 100644 --- a/modules/validations/mapcss_checks.js +++ b/modules/validations/maprules.js @@ -1,8 +1,8 @@ import { services } from '../services'; import { ValidationIssueType -} from './validation_issue'; -export function validationMapCSSChecks() { +} from '../core/validator'; +export function validationMaprules() { var validation = function(entity, context) { if (!services.maprules) return []; @@ -19,7 +19,7 @@ export function validationMapCSSChecks() { return issues; }; - validation.type = ValidationIssueType.map_rule_issue; + validation.type = ValidationIssueType.maprules; return validation; } diff --git a/modules/validations/missing_tag.js b/modules/validations/missing_tag.js index c2d9aefb0..c9cd830a2 100644 --- a/modules/validations/missing_tag.js +++ b/modules/validations/missing_tag.js @@ -9,7 +9,7 @@ import { ValidationIssueSeverity, validationIssue, validationIssueFix -} from './validation_issue'; +} from '../core/validator'; import { operationDelete } from '../operations/index'; export function validationMissingTag() { diff --git a/modules/validations/old_multipolygon.js b/modules/validations/old_multipolygon.js index e982e8ab1..5dea893f9 100644 --- a/modules/validations/old_multipolygon.js +++ b/modules/validations/old_multipolygon.js @@ -6,7 +6,7 @@ import { ValidationIssueSeverity, validationIssue, validationIssueFix -} from './validation_issue'; +} from '../core/validator'; import { actionChangeTags } from '../actions'; diff --git a/modules/validations/tag_suggests_area.js b/modules/validations/tag_suggests_area.js index f93f0ac49..c7b6be389 100644 --- a/modules/validations/tag_suggests_area.js +++ b/modules/validations/tag_suggests_area.js @@ -9,7 +9,7 @@ import { ValidationIssueSeverity, validationIssue, validationIssueFix -} from './validation_issue'; +} from '../core/validator'; import { actionChangeTags } from '../actions'; @@ -39,7 +39,6 @@ export function validationTagSuggestsArea() { return false; } - var validation = function(entity, context) { var issues = []; var graph = context.graph(); diff --git a/modules/validations/validation_issue.js b/modules/validations/validation_issue.js deleted file mode 100644 index 7cae7c91a..000000000 --- a/modules/validations/validation_issue.js +++ /dev/null @@ -1,101 +0,0 @@ -import _isObject from 'lodash-es/isObject'; -import _map from 'lodash-es/map'; -import { osmEntity } from '../osm'; - -var ValidationIssueType = Object.freeze({ - deprecated_tags: 'deprecated_tags', - disconnected_highway: 'disconnected_highway', - many_deletions: 'many_deletions', - missing_tag: 'missing_tag', - old_multipolygon: 'old_multipolygon', - tag_suggests_area: 'tag_suggests_area', - map_rule_issue: 'map_rule_issue', - crossing_ways: 'crossing_ways', - highway_almost_junction: 'highway_almost_junction', - generic_name: 'generic_name' -}); - - -var ValidationIssueSeverity = Object.freeze({ - warning: 'warning', - error: 'error', -}); - - -export { ValidationIssueType, ValidationIssueSeverity }; - - -export function validationIssue(attrs) { - - // A unique, deterministic string hash. - // Issues with identical id values are considered identical. - this.id = function () { - var id = this.type; - - if (this.hash) { - // subclasses can pass in their own differentiator - id += this.hash; - } - - // issue subclasses set the entity order but it must be deterministic - var entityKeys = _map(this.entities, function(entity) { - // use the key since it factors in the entity's local version - return osmEntity.key(entity); - }); - // factor in the entities this issue is for - id += entityKeys.join(); - if (this.coordinates) { - // factor in coordinates since two separate issues can have an - // idential type and entities, e.g. in crossing_ways - id += this.coordinates.join(); - } - return id; - }; - - if (!_isObject(attrs)) throw new Error('Input attrs is not an object'); - if (!attrs.type || !ValidationIssueType.hasOwnProperty(attrs.type)) { - throw new Error('Invalid attrs.type: ' + attrs.type); - } - if (!attrs.severity || !ValidationIssueSeverity.hasOwnProperty(attrs.severity)) { - throw new Error('Invalid attrs.severity: ' + attrs.severity); - } - if (!attrs.message) throw new Error('attrs.message is empty'); - - this.type = attrs.type; - this.severity = attrs.severity; - this.message = attrs.message; - this.tooltip = attrs.tooltip; - this.entities = attrs.entities; // expect an array of entities - this.coordinates = attrs.coordinates; // expect a [lon, lat] array - this.info = attrs.info; // an object containing arbitrary extra information - this.fixes = attrs.fixes; // expect an array of functions for possible fixes - - this.hash = attrs.hash; // an optional string to further differentiate the issue - - this.loc = function() { - if (this.coordinates && Array.isArray(this.coordinates) && this.coordinates.length === 2) { - return this.coordinates; - } - if (this.entities && this.entities.length > 0) { - if (this.entities[0].loc) { - return this.entities[0].loc; - } - } - }; - - if (this.fixes) { - for (var i=0; i