From 74d5faf3fa89466a8de4d450fdcb42d6b0e66044 Mon Sep 17 00:00:00 2001
From: Greyson Parrelli <greyson@signal.org>
Date: Thu, 17 Aug 2023 09:14:58 -0400
Subject: [PATCH] Allow PNI-only contact inserts.

---
 .../RecipientTableTest_getAndPossiblyMerge.kt | 48 ++++++++++---------
 .../securesms/database/RecipientTable.kt      |  2 +-
 .../api/storage/SignalContactRecord.java      |  4 +-
 3 files changed, 28 insertions(+), 26 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 aab2c0a918..5ff3d965f3 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
@@ -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,9 +118,9 @@ 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") {
@@ -147,6 +128,11 @@ class RecipientTableTest_getAndPossiblyMerge {
       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
index 86b627dde3..a0084d49f6 100644
--- 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/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java
index 8c49eb487e..120553bbc4 100644
--- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java
+++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java
@@ -42,8 +42,8 @@ public final class SignalContactRecord implements SignalRecord {
     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());