Merge pull request #1150 from pelias/interpolation_sorting

fix(interpolation): Ensure proper sorting of streets with interpolated addresses

middleware/interpolate.js

@@ -2,6 +2,7 @@ const async = require('async');
 const logger = require( 'pelias-logger' ).get( 'api' );
 const source_mapping = require('../helper/type_mapping').source_mapping;
 const _ = require('lodash');
+const stable = require('stable');
 
 /**
 example response from interpolation web service:
@@ -116,7 +117,7 @@ function setup(service, should_execute) {
 
       // sort the results to ensure that addresses show up higher than street centroids
       if (_.has(res, 'data')) {
-        res.data.sort((a, b) => {
+        res.data = stable(res.data, (a, b) => {
           if (a.layer === 'address' && b.layer !== 'address') { return -1; }
           if (a.layer !== 'address' && b.layer === 'address') { return 1; }
           return 0;

package.json

@@ -62,6 +62,7 @@
     "pelias-sorting": "1.1.0",
     "predicates": "^2.0.0",
     "retry": "^0.10.1",
+    "stable": "^0.1.8",
     "stats-lite": "^2.0.4",
     "through2": "^2.0.3"
   },

test/unit/middleware/interpolate.js

@@ -277,6 +277,81 @@ module.exports.tests.success_conditions = (test, common) => {
 
   });
 
+  test('results should be sorted first by address/non-address. previous ordering should otherwise be maintained via a stable sort', t => {
+    const service = (req, res, callback) => {
+      // results 5 and 7 will have interpolated results returned
+      // this is to ensure results are re-sorted to move the addresses first
+      if (res.id === 5 || res.id === 7) {
+        callback(null, {
+          properties: {
+            number: 17,
+            source: 'Source Abbr 1',
+            source_id: 'source 1 source id',
+            lat: 12.121212,
+            lon: 21.212121
+          }
+        });
+      } else {
+        // return empty results in most cases
+        callback(null, {});
+      }
+    };
+
+    const logger = require('pelias-mock-logger')();
+
+    const controller = proxyquire('../../../middleware/interpolate', {
+      'pelias-logger': logger
+    })(service, () => true);
+
+    const req = {
+      clean: {
+        parsed_text: 'this is req.clean.parsed_text'
+      }
+    };
+    const res = {};
+
+    // helper method to generate test results which default to streets
+    function generateTestStreets(id) {
+      return {
+        id: id+1,
+        layer: 'street',
+        name: { default: `name ${id+1}` },
+        address_parts: {},
+        source_id: 'original source_id'
+      };
+    }
+
+    // generate a set of street results of desired size
+    // NOTE: this set must be of 11 elements or greater
+    // Node.js uses stable insertion sort for arrays of 10 or fewer elements,
+    // but _unstable_ QuickSort for larger arrays
+    const resultCount = 11;
+    const sequence_array = Array.from(new Array(resultCount),(val,index)=>index);
+    res.data = sequence_array.map(generateTestStreets);
+
+    controller(req, res, () => {
+      t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages');
+
+      const results = res.data;
+
+      t.equals(results.length, results.length, 'correct number of results should be returned');
+      t.equals(results[0].layer, 'address', 'first result should be interpolated address');
+      t.equals(results[1].layer, 'address', 'second result should be interpolated address');
+
+      // iterate through all remaining records, ensuring their ids are increasing,
+      // as was the case when the set of streets was originally generated
+      let previous_id;
+      for (let i = 2; i < results.length; i++) {
+        if (previous_id) {
+          t.ok(results[i].id > previous_id, `id ${results[i].id} should be higher than ${previous_id}, to ensure sort is stable`);
+        }
+        previous_id = results[i].id;
+      }
+
+      t.end();
+    });
+  });
+
   test('service call returning error should not map in interpolated results for non-errors', t => {
     const service = (req, res, callback) => {
       if (res.id === 1) {