From bce56c53ed2fd95a3acf22dd3813ba3241236a48 Mon Sep 17 00:00:00 2001 From: slhale Date: Thu, 30 Jul 2015 13:19:19 -0700 Subject: [PATCH] [Search] Clean up Updated comments and documentation. Changed the generic worker to return results as {id: score} key value pairs rather than {id: {score: score}} format. Removed unused getLatestResults and getLatestTimestamps functions from the providers. --- .../features/search/src/SearchAggregator.js | 36 +++------- .../providers/ElasticsearchSearchProvider.js | 66 +++++-------------- .../src/providers/GenericSearchProvider.js | 50 ++++---------- .../search/src/workers/GenericSearchWorker.js | 8 +-- 4 files changed, 41 insertions(+), 119 deletions(-) diff --git a/platform/features/search/src/SearchAggregator.js b/platform/features/search/src/SearchAggregator.js index 24e811aa13..7a33461eab 100644 --- a/platform/features/search/src/SearchAggregator.js +++ b/platform/features/search/src/SearchAggregator.js @@ -34,7 +34,7 @@ define( /** * Allows multiple services which provide search functionality - * to be treated as one. + * to be treated as one. * * @constructor * @param $q Angular's $q, for promise consolidation @@ -43,19 +43,6 @@ define( */ function SearchAggregator($q, providers) { var loading; - //var loadingQueue = {}; - // {key: value} are {timestamp: isLoading} - - // Takes an array of arrays and returns an array containing all - // of the sub-arrays' contents - function concatArrays(arrays) { - var i, - array = []; - for (i = 0; i < arrays.length; i += 1) { - array = array.concat(arrays[i]); - } - return array; - } // Remove duplicate objects that have the same ID function filterRepeats(results) { @@ -129,7 +116,6 @@ define( // We are loading loading = true; - //loadingQueue[timestamp] = true; // Send the query to all the providers for (i = 0; i < providers.length; i += 1) { @@ -140,14 +126,18 @@ define( // Get promises for results arrays return $q.all(resultPromises).then(function (resultsArrays) { + var results = [], + i; + // Merge results - var results = concatArrays(resultsArrays); + for (i = 0; i < resultsArrays.length; i += 1) { + results = results.concat(resultsArrays[i]); + } results = filterRepeats(results); results = orderByScore(results); // We are done loading loading = false; - //loadingQueue[timestamp] = false; return results; }); @@ -155,8 +145,8 @@ define( return { /** - * Sends a query to each of the providers, then updates the global - * latestMergedResults accordingly. + * Sends a query to each of the providers. Returns a promise + * for an array of domain object results. * * @param inputText The text input that is the query. */ @@ -168,14 +158,6 @@ define( */ isLoading: function () { return loading; - /* - for (var timestamp in loadingQueue) { - if (loadingQueue[timestamp] === true) { - return true; - } - } - return false; - */ } }; } diff --git a/platform/features/search/src/providers/ElasticsearchSearchProvider.js b/platform/features/search/src/providers/ElasticsearchSearchProvider.js index 2e754a7898..01f66026cc 100644 --- a/platform/features/search/src/providers/ElasticsearchSearchProvider.js +++ b/platform/features/search/src/providers/ElasticsearchSearchProvider.js @@ -33,7 +33,7 @@ define( // so hide them here. var ID = "_id", SCORE = "_score", - DEFAULT_MAX_RESULTS = 100; + DEFAULT_MAX_RESULTS = 999999; /** * A search service which searches through domain objects in @@ -47,8 +47,6 @@ define( * interact with ElasticSearch. */ function ElasticsearchSearchProvider($http, objectService, ROOT) { - var latestResults = [], - lastSearchTimestamp = 0; // Check to see if the input has any special options function isDefaultFormat(searchTerm) { @@ -115,8 +113,6 @@ define( searchResults = [], i; - //console.log('es provider - raw results', rawResults); - /* if (rawResults.data.hits.total > resultsLength) { // TODO: Somehow communicate this to the user @@ -131,19 +127,14 @@ define( // Get the result objects' scores for (i = 0; i < resultsLength; i += 1) { - //scores.push(results[i][SCORE]); scores[ids[i]] = results[i][SCORE]; } - //console.log('es provider - ids', ids); - // Get the domain objects from their IDs return objectService.getObjects(ids).then(function (objects) { var j, id; - //console.log('es provider - objects', objects); - for (j = 0; j < resultsLength; j += 1) { id = ids[j]; @@ -158,17 +149,12 @@ define( } } - latestResults = searchResults; - lastSearchTimestamp = timestamp; - - //console.log('es provider - search results', searchResults); - return searchResults; }); } // For documentation, see query below. - function queryElasticsearch(searchTerm, timestamp, maxResults, timeout) { + function query(searchTerm, timestamp, maxResults, timeout) { var esQuery; // Check to see if the user provided a maximum @@ -199,53 +185,33 @@ define( return processResults(rawResults, timestamp); }); } else { - //console.log('es provider - empty search input'); - latestResults = []; - lastSearchTimestamp = timestamp; - return latestResults; + return []; } } return { /** * Searches through the filetree for domain objects using a search - * term. This is done through querying elasticsearch. + * term. This is done through querying elasticsearch. Returns a + * promise for an array of domain object results. * Notes: * * The order of the results is from highest to lowest score, * as elsaticsearch determines them to be. - * * Fuzziness is used to produce more results that are still - * relevant. (All results within a certain edit distance.) - * * More search details at + * * Uses the fuzziness operator to get more results. + * * More about this search's behavior at * https://www.elastic.co/guide/en/elasticsearch/reference/current/search-uri-request.html * * @param searchTerm The text input that is the query. - * input where this funcion should find the search term - * @param timestamp the time at which this function was called, - * this timestamp will be associated with the latest results - * list, which allows the aggregator to see if it has been - * updated - * @param maxResults (optional) the maximum number of results - * that this function should return - * @param timeout (optional) the time after which the search should - * stop calculations and return partial results + * @param timestamp The time at which this function was called. + * This timestamp is used as a unique identifier for this + * query and the corresponding results. + * @param maxResults (optional) The maximum number of results + * that this function should return. + * @param timeout (optional) The time after which the search should + * stop calculations and return partial results. Elasticsearch + * does not guarentee that this timeout will be strictly followed. */ - query: queryElasticsearch, - - /** - * Get the latest search results that have been calculated. The - * format of the returned objects are searchResult objects, which - * have the members id, object, and score. - */ - getLatestResults: function () { - return latestResults; - }, - - /** - * Get the timestamp for the last update of latestResults. - */ - getLatestTimestamp: function () { - return lastSearchTimestamp; - } + query: query }; } diff --git a/platform/features/search/src/providers/GenericSearchProvider.js b/platform/features/search/src/providers/GenericSearchProvider.js index 9e67dc89c2..86bdd312ff 100644 --- a/platform/features/search/src/providers/GenericSearchProvider.js +++ b/platform/features/search/src/providers/GenericSearchProvider.js @@ -47,8 +47,6 @@ define( */ function GenericSearchProvider($q, objectService, workerService, roots) { var worker = workerService.run('genericSearchWorker'), - //latestResults = [], - //lastSearchTimestamp = 0, pendingQueries = {}; // pendingQueries is a dictionary with the key value pairs st // the key is the timestamp and the value is the promise @@ -98,19 +96,13 @@ define( id; // Reset and repopulate the latest results - //latestResults = []; for (id in objects) { - //latestResults.push({ results.push({ object: objects[id], id: id, - score: event.data.results[id].score + score: event.data.results[id] }); } - // Update the timestamp to the one that this search was made with - //lastSearchTimestamp = event.data.timestamp; - - //console.log('provider - about to resolve', latestResults); // Resove the promise corresponding to this pendingQueries[event.data.timestamp].resolve(results); @@ -174,14 +166,14 @@ define( pendingQueries[timestamp] = defer; // Check to see if the user provided a maximum - // number of results to display + // number of results to display if (!maxResults) { // Else, we provide a default value. maxResults = DEFAULT_MAX_RESULTS; } // Instead, assume that the items have already been indexed, and - // just send the query to the worker. + // just send the query to the worker. workerSearch(input, maxResults, timestamp); return defer.promise; @@ -199,7 +191,8 @@ define( /** * Searches through the filetree for domain objects which match * the search term. This function is to be used as a fallback - * in the case where other search services are not avaliable. + * in the case where other search services are not avaliable. + * Returns a promise for an array of domain object results. * Notes: * * The order of the results is not guarenteed. * * A domain object qualifies as a match for a search input if @@ -209,32 +202,15 @@ define( * the terms as substrings. * * @param input The text input that is the query. - * @param timestamp the time at which this function was called, - * this timestamp will be associated with the latest results - * list, which allows the aggregator to see if it has been - * updated - * @param maxResults (optional) the maximum number of results - * that this function should return - * @param timeout (optional) the time after which the search should - * stop calculations and return partial results + * @param timestamp The time at which this function was called. + * This timestamp is used as a unique identifier for this + * query and the corresponding results. + * @param maxResults (optional) The maximum number of results + * that this function should return. + * @param timeout (optional) The time after which the search should + * stop calculations and return partial results. */ - query: query, - - /** - * Get the latest search results that have been calculated. The - * format of the returned objects are searchResult objects, which - * have the members id, object, and score. - */ - getLatestResults: function () { - return latestResults; - }, - - /** - * Get the timestamp for the last update of latestResults. - */ - getLatestTimestamp: function () { - return lastSearchTimestamp; - } + query: query }; } diff --git a/platform/features/search/src/workers/GenericSearchWorker.js b/platform/features/search/src/workers/GenericSearchWorker.js index 011df8c8b5..484dcc5a54 100644 --- a/platform/features/search/src/workers/GenericSearchWorker.js +++ b/platform/features/search/src/workers/GenericSearchWorker.js @@ -46,8 +46,8 @@ * Indexes an item to indexedItems. * * @param data An object which contains: - * * model: The model of the domain object - * * id: The ID of the domain object + * * model: The model of the domain object + * * id: The ID of the domain object */ function index(data) { var message; @@ -142,9 +142,7 @@ for (i = 0; i < timesToLoop; i += 1) { score = scoreItem(indexedItems[i], input, terms); if (score > 0) { - results[indexedItems[i].id] = { - score: score - }; + results[indexedItems[i].id] = score; } } }