Do not use custom serializer for all the plugins

Summary: This diff solves the problem where the export for the graphql plugin was super super super sloooooowwww...... The reason being that the graphql plugin had chunky graphql responses which were json blob which was being serialized by our custom serializer. Instead of serializing those with custom serializer we can directly serialize them as they won't have any map's, sets, classes etc. This diff adds the two static functions on the plugin which will provide the serialized and deserialized object for the persistedstate. As the plugin knows the structure of its state it can optimize the serialization and deserialization of its data. This change solves the slow export issue and makes it blazing fast..... 🏎 Bug: {F206550514} Reviewed By: danielbuechele Differential Revision: D17166054 fbshipit-source-id: 058b903c03c12c9194702162c46763ef5b5e7283
2019-09-09 06:11:54 -07:00
parent 9ebf5346df
commit 566f2bf96e
8 changed files with 204 additions and 59 deletions
--- a/flow-typed/flipper.js
+++ b/flow-typed/flipper.js
@@ -94,6 +94,14 @@ declare module 'flipper' {
      persistedState: ?PersistedState,
      store: ?MiddlewareAPI,
    ) => Promise<?PersistedState>;
+    static serializePersistedState: (
+      persistedState: PersistedState,
+      statusUpdate?: (msg: string) => void,
+      idler?: Idler,
+    ) => Promise<string>;
+    static deserializePersistedState: (
+      serializedString: string,
+    ) => PersistedState;
    static getActiveNotifications: ?(
      persistedState: PersistedState,
    ) => Array<Notification>;
--- a/headless-tests/tests/headlessIntegrationTests.js
+++ b/headless-tests/tests/headlessIntegrationTests.js
@@ -87,7 +87,7 @@ const runHeadless = memoize(
  },
 );

-function getPluginState(app: string, plugin: string): Promise<Object> {
+function getPluginState(app: string, plugin: string): Promise<string> {
  return runHeadless(basicArgs).then(result => {
    const pluginStates = result.output.store.pluginStates;
    for (const pluginId of Object.keys(pluginStates)) {
@@ -225,7 +225,8 @@ test('test layout snapshot stripping', () => {
 });

 test('Sample app layout hierarchy matches snapshot', () => {
-  return getPluginState('Flipper', 'Inspector').then(state => {
+  return getPluginState('Flipper', 'Inspector').then(result => {
+    const state = JSON.parse(result);
    expect(state.rootAXElement).toBe('com.facebook.flipper.sample');
    expect(state.rootElement).toBe('com.facebook.flipper.sample');
    const canonicalizedElements = Object.values(state.elements)
--- a/src/devices/BaseDevice.tsx
+++ b/src/devices/BaseDevice.tsx
@@ -41,7 +41,7 @@ export type DeviceType =
  | 'archivedPhysical';

 export type DeviceExport = {
-  os: string;
+  os: OS;
  title: string;
  deviceType: DeviceType;
  serial: string;
--- a/src/dispatcher/application.tsx
+++ b/src/dispatcher/application.tsx
@@ -51,14 +51,14 @@ export default (store: Store, logger: Logger) => {
    });
  });

-  ipcRenderer.on('flipper-protocol-handler', (event, url) => {
-    if (url.startsWith('flipper://import')) {
-      const {search} = new URL(url);
-      const download = qs.parse(search) ? qs.parse(search) : undefined;
+  ipcRenderer.on('flipper-protocol-handler', (event, query: string) => {
+    if (query.startsWith('flipper://import')) {
+      const {search} = new URL(query);
+      const {url} = qs.parse(search);
      store.dispatch(toggleAction('downloadingImportData', true));
      return (
-        download &&
-        fetch(String(download))
+        typeof url === 'string' &&
+        fetch(url)
          .then(res => res.text())
          .then(data => importDataToStore(data, store))
          .then(() => {
@@ -70,7 +70,7 @@ export default (store: Store, logger: Logger) => {
          })
      );
    }
-    const match = uriComponents(url);
+    const match = uriComponents(query);
    if (match.length > 1) {
      // flipper://<client>/<pluginId>/<payload>
      return store.dispatch(
--- a/src/index.tsx
+++ b/src/index.tsx
@@ -26,6 +26,7 @@ export {connect} from 'react-redux';
 export {selectPlugin} from './reducers/connections';
 export {writeBufferToFile, bufferToBlob} from './utils/screenshot';
 export {getPluginKey, getPersistedState} from './utils/pluginUtils';
+export {Idler} from './utils/Idler';
 export {Store, MiddlewareAPI} from './reducers/index';
 export {default as BaseDevice} from './devices/BaseDevice';
 export {
--- a/src/plugin.tsx
+++ b/src/plugin.tsx
@@ -13,7 +13,8 @@ import {Store, MiddlewareAPI} from './reducers/index';
 import {MetricType} from './utils/exportMetrics';
 import {ReactNode, Component} from 'react';
 import BaseDevice from './devices/BaseDevice';
-
+import {serialize, deserialize} from './utils/serialization';
+import {Idler} from './utils/Idler';
 type Parameters = any;

 // This function is intended to be called from outside of the plugin.
@@ -135,6 +136,22 @@ export abstract class FlipperBasePlugin<

  // methods to be overriden by plugins
  init(): void {}
+  static serializePersistedState: (
+    persistedState: StaticPersistedState,
+    statusUpdate?: (msg: string) => void,
+    idler?: Idler,
+  ) => Promise<string> = (
+    persistedState: StaticPersistedState,
+    statusUpdate?: (msg: string) => void,
+    idler?: Idler,
+  ) => {
+    return serialize(persistedState, idler, statusUpdate);
+  };
+  static deserializePersistedState: (
+    serializedString: string,
+  ) => StaticPersistedState = (serializedString: string) => {
+    return deserialize(serializedString);
+  };
  teardown(): void {}
  computeNotifications(
    _props: Props<PersistedState>,
--- a/src/utils/tests/exportData.electron.js
+++ b/src/utils/tests/exportData.electron.js
@@ -156,6 +156,7 @@ test('test processStore function for empty state', () => {
    pluginStates: {},
    clients: [],
    devicePlugins: new Map(),
+    clientPlugins: new Map(),
    salt: 'salt',
    selectedPlugins: [],
  });
@@ -169,6 +170,7 @@ test('test processStore function for an iOS device connected', async () => {
    pluginStates: {},
    clients: [],
    devicePlugins: new Map(),
+    clientPlugins: new Map(),
    salt: 'salt',
    selectedPlugins: [],
  });
@@ -198,18 +200,24 @@ test('test processStore function for an iOS device connected with client plugin
  const json = await processStore({
    activeNotifications: [],
    device,
-    pluginStates: {[clientIdentifier]: {msg: 'Test plugin'}},
+    pluginStates: {
+      [`${clientIdentifier}#TestDevicePlugin`]: {msg: 'Test plugin'},
+    },
    clients: [generateClientFromDevice(device, 'testapp')],
    devicePlugins: new Map(),
+    clientPlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]),
    salt: 'salt',
    selectedPlugins: [],
  });
  expect(json).toBeDefined();
  const {pluginStates} = json.store;
  const expectedPluginState = {
-    [generateClientIdentifierWithSalt(clientIdentifier, 'salt')]: {
+    [`${generateClientIdentifierWithSalt(
+      clientIdentifier,
+      'salt',
+    )}#TestDevicePlugin`]: JSON.stringify({
      msg: 'Test plugin',
-    },
+    }),
  };
  expect(pluginStates).toEqual(expectedPluginState);
 });
@@ -246,10 +254,10 @@ test('test processStore function to have only the client for the selected device
    activeNotifications: [],
    device: selectedDevice,
    pluginStates: {
-      [unselectedDeviceClientIdentifier + '#testapp']: {
+      [unselectedDeviceClientIdentifier + '#TestDevicePlugin']: {
        msg: 'Test plugin unselected device',
      },
-      [selectedDeviceClientIdentifier + '#testapp']: {
+      [selectedDeviceClientIdentifier + '#TestDevicePlugin']: {
        msg: 'Test plugin selected device',
      },
    },
@@ -258,6 +266,7 @@ test('test processStore function to have only the client for the selected device
      generateClientFromDevice(unselectedDevice, 'testapp'),
    ],
    devicePlugins: new Map(),
+    clientPlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]),
    salt: 'salt',
    selectedPlugins: [],
  });
@@ -267,9 +276,9 @@ test('test processStore function to have only the client for the selected device
  const {pluginStates} = json.store;
  const expectedPluginState = {
    [generateClientIdentifierWithSalt(selectedDeviceClientIdentifier, 'salt') +
-    '#testapp']: {
+    '#TestDevicePlugin']: JSON.stringify({
      msg: 'Test plugin selected device',
-    },
+    }),
  };
  expect(clients).toEqual([
    generateClientFromClientWithSalt(selectedDeviceClient, 'salt'),
@@ -302,10 +311,10 @@ test('test processStore function to have multiple clients for the selected devic
    activeNotifications: [],
    device: selectedDevice,
    pluginStates: {
-      [clientIdentifierApp1 + '#testapp1']: {
+      [clientIdentifierApp1 + '#TestDevicePlugin']: {
        msg: 'Test plugin App1',
      },
-      [clientIdentifierApp2 + '#testapp2']: {
+      [clientIdentifierApp2 + '#TestDevicePlugin']: {
        msg: 'Test plugin App2',
      },
    },
@@ -314,6 +323,7 @@ test('test processStore function to have multiple clients for the selected devic
      generateClientFromDevice(selectedDevice, 'testapp2'),
    ],
    devicePlugins: new Map(),
+    clientPlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]),
    salt: 'salt',
    selectedPlugins: [],
  });
@@ -322,13 +332,13 @@ test('test processStore function to have multiple clients for the selected devic
  const {pluginStates} = json.store;
  const expectedPluginState = {
    [generateClientIdentifierWithSalt(clientIdentifierApp1, 'salt') +
-    '#testapp1']: {
+    '#TestDevicePlugin']: JSON.stringify({
      msg: 'Test plugin App1',
-    },
+    }),
    [generateClientIdentifierWithSalt(clientIdentifierApp2, 'salt') +
-    '#testapp2']: {
+    '#TestDevicePlugin']: JSON.stringify({
      msg: 'Test plugin App2',
-    },
+    }),
  };
  expect(clients).toEqual([
    generateClientFromClientWithSalt(client1, 'salt'),
@@ -356,6 +366,7 @@ test('test processStore function for device plugin state and no clients', async
    },
    clients: [],
    devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]),
+    clientPlugins: new Map(),
    salt: 'salt',
    selectedPlugins: [],
  });
@@ -363,7 +374,7 @@ test('test processStore function for device plugin state and no clients', async
  const {pluginStates} = json.store;
  const {clients} = json;
  const expectedPluginState = {
-    'salt-serial#TestDevicePlugin': {msg: 'Test Device plugin'},
+    'salt-serial#TestDevicePlugin': JSON.stringify({msg: 'Test Device plugin'}),
  };
  expect(pluginStates).toEqual(expectedPluginState);
  expect(clients).toEqual([]);
@@ -388,6 +399,7 @@ test('test processStore function for unselected device plugin state and no clien
    },
    clients: [],
    devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]),
+    clientPlugins: new Map(),
    salt: 'salt',
    selectedPlugins: [],
  });
@@ -425,6 +437,7 @@ test('test processStore function for notifications for selected device', async (
    pluginStates: {},
    clients: [client],
    devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]),
+    clientPlugins: new Map(),
    salt: 'salt',
    selectedPlugins: [],
  });
@@ -482,6 +495,7 @@ test('test processStore function for notifications for unselected device', async
    pluginStates: {},
    clients: [client, unselectedclient],
    devicePlugins: new Map(),
+    clientPlugins: new Map(),
    salt: 'salt',
    selectedPlugins: [],
  });
@@ -505,10 +519,10 @@ test('test processStore function for selected plugins', async () => {

  const client = generateClientFromDevice(selectedDevice, 'app');
  const pluginstates = {
-    [generateClientIdentifier(selectedDevice, 'app') + '#plugin1']: {
+    [generateClientIdentifier(selectedDevice, 'app') + '#TestDevicePlugin1']: {
      msg: 'Test plugin1',
    },
-    [generateClientIdentifier(selectedDevice, 'app') + '#plugin2']: {
+    [generateClientIdentifier(selectedDevice, 'app') + '#TestDevicePlugin2']: {
      msg: 'Test plugin2',
    },
  };
@@ -517,9 +531,13 @@ test('test processStore function for selected plugins', async () => {
    device: selectedDevice,
    pluginStates: pluginstates,
    clients: [client],
-    devicePlugins: new Map(),
+    devicePlugins: new Map([
+      ['TestDevicePlugin1', TestDevicePlugin],
+      ['TestDevicePlugin2', TestDevicePlugin],
+    ]),
+    clientPlugins: new Map(),
    salt: 'salt',
-    selectedPlugins: ['plugin2'],
+    selectedPlugins: ['TestDevicePlugin2'],
  });
  expect(json).toBeDefined();
  const {pluginStates} = json.store;
@@ -528,9 +546,9 @@ test('test processStore function for selected plugins', async () => {
    [generateClientIdentifierWithSalt(
      generateClientIdentifier(selectedDevice, 'app'),
      'salt',
-    ) + '#plugin2']: {
+    ) + '#TestDevicePlugin2']: JSON.stringify({
      msg: 'Test plugin2',
-    },
+    }),
  });
  expect(clients).toEqual([generateClientFromClientWithSalt(client, 'salt')]);
  const {activeNotifications} = json.store;
@@ -547,10 +565,10 @@ test('test processStore function for no selected plugins', async () => {
  );
  const client = generateClientFromDevice(selectedDevice, 'app');
  const pluginstates = {
-    [generateClientIdentifier(selectedDevice, 'app') + '#plugin1']: {
+    [generateClientIdentifier(selectedDevice, 'app') + '#TestDevicePlugin1']: {
      msg: 'Test plugin1',
    },
-    [generateClientIdentifier(selectedDevice, 'app') + '#plugin2']: {
+    [generateClientIdentifier(selectedDevice, 'app') + '#TestDevicePlugin2']: {
      msg: 'Test plugin2',
    },
  };
@@ -559,7 +577,11 @@ test('test processStore function for no selected plugins', async () => {
    device: selectedDevice,
    pluginStates: pluginstates,
    clients: [client],
-    devicePlugins: new Map(),
+    devicePlugins: new Map([
+      ['TestDevicePlugin1', TestDevicePlugin],
+      ['TestDevicePlugin2', TestDevicePlugin],
+    ]),
+    clientPlugins: new Map(),
    salt: 'salt',
    selectedPlugins: [],
  });
@@ -571,15 +593,15 @@ test('test processStore function for no selected plugins', async () => {
    [generateClientIdentifierWithSalt(
      generateClientIdentifier(selectedDevice, 'app'),
      'salt',
-    ) + '#plugin2']: {
+    ) + '#TestDevicePlugin2']: JSON.stringify({
      msg: 'Test plugin2',
-    },
+    }),
    [generateClientIdentifierWithSalt(
      generateClientIdentifier(selectedDevice, 'app'),
      'salt',
-    ) + '#plugin1']: {
+    ) + '#TestDevicePlugin1']: JSON.stringify({
      msg: 'Test plugin1',
-    },
+    }),
  });
  expect(clients).toEqual([generateClientFromClientWithSalt(client, 'salt')]);
  const {activeNotifications} = json.store;
--- a/src/utils/exportData.tsx
+++ b/src/utils/exportData.tsx
@@ -12,7 +12,12 @@ import {PluginNotification} from '../reducers/notifications';
 import {ClientExport} from '../Client.js';
 import {State as PluginsState} from '../reducers/plugins';
 import {pluginKey} from '../reducers/pluginStates';
-import {FlipperDevicePlugin, FlipperPlugin, callClient} from '../plugin';
+import {
+  FlipperDevicePlugin,
+  FlipperPlugin,
+  callClient,
+  FlipperBasePlugin,
+} from '../plugin';
 import {default as BaseDevice} from '../devices/BaseDevice';
 import {default as ArchivedDevice} from '../devices/ArchivedDevice';
 import {default as Client} from '../Client';
@@ -28,13 +33,16 @@ import {Idler} from './Idler';
 export const IMPORT_FLIPPER_TRACE_EVENT = 'import-flipper-trace';
 export const EXPORT_FLIPPER_TRACE_EVENT = 'export-flipper-trace';

+export type PluginStatesExportState = {
+  [pluginKey: string]: string;
+};
 export type ExportType = {
  fileVersion: string;
  flipperReleaseRevision: string | null;
  clients: Array<ClientExport>;
  device: DeviceExport | null;
  store: {
-    pluginStates: PluginStatesState;
+    pluginStates: PluginStatesExportState;
    activeNotifications: Array<PluginNotification>;
  };
 };
@@ -56,11 +64,15 @@ type ProcessNotificationStatesOptions = {
  statusUpdate?: (msg: string) => void;
 };

+type SerializePluginStatesOptions = {
+  pluginStates: PluginStatesState;
+};
+
 type AddSaltToDeviceSerialOptions = {
  salt: string;
  device: BaseDevice;
  clients: Array<ClientExport>;
-  pluginStates: PluginStatesState;
+  pluginStates: PluginStatesExportState;
  pluginNotification: Array<PluginNotification>;
  selectedPlugins: Array<string>;
  statusUpdate?: (msg: string) => void;
@@ -107,7 +119,7 @@ export function processPluginStates(
    statusUpdate,
  } = options;

-  let pluginStates = {};
+  let pluginStates: PluginStatesState = {};
  statusUpdate &&
    statusUpdate('Filtering the plugin states for the filtered Clients...');
  for (const key in allPluginStates) {
@@ -155,6 +167,61 @@ export function processNotificationStates(
  return activeNotifications;
 }

+const serializePluginStates = async (
+  pluginStates: PluginStatesState,
+  clientPlugins: Map<string, typeof FlipperPlugin>,
+  devicePlugins: Map<string, typeof FlipperDevicePlugin>,
+  statusUpdate?: (msg: string) => void,
+  idler?: Idler,
+): Promise<PluginStatesExportState> => {
+  const pluginsMap: Map<string, typeof FlipperBasePlugin> = new Map([]);
+  clientPlugins.forEach((val, key) => {
+    pluginsMap.set(key, val);
+  });
+  devicePlugins.forEach((val, key) => {
+    pluginsMap.set(key, val);
+  });
+  const pluginExportState: PluginStatesExportState = {};
+  for (const key in pluginStates) {
+    const keyArray = key.split('#');
+    const pluginName = keyArray.pop();
+    statusUpdate && statusUpdate(`Serialising ${pluginName}...`);
+
+    const pluginClass = pluginsMap.get(pluginName);
+    if (pluginClass) {
+      pluginExportState[key] = await pluginClass.serializePersistedState(
+        pluginStates[key],
+        statusUpdate,
+        idler,
+      );
+    }
+  }
+  return pluginExportState;
+};
+
+const deserializePluginStates = (
+  pluginStatesExportState: PluginStatesExportState,
+  clientPlugins: Map<string, typeof FlipperPlugin>,
+  devicePlugins: Map<string, typeof FlipperDevicePlugin>,
+): PluginStatesState => {
+  const pluginsMap: Map<string, typeof FlipperBasePlugin> = new Map([]);
+  clientPlugins.forEach((val, key) => {
+    pluginsMap.set(key, val);
+  });
+  devicePlugins.forEach((val, key) => {
+    pluginsMap.set(key, val);
+  });
+  const pluginsState: PluginStatesState = {};
+  for (const key in pluginStatesExportState) {
+    const keyArray = key.split('#');
+    const pluginName = keyArray.pop();
+    pluginsState[key] = pluginsMap
+      .get(pluginName)
+      .deserializePersistedState(pluginStatesExportState[key]);
+  }
+  return pluginsState;
+};
+
 const addSaltToDeviceSerial = async (
  options: AddSaltToDeviceSerialOptions,
 ): Promise<ExportType> => {
@@ -235,6 +302,7 @@ type ProcessStoreOptions = {
  pluginStates: PluginStatesState;
  clients: Array<ClientExport>;
  devicePlugins: Map<string, typeof FlipperDevicePlugin>;
+  clientPlugins: Map<string, typeof FlipperPlugin>;
  salt: string;
  selectedPlugins: Array<string>;
  statusUpdate?: (msg: string) => void;
@@ -242,6 +310,7 @@ type ProcessStoreOptions = {

 export const processStore = async (
  options: ProcessStoreOptions,
+  idler?: Idler,
 ): Promise<ExportType | null> => {
  const {
    activeNotifications,
@@ -249,6 +318,7 @@ export const processStore = async (
    pluginStates,
    clients,
    devicePlugins,
+    clientPlugins,
    salt,
    selectedPlugins,
    statusUpdate,
@@ -272,16 +342,25 @@ export const processStore = async (
      devicePlugins,
      statusUpdate,
    });
+
+    const exportPluginState = await serializePluginStates(
+      processedPluginStates,
+      clientPlugins,
+      devicePlugins,
+      statusUpdate,
+      idler,
+    );
    // Adding salt to the device id, so that the device_id in the device list is unique.
    const exportFlipperData = await addSaltToDeviceSerial({
      salt,
      device,
      clients: processedClients,
-      pluginStates: processedPluginStates,
+      pluginStates: exportPluginState,
      pluginNotification: processedActiveNotifications,
      statusUpdate,
      selectedPlugins,
    });
+
    return exportFlipperData;
  }
  return null;
@@ -340,6 +419,7 @@ export async function fetchMetadata(
 export async function getStoreExport(
  store: MiddlewareAPI,
  statusUpdate?: (msg: string) => void,
+  idler?: Idler,
 ): Promise<{exportData: ExportType | null; errorArray: Array<Error>}> {
  const state = store.getState();
  const {clients} = state.connections;
@@ -373,17 +453,21 @@ export async function getStoreExport(
  const newPluginState = metadata.pluginStates;

  const {activeNotifications} = store.getState().notifications;
-  const {devicePlugins} = store.getState().plugins;
-  const exportData = await processStore({
+  const {devicePlugins, clientPlugins} = store.getState().plugins;
+  const exportData = await processStore(
+    {
      activeNotifications,
      device: selectedDevice,
      pluginStates: newPluginState,
      clients: clients.map(client => client.toJSON()),
      devicePlugins,
+      clientPlugins,
      salt: uuid.v4(),
      selectedPlugins: store.getState().plugins.selectedPlugins,
      statusUpdate,
-  });
+    },
+    idler,
+  );
  return {exportData, errorArray};
 }

@@ -399,6 +483,7 @@ export function exportStore(
      const {exportData, errorArray} = await getStoreExport(
        store,
        statusUpdate,
+        idler,
      );
      if (!exportData) {
        console.error('Make sure a device is connected');
@@ -443,8 +528,11 @@ export const exportStoreToFile = (

 export function importDataToStore(data: string, store: Store) {
  getLogger().track('usage', IMPORT_FLIPPER_TRACE_EVENT);
-  const json = deserialize(data);
+  const json: ExportType = deserialize(data);
  const {device, clients} = json;
+  if (device == null) {
+    return;
+  }
  const {serial, deviceType, title, os, logs} = device;
  const archivedDevice = new ArchivedDevice(
    serial,
@@ -474,25 +562,33 @@ export function importDataToStore(data: string, store: Store) {
  });

  const {pluginStates} = json.store;
-  const keys = Object.keys(pluginStates);
+  const processedPluginStates: PluginStatesState = deserializePluginStates(
+    pluginStates,
+    store.getState().plugins.clientPlugins,
+    store.getState().plugins.devicePlugins,
+  );
+  const keys = Object.keys(processedPluginStates);
  keys.forEach(key => {
    store.dispatch({
      type: 'SET_PLUGIN_STATE',
      payload: {
        pluginKey: key,
-        state: pluginStates[key],
+        state: processedPluginStates[key],
      },
    });
  });
  clients.forEach(client => {
-    const clientPlugins = keys
+    const clientPlugins: Array<string> = keys
      .filter(key => {
        const arr = key.split('#');
        arr.pop();
        const clientPlugin = arr.join('#');
        return client.id === clientPlugin;
      })
-      .map(client => client.split('#').pop());
+      .map(client => {
+        const elem = client.split('#').pop();
+        return elem || '';
+      });
    store.dispatch({
      type: 'NEW_CLIENT',
      payload: new Client(