CR-263: Optimize recommendations lookup and creation

Author: seth-riedel

redisinsight/api/src/__mocks__/database-recommendation.ts

@@ -34,6 +34,7 @@ export const mockDatabaseRecommendationService = () => ({
   create: jest.fn(),
   list: jest.fn(),
   check: jest.fn(),
+  checkMulti: jest.fn(),
   read: jest.fn(),
 });
 

redisinsight/api/src/modules/browser/keys/keys.service.ts

@@ -145,21 +145,17 @@ export class KeysService {
 
       const result = await this.keyInfoProvider.getStrategy(type).getInfo(client, key, type);
       this.logger.debug('Succeed to get key info', clientMetadata);
-      this.recommendationService.check(
-        clientMetadata,
-        RECOMMENDATION_NAMES.BIG_SETS,
-        result,
-      );
-      this.recommendationService.check(
-        clientMetadata,
-        RECOMMENDATION_NAMES.BIG_STRINGS,
-        result,
-      );
-      this.recommendationService.check(
+
+      this.recommendationService.checkMulti(
         clientMetadata,
-        RECOMMENDATION_NAMES.COMPRESSION_FOR_LIST,
+        [
+          RECOMMENDATION_NAMES.BIG_SETS,
+          RECOMMENDATION_NAMES.BIG_STRINGS,
+          RECOMMENDATION_NAMES.COMPRESSION_FOR_LIST,
+        ],
         result,
       );
+
       return plainToClass(GetKeyInfoResponse, result);
     } catch (error) {
       this.logger.error('Failed to get key info.', error, clientMetadata);

redisinsight/api/src/modules/database-recommendation/database-recommendation.service.ts

@@ -52,17 +52,38 @@ export class DatabaseRecommendationService {
     return this.databaseRecommendationRepository.list({ ...clientMetadata, db });
   }
 
-  /**
-   * Check recommendation condition
-   * @param clientMetadata
-   * @param recommendationName
-   * @param data
-   */
-  public async check(
-    clientMetadata: ClientMetadata,
+  private async checkRecommendation(
     recommendationName: string,
-    data: any,
+    exists: boolean,
+    clientMetadata: ClientMetadata, data: any,
   ): Promise<DatabaseRecommendation> {
+    if (exists) {
+      return null;
+    }
+
+    const recommendation = await this.scanner.determineRecommendation(
+      clientMetadata.sessionMetadata,
+      recommendationName,
+      data,
+    );
+
+    if (recommendation) {
+      const entity = plainToClass(
+        DatabaseRecommendation,
+        { databaseId: clientMetadata?.databaseId, ...recommendation },
+      );
+
+      return await this.create(clientMetadata, entity);
+    }
+
+    return null;
+  }
+
+  public async checkMulti(
+    clientMetadata: ClientMetadata,
+    recommendationNames: string[],
+    data: any,
+  ): Promise<Map<string, DatabaseRecommendation[]>> {
     try {
       const newClientMetadata = {
         ...clientMetadata,
@@ -72,32 +93,40 @@ export class DatabaseRecommendationService {
       };
       const isRecommendationExist = await this.databaseRecommendationRepository.isExist(
         newClientMetadata,
-        recommendationName,
+        recommendationNames,
       );
-      if (!isRecommendationExist) {
-        const recommendation = await this.scanner.determineRecommendation(
-          clientMetadata.sessionMetadata,
-          recommendationName,
-          data,
-        );
-
-        if (recommendation) {
-          const entity = plainToClass(
-            DatabaseRecommendation,
-            { databaseId: newClientMetadata?.databaseId, ...recommendation },
-          );
-
-          return await this.create(newClientMetadata, entity);
-        }
-      }
 
-      return null;
+      const results = await Promise.all(
+        recommendationNames.map(
+          (name) => this.checkRecommendation(name, isRecommendationExist[name], newClientMetadata, data),
+        ),
+      );
+
+      return results.reduce((acc, result, idx) => ({
+        ...acc,
+        [recommendationNames[idx]]: result,
+      }), {} as Map<string, DatabaseRecommendation[]>);
     } catch (e) {
       this.logger.warn('Unable to check recommendation', e, clientMetadata);
       return null;
     }
   }
 
+  /**
+   * Check recommendation condition
+   * @param clientMetadata
+   * @param recommendationName
+   * @param data
+   */
+  public async check(
+    clientMetadata: ClientMetadata,
+    recommendationName: string,
+    data: any,
+  ): Promise<DatabaseRecommendation> {
+    const result = await this.checkMulti(clientMetadata, [recommendationName], data);
+    return result[recommendationName];
+  }
+
   /**
    * Mark all recommendations as read for particular database
    * @param clientMetadata

redisinsight/api/src/modules/database-recommendation/repositories/database-recommendation.repository.ts

@@ -44,10 +44,10 @@ export abstract class DatabaseRecommendationRepository {
   /**
    * Check is recommendation already exist in repository
    * @param clientMetadata
-   * @param name
-   * @return Boolean
+   * @param names
+   * @return Map<string, boolean>
    */
-  abstract isExist(clientMetadata: ClientMetadata, name: string): Promise<boolean>;
+  abstract isExist(clientMetadata: ClientMetadata, names: string[]): Promise<Map<string, boolean>>;
 
   /**
    * Get database recommendation by id

redisinsight/api/src/modules/database-recommendation/repositories/local.database.recommendation.repository.spec.ts

@@ -64,17 +64,17 @@ describe('LocalDatabaseRecommendationRepository', () => {
 
   describe('isExist', () => {
     it('should return true when receive database entity', async () => {
-      expect(await service.isExist(mockClientMetadata, mockRecommendationName)).toEqual(true);
+      expect(await service.isExist(mockClientMetadata, [mockRecommendationName])).toEqual({ [mockRecommendationName]: true });
     });
 
     it('should return false when no database received', async () => {
       repository.findOneBy.mockResolvedValueOnce(null);
-      expect(await service.isExist(mockClientMetadata, mockRecommendationName)).toEqual(false);
+      expect(await service.isExist(mockClientMetadata, [mockRecommendationName])).toEqual({ [mockRecommendationName]: false });
     });
 
-    it('should return false when received error', async () => {
+    it('should return empty object when received error', async () => {
       repository.findOneBy.mockRejectedValueOnce(new Error());
-      expect(await service.isExist(mockClientMetadata, mockRecommendationName)).toEqual(false);
+      expect(await service.isExist(mockClientMetadata, [mockRecommendationName])).toEqual({});
     });
   });
 

redisinsight/api/src/modules/database-recommendation/repositories/local.database.recommendation.repository.ts

@@ -147,22 +147,25 @@ export class LocalDatabaseRecommendationRepository extends DatabaseRecommendatio
   /**
    * Check is recommendation exist in database
    * @param clientMetadata
-   * @param name
+   * @param names
    */
   async isExist(
     clientMetadata: ClientMetadata,
-    name: string,
-  ): Promise<boolean> {
+    names: string[],
+  ): Promise<Map<string, boolean>> {
     const { databaseId } = clientMetadata;
     try {
-      this.logger.debug(`Checking is recommendation ${name} exist`, clientMetadata);
-      const recommendation = await this.repository.findOneBy({ databaseId, name });
+      this.logger.debug('Checking if recommendations exist', clientMetadata, names);
 
-      this.logger.debug(`Succeed to check is recommendation ${name} exist'`, clientMetadata);
-      return !!recommendation;
+      const results = await Promise.all(names.map((name) => this.repository.findOneBy({ databaseId, name })));
+
+      return results.reduce((acc, result, idx) => ({
+        ...acc,
+        [names[idx]]: !!result,
+      }), {} as Map<string, boolean>);
     } catch (err) {
-      this.logger.error(`Failed to check is recommendation ${name} exist'`, err, clientMetadata);
-      return false;
+      this.logger.error('Failed to check existence of recommendations', err, clientMetadata, names);
+      return {} as Map<string, boolean>;
     }
   }
 

redisinsight/api/src/modules/database/database-connection.service.spec.ts

@@ -90,21 +90,15 @@ describe('DatabaseConnectionService', () => {
     it('should call recommendationService', async () => {
       expect(await service.connect(mockCommonClientMetadata)).toEqual(undefined);
 
-      expect(recommendationService.check).toHaveBeenCalledTimes(3);
+      expect(recommendationService.checkMulti).toHaveBeenCalledTimes(1);
 
-      expect(recommendationService.check).toBeCalledWith(
-        mockCommonClientMetadata,
-        RECOMMENDATION_NAMES.REDIS_VERSION,
-        mockRedisGeneralInfo,
-      );
-      expect(recommendationService.check).toBeCalledWith(
-        mockCommonClientMetadata,
-        RECOMMENDATION_NAMES.LUA_SCRIPT,
-        mockRedisGeneralInfo,
-      );
-      expect(recommendationService.check).toBeCalledWith(
+      expect(recommendationService.checkMulti).toBeCalledWith(
         mockCommonClientMetadata,
-        RECOMMENDATION_NAMES.BIG_AMOUNT_OF_CONNECTED_CLIENTS,
+        [
+          RECOMMENDATION_NAMES.REDIS_VERSION,
+          RECOMMENDATION_NAMES.LUA_SCRIPT,
+          RECOMMENDATION_NAMES.BIG_AMOUNT_OF_CONNECTED_CLIENTS,
+        ],
         mockRedisGeneralInfo,
       );
     });
@@ -116,21 +110,16 @@ describe('DatabaseConnectionService', () => {
 
       expect(await service.connect(mockCommonClientMetadata)).toEqual(undefined);
 
-      expect(recommendationService.check).toHaveBeenCalledTimes(4);
+      expect(recommendationService.check).toHaveBeenCalledTimes(1);
+      expect(recommendationService.checkMulti).toHaveBeenCalledTimes(1);
 
-      expect(recommendationService.check).toBeCalledWith(
-        mockCommonClientMetadata,
-        RECOMMENDATION_NAMES.REDIS_VERSION,
-        mockRedisGeneralInfo,
-      );
-      expect(recommendationService.check).toBeCalledWith(
-        mockCommonClientMetadata,
-        RECOMMENDATION_NAMES.LUA_SCRIPT,
-        mockRedisGeneralInfo,
-      );
-      expect(recommendationService.check).toBeCalledWith(
+      expect(recommendationService.checkMulti).toBeCalledWith(
         mockCommonClientMetadata,
-        RECOMMENDATION_NAMES.BIG_AMOUNT_OF_CONNECTED_CLIENTS,
+        [
+          RECOMMENDATION_NAMES.REDIS_VERSION,
+          RECOMMENDATION_NAMES.LUA_SCRIPT,
+          RECOMMENDATION_NAMES.BIG_AMOUNT_OF_CONNECTED_CLIENTS,
+        ],
         mockRedisGeneralInfo,
       );
 

redisinsight/api/src/modules/database/database-connection.service.ts

@@ -54,19 +54,13 @@ export class DatabaseConnectionService {
 
     const generalInfo = await this.databaseInfoProvider.getRedisGeneralInfo(client);
 
-    this.recommendationService.check(
+    this.recommendationService.checkMulti(
       clientMetadata,
-      RECOMMENDATION_NAMES.REDIS_VERSION,
-      generalInfo,
-    );
-    this.recommendationService.check(
-      clientMetadata,
-      RECOMMENDATION_NAMES.LUA_SCRIPT,
-      generalInfo,
-    );
-    this.recommendationService.check(
-      clientMetadata,
-      RECOMMENDATION_NAMES.BIG_AMOUNT_OF_CONNECTED_CLIENTS,
+      [
+        RECOMMENDATION_NAMES.REDIS_VERSION,
+        RECOMMENDATION_NAMES.LUA_SCRIPT,
+        RECOMMENDATION_NAMES.BIG_AMOUNT_OF_CONNECTED_CLIENTS,
+      ],
       generalInfo,
     );