Process username changes in storage service

ts/background.ts

@@ -146,7 +146,6 @@ import { showToast } from './util/showToast';
 import { startInteractionMode } from './windows/startInteractionMode';
 import type { MainWindowStatsType } from './windows/context';
 import { deliveryReceiptsJobQueue } from './jobs/deliveryReceiptsJobQueue';
-import { updateOurUsernameAndPni } from './util/updateOurUsernameAndPni';
 import { ReactionSource } from './reactions/ReactionSource';
 import { singleProtoJobQueue } from './jobs/singleProtoJobQueue';
 import { getInitialState } from './state/getInitialState';
@@ -2262,18 +2261,15 @@ export async function startApp(): Promise<void> {
         try {
           // Note: we always have to register our capabilities all at once, so we do this
           //   after connect on every startup
-          await Promise.all([
-            server.registerCapabilities({
-              announcementGroup: true,
-              giftBadges: true,
-              'gv2-3': true,
-              senderKey: true,
-              changeNumber: true,
-              stories: true,
-              pni: isPnpEnabled(),
-            }),
-            updateOurUsernameAndPni(),
-          ]);
+          await server.registerCapabilities({
+            announcementGroup: true,
+            giftBadges: true,
+            'gv2-3': true,
+            senderKey: true,
+            changeNumber: true,
+            stories: true,
+            pni: isPnpEnabled(),
+          });
         } catch (error) {
           log.error(
             'Error: Unable to register our capabilities.',
@@ -3532,10 +3528,7 @@ export async function startApp(): Promise<void> {
         log.info('onFetchLatestSync: fetching latest local profile');
         const ourUuid = window.textsecure.storage.user.getUuid()?.toString();
         const ourE164 = window.textsecure.storage.user.getNumber();
-        await Promise.all([
-          getProfile(ourUuid, ourE164),
-          updateOurUsernameAndPni(),
-        ]);
+        await getProfile(ourUuid, ourE164);
         break;
       }
       case FETCH_LATEST_ENUM.STORAGE_MANIFEST:

ts/models/conversations.ts

@@ -97,6 +97,7 @@ import {
   getProfileName,
   getTitle,
   getTitleNoDefault,
+  canHaveUsername,
 } from '../util/getTitle';
 import { markConversationRead } from '../util/markConversationRead';
 import { handleMessageSend } from '../util/handleMessageSend';
@@ -349,6 +350,10 @@ export class ConversationModel extends window.Backbone
 
     this.on('newmessage', this.onNewMessage);
     this.on('change:profileKey', this.onChangeProfileKey);
+    this.on(
+      'change:name change:profileName change:profileFamilyName change:e164',
+      () => this.maybeClearUsername()
+    );
 
     const sealedSender = this.get('sealedSender');
     if (sealedSender === undefined) {
@@ -1826,7 +1831,9 @@ export class ConversationModel extends window.Backbone
 
       // We had previously stored `null` instead of `undefined` in some cases. We should
       //   be able to remove this `dropNull` once usernames have gone to production.
-      username: dropNull(this.get('username')),
+      username: canHaveUsername(this.attributes, ourConversationId)
+        ? dropNull(this.get('username'))
+        : undefined,
 
       about: this.getAboutText(),
       aboutText: this.get('about'),
@@ -4218,6 +4225,50 @@ export class ConversationModel extends window.Backbone
     );
   }
 
+  async maybeClearUsername(): Promise<void> {
+    const ourConversationId =
+      window.ConversationController.getOurConversationId();
+
+    // Clear username once we have other information about the contact
+    if (
+      canHaveUsername(this.attributes, ourConversationId) ||
+      !this.get('username')
+    ) {
+      return;
+    }
+
+    log.info(`maybeClearUsername(${this.idForLogging()}): clearing username`);
+
+    this.unset('username');
+    window.Signal.Data.updateConversation(this.attributes);
+    this.captureChange('clearUsername');
+  }
+
+  async updateUsername(
+    username: string | undefined,
+    { shouldSave = true }: { shouldSave?: boolean } = {}
+  ): Promise<void> {
+    const ourConversationId =
+      window.ConversationController.getOurConversationId();
+
+    if (!canHaveUsername(this.attributes, ourConversationId)) {
+      return;
+    }
+
+    if (this.get('username') === username) {
+      return;
+    }
+
+    log.info(`updateUsername(${this.idForLogging()}): updating username`);
+
+    this.set('username', username);
+    this.captureChange('updateUsername');
+
+    if (shouldSave) {
+      await window.Signal.Data.updateConversation(this.attributes);
+    }
+  }
+
   async updateLastMessage(): Promise<void> {
     if (!this.id) {
       return;

ts/services/storageRecordOps.ts

@@ -33,6 +33,7 @@ import {
   getSafeLongFromTimestamp,
   getTimestampFromLong,
 } from '../util/timestampLongUtils';
+import { canHaveUsername } from '../util/getTitle';
 import {
   get as getUniversalExpireTimer,
   set as setUniversalExpireTimer,
@@ -156,6 +157,11 @@ export async function toContactRecord(
   if (e164) {
     contactRecord.serviceE164 = e164;
   }
+  const username = conversation.get('username');
+  const ourID = window.ConversationController.getOurConversationId();
+  if (username && canHaveUsername(conversation.attributes, ourID)) {
+    contactRecord.username = username;
+  }
   const pni = conversation.get('pni');
   if (pni && RemoteConfig.isEnabled('desktop.pnp')) {
     contactRecord.pni = pni;
@@ -978,6 +984,10 @@ export async function mergeContactRecord(
     };
   }
 
+  await conversation.updateUsername(dropNull(contactRecord.username), {
+    shouldSave: false,
+  });
+
   let needsProfileFetch = false;
   if (contactRecord.profileKey && contactRecord.profileKey.length > 0) {
     needsProfileFetch = await conversation.setProfileKey(

ts/services/username.ts

@@ -2,8 +2,6 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import { singleProtoJobQueue } from '../jobs/singleProtoJobQueue';
-import dataInterface from '../sql/Client';
-import { updateOurUsernameAndPni } from '../util/updateOurUsernameAndPni';
 import { sleep } from '../util/sleep';
 import type { UsernameReservationType } from '../types/Username';
 import { ReserveUsernameError } from '../types/Username';
@@ -54,7 +52,6 @@ export async function reserveUsername(
   const { nickname, previousUsername, abortSignal } = options;
 
   const me = window.ConversationController.getOurConversationOrThrow();
-  await updateOurUsernameAndPni();
 
   if (me.get('username') !== previousUsername) {
     throw new Error('reserveUsername: Username has changed on another device');
@@ -96,8 +93,7 @@ async function updateUsernameAndSyncProfile(
   const me = window.ConversationController.getOurConversationOrThrow();
 
   // Update backbone, update DB, then tell linked devices about profile update
-  me.set({ username });
-  dataInterface.updateConversation(me.attributes);
+  await me.updateUsername(username);
 
   try {
     await singleProtoJobQueue.add(
@@ -123,7 +119,6 @@ export async function confirmUsername(
   const { previousUsername, username, reservationToken } = reservation;
 
   const me = window.ConversationController.getOurConversationOrThrow();
-  await updateOurUsernameAndPni();
 
   if (me.get('username') !== previousUsername) {
     throw new Error('Username has changed on another device');
@@ -161,7 +156,6 @@ export async function deleteUsername(
   }
 
   const me = window.ConversationController.getOurConversationOrThrow();
-  await updateOurUsernameAndPni();
 
   if (me.get('username') !== previousUsername) {
     throw new Error('Username has changed on another device');

ts/test-mock/pnp/username_test.ts

@@ -0,0 +1,137 @@
+// Copyright 2022 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+import { Proto, StorageState } from '@signalapp/mock-server';
+import type { PrimaryDevice } from '@signalapp/mock-server';
+import createDebug from 'debug';
+
+import * as durations from '../../util/durations';
+import { uuidToBytes } from '../../util/uuidToBytes';
+import { MY_STORY_ID } from '../../types/Stories';
+import { Bootstrap } from '../bootstrap';
+import type { App } from '../bootstrap';
+
+export const debug = createDebug('mock:test:username');
+
+const IdentifierType = Proto.ManifestRecord.Identifier.Type;
+
+const USERNAME = 'signalapp.55';
+
+describe('pnp/username', function needsName() {
+  this.timeout(durations.MINUTE);
+
+  let bootstrap: Bootstrap;
+  let app: App;
+  let usernameContact: PrimaryDevice;
+
+  beforeEach(async () => {
+    bootstrap = new Bootstrap({ contactCount: 0 });
+    await bootstrap.init();
+
+    const { server, phone } = bootstrap;
+
+    usernameContact = await server.createPrimaryDevice({
+      profileName: 'ACI Contact',
+    });
+
+    let state = StorageState.getEmpty();
+
+    state = state.updateAccount({
+      profileKey: phone.profileKey.serialize(),
+      e164: phone.device.number,
+    });
+
+    state = state.addContact(usernameContact, {
+      username: USERNAME,
+      serviceE164: undefined,
+    });
+
+    // Put contact into left pane
+    state = state.pin(usernameContact);
+
+    // Add my story
+    state = state.addRecord({
+      type: IdentifierType.STORY_DISTRIBUTION_LIST,
+      record: {
+        storyDistributionList: {
+          allowsReplies: true,
+          identifier: uuidToBytes(MY_STORY_ID),
+          isBlockList: true,
+          name: MY_STORY_ID,
+          recipientUuids: [],
+        },
+      },
+    });
+
+    await phone.setStorageState(state);
+
+    app = await bootstrap.link();
+  });
+
+  afterEach(async function after() {
+    if (this.currentTest?.state !== 'passed') {
+      await bootstrap.saveLogs(app);
+    }
+
+    await app.close();
+    await bootstrap.teardown();
+  });
+
+  it('drops username when contact name becomes known', async () => {
+    const { phone } = bootstrap;
+
+    const window = await app.getWindow();
+    const leftPane = window.locator('.left-pane-wrapper');
+
+    debug('find username in the left pane');
+    await leftPane
+      .locator(
+        `[data-testid="${usernameContact.device.uuid}"] >> "@${USERNAME}"`
+      )
+      .waitFor();
+
+    debug('adding profile key for username contact');
+    let state: StorageState = await phone.expectStorageState(
+      'consistency check'
+    );
+    state = state.updateContact(usernameContact, {
+      profileKey: usernameContact.profileKey.serialize(),
+    });
+    await phone.setStorageState(state);
+    await phone.sendFetchStorage({
+      timestamp: bootstrap.getTimestamp(),
+    });
+
+    debug('find profile name in the left pane');
+    await leftPane
+      .locator(
+        `[data-testid="${usernameContact.device.uuid}"] >> ` +
+          `"${usernameContact.profileName}"`
+      )
+      .waitFor();
+
+    debug('verify that storage service state is updated');
+    {
+      const newState = await phone.waitForStorageState({
+        after: state,
+      });
+
+      const { added, removed } = newState.diff(state);
+      assert.strictEqual(added.length, 1, 'only one record must be added');
+      assert.strictEqual(removed.length, 1, 'only one record must be removed');
+
+      assert.strictEqual(
+        added[0].contact?.serviceUuid,
+        usernameContact.device.uuid
+      );
+      assert.strictEqual(added[0].contact?.username, '');
+
+      assert.strictEqual(
+        removed[0].contact?.serviceUuid,
+        usernameContact.device.uuid
+      );
+      assert.strictEqual(removed[0].contact?.username, USERNAME);
+    }
+  });
+});

ts/util/getTitle.ts

@@ -36,7 +36,7 @@ export function getTitleNoDefault(
 
   return (
     (isShort ? attributes.systemGivenName : undefined) ||
-    attributes.name ||
+    getSystemName(attributes) ||
     (isShort ? attributes.profileName : undefined) ||
     getProfileName(attributes) ||
     getNumber(attributes) ||
@@ -44,6 +44,30 @@ export function getTitleNoDefault(
   );
 }
 
+// Note that the used attributes field should match the ones we listen for
+// change on in ConversationModel (see `ConversationModel#maybeClearUsername`)
+export function canHaveUsername(
+  attributes: Pick<
+    ConversationAttributesType,
+    'id' | 'type' | 'name' | 'profileName' | 'profileFamilyName' | 'e164'
+  >,
+  ourConversationId: string | undefined
+): boolean {
+  if (!isDirectConversation(attributes)) {
+    return false;
+  }
+
+  if (ourConversationId === attributes.id) {
+    return true;
+  }
+
+  return (
+    !getSystemName(attributes) &&
+    !getProfileName(attributes) &&
+    !getNumber(attributes)
+  );
+}
+
 export function getProfileName(
   attributes: Pick<
     ConversationAttributesType,
@@ -57,6 +81,22 @@ export function getProfileName(
   return undefined;
 }
 
+export function getSystemName(
+  attributes: Pick<
+    ConversationAttributesType,
+    'systemGivenName' | 'systemFamilyName' | 'type'
+  >
+): string | undefined {
+  if (isDirectConversation(attributes)) {
+    return combineNames(
+      attributes.systemGivenName,
+      attributes.systemFamilyName
+    );
+  }
+
+  return undefined;
+}
+
 export function getNumber(
   attributes: Pick<ConversationAttributesType, 'e164' | 'type'>
 ): string {

ts/util/lookupConversationWithoutUuid.ts

@@ -97,7 +97,7 @@ export async function lookupConversationWithoutUuid(
 
         conversationId = convo.id;
 
-        convo.set({ username: foundUsername.username });
+        await convo.updateUsername(foundUsername.username);
       }
     }
 

ts/util/updateOurUsernameAndPni.ts

@@ -1,26 +0,0 @@
-// Copyright 2021 Signal Messenger, LLC
-// SPDX-License-Identifier: AGPL-3.0-only
-
-import { strictAssert } from './assert';
-import { dropNull } from './dropNull';
-
-export async function updateOurUsernameAndPni(): Promise<void> {
-  const { server } = window.textsecure;
-
-  strictAssert(
-    server,
-    'updateOurUsernameAndPni: window.textsecure.server not available'
-  );
-
-  const me = window.ConversationController.getOurConversationOrThrow();
-  const { username } = await server.whoami();
-
-  me.set({ username: dropNull(username) });
-  window.Signal.Data.updateConversation(me.attributes);
-
-  const manager = window.getAccountManager();
-  strictAssert(
-    manager,
-    'updateOurUsernameAndPni: AccountManager not available'
-  );
-}