From ab0a33d64103c25d4d806bcb577b41c7c25f0de0 Mon Sep 17 00:00:00 2001 From: zalenskiSofteq Date: Wed, 22 Mar 2023 18:14:06 +0800 Subject: [PATCH 1/4] #RI-4304 - Run FT.CONFIG per database --- .../src/modules/browser/interfaces/index.ts | 1 + .../interfaces/redisearch.interface.ts | 3 ++ .../services/redisearch/redisearch.service.ts | 21 ++++++++------ ...OST-databases-id-redisearch-search.test.ts | 28 +++++++++---------- 4 files changed, 30 insertions(+), 23 deletions(-) create mode 100644 redisinsight/api/src/modules/browser/interfaces/index.ts create mode 100644 redisinsight/api/src/modules/browser/interfaces/redisearch.interface.ts diff --git a/redisinsight/api/src/modules/browser/interfaces/index.ts b/redisinsight/api/src/modules/browser/interfaces/index.ts new file mode 100644 index 0000000000..82bd738e96 --- /dev/null +++ b/redisinsight/api/src/modules/browser/interfaces/index.ts @@ -0,0 +1 @@ +export * from './redisearch.interface' diff --git a/redisinsight/api/src/modules/browser/interfaces/redisearch.interface.ts b/redisinsight/api/src/modules/browser/interfaces/redisearch.interface.ts new file mode 100644 index 0000000000..c7aa072ca3 --- /dev/null +++ b/redisinsight/api/src/modules/browser/interfaces/redisearch.interface.ts @@ -0,0 +1,3 @@ +export interface IMaxSearchResults { + [key: string]: number | null; +} diff --git a/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts b/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts index c15a185a32..89f3d29dfd 100644 --- a/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts +++ b/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts @@ -1,5 +1,5 @@ import { Cluster, Command, Redis } from 'ioredis'; -import { isNull, toNumber, uniq } from 'lodash'; +import { isUndefined, toNumber, uniq } from 'lodash'; import { BadRequestException, ConflictException, @@ -23,10 +23,11 @@ import { BrowserHistoryMode } from 'src/common/constants'; import { BrowserToolService } from '../browser-tool/browser-tool.service'; import { BrowserHistoryService } from '../browser-history/browser-history.service'; import { CreateBrowserHistoryDto } from '../../dto/browser-history/create.browser-history.dto'; +import { IMaxSearchResults } from '../../interfaces'; @Injectable() export class RedisearchService { - private maxSearchResults: number | null = null; + private maxSearchResults: IMaxSearchResults = {}; private logger = new Logger('RedisearchService'); @@ -153,24 +154,26 @@ export class RedisearchService { const client = await this.browserTool.getRedisClient(clientMetadata); - if (isNull(this.maxSearchResults)) { + if (isUndefined(this.maxSearchResults[clientMetadata.databaseId])) { try { + // response: [ [ 'MAXSEARCHRESULTS', '10000' ] ] const [[, maxSearchResults]] = await client.sendCommand( - // response: [ [ 'MAXSEARCHRESULTS', '10000' ] ] new Command('FT.CONFIG', ['GET', 'MAXSEARCHRESULTS'], { replyEncoding: 'utf8', }), ) as [[string, string]]; - this.maxSearchResults = toNumber(maxSearchResults); + this.maxSearchResults[clientMetadata.databaseId] = toNumber(maxSearchResults); } catch (error) { - this.maxSearchResults = null; + this.maxSearchResults[clientMetadata.databaseId] = null; } } // Workaround: recalculate limit to not query more then MAXSEARCHRESULTS let safeLimit = limit; - if (this.maxSearchResults && offset + limit > this.maxSearchResults) { - safeLimit = offset <= this.maxSearchResults ? this.maxSearchResults - offset : limit; + const maxSearchResult = this.maxSearchResults[clientMetadata.databaseId] + + if (maxSearchResult && offset + limit > maxSearchResult) { + safeLimit = offset <= maxSearchResult ? maxSearchResult - offset : limit; } const [total, ...keyNames] = await client.sendCommand( @@ -193,7 +196,7 @@ export class RedisearchService { total, scanned: keyNames.length + offset, keys: keyNames.map((name) => ({ name })), - maxResults: this.maxSearchResults, + maxResults: maxSearchResult, }); } catch (e) { this.logger.error('Failed to search keys using redisearch index', e); diff --git a/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts b/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts index f4fa0dfa30..1e94f8866d 100644 --- a/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts +++ b/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts @@ -123,6 +123,20 @@ describe('POST /databases/:id/redisearch/search', () => { before(async () => rte.data.setAclUserRules('~* +@all')); [ + { + name: 'Should return response with maxResults = null if no permissions for "ft.config" command', + endpoint: () => endpoint(constants.TEST_INSTANCE_ACL_ID), + data: validInputData, + responseSchema, + checkFn: async ({ body }) => { + expect(body.keys.length).to.eq(10); + expect(body.cursor).to.eq(10); + expect(body.scanned).to.eq(10); + expect(body.total).to.eq(2000); + expect(body.maxResults).to.eq(null); + }, + before: () => rte.data.setAclUserRules('~* +@all -ft.config') + }, { name: 'Should search', endpoint: () => endpoint(constants.TEST_INSTANCE_ACL_ID), @@ -143,20 +157,6 @@ describe('POST /databases/:id/redisearch/search', () => { }, before: () => rte.data.setAclUserRules('~* +@all -ft.search') }, - { - name: 'Should return response with maxResults = null if no permissions for "ft.config" command', - endpoint: () => endpoint(constants.TEST_INSTANCE_ACL_ID), - data: validInputData, - responseSchema, - checkFn: async ({ body }) => { - expect(body.keys.length).to.eq(10); - expect(body.cursor).to.eq(10); - expect(body.scanned).to.eq(10); - expect(body.total).to.eq(2000); - expect(body.maxResults).to.eq(null); - }, - before: () => rte.data.setAclUserRules('~* +@all -ft.config') - }, ].map(mainCheckFn); }); }); From 11098cdebb3038d9b3a9e1d3ef48f0cb0db8ba2a Mon Sep 17 00:00:00 2001 From: zalenskiSofteq Date: Wed, 22 Mar 2023 18:45:02 +0800 Subject: [PATCH 2/4] #RI-4304 - fix tests --- .../POST-databases-id-redisearch-search.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts b/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts index 1e94f8866d..1ad090fcd7 100644 --- a/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts +++ b/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts @@ -88,17 +88,17 @@ describe('POST /databases/:id/redisearch/search', () => { name: 'Should modify limit to not exceed available search limitation', data: { ...validInputData, - offset: 0, + offset: 10, limit: 10, }, checkFn: async ({ body }) => { expect(body.keys.length).to.eq(1); - expect(body.cursor).to.eq(10); - expect(body.scanned).to.eq(1); + expect(body.cursor).to.eq(20); + expect(body.scanned).to.eq(11); expect(body.total).to.eq(2000); - expect(body.maxResults).to.gte(1); + expect(body.maxResults).to.gte(11); }, - before: () => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '1'), + before: () => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '11'), }, { name: 'Should return custom error message if MAXSEARCHRESULTS less than request.limit', From 20cba2b31a65d6d1824e28b03b3885e76e7921b6 Mon Sep 17 00:00:00 2001 From: zalenskiSofteq Date: Fri, 24 Mar 2023 15:32:06 +0800 Subject: [PATCH 3/4] * #RI-4304 - fix IT --- ...OST-databases-id-redisearch-search.test.ts | 37 ++++++++++++++----- redisinsight/api/test/helpers/local-db.ts | 6 +-- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts b/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts index 1ad090fcd7..1dc7c2eb2c 100644 --- a/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts +++ b/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts @@ -10,7 +10,7 @@ import { validateInvalidDataTestCase, getMainCheckFn, JoiRedisString } from '../deps'; -const { server, request, constants, rte } = deps; +const { server, request, constants, rte, localDb } = deps; // endpoint to test const endpoint = (instanceId = constants.TEST_INSTANCE_ID) => @@ -44,10 +44,13 @@ const mainCheckFn = getMainCheckFn(endpoint); describe('POST /databases/:id/redisearch/search', () => { requirements('!rte.bigData', 'rte.modules.search'); - before(async () => rte.data.generateRedisearchIndexes(true)); - beforeEach(() => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '10000')); + before(async () => { + await rte.data.generateRedisearchIndexes(true) + await localDb.createTestDbInstance(rte, {}, { id: constants.TEST_INSTANCE_ID_2 }) + }); describe('Main', () => { + before(() => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '10000')); describe('Validation', () => { generateInvalidDataTestCases(dataSchema, validInputData).map( validateInvalidDataTestCase(endpoint, dataSchema), @@ -84,24 +87,33 @@ describe('POST /databases/:id/redisearch/search', () => { expect(body.maxResults).to.gte(10000); }, }, + ].map(mainCheckFn); + }); + + describe('maxSearchResults', () => { + [ { name: 'Should modify limit to not exceed available search limitation', + endpoint: () => endpoint(constants.TEST_INSTANCE_ID_2), data: { ...validInputData, - offset: 10, + offset: 0, limit: 10, }, checkFn: async ({ body }) => { expect(body.keys.length).to.eq(1); - expect(body.cursor).to.eq(20); - expect(body.scanned).to.eq(11); + expect(body.cursor).to.eq(10); + expect(body.scanned).to.eq(1); expect(body.total).to.eq(2000); - expect(body.maxResults).to.gte(11); + expect(body.maxResults).to.gte(1); + }, + before: async () => { + await rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '1') }, - before: () => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '11'), }, { name: 'Should return custom error message if MAXSEARCHRESULTS less than request.limit', + endpoint: () => endpoint(constants.TEST_INSTANCE_ID_2), data: { ...validInputData, offset: 10, @@ -113,14 +125,19 @@ describe('POST /databases/:id/redisearch/search', () => { error: 'Bad Request', message: `Set MAXSEARCHRESULTS to at least ${numberWithSpaces(validInputData.limit)}.`, }, - before: () => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '1'), + before: async () => { + await rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '1') + }, }, ].map(mainCheckFn); }); describe('ACL', () => { requirements('rte.acl'); - before(async () => rte.data.setAclUserRules('~* +@all')); + before(async () => { + await rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '10000') + await rte.data.setAclUserRules('~* +@all') + }); [ { diff --git a/redisinsight/api/test/helpers/local-db.ts b/redisinsight/api/test/helpers/local-db.ts index 0c040b35ea..8a5842535e 100644 --- a/redisinsight/api/test/helpers/local-db.ts +++ b/redisinsight/api/test/helpers/local-db.ts @@ -262,7 +262,7 @@ const createClientCertificate = async (certificate) => { return rep.save(certificate); } -const createTesDbInstance = async (rte, server): Promise => { +export const createTestDbInstance = async (rte, server, data: any = {}): Promise => { const rep = await getRepository(repositories.DATABASE); const instance: any = { @@ -324,7 +324,7 @@ const createTesDbInstance = async (rte, server): Promise => { passphrase: encryptData(constants.TEST_SSH_PASSPHRASE), }; } - await rep.save(instance); + await rep.save({ ...instance, ...data}); } export const createDatabaseInstances = async () => { @@ -524,7 +524,7 @@ const truncateAll = async () => { export const initLocalDb = async (rte, server) => { await truncateAll(); - await createTesDbInstance(rte, server); + await createTestDbInstance(rte, server); await initAgreements(); if (rte.env.acl) { await createAclInstance(rte, server); From e1fa872c21c3670004db2e19d9ff6784097f42d0 Mon Sep 17 00:00:00 2001 From: zalenskiSofteq Date: Fri, 24 Mar 2023 16:02:47 +0800 Subject: [PATCH 4/4] #RI-4304 - fix pr comment --- .../api/src/modules/browser/interfaces/index.ts | 1 - .../browser/interfaces/redisearch.interface.ts | 3 --- .../browser/services/redisearch/redisearch.service.ts | 11 +++++------ 3 files changed, 5 insertions(+), 10 deletions(-) delete mode 100644 redisinsight/api/src/modules/browser/interfaces/index.ts delete mode 100644 redisinsight/api/src/modules/browser/interfaces/redisearch.interface.ts diff --git a/redisinsight/api/src/modules/browser/interfaces/index.ts b/redisinsight/api/src/modules/browser/interfaces/index.ts deleted file mode 100644 index 82bd738e96..0000000000 --- a/redisinsight/api/src/modules/browser/interfaces/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './redisearch.interface' diff --git a/redisinsight/api/src/modules/browser/interfaces/redisearch.interface.ts b/redisinsight/api/src/modules/browser/interfaces/redisearch.interface.ts deleted file mode 100644 index c7aa072ca3..0000000000 --- a/redisinsight/api/src/modules/browser/interfaces/redisearch.interface.ts +++ /dev/null @@ -1,3 +0,0 @@ -export interface IMaxSearchResults { - [key: string]: number | null; -} diff --git a/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts b/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts index 89f3d29dfd..8ff0407dcf 100644 --- a/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts +++ b/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts @@ -23,11 +23,10 @@ import { BrowserHistoryMode } from 'src/common/constants'; import { BrowserToolService } from '../browser-tool/browser-tool.service'; import { BrowserHistoryService } from '../browser-history/browser-history.service'; import { CreateBrowserHistoryDto } from '../../dto/browser-history/create.browser-history.dto'; -import { IMaxSearchResults } from '../../interfaces'; @Injectable() export class RedisearchService { - private maxSearchResults: IMaxSearchResults = {}; + private maxSearchResults: Map = new Map(); private logger = new Logger('RedisearchService'); @@ -154,7 +153,7 @@ export class RedisearchService { const client = await this.browserTool.getRedisClient(clientMetadata); - if (isUndefined(this.maxSearchResults[clientMetadata.databaseId])) { + if (isUndefined(this.maxSearchResults.get(clientMetadata.databaseId))) { try { // response: [ [ 'MAXSEARCHRESULTS', '10000' ] ] const [[, maxSearchResults]] = await client.sendCommand( @@ -163,14 +162,14 @@ export class RedisearchService { }), ) as [[string, string]]; - this.maxSearchResults[clientMetadata.databaseId] = toNumber(maxSearchResults); + this.maxSearchResults.set(clientMetadata.databaseId, toNumber(maxSearchResults)); } catch (error) { - this.maxSearchResults[clientMetadata.databaseId] = null; + this.maxSearchResults.set(clientMetadata.databaseId, null); } } // Workaround: recalculate limit to not query more then MAXSEARCHRESULTS let safeLimit = limit; - const maxSearchResult = this.maxSearchResults[clientMetadata.databaseId] + const maxSearchResult = this.maxSearchResults.get(clientMetadata.databaseId) if (maxSearchResult && offset + limit > maxSearchResult) { safeLimit = offset <= maxSearchResult ? maxSearchResult - offset : limit;