diff --git a/modules/core/context.js b/modules/core/context.js index 582e148a5..e45f46825 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -55,6 +55,7 @@ export function coreContext() { setLocale('en'); var dispatch = d3_dispatch('enter', 'exit', 'change'); + context = utilRebind(context, dispatch, 'on'); // https://github.com/openstreetmap/iD/issues/772 // http://mathiasbynens.be/notes/localstorage-pattern#comment-9 @@ -457,6 +458,7 @@ export function coreContext() { } history = coreHistory(context); + validator = coreValidator(context); context.graph = history.graph; context.changes = history.changes; @@ -464,25 +466,6 @@ export function coreContext() { context.pauseChangeDispatch = history.pauseChangeDispatch; context.resumeChangeDispatch = history.resumeChangeDispatch; - validator = coreValidator(context); - - // run validation upon restoring from page reload - history.on('restore', function() { - validator.validate(); - }); - // re-run validation upon a significant graph change - history.on('change', function(difference) { - if (difference) { - validator.validate(difference); - } - }); - // re-run validation upon merging fetched data - // history.on('merge', function(entities) { - // if (entities && entities.length > 0) { - // validator.validate(); - // } - // }); - // Debounce save, since it's a synchronous localStorage write, // and history changes can happen frequently (e.g. when dragging). context.debouncedSave = _debounce(context.save, 350); @@ -552,5 +535,5 @@ export function coreContext() { areaKeys = presets.areaKeys(); } - return utilRebind(context, dispatch, 'on'); + return context; } diff --git a/modules/core/validator.js b/modules/core/validator.js index a0efcf58c..25b0b9b0c 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -1,5 +1,6 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; +import { coreDifference } from './difference'; import { geoExtent } from '../geo'; import { osmEntity } from '../osm'; import { utilRebind } from '../util'; @@ -7,7 +8,7 @@ import * as Validations from '../validations/index'; export function coreValidator(context) { - var dispatch = d3_dispatch('reload'); + var dispatch = d3_dispatch('validated'); var validator = {}; var _rules = {}; @@ -17,9 +18,12 @@ export function coreValidator(context) { var _issuesByIssueID = {}; var _issuesByEntityID = {}; + var _validatedGraph = null; - + // + // initialize the validator rulesets + // validator.init = function() { Object.values(Validations).forEach(function(validation) { if (typeof validation !== 'function') return; @@ -37,10 +41,14 @@ export function coreValidator(context) { }; + // + // reset caches, called whenever iD resets after a save + // validator.reset = function() { // clear caches _issuesByIssueID = {}; _issuesByEntityID = {}; + _validatedGraph = null; for (var key in _rules) { if (typeof _rules[key].reset === 'function') { @@ -93,7 +101,25 @@ export function coreValidator(context) { }; - validator.validateEntity = function(entity) { + // + // Remove a single entity from the caches + // + function uncacheEntity(entity) { + var issues = _issuesByEntityID[entity.id] || []; + issues.forEach(function(issue) { + var entities = issue.entities || []; + entities.forEach(function(entity) { + delete _issuesByEntityID[entity.id]; + }); + delete _issuesByIssueID[issue.id]; + }); + } + + + // + // Run validation on a single entity + // + function validateEntity(entity) { var entityIssues = []; var ran = {}; @@ -152,36 +178,26 @@ export function coreValidator(context) { _entityRules.forEach(runValidation); return entityIssues; - }; + } - validator.validate = function(difference) { - difference = difference || context.history().difference(); - - for (var key in _rules) { - if (typeof _rules[key].reset === 'function') { - _rules[key].reset(); // 'crossing_ways' is the only one like this - } - } - - // _issues = utilArrayFlatten(_changesRules.map(function(ruleID) { - // if (_disabledRules[ruleID]) return []; - // var fn = _rules[ruleID]; - // return fn(changes, context); - // })); - - var graph = context.graph(); - var entityIDs = difference.extantIDs(); // created and modified + // + // Run validation for several entities, supplied `entityIDs` + // It uses the "validatedGraph", so this can be called asynchronously + // as entities are loaded from OSM if we want to. + // + validator.validateEntities = function(entityIDs) { + _validatedGraph = _validatedGraph || context.history().base(); var entitiesToCheck = entityIDs.reduce(function(acc, entityID) { - var entity = graph.entity(entityID); + var entity = _validatedGraph.entity(entityID); if (acc.has(entity)) return acc; acc.add(entity); var checkParentRels = [entity]; if (entity.type === 'node') { // include parent ways - graph.parentWays(entity).forEach(function(parentWay) { + _validatedGraph.parentWays(entity).forEach(function(parentWay) { checkParentRels.push(parentWay); acc.add(parentWay); }); @@ -189,7 +205,7 @@ export function coreValidator(context) { checkParentRels.forEach(function(entity) { // include parent relations if (entity.type !== 'relation') { // but not super-relations - graph.parentRelations(entity).forEach(function(parentRel) { + _validatedGraph.parentRelations(entity).forEach(function(parentRel) { acc.add(parentRel); }); } @@ -199,15 +215,12 @@ export function coreValidator(context) { }, new Set()); - // clear caches for existing issues related to changed entities - difference.deleted().forEach(clearCaches); - entitiesToCheck.forEach(clearCaches); - + entitiesToCheck.forEach(uncacheEntity); // detect new issues and update caches entitiesToCheck.forEach(function(entity) { - var issues = validator.validateEntity(entity); + var issues = validateEntity(entity); issues.forEach(function(issue) { var entities = issue.entities || []; @@ -221,26 +234,72 @@ export function coreValidator(context) { }); }); - dispatch.call('reload'); + dispatch.call('validated'); }; + + // + // Validates anything that has changed since the last time it was run. + // Also updates the "validatedGraph" to be the current graph + // and dispatches a `validated` event when finished. + // + validator.validate = function() { + var currGraph = context.graph(); + _validatedGraph = _validatedGraph || context.history().base(); + if (currGraph === _validatedGraph) return; + + var difference = coreDifference(_validatedGraph, currGraph); + _validatedGraph = currGraph; + + for (var key in _rules) { + if (typeof _rules[key].reset === 'function') { + _rules[key].reset(); // 'crossing_ways' is the only one like this + } + } + + // _issues = utilArrayFlatten(_changesRules.map(function(ruleID) { + // if (_disabledRules[ruleID]) return []; + // var fn = _rules[ruleID]; + // return fn(changes, context); + // })); + + var entityIDs = difference.extantIDs(); // created and modified + difference.deleted().forEach(uncacheEntity); // deleted + + validator.validateEntities(entityIDs); // dispatches 'validated' + }; + + + + // run validation upon restoring user's changes + context.history().on('restore.validator', function() { + validator.validate(); + }); + + // re-run validation upon merging fetched data + context.history().on('merge.validator', function(entities) { + if (!entities) return; + var ids = entities.map(function(entity) { return entity.id; }); + validator.validateEntities(ids); + }); + + // // re-run validation on history change (frequent) + // context.history().on('change.validator', function(difference) { + // if (!difference) return; + // validator.validate(); + // }); + + // re-run validation when the user switches editing modes (less frequent) + context.on('exit.validator', function() { + validator.validate(); + }); + + return utilRebind(validator, dispatch, 'on'); - - - - function clearCaches(entity) { - var issues = _issuesByEntityID[entity.id] || []; - issues.forEach(function(issue) { - var entities = issue.entities || []; - entities.forEach(function(entity) { - delete _issuesByEntityID[entity.id]; - }); - delete _issuesByIssueID[issue.id]; - }); - } } + export function validationIssue(attrs) { this.type = attrs.type; // required this.severity = attrs.severity; // required - 'warning' or 'error' diff --git a/modules/ui/entity_issues.js b/modules/ui/entity_issues.js index 48d373532..fa202eabc 100644 --- a/modules/ui/entity_issues.js +++ b/modules/ui/entity_issues.js @@ -13,10 +13,10 @@ export function uiEntityIssues(context) { var _expandedIssueID; var _entityID; - // Listen for validation reload even though the entity editor is reloaded on + // Listen for validation event even though the entity editor is reloaded on // every graph change since the graph change event may happen before the issue // cache is refreshed - context.validator().on('reload.entity_issues', function() { + context.validator().on('validated.entity_issues', function() { _selection.selectAll('.disclosure-wrap-entity_issues') .call(render); diff --git a/modules/ui/issues.js b/modules/ui/issues.js index 878f5363e..cef657035 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -1,7 +1,4 @@ -import { - event as d3_event, - select as d3_select -} from 'd3-selection'; +import { event as d3_event, select as d3_select } from 'd3-selection'; import { svgIcon } from '../svg'; import { t, textDirection } from '../util/locale'; @@ -24,7 +21,7 @@ export function uiIssues(context) { var _toggleButton = d3_select(null); var _shown = false; - context.validator().on('reload.issues_pane', update); + context.validator().on('validated.issues_pane', update); /*function renderIssuesOptions(selection) { var container = selection.selectAll('.issues-options-container')