fix(search): correctly handle duplicate entries

The implementation for comparing entries did not properly account for
entries that are "equal", and whenever they occur it ran into a loop.

The implementation proposed does two things:

* First we simplify the existing logic to be a two step
  filter + sort mechanism
* Second, the search comparison is simple and robust, and handles
  equality well.

This should fix the loop issues we're seeing.

lib/features/search/search.js

@@ -27,22 +27,23 @@
  * @returns {SearchResults}
  */
 export default function search(items, pattern, options) {
-  return items.reduce((results, item) => {
-    const tokens = getTokens(item, pattern, options.keys);
 
-    if (Object.keys(tokens).length) {
-      const result = {
-        item,
-        tokens
-      };
+  const {
+    keys
+  } = options;
 
-      const index = getIndex(result, results, options.keys);
+  return items.flatMap((item, idx) => {
+    const tokens = getTokens(item, pattern, keys);
 
-      results.splice(index, 0, result);
+    if (!Object.keys(tokens).length) {
+      return [];
     }
 
-    return results;
-  }, []);
+    return {
+      item,
+      tokens
+    };
+  }).sort(createResultSorter(keys));
 }
 
 /**
@@ -71,59 +72,43 @@ function getTokens(item, pattern, keys) {
 /**
  * Get index of result in list of results.
  *
- * @param {SearchResult} result
- * @param {SearchResults} results
  * @param {string[]} keys
  *
- * @returns {number}
+ * @returns { (resultA: SearchResult, resultB: SearchResult) => number}
  */
-function getIndex(result, results, keys) {
-  if (!results.length) {
-    return 0;
-  }
+function createResultSorter(keys) {
 
-  let index = 0;
+  /**
+   * @param {SearchResult} resultA
+   * @param {SearchResult} resultB
+   */
+  return (resultA, resultB) => {
 
-  do {
     for (const key of keys) {
-      const tokens = result.tokens[ key ],
-            tokensOther = results[ index ].tokens[ key ];
-
-      if (tokens && !tokensOther) {
-        return index;
-      } else if (!tokens && tokensOther) {
-        index++;
-
-        break;
-      } else if (!tokens && !tokensOther) {
-        continue;
-      }
 
-      const tokenComparison = compareTokens(tokens, tokensOther);
+      const tokensA = resultA.tokens[key];
+      const tokensB = resultB.tokens[key];
 
-      if (tokenComparison === -1) {
-        return index;
-      } else if (tokenComparison === 1) {
-        index++;
+      const tokenComparison = compareTokens(tokensA, tokensB);
 
-        break;
-      } else {
-        const stringComparison = compareStrings(result.item[ key ], results[ index ].item[ key ]);
+      if (tokenComparison !== 0) {
+        return tokenComparison;
+      }
 
-        if (stringComparison === -1) {
-          return index;
-        } else if (stringComparison === 1) {
-          index++;
+      const stringComparison = compareStrings(resultA.item[ key ], resultB.item[ key ]);
 
-          break;
-        } else {
-          continue;
-        }
+      if (stringComparison !== 0) {
+        return stringComparison;
       }
+
+      // fall back to next key
+      continue;
     }
-  } while (index < results.length);
 
-  return index;
+    // eventually call equality
+    return 0;
+  };
+
 }
 
 /**
@@ -147,14 +132,15 @@ export function hasMatch(tokens) {
 /**
 * Compares two token arrays.
 *
-* @param {Token[]} tokensA
-* @param {Token[]} tokensB
+* @param {Token[]} [tokensA]
+* @param {Token[]} [tokensB]
 *
 * @returns {number}
 */
 export function compareTokens(tokensA, tokensB) {
-  const tokensAHasMatch = hasMatch(tokensA),
-        tokensBHasMatch = hasMatch(tokensB);
+
+  const tokensAHasMatch = tokensA && hasMatch(tokensA),
+        tokensBHasMatch = tokensB && hasMatch(tokensB);
 
   if (tokensAHasMatch && !tokensBHasMatch) {
     return -1;
@@ -185,12 +171,12 @@ export function compareTokens(tokensA, tokensB) {
 /**
 * Compares two strings.
 *
-* @param {string} a
-* @param {string} b
+* @param {string} [a = '']
+* @param {string} [b = '']
 *
 * @returns {number}
 */
-export function compareStrings(a, b) {
+export function compareStrings(a = '', b = '') {
   return a.localeCompare(b);
 }
 

test/spec/features/search/searchSpec.js

@@ -15,7 +15,7 @@ describe('search', function() {
   }));
 
 
-  it('complex', inject(function(search) {
+  it('should search complex', inject(function(search) {
 
     // given
     const items = [
@@ -63,7 +63,7 @@ describe('search', function() {
   }));
 
 
-  it('should by match', inject(function(search) {
+  it('should sort by match', inject(function(search) {
 
     // given
     const items = [
@@ -96,7 +96,7 @@ describe('search', function() {
   }));
 
 
-  it('should by match location', inject(function(search) {
+  it('should sort by match location', inject(function(search) {
 
     // given
     const items = [
@@ -195,6 +195,38 @@ describe('search', function() {
     expect(results[1].item).to.eql(items[0]);
   }));
 
+
+  it('should handle duplicate entries', inject(function(search) {
+
+    // given
+    const items = [
+      {
+        title: 'baz',
+        description: 'baz'
+      },
+      {
+        title: 'Kafka message',
+        description: 'Nope'
+      },
+      {
+        title: 'Kafka message',
+        description: 'Nope'
+      }
+    ];
+
+    // when
+    const results = search(items, 'g', {
+      keys: [
+        'title',
+        'description',
+        'search'
+      ]
+    });
+
+    // then
+    expect(results).to.have.length(2);
+  }));
+
 });