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.
This commit is contained in:
Dominic Fox 2018-11-26 20:28:18 +00:00 committed by Shams Asari
parent b881fbd330
commit c533e0ab0c
4 changed files with 25 additions and 38 deletions

View File

@ -7,7 +7,6 @@ import net.corda.core.serialization.SerializationFactory
import net.corda.core.utilities.contextLogger import net.corda.core.utilities.contextLogger
import net.corda.serialization.internal.amqp.* import net.corda.serialization.internal.amqp.*
import net.corda.serialization.internal.model.LocalConstructorInformation import net.corda.serialization.internal.model.LocalConstructorInformation
import net.corda.serialization.internal.model.LocalPropertyInformation
import net.corda.serialization.internal.model.LocalTypeInformation import net.corda.serialization.internal.model.LocalTypeInformation
import java.io.NotSerializableException import java.io.NotSerializableException
@ -26,7 +25,7 @@ class ThrowableSerializer(factory: LocalSerializerFactory) : CustomSerializer.Pr
is LocalTypeInformation.NonComposable -> constructor ?: is LocalTypeInformation.NonComposable -> constructor ?:
throw NotSerializableException("$this has no deserialization constructor") throw NotSerializableException("$this has no deserialization constructor")
is LocalTypeInformation.Composable -> constructor is LocalTypeInformation.Composable -> constructor
is LocalTypeInformation.Opaque -> expand.constructor is LocalTypeInformation.Opaque -> wrapped.constructor
else -> throw NotSerializableException("$this has no deserialization constructor") else -> throw NotSerializableException("$this has no deserialization constructor")
} }

View File

@ -91,7 +91,7 @@ sealed class LocalTypeInformation {
is LocalTypeInformation.Abstract -> properties is LocalTypeInformation.Abstract -> properties
is LocalTypeInformation.AnInterface -> properties is LocalTypeInformation.AnInterface -> properties
is LocalTypeInformation.NonComposable -> properties is LocalTypeInformation.NonComposable -> properties
is LocalTypeInformation.Opaque -> expand.propertiesOrEmptyMap is LocalTypeInformation.Opaque -> wrapped.propertiesOrEmptyMap
else -> emptyMap() 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. * 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, data class Opaque(override val observedType: Class<*>, override val typeIdentifier: TypeIdentifier,
private val _expand: () -> LocalTypeInformation) : LocalTypeInformation() { val wrapped: 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)"
}
/** /**
* Represents a scalar type such as [Int]. * Represents a scalar type such as [Int].

View File

@ -94,12 +94,11 @@ internal data class LocalTypeInformationBuilder(val lookup: LocalTypeLookup,
buildInterfaceInformation(type)) buildInterfaceInformation(type))
type.isInterface -> buildInterface(type, typeIdentifier, emptyList()) type.isInterface -> buildInterface(type, typeIdentifier, emptyList())
type.isAbstractClass -> buildAbstract(type, typeIdentifier, emptyList()) type.isAbstractClass -> buildAbstract(type, typeIdentifier, emptyList())
else -> when { isOpaque -> LocalTypeInformation.Opaque(
isOpaque -> LocalTypeInformation.Opaque(type, typeIdentifier) { type,
buildNonAtomic(type, type, typeIdentifier, emptyList()) typeIdentifier,
} buildNonAtomic(type, type, typeIdentifier, emptyList(), true))
else -> buildNonAtomic(type, type, typeIdentifier, emptyList()) 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.isInterface -> buildInterface(type, typeIdentifier, buildTypeParameterInformation(type))
rawType.isAbstractClass -> buildAbstract(type, typeIdentifier, buildTypeParameterInformation(type)) rawType.isAbstractClass -> buildAbstract(type, typeIdentifier, buildTypeParameterInformation(type))
else -> when { isOpaque -> LocalTypeInformation.Opaque(rawType,
isOpaque -> LocalTypeInformation.Opaque(rawType, typeIdentifier) { typeIdentifier,
buildNonAtomic(rawType, type, typeIdentifier, buildTypeParameterInformation(type)) buildNonAtomic(rawType, type, typeIdentifier, buildTypeParameterInformation(type), true))
} else -> buildNonAtomic(rawType, type, typeIdentifier, buildTypeParameterInformation(type))
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 * 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. * 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>): LocalTypeInformation { private fun buildNonAtomic(rawType: Class<*>, type: Type, typeIdentifier: TypeIdentifier, typeParameterInformation: List<LocalTypeInformation>, suppressWarning: Boolean = false): LocalTypeInformation {
val superclassInformation = buildSuperclassInformation(type) val superclassInformation = buildSuperclassInformation(type)
val interfaceInformation = buildInterfaceInformation(type) val interfaceInformation = buildInterfaceInformation(type)
val observedConstructor = constructorForDeserialization(type) val observedConstructor = constructorForDeserialization(type)
if (observedConstructor == null) { 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), return LocalTypeInformation.NonComposable(type, typeIdentifier, null, buildReadOnlyProperties(rawType),
superclassInformation, interfaceInformation, typeParameterInformation) superclassInformation, interfaceInformation, typeParameterInformation)
} }
@ -176,10 +175,12 @@ internal data class LocalTypeInformationBuilder(val lookup: LocalTypeLookup,
val hasNonComposableProperties = properties.values.any { it.type is LocalTypeInformation.NonComposable } val hasNonComposableProperties = properties.values.any { it.type is LocalTypeInformation.NonComposable }
if (!propertiesSatisfyConstructor(constructorInformation, properties) || hasNonComposableProperties) { if (!propertiesSatisfyConstructor(constructorInformation, properties) || hasNonComposableProperties) {
if (hasNonComposableProperties) { if (!suppressWarning) {
logger.warn("Type ${type.typeName} has non-composable properties and has been marked as non-composable") if (hasNonComposableProperties) {
} else { logger.info("Type ${type.typeName} has non-composable properties and has been marked as non-composable")
logger.warn("Properties of type ${type.typeName} do not satisfy its constructor, type 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, return LocalTypeInformation.NonComposable(type, typeIdentifier, constructorInformation, properties, superclassInformation,
interfaceInformation, typeParameterInformation) interfaceInformation, typeParameterInformation)

View File

@ -90,7 +90,9 @@ sealed class TypeIdentifier {
(type.rawType as Class<*>).name, (type.rawType as Class<*>).name,
type.ownerType?.let { forGenericType(it) }, type.ownerType?.let { forGenericType(it) },
type.actualTypeArguments.map { type.actualTypeArguments.map {
forGenericType(it.resolveAgainst(resolutionContext)) val resolved = it.resolveAgainst(resolutionContext)
// Avoid cycles, e.g. Enum<E> where E resolves to Enum<E>
if (resolved == type) UnknownType else forGenericType(resolved)
}) })
is Class<*> -> forClass(type) is Class<*> -> forClass(type)
is GenericArrayType -> ArrayOf(forGenericType(type.genericComponentType.resolveAgainst(resolutionContext))) is GenericArrayType -> ArrayOf(forGenericType(type.genericComponentType.resolveAgainst(resolutionContext)))