Merge pull request #1025 from aSemy/fix/dedupe_proxymaker

De-duplicate ProxyMaker
mockk · Jan 20, 2023 · cac70d9 · cac70d9
2 parents 7d322e3 + ffcf31a
commit cac70d9
Show file tree

Hide file tree

Showing 9 changed files with 41 additions and 284 deletions.
diff --git a/...d/src/main/kotlin/io/mockk/proxy/android/transformation/AndroidSubclassInstrumentation.kt b/...d/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))
-        }
-    }
-
-}
+}
diff --git 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;
 }
 

diff --git 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 {

diff --git a/...api/src/commonMain/kotlin/io/mockk/proxy/common/transformation/SubclassInstrumentation.kt b/...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 <T> subclass(
         clazz: Class<T>,
         interfaces: Array<Class<*>>
     ): Class<T>
-
-    fun setProxyHandler(proxy: Any, handler: MockKInvocationHandler)
 }
diff --git a/...t-api/src/commonMain/kotlin/io/mockk/proxy/common/transformation/TransformationRequest.kt b/...t-api/src/commonMain/kotlin/io/mockk/proxy/common/transformation/TransformationRequest.kt
@@ -10,6 +10,4 @@ data class TransformationRequest(
         type,
         !untransform
     )
-
-
-}
+}
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
@@ -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<Any, MockKInvocationHandler>
 ) : MockKProxyMaker {
+
     override fun <T : Any> proxy(
         clazz: Class<T>,
         interfaces: Array<Class<*>>,
@@ -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<T>(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,13 +115,31 @@ class ProxyMaker(
         }
     }
 
+    private fun <T : Any> findActualClassToBeProxied(
+        clazz: Class<T>,
+    ): Class<T> {
+        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<T>
+    }
+
     private fun <T : Any> subclass(
         clazz: Class<T>,
         interfaces: Array<Class<*>>
     ): Class<T> {
         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
@@ -1,12 +1,13 @@
 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
 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,8 +92,8 @@ class JvmMockKAgentFactory : MockKAgentFactory {
                     )
                 }
 
-                val subclasser = SubclassInstrumentation(
-                    logFactory.logger(SubclassInstrumentation::class.java),
+                val subclasser = JvmSubclassInstrumentation(
+                    logFactory.logger(JvmSubclassInstrumentation::class.java),
                     handlers,
                     byteBuddy
                 )