From 100a6fcb56ef103197ffd777a36aa2eafa06c914 Mon Sep 17 00:00:00 2001 From: josecoll Date: Tue, 22 Jan 2019 22:57:43 +0000 Subject: [PATCH] CORDA-2475 Adjust attachments query logic to return correct results (#4612) * CORDA-2475 Adjust attachments query logic to return correct results sets for signed/unsigned jars. * Updates following PR review feedback by RP. --- .../core/node/services/AttachmentStorage.kt | 22 ++-- .../core/transactions/TransactionBuilder.kt | 4 +- .../transactions/TransactionBuilderTest.kt | 8 +- ...tachmentsClassLoaderStaticContractTests.kt | 5 +- .../persistence/NodeAttachmentService.kt | 16 +-- .../persistence/NodeAttachmentServiceTest.kt | 103 ++++++++++++++++++ .../testing/services/MockAttachmentStorage.kt | 11 +- 7 files changed, 129 insertions(+), 40 deletions(-) diff --git a/core/src/main/kotlin/net/corda/core/node/services/AttachmentStorage.kt b/core/src/main/kotlin/net/corda/core/node/services/AttachmentStorage.kt index b07de8a19c..7060c110c3 100644 --- a/core/src/main/kotlin/net/corda/core/node/services/AttachmentStorage.kt +++ b/core/src/main/kotlin/net/corda/core/node/services/AttachmentStorage.kt @@ -1,10 +1,9 @@ package net.corda.core.node.services -import net.corda.core.CordaInternal import net.corda.core.DoNotImplement import net.corda.core.contracts.Attachment -import net.corda.core.contracts.ContractAttachment import net.corda.core.crypto.SecureHash +import net.corda.core.internal.cordapp.CordappImpl.Companion.DEFAULT_CORDAPP_VERSION import net.corda.core.node.services.vault.AttachmentQueryCriteria import net.corda.core.node.services.vault.AttachmentSort import java.io.IOException @@ -79,22 +78,15 @@ interface AttachmentStorage { } /** - * Find the Attachment Id of the contract attachment with the highest version for a given contract class name - * from trusted upload sources. If both a signed and unsigned attachment exist, prefer the signed one. + * Find the Attachment Id(s) of the contract attachments with the highest version for a given contract class name + * from trusted upload sources. + * Return highest version of both signed and unsigned attachment ids (signed first, unsigned second), otherwise return a + * single signed or unsigned version id, or an empty list if none meet the criteria. * * @param contractClassName The fully qualified name of the contract class. * @param minContractVersion The minimum contract version that should be returned. - * @return the [AttachmentId] of the contract, or null if none meet the criteria. + * @return the [AttachmentId]s of the contract attachments (signed always first in list), or an empty list if none meet the criteria. */ - fun getContractAttachmentWithHighestContractVersion(contractClassName: String, minContractVersion: Int): AttachmentId? - - /** - * Find the Attachment Ids of the contract attachments for a given contract class name - * from trusted upload sources. - * - * @param contractClassName The fully qualified name of the contract class. - * @return the [AttachmentId]s of the contract attachments, or an empty set if none meet the criteria. - */ - fun getContractAttachments(contractClassName: String): Set + fun getLatestContractAttachments(contractClassName: String, minContractVersion: Int = DEFAULT_CORDAPP_VERSION): List } diff --git a/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt b/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt index b1799233e7..c7f548f5a0 100644 --- a/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt +++ b/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt @@ -288,7 +288,7 @@ open class TransactionBuilder( val outputHashConstraints = outputStates?.filter { it.constraint is HashAttachmentConstraint } ?: emptyList() val outputSignatureConstraints = outputStates?.filter { it.constraint is SignatureAttachmentConstraint } ?: emptyList() if (inputsHashConstraints.isNotEmpty() && (outputHashConstraints.isNotEmpty() || outputSignatureConstraints.isNotEmpty())) { - val attachmentIds = services.attachments.getContractAttachments(contractClassName) + val attachmentIds = services.attachments.getLatestContractAttachments(contractClassName) // only switchover if we have both signed and unsigned attachments for the given contract class name if (attachmentIds.isNotEmpty() && attachmentIds.size == 2) { val attachmentsToUse = attachmentIds.map { @@ -465,7 +465,7 @@ open class TransactionBuilder( require(isReference || constraints.none { it is HashAttachmentConstraint }) val minimumRequiredContractClassVersion = stateRefs?.map { services.loadContractAttachment(it).contractVersion }?.max() ?: DEFAULT_CORDAPP_VERSION - return services.attachments.getContractAttachmentWithHighestContractVersion(contractClassName, minimumRequiredContractClassVersion) + return services.attachments.getLatestContractAttachments(contractClassName, minimumRequiredContractClassVersion).firstOrNull() ?: throw MissingContractAttachments(states, minimumRequiredContractClassVersion) } diff --git a/core/src/test/kotlin/net/corda/core/transactions/TransactionBuilderTest.kt b/core/src/test/kotlin/net/corda/core/transactions/TransactionBuilderTest.kt index 8106e47b05..25cc6ff18e 100644 --- a/core/src/test/kotlin/net/corda/core/transactions/TransactionBuilderTest.kt +++ b/core/src/test/kotlin/net/corda/core/transactions/TransactionBuilderTest.kt @@ -59,8 +59,8 @@ class TransactionBuilderTest { doReturn(setOf(DummyContract.PROGRAM_ID)).whenever(attachment).allContracts doReturn("app").whenever(attachment).uploader doReturn(emptyList()).whenever(attachment).signerKeys - doReturn(contractAttachmentId).whenever(attachmentStorage) - .getContractAttachmentWithHighestContractVersion("net.corda.testing.contracts.DummyContract", DEFAULT_CORDAPP_VERSION) + doReturn(listOf(contractAttachmentId)).whenever(attachmentStorage) + .getLatestContractAttachments("net.corda.testing.contracts.DummyContract") } @Test @@ -146,8 +146,8 @@ class TransactionBuilderTest { doReturn(attachments).whenever(services).attachments doReturn(signedAttachment).whenever(attachments).openAttachment(contractAttachmentId) - doReturn(contractAttachmentId).whenever(attachments) - .getContractAttachmentWithHighestContractVersion("net.corda.testing.contracts.DummyContract", DEFAULT_CORDAPP_VERSION) + doReturn(listOf(contractAttachmentId)).whenever(attachments) + .getLatestContractAttachments("net.corda.testing.contracts.DummyContract") val outputState = TransactionState(data = DummyState(), contract = DummyContract.PROGRAM_ID, notary = notary) val builder = TransactionBuilder() diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/AttachmentsClassLoaderStaticContractTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/AttachmentsClassLoaderStaticContractTests.kt index a58df52a51..fae909277a 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/AttachmentsClassLoaderStaticContractTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/AttachmentsClassLoaderStaticContractTests.kt @@ -9,7 +9,6 @@ import net.corda.core.crypto.SecureHash import net.corda.core.identity.AbstractParty import net.corda.core.identity.CordaX500Name import net.corda.core.identity.Party -import net.corda.core.internal.cordapp.CordappImpl.Companion.DEFAULT_CORDAPP_VERSION import net.corda.core.node.ServicesForResolution import net.corda.core.node.services.AttachmentStorage import net.corda.core.node.services.NetworkParametersService @@ -90,8 +89,8 @@ class AttachmentsClassLoaderStaticContractTests { doReturn("app").whenever(attachment).uploader doReturn(emptyList()).whenever(attachment).signerKeys val contractAttachmentId = SecureHash.randomSHA256() - doReturn(contractAttachmentId).whenever(attachmentStorage) - .getContractAttachmentWithHighestContractVersion(AttachmentDummyContract.ATTACHMENT_PROGRAM_ID, DEFAULT_CORDAPP_VERSION) + doReturn(listOf(contractAttachmentId)).whenever(attachmentStorage) + .getLatestContractAttachments(AttachmentDummyContract.ATTACHMENT_PROGRAM_ID) } @Test diff --git a/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt b/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt index 84a648245b..62d5367a4e 100644 --- a/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt +++ b/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt @@ -497,14 +497,14 @@ class NodeAttachmentService( return it.key to AttachmentIds(signed.singleOrNull(), unsigned.firstOrNull()) } - override fun getContractAttachmentWithHighestContractVersion(contractClassName: String, minContractVersion: Int): AttachmentId? { + override fun getLatestContractAttachments(contractClassName: String, minContractVersion: Int): List { val versions: NavigableMap = getContractAttachmentVersions(contractClassName) - val newestAttachmentIds = versions.tailMap(minContractVersion, true).lastEntry()?.value - return newestAttachmentIds?.toList()?.first() - } - - override fun getContractAttachments(contractClassName: String): Set { - val versions: NavigableMap = getContractAttachmentVersions(contractClassName) - return versions.values.flatMap { it.toList() }.toSet() + val newestAttachmentIds = versions.tailMap(minContractVersion, true) + val newestSignedAttachment = newestAttachmentIds.values.map { it.signed }.lastOrNull { it != null } + val newestUnsignedAttachment = newestAttachmentIds.values.map { it.unsigned }.lastOrNull { it != null } + return if (newestSignedAttachment != null || newestUnsignedAttachment != null) + AttachmentIds(newestSignedAttachment, newestUnsignedAttachment).toList() + else + emptyList() } } \ No newline at end of file diff --git a/node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentServiceTest.kt b/node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentServiceTest.kt index af32b57b3c..9b61991b27 100644 --- a/node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentServiceTest.kt +++ b/node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentServiceTest.kt @@ -13,6 +13,7 @@ import net.corda.core.flows.FlowLogic import net.corda.core.internal.* import net.corda.core.internal.cordapp.CordappImpl.Companion.DEFAULT_CORDAPP_VERSION import net.corda.core.node.ServicesForResolution +import net.corda.core.node.services.AttachmentId import net.corda.core.node.services.vault.AttachmentQueryCriteria.AttachmentsQueryCriteria import net.corda.core.node.services.vault.AttachmentSort import net.corda.core.node.services.vault.Builder @@ -585,6 +586,108 @@ class NodeAttachmentServiceTest { } } + @Test + fun `retrieve latest versions of unsigned and signed contracts - both exist at same version`() { + SelfCleaningDir().use { file -> + val contractJar = makeTestContractJar(file.path, "com.example.MyContract") + val (signedContractJar, publicKey) = makeTestSignedContractJar(file.path, "com.example.MyContract") + val contractJarV2 = makeTestContractJar(file.path,"com.example.MyContract", version = 2) + val (signedContractJarV2, _) = makeTestSignedContractJar(file.path,"com.example.MyContract", version = 2) + + contractJar.read { storage.privilegedImportAttachment(it, "app", "contract.jar") } + signedContractJar.read { storage.privilegedImportAttachment(it, "app", "contract-signed.jar") } + var attachmentIdV2Unsigned: AttachmentId? = null + contractJarV2.read { attachmentIdV2Unsigned = storage.privilegedImportAttachment(it, "app", "contract-V2.jar") } + var attachmentIdV2Signed: AttachmentId? = null + signedContractJarV2.read { attachmentIdV2Signed = storage.privilegedImportAttachment(it, "app", "contract-signed-V2.jar") } + + val latestAttachments = storage.getLatestContractAttachments("com.example.MyContract") + assertEquals(2, latestAttachments.size) + assertEquals(attachmentIdV2Signed, latestAttachments[0]) + assertEquals(attachmentIdV2Unsigned, latestAttachments[1]) + } + } + + @Test + fun `retrieve latest versions of unsigned and signed contracts - signed is later version than unsigned`() { + SelfCleaningDir().use { file -> + val contractJar = makeTestContractJar(file.path, "com.example.MyContract") + val (signedContractJar, publicKey) = makeTestSignedContractJar(file.path, "com.example.MyContract") + val contractJarV2 = makeTestContractJar(file.path,"com.example.MyContract", version = 2) + + contractJar.read { storage.privilegedImportAttachment(it, "app", "contract.jar") } + var attachmentIdV1Signed: AttachmentId? = null + signedContractJar.read { attachmentIdV1Signed = storage.privilegedImportAttachment(it, "app", "contract-signed.jar") } + var attachmentIdV2Unsigned: AttachmentId? = null + contractJarV2.read { attachmentIdV2Unsigned = storage.privilegedImportAttachment(it, "app", "contract-V2.jar") } + + val latestAttachments = storage.getLatestContractAttachments("com.example.MyContract") + assertEquals(2, latestAttachments.size) + assertEquals(attachmentIdV1Signed, latestAttachments[0]) + assertEquals(attachmentIdV2Unsigned, latestAttachments[1]) + } + } + + @Test + fun `retrieve latest versions of unsigned and signed contracts - unsigned is later version than signed`() { + SelfCleaningDir().use { file -> + val contractJar = makeTestContractJar(file.path, "com.example.MyContract") + val (signedContractJar, publicKey) = makeTestSignedContractJar(file.path, "com.example.MyContract") + val contractJarV2 = makeTestContractJar(file.path,"com.example.MyContract", version = 2) + + contractJar.read { storage.privilegedImportAttachment(it, "app", "contract.jar") } + var attachmentIdV1Signed: AttachmentId? = null + signedContractJar.read { attachmentIdV1Signed = storage.privilegedImportAttachment(it, "app", "contract-signed.jar") } + var attachmentIdV2Unsigned: AttachmentId? = null + contractJarV2.read { attachmentIdV2Unsigned = storage.privilegedImportAttachment(it, "app", "contract-V2.jar") } + + val latestAttachments = storage.getLatestContractAttachments("com.example.MyContract") + assertEquals(2, latestAttachments.size) + assertEquals(attachmentIdV1Signed, latestAttachments[0]) + assertEquals(attachmentIdV2Unsigned, latestAttachments[1]) + } + } + + @Test + fun `retrieve latest versions of unsigned and signed contracts - only signed contracts exist in store`() { + SelfCleaningDir().use { file -> + val (signedContractJar, publicKey) = makeTestSignedContractJar(file.path, "com.example.MyContract") + val (signedContractJarV2, _) = makeTestSignedContractJar(file.path,"com.example.MyContract", version = 2) + + signedContractJar.read { storage.privilegedImportAttachment(it, "app", "contract-signed.jar") } + var attachmentIdV2Signed: AttachmentId? = null + signedContractJarV2.read { attachmentIdV2Signed = storage.privilegedImportAttachment(it, "app", "contract-signed-V2.jar") } + + val latestAttachments = storage.getLatestContractAttachments("com.example.MyContract") + assertEquals(1, latestAttachments.size) + assertEquals(attachmentIdV2Signed, latestAttachments[0]) + } + } + + @Test + fun `retrieve latest versions of unsigned and signed contracts - only unsigned contracts exist in store`() { + SelfCleaningDir().use { file -> + val contractJar = makeTestContractJar(file.path, "com.example.MyContract") + val contractJarV2 = makeTestContractJar(file.path,"com.example.MyContract", version = 2) + + contractJar.read { storage.privilegedImportAttachment(it, "app", "contract.jar") } + var attachmentIdV2Unsigned: AttachmentId? = null + contractJarV2.read { attachmentIdV2Unsigned = storage.privilegedImportAttachment(it, "app", "contract-V2.jar") } + + val latestAttachments = storage.getLatestContractAttachments("com.example.MyContract") + assertEquals(1, latestAttachments.size) + assertEquals(attachmentIdV2Unsigned, latestAttachments[0]) + } + } + + @Test + fun `retrieve latest versions of unsigned and signed contracts - none exist in store`() { + SelfCleaningDir().use { _ -> + val latestAttachments = storage.getLatestContractAttachments("com.example.MyContract") + assertEquals(0, latestAttachments.size) + } + } + // Not the real FetchAttachmentsFlow! private class FetchAttachmentsFlow : FlowLogic() { @Suspendable diff --git a/testing/test-utils/src/main/kotlin/net/corda/testing/services/MockAttachmentStorage.kt b/testing/test-utils/src/main/kotlin/net/corda/testing/services/MockAttachmentStorage.kt index 0f02305427..d7e0b07b0d 100644 --- a/testing/test-utils/src/main/kotlin/net/corda/testing/services/MockAttachmentStorage.kt +++ b/testing/test-utils/src/main/kotlin/net/corda/testing/services/MockAttachmentStorage.kt @@ -44,7 +44,7 @@ class MockAttachmentStorage : AttachmentStorage, SingletonSerializeAsToken() { override fun openAttachment(id: SecureHash): Attachment? = files[id]?.first - override fun queryAttachments(criteria: AttachmentQueryCriteria, sorting: AttachmentSort?): List { + override fun queryAttachments(criteria: AttachmentQueryCriteria, sorting: AttachmentSort?): List { criteria as AttachmentQueryCriteria.AttachmentsQueryCriteria val contractClassNames = if (criteria.contractClassNamesCondition is ColumnPredicate.EqualityComparison) @@ -113,15 +113,10 @@ class MockAttachmentStorage : AttachmentStorage, SingletonSerializeAsToken() { return sha256 } - override fun getContractAttachmentWithHighestContractVersion(contractClassName: String, minContractVersion: Int): AttachmentId? { + override fun getLatestContractAttachments(contractClassName: String, minContractVersion: Int): List { val attachmentQueryCriteria = AttachmentQueryCriteria.AttachmentsQueryCriteria(contractClassNamesCondition = Builder.equal(listOf(contractClassName)), versionCondition = Builder.greaterThanOrEqual(minContractVersion), uploaderCondition = Builder.`in`(TRUSTED_UPLOADERS)) val attachmentSort = AttachmentSort(listOf(AttachmentSort.AttachmentSortColumn(AttachmentSort.AttachmentSortAttribute.VERSION, Sort.Direction.DESC))) - return queryAttachments(attachmentQueryCriteria, attachmentSort).firstOrNull() - } - - override fun getContractAttachments(contractClassName: String): Set { - val attachmentQueryCriteria = AttachmentQueryCriteria.AttachmentsQueryCriteria(contractClassNamesCondition = Builder.equal(listOf(contractClassName))) - return queryAttachments(attachmentQueryCriteria).toSet() + return queryAttachments(attachmentQueryCriteria, attachmentSort) } } \ No newline at end of file