Merge pull request #2797 from RedisInsight/bugfix/RI-5163_fix-bulk-delete_&_monitor

#RI-5163 - fix bulk action disconnection flow, monitor

Author: vlad-dargel

redisinsight/ui/src/components/bulk-actions-config/BulkActionsConfig.spec.tsx

@@ -4,13 +4,13 @@ import React from 'react'
 import MockedSocket from 'socket.io-mock'
 import socketIO from 'socket.io-client'
 import { cleanup, mockedStore, render } from 'uiSrc/utils/test-utils'
-import { BulkActionsServerEvent, BulkActionsType, SocketEvent } from 'uiSrc/constants'
+import { BulkActionsServerEvent, BulkActionsStatus, BulkActionsType, SocketEvent } from 'uiSrc/constants'
 import {
   bulkActionsDeleteSelector,
   bulkActionsSelector,
   disconnectBulkDeleteAction,
   setBulkActionConnected,
-  setBulkDeleteLoading
+  setBulkDeleteLoading, setDeleteOverviewStatus
 } from 'uiSrc/slices/browser/bulkActions'
 import BulkActionsConfig from './BulkActionsConfig'
 
@@ -110,6 +110,7 @@ describe('BulkActionsConfig', () => {
     const afterRenderActions = [
       setBulkActionConnected(true),
       setBulkDeleteLoading(true),
+      setDeleteOverviewStatus(BulkActionsStatus.Disconnected),
       disconnectBulkDeleteAction(),
     ]
     expect(store.getActions()).toEqual([...afterRenderActions])

redisinsight/ui/src/components/bulk-actions-config/BulkActionsConfig.tsx

@@ -11,6 +11,7 @@ import {
   setDeleteOverview,
   setBulkActionsInitialState,
   bulkActionsDeleteSelector,
+  setDeleteOverviewStatus,
 } from 'uiSrc/slices/browser/bulkActions'
 import { getBaseApiUrl, Nullable } from 'uiSrc/utils'
 import { sessionStorageService } from 'uiSrc/services'
@@ -21,11 +22,7 @@ import { BrowserStorageItem, BulkActionsServerEvent, BulkActionsStatus, BulkActi
 import { addErrorNotification } from 'uiSrc/slices/app/notifications'
 import { CustomHeaders } from 'uiSrc/constants/api'
 
-interface IProps {
-  retryDelay?: number
-}
-
-const BulkActionsConfig = ({ retryDelay = 5000 } : IProps) => {
+const BulkActionsConfig = () => {
   const { id: instanceId = '', db } = useSelector(connectedInstanceSelector)
   const { isConnected } = useSelector(bulkActionsSelector)
   const { isActionTriggered: isDeleteTriggered } = useSelector(bulkActionsDeleteSelector)
@@ -60,11 +57,8 @@ const BulkActionsConfig = ({ retryDelay = 5000 } : IProps) => {
 
     // Catch disconnect
     socketRef.current?.on(SocketEvent.Disconnect, () => {
-      if (retryDelay) {
-        retryTimer = setTimeout(handleDisconnect, retryDelay)
-      } else {
-        handleDisconnect()
-      }
+      dispatch(setDeleteOverviewStatus(BulkActionsStatus.Disconnected))
+      handleDisconnect()
     })
   }, [instanceId, isDeleteTriggered])
 
@@ -147,10 +141,8 @@ const BulkActionsConfig = ({ retryDelay = 5000 } : IProps) => {
   const onBulkDeleteAborted = (data: any) => {
     dispatch(setBulkDeleteLoading(false))
     sessionStorageService.set(BrowserStorageItem.bulkActionDeleteId, '')
-
-    if (data.status === 'aborted') {
-      dispatch(setDeleteOverview(data))
-    }
+    dispatch(setDeleteOverview(data))
+    handleDisconnect()
   }
 
   useEffect(() => {

redisinsight/ui/src/components/monitor-config/MonitorConfig.spec.tsx

@@ -9,7 +9,7 @@ import {
   pauseMonitor,
   setSocket,
   stopMonitor,
-  lockResume
+  lockResume, setLogFileId, setStartTimestamp
 } from 'uiSrc/slices/cli/monitor'
 import { cleanup, mockedStore, render } from 'uiSrc/utils/test-utils'
 import { MonitorEvent, SocketEvent } from 'uiSrc/constants'
@@ -69,26 +69,47 @@ describe('MonitorConfig', () => {
   it(`should emit ${MonitorEvent.Monitor} event`, () => {
     const monitorSelectorMock = jest.fn().mockReturnValue({
       isRunning: true,
+      isSaveToFile: true
     })
     monitorSelector.mockImplementation(monitorSelectorMock)
 
     const { unmount } = render(<MonitorConfig />)
 
-    socket.on(MonitorEvent.MonitorData, (data: []) => {
-      expect(data).toEqual(['message1', 'message2'])
+    socket.socketClient.on(MonitorEvent.Monitor, (data: any) => {
+      expect(data).toEqual({ logFileId: expect.any(String) })
     })
 
-    socket.socketClient.emit(MonitorEvent.MonitorData, ['message1', 'message2'])
+    socket.socketClient.emit(SocketEvent.Connect)
 
     const afterRenderActions = [
       setSocket(socket),
-      setMonitorLoadingPause(true)
+      setMonitorLoadingPause(true),
+      setLogFileId(expect.any(String)),
+      setStartTimestamp(expect.any(Number))
     ]
     expect(store.getActions()).toEqual([...afterRenderActions])
 
     unmount()
   })
 
+  it(`should not emit ${MonitorEvent.Monitor} event when paused`, () => {
+    const monitorSelectorMock = jest.fn().mockReturnValue({
+      isRunning: true,
+      isPaused: true
+    })
+    monitorSelector.mockImplementation(monitorSelectorMock)
+
+    const { unmount } = render(<MonitorConfig />)
+    const mockedMonitorEvent = jest.fn()
+
+    socket.socketClient.on(MonitorEvent.Monitor, mockedMonitorEvent)
+    socket.socketClient.emit(SocketEvent.Connect)
+
+    expect(mockedMonitorEvent).not.toBeCalled()
+
+    unmount()
+  })
+
   it('monitor should catch Exception', () => {
     const { unmount } = render(<MonitorConfig />)
 

redisinsight/ui/src/components/monitor-config/MonitorConfig.tsx

@@ -1,7 +1,7 @@
-import { useEffect } from 'react'
+import { useEffect, useRef } from 'react'
 import { useDispatch, useSelector } from 'react-redux'
 import { debounce } from 'lodash'
-import { io } from 'socket.io-client'
+import { io, Socket } from 'socket.io-client'
 import { v4 as uuidv4 } from 'uuid'
 
 import {
@@ -16,7 +16,7 @@ import {
   setLogFileId,
   pauseMonitor, lockResume
 } from 'uiSrc/slices/cli/monitor'
-import { getBaseApiUrl } from 'uiSrc/utils'
+import { getBaseApiUrl, Nullable } from 'uiSrc/utils'
 import { MonitorErrorMessages, MonitorEvent, SocketErrors, SocketEvent } from 'uiSrc/constants'
 import { IMonitorDataPayload } from 'uiSrc/slices/interfaces'
 import { connectedInstanceSelector } from 'uiSrc/slices/instances/instances'
@@ -26,12 +26,18 @@ import { IMonitorData } from 'apiSrc/modules/profiler/interfaces/monitor-data.in
 import ApiStatusCode from '../../constants/apiStatusCode'
 
 interface IProps {
-  retryDelay?: number;
+  retryDelay?: number
 }
 const MonitorConfig = ({ retryDelay = 15000 } : IProps) => {
   const { id: instanceId = '' } = useSelector(connectedInstanceSelector)
   const { socket, isRunning, isPaused, isSaveToFile, isMinimizedMonitor, isShowMonitor } = useSelector(monitorSelector)
 
+  const socketRef = useRef<Nullable<Socket>>(null)
+  const logFileIdRef = useRef<string>()
+  const timestampRef = useRef<number>()
+  const retryTimerRef = useRef<NodeJS.Timer>()
+  const payloadsRef = useRef<IMonitorDataPayload[]>([])
+
   const dispatch = useDispatch()
 
   const setNewItems = debounce((items, onSuccess?) => {
@@ -52,83 +58,80 @@ const MonitorConfig = ({ retryDelay = 15000 } : IProps) => {
     if (!isRunning || !instanceId || socket?.connected) {
       return
     }
-    const logFileId = `_redis_${uuidv4()}`
-    const timestamp = Date.now()
-    let retryTimer: NodeJS.Timer
+
+    logFileIdRef.current = `_redis_${uuidv4()}`
+    timestampRef.current = Date.now()
 
     // Create SocketIO connection to instance by instanceId
-    const newSocket = io(`${getBaseApiUrl()}/monitor`, {
+    socketRef.current = io(`${getBaseApiUrl()}/monitor`, {
       forceNew: true,
       query: { instanceId },
       extraHeaders: { [CustomHeaders.WindowId]: window.windowId || '' },
       rejectUnauthorized: false,
     })
-    dispatch(setSocket(newSocket))
-    let payloads: IMonitorDataPayload[] = []
-
-    const handleMonitorEvents = () => {
-      dispatch(setMonitorLoadingPause(false))
-      newSocket.on(MonitorEvent.MonitorData, (payload: IMonitorData[]) => {
-        payloads = payloads.concat(payload)
-
-        // set batch of payloads and then clear batch
-        setNewItems(payloads, () => {
-          payloads.length = 0
-          // reset all timings after items were changed
-          setNewItems.cancel()
-        })
-      })
-    }
+    dispatch(setSocket(socketRef.current))
 
     const handleDisconnect = () => {
-      newSocket.removeAllListeners()
+      socketRef.current?.removeAllListeners()
       dispatch(pauseMonitor())
       dispatch(stopMonitor())
       dispatch(lockResume())
     }
 
-    newSocket.on(SocketEvent.Connect, () => {
-      // Trigger Monitor event
-      clearTimeout(retryTimer)
-
-      dispatch(setLogFileId(logFileId))
-      dispatch(setStartTimestamp(timestamp))
-      newSocket.emit(
-        MonitorEvent.Monitor,
-        { logFileId: isSaveToFile ? logFileId : null },
-        handleMonitorEvents
-      )
-    })
-
     // Catch exceptions
-    newSocket.on(MonitorEvent.Exception, (payload) => {
+    socketRef.current?.on(MonitorEvent.Exception, (payload) => {
       if (payload.status === ApiStatusCode.Forbidden) {
         handleDisconnect()
         dispatch(setError(MonitorErrorMessages.NoPerm))
         dispatch(resetMonitorItems())
         return
       }
 
-      payloads.push({ isError: true, time: `${Date.now()}`, ...payload })
-      setNewItems(payloads, () => { payloads.length = 0 })
+      payloadsRef.current.push({ isError: true, time: `${Date.now()}`, ...payload })
+      setNewItems(payloadsRef.current, () => { payloads.length = 0 })
       dispatch(pauseMonitor())
     })
 
     // Catch disconnect
-    newSocket.on(SocketEvent.Disconnect, () => {
+    socketRef.current?.on(SocketEvent.Disconnect, () => {
       if (retryDelay) {
-        retryTimer = setTimeout(handleDisconnect, retryDelay)
+        retryTimerRef.current = setTimeout(handleDisconnect, retryDelay)
       } else {
         handleDisconnect()
       }
     })
 
     // Catch connect error
-    newSocket.on(SocketEvent.ConnectionError, (error) => {
-      payloads.push({ isError: true, time: `${Date.now()}`, message: getErrorMessage(error) })
-      setNewItems(payloads, () => { payloads.length = 0 })
+    socketRef.current?.on(SocketEvent.ConnectionError, (error) => {
+      payloadsRef.current.push({ isError: true, time: `${Date.now()}`, message: getErrorMessage(error) })
+      setNewItems(payloadsRef.current, () => { payloadsRef.current.length = 0 })
+    })
+  }, [instanceId, isRunning, isPaused])
+
+  useEffect(() => {
+    if (!isRunning) {
+      return
+    }
+
+    socketRef.current?.removeAllListeners(SocketEvent.Connect)
+    socketRef.current?.on(SocketEvent.Connect, () => {
+      // Trigger Monitor event
+      clearTimeout(retryTimerRef.current!)
+      dispatch(setLogFileId(logFileIdRef.current))
+      dispatch(setStartTimestamp(timestampRef.current))
+      if (!isPaused) {
+        subscribeMonitorEvents()
+      }
     })
-  }, [instanceId, isRunning, isSaveToFile])
+  }, [isRunning, isPaused])
+
+  useEffect(() => {
+    if (!isRunning || isPaused || !socketRef.current?.connected) {
+      return
+    }
+
+    subscribeMonitorEvents()
+  }, [isRunning, isPaused])
 
   useEffect(() => {
     if (!isRunning) return
@@ -150,6 +153,29 @@ const MonitorConfig = ({ retryDelay = 15000 } : IProps) => {
     }
   }, [socket, isRunning, isShowMonitor, isMinimizedMonitor])
 
+  const subscribeMonitorEvents = () => {
+    socketRef.current?.removeAllListeners(MonitorEvent.MonitorData)
+    socketRef.current?.emit(
+      MonitorEvent.Monitor,
+      { logFileId: isSaveToFile ? logFileIdRef.current : null },
+      handleMonitorEvents
+    )
+  }
+
+  const handleMonitorEvents = () => {
+    dispatch(setMonitorLoadingPause(false))
+    socketRef.current?.on(MonitorEvent.MonitorData, (payload: IMonitorData[]) => {
+      payloadsRef.current = payloadsRef.current.concat(payload)
+
+      // set batch of payloads and then clear batch
+      setNewItems(payloadsRef.current, () => {
+        payloadsRef.current.length = 0
+        // reset all timings after items were changed
+        setNewItems.cancel()
+      })
+    })
+  }
+
   return null
 }
 

redisinsight/ui/src/constants/bulkActions.ts

@@ -20,6 +20,7 @@ export enum BulkActionsStatus {
   Completed = 'completed',
   Failed = 'failed',
   Aborted = 'aborted',
+  Disconnected = 'disconnected'
 }
 
 export const MAX_BULK_ACTION_ERRORS_LENGTH = 500

redisinsight/ui/src/pages/browser/components/bulk-actions/BulkActionsInfo/BulkActionsInfo.spec.tsx

@@ -1,6 +1,6 @@
 import React from 'react'
 import { mock } from 'ts-mockito'
-import { KeyTypes } from 'uiSrc/constants'
+import { BulkActionsStatus, KeyTypes } from 'uiSrc/constants'
 import { render, screen } from 'uiSrc/utils/test-utils'
 
 import BulkActionsInfo, { Props } from './BulkActionsInfo'
@@ -25,4 +25,10 @@ describe('BulkActionsInfo', () => {
 
     expect(screen.queryByTestId('bulk-actions-info-filter')).not.toBeInTheDocument()
   })
+
+  it('should show connection lost when status is disconnect', () => {
+    render(<BulkActionsInfo {...mockedProps} filter={null} status={BulkActionsStatus.Disconnected} />)
+
+    expect(screen.getByTestId('bulk-status-disconnected')).toHaveTextContent('Connection Lost')
+  })
 })

redisinsight/ui/src/pages/browser/components/bulk-actions/BulkActionsInfo/BulkActionsInfo.tsx

@@ -7,6 +7,7 @@ import { getApproximatePercentage, Maybe, Nullable } from 'uiSrc/utils'
 import Divider from 'uiSrc/components/divider/Divider'
 import { BulkActionsStatus, KeyTypes } from 'uiSrc/constants'
 import GroupBadge from 'uiSrc/components/group-badge/GroupBadge'
+import { isProcessedBulkAction } from 'uiSrc/pages/browser/components/bulk-actions/utils'
 import styles from './styles.module.scss'
 
 export interface Props {
@@ -46,7 +47,7 @@ const BulkActionsInfo = (props: Props) => {
             </div>
           )}
         </EuiText>
-        {!isUndefined(status) && status !== BulkActionsStatus.Completed && status !== BulkActionsStatus.Aborted && (
+        {!isUndefined(status) && !isProcessedBulkAction(status) && (
           <EuiText color="subdued" className={styles.progress} data-testid="bulk-status-progress">
             In progress:
             <span>{` ${getApproximatePercentage(total, scanned)}`}</span>
@@ -62,6 +63,11 @@ const BulkActionsInfo = (props: Props) => {
             Action completed
           </EuiText>
         )}
+        {status === BulkActionsStatus.Disconnected && (
+          <EuiText color="danger" className={styles.progress} data-testid="bulk-status-disconnected">
+            Connection Lost: {getApproximatePercentage(total, scanned)}
+          </EuiText>
+        )}
       </div>
       <Divider colorVariable="separatorColor" className={styles.divider} />
       {loading && (

redisinsight/ui/src/pages/browser/components/bulk-actions/utils.ts

@@ -9,3 +9,4 @@ export const isProcessedBulkAction = (status?: BulkActionsStatus) =>
   status === BulkActionsStatus.Completed
     || status === BulkActionsStatus.Aborted
     || status === BulkActionsStatus.Failed
+    || status === BulkActionsStatus.Disconnected