From 587cb5de1663fa58f813ed671a852848e497d2bc Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Fri, 18 Aug 2023 11:07:12 -0400 Subject: [PATCH] Fix unexpected SSE's. Fixes #13115 --- .../RecipientTableTest_getAndPossiblyMerge.kt | 69 ++++++++++++++++++- .../securesms/database/RecipientTable.kt | 26 ++++--- .../database/model/RecipientRecord.kt | 2 +- 3 files changed, 81 insertions(+), 16 deletions(-) diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt index 5ff3d965f3..a3afff91cd 100644 --- a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt +++ b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt @@ -444,6 +444,64 @@ class RecipientTableTest_getAndPossiblyMerge { expectChangeNumberEvent() } + test("steal, e164+aci & aci, no pni provided, existing aci session") { + given(E164_A, null, ACI_A, aciSession = true) + given(null, null, ACI_B) + + process(E164_A, null, ACI_B) + + expect(null, null, ACI_A) + expect(E164_A, null, ACI_B) + + expectNoSessionSwitchoverEvent() + } + + test("steal, e164+pni+aci & aci, no pni provided, existing aci session") { + given(E164_A, PNI_A, ACI_A, aciSession = true) + given(null, null, ACI_B) + + process(E164_A, null, ACI_B) + + expect(null, PNI_A, ACI_A) + expect(E164_A, null, ACI_B) + + expectNoSessionSwitchoverEvent() + } + + test("steal, e164+pni+aci & aci, no pni provided, existing pni session") { + given(E164_A, PNI_A, ACI_A, pniSession = true) + given(null, null, ACI_B) + + process(E164_A, null, ACI_B) + + expect(null, PNI_A, ACI_A) + expect(E164_A, null, ACI_B) + + expectNoSessionSwitchoverEvent() + } + + test("steal, e164+pni & aci, no pni provided, no pni session") { + given(E164_A, PNI_A, null) + given(null, null, ACI_A) + + process(E164_A, null, ACI_A) + + expect(null, PNI_A, null) + expect(E164_A, null, ACI_A) + } + + test("steal, e164+pni & aci, no pni provided, pni session") { + given(E164_A, PNI_A, null, pniSession = true) + given(null, null, ACI_A) + + process(E164_A, null, ACI_A) + + expect(null, PNI_A, null) + expect(E164_A, null, ACI_A) + + expectNoSessionSwitchoverEvent() + } + test("merge, e164 & pni & aci, all provided") { given(E164_A, null, null) given(null, PNI_A, null) @@ -967,7 +1025,8 @@ class RecipientTableTest_getAndPossiblyMerge { pni: PNI?, aci: ACI?, createThread: Boolean = true, - pniSession: Boolean = false + pniSession: Boolean = false, + aciSession: Boolean = false ): RecipientId { val id = insert(e164, pni, aci) generatedIds += id @@ -985,6 +1044,14 @@ class RecipientTableTest_getAndPossiblyMerge { SignalDatabase.sessions.store(pni, SignalProtocolAddress(pni.toString(), 1), SessionRecord()) } + if (aciSession) { + if (aci == null) { + throw IllegalArgumentException("aciSession = true but aci is null!") + } + + SignalDatabase.sessions.store(aci, SignalProtocolAddress(aci.toString(), 1), SessionRecord()) + } + if (aci != null) { SignalDatabase.identities.saveIdentity( addressName = aci.toString(), diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt index 1e6210cce2..573028056f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt @@ -2611,14 +2611,14 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da ) } - val preSseOperationCount = operations.size + sessionSwitchoverEventIfNeeded(pniVerified, preMergeData.pniRecord, postMergeData.pniRecord)?.let { + breadCrumbs += "FinalUpdateSSEPniRecord" + operations += it + } - operations += sessionSwitchoverEventIfNeeded(pniVerified, preMergeData.e164Record, postMergeData.e164Record) - operations += sessionSwitchoverEventIfNeeded(pniVerified, preMergeData.pniRecord, postMergeData.pniRecord) - operations += sessionSwitchoverEventIfNeeded(pniVerified, preMergeData.aciRecord, postMergeData.aciRecord) - - if (operations.size > preSseOperationCount) { - breadCrumbs += "FinalUpdateSSE" + sessionSwitchoverEventIfNeeded(pniVerified, preMergeData.aciRecord, postMergeData.aciRecord)?.let { + breadCrumbs += "FinalUpdateSSEPniAciRecord" + operations += it } return PnpChangeSet( @@ -2646,16 +2646,14 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da * For details on SSE's, see [needsSessionSwitchoverEvent]. This method is just a helper around comparing service ID's from two * records and turning it into a possible event. */ - private fun sessionSwitchoverEventIfNeeded(pniVerified: Boolean, oldRecord: RecipientRecord?, newRecord: RecipientRecord?): List { + private fun sessionSwitchoverEventIfNeeded(pniVerified: Boolean, oldRecord: RecipientRecord?, newRecord: RecipientRecord?): PnpOperation? { return if (oldRecord != null && newRecord != null && needsSessionSwitchoverEvent(pniVerified, oldRecord.serviceId, newRecord.serviceId)) { - listOf( - PnpOperation.SessionSwitchoverInsert( - recipientId = newRecord.id, - e164 = newRecord.e164 - ) + PnpOperation.SessionSwitchoverInsert( + recipientId = newRecord.id, + e164 = newRecord.e164 ) } else { - emptyList() + null } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/model/RecipientRecord.kt b/app/src/main/java/org/thoughtcrime/securesms/database/model/RecipientRecord.kt index 0446bad3c1..e279f3479b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/model/RecipientRecord.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/model/RecipientRecord.kt @@ -80,7 +80,7 @@ data class RecipientRecord( ) { fun e164Only(): Boolean { - return this.e164 != null && this.aci == null + return this.e164 != null && this.aci == null && this.pni == null } fun pniOnly(): Boolean {