Add PNP change number insert events and tests.

main
Greyson Parrelli 2022-11-07 19:11:54 -05:00 zatwierdzone przez Cody Henthorne
rodzic 433b5ebc13
commit 739a8e9451
6 zmienionych plików z 77 dodań i 28 usunięć

Wyświetl plik

@ -12,16 +12,22 @@ import org.signal.core.util.requireLong
import org.signal.core.util.requireString
import org.signal.core.util.select
import org.thoughtcrime.securesms.keyvalue.SignalStore
import org.thoughtcrime.securesms.recipients.Recipient
import org.thoughtcrime.securesms.recipients.RecipientId
import org.thoughtcrime.securesms.sms.IncomingEncryptedMessage
import org.thoughtcrime.securesms.sms.IncomingTextMessage
import org.whispersystems.signalservice.api.push.ACI
import org.whispersystems.signalservice.api.push.PNI
import org.whispersystems.signalservice.api.push.ServiceId
import java.util.Optional
import java.util.UUID
@RunWith(AndroidJUnit4::class)
class RecipientDatabaseTest_processPnpTuple {
private lateinit var recipientDatabase: RecipientDatabase
private lateinit var smsDatabase: SmsDatabase
private lateinit var threadDatabase: ThreadDatabase
private val localAci = ACI.from(UUID.randomUUID())
private val localPni = PNI.from(UUID.randomUUID())
@ -29,6 +35,8 @@ class RecipientDatabaseTest_processPnpTuple {
@Before
fun setup() {
recipientDatabase = SignalDatabase.recipients
smsDatabase = SignalDatabase.sms
threadDatabase = SignalDatabase.threads
ensureDbEmpty()
@ -180,11 +188,12 @@ class RecipientDatabaseTest_processPnpTuple {
fun onlyPniMatches_noExistingPniSession_changeNumber() {
// This test, I could go either way. We decide to change the E164 on the existing row rather than create a new one.
// But it's an "unstable E164->PNI mapping" case, which we don't expect, so as long as there's a user-visible impact that should be fine.
// TODO Verify change number
test {
given(E164_B, PNI_A, null)
given(E164_B, PNI_A, null, createThread = true)
process(E164_A, PNI_A, ACI_A)
expect(E164_A, PNI_A, ACI_A)
expectChangeNumberEvent()
}
}
@ -192,21 +201,23 @@ class RecipientDatabaseTest_processPnpTuple {
fun pniAndAciMatches_changeNumber() {
// This test, I could go either way. We decide to change the E164 on the existing row rather than create a new one.
// But it's an "unstable E164->PNI mapping" case, which we don't expect, so as long as there's a user-visible impact that should be fine.
// TODO Verify change number
test {
given(E164_B, PNI_A, ACI_A)
given(E164_B, PNI_A, ACI_A, createThread = true)
process(E164_A, PNI_A, ACI_A)
expect(E164_A, PNI_A, ACI_A)
expectChangeNumberEvent()
}
}
@Test
fun onlyAciMatches_changeNumber() {
// TODO Verify change number
test {
given(E164_B, null, ACI_A)
given(E164_B, null, ACI_A, createThread = true)
process(E164_A, PNI_A, ACI_A)
expect(E164_A, PNI_A, ACI_A)
expectChangeNumberEvent()
}
}
@ -318,29 +329,31 @@ class RecipientDatabaseTest_processPnpTuple {
@Test
fun merge_e164AndPni_e164AndPniAndAci_changeNumber() {
// TODO Verify change number
test {
given(E164_A, PNI_A, null)
given(E164_B, PNI_B, ACI_A)
given(E164_B, PNI_B, ACI_A, createThread = true)
process(E164_A, PNI_A, ACI_A)
expectDeleted()
expect(E164_A, PNI_A, ACI_A)
expectChangeNumberEvent()
}
}
@Test
fun merge_e164AndPni_e164Aci_changeNumber() {
// TODO Verify change number
test {
given(E164_A, PNI_A, null)
given(E164_B, null, ACI_A)
given(E164_B, null, ACI_A, createThread = true)
process(E164_A, PNI_A, ACI_A)
expectDeleted()
expect(E164_A, PNI_A, ACI_A)
expectChangeNumberEvent()
}
}
@ -402,15 +415,23 @@ class RecipientDatabaseTest_processPnpTuple {
private inner class TestCase {
private val generatedIds: LinkedHashSet<RecipientId> = LinkedHashSet()
private var expectCount = 0
private lateinit var outputRecipientId: RecipientId
fun given(e164: String?, pni: PNI?, aci: ACI?) {
generatedIds += insert(e164, pni, aci)
fun given(e164: String?, pni: PNI?, aci: ACI?, createThread: Boolean = false) {
val id = insert(e164, pni, aci)
generatedIds += id
if (createThread) {
// Create a thread and throw a dummy message in it so it doesn't get automatically deleted
threadDatabase.getOrCreateThreadIdFor(Recipient.resolved(id))
smsDatabase.insertMessageInbox(IncomingEncryptedMessage(IncomingTextMessage(id, 1, 0, 0, 0, "", Optional.empty(), 0, false, ""), ""))
}
}
fun process(e164: String?, pni: PNI?, aci: ACI?) {
SignalDatabase.rawDatabase.beginTransaction()
try {
generatedIds += recipientDatabase.processPnpTuple(e164, pni, aci, pniVerified = false).finalId
outputRecipientId = recipientDatabase.processPnpTuple(e164, pni, aci, pniVerified = false).finalId
generatedIds += outputRecipientId
SignalDatabase.rawDatabase.setTransactionSuccessful()
} finally {
SignalDatabase.rawDatabase.endTransaction()
@ -435,6 +456,10 @@ class RecipientDatabaseTest_processPnpTuple {
fun expectDeleted(id: RecipientId) {
assertNull(get(id))
}
fun expectChangeNumberEvent() {
assertEquals(1, smsDatabase.getChangeNumberMessageCount(outputRecipientId))
}
}
private data class IdRecord(

Wyświetl plik

@ -178,7 +178,7 @@ public abstract class MessageDatabase extends Database implements MmsSmsColumns,
public abstract long insertMessageOutbox(@NonNull OutgoingMediaMessage message, long threadId, boolean forceSms, int defaultReceiptStatus, @Nullable SmsDatabase.InsertListener insertListener) throws MmsException;
public abstract void insertProfileNameChangeMessages(@NonNull Recipient recipient, @NonNull String newProfileName, @NonNull String previousProfileName);
public abstract void insertGroupV1MigrationEvents(@NonNull RecipientId recipientId, long threadId, @NonNull GroupMigrationMembershipChange membershipChange);
public abstract void insertNumberChangeMessages(@NonNull Recipient recipient);
public abstract void insertNumberChangeMessages(@NonNull RecipientId recipientId);
public abstract void insertBoostRequestMessage(@NonNull RecipientId recipientId, long threadId);
public abstract void insertThreadMergeEvent(@NonNull RecipientId recipientId, long threadId, @NonNull ThreadMergeEvent event);
public abstract void insertSmsExportMessage(@NonNull RecipientId recipientId, long threadId);

Wyświetl plik

@ -572,7 +572,7 @@ public class MmsDatabase extends MessageDatabase {
}
@Override
public void insertNumberChangeMessages(@NonNull Recipient recipient) {
public void insertNumberChangeMessages(@NonNull RecipientId recipientId) {
throw new UnsupportedOperationException();
}

Wyświetl plik

@ -2445,8 +2445,11 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) :
Log.w(TAG, "Session switchover events aren't implemented yet!")
}
is PnpOperation.ChangeNumberInsert -> {
// TODO [pnp]
Log.w(TAG, "Change number inserts aren't implemented yet!")
if (changeSet.id is PnpIdResolver.PnpNoopId) {
SignalDatabase.sms.insertNumberChangeMessages(changeSet.id.recipientId)
} else {
throw IllegalStateException("There's a change number event on a newly-inserted recipient?")
}
}
}
}
@ -2649,7 +2652,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) :
)
}
if (record.e164 != null && updatedNumber) {
if (record.e164 != null && updatedNumber && notSelf(e164, pni, aci) && !record.isBlocked) {
operations += PnpOperation.ChangeNumberInsert(
recipientId = commonId,
oldE164 = record.e164,
@ -2762,7 +2765,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) :
secondaryId = data.byPniSid
)
if (data.aciSidRecord.e164 != null && data.aciSidRecord.e164 != e164) {
if (data.aciSidRecord.e164 != null && data.aciSidRecord.e164 != e164 && notSelf(e164, pni, aci) && !data.aciSidRecord.isBlocked) {
operations += PnpOperation.ChangeNumberInsert(
recipientId = data.byAciSid,
oldE164 = data.aciSidRecord.e164,
@ -2788,7 +2791,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) :
e164 = e164
)
if (data.aciSidRecord.e164 != null) {
if (data.aciSidRecord.e164 != null && notSelf(e164, pni, aci) && !data.aciSidRecord.isBlocked) {
operations += PnpOperation.ChangeNumberInsert(
recipientId = data.byAciSid,
oldE164 = data.aciSidRecord.e164,
@ -2829,7 +2832,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) :
secondaryId = data.byE164
)
if (data.aciSidRecord.e164 != null && data.aciSidRecord.e164 != e164) {
if (data.aciSidRecord.e164 != null && data.aciSidRecord.e164 != e164 && notSelf(e164, pni, aci) && !data.aciSidRecord.isBlocked) {
operations += PnpOperation.ChangeNumberInsert(
recipientId = data.byAciSid,
oldE164 = data.aciSidRecord.e164,
@ -2854,7 +2857,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) :
secondaryId = data.byE164
)
if (data.aciSidRecord.e164 != null && data.aciSidRecord.e164 != e164) {
if (data.aciSidRecord.e164 != null && data.aciSidRecord.e164 != e164 && notSelf(e164, pni, aci) && !data.aciSidRecord.isBlocked) {
operations += PnpOperation.ChangeNumberInsert(
recipientId = data.byAciSid,
oldE164 = data.aciSidRecord.e164,
@ -2872,7 +2875,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) :
e164 = e164
)
if (data.aciSidRecord.e164 != null && data.aciSidRecord.e164 != e164) {
if (data.aciSidRecord.e164 != null && data.aciSidRecord.e164 != e164 && notSelf(e164, pni, aci) && !data.aciSidRecord.isBlocked) {
operations += PnpOperation.ChangeNumberInsert(
recipientId = data.byAciSid,
oldE164 = data.aciSidRecord.e164,

Wyświetl plik

@ -88,6 +88,7 @@ import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import static org.thoughtcrime.securesms.database.MmsSmsColumns.Types.CHANGE_NUMBER_TYPE;
import static org.thoughtcrime.securesms.database.MmsSmsColumns.Types.GROUP_V2_LEAVE_BITS;
/**
@ -1070,16 +1071,16 @@ public class SmsDatabase extends MessageDatabase {
}
@Override
public void insertNumberChangeMessages(@NonNull Recipient recipient) {
public void insertNumberChangeMessages(@NonNull RecipientId recipientId) {
ThreadDatabase threadDatabase = SignalDatabase.threads();
List<GroupDatabase.GroupRecord> groupRecords = SignalDatabase.groups().getGroupsContainingMember(recipient.getId(), false);
List<GroupDatabase.GroupRecord> groupRecords = SignalDatabase.groups().getGroupsContainingMember(recipientId, false);
List<Long> threadIdsToUpdate = new LinkedList<>();
SQLiteDatabase db = databaseHelper.getSignalWritableDatabase();
db.beginTransaction();
try {
threadIdsToUpdate.add(threadDatabase.getThreadIdFor(recipient.getId()));
threadIdsToUpdate.add(threadDatabase.getThreadIdFor(recipientId));
for (GroupDatabase.GroupRecord groupRecord : groupRecords) {
if (groupRecord.isActive()) {
threadIdsToUpdate.add(threadDatabase.getThreadIdFor(groupRecord.getRecipientId()));
@ -1090,7 +1091,7 @@ public class SmsDatabase extends MessageDatabase {
.filter(Objects::nonNull)
.forEach(threadId -> {
ContentValues values = new ContentValues();
values.put(RECIPIENT_ID, recipient.getId().serialize());
values.put(RECIPIENT_ID, recipientId.serialize());
values.put(ADDRESS_DEVICE_ID, 1);
values.put(DATE_RECEIVED, System.currentTimeMillis());
values.put(DATE_SENT, System.currentTimeMillis());
@ -2018,6 +2019,26 @@ public class SmsDatabase extends MessageDatabase {
}
}
/**
* The number of change number messages in the thread.
* Currently only used for tests.
*/
@VisibleForTesting
int getChangeNumberMessageCount(@NonNull RecipientId recipientId) {
try (Cursor cursor = SQLiteDatabaseExtensionsKt
.select(getReadableDatabase(), "COUNT(*)")
.from(TABLE_NAME)
.where(RECIPIENT_ID + " = ? AND " + TYPE + " = ?", recipientId, CHANGE_NUMBER_TYPE)
.run())
{
if (cursor.moveToFirst()) {
return cursor.getInt(0);
} else {
return 0;
}
}
}
@VisibleForTesting
Optional<InsertResult> collapseJoinRequestEventsIfPossible(long threadId, IncomingGroupUpdateMessage message) {
InsertResult result = null;

Wyświetl plik

@ -32,7 +32,7 @@ class RecipientChangedNumberJob(parameters: Parameters, private val recipientId:
if (!recipient.isBlocked && !recipient.isGroup && !recipient.isSelf) {
Log.i(TAG, "Writing a number change event.")
SignalDatabase.sms.insertNumberChangeMessages(recipient)
SignalDatabase.sms.insertNumberChangeMessages(recipient.id)
} else {
Log.i(TAG, "Number changed but not relevant. blocked: ${recipient.isBlocked} isGroup: ${recipient.isGroup} isSelf: ${recipient.isSelf}")
}