Skip to content

Commit

Permalink
fix(searchForFacetValues): only call client.search (#6465)
Browse files Browse the repository at this point in the history
This signature is the easiest to detect and makes the required signature of the searchClient simpler (only `search`, `getRecommendations` and `addAlgoliaAgent` are now used).

In this PR I also fix the signature of `search` in `SearchClient` type to make this usage fit reality.

BREAKING CHANGE: in helper.searchForFacetFalues only the method client.search is used, none of the previous fallbacks.
  • Loading branch information
Haroenv committed Dec 9, 2024
1 parent 8a95c6f commit 97de992
Show file tree
Hide file tree
Showing 12 changed files with 559 additions and 912 deletions.
58 changes: 12 additions & 46 deletions packages/algoliasearch-helper/src/algoliasearch.helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,21 +322,6 @@ AlgoliaSearchHelper.prototype.searchForFacetValues = function (
maxFacetHits,
userState
) {
var clientHasSFFV =
typeof this.client.searchForFacetValues === 'function' &&
// v5 has a wrong sffv signature
typeof this.client.searchForFacets !== 'function';
var clientHasInitIndex = typeof this.client.initIndex === 'function';
if (
!clientHasSFFV &&
!clientHasInitIndex &&
typeof this.client.search !== 'function'
) {
throw new Error(
'search for facet values (searchable) was called, but this client does not have a function client.searchForFacetValues or client.initIndex(index).searchForFacetValues'
);
}

var state = this.state.setQueryParameters(userState || {});
var isDisjunctive = state.isDisjunctiveFacet(facet);
var algoliaQuery = requestBuilder.getSearchForFacetQuery(
Expand All @@ -349,34 +334,15 @@ AlgoliaSearchHelper.prototype.searchForFacetValues = function (
this._currentNbQueries++;
// eslint-disable-next-line consistent-this
var self = this;
var searchForFacetValuesPromise;
// newer algoliasearch ^3.27.1 - ~4.0.0
if (clientHasSFFV) {
searchForFacetValuesPromise = this.client.searchForFacetValues([
{ indexName: state.index, params: algoliaQuery },
]);
// algoliasearch < 3.27.1
} else if (clientHasInitIndex) {
searchForFacetValuesPromise = this.client
.initIndex(state.index)
.searchForFacetValues(algoliaQuery);
// algoliasearch ~5.0.0
} else {
// @MAJOR only use client.search
delete algoliaQuery.facetName;
searchForFacetValuesPromise = this.client
.search([
{
type: 'facet',
facet: facet,
indexName: state.index,
params: algoliaQuery,
},
])
.then(function processResponse(response) {
return response.results[0];
});
}

var searchForFacetValuesPromise = this.client.search([
{
type: 'facet',
facet: facet,
indexName: state.index,
params: algoliaQuery,
},
]);

this.emit('searchForFacetValues', {
state: state,
Expand All @@ -389,16 +355,16 @@ AlgoliaSearchHelper.prototype.searchForFacetValues = function (
self._currentNbQueries--;
if (self._currentNbQueries === 0) self.emit('searchQueueEmpty');

content = Array.isArray(content) ? content[0] : content;
var result = content.results[0];

content.facetHits.forEach(function (f) {
result.facetHits.forEach(function (f) {
f.escapedValue = escapeFacetValue(f.value);
f.isRefined = isDisjunctive
? state.isDisjunctiveFacetRefined(facet, f.escapedValue)
: state.isFacetRefined(facet, f.escapedValue);
});

return content;
return result;
},
function (e) {
self._currentNbQueries--;
Expand Down
1 change: 0 additions & 1 deletion packages/algoliasearch-helper/src/requestBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,6 @@ var requestBuilder = {
: state;
var searchForFacetSearchParameters = {
facetQuery: query,
facetName: facetName,
};
if (typeof maxFacetHits === 'number') {
searchForFacetSearchParameters.maxFacetHits = maxFacetHits;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ function makeFakeClient() {
search: jest.fn(function () {
return new Promise(function () {});
}),
searchForFacetValues: jest.fn(function () {
return new Promise(function () {});
}),
};
}

Expand Down Expand Up @@ -297,7 +294,7 @@ test(
helper.on('searchForFacetValues', searchedForFacetValues);

expect(searchedForFacetValues).toHaveBeenCalledTimes(0);
expect(fakeClient.searchForFacetValues).toHaveBeenCalledTimes(0);
expect(fakeClient.search).toHaveBeenCalledTimes(0);

helper.searchForFacetValues('city', 'NYC');
expect(searchedForFacetValues).toHaveBeenCalledTimes(1);
Expand All @@ -306,7 +303,7 @@ test(
facet: 'city',
query: 'NYC',
});
expect(fakeClient.searchForFacetValues).toHaveBeenCalledTimes(1);
expect(fakeClient.search).toHaveBeenCalledTimes(1);
}
);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict';

var algoliasearch = require('algoliasearch');
var isV5 = (algoliasearch.apiClientVersion || '')[0] === '5';
algoliasearch = algoliasearch.algoliasearch || algoliasearch;

var algoliasearchHelper = require('../../../index');
Expand Down Expand Up @@ -72,85 +71,45 @@ test('When searchOnce with promises, hasPendingRequests is true', function (done
triggerCb();
});

if (!isV5) {
test('When searchForFacetValues, hasPendingRequests is true (v3, v4)', function (done) {
var client = algoliasearch('dsf', 'dsfdf');
test('When searchForFacetValues, hasPendingRequests is true', function (done) {
var client = algoliasearch('dsf', 'dsfdf');

let triggerCb;
client.searchForFacetValues = function () {
return new Promise(function (resolve) {
triggerCb = function () {
resolve([
let triggerCb;
client.search = function () {
return new Promise(function (resolve) {
triggerCb = function () {
resolve({
results: [
{
exhaustiveFacetsCount: true,
facetHits: [],
processingTimeMS: 3,
},
]);
};
});
};

var helper = algoliasearchHelper(client, 'test_hotels-node');
var countNoMoreSearch = 0;
helper.on('searchQueueEmpty', function () {
countNoMoreSearch += 1;
});

expect(helper.hasPendingRequests()).toBe(false);

helper.searchForFacetValues('').then(function () {
expect(helper.hasPendingRequests()).toBe(false);
expect(countNoMoreSearch).toBe(1);
done();
],
});
};
});
};

expect(helper.hasPendingRequests()).toBe(true);
expect(countNoMoreSearch).toBe(0);

triggerCb();
var helper = algoliasearchHelper(client, 'test_hotels-node');
var countNoMoreSearch = 0;
helper.on('searchQueueEmpty', function () {
countNoMoreSearch += 1;
});
} else {
test('When searchForFacetValues, hasPendingRequests is true (v5)', function (done) {
var client = algoliasearch('dsf', 'dsfdf');

let triggerCb;
client.search = function () {
return new Promise(function (resolve) {
triggerCb = function () {
resolve({
results: [
{
exhaustiveFacetsCount: true,
facetHits: [],
processingTimeMS: 3,
},
],
});
};
});
};

var helper = algoliasearchHelper(client, 'test_hotels-node');
var countNoMoreSearch = 0;
helper.on('searchQueueEmpty', function () {
countNoMoreSearch += 1;
});
expect(helper.hasPendingRequests()).toBe(false);

helper.searchForFacetValues('').then(function () {
expect(helper.hasPendingRequests()).toBe(false);
expect(countNoMoreSearch).toBe(1);
done();
});

helper.searchForFacetValues('').then(function () {
expect(helper.hasPendingRequests()).toBe(false);
expect(countNoMoreSearch).toBe(1);
done();
});

expect(helper.hasPendingRequests()).toBe(true);
expect(countNoMoreSearch).toBe(0);
expect(helper.hasPendingRequests()).toBe(true);
expect(countNoMoreSearch).toBe(0);

triggerCb();
});
}
triggerCb();
});

test('When helper.search(), hasPendingRequests is true', function (done) {
var testData = require('../../datasets/SearchParameters/search.dataset')();
Expand Down
Loading

0 comments on commit 97de992

Please sign in to comment.