From cd4626d8c2b538154530f8074fb2769c1ac77383 Mon Sep 17 00:00:00 2001 From: Ross Nicoll Date: Wed, 22 Jul 2020 08:08:49 +0000 Subject: [PATCH 1/5] CORDA-3917 Update to Jackson 2.9.8 (#6493) * Update to Jackson 2.9.8 to address multiple security issues, and update warning note about updates to clarify that it refers to 2.10+. When the note was added 2.9.7 as the highest available version in the 2.9.x series. * Add PR code checks Jenkinsfile --- .ci/dev/pr-code-checks/Jenkinsfile | 76 ++++++++++++++++++++++++++++++ build.gradle | 4 +- 2 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 .ci/dev/pr-code-checks/Jenkinsfile diff --git a/.ci/dev/pr-code-checks/Jenkinsfile b/.ci/dev/pr-code-checks/Jenkinsfile new file mode 100644 index 0000000000..a64813c92f --- /dev/null +++ b/.ci/dev/pr-code-checks/Jenkinsfile @@ -0,0 +1,76 @@ +@Library('corda-shared-build-pipeline-steps') +import static com.r3.build.BuildControl.killAllExistingBuildsForJob + +killAllExistingBuildsForJob(env.JOB_NAME, env.BUILD_NUMBER.toInteger()) + +pipeline { + agent { label 'k8s' } + options { + timestamps() + timeout(time: 3, unit: 'HOURS') + buildDiscarder(logRotator(daysToKeepStr: '14', artifactDaysToKeepStr: '14')) + } + + environment { + PR_CONTEXT_STRING = "PR Code Checks" + } + + stages { + stage('Detekt check') { + steps { + script { + pullRequest.createStatus( + status: 'pending', + context: "${PR_CONTEXT_STRING}", + description: "Running code checks", + targetUrl: "${env.BUILD_URL}") + } + sh "./gradlew --no-daemon clean detekt" + } + } + + stage('Compilation warnings check') { + steps { + sh "./gradlew --no-daemon -Pcompilation.warningsAsErrors=true compileAll" + } + } + + stage('No API change check') { + steps { + sh "./gradlew --no-daemon generateApi" + sh ".ci/check-api-changes.sh" + } + } + + stage('Deploy Nodes') { + steps { + sh "./gradlew --no-daemon jar deployNodes" + } + } + } + + post { + success { + script { + pullRequest.createStatus( + status: 'success', + context: "${PR_CONTEXT_STRING}", + description: 'Code checks passed', + targetUrl: "${env.BUILD_URL}") + } + } + + failure { + script { + pullRequest.createStatus( + status: 'failure', + context: "${PR_CONTEXT_STRING}", + description: 'Code checks failed', + targetUrl: "${env.BUILD_URL}") + } + } + cleanup { + deleteDir() /* clean up our workspace */ + } + } +} diff --git a/build.gradle b/build.gradle index e1b3594f59..00cc0470be 100644 --- a/build.gradle +++ b/build.gradle @@ -61,8 +61,8 @@ buildscript { ext.asm_version = '7.1' ext.artemis_version = '2.6.2' - // TODO Upgrade Jackson only when corda is using kotlin 1.3.10 - ext.jackson_version = '2.9.7' + // TODO Upgrade to Jackson 2.10+ only when corda is using kotlin 1.3.10 + ext.jackson_version = '2.9.8' ext.jetty_version = '9.4.19.v20190610' ext.jersey_version = '2.25' ext.servlet_version = '4.0.1' From 8cf3fa4ac85c7a21e34d1602491cac5297fafee3 Mon Sep 17 00:00:00 2001 From: Ross Nicoll Date: Wed, 22 Jul 2020 15:35:03 +0000 Subject: [PATCH 2/5] CORDA-3916 Update to BouncyCastle 1.61 (#6492) Update to BouncyCastle 1.61. Updating one version at a time to mitigate risk of a complex breaking change being introduced. --- constants.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/constants.properties b/constants.properties index 915b7e141f..6cf6942c4d 100644 --- a/constants.properties +++ b/constants.properties @@ -22,7 +22,7 @@ quasarVersion11=0.8.0 # Specify a classifier for Java 11 built artifacts jdkClassifier11=jdk11 proguardVersion=6.1.1 -bouncycastleVersion=1.60 +bouncycastleVersion=1.61 classgraphVersion=4.8.41 disruptorVersion=3.4.2 typesafeConfigVersion=1.3.4 From 416d27a909ad07f0f2cac5fe2d149a380d8fb210 Mon Sep 17 00:00:00 2001 From: Ross Nicoll Date: Tue, 11 Aug 2020 23:42:00 +0100 Subject: [PATCH 3/5] CORDA-3982 Revert "CORDA-3917 Update to Jackson 2.9.8 (#6493)" (#6615) This reverts commit cd4626d8c2b538154530f8074fb2769c1ac77383. --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 00cc0470be..e1b3594f59 100644 --- a/build.gradle +++ b/build.gradle @@ -61,8 +61,8 @@ buildscript { ext.asm_version = '7.1' ext.artemis_version = '2.6.2' - // TODO Upgrade to Jackson 2.10+ only when corda is using kotlin 1.3.10 - ext.jackson_version = '2.9.8' + // TODO Upgrade Jackson only when corda is using kotlin 1.3.10 + ext.jackson_version = '2.9.7' ext.jetty_version = '9.4.19.v20190610' ext.jersey_version = '2.25' ext.servlet_version = '4.0.1' From 21985243159f7797fbd507d96819c531f7f22ee6 Mon Sep 17 00:00:00 2001 From: Joseph Zuniga-Daly <59851625+josephzunigadaly@users.noreply.github.com> Date: Wed, 19 Aug 2020 10:41:51 +0100 Subject: [PATCH 4/5] CORDA-3824: Fix property rename in AMQP object evolution (#6616) * CORDA-3824: Add unit tests * CORDA-3824: Fix property rename in AMQP object evolution * Rename deserializedException to deserializedObject * Rename test class to EvolutionObjectBuilderRenamedPropertyTests * Added descriptions of the different object evolution stages in this test * Rename file containing the serialized object * Regenerate serialized data * Add a comment explaining the commented out code. * Restrict new behaviour to EvolutionObjectBuilder and simplify the loop that builds constructor slots. --- .../internal/amqp/ObjectBuilder.kt | 20 +++- ...lutionObjectBuilderRenamedPropertyTests.kt | 90 ++++++++++++++++++ ...ionObjectBuilderRenamedPropertyTests.Step1 | Bin 0 -> 1437 bytes 3 files changed, 105 insertions(+), 5 deletions(-) create mode 100644 serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EvolutionObjectBuilderRenamedPropertyTests.kt create mode 100644 serialization/src/test/resources/net/corda/serialization/internal/amqp/EvolutionObjectBuilderRenamedPropertyTests.Step1 diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectBuilder.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectBuilder.kt index ab1150ffff..056cfc1197 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectBuilder.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectBuilder.kt @@ -69,7 +69,7 @@ interface ObjectBuilder { * Create an [ObjectBuilderProvider] for the given [LocalTypeInformation.Composable]. */ fun makeProvider(typeInformation: LocalTypeInformation.Composable): ObjectBuilderProvider = - makeProvider(typeInformation.typeIdentifier, typeInformation.constructor, typeInformation.properties) + makeProvider(typeInformation.typeIdentifier, typeInformation.constructor, typeInformation.properties, false) /** * Create an [ObjectBuilderProvider] for the given type, constructor and set of properties. @@ -79,11 +79,12 @@ interface ObjectBuilder { */ fun makeProvider(typeIdentifier: TypeIdentifier, constructor: LocalConstructorInformation, - properties: Map): ObjectBuilderProvider = - if (constructor.hasParameters) makeConstructorBasedProvider(properties, typeIdentifier, constructor) + properties: Map, + includeAllConstructorParameters: Boolean): ObjectBuilderProvider = + if (constructor.hasParameters) makeConstructorBasedProvider(properties, typeIdentifier, constructor, includeAllConstructorParameters) else makeGetterSetterProvider(properties, typeIdentifier, constructor) - private fun makeConstructorBasedProvider(properties: Map, typeIdentifier: TypeIdentifier, constructor: LocalConstructorInformation): ObjectBuilderProvider { + private fun makeConstructorBasedProvider(properties: Map, typeIdentifier: TypeIdentifier, constructor: LocalConstructorInformation, includeAllConstructorParameters: Boolean): ObjectBuilderProvider { val constructorIndices = properties.mapValues { (name, property) -> when (property) { is LocalPropertyInformation.ConstructorPairedProperty -> property.constructorSlot.parameterIndex @@ -94,6 +95,15 @@ interface ObjectBuilder { "but property $name is not constructor-paired" ) } + }.toMutableMap() + + if (includeAllConstructorParameters) { + // Add constructor parameters not in the list of properties + // so we can use them in object evolution + for ((parameterIndex, parameter) in constructor.parameters.withIndex()) { + // Only use the parameters not already matched to properties + constructorIndices.putIfAbsent(parameter.name, parameterIndex) + } } val propertySlots = constructorIndices.keys.mapIndexed { slot, name -> name to slot }.toMap() @@ -203,7 +213,7 @@ class EvolutionObjectBuilder(private val localBuilder: ObjectBuilder, localProperties: Map, remoteTypeInformation: RemoteTypeInformation.Composable, mustPreserveData: Boolean): () -> ObjectBuilder { - val localBuilderProvider = ObjectBuilder.makeProvider(typeIdentifier, constructor, localProperties) + val localBuilderProvider = ObjectBuilder.makeProvider(typeIdentifier, constructor, localProperties, true) val remotePropertyNames = remoteTypeInformation.properties.keys.sorted() val reroutedIndices = remotePropertyNames.map { propertyName -> diff --git a/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EvolutionObjectBuilderRenamedPropertyTests.kt b/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EvolutionObjectBuilderRenamedPropertyTests.kt new file mode 100644 index 0000000000..0e7f795839 --- /dev/null +++ b/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EvolutionObjectBuilderRenamedPropertyTests.kt @@ -0,0 +1,90 @@ +package net.corda.serialization.internal.amqp + +import net.corda.core.contracts.BelongsToContract +import net.corda.core.contracts.Contract +import net.corda.core.contracts.ContractState +import net.corda.core.identity.AbstractParty +import net.corda.core.serialization.DeprecatedConstructorForDeserialization +import net.corda.core.serialization.SerializedBytes +import net.corda.core.transactions.LedgerTransaction +import net.corda.serialization.internal.amqp.testutils.deserialize +import net.corda.serialization.internal.amqp.testutils.serialize +import net.corda.serialization.internal.amqp.testutils.testDefaultFactory +import net.corda.serialization.internal.amqp.testutils.writeTestResource +import org.assertj.core.api.Assertions +import org.junit.Test + +class EvolutionObjectBuilderRenamedPropertyTests +{ + private val cordappVersionTestValue = 38854445 + private val dataTestValue = "d7af8af0-c10e-45bc-a5f7-92de432be0ef" + private val xTestValue = 7568055 + private val yTestValue = 4113687 + + class TemplateContract : Contract { + override fun verify(tx: LedgerTransaction) { } + } + + /** + * Step 1 + * + * This is the original class definition in object evolution. + */ +// @BelongsToContract(TemplateContract::class) +// data class TemplateState(val cordappVersion: Int, val data: String, val x : Int?, override val participants: List = listOf()) : ContractState + + /** + * Step 2 + * + * This is an intermediate class definition in object evolution. + * The y property has been added and a constructor copies the value of x into y. x is now set to null by the constructor. + */ +// @BelongsToContract(TemplateContract::class) +// data class TemplateState(val cordappVersion: Int, val data: String, val x : Int?, val y : String?, override val participants: List = listOf()) : ContractState { +// @DeprecatedConstructorForDeserialization(1) +// constructor(cordappVersion: Int, data : String, x : Int?, participants: List) +// : this(cordappVersion, data, null, x?.toString(), participants) +// } + + /** + * Step 3 + * + * This is the final class definition in object evolution. + * The x property has been removed but the constructor that copies values of x into y still exists. We expect previous versions of this + * object to pass the value of x to the constructor when deserialized. + */ + @BelongsToContract(TemplateContract::class) + data class TemplateState(val cordappVersion: Int, val data: String, val y : String?, override val participants: List = listOf()) : ContractState { + @DeprecatedConstructorForDeserialization(1) + constructor(cordappVersion: Int, data : String, x : Int?, participants: List) : this(cordappVersion, data, x?.toString(), participants) + } + + @Test(timeout=300_000) + fun `Step 1 to Step 3`() { + + // The next two commented lines are how the serialized data is generated. To regenerate the data, uncomment these along + // with the correct version of the class and rerun the test. This will generate a new file in the project resources. + +// val step1 = TemplateState(cordappVersionTestValue, dataTestValue, xTestValue) +// saveSerializedObject(step1) + + // serialization/src/test/resources/net/corda/serialization/internal/amqp/EvolutionObjectBuilderRenamedPropertyTests.Step1 + val bytes = this::class.java.getResource("EvolutionObjectBuilderRenamedPropertyTests.Step1").readBytes() + + val serializerFactory: SerializerFactory = testDefaultFactory() + val deserializedObject = DeserializationInput(serializerFactory) + .deserialize(SerializedBytes(bytes)) + + Assertions.assertThat(deserializedObject.cordappVersion).isEqualTo(cordappVersionTestValue) + Assertions.assertThat(deserializedObject.data).isEqualTo(dataTestValue) +// Assertions.assertThat(deserializedObject.x).isEqualTo(xTestValue) + Assertions.assertThat(deserializedObject.y).isEqualTo(xTestValue.toString()) + Assertions.assertThat(deserializedObject).isInstanceOf(TemplateState::class.java) + } + + /** + * Write serialized object to resources folder + */ + @Suppress("unused") + fun saveSerializedObject(obj : T) = writeTestResource(SerializationOutput(testDefaultFactory()).serialize(obj)) +} \ No newline at end of file diff --git a/serialization/src/test/resources/net/corda/serialization/internal/amqp/EvolutionObjectBuilderRenamedPropertyTests.Step1 b/serialization/src/test/resources/net/corda/serialization/internal/amqp/EvolutionObjectBuilderRenamedPropertyTests.Step1 new file mode 100644 index 0000000000000000000000000000000000000000..c2a94d7f11f57bc2446a00d6cd6ec0e57d769e78 GIT binary patch literal 1437 zcmb_c%}(1u5VjMW2&qD}46cZ9mpdyuVYDyph11(e>Dy{8xw66bRuY<#> zhpOtmx87ELh+emc-unhz`xxD%)Fe?N(W;l#Xl8c4pIO@sA`9>w$G!V*u#@BX9~>us zU_qYy^2mo+GW#7*(Q!j=dHc;u(&{#Kr*rP0LwToGqvt{=r(YMzqE!X#|`MfoxZcTV)HbSIPz~L3?E6Md}4UY_zu0eaBlp!YObXLAzGdI$V79H6xT`goK|r z%!F$e6tbGgYW#ba63Ls4s}#cs0T*2Xi~?UmK8BGGTnTucP|~`AoAmQXh67EkCdjp5 zbOL?gK}(N<5JtH762>@Qdmm@AN3j*gnQrn#c0dG*KS-fKI{ou88ZOLej%({x z;a!$f%J#3c!=PCQU s4Xs0@mN Date: Wed, 19 Aug 2020 17:18:39 +0100 Subject: [PATCH 5/5] Extract method to fix detekt ComplexMethod failure --- .../serialization/internal/amqp/ObjectBuilder.kt | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectBuilder.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectBuilder.kt index 373f96ca8f..ffc2fae5b5 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectBuilder.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectBuilder.kt @@ -124,12 +124,7 @@ interface ObjectBuilder { }.toMutableMap() if (includeAllConstructorParameters) { - // Add constructor parameters not in the list of properties - // so we can use them in object evolution - for ((parameterIndex, parameter) in constructor.parameters.withIndex()) { - // Only use the parameters not already matched to properties - constructorIndices.putIfAbsent(parameter.name, parameterIndex) - } + addMissingConstructorParameters(constructorIndices, constructor) } val propertySlots = constructorIndices.keys.mapIndexed { slot, name -> name to slot }.toMap() @@ -139,6 +134,15 @@ interface ObjectBuilder { } } + private fun addMissingConstructorParameters(constructorIndices: MutableMap, constructor: LocalConstructorInformation) { + // Add constructor parameters not in the list of properties + // so we can use them in object evolution + for ((parameterIndex, parameter) in constructor.parameters.withIndex()) { + // Only use the parameters not already matched to properties + constructorIndices.putIfAbsent(parameter.name, parameterIndex) + } + } + private fun makeSetterBasedProvider( properties: Map, typeIdentifier: TypeIdentifier,