WIP: switch from validating everything to validating differences

This commit is contained in:
Bryan Housel
2019-04-05 13:29:57 -04:00
parent af45dbce10
commit 78acd999c8
4 changed files with 74 additions and 61 deletions
+11 -9
View File
@@ -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).
+6 -2
View File
@@ -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;
};
+54 -49
View File
@@ -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');
+3 -1
View File
@@ -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;