Merge pull request #4210 from RedisInsight/be/feature/CR-263-recommendations

seth-riedel · web-flow · commit 3026f0ddc29a · 2024-12-17T09:35:46.000-06:00
CR-263: Optimize recommendations lookup and creation
diff --git a/redisinsight/api/src/__mocks__/database-recommendation.ts b/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(),
 });
 
diff --git a/redisinsight/api/src/modules/browser/keys/keys.service.spec.ts b/redisinsight/api/src/modules/browser/keys/keys.service.spec.ts
@@ -153,19 +153,13 @@ describe('KeysService', () => {
         getKeyInfoResponse.name,
       );
 
-      expect(recommendationService.check).toBeCalledWith(
-        mockBrowserClientMetadata,
-        RECOMMENDATION_NAMES.BIG_SETS,
-        result,
-      );
-      expect(recommendationService.check).toBeCalledWith(
+      expect(recommendationService.checkMulti).toBeCalledWith(
         mockBrowserClientMetadata,
-        RECOMMENDATION_NAMES.BIG_STRINGS,
-        result,
-      );
-      expect(recommendationService.check).toBeCalledWith(
-        mockBrowserClientMetadata,
-        RECOMMENDATION_NAMES.COMPRESSION_FOR_LIST,
+        [
+          RECOMMENDATION_NAMES.BIG_SETS,
+          RECOMMENDATION_NAMES.BIG_STRINGS,
+          RECOMMENDATION_NAMES.COMPRESSION_FOR_LIST,
+        ],
         result,
       );
     });
diff --git a/redisinsight/api/src/modules/browser/keys/keys.service.ts b/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);
diff --git a/redisinsight/api/src/modules/database-recommendation/database-recommendation.service.ts b/redisinsight/api/src/modules/database-recommendation/database-recommendation.service.ts
@@ -52,52 +52,84 @@ 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) {
+      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,
         db: clientMetadata.db
           ?? (await this.databaseService.get(clientMetadata.sessionMetadata, clientMetadata.databaseId))?.db
           ?? 0,
       };
-      const isRecommendationExist = await this.databaseRecommendationRepository.isExist(
+      const isRecommendationExist = await this.databaseRecommendationRepository.isExistMulti(
         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);
-        }
-      }
+      const results = await Promise.all(
+        recommendationNames.map(
+          (name) => this.checkRecommendation(
+            name,
+            isRecommendationExist[name],
+            newClientMetadata,
+            data,
+          ),
+        ),
+      );
 
-      return null;
+      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
diff --git a/redisinsight/api/src/modules/database-recommendation/repositories/database-recommendation.repository.ts b/redisinsight/api/src/modules/database-recommendation/repositories/database-recommendation.repository.ts
@@ -42,13 +42,20 @@ export abstract class DatabaseRecommendationRepository {
   ): Promise<DatabaseRecommendation>;
 
   /**
-   * Check is recommendation already exist in repository
+   * Check if recommendation already exist in repository
    * @param clientMetadata
    * @param name
    * @return Boolean
    */
   abstract isExist(clientMetadata: ClientMetadata, name: string): Promise<boolean>;
 
+  /**
+   * Check if one or more recommendations exist in repository
+   * @param clientMetadata
+   * @param names
+   */
+  abstract isExistMulti(clientMetadata: ClientMetadata, names: string[]): Promise<Map<string, boolean>>;
+
   /**
    * Get database recommendation by id
    * @param sessionMetadata
diff --git a/redisinsight/api/src/modules/database-recommendation/repositories/local.database.recommendation.repository.spec.ts b/redisinsight/api/src/modules/database-recommendation/repositories/local.database.recommendation.repository.spec.ts
@@ -78,6 +78,22 @@ describe('LocalDatabaseRecommendationRepository', () => {
     });
   });
 
+  describe('isExistMulti', () => {
+    it('should return results for multiple recommendation names', async () => {
+      repository.findOneBy.mockResolvedValueOnce(null);
+      repository.findOneBy.mockResolvedValueOnce({});
+      expect(await service.isExistMulti(mockClientMetadata, ['test1', 'test2'])).toEqual({
+        test1: false,
+        test2: true,
+      });
+    });
+
+    it('should return empty Map when received error', async () => {
+      repository.findOneBy.mockRejectedValueOnce(new Error());
+      expect(await service.isExistMulti(mockClientMetadata, ['test1', 'test2'])).toEqual({});
+    });
+  });
+
   describe('create', () => {
     it('should create recommendation', async () => {
       const result = await service.create(mockClientMetadata.sessionMetadata, mockDatabaseRecommendation);
diff --git a/redisinsight/api/src/modules/database-recommendation/repositories/local.database.recommendation.repository.ts b/redisinsight/api/src/modules/database-recommendation/repositories/local.database.recommendation.repository.ts
@@ -41,7 +41,7 @@ export class LocalDatabaseRecommendationRepository extends DatabaseRecommendatio
 
   /**
    * Save entire entity
-   * @param _
+   * @param sessionMetadata
    * @param entity
    */
   async create(sessionMetadata: SessionMetadata, entity: DatabaseRecommendation): Promise<DatabaseRecommendation> {
@@ -102,7 +102,7 @@ export class LocalDatabaseRecommendationRepository extends DatabaseRecommendatio
   }
 
   /**
-   * Read all recommendations recommendations
+   * Read all recommendations
    * @param clientMetadata
    */
   async read(clientMetadata: ClientMetadata): Promise<void> {
@@ -166,6 +166,31 @@ export class LocalDatabaseRecommendationRepository extends DatabaseRecommendatio
     }
   }
 
+  /**
+   * Check if one or more recommendations exist in database
+   * @param clientMetadata
+   * @param names
+   */
+  async isExistMulti(clientMetadata: ClientMetadata, names: string[]): Promise<Map<string, boolean>> {
+    const { databaseId } = clientMetadata;
+
+    try {
+      this.logger.debug('Checking if recommendations exist', names, clientMetadata);
+
+      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 existence of recommendations', err, names, clientMetadata);
+      return {} as Map<string, boolean>;
+    }
+  }
+
   /**
    * Get recommendation by id
    * @param sessionMetadata
diff --git a/redisinsight/api/src/modules/database/database-connection.service.spec.ts b/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,
       );
 
diff --git a/redisinsight/api/src/modules/database/database-connection.service.ts b/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,
     );
 
