From ce42429fbdd23bc5eb52cb542e0f3c1be87398f0 Mon Sep 17 00:00:00 2001 From: Pete Richards Date: Tue, 20 Oct 2015 15:14:43 -0700 Subject: [PATCH] [Search] expose constants, add fudge factor The SearchAggregator exposes it's constants to add stability to tests. It also has a fudge factor which increaases the number of results it requests from providers to better support pagination when using client side filtering. --- .../search/src/services/SearchAggregator.js | 23 +++++++++++++++---- .../test/services/SearchAggregatorSpec.js | 18 +++++++++++++-- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/platform/search/src/services/SearchAggregator.js b/platform/search/src/services/SearchAggregator.js index 0cdcc5b922..40e77c271b 100644 --- a/platform/search/src/services/SearchAggregator.js +++ b/platform/search/src/services/SearchAggregator.js @@ -31,8 +31,6 @@ define([ ) { "use strict"; - var DEFAULT_MAX_RESULTS = 100; - /** * Aggregates multiple search providers as a singular search provider. * Search providers are expected to implement a `query` method which returns @@ -53,6 +51,23 @@ define([ this.providers = providers; } + /** + * If max results is not specified in query, use this as default. + */ + SearchAggregator.prototype.DEFAULT_MAX_RESULTS = 100; + + /** + * Because filtering isn't implemented inside each provider, the fudge + * factor is a multiplier on the number of results returned-- more results + * than requested will be fetched, and then will be fetched. This helps + * provide more predictable pagination when large numbers of matches exist + * but very few matches match filters. + * + * If a provider level filter implementation is implemented in the future, + * remove this. + */ + SearchAggregator.prototype.FUDGE_FACTOR = 5; + /** * Sends a query to each of the providers. Returns a promise for * a result object that has the format @@ -79,13 +94,13 @@ define([ resultPromises; if (!maxResults) { - maxResults = DEFAULT_MAX_RESULTS; + maxResults = this.DEFAULT_MAX_RESULTS; } resultPromises = this.providers.map(function (provider) { return provider.query( inputText, - maxResults + maxResults * aggregator.FUDGE_FACTOR ); }); diff --git a/platform/search/test/services/SearchAggregatorSpec.js b/platform/search/test/services/SearchAggregatorSpec.js index 044a5f4609..d63b672b2b 100644 --- a/platform/search/test/services/SearchAggregatorSpec.js +++ b/platform/search/test/services/SearchAggregatorSpec.js @@ -49,6 +49,13 @@ define([ aggregator = new SearchAggregator($q, objectService, providers); }); + it("has a fudge factor", function () { + expect(aggregator.FUDGE_FACTOR).toBe(5); + }); + + it("has default max results", function () { + expect(aggregator.DEFAULT_MAX_RESULTS).toBe(100); + }); it("can order model results by score", function () { var modelResults = { @@ -170,7 +177,11 @@ define([ providers.push(provider); aggregator.query('find me', 123, 'filter'); - expect(provider.query).toHaveBeenCalledWith('find me', 123); + expect(provider.query) + .toHaveBeenCalledWith( + 'find me', + 123 * aggregator.FUDGE_FACTOR + ); expect($q.all).toHaveBeenCalledWith(['i prooomise!']); }); @@ -181,7 +192,10 @@ define([ ); providers.push(provider); aggregator.query('find me'); - expect(provider.query).toHaveBeenCalledWith('find me', 100); + expect(provider.query).toHaveBeenCalledWith( + 'find me', + aggregator.DEFAULT_MAX_RESULTS * aggregator.FUDGE_FACTOR + ); }); it('can combine responses from multiple providers', function () {