From 5db1c535ba8b2c45529553acb94cb86527aa1ef0 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 26 Jan 2021 17:10:09 -0500 Subject: [PATCH] Allow validators to return provisional results, revalidate after delay Also add a ton of commments to validator.js --- modules/core/validator.js | 411 +++++++++++++++++------- modules/ui/sections/validation_rules.js | 2 +- modules/validations/outdated_tags.js | 33 +- modules/validations/suspicious_name.js | 23 +- modules/validations/unsquare_way.js | 2 +- 5 files changed, 324 insertions(+), 147 deletions(-) diff --git a/modules/core/validator.js b/modules/core/validator.js index d64ec9c35bd..be18bba5bfa 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -21,14 +21,17 @@ export function coreValidator(context) { let _headCache = validationCache(); // issues after all user edits let _previousGraph = null; - let _deferred = new Set(); // Set( IdleCallback handles ) - let _inProcess; // Promise fulfilled when validation complete + let _deferredRIC = new Set(); // Set( RequestIdleCallback handles ) + let _deferredST = new Set(); // Set( SetTimeout handles ) + let _inProcess; // Promise fulfilled when validation complete + const RETRY = 5000; // wait 5sec before revalidating provisional entities + + // `init()` + // Initialize the validator, called once on iD startup // - // initialize the validator rulesets - // - validator.init = function() { + validator.init = () => { Object.values(Validations).forEach(validation => { if (typeof validation !== 'function') return; const fn = validation(context); @@ -42,12 +45,19 @@ export function coreValidator(context) { } }; + + // `reset()` (private) + // Cancels deferred work and resets all caches + // + // Arguments + // `resetIgnored` - `true` to clear the list of user-ignored issues + // function reset(resetIgnored) { // cancel deferred work - Array.from(_deferred).forEach(handle => { - window.cancelIdleCallback(handle); - _deferred.delete(handle); - }); + _deferredRIC.forEach(window.cancelIdleCallback); + _deferredRIC.clear(); + _deferredST.forEach(window.clearTimeout); + _deferredST.clear(); // empty queues and resolve any pending promise _baseCache.queue = []; @@ -64,21 +74,28 @@ export function coreValidator(context) { } - // - // clear caches, called whenever iD resets after a save + // `reset()` + // clear caches, called whenever iD resets after a save or switches sources + // (clears out the _ignoredIssueIDs set also) // validator.reset = () => { reset(true); }; + // `resetIgnoredIssues()` + // clears out the _ignoredIssueIDs Set + // validator.resetIgnoredIssues = () => { - _ignoredIssueIDs = {}; + _ignoredIssueIDs.clear(); dispatch.call('validated'); // redraw UI }; - // must update issues when the user changes the unsquare thereshold + // `revalidateUnsquare()` + // Called whenever the user changes the unsquare threshold + // It reruns just the "unsquare_way" validation on all buildings. + // validator.revalidateUnsquare = () => { revalidateUnsquare(_headCache, context.graph()); revalidateUnsquare(_baseCache, context.history().base()); @@ -99,82 +116,93 @@ export function coreValidator(context) { buildings.forEach(entity => { const detected = checkUnsquareWay(entity, graph); if (!detected.length) return; - - const issue = detected[0]; - if (!cache.issuesByEntityID[entity.id]) { - cache.issuesByEntityID[entity.id] = new Set(); - } - cache.issuesByEntityID[entity.id].add(issue.id); - cache.issuesByIssueID[issue.id] = issue; + cache.cacheIssues(detected); }); } - // options = { - // what: 'all', // 'all' or 'edited' - // where: 'all', // 'all' or 'visible' - // includeIgnored: false, // true, false, or 'only' - // includeDisabledRules: false // true, false, or 'only' - // }; + // `getIssues()` + // Gets all issues that match the given options + // This is called by many other places + // + // Arguments + // `options` Object like: + // { + // what: 'all', // 'all' or 'edited' + // where: 'all', // 'all' or 'visible' + // includeIgnored: false, // true, false, or 'only' + // includeDisabledRules: false // true, false, or 'only' + // } + // + // Returns + // An Array containing the issues + // validator.getIssues = (options) => { const opts = Object.assign({ what: 'all', where: 'all', includeIgnored: false, includeDisabledRules: false }, options); const view = context.map().extent(); let issues = []; let seen = new Set(); - function filter(issue, resolver) { + // collect head issues - caused by user edits + let cache = _headCache; + let graph = context.graph(); + Object.values(cache.issuesByIssueID).forEach(issue => { + if (!filter(issue, graph, cache)) return; + seen.add(issue.id); + issues.push(issue); + }); + + // collect base issues - not caused by user edits + if (opts.what === 'all') { + cache = _baseCache; + graph = context.history().base(); + Object.values(cache.issuesByIssueID).forEach(issue => { + if (!filter(issue, graph, cache)) return; + seen.add(issue.id); + issues.push(issue); + }); + } + + return issues; + + + function filter(issue, resolver, cache) { if (!issue) return false; if (seen.has(issue.id)) return false; if (_resolvedIssueIDs.has(issue.id)) return false; if (opts.includeDisabledRules === 'only' && !_disabledRules[issue.type]) return false; if (!opts.includeDisabledRules && _disabledRules[issue.type]) return false; - if (opts.includeIgnored === 'only' && !_ignoredIssueIDs[issue.id]) return false; - if (!opts.includeIgnored && _ignoredIssueIDs[issue.id]) return false; - - if (opts.where === 'visible') { - const extent = issue.extent(resolver); - if (!view.intersects(extent)) return false; - } - - return true; - } - - // collect head issues - caused by user edits - Object.values(_headCache.issuesByIssueID).forEach(issue => { - if (!filter(issue, context.graph())) return; // pass head graph + if (opts.includeIgnored === 'only' && !_ignoredIssueIDs.has(issue.id)) return false; + if (!opts.includeIgnored && _ignoredIssueIDs.has(issue.id)) return false; // Sanity check: This issue may be for an entity that not longer exists. - // If we detect this, uncache and return so it is not included.. + // If we detect this, uncache and return false so it is not included.. const entityIDs = issue.entityIds || []; for (let i = 0; i < entityIDs.length; i++) { const entityID = entityIDs[i]; - if (!context.hasEntity(entityID)) { - _headCache.uncacheEntityID(entityID); - return; + if (!resolver.hasEntity(entityID)) { + cache.uncacheEntityID(entityID); + return false; } } - // keep the issue - seen.add(issue.id); - issues.push(issue); - }); - // collect base issues - not caused by user edits - if (opts.what === 'all') { - Object.values(_baseCache.issuesByIssueID).forEach(issue => { - if (!filter(issue, context.history().base())) return; // pass base graph - // keep the issue - seen.add(issue.id); - issues.push(issue); - }); - } + if (opts.where === 'visible') { + const extent = issue.extent(resolver); + if (!view.intersects(extent)) return false; + } - return issues; + return true; + } }; + // `getResolvedIssues()` + // Gets the issues that have been fixed by the user. + // Resolved issues are tracked in the `_resolvedIssueIDs` Set // - // issues fixed by the user + // Returns + // An Array containing the issues // validator.getResolvedIssues = () => { let collected = new Set(); @@ -190,6 +218,13 @@ export function coreValidator(context) { }; + // `focusIssue()` + // Adjusts the map to focus on the given issue. + // (requires the issue to have a reasonable extent defined) + // + // Arguments + // `issue` - the issue to focus on + // validator.focusIssue = (issue) => { const extent = issue.extent(context.graph()); if (!extent) return; @@ -208,6 +243,19 @@ export function coreValidator(context) { }; + // `getIssuesBySeverity()` + // Gets the issues then groups them by error/warning + // (This just calls getIssues, then puts issues in groups) + // + // Arguments + // `options` - (see `getIssues`) + // Returns + // Object result like: + // { + // error: Array of errors, + // warning: Array of warnings + // } + // validator.getIssuesBySeverity = (options) => { let groups = utilArrayGroupBy(validator.getIssues(options), 'severity'); groups.error = groups.error || []; @@ -216,20 +264,30 @@ export function coreValidator(context) { }; - // show some issue types in a particular order - const orderedIssueTypes = [ - // flag missing data first - 'missing_tag', 'missing_role', - // then flag identity issues - 'outdated_tags', 'mismatched_geometry', - // flag geometry issues where fixing them might solve connectivity issues - 'crossing_ways', 'almost_junction', - // then flag connectivity issues - 'disconnected_way', 'impossible_oneway' - ]; - - // returns the issues that the given entity IDs have in common, matching the given options - validator.getSharedEntityIssues = function(entityIDs, options) { + // `getEntityIssues()` + // Gets the issues that the given entity IDs have in common, matching the given options + // (This just calls getIssues, then filters for the given entity IDs) + // The issues are sorted for relevance + // + // Arguments + // `entityIDs` - Array or Set of entityIDs to get issues for + // `options` - (see `getIssues`) + // Returns + // An Array containing the issues + // + validator.getSharedEntityIssues = (entityIDs, options) => { + // show some issue types in a particular order + const orderedIssueTypes = [ + // flag missing data first + 'missing_tag', 'missing_role', + // then flag identity issues + 'outdated_tags', 'mismatched_geometry', + // flag geometry issues where fixing them might solve connectivity issues + 'crossing_ways', 'almost_junction', + // then flag connectivity issues + 'disconnected_way', 'impossible_oneway' + ]; + const allIssues = validator.getIssues(options); const forEntityIDs = new Set(entityIDs); @@ -252,21 +310,50 @@ export function coreValidator(context) { }; + // `getEntityIssues()` + // Get an array of detected issues for the given entityID. + // (This just calls getSharedEntityIssues for a single entity) + // + // Arguments + // `entityID` - the entity ID to get the issues for + // `options` - (see `getIssues`) + // Returns + // An Array containing the issues + // validator.getEntityIssues = (entityID, options) => { return validator.getSharedEntityIssues([entityID], options); }; + // `getRuleKeys()` + // + // Returns + // An Array containing the rule keys + // validator.getRuleKeys = () => { return Object.keys(_rules); }; + // `isRuleEnabled()` + // + // Arguments + // `key` - the rule to check (e.g. 'crossing_ways') + // Returns + // `true`/`false` + // validator.isRuleEnabled = (key) => { return !_disabledRules[key]; }; + // `toggleRule()` + // Toggles a single validation rule, + // then reruns the validation so that the user sees something happen in the UI + // + // Arguments + // `key` - the rule to toggle (e.g. 'crossing_ways') + // validator.toggleRule = (key) => { if (_disabledRules[key]) { delete _disabledRules[key]; @@ -279,6 +366,13 @@ export function coreValidator(context) { }; + // `disableRules()` + // Disables given validation rules, + // then reruns the validation so that the user sees something happen in the UI + // + // Arguments + // `keys` - Array or Set containing rule keys to disable + // validator.disableRules = (keys) => { _disabledRules = {}; keys.forEach(k => _disabledRules[k] = true); @@ -288,17 +382,24 @@ export function coreValidator(context) { }; - validator.ignoreIssue = (id) => { - _ignoredIssueIDs[id] = true; + // `ignoreIssue()` + // Don't show the given issue in lists + // + // Arguments + // `issueID` - the issueID + // + validator.ignoreIssue = (issueID) => { + _ignoredIssueIDs.add(issueID); }; - // + // `validate()` // Validates anything that has changed in the head graph since the last time it was run. // (head graph contains user's edits) // - // Returns a Promise fulfilled when the validation has completed and then dispatches a `validated` event. - // This may take time but happen in the background during browser idle time. + // Returns + // A Promise fulfilled when the validation has completed and then dispatches a `validated` event. + // This may take time but happen in the background during browser idle time. // validator.validate = () => { const currGraph = context.graph(); @@ -331,12 +432,12 @@ export function coreValidator(context) { return validateEntitiesAsync(entityIDsToCheck, currGraph, _headCache) .then(() => updateResolvedIssues(entityIDsToCheck)) .then(() => dispatch.call('validated')); + // .then(() => validateProvisionalEntitiesAsync(currGraph)) }; // register event handlers: - // WHEN TO RUN VALIDATION: // When graph changes: context.history() @@ -365,11 +466,28 @@ export function coreValidator(context) { + // `validateEntity()` (private) + // Runs all validation rules on a single entity. + // Some things to note: + // - Graph is passed in from whenever the validation was started. Validators shouldn't use + // `context.graph()` because this all happens async, and the graph might have changed + // (for example, nodes getting deleted before the validation can run) + // - Validator functions may still be waiting on something and return a "provisional" result. + // In this situation, we will schedule to revalidate the entity sometime later. // - // Run validation on a single entity for the given graph + // Arguments + // `entity` - The entity + // `graph` - graph containing the entity + // + // Returns + // Object result like: + // { + // issues: Array of detected issues + // provisional: `true` if provisional result, `false` if final result + // } // function validateEntity(entity, graph) { - let entityIssues = []; + let result = { issues: [], provisional: false }; // runs validation and appends resulting issues function runValidation(key) { @@ -380,16 +498,29 @@ export function coreValidator(context) { } const detected = fn(entity, graph); - entityIssues = entityIssues.concat(detected); + if (detected.provisional) { // this validation should be run again later + result.provisional = true; + } + result.issues = result.issues.concat(detected); } // run all rules Object.keys(_rules).forEach(runValidation); - return entityIssues; + return result; } + // `entityIDsToValidate()` (private) + // Collects the complete list of entityIDs related to the input entityIDs. + // + // Arguments + // `entityIDs` - Set or Array containing entityIDs. + // `graph` - graph containing the entities + // + // Returns + // Set containing entityIDs + // function entityIDsToValidate(entityIDs, graph) { let seen = new Set(); let collected = new Set(); @@ -434,9 +565,15 @@ export function coreValidator(context) { } - // Determine what issues were resolved for the given entities - function updateResolvedIssues(entityIDsToCheck) { - entityIDsToCheck.forEach(entityID => { + // `updateResolvedIssues()` (private) + // Determine if any issues were resolved for the given entities. + // This is called by `validate()` after validation of the head graph + // + // Arguments + // `entityIDs` - Set containing entity IDs. + // + function updateResolvedIssues(entityIDs) { + entityIDs.forEach(entityID => { const headIssues = _headCache.issuesByEntityID[entityID]; const baseIssues = _baseCache.issuesByEntityID[entityID]; if (!baseIssues) return; @@ -452,36 +589,38 @@ export function coreValidator(context) { } + // `validateEntitiesAsync()` (private) + // Schedule validation for many entities. // - // Run validation for several entities, supplied `entityIDs`, - // against `graph` for the given `cache` - // - // Returns a Promise fulfilled when the validation has completed. - // This may take time but happen in the background during browser idle time. + // Arguments + // `entityIDs` - Set containing entity IDs. + // `graph` - the graph to validate that contains those entities + // `cache` - the cache to store results in (_headCache or _baseCache) // - // `entityIDs` - Array or Set containing entity IDs. - // `graph` - the graph to validate that contains those entities - // `cache` - the cache to store results in (_headCache or _baseCache) + // Returns + // A Promise fulfilled when the validation has completed. + // This may take time but happen in the background during browser idle time. // function validateEntitiesAsync(entityIDs, graph, cache) { // Enqueue the work const jobs = Array.from(entityIDs).map(entityID => { - if (cache.queuedIDs.has(entityID)) return null; // queued already - cache.queuedIDs.add(entityID); + if (cache.queuedEntityIDs.has(entityID)) return null; // queued already + cache.queuedEntityIDs.add(entityID); return () => { // clear caches for existing issues related to this entity cache.uncacheEntityID(entityID); + cache.queuedEntityIDs.delete(entityID); // detect new issues and update caches - const entity = graph.hasEntity(entityID); - if (entity) { // don't validate deleted entities - // todo next: promisify this part - const issues = validateEntity(entity, graph); - cache.cacheIssues(issues); // update cache + const entity = graph.hasEntity(entityID); // Sanity check: don't validate deleted entities + if (entity) { + const result = validateEntity(entity, graph); + if (result.provisional) { // provisional result + cache.provisionalEntityIDs.add(entityID); // we'll need to revalidate this entity again later + } + cache.cacheIssues(result.issues); // update cache } - - cache.queuedIDs.delete(entityID); }; }).filter(Boolean); @@ -496,6 +635,7 @@ export function coreValidator(context) { if (_inProcess) return _inProcess; _inProcess = processQueue() + .then(() => revalidateProvisionalEntities(cache)) .catch(() => { /* ignore */ }) .finally(() => _inProcess = null); @@ -503,11 +643,35 @@ export function coreValidator(context) { } - // `processQueue()` - // Process some deferred validation work + // `revalidateProvisionalEntities()` (private) + // Sometimes a validator will return a "provisional" result. + // In this situation, we'll need to revalidate the entity later. + // This function waits a delay, then places them back into the validation queue. + // + // Arguments + // `cache` - The cache (_headCache or _baseCache) + // + function revalidateProvisionalEntities(cache) { + if (!cache.provisionalEntityIDs.size) return; // nothing to do + + const handle = window.setTimeout(() => { + _deferredST.delete(handle); + if (!cache.provisionalEntityIDs.size) return; // nothing to do + + const graph = (cache === _headCache ? context.graph() : context.history().base()); + validateEntitiesAsync(cache.provisionalEntityIDs, graph, cache); + }, RETRY); + + _deferredST.add(handle); + } + + + // `processQueue()` (private) + // Process the next chunk of deferred validation work // - // Returns a Promise fulfilled when the validation has completed. - // This may take time but happen in the background during browser idle time. + // Returns + // A Promise fulfilled when the validation has completed. + // This may take time but happen in the background during browser idle time. // function processQueue() { // console.log(`head queue length ${_headCache.queue.length}`); @@ -522,15 +686,15 @@ export function coreValidator(context) { if (!chunk) return Promise.resolve(); // we're done return new Promise(resolvePromise => { - const handle = window.requestIdleCallback(() => { - _deferred.delete(handle); + const handle = window.requestIdleCallback(() => { + _deferredRIC.delete(handle); // const t0 = performance.now(); chunk.forEach(job => job()); // const t1 = performance.now(); // console.log('chunk processed in ' + (t1 - t0) + ' ms'); resolvePromise(); }); - _deferred.add(handle); + _deferredRIC.add(handle); }) .then(() => { // dispatch an event sometimes to redraw various UI things const count = _headCache.queue.length + _baseCache.queue.length; @@ -544,22 +708,29 @@ export function coreValidator(context) { } +// `validationCache()` (private) +// Creates a cache to store validation state +// We create 2 of these: +// `_baseCache` for validation on the base graph (unedited) +// `_headCache` for validation on the head graph (user edits applied) +// function validationCache() { let cache = { queue: [], - queuedIDs: new Set(), + queuedEntityIDs: new Set(), + provisionalEntityIDs: new Set(), issuesByIssueID: {}, // issue.id -> issue 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(); + const entityIDs = issue.entityIds || []; + entityIDs.forEach(entityID => { + if (!cache.issuesByEntityID[entityID]) { + cache.issuesByEntityID[entityID] = new Set(); } - cache.issuesByEntityID[entityId].add(issue.id); + cache.issuesByEntityID[entityID].add(issue.id); }); cache.issuesByIssueID[issue.id] = issue; }); @@ -568,8 +739,8 @@ function validationCache() { 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 => { + const entityIDs = issue.entityIds || []; + entityIDs.forEach(entityID => { if (cache.issuesByEntityID[entityID]) { cache.issuesByEntityID[entityID].delete(issue.id); } @@ -590,7 +761,6 @@ function validationCache() { // 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 issue = cache.issuesByIssueID[issueID]; @@ -603,6 +773,7 @@ function validationCache() { } delete cache.issuesByEntityID[entityID]; + cache.provisionalEntityIDs.delete(entityID); }; diff --git a/modules/ui/sections/validation_rules.js b/modules/ui/sections/validation_rules.js index 07858a4606e..d8fbd586459 100644 --- a/modules/ui/sections/validation_rules.js +++ b/modules/ui/sections/validation_rules.js @@ -175,7 +175,7 @@ export function uiSectionValidationRules(context) { .property('value', degStr); prefs('validate-square-degrees', degStr); - context.validator().reloadUnsquareIssues(); + context.validator().revalidateUnsquare(); } function isRuleEnabled(d) { diff --git a/modules/validations/outdated_tags.js b/modules/validations/outdated_tags.js index 722823a3611..58c2f5e5675 100644 --- a/modules/validations/outdated_tags.js +++ b/modules/validations/outdated_tags.js @@ -7,24 +7,22 @@ import { actionChangeTags } from '../actions/change_tags'; import { actionUpgradeTags } from '../actions/upgrade_tags'; import { presetManager } from '../presets'; import { osmIsOldMultipolygonOuterMember, osmOldMultipolygonOuterMemberOfRelation } from '../osm/multipolygon'; -import { utilArrayUniq, utilDisplayLabel, utilTagDiff } from '../util'; +import { utilArrayUniq, utilDisplayLabel, utilHashcode, utilTagDiff } from '../util'; import { validationIssue, validationIssueFix } from '../core/validation'; -let _dataDeprecated; -let _nsi; - export function validationOutdatedTags() { const type = 'outdated_tags'; - - // A concern here in switching to async data means that `_dataDeprecated` - // and `_nsi` will not be available at first, so the data on early tiles - // may not have tags validated fully. + let _waitingForDeprecated = true; + let _waitingForNSI = true; + let _dataDeprecated; + let _nsi; // fetch deprecated tags fileFetcher.get('deprecated') .then(d => _dataDeprecated = d) - .catch(() => { /* ignore */ }); + .catch(() => { /* ignore */ }) + .finally(() => _waitingForDeprecated = false); function delay(msec) { @@ -92,10 +90,10 @@ export function validationOutdatedTags() { }); _nsi.keys.add('building'); // fallback can match building=* for some categories - return _nsi; }) - .catch(() => { /* ignore */ }); + .catch(() => { /* ignore */ }) + .finally(() => _waitingForNSI = false); // Returns true if this tag key is a "namelike" tag that the NSI matcher would have indexed.. @@ -124,6 +122,7 @@ export function validationOutdatedTags() { let preset = presetManager.match(entity, graph); let subtype = 'deprecated_tags'; if (!preset) return []; + if (!entity.hasInterestingTags()) return []; // Upgrade preset, if a replacement is available.. if (preset.replacement) { @@ -301,9 +300,12 @@ export function validationOutdatedTags() { } // end if _nsi + let issues = []; + issues.provisional = (_waitingForDeprecated || _waitingForNSI); + // determine diff const tagDiff = utilTagDiff(oldTags, newTags); - if (!tagDiff.length) return []; + if (!tagDiff.length) return issues; const isOnlyAddingTags = tagDiff.every(d => d.type === '+'); @@ -318,14 +320,14 @@ export function validationOutdatedTags() { // don't allow autofixing brand tags let autoArgs = subtype !== 'noncanonical_brand' ? [doUpgrade, t('issues.fix.upgrade_tags.annotation')] : null; - return [new validationIssue({ + issues.push(new validationIssue({ type: type, subtype: subtype, severity: 'warning', message: showMessage, reference: showReference, entityIds: [entity.id], - hash: JSON.stringify(tagDiff), + hash: utilHashcode(JSON.stringify(tagDiff)), dynamicFixes: () => { return [ new validationIssueFix({ @@ -337,7 +339,8 @@ export function validationOutdatedTags() { }) ]; } - })]; + })); + return issues; function doUpgrade(graph) { diff --git a/modules/validations/suspicious_name.js b/modules/validations/suspicious_name.js index b373434fbbe..75baf119b01 100644 --- a/modules/validations/suspicious_name.js +++ b/modules/validations/suspicious_name.js @@ -5,28 +5,30 @@ import { validationIssue, validationIssueFix } from '../core/validation'; import { actionChangeTags } from '../actions/change_tags'; -let _discardNameRegexes = []; - export function validationSuspiciousName() { const type = 'suspicious_name'; const keysToTestForGenericValues = [ 'aerialway', 'aeroway', 'amenity', 'building', 'craft', 'highway', 'leisure', 'railway', 'man_made', 'office', 'shop', 'tourism', 'waterway' ]; - - // A concern here in switching to async data means that `_nsiFilters` will not - // be available at first, so the data on early tiles may not have tags validated fully. + let _dataGenerics; + let _waitingForGenerics = true; fileFetcher.get('nsi_generics') .then(data => { + if (_dataGenerics) return _dataGenerics; + // known list of generic names (e.g. "bar") - _discardNameRegexes = data.genericWords.map(pattern => new RegExp(pattern, 'i')); + _dataGenerics = data.genericWords.map(pattern => new RegExp(pattern, 'i')); + return _dataGenerics; }) - .catch(() => { /* ignore */ }); + .catch(() => { /* ignore */ }) + .finally(() => _waitingForGenerics = false); function isDiscardedSuggestionName(lowercaseName) { - return _discardNameRegexes.some(regex => regex.test(lowercaseName)); + if (!_dataGenerics) return false; + return _dataGenerics.some(regex => regex.test(lowercaseName)); } // test if the name is just the key or tag value (e.g. "park") @@ -68,7 +70,7 @@ export function validationSuspiciousName() { }, reference: showReference, entityIds: [entityId], - hash: nameKey + '=' + genericName, + hash: `${nameKey}=${genericName}`, dynamicFixes: function() { return [ new validationIssueFix({ @@ -114,7 +116,7 @@ export function validationSuspiciousName() { }, reference: showReference, entityIds: [entityId], - hash: nameKey + '=' + incorrectName, + hash: `${nameKey}=${incorrectName}`, dynamicFixes: function() { return [ new validationIssueFix({ @@ -168,6 +170,7 @@ export function validationSuspiciousName() { } } if (isGenericName(value, entity.tags)) { + issues.provisional = _waitingForGenerics; // retry later if we don't have the generics yet issues.push(makeGenericNameIssue(entity.id, key, value, langCode)); } } diff --git a/modules/validations/unsquare_way.js b/modules/validations/unsquare_way.js index 47873315561..cd837e21deb 100644 --- a/modules/validations/unsquare_way.js +++ b/modules/validations/unsquare_way.js @@ -80,7 +80,7 @@ export function validationUnsquareWay(context) { }, reference: showReference, entityIds: [entity.id], - hash: JSON.stringify(autoArgs !== undefined) + degreeThreshold, + hash: degreeThreshold, dynamicFixes: function() { return [ new validationIssueFix({