diff --git a/bundlesize.config.json b/bundlesize.config.json index fb7a017a58..74457c551e 100644 --- a/bundlesize.config.json +++ b/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", diff --git a/packages/algoliasearch-helper/src/SearchResults/index.js b/packages/algoliasearch-helper/src/SearchResults/index.js index d607526adb..6689126a74 100644 --- a/packages/algoliasearch-helper/src/SearchResults/index.js +++ b/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( diff --git a/packages/algoliasearch-helper/src/functions/mergeNumericMax.js b/packages/algoliasearch-helper/src/functions/mergeNumericMax.js new file mode 100644 index 0000000000..521a16c9c8 --- /dev/null +++ b/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; diff --git a/packages/algoliasearch-helper/test/spec/SearchResults/getFacetValues/disjunctive-non-exhaustive.json b/packages/algoliasearch-helper/test/spec/SearchResults/getFacetValues/disjunctive-non-exhaustive.json index edf81b0a5d..89774d7de4 100644 --- a/packages/algoliasearch-helper/test/spec/SearchResults/getFacetValues/disjunctive-non-exhaustive.json +++ b/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, diff --git a/packages/algoliasearch-helper/test/spec/SearchResults/getFacetValues/hierarchical-non-exhaustive.json b/packages/algoliasearch-helper/test/spec/SearchResults/getFacetValues/hierarchical-non-exhaustive.json index 5e11cd2050..1645855666 100644 --- a/packages/algoliasearch-helper/test/spec/SearchResults/getFacetValues/hierarchical-non-exhaustive.json +++ b/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, diff --git a/packages/algoliasearch-helper/test/spec/functions/defaultsPure.js b/packages/algoliasearch-helper/test/spec/functions/defaultsPure.js index 49bc0eeea9..fea13b9e7f 100644 --- a/packages/algoliasearch-helper/test/spec/functions/defaultsPure.js +++ b/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({}); diff --git a/packages/algoliasearch-helper/test/spec/functions/mergeNumericMax.js b/packages/algoliasearch-helper/test/spec/functions/mergeNumericMax.js new file mode 100644 index 0000000000..1d3fcfcc4e --- /dev/null +++ b/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); +});