From f35c16a2af19da63db47d36c291debc44d54c501 Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Mon, 8 Nov 2021 11:50:20 -0300
Subject: [PATCH] Convert more DB access to new TransactionManager
---
.../java/org/briarproject/mailbox/cli/Main.kt | 4 +--
.../mailbox/core/contacts/ContactsManager.kt | 6 ++--
.../mailbox/core/db/JdbcDatabase.kt | 27 ++++++----------
.../mailbox/core/db/TransactionManager.kt | 20 +++---------
.../mailbox/core/files/FileManager.kt | 2 +-
.../mailbox/core/server/AuthManager.kt | 6 ++--
.../core/settings/SettingsManagerImpl.kt | 2 +-
.../mailbox/core/setup/QrCodeEncoder.kt | 2 +-
.../briarproject/mailbox/core/TestUtils.kt | 31 +++++++++++++++----
.../mailbox/core/server/AuthManagerTest.kt | 18 +++++------
.../mailbox/core/setup/WipeManagerTest.kt | 4 +--
11 files changed, 61 insertions(+), 61 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 4faab226..0a00e4f5 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
@@ -82,10 +82,10 @@ class Main : CliktCommand(
lifecycleManager.waitForStartup()
// TODO this is obviously not the final code, just a stub to get us started
- val setupTokenExists = db.transactionWithResult(true) { txn ->
+ val setupTokenExists = db.read { txn ->
setupManager.getSetupToken(txn) != null
}
- val ownerTokenExists = db.transactionWithResult(true) { txn ->
+ val ownerTokenExists = db.read { txn ->
setupManager.getOwnerToken(txn) != null
}
if (!setupTokenExists && !ownerTokenExists) setupManager.restartSetup()
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/contacts/ContactsManager.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/contacts/ContactsManager.kt
index 240a6b5c..e4e9bd42 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/contacts/ContactsManager.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/contacts/ContactsManager.kt
@@ -34,7 +34,7 @@ class ContactsManager @Inject constructor(
suspend fun listContacts(call: ApplicationCall) {
authManager.assertIsOwner(call.principal())
- val contacts = db.transactionWithResult(true) { txn ->
+ val contacts = db.read { txn ->
db.getContacts(txn)
}
val contactIds = contacts.map { contact -> contact.contactId }
@@ -60,7 +60,7 @@ class ContactsManager @Inject constructor(
randomIdManager.assertIsRandomId(c.inboxId)
randomIdManager.assertIsRandomId(c.outboxId)
- val status = db.transactionWithResult(false) { txn ->
+ val status = db.write { txn ->
if (db.getContact(txn, c.contactId) != null) {
Conflict
} else {
@@ -83,7 +83,7 @@ class ContactsManager @Inject constructor(
throw BadRequestException("Invalid value for parameter contactId")
}
- val status = db.transactionWithResult(false) { txn ->
+ val status = db.write { txn ->
if (db.getContact(txn, contactId) == null) {
NotFound
} else {
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 1aad7a74..fe61611b 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
@@ -18,7 +18,6 @@ import java.sql.PreparedStatement
import java.sql.ResultSet
import java.sql.SQLException
import java.sql.Statement
-import java.util.Arrays
import java.util.LinkedList
import java.util.concurrent.locks.Lock
import java.util.concurrent.locks.ReentrantLock
@@ -149,7 +148,7 @@ abstract class JdbcDatabase(private val dbTypes: DatabaseTypes, private val cloc
@Suppress("MemberVisibilityCanBePrivate") // visible for testing
internal fun getMigrations(): List<Migration<Connection>> {
- return Arrays.asList<Migration<Connection>>(
+ return listOf(
// Migration1_2(dbTypes),
)
}
@@ -604,25 +603,19 @@ abstract class JdbcDatabase(private val dbTypes: DatabaseTypes, private val cloc
}
}
- override fun read(task: (Transaction) -> Unit) {
- transaction(true, task)
+ override fun <R> read(task: (Transaction) -> R): R {
+ return transaction(true, task)
}
- override fun write(task: (Transaction) -> Unit) {
- transaction(false, task)
+ override fun <R> write(task: (Transaction) -> R): R {
+ return transaction(false, task)
}
- override fun transaction(readOnly: Boolean, task: (Transaction) -> Unit) {
- val txn = startTransaction(readOnly)
- try {
- task(txn)
- commitTransaction(txn)
- } finally {
- endTransaction(txn)
- }
- }
-
- override fun <R> transactionWithResult(readOnly: Boolean, task: (Transaction) -> R): R {
+ /**
+ * Runs the given task within a transaction and returns the result of the
+ * task.
+ */
+ private fun <R> transaction(readOnly: Boolean, task: (Transaction) -> R): R {
val txn = startTransaction(readOnly)
try {
val result = task(txn)
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 edb9d825..679457c9 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
@@ -3,27 +3,15 @@ package org.briarproject.mailbox.core.db
interface TransactionManager {
/**
- * Runs the given task within a read-only transaction.
+ * Runs the given task within a read-only transaction and returns its result.
*/
@Throws(DbException::class)
- fun read(task: (Transaction) -> Unit)
+ fun <R> read(task: (Transaction) -> R): R
/**
- * Runs the given task within a read/write transaction.
+ * Runs the given task within a read/write transaction and returns its result.
*/
@Throws(DbException::class)
- fun write(task: (Transaction) -> Unit)
+ fun <R> write(task: (Transaction) -> R): R
- /**
- * Runs the given task within a transaction.
- */
- @Throws(DbException::class)
- fun transaction(readOnly: Boolean, task: (Transaction) -> Unit)
-
- /**
- * Runs the given task within a transaction and returns the result of the
- * task.
- */
- @Throws(DbException::class)
- fun <R> transactionWithResult(readOnly: Boolean, task: (Transaction) -> R): R
}
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/files/FileManager.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/files/FileManager.kt
index f38e3af8..05f5db54 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/files/FileManager.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/files/FileManager.kt
@@ -125,7 +125,7 @@ class FileManager @Inject constructor(
val folderListResponse = withContext(Dispatchers.IO) {
val list = ArrayList<FolderResponse>()
- val contacts = db.transactionWithResult(true) { txn -> db.getContacts(txn) }
+ val contacts = db.read { txn -> db.getContacts(txn) }
contacts.forEach { c ->
val id = c.outboxId
val folder = fileProvider.getFolder(id)
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/AuthManager.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/AuthManager.kt
index 3c830129..0aaf277b 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/AuthManager.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/AuthManager.kt
@@ -24,7 +24,7 @@ class AuthManager @Inject constructor(
*/
fun getPrincipal(token: String): MailboxPrincipal? {
randomIdManager.assertIsRandomId(token)
- return db.transactionWithResult(true) { txn ->
+ return db.read { txn ->
val contact = db.getContactWithToken(txn, token)
when {
contact != null -> ContactPrincipal(contact)
@@ -44,7 +44,7 @@ class AuthManager @Inject constructor(
if (principal == null) throw AuthException()
if (principal is OwnerPrincipal) {
- val contacts = db.transactionWithResult(true) { txn -> db.getContacts(txn) }
+ val contacts = db.read { txn -> db.getContacts(txn) }
val noOutboxFound = contacts.none { c -> folderId == c.outboxId }
if (noOutboxFound) throw AuthException()
} else if (principal is ContactPrincipal) {
@@ -61,7 +61,7 @@ class AuthManager @Inject constructor(
if (principal == null) throw AuthException()
if (principal is OwnerPrincipal) {
- val contacts = db.transactionWithResult(true) { txn -> db.getContacts(txn) }
+ val contacts = db.read { txn -> db.getContacts(txn) }
val noInboxFound = contacts.none { c -> folderId == c.inboxId }
if (noInboxFound) throw AuthException()
} else if (principal is ContactPrincipal) {
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 0daee275..59a199d2 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
@@ -11,7 +11,7 @@ internal class SettingsManagerImpl @Inject constructor(private val db: Database)
@Throws(DbException::class)
override fun getSettings(namespace: String): Settings {
- return db.transactionWithResult(true) { txn ->
+ return db.read { txn ->
db.getSettings(txn, 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 94647cc3..29a47061 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
@@ -62,7 +62,7 @@ class QrCodeEncoder @Inject constructor(
private fun getSetupTokenBytes(): ByteArray? {
val tokenString = try {
- db.transactionWithResult(true) { txn ->
+ db.read { txn ->
setupManager.getSetupToken(txn)
}
} catch (e: DbException) {
diff --git a/mailbox-core/src/test/java/org/briarproject/mailbox/core/TestUtils.kt b/mailbox-core/src/test/java/org/briarproject/mailbox/core/TestUtils.kt
index 6849f8c3..90b367b0 100644
--- a/mailbox-core/src/test/java/org/briarproject/mailbox/core/TestUtils.kt
+++ b/mailbox-core/src/test/java/org/briarproject/mailbox/core/TestUtils.kt
@@ -7,8 +7,8 @@ import io.ktor.http.HttpStatusCode
import io.mockk.every
import io.mockk.mockk
import org.briarproject.mailbox.core.contacts.Contact
-import org.briarproject.mailbox.core.db.Database
import org.briarproject.mailbox.core.db.Transaction
+import org.briarproject.mailbox.core.db.TransactionManager
import org.briarproject.mailbox.core.system.ID_SIZE
import org.briarproject.mailbox.core.system.toHex
import org.briarproject.mailbox.core.util.IoUtils
@@ -33,13 +33,32 @@ object TestUtils {
)
/**
- * Allows you to mock [Database] access happening within a [Transaction] more comfortably.
- * Calls to [Database.transactionWithResult] will be mocked.
+ * Allows you to mock [TransactionManager] access
+ * happening within a [Transaction] more comfortably.
+ * Calls to [TransactionManager.read] will be mocked.
* The given lambda [block] will get captured and invoked.
*/
- fun <T> everyTransactionWithResult(db: Database, readOnly: Boolean, block: (Transaction) -> T) {
- val txn = Transaction(mockk(), readOnly)
- every { db.transactionWithResult<T>(true, captureLambda()) } answers {
+ fun <T> TransactionManager.everyRead(
+ block: (Transaction) -> T,
+ ) {
+ val txn = Transaction(mockk(), true)
+ every { read<T>(captureLambda()) } answers {
+ lambda<(Transaction) -> T>().captured.invoke(txn)
+ }
+ block(txn)
+ }
+
+ /**
+ * Allows you to mock [TransactionManager] access
+ * happening within a [Transaction] more comfortably.
+ * Calls to [TransactionManager.write] will be mocked.
+ * The given lambda [block] will get captured and invoked.
+ */
+ fun <T> TransactionManager.everyWrite(
+ block: (Transaction) -> T,
+ ) {
+ val txn = Transaction(mockk(), true)
+ every { write<T>(captureLambda()) } answers {
lambda<(Transaction) -> T>().captured.invoke(txn)
}
block(txn)
diff --git a/mailbox-core/src/test/java/org/briarproject/mailbox/core/server/AuthManagerTest.kt b/mailbox-core/src/test/java/org/briarproject/mailbox/core/server/AuthManagerTest.kt
index 397b11f8..4501fcf4 100644
--- a/mailbox-core/src/test/java/org/briarproject/mailbox/core/server/AuthManagerTest.kt
+++ b/mailbox-core/src/test/java/org/briarproject/mailbox/core/server/AuthManagerTest.kt
@@ -2,7 +2,7 @@ package org.briarproject.mailbox.core.server
import io.mockk.every
import io.mockk.mockk
-import org.briarproject.mailbox.core.TestUtils.everyTransactionWithResult
+import org.briarproject.mailbox.core.TestUtils.everyRead
import org.briarproject.mailbox.core.TestUtils.getNewRandomContact
import org.briarproject.mailbox.core.TestUtils.getNewRandomId
import org.briarproject.mailbox.core.db.Database
@@ -41,7 +41,7 @@ class AuthManagerTest {
@Test
fun `getPrincipal() returns authenticated contact`() {
- everyTransactionWithResult(db, true) { txn ->
+ db.everyRead { txn ->
every { db.getContactWithToken(txn, id) } returns contactPrincipal.contact
}
assertEquals(contactPrincipal, authManager.getPrincipal(id))
@@ -49,7 +49,7 @@ class AuthManagerTest {
@Test
fun `getPrincipal() returns authenticated owner`() {
- everyTransactionWithResult(db, true) { txn ->
+ db.everyRead { txn ->
every { db.getContactWithToken(txn, id) } returns null
every { setupManager.getOwnerToken(txn) } returns id
}
@@ -59,7 +59,7 @@ class AuthManagerTest {
@Test
fun `getPrincipal() returns null when unauthenticated`() {
- everyTransactionWithResult(db, true) { txn ->
+ db.everyRead { txn ->
every { db.getContactWithToken(txn, id) } returns null
every { setupManager.getOwnerToken(txn) } returns otherId
every { setupManager.getSetupToken(txn) } returns otherId
@@ -70,7 +70,7 @@ class AuthManagerTest {
@Test
fun `getPrincipal() returns SetupPrincipal`() {
- everyTransactionWithResult(db, true) { txn ->
+ db.everyRead { txn ->
every { db.getContactWithToken(txn, id) } returns null
every { setupManager.getOwnerToken(txn) } returns otherId
every { setupManager.getSetupToken(txn) } returns id
@@ -88,7 +88,7 @@ class AuthManagerTest {
@Test
fun `assertCanDownloadFromFolder() throws if owner wants non-existent folder`() {
- everyTransactionWithResult(db, true) { txn ->
+ db.everyRead { txn ->
every { db.getContacts(txn) } returns emptyList()
}
@@ -109,7 +109,7 @@ class AuthManagerTest {
@Test
fun `assertCanDownloadFromFolder() lets owner access contact's outbox folder`() {
- everyTransactionWithResult(db, true) { txn ->
+ db.everyRead { txn ->
every { db.getContacts(txn) } returns listOf(contact, getNewRandomContact())
}
@@ -130,7 +130,7 @@ class AuthManagerTest {
@Test
fun `assertCanPostToFolder() throws if owner wants non-existent folder`() {
- everyTransactionWithResult(db, true) { txn ->
+ db.everyRead { txn ->
every { db.getContacts(txn) } returns emptyList()
}
@@ -151,7 +151,7 @@ class AuthManagerTest {
@Test
fun `assertCanPostToFolder() lets owner access contact's inbox folder`() {
- everyTransactionWithResult(db, true) { txn ->
+ db.everyRead { txn ->
every { db.getContacts(txn) } returns listOf(contact, getNewRandomContact())
}
diff --git a/mailbox-core/src/test/java/org/briarproject/mailbox/core/setup/WipeManagerTest.kt b/mailbox-core/src/test/java/org/briarproject/mailbox/core/setup/WipeManagerTest.kt
index 23fb16a7..a4d847aa 100644
--- a/mailbox-core/src/test/java/org/briarproject/mailbox/core/setup/WipeManagerTest.kt
+++ b/mailbox-core/src/test/java/org/briarproject/mailbox/core/setup/WipeManagerTest.kt
@@ -54,11 +54,11 @@ class WipeManagerTest : IntegrationTest() {
assertEquals(HttpStatusCode.NoContent, response.status)
// no more contacts in DB
- val contacts = db.transactionWithResult(true) { db.getContacts(it) }
+ val contacts = db.read { db.getContacts(it) }
assertEquals(0, contacts.size)
// owner token was cleared as well
- val token = db.transactionWithResult(true) { txn ->
+ val token = db.read { txn ->
testComponent.getSetupManager().getOwnerToken(txn)
}
assertNull(token)
--
GitLab