CORDA-3489 State Evolution: Support adding new mandatory field and removal of optional (#5817)

* Reproduce

* Another failing test

* rename test

* rename test

* slot mapping

* pass all tests

* remove comments

* refactor

* broken test

* broken test

* detekt

* simplify

* simplify

* detekt baseline cleanup

* Add check

* requireForSer

* simplify

* Remove check

* use indices

detekt-baseline.xml

@@ -215,10 +215,6 @@
     <ID>EmptyCatchBlock:ScheduledFlowIntegrationTests.kt$ScheduledFlowIntegrationTests${ }</ID>
     <ID>EmptyCatchBlock:TransactionCallbackTest.kt$TransactionCallbackTest${ }</ID>
     <ID>EmptyCatchBlock:WebServer.kt$WebServer${ }</ID>
-    <ID>EmptyClassBlock:ClassLoadingUtilsTest.kt$ClassLoadingUtilsTest$BaseInterface</ID>
-    <ID>EmptyClassBlock:ClassLoadingUtilsTest.kt$ClassLoadingUtilsTest$BaseInterface2</ID>
-    <ID>EmptyClassBlock:ClassLoadingUtilsTest.kt$ClassLoadingUtilsTest$ConcreteClassWithEmptyConstructor : BaseInterface</ID>
-    <ID>EmptyClassBlock:ClassLoadingUtilsTest.kt$ClassLoadingUtilsTest$ConcreteClassWithNonEmptyConstructor : BaseInterface2</ID>
     <ID>EmptyClassBlock:CordaRPCClient.kt$CordaRPCClient$Companion</ID>
     <ID>EmptyDefaultConstructor:FlowRetryTest.kt$AsyncRetryFlow$()</ID>
     <ID>EmptyDefaultConstructor:FlowRetryTest.kt$RetryFlow$()</ID>
@@ -545,7 +541,6 @@
     <ID>FunctionNaming:Currencies.kt$fun SWISS_FRANCS(amount: Double): Amount&lt;Currency&gt;</ID>
     <ID>FunctionNaming:Currencies.kt$fun SWISS_FRANCS(amount: Int): Amount&lt;Currency&gt;</ID>
     <ID>FunctionNaming:Currencies.kt$fun SWISS_FRANCS(amount: Long): Amount&lt;Currency&gt;</ID>
-    <ID>FunctionNaming:InteractiveShell.kt$InteractiveShell$private fun _startShell(configuration: ShellConfiguration, classLoader: ClassLoader? = null)</ID>
     <ID>FunctionNaming:JacksonSupportTest.kt$JacksonSupportTest$@Test fun AnonymousParty()</ID>
     <ID>FunctionNaming:JacksonSupportTest.kt$JacksonSupportTest$@Test fun ByteSequence()</ID>
     <ID>FunctionNaming:JacksonSupportTest.kt$JacksonSupportTest$@Test fun CertPath()</ID>
@@ -755,7 +750,6 @@
     <ID>LongParameterList:X509Utilities.kt$X509Utilities$(certificateType: CertificateType, issuer: X500Principal, issuerPublicKey: PublicKey, subject: X500Principal, subjectPublicKey: PublicKey, validityWindow: Pair&lt;Date, Date&gt;, nameConstraints: NameConstraints? = null, crlDistPoint: String? = null, crlIssuer: X500Name? = null)</ID>
     <ID>LongParameterList:X509Utilities.kt$X509Utilities$(certificateType: CertificateType, issuerCertificate: X509Certificate, issuerKeyPair: KeyPair, subject: X500Principal, subjectPublicKey: PublicKey, validityWindow: Pair&lt;Duration, Duration&gt; = DEFAULT_VALIDITY_WINDOW, nameConstraints: NameConstraints? = null, crlDistPoint: String? = null, crlIssuer: X500Name? = null)</ID>
     <ID>LongParameterList:internalAccessTestHelpers.kt$( inputs: List&lt;StateAndRef&lt;ContractState&gt;&gt;, outputs: List&lt;TransactionState&lt;ContractState&gt;&gt;, commands: List&lt;CommandWithParties&lt;CommandData&gt;&gt;, attachments: List&lt;Attachment&gt;, id: SecureHash, notary: Party?, timeWindow: TimeWindow?, privacySalt: PrivacySalt, networkParameters: NetworkParameters, references: List&lt;StateAndRef&lt;ContractState&gt;&gt;, componentGroups: List&lt;ComponentGroup&gt;? = null, serializedInputs: List&lt;SerializedStateAndRef&gt;? = null, serializedReferences: List&lt;SerializedStateAndRef&gt;? = null, isAttachmentTrusted: (Attachment) -&gt; Boolean )</ID>
-    <ID>MagicNumber:AMQPClientSerializationScheme.kt$AMQPClientSerializationScheme$128</ID>
     <ID>MagicNumber:AMQPClientSerializationScheme.kt$AMQPClientSerializationScheme.Companion$128</ID>
     <ID>MagicNumber:AMQPSerializationScheme.kt$AbstractAMQPSerializationScheme$128</ID>
     <ID>MagicNumber:AMQPServer.kt$AMQPServer$100</ID>
@@ -782,10 +776,6 @@
     <ID>MagicNumber:BFTSmart.kt$BFTSmart.Replica.&lt;no name provided&gt;$20000</ID>
     <ID>MagicNumber:BFTSmartConfigInternal.kt$3</ID>
     <ID>MagicNumber:BFTSmartConfigInternal.kt$BFTSmartConfigInternal$200</ID>
-    <ID>MagicNumber:BlobWriter.kt$3</ID>
-    <ID>MagicNumber:BlobWriter.kt$4</ID>
-    <ID>MagicNumber:BlobWriter.kt$5</ID>
-    <ID>MagicNumber:BlobWriter.kt$6</ID>
     <ID>MagicNumber:BootstrapperView.kt$BootstrapperView$4</ID>
     <ID>MagicNumber:BusinessCalendar.kt$BusinessCalendar.Companion$30.0</ID>
     <ID>MagicNumber:BusinessCalendar.kt$BusinessCalendar.Companion$360.0</ID>
@@ -1187,8 +1177,6 @@
     <ID>MagicNumber:TransactionDSLInterpreter.kt$TransactionDSL$30</ID>
     <ID>MagicNumber:TransactionUtils.kt$4</ID>
     <ID>MagicNumber:TransactionVerificationException.kt$TransactionVerificationException.ConstraintPropagationRejection$3</ID>
-    <ID>MagicNumber:TransactionVerifierServiceInternal.kt$Verifier$4</ID>
-    <ID>MagicNumber:TransactionVerifierServiceInternal.kt$Verifier$5</ID>
     <ID>MagicNumber:TransactionViewer.kt$TransactionViewer$15.0</ID>
     <ID>MagicNumber:TransactionViewer.kt$TransactionViewer$20.0</ID>
     <ID>MagicNumber:TransactionViewer.kt$TransactionViewer$200.0</ID>
@@ -1271,9 +1259,6 @@
     <ID>MaxLineLength:AMQPBridgeTest.kt$AMQPBridgeTest$private</ID>
     <ID>MaxLineLength:AMQPChannelHandler.kt$AMQPChannelHandler$eventProcessor = EventProcessor(ch, serverMode, localCert!!.subjectX500Principal.toString(), remoteCert!!.subjectX500Principal.toString(), userName, password)</ID>
     <ID>MaxLineLength:AMQPChannelHandler.kt$AMQPChannelHandler${ logWarnWithMDC("SSL Handshake closed early.") }</ID>
-    <ID>MaxLineLength:AMQPClientSerializationScheme.kt$AMQPClientSerializationScheme$@Suppress("UNUSED") constructor() : this(emptySet(), emptySet(), AccessOrderLinkedHashMap&lt;SerializationFactoryCacheKey, SerializerFactory&gt;(128).toSynchronised())</ID>
-    <ID>MaxLineLength:AMQPClientSerializationScheme.kt$AMQPClientSerializationScheme$constructor(cordapps: List&lt;Cordapp&gt;) : this(cordapps.customSerializers, cordapps.serializationWhitelists, AccessOrderLinkedHashMap&lt;SerializationFactoryCacheKey, SerializerFactory&gt;(128).toSynchronised())</ID>
-    <ID>MaxLineLength:AMQPClientSerializationScheme.kt$AMQPClientSerializationScheme$constructor(cordapps: List&lt;Cordapp&gt;, serializerFactoriesForContexts: MutableMap&lt;SerializationFactoryCacheKey, SerializerFactory&gt;) : this(cordapps.customSerializers, cordapps.serializationWhitelists, serializerFactoriesForContexts)</ID>
     <ID>MaxLineLength:AMQPClientSerializationScheme.kt$AMQPClientSerializationScheme$return SerializerFactoryBuilder.build(context.whitelist, context.deserializationClassLoader, context.lenientCarpenterEnabled).apply { register(RpcClientObservableDeSerializer) register(RpcClientCordaFutureSerializer(this)) register(RxNotificationSerializer(this)) }</ID>
     <ID>MaxLineLength:AMQPClientSerializationScheme.kt$AMQPClientSerializationScheme.Companion$ fun initialiseSerialization(classLoader: ClassLoader? = null, customSerializers: Set&lt;SerializationCustomSerializer&lt;*, *&gt;&gt; = emptySet(), serializationWhitelists: Set&lt;SerializationWhitelist&gt; = emptySet(), serializerFactoriesForContexts: MutableMap&lt;SerializationFactoryCacheKey, SerializerFactory&gt; = AccessOrderLinkedHashMap&lt;SerializationFactoryCacheKey, SerializerFactory&gt;(128).toSynchronised())</ID>
     <ID>MaxLineLength:AMQPClientSerializationScheme.kt$AMQPClientSerializationScheme.Companion$fun createSerializationEnv(classLoader: ClassLoader? = null, customSerializers: Set&lt;SerializationCustomSerializer&lt;*, *&gt;&gt; = emptySet(), serializationWhitelists: Set&lt;SerializationWhitelist&gt; = emptySet(), serializerFactoriesForContexts: MutableMap&lt;SerializationFactoryCacheKey, SerializerFactory&gt; = AccessOrderLinkedHashMap&lt;SerializationFactoryCacheKey, SerializerFactory&gt;(128).toSynchronised()): SerializationEnvironment</ID>
@@ -1487,8 +1472,6 @@
     <ID>MaxLineLength:BFTSmartNotaryService.kt$BFTSmartNotaryService$CommittedState : BaseComittedState</ID>
     <ID>MaxLineLength:BFTSmartNotaryService.kt$BFTSmartNotaryService.Replica$private</ID>
     <ID>MaxLineLength:BFTSmartNotaryService.kt$BFTSmartNotaryService.Replica$val response = verifyAndCommitTx(commitRequest.payload.coreTransaction, commitRequest.callerIdentity, commitRequest.payload.requestSignature)</ID>
-    <ID>MaxLineLength:BackpressureAwareTimedFlow.kt$BackpressureAwareTimedFlow$else -&gt; throw throw IllegalArgumentException("We were expecting a ${ReceiveType::class.java.name} or WaitTimeUpdate but we instead got a ${unwrapped.javaClass.name} ($unwrapped)")</ID>
-    <ID>MaxLineLength:BackpressureAwareTimedFlow.kt$BackpressureAwareTimedFlow$logger.info("Counterparty [${session.counterparty}] is busy - TimedFlow $runId has been asked to wait for an additional ${unwrapped.waitTime} seconds for completion.")</ID>
     <ID>MaxLineLength:BankOfCordaWebApi.kt$BankOfCordaWebApi$?:</ID>
     <ID>MaxLineLength:BankOfCordaWebApi.kt$BankOfCordaWebApi$rpc.startFlow(::CashIssueAndPaymentFlow, params.amount, issuerBankPartyRef, issueToParty, anonymous, notaryParty).returnValue.getOrThrow()</ID>
     <ID>MaxLineLength:BankOfCordaWebApi.kt$BankOfCordaWebApi$rpc.wellKnownPartyFromX500Name(params.issuerBankName) ?: return Response.status(Response.Status.FORBIDDEN).entity("Unable to locate ${params.issuerBankName} in identity service").build()</ID>
@@ -2008,7 +1991,6 @@
     <ID>MaxLineLength:DBRunnerExtension.kt$DBRunnerExtension : ExtensionBeforeAllCallbackAfterAllCallbackBeforeEachCallbackAfterEachCallback</ID>
     <ID>MaxLineLength:DBTransactionMappingStorage.kt$DBTransactionMappingStorage$cq.multiselect(from.get&lt;String&gt;(DBTransactionStorage.DBTransaction::stateMachineRunId.name), from.get&lt;String&gt;(DBTransactionStorage.DBTransaction::txId.name))</ID>
     <ID>MaxLineLength:DBTransactionMappingStorage.kt$DBTransactionMappingStorage$val flowIds = session.createQuery(cq).resultList.map { StateMachineTransactionMapping(StateMachineRunId(UUID.fromString(it[0] as String)), SecureHash.parse(it[1] as String)) }</ID>
-    <ID>MaxLineLength:DBTransactionStorage.kt$DBTransactionStorage : WritableTransactionStorageSingletonSerializeAsToken</ID>
     <ID>MaxLineLength:DBTransactionStorage.kt$DBTransactionStorage.TransactionStatus$UnexpectedStatusValueException : Exception</ID>
     <ID>MaxLineLength:DBTransactionStorageTests.kt$DBTransactionStorageTests$listOf(TransactionSignature(ByteArray(1), ALICE_PUBKEY, SignatureMetadata(1, Crypto.findSignatureScheme(ALICE_PUBKEY).schemeNumberID)))</ID>
     <ID>MaxLineLength:DatabaseTransaction.kt$get() = if (_prohibitDatabaseAccess.get() == true) throw IllegalAccessException("Database access is disabled in this context.") else _contextTransaction.get()</ID>
@@ -2848,7 +2830,6 @@
     <ID>MaxLineLength:OGSwapPricingExample.kt$SwapPricingExample$val receiveLeg = RateCalculationSwapLeg.builder().payReceive(PayReceive.RECEIVE).accrualSchedule(PeriodicSchedule.builder().startDate(LocalDate.of(2014, 9, 12)).endDate(LocalDate.of(2016, 7, 12)).stubConvention(StubConvention.SHORT_INITIAL).frequency(Frequency.P6M).businessDayAdjustment(BusinessDayAdjustment.of(MODIFIED_FOLLOWING, HolidayCalendarIds.USNY)).build()).paymentSchedule(PaymentSchedule.builder().paymentFrequency(Frequency.P6M).paymentDateOffset(DaysAdjustment.NONE).build()).notionalSchedule(notional).calculation(FixedRateCalculation.of(0.01, DayCounts.THIRTY_U_360)).build()</ID>
     <ID>MaxLineLength:OGSwapPricingExample.kt$SwapPricingExample$val receiveLeg = RateCalculationSwapLeg.builder().payReceive(PayReceive.RECEIVE).accrualSchedule(PeriodicSchedule.builder().startDate(LocalDate.of(2014, 9, 12)).endDate(LocalDate.of(2020, 9, 12)).frequency(Frequency.P3M).businessDayAdjustment(BusinessDayAdjustment.of(MODIFIED_FOLLOWING, HolidayCalendarIds.USNY)).build()).paymentSchedule(PaymentSchedule.builder().paymentFrequency(Frequency.P3M).paymentDateOffset(DaysAdjustment.NONE).build()).notionalSchedule(notional).calculation(OvernightRateCalculation.builder().index(OvernightIndices.USD_FED_FUND).accrualMethod(OvernightAccrualMethod.AVERAGED).build()).build()</ID>
     <ID>MaxLineLength:OGSwapPricingExample.kt$SwapPricingExample$val receiveLeg = RateCalculationSwapLeg.builder().payReceive(PayReceive.RECEIVE).accrualSchedule(PeriodicSchedule.builder().startDate(LocalDate.of(2014, 9, 12)).endDate(LocalDate.of(2021, 9, 12)).frequency(Frequency.P3M).businessDayAdjustment(BusinessDayAdjustment.of(MODIFIED_FOLLOWING, HolidayCalendarIds.USNY)).build()).paymentSchedule(PaymentSchedule.builder().paymentFrequency(Frequency.TERM).paymentDateOffset(DaysAdjustment.NONE).compoundingMethod(CompoundingMethod.STRAIGHT).build()).notionalSchedule(notional).calculation(IborRateCalculation.of(IborIndices.USD_LIBOR_3M)).build()</ID>
-    <ID>MaxLineLength:ObjectBuilder.kt$ObjectBuilder.Companion$private</ID>
     <ID>MaxLineLength:ObjectSerializer.kt$AbstractObjectSerializer$override</ID>
     <ID>MaxLineLength:ObjectSerializer.kt$ComposableObjectSerializer$override</ID>
     <ID>MaxLineLength:ObjectSerializer.kt$EvolutionObjectSerializer$override</ID>
@@ -3774,7 +3755,6 @@
     <ID>NestedBlockDepth:StartedFlowTransition.kt$StartedFlowTransition$private fun TransitionBuilder.sendToSessionsTransition(sourceSessionIdToMessage: Map&lt;SessionId, SerializedBytes&lt;Any&gt;&gt;)</ID>
     <ID>NestedBlockDepth:StatusTransitions.kt$StatusTransitions$ fun verify(tx: LedgerTransaction)</ID>
     <ID>NestedBlockDepth:ThrowableSerializer.kt$ThrowableSerializer$override fun fromProxy(proxy: ThrowableProxy): Throwable</ID>
-    <ID>NestedBlockDepth:TransactionVerifierServiceInternal.kt$Verifier$ private fun verifyConstraints(contractAttachmentsByContract: Map&lt;ContractClassName, ContractAttachment&gt;)</ID>
     <ID>NestedBlockDepth:TransactionVerifierServiceInternal.kt$Verifier$ private fun verifyConstraintsValidity(contractAttachmentsByContract: Map&lt;ContractClassName, ContractAttachment&gt;)</ID>
     <ID>SpreadOperator:AMQPSerializationScheme.kt$AbstractAMQPSerializationScheme$(*it.whitelist.toTypedArray())</ID>
     <ID>SpreadOperator:AbstractNode.kt$FlowStarterImpl$(logicType, *args)</ID>
@@ -4382,7 +4362,6 @@
     <ID>WildcardImport:ANSIProgressRendererTest.kt$import com.nhaarman.mockito_kotlin.*</ID>
     <ID>WildcardImport:AbstractCashFlow.kt$import net.corda.core.flows.*</ID>
     <ID>WildcardImport:AbstractCashSelection.kt$import net.corda.core.utilities.*</ID>
-    <ID>WildcardImport:ActionExecutorImpl.kt$import com.codahale.metrics.*</ID>
     <ID>WildcardImport:AdvancedExceptionDialog.kt$import javafx.scene.control.*</ID>
     <ID>WildcardImport:AffinityExecutorTests.kt$import kotlin.test.*</ID>
     <ID>WildcardImport:AliasPrivateKey.kt$import org.bouncycastle.asn1.*</ID>
@@ -4533,11 +4512,8 @@
     <ID>WildcardImport:CordappSmokeTest.kt$import net.corda.core.flows.*</ID>
     <ID>WildcardImport:CordappSmokeTest.kt$import net.corda.core.internal.*</ID>
     <ID>WildcardImport:CoreFlowHandlers.kt$import net.corda.core.flows.*</ID>
-    <ID>WildcardImport:Crypto.kt$import java.security.*</ID>
-    <ID>WildcardImport:Crypto.kt$import net.corda.core.crypto.internal.*</ID>
     <ID>WildcardImport:CryptoSignUtils.kt$import java.security.*</ID>
     <ID>WildcardImport:CryptoSignUtils.kt$import net.corda.core.crypto.*</ID>
-    <ID>WildcardImport:CryptoUtils.kt$import java.security.*</ID>
     <ID>WildcardImport:CryptoUtilsTest.kt$import kotlin.test.*</ID>
     <ID>WildcardImport:CustomCordapp.kt$import net.corda.core.internal.*</ID>
     <ID>WildcardImport:CustomVaultQuery.kt$import net.corda.core.flows.*</ID>
@@ -4586,7 +4562,6 @@
     <ID>WildcardImport:EvolutionSerializerFactory.kt$import net.corda.serialization.internal.model.*</ID>
     <ID>WildcardImport:EvolutionSerializerFactoryTests.kt$import kotlin.test.*</ID>
     <ID>WildcardImport:EvolutionSerializerFactoryTests.kt$import net.corda.serialization.internal.amqp.testutils.*</ID>
-    <ID>WildcardImport:EvolvabilityTests.kt$import net.corda.serialization.internal.amqp.testutils.*</ID>
     <ID>WildcardImport:Explorer.kt$import tornadofx.*</ID>
     <ID>WildcardImport:FiberDeserializationCheckingInterceptor.kt$import net.corda.node.services.statemachine.*</ID>
     <ID>WildcardImport:FinalityFlowMigration.kt$import net.corda.core.flows.*</ID>
@@ -4598,7 +4573,6 @@
     <ID>WildcardImport:FixingFlow.kt$import net.corda.core.contracts.*</ID>
     <ID>WildcardImport:FixingFlow.kt$import net.corda.core.flows.*</ID>
     <ID>WildcardImport:FixingFlow.kt$import net.corda.core.utilities.*</ID>
-    <ID>WildcardImport:FlowAsyncOperationTests.kt$import net.corda.testing.node.internal.*</ID>
     <ID>WildcardImport:FlowCheckpointCordapp.kt$import net.corda.core.flows.*</ID>
     <ID>WildcardImport:FlowCheckpointVersionNodeStartupCheckTest.kt$import net.corda.core.flows.*</ID>
     <ID>WildcardImport:FlowCheckpointVersionNodeStartupCheckTest.kt$import net.corda.core.internal.*</ID>
@@ -4837,8 +4811,6 @@
     <ID>WildcardImport:NotaryWhitelistTests.kt$import net.corda.testing.node.internal.*</ID>
     <ID>WildcardImport:OGSwapPricingExample.kt$import com.opengamma.strata.product.swap.*</ID>
     <ID>WildcardImport:OGTrade.kt$import net.corda.core.contracts.*</ID>
-    <ID>WildcardImport:ObjectBuilder.kt$import net.corda.serialization.internal.model.*</ID>
-    <ID>WildcardImport:ObjectSerializer.kt$import net.corda.serialization.internal.model.*</ID>
     <ID>WildcardImport:ObligationTests.kt$import net.corda.core.contracts.*</ID>
     <ID>WildcardImport:ObligationTests.kt$import net.corda.finance.*</ID>
     <ID>WildcardImport:ObligationTests.kt$import net.corda.testing.core.*</ID>
@@ -4944,7 +4916,6 @@
     <ID>WildcardImport:SerializeAsTokenContextImpl.kt$import net.corda.core.serialization.*</ID>
     <ID>WildcardImport:SerializerFactoryBuilder.kt$import net.corda.serialization.internal.model.*</ID>
     <ID>WildcardImport:ServiceHub.kt$import net.corda.core.contracts.*</ID>
-    <ID>WildcardImport:ServiceHub.kt$import net.corda.core.crypto.*</ID>
     <ID>WildcardImport:ServiceHub.kt$import net.corda.core.node.services.*</ID>
     <ID>WildcardImport:ServiceHubInternal.kt$import net.corda.core.internal.*</ID>
     <ID>WildcardImport:ServicesForResolutionImpl.kt$import net.corda.core.contracts.*</ID>
@@ -4983,7 +4954,6 @@
     <ID>WildcardImport:TimedFlowTests.kt$import net.corda.testing.node.internal.*</ID>
     <ID>WildcardImport:TlsDiffAlgorithmsTest.kt$import javax.net.ssl.*</ID>
     <ID>WildcardImport:TlsDiffProtocolsTest.kt$import javax.net.ssl.*</ID>
-    <ID>WildcardImport:TopLevelTransition.kt$import net.corda.node.services.statemachine.*</ID>
     <ID>WildcardImport:TraderDemoTest.kt$import net.corda.testing.driver.*</ID>
     <ID>WildcardImport:TransactionBuilder.kt$import net.corda.core.contracts.*</ID>
     <ID>WildcardImport:TransactionBuilder.kt$import net.corda.core.internal.*</ID>
@@ -5009,7 +4979,6 @@
     <ID>WildcardImport:TransactionViewer.kt$import net.corda.client.jfx.utils.*</ID>
     <ID>WildcardImport:TransactionViewer.kt$import net.corda.core.contracts.*</ID>
     <ID>WildcardImport:TransactionViewer.kt$import tornadofx.*</ID>
-    <ID>WildcardImport:TransitionBuilder.kt$import net.corda.node.services.statemachine.*</ID>
     <ID>WildcardImport:TutorialContract.kt$import net.corda.core.contracts.*</ID>
     <ID>WildcardImport:TutorialTestDSL.kt$import net.corda.testing.core.*</ID>
     <ID>WildcardImport:TwoPartyDealFlow.kt$import net.corda.core.flows.*</ID>

serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectBuilder.kt

@@ -1,6 +1,10 @@
 package net.corda.serialization.internal.amqp
 
-import net.corda.serialization.internal.model.*
+import net.corda.serialization.internal.model.LocalConstructorInformation
+import net.corda.serialization.internal.model.LocalPropertyInformation
+import net.corda.serialization.internal.model.LocalTypeInformation
+import net.corda.serialization.internal.model.RemoteTypeInformation
+import net.corda.serialization.internal.model.TypeIdentifier
 import java.io.NotSerializableException
 import java.lang.reflect.Constructor
 import java.lang.reflect.InvocationTargetException
@@ -14,34 +18,38 @@ private const val IGNORE_COMPUTED = -1
  * @property propertySlots The slot indices of the properties written by the provided [ObjectBuilder], by property name.
  * @param provider The thunk that provides a new, empty [ObjectBuilder]
  */
-data class ObjectBuilderProvider(val propertySlots: Map<String, Int>, private val provider: () -> ObjectBuilder)
+data class ObjectBuilderProvider(
-    : () -> ObjectBuilder by provider
+        val propertySlots: Map<String, Int>,
+        private val provider: () -> ObjectBuilder
+) : () -> ObjectBuilder by provider
 
 /**
  * Wraps the operation of calling a constructor, with helpful exception handling.
  */
-private class ConstructorCaller(private val javaConstructor: Constructor<Any>): (Array<Any?>) -> Any {
+private class ConstructorCaller(private val javaConstructor: Constructor<Any>) : (Array<Any?>) -> Any {
 
     override fun invoke(parameters: Array<Any?>): Any =
-        try {
+            try {
-            javaConstructor.newInstance(*parameters)
+                javaConstructor.newInstance(*parameters)
-        } catch (e: InvocationTargetException) {
+            } catch (e: InvocationTargetException) {
-            @Suppress("DEPRECATION")    // JDK11: isAccessible() should be replaced with canAccess() (since 9)
+                @Suppress("DEPRECATION")    // JDK11: isAccessible() should be replaced with canAccess() (since 9)
-            throw NotSerializableException(
+                throw NotSerializableException(
-                    "Constructor for ${javaConstructor.declaringClass} (isAccessible=${javaConstructor.isAccessible}) " +
+                        "Constructor for ${javaConstructor.declaringClass} (isAccessible=${javaConstructor.isAccessible}) " +
-                    "failed when called with parameters ${parameters.toList()}: ${e.cause!!.message}")
+                                "failed when called with parameters ${parameters.toList()}: ${e.cause!!.message}"
-        } catch (e: IllegalAccessException) {
+                )
-            @Suppress("DEPRECATION")    // JDK11: isAccessible() should be replaced with canAccess() (since 9)
+            } catch (e: IllegalAccessException) {
-            throw NotSerializableException(
+                @Suppress("DEPRECATION")    // JDK11: isAccessible() should be replaced with canAccess() (since 9)
-                    "Constructor for ${javaConstructor.declaringClass} (isAccessible=${javaConstructor.isAccessible}) " +
+                throw NotSerializableException(
-                    "not accessible: ${e.message}")
+                        "Constructor for ${javaConstructor.declaringClass} (isAccessible=${javaConstructor.isAccessible}) " +
-        }
+                                "not accessible: ${e.message}"
+                )
+            }
 }
 
 /**
  * Wraps the operation of calling a setter, with helpful exception handling.
  */
-private class SetterCaller(val setter: Method): (Any, Any?) -> Unit {
+private class SetterCaller(val setter: Method) : (Any, Any?) -> Unit {
     override fun invoke(target: Any, value: Any?) {
         try {
             setter.invoke(target, value)
@@ -49,12 +57,14 @@ private class SetterCaller(val setter: Method): (Any, Any?) -> Unit {
             @Suppress("DEPRECATION")    // JDK11: isAccessible() should be replaced with canAccess() (since 9)
             throw NotSerializableException(
                     "Setter ${setter.declaringClass}.${setter.name} (isAccessible=${setter.isAccessible} " +
-                            "failed when called with parameter $value: ${e.cause!!.message}")
+                            "failed when called with parameter $value: ${e.cause!!.message}"
+            )
         } catch (e: IllegalAccessException) {
             @Suppress("DEPRECATION")    // JDK11: isAccessible() should be replaced with canAccess() (since 9)
             throw NotSerializableException(
                     "Setter ${setter.declaringClass}.${setter.name} (isAccessible=${setter.isAccessible} " +
-                    "not accessible: ${e.message}")
+                            "not accessible: ${e.message}"
+            )
         }
     }
 }
@@ -75,15 +85,30 @@ interface ObjectBuilder {
          * Create an [ObjectBuilderProvider] for the given type, constructor and set of properties.
          *
          * The [EvolutionObjectBuilder] uses this to create [ObjectBuilderProvider]s for objects initialised via an
-         * evolution constructor (i.e. a constructor annotated with [DeprecatedConstructorForDeserialization]).
+         * evolution constructor (i.e. a constructor annotated with [net.corda.core.serialization.DeprecatedConstructorForDeserialization]).
          */
-        fun makeProvider(typeIdentifier: TypeIdentifier,
+        fun makeProvider(
-                         constructor: LocalConstructorInformation,
+                typeIdentifier: TypeIdentifier,
-                         properties: Map<String, LocalPropertyInformation>): ObjectBuilderProvider =
+                constructor: LocalConstructorInformation,
-            if (constructor.hasParameters) makeConstructorBasedProvider(properties, typeIdentifier, constructor)
+                properties: Map<String, LocalPropertyInformation>
-            else makeGetterSetterProvider(properties, typeIdentifier, constructor)
+        ): ObjectBuilderProvider =
+                if (constructor.hasParameters) makeConstructorBasedProvider(properties, typeIdentifier, constructor)
+                else makeSetterBasedProvider(properties, typeIdentifier, constructor)
 
-        private fun makeConstructorBasedProvider(properties: Map<String, LocalPropertyInformation>, typeIdentifier: TypeIdentifier, constructor: LocalConstructorInformation): ObjectBuilderProvider {
+        private fun makeConstructorBasedProvider(
+                properties: Map<String, LocalPropertyInformation>,
+                typeIdentifier: TypeIdentifier,
+                constructor: LocalConstructorInformation
+        ): ObjectBuilderProvider {
+            requireForSer(properties.values.all {
+                when (it) {
+                    is LocalPropertyInformation.ConstructorPairedProperty ->
+                        it.constructorSlot.constructorInformation == constructor
+                    is LocalPropertyInformation.PrivateConstructorPairedProperty ->
+                        it.constructorSlot.constructorInformation == constructor
+                    else -> true
+                }
+            }) { "Constructor passed in must match the constructor the properties are referring to" }
             val constructorIndices = properties.mapValues { (name, property) ->
                 when (property) {
                     is LocalPropertyInformation.ConstructorPairedProperty -> property.constructorSlot.parameterIndex
@@ -99,11 +124,15 @@ interface ObjectBuilder {
             val propertySlots = constructorIndices.keys.mapIndexed { slot, name -> name to slot }.toMap()
 
             return ObjectBuilderProvider(propertySlots) {
-                ConstructorBasedObjectBuilder(ConstructorCaller(constructor.observedMethod), constructorIndices.values.toIntArray())
+                ConstructorBasedObjectBuilder(constructor, constructorIndices.values.toIntArray())
             }
         }
 
-        private fun makeGetterSetterProvider(properties: Map<String, LocalPropertyInformation>, typeIdentifier: TypeIdentifier, constructor: LocalConstructorInformation): ObjectBuilderProvider {
+        private fun makeSetterBasedProvider(
+                properties: Map<String, LocalPropertyInformation>,
+                typeIdentifier: TypeIdentifier,
+                constructor: LocalConstructorInformation
+        ): ObjectBuilderProvider {
             val setters = properties.mapValues { (name, property) ->
                 when (property) {
                     is LocalPropertyInformation.GetterSetterProperty -> SetterCaller(property.observedSetter)
@@ -145,7 +174,8 @@ interface ObjectBuilder {
  */
 private class SetterBasedObjectBuilder(
         private val constructor: ConstructorCaller,
-        private val setters: List<SetterCaller?>): ObjectBuilder {
+        private val setters: List<SetterCaller?>
+) : ObjectBuilder {
 
     private lateinit var target: Any
 
@@ -165,44 +195,65 @@ private class SetterBasedObjectBuilder(
  * and calling a constructor with those parameters to obtain the configured object instance.
  */
 private class ConstructorBasedObjectBuilder(
-        private val constructor: ConstructorCaller,
+        private val constructorInfo: LocalConstructorInformation,
-        private val parameterIndices: IntArray): ObjectBuilder {
+        private val slotToCtorArgIdx: IntArray
+) : ObjectBuilder {
 
-    private val params = arrayOfNulls<Any>(parameterIndices.count { it != IGNORE_COMPUTED })
+    private val constructor = ConstructorCaller(constructorInfo.observedMethod)
+    private val params = arrayOfNulls<Any>(constructorInfo.parameters.size)
+
+    init {
+        requireForSer(slotToCtorArgIdx.all { it in params.indices || it == IGNORE_COMPUTED }) {
+            "Argument indexes must be in ${params.indices}. Slot to arg indexes passed in are ${slotToCtorArgIdx.toList()}"
+        }
+    }
 
     override fun initialize() {}
 
     override fun populate(slot: Int, value: Any?) {
-        val parameterIndex = parameterIndices[slot]
+        val parameterIndex = slotToCtorArgIdx[slot]
         if (parameterIndex != IGNORE_COMPUTED) params[parameterIndex] = value
     }
 
-    override fun build(): Any = constructor.invoke(params)
+    override fun build(): Any {
+        // CORDA-3504
+        // The check below would cause failures, because in some cases objects ARE instantiated with
+        // parameters that are detected as mandatory but not actually set
+//        requireForSer(
+//                constructorInfo.parameters.zip(params)
+//                        .all { (param, value) -> !param.isMandatory || value != null }
+//        ) { "Some mandatory constructor parameters are not set" }
+        return constructor.invoke(params)
+    }
 }
 
 /**
  * An [ObjectBuilder] that wraps an underlying [ObjectBuilder], routing the property values assigned to its slots to the
  * matching slots in the underlying builder, and discarding values for which the underlying builder has no slot.
  */
-class EvolutionObjectBuilder(private val localBuilder: ObjectBuilder,
+class EvolutionObjectBuilder(
-                             private val slotAssignments: IntArray,
+        private val localBuilder: ObjectBuilder,
-                             private val remoteProperties: List<String>,
+        private val slotAssignments: IntArray,
-                             private val mustPreserveData: Boolean): ObjectBuilder {
+        private val remoteProperties: List<String>,
+        private val mustPreserveData: Boolean
+) : ObjectBuilder {
 
     companion object {
 
-        const val DISCARDED : Int = -1
+        const val DISCARDED: Int = -1
 
         /**
          * Construct an [EvolutionObjectBuilder] for the specified type, constructor and properties, mapping the list of
          * properties defined in the remote type into the matching slots on the local type's [ObjectBuilder], and discarding
          * any for which there is no matching slot.
          */
-        fun makeProvider(typeIdentifier: TypeIdentifier,
+        fun makeProvider(
-                         constructor: LocalConstructorInformation,
+                typeIdentifier: TypeIdentifier,
-                         localProperties: Map<String, LocalPropertyInformation>,
+                constructor: LocalConstructorInformation,
-                         remoteTypeInformation: RemoteTypeInformation.Composable,
+                localProperties: Map<String, LocalPropertyInformation>,
-                         mustPreserveData: Boolean): () -> ObjectBuilder {
+                remoteTypeInformation: RemoteTypeInformation.Composable,
+                mustPreserveData: Boolean
+        ): () -> ObjectBuilder {
             val localBuilderProvider = ObjectBuilder.makeProvider(typeIdentifier, constructor, localProperties)
 
             val remotePropertyNames = remoteTypeInformation.properties.keys.sorted()
@@ -215,7 +266,8 @@ class EvolutionObjectBuilder(private val localBuilder: ObjectBuilder,
                         localBuilderProvider(),
                         reroutedIndices,
                         remotePropertyNames,
-                        mustPreserveData)
+                        mustPreserveData
+                )
             }
         }
     }
@@ -230,7 +282,7 @@ class EvolutionObjectBuilder(private val localBuilder: ObjectBuilder,
             if (mustPreserveData && value != null) {
                 throw NotSerializableException(
                         "Non-null value $value provided for property ${remoteProperties[slot]}, " +
-                        "which is not supported in this version"
+                                "which is not supported in this version"
                 )
             }
         } else {
@@ -240,3 +292,7 @@ class EvolutionObjectBuilder(private val localBuilder: ObjectBuilder,
 
     override fun build(): Any = localBuilder.build()
 }
+
+private fun requireForSer(requirement: Boolean, message: () -> String) {
+    if (!requirement) throw NotSerializableException(message())
+}

serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectSerializer.kt

@@ -1,7 +1,13 @@
 package net.corda.serialization.internal.amqp
 
 import net.corda.core.serialization.SerializationContext
-import net.corda.serialization.internal.model.*
+import net.corda.serialization.internal.model.LocalConstructorInformation
+import net.corda.serialization.internal.model.LocalPropertyInformation
+import net.corda.serialization.internal.model.LocalTypeInformation
+import net.corda.serialization.internal.model.PropertyName
+import net.corda.serialization.internal.model.RemotePropertyInformation
+import net.corda.serialization.internal.model.RemoteTypeInformation
+import net.corda.serialization.internal.model.TypeIdentifier
 import org.apache.qpid.proton.amqp.Symbol
 import org.apache.qpid.proton.codec.Data
 import java.io.NotSerializableException
@@ -158,9 +164,9 @@ class ComposableObjectReader(
                 builder.initialize()
                 obj.asSequence().zip(propertySerializers.values.asSequence())
                         // Read _all_ properties from the stream
-                        .map { (item, property) -> property to property.readProperty(item, schemas, input, context) }
+                        .map { (item, property) -> property.readProperty(item, schemas, input, context) }
                         // Write them into the builder (computed properties will be thrown away)
-                        .forEachIndexed { slot, (_, propertyValue) -> builder.populate(slot, propertyValue) }
+                        .forEachIndexed { slot, propertyValue -> builder.populate(slot, propertyValue) }
                 return builder.build()
             }
 }
@@ -189,7 +195,8 @@ class EvolutionObjectSerializer(
 
     companion object {
         fun make(localTypeInformation: LocalTypeInformation.Composable,
-                 remoteTypeInformation: RemoteTypeInformation.Composable, constructor: LocalConstructorInformation,
+                 remoteTypeInformation: RemoteTypeInformation.Composable,
+                 constructor: LocalConstructorInformation,
                  properties: Map<String, LocalPropertyInformation>,
                  classLoader: ClassLoader,
                  mustPreserveData: Boolean): EvolutionObjectSerializer {

serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializerFactoryTests.kt

@@ -5,6 +5,7 @@ import net.corda.serialization.internal.AllWhitelist
 import net.corda.serialization.internal.amqp.testutils.*
 import net.corda.serialization.internal.carpenter.ClassCarpenterImpl
 import org.junit.Test
+import org.junit.jupiter.api.assertThrows
 import java.io.NotSerializableException
 import kotlin.test.*
 
@@ -14,13 +15,15 @@ class EvolutionSerializerFactoryTests {
             AllWhitelist,
             ClassCarpenterImpl(AllWhitelist, ClassLoader.getSystemClassLoader()),
             descriptorBasedSerializerRegistry = DefaultDescriptorBasedSerializerRegistry(),
-            mustPreserveDataWhenEvolving = false)
+            mustPreserveDataWhenEvolving = false
+    )
 
     private val strictFactory = SerializerFactoryBuilder.build(
             AllWhitelist,
             ClassCarpenterImpl(AllWhitelist, ClassLoader.getSystemClassLoader()),
             descriptorBasedSerializerRegistry = DefaultDescriptorBasedSerializerRegistry(),
-            mustPreserveDataWhenEvolving = true)
+            mustPreserveDataWhenEvolving = true
+    )
 
     // Version of the class as it was serialised
     //
@@ -56,13 +59,9 @@ class EvolutionSerializerFactoryTests {
         assertEquals(1, withNonNullTarget.a)
 
         // The strict factory cannot deserialize the evolved instance where the original value of 'b' is non-null.
-        try {
+        val e = assertThrows<NotSerializableException> {
             DeserializationInput(strictFactory).deserialize(SerializedBytes<C>(withoutNullUrl.readBytes()))
-            fail("Expected deserialisation of object with non-null value for 'b' to fail")
-        } catch (e: NotSerializableException) {
-            assertTrue(e.message!!.contains(
-                    "Non-null value 1 provided for property b, which is not supported in this version"))
         }
+        assertTrue(e.message!!.contains("Non-null value 1 provided for property b, which is not supported in this version"))
     }
-
 }

serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EvolvabilityTests.kt

@@ -12,17 +12,21 @@ import net.corda.core.serialization.ConstructorForDeserialization
 import net.corda.core.serialization.DeprecatedConstructorForDeserialization
 import net.corda.core.serialization.SerializableCalculatedProperty
 import net.corda.core.serialization.SerializedBytes
-import net.corda.serialization.internal.amqp.testutils.*
+import net.corda.serialization.internal.amqp.custom.InstantSerializer
+import net.corda.serialization.internal.amqp.testutils.ProjectStructure.projectRootDir
+import net.corda.serialization.internal.amqp.testutils.TestSerializationOutput
+import net.corda.serialization.internal.amqp.testutils.deserialize
+import net.corda.serialization.internal.amqp.testutils.serializeAndReturnSchema
+import net.corda.serialization.internal.amqp.testutils.testDefaultFactory
+import net.corda.serialization.internal.amqp.testutils.testName
 import org.junit.Ignore
 import org.junit.Test
 import java.io.File
 import java.io.NotSerializableException
+import java.math.BigInteger
 import java.net.URI
 import java.time.Instant
 import kotlin.test.assertEquals
-import net.corda.serialization.internal.amqp.custom.InstantSerializer
-import net.corda.serialization.internal.amqp.testutils.ProjectStructure.projectRootDir
-import java.math.BigInteger
 import kotlin.test.fail
 
 // To regenerate any of the binary test files do the following
@@ -791,4 +795,49 @@ class EvolvabilityTests {
         assertEquals(-1, deserializedCC.a)
         assertEquals(42, deserializedCC.b)
     }
+
+    @Test
+    fun addMandatoryFieldAndRemoveExistingNullableIntField() {
+        val sf = testDefaultFactory()
+        val resource = "EvolvabilityTests.addMandatoryFieldAndRemoveExistingNullableIntField"
+
+        // Original version of the class as it was serialised
+        // data class CC(val data: String, val a: Int?)
+        // File(URI("$localPath/$resource")).writeBytes(SerializationOutput(sf).serialize(CC("written", null)).bytes)
+
+        data class CC(val data: String, val b: String) {
+            @DeprecatedConstructorForDeserialization(1)
+            @Suppress("unused")
+            constructor(data: String, a: Int?) : this(data, a?.toString() ?: "<not provided>")
+        }
+
+        val url = EvolvabilityTests::class.java.getResource(resource) ?: fail("Not found!")
+        val sc2 = url.readBytes()
+        val deserializedCC = DeserializationInput(sf).deserialize(SerializedBytes<CC>(sc2))
+
+        assertEquals("written", deserializedCC.data)
+        assertEquals("<not provided>", deserializedCC.b)
+    }
+
+    @Test
+    fun removeExistingNullableIntFieldWithAltConstructor() {
+        val sf = testDefaultFactory()
+        val resource = "EvolvabilityTests.removeExistingNullableIntFieldWithAltConstructor"
+
+        // Original version of the class as it was serialised
+//         data class CC(val data: String, val a: Int?)
+//         File(URI("$localPath/$resource")).writeBytes(SerializationOutput(sf).serialize(CC("written", null)).bytes)
+
+        data class CC(val data: String) {
+            @DeprecatedConstructorForDeserialization(1)
+            @Suppress("unused")
+            constructor(data: String, a: Int?) : this(data + (a?.toString() ?: "<not provided>"))
+        }
+
+        val url = EvolvabilityTests::class.java.getResource(resource) ?: fail("Not found!")
+        val sc2 = url.readBytes()
+        val deserializedCC = DeserializationInput(sf).deserialize(SerializedBytes<CC>(sc2))
+
+        assertEquals("written<not provided>", deserializedCC.data)
+    }
 }