fix(facets): choose highest value in disjunctive

bundlesize.config.json

@@ -2,11 +2,11 @@
   "files": [
     {
       "path": "packages/algoliasearch-helper/dist/algoliasearch.helper.js",
-      "maxSize": "41.75 kB"
+      "maxSize": "42 kB"
     },
     {
       "path": "packages/algoliasearch-helper/dist/algoliasearch.helper.min.js",
-      "maxSize": "13.50 kB"
+      "maxSize": "13.75 kB"
     },
     {
       "path": "./packages/instantsearch.js/dist/instantsearch.production.min.js",

packages/algoliasearch-helper/src/SearchResults/index.js

@@ -6,6 +6,7 @@ var fv = require('../functions/escapeFacetValue');
 var find = require('../functions/find');
 var findIndex = require('../functions/findIndex');
 var formatSort = require('../functions/formatSort');
+var mergeNumericMax = require('../functions/mergeNumericMax');
 var orderBy = require('../functions/orderBy');
 var escapeFacetValue = fv.escapeFacetValue;
 var unescapeFacetValue = fv.unescapeFacetValue;
@@ -514,7 +515,7 @@ function SearchResults(state, results, options) {
 
         self.hierarchicalFacets[position][attributeIndex].data =
           self.persistHierarchicalRootCount
-            ? defaultsPure(
+            ? mergeNumericMax(
                 self.hierarchicalFacets[position][attributeIndex].data,
                 facetResults
               )
@@ -530,7 +531,7 @@ function SearchResults(state, results, options) {
 
         self.disjunctiveFacets[position] = {
           name: dfacet,
-          data: defaultsPure(dataFromMainRequest, facetResults),
+          data: mergeNumericMax(dataFromMainRequest, facetResults),
           exhaustive: result.exhaustiveFacetsCount,
         };
         assignFacetStats(

packages/algoliasearch-helper/src/functions/mergeNumericMax.js

@@ -0,0 +1,29 @@
+'use strict';
+
+// NOTE: this behaves like lodash/defaults, but doesn't mutate the target
+// it also preserve keys order and keep the highest numeric value
+function mergeNumericMax() {
+  var sources = Array.prototype.slice.call(arguments);
+
+  return sources.reduceRight(function (acc, source) {
+    Object.keys(Object(source)).forEach(function (key) {
+      var accValue = typeof acc[key] === 'number' ? acc[key] : 0;
+      var sourceValue = source[key];
+
+      if (sourceValue === undefined) {
+        return;
+      }
+
+      if (sourceValue >= accValue) {
+        if (acc[key] !== undefined) {
+          // remove if already added, so that we can add it in correct order
+          delete acc[key];
+        }
+        acc[key] = sourceValue;
+      }
+    });
+    return acc;
+  }, {});
+}
+
+module.exports = mergeNumericMax;

packages/algoliasearch-helper/test/spec/SearchResults/getFacetValues/disjunctive-non-exhaustive.json

@@ -10,7 +10,8 @@
         "processingTimeMS": 1,
         "facets": {
           "brand": {
-            "Apple": 1000
+            "Apple": 1000,
+            "Samsung": 1
           }
         },
         "exhaustiveFacetsCount": true,
@@ -22,6 +23,9 @@
         "hits": [
           {
             "objectID": "1696302"
+          },
+          {
+            "objectID": "1696301"
           }
         ],
         "nbHits": 10000,

packages/algoliasearch-helper/test/spec/SearchResults/getFacetValues/hierarchical-non-exhaustive.json

@@ -9,7 +9,8 @@
         "hitsPerPage": 8,
         "facets": {
           "brand": {
-            "Apple": 1000
+            "Apple": 1000,
+            "Samsung": 1
           }
         },
         "exhaustiveFacetsCount": true,

packages/algoliasearch-helper/test/spec/functions/defaultsPure.js

@@ -1,37 +1,37 @@
 'use strict';
 
-var defaults = require('../../../src/functions/defaultsPure');
+var defaultsPure = require('../../../src/functions/defaultsPure');
 
 // tests modified from lodash source
 
 it('should assign source properties if missing on `object`', function () {
-  var actual = defaults({ a: 1 }, { a: 2, b: 2 });
+  var actual = defaultsPure({ a: 1 }, { a: 2, b: 2 });
   expect(actual).toEqual({ a: 1, b: 2 });
 });
 
 it('should accept multiple sources', function () {
   var expected = { a: 1, b: 2, c: 3 };
-  var actual = defaults({ a: 1, b: 2 }, { b: 3 }, { c: 3 });
+  var actual = defaultsPure({ a: 1, b: 2 }, { b: 3 }, { c: 3 });
 
   expect(actual).toEqual(expected);
 
-  actual = defaults({ a: 1, b: 2 }, { b: 3, c: 3 }, { c: 2 });
+  actual = defaultsPure({ a: 1, b: 2 }, { b: 3, c: 3 }, { c: 2 });
   expect(actual).toEqual(expected);
 });
 
 it('should not overwrite `null` values', function () {
-  var actual = defaults({ a: null }, { a: 1 });
+  var actual = defaultsPure({ a: null }, { a: 1 });
   expect(actual.a).toBe(null);
 });
 
 it('should overwrite `undefined` values', function () {
-  var actual = defaults({ a: undefined }, { a: 1 });
+  var actual = defaultsPure({ a: undefined }, { a: 1 });
   expect(actual.a).toBe(1);
 });
 
 it('should assign `undefined` values', function () {
   var source = { a: undefined, b: 1 };
-  var actual = defaults({}, source);
+  var actual = defaultsPure({}, source);
 
   expect(actual).toEqual({ a: undefined, b: 1 });
 });
@@ -58,14 +58,14 @@ it('should assign properties that shadow those on `Object.prototype`', function
   };
 
   var expected = Object.assign({}, source);
-  expect(defaults({}, source)).toEqual(expected);
+  expect(defaultsPure({}, source)).toEqual(expected);
 
   expected = Object.assign({}, object);
-  expect(defaults({}, object, source)).toEqual(expected);
+  expect(defaultsPure({}, object, source)).toEqual(expected);
 });
 
 it('should keep the keys order with facets', function () {
-  var actual = defaults(
+  var actual = defaultsPure(
     {},
     {
       'Insignia™': 551,
@@ -80,7 +80,7 @@ it('should keep the keys order with facets', function () {
 });
 
 it('should keep the keys order when adding facet refinements', function () {
-  var actual = defaults(
+  var actual = defaultsPure(
     {},
     {
       facet2: ['facetValue'],
@@ -98,7 +98,7 @@ it('does not pollute the prototype', () => {
 
   expect(subject.polluted).toBe(undefined);
 
-  const out = defaults({}, payload);
+  const out = defaultsPure({}, payload);
 
   expect(out).toEqual({});
 

packages/algoliasearch-helper/test/spec/functions/mergeNumericMax.js

@@ -0,0 +1,131 @@
+'use strict';
+
+var mergeNumericMax = require('../../../src/functions/mergeNumericMax');
+
+// tests modified from defaultsPure
+
+it('should assign source properties if missing on `object`', function () {
+  var actual = mergeNumericMax({ a: 1 }, { a: 1, b: 2 });
+  expect(actual).toEqual({ a: 1, b: 2 });
+});
+
+it('should override with higher value', function () {
+  var actual = mergeNumericMax({ a: 1 }, { a: 2, b: 2 });
+  expect(actual).toEqual({ a: 2, b: 2 });
+});
+
+it('should accept multiple sources', function () {
+  expect(mergeNumericMax({ a: 1, b: 2 }, { b: 3 }, { c: 3 })).toEqual({
+    a: 1,
+    b: 3,
+    c: 3,
+  });
+
+  expect(mergeNumericMax({ a: 1, b: 2 }, { b: 3, c: 3 }, { c: 2 })).toEqual({
+    a: 1,
+    b: 3,
+    c: 3,
+  });
+});
+
+it('should overwrite `null` values', function () {
+  var actual = mergeNumericMax({ a: null }, { a: 1 });
+  expect(actual.a).toBe(1);
+});
+
+it('should overwrite `undefined` values', function () {
+  var actual = mergeNumericMax({ a: undefined }, { a: 1 });
+  expect(actual.a).toBe(1);
+});
+
+it('should assign `undefined` values', function () {
+  var source = { a: undefined, b: 1 };
+  var actual = mergeNumericMax({}, source);
+
+  expect(actual).toEqual({ a: undefined, b: 1 });
+});
+
+it('should assign properties that shadow those on `Object.prototype`', function () {
+  expect(
+    mergeNumericMax(
+      {},
+      {
+        constructor: 1,
+        hasOwnProperty: 2,
+        isPrototypeOf: 3,
+        propertyIsEnumerable: 4,
+        toLocaleString: 5,
+        toString: 6,
+        valueOf: 7,
+      }
+    )
+  ).toEqual({
+    constructor: 1,
+    hasOwnProperty: 2,
+    isPrototypeOf: 3,
+    propertyIsEnumerable: 4,
+    toLocaleString: 5,
+    toString: 6,
+    valueOf: 7,
+  });
+
+  expect(
+    mergeNumericMax(
+      {},
+      {
+        constructor: Object.prototype.constructor,
+        hasOwnProperty: Object.prototype.hasOwnProperty,
+        isPrototypeOf: Object.prototype.isPrototypeOf,
+        propertyIsEnumerable: Object.prototype.propertyIsEnumerable,
+        toLocaleString: Object.prototype.toLocaleString,
+        toString: Object.prototype.toString,
+        valueOf: Object.prototype.valueOf,
+      },
+      {
+        constructor: 1,
+        hasOwnProperty: 2,
+        isPrototypeOf: 3,
+        propertyIsEnumerable: 4,
+        toLocaleString: 5,
+        toString: 6,
+        valueOf: 7,
+      }
+    )
+  ).toEqual({
+    constructor: 1,
+    hasOwnProperty: 2,
+    isPrototypeOf: 3,
+    propertyIsEnumerable: 4,
+    toLocaleString: 5,
+    toString: 6,
+    valueOf: 7,
+  });
+});
+
+it('should keep the keys order with facets', function () {
+  var actual = mergeNumericMax(
+    {},
+    {
+      'Insignia™': 551,
+      Samsung: 511,
+      Apple: 386,
+    },
+    {
+      Apple: 386,
+    }
+  );
+  expect(Object.keys(actual)).toEqual(['Insignia™', 'Samsung', 'Apple']);
+});
+
+it('does not pollute the prototype', () => {
+  var payload = JSON.parse('{"__proto__": {"polluted": "vulnerable to PP"}}');
+  var subject = {};
+
+  expect(subject.polluted).toBe(undefined);
+
+  const out = mergeNumericMax({}, payload);
+
+  expect(out).toEqual({});
+
+  expect({}.polluted).toBe(undefined);
+});