From 350066d386ee6ec710f97aacb23b38aea7c5ff7e Mon Sep 17 00:00:00 2001 From: nikinagy <61757742+nikinagy@users.noreply.github.com> Date: Mon, 11 May 2020 15:41:50 +0100 Subject: [PATCH 1/4] fix for handling empty lists in vault query (#6231) --- .../core/node/services/vault/QueryCriteria.kt | 8 +- .../vault/HibernateQueryCriteriaParser.kt | 100 +++++++++++++++--- .../persistence/NodeAttachmentServiceTest.kt | 18 ++++ .../node/services/vault/VaultQueryTests.kt | 92 ++++++++++++++++ 4 files changed, 205 insertions(+), 13 deletions(-) diff --git a/core/src/main/kotlin/net/corda/core/node/services/vault/QueryCriteria.kt b/core/src/main/kotlin/net/corda/core/node/services/vault/QueryCriteria.kt index 91bfe9bc0d..3f855b1936 100644 --- a/core/src/main/kotlin/net/corda/core/node/services/vault/QueryCriteria.kt +++ b/core/src/main/kotlin/net/corda/core/node/services/vault/QueryCriteria.kt @@ -303,7 +303,13 @@ sealed class QueryCriteria : GenericQueryCriteria>? = null, relevancyStatus: Vault.RelevancyStatus = Vault.RelevancyStatus.ALL - ) : this(participants, linearId?.map { it.id }, linearId?.mapNotNull { it.externalId }, status, contractStateTypes, relevancyStatus) + ) : this(participants, + linearId?.map { it.id }.takeIf { it != null && it.isNotEmpty() }, + linearId?.mapNotNull { it.externalId }.takeIf { it != null && it.isNotEmpty() }, + status, + contractStateTypes, + relevancyStatus + ) // V3 c'tor @DeprecatedConstructorForDeserialization(version = 1) diff --git a/node/src/main/kotlin/net/corda/node/services/vault/HibernateQueryCriteriaParser.kt b/node/src/main/kotlin/net/corda/node/services/vault/HibernateQueryCriteriaParser.kt index 60f6ea8971..2a52f9b948 100644 --- a/node/src/main/kotlin/net/corda/node/services/vault/HibernateQueryCriteriaParser.kt +++ b/node/src/main/kotlin/net/corda/node/services/vault/HibernateQueryCriteriaParser.kt @@ -153,6 +153,18 @@ abstract class AbstractQueryCriteriaParser, in P: NOT_NULL -> criteriaBuilder.isNotNull(column) } } + + /** + * Returns the given predicate if the provided `args` list is not empty + * If the list is empty it returns an always false predicate (1=0) + */ + protected fun checkIfListIsEmpty(args: List, criteriaBuilder: CriteriaBuilder, predicate: Predicate): Predicate { + return if (args.isEmpty()) { + criteriaBuilder.and(criteriaBuilder.equal(criteriaBuilder.literal(1), 0)) + } else { + predicate + } + } } class HibernateAttachmentQueryCriteriaParser(override val criteriaBuilder: CriteriaBuilder, @@ -219,7 +231,14 @@ class HibernateAttachmentQueryCriteriaParser(override val criteriaBuilder: Crite (criteria.contractClassNamesCondition as EqualityComparison>).rightLiteral else emptyList() val joinDBAttachmentToContractClassNames = root.joinList("contractClassNames") - predicateSet.add(criteriaBuilder.and(joinDBAttachmentToContractClassNames.`in`(contractClassNames))) + + predicateSet.add( + checkIfListIsEmpty( + args = contractClassNames, + criteriaBuilder = criteriaBuilder, + predicate = criteriaBuilder.and(joinDBAttachmentToContractClassNames.`in`(contractClassNames)) + ) + ) } criteria.signersCondition?.let { @@ -228,7 +247,14 @@ class HibernateAttachmentQueryCriteriaParser(override val criteriaBuilder: Crite (criteria.signersCondition as EqualityComparison>).rightLiteral else emptyList() val joinDBAttachmentToSigners = root.joinList("signers") - predicateSet.add(criteriaBuilder.and(joinDBAttachmentToSigners.`in`(signers))) + + predicateSet.add( + checkIfListIsEmpty( + args = signers, + criteriaBuilder = criteriaBuilder, + predicate = criteriaBuilder.and(joinDBAttachmentToSigners.`in`(signers)) + ) + ) } criteria.isSignedCondition?.let { isSigned -> @@ -294,14 +320,27 @@ class HibernateQueryCriteriaParser(val contractStateType: Class("notary").`in`(criteria.notary))) + predicateSet.add( + checkIfListIsEmpty( + args = criteria.notary!!, + criteriaBuilder = criteriaBuilder, + predicate = criteriaBuilder.and(vaultStates.get("notary").`in`(criteria.notary)) + ) + ) } // state references criteria.stateRefs?.let { val persistentStateRefs = (criteria.stateRefs as List).map(::PersistentStateRef) val compositeKey = vaultStates.get("stateRef") - predicateSet.add(criteriaBuilder.and(compositeKey.`in`(persistentStateRefs))) + + predicateSet.add( + checkIfListIsEmpty( + args = persistentStateRefs, + criteriaBuilder = criteriaBuilder, + predicate = criteriaBuilder.and(compositeKey.`in`(persistentStateRefs)) + ) + ) } // time constraints (recorded, consumed) @@ -451,7 +490,14 @@ class HibernateQueryCriteriaParser(val contractStateType: Class - predicateSet.add(criteriaBuilder.and(vaultFungibleStatesRoot.get("owner").`in`(owners))) + + predicateSet.add( + checkIfListIsEmpty( + args = owners, + criteriaBuilder = criteriaBuilder, + predicate = criteriaBuilder.and(vaultFungibleStatesRoot.get("owner").`in`(owners)) + ) + ) } // quantity @@ -462,13 +508,27 @@ class HibernateQueryCriteriaParser(val contractStateType: Class - predicateSet.add(criteriaBuilder.and(vaultFungibleStatesRoot.get("issuer").`in`(issuerParties))) + + predicateSet.add( + checkIfListIsEmpty( + args = issuerParties, + criteriaBuilder = criteriaBuilder, + predicate = criteriaBuilder.and(vaultFungibleStatesRoot.get("issuer").`in`(issuerParties)) + ) + ) } // issuer reference criteria.issuerRef?.let { val issuerRefs = (criteria.issuerRef as List).map { it.bytes } - predicateSet.add(criteriaBuilder.and(vaultFungibleStatesRoot.get("issuerRef").`in`(issuerRefs))) + + predicateSet.add( + checkIfListIsEmpty( + args = issuerRefs, + criteriaBuilder = criteriaBuilder, + predicate = criteriaBuilder.and(vaultFungibleStatesRoot.get("issuerRef").`in`(issuerRefs)) + ) + ) } if (criteria.participants != null && criteria.exactParticipants != null) @@ -502,14 +562,27 @@ class HibernateQueryCriteriaParser(val contractStateType: Class - predicateSet.add(criteriaBuilder.and(vaultLinearStatesRoot.get("uuid").`in`(uuids))) + + predicateSet.add( + checkIfListIsEmpty( + args = uuids, + criteriaBuilder = criteriaBuilder, + predicate = criteriaBuilder.and(vaultLinearStatesRoot.get("uuid").`in`(uuids)) + ) + ) } // linear ids externalId criteria.externalId?.let { val externalIds = criteria.externalId as List - if (externalIds.isNotEmpty()) - predicateSet.add(criteriaBuilder.and(vaultLinearStatesRoot.get("externalId").`in`(externalIds))) + + predicateSet.add( + checkIfListIsEmpty( + args = externalIds, + criteriaBuilder = criteriaBuilder, + predicate = criteriaBuilder.and(vaultLinearStatesRoot.get("externalId").`in`(externalIds)) + ) + ) } if (criteria.participants != null && criteria.exactParticipants != null) @@ -698,8 +771,11 @@ class HibernateQueryCriteriaParser(val contractStateType: Class("x500Name").`in`(participants)) + commonPredicates[predicateID] = checkIfListIsEmpty( + args = participants, + criteriaBuilder = criteriaBuilder, + predicate = criteriaBuilder.and(getPersistentPartyRoot().get("x500Name").`in`(participants)) + ) } // Add the join for vault states to persistent entities (if this is not a Fungible nor Linear criteria query) 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 aa0d828a7d..50bd56921d 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 @@ -259,6 +259,24 @@ class NodeAttachmentServiceTest { ) } + @Test(timeout=300_000) + fun `AttachmentsQueryCriteria returns empty resultset without errors if there is an empty list after the 'in' clause`() { + SelfCleaningDir().use { file -> + val contractJar = makeTestContractJar(file.path, "com.example.MyContract") + contractJar.read { storage.importAttachment(it, "uploaderB", "contract.jar") } + + assertEquals( + 1, + storage.queryAttachments(AttachmentsQueryCriteria(contractClassNamesCondition = Builder.equal(listOf("com.example.MyContract")))).size + ) + + assertEquals( + 0, + storage.queryAttachments(AttachmentsQueryCriteria(contractClassNamesCondition = Builder.equal(emptyList()))).size + ) + } + } + @Test(timeout=300_000) fun `contract class, versioning and signing metadata can be used to search`() { SelfCleaningDir().use { file -> diff --git a/node/src/test/kotlin/net/corda/node/services/vault/VaultQueryTests.kt b/node/src/test/kotlin/net/corda/node/services/vault/VaultQueryTests.kt index 1166a1a500..ef3b1f5a02 100644 --- a/node/src/test/kotlin/net/corda/node/services/vault/VaultQueryTests.kt +++ b/node/src/test/kotlin/net/corda/node/services/vault/VaultQueryTests.kt @@ -267,6 +267,31 @@ abstract class VaultQueryTestsBase : VaultQueryParties { } } + @Test(timeout=300_000) + fun `VaultQueryCriteria returns empty resultset without errors if there is an empty list after the 'in' clause`() { + database.transaction { + val states = vaultFiller.fillWithSomeTestLinearStates(1, "TEST") + val stateRefs = states.states.map { it.ref } + + val criteria = VaultQueryCriteria(notary = listOf(DUMMY_NOTARY)) + val results = vaultService.queryBy(criteria) + assertThat(results.states).hasSize(1) + + val emptyCriteria = VaultQueryCriteria(notary = emptyList()) + val emptyResults = vaultService.queryBy(emptyCriteria) + assertThat(emptyResults.states).hasSize(0) + + val stateCriteria = VaultQueryCriteria(stateRefs = stateRefs) + val stateResults = vaultService.queryBy(stateCriteria) + assertThat(stateResults.states).hasSize(1) + + val emptyStateCriteria = VaultQueryCriteria(stateRefs = emptyList()) + val emptyStateResults = vaultService.queryBy(emptyStateCriteria) + assertThat(emptyStateResults.states).hasSize(0) + + } + } + /** Generic Query tests (combining both FungibleState and LinearState contract types) */ @@ -1823,6 +1848,33 @@ abstract class VaultQueryTestsBase : VaultQueryParties { /** LinearState tests */ + @Test(timeout=300_000) + fun `LinearStateQueryCriteria returns empty resultset without errors if there is an empty list after the 'in' clause`() { + database.transaction { + val uid = UniqueIdentifier("999") + vaultFiller.fillWithSomeTestLinearStates(numberToCreate = 1, uniqueIdentifier = uid) + vaultFiller.fillWithSomeTestLinearStates(numberToCreate = 1, externalId = "1234") + + val uuidCriteria = LinearStateQueryCriteria(uuid = listOf(uid.id)) + val externalIdCriteria = LinearStateQueryCriteria(externalId = listOf("1234")) + + val uuidResults = vaultService.queryBy(uuidCriteria) + val externalIdResults = vaultService.queryBy(externalIdCriteria) + + assertThat(uuidResults.states).hasSize(1) + assertThat(externalIdResults.states).hasSize(1) + + val uuidCriteriaEmpty = LinearStateQueryCriteria(uuid = emptyList()) + val externalIdCriteriaEmpty = LinearStateQueryCriteria(externalId = emptyList()) + + val uuidResultsEmpty = vaultService.queryBy(uuidCriteriaEmpty) + val externalIdResultsEmpty = vaultService.queryBy(externalIdCriteriaEmpty) + + assertThat(uuidResultsEmpty.states).hasSize(0) + assertThat(externalIdResultsEmpty.states).hasSize(0) + } + } + @Test(timeout=300_000) fun `unconsumed linear heads for linearId without external Id`() { database.transaction { @@ -2030,6 +2082,46 @@ abstract class VaultQueryTestsBase : VaultQueryParties { /** FungibleAsset tests */ + @Test(timeout=300_000) + fun `FungibleAssetQueryCriteria returns empty resultset without errors if there is an empty list after the 'in' clause`() { + database.transaction { + vaultFiller.fillWithSomeTestCash(100.DOLLARS, notaryServices, 1, MEGA_CORP.ref(0)) + + val ownerCriteria = FungibleAssetQueryCriteria(owner = listOf(MEGA_CORP)) + val ownerResults = vaultService.queryBy>(ownerCriteria) + + assertThat(ownerResults.states).hasSize(1) + + val emptyOwnerCriteria = FungibleAssetQueryCriteria(owner = emptyList()) + val emptyOwnerResults = vaultService.queryBy>(emptyOwnerCriteria) + + assertThat(emptyOwnerResults.states).hasSize(0) + + // Issuer field checks + val issuerCriteria = FungibleAssetQueryCriteria(issuer = listOf(MEGA_CORP)) + val issuerResults = vaultService.queryBy>(issuerCriteria) + + assertThat(issuerResults.states).hasSize(1) + + val emptyIssuerCriteria = FungibleAssetQueryCriteria(issuer = emptyList()) + val emptyIssuerResults = vaultService.queryBy>(emptyIssuerCriteria) + + assertThat(emptyIssuerResults.states).hasSize(0) + + // Issuer Ref field checks + val issuerRefCriteria = FungibleAssetQueryCriteria(issuerRef = listOf(MINI_CORP.ref(0).reference)) + val issuerRefResults = vaultService.queryBy>(issuerRefCriteria) + + assertThat(issuerRefResults.states).hasSize(1) + + val emptyIssuerRefCriteria = FungibleAssetQueryCriteria(issuerRef = emptyList()) + val emptyIssuerRefResults = vaultService.queryBy>(emptyIssuerRefCriteria) + + assertThat(emptyIssuerRefResults.states).hasSize(0) + + } + } + @Test(timeout=300_000) fun `unconsumed fungible assets for specific issuer party and refs`() { database.transaction { From ddc49babedb8ffbdae78fd489610206b1612e5fd Mon Sep 17 00:00:00 2001 From: James Higgs <45565019+JamesHR3@users.noreply.github.com> Date: Mon, 11 May 2020 16:10:38 +0100 Subject: [PATCH 2/4] EG-441 - Update properties files after docs review (#6223) * Update properties files after docs review * Cosmetic changes to properties files --- .../cordapp-duplicate-cordapps-installed.properties | 4 ++-- .../cordapp-duplicate-cordapps-installed_en_US.properties | 5 +++-- .../cordapp-invalid-version-identifier.properties | 4 ++-- .../cordapp-invalid-version-identifier_en_US.properties | 4 ++-- .../cordapp-missing-version-attribute.properties | 4 ++-- .../cordapp-missing-version-attribute_en_US.properties | 5 +++-- .../error-codes/database-could-not-connect.properties | 4 ++-- .../database-could-not-connect_en_US.properties | 5 +++-- .../error-codes/database-missing-driver.properties | 4 ++-- .../error-codes/database-missing-driver_en_US.properties | 5 +++-- .../database-password-required-for-h2.properties | 6 +++--- .../database-password-required-for-h2_en_US.properties | 7 ++++--- .../src/main/resources/ErrorPageHeadings.properties | 4 ++-- 13 files changed, 33 insertions(+), 28 deletions(-) diff --git a/common/logging/src/main/resources/error-codes/cordapp-duplicate-cordapps-installed.properties b/common/logging/src/main/resources/error-codes/cordapp-duplicate-cordapps-installed.properties index 0fd778435c..b6ff9290cb 100644 --- a/common/logging/src/main/resources/error-codes/cordapp-duplicate-cordapps-installed.properties +++ b/common/logging/src/main/resources/error-codes/cordapp-duplicate-cordapps-installed.properties @@ -1,4 +1,4 @@ errorTemplate = The CorDapp (name: {0}, file: {1}) is installed multiple times on the node. The following files correspond to the exact same content: {2} -shortDescription = A CorDapp has been installed multiple times on the same node. -actionsToFix = Investigate the logs to determine the files with duplicate content, and remove one of them from the cordapps directory. +shortDescription = A CorDapp was installed multiple times on the same node. This is not permitted and causes the node to shut down. +actionsToFix = Investigate the logs to determine the CorDapps with duplicate content, and remove one of them from the 'cordapps' directory. It does not matter which of the CorDapps you choose to remove as their content is identical. aliases = iw8d4e \ No newline at end of file diff --git a/common/logging/src/main/resources/error-codes/cordapp-duplicate-cordapps-installed_en_US.properties b/common/logging/src/main/resources/error-codes/cordapp-duplicate-cordapps-installed_en_US.properties index 2354427df3..b6ff9290cb 100644 --- a/common/logging/src/main/resources/error-codes/cordapp-duplicate-cordapps-installed_en_US.properties +++ b/common/logging/src/main/resources/error-codes/cordapp-duplicate-cordapps-installed_en_US.properties @@ -1,3 +1,4 @@ errorTemplate = The CorDapp (name: {0}, file: {1}) is installed multiple times on the node. The following files correspond to the exact same content: {2} -shortDescription = A CorDapp has been installed multiple times on the same node. -actionsToFix = Investigate the logs to determine the files with duplicate content, and remove one of them from the cordapps directory. \ No newline at end of file +shortDescription = A CorDapp was installed multiple times on the same node. This is not permitted and causes the node to shut down. +actionsToFix = Investigate the logs to determine the CorDapps with duplicate content, and remove one of them from the 'cordapps' directory. It does not matter which of the CorDapps you choose to remove as their content is identical. +aliases = iw8d4e \ No newline at end of file diff --git a/common/logging/src/main/resources/error-codes/cordapp-invalid-version-identifier.properties b/common/logging/src/main/resources/error-codes/cordapp-invalid-version-identifier.properties index 0922f80bb4..64f4416164 100644 --- a/common/logging/src/main/resources/error-codes/cordapp-invalid-version-identifier.properties +++ b/common/logging/src/main/resources/error-codes/cordapp-invalid-version-identifier.properties @@ -1,4 +1,4 @@ errorTemplate = Version identifier ({0}) for attribute {1} must be a whole number starting from 1. -shortDescription = A version attribute was specified in the CorDapp manifest with an invalid value. The value must be a whole number, and it must be greater than or equal to 1. -actionsToFix = Investigate the logs to find the invalid attribute, and change the attribute value to be valid (a whole number greater than or equal to 1). +shortDescription = A version attribute with an invalid value was specified in the manifest of the CorDapp JAR. The version attribute value must be a whole number that is greater than or equal to 1. +actionsToFix = Investigate the logs to find the invalid version attribute, and change its value to a valid one (a whole number greater than or equal to 1). aliases = \ No newline at end of file diff --git a/common/logging/src/main/resources/error-codes/cordapp-invalid-version-identifier_en_US.properties b/common/logging/src/main/resources/error-codes/cordapp-invalid-version-identifier_en_US.properties index 0922f80bb4..64f4416164 100644 --- a/common/logging/src/main/resources/error-codes/cordapp-invalid-version-identifier_en_US.properties +++ b/common/logging/src/main/resources/error-codes/cordapp-invalid-version-identifier_en_US.properties @@ -1,4 +1,4 @@ errorTemplate = Version identifier ({0}) for attribute {1} must be a whole number starting from 1. -shortDescription = A version attribute was specified in the CorDapp manifest with an invalid value. The value must be a whole number, and it must be greater than or equal to 1. -actionsToFix = Investigate the logs to find the invalid attribute, and change the attribute value to be valid (a whole number greater than or equal to 1). +shortDescription = A version attribute with an invalid value was specified in the manifest of the CorDapp JAR. The version attribute value must be a whole number that is greater than or equal to 1. +actionsToFix = Investigate the logs to find the invalid version attribute, and change its value to a valid one (a whole number greater than or equal to 1). aliases = \ No newline at end of file diff --git a/common/logging/src/main/resources/error-codes/cordapp-missing-version-attribute.properties b/common/logging/src/main/resources/error-codes/cordapp-missing-version-attribute.properties index 772b459195..b820268356 100644 --- a/common/logging/src/main/resources/error-codes/cordapp-missing-version-attribute.properties +++ b/common/logging/src/main/resources/error-codes/cordapp-missing-version-attribute.properties @@ -1,4 +1,4 @@ errorTemplate = Target versionId attribute {0} not specified. Please specify a whole number starting from 1. -shortDescription = A required version attribute was not specified in the manifest of the CorDapp JAR. -actionsToFix = Investigate the logs to find out which version attribute has not been specified, and add that version attribute to the CorDapp manifest. +shortDescription = A required version attribute was not specified in the manifest of the CorDapp JAR. +actionsToFix = Investigate the logs to find out which version attribute was not specified, and add that version attribute to the CorDapp manifest. aliases = \ No newline at end of file diff --git a/common/logging/src/main/resources/error-codes/cordapp-missing-version-attribute_en_US.properties b/common/logging/src/main/resources/error-codes/cordapp-missing-version-attribute_en_US.properties index d8a7ab050a..b820268356 100644 --- a/common/logging/src/main/resources/error-codes/cordapp-missing-version-attribute_en_US.properties +++ b/common/logging/src/main/resources/error-codes/cordapp-missing-version-attribute_en_US.properties @@ -1,3 +1,4 @@ errorTemplate = Target versionId attribute {0} not specified. Please specify a whole number starting from 1. -shortDescription = A required version attribute was not specified in the manifest of the CorDapp JAR. -actionsToFix = Investigate the logs to find out which version attribute has not been specified, and add that version attribute to the CorDapp manifest. \ No newline at end of file +shortDescription = A required version attribute was not specified in the manifest of the CorDapp JAR. +actionsToFix = Investigate the logs to find out which version attribute was not specified, and add that version attribute to the CorDapp manifest. +aliases = \ No newline at end of file diff --git a/common/logging/src/main/resources/error-codes/database-could-not-connect.properties b/common/logging/src/main/resources/error-codes/database-could-not-connect.properties index d2c349f8ca..8d84c07729 100644 --- a/common/logging/src/main/resources/error-codes/database-could-not-connect.properties +++ b/common/logging/src/main/resources/error-codes/database-could-not-connect.properties @@ -1,4 +1,4 @@ errorTemplate = Could not connect to the database. Please check your JDBC connection URL, or the connectivity to the database. -shortDescription = The node failed to connect to the database on node startup, preventing the node from starting correctly. -actionsToFix = This happens either because the database connection has been misconfigured or the database is unreachable. Check that the JDBC URL is configured correctly in your node.conf. If this is correctly configured, then check your database connection. +shortDescription = The node failed to connect to the database on node startup and thus prevented the node from starting correctly. +actionsToFix = This happened either because the database connection was misconfigured or because the database was unreachable. Check that the JDBC URL is configured correctly in your 'node.conf' file. If this is correctly configured, then check your database connection. aliases = \ No newline at end of file diff --git a/common/logging/src/main/resources/error-codes/database-could-not-connect_en_US.properties b/common/logging/src/main/resources/error-codes/database-could-not-connect_en_US.properties index bfd5973209..8d84c07729 100644 --- a/common/logging/src/main/resources/error-codes/database-could-not-connect_en_US.properties +++ b/common/logging/src/main/resources/error-codes/database-could-not-connect_en_US.properties @@ -1,3 +1,4 @@ errorTemplate = Could not connect to the database. Please check your JDBC connection URL, or the connectivity to the database. -shortDescription = The node failed to connect to the database on node startup, preventing the node from starting correctly. -actionsToFix = This happens either because the database connection has been misconfigured or the database is unreachable. Check that the JDBC URL is configured correctly in your node.conf. If this is correctly configured, then check your database connection. \ No newline at end of file +shortDescription = The node failed to connect to the database on node startup and thus prevented the node from starting correctly. +actionsToFix = This happened either because the database connection was misconfigured or because the database was unreachable. Check that the JDBC URL is configured correctly in your 'node.conf' file. If this is correctly configured, then check your database connection. +aliases = \ No newline at end of file diff --git a/common/logging/src/main/resources/error-codes/database-missing-driver.properties b/common/logging/src/main/resources/error-codes/database-missing-driver.properties index dfb6b460db..9be44e623f 100644 --- a/common/logging/src/main/resources/error-codes/database-missing-driver.properties +++ b/common/logging/src/main/resources/error-codes/database-missing-driver.properties @@ -1,4 +1,4 @@ -errorTemplate = Could not find the database driver class. Please add it to the 'drivers' folder. +errorTemplate = Could not find the database driver class. Please add it to the 'drivers' directory. shortDescription = The node could not find the driver in the 'drivers' directory. -actionsToFix = Please ensure that the correct database driver has been placed in the 'drivers' folder. The driver must contain the driver main class specified in 'node.conf'. +actionsToFix = Ensure that the 'drivers' directory contains the correct database driver. The driver must contain the driver class as specified in 'node.conf'. aliases = \ No newline at end of file diff --git a/common/logging/src/main/resources/error-codes/database-missing-driver_en_US.properties b/common/logging/src/main/resources/error-codes/database-missing-driver_en_US.properties index c52708a02b..9be44e623f 100644 --- a/common/logging/src/main/resources/error-codes/database-missing-driver_en_US.properties +++ b/common/logging/src/main/resources/error-codes/database-missing-driver_en_US.properties @@ -1,3 +1,4 @@ -errorTemplate = Could not find the database driver class. Please add it to the 'drivers' folder. +errorTemplate = Could not find the database driver class. Please add it to the 'drivers' directory. shortDescription = The node could not find the driver in the 'drivers' directory. -actionsToFix = Please ensure that the correct database driver has been placed in the 'drivers' folder. The driver must contain the driver main class specified in 'node.conf'. \ No newline at end of file +actionsToFix = Ensure that the 'drivers' directory contains the correct database driver. The driver must contain the driver class as specified in 'node.conf'. +aliases = \ No newline at end of file diff --git a/common/logging/src/main/resources/error-codes/database-password-required-for-h2.properties b/common/logging/src/main/resources/error-codes/database-password-required-for-h2.properties index 7284fa125e..1bf619240d 100644 --- a/common/logging/src/main/resources/error-codes/database-password-required-for-h2.properties +++ b/common/logging/src/main/resources/error-codes/database-password-required-for-h2.properties @@ -1,4 +1,4 @@ -errorTemplate = Database password is required for H2 server listening on {0} -shortDescription = A password is required to access the H2 server the node is trying to access, and this password is missing. -actionsToFix = Add the required password to the 'datasource.password' configuration in 'node.conf'. +errorTemplate = A database password is required for H2 server listening on {0} +shortDescription = The node is trying to access an H2 server that requires a password, which is missing. +actionsToFix = Add the required password to the 'datasource.password' configuration section in the 'node.conf' file. aliases = \ No newline at end of file diff --git a/common/logging/src/main/resources/error-codes/database-password-required-for-h2_en_US.properties b/common/logging/src/main/resources/error-codes/database-password-required-for-h2_en_US.properties index 3afefc34d9..1bf619240d 100644 --- a/common/logging/src/main/resources/error-codes/database-password-required-for-h2_en_US.properties +++ b/common/logging/src/main/resources/error-codes/database-password-required-for-h2_en_US.properties @@ -1,3 +1,4 @@ -errorTemplate = Database password is required for H2 server listening on {0} -shortDescription = A password is required to access the H2 server the node is trying to access, and this password is missing. -actionsToFix = Add the required password to the 'datasource.password' configuration in 'node.conf'. \ No newline at end of file +errorTemplate = A database password is required for H2 server listening on {0} +shortDescription = The node is trying to access an H2 server that requires a password, which is missing. +actionsToFix = Add the required password to the 'datasource.password' configuration section in the 'node.conf' file. +aliases = \ No newline at end of file diff --git a/tools/error-tool/src/main/resources/ErrorPageHeadings.properties b/tools/error-tool/src/main/resources/ErrorPageHeadings.properties index 5741987c8a..176e29b38c 100644 --- a/tools/error-tool/src/main/resources/ErrorPageHeadings.properties +++ b/tools/error-tool/src/main/resources/ErrorPageHeadings.properties @@ -1,4 +1,4 @@ -codeHeading = Error Code +codeHeading = Error code aliasesHeading = Aliases descriptionHeading = Description -toFixHeading = Actions to Fix \ No newline at end of file +toFixHeading = Actions to fix \ No newline at end of file From 1547efb0932d27aac40ff69fc25742c034335689 Mon Sep 17 00:00:00 2001 From: Adel El-Beik <48713346+adelel1@users.noreply.github.com> Date: Tue, 12 May 2020 09:51:12 +0100 Subject: [PATCH 3/4] CORDA-3755: Switched attachments map to a WeakHashMap (#6214) * Bump OS release version 4.6 * CORDA-3755: Switched attachments map to a WeakHashMap * CORDA-3755: Added explicit strong references to map key. * CORDA-3755: Keeping detekt happy. * CORDA-3755: Test a gc in verify. * CORDA-3755: Making detekt happy. * CORDA-3755: Suppress warnings for weak reference test. * CORDA-3755: Fixing build failure with attachments. * CORDA-3755: Rewrite based on Ricks input - now handles attachment already existing in map! * CORDA-3755: Refactor WeakReference behaviour into AttachmentsHolderImpl and provide alternate version of this class for core-deterministic. * CORDA-3755: Added more tests for WeakHashMap. * CORDA-3755: Ignore the tests using System.gc keep for local testing only * CORDA-3755: Adding comment to explain the ignored tests. * Make AttachmentsHolderImpl package-private inside core-deterministic, just like it is inside core. * CORDA-3755: Update assertions following review comments. * CORDA-3755: Removing import * CORDA-3755: Removed unused var. * CORDA-3755: Reverting files that somehow got changed in rebase. Co-authored-by: nargas-ritu Co-authored-by: Chris Rankin --- core-deterministic/build.gradle | 1 + .../internal/AttachmentsHolderImpl.kt | 23 ++++ .../AttachmentsClassLoaderTests.kt | 126 ++++++++++++++++- .../internal/AttachmentsClassLoader.kt | 95 +++++++++---- .../core/internal/ClassLoadingUtilsTest.kt | 127 +++++++++++++++++- .../core/internal/ContractJarTestUtils.kt | 10 +- 6 files changed, 346 insertions(+), 36 deletions(-) create mode 100644 core-deterministic/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsHolderImpl.kt diff --git a/core-deterministic/build.gradle b/core-deterministic/build.gradle index 73209915d2..636ce86800 100644 --- a/core-deterministic/build.gradle +++ b/core-deterministic/build.gradle @@ -71,6 +71,7 @@ def patchCore = tasks.register('patchCore', Zip) { exclude 'net/corda/core/crypto/SHA256DigestSupplier.class' exclude 'net/corda/core/internal/*ToggleField*.class' exclude 'net/corda/core/serialization/*SerializationFactory*.class' + exclude 'net/corda/core/serialization/internal/AttachmentsHolderImpl.class' exclude 'net/corda/core/serialization/internal/CheckpointSerializationFactory*.class' exclude 'net/corda/core/internal/rules/*.class' } diff --git a/core-deterministic/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsHolderImpl.kt b/core-deterministic/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsHolderImpl.kt new file mode 100644 index 0000000000..a2f7b8ab30 --- /dev/null +++ b/core-deterministic/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsHolderImpl.kt @@ -0,0 +1,23 @@ +package net.corda.core.serialization.internal + +import net.corda.core.contracts.Attachment +import java.net.URL + +@Suppress("unused") +private class AttachmentsHolderImpl : AttachmentsHolder { + private val attachments = LinkedHashMap>() + + override val size: Int get() = attachments.size + + override fun getKey(key: URL): URL? { + return attachments[key]?.first + } + + override fun get(key: URL): Attachment? { + return attachments[key]?.second + } + + override fun set(key: URL, value: Attachment) { + attachments[key] = key to value + } +} diff --git a/core-tests/src/test/kotlin/net/corda/coretests/transactions/AttachmentsClassLoaderTests.kt b/core-tests/src/test/kotlin/net/corda/coretests/transactions/AttachmentsClassLoaderTests.kt index 6a98b20172..fcc081efb6 100644 --- a/core-tests/src/test/kotlin/net/corda/coretests/transactions/AttachmentsClassLoaderTests.kt +++ b/core-tests/src/test/kotlin/net/corda/coretests/transactions/AttachmentsClassLoaderTests.kt @@ -1,19 +1,37 @@ package net.corda.coretests.transactions +import net.corda.core.contracts.AlwaysAcceptAttachmentConstraint import net.corda.core.contracts.Attachment +import net.corda.core.contracts.CommandData +import net.corda.core.contracts.CommandWithParties import net.corda.core.contracts.Contract +import net.corda.core.contracts.ContractAttachment +import net.corda.core.contracts.PrivacySalt +import net.corda.core.contracts.StateAndRef +import net.corda.core.contracts.TimeWindow +import net.corda.core.contracts.TransactionState import net.corda.core.contracts.TransactionVerificationException import net.corda.core.crypto.Crypto import net.corda.core.crypto.SecureHash +import net.corda.core.identity.Party +import net.corda.core.internal.AbstractAttachment import net.corda.core.internal.AttachmentTrustCalculator +import net.corda.core.internal.createLedgerTransaction import net.corda.core.internal.declaredField import net.corda.core.internal.hash import net.corda.core.internal.inputStream import net.corda.core.node.NetworkParameters import net.corda.core.node.services.AttachmentId import net.corda.core.serialization.internal.AttachmentsClassLoader -import net.corda.node.services.attachments.NodeAttachmentTrustCalculator import net.corda.testing.common.internal.testNetworkParameters +import net.corda.node.services.attachments.NodeAttachmentTrustCalculator +import net.corda.testing.contracts.DummyContract +import net.corda.testing.core.ALICE_NAME +import net.corda.testing.core.BOB_NAME +import net.corda.testing.core.DUMMY_NOTARY_NAME +import net.corda.testing.core.SerializationEnvironmentRule +import net.corda.testing.core.TestIdentity +import net.corda.testing.core.internal.ContractJarTestUtils import net.corda.testing.core.internal.ContractJarTestUtils.signContractJar import net.corda.testing.internal.TestingNamedCacheFactory import net.corda.testing.internal.fakeAttachment @@ -27,10 +45,14 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before +import org.junit.Rule import org.junit.Test +import org.junit.rules.TemporaryFolder import java.io.ByteArrayOutputStream import java.io.InputStream import java.net.URL +import java.nio.file.Path +import java.security.PublicKey import kotlin.test.assertFailsWith import kotlin.test.fail @@ -47,8 +69,21 @@ class AttachmentsClassLoaderTests { it.toByteArray() } } + val ALICE = TestIdentity(ALICE_NAME, 70).party + val BOB = TestIdentity(BOB_NAME, 80).party + val dummyNotary = TestIdentity(DUMMY_NOTARY_NAME, 20) + val DUMMY_NOTARY get() = dummyNotary.party + val PROGRAM_ID: String = "net.corda.testing.contracts.MyDummyContract" } + @Rule + @JvmField + val tempFolder = TemporaryFolder() + + @Rule + @JvmField + val testSerialization = SerializationEnvironmentRule() + private lateinit var storage: MockAttachmentStorage private lateinit var internalStorage: InternalMockAttachmentStorage private lateinit var attachmentTrustCalculator: AttachmentTrustCalculator @@ -469,4 +504,93 @@ class AttachmentsClassLoaderTests { createClassloader(trustedAttachment).use {} } + + @Test(timeout=300_000) + fun `attachment still available in verify after forced gc in verify`() { + tempFolder.root.toPath().let { path -> + val baseOutState = TransactionState(DummyContract.SingleOwnerState(0, ALICE), PROGRAM_ID, DUMMY_NOTARY, constraint = AlwaysAcceptAttachmentConstraint) + val inputs = emptyList>() + val outputs = listOf(baseOutState, baseOutState.copy(notary = ALICE), baseOutState.copy(notary = BOB)) + val commands = emptyList>() + + val content = createContractString(PROGRAM_ID) + val contractJarPath = ContractJarTestUtils.makeTestContractJar(path, PROGRAM_ID, content = content) + + val attachments = createAttachments(contractJarPath) + + val id = SecureHash.randomSHA256() + val timeWindow: TimeWindow? = null + val privacySalt = PrivacySalt() + val transaction = createLedgerTransaction( + inputs, + outputs, + commands, + attachments, + id, + null, + timeWindow, + privacySalt, + testNetworkParameters(), + emptyList(), + isAttachmentTrusted = { true } + ) + transaction.verify() + } + } + + private fun createContractString(contractName: String, versionSeed: Int = 0): String { + val pkgs = contractName.split(".") + val className = pkgs.last() + val packages = pkgs.subList(0, pkgs.size - 1) + + val output = """package ${packages.joinToString(".")}; + import net.corda.core.contracts.*; + import net.corda.core.transactions.*; + import java.net.URL; + import java.io.InputStream; + + public class $className implements Contract { + private int seed = $versionSeed; + @Override + public void verify(LedgerTransaction tx) throws IllegalArgumentException { + System.gc(); + InputStream str = this.getClass().getClassLoader().getResourceAsStream("importantDoc.pdf"); + if (str == null) throw new IllegalStateException("Could not find importantDoc.pdf"); + } + } + """.trimIndent() + + System.out.println(output) + return output + } + + private fun createAttachments(contractJarPath: Path) : List { + + val attachment = object : AbstractAttachment({contractJarPath.inputStream().readBytes()}, uploader = "app") { + @Suppress("OverridingDeprecatedMember") + override val signers: List = emptyList() + override val signerKeys: List = emptyList() + override val size: Int = 1234 + override val id: SecureHash = SecureHash.sha256(attachmentData) + } + val contractAttachment = ContractAttachment(attachment, PROGRAM_ID) + + return listOf( + object : AbstractAttachment({ISOLATED_CONTRACTS_JAR_PATH.openStream().readBytes()}, uploader = "app") { + @Suppress("OverridingDeprecatedMember") + override val signers: List = emptyList() + override val signerKeys: List = emptyList() + override val size: Int = 1234 + override val id: SecureHash = SecureHash.sha256(attachmentData) + }, + object : AbstractAttachment({fakeAttachment("importantDoc.pdf", "I am a pdf!").inputStream().readBytes() + }, uploader = "app") { + @Suppress("OverridingDeprecatedMember") + override val signers: List = emptyList() + override val signerKeys: List = emptyList() + override val size: Int = 1234 + override val id: SecureHash = SecureHash.sha256(attachmentData) + }, + contractAttachment) + } } diff --git a/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt b/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt index dfb4e95481..e4b00a6c8b 100644 --- a/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt +++ b/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt @@ -17,6 +17,7 @@ import net.corda.core.utilities.debug import java.io.ByteArrayOutputStream import java.io.IOException import java.io.InputStream +import java.lang.ref.WeakReference import java.net.* import java.security.Permission import java.util.* @@ -53,14 +54,6 @@ class AttachmentsClassLoader(attachments: List, private val ignoreDirectories = listOf("org/jolokia/", "org/json/simple/") private val ignorePackages = ignoreDirectories.map { it.replace("/", ".") } - @VisibleForTesting - private fun readAttachment(attachment: Attachment, filepath: String): ByteArray { - ByteArrayOutputStream().use { - attachment.extractFile(filepath, it) - return it.toByteArray() - } - } - /** * Apply our custom factory either directly, if `URL.setURLStreamHandlerFactory` has not been called yet, * or use a decorator and reflection to bypass the single-call-per-JVM restriction otherwise. @@ -359,8 +352,7 @@ object AttachmentsClassLoaderBuilder { object AttachmentURLStreamHandlerFactory : URLStreamHandlerFactory { internal const val attachmentScheme = "attachment" - // TODO - what happens if this grows too large? - private val loadedAttachments = mutableMapOf().toSynchronised() + private val loadedAttachments: AttachmentsHolder = AttachmentsHolderImpl() override fun createURLStreamHandler(protocol: String): URLStreamHandler? { return if (attachmentScheme == protocol) { @@ -368,34 +360,79 @@ object AttachmentURLStreamHandlerFactory : URLStreamHandlerFactory { } else null } + @Synchronized fun toUrl(attachment: Attachment): URL { - val id = attachment.id.toString() - loadedAttachments[id] = attachment - return URL(attachmentScheme, "", -1, id, AttachmentURLStreamHandler) + val proposedURL = URL(attachmentScheme, "", -1, attachment.id.toString(), AttachmentURLStreamHandler) + val existingURL = loadedAttachments.getKey(proposedURL) + return if (existingURL == null) { + loadedAttachments[proposedURL] = attachment + proposedURL + } else { + existingURL + } } + @VisibleForTesting + fun loadedAttachmentsSize(): Int = loadedAttachments.size + private object AttachmentURLStreamHandler : URLStreamHandler() { override fun openConnection(url: URL): URLConnection { if (url.protocol != attachmentScheme) throw IOException("Cannot handle protocol: ${url.protocol}") - val attachment = loadedAttachments[url.path] ?: throw IOException("Could not load url: $url .") + val attachment = loadedAttachments[url] ?: throw IOException("Could not load url: $url .") return AttachmentURLConnection(url, attachment) } - } - private class AttachmentURLConnection(url: URL, private val attachment: Attachment) : URLConnection(url) { - override fun getContentLengthLong(): Long = attachment.size.toLong() - override fun getInputStream(): InputStream = attachment.open() - /** - * Define the permissions that [AttachmentsClassLoader] will need to - * use this [URL]. The attachment is stored in memory, and so we - * don't need any extra permissions here. But if we don't override - * [getPermission] then [AttachmentsClassLoader] will assign the - * default permission of ALL_PERMISSION to these classes' - * [java.security.ProtectionDomain]. This would be a security hole! - */ - override fun getPermission(): Permission? = null - override fun connect() { - connected = true + override fun equals(attachmentUrl: URL, otherURL: URL?): Boolean { + if (attachmentUrl.protocol != otherURL?.protocol) return false + if (attachmentUrl.protocol != attachmentScheme) throw IllegalArgumentException("Cannot handle protocol: ${attachmentUrl.protocol}") + return attachmentUrl.file == otherURL?.file + } + + override fun hashCode(url: URL): Int { + if (url.protocol != attachmentScheme) throw IllegalArgumentException("Cannot handle protocol: ${url.protocol}") + return url.file.hashCode() } } +} + +interface AttachmentsHolder { + val size: Int + fun getKey(key: URL): URL? + operator fun get(key: URL): Attachment? + operator fun set(key: URL, value: Attachment) +} + +private class AttachmentsHolderImpl : AttachmentsHolder { + private val attachments = WeakHashMap, Attachment>>().toSynchronised() + + override val size: Int get() = attachments.size + + override fun getKey(key: URL): URL? { + return attachments[key]?.first?.get() + } + + override fun get(key: URL): Attachment? { + return attachments[key]?.second + } + + override fun set(key: URL, value: Attachment) { + attachments[key] = WeakReference(key) to value + } +} + +private class AttachmentURLConnection(url: URL, private val attachment: Attachment) : URLConnection(url) { + override fun getContentLengthLong(): Long = attachment.size.toLong() + override fun getInputStream(): InputStream = attachment.open() + /** + * Define the permissions that [AttachmentsClassLoader] will need to + * use this [URL]. The attachment is stored in memory, and so we + * don't need any extra permissions here. But if we don't override + * [getPermission] then [AttachmentsClassLoader] will assign the + * default permission of ALL_PERMISSION to these classes' + * [java.security.ProtectionDomain]. This would be a security hole! + */ + override fun getPermission(): Permission? = null + override fun connect() { + connected = true + } } \ No newline at end of file diff --git a/core/src/test/kotlin/net/corda/core/internal/ClassLoadingUtilsTest.kt b/core/src/test/kotlin/net/corda/core/internal/ClassLoadingUtilsTest.kt index 3e19491b94..68bd3ba625 100644 --- a/core/src/test/kotlin/net/corda/core/internal/ClassLoadingUtilsTest.kt +++ b/core/src/test/kotlin/net/corda/core/internal/ClassLoadingUtilsTest.kt @@ -5,13 +5,20 @@ import net.corda.core.contracts.ContractAttachment import net.corda.core.contracts.ContractClassName import net.corda.core.crypto.SecureHash import net.corda.core.identity.Party +import net.corda.core.node.services.AttachmentId import net.corda.core.serialization.internal.AttachmentURLStreamHandlerFactory import net.corda.core.serialization.internal.AttachmentsClassLoader import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Ignore import org.junit.Test import java.io.ByteArrayOutputStream +import java.lang.ref.ReferenceQueue +import java.lang.ref.WeakReference +import java.net.URL import java.net.URLClassLoader import java.security.PublicKey import java.util.jar.JarOutputStream @@ -120,9 +127,125 @@ class ClassLoadingUtilsTest { } } - private fun signedAttachment(data: ByteArray, vararg parties: Party) = ContractAttachment.create( + @Ignore("Using System.gc in this test which has no guarantees when/if gc occurs.") + @Test(timeout=300_000) + @Suppress("ExplicitGarbageCollectionCall", "UNUSED_VALUE") + fun `test weak reference removed from map`() { + val jarData = with(ByteArrayOutputStream()) { + val internalName = STANDALONE_CLASS_NAME.asInternalName + JarOutputStream(this, Manifest()).use { + it.setLevel(NO_COMPRESSION) + it.setMethod(DEFLATED) + it.putNextEntry(directoryEntry("com")) + it.putNextEntry(directoryEntry("com/example")) + it.putNextEntry(classEntry(internalName)) + it.write(TemplateClassWithEmptyConstructor::class.java.renameTo(internalName)) + } + toByteArray() + } + val attachment = signedAttachment(jarData) + var url: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment) + + val referenceQueue: ReferenceQueue = ReferenceQueue() + val weakReference = WeakReference(url, referenceQueue) + + assertEquals(1, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + // Clear strong reference + url = null + System.gc() + val ref = referenceQueue.remove(100000) + assertSame(weakReference, ref) + assertEquals(0, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + } + + @Ignore("Using System.gc in this test which has no guarantees when/if gc occurs.") + @Test(timeout=300_000) + @Suppress("ExplicitGarbageCollectionCall", "UNUSED_VALUE") + fun `test adding same attachment twice then removing`() { + val jarData = with(ByteArrayOutputStream()) { + val internalName = STANDALONE_CLASS_NAME.asInternalName + JarOutputStream(this, Manifest()).use { + it.setLevel(NO_COMPRESSION) + it.setMethod(DEFLATED) + it.putNextEntry(directoryEntry("com")) + it.putNextEntry(directoryEntry("com/example")) + it.putNextEntry(classEntry(internalName)) + it.write(TemplateClassWithEmptyConstructor::class.java.renameTo(internalName)) + } + toByteArray() + } + val attachment1 = signedAttachment(jarData) + val attachment2 = signedAttachment(jarData) + var url1: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment1) + var url2: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment2) + + val referenceQueue1: ReferenceQueue = ReferenceQueue() + val weakReference1 = WeakReference(url1, referenceQueue1) + + val referenceQueue2: ReferenceQueue = ReferenceQueue() + val weakReference2 = WeakReference(url2, referenceQueue2) + + assertEquals(1, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + url1 = null + System.gc() + val ref1 = referenceQueue1.remove(500) + assertNull(ref1) + assertEquals(1, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + + url2 = null + System.gc() + val ref2 = referenceQueue2.remove(100000) + assertSame(weakReference2, ref2) + assertSame(weakReference1, referenceQueue1.poll()) + assertEquals(0, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + } + + @Ignore("Using System.gc in this test which has no guarantees when/if gc occurs.") + @Test(timeout=300_000) + @Suppress("ExplicitGarbageCollectionCall", "UNUSED_VALUE") + fun `test adding two different attachments then removing`() { + val jarData1 = with(ByteArrayOutputStream()) { + val internalName = STANDALONE_CLASS_NAME.asInternalName + JarOutputStream(this, Manifest()).use { + it.setLevel(NO_COMPRESSION) + it.setMethod(DEFLATED) + it.putNextEntry(directoryEntry("com")) + it.putNextEntry(directoryEntry("com/example")) + it.putNextEntry(classEntry(internalName)) + it.write(TemplateClassWithEmptyConstructor::class.java.renameTo(internalName)) + } + toByteArray() + } + + val attachment1 = signedAttachment(jarData1) + val attachment2 = signedAttachment(jarData1, id = SecureHash.randomSHA256()) + var url1: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment1) + var url2: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment2) + + val referenceQueue1: ReferenceQueue = ReferenceQueue() + val weakReference1 = WeakReference(url1, referenceQueue1) + + val referenceQueue2: ReferenceQueue = ReferenceQueue() + val weakReference2 = WeakReference(url2, referenceQueue2) + + assertEquals(2, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + url1 = null + System.gc() + val ref1 = referenceQueue1.remove(100000) + assertSame(weakReference1, ref1) + assertEquals(1, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + + url2 = null + System.gc() + val ref2 = referenceQueue2.remove(100000) + assertSame(weakReference2, ref2) + assertEquals(0, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + } + + private fun signedAttachment(data: ByteArray, id: AttachmentId = contractAttachmentId, + vararg parties: Party) = ContractAttachment.create( object : AbstractAttachment({ data }, "test") { - override val id: SecureHash get() = contractAttachmentId + override val id: SecureHash get() = id override val signerKeys: List get() = parties.map(Party::owningKey) }, PROGRAM_ID, signerKeys = parties.map(Party::owningKey) diff --git a/testing/core-test-utils/src/main/kotlin/net/corda/testing/core/internal/ContractJarTestUtils.kt b/testing/core-test-utils/src/main/kotlin/net/corda/testing/core/internal/ContractJarTestUtils.kt index cefc596c3a..413f2bf843 100644 --- a/testing/core-test-utils/src/main/kotlin/net/corda/testing/core/internal/ContractJarTestUtils.kt +++ b/testing/core-test-utils/src/main/kotlin/net/corda/testing/core/internal/ContractJarTestUtils.kt @@ -59,12 +59,14 @@ object ContractJarTestUtils { return workingDir.resolve(jarName) to signer } + @Suppress("LongParameterList") @JvmOverloads - fun makeTestContractJar(workingDir: Path, contractName: String, signed: Boolean = false, version: Int = 1, versionSeed: Int = 0): Path { + fun makeTestContractJar(workingDir: Path, contractName: String, signed: Boolean = false, version: Int = 1, versionSeed: Int = 0, + content: String? = null): Path { val packages = contractName.split(".") val jarName = "attachment-${packages.last()}-$version-$versionSeed-${(if (signed) "signed" else "")}.jar" val className = packages.last() - createTestClass(workingDir, className, packages.subList(0, packages.size - 1), versionSeed) + createTestClass(workingDir, className, packages.subList(0, packages.size - 1), versionSeed, content) workingDir.createJar(jarName, "${contractName.replace(".", "/")}.class") workingDir.addManifest(jarName, Pair(Attributes.Name(CORDAPP_CONTRACT_VERSION), version.toString())) return workingDir.resolve(jarName) @@ -87,8 +89,8 @@ object ContractJarTestUtils { return workingDir.resolve(jarName) } - private fun createTestClass(workingDir: Path, className: String, packages: List, versionSeed: Int = 0): Path { - val newClass = """package ${packages.joinToString(".")}; + private fun createTestClass(workingDir: Path, className: String, packages: List, versionSeed: Int = 0, content: String? = null): Path { + val newClass = content ?: """package ${packages.joinToString(".")}; import net.corda.core.contracts.*; import net.corda.core.transactions.*; From 3ced61103169b9207dc7ea278f20296f323ca554 Mon Sep 17 00:00:00 2001 From: Adel El-Beik <48713346+adelel1@users.noreply.github.com> Date: Tue, 12 May 2020 09:52:32 +0100 Subject: [PATCH 4/4] CORDA-3730: Remove unused feature. (#6234) --- .../kotlin/net/corda/core/internal/PlatformVersionSwitches.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/kotlin/net/corda/core/internal/PlatformVersionSwitches.kt b/core/src/main/kotlin/net/corda/core/internal/PlatformVersionSwitches.kt index 144b5b5821..8d24cb03d3 100644 --- a/core/src/main/kotlin/net/corda/core/internal/PlatformVersionSwitches.kt +++ b/core/src/main/kotlin/net/corda/core/internal/PlatformVersionSwitches.kt @@ -5,6 +5,5 @@ Constants for new features that can only be switched on at specific platform ver The text constant describes the feature and the numeric specifies the platform version the feature is enabled at. */ object PlatformVersionSwitches { - const val REMOVE_NO_OVERLAP_RULE_FOR_REFERENCE_DATA_ATTACHMENTS = 7 const val ENABLE_P2P_COMPRESSION = 7 } \ No newline at end of file