mirror of
https://github.com/corda/corda.git
synced 2024-12-22 14:22:28 +00:00
CORDA-1134 - Don't use private serializers for all CAPS properties (#2708)
Whilst it does currently work it only does so by going down an incorrect code path. A property THING that is public and has a getter should be fetched using that getter, not trip into the private property accessor.
This commit is contained in:
Katelyn Baker
GitHub
parent
ba925c0277
commit
9e9fab7e1a
@ -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(
|
||||
|
||||
@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user