From 2b2a71f59726804c3b0f05feb9d609b3e39396e7 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 28 Oct 2022 10:49:01 -0400 Subject: [PATCH] Don't pre-resolve and index complex locationSets into GeoJSON. This was taking a lot of time at app startup. Instad now we resolve and index only the include and exclude parts. We can still determine the valid locationSets at runtime in `locationSetsAt()` by checking the `_locationIncludedIn` and `_locationExcludedIn` caches. This also upgrades the locationManger to an ES6 class. This also includes some hacky code in nsi.js so that the NSI will continue to work. The NSI matcher can build its own location index, but it doesn't need to do this. We monkeypatch a few of the matcher collections to work with the new LocationManager. --- .eslintrc | 3 +- modules/core/LocationManager.js | 342 ++++++++++++++++++++++++++++++ modules/core/index.js | 2 +- modules/core/locations.js | 270 ----------------------- modules/presets/collection.js | 6 +- modules/presets/index.js | 14 +- modules/services/nsi.js | 102 ++++++--- modules/ui/field.js | 6 +- modules/ui/success.js | 6 +- test/spec/core/LocationManager.js | 153 +++++++++++++ test/spec/core/locations.js | 150 ------------- 11 files changed, 581 insertions(+), 473 deletions(-) create mode 100644 modules/core/LocationManager.js delete mode 100644 modules/core/locations.js create mode 100644 test/spec/core/LocationManager.js delete mode 100644 test/spec/core/locations.js diff --git a/.eslintrc b/.eslintrc index 95932d23db..e70eaad321 100644 --- a/.eslintrc +++ b/.eslintrc @@ -21,7 +21,6 @@ "block-scoped-var": "error", "block-spacing": ["warn", "always"], "brace-style": ["warn", "1tbs", { "allowSingleLine": true }], - "class-methods-use-this": "error", "complexity": ["warn", 50], "curly": ["warn", "multi-line"], "default-case-last": "error", @@ -107,4 +106,4 @@ "space-unary-ops": "error", "wrap-regex": "off" } -} +} \ No newline at end of file diff --git a/modules/core/LocationManager.js b/modules/core/LocationManager.js new file mode 100644 index 0000000000..08984208c1 --- /dev/null +++ b/modules/core/LocationManager.js @@ -0,0 +1,342 @@ +import LocationConflation from '@ideditor/location-conflation'; +import whichPolygon from 'which-polygon'; +import calcArea from '@mapbox/geojson-area'; + +const _loco = new LocationConflation(); // instance of a location-conflation resolver + + +/** + * `LocationManager` maintains an internal index of all the boundaries/geofences used by iD. + * It's used by presets, community index, background imagery, to know where in the world these things are valid. + * These geofences should be defined by `locationSet` objects: + * + * let locationSet = { + * include: [ Array of locations ], + * exclude: [ Array of locations ] + * }; + * + * For more info see the location-conflation and country-coder projects, see: + * https://github.com/ideditor/location-conflation + * https://github.com/ideditor/country-coder + */ +export class LocationManager { + + /** + * @constructor + */ + constructor() { + this._wp = null; // A which-polygon index + this._resolved = new Map(); // Map (id -> GeoJSON feature) + this._knownLocationSets = new Map(); // Map (locationSetID -> Number area) + this._locationIncludedIn = new Map(); // Map (locationID -> Set(locationSetID) ) + this._locationExcludedIn = new Map(); // Map (locationID -> Set(locationSetID) ) + + // pre-resolve the worldwide locationSet + const world = { locationSet: { include: ['Q2'] } }; + this._resolveLocationSet(world); + this._rebuildIndex(); + } + + + /** + * _validateLocationSet + * Pass an Object with a `locationSet` property. + * Validates the `locationSet` and sets a `locationSetID` property on the object. + * To avoid so much computation we only resolve the include and exclude regions, but not the locationSet itself. + * + * Use `_resolveLocationSet()` instead if you need to resolve geojson of locationSet, for example to render it. + * Note: You need to call `_rebuildIndex()` after you're all finished validating the locationSets. + * + * @param `obj` Object to check, it should have `locationSet` property + */ + _validateLocationSet(obj) { + if (obj.locationSetID) return; // work was done already + + try { + let locationSet = obj.locationSet; + if (!locationSet) { + throw new Error('object missing locationSet property'); + } + if (!locationSet.include) { // missing `include`, default to worldwide include + locationSet.include = ['Q2']; // https://github.com/openstreetmap/iD/pull/8305#discussion_r662344647 + } + + // Validate the locationSet only + // Resolve the include/excludes + const locationSetID = _loco.validateLocationSet(locationSet).id; + obj.locationSetID = locationSetID; + if (this._knownLocationSets.has(locationSetID)) return; // seen one like this before + + let area = 0; + + // Resolve and index the 'includes' + (locationSet.include || []).forEach(location => { + const locationID = _loco.validateLocation(location).id; + let geojson = this._resolved.get(locationID); + + if (!geojson) { // first time seeing a location like this + geojson = _loco.resolveLocation(location).feature; // resolve to GeoJSON + this._resolved.set(locationID, geojson); + } + area += geojson.properties.area; + + let s = this._locationIncludedIn.get(locationID); + if (!s) { + s = new Set(); + this._locationIncludedIn.set(locationID, s); + } + s.add(locationSetID); + }); + + // Resolve and index the 'excludes' + (locationSet.exclude || []).forEach(location => { + const locationID = _loco.validateLocation(location).id; + let geojson = this._resolved.get(locationID); + + if (!geojson) { // first time seeing a location like this + geojson = _loco.resolveLocation(location).feature; // resolve to GeoJSON + this._resolved.set(locationID, geojson); + } + area -= geojson.properties.area; + + let s = this._locationExcludedIn.get(locationID); + if (!s) { + s = new Set(); + this._locationExcludedIn.set(locationID, s); + } + s.add(locationSetID); + }); + + this._knownLocationSets.set(locationSetID, area); + + } catch (err) { + obj.locationSet = { include: ['Q2'] }; // default worldwide + obj.locationSetID = '+[Q2]'; + } + } + + + /** + * _resolveLocationSet + * Does everything that `_validateLocationSet()` does, but then "resolves" the locationSet into GeoJSON. + * This step is a bit more computationally expensive, so really only needed if you intend to render the shape. + * + * Note: You need to call `_rebuildIndex()` after you're all finished validating the locationSets. + * + * @param `obj` Object to check, it should have `locationSet` property + */ + _resolveLocationSet(obj) { + this._validateLocationSet(obj); + + if (this._resolved.has(obj.locationSetID)) return; // work was done already + + try { + const result = _loco.resolveLocationSet(obj.locationSet); + const locationSetID = result.id; + obj.locationSetID = locationSetID; + + if (!result.feature.geometry.coordinates.length || !result.feature.properties.area) { + throw new Error(`locationSet ${locationSetID} resolves to an empty feature.`); + } + + let geojson = JSON.parse(JSON.stringify(result.feature)); // deep clone + geojson.id = locationSetID; // Important: always use the locationSet `id` (`+[Q30]`), not the feature `id` (`Q30`) + geojson.properties.id = locationSetID; + this._resolved.set(locationSetID, geojson); + + } catch (err) { + obj.locationSet = { include: ['Q2'] }; // default worldwide + obj.locationSetID = '+[Q2]'; + } + } + + + /** + * _rebuildIndex + * Rebuilds the whichPolygon index with whatever features have been resolved into GeoJSON. + */ + _rebuildIndex() { + this._wp = whichPolygon({ features: [...this._resolved.values()] }); + } + + + /** + * mergeCustomGeoJSON + * Accepts a FeatureCollection-like object containing custom locations + * Each feature must have a filename-like `id`, for example: `something.geojson` + * { + * "type": "FeatureCollection" + * "features": [ + * { + * "type": "Feature", + * "id": "philly_metro.geojson", + * "properties": { … }, + * "geometry": { … } + * } + * ] + * } + * + * @param `fc` FeatureCollection-like Object containing custom locations + */ + mergeCustomGeoJSON(fc) { + if (!fc || fc.type !== 'FeatureCollection' || !Array.isArray(fc.features)) return; + + fc.features.forEach(feature => { + feature.properties = feature.properties || {}; + let props = feature.properties; + + // Get `id` from either `id` or `properties` + let id = feature.id || props.id; + if (!id || !/^\S+\.geojson$/i.test(id)) return; + + // Ensure `id` exists and is lowercase + id = id.toLowerCase(); + feature.id = id; + props.id = id; + + // Ensure `area` property exists + if (!props.area) { + const area = calcArea.geometry(feature.geometry) / 1e6; // m² to km² + props.area = Number(area.toFixed(2)); + } + + _loco._cache[id] = feature; // insert directly into LocationConflations internal cache + }); + } + + + /** + * mergeLocationSets + * Accepts an Array of Objects containing `locationSet` properties: + * [ + * { id: 'preset1', locationSet: {…} }, + * { id: 'preset2', locationSet: {…} }, + * … + * ] + * After validating, the Objects will be decorated with a `locationSetID` property: + * [ + * { id: 'preset1', locationSet: {…}, locationSetID: '+[Q2]' }, + * { id: 'preset2', locationSet: {…}, locationSetID: '+[Q30]' }, + * … + * ] + * + * @param `objects` Objects to check - they should have `locationSet` property + * @return Promise resolved true (this function used to be slow/async, now it's faster and sync) + */ + mergeLocationSets(objects) { + if (!Array.isArray(objects)) return Promise.reject('nothing to do'); + + objects.forEach(obj => this._validateLocationSet(obj)); + this._rebuildIndex(); + return Promise.resolve(objects); + } + + + /** + * locationSetID + * Returns a locationSetID for a given locationSet (fallback to `+[Q2]`, world) + * (The locationSet doesn't necessarily need to be resolved to compute its `id`) + * + * @param `locationSet` A locationSet Object, e.g. `{ include: ['us'] }` + * @return String locationSetID, e.g. `+[Q30]` + */ + locationSetID(locationSet) { + let locationSetID; + try { + locationSetID = _loco.validateLocationSet(locationSet).id; + } catch (err) { + locationSetID = '+[Q2]'; // the world + } + return locationSetID; + } + + + /** + * feature + * Returns the resolved GeoJSON feature for a given locationSetID (fallback to 'world') + * A GeoJSON feature: + * { + * type: 'Feature', + * id: '+[Q30]', + * properties: { id: '+[Q30]', area: 21817019.17, … }, + * geometry: { … } + * } + * + * @param `locationSetID` String identifier, e.g. `+[Q30]` + * @return GeoJSON object (fallback to world) + */ + feature(locationSetID = '+[Q2]') { + const feature = this._resolved.get(locationSetID); + return feature || this._resolved.get('+[Q2]'); + } + + + /** + * locationSetsAt + * Find all the locationSets valid at the given location. + * Results include the area (in km²) to facilitate sorting. + * + * Object of locationSetIDs to areas (in km²) + * { + * "+[Q2]": 511207893.3958111, + * "+[Q30]": 21817019.17, + * "+[new_jersey.geojson]": 22390.77, + * … + * } + * + * @param `loc` `[lon,lat]` location to query, e.g. `[-74.4813, 40.7967]` + * @return Object of locationSetIDs valid at given location + */ + locationSetsAt(loc) { + let result = {}; + + const hits = this._wp(loc, true) || []; + const thiz = this; + + // locationSets + hits.forEach(prop => { + if (prop.id[0] !== '+') return; // skip - it's a location + const locationSetID = prop.id; + const area = thiz._knownLocationSets.get(locationSetID); + if (area) { + result[locationSetID] = area; + } + }); + + // locations included + hits.forEach(prop => { + if (prop.id[0] === '+') return; // skip - it's a locationset + const locationID = prop.id; + const included = thiz._locationIncludedIn.get(locationID); + (included || []).forEach(locationSetID => { + const area = thiz._knownLocationSets.get(locationSetID); + if (area) { + result[locationSetID] = area; + } + }); + }); + + // locations excluded + hits.forEach(prop => { + if (prop.id[0] === '+') return; // skip - it's a locationset + const locationID = prop.id; + const excluded = thiz._locationExcludedIn.get(locationID); + (excluded || []).forEach(locationSetID => { + delete result[locationSetID]; + }); + }); + + return result; + } + + + // Direct access to the location-conflation resolver + loco() { + return _loco; + } +} + + +const _sharedLocationManager = new LocationManager(); +export { _sharedLocationManager as locationManager }; + diff --git a/modules/core/index.js b/modules/core/index.js index 98f01b97ef..5615d93f4e 100644 --- a/modules/core/index.js +++ b/modules/core/index.js @@ -4,7 +4,7 @@ export { coreDifference } from './difference'; export { coreGraph } from './graph'; export { coreHistory } from './history'; export { coreLocalizer, t, localizer } from './localizer'; -export { coreLocations, locationManager } from './locations'; +export { LocationManager, locationManager } from './LocationManager'; export { prefs } from './preferences'; export { coreTree } from './tree'; export { coreUploader } from './uploader'; diff --git a/modules/core/locations.js b/modules/core/locations.js deleted file mode 100644 index 49e4daa431..0000000000 --- a/modules/core/locations.js +++ /dev/null @@ -1,270 +0,0 @@ -import LocationConflation from '@ideditor/location-conflation'; -import whichPolygon from 'which-polygon'; -import calcArea from '@mapbox/geojson-area'; -import { utilArrayChunk } from '../util'; - -let _mainLocations = coreLocations(); // singleton -export { _mainLocations as locationManager }; - -// -// `coreLocations` maintains an internal index of all the boundaries/geofences used by iD. -// It's used by presets, community index, background imagery, to know where in the world these things are valid. -// These geofences should be defined by `locationSet` objects: -// -// let locationSet = { -// include: [ Array of locations ], -// exclude: [ Array of locations ] -// }; -// -// For more info see the location-conflation and country-coder projects, see: -// https://github.com/ideditor/location-conflation -// https://github.com/ideditor/country-coder -// -export function coreLocations() { - let _this = {}; - let _resolvedFeatures = {}; // cache of *resolved* locationSet features - let _loco = new LocationConflation(); // instance of a location-conflation resolver - let _wp; // instance of a which-polygon index - - // pre-resolve the worldwide locationSet - const world = { locationSet: { include: ['Q2'] } }; - resolveLocationSet(world); - rebuildIndex(); - - let _queue = []; - let _deferred = new Set(); - let _inProcess; - - - // Returns a Promise to process the queue - function processQueue() { - if (!_queue.length) return Promise.resolve(); - - // console.log(`queue length ${_queue.length}`); - const chunk = _queue.pop(); - return new Promise(resolvePromise => { - const handle = window.requestIdleCallback(() => { - _deferred.delete(handle); - // const t0 = performance.now(); - chunk.forEach(resolveLocationSet); - // const t1 = performance.now(); - // console.log('chunk processed in ' + (t1 - t0) + ' ms'); - resolvePromise(); - }); - _deferred.add(handle); - }) - .then(() => processQueue()); - } - - // Pass an Object with a `locationSet` property, - // Performs the locationSet resolution, caches the result, and sets a `locationSetID` property on the object. - function resolveLocationSet(obj) { - if (obj.locationSetID) return; // work was done already - - try { - let locationSet = obj.locationSet; - if (!locationSet) { - throw new Error('object missing locationSet property'); - } - if (!locationSet.include) { // missing `include`, default to worldwide include - locationSet.include = ['Q2']; // https://github.com/openstreetmap/iD/pull/8305#discussion_r662344647 - } - const resolved = _loco.resolveLocationSet(locationSet); - const locationSetID = resolved.id; - obj.locationSetID = locationSetID; - - if (!resolved.feature.geometry.coordinates.length || !resolved.feature.properties.area) { - throw new Error(`locationSet ${locationSetID} resolves to an empty feature.`); - } - if (!_resolvedFeatures[locationSetID]) { // First time seeing this locationSet feature - let feature = JSON.parse(JSON.stringify(resolved.feature)); // deep clone - feature.id = locationSetID; // Important: always use the locationSet `id` (`+[Q30]`), not the feature `id` (`Q30`) - feature.properties.id = locationSetID; - _resolvedFeatures[locationSetID] = feature; // insert into cache - } - } catch (err) { - obj.locationSet = { include: ['Q2'] }; // default worldwide - obj.locationSetID = '+[Q2]'; - } - } - - // Rebuilds the whichPolygon index with whatever features have been resolved. - function rebuildIndex() { - _wp = whichPolygon({ features: Object.values(_resolvedFeatures) }); - } - - // - // `mergeCustomGeoJSON` - // Accepts an FeatureCollection-like object containing custom locations - // Each feature must have a filename-like `id`, for example: `something.geojson` - // - // { - // "type": "FeatureCollection" - // "features": [ - // { - // "type": "Feature", - // "id": "philly_metro.geojson", - // "properties": { … }, - // "geometry": { … } - // } - // ] - // } - // - _this.mergeCustomGeoJSON = (fc) => { - if (fc && fc.type === 'FeatureCollection' && Array.isArray(fc.features)) { - fc.features.forEach(feature => { - feature.properties = feature.properties || {}; - let props = feature.properties; - - // Get `id` from either `id` or `properties` - let id = feature.id || props.id; - if (!id || !/^\S+\.geojson$/i.test(id)) return; - - // Ensure `id` exists and is lowercase - id = id.toLowerCase(); - feature.id = id; - props.id = id; - - // Ensure `area` property exists - if (!props.area) { - const area = calcArea.geometry(feature.geometry) / 1e6; // m² to km² - props.area = Number(area.toFixed(2)); - } - - _loco._cache[id] = feature; - }); - } - }; - - - // - // `mergeLocationSets` - // Accepts an Array of Objects containing `locationSet` properties. - // The locationSets will be resolved and indexed in the background. - // [ - // { id: 'preset1', locationSet: {…} }, - // { id: 'preset2', locationSet: {…} }, - // { id: 'preset3', locationSet: {…} }, - // … - // ] - // After resolving and indexing, the Objects will be decorated with a - // `locationSetID` property. - // [ - // { id: 'preset1', locationSet: {…}, locationSetID: '+[Q2]' }, - // { id: 'preset2', locationSet: {…}, locationSetID: '+[Q30]' }, - // { id: 'preset3', locationSet: {…}, locationSetID: '+[Q2]' }, - // … - // ] - // - // Returns a Promise fulfilled when the resolving/indexing has been completed - // This will take some seconds but happen in the background during browser idle time. - // - _this.mergeLocationSets = (objects) => { - if (!Array.isArray(objects)) return Promise.reject('nothing to do'); - - // Resolve all locationSets -> geojson, processing data in chunks - // - // Because this will happen during idle callbacks, we want to choose a chunk size - // that won't make the browser stutter too badly. LocationSets that are a simple - // country coder include will resolve instantly, but ones that involve complex - // include/exclude operations will take some milliseconds longer. - // - // Some discussion and performance results on these tickets: - // https://github.com/ideditor/location-conflation/issues/26 - // https://github.com/osmlab/name-suggestion-index/issues/4784#issuecomment-742003434 - _queue = _queue.concat(utilArrayChunk(objects, 200)); - - if (!_inProcess) { - _inProcess = processQueue() - .then(() => { - rebuildIndex(); - _inProcess = null; - return objects; - }); - } - return _inProcess; - }; - - - // - // `locationSetID` - // Returns a locationSetID for a given locationSet (fallback to `+[Q2]`, world) - // (The locationset doesn't necessarily need to be resolved to compute its `id`) - // - // Arguments - // `locationSet`: A locationSet, e.g. `{ include: ['us'] }` - // Returns - // The locationSetID, e.g. `+[Q30]` - // - _this.locationSetID = (locationSet) => { - let locationSetID; - try { - locationSetID = _loco.validateLocationSet(locationSet).id; - } catch (err) { - locationSetID = '+[Q2]'; // the world - } - return locationSetID; - }; - - - // - // `feature` - // Returns the resolved GeoJSON feature for a given locationSetID (fallback to 'world') - // - // Arguments - // `locationSetID`: id of the form like `+[Q30]` (United States) - // Returns - // A GeoJSON feature: - // { - // type: 'Feature', - // id: '+[Q30]', - // properties: { id: '+[Q30]', area: 21817019.17, … }, - // geometry: { … } - // } - _this.feature = (locationSetID) => _resolvedFeatures[locationSetID] || _resolvedFeatures['+[Q2]']; - - - // - // `locationsAt` - // Find all the resolved locationSets valid at the given location. - // Results include the area (in km²) to facilitate sorting. - // - // Arguments - // `loc`: the [lon,lat] location to query, e.g. `[-74.4813, 40.7967]` - // Returns - // Object of locationSetIDs to areas (in km²) - // { - // "+[Q2]": 511207893.3958111, - // "+[Q30]": 21817019.17, - // "+[new_jersey.geojson]": 22390.77, - // … - // } - // - _this.locationsAt = (loc) => { - let result = {}; - (_wp(loc, true) || []).forEach(prop => result[prop.id] = prop.area); - return result; - }; - - // - // `query` - // Execute a query directly against which-polygon - // https://github.com/mapbox/which-polygon - // - // Arguments - // `loc`: the [lon,lat] location to query, - // `multi`: `true` to return all results, `false` to return first result - // Returns - // Array of GeoJSON *properties* for the locationSet features that exist at `loc` - // - _this.query = (loc, multi) => _wp(loc, multi); - - // Direct access to the location-conflation resolver - _this.loco = () => _loco; - - // Direct access to the which-polygon index - _this.wp = () => _wp; - - - return _this; -} diff --git a/modules/presets/collection.js b/modules/presets/collection.js index 1968a3b0e4..1b054677f3 100644 --- a/modules/presets/collection.js +++ b/modules/presets/collection.js @@ -1,4 +1,4 @@ -import { locationManager } from '../core/locations'; +import { locationManager } from '../core/LocationManager'; import { utilArrayUniq } from '../util/array'; import { utilEditDistance } from '../util'; @@ -101,8 +101,8 @@ export function presetCollection(collection) { let pool = _this.collection; if (Array.isArray(loc)) { - const validLocations = locationManager.locationsAt(loc); - pool = pool.filter(a => !a.locationSetID || validLocations[a.locationSetID]); + const validHere = locationManager.locationSetsAt(loc); + pool = pool.filter(a => !a.locationSetID || validHere[a.locationSetID]); } const searchable = pool.filter(a => a.searchable !== false && a.suggestion !== true); diff --git a/modules/presets/index.js b/modules/presets/index.js index 1a46e52196..79cfab1330 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -2,7 +2,7 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; import { prefs } from '../core/preferences'; import { fileFetcher } from '../core/file_fetcher'; -import { locationManager } from '../core/locations'; +import { locationManager } from '../core/LocationManager'; import { osmNodeGeometriesForTags, osmSetAreaKeys, osmSetPointTags, osmSetVertexTags } from '../osm/tags'; import { presetCategory } from './category'; @@ -241,12 +241,12 @@ export function presetIndex() { } if (bestMatch && bestMatch.locationSetID && bestMatch.locationSetID !== '+[Q2]' && Array.isArray(loc)){ - let validLocations = locationManager.locationsAt(loc); - if (!validLocations[bestMatch.locationSetID]){ + const validHere = locationManager.locationSetsAt(loc); + if (!validHere[bestMatch.locationSetID]) { matchCandidates.sort((a, b) => (a.score < b.score) ? 1 : -1); - for (let i = 0; i < matchCandidates.length; i++){ + for (let i = 0; i < matchCandidates.length; i++) { const candidateScore = matchCandidates[i]; - if (!candidateScore.candidate.locationSetID || validLocations[candidateScore.candidate.locationSetID]){ + if (!candidateScore.candidate.locationSetID || validHere[candidateScore.candidate.locationSetID]) { bestMatch = candidateScore.candidate; bestScore = candidateScore.score; break; @@ -408,8 +408,8 @@ export function presetIndex() { ); if (Array.isArray(loc)) { - const validLocations = locationManager.locationsAt(loc); - result.collection = result.collection.filter(a => !a.locationSetID || validLocations[a.locationSetID]); + const validHere = locationManager.locationSetsAt(loc); + result.collection = result.collection.filter(a => !a.locationSetID || validHere[a.locationSetID]); } return result; diff --git a/modules/services/nsi.js b/modules/services/nsi.js index 09ec82d364..8664a364be 100644 --- a/modules/services/nsi.js +++ b/modules/services/nsi.js @@ -10,6 +10,7 @@ import { nsiCdnUrl } from '../../config/id.js'; // If you mess up the `../`s, the resolver may import another random package.json from somewhere else. import packageJSON from '../../package.json'; + // This service contains all the code related to the **name-suggestion-index** (aka NSI) // NSI contains the most correct tagging for many commonly mapped features. // See https://github.com/osmlab/name-suggestion-index and https://nsi.guide @@ -112,9 +113,61 @@ function loadNsiData() { ids: new Map() // Map (id -> NSI item) }; - _nsi.matcher = new Matcher(); - _nsi.matcher.buildMatchIndex(_nsi.data); - _nsi.matcher.buildLocationIndex(_nsi.data, locationManager.loco()); + const matcher = _nsi.matcher = new Matcher(); + matcher.buildMatchIndex(_nsi.data); + +// *** BEGIN HACK *** + +// old - built in matcher will set up the locationindex by resolving all the locationSets one-by-one + // matcher.buildLocationIndex(_nsi.data, locationManager.loco()); + +// new - Use the location manager instead of redoing that work +// It has already processed the presets at this point + +// We need to monkeypatch a few of the collections that the NSI matcher depends on. +// The `itemLocation` structure maps itemIDs to locationSetIDs +matcher.itemLocation = new Map(); + +// The `locationSets` structure maps locationSetIDs to GeoJSON +// We definitely need this, but don't need full geojson, just { properties: { area: xxx }} +matcher.locationSets = new Map(); + +Object.keys(_nsi.data).forEach(tkv => { + const items = _nsi.data[tkv].items; + if (!Array.isArray(items) || !items.length) return; + + items.forEach(item => { + if (matcher.itemLocation.has(item.id)) return; // we've seen item id already - shouldn't be possible? + + const locationSetID = locationManager.locationSetID(item.locationSet); + matcher.itemLocation.set(item.id, locationSetID); + + if (matcher.locationSets.has(locationSetID)) return; // we've seen this locationSet before.. + + const fakeFeature = { id: locationSetID, properties: { id: locationSetID, area: 1 } }; + matcher.locationSets.set(locationSetID, fakeFeature); + }); +}); + +// The `locationIndex` is an instance of which-polygon spatial index for the locationSets. +// We only really need this to _look like_ which-polygon query `_wp.locationIndex(bbox, true);` +// i.e. it needs to return the properties of the locationsets +matcher.locationIndex = (bbox) => { + const validHere = locationManager.locationSetsAt([bbox[0], bbox[1]]); + const results = []; + + for (const [locationSetID, area] of Object.entries(validHere)) { + const fakeFeature = matcher.locationSets.get(locationSetID); + if (fakeFeature) { + fakeFeature.properties.area = area; + results.push(fakeFeature); + } + } + return results; +}; + +// *** END HACK *** + Object.keys(_nsi.data).forEach(tkv => { const category = _nsi.data[tkv]; @@ -461,13 +514,10 @@ function _upgradeTags(tags, loc) { return changed ? { newTags: newTags, matched: null } : null; } - // Order the [key,value,name] tuples - test primary names before alternate names + // Order the [key,value,name] tuples - test primary before alternate const tuples = gatherTuples(tryKVs, tryNames); - let foundPrimary = false; - let bestItem; - // Test [key,value,name] tuples against the NSI matcher until we get a primary match or exhaust all options. - for (let i = 0; (i < tuples.length && !foundPrimary); i++) { + for (let i = 0; i < tuples.length; i++) { const tuple = tuples[i]; const hits = _nsi.matcher.match(tuple.k, tuple.v, tuple.n, loc); // Attempt to match an item in NSI @@ -476,15 +526,14 @@ function _upgradeTags(tags, loc) { // A match may contain multiple results, the first one is likely the best one for this location // e.g. `['pfk-a54c14', 'kfc-1ff19c', 'kfc-658eea']` + let itemID, item; for (let j = 0; j < hits.length; j++) { const hit = hits[j]; - const isPrimary = (hits[j].match === 'primary'); - const itemID = hit.itemID; + itemID = hit.itemID; if (_nsi.dissolved[itemID]) continue; // Don't upgrade to a dissolved item - const item = _nsi.ids.get(itemID); + item = _nsi.ids.get(itemID); if (!item) continue; - const mainTag = item.mainTag; // e.g. `brand:wikidata` const itemQID = item.tags[mainTag]; // e.g. `brand:wikidata` qid const notQID = newTags[`not:${mainTag}`]; // e.g. `not:brand:wikidata` qid @@ -493,25 +542,18 @@ function _upgradeTags(tags, loc) { (!itemQID || itemQID === notQID) || // No `*:wikidata` or matched a `not:*:wikidata` (newTags.office && !item.tags.office) // feature may be a corporate office for a brand? - #6416 ) { + item = null; continue; // continue looking - } - - // If we get here, the hit is good.. - if (!bestItem || isPrimary) { - bestItem = item; - if (isPrimary) { - foundPrimary = true; - } - break; // can ignore the rest of the hits from this match + } else { + break; // use `item` } } - } + // Can't use any of these hits, try next tuple.. + if (!item) continue; - // At this point we have matched a canonical item and can suggest tag upgrades.. - if (bestItem) { - const itemID = bestItem.id; - const item = JSON.parse(JSON.stringify(bestItem)); // deep copy + // At this point we have matched a canonical item and can suggest tag upgrades.. + item = JSON.parse(JSON.stringify(item)); // deep copy const tkv = item.tkv; const parts = tkv.split('/', 3); // tkv = "tree/key/value" const k = parts[1]; @@ -657,17 +699,9 @@ export default { setNsiSources(); presetManager.ensureLoaded() .then(() => loadNsiPresets()) - .then(() => delay(100)) // wait briefly for locationSets to enter the locationManager queue - .then(() => locationManager.mergeLocationSets([])) // wait for locationSets to resolve .then(() => loadNsiData()) .then(() => _nsiStatus = 'ok') .catch(() => _nsiStatus = 'failed'); - - function delay(msec) { - return new Promise(resolve => { - window.setTimeout(resolve, msec); - }); - } }, diff --git a/modules/ui/field.js b/modules/ui/field.js index 1702a2784d..416b15cc1d 100644 --- a/modules/ui/field.js +++ b/modules/ui/field.js @@ -2,7 +2,7 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; import { select as d3_select } from 'd3-selection'; import { t, localizer } from '../core/localizer'; -import { locationManager } from '../core/locations'; +import { locationManager } from '../core/LocationManager'; import { svgIcon } from '../svg/icon'; import { uiTooltip } from './tooltip'; import { geoExtent } from '../geo/extent'; @@ -310,8 +310,8 @@ export function uiField(context, presetField, entityIDs, options) { })) return false; if (entityIDs && _entityExtent && field.locationSetID) { // is field allowed in this location? - var validLocations = locationManager.locationsAt(_entityExtent.center()); - if (!validLocations[field.locationSetID]) return false; + var validHere = locationManager.locationSetsAt(_entityExtent.center()); + if (!validHere[field.locationSetID]) return false; } var prerequisiteTag = field.prerequisiteTag; diff --git a/modules/ui/success.js b/modules/ui/success.js index 42d4836e88..91369fbdd5 100644 --- a/modules/ui/success.js +++ b/modules/ui/success.js @@ -4,7 +4,7 @@ import { select as d3_select } from 'd3-selection'; import { resolveStrings } from 'osm-community-index'; import { fileFetcher } from '../core/file_fetcher'; -import { locationManager } from '../core/locations'; +import { locationManager } from '../core/LocationManager'; import { t, localizer } from '../core/localizer'; import { svgIcon } from '../svg/icon'; @@ -160,12 +160,12 @@ export function uiSuccess(context) { ensureOSMCommunityIndex() .then(oci => { const loc = context.map().center(); - const validLocations = locationManager.locationsAt(loc); + const validHere = locationManager.locationSetsAt(loc); // Gather the communities let communities = []; oci.resources.forEach(resource => { - let area = validLocations[resource.locationSetID]; + let area = validHere[resource.locationSetID]; if (!area) return; // Resolve strings diff --git a/test/spec/core/LocationManager.js b/test/spec/core/LocationManager.js new file mode 100644 index 0000000000..955ba81691 --- /dev/null +++ b/test/spec/core/LocationManager.js @@ -0,0 +1,153 @@ +describe('LocationManager', () => { + let locationManager; + + const colorado = { + type: 'Feature', + id: 'colorado.geojson', + properties: {}, + geometry: { + type: 'Polygon', + coordinates: [ + [ + [-107.9197, 41.0039], + [-102.0539, 41.0039], + [-102.043, 36.9948], + [-109.0425, 37.0003], + [-109.048, 40.9984], + [-107.9197, 41.0039] + ] + ] + } + }; + + const fc = { type: 'FeatureCollection', features: [colorado] }; + + + beforeEach(() => { + // make a new one each time, so we aren't accidentally testing the "global" locationManager + locationManager = new iD.LocationManager(); + }); + + + describe('#mergeCustomGeoJSON', () => { + it('merges geojson into lococation-conflation cache', () => { + locationManager.mergeCustomGeoJSON(fc); + expect(locationManager.loco()._cache['colorado.geojson']).to.be.eql(colorado); + }); + }); + + + describe('#mergeLocationSets', () => { + it('returns a promise rejected if not passed an array', done => { + const prom = locationManager.mergeLocationSets({}); + prom + .then(() => { + done(new Error('This was supposed to fail, but somehow succeeded.')); + }) + .catch(err => { + expect(/^nothing to do/.test(err)).to.be.true; + done(); + }); + + window.setTimeout(() => {}, 20); // async - to let the promise settle in phantomjs + }); + + it('resolves locationSets, assigning locationSetID', done => { + const data = [ + { id: 'world', locationSet: { include: ['001'] } }, + { id: 'usa', locationSet: { include: ['usa'] } } + ]; + + const prom = locationManager.mergeLocationSets(data); + prom + .then(data => { + expect(data).to.be.a('array'); + expect(data[0].locationSetID).to.eql('+[Q2]'); + expect(data[1].locationSetID).to.eql('+[Q30]'); + done(); + }) + .catch(err => done(err)); + + window.setTimeout(() => {}, 20); // async - to let the promise settle in phantomjs + }); + + it('resolves locationSets, falls back to world locationSetID on errror', done => { + const data = [ + { id: 'bogus1', locationSet: { foo: 'bar' } }, + { id: 'bogus2', locationSet: { include: ['fake.geojson'] } } + ]; + + const prom = locationManager.mergeLocationSets(data); + prom + .then(data => { + expect(data).to.be.a('array'); + expect(data[0].locationSetID).to.eql('+[Q2]'); + expect(data[1].locationSetID).to.eql('+[Q2]'); + done(); + }) + .catch(err => done(err)); + + window.setTimeout(() => {}, 20); // async - to let the promise settle in phantomjs + }); + }); + + + describe('#locationSetID', () => { + it('calculates a locationSetID for a locationSet', () => { + expect(locationManager.locationSetID({ include: ['usa'] })).to.be.eql('+[Q30]'); + }); + + it('falls back to the world locationSetID in case of errors', () => { + expect(locationManager.locationSetID({ foo: 'bar' })).to.be.eql('+[Q2]'); + expect(locationManager.locationSetID({ include: ['fake.geojson'] })).to.be.eql('+[Q2]'); + }); + }); + + + describe('#feature', () => { + it('has the world locationSet pre-resolved', () => { + const result = locationManager.feature('+[Q2]'); + expect(result).to.include({ type: 'Feature', id: '+[Q2]' }); + }); + + it('falls back to the world locationSetID in case of errors', () => { + const result = locationManager.feature('fake'); + expect(result).to.include({ type: 'Feature', id: '+[Q2]' }); + }); + }); + + + describe('#locationSetsAt', () => { + it('has the world locationSet pre-resolved', () => { + const result1 = locationManager.locationSetsAt([-108.557, 39.065]); // Grand Junction + expect(result1).to.be.an('object').that.has.all.keys('+[Q2]'); + const result2 = locationManager.locationSetsAt([-74.481, 40.797]); // Morristown + expect(result2).to.be.an('object').that.has.all.keys('+[Q2]'); + const result3 = locationManager.locationSetsAt([13.575, 41.207,]); // Gaeta + expect(result3).to.be.an('object').that.has.all.keys('+[Q2]'); + }); + + it('returns valid locationSets at a given lon,lat', done => { + // setup, load colorado.geojson and resolve some locationSets + locationManager.mergeCustomGeoJSON(fc); + locationManager.mergeLocationSets([ + { id: 'OSM-World', locationSet: { include: ['001'] } }, + { id: 'OSM-USA', locationSet: { include: ['us'] } }, + { id: 'OSM-Colorado', locationSet: { include: ['colorado.geojson'] } } + ]) + .then(() => { + const result1 = locationManager.locationSetsAt([-108.557, 39.065]); // Grand Junction + expect(result1).to.be.an('object').that.has.all.keys('+[Q2]', '+[Q30]', '+[colorado.geojson]'); + const result2 = locationManager.locationSetsAt([-74.481, 40.797]); // Morristown + expect(result2).to.be.an('object').that.has.all.keys('+[Q2]', '+[Q30]'); + const result3 = locationManager.locationSetsAt([13.575, 41.207,]); // Gaeta + expect(result3).to.be.an('object').that.has.all.keys('+[Q2]'); + done(); + }) + .catch(err => done(err)); + + window.setTimeout(() => {}, 20); // async - to let the promise settle in phantomjs + }); + }); + +}); diff --git a/test/spec/core/locations.js b/test/spec/core/locations.js deleted file mode 100644 index 5931d36a69..0000000000 --- a/test/spec/core/locations.js +++ /dev/null @@ -1,150 +0,0 @@ -describe('iD.coreLocations', function() { - var locationManager, loco; - - var colorado = { - type: 'Feature', - id: 'colorado.geojson', - properties: {}, - geometry: { - type: 'Polygon', - coordinates: [ - [ - [-107.9197, 41.0039], - [-102.0539, 41.0039], - [-102.043, 36.9948], - [-109.0425, 37.0003], - [-109.048, 40.9984], - [-107.9197, 41.0039] - ] - ] - } - }; - - var fc = { type: 'FeatureCollection', features: [colorado] }; - - - beforeEach(function() { - // make a new one each time, so we aren't accidentally testing the "global" locationManager - locationManager = iD.coreLocations(); - loco = locationManager.loco(); - }); - - - describe('#mergeCustomGeoJSON', function() { - it('merges geojson into lococation-conflation cache', function() { - locationManager.mergeCustomGeoJSON(fc); - expect(loco._cache['colorado.geojson']).to.be.eql(colorado); - }); - }); - - - describe('#mergeLocationSets', function() { - it('returns a promise rejected if not passed an array', function(done) { - var prom = locationManager.mergeLocationSets({}); - prom - .then(function() { - done(new Error('This was supposed to fail, but somehow succeeded.')); - }) - .catch(function(err) { - expect(/^nothing to do/.test(err)).to.be.true; - done(); - }); - }); - - it('resolves locationSets, assigning locationSetID', function(done) { - var data = [ - { id: 'world', locationSet: { include: ['001'] } }, - { id: 'usa', locationSet: { include: ['usa'] } } - ]; - var prom = locationManager.mergeLocationSets(data); - prom - .then(function(data) { - expect(data).to.be.a('array'); - expect(data[0].locationSetID).to.eql('+[Q2]'); - expect(data[1].locationSetID).to.eql('+[Q30]'); - done(); - }) - .catch(function(err) { - done(err); - }); - }); - - it('resolves locationSets, falls back to world locationSetID on errror', function(done) { - var data = [ - { id: 'bogus1', locationSet: { foo: 'bar' } }, - { id: 'bogus2', locationSet: { include: ['fake.geojson'] } } - ]; - var prom = locationManager.mergeLocationSets(data); - prom - .then(function(data) { - expect(data).to.be.a('array'); - expect(data[0].locationSetID).to.eql('+[Q2]'); - expect(data[1].locationSetID).to.eql('+[Q2]'); - done(); - }) - .catch(function(err) { - done(err); - }); - }); - }); - - - describe('#locationSetID', function() { - it('calculates a locationSetID for a locationSet', function() { - expect(locationManager.locationSetID({ include: ['usa'] })).to.be.eql('+[Q30]'); - }); - - it('falls back to the world locationSetID in case of errors', function() { - expect(locationManager.locationSetID({ foo: 'bar' })).to.be.eql('+[Q2]'); - expect(locationManager.locationSetID({ include: ['fake.geojson'] })).to.be.eql('+[Q2]'); - }); - }); - - - describe('#feature', function() { - it('has the world locationSet pre-resolved', function() { - var result = locationManager.feature('+[Q2]'); - expect(result).to.include({ type: 'Feature', id: '+[Q2]' }); - }); - - it('falls back to the world locationSetID in case of errors', function() { - var result = locationManager.feature('fake'); - expect(result).to.include({ type: 'Feature', id: '+[Q2]' }); - }); - }); - - - describe('#locationsAt', function() { - it('has the world locationSet pre-resolved', function() { - var result1 = locationManager.locationsAt([-108.557, 39.065]); // Grand Junction - expect(result1).to.be.an('object').that.has.all.keys('+[Q2]'); - var result2 = locationManager.locationsAt([-74.481, 40.797]); // Morristown - expect(result2).to.be.an('object').that.has.all.keys('+[Q2]'); - var result3 = locationManager.locationsAt([13.575, 41.207,]); // Gaeta - expect(result3).to.be.an('object').that.has.all.keys('+[Q2]'); - }); - - it('returns valid locations at a given lon,lat', function(done) { - // setup, load colorado.geojson and resolve some locationSets - locationManager.mergeCustomGeoJSON(fc); - locationManager.mergeLocationSets([ - { id: 'OSM-World', locationSet: { include: ['001'] } }, - { id: 'OSM-USA', locationSet: { include: ['us'] } }, - { id: 'OSM-Colorado', locationSet: { include: ['colorado.geojson'] } } - ]) - .then(function() { - var result1 = locationManager.locationsAt([-108.557, 39.065]); // Grand Junction - expect(result1).to.be.an('object').that.has.all.keys('+[Q2]', '+[Q30]', '+[colorado.geojson]'); - var result2 = locationManager.locationsAt([-74.481, 40.797]); // Morristown - expect(result2).to.be.an('object').that.has.all.keys('+[Q2]', '+[Q30]'); - var result3 = locationManager.locationsAt([13.575, 41.207,]); // Gaeta - expect(result3).to.be.an('object').that.has.all.keys('+[Q2]'); - done(); - }) - .catch(function(err) { - done(err); - }); - }); - }); - -});