From d9ceddcbdd1d7f6e6cad291bb1a3d7bdb512ab68 Mon Sep 17 00:00:00 2001 From: Adam <897017+aSemy@users.noreply.github.com> Date: Mon, 16 Jan 2023 00:49:45 +0100 Subject: [PATCH 1/4] de-duplicate ProxyMaker Note that the git history of one of the ProxyMaker files won't be reflected in the surviving class * rename JvmSubclassInstrumentation to avoid name class with SubclassInstrumentation * update JvmSubclassInstrumentation to implement SubclassInstrumentation * remove unused method setProxyHandler() from SubclassInstrumentation --- .../AndroidSubclassInstrumentation.kt | 9 +- .../transformation/SubclassInstrumentation.kt | 4 - .../transformation/TransformationRequest.kt | 4 +- .../io/mockk/proxy/common/ProxyMaker.kt | 56 +++-- .../mockk/proxy/jvm/JvmMockKAgentFactory.kt | 10 +- .../kotlin/io/mockk/proxy/jvm/ProxyMaker.kt | 235 ------------------ ...ation.kt => JvmSubclassInstrumentation.kt} | 8 +- 7 files changed, 41 insertions(+), 285 deletions(-) delete mode 100644 modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/ProxyMaker.kt rename modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/transformation/{SubclassInstrumentation.kt => JvmSubclassInstrumentation.kt} (95%) diff --git a/modules/mockk-agent-android/src/main/kotlin/io/mockk/proxy/android/transformation/AndroidSubclassInstrumentation.kt b/modules/mockk-agent-android/src/main/kotlin/io/mockk/proxy/android/transformation/AndroidSubclassInstrumentation.kt index ba0cc8579..efa4eedb3 100644 --- a/modules/mockk-agent-android/src/main/kotlin/io/mockk/proxy/android/transformation/AndroidSubclassInstrumentation.kt +++ b/modules/mockk-agent-android/src/main/kotlin/io/mockk/proxy/android/transformation/AndroidSubclassInstrumentation.kt @@ -74,11 +74,4 @@ internal class AndroidSubclassInstrumentation( return abstractMethods.map { it.originalMethod }.toTypedArray() } - - override fun setProxyHandler(proxy: Any, handler: MockKInvocationHandler) { - if (ProxyBuilder.isProxyClass(proxy::class.java)) { - ProxyBuilder.setInvocationHandler(proxy, ProxyInvocationHandler(handler)) - } - } - -} \ No newline at end of file +} diff --git a/modules/mockk-agent-api/src/commonMain/kotlin/io/mockk/proxy/common/transformation/SubclassInstrumentation.kt b/modules/mockk-agent-api/src/commonMain/kotlin/io/mockk/proxy/common/transformation/SubclassInstrumentation.kt index eca1c3ae5..7341e17a9 100644 --- a/modules/mockk-agent-api/src/commonMain/kotlin/io/mockk/proxy/common/transformation/SubclassInstrumentation.kt +++ b/modules/mockk-agent-api/src/commonMain/kotlin/io/mockk/proxy/common/transformation/SubclassInstrumentation.kt @@ -1,12 +1,8 @@ package io.mockk.proxy.common.transformation -import io.mockk.proxy.MockKInvocationHandler - interface SubclassInstrumentation { fun subclass( clazz: Class, interfaces: Array> ): Class - - fun setProxyHandler(proxy: Any, handler: MockKInvocationHandler) } diff --git a/modules/mockk-agent-api/src/commonMain/kotlin/io/mockk/proxy/common/transformation/TransformationRequest.kt b/modules/mockk-agent-api/src/commonMain/kotlin/io/mockk/proxy/common/transformation/TransformationRequest.kt index 154e961ce..ed61cd435 100644 --- a/modules/mockk-agent-api/src/commonMain/kotlin/io/mockk/proxy/common/transformation/TransformationRequest.kt +++ b/modules/mockk-agent-api/src/commonMain/kotlin/io/mockk/proxy/common/transformation/TransformationRequest.kt @@ -10,6 +10,4 @@ data class TransformationRequest( type, !untransform ) - - -} \ No newline at end of file +} diff --git a/modules/mockk-agent-api/src/jvmMain/kotlin/io/mockk/proxy/common/ProxyMaker.kt b/modules/mockk-agent-api/src/jvmMain/kotlin/io/mockk/proxy/common/ProxyMaker.kt index 6de9e583e..edd2dc79e 100644 --- a/modules/mockk-agent-api/src/jvmMain/kotlin/io/mockk/proxy/common/ProxyMaker.kt +++ b/modules/mockk-agent-api/src/jvmMain/kotlin/io/mockk/proxy/common/ProxyMaker.kt @@ -13,19 +13,16 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package io.mockk.proxy.common import io.mockk.proxy.* import io.mockk.proxy.common.transformation.InlineInstrumentation -import io.mockk.proxy.common.transformation.SubclassInstrumentation import io.mockk.proxy.common.transformation.TransformationRequest +import io.mockk.proxy.common.transformation.SubclassInstrumentation import io.mockk.proxy.common.transformation.TransformationType -import java.lang.ref.SoftReference import java.lang.ref.WeakReference import java.lang.reflect.Method import java.lang.reflect.Modifier -import java.lang.reflect.Proxy class ProxyMaker( private val log: MockKAgentLogger, @@ -34,6 +31,7 @@ class ProxyMaker( private val instantiator: MockKInstantiatior, private val handlers: MutableMap ) : MockKProxyMaker { + override fun proxy( clazz: Class, interfaces: Array>, @@ -44,31 +42,24 @@ class ProxyMaker( throwIfNotPossibleToProxy(clazz, interfaces) - if (clazz.isInterface) { - val proxyInstance = Proxy.newProxyInstance( - clazz.classLoader, - interfaces + clazz, - ProxyInvocationHandler(handler) - ) + // Sometimes (e.g. in case of sealed classes) we will create the proxy for a subclass of `clazz` and not `clazz` + // itself. We need to determine this early, so that the subclass will be inlined as well. + val actualClass = findActualClassToBeProxied(clazz) - return CancelableResult(clazz.cast(proxyInstance)) - } - - val cancellation = inline(clazz) + val cancellation = inline(actualClass) val result = CancelableResult(cancelBlock = cancellation) val proxyClass = try { - subclass(clazz, interfaces) + subclass(actualClass, interfaces) } catch (ex: Exception) { result.cancel() - throw MockKAgentException("Failed to subclass $clazz", ex) + throw MockKAgentException("Failed to subclass $actualClass", ex) } try { - val proxy = instantiate(clazz, proxyClass, useDefaultConstructor, instance) + val proxy = instantiate(actualClass, proxyClass, useDefaultConstructor, instance) - subclasser.setProxyHandler(proxy, handler) handlers[proxy] = handler val callbackRef = WeakReference(proxy) return result @@ -77,11 +68,9 @@ class ProxyMaker( callbackRef.get()?.let { handlers.remove(it) } - } } catch (e: Exception) { result.cancel() - throw MockKAgentException("Instantiation exception", e) } } @@ -114,11 +103,7 @@ class ProxyMaker( val superclasses = getAllSuperclasses(clazz) return if (inliner != null) { - val transformRequest = - TransformationRequest( - superclasses, - TransformationType.SIMPLE - ) + val transformRequest = TransformationRequest(superclasses, TransformationType.SIMPLE) inliner.execute(transformRequest) } else { @@ -130,6 +115,21 @@ class ProxyMaker( } } + private fun findActualClassToBeProxied( + clazz: Class, + ): Class { + val kClass = clazz.kotlin + if (!kClass.isSealed) { + return clazz + } + + val subclass = kClass.sealedSubclasses.firstOrNull()?.java + ?: error("Unable to create proxy for sealed class $clazz, no subclasses available") + log.trace("Class $clazz is sealed, will use its subclass $subclass to build proxy") + @Suppress("UNCHECKED_CAST") + return findActualClassToBeProxied(subclass) as Class + } + private fun subclass( clazz: Class, interfaces: Array> @@ -137,6 +137,9 @@ class ProxyMaker( return if (Modifier.isFinal(clazz.modifiers)) { log.trace("Taking instance of $clazz itself because it is final.") clazz + } else if (interfaces.isEmpty() && !Modifier.isAbstract(clazz.modifiers) && inliner != null) { + log.trace("Taking instance of $clazz itself because it is not abstract and no additional interfaces specified.") + clazz } else { log.trace( "Building subclass proxy for $clazz with " + @@ -175,7 +178,7 @@ class ProxyMaker( val defaultConstructor = cls.getDeclaredConstructor() try { defaultConstructor.isAccessible = true - } catch (ex: Exception) { + } catch (ignored: Exception) { // skip } @@ -200,6 +203,7 @@ class ProxyMaker( companion object { + private val notMockableClasses = setOf( Class::class.java, Boolean::class.java, diff --git a/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/JvmMockKAgentFactory.kt b/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/JvmMockKAgentFactory.kt index 56e7ec537..a7834716f 100644 --- a/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/JvmMockKAgentFactory.kt +++ b/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/JvmMockKAgentFactory.kt @@ -6,7 +6,7 @@ import io.mockk.proxy.jvm.advice.jvm.MockHandlerMap import io.mockk.proxy.jvm.dispatcher.BootJarLoader import io.mockk.proxy.jvm.transformation.InliningClassTransformer import io.mockk.proxy.jvm.transformation.JvmInlineInstrumentation -import io.mockk.proxy.jvm.transformation.SubclassInstrumentation +import io.mockk.proxy.jvm.transformation.JvmSubclassInstrumentation import net.bytebuddy.ByteBuddy import net.bytebuddy.NamingStrategy import net.bytebuddy.agent.ByteBuddyAgent @@ -91,15 +91,15 @@ class JvmMockKAgentFactory : MockKAgentFactory { ) } - val subclasser = SubclassInstrumentation( - logFactory.logger(SubclassInstrumentation::class.java), + val subclasser = JvmSubclassInstrumentation( + logFactory.logger(JvmSubclassInstrumentation::class.java), handlers, byteBuddy ) - jvmProxyMaker = ProxyMaker( - logFactory.logger(ProxyMaker::class.java), + jvmProxyMaker = io.mockk.proxy.common.ProxyMaker( + logFactory.logger(io.mockk.proxy.common.ProxyMaker::class.java), inliner, subclasser, jvmInstantiator, diff --git a/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/ProxyMaker.kt b/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/ProxyMaker.kt deleted file mode 100644 index dbee2f969..000000000 --- a/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/ProxyMaker.kt +++ /dev/null @@ -1,235 +0,0 @@ -package io.mockk.proxy.jvm - -import io.mockk.proxy.* -import io.mockk.proxy.common.CancelableResult -import io.mockk.proxy.common.transformation.InlineInstrumentation -import io.mockk.proxy.common.transformation.TransformationRequest -import io.mockk.proxy.common.transformation.TransformationType.SIMPLE -import io.mockk.proxy.jvm.transformation.SubclassInstrumentation -import java.lang.ref.SoftReference -import java.lang.ref.WeakReference -import java.lang.reflect.Method -import java.lang.reflect.Modifier - -internal class ProxyMaker( - private val log: MockKAgentLogger, - private val inliner: InlineInstrumentation?, - private val subclasser: SubclassInstrumentation, - private val instantiator: MockKInstantiatior, - private val handlers: MutableMap -) : MockKProxyMaker { - - override fun proxy( - clazz: Class, - interfaces: Array>, - handler: MockKInvocationHandler, - useDefaultConstructor: Boolean, - instance: Any? - ): Cancelable { - - throwIfNotPossibleToProxy(clazz, interfaces) - - // Sometimes (e.g. in case of sealed classes) we will create the proxy for a subclass of `clazz` and not `clazz` - // itself. We need to determine this early, so that the subclass will be inlined as well. - val actualClass = findActualClassToBeProxied(clazz) - - val cancellation = inline(actualClass) - - val result = CancelableResult(cancelBlock = cancellation) - - val proxyClass = try { - subclass(actualClass, interfaces) - } catch (ex: Exception) { - result.cancel() - throw MockKAgentException("Failed to subclass $actualClass", ex) - } - - try { - val proxy = instantiate(actualClass, proxyClass, useDefaultConstructor, instance) - - handlers[proxy] = handler - val callbackRef = WeakReference(proxy) - return result - .withValue(proxy) - .alsoOnCancel { - callbackRef.get()?.let { - handlers.remove(it) - } - } - } catch (e: Exception) { - result.cancel() - throw MockKAgentException("Instantiation exception", e) - } - } - - private fun instantiate( - clazz: Class, - proxyClass: Class, - useDefaultConstructor: Boolean, - instance: Any? - ): T { - return when { - instance != null -> { - log.trace("Attaching to object mock for $clazz") - clazz.cast(instance) - } - useDefaultConstructor -> { - log.trace("Instantiating proxy for $clazz via default constructor") - clazz.cast(newInstanceViaDefaultConstructor(proxyClass)) - } - else -> { - log.trace("Instantiating proxy for $clazz via instantiator") - instantiator.instance(proxyClass) - } - } - } - - private fun inline( - clazz: Class - ): () -> Unit { - val superclasses = getAllSuperclasses(clazz) - - return if (inliner != null) { - val transformRequest = TransformationRequest(superclasses, SIMPLE) - - inliner.execute(transformRequest) - } else { - if (!Modifier.isFinal(clazz.modifiers)) { - warnOnFinalMethods(clazz) - } - - {} - } - } - - private fun findActualClassToBeProxied( - clazz: Class, - ): Class { - val kClass = clazz.kotlin - if (!kClass.isSealed) { - return clazz - } - - val subclass = kClass.sealedSubclasses.firstOrNull()?.java - ?: error("Unable to create proxy for sealed class $clazz, no subclasses available") - log.trace("Class $clazz is sealed, will use its subclass $subclass to build proxy") - @Suppress("UNCHECKED_CAST") - return findActualClassToBeProxied(subclass) as Class - } - - private fun subclass( - clazz: Class, - interfaces: Array> - ): Class { - return if (Modifier.isFinal(clazz.modifiers)) { - log.trace("Taking instance of $clazz itself because it is final.") - clazz - } else if (interfaces.isEmpty() && !Modifier.isAbstract(clazz.modifiers) && inliner != null) { - log.trace("Taking instance of $clazz itself because it is not abstract and no additional interfaces specified.") - clazz - } else { - log.trace( - "Building subclass proxy for $clazz with " + - "additional interfaces ${interfaces.toList()}" - ) - subclasser.subclass(clazz, interfaces) - } - } - - private fun throwIfNotPossibleToProxy( - clazz: Class, - interfaces: Array> - ) { - when { - clazz.isPrimitive -> - throw MockKAgentException( - "Failed to create proxy for $clazz.\n$clazz is a primitive" - ) - clazz.isArray -> - throw MockKAgentException( - "Failed to create proxy for $clazz.\n$clazz is an array" - ) - clazz as Class<*> in notMockableClasses -> - throw MockKAgentException( - "Failed to create proxy for $clazz.\n$clazz is one of excluded classes" - ) - interfaces.isNotEmpty() && Modifier.isFinal(clazz.modifiers) -> - throw MockKAgentException( - "Failed to create proxy for $clazz.\nMore interfaces requested and class is final." - ) - } - } - - private fun newInstanceViaDefaultConstructor(cls: Class<*>): Any { - try { - val defaultConstructor = cls.getDeclaredConstructor() - try { - defaultConstructor.isAccessible = true - } catch (ignored: Exception) { - // skip - } - - return defaultConstructor.newInstance() - } catch (e: Exception) { - throw MockKAgentException("Default constructor instantiation exception", e) - } - } - - - private fun warnOnFinalMethods(clazz: Class<*>) { - for (method in gatherAllMethods(clazz)) { - val modifiers = method.modifiers - if (!Modifier.isPrivate(modifiers) && Modifier.isFinal(modifiers)) { - log.debug( - "It is impossible to intercept calls to $method " + - "for ${method.declaringClass} because it is final" - ) - } - } - } - - - companion object { - - private val notMockableClasses = setOf( - Class::class.java, - Boolean::class.java, - Byte::class.java, - Short::class.java, - Char::class.java, - Int::class.java, - Long::class.java, - Float::class.java, - Double::class.java, - String::class.java - ) - - private fun gatherAllMethods(clazz: Class<*>): Array = - if (clazz.superclass == null) { - clazz.declaredMethods - } else { - gatherAllMethods(clazz.superclass) + clazz.declaredMethods - } - - private fun getAllSuperclasses(cls: Class<*>): Set> { - val result = mutableSetOf>() - - var clazz = cls - while (true) { - result.add(clazz) - addInterfaces(result, clazz) - clazz = clazz.superclass ?: break - } - - return result - } - - private fun addInterfaces(result: MutableSet>, clazz: Class<*>) { - for (intf in clazz.interfaces) { - if (result.add(intf)) { - addInterfaces(result, intf) - } - } - } - } -} diff --git a/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/transformation/SubclassInstrumentation.kt b/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/transformation/JvmSubclassInstrumentation.kt similarity index 95% rename from modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/transformation/SubclassInstrumentation.kt rename to modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/transformation/JvmSubclassInstrumentation.kt index be8e4a404..224ab8a8b 100644 --- a/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/transformation/SubclassInstrumentation.kt +++ b/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/transformation/JvmSubclassInstrumentation.kt @@ -1,6 +1,7 @@ package io.mockk.proxy.jvm.transformation import io.mockk.proxy.MockKAgentLogger +import io.mockk.proxy.common.transformation.SubclassInstrumentation import io.mockk.proxy.jvm.ClassLoadingStrategyChooser import io.mockk.proxy.jvm.advice.ProxyAdviceId import io.mockk.proxy.jvm.advice.jvm.JvmMockKProxyInterceptor @@ -17,11 +18,11 @@ import java.io.File import java.lang.Thread.currentThread import java.util.concurrent.atomic.AtomicLong -internal class SubclassInstrumentation( +internal class JvmSubclassInstrumentation( private val log: MockKAgentLogger, private val handlers: MockHandlerMap, private val byteBuddy: ByteBuddy -) { +): SubclassInstrumentation { private val bootstrapMonitor = Any() private val proxyClassCache = TypeCache(TypeCache.Sort.WEAK) private lateinit var interceptor: JvmMockKProxyInterceptor @@ -35,11 +36,10 @@ internal class SubclassInstrumentation( } } AdviceBuilder().build() - } @Suppress("UNCHECKED_CAST") - fun subclass( + override fun subclass( clazz: Class, interfaces: Array> ): Class { From e4ecd518ecc9b2fbd64738a23f6a3faa9ca0eeec Mon Sep 17 00:00:00 2001 From: Adam <897017+aSemy@users.noreply.github.com> Date: Mon, 16 Jan 2023 00:52:16 +0100 Subject: [PATCH 2/4] update API spec after unused function was removed --- modules/mockk-agent-api/api/mockk-agent-api.api | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/mockk-agent-api/api/mockk-agent-api.api b/modules/mockk-agent-api/api/mockk-agent-api.api index 781b393a4..490bb6134 100644 --- a/modules/mockk-agent-api/api/mockk-agent-api.api +++ b/modules/mockk-agent-api/api/mockk-agent-api.api @@ -125,7 +125,6 @@ public abstract class io/mockk/proxy/common/transformation/RetransformInlineInst } public abstract interface class io/mockk/proxy/common/transformation/SubclassInstrumentation { - public abstract fun setProxyHandler (Ljava/lang/Object;Lio/mockk/proxy/MockKInvocationHandler;)V public abstract fun subclass (Ljava/lang/Class;[Ljava/lang/Class;)Ljava/lang/Class; } From 6bb19113715ad32336a038e2c334377e9d00862a Mon Sep 17 00:00:00 2001 From: Adam <897017+aSemy@users.noreply.github.com> Date: Mon, 16 Jan 2023 00:52:36 +0100 Subject: [PATCH 3/4] add Kotlin reflect (required for checking if class is sealed, in ProxyMaker) --- modules/mockk-agent-api/build.gradle.kts | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/mockk-agent-api/build.gradle.kts b/modules/mockk-agent-api/build.gradle.kts index 0c985f6dc..7356bcc22 100644 --- a/modules/mockk-agent-api/build.gradle.kts +++ b/modules/mockk-agent-api/build.gradle.kts @@ -17,6 +17,7 @@ kotlin { sourceSets { val commonMain by getting { dependencies { + implementation(kotlin("reflect")) } } val commonTest by getting { From ffcf31a1f4c24ae6232acb5da90b5a41bde48365 Mon Sep 17 00:00:00 2001 From: Adam <897017+aSemy@users.noreply.github.com> Date: Fri, 20 Jan 2023 11:58:25 +0100 Subject: [PATCH 4/4] add import --- .../kotlin/io/mockk/proxy/jvm/JvmMockKAgentFactory.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/JvmMockKAgentFactory.kt b/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/JvmMockKAgentFactory.kt index a7834716f..9cc714eab 100644 --- a/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/JvmMockKAgentFactory.kt +++ b/modules/mockk-agent/src/jvmMain/kotlin/io/mockk/proxy/jvm/JvmMockKAgentFactory.kt @@ -1,6 +1,7 @@ package io.mockk.proxy.jvm import io.mockk.proxy.* +import io.mockk.proxy.common.ProxyMaker import io.mockk.proxy.common.transformation.ClassTransformationSpecMap import io.mockk.proxy.jvm.advice.jvm.MockHandlerMap import io.mockk.proxy.jvm.dispatcher.BootJarLoader @@ -98,8 +99,8 @@ class JvmMockKAgentFactory : MockKAgentFactory { ) - jvmProxyMaker = io.mockk.proxy.common.ProxyMaker( - logFactory.logger(io.mockk.proxy.common.ProxyMaker::class.java), + jvmProxyMaker = ProxyMaker( + logFactory.logger(ProxyMaker::class.java), inliner, subclasser, jvmInstantiator,