Review comments

* Rename some functions to more descriptive names
 * Remove some egregious whitespace
 * Revert some changes that were a dangling holdover from an aborted
   refactoring - put it back how it was
This commit is contained in:
Katelyn Baker 2017-07-14 15:16:42 +01:00
parent f19ec4e3ba
commit 0bfe1de3ba
7 changed files with 25 additions and 48 deletions

View File

@ -19,7 +19,7 @@ fun AMQPSchema.carpenterSchema(
/**
* if we can load the class then we MUST know about all of it's composite elements
*/
private fun CompositeType.validateKnown (
private fun CompositeType.validatePropertyTypes(
classLoaders: List<ClassLoader> = listOf<ClassLoader> (ClassLoader.getSystemClassLoader())){
fields.forEach {
if (!it.validateType(classLoaders)) throw UncarpentableException (name, it.name, it.type)
@ -44,7 +44,7 @@ fun CompositeType.carpenterSchema(
carpenterSchemas : CarpenterSchemas,
force : Boolean = false) {
if (classLoaders.exists(name)) {
validateKnown(classLoaders)
validatePropertyTypes(classLoaders)
if (!force) return
}

View File

@ -201,8 +201,7 @@ class ClassCarpenter {
private fun ClassWriter.generateGetters(schema: Schema) {
for ((name, type) in schema.fields) {
val opcodes = ACC_PUBLIC
with(visitMethod(opcodes, "get" + name.capitalize(), "()" + type.descriptor, null, null)) {
with(visitMethod(ACC_PUBLIC, "get" + name.capitalize(), "()" + type.descriptor, null, null)) {
type.addNullabilityAnnotation(this)
visitCode()
visitVarInsn(ALOAD, 0) // Load 'this'

View File

@ -8,8 +8,6 @@ import org.objectweb.asm.MethodVisitor
import org.objectweb.asm.Type
import java.util.LinkedHashMap
/**********************************************************************************************************************/
/**
* A Schema represents a desired class.
*/
@ -36,8 +34,6 @@ abstract class Schema(
get() = name.replace(".", "/")
}
/**********************************************************************************************************************/
class ClassSchema(
name: String,
fields: Map<String, Field>,
@ -45,8 +41,6 @@ class ClassSchema(
interfaces: List<Class<*>> = emptyList()
) : Schema (name, fields, superclass, interfaces)
/**********************************************************************************************************************/
class InterfaceSchema(
name: String,
fields: Map<String, Field>,
@ -54,8 +48,6 @@ class InterfaceSchema(
interfaces: List<Class<*>> = emptyList()
) : Schema (name, fields, superclass, interfaces)
/**********************************************************************************************************************/
object CarpenterSchemaFactory {
fun newInstance (
name: String,
@ -68,8 +60,6 @@ object CarpenterSchemaFactory {
else ClassSchema (name, fields, superclass, interfaces)
}
/**********************************************************************************************************************/
abstract class Field(val field: Class<out Any?>) {
companion object {
const val unsetName = "Unset"
@ -107,8 +97,6 @@ abstract class Field(val field: Class<out Any?>) {
abstract fun nullTest(mv: MethodVisitor, slot: Int)
}
/**********************************************************************************************************************/
class NonNullableField(field: Class<out Any?>) : Field(field) {
override val nullabilityAnnotation = "Ljavax/annotation/Nonnull;"
@ -136,8 +124,6 @@ class NonNullableField(field: Class<out Any?>) : Field(field) {
}
}
/**********************************************************************************************************************/
class NullableField(field: Class<out Any?>) : Field(field) {
override val nullabilityAnnotation = "Ljavax/annotation/Nullable;"
@ -157,12 +143,8 @@ class NullableField(field: Class<out Any?>) : Field(field) {
}
}
/**********************************************************************************************************************/
object FieldFactory {
fun newInstance (mandatory: Boolean, name: String, field: Class<out Any?>) =
if (mandatory) NonNullableField (name, field) else NullableField (name, field)
}
/**********************************************************************************************************************/

View File

@ -483,9 +483,9 @@ class ClassCarpenterTest {
@Test
fun beanTest() {
val schema = ClassCarpenter.ClassSchema(
val schema = ClassSchema(
"pantsPantsPants",
mapOf("a" to ClassCarpenter.NonNullableField(Integer::class.java)))
mapOf("a" to NonNullableField(Integer::class.java)))
val clazz = cc.build(schema)
val descriptors = Introspector.getBeanInfo(clazz).propertyDescriptors

View File

@ -13,7 +13,7 @@ fun mangleName(name: String) = "${name}__carpenter"
* given a list of class names work through the amqp envelope schema and alter any that
* match in the fashion defined above
*/
fun Schema.mangleName(names: List<String>): Schema {
fun Schema.mangleNames(names: List<String>): Schema {
val newTypes: MutableList<TypeNotation> = mutableListOf()
for (type in types) {

View File

@ -89,7 +89,7 @@ class CompositeMembers : AmqpCarpenterBase() {
val b = B(A(testA), testB)
val obj = DeserializationInput(factory).deserializeAndReturnEnvelope(serialise(b))
val amqpSchema = obj.envelope.schema.mangleName(listOf (classTestName ("A")))
val amqpSchema = obj.envelope.schema.mangleNames(listOf (classTestName ("A")))
assert(obj.obj is B)
@ -112,7 +112,7 @@ class CompositeMembers : AmqpCarpenterBase() {
assert(obj.obj is B)
val amqpSchema = obj.envelope.schema.mangleName(listOf(classTestName("B")))
val amqpSchema = obj.envelope.schema.mangleNames(listOf(classTestName("B")))
val carpenterSchema = amqpSchema.carpenterSchema()
assertEquals(1, carpenterSchema.size)
@ -140,7 +140,7 @@ class CompositeMembers : AmqpCarpenterBase() {
assert(obj.obj is B)
val amqpSchema = obj.envelope.schema.mangleName(listOf(classTestName("A"), classTestName("B")))
val amqpSchema = obj.envelope.schema.mangleNames(listOf(classTestName("A"), classTestName("B")))
val carpenterSchema = amqpSchema.carpenterSchema()
// just verify we're in the expected initial state, A is carpentable, B is not because
@ -180,7 +180,7 @@ class CompositeMembers : AmqpCarpenterBase() {
@Test(expected = UncarpentableException::class)
@Suppress("UNUSED")
fun nestedIsUnkownInherited() {
fun nestedIsUnknownInherited() {
val testA = 10
val testB = 20
val testC = 30
@ -199,7 +199,7 @@ class CompositeMembers : AmqpCarpenterBase() {
assert(obj.obj is C)
val amqpSchema = obj.envelope.schema.mangleName(listOf(classTestName("A"), classTestName("B")))
val amqpSchema = obj.envelope.schema.mangleNames(listOf(classTestName("A"), classTestName("B")))
amqpSchema.carpenterSchema()
}
@ -225,7 +225,7 @@ class CompositeMembers : AmqpCarpenterBase() {
assert(obj.obj is C)
val amqpSchema = obj.envelope.schema.mangleName(listOf(classTestName("A"), classTestName("B")))
val amqpSchema = obj.envelope.schema.mangleNames(listOf(classTestName("A"), classTestName("B")))
amqpSchema.carpenterSchema()
}
@ -251,7 +251,7 @@ class CompositeMembers : AmqpCarpenterBase() {
assert(obj.obj is C)
val carpenterSchema = obj.envelope.schema.mangleName(listOf(classTestName("A"), classTestName("B")))
val carpenterSchema = obj.envelope.schema.mangleNames(listOf(classTestName("A"), classTestName("B")))
TestMetaCarpenter(carpenterSchema.carpenterSchema())
}
@ -273,13 +273,13 @@ class CompositeMembers : AmqpCarpenterBase() {
assert(obj.obj is B)
val carpenterSchema = obj.envelope.schema.mangleName(listOf(classTestName("A"), classTestName("B")))
val carpenterSchema = obj.envelope.schema.mangleNames(listOf(classTestName("A"), classTestName("B")))
val metaCarpenter = TestMetaCarpenter(carpenterSchema.carpenterSchema())
assertEquals(1, metaCarpenter.schemas.carpenterSchemas.size)
assertEquals(mangleName(classTestName("B")), metaCarpenter.schemas.carpenterSchemas.first().name)
assertEquals(mangleNames(classTestName("B")), metaCarpenter.schemas.carpenterSchemas.first().name)
assertEquals(1, metaCarpenter.schemas.dependencies.size)
assertTrue(mangleName(classTestName("A")) in metaCarpenter.schemas.dependencies)
assertTrue(mangleNames(classTestName("A")) in metaCarpenter.schemas.dependencies)
}
*/
}

View File

@ -1,7 +1,6 @@
package net.corda.core.serialization.carpenter
import net.corda.core.serialization.CordaSerializable
import net.corda.core.serialization.carpenter.MetaCarpenter
import net.corda.core.serialization.carpenter.test.*
import net.corda.core.serialization.amqp.*
@ -54,7 +53,7 @@ class InheritanceSchemaToClassCarpenterTests : AmqpCarpenterBase() {
// it's extremely unlikely we'd need to carpent any classes
assertEquals(0, l1.size)
val mangleSchema = serSchema.mangleName(listOf(classTestName("A")))
val mangleSchema = serSchema.mangleNames(listOf(classTestName("A")))
val l2 = mangleSchema.carpenterSchema()
assertEquals(1, l2.size)
@ -95,9 +94,8 @@ class InheritanceSchemaToClassCarpenterTests : AmqpCarpenterBase() {
assertEquals(0, l1.size)
val mangleSchema = serSchema.mangleName(listOf(classTestName("A")))
val mangleSchema = serSchema.mangleNames(listOf(classTestName("A")))
val aName = mangleName(classTestName("A"))
val l2 = mangleSchema.carpenterSchema()
assertEquals(1, l2.size)
@ -111,7 +109,6 @@ class InheritanceSchemaToClassCarpenterTests : AmqpCarpenterBase() {
assertEquals(net.corda.core.serialization.carpenter.J::class.java, aSchema.interfaces[0])
val aBuilder = ClassCarpenter().build(aSchema)
val objJ = aBuilder.constructors[0].newInstance(testJ, testJJ)
val j = objJ as J
@ -145,7 +142,7 @@ class InheritanceSchemaToClassCarpenterTests : AmqpCarpenterBase() {
// pretend we don't know the class we've been sent, i.e. it's unknown to the class loader, and thus
// needs some carpentry
val mangleSchema = serSchema.mangleName(listOf(classTestName("A")))
val mangleSchema = serSchema.mangleNames(listOf(classTestName("A")))
val l2 = mangleSchema.carpenterSchema()
val aName = mangleName(classTestName("A"))
@ -191,7 +188,7 @@ class InheritanceSchemaToClassCarpenterTests : AmqpCarpenterBase() {
// it's extremely unlikely we'd need to carpent any classes
assertEquals(0, l1.size)
val mangleSchema = serSchema.mangleName(listOf(classTestName("A")))
val mangleSchema = serSchema.mangleNames(listOf(classTestName("A")))
val l2 = mangleSchema.carpenterSchema()
val aName = mangleName(classTestName("A"))
@ -239,7 +236,7 @@ class InheritanceSchemaToClassCarpenterTests : AmqpCarpenterBase() {
// * class B's interface (class IIII)
assertEquals(4, serSchema.types.size)
val mangleSchema = serSchema.mangleName(listOf(classTestName("A"), classTestName("B")))
val mangleSchema = serSchema.mangleNames(listOf(classTestName("A"), classTestName("B")))
val cSchema = mangleSchema.carpenterSchema()
val aName = mangleName(classTestName("A"))
val bName = mangleName(classTestName("B"))
@ -296,7 +293,7 @@ class InheritanceSchemaToClassCarpenterTests : AmqpCarpenterBase() {
assertEquals(4, serSchema.types.size)
// ignore the return as we expect this to throw
serSchema.mangleName(listOf(
serSchema.mangleNames(listOf(
classTestName("A"), "${this.javaClass.`package`.name}.I")).carpenterSchema()
}
@ -317,7 +314,7 @@ class InheritanceSchemaToClassCarpenterTests : AmqpCarpenterBase() {
// * class A's interface (class I)
assertEquals(2, serSchema.types.size)
val amqpSchema = serSchema.mangleName(listOf(classTestName("A"), "${this.javaClass.`package`.name}.I"))
val amqpSchema = serSchema.mangleNames(listOf(classTestName("A"), "${this.javaClass.`package`.name}.I"))
val aName = mangleName(classTestName("A"))
val iName = mangleName("${this.javaClass.`package`.name}.I")
val carpenterSchema = amqpSchema.carpenterSchema()
@ -359,7 +356,7 @@ class InheritanceSchemaToClassCarpenterTests : AmqpCarpenterBase() {
val a = A(testI, testII)
val obj = DeserializationInput(factory).deserializeAndReturnEnvelope(serialise(a))
val amqpSchema = obj.envelope.schema.mangleName(listOf(
val amqpSchema = obj.envelope.schema.mangleNames(listOf(
classTestName("A"),
"${this.javaClass.`package`.name}.I",
"${this.javaClass.`package`.name}.II"))
@ -411,7 +408,7 @@ class InheritanceSchemaToClassCarpenterTests : AmqpCarpenterBase() {
val a = A(testI, testIII)
val obj = DeserializationInput(factory).deserializeAndReturnEnvelope(serialise(a))
val amqpSchema = obj.envelope.schema.mangleName(listOf(
val amqpSchema = obj.envelope.schema.mangleNames(listOf(
classTestName("A"),
"${this.javaClass.`package`.name}.I",
"${this.javaClass.`package`.name}.III"))
@ -419,7 +416,6 @@ class InheritanceSchemaToClassCarpenterTests : AmqpCarpenterBase() {
val aName = mangleName(classTestName("A"))
val iName = mangleName("${this.javaClass.`package`.name}.I")
val iiiName = mangleName("${this.javaClass.`package`.name}.III")
val carpenterSchema = amqpSchema.carpenterSchema()
// Since A depends on III and III extends I we will have to construct them