Allow PNI-only contact inserts.

signalapp · Aug 17, 2023 · 74d5faf · 74d5faf
1 parent 15204a2
commit 74d5faf
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 26 deletions.
diff --git a/...idTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt b/...idTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt
@@ -60,25 +60,6 @@ class RecipientTableTest_getAndPossiblyMerge {
     FeatureFlagsAccessor.forceValue(FeatureFlags.PHONE_NUMBER_PRIVACY, true)
   }
 
-  @Test
-  fun single() {
-    test("local user, local e164+aci provided, changeSelf=false, leave pni alone") {
-      given(E164_SELF, PNI_SELF, ACI_SELF)
-
-      process(E164_SELF, PNI_A, ACI_A)
-
-      expect(E164_SELF, PNI_SELF, ACI_SELF)
-    }
-
-    test("local user, local e164+aci provided, changeSelf=false, leave pni alone") {
-      given(E164_SELF, PNI_A, ACI_SELF)
-
-      process(E164_SELF, PNI_SELF, ACI_A)
-
-      expect(E164_SELF, PNI_A, ACI_SELF)
-    }
-  }
-
   @Test
   fun allNonMergeTests() {
     test("e164-only insert") {
@@ -89,7 +70,7 @@ class RecipientTableTest_getAndPossiblyMerge {
       assertEquals(RecipientTable.RegisteredState.UNKNOWN, record.registered)
     }
 
-    test("pni-only insert", exception = IllegalArgumentException::class.java) {
+    test("pni-only insert") {
       val id = process(null, PNI_A, null)
       expect(null, PNI_A, null)
 
@@ -137,16 +118,21 @@ class RecipientTableTest_getAndPossiblyMerge {
       expect(E164_A, null, null)
     }
 
-    test("no match, e164 and pni") {
-      process(E164_A, PNI_A, null)
-      expect(E164_A, PNI_A, null)
+    test("no match, pni-only") {
+      process(null, PNI_A, null)
+      expect(null, PNI_A, null)
     }
 
     test("no match, aci-only") {
       process(null, null, ACI_A)
       expect(null, null, ACI_A)
     }
 
+    test("no match, e164 and pni") {
+      process(E164_A, PNI_A, null)
+      expect(E164_A, PNI_A, null)
+    }
+
     test("no match, e164 and aci") {
       process(E164_A, null, ACI_A)
       expect(E164_A, null, ACI_A)
@@ -713,6 +699,22 @@ class RecipientTableTest_getAndPossiblyMerge {
       process(E164_A, null, ACI_SELF, changeSelf = true)
       expect(E164_A, null, ACI_SELF)
     }
+
+    test("local user, local e164+aci provided, changeSelf=false, leave pni alone") {
+      given(E164_SELF, PNI_SELF, ACI_SELF)
+
+      process(E164_SELF, PNI_A, ACI_A)
+
+      expect(E164_SELF, PNI_SELF, ACI_SELF)
+    }
+
+    test("local user, local e164+aci provided, changeSelf=false, leave pni alone") {
+      given(E164_SELF, PNI_A, ACI_SELF)
+
+      process(E164_SELF, PNI_SELF, ACI_A)
+
+      expect(E164_SELF, PNI_A, ACI_SELF)
+    }
   }
 
   /**

diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt
@@ -445,7 +445,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
 
   @VisibleForTesting
   fun getAndPossiblyMerge(aci: ACI?, pni: PNI?, e164: String?, pniVerified: Boolean = false, changeSelf: Boolean = false): RecipientId {
-    require(aci != null || e164 != null) { "Must provide an ACI or E164!" }
+    require(aci != null || pni != null || e164 != null) { "Must provide an ACI, PNI, or E164!" }
 
     val db = writableDatabase
     var transactionSuccessful = false

diff --git a/...rvice/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java b/...rvice/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java
@@ -42,8 +42,8 @@ public SignalContactRecord(StorageId id, ContactRecord proto) {
     this.id                = id;
     this.proto             = proto;
     this.hasUnknownFields  = ProtoUtil.hasUnknownFields(proto);
-    this.aci               = OptionalUtil.absentIfEmpty(proto.getAci()).map(ACI::parseOrNull);
-    this.pni               = OptionalUtil.absentIfEmpty(proto.getPni()).map(PNI::parseOrNull);
+    this.aci               = OptionalUtil.absentIfEmpty(proto.getAci()).map(ACI::parseOrNull).map(it -> it.isUnknown() ? null : it);
+    this.pni               = OptionalUtil.absentIfEmpty(proto.getPni()).map(PNI::parseOrNull).map(it -> it.isUnknown() ? null : it);
     this.e164              = OptionalUtil.absentIfEmpty(proto.getE164());
     this.profileGivenName  = OptionalUtil.absentIfEmpty(proto.getGivenName());
     this.profileFamilyName = OptionalUtil.absentIfEmpty(proto.getFamilyName());