From f008fd082e7ca8da211068db1fa3cb9351740b99 Mon Sep 17 00:00:00 2001 From: Charles Date: Thu, 5 May 2022 20:34:58 +0100 Subject: [PATCH] 2.6.23: websocket memory leak fix, fixes for device page refresh on websocket notification --- package-lock.json | 4 +-- package.json | 2 +- src/components/DeviceListTable/index.js | 26 ++++++++------- src/components/DeviceSearchBar/index.js | 10 +++--- src/components/RebootModal/index.js | 6 ---- src/contexts/WebSocketProvider/index.js | 6 ++-- .../WebSocketProvider/useSocketReducer.js | 32 +++++-------------- src/contexts/WebSocketProvider/utils.js | 1 + src/pages/DevicePage/index.js | 9 ++++-- src/utils/helper.js | 8 +++++ 10 files changed, 48 insertions(+), 56 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6764f0b..60960bb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "ucentral-client", - "version": "2.6.20", + "version": "2.6.23", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "ucentral-client", - "version": "2.6.20", + "version": "2.6.23", "dependencies": { "@coreui/coreui": "^3.4.0", "@coreui/icons": "^2.0.1", diff --git a/package.json b/package.json index 485d88d..76ce847 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ucentral-client", - "version": "2.6.20", + "version": "2.6.23", "dependencies": { "@coreui/coreui": "^3.4.0", "@coreui/icons": "^2.0.1", diff --git a/src/components/DeviceListTable/index.js b/src/components/DeviceListTable/index.js index ab05561..8e8ed7e 100644 --- a/src/components/DeviceListTable/index.js +++ b/src/components/DeviceListTable/index.js @@ -18,6 +18,7 @@ const DeviceList = () => { const { t } = useTranslation(); const { addToast } = useToast(); const history = useHistory(); + const [overrides, setOverrides] = useState({}); const [page, setPage] = useState(parseInt(sessionStorage.getItem('deviceTable') ?? 0, 10)); const { currentToken, endpoints } = useAuth(); const [upgradeStatus, setUpgradeStatus] = useState({ @@ -58,6 +59,7 @@ const DeviceList = () => { const getDeviceInformation = (selectedPage = page, devicePerPage = devicesPerPage) => { setLoading(true); + setOverrides({}); const options = { headers: { @@ -357,6 +359,15 @@ const DeviceList = () => { }); }; + const displayDevices = () => + devices.map((device) => ({ + ...device, + connected: + overrides[device.serialNumber] !== undefined + ? overrides[device.serialNumber] + : device.connected, + })); + useEffect(() => { getCount(); }, []); @@ -364,18 +375,11 @@ const DeviceList = () => { useEffect(() => { if (lastMessage && lastMessage.type === 'DEVICE') { const { serialNumber: msgSerial, isConnected } = lastMessage; - if (devices.find(({ serialNumber }) => serialNumber === msgSerial)) { - const newDevices = devices.map((device) => { - if (device.serialNumber !== msgSerial) return device; - return { - ...device, - connected: isConnected, - }; - }); - setDevices(newDevices); + if (overrides[msgSerial] === undefined || overrides[msgSerial] !== isConnected) { + setOverrides({ ...overrides, [msgSerial]: isConnected }); } } - }, [lastMessage, devices]); + }, [lastMessage, overrides]); useEffect(() => { if (upgradeStatus.result !== undefined) { @@ -400,7 +404,7 @@ const DeviceList = () => { currentPage={page} t={t} searchBar={} - devices={devices} + devices={displayDevices()} loading={loading} updateDevicesPerPage={updateDevicesPerPage} devicesPerPage={devicesPerPage} diff --git a/src/components/DeviceSearchBar/index.js b/src/components/DeviceSearchBar/index.js index 207626f..7bc65e4 100644 --- a/src/components/DeviceSearchBar/index.js +++ b/src/components/DeviceSearchBar/index.js @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; import { useTranslation } from 'react-i18next'; import { useHistory } from 'react-router-dom'; import { useAuth, DeviceSearchBar as SearchBar } from 'ucentral-libs'; -import { checkIfJson } from 'utils/helper'; +import { toJson } from 'utils/helper'; const DeviceSearchBar = ({ action }) => { const { t } = useTranslation(); @@ -44,11 +44,9 @@ const DeviceSearchBar = ({ action }) => { }; socket.onmessage = (event) => { - if (checkIfJson(event.data)) { - const result = JSON.parse(event.data); - if (result.serialNumbers) { - setResults(result.serialNumbers); - } + const result = toJson(event.data); + if (result && result.serialNumbers) { + setResults(result.serialNumbers); } }; diff --git a/src/components/RebootModal/index.js b/src/components/RebootModal/index.js index 59a3154..c8b09cb 100644 --- a/src/components/RebootModal/index.js +++ b/src/components/RebootModal/index.js @@ -87,12 +87,6 @@ const ActionModal = ({ show, toggleModal }) => { autohide: true, }), }); - addToast({ - title: t('common.success'), - body: t('commands.reboot_start'), - color: 'success', - autohide: true, - }); toggleModal(); }) .catch(() => { diff --git a/src/contexts/WebSocketProvider/index.js b/src/contexts/WebSocketProvider/index.js index cd74e1c..53b8b6a 100644 --- a/src/contexts/WebSocketProvider/index.js +++ b/src/contexts/WebSocketProvider/index.js @@ -8,7 +8,6 @@ import { extractWebSocketResponse } from './utils'; const WebSocketContext = React.createContext({ webSocket: undefined, isOpen: false, - allMessages: [], addDeviceListener: () => {}, }); @@ -16,7 +15,7 @@ export const WebSocketProvider = ({ children }) => { const { currentToken, endpoints } = useAuth(); const [isOpen, setIsOpen] = useState(false); const ws = useRef(undefined); - const { allMessages, lastMessage, dispatch } = useSocketReducer(); + const { lastMessage, dispatch } = useSocketReducer(); const { pushNotification } = useWebSocketNotification(); const onMessage = useCallback((message) => { @@ -68,7 +67,6 @@ export const WebSocketProvider = ({ children }) => { }, [ws?.current]); const values = useMemo( () => ({ - allMessages, lastMessage, webSocket: ws.current, addDeviceListener: ({ serialNumber, types, addToast, onTrigger }) => @@ -77,7 +75,7 @@ export const WebSocketProvider = ({ children }) => { dispatch({ type: 'REMOVE_DEVICE_LISTENER', serialNumber }), isOpen, }), - [ws, isOpen, allMessages, lastMessage], + [ws, isOpen, lastMessage], ); return {children}; diff --git a/src/contexts/WebSocketProvider/useSocketReducer.js b/src/contexts/WebSocketProvider/useSocketReducer.js index b5c1118..dc27293 100644 --- a/src/contexts/WebSocketProvider/useSocketReducer.js +++ b/src/contexts/WebSocketProvider/useSocketReducer.js @@ -15,7 +15,7 @@ const reducer = (state, action) => { switch (action.type) { case 'NEW_NOTIFICATION': { const obj = { type: 'NOTIFICATION', data: action.notification, timestamp: new Date() }; - return { allMessages: [...state.allMessages, obj], lastMessage: obj }; + return { ...state, lastMessage: obj }; } case 'NEW_COMMAND': { const obj = { @@ -24,7 +24,7 @@ const reducer = (state, action) => { timestamp: new Date(), id: action.data.command_response_id, }; - return { allMessages: [...state.allMessages, obj], lastMessage: obj }; + return { ...state, lastMessage: obj }; } case 'NEW_DEVICE_NOTIFICATION': { const newListeners = state.deviceListeners; @@ -59,11 +59,7 @@ const reducer = (state, action) => { } } - return { - allMessages: state.allMessages, - lastMessage: obj ?? state.lastMessage, - deviceListeners: newListeners, - }; + return { ...state, lastMessage: obj ?? state.lastMessage, deviceListeners: newListeners }; } case 'ADD_DEVICE_LISTENER': { let newListeners = action.types.map((actionType) => ({ @@ -73,29 +69,18 @@ const reducer = (state, action) => { onTrigger: action.onTrigger, })); newListeners = newListeners.concat(state.deviceListeners); - return { - allMessages: state.allMessages, - lastMessage: state.lastMessage, - deviceListeners: newListeners, - }; + return { ...state, lastMessage: state.lastMessage, deviceListeners: newListeners }; } case 'REMOVE_DEVICE_LISTENER': { const newListeners = state.deviceListeners.filter( (listener) => listener.serialNumber !== action.serialNumber || listener.onTrigger === undefined, ); - return { - allMessages: state.allMessages, - lastMessage: state.lastMessage, - deviceListeners: newListeners, - }; + return { ...state, lastMessage: state.lastMessage, deviceListeners: newListeners }; } case 'UNKNOWN': { const obj = { type: 'UNKNOWN', data: action.newMessage, timestamp: new Date() }; - return { - allMessages: [...state.allMessages, obj], - lastMessage: obj, - }; + return { ...state, lastMessage: obj }; } default: throw new Error(); @@ -103,12 +88,11 @@ const reducer = (state, action) => { }; const useSocketReducer = () => { - const [{ allMessages, lastMessage, deviceListeners }, dispatch] = useReducer(reducer, { - allMessages: [], + const [{ lastMessage, deviceListeners }, dispatch] = useReducer(reducer, { deviceListeners: [], }); - return { allMessages, lastMessage, deviceListeners, dispatch }; + return { lastMessage, deviceListeners, dispatch }; }; export default useSocketReducer; diff --git a/src/contexts/WebSocketProvider/utils.js b/src/contexts/WebSocketProvider/utils.js index 28a7d20..4204b41 100644 --- a/src/contexts/WebSocketProvider/utils.js +++ b/src/contexts/WebSocketProvider/utils.js @@ -6,6 +6,7 @@ export const deviceNotificationTypes = [ 'device_connection', 'device_disconnection', 'device_firmware_upgrade', + 'device_statistics', ]; export const extractWebSocketResponse = (message) => { diff --git a/src/pages/DevicePage/index.js b/src/pages/DevicePage/index.js index c8680fc..b456be2 100644 --- a/src/pages/DevicePage/index.js +++ b/src/pages/DevicePage/index.js @@ -119,8 +119,13 @@ const DevicePage = () => { if (deviceId) { addDeviceListener({ serialNumber: deviceId, - types: ['device_connection', 'device_disconnection', 'device_firmware_upgrade'], - onTrigger: () => getData(), + types: [ + 'device_connection', + 'device_disconnection', + 'device_firmware_upgrade', + 'device_statistics', + ], + onTrigger: () => refresh(), }); getDevice(); getData(); diff --git a/src/utils/helper.js b/src/utils/helper.js index cac8200..f59a200 100644 --- a/src/utils/helper.js +++ b/src/utils/helper.js @@ -59,6 +59,14 @@ export const checkIfJson = (string) => { return true; }; +export const toJson = (string) => { + try { + return JSON.parse(string); + } catch (e) { + return undefined; + } +}; + export const secondsToDetailed = ( seconds, dayLabel,