Skip to content

Commit

Permalink
feat(controller): Remove search_with_ids controller
Browse files Browse the repository at this point in the history
I was attempting to use the new debug output from
#1313 only to realize it was not added
to the `search_with_ids` controller, only the `search` controller.

These two controllers are very similar, so this PR combines them into
one that can be used in all cases.
  • Loading branch information
orangejulius committed Jun 18, 2019
1 parent 106a90a commit 198999a
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 703 deletions.
24 changes: 16 additions & 8 deletions controller/search.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ function setup( apiConfig, esclient, query, should_execute ){
cleanOutput = logging.removeFields(cleanOutput);
}

const renderedQuery = query(req.clean);
// rendering a query requires passing the `clean` object, which contains
// validated options from query parameters, and the `res` object, since
// some queries use the results of previous queries to Placeholder
const renderedQuery = query(req.clean, res);

// if there's no query to call ES with, skip the service
if (_.isUndefined(renderedQuery)) {
Expand Down Expand Up @@ -79,10 +82,10 @@ function setup( apiConfig, esclient, query, should_execute ){
}

// if execution has gotten this far then one of three things happened:
// - the request didn't time out
// - the request returned a response, either success or error
// - maxRetries has been hit so we're giving up
// - another error occurred
// in either case, handle the error or results
// in any case, handle the error or results

// error handler
if( err ){
Expand All @@ -94,11 +97,17 @@ function setup( apiConfig, esclient, query, should_execute ){
}
// set response data
else {
res.data = docs;
res.meta = meta || {};
// store the query_type for subsequent middleware
res.meta.query_type = renderedQuery.type;
// because this controller may be called multiple times, there may already
// be results. if there are no results from this ES call, don't overwrite
// what's already there from a previous call.
if (!_.isEmpty(docs)) {
res.data = docs;
res.meta = meta || {};
// store the query_type for subsequent middleware
res.meta.query_type = renderedQuery.type;
}

// put an entry in the debug log no matter the number of results
debugLog.push(req, {queryType: {
[renderedQuery.type] : {
es_took: message.es_took,
Expand All @@ -114,7 +123,6 @@ function setup( apiConfig, esclient, query, should_execute ){
});
debugLog.stopTimer(req, initialTime);
});

}

return controller;
Expand Down
127 changes: 0 additions & 127 deletions controller/search_with_ids.js

This file was deleted.

8 changes: 3 additions & 5 deletions routes/v1.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ var controllers = {
place: require('../controller/place'),
placeholder: require('../controller/placeholder'),
search: require('../controller/search'),
search_with_ids: require('../controller/search_with_ids'),
status: require('../controller/status')
};

Expand Down Expand Up @@ -289,11 +288,10 @@ function addRoutes(app, peliasConfig) {
controllers.libpostal(libpostalService, libpostalShouldExecute),
controllers.placeholder(placeholderService, geometricFiltersApply, placeholderGeodisambiguationShouldExecute),
controllers.placeholder(placeholderService, geometricFiltersApply, placeholderIdsLookupShouldExecute),
controllers.search_with_ids(peliasConfig.api, esclient, queries.address_using_ids, searchWithIdsShouldExecute),
// 3rd parameter is which query module to use, use fallback first, then
// use original search strategy if first query didn't return anything
// try 3 different query types: address search using ids, cascading fallback, addressit(very_old_prod)
controllers.search(peliasConfig.api, esclient, queries.address_using_ids, searchWithIdsShouldExecute),
controllers.search(peliasConfig.api, esclient, queries.cascading_fallback, fallbackQueryShouldExecute),
sanitizers.defer_to_addressit(shouldDeferToAddressIt),
sanitizers.defer_to_addressit(shouldDeferToAddressIt), //run additional sanitizers needed for addressit parser
controllers.search(peliasConfig.api, esclient, queries.very_old_prod, oldProdQueryShouldExecute),
postProc.trimByGranularity(),
postProc.distances('focus.point.'),
Expand Down
11 changes: 8 additions & 3 deletions test/unit/controller/search.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,21 @@ module.exports.tests.success = function(test, common) {
})(config, esclient, query, () => { return true; });

const req = { clean: { }, errors: [], warnings: [] };
const res = {};
const res = { meta: { query_type: 'this is the query type from a previous query'} };

const next = () => {
t.deepEqual(req, {
clean: {},
errors: [],
warnings: []
});
t.equals(res.data, undefined);
t.deepEquals(res.meta, { key: 'value', query_type: 'this is the query type' });

t.equals(res.data, undefined, 'no data set');

const expected_meta = {
query_type: 'this is the query type from a previous query'
};
t.deepEquals(res.meta, expected_meta, 'previous query meta information left unchanged');

t.end();
};
Expand Down
Loading

0 comments on commit 198999a

Please sign in to comment.