From c533e0ab0ca38d39d055aa9112052213cb69fe7b Mon Sep 17 00:00:00 2001 From: Dominic Fox <40790090+distributedleetravis@users.noreply.github.com> Date: Mon, 26 Nov 2018 20:28:18 +0000 Subject: [PATCH] CORDA-2255 reduce noise from serialisation warnings (#4299) Suppress warnings when constructing NonConstructible types that are wrapped by Opaque, and reduce warning level to info. Construct wrapped LocalTypeInformation for Opaque types immediately, rather than deferring with a thunk. --- .../amqp/custom/ThrowableSerializer.kt | 3 +- .../internal/model/LocalTypeInformation.kt | 19 +--------- .../model/LocalTypeInformationBuilder.kt | 37 ++++++++++--------- .../internal/model/TypeIdentifier.kt | 4 +- 4 files changed, 25 insertions(+), 38 deletions(-) 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 b3082a629e..ba93143cd4 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 @@ -7,7 +7,6 @@ import net.corda.core.serialization.SerializationFactory import net.corda.core.utilities.contextLogger import net.corda.serialization.internal.amqp.* import net.corda.serialization.internal.model.LocalConstructorInformation -import net.corda.serialization.internal.model.LocalPropertyInformation import net.corda.serialization.internal.model.LocalTypeInformation import java.io.NotSerializableException @@ -26,7 +25,7 @@ class ThrowableSerializer(factory: LocalSerializerFactory) : CustomSerializer.Pr is LocalTypeInformation.NonComposable -> constructor ?: throw NotSerializableException("$this has no deserialization constructor") is LocalTypeInformation.Composable -> constructor - is LocalTypeInformation.Opaque -> expand.constructor + is LocalTypeInformation.Opaque -> wrapped.constructor else -> throw NotSerializableException("$this has no deserialization constructor") } diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt index 9c815fe84f..0d4c555269 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt @@ -91,7 +91,7 @@ sealed class LocalTypeInformation { is LocalTypeInformation.Abstract -> properties is LocalTypeInformation.AnInterface -> properties is LocalTypeInformation.NonComposable -> properties - is LocalTypeInformation.Opaque -> expand.propertiesOrEmptyMap + is LocalTypeInformation.Opaque -> wrapped.propertiesOrEmptyMap else -> emptyMap() } @@ -154,22 +154,7 @@ sealed class LocalTypeInformation { * May in fact be a more complex class, but is treated as if atomic, i.e. we don't further expand its properties. */ data class Opaque(override val observedType: Class<*>, override val typeIdentifier: TypeIdentifier, - private val _expand: () -> LocalTypeInformation) : LocalTypeInformation() { - /** - * In some rare cases, e.g. during Exception serialisation, we may want to "look inside" an opaque type. - */ - val expand: LocalTypeInformation by lazy { _expand() } - - // Custom equals / hashcode because otherwise the "expand" lambda makes equality harder to reason about. - override fun equals(other: Any?): Boolean = - other is Cycle && - other.observedType == observedType && - other.typeIdentifier == typeIdentifier - - override fun hashCode(): Int = Objects.hash(observedType, typeIdentifier) - - override fun toString(): String = "Opaque($observedType, $typeIdentifier)" - } + val wrapped: LocalTypeInformation) : LocalTypeInformation() /** * Represents a scalar type such as [Int]. diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformationBuilder.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformationBuilder.kt index 5e596d6465..b99abb9bda 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformationBuilder.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformationBuilder.kt @@ -94,12 +94,11 @@ internal data class LocalTypeInformationBuilder(val lookup: LocalTypeLookup, buildInterfaceInformation(type)) type.isInterface -> buildInterface(type, typeIdentifier, emptyList()) type.isAbstractClass -> buildAbstract(type, typeIdentifier, emptyList()) - else -> when { - isOpaque -> LocalTypeInformation.Opaque(type, typeIdentifier) { - buildNonAtomic(type, type, typeIdentifier, emptyList()) - } - else -> buildNonAtomic(type, type, typeIdentifier, emptyList()) - } + isOpaque -> LocalTypeInformation.Opaque( + type, + typeIdentifier, + buildNonAtomic(type, type, typeIdentifier, emptyList(), true)) + else -> buildNonAtomic(type, type, typeIdentifier, emptyList()) } } @@ -118,12 +117,10 @@ internal data class LocalTypeInformationBuilder(val lookup: LocalTypeLookup, } rawType.isInterface -> buildInterface(type, typeIdentifier, buildTypeParameterInformation(type)) rawType.isAbstractClass -> buildAbstract(type, typeIdentifier, buildTypeParameterInformation(type)) - else -> when { - isOpaque -> LocalTypeInformation.Opaque(rawType, typeIdentifier) { - buildNonAtomic(rawType, type, typeIdentifier, buildTypeParameterInformation(type)) - } - else -> buildNonAtomic(rawType, type, typeIdentifier, buildTypeParameterInformation(type)) - } + isOpaque -> LocalTypeInformation.Opaque(rawType, + typeIdentifier, + buildNonAtomic(rawType, type, typeIdentifier, buildTypeParameterInformation(type), true)) + else -> buildNonAtomic(rawType, type, typeIdentifier, buildTypeParameterInformation(type)) } } @@ -159,13 +156,15 @@ internal data class LocalTypeInformationBuilder(val lookup: LocalTypeLookup, * Rather than throwing an exception if a type is [NonComposable], we capture its type information so that it can * still be used to _serialize_ values, or as the basis for deciding on an evolution strategy. */ - private fun buildNonAtomic(rawType: Class<*>, type: Type, typeIdentifier: TypeIdentifier, typeParameterInformation: List): LocalTypeInformation { + private fun buildNonAtomic(rawType: Class<*>, type: Type, typeIdentifier: TypeIdentifier, typeParameterInformation: List, suppressWarning: Boolean = false): LocalTypeInformation { val superclassInformation = buildSuperclassInformation(type) val interfaceInformation = buildInterfaceInformation(type) val observedConstructor = constructorForDeserialization(type) if (observedConstructor == null) { - logger.warn("No unique deserialisation constructor found for class $rawType, type is marked as non-composable") + if (!suppressWarning) { + logger.info("No unique deserialisation constructor found for class $rawType, type is marked as non-composable") + } return LocalTypeInformation.NonComposable(type, typeIdentifier, null, buildReadOnlyProperties(rawType), superclassInformation, interfaceInformation, typeParameterInformation) } @@ -176,10 +175,12 @@ internal data class LocalTypeInformationBuilder(val lookup: LocalTypeLookup, val hasNonComposableProperties = properties.values.any { it.type is LocalTypeInformation.NonComposable } if (!propertiesSatisfyConstructor(constructorInformation, properties) || hasNonComposableProperties) { - if (hasNonComposableProperties) { - logger.warn("Type ${type.typeName} has non-composable properties and has been marked as non-composable") - } else { - logger.warn("Properties of type ${type.typeName} do not satisfy its constructor, type has been marked as non-composable") + if (!suppressWarning) { + if (hasNonComposableProperties) { + logger.info("Type ${type.typeName} has non-composable properties and has been marked as non-composable") + } else { + logger.info("Properties of type ${type.typeName} do not satisfy its constructor, type has been marked as non-composable") + } } return LocalTypeInformation.NonComposable(type, typeIdentifier, constructorInformation, properties, superclassInformation, interfaceInformation, typeParameterInformation) diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/model/TypeIdentifier.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/model/TypeIdentifier.kt index 95e4a6132a..5777c96cea 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/model/TypeIdentifier.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/model/TypeIdentifier.kt @@ -90,7 +90,9 @@ sealed class TypeIdentifier { (type.rawType as Class<*>).name, type.ownerType?.let { forGenericType(it) }, type.actualTypeArguments.map { - forGenericType(it.resolveAgainst(resolutionContext)) + val resolved = it.resolveAgainst(resolutionContext) + // Avoid cycles, e.g. Enum where E resolves to Enum + if (resolved == type) UnknownType else forGenericType(resolved) }) is Class<*> -> forClass(type) is GenericArrayType -> ArrayOf(forGenericType(type.genericComponentType.resolveAgainst(resolutionContext)))