diff --git a/config/test.json b/config/test.json index ade27eef3..fbec72db9 100644 --- a/config/test.json +++ b/config/test.json @@ -1,4 +1,4 @@ { "storageProfile": "test", - "openDevTools": false + "openDevTools": true } diff --git a/ts/ConversationController.ts b/ts/ConversationController.ts index 56bc0a776..2b04e2581 100644 --- a/ts/ConversationController.ts +++ b/ts/ConversationController.ts @@ -13,17 +13,107 @@ import type { import type { ConversationModel } from './models/conversations'; import { getContactId } from './messages/helpers'; import { maybeDeriveGroupV2Id } from './groups'; -import { assert } from './util/assert'; +import { assert, strictAssert } from './util/assert'; import { isGroupV1, isGroupV2 } from './util/whatTypeOfConversation'; import { getConversationUnreadCountForAppBadge } from './util/getConversationUnreadCountForAppBadge'; -import { UUID, isValidUuid } from './types/UUID'; +import { UUID, isValidUuid, UUIDKind } from './types/UUID'; +import type { UUIDStringType } from './types/UUID'; import { Address } from './types/Address'; import { QualifiedAddress } from './types/QualifiedAddress'; import * as log from './logging/log'; +import * as Errors from './types/errors'; import { sleep } from './util/sleep'; import { isNotNil } from './util/isNotNil'; import { MINUTE, SECOND } from './util/durations'; +type ConvoMatchType = + | { + key: 'uuid' | 'pni'; + value: UUIDStringType | undefined; + match: ConversationModel | undefined; + } + | { + key: 'e164'; + value: string | undefined; + match: ConversationModel | undefined; + }; + +const { hasOwnProperty } = Object.prototype; + +function applyChangeToConversation( + conversation: ConversationModel, + suggestedChange: Partial< + Pick + > +) { + const change = { ...suggestedChange }; + + // Clear PNI if changing e164 without associated PNI + if (hasOwnProperty.call(change, 'e164') && !change.pni) { + change.pni = undefined; + } + + // If we have a PNI but not an ACI, then the PNI will go in the UUID field + // Tricky: We need a special check here, because the PNI can be in the uuid slot + if ( + change.pni && + !change.uuid && + (!conversation.get('uuid') || + conversation.get('uuid') === conversation.get('pni')) + ) { + change.uuid = change.pni; + } + + // If we're clearing a PNI, but we didn't have an ACI - we need to clear UUID field + if ( + !change.uuid && + hasOwnProperty.call(change, 'pni') && + !change.pni && + conversation.get('uuid') === conversation.get('pni') + ) { + change.uuid = undefined; + } + + if (hasOwnProperty.call(change, 'uuid')) { + conversation.updateUuid(change.uuid); + } + if (hasOwnProperty.call(change, 'e164')) { + conversation.updateE164(change.e164); + } + if (hasOwnProperty.call(change, 'pni')) { + conversation.updatePni(change.pni); + } + + // Note: we don't do a conversation.set here, because change is limited to these fields +} + +async function mergeConversations({ + logId, + oldConversation, + newConversation, +}: { + logId: string; + oldConversation: ConversationModel; + newConversation: ConversationModel; +}) { + try { + await window.ConversationController.combineConversations( + newConversation, + oldConversation + ); + + // If the old conversation was currently displayed, we load the new one + window.Whisper.events.trigger('refreshConversation', { + newId: newConversation.get('id'), + oldId: oldConversation.get('id'), + }); + } catch (error) { + log.warn( + `${logId}: error combining contacts: ${Errors.toLogFormat(error)}` + ); + } +} + const MAX_MESSAGE_BODY_LENGTH = 64 * 1024; const { @@ -55,6 +145,8 @@ export class ConversationController { private _hasQueueEmptied = false; + private _combineConversationsQueue = new PQueue({ concurrency: 1 }); + constructor(private _conversations: ConversationModelCollectionType) { const debouncedUpdateUnreadCount = debounce( this.updateUnreadCount.bind(this), @@ -172,8 +264,8 @@ export class ConversationController { if (type === 'group') { conversation = this._conversations.add({ id, - uuid: null, - e164: null, + uuid: undefined, + e164: undefined, groupId: identifier, type, version: 2, @@ -183,8 +275,8 @@ export class ConversationController { conversation = this._conversations.add({ id, uuid: identifier, - e164: null, - groupId: null, + e164: undefined, + groupId: undefined, type, version: 2, ...additionalInitialProps, @@ -192,9 +284,9 @@ export class ConversationController { } else { conversation = this._conversations.add({ id, - uuid: null, + uuid: undefined, e164: identifier, - groupId: null, + groupId: undefined, type, version: 2, ...additionalInitialProps, @@ -270,13 +362,25 @@ export class ConversationController { getOurConversationId(): string | undefined { const e164 = window.textsecure.storage.user.getNumber(); - const uuid = window.textsecure.storage.user.getUuid()?.toString(); - return this.ensureContactIds({ + const aci = window.textsecure.storage.user + .getUuid(UUIDKind.ACI) + ?.toString(); + const pni = window.textsecure.storage.user + .getUuid(UUIDKind.PNI) + ?.toString(); + + if (!e164 && !aci && !pni) { + return undefined; + } + + const conversation = this.maybeMergeContacts({ + aci, e164, - uuid, - highTrust: true, + pni, reason: 'getOurConversationId', }); + + return conversation?.id; } getOurConversationIdOrThrow(): string { @@ -311,39 +415,210 @@ export class ConversationController { return ourDeviceId === 1; } + // Note: If you don't know what kind of UUID it is, put it in the 'aci' param. + maybeMergeContacts({ + aci: providedAci, + e164, + pni: providedPni, + reason, + mergeOldAndNew = mergeConversations, + }: { + aci?: string; + e164?: string; + pni?: string; + reason: string; + recursionCount?: number; + mergeOldAndNew?: (options: { + logId: string; + oldConversation: ConversationModel; + newConversation: ConversationModel; + }) => Promise; + }): ConversationModel | undefined { + const dataProvided = []; + if (providedAci) { + dataProvided.push('aci'); + } + if (e164) { + dataProvided.push('e164'); + } + if (providedPni) { + dataProvided.push('pni'); + } + const logId = `maybeMergeContacts/${reason}/${dataProvided.join('+')}`; + + const aci = providedAci ? UUID.cast(providedAci) : undefined; + const pni = providedPni ? UUID.cast(providedPni) : undefined; + + if (!aci && !e164 && !pni) { + throw new Error( + `${logId}: Need to provide at least one of: aci, e164, pni` + ); + } + + if (pni && !e164) { + throw new Error(`${logId}: Cannot provide pni without an e164`); + } + + const identifier = aci || e164 || pni; + strictAssert(identifier, `${logId}: identifier must be truthy!`); + + const matches: Array = [ + { + key: 'uuid', + value: aci, + match: window.ConversationController.get(aci), + }, + { + key: 'e164', + value: e164, + match: window.ConversationController.get(e164), + }, + { key: 'pni', value: pni, match: window.ConversationController.get(pni) }, + ]; + let unusedMatches: Array = []; + + let targetConversation: ConversationModel | undefined; + let matchCount = 0; + matches.forEach(item => { + const { key, value, match } = item; + + if (!value) { + return; + } + + if (!match) { + if (targetConversation) { + log.info( + `${logId}: No match for ${key}, applying to target conversation` + ); + // Note: This line might erase a known e164 or PNI + applyChangeToConversation(targetConversation, { + [key]: value, + }); + } else { + unusedMatches.push(item); + } + return; + } + + matchCount += 1; + unusedMatches.forEach(unused => { + strictAssert(unused.value, 'An unused value should always be truthy'); + + // Example: If we find that our PNI match has no ACI, then it will be our target. + // Tricky: PNI can end up in UUID slot, so we need to special-case it + if ( + !targetConversation && + (!match.get(unused.key) || + (unused.key === 'uuid' && match.get(unused.key) === pni)) + ) { + log.info( + `${logId}: Match on ${key} does not have ${unused.key}, ` + + `so it will be our target conversation - ${match.idForLogging()}` + ); + targetConversation = match; + } + + // If PNI match already has an ACI, then we need to create a new one + if (!targetConversation) { + targetConversation = this.getOrCreate(unused.value, 'private'); + log.info( + `${logId}: Match on ${key} already had ${unused.key}, ` + + `so created new target conversation - ${targetConversation.idForLogging()}` + ); + } + + log.info( + `${logId}: Applying new value for ${unused.key} to target conversation` + ); + applyChangeToConversation(targetConversation, { + [unused.key]: unused.value, + }); + }); + + unusedMatches = []; + + if (targetConversation && targetConversation !== match) { + // Clear the value on the current match, since it belongs on targetConversation! + // Note: we need to do the remove first, because it will clear the lookup! + log.info( + `${logId}: Clearing ${key} on match, and adding it to target conversation` + ); + const change: Pick< + Partial, + 'uuid' | 'e164' | 'pni' + > = { + [key]: undefined, + }; + // When the PNI is being used in the uuid field alone, we need to clear it + if (key === 'pni' && match.get('uuid') === pni) { + change.uuid = undefined; + } + applyChangeToConversation(match, change); + + applyChangeToConversation(targetConversation, { + [key]: value, + }); + + // Note: The PNI check here is just to be bulletproof; if we know a UUID is a PNI, + // then that should be put in the UUID field as well! + if (!match.get('uuid') && !match.get('e164') && !match.get('pni')) { + log.warn( + `${logId}: Removing old conversation which matched on ${key}. ` + + 'Merging with target conversation.' + ); + mergeOldAndNew({ + logId, + oldConversation: match, + newConversation: targetConversation, + }); + } + } else if (targetConversation && !targetConversation?.get(key)) { + // This is mostly for the situation where PNI was erased when updating e164 + // log.debug(`${logId}: Re-adding ${key} on target conversation`); + applyChangeToConversation(targetConversation, { + [key]: value, + }); + } + + if (!targetConversation) { + // log.debug( + // `${logId}: Match on ${key} is target conversation - ${match.idForLogging()}` + // ); + targetConversation = match; + } + }); + + if (targetConversation) { + return targetConversation; + } + + strictAssert( + matchCount === 0, + `${logId}: should be no matches if no targetConversation` + ); + + log.info(`${logId}: Creating a new conversation with all inputs`); + return this.getOrCreate(identifier, 'private', { e164, pni }); + } + /** - * Given a UUID and/or an E164, resolves to a string representing the local - * database id of the given contact. In high trust mode, it may create new contacts, - * and it may merge contacts. - * - * highTrust = uuid/e164 pairing came from CDS, the server, or your own device + * Given a UUID and/or an E164, returns a string representing the local + * database id of the given contact. Will create a new conversation if none exists; + * otherwise will return whatever is found. */ - ensureContactIds({ + lookupOrCreate({ e164, uuid, - highTrust, - reason, - }: - | { - e164?: string | null; - uuid?: string | null; - highTrust?: false; - reason?: void; - } - | { - e164?: string | null; - uuid?: string | null; - highTrust: true; - reason: string; - }): string | undefined { - // Check for at least one parameter being provided. This is necessary - // because this path can be called on startup to resolve our own ID before - // our phone number or UUID are known. The existing behavior in these - // cases can handle a returned `undefined` id, so we do that. + }: { + e164?: string | null; + uuid?: string | null; + }): ConversationModel | undefined { const normalizedUuid = uuid ? uuid.toLowerCase() : undefined; const identifier = normalizedUuid || e164; if ((!e164 && !uuid) || !identifier) { + log.warn('lookupOrCreate: Called with neither e164 nor uuid!'); return undefined; } @@ -352,147 +627,52 @@ export class ConversationController { // 1. Handle no match at all if (!convoE164 && !convoUuid) { - log.info( - 'ensureContactIds: Creating new contact, no matches found', - highTrust ? reason : 'no reason' - ); + log.info('lookupOrCreate: Creating new contact, no matches found'); const newConvo = this.getOrCreate(identifier, 'private'); - if (highTrust && e164) { + + // `identifier` would resolve to uuid if we had both, so fix up e164 + if (normalizedUuid && e164) { newConvo.updateE164(e164); } - if (normalizedUuid) { - newConvo.updateUuid(normalizedUuid); - } - if ((highTrust && e164) || normalizedUuid) { - updateConversation(newConvo.attributes); - } - return newConvo.get('id'); - - // 2. Handle match on only E164 + return newConvo; } - if (convoE164 && !convoUuid) { - const haveUuid = Boolean(normalizedUuid); - log.info( - `ensureContactIds: e164-only match found (have UUID: ${haveUuid})` - ); - // If we are only searching based on e164 anyway, then return the first result - if (!normalizedUuid) { - return convoE164.get('id'); - } - // Fill in the UUID for an e164-only contact - if (normalizedUuid && !convoE164.get('uuid')) { - if (highTrust) { - log.info( - `ensureContactIds: Adding UUID (${uuid}) to e164-only match ` + - `(${e164}), reason: ${reason}` - ); - convoE164.updateUuid(normalizedUuid); - updateConversation(convoE164.attributes); - } - return convoE164.get('id'); - } - - log.info( - 'ensureContactIds: e164 already had UUID, creating a new contact' - ); - // If existing e164 match already has UUID, create a new contact... - const newConvo = this.getOrCreate(normalizedUuid, 'private'); - - if (highTrust) { - log.info( - `ensureContactIds: Moving e164 (${e164}) from old contact ` + - `(${convoE164.get('uuid')}) to new (${uuid}), reason: ${reason}` - ); - - // Remove the e164 from the old contact... - convoE164.set({ e164: undefined }); - updateConversation(convoE164.attributes); - - // ...and add it to the new one. - newConvo.updateE164(e164); - updateConversation(newConvo.attributes); - } - - return newConvo.get('id'); - - // 3. Handle match on only UUID - } + // 2. Handle match on only UUID if (!convoE164 && convoUuid) { - if (e164 && highTrust) { - log.info( - `ensureContactIds: Adding e164 (${e164}) to UUID-only match ` + - `(${uuid}), reason: ${reason}` - ); - convoUuid.updateE164(e164); - updateConversation(convoUuid.attributes); - } - return convoUuid.get('id'); + return convoUuid; + } + + // 3. Handle match on only E164 + if (convoE164 && !convoUuid) { + return convoE164; } // For some reason, TypeScript doesn't believe that we can trust that these two values - // are truthy by this point. So we'll throw if we get there. + // are truthy by this point. So we'll throw if that isn't the case. if (!convoE164 || !convoUuid) { - throw new Error('ensureContactIds: convoE164 or convoUuid are falsey!'); - } - - // Now, we know that we have a match for both e164 and uuid checks - - if (convoE164 === convoUuid) { - return convoUuid.get('id'); - } - - if (highTrust) { - // Conflict: If e164 match already has a UUID, we remove its e164. - if (convoE164.get('uuid') && convoE164.get('uuid') !== normalizedUuid) { - log.info( - `ensureContactIds: e164 match (${e164}) had different ` + - `UUID(${convoE164.get('uuid')}) than incoming pair (${uuid}), ` + - `removing its e164, reason: ${reason}` - ); - - // Remove the e164 from the old contact... - convoE164.set({ e164: undefined }); - updateConversation(convoE164.attributes); - - // ...and add it to the new one. - convoUuid.updateE164(e164); - updateConversation(convoUuid.attributes); - - return convoUuid.get('id'); - } - - log.warn( - `ensureContactIds: Found a split contact - UUID ${normalizedUuid} and E164 ${e164}. Merging.` + throw new Error( + 'lookupOrCreate: convoE164 or convoUuid are falsey but should both be true!' ); - - // Conflict: If e164 match has no UUID, we merge. We prefer the UUID match. - // Note: no await here, we want to keep this function synchronous - convoUuid.updateE164(e164); - // `then` is used to trigger async updates, not affecting return value - // eslint-disable-next-line more/no-then - this.combineConversations(convoUuid, convoE164) - .then(() => { - // If the old conversation was currently displayed, we load the new one - window.Whisper.events.trigger('refreshConversation', { - newId: convoUuid.get('id'), - oldId: convoE164.get('id'), - }); - }) - .catch(error => { - const errorText = error && error.stack ? error.stack : error; - log.warn(`ensureContactIds error combining contacts: ${errorText}`); - }); } - return convoUuid.get('id'); + // 4. If the two lookups agree, return that conversation + if (convoE164 === convoUuid) { + return convoUuid; + } + + // 5. If the two lookups disagree, log and return the UUID match + log.warn( + `lookupOrCreate: Found a split contact - UUID ${normalizedUuid} and E164 ${e164}. Returning UUID match.` + ); + return convoUuid; } async checkForConflicts(): Promise { log.info('checkForConflicts: starting...'); const byUuid = Object.create(null); const byE164 = Object.create(null); + const byPni = Object.create(null); const byGroupV2Id = Object.create(null); // We also want to find duplicate GV1 IDs. You might expect to see a "byGroupV1Id" map // here. Instead, we check for duplicates on the derived GV2 ID. @@ -509,6 +689,7 @@ export class ConversationController { ); const uuid = conversation.get('uuid'); + const pni = conversation.get('pni'); const e164 = conversation.get('e164'); if (uuid) { @@ -532,6 +713,27 @@ export class ConversationController { } } + if (pni) { + const existing = byPni[pni]; + if (!existing) { + byPni[pni] = conversation; + } else { + log.warn(`checkForConflicts: Found conflict with pni ${pni}`); + + // Keep the newer one if it has a uuid, otherwise keep existing + if (conversation.get('uuid')) { + // Keep new one + // eslint-disable-next-line no-await-in-loop + await this.combineConversations(conversation, existing); + byPni[pni] = conversation; + } else { + // Keep existing - note that this applies if neither had an e164 + // eslint-disable-next-line no-await-in-loop + await this.combineConversations(existing, conversation); + } + } + } + if (e164) { const existing = byE164[e164]; if (!existing) { @@ -619,100 +821,110 @@ export class ConversationController { current: ConversationModel, obsolete: ConversationModel ): Promise { - const conversationType = current.get('type'); + return this._combineConversationsQueue.add(async () => { + const conversationType = current.get('type'); - if (obsolete.get('type') !== conversationType) { - assert( - false, - 'combineConversations cannot combine a private and group conversation. Doing nothing' - ); - return; - } - - const obsoleteId = obsolete.get('id'); - const obsoleteUuid = obsolete.getUuid(); - const currentId = current.get('id'); - log.warn('combineConversations: Combining two conversations', { - obsolete: obsoleteId, - current: currentId, - }); - - if (conversationType === 'private' && obsoleteUuid) { - if (!current.get('profileKey') && obsolete.get('profileKey')) { + if (!this.get(obsolete.id)) { log.warn( - 'combineConversations: Copying profile key from old to new contact' + `combineConversations: Already combined obsolete conversation ${obsolete.id}` ); - - const profileKey = obsolete.get('profileKey'); - - if (profileKey) { - await current.setProfileKey(profileKey); - } } - log.warn( - 'combineConversations: Delete all sessions tied to old conversationId' - ); - const ourUuid = window.textsecure.storage.user.getCheckedUuid(); - const deviceIds = await window.textsecure.storage.protocol.getDeviceIds({ - ourUuid, - identifier: obsoleteUuid.toString(), + if (obsolete.get('type') !== conversationType) { + assert( + false, + 'combineConversations cannot combine a private and group conversation. Doing nothing' + ); + return; + } + + const obsoleteId = obsolete.get('id'); + const obsoleteUuid = obsolete.getUuid(); + const currentId = current.get('id'); + log.warn('combineConversations: Combining two conversations', { + obsolete: obsoleteId, + current: currentId, }); - await Promise.all( - deviceIds.map(async deviceId => { - const addr = new QualifiedAddress( - ourUuid, - new Address(obsoleteUuid, deviceId) + + if (conversationType === 'private' && obsoleteUuid) { + if (!current.get('profileKey') && obsolete.get('profileKey')) { + log.warn( + 'combineConversations: Copying profile key from old to new contact' ); - await window.textsecure.storage.protocol.removeSession(addr); - }) - ); - log.warn( - 'combineConversations: Delete all identity information tied to old conversationId' - ); + const profileKey = obsolete.get('profileKey'); - if (obsoleteUuid) { - await window.textsecure.storage.protocol.removeIdentityKey( - obsoleteUuid + if (profileKey) { + await current.setProfileKey(profileKey); + } + } + + log.warn( + 'combineConversations: Delete all sessions tied to old conversationId' ); + const ourUuid = window.textsecure.storage.user.getCheckedUuid(); + const deviceIds = await window.textsecure.storage.protocol.getDeviceIds( + { + ourUuid, + identifier: obsoleteUuid.toString(), + } + ); + await Promise.all( + deviceIds.map(async deviceId => { + const addr = new QualifiedAddress( + ourUuid, + new Address(obsoleteUuid, deviceId) + ); + await window.textsecure.storage.protocol.removeSession(addr); + }) + ); + + log.warn( + 'combineConversations: Delete all identity information tied to old conversationId' + ); + + if (obsoleteUuid) { + await window.textsecure.storage.protocol.removeIdentityKey( + obsoleteUuid + ); + } + + log.warn( + 'combineConversations: Ensure that all V1 groups have new conversationId instead of old' + ); + const groups = await this.getAllGroupsInvolvingUuid(obsoleteUuid); + groups.forEach(group => { + const members = group.get('members'); + const withoutObsolete = without(members, obsoleteId); + const currentAdded = uniq([...withoutObsolete, currentId]); + + group.set({ + members: currentAdded, + }); + updateConversation(group.attributes); + }); } + // Note: we explicitly don't want to update V2 groups + log.warn( - 'combineConversations: Ensure that all V1 groups have new conversationId instead of old' + 'combineConversations: Delete the obsolete conversation from the database' ); - const groups = await this.getAllGroupsInvolvingUuid(obsoleteUuid); - groups.forEach(group => { - const members = group.get('members'); - const withoutObsolete = without(members, obsoleteId); - const currentAdded = uniq([...withoutObsolete, currentId]); + await removeConversation(obsoleteId); - group.set({ - members: currentAdded, - }); - updateConversation(group.attributes); + log.warn('combineConversations: Update messages table'); + await migrateConversationMessages(obsoleteId, currentId); + + log.warn( + 'combineConversations: Eliminate old conversation from ConversationController lookups' + ); + this._conversations.remove(obsolete); + this._conversations.resetLookups(); + + log.warn('combineConversations: Complete!', { + obsolete: obsoleteId, + current: currentId, }); - } - - // Note: we explicitly don't want to update V2 groups - - log.warn( - 'combineConversations: Delete the obsolete conversation from the database' - ); - await removeConversation(obsoleteId); - - log.warn('combineConversations: Update messages table'); - await migrateConversationMessages(obsoleteId, currentId); - - log.warn( - 'combineConversations: Eliminate old conversation from ConversationController lookups' - ); - this._conversations.remove(obsolete); - this._conversations.resetLookups(); - - log.warn('combineConversations: Complete!', { - obsolete: obsoleteId, - current: currentId, }); } diff --git a/ts/SignalProtocolStore.ts b/ts/SignalProtocolStore.ts index db6bfb8d0..8a4db04b7 100644 --- a/ts/SignalProtocolStore.ts +++ b/ts/SignalProtocolStore.ts @@ -1045,11 +1045,11 @@ export class SignalProtocolStore extends EventsMixin { } const { uuid, deviceId } = qualifiedAddress; - const conversationId = window.ConversationController.ensureContactIds({ + const conversation = window.ConversationController.lookupOrCreate({ uuid: uuid.toString(), }); strictAssert( - conversationId !== undefined, + conversation !== undefined, 'storeSession: Ensure contact ids failed' ); const id = qualifiedAddress.toString(); @@ -1059,7 +1059,7 @@ export class SignalProtocolStore extends EventsMixin { id, version: 2, ourUuid: qualifiedAddress.ourUuid.toString(), - conversationId, + conversationId: conversation.id, uuid: uuid.toString(), deviceId, record: record.serialize().toString('base64'), @@ -1376,12 +1376,9 @@ export class SignalProtocolStore extends EventsMixin { const { uuid } = qualifiedAddress; // First, fetch this conversation - const conversationId = window.ConversationController.ensureContactIds({ + const conversation = window.ConversationController.lookupOrCreate({ uuid: uuid.toString(), }); - assert(conversationId, `lightSessionReset/${id}: missing conversationId`); - - const conversation = window.ConversationController.get(conversationId); assert(conversation, `lightSessionReset/${id}: missing conversation`); log.warn(`lightSessionReset/${id}: Resetting session`); diff --git a/ts/background.ts b/ts/background.ts index 1171ac181..204d97567 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -2593,12 +2593,13 @@ export async function startApp(): Promise { let conversation; - const senderId = window.ConversationController.ensureContactIds({ - e164: sender, - uuid: senderUuid, - highTrust: true, - reason: `onTyping(${typing.timestamp})`, - }); + const senderConversation = window.ConversationController.maybeMergeContacts( + { + e164: sender, + aci: senderUuid, + reason: `onTyping(${typing.timestamp})`, + } + ); // We multiplex between GV1/GV2 groups here, but we don't kick off migrations if (groupV2Id) { @@ -2607,14 +2608,14 @@ export async function startApp(): Promise { if (!conversation && groupId) { conversation = window.ConversationController.get(groupId); } - if (!groupV2Id && !groupId && senderId) { - conversation = window.ConversationController.get(senderId); + if (!groupV2Id && !groupId && senderConversation) { + conversation = senderConversation; } const ourId = window.ConversationController.getOurConversationId(); - if (!senderId) { - log.warn('onTyping: ensureContactIds returned falsey senderId!'); + if (!senderConversation) { + log.warn('onTyping: maybeMergeContacts returned falsey sender!'); return; } if (!ourId) { @@ -2648,7 +2649,6 @@ export async function startApp(): Promise { ); return; } - const senderConversation = window.ConversationController.get(senderId); if (!senderConversation) { log.warn('onTyping: No conversation for sender!'); return; @@ -2660,6 +2660,7 @@ export async function startApp(): Promise { return; } + const senderId = senderConversation.id; conversation.notifyTyping({ isTyping: started, fromMe: senderId === ourId, @@ -2728,14 +2729,12 @@ export async function startApp(): Promise { return; } - const detailsId = window.ConversationController.ensureContactIds({ + const conversation = window.ConversationController.maybeMergeContacts({ e164: details.number, - uuid: details.uuid, - highTrust: true, + aci: details.uuid, reason: 'onContactReceived', }); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const conversation = window.ConversationController.get(detailsId)!; + strictAssert(conversation, 'need conversation to queue the job!'); // It's important to use queueJob here because we might update the expiration timer // and we don't want conflicts with incoming message processing happening on the @@ -2918,10 +2917,9 @@ export async function startApp(): Promise { function onEnvelopeReceived({ envelope }: EnvelopeEvent) { const ourUuid = window.textsecure.storage.user.getUuid()?.toString(); if (envelope.sourceUuid && envelope.sourceUuid !== ourUuid) { - window.ConversationController.ensureContactIds({ + window.ConversationController.maybeMergeContacts({ e164: envelope.source, - uuid: envelope.sourceUuid, - highTrust: true, + aci: envelope.sourceUuid, reason: `onEnvelopeReceived(${envelope.timestamp})`, }); } @@ -2991,11 +2989,11 @@ export async function startApp(): Promise { reaction.targetTimestamp, 'Reaction without targetTimestamp' ); - const fromId = window.ConversationController.ensureContactIds({ + const fromConversation = window.ConversationController.lookupOrCreate({ e164: data.source, uuid: data.sourceUuid, }); - strictAssert(fromId, 'Reaction without fromId'); + strictAssert(fromConversation, 'Reaction without fromConversation'); log.info('Queuing incoming reaction for', reaction.targetTimestamp); const attributes: ReactionAttributesType = { @@ -3004,7 +3002,7 @@ export async function startApp(): Promise { targetAuthorUuid, targetTimestamp: reaction.targetTimestamp, timestamp, - fromId, + fromId: fromConversation.id, source: ReactionSource.FromSomeoneElse, }; const reactionModel = Reactions.getSingleton().add(attributes); @@ -3024,16 +3022,16 @@ export async function startApp(): Promise { 'Delete missing targetSentTimestamp' ); strictAssert(data.serverTimestamp, 'Delete missing serverTimestamp'); - const fromId = window.ConversationController.ensureContactIds({ + const fromConversation = window.ConversationController.lookupOrCreate({ e164: data.source, uuid: data.sourceUuid, }); - strictAssert(fromId, 'Delete missing fromId'); + strictAssert(fromConversation, 'Delete missing fromConversation'); const attributes: DeleteAttributesType = { targetSentTimestamp: del.targetSentTimestamp, serverTimestamp: data.serverTimestamp, - fromId, + fromId: fromConversation.id, }; const deleteModel = Deletes.getSingleton().add(attributes); @@ -3056,13 +3054,11 @@ export async function startApp(): Promise { } async function onProfileKeyUpdate({ data, confirm }: ProfileKeyUpdateEvent) { - const conversationId = window.ConversationController.ensureContactIds({ + const conversation = window.ConversationController.maybeMergeContacts({ + aci: data.sourceUuid, e164: data.source, - uuid: data.sourceUuid, - highTrust: true, reason: 'onProfileKeyUpdate', }); - const conversation = window.ConversationController.get(conversationId); if (!conversation) { log.error( @@ -3141,19 +3137,17 @@ export async function startApp(): Promise { result: SendStateByConversationId, { destinationUuid, destination, isAllowedToReplyToStory } ) => { - const conversationId = window.ConversationController.ensureContactIds( - { - uuid: destinationUuid, - e164: destination, - } - ); - if (!conversationId || conversationId === ourId) { + const conversation = window.ConversationController.lookupOrCreate({ + uuid: destinationUuid, + e164: destination, + }); + if (!conversation || conversation.id === ourId) { return result; } return { ...result, - [conversationId]: { + [conversation.id]: { isAllowedToReplyToStory, status: SendStatus.Sent, updatedAt: timestamp, @@ -3283,15 +3277,14 @@ export async function startApp(): Promise { } // If we can't find one, we treat this as a normal GroupV1 group - const fromContactId = window.ConversationController.ensureContactIds({ + const fromContact = window.ConversationController.maybeMergeContacts({ + aci: sourceUuid, e164: source, - uuid: sourceUuid, - highTrust: true, reason: `getMessageDescriptor(${message.timestamp}): group v1`, }); const conversationId = window.ConversationController.ensureGroup(id, { - addedBy: fromContactId, + addedBy: fromContact?.id, }); return { @@ -3300,22 +3293,21 @@ export async function startApp(): Promise { }; } - const id = window.ConversationController.ensureContactIds({ + const conversation = window.ConversationController.maybeMergeContacts({ + aci: destinationUuid, e164: destination, - uuid: destinationUuid, - highTrust: true, reason: `getMessageDescriptor(${message.timestamp}): private`, }); - if (!id) { + if (!conversation) { confirm(); throw new Error( - `getMessageDescriptor/${message.timestamp}: ensureContactIds returned falsey id` + `getMessageDescriptor/${message.timestamp}: maybeMergeContacts returned falsey conversation` ); } return { type: Message.PRIVATE, - id, + id: conversation.id, }; }; @@ -3726,11 +3718,10 @@ export async function startApp(): Promise { }>): void { const { envelopeTimestamp, timestamp, source, sourceUuid, sourceDevice } = event.receipt; - const sourceConversationId = window.ConversationController.ensureContactIds( + const sourceConversation = window.ConversationController.maybeMergeContacts( { + aci: sourceUuid, e164: source, - uuid: sourceUuid, - highTrust: true, reason: `onReadOrViewReceipt(${envelopeTimestamp})`, } ); @@ -3740,14 +3731,14 @@ export async function startApp(): Promise { sourceUuid, sourceDevice, envelopeTimestamp, - sourceConversationId, + sourceConversation?.id, 'for sent message', timestamp ); event.confirm(); - if (!window.storage.get('read-receipt-setting') || !sourceConversationId) { + if (!window.storage.get('read-receipt-setting') || !sourceConversation) { return; } @@ -3760,7 +3751,7 @@ export async function startApp(): Promise { const attributes: MessageReceiptAttributesType = { messageSentAt: timestamp, receiptTimestamp: envelopeTimestamp, - sourceConversationId, + sourceConversationId: sourceConversation?.id, sourceUuid, sourceDevice, type, @@ -3774,10 +3765,11 @@ export async function startApp(): Promise { function onReadSync(ev: ReadSyncEvent) { const { envelopeTimestamp, sender, senderUuid, timestamp } = ev.read; const readAt = envelopeTimestamp; - const senderId = window.ConversationController.ensureContactIds({ + const senderConversation = window.ConversationController.lookupOrCreate({ e164: sender, uuid: senderUuid, }); + const senderId = senderConversation?.id; log.info( 'read sync', @@ -3811,10 +3803,11 @@ export async function startApp(): Promise { function onViewSync(ev: ViewSyncEvent) { const { envelopeTimestamp, senderE164, senderUuid, timestamp } = ev.view; - const senderId = window.ConversationController.ensureContactIds({ + const senderConversation = window.ConversationController.lookupOrCreate({ e164: senderE164, uuid: senderUuid, }); + const senderId = senderConversation?.id; log.info( 'view sync', @@ -3853,11 +3846,10 @@ export async function startApp(): Promise { ev.confirm(); - const sourceConversationId = window.ConversationController.ensureContactIds( + const sourceConversation = window.ConversationController.maybeMergeContacts( { + aci: sourceUuid, e164: source, - uuid: sourceUuid, - highTrust: true, reason: `onDeliveryReceipt(${envelopeTimestamp})`, } ); @@ -3867,13 +3859,13 @@ export async function startApp(): Promise { source, sourceUuid, sourceDevice, - sourceConversationId, + sourceConversation?.id, envelopeTimestamp, 'for sent message', timestamp ); - if (!sourceConversationId) { + if (!sourceConversation) { log.info('no conversation for', source, sourceUuid); return; } @@ -3891,7 +3883,7 @@ export async function startApp(): Promise { const attributes: MessageReceiptAttributesType = { messageSentAt: timestamp, receiptTimestamp: envelopeTimestamp, - sourceConversationId, + sourceConversationId: sourceConversation?.id, sourceUuid, sourceDevice, type: MessageReceiptType.Delivery, diff --git a/ts/groups.ts b/ts/groups.ts index fe00cd633..f1fa820ba 100644 --- a/ts/groups.ts +++ b/ts/groups.ts @@ -2501,11 +2501,11 @@ export function buildMigrationBubble( ...(newAttributes.membersV2 || []).map(item => item.uuid), ...(newAttributes.pendingMembersV2 || []).map(item => item.uuid), ].map(uuid => { - const conversationId = window.ConversationController.ensureContactIds({ + const conversation = window.ConversationController.lookupOrCreate({ uuid, }); - strictAssert(conversationId, `Conversation not found for ${uuid}`); - return conversationId; + strictAssert(conversation, `Conversation not found for ${uuid}`); + return conversation.id; }); const droppedMemberIds: Array = difference( previousGroupV1MembersIds, diff --git a/ts/messageModifiers/MessageRequests.ts b/ts/messageModifiers/MessageRequests.ts index 581c1c978..7a9f5ca1f 100644 --- a/ts/messageModifiers/MessageRequests.ts +++ b/ts/messageModifiers/MessageRequests.ts @@ -103,12 +103,10 @@ export class MessageRequests extends Collection { conversation = window.ConversationController.get(groupId); } if (!conversation && (threadE164 || threadUuid)) { - conversation = window.ConversationController.get( - window.ConversationController.ensureContactIds({ - e164: threadE164, - uuid: threadUuid, - }) - ); + conversation = window.ConversationController.lookupOrCreate({ + e164: threadE164, + uuid: threadUuid, + }); } if (!conversation) { diff --git a/ts/messageModifiers/Reactions.ts b/ts/messageModifiers/Reactions.ts index 99f96eb98..a6a5e569c 100644 --- a/ts/messageModifiers/Reactions.ts +++ b/ts/messageModifiers/Reactions.ts @@ -44,11 +44,11 @@ export class Reactions extends Collection { const senderId = getContactId(message.attributes); const sentAt = message.get('sent_at'); const reactionsBySource = this.filter(re => { - const targetSenderId = window.ConversationController.ensureContactIds({ + const targetSender = window.ConversationController.lookupOrCreate({ uuid: re.get('targetAuthorUuid'), }); const targetTimestamp = re.get('targetTimestamp'); - return targetSenderId === senderId && targetTimestamp === sentAt; + return targetSender?.id === senderId && targetTimestamp === sentAt; }); if (reactionsBySource.length > 0) { @@ -87,13 +87,14 @@ export class Reactions extends Collection { try { // The conversation the target message was in; we have to find it in the database // to to figure that out. - const targetConversationId = - window.ConversationController.ensureContactIds({ + const targetAuthorConversation = + window.ConversationController.lookupOrCreate({ uuid: reaction.get('targetAuthorUuid'), }); + const targetConversationId = targetAuthorConversation?.id; if (!targetConversationId) { throw new Error( - 'onReaction: No conversationId returned from ensureContactIds!' + 'onReaction: No conversationId returned from lookupOrCreate!' ); } diff --git a/ts/messageModifiers/ReadSyncs.ts b/ts/messageModifiers/ReadSyncs.ts index 6b4b4f745..b98229b7a 100644 --- a/ts/messageModifiers/ReadSyncs.ts +++ b/ts/messageModifiers/ReadSyncs.ts @@ -58,13 +58,13 @@ export class ReadSyncs extends Collection { } forMessage(message: MessageModel): ReadSyncModel | null { - const senderId = window.ConversationController.ensureContactIds({ + const sender = window.ConversationController.lookupOrCreate({ e164: message.get('source'), uuid: message.get('sourceUuid'), }); const sync = this.find(item => { return ( - item.get('senderId') === senderId && + item.get('senderId') === sender?.id && item.get('timestamp') === message.get('sent_at') ); }); @@ -84,12 +84,12 @@ export class ReadSyncs extends Collection { ); const found = messages.find(item => { - const senderId = window.ConversationController.ensureContactIds({ + const sender = window.ConversationController.lookupOrCreate({ e164: item.source, uuid: item.sourceUuid, }); - return isIncoming(item) && senderId === sync.get('senderId'); + return isIncoming(item) && sender?.id === sync.get('senderId'); }); if (!found) { diff --git a/ts/messageModifiers/ViewSyncs.ts b/ts/messageModifiers/ViewSyncs.ts index d338256d3..2f1df154f 100644 --- a/ts/messageModifiers/ViewSyncs.ts +++ b/ts/messageModifiers/ViewSyncs.ts @@ -35,13 +35,13 @@ export class ViewSyncs extends Collection { } forMessage(message: MessageModel): Array { - const senderId = window.ConversationController.ensureContactIds({ + const sender = window.ConversationController.lookupOrCreate({ e164: message.get('source'), uuid: message.get('sourceUuid'), }); const syncs = this.filter(item => { return ( - item.get('senderId') === senderId && + item.get('senderId') === sender?.id && item.get('timestamp') === message.get('sent_at') ); }); @@ -63,12 +63,12 @@ export class ViewSyncs extends Collection { ); const found = messages.find(item => { - const senderId = window.ConversationController.ensureContactIds({ + const sender = window.ConversationController.lookupOrCreate({ e164: item.source, uuid: item.sourceUuid, }); - return senderId === sync.get('senderId'); + return sender?.id === sync.get('senderId'); }); if (!found) { diff --git a/ts/messages/helpers.ts b/ts/messages/helpers.ts index 44925d040..46998e20d 100644 --- a/ts/messages/helpers.ts +++ b/ts/messages/helpers.ts @@ -36,7 +36,7 @@ export function isQuoteAMatch( } const { authorUuid, id } = quote; - const authorConversationId = window.ConversationController.ensureContactIds({ + const authorConversation = window.ConversationController.lookupOrCreate({ e164: 'author' in quote ? quote.author : undefined, uuid: authorUuid, }); @@ -44,7 +44,7 @@ export function isQuoteAMatch( return ( message.sent_at === id && message.conversationId === conversationId && - getContactId(message) === authorConversationId + getContactId(message) === authorConversation?.id ); } @@ -58,10 +58,11 @@ export function getContactId( return window.ConversationController.getOurConversationId(); } - return window.ConversationController.ensureContactIds({ + const conversation = window.ConversationController.lookupOrCreate({ e164: source, uuid: sourceUuid, }); + return conversation?.id; } export function getContact( diff --git a/ts/model-types.d.ts b/ts/model-types.d.ts index 14f1138d7..9b948dfad 100644 --- a/ts/model-types.d.ts +++ b/ts/model-types.d.ts @@ -322,6 +322,7 @@ export type ConversationAttributesType = { // Private core info uuid?: UUIDStringType; + pni?: UUIDStringType; e164?: string; // Private other fields diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index e411a9ed9..40d3f1e91 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -1275,11 +1275,11 @@ export class ConversationModel extends window.Backbone const e164 = message.get('source'); const sourceDevice = message.get('sourceDevice'); - const sourceId = window.ConversationController.ensureContactIds({ + const source = window.ConversationController.lookupOrCreate({ uuid, e164, }); - const typingToken = `${sourceId}.${sourceDevice}`; + const typingToken = `${source?.id}.${sourceDevice}`; // Clear typing indicator for a given contact if we receive a message from them this.clearContactTypingTimer(typingToken); @@ -1869,10 +1869,10 @@ export class ConversationModel extends window.Backbone updateE164(e164?: string | null): void { const oldValue = this.get('e164'); - if (e164 && e164 !== oldValue) { - this.set('e164', e164); + if (e164 !== oldValue) { + this.set('e164', e164 || undefined); - if (oldValue) { + if (oldValue && e164) { this.addChangeNumberNotification(oldValue, e164); } @@ -1883,13 +1883,32 @@ export class ConversationModel extends window.Backbone updateUuid(uuid?: string): void { const oldValue = this.get('uuid'); - if (uuid && uuid !== oldValue) { - this.set('uuid', UUID.cast(uuid.toLowerCase())); + if (uuid !== oldValue) { + this.set('uuid', uuid ? UUID.cast(uuid.toLowerCase()) : undefined); window.Signal.Data.updateConversation(this.attributes); this.trigger('idUpdated', this, 'uuid', oldValue); } } + updatePni(pni?: string): void { + const oldValue = this.get('pni'); + if (pni !== oldValue) { + this.set('pni', pni ? UUID.cast(pni.toLowerCase()) : undefined); + + if ( + oldValue && + pni && + (!this.get('uuid') || this.get('uuid') === oldValue) + ) { + // TODO: DESKTOP-3974 + this.addKeyChange(UUID.checkedLookup(oldValue)); + } + + window.Signal.Data.updateConversation(this.attributes); + this.trigger('idUpdated', this, 'pni', oldValue); + } + } + updateGroupId(groupId?: string): void { const oldValue = this.get('groupId'); if (groupId && groupId !== oldValue) { @@ -5389,6 +5408,9 @@ window.Whisper.ConversationCollection = window.Backbone.Collection.extend({ if (idProp === 'uuid') { delete this._byUuid[oldValue]; } + if (idProp === 'pni') { + delete this._byPni[oldValue]; + } if (idProp === 'groupId') { delete this._byGroupId[oldValue]; } @@ -5401,6 +5423,10 @@ window.Whisper.ConversationCollection = window.Backbone.Collection.extend({ if (uuid) { this._byUuid[uuid] = model; } + const pni = model.get('pni'); + if (pni) { + this._byPni[pni] = model; + } const groupId = model.get('groupId'); if (groupId) { this._byGroupId[groupId] = model; @@ -5441,6 +5467,16 @@ window.Whisper.ConversationCollection = window.Backbone.Collection.extend({ } } + const pni = model.get('pni'); + if (pni) { + const existing = this._byPni[pni]; + + // Prefer the contact with both uuid and pni + if (!existing || (existing && !existing.get('uuid'))) { + this._byPni[pni] = model; + } + } + const groupId = model.get('groupId'); if (groupId) { this._byGroupId[groupId] = model; @@ -5451,6 +5487,7 @@ window.Whisper.ConversationCollection = window.Backbone.Collection.extend({ eraseLookups() { this._byE164 = Object.create(null); this._byUuid = Object.create(null); + this._byPni = Object.create(null); this._byGroupId = Object.create(null); }, @@ -5510,6 +5547,7 @@ window.Whisper.ConversationCollection = window.Backbone.Collection.extend({ this._byE164[id] || this._byE164[`+${id}`] || this._byUuid[id] || + this._byPni[id] || this._byGroupId[id] || window.Backbone.Collection.prototype.get.call(this, id) ); diff --git a/ts/models/messages.ts b/ts/models/messages.ts index 874ba531d..337fab00c 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -291,12 +291,12 @@ export class MessageModel extends window.Backbone.Model { const sourceDevice = this.get('sourceDevice'); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const sourceId = window.ConversationController.ensureContactIds({ + const conversation = window.ConversationController.lookupOrCreate({ e164: source, uuid: sourceUuid, })!; - return `${sourceId}.${sourceDevice}-${sentAt}`; + return `${conversation?.id}.${sourceDevice}-${sentAt}`; } getReceivedAt(): number { @@ -2137,14 +2137,13 @@ export class MessageModel extends window.Backbone.Model { return; } - const destinationConversationId = - window.ConversationController.ensureContactIds({ - uuid: destinationUuid, - e164: destination, - highTrust: true, + const destinationConversation = + window.ConversationController.maybeMergeContacts({ + aci: destinationUuid, + e164: destination || undefined, reason: `handleDataMessage(${initialMessage.timestamp})`, }); - if (!destinationConversationId) { + if (!destinationConversation) { return; } @@ -2155,9 +2154,9 @@ export class MessageModel extends window.Backbone.Model { const previousSendState = getOwn( sendStateByConversationId, - destinationConversationId + destinationConversation.id ); - sendStateByConversationId[destinationConversationId] = + sendStateByConversationId[destinationConversation.id] = previousSendState ? sendStateReducer(previousSendState, { type: SendActionType.Sent, @@ -2274,7 +2273,7 @@ export class MessageModel extends window.Backbone.Model { UUIDKind.ACI ); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const senderId = window.ConversationController.ensureContactIds({ + const sender = window.ConversationController.lookupOrCreate({ e164: source, uuid: sourceUuid, })!; @@ -2348,7 +2347,7 @@ export class MessageModel extends window.Backbone.Model { // Drop incoming messages to announcement only groups where sender is not admin if ( conversation.get('announcementsOnly') && - !conversation.isAdmin(UUID.checkedLookup(senderId)) + !conversation.isAdmin(UUID.checkedLookup(sender?.id)) ) { confirm(); return; @@ -2565,8 +2564,6 @@ export class MessageModel extends window.Backbone.Model { conversation.set({ addedBy: getContactId(message.attributes) }); } } else if (initialMessage.group.type === GROUP_TYPES.QUIT) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const sender = window.ConversationController.get(senderId)!; const inGroup = Boolean( sender && (conversation.get('members') || []).includes(sender.id) @@ -2682,14 +2679,11 @@ export class MessageModel extends window.Backbone.Model { } else if (isDirectConversation(conversation.attributes)) { conversation.setProfileKey(profileKey); } else { - const localId = window.ConversationController.ensureContactIds({ + const local = window.ConversationController.lookupOrCreate({ e164: source, uuid: sourceUuid, }); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - window.ConversationController.get(localId)!.setProfileKey( - profileKey - ); + local?.setProfileKey(profileKey); } } diff --git a/ts/services/storageRecordOps.ts b/ts/services/storageRecordOps.ts index d4de21f70..b726a4280 100644 --- a/ts/services/storageRecordOps.ts +++ b/ts/services/storageRecordOps.ts @@ -863,22 +863,16 @@ export async function mergeContactRecord( return { hasConflict: false, shouldDrop: true, details: ['our own uuid'] }; } - const id = window.ConversationController.ensureContactIds({ + const conversation = window.ConversationController.maybeMergeContacts({ + aci: uuid, e164, - uuid, - highTrust: true, reason: 'mergeContactRecord', }); - if (!id) { - throw new Error(`No ID for ${storageID}`); + if (!conversation) { + throw new Error(`No conversation for ${storageID}`); } - const conversation = await window.ConversationController.getOrCreateAndWait( - id, - 'private' - ); - let needsProfileFetch = false; if (contactRecord.profileKey && contactRecord.profileKey.length > 0) { needsProfileFetch = await conversation.setProfileKey( @@ -1129,32 +1123,32 @@ export async function mergeAccountRecord( const remotelyPinnedConversationPromises = pinnedConversations.map( async ({ contact, legacyGroupId, groupMasterKey }) => { - let conversationId: string | undefined; + let conversation: ConversationModel | undefined; if (contact) { - conversationId = - window.ConversationController.ensureContactIds(contact); + conversation = window.ConversationController.lookupOrCreate(contact); } else if (legacyGroupId && legacyGroupId.length) { - conversationId = Bytes.toBinary(legacyGroupId); + const groupId = Bytes.toBinary(legacyGroupId); + conversation = window.ConversationController.get(groupId); } else if (groupMasterKey && groupMasterKey.length) { const groupFields = deriveGroupFields(groupMasterKey); const groupId = Bytes.toBase64(groupFields.id); - conversationId = groupId; + conversation = window.ConversationController.get(groupId); } else { log.error( 'storageService.mergeAccountRecord: Invalid identifier received' ); } - if (!conversationId) { + if (!conversation) { log.error( 'storageService.mergeAccountRecord: missing conversation id.' ); return undefined; } - return window.ConversationController.get(conversationId); + return conversation; } ); diff --git a/ts/sql/Client.ts b/ts/sql/Client.ts index 4a0563af4..eea321517 100644 --- a/ts/sql/Client.ts +++ b/ts/sql/Client.ts @@ -215,6 +215,7 @@ const dataInterface: ClientInterface = { updateConversation, updateConversations, removeConversation, + _removeAllConversations, updateAllConversationColors, removeAllProfileKeyCredentials, @@ -1084,6 +1085,10 @@ async function removeConversation(id: string): Promise { } } +async function _removeAllConversations(): Promise { + await channels._removeAllConversations(); +} + async function eraseStorageServiceStateFromConversations(): Promise { await channels.eraseStorageServiceStateFromConversations(); } diff --git a/ts/sql/Interface.ts b/ts/sql/Interface.ts index 3fe6b09ee..3eff8c1b4 100644 --- a/ts/sql/Interface.ts +++ b/ts/sql/Interface.ts @@ -414,6 +414,7 @@ export type DataInterface = { // updateConversation is a normal data method on Server, a sync batch-add on Client updateConversations: (array: Array) => Promise; // removeConversation handles either one id or an array on Server, and one id on Client + _removeAllConversations: () => Promise; updateAllConversationColors: ( conversationColor?: ConversationColorType, customColorData?: { diff --git a/ts/sql/Server.ts b/ts/sql/Server.ts index 176ef953e..745f34d31 100644 --- a/ts/sql/Server.ts +++ b/ts/sql/Server.ts @@ -207,6 +207,7 @@ const dataInterface: ServerInterface = { updateConversation, updateConversations, removeConversation, + _removeAllConversations, updateAllConversationColors, removeAllProfileKeyCredentials, @@ -1478,6 +1479,11 @@ async function removeConversation(id: Array | string): Promise { batchMultiVarQuery(db, id, removeConversationsSync); } +async function _removeAllConversations(): Promise { + const db = getInstance(); + db.prepare('DELETE from conversations;').run(); +} + async function getConversationById( id: string ): Promise { diff --git a/ts/state/smart/CallManager.tsx b/ts/state/smart/CallManager.tsx index 2c812e76d..5f0a6688b 100644 --- a/ts/state/smart/CallManager.tsx +++ b/ts/state/smart/CallManager.tsx @@ -121,10 +121,10 @@ const mapStateToActiveCallProp = ( const conversationSelectorByUuid = memoize< (uuid: UUIDStringType) => undefined | ConversationType >(uuid => { - const conversationId = window.ConversationController.ensureContactIds({ + const convoForUuid = window.ConversationController.lookupOrCreate({ uuid, }); - return conversationId ? conversationSelector(conversationId) : undefined; + return convoForUuid ? conversationSelector(convoForUuid.id) : undefined; }); const baseResult = { diff --git a/ts/test-electron/ConversationController_test.ts b/ts/test-electron/ConversationController_test.ts new file mode 100644 index 000000000..49f22d45e --- /dev/null +++ b/ts/test-electron/ConversationController_test.ts @@ -0,0 +1,865 @@ +// Copyright 2022 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; + +import { UUID } from '../types/UUID'; +import { strictAssert } from '../util/assert'; + +import type { ConversationModel } from '../models/conversations'; +import type { UUIDStringType } from '../types/UUID'; + +const ACI_1 = UUID.generate().toString(); +const ACI_2 = UUID.generate().toString(); +const E164_1 = '+14155550111'; +const E164_2 = '+14155550112'; +const PNI_1 = UUID.generate().toString(); +const PNI_2 = UUID.generate().toString(); +const reason = 'test'; + +type ParamsType = { + uuid?: UUIDStringType; + aci?: UUIDStringType; + e164?: string; + pni?: UUIDStringType; +}; + +describe('ConversationController', () => { + describe('maybeMergeContacts', () => { + let mergeOldAndNew: (options: { + logId: string; + oldConversation: ConversationModel; + newConversation: ConversationModel; + }) => Promise; + + beforeEach(async () => { + await window.Signal.Data._removeAllConversations(); + + window.ConversationController.reset(); + await window.ConversationController.load(); + + mergeOldAndNew = () => { + throw new Error('mergeOldAndNew: Should not be called!'); + }; + }); + + // Verifying incoming data + describe('data validation', () => { + it('throws when provided no data', () => { + assert.throws(() => { + window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + reason, + }); + }, 'Need to provide at least one'); + }); + it('throws when provided a pni with no e164', () => { + assert.throws(() => { + window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + pni: PNI_1, + reason, + }); + }, 'Cannot provide pni without an e164'); + }); + }); + + function create( + name: string, + { uuid, aci, e164, pni }: ParamsType + ): ConversationModel { + const identifier = aci || uuid || e164 || pni; + const serviceId = aci || uuid || pni; + + strictAssert(identifier, 'create needs aci, e164, pni, or uuid'); + + const conversation = window.ConversationController.getOrCreate( + identifier, + 'private', + { uuid: serviceId, e164, pni } + ); + expectLookups(conversation, name, { uuid, aci, e164, pni }); + + return conversation; + } + + function expectLookups( + conversation: ConversationModel | undefined, + name: string, + { uuid, aci, e164, pni }: ParamsType + ) { + assert.exists(conversation, `${name} conversation exists`); + + // Verify that this conversation hasn't been deleted + assert.strictEqual( + window.ConversationController.get(conversation?.id)?.id, + conversation?.id, + `${name} vs. lookup by id` + ); + + if (uuid) { + assert.strictEqual( + window.ConversationController.get(uuid)?.id, + conversation?.id, + `${name} vs. lookup by uuid` + ); + } + if (aci) { + assert.strictEqual( + window.ConversationController.get(aci)?.id, + conversation?.id, + `${name} vs. lookup by aci` + ); + } + if (e164) { + assert.strictEqual( + window.ConversationController.get(e164)?.id, + conversation?.id, + `${name} vs. lookup by e164` + ); + } + if (pni) { + assert.strictEqual( + window.ConversationController.get(pni)?.id, + conversation?.id, + `${name} vs. lookup by pni` + ); + } + } + + function expectPropsAndLookups( + conversation: ConversationModel | undefined, + name: string, + { uuid, aci, e164, pni }: ParamsType + ) { + assert.exists(conversation, `${name} conversation exists`); + assert.strictEqual( + conversation?.get('uuid'), + aci || uuid, + `${name} uuid matches` + ); + assert.strictEqual( + conversation?.get('e164'), + e164, + `${name} e164 matches` + ); + assert.strictEqual(conversation?.get('pni'), pni, `${name} pni matches`); + + expectLookups(conversation, name, { uuid, e164, pni }); + } + + function expectDeleted(conversation: ConversationModel, name: string) { + assert.isUndefined( + window.ConversationController.get(conversation.id), + `${name} has been deleted` + ); + } + + describe('non-destructive updates', () => { + it('creates a new conversation with just ACI if no matches', () => { + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + reason, + }); + + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + }); + + const second = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + reason, + }); + + expectPropsAndLookups(second, 'second', { + aci: ACI_1, + }); + + assert.strictEqual(result?.id, second?.id, 'result and second match'); + }); + it('creates a new conversation with just e164 if no matches', () => { + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + e164: E164_1, + reason, + }); + + expectPropsAndLookups(result, 'result', { + e164: E164_1, + }); + + const second = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + e164: E164_1, + reason, + }); + + expectPropsAndLookups(second, 'second', { + e164: E164_1, + }); + + assert.strictEqual(result?.id, second?.id, 'result and second match'); + }); + it('creates a new conversation with all data if no matches', () => { + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + reason, + }); + + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + const second = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + reason, + }); + + expectPropsAndLookups(second, 'second', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + assert.strictEqual(result?.id, second?.id, 'result and second match'); + }); + + it('fetches all-data conversation with ACI-only query', () => { + const initial = create('initial', { + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + reason, + }); + + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + assert.strictEqual(result?.id, initial?.id, 'result and initial match'); + }); + + it('fetches all-data conversation with e164+PNI query', () => { + const initial = create('initial', { + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + e164: E164_1, + pni: PNI_1, + reason, + }); + + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + assert.strictEqual(result?.id, initial?.id, 'result and initial match'); + }); + + it('adds ACI to conversation with e164+PNI', () => { + const initial = create('initial', { + e164: E164_1, + pni: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + assert.strictEqual(initial?.id, result?.id, 'result and initial match'); + }); + it('adds e164+PNI to conversation with just ACI', () => { + const initial = create('initial', { + uuid: ACI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + assert.strictEqual(result?.id, initial?.id, 'result and initial match'); + }); + it('adds e164 to conversation with ACI+PNI', () => { + const initial = create('initial', { + aci: ACI_1, + pni: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + assert.strictEqual(result?.id, initial?.id, 'result and initial match'); + }); + it('adds PNI to conversation with ACI+e164', () => { + const initial = create('initial', { + aci: ACI_1, + e164: E164_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + assert.strictEqual(initial?.id, result?.id, 'result and initial match'); + }); + it('adds PNI to conversation with just e164', () => { + const initial = create('initial', { + e164: E164_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: PNI_1, + e164: E164_1, + pni: PNI_1, + }); + + assert.strictEqual(initial?.id, result?.id, 'result and initial match'); + }); + it('adds PNI+ACI to conversation with just e164', () => { + const initial = create('initial', { + e164: E164_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + assert.strictEqual(initial?.id, result?.id, 'result and initial match'); + }); + it('adds ACI+e164 to conversation with just PNI', () => { + const initial = create('initial', { + pni: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + assert.strictEqual(initial?.id, result?.id, 'result and initial match'); + }); + + it('promotes PNI used as generic UUID to be in the PNI field as well', () => { + const initial = create('initial', { + aci: PNI_1, + e164: E164_1, + }); + expectPropsAndLookups(initial, 'initial', { + uuid: PNI_1, + e164: E164_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: PNI_1, + e164: E164_1, + pni: PNI_1, + }); + + assert.strictEqual(initial?.id, result?.id, 'result and initial match'); + }); + }); + + describe('with destructive updates', () => { + it('replaces e164+PNI in conversation with matching ACI', () => { + const initial = create('initial', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_2, + pni: PNI_2, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_2, + pni: PNI_2, + }); + + assert.isUndefined( + window.ConversationController.get(E164_1), + 'old e164 no longer found' + ); + assert.isUndefined( + window.ConversationController.get(PNI_1), + 'old pni no longer found' + ); + + assert.strictEqual(result?.id, initial?.id, 'result and initial match'); + }); + + it('replaces PNI in conversation with e164+PNI', () => { + const initial = create('initial', { + pni: PNI_1, + e164: E164_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + pni: PNI_2, + e164: E164_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: PNI_2, + e164: E164_1, + pni: PNI_2, + }); + + assert.isUndefined( + window.ConversationController.get(PNI_1), + 'old pni no longer found' + ); + + assert.strictEqual(result?.id, initial?.id, 'result and initial match'); + }); + it('replaces PNI in conversation with all data', () => { + const initial = create('initial', { + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_2, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_2, + }); + + assert.isUndefined( + window.ConversationController.get(PNI_1), + 'old pni no longer found' + ); + + assert.strictEqual(result?.id, initial?.id, 'result and initial match'); + }); + + it('removes e164+PNI from previous conversation with an ACI, adds all data to new conversation', () => { + const initial = create('initial', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_2, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: ACI_2, + e164: E164_1, + pni: PNI_1, + }); + + expectPropsAndLookups(initial, 'initial', { uuid: ACI_1 }); + + assert.notStrictEqual( + initial?.id, + result?.id, + 'result and initial should not match' + ); + }); + it('removes e164+PNI from previous conversation with an ACI, adds to ACI match', () => { + const initial = create('initial', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + const aciOnly = create('aciOnly', { + uuid: ACI_2, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_2, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(aciOnly, 'aciOnly', { + uuid: ACI_2, + e164: E164_1, + pni: PNI_1, + }); + + expectPropsAndLookups(initial, 'initial', { uuid: ACI_1 }); + + assert.strictEqual( + aciOnly?.id, + result?.id, + 'result and aciOnly should match' + ); + }); + + it('removes PNI from previous conversation, adds it to e164-only match', () => { + const withE164 = create('withE164', { + e164: E164_1, + }); + const withPNI = create('withPNI', { + e164: E164_2, + pni: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: PNI_1, + e164: E164_1, + pni: PNI_1, + }); + + expectPropsAndLookups(withPNI, 'withPNI', { e164: E164_2 }); + + assert.strictEqual( + withE164?.id, + result?.id, + 'result and initial should match' + ); + }); + it('removes PNI from previous conversation, adds it new e164+PNI conversation', () => { + const initial = create('initial', { + e164: E164_1, + pni: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + e164: E164_2, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: PNI_1, + e164: E164_2, + pni: PNI_1, + }); + + expectPropsAndLookups(initial, 'initial', { e164: E164_1 }); + + assert.notStrictEqual( + initial?.id, + result?.id, + 'result and initial should not match' + ); + }); + it('deletes PNI-only previous conversation, adds it to e164 match', () => { + mergeOldAndNew = ({ oldConversation }) => { + window.ConversationController.dangerouslyRemoveById( + oldConversation.id + ); + return Promise.resolve(); + }; + + const withE164 = create('withE164', { + e164: E164_1, + }); + const withPNI = create('withPNI', { + pni: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: PNI_1, + e164: E164_1, + pni: PNI_1, + }); + + expectDeleted(withPNI, 'withPNI'); + + assert.strictEqual( + withE164?.id, + result?.id, + 'result and initial should match' + ); + }); + it('deletes previous conversation with PNI as UUID only, adds it to e164 match', () => { + mergeOldAndNew = ({ oldConversation }) => { + window.ConversationController.dangerouslyRemoveById( + oldConversation.id + ); + return Promise.resolve(); + }; + + const withE164 = create('withE164', { + e164: E164_1, + }); + const withPNI = create('withPNI', { + uuid: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: PNI_1, + e164: E164_1, + pni: PNI_1, + }); + + expectDeleted(withPNI, 'withPNI'); + + assert.strictEqual( + withE164?.id, + result?.id, + 'result and initial should match' + ); + }); + it('deletes e164+PNI previous conversation, adds data to ACI match', () => { + mergeOldAndNew = ({ oldConversation }) => { + window.ConversationController.dangerouslyRemoveById( + oldConversation.id + ); + return Promise.resolve(); + }; + + const withE164 = create('withE164', { + e164: E164_1, + pni: PNI_1, + }); + const withACI = create('withPNI', { + aci: ACI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + expectDeleted(withE164, 'withE164'); + + assert.strictEqual( + withACI?.id, + result?.id, + 'result and initial should match' + ); + }); + + it('handles three matching conversations: ACI-only, with E164, and with PNI', () => { + const withACI = create('withACI', { + aci: ACI_1, + }); + const withE164 = create('withE164', { + aci: ACI_2, + e164: E164_1, + }); + const withPNI = create('withPNI', { + pni: PNI_1, + e164: E164_2, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + expectPropsAndLookups(withE164, 'withE164', { aci: ACI_2 }); + expectPropsAndLookups(withPNI, 'withPNI', { e164: E164_2 }); + + assert.strictEqual(result?.id, withACI?.id, 'result and withACI match'); + }); + it('handles three matching conversations: ACI-only, E164-only (deleted), and with PNI', () => { + mergeOldAndNew = ({ oldConversation }) => { + window.ConversationController.dangerouslyRemoveById( + oldConversation.id + ); + return Promise.resolve(); + }; + + const withACI = create('withACI', { + aci: ACI_1, + }); + const withE164 = create('withE164', { + e164: E164_1, + }); + const withPNI = create('withPNI', { + pni: PNI_1, + e164: E164_2, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + expectPropsAndLookups(withPNI, 'withPNI', { e164: E164_2 }); + + expectDeleted(withE164, 'withE164'); + + assert.strictEqual(result?.id, withACI?.id, 'result and withACI match'); + }); + it('merges three matching conversations: ACI-only, E164-only (deleted), PNI-only (deleted)', () => { + mergeOldAndNew = ({ oldConversation }) => { + window.ConversationController.dangerouslyRemoveById( + oldConversation.id + ); + return Promise.resolve(); + }; + + const withACI = create('withACI', { + aci: ACI_1, + }); + const withE164 = create('withE164', { + e164: E164_1, + }); + const withPNI = create('withPNI', { + pni: PNI_1, + }); + + const result = window.ConversationController.maybeMergeContacts({ + mergeOldAndNew, + aci: ACI_1, + e164: E164_1, + pni: PNI_1, + reason, + }); + expectPropsAndLookups(result, 'result', { + uuid: ACI_1, + e164: E164_1, + pni: PNI_1, + }); + + expectDeleted(withPNI, 'withPNI'); + expectDeleted(withE164, 'withE164'); + + assert.strictEqual(result?.id, withACI?.id, 'result and withACI match'); + }); + }); + }); +}); diff --git a/ts/test-electron/updateConversationsWithUuidLookup_test.ts b/ts/test-electron/updateConversationsWithUuidLookup_test.ts index 763065711..48528f16b 100644 --- a/ts/test-electron/updateConversationsWithUuidLookup_test.ts +++ b/ts/test-electron/updateConversationsWithUuidLookup_test.ts @@ -27,15 +27,15 @@ describe('updateConversationsWithUuidLookup', () => { ); } - ensureContactIds({ + maybeMergeContacts({ e164, - uuid: uuidFromServer, - highTrust, + aci: uuidFromServer, + reason, }: { e164?: string | null; - uuid?: string | null; - highTrust?: boolean; - }): string | undefined { + aci?: string | null; + reason?: string; + }): ConversationModel | undefined { assert( e164, 'FakeConversationController is not set up for this case (E164 must be provided)' @@ -45,8 +45,51 @@ describe('updateConversationsWithUuidLookup', () => { 'FakeConversationController is not set up for this case (UUID must be provided)' ); assert( - highTrust, - 'FakeConversationController is not set up for this case (must be "high trust")' + reason, + 'FakeConversationController must be provided a reason when merging' + ); + const normalizedUuid = uuidFromServer!.toLowerCase(); + + const convoE164 = this.get(e164); + const convoUuid = this.get(normalizedUuid); + assert( + convoE164 || convoUuid, + 'FakeConversationController is not set up for this case (at least one conversation should be found)' + ); + + if (convoE164 && convoUuid) { + if (convoE164 === convoUuid) { + return convoUuid; + } + + convoE164.unset('e164'); + convoUuid.updateE164(e164); + return convoUuid; + } + + if (convoE164 && !convoUuid) { + convoE164.updateUuid(normalizedUuid); + return convoE164; + } + + assert.fail('FakeConversationController should never get here'); + return undefined; + } + + lookupOrCreate({ + e164, + uuid: uuidFromServer, + }: { + e164?: string | null; + uuid?: string | null; + }): string | undefined { + assert( + e164, + 'FakeConversationController is not set up for this case (E164 must be provided)' + ); + assert( + uuidFromServer, + 'FakeConversationController is not set up for this case (UUID must be provided)' ); const normalizedUuid = uuidFromServer!.toLowerCase(); @@ -62,13 +105,10 @@ describe('updateConversationsWithUuidLookup', () => { return convoUuid.get('id'); } - convoE164.unset('e164'); - convoUuid.updateE164(e164); return convoUuid.get('id'); } if (convoE164 && !convoUuid) { - convoE164.updateUuid(normalizedUuid); return convoE164.get('id'); } @@ -218,7 +258,7 @@ describe('updateConversationsWithUuidLookup', () => { assert.isUndefined(conversation.get('discoveredUnregisteredAt')); }); - it('marks conversations unregistered if we already had a UUID for them, even if the account does not exist on server', async () => { + it('marks conversations unregistered and removes UUID if the account does not exist on server', async () => { const existingUuid = UUID.generate().toString(); const conversation = createConversation({ e164: '+13215559876', @@ -238,7 +278,7 @@ describe('updateConversationsWithUuidLookup', () => { messaging: fakeMessaging, }); - assert.strictEqual(conversation.get('uuid'), existingUuid); + assert.isUndefined(conversation.get('uuid')); assert.isNumber(conversation.get('discoveredUnregisteredAt')); }); }); diff --git a/ts/textsecure/AccountManager.ts b/ts/textsecure/AccountManager.ts index 5128b5e08..24a3b8180 100644 --- a/ts/textsecure/AccountManager.ts +++ b/ts/textsecure/AccountManager.ts @@ -615,10 +615,9 @@ export default class AccountManager extends EventTarget { // This needs to be done very early, because it changes how things are saved in the // database. Your identity, for example, in the saveIdentityWithAttributes call // below. - const conversationId = window.ConversationController.ensureContactIds({ + const conversationId = window.ConversationController.maybeMergeContacts({ + aci: ourUuid, e164: number, - uuid: ourUuid, - highTrust: true, reason: 'createAccount', }); diff --git a/ts/types/StorageUIKeys.ts b/ts/types/StorageUIKeys.ts index df7d12b10..a7e4cedc1 100644 --- a/ts/types/StorageUIKeys.ts +++ b/ts/types/StorageUIKeys.ts @@ -22,6 +22,7 @@ export const STORAGE_UI_KEYS: ReadonlyArray = [ 'incoming-call-notification', 'notification-draw-attention', 'notification-setting', + 'pinnedConversationIds', 'preferred-audio-input-device', 'preferred-audio-output-device', 'preferred-video-input-device', diff --git a/ts/updateConversationsWithUuidLookup.ts b/ts/updateConversationsWithUuidLookup.ts index 03eb2fde3..56c6e3432 100644 --- a/ts/updateConversationsWithUuidLookup.ts +++ b/ts/updateConversationsWithUuidLookup.ts @@ -15,7 +15,7 @@ export async function updateConversationsWithUuidLookup({ }: Readonly<{ conversationController: Pick< ConversationController, - 'ensureContactIds' | 'get' + 'maybeMergeContacts' | 'get' >; conversations: ReadonlyArray; messaging: Pick; @@ -40,14 +40,12 @@ export async function updateConversationsWithUuidLookup({ const uuidFromServer = getOwn(serverLookup, e164); if (uuidFromServer) { - const finalConversationId = conversationController.ensureContactIds({ - e164, - uuid: uuidFromServer, - highTrust: true, - reason: 'updateConversationsWithUuidLookup', - }); const maybeFinalConversation = - conversationController.get(finalConversationId); + conversationController.maybeMergeContacts({ + aci: uuidFromServer, + e164, + reason: 'updateConversationsWithUuidLookup', + }); assert( maybeFinalConversation, 'updateConversationsWithUuidLookup: expected a conversation to be found or created' diff --git a/ts/util/findStoryMessage.ts b/ts/util/findStoryMessage.ts index c076292ec..3badd9671 100644 --- a/ts/util/findStoryMessage.ts +++ b/ts/util/findStoryMessage.ts @@ -68,14 +68,14 @@ function isStoryAMatch( return false; } - const authorConversationId = window.ConversationController.ensureContactIds({ + const authorConversation = window.ConversationController.lookupOrCreate({ e164: undefined, uuid: authorUuid, }); return ( message.sent_at === sentTimestamp && - getContactId(message) === authorConversationId && + getContactId(message) === authorConversation?.id && (message.conversationId === conversationId || message.conversationId === ourConversationId) ); diff --git a/ts/util/getProfile.ts b/ts/util/getProfile.ts index bcece3048..013d2be16 100644 --- a/ts/util/getProfile.ts +++ b/ts/util/getProfile.ts @@ -4,15 +4,11 @@ import * as log from '../logging/log'; import { profileService } from '../services/profiles'; -export async function getProfile( - providedUuid?: string, - providedE164?: string -): Promise { - const id = window.ConversationController.ensureContactIds({ - uuid: providedUuid, - e164: providedE164, +export async function getProfile(uuid?: string, e164?: string): Promise { + const c = window.ConversationController.lookupOrCreate({ + uuid, + e164, }); - const c = window.ConversationController.get(id); if (!c) { log.error('getProfile: failed to find conversation; doing nothing'); return; diff --git a/ts/util/handleRetry.ts b/ts/util/handleRetry.ts index 495c1fa88..f7bbb31ea 100644 --- a/ts/util/handleRetry.ts +++ b/ts/util/handleRetry.ts @@ -616,18 +616,9 @@ function startAutomaticSessionReset(decryptionError: DecryptionErrorEventData) { scheduleSessionReset(senderUuid, senderDevice); - const conversationId = window.ConversationController.ensureContactIds({ + const conversation = window.ConversationController.lookupOrCreate({ uuid: senderUuid, }); - - if (!conversationId) { - log.warn( - 'onLightSessionReset: No conversation id, cannot add message to timeline' - ); - return; - } - const conversation = window.ConversationController.get(conversationId); - if (!conversation) { log.warn( 'onLightSessionReset: No conversation, cannot add message to timeline' diff --git a/ts/util/lookupConversationWithoutUuid.ts b/ts/util/lookupConversationWithoutUuid.ts index 1d6904994..36af8d1e7 100644 --- a/ts/util/lookupConversationWithoutUuid.ts +++ b/ts/util/lookupConversationWithoutUuid.ts @@ -73,25 +73,24 @@ export async function lookupConversationWithoutUuid( const serverLookup = await messaging.getUuidsForE164s([options.e164]); if (serverLookup[options.e164]) { - conversationId = window.ConversationController.ensureContactIds({ + const convo = window.ConversationController.maybeMergeContacts({ + aci: serverLookup[options.e164] || undefined, e164: options.e164, - uuid: serverLookup[options.e164], - highTrust: true, reason: 'startNewConversationWithoutUuid(e164)', }); + conversationId = convo?.id; } } else { const foundUsername = await checkForUsername(options.username); if (foundUsername) { - conversationId = window.ConversationController.ensureContactIds({ + const convo = window.ConversationController.lookupOrCreate({ uuid: foundUsername.uuid, - highTrust: true, - reason: 'startNewConversationWithoutUuid(username)', }); - const convo = window.ConversationController.get(conversationId); strictAssert(convo, 'We just ensured conversation existence'); + conversationId = convo.id; + convo.set({ username: foundUsername.username }); } } diff --git a/ts/util/markConversationRead.ts b/ts/util/markConversationRead.ts index f132d3de1..86bc964b3 100644 --- a/ts/util/markConversationRead.ts +++ b/ts/util/markConversationRead.ts @@ -89,10 +89,10 @@ export async function markConversationRead( originalReadStatus: messageSyncData.originalReadStatus, senderE164: messageSyncData.source, senderUuid: messageSyncData.sourceUuid, - senderId: window.ConversationController.ensureContactIds({ + senderId: window.ConversationController.lookupOrCreate({ e164: messageSyncData.source, uuid: messageSyncData.sourceUuid, - }), + })?.id, timestamp: messageSyncData.sent_at, hasErrors: message ? hasErrors(message.attributes) : false, }; diff --git a/ts/util/sendReceipts.ts b/ts/util/sendReceipts.ts index 71307b3e7..fe53fbec2 100644 --- a/ts/util/sendReceipts.ts +++ b/ts/util/sendReceipts.ts @@ -63,21 +63,21 @@ export async function sendReceipts({ return result; } - const senderId = window.ConversationController.ensureContactIds({ + const sender = window.ConversationController.lookupOrCreate({ e164: senderE164, uuid: senderUuid, }); - if (!senderId) { + if (!sender) { throw new Error( 'no conversation found with that E164/UUID. Cannot send this receipt' ); } - const existingGroup = result.get(senderId); + const existingGroup = result.get(sender.id); if (existingGroup) { existingGroup.push(receipt); } else { - result.set(senderId, [receipt]); + result.set(sender.id, [receipt]); } return result; diff --git a/ts/views/conversation_view.tsx b/ts/views/conversation_view.tsx index 55ff21fb3..132137c5a 100644 --- a/ts/views/conversation_view.tsx +++ b/ts/views/conversation_view.tsx @@ -1354,12 +1354,10 @@ export class ConversationView extends window.Backbone.View { message: { attachments: message.attachments || [], conversationId: - window.ConversationController.get( - window.ConversationController.ensureContactIds({ - uuid: message.sourceUuid, - e164: message.source, - }) - )?.id || message.conversationId, + window.ConversationController.lookupOrCreate({ + uuid: message.sourceUuid, + e164: message.source, + })?.id || message.conversationId, id: message.id, received_at: message.received_at, received_at_ms: Number(message.received_at_ms), @@ -1816,12 +1814,10 @@ export class ConversationView extends window.Backbone.View { attachments: message.get('attachments') || [], id: message.get('id'), conversationId: - window.ConversationController.get( - window.ConversationController.ensureContactIds({ - uuid: message.get('sourceUuid'), - e164: message.get('source'), - }) - )?.id || message.get('conversationId'), + window.ConversationController.lookupOrCreate({ + uuid: message.get('sourceUuid'), + e164: message.get('source'), + })?.id || message.get('conversationId'), received_at: message.get('received_at'), received_at_ms: Number(message.get('received_at_ms')), sent_at: message.get('sent_at'), @@ -2116,16 +2112,16 @@ export class ConversationView extends window.Backbone.View { } startConversation(e164: string, uuid: UUIDStringType): void { - const conversationId = window.ConversationController.ensureContactIds({ + const conversation = window.ConversationController.lookupOrCreate({ e164, uuid, }); strictAssert( - conversationId, + conversation, `startConversation failed given ${e164}/${uuid} combination` ); - this.openConversation(conversationId); + this.openConversation(conversation.id); } async openConversation(