From 18915ba43c500f4ff69a036a3144a1efd1867986 Mon Sep 17 00:00:00 2001 From: Pritesh Nandgaonkar Date: Mon, 16 Mar 2020 05:20:11 -0700 Subject: [PATCH] Fix the bug where flipper tried to fetch meta data from archived device after importing from same device Summary: This diff fixes an issue where when one imports an archived device and tries to export again through the same device with which the initial import was made then the export functionality tries to also fetch metadata from the archived device, it waits for the metadata till it timeouts. Also this diff fixes an issue where if the selected plugin is not present in the `selectedClient` then it will also ignore that plugin from the other clients even if it was supported by those clients. Unit test cases for all the above cases are added. Bug: {F231519408} Reviewed By: mweststrate Differential Revision: D20459179 fbshipit-source-id: 4f0d8c40bec875e3cc43cd6aa70061c8b8da7b05 --- .../utils/__tests__/exportData.electron.tsx | 283 +++++++++++++++++- desktop/src/utils/exportData.tsx | 58 ++-- desktop/src/utils/exportMetrics.tsx | 7 +- 3 files changed, 315 insertions(+), 33 deletions(-) diff --git a/desktop/src/utils/__tests__/exportData.electron.tsx b/desktop/src/utils/__tests__/exportData.electron.tsx index 0538198c5..d7d7cc97e 100644 --- a/desktop/src/utils/__tests__/exportData.electron.tsx +++ b/desktop/src/utils/__tests__/exportData.electron.tsx @@ -12,16 +12,31 @@ try { } catch (e) { jest.mock('../../fb-stubs/Logger'); } - +import {State} from '../../reducers/index'; +import configureStore from 'redux-mock-store'; import {default as BaseDevice} from '../../devices/BaseDevice'; import {default as ArchivedDevice} from '../../devices/ArchivedDevice'; -import {processStore} from '../exportData'; +import {processStore, determinePluginsToProcess} from '../exportData'; import {FlipperPlugin, FlipperDevicePlugin} from '../../plugin'; import {Notification} from '../../plugin'; -import {ClientExport} from '../../Client'; +import {default as Client, ClientExport} from '../../Client'; +import {State as PluginsState} from '../../reducers/plugins'; class TestPlugin extends FlipperPlugin {} +TestPlugin.title = 'TestPlugin'; +TestPlugin.id = 'TestPlugin'; class TestDevicePlugin extends FlipperDevicePlugin {} +TestDevicePlugin.title = 'TestDevicePlugin'; +TestDevicePlugin.id = 'TestDevicePlugin'; +const logger = { + track: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + debug: () => {}, + trackTimeSince: () => {}, +}; +const mockStore = configureStore([])(); function generateNotifications( id: string, @@ -672,3 +687,265 @@ test('test processStore function for no selected plugins', async () => { const {activeNotifications} = json.store; expect(activeNotifications).toEqual([]); }); + +test('test determinePluginsToProcess for mutilple clients having plugins present', async () => { + const device1 = new BaseDevice('serial1', 'emulator', 'TestiPhone', 'iOS'); + const client1 = new Client( + generateClientIdentifier(device1, 'app'), + {app: 'app', os: 'iOS', device: 'TestiPhone', device_id: 'serial1'}, + null, + logger, + mockStore, + ['TestPlugin', 'TestDevicePlugin'], + ); + const client2 = new Client( + generateClientIdentifier(device1, 'app2'), + {app: 'app2', os: 'iOS', device: 'TestiPhone', device_id: 'serial1'}, + null, + logger, + mockStore, + ['TestDevicePlugin'], + ); + const client3 = new Client( + generateClientIdentifier(device1, 'app3'), + {app: 'app3', os: 'iOS', device: 'TestiPhone', device_id: 'serial1'}, + null, + logger, + mockStore, + ['TestPlugin', 'TestDevicePlugin'], + ); + const plugins: PluginsState = { + clientPlugins: new Map([ + ['TestPlugin', TestPlugin], + ['RandomPlugin', TestPlugin], + ]), + devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), + gatekeepedPlugins: [], + disabledPlugins: [], + failedPlugins: [], + selectedPlugins: ['TestPlugin'], + }; + const op = determinePluginsToProcess( + [client1, client2, client3], + device1, + plugins, + ); + expect(op).toBeDefined(); + expect(op).toEqual([ + { + pluginKey: `${client1.id}#TestPlugin`, + pluginId: 'TestPlugin', + pluginName: 'TestPlugin', + pluginClass: TestPlugin, + client: client1, + }, + { + pluginKey: `${client3.id}#TestPlugin`, + pluginId: 'TestPlugin', + pluginName: 'TestPlugin', + pluginClass: TestPlugin, + client: client3, + }, + ]); +}); + +test('test determinePluginsToProcess for no selected plugin present in any clients', async () => { + const device1 = new BaseDevice('serial1', 'emulator', 'TestiPhone', 'iOS'); + const client1 = new Client( + generateClientIdentifier(device1, 'app'), + {app: 'app', os: 'iOS', device: 'TestiPhone', device_id: 'serial1'}, + null, + logger, + mockStore, + ['TestPlugin', 'TestDevicePlugin'], + ); + const client2 = new Client( + generateClientIdentifier(device1, 'app2'), + {app: 'app2', os: 'iOS', device: 'TestiPhone', device_id: 'serial1'}, + null, + logger, + mockStore, + ['TestDevicePlugin'], + ); + const plugins: PluginsState = { + clientPlugins: new Map([ + ['TestPlugin', TestPlugin], + ['RandomPlugin', TestPlugin], + ]), + devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), + gatekeepedPlugins: [], + disabledPlugins: [], + failedPlugins: [], + selectedPlugins: ['RandomPlugin'], + }; + const op = determinePluginsToProcess([client1, client2], device1, plugins); + expect(op).toBeDefined(); + expect(op).toEqual([]); +}); + +test('test determinePluginsToProcess for multiple clients on same device', async () => { + const device1 = new BaseDevice('serial1', 'emulator', 'TestiPhone', 'iOS'); + const client1 = new Client( + generateClientIdentifier(device1, 'app'), + {app: 'app', os: 'iOS', device: 'TestiPhone', device_id: 'serial1'}, + null, + logger, + mockStore, + ['TestPlugin', 'TestDevicePlugin'], + ); + const client2 = new Client( + generateClientIdentifier(device1, 'app2'), + {app: 'app2', os: 'iOS', device: 'TestiPhone', device_id: 'serial1'}, + null, + logger, + mockStore, + ['TestDevicePlugin'], + ); + const plugins: PluginsState = { + clientPlugins: new Map([['TestPlugin', TestPlugin]]), + devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), + gatekeepedPlugins: [], + disabledPlugins: [], + failedPlugins: [], + selectedPlugins: ['TestPlugin'], + }; + const op = determinePluginsToProcess([client1, client2], device1, plugins); + expect(op).toBeDefined(); + expect(op.length).toEqual(1); + expect(op).toEqual([ + { + pluginKey: `${client1.id}#TestPlugin`, + pluginId: 'TestPlugin', + pluginName: 'TestPlugin', + pluginClass: TestPlugin, + client: client1, + }, + ]); +}); + +test('test determinePluginsToProcess for multiple clients on different device', async () => { + const device1 = new BaseDevice('serial1', 'emulator', 'TestiPhone', 'iOS'); + const device2 = new BaseDevice('serial2', 'emulator', 'TestiPhone', 'iOS'); + const client1Device1 = new Client( + generateClientIdentifier(device1, 'app'), + {app: 'app', os: 'iOS', device: 'TestiPhone', device_id: 'serial1'}, + null, + logger, + mockStore, + ['TestPlugin', 'TestDevicePlugin'], + ); + const client2Device1 = new Client( + generateClientIdentifier(device1, 'app2'), + {app: 'app1', os: 'iOS', device: 'TestiPhone', device_id: 'serial1'}, + null, + logger, + mockStore, + ['TestDevicePlugin'], + ); + const client1Device2 = new Client( + generateClientIdentifier(device2, 'app'), + {app: 'app', os: 'iOS', device: 'TestiPhone', device_id: 'serial2'}, + null, + logger, + mockStore, + ['TestPlugin', 'TestDevicePlugin'], + ); + const client2Device2 = new Client( + generateClientIdentifier(device2, 'app2'), + {app: 'app1', os: 'iOS', device: 'TestiPhone', device_id: 'serial2'}, + null, + logger, + mockStore, + ['TestDevicePlugin'], + ); + const plugins: PluginsState = { + clientPlugins: new Map([['TestPlugin', TestPlugin]]), + devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), + gatekeepedPlugins: [], + disabledPlugins: [], + failedPlugins: [], + selectedPlugins: ['TestPlugin'], + }; + const op = determinePluginsToProcess( + [client1Device1, client2Device1, client1Device2, client2Device2], + device2, + plugins, + ); + expect(op).toBeDefined(); + expect(op.length).toEqual(1); + expect(op).toEqual([ + { + pluginKey: `${client1Device2.id}#TestPlugin`, + pluginId: 'TestPlugin', + pluginName: 'TestPlugin', + pluginClass: TestPlugin, + client: client1Device2, + }, + ]); +}); + +test('test determinePluginsToProcess to ignore archived clients', async () => { + const selectedDevice = new BaseDevice( + 'serial', + 'emulator', + 'TestiPhone', + 'iOS', + ); + const archivedDevice = new ArchivedDevice({ + serial: 'serial-archived', + deviceType: 'emulator', + title: 'TestiPhone', + os: 'iOS', + logEntries: [], + screenshotHandle: null, + }); + const logger = { + track: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + debug: () => {}, + trackTimeSince: () => {}, + }; + const mockStore = configureStore([])(); + const client = new Client( + generateClientIdentifier(selectedDevice, 'app'), + {app: 'app', os: 'iOS', device: 'TestiPhone', device_id: 'serial'}, + null, + logger, + mockStore, + ['TestPlugin', 'TestDevicePlugin'], + ); + const archivedClient = new Client( + generateClientIdentifier(archivedDevice, 'app'), + {app: 'app', os: 'iOS', device: 'TestiPhone', device_id: 'serial-archived'}, + null, + logger, + mockStore, + ['TestPlugin', 'TestDevicePlugin'], + ); + const plugins: PluginsState = { + clientPlugins: new Map([['TestPlugin', TestPlugin]]), + devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), + gatekeepedPlugins: [], + disabledPlugins: [], + failedPlugins: [], + selectedPlugins: ['TestPlugin'], + }; + const op = determinePluginsToProcess( + [client, archivedClient], + selectedDevice, + plugins, + ); + expect(op).toBeDefined(); + expect(op.length).toEqual(1); + expect(op).toEqual([ + { + pluginKey: `${client.id}#TestPlugin`, + pluginId: 'TestPlugin', + pluginName: 'TestPlugin', + pluginClass: TestPlugin, + client: client, + }, + ]); +}); diff --git a/desktop/src/utils/exportData.tsx b/desktop/src/utils/exportData.tsx index cbfeb9f24..fc0703166 100644 --- a/desktop/src/utils/exportData.tsx +++ b/desktop/src/utils/exportData.tsx @@ -14,6 +14,7 @@ import {getInstance as getLogger} from '../fb-stubs/Logger'; import {Store, State as ReduxState, MiddlewareAPI} from '../reducers'; import {DeviceExport} from '../devices/BaseDevice'; import {State as PluginStatesState} from '../reducers/pluginStates'; +import {State as PluginsState} from '../reducers/plugins'; import {PluginNotification} from '../reducers/notifications'; import Client, {ClientExport, ClientQuery} from '../Client'; import {pluginKey} from '../reducers/pluginStates'; @@ -502,38 +503,32 @@ async function processQueues( } } -function getSelection( - store: MiddlewareAPI, -): {client: Client | null; device: BaseDevice | null} { - const state = store.getState(); - const {clients} = state.connections; - const client = clients.find( - client => client.id === state.connections.selectedApp, - ); - const {selectedDevice} = state.connections; - return {client: client ?? null, device: selectedDevice}; -} - export function determinePluginsToProcess( - store: MiddlewareAPI, + clients: Array, + selectedDevice: null | BaseDevice, + plugins: PluginsState, ): PluginsToProcess { - const state = store.getState(); - const {plugins} = state; - const {clients} = state.connections; - const {client, device} = getSelection(store); - const pluginsToProcess: PluginsToProcess = []; - const selectedPlugins = state.plugins.selectedPlugins; - const selectedFilteredPlugins = client - ? selectedPlugins.length > 0 - ? client.plugins.filter(plugin => selectedPlugins.includes(plugin)) - : client.plugins - : []; + const selectedPlugins = plugins.selectedPlugins; + for (const client of clients) { - if (!device || device.isArchived || !client.id.includes(device.serial)) { + if ( + !selectedDevice || + selectedDevice.isArchived || + client.query.device_id !== selectedDevice.serial + ) { continue; } + const selectedFilteredPlugins = client + ? selectedPlugins.length > 0 + ? client.plugins.filter(plugin => selectedPlugins.includes(plugin)) + : client.plugins + : []; for (const plugin of selectedFilteredPlugins) { + if (!client.plugins.includes(plugin)) { + // Ignore clients which doesn't support the selected plugins. + continue; + } const pluginClass = plugins.clientPlugins.get(plugin) || plugins.devicePlugins.get(plugin); if (pluginClass) { @@ -556,7 +551,13 @@ export async function getStoreExport( statusUpdate?: (msg: string) => void, idler?: Idler, ): Promise<{exportData: ExportType | null; errorArray: Array}> { - const pluginsToProcess = determinePluginsToProcess(store); + const state = store.getState(); + const {clients, selectedApp, selectedDevice} = state.connections; + const pluginsToProcess = determinePluginsToProcess( + clients, + selectedDevice, + state.plugins, + ); statusUpdate?.('Preparing to process data queues for plugins...'); await processQueues(store, pluginsToProcess, statusUpdate, idler); @@ -565,8 +566,7 @@ export async function getStoreExport( const fetchMetaDataMarker = `${EXPORT_FLIPPER_TRACE_EVENT}:fetch-meta-data`; performance.mark(fetchMetaDataMarker); - const {device, client} = getSelection(store); - const state = store.getState(); + const client = clients.find(client => client.id === selectedApp); const metadata = await fetchMetadata( pluginsToProcess, state.pluginStates, @@ -586,7 +586,7 @@ export async function getStoreExport( const exportData = await processStore( { activeNotifications, - device, + device: selectedDevice, pluginStates: newPluginState, clients: client ? [client.toJSON()] : [], devicePlugins, diff --git a/desktop/src/utils/exportMetrics.tsx b/desktop/src/utils/exportMetrics.tsx index 30b520189..e8b0ba2c7 100644 --- a/desktop/src/utils/exportMetrics.tsx +++ b/desktop/src/utils/exportMetrics.tsx @@ -71,7 +71,12 @@ export async function exportMetricsWithoutTrace( string, typeof FlipperDevicePlugin | typeof FlipperPlugin > = pluginsClassMap(store.getState().plugins); - const pluginsToProcess = determinePluginsToProcess(store); + const {clients, selectedDevice} = store.getState().connections; + const pluginsToProcess = determinePluginsToProcess( + clients, + selectedDevice, + store.getState().plugins, + ); const metadata = await fetchMetadata( pluginsToProcess, pluginStates,