From a2589cfeb7b410e369e9683acf9d4f1d2495035b Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 2 Jun 2017 13:21:35 -0400
Subject: [PATCH 01/10] switched to _.isEmpty check

---
 controller/predicates/is_coarse_reverse.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/controller/predicates/is_coarse_reverse.js b/controller/predicates/is_coarse_reverse.js
index c8a292dd..e02f90c5 100644
--- a/controller/predicates/is_coarse_reverse.js
+++ b/controller/predicates/is_coarse_reverse.js
@@ -5,5 +5,5 @@ const non_coarse_layers = ['address', 'street', 'venue'];
 module.exports = (req, res) => {
   // returns true if layers is undefined, empty, or contains 'address', 'street', or 'venue'
   return !_.isEmpty(req.clean.layers) &&
-          _.intersection(req.clean.layers, non_coarse_layers).length === 0;
+          _.isEmpty(_.intersection(req.clean.layers, non_coarse_layers));
 };

From 6e693db87e9b03b0b1a207428fc712c752a32e2f Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 2 Jun 2017 13:47:45 -0400
Subject: [PATCH 02/10] added support for non-coarse req.clean.layers

---
 controller/coarse_reverse.js           |   4 +-
 routes/v1.js                           |   5 +
 test/unit/controller/coarse_reverse.js | 182 +++++++++++++++++++++++++
 3 files changed, 189 insertions(+), 2 deletions(-)

diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js
index 07a370d7..ea9791cb 100644
--- a/controller/coarse_reverse.js
+++ b/controller/coarse_reverse.js
@@ -22,7 +22,7 @@ function getMostGranularLayer(results) {
 }
 
 function hasResultsAtRequestedLayers(results, layers) {
-  return _.intersection(layers, Object.keys(results)).length > 0;
+  return _.isEmpty(layers) || !_.isEmpty(_.intersection(layers, Object.keys(results)));
 }
 
 function synthesizeDoc(results) {
@@ -110,7 +110,7 @@ function setup(service, should_execute) {
       res.meta = {};
       res.data = [];
       // synthesize a doc from results if there's a result at the request layer(s)
-      if (hasResultsAtRequestedLayers(results, req.clean.layers)) {
+      if (hasResultsAtRequestedLayers(results, _.without(req.clean.layers, 'venue', 'address', 'street'))) {
         res.data.push(synthesizeDoc(results));
       }
 
diff --git a/routes/v1.js b/routes/v1.js
index da4936be..fc8fe556 100644
--- a/routes/v1.js
+++ b/routes/v1.js
@@ -101,6 +101,10 @@ function addRoutes(app, peliasConfig) {
     not(hasRequestErrors), isPipServiceEnabled, isCoarseReverse
   );
 
+  const coarse_reverse_fallback_should_execute = all(
+    isPipServiceEnabled, not(hasRequestErrors), not(hasResponseData), not(isCoarseReverse)
+  );
+
   const placeholderShouldExecute = all(
     not(hasResponseDataOrRequestErrors), isPlaceholderServiceEnabled, isAdminOnlyAnalysis
   );
@@ -198,6 +202,7 @@ function addRoutes(app, peliasConfig) {
       middleware.calcSize(),
       controllers.coarse_reverse(pipService, coarse_reverse_should_execute),
       controllers.search(peliasConfig.api, esclient, queries.reverse, original_reverse_should_execute),
+      controllers.coarse_reverse(pipService, coarse_reverse_fallback_should_execute),
       postProc.distances('point.'),
       // reverse confidence scoring depends on distance from origin
       //  so it must be calculated first
diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js
index c37c76e0..1f609840 100644
--- a/test/unit/controller/coarse_reverse.js
+++ b/test/unit/controller/coarse_reverse.js
@@ -465,6 +465,188 @@ module.exports.tests.success_conditions = (test, common) => {
 
   });
 
+  test('layers specifying only venue, address, or street should not exclude coarse results', (t) => {
+    // this test is used to test coarse reverse fallback for when non-coarse reverse
+    //  was requested but no non-coarse results were found
+    const non_coarse_layers = ['venue', 'address', 'street'];
+    const tests_per_non_coarse_layer = 4;
+
+    // by plan'ing the number of tests, we can verify that next() was called w/o
+    //  additional bookkeeping
+    t.plan(non_coarse_layers.length * tests_per_non_coarse_layer);
+
+    non_coarse_layers.forEach((non_coarse_layer) => {
+      const service = (point, do_not_track, callback) => {
+        t.equals(do_not_track, 'do_not_track value');
+        const results = {
+          neighbourhood: [
+            {
+              id: 10,
+              name: 'neighbourhood name',
+              abbr: 'neighbourhood abbr'
+            }
+          ]
+        };
+
+        callback(undefined, results);
+      };
+
+      const logger = require('pelias-mock-logger')();
+
+      const should_execute = () => { return true; };
+      const controller = proxyquire('../../../controller/coarse_reverse', {
+        'pelias-logger': logger,
+        '../helper/logging': {
+          isDNT: () => {
+            return 'do_not_track value';
+          }
+        }
+      })(service, should_execute);
+
+      const req = {
+        clean: {
+          layers: [non_coarse_layer],
+          point: {
+            lat: 12.121212,
+            lon: 21.212121
+          }
+        }
+      };
+
+      const res = { };
+
+      // verify that next was called
+      const next = () => {
+        t.pass('next() should have been called');
+      };
+
+      controller(req, res, next);
+
+      const expected = {
+        meta: {},
+        data: [
+          {
+            _id: '10',
+            _type: 'neighbourhood',
+            layer: 'neighbourhood',
+            source: 'whosonfirst',
+            source_id: '10',
+            name: {
+              'default': 'neighbourhood name'
+            },
+            phrase: {
+              'default': 'neighbourhood name'
+            },
+            parent: {
+              neighbourhood: ['neighbourhood name'],
+              neighbourhood_id: ['10'],
+              neighbourhood_a: ['neighbourhood abbr']
+            }
+          }
+        ]
+      };
+
+      t.deepEquals(res, expected);
+
+      t.notOk(logger.hasErrorMessages());
+
+    });
+
+    t.end();
+
+  });
+
+  test('layers specifying venue, address, or street AND coarse layer should not exclude coarse results', (t) => {
+    // this test is used to test coarse reverse fallback for when non-coarse reverse
+    //  was requested but no non-coarse results were found
+    const non_coarse_layers = ['venue', 'address', 'street'];
+    const tests_per_non_coarse_layer = 4;
+
+    // by plan'ing the number of tests, we can verify that next() was called w/o
+    //  additional bookkeeping
+    t.plan(non_coarse_layers.length * tests_per_non_coarse_layer);
+
+    non_coarse_layers.forEach((non_coarse_layer) => {
+      const service = (point, do_not_track, callback) => {
+        t.equals(do_not_track, 'do_not_track value');
+        const results = {
+          neighbourhood: [
+            {
+              id: 10,
+              name: 'neighbourhood name',
+              abbr: 'neighbourhood abbr'
+            }
+          ]
+        };
+
+        callback(undefined, results);
+      };
+
+      const logger = require('pelias-mock-logger')();
+
+      const should_execute = () => { return true; };
+      const controller = proxyquire('../../../controller/coarse_reverse', {
+        'pelias-logger': logger,
+        '../helper/logging': {
+          isDNT: () => {
+            return 'do_not_track value';
+          }
+        }
+      })(service, should_execute);
+
+      const req = {
+        clean: {
+          layers: [non_coarse_layer, 'neighbourhood'],
+          point: {
+            lat: 12.121212,
+            lon: 21.212121
+          }
+        }
+      };
+
+      const res = { };
+
+      // verify that next was called
+      const next = () => {
+        t.pass('next() should have been called');
+      };
+
+      controller(req, res, next);
+
+      const expected = {
+        meta: {},
+        data: [
+          {
+            _id: '10',
+            _type: 'neighbourhood',
+            layer: 'neighbourhood',
+            source: 'whosonfirst',
+            source_id: '10',
+            name: {
+              'default': 'neighbourhood name'
+            },
+            phrase: {
+              'default': 'neighbourhood name'
+            },
+            parent: {
+              neighbourhood: ['neighbourhood name'],
+              neighbourhood_id: ['10'],
+              neighbourhood_a: ['neighbourhood abbr']
+            }
+          }
+        ]
+      };
+
+      t.deepEquals(res, expected);
+
+      t.notOk(logger.hasErrorMessages());
+
+    });
+
+    t.end();
+
+  });
+
 };
 
 module.exports.tests.failure_conditions = (test, common) => {

From ecf7a90f68cc897e812e8e253a68f8fd56909d31 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 2 Jun 2017 13:51:37 -0400
Subject: [PATCH 03/10] add test that req.clean.layers should be unmodified

---
 test/unit/controller/coarse_reverse.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js
index 1f609840..2a086b14 100644
--- a/test/unit/controller/coarse_reverse.js
+++ b/test/unit/controller/coarse_reverse.js
@@ -469,7 +469,7 @@ module.exports.tests.success_conditions = (test, common) => {
     // this test is used to test coarse reverse fallback for when non-coarse reverse
     //  was requested but no non-coarse results were found
     const non_coarse_layers = ['venue', 'address', 'street'];
-    const tests_per_non_coarse_layer = 4;
+    const tests_per_non_coarse_layer = 5;
 
     // by plan'ing the number of tests, we can verify that next() was called w/o
     //  additional bookkeeping
@@ -546,8 +546,8 @@ module.exports.tests.success_conditions = (test, common) => {
         ]
       };
 
+      t.deepEquals(req.clean.layers, [non_coarse_layer], 'req.clean.layers should be unmodified');
       t.deepEquals(res, expected);
-
       t.notOk(logger.hasErrorMessages());
 
     });
@@ -560,7 +560,7 @@ module.exports.tests.success_conditions = (test, common) => {
     // this test is used to test coarse reverse fallback for when non-coarse reverse
     //  was requested but no non-coarse results were found
     const non_coarse_layers = ['venue', 'address', 'street'];
-    const tests_per_non_coarse_layer = 4;
+    const tests_per_non_coarse_layer = 5;
 
     // by plan'ing the number of tests, we can verify that next() was called w/o
     //  additional bookkeeping
@@ -637,8 +637,8 @@ module.exports.tests.success_conditions = (test, common) => {
         ]
       };
 
+      t.deepEquals(req.clean.layers, [non_coarse_layer, 'neighbourhood'], 'req.clean.layers should be unmodified');
       t.deepEquals(res, expected);
-
       t.notOk(logger.hasErrorMessages());
 
     });

From 1d4318868bb594936d3a5d13dd579fb86d87f20c Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 2 Jun 2017 14:07:30 -0400
Subject: [PATCH 04/10] added helper for removed non-coarse layers

---
 controller/coarse_reverse.js | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js
index ea9791cb..d86a0951 100644
--- a/controller/coarse_reverse.js
+++ b/controller/coarse_reverse.js
@@ -21,6 +21,8 @@ function getMostGranularLayer(results) {
   });
 }
 
+//  removing non-coarse layers could leave effective_layers empty, so it's
+//  important to check for empty layers here
 function hasResultsAtRequestedLayers(results, layers) {
   return _.isEmpty(layers) || !_.isEmpty(_.intersection(layers, Object.keys(results)));
 }
@@ -83,6 +85,15 @@ function setup(service, should_execute) {
       return next();
     }
 
+    // because coarse reverse is called when non-coarse reverse didn't return
+    //  anything, don't consider non-coarse layers when filtering
+    const effective_layers = _.without(req.clean.layers, 'venue', 'address', 'street');
+
+    const centroid = {
+      lat: req.clean['point.lat'],
+      lon: req.clean['point.lon']
+    };
+
     service(req, (err, results) => {
       // if there's an error, log it and bail
       if (err) {
@@ -93,13 +104,13 @@ function setup(service, should_execute) {
 
       // find the finest granularity requested
       const finest_granularity_requested = granularities.findIndex((granularity) => {
-        return req.clean.layers.indexOf(granularity) !== -1;
+        return effective_layers.indexOf(granularity) !== -1;
       });
 
       logger.info(`[controller:coarse_reverse][queryType:pip][result_count:${Object.keys(results).length}]`);
 
       // now remove everything from the response that is more granular than the
-      // most granular layer requested.  that is, if req.clean.layers=['county'],
+      // most granular layer requested.  that is, if effective_layers=['county'],
       // remove neighbourhoods, localities, and localadmins
       Object.keys(results).forEach((layer) => {
         if (granularities.indexOf(layer) < finest_granularity_requested) {
@@ -110,7 +121,7 @@ function setup(service, should_execute) {
       res.meta = {};
       res.data = [];
       // synthesize a doc from results if there's a result at the request layer(s)
-      if (hasResultsAtRequestedLayers(results, _.without(req.clean.layers, 'venue', 'address', 'street'))) {
+      if (hasResultsAtRequestedLayers(results, effective_layers)) {
         res.data.push(synthesizeDoc(results));
       }
 

From 39e91d82338e65159ac806cc5ec3fa465729f613 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 2 Jun 2017 16:27:15 -0400
Subject: [PATCH 05/10] remove address/venue/street from requested layers when
 creating results

refactored for cleanliness and clarity
---
 controller/coarse_reverse.js           | 77 ++++++++++++++---------
 test/unit/controller/coarse_reverse.js | 86 ++++++++++++++++++++++++++
 2 files changed, 133 insertions(+), 30 deletions(-)

diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js
index d86a0951..90affb5f 100644
--- a/controller/coarse_reverse.js
+++ b/controller/coarse_reverse.js
@@ -2,7 +2,8 @@ const logger = require('pelias-logger').get('coarse_reverse');
 const _ = require('lodash');
 const Document = require('pelias-model').Document;
 
-const granularities = [
+// do not change order, other functionality depends on most-to-least granular order
+const coarse_granularities = [
   'neighbourhood',
   'borough',
   'locality',
@@ -15,24 +16,47 @@ const granularities = [
   'country'
 ];
 
-function getMostGranularLayer(results) {
-  return granularities.find((granularity) => {
-    return results.hasOwnProperty(granularity);
+// remove non-coarse layers and return what's left (or all if empty)
+function getEffectiveLayers(requested_layers) {
+  const non_coarse_layers_removed = _.without(requested_layers, 'venue', 'address', 'street');
+
+  // if resulting array is empty, use all coarse granularities
+  if (_.isEmpty(non_coarse_layers_removed)) {
+    return coarse_granularities;
+  }
+
+  // otherwise use requested layers with non-coarse layers removed
+  return non_coarse_layers_removed;
+
+}
+
+// drop from coarse_granularities until there's one that was requested
+// this depends on coarse_granularities being ordered
+function getApplicableRequestedLayers(requested_layers) {
+  return _.dropWhile(coarse_granularities, (coarse_granularity) => {
+    return !_.includes(requested_layers, coarse_granularity);
   });
 }
 
 //  removing non-coarse layers could leave effective_layers empty, so it's
 //  important to check for empty layers here
-function hasResultsAtRequestedLayers(results, layers) {
-  return _.isEmpty(layers) || !_.isEmpty(_.intersection(layers, Object.keys(results)));
+function hasResultsAtRequestedLayers(results, requested_layers) {
+  return !_.isEmpty(_.intersection(_.keys(results), requested_layers));
+}
+
+// get the most granular layer from the results by taking the head of the intersection
+// of coarse_granularities (which are ordered) and the result layers
+// ['neighbourhood', 'borough', 'locality'] - ['locality', 'borough'] = 'borough'
+// this depends on coarse_granularities being ordered
+function getMostGranularLayerOfResult(result_layers) {
+  return _.head(_.intersection(coarse_granularities, result_layers));
 }
 
+// create a model.Document from what's left, using the most granular
+// result available as the starting point
 function synthesizeDoc(results) {
-  // now create a model.Document from what's level, using the most granular
-  // result available as the starting point
-  // the immediately above cannot be re-used since county may be the most
-  // granular layer requested but the results may start at region (no county found)
-  const most_granular_layer = getMostGranularLayer(results);
+  // find the most granular layer to use as the document layer
+  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());
@@ -86,8 +110,8 @@ function setup(service, should_execute) {
     }
 
     // because coarse reverse is called when non-coarse reverse didn't return
-    //  anything, don't consider non-coarse layers when filtering
-    const effective_layers = _.without(req.clean.layers, 'venue', 'address', 'street');
+    //  anything, treat requested layers as if it didn't contain non-coarse layers
+    const effective_layers = getEffectiveLayers(req.clean.layers);
 
     const centroid = {
       lat: req.clean['point.lat'],
@@ -102,27 +126,20 @@ function setup(service, should_execute) {
         return next();
       }
 
-      // find the finest granularity requested
-      const finest_granularity_requested = granularities.findIndex((granularity) => {
-        return effective_layers.indexOf(granularity) !== -1;
-      });
+      // log how many results there were
+      logger.info(`[controller:coarse_reverse][queryType:pip][result_count:${_.size(results)}]`);
 
-      logger.info(`[controller:coarse_reverse][queryType:pip][result_count:${Object.keys(results).length}]`);
-
-      // now remove everything from the response that is more granular than the
-      // most granular layer requested.  that is, if effective_layers=['county'],
-      // remove neighbourhoods, localities, and localadmins
-      Object.keys(results).forEach((layer) => {
-        if (granularities.indexOf(layer) < finest_granularity_requested) {
-          delete results[layer];
-        }
-      });
+      // now keep everything from the response that is equal to or less granular
+      // than the most granular layer requested.  that is, if effective_layers=['county'],
+      // remove neighbourhoods, boroughs, localities, localadmins
+      const applicable_results = _.pick(results, getApplicableRequestedLayers(effective_layers));
 
       res.meta = {};
       res.data = [];
-      // synthesize a doc from results if there's a result at the request layer(s)
-      if (hasResultsAtRequestedLayers(results, effective_layers)) {
-        res.data.push(synthesizeDoc(results));
+
+      // 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));
       }
 
       return next();
diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js
index 2a086b14..e44f73f1 100644
--- a/test/unit/controller/coarse_reverse.js
+++ b/test/unit/controller/coarse_reverse.js
@@ -465,6 +465,92 @@ module.exports.tests.success_conditions = (test, common) => {
 
   });
 
+  test('no requested layers should use everything', (t) => {
+    // this test is used to test coarse reverse fallback for when non-coarse reverse
+    //  was requested but no non-coarse results were found
+
+    // by plan'ing the number of tests, we can verify that next() was called w/o
+    //  additional bookkeeping
+    t.plan(5);
+
+    const service = (point, do_not_track, callback) => {
+      t.equals(do_not_track, 'do_not_track value');
+      const results = {
+        neighbourhood: [
+          {
+            id: 10,
+            name: 'neighbourhood name',
+            abbr: 'neighbourhood abbr'
+          }
+        ]
+      };
+
+      callback(undefined, results);
+    };
+
+    const logger = require('pelias-mock-logger')();
+
+    const should_execute = () => { return true; };
+    const controller = proxyquire('../../../controller/coarse_reverse', {
+      'pelias-logger': logger,
+      '../helper/logging': {
+        isDNT: () => {
+          return 'do_not_track value';
+        }
+      }
+    })(service, should_execute);
+
+    const req = {
+      clean: {
+        layers: [],
+        point: {
+          lat: 12.121212,
+          lon: 21.212121
+        }
+      }
+    };
+
+    const res = { };
+
+    // verify that next was called
+    const next = () => {
+      t.pass('next() should have been called');
+    };
+
+    controller(req, res, next);
+
+    const expected = {
+      meta: {},
+      data: [
+        {
+          _id: '10',
+          _type: 'neighbourhood',
+          layer: 'neighbourhood',
+          source: 'whosonfirst',
+          source_id: '10',
+          name: {
+            'default': 'neighbourhood name'
+          },
+          phrase: {
+            'default': 'neighbourhood name'
+          },
+          parent: {
+            neighbourhood: ['neighbourhood name'],
+            neighbourhood_id: ['10'],
+            neighbourhood_a: ['neighbourhood abbr']
+          }
+        }
+      ]
+    };
+
+    t.deepEquals(req.clean.layers, [], 'req.clean.layers should be unmodified');
+    t.deepEquals(res, expected);
+    t.notOk(logger.hasErrorMessages());
+
+    t.end();
+
+  });
+
   test('layers specifying only venue, address, or street should not exclude coarse results', (t) => {
     // this test is used to test coarse reverse fallback for when non-coarse reverse
     //  was requested but no non-coarse results were found

From 3764b928c5ccaaed84f5d57b74a6c27564c1c744 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 2 Jun 2017 16:36:48 -0400
Subject: [PATCH 06/10] reordered predicates, added comments

---
 routes/v1.js | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/routes/v1.js b/routes/v1.js
index fc8fe556..80c3d9b5 100644
--- a/routes/v1.js
+++ b/routes/v1.js
@@ -97,12 +97,14 @@ function addRoutes(app, peliasConfig) {
   const placeholderService = serviceWrapper(placeholderConfiguration);
   const isPlaceholderServiceEnabled = _.constant(placeholderConfiguration.isEnabled());
 
+  // use coarse reverse when requested layers are all coarse
   const coarse_reverse_should_execute = all(
-    not(hasRequestErrors), isPipServiceEnabled, isCoarseReverse
+    isPipServiceEnabled, isCoarseReverse, not(hasRequestErrors)
   );
 
+  // fallback to coarse reverse when regular reverse didn't return anything
   const coarse_reverse_fallback_should_execute = all(
-    isPipServiceEnabled, not(hasRequestErrors), not(hasResponseData), not(isCoarseReverse)
+    isPipServiceEnabled, not(isCoarseReverse), not(hasRequestErrors), not(hasResponseData)
   );
 
   const placeholderShouldExecute = all(

From f721d68e9ca30b1ec2d08843ce286645951d369e Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 2 Jun 2017 16:40:11 -0400
Subject: [PATCH 07/10] removed alpha3 set

---
 controller/coarse_reverse.js           | 4 ----
 test/unit/controller/coarse_reverse.js | 1 -
 2 files changed, 5 deletions(-)

diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js
index 90affb5f..169925b4 100644
--- a/controller/coarse_reverse.js
+++ b/controller/coarse_reverse.js
@@ -81,10 +81,6 @@ function synthesizeDoc(results) {
 
   }
 
-  if (_.has(results, 'country[0].abbr')) {
-    doc.setAlpha3(results.country[0].abbr);
-  }
-
   // assign the administrative hierarchy
   Object.keys(results).forEach((layer) => {
     if (results[layer][0].hasOwnProperty('abbr')) {
diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js
index e44f73f1..8fef004c 100644
--- a/test/unit/controller/coarse_reverse.js
+++ b/test/unit/controller/coarse_reverse.js
@@ -211,7 +211,6 @@ module.exports.tests.success_conditions = (test, common) => {
             country_id: ['100'],
             country_a: ['xyz']
           },
-          alpha3: 'XYZ',
           center_point: {
             lat: 12.121212,
             lon: 21.212121

From d67830ca485985dc875ab6296fda44fb155fa151 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 2 Jun 2017 16:44:54 -0400
Subject: [PATCH 08/10] removed unneeded conditional for abbreviation set

---
 controller/coarse_reverse.js           | 7 +------
 test/unit/controller/coarse_reverse.js | 3 +--
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js
index 169925b4..0d64855f 100644
--- a/controller/coarse_reverse.js
+++ b/controller/coarse_reverse.js
@@ -83,12 +83,7 @@ function synthesizeDoc(results) {
 
   // assign the administrative hierarchy
   Object.keys(results).forEach((layer) => {
-    if (results[layer][0].hasOwnProperty('abbr')) {
-      doc.addParent(layer, results[layer][0].name, results[layer][0].id.toString(), results[layer][0].abbr);
-    } else {
-      doc.addParent(layer, results[layer][0].name, results[layer][0].id.toString());
-    }
-
+    doc.addParent(layer, results[layer][0].name, results[layer][0].id.toString(), results[layer][0].abbr);
   });
 
   const esDoc = doc.toESDocument();
diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js
index 8fef004c..56f4e129 100644
--- a/test/unit/controller/coarse_reverse.js
+++ b/test/unit/controller/coarse_reverse.js
@@ -237,7 +237,6 @@ module.exports.tests.success_conditions = (test, common) => {
           {
             id: 10,
             name: 'neighbourhood name',
-            abbr: 'neighbourhood abbr',
             centroid: {
               lat: 12.121212,
               lon: 21.212121
@@ -291,7 +290,7 @@ module.exports.tests.success_conditions = (test, common) => {
           parent: {
             neighbourhood: ['neighbourhood name'],
             neighbourhood_id: ['10'],
-            neighbourhood_a: ['neighbourhood abbr']
+            neighbourhood_a: [null]
           },
           center_point: {
             lat: 12.121212,

From 788a8c8069c85187ecea3b7f7ed48b44b877bed5 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 2 Jun 2017 16:55:17 -0400
Subject: [PATCH 09/10] refactored for lodash because its siren song is so
 sweet

---
 controller/coarse_reverse.js | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js
index 0d64855f..bb974940 100644
--- a/controller/coarse_reverse.js
+++ b/controller/coarse_reverse.js
@@ -18,6 +18,7 @@ const coarse_granularities = [
 
 // remove non-coarse layers and return what's left (or all if empty)
 function getEffectiveLayers(requested_layers) {
+  // remove non-coarse layers
   const non_coarse_layers_removed = _.without(requested_layers, 'venue', 'address', 'street');
 
   // if resulting array is empty, use all coarse granularities
@@ -62,30 +63,32 @@ function synthesizeDoc(results) {
   const doc = new Document('whosonfirst', most_granular_layer, id.toString());
   doc.setName('default', results[most_granular_layer][0].name);
 
-  if (results[most_granular_layer][0].hasOwnProperty('centroid')) {
+  // 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 );
   }
 
-  if (results[most_granular_layer][0].hasOwnProperty('bounding_box')) {
-    const parsedBoundingBox = results[most_granular_layer][0].bounding_box.split(',').map(parseFloat);
+  // 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: parsedBoundingBox[3],
-        lon: parsedBoundingBox[0]
+        lat: parsed_bounding_box[3],
+        lon: parsed_bounding_box[0]
       },
       lowerRight: {
-        lat: parsedBoundingBox[1],
-        lon: parsedBoundingBox[2]
+        lat: parsed_bounding_box[1],
+        lon: parsed_bounding_box[2]
       }
     });
 
   }
 
-  // assign the administrative hierarchy
-  Object.keys(results).forEach((layer) => {
-    doc.addParent(layer, results[layer][0].name, results[layer][0].id.toString(), results[layer][0].abbr);
-  });
-
   const esDoc = doc.toESDocument();
   esDoc.data._id = esDoc._id;
   esDoc.data._type = esDoc._type;

From 615f7b720e9b742177a91b806625b0674eacfdee Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 2 Jun 2017 17:08:20 -0400
Subject: [PATCH 10/10] switched to t.plan for easier testing with next()

---
 test/unit/controller/coarse_reverse.js | 114 +++++++++----------------
 1 file changed, 41 insertions(+), 73 deletions(-)

diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js
index 56f4e129..bb170a37 100644
--- a/test/unit/controller/coarse_reverse.js
+++ b/test/unit/controller/coarse_reverse.js
@@ -2,6 +2,7 @@
 
 const setup = require('../../../controller/coarse_reverse');
 const proxyquire =  require('proxyquire').noCallThru();
+const _  = require('lodash');
 
 module.exports.tests = {};
 
@@ -15,12 +16,13 @@ module.exports.tests.interface = (test, common) => {
 
 module.exports.tests.early_exit_conditions = (test, common) => {
   test('should_execute returning false should not call service', (t) => {
+    t.plan(2);
+
     const service = () => {
       throw Error('service should not have been called');
     };
 
-    const should_execute = () => { return false; };
-    const controller = setup(service, should_execute);
+    const controller = setup(service, _.constant(false));
 
     const req = {
       clean: {
@@ -30,14 +32,12 @@ module.exports.tests.early_exit_conditions = (test, common) => {
     };
 
     // verify that next was called
-    let next_was_called = false;
     const next = () => {
-      next_was_called = true;
+      t.pass('next() was called');
     };
 
     // passing res=undefined verifies that it wasn't interacted with
     t.doesNotThrow(controller.bind(null, req, undefined, next));
-    t.ok(next_was_called);
     t.end();
 
   });
@@ -46,6 +46,8 @@ module.exports.tests.early_exit_conditions = (test, common) => {
 
 module.exports.tests.error_conditions = (test, common) => {
   test('service error should log and call next', (t) => {
+    t.plan(3);
+
     const service = (req, callback) => {
       t.deepEquals(req, { clean: { layers: ['locality'] } } );
       callback('this is an error');
@@ -53,10 +55,9 @@ module.exports.tests.error_conditions = (test, common) => {
 
     const logger = require('pelias-mock-logger')();
 
-    const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
       'pelias-logger': logger
-    })(service, should_execute);
+    })(service, _.constant(true));
 
     const req = {
       clean: {
@@ -65,16 +66,14 @@ module.exports.tests.error_conditions = (test, common) => {
     };
 
     // verify that next was called
-    let next_was_called = false;
     const next = () => {
-      next_was_called = true;
+      t.pass('next() was called');
     };
 
     // passing res=undefined verifies that it wasn't interacted with
     controller(req, undefined, next);
 
     t.ok(logger.isErrorMessage('this is an error'));
-    t.ok(next_was_called);
     t.end();
 
   });
@@ -83,6 +82,8 @@ module.exports.tests.error_conditions = (test, common) => {
 
 module.exports.tests.success_conditions = (test, common) => {
   test('service returning results should use first entry for each layer', (t) => {
+    t.plan(4);
+
     const service = (req, callback) => {
       t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } );
 
@@ -143,10 +144,9 @@ module.exports.tests.success_conditions = (test, common) => {
 
     const logger = require('pelias-mock-logger')();
 
-    const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
       'pelias-logger': logger
-    })(service, should_execute);
+    })(service, _.constant(true));
 
     const req = {
       clean: {
@@ -157,9 +157,8 @@ module.exports.tests.success_conditions = (test, common) => {
     const res = { };
 
     // verify that next was called
-    let next_was_called = false;
     const next = () => {
-      next_was_called = true;
+      t.pass('next() was called');
     };
 
     controller(req, res, next);
@@ -221,14 +220,14 @@ module.exports.tests.success_conditions = (test, common) => {
     };
 
     t.deepEquals(res, expected);
-
     t.notOk(logger.hasErrorMessages());
-    t.ok(next_was_called);
     t.end();
 
   });
 
   test('layers missing from results should be ignored', (t) => {
+    t.plan(4);
+
     const service = (req, callback) => {
       t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } );
 
@@ -251,10 +250,9 @@ module.exports.tests.success_conditions = (test, common) => {
 
     const logger = require('pelias-mock-logger')();
 
-    const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
       'pelias-logger': logger
-    })(service, should_execute);
+    })(service, _.constant(true));
 
     const req = {
       clean: {
@@ -265,9 +263,8 @@ module.exports.tests.success_conditions = (test, common) => {
     const res = { };
 
     // verify that next was called
-    let next_was_called = false;
     const next = () => {
-      next_was_called = true;
+      t.pass('next() was called');
     };
 
     controller(req, res, next);
@@ -302,14 +299,14 @@ module.exports.tests.success_conditions = (test, common) => {
     };
 
     t.deepEquals(res, expected);
-
     t.notOk(logger.hasErrorMessages());
-    t.ok(next_was_called);
     t.end();
 
   });
 
   test('most granular layer missing centroid should not set', (t) => {
+    t.plan(4);
+
     const service = (req, callback) => {
       t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } );
 
@@ -329,10 +326,9 @@ module.exports.tests.success_conditions = (test, common) => {
 
     const logger = require('pelias-mock-logger')();
 
-    const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
       'pelias-logger': logger
-    })(service, should_execute);
+    })(service, _.constant(true));
 
     const req = {
       clean: {
@@ -343,9 +339,8 @@ module.exports.tests.success_conditions = (test, common) => {
     const res = { };
 
     // verify that next was called
-    let next_was_called = false;
     const next = () => {
-      next_was_called = true;
+      t.pass('next() was called');
     };
 
     controller(req, res, next);
@@ -376,14 +371,14 @@ module.exports.tests.success_conditions = (test, common) => {
     };
 
     t.deepEquals(res, expected);
-
     t.notOk(logger.hasErrorMessages());
-    t.ok(next_was_called);
     t.end();
 
   });
 
   test('most granular layer missing bounding_box should not set', (t) => {
+    t.plan(4);
+
     const service = (req, callback) => {
       t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } );
 
@@ -406,10 +401,9 @@ module.exports.tests.success_conditions = (test, common) => {
 
     const logger = require('pelias-mock-logger')();
 
-    const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
       'pelias-logger': logger
-    })(service, should_execute);
+    })(service, _.constant(true));
 
     const req = {
       clean: {
@@ -420,9 +414,8 @@ module.exports.tests.success_conditions = (test, common) => {
     const res = { };
 
     // verify that next was called
-    let next_was_called = false;
     const next = () => {
-      next_was_called = true;
+      t.pass('next() was called');
     };
 
     controller(req, res, next);
@@ -456,9 +449,7 @@ module.exports.tests.success_conditions = (test, common) => {
     };
 
     t.deepEquals(res, expected);
-
     t.notOk(logger.hasErrorMessages());
-    t.ok(next_was_called);
     t.end();
 
   });
@@ -469,10 +460,9 @@ module.exports.tests.success_conditions = (test, common) => {
 
     // by plan'ing the number of tests, we can verify that next() was called w/o
     //  additional bookkeeping
-    t.plan(5);
+    t.plan(4);
 
-    const service = (point, do_not_track, callback) => {
-      t.equals(do_not_track, 'do_not_track value');
+    const service = (req, callback) => {
       const results = {
         neighbourhood: [
           {
@@ -488,15 +478,9 @@ module.exports.tests.success_conditions = (test, common) => {
 
     const logger = require('pelias-mock-logger')();
 
-    const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
-      'pelias-logger': logger,
-      '../helper/logging': {
-        isDNT: () => {
-          return 'do_not_track value';
-        }
-      }
-    })(service, should_execute);
+      'pelias-logger': logger
+    })(service, _.constant(true));
 
     const req = {
       clean: {
@@ -553,15 +537,14 @@ module.exports.tests.success_conditions = (test, common) => {
     // this test is used to test coarse reverse fallback for when non-coarse reverse
     //  was requested but no non-coarse results were found
     const non_coarse_layers = ['venue', 'address', 'street'];
-    const tests_per_non_coarse_layer = 5;
+    const tests_per_non_coarse_layer = 4;
 
     // by plan'ing the number of tests, we can verify that next() was called w/o
     //  additional bookkeeping
     t.plan(non_coarse_layers.length * tests_per_non_coarse_layer);
 
     non_coarse_layers.forEach((non_coarse_layer) => {
-      const service = (point, do_not_track, callback) => {
-        t.equals(do_not_track, 'do_not_track value');
+      const service = (req, callback) => {
         const results = {
           neighbourhood: [
             {
@@ -577,15 +560,9 @@ module.exports.tests.success_conditions = (test, common) => {
 
       const logger = require('pelias-mock-logger')();
 
-      const should_execute = () => { return true; };
       const controller = proxyquire('../../../controller/coarse_reverse', {
-        'pelias-logger': logger,
-        '../helper/logging': {
-          isDNT: () => {
-            return 'do_not_track value';
-          }
-        }
-      })(service, should_execute);
+        'pelias-logger': logger
+      })(service, _.constant(true));
 
       const req = {
         clean: {
@@ -644,15 +621,14 @@ module.exports.tests.success_conditions = (test, common) => {
     // this test is used to test coarse reverse fallback for when non-coarse reverse
     //  was requested but no non-coarse results were found
     const non_coarse_layers = ['venue', 'address', 'street'];
-    const tests_per_non_coarse_layer = 5;
+    const tests_per_non_coarse_layer = 4;
 
     // by plan'ing the number of tests, we can verify that next() was called w/o
     //  additional bookkeeping
     t.plan(non_coarse_layers.length * tests_per_non_coarse_layer);
 
     non_coarse_layers.forEach((non_coarse_layer) => {
-      const service = (point, do_not_track, callback) => {
-        t.equals(do_not_track, 'do_not_track value');
+      const service = (req, callback) => {
         const results = {
           neighbourhood: [
             {
@@ -668,15 +644,9 @@ module.exports.tests.success_conditions = (test, common) => {
 
       const logger = require('pelias-mock-logger')();
 
-      const should_execute = () => { return true; };
       const controller = proxyquire('../../../controller/coarse_reverse', {
-        'pelias-logger': logger,
-        '../helper/logging': {
-          isDNT: () => {
-            return 'do_not_track value';
-          }
-        }
-      })(service, should_execute);
+        'pelias-logger': logger
+      })(service, _.constant(true));
 
       const req = {
         clean: {
@@ -735,6 +705,8 @@ module.exports.tests.success_conditions = (test, common) => {
 
 module.exports.tests.failure_conditions = (test, common) => {
   test('service returning 0 results at the requested layer should return nothing', (t) => {
+    t.plan(4);
+
     const service = (req, callback) => {
       t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } );
 
@@ -783,10 +755,9 @@ module.exports.tests.failure_conditions = (test, common) => {
 
     const logger = require('pelias-mock-logger')();
 
-    const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
       'pelias-logger': logger
-    })(service, should_execute);
+    })(service, _.constant(true));
 
     const req = {
       clean: {
@@ -797,9 +768,8 @@ module.exports.tests.failure_conditions = (test, common) => {
     const res = { };
 
     // verify that next was called
-    let next_was_called = false;
     const next = () => {
-      next_was_called = true;
+      t.pass('next() was called');
     };
 
     controller(req, res, next);
@@ -810,9 +780,7 @@ module.exports.tests.failure_conditions = (test, common) => {
     };
 
     t.deepEquals(res, expected);
-
     t.notOk(logger.hasErrorMessages());
-    t.ok(next_was_called);
     t.end();
 
   });