Basic implementation of writing a PnpChangeSet to disk.

app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientDatabaseTest_processCdsV2Result.kt

@@ -1,206 +0,0 @@
-package org.thoughtcrime.securesms.database
-
-import androidx.core.content.contentValuesOf
-import androidx.test.ext.junit.runners.AndroidJUnit4
-import org.junit.Assert.assertEquals
-import org.junit.Assert.assertTrue
-import org.junit.Before
-import org.junit.Test
-import org.junit.runner.RunWith
-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.RecipientId
-import org.whispersystems.signalservice.api.push.ACI
-import org.whispersystems.signalservice.api.push.PNI
-import org.whispersystems.signalservice.api.push.ServiceId
-import java.util.UUID
-
-@RunWith(AndroidJUnit4::class)
-class RecipientDatabaseTest_processCdsV2Result {
-
-  private lateinit var recipientDatabase: RecipientDatabase
-
-  private val localAci = ACI.from(UUID.randomUUID())
-  private val localPni = PNI.from(UUID.randomUUID())
-
-  @Before
-  fun setup() {
-    recipientDatabase = SignalDatabase.recipients
-
-    ensureDbEmpty()
-
-    SignalStore.account().setAci(localAci)
-    SignalStore.account().setPni(localPni)
-  }
-
-  @Test
-  fun processCdsV2Result_noMatch() {
-    // Note that we haven't inserted any test data
-
-    val resultId: RecipientId = recipientDatabase.processCdsV2Result(E164_A, PNI_A, ACI_A)
-
-    val record: IdRecord = require(resultId)
-
-    assertEquals(resultId, record.id)
-    assertEquals(E164_A, record.e164)
-    assertEquals(ACI_A, record.sid)
-    assertEquals(PNI_A, record.pni)
-  }
-
-  @Test
-  fun processCdsV2Result_fullMatch() {
-    val inputId: RecipientId = insert(E164_A, PNI_A, ACI_A)
-    val resultId: RecipientId = recipientDatabase.processCdsV2Result(E164_A, PNI_A, ACI_A)
-
-    val record: IdRecord = require(resultId)
-
-    assertEquals(inputId, record.id)
-    assertEquals(E164_A, record.e164)
-    assertEquals(ACI_A, record.sid)
-    assertEquals(PNI_A, record.pni)
-  }
-
-  @Test
-  fun processCdsV2Result_onlyE164Matches() {
-    val inputId: RecipientId = insert(E164_A, null, null)
-    val resultId: RecipientId = recipientDatabase.processCdsV2Result(E164_A, PNI_A, ACI_A)
-
-    val record: IdRecord = require(resultId)
-
-    assertEquals(inputId, record.id)
-    assertEquals(E164_A, record.e164)
-    assertEquals(ACI_A, record.sid)
-    assertEquals(PNI_A, record.pni)
-  }
-
-  @Test
-  fun processCdsV2Result_e164AndPniMatches() {
-    val inputId: RecipientId = insert(E164_A, PNI_A, null)
-    val resultId: RecipientId = recipientDatabase.processCdsV2Result(E164_A, PNI_A, ACI_A)
-
-    val record: IdRecord = require(resultId)
-
-    assertEquals(inputId, record.id)
-    assertEquals(E164_A, record.e164)
-    assertEquals(ACI_A, record.sid)
-    assertEquals(PNI_A, record.pni)
-  }
-
-  @Test
-  fun processCdsV2Result_e164AndAciMatches() {
-    val inputId: RecipientId = insert(E164_A, null, ACI_A)
-    val resultId: RecipientId = recipientDatabase.processCdsV2Result(E164_A, PNI_A, ACI_A)
-
-    val record: IdRecord = require(resultId)
-
-    assertEquals(inputId, record.id)
-    assertEquals(E164_A, record.e164)
-    assertEquals(ACI_A, record.sid)
-    assertEquals(PNI_A, record.pni)
-  }
-
-  @Test
-  fun processCdsV2Result_onlyPniMatches() {
-    val inputId: RecipientId = insert(null, PNI_A, null)
-    val resultId: RecipientId = recipientDatabase.processCdsV2Result(E164_A, PNI_A, ACI_A)
-
-    val record: IdRecord = require(resultId)
-
-    assertEquals(inputId, record.id)
-    assertEquals(E164_A, record.e164)
-    assertEquals(ACI_A, record.sid)
-    assertEquals(PNI_A, record.pni)
-  }
-
-  @Test
-  fun processCdsV2Result_pniAndAciMatches() {
-    val inputId: RecipientId = insert(null, PNI_A, ACI_A)
-    val resultId: RecipientId = recipientDatabase.processCdsV2Result(E164_A, PNI_A, ACI_A)
-
-    val record: IdRecord = require(resultId)
-
-    assertEquals(inputId, record.id)
-    assertEquals(E164_A, record.e164)
-    assertEquals(ACI_A, record.sid)
-    assertEquals(PNI_A, record.pni)
-  }
-
-  @Test
-  fun processCdsV2Result_onlyAciMatches() {
-    val inputId: RecipientId = insert(null, null, ACI_A)
-    val resultId: RecipientId = recipientDatabase.processCdsV2Result(E164_A, PNI_A, ACI_A)
-
-    val record: IdRecord = require(resultId)
-
-    assertEquals(inputId, record.id)
-    assertEquals(E164_A, record.e164)
-    assertEquals(ACI_A, record.sid)
-    assertEquals(PNI_A, record.pni)
-  }
-
-  private fun insert(e164: String?, pni: PNI?, aci: ACI?): RecipientId {
-    val id: Long = SignalDatabase.rawDatabase.insert(
-      RecipientDatabase.TABLE_NAME,
-      null,
-      contentValuesOf(
-        RecipientDatabase.PHONE to e164,
-        RecipientDatabase.SERVICE_ID to (aci ?: pni)?.toString(),
-        RecipientDatabase.PNI_COLUMN to pni?.toString(),
-        RecipientDatabase.REGISTERED to RecipientDatabase.RegisteredState.REGISTERED.id
-      )
-    )
-
-    return RecipientId.from(id)
-  }
-
-  private fun require(id: RecipientId): IdRecord {
-    return get(id)!!
-  }
-
-  private fun get(id: RecipientId): IdRecord? {
-    SignalDatabase.rawDatabase
-      .select(RecipientDatabase.ID, RecipientDatabase.PHONE, RecipientDatabase.SERVICE_ID, RecipientDatabase.PNI_COLUMN)
-      .from(RecipientDatabase.TABLE_NAME)
-      .where("${RecipientDatabase.ID} = ?", id)
-      .run()
-      .use { cursor ->
-        return if (cursor.moveToFirst()) {
-          IdRecord(
-            id = RecipientId.from(cursor.requireLong(RecipientDatabase.ID)),
-            e164 = cursor.requireString(RecipientDatabase.PHONE),
-            sid = ServiceId.parseOrNull(cursor.requireString(RecipientDatabase.SERVICE_ID)),
-            pni = PNI.parseOrNull(cursor.requireString(RecipientDatabase.PNI_COLUMN))
-          )
-        } else {
-          null
-        }
-      }
-  }
-
-  private fun ensureDbEmpty() {
-    SignalDatabase.rawDatabase.rawQuery("SELECT COUNT(*) FROM ${RecipientDatabase.TABLE_NAME} WHERE ${RecipientDatabase.DISTRIBUTION_LIST_ID} IS NULL ", null).use { cursor ->
-      assertTrue(cursor.moveToFirst())
-      assertEquals(0, cursor.getLong(0))
-    }
-  }
-
-  private data class IdRecord(
-    val id: RecipientId,
-    val e164: String?,
-    val sid: ServiceId?,
-    val pni: PNI?,
-  )
-
-  companion object {
-    val ACI_A = ACI.from(UUID.fromString("3436efbe-5a76-47fa-a98a-7e72c948a82e"))
-    val ACI_B = ACI.from(UUID.fromString("8de7f691-0b60-4a68-9cd9-ed2f8453f9ed"))
-
-    val PNI_A = PNI.from(UUID.fromString("154b8d92-c960-4f6c-8385-671ad2ffb999"))
-    val PNI_B = PNI.from(UUID.fromString("ba92b1fb-cd55-40bf-adda-c35a85375533"))
-
-    const val E164_A = "+12221234567"
-    const val E164_B = "+13331234567"
-  }
-}

app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientDatabaseTest_processPnpTuple.kt

@@ -0,0 +1,496 @@
+package org.thoughtcrime.securesms.database
+
+import androidx.core.content.contentValuesOf
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+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.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.lang.IllegalStateException
+import java.util.UUID
+
+@RunWith(AndroidJUnit4::class)
+class RecipientDatabaseTest_processPnpTuple {
+
+  private lateinit var recipientDatabase: RecipientDatabase
+
+  private val localAci = ACI.from(UUID.randomUUID())
+  private val localPni = PNI.from(UUID.randomUUID())
+
+  @Before
+  fun setup() {
+    recipientDatabase = SignalDatabase.recipients
+
+    ensureDbEmpty()
+
+    SignalStore.account().setAci(localAci)
+    SignalStore.account().setPni(localPni)
+  }
+
+  @Test
+  fun noMatch_e164Only() {
+    test {
+      process(E164_A, null, null)
+      expect(E164_A, null, null)
+    }
+  }
+
+  @Test
+  fun noMatch_e164AndPni() {
+    test {
+      process(E164_A, PNI_A, null)
+      expect(E164_A, PNI_A, null)
+    }
+  }
+
+  @Test
+  fun noMatch_aciOnly() {
+    test {
+      process(null, null, ACI_A)
+      expect(null, null, ACI_A)
+    }
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun noMatch_pniOnly() {
+    test {
+      process(null, PNI_A, null)
+    }
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun noMatch_noData() {
+    test {
+      process(null, null, null)
+    }
+  }
+
+  @Test
+  fun noMatch_allFields() {
+    test {
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun fullMatch() {
+    test {
+      given(E164_A, PNI_A, ACI_A)
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun onlyE164Matches() {
+    test {
+      given(E164_A, null, null)
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun e164AndPniMatches() {
+    test {
+      given(E164_A, PNI_A, null)
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun e164AndAciMatches() {
+    test {
+      given(E164_A, null, ACI_A)
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun onlyPniMatches() {
+    test {
+      given(null, PNI_A, null)
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun pniAndAciMatches() {
+    test {
+      given(null, PNI_A, ACI_A)
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun onlyAciMatches() {
+    test {
+      given(null, null, ACI_A)
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun onlyE164Matches_pniChanges_noAciProvided_noPniSession() {
+    test {
+      given(E164_A, PNI_B, null)
+      process(E164_A, PNI_A, null)
+      expect(E164_A, PNI_A, null)
+    }
+  }
+
+  @Test
+  fun e164AndPniMatches_noExistingSession() {
+    test {
+      given(E164_A, PNI_A, null)
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun onlyPniMatches_noExistingSession() {
+    test {
+      given(null, PNI_A, null)
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  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)
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  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)
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun onlyAciMatches_changeNumber() {
+    // TODO Verify change number
+    test {
+      given(E164_B, null, ACI_A)
+      process(E164_A, PNI_A, ACI_A)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun merge_e164Only_pniOnly_aciOnly() {
+    test {
+      given(E164_A, null, null)
+      given(null, PNI_A, null)
+      given(null, null, ACI_A)
+
+      process(E164_A, PNI_A, ACI_A)
+
+      expectDeleted()
+      expectDeleted()
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun merge_e164Only_pniOnly_noAciProvided() {
+    test {
+      given(E164_A, null, null)
+      given(null, PNI_A, null)
+
+      process(E164_A, PNI_A, null)
+
+      expect(E164_A, PNI_A, null)
+      expectDeleted()
+    }
+  }
+
+  @Test
+  fun merge_e164Only_pniOnly_aciProvidedButNoAciRecord() {
+    test {
+      given(E164_A, null, null)
+      given(null, PNI_A, null)
+
+      process(E164_A, PNI_A, ACI_A)
+
+      expect(E164_A, PNI_A, ACI_A)
+      expectDeleted()
+    }
+  }
+
+  @Test
+  fun merge_e164Only_pniAndE164_noAciProvided() {
+    test {
+      given(E164_A, null, null)
+      given(E164_B, PNI_A, null)
+
+      process(E164_A, PNI_A, null)
+
+      expect(E164_A, PNI_A, null)
+      expect(E164_B, null, null)
+    }
+  }
+
+  @Test
+  fun merge_e164AndPni_pniOnly_noAciProvided() {
+    test {
+      given(E164_A, PNI_B, null)
+      given(null, PNI_A, null)
+
+      process(E164_A, PNI_A, null)
+
+      expect(E164_A, PNI_A, null)
+      expectDeleted()
+    }
+  }
+
+  @Test
+  fun merge_e164AndPni_e164AndPni_noAciProvided_noSessions() {
+    test {
+      given(E164_A, PNI_B, null)
+      given(E164_B, PNI_A, null)
+
+      process(E164_A, PNI_A, null)
+
+      expect(E164_A, PNI_A, null)
+      expect(E164_B, null, null)
+    }
+  }
+
+  @Test
+  fun merge_e164AndPni_aciOnly() {
+    test {
+      given(E164_A, PNI_A, null)
+      given(null, null, ACI_A)
+
+      process(E164_A, PNI_A, ACI_A)
+
+      expectDeleted()
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun merge_e164AndPni_aciOnly_e164RecordHasSeparateE164() {
+    test {
+      given(E164_B, PNI_A, null)
+      given(null, null, ACI_A)
+
+      process(E164_A, PNI_A, ACI_A)
+
+      expect(E164_B, null, null)
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun merge_e164AndPni_e164AndPniAndAci_changeNumber() {
+    // TODO Verify change number
+    test {
+      given(E164_A, PNI_A, null)
+      given(E164_B, PNI_B, ACI_A)
+
+      process(E164_A, PNI_A, ACI_A)
+
+      expectDeleted()
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  @Test
+  fun merge_e164AndPni_e164Aci_changeNumber() {
+    // TODO Verify change number
+    test {
+      given(E164_A, PNI_A, null)
+      given(E164_B, null, ACI_A)
+
+      process(E164_A, PNI_A, ACI_A)
+
+      expectDeleted()
+      expect(E164_A, PNI_A, ACI_A)
+    }
+  }
+
+  private fun insert(e164: String?, pni: PNI?, aci: ACI?): RecipientId {
+    val id: Long = SignalDatabase.rawDatabase.insert(
+      RecipientDatabase.TABLE_NAME,
+      null,
+      contentValuesOf(
+        RecipientDatabase.PHONE to e164,
+        RecipientDatabase.SERVICE_ID to (aci ?: pni)?.toString(),
+        RecipientDatabase.PNI_COLUMN to pni?.toString(),
+        RecipientDatabase.REGISTERED to RecipientDatabase.RegisteredState.REGISTERED.id
+      )
+    )
+
+    return RecipientId.from(id)
+  }
+
+  private fun require(id: RecipientId): IdRecord {
+    return get(id)!!
+  }
+
+  private fun get(id: RecipientId): IdRecord? {
+    SignalDatabase.rawDatabase
+      .select(RecipientDatabase.ID, RecipientDatabase.PHONE, RecipientDatabase.SERVICE_ID, RecipientDatabase.PNI_COLUMN)
+      .from(RecipientDatabase.TABLE_NAME)
+      .where("${RecipientDatabase.ID} = ?", id)
+      .run()
+      .use { cursor ->
+        return if (cursor.moveToFirst()) {
+          IdRecord(
+            id = RecipientId.from(cursor.requireLong(RecipientDatabase.ID)),
+            e164 = cursor.requireString(RecipientDatabase.PHONE),
+            sid = ServiceId.parseOrNull(cursor.requireString(RecipientDatabase.SERVICE_ID)),
+            pni = PNI.parseOrNull(cursor.requireString(RecipientDatabase.PNI_COLUMN))
+          )
+        } else {
+          null
+        }
+      }
+  }
+
+  private fun ensureDbEmpty() {
+    SignalDatabase.rawDatabase.rawQuery("SELECT COUNT(*) FROM ${RecipientDatabase.TABLE_NAME} WHERE ${RecipientDatabase.DISTRIBUTION_LIST_ID} IS NULL ", null).use { cursor ->
+      assertTrue(cursor.moveToFirst())
+      assertEquals(0, cursor.getLong(0))
+    }
+  }
+
+  /**
+   * Baby DSL for making tests readable.
+   */
+  private fun test(init: TestCase.() -> Unit): TestCase {
+    val test = TestCase()
+    test.init()
+    return test
+  }
+
+  private inner class TestCase {
+    private val generatedIds: LinkedHashSet<RecipientId> = LinkedHashSet()
+    private var expectCount = 0
+
+    val id: RecipientId
+      get() = if (generatedIds.size == 1) {
+        generatedIds.elementAt(0)
+      } else {
+        throw IllegalStateException()
+      }
+
+    val firstId: RecipientId
+      get() = if (generatedIds.size > 1) {
+        generatedIds.elementAt(0)
+      } else {
+        throw IllegalStateException()
+      }
+
+    val secondId: RecipientId
+      get() = if (generatedIds.size > 1) {
+        generatedIds.elementAt(1)
+      } else {
+        throw IllegalStateException()
+      }
+
+    val thirdId: RecipientId
+      get() = if (generatedIds.size > 1) {
+        generatedIds.elementAt(2)
+      } else {
+        throw IllegalStateException()
+      }
+
+    fun given(e164: String?, pni: PNI?, aci: ACI?) {
+      generatedIds += insert(e164, pni, aci)
+    }
+
+    fun process(e164: String?, pni: PNI?, aci: ACI?) {
+      generatedIds += recipientDatabase.processPnpTuple(e164, pni, aci, false)
+    }
+
+    fun expect(e164: String?, pni: PNI?, aci: ACI?) {
+      expect(generatedIds.elementAt(expectCount++), e164, pni, aci)
+    }
+
+    fun expect(id: RecipientId, e164: String?, pni: PNI?, aci: ACI?) {
+      val record: IdRecord = require(id)
+      assertEquals(e164, record.e164)
+      assertEquals(pni, record.pni)
+      assertEquals(aci ?: pni, record.sid)
+    }
+
+    fun expectDeleted() {
+      expectDeleted(generatedIds.elementAt(expectCount++))
+    }
+
+    fun expectDeleted(id: RecipientId) {
+      assertNull(get(id))
+    }
+  }
+
+  private data class Input(
+    val e164: String?,
+    val pni: PNI?,
+    val aci: ACI?
+  )
+
+  private data class Update(
+    val e164: String?,
+    val pni: PNI?,
+    val aci: ACI?
+  )
+
+  private data class Output(
+    val id: RecipientId,
+    val e164: String?,
+    val pni: PNI?,
+    val aci: ACI?
+  )
+
+  private data class IdRecord(
+    val id: RecipientId,
+    val e164: String?,
+    val sid: ServiceId?,
+    val pni: PNI?,
+  )
+
+  companion object {
+    val ACI_A = ACI.from(UUID.fromString("3436efbe-5a76-47fa-a98a-7e72c948a82e"))
+    val ACI_B = ACI.from(UUID.fromString("8de7f691-0b60-4a68-9cd9-ed2f8453f9ed"))
+
+    val PNI_A = PNI.from(UUID.fromString("154b8d92-c960-4f6c-8385-671ad2ffb999"))
+    val PNI_B = PNI.from(UUID.fromString("ba92b1fb-cd55-40bf-adda-c35a85375533"))
+
+    const val E164_A = "+12221234567"
+    const val E164_B = "+13331234567"
+  }
+}

app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt

@@ -14,6 +14,7 @@ import net.zetetic.database.sqlcipher.SQLiteConstraintException
 import org.signal.core.util.Bitmask
 import org.signal.core.util.CursorUtil
 import org.signal.core.util.SqlUtil
+import org.signal.core.util.delete
 import org.signal.core.util.logging.Log
 import org.signal.core.util.optionalBlob
 import org.signal.core.util.optionalBoolean
@@ -2167,7 +2168,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) :
     db.beginTransaction()
     try {
       for ((e164, result) in mapping) {
-        ids += processCdsV2Result(e164, result.pni, result.aci)
+        ids += processPnpTuple(e164, result.pni, result.aci, false)
       }
 
       db.setTransactionSuccessful()
@@ -2179,20 +2180,24 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) :
   }
 
   @VisibleForTesting
-  fun processCdsV2Result(e164: String, pni: PNI, aci: ACI?): RecipientId {
+  fun processPnpTuple(e164: String?, pni: PNI?, aci: ACI?, pniVerified: Boolean): RecipientId {
-    val result = processPnpTupleToChangeSet(e164, pni, aci, pniVerified = false)
+    val changeSet = processPnpTupleToChangeSet(e164, pni, aci, pniVerified)
+    return writePnpChangeSetToDisk(changeSet)
+  }
 
-    val id: RecipientId = when (result.id) {
+  @VisibleForTesting
+  fun writePnpChangeSetToDisk(changeSet: PnpChangeSet): RecipientId {
+    val id: RecipientId = when (changeSet.id) {
       is PnpIdResolver.PnpNoopId -> {
-        result.id.recipientId
+        changeSet.id.recipientId
       }
       is PnpIdResolver.PnpInsert -> {
-        val id: Long = writableDatabase.insert(TABLE_NAME, null, buildContentValuesForCdsInsert(result.id.e164, result.id.pni, result.id.aci))
+        val id: Long = writableDatabase.insert(TABLE_NAME, null, buildContentValuesForPnpInsert(changeSet.id.e164, changeSet.id.pni, changeSet.id.aci))
         RecipientId.from(id)
       }
     }
 
-    for (operation in result.operations) {
+    for (operation in changeSet.operations) {
       @Exhaustive
       when (operation) {
         is PnpOperation.Update -> {
@@ -2205,37 +2210,81 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) :
             .where("$ID = ?", operation.recipientId)
             .run()
         }
+        is PnpOperation.RemoveE164 -> {
+          writableDatabase
+            .update(TABLE_NAME)
+            .values(PHONE to null)
+            .where("$ID = ?", operation.recipientId)
+            .run()
+        }
+        is PnpOperation.RemovePni -> {
+          writableDatabase
+            .update(TABLE_NAME)
+            .values(SERVICE_ID to null)
+            .where("$ID = ? AND $SERVICE_ID NOT NULL AND $SERVICE_ID = $PNI_COLUMN", operation.recipientId)
+            .run()
+
+          writableDatabase
+            .update(TABLE_NAME)
+            .values(PNI_COLUMN to null)
+            .where("$ID = ?", operation.recipientId)
+            .run()
+        }
+        is PnpOperation.SetAci -> {
+          writableDatabase
+            .update(TABLE_NAME)
+            .values(SERVICE_ID to operation.aci.toString())
+            .where("$ID = ?", operation.recipientId)
+            .run()
+        }
+        is PnpOperation.SetE164 -> {
+          writableDatabase
+            .update(TABLE_NAME)
+            .values(PHONE to operation.e164)
+            .where("$ID = ?", operation.recipientId)
+            .run()
+        }
+        is PnpOperation.SetPni -> {
+          writableDatabase
+            .update(TABLE_NAME)
+            .values(SERVICE_ID to operation.pni.toString())
+            .where("$ID = ? AND ($SERVICE_ID IS NULL OR $SERVICE_ID = $PNI_COLUMN)", operation.recipientId)
+            .run()
+
+          writableDatabase
+            .update(TABLE_NAME)
+            .values(PNI_COLUMN to operation.pni.toString())
+            .where("$ID = ?", operation.recipientId)
+            .run()
+        }
         is PnpOperation.Merge -> {
-          // TODO [pnp]
+          Log.w(TAG, "WARNING: Performing a PNP merge! This operation currently only has a basic implementation only suitable for basic testing!")
-          error("Not yet implemented")
+
+          val primary = getRecord(operation.primaryId)
+          val secondary = getRecord(operation.secondaryId)
+
+          writableDatabase
+            .delete(TABLE_NAME)
+            .where("$ID = ?", operation.secondaryId)
+            .run()
+
+          writableDatabase
+            .update(TABLE_NAME)
+            .values(
+              PHONE to (primary.e164 ?: secondary.e164),
+              PNI_COLUMN to (primary.pni ?: secondary.pni)?.toString(),
+              SERVICE_ID to (primary.serviceId ?: secondary.serviceId)?.toString()
+            )
+            .where("$ID = ?", operation.primaryId)
+            .run()
         }
         is PnpOperation.SessionSwitchoverInsert -> {
           // TODO [pnp]
-          error("Not yet implemented")
+          Log.w(TAG, "Session switchover events aren't implemented yet!")
         }
         is PnpOperation.ChangeNumberInsert -> {
           // TODO [pnp]
-          error("Not yet implemented")
+          Log.w(TAG, "Change number inserts aren't implemented yet!")
-        }
-        is PnpOperation.RemoveE164 -> {
-          // TODO [pnp]
-          error("Not yet implemented")
-        }
-        is PnpOperation.RemovePni -> {
-          // TODO [pnp]
-          error("Not yet implemented")
-        }
-        is PnpOperation.SetAci -> {
-          // TODO [pnp]
-          error("Not yet implemented")
-        }
-        is PnpOperation.SetE164 -> {
-          // TODO [pnp]
-          error("Not yet implemented")
-        }
-        is PnpOperation.SetPni -> {
-          // TODO [pnp]
-          error("Not yet implemented")
         }
       }
     }
@@ -3285,13 +3334,12 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) :
     return values
   }
 
-  private fun buildContentValuesForCdsInsert(e164: String?, pni: PNI?, aci: ACI?): ContentValues {
+  private fun buildContentValuesForPnpInsert(e164: String?, pni: PNI?, aci: ACI?): ContentValues {
-    Preconditions.checkArgument(pni != null || aci != null, "Must provide a serviceId!")
+    check(e164 != null || pni != null || aci != null) { "Must provide some sort of identifier!" }
 
-    val serviceId: ServiceId = aci ?: pni!!
     return contentValuesOf(
       PHONE to e164,
-      SERVICE_ID to serviceId.toString(),
+      SERVICE_ID to (aci ?: pni)?.toString(),
       PNI_COLUMN to pni.toString(),
       REGISTERED to RegisteredState.REGISTERED.id,
       STORAGE_SERVICE_ID to Base64.encodeBytes(StorageSyncHelper.generateKey()),