Addressed some earlier diff comments

Summary: Async processed review feedback from stack D31474919 (cfd44b592a) e.o. (decapitate)

Reviewed By: passy

Differential Revision: D31608309

fbshipit-source-id: 536b3c9350f7acc40530000ecf5e46d5b074d50f

desktop/app/src/dispatcher/flipperServer.tsx

@@ -10,7 +10,7 @@
 import React from 'react';
 import {State, Store} from '../reducers/index';
 import {Logger} from 'flipper-common';
-import {FlipperServerImpl, setFlipperServerConfig} from 'flipper-server-core';
+import {FlipperServerImpl} from 'flipper-server-core';
 import {selectClient} from '../reducers/connections';
 import Client from '../Client';
 import {notification} from 'antd';
@@ -24,7 +24,9 @@ import constants from '../fb-stubs/constants';
 export default async (store: Store, logger: Logger) => {
   const {enableAndroid, androidHome, idbPath, enableIOS, enablePhysicalIOS} =
     store.getState().settingsState;
-  setFlipperServerConfig({
+
+  const server = new FlipperServerImpl(
+    {
       enableAndroid,
       androidHome,
       idbPath,
@@ -33,8 +35,9 @@ export default async (store: Store, logger: Logger) => {
       staticPath: getStaticPath(),
       tmpPath: getAppTempPath(),
       validWebSocketOrigins: constants.VALID_WEB_SOCKET_REQUEST_ORIGIN_PREFIXES,
-  });
+    },
-  const server = new FlipperServerImpl(logger);
+    logger,
+  );
 
   store.dispatch({
     type: 'SET_FLIPPER_SERVER',

desktop/flipper-dump/src/index.tsx

@@ -10,7 +10,7 @@
 import fs from 'fs';
 import os from 'os';
 import yargs from 'yargs';
-import {FlipperServerImpl, setFlipperServerConfig} from 'flipper-server-core';
+import {FlipperServerImpl} from 'flipper-server-core';
 import {
   ClientDescription,
   Logger,
@@ -63,8 +63,11 @@ async function start(deviceTitle: string, appName: string, pluginId: string) {
     console.debug = () => {};
     console.info = console.error;
 
+    // TODO: initialise FB user manager to be able to do certificate exchange
+
+    const server = new FlipperServerImpl(
+      {
         // TODO: make these better overridable
-    setFlipperServerConfig({
         enableAndroid: true,
         androidHome: process.env.ANDROID_HOME || '/opt/android_sdk',
         idbPath: '/usr/local/bin/idb',
@@ -73,10 +76,9 @@ async function start(deviceTitle: string, appName: string, pluginId: string) {
         staticPath: path.resolve(__dirname, '..', '..', 'static'),
         tmpPath: os.tmpdir(),
         validWebSocketOrigins: [],
-    });
+      },
-    // TODO: initialise FB user manager to be able to do certificate exchange
+      logger,
-
+    );
-    const server = new FlipperServerImpl(logger);
 
     logger.info(
       `Waiting for device '${deviceTitle}' client '${appName}' plugin '${pluginId}' ...`,

desktop/flipper-server-core/src/FlipperServerImpl.tsx

@@ -29,7 +29,10 @@ import {ServerDevice} from './devices/ServerDevice';
 import {Base64} from 'js-base64';
 import MetroDevice from './devices/metro/MetroDevice';
 import {launchEmulator} from './devices/android/AndroidDevice';
-import {getFlipperServerConfig} from './FlipperServerConfig';
+import {
+  FlipperServerConfig,
+  setFlipperServerConfig,
+} from './FlipperServerConfig';
 
 /**
  * FlipperServer takes care of all incoming device & client connections.
@@ -49,8 +52,8 @@ export class FlipperServerImpl implements FlipperServer {
   android: AndroidDeviceManager;
   ios: IOSDeviceManager;
 
-  constructor(public logger: Logger) {
+  constructor(config: FlipperServerConfig, public logger: Logger) {
-    getFlipperServerConfig(); // Config should be available at this point!
+    setFlipperServerConfig(config);
     const server = (this.server = new ServerController(this));
     this.android = new AndroidDeviceManager(this);
     this.ios = new IOSDeviceManager(this);
@@ -89,7 +92,7 @@ export class FlipperServerImpl implements FlipperServer {
           title: `Timed out establishing connection with "${client.appName}" on "${client.deviceName}".`,
           description:
             medium === 'WWW'
-              ? `Verify that both your computer and mobile device are on Lighthouse/VPN that you are logged in to Facebook Intern so that certificates can be exhanged. See: https://www.internalfb.com/intern/wiki/Ops/Network/Enterprise_Network_Engineering/ene_wlra/VPN_Help/Vpn/mobile/`
+              ? `Verify that both your computer and mobile device are on Lighthouse/VPN that you are logged in to Facebook Intern so that certificates can be exhanged. See: https://fburl.com/flippervpn`
               : 'Verify that your client is connected to Flipper and that there is no error related to idb.',
         });
       },

desktop/flipper-server-core/src/devices/ios/__tests__/iOSDevice.node.tsx

@@ -11,6 +11,9 @@ import {parseXcodeFromCoreSimPath} from '../iOSDeviceManager';
 import {getLogger} from 'flipper-common';
 import {IOSBridge} from '../IOSBridge';
 import {FlipperServerImpl} from '../../../FlipperServerImpl';
+import {getFlipperServerConfig} from '../../../FlipperServerConfig';
+
+const testConfig = getFlipperServerConfig();
 
 const standardCoresimulatorLog =
   'username            1264   0.0  0.1  5989740  41648   ??  Ss    2:23PM   0:12.92 /Applications/Xcode_12.4.0_fb.app/Contents/Developer/Platforms/iPhoneOS.platform/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot/usr/libexec/mobileassetd';
@@ -53,7 +56,7 @@ test('test parseXcodeFromCoreSimPath from standard locations', () => {
 });
 
 test('test getAllPromisesForQueryingDevices when xcode detected', () => {
-  const flipperServer = new FlipperServerImpl(getLogger());
+  const flipperServer = new FlipperServerImpl(testConfig, getLogger());
   flipperServer.ios.iosBridge = {} as IOSBridge;
   const promises = flipperServer.ios.getAllPromisesForQueryingDevices(
     true,
@@ -63,7 +66,7 @@ test('test getAllPromisesForQueryingDevices when xcode detected', () => {
 });
 
 test('test getAllPromisesForQueryingDevices when xcode is not detected', () => {
-  const flipperServer = new FlipperServerImpl(getLogger());
+  const flipperServer = new FlipperServerImpl(testConfig, getLogger());
   flipperServer.ios.iosBridge = {} as IOSBridge;
   const promises = flipperServer.ios.getAllPromisesForQueryingDevices(
     false,
@@ -73,7 +76,7 @@ test('test getAllPromisesForQueryingDevices when xcode is not detected', () => {
 });
 
 test('test getAllPromisesForQueryingDevices when xcode and idb are both unavailable', () => {
-  const flipperServer = new FlipperServerImpl(getLogger());
+  const flipperServer = new FlipperServerImpl(testConfig, getLogger());
   flipperServer.ios.iosBridge = {} as IOSBridge;
   const promises = flipperServer.ios.getAllPromisesForQueryingDevices(
     false,
@@ -83,7 +86,7 @@ test('test getAllPromisesForQueryingDevices when xcode and idb are both unavaila
 });
 
 test('test getAllPromisesForQueryingDevices when both idb and xcode are available', () => {
-  const flipperServer = new FlipperServerImpl(getLogger());
+  const flipperServer = new FlipperServerImpl(testConfig, getLogger());
   flipperServer.ios.iosBridge = {} as IOSBridge;
   const promises = flipperServer.ios.getAllPromisesForQueryingDevices(
     true,

desktop/flipper-server-core/src/index.tsx

@@ -7,10 +7,4 @@
  * @format
  */
 
-export {
-  FlipperServerConfig,
-  getFlipperServerConfig,
-  setFlipperServerConfig,
-} from './FlipperServerConfig';
-
 export {FlipperServerImpl} from './FlipperServerImpl';