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 4faab22694a160dea955aac55c330d0304e0ecc9..0a00e4f5685d68d48d6489a23e0a6557cfd1d415 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 240a6b5c69107632327c30a7833815c98d569407..e4e9bd422ab16780e127946b14fac02b848788c4 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 1aad7a74557b8834c50506b76a8d3747e62b0bb1..fe61611bcf7ac2f461e05248838b7da3814edbf5 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 edb9d82513ec9adcc1597d3597659e1a23138285..679457c94b193551dc9ec755be7c6cc4b8e77baf 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 f38e3af89595ce62d02de84c6ae9018a65a33755..05f5db54d1958b3da471223b42128ec6fed7e973 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 3c8301296828eb018621f9eb26fc84e646757b83..0aaf277b5c9bbe07ca044abfd33c1f842ba585b1 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 0daee2753cd0478411bbd7d8d7a4bd20875ae20e..59a199d2f3eb50c02ec1edc9614eb6154cbc54ee 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 94647cc3342cbbbe733d907fe8455d8c873bbe4d..29a47061d7a362064edeaea70155ce69c39b4efe 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 6849f8c32a4b343f9474689da3b5d64183639a30..90b367b0fc48784c38397a24d15bb0b30b2adc8b 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 397b11f860268b36d1a928080918db245d66fdc8..4501fcf4ffa037ae38d1bbdac0ca243f5151c1d9 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 23fb16a71573106beaa0761d53702ba86e1d6ffc..a4d847aab563c9efce1f22ead183da3ad3d6b634 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)