From ac1959cda31539e3a276ae0f55de8f36343921cb Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Mon, 27 Feb 2023 17:37:15 +0100 Subject: [PATCH 1/4] fix(userToken): prevent search waterfall This makes sure when the same userToken is set multiple times, it doesn't cause multiple search requests, as could be the case if something would call setUserToken without condition. Discovered in #5500, but should be merged separately. --- .../__tests__/createInsightsMiddleware.ts | 28 +++++++++++++++++++ .../middlewares/createInsightsMiddleware.ts | 13 ++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts b/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts index 857e88de0d..de8aabef33 100644 --- a/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts +++ b/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts @@ -16,6 +16,8 @@ import type { JSDOM } from 'jsdom'; import type { PlainSearchParameters } from 'algoliasearch-helper'; import { fireEvent } from '@testing-library/dom'; import { createInstantSearch } from '../../../test/createInstantSearch'; +import { castToJestMock } from '@instantsearch/testutils'; +import { connectSearchBox } from '../../connectors'; declare const jsdom: JSDOM; @@ -43,6 +45,7 @@ describe('insights', () => { started = true, insights = false, } = {}) => { + castToJestMock(searchClient.search).mockClear(); const { analytics, insightsClient } = createInsights(); const indexName = 'my-index'; const instantSearchInstance = instantsearch({ @@ -621,6 +624,31 @@ See documentation: https://www.algolia.com/doc/guides/building-search-ui/going-f ).toEqual('def'); }); + it('searches once per unique userToken', async () => { + const { insightsClient, instantSearchInstance, getUserToken } = + createTestEnvironment(); + + instantSearchInstance.addWidgets([connectSearchBox(() => ({}))({})]); + + await wait(0); + expect(instantSearchInstance.client.search).toHaveBeenCalledTimes(1); + + insightsClient('setUserToken', 'abc'); + instantSearchInstance.use( + createInsightsMiddleware({ + insightsClient, + }) + ); + + await wait(0); + expect(instantSearchInstance.client.search).toHaveBeenCalledTimes(2); + + insightsClient('setUserToken', 'abc'); + + await wait(0); + expect(instantSearchInstance.client.search).toHaveBeenCalledTimes(2); + }); + describe('umd', () => { it('applies userToken from queue if exists', () => { const { diff --git a/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts b/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts index cd098d1958..982b4c8c0d 100644 --- a/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts +++ b/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts @@ -186,12 +186,21 @@ export function createInsightsMiddleware< } const setUserTokenToSearch = (userToken?: string) => { + const existingToken = (helper.state as PlainSearchParameters) + .userToken; + + if (!userToken) { + return; + } + helper.overrideStateWithoutTriggeringChangeEvent({ ...helper.state, userToken, }); - instantSearchInstance.scheduleSearch(); + if (existingToken && existingToken !== userToken) { + instantSearchInstance.scheduleSearch(); + } }; const anonymousUserToken = getInsightsAnonymousUserTokenInternal(); @@ -204,8 +213,10 @@ export function createInsightsMiddleware< // We consider the `userToken` coming from a `init` call to have a higher // importance than the one coming from the queue. if (userTokenBeforeInit) { + setUserTokenToSearch(userTokenBeforeInit); insightsClient('setUserToken', userTokenBeforeInit); } else if (queuedUserToken) { + setUserTokenToSearch(queuedUserToken); insightsClient('setUserToken', queuedUserToken); } From 75842dc99a4ec269a58753bb4bf5e55e732c3b6a Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Tue, 28 Feb 2023 11:52:18 +0100 Subject: [PATCH 2/4] Update packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts Co-authored-by: Fabien Motte --- .../src/middlewares/__tests__/createInsightsMiddleware.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts b/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts index de8aabef33..5a25131a26 100644 --- a/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts +++ b/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts @@ -625,7 +625,7 @@ See documentation: https://www.algolia.com/doc/guides/building-search-ui/going-f }); it('searches once per unique userToken', async () => { - const { insightsClient, instantSearchInstance, getUserToken } = + const { insightsClient, instantSearchInstance } = createTestEnvironment(); instantSearchInstance.addWidgets([connectSearchBox(() => ({}))({})]); From 362dc0bd3bc99a477983dc28dcaeaf59aa9d7bed Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Tue, 28 Feb 2023 11:54:03 +0100 Subject: [PATCH 3/4] Update createInsightsMiddleware.ts --- .../src/middlewares/createInsightsMiddleware.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts b/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts index 982b4c8c0d..a8b13e560f 100644 --- a/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts +++ b/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts @@ -186,13 +186,13 @@ export function createInsightsMiddleware< } const setUserTokenToSearch = (userToken?: string) => { - const existingToken = (helper.state as PlainSearchParameters) - .userToken; - if (!userToken) { return; } + const existingToken = (helper.state as PlainSearchParameters) + .userToken; + helper.overrideStateWithoutTriggeringChangeEvent({ ...helper.state, userToken, From cc0277530454e9f77252fa50279f2bbc61f40af4 Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Tue, 28 Feb 2023 13:38:30 +0100 Subject: [PATCH 4/4] test --- .../__tests__/createInsightsMiddleware.ts | 53 +++++++++++++++++-- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts b/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts index 5a25131a26..0030ae9a4c 100644 --- a/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts +++ b/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts @@ -110,6 +110,8 @@ describe('insights', () => { (window as any).aa = undefined; document.body.innerHTML = ''; + + document.cookie = '_ALGOLIA=;'; }); describe('usage', () => { @@ -625,8 +627,7 @@ See documentation: https://www.algolia.com/doc/guides/building-search-ui/going-f }); it('searches once per unique userToken', async () => { - const { insightsClient, instantSearchInstance } = - createTestEnvironment(); + const { insightsClient, instantSearchInstance } = createTestEnvironment(); instantSearchInstance.addWidgets([connectSearchBox(() => ({}))({})]); @@ -649,6 +650,52 @@ See documentation: https://www.algolia.com/doc/guides/building-search-ui/going-f expect(instantSearchInstance.client.search).toHaveBeenCalledTimes(2); }); + it("doesn't search when userToken is falsy", async () => { + const { insightsClient, instantSearchInstance } = createTestEnvironment(); + + instantSearchInstance.addWidgets([connectSearchBox(() => ({}))({})]); + + await wait(0); + expect(instantSearchInstance.client.search).toHaveBeenCalledTimes(1); + expect(instantSearchInstance.client.search).toHaveBeenLastCalledWith([ + { + indexName: 'my-index', + params: { + facets: [], + query: '', + tagFilters: '', + }, + }, + ]); + + insightsClient('setUserToken', 0); + instantSearchInstance.use( + createInsightsMiddleware({ + insightsClient, + insightsInitParams: { useCookie: false }, + }) + ); + + await wait(0); + expect(instantSearchInstance.client.search).toHaveBeenCalledTimes(2); + expect(instantSearchInstance.client.search).toHaveBeenLastCalledWith([ + { + indexName: 'my-index', + params: { + clickAnalytics: true, + facets: [], + query: '', + tagFilters: '', + }, + }, + ]); + + insightsClient('setUserToken', undefined); + + await wait(0); + expect(instantSearchInstance.client.search).toHaveBeenCalledTimes(2); + }); + describe('umd', () => { it('applies userToken from queue if exists', () => { const { @@ -844,8 +891,6 @@ See documentation: https://www.algolia.com/doc/guides/building-search-ui/going-f await wait(100); // url should not get cleared expect(document.location.href).toEqual(url); - - document.cookie = ''; }); test('does not throw error when document or cookie are undefined', () => {