CalvinBackup/iD
1
0
Fork 0
mirror of https://github.com/FoggedLens/iD.git synced 2026-05-23 08:39:56 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #8663 from openstreetmap/issue_key_8655

Browse Source 
More validator fixes
This commit is contained in:
Milos Brzakovic
2021-09-14 11:34:58 +02:00
committed by GitHub
parent 9a26ae3e9e 78961072a7
commit f08b89f4c3
6 changed files with 84 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/core/validation/models.js
+8
View File
@@ -14,6 +14,7 @@ export function validationIssue(attrs) {
this.hash = attrs.hash; // optional - string to further differentiate the issue
this.id = generateID.apply(this); // generated - see below
this.key = generateKey.apply(this); // generated - see below (call after generating this.id)
this.autoFix = null; // generated - if autofix exists, will be set below
// A unique, deterministic string hash.
@@ -39,6 +40,13 @@ export function validationIssue(attrs) {
return parts.join(':');
}
// An identifier suitable for use as the second argument to d3.selection#data().
// (i.e. this should change whenever the data needs to be refreshed)
function generateKey() {
return this.id + ':' + Date.now().toString(); // include time of creation
}
this.extent = function(resolver) {
if (this.loc) {
return geoExtent(this.loc);
modules/core/validator.js
+68 -35
View File
@@ -189,16 +189,22 @@ export function coreValidator(context) {
let seen = new Set();
let results = [];
// collect head issues - caused by user edits
// collect head issues - present in the user edits
if (_headCache.graph && _headCache.graph !== _baseCache.graph) {
Object.values(_headCache.issuesByIssueID).forEach(issue => {
// In the head cache, only count features that the user is responsible for - #8632
// For example, a user can undo some work and an issue will still present in the
// head graph, but we don't want to credit the user for causing that issue.
const userModified = (issue.entityIds || []).some(id => _completeDiff.hasOwnProperty(id));
if (opts.what === 'edited' && !userModified) return; // present in head but user didn't touch it
if (!filter(issue)) return;
seen.add(issue.id);
results.push(issue);
});
}
// collect base issues - not caused by user edits
// collect base issues - present before user edits
if (opts.what === 'all') {
Object.values(_baseCache.issuesByIssueID).forEach(issue => {
if (!filter(issue)) return;
@@ -488,10 +494,10 @@ export function coreValidator(context) {
_headCache.graph = currGraph; // take snapshot
_completeDiff = context.history().difference().complete();
const incrementalDiff = coreDifference(prevGraph, currGraph);
const entityIDs = Object.keys(incrementalDiff.complete());
let entityIDs = Object.keys(incrementalDiff.complete());
entityIDs = _headCache.withAllRelatedEntities(entityIDs); // expand set
// if (!entityIDs.size) {
if (!entityIDs.length) {
if (!entityIDs.size) {
dispatch.call('validated');
return Promise.resolve();
}
@@ -540,8 +546,8 @@ export function coreValidator(context) {
if (!_headCache.graph) _headCache.graph = baseGraph;
if (!_baseCache.graph) _baseCache.graph = baseGraph;
const entityIDs = entities.map(entity => entity.id);
// entityIDs = entityIDsToValidate(entityIDs, baseGraph); // expand set
let entityIDs = entities.map(entity => entity.id);
entityIDs = _baseCache.withAllRelatedEntities(entityIDs); // expand set
validateEntitiesAsync(entityIDs, _baseCache);
});
@@ -629,7 +635,7 @@ export function coreValidator(context) {
// - the user did something to one of the entities involved in the issue
//
// Arguments
// `entityIDs` - Array containing entity IDs.
// `entityIDs` - Array or Set containing entity IDs.
//
function updateResolvedIssues(entityIDs) {
entityIDs.forEach(entityID => {
@@ -656,7 +662,7 @@ export function coreValidator(context) {
// Schedule validation for many entities.
//
// Arguments
// `entityIDs` - Array containing entity IDs.
// `entityIDs` - Array or Set containing entityIDs.
// `graph` - the graph to validate that contains those entities
// `cache` - the cache to store results in (_headCache or _baseCache)
//
@@ -666,13 +672,14 @@ export function coreValidator(context) {
//
function validateEntitiesAsync(entityIDs, cache) {
// Enqueue the work
const jobs = entityIDs.map(entityID => {
const jobs = Array.from(entityIDs).map(entityID => {
if (cache.queuedEntityIDs.has(entityID)) return null; // queued already
cache.queuedEntityIDs.add(entityID);
// Clear caches for existing issues related to this entity
cache.uncacheEntityID(entityID);
return () => {
// Clear caches for existing issues related to this entity
cache.uncacheEntityID(entityID);
cache.queuedEntityIDs.delete(entityID);
const graph = cache.graph;
@@ -681,11 +688,6 @@ export function coreValidator(context) {
const entity = graph.hasEntity(entityID); // Sanity check: don't validate deleted entities
if (!entity) return;
// In the head cache, only validate features that the user is responsible for - #8632
// For example, a user can undo some work and an issue will still present in the
// head graph, but we don't want to credit the user for causing that issue.
if (cache.which === 'head' && !_completeDiff.hasOwnProperty(entityID)) return;
// detect new issues and update caches
const result = validateEntity(entity, graph);
if (result.provisional) { // provisional result
@@ -795,24 +797,20 @@ function validationCache(which) {
issuesByEntityID: {} // entity.id -> Set(issue.id)
};
cache.cacheIssues = (issues) => {
issues.forEach(issue => {
const entityIDs = issue.entityIds || [];
entityIDs.forEach(entityID => {
if (!cache.issuesByEntityID[entityID]) {
cache.issuesByEntityID[entityID] = new Set();
}
cache.issuesByEntityID[entityID].add(issue.id);
});
cache.issuesByIssueID[issue.id] = issue;
cache.cacheIssue = (issue) => {
(issue.entityIds || []).forEach(entityID => {
if (!cache.issuesByEntityID[entityID]) {
cache.issuesByEntityID[entityID] = new Set();
}
cache.issuesByEntityID[entityID].add(issue.id);
});
cache.issuesByIssueID[issue.id] = issue;
};
cache.uncacheIssue = (issue) => {
// When multiple entities are involved (e.g. crossing_ways),
// remove this issue from the other entity caches too..
const entityIDs = issue.entityIds || [];
entityIDs.forEach(entityID => {
(issue.entityIds || []).forEach(entityID => {
if (cache.issuesByEntityID[entityID]) {
cache.issuesByEntityID[entityID].delete(issue.id);
}
@@ -820,25 +818,33 @@ function validationCache(which) {
delete cache.issuesByIssueID[issue.id];
};
cache.cacheIssues = (issues) => {
issues.forEach(cache.cacheIssue);
};
cache.uncacheIssues = (issues) => {
issues.forEach(cache.uncacheIssue);
};
cache.uncacheIssuesOfType = (type) => {
const issuesOfType = Object.values(cache.issuesByIssueID)
.filter(issue => issue.type === type);
cache.uncacheIssues(issuesOfType);
};
// Remove a single entity and all its related issues from the caches
cache.uncacheEntityID = (entityID) => {
const issueIDs = cache.issuesByEntityID[entityID];
if (issueIDs) {
issueIDs.forEach(issueID => {
const entityIssueIDs = cache.issuesByEntityID[entityID];
if (entityIssueIDs) {
entityIssueIDs.forEach(issueID => {
const issue = cache.issuesByIssueID[issueID];
if (issue) {
cache.uncacheIssue(issue);
} else {
} else { // shouldnt happen, clean up
delete cache.issuesByIssueID[issueID];
}
});
@@ -849,5 +855,32 @@ function validationCache(which) {
};
// Return the expandeded set of entityIDs related to issues for the given entityIDs
//
// Arguments
// `entityIDs` - Array or Set containing entityIDs.
//
cache.withAllRelatedEntities = (entityIDs) => {
let result = new Set();
(entityIDs || []).forEach(entityID => {
result.add(entityID); // include self
const entityIssueIDs = cache.issuesByEntityID[entityID];
if (entityIssueIDs) {
entityIssueIDs.forEach(issueID => {
const issue = cache.issuesByIssueID[issueID];
if (issue) {
(issue.entityIds || []).forEach(relatedID => result.add(relatedID));
} else { // shouldnt happen, clean up
delete cache.issuesByIssueID[issueID];
}
});
}
});
return result;
};
return cache;
}
modules/modes/select.js
+5 -1
View File
@@ -429,11 +429,14 @@ export function modeSelect(context, selectedIDs) {
actionAddMidpoint({ loc: choice.loc, edge: [prev, next] }, osmNode()),
t('operations.add.annotation.vertex')
);
context.validator().validate();
} else if (entity.type === 'midpoint') {
context.perform(
actionAddMidpoint({ loc: entity.loc, edge: entity.edge }, osmNode()),
t('operations.add.annotation.vertex'));
t('operations.add.annotation.vertex')
);
context.validator().validate();
}
}
@@ -688,6 +691,7 @@ export function modeSelect(context, selectedIDs) {
// the user added this relation but didn't edit it at all, so just delete it
var deleteAction = actionDeleteRelation(entity.id, true /* don't delete untagged members */);
context.perform(deleteAction, t('operations.delete.annotation.relation'));
context.validator().validate();
}
};
modules/ui/commit_warnings.js
+1 -1
View File
@@ -43,7 +43,7 @@ export function uiCommitWarnings(context) {
var items = container.select('ul').selectAll('li')
.data(issues, function(d) { return d.id; });
.data(issues, function(d) { return d.key; });
items.exit()
.remove();
modules/ui/sections/entity_issues.js
+1 -1
View File
@@ -55,7 +55,7 @@ export function uiSectionEntityIssues(context) {
_activeIssueID = _issues.length > 0 ? _issues[0].id : null;
var containers = selection.selectAll('.issue-container')
.data(_issues, function(d) { return d.id; });
.data(_issues, function(d) { return d.key; });
// Exit
containers.exit()
modules/ui/sections/validation_issues.js
+1 -1
View File
@@ -73,7 +73,7 @@ export function uiSectionValidationIssues(id, severity, context) {
var items = list.selectAll('li')
.data(issues, function(d) { return d.id; });
.data(issues, function(d) { return d.key; });
// Exit
items.exit()