From a60865f0be5e5f67d4180fc067db3f6941c698ec Mon Sep 17 00:00:00 2001
From: Andrey Goncharov <aigoncharov@fb.com>
Date: Mon, 28 Feb 2022 03:50:34 -0800
Subject: [PATCH] Intergrate ServerAddOn with PluginManager

Reviewed By: lblasa

Differential Revision: D34042338

fbshipit-source-id: 2ae3992738e647075f19c4dba0ebfb01e6c12b8f
---
 .../src/FlipperServerImpl.tsx                 |  2 +-
 .../src/plugins/PluginManager.tsx             | 28 ++++++++++++++++++-
 .../src/plugins/ServerAddOn.tsx               | 15 ++++++++--
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/desktop/flipper-server-core/src/FlipperServerImpl.tsx b/desktop/flipper-server-core/src/FlipperServerImpl.tsx
index 37d800d68..af20e9890 100644
--- a/desktop/flipper-server-core/src/FlipperServerImpl.tsx
+++ b/desktop/flipper-server-core/src/FlipperServerImpl.tsx
@@ -421,7 +421,7 @@ export class FlipperServerImpl implements FlipperServer {
     'plugin-source': (path) => this.pluginManager.loadSource(path),
     'plugins-server-add-on-start': (pluginName) =>
       this.pluginManager.startServerAddOn(pluginName),
-    'plugins-server-add-on-stop': (pluginName) =>
+    'plugins-server-add-on-stop': async (pluginName) =>
       this.pluginManager.stopServerAddOn(pluginName),
     'doctor-get-healthchecks': getHealthChecks,
     'doctor-run-healthcheck': runHealthcheck,
diff --git a/desktop/flipper-server-core/src/plugins/PluginManager.tsx b/desktop/flipper-server-core/src/plugins/PluginManager.tsx
index 3156da938..a92d0edc4 100644
--- a/desktop/flipper-server-core/src/plugins/PluginManager.tsx
+++ b/desktop/flipper-server-core/src/plugins/PluginManager.tsx
@@ -30,6 +30,7 @@ import {
   getInstalledPlugin,
   installPluginFromNpm,
 } from 'flipper-plugin-lib';
+import {ServerAddOn} from './ServerAddOn';
 
 const maxInstalledPluginVersionsToKeep = 2;
 
@@ -43,6 +44,8 @@ const getTempDirName = promisify(tmp.dir) as (
 ) => Promise<string>;
 
 export class PluginManager {
+  private readonly serverAddOns = new Map<string, ServerAddOn>();
+
   async start() {
     // This needn't happen immediately and is (light) I/O work.
     setTimeout(() => {
@@ -154,9 +157,32 @@ export class PluginManager {
 
   async startServerAddOn(pluginName: string) {
     console.debug('PluginManager.startServerAddOn', pluginName);
+    // TODO: Get owner from websocket connection ID
+    const fakeOwner = 'test';
+    const existingServerAddOn = this.serverAddOns.get(pluginName);
+    if (existingServerAddOn) {
+      console.debug(
+        'PluginManager.startServerAddOn -> already started, adding an owner',
+        pluginName,
+        fakeOwner,
+      );
+      existingServerAddOn.addOwner(fakeOwner);
+      return;
+    }
+
+    const newServerAddOn = await ServerAddOn.start(pluginName, fakeOwner);
+    this.serverAddOns.set(pluginName, newServerAddOn);
   }
 
-  async stopServerAddOn(pluginName: string) {
+  stopServerAddOn(pluginName: string) {
     console.debug('PluginManager.stopServerAddOn', pluginName);
+    const serverAddOn = this.serverAddOns.get(pluginName);
+    if (!serverAddOn) {
+      console.debug('PluginManager.stopServerAddOn -> not started', pluginName);
+      return;
+    }
+    // TODO: Get owner from websocket connection ID
+    const fakeOwner = 'test';
+    serverAddOn.removeOwner(fakeOwner);
   }
 }
diff --git a/desktop/flipper-server-core/src/plugins/ServerAddOn.tsx b/desktop/flipper-server-core/src/plugins/ServerAddOn.tsx
index 7fcfe6643..1ea8cdd93 100644
--- a/desktop/flipper-server-core/src/plugins/ServerAddOn.tsx
+++ b/desktop/flipper-server-core/src/plugins/ServerAddOn.tsx
@@ -15,6 +15,13 @@ interface ServerAddOnModule {
   serverAddOn?: () => Promise<ServerAddOnCleanup>;
 }
 
+// TODO: Metro does not like dynamic requires. Figure out how to properly configure metro to handle them.
+// https://github.com/webpack/webpack/issues/4175#issuecomment-323023911
+function requireDynamically(path: string) {
+  // eslint-disable-next-line no-eval
+  return eval(`require('${path}');`); // Ensure Metro does not analyze the require statement
+}
+
 // TODO: Fix potential race conditions when starting/stopping concurrently
 export class ServerAddOn {
   private owners: Set<string>;
@@ -30,7 +37,7 @@ export class ServerAddOn {
   static async start(path: string, initialOwner: string): Promise<ServerAddOn> {
     console.info('ServerAddOn.start', path);
 
-    const {serverAddOn} = require(path) as ServerAddOnModule;
+    const {serverAddOn} = requireDynamically(path) as ServerAddOnModule;
     assertNotNull(serverAddOn);
     assert(
       typeof serverAddOn === 'function',
@@ -66,6 +73,10 @@ export class ServerAddOn {
 
   private async stop() {
     console.info('ServerAddOn.stop', this.path);
-    await this.cleanup();
+    try {
+      await this.cleanup();
+    } catch (e) {
+      console.error('ServerAddOn.stop -> failed to clean up', this.path);
+    }
   }
 }