Use the PNP merging function for everything.

This commit is contained in:
Greyson Parrelli
2022-08-09 18:36:04 -04:00
committed by GitHub
parent e83a4692c5
commit f004b72ba2
8 changed files with 430 additions and 212 deletions

View File

@@ -292,6 +292,7 @@ class RecipientDatabaseTest_getAndPossiblyMergeLegacy {
assertEquals(ACI_B, existingRecipient2.requireServiceId())
assertFalse(existingRecipient2.hasE164())
changeNumberListener.waitForJobManager()
assert(changeNumberListener.numberChangeWasEnqueued)
}

View File

@@ -9,7 +9,6 @@ import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotEquals
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Ignore
import org.junit.Test
import org.junit.runner.RunWith
import org.signal.core.util.CursorUtil
@@ -188,7 +187,6 @@ class RecipientDatabaseTest_getAndPossiblyMergePnp {
}
/** We never want to remove the e164 of our own contact entry. Leave the e164 alone. */
@Ignore("Change self isn't implemented yet!")
@Test
fun getAndPossiblyMerge_e164MapsToExistingUserButAciDoesNot_e164BelongsToLocalUser() {
val dataSet = KeyValueDataSet().apply {
@@ -237,16 +235,15 @@ class RecipientDatabaseTest_getAndPossiblyMergePnp {
val existingAciId: RecipientId = recipientDatabase.getAndPossiblyMergePnp(ACI_A, null)
val existingE164Id: RecipientId = recipientDatabase.getAndPossiblyMergePnp(null, E164_A)
val retrievedId: RecipientId = recipientDatabase.getAndPossiblyMergePnp(ACI_A, E164_A)
assertEquals(existingAciId, retrievedId)
val mergedId: RecipientId = recipientDatabase.getAndPossiblyMergePnp(ACI_A, E164_A)
assertEquals(existingAciId, mergedId)
val retrievedRecipient = Recipient.resolved(retrievedId)
val retrievedRecipient = Recipient.resolved(mergedId)
assertEquals(ACI_A, retrievedRecipient.requireServiceId())
assertEquals(E164_A, retrievedRecipient.requireE164())
// TODO: Recipient remapping!
// val existingE164Recipient = Recipient.resolved(existingE164Id)
// assertEquals(retrievedId, existingE164Recipient.id)
val existingE164Recipient = Recipient.resolved(existingE164Id)
assertEquals(mergedId, existingE164Recipient.id)
changeNumberListener.waitForJobManager()
assertFalse(changeNumberListener.numberChangeWasEnqueued)
@@ -268,13 +265,11 @@ class RecipientDatabaseTest_getAndPossiblyMergePnp {
assertEquals(ACI_A, retrievedRecipient.requireServiceId())
assertEquals(E164_A, retrievedRecipient.requireE164())
// TODO: Recipient remapping!
// val existingE164Recipient = Recipient.resolved(existingE164Id)
// assertEquals(retrievedId, existingE164Recipient.id)
val existingE164Recipient = Recipient.resolved(existingE164Id)
assertEquals(retrievedId, existingE164Recipient.id)
// TODO: Change number!
// changeNumberListener.waitForJobManager()
// assert(changeNumberListener.numberChangeWasEnqueued)
changeNumberListener.waitForJobManager()
assert(changeNumberListener.numberChangeWasEnqueued)
}
/** No new rules here, just a more complex scenario to show how different rules interact. */
@@ -297,8 +292,8 @@ class RecipientDatabaseTest_getAndPossiblyMergePnp {
assertEquals(ACI_B, existingRecipient2.requireServiceId())
assertFalse(existingRecipient2.hasE164())
// TODO: Change number!
// assert(changeNumberListener.numberChangeWasEnqueued)
changeNumberListener.waitForJobManager()
assert(changeNumberListener.numberChangeWasEnqueued)
}
/**
@@ -319,13 +314,11 @@ class RecipientDatabaseTest_getAndPossiblyMergePnp {
assertFalse(recipientDatabase.getByE164(E164_B).isPresent)
// TODO: Recipient remapping!
// val recipientWithId2 = Recipient.resolved(existingId2)
// assertEquals(retrievedId, recipientWithId2.id)
val recipientWithId2 = Recipient.resolved(existingId2)
assertEquals(retrievedId, recipientWithId2.id)
}
/** We never want to remove the e164 of our own contact entry. Leave the e164 alone. */
@Ignore("Change self isn't implemented yet!")
@Test
fun getAndPossiblyMerge_bothAciAndE164MapToExistingUser_e164BelongsToLocalUser() {
val dataSet = KeyValueDataSet().apply {
@@ -402,13 +395,11 @@ class RecipientDatabaseTest_getAndPossiblyMergePnp {
assertEquals(ACI_A, retrievedRecipient.requireServiceId())
assertEquals(E164_B, retrievedRecipient.requireE164())
// TODO: Change number!
// changeNumberListener.waitForJobManager()
// assert(changeNumberListener.numberChangeWasEnqueued)
changeNumberListener.waitForJobManager()
assert(changeNumberListener.numberChangeWasEnqueued)
}
/** High trust lets you merge two different users into one. You should prefer the ACI user. Not shown: merging threads, dropping e164 sessions, etc. */
@Ignore("This level of merging isn't implemented yet!")
@Test
fun getAndPossiblyMerge_merge_general() {
// Setup

View File

@@ -16,7 +16,6 @@ import org.thoughtcrime.securesms.recipients.RecipientId
import org.whispersystems.signalservice.api.push.ACI
import org.whispersystems.signalservice.api.push.PNI
import org.whispersystems.signalservice.api.push.ServiceId
import java.lang.IllegalArgumentException
import java.util.UUID
@RunWith(AndroidJUnit4::class)
@@ -61,14 +60,14 @@ class RecipientDatabaseTest_processPnpTuple {
}
}
@Test(expected = IllegalArgumentException::class)
@Test(expected = IllegalStateException::class)
fun noMatch_pniOnly() {
test {
process(null, PNI_A, null)
}
}
@Test(expected = IllegalArgumentException::class)
@Test(expected = IllegalStateException::class)
fun noMatch_noData() {
test {
process(null, null, null)
@@ -416,7 +415,13 @@ class RecipientDatabaseTest_processPnpTuple {
}
fun process(e164: String?, pni: PNI?, aci: ACI?) {
generatedIds += recipientDatabase.processPnpTuple(e164, pni, aci, false)
SignalDatabase.rawDatabase.beginTransaction()
try {
generatedIds += recipientDatabase.processPnpTuple(e164, pni, aci, pniVerified = false, pnpEnabled = true).finalId
SignalDatabase.rawDatabase.setTransactionSuccessful()
} finally {
SignalDatabase.rawDatabase.endTransaction()
}
}
fun expect(e164: String?, pni: PNI?, aci: ACI?) {

View File

@@ -14,7 +14,6 @@ import org.whispersystems.signalservice.api.push.ACI
import org.whispersystems.signalservice.api.push.PNI
import org.whispersystems.signalservice.api.push.ServiceId
import java.lang.AssertionError
import java.lang.IllegalArgumentException
import java.lang.IllegalStateException
import java.util.UUID
@@ -68,12 +67,12 @@ class RecipientDatabaseTest_processPnpTupleToChangeSet {
)
}
@Test(expected = IllegalArgumentException::class)
@Test(expected = IllegalStateException::class)
fun noMatch_pniOnly() {
db.processPnpTupleToChangeSet(null, PNI_A, null, pniVerified = false)
}
@Test(expected = IllegalArgumentException::class)
@Test(expected = IllegalStateException::class)
fun noMatch_noData() {
db.processPnpTupleToChangeSet(null, null, null, pniVerified = false)
}
@@ -603,11 +602,48 @@ class RecipientDatabaseTest_processPnpTupleToChangeSet {
id = PnpIdResolver.PnpNoopId(result.secondId),
operations = listOf(
PnpOperation.RemovePni(result.firstId),
PnpOperation.Update(
PnpOperation.SetPni(
recipientId = result.secondId,
pni = PNI_A,
),
PnpOperation.SetE164(
recipientId = result.secondId,
e164 = E164_A,
)
)
),
result.changeSet
)
}
@Test
fun merge_e164AndPni_aciOnly_e164RecordHasSeparateE164_changeNumber() {
val result = applyAndAssert(
listOf(
Input(E164_B, PNI_A, null),
Input(E164_C, null, ACI_A),
),
Update(E164_A, PNI_A, ACI_A),
Output(E164_A, PNI_A, ACI_A)
)
assertEquals(
PnpChangeSet(
id = PnpIdResolver.PnpNoopId(result.secondId),
operations = listOf(
PnpOperation.RemovePni(result.firstId),
PnpOperation.SetPni(
recipientId = result.secondId,
pni = PNI_A,
aci = ACI_A
),
PnpOperation.SetE164(
recipientId = result.secondId,
e164 = E164_A,
),
PnpOperation.ChangeNumberInsert(
recipientId = result.secondId,
oldE164 = E164_C,
newE164 = E164_A
)
)
),
@@ -806,5 +842,6 @@ class RecipientDatabaseTest_processPnpTupleToChangeSet {
const val E164_A = "+12221234567"
const val E164_B = "+13331234567"
const val E164_C = "+14441234567"
}
}