From 0ca1500422f043394f31f6ed1f41ad724af3ce8a Mon Sep 17 00:00:00 2001 From: Katelyn Baker Date: Tue, 25 Jul 2017 10:32:59 +0100 Subject: [PATCH] Review Comments --- .../serialization/amqp/ArraySerializer.kt | 31 +++++------ .../serialization/amqp/SerializerFactory.kt | 3 ++ .../amqp/DeserializeSimpleTypesTests.kt | 53 ++++++++++++------- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/core/src/main/kotlin/net/corda/core/serialization/amqp/ArraySerializer.kt b/core/src/main/kotlin/net/corda/core/serialization/amqp/ArraySerializer.kt index 7f11aedacf..3819ae3812 100644 --- a/core/src/main/kotlin/net/corda/core/serialization/amqp/ArraySerializer.kt +++ b/core/src/main/kotlin/net/corda/core/serialization/amqp/ArraySerializer.kt @@ -10,8 +10,8 @@ import java.lang.reflect.Type open class ArraySerializer(override val type: Type, factory: SerializerFactory) : AMQPSerializer { companion object { fun make(type: Type, factory: SerializerFactory) = when (type) { - Array::class.java -> CharArraySerializer (factory) - else -> ArraySerializer(type, factory) + Array::class.java -> CharArraySerializer(factory) + else -> ArraySerializer(type, factory) } } @@ -55,10 +55,8 @@ open class ArraySerializer(override val type: Type, factory: SerializerFactory) } } -/** - * Boxed Character arrays required a specialisation to handle the type conversion properly when populating - * the array since Kotlin won't allow an implicit cast from Int (as they're stored as 16bit ints) to Char - */ +// Boxed Character arrays required a specialisation to handle the type conversion properly when populating +// the array since Kotlin won't allow an implicit cast from Int (as they're stored as 16bit ints) to Char class CharArraySerializer(factory: SerializerFactory) : ArraySerializer(Array::class.java, factory) { override fun List.toArrayOfType(type: Type): Any { val elementType = type.asClass() ?: throw NotSerializableException("Unexpected array element type $type") @@ -69,10 +67,8 @@ class CharArraySerializer(factory: SerializerFactory) : ArraySerializer(Array PrimShortArraySerializer(f, "short[p]") }, DoubleArray::class.java to { f -> PrimDoubleArraySerializer(f, "double[p]") }, LongArray::class.java to { f -> PrimLongArraySerializer(f, "long[p]") } + // ByteArray::class.java <-> NOT NEEDED HERE (see comment above) ) fun make(type: Type, factory: SerializerFactory) = primTypes[type]!!(factory) } - fun localWriteObject(data: Data, func : () -> Unit) { + fun localWriteObject(data: Data, func: () -> Unit) { data.withDescribed(typeNotation.descriptor) { withList { func() } } } } -class PrimIntArraySerializer(factory: SerializerFactory, override val typeName : String) : +class PrimIntArraySerializer(factory: SerializerFactory, override val typeName: String) : PrimArraySerializer(IntArray::class.java, factory) { override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput) { - localWriteObject(data) { (obj as IntArray).forEach { output.writeObjectOrNull(it, data, elementType) }} + localWriteObject(data) { (obj as IntArray).forEach { output.writeObjectOrNull(it, data, elementType) } } } } -class PrimCharArraySerializer(factory: SerializerFactory, override val typeName : String) : +class PrimCharArraySerializer(factory: SerializerFactory, override val typeName: String) : PrimArraySerializer(CharArray::class.java, factory) { override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput) { - localWriteObject(data) { (obj as CharArray).forEach { output.writeObjectOrNull(it, data, elementType) }} + localWriteObject(data) { (obj as CharArray).forEach { output.writeObjectOrNull(it, data, elementType) } } } override fun List.toArrayOfType(type: Type): Any { @@ -117,14 +114,14 @@ class PrimCharArraySerializer(factory: SerializerFactory, override val typeName } } -class PrimBooleanArraySerializer(factory: SerializerFactory, override val typeName : String) : +class PrimBooleanArraySerializer(factory: SerializerFactory, override val typeName: String) : PrimArraySerializer(BooleanArray::class.java, factory) { override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput) { localWriteObject(data) { (obj as BooleanArray).forEach { output.writeObjectOrNull(it, data, elementType) } } } } -class PrimDoubleArraySerializer(factory: SerializerFactory, override val typeName : String) : +class PrimDoubleArraySerializer(factory: SerializerFactory, override val typeName: String) : PrimArraySerializer(DoubleArray::class.java, factory) { override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput) { localWriteObject(data) { (obj as DoubleArray).forEach { output.writeObjectOrNull(it, data, elementType) } } diff --git a/core/src/main/kotlin/net/corda/core/serialization/amqp/SerializerFactory.kt b/core/src/main/kotlin/net/corda/core/serialization/amqp/SerializerFactory.kt index 4c85faeabd..97b3fc26d1 100644 --- a/core/src/main/kotlin/net/corda/core/serialization/amqp/SerializerFactory.kt +++ b/core/src/main/kotlin/net/corda/core/serialization/amqp/SerializerFactory.kt @@ -315,6 +315,9 @@ class SerializerFactory(val whitelist: ClassWhitelist = AllWhitelist) { throw NotSerializableException("Not able to deserialize array type: $name") } } else if (name.endsWith("[p]")) { + // There is no need to handle the ByteArray case as that type is coercible automatically + // to the binary type and is thus handled by the main steriliser and doesn't need a + // special case for a primitive array of bytes when(name) { "int[p]" -> IntArray::class.java "char[p]" -> CharArray::class.java diff --git a/core/src/test/kotlin/net/corda/core/serialization/amqp/DeserializeSimpleTypesTests.kt b/core/src/test/kotlin/net/corda/core/serialization/amqp/DeserializeSimpleTypesTests.kt index f90e970610..41ab0ad745 100644 --- a/core/src/test/kotlin/net/corda/core/serialization/amqp/DeserializeSimpleTypesTests.kt +++ b/core/src/test/kotlin/net/corda/core/serialization/amqp/DeserializeSimpleTypesTests.kt @@ -4,19 +4,17 @@ import org.junit.Test import kotlin.test.assertEquals import org.apache.qpid.proton.codec.Data -/** - * Prior to certain fixes being made within the [PropertySerializaer] classes these simple - * deserialization operations would've blown up with type mismatch errors where the deserlized - * char property of the class would've been treated as an Integer and given to the constructor - * as such - */ +// Prior to certain fixes being made within the [PropertySerializaer] classes these simple +// deserialization operations would've blown up with type mismatch errors where the deserlized +// char property of the class would've been treated as an Integer and given to the constructor +// as such class DeserializeSimpleTypesTests { class TestSerializationOutput( private val verbose: Boolean, serializerFactory: SerializerFactory = SerializerFactory()) : SerializationOutput(serializerFactory) { override fun writeSchema(schema: Schema, data: Data) { - if(verbose) println(schema) + if (verbose) println(schema) super.writeSchema(schema, data) } } @@ -33,6 +31,7 @@ class DeserializeSimpleTypesTests { @Test fun testChar() { data class C(val c: Char) + val c = C('c') val serialisedC = SerializationOutput().serialize(c) val deserializedC = DeserializationInput().deserialize(serialisedC) @@ -43,7 +42,8 @@ class DeserializeSimpleTypesTests { @Test fun testCharacter() { data class C(val c: Character) - val c = C(Character ('c')) + + val c = C(Character('c')) val serialisedC = SerializationOutput().serialize(c) val deserializedC = DeserializationInput().deserialize(serialisedC) @@ -53,6 +53,7 @@ class DeserializeSimpleTypesTests { @Test fun testArrayOfInt() { class IA(val ia: Array) + val ia = IA(arrayOf(1, 2, 3)) assertEquals("class [Ljava.lang.Integer;", ia.ia::class.java.toString()) @@ -69,7 +70,8 @@ class DeserializeSimpleTypesTests { @Test fun testArrayOfInteger() { - class IA (val ia: Array) + class IA(val ia: Array) + val ia = IA(arrayOf(Integer(1), Integer(2), Integer(3))) assertEquals("class [Ljava.lang.Integer;", ia.ia::class.java.toString()) @@ -89,7 +91,8 @@ class DeserializeSimpleTypesTests { */ @Test fun testIntArray() { - class IA (val ia: IntArray) + class IA(val ia: IntArray) + val v = IntArray(3) v[0] = 1; v[1] = 2; v[2] = 3 val ia = IA(v) @@ -108,8 +111,9 @@ class DeserializeSimpleTypesTests { @Test fun testArrayOfChars() { - class C (val c: Array) - val c = C(arrayOf ('a', 'b', 'c')) + class C(val c: Array) + + val c = C(arrayOf('a', 'b', 'c')) assertEquals("class [Ljava.lang.Character;", c.c::class.java.toString()) assertEquals(SerializerFactory.nameForType(c.c::class.java), "char[]") @@ -126,6 +130,7 @@ class DeserializeSimpleTypesTests { @Test fun testCharArray() { class C(val c: CharArray) + val v = CharArray(3) v[0] = 'a'; v[1] = 'b'; v[2] = 'c' val c = C(v) @@ -144,8 +149,9 @@ class DeserializeSimpleTypesTests { @Test fun testArrayOfBoolean() { - class C (val c: Array) - val c = C(arrayOf (true, false, false, true)) + class C(val c: Array) + + val c = C(arrayOf(true, false, false, true)) assertEquals("class [Ljava.lang.Boolean;", c.c::class.java.toString()) assertEquals(SerializerFactory.nameForType(c.c::class.java), "boolean[]") @@ -163,6 +169,7 @@ class DeserializeSimpleTypesTests { @Test fun testBooleanArray() { class C(val c: BooleanArray) + val c = C(BooleanArray(4)) c.c[0] = true; c.c[1] = false; c.c[2] = false; c.c[3] = true @@ -181,8 +188,9 @@ class DeserializeSimpleTypesTests { @Test fun testArrayOfByte() { - class C (val c: Array) - val c = C(arrayOf (0b0001, 0b0101, 0b1111)) + class C(val c: Array) + + val c = C(arrayOf(0b0001, 0b0101, 0b1111)) assertEquals("class [Ljava.lang.Byte;", c.c::class.java.toString()) assertEquals(SerializerFactory.nameForType(c.c::class.java), "byte[]") @@ -199,6 +207,7 @@ class DeserializeSimpleTypesTests { @Test fun testByteArray() { class C(val c: ByteArray) + val c = C(ByteArray(3)) c.c[0] = 0b0001; c.c[1] = 0b0101; c.c[2] = 0b1111 @@ -217,7 +226,8 @@ class DeserializeSimpleTypesTests { @Test fun testArrayOfShort() { class C(val c: Array) - val c = C(arrayOf (1, 2, 3)) + + val c = C(arrayOf(1, 2, 3)) assertEquals("class [Ljava.lang.Short;", c.c::class.java.toString()) assertEquals(SerializerFactory.nameForType(c.c::class.java), "short[]") @@ -234,6 +244,7 @@ class DeserializeSimpleTypesTests { @Test fun testShortArray() { class C(val c: ShortArray) + val c = C(ShortArray(3)) c.c[0] = 1; c.c[1] = 2; c.c[2] = 5 @@ -252,7 +263,8 @@ class DeserializeSimpleTypesTests { @Test fun testArrayOfLong() { class C(val c: Array) - val c = C(arrayOf (2147483650, -2147483800, 10)) + + val c = C(arrayOf(2147483650, -2147483800, 10)) assertEquals("class [Ljava.lang.Long;", c.c::class.java.toString()) assertEquals(SerializerFactory.nameForType(c.c::class.java), "long[]") @@ -269,6 +281,7 @@ class DeserializeSimpleTypesTests { @Test fun testLongArray() { class C(val c: LongArray) + val c = C(LongArray(3)) c.c[0] = 2147483650; c.c[1] = -2147483800; c.c[2] = 10 @@ -287,6 +300,7 @@ class DeserializeSimpleTypesTests { @Test fun testArrayOfFloat() { class C(val c: Array) + val c = C(arrayOf(10F, 100.023232F, -1455.433400F)) assertEquals("class [Ljava.lang.Float;", c.c::class.java.toString()) @@ -304,6 +318,7 @@ class DeserializeSimpleTypesTests { @Test fun testFloatArray() { class C(val c: FloatArray) + val c = C(FloatArray(3)) c.c[0] = 10F; c.c[1] = 100.023232F; c.c[2] = -1455.433400F @@ -322,6 +337,7 @@ class DeserializeSimpleTypesTests { @Test fun testArrayOfDouble() { class C(val c: Array) + val c = C(arrayOf(10.0, 100.2, -1455.2)) assertEquals("class [Ljava.lang.Double;", c.c::class.java.toString()) @@ -339,6 +355,7 @@ class DeserializeSimpleTypesTests { @Test fun testDoubleArray() { class C(val c: DoubleArray) + val c = C(DoubleArray(3)) c.c[0] = 10.0; c.c[1] = 100.2; c.c[2] = -1455.2