Merge pull request #570 from RedisInsight/bugfix/RI-2735-file-name

fix few bugs + add edge case tests

Author: arthosofteq

redisinsight/api/src/modules/profiler/models/log-file.spec.ts

@@ -97,8 +97,15 @@ describe('LogFile', () => {
     expect(fs.existsSync(logFile['filePath'])).toEqual(false);
     const stream = logFile.getWriteStream();
     expect(stream['_writableState'].ended).toEqual(false);
-    stream.write('somedata', () => {
-      expect(fs.existsSync(logFile['filePath'])).toEqual(true);
+    await new Promise((res, rej) => {
+      stream.write('somedata', (err) => {
+        if (err) {
+          return rej(err);
+        }
+
+        expect(fs.existsSync(logFile['filePath'])).toEqual(true);
+        return res(true);
+      });
     });
     expect(logFile['writeStream']).toEqual(stream);
     await logFile.destroy();

redisinsight/api/src/modules/profiler/models/redis.observer.spec.ts

@@ -43,6 +43,7 @@ describe('RedisObserver', () => {
   describe('init', () => {
     it('successfully init', async () => {
       await new Promise((resolve) => {
+        redisObserver['connect'] = jest.fn();
         redisObserver.init(getRedisClientFn);
         expect(redisObserver['status']).toEqual(RedisObserverStatus.Initializing);
         redisObserver.on('connect', () => {
@@ -53,10 +54,14 @@ describe('RedisObserver', () => {
       expect(redisObserver['redis']).toEqual(nodeClient);
     });
     it('init error due to redis connection', async () => {
-      getRedisClientFn.mockRejectedValueOnce(new Error('error'));
-      await redisObserver.init(getRedisClientFn);
-      expect(redisObserver['status']).toEqual(RedisObserverStatus.Error);
-      expect(redisObserver['redis']).toEqual(undefined);
+      try {
+        getRedisClientFn.mockRejectedValueOnce(new Error('error'));
+        await redisObserver.init(getRedisClientFn);
+        fail();
+      } catch (e) {
+        expect(redisObserver['status']).toEqual(RedisObserverStatus.Error);
+        expect(redisObserver['redis']).toEqual(undefined);
+      }
     });
   });
 
@@ -96,7 +101,7 @@ describe('RedisObserver', () => {
       await redisObserver.subscribe(mockProfilerClient);
       redisObserver['status'] = RedisObserverStatus.Ready;
       await redisObserver.subscribe(mockProfilerClient);
-      expect(redisObserver['connect']).toHaveBeenCalledTimes(1);
+      expect(redisObserver['connect']).toHaveBeenCalledTimes(2);
       expect(redisObserver['shardsObservers'].length).toEqual(2);
       expect(redisObserver['profilerClientsListeners'].size).toEqual(1);
       expect(redisObserver['profilerClientsListeners'].get(mockProfilerClient.id).length).toEqual(4);
@@ -194,10 +199,8 @@ describe('RedisObserver', () => {
 
     it('connect fail due to NOPERM', async () => {
       try {
-        await redisObserver.init(getRedisClientFn);
         nodeClient.send_command.mockRejectedValueOnce(NO_PERM_ERROR);
-        const profilerClient = new ProfilerClient('1', mockSocket);
-        await redisObserver.subscribe(profilerClient);
+        await redisObserver.init(getRedisClientFn);
         fail();
       } catch (e) {
         expect(e).toBeInstanceOf(ForbiddenException);
@@ -208,10 +211,8 @@ describe('RedisObserver', () => {
 
     it('connect fail due an error', async () => {
       try {
-        await redisObserver.init(getRedisClientFn);
         nodeClient.send_command.mockRejectedValueOnce(new Error('some error'));
-        const profilerClient = new ProfilerClient('1', mockSocket);
-        await redisObserver.subscribe(profilerClient);
+        await redisObserver.init(getRedisClientFn);
         fail();
       } catch (e) {
         expect(e).toBeInstanceOf(ServiceUnavailableException);
@@ -222,14 +223,10 @@ describe('RedisObserver', () => {
 
     it('connect to cluster', async () => {
       getRedisClientFn.mockResolvedValue(clusterClient);
+      clusterClient.nodes = jest.fn().mockReturnValue([mockClusterNode1, mockClusterNode2]);
       await redisObserver.init(getRedisClientFn);
 
       redisObserver['redis'] = clusterClient;
-      clusterClient.nodes = jest.fn().mockReturnValue([mockClusterNode1, mockClusterNode2]);
-
-      // RedisObserver.createShardObserver = jest.fn().mockResolvedValue(mockRedisShardObserver);
-      const profilerClient = new ProfilerClient('1', mockSocket);
-      await redisObserver.subscribe(profilerClient);
       expect(redisObserver['shardsObservers']).toEqual([mockRedisShardObserver, mockRedisShardObserver]);
       expect(redisObserver['status']).toEqual(RedisObserverStatus.Ready);
     });

redisinsight/api/src/modules/profiler/models/redis.observer.ts

@@ -32,11 +32,16 @@ export class RedisObserver extends EventEmitter2 {
       .then((redis) => {
         this.redis = redis;
         this.status = RedisObserverStatus.Connected;
+      })
+      .then(() => this.connect())
+      .then(() => {
         this.emit('connect');
+        return Promise.resolve();
       })
       .catch((err) => {
         this.status = RedisObserverStatus.Error;
         this.emit('connect_error', err);
+        return Promise.reject(err);
       });
   }
 

redisinsight/api/src/modules/profiler/profiler-analytics.service.ts

@@ -21,27 +21,27 @@ export class ProfilerAnalyticsService extends TelemetryBaseService {
     this.events.set(TelemetryEvents.ProfilerLogDeleted, this.sendLogDeleted.bind(this));
   }
 
-  sendLogDeleted(databaseId: string, fileSize: number): void {
+  sendLogDeleted(databaseId: string, fileSizeBytes: number): void {
     try {
       this.sendEvent(
         TelemetryEvents.ProfilerLogDeleted,
         {
           databaseId,
-          fileSize,
+          fileSizeBytes,
         },
       );
     } catch (e) {
       // continue regardless of error
     }
   }
 
-  sendLogDownloaded(databaseId: string, fileSize: number): void {
+  sendLogDownloaded(databaseId: string, fileSizeBytes: number): void {
     try {
       this.sendEvent(
         TelemetryEvents.ProfilerLogDownloaded,
         {
           databaseId,
-          fileSize,
+          fileSizeBytes,
         },
       );
     } catch (e) {

redisinsight/api/src/modules/profiler/providers/redis-observer.provider.spec.ts

@@ -1,7 +1,7 @@
 import { Test, TestingModule } from '@nestjs/testing';
-import * as Redis from 'ioredis-mock';
+import * as Redis from 'ioredis';
 import {
-  mockLogFile,
+  mockLogFile, mockRedisShardObserver,
   MockType,
 } from 'src/__mocks__';
 import { InstancesBusinessService } from 'src/modules/shared/services/instances-business/instances-business.service';
@@ -10,7 +10,12 @@ import { RedisService } from 'src/modules/core/services/redis/redis.service';
 import { mockRedisClientInstance } from 'src/modules/shared/services/base/redis-consumer.abstract.service.spec';
 import { RedisObserverStatus } from 'src/modules/profiler/constants';
 
-const mockRedisClient = new Redis();
+const nodeClient = Object.create(Redis.prototype);
+nodeClient.monitor = jest.fn();
+nodeClient.status = 'ready';
+nodeClient.disconnect = jest.fn();
+nodeClient.duplicate = jest.fn();
+nodeClient.send_command = jest.fn();
 
 describe('RedisObserverProvider', () => {
   let service: RedisObserverProvider;
@@ -44,17 +49,20 @@ describe('RedisObserverProvider', () => {
     redisService = await module.get(RedisService);
     databaseService = await module.get(InstancesBusinessService);
 
-    redisService.getClientInstance.mockReturnValue(mockRedisClientInstance);
+    redisService.getClientInstance.mockReturnValue({ ...mockRedisClientInstance, client: nodeClient });
     redisService.isClientConnected.mockReturnValue(true);
-    databaseService.connectToInstance.mockResolvedValue(mockRedisClient);
+    databaseService.connectToInstance.mockResolvedValue(nodeClient);
+    nodeClient.send_command.mockResolvedValue('OK');
+    nodeClient.duplicate.mockReturnValue(nodeClient);
+    nodeClient.monitor.mockReturnValue(mockRedisShardObserver);
   });
 
   it('getOrCreateObserver new observer get existing redis client', async () => {
     const redisObserver = await service.getOrCreateObserver(
       mockLogFile.instanceId,
     );
 
-    expect(redisObserver['redis']).toEqual(mockRedisClientInstance.client);
+    expect(redisObserver['redis']).toEqual(nodeClient);
     expect(service['redisObservers'].size).toEqual(1);
 
     expect(await service.getObserver(mockLogFile.instanceId)).toEqual(redisObserver);
@@ -77,9 +85,9 @@ describe('RedisObserverProvider', () => {
       mockLogFile.instanceId,
     );
 
-    redisObserver['init'] = jest.fn();
-    expect(redisObserver['redis']).toEqual(mockRedisClientInstance.client);
-    expect(redisObserver['status']).toEqual(RedisObserverStatus.Connected);
+    redisObserver['init'] = jest.fn().mockResolvedValue(Promise.resolve());
+    expect(redisObserver['redis']).toEqual(nodeClient);
+    expect(redisObserver['status']).toEqual(RedisObserverStatus.Ready);
 
     const promise = service.getOrCreateObserver(mockLogFile.instanceId);
     redisObserver.emit('connect');

redisinsight/api/src/modules/profiler/providers/redis-observer.provider.ts

@@ -38,7 +38,7 @@ export class RedisObserverProvider {
         this.redisObservers.set(instanceId, redisObserver);
 
         // initialize redis observer
-        redisObserver.init(this.getRedisClientFn(instanceId));
+        redisObserver.init(this.getRedisClientFn(instanceId)).catch();
       } else {
         switch (redisObserver.status) {
           case RedisObserverStatus.Ready:
@@ -49,7 +49,7 @@ export class RedisObserverProvider {
           case RedisObserverStatus.Error:
             this.logger.debug(`Trying to reconnect. Current status: ${redisObserver.status}`);
             // try to reconnect
-            redisObserver.init(this.getRedisClientFn(instanceId));
+            redisObserver.init(this.getRedisClientFn(instanceId)).catch();
             break;
           case RedisObserverStatus.Initializing:
           case RedisObserverStatus.Wait:

redisinsight/api/test/api/ws/monitor/monitor.test.ts

@@ -13,12 +13,25 @@ const { getSocket, constants, rte } = deps;
 const getMonitorClient = async (instanceId): Promise<Socket> => {
   return getSocket('monitor', {
     query: { instanceId },
-  })
+  });
 };
 
 describe('monitor', function () {
   this.timeout(4000);
 
+  describe('Connection edge cases', () => {
+    it('should not crash on 100 concurrent monitor connections to the same db', async () => {
+      const client = await getMonitorClient(constants.TEST_INSTANCE_ID);
+      await Promise.all((new Array(100).fill(1)).map(() => new Promise((res, rej) => {
+        client.emit('monitor', { logFileId: constants.getRandomString() }, (ack) => {
+          expect(ack).to.eql({ status: 'ok' });
+          res(ack);
+        });
+        client.on('exception', rej);
+      })));
+    });
+  });
+
   describe('Client creation', () => {
     it('Should successfully create a client', async () => {
       const client = await getMonitorClient(constants.TEST_INSTANCE_ID);