diff --git a/desktop/app/src/Client.tsx b/desktop/app/src/Client.tsx index 6d768ba7c..b216a0354 100644 --- a/desktop/app/src/Client.tsx +++ b/desktop/app/src/Client.tsx @@ -336,6 +336,12 @@ export default class Client extends EventEmitter { } stopPluginIfNeeded(pluginId: string) { + const pluginKey = getPluginKey( + this.id, + {serial: this.query.device_id}, + pluginId, + ); + delete this.messageBuffer[pluginKey]; const instance = this.sandyPluginStates.get(pluginId); if (instance) { instance.destroy(); diff --git a/desktop/app/src/reducers/connections.tsx b/desktop/app/src/reducers/connections.tsx index 03c66f385..188f05b2b 100644 --- a/desktop/app/src/reducers/connections.tsx +++ b/desktop/app/src/reducers/connections.tsx @@ -135,6 +135,7 @@ export type Action = payload: number; } | { + // Implemented by rootReducer in `store.tsx` type: 'STAR_PLUGIN'; payload: { selectedApp: string; @@ -277,51 +278,6 @@ export default (state: State = INITAL_STATE, action: Actions): State => { }); } - case 'STAR_PLUGIN': { - const {plugin, selectedApp} = action.payload; - const selectedPlugin = plugin.id; - const clients = state.clients.filter( - (client) => client.query.app === selectedApp, - ); - return produce(state, (draft) => { - if (!draft.userStarredPlugins[selectedApp]) { - draft.userStarredPlugins[selectedApp] = []; - } - const plugins = draft.userStarredPlugins[selectedApp]; - const idx = plugins.indexOf(selectedPlugin); - if (idx === -1) { - plugins.push(selectedPlugin); - // enabling a plugin on one device enables it on all... - clients.forEach((client) => { - // sandy plugin? initialize it - client.startPluginIfNeeded(plugin, true); - // background plugin? connect it needed - if ( - !defaultEnabledBackgroundPlugins.includes(selectedPlugin) && - client?.isBackgroundPlugin(selectedPlugin) - ) { - client.initPlugin(selectedPlugin); - } - }); - } else { - plugins.splice(idx, 1); - // enabling a plugin on one device disables it on all... - clients.forEach((client) => { - // disconnect background plugins - if ( - !defaultEnabledBackgroundPlugins.includes(selectedPlugin) && - client?.isBackgroundPlugin(selectedPlugin) - ) { - client.deinitPlugin(selectedPlugin); - } - // stop sandy plugins - // TODO: forget any persisted state as well T68683449 - client.stopPluginIfNeeded(plugin.id); - }); - } - }); - } - case 'SELECT_USER_PREFERRED_PLUGIN': { const {payload} = action; return {...state, userPreferredPlugin: payload}; diff --git a/desktop/app/src/store.tsx b/desktop/app/src/store.tsx index f6186360a..d73d8cefc 100644 --- a/desktop/app/src/store.tsx +++ b/desktop/app/src/store.tsx @@ -11,9 +11,14 @@ import {createStore} from 'redux'; import reducers, {Actions, State as StoreState} from './reducers/index'; import {stateSanitizer} from './utils/reduxDevToolsConfig'; import isProduction from './utils/isProduction'; +import produce from 'immer'; +import { + defaultEnabledBackgroundPlugins, + getPluginKey, +} from './utils/pluginUtils'; export const store = createStore( - reducers, + rootReducer, // @ts-ignore Type definition mismatch window.__REDUX_DEVTOOLS_EXTENSION__ ? window.__REDUX_DEVTOOLS_EXTENSION__({ @@ -23,6 +28,62 @@ export const store = createStore( : undefined, ); +export function rootReducer( + state: StoreState | undefined, + action: Actions, +): StoreState { + if (action.type === 'STAR_PLUGIN' && state) { + const {plugin, selectedApp} = action.payload; + const selectedPlugin = plugin.id; + const clients = state.connections.clients.filter( + (client) => client.query.app === selectedApp, + ); + return produce(state, (draft) => { + if (!draft.connections.userStarredPlugins[selectedApp]) { + draft.connections.userStarredPlugins[selectedApp] = []; + } + const plugins = draft.connections.userStarredPlugins[selectedApp]; + const idx = plugins.indexOf(selectedPlugin); + if (idx === -1) { + plugins.push(selectedPlugin); + // enabling a plugin on one device enables it on all... + clients.forEach((client) => { + // sandy plugin? initialize it + client.startPluginIfNeeded(plugin, true); + // background plugin? connect it needed + if ( + !defaultEnabledBackgroundPlugins.includes(selectedPlugin) && + client?.isBackgroundPlugin(selectedPlugin) + ) { + client.initPlugin(selectedPlugin); + } + }); + } else { + plugins.splice(idx, 1); + // enabling a plugin on one device disables it on all... + clients.forEach((client) => { + // disconnect background plugins + if ( + !defaultEnabledBackgroundPlugins.includes(selectedPlugin) && + client?.isBackgroundPlugin(selectedPlugin) + ) { + client.deinitPlugin(selectedPlugin); + } + // stop sandy plugins + // TODO: forget any persisted state as well T68683449 + client.stopPluginIfNeeded(plugin.id); + delete draft.pluginMessageQueue[ + getPluginKey(client.id, {serial: client.query.device_id}, plugin.id) + ]; + }); + } + }); + } + + // otherwise + return reducers(state, action); +} + if (!isProduction()) { // For debugging purposes only // @ts-ignore diff --git a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx index 9a1253d17..d9af3ad90 100644 --- a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx +++ b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx @@ -24,7 +24,8 @@ import { } from '../reducers/connections'; import BaseDevice from '../devices/BaseDevice'; -import reducers, {Store} from '../reducers/index'; +import {rootReducer} from '../store'; +import {Store} from '../reducers/index'; import Client, {ClientQuery} from '../Client'; import {buildClientId} from '../utils/clientUtils'; @@ -58,7 +59,7 @@ export async function createMockFlipperWithPlugin( pluginClazz: PluginDefinition, options?: MockOptions, ): Promise { - const store = createStore(reducers); + const store = createStore(rootReducer); const logger = getInstance(); store.dispatch(registerPlugins([pluginClazz])); diff --git a/desktop/app/src/utils/__tests__/messageQueue.node.tsx b/desktop/app/src/utils/__tests__/messageQueue.node.tsx index 585605ae9..fe69a0a7b 100644 --- a/desktop/app/src/utils/__tests__/messageQueue.node.tsx +++ b/desktop/app/src/utils/__tests__/messageQueue.node.tsx @@ -190,9 +190,7 @@ test('queue - events are NOT processed immediately if plugin is NOT selected (bu selectDeviceLogs(store); sendMessage('inc', {delta: 4}); client.flushMessageBuffer(); - expect(store.getState().pluginMessageQueue).toEqual({ - [pluginKey]: [], - }); + expect(store.getState().pluginMessageQueue).toEqual({}); // star again, plugin still not selected, message is queued starTestPlugin(store, client); @@ -646,3 +644,57 @@ test('client - incoming messages are buffered and flushed together', async () => } `); }); + +test('queue - messages that have not yet flushed be lost when disabling the plugin', async () => { + const { + client, + store, + sendMessage, + pluginKey, + } = await createMockFlipperWithPlugin(TestPlugin); + selectDeviceLogs(store); + + sendMessage('inc', {}); + sendMessage('inc', {delta: 2}); + + expect(client.messageBuffer).toMatchInlineSnapshot(` + Object { + "TestApp#Android#MockAndroidDevice#serial#TestPlugin": Object { + "messages": Array [ + Object { + "api": "TestPlugin", + "method": "inc", + "params": Object { + "delta": 2, + }, + }, + ], + "plugin": [Function], + }, + } + `); + expect(store.getState().pluginMessageQueue).toMatchInlineSnapshot(` + Object { + "TestApp#Android#MockAndroidDevice#serial#TestPlugin": Array [ + Object { + "api": "TestPlugin", + "method": "inc", + "params": Object {}, + }, + ], + } + `); + + // disable + starTestPlugin(store, client); + expect(client.messageBuffer).toMatchInlineSnapshot(`Object {}`); + expect(store.getState().pluginMessageQueue).toMatchInlineSnapshot( + `Object {}`, + ); + + // re-enable, no messages arrive + starTestPlugin(store, client); + client.flushMessageBuffer(); + processMessageQueue(TestPlugin, pluginKey, store); + expect(store.getState().pluginStates).toMatchInlineSnapshot(`Object {}`); +}); diff --git a/desktop/app/src/utils/__tests__/messageQueueSandy.node.tsx b/desktop/app/src/utils/__tests__/messageQueueSandy.node.tsx index bcedee7a4..002a492e4 100644 --- a/desktop/app/src/utils/__tests__/messageQueueSandy.node.tsx +++ b/desktop/app/src/utils/__tests__/messageQueueSandy.node.tsx @@ -197,9 +197,8 @@ test('queue - events are NOT processed immediately if plugin is NOT selected (bu selectDeviceLogs(store); sendMessage('inc', {delta: 4}); client.flushMessageBuffer(); - expect(store.getState().pluginMessageQueue).toEqual({ - [pluginKey]: [], - }); + expect(client.messageBuffer).toMatchInlineSnapshot(`Object {}`); + expect(store.getState().pluginMessageQueue).toEqual({}); // star again, plugin still not selected, message is queued starTestPlugin(store, client); @@ -637,3 +636,61 @@ test('client - incoming messages are buffered and flushed together', async () => } `); }); + +test('queue - messages that have not yet flushed be lost when disabling the plugin', async () => { + const { + client, + store, + sendMessage, + pluginKey, + } = await createMockFlipperWithPlugin(TestPlugin); + selectDeviceLogs(store); + + sendMessage('inc', {}); + sendMessage('inc', {delta: 2}); + + expect(client.messageBuffer).toMatchInlineSnapshot(` + Object { + "TestApp#Android#MockAndroidDevice#serial#TestPlugin": Object { + "messages": Array [ + Object { + "api": "TestPlugin", + "method": "inc", + "params": Object { + "delta": 2, + }, + }, + ], + "plugin": undefined, + }, + } + `); + expect(store.getState().pluginMessageQueue).toMatchInlineSnapshot(` + Object { + "TestApp#Android#MockAndroidDevice#serial#TestPlugin": Array [ + Object { + "api": "TestPlugin", + "method": "inc", + "params": Object {}, + }, + ], + } + `); + + // disable + starTestPlugin(store, client); + expect(client.messageBuffer).toMatchInlineSnapshot(`Object {}`); + expect(store.getState().pluginMessageQueue).toMatchInlineSnapshot( + `Object {}`, + ); + + // re-enable, no messages arrive + starTestPlugin(store, client); + client.flushMessageBuffer(); + processMessageQueue( + client.sandyPluginStates.get(TestPlugin.id)!, + pluginKey, + store, + ); + expect(getTestPluginState(client)).toEqual({count: 0}); +});