Skip to content

Commit

Permalink
ByteBuddy: Do not use Lookup for non-local mocks (#2019)
Browse files Browse the repository at this point in the history
This PR introduces the concept of local mocks. A mock is considered to
be local if the target class' classloader can also load all additional
interfaces (incl. `ISpockMockObject`).

The lookup classloading strategy may only be used if a mock is local.
Otherwise, we have to defer to other strategies.

Fixes #2017
  • Loading branch information
beatbrot authored Oct 18, 2024
1 parent f214a8a commit 3076826
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 7 deletions.
1 change: 1 addition & 0 deletions docs/release_notes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ include::include.adoc[]
** This enables loading of optional classes in e.g. OSGi environments
* `EmbeddedSpecRunner` and `EmbeddedSpecCompiler` now support the construction with a custom `ClassLoader` spockPull:1988[]
** This allows the use of these classes in an OSGi environment, where the class imports in the embedded spec are not visible to the Spock OSGi bundle ClassLoader
* Fix mocking issue with the ByteBuddy MockMaker when using multiple classloaders in Java 21 spockIssue:2017[]

== 2.4-M4 (2024-03-21)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import net.bytebuddy.dynamic.Transformer;
import net.bytebuddy.dynamic.loading.ClassInjector;
import net.bytebuddy.dynamic.loading.ClassLoadingStrategy;
import net.bytebuddy.dynamic.loading.MultipleParentClassLoader;
import net.bytebuddy.dynamic.scaffold.TypeValidation;
import net.bytebuddy.implementation.FieldAccessor;
import net.bytebuddy.implementation.MethodDelegation;
Expand All @@ -37,10 +38,9 @@
import org.jetbrains.annotations.VisibleForTesting;
import org.spockframework.mock.ISpockMockObject;
import org.spockframework.mock.codegen.Target;
import org.spockframework.util.Nullable;

import java.lang.reflect.Method;
import java.util.List;
import java.util.Collection;
import java.util.concurrent.Callable;
import java.util.concurrent.ThreadLocalRandom;

Expand Down Expand Up @@ -83,6 +83,27 @@ class ByteBuddyMockFactory {
}
}

/**
* Returns {@code true}, if the mock is considered local
* <p>
* A mock is considered local, if all additional interfaces of the mock (including {@link ISpockMockObject}) are
* loadable by the target class' classloader.
*
* @param targetClass The to-be-mocked class
* @param additionalInterfaces Additional interfaces of the to-be-mocked type
* @return true, if this is a local mock. Otherwise false
*/
@VisibleForTesting
static boolean isLocalMock(Class<?> targetClass, Collection<Class<?>> additionalInterfaces) {
// Inspired by https://github.com/mockito/mockito/blob/99426415c0ceb30e55216c3934854528c83f410e/mockito-core/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java#L165-L166
ClassLoader cl = new MultipleParentClassLoader.Builder()
.appendMostSpecific(targetClass)
.appendMostSpecific(additionalInterfaces)
.appendMostSpecific(ISpockMockObject.class)
.build();
return cl == targetClass.getClassLoader();
}

Object createMock(IMockMaker.IMockCreationSettings settings) {
final Class<?> type = settings.getMockType();
TypeCache.SimpleKey key = new TypeCache.SimpleKey(type, settings.getAdditionalInterface());
Expand All @@ -99,7 +120,8 @@ Object createMock(IMockMaker.IMockCreationSettings settings) {
}
int randomNumber = Math.abs(ThreadLocalRandom.current().nextInt());
String name = String.format("%s$%s$%d", typeName, "SpockMock", randomNumber);
ClassLoadingStrategy<ClassLoader> strategy = determineBestClassLoadingStrategy(targetClass);
ClassLoadingStrategy<ClassLoader> strategy = determineBestClassLoadingStrategy(targetClass, settings);
//noinspection resource
return new ByteBuddy()
.with(TypeValidation.DISABLED) // https://github.com/spockframework/spock/issues/776
.ignore(none())
Expand Down Expand Up @@ -190,8 +212,8 @@ private static boolean isComingFromSignedJar(Class<?> type) {
}

@NotNull
private static ClassLoadingStrategy<ClassLoader> determineBestClassLoadingStrategy(Class<?> targetClass) throws Exception {
if (ClassInjector.UsingLookup.isAvailable()) {
private static ClassLoadingStrategy<ClassLoader> determineBestClassLoadingStrategy(Class<?> targetClass, IMockMaker.IMockCreationSettings settings) throws Exception {
if (ClassInjector.UsingLookup.isAvailable() && isLocalMock(targetClass, settings.getAdditionalInterface())) {
Class<?> methodHandlesClass = Class.forName("java.lang.invoke.MethodHandles");
Class<?> lookupClass = Class.forName("java.lang.invoke.MethodHandles$Lookup");
Method lookupMethod = methodHandlesClass.getMethod("lookup");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@

package org.spockframework.mock.runtime

import net.bytebuddy.dynamic.loading.MultipleParentClassLoader
import org.spockframework.mock.CannotCreateMockException
import spock.lang.Issue
import spock.lang.Specification
import spock.lang.Unroll
import spock.mock.DetachedMockFactory
import spock.mock.MockMakers
import spock.util.EmbeddedSpecRunner

import java.lang.reflect.Proxy
import java.util.concurrent.Callable
Expand Down Expand Up @@ -132,7 +136,7 @@ class ByteBuddyMockMakerSpec extends Specification {

when:
Mock(interfaceClass, mockMaker: MockMakers.byteBuddy)

then:
CannotCreateMockException ex = thrown()
ex.message.startsWith("Cannot create mock for interface Interface. byte-buddy: The class Interface is not visible by the classloader")
Expand All @@ -150,6 +154,40 @@ class ByteBuddyMockMakerSpec extends Specification {
CannotCreateMockException ex = thrown()
ex.message.startsWith("Cannot create mock for interface java.lang.Runnable. byte-buddy: The class AdditionalInterface is not visible by the classloader")
}

@Issue("https://github.com/spockframework/spock/issues/2017")
def "ByteBuddy Mocks with interfaces that are not visible to the mocked type's classloader"() {
given:
def testCl1 = new ByteBuddyTestClassLoader()
def testCl2 = new ByteBuddyTestClassLoader()
testCl1.defineInterface("foo.Bar")
testCl2.defineClass("iam.Mocked")
def cl = new MultipleParentClassLoader([getClass().classLoader, testCl1, testCl2])
def runner = new EmbeddedSpecRunner(cl)

when: "A type is mocked with an additional interface from a classloader outside the mocked type's parent chain"
runner.addClassImport(MockMakers)
runner.runFeatureBody("""
def mock = Mock(iam.Mocked, mockMaker: MockMakers.byteBuddy, additionalInterfaces: [foo.Bar])
expect:
true
""")

then:
noExceptionThrown()
}

@Unroll("For target class #targetClass with additional interfaces #additionalInterfaces, isLocal() == #expected")
def "Local mocks are detected correctly"(Class<?> targetClass, Collection<Class<?>> additionalInterfaces, boolean expected) {
expect:
ByteBuddyMockFactory.isLocalMock(targetClass, additionalInterfaces) == expected

where:
targetClass | additionalInterfaces | expected
DataClass | [] | true
DataClass | [Serializable] | true
DataClass | [new ByteBuddyTestClassLoader().defineInterface("foo")] | false
}
}

@SuppressWarnings('unused')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static ByteBuddyTestClassLoader withInterface(String name) {
}

/**
* Defines an empty interface with the passed {@code node}.
* Defines an empty interface with the passed {@code name}.
*
* @param name the name of the interface
* @return the loaded {@code Class}
Expand All @@ -64,4 +64,20 @@ public synchronized Class<?> defineInterface(String name) {
.load(this, loadingStrategy)
.getLoaded());
}

/**
* Defines an empty class with the passed {@code name}
*
* @param name the name of the class
* @return the loaded {@link Class}
*/
public synchronized Class<?> defineClass(String name) {
//noinspection resource
return cache.computeIfAbsent(name, nameKey -> new ByteBuddy()
.subclass(Object.class)
.name(nameKey)
.make()
.load(this, loadingStrategy)
.getLoaded());
}
}

0 comments on commit 3076826

Please sign in to comment.