CORDA-1497 enum evolution (#4343)

* Get enum transforms during type info building

* Eliminate transforms cache

* Allow combined renaming and defaulting for enums

* Comments and moving build method to EnumTransforms

* Force validation on EnumTransforms creation

* Remove commented code

* Cope with multiple renames

serialization/src/main/kotlin/net/corda/serialization/internal/amqp/AMQPRemoteTypeModel.kt

@@ -1,8 +1,8 @@
 package net.corda.serialization.internal.amqp
 
+import net.corda.serialization.internal.NotSerializableDetailedException
 import net.corda.serialization.internal.model.*
 import java.io.NotSerializableException
-import java.util.*
 import kotlin.collections.LinkedHashMap
 
 /**
@@ -31,7 +31,7 @@ class AMQPRemoteTypeModel {
         val notationLookup = schema.types.associateBy { it.name.typeIdentifier }
         val byTypeDescriptor = schema.types.associateBy { it.typeDescriptor }
         val enumTransformsLookup = transforms.types.asSequence().map { (name, transformSet) ->
-            name.typeIdentifier to interpretTransformSet(transformSet)
+            name.typeIdentifier to transformSet
         }.toMap()
 
         val interpretationState = InterpretationState(notationLookup, enumTransformsLookup, cache, emptySet())
@@ -50,7 +50,7 @@ class AMQPRemoteTypeModel {
     }
 
     data class InterpretationState(val notationLookup: Map<TypeIdentifier, TypeNotation>,
-                                   val enumTransformsLookup: Map<TypeIdentifier, EnumTransforms>,
+                                   val enumTransformsLookup: Map<TypeIdentifier, TransformsMap>,
                                    val cache: MutableMap<TypeDescriptor, RemoteTypeInformation>,
                                    val seen: Set<TypeIdentifier>) {
 
@@ -131,14 +131,25 @@ class AMQPRemoteTypeModel {
                     RemoteTypeInformation.Unparameterised(
                             typeDescriptor,
                             identifier)
-                } else RemoteTypeInformation.AnEnum(
+                } else interpretEnum(identifier)
-                        typeDescriptor,
+
-                        identifier,
-                        choices.map { it.name },
-                        enumTransformsLookup[identifier] ?: EnumTransforms.empty)
             else -> throw NotSerializableException("Cannot interpret restricted type $this")
         }
 
+        private fun RestrictedType.interpretEnum(identifier: TypeIdentifier): RemoteTypeInformation.AnEnum {
+            val constants = choices.asSequence().mapIndexed { index, choice -> choice.name to index }.toMap(LinkedHashMap())
+            val transforms = try {
+                enumTransformsLookup[identifier]?.let { EnumTransforms.build(it, constants) } ?: EnumTransforms.empty
+            } catch (e: InvalidEnumTransformsException) {
+                throw NotSerializableDetailedException(name, e.message!!)
+            }
+            return RemoteTypeInformation.AnEnum(
+                    typeDescriptor,
+                    identifier,
+                    constants.keys.toList(),
+                    transforms)
+        }
+
         /**
          * Interpret a [Field] into a name/[RemotePropertyInformation] pair.
          */
@@ -181,20 +192,6 @@ class AMQPRemoteTypeModel {
     }
 }
 
-fun LocalTypeInformation.getEnumTransforms(factory: LocalSerializerFactory): EnumTransforms {
-    val transformsSchema = TransformsSchema.get(typeIdentifier.name, factory)
-    return interpretTransformSet(transformsSchema)
-}
-
-private fun interpretTransformSet(transformSet: EnumMap<TransformTypes, MutableList<Transform>>): EnumTransforms {
-    val defaultTransforms = transformSet[TransformTypes.EnumDefault]?.toList() ?: emptyList()
-    val defaults = defaultTransforms.associate { transform -> (transform as EnumDefaultSchemaTransform).new to transform.old }
-    val renameTransforms = transformSet[TransformTypes.Rename]?.toList() ?: emptyList()
-    val renames = renameTransforms.associate { transform -> (transform as RenameSchemaTransform).to to transform.from }
-
-    return EnumTransforms(defaults, renames)
-}
-
 private val TypeNotation.typeDescriptor: String get() = descriptor.name?.toString() ?:
 throw NotSerializableException("Type notation has no type descriptor: $this")
 

serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializerFactory.kt

@@ -132,7 +132,7 @@ class DefaultEvolutionSerializerFactory(
         if (members == localTypeInformation.members) return null
 
         val remoteTransforms = transforms
-        val localTransforms = localTypeInformation.getEnumTransforms(localSerializerFactory)
+        val localTransforms = localTypeInformation.transforms
         val transforms = if (remoteTransforms.size > localTransforms.size) remoteTransforms else localTransforms
 
         val localOrdinals = localTypeInformation.members.asSequence().mapIndexed { ord, member -> member to ord }.toMap()

serialization/src/main/kotlin/net/corda/serialization/internal/amqp/LocalSerializerFactory.kt

@@ -62,14 +62,6 @@ interface LocalSerializerFactory {
      * Use the [FingerPrinter] to create a type descriptor for the given [typeInformation].
      */
     fun createDescriptor(typeInformation: LocalTypeInformation): Symbol
-
-    /**
-     * Obtain or register [Transform]s for the given class [name].
-     *
-     * Eventually this information should be moved into the [LocalTypeInformation] for the type.
-     */
-    fun getOrBuildTransform(name: String, builder: () -> EnumMap<TransformTypes, MutableList<Transform>>):
-            EnumMap<TransformTypes, MutableList<Transform>>
 }
 
 /**
@@ -91,7 +83,6 @@ class DefaultLocalSerializerFactory(
         val logger = contextLogger()
     }
 
-    private val transformsCache: MutableMap<String, EnumMap<TransformTypes, MutableList<Transform>>> = DefaultCacheProvider.createCache()
     private val serializersByType: MutableMap<TypeIdentifier, AMQPSerializer<Any>> = DefaultCacheProvider.createCache()
 
     override fun createDescriptor(typeInformation: LocalTypeInformation): Symbol =
@@ -99,10 +90,6 @@ class DefaultLocalSerializerFactory(
 
     override fun getTypeInformation(type: Type): LocalTypeInformation = typeModel.inspect(type)
 
-    override fun getOrBuildTransform(name: String, builder: () -> EnumMap<TransformTypes, MutableList<Transform>>):
-            EnumMap<TransformTypes, MutableList<Transform>> =
-            transformsCache.computeIfAbsent(name) { _ -> builder() }
-
     override fun get(typeInformation: LocalTypeInformation): AMQPSerializer<Any> =
             get(typeInformation.observedType, typeInformation)
 

serialization/src/main/kotlin/net/corda/serialization/internal/amqp/TransformTypes.kt

@@ -1,7 +1,6 @@
 package net.corda.serialization.internal.amqp
 
 import net.corda.core.KeepForDJVM
-import net.corda.core.internal.uncheckedCast
 import net.corda.core.serialization.CordaSerializationTransformEnumDefault
 import net.corda.core.serialization.CordaSerializationTransformEnumDefaults
 import net.corda.core.serialization.CordaSerializationTransformRename
@@ -30,109 +29,24 @@ enum class TransformTypes(val build: (Annotation) -> Transform) : DescribedType
     Unknown({ UnknownTransform() }) {
         override fun getDescriptor(): Any = DESCRIPTOR
         override fun getDescribed(): Any = ordinal
-        override fun validate(list: List<Transform>, constants: Map<String, Int>) {}
     },
     EnumDefault({ a -> EnumDefaultSchemaTransform((a as CordaSerializationTransformEnumDefault).old, a.new) }) {
         override fun getDescriptor(): Any = DESCRIPTOR
         override fun getDescribed(): Any = ordinal
-
-        /**
-         * Validates a list of constant additions to an enumerated type. To be valid a default (the value
-         * that should be used when we cannot use the new value) must refer to a constant that exists in the
-         * enum class as it exists now and it cannot refer to itself.
-         *
-         * @param list The list of transforms representing new constants and the mapping from that constant to an
-         * existing value
-         * @param constants The list of enum constants on the type the transforms are being applied to
-         */
-        override fun validate(list: List<Transform>, constants: Map<String, Int>) {
-            uncheckedCast<List<Transform>, List<EnumDefaultSchemaTransform>>(list).forEach {
-                requireThat(constants.contains(it.new)) {"Unknown enum constant ${it.new}"}
-                requireThat(constants.contains(it.old)) { "Enum extension defaults must be to a valid constant: ${it.new} -> ${it.old}. ${it.old} " +
-                        "doesn't exist in constant set $constants" }
-                requireThat(it.old != it.new) { "Enum extension ${it.new} cannot default to itself" }
-                requireThat(constants[it.old]!! < constants[it.new]!!) { "Enum extensions must default to older constants. ${it.new}[${constants[it.new]}] " +
-                        "defaults to ${it.old}[${constants[it.old]}] which is greater" }
-            }
-        }
     },
     Rename({ a -> RenameSchemaTransform((a as CordaSerializationTransformRename).from, a.to) }) {
         override fun getDescriptor(): Any = DESCRIPTOR
         override fun getDescribed(): Any = ordinal
-
-        /**
-         * Validates a list of rename transforms is valid. Such a list isn't valid if we detect a cyclic chain,
-         * that is a constant is renamed to something that used to exist in the enum. We do this for both
-         * the same constant (i.e. C -> D -> C) and multiple constants (C->D, B->C)
-         *
-         * @param list The list of transforms representing the renamed constants and the mapping between their new
-         * and old values
-         * @param constants The list of enum constants on the type the transforms are being applied to
-         */
-        override fun validate(list: List<Transform>, constants: Map<String, Int>) {
-            @KeepForDJVM
-            data class Node(val transform: RenameSchemaTransform, var next: Node?, var prev: Node?, var visitedBy: Node? = null) {
-                fun visit(visitedBy: Node) {
-                    this.visitedBy = visitedBy
-                }
-                val visited get() = visitedBy != null
-            }
-
-            val graph = mutableListOf<Node>()
-            // Keep two maps of forward links and back links in order to build the graph in one pass
-            val forwardLinks = hashMapOf<String, Node>()
-            val reverseLinks = hashMapOf<String, Node>()
-
-            // build a dependency graph
-            val transforms: List<RenameSchemaTransform> = uncheckedCast(list)
-            transforms.forEach { rename ->
-                requireThat(!forwardLinks.contains(rename.from)) { "There are multiple transformations from ${rename.from}, which is not allowed" }
-                requireThat(!reverseLinks.contains(rename.to)) { "There are multiple transformations to ${rename.to}, which is not allowed" }
-                val node = Node(rename, forwardLinks[rename.to], reverseLinks[rename.from])
-                graph.add(node)
-                node.next?.prev = node
-                node.prev?.next = node
-                forwardLinks[rename.from] = node
-                reverseLinks[rename.to] = node
-            }
-
-            // Check that every property in the current type is at the end of a renaming chain, if it is in one
-            constants.keys.forEach {
-                requireThat(reverseLinks[it]?.next == null) { "$it is specified as a previously evolved type, but it also exists in the current type" }
-            }
-
-            // Check for cyclic dependencies
-            graph.forEach {
-                if (it.visited) return@forEach
-                // Find an unvisited node
-                var currentNode = it
-                currentNode.visit(it)
-                while (currentNode.next != null) {
-                    currentNode = currentNode.next!!
-                    if (currentNode.visited) {
-                        requireThat(currentNode.visitedBy != it) { "Cyclic renames are not allowed (${currentNode.transform.from})" }
-                        // we have found the start of another non-cyclic chain of dependencies
-                        // if they were cyclic we would have gone round in a loop and already thrown
-                        break
-                    }
-                    currentNode.visit(it)
-                }
-            }
-        }
     }
-
     // Transform used to test the unknown handler, leave this at as the final constant, uncomment
     // when regenerating test cases - if Java had a pre-processor this would be much neater
     //
     //,UnknownTest({ a -> UnknownTestTransform((a as UnknownTransformAnnotation).a, a.b, a.c)}) {
     //    override fun getDescriptor(): Any = DESCRIPTOR
     //    override fun getDescribed(): Any = ordinal
-    //    override fun validate(list: List<Transform>, constants: Map<String, Int>) = Unit
     //}
     ;
 
-    abstract fun validate(list: List<Transform>, constants: Map<String, Int>)
-
     companion object : DescribedTypeConstructor<TransformTypes> {
         val DESCRIPTOR = AMQPDescriptorRegistry.TRANSFORM_ELEMENT_KEY.amqpDescriptor
 
@@ -144,7 +58,9 @@ enum class TransformTypes(val build: (Annotation) -> Transform) : DescribedType
         override fun newInstance(obj: Any?): TransformTypes {
             val describedType = obj as DescribedType
 
-            requireThat(describedType.descriptor == DESCRIPTOR) { "Unexpected descriptor ${describedType.descriptor}." }
+            if (describedType.descriptor != DESCRIPTOR) {
+                throw NotSerializableWithReasonException("Unexpected descriptor ${describedType.descriptor}.")
+            }
 
             return try {
                 values()[describedType.described as Int]
@@ -154,11 +70,5 @@ enum class TransformTypes(val build: (Annotation) -> Transform) : DescribedType
         }
 
         override fun getTypeClass(): Class<*> = TransformTypes::class.java
-
-        protected inline fun requireThat(expr: Boolean, errorMessage: () -> String) {
-            if (!expr) {
-                throw NotSerializableWithReasonException(errorMessage())
-            }
-        }
     }
 }

serialization/src/main/kotlin/net/corda/serialization/internal/amqp/TransformsSchema.kt

@@ -3,11 +3,10 @@ package net.corda.serialization.internal.amqp
 import net.corda.core.KeepForDJVM
 import net.corda.core.serialization.CordaSerializationTransformEnumDefault
 import net.corda.core.serialization.CordaSerializationTransformRename
-import net.corda.core.utilities.contextLogger
-import net.corda.core.utilities.trace
 import net.corda.serialization.internal.NotSerializableDetailedException
-import net.corda.serialization.internal.NotSerializableWithReasonException
+import net.corda.serialization.internal.model.EnumTransforms
-import net.corda.serialization.internal.model.DefaultCacheProvider
+import net.corda.serialization.internal.model.InvalidEnumTransformsException
+import net.corda.serialization.internal.model.LocalTypeInformation
 import org.apache.qpid.proton.amqp.DescribedType
 import org.apache.qpid.proton.codec.DescribedTypeConstructor
 import java.io.NotSerializableException
@@ -188,6 +187,55 @@ class RenameSchemaTransform(val from: String, val to: String) : Transform() {
     override val name: String get() = typeName
 }
 
+typealias TransformsMap =  EnumMap<TransformTypes, MutableList<Transform>>
+
+/**
+ * Processes the annotations applied to classes intended for serialisation, to get the transforms that can be applied to them.
+ */
+object TransformsAnnotationProcessor {
+
+    /**
+     * Obtain all of the transforms applied for the given [Class].
+     */
+    fun getTransformsSchema(type: Class<*>): TransformsMap {
+        val result = TransformsMap(TransformTypes::class.java)
+        // We only have transforms for enums at present.
+        if (!type.isEnum) return result
+
+        supportedTransforms.forEach { supportedTransform ->
+            val annotationContainer = type.getAnnotation(supportedTransform.type) ?: return@forEach
+            result.processAnnotations(
+                    type,
+                    supportedTransform.enum,
+                    supportedTransform.getAnnotations(annotationContainer))
+        }
+
+        return result
+    }
+
+    private fun TransformsMap.processAnnotations(type: Class<*>, transformType: TransformTypes, annotations: List<Annotation>) {
+        annotations.forEach { annotation ->
+            addTransform(type, transformType, transformType.build(annotation))
+        }
+    }
+
+    private fun TransformsMap.addTransform(type: Class<*>, transformType: TransformTypes, transform: Transform) {
+        // we're explicitly rejecting repeated annotations, whilst it's fine and we'd just
+        // ignore them it feels like a good thing to alert the user to since this is
+        // more than likely a typo in their code so best make it an actual error
+        compute(transformType) { _, transforms ->
+            when {
+                transforms == null -> mutableListOf(transform)
+                transform in transforms -> throw AMQPNotSerializableException(
+                        type,
+                        "Repeated unique transformation annotation of type ${transform.name}")
+                else -> transforms.apply { this += transform }
+            }
+        }
+    }
+
+}
+
 /**
  * Represents the set of all transforms that can be a applied to all classes represented as part of
  * an AMQP schema. It forms a part of the AMQP envelope alongside the [Schema] and the serialized bytes
@@ -198,69 +246,6 @@ class RenameSchemaTransform(val from: String, val to: String) : Transform() {
 data class TransformsSchema(val types: Map<String, EnumMap<TransformTypes, MutableList<Transform>>>) : DescribedType {
     companion object : DescribedTypeConstructor<TransformsSchema> {
         val DESCRIPTOR = AMQPDescriptorRegistry.TRANSFORM_SCHEMA.amqpDescriptor
-        private val logger = contextLogger()
-
-        /**
-         * Takes a class name and either returns a cached instance of the TransformSet for it or, on a cache miss,
-         * instantiates the transform set before inserting into the cache and returning it.
-         *
-         * @param name fully qualified class name to lookup transforms for
-         * @param sf the [SerializerFactory] building this transform set. Needed as each can define it's own
-         * class loader and this dictates which classes we can and cannot see
-         */
-        fun get(name: String, sf: LocalSerializerFactory) =
-                sf.getOrBuildTransform(name) {
-            val transforms = EnumMap<TransformTypes, MutableList<Transform>>(TransformTypes::class.java)
-            try {
-                val clazz = sf.classloader.loadClass(name)
-
-                supportedTransforms.forEach { transform ->
-                    clazz.getAnnotation(transform.type)?.let { list ->
-                        transform.getAnnotations(list).forEach { annotation ->
-                            val t = transform.enum.build(annotation)
-
-                            // we're explicitly rejecting repeated annotations, whilst it's fine and we'd just
-                            // ignore them it feels like a good thing to alert the user to since this is
-                            // more than likely a typo in their code so best make it an actual error
-                            if (transforms.computeIfAbsent(transform.enum) { mutableListOf() }.any { t == it }) {
-                                throw AMQPNotSerializableException(
-                                        clazz,
-                                        "Repeated unique transformation annotation of type ${t.name}")
-                            }
-
-                            transforms[transform.enum]!!.add(t)
-                        }
-
-                        transform.enum.validate(
-                                transforms[transform.enum] ?: emptyList(),
-                                clazz.enumConstants.mapIndexed { i, s -> Pair(s.toString(), i) }.toMap())
-                    }
-                }
-            } catch (_: ClassNotFoundException) {
-                // if we can't load the class we'll end up caching an empty list which is fine as that
-                // list, on lookup, won't be included in the schema because it's empty
-            }
-
-            transforms
-        }
-
-        private fun getAndAdd(
-                type: String,
-                sf: LocalSerializerFactory,
-                map: MutableMap<String, EnumMap<TransformTypes, MutableList<Transform>>>) {
-            try {
-                get(type, sf).apply {
-                    if (isNotEmpty()) {
-                        map[type] = this
-                    }
-                }
-            } catch (e: NotSerializableWithReasonException) {
-                val message = "Error running transforms for $type: ${e.message}"
-                logger.error(message)
-                logger.trace { e.toString() }
-                throw NotSerializableDetailedException(type, e.message ?: "")
-            }
-        }
 
         /**
          * Prepare a schema for encoding, takes all of the types being transmitted and inspects each
@@ -270,10 +255,23 @@ data class TransformsSchema(val types: Map<String, EnumMap<TransformTypes, Mutab
          * @param schema should be a [Schema] generated for a serialised data structure
          * @param sf should be provided by the same serialization context that generated the schema
          */
-        fun build(schema: Schema, sf: LocalSerializerFactory) = TransformsSchema(
+        fun build(schema: Schema, sf: LocalSerializerFactory): TransformsSchema {
-                mutableMapOf<String, EnumMap<TransformTypes, MutableList<Transform>>>().apply {
+            val transformsMap = schema.types.asSequence().mapNotNull { type ->
-                    schema.types.forEach { type -> getAndAdd(type.name, sf, this) }
+                val localType = try {
-                })
+                    sf.classloader.loadClass(type.name)
+                } catch (_: ClassNotFoundException) {
+                    return@mapNotNull null
+                }
+                val localTypeInformation = sf.getTypeInformation(localType)
+                if (localTypeInformation is LocalTypeInformation.AnEnum) {
+                    localTypeInformation.transforms.source.let {
+                        if (it.isEmpty()) null else type.name to it
+                    }
+                }
+                else null
+            }.toMap()
+            return TransformsSchema(transformsMap)
+        }
 
         override fun getTypeClass(): Class<*> = TransformsSchema::class.java
 

serialization/src/main/kotlin/net/corda/serialization/internal/model/EnumTransforms.kt

@@ -0,0 +1,140 @@
+package net.corda.serialization.internal.model
+
+import net.corda.serialization.internal.amqp.EnumDefaultSchemaTransform
+import net.corda.serialization.internal.amqp.RenameSchemaTransform
+import net.corda.serialization.internal.amqp.TransformTypes
+import net.corda.serialization.internal.amqp.TransformsMap
+
+class InvalidEnumTransformsException(message: String): Exception(message)
+
+/**
+ * Contains all of the transforms that have been defined against an enum.
+ *
+ * @param defaults A [Map] of "new" to "old" for enum constant defaults
+ * @param renames A [Map] of "to" to "from" for enum constant renames.
+ * @param source The [TransformsMap] from which this data was derived.
+ */
+data class EnumTransforms(
+        val defaults: Map<String, String>,
+        val renames: Map<String, String>,
+        val source: TransformsMap) {
+
+    val size: Int get() = defaults.size + renames.size
+
+    companion object {
+        /**
+         * Build a set of [EnumTransforms] from a [TransformsMap], and validate it against the supplied constants.
+         */
+        fun build(source: TransformsMap, constants: Map<String, Int>): EnumTransforms {
+            val defaultTransforms = source[TransformTypes.EnumDefault]?.asSequence()
+                    ?.filterIsInstance<EnumDefaultSchemaTransform>()
+                    ?.toList() ?: emptyList()
+
+            val renameTransforms = source[TransformTypes.Rename]?.asSequence()
+                    ?.filterIsInstance<RenameSchemaTransform>()
+                    ?.toList() ?: emptyList()
+
+            // We have to do this validation here, because duplicate keys are discarded in EnumTransforms.
+            renameTransforms.groupingBy { it.from }.eachCount().forEach { from, count ->
+                if (count > 1) throw InvalidEnumTransformsException(
+                        "There are multiple transformations from $from, which is not allowed")
+            }
+            renameTransforms.groupingBy { it.to }.eachCount().forEach { to, count ->
+                if (count > 1) throw InvalidEnumTransformsException(
+                        "There are multiple transformations to $to, which is not allowed")
+            }
+
+            val defaults = defaultTransforms.associate { transform -> transform.new to transform.old }
+            val renames = renameTransforms.associate { transform -> transform.to to transform.from }
+            return EnumTransforms(defaults, renames, source).validate(constants)
+        }
+
+        val empty = EnumTransforms(emptyMap(), emptyMap(), TransformsMap(TransformTypes::class.java))
+    }
+
+    private fun validate(constants: Map<String, Int>): EnumTransforms {
+        validateNoCycles()
+
+        // For any name in the enum's constants, get all its previous names
+        fun renameChain(newName: String): Sequence<String> = generateSequence(newName) { renames[it] }
+
+        // Map all previous names to the current name's index.
+        val constantsBeforeRenaming = constants.asSequence().flatMap { (name, index) ->
+            renameChain(name).map { it to index }
+        }.toMap()
+
+        validateDefaults(constantsBeforeRenaming + constants)
+
+        return this
+    }
+
+    /**
+     * Verify that there are no rename cycles, i.e. C -> D -> C, or A -> B -> C -> A.
+     *
+     * This algorithm depends on the precondition (which is validated during construction of [EnumTransforms]) that there is at
+     * most one edge (a rename "from" one constant "to" another) between any two nodes (the constants themselves) in the rename
+     * graph. It makes a single pass over the set of edges, attempting to add each new edge to any existing chain of edges, or
+     * starting a new chain if there is no existing chain.
+     *
+     * For each new edge, one of the following must true:
+     *
+     * 1) There is no existing chain to which the edge can be connected, in which case it starts a new chain.
+     * 2) The edge can be added to one existing chain, either at the start or the end of the chain.
+     * 3) The edge is the "missing link" between two unconnected chains.
+     * 4) The edge is the "missing link" between the start of a chain and the end of that same chain, in which case we have a cycle.
+     *
+     * By detecting each condition, and updating the chains accordingly, we can perform cycle-detection in O(n) time.
+     */
+    private fun validateNoCycles() {
+        // We keep track of chains in both directions
+        val chainStartsToEnds = mutableMapOf<String, String>()
+        val chainEndsToStarts = mutableMapOf<String, String>()
+
+        for ((from, to) in renames) {
+            // If there is an existing chain, starting at the "to" node of this edge, then there is a chain from this edge's
+            // "from" to that chain's end.
+            val newEnd = chainStartsToEnds[to] ?: to
+
+            // If there is an existing chain, ending at the "from" node of this edge, then there is a chain from that chain's start
+            // to this edge's "to".
+            val newStart = chainEndsToStarts[from] ?: from
+
+            // If either chain ends where it begins, we have closed a loop, and detected a cycle.
+            if (newEnd == from || newStart == to) {
+                throw InvalidEnumTransformsException("Rename cycle detected in rename map starting from $newStart")
+            }
+
+            // Either update, or create, the chains in both directions.
+            chainStartsToEnds[from] = newEnd
+            chainEndsToStarts[to] = newStart
+
+            // If we have joined two previously unconnected chains, update their starts and ends accordingly.
+            chainStartsToEnds[newStart] = newEnd
+            chainEndsToStarts[newEnd] = newStart
+        }
+    }
+
+    /**
+     * Verify that defaults match up to existing constants (prior to their renaming).
+     */
+    private fun validateDefaults(constantsBeforeRenaming: Map<String, Int>) {
+        defaults.forEach { (new, old) ->
+            requireThat(constantsBeforeRenaming.contains(new)) { "Unknown enum constant $new" }
+            requireThat(constantsBeforeRenaming.contains(old)) {
+                "Enum extension defaults must be to a valid constant: $new -> $old. $old " +
+                        "doesn't exist in constant set $constantsBeforeRenaming"
+            }
+            requireThat(old != new) { "Enum extension $new cannot default to itself" }
+            requireThat(constantsBeforeRenaming[old]!! < constantsBeforeRenaming[new]!!) {
+                "Enum extensions must default to older constants. $new[${constantsBeforeRenaming[new]}] " +
+                        "defaults to $old[${constantsBeforeRenaming[old]}] which is greater"
+            }
+        }
+    }
+
+    private inline fun requireThat(expr: Boolean, errorMessage: () -> String) {
+        if (!expr) {
+            throw InvalidEnumTransformsException(errorMessage())
+        }
+    }
+}

serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt

@@ -179,7 +179,8 @@ sealed class LocalTypeInformation {
             override val observedType: Class<*>,
             override val typeIdentifier: TypeIdentifier,
             val members: List<String>,
-            val interfaces: List<LocalTypeInformation>): LocalTypeInformation()
+            val interfaces: List<LocalTypeInformation>,
+            val transforms: EnumTransforms): LocalTypeInformation()
 
     /**
      * Represents a type whose underlying class is an interface.

serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformationBuilder.kt

@@ -6,6 +6,7 @@ import net.corda.core.internal.kotlinObjectInstance
 import net.corda.core.serialization.ConstructorForDeserialization
 import net.corda.core.serialization.DeprecatedConstructorForDeserialization
 import net.corda.core.utilities.contextLogger
+import net.corda.serialization.internal.NotSerializableDetailedException
 import net.corda.serialization.internal.amqp.*
 import java.io.NotSerializableException
 import java.lang.reflect.Method
@@ -107,7 +108,8 @@ internal data class LocalTypeInformationBuilder(val lookup: LocalTypeLookup,
                     type,
                     typeIdentifier,
                     type.enumConstants.map { it.toString() },
-                    buildInterfaceInformation(type))
+                    buildInterfaceInformation(type),
+                    getEnumTransforms(type))
             type.kotlinObjectInstance != null -> LocalTypeInformation.Singleton(
                     type,
                     typeIdentifier,
@@ -126,6 +128,15 @@ internal data class LocalTypeInformationBuilder(val lookup: LocalTypeLookup,
         }
     }
 
+    private fun getEnumTransforms(type: Class<*>): EnumTransforms {
+        try {
+            val constants = type.enumConstants.asSequence().mapIndexed { index, constant -> constant.toString() to index }.toMap()
+            return EnumTransforms.build(TransformsAnnotationProcessor.getTransformsSchema(type), constants)
+        } catch (e: InvalidEnumTransformsException) {
+            throw NotSerializableDetailedException(type.name, e.message!!)
+        }
+    }
+
     private fun buildForParameterised(
             rawType: Class<*>,
             type: ParameterizedType,

serialization/src/main/kotlin/net/corda/serialization/internal/model/RemoteTypeInformation.kt

@@ -182,11 +182,3 @@ private data class RemoteTypeInformationPrettyPrinter(private val simplifyClassN
                     ": " + prettyPrint(value.type)
 }
 
-data class EnumTransforms(val defaults: Map<String, String>, val renames: Map<String, String>) {
-
-    val size: Int get() = defaults.size + renames.size
-
-    companion object {
-        val empty = EnumTransforms(emptyMap(), emptyMap())
-    }
-}

serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EnumEvolvabilityTests.kt

@@ -516,27 +516,6 @@ class EnumEvolvabilityTests {
         }.isInstanceOf(NotSerializableException::class.java)
     }
 
-    //
-    // In this test, like the above, we're looking to ensure repeated renames are rejected as
-    // unserailzble. However, in this case, it isn't a struct cycle, rather one element
-    // is renamed to match what a different element used to be called
-    //
-    @CordaSerializationTransformRenames(
-            CordaSerializationTransformRename(from = "B", to = "C"),
-            CordaSerializationTransformRename(from = "C", to = "D")
-    )
-    enum class RejectCyclicRenameAlt { A, C, D }
-
-    @Test
-    fun rejectCyclicRenameAlt() {
-        data class C(val e: RejectCyclicRenameAlt)
-
-        val sf = testDefaultFactory()
-        assertThatThrownBy {
-            SerializationOutput(sf).serialize(C(RejectCyclicRenameAlt.A))
-        }.isInstanceOf(NotSerializableException::class.java)
-    }
-
     @CordaSerializationTransformRenames(
             CordaSerializationTransformRename("G", "C"),
             CordaSerializationTransformRename("F", "G"),

serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EnumTransformationTests.kt

@@ -0,0 +1,56 @@
+package net.corda.serialization.internal.amqp
+
+import net.corda.core.serialization.CordaSerializationTransformEnumDefault
+import net.corda.core.serialization.CordaSerializationTransformEnumDefaults
+import net.corda.core.serialization.CordaSerializationTransformRename
+import net.corda.core.serialization.CordaSerializationTransformRenames
+import net.corda.serialization.internal.model.EnumTransforms
+import net.corda.serialization.internal.model.InvalidEnumTransformsException
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import kotlin.test.assertFailsWith
+
+class EnumTransformationTests {
+
+    @CordaSerializationTransformEnumDefaults(
+            CordaSerializationTransformEnumDefault(old = "C", new = "D"),
+            CordaSerializationTransformEnumDefault(old = "D", new = "E")
+    )
+    @CordaSerializationTransformRenames(
+        CordaSerializationTransformRename(to = "BOB", from = "FRED"),
+        CordaSerializationTransformRename(to = "FRED", from = "E")
+    )
+    enum class MultiOperations { A, B, C, D, BOB }
+
+    // See https://r3-cev.atlassian.net/browse/CORDA-1497
+    @Test
+    fun defaultAndRename() {
+        val transforms = EnumTransforms.build(
+                TransformsAnnotationProcessor.getTransformsSchema(MultiOperations::class.java),
+                MultiOperations::class.java.constants)
+
+        assertEquals(mapOf("BOB" to "FRED", "FRED" to "E"), transforms.renames)
+        assertEquals(mapOf("D" to "C", "E" to "D"), transforms.defaults)
+    }
+
+    @CordaSerializationTransformRenames(
+            CordaSerializationTransformRename(from = "A", to = "C"),
+            CordaSerializationTransformRename(from = "B", to = "D"),
+            CordaSerializationTransformRename(from = "C", to = "E"),
+            CordaSerializationTransformRename(from = "E", to = "B"),
+            CordaSerializationTransformRename(from = "D", to = "A")
+    )
+    enum class RenameCycle { A, B, C, D, E}
+
+    @Test
+    fun cycleDetection() {
+        assertFailsWith<InvalidEnumTransformsException> {
+            EnumTransforms.build(
+                    TransformsAnnotationProcessor.getTransformsSchema(RenameCycle::class.java),
+                    RenameCycle::class.java.constants)
+        }
+    }
+
+    private val Class<*>.constants: Map<String, Int> get() =
+        enumConstants.asSequence().mapIndexed { index, constant -> constant.toString() to index }.toMap()
+}