From f3d2019748512a66c748da5d18750eaffd9e53b9 Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Mon, 24 Jul 2023 10:14:13 +0200 Subject: [PATCH] feat(caching): sort filters and queries (#5764) * WIP: sort request parameters - facetFilters - disjunctive queries sorted alphabetically through facet name This basically will allow a better cache hit rate. To figure out: - does this have a perf impact (likely not) - should it be an option - how do we ensure everything is sorted - are the right facet values read (no actually, it should be done in getRefinedDisjunctiveFacets etc. * move to search params, so it's used in result too * don't locale compare much slower, the default sort works just fine (the values are search parameter keys, so english) follow up on https://github.com/algolia/algoliasearch-helper-js/pull/911 * change existing tests * tests --- .../src/SearchParameters/index.js | 5 +- .../src/requestBuilder.js | 137 +++++++++--------- .../SearchParameters/search.dataset.js | 2 +- .../spec/algoliasearch.helper/getQuery.js | 2 +- .../with-a-disjunctive-facet.js | 2 +- .../test/spec/requestBuilder.js | 134 ++++++++++++++++- 6 files changed, 204 insertions(+), 78 deletions(-) diff --git a/packages/algoliasearch-helper/src/SearchParameters/index.js b/packages/algoliasearch-helper/src/SearchParameters/index.js index d2858c7473..4289c7a463 100644 --- a/packages/algoliasearch-helper/src/SearchParameters/index.js +++ b/packages/algoliasearch-helper/src/SearchParameters/index.js @@ -1446,7 +1446,8 @@ SearchParameters.prototype = { return self.disjunctiveFacetsRefinements[facet].length > 0; }) .concat(disjunctiveNumericRefinedFacets) - .concat(this.getRefinedHierarchicalFacets()); + .concat(this.getRefinedHierarchicalFacets()) + .sort(); }, /** * Returns the list of all disjunctive facets refined @@ -1467,7 +1468,7 @@ SearchParameters.prototype = { Object.keys(this.hierarchicalFacetsRefinements).filter(function (facet) { return self.hierarchicalFacetsRefinements[facet].length > 0; }) - ); + ).sort(); }, /** * Returned the list of all disjunctive facets not refined diff --git a/packages/algoliasearch-helper/src/requestBuilder.js b/packages/algoliasearch-helper/src/requestBuilder.js index 70b3e62249..b3fa36b065 100644 --- a/packages/algoliasearch-helper/src/requestBuilder.js +++ b/packages/algoliasearch-helper/src/requestBuilder.js @@ -4,9 +4,7 @@ var merge = require('./functions/merge'); function sortObject(obj) { return Object.keys(obj) - .sort(function (a, b) { - return a.localeCompare(b); - }) + .sort() .reduce(function (acc, curr) { acc[curr] = obj[curr]; return acc; @@ -135,7 +133,8 @@ var requestBuilder = { _getHitsSearchParams: function (state) { var facets = state.facets .concat(state.disjunctiveFacets) - .concat(requestBuilder._getHitsHierarchicalFacetsAttributes(state)); + .concat(requestBuilder._getHitsHierarchicalFacetsAttributes(state)) + .sort(); var facetFilters = requestBuilder._getFacetFilters(state); var numericFilters = requestBuilder._getNumericFilters(state); @@ -274,85 +273,93 @@ var requestBuilder = { var facetFilters = []; var facetsRefinements = state.facetsRefinements || {}; - Object.keys(facetsRefinements).forEach(function (facetName) { - var facetValues = facetsRefinements[facetName] || []; - facetValues.forEach(function (facetValue) { - facetFilters.push(facetName + ':' + facetValue); + Object.keys(facetsRefinements) + .sort() + .forEach(function (facetName) { + var facetValues = facetsRefinements[facetName] || []; + facetValues.sort().forEach(function (facetValue) { + facetFilters.push(facetName + ':' + facetValue); + }); }); - }); var facetsExcludes = state.facetsExcludes || {}; - Object.keys(facetsExcludes).forEach(function (facetName) { - var facetValues = facetsExcludes[facetName] || []; - facetValues.forEach(function (facetValue) { - facetFilters.push(facetName + ':-' + facetValue); + Object.keys(facetsExcludes) + .sort() + .forEach(function (facetName) { + var facetValues = facetsExcludes[facetName] || []; + facetValues.sort().forEach(function (facetValue) { + facetFilters.push(facetName + ':-' + facetValue); + }); }); - }); var disjunctiveFacetsRefinements = state.disjunctiveFacetsRefinements || {}; - Object.keys(disjunctiveFacetsRefinements).forEach(function (facetName) { - var facetValues = disjunctiveFacetsRefinements[facetName] || []; - if (facetName === facet || !facetValues || facetValues.length === 0) { - return; - } - var orFilters = []; + Object.keys(disjunctiveFacetsRefinements) + .sort() + .forEach(function (facetName) { + var facetValues = disjunctiveFacetsRefinements[facetName] || []; + if (facetName === facet || !facetValues || facetValues.length === 0) { + return; + } + var orFilters = []; - facetValues.forEach(function (facetValue) { - orFilters.push(facetName + ':' + facetValue); - }); + facetValues.sort().forEach(function (facetValue) { + orFilters.push(facetName + ':' + facetValue); + }); - facetFilters.push(orFilters); - }); + facetFilters.push(orFilters); + }); var hierarchicalFacetsRefinements = state.hierarchicalFacetsRefinements || {}; - Object.keys(hierarchicalFacetsRefinements).forEach(function (facetName) { - var facetValues = hierarchicalFacetsRefinements[facetName] || []; - var facetValue = facetValues[0]; + Object.keys(hierarchicalFacetsRefinements) + .sort() + .forEach(function (facetName) { + var facetValues = hierarchicalFacetsRefinements[facetName] || []; + var facetValue = facetValues[0]; - if (facetValue === undefined) { - return; - } - - var hierarchicalFacet = state.getHierarchicalFacetByName(facetName); - var separator = state._getHierarchicalFacetSeparator(hierarchicalFacet); - var rootPath = state._getHierarchicalRootPath(hierarchicalFacet); - var attributeToRefine; - var attributesIndex; - - // we ask for parent facet values only when the `facet` is the current hierarchical facet - if (facet === facetName) { - // if we are at the root level already, no need to ask for facet values, we get them from - // the hits query - if ( - facetValue.indexOf(separator) === -1 || - (!rootPath && hierarchicalRootLevel === true) || - (rootPath && - rootPath.split(separator).length === - facetValue.split(separator).length) - ) { + if (facetValue === undefined) { return; } - if (!rootPath) { - attributesIndex = facetValue.split(separator).length - 2; - facetValue = facetValue.slice(0, facetValue.lastIndexOf(separator)); - } else { - attributesIndex = rootPath.split(separator).length - 1; - facetValue = rootPath; - } + var hierarchicalFacet = state.getHierarchicalFacetByName(facetName); + var separator = state._getHierarchicalFacetSeparator(hierarchicalFacet); + var rootPath = state._getHierarchicalRootPath(hierarchicalFacet); + var attributeToRefine; + var attributesIndex; + + // we ask for parent facet values only when the `facet` is the current hierarchical facet + if (facet === facetName) { + // if we are at the root level already, no need to ask for facet values, we get them from + // the hits query + if ( + facetValue.indexOf(separator) === -1 || + (!rootPath && hierarchicalRootLevel === true) || + (rootPath && + rootPath.split(separator).length === + facetValue.split(separator).length) + ) { + return; + } - attributeToRefine = hierarchicalFacet.attributes[attributesIndex]; - } else { - attributesIndex = facetValue.split(separator).length - 1; + if (!rootPath) { + attributesIndex = facetValue.split(separator).length - 2; + facetValue = facetValue.slice(0, facetValue.lastIndexOf(separator)); + } else { + attributesIndex = rootPath.split(separator).length - 1; + facetValue = rootPath; + } - attributeToRefine = hierarchicalFacet.attributes[attributesIndex]; - } + attributeToRefine = hierarchicalFacet.attributes[attributesIndex]; + } else { + attributesIndex = facetValue.split(separator).length - 1; - if (attributeToRefine) { - facetFilters.push([attributeToRefine + ':' + facetValue]); - } - }); + attributeToRefine = hierarchicalFacet.attributes[attributesIndex]; + } + + if (attributeToRefine) { + facetFilters.push([attributeToRefine + ':' + facetValue]); + } + }); return facetFilters; }, diff --git a/packages/algoliasearch-helper/test/datasets/SearchParameters/search.dataset.js b/packages/algoliasearch-helper/test/datasets/SearchParameters/search.dataset.js index a46c600f3d..a821b01be9 100644 --- a/packages/algoliasearch-helper/test/datasets/SearchParameters/search.dataset.js +++ b/packages/algoliasearch-helper/test/datasets/SearchParameters/search.dataset.js @@ -188,7 +188,7 @@ function getData() { index: 'test_hotels-node', disjunctiveFacets: ['city'], disjunctiveFacetsRefinements: { - city: ['Paris', 'New York'], + city: ['New York', 'Paris'], }, }); diff --git a/packages/algoliasearch-helper/test/spec/algoliasearch.helper/getQuery.js b/packages/algoliasearch-helper/test/spec/algoliasearch.helper/getQuery.js index 28b029d641..32bcce69d8 100644 --- a/packages/algoliasearch-helper/test/spec/algoliasearch.helper/getQuery.js +++ b/packages/algoliasearch-helper/test/spec/algoliasearch.helper/getQuery.js @@ -23,7 +23,7 @@ test('getQuery', function () { expect(helper.getQuery()).toEqual({ minWordSizefor1Typo: 8, ignorePlurals: true, - facets: ['facet1', 'facet2', 'facet3', 'df1', 'df2', 'df3'], + facets: ['df1', 'df2', 'df3', 'facet1', 'facet2', 'facet3'], tagFilters: '', facetFilters: [ 'facet1:FACET1-VAL-1', diff --git a/packages/algoliasearch-helper/test/spec/hierarchical-facets/with-a-disjunctive-facet.js b/packages/algoliasearch-helper/test/spec/hierarchical-facets/with-a-disjunctive-facet.js index e9118d60c1..2d97bc3731 100644 --- a/packages/algoliasearch-helper/test/spec/hierarchical-facets/with-a-disjunctive-facet.js +++ b/packages/algoliasearch-helper/test/spec/hierarchical-facets/with-a-disjunctive-facet.js @@ -29,7 +29,7 @@ test('hierarchical facets: combined with a disjunctive facet', function () { helper.setQuery('a').search(); - var disjunctiveFacetsValuesQuery = client.search.mock.calls[0][0][1]; + var disjunctiveFacetsValuesQuery = client.search.mock.calls[0][0][2]; expect(disjunctiveFacetsValuesQuery.params.facetFilters).toEqual([ ['categories.lvl1:beers > IPA'], diff --git a/packages/algoliasearch-helper/test/spec/requestBuilder.js b/packages/algoliasearch-helper/test/spec/requestBuilder.js index 42f94ae76a..2b6bcab70d 100644 --- a/packages/algoliasearch-helper/test/spec/requestBuilder.js +++ b/packages/algoliasearch-helper/test/spec/requestBuilder.js @@ -176,12 +176,10 @@ test('orders parameters alphabetically in every query', function () { 'this is last in parameters, but first in queries', ], clickAnalytics: false, - facetFilters: [ - ['test_disjunctive:test_disjunctive_value'], - ['whatever:item'], - ], - facets: 'test_numeric', + facetFilters: [['test_disjunctive:test_disjunctive_value']], + facets: ['whatever'], hitsPerPage: 0, + numericFilters: ['test_numeric>=10'], page: 0, }) ); @@ -192,10 +190,12 @@ test('orders parameters alphabetically in every query', function () { 'this is last in parameters, but first in queries', ], clickAnalytics: false, - facetFilters: [['test_disjunctive:test_disjunctive_value']], - facets: ['whatever'], + facetFilters: [ + ['test_disjunctive:test_disjunctive_value'], + ['whatever:item'], + ], + facets: 'test_numeric', hitsPerPage: 0, - numericFilters: ['test_numeric>=10'], page: 0, }) ); @@ -269,3 +269,121 @@ describe('wildcard facets', function () { expect(queries[1].params.facets).toEqual('test_disjunctive'); }); }); + +describe('request ordering', function () { + test('should order queries and facet values by name', function () { + // These facets are all sorted reverse-alphabetically + var searchParams = new SearchParameters({ + facets: [ + 'c_test_conjunctive_2', + 'c_conjunctive_1', + 'd_exclude_2', + 'd_exclude_1', + ], + disjunctiveFacets: ['b_disjunctive_2', 'b_disjunctive_1'], + hierarchicalFacets: [ + { + name: 'a_hierarchical_2', + attributes: ['a_hierarchical_2'], + }, + { + name: 'a_hierarchical_1', + attributes: ['a_hierarchical_1'], + }, + ], + hierarchicalFacetsRefinements: { + a_hierarchical_2: ['beta'], + a_hierarchical_1: ['alpha'], + }, + disjunctiveFacetsRefinements: { + b_disjunctive_2: ['beta', 'alpha'], + b_disjunctive_1: ['beta', 'alpha'], + }, + facetsRefinements: { + c_conjunctive_2: ['beta', 'alpha'], + c_conjunctive_1: ['beta', 'alpha'], + }, + facetsExcludes: { + d_exclude_2: ['beta', 'alpha'], + d_exclude_1: ['beta', 'alpha'], + }, + }); + + var queries = getQueries(searchParams.index, searchParams); + + // Order of filters is alphabetical within type + // Order of queries is alphabetical within type + expect(queries.length).toBe(5); + // Hits + expect(queries[0].params.facetFilters).toEqual([ + 'c_conjunctive_1:alpha', + 'c_conjunctive_1:beta', + 'c_conjunctive_2:alpha', + 'c_conjunctive_2:beta', + 'd_exclude_1:-alpha', + 'd_exclude_1:-beta', + 'd_exclude_2:-alpha', + 'd_exclude_2:-beta', + ['b_disjunctive_1:alpha', 'b_disjunctive_1:beta'], + ['b_disjunctive_2:alpha', 'b_disjunctive_2:beta'], + ['a_hierarchical_1:alpha'], + ['a_hierarchical_2:beta'], + ]); + // Hierarchical 1 + expect(queries[1].params.facetFilters).toEqual([ + 'c_conjunctive_1:alpha', + 'c_conjunctive_1:beta', + 'c_conjunctive_2:alpha', + 'c_conjunctive_2:beta', + 'd_exclude_1:-alpha', + 'd_exclude_1:-beta', + 'd_exclude_2:-alpha', + 'd_exclude_2:-beta', + ['b_disjunctive_1:alpha', 'b_disjunctive_1:beta'], + ['b_disjunctive_2:alpha', 'b_disjunctive_2:beta'], + ['a_hierarchical_2:beta'], + ]); + // Hierarchical 2 + expect(queries[2].params.facetFilters).toEqual([ + 'c_conjunctive_1:alpha', + 'c_conjunctive_1:beta', + 'c_conjunctive_2:alpha', + 'c_conjunctive_2:beta', + 'd_exclude_1:-alpha', + 'd_exclude_1:-beta', + 'd_exclude_2:-alpha', + 'd_exclude_2:-beta', + ['b_disjunctive_1:alpha', 'b_disjunctive_1:beta'], + ['b_disjunctive_2:alpha', 'b_disjunctive_2:beta'], + ['a_hierarchical_1:alpha'], + ]); + // Disjunctive 1 + expect(queries[3].params.facetFilters).toEqual([ + 'c_conjunctive_1:alpha', + 'c_conjunctive_1:beta', + 'c_conjunctive_2:alpha', + 'c_conjunctive_2:beta', + 'd_exclude_1:-alpha', + 'd_exclude_1:-beta', + 'd_exclude_2:-alpha', + 'd_exclude_2:-beta', + ['b_disjunctive_2:alpha', 'b_disjunctive_2:beta'], + ['a_hierarchical_1:alpha'], + ['a_hierarchical_2:beta'], + ]); + // Disjunctive 2 + expect(queries[4].params.facetFilters).toEqual([ + 'c_conjunctive_1:alpha', + 'c_conjunctive_1:beta', + 'c_conjunctive_2:alpha', + 'c_conjunctive_2:beta', + 'd_exclude_1:-alpha', + 'd_exclude_1:-beta', + 'd_exclude_2:-alpha', + 'd_exclude_2:-beta', + ['b_disjunctive_1:alpha', 'b_disjunctive_1:beta'], + ['a_hierarchical_1:alpha'], + ['a_hierarchical_2:beta'], + ]); + }); +});