Fix issue where messages where not queued for non-selected apps

Summary: When determing whether to queue a message, the logic checked if the plugin is enabled on the currently selected app, rather than checking if it is enabled for the receiving app. This diff fixes that.

Reviewed By: passy

Differential Revision: D20000055

fbshipit-source-id: 665f0a650dcee8f7f46aa56f399a4f7d0d0aa1e0

src/PluginContainer.tsx

@@ -417,7 +417,8 @@ export default connect<StateFromProps, DispatchFromProps, OwnProps, Store>(
         if (activePlugin && target) {
           pluginKey = getPluginKey(target.id, activePlugin.id);
           pluginIsEnabled = pluginIsStarred(
-            {selectedApp, userStarredPlugins},
+            userStarredPlugins,
+            selectedApp,
             activePlugin.id,
           );
         }

src/__tests__/__snapshots__/createMockFlipperWithPlugin.node.tsx.snap

@@ -5,10 +5,10 @@ Object {
   "androidEmulators": Array [],
   "clients": Array [
     Object {
-      "id": "TestApp#Android#unit_test#serial",
+      "id": "TestApp#Android#MockAndroidDevice#serial",
       "query": Object {
         "app": "TestApp",
-        "device": "unit_test",
+        "device": "MockAndroidDevice",
         "device_id": "serial",
         "os": "Android",
       },
@@ -25,7 +25,7 @@ Object {
     },
   ],
   "errors": Array [],
-  "selectedApp": "TestApp#Android#unit_test#serial",
+  "selectedApp": "TestApp#Android#MockAndroidDevice#serial",
   "selectedDevice": Object {
     "deviceType": "physical",
     "logs": Array [],
@@ -36,7 +36,7 @@ Object {
   "selectedPlugin": "TestPlugin",
   "staticView": null,
   "uninitializedClients": Array [],
-  "userPreferredApp": "TestApp#Android#unit_test#serial",
+  "userPreferredApp": "TestApp#Android#MockAndroidDevice#serial",
   "userPreferredDevice": "MockAndroidDevice",
   "userPreferredPlugin": "TestPlugin",
   "userStarredPlugins": Object {

src/__tests__/createMockFlipperWithPlugin.node.tsx

@@ -53,7 +53,7 @@ test('can create a Fake flipper', async () => {
       sendMessage('inc', {});
       expect(store.getState().pluginStates).toMatchInlineSnapshot(`
         Object {
-          "TestApp#Android#unit_test#serial#TestPlugin": Object {
+          "TestApp#Android#MockAndroidDevice#serial#TestPlugin": Object {
             "count": 1,
           },
         }

src/reducers/connections.tsx

@@ -621,17 +621,14 @@ export function getSelectedPluginKey(state: State): string | undefined {
 }
 
 export function pluginIsStarred(
-  state: {
+  userStarredPlugins: State['userStarredPlugins'],
-    selectedApp: string | null;
+  app: string | null,
-    userStarredPlugins: State['userStarredPlugins'];
-  },
   pluginId: string,
 ): boolean {
-  const {selectedApp} = state;
+  if (!app) {
-  if (!selectedApp) {
     return false;
   }
-  const appInfo = deconstructClientId(selectedApp);
+  const appInfo = deconstructClientId(app);
-  const starred = state.userStarredPlugins[appInfo.app];
+  const starred = userStarredPlugins[appInfo.app];
   return starred && starred.indexOf(pluginId) > -1;
 }

src/test-utils/createMockFlipperWithPlugin.tsx

@@ -40,63 +40,72 @@ export async function createMockFlipperWithPlugin(
     device: BaseDevice;
     store: Store;
     sendMessage(method: string, params: any): void;
+    createDevice(serial: string): BaseDevice;
+    createClient(device: BaseDevice, name: string): Client;
   }) => Promise<void>,
 ) {
   const store = createStore(reducers);
-  const device = new BaseDevice(
-    'serial',
-    'physical',
-    'MockAndroidDevice',
-    'Android',
-  );
   const logger = createStubLogger();
-
   store.dispatch(registerPlugins([pluginClazz]));
 
-  store.dispatch({
+  function createDevice(serial: string): BaseDevice {
-    type: 'REGISTER_DEVICE',
+    const device = new BaseDevice(
-    payload: device,
+      serial,
-  });
+      'physical',
+      'MockAndroidDevice',
+      'Android',
+    );
+    store.dispatch({
+      type: 'REGISTER_DEVICE',
+      payload: device,
+    });
+    return device;
+  }
 
-  const query: ClientQuery = {
+  function createClient(device: BaseDevice, name: string): Client {
-    app: 'TestApp',
+    const query: ClientQuery = {
-    os: 'Android',
+      app: name,
-    device: 'unit_test',
+      os: 'Android',
-    device_id: device.serial,
+      device: device.title,
-  };
+      device_id: device.serial,
-  const id = buildClientId({
+    };
-    app: query.app,
+    const id = buildClientId({
-    os: query.os,
+      app: query.app,
-    device: query.device,
+      os: query.os,
-    device_id: query.device_id,
+      device: query.device,
-  });
+      device_id: query.device_id,
+    });
 
-  const client = new Client(
+    const client = new Client(
-    id,
+      id,
-    query,
+      query,
-    null, // create a stub connection to avoid this plugin to be archived?
+      null, // create a stub connection to avoid this plugin to be archived?
-    logger,
+      logger,
-    store,
+      store,
-    [pluginClazz.id],
+      [pluginClazz.id],
-    device,
+      device,
-  );
+    );
 
-  // yikes
+    // yikes
-  client._deviceSet = device;
+    client._deviceSet = device;
-  client.device = {
+    client.device = {
-    then() {
+      then() {
-      return device;
+        return device;
-    },
+      },
-  } as any;
+    } as any;
 
-  // As convenience, by default we select the new client, star the plugin, and select it
+    // As convenience, by default we select the new client, star the plugin, and select it
-  store.dispatch({
+    store.dispatch({
-    type: 'NEW_CLIENT',
+      type: 'NEW_CLIENT',
-    payload: client,
+      payload: client,
-  });
+    });
+    return client;
+  }
+
+  const device = createDevice('serial');
+  const client = createClient(device, 'TestApp');
 
   store.dispatch(selectDevice(device));
-
   store.dispatch(selectClient(client.id));
 
   store.dispatch(
@@ -131,5 +140,7 @@ export async function createMockFlipperWithPlugin(
         }),
       );
     },
+    createDevice,
+    createClient,
   });
 }

src/utils/__tests__/messageQueue.node.tsx

@@ -9,8 +9,13 @@
 
 import {FlipperPlugin} from '../../plugin';
 import {createMockFlipperWithPlugin} from '../../test-utils/createMockFlipperWithPlugin';
-import {GK, Store, Client} from 'flipper';
+import {GK, Store, Client} from '../../';
-import {selectPlugin, starPlugin} from '../../reducers/connections';
+import {
+  selectPlugin,
+  starPlugin,
+  selectClient,
+  selectDevice,
+} from '../../reducers/connections';
 import {processMessageQueue} from '../messageQueue';
 import {getPluginKey} from '../pluginUtils';
 import {TestIdler} from '../Idler';
@@ -87,7 +92,7 @@ test('will process event with GK disabled', async () => {
       sendMessage('inc', {});
       expect(store.getState().pluginStates).toMatchInlineSnapshot(`
         Object {
-          "TestApp#Android#unit_test#serial#TestPlugin": Object {
+          "TestApp#Android#MockAndroidDevice#serial#TestPlugin": Object {
             "count": 1,
           },
         }
@@ -104,12 +109,12 @@ test('queue - events are processed immediately if plugin is selected', async ()
         expect(store.getState().connections.selectedPlugin).toBe('TestPlugin');
         sendMessage('inc', {});
         expect(store.getState().pluginStates).toMatchInlineSnapshot(`
-                  Object {
+          Object {
-                    "TestApp#Android#unit_test#serial#TestPlugin": Object {
+            "TestApp#Android#MockAndroidDevice#serial#TestPlugin": Object {
-                      "count": 1,
+              "count": 1,
-                    },
+            },
-                  }
+          }
-              `);
+        `);
         expect(store.getState().pluginMessageQueue).toMatchInlineSnapshot(
           `Object {}`,
         );
@@ -135,7 +140,7 @@ test('queue - events are NOT processed immediately if plugin is NOT selected (bu
         );
         expect(store.getState().pluginMessageQueue).toMatchInlineSnapshot(`
           Object {
-            "TestApp#Android#unit_test#serial#TestPlugin": Array [
+            "TestApp#Android#MockAndroidDevice#serial#TestPlugin": Array [
               Object {
                 "method": "inc",
                 "params": Object {},
@@ -193,6 +198,84 @@ test('queue - events are NOT processed immediately if plugin is NOT selected (bu
   );
 });
 
+test('queue - events are queued for plugins that are favorite when app is not selected', async () => {
+  await createMockFlipperWithPlugin(
+    TestPlugin,
+    async ({device, store, sendMessage, createClient}) => {
+      await GK.withWhitelistedGK('flipper_event_queue', async () => {
+        selectDeviceLogs(store);
+        expect(store.getState().connections.selectedPlugin).not.toBe(
+          'TestPlugin',
+        );
+
+        const client2 = createClient(device, 'TestApp2');
+        store.dispatch(selectClient(client2.id));
+
+        // Now we send a message to the second client, it should arrive,
+        // as the plugin was enabled already on the first client as well
+        sendMessage('inc', {delta: 2});
+        expect(store.getState().pluginStates).toMatchInlineSnapshot(
+          `Object {}`,
+        );
+        expect(store.getState().pluginMessageQueue).toMatchInlineSnapshot(
+          `
+          Object {
+            "TestApp#Android#MockAndroidDevice#serial#TestPlugin": Array [
+              Object {
+                "method": "inc",
+                "params": Object {
+                  "delta": 2,
+                },
+              },
+            ],
+          }
+        `,
+        );
+      });
+    },
+  );
+});
+
+test('queue - events are queued for plugins that are favorite when app is selected on different device', async () => {
+  await createMockFlipperWithPlugin(
+    TestPlugin,
+    async ({client, store, sendMessage, createDevice, createClient}) => {
+      await GK.withWhitelistedGK('flipper_event_queue', async () => {
+        selectDeviceLogs(store);
+        expect(store.getState().connections.selectedPlugin).not.toBe(
+          'TestPlugin',
+        );
+
+        const device2 = createDevice('serial2');
+        const client2 = createClient(device2, client.query.app); // same app id
+        store.dispatch(selectDevice(device2));
+        store.dispatch(selectClient(client2.id));
+
+        // Now we send a message to the second client, it should arrive,
+        // as the plugin was enabled already on the first client as well
+        sendMessage('inc', {delta: 2});
+        expect(store.getState().pluginStates).toMatchInlineSnapshot(
+          `Object {}`,
+        );
+        expect(store.getState().pluginMessageQueue).toMatchInlineSnapshot(
+          `
+          Object {
+            "TestApp#Android#MockAndroidDevice#serial#TestPlugin": Array [
+              Object {
+                "method": "inc",
+                "params": Object {
+                  "delta": 2,
+                },
+              },
+            ],
+          }
+        `,
+        );
+      });
+    },
+  );
+});
+
 test('queue - events processing will be paused', async () => {
   await createMockFlipperWithPlugin(
     TestPlugin,

src/utils/messageQueue.tsx

@@ -18,6 +18,7 @@ import {
 } from '../reducers/pluginMessageQueue';
 import {Idler, BaseIdler} from './Idler';
 import {pluginIsStarred, getSelectedPluginKey} from '../reducers/connections';
+import {deconstructPluginKey} from './clientUtils';
 
 const MAX_BACKGROUND_TASK_TIME = 25;
 
@@ -180,7 +181,11 @@ export function processMessageLater(
       break;
     case isSelected:
     case plugin instanceof FlipperDevicePlugin:
-    case pluginIsStarred(store.getState().connections, plugin.id):
+    case pluginIsStarred(
+      store.getState().connections.userStarredPlugins,
+      deconstructPluginKey(pluginKey).client,
+      plugin.id,
+    ):
       store.dispatch(
         queueMessage(
           pluginKey,