From 28e8bf69cf93da58273a53358d0dc8e828c78e76 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 8 Sep 2017 15:15:14 -0400 Subject: [PATCH 01/42] Remove olde CircleCI config This configuration was still around before we added a new one in https://github.com/pelias/api/pull/956. --- circle.yml | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100644 circle.yml diff --git a/circle.yml b/circle.yml deleted file mode 100644 index 59a2989c..00000000 --- a/circle.yml +++ /dev/null @@ -1,17 +0,0 @@ -machine: - ruby: - version: 2.1.2 - node: - version: 0.12.2 - -deployment: - dev: - branch: master - commands: - - git clone git@github.com:mapzen/pelias-deploy.git && cd pelias-deploy && bundle install - - cd pelias-deploy && bundle exec rake deploy:api dev - prod_build: - branch: staging - commands: - - git clone git@github.com:mapzen/pelias-deploy.git && cd pelias-deploy && bundle install - - cd pelias-deploy && bundle exec rake deploy:api prod_build From a233e11d59b4bfc0483033e5edc38e922fca0635 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 8 Sep 2017 15:23:32 -0400 Subject: [PATCH 02/42] Remove Access-Control-Allow-Credentials header This header is only relevant if dealing with authentication via cookies or other methods in HTTP requests. The Pelias API intentionally doesn't deal with authentication at all, assuming anyone who wants authentication will deal with it using a service placed between Pelias and end users. Additionally the CORS spec [does not allow](https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS#Credentialed_requests_and_wildcards) specifying this header while setting a wildcard for `Access-Control-Allow-Origin`, so it can cause problems in some cases. Fixes https://github.com/pelias/api/issues/971 --- middleware/cors.js | 3 +-- test/ciao/CORS/headers_GET.coffee | 1 - test/ciao/CORS/headers_OPTIONS.coffee | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/middleware/cors.js b/middleware/cors.js index d090f46f..6d30529f 100644 --- a/middleware/cors.js +++ b/middleware/cors.js @@ -3,8 +3,7 @@ function middleware(req, res, next){ res.header('Access-Control-Allow-Origin', '*'); res.header('Access-Control-Allow-Methods', 'GET, OPTIONS'); res.header('Access-Control-Allow-Headers', 'X-Requested-With,content-type'); - res.header('Access-Control-Allow-Credentials', true); next(); } -module.exports = middleware; \ No newline at end of file +module.exports = middleware; diff --git a/test/ciao/CORS/headers_GET.coffee b/test/ciao/CORS/headers_GET.coffee index 53866718..185838ab 100644 --- a/test/ciao/CORS/headers_GET.coffee +++ b/test/ciao/CORS/headers_GET.coffee @@ -6,4 +6,3 @@ path: '/' response.should.have.header 'Access-Control-Allow-Origin','*' response.should.have.header 'Access-Control-Allow-Methods','GET, OPTIONS' response.should.have.header 'Access-Control-Allow-Headers','X-Requested-With,content-type' -response.should.have.header 'Access-Control-Allow-Credentials','true' \ No newline at end of file diff --git a/test/ciao/CORS/headers_OPTIONS.coffee b/test/ciao/CORS/headers_OPTIONS.coffee index 5575391a..3cb47b37 100644 --- a/test/ciao/CORS/headers_OPTIONS.coffee +++ b/test/ciao/CORS/headers_OPTIONS.coffee @@ -7,4 +7,3 @@ method: 'OPTIONS' response.should.have.header 'Access-Control-Allow-Origin','*' response.should.have.header 'Access-Control-Allow-Methods','GET, OPTIONS' response.should.have.header 'Access-Control-Allow-Headers','X-Requested-With,content-type' -response.should.have.header 'Access-Control-Allow-Credentials','true' \ No newline at end of file From e572dbc439a2c6bc0102963650072931d858df21 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Fri, 15 Sep 2017 22:32:32 +0000 Subject: [PATCH 03/42] fix(package): update pelias-text-analyzer to version 1.9.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c5f731a9..30587df2 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,7 @@ "pelias-model": "5.0.1", "pelias-query": "9.1.0", "pelias-sorting": "1.0.1", - "pelias-text-analyzer": "1.9.1", + "pelias-text-analyzer": "1.9.2", "predicates": "^1.0.1", "retry": "^0.10.1", "stats-lite": "^2.0.4", From 7acb1c094a43202a40f3ec9970c582982ef5a784 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Sat, 16 Sep 2017 02:02:29 +0000 Subject: [PATCH 04/42] fix(package): update pelias-config to version 2.12.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c5f731a9..5e273a2c 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,7 @@ "markdown": "0.5.0", "morgan": "^1.8.2", "pelias-categories": "1.2.0", - "pelias-config": "2.12.0", + "pelias-config": "2.12.1", "pelias-labels": "1.6.0", "pelias-logger": "0.2.0", "pelias-microservice-wrapper": "1.2.0", From 7f516ab1b52a9c4cbef909cbb02c784d655bd8d0 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Sat, 16 Sep 2017 23:53:20 +0000 Subject: [PATCH 05/42] chore(package): update semantic-release to version 8.0.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c5f731a9..b3ce0c48 100644 --- a/package.json +++ b/package.json @@ -78,7 +78,7 @@ "pelias-mock-logger": "1.1.1", "precommit-hook": "^3.0.0", "proxyquire": "^1.7.10", - "semantic-release": "^7.0.1", + "semantic-release": "^8.0.0", "source-map": "^0.5.6", "tap-dot": "1.0.5", "tape": "^4.5.1", From af25ab9763a02155698c02ef5e93698e0a59d32d Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Tue, 19 Sep 2017 19:02:08 +0000 Subject: [PATCH 06/42] chore(package): update pelias-mock-logger to version 1.2.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c5f731a9..fa74e2a8 100644 --- a/package.json +++ b/package.json @@ -75,7 +75,7 @@ "jshint": "^2.5.6", "npm-check": "git://github.com/orangejulius/npm-check.git#disable-update-check", "nsp": "^2.2.0", - "pelias-mock-logger": "1.1.1", + "pelias-mock-logger": "1.2.0", "precommit-hook": "^3.0.0", "proxyquire": "^1.7.10", "semantic-release": "^7.0.1", From 7aacf987ea00f4d7702606441ab30894e5dbd870 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Wed, 20 Sep 2017 16:26:59 +0000 Subject: [PATCH 07/42] fix(package): update pelias-model to version 5.1.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c5f731a9..98fdcaff 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "pelias-labels": "1.6.0", "pelias-logger": "0.2.0", "pelias-microservice-wrapper": "1.2.0", - "pelias-model": "5.0.1", + "pelias-model": "5.1.0", "pelias-query": "9.1.0", "pelias-sorting": "1.0.1", "pelias-text-analyzer": "1.9.1", From e67e49421bfc9078fdb56271c6231303f21d1151 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 20 Sep 2017 17:42:03 -0400 Subject: [PATCH 08/42] updated test --- test/unit/controller/search_with_ids.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/controller/search_with_ids.js b/test/unit/controller/search_with_ids.js index e4c1d21d..ff884be9 100644 --- a/test/unit/controller/search_with_ids.js +++ b/test/unit/controller/search_with_ids.js @@ -388,7 +388,7 @@ module.exports.tests.service_errors = (test, common) => { const next = () => { t.deepEqual(logger.getInfoMessages(), [ - '[req]', + '[req] endpoint=undefined {}', 'request timed out on attempt 1, retrying', 'request timed out on attempt 2, retrying', 'request timed out on attempt 3, retrying' From 78beaece67a3c7172abc4f96322540f430e64ccb Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Thu, 21 Sep 2017 14:38:41 +0000 Subject: [PATCH 09/42] fix(package): update pelias-microservice-wrapper to version 1.2.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2fc606cc..a73785a4 100644 --- a/package.json +++ b/package.json @@ -58,7 +58,7 @@ "pelias-config": "2.12.1", "pelias-labels": "1.6.0", "pelias-logger": "0.2.0", - "pelias-microservice-wrapper": "1.2.0", + "pelias-microservice-wrapper": "1.2.1", "pelias-model": "5.1.0", "pelias-query": "9.1.0", "pelias-sorting": "1.0.1", From 1dc115d08fad25c5f75764f8c06e06e26244d6ba Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Thu, 21 Sep 2017 11:47:09 -0400 Subject: [PATCH 10/42] Use NPX to run Greenkeeper with Node.js 8 --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 28a218c8..a86d7215 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,6 +9,7 @@ matrix: fast_finish: true env: global: + - BUILD_LEADER_ID=2 - CXX=g++-4.8 script: npm run travis addons: @@ -22,7 +23,8 @@ before_install: before_script: - npm prune after_success: - - npm run semantic-release + - npm install -g npx + - npx -p node@8 npm run semantic-release branches: except: - /^v\d+\.\d+\.\d+$/ From c6e5611863213d678bbb48c14e772587c64b5400 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Thu, 21 Sep 2017 11:48:16 -0400 Subject: [PATCH 11/42] Remove install of gcc 4.8 in TravisCI --- .travis.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index a86d7215..3ce77f80 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,14 +10,7 @@ matrix: env: global: - BUILD_LEADER_ID=2 - - CXX=g++-4.8 script: npm run travis -addons: - apt: - sources: - - ubuntu-toolchain-r-test - packages: - - g++-4.8 before_install: - npm i -g npm@^3.0.0 before_script: From c72822b8055725bb81dfc975731c33a822d3c9d9 Mon Sep 17 00:00:00 2001 From: missinglink Date: Mon, 25 Sep 2017 17:40:47 +0200 Subject: [PATCH 12/42] feat(coarse_reverse): add try/catch block around synthesizeDoc code --- controller/coarse_reverse.js | 71 +++++++++++++++----------- test/unit/controller/coarse_reverse.js | 54 ++++++++++++++++++++ 2 files changed, 96 insertions(+), 29 deletions(-) diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js index e49dc8c9..1ed47c58 100644 --- a/controller/coarse_reverse.js +++ b/controller/coarse_reverse.js @@ -62,40 +62,50 @@ function synthesizeDoc(results) { const most_granular_layer = getMostGranularLayerOfResult(_.keys(results)); const id = results[most_granular_layer][0].id; - const doc = new Document('whosonfirst', most_granular_layer, id.toString()); - doc.setName('default', results[most_granular_layer][0].name); + try { + const doc = new Document('whosonfirst', most_granular_layer, id.toString()); + doc.setName('default', results[most_granular_layer][0].name); - // assign the administrative hierarchy - _.keys(results).forEach((layer) => { - doc.addParent(layer, results[layer][0].name, results[layer][0].id.toString(), results[layer][0].abbr); - }); + // assign the administrative hierarchy + _.keys(results).forEach((layer) => { + doc.addParent(layer, results[layer][0].name, results[layer][0].id.toString(), results[layer][0].abbr); + }); - // set centroid if available - if (_.has(results[most_granular_layer][0], 'centroid')) { - doc.setCentroid( results[most_granular_layer][0].centroid ); - } + // set centroid if available + if (_.has(results[most_granular_layer][0], 'centroid')) { + doc.setCentroid( results[most_granular_layer][0].centroid ); + } - // set bounding box if available - if (_.has(results[most_granular_layer][0], 'bounding_box')) { - const parsed_bounding_box = results[most_granular_layer][0].bounding_box.split(',').map(parseFloat); - doc.setBoundingBox({ - upperLeft: { - lat: parsed_bounding_box[3], - lon: parsed_bounding_box[0] - }, - lowerRight: { - lat: parsed_bounding_box[1], - lon: parsed_bounding_box[2] - } - }); + // set bounding box if available + if (_.has(results[most_granular_layer][0], 'bounding_box')) { + const parsed_bounding_box = results[most_granular_layer][0].bounding_box.split(',').map(parseFloat); + doc.setBoundingBox({ + upperLeft: { + lat: parsed_bounding_box[3], + lon: parsed_bounding_box[0] + }, + lowerRight: { + lat: parsed_bounding_box[1], + lon: parsed_bounding_box[2] + } + }); - } + } + + const esDoc = doc.toESDocument(); + esDoc.data._id = esDoc._id; + esDoc.data._type = esDoc._type; + return esDoc.data; - const esDoc = doc.toESDocument(); - esDoc.data._id = esDoc._id; - esDoc.data._type = esDoc._type; - return esDoc.data; + } catch( e ) { + // an error occurred when generating a new Document + logger.info(`[controller:coarse_reverse][error]`); + logger.error(e); + logger.error(results); + + return null; + } } function setup(service, should_execute) { @@ -141,7 +151,10 @@ function setup(service, should_execute) { // if there's a result at the requested layer(s), synthesize a doc from results if (hasResultsAtRequestedLayers(applicable_results, effective_layers)) { - res.data.push(synthesizeDoc(applicable_results)); + const doc = synthesizeDoc(applicable_results); + if (doc){ + res.data.push(doc); + } } debugLog.stopTimer(req, initialTime); return next(); diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js index fa42ef0a..cc7c5443 100644 --- a/test/unit/controller/coarse_reverse.js +++ b/test/unit/controller/coarse_reverse.js @@ -860,6 +860,60 @@ module.exports.tests.failure_conditions = (test, common) => { }); + test('service returns 0 length name', (t) => { + t.plan(5); + + const service = (req, callback) => { + t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } ); + + const results = { + neighbourhood: [ + { id: 20, name: '' } + ] + }; + + callback(undefined, results); + }; + + const logger = require('pelias-mock-logger')(); + + const controller = proxyquire('../../../controller/coarse_reverse', { + 'pelias-logger': logger + })(service, _.constant(true)); + + const req = { + clean: { + layers: ['neighbourhood'] + } + }; + + const res = { }; + + // verify that next was called + const next = () => { + t.pass('next() was called'); + }; + + controller(req, res, next); + + const expected = { + meta: {}, + data: [] + }; + + t.deepEquals(res, expected); + t.deepEquals(logger.getMessages('info'), [ + '[controller:coarse_reverse][queryType:pip][result_count:1]', + '[controller:coarse_reverse][error]' + ]); + t.deepEquals(logger.getMessages('error'), [ + '{ [PeliasModelError: invalid document type, expecting: truthy, ' + + 'got: ]\n name: \'PeliasModelError\',\n message: \'invalid document type, expecting: truthy, got: \' }', + '{ neighbourhood: [ { id: 20, name: \'\' } ] }' + ]); + t.end(); + + }); }; module.exports.all = (tape, common) => { From 45f313d8d90553f1a92f92c4517eb2f1a21eaed1 Mon Sep 17 00:00:00 2001 From: missinglink Date: Mon, 25 Sep 2017 18:05:25 +0200 Subject: [PATCH 13/42] feat(coarse_reverse): improve logger assertions --- controller/coarse_reverse.js | 2 +- test/unit/controller/coarse_reverse.js | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js index 1ed47c58..cae79a78 100644 --- a/controller/coarse_reverse.js +++ b/controller/coarse_reverse.js @@ -102,7 +102,7 @@ function synthesizeDoc(results) { // an error occurred when generating a new Document logger.info(`[controller:coarse_reverse][error]`); logger.error(e); - logger.error(results); + logger.info(results); return null; } diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js index cc7c5443..d4e357cb 100644 --- a/test/unit/controller/coarse_reverse.js +++ b/test/unit/controller/coarse_reverse.js @@ -861,7 +861,7 @@ module.exports.tests.failure_conditions = (test, common) => { }); test('service returns 0 length name', (t) => { - t.plan(5); + t.plan(6); const service = (req, callback) => { t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } ); @@ -902,15 +902,12 @@ module.exports.tests.failure_conditions = (test, common) => { }; t.deepEquals(res, expected); - t.deepEquals(logger.getMessages('info'), [ - '[controller:coarse_reverse][queryType:pip][result_count:1]', - '[controller:coarse_reverse][error]' - ]); - t.deepEquals(logger.getMessages('error'), [ - '{ [PeliasModelError: invalid document type, expecting: truthy, ' + - 'got: ]\n name: \'PeliasModelError\',\n message: \'invalid document type, expecting: truthy, got: \' }', - '{ neighbourhood: [ { id: 20, name: \'\' } ] }' - ]); + + // logger messages + t.true(logger.hasMessages('info'), '[controller:coarse_reverse][error]'); + t.true(logger.hasMessages('error'), 'invalid document type, expecting: truthy, got: '); + t.true(logger.hasMessages('info'), '{ neighbourhood: [ { id: 20, name: \'\' } ] }'); + t.end(); }); From b60e0113b376782ba901197f11cdfcc10387923f Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Mon, 25 Sep 2017 12:44:00 -0400 Subject: [PATCH 14/42] check for missing parent object in confidence score computation --- middleware/confidenceScore.js | 7 +++--- test/unit/middleware/confidenceScore.js | 32 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/middleware/confidenceScore.js b/middleware/confidenceScore.js index 8f5b61fb..5f82ddf6 100644 --- a/middleware/confidenceScore.js +++ b/middleware/confidenceScore.js @@ -94,7 +94,8 @@ function checkForDealBreakers(req, hit) { return false; } - if (check.assigned(req.clean.parsed_text.state) && hit.parent.region_a && req.clean.parsed_text.state !== hit.parent.region_a[0]) { + if (check.assigned(req.clean.parsed_text.state) && check.assigned(hit.parent) && + hit.parent.region_a && req.clean.parsed_text.state !== hit.parent.region_a[0]) { logger.debug('[confidence][deal-breaker]: state !== region_a'); return true; } @@ -220,8 +221,8 @@ function checkAddress(text, hit) { res += propMatch(text.number, (hit.address_parts ? hit.address_parts.number : null), false); res += propMatch(text.street, (hit.address_parts ? hit.address_parts.street : null), false); res += propMatch(text.postalcode, (hit.address_parts ? hit.address_parts.zip: null), true); - res += propMatch(text.state, (hit.parent.region_a ? hit.parent.region_a[0] : null), true); - res += propMatch(text.country, (hit.parent.country_a ? hit.parent.country_a[0] :null), true); + res += propMatch(text.state, ((hit.parent && hit.parent.region_a) ? hit.parent.region_a[0] : null), true); + res += propMatch(text.country, ((hit.parent && hit.parent.country_a) ? hit.parent.country_a[0] :null), true); res /= checkCount; } diff --git a/test/unit/middleware/confidenceScore.js b/test/unit/middleware/confidenceScore.js index a0b4de6f..9e10775f 100644 --- a/test/unit/middleware/confidenceScore.js +++ b/test/unit/middleware/confidenceScore.js @@ -169,6 +169,38 @@ module.exports.tests.confidenceScore = function(test, common) { t.false(res.data[0].hasOwnProperty('confidence'), 'score was not set'); t.end(); }); + + test('missing parent object should not throw an exception', function(t) { + var req = { + clean: { + text: '123 Main St, City, NM', + parsed_text: { + number: 123, + street: 'Main St', + state: 'NM' + } + } + }; + var res = { + data: [{ + _score: 10, + found: true, + value: 1, + center_point: { lat: 100.1, lon: -50.5 }, + name: { default: 'test name1' }, + }], + meta: { + scores: [10], + query_type: 'original' + } + }; + + t.doesNotThrow(() => { + confidenceScore(req, res, () => {}); + }); + t.equal(res.data[0].confidence, 0.28, 'score was set'); + t.end(); + }); }; module.exports.all = function (tape, common) { From 5d1d827bad5c3a883be922155b71fed088c92500 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Wed, 27 Sep 2017 14:37:14 +0000 Subject: [PATCH 15/42] chore(package): update source-map to version 0.6.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a73785a4..213948d1 100644 --- a/package.json +++ b/package.json @@ -79,7 +79,7 @@ "precommit-hook": "^3.0.0", "proxyquire": "^1.7.10", "semantic-release": "^8.0.0", - "source-map": "^0.5.6", + "source-map": "^0.6.0", "tap-dot": "1.0.5", "tape": "^4.5.1", "tmp": "0.0.33", From 69a820fae0a9ae3eb02bce431f44d7ffcb7f503a Mon Sep 17 00:00:00 2001 From: missinglink Date: Thu, 28 Sep 2017 14:30:37 +0200 Subject: [PATCH 16/42] feat(coarse_reverse): ignore empty abbr properties from PIP service --- controller/coarse_reverse.js | 2 +- test/unit/controller/coarse_reverse.js | 60 ++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js index cae79a78..2c7b370b 100644 --- a/controller/coarse_reverse.js +++ b/controller/coarse_reverse.js @@ -68,7 +68,7 @@ function synthesizeDoc(results) { // assign the administrative hierarchy _.keys(results).forEach((layer) => { - doc.addParent(layer, results[layer][0].name, results[layer][0].id.toString(), results[layer][0].abbr); + doc.addParent(layer, results[layer][0].name, results[layer][0].id.toString(), results[layer][0].abbr || undefined); }); // set centroid if available diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js index d4e357cb..f9e3709f 100644 --- a/test/unit/controller/coarse_reverse.js +++ b/test/unit/controller/coarse_reverse.js @@ -911,6 +911,66 @@ module.exports.tests.failure_conditions = (test, common) => { t.end(); }); + + test('service returns 0 length abbr', (t) => { + t.plan(4); + + const service = (req, callback) => { + t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } ); + + const results = { + neighbourhood: [ + { id: 20, name: 'Example', abbr: '' } + ] + }; + + callback(undefined, results); + }; + + const logger = require('pelias-mock-logger')(); + + const controller = proxyquire('../../../controller/coarse_reverse', { + 'pelias-logger': logger + })(service, _.constant(true)); + + const req = { + clean: { + layers: ['neighbourhood'] + } + }; + + const res = { }; + + // verify that next was called + const next = () => { + t.pass('next() was called'); + }; + + controller(req, res, next); + + const expected = { + meta: {}, + data: [{ + name: { default: 'Example' }, + phrase: { default: 'Example' }, + parent: { + neighbourhood: [ 'Example' ], + neighbourhood_id: [ '20' ], + neighbourhood_a: [ null ] + }, + source: 'whosonfirst', + layer: 'neighbourhood', + source_id: '20', + _id: '20', + _type: 'neighbourhood' + }] + }; + + t.deepEquals(res, expected); + t.notOk(logger.hasErrorMessages()); + t.end(); + + }); }; module.exports.all = (tape, common) => { From e70a668056e3ed85266784f4e400c59aacad5eaa Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Fri, 8 Sep 2017 16:15:45 -0400 Subject: [PATCH 17/42] added support for continent, ocean, and marinearea placetypes `coarse` now maps to these as well --- controller/coarse_reverse.js | 5 ++- helper/type_mapping.js | 6 ++-- test/unit/controller/coarse_reverse.js | 35 ++++++++++++++++++- .../predicates/is_coarse_reverse.js | 4 ++- test/unit/helper/type_mapping.js | 3 +- test/unit/sanitizer/_layers.js | 14 ++++---- 6 files changed, 55 insertions(+), 12 deletions(-) diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js index 2c7b370b..ce3ad52a 100644 --- a/controller/coarse_reverse.js +++ b/controller/coarse_reverse.js @@ -15,7 +15,10 @@ const coarse_granularities = [ 'region', 'macroregion', 'dependency', - 'country' + 'country', + 'continent', + 'ocean', + 'marinearea' ]; // remove non-coarse layers and return what's left (or all if empty) diff --git a/helper/type_mapping.js b/helper/type_mapping.js index 7b9de5a0..d17fd058 100644 --- a/helper/type_mapping.js +++ b/helper/type_mapping.js @@ -51,7 +51,8 @@ var LAYERS_BY_SOURCE = { 'locality','borough', 'neighbourhood', 'venue' ], whosonfirst: [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'borough', - 'neighbourhood', 'microhood', 'disputed', 'venue', 'postalcode'] + 'neighbourhood', 'microhood', 'disputed', 'venue', 'postalcode', + 'continent', 'ocean', 'marinearea'] }; /* @@ -62,7 +63,8 @@ var LAYERS_BY_SOURCE = { var LAYER_ALIASES = { 'coarse': [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'borough', - 'neighbourhood', 'microhood', 'disputed', 'postalcode' ] + 'neighbourhood', 'microhood', 'disputed', 'postalcode', + 'continent', 'ocean', 'marinearea'] }; // create a list of all layers by combining each entry from LAYERS_BY_SOURCE diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js index f9e3709f..403e5875 100644 --- a/test/unit/controller/coarse_reverse.js +++ b/test/unit/controller/coarse_reverse.js @@ -210,6 +210,18 @@ module.exports.tests.success_conditions = (test, common) => { country: [ { id: 100, name: 'country name', abbr: 'xyz'}, { id: 101, name: 'country name 2'} + ], + continent: [ + { id: 110, name: 'continent name', abbr: 'continent abbr'}, + { id: 111, name: 'continent name 2'} + ], + ocean: [ + { id: 120, name: 'ocean name', abbr: 'ocean abbr'}, + { id: 121, name: 'ocean name 2'} + ], + marinearea: [ + { id: 130, name: 'marinearea name', abbr: 'marinearea abbr'}, + { id: 131, name: 'marinearea name 2'} ] }; @@ -282,7 +294,16 @@ module.exports.tests.success_conditions = (test, common) => { dependency_a: ['dependency abbr'], country: ['country name'], country_id: ['100'], - country_a: ['xyz'] + country_a: ['xyz'], + continent: ['continent name'], + continent_id: ['110'], + continent_a: ['continent abbr'], + ocean: ['ocean name'], + ocean_id: ['120'], + ocean_a: ['ocean abbr'], + marinearea: ['marinearea name'], + marinearea_id: ['130'], + marinearea_a: ['marinearea abbr'], }, center_point: { lat: 12.121212, @@ -822,6 +843,18 @@ module.exports.tests.failure_conditions = (test, common) => { country: [ { id: 100, name: 'country name', abbr: 'xyz'}, { id: 101, name: 'country name 2'} + ], + continent: [ + { id: 110, name: 'continent name', abbr: 'continent abbr'}, + { id: 111, name: 'continent name 2'} + ], + ocean: [ + { id: 120, name: 'ocean name', abbr: 'ocean abbr'}, + { id: 121, name: 'ocean name 2'} + ], + marinearea: [ + { id: 130, name: 'marinearea name', abbr: 'marinearea abbr'}, + { id: 131, name: 'marinearea name 2'} ] }; diff --git a/test/unit/controller/predicates/is_coarse_reverse.js b/test/unit/controller/predicates/is_coarse_reverse.js index 2a38e401..33b0611d 100644 --- a/test/unit/controller/predicates/is_coarse_reverse.js +++ b/test/unit/controller/predicates/is_coarse_reverse.js @@ -17,7 +17,9 @@ const coarse_layers = [ 'borough', 'neighbourhood', 'microhood', - 'disputed' + 'disputed', + 'ocean', + 'marinearea' ]; module.exports.tests = {}; diff --git a/test/unit/helper/type_mapping.js b/test/unit/helper/type_mapping.js index d67218c5..2ec86e40 100644 --- a/test/unit/helper/type_mapping.js +++ b/test/unit/helper/type_mapping.js @@ -14,7 +14,8 @@ module.exports.tests.interfaces = function(test, common) { t.deepEquals(type_mapping.layer_mapping.coarse, [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', - 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode' ]); + 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode', + 'continent', 'ocean', 'marinearea']); t.end(); }); diff --git a/test/unit/sanitizer/_layers.js b/test/unit/sanitizer/_layers.js index 11c618c8..adcffcc2 100644 --- a/test/unit/sanitizer/_layers.js +++ b/test/unit/sanitizer/_layers.js @@ -34,16 +34,17 @@ module.exports.tests.sanitize_layers = function(test, common) { t.deepEqual(clean.layers, venue_layers, 'venue layers set'); t.end(); }); - + test('coarse (alias) layer', function(t) { var raw = { layers: 'coarse' }; var clean = {}; sanitizer.sanitize(raw, clean); - var admin_layers = [ 'continent', 'country', 'dependency', - 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', - 'macrohood', 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode' ]; + var admin_layers = [ 'continent', 'country', 'dependency', 'macroregion', + 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', + 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode', 'ocean', + 'marinearea' ]; t.deepEqual(clean.layers, admin_layers, 'coarse layers set'); t.end(); @@ -78,7 +79,8 @@ module.exports.tests.sanitize_layers = function(test, common) { var expected_layers = [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', - 'macrohood', 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode' ]; + 'macrohood', 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode', + 'ocean', 'marinearea']; t.deepEqual(clean.layers, expected_layers, 'coarse + regular layers set'); t.end(); @@ -115,7 +117,7 @@ module.exports.tests.sanitize_layers = function(test, common) { var coarse_layers = [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'borough', 'neighbourhood', 'microhood', - 'disputed', 'postalcode' ]; + 'disputed', 'postalcode', 'ocean', 'marinearea' ]; var venue_layers = [ 'venue' ]; var expected_layers = venue_layers.concat(coarse_layers); From 414043ba308e6dee92c7a8e50a33bc0ad9653824 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 11:42:30 -0400 Subject: [PATCH 18/42] removed unused dependencies --- helper/geojsonify.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 01acb886..63dffa6f 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -2,8 +2,6 @@ var GeoJSON = require('geojson'); var extent = require('@mapbox/geojson-extent'); var logger = require('pelias-logger').get('api'); -var type_mapping = require('./type_mapping'); -var _ = require('lodash'); var addDetails = require('./geojsonify_place_details'); var addMetaData = require('./geojsonify_meta_data'); From cc0200dcc914dcd98a8bed9baa83b0fbed7df8db Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 12:07:57 -0400 Subject: [PATCH 19/42] added prune target to pre-commit to avoid npm weirdness --- package.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index a73785a4..cf4b48cd 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,8 @@ "validate": "npm ls", "semantic-release": "semantic-release pre && npm publish && semantic-release post", "config": "node -e \"console.log(JSON.stringify(require( 'pelias-config' ).generate(require('./schema')), null, 2))\"", - "check-dependencies": "node_modules/.bin/npm-check --production --ignore pelias-interpolation" + "check-dependencies": "node_modules/.bin/npm-check --production --ignore pelias-interpolation", + "prune": "npm prune" }, "repository": { "type": "git", @@ -87,6 +88,7 @@ }, "pre-commit": [ "lint", + "prune", "validate", "test", "check-dependencies" From 27194a620e9b85408275b29e9957a1e861f25777 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 15:21:42 -0400 Subject: [PATCH 20/42] added tests for geocode_place_details --- test/unit/helper/geojsonify_place_details.js | 537 +++++++++++++++++++ test/unit/run.js | 1 + 2 files changed, 538 insertions(+) create mode 100644 test/unit/helper/geojsonify_place_details.js diff --git a/test/unit/helper/geojsonify_place_details.js b/test/unit/helper/geojsonify_place_details.js new file mode 100644 index 00000000..3bc96adc --- /dev/null +++ b/test/unit/helper/geojsonify_place_details.js @@ -0,0 +1,537 @@ +const geojsonify = require('../../../helper/geojsonify_place_details'); + +module.exports.tests = {}; + +module.exports.tests.geojsonify_place_details = (test, common) => { + test('plain old string values should be copied verbatim, replacing old values', t => { + const source = { + unknown_field: 'original unknown_field value', + housenumber: 'housenumber value', + street: 'street value', + postalcode: 'postalcode value', + postalcode_gid: 'postalcode_gid value', + match_type: 'match_type value', + accuracy: 'accuracy value', + country: 'country value', + country_gid: 'country_gid value', + country_a: 'country_a value', + dependency: 'dependency value', + dependency_gid: 'dependency_gid value', + dependency_a: 'dependency_a value', + macroregion: 'macroregion value', + macroregion_gid: 'macroregion_gid value', + macroregion_a: 'macroregion_a value', + region: 'region value', + region_gid: 'region_gid value', + region_a: 'region_a value', + macrocounty: 'macrocounty value', + macrocounty_gid: 'macrocounty_gid value', + macrocounty_a: 'macrocounty_a value', + county: 'county value', + county_gid: 'county_gid value', + county_a: 'county_a value', + localadmin: 'localadmin value', + localadmin_gid: 'localadmin_gid value', + localadmin_a: 'localadmin_a value', + locality: 'locality value', + locality_gid: 'locality_gid value', + locality_a: 'locality_a value', + borough: 'borough value', + borough_gid: 'borough_gid value', + borough_a: 'borough_a value', + neighbourhood: 'neighbourhood value', + neighbourhood_gid: 'neighbourhood_gid value', + label: 'label value' + }; + const destination = { + unknown_field: 'original unknown_field value', + housenumber: 'original housenumber value', + street: 'original street value', + postalcode: 'original postalcode value', + postalcode_gid: 'original postalcode_gid value', + match_type: 'original match_type value', + accuracy: 'original accuracy value', + country: 'original country value', + country_gid: 'original country_gid value', + country_a: 'original country_a value', + dependency: 'original dependency value', + dependency_gid: 'original dependency_gid value', + dependency_a: 'original dependency_a value', + macroregion: 'original macroregion value', + macroregion_gid: 'original macroregion_gid value', + macroregion_a: 'original macroregion_a value', + region: 'original region value', + region_gid: 'original region_gid value', + region_a: 'original region_a value', + macrocounty: 'original macrocounty value', + macrocounty_gid: 'original macrocounty_gid value', + macrocounty_a: 'original macrocounty_a value', + county: 'original county value', + county_gid: 'original county_gid value', + county_a: 'original county_a value', + localadmin: 'original localadmin value', + localadmin_gid: 'original localadmin_gid value', + localadmin_a: 'original localadmin_a value', + locality: 'original locality value', + locality_gid: 'original locality_gid value', + locality_a: 'original locality_a value', + borough: 'original borough value', + borough_gid: 'original borough_gid value', + borough_a: 'original borough_a value', + neighbourhood: 'original neighbourhood value', + neighbourhood_gid: 'original neighbourhood_gid value', + label: 'original label value' + }; + + const expected = { + unknown_field: 'original unknown_field value', + housenumber: 'housenumber value', + street: 'street value', + postalcode: 'postalcode value', + postalcode_gid: 'postalcode_gid value', + match_type: 'match_type value', + accuracy: 'accuracy value', + country: 'country value', + country_gid: 'country_gid value', + country_a: 'country_a value', + dependency: 'dependency value', + dependency_gid: 'dependency_gid value', + dependency_a: 'dependency_a value', + macroregion: 'macroregion value', + macroregion_gid: 'macroregion_gid value', + macroregion_a: 'macroregion_a value', + region: 'region value', + region_gid: 'region_gid value', + region_a: 'region_a value', + macrocounty: 'macrocounty value', + macrocounty_gid: 'macrocounty_gid value', + macrocounty_a: 'macrocounty_a value', + county: 'county value', + county_gid: 'county_gid value', + county_a: 'county_a value', + localadmin: 'localadmin value', + localadmin_gid: 'localadmin_gid value', + localadmin_a: 'localadmin_a value', + locality: 'locality value', + locality_gid: 'locality_gid value', + locality_a: 'locality_a value', + borough: 'borough value', + borough_gid: 'borough_gid value', + borough_a: 'borough_a value', + neighbourhood: 'neighbourhood value', + neighbourhood_gid: 'neighbourhood_gid value', + label: 'label value' + }; + + geojsonify({}, source, destination); + + t.deepEqual(destination, expected); + t.end(); + + }); + + test('\'empty\' string-type values should be output as \'\'', t => { + [ [], {}, '', 17, true, null, undefined ].forEach(empty_value => { + const source = { + housenumber: empty_value, + street: empty_value, + postalcode: empty_value, + postalcode_gid: empty_value, + match_type: empty_value, + accuracy: empty_value, + country: empty_value, + country_gid: empty_value, + country_a: empty_value, + dependency: empty_value, + dependency_gid: empty_value, + dependency_a: empty_value, + macroregion: empty_value, + macroregion_gid: empty_value, + macroregion_a: empty_value, + region: empty_value, + region_gid: empty_value, + region_a: empty_value, + macrocounty: empty_value, + macrocounty_gid: empty_value, + macrocounty_a: empty_value, + county: empty_value, + county_gid: empty_value, + county_a: empty_value, + localadmin: empty_value, + localadmin_gid: empty_value, + localadmin_a: empty_value, + locality: empty_value, + locality_gid: empty_value, + locality_a: empty_value, + borough: empty_value, + borough_gid: empty_value, + borough_a: empty_value, + neighbourhood: empty_value, + neighbourhood_gid: empty_value, + label: empty_value + }; + const destination = {}; + const expected = {}; + + geojsonify({}, source, destination); + + t.deepEqual(destination, expected); + + }); + + t.end(); + + }); + + test('source arrays should be copy first value', t => { + const source = { + housenumber: ['housenumber value 1', 'housenumber value 2'], + street: ['street value 1', 'street value 2'], + postalcode: ['postalcode value 1', 'postalcode value 2'], + postalcode_gid: ['postalcode_gid value 1', 'postalcode_gid value 2'], + match_type: ['match_type value 1', 'match_type value 2'], + accuracy: ['accuracy value 1', 'accuracy value 2'], + country: ['country value 1', 'country value 2'], + country_gid: ['country_gid value 1', 'country_gid value 2'], + country_a: ['country_a value 1', 'country_a value 2'], + dependency: ['dependency value 1', 'dependency value 2'], + dependency_gid: ['dependency_gid value 1', 'dependency_gid value 2'], + dependency_a: ['dependency_a value 1', 'dependency_a value 2'], + macroregion: ['macroregion value 1', 'macroregion value 2'], + macroregion_gid: ['macroregion_gid value 1', 'macroregion_gid value 2'], + macroregion_a: ['macroregion_a value 1', 'macroregion_a value 2'], + region: ['region value 1', 'region value 2'], + region_gid: ['region_gid value 1', 'region_gid value 2'], + region_a: ['region_a value 1', 'region_a value 2'], + macrocounty: ['macrocounty value 1', 'macrocounty value 2'], + macrocounty_gid: ['macrocounty_gid value 1', 'macrocounty_gid value 2'], + macrocounty_a: ['macrocounty_a value 1', 'macrocounty_a value 2'], + county: ['county value 1', 'county value 2'], + county_gid: ['county_gid value 1', 'county_gid value 2'], + county_a: ['county_a value 1', 'county_a value 2'], + localadmin: ['localadmin value 1', 'localadmin value 2'], + localadmin_gid: ['localadmin_gid value 1', 'localadmin_gid value 2'], + localadmin_a: ['localadmin_a value 1', 'localadmin_a value 2'], + locality: ['locality value 1', 'locality value 2'], + locality_gid: ['locality_gid value 1', 'locality_gid value 2'], + locality_a: ['locality_a value 1', 'locality_a value 2'], + borough: ['borough value 1', 'borough value 2'], + borough_gid: ['borough_gid value 1', 'borough_gid value 2'], + borough_a: ['borough_a value 1', 'borough_a value 2'], + neighbourhood: ['neighbourhood value 1', 'neighbourhood value 2'], + neighbourhood_gid: ['neighbourhood_gid value 1', 'neighbourhood_gid value 2'], + label: ['label value 1', 'label value 2'] + }; + const destination = { }; + + const expected = { + housenumber: 'housenumber value 1', + street: 'street value 1', + postalcode: 'postalcode value 1', + postalcode_gid: 'postalcode_gid value 1', + match_type: 'match_type value 1', + accuracy: 'accuracy value 1', + country: 'country value 1', + country_gid: 'country_gid value 1', + country_a: 'country_a value 1', + dependency: 'dependency value 1', + dependency_gid: 'dependency_gid value 1', + dependency_a: 'dependency_a value 1', + macroregion: 'macroregion value 1', + macroregion_gid: 'macroregion_gid value 1', + macroregion_a: 'macroregion_a value 1', + region: 'region value 1', + region_gid: 'region_gid value 1', + region_a: 'region_a value 1', + macrocounty: 'macrocounty value 1', + macrocounty_gid: 'macrocounty_gid value 1', + macrocounty_a: 'macrocounty_a value 1', + county: 'county value 1', + county_gid: 'county_gid value 1', + county_a: 'county_a value 1', + localadmin: 'localadmin value 1', + localadmin_gid: 'localadmin_gid value 1', + localadmin_a: 'localadmin_a value 1', + locality: 'locality value 1', + locality_gid: 'locality_gid value 1', + locality_a: 'locality_a value 1', + borough: 'borough value 1', + borough_gid: 'borough_gid value 1', + borough_a: 'borough_a value 1', + neighbourhood: 'neighbourhood value 1', + neighbourhood_gid: 'neighbourhood_gid value 1', + label: 'label value 1' + }; + + geojsonify({}, source, destination); + + t.deepEqual(destination, expected); + t.end(); + + }); + + test('non-empty objects should be converted to strings', t => { + // THIS TEST SHOWS THAT THE CODE DOES NOT DO WHAT IT EXPECTED + const source = { + housenumber: { housenumber: 'housenumber value'}, + street: { street: 'street value'}, + postalcode: { postalcode: 'postalcode value'}, + postalcode_gid: { postalcode_gid: 'postalcode_gid value'}, + match_type: { match_type: 'match_type value'}, + accuracy: { accuracy: 'accuracy value'}, + country: { country: 'country value'}, + country_gid: { country_gid: 'country_gid value'}, + country_a: { country_a: 'country_a value'}, + dependency: { dependency: 'dependency value'}, + dependency_gid: { dependency_gid: 'dependency_gid value'}, + dependency_a: { dependency_a: 'dependency_a value'}, + macroregion: { macroregion: 'macroregion value'}, + macroregion_gid: { macroregion_gid: 'macroregion_gid value'}, + macroregion_a: { macroregion_a: 'macroregion_a value'}, + region: { region: 'region value'}, + region_gid: { region_gid: 'region_gid value'}, + region_a: { region_a: 'region_a value'}, + macrocounty: { macrocounty: 'macrocounty value'}, + macrocounty_gid: { macrocounty_gid: 'macrocounty_gid value'}, + macrocounty_a: { macrocounty_a: 'macrocounty_a value'}, + county: { county: 'county value'}, + county_gid: { county_gid: 'county_gid value'}, + county_a: { county_a: 'county_a value'}, + localadmin: { localadmin: 'localadmin value'}, + localadmin_gid: { localadmin_gid: 'localadmin_gid value'}, + localadmin_a: { localadmin_a: 'localadmin_a value'}, + locality: { locality: 'locality value'}, + locality_gid: { locality_gid: 'locality_gid value'}, + locality_a: { locality_a: 'locality_a value'}, + borough: { borough: 'borough value'}, + borough_gid: { borough_gid: 'borough_gid value'}, + borough_a: { borough_a: 'borough_a value'}, + neighbourhood: { neighbourhood: 'neighbourhood value'}, + neighbourhood_gid: { neighbourhood_gid: 'neighbourhood_gid value'}, + label: { label: 'label value'} + }; + const destination = { }; + + const expected = { + housenumber: '[object Object]', + street: '[object Object]', + postalcode: '[object Object]', + postalcode_gid: '[object Object]', + match_type: '[object Object]', + accuracy: '[object Object]', + country: '[object Object]', + country_gid: '[object Object]', + country_a: '[object Object]', + dependency: '[object Object]', + dependency_gid: '[object Object]', + dependency_a: '[object Object]', + macroregion: '[object Object]', + macroregion_gid: '[object Object]', + macroregion_a: '[object Object]', + region: '[object Object]', + region_gid: '[object Object]', + region_a: '[object Object]', + macrocounty: '[object Object]', + macrocounty_gid: '[object Object]', + macrocounty_a: '[object Object]', + county: '[object Object]', + county_gid: '[object Object]', + county_a: '[object Object]', + localadmin: '[object Object]', + localadmin_gid: '[object Object]', + localadmin_a: '[object Object]', + locality: '[object Object]', + locality_gid: '[object Object]', + locality_a: '[object Object]', + borough: '[object Object]', + borough_gid: '[object Object]', + borough_a: '[object Object]', + neighbourhood: '[object Object]', + neighbourhood_gid: '[object Object]', + label: '[object Object]' + }; + + geojsonify({}, source, destination); + + t.deepEqual(destination, expected); + t.end(); + + }); + + test('\'default\'-type properties should be copied without type conversion and overwrite old values', t => { + [ 'this is a string', 17.3, { a: 1 }, [1, 2, 3] ].forEach(value => { + const source = { + confidence: value, + distance: value, + bounding_box: value + }; + const destination = { + confidence: 'original confidence value', + distance: 'original distance value', + bounding_box: 'original bounding_box value' + }; + + const expected = { + confidence: value, + distance: value, + bounding_box: value + }; + + geojsonify({}, source, destination); + + t.deepEqual(destination, expected); + + }); + + t.end(); + + }); + + test('\'default\'-type properties that are numbers should be output as numbers', t => { + [ 17, 17.3 ].forEach(value => { + const source = { + confidence: value, + distance: value, + bounding_box: value + }; + const destination = {}; + const expected = { + confidence: value, + distance: value, + bounding_box: value + }; + + geojsonify({}, source, destination); + + t.deepEqual(destination, expected); + + }); + + t.end(); + + }); + + test('\'empty\' values for default-type properties should not be output', t => { + [ undefined, null, true, {}, [] ].forEach(value => { + const source = { + confidence: value, + distance: value, + bounding_box: value + }; + const destination = {}; + const expected = {}; + + geojsonify({}, source, destination); + + t.deepEqual(destination, expected); + + }); + + t.end(); + + }); + + test('array-type properties should not be output when empty', t => { + const source = { + category: [] + }; + const destination = {}; + const expected = {}; + + geojsonify({}, source, destination); + + t.deepEqual(destination, expected); + t.end(); + + }); + + test('array-type properties with array values should be output as arrays', t => { + const source = { + category: [ 1, 2 ] + }; + const destination = {}; + const expected = { + category: [ 1, 2 ] + }; + + const clean = { + categories: true + }; + + geojsonify(clean, source, destination); + + t.deepEqual(destination, expected); + t.end(); + + }); + + test('category property should be output when params contains \'category\' property', t => { + [ {a: 1}, 'this is a string'].forEach(value => { + const source = { + category: value + }; + const destination = {}; + const expected = { + category: [ value ] + }; + + const clean = { + categories: true + }; + + geojsonify(clean, source, destination); + + t.deepEqual(destination, expected); + + }); + + t.end(); + + }); + + test('category property should not be output when params does not contain \'category\' property', t => { + const source = { + category: [ 1, 2 ] + }; + const destination = {}; + const expected = { + }; + + const clean = {}; + + geojsonify(clean, source, destination); + + t.deepEqual(destination, expected); + t.end(); + + }); + + test('category property should not be output when params is not an object', t => { + const source = { + category: [ 1, 2 ] + }; + const destination = {}; + const expected = { + }; + + const clean = 'this is not an object'; + + geojsonify(clean, source, destination); + + t.deepEqual(destination, expected); + t.end(); + + }); + +}; + +module.exports.all = (tape, common) => { + + function test(name, testFunction) { + return tape(`geojsonify: ${name}`, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/run.js b/test/unit/run.js index 0cd9d4c9..5631b9f3 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -31,6 +31,7 @@ var tests = [ require('./controller/predicates/is_request_sources_only_whosonfirst'), require('./helper/debug'), require('./helper/diffPlaces'), + require('./helper/geojsonify_place_details'), require('./helper/geojsonify'), require('./helper/logging'), require('./helper/type_mapping'), From 0548192b6e84bd494e01a772d98bec031ebe2470 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 15:27:00 -0400 Subject: [PATCH 21/42] removed level of indirection --- helper/geojsonify_place_details.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/helper/geojsonify_place_details.js b/helper/geojsonify_place_details.js index d9a000ee..dfe26963 100644 --- a/helper/geojsonify_place_details.js +++ b/helper/geojsonify_place_details.js @@ -57,9 +57,9 @@ function checkCategoryParam(params) { * @param {object} src * @param {object} dst */ -function addDetails(params, src, dst) { - copyProperties(params, src, DETAILS_PROPS, dst); -} +// function addDetails(params, src, dst) { +// copyProperties(params, src, DETAILS_PROPS, dst); +// } /** * Copy specified properties from source to dest. @@ -70,8 +70,8 @@ function addDetails(params, src, dst) { * @param {[]} props * @param {object} dst */ -function copyProperties( params, source, props, dst ) { - props.forEach( function ( prop ) { +function copyProperties( params, source, dst ) { + DETAILS_PROPS.forEach( function ( prop ) { // if condition isn't met, just return without setting the property if (_.isFunction(prop.condition) && !prop.condition(params)) { @@ -137,4 +137,4 @@ function getArrayValue(property) { return [property]; } -module.exports = addDetails; +module.exports = copyProperties; From 5ac373c0ca3999f21d828a2f46bd9b5ca150c11f Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 15:28:58 -0400 Subject: [PATCH 22/42] removed outdated comments --- helper/geojsonify_place_details.js | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/helper/geojsonify_place_details.js b/helper/geojsonify_place_details.js index dfe26963..d9d3ade2 100644 --- a/helper/geojsonify_place_details.js +++ b/helper/geojsonify_place_details.js @@ -50,24 +50,12 @@ function checkCategoryParam(params) { return _.isObject(params) && params.hasOwnProperty('categories'); } -/** - * Add details properties - * - * @param {object} params clean query params - * @param {object} src - * @param {object} dst - */ -// function addDetails(params, src, dst) { -// copyProperties(params, src, DETAILS_PROPS, dst); -// } - /** * Copy specified properties from source to dest. * Ignore missing properties. * * @param {object} params clean query params * @param {object} source - * @param {[]} props * @param {object} dst */ function copyProperties( params, source, dst ) { From 7eb2c68090a7e4f11fed7e88950a9e1d56d91a39 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 15:31:34 -0400 Subject: [PATCH 23/42] removed bookkeeping object, it was adding unneeded complexity --- helper/geojsonify_place_details.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/helper/geojsonify_place_details.js b/helper/geojsonify_place_details.js index d9d3ade2..bbe48d2c 100644 --- a/helper/geojsonify_place_details.js +++ b/helper/geojsonify_place_details.js @@ -66,28 +66,23 @@ function copyProperties( params, source, dst ) { return; } - var property = { - name: prop.name || prop, - type: prop.type || 'default' - }; - var value = null; - if ( source.hasOwnProperty( property.name ) ) { + if ( source.hasOwnProperty( prop.name ) ) { - switch (property.type) { + switch (_.defaultTo(prop.type, 'default')) { case 'string': - value = getStringValue(source[property.name]); + value = getStringValue(source[prop.name]); break; case 'array': - value = getArrayValue(source[property.name]); + value = getArrayValue(source[prop.name]); break; // default behavior is to copy property exactly as is default: - value = source[property.name]; + value = source[prop.name]; } if (_.isNumber(value) || (value && !_.isEmpty(value))) { - dst[property.name] = value; + dst[prop.name] = value; } } }); From 5721b591fba6063ef7d857dd7b7f1069061ac333 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 15:33:03 -0400 Subject: [PATCH 24/42] cleaned up scope --- helper/geojsonify_place_details.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helper/geojsonify_place_details.js b/helper/geojsonify_place_details.js index bbe48d2c..26059a1b 100644 --- a/helper/geojsonify_place_details.js +++ b/helper/geojsonify_place_details.js @@ -66,8 +66,8 @@ function copyProperties( params, source, dst ) { return; } - var value = null; if ( source.hasOwnProperty( prop.name ) ) { + var value = null; switch (_.defaultTo(prop.type, 'default')) { case 'string': From 6d9090fa8459166eb8488914583dbdf82976b839 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 15:33:43 -0400 Subject: [PATCH 25/42] removed redundant 'default' check --- helper/geojsonify_place_details.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helper/geojsonify_place_details.js b/helper/geojsonify_place_details.js index 26059a1b..abf00dbc 100644 --- a/helper/geojsonify_place_details.js +++ b/helper/geojsonify_place_details.js @@ -69,7 +69,7 @@ function copyProperties( params, source, dst ) { if ( source.hasOwnProperty( prop.name ) ) { var value = null; - switch (_.defaultTo(prop.type, 'default')) { + switch (prop.type) { case 'string': value = getStringValue(source[prop.name]); break; From 69c60dae9c35ff2ae53cbd9b63a7eeb68ff71e97 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 15:35:06 -0400 Subject: [PATCH 26/42] converted to const/let --- helper/geojsonify_place_details.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/helper/geojsonify_place_details.js b/helper/geojsonify_place_details.js index abf00dbc..33879c39 100644 --- a/helper/geojsonify_place_details.js +++ b/helper/geojsonify_place_details.js @@ -1,9 +1,11 @@ -var _ = require('lodash'); +'use strict'; + +const _ = require('lodash'); // Properties to be copied // If a property is identified as a single string, assume it should be presented as a string in response // If something other than string is desired, use the following structure: { name: 'category', type: 'array' } -var DETAILS_PROPS = [ +const DETAILS_PROPS = [ { name: 'housenumber', type: 'string' }, { name: 'street', type: 'string' }, { name: 'postalcode', type: 'string' }, @@ -67,7 +69,7 @@ function copyProperties( params, source, dst ) { } if ( source.hasOwnProperty( prop.name ) ) { - var value = null; + let value = null; switch (prop.type) { case 'string': From 5e1360f0225424a4a869434ff5ad9bfb4a496d0e Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 17:00:41 -0400 Subject: [PATCH 27/42] refactored geojsonify, improved test coverage --- helper/geojsonify.js | 63 +-- test/unit/helper/geojsonify.js | 867 +++++++++++++++++++-------------- 2 files changed, 527 insertions(+), 403 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 63dffa6f..79356516 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -1,26 +1,32 @@ -var GeoJSON = require('geojson'); -var extent = require('@mapbox/geojson-extent'); -var logger = require('pelias-logger').get('api'); -var addDetails = require('./geojsonify_place_details'); -var addMetaData = require('./geojsonify_meta_data'); +const GeoJSON = require('geojson'); +const extent = require('@mapbox/geojson-extent'); +const logger = require('pelias-logger').get('geojsonify'); +const addDetails = require('./geojsonify_place_details'); +const addMetaData = require('./geojsonify_meta_data'); +const _ = require('lodash'); function geojsonifyPlaces( params, docs ){ // flatten & expand data for geojson conversion - var geodata = docs - .map(geojsonifyPlace.bind(null, params)) - .filter( function( doc ){ - return !!doc; - }); + const geodata = docs + .filter(doc => { + if (!_.has(doc, 'center_point')) { + logger.warn('No doc or center_point property'); + return false; + } else { + return true; + } + }) + .map(geojsonifyPlace.bind(null, params)); // get all the bounding_box corners as well as single points // to be used for computing the overall bounding_box for the FeatureCollection - var extentPoints = extractExtentPoints(geodata); + const extentPoints = extractExtentPoints(geodata); // convert to geojson - var geojson = GeoJSON.parse( geodata, { Point: ['lat', 'lng'] }); - var geojsonExtentPoints = GeoJSON.parse( extentPoints, { Point: ['lat', 'lng'] }); + const geojson = GeoJSON.parse( geodata, { Point: ['lat', 'lng'] }); + const geojsonExtentPoints = GeoJSON.parse( extentPoints, { Point: ['lat', 'lng'] }); // to insert the bbox property at the top level of each feature, it must be done separately after // initial geojson construction is finished @@ -33,13 +39,7 @@ function geojsonifyPlaces( params, docs ){ } function geojsonifyPlace(params, place) { - - // something went very wrong - if( !place || !place.hasOwnProperty( 'center_point' ) ) { - return warning('No doc or center_point property'); - } - - var output = {}; + const output = {}; addMetaData(place, output); addName(place, output); @@ -60,9 +60,11 @@ function geojsonifyPlace(params, place) { * @param {object} dst */ function addName(src, dst) { - // map name - if( !src.name || !src.name.default ) { return warning(src); } - dst.name = src.name.default; + if (_.has(src, 'name.default')) { + dst.name = src.name.default; + } else { + logger.warn(`doc ${dst.gid} does not contain name.default`); + } } /** @@ -71,7 +73,7 @@ function addName(src, dst) { * @param {object} geojson */ function addBBoxPerFeature(geojson) { - geojson.features.forEach(function (feature) { + geojson.features.forEach(feature => { if (!feature.properties.hasOwnProperty('bounding_box')) { return; @@ -99,7 +101,7 @@ function addBBoxPerFeature(geojson) { * @returns {Array} */ function extractExtentPoints(geodata) { - var extentPoints = []; + const extentPoints = []; geodata.forEach(function (place) { if (place.bounding_box) { extentPoints.push({ @@ -142,15 +144,4 @@ function computeBBox(geojson, geojsonExtentPoints) { } } -/** - * emit a warning if the doc format is invalid - * - * @note: if you see this error, fix it ASAP! - */ -function warning( doc ) { - console.error( 'error: invalid doc', __filename, doc); - return false; // remove offending doc from results -} - - module.exports = geojsonifyPlaces; diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index 809b1ae9..08571aee 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -1,5 +1,6 @@ +const geojsonify = require('../../../helper/geojsonify'); -var geojsonify = require('../../../helper/geojsonify'); +const proxyquire = require('proxyquire').noCallThru(); module.exports.tests = {}; @@ -36,433 +37,565 @@ module.exports.tests.earth = function(test, common) { }; -module.exports.tests.geojsonify = function(test, common) { +module.exports.tests.all = (test, common) => { + test('bounding_box should be calculated using points when avaiable', t => { + const input = [ + { + _id: 'id 1', + source: 'source 1', + source_id: 'source_id 1', + layer: 'layer 1', + name: { + default: 'name 1', + }, + center_point: { + lat: 12.121212, + lon: 21.212121 + } + }, + { + _id: 'id 2', + source: 'source 2', + source_id: 'source_id 2', + layer: 'layer 2', + name: { + default: 'name 2', + }, + center_point: { + lat: 13.131313, + lon: 31.313131 + } + } + ]; - test('geojsonify(doc)', function(t) { - var input = [ + const geojsonify = proxyquire('../../../helper/geojsonify', { + './geojsonify_place_details': (params, source, dst) => { + if (source._id === 'id 1') { + dst.property1 = 'property 1'; + dst.property2 = 'property 2'; + } else if (source._id === 'id 2') { + dst.property3 = 'property 3'; + dst.property4 = 'property 4'; + } + + } + }); + + const actual = geojsonify({}, input); + + const expected = { + type: 'FeatureCollection', + features: [ + { + type: 'Feature', + geometry: { + type: 'Point', + coordinates: [ 21.212121, 12.121212 ] + }, + properties: { + id: 'id 1', + gid: 'source 1:layer 1:id 1', + layer: 'layer 1', + source: 'source 1', + source_id: 'source_id 1', + name: 'name 1', + property1: 'property 1', + property2: 'property 2' + } + }, + { + type: 'Feature', + geometry: { + type: 'Point', + coordinates: [ 31.313131, 13.131313 ] + }, + properties: { + id: 'id 2', + gid: 'source 2:layer 2:id 2', + layer: 'layer 2', + source: 'source 2', + source_id: 'source_id 2', + name: 'name 2', + property3: 'property 3', + property4: 'property 4' + } + } + ], + bbox: [21.212121, 12.121212, 31.313131, 13.131313] + }; + + t.deepEquals(actual, expected); + t.end(); + + }); + + test('bounding_box should be calculated using polygons when avaiable', t => { + const input = [ { - '_id': 'id1', - '_type': 'layer1', - 'source': 'source1', - 'source_id': 'source_id_1', - 'layer': 'layer1', - 'center_point': { - 'lat': 51.5337144, - 'lon': -0.1069716 + _id: 'id 1', + source: 'source 1', + source_id: 'source_id 1', + layer: 'layer 1', + name: { + default: 'name 1', }, - 'name': { - 'default': '\'Round Midnight Jazz and Blues Bar' + bounding_box: { + min_lon: 1, + min_lat: 1, + max_lon: 2, + max_lat: 2 }, - 'housenumber': '13', - 'street': 'Liverpool Road', - 'postalcode': 'N1 0RW', - 'country_a': 'GBR', - 'country': 'United Kingdom', - 'dependency': 'dependency name', - 'region': 'Islington', - 'region_a': 'ISL', - 'macroregion': 'England', - 'county': 'Angel', - 'localadmin': 'test1', - 'locality': 'test2', - 'neighbourhood': 'test3', - 'category': [ - 'food', - 'nightlife' - ], - 'label': 'label for id id1' + center_point: { + lat: 12.121212, + lon: 21.212121 + } }, { - '_id': 'id2', - '_type': 'layer2', - 'source': 'source2', - 'source_id': 'source_id_2', - 'layer': 'layer2', - 'name': { - 'default': 'Blues Cafe' + _id: 'id 2', + source: 'source 2', + source_id: 'source_id 2', + layer: 'layer 2', + name: { + default: 'name 2', + }, + bounding_box: { + min_lon: -3, + min_lat: -3, + max_lon: -1, + max_lat: -1 + }, + center_point: { + lat: 13.131313, + lon: 31.313131 + } + } + ]; + + const geojsonify = proxyquire('../../../helper/geojsonify', { + './geojsonify_place_details': (params, source, dst) => { + if (source._id === 'id 1') { + dst.property1 = 'property 1'; + dst.property2 = 'property 2'; + } else if (source._id === 'id 2') { + dst.property3 = 'property 3'; + dst.property4 = 'property 4'; + } + + } + }); + + const actual = geojsonify({}, input); + + const expected = { + type: 'FeatureCollection', + features: [ + { + type: 'Feature', + geometry: { + type: 'Point', + coordinates: [ 21.212121, 12.121212 ] + }, + properties: { + id: 'id 1', + gid: 'source 1:layer 1:id 1', + layer: 'layer 1', + source: 'source 1', + source_id: 'source_id 1', + name: 'name 1', + property1: 'property 1', + property2: 'property 2' + }, + bbox: [ 1, 1, 2, 2 ] }, - 'center_point': { - 'lat': '51.517806', - 'lon': '-0.101795' + { + type: 'Feature', + geometry: { + type: 'Point', + coordinates: [ 31.313131, 13.131313 ] + }, + properties: { + id: 'id 2', + gid: 'source 2:layer 2:id 2', + layer: 'layer 2', + source: 'source 2', + source_id: 'source_id 2', + name: 'name 2', + property3: 'property 3', + property4: 'property 4' + }, + bbox: [ -3, -3, -1, -1 ] + } + ], + bbox: [ -3, -3, 2, 2 ] + }; + + t.deepEquals(actual, expected); + t.end(); + + }); + + test('bounding_box should be calculated using polygons AND points when avaiable', t => { + const input = [ + { + _id: 'id 1', + source: 'source 1', + source_id: 'source_id 1', + layer: 'layer 1', + name: { + default: 'name 1', }, - 'country_a': 'GBR', - 'country': 'United Kingdom', - 'dependency': 'dependency name', - 'region': 'City And County Of The City Of London', - 'region_a': 'COL', - 'macroregion': 'England', - 'county': 'Smithfield', - 'localadmin': 'test1', - 'locality': 'test2', - 'neighbourhood': 'test3', - 'label': 'label for id id2' + center_point: { + lat: 12.121212, + lon: 21.212121 + } }, { - '_id': 'node:34633854', - '_type': 'venue', - 'source': 'openstreetmap', - 'source_id': 'source_id_3', - 'layer': 'venue', - 'name': { - 'default': 'Empire State Building' + _id: 'id 2', + source: 'source 2', + source_id: 'source_id 2', + layer: 'layer 2', + name: { + default: 'name 2', }, - 'center_point': { - 'lat': '40.748432', - 'lon': '-73.985656' + bounding_box: { + min_lon: -3, + min_lat: -3, + max_lon: -1, + max_lat: -1 }, - 'country_a': 'USA', - 'country': 'United States', - 'dependency': 'dependency name', - 'region': 'New York', - 'region_a': 'NY', - 'county': 'New York', - 'borough': 'Manhattan', - 'locality': 'New York', - 'neighbourhood': 'Koreatown', - 'category': [ - 'tourism', - 'transport' - ], - 'label': 'label for id node:34633854' + center_point: { + lat: 13.131313, + lon: 31.313131 + } } ]; - var expected = { - 'type': 'FeatureCollection', - 'bbox': [ -73.985656, 40.748432, -0.101795, 51.5337144 ], - 'features': [ + const geojsonify = proxyquire('../../../helper/geojsonify', { + './geojsonify_place_details': (params, source, dst) => { + if (source._id === 'id 1') { + dst.property1 = 'property 1'; + dst.property2 = 'property 2'; + } else if (source._id === 'id 2') { + dst.property3 = 'property 3'; + dst.property4 = 'property 4'; + } + + } + }); + + const actual = geojsonify({}, input); + + const expected = { + type: 'FeatureCollection', + features: [ { - 'type': 'Feature', - 'geometry': { - 'type': 'Point', - 'coordinates': [ - -0.1069716, - 51.5337144 - ] + type: 'Feature', + geometry: { + type: 'Point', + coordinates: [ 21.212121, 12.121212 ] }, - 'properties': { - 'id': 'id1', - 'gid': 'source1:layer1:id1', - 'layer': 'layer1', - 'source': 'source1', - 'source_id': 'source_id_1', - 'name': '\'Round Midnight Jazz and Blues Bar', - 'country_a': 'GBR', - 'country': 'United Kingdom', - 'dependency': 'dependency name', - 'macroregion': 'England', - 'region': 'Islington', - 'region_a': 'ISL', - 'county': 'Angel', - 'localadmin': 'test1', - 'locality': 'test2', - 'neighbourhood': 'test3', - 'housenumber': '13', - 'street': 'Liverpool Road', - 'postalcode': 'N1 0RW', - 'category': [ - 'food', - 'nightlife' - ], - 'label': 'label for id id1' + properties: { + id: 'id 1', + gid: 'source 1:layer 1:id 1', + layer: 'layer 1', + source: 'source 1', + source_id: 'source_id 1', + name: 'name 1', + property1: 'property 1', + property2: 'property 2' } }, { - 'type': 'Feature', - 'geometry': { - 'type': 'Point', - 'coordinates': [ - -0.101795, - 51.517806 - ] + type: 'Feature', + geometry: { + type: 'Point', + coordinates: [ 31.313131, 13.131313 ] }, - 'properties': { - 'id': 'id2', - 'gid': 'source2:layer2:id2', - 'layer': 'layer2', - 'source': 'source2', - 'source_id': 'source_id_2', - 'name': 'Blues Cafe', - 'country_a': 'GBR', - 'country': 'United Kingdom', - 'dependency': 'dependency name', - 'macroregion': 'England', - 'region': 'City And County Of The City Of London', - 'region_a': 'COL', - 'county': 'Smithfield', - 'localadmin': 'test1', - 'locality': 'test2', - 'neighbourhood': 'test3', - 'label': 'label for id id2' - } + properties: { + id: 'id 2', + gid: 'source 2:layer 2:id 2', + layer: 'layer 2', + source: 'source 2', + source_id: 'source_id 2', + name: 'name 2', + property3: 'property 3', + property4: 'property 4' + }, + bbox: [ -3, -3, -1, -1 ] + } + ], + bbox: [ -3, -3, 21.212121, 12.121212 ] + }; + + t.deepEquals(actual, expected); + t.end(); + + }); + +}; + +module.exports.tests.non_optimal_conditions = (test, common) => { + test('null/undefined places should log warnings and be ignored', t => { + const logger = require('pelias-mock-logger')(); + + const input = [ + null, + undefined, + { + _id: 'id 1', + source: 'source 1', + source_id: 'source_id 1', + layer: 'layer 1', + name: { + default: 'name 1', }, + center_point: { + lat: 12.121212, + lon: 21.212121 + } + } + ]; + + const geojsonify = proxyquire('../../../helper/geojsonify', { + './geojsonify_place_details': (params, source, dst) => { + if (source._id === 'id 1') { + dst.property1 = 'property 1'; + dst.property2 = 'property 2'; + } + }, + 'pelias-logger': logger + }); + + const actual = geojsonify({}, input); + + const expected = { + type: 'FeatureCollection', + features: [ { - 'type': 'Feature', - 'geometry': { - 'type': 'Point', - 'coordinates': [ - -73.985656, - 40.748432 - ] + type: 'Feature', + geometry: { + type: 'Point', + coordinates: [ 21.212121, 12.121212 ] }, - 'properties': { - 'id': 'node:34633854', - 'gid': 'openstreetmap:venue:node:34633854', - 'layer': 'venue', - 'source': 'openstreetmap', - 'source_id': 'source_id_3', - 'name': 'Empire State Building', - 'country_a': 'USA', - 'country': 'United States', - 'dependency': 'dependency name', - 'region': 'New York', - 'region_a': 'NY', - 'county': 'New York', - 'borough': 'Manhattan', - 'locality': 'New York', - 'neighbourhood': 'Koreatown', - 'category': [ - 'tourism', - 'transport' - ], - 'label': 'label for id node:34633854' + properties: { + id: 'id 1', + gid: 'source 1:layer 1:id 1', + layer: 'layer 1', + source: 'source 1', + source_id: 'source_id 1', + name: 'name 1', + property1: 'property 1', + property2: 'property 2' } } - ] + ], + bbox: [21.212121, 12.121212, 21.212121, 12.121212] }; - var json = geojsonify( {categories: 'foo'}, input ); - - t.deepEqual(json, expected, 'all docs mapped'); + t.deepEquals(actual, expected); + t.ok(logger.isWarnMessage('No doc or center_point property')); t.end(); + }); - test('filtering out empty items', function (t) { - var input = [ + test('places w/o center_point should log warnings and be ignored', t => { + const logger = require('pelias-mock-logger')(); + + const input = [ { - 'bounding_box': { - 'min_lat': 40.6514712164, - 'max_lat': 40.6737320588, - 'min_lon': -73.8967895508, - 'max_lon': -73.8665771484 - }, - 'locality': [ - 'New York' - ], - 'source': 'whosonfirst', - 'layer': 'neighbourhood', - 'population': 173198, - 'popularity': 495, - 'center_point': { - 'lon': -73.881319, - 'lat': 40.663303 - }, - 'name': { - 'default': 'East New York' + _id: 'id 1', + source: 'source 1', + source_id: 'source_id 1', + layer: 'layer 1', + name: { + default: 'name 1', + } + }, + { + _id: 'id 2', + source: 'source 2', + source_id: 'source_id 2', + layer: 'layer 2', + name: { + default: 'name 2', }, - 'source_id': '85816607', - 'category': ['government'], - '_id': '85816607', - '_type': 'neighbourhood', - '_score': 21.434, - 'confidence': 0.888, - 'country': [ - 'United States' - ], - 'country_gid': [ - '85633793' - ], - 'country_a': [ - 'USA' - ], - 'dependency': [ - 'dependency name' - ], - 'dependency_gid': [ - 'dependency id' - ], - 'dependency_a': [ - 'dependency abbrevation' - ], - 'macroregion': [ - 'MacroRegion Name' - ], - 'macroregion_gid': [ - 'MacroRegion Id' - ], - 'macroregion_a': [ - 'MacroRegion Abbreviation' - ], - 'region': [ - 'New York' - ], - 'region_gid': [ - '85688543' - ], - 'region_a': [ - 'NY' - ], - 'macrocounty': [ - 'MacroCounty Name' - ], - 'macrocounty_gid': [ - 'MacroCounty Id' - ], - 'macrocounty_a': [ - 'MacroCounty Abbreviation' - ], - 'county': [ - 'Kings County' - ], - 'county_gid': [ - '102082361' - ], - 'county_a': [ - null - ], - 'borough': [ - 'Brooklyn' - ], - 'localadmin_gid': [ - '404521211' - ], - 'borough_a': [ - null - ], - 'locality_gid': [ - '85977539' - ], - 'locality_a': [ - null - ], - 'neighbourhood': [], - 'neighbourhood_gid': [], - 'label': 'label for id 85816607' + center_point: { + lat: 13.131313, + lon: 31.313131 + } } ]; - var expected = { - 'type': 'FeatureCollection', - 'bbox': [-73.8967895508, 40.6514712164, -73.8665771484, 40.6737320588], - 'features': [ + const geojsonify = proxyquire('../../../helper/geojsonify', { + './geojsonify_place_details': (params, source, dst) => { + if (source._id === 'id 2') { + dst.property3 = 'property 3'; + dst.property4 = 'property 4'; + } + }, + 'pelias-logger': logger + }); + + const actual = geojsonify({}, input); + + const expected = { + type: 'FeatureCollection', + features: [ { - 'type': 'Feature', - 'properties': { - 'id': '85816607', - 'gid': 'whosonfirst:neighbourhood:85816607', - 'layer': 'neighbourhood', - 'source': 'whosonfirst', - 'source_id': '85816607', - 'name': 'East New York', - 'category': ['government'], - 'confidence': 0.888, - 'country': 'United States', - 'country_gid': '85633793', - 'country_a': 'USA', - 'dependency': 'dependency name', - 'dependency_gid': 'dependency id', - 'dependency_a': 'dependency abbrevation', - 'macroregion': 'MacroRegion Name', - 'macroregion_gid': 'MacroRegion Id', - 'macroregion_a': 'MacroRegion Abbreviation', - 'region': 'New York', - 'region_gid': '85688543', - 'region_a': 'NY', - 'macrocounty': 'MacroCounty Name', - 'macrocounty_gid': 'MacroCounty Id', - 'macrocounty_a': 'MacroCounty Abbreviation', - 'county': 'Kings County', - 'borough': 'Brooklyn', - 'county_gid': '102082361', - 'localadmin_gid': '404521211', - 'locality': 'New York', - 'locality_gid': '85977539', - 'label': 'label for id 85816607' + type: 'Feature', + geometry: { + type: 'Point', + coordinates: [ 31.313131, 13.131313 ] }, - 'bbox': [-73.8967895508,40.6514712164,-73.8665771484,40.6737320588], - 'geometry': { - 'type': 'Point', - 'coordinates': [ - -73.881319, - 40.663303 - ] + properties: { + id: 'id 2', + gid: 'source 2:layer 2:id 2', + layer: 'layer 2', + source: 'source 2', + source_id: 'source_id 2', + name: 'name 2', + property3: 'property 3', + property4: 'property 4' } } - ] + ], + bbox: [31.313131, 13.131313, 31.313131, 13.131313] }; - var json = geojsonify( {categories: 'foo'}, input ); - - t.deepEqual(json, expected, 'all wanted properties exposed'); + t.deepEquals(actual, expected); + t.ok(logger.isWarnMessage('No doc or center_point property')); t.end(); + }); -}; -module.exports.tests.categories = function (test, common) { - test('only set category if categories filter was used', function (t) { - var input = [ + test('places w/o names should log warnings and be ignored', t => { + const logger = require('pelias-mock-logger')(); + + const input = [ { - '_id': '85816607', - 'bounding_box': { - 'min_lat': 40.6514712164, - 'max_lat': 40.6737320588, - 'min_lon': -73.8967895508, - 'max_lon': -73.8665771484 - }, - 'source': 'whosonfirst', - 'layer': 'neighbourhood', - 'center_point': { - 'lon': -73.881319, - 'lat': 40.663303 - }, - 'name': { - 'default': 'East New York' + _id: 'id 1', + source: 'source 1', + source_id: 'source_id 1', + layer: 'layer 1', + center_point: { + lat: 12.121212, + lon: 21.212121 + } + }, + { + _id: 'id 2', + source: 'source 2', + source_id: 'source_id 2', + layer: 'layer 2', + name: {}, + center_point: { + lat: 13.131313, + lon: 31.313131 + } + }, + { + _id: 'id 3', + source: 'source 3', + source_id: 'source_id 3', + layer: 'layer 3', + name: { + default: 'name 3', }, - 'source_id': '85816607', - 'category': ['government'], - 'label': 'label for id 85816607' + center_point: { + lat: 14.141414, + lon: 41.414141 + } } ]; - var expected = { - 'type': 'FeatureCollection', - 'bbox': [-73.8967895508, 40.6514712164, -73.8665771484, 40.6737320588], - 'features': [ + const geojsonify = proxyquire('../../../helper/geojsonify', { + './geojsonify_place_details': (params, source, dst) => { + if (source._id === 'id 1') { + dst.property1 = 'property 1'; + dst.property2 = 'property 2'; + } + else if (source._id === 'id 2') { + dst.property3 = 'property 3'; + dst.property4 = 'property 4'; + } + else if (source._id === 'id 3') { + dst.property5 = 'property 5'; + dst.property6 = 'property 6'; + } + }, + 'pelias-logger': logger + }); + + const actual = geojsonify({}, input); + + const expected = { + type: 'FeatureCollection', + features: [ + { + type: 'Feature', + geometry: { + type: 'Point', + coordinates: [ 21.212121, 12.121212 ] + }, + properties: { + id: 'id 1', + gid: 'source 1:layer 1:id 1', + layer: 'layer 1', + source: 'source 1', + source_id: 'source_id 1', + property1: 'property 1', + property2: 'property 2' + } + }, { - 'type': 'Feature', - 'properties': { - 'id': '85816607', - 'gid': 'whosonfirst:neighbourhood:85816607', - 'layer': 'neighbourhood', - 'source': 'whosonfirst', - 'source_id': '85816607', - 'name': 'East New York', - 'category': ['government'], - 'label': 'label for id 85816607' + type: 'Feature', + geometry: { + type: 'Point', + coordinates: [ 31.313131, 13.131313 ] }, - 'bbox': [-73.8967895508,40.6514712164,-73.8665771484,40.6737320588], - 'geometry': { - 'type': 'Point', - 'coordinates': [ - -73.881319, - 40.663303 - ] + properties: { + id: 'id 2', + gid: 'source 2:layer 2:id 2', + layer: 'layer 2', + source: 'source 2', + source_id: 'source_id 2', + property3: 'property 3', + property4: 'property 4' + } + }, + { + type: 'Feature', + geometry: { + type: 'Point', + coordinates: [ 41.414141, 14.141414 ] + }, + properties: { + id: 'id 3', + gid: 'source 3:layer 3:id 3', + layer: 'layer 3', + source: 'source 3', + source_id: 'source_id 3', + name: 'name 3', + property5: 'property 5', + property6: 'property 6' } } - ] + ], + bbox: [21.212121, 12.121212, 41.414141, 14.141414] }; - var json = geojsonify( {categories: 'foo'}, input ); - - t.deepEqual(json, expected, 'all wanted properties exposed'); + t.deepEquals(actual, expected); + t.ok(logger.isWarnMessage('doc source 1:layer 1:id 1 does not contain name.default')); + t.ok(logger.isWarnMessage('doc source 2:layer 2:id 2 does not contain name.default')); t.end(); + }); -}; -module.exports.all = function (tape, common) { +}; +module.exports.all = (tape, common) => { function test(name, testFunction) { - return tape('geojsonify: ' + name, testFunction); + return tape(`geojsonify: ${name}`, testFunction); } for( var testCase in module.exports.tests ){ From 20bf53ef0aac494b2413e57e99097df9ced88b02 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 17:05:29 -0400 Subject: [PATCH 28/42] inlined geojsonify_meta_data since it's pretty simple --- helper/geojsonify.js | 14 ++++++++++-- helper/geojsonify_meta_data.js | 42 ---------------------------------- 2 files changed, 12 insertions(+), 44 deletions(-) delete mode 100644 helper/geojsonify_meta_data.js diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 79356516..ad67296c 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -3,8 +3,9 @@ const GeoJSON = require('geojson'); const extent = require('@mapbox/geojson-extent'); const logger = require('pelias-logger').get('geojsonify'); const addDetails = require('./geojsonify_place_details'); -const addMetaData = require('./geojsonify_meta_data'); +// const addMetaData = require('./geojsonify_meta_data'); const _ = require('lodash'); +const Document = require('pelias-model').Document; function geojsonifyPlaces( params, docs ){ @@ -41,7 +42,16 @@ function geojsonifyPlaces( params, docs ){ function geojsonifyPlace(params, place) { const output = {}; - addMetaData(place, output); + output.id = place._id; + output.gid = new Document(place.source, place.layer, place._id).getGid(); + output.layer = place.layer; + output.source = place.source; + output.source_id = place.source_id; + if (place.hasOwnProperty('bounding_box')) { + output.bounding_box = place.bounding_box; + } + + // addMetaData(place, output); addName(place, output); addDetails(params, place, output); diff --git a/helper/geojsonify_meta_data.js b/helper/geojsonify_meta_data.js deleted file mode 100644 index 7fdf0394..00000000 --- a/helper/geojsonify_meta_data.js +++ /dev/null @@ -1,42 +0,0 @@ -var Document = require('pelias-model').Document; - -/** - * Determine and set place id, type, and source - * - * @param {object} src - * @param {object} dst - */ -function addMetaData(src, dst) { - dst.id = src._id; - dst.gid = makeGid(src); - dst.layer = lookupLayer(src); - dst.source = lookupSource(src); - dst.source_id = lookupSourceId(src); - if (src.hasOwnProperty('bounding_box')) { - dst.bounding_box = src.bounding_box; - } -} - -/** - * Create a gid from a document - * - * @param {object} src - */ -function makeGid(src) { - var doc = new Document(lookupSource(src), lookupLayer(src), src._id); - return doc.getGid(); -} - -function lookupSource(src) { - return src.source; -} - -function lookupSourceId(src) { - return src.source_id; -} - -function lookupLayer(src) { - return src.layer; -} - -module.exports = addMetaData; \ No newline at end of file From b02d20c5ee0c53996f7b33ad8f08eaf8223537c9 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 17:06:24 -0400 Subject: [PATCH 29/42] removed commented out line --- helper/geojsonify.js | 1 - 1 file changed, 1 deletion(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index ad67296c..45aad1f8 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -51,7 +51,6 @@ function geojsonifyPlace(params, place) { output.bounding_box = place.bounding_box; } - // addMetaData(place, output); addName(place, output); addDetails(params, place, output); From 0dfe67046bdfa2f4e75b98132f90884b41c36fcc Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 17:06:47 -0400 Subject: [PATCH 30/42] removed commented out line --- helper/geojsonify.js | 1 - 1 file changed, 1 deletion(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 45aad1f8..9b3820b9 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -3,7 +3,6 @@ const GeoJSON = require('geojson'); const extent = require('@mapbox/geojson-extent'); const logger = require('pelias-logger').get('geojsonify'); const addDetails = require('./geojsonify_place_details'); -// const addMetaData = require('./geojsonify_meta_data'); const _ = require('lodash'); const Document = require('pelias-model').Document; From a85e079544777986c29012768c54c1d56e0c1faa Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 17:09:02 -0400 Subject: [PATCH 31/42] inlined metadata assignments --- helper/geojsonify.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 9b3820b9..0fc96ea1 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -39,13 +39,14 @@ function geojsonifyPlaces( params, docs ){ } function geojsonifyPlace(params, place) { - const output = {}; + const output = { + id: place._id, + gid: new Document(place.source, place.layer, place._id).getGid(), + layer: place.layer, + source: place.source, + source_id: place.source_id + }; - output.id = place._id; - output.gid = new Document(place.source, place.layer, place._id).getGid(); - output.layer = place.layer; - output.source = place.source; - output.source_id = place.source_id; if (place.hasOwnProperty('bounding_box')) { output.bounding_box = place.bounding_box; } From 1fb50dc6bd36582093d5bf8009f7030214cfe3ef Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 17:10:01 -0400 Subject: [PATCH 32/42] removed superfluous conditional --- helper/geojsonify.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 0fc96ea1..92b17b2f 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -83,11 +83,6 @@ function addName(src, dst) { */ function addBBoxPerFeature(geojson) { geojson.features.forEach(feature => { - - if (!feature.properties.hasOwnProperty('bounding_box')) { - return; - } - if (feature.properties.bounding_box) { feature.bbox = [ feature.properties.bounding_box.min_lon, From 314e7eb45060851b818a670d22cd256c4c30a276 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 11 Sep 2017 17:11:47 -0400 Subject: [PATCH 33/42] converted to reduce --- helper/geojsonify.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 92b17b2f..107c19c2 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -105,8 +105,7 @@ function addBBoxPerFeature(geojson) { * @returns {Array} */ function extractExtentPoints(geodata) { - const extentPoints = []; - geodata.forEach(function (place) { + return geodata.reduce((extentPoints, place) => { if (place.bounding_box) { extentPoints.push({ lng: place.bounding_box.min_lon, @@ -116,16 +115,19 @@ function extractExtentPoints(geodata) { lng: place.bounding_box.max_lon, lat: place.bounding_box.max_lat }); + } else { extentPoints.push({ lng: place.lng, lat: place.lat }); + } - }); + return extentPoints; + + }, []); - return extentPoints; } /** From cd99f87c04a2593d1eb743594f9d1b26281793bd Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 12 Sep 2017 10:28:24 -0400 Subject: [PATCH 34/42] inlined and condensed doc setup --- helper/geojsonify.js | 41 +++++++++++++--------------------- test/unit/helper/geojsonify.js | 24 ++++++++++++++++++++ 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 107c19c2..da7285a5 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -39,41 +39,28 @@ function geojsonifyPlaces( params, docs ){ } function geojsonifyPlace(params, place) { - const output = { + // setup the base doc + const doc = { id: place._id, gid: new Document(place.source, place.layer, place._id).getGid(), layer: place.layer, source: place.source, - source_id: place.source_id + source_id: place.source_id, + bounding_box: place.bounding_box, + lat: parseFloat(place.center_point.lat), + lng: parseFloat(place.center_point.lon) }; - if (place.hasOwnProperty('bounding_box')) { - output.bounding_box = place.bounding_box; + // assign name, logging a warning if it doesn't exist + if (_.has(place, 'name.default')) { + doc.name = place.name.default; + } else { + logger.warn(`doc ${doc.gid} does not contain name.default`); } - addName(place, output); - addDetails(params, place, output); - - // map center_point for GeoJSON to work properly - // these should not show up in the final feature properties - output.lat = parseFloat(place.center_point.lat); - output.lng = parseFloat(place.center_point.lon); + addDetails(params, place, doc); - return output; -} - -/** - * Validate and add name property - * - * @param {object} src - * @param {object} dst - */ -function addName(src, dst) { - if (_.has(src, 'name.default')) { - dst.name = src.name.default; - } else { - logger.warn(`doc ${dst.gid} does not contain name.default`); - } + return doc; } /** @@ -106,6 +93,7 @@ function addBBoxPerFeature(geojson) { */ function extractExtentPoints(geodata) { return geodata.reduce((extentPoints, place) => { + // if there's a bounding_box, use the LL/UR for the extent if (place.bounding_box) { extentPoints.push({ lng: place.bounding_box.min_lon, @@ -118,6 +106,7 @@ function extractExtentPoints(geodata) { } else { + // otherwise, use the point for the extent extentPoints.push({ lng: place.lng, lat: place.lat diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index 08571aee..8d6fc490 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -591,6 +591,30 @@ module.exports.tests.non_optimal_conditions = (test, common) => { }); + test('no points', t => { + const logger = require('pelias-mock-logger')(); + + const input = []; + + const geojsonify = proxyquire('../../../helper/geojsonify', { + './geojsonify_place_details': (params, source, dst) => { + t.fail('should not have bee called'); + }, + 'pelias-logger': logger + }); + + const actual = geojsonify({}, input); + + const expected = { + type: 'FeatureCollection', + features: [] + }; + + t.deepEquals(actual, expected); + t.end(); + + }); + }; module.exports.all = (tape, common) => { From 9d54735e4112610d381562465a17675cba34f60a Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 12 Sep 2017 11:12:26 -0400 Subject: [PATCH 35/42] converted geojsonify_place_details to return instead of modify --- helper/geojsonify.js | 2 +- helper/geojsonify_place_details.js | 22 ++-- test/unit/helper/geojsonify.js | 73 ++++++++----- test/unit/helper/geojsonify_place_details.js | 106 +++++-------------- 4 files changed, 85 insertions(+), 118 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index da7285a5..fdadc2b2 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -58,7 +58,7 @@ function geojsonifyPlace(params, place) { logger.warn(`doc ${doc.gid} does not contain name.default`); } - addDetails(params, place, doc); + _.assign(doc, addDetails(params, place)); return doc; } diff --git a/helper/geojsonify_place_details.js b/helper/geojsonify_place_details.js index 33879c39..bca94fab 100644 --- a/helper/geojsonify_place_details.js +++ b/helper/geojsonify_place_details.js @@ -53,19 +53,18 @@ function checkCategoryParam(params) { } /** - * Copy specified properties from source to dest. + * Collect the specified properties from source into an object and return it * Ignore missing properties. * * @param {object} params clean query params * @param {object} source * @param {object} dst */ -function copyProperties( params, source, dst ) { - DETAILS_PROPS.forEach( function ( prop ) { - - // if condition isn't met, just return without setting the property +function collectProperties( params, source ) { + return DETAILS_PROPS.reduce((result, prop) => { + // if condition isn't met, don't set the property if (_.isFunction(prop.condition) && !prop.condition(params)) { - return; + return result; } if ( source.hasOwnProperty( prop.name ) ) { @@ -84,10 +83,14 @@ function copyProperties( params, source, dst ) { } if (_.isNumber(value) || (value && !_.isEmpty(value))) { - dst[prop.name] = value; + result[prop.name] = value; } } - }); + + return result; + + }, {}); + } function getStringValue(property) { @@ -108,7 +111,6 @@ function getStringValue(property) { return _.toString(property); } - function getArrayValue(property) { // isEmpty check works for all types of values: strings, arrays, objects if (_.isEmpty(property)) { @@ -122,4 +124,4 @@ function getArrayValue(property) { return [property]; } -module.exports = copyProperties; +module.exports = collectProperties; diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index 8d6fc490..72ff6392 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -71,11 +71,15 @@ module.exports.tests.all = (test, common) => { const geojsonify = proxyquire('../../../helper/geojsonify', { './geojsonify_place_details': (params, source, dst) => { if (source._id === 'id 1') { - dst.property1 = 'property 1'; - dst.property2 = 'property 2'; + return { + property1: 'property 1', + property2: 'property 2' + }; } else if (source._id === 'id 2') { - dst.property3 = 'property 3'; - dst.property4 = 'property 4'; + return { + property3: 'property 3', + property4: 'property 4' + }; } } @@ -174,11 +178,15 @@ module.exports.tests.all = (test, common) => { const geojsonify = proxyquire('../../../helper/geojsonify', { './geojsonify_place_details': (params, source, dst) => { if (source._id === 'id 1') { - dst.property1 = 'property 1'; - dst.property2 = 'property 2'; + return { + property1: 'property 1', + property2: 'property 2' + }; } else if (source._id === 'id 2') { - dst.property3 = 'property 3'; - dst.property4 = 'property 4'; + return { + property3: 'property 3', + property4: 'property 4' + }; } } @@ -273,11 +281,15 @@ module.exports.tests.all = (test, common) => { const geojsonify = proxyquire('../../../helper/geojsonify', { './geojsonify_place_details': (params, source, dst) => { if (source._id === 'id 1') { - dst.property1 = 'property 1'; - dst.property2 = 'property 2'; + return { + property1: 'property 1', + property2: 'property 2' + }; } else if (source._id === 'id 2') { - dst.property3 = 'property 3'; - dst.property4 = 'property 4'; + return { + property3: 'property 3', + property4: 'property 4' + }; } } @@ -359,8 +371,10 @@ module.exports.tests.non_optimal_conditions = (test, common) => { const geojsonify = proxyquire('../../../helper/geojsonify', { './geojsonify_place_details': (params, source, dst) => { if (source._id === 'id 1') { - dst.property1 = 'property 1'; - dst.property2 = 'property 2'; + return { + property1: 'property 1', + property2: 'property 2' + }; } }, 'pelias-logger': logger @@ -429,8 +443,10 @@ module.exports.tests.non_optimal_conditions = (test, common) => { const geojsonify = proxyquire('../../../helper/geojsonify', { './geojsonify_place_details': (params, source, dst) => { if (source._id === 'id 2') { - dst.property3 = 'property 3'; - dst.property4 = 'property 4'; + return { + property3: 'property 3', + property4: 'property 4' + }; } }, 'pelias-logger': logger @@ -511,16 +527,21 @@ module.exports.tests.non_optimal_conditions = (test, common) => { const geojsonify = proxyquire('../../../helper/geojsonify', { './geojsonify_place_details': (params, source, dst) => { if (source._id === 'id 1') { - dst.property1 = 'property 1'; - dst.property2 = 'property 2'; - } - else if (source._id === 'id 2') { - dst.property3 = 'property 3'; - dst.property4 = 'property 4'; - } - else if (source._id === 'id 3') { - dst.property5 = 'property 5'; - dst.property6 = 'property 6'; + return { + property1: 'property 1', + property2: 'property 2' + }; + } else if (source._id === 'id 2') { + return { + property3: 'property 3', + property4: 'property 4' + }; + } else if (source._id === 'id 3') { + return { + property5: 'property 5', + property6: 'property 6' + }; + } }, 'pelias-logger': logger diff --git a/test/unit/helper/geojsonify_place_details.js b/test/unit/helper/geojsonify_place_details.js index 3bc96adc..97b444f7 100644 --- a/test/unit/helper/geojsonify_place_details.js +++ b/test/unit/helper/geojsonify_place_details.js @@ -5,7 +5,6 @@ module.exports.tests = {}; module.exports.tests.geojsonify_place_details = (test, common) => { test('plain old string values should be copied verbatim, replacing old values', t => { const source = { - unknown_field: 'original unknown_field value', housenumber: 'housenumber value', street: 'street value', postalcode: 'postalcode value', @@ -43,48 +42,8 @@ module.exports.tests.geojsonify_place_details = (test, common) => { neighbourhood_gid: 'neighbourhood_gid value', label: 'label value' }; - const destination = { - unknown_field: 'original unknown_field value', - housenumber: 'original housenumber value', - street: 'original street value', - postalcode: 'original postalcode value', - postalcode_gid: 'original postalcode_gid value', - match_type: 'original match_type value', - accuracy: 'original accuracy value', - country: 'original country value', - country_gid: 'original country_gid value', - country_a: 'original country_a value', - dependency: 'original dependency value', - dependency_gid: 'original dependency_gid value', - dependency_a: 'original dependency_a value', - macroregion: 'original macroregion value', - macroregion_gid: 'original macroregion_gid value', - macroregion_a: 'original macroregion_a value', - region: 'original region value', - region_gid: 'original region_gid value', - region_a: 'original region_a value', - macrocounty: 'original macrocounty value', - macrocounty_gid: 'original macrocounty_gid value', - macrocounty_a: 'original macrocounty_a value', - county: 'original county value', - county_gid: 'original county_gid value', - county_a: 'original county_a value', - localadmin: 'original localadmin value', - localadmin_gid: 'original localadmin_gid value', - localadmin_a: 'original localadmin_a value', - locality: 'original locality value', - locality_gid: 'original locality_gid value', - locality_a: 'original locality_a value', - borough: 'original borough value', - borough_gid: 'original borough_gid value', - borough_a: 'original borough_a value', - neighbourhood: 'original neighbourhood value', - neighbourhood_gid: 'original neighbourhood_gid value', - label: 'original label value' - }; const expected = { - unknown_field: 'original unknown_field value', housenumber: 'housenumber value', street: 'street value', postalcode: 'postalcode value', @@ -123,9 +82,9 @@ module.exports.tests.geojsonify_place_details = (test, common) => { label: 'label value' }; - geojsonify({}, source, destination); + const actual = geojsonify({}, source); - t.deepEqual(destination, expected); + t.deepEqual(actual, expected); t.end(); }); @@ -170,12 +129,11 @@ module.exports.tests.geojsonify_place_details = (test, common) => { neighbourhood_gid: empty_value, label: empty_value }; - const destination = {}; const expected = {}; - geojsonify({}, source, destination); + const actual = geojsonify({}, source); - t.deepEqual(destination, expected); + t.deepEqual(actual, expected); }); @@ -222,7 +180,6 @@ module.exports.tests.geojsonify_place_details = (test, common) => { neighbourhood_gid: ['neighbourhood_gid value 1', 'neighbourhood_gid value 2'], label: ['label value 1', 'label value 2'] }; - const destination = { }; const expected = { housenumber: 'housenumber value 1', @@ -263,9 +220,9 @@ module.exports.tests.geojsonify_place_details = (test, common) => { label: 'label value 1' }; - geojsonify({}, source, destination); + const actual = geojsonify({}, source); - t.deepEqual(destination, expected); + t.deepEqual(actual, expected); t.end(); }); @@ -310,7 +267,6 @@ module.exports.tests.geojsonify_place_details = (test, common) => { neighbourhood_gid: { neighbourhood_gid: 'neighbourhood_gid value'}, label: { label: 'label value'} }; - const destination = { }; const expected = { housenumber: '[object Object]', @@ -351,9 +307,9 @@ module.exports.tests.geojsonify_place_details = (test, common) => { label: '[object Object]' }; - geojsonify({}, source, destination); + const actual = geojsonify({}, source); - t.deepEqual(destination, expected); + t.deepEqual(actual, expected); t.end(); }); @@ -365,11 +321,6 @@ module.exports.tests.geojsonify_place_details = (test, common) => { distance: value, bounding_box: value }; - const destination = { - confidence: 'original confidence value', - distance: 'original distance value', - bounding_box: 'original bounding_box value' - }; const expected = { confidence: value, @@ -377,9 +328,9 @@ module.exports.tests.geojsonify_place_details = (test, common) => { bounding_box: value }; - geojsonify({}, source, destination); + const actual = geojsonify({}, source); - t.deepEqual(destination, expected); + t.deepEqual(actual, expected); }); @@ -394,16 +345,15 @@ module.exports.tests.geojsonify_place_details = (test, common) => { distance: value, bounding_box: value }; - const destination = {}; const expected = { confidence: value, distance: value, bounding_box: value }; - geojsonify({}, source, destination); + const actual = geojsonify({}, source); - t.deepEqual(destination, expected); + t.deepEqual(actual, expected); }); @@ -418,12 +368,11 @@ module.exports.tests.geojsonify_place_details = (test, common) => { distance: value, bounding_box: value }; - const destination = {}; const expected = {}; - geojsonify({}, source, destination); + const actual = geojsonify({}, source); - t.deepEqual(destination, expected); + t.deepEqual(actual, expected); }); @@ -435,12 +384,11 @@ module.exports.tests.geojsonify_place_details = (test, common) => { const source = { category: [] }; - const destination = {}; const expected = {}; - geojsonify({}, source, destination); + const actual = geojsonify({}, source); - t.deepEqual(destination, expected); + t.deepEqual(actual, expected); t.end(); }); @@ -449,7 +397,6 @@ module.exports.tests.geojsonify_place_details = (test, common) => { const source = { category: [ 1, 2 ] }; - const destination = {}; const expected = { category: [ 1, 2 ] }; @@ -458,9 +405,9 @@ module.exports.tests.geojsonify_place_details = (test, common) => { categories: true }; - geojsonify(clean, source, destination); + const actual = geojsonify(clean, source); - t.deepEqual(destination, expected); + t.deepEqual(actual, expected); t.end(); }); @@ -470,7 +417,6 @@ module.exports.tests.geojsonify_place_details = (test, common) => { const source = { category: value }; - const destination = {}; const expected = { category: [ value ] }; @@ -479,9 +425,9 @@ module.exports.tests.geojsonify_place_details = (test, common) => { categories: true }; - geojsonify(clean, source, destination); + const actual = geojsonify(clean, source); - t.deepEqual(destination, expected); + t.deepEqual(actual, expected); }); @@ -493,15 +439,14 @@ module.exports.tests.geojsonify_place_details = (test, common) => { const source = { category: [ 1, 2 ] }; - const destination = {}; const expected = { }; const clean = {}; - geojsonify(clean, source, destination); + const actual = geojsonify(clean, source); - t.deepEqual(destination, expected); + t.deepEqual(actual, expected); t.end(); }); @@ -510,19 +455,18 @@ module.exports.tests.geojsonify_place_details = (test, common) => { const source = { category: [ 1, 2 ] }; - const destination = {}; const expected = { }; const clean = 'this is not an object'; - geojsonify(clean, source, destination); + const actual = geojsonify(clean, source); - t.deepEqual(destination, expected); + t.deepEqual(actual, expected); t.end(); }); - + }; module.exports.all = (tape, common) => { From 81be0654180467dc5d23b13638e7426a318f8b02 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 12 Sep 2017 11:17:33 -0400 Subject: [PATCH 36/42] renamed import, added comment --- helper/geojsonify.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index fdadc2b2..150a35f7 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -2,7 +2,7 @@ const GeoJSON = require('geojson'); const extent = require('@mapbox/geojson-extent'); const logger = require('pelias-logger').get('geojsonify'); -const addDetails = require('./geojsonify_place_details'); +const collectDetails = require('./geojsonify_place_details'); const _ = require('lodash'); const Document = require('pelias-model').Document; @@ -58,7 +58,8 @@ function geojsonifyPlace(params, place) { logger.warn(`doc ${doc.gid} does not contain name.default`); } - _.assign(doc, addDetails(params, place)); + // extend doc with all the details info + _.assign(doc, collectDetails(params, place)); return doc; } From 214c3807eb4c31555834fd77dd00b70ebc119dae Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 12 Sep 2017 11:26:46 -0400 Subject: [PATCH 37/42] added continent, ocean, and marinearea to geojsonify --- helper/geojsonify_place_details.js | 9 +++ test/unit/helper/geojsonify_place_details.js | 65 +++++++++++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/helper/geojsonify_place_details.js b/helper/geojsonify_place_details.js index bca94fab..b546c9a9 100644 --- a/helper/geojsonify_place_details.js +++ b/helper/geojsonify_place_details.js @@ -43,6 +43,15 @@ const DETAILS_PROPS = [ { name: 'borough_a', type: 'string' }, { name: 'neighbourhood', type: 'string' }, { name: 'neighbourhood_gid', type: 'string' }, + { name: 'continent', type: 'string' }, + { name: 'continent_gid', type: 'string' }, + { name: 'continent_a', type: 'string' }, + { name: 'ocean', type: 'string' }, + { name: 'ocean_gid', type: 'string' }, + { name: 'ocean_a', type: 'string' }, + { name: 'marinearea', type: 'string' }, + { name: 'marinearea_gid', type: 'string' }, + { name: 'marinearea_a', type: 'string' }, { name: 'bounding_box', type: 'default' }, { name: 'label', type: 'string' }, { name: 'category', type: 'array', condition: checkCategoryParam } diff --git a/test/unit/helper/geojsonify_place_details.js b/test/unit/helper/geojsonify_place_details.js index 97b444f7..c1fe22da 100644 --- a/test/unit/helper/geojsonify_place_details.js +++ b/test/unit/helper/geojsonify_place_details.js @@ -40,6 +40,15 @@ module.exports.tests.geojsonify_place_details = (test, common) => { borough_a: 'borough_a value', neighbourhood: 'neighbourhood value', neighbourhood_gid: 'neighbourhood_gid value', + continent: 'continent value', + continent_gid: 'continent_gid value', + continent_a: 'continent_a value', + ocean: 'ocean value', + ocean_gid: 'ocean_gid value', + ocean_a: 'ocean_a value', + marinearea: 'marinearea value', + marinearea_gid: 'marinearea_gid value', + marinearea_a: 'marinearea_a value', label: 'label value' }; @@ -79,6 +88,15 @@ module.exports.tests.geojsonify_place_details = (test, common) => { borough_a: 'borough_a value', neighbourhood: 'neighbourhood value', neighbourhood_gid: 'neighbourhood_gid value', + continent: 'continent value', + continent_gid: 'continent_gid value', + continent_a: 'continent_a value', + ocean: 'ocean value', + ocean_gid: 'ocean_gid value', + ocean_a: 'ocean_a value', + marinearea: 'marinearea value', + marinearea_gid: 'marinearea_gid value', + marinearea_a: 'marinearea_a value', label: 'label value' }; @@ -127,6 +145,15 @@ module.exports.tests.geojsonify_place_details = (test, common) => { borough_a: empty_value, neighbourhood: empty_value, neighbourhood_gid: empty_value, + continent: empty_value, + continent_gid: empty_value, + continent_a: empty_value, + ocean: empty_value, + ocean_gid: empty_value, + ocean_a: empty_value, + marinearea: empty_value, + marinearea_gid: empty_value, + marinearea_a: empty_value, label: empty_value }; const expected = {}; @@ -178,6 +205,15 @@ module.exports.tests.geojsonify_place_details = (test, common) => { borough_a: ['borough_a value 1', 'borough_a value 2'], neighbourhood: ['neighbourhood value 1', 'neighbourhood value 2'], neighbourhood_gid: ['neighbourhood_gid value 1', 'neighbourhood_gid value 2'], + continent: ['continent value 1', 'continent value 2'], + continent_gid: ['continent_gid value 1', 'continent_gid value 2'], + continent_a: ['continent_a value 1', 'continent_a value 2'], + ocean: ['ocean value 1', 'ocean value 2'], + ocean_gid: ['ocean_gid value 1', 'ocean_gid value 2'], + ocean_a: ['ocean_a value 1', 'ocean_a value 2'], + marinearea: ['marinearea value 1', 'marinearea value 2'], + marinearea_gid: ['marinearea_gid value 1', 'marinearea_gid value 2'], + marinearea_a: ['marinearea_a value 1','marinearea_a value 2'], label: ['label value 1', 'label value 2'] }; @@ -217,6 +253,15 @@ module.exports.tests.geojsonify_place_details = (test, common) => { borough_a: 'borough_a value 1', neighbourhood: 'neighbourhood value 1', neighbourhood_gid: 'neighbourhood_gid value 1', + continent: 'continent value 1', + continent_gid: 'continent_gid value 1', + continent_a: 'continent_a value 1', + ocean: 'ocean value 1', + ocean_gid: 'ocean_gid value 1', + ocean_a: 'ocean_a value 1', + marinearea: 'marinearea value 1', + marinearea_gid: 'marinearea_gid value 1', + marinearea_a: 'marinearea_a value 1', label: 'label value 1' }; @@ -265,6 +310,15 @@ module.exports.tests.geojsonify_place_details = (test, common) => { borough_a: { borough_a: 'borough_a value'}, neighbourhood: { neighbourhood: 'neighbourhood value'}, neighbourhood_gid: { neighbourhood_gid: 'neighbourhood_gid value'}, + continent: { continent: 'continent value'} , + continent_gid: { continent: 'continent_gid value'}, + continent_a: { continent: 'continent_a value'}, + ocean: { ocean: 'ocean value'}, + ocean_gid: { ocean_gid: 'ocean_gid value'}, + ocean_a: { ocean_a: 'ocean_a value'}, + marinearea: { marinearea: 'marinearea value'}, + marinearea_gid: { marinearea_gid: 'marinearea_gid value'}, + marinearea_a: { marinearea_a: 'marinearea_a value'}, label: { label: 'label value'} }; @@ -304,6 +358,15 @@ module.exports.tests.geojsonify_place_details = (test, common) => { borough_a: '[object Object]', neighbourhood: '[object Object]', neighbourhood_gid: '[object Object]', + continent: '[object Object]', + continent_gid: '[object Object]', + continent_a: '[object Object]', + ocean: '[object Object]', + ocean_gid: '[object Object]', + ocean_a: '[object Object]', + marinearea: '[object Object]', + marinearea_gid: '[object Object]', + marinearea_a: '[object Object]', label: '[object Object]' }; @@ -466,7 +529,7 @@ module.exports.tests.geojsonify_place_details = (test, common) => { t.end(); }); - + }; module.exports.all = (tape, common) => { From 473c79e13cef355ffe832bae39c2306faa0fbb52 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 13 Sep 2017 13:10:26 -0400 Subject: [PATCH 38/42] use Object.assign instead of lodash.assign --- helper/geojsonify.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 150a35f7..515446ce 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -58,8 +58,8 @@ function geojsonifyPlace(params, place) { logger.warn(`doc ${doc.gid} does not contain name.default`); } - // extend doc with all the details info - _.assign(doc, collectDetails(params, place)); + // assign all the details info into the doc + Object.assign(doc, collectDetails(params, place)); return doc; } From a76859dfa72836f992bbefedb837a624bdb3f9aa Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 13 Sep 2017 13:11:32 -0400 Subject: [PATCH 39/42] added support for ocean, marinearea, continent, and empire --- helper/placeTypes.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/helper/placeTypes.js b/helper/placeTypes.js index defc9274..3591521c 100644 --- a/helper/placeTypes.js +++ b/helper/placeTypes.js @@ -1,4 +1,8 @@ module.exports = [ + 'ocean', + 'marinearea', + 'continent', + 'empire', 'country', 'dependency', 'macroregion', From 5b86fbd7d8a3c9627c802bbcab651e826b5306cf Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 19 Sep 2017 15:15:57 -0400 Subject: [PATCH 40/42] add layers parameter support to PointInPolygon service request --- service/configurations/PointInPolygon.js | 10 ++++ .../service/configurations/PointInPolygon.js | 56 +++++++++++++++++-- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/service/configurations/PointInPolygon.js b/service/configurations/PointInPolygon.js index 66a766f9..58b52093 100644 --- a/service/configurations/PointInPolygon.js +++ b/service/configurations/PointInPolygon.js @@ -11,6 +11,16 @@ class PointInPolygon extends ServiceConfiguration { super('pip', o); } + getParameters(req) { + if (_.has(req, 'clean.layers')) { + return { + layers: _.join(req.clean.layers, ',') + }; + } + + return {}; + } + getUrl(req) { // use resolve to eliminate possibility of duplicate /'s in URL return url.resolve(this.baseUrl, `${req.clean['point.lon']}/${req.clean['point.lat']}`); diff --git a/test/unit/service/configurations/PointInPolygon.js b/test/unit/service/configurations/PointInPolygon.js index c754e7f9..6906950f 100644 --- a/test/unit/service/configurations/PointInPolygon.js +++ b/test/unit/service/configurations/PointInPolygon.js @@ -53,16 +53,62 @@ module.exports.tests.all = (test, common) => { }); - test('getParameters should return an empty object', (t) => { + test('getParameters should return an empty object when req is undefined', (t) => { const configBlob = { - url: 'http://localhost:1234', - timeout: 17, - retries: 19 + url: 'http://localhost:1234' }; const pointInPolygon = new PointInPolygon(configBlob); - t.deepEquals(pointInPolygon.getParameters(), {}); + t.deepEquals(pointInPolygon.getParameters(undefined), {}); + t.end(); + + }); + + test('getParameters should return an empty object when req.clean is undefined', (t) => { + const configBlob = { + url: 'http://localhost:1234' + }; + + const pointInPolygon = new PointInPolygon(configBlob); + + const req = {}; + + t.deepEquals(pointInPolygon.getParameters(req), {}); + t.end(); + + }); + + test('getParameters should return an empty object when req.clean.layers is undefined', (t) => { + const configBlob = { + url: 'http://localhost:1234' + }; + + const pointInPolygon = new PointInPolygon(configBlob); + + const req = { + clean: {} + }; + + t.deepEquals(pointInPolygon.getParameters(req), {}); + t.end(); + + }); + + test('getParameters should return object with layers property when req.clean.layers is specified', t => { + const configBlob = { + url: 'http://localhost:1234' + }; + + const pointInPolygon = new PointInPolygon(configBlob); + + const req = { + clean: { + layers: ['layer1', 'layer2'] + } + }; + + t.deepEquals(pointInPolygon.getParameters(req), { layers: 'layer1,layer2'}); t.end(); }); From 2df5d6d23bc86758dd8814e6565218161590fbc4 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 19 Sep 2017 15:33:56 -0400 Subject: [PATCH 41/42] add empire to coarse reverse controller --- controller/coarse_reverse.js | 1 + test/unit/controller/coarse_reverse.js | 41 ++++++++++++++++---------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js index ce3ad52a..3f8129a3 100644 --- a/controller/coarse_reverse.js +++ b/controller/coarse_reverse.js @@ -16,6 +16,7 @@ const coarse_granularities = [ 'macroregion', 'dependency', 'country', + 'empire', 'continent', 'ocean', 'marinearea' diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js index 403e5875..df231075 100644 --- a/test/unit/controller/coarse_reverse.js +++ b/test/unit/controller/coarse_reverse.js @@ -211,17 +211,21 @@ module.exports.tests.success_conditions = (test, common) => { { id: 100, name: 'country name', abbr: 'xyz'}, { id: 101, name: 'country name 2'} ], + empire: [ + { id: 110, name: 'empire name', abbr: 'empire abbr'}, + { id: 111, name: 'empire name 2'} + ], continent: [ - { id: 110, name: 'continent name', abbr: 'continent abbr'}, - { id: 111, name: 'continent name 2'} + { id: 120, name: 'continent name', abbr: 'continent abbr'}, + { id: 121, name: 'continent name 2'} ], ocean: [ - { id: 120, name: 'ocean name', abbr: 'ocean abbr'}, - { id: 121, name: 'ocean name 2'} + { id: 130, name: 'ocean name', abbr: 'ocean abbr'}, + { id: 131, name: 'ocean name 2'} ], marinearea: [ - { id: 130, name: 'marinearea name', abbr: 'marinearea abbr'}, - { id: 131, name: 'marinearea name 2'} + { id: 140, name: 'marinearea name', abbr: 'marinearea abbr'}, + { id: 141, name: 'marinearea name 2'} ] }; @@ -295,14 +299,17 @@ module.exports.tests.success_conditions = (test, common) => { country: ['country name'], country_id: ['100'], country_a: ['xyz'], + empire: ['empire name'], + empire_id: ['110'], + empire_a: ['empire abbr'], continent: ['continent name'], - continent_id: ['110'], + continent_id: ['120'], continent_a: ['continent abbr'], ocean: ['ocean name'], - ocean_id: ['120'], + ocean_id: ['130'], ocean_a: ['ocean abbr'], marinearea: ['marinearea name'], - marinearea_id: ['130'], + marinearea_id: ['140'], marinearea_a: ['marinearea abbr'], }, center_point: { @@ -844,17 +851,21 @@ module.exports.tests.failure_conditions = (test, common) => { { id: 100, name: 'country name', abbr: 'xyz'}, { id: 101, name: 'country name 2'} ], + empire: [ + { id: 110, name: 'empire name', abbr: 'empire abbr'}, + { id: 111, name: 'empire name 2'} + ], continent: [ - { id: 110, name: 'continent name', abbr: 'continent abbr'}, - { id: 111, name: 'continent name 2'} + { id: 120, name: 'continent name', abbr: 'continent abbr'}, + { id: 121, name: 'continent name 2'} ], ocean: [ - { id: 120, name: 'ocean name', abbr: 'ocean abbr'}, - { id: 121, name: 'ocean name 2'} + { id: 130, name: 'ocean name', abbr: 'ocean abbr'}, + { id: 131, name: 'ocean name 2'} ], marinearea: [ - { id: 130, name: 'marinearea name', abbr: 'marinearea abbr'}, - { id: 131, name: 'marinearea name 2'} + { id: 140, name: 'marinearea name', abbr: 'marinearea abbr'}, + { id: 141, name: 'marinearea name 2'} ] }; From 18e7673dc1b2e381c855835d3a2ede08756216c9 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 19 Sep 2017 15:35:29 -0400 Subject: [PATCH 42/42] added empire to coarse translation --- helper/type_mapping.js | 10 +++++----- test/unit/helper/type_mapping.js | 2 +- test/unit/sanitizer/_layers.js | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/helper/type_mapping.js b/helper/type_mapping.js index d17fd058..3caea41d 100644 --- a/helper/type_mapping.js +++ b/helper/type_mapping.js @@ -49,7 +49,7 @@ var LAYERS_BY_SOURCE = { openaddresses: [ 'address' ], geonames: [ 'country','macroregion', 'region', 'county','localadmin', 'locality','borough', 'neighbourhood', 'venue' ], - whosonfirst: [ 'continent', 'country', 'dependency', 'macroregion', 'region', + whosonfirst: [ 'continent', 'empire', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'borough', 'neighbourhood', 'microhood', 'disputed', 'venue', 'postalcode', 'continent', 'ocean', 'marinearea'] @@ -61,10 +61,10 @@ var LAYERS_BY_SOURCE = { * may have layers that mean the same thing but have a different name */ var LAYER_ALIASES = { - 'coarse': [ 'continent', 'country', 'dependency', 'macroregion', 'region', - 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'borough', - 'neighbourhood', 'microhood', 'disputed', 'postalcode', - 'continent', 'ocean', 'marinearea'] + 'coarse': [ 'continent', 'empire', 'country', 'dependency', 'macroregion', + 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', + 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode', + 'continent', 'ocean', 'marinearea'] }; // create a list of all layers by combining each entry from LAYERS_BY_SOURCE diff --git a/test/unit/helper/type_mapping.js b/test/unit/helper/type_mapping.js index 2ec86e40..ec8cd379 100644 --- a/test/unit/helper/type_mapping.js +++ b/test/unit/helper/type_mapping.js @@ -12,7 +12,7 @@ module.exports.tests.interfaces = function(test, common) { test('alias layer mapping', function(t) { t.deepEquals(type_mapping.layer_mapping.coarse, - [ 'continent', 'country', 'dependency', 'macroregion', + [ 'continent', 'empire', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode', 'continent', 'ocean', 'marinearea']); diff --git a/test/unit/sanitizer/_layers.js b/test/unit/sanitizer/_layers.js index adcffcc2..10e444c2 100644 --- a/test/unit/sanitizer/_layers.js +++ b/test/unit/sanitizer/_layers.js @@ -34,14 +34,14 @@ module.exports.tests.sanitize_layers = function(test, common) { t.deepEqual(clean.layers, venue_layers, 'venue layers set'); t.end(); }); - + test('coarse (alias) layer', function(t) { var raw = { layers: 'coarse' }; var clean = {}; sanitizer.sanitize(raw, clean); - var admin_layers = [ 'continent', 'country', 'dependency', 'macroregion', + var admin_layers = [ 'continent', 'empire', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode', 'ocean', 'marinearea' ]; @@ -77,7 +77,7 @@ module.exports.tests.sanitize_layers = function(test, common) { sanitizer.sanitize(raw, clean); - var expected_layers = [ 'continent', 'country', 'dependency', + var expected_layers = [ 'continent', 'empire', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode', 'ocean', 'marinearea']; @@ -114,7 +114,7 @@ module.exports.tests.sanitize_layers = function(test, common) { sanitizer.sanitize(raw, clean); - var coarse_layers = [ 'continent', + var coarse_layers = [ 'continent', 'empire', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode', 'ocean', 'marinearea' ];