Make sure Sandy Devices Plugins are loaded in Flipper devices

Summary:
This diff makes sure that devices will actually instantiate applicable sandy device plugins. Similar to how client plugins are owned by Client, device plugins are directly owned by BaseDevice, which significantly simplifies life cycle management and doesn't dispatch updates to all Redux connect components whenever something irrelevant changes.

Also made sure `device.teardown()` is called. That API already existed, but wasn't used or implemented before.

Updated Flipper test utils to support testing device plugins as well (both Sandy and classic ones)

Reviewed By: passy, nikoant

Differential Revision: D22693929

fbshipit-source-id: 73b2b8666ef7a0e748ea89360db84734d37eb5be

desktop/app/src/Client.tsx

@@ -68,12 +68,16 @@ const handleError = (store: Store, device: BaseDevice, error: ErrorType) => {
   if (isProduction()) {
     return;
   }
-  const crashReporterPlugin = store
+  const crashReporterPlugin: typeof FlipperDevicePlugin = store
     .getState()
-    .plugins.devicePlugins.get('CrashReporter');
+    .plugins.devicePlugins.get('CrashReporter') as any;
   if (!crashReporterPlugin) {
     return;
   }
+  if (!crashReporterPlugin.persistedStateReducer) {
+    console.error('CrashReporterPlugin persistedStateReducer broken'); // Make sure we update this code if we ever convert it to Sandy
+    return;
+  }
 
   const pluginKey = getPluginKey(null, device, 'CrashReporter');
 

desktop/app/src/devices/BaseDevice.tsx

@@ -8,9 +8,14 @@
  */
 
 import stream from 'stream';
-import {FlipperDevicePlugin, DeviceLogListener} from 'flipper';
+import {DeviceLogListener} from 'flipper';
 import {sortPluginsByName} from '../utils/pluginUtils';
-import {DeviceLogEntry} from 'flipper-plugin';
+import {
+  DeviceLogEntry,
+  SandyDevicePluginInstance,
+  SandyPluginDefinition,
+} from 'flipper-plugin';
+import {DevicePluginMap} from '../plugin';
 
 export type DeviceShell = {
   stdout: stream.Readable;
@@ -64,7 +69,9 @@ export default class BaseDevice {
   source = '';
 
   // sorted list of supported device plugins
-  devicePlugins!: string[];
+  devicePlugins: string[] = [];
+
+  sandyPluginStates = new Map<string, SandyDevicePluginInstance>();
 
   supportsOS(os: OS) {
     return os.toLowerCase() === this.os.toLowerCase();
@@ -84,7 +91,11 @@ export default class BaseDevice {
     };
   }
 
-  teardown() {}
+  teardown() {
+    for (const instance of this.sandyPluginStates.values()) {
+      instance.destroy();
+    }
+  }
 
   supportedColumns(): Array<string> {
     return ['date', 'pid', 'tid', 'tag', 'message', 'type', 'time'];
@@ -158,10 +169,26 @@ export default class BaseDevice {
     return null;
   }
 
-  loadDevicePlugins(devicePlugins?: Map<string, typeof FlipperDevicePlugin>) {
+  loadDevicePlugins(devicePlugins?: DevicePluginMap) {
-    this.devicePlugins = Array.from(devicePlugins ? devicePlugins.values() : [])
+    if (!devicePlugins) {
-      .filter((plugin) => plugin.supportsDevice(this))
+      return;
-      .sort(sortPluginsByName)
+    }
-      .map((plugin) => plugin.id);
+    const plugins = Array.from(devicePlugins.values());
+    plugins.sort(sortPluginsByName);
+    for (const plugin of plugins) {
+      if (plugin instanceof SandyPluginDefinition) {
+        if (plugin.asDevicePluginModule().supportsDevice(this as any)) {
+          this.devicePlugins.push(plugin.id);
+          this.sandyPluginStates.set(
+            plugin.id,
+            new SandyDevicePluginInstance(this, plugin),
+          ); // TODO: pass initial state if applicable
+        }
+      } else {
+        if (plugin.supportsDevice(this)) {
+          this.devicePlugins.push(plugin.id);
+        }
+      }
+    }
   }
 }

desktop/app/src/plugin.tsx

@@ -23,12 +23,14 @@ import {DEFAULT_MAX_QUEUE_SIZE} from './reducers/pluginMessageQueue';
 import {PluginDetails} from 'flipper-plugin-lib';
 import {Settings} from './reducers/settings';
 import {SandyPluginDefinition} from 'flipper-plugin';
+
 type Parameters = {[key: string]: any};
 
 export type PluginDefinition = ClientPluginDefinition | DevicePluginDefinition;
 
-// TODO: T68738317 add SandyPluginDefinition
+export type DevicePluginDefinition =
-export type DevicePluginDefinition = typeof FlipperDevicePlugin;
+  | typeof FlipperDevicePlugin
+  | SandyPluginDefinition;
 
 export type ClientPluginDefinition =
   | typeof FlipperPlugin
@@ -244,7 +246,7 @@ export class FlipperDevicePlugin<
     this.teardown();
   }
 
-  static supportsDevice(_device: BaseDevice) {
+  static supportsDevice(_device: BaseDevice): boolean {
     throw new Error(
       'supportsDevice is unimplemented in FlipperDevicePlugin class',
     );

desktop/app/src/reducers/__tests__/sandydeviceplugins.node.tsx

@@ -0,0 +1,107 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @format
+ */
+
+import {createMockFlipperWithPlugin} from '../../test-utils/createMockFlipperWithPlugin';
+import {Store} from '../../';
+import {selectPlugin} from '../../reducers/connections';
+import {
+  SandyPluginDefinition,
+  SandyDevicePluginInstance,
+  DevicePluginClient,
+  TestUtils,
+} from 'flipper-plugin';
+
+interface PersistedState {
+  count: 1;
+}
+
+const pluginDetails = TestUtils.createMockPluginDetails();
+
+let initialized = false;
+
+beforeEach(() => {
+  initialized = false;
+});
+
+function devicePlugin(client: DevicePluginClient) {
+  const activateStub = jest.fn();
+  const deactivateStub = jest.fn();
+  const destroyStub = jest.fn();
+
+  client.onActivate(activateStub);
+  client.onDeactivate(deactivateStub);
+  client.onDestroy(destroyStub);
+
+  initialized = true;
+
+  return {
+    activateStub: activateStub,
+    deactivateStub: deactivateStub,
+    destroyStub,
+  };
+}
+const TestPlugin = new SandyPluginDefinition(pluginDetails, {
+  supportsDevice: jest.fn().mockImplementation(() => true),
+  devicePlugin: jest
+    .fn()
+    .mockImplementation(devicePlugin) as typeof devicePlugin,
+  Component: jest.fn().mockImplementation(() => null),
+});
+
+type PluginApi = ReturnType<typeof devicePlugin>;
+
+function selectTestPlugin(store: Store) {
+  store.dispatch(
+    selectPlugin({
+      selectedPlugin: TestPlugin.id,
+      selectedApp: null,
+      deepLinkPayload: null,
+      selectedDevice: store.getState().connections.selectedDevice!,
+    }),
+  );
+}
+
+test('it should initialize device sandy plugins', async () => {
+  const {device, store} = await createMockFlipperWithPlugin(TestPlugin);
+
+  // already started, so initialized immediately
+  expect(initialized).toBe(true);
+  expect(device.sandyPluginStates.get(TestPlugin.id)).toBeInstanceOf(
+    SandyDevicePluginInstance,
+  );
+  expect(TestPlugin.asDevicePluginModule().supportsDevice).toBeCalledTimes(1);
+  const instanceApi: PluginApi = device.sandyPluginStates.get(TestPlugin.id)!
+    .instanceApi;
+
+  expect(instanceApi.activateStub).toBeCalledTimes(0);
+  selectTestPlugin(store);
+
+  // without rendering, non-bg plugins won't connect automatically,
+  // so this isn't the best test, but PluginContainer tests do test that part of the lifecycle
+  device.sandyPluginStates.get(TestPlugin.id)!.activate();
+  expect(instanceApi.activateStub).toBeCalledTimes(1);
+  device.sandyPluginStates.get(TestPlugin.id)!.deactivate();
+  expect(instanceApi.deactivateStub).toBeCalledTimes(1);
+  expect(instanceApi.destroyStub).toBeCalledTimes(0);
+});
+
+test('it should cleanup if device is removed', async () => {
+  const {device, store} = await createMockFlipperWithPlugin(TestPlugin);
+  const pluginInstance = device.sandyPluginStates.get(TestPlugin.id)!;
+  expect(pluginInstance.instanceApi.destroyStub).toHaveBeenCalledTimes(0);
+
+  // close device
+  store.dispatch({
+    type: 'UNREGISTER_DEVICES',
+    payload: new Set([device.serial]),
+  });
+  expect(
+    (pluginInstance.instanceApi as PluginApi).destroyStub,
+  ).toHaveBeenCalledTimes(1);
+});

desktop/app/src/reducers/__tests__/sandyplugins.node.tsx

@@ -233,5 +233,3 @@ test('it can send messages from sandy clients', async () => {
     }
   `);
 });
-
-// TODO: T68683449 state is persisted if a plugin connects and reconnects

desktop/app/src/reducers/connections.tsx

@@ -232,9 +232,14 @@ export default (state: State = INITAL_STATE, action: Actions): State => {
 
       return updateSelection(
         produce(state, (draft) => {
-          draft.devices = draft.devices.filter(
+          draft.devices = draft.devices.filter((device) => {
-            (device) => !deviceSerials.has(device.serial),
+            if (!deviceSerials.has(device.serial)) {
-          );
+              return true;
+            } else {
+              device.teardown();
+              return false;
+            }
+          });
         }),
       );
     }

desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx

@@ -34,7 +34,7 @@ import {Logger} from '../fb-interfaces/Logger';
 import {PluginDefinition} from '../plugin';
 import {registerPlugins} from '../reducers/plugins';
 import PluginContainer from '../PluginContainer';
-import {getPluginKey} from '../utils/pluginUtils';
+import {getPluginKey, isDevicePluginDefinition} from '../utils/pluginUtils';
 import {getInstance} from '../fb-stubs/Logger';
 
 type MockFlipperResult = {
@@ -75,6 +75,7 @@ export async function createMockFlipperWithPlugin(
       type: 'REGISTER_DEVICE',
       payload: device,
     });
+    device.loadDevicePlugins(store.getState().plugins.devicePlugins);
     return device;
   }
 
@@ -102,7 +103,7 @@ export async function createMockFlipperWithPlugin(
       null, // create a stub connection to avoid this plugin to be archived?
       logger,
       store,
-      [pluginClazz.id],
+      isDevicePluginDefinition(pluginClazz) ? [] : [pluginClazz.id],
       device,
     );
 
@@ -125,10 +126,7 @@ export async function createMockFlipperWithPlugin(
         case 'getPlugins':
           // assuming this plugin supports all plugins for now
           return {
-            plugins: [
+            plugins: [...store.getState().plugins.clientPlugins.keys()],
-              ...store.getState().plugins.clientPlugins.keys(),
-              ...store.getState().plugins.devicePlugins.keys(),
-            ],
           };
         case 'getBackgroundPlugins':
           return {plugins: []};
@@ -142,6 +140,7 @@ export async function createMockFlipperWithPlugin(
 
     // enable the plugin
     if (
+      !isDevicePluginDefinition(pluginClazz) &&
       !store
         .getState()
         .connections.userStarredPlugins[client.query.app]?.includes(

desktop/app/src/utils/exportData.tsx

@@ -19,7 +19,6 @@ import {PluginNotification} from '../reducers/notifications';
 import Client, {ClientExport, ClientQuery} from '../Client';
 import {pluginKey} from '../reducers/pluginStates';
 import {
-  FlipperDevicePlugin,
   callClient,
   supportsMethod,
   PluginDefinition,
@@ -82,7 +81,7 @@ type ProcessPluginStatesOptions = {
   clients: Array<ClientExport>;
   serial: string;
   allPluginStates: PluginStatesState;
-  devicePlugins: Map<string, typeof FlipperDevicePlugin>;
+  devicePlugins: DevicePluginMap;
   selectedPlugins: Array<string>;
   statusUpdate?: (msg: string) => void;
 };
@@ -91,7 +90,7 @@ type ProcessNotificationStatesOptions = {
   clients: Array<ClientExport>;
   serial: string;
   allActiveNotifications: Array<PluginNotification>;
-  devicePlugins: Map<string, typeof FlipperDevicePlugin>;
+  devicePlugins: DevicePluginMap;
   statusUpdate?: (msg: string) => void;
 };
 

desktop/app/src/utils/pluginUtils.tsx

@@ -18,6 +18,7 @@ import {State as PluginStatesState} from '../reducers/pluginStates';
 import {State as PluginsState} from '../reducers/plugins';
 import {State as PluginMessageQueueState} from '../reducers/pluginMessageQueue';
 import {deconstructPluginKey, deconstructClientId} from './clientUtils';
+import {SandyPluginDefinition} from 'flipper-plugin';
 
 type Client = import('../Client').default;
 
@@ -231,7 +232,8 @@ export function sortPluginsByName(
 export function isDevicePluginDefinition(
   definition: PluginDefinition,
 ): definition is DevicePluginDefinition {
-  // TODO: support Sandy device plugins T68738317
+  return (
-  // @ts-ignore
+    (definition as any).prototype instanceof FlipperDevicePlugin ||
-  return definition.prototype instanceof FlipperDevicePlugin;
+    (definition instanceof SandyPluginDefinition && definition.isDevicePlugin)
+  );
 }

desktop/flipper-plugin/src/index.ts

@@ -16,6 +16,7 @@ export {
   DeviceLogListener,
   DevicePluginClient,
   LogLevel,
+  SandyDevicePluginInstance,
 } from './plugin/DevicePlugin';
 export {SandyPluginDefinition} from './plugin/SandyPluginDefinition';
 export {SandyPluginRenderer} from './plugin/PluginRenderer';

desktop/plugins/crash_reporter/index.tsx

@@ -19,7 +19,6 @@ import {
   ContextMenu,
   clipboard,
   Button,
-  FlipperPlugin,
   getPluginKey,
   getPersistedState,
   BaseDevice,
@@ -239,10 +238,14 @@ export function parseCrashLogAndUpdateState(
     | typeof FlipperBasePlugin
     | undefined = store
     .getState()
-    .plugins.devicePlugins.get(CrashReporterPlugin.id);
+    .plugins.devicePlugins.get(CrashReporterPlugin.id) as any;
   if (!persistingPlugin) {
     return;
   }
+  if (!persistingPlugin.persistedStateReducer) {
+    console.error('CrashReporterPlugin is incompatible');
+    return;
+  }
   const pluginStates = store.getState().pluginStates;
   const persistedState = getPersistedState(
     pluginKey,