diff --git a/modules/core/context.js b/modules/core/context.js index fbca3ff7f..864b8ef04 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -466,22 +466,24 @@ export function coreContext() { validator = coreValidator(context); +// todo: put these back in somehow, +// but for now try to just validate an edit difference // run validation upon restoring from page reload - history.on('restore', function() { - validator.validate(); - }); + // history.on('restore', function() { + // validator.validate(); + // }); // re-run validation upon a significant graph change history.on('change', function(difference) { if (difference) { - validator.validate(); + validator.validate(difference); } }); // re-run validation upon merging fetched data - history.on('merge', function(entities) { - if (entities && entities.length > 0) { - validator.validate(); - } - }); + // 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). diff --git a/modules/core/difference.js b/modules/core/difference.js index 940df480a..c51c8ed7b 100644 --- a/modules/core/difference.js +++ b/modules/core/difference.js @@ -100,7 +100,9 @@ export function coreDifference(base, head) { _diff.created = function created() { var result = []; Object.values(_changes).forEach(function(change) { - if (!change.base && change.head) result.push(change.head); + if (!change.base && change.head) { + result.push(change.head); + } }); return result; }; @@ -109,7 +111,9 @@ export function coreDifference(base, head) { _diff.deleted = function deleted() { var result = []; Object.values(_changes).forEach(function(change) { - if (change.base && !change.head) result.push(change.base); + if (change.base && !change.head) { + result.push(change.base); + } }); return result; }; diff --git a/modules/core/validator.js b/modules/core/validator.js index 808457608..d3e899066 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -2,7 +2,7 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; import { geoExtent } from '../geo'; import { osmEntity } from '../osm'; -import { utilArrayFlatten, utilRebind } from '../util'; +import { utilRebind } from '../util'; import * as Validations from '../validations/index'; @@ -15,7 +15,8 @@ export function coreValidator(context) { var _entityRules = []; // var _changesRules = []; // skip for now - var _issues = []; + // var _issues = []; + var _issuesByIssueID = {}; var _issuesByEntityID = {}; @@ -39,8 +40,10 @@ export function coreValidator(context) { validator.reset = function() { // clear caches - _issues = []; + // _issues = []; + _issuesByIssueID = {}; _issuesByEntityID = {}; + for (var key in _rules) { if (typeof _rules[key].reset === 'function') { _rules[key].reset(); // 'crossing_ways' is the only one like this @@ -50,30 +53,33 @@ export function coreValidator(context) { validator.getIssues = function() { - return _issues; + return Object.values(_issuesByIssueID); + // return _issues; }; validator.getWarnings = function() { - return _issues + // return _issues + return Object.values(_issuesByIssueID) .filter(function(d) { return d.severity === 'warning'; }); }; validator.getErrors = function() { - return _issues + // return _issues + return Object.values(_issuesByIssueID) .filter(function(d) { return d.severity === 'error'; }); }; validator.getEntityIssues = function(entityID) { - var entity = context.hasEntity(entityID); - if (!entity) return []; + // var entity = context.hasEntity(entityID); + // if (!entity) return []; - if (!_issuesByEntityID[entityID]) { - _issuesByEntityID[entityID] = validateEntity(entity); - } - return _issuesByEntityID[entityID]; + // if (!_issuesByEntityID[entityID]) { + // _issuesByEntityID[entityID] = validator.validateEntity(entity); + // } + return _issuesByEntityID[entityID] || []; }; @@ -98,8 +104,8 @@ export function coreValidator(context) { }; - function validateEntity(entity) { - var _issues = []; + validator.validateEntity = function(entity) { + var entityIssues = []; var ran = {}; // runs validation and appends resulting issues, @@ -114,14 +120,14 @@ export function coreValidator(context) { return true; } - if (_disabledRules[key]) { // don't run disabled, but mark as having run + if (_disabledRules[key]) { // skip disabled rules, but mark as having run ran[key] = true; return true; } var detected = fn(entity, context); - _issues = _issues.concat(detected); - ran[key] = true; // mark this validation as having run + entityIssues = entityIssues.concat(detected); + ran[key] = true; return !detected.length; } @@ -135,7 +141,7 @@ export function coreValidator(context) { } // other _rules require feature to be tagged - if (!runValidation('missing_tag')) return _issues; + if (!runValidation('missing_tag')) return entityIssues; // run outdated_tags early runValidation('outdated_tags'); @@ -156,46 +162,44 @@ export function coreValidator(context) { // run all _rules not yet run manually _entityRules.forEach(runValidation); - return _issues; - } + return entityIssues; + }; - validator.validate = function() { - // clear caches - _issuesByEntityID = {}; - _issues = []; + 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 } } - var history = context.history(); - var changes = history.changes(); - var changesToCheck = changes.created.concat(changes.modified); - var graph = history.graph(); - // _issues = utilArrayFlatten(_changesRules.map(function(ruleID) { // if (_disabledRules[ruleID]) return []; // var fn = _rules[ruleID]; // return fn(changes, context); // })); - var entitiesToCheck = changesToCheck.reduce(function(acc, entity) { - var entities = [entity]; - acc.add(entity); + var graph = context.graph(); + var entityIDs = difference.extantIDs(); - if (entity.type === 'node') { - // check parent ways if their nodes have changed + var entitiesToCheck = entityIDs.reduce(function(acc, entityID) { + var entity = graph.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) { - entities.push(parentWay); + checkParentRels.push(parentWay); acc.add(parentWay); }); } - entities.forEach(function(entity) { - // check parent relations if their geometries have changed - if (entity.type !== 'relation') { + checkParentRels.forEach(function(entity) { // include parent relations + if (entity.type !== 'relation') { // but not super-relations graph.parentRelations(entity).forEach(function(parentRel) { acc.add(parentRel); }); @@ -203,27 +207,28 @@ export function coreValidator(context) { }); return acc; - }, new Set()); - var issuesByID = {}; + // var issuesByID = {}; entitiesToCheck.forEach(function(entity) { - var entityIssues = validateEntity(entity); - _issuesByEntityID[entity.id] = entityIssues; - entityIssues.forEach(function(issue) { + var detected = validator.validateEntity(entity); + _issuesByEntityID[entity.id] = detected; + detected.forEach(function(d) { _issuesByIssueID[d.id] = d; }); + // entityIssues.forEach(function(issue) { // Different entities can produce the same issue so store them by // the ID to ensure that there are no duplicate issues. - issuesByID[issue.id()] = issue; - }); + // issuesByID[issue.id()] = issue; + // }); }); - for (var issueID in issuesByID) { - _issues.push(issuesByID[issueID]); - } + // for (var issueID in issuesByID) { + // _issues.push(issuesByID[issueID]); + // } - dispatch.call('reload', validator, _issues); + // dispatch.call('reload', validator, _issues); + dispatch.call('reload'); }; return utilRebind(validator, dispatch, 'on'); diff --git a/modules/ui/entity_editor.js b/modules/ui/entity_editor.js index 6690297c0..21e190880 100644 --- a/modules/ui/entity_editor.js +++ b/modules/ui/entity_editor.js @@ -311,11 +311,13 @@ export function uiEntityEditor(context) { entityEditor.entityID = function(val) { if (!arguments.length) return _entityID; + if (_entityID === val) return entityEditor; // exit early if no change + _entityID = val; _base = context.graph(); _coalesceChanges = false; - // reset the scroll to the top of the inspector + // reset the scroll to the top of the inspector (warning: triggers reflow) var body = d3_selectAll('.entity-editor-pane .inspector-body'); if (!body.empty()) { body.node().scrollTop = 0;