ExternalVendorCode/corda
1
0
Fork 0
mirror of https://github.com/corda/corda.git synced 2025-02-21 01:42:24 +00:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

CORDA-978 - Only consider getters that accpet zero parameters (#2462)

Browse Source 
* CORDA-978 - Only take getters with zero parameters

* tidy up
This commit is contained in:
Katelyn Baker 2018-02-05 09:48:41 +00:00 committed by GitHub
parent 8e2524f35d
commit a08d333d5b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 64 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt
View File

@ -127,7 +127,7 @@ fun Class<out Any?>.propertyDescriptors(): Map<String, PropertyDescriptor> {
clazz!!.declaredFields.forEach { classProperties.put(it.name, PropertyDescriptor(it, null, null)) }
// then pair them up with the declared getter and setter
// Note: It is possible for a class to have multipleinstancess of a function where the types
// Note: It is possible for a class to have multiple instancess of a function where the types
// differ. For example:
// interface I<out T> { val a: T }
// class D(override val a: String) : I<String>
@ -135,28 +135,37 @@ fun Class<out Any?>.propertyDescriptors(): Map<String, PropertyDescriptor> {
// getA - returning a String (java.lang.String) and
// getA - returning an Object (java.lang.Object)
// In this instance we take the most derived object
clazz.declaredMethods?.map {
PropertyDescriptorsRegex.re.find(it.name)?.apply {
//
// In addition, only getters that take zero parameters and setters that take a single
// parameter will be considered
clazz.declaredMethods?.map { func ->
PropertyDescriptorsRegex.re.find(func.name)?.apply {
try {
classProperties.getValue(groups[2]!!.value.decapitalize()).apply {
when (groups[1]!!.value) {
"set" -> {
if (setter == null) setter = it
else if (TypeToken.of(setter!!.genericReturnType).isSupertypeOf(it.genericReturnType)) {
setter = it
if (func.parameterCount == 1) {
if (setter == null) setter = func
else if (TypeToken.of(setter!!.genericReturnType).isSupertypeOf(func.genericReturnType)) {
setter = func
}
}
}
"get" -> {
if (getter == null) getter = it
else if (TypeToken.of(getter!!.genericReturnType).isSupertypeOf(it.genericReturnType)) {
getter = it
if (func.parameterCount == 0) {
if (getter == null) getter = func
else if (TypeToken.of(getter!!.genericReturnType).isSupertypeOf(func.genericReturnType)) {
getter = func
}
}
}
"is" -> {
val rtnType = TypeToken.of(it.genericReturnType)
if ((rtnType == TypeToken.of(Boolean::class.java))
|| (rtnType == TypeToken.of(Boolean::class.javaObjectType))) {
if (getter == null) getter = it
if (func.parameterCount == 0) {
val rtnType = TypeToken.of(func.genericReturnType)
if ((rtnType == TypeToken.of(Boolean::class.java))
|| (rtnType == TypeToken.of(Boolean::class.javaObjectType))) {
if (getter == null) getter = func
}
}
}
}

9
node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/PrivatePropertyTests.kt
View File

@ -86,7 +86,7 @@ class PrivatePropertyTests {
@Test
fun testMultiArgSetter() {
@Suppress("UNUSED")
data class C(private var a: Int, val b: Int) {
data class C(private var a: Int, var b: Int) {
// This will force the serialization engine to use getter / setter
// instantiation for the object rather than construction
@ConstructorForDeserialization
@ -97,10 +97,9 @@ class PrivatePropertyTests {
}
val c1 = C(33, 44)
Assertions.assertThatThrownBy {
SerializationOutput(factory).serialize(c1)
}.isInstanceOf(NotSerializableException::class.java).hasMessageContaining(
"Defined setter for parameter a takes too many arguments")
val c2 = DeserializationInput(factory).deserialize(SerializationOutput(factory).serialize(c1))
assertEquals(0, c2.getA())
assertEquals(44, c2.b)
}
@Test

38
node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutputTests.kt
View File

@ -20,6 +20,8 @@ import net.corda.nodeapi.internal.serialization.AllWhitelist
import net.corda.nodeapi.internal.serialization.EmptyWhitelist
import net.corda.nodeapi.internal.serialization.GeneratedAttachment
import net.corda.nodeapi.internal.serialization.amqp.SerializerFactory.Companion.isPrimitive
import net.corda.nodeapi.internal.serialization.amqp.custom.BigDecimalSerializer
import net.corda.nodeapi.internal.serialization.amqp.custom.CurrencySerializer
import net.corda.testing.contracts.DummyContract
import net.corda.testing.core.BOB_NAME
import net.corda.testing.core.SerializationEnvironmentRule
@ -36,11 +38,13 @@ import org.junit.Test
import java.io.ByteArrayInputStream
import java.io.IOException
import java.io.NotSerializableException
import java.lang.reflect.Type
import java.math.BigDecimal
import java.nio.ByteBuffer
import java.time.*
import java.time.temporal.ChronoUnit
import java.util.*
import java.util.concurrent.ConcurrentHashMap
import kotlin.reflect.full.superclasses
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
@ -1080,4 +1084,38 @@ class SerializationOutputTests {
serdes(obj, factory, factory2, expectedEqual = false, expectDeserializedEqual = false)
}.isInstanceOf(MissingAttachmentsException::class.java)
}
//
// Example stacktrace that this test is tryint to reproduce
//
// java.lang.IllegalArgumentException:
// net.corda.core.contracts.TransactionState ->
// data(net.corda.core.contracts.ContractState) ->
// net.corda.finance.contracts.asset.Cash$State ->
// amount(net.corda.core.contracts.Amount<net.corda.core.contracts.Issued<java.util.Currency>>) ->
// net.corda.core.contracts.Amount<net.corda.core.contracts.Issued<java.util.Currency>> ->
// displayTokenSize(java.math.BigDecimal) ->
// wrong number of arguments
//
// So the actual problem was objects with multiple getters. The code wasn't looking for one with zero
// properties, just taking the first one it found with with the most applicable type, and the reflection
// ordering of the methods was random, thus occasionally we select the wrong one
//
@Test
fun reproduceWrongNumberOfArguments() {
val field = SerializerFactory::class.java.getDeclaredField("serializersByType").apply {
this.isAccessible = true
}
data class C(val a: Amount<Currency>)
val factory = testDefaultFactoryNoEvolution()
factory.register(net.corda.nodeapi.internal.serialization.amqp.custom.BigDecimalSerializer)
factory.register(net.corda.nodeapi.internal.serialization.amqp.custom.CurrencySerializer)
val c = C(Amount<Currency>(100, BigDecimal("1.5"), Currency.getInstance("USD")))
// were the issue not fixed we'd blow up here
SerializationOutput(factory).serialize(c)
}
}