Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mock resolver plugin #2042

Merged
merged 3 commits into from Oct 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -6,6 +6,8 @@

import java.io.InputStream;
import java.net.URL;
import java.util.ArrayList;
import java.util.List;

import org.mockito.exceptions.base.MockitoException;
import org.mockito.internal.util.io.IOUtil;
Expand Down Expand Up @@ -43,4 +45,30 @@ String findPluginClass(Iterable<URL> resources) {
}
return null;
}

List<String> findPluginClasses(Iterable<URL> resources) {
List<String> pluginClassNames = new ArrayList<>();
for (URL resource : resources) {
InputStream s = null;
try {
s = resource.openStream();
String pluginClassName = new PluginFileReader().readPluginClass(s);
if (pluginClassName == null) {
// For backwards compatibility
// If the resource does not have plugin class name we're ignoring it
continue;
}
if (!pluginSwitch.isEnabled(pluginClassName)) {
continue;
}
pluginClassNames.add(pluginClassName);
} catch (Exception e) {
throw new MockitoException(
"Problems reading plugin implementation from: " + resource, e);
} finally {
IOUtil.closeQuietly(s);
}
}
return pluginClassNames;
}
}
Expand Up @@ -6,7 +6,9 @@

import java.io.IOException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.List;

import org.mockito.internal.util.collections.Iterables;
import org.mockito.plugins.PluginSwitch;
Expand Down Expand Up @@ -56,4 +58,36 @@ public <T> T loadImpl(Class<T> service) {
"Failed to load " + service + " implementation declared in " + resources, e);
}
}

public <T> List<T> loadImpls(Class<T> service) {
ClassLoader loader = Thread.currentThread().getContextClassLoader();
if (loader == null) {
loader = ClassLoader.getSystemClassLoader();
}
Enumeration<URL> resources;
try {
resources = loader.getResources("mockito-extensions/" + service.getName());
} catch (IOException e) {
throw new IllegalStateException("Failed to load " + service, e);
}

try {
List<String> classesOrAliases =
new PluginFinder(pluginSwitch)
.findPluginClasses(Iterables.toIterable(resources));
List<T> impls = new ArrayList<>();
for (String classOrAlias : classesOrAliases) {
if (classOrAlias.equals(alias)) {
classOrAlias = plugins.getDefaultPluginClass(alias);
}
Class<?> pluginClass = loader.loadClass(classOrAlias);
Object plugin = pluginClass.getDeclaredConstructor().newInstance();
impls.add(service.cast(plugin));
}
return impls;
} catch (Exception e) {
throw new IllegalStateException(
"Failed to load " + service + " implementation declared in " + resources, e);
}
}
}
Expand Up @@ -7,6 +7,8 @@
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.Collections;
import java.util.List;

import org.mockito.plugins.PluginSwitch;

Expand Down Expand Up @@ -90,4 +92,32 @@ public Object invoke(Object proxy, Method method, Object[] args)
});
}
}

/**
* Scans the classpath for given {@code pluginType} and returns a list of its instances.
*
* @return An list of {@code pluginType} or an empty list if none was found.
*/
@SuppressWarnings("unchecked")
<T> List<T> loadPlugins(final Class<T> pluginType) {
try {
return initializer.loadImpls(pluginType);
} catch (final Throwable t) {
return Collections.singletonList(
(T)
Proxy.newProxyInstance(
pluginType.getClassLoader(),
new Class<?>[] {pluginType},
new InvocationHandler() {
@Override
public Object invoke(
Object proxy, Method method, Object[] args)
throws Throwable {
throw new IllegalStateException(
"Could not initialize plugin: " + pluginType,
t);
}
}));
}
}
}
Expand Up @@ -7,6 +7,8 @@
import org.mockito.internal.creation.instance.InstantiatorProviderAdapter;
import org.mockito.plugins.*;

import java.util.List;

class PluginRegistry {

private final PluginSwitch pluginSwitch =
Expand All @@ -31,6 +33,9 @@ class PluginRegistry {
private final MockitoLogger mockitoLogger =
new PluginLoader(pluginSwitch).loadPlugin(MockitoLogger.class);

private final List<MockResolver> mockResolvers =
new PluginLoader(pluginSwitch).loadPlugins(MockResolver.class);

PluginRegistry() {
Object impl =
new PluginLoader(pluginSwitch)
Expand Down Expand Up @@ -100,4 +105,13 @@ AnnotationEngine getAnnotationEngine() {
MockitoLogger getMockitoLogger() {
return mockitoLogger;
}

/**
* Returns a list of available mock resolvers if any.
*
* @return A list of available mock resolvers or an empty list if none are registered.
*/
List<MockResolver> getMockResolvers() {
return mockResolvers;
}
}
Expand Up @@ -6,6 +6,8 @@

import org.mockito.plugins.*;

import java.util.List;

/**
* Access to Mockito behavior that can be reconfigured by plugins
*/
Expand Down Expand Up @@ -71,6 +73,15 @@ public static MockitoLogger getMockitoLogger() {
return registry.getMockitoLogger();
}

/**
* Returns a list of available mock resolvers if any.
*
* @return A list of available mock resolvers or an empty list if none are registered.
*/
public static List<MockResolver> getMockResolvers() {
return registry.getMockResolvers();
}

/**
* @return instance of mockito plugins type
*/
Expand Down
Expand Up @@ -397,13 +397,17 @@ public MethodVisitor wrap(
.getDeclaredMethods()
.filter(isConstructor().and(not(isPrivate())));
int arguments = Integer.MAX_VALUE;
boolean visible = false;
boolean packagePrivate = true;
MethodDescription.InDefinedShape current = null;
for (MethodDescription.InDefinedShape constructor : constructors) {
// We are choosing the shortest constructor with regards to arguments.
// Yet, we prefer a non-package-private constructor since they require
// the super class to be on the same class loader.
if (constructor.getParameters().size() < arguments
&& (!visible || constructor.isPackagePrivate())) {
&& (packagePrivate || !constructor.isPackagePrivate())) {
arguments = constructor.getParameters().size();
packagePrivate = constructor.isPackagePrivate();
current = constructor;
visible = constructor.isPackagePrivate();
}
}
if (current != null) {
Expand Down
Expand Up @@ -73,7 +73,7 @@ public Object getMock() {
return toInspect;
}

private MockHandler<Object> mockHandler() {
private MockHandler<?> mockHandler() {
assertGoodMock();
return MockUtil.getMockHandler(toInspect);
}
Expand Down
31 changes: 26 additions & 5 deletions src/main/java/org/mockito/internal/util/MockUtil.java
Expand Up @@ -16,6 +16,7 @@
import org.mockito.mock.MockName;
import org.mockito.plugins.MockMaker;
import org.mockito.plugins.MockMaker.TypeMockability;
import org.mockito.plugins.MockResolver;

import java.util.function.Function;

Expand Down Expand Up @@ -55,21 +56,25 @@ public static <T> T createMock(MockCreationSettings<T> settings) {
return mock;
}

public static <T> void resetMock(T mock) {
public static void resetMock(Object mock) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why were the generic parameters removed from this and the other methods? I understand that they are not used, but I wonder if there are unintended side-effects of this change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as a cleanup on the side since I am touching the code already. The generic type is erased to Object and has no other function, that's why I removed it. No impact on source or binary compatibility.

MockHandler oldHandler = getMockHandler(mock);
MockCreationSettings settings = oldHandler.getMockSettings();
MockHandler newHandler = createMockHandler(settings);

mock = resolve(mock);
mockMaker.resetMock(mock, newHandler, settings);
}

public static <T> MockHandler<T> getMockHandler(T mock) {
public static MockHandler<?> getMockHandler(Object mock) {
if (mock == null) {
throw new NotAMockException("Argument should be a mock, but is null!");
}

if (isMock(mock)) {
return mockMaker.getHandler(mock);
mock = resolve(mock);

MockHandler handler = mockMaker.getHandler(mock);
if (handler != null) {
return handler;
} else {
throw new NotAMockException("Argument should be a mock, but is: " + mock.getClass());
}
Expand All @@ -96,7 +101,23 @@ public static boolean isMock(Object mock) {
// Potentially we could also move other methods to MockitoMock, some other candidates:
// getInvocationContainer, isSpy, etc.
// This also allows us to reuse our public API MockingDetails
return mock != null && mockMaker.getHandler(mock) != null;
if (mock == null) {
return false;
}

mock = resolve(mock);

return mockMaker.getHandler(mock) != null;
}

private static Object resolve(Object mock) {
if (mock instanceof Class<?>) { // static mocks are resolved by definition
return mock;
}
for (MockResolver mockResolver : Plugins.getMockResolvers()) {
mock = mockResolver.resolve(mock);
}
return mock;
}

public static MockName getMockName(Object mock) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/mockito/plugins/MockMaker.java
Expand Up @@ -85,7 +85,7 @@ public interface MockMaker {
* {@link #getHandler(Object)} will return this instance.
* @param instance The object to spy upon.
* @param <T> Type of the mock to return, actually the <code>settings.getTypeToMock</code>.
* @return
* @return The spy instance, if this mock maker supports direct spy creation.
* @since 3.5.0
*/
default <T> Optional<T> createSpy(
Expand Down
22 changes: 22 additions & 0 deletions src/main/java/org/mockito/plugins/MockResolver.java
@@ -0,0 +1,22 @@
/*
* Copyright (c) 2020 Mockito contributors
* This program is made available under the terms of the MIT License.
*/
package org.mockito.plugins;

/**
* A mock resolver offers an opportunity to resolve a mock from any instance that is
* provided to the {@link org.mockito.Mockito}-DSL. This mechanism can be used by
* frameworks that provide mocks that are implemented by Mockito but which are wrapped
* by other instances to enhance the proxy further.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be marked as @Incubating

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arghs, I missed this and just hit merge. Sorry, we can still add this, though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes let's still add it.

public interface MockResolver {

/**
* Returns the provided instance or the unwrapped mock that the provided
* instance represents. This method must not return {@code null}.
* @param instance The instance passed to the {@link org.mockito.Mockito}-DSL.
* @return The provided instance or the unwrapped mock.
*/
Object resolve(Object instance);
}
@@ -0,0 +1,51 @@
/*
* Copyright (c) 2020 Mockito contributors
* This program is made available under the terms of the MIT License.
*/
package org.mockitousage.plugins.resolver;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.*;

@MockitoSettings(strictness = Strictness.WARN)
@ExtendWith(MockitoExtension.class)
class MockResolverTest {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test that uses multiple resolvers? I can think of the following scenarios:

  1. Multiple resolvers, no match
  2. Multiple resolvers, one match
  3. Multiple resolvers, multiple match

For scenario 3, should we throw maybe? What if it is order-dependent? Should we continue iterating over the resolvers until none match?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think multiple resolvers should be allowed. You can technically add several in your specified order and decompose multiple levels of nesting. It's of course an obscure scenario, but it makes sense to not forbid it since it would be extra work. I can add the tests you suggest.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, registering multiple converters in a test is not quite that easy since it would require multiple modules. I don't think its worth the noise to add this functionality just for testing such a simple corner case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. How would that work when a user wants to integrate with multiple frameworks that all expose a resolver?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They would all expose their resolver and they would be invoked in their registration order. For instance, let's say Hibernate offered a Mockito-based mocking API but returns Hibernate proxies on mocks. Those proxies are by definition never Spring Beans. But if one passed a Spring bean or a Hibernate proxy, both would traverse the resolver chain and be unpacked accordingly.

I assume that normally, only one resolver would apply to any wrapped mock but since Mockito should remain agnostic to this, that would be the best approach in my eyes.


@Test
void mock_resolver_can_unwrap_mocked_instance() {
Foo mock = mock(Foo.class), wrapper = new MockWrapper(mock);
when(wrapper.doIt()).thenReturn(123);
assertThat(mock.doIt()).isEqualTo(123);
assertThat(wrapper.doIt()).isEqualTo(123);
verify(wrapper, times(2)).doIt();
}

interface Foo {
int doIt();
}

static class MockWrapper implements Foo {

private final Foo mock;

MockWrapper(Foo mock) {
this.mock = mock;
}

Object getMock() {
return mock;
}

@Override
public int doIt() {
return mock.doIt();
}
}

}
@@ -0,0 +1,18 @@
/*
* Copyright (c) 2020 Mockito contributors
* This program is made available under the terms of the MIT License.
*/
package org.mockitousage.plugins.resolver;

import org.mockito.plugins.MockResolver;

public class MyMockResolver implements MockResolver {

@Override
public Object resolve(Object instance) {
if (instance instanceof MockResolverTest.MockWrapper) {
return ((MockResolverTest.MockWrapper) instance).getMock();
}
return instance;
}
}
Expand Up @@ -7,6 +7,7 @@
import org.junit.Test;
import org.mockitousage.plugins.instantiator.MyInstantiatorProvider2;
import org.mockitousage.plugins.logger.MyMockitoLogger;
import org.mockitousage.plugins.resolver.MyMockResolver;
import org.mockitousage.plugins.stacktrace.MyStackTraceCleanerProvider;

import java.util.List;
Expand All @@ -25,6 +26,7 @@ public void plugin_switcher_is_used() {
assertEquals(MyPluginSwitch.invokedFor, asList(MyMockMaker.class.getName(),
MyStackTraceCleanerProvider.class.getName(),
MyMockitoLogger.class.getName(),
MyMockResolver.class.getName(),
MyInstantiatorProvider2.class.getName()));
}

Expand Down