From ab155dd5bdb190589a58462439de1e8153f75035 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20K=C3=BCrten?= <sebastian@mobanisto.de>
Date: Fri, 5 Nov 2021 13:29:57 +0100
Subject: [PATCH] Add shortcut methods Database#read() and #write()

---
 .../main/java/org/briarproject/mailbox/cli/Main.kt   |  4 ++--
 .../briarproject/mailbox/core/db/DatabaseModule.kt   |  5 +++++
 .../org/briarproject/mailbox/core/db/JdbcDatabase.kt | 12 ++++++++++--
 .../mailbox/core/db/TransactionManager.kt            | 12 ++++++++++++
 .../mailbox/core/settings/SettingsManagerImpl.kt     |  2 +-
 .../briarproject/mailbox/core/setup/QrCodeEncoder.kt |  4 ++--
 .../briarproject/mailbox/core/setup/WipeManager.kt   |  2 +-
 .../core/contacts/ContactsManagerIntegrationTest.kt  |  4 ++--
 .../ContactsManagerMalformedInputIntegrationTest.kt  |  6 +++---
 .../briarproject/mailbox/core/db/JdbcDatabaseTest.kt |  8 ++++----
 .../mailbox/core/server/IntegrationTest.kt           |  2 +-
 .../mailbox/core/setup/SetupManagerTest.kt           | 10 +++++-----
 12 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/mailbox-cli/src/main/java/org/briarproject/mailbox/cli/Main.kt b/mailbox-cli/src/main/java/org/briarproject/mailbox/cli/Main.kt
index cb2abda0..4faab226 100644
--- a/mailbox-cli/src/main/java/org/briarproject/mailbox/cli/Main.kt
+++ b/mailbox-cli/src/main/java/org/briarproject/mailbox/cli/Main.kt
@@ -8,7 +8,7 @@ import com.github.ajalt.clikt.parameters.options.flag
 import com.github.ajalt.clikt.parameters.options.option
 import org.briarproject.mailbox.core.CoreEagerSingletons
 import org.briarproject.mailbox.core.JavaCliEagerSingletons
-import org.briarproject.mailbox.core.db.Database
+import org.briarproject.mailbox.core.db.TransactionManager
 import org.briarproject.mailbox.core.lifecycle.LifecycleManager
 import org.briarproject.mailbox.core.setup.QrCodeEncoder
 import org.briarproject.mailbox.core.setup.SetupManager
@@ -42,7 +42,7 @@ class Main : CliktCommand(
     internal lateinit var lifecycleManager: LifecycleManager
 
     @Inject
-    internal lateinit var db: Database
+    internal lateinit var db: TransactionManager
 
     @Inject
     internal lateinit var setupManager: SetupManager
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/DatabaseModule.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/DatabaseModule.kt
index e32d0fc7..31cf5230 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/DatabaseModule.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/DatabaseModule.kt
@@ -17,4 +17,9 @@ internal class DatabaseModule {
         return H2Database(config, clock)
     }
 
+    @Provides
+    fun provideTransactionManager(db: Database): TransactionManager {
+        return db
+    }
+
 }
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/JdbcDatabase.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/JdbcDatabase.kt
index 31c5a8a3..1aad7a74 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/JdbcDatabase.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/JdbcDatabase.kt
@@ -76,7 +76,7 @@ abstract class JdbcDatabase(private val dbTypes: DatabaseTypes, private val cloc
         }
         // Open the database and create the tables and indexes if necessary
         var compact = false
-        transaction(false) { txn ->
+        write { txn ->
             val connection = txn.unbox()
             compact = if (reopen) {
                 val s: Settings = getSettings(connection, DB_SETTINGS_NAMESPACE)
@@ -101,7 +101,7 @@ abstract class JdbcDatabase(private val dbTypes: DatabaseTypes, private val cloc
             logDuration(LOG, { "Compacting database" }, start)
             // Allow the next transaction to reopen the DB
             synchronized(connectionsLock) { closed = false }
-            transaction(false) { txn ->
+            write { txn ->
                 storeLastCompacted(txn.unbox())
             }
         }
@@ -604,6 +604,14 @@ abstract class JdbcDatabase(private val dbTypes: DatabaseTypes, private val cloc
         }
     }
 
+    override fun read(task: (Transaction) -> Unit) {
+        transaction(true, task)
+    }
+
+    override fun write(task: (Transaction) -> Unit) {
+        transaction(false, task)
+    }
+
     override fun transaction(readOnly: Boolean, task: (Transaction) -> Unit) {
         val txn = startTransaction(readOnly)
         try {
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/TransactionManager.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/TransactionManager.kt
index d663509b..edb9d825 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/TransactionManager.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/TransactionManager.kt
@@ -2,6 +2,18 @@ package org.briarproject.mailbox.core.db
 
 interface TransactionManager {
 
+    /**
+     * Runs the given task within a read-only transaction.
+     */
+    @Throws(DbException::class)
+    fun read(task: (Transaction) -> Unit)
+
+    /**
+     * Runs the given task within a read/write transaction.
+     */
+    @Throws(DbException::class)
+    fun write(task: (Transaction) -> Unit)
+
     /**
      * Runs the given task within a transaction.
      */
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/settings/SettingsManagerImpl.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/settings/SettingsManagerImpl.kt
index c1ccc667..0daee275 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/settings/SettingsManagerImpl.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/settings/SettingsManagerImpl.kt
@@ -23,6 +23,6 @@ internal class SettingsManagerImpl @Inject constructor(private val db: Database)
 
     @Throws(DbException::class)
     override fun mergeSettings(s: Settings, namespace: String) {
-        db.transaction(false) { txn -> db.mergeSettings(txn, s, namespace) }
+        db.write { txn -> db.mergeSettings(txn, s, namespace) }
     }
 }
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/setup/QrCodeEncoder.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/setup/QrCodeEncoder.kt
index 334482d3..94647cc3 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/setup/QrCodeEncoder.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/setup/QrCodeEncoder.kt
@@ -4,8 +4,8 @@ import com.google.zxing.BarcodeFormat.QR_CODE
 import com.google.zxing.common.BitMatrix
 import com.google.zxing.qrcode.QRCodeWriter
 import dev.keiji.util.Base32
-import org.briarproject.mailbox.core.db.Database
 import org.briarproject.mailbox.core.db.DbException
+import org.briarproject.mailbox.core.db.TransactionManager
 import org.briarproject.mailbox.core.tor.TorPlugin
 import org.briarproject.mailbox.core.util.LogUtils.logException
 import org.briarproject.mailbox.core.util.StringUtils.fromHexString
@@ -18,7 +18,7 @@ private const val VERSION = 32
 private val LOG = getLogger(QrCodeEncoder::class.java)
 
 class QrCodeEncoder @Inject constructor(
-    private val db: Database,
+    private val db: TransactionManager,
     private val setupManager: SetupManager,
     private val torPlugin: TorPlugin,
 ) {
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/setup/WipeManager.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/setup/WipeManager.kt
index 3da9b2e0..c87f4ca2 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/setup/WipeManager.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/setup/WipeManager.kt
@@ -26,7 +26,7 @@ class WipeManager @Inject constructor(
         val principal = call.principal<MailboxPrincipal>()
         if (principal !is MailboxPrincipal.OwnerPrincipal) throw AuthException()
 
-        db.transaction(false) { txn ->
+        db.write { txn ->
             db.clearDatabase(txn)
         }
         fileManager.deleteAllFiles()
diff --git a/mailbox-core/src/test/java/org/briarproject/mailbox/core/contacts/ContactsManagerIntegrationTest.kt b/mailbox-core/src/test/java/org/briarproject/mailbox/core/contacts/ContactsManagerIntegrationTest.kt
index 767df759..bb11417d 100644
--- a/mailbox-core/src/test/java/org/briarproject/mailbox/core/contacts/ContactsManagerIntegrationTest.kt
+++ b/mailbox-core/src/test/java/org/briarproject/mailbox/core/contacts/ContactsManagerIntegrationTest.kt
@@ -35,7 +35,7 @@ class ContactsManagerIntegrationTest : IntegrationTest() {
 
     @AfterEach
     fun clearDb() {
-        db.transaction(false) { txn ->
+        db.write { txn ->
             db.clearDatabase(txn)
         }
     }
@@ -124,7 +124,7 @@ class ContactsManagerIntegrationTest : IntegrationTest() {
         }
         assertJson("""{ "contacts": [ 1, 2, 3 ] }""", response2)
 
-        db.transaction(true) { txn ->
+        db.read { txn ->
             assertEquals(c1, db.getContact(txn, 1))
             assertEquals(c2, db.getContact(txn, 2))
             assertEquals(c3, db.getContact(txn, 3))
diff --git a/mailbox-core/src/test/java/org/briarproject/mailbox/core/contacts/ContactsManagerMalformedInputIntegrationTest.kt b/mailbox-core/src/test/java/org/briarproject/mailbox/core/contacts/ContactsManagerMalformedInputIntegrationTest.kt
index e538c35e..677fe040 100644
--- a/mailbox-core/src/test/java/org/briarproject/mailbox/core/contacts/ContactsManagerMalformedInputIntegrationTest.kt
+++ b/mailbox-core/src/test/java/org/briarproject/mailbox/core/contacts/ContactsManagerMalformedInputIntegrationTest.kt
@@ -27,7 +27,7 @@ class ContactsManagerMalformedInputIntegrationTest : IntegrationTest(false) {
     @AfterEach
     fun clearDb() {
         val db = testComponent.getDatabase()
-        db.transaction(false) { txn ->
+        db.write { txn ->
             db.clearDatabase(txn)
         }
     }
@@ -62,7 +62,7 @@ class ContactsManagerMalformedInputIntegrationTest : IntegrationTest(false) {
         TestUtils.assertJson("""{ "contacts": [ 1, 2, 3 ] }""", response2)
 
         val db = testComponent.getDatabase()
-        db.transaction(true) { txn ->
+        db.read { txn ->
             assertEquals(c1, db.getContact(txn, 1))
             assertEquals(c2, db.getContact(txn, 2))
             assertEquals(c3, db.getContact(txn, 3))
@@ -220,7 +220,7 @@ class ContactsManagerMalformedInputIntegrationTest : IntegrationTest(false) {
         TestUtils.assertJson("""{ "contacts": [ 1, 2 ] }""", response2)
 
         val db = testComponent.getDatabase()
-        db.transaction(true) { txn ->
+        db.read { txn ->
             assertEquals(c1, db.getContact(txn, 1))
             assertEquals(c2, db.getContact(txn, 2))
         }
diff --git a/mailbox-core/src/test/java/org/briarproject/mailbox/core/db/JdbcDatabaseTest.kt b/mailbox-core/src/test/java/org/briarproject/mailbox/core/db/JdbcDatabaseTest.kt
index 67b41d18..d24afe2d 100644
--- a/mailbox-core/src/test/java/org/briarproject/mailbox/core/db/JdbcDatabaseTest.kt
+++ b/mailbox-core/src/test/java/org/briarproject/mailbox/core/db/JdbcDatabaseTest.kt
@@ -52,7 +52,7 @@ abstract class JdbcDatabaseTest {
             outboxId = randomIdManager.getNewRandomId()
         )
         var db: Database = open(false)
-        db.transaction(false) { txn ->
+        db.write { txn ->
 
             db.addContact(txn, contact1)
             db.addContact(txn, contact2)
@@ -61,7 +61,7 @@ abstract class JdbcDatabaseTest {
 
         // Check that the records are still there
         db = open(true)
-        db.transaction(false) { txn ->
+        db.write { txn ->
             val contact1Reloaded1 = db.getContact(txn, 1)
             val contact2Reloaded1 = db.getContact(txn, 2)
             assertEquals(contact1, contact1Reloaded1)
@@ -77,7 +77,7 @@ abstract class JdbcDatabaseTest {
 
         // Check that the record is gone
         db = open(true)
-        db.transaction(true) { txn ->
+        db.read { txn ->
             val contact1Reloaded2 = db.getContact(txn, 1)
             val contact2Reloaded2 = db.getContact(txn, 2)
             assertNull(contact1Reloaded2)
@@ -99,7 +99,7 @@ abstract class JdbcDatabaseTest {
         merged["baz"] = "qux"
 
         val db: Database = open(false)
-        db.transaction(false) { txn ->
+        db.write { txn ->
             // store 'before'
             db.mergeSettings(txn, before, "namespace")
             assertEquals(before, db.getSettings(txn, "namespace"))
diff --git a/mailbox-core/src/test/java/org/briarproject/mailbox/core/server/IntegrationTest.kt b/mailbox-core/src/test/java/org/briarproject/mailbox/core/server/IntegrationTest.kt
index 238b39da..54752fac 100644
--- a/mailbox-core/src/test/java/org/briarproject/mailbox/core/server/IntegrationTest.kt
+++ b/mailbox-core/src/test/java/org/briarproject/mailbox/core/server/IntegrationTest.kt
@@ -62,7 +62,7 @@ abstract class IntegrationTest(private val installJsonFeature: Boolean = true) {
 
     protected fun addContact(c: Contact) {
         val db = testComponent.getDatabase()
-        db.transaction(false) { txn ->
+        db.write { txn ->
             db.addContact(txn, c)
         }
     }
diff --git a/mailbox-core/src/test/java/org/briarproject/mailbox/core/setup/SetupManagerTest.kt b/mailbox-core/src/test/java/org/briarproject/mailbox/core/setup/SetupManagerTest.kt
index 9ee9c88e..aa4ad01e 100644
--- a/mailbox-core/src/test/java/org/briarproject/mailbox/core/setup/SetupManagerTest.kt
+++ b/mailbox-core/src/test/java/org/briarproject/mailbox/core/setup/SetupManagerTest.kt
@@ -25,21 +25,21 @@ class SetupManagerTest : IntegrationTest() {
     @Test
     fun `restarting setup wipes owner token and creates setup token`() {
         // initially, there's no setup and no owner token
-        db.transaction(true) { txn ->
+        db.read { txn ->
             assertNull(setupManager.getSetupToken(txn))
             assertNull(setupManager.getOwnerToken(txn))
         }
 
         // setting an owner token stores it in DB
         setupManager.setToken(null, ownerToken)
-        db.transaction(true) { txn ->
+        db.read { txn ->
             assertNull(setupManager.getSetupToken(txn))
             assertEquals(ownerToken, setupManager.getOwnerToken(txn))
         }
 
         // restarting setup wipes owner token, creates setup token
         setupManager.restartSetup()
-        db.transaction(true) { txn ->
+        db.read { txn ->
             val setupToken = setupManager.getSetupToken(txn)
             assertNotNull(setupToken)
             testComponent.getRandomIdManager().assertIsRandomId(setupToken)
@@ -50,7 +50,7 @@ class SetupManagerTest : IntegrationTest() {
     @Test
     fun `setup request gets rejected when using non-setup token`() = runBlocking {
         // initially, there's no setup and no owner token
-        db.transaction(true) { txn ->
+        db.read { txn ->
             assertNull(setupManager.getSetupToken(txn))
             assertNull(setupManager.getOwnerToken(txn))
         }
@@ -91,7 +91,7 @@ class SetupManagerTest : IntegrationTest() {
             authenticateWithToken(token)
         }
         // setup token got wiped and new owner token from response got stored
-        db.transaction(true) { txn ->
+        db.read { txn ->
             assertNull(setupManager.getSetupToken(txn))
             assertEquals(setupManager.getOwnerToken(txn), response.token)
         }
-- 
GitLab