Add salt to the exported device serial id

Summary:
This diff solves the following bug.

When the user imports the exported flipper data for the device which is currently running, there was a bug. A duplicate device with the same serial got created after importing the file. Due to the same `device_id`, the plugins of the exported device got imported in the already running device(which has the same device id).

To solve this problem, I have prefixed the exported `device_id` with a random string, so that the exported device is unique across the device list in the Flipper app.

Look at the vide to understand the bug

Also updated the test case accordingly

Reviewed By: danielbuechele

Differential Revision: D13973180

fbshipit-source-id: df6ee00987e81923914855cea4d76e2bd7167358

src/utils/__tests__/exportData.electron.js

@@ -8,7 +8,6 @@
 import {default as BaseDevice} from '../../devices/BaseDevice';
 import {default as ArchivedDevice} from '../../devices/ArchivedDevice';
 import {processStore} from '../exportData';
-import {IOSDevice} from '../../..';
 import {FlipperDevicePlugin} from '../../plugin.js';
 import type {Notification} from '../../plugin.js';
 import type {ClientExport} from '../../Client.js';
@@ -32,6 +31,26 @@ function generateClientIdentifier(device: BaseDevice, app: string): string {
   return identifier;
 }
 
+function generateClientIdentifierWithSalt(
+  identifier: string,
+  salt: string,
+): string {
+  let array = identifier.split('#');
+  const serial = array.pop();
+  return array.join('#') + '#' + salt + '-' + serial;
+}
+
+function generateClientFromClientWithSalt(
+  client: ClientExport,
+  salt: string,
+): ClientExport {
+  const {os, device, device_id, app} = client.query;
+  const identifier = generateClientIdentifierWithSalt(client.id, salt);
+  return {
+    id: identifier,
+    query: {app, os, device, device_id: salt + '-' + device_id},
+  };
+}
 function generateClientFromDevice(
   device: BaseDevice,
   app: string,
@@ -44,12 +63,49 @@ function generateClientFromDevice(
   };
 }
 
+test('test generateClientIndentifierWithSalt helper function', () => {
+  const device = new ArchivedDevice('serial', 'emulator', 'TestiPhone', 'iOS');
+  const identifier = generateClientIdentifier(device, 'app');
+  const saltIdentifier = generateClientIdentifierWithSalt(identifier, 'salt');
+  expect(saltIdentifier).toEqual('app#iOS#emulator#salt-serial');
+  expect(identifier).toEqual('app#iOS#emulator#serial');
+});
+
+test('test generateClientFromClientWithSalt helper function', () => {
+  const device = new ArchivedDevice('serial', 'emulator', 'TestiPhone', 'iOS');
+  const client = generateClientFromDevice(device, 'app');
+  const saltedClient = generateClientFromClientWithSalt(client, 'salt');
+  expect(saltedClient).toEqual({
+    id: 'app#iOS#emulator#salt-serial',
+    query: {
+      app: 'app',
+      os: 'iOS',
+      device: 'emulator',
+      device_id: 'salt-serial',
+    },
+  });
+  expect(client).toEqual({
+    id: 'app#iOS#emulator#serial',
+    query: {
+      app: 'app',
+      os: 'iOS',
+      device: 'emulator',
+      device_id: 'serial',
+    },
+  });
+});
+
 test('test generateClientFromDevice helper function', () => {
   const device = new ArchivedDevice('serial', 'emulator', 'TestiPhone', 'iOS');
   const client = generateClientFromDevice(device, 'app');
   expect(client).toEqual({
     id: 'app#iOS#emulator#serial',
-    query: {app: 'app', os: 'iOS', device: 'emulator', device_id: 'serial'},
+    query: {
+      app: 'app',
+      os: 'iOS',
+      device: 'emulator',
+      device_id: 'serial',
+    },
   });
 });
 
@@ -70,7 +126,7 @@ test('test generateNotifications helper function', () => {
 });
 
 test('test processStore function for empty state', () => {
-  const json = processStore([], null, {}, [], new Map());
+  const json = processStore([], null, {}, [], new Map(), 'salt');
   expect(json).toBeNull();
 });
 
@@ -81,6 +137,7 @@ test('test processStore function for an iOS device connected', () => {
     {},
     [],
     new Map(),
+    'salt',
   );
   expect(json).toBeDefined();
   // $FlowFixMe Flow doesn't that its a test and the assertion for null is already done
@@ -89,7 +146,7 @@ test('test processStore function for an iOS device connected', () => {
   expect(clients).toEqual([]);
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already done
   const {serial, deviceType, title, os} = device;
-  expect(serial).toEqual('serial');
+  expect(serial).toEqual('salt-serial');
   expect(deviceType).toEqual('emulator');
   expect(title).toEqual('TestiPhone');
   expect(os).toEqual('iOS');
@@ -108,12 +165,15 @@ test('test processStore function for an iOS device connected with client plugin
     {[clientIdentifier]: {msg: 'Test plugin'}},
     [generateClientFromDevice(device, 'testapp')],
     new Map(),
+    'salt',
   );
   expect(json).toBeDefined();
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already done
   const {pluginStates} = json.store;
   let expectedPluginState = {
-    [clientIdentifier]: {msg: 'Test plugin'},
+    [generateClientIdentifierWithSalt(clientIdentifier, 'salt')]: {
+      msg: 'Test plugin',
+    },
   };
   expect(pluginStates).toEqual(expectedPluginState);
 });
@@ -144,6 +204,7 @@ test('test processStore function to have only the client for the selected device
     selectedDevice,
     'testapp',
   );
+
   const json = processStore(
     [],
     selectedDevice,
@@ -160,18 +221,23 @@ test('test processStore function to have only the client for the selected device
       generateClientFromDevice(unselectedDevice, 'testapp'),
     ],
     new Map(),
+    'salt',
   );
+
   expect(json).toBeDefined();
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already added
   const {clients} = json;
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already added
   const {pluginStates} = json.store;
   let expectedPluginState = {
-    [selectedDeviceClientIdentifier + '#testapp']: {
+    [generateClientIdentifierWithSalt(selectedDeviceClientIdentifier, 'salt') +
+    '#testapp']: {
       msg: 'Test plugin selected device',
     },
   };
-  expect(clients).toEqual([selectedDeviceClient]);
+  expect(clients).toEqual([
+    generateClientFromClientWithSalt(selectedDeviceClient, 'salt'),
+  ]);
   expect(pluginStates).toEqual(expectedPluginState);
 });
 
@@ -211,6 +277,7 @@ test('test processStore function to have multiple clients for the selected devic
       generateClientFromDevice(selectedDevice, 'testapp2'),
     ],
     new Map(),
+    'salt',
   );
   expect(json).toBeDefined();
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already added
@@ -218,14 +285,19 @@ test('test processStore function to have multiple clients for the selected devic
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already added
   const {pluginStates} = json.store;
   let expectedPluginState = {
-    [clientIdentifierApp1 + '#testapp1']: {
+    [generateClientIdentifierWithSalt(clientIdentifierApp1, 'salt') +
+    '#testapp1']: {
       msg: 'Test plugin App1',
     },
-    [clientIdentifierApp2 + '#testapp2']: {
+    [generateClientIdentifierWithSalt(clientIdentifierApp2, 'salt') +
+    '#testapp2']: {
       msg: 'Test plugin App2',
     },
   };
-  expect(clients).toEqual([client1, client2]);
+  expect(clients).toEqual([
+    generateClientFromClientWithSalt(client1, 'salt'),
+    generateClientFromClientWithSalt(client2, 'salt'),
+  ]);
   expect(pluginStates).toEqual(expectedPluginState);
 });
 
@@ -247,6 +319,7 @@ test('test processStore function for device plugin state and no clients', () =>
     },
     [],
     new Map([['TestDevicePlugin', TestDevicePlugin]]),
+    'salt',
   );
   expect(json).toBeDefined();
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already done
@@ -254,7 +327,7 @@ test('test processStore function for device plugin state and no clients', () =>
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already done
   const {clients} = json;
   let expectedPluginState = {
-    'serial#TestDevicePlugin': {msg: 'Test Device plugin'},
+    'salt-serial#TestDevicePlugin': {msg: 'Test Device plugin'},
   };
   expect(pluginStates).toEqual(expectedPluginState);
   expect(clients).toEqual([]);
@@ -278,6 +351,7 @@ test('test processStore function for unselected device plugin state and no clien
     },
     [],
     new Map([['TestDevicePlugin', TestDevicePlugin]]),
+    'salt',
   );
   expect(json).toBeDefined();
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already done
@@ -308,23 +382,31 @@ test('test processStore function for notifications for selected device', () => {
     notification,
     client: client.id,
   };
+
   const json = processStore(
     [activeNotification],
     selectedDevice,
     {},
     [client],
     new Map([['TestDevicePlugin', TestDevicePlugin]]),
+    'salt',
   );
+
   expect(json).toBeDefined();
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already done
   const {pluginStates} = json.store;
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already done
   const {clients} = json;
   expect(pluginStates).toEqual({});
-  expect(clients).toEqual([client]);
+  expect(clients).toEqual([generateClientFromClientWithSalt(client, 'salt')]);
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already done
   const {activeNotifications} = json.store;
-  expect(activeNotifications).toEqual([activeNotification]);
+  const expectedActiveNotification = {
+    pluginId: 'TestNotification',
+    notification,
+    client: generateClientIdentifierWithSalt(client.id, 'salt'),
+  };
+  expect(activeNotifications).toEqual([expectedActiveNotification]);
 });
 
 test('test processStore function for notifications for unselected device', () => {
@@ -364,6 +446,7 @@ test('test processStore function for notifications for unselected device', () =>
     {},
     [client, unselectedclient],
     new Map(),
+    'salt',
   );
   expect(json).toBeDefined();
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already done
@@ -371,7 +454,7 @@ test('test processStore function for notifications for unselected device', () =>
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already done
   const {clients} = json;
   expect(pluginStates).toEqual({});
-  expect(clients).toEqual([client]);
+  expect(clients).toEqual([generateClientFromClientWithSalt(client, 'salt')]);
   //$FlowFixMe Flow doesn't that its a test and the assertion for null is already done
   const {activeNotifications} = json.store;
   expect(activeNotifications).toEqual([]);

src/utils/exportData.js

@@ -17,6 +17,7 @@ import {default as ArchivedDevice} from '../devices/ArchivedDevice';
 import {default as Client} from '../Client';
 import {getInstance} from '../fb-stubs/Logger.js';
 import fs from 'fs';
+import uuid from 'uuid';
 
 export type ExportType = {|
   fileVersion: '1.0.0',
@@ -78,12 +79,69 @@ export function processNotificationStates(
   return activeNotifications;
 }
 
+const addSaltToDeviceSerial = (
+  salt: string,
+  device: BaseDevice,
+  clients: Array<ClientExport>,
+  pluginStates: PluginStatesState,
+  pluginNotification: Array<PluginNotification>,
+): ExportType => {
+  const {serial} = device;
+  const newSerial = salt + '-' + serial;
+  const newDevice = new ArchivedDevice(
+    newSerial,
+    device.deviceType,
+    device.title,
+    device.os,
+  );
+  const updatedClients = clients.map((client: ClientExport) => {
+    return {
+      ...client,
+      id: client.id.replace(serial, newSerial),
+      query: {...client.query, device_id: newSerial},
+    };
+  });
+
+  const updatedPluginStates: PluginStatesState = {};
+  for (let key in pluginStates) {
+    if (!key.includes(serial)) {
+      throw new Error(
+        `Error while exporting, plugin state (${key}) does not have ${serial} in its key`,
+      );
+    }
+    const pluginData = pluginStates[key];
+    key = key.replace(serial, newSerial);
+    updatedPluginStates[key] = pluginData;
+  }
+
+  const updatedPluginNotifications = pluginNotification.map(notif => {
+    if (!notif.client || !notif.client.includes(serial)) {
+      throw new Error(
+        `Error while exporting, plugin state (${
+          notif.pluginId
+        }) does not have ${serial} in it`,
+      );
+    }
+    return {...notif, client: notif.client.replace(serial, newSerial)};
+  });
+  return {
+    fileVersion: '1.0.0',
+    clients: updatedClients,
+    device: newDevice.toJSON(),
+    store: {
+      pluginStates: updatedPluginStates,
+      activeNotifications: updatedPluginNotifications,
+    },
+  };
+};
+
 export const processStore = (
   activeNotifications: Array<PluginNotification>,
   device: ?BaseDevice,
   pluginStates: PluginStatesState,
   clients: Array<ClientExport>,
   devicePlugins: Map<string, Class<FlipperDevicePlugin<>>>,
+  salt: string,
 ): ?ExportType => {
   if (device) {
     const {serial} = device;
@@ -100,15 +158,15 @@ export const processStore = (
       activeNotifications,
       devicePlugins,
     );
-    return {
+    // Adding salt to the device id, so that the device_id in the device list is unique.
-      fileVersion: '1.0.0',
+    const exportFlipperData = addSaltToDeviceSerial(
-      clients: processedClients,
+      salt,
-      device: device.toJSON(),
+      device,
-      store: {
+      processedClients,
-        pluginStates: processedPluginStates,
+      processedPluginStates,
-        activeNotifications: processedActiveNotifications,
+      processedActiveNotifications,
-      },
+    );
-    };
+    return exportFlipperData;
   }
   return null;
 };
@@ -126,6 +184,7 @@ export function serializeStore(state: State): ?ExportType {
     pluginStates,
     clients.map(client => client.toJSON()),
     devicePlugins,
+    uuid.v4(),
   );
 }