diff --git a/build.gradle b/build.gradle index 3c394a9e99..778fe39f7e 100644 --- a/build.gradle +++ b/build.gradle @@ -58,7 +58,7 @@ buildscript { ext.hibernate_version = '5.2.6.Final' ext.h2_version = '1.4.197' // Update docs if renamed or removed. ext.postgresql_version = '42.1.4' - ext.rxjava_version = '1.2.4' + ext.rxjava_version = '1.3.8' ext.dokka_version = '0.9.17' ext.eddsa_version = '0.3.0' // Performance tuned version for enterprise. ext.dependency_checker_version = '3.1.0' @@ -71,7 +71,7 @@ buildscript { ext.shadow_version = '2.0.4' ext.artifactory_plugin_version = constants.getProperty('artifactoryPluginVersion') ext.hikari_version = '2.5.1' - ext.liquibase_version = '3.6.2' + ext.liquibase_version = '3.5.5' ext.artifactory_contextUrl = 'https://ci-artifactory.corda.r3cev.com/artifactory' ext.snake_yaml_version = constants.getProperty('snakeYamlVersion') ext.docker_compose_rule_version = '0.33.0' @@ -230,6 +230,7 @@ allprojects { if (System.getProperty("test.maxParallelForks") != null) { maxParallelForks = Integer.valueOf(System.getProperty("test.maxParallelForks")) + logger.debug("System property test.maxParallelForks found - setting max parallel forks to $maxParallelForks for $project") } if (project.path.startsWith(':experimental') && System.getProperty("experimental.test.enable") == null) { @@ -283,6 +284,8 @@ allprojects { // We want to use SLF4J's version of these bindings: jcl-over-slf4j // Remove any transitive dependency on Apache's version. exclude group: 'commons-logging', module: 'commons-logging' + // Remove any transitive dependency on Logback (e.g. Liquibase 3.6 introduces this dependency) + exclude group: 'ch.qos.logback' // Netty-All is an uber-jar which contains every Netty module. // Exclude it to force us to use the individual Netty modules instead. diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst index 8e2b67071e..4923577934 100644 --- a/docs/source/changelog.rst +++ b/docs/source/changelog.rst @@ -17,7 +17,7 @@ Unreleased * Introduced ``TestCorDapp`` and utilities to support asymmetric setups for nodes through ``DriverDSL``, ``MockNetwork`` and ``MockServices``. -* Change type of the `checkpoint_value` column. Please check the upgrade-notes on how to update your database. +* Change type of the ``checkpoint_value`` column. Please check the upgrade-notes on how to update your database. * Removed buggy :serverNameTablePrefix: configuration. @@ -153,7 +153,7 @@ Unreleased Values are: [FAIL, WARN, IGNORE], default to FAIL if unspecified. * Introduced a placeholder for custom properties within ``node.conf``; the property key is "custom". * The deprecated web server now has its own ``web-server.conf`` file, separate from ``node.conf``. - * Property keys with double quotes (e.g. `"key"`) in ``node.conf`` are no longer allowed, for rationale refer to :doc:`corda-configuration-file`. + * Property keys with double quotes (e.g. "key") in ``node.conf`` are no longer allowed, for rationale refer to :doc:`corda-configuration-file`. * Added public support for creating ``CordaRPCClient`` using SSL. For this to work the node needs to provide client applications a certificate to be added to a truststore. See :doc:`tutorial-clientrpc-api` @@ -170,7 +170,7 @@ Unreleased * The whitelist.txt file is no longer needed. The existing network parameters file is used to update the current contracts whitelist. - * The CorDapp jars are also copied to each nodes' `cordapps` directory. + * The CorDapp jars are also copied to each nodes' ``cordapps`` directory. * Errors thrown by a Corda node will now reported to a calling RPC client with attention to serialization and obfuscation of internal data. @@ -180,6 +180,8 @@ Unreleased reference to the outer class) as per the Java documentation `here `_ we are disallowing this as the paradigm in general makes little sense for contract states. +* Node can be shut down abruptly by ``shutdown`` function in ``CordaRPCOps`` or gracefully (draining flows first) through ``gracefulShutdown`` command from shell. + * API change: ``net.corda.core.schemas.PersistentStateRef`` fields (index and txId) are now non-nullable. The fields were always effectively non-nullable - values were set from non-nullable fields of other objects. The class is used as database Primary Key columns of other entities and databases already impose those columns as non-nullable @@ -192,8 +194,8 @@ Unreleased * Table name with a typo changed from ``NODE_ATTCHMENTS_CONTRACTS`` to ``NODE_ATTACHMENTS_CONTRACTS``. -* Node logs a warning for any ``MappedSchema`` containing a JPA entity referencing another JPA entity from a different ``MappedSchema`. - The log entry starts with `Cross-reference between MappedSchemas.`. +* Node logs a warning for any ``MappedSchema`` containing a JPA entity referencing another JPA entity from a different ``MappedSchema``. + The log entry starts with "Cross-reference between MappedSchemas". API: Persistence documentation no longer suggests mapping between different schemas. * Upgraded Artemis to v2.6.2. @@ -476,6 +478,9 @@ Corda Enterprise 3.0 Developer Preview will help make it clearer which parts of the api are stable. Scripts have been provided to smooth the upgrade process for existing projects in the ``tools\scripts`` directory of the Corda repo. +* ``TransactionSignature`` includes a new ``partialMerkleTree`` property, required for future support of signing over + multiple transactions at once. + * Shell (embedded available only in dev mode or via SSH) connects to the node via RPC instead of using the ``CordaRPCOps`` object directly. To enable RPC connectivity ensure node’s ``rpcSettings.address`` and ``rpcSettings.adminAddress`` settings are present. diff --git a/docs/source/design/data-model-upgrades/signature-constraints.md b/docs/source/design/data-model-upgrades/signature-constraints.md index 67f5de1b95..09bbeef5f1 100644 --- a/docs/source/design/data-model-upgrades/signature-constraints.md +++ b/docs/source/design/data-model-upgrades/signature-constraints.md @@ -4,7 +4,7 @@ This design document outlines an additional kind of *contract constraint*, used ## Background -Contract constraints are a part of how Corda manages application upgrades. There are two kinds of upgrade that can be applied to the ledger: +Contract constraints are a part of how Corda ensures the correct code is executed to verify transactions, and also how it manages application upgrades. There are two kinds of upgrade that can be applied to the ledger: * Explicit * Implicit @@ -31,21 +31,20 @@ We would like a new kind of constraint that is more convenient and decentralised ## Goals * Improve usability by eliminating the need to change the network parameters. - * Improve decentralisation by allowing apps to be developed and upgraded without the zone operator knowing or being able to influence it. - * Eventually, phase out zone whitelisting constraints. - ## Non-goals * Preventing downgrade attacks. Downgrade attack prevention will be tackled in a different design effort. * Phase out of hash constraints. If malicious app creators are in the users threat model then hash constraints are the way to go. * Handling the case where third parties re-sign app jars. +* Package namespace ownership (a separate effort). +* Allowing the zone operator to override older constraints, to provide a non-explicit upgrade path. ## Design details -We propose being able to constrain to any attachments signed by a specified set of keys. +We propose being able to constrain to any attachments whose files are signed by a specified set of keys. This satisfies the usability requirement because the creation of a new application is as simple as invoking the `jarsigner` tool that comes with the JDK. This can be integrated with the build system via a Gradle or Maven task. For example, Gradle can use jarsigner via [the signjar task](https://ant.apache.org/manual/Tasks/signjar.html) ([example](https://gist.github.com/Lien/7150434)). @@ -87,7 +86,7 @@ The `TransactionBuilder` class can select the right constraint given what it alr ### Tooling and workflow -The primary tool required is of course `jarsigner`. In dev and integration test modes, the node will ignore missing signatures in attachment JARs and will simply log a warning if no signature is present. +The primary tool required is of course `jarsigner`. In dev mode, the node will ignore missing signatures in attachment JARs and will simply log an error if no signature is present when a constraint requires one. To verify and print information about the signatures on a JAR, the `jarsigner` tool can be used again. In addition, we should add some new shell commands that do the same thing, but for a given attachment hash or transaction hash - these may be useful for debugging and analysis. Actually a new shell command should cover all aspects of inspecting attachments - not just signatures but what's inside them, simple way to save them to local disk etc. diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/persistence/SchemaMigration.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/persistence/SchemaMigration.kt index 3364c7a468..af58394df2 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/persistence/SchemaMigration.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/persistence/SchemaMigration.kt @@ -164,10 +164,16 @@ class SchemaMigration( /** For existing database created before verions 4.0 add Liquibase support - creates DATABASECHANGELOG and DATABASECHANGELOGLOCK tables and mark changesets are executed. */ private fun migrateOlderDatabaseToUseLiquibase(existingCheckpoints: Boolean): Boolean { + //workaround to detect that if Corda finance module is in use then the most recent version with Liquibase migration scripts was deployed + if (schemas.any { schema -> + (schema::class.qualifiedName == "net.corda.finance.schemas.CashSchemaV1" || schema::class.qualifiedName == "net.corda.finance.schemas.CommercialPaperSchemaV1") + && schema.migrationResource == null + }) + throw DatabaseMigrationException("Detected incompatible corda-finance cordapp without database migration scripts, replace the existing corda-finance-VERSION.jar with the latest one.") + val isExistingDBWithoutLiquibase = dataSource.connection.use { - it.metaData.getTables(null, null, "NODE%", null).next() && - !it.metaData.getTables(null, null, "DATABASECHANGELOG", null).next() && - !it.metaData.getTables(null, null, "DATABASECHANGELOGLOCK", null).next() + (it.metaData.getTables(null, null, "NODE%", null).next() && + !it.metaData.getTables(null, null, "DATABASECHANGELOG%", null).next()) } when { isExistingDBWithoutLiquibase && existingCheckpoints -> throw CheckpointsException() @@ -177,29 +183,31 @@ class SchemaMigration( dataSource.connection.use { connection -> // Schema migrations pre release 4.0 - val preV4Baseline = - listOf("migration/common.changelog-init.xml", - "migration/node-info.changelog-init.xml", - "migration/node-info.changelog-v1.xml", - "migration/node-info.changelog-v2.xml", - "migration/node-core.changelog-init.xml", - "migration/node-core.changelog-v3.xml", - "migration/node-core.changelog-v4.xml", - "migration/node-core.changelog-v5.xml", - "migration/node-core.changelog-pkey.xml", - "migration/vault-schema.changelog-init.xml", - "migration/vault-schema.changelog-v3.xml", - "migration/vault-schema.changelog-v4.xml", - "migration/vault-schema.changelog-pkey.xml", - "migration/cash.changelog-init.xml", - "migration/cash.changelog-v1.xml", - "migration/commercial-paper.changelog-init.xml", - "migration/commercial-paper.changelog-v1.xml") + - if (schemas.any { schema -> schema.migrationResource == "node-notary.changelog-master" }) - listOf("migration/node-notary.changelog-init.xml", - "migration/node-notary.changelog-v1.xml", - "migration/vault-schema.changelog-pkey.xml") - else emptyList() + val preV4Baseline = mutableListOf("migration/common.changelog-init.xml", + "migration/node-info.changelog-init.xml", + "migration/node-info.changelog-v1.xml", + "migration/node-info.changelog-v2.xml", + "migration/node-core.changelog-init.xml", + "migration/node-core.changelog-v3.xml", + "migration/node-core.changelog-v4.xml", + "migration/node-core.changelog-v5.xml", + "migration/node-core.changelog-pkey.xml", + "migration/vault-schema.changelog-init.xml", + "migration/vault-schema.changelog-v3.xml", + "migration/vault-schema.changelog-v4.xml", + "migration/vault-schema.changelog-pkey.xml") + + if (schemas.any { schema -> schema.migrationResource == "cash.changelog-master" }) + preV4Baseline.addAll(listOf("migration/cash.changelog-init.xml", + "migration/cash.changelog-v1.xml")) + + if (schemas.any { schema -> schema.migrationResource == "commercial-paper.changelog-master" }) + preV4Baseline.addAll(listOf("migration/commercial-paper.changelog-init.xml", + "migration/commercial-paper.changelog-v1.xml")) + + if (schemas.any { schema -> schema.migrationResource == "node-notary.changelog-master" }) + preV4Baseline.addAll(listOf("migration/node-notary.changelog-init.xml", + "migration/node-notary.changelog-v1.xml")) val customResourceAccessor = CustomResourceAccessor(dynamicInclude, preV4Baseline, classLoader) val liquibase = Liquibase(dynamicInclude, customResourceAccessor, getLiquibaseDatabase(JdbcConnection(connection))) diff --git a/node/src/main/kotlin/net/corda/node/services/schema/NodeSchemaService.kt b/node/src/main/kotlin/net/corda/node/services/schema/NodeSchemaService.kt index 5ba3223ea8..6e4b7b897d 100644 --- a/node/src/main/kotlin/net/corda/node/services/schema/NodeSchemaService.kt +++ b/node/src/main/kotlin/net/corda/node/services/schema/NodeSchemaService.kt @@ -81,7 +81,7 @@ class NodeSchemaService(private val extraSchemas: Set = emptySet() if (includeNotarySchemas) mapOf(Pair(NodeNotaryV1, SchemaOptions())) else emptyMap() fun internalSchemas() = requiredSchemas.keys + extraSchemas.filter { schema -> // when mapped schemas from the finance module are present, they are considered as internal ones - schema::class.simpleName == "net.corda.finance.schemas.CashSchemaV1" || schema::class.simpleName == "net.corda.finance.schemas.CommercialPaperSchemaV1" } + schema::class.qualifiedName == "net.corda.finance.schemas.CashSchemaV1" || schema::class.qualifiedName == "net.corda.finance.schemas.CommercialPaperSchemaV1" } override val schemaOptions: Map = requiredSchemas + extraSchemas.associateBy({ it }, { SchemaOptions() }) diff --git a/node/src/main/resources/migration/node-core.changelog-tx-mapping.xml b/node/src/main/resources/migration/node-core.changelog-tx-mapping.xml index c85c5461d9..ecf563e3e3 100644 --- a/node/src/main/resources/migration/node-core.changelog-tx-mapping.xml +++ b/node/src/main/resources/migration/node-core.changelog-tx-mapping.xml @@ -13,6 +13,7 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.5.xsd"> + diff --git a/node/src/main/resources/migration/vault-schema.changelog-v5.xml b/node/src/main/resources/migration/vault-schema.changelog-v5.xml index 6ed365a363..86064e9deb 100644 --- a/node/src/main/resources/migration/vault-schema.changelog-v5.xml +++ b/node/src/main/resources/migration/vault-schema.changelog-v5.xml @@ -1,15 +1,15 @@ + xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.5.xsd"> - + + - - - - - + + + + + diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ArraySerializer.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ArraySerializer.kt index a46353664d..e21d5251a9 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ArraySerializer.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/ArraySerializer.kt @@ -59,7 +59,7 @@ open class ArraySerializer(override val type: Type, factory: SerializerFactory) "$typeName[]" } else { - val arrayType = if (type.asClass()!!.componentType.isPrimitive) "[p]" else "[]" + val arrayType = if (type.asClass().componentType.isPrimitive) "[p]" else "[]" "${type.componentType().typeName}$arrayType" } } @@ -103,7 +103,7 @@ open class ArraySerializer(override val type: Type, factory: SerializerFactory) } open fun List.toArrayOfType(type: Type): Any { - val elementType = type.asClass() ?: throw AMQPNotSerializableException(type, "Unexpected array element type $type") + val elementType = type.asClass() val list = this return java.lang.reflect.Array.newInstance(elementType, this.size).apply { (0..lastIndex).forEach { java.lang.reflect.Array.set(this, it, list[it]) } @@ -115,7 +115,7 @@ open class ArraySerializer(override val type: Type, factory: SerializerFactory) // the array since Kotlin won't allow an implicit cast from Int (as they're stored as 16bit ints) to Char class CharArraySerializer(factory: SerializerFactory) : ArraySerializer(Array::class.java, factory) { override fun List.toArrayOfType(type: Type): Any { - val elementType = type.asClass() ?: throw AMQPNotSerializableException(type, "Unexpected array element type $type") + val elementType = type.asClass() val list = this return java.lang.reflect.Array.newInstance(elementType, this.size).apply { (0..lastIndex).forEach { java.lang.reflect.Array.set(this, it, (list[it] as Int).toChar()) } @@ -169,11 +169,7 @@ class PrimCharArraySerializer(factory: SerializerFactory) : PrimArraySerializer( } override fun List.toArrayOfType(type: Type): Any { - val elementType = type.asClass() ?: throw AMQPNotSerializableException( - type, - "Unexpected array element type $type", - "blob is corrupt") - + val elementType = type.asClass() val list = this return java.lang.reflect.Array.newInstance(elementType, this.size).apply { val array = this diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/CorDappCustomSerializer.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/CorDappCustomSerializer.kt index 4271ee62f5..ad215b2417 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/CorDappCustomSerializer.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/CorDappCustomSerializer.kt @@ -103,8 +103,8 @@ class CorDappCustomSerializer( * For 3rd party plugin serializers we are going to exist on exact type matching. i.e. we will * not support base class serializers for derivedtypes */ - override fun isSerializerFor(clazz: Class<*>) : Boolean { - return type.asClass()?.let { TypeToken.of(it) == TypeToken.of(clazz) } ?: false - } + override fun isSerializerFor(clazz: Class<*>) = + TypeToken.of(type.asClass()) == TypeToken.of(clazz) + } diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/CustomSerializer.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/CustomSerializer.kt index 0674c3756e..7acffc9ae7 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/CustomSerializer.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/CustomSerializer.kt @@ -77,7 +77,7 @@ abstract class CustomSerializer : AMQPSerializer, SerializerFor { override fun isSerializerFor(clazz: Class<*>): Boolean = clazz == this.clazz override val type: Type get() = clazz override val typeDescriptor: Symbol by lazy { - Symbol.valueOf("$DESCRIPTOR_DOMAIN:${SerializerFingerPrinter().fingerprintForDescriptors(superClassSerializer.typeDescriptor.toString(), nameForType(clazz))}") + Symbol.valueOf("$DESCRIPTOR_DOMAIN:${fingerprintForDescriptors(superClassSerializer.typeDescriptor.toString(), nameForType(clazz))}") } private val typeNotation: TypeNotation = RestrictedType( SerializerFactory.nameForType(clazz), diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/DeserializationInput.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/DeserializationInput.kt index 232b554ad7..0ed4820173 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/DeserializationInput.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/DeserializationInput.kt @@ -166,7 +166,7 @@ class DeserializationInput constructor( "is outside of the bounds for the list of size: ${objectHistory.size}") val objectRetrieved = objectHistory[objectIndex] - if (!objectRetrieved::class.java.isSubClassOf(type.asClass()!!)) { + if (!objectRetrieved::class.java.isSubClassOf(type.asClass())) { throw AMQPNotSerializableException( type, "Existing reference type mismatch. Expected: '$type', found: '${objectRetrieved::class.java}' " + diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EnumEvolutionSerializer.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EnumEvolutionSerializer.kt index ef3838340a..546ef81b3e 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EnumEvolutionSerializer.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EnumEvolutionSerializer.kt @@ -90,7 +90,7 @@ class EnumEvolutionSerializer( val renameRules: List? = uncheckedCast(transforms[TransformTypes.Rename]) // What values exist on the enum as it exists on the class path - val localValues = new.type.asClass()!!.enumConstants.map { it.toString() } + val localValues = new.type.asClass().enumConstants.map { it.toString() } val conversions: MutableMap = localValues .union(defaultRules?.map { it.new }?.toSet() ?: emptySet()) @@ -140,7 +140,7 @@ class EnumEvolutionSerializer( throw AMQPNotSerializableException(type, "No rule to evolve enum constant $type::$enumName") } - return type.asClass()!!.enumConstants[ordinals[conversions[enumName]]!!] + return type.asClass().enumConstants[ordinals[conversions[enumName]]!!] } override fun writeClassInfo(output: SerializationOutput) { diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EnumSerializer.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EnumSerializer.kt index bd5aae679f..31eed938d2 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EnumSerializer.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EnumSerializer.kt @@ -44,7 +44,7 @@ class EnumSerializer(declaredType: Type, declaredClass: Class<*>, factory: Seria ): Any { val enumName = (obj as List<*>)[0] as String val enumOrd = obj[1] as Int - val fromOrd = type.asClass()!!.enumConstants[enumOrd] as Enum<*>? + val fromOrd = type.asClass().enumConstants[enumOrd] as Enum<*>? if (enumName != fromOrd?.name) { throw AMQPNotSerializableException( diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializer.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializer.kt index d9c86c72ff..0774bbfb7e 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializer.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializer.kt @@ -42,7 +42,7 @@ abstract class EvolutionSerializer( clazz: Type, factory: SerializerFactory, protected val oldReaders: Map, - override val kotlinConstructor: KFunction? + override val kotlinConstructor: KFunction ) : ObjectSerializer(clazz, factory) { // explicitly set as empty to indicate it's unused by this type of serializer override val propertySerializers = PropertySerializersEvolution() @@ -84,7 +84,7 @@ abstract class EvolutionSerializer( * TODO: rename annotation */ private fun getEvolverConstructor(type: Type, oldArgs: Map): KFunction? { - val clazz: Class<*> = type.asClass()!! + val clazz: Class<*> = type.asClass() if (!clazz.isConcreteClass) return null @@ -199,7 +199,7 @@ abstract class EvolutionSerializer( // return the synthesised object which is, given the absence of a constructor, a no op val constructor = getEvolverConstructor(new.type, readersAsSerialized) ?: return new - val classProperties = new.type.asClass()?.propertyDescriptors() ?: emptyMap() + val classProperties = new.type.asClass().propertyDescriptors() return if (classProperties.isNotEmpty() && constructor.parameters.isEmpty()) { makeWithSetters(new, factory, constructor, readersAsSerialized, classProperties) @@ -220,7 +220,7 @@ class EvolutionSerializerViaConstructor( clazz: Type, factory: SerializerFactory, oldReaders: Map, - kotlinConstructor: KFunction?, + kotlinConstructor: KFunction, private val constructorArgs: Array) : EvolutionSerializer(clazz, factory, oldReaders, kotlinConstructor) { /** * Unlike a normal [readObject] call where we simply apply the parameter deserialisers @@ -252,7 +252,7 @@ class EvolutionSerializerViaSetters( clazz: Type, factory: SerializerFactory, oldReaders: Map, - kotlinConstructor: KFunction?, + kotlinConstructor: KFunction, private val setters: Map) : EvolutionSerializer(clazz, factory, oldReaders, kotlinConstructor) { override fun readObject(obj: Any, schemas: SerializationSchemas, input: DeserializationInput, diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/FingerPrinter.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/FingerPrinter.kt index 142f3c7134..3363ebd320 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/FingerPrinter.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/FingerPrinter.kt @@ -13,15 +13,15 @@ package net.corda.serialization.internal.amqp import com.google.common.hash.Hasher import com.google.common.hash.Hashing import net.corda.core.KeepForDJVM +import net.corda.core.internal.isConcreteClass import net.corda.core.internal.kotlinObjectInstance -import net.corda.core.utilities.loggerFor import net.corda.core.utilities.toBase64 -import java.io.NotSerializableException +import net.corda.serialization.internal.amqp.SerializerFactory.Companion.isPrimitive import java.lang.reflect.* import java.util.* /** - * Should be implemented by classes which wish to provide plugable fingerprinting og types for a [SerializerFactory] + * Should be implemented by classes which wish to provide pluggable fingerprinting on types for a [SerializerFactory] */ @KeepForDJVM interface FingerPrinter { @@ -30,34 +30,13 @@ interface FingerPrinter { * of said type such that any modification to any sub element wll generate a different fingerprint */ fun fingerprint(type: Type): String - - /** - * If required, associate an instance of the fingerprinter with a specific serializer factory - */ - fun setOwner(factory: SerializerFactory) } /** * Implementation of the finger printing mechanism used by default */ @KeepForDJVM -class SerializerFingerPrinter : FingerPrinter { - private var factory: SerializerFactory? = null - - private val ARRAY_HASH: String = "Array = true" - private val ENUM_HASH: String = "Enum = true" - private val ALREADY_SEEN_HASH: String = "Already seen = true" - private val NULLABLE_HASH: String = "Nullable = true" - private val NOT_NULLABLE_HASH: String = "Nullable = false" - private val ANY_TYPE_HASH: String = "Any type = true" - private val TYPE_VARIABLE_HASH: String = "Type variable = true" - private val WILDCARD_TYPE_HASH: String = "Wild card = true" - - private val logger by lazy { loggerFor() } - - override fun setOwner(factory: SerializerFactory) { - this.factory = factory - } +class SerializerFingerPrinter(val factory: SerializerFactory) : FingerPrinter { /** * The method generates a fingerprint for a given JVM [Type] that should be unique to the schema representation. @@ -67,147 +46,167 @@ class SerializerFingerPrinter : FingerPrinter { * The idea being that even for two classes that share the same name but differ in a minor way, the fingerprint will be * different. */ - override fun fingerprint(type: Type): String { - return fingerprintForType( - type, null, HashSet(), Hashing.murmur3_128().newHasher(), debugIndent = 1).hash().asBytes().toBase64() + override fun fingerprint(type: Type): String = FingerPrintingState(factory).fingerprint(type) +} + +// Representation of the current state of fingerprinting +internal class FingerPrintingState(private val factory: SerializerFactory) { + + companion object { + private const val ARRAY_HASH: String = "Array = true" + private const val ENUM_HASH: String = "Enum = true" + private const val ALREADY_SEEN_HASH: String = "Already seen = true" + private const val NULLABLE_HASH: String = "Nullable = true" + private const val NOT_NULLABLE_HASH: String = "Nullable = false" + private const val ANY_TYPE_HASH: String = "Any type = true" } - private fun isCollectionOrMap(type: Class<*>) = - (Collection::class.java.isAssignableFrom(type) || Map::class.java.isAssignableFrom(type)) - && !EnumSet::class.java.isAssignableFrom(type) + private val typesSeen: MutableSet = mutableSetOf() + private var currentContext: Type? = null + private var hasher: Hasher = newDefaultHasher() - internal fun fingerprintForDescriptors(vararg typeDescriptors: String): String { - val hasher = Hashing.murmur3_128().newHasher() - for (typeDescriptor in typeDescriptors) { - hasher.putUnencodedChars(typeDescriptor) - } - return hasher.hash().asBytes().toBase64() - } - - private fun Hasher.fingerprintWithCustomSerializerOrElse( - factory: SerializerFactory, - clazz: Class<*>, - declaredType: Type, - block: () -> Hasher): Hasher { - // Need to check if a custom serializer is applicable - val customSerializer = factory.findCustomSerializer(clazz, declaredType) - return if (customSerializer != null) { - putUnencodedChars(customSerializer.typeDescriptor) - } else { - block() - } - } + // Fingerprint the type recursively, and return the encoded fingerprint written into the hasher. + fun fingerprint(type: Type) = fingerprintType(type).hasher.fingerprint // This method concatenates various elements of the types recursively as unencoded strings into the hasher, // effectively creating a unique string for a type which we then hash in the calling function above. - private fun fingerprintForType(type: Type, contextType: Type?, alreadySeen: MutableSet, - hasher: Hasher, debugIndent: Int = 1): Hasher { - // We don't include Example and Example where type is ? or T in this otherwise we - // generate different fingerprints for class Outer(val a: Inner) when serialising - // and deserializing (assuming deserialization is occurring in a factory that didn't - // serialise the object in the first place (and thus the cache lookup fails). This is also - // true of Any, where we need Example and Example to have the same fingerprint - return if ((type in alreadySeen) - && (type !== SerializerFactory.AnyType) - && (type !is TypeVariable<*>) - && (type !is WildcardType) - ) { - hasher.putUnencodedChars(ALREADY_SEEN_HASH) - } else { - alreadySeen += type - ifThrowsAppend({ type.typeName }) { - when (type) { - is ParameterizedType -> { - // Hash the rawType + params - val clazz = type.rawType as Class<*> + private fun fingerprintType(type: Type): FingerPrintingState = apply { + // Don't go round in circles. + if (hasSeen(type)) append(ALREADY_SEEN_HASH) + else ifThrowsAppend( + { type.typeName }, + { + typesSeen.add(type) + currentContext = type + fingerprintNewType(type) + }) + } - val startingHash = if (isCollectionOrMap(clazz)) { - hasher.putUnencodedChars(clazz.name) - } else { - hasher.fingerprintWithCustomSerializerOrElse(factory!!, clazz, type) { - fingerprintForObject(type, type, alreadySeen, hasher, factory!!, debugIndent + 1) - } - } + // For a type we haven't seen before, determine the correct path depending on the type of type it is. + private fun fingerprintNewType(type: Type) = when (type) { + is ParameterizedType -> fingerprintParameterizedType(type) + // Previously, we drew a distinction between TypeVariable, WildcardType, and AnyType, changing + // the signature of the fingerprinted object. This, however, doesn't work as it breaks bi- + // directional fingerprints. That is, fingerprinting a concrete instance of a generic + // type (Example), creates a different fingerprint from the generic type itself (Example) + // + // On serialization Example is treated as Example, a TypeVariable + // On deserialisation it is seen as Example, A WildcardType *and* a TypeVariable + // Note: AnyType is a special case of WildcardType used in other parts of the + // serializer so both cases need to be dealt with here + // + // If we treat these types as fundamentally different and alter the fingerprint we will + // end up breaking into the evolver when we shouldn't or, worse, evoking the carpenter. + is SerializerFactory.AnyType, + is WildcardType, + is TypeVariable<*> -> append("?$ANY_TYPE_HASH") + is Class<*> -> fingerprintClass(type) + is GenericArrayType -> fingerprintType(type.genericComponentType).append(ARRAY_HASH) + else -> throw AMQPNotSerializableException(type, "Don't know how to hash") + } - // ... and concatenate the type data for each parameter type. - type.actualTypeArguments.fold(startingHash) { orig, paramType -> - fingerprintForType(paramType, type, alreadySeen, orig, debugIndent + 1) - } - } - // Previously, we drew a distinction between TypeVariable, WildcardType, and AnyType, changing - // the signature of the fingerprinted object. This, however, doesn't work as it breaks bi- - // directional fingerprints. That is, fingerprinting a concrete instance of a generic - // type (Example), creates a different fingerprint from the generic type itself (Example) - // - // On serialization Example is treated as Example, a TypeVariable - // On deserialisation it is seen as Example, A WildcardType *and* a TypeVariable - // Note: AnyType is a special case of WildcardType used in other parts of the - // serializer so both cases need to be dealt with here - // - // If we treat these types as fundamentally different and alter the fingerprint we will - // end up breaking into the evolver when we shouldn't or, worse, evoking the carpenter. - is SerializerFactory.AnyType, - is WildcardType, - is TypeVariable<*> -> { - hasher.putUnencodedChars("?").putUnencodedChars(ANY_TYPE_HASH) - } - is Class<*> -> { - if (type.isArray) { - fingerprintForType(type.componentType, contextType, alreadySeen, hasher, debugIndent + 1) - .putUnencodedChars(ARRAY_HASH) - } else if (SerializerFactory.isPrimitive(type)) { - hasher.putUnencodedChars(type.name) - } else if (isCollectionOrMap(type)) { - hasher.putUnencodedChars(type.name) - } else if (type.isEnum) { - // ensures any change to the enum (adding constants) will trigger the need for evolution - hasher.apply { - type.enumConstants.forEach { - putUnencodedChars(it.toString()) - } - }.putUnencodedChars(type.name).putUnencodedChars(ENUM_HASH) - } else { - hasher.fingerprintWithCustomSerializerOrElse(factory!!, type, type) { - if (type.kotlinObjectInstance != null) { - // TODO: name collision is too likely for kotlin objects, we need to introduce some - // reference to the CorDapp but maybe reference to the JAR in the short term. - hasher.putUnencodedChars(type.name) - } else { - fingerprintForObject(type, type, alreadySeen, hasher, factory!!, debugIndent + 1) - } - } - } - } - // Hash the element type + some array hash - is GenericArrayType -> { - fingerprintForType(type.genericComponentType, contextType, alreadySeen, - hasher, debugIndent + 1).putUnencodedChars(ARRAY_HASH) - } - else -> throw AMQPNotSerializableException(type, "Don't know how to hash") - } - } + private fun fingerprintClass(type: Class<*>) = when { + type.isArray -> fingerprintType(type.componentType).append(ARRAY_HASH) + type.isPrimitiveOrCollection -> append(type.name) + type.isEnum -> fingerprintEnum(type) + else -> fingerprintWithCustomSerializerOrElse(type, type) { + if (type.kotlinObjectInstance != null) append(type.name) + else fingerprintObject(type) } } - private fun fingerprintForObject( - type: Type, - contextType: Type?, - alreadySeen: MutableSet, - hasher: Hasher, - factory: SerializerFactory, - debugIndent: Int = 0): Hasher { - // Hash the class + properties + interfaces - val name = type.asClass()?.name - ?: throw AMQPNotSerializableException(type, "Expected only Class or ParameterizedType but found $type") + private fun fingerprintParameterizedType(type: ParameterizedType) { + // Hash the rawType + params + type.asClass().let { clazz -> + if (clazz.isCollectionOrMap) append(clazz.name) + else fingerprintWithCustomSerializerOrElse(clazz, type) { + fingerprintObject(type) + } + } - propertiesForSerialization(constructorForDeserialization(type), contextType ?: type, factory) - .serializationOrder - .fold(hasher.putUnencodedChars(name)) { orig, prop -> - fingerprintForType(prop.serializer.resolvedType, type, alreadySeen, orig, debugIndent + 1) - .putUnencodedChars(prop.serializer.name) - .putUnencodedChars(if (prop.serializer.mandatory) NOT_NULLABLE_HASH else NULLABLE_HASH) - } - interfacesForSerialization(type, factory).map { fingerprintForType(it, type, alreadySeen, hasher, debugIndent + 1) } - return hasher + // ...and concatenate the type data for each parameter type. + type.actualTypeArguments.forEach { paramType -> + fingerprintType(paramType) + } } -} \ No newline at end of file + + private fun fingerprintObject(type: Type) { + // Hash the class + properties + interfaces + append(type.asClass().name) + + orderedPropertiesForSerialization(type).forEach { prop -> + fingerprintType(prop.serializer.resolvedType) + fingerprintPropSerialiser(prop) + } + + interfacesForSerialization(type, factory).forEach { iface -> + fingerprintType(iface) + } + } + + // ensures any change to the enum (adding constants) will trigger the need for evolution + private fun fingerprintEnum(type: Class<*>) { + append(type.enumConstants.joinToString()) + append(type.name) + append(ENUM_HASH) + } + + private fun fingerprintPropSerialiser(prop: PropertyAccessor) { + append(prop.serializer.name) + append(if (prop.serializer.mandatory) NOT_NULLABLE_HASH + else NULLABLE_HASH) + } + + // Write the given character sequence into the hasher. + private fun append(chars: CharSequence) { + hasher = hasher.putUnencodedChars(chars) + } + + // Give any custom serializers loaded into the factory the chance to supply their own type-descriptors + private fun fingerprintWithCustomSerializerOrElse( + clazz: Class<*>, + declaredType: Type, + defaultAction: () -> Unit) + : Unit = factory.findCustomSerializer(clazz, declaredType)?.let { + append(it.typeDescriptor) + } ?: defaultAction() + + // Test whether we are in a state in which we have already seen the given type. + // + // We don't include Example and Example where type is ? or T in this otherwise we + // generate different fingerprints for class Outer(val a: Inner) when serialising + // and deserializing (assuming deserialization is occurring in a factory that didn't + // serialise the object in the first place (and thus the cache lookup fails). This is also + // true of Any, where we need Example and Example to have the same fingerprint + private fun hasSeen(type: Type) = (type in typesSeen) + && (type !== SerializerFactory.AnyType) + && (type !is TypeVariable<*>) + && (type !is WildcardType) + + private fun orderedPropertiesForSerialization(type: Type): List { + return propertiesForSerialization( + if (type.asClass().isConcreteClass) constructorForDeserialization(type) else null, + currentContext ?: type, + factory).serializationOrder + } + +} + +// region Utility functions + +// Create a new instance of the [Hasher] used for fingerprinting by the default [SerializerFingerPrinter] +private fun newDefaultHasher() = Hashing.murmur3_128().newHasher() + +// We obtain a fingerprint from a [Hasher] by taking the Base 64 encoding of its hash bytes +private val Hasher.fingerprint get() = hash().asBytes().toBase64() + +internal fun fingerprintForDescriptors(vararg typeDescriptors: String): String = + newDefaultHasher().putUnencodedChars(typeDescriptors.joinToString()).fingerprint + +private val Class<*>.isCollectionOrMap get() = + (Collection::class.java.isAssignableFrom(this) || Map::class.java.isAssignableFrom(this)) + && !EnumSet::class.java.isAssignableFrom(this) + +private val Class<*>.isPrimitiveOrCollection get() = + isPrimitive(this) || isCollectionOrMap +// endregion 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 b52c5cca8a..4fdac3b636 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 @@ -10,6 +10,7 @@ package net.corda.serialization.internal.amqp +import net.corda.core.internal.isConcreteClass import net.corda.core.serialization.SerializationContext import net.corda.core.utilities.contextLogger import net.corda.core.utilities.trace @@ -28,7 +29,7 @@ import kotlin.reflect.jvm.javaConstructor */ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPSerializer { override val type: Type get() = clazz - open val kotlinConstructor = constructorForDeserialization(clazz) + open val kotlinConstructor = if (clazz.asClass().isConcreteClass) constructorForDeserialization(clazz) else null val javaConstructor by lazy { kotlinConstructor?.javaConstructor } companion object { diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/PropertyDescriptor.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/PropertyDescriptor.kt new file mode 100644 index 0000000000..882f067eba --- /dev/null +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/PropertyDescriptor.kt @@ -0,0 +1,205 @@ +package net.corda.serialization.internal.amqp + +import com.google.common.reflect.TypeToken +import net.corda.core.KeepForDJVM +import net.corda.core.internal.isPublic +import net.corda.serialization.internal.amqp.MethodClassifier.* +import java.lang.reflect.Field +import java.lang.reflect.Method +import java.lang.reflect.Type +import java.util.* + +/** + * Encapsulates the property of a class and its potential getter and setter methods. + * + * @property field a property of a class. + * @property setter the method of a class that sets the field. Determined by locating + * a function called setXyz on the class for the property named in field as xyz. + * @property getter the method of a class that returns a fields value. Determined by + * locating a function named getXyz for the property named in field as xyz. + */ +@KeepForDJVM +data class PropertyDescriptor(val field: Field?, val setter: Method?, val getter: Method?) { + override fun toString() = StringBuilder("").apply { + appendln("Property - ${field?.name ?: "null field"}\n") + appendln(" getter - ${getter?.name ?: "no getter"}") + appendln(" setter - ${setter?.name ?: "no setter"}") + }.toString() + + /** + * Check the types of the field, getter and setter methods against each other. + */ + fun validate() { + getter?.apply { + val getterType = genericReturnType + field?.apply { + if (!getterType.isSupertypeOf(genericReturnType)) + throw AMQPNotSerializableException( + declaringClass, + "Defined getter for parameter $name returns type $getterType " + + "yet underlying type is $genericType") + } + } + + setter?.apply { + val setterType = genericParameterTypes[0]!! + + field?.apply { + if (!genericType.isSupertypeOf(setterType)) + throw AMQPNotSerializableException( + declaringClass, + "Defined setter for parameter $name takes parameter of type $setterType " + + "yet underlying type is $genericType") + } + + getter?.apply { + if (!genericReturnType.isSupertypeOf(setterType)) + throw AMQPNotSerializableException( + declaringClass, + "Defined setter for parameter $name takes parameter of type $setterType, " + + "but getter returns $genericReturnType") + } + } + } +} + +private fun Type.isSupertypeOf(that: Type) = TypeToken.of(this).isSupertypeOf(that) + +// match an uppercase letter that also has a corresponding lower case equivalent +private val propertyMethodRegex = Regex("(?get|set|is)(?\\p{Lu}.*)") + +/** + * Collate the properties of a class and match them with their getter and setter + * methods as per a JavaBean. + * + * for a property + * exampleProperty + * + * We look for methods + * setExampleProperty + * getExampleProperty + * isExampleProperty + * + * Where getExampleProperty must return a type compatible with exampleProperty, setExampleProperty must + * take a single parameter of a type compatible with exampleProperty and isExampleProperty must + * return a boolean + */ +fun Class.propertyDescriptors(): Map { + val fieldProperties = superclassChain().declaredFields().byFieldName() + + return superclassChain().declaredMethods() + .thatArePublic() + .thatArePropertyMethods() + .withValidSignature() + .byNameAndClassifier(fieldProperties.keys) + .toClassProperties(fieldProperties) + .validated() +} + +// Generate the sequence of classes starting with this class and ascending through it superclasses. +private fun Class<*>.superclassChain() = generateSequence(this, Class<*>::getSuperclass) + +// Obtain the fields declared by all classes in this sequence of classes. +private fun Sequence>.declaredFields() = flatMap { it.declaredFields.asSequence() } + +// Obtain the methods declared by all classes in this sequence of classes. +private fun Sequence>.declaredMethods() = flatMap { it.declaredMethods.asSequence() } + +// Map a sequence of fields by field name. +private fun Sequence.byFieldName() = map { it.name to it }.toMap() + +// Select only those methods that are public (and are not the "getClass" method) +private fun Sequence.thatArePublic() = filter { it.isPublic && it.name != "getClass" } + +// Select only those methods that are isX/getX/setX methods +private fun Sequence.thatArePropertyMethods() = map { method -> + propertyMethodRegex.find(method.name)?.let { result -> + PropertyNamedMethod( + result.groups[2]!!.value, + MethodClassifier.valueOf(result.groups[1]!!.value.toUpperCase()), + method) + } +}.filterNotNull() + +// Pick only those methods whose signatures are valid, discarding the remainder without warning. +private fun Sequence.withValidSignature() = filter { it.hasValidSignature() } + +// Group methods by name and classifier, picking the method with the least generic signature if there is more than one +// of a given name and type. +private fun Sequence.byNameAndClassifier(fieldNames: Set): Map> { + val result = mutableMapOf>() + + forEach { (fieldName, classifier, method) -> + result.compute(getPropertyName(fieldName, fieldNames)) { _, byClassifier -> + (byClassifier ?: EnumMap(MethodClassifier::class.java)).merge(classifier, method) + } + } + + return result +} + +// Merge the given method into a map of methods by method classifier, picking the least generic method for each classifier. +private fun EnumMap.merge(classifier: MethodClassifier, method: Method): EnumMap { + compute(classifier) { _, existingMethod -> + if (existingMethod == null) method + else when (classifier) { + IS -> existingMethod + GET -> leastGenericBy({ genericReturnType }, existingMethod, method) + SET -> leastGenericBy({ genericParameterTypes[0] }, existingMethod, method) + } + } + return this +} + +// Make the property name conform to the underlying field name, if there is one. +private fun getPropertyName(propertyName: String, fieldNames: Set) = + if (propertyName.decapitalize() in fieldNames) propertyName.decapitalize() + else propertyName + + +// Which of the three types of property method the method is. +private enum class MethodClassifier { GET, SET, IS } + +private data class PropertyNamedMethod(val fieldName: String, val classifier: MethodClassifier, val method: Method) { + // Validate the method's signature against its classifier + fun hasValidSignature(): Boolean = method.run { + when (classifier) { + GET -> parameterCount == 0 && returnType != Void.TYPE + SET -> parameterCount == 1 && returnType == Void.TYPE + IS -> parameterCount == 0 && + (returnType == Boolean::class.java || + returnType == Boolean::class.javaObjectType) + } + } +} + +// Construct a map of PropertyDescriptors by name, by merging the raw field map with the map of classified property methods +private fun Map>.toClassProperties(fieldMap: Map): Map { + val result = mutableMapOf() + + // Fields for which we have no property methods + for ((name, field) in fieldMap) { + if (name !in keys) { + result[name] = PropertyDescriptor(field, null, null) + } + } + + for ((name, methodMap) in this) { + result[name] = PropertyDescriptor( + fieldMap[name], + methodMap[SET], + methodMap[GET] ?: methodMap[IS] + ) + } + + return result +} + +// Select the least generic of two methods by a type associated with each. +private fun leastGenericBy(feature: Method.() -> Type, first: Method, second: Method) = + if (first.feature().isSupertypeOf(second.feature())) second else first + +// Throw an exception if any property descriptor is inconsistent, e.g. the types don't match +private fun Map.validated() = apply { + forEach { _, value -> value.validate() } +} diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/PropertySerializer.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/PropertySerializer.kt index 8d34efe137..31ae3670b6 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/PropertySerializer.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/PropertySerializer.kt @@ -29,8 +29,8 @@ sealed class PropertySerializer(val name: String, val propertyReader: PropertyRe val default: String? = generateDefault() val mandatory: Boolean = generateMandatory() - private val isInterface: Boolean get() = resolvedType.asClass()?.isInterface == true - private val isJVMPrimitive: Boolean get() = resolvedType.asClass()?.isPrimitive == true + private val isInterface: Boolean get() = resolvedType.asClass().isInterface + private val isJVMPrimitive: Boolean get() = resolvedType.asClass().isPrimitive private fun generateType(): String { return if (isInterface || resolvedType == Any::class.java) "*" else SerializerFactory.nameForType(resolvedType) diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializationHelper.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializationHelper.kt index df7663ca19..4e7c79d3f3 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializationHelper.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializationHelper.kt @@ -12,15 +12,12 @@ package net.corda.serialization.internal.amqp import com.google.common.primitives.Primitives import com.google.common.reflect.TypeToken -import net.corda.core.KeepForDJVM import net.corda.core.internal.isConcreteClass -import net.corda.core.internal.isPublic import net.corda.core.serialization.ClassWhitelist import net.corda.core.serialization.ConstructorForDeserialization import net.corda.core.serialization.CordaSerializable import net.corda.core.serialization.SerializationContext import org.apache.qpid.proton.codec.Data -import java.io.NotSerializableException import java.lang.reflect.* import java.lang.reflect.Field import java.util.* @@ -36,42 +33,37 @@ import kotlin.reflect.jvm.javaType /** * Code for finding the constructor we will use for deserialization. * - * If there's only one constructor, it selects that. If there are two and one is the default, it selects the other. - * Otherwise it starts with the primary constructor in kotlin, if there is one, and then will override this with any that is - * annotated with [@ConstructorForDeserialization]. It will report an error if more than one constructor is annotated. + * If any constructor is uniquely annotated with [@ConstructorForDeserialization], then that constructor is chosen. + * An error is reported if more than one constructor is annotated. + * + * Otherwise, if there is a Kotlin primary constructor, it selects that, and if not it selects either the unique + * constructor or, if there are two and one is the default no-argument constructor, the non-default constructor. */ -fun constructorForDeserialization(type: Type): KFunction? { - val clazz: Class<*> = type.asClass()!! - if (clazz.isConcreteClass) { - var preferredCandidate: KFunction? = clazz.kotlin.primaryConstructor - var annotatedCount = 0 - val kotlinConstructors = clazz.kotlin.constructors - val hasDefault = kotlinConstructors.any { it.parameters.isEmpty() } - - for (kotlinConstructor in kotlinConstructors) { - if (preferredCandidate == null && kotlinConstructors.size == 1) { - preferredCandidate = kotlinConstructor - } else if (preferredCandidate == null && - kotlinConstructors.size == 2 && - hasDefault && - kotlinConstructor.parameters.isNotEmpty() - ) { - preferredCandidate = kotlinConstructor - } else if (kotlinConstructor.findAnnotation() != null) { - if (annotatedCount++ > 0) { - throw AMQPNotSerializableException( - type, - "More than one constructor for $clazz is annotated with @ConstructorForDeserialization.") - } - preferredCandidate = kotlinConstructor - } - } - - return preferredCandidate?.apply { isAccessible = true } - ?: throw AMQPNotSerializableException(type, "No constructor for deserialization found for $clazz.") - } else { - return null +fun constructorForDeserialization(type: Type): KFunction { + val clazz = type.asClass().apply { + if (!isConcreteClass) throw AMQPNotSerializableException(type, + "Cannot find deserialisation constructor for non-concrete class $this") } + + val kotlinCtors = clazz.kotlin.constructors + + val annotatedCtors = kotlinCtors.filter { it.findAnnotation() != null } + if (annotatedCtors.size > 1) throw AMQPNotSerializableException( + type, + "More than one constructor for $clazz is annotated with @ConstructorForDeserialization.") + + val defaultCtor = kotlinCtors.firstOrNull { it.parameters.isEmpty() } + val nonDefaultCtors = kotlinCtors.filter { it != defaultCtor } + + val preferredCandidate = annotatedCtors.firstOrNull() ?: + clazz.kotlin.primaryConstructor ?: + when(nonDefaultCtors.size) { + 1 -> nonDefaultCtors.first() + 0 -> defaultCtor ?: throw AMQPNotSerializableException(type, "No constructor found for $clazz.") + else -> throw AMQPNotSerializableException(type, "No unique non-default constructor found for $clazz.") + } + + return preferredCandidate.apply { isAccessible = true } } /** @@ -85,145 +77,13 @@ fun constructorForDeserialization(type: Type): KFunction? { fun propertiesForSerialization( kotlinConstructor: KFunction?, type: Type, - factory: SerializerFactory): PropertySerializers { - return PropertySerializers.make( + factory: SerializerFactory): PropertySerializers = PropertySerializers.make( if (kotlinConstructor != null) { propertiesForSerializationFromConstructor(kotlinConstructor, type, factory) } else { - propertiesForSerializationFromAbstract(type.asClass()!!, type, factory) + propertiesForSerializationFromAbstract(type.asClass(), type, factory) }.sortedWith(PropertyAccessor) ) -} - -/** - * Encapsulates the property of a class and its potential getter and setter methods. - * - * @property field a property of a class. - * @property setter the method of a class that sets the field. Determined by locating - * a function called setXyz on the class for the property named in field as xyz. - * @property getter the method of a class that returns a fields value. Determined by - * locating a function named getXyz for the property named in field as xyz. - */ -@KeepForDJVM -data class PropertyDescriptor(var field: Field?, var setter: Method?, var getter: Method?, var iser: Method?) { - override fun toString() = StringBuilder("").apply { - appendln("Property - ${field?.name ?: "null field"}\n") - appendln(" getter - ${getter?.name ?: "no getter"}") - appendln(" setter - ${setter?.name ?: "no setter"}") - appendln(" iser - ${iser?.name ?: "no isXYZ defined"}") - }.toString() - - constructor() : this(null, null, null, null) - - fun preferredGetter(): Method? = getter ?: iser -} - -object PropertyDescriptorsRegex { - // match an uppercase letter that also has a corresponding lower case equivalent - val re = Regex("(?get|set|is)(?\\p{Lu}.*)") -} - -/** - * Collate the properties of a class and match them with their getter and setter - * methods as per a JavaBean. - * - * for a property - * exampleProperty - * - * We look for methods - * setExampleProperty - * getExampleProperty - * isExampleProperty - * - * Where setExampleProperty must return a type compatible with exampleProperty, getExampleProperty must - * take a single parameter of a type compatible with exampleProperty and isExampleProperty must - * return a boolean - */ -fun Class.propertyDescriptors(): Map { - val classProperties = mutableMapOf() - - var clazz: Class? = this - - do { - clazz!!.declaredFields.forEach { property -> - classProperties.computeIfAbsent(property.name) { - PropertyDescriptor() - }.apply { - this.field = property - } - } - clazz = clazz.superclass - } while (clazz != null) - - // - // Running as two loops rather than one as we need to ensure we have captured all of the properties - // before looking for interacting methods and need to cope with the class hierarchy introducing - // new properties / methods - // - clazz = this - do { - // Note: It is possible for a class to have multiple instances of a function where the types - // differ. For example: - // interface I { val a: T } - // class D(override val a: String) : I - // instances of D will have both - // getA - returning a String (java.lang.String) and - // getA - returning an Object (java.lang.Object) - // In this instance we take the most derived object - // - // In addition, only getters that take zero parameters and setters that take a single - // parameter will be considered - clazz!!.declaredMethods?.map { func -> - if (!func.isPublic) return@map - if (func.name == "getClass") return@map - - PropertyDescriptorsRegex.re.find(func.name)?.apply { - // matching means we have an func getX where the property could be x or X - // so having pre-loaded all of the properties we try to match to either case. If that - // fails the getter doesn't refer to a property directly, but may refer to a constructor - // parameter that shadows a property - val properties = - classProperties[groups[2]!!.value] ?: classProperties[groups[2]!!.value.decapitalize()] ?: - // take into account those constructor properties that don't directly map to a named - // property which are, by default, already added to the map - classProperties.computeIfAbsent(groups[2]!!.value) { PropertyDescriptor() } - - properties.apply { - when (groups[1]!!.value) { - "set" -> { - if (func.parameterCount == 1) { - if (setter == null) setter = func - else if (TypeToken.of(setter!!.genericReturnType).isSupertypeOf(func.genericReturnType)) { - setter = func - } - } - } - "get" -> { - if (func.parameterCount == 0) { - if (getter == null) getter = func - else if (TypeToken.of(getter!!.genericReturnType).isSupertypeOf(func.genericReturnType)) { - getter = func - } - } - } - "is" -> { - if (func.parameterCount == 0) { - val rtnType = TypeToken.of(func.genericReturnType) - if ((rtnType == TypeToken.of(Boolean::class.java)) - || (rtnType == TypeToken.of(Boolean::class.javaObjectType))) { - if (iser == null) iser = func - } - } - } - } - } - } - } - clazz = clazz.superclass - } while (clazz != null) - - return classProperties -} /** * From a constructor, determine which properties of a class are to be serialized. @@ -245,66 +105,48 @@ internal fun propertiesForSerializationFromConstructor( // think you could inspect the parameter and check the isSynthetic flag but that is always // false so given the naming convention is specified by the standard we can just check for // this - if (kotlinConstructor.javaConstructor?.parameterCount ?: 0 > 0 && - kotlinConstructor.javaConstructor?.parameters?.get(0)?.name == "this$0" - ) { - throw SyntheticParameterException(type) + kotlinConstructor.javaConstructor?.apply { + if (parameterCount > 0 && parameters[0].name == "this$0") throw SyntheticParameterException(type) } if (classProperties.isNotEmpty() && kotlinConstructor.parameters.isEmpty()) { return propertiesForSerializationFromSetters(classProperties, type, factory) } - return mutableListOf().apply { - kotlinConstructor.parameters.withIndex().forEach { param -> - // name cannot be null, if it is then this is a synthetic field and we will have bailed - // out prior to this - val name = param.value.name!! - - // We will already have disambiguated getA for property A or a but we still need to cope - // with the case we don't know the case of A when the parameter doesn't match a property - // but has a getter - val matchingProperty = classProperties[name] ?: classProperties[name.capitalize()] - ?: throw AMQPNotSerializableException(type, - "Constructor parameter - \"$name\" - doesn't refer to a property of \"$clazz\"") - - // If the property has a getter we'll use that to retrieve it's value from the instance, if it doesn't - // *for *know* we switch to a reflection based method - val propertyReader = if (matchingProperty.getter != null) { - val getter = matchingProperty.getter ?: throw AMQPNotSerializableException( - type, - "Property has no getter method for - \"$name\" - of \"$clazz\". If using Java and the parameter name" - + "looks anonymous, check that you have the -parameters option specified in the " - + "Java compiler. Alternately, provide a proxy serializer " - + "(SerializationCustomSerializer) if recompiling isn't an option.") - - val returnType = resolveTypeVariables(getter.genericReturnType, type) - if (!constructorParamTakesReturnTypeOfGetter(returnType, getter.genericReturnType, param.value)) { - throw AMQPNotSerializableException( - type, - "Property - \"$name\" - has type \"$returnType\" on \"$clazz\" but differs from constructor " + - "parameter type \"${param.value.type.javaType}\"") - } - - Pair(PublicPropertyReader(getter), returnType) - } else { - val field = classProperties[name]!!.field - ?: throw AMQPNotSerializableException(type, - "No property matching constructor parameter named - \"$name\" - " + - "of \"$clazz\". If using Java, check that you have the -parameters option specified " + - "in the Java compiler. Alternately, provide a proxy serializer " + - "(SerializationCustomSerializer) if recompiling isn't an option") - - Pair(PrivatePropertyReader(field, type), resolveTypeVariables(field.genericType, type)) - } - - this += PropertyAccessorConstructor( - param.index, - PropertySerializer.make(name, propertyReader.first, propertyReader.second, factory)) - } + return kotlinConstructor.parameters.withIndex().map { param -> + toPropertyAccessorConstructor(param.index, param.value, classProperties, type, clazz, factory) } } +private fun toPropertyAccessorConstructor(index: Int, param: KParameter, classProperties: Map, type: Type, clazz: Class, factory: SerializerFactory): PropertyAccessorConstructor { + // name cannot be null, if it is then this is a synthetic field and we will have bailed + // out prior to this + val name = param.name!! + + // We will already have disambiguated getA for property A or a but we still need to cope + // with the case we don't know the case of A when the parameter doesn't match a property + // but has a getter + val matchingProperty = classProperties[name] ?: classProperties[name.capitalize()] + ?: throw AMQPNotSerializableException(type, + "Constructor parameter - \"$name\" - doesn't refer to a property of \"$clazz\"") + + // If the property has a getter we'll use that to retrieve it's value from the instance, if it doesn't + // *for *now* we switch to a reflection based method + val propertyReader = matchingProperty.getter?.let { getter -> + getPublicPropertyReader(getter, type, param, name, clazz) + } ?: matchingProperty.field?.let { field -> + getPrivatePropertyReader(field, type) + } ?: throw AMQPNotSerializableException(type, + "No property matching constructor parameter named - \"$name\" - " + + "of \"${param}\". If using Java, check that you have the -parameters option specified " + + "in the Java compiler. Alternately, provide a proxy serializer " + + "(SerializationCustomSerializer) if recompiling isn't an option") + + return PropertyAccessorConstructor( + index, + PropertySerializer.make(name, propertyReader.first, propertyReader.second, factory)) +} + /** * If we determine a class has a constructor that takes no parameters then check for pairs of getters / setters * and use those @@ -312,107 +154,83 @@ internal fun propertiesForSerializationFromConstructor( fun propertiesForSerializationFromSetters( properties: Map, type: Type, - factory: SerializerFactory): List { - return mutableListOf().apply { - var idx = 0 + factory: SerializerFactory): List = + properties.asSequence().withIndex().map { (index, entry) -> + val (name, property) = entry - properties.forEach { property -> - val getter: Method? = property.value.preferredGetter() - val setter: Method? = property.value.setter + val getter = property.getter + val setter = property.setter - if (getter == null || setter == null) return@forEach + if (getter == null || setter == null) return@map null - if (setter.parameterCount != 1) { - throw AMQPNotSerializableException( - type, - "Defined setter for parameter ${property.value.field?.name} takes too many arguments") - } - - val setterType = setter.genericParameterTypes[0]!! - - if ((property.value.field != null) && - (!(TypeToken.of(property.value.field?.genericType!!).isSupertypeOf(setterType))) - ) { - throw AMQPNotSerializableException( - type, - "Defined setter for parameter ${property.value.field?.name} " + - "takes parameter of type $setterType yet underlying type is " + - "${property.value.field?.genericType!!}") - } - - // Make sure the getter returns the same type (within inheritance bounds) the setter accepts. - if (!(TypeToken.of(getter.genericReturnType).isSupertypeOf(setterType))) { - throw AMQPNotSerializableException( - type, - "Defined setter for parameter ${property.value.field?.name} " + - "takes parameter of type $setterType yet the defined getter returns a value of type " + - "${getter.returnType} [${getter.genericReturnType}]") - } - this += PropertyAccessorGetterSetter( - idx++, - PropertySerializer.make(property.key, PublicPropertyReader(getter), - resolveTypeVariables(getter.genericReturnType, type), factory), + PropertyAccessorGetterSetter( + index, + PropertySerializer.make( + name, + PublicPropertyReader(getter), + resolveTypeVariables(getter.genericReturnType, type), + factory), setter) - } - } -} + }.filterNotNull().toList() -private fun constructorParamTakesReturnTypeOfGetter( - getterReturnType: Type, - rawGetterReturnType: Type, - param: KParameter): Boolean { +private fun getPrivatePropertyReader(field: Field, type: Type) = + PrivatePropertyReader(field, type) to resolveTypeVariables(field.genericType, type) + +private fun getPublicPropertyReader(getter: Method, type: Type, param: KParameter, name: String, clazz: Class): Pair { + val returnType = resolveTypeVariables(getter.genericReturnType, type) val paramToken = TypeToken.of(param.type.javaType) val rawParamType = TypeToken.of(paramToken.rawType) - return paramToken.isSupertypeOf(getterReturnType) - || paramToken.isSupertypeOf(rawGetterReturnType) - // cope with the case where the constructor parameter is a generic type (T etc) but we - // can discover it's raw type. When bounded this wil be the bounding type, unbounded - // generics this will be object - || rawParamType.isSupertypeOf(getterReturnType) - || rawParamType.isSupertypeOf(rawGetterReturnType) + if (!(paramToken.isSupertypeOf(returnType) + || paramToken.isSupertypeOf(getter.genericReturnType) + // cope with the case where the constructor parameter is a generic type (T etc) but we + // can discover it's raw type. When bounded this wil be the bounding type, unbounded + // generics this will be object + || rawParamType.isSupertypeOf(returnType) + || rawParamType.isSupertypeOf(getter.genericReturnType))) { + throw AMQPNotSerializableException( + type, + "Property - \"$name\" - has type \"$returnType\" on \"$clazz\" " + + "but differs from constructor parameter type \"${param.type.javaType}\"") + } + + return PublicPropertyReader(getter) to returnType } private fun propertiesForSerializationFromAbstract( clazz: Class<*>, type: Type, - factory: SerializerFactory): List { - val properties = clazz.propertyDescriptors() - - return mutableListOf().apply { - properties.toList().withIndex().forEach { - val getter = it.value.second.getter ?: return@forEach - if (it.value.second.field == null) return@forEach + factory: SerializerFactory): List = + clazz.propertyDescriptors().asSequence().withIndex().map { (index, entry) -> + val (name, property) = entry + if (property.getter == null || property.field == null) return@map null + val getter = property.getter val returnType = resolveTypeVariables(getter.genericReturnType, type) - this += PropertyAccessorConstructor( - it.index, - PropertySerializer.make(it.value.first, PublicPropertyReader(getter), returnType, factory)) - } - } -} -internal fun interfacesForSerialization(type: Type, serializerFactory: SerializerFactory): List { - val interfaces = LinkedHashSet() - exploreType(type, interfaces, serializerFactory) - return interfaces.toList() -} + PropertyAccessorConstructor( + index, + PropertySerializer.make(name, PublicPropertyReader(getter), returnType, factory)) + }.filterNotNull().toList() -private fun exploreType(type: Type?, interfaces: MutableSet, serializerFactory: SerializerFactory) { - val clazz = type?.asClass() - if (clazz != null) { - if (clazz.isInterface) { - if (serializerFactory.whitelist.isNotWhitelisted(clazz)) return // We stop exploring once we reach a branch that has no `CordaSerializable` annotation or whitelisting. - else interfaces += type - } - for (newInterface in clazz.genericInterfaces) { - if (newInterface !in interfaces) { - exploreType(resolveTypeVariables(newInterface, type), interfaces, serializerFactory) - } - } - val superClass = clazz.genericSuperclass ?: return - exploreType(resolveTypeVariables(superClass, type), interfaces, serializerFactory) +internal fun interfacesForSerialization(type: Type, serializerFactory: SerializerFactory): List = + exploreType(type, serializerFactory).toList() + +private fun exploreType(type: Type, serializerFactory: SerializerFactory, interfaces: MutableSet = LinkedHashSet()): MutableSet { + val clazz = type.asClass() + + if (clazz.isInterface) { + // Ignore classes we've already seen, and stop exploring once we reach a branch that has no `CordaSerializable` + // annotation or whitelisting. + if (clazz in interfaces || serializerFactory.whitelist.isNotWhitelisted(clazz)) return interfaces + else interfaces += type } + + (clazz.genericInterfaces.asSequence() + clazz.genericSuperclass) + .filterNotNull() + .forEach { exploreType(resolveTypeVariables(it, type), serializerFactory, interfaces) } + + return interfaces } /** @@ -469,21 +287,23 @@ fun resolveTypeVariables(actualType: Type, contextType: Type?): Type { } } -internal fun Type.asClass(): Class<*>? { - return when { - this is Class<*> -> this - this is ParameterizedType -> this.rawType.asClass() - this is GenericArrayType -> this.genericComponentType.asClass()?.arrayClass() - this is TypeVariable<*> -> this.bounds.first().asClass() - this is WildcardType -> this.upperBounds.first().asClass() - else -> null +internal fun Type.asClass(): Class<*> { + return when(this) { + is Class<*> -> this + is ParameterizedType -> this.rawType.asClass() + is GenericArrayType -> this.genericComponentType.asClass().arrayClass() + is TypeVariable<*> -> this.bounds.first().asClass() + is WildcardType -> this.upperBounds.first().asClass() + // Per https://docs.oracle.com/javase/8/docs/api/java/lang/reflect/Type.html, + // there is nothing else that it can be, so this can never happen. + else -> throw UnsupportedOperationException("Cannot convert $this to class") } } internal fun Type.asArray(): Type? { - return when { - this is Class<*> -> this.arrayClass() - this is ParameterizedType -> DeserializedGenericArrayType(this) + return when(this) { + is Class<*> -> this.arrayClass() + is ParameterizedType -> DeserializedGenericArrayType(this) else -> null } } @@ -516,7 +336,7 @@ internal fun Type.isSubClassOf(type: Type): Boolean { // ByteArrays, primitives and boxed primitives are not stored in the object history internal fun suitableForObjectReference(type: Type): Boolean { val clazz = type.asClass() - return type != ByteArray::class.java && (clazz != null && !clazz.isPrimitive && !Primitives.unwrap(clazz).isPrimitive) + return type != ByteArray::class.java && (!clazz.isPrimitive && !Primitives.unwrap(clazz).isPrimitive) } /** @@ -529,7 +349,7 @@ internal enum class CommonPropertyNames { fun ClassWhitelist.requireWhitelisted(type: Type) { - if (!this.isWhitelisted(type.asClass()!!)) { + if (!this.isWhitelisted(type.asClass())) { throw AMQPNotSerializableException( type, "Class \"$type\" is not on the whitelist or annotated with @CordaSerializable.") diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializerFactory.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializerFactory.kt index 23a7b16951..aedbef175f 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializerFactory.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializerFactory.kt @@ -11,7 +11,6 @@ package net.corda.serialization.internal.amqp import com.google.common.primitives.Primitives -import com.google.common.reflect.TypeResolver import net.corda.core.DeleteForDJVM import net.corda.core.KeepForDJVM import net.corda.core.StubOutForDJVM @@ -64,7 +63,7 @@ open class SerializerFactory( val whitelist: ClassWhitelist, val classCarpenter: ClassCarpenter, private val evolutionSerializerGetter: EvolutionSerializerGetterBase = EvolutionSerializerGetter(), - val fingerPrinter: FingerPrinter = SerializerFingerPrinter(), + val fingerPrinterConstructor: (SerializerFactory) -> FingerPrinter = ::SerializerFingerPrinter, private val serializersByType: MutableMap>, val serializersByDescriptor: MutableMap>, private val customSerializers: MutableList, @@ -76,13 +75,13 @@ open class SerializerFactory( constructor(whitelist: ClassWhitelist, classCarpenter: ClassCarpenter, evolutionSerializerGetter: EvolutionSerializerGetterBase = EvolutionSerializerGetter(), - fingerPrinter: FingerPrinter = SerializerFingerPrinter(), + fingerPrinterConstructor: (SerializerFactory) -> FingerPrinter = ::SerializerFingerPrinter, onlyCustomSerializers: Boolean = false ) : this( whitelist, classCarpenter, evolutionSerializerGetter, - fingerPrinter, + fingerPrinterConstructor, ConcurrentHashMap(), ConcurrentHashMap(), CopyOnWriteArrayList(), @@ -96,18 +95,16 @@ open class SerializerFactory( carpenterClassLoader: ClassLoader, lenientCarpenter: Boolean = false, evolutionSerializerGetter: EvolutionSerializerGetterBase = EvolutionSerializerGetter(), - fingerPrinter: FingerPrinter = SerializerFingerPrinter(), + fingerPrinterConstructor: (SerializerFactory) -> FingerPrinter = ::SerializerFingerPrinter, onlyCustomSerializers: Boolean = false ) : this( whitelist, ClassCarpenterImpl(whitelist, carpenterClassLoader, lenientCarpenter), evolutionSerializerGetter, - fingerPrinter, + fingerPrinterConstructor, onlyCustomSerializers) - init { - fingerPrinter.setOwner(this) - } + val fingerPrinter by lazy { fingerPrinterConstructor(this) } val classloader: ClassLoader get() = classCarpenter.classloader @@ -128,11 +125,9 @@ open class SerializerFactory( // can be useful to enable but will be *extremely* chatty if you do logger.trace { "Get Serializer for $actualClass ${declaredType.typeName}" } - val declaredClass = declaredType.asClass() ?: throw AMQPNotSerializableException( - declaredType, - "Declared types of $declaredType are not supported.") - - val actualType: Type = inferTypeVariables(actualClass, declaredClass, declaredType) ?: declaredType + val declaredClass = declaredType.asClass() + val actualType: Type = if (actualClass == null) declaredType + else inferTypeVariables(actualClass, declaredClass, declaredType) ?: declaredType val serializer = when { // Declared class may not be set to Collection, but actual class could be a collection. @@ -176,78 +171,6 @@ open class SerializerFactory( return serializer } - /** - * Try and infer concrete types for any generics type variables for the actual class encountered, - * based on the declared type. - */ - // TODO: test GenericArrayType - private fun inferTypeVariables(actualClass: Class<*>?, declaredClass: Class<*>, - declaredType: Type): Type? = when (declaredType) { - is ParameterizedType -> inferTypeVariables(actualClass, declaredClass, declaredType) - // Nothing to infer, otherwise we'd have ParameterizedType - is Class<*> -> actualClass - is GenericArrayType -> { - val declaredComponent = declaredType.genericComponentType - inferTypeVariables(actualClass?.componentType, declaredComponent.asClass()!!, declaredComponent)?.asArray() - } - is TypeVariable<*> -> actualClass - is WildcardType -> actualClass - else -> null - } - - /** - * Try and infer concrete types for any generics type variables for the actual class encountered, based on the declared - * type, which must be a [ParameterizedType]. - */ - private fun inferTypeVariables(actualClass: Class<*>?, declaredClass: Class<*>, declaredType: ParameterizedType): Type? { - if (actualClass == null || declaredClass == actualClass) { - return null - } else if (declaredClass.isAssignableFrom(actualClass)) { - return if (actualClass.typeParameters.isNotEmpty()) { - // The actual class can never have type variables resolved, due to the JVM's use of type erasure, so let's try and resolve them - // Search for declared type in the inheritance hierarchy and then see if that fills in all the variables - val implementationChain: List? = findPathToDeclared(actualClass, declaredType, mutableListOf()) - if (implementationChain != null) { - val start = implementationChain.last() - val rest = implementationChain.dropLast(1).drop(1) - val resolver = rest.reversed().fold(TypeResolver().where(start, declaredType)) { resolved, chainEntry -> - val newResolved = resolved.resolveType(chainEntry) - TypeResolver().where(chainEntry, newResolved) - } - // The end type is a special case as it is a Class, so we need to fake up a ParameterizedType for it to get the TypeResolver to do anything. - val endType = DeserializedParameterizedType(actualClass, actualClass.typeParameters) - val resolvedType = resolver.resolveType(endType) - resolvedType - } else throw AMQPNotSerializableException(declaredType, - "No inheritance path between actual $actualClass and declared $declaredType.") - } else actualClass - } else throw AMQPNotSerializableException( - declaredType, - "Found object of type $actualClass in a property expecting $declaredType") - } - - // Stop when reach declared type or return null if we don't find it. - private fun findPathToDeclared(startingType: Type, declaredType: Type, chain: MutableList): List? { - chain.add(startingType) - val startingClass = startingType.asClass() - if (startingClass == declaredType.asClass()) { - // We're done... - return chain - } - // Now explore potential options of superclass and all interfaces - val superClass = startingClass?.genericSuperclass - val superClassChain = if (superClass != null) { - val resolved = TypeResolver().where(startingClass.asParameterizedType(), startingType.asParameterizedType()).resolveType(superClass) - findPathToDeclared(resolved, declaredType, ArrayList(chain)) - } else null - if (superClassChain != null) return superClassChain - for (iface in startingClass?.genericInterfaces ?: emptyArray()) { - val resolved = TypeResolver().where(startingClass!!.asParameterizedType(), startingType.asParameterizedType()).resolveType(iface) - return findPathToDeclared(resolved, declaredType, ArrayList(chain)) ?: continue - } - return null - } - /** * Lookup and manufacture a serializer for the given AMQP type descriptor, assuming we also have the necessary types * contained in the [Schema]. @@ -359,7 +282,7 @@ open class SerializerFactory( // TODO: class loader logic, and compare the schema. val type = typeForName(typeNotation.name, classloader) return get( - type.asClass() ?: throw AMQPNotSerializableException(type, "Unable to build composite type for $type"), + type.asClass(), type) } @@ -412,7 +335,7 @@ open class SerializerFactory( // super type. Could be done, but do we need it? for (customSerializer in customSerializers) { if (customSerializer.isSerializerFor(clazz)) { - val declaredSuperClass = declaredType.asClass()?.superclass + val declaredSuperClass = declaredType.asClass().superclass return if (declaredSuperClass == null diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/TypeParameterUtils.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/TypeParameterUtils.kt new file mode 100644 index 0000000000..72720add79 --- /dev/null +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/TypeParameterUtils.kt @@ -0,0 +1,94 @@ +package net.corda.serialization.internal.amqp + +import com.google.common.reflect.TypeResolver +import java.lang.reflect.* + +/** + * Try and infer concrete types for any generics type variables for the actual class encountered, + * based on the declared type. + */ +// TODO: test GenericArrayType +fun inferTypeVariables(actualClass: Class<*>, + declaredClass: Class<*>, + declaredType: Type): Type? = when (declaredType) { + is ParameterizedType -> inferTypeVariables(actualClass, declaredClass, declaredType) + is GenericArrayType -> { + val declaredComponent = declaredType.genericComponentType + inferTypeVariables(actualClass.componentType, declaredComponent.asClass(), declaredComponent)?.asArray() + } + // Nothing to infer, otherwise we'd have ParameterizedType + is Class<*> -> actualClass + is TypeVariable<*> -> actualClass + is WildcardType -> actualClass + else -> throw UnsupportedOperationException("Cannot infer type variables for type $declaredType") +} + +/** + * Try and infer concrete types for any generics type variables for the actual class encountered, based on the declared + * type, which must be a [ParameterizedType]. + */ +private fun inferTypeVariables(actualClass: Class<*>, declaredClass: Class<*>, declaredType: ParameterizedType): Type? { + if (declaredClass == actualClass) { + return null + } + + if (!declaredClass.isAssignableFrom(actualClass)) { + throw AMQPNotSerializableException( + declaredType, + "Found object of type $actualClass in a property expecting $declaredType") + } + + if (actualClass.typeParameters.isEmpty()) { + return actualClass + } + // The actual class can never have type variables resolved, due to the JVM's use of type erasure, so let's try and resolve them + // Search for declared type in the inheritance hierarchy and then see if that fills in all the variables + val implementationChain: List = findPathToDeclared(actualClass, declaredType)?.toList() + ?: throw AMQPNotSerializableException( + declaredType, + "No inheritance path between actual $actualClass and declared $declaredType.") + + val start = implementationChain.last() + val rest = implementationChain.dropLast(1).drop(1) + val resolver = rest.reversed().fold(TypeResolver().where(start, declaredType)) { resolved, chainEntry -> + val newResolved = resolved.resolveType(chainEntry) + TypeResolver().where(chainEntry, newResolved) + } + // The end type is a special case as it is a Class, so we need to fake up a ParameterizedType for it to get the TypeResolver to do anything. + val endType = DeserializedParameterizedType(actualClass, actualClass.typeParameters) + return resolver.resolveType(endType) +} + +// Stop when reach declared type or return null if we don't find it. +private fun findPathToDeclared(startingType: Type, declaredType: Type, chain: Sequence = emptySequence()): Sequence? { + val extendedChain = chain + startingType + val startingClass = startingType.asClass() + + if (startingClass == declaredType.asClass()) { + // We're done... + return extendedChain + } + + val resolver = { type: Type -> + TypeResolver().where( + startingClass.asParameterizedType(), + startingType.asParameterizedType()) + .resolveType(type) + } + + // Now explore potential options of superclass and all interfaces + return findPathViaGenericSuperclass(startingClass, resolver, declaredType, extendedChain) + ?: findPathViaInterfaces(startingClass, resolver, declaredType, extendedChain) +} + +private fun findPathViaInterfaces(startingClass: Class<*>, resolver: (Type) -> Type, declaredType: Type, extendedChain: Sequence): Sequence? = + startingClass.genericInterfaces.asSequence().map { + findPathToDeclared(resolver(it), declaredType, extendedChain) + }.filterNotNull().firstOrNull() + + +private fun findPathViaGenericSuperclass(startingClass: Class<*>, resolver: (Type) -> Type, declaredType: Type, extendedChain: Sequence): Sequence? { + val superClass = startingClass.genericSuperclass ?: return null + return findPathToDeclared(resolver(superClass), declaredType, extendedChain) +} + diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/custom/ThrowableSerializer.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/custom/ThrowableSerializer.kt index 75687bc9ad..214966ba50 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/custom/ThrowableSerializer.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/custom/ThrowableSerializer.kt @@ -35,7 +35,7 @@ class ThrowableSerializer(factory: SerializerFactory) : CustomSerializer.Proxy + propertiesForSerializationFromConstructor(constructor, obj.javaClass, factory).forEach { property -> extraProperties[property.serializer.name] = property.serializer.propertyReader.read(obj) } } catch (e: NotSerializableException) { @@ -62,7 +62,7 @@ class ThrowableSerializer(factory: SerializerFactory) : CustomSerializer.Proxy { validateSchema(schema) - // Walk up the inheritance hierarchy and then start walking back down once we either hit the top, or - // find a class we haven't generated yet. + // Walk up the inheritance hierarchy until we hit either the top or a class we've already generated, + // then walk back down it generating classes. val hierarchy = ArrayList() hierarchy += schema var cursor = schema.superclass @@ -316,16 +316,16 @@ class ClassCarpenterImpl @JvmOverloads constructor (override val whitelist: Clas visitInsn(DUP) var idx = 0 - schema.fields.forEach { + schema.fields.keys.forEach { key -> visitInsn(DUP) visitIntInsn(BIPUSH, idx) visitTypeInsn(NEW, schema.jvmName) visitInsn(DUP) - visitLdcInsn(it.key) + visitLdcInsn(key) visitIntInsn(BIPUSH, idx++) visitMethodInsn(INVOKESPECIAL, schema.jvmName, "", "(L$jlString;I)V", false) visitInsn(DUP) - visitFieldInsn(PUTSTATIC, schema.jvmName, it.key, "L${schema.jvmName};") + visitFieldInsn(PUTSTATIC, schema.jvmName, key, "L${schema.jvmName};") visitInsn(AASTORE) } @@ -391,20 +391,18 @@ class ClassCarpenterImpl @JvmOverloads constructor (override val whitelist: Clas visitCode() // Calculate the super call. - val superclassFields = schema.superclass?.fieldsIncludingSuperclasses() ?: emptyMap() visitVarInsn(ALOAD, 0) val sc = schema.superclass + var slot = 1 if (sc == null) { visitMethodInsn(INVOKESPECIAL, jlObject, "", "()V", false) } else { - var slot = 1 - superclassFields.values.forEach { slot += load(slot, it) } + slot = sc.fieldsIncludingSuperclasses().values.fold(slot) { acc, field -> acc + load(acc, field) } val superDesc = sc.descriptorsIncludingSuperclasses().values.joinToString("") visitMethodInsn(INVOKESPECIAL, sc.jvmName, "", "($superDesc)V", false) } // Assign the fields from parameters. - var slot = 1 + superclassFields.size for ((name, field) in schema.fields) { (field as ClassField).nullTest(this, slot) diff --git a/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/FingerPrinterTesting.kt b/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/FingerPrinterTesting.kt index 8fd3faad27..c80fceb094 100644 --- a/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/FingerPrinterTesting.kt +++ b/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/FingerPrinterTesting.kt @@ -25,10 +25,6 @@ class FingerPrinterTesting : FingerPrinter { return cache.computeIfAbsent(type) { index++.toString() } } - override fun setOwner(factory: SerializerFactory) { - return - } - @Suppress("UNUSED") fun changeFingerprint(type: Type) { cache.computeIfAbsent(type) { "" }.apply { index++.toString() } @@ -57,7 +53,7 @@ class FingerPrinterTestingTests { AllWhitelist, ClassLoader.getSystemClassLoader(), evolutionSerializerGetter = EvolutionSerializerGetterTesting(), - fingerPrinter = FingerPrinterTesting()) + fingerPrinterConstructor = { _ -> FingerPrinterTesting() }) val blob = TestSerializationOutput(VERBOSE, factory).serializeAndReturnSchema(C(1, 2L)) diff --git a/serialization/src/test/kotlin/net/corda/serialization/internal/carpenter/ClassCarpenterTest.kt b/serialization/src/test/kotlin/net/corda/serialization/internal/carpenter/ClassCarpenterTest.kt index 6796986270..36642e6ac4 100644 --- a/serialization/src/test/kotlin/net/corda/serialization/internal/carpenter/ClassCarpenterTest.kt +++ b/serialization/src/test/kotlin/net/corda/serialization/internal/carpenter/ClassCarpenterTest.kt @@ -139,6 +139,27 @@ class ClassCarpenterTest { assertEquals("B{a=xa, b=xb}", i.toString()) } + /** + * Tests the fix for [Corda-1945](https://r3-cev.atlassian.net/secure/RapidBoard.jspa?rapidView=83&modal=detail&selectedIssue=CORDA-1945) + */ + @Test + fun `superclasses with double-size primitive constructor parameters`() { + val schema1 = ClassSchema( + "gen.A", + mapOf("a" to NonNullableField(Long::class.javaPrimitiveType!!))) + + val schema2 = ClassSchema( + "gen.B", + mapOf("b" to NonNullableField(String::class.java)), + schema1) + + val clazz = cc.build(schema2) + val i = clazz.constructors[0].newInstance(1L, "xb") as SimpleFieldAccess + assertEquals(1L, i["a"]) + assertEquals("xb", i["b"]) + assertEquals("B{a=1, b=xb}", i.toString()) + } + @Test fun interfaces() { val schema1 = ClassSchema( diff --git a/tools/shell/src/main/kotlin/net/corda/tools/shell/InteractiveShell.kt b/tools/shell/src/main/kotlin/net/corda/tools/shell/InteractiveShell.kt index 6245b4d387..25f61171a2 100644 --- a/tools/shell/src/main/kotlin/net/corda/tools/shell/InteractiveShell.kt +++ b/tools/shell/src/main/kotlin/net/corda/tools/shell/InteractiveShell.kt @@ -57,8 +57,7 @@ import java.io.FileDescriptor import java.io.FileInputStream import java.io.InputStream import java.io.PrintWriter -import java.lang.reflect.InvocationTargetException -import java.lang.reflect.UndeclaredThrowableException +import java.lang.reflect.* import java.nio.file.Path import java.util.* import java.util.concurrent.CountDownLatch @@ -310,6 +309,38 @@ object InteractiveShell { override fun toString() = (listOf("No applicable constructor for flow. Problems were:") + errors).joinToString(System.lineSeparator()) } + /** + * Tidies up a possibly generic type name by chopping off the package names of classes in a hard-coded set of + * hierarchies that are known to be widely used and recognised, and also not have (m)any ambiguous names in them. + * + * This is used for printing error messages when something doesn't match. + */ + private fun maybeAbbreviateGenericType(type: Type, extraRecognisedPackage: String): String { + val packagesToAbbreviate = listOf("java.", "net.corda.core.", "kotlin.", extraRecognisedPackage) + + fun shouldAbbreviate(typeName: String) = packagesToAbbreviate.any { typeName.startsWith(it) } + fun abbreviated(typeName: String) = if (shouldAbbreviate(typeName)) typeName.split('.').last() else typeName + + fun innerLoop(type: Type): String = when (type) { + is ParameterizedType -> { + val args: List = type.actualTypeArguments.map(::innerLoop) + abbreviated(type.rawType.typeName) + '<' + args.joinToString(", ") + '>' + } + is GenericArrayType -> { + innerLoop(type.genericComponentType) + "[]" + } + is Class<*> -> { + if (type.isArray) + abbreviated(type.simpleName) + else + abbreviated(type.name).replace('$', '.') + } + else -> type.toString() + } + + return innerLoop(type) + } + // TODO: This utility is generally useful and might be better moved to the node class, or an RPC, if we can commit to making it stable API. /** * Given a [FlowLogic] class and a string in one-line Yaml form, finds an applicable constructor and starts @@ -329,10 +360,17 @@ object InteractiveShell { // and keep track of the reasons we failed so we can print them out if no constructors are usable. val parser = StringToMethodCallParser(clazz, om) val errors = ArrayList() + + val classPackage = clazz.packageName for (ctor in clazz.constructors) { var paramNamesFromConstructor: List? = null + fun getPrototype(): List { - val argTypes = ctor.genericParameterTypes.map { it.typeName } + val argTypes = ctor.genericParameterTypes.map { it: Type -> + // If the type name is in the net.corda.core or java namespaces, chop off the package name + // because these hierarchies don't have (m)any ambiguous names and the extra detail is just noise. + maybeAbbreviateGenericType(it, classPackage) + } return paramNamesFromConstructor!!.zip(argTypes).map { (name, type) -> "$name: $type" } } diff --git a/tools/shell/src/test/java/net/corda/tools/shell/InteractiveShellJavaTest.java b/tools/shell/src/test/java/net/corda/tools/shell/InteractiveShellJavaTest.java index 7a66ac3881..09f91f5148 100644 --- a/tools/shell/src/test/java/net/corda/tools/shell/InteractiveShellJavaTest.java +++ b/tools/shell/src/test/java/net/corda/tools/shell/InteractiveShellJavaTest.java @@ -52,8 +52,8 @@ public class InteractiveShellJavaTest { } } - public FlowA(Integer b) { - this(b.toString()); + public FlowA(int b) { + this(Integer.valueOf(b).toString()); } public FlowA(Integer b, String c) { @@ -111,6 +111,9 @@ public class InteractiveShellJavaTest { this.a = a; } + public FlowB(Amount amount, int abc) { + } + @Nullable @Override public ProgressTracker getProgressTracker() { @@ -142,6 +145,7 @@ public class InteractiveShellJavaTest { this.label = label; } + @SuppressWarnings("unused") // Used via reflection. public String getLabel() { return label; } @@ -161,17 +165,17 @@ public class InteractiveShellJavaTest { private void check(String input, String expected, Class flowClass) throws InteractiveShell.NoApplicableConstructor { InteractiveShell.INSTANCE.runFlowFromString((clazz, args) -> { - StringFlow instance = null; try { instance = (StringFlow)clazz.getConstructor(Arrays.stream(args).map(Object::getClass).toArray(Class[]::new)).newInstance(args); } catch (Exception e) { System.out.println(e); + throw new RuntimeException(e); } output = instance.getA(); OpenFuture future = CordaFutureImplKt.openFuture(); future.set("ABC"); - return new FlowProgressHandleImpl(StateMachineRunId.Companion.createRandom(), future, Observable.just("Some string")); + return new FlowProgressHandleImpl(StateMachineRunId.Companion.createRandom(), future, Observable.just("Some string")); }, input, flowClass, om); assertEquals(input, expected, output); } @@ -246,4 +250,14 @@ public class InteractiveShellJavaTest { public void unwrapLambda() throws InteractiveShell.NoApplicableConstructor { check("party: \"" + megaCorp.getName() + "\", a: Bambam", "Bambam", FlowB.class); } + + @Test + public void niceErrors() { + // Most cases are checked in the Kotlin test, so we only check raw types here. + try { + check("amount: $100", "", FlowB.class); + } catch (InteractiveShell.NoApplicableConstructor e) { + assertEquals("[amount: Amount, abc: int]: missing parameter abc", e.getErrors().get(1)); + } + } } diff --git a/tools/shell/src/test/kotlin/net/corda/tools/shell/InteractiveShellTest.kt b/tools/shell/src/test/kotlin/net/corda/tools/shell/InteractiveShellTest.kt index 6884fa7371..6f47b328a2 100644 --- a/tools/shell/src/test/kotlin/net/corda/tools/shell/InteractiveShellTest.kt +++ b/tools/shell/src/test/kotlin/net/corda/tools/shell/InteractiveShellTest.kt @@ -24,12 +24,13 @@ import net.corda.core.internal.concurrent.openFuture import net.corda.core.messaging.FlowProgressHandleImpl import net.corda.core.utilities.ProgressTracker import net.corda.node.services.identity.InMemoryIdentityService -import net.corda.testing.internal.DEV_ROOT_CA import net.corda.testing.core.TestIdentity +import net.corda.testing.internal.DEV_ROOT_CA import org.junit.Test import rx.Observable import java.util.* import kotlin.test.assertEquals +import kotlin.test.assertFailsWith class InteractiveShellTest { companion object { @@ -38,7 +39,7 @@ class InteractiveShellTest { @Suppress("UNUSED") class FlowA(val a: String) : FlowLogic() { - constructor(b: Int?) : this(b.toString()) + constructor(b: Int) : this(b.toString()) constructor(b: Int?, c: String) : this(b.toString() + c) constructor(amount: Amount) : this(amount.toString()) constructor(pair: Pair, SecureHash.SHA256>) : this(pair.toString()) @@ -58,7 +59,6 @@ class InteractiveShellTest { private fun check(input: String, expected: String) { var output: String? = null InteractiveShell.runFlowFromString({ clazz, args -> - val instance = clazz.getConstructor(*args.map { it!!::class.java }.toTypedArray()).newInstance(*args) as FlowA output = instance.a val future = openFuture() @@ -111,6 +111,27 @@ class InteractiveShellTest { @Test(expected = InteractiveShell.NoApplicableConstructor::class) fun flowTooManyParams() = check("b: 12, c: Yo, d: Bar", "") + @Test + fun niceTypeNamesInErrors() { + val e = assertFailsWith { + check("", expected = "") + } + val correct = setOf( + "[amounts: Amount[]]: missing parameter amounts", + "[amount: Amount]: missing parameter amount", + "[pair: Pair, SecureHash.SHA256>]: missing parameter pair", + "[party: Party]: missing parameter party", + "[b: Integer, amount: Amount]: missing parameter b", + "[b: String[]]: missing parameter b", + "[b: Integer, c: String]: missing parameter b", + "[a: String]: missing parameter a", + "[b: int]: missing parameter b" + ) + val errors = e.errors.toHashSet() + errors.removeAll(correct) + assert(errors.isEmpty()) { errors.joinToString(", ") } + } + @Test fun party() = check("party: \"${megaCorp.name}\"", megaCorp.name.toString())