CORDA-1699: Restore binary compatibility for UniqueIdentifier class. (#3505)

* Update JarFilter to remove certain annotations from primary constructors.
* Enhance unit tests for deleting constructors.
* Update the documentation to explain how to handle non-deterministic default
constructor parameters.
* Reduce the base logging level of SanitisingTransformer to DEBUG.
* Simplify the execution of repeatable ClassVisitors.
* Simplify Gradle usage slightly.
* Add test for deterministic UniqueIdentifier.
* Update README to cover handling default parameters.

buildSrc/build.gradle

@@ -22,9 +22,6 @@ buildscript {
     }
 }
 
-apply plugin: 'maven'
-apply plugin: 'java'
-
 repositories {
     mavenLocal()
     mavenCentral()
@@ -51,12 +48,13 @@ allprojects {
     }
 }
 
-dependencies {
+configurations {
-    // Add the top-level projects ONLY to the host project.
+    runtime
-    runtime project.childProjects.values().collect {
-        project(it.path)
-    }
 }
 
-// Don't create an empty jar. The plugins are now in child projects.
+dependencies {
-jar.enabled = false
+    // Add the top-level projects ONLY to the host project.
+    runtime project.childProjects.collect { n, p ->
+        project(p.path)
+    }
+}

buildSrc/jarfilter/README.md

@@ -59,6 +59,72 @@ task jarFilter(type: JarFilterTask) {
 You can specify as many annotations for each role as you like. The only constraint is that a given
 annotation cannot be assigned to more than one role.
 
+#### Removing unwanted default parameter values
+It is possible to assign non-deterministic expressions as default values for Kotlin constructors and functions. For
+example:
+```kotlin
+data class UniqueIdentifier(val externalId: String? = null, val id: UUID = UUID.randomUUID())
+```
+
+The Kotlin compiler will generate _two_ constructors in this case:
+```
+UniqueIdentifier(String?, UUID)
+UniqueIdentifier(String?, UUID, Int, DefaultConstructorMarker)
+```
+
+The first constructor is the primary constructor that we would expect (and which we'd like to keep), whereas the
+second is a public synthetic constructor that Kotlin applications invoke to handle the different combinations of
+default parameter values. Unfortunately, this synthetic constructor is therefore also part of the Kotlin ABI and
+so we _cannot_ rewrite the class like this to remove the default values:
+```kotlin
+// THIS REFACTOR WOULD BREAK THE KOTLIN ABI!
+data class UniqueIdentifier(val externalId: String?, val id: UUID) {
+    constructor(externalId: String?) : this(externalId, UUID.randomUUID())
+    constructor() : this(null)
+}
+```
+
+The refactored class would have the following constructors, and would require client applications to be recompiled:
+```
+UniqueIdentifier(String?, UUID)
+UniqueIdentifier(String?)
+UniqueIdentifier()
+```
+
+We therefore need to keep the default constructor parameters in order to preserve the ABI for the unfiltered code,
+which in turn means that `JarFilter` will need to delete only the synthetic constructor and leave the primary
+constructor intact. However, Kotlin does not currently allow us to annotate _specific_ constructors - see
+[KT-22524](https://youtrack.jetbrains.com/issue/KT-22524). Until it does, `JarFilter` will perform an initial
+"sanitising" pass over the JAR file to remove any unwanted annotations from the primary constructors. These unwanted
+annotations are configured in the `JarFilter` task definition:
+```gradle
+task jarFilter(type: JarFilterTask) {
+    ...
+    annotations {
+        ...
+        forSanitise = [
+            "org.testing.DeleteMe"
+        ]
+    }
+}
+```
+
+This allows us to annotate the `UniqueIdentifier` class like this:
+```kotlin
+data class UniqueIdentifier @DeleteMe constructor(val externalId: String? = null, val id: UUID = UUID.randomUUID())
+```
+
+to generate these constructors:
+```
+UniqueIdentifier(String?, UUID)
+@DeleteMe UniqueIdentifier(String?, UUID, Int, DefaultConstructorMarker)
+```
+
+We currently **do not** sanitise annotations from functions with default parameter values, although (in theory) these
+may also be non-deterministic. We will need to extend the sanitation pass to include such functions if/when the need
+arises. At the moment, deleting such functions _entirely_ is enough, whereas also deleting a primary constructor means
+that we can no longer create instances of that class either.
+
 ### The `MetaFixer` task
 The `MetaFixer` task updates the `@kotlin.Metadata` annotations by removing references to any functions,
 constructors, properties or nested classes that no longer exist in the byte-code. This is primarily to

buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/FilterTransformer.kt

@@ -26,7 +26,7 @@ class FilterTransformer private constructor (
     private val unwantedFields: MutableSet<FieldElement>,
     private val deletedMethods: MutableSet<MethodElement>,
     private val stubbedMethods: MutableSet<MethodElement>
-) : KotlinAwareVisitor(ASM6, visitor, logger, kotlinMetadata), Repeatable<FilterTransformer> {
+) : KotlinAfterProcessor(ASM6, visitor, logger, kotlinMetadata), Repeatable<FilterTransformer> {
     constructor(
         visitor: ClassVisitor,
         logger: Logger,
@@ -47,8 +47,8 @@ class FilterTransformer private constructor (
         stubbedMethods = mutableSetOf()
     )
 
-    private var _className: String = "(unknown)"
+    var className: String = "(unknown)"
-    val className: String get() = _className
+        private set
 
     val isUnwantedClass: Boolean get() = isUnwantedClass(className)
     override val hasUnwantedElements: Boolean
@@ -76,7 +76,7 @@ class FilterTransformer private constructor (
     )
 
     override fun visit(version: Int, access: Int, clsName: String, signature: String?, superName: String?, interfaces: Array<String>?) {
-        _className = clsName
+        className = clsName
         logger.info("Class {}", clsName)
         super.visit(version, access, clsName, signature, superName, interfaces)
     }
@@ -172,7 +172,7 @@ class FilterTransformer private constructor (
     /**
      * Removes the deleted methods and fields from the Kotlin Class metadata.
      */
-    override fun transformClassMetadata(d1: List<String>, d2: List<String>): List<String> {
+    override fun processClassMetadata(d1: List<String>, d2: List<String>): List<String> {
         val partitioned = deletedMethods.groupBy(MethodElement::isConstructor)
         val prefix = "$className$"
         return ClassMetadataTransformer(
@@ -191,7 +191,7 @@ class FilterTransformer private constructor (
     /**
      * Removes the deleted methods and fields from the Kotlin Package metadata.
      */
-    override fun transformPackageMetadata(d1: List<String>, d2: List<String>): List<String> {
+    override fun processPackageMetadata(d1: List<String>, d2: List<String>): List<String> {
         return PackageMetadataTransformer(
                 logger = logger,
                 deletedFields = unwantedFields,

buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/JarFilterTask.kt

@@ -46,6 +46,9 @@ open class JarFilterTask : DefaultTask() {
     @get:Input
     protected var forRemove: Set<String> = emptySet()
 
+    @get:Input
+    protected var forSanitise: Set<String> = emptySet()
+
     fun annotations(assign: Closure<List<String>>) {
         assign.call()
     }
@@ -90,6 +93,9 @@ open class JarFilterTask : DefaultTask() {
         if (forRemove.isNotEmpty()) {
             logger.info("- Annotations '{}' will be removed entirely", forRemove.joinToString())
         }
+        if (forSanitise.isNotEmpty()) {
+            logger.info("- Annotations '{}' will be removed from primary constructors", forSanitise.joinToString())
+        }
         checkDistinctAnnotations()
         try {
             jars.forEach { jar ->
@@ -136,6 +142,11 @@ open class JarFilterTask : DefaultTask() {
         private val source: Path = inFile.toPath()
         private val target: Path = toFiltered(inFile).toPath()
 
+        private val descriptorsForRemove = toDescriptors(forRemove)
+        private val descriptorsForDelete = toDescriptors(forDelete)
+        private val descriptorsForStub = toDescriptors(forStub)
+        private val descriptorsForSanitising = toDescriptors(forSanitise)
+
         init {
             Files.deleteIfExists(target)
         }
@@ -145,10 +156,14 @@ open class JarFilterTask : DefaultTask() {
             var input = source
 
             try {
+                if (descriptorsForSanitising.isNotEmpty() && SanitisingPass(input).use { it.run() }) {
+                    input = target.moveToInput()
+                }
+
                 var passes = 1
                 while (true) {
                     verbose("Pass {}", passes)
-                    val isModified = Pass(input).use { it.run() }
+                    val isModified = FilterPass(input).use { it.run() }
 
                     if (!isModified) {
                         logger.info("No changes after latest pass - exiting.")
@@ -157,9 +172,7 @@ open class JarFilterTask : DefaultTask() {
                         break
                     }
 
-                    input = Files.move(
+                    input = target.moveToInput()
-                        target, Files.createTempFile(target.parent, "filter-", ".tmp"), REPLACE_EXISTING)
-                    verbose("New input JAR: {}", input)
                 }
             } catch (e: Exception) {
                 logger.error("Error filtering '{}' elements from {}", ArrayList(forRemove).apply { addAll(forDelete); addAll(forStub) }, input)
@@ -167,14 +180,20 @@ open class JarFilterTask : DefaultTask() {
             }
         }
 
-        private inner class Pass(input: Path): Closeable {
+        private fun Path.moveToInput(): Path {
+            return Files.move(this, Files.createTempFile(parent, "filter-", ".tmp"), REPLACE_EXISTING).also {
+                verbose("New input JAR: {}", it)
+            }
+        }
+
+        private abstract inner class Pass(input: Path): Closeable {
             /**
              * Use [ZipFile] instead of [java.util.jar.JarInputStream] because
              * JarInputStream consumes MANIFEST.MF when it's the first or second entry.
              */
-            private val inJar = ZipFile(input.toFile())
+            protected val inJar = ZipFile(input.toFile())
-            private val outJar = ZipOutputStream(Files.newOutputStream(target))
+            protected val outJar = ZipOutputStream(Files.newOutputStream(target))
-            private var isModified = false
+            protected var isModified = false
 
             @Throws(IOException::class)
             override fun close() {
@@ -183,6 +202,8 @@ open class JarFilterTask : DefaultTask() {
                 }
             }
 
+            abstract fun transform(inBytes: ByteArray): ByteArray
+
             fun run(): Boolean {
                 outJar.setLevel(BEST_COMPRESSION)
                 outJar.setComment(inJar.comment)
@@ -207,16 +228,29 @@ open class JarFilterTask : DefaultTask() {
                 }
                 return isModified
             }
+        }
 
-            private fun transform(inBytes: ByteArray): ByteArray {
+        private inner class SanitisingPass(input: Path) : Pass(input) {
+            override fun transform(inBytes: ByteArray): ByteArray {
+                return ClassWriter(0).let { writer ->
+                    val transformer = SanitisingTransformer(writer, logger, descriptorsForSanitising)
+                    ClassReader(inBytes).accept(transformer, 0)
+                    isModified = isModified or transformer.isModified
+                    writer.toByteArray()
+                }
+            }
+        }
+
+        private inner class FilterPass(input: Path) : Pass(input) {
+            override fun transform(inBytes: ByteArray): ByteArray {
                 var reader = ClassReader(inBytes)
                 var writer = ClassWriter(COMPUTE_MAXS)
                 var transformer = FilterTransformer(
                     visitor = writer,
                     logger = logger,
-                    removeAnnotations = toDescriptors(forRemove),
+                    removeAnnotations = descriptorsForRemove,
-                    deleteAnnotations = toDescriptors(forDelete),
+                    deleteAnnotations = descriptorsForDelete,
-                    stubAnnotations = toDescriptors(forStub),
+                    stubAnnotations = descriptorsForStub,
                     unwantedClasses = unwantedClasses
                 )
 

buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/KotlinAwareVisitor.kt

@@ -1,13 +1,13 @@
 package net.corda.gradle.jarfilter
 
+import org.gradle.api.logging.LogLevel
 import org.gradle.api.logging.Logger
 import org.jetbrains.kotlin.load.java.JvmAnnotationNames.*
 import org.objectweb.asm.AnnotationVisitor
 import org.objectweb.asm.ClassVisitor
 
 /**
- * Kotlin support: Loads the ProtoBuf data from the [kotlin.Metadata] annotation,
+ * Kotlin support: Loads the ProtoBuf data from the [kotlin.Metadata] annotation.
- * or writes new ProtoBuf data that was created during a previous pass.
  */
 abstract class KotlinAwareVisitor(
     api: Int,
@@ -27,23 +27,24 @@ abstract class KotlinAwareVisitor(
     private var classKind: Int = 0
 
     open val hasUnwantedElements: Boolean get() = kotlinMetadata.isNotEmpty()
+    protected open val level: LogLevel = LogLevel.INFO
 
-    protected abstract fun transformClassMetadata(d1: List<String>, d2: List<String>): List<String>
+    protected abstract fun processClassMetadata(d1: List<String>, d2: List<String>): List<String>
-    protected abstract fun transformPackageMetadata(d1: List<String>, d2: List<String>): List<String>
+    protected abstract fun processPackageMetadata(d1: List<String>, d2: List<String>): List<String>
+    protected abstract fun processKotlinAnnotation()
 
     override fun visitAnnotation(descriptor: String, visible: Boolean): AnnotationVisitor? {
         val av = super.visitAnnotation(descriptor, visible) ?: return null
         return if (descriptor == METADATA_DESC) KotlinMetadataAdaptor(av) else av
     }
 
-    override fun visitEnd() {
+    protected fun processMetadata() {
-        super.visitEnd()
         if (kotlinMetadata.isNotEmpty()) {
-            logger.info("- Examining Kotlin @Metadata[k={}]", classKind)
+            logger.log(level, "- Examining Kotlin @Metadata[k={}]", classKind)
             val d1 = kotlinMetadata.remove(METADATA_DATA_FIELD_NAME)
             val d2 = kotlinMetadata.remove(METADATA_STRINGS_FIELD_NAME)
             if (d1 != null && d1.isNotEmpty() && d2 != null) {
-                transformMetadata(d1, d2).apply {
+                processMetadata(d1, d2).apply {
                     if (isNotEmpty()) {
                         kotlinMetadata[METADATA_DATA_FIELD_NAME] = this
                         kotlinMetadata[METADATA_STRINGS_FIELD_NAME] = d2
@@ -53,12 +54,12 @@ abstract class KotlinAwareVisitor(
         }
     }
 
-    private fun transformMetadata(d1: List<String>, d2: List<String>): List<String> {
+    private fun processMetadata(d1: List<String>, d2: List<String>): List<String> {
         return when (classKind) {
-            KOTLIN_CLASS -> transformClassMetadata(d1, d2)
+            KOTLIN_CLASS -> processClassMetadata(d1, d2)
-            KOTLIN_FILE, KOTLIN_MULTIFILE_PART -> transformPackageMetadata(d1, d2)
+            KOTLIN_FILE, KOTLIN_MULTIFILE_PART -> processPackageMetadata(d1, d2)
             KOTLIN_SYNTHETIC -> {
-                logger.info("-- synthetic class ignored")
+                logger.log(level,"-- synthetic class ignored")
                 emptyList()
             }
             else -> {
@@ -66,7 +67,7 @@ abstract class KotlinAwareVisitor(
                  * For class-kind=4 (i.e. "multi-file"), we currently
                  * expect d1=[list of multi-file-part classes], d2=null.
                  */
-                logger.info("-- unsupported class-kind {}", classKind)
+                logger.log(level,"-- unsupported class-kind {}", classKind)
                 emptyList()
             }
         }
@@ -91,19 +92,68 @@ abstract class KotlinAwareVisitor(
             return null
         }
 
-        private inner class ArrayAccumulator(av: AnnotationVisitor, private val name: String) : AnnotationVisitor(api, av) {
+        override fun visitEnd() {
-            private val data: MutableList<String> = mutableListOf()
+            super.visitEnd()
-
+            processKotlinAnnotation()
-            override fun visit(name: String?, value: Any?) {
-                super.visit(name, value)
-                data.add(value as String)
-            }
-
-            override fun visitEnd() {
-                super.visitEnd()
-                kotlinMetadata[name] = data
-                logger.debug("-- read @Metadata.{}[{}]", name, data.size)
-            }
         }
     }
-}
+
+    private inner class ArrayAccumulator(av: AnnotationVisitor, private val name: String) : AnnotationVisitor(api, av) {
+        private val data: MutableList<String> = mutableListOf()
+
+        override fun visit(name: String?, value: Any?) {
+            super.visit(name, value)
+            data.add(value as String)
+        }
+
+        override fun visitEnd() {
+            super.visitEnd()
+            kotlinMetadata[name] = data
+            logger.debug("-- read @Metadata.{}[{}]", name, data.size)
+        }
+    }
+}
+
+/**
+ * Loads the ProtoBuf data from the [kotlin.Metadata] annotation, or
+ * writes new ProtoBuf data that was created during a previous pass.
+ */
+abstract class KotlinAfterProcessor(
+    api: Int,
+    visitor: ClassVisitor,
+    logger: Logger,
+    kotlinMetadata: MutableMap<String, List<String>>
+) : KotlinAwareVisitor(api, visitor, logger, kotlinMetadata) {
+
+    /**
+     * Process the metadata once we have finished visiting the class.
+     * This will allow us to rewrite the [kotlin.Metadata] annotation
+     * in the next visit.
+     */
+    override fun visitEnd() {
+        super.visitEnd()
+        processMetadata()
+    }
+
+    /**
+     * Do nothing immediately after we have parsed [kotlin.Metadata].
+     */
+    final override fun processKotlinAnnotation() {}
+}
+
+/**
+ * Loads the ProtoBuf data from the [kotlin.Metadata] annotation
+ * and then processes it before visiting the rest of the class.
+ */
+abstract class KotlinBeforeProcessor(
+    api: Int,
+    visitor: ClassVisitor,
+    logger: Logger,
+    kotlinMetadata: MutableMap<String, List<String>>
+) : KotlinAwareVisitor(api, visitor, logger, kotlinMetadata) {
+
+    /**
+     * Process the ProtoBuf data as soon as we have parsed [kotlin.Metadata].
+     */
+    final override fun processKotlinAnnotation() = processMetadata()
+}

buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/MetaFixerVisitor.kt

@@ -17,7 +17,7 @@ class MetaFixerVisitor private constructor(
     private val fields: MutableSet<FieldElement>,
     private val methods: MutableSet<String>,
     private val nestedClasses: MutableSet<String>
-) : KotlinAwareVisitor(ASM6, visitor, logger, kotlinMetadata), Repeatable<MetaFixerVisitor> {
+) : KotlinAfterProcessor(ASM6, visitor, logger, kotlinMetadata), Repeatable<MetaFixerVisitor> {
     constructor(visitor: ClassVisitor, logger: Logger, classNames: Set<String>)
         : this(visitor, logger, mutableMapOf(), classNames, mutableSetOf(), mutableSetOf(), mutableSetOf())
 
@@ -52,7 +52,7 @@ class MetaFixerVisitor private constructor(
         return super.visitInnerClass(clsName, outerName, innerName, access)
     }
 
-    override fun transformClassMetadata(d1: List<String>, d2: List<String>): List<String> {
+    override fun processClassMetadata(d1: List<String>, d2: List<String>): List<String> {
         return ClassMetaFixerTransformer(
                 logger = logger,
                 actualFields = fields,
@@ -64,7 +64,7 @@ class MetaFixerVisitor private constructor(
             .transform()
     }
 
-    override fun transformPackageMetadata(d1: List<String>, d2: List<String>): List<String> {
+    override fun processPackageMetadata(d1: List<String>, d2: List<String>): List<String> {
         return PackageMetaFixerTransformer(
                 logger = logger,
                 actualFields = fields,

buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/SanitisingTransformer.kt

@@ -0,0 +1,81 @@
+package net.corda.gradle.jarfilter
+
+import org.gradle.api.logging.LogLevel
+import org.gradle.api.logging.Logger
+import org.jetbrains.kotlin.metadata.ProtoBuf
+import org.jetbrains.kotlin.metadata.deserialization.Flags.*
+import org.jetbrains.kotlin.metadata.deserialization.TypeTable
+import org.jetbrains.kotlin.metadata.jvm.JvmProtoBuf.*
+import org.jetbrains.kotlin.metadata.jvm.deserialization.BitEncoding
+import org.jetbrains.kotlin.metadata.jvm.deserialization.JvmNameResolver
+import org.jetbrains.kotlin.metadata.jvm.deserialization.JvmProtoBufUtil.EXTENSION_REGISTRY
+import org.jetbrains.kotlin.metadata.jvm.deserialization.JvmProtoBufUtil.getJvmConstructorSignature
+import org.objectweb.asm.AnnotationVisitor
+import org.objectweb.asm.ClassVisitor
+import org.objectweb.asm.MethodVisitor
+import org.objectweb.asm.Opcodes.*
+import java.io.ByteArrayInputStream
+
+/**
+ * This is (hopefully?!) a temporary solution for classes with [JvmOverloads] constructors.
+ * We need to be able to annotate ONLY the secondary constructors for such classes, but Kotlin
+ * will apply any annotation to all constructors equally. Nor can we replace the overloaded
+ * constructor with individual constructors because this will break ABI compatibility. (Kotlin
+ * generates a synthetic public constructor to handle default parameter values.)
+ *
+ * This transformer identifies a class's primary constructor and removes all of its unwanted annotations.
+ * It will become superfluous when Kotlin allows us to target only the secondary constructors with our
+ * filtering annotations in the first place.
+ */
+class SanitisingTransformer(visitor: ClassVisitor, logger: Logger, private val unwantedAnnotations: Set<String>)
+    : KotlinBeforeProcessor(ASM6, visitor, logger, mutableMapOf()) {
+
+    var isModified: Boolean = false
+        private set
+    override val level: LogLevel = LogLevel.DEBUG
+
+    private var className: String = "(unknown)"
+    private var primaryConstructor: MethodElement? = null
+
+    override fun processPackageMetadata(d1: List<String>, d2: List<String>): List<String> = emptyList()
+
+    override fun processClassMetadata(d1: List<String>, d2: List<String>): List<String> {
+        val input = ByteArrayInputStream(BitEncoding.decodeBytes(d1.toTypedArray()))
+        val stringTableTypes = StringTableTypes.parseDelimitedFrom(input, EXTENSION_REGISTRY)
+        val nameResolver = JvmNameResolver(stringTableTypes, d2.toTypedArray())
+        val message = ProtoBuf.Class.parseFrom(input, EXTENSION_REGISTRY)
+        val typeTable = TypeTable(message.typeTable)
+
+        for (constructor in message.constructorList) {
+            if (!IS_SECONDARY.get(constructor.flags)) {
+                val signature = getJvmConstructorSignature(constructor, nameResolver, typeTable) ?: break
+                primaryConstructor = MethodElement("<init>", signature.drop("<init>".length))
+                logger.log(level, "Class {} has primary constructor {}", className, signature)
+                break
+            }
+        }
+        return emptyList()
+    }
+
+    override fun visit(version: Int, access: Int, clsName: String, signature: String?, superName: String?, interfaces: Array<String>?) {
+        className = clsName
+        super.visit(version, access, clsName, signature, superName, interfaces)
+    }
+
+    override fun visitMethod(access: Int, methodName: String, descriptor: String, signature: String?, exceptions: Array<String>?): MethodVisitor? {
+        val method = MethodElement(methodName, descriptor, access)
+        val mv = super.visitMethod(access, methodName, descriptor, signature, exceptions) ?: return null
+        return if (method == primaryConstructor) SanitisingMethodAdapter(mv, method) else mv
+    }
+
+    private inner class SanitisingMethodAdapter(mv: MethodVisitor, private val method: MethodElement) : MethodVisitor(api, mv) {
+        override fun visitAnnotation(descriptor: String, visible: Boolean): AnnotationVisitor? {
+            if (unwantedAnnotations.contains(descriptor)) {
+                logger.info("Sanitising annotation {} from method {}.{}{}", descriptor, className, method.name, method.descriptor)
+                isModified = true
+                return null
+            }
+            return super.visitAnnotation(descriptor, visible)
+        }
+    }
+}

buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/Utils.kt

@@ -73,17 +73,20 @@ internal val String.descriptor: String get() = "L$toPathFormat;"
 internal fun <T> ByteArray.execute(visitor: (ClassVisitor) -> T, flags: Int = 0, passes: Int = 2): ByteArray
     where T : ClassVisitor,
           T : Repeatable<T> {
-    var reader = ClassReader(this)
+    var bytecode = this
     var writer = ClassWriter(flags)
-    val transformer = visitor(writer)
+    var transformer = visitor(writer)
     var count = max(passes, 1)
 
-    reader.accept(transformer, 0)
+    while (--count >= 0) {
-    while (transformer.hasUnwantedElements && --count > 0) {
+        ClassReader(bytecode).accept(transformer, 0)
-        reader = ClassReader(writer.toByteArray())
+        bytecode = writer.toByteArray()
+
+        if (!transformer.hasUnwantedElements) break
+
         writer = ClassWriter(flags)
-        reader.accept(transformer.recreate(writer), 0)
+        transformer = transformer.recreate(writer)
     }
 
-    return writer.toByteArray()
+    return bytecode
 }

buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/DeleteConstructorTest.kt

@@ -36,7 +36,7 @@ class DeleteConstructorTest {
 
     @Test
     fun deleteConstructorWithLongParameter() {
-        val longConstructor = isConstructor(SECONDARY_CONSTRUCTOR_CLASS, hasParam(Long::class))
+        val longConstructor = isConstructor(SECONDARY_CONSTRUCTOR_CLASS, Long::class)
 
         classLoaderFor(testProject.sourceJar).use { cl ->
             cl.load<HasLong>(SECONDARY_CONSTRUCTOR_CLASS).apply {
@@ -58,7 +58,7 @@ class DeleteConstructorTest {
 
     @Test
     fun deleteConstructorWithStringParameter() {
-        val stringConstructor = isConstructor(SECONDARY_CONSTRUCTOR_CLASS, hasParam(String::class))
+        val stringConstructor = isConstructor(SECONDARY_CONSTRUCTOR_CLASS, String::class)
 
         classLoaderFor(testProject.sourceJar).use { cl ->
             cl.load<HasString>(SECONDARY_CONSTRUCTOR_CLASS).apply {
@@ -80,7 +80,7 @@ class DeleteConstructorTest {
 
     @Test
     fun showUnannotatedConstructorIsUnaffected() {
-        val intConstructor = isConstructor(SECONDARY_CONSTRUCTOR_CLASS, hasParam(Int::class))
+        val intConstructor = isConstructor(SECONDARY_CONSTRUCTOR_CLASS, Int::class)
         classLoaderFor(testProject.filteredJar).use { cl ->
             cl.load<HasAll>(SECONDARY_CONSTRUCTOR_CLASS).apply {
                 getDeclaredConstructor(Int::class.java).newInstance(NUMBER).also {
@@ -96,7 +96,7 @@ class DeleteConstructorTest {
 
     @Test
     fun deletePrimaryConstructorWithStringParameter() {
-        val stringConstructor = isConstructor(STRING_PRIMARY_CONSTRUCTOR_CLASS, hasParam(String::class))
+        val stringConstructor = isConstructor(STRING_PRIMARY_CONSTRUCTOR_CLASS, String::class)
 
         classLoaderFor(testProject.sourceJar).use { cl ->
             cl.load<HasString>(STRING_PRIMARY_CONSTRUCTOR_CLASS).apply {
@@ -119,7 +119,7 @@ class DeleteConstructorTest {
 
     @Test
     fun deletePrimaryConstructorWithLongParameter() {
-        val longConstructor = isConstructor(LONG_PRIMARY_CONSTRUCTOR_CLASS, hasParam(Long::class))
+        val longConstructor = isConstructor(LONG_PRIMARY_CONSTRUCTOR_CLASS, Long::class)
 
         classLoaderFor(testProject.sourceJar).use { cl ->
             cl.load<HasLong>(LONG_PRIMARY_CONSTRUCTOR_CLASS).apply {
@@ -142,7 +142,7 @@ class DeleteConstructorTest {
 
     @Test
     fun deletePrimaryConstructorWithIntParameter() {
-        val intConstructor = isConstructor(INT_PRIMARY_CONSTRUCTOR_CLASS, hasParam(Int::class))
+        val intConstructor = isConstructor(INT_PRIMARY_CONSTRUCTOR_CLASS, Int::class)
 
         classLoaderFor(testProject.sourceJar).use { cl ->
             cl.load<HasInt>(INT_PRIMARY_CONSTRUCTOR_CLASS).apply {

buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/SanitiseConstructorTest.kt

@@ -0,0 +1,183 @@
+package net.corda.gradle.jarfilter
+
+import net.corda.gradle.jarfilter.matcher.*
+import net.corda.gradle.unwanted.HasInt
+import net.corda.gradle.unwanted.HasLong
+import net.corda.gradle.unwanted.HasString
+import org.assertj.core.api.Assertions.*
+import org.hamcrest.core.IsCollectionContaining.hasItem
+import org.junit.Assert.*
+import org.junit.ClassRule
+import org.junit.Test
+import org.junit.rules.RuleChain
+import org.junit.rules.TemporaryFolder
+import org.junit.rules.TestRule
+import kotlin.jvm.kotlin
+import kotlin.reflect.full.primaryConstructor
+import kotlin.test.assertFailsWith
+
+class SanitiseConstructorTest {
+    companion object {
+        private const val COUNT_INITIAL_OVERLOADED = 1
+        private const val COUNT_INITIAL_MULTIPLE = 2
+        private val testProjectDir = TemporaryFolder()
+        private val testProject = JarFilterProject(testProjectDir, "sanitise-constructor")
+
+        @ClassRule
+        @JvmField
+        val rules: TestRule = RuleChain
+            .outerRule(testProjectDir)
+            .around(testProject)
+    }
+
+    @Test
+    fun deleteOverloadedLongConstructor() = checkClassWithLongParameter(
+        "net.corda.gradle.HasOverloadedLongConstructor",
+        COUNT_INITIAL_OVERLOADED
+    )
+
+    @Test
+    fun deleteMultipleLongConstructor() = checkClassWithLongParameter(
+        "net.corda.gradle.HasMultipleLongConstructors",
+        COUNT_INITIAL_MULTIPLE
+    )
+
+    private fun checkClassWithLongParameter(longClass: String, initialCount: Int) {
+        val longConstructor = isConstructor(longClass, Long::class)
+
+        classLoaderFor(testProject.sourceJar).use { cl ->
+            cl.load<HasLong>(longClass).apply {
+                getDeclaredConstructor(Long::class.java).newInstance(BIG_NUMBER).also {
+                    assertEquals(BIG_NUMBER, it.longData())
+                }
+                kotlin.constructors.apply {
+                    assertThat("<init>(J) not found", this, hasItem(longConstructor))
+                    assertEquals(initialCount, this.size)
+                }
+                val primary = kotlin.primaryConstructor ?: throw AssertionError("primary constructor missing")
+                assertThat(primary.call(BIG_NUMBER).longData()).isEqualTo(BIG_NUMBER)
+
+                newInstance().also {
+                    assertEquals(0, it.longData())
+                }
+            }
+        }
+
+        classLoaderFor(testProject.filteredJar).use { cl ->
+            cl.load<HasLong>(longClass).apply {
+                getDeclaredConstructor(Long::class.java).newInstance(BIG_NUMBER).also {
+                    assertEquals(BIG_NUMBER, it.longData())
+                }
+                kotlin.constructors.apply {
+                    assertThat("<init>(J) not found", this, hasItem(longConstructor))
+                    assertEquals(1, this.size)
+                }
+                val primary = kotlin.primaryConstructor ?: throw AssertionError("primary constructor missing")
+                assertThat(primary.call(BIG_NUMBER).longData()).isEqualTo(BIG_NUMBER)
+
+                assertFailsWith<NoSuchMethodException> { getDeclaredConstructor() }
+            }
+        }
+    }
+
+    @Test
+    fun deleteOverloadedIntConstructor() = checkClassWithIntParameter(
+        "net.corda.gradle.HasOverloadedIntConstructor",
+        COUNT_INITIAL_OVERLOADED
+    )
+
+    @Test
+    fun deleteMultipleIntConstructor() = checkClassWithIntParameter(
+        "net.corda.gradle.HasMultipleIntConstructors",
+        COUNT_INITIAL_MULTIPLE
+    )
+
+    private fun checkClassWithIntParameter(intClass: String, initialCount: Int) {
+        val intConstructor = isConstructor(intClass, Int::class)
+
+        classLoaderFor(testProject.sourceJar).use { cl ->
+            cl.load<HasInt>(intClass).apply {
+                getDeclaredConstructor(Int::class.java).newInstance(NUMBER).also {
+                    assertEquals(NUMBER, it.intData())
+                }
+                kotlin.constructors.apply {
+                    assertThat("<init>(I) not found", this, hasItem(intConstructor))
+                    assertEquals(initialCount, this.size)
+                }
+                val primary = kotlin.primaryConstructor ?: throw AssertionError("primary constructor missing")
+                assertThat(primary.call(NUMBER).intData()).isEqualTo(NUMBER)
+
+                //assertThat("", constructors, hasItem(isConstructor(""))
+                newInstance().also {
+                    assertEquals(0, it.intData())
+                }
+            }
+        }
+
+        classLoaderFor(testProject.filteredJar).use { cl ->
+            cl.load<HasInt>(intClass).apply {
+                getDeclaredConstructor(Int::class.java).newInstance(NUMBER).also {
+                    assertEquals(NUMBER, it.intData())
+                }
+                kotlin.constructors.apply {
+                    assertThat("<init>(I) not found", this, hasItem(intConstructor))
+                    assertEquals(1, this.size)
+                }
+                val primary = kotlin.primaryConstructor ?: throw AssertionError("primary constructor missing")
+                assertThat(primary.call(NUMBER).intData()).isEqualTo(NUMBER)
+
+                assertFailsWith<NoSuchMethodException> { getDeclaredConstructor() }
+            }
+        }
+    }
+
+    @Test
+    fun deleteOverloadedStringConstructor() = checkClassWithStringParameter(
+        "net.corda.gradle.HasOverloadedStringConstructor",
+        COUNT_INITIAL_OVERLOADED
+    )
+
+    @Test
+    fun deleteMultipleStringConstructor() = checkClassWithStringParameter(
+        "net.corda.gradle.HasMultipleStringConstructors",
+        COUNT_INITIAL_MULTIPLE
+    )
+
+    private fun checkClassWithStringParameter(stringClass: String, initialCount: Int) {
+        val stringConstructor = isConstructor(stringClass, String::class)
+
+        classLoaderFor(testProject.sourceJar).use { cl ->
+            cl.load<HasString>(stringClass).apply {
+                getDeclaredConstructor(String::class.java).newInstance(MESSAGE).also {
+                    assertEquals(MESSAGE, it.stringData())
+                }
+                kotlin.constructors.apply {
+                    assertThat("<init>(String) not found", this, hasItem(stringConstructor))
+                    assertEquals(initialCount, this.size)
+                }
+                val primary = kotlin.primaryConstructor ?: throw AssertionError("primary constructor missing")
+                assertThat(primary.call(MESSAGE).stringData()).isEqualTo(MESSAGE)
+
+                newInstance().also {
+                    assertEquals(DEFAULT_MESSAGE, it.stringData())
+                }
+            }
+        }
+
+        classLoaderFor(testProject.filteredJar).use { cl ->
+            cl.load<HasString>(stringClass).apply {
+                getDeclaredConstructor(String::class.java).newInstance(MESSAGE).also {
+                    assertEquals(MESSAGE, it.stringData())
+                }
+                kotlin.constructors.apply {
+                    assertThat("<init>(String) not found", this, hasItem(stringConstructor))
+                    assertEquals(1, this.size)
+                }
+                val primary = kotlin.primaryConstructor ?: throw AssertionError("primary constructor missing")
+                assertThat(primary.call(MESSAGE).stringData()).isEqualTo(MESSAGE)
+
+                assertFailsWith<NoSuchMethodException> { getDeclaredConstructor() }
+            }
+        }
+    }
+}

buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/matcher/JavaMatchers.kt

@@ -13,10 +13,12 @@ fun isMethod(name: Matcher<in String>, returnType: Matcher<in Class<*>>, vararg
 }
 
 fun isMethod(name: String, returnType: Class<*>, vararg parameters: Class<*>): Matcher<Method> {
-    return isMethod(equalTo(name), equalTo(returnType), *parameters.map(::equalTo).toTypedArray())
+    return isMethod(equalTo(name), equalTo(returnType), *parameters.toMatchers())
 }
 
-val <T: Any> KClass<T>.javaDeclaredMethods: List<Method> get() = java.declaredMethods.toList()
+private fun Array<out Class<*>>.toMatchers() = map(::equalTo).toTypedArray()
+
+val KClass<*>.javaDeclaredMethods: List<Method> get() = java.declaredMethods.toList()
 
 /**
  * Matcher logic for a Java [Method] object. Also applicable to constructors.

buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/matcher/KotlinMatchers.kt

@@ -17,7 +17,7 @@ fun isFunction(name: Matcher<in String>, returnType: Matcher<in String>, vararg
 }
 
 fun isFunction(name: String, returnType: KClass<*>, vararg parameters: KClass<*>): Matcher<KFunction<*>> {
-    return isFunction(equalTo(name), matches(returnType), *parameters.map(::hasParam).toTypedArray())
+    return isFunction(equalTo(name), matches(returnType), *parameters.toMatchers())
 }
 
 fun isConstructor(returnType: Matcher<in String>, vararg parameters: Matcher<in KParameter>): Matcher<KFunction<*>> {
@@ -25,11 +25,11 @@ fun isConstructor(returnType: Matcher<in String>, vararg parameters: Matcher<in
 }
 
 fun isConstructor(returnType: KClass<*>, vararg parameters: KClass<*>): Matcher<KFunction<*>> {
-    return isConstructor(matches(returnType), *parameters.map(::hasParam).toTypedArray())
+    return isConstructor(matches(returnType), *parameters.toMatchers())
 }
 
-fun isConstructor(returnType: String, vararg parameters: Matcher<in KParameter>): Matcher<KFunction<*>> {
+fun isConstructor(returnType: String, vararg parameters: KClass<*>): Matcher<KFunction<*>> {
-    return isConstructor(equalTo(returnType), *parameters)
+    return isConstructor(equalTo(returnType), *parameters.toMatchers())
 }
 
 fun hasParam(type: Matcher<in String>): Matcher<KParameter> = KParameterMatcher(type)
@@ -44,6 +44,8 @@ fun isClass(name: String): Matcher<KClass<*>> = KClassMatcher(equalTo(name))
 
 fun matches(type: KClass<*>): Matcher<in String> = equalTo(type.qualifiedName)
 
+private fun Array<out KClass<*>>.toMatchers() = map(::hasParam).toTypedArray()
+
 /**
  * Matcher logic for a Kotlin [KFunction] object. Also applicable to constructors.
  */

buildSrc/jarfilter/src/test/resources/sanitise-constructor/build.gradle

@@ -0,0 +1,34 @@
+plugins {
+    id 'org.jetbrains.kotlin.jvm' version '$kotlin_version'
+    id 'net.corda.plugins.jar-filter'
+}
+apply from: 'repositories.gradle'
+
+sourceSets {
+    main {
+        kotlin {
+            srcDir files(
+                '../resources/test/sanitise-constructor/kotlin',
+                '../resources/test/annotations/kotlin'
+            )
+        }
+    }
+}
+
+dependencies {
+    compile 'org.jetbrains.kotlin:kotlin-stdlib-jdk8'
+    compileOnly files('../../unwanteds/build/libs/unwanteds.jar')
+}
+
+jar {
+    baseName = 'sanitise-constructor'
+}
+
+import net.corda.gradle.jarfilter.JarFilterTask
+task jarFilter(type: JarFilterTask) {
+    jars jar
+    annotations {
+        forDelete = ["net.corda.gradle.jarfilter.DeleteMe"]
+        forSanitise = ["net.corda.gradle.jarfilter.DeleteMe"]
+    }
+}

buildSrc/jarfilter/src/test/resources/sanitise-constructor/kotlin/net/corda/gradle/HasOverloadedConstructors.kt

@@ -0,0 +1,34 @@
+@file:Suppress("UNUSED")
+package net.corda.gradle
+
+import net.corda.gradle.jarfilter.DeleteMe
+import net.corda.gradle.unwanted.*
+
+private const val DEFAULT_MESSAGE = "<default-value>"
+
+class HasOverloadedStringConstructor @JvmOverloads @DeleteMe constructor(private val message: String = DEFAULT_MESSAGE) : HasString {
+    override fun stringData(): String = message
+}
+
+class HasOverloadedLongConstructor @JvmOverloads @DeleteMe constructor(private val data: Long = 0) : HasLong {
+    override fun longData(): Long = data
+}
+
+class HasOverloadedIntConstructor @JvmOverloads @DeleteMe constructor(private val data: Int = 0) : HasInt {
+    override fun intData(): Int = data
+}
+
+class HasMultipleStringConstructors(private val message: String) : HasString {
+    @DeleteMe constructor() : this(DEFAULT_MESSAGE)
+    override fun stringData(): String = message
+}
+
+class HasMultipleLongConstructors(private val data: Long) : HasLong {
+    @DeleteMe constructor() : this(0)
+    override fun longData(): Long = data
+}
+
+class HasMultipleIntConstructors(private val data: Int) : HasInt {
+    @DeleteMe constructor() : this(0)
+    override fun intData(): Int = data
+}

core-deterministic/build.gradle

@@ -103,6 +103,9 @@ task jarFilter(type: JarFilterTask) {
             "co.paralleluniverse.fibers.Suspendable",
             "org.hibernate.annotations.Immutable"
         ]
+        forSanitise = [
+            "net.corda.core.DeleteForDJVM"
+        ]
     }
 }
 

core-deterministic/testing/build.gradle

@@ -14,3 +14,6 @@ dependencies {
     testCompile "org.assertj:assertj-core:$assertj_version"
     testCompile "junit:junit:$junit_version"
 }
+
+// This module has no artifact and only contains tests.
+jar.enabled = false

core-deterministic/testing/src/test/kotlin/net/corda/deterministic/contracts/UniqueIdentifierTest.kt

@@ -0,0 +1,37 @@
+package net.corda.deterministic.contracts
+
+import net.corda.core.contracts.UniqueIdentifier
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.Assert.*
+import org.junit.Test
+import java.util.*
+import kotlin.reflect.full.primaryConstructor
+import kotlin.test.assertFailsWith
+
+class UniqueIdentifierTest {
+    private companion object {
+        private const val NAME = "MyName"
+        private val TEST_UUID: UUID = UUID.fromString("00000000-1111-2222-3333-444444444444")
+    }
+
+    @Test
+    fun testNewInstance() {
+        val id = UniqueIdentifier(NAME, TEST_UUID)
+        assertEquals("${NAME}_$TEST_UUID", id.toString())
+        assertEquals(NAME, id.externalId)
+        assertEquals(TEST_UUID, id.id)
+    }
+
+    @Test
+    fun testPrimaryConstructor() {
+        val primary = UniqueIdentifier::class.primaryConstructor ?: throw AssertionError("primary constructor missing")
+        assertThat(primary.call(NAME, TEST_UUID)).isEqualTo(UniqueIdentifier(NAME, TEST_UUID))
+    }
+
+    @Test
+    fun testConstructors() {
+        assertEquals(1, UniqueIdentifier::class.constructors.size)
+        val ex = assertFailsWith<IllegalArgumentException> { UniqueIdentifier::class.constructors.first().call() }
+        assertThat(ex).hasMessage("Callable expects 2 arguments, but 0 were provided.")
+    }
+}

core/src/main/kotlin/net/corda/core/contracts/UniqueIdentifier.kt

@@ -20,10 +20,7 @@ import java.util.*
  */
 @CordaSerializable
 @KeepForDJVM
-data class UniqueIdentifier(val externalId: String?, val id: UUID) : Comparable<UniqueIdentifier> {
+data class UniqueIdentifier @JvmOverloads @DeleteForDJVM constructor(val externalId: String? = null, val id: UUID = UUID.randomUUID()) : Comparable<UniqueIdentifier> {
-    @DeleteForDJVM constructor(externalId: String?) : this(externalId, UUID.randomUUID())
-    @DeleteForDJVM constructor() : this(null)
-
     override fun toString(): String = if (externalId != null) "${externalId}_$id" else id.toString()
 
     companion object {

docs/source/deterministic-modules.rst

@@ -62,15 +62,16 @@ The build generates each of Corda's deterministic JARs in six steps:
 
         @CordaSerializable
         @KeepForDJVM
-        data class UniqueIdentifier(val externalId: String?, val id: UUID) : Comparable<UniqueIdentifier> {
+        data class UniqueIdentifier @JvmOverloads @DeleteForDJVM constructor(
-            @DeleteForDJVM constructor(externalId: String?) : this(externalId, UUID.randomUUID())
+            val externalId: String? = null,
-            @DeleteForDJVM constructor() : this(null)
+            val id: UUID = UUID.randomUUID()
+        ) : Comparable<UniqueIdentifier> {
             ...
         }
 
     ..
 
-    While CorDapps will definitely need to handle ``UniqueIdentifier`` objects, both of the secondary constructors
+    While CorDapps will definitely need to handle ``UniqueIdentifier`` objects, all of the secondary constructors
     generate a new random ``UUID`` and so are non-deterministic. Hence the next "determinising" step is to pass the
     classes to the ``JarFilter`` tool, which strips out all of the elements which have been annotated as
     ``@DeleteForDJVM`` and stubs out any functions annotated with ``@StubOutForDJVM``. (Stub functions that
@@ -270,11 +271,34 @@ Non-Deterministic Elements
     ..
 
     You must also ensure that a deterministic class's primary constructor does not reference any classes that are
-    not available in the deterministic ``rt.jar``, nor have any non-deterministic default parameter values such as
+    not available in the deterministic ``rt.jar``. The biggest risk here would be that ``JarFilter`` would delete the
-    ``UUID.randomUUID()``. The biggest risk here would be that ``JarFilter`` would delete the primary constructor
+    primary constructor and that the class could no longer be instantiated, although ``JarFilter`` will print a warning
-    and that the class could no longer be instantiated, although ``JarFilter`` will print a warning in this case.
+    in this case. However, it is also likely that the "determinised" class would have a different serialisation
-    However, it is also likely that the "determinised" class would have a different serialisation signature than
+    signature than its non-deterministic version and so become unserialisable on the deterministic JVM.
-    its non-deterministic version and so become unserialisable on the deterministic JVM.
+
+    Primary constructors that have non-deterministic default parameter values must still be annotated as
+    ``@DeleteForDJVM`` because they cannot be refactored without breaking Corda's binary interface. The Kotlin compiler
+    will automatically apply this ``@DeleteForDJVM`` annotation - along with any others - to all of the class's
+    secondary constructors too. The ``JarFilter`` plugin can then remove the ``@DeleteForDJVM`` annotation from the
+    primary constructor so that it can subsequently delete only the secondary constructors.
+
+    The annotations that ``JarFilter`` will "sanitise" from primary constructors in this way are listed in the plugin's
+    configuration block, e.g.
+
+    .. sourcecode:: groovy
+
+        task jarFilter(type: JarFilterTask) {
+            ...
+            annotations {
+                ...
+
+                forSanitise = [
+                    "net.corda.core.DeleteForDJVM"
+                ]
+            }
+        }
+
+    ..
 
     Be aware that package-scoped Kotlin properties are all initialised within a common ``<clinit>`` block inside
     their host ``.class`` file. This means that when ``JarFilter`` deletes these properties, it cannot also remove

serialization-deterministic/build.gradle

@@ -95,6 +95,9 @@ task jarFilter(type: JarFilterTask) {
         forRemove = [
             "co.paralleluniverse.fibers.Suspendable"
         ]
+        forSanitise = [
+            "net.corda.core.DeleteForDJVM"
+        ]
     }
 }