Merge pull request #1590 from RedisInsight/feature/RI-3971-3572_add_recommendations

Feature/ri 3971 3572 add recommendations

Author: nmammadli

redisinsight/api/src/constants/recommendations.ts

@@ -16,4 +16,14 @@ export const RECOMMENDATION_NAMES = Object.freeze({
   RTS: 'RTS',
   REDIS_VERSION: 'redisVersion',
   REDIS_SEARCH: 'redisSearch',
+  SEARCH_INDEXES: 'searchIndexes',
+  DANGEROUS_COMMANDS: 'dangerousCommands',
 });
+
+export const ONE_NODE_RECOMMENDATIONS = [
+  RECOMMENDATION_NAMES.LUA_SCRIPT,
+  RECOMMENDATION_NAMES.DANGEROUS_COMMANDS,
+  RECOMMENDATION_NAMES.AVOID_LOGICAL_DATABASES,
+  RECOMMENDATION_NAMES.RTS,
+  RECOMMENDATION_NAMES.REDIS_VERSION,
+];

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

@@ -1,15 +1,14 @@
 import { HttpException, Injectable, Logger } from '@nestjs/common';
-import { isNull, flatten, uniqBy } from 'lodash';
+import { isNull, flatten, concat } from 'lodash';
 import { RecommendationService } from 'src/modules/recommendation/recommendation.service';
 import { catchAclError } from 'src/utils';
-import { RECOMMENDATION_NAMES } from 'src/constants';
+import { ONE_NODE_RECOMMENDATIONS } from 'src/constants';
 import { DatabaseAnalyzer } from 'src/modules/database-analysis/providers/database-analyzer';
 import { plainToClass } from 'class-transformer';
 import { DatabaseAnalysis, ShortDatabaseAnalysis } from 'src/modules/database-analysis/models';
 import { DatabaseAnalysisProvider } from 'src/modules/database-analysis/providers/database-analysis.provider';
 import { CreateDatabaseAnalysisDto } from 'src/modules/database-analysis/dto';
 import { KeysScanner } from 'src/modules/database-analysis/scanner/keys-scanner';
-import { Recommendation } from 'src/modules/database-analysis/models/recommendation';
 import { DatabaseConnectionService } from 'src/modules/database/database-connection.service';
 import { ClientMetadata } from 'src/common/models';
 
@@ -55,19 +54,28 @@ export class DatabaseAnalysisService {
         progress.total += nodeResult.progress.total;
       });
 
-      const recommendations = DatabaseAnalysisService.getRecommendationsSummary(
-        flatten(await Promise.all(
-          scanResults.map(async (nodeResult, idx) => (
-            await this.recommendationService.getRecommendations({
-              client: nodeResult.client,
-              keys: nodeResult.keys,
-              total: progress.total,
-              // TODO: create generic solution to exclude recommendations
-              exclude: idx !== 0 ? [RECOMMENDATION_NAMES.RTS] : [],
-            })
-          )),
-        )),
-      );
+      let recommendationToExclude = [];
+
+      const recommendations = await scanResults.reduce(async (previousPromise, nodeResult, idx) => {
+        const jobsArray = await previousPromise;
+        const nodeRecommendations = await this.recommendationService.getRecommendations({
+          client: nodeResult.client,
+          keys: nodeResult.keys,
+          total: progress.total,
+          globalClient: client,
+          exclude: recommendationToExclude,
+        });
+        if (idx === 0) {
+          recommendationToExclude = concat(recommendationToExclude, ONE_NODE_RECOMMENDATIONS);
+        }
+        const foundedRecommendations = nodeRecommendations.filter((recommendation) => !isNull(recommendation));
+        const foundedRecommendationNames = foundedRecommendations.map(({ name }) => name);
+        recommendationToExclude = concat(recommendationToExclude, foundedRecommendationNames);
+        recommendationToExclude.push(...foundedRecommendationNames);
+        jobsArray.push(foundedRecommendations);
+        return flatten(jobsArray);
+      }, Promise.resolve([]));
+
       const analysis = plainToClass(DatabaseAnalysis, await this.analyzer.analyze({
         databaseId: clientMetadata.databaseId,
         ...dto,
@@ -104,16 +112,4 @@ export class DatabaseAnalysisService {
   async list(databaseId: string): Promise<ShortDatabaseAnalysis[]> {
     return this.databaseAnalysisProvider.list(databaseId);
   }
-
-  /**
-   * Get recommendations summary
-   * @param recommendations
-   */
-
-  static getRecommendationsSummary(recommendations: Recommendation[]): Recommendation[] {
-    return uniqBy(
-      recommendations.filter((recommendation) => !isNull(recommendation)),
-      'name',
-    );
-  }
 }

redisinsight/api/src/modules/database-analysis/models/recommendation.ts

@@ -1,5 +1,5 @@
 import { Expose } from 'class-transformer';
-import { ApiProperty } from '@nestjs/swagger';
+import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
 
 export class Recommendation {
   @ApiProperty({
@@ -9,4 +9,11 @@ export class Recommendation {
   })
   @Expose()
   name: string;
+
+  @ApiPropertyOptional({
+    description: 'Additional recommendation params',
+    example: 'luaScript',
+  })
+  @Expose()
+  params?: any;
 }

redisinsight/api/src/modules/recommendation/providers/recommendation.provider.spec.ts

@@ -562,59 +562,59 @@ describe('RecommendationProvider', () => {
         .calledWith(jasmine.objectContaining({ name: 'FT._LIST' }))
         .mockResolvedValue(mockFTListResponse_1);
 
-      const redisServerRecommendation = await service
+      const redisSearchRecommendation = await service
         .determineRediSearchRecommendation(nodeClient, [mockJSONKey]);
-      expect(redisServerRecommendation).toEqual({ name: RECOMMENDATION_NAMES.REDIS_SEARCH });
+      expect(redisSearchRecommendation).toEqual({ name: RECOMMENDATION_NAMES.REDIS_SEARCH });
     });
 
     it('should return rediSearch recommendation when there is huge string key', async () => {
       when(nodeClient.sendCommand)
         .calledWith(jasmine.objectContaining({ name: 'FT._LIST' }))
         .mockResolvedValue(mockFTListResponse_1);
 
-      const redisServerRecommendation = await service
+      const redisSearchRecommendation = await service
         .determineRediSearchRecommendation(nodeClient, [mockRediSearchStringKey_1]);
-      expect(redisServerRecommendation).toEqual({ name: RECOMMENDATION_NAMES.REDIS_SEARCH });
+      expect(redisSearchRecommendation).toEqual({ name: RECOMMENDATION_NAMES.REDIS_SEARCH });
     });
 
     it('should not return rediSearch recommendation when there is small string key', async () => {
       when(nodeClient.sendCommand)
         .calledWith(jasmine.objectContaining({ name: 'FT._LIST' }))
         .mockResolvedValue(mockFTListResponse_1);
 
-      const redisServerRecommendation = await service
+      const redisSearchRecommendation = await service
         .determineRediSearchRecommendation(nodeClient, [mockRediSearchStringKey_2]);
-      expect(redisServerRecommendation).toEqual(null);
+      expect(redisSearchRecommendation).toEqual(null);
     });
 
     it('should not return rediSearch recommendation when there are no indexes', async () => {
       when(nodeClient.sendCommand)
         .calledWith(jasmine.objectContaining({ name: 'FT._LIST' }))
         .mockResolvedValue(mockFTListResponse_2);
 
-      const redisServerRecommendation = await service
+      const redisSearchRecommendation = await service
         .determineRediSearchRecommendation(nodeClient, [mockJSONKey]);
-      expect(redisServerRecommendation).toEqual(null);
+      expect(redisSearchRecommendation).toEqual(null);
     });
 
     it('should ignore errors when ft command execute with error', async () => {
       when(nodeClient.sendCommand)
         .calledWith(jasmine.objectContaining({ name: 'FT._LIST' }))
         .mockRejectedValue("some error");
 
-      const redisServerRecommendation = await service
+      const redisSearchRecommendation = await service
         .determineRediSearchRecommendation(nodeClient, [mockJSONKey]);
-      expect(redisServerRecommendation).toEqual({ name: RECOMMENDATION_NAMES.REDIS_SEARCH });
+      expect(redisSearchRecommendation).toEqual({ name: RECOMMENDATION_NAMES.REDIS_SEARCH });
     });
 
     it('should ignore errors when ft command execute with error', async () => {
       when(nodeClient.sendCommand)
         .calledWith(jasmine.objectContaining({ name: 'FT._LIST' }))
         .mockRejectedValue("some error");
 
-      const redisServerRecommendation = await service
+      const redisSearchRecommendation = await service
         .determineRediSearchRecommendation(nodeClient, [mockRediSearchStringKey_2]);
-      expect(redisServerRecommendation).toEqual(null);
+      expect(redisSearchRecommendation).toEqual(null);
     });
   });
 
@@ -624,19 +624,19 @@ describe('RecommendationProvider', () => {
         .calledWith(jasmine.objectContaining({ name: 'info' }))
         .mockResolvedValue(mockRedisServerResponse_1);
 
-      const redisServerRecommendation = await service
+      const redisVersionRecommendation = await service
         .determineRedisVersionRecommendation(nodeClient);
-      expect(redisServerRecommendation).toEqual(null);
+      expect(redisVersionRecommendation).toEqual(null);
     });
 
     it('should return redis version recommendation', async () => {
       when(nodeClient.sendCommand)
         .calledWith(jasmine.objectContaining({ name: 'info' }))
         .mockResolvedValueOnce(mockRedisServerResponse_2);
 
-      const redisServerRecommendation = await service
+      const redisVersionRecommendation = await service
         .determineRedisVersionRecommendation(nodeClient);
-      expect(redisServerRecommendation).toEqual({ name: RECOMMENDATION_NAMES.REDIS_VERSION });
+      expect(redisVersionRecommendation).toEqual({ name: RECOMMENDATION_NAMES.REDIS_VERSION });
     });
 
     it('should not return redis version recommendation when info command executed with error',
@@ -646,9 +646,23 @@ describe('RecommendationProvider', () => {
           .calledWith(jasmine.objectContaining({ name: 'info' }))
           .mockRejectedValue('some error');
 
-        const redisServerRecommendation = await service
+        const redisVersionRecommendation = await service
           .determineRedisVersionRecommendation(nodeClient);
-        expect(redisServerRecommendation).toEqual(null);
+        expect(redisVersionRecommendation).toEqual(null);
+      });
+  });
+
+  describe('determineDangerousCommandsRecommendation', () => {
+    it('should not return dangerous commands recommendation when "command" command executed with error',
+      async () => {
+        resetAllWhenMocks();
+        when(nodeClient.sendCommand)
+          .calledWith(jasmine.objectContaining({ name: 'command' }))
+          .mockRejectedValue('some error');
+
+        const dangerousCommandsRecommendation = await service
+          .determineDangerousCommandsRecommendation(nodeClient);
+        expect(dangerousCommandsRecommendation).toEqual(null);
       });
   });
 });