From c1447663ca42321c0b9d353b75aa9e852fc08cc5 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 17 Oct 2016 15:55:19 -0400 Subject: [PATCH 1/5] removed hierarchy walking by parent_id field --- src/hierarchyFinder.js | 25 ------------- test/hierarchyFinderTest.js | 75 ------------------------------------- 2 files changed, 100 deletions(-) diff --git a/src/hierarchyFinder.js b/src/hierarchyFinder.js index b7e53d67..ee0f675e 100644 --- a/src/hierarchyFinder.js +++ b/src/hierarchyFinder.js @@ -1,5 +1,3 @@ -var _ = require('lodash'); - module.exports = {}; var hasName = function(r) { @@ -10,29 +8,6 @@ var isDefined = function(r) { return r; }; -/* - This function builds a hierarchy by starting with the current record and - walking up by parent_id until a parent can't be found, filtering out those - w/o name. -*/ -module.exports.parent_id_walker = function(wofRecords) { - return function(wofRecord) { - // collect all the defined parents, starting with the current record - var parent; - var parents = []; - var parent_id = wofRecord.id; - - while (!_.isUndefined(parent = wofRecords[parent_id])) { - parents.push(parent); - parent_id = parent.parent_id; - } - - return [parents.filter(hasName)]; - - }; - -}; - /* This function finds all the WOF records associated with a hierarchy diff --git a/test/hierarchyFinderTest.js b/test/hierarchyFinderTest.js index 476ea50c..875f53c7 100644 --- a/test/hierarchyFinderTest.js +++ b/test/hierarchyFinderTest.js @@ -2,81 +2,6 @@ var tape = require('tape'); var hierarchyFinder = require('../src/hierarchyFinder'); -tape('parent_id_walker tests', function(test) { - test.test('all parents up to undefined should be included', function(t) { - // records are limited to just the fields needed to operate - var wofRecords = { - 1: { - name: 'name 1', - parent_id: undefined - }, - 2: { - name: 'name 2', - parent_id: 1 - }, - 3: { - name: 'name 3', - parent_id: 2 - }, - 4: { - id: 4, - name: 'name 4', - parent_id: 3 - } - }; - - // seed the parent_id_walker with wofRecords - var parent_id_walker = hierarchyFinder.parent_id_walker(wofRecords); - - var hierarchies = parent_id_walker(wofRecords['4']); - - t.deepEqual(hierarchies, [[ - wofRecords['4'], - wofRecords['3'], - wofRecords['2'], - wofRecords['1'] - ]]); - t.end(); - - }); - - test.test('parents without names should not be included', function(t) { - // records are limited to just the fields needed to operate - var wofRecords = { - 1: { - name: 'name 1', - parent_id: undefined - }, - 2: { - name: undefined, - parent_id: 1 - }, - 3: { - name: undefined, - parent_id: 2 - }, - 4: { - id: 4, - name: 'name 4', - parent_id: 3 - } - }; - - // seed the parent_id_walker with wofRecords - var parent_id_walker = hierarchyFinder.parent_id_walker(wofRecords); - - var hierarchies = parent_id_walker(wofRecords['4']); - - t.deepEqual(hierarchies, [[ - wofRecords['4'], - wofRecords['1'] - ]]); - t.end(); - - }); - -}); - tape('hierarchies_walker tests', function(test) { test.test('all hierarchies should be returned', function(t) { // records are limited to just the fields needed to operate From bfba10bc78f94fa9f3000b4261003fd8e2b6b608 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 17 Oct 2016 16:04:49 -0400 Subject: [PATCH 2/5] removed last references to parent_id --- src/components/extractFields.js | 1 - test/components/extractFieldsTest.js | 39 +++------------------------- test/peliasDocGeneratorsTest.js | 2 -- test/readStreamTest.js | 5 ---- 4 files changed, 4 insertions(+), 43 deletions(-) diff --git a/src/components/extractFields.js b/src/components/extractFields.js index bff87643..82964651 100644 --- a/src/components/extractFields.js +++ b/src/components/extractFields.js @@ -67,7 +67,6 @@ module.exports.create = function map_fields_stream() { name: getName(json_object.properties), abbreviation: json_object.properties['wof:abbreviation'], place_type: json_object.properties['wof:placetype'], - parent_id: json_object.properties['wof:parent_id'], lat: getLat(json_object.properties), lon: getLon(json_object.properties), bounding_box: getBoundingBox(json_object.properties), diff --git a/test/components/extractFieldsTest.js b/test/components/extractFieldsTest.js index acbe31eb..53321de9 100644 --- a/test/components/extractFieldsTest.js +++ b/test/components/extractFieldsTest.js @@ -30,16 +30,15 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'name 1', 'wof:placetype': 'place type 1', - 'wof:parent_id': 'parent id 1', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'geom:bbox': '-13.691314,49.909613,1.771169,60.847886', 'wof:hierarchy': [ { - 'parent_id': 12345 + 'country_id': 12345 }, { - 'parent_id': 23456 + 'country_id': 23456 } ], 'iso:country': 'YZ', @@ -61,7 +60,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'name 1', place_type: 'place type 1', - parent_id: 'parent id 1', lat: 12.121212, lon: 21.212121, iso2: 'YZ', @@ -71,10 +69,10 @@ tape('readStreamComponents', function(test) { bounding_box: '-13.691314,49.909613,1.771169,60.847886', hierarchies: [ { - 'parent_id': 12345 + 'country_id': 12345 }, { - 'parent_id': 23456 + 'country_id': 23456 } ] }, @@ -82,7 +80,6 @@ tape('readStreamComponents', function(test) { id: 23456, name: undefined, place_type: undefined, - parent_id: undefined, lat: undefined, lon: undefined, iso2: undefined, @@ -108,7 +105,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'name 1', 'wof:placetype': 'place type 1', - 'wof:parent_id': 'parent id 1', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'geom:bbox': '-13.691314,49.909613,1.771169,60.847886', @@ -125,7 +121,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'name 1', place_type: 'place type 1', - parent_id: 'parent id 1', lat: 12.121212, lon: 21.212121, iso2: 'YZ', @@ -151,7 +146,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'name 1', 'wof:placetype': 'place type 1', - 'wof:parent_id': 'parent id 1', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'geom:bbox': '-13.691314,49.909613,1.771169,60.847886', @@ -167,7 +161,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'name 1', place_type: 'place type 1', - parent_id: 'parent id 1', lat: 12.121212, lon: 21.212121, iso2: 'YZ', @@ -193,7 +186,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'name 1', 'wof:placetype': 'place type 1', - 'wof:parent_id': 'parent id 1', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'geom:bbox': '-13.691314,49.909613,1.771169,60.847886', @@ -209,7 +201,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'name 1', place_type: 'place type 1', - parent_id: 'parent id 1', lat: 12.121212, lon: 21.212121, iso2: 'YZ', @@ -235,7 +226,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'name 1', 'wof:placetype': 'place type 1', - 'wof:parent_id': 'parent id 1', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'geom:bbox': '-13.691314,49.909613,1.771169,60.847886', @@ -251,7 +241,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'name 1', place_type: 'place type 1', - parent_id: 'parent id 1', lat: 12.121212, lon: 21.212121, iso2: 'YZ', @@ -277,7 +266,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'name 1', 'wof:placetype': 'place type 1', - 'wof:parent_id': 'parent id 1', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'geom:bbox': '-13.691314,49.909613,1.771169,60.847886', @@ -293,7 +281,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'name 1', place_type: 'place type 1', - parent_id: 'parent id 1', lat: 12.121212, lon: 21.212121, iso2: 'YZ', @@ -319,7 +306,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'name 1', 'wof:placetype': 'place type 1', - 'wof:parent_id': 'parent id 1', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'geom:bbox': '-13.691314,49.909613,1.771169,60.847886', @@ -334,7 +320,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'name 1', place_type: 'place type 1', - parent_id: 'parent id 1', lat: 12.121212, lon: 21.212121, iso2: 'YZ', @@ -360,7 +345,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'name 1', 'wof:placetype': 'place type 1', - 'wof:parent_id': 'parent id 1', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'geom:bbox': '-13.691314,49.909613,1.771169,60.847886', @@ -375,7 +359,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'name 1', place_type: 'place type 1', - parent_id: 'parent id 1', lat: 12.121212, lon: 21.212121, iso2: 'YZ', @@ -401,7 +384,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'wof:name value', 'wof:placetype': 'county', - 'wof:parent_id': 'parent id', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'iso:country': 'US', @@ -415,7 +397,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'qs:a2_alt value', place_type: 'county', - parent_id: 'parent id', lat: 12.121212, lon: 21.212121, iso2: 'US', @@ -441,7 +422,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'wof:name value', 'wof:placetype': 'county', - 'wof:parent_id': 'parent id', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'iso:country': 'US' @@ -454,7 +434,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'wof:name value', place_type: 'county', - parent_id: 'parent id', lat: 12.121212, lon: 21.212121, iso2: 'US', @@ -480,7 +459,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'wof:name value', 'wof:placetype': 'county', - 'wof:parent_id': 'parent id', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'iso:country': 'not US', @@ -494,7 +472,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'wof:name value', place_type: 'county', - parent_id: 'parent id', lat: 12.121212, lon: 21.212121, iso2: 'not US', @@ -520,7 +497,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'wof:name value', 'wof:placetype': 'county', - 'wof:parent_id': 'parent id', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'lbl:latitude': 14.141414, @@ -536,7 +512,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'wof:name value', place_type: 'county', - parent_id: 'parent id', lat: 14.141414, lon: 23.232323, iso2: 'not US', @@ -561,7 +536,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'wof:name value', 'wof:placetype': 'county', - 'wof:parent_id': 'parent id', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'geom:bbox': '-13.691314,49.909613,1.771169,60.847886', @@ -576,7 +550,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'wof:name value', place_type: 'county', - parent_id: 'parent id', lat: 12.121212, lon: 21.212121, iso2: undefined, @@ -601,7 +574,6 @@ tape('readStreamComponents', function(test) { properties: { 'wof:name': 'wof:name value', 'wof:placetype': 'county', - 'wof:parent_id': 'parent id', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'geom:bbox': '-13.691314,49.909613,1.771169,60.847886', @@ -616,7 +588,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'wof:name value', place_type: 'county', - parent_id: 'parent id', lat: 12.121212, lon: 21.212121, iso2: undefined, @@ -642,7 +613,6 @@ tape('readStreamComponents', function(test) { 'wof:name': 'wof:name value', 'wof:label': 'wof:label value', 'wof:placetype': 'county', - 'wof:parent_id': 'parent id', 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'lbl:bbox': '' @@ -655,7 +625,6 @@ tape('readStreamComponents', function(test) { id: 12345, name: 'wof:label value', place_type: 'county', - parent_id: 'parent id', lat: 12.121212, lon: 21.212121, iso2: undefined, diff --git a/test/peliasDocGeneratorsTest.js b/test/peliasDocGeneratorsTest.js index 07d176aa..53b7bc30 100644 --- a/test/peliasDocGeneratorsTest.js +++ b/test/peliasDocGeneratorsTest.js @@ -150,7 +150,6 @@ tape('create', function(test) { name: 'name 1', lat: 12.121212, lon: 21.212121, - parent_id: undefined, place_type: 'continent' } }; @@ -185,7 +184,6 @@ tape('create', function(test) { id: 1, lat: 12.121212, lon: 21.212121, - parent_id: undefined, place_type: 'continent', bounding_box: '-13.691314,49.909613,1.771169,60.847886', hierarchy: undefined diff --git a/test/readStreamTest.js b/test/readStreamTest.js index 5a3606ae..d8662b94 100644 --- a/test/readStreamTest.js +++ b/test/readStreamTest.js @@ -24,7 +24,6 @@ tape('readStream', function(test) { properties: { 'wof:name': 'name 1', 'wof:placetype': 'place type 1', - 'wof:parent_id': 2, 'geom:latitude': 12.121212, 'geom:longitude': 21.212121, 'iso:country': 'YZ', @@ -43,7 +42,6 @@ tape('readStream', function(test) { properties: { 'wof:name': 'name 2', 'wof:placetype': 'place type 2', - 'wof:parent_id': 3, 'geom:latitude': 13.131313, 'geom:longitude': 31.313131, 'iso:country': 'XZ', @@ -62,7 +60,6 @@ tape('readStream', function(test) { properties: { 'wof:name': 'name 3', 'wof:placetype': 'place type 3', - 'wof:parent_id': 4, 'geom:latitude': 14.141414, 'geom:longitude': 41.414141, 'geom:bbox': '-24.539906,34.815009,69.033946,81.85871' @@ -86,7 +83,6 @@ tape('readStream', function(test) { id: 1234567, name: 'name 1', place_type: 'place type 1', - parent_id: 2, lat: 12.121212, lon: 21.212121, iso2: 'YZ', @@ -101,7 +97,6 @@ tape('readStream', function(test) { id: 12345678, name: 'name 2', place_type: 'place type 2', - parent_id: 3, lat: 13.131313, lon: 31.313131, iso2: 'XZ', From 7f878ee236767e16085eec096ff304245a48d3bf Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 17 Oct 2016 16:11:26 -0400 Subject: [PATCH 3/5] broke out separate test for clarity --- test/components/extractFieldsTest.js | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/test/components/extractFieldsTest.js b/test/components/extractFieldsTest.js index 53321de9..156d030d 100644 --- a/test/components/extractFieldsTest.js +++ b/test/components/extractFieldsTest.js @@ -48,10 +48,6 @@ tape('readStreamComponents', function(test) { ignoreField3: 'ignoreField3', ignoreField4: 'ignoreField4', } - }, - { - id: 23456, - properties: {} } ]; @@ -75,7 +71,25 @@ tape('readStreamComponents', function(test) { 'country_id': 23456 } ] - }, + } + ]; + + test_stream(input, extractFields.create(), function(err, actual) { + t.deepEqual(actual, expected, 'stream should contain only objects with id and properties'); + t.end(); + }); + + }); + + test.test('missing fields should return undefined and empty array for hierarchies', function(t) { + var input = [ + { + id: 23456, + properties: {} + } + ]; + + var expected = [ { id: 23456, name: undefined, From b9001549608db54f916a1a08feed8d3d2c4bd505 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 17 Oct 2016 16:17:49 -0400 Subject: [PATCH 4/5] reduced `hierarchyFinder` to a single function export --- import.js | 3 +-- src/hierarchyFinder.js | 2 +- test/hierarchyFinderTest.js | 15 +++------------ 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/import.js b/import.js index 7993d279..b0c0fc8c 100644 --- a/import.js +++ b/import.js @@ -39,8 +39,7 @@ if (peliasConfig.imports.whosonfirst.importVenues) { var readStream = readStreamModule.create(directory, bundlesToImport, wofAdminRecords); // how to convert WOF records to Pelias Documents -var documentGenerator = peliasDocGenerators.create( - hierarchyFinder.hierarchies_walker(wofAdminRecords)); +var documentGenerator = peliasDocGenerators.create(hierarchyFinder(wofAdminRecords)); // the final destination of Pelias Documents var dbClientStream = peliasDbclient(); diff --git a/src/hierarchyFinder.js b/src/hierarchyFinder.js index ee0f675e..b88adc08 100644 --- a/src/hierarchyFinder.js +++ b/src/hierarchyFinder.js @@ -39,7 +39,7 @@ function resolveHierarchy(wofRecords, hierarchy) { wofRecord can have multiple hierarchies, so resolve them by looking up the referenced wofRecord in the big collection of wofRecords. */ -module.exports.hierarchies_walker = function(wofRecords) { +module.exports = function(wofRecords) { return function(wofRecord) { return wofRecord.hierarchies.reduce(function(resolvedHierarchies, hierarchy) { resolvedHierarchies.push(resolveHierarchy(wofRecords, hierarchy)); diff --git a/test/hierarchyFinderTest.js b/test/hierarchyFinderTest.js index 875f53c7..363034ca 100644 --- a/test/hierarchyFinderTest.js +++ b/test/hierarchyFinderTest.js @@ -32,10 +32,7 @@ tape('hierarchies_walker tests', function(test) { } }; - // seed the hierarchies_walker with wofRecords - var hierarchies_walker = hierarchyFinder.hierarchies_walker(wofRecords); - - var hierarchies = hierarchies_walker(wofRecords['4']); + var hierarchies = hierarchyFinder(wofRecords)(wofRecords['4']); t.deepEqual(hierarchies, [ [ @@ -78,10 +75,7 @@ tape('hierarchies_walker tests', function(test) { } }; - // seed the hierarchies_walker with wofRecords - var hierarchies_walker = hierarchyFinder.hierarchies_walker(wofRecords); - - var hierarchies = hierarchies_walker(wofRecords['4']); + var hierarchies = hierarchyFinder(wofRecords)(wofRecords['4']); t.deepEqual(hierarchies, [[ wofRecords['4'], @@ -112,10 +106,7 @@ tape('hierarchies_walker tests', function(test) { } }; - // seed the hierarchies_walker with wofRecords - var hierarchies_walker = hierarchyFinder.hierarchies_walker(wofRecords); - - var hierarchy = hierarchies_walker(wofRecords['4']); + var hierarchy = hierarchyFinder(wofRecords)(wofRecords['4']); t.deepEqual(hierarchy, [ [ From 7e0e59d17e22e2255f76e4daa109279c1f5f60fe Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 17 Oct 2016 16:19:16 -0400 Subject: [PATCH 5/5] rename test --- test/hierarchyFinderTest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hierarchyFinderTest.js b/test/hierarchyFinderTest.js index 363034ca..ea4338ba 100644 --- a/test/hierarchyFinderTest.js +++ b/test/hierarchyFinderTest.js @@ -2,7 +2,7 @@ var tape = require('tape'); var hierarchyFinder = require('../src/hierarchyFinder'); -tape('hierarchies_walker tests', function(test) { +tape('tests for looking up hierarchies', function(test) { test.test('all hierarchies should be returned', function(t) { // records are limited to just the fields needed to operate var wofRecords = {