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

Make "edited issues" mean "issues the user created" instead of "issues for features edited by the user"

Browse Source 
Only include issues created by the user in the "warnings" changeset tags
Include counts of issues resolved by the user in the changeset tags (close #6459)
Don't include "fixme" issue counts in "warnings" changeset tags since they're not created by the user (close #6658)
Don't cache crossing ways issues at the rule level
This commit is contained in:
Quincy Morgan
2019-10-04 16:50:44 +02:00
parent 51dfc8f96b
commit 7251a2ab90
3 changed files with 151 additions and 151 deletions
Download Patch File Download Diff File Expand all files Collapse all files

213
modules/core/validator.js
View File

@@ -16,9 +16,9 @@ export function coreValidator(context) {
var _rules = {};
var _disabledRules = {};
var _ignoredIssueIDs = {}; // issue.id -> true
var _issuesByIssueID = {}; // issue.id -> issue
var _issuesByEntityID = {}; // entity.id -> set(issue.id)
var _ignoredIssueIDs = {}; // issue.id -> true
var _baseCache = validationCache(); // issues before any user edits
var _headCache = validationCache(); // issues after all user edits
var _validatedGraph = null;
var _deferred = new Set();
@@ -53,15 +53,9 @@ export function coreValidator(context) {
// clear caches
_ignoredIssueIDs = {};
_issuesByIssueID = {};
_issuesByEntityID = {};
_baseCache = validationCache();
_headCache = validationCache();
_validatedGraph = null;
for (var key in _rules) {
if (typeof _rules[key].reset === 'function') {
_rules[key].reset(); // 'crossing_ways' is the only one like this
}
}
};
validator.resetIgnoredIssues = function() {
@@ -73,28 +67,29 @@ export function coreValidator(context) {
// when the user changes the squaring thereshold, rerun this on all buildings
validator.changeSquareThreshold = function() {
reloadUnsquareIssues(_headCache, context.graph());
reloadUnsquareIssues(_baseCache, context.history().base());
dispatch.call('validated');
};
function reloadUnsquareIssues(cache, graph) {
var checkUnsquareWay = _rules.unsquare_way;
if (typeof checkUnsquareWay !== 'function') return;
// uncache existing
Object.values(_issuesByIssueID)
.filter(function(issue) { return issue.type === 'unsquare_way'; })
.forEach(function(issue) {
var entityId = issue.entityIds[0]; // always 1 entity for unsquare way
if (_issuesByEntityID[entityId]) {
_issuesByEntityID[entityId].delete(issue.id);
}
delete _issuesByIssueID[issue.id];
});
cache.uncacheIssuesOfType('unsquare_way');
var buildings = context.intersects(geoExtent([-180,-90],[180, 90])) // everywhere
var buildings = context.history().tree().intersects(geoExtent([-180,-90],[180, 90]), graph) // everywhere
.filter(function(entity) {
return entity.type === 'way' && entity.tags.building && entity.tags.building !== 'no';
});
// rerun for all buildings
buildings.forEach(function(entity) {
var detected = checkUnsquareWay(entity, context.graph());
var detected = checkUnsquareWay(entity, graph);
if (detected.length !== 1) return;
var issue = detected[0];
@@ -109,15 +104,13 @@ export function coreValidator(context) {
ignoreFix.issue = issue;
issue.fixes.push(ignoreFix);
if (!_issuesByEntityID[entity.id]) {
_issuesByEntityID[entity.id] = new Set();
if (!cache.issuesByEntityID[entity.id]) {
cache.issuesByEntityID[entity.id] = new Set();
}
_issuesByEntityID[entity.id].add(issue.id);
_issuesByIssueID[issue.id] = issue;
cache.issuesByEntityID[entity.id].add(issue.id);
cache.issuesByIssueID[issue.id] = issue;
});
dispatch.call('validated');
};
}
// options = {
@@ -128,8 +121,7 @@ export function coreValidator(context) {
// };
validator.getIssues = function(options) {
var opts = Object.assign({ what: 'all', where: 'all', includeIgnored: false, includeDisabledRules: false }, options);
var issues = Object.values(_issuesByIssueID);
var changes = context.history().difference().changes();
var issues = Object.values(_headCache.issuesByIssueID);
var view = context.map().extent();
return issues.filter(function(issue) {
@@ -145,16 +137,13 @@ export function coreValidator(context) {
for (var i = 0; i < entityIds.length; i++) {
var entityId = entityIds[i];
if (!context.hasEntity(entityId)) {
delete _issuesByEntityID[entityId];
delete _issuesByIssueID[issue.id];
delete _headCache.issuesByEntityID[entityId];
delete _headCache.issuesByIssueID[issue.id];
return false;
}
}
if (opts.what === 'edited') {
var isEdited = entityIds.some(function(entityId) { return changes[entityId]; });
if (entityIds.length && !isEdited) return false;
}
if (opts.what === 'edited' && _baseCache.issuesByIssueID[issue.id]) return false;
if (opts.where === 'visible') {
var extent = issue.extent(context.graph());
@@ -165,6 +154,13 @@ export function coreValidator(context) {
});
};
validator.getResolvedIssues = function() {
var baseIssues = Object.values(_baseCache.issuesByIssueID);
return baseIssues.filter(function(issue) {
return !_headCache.issuesByIssueID[issue.id];
});
};
validator.focusIssue = function(issue) {
var extent = issue.extent(context.graph());
@@ -193,13 +189,15 @@ export function coreValidator(context) {
validator.getEntityIssues = function(entityID, options) {
var issueIDs = _issuesByEntityID[entityID];
var cache = _headCache;
var issueIDs = cache.issuesByEntityID[entityID];
if (!issueIDs) return [];
var opts = options || {};
return Array.from(issueIDs)
.map(function(id) { return _issuesByIssueID[id]; })
.map(function(id) { return cache.issuesByIssueID[id]; })
.filter(function(issue) {
if (opts.includeDisabledRules === 'only' && !_disabledRules[issue.type]) return false;
if (!opts.includeDisabledRules && _disabledRules[issue.type]) return false;
@@ -250,35 +248,6 @@ export function coreValidator(context) {
};
//
// Remove a single entity and all its related issues from the caches
//
function uncacheEntityID(entityID) {
var issueIDs = _issuesByEntityID[entityID];
if (!issueIDs) return;
issueIDs.forEach(function(issueID) {
var issue = _issuesByIssueID[issueID];
if (issue) {
// When multiple entities are involved (e.g. crossing_ways),
// remove this issue from the other entity caches too..
var entityIds = issue.entityIds || [];
entityIds.forEach(function(other) {
if (other !== entityID) {
if (_issuesByEntityID[other]) {
_issuesByEntityID[other].delete(issueID);
}
}
});
}
delete _issuesByIssueID[issueID];
});
delete _issuesByEntityID[entityID];
}
function ignoreIssue(id) {
_ignoredIssueIDs[id] = true;
}
@@ -404,14 +373,13 @@ export function coreValidator(context) {
}
//
// Run validation for several entities, supplied `entityIDs`
// Run validation for several entities, supplied `entityIDs`,
// against `graph` for the given `cache`
//
function validateEntities(entityIDs) {
var graph = context.graph();
function validateEntities(entityIDs, graph, cache) {
// clear caches for existing issues related to these entities
entityIDs.forEach(uncacheEntityID);
entityIDs.forEach(cache.uncacheEntityID);
// detect new issues and update caches
entityIDs.forEach(function(entityID) {
@@ -420,19 +388,8 @@ export function coreValidator(context) {
if (!entity) return;
var issues = validateEntity(entity, graph);
issues.forEach(function(issue) {
var entityIds = issue.entityIds || [];
entityIds.forEach(function(entityId) {
if (!_issuesByEntityID[entityId]) {
_issuesByEntityID[entityId] = new Set();
}
_issuesByEntityID[entityId].add(issue.id);
});
_issuesByIssueID[issue.id] = issue;
});
cache.cacheIssues(issues);
});
dispatch.call('validated');
}
@@ -442,6 +399,7 @@ export function coreValidator(context) {
// and dispatches a `validated` event when finished.
//
validator.validate = function() {
var currGraph = context.graph();
_validatedGraph = _validatedGraph || context.history().base();
if (currGraph === _validatedGraph) {
@@ -452,12 +410,6 @@ export function coreValidator(context) {
var difference = coreDifference(oldGraph, 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
}
}
var createdAndModifiedEntityIDs = difference.extantIDs(true); // created/modified (true = w/relation members)
var entityIDsToCheck = entityIDsToValidate(createdAndModifiedEntityIDs, currGraph);
@@ -470,7 +422,9 @@ export function coreValidator(context) {
// concat the sets
entityIDsToCheckForOldGraph.forEach(entityIDsToCheck.add, entityIDsToCheck);
validateEntities(entityIDsToCheck); // dispatches 'validated'
validateEntities(entityIDsToCheck, context.graph(), _headCache);
dispatch.call('validated');
};
@@ -491,8 +445,14 @@ export function coreValidator(context) {
.on('merge.validator', function(entities) {
if (!entities) return;
var handle = window.requestIdleCallback(function() {
var ids = entities.map(function(entity) { return entity.id; });
validateEntities(entityIDsToValidate(ids, context.graph()));
var entityIDs = entities.map(function(entity) { return entity.id; });
var headGraph = context.graph();
validateEntities(entityIDsToValidate(entityIDs, headGraph), headGraph, _headCache);
var baseGraph = context.history().base();
validateEntities(entityIDsToValidate(entityIDs, baseGraph), baseGraph, _baseCache);
dispatch.call('validated');
});
_deferred.add(handle);
});
@@ -500,3 +460,68 @@ export function coreValidator(context) {
return validator;
}
function validationCache() {
var cache = {
issuesByIssueID: {}, // issue.id -> issue
issuesByEntityID: {} // entity.id -> set(issue.id)
};
cache.cacheIssues = function(issues) {
issues.forEach(function(issue) {
var entityIds = issue.entityIds || [];
entityIds.forEach(function(entityId) {
if (!cache.issuesByEntityID[entityId]) {
cache.issuesByEntityID[entityId] = new Set();
}
cache.issuesByEntityID[entityId].add(issue.id);
});
cache.issuesByIssueID[issue.id] = issue;
});
};
cache.uncacheIssue = function(issue) {
// When multiple entities are involved (e.g. crossing_ways),
// remove this issue from the other entity caches too..
var entityIds = issue.entityIds || [];
entityIds.forEach(function(entityId) {
if (cache.issuesByEntityID[entityId]) {
cache.issuesByEntityID[entityId].delete(issue.id);
}
});
delete cache.issuesByIssueID[issue.id];
};
cache.uncacheIssues = function(issues) {
issues.forEach(cache.uncacheIssue);
};
cache.uncacheIssuesOfType = function(type) {
var issuesOfType = Object.values(cache.issuesByIssueID)
.filter(function(issue) { return issue.type === type; });
cache.uncacheIssues(issuesOfType);
};
//
// Remove a single entity and all its related issues from the caches
//
cache.uncacheEntityID = function(entityID) {
var issueIDs = cache.issuesByEntityID[entityID];
if (!issueIDs) return;
issueIDs.forEach(function(issueID) {
var issue = cache.issuesByIssueID[issueID];
if (issue) {
cache.uncacheIssue(issue);
} else {
delete cache.issuesByIssueID[issueID];
}
});
delete cache.issuesByEntityID[entityID];
};
return cache;
}

41
modules/ui/commit.js
View File

@@ -24,7 +24,8 @@ var readOnlyTags = [
/^imagery_used$/,
/^host$/,
/^locale$/,
/^warnings:/
/^warnings:/,
/^resolved:/
];
// treat most punctuation (except -, _, +, &) as hashtag delimiters - #4398
@@ -147,31 +148,37 @@ export function uiCommit(context) {
}
}
// remove existing warning counts
// remove existing issue counts
for (var key in tags) {
if (key.match(/^warnings:/)) {
if (key.match(/(^warnings:)|(^resolved:)/)) {
delete tags[key];
}
}
function addIssueCounts(issues, prefix) {
var issuesByType = utilArrayGroupBy(issues, 'type');
for (var issueType in issuesByType) {
var issuesOfType = issuesByType[issueType];
if (issuesOfType[0].subtype) {
var issuesBySubtype = utilArrayGroupBy(issuesOfType, 'subtype');
for (var issueSubtype in issuesBySubtype) {
var issuesOfSubtype = issuesBySubtype[issueSubtype];
tags[prefix + ':' + issueType + ':' + issueSubtype] = issuesOfSubtype.length.toString().substr(0, 255);
}
} else {
tags[prefix + ':' + issueType] = issuesOfType.length.toString().substr(0, 255);
}
}
}
// add counts of warnings generated by the user's edits
var warnings = context.validator()
.getIssuesBySeverity({ what: 'edited', where: 'all', includeIgnored: true, includeDisabledRules: true }).warning;
addIssueCounts(warnings, 'warnings');
var warningsByType = utilArrayGroupBy(warnings, 'type');
for (var warningType in warningsByType) {
var warningsOfType = warningsByType[warningType];
if (warningsOfType[0].subtype) {
var warningsBySubtype = utilArrayGroupBy(warningsOfType, 'subtype');
for (var warningSubtype in warningsBySubtype) {
var warningsOfSubtype = warningsBySubtype[warningSubtype];
tags['warnings:' + warningType + ':' + warningSubtype] = warningsOfSubtype.length.toString();
}
} else {
tags['warnings:' + warningType] = warningsOfType.length.toString();
}
}
// add counts of issues resolved by the user's edits
var resolvedIssues = context.validator().getResolvedIssues();
addIssueCounts(resolvedIssues, 'resolved');
_changeset = _changeset.update({ tags: tags });

48
modules/validations/crossing_ways.js
View File

@@ -12,19 +12,6 @@ import { validationIssue, validationIssueFix } from '../core/validation';
export function validationCrossingWays(context) {
var type = 'crossing_ways';
/*
Avoid duplicate work by cacheing issues. The same issues live under two paths.
{
w-123: {
w-456: [{issue1}, {issue2}…]
},
w-456: {
w-123: [{issue1}, {issue2}…]
}
}
*/
var _issueCache = {};
// returns the way or its parent relation, whichever has a useful feature type
function getFeatureWithFeatureTypeTagsForWay(way, graph) {
if (getFeatureTypeForTags(way.tags) === null) {
@@ -252,9 +239,6 @@ export function validationCrossingWays(context) {
// skip if this way was already checked and only one issue is needed
if (checkedSingleCrossingWays[way2.id]) continue;
// don't re-check previously checked features
if (_issueCache[way1.id] && _issueCache[way1.id][way2.id]) continue;
// mark this way as checked even if there are no crossings
comparedWays[way2.id] = true;
@@ -295,12 +279,6 @@ export function validationCrossingWays(context) {
}
}
}
for (var way2ID in comparedWays) {
if (!_issueCache[way1.id]) _issueCache[way1.id] = {};
if (!_issueCache[way1.id][way2ID]) _issueCache[way1.id][way2ID] = [];
if (!_issueCache[way2ID]) _issueCache[way2ID] = {};
if (!_issueCache[way2ID][way1.id]) _issueCache[way2ID][way1.id] = [];
}
return edgeCrossInfos;
}
@@ -337,20 +315,11 @@ export function validationCrossingWays(context) {
var issues = [];
// declare these here to reduce garbage collection
var wayIndex, crossingIndex, key, crossings, crossing, issue;
var wayIndex, crossingIndex, crossings;
for (wayIndex in ways) {
var way = ways[wayIndex];
crossings = findCrossingsByWay(way, graph, tree);
crossings = findCrossingsByWay(ways[wayIndex], graph, tree);
for (crossingIndex in crossings) {
crossing = crossings[crossingIndex];
var way2 = crossing.ways[1];
issue = createIssue(crossing, graph);
// cache the issues for each way
_issueCache[way.id][way2.id].push(issue);
_issueCache[way2.id][way.id].push(issue);
}
for (key in _issueCache[way.id]) {
issues = issues.concat(_issueCache[way.id][key]);
issues.push(createIssue(crossings[crossingIndex], graph));
}
}
return issues;
@@ -457,9 +426,12 @@ export function validationCrossingWays(context) {
connectionTags: connectionTags
},
// differentiate based on the loc since two ways can cross multiple times
hash: JSON.stringify(crossing.crossPoint) +
hash: crossing.crossPoint.toString() +
// if the edges change then so does the fix
JSON.stringify(crossing.edges) +
crossing.edges.slice().sort(function(edge1, edge2) {
// order to assure hash is deterministic
return edge1[0] < edge2[0] ? -1 : 1;
}).toString() +
// ensure the correct connection tags are added in the fix
JSON.stringify(connectionTags),
loc: crossing.crossPoint,
@@ -569,10 +541,6 @@ export function validationCrossingWays(context) {
});
}
validation.reset = function() {
_issueCache = {};
};
validation.type = type;
return validation;