From e1eeb25776123cf06d7b7fa5f0fe4a44fe88575e Mon Sep 17 00:00:00 2001
From: Diana Shkolnikov <dianashk@gmail.com>
Date: Thu, 6 Apr 2017 18:41:16 -0400
Subject: [PATCH 1/2] fix: geonames ids should not be misrepresented in
 hiearchy

---
 middleware/normalizeParentIds.js           |  17 +++-
 test/unit/middleware/normalizeParentIds.js | 106 +++++++++++++++++++++
 2 files changed, 120 insertions(+), 3 deletions(-)

diff --git a/middleware/normalizeParentIds.js b/middleware/normalizeParentIds.js
index ae22a3c6..60c1fa04 100644
--- a/middleware/normalizeParentIds.js
+++ b/middleware/normalizeParentIds.js
@@ -15,6 +15,8 @@ function setup() {
       return next();
     }
 
+    console.log(JSON.stringify(res.data, null, 2));
+    
     res.data = res.data.map(normalizeParentIds);
 
     next();
@@ -32,7 +34,16 @@ function normalizeParentIds(place) {
   if (place) {
     placeTypes.forEach(function (placeType) {
       if (place[placeType] && place[placeType].length > 0 && place[placeType][0]) {
-        place[placeType + '_gid'] = [ makeNewId(placeType, place[placeType + '_gid']) ];
+        var source = 'whosonfirst';
+
+        // looking forward to the day we can remove all geonames specific hacks, but until then...
+        // geonames sometimes has its own ids in the parent hierarchy, so it's dangerous to assume that 
+        // it's always WOF ids and hardcode to that
+        if (place.source === 'geonames' && place.source_id === place[placeType + '_gid'][0]) {
+          source = place.source;
+        }
+        
+        place[placeType + '_gid'] = [ makeNewId(source, placeType, place[placeType + '_gid']) ];
       }
     });
   }
@@ -48,12 +59,12 @@ function normalizeParentIds(place) {
  * @param {number} id
  * @return {string}
  */
-function makeNewId(placeType, id) {
+function makeNewId(source, placeType, id) {
   if (!id) {
     return;
   }
 
-  var doc = new Document('whosonfirst', placeType, id);
+  var doc = new Document(source, placeType, id);
   return doc.getGid();
 }
 
diff --git a/test/unit/middleware/normalizeParentIds.js b/test/unit/middleware/normalizeParentIds.js
index 472df0e3..8388f67d 100644
--- a/test/unit/middleware/normalizeParentIds.js
+++ b/test/unit/middleware/normalizeParentIds.js
@@ -77,6 +77,112 @@ module.exports.tests.interface = function(test, common) {
     });
 
   });
+
+  test('Geonames ids do not override parent hierarchy with WOF equivalents', function(t) {
+
+    var input = {
+      data: [{
+        'parent': {
+          'country_id': [ '85633793' ],
+          'country': [ 'United States' ],
+          'country_a': [ 'USA' ],
+          'region_id': [ '85688579' ],
+          'region': [ 'Mississippi' ],
+          'region_a': [ 'MS' ]
+        },
+        'source': 'geonames',
+        'source_id': '4436296',
+        'layer': 'region',
+        'country': [ 'United States' ],
+        'country_a': [ 'USA' ],
+        'country_gid': [ '85633793' ],
+        'region': [ 'Mississippi' ],
+        'region_a': [ 'MS' ],
+        'region_gid': [ '85688579' ]
+      }]
+    };
+
+    var expected = {
+      data: [{
+        'parent': {
+          'country_id': [ '85633793' ],
+          'country': [ 'United States' ],
+          'country_a': [ 'USA' ],
+          'region_id': [ '85688579' ],
+          'region': [ 'Mississippi' ],
+          'region_a': [ 'MS' ]
+        },
+        'layer': 'region',
+        'source': 'geonames',
+        'source_id': '4436296',
+        'country': ['United States'],
+        'country_gid': ['whosonfirst:country:85633793'],
+        'country_a': ['USA'],
+        'region': ['Mississippi'],
+        'region_gid': ['whosonfirst:region:85688579'],
+        'region_a': ['MS']
+      }]
+    };
+
+    normalizer({}, input, function () {
+      t.deepEqual(input, expected);
+      t.end();
+    });
+
+  });
+
+  test('Geonames ids do not override parent hierarchy with WOF equivalents', function(t) {
+
+    var input = {
+      data: [{
+        'parent': {
+          'country_id': [ '85633793' ],
+          'country': [ 'United States' ],
+          'country_a': ['USA'],
+          'region_id': [ '4436296' ],
+          'region': [ 'Mississippi' ],
+          'region_a': [ 'MS' ]
+        },
+        'source': 'geonames',
+        'source_id': '4436296',
+        'layer': 'region',
+        'country': [ 'United States' ],
+        'country_a': [ 'USA' ],
+        'country_gid': ['85633793'],
+        'region': [ 'Mississippi' ],
+        'region_a': [ 'MS' ],
+        'region_gid': [ '4436296' ]
+      }]
+    };
+
+    var expected = {
+      data: [{
+        'parent': {
+          'country_id': [ '85633793' ],
+          'country': [ 'United States' ],
+          'country_a': [ 'USA' ],
+          'region_id': [ '4436296' ],
+          'region': [ 'Mississippi' ],
+          'region_a': [ 'MS' ]
+        },
+        'layer': 'region',
+        'source': 'geonames',
+        'source_id': '4436296',
+        'country': ['United States'],
+        'country_gid': ['whosonfirst:country:85633793'],
+        'country_a': ['USA'],
+        'region': ['Mississippi'],
+        'region_gid': ['geonames:region:4436296'],
+        'region_a': ['MS']
+      }]
+    };
+
+    normalizer({}, input, function () {
+      t.deepEqual(input, expected);
+      t.end();
+    });
+
+  });
 };
 
 module.exports.all = function (tape, common) {

From 798363da1e200323cd22e41648996e12841002a0 Mon Sep 17 00:00:00 2001
From: Diana Shkolnikov <diana@mapzen.com>
Date: Fri, 7 Apr 2017 10:57:43 -0400
Subject: [PATCH 2/2] remove unwanted log statement

---
 middleware/normalizeParentIds.js | 2 --
 1 file changed, 2 deletions(-)

diff --git a/middleware/normalizeParentIds.js b/middleware/normalizeParentIds.js
index 60c1fa04..22fc43f8 100644
--- a/middleware/normalizeParentIds.js
+++ b/middleware/normalizeParentIds.js
@@ -15,8 +15,6 @@ function setup() {
       return next();
     }
 
-    console.log(JSON.stringify(res.data, null, 2));
-    
     res.data = res.data.map(normalizeParentIds);
 
     next();