Merge remote-tracking branch 'open/master' into mike-merge-80c00b920b

This commit is contained in:
Mike Hearn
2018-03-02 15:17:32 +01:00
13 changed files with 223 additions and 71 deletions

View File

@ -153,7 +153,7 @@ class SerializerFingerPrinter : FingerPrinter {
}.putUnencodedChars(type.name).putUnencodedChars(ENUM_HASH)
} else {
hasher.fingerprintWithCustomSerializerOrElse(factory!!, type, type) {
if (type.kotlin.objectInstance != null) {
if (type.objectInstance() != 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)

View File

@ -136,7 +136,16 @@ fun Class<out Any?>.propertyDescriptors(): Map<String, PropertyDescriptor> {
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<out T> { val a: T }
@ -148,14 +157,23 @@ fun Class<out Any?>.propertyDescriptors(): Map<String, PropertyDescriptor> {
//
// In addition, only getters that take zero parameters and setters that take a single
// parameter will be considered
clazz.declaredMethods?.map { func ->
clazz!!.declaredMethods?.map { func ->
if (!Modifier.isPublic(func.modifiers)) return@map
if (func.name == "getClass") return@map
PropertyDescriptorsRegex.re.find(func.name)?.apply {
// 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.decapitalize()) { PropertyDescriptor() }.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 to a cosntructor
// 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) {
@ -221,38 +239,38 @@ internal fun <T : Any> propertiesForSerializationFromConstructor(
// it so just ignore it as it'll be supplied at runtime anyway on invocation
val name = param.value.name ?: return@forEach
val propertyReader = if (name in classProperties) {
if (classProperties[name]!!.getter != null) {
// it's a publicly accessible property
val matchingProperty = classProperties[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 NotSerializableException(
"Constructor parameter - \"$name\" - doesn't refer to a property of \"$clazz\"")
// Check that the method has a getter in java.
val getter = matchingProperty.getter ?: throw NotSerializableException(
"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.")
// 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 NotSerializableException(
"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 NotSerializableException(
"Property - \"$name\" - has type \"$returnType\" on \"$clazz\" but differs from constructor " +
"parameter type \"${param.value.type.javaType}\"")
}
val returnType = resolveTypeVariables(getter.genericReturnType, type)
if (!constructorParamTakesReturnTypeOfGetter(returnType, getter.genericReturnType, param.value)) {
throw NotSerializableException(
"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 ?:
Pair(PublicPropertyReader(getter), returnType)
} else {
val field = classProperties[name]!!.field ?:
throw NotSerializableException("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), field.genericType)
}
} else {
throw NotSerializableException(
"Constructor parameter - \"$name\" - doesn't refer to a property of \"$clazz\"")
Pair(PrivatePropertyReader(field, type), field.genericType)
}
this += PropertyAccessorConstructor(
@ -513,3 +531,48 @@ fun ClassWhitelist.hasAnnotationInHierarchy(type: Class<*>): Boolean {
|| type.interfaces.any { hasAnnotationInHierarchy(it) }
|| (type.superclass != null && hasAnnotationInHierarchy(type.superclass))
}
/**
* By default use Kotlin reflection and grab the objectInstance. However, that doesn't play nicely with nested
* private objects. Even setting the accessibility override (setAccessible) still causes an
* [IllegalAccessException] when attempting to retrieve the value of the INSTANCE field.
*
* Whichever reference to the class Kotlin reflection uses, override (set from setAccessible) on that field
* isn't set even when it was explicitly set as accessible before calling into the kotlin reflection routines.
*
* For example
*
* clazz.getDeclaredField("INSTANCE")?.apply {
* isAccessible = true
* kotlin.objectInstance // This throws as the INSTANCE field isn't accessible
* }
*
* Therefore default back to good old java reflection and simply look for the INSTANCE field as we are never going
* to serialize a companion object.
*
* As such, if objectInstance fails access, revert to Java reflection and try that
*/
fun Class<*>.objectInstance() =
try {
this.kotlin.objectInstance
} catch (e: IllegalAccessException) {
// Check it really is an object (i.e. it has no constructor)
if (constructors.isNotEmpty()) null
else {
try {
this.getDeclaredField("INSTANCE")?.let { field ->
// and must be marked as both static and final (>0 means they're set)
if (modifiers and Modifier.STATIC == 0 || modifiers and Modifier.FINAL == 0) null
else {
val accessibility = field.isAccessible
field.isAccessible = true
val obj = field.get(null)
field.isAccessible = accessibility
obj
}
}
} catch (e: NoSuchFieldException) {
null
}
}
}

View File

@ -267,12 +267,15 @@ open class SerializerFactory(
// Don't need to check the whitelist since each element will come back through the whitelisting process.
if (clazz.componentType.isPrimitive) PrimArraySerializer.make(type, this)
else ArraySerializer.make(type, this)
} else if (clazz.kotlin.objectInstance != null) {
whitelist.requireWhitelisted(clazz)
SingletonSerializer(clazz, clazz.kotlin.objectInstance!!, this)
} else {
whitelist.requireWhitelisted(type)
ObjectSerializer(type, this)
val singleton = clazz.objectInstance()
if (singleton != null) {
whitelist.requireWhitelisted(clazz)
SingletonSerializer(clazz, singleton, this)
} else {
whitelist.requireWhitelisted(type)
ObjectSerializer(type, this)
}
}
}
}

View File

@ -196,4 +196,32 @@ class PrivatePropertyTests {
assertEquals(c1, c2)
}
//
// Reproduces CORDA-1134
//
@Suppress("UNCHECKED_CAST")
@Test
fun allCapsProprtyNotPrivate() {
data class C (val CCC: String)
val output = SerializationOutput(factory).serializeAndReturnSchema(C("this is nice"))
val serializersByDescriptor = fields["serializersByDesc"]!!.get(factory) as ConcurrentHashMap<Any, AMQPSerializer<Any>>
val schemaDescriptor = output.schema.types.first().descriptor.name
serializersByDescriptor.filterKeys { (it as Symbol) == schemaDescriptor }.values.apply {
assertEquals(1, size)
assertTrue(this.first() is ObjectSerializer)
val propertySerializers = (this.first() as ObjectSerializer).propertySerializers.serializationOrder.map { it.getter }
// CCC is the only property to be serialised
assertEquals(1, propertySerializers.size)
// and despite being all caps it should still be a public getter
assertTrue(propertySerializers[0].propertyReader is PublicPropertyReader)
}
}
}

View File

@ -47,6 +47,22 @@ import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertTrue
object AckWrapper {
object Ack
fun serialize() {
val factory = testDefaultFactoryNoEvolution()
SerializationOutput(factory).serialize(Ack)
}
}
object PrivateAckWrapper {
private object Ack
fun serialize() {
val factory = testDefaultFactoryNoEvolution()
SerializationOutput(factory).serialize(Ack)
}
}
@RunWith(Parameterized::class)
class SerializationOutputTests(private val compression: CordaSerializationEncoding?) {
private companion object {
@ -1148,4 +1164,18 @@ class SerializationOutputTests(private val compression: CordaSerializationEncodi
assertEquals(encodingNotPermittedFormat.format(compression), message)
}
}
}
@Test
fun nestedObjects() {
// The "test" is that this doesn't throw, anything else is a success
AckWrapper.serialize()
}
@Test
fun privateNestedObjects() {
// The "test" is that this doesn't throw, anything else is a success
PrivateAckWrapper.serialize()
}
}