diff --git a/src/pip/index.js b/src/pip/index.js index 7d83819..6d14c9d 100644 --- a/src/pip/index.js +++ b/src/pip/index.js @@ -108,6 +108,7 @@ module.exports.create = function createPIPService(datapath, layers, localizedAdm // bookkeeping object that tracks the progress of the request responseQueue[id] = { latLon: {latitude: latitude, longitude: longitude}, + hierarchy: [], // copy of layers to search search_layers: search_layers.slice(), responseCallback: responseCallback @@ -192,19 +193,47 @@ function handleResults(msg) { // if there are no results, then call the next layer but only if there are more layer to call searchWorker(msg.id); } else { - // no layers left to search, so return an empty array - responseQueue[msg.id].responseCallback(null, []); + // no layers left to search, check if there is a partial hierarchy from an untrusted layer + if (responseQueue[msg.id].hierarchy.length > 0) { + responseQueue[msg.id].responseCallback(null, responseQueue[msg.id].hierarchy); + } else { + //respond with empty aresult + responseQueue[msg.id].responseCallback(null, []); + } delete responseQueue[msg.id]; } } else { // there was a hit, so find the hierachy and assemble all the pieces - const results = _.compact(msg.results.Hierarchy[0].map(id => wofData[id])); + const untrustedLayers = ['neighbourhood']; - responseQueue[msg.id].responseCallback(null, results); + if (untrustedLayers.includes(msg.layer)) { + // push _only_ the first layers results onto the hierarchy + const this_layer_id = msg.results.Id; + if (this_layer_id) { + responseQueue[msg.id].hierarchy.push(wofData[this_layer_id]); + } - delete responseQueue[msg.id]; - } + // continue search if there are more layers. Otherwise return full hierarchy + if (responseQueue[msg.id].search_layers !== 0) { + searchWorker(msg.id); + } else { + // assemble full hierarchy and return + const results = _.compact(msg.results.Hierarchy[0].map(id => wofData[id])); + + responseQueue[msg.id].responseCallback(null, results); + + delete responseQueue[msg.id]; + } + + } else { + // assemble full hierarchy and return + const results = responseQueue[msg.id].hierarchy.concat(_.compact(msg.results.Hierarchy[0].map(id => wofData[id]))); + + responseQueue[msg.id].responseCallback(null, results); + delete responseQueue[msg.id]; + } + } } diff --git a/src/pip/worker.js b/src/pip/worker.js index 98fd38e..5380ca3 100644 --- a/src/pip/worker.js +++ b/src/pip/worker.js @@ -33,10 +33,11 @@ process.on('SIGTERM', () => { readStream(datapath, layer, localizedAdminNames, (features) => { // find all the properties of all features and write them to a file - // at the same time, limit the feature.properties to just Id since it's all that's needed in the worker + // at the same time, limit the feature.properties to just Id and Hierarchy since it's all that's needed in the worker const data = features.reduce((acc, feature) => { acc[feature.properties.Id] = feature.properties; feature.properties = { + Id: feature.properties.Id, Hierarchy: feature.properties.Hierarchy }; return acc; diff --git a/test/pip/index.js b/test/pip/index.js index d5aab08..f4da56c 100644 --- a/test/pip/index.js +++ b/test/pip/index.js @@ -66,7 +66,7 @@ tape('PiP tests', test => { }); - test.test('requested lat/lon inside a polygon should return that record\'s hierarchy', t => { + test.test('requested lat/lon inside a neighbourhood polygon should return only neighbourhood', t => { temp.mkdir('tmp_wof_data', (err, temp_dir) => { fs.mkdirSync(path.join(temp_dir, 'data')); fs.mkdirSync(path.join(temp_dir, 'meta')); @@ -138,6 +138,7 @@ tape('PiP tests', test => { fs.writeFileSync(path.join(temp_dir, 'data', 'borough_record.geojson'), JSON.stringify(borough_record)); const service = pip.create(temp_dir, ['neighbourhood', 'borough'], false, (err, o) => { + // lookup of point that only hits neighbourhood will NOT return borough o.lookup(1.5, 1.5, undefined, (err, results) => { t.deepEquals(results, [ { @@ -150,6 +151,104 @@ tape('PiP tests', test => { }, BoundingBox: '1,1,2,2', Hierarchy: [ [ 123, 456 ] ] + } + ]); + // must be explicitly ended or the test hangs + o.end(); + t.end(); + }); + + }); + + }); + + }); + + test.test('requested lat/lon inside a neighbourhood and parent borough should return full hierarchy', t => { + temp.mkdir('tmp_wof_data', (err, temp_dir) => { + fs.mkdirSync(path.join(temp_dir, 'data')); + fs.mkdirSync(path.join(temp_dir, 'meta')); + + // write out the WOF meta files with the minimum required fields + fs.writeFileSync( + path.join(temp_dir, 'meta', 'whosonfirst-data-neighbourhood-latest.csv'), + `id,name,path${EOL}123,place name,neighbourhood_record.geojson${EOL}`); + fs.writeFileSync( + path.join(temp_dir, 'meta', 'whosonfirst-data-borough-latest.csv'), + `id,name,path${EOL}456,borough name,borough_record.geojson${EOL}`); + + // setup a neighbourhood WOF record + const neighbourhood_record = { + id: 123, + type: 'Feature', + properties: { + 'geom:bbox': '1,1,4,4', + 'geom:latitude': 1.5, + 'geom:longitude': 1.5, + 'mz:hierarchy_label': 1, + 'wof:hierarchy': [ + { + locality_id: 123, + region_id: 456 + } + ], + 'wof:id': 123, + 'wof:name': 'neighbourhood name', + 'wof:placetype': 'neighbourhood' + }, + geometry: { + coordinates: [ + [ + [1,1],[4,1],[4,4],[1,4],[1,1] + ] + ], + type: 'Polygon' + } + }; + + // setup a borough WOF record that doesn't contain the lookup point to + // show that hierarchy is used to establish the response + const borough_record = { + id: 456, + type: 'Feature', + properties: { + 'geom:bbox': '3,3,4,4', + 'geom:latitude': 3.5, + 'geom:longitude': 3.5, + 'mz:hierarchy_label': 1, + 'wof:hierarchy': [], + 'wof:id': 456, + 'wof:name': 'borough name', + 'wof:placetype': 'borough' + }, + geometry: { + coordinates: [ + [ + [3,3],[3,4],[4,4],[4,3],[3,3] + ] + ], + type: 'Polygon' + } + }; + + // and write the records to file + fs.writeFileSync(path.join(temp_dir, 'data', 'neighbourhood_record.geojson'), JSON.stringify(neighbourhood_record)); + fs.writeFileSync(path.join(temp_dir, 'data', 'borough_record.geojson'), JSON.stringify(borough_record)); + + const service = pip.create(temp_dir, ['neighbourhood', 'borough'], false, (err, o) => { + // lookup of point that only hits neighbourhood will NOT return borough + o.lookup(3.5, 3.5, undefined, (err, results) => { + t.deepEquals(results, [ + { + Id: 123, + Name: 'neighbourhood name', + Placetype: 'neighbourhood', + Centroid: { + lat: 1.5, + lon: 1.5 + }, + BoundingBox: '1,1,4,4', + Hierarchy: [ [ 123, 456 ] ] }, { Id: 456, diff --git a/test/pip/worker.js b/test/pip/worker.js index cbb4c8d..d5d0545 100644 --- a/test/pip/worker.js +++ b/test/pip/worker.js @@ -78,6 +78,7 @@ tape('worker tests', (test) => { type: 'results', layer: 'test_layer', results: { + Id: 17, Hierarchy: [ [11, 13] ] } }, 'the hierarchy of the WOF record should be returned');