Request Configuration Sync After Relink (#2218)

Users lose their read receipt setting after relinking their app.
See: https://github.com/signalapp/Signal-Android/issues/7535

- [x] Request configuration sync after relink, not just on first run.
- [x] Minor: Add `is` dependency as discussed.
- [x] Unit tests!! Started a pattern where functions return their status that
      can be logged instead of logging themselves. Makes for great tests and
      might also help us debug user logs as the status will always be logged
      (with `reason` field) whether it was done or not. Otherwise, we have to
      look for the presence or absence of a log line. Of course, we have to be
      careful about log noise, but I thought this was a nice fit here.

**Manual Tests**

- Started app and set read receipts on iOS.
- Sent message from another device with read receipts enabled.
- Confirmed messages were marked read on both sides.
- Unlinked primary device.
- Relinked primary device.
- Sent messages between primary and other device.
- Confirmed read receipts are still shown.
- Confirmed logs show configuration sync.

js/background.js

@@ -294,7 +294,7 @@
     }
 
     var connectCount = 0;
-    function connect(firstRun) {
+    async function connect(firstRun) {
         console.log('connect');
 
         // Bootstrap our online/offline detection, only the first time we connect
@@ -359,22 +359,18 @@
             }
         }
 
-        // If we've just upgraded to read receipt support on desktop, kick off a
+    /* eslint-enable */
-        // one-time configuration sync request to get the read-receipt setting
+    const deviceId = textsecure.storage.user.getDeviceId();
-        // from the master device.
+    const { sendRequestConfigurationSyncMessage } = textsecure.messaging;
-        var readReceiptConfigurationSync = 'read-receipt-configuration-sync';
+    const status = await Signal.Startup.syncReadReceiptConfiguration({
-        if (!storage.get(readReceiptConfigurationSync)) {
+      deviceId,
+      sendRequestConfigurationSyncMessage,
+      storage,
+    });
+    console.log('Sync read receipt configuration status:', status);
+    /* eslint-disable */
 
-            if (!firstRun && textsecure.storage.user.getDeviceId() != '1') {
+        if (firstRun === true && deviceId != '1') {
-                textsecure.messaging.sendRequestConfigurationSyncMessage().then(function() {
-                    storage.put(readReceiptConfigurationSync, true);
-                }).catch(function(e) {
-                    console.log(e);
-                });
-            }
-        }
-
-        if (firstRun === true && textsecure.storage.user.getDeviceId() != '1') {
             if (!storage.get('theme-setting') && textsecure.storage.get('userAgent') === 'OWI') {
                 storage.put('theme-setting', 'ios');
             }

js/modules/settings.js

@@ -6,6 +6,8 @@ const LAST_PROCESSED_INDEX_KEY = 'attachmentMigration_lastProcessedIndex';
 const IS_MIGRATION_COMPLETE_KEY = 'attachmentMigration_isComplete';
 
 // Public API
+exports.READ_RECEIPT_CONFIGURATION_SYNC = 'read-receipt-configuration-sync';
+
 exports.getAttachmentMigrationLastProcessedIndex = connection =>
   exports._getItem(connection, LAST_PROCESSED_INDEX_KEY);
 

js/modules/startup.js

@@ -0,0 +1,55 @@
+const is = require('@sindresorhus/is');
+
+const Errors = require('./types/errors');
+const Settings = require('./settings');
+
+
+exports.syncReadReceiptConfiguration = async ({
+  deviceId,
+  sendRequestConfigurationSyncMessage,
+  storage,
+}) => {
+  if (!is.string(deviceId)) {
+    throw new TypeError('"deviceId" is required');
+  }
+
+  if (!is.function(sendRequestConfigurationSyncMessage)) {
+    throw new TypeError('"sendRequestConfigurationSyncMessage" is required');
+  }
+
+  if (!is.object(storage)) {
+    throw new TypeError('"storage" is required');
+  }
+
+  const isPrimaryDevice = deviceId === '1';
+  if (isPrimaryDevice) {
+    return {
+      status: 'skipped',
+      reason: 'isPrimaryDevice',
+    };
+  }
+
+  const settingName = Settings.READ_RECEIPT_CONFIGURATION_SYNC;
+  const hasPreviouslySynced = Boolean(storage.get(settingName));
+  if (hasPreviouslySynced) {
+    return {
+      status: 'skipped',
+      reason: 'hasPreviouslySynced',
+    };
+  }
+
+  try {
+    await sendRequestConfigurationSyncMessage();
+    storage.put(settingName, true);
+  } catch (error) {
+    return {
+      status: 'error',
+      reason: 'failedToSendSyncMessage',
+      error: Errors.toLogFormat(error),
+    };
+  }
+
+  return {
+    status: 'complete',
+  };
+};

package.json

@@ -44,6 +44,7 @@
     "open-coverage": "open coverage/lcov-report/index.html"
   },
   "dependencies": {
+    "@sindresorhus/is": "^0.8.0",
     "archiver": "^2.1.1",
     "blob-util": "^1.3.0",
     "blueimp-canvas-to-blob": "^3.14.0",

preload.js

@@ -146,6 +146,7 @@ window.Signal.Migrations.Migrations1DatabaseWithoutAttachmentData =
 window.Signal.Migrations.upgradeMessageSchema = upgradeMessageSchema;
 window.Signal.OS = require('./js/modules/os');
 window.Signal.Settings = require('./js/modules/settings');
+window.Signal.Startup = require('./js/modules/startup');
 
 window.Signal.Types = {};
 window.Signal.Types.Attachment = Attachment;

test/modules/startup_test.js

@@ -0,0 +1,123 @@
+const sinon = require('sinon');
+const { assert } = require('chai');
+
+const Startup = require('../../js/modules/startup');
+
+
+describe('Startup', () => {
+  const sandbox = sinon.createSandbox();
+
+  describe('syncReadReceiptConfiguration', () => {
+    afterEach(() => {
+      sandbox.restore();
+    });
+
+    it('should complete if user hasn’t previously synced', async () => {
+      const deviceId = '2';
+      const sendRequestConfigurationSyncMessage = sandbox.spy();
+      const storagePutSpy = sandbox.spy();
+      const storage = {
+        get(name) {
+          if (name !== 'read-receipt-configuration-sync') {
+            return true;
+          }
+
+          return false;
+        },
+        put: storagePutSpy,
+      };
+
+      const expected = {
+        status: 'complete',
+      };
+
+      const actual = await Startup.syncReadReceiptConfiguration({
+        deviceId,
+        sendRequestConfigurationSyncMessage,
+        storage,
+      });
+
+      assert.deepEqual(actual, expected);
+      assert.equal(sendRequestConfigurationSyncMessage.callCount, 1);
+      assert.equal(storagePutSpy.callCount, 1);
+      assert(storagePutSpy.calledWith('read-receipt-configuration-sync', true));
+    });
+
+    it('should be skipped if this is the primary device', async () => {
+      const deviceId = '1';
+      const sendRequestConfigurationSyncMessage = () => {};
+      const storage = {};
+
+      const expected = {
+        status: 'skipped',
+        reason: 'isPrimaryDevice',
+      };
+
+      const actual = await Startup.syncReadReceiptConfiguration({
+        deviceId,
+        sendRequestConfigurationSyncMessage,
+        storage,
+      });
+
+      assert.deepEqual(actual, expected);
+    });
+
+    it('should be skipped if user has previously synced', async () => {
+      const deviceId = '2';
+      const sendRequestConfigurationSyncMessage = () => {};
+      const storage = {
+        get(name) {
+          if (name !== 'read-receipt-configuration-sync') {
+            return false;
+          }
+
+          return true;
+        },
+      };
+
+      const expected = {
+        status: 'skipped',
+        reason: 'hasPreviouslySynced',
+      };
+
+      const actual = await Startup.syncReadReceiptConfiguration({
+        deviceId,
+        sendRequestConfigurationSyncMessage,
+        storage,
+      });
+
+      assert.deepEqual(actual, expected);
+    });
+
+    it('should return error if sending of sync request fails', async () => {
+      const deviceId = '2';
+
+      const sendRequestConfigurationSyncMessage = sandbox.stub();
+      sendRequestConfigurationSyncMessage.rejects(new Error('boom'));
+
+      const storagePutSpy = sandbox.spy();
+      const storage = {
+        get(name) {
+          if (name !== 'read-receipt-configuration-sync') {
+            return true;
+          }
+
+          return false;
+        },
+        put: storagePutSpy,
+      };
+
+      const actual = await Startup.syncReadReceiptConfiguration({
+        deviceId,
+        sendRequestConfigurationSyncMessage,
+        storage,
+      });
+
+      assert.equal(actual.status, 'error');
+      assert.include(actual.error, 'boom');
+
+      assert.equal(sendRequestConfigurationSyncMessage.callCount, 1);
+      assert.equal(storagePutSpy.callCount, 0);
+    });
+  });
+});

yarn.lock

@@ -26,6 +26,10 @@
   version "0.7.0"
   resolved "https://registry.yarnpkg.com/@sindresorhus/is/-/is-0.7.0.tgz#9a06f4f137ee84d7df0460c1fdb1135ffa6c50fd"
 
+"@sindresorhus/is@^0.8.0":
+  version "0.8.0"
+  resolved "https://registry.yarnpkg.com/@sindresorhus/is/-/is-0.8.0.tgz#073aee40b0aab2d4ace33c0a2a2672a37da6fa12"
+
 "@sinonjs/formatio@^2.0.0":
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/@sinonjs/formatio/-/formatio-2.0.0.tgz#84db7e9eb5531df18a8c5e0bfb6e449e55e654b2"