From 9b78a88f79d0227b2c38ad0723504c732e03196d Mon Sep 17 00:00:00 2001 From: Steve Blaurock Date: Mon, 18 Dec 2023 17:12:47 -0700 Subject: [PATCH 1/7] Do not trim autocomplete queries, add test cases for expected behavior --- spec/src/modules/autocomplete.js | 34 ++++++++++++++++++++++++++++++++ spec/src/modules/search.js | 25 +++++++++++++++++++++++ src/modules/autocomplete.js | 2 +- src/modules/search.js | 5 ++++- 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/spec/src/modules/autocomplete.js b/spec/src/modules/autocomplete.js index d2b0c4de..561013ea 100644 --- a/spec/src/modules/autocomplete.js +++ b/spec/src/modules/autocomplete.js @@ -400,6 +400,22 @@ describe('ConstructorIO - Autocomplete', () => { }); }); + it('Should return a response with a valid query consisting of only non-breaking spaces', (done) => { + const { autocomplete } = new ConstructorIO({ + apiKey: testApiKey, + fetch: fetchSpy, + }); + + autocomplete.getAutocompleteResults(' ').then((res) => { + const requestedUrlParams = helpers.extractUrlParamsFromFetch(fetchSpy); + + expect(res).to.have.property('request').to.be.an('object'); + expect(res).to.have.property('sections').to.be.an('object'); + expect(res).to.have.property('result_id').to.be.an('string'); + done(); + }); + }); + it('Should return a variations_map object in the response', (done) => { const variationsMap = { group_by: [ @@ -473,6 +489,24 @@ describe('ConstructorIO - Autocomplete', () => { }); }); + it('Should not trim spaces from query', (done) => { + const queryWithSpaces = ` ${query} `; + const { autocomplete } = new ConstructorIO({ + apiKey: testApiKey, + fetch: fetchSpy, + }); + + autocomplete.getAutocompleteResults(queryWithSpaces).then((res) => { + const requestedUrlParams = helpers.extractUrlParamsFromFetch(fetchSpy); + + expect(res.request.term).to.equal(queryWithSpaces); + expect(res).to.have.property('request').to.be.an('object'); + expect(res).to.have.property('sections').to.be.an('object'); + expect(res).to.have.property('result_id').to.be.an('string'); + done(); + }); + }); + it('Should return a response with a / query', (done) => { const { autocomplete } = new ConstructorIO({ apiKey: testApiKey, diff --git a/spec/src/modules/search.js b/spec/src/modules/search.js index 3f797608..71bfb797 100644 --- a/spec/src/modules/search.js +++ b/spec/src/modules/search.js @@ -6,6 +6,7 @@ const sinon = require('sinon'); const sinonChai = require('sinon-chai'); const ConstructorIO = require('../../../test/constructorio'); // eslint-disable-line import/extensions const helpers = require('../../mocha.helpers'); +const utilsHelpers = require('../../../src/utils/helpers'); const nodeFetch = (...args) => import('node-fetch').then(({ default: fetch }) => fetch(...args)); @@ -563,6 +564,24 @@ describe('ConstructorIO - Search', () => { }); }); + it('Should trim non-breaking spaces from query', (done) => { + const queryWithSpaces = ` ${query} `; + const { search } = new ConstructorIO({ + apiKey: testApiKey, + fetch: fetchSpy, + }); + + search.getSearchResults(queryWithSpaces).then((res) => { + const requestedUrlParams = helpers.extractUrlParamsFromFetch(fetchSpy); + + expect(res.request.term).to.equal(utilsHelpers.trimNonBreakingSpaces(queryWithSpaces)); + expect(res).to.have.property('request').to.be.an('object'); + expect(res).to.have.property('response').to.be.an('object'); + expect(res).to.have.property('result_id').to.be.an('string'); + done(); + }); + }); + it('Should pass the correct custom headers passed in function networkParameters', (done) => { const { search } = new ConstructorIO({ ...validOptions, @@ -664,6 +683,12 @@ describe('ConstructorIO - Search', () => { return expect(search.getSearchResults(null, { section })).to.eventually.be.rejected; }); + it('Should be rejected when query consisting of only non-breaking spaces is provided', () => { + const { search } = new ConstructorIO(validOptions); + + return expect(search.getSearchResults(' ', { section })).to.eventually.be.rejected; + }); + it('Should be rejected when invalid page parameter is provided', () => { const { search } = new ConstructorIO(validOptions); const searchParams = { diff --git a/src/modules/autocomplete.js b/src/modules/autocomplete.js index 9b8f2808..a58932f1 100644 --- a/src/modules/autocomplete.js +++ b/src/modules/autocomplete.js @@ -112,7 +112,7 @@ function createAutocompleteUrl(query, parameters, userParameters, options) { const queryString = qs.stringify(queryParams, { indices: false }); const cleanedQuery = query.replace(/^\//, '|'); // For compatibility with backend API - return `${serviceUrl}/autocomplete/${helpers.encodeURIComponentRFC3986(helpers.trimNonBreakingSpaces(cleanedQuery))}?${queryString}`; + return `${serviceUrl}/autocomplete/${helpers.encodeURIComponentRFC3986(cleanedQuery)}?${queryString}`; } /** diff --git a/src/modules/search.js b/src/modules/search.js index 3fa829ed..d774391a 100644 --- a/src/modules/search.js +++ b/src/modules/search.js @@ -25,6 +25,9 @@ function createSearchUrl(query, parameters, userParameters, options, isVoiceSear queryParams.i = clientId; queryParams.s = sessionId; + // Trim non breaking spaces from query + query = helpers.trimNonBreakingSpaces(query); + // Validate query (term) is provided if (!query || typeof query !== 'string') { throw new Error('query is a required parameter of type string'); @@ -145,7 +148,7 @@ function createSearchUrl(query, parameters, userParameters, options, isVoiceSear const searchUrl = isVoiceSearch ? 'search/natural_language' : 'search'; - return `${serviceUrl}/${searchUrl}/${helpers.encodeURIComponentRFC3986(helpers.trimNonBreakingSpaces(query))}?${queryString}`; + return `${serviceUrl}/${searchUrl}/${helpers.encodeURIComponentRFC3986(query)}?${queryString}`; } /** From f25457a68bb62ed2d48a2591b98a75001d8387e3 Mon Sep 17 00:00:00 2001 From: Steve Blaurock Date: Mon, 18 Dec 2023 17:25:56 -0700 Subject: [PATCH 2/7] Lint fixes, adjust implementation and add tests --- spec/src/modules/autocomplete.js | 10 ++++++---- spec/src/modules/search.js | 2 -- src/modules/autocomplete.js | 8 ++++++-- src/modules/search.js | 8 ++++---- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/spec/src/modules/autocomplete.js b/spec/src/modules/autocomplete.js index 561013ea..a8e3ee0d 100644 --- a/spec/src/modules/autocomplete.js +++ b/spec/src/modules/autocomplete.js @@ -407,8 +407,6 @@ describe('ConstructorIO - Autocomplete', () => { }); autocomplete.getAutocompleteResults(' ').then((res) => { - const requestedUrlParams = helpers.extractUrlParamsFromFetch(fetchSpy); - expect(res).to.have.property('request').to.be.an('object'); expect(res).to.have.property('sections').to.be.an('object'); expect(res).to.have.property('result_id').to.be.an('string'); @@ -497,8 +495,6 @@ describe('ConstructorIO - Autocomplete', () => { }); autocomplete.getAutocompleteResults(queryWithSpaces).then((res) => { - const requestedUrlParams = helpers.extractUrlParamsFromFetch(fetchSpy); - expect(res.request.term).to.equal(queryWithSpaces); expect(res).to.have.property('request').to.be.an('object'); expect(res).to.have.property('sections').to.be.an('object'); @@ -627,6 +623,12 @@ describe('ConstructorIO - Autocomplete', () => { return expect(autocomplete.getAutocompleteResults(null)).to.eventually.be.rejected; }); + it('Should be rejected when query consisting of only non-breaking spaces is provided', () => { + const { autocomplete } = new ConstructorIO(validOptions); + + return expect(autocomplete.getAutocompleteResults(' ')).to.eventually.be.rejected; + }); + it('Should be rejected when invalid numResults parameter is provided', () => { const { autocomplete } = new ConstructorIO(validOptions); diff --git a/spec/src/modules/search.js b/spec/src/modules/search.js index 71bfb797..a52d1372 100644 --- a/spec/src/modules/search.js +++ b/spec/src/modules/search.js @@ -572,8 +572,6 @@ describe('ConstructorIO - Search', () => { }); search.getSearchResults(queryWithSpaces).then((res) => { - const requestedUrlParams = helpers.extractUrlParamsFromFetch(fetchSpy); - expect(res.request.term).to.equal(utilsHelpers.trimNonBreakingSpaces(queryWithSpaces)); expect(res).to.have.property('request').to.be.an('object'); expect(res).to.have.property('response').to.be.an('object'); diff --git a/src/modules/autocomplete.js b/src/modules/autocomplete.js index a58932f1..736e36d9 100644 --- a/src/modules/autocomplete.js +++ b/src/modules/autocomplete.js @@ -23,8 +23,11 @@ function createAutocompleteUrl(query, parameters, userParameters, options) { queryParams.i = clientId; queryParams.s = sessionId; - // Validate query (term) is provided - if (!query || typeof query !== 'string') { + // Trim non breaking spaces from query + const queryTrimmed = helpers.trimNonBreakingSpaces(query); + + // Validate query (term) is provided and consists of more than non-breaking spaces + if (!queryTrimmed || typeof queryTrimmed !== 'string') { throw new Error('query is a required parameter of type string'); } @@ -112,6 +115,7 @@ function createAutocompleteUrl(query, parameters, userParameters, options) { const queryString = qs.stringify(queryParams, { indices: false }); const cleanedQuery = query.replace(/^\//, '|'); // For compatibility with backend API + // Note: it is intentional that query is dispatched without being trimmed (`queryTrimmed`) return `${serviceUrl}/autocomplete/${helpers.encodeURIComponentRFC3986(cleanedQuery)}?${queryString}`; } diff --git a/src/modules/search.js b/src/modules/search.js index d774391a..c63c7cd3 100644 --- a/src/modules/search.js +++ b/src/modules/search.js @@ -26,10 +26,10 @@ function createSearchUrl(query, parameters, userParameters, options, isVoiceSear queryParams.s = sessionId; // Trim non breaking spaces from query - query = helpers.trimNonBreakingSpaces(query); + const queryTrimmed = helpers.trimNonBreakingSpaces(query); - // Validate query (term) is provided - if (!query || typeof query !== 'string') { + // Validate query (term) is provided and consists of more than non-breaking spaces + if (!queryTrimmed || typeof queryTrimmed !== 'string') { throw new Error('query is a required parameter of type string'); } @@ -148,7 +148,7 @@ function createSearchUrl(query, parameters, userParameters, options, isVoiceSear const searchUrl = isVoiceSearch ? 'search/natural_language' : 'search'; - return `${serviceUrl}/${searchUrl}/${helpers.encodeURIComponentRFC3986(query)}?${queryString}`; + return `${serviceUrl}/${searchUrl}/${helpers.encodeURIComponentRFC3986(queryTrimmed)}?${queryString}`; } /** From c741aba8300e524605381846526e1b155c7f533d Mon Sep 17 00:00:00 2001 From: Steve Blaurock Date: Mon, 18 Dec 2023 17:33:32 -0700 Subject: [PATCH 3/7] Remove outdated test --- spec/src/modules/autocomplete.js | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/spec/src/modules/autocomplete.js b/spec/src/modules/autocomplete.js index a8e3ee0d..94a03f7d 100644 --- a/spec/src/modules/autocomplete.js +++ b/spec/src/modules/autocomplete.js @@ -400,20 +400,6 @@ describe('ConstructorIO - Autocomplete', () => { }); }); - it('Should return a response with a valid query consisting of only non-breaking spaces', (done) => { - const { autocomplete } = new ConstructorIO({ - apiKey: testApiKey, - fetch: fetchSpy, - }); - - autocomplete.getAutocompleteResults(' ').then((res) => { - expect(res).to.have.property('request').to.be.an('object'); - expect(res).to.have.property('sections').to.be.an('object'); - expect(res).to.have.property('result_id').to.be.an('string'); - done(); - }); - }); - it('Should return a variations_map object in the response', (done) => { const variationsMap = { group_by: [ From 73cc2a852e6b9f65c82407db49f25c82a623cd9f Mon Sep 17 00:00:00 2001 From: Steve Blaurock Date: Mon, 18 Dec 2023 17:36:20 -0700 Subject: [PATCH 4/7] Update test description for consistency --- spec/src/modules/autocomplete.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/src/modules/autocomplete.js b/spec/src/modules/autocomplete.js index 94a03f7d..b9df4ec1 100644 --- a/spec/src/modules/autocomplete.js +++ b/spec/src/modules/autocomplete.js @@ -473,7 +473,7 @@ describe('ConstructorIO - Autocomplete', () => { }); }); - it('Should not trim spaces from query', (done) => { + it('Should not trim non-breaking spaces from query', (done) => { const queryWithSpaces = ` ${query} `; const { autocomplete } = new ConstructorIO({ apiKey: testApiKey, From 15efffab3d385a955faaf11ef91ad112e8962f62 Mon Sep 17 00:00:00 2001 From: Steve Blaurock Date: Tue, 19 Dec 2023 15:10:14 -0700 Subject: [PATCH 5/7] Do not trim spaces from search and autocomplete requests --- spec/src/modules/autocomplete.js | 8 +------- spec/src/modules/search.js | 22 ++++++++++++++++------ src/modules/autocomplete.js | 9 +++------ src/modules/search.js | 10 ++++------ 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/spec/src/modules/autocomplete.js b/spec/src/modules/autocomplete.js index b9df4ec1..b89f1770 100644 --- a/spec/src/modules/autocomplete.js +++ b/spec/src/modules/autocomplete.js @@ -473,7 +473,7 @@ describe('ConstructorIO - Autocomplete', () => { }); }); - it('Should not trim non-breaking spaces from query', (done) => { + it.only('Should not trim spaces from query', (done) => { const queryWithSpaces = ` ${query} `; const { autocomplete } = new ConstructorIO({ apiKey: testApiKey, @@ -609,12 +609,6 @@ describe('ConstructorIO - Autocomplete', () => { return expect(autocomplete.getAutocompleteResults(null)).to.eventually.be.rejected; }); - it('Should be rejected when query consisting of only non-breaking spaces is provided', () => { - const { autocomplete } = new ConstructorIO(validOptions); - - return expect(autocomplete.getAutocompleteResults(' ')).to.eventually.be.rejected; - }); - it('Should be rejected when invalid numResults parameter is provided', () => { const { autocomplete } = new ConstructorIO(validOptions); diff --git a/spec/src/modules/search.js b/spec/src/modules/search.js index a52d1372..927d0e0a 100644 --- a/spec/src/modules/search.js +++ b/spec/src/modules/search.js @@ -543,6 +543,22 @@ describe('ConstructorIO - Search', () => { }); }); + it.only('Should not trim spaces from query', (done) => { + const queryWithSpaces = ` ${query} `; + const { search } = new ConstructorIO({ + apiKey: testApiKey, + fetch: fetchSpy, + }); + + search.getSearchResults(queryWithSpaces).then((res) => { + expect(res.request.term).to.equal(queryWithSpaces); + expect(res).to.have.property('request').to.be.an('object'); + expect(res).to.have.property('response').to.be.an('object'); + expect(res).to.have.property('result_id').to.be.an('string'); + done(); + }); + }); + it('Should properly transform non-breaking spaces in parameters', (done) => { const breakingSpaces = ' '; const sortBy = `relevance ${breakingSpaces} relevance`; @@ -681,12 +697,6 @@ describe('ConstructorIO - Search', () => { return expect(search.getSearchResults(null, { section })).to.eventually.be.rejected; }); - it('Should be rejected when query consisting of only non-breaking spaces is provided', () => { - const { search } = new ConstructorIO(validOptions); - - return expect(search.getSearchResults(' ', { section })).to.eventually.be.rejected; - }); - it('Should be rejected when invalid page parameter is provided', () => { const { search } = new ConstructorIO(validOptions); const searchParams = { diff --git a/src/modules/autocomplete.js b/src/modules/autocomplete.js index 736e36d9..c0c06771 100644 --- a/src/modules/autocomplete.js +++ b/src/modules/autocomplete.js @@ -23,11 +23,8 @@ function createAutocompleteUrl(query, parameters, userParameters, options) { queryParams.i = clientId; queryParams.s = sessionId; - // Trim non breaking spaces from query - const queryTrimmed = helpers.trimNonBreakingSpaces(query); - - // Validate query (term) is provided and consists of more than non-breaking spaces - if (!queryTrimmed || typeof queryTrimmed !== 'string') { + // Validate query (term) is provided + if (!query || typeof query !== 'string') { throw new Error('query is a required parameter of type string'); } @@ -115,7 +112,7 @@ function createAutocompleteUrl(query, parameters, userParameters, options) { const queryString = qs.stringify(queryParams, { indices: false }); const cleanedQuery = query.replace(/^\//, '|'); // For compatibility with backend API - // Note: it is intentional that query is dispatched without being trimmed (`queryTrimmed`) + // Note: it is intentional that query is dispatched without being trimmed return `${serviceUrl}/autocomplete/${helpers.encodeURIComponentRFC3986(cleanedQuery)}?${queryString}`; } diff --git a/src/modules/search.js b/src/modules/search.js index c63c7cd3..9d94baef 100644 --- a/src/modules/search.js +++ b/src/modules/search.js @@ -25,11 +25,8 @@ function createSearchUrl(query, parameters, userParameters, options, isVoiceSear queryParams.i = clientId; queryParams.s = sessionId; - // Trim non breaking spaces from query - const queryTrimmed = helpers.trimNonBreakingSpaces(query); - - // Validate query (term) is provided and consists of more than non-breaking spaces - if (!queryTrimmed || typeof queryTrimmed !== 'string') { + // Validate query (term) is provided + if (!query || typeof query !== 'string') { throw new Error('query is a required parameter of type string'); } @@ -148,7 +145,8 @@ function createSearchUrl(query, parameters, userParameters, options, isVoiceSear const searchUrl = isVoiceSearch ? 'search/natural_language' : 'search'; - return `${serviceUrl}/${searchUrl}/${helpers.encodeURIComponentRFC3986(queryTrimmed)}?${queryString}`; + // Note: it is intentional that query is dispatched without being trimmed + return `${serviceUrl}/${searchUrl}/${helpers.encodeURIComponentRFC3986(query)}?${queryString}`; } /** From 7ef65452b48733c50bbb158c0683530bc6ce4bd4 Mon Sep 17 00:00:00 2001 From: Steve Blaurock Date: Tue, 19 Dec 2023 16:48:32 -0700 Subject: [PATCH 6/7] Remove only from tests --- spec/src/modules/autocomplete.js | 2 +- spec/src/modules/search.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/src/modules/autocomplete.js b/spec/src/modules/autocomplete.js index b89f1770..aff7b15d 100644 --- a/spec/src/modules/autocomplete.js +++ b/spec/src/modules/autocomplete.js @@ -473,7 +473,7 @@ describe('ConstructorIO - Autocomplete', () => { }); }); - it.only('Should not trim spaces from query', (done) => { + it('Should not trim spaces from query', (done) => { const queryWithSpaces = ` ${query} `; const { autocomplete } = new ConstructorIO({ apiKey: testApiKey, diff --git a/spec/src/modules/search.js b/spec/src/modules/search.js index 927d0e0a..d6713583 100644 --- a/spec/src/modules/search.js +++ b/spec/src/modules/search.js @@ -543,7 +543,7 @@ describe('ConstructorIO - Search', () => { }); }); - it.only('Should not trim spaces from query', (done) => { + it('Should not trim spaces from query', (done) => { const queryWithSpaces = ` ${query} `; const { search } = new ConstructorIO({ apiKey: testApiKey, From 487acb274c17bbe9832b61d98f85090bf136553a Mon Sep 17 00:00:00 2001 From: Steve Blaurock Date: Wed, 20 Dec 2023 14:09:21 -0700 Subject: [PATCH 7/7] Remove unused test --- spec/src/modules/search.js | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/spec/src/modules/search.js b/spec/src/modules/search.js index d6713583..9226cb38 100644 --- a/spec/src/modules/search.js +++ b/spec/src/modules/search.js @@ -6,7 +6,6 @@ const sinon = require('sinon'); const sinonChai = require('sinon-chai'); const ConstructorIO = require('../../../test/constructorio'); // eslint-disable-line import/extensions const helpers = require('../../mocha.helpers'); -const utilsHelpers = require('../../../src/utils/helpers'); const nodeFetch = (...args) => import('node-fetch').then(({ default: fetch }) => fetch(...args)); @@ -580,22 +579,6 @@ describe('ConstructorIO - Search', () => { }); }); - it('Should trim non-breaking spaces from query', (done) => { - const queryWithSpaces = ` ${query} `; - const { search } = new ConstructorIO({ - apiKey: testApiKey, - fetch: fetchSpy, - }); - - search.getSearchResults(queryWithSpaces).then((res) => { - expect(res.request.term).to.equal(utilsHelpers.trimNonBreakingSpaces(queryWithSpaces)); - expect(res).to.have.property('request').to.be.an('object'); - expect(res).to.have.property('response').to.be.an('object'); - expect(res).to.have.property('result_id').to.be.an('string'); - done(); - }); - }); - it('Should pass the correct custom headers passed in function networkParameters', (done) => { const { search } = new ConstructorIO({ ...validOptions,