diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializerFactory.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializerFactory.kt index 9258bb888b..0e3804923c 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializerFactory.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializerFactory.kt @@ -103,15 +103,8 @@ class DefaultEvolutionSerializerFactory( private fun RemoteTypeInformation.Composable.validateEvolvability(localProperties: Map) { val remotePropertyNames = properties.keys val localPropertyNames = localProperties.keys - val deletedProperties = remotePropertyNames - localPropertyNames val newProperties = localPropertyNames - remotePropertyNames - // Here is where we can exercise a veto on evolutions that remove properties. - if (deletedProperties.isNotEmpty() && mustPreserveDataWhenEvolving) - throw EvolutionSerializationException(this, - "Property ${deletedProperties.first()} of remote ContractState type is not present in local type, " + - "and context is configured to prevent forwards-compatible deserialization.") - // Check mandatory-ness of constructor-set properties. newProperties.forEach { propertyName -> if (localProperties[propertyName]!!.mustBeProvided) throw EvolutionSerializationException( @@ -170,5 +163,6 @@ class DefaultEvolutionSerializerFactory( this, constructor, properties, - classLoader) + classLoader, + mustPreserveDataWhenEvolving) } \ No newline at end of file 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 01db0ab66b..3c38f0dedf 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 @@ -180,23 +180,38 @@ private class ConstructorBasedObjectBuilder( * 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, private val slotAssignments: IntArray): ObjectBuilder { +class EvolutionObjectBuilder(private val localBuilder: ObjectBuilder, + private val slotAssignments: IntArray, + private val remoteProperties: List, + private val mustPreserveData: Boolean): ObjectBuilder { companion object { + + 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, constructor: LocalConstructorInformation, localProperties: Map, remoteProperties: List): () -> ObjectBuilder { + fun makeProvider(typeIdentifier: TypeIdentifier, + constructor: LocalConstructorInformation, + localProperties: Map, + remoteTypeInformation: RemoteTypeInformation.Composable, + mustPreserveData: Boolean): () -> ObjectBuilder { val localBuilderProvider = ObjectBuilder.makeProvider(typeIdentifier, constructor, localProperties) - val reroutedIndices = remoteProperties.map { propertyName -> - localBuilderProvider.propertySlots[propertyName] ?: -1 + val remotePropertyNames = remoteTypeInformation.properties.keys.sorted() + val reroutedIndices = remotePropertyNames.map { propertyName -> + localBuilderProvider.propertySlots[propertyName] ?: DISCARDED }.toIntArray() return { - EvolutionObjectBuilder(localBuilderProvider(), reroutedIndices) + EvolutionObjectBuilder( + localBuilderProvider(), + reroutedIndices, + remotePropertyNames, + mustPreserveData) } } } @@ -207,7 +222,16 @@ class EvolutionObjectBuilder(private val localBuilder: ObjectBuilder, private va override fun populate(slot: Int, value: Any?) { val slotAssignment = slotAssignments[slot] - if (slotAssignment != -1) localBuilder.populate(slotAssignment, value) + if (slotAssignment == DISCARDED) { + if (mustPreserveData && value != null) { + throw NotSerializableException( + "Non-null value $value provided for property ${remoteProperties[slot]}, " + + "which is not supported in this version" + ) + } + } else { + localBuilder.populate(slotAssignment, value) + } } override fun build(): Any = localBuilder.build() diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectSerializer.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectSerializer.kt index ddf2ed1855..03722bbd7f 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectSerializer.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ObjectSerializer.kt @@ -167,13 +167,21 @@ class EvolutionObjectSerializer( private val reader: ComposableObjectReader): ObjectSerializer { companion object { - fun make(localTypeInformation: LocalTypeInformation.Composable, remoteTypeInformation: RemoteTypeInformation.Composable, constructor: LocalConstructorInformation, - properties: Map, classLoader: ClassLoader): EvolutionObjectSerializer { + fun make(localTypeInformation: LocalTypeInformation.Composable, + remoteTypeInformation: RemoteTypeInformation.Composable, constructor: LocalConstructorInformation, + properties: Map, + classLoader: ClassLoader, + mustPreserveData: Boolean): EvolutionObjectSerializer { val propertySerializers = makePropertySerializers(properties, remoteTypeInformation.properties, classLoader) val reader = ComposableObjectReader( localTypeInformation.typeIdentifier, propertySerializers, - EvolutionObjectBuilder.makeProvider(localTypeInformation.typeIdentifier, constructor, properties, remoteTypeInformation.properties.keys.sorted())) + EvolutionObjectBuilder.makeProvider( + localTypeInformation.typeIdentifier, + constructor, + properties, + remoteTypeInformation, + mustPreserveData)) return EvolutionObjectSerializer( localTypeInformation.observedType, diff --git a/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializerFactoryTests.kt b/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializerFactoryTests.kt index 0dda90607b..e11c638bb3 100644 --- a/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializerFactoryTests.kt +++ b/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializerFactoryTests.kt @@ -1,54 +1,64 @@ package net.corda.serialization.internal.amqp -import net.corda.serialization.internal.amqp.testutils.deserializeAndReturnEnvelope -import net.corda.serialization.internal.amqp.testutils.serialize -import net.corda.serialization.internal.amqp.testutils.testDefaultFactory +import com.natpryce.hamkrest.should.shouldMatch +import net.corda.core.serialization.SerializedBytes +import net.corda.core.serialization.serialize +import net.corda.serialization.internal.AllWhitelist +import net.corda.serialization.internal.amqp.GenericsTests.Companion.localPath +import net.corda.serialization.internal.amqp.testutils.* +import net.corda.serialization.internal.carpenter.ClassCarpenterImpl import net.corda.serialization.internal.model.RemoteTypeInformation import net.corda.serialization.internal.model.TypeIdentifier import org.junit.Test -import kotlin.test.assertFailsWith -import kotlin.test.assertNotNull -import kotlin.test.assertNull +import java.io.File +import java.io.NotSerializableException +import java.net.URI +import kotlin.test.* class EvolutionSerializerFactoryTests { - private val factory = testDefaultFactory() + private val factory = SerializerFactoryBuilder.build( + AllWhitelist, + ClassCarpenterImpl(AllWhitelist, ClassLoader.getSystemClassLoader()), + descriptorBasedSerializerRegistry = DefaultDescriptorBasedSerializerRegistry(), + mustPreserveDataWhenEvolving = true) + + // Version of the class as it was serialised + // + // data class C(val a: Int, val b: Int?) + // + // Version of the class as it's used in the test + data class C(val a: Int) @Test fun preservesDataWhenFlagSet() { - val nonStrictEvolutionSerializerFactory = DefaultEvolutionSerializerFactory( - factory, - ClassLoader.getSystemClassLoader(), - false) + val resource = "${javaClass.simpleName}.${testName()}" - val strictEvolutionSerializerFactory = DefaultEvolutionSerializerFactory( - factory, - ClassLoader.getSystemClassLoader(), - true) + val withNullResource = "${resource}_with_null" + val withoutNullResource = "${resource}_without_null" - @Suppress("unused") - class C(val importantFieldA: Int) - val (_, env) = DeserializationInput(factory).deserializeAndReturnEnvelope( - SerializationOutput(factory).serialize(C(1))) + // Uncomment to re-generate test files + // val withNullOriginal = C(1, null) + // val withoutNullOriginal = C(1, 1) + // File(URI("$localPath/$withNullResource")).writeBytes( + // SerializationOutput(factory).serialize(withNullOriginal).bytes) + // File(URI("$localPath/$withoutNullResource")).writeBytes( + // SerializationOutput(factory).serialize(withoutNullOriginal).bytes) - val remoteTypeInformation = AMQPRemoteTypeModel().interpret(SerializationSchemas(env.schema, env.transformsSchema)) - .values.find { it.typeIdentifier == TypeIdentifier.forClass(C::class.java) } - as RemoteTypeInformation.Composable + val withoutNullUrl = javaClass.getResource(withoutNullResource) + val withNullUrl = javaClass.getResource(withNullResource) - val withAddedField = remoteTypeInformation.copy(properties = remoteTypeInformation.properties.plus( - "importantFieldB" to remoteTypeInformation.properties["importantFieldA"]!!)) + // We can deserialize the evolved instance where the original value of 'b' is null. + val withNullTarget = DeserializationInput(factory).deserialize(SerializedBytes(withNullUrl.readBytes())) + assertEquals(1, withNullTarget.a) - val localTypeInformation = factory.getTypeInformation(C::class.java) - - // No evolution required with original fields. - assertNull(strictEvolutionSerializerFactory.getEvolutionSerializer(remoteTypeInformation, localTypeInformation)) - - // Returns an evolution serializer if the fields have changed. - assertNotNull(nonStrictEvolutionSerializerFactory.getEvolutionSerializer(withAddedField, localTypeInformation)) - - // Fails in strict mode if the remote type information includes a field not included in the local type. - assertFailsWith { - strictEvolutionSerializerFactory.getEvolutionSerializer(withAddedField, localTypeInformation) + // We cannot deserialize the evolved instance where the original value of 'b' is non-null. + try { + DeserializationInput(factory).deserialize(SerializedBytes(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")) } } diff --git a/serialization/src/test/resources/net/corda/serialization/internal/amqp/EvolutionSerializerFactoryTests.preservesDataWhenFlagSet_with_null b/serialization/src/test/resources/net/corda/serialization/internal/amqp/EvolutionSerializerFactoryTests.preservesDataWhenFlagSet_with_null new file mode 100644 index 0000000000..bd4888916d Binary files /dev/null and b/serialization/src/test/resources/net/corda/serialization/internal/amqp/EvolutionSerializerFactoryTests.preservesDataWhenFlagSet_with_null differ diff --git a/serialization/src/test/resources/net/corda/serialization/internal/amqp/EvolutionSerializerFactoryTests.preservesDataWhenFlagSet_without_null b/serialization/src/test/resources/net/corda/serialization/internal/amqp/EvolutionSerializerFactoryTests.preservesDataWhenFlagSet_without_null new file mode 100644 index 0000000000..5e5aa4e50b Binary files /dev/null and b/serialization/src/test/resources/net/corda/serialization/internal/amqp/EvolutionSerializerFactoryTests.preservesDataWhenFlagSet_without_null differ