From 8aa325d9f7af34906b1ed6c55866fe370942f7b7 Mon Sep 17 00:00:00 2001 From: josecoll Date: Tue, 20 Jun 2017 20:33:31 +0100 Subject: [PATCH] Tighten up transaction creation code to flag situations where calling code SHOULD BE within a demarcated transaction boundary. (#869) Now fail fast with an appropriate message indicating course of action to be taken. --- .../KotlinConfigurationTransactionWrapper.kt | 2 +- .../services/database/RequeryConfiguration.kt | 2 +- .../persistence/NodeAttachmentStorageTest.kt | 100 ++++++++++-------- 3 files changed, 56 insertions(+), 48 deletions(-) diff --git a/node/src/main/kotlin/net/corda/node/services/database/KotlinConfigurationTransactionWrapper.kt b/node/src/main/kotlin/net/corda/node/services/database/KotlinConfigurationTransactionWrapper.kt index a122022a62..e9652e9556 100644 --- a/node/src/main/kotlin/net/corda/node/services/database/KotlinConfigurationTransactionWrapper.kt +++ b/node/src/main/kotlin/net/corda/node/services/database/KotlinConfigurationTransactionWrapper.kt @@ -131,7 +131,7 @@ class KotlinConfigurationTransactionWrapper(private val model: EntityModel, override fun getConnection(): Connection { val tx = TransactionManager.manager.currentOrNull() return CordaConnection( - tx?.connection ?: TransactionManager.manager.newTransaction(Connection.TRANSACTION_REPEATABLE_READ).connection + tx?.connection ?: throw IllegalStateException("Was expecting to find database transaction: must wrap calling code within a transaction.") ) } } diff --git a/node/src/main/kotlin/net/corda/node/services/database/RequeryConfiguration.kt b/node/src/main/kotlin/net/corda/node/services/database/RequeryConfiguration.kt index 908661d837..6bbc6ae0bb 100644 --- a/node/src/main/kotlin/net/corda/node/services/database/RequeryConfiguration.kt +++ b/node/src/main/kotlin/net/corda/node/services/database/RequeryConfiguration.kt @@ -48,7 +48,7 @@ class RequeryConfiguration(val properties: Properties, val useDefaultLogging: Bo // TODO: remove once Requery supports QUERY WITH COMPOSITE_KEY IN fun jdbcSession(): Connection { val ctx = TransactionManager.manager.currentOrNull() - return ctx?.connection ?: TransactionManager.manager.newTransaction(Connection.TRANSACTION_REPEATABLE_READ).connection + return ctx?.connection ?: throw IllegalStateException("Was expecting to find database transaction: must wrap calling code within a transaction.") } } diff --git a/node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentStorageTest.kt b/node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentStorageTest.kt index d951f9d35b..1233f22951 100644 --- a/node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentStorageTest.kt +++ b/node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentStorageTest.kt @@ -14,9 +14,9 @@ import net.corda.node.services.database.RequeryConfiguration import net.corda.node.services.persistence.schemas.AttachmentEntity import net.corda.node.services.transactions.PersistentUniquenessProvider import net.corda.node.utilities.configureDatabase +import net.corda.node.utilities.transaction import net.corda.testing.node.makeTestDataSourceProperties import org.jetbrains.exposed.sql.Database -import org.jetbrains.exposed.sql.transactions.TransactionManager import org.junit.After import org.junit.Before import org.junit.Test @@ -55,7 +55,7 @@ class NodeAttachmentStorageTest { @After fun tearDown() { - TransactionManager.current().close() + dataSource.close() } @Test @@ -63,76 +63,84 @@ class NodeAttachmentStorageTest { val testJar = makeTestJar() val expectedHash = testJar.readAll().sha256() - val storage = NodeAttachmentService(fs.getPath("/"), dataSourceProperties, MetricRegistry()) - val id = testJar.read { storage.importAttachment(it) } - assertEquals(expectedHash, id) + database.transaction { + val storage = NodeAttachmentService(fs.getPath("/"), dataSourceProperties, MetricRegistry()) + val id = testJar.read { storage.importAttachment(it) } + assertEquals(expectedHash, id) - assertNull(storage.openAttachment(SecureHash.randomSHA256())) - val stream = storage.openAttachment(expectedHash)!!.openAsJAR() - val e1 = stream.nextJarEntry!! - assertEquals("test1.txt", e1.name) - assertEquals(stream.readBytes().toString(Charset.defaultCharset()), "This is some useful content") - val e2 = stream.nextJarEntry!! - assertEquals("test2.txt", e2.name) - assertEquals(stream.readBytes().toString(Charset.defaultCharset()), "Some more useful content") + assertNull(storage.openAttachment(SecureHash.randomSHA256())) + val stream = storage.openAttachment(expectedHash)!!.openAsJAR() + val e1 = stream.nextJarEntry!! + assertEquals("test1.txt", e1.name) + assertEquals(stream.readBytes().toString(Charset.defaultCharset()), "This is some useful content") + val e2 = stream.nextJarEntry!! + assertEquals("test2.txt", e2.name) + assertEquals(stream.readBytes().toString(Charset.defaultCharset()), "Some more useful content") - stream.close() + stream.close() - storage.openAttachment(id)!!.openAsJAR().use { - it.nextJarEntry - it.readBytes() + storage.openAttachment(id)!!.openAsJAR().use { + it.nextJarEntry + it.readBytes() + } } } @Test fun `duplicates not allowed`() { val testJar = makeTestJar() - val storage = NodeAttachmentService(fs.getPath("/"), dataSourceProperties, MetricRegistry()) - testJar.read { - storage.importAttachment(it) - } - assertFailsWith { + database.transaction { + val storage = NodeAttachmentService(fs.getPath("/"), dataSourceProperties, MetricRegistry()) testJar.read { storage.importAttachment(it) } + assertFailsWith { + testJar.read { + storage.importAttachment(it) + } + } } } @Test fun `corrupt entry throws exception`() { val testJar = makeTestJar() - val storage = NodeAttachmentService(fs.getPath("/"), dataSourceProperties, MetricRegistry()) - val id = testJar.read { storage.importAttachment(it) } + database.transaction { + val storage = NodeAttachmentService(fs.getPath("/"), dataSourceProperties, MetricRegistry()) + val id = testJar.read { storage.importAttachment(it) } - // Corrupt the file in the store. - val bytes = testJar.readAll() - val corruptBytes = "arggghhhh".toByteArray() - System.arraycopy(corruptBytes, 0, bytes, 0, corruptBytes.size) - val corruptAttachment = AttachmentEntity() - corruptAttachment.attId = id - corruptAttachment.content = bytes - storage.session.update(corruptAttachment) + // Corrupt the file in the store. + val bytes = testJar.readAll() + val corruptBytes = "arggghhhh".toByteArray() + System.arraycopy(corruptBytes, 0, bytes, 0, corruptBytes.size) + val corruptAttachment = AttachmentEntity() + corruptAttachment.attId = id + corruptAttachment.content = bytes + storage.session.update(corruptAttachment) - val e = assertFailsWith { - storage.openAttachment(id)!!.open().use { it.readBytes() } - } - assertEquals(e.expected, id) + val e = assertFailsWith { + storage.openAttachment(id)!!.open().use { it.readBytes() } + } + assertEquals(e.expected, id) - // But if we skip around and read a single entry, no exception is thrown. - storage.openAttachment(id)!!.openAsJAR().use { - it.nextJarEntry - it.readBytes() + // But if we skip around and read a single entry, no exception is thrown. + storage.openAttachment(id)!!.openAsJAR().use { + it.nextJarEntry + it.readBytes() + } } } @Test fun `non jar rejected`() { - val storage = NodeAttachmentService(fs.getPath("/"), dataSourceProperties, MetricRegistry()) - val path = fs.getPath("notajar") - path.writeLines(listOf("Hey", "there!")) - path.read { - assertFailsWith("either empty or not a JAR") { - storage.importAttachment(it) + database.transaction { + val storage = NodeAttachmentService(fs.getPath("/"), dataSourceProperties, MetricRegistry()) + val path = fs.getPath("notajar") + path.writeLines(listOf("Hey", "there!")) + path.read { + assertFailsWith("either empty or not a JAR") { + storage.importAttachment(it) + } } } }