From ab49b17c7b0386006061a6786bbe0c187e781a6e Mon Sep 17 00:00:00 2001 From: Feiyu Wong Date: Thu, 28 Jul 2022 13:56:34 -0700 Subject: [PATCH] Save logs that are shown in logs plugin if persisted logging is enabled Summary: This is the second part of the feature which enables data/log persistence by adding a global setting which controls whether the data is wiped when the same device is disconnected and reconnected. Building on the previous diff, this diff uses the setting from the Redux Store to determine whether to "wipe" the data or not. Changelog: Added option in Flipper settings to persist device data upon reconnection instead of wiping everything. Reviewed By: mweststrate Differential Revision: D38076567 fbshipit-source-id: 83658ac09bc88921a56517e1a1f624d4a4bbc2c5 --- .../src/__tests__/disconnect.node.tsx | 165 +++++++++++++++++- .../src/dispatcher/flipperServer.tsx | 35 +++- 2 files changed, 191 insertions(+), 9 deletions(-) diff --git a/desktop/flipper-ui-core/src/__tests__/disconnect.node.tsx b/desktop/flipper-ui-core/src/__tests__/disconnect.node.tsx index 672c88779..3d1bfbe58 100644 --- a/desktop/flipper-ui-core/src/__tests__/disconnect.node.tsx +++ b/desktop/flipper-ui-core/src/__tests__/disconnect.node.tsx @@ -15,7 +15,11 @@ import { DevicePluginClient, PluginClient, } from 'flipper-plugin'; -import {handleClientConnected} from '../dispatcher/flipperServer'; +import { + handleClientConnected, + handleDeviceConnected, + handleDeviceDisconnected, +} from '../dispatcher/flipperServer'; import {TestDevice} from 'flipper-frontend-core'; test('Devices can disconnect', async () => { @@ -131,6 +135,165 @@ test('New device with same serial removes & cleans the old one', async () => { expect(store.getState().connections.devices[0]).toBe(device2); }); +test('Persist data enabled reconnect same device does not wipe data', async () => { + const deviceplugin = new _SandyPluginDefinition( + TestUtils.createMockPluginDetails({pluginType: 'device'}), + { + devicePlugin(client: DevicePluginClient) { + const destroy = jest.fn(); + client.onDestroy(destroy); + return { + destroy, + }; + }, + supportsDevice() { + return true; + }, + Component() { + return null; + }, + }, + ); + const {device, store, logger} = await createMockFlipperWithPlugin( + deviceplugin, + ); + const server = TestUtils.createFlipperServerMock({ + 'client-request-response': async () => ({ + success: [], + length: 0, + }), + }); + const instance = device.sandyPluginStates.get(deviceplugin.id)!; + + expect(device.isArchived).toBe(false); + expect(device.connected.get()).toBe(true); + expect(instance.instanceApi.destroy).toBeCalledTimes(0); + expect(store.getState().connections.devices[0]).toEqual(device); + expect(store.getState().settingsState.persistDeviceData).toBe(false); + //Update the persist data setting + store.dispatch({ + type: 'UPDATE_SETTINGS', + payload: {...store.getState().settingsState, persistDeviceData: true}, + }); + expect(store.getState().settingsState.persistDeviceData).toBe(true); + + handleDeviceConnected(server, store, logger, device.description); + expect(instance.instanceApi.destroy).toBeCalledTimes(0); + expect(store.getState().connections.devices.length).toBe(1); + expect(store.getState().connections.devices[0].sandyPluginStates).toBe( + device.sandyPluginStates, + ); +}); + +test('Persist data enabled multiple devices maintain same data', async () => { + const deviceplugin = new _SandyPluginDefinition( + TestUtils.createMockPluginDetails({pluginType: 'device'}), + { + devicePlugin(client: DevicePluginClient) { + const destroy = jest.fn(); + client.onDestroy(destroy); + return { + destroy, + }; + }, + supportsDevice() { + return true; + }, + Component() { + return null; + }, + }, + ); + const {device, store, logger, server} = await createMockFlipperWithPlugin( + deviceplugin, + ); + + const instance = device.sandyPluginStates.get(deviceplugin.id)!; + + expect(device.isArchived).toBe(false); + expect(device.connected.get()).toBe(true); + expect(instance.instanceApi.destroy).toBeCalledTimes(0); + expect(store.getState().connections.devices[0]).toEqual(device); + expect(store.getState().settingsState.persistDeviceData).toBe(false); + + store.dispatch({ + type: 'UPDATE_SETTINGS', + payload: {...store.getState().settingsState, persistDeviceData: true}, + }); + + expect(store.getState().settingsState.persistDeviceData).toBe(true); + + // Same device, data should be persisted + handleDeviceConnected(server, store, logger, device.description); + expect(instance.instanceApi.destroy).toBeCalledTimes(0); + expect(store.getState().connections.devices.length).toBe(1); + expect(store.getState().connections.devices[0]).toBe(device); + + const device2 = new TestDevice( + 'test2', + 'physical', + 'MockAndroidDevice', + 'Android', + ); + handleDeviceConnected(server, store, logger, device2.description); + expect(instance.instanceApi.destroy).toBeCalledTimes(0); + expect(store.getState().connections.devices.length).toBe(2); + expect(store.getState().connections.selectedDevice).not.toBe(device); + expect(store.getState().connections.selectedDevice?.serial).toBe( + device2.serial, + ); + + //Connect back the device1 + handleDeviceConnected(server, store, logger, device.description); + expect(instance.instanceApi.destroy).toBeCalledTimes(0); + expect(store.getState().connections.devices.length).toBe(2); + expect(store.getState().connections.selectedDevice).toBe(device); +}); + +test('Persist data enabled device is disconnected and reconnected properly', async () => { + const deviceplugin = new _SandyPluginDefinition( + TestUtils.createMockPluginDetails({pluginType: 'device'}), + { + devicePlugin(client: DevicePluginClient) { + const destroy = jest.fn(); + client.onDestroy(destroy); + return { + destroy, + }; + }, + supportsDevice() { + return true; + }, + Component() { + return null; + }, + }, + ); + const {device, store, logger, server} = await createMockFlipperWithPlugin( + deviceplugin, + ); + + const instance = device.sandyPluginStates.get(deviceplugin.id)!; + + expect(device.isArchived).toBe(false); + expect(device.connected.get()).toBe(true); + expect(instance.instanceApi.destroy).toBeCalledTimes(0); + expect(store.getState().connections.devices[0]).toEqual(device); + + handleDeviceDisconnected(store, logger, device.description); + expect(device.connected.get()).toBe(false); + + // We want to test with the same device + store.dispatch({ + type: 'UPDATE_SETTINGS', + payload: {...store.getState().settingsState, persistDeviceData: true}, + }); + + handleDeviceConnected(server, store, logger, device.description); + expect(device.connected.get()).toBe(true); + expect(store.getState().connections.selectedDevice).toBe(device); +}); + test('clients can disconnect but preserve state', async () => { const plugin = new _SandyPluginDefinition( TestUtils.createMockPluginDetails(), diff --git a/desktop/flipper-ui-core/src/dispatcher/flipperServer.tsx b/desktop/flipper-ui-core/src/dispatcher/flipperServer.tsx index a047f115c..48b581acb 100644 --- a/desktop/flipper-ui-core/src/dispatcher/flipperServer.tsx +++ b/desktop/flipper-ui-core/src/dispatcher/flipperServer.tsx @@ -72,12 +72,9 @@ export function connectFlipperServerToStore( handleDeviceConnected(server, store, logger, deviceInfo); }); - server.on('device-disconnected', (device) => { - logger.track('usage', 'unregister-device', { - os: device.os, - serial: device.serial, - }); + server.on('device-disconnected', (deviceInfo) => { // N.B.: note that we don't remove the device, we keep it in offline + handleDeviceDisconnected(store, logger, deviceInfo); }); server.on('client-setup', (client) => { @@ -202,7 +199,7 @@ function handleServerStateChange({ } } -function handleDeviceConnected( +export function handleDeviceConnected( server: FlipperServer, store: Store, logger: Logger, @@ -224,6 +221,15 @@ function handleDeviceConnected( `Tried to replace still connected device '${existing.serial}' with a new instance.`, ); } + if (store.getState().settingsState.persistDeviceData) { + //Recycle device + existing?.connected.set(true); + store.dispatch({ + type: 'SELECT_DEVICE', + payload: existing, + }); + return; + } existing.destroy(); } @@ -232,13 +238,27 @@ function handleDeviceConnected( store.getState().plugins.devicePlugins, store.getState().connections.enabledDevicePlugins, ); - store.dispatch({ type: 'REGISTER_DEVICE', payload: device, }); } +export function handleDeviceDisconnected( + store: Store, + logger: Logger, + deviceInfo: DeviceDescription, +) { + logger.track('usage', 'unregister-device', { + os: deviceInfo.os, + serial: deviceInfo.serial, + }); + const existing = store + .getState() + .connections.devices.find((device) => device.serial === deviceInfo.serial); + existing?.connected.set(false); +} + export async function handleClientConnected( server: FlipperServer, store: Store, @@ -247,7 +267,6 @@ export async function handleClientConnected( ) { const {connections} = store.getState(); const existingClient = connections.clients.get(id); - if (existingClient) { existingClient.destroy(); store.dispatch({