diff --git a/middleware/interpolate.js b/middleware/interpolate.js index 10c3f7e8..8507709a 100644 --- a/middleware/interpolate.js +++ b/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; diff --git a/package.json b/package.json index 3c5759da..196c8a78 100644 --- a/package.json +++ b/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" }, diff --git a/test/unit/middleware/interpolate.js b/test/unit/middleware/interpolate.js index 3602592c..537c7444 100644 --- a/test/unit/middleware/interpolate.js +++ b/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) {