From d672cba8777e754622e8fb659f5d164b2e9a67bc Mon Sep 17 00:00:00 2001 From: Chris Rankin Date: Mon, 16 Jul 2018 12:03:00 +0100 Subject: [PATCH] CORDA-1785: Update JarFilter to delete classes whose outer class or enclosing method is removed. (#3562) --- .../net/corda/gradle/jarfilter/Elements.kt | 6 +- .../gradle/jarfilter/FilterTransformer.kt | 25 +++--- .../corda/gradle/jarfilter/JarFilterTask.kt | 4 +- .../corda/gradle/jarfilter/UnwantedCache.kt | 46 ++++++++++ .../gradle/jarfilter/DeleteInnerLambdaTest.kt | 84 +++++++++++++++++++ .../gradle/jarfilter/FieldRemovalTest.kt | 2 +- .../jarfilter/StaticFieldRemovalTest.kt | 2 +- .../gradle/jarfilter/UnwantedCacheTest.kt | 54 ++++++++++++ .../delete-inner-lambda/build.gradle | 33 ++++++++ .../kotlin/net/corda/gradle/HasInnerLambda.kt | 19 +++++ 10 files changed, 257 insertions(+), 18 deletions(-) create mode 100644 buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/UnwantedCache.kt create mode 100644 buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/DeleteInnerLambdaTest.kt create mode 100644 buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/UnwantedCacheTest.kt create mode 100644 buildSrc/jarfilter/src/test/resources/delete-inner-lambda/build.gradle create mode 100644 buildSrc/jarfilter/src/test/resources/delete-inner-lambda/kotlin/net/corda/gradle/HasInnerLambda.kt diff --git a/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/Elements.kt b/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/Elements.kt index 34da8fbba5..17810136a8 100644 --- a/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/Elements.kt +++ b/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/Elements.kt @@ -18,14 +18,14 @@ private const val DUMMY_PASSES = 1 private val DECLARES_DEFAULT_VALUE_MASK: Int = DECLARES_DEFAULT_VALUE.toFlags(true).inv() -internal abstract class Element(val name: String, val descriptor: String) { +abstract class Element(val name: String, val descriptor: String) { private var lifetime: Int = DUMMY_PASSES open val isExpired: Boolean get() = --lifetime < 0 } -internal class MethodElement(name: String, descriptor: String, val access: Int = 0) : Element(name, descriptor) { +class MethodElement(name: String, descriptor: String, val access: Int = 0) : Element(name, descriptor) { override fun equals(other: Any?): Boolean { if (this === other) return true if (other?.javaClass != javaClass) return false @@ -66,7 +66,7 @@ internal class MethodElement(name: String, descriptor: String, val access: Int = * A class cannot have two fields with the same name but different types. However, * it can define extension functions and properties. */ -internal class FieldElement(name: String, descriptor: String = "?", val extension: String = "()") : Element(name, descriptor) { +class FieldElement(name: String, descriptor: String = "?", val extension: String = "()") : Element(name, descriptor) { override fun equals(other: Any?): Boolean { if (this === other) return true if (other?.javaClass != javaClass) return false diff --git a/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/FilterTransformer.kt b/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/FilterTransformer.kt index e6e2089381..164f3bf5d7 100644 --- a/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/FilterTransformer.kt +++ b/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/FilterTransformer.kt @@ -22,7 +22,7 @@ class FilterTransformer private constructor ( private val removeAnnotations: Set, private val deleteAnnotations: Set, private val stubAnnotations: Set, - private val unwantedClasses: MutableSet, + private val unwantedElements: UnwantedCache, private val unwantedFields: MutableSet, private val deletedMethods: MutableSet, private val stubbedMethods: MutableSet @@ -33,7 +33,7 @@ class FilterTransformer private constructor ( removeAnnotations: Set, deleteAnnotations: Set, stubAnnotations: Set, - unwantedClasses: MutableSet + unwantedElements: UnwantedCache ) : this( visitor = visitor, logger = logger, @@ -41,7 +41,7 @@ class FilterTransformer private constructor ( removeAnnotations = removeAnnotations, deleteAnnotations = deleteAnnotations, stubAnnotations = stubAnnotations, - unwantedClasses = unwantedClasses, + unwantedElements = unwantedElements, unwantedFields = mutableSetOf(), deletedMethods = mutableSetOf(), stubbedMethods = mutableSetOf() @@ -57,7 +57,7 @@ class FilterTransformer private constructor ( || stubbedMethods.isNotEmpty() || super.hasUnwantedElements - private fun isUnwantedClass(name: String): Boolean = unwantedClasses.contains(name) + private fun isUnwantedClass(name: String): Boolean = unwantedElements.containsClass(name) private fun hasDeletedSyntheticMethod(name: String): Boolean = deletedMethods.any { method -> name.startsWith("$className\$${method.visibleName}\$") } @@ -69,7 +69,7 @@ class FilterTransformer private constructor ( removeAnnotations = removeAnnotations, deleteAnnotations = deleteAnnotations, stubAnnotations = stubAnnotations, - unwantedClasses = unwantedClasses, + unwantedElements = unwantedElements, unwantedFields = unwantedFields, deletedMethods = deletedMethods, stubbedMethods = stubbedMethods @@ -86,7 +86,7 @@ class FilterTransformer private constructor ( logger.info("- Removing annotation {}", descriptor) return null } else if (deleteAnnotations.contains(descriptor)) { - if (unwantedClasses.add(className)) { + if (unwantedElements.addClass(className)) { logger.info("- Identified class {} as unwanted", className) } } @@ -110,6 +110,7 @@ class FilterTransformer private constructor ( logger.debug("--- method ---> {}", method) if (deletedMethods.contains(method)) { logger.info("- Deleted method {}{}", method.name, method.descriptor) + unwantedElements.addMethod(className, method) deletedMethods.remove(method) return null } @@ -131,7 +132,7 @@ class FilterTransformer private constructor ( override fun visitInnerClass(clsName: String, outerName: String?, innerName: String?, access: Int) { logger.debug("--- inner class {} [outer: {}, inner: {}]", clsName, outerName, innerName) if (isUnwantedClass || hasDeletedSyntheticMethod(clsName)) { - if (unwantedClasses.add(clsName)) { + if (unwantedElements.addClass(clsName)) { logger.info("- Deleted inner class {}", clsName) } } else if (isUnwantedClass(clsName)) { @@ -143,8 +144,10 @@ class FilterTransformer private constructor ( override fun visitOuterClass(outerName: String, methodName: String?, methodDescriptor: String?) { logger.debug("--- outer class {} [enclosing method {},{}]", outerName, methodName, methodDescriptor) - if (isUnwantedClass(outerName)) { - logger.info("- Deleted reference to outer class {}", outerName) + if (unwantedElements.containsMethod(outerName, methodName, methodDescriptor)) { + if (unwantedElements.addClass(className)) { + logger.info("- Identified class {} as unwanted by its outer class", className) + } } else { super.visitOuterClass(outerName, methodName, methodDescriptor) } @@ -180,8 +183,8 @@ class FilterTransformer private constructor ( deletedFields = unwantedFields, deletedFunctions = partitioned[false] ?: emptyList(), deletedConstructors = partitioned[true] ?: emptyList(), - deletedNestedClasses = unwantedClasses.filter { it.startsWith(prefix) }.map { it.drop(prefix.length) }, - deletedClasses = unwantedClasses, + deletedNestedClasses = unwantedElements.classes.filter { it.startsWith(prefix) }.map { it.drop(prefix.length) }, + deletedClasses = unwantedElements.classes, handleExtraMethod = ::delete, d1 = d1, d2 = d2) diff --git a/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/JarFilterTask.kt b/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/JarFilterTask.kt index 1d0c99f4cc..78100b2613 100644 --- a/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/JarFilterTask.kt +++ b/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/JarFilterTask.kt @@ -138,7 +138,7 @@ open class JarFilterTask : DefaultTask() { } private inner class Filter(inFile: File) { - private val unwantedClasses: MutableSet = mutableSetOf() + private val unwantedElements = UnwantedCache() private val source: Path = inFile.toPath() private val target: Path = toFiltered(inFile).toPath() @@ -251,7 +251,7 @@ open class JarFilterTask : DefaultTask() { removeAnnotations = descriptorsForRemove, deleteAnnotations = descriptorsForDelete, stubAnnotations = descriptorsForStub, - unwantedClasses = unwantedClasses + unwantedElements = unwantedElements ) /* diff --git a/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/UnwantedCache.kt b/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/UnwantedCache.kt new file mode 100644 index 0000000000..5a8d28bc73 --- /dev/null +++ b/buildSrc/jarfilter/src/main/kotlin/net/corda/gradle/jarfilter/UnwantedCache.kt @@ -0,0 +1,46 @@ +package net.corda.gradle.jarfilter + +import java.util.Collections.unmodifiableMap + +/** + * A persistent cache of all of the classes and methods that JarFilter has + * removed. This cache belongs to the Gradle task itself and so is shared + * by successive filter passes. + * + * The internal method cache is only required for those classes which are + * being kept. When an entire class is declared as "unwanted", any entry + * it may have in the method cache is removed. + */ +class UnwantedCache { + private val _classes: MutableSet = mutableSetOf() + private val _classMethods: MutableMap> = mutableMapOf() + + val classes: Set get() = _classes + val classMethods: Map> get() = unmodifiableMap(_classMethods) + + fun containsClass(className: String): Boolean = _classes.contains(className) + + fun addClass(className: String): Boolean { + return _classes.add(className).also { isAdded -> + if (isAdded) { + _classMethods.remove(className) + } + } + } + + fun addMethod(className: String, method: MethodElement) { + if (!containsClass(className)) { + _classMethods.getOrPut(className) { mutableSetOf() }.add(method) + } + } + + private fun containsMethod(className: String, method: MethodElement): Boolean { + return _classMethods[className]?.contains(method) ?: false + } + + fun containsMethod(className: String, methodName: String?, methodDescriptor: String?): Boolean { + return containsClass(className) || + (methodName != null && methodDescriptor != null && containsMethod(className, MethodElement(methodName, methodDescriptor))) + } +} + diff --git a/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/DeleteInnerLambdaTest.kt b/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/DeleteInnerLambdaTest.kt new file mode 100644 index 0000000000..14f1f4edc9 --- /dev/null +++ b/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/DeleteInnerLambdaTest.kt @@ -0,0 +1,84 @@ +package net.corda.gradle.jarfilter + +import net.corda.gradle.jarfilter.matcher.isConstructor +import net.corda.gradle.unwanted.HasInt +import org.assertj.core.api.Assertions.assertThat +import org.hamcrest.core.IsCollectionContaining.* +import org.hamcrest.core.IsNot.* +import org.junit.Assert.* +import org.junit.BeforeClass +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.test.assertFailsWith + +class DeleteInnerLambdaTest { + companion object { + private const val LAMBDA_CLASS = "net.corda.gradle.HasInnerLambda" + private const val SIZE = 64 + + private val testProjectDir = TemporaryFolder() + private val testProject = JarFilterProject(testProjectDir, "delete-inner-lambda") + private val constructInt = isConstructor(LAMBDA_CLASS, Int::class) + private val constructBytes = isConstructor(LAMBDA_CLASS, ByteArray::class) + + private lateinit var sourceClasses: List + private lateinit var filteredClasses: List + + @ClassRule + @JvmField + val rules: TestRule = RuleChain + .outerRule(testProjectDir) + .around(testProject) + + @BeforeClass + @JvmStatic + fun setup() { + sourceClasses = testProject.sourceJar.getClassNames(LAMBDA_CLASS) + filteredClasses = testProject.filteredJar.getClassNames(LAMBDA_CLASS) + } + } + + @Test + fun `test lambda class is deleted`() { + assertThat(sourceClasses) + .contains(LAMBDA_CLASS) + .hasSize(2) + assertThat(filteredClasses).containsExactly(LAMBDA_CLASS) + } + + @Test + fun `test host class`() { + classLoaderFor(testProject.sourceJar).use { cl -> + cl.load(LAMBDA_CLASS).apply { + getConstructor(Int::class.java).newInstance(SIZE).also { obj -> + assertEquals(SIZE, obj.intData()) + } + kotlin.constructors.also { ctors -> + assertThat("(Int) not found", ctors, hasItem(constructInt)) + assertThat("(byte[]) not found", ctors, hasItem(constructBytes)) + } + + getConstructor(ByteArray::class.java).newInstance(ByteArray(SIZE)).also { obj -> + assertEquals(SIZE, obj.intData()) + } + } + } + + classLoaderFor(testProject.filteredJar).use { cl -> + cl.load(LAMBDA_CLASS).apply { + assertFailsWith { getConstructor(Int::class.java) } + kotlin.constructors.also { ctors -> + assertThat("(Int) still exists", ctors, not(hasItem(constructInt))) + assertThat("(byte[]) not found", ctors, hasItem(constructBytes)) + } + + getConstructor(ByteArray::class.java).newInstance(ByteArray(SIZE)).also { obj -> + assertEquals(SIZE, obj.intData()) + } + } + } + } +} \ No newline at end of file diff --git a/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/FieldRemovalTest.kt b/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/FieldRemovalTest.kt index a6cfdb6a8c..9e99d4e470 100644 --- a/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/FieldRemovalTest.kt +++ b/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/FieldRemovalTest.kt @@ -52,7 +52,7 @@ class FieldRemovalTest { removeAnnotations = emptySet(), deleteAnnotations = setOf(Deletable::class.jvmName.descriptor), stubAnnotations = emptySet(), - unwantedClasses = mutableSetOf() + unwantedElements = UnwantedCache() ) }, COMPUTE_MAXS) return bytecode.toClass(type, asType) diff --git a/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/StaticFieldRemovalTest.kt b/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/StaticFieldRemovalTest.kt index 83ef0258ba..e174ed147a 100644 --- a/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/StaticFieldRemovalTest.kt +++ b/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/StaticFieldRemovalTest.kt @@ -34,7 +34,7 @@ class StaticFieldRemovalTest { removeAnnotations = emptySet(), deleteAnnotations = setOf(Deletable::class.jvmName.descriptor), stubAnnotations = emptySet(), - unwantedClasses = mutableSetOf() + unwantedElements = UnwantedCache() ) }, COMPUTE_MAXS) return bytecode.toClass(type, asType) diff --git a/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/UnwantedCacheTest.kt b/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/UnwantedCacheTest.kt new file mode 100644 index 0000000000..53abf1de1b --- /dev/null +++ b/buildSrc/jarfilter/src/test/kotlin/net/corda/gradle/jarfilter/UnwantedCacheTest.kt @@ -0,0 +1,54 @@ +package net.corda.gradle.jarfilter + +import org.junit.Assert.* +import org.junit.Before +import org.junit.Test + +class UnwantedCacheTest { + private companion object { + private const val CLASS_NAME = "org.testing.MyClass" + private const val LONG_ARG = "(J)V" + private const val NO_ARG = "()V" + } + + private lateinit var cache: UnwantedCache + + @Before + fun setup() { + cache = UnwantedCache() + } + + @Test + fun testEmptyCache() { + assertFalse(cache.containsClass(CLASS_NAME)) + assertFalse(cache.containsMethod(CLASS_NAME, null, null)) + assertFalse(cache.containsMethod(CLASS_NAME, "", NO_ARG)) + } + + @Test + fun testAddingClass() { + cache.addClass(CLASS_NAME) + assertTrue(cache.containsClass(CLASS_NAME)) + assertTrue(cache.containsMethod(CLASS_NAME, null, null)) + assertTrue(cache.containsMethod(CLASS_NAME, "", NO_ARG)) + } + + @Test + fun testAddingMethod() { + cache.addMethod(CLASS_NAME, MethodElement("", LONG_ARG)) + assertTrue(cache.containsMethod(CLASS_NAME, "", LONG_ARG)) + assertFalse(cache.containsMethod(CLASS_NAME, "", NO_ARG)) + assertFalse(cache.containsMethod(CLASS_NAME, "destroy", LONG_ARG)) + assertFalse(cache.containsMethod(CLASS_NAME, null, null)) + assertFalse(cache.containsMethod(CLASS_NAME, "nonsense", null)) + assertFalse(cache.containsClass(CLASS_NAME)) + } + + @Test + fun testAddingMethodFollowedByClass() { + cache.addMethod(CLASS_NAME, MethodElement("", LONG_ARG)) + cache.addClass(CLASS_NAME) + assertTrue(cache.containsMethod(CLASS_NAME, "", LONG_ARG)) + assertEquals(0, cache.classMethods.size) + } +} \ No newline at end of file diff --git a/buildSrc/jarfilter/src/test/resources/delete-inner-lambda/build.gradle b/buildSrc/jarfilter/src/test/resources/delete-inner-lambda/build.gradle new file mode 100644 index 0000000000..f5725b6e52 --- /dev/null +++ b/buildSrc/jarfilter/src/test/resources/delete-inner-lambda/build.gradle @@ -0,0 +1,33 @@ +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/delete-inner-lambda/kotlin', + '../resources/test/annotations/kotlin' + ) + } + } +} + +dependencies { + compile 'org.jetbrains.kotlin:kotlin-stdlib-jdk8' + compileOnly files('../../unwanteds/build/libs/unwanteds.jar') +} + +jar { + baseName = 'delete-inner-lambda' +} + +import net.corda.gradle.jarfilter.JarFilterTask +task jarFilter(type: JarFilterTask) { + jars jar + annotations { + forDelete = ["net.corda.gradle.jarfilter.DeleteMe"] + } +} diff --git a/buildSrc/jarfilter/src/test/resources/delete-inner-lambda/kotlin/net/corda/gradle/HasInnerLambda.kt b/buildSrc/jarfilter/src/test/resources/delete-inner-lambda/kotlin/net/corda/gradle/HasInnerLambda.kt new file mode 100644 index 0000000000..774ebb80d6 --- /dev/null +++ b/buildSrc/jarfilter/src/test/resources/delete-inner-lambda/kotlin/net/corda/gradle/HasInnerLambda.kt @@ -0,0 +1,19 @@ +@file:Suppress("UNUSED") +package net.corda.gradle + +import net.corda.gradle.jarfilter.DeleteMe +import net.corda.gradle.unwanted.HasInt + +class HasInnerLambda(private val bytes: ByteArray) : HasInt { + @DeleteMe + constructor(size: Int) : this(ZeroArray { size }.bytes) + + override fun intData() = bytes.size +} + +/** + * Do NOT inline this lambda! + */ +class ZeroArray(initialSize: () -> Int) { + val bytes: ByteArray = ByteArray(initialSize()) { 0 } +}