From 0bf905e02f8a037151b0d7a9c988d70e6149ce2c Mon Sep 17 00:00:00 2001 From: John Knox Date: Thu, 5 Dec 2019 07:36:18 -0800 Subject: [PATCH] Replace all manual pluginKey parsing with a utility Summary: Ok this diff got a bit bigger than expected, but I think it makes it easier to understand what "plugin keys" are, and makes them less prone to error. Previously pluginKeys were composed of: clientId#pluginName, where clientId was itself: app#os#device#device_id But also, there were some plugin keys where the clientId was a device_id. Now you deconstruct a plugin key, and will get a tagged object with type: 'device' or 'client', and the properties that they each have. There is now no custom parsing of these afaik, let's keep it that way. Since it took me a while to figure out what all these IDs are, I've documented it a bit in clientUtils. Reviewed By: passy Differential Revision: D18811848 fbshipit-source-id: eed2e2b5eedafb9e27900dbcf79a389fcaffae95 --- src/dispatcher/notifications.tsx | 13 ++++---- src/reducers/pluginStates.tsx | 6 ++-- src/utils/clientUtils.tsx | 51 ++++++++++++++++++++++++++++++++ src/utils/exportData.tsx | 45 ++++++++++++++-------------- src/utils/exportMetrics.tsx | 20 +++++++------ src/utils/pluginUtils.tsx | 13 ++++---- 6 files changed, 104 insertions(+), 44 deletions(-) diff --git a/src/dispatcher/notifications.tsx b/src/dispatcher/notifications.tsx index c8c144daa..df9f65f14 100644 --- a/src/dispatcher/notifications.tsx +++ b/src/dispatcher/notifications.tsx @@ -21,6 +21,7 @@ import { } from '../reducers/notifications'; import {textContent} from '../utils/index'; import GK from '../fb-stubs/GK'; +import {deconstructPluginKey} from '../utils/clientUtils'; type NotificationEvents = 'show' | 'click' | 'close' | 'reply' | 'action'; const NOTIFICATION_THROTTLE = 5 * 1000; // in milliseconds @@ -106,18 +107,18 @@ export default (store: Store, logger: Logger) => { Object.keys(pluginStates).forEach(key => { if (knownPluginStates.get(key) !== pluginStates[key]) { knownPluginStates.set(key, pluginStates[key]); - const split = key.split('#'); - const pluginId = split.pop(); - const client = split.join('#'); + const plugin = deconstructPluginKey(key); + const pluginName = plugin.pluginName; + const client = plugin.client; - if (!pluginId) { + if (!pluginName) { return; } const persistingPlugin: | undefined | typeof FlipperPlugin - | typeof FlipperDevicePlugin = pluginMap.get(pluginId); + | typeof FlipperDevicePlugin = pluginMap.get(pluginName); if (persistingPlugin && persistingPlugin.getActiveNotifications) { store.dispatch( setActiveNotifications({ @@ -125,7 +126,7 @@ export default (store: Store, logger: Logger) => { pluginStates[key], ), client, - pluginId, + pluginId: pluginName, }), ); } diff --git a/src/reducers/pluginStates.tsx b/src/reducers/pluginStates.tsx index 62a068fe7..52e1c9b4f 100644 --- a/src/reducers/pluginStates.tsx +++ b/src/reducers/pluginStates.tsx @@ -8,6 +8,7 @@ */ import {Actions} from '.'; +import {deconstructPluginKey} from '../utils/clientUtils'; export type State = { [pluginKey: string]: Object; @@ -53,8 +54,9 @@ export default function reducer( return Object.keys(state).reduce((newState: State, pluginKey) => { // Only add the pluginState, if its from a plugin other than the one that // was removed. pluginKeys are in the form of ${clientID}#${pluginID}. - const clientId = pluginKey.slice(0, pluginKey.lastIndexOf('#')); - const pluginId = pluginKey.split('#').pop(); + const plugin = deconstructPluginKey(pluginKey); + const clientId = plugin.client; + const pluginId = plugin.pluginName; if ( clientId !== payload.clientId || (pluginId && payload.devicePlugins.has(pluginId)) diff --git a/src/utils/clientUtils.tsx b/src/utils/clientUtils.tsx index 5f971af55..0ba653584 100644 --- a/src/utils/clientUtils.tsx +++ b/src/utils/clientUtils.tsx @@ -10,6 +10,10 @@ import Client from '../Client'; import BaseDevice from '../devices/BaseDevice'; +/* A Client uniuely identifies an app running on some device. + + Always use this utility to construct and parse clientId strings. + */ export type ClientIdConstituents = { app: string; os: string; @@ -17,6 +21,25 @@ export type ClientIdConstituents = { device_id: string; }; +/* A plugin key is a string uniquely identifying an instance of a plugin. + This can be a device plugin for a particular device, or a client plugin for a particular client (app). + In the device plugin case, the "client" is the device it's connected to. + In the client plugin case (normal plugins), the "client" is the app it's connected to. + + Always use this utility to construct and parse pluginKey strings. + */ +type PluginKeyConstituents = + | { + type: 'device'; + pluginName: string; + client: string; + } + | ({ + type: 'client'; + pluginName: string; + client: string; + } & ClientIdConstituents); + export function currentActiveApps( clients: Array, selectedDevice: null | BaseDevice, @@ -64,3 +87,31 @@ export function deconstructClientId(clientId: string): ClientIdConstituents { device_id, }; } + +export function deconstructPluginKey(pluginKey: string): PluginKeyConstituents { + const parts = pluginKey.split('#'); + if (parts.length === 2) { + // Device plugin + return { + type: 'device', + client: parts[0], + pluginName: parts[1], + }; + } else { + // Client plugin + const lastHashIndex = pluginKey.lastIndexOf('#'); + const clientId = pluginKey.slice(0, lastHashIndex); + const pluginName = pluginKey.slice(lastHashIndex + 1); + if (!pluginName) { + console.error( + `Attempted to deconstruct invalid pluginKey: "${pluginKey}"`, + ); + } + return { + type: 'client', + ...deconstructClientId(clientId), + client: clientId, + pluginName: pluginName, + }; + } +} diff --git a/src/utils/exportData.tsx b/src/utils/exportData.tsx index 9de51b517..2241cd56e 100644 --- a/src/utils/exportData.tsx +++ b/src/utils/exportData.tsx @@ -41,7 +41,7 @@ import { SupportFormRequestDetailsState, } from '../reducers/supportForm'; import {setSelectPluginsToExportActiveSheet} from '../reducers/application'; -import {deconstructClientId} from '../utils/clientUtils'; +import {deconstructClientId, deconstructPluginKey} from '../utils/clientUtils'; export const IMPORT_FLIPPER_TRACE_EVENT = 'import-flipper-trace'; export const EXPORT_FLIPPER_TRACE_EVENT = 'export-flipper-trace'; @@ -138,8 +138,9 @@ export function processPluginStates( statusUpdate && statusUpdate('Filtering the plugin states for the filtered Clients...'); for (const key in allPluginStates) { - const keyArray = key.split('#'); - const pluginName = keyArray.pop(); + const plugin = deconstructPluginKey(key); + + const pluginName = plugin.pluginName; if ( pluginName && selectedPlugins.length > 0 && @@ -147,17 +148,21 @@ export function processPluginStates( ) { continue; } - const filteredClients = clients.filter(client => { - // Remove the last entry related to plugin - return client.id.includes(keyArray.join('#')); - }); - if ( - filteredClients.length > 0 || - (pluginName && devicePlugins.has(pluginName) && serial === keyArray[0]) - ) { - // There need not be any client for device Plugins - pluginStates = {...pluginStates, [key]: allPluginStates[key]}; + if (plugin.type === 'client') { + if (!clients.some(c => c.id.includes(plugin.client))) { + continue; + } } + if (plugin.type === 'device') { + if ( + !pluginName || + !devicePlugins.has(pluginName) || + serial !== plugin.client + ) { + continue; + } + } + pluginStates = {...pluginStates, [key]: allPluginStates[key]}; } return pluginStates; } @@ -202,8 +207,7 @@ const serializePluginStates = async ( }); const pluginExportState: PluginStatesExportState = {}; for (const key in pluginStates) { - const keyArray = key.split('#'); - const pluginName = keyArray.pop(); + const pluginName = deconstructPluginKey(key).pluginName; statusUpdate && statusUpdate(`Serialising ${pluginName}...`); const serializationMarker = `${EXPORT_FLIPPER_TRACE_EVENT}:serialization-per-plugin`; performance.mark(serializationMarker); @@ -237,8 +241,7 @@ const deserializePluginStates = ( }); const pluginsState: PluginStatesState = {}; for (const key in pluginStatesExportState) { - const keyArray = key.split('#'); - const pluginName = keyArray.pop(); + const pluginName = deconstructPluginKey(key).pluginName; if (!pluginName || !pluginsMap.get(pluginName)) { continue; } @@ -655,12 +658,10 @@ export function importDataToStore(source: string, data: string, store: Store) { clients.forEach((client: {id: string; query: ClientQuery}) => { const clientPlugins: Array = keys .filter(key => { - const arr = key.split('#'); - arr.pop(); - const clientPlugin = arr.join('#'); - return client.id === clientPlugin; + const plugin = deconstructPluginKey(key); + return plugin.type === 'client' && client.id === plugin.client; }) - .map(client => client.split('#').pop() || ''); + .map(pluginKey => deconstructPluginKey(pluginKey).pluginName); store.dispatch({ type: 'NEW_CLIENT', payload: new Client( diff --git a/src/utils/exportMetrics.tsx b/src/utils/exportMetrics.tsx index 57c20adbc..43c30f83a 100644 --- a/src/utils/exportMetrics.tsx +++ b/src/utils/exportMetrics.tsx @@ -14,6 +14,7 @@ import {Store} from '../reducers'; import fs from 'fs'; import {ExportType, fetchMetadata, pluginsClassMap} from './exportData'; import {deserializeObject} from './serialization'; +import {deconstructPluginKey} from './clientUtils'; export type MetricType = {[metricName: string]: number}; type MetricPluginType = {[pluginID: string]: MetricType}; @@ -27,30 +28,31 @@ async function exportMetrics( const metrics: ExportMetricType = {}; for (const key in pluginStates) { const pluginStateData = pluginStates[key]; - const arr = key.split('#'); - const pluginName = arr.pop(); + + const plugin = deconstructPluginKey(key); + const pluginName = plugin.pluginName; if ( pluginName === undefined || (selectedPlugins.length > 0 && !selectedPlugins.includes(pluginName)) ) { continue; } - const clientID = arr.join('#'); - const plugin = pluginsMap.get(pluginName); + const client = plugin.client; + const pluginClass = pluginsMap.get(pluginName); const metricsReducer: | ((persistedState: U) => Promise) | null - | undefined = plugin && plugin.metricsReducer; + | undefined = pluginClass && pluginClass.metricsReducer; if (pluginsMap.has(pluginName) && metricsReducer) { const metricsObject = await metricsReducer(pluginStateData); const pluginObject: MetricPluginType = {}; pluginObject[pluginName] = metricsObject; - if (!metrics[clientID]) { - metrics[clientID] = pluginObject; + if (!metrics[client]) { + metrics[client] = pluginObject; continue; } - const mergedMetrics = {...metrics[clientID], ...pluginObject}; - metrics[clientID] = mergedMetrics; + const mergedMetrics = {...metrics[client], ...pluginObject}; + metrics[client] = mergedMetrics; } } return Promise.resolve(await serialize(metrics)); diff --git a/src/utils/pluginUtils.tsx b/src/utils/pluginUtils.tsx index cf43715c5..21d405ac0 100644 --- a/src/utils/pluginUtils.tsx +++ b/src/utils/pluginUtils.tsx @@ -13,6 +13,7 @@ import {State as PluginStatesState} from '../reducers/pluginStates'; import {pluginsClassMap} from './exportData'; import {State as PluginsState} from '../reducers/plugins'; import {PluginDefinition} from '../dispatcher/plugins'; +import {deconstructPluginKey} from './clientUtils'; export function getPluginKey( selectedApp: string | null, @@ -53,13 +54,15 @@ export function getActivePersistentPlugins( string, typeof FlipperDevicePlugin | typeof FlipperPlugin > = pluginsClassMap(plugins); - return getPersistentPlugins(plugins).filter(plugin => { - const pluginClass = pluginsMap.get(plugin); - const keys = Object.keys(pluginsState).map(key => key.split('#').pop()); + return getPersistentPlugins(plugins).filter(pluginName => { + const pluginClass = pluginsMap.get(pluginName); + const pluginNames = Object.keys(pluginsState).map( + pluginKey => deconstructPluginKey(pluginKey).pluginName, + ); return ( (pluginClass && pluginClass.exportPersistedState != undefined) || - plugin == 'DeviceLogs' || - keys.includes(plugin) + pluginName == 'DeviceLogs' || + pluginNames.includes(pluginName) ); }); }