Skip to content

Commit

Permalink
Merge branch 'safe-accessibility' into release/3.x
Browse files Browse the repository at this point in the history
# Conflicts:
#	src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java
#	src/main/java/org/mockito/plugins/MockMaker.java
  • Loading branch information
raphw committed Aug 14, 2020
2 parents e21d620 + 24c0e45 commit 8bc21ed
Show file tree
Hide file tree
Showing 37 changed files with 1,097 additions and 434 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package org.mockito.internal.configuration;

import static org.mockito.internal.exceptions.Reporter.moreThanOneAnnotationNotAllowed;
import static org.mockito.internal.util.reflection.FieldSetter.setField;

import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
Expand All @@ -19,7 +18,9 @@
import org.mockito.MockitoAnnotations;
import org.mockito.ScopedMock;
import org.mockito.exceptions.base.MockitoException;
import org.mockito.internal.configuration.plugins.Plugins;
import org.mockito.plugins.AnnotationEngine;
import org.mockito.plugins.MemberAccessor;

/**
* Initializes fields annotated with @{@link org.mockito.Mock} or @{@link org.mockito.Captor}.
Expand Down Expand Up @@ -76,8 +77,9 @@ public AutoCloseable process(Class<?> clazz, Object testInstance) {
if (mock != null) {
throwIfAlreadyAssigned(field, alreadyAssigned);
alreadyAssigned = true;
final MemberAccessor accessor = Plugins.getMemberAccessor();
try {
setField(testInstance, field, mock);
accessor.set(field, testInstance, mock);
} catch (Exception e) {
for (ScopedMock scopedMock : scopedMocks) {
scopedMock.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.exceptions.base.MockitoException;
import org.mockito.internal.configuration.plugins.Plugins;
import org.mockito.internal.util.MockUtil;
import org.mockito.plugins.AnnotationEngine;
import org.mockito.plugins.MemberAccessor;

/**
* Process fields annotated with &#64;Spy.
Expand All @@ -50,23 +52,23 @@ public class SpyAnnotationEngine
@Override
public AutoCloseable process(Class<?> context, Object testInstance) {
Field[] fields = context.getDeclaredFields();
MemberAccessor accessor = Plugins.getMemberAccessor();
for (Field field : fields) {
if (field.isAnnotationPresent(Spy.class)
&& !field.isAnnotationPresent(InjectMocks.class)) {
assertNoIncompatibleAnnotations(Spy.class, field, Mock.class, Captor.class);
field.setAccessible(true);
Object instance;
try {
instance = field.get(testInstance);
instance = accessor.get(field, testInstance);
if (MockUtil.isMock(instance)) {
// instance has been spied earlier
// for example happens when MockitoAnnotations.openMocks is called two
// times.
Mockito.reset(instance);
} else if (instance != null) {
field.set(testInstance, spyInstance(field, instance));
accessor.set(field, testInstance, spyInstance(field, instance));
} else {
field.set(testInstance, spyNewInstance(testInstance, field));
accessor.set(field, testInstance, spyNewInstance(testInstance, field));
}
} catch (Exception e) {
throw new MockitoException(
Expand Down Expand Up @@ -123,8 +125,8 @@ private static Object spyNewInstance(Object testInstance, Field field)

Constructor<?> constructor = noArgConstructorOf(type);
if (Modifier.isPrivate(constructor.getModifiers())) {
constructor.setAccessible(true);
return Mockito.mock(type, settings.spiedInstance(constructor.newInstance()));
MemberAccessor accessor = Plugins.getMemberAccessor();
return Mockito.mock(type, settings.spiedInstance(accessor.newInstance(constructor)));
} else {
return Mockito.mock(type, settings.useConstructor());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,17 @@
package org.mockito.internal.configuration.injection;

import static org.mockito.Mockito.withSettings;
import static org.mockito.internal.util.reflection.FieldSetter.setField;

import java.lang.reflect.Field;
import java.util.Set;

import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.exceptions.base.MockitoException;
import org.mockito.internal.configuration.plugins.Plugins;
import org.mockito.internal.util.MockUtil;
import org.mockito.internal.util.reflection.FieldReader;
import org.mockito.plugins.MemberAccessor;

/**
* Handler for field annotated with &#64;InjectMocks and &#64;Spy.
Expand All @@ -26,6 +27,8 @@
*/
public class SpyOnInjectedFieldsHandler extends MockInjectionStrategy {

private final MemberAccessor accessor = Plugins.getMemberAccessor();

@Override
protected boolean processInjection(Field field, Object fieldOwner, Set<Object> mockCandidates) {
FieldReader fieldReader = new FieldReader(fieldOwner, field);
Expand All @@ -46,7 +49,7 @@ protected boolean processInjection(Field field, Object fieldOwner, Set<Object> m
.spiedInstance(instance)
.defaultAnswer(Mockito.CALLS_REAL_METHODS)
.name(field.getName()));
setField(fieldOwner, field, mock);
accessor.set(field, fieldOwner, mock);
}
} catch (Exception e) {
throw new MockitoException("Problems initiating spied field " + field.getName(), e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
package org.mockito.internal.configuration.injection.filter;

import static org.mockito.internal.exceptions.Reporter.cannotInjectDependency;
import static org.mockito.internal.util.reflection.FieldSetter.setField;

import java.lang.reflect.Field;
import java.util.Collection;
import java.util.List;

import org.mockito.internal.configuration.plugins.Plugins;
import org.mockito.internal.util.reflection.BeanPropertySetter;
import org.mockito.plugins.MemberAccessor;

/**
* This node returns an actual injecter which will be either :
Expand All @@ -30,18 +31,17 @@ public OngoingInjector filterCandidate(
if (mocks.size() == 1) {
final Object matchingMock = mocks.iterator().next();

return new OngoingInjector() {
public Object thenInject() {
try {
if (!new BeanPropertySetter(injectee, candidateFieldToBeInjected)
.set(matchingMock)) {
setField(injectee, candidateFieldToBeInjected, matchingMock);
}
} catch (RuntimeException e) {
throw cannotInjectDependency(candidateFieldToBeInjected, matchingMock, e);
MemberAccessor accessor = Plugins.getMemberAccessor();
return () -> {
try {
if (!new BeanPropertySetter(injectee, candidateFieldToBeInjected)
.set(matchingMock)) {
accessor.set(candidateFieldToBeInjected, injectee, matchingMock);
}
return matchingMock;
} catch (RuntimeException | IllegalAccessException e) {
throw cannotInjectDependency(candidateFieldToBeInjected, matchingMock, e);
}
return matchingMock;
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,13 @@
import java.util.Map;

import org.mockito.internal.creation.instance.InstantiatorProvider2Adapter;
import org.mockito.plugins.AnnotationEngine;
import org.mockito.plugins.InstantiatorProvider;
import org.mockito.plugins.InstantiatorProvider2;
import org.mockito.plugins.MockMaker;
import org.mockito.plugins.MockitoLogger;
import org.mockito.plugins.MockitoPlugins;
import org.mockito.plugins.PluginSwitch;
import org.mockito.plugins.StackTraceCleanerProvider;
import org.mockito.plugins.*;

class DefaultMockitoPlugins implements MockitoPlugins {

private static final Map<String, String> DEFAULT_PLUGINS = new HashMap<String, String>();
static final String INLINE_ALIAS = "mock-maker-inline";
static final String MODULE_ALIAS = "member-accessor-module";

static {
// Keep the mapping: plugin interface name -> plugin implementation class name
Expand All @@ -41,6 +35,11 @@ class DefaultMockitoPlugins implements MockitoPlugins {
INLINE_ALIAS, "org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker");
DEFAULT_PLUGINS.put(
MockitoLogger.class.getName(), "org.mockito.internal.util.ConsoleMockitoLogger");
DEFAULT_PLUGINS.put(
MemberAccessor.class.getName(),
"org.mockito.internal.util.reflection.ReflectionMemberAccessor");
DEFAULT_PLUGINS.put(
MODULE_ALIAS, "org.mockito.internal.util.reflection.ModuleMemberAccessor");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,7 @@
package org.mockito.internal.configuration.plugins;

import org.mockito.internal.creation.instance.InstantiatorProviderAdapter;
import org.mockito.plugins.AnnotationEngine;
import org.mockito.plugins.InstantiatorProvider;
import org.mockito.plugins.InstantiatorProvider2;
import org.mockito.plugins.MockMaker;
import org.mockito.plugins.MockitoLogger;
import org.mockito.plugins.PluginSwitch;
import org.mockito.plugins.StackTraceCleanerProvider;
import org.mockito.plugins.*;

class PluginRegistry {

Expand All @@ -22,6 +16,10 @@ class PluginRegistry {
new PluginLoader(pluginSwitch, DefaultMockitoPlugins.INLINE_ALIAS)
.loadPlugin(MockMaker.class);

private final MemberAccessor memberAccessor =
new PluginLoader(pluginSwitch, DefaultMockitoPlugins.MODULE_ALIAS)
.loadPlugin(MemberAccessor.class);

private final StackTraceCleanerProvider stackTraceCleanerProvider =
new PluginLoader(pluginSwitch).loadPlugin(StackTraceCleanerProvider.class);

Expand Down Expand Up @@ -62,6 +60,16 @@ MockMaker getMockMaker() {
return mockMaker;
}

/**
* Returns the implementation of the member accessor available for the current runtime.
*
* <p>Returns {@link org.mockito.internal.util.reflection.ReflectionMemberAccessor} if no
* {@link org.mockito.plugins.MockMaker} extension exists or is visible in the current classpath.</p>
*/
MemberAccessor getMemberAccessor() {
return memberAccessor;
}

/**
* Returns the instantiator provider available for the current runtime.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@
*/
package org.mockito.internal.configuration.plugins;

import org.mockito.plugins.AnnotationEngine;
import org.mockito.plugins.InstantiatorProvider2;
import org.mockito.plugins.MockMaker;
import org.mockito.plugins.MockitoLogger;
import org.mockito.plugins.MockitoPlugins;
import org.mockito.plugins.StackTraceCleanerProvider;
import org.mockito.plugins.*;

/**
* Access to Mockito behavior that can be reconfigured by plugins
Expand All @@ -35,6 +30,16 @@ public static MockMaker getMockMaker() {
return registry.getMockMaker();
}

/**
* Returns the implementation of the member accessor available for the current runtime.
*
* <p>Returns default member accessor if no
* {@link org.mockito.plugins.MemberAccessor} extension exists or is visible in the current classpath.</p>
*/
public static MemberAccessor getMemberAccessor() {
return registry.getMemberAccessor();
}

/**
* Returns the instantiator provider available for the current runtime.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import static org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.ForWriteReplace;
import static org.mockito.internal.util.StringUtil.join;
import static org.mockito.internal.util.reflection.FieldSetter.setField;

import java.io.*;
import java.lang.reflect.Field;
Expand All @@ -22,6 +21,7 @@
import org.mockito.mock.MockCreationSettings;
import org.mockito.mock.MockName;
import org.mockito.mock.SerializableMode;
import org.mockito.plugins.MemberAccessor;

/**
* This is responsible for serializing a mock, it is enabled if the mock is implementing {@link Serializable}.
Expand Down Expand Up @@ -318,8 +318,14 @@ protected Class<?> resolveClass(ObjectStreamClass desc)
private void hackClassNameToMatchNewlyCreatedClass(
ObjectStreamClass descInstance, Class<?> proxyClass) throws ObjectStreamException {
try {
MemberAccessor accessor = Plugins.getMemberAccessor();
Field classNameField = descInstance.getClass().getDeclaredField("name");
setField(descInstance, classNameField, proxyClass.getCanonicalName());
try {
accessor.set(classNameField, descInstance, proxyClass.getCanonicalName());
} catch (IllegalAccessException e) {
throw new MockitoSerializationIssue(
"Access to " + classNameField + " was denied", e);
}
} catch (NoSuchFieldException nsfe) {
throw new MockitoSerializationIssue(
join(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import net.bytebuddy.pool.TypePool;
import net.bytebuddy.utility.OpenedClassReader;
import org.mockito.exceptions.base.MockitoException;
import org.mockito.internal.configuration.plugins.Plugins;
import org.mockito.internal.creation.bytebuddy.inject.MockMethodDispatcher;
import org.mockito.internal.debugging.LocationImpl;
import org.mockito.internal.exceptions.stacktrace.ConditionalStackTraceFilter;
Expand All @@ -49,6 +50,7 @@
import org.mockito.internal.invocation.mockref.MockWeakReference;
import org.mockito.internal.util.concurrent.DetachedThreadLocal;
import org.mockito.internal.util.concurrent.WeakConcurrentMap;
import org.mockito.plugins.MemberAccessor;

import static net.bytebuddy.matcher.ElementMatchers.*;

Expand Down Expand Up @@ -244,10 +246,6 @@ public boolean isInvokable() {

@Override
public Object invoke() throws Throwable {
if (!Modifier.isPublic(
origin.getDeclaringClass().getModifiers() & origin.getModifiers())) {
origin.setAccessible(true);
}
selfCallInfo.set(instanceRef.get());
return tryInvoke(origin, instanceRef.get(), arguments);
}
Expand Down Expand Up @@ -279,10 +277,6 @@ public boolean isInvokable() {
@Override
public Object invoke() throws Throwable {
Method method = origin.getJavaMethod();
if (!Modifier.isPublic(
method.getDeclaringClass().getModifiers() & method.getModifiers())) {
method.setAccessible(true);
}
MockMethodDispatcher mockMethodDispatcher =
MockMethodDispatcher.get(identifier, instanceRef.get());
if (!(mockMethodDispatcher instanceof MockMethodAdvice)) {
Expand Down Expand Up @@ -324,18 +318,16 @@ public boolean isInvokable() {

@Override
public Object invoke() throws Throwable {
if (!Modifier.isPublic(type.getModifiers() & origin.getModifiers())) {
origin.setAccessible(true);
}
selfCallInfo.set(type);
return tryInvoke(origin, null, arguments);
}
}

private static Object tryInvoke(Method origin, Object instance, Object[] arguments)
throws Throwable {
MemberAccessor accessor = Plugins.getMemberAccessor();
try {
return origin.invoke(instance, arguments);
return accessor.invoke(origin, instance, arguments);
} catch (InvocationTargetException exception) {
Throwable cause = exception.getCause();
new ConditionalStackTraceFilter()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@

import org.mockito.creation.instance.InstantiationException;
import org.mockito.creation.instance.Instantiator;
import org.mockito.internal.configuration.plugins.Plugins;
import org.mockito.internal.util.Primitives;
import org.mockito.internal.util.reflection.AccessibilityChanger;
import org.mockito.plugins.MemberAccessor;

public class ConstructorInstantiator implements Instantiator {

Expand Down Expand Up @@ -64,9 +65,8 @@ private <T> T withParams(Class<T> cls, Object... params) {
private static <T> T invokeConstructor(Constructor<?> constructor, Object... params)
throws java.lang.InstantiationException, IllegalAccessException,
InvocationTargetException {
AccessibilityChanger accessibility = new AccessibilityChanger();
accessibility.enableAccess(constructor);
return (T) constructor.newInstance(params);
MemberAccessor accessor = Plugins.getMemberAccessor();
return (T) accessor.newInstance(constructor, params);
}

private InstantiationException paramsException(Class<?> cls, Exception e) {
Expand Down
Loading

0 comments on commit 8bc21ed

Please sign in to comment.