fixup! Fixes mockito#2626 : Introduce MockSettings.mockMaker

JojOatXGME · Jul 16, 2022 · fc3f0b1 · fc3f0b1
1 parent f58affa
commit fc3f0b1
Show file tree

Hide file tree

Showing 15 changed files with 298 additions and 82 deletions.
diff --git a/src/main/java/org/mockito/Mock.java b/src/main/java/org/mockito/Mock.java
@@ -13,6 +13,7 @@
 import java.lang.annotation.Target;
 
 import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.plugins.MockMaker;
 import org.mockito.stubbing.Answer;
 
 /**
@@ -124,6 +125,8 @@
      */
     Strictness strictness() default Strictness.TEST_LEVEL_DEFAULT;
 
+    Class<? extends MockMaker>[] mockMaker() default {};
+
     enum Strictness {
 
         /**

diff --git a/src/main/java/org/mockito/internal/MockitoCore.java b/src/main/java/org/mockito/internal/MockitoCore.java
@@ -22,7 +22,6 @@
 import static org.mockito.internal.util.MockUtil.getMockHandler;
 import static org.mockito.internal.util.MockUtil.isMock;
 import static org.mockito.internal.util.MockUtil.resetMock;
-import static org.mockito.internal.util.MockUtil.typeMockabilityOf;
 import static org.mockito.internal.verification.VerificationModeFactory.noInteractions;
 import static org.mockito.internal.verification.VerificationModeFactory.noMoreInteractions;
 
@@ -31,6 +30,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.function.Function;
 
 import org.mockito.InOrder;
 import org.mockito.MockSettings;
@@ -67,21 +67,13 @@
 import org.mockito.stubbing.Stubber;
 import org.mockito.verification.VerificationMode;
 
-import java.util.Arrays;
-import java.util.List;
-import java.util.function.Function;
-
 @SuppressWarnings("unchecked")
 public class MockitoCore {
 
     private static final DoNotMockEnforcer DO_NOT_MOCK_ENFORCER = Plugins.getDoNotMockEnforcer();
     private static final Set<Class<?>> MOCKABLE_CLASSES =
             Collections.synchronizedSet(new HashSet<>());
 
-    public boolean isTypeMockable(Class<?> typeToMock) {
-        return typeMockabilityOf(typeToMock).mockable();
-    }
-
     public <T> T mock(Class<T> typeToMock, MockSettings settings) {
         if (!(settings instanceof MockSettingsImpl)) {
             throw new IllegalArgumentException(
@@ -166,7 +158,7 @@ public <T> MockedConstruction<T> mockConstruction(
                                 "Unexpected MockMaker '"
                                         + mockMaker.getCanonicalName()
                                         + "'\n"
-                                        + "You cannot override the MockMaker for construction mocks.");
+                                        + "At the moment, you cannot override the MockMaker for construction mocks.");
                     }
                     return impl.build(typeToMock);
                 };

diff --git a/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java b/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java
@@ -53,6 +53,11 @@ public static Object processAnnotationForMock(
         if (annotation.strictness() != Mock.Strictness.TEST_LEVEL_DEFAULT) {
             mockSettings.strictness(Strictness.valueOf(annotation.strictness().toString()));
         }
+        if (annotation.mockMaker().length > 0) {
+            // TODO: Use a special placeholder class for the default, instead of using an array?
+            assert annotation.mockMaker().length == 1 : "cannon use multiple mock makers";
+            mockSettings.mockMaker(annotation.mockMaker()[0]);
+        }
 
         // see @Mock answer default value
         mockSettings.defaultAnswer(annotation.answer());

diff --git a/src/main/java/org/mockito/internal/configuration/SpyAnnotationEngine.java b/src/main/java/org/mockito/internal/configuration/SpyAnnotationEngine.java
@@ -83,6 +83,7 @@ public AutoCloseable process(Class<?> context, Object testInstance) {
     }
 
     private static Object spyInstance(Field field, Object instance) {
+        // TODO: Should we also add an option to @Spy?
         return Mockito.mock(
                 instance.getClass(),
                 withSettings()
@@ -93,6 +94,7 @@ private static Object spyInstance(Field field, Object instance) {
 
     private static Object spyNewInstance(Object testInstance, Field field)
             throws InstantiationException, IllegalAccessException, InvocationTargetException {
+        // TODO: Should we also add an option to @Spy?
         MockSettings settings =
                 withSettings().defaultAnswer(CALLS_REAL_METHODS).name(field.getName());
         Class<?> type = field.getType();

diff --git a/src/main/java/org/mockito/internal/configuration/injection/SpyOnInjectedFieldsHandler.java b/src/main/java/org/mockito/internal/configuration/injection/SpyOnInjectedFieldsHandler.java
@@ -42,6 +42,7 @@ protected boolean processInjection(Field field, Object fieldOwner, Set<Object> m
                     // B. protect against multiple use of MockitoAnnotations.openMocks()
                     Mockito.reset(instance);
                 } else {
+                    // TODO: Should we also add an option to @Spy?
                     Object mock =
                             Mockito.mock(
                                     instance.getClass(),

diff --git a/.../java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java b/.../java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java
@@ -8,16 +8,13 @@
 import java.lang.reflect.Type;
 import java.lang.reflect.TypeVariable;
 
-import org.mockito.internal.MockitoCore;
 import org.mockito.internal.util.MockUtil;
 import org.mockito.internal.util.reflection.GenericMetadataSupport;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.mock.MockCreationSettings;
 
 final class RetrieveGenericsForDefaultAnswers {
 
-    private static final MockitoCore MOCKITO_CORE = new MockitoCore();
-
     static Object returnTypeForMockWithCorrectGenerics(
             InvocationOnMock invocation, AnswerCallback answerCallback) {
         Class<?> type = invocation.getMethod().getReturnType();
@@ -38,8 +35,9 @@ static Object returnTypeForMockWithCorrectGenerics(
         }
 
         if (type != null) {
-            // TODO: Should we use the mockMaker of the mock?
-            if (!MOCKITO_CORE.isTypeMockable(type)) {
+            final MockCreationSettings<?> mockSettings =
+                    MockUtil.getMockSettings(invocation.getMock());
+            if (!MockUtil.typeMockabilityOf(type, mockSettings.getMockMaker()).mockable()) {
                 return null;
             }
 

diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java
@@ -5,6 +5,7 @@
 package org.mockito.internal.stubbing.defaultanswers;
 
 import static org.mockito.Mockito.withSettings;
+import static org.mockito.internal.util.MockUtil.typeMockabilityOf;
 
 import java.io.IOException;
 import java.io.Serializable;
@@ -50,10 +51,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
         GenericMetadataSupport returnTypeGenericMetadata =
                 actualParameterizedType(invocation.getMock())
                         .resolveGenericReturnType(invocation.getMethod());
+        MockCreationSettings<?> mockSettings = MockUtil.getMockSettings(invocation.getMock());
 
         Class<?> rawType = returnTypeGenericMetadata.rawType();
-        // TODO: Should we use the mockMaker from the mock?
-        if (!mockitoCore().isTypeMockable(rawType)) {
+        if (!typeMockabilityOf(rawType, mockSettings.getMockMaker()).mockable()) {
             if (invocation.getMethod().getReturnType().equals(rawType)) {
                 return delegate().answer(invocation);
             } else {
@@ -120,15 +121,16 @@ private Object newDeepStubMock(
 
     private MockSettings withSettingsUsing(
             GenericMetadataSupport returnTypeGenericMetadata,
-            MockCreationSettings parentMockSettings) {
+            MockCreationSettings<?> parentMockSettings) {
         MockSettings mockSettings =
                 returnTypeGenericMetadata.hasRawExtraInterfaces()
                         ? withSettings()
                                 .extraInterfaces(returnTypeGenericMetadata.rawExtraInterfaces())
                         : withSettings();
 
         return propagateSerializationSettings(mockSettings, parentMockSettings)
-                .defaultAnswer(returnsDeepStubsAnswerUsing(returnTypeGenericMetadata));
+                .defaultAnswer(returnsDeepStubsAnswerUsing(returnTypeGenericMetadata))
+                .mockMaker(parentMockSettings.getMockMaker());
     }
 
     private MockSettings propagateSerializationSettings(

diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocks.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocks.java
@@ -8,7 +8,9 @@
 
 import org.mockito.Mockito;
 import org.mockito.internal.creation.MockSettingsImpl;
+import org.mockito.internal.util.MockUtil;
 import org.mockito.invocation.InvocationOnMock;
+import org.mockito.mock.MockCreationSettings;
 import org.mockito.stubbing.Answer;
 
 public class ReturnsMocks implements Answer<Object>, Serializable {
@@ -33,9 +35,14 @@ public Object apply(Class<?> type) {
                             return null;
                         }
 
+                        MockCreationSettings<?> mockSettings =
+                                MockUtil.getMockSettings(invocation.getMock());
+
                         return Mockito.mock(
                                 type,
-                                new MockSettingsImpl<Object>().defaultAnswer(ReturnsMocks.this));
+                                new MockSettingsImpl<>()
+                                        .defaultAnswer(ReturnsMocks.this)
+                                        .mockMaker(mockSettings.getMockMaker()));
                     }
                 });
     }

diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNulls.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNulls.java
@@ -10,9 +10,12 @@
 import java.io.Serializable;
 
 import org.mockito.Mockito;
+import org.mockito.internal.creation.MockSettingsImpl;
 import org.mockito.internal.debugging.LocationImpl;
+import org.mockito.internal.util.MockUtil;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.invocation.Location;
+import org.mockito.mock.MockCreationSettings;
 import org.mockito.stubbing.Answer;
 
 /**
@@ -56,8 +59,16 @@ public Object apply(Class<?> type) {
                             return null;
                         }
 
+                        MockCreationSettings<?> mockSettings =
+                                MockUtil.getMockSettings(invocation.getMock());
+                        Answer<?> defaultAnswer =
+                                new ThrowsSmartNullPointer(invocation, new LocationImpl());
+
                         return Mockito.mock(
-                                type, new ThrowsSmartNullPointer(invocation, new LocationImpl()));
+                                type,
+                                new MockSettingsImpl<>()
+                                        .defaultAnswer(defaultAnswer)
+                                        .mockMaker(mockSettings.getMockMaker()));
                     }
                 });
     }

diff --git a/src/main/java/org/mockito/internal/util/MockUtil.java b/src/main/java/org/mockito/internal/util/MockUtil.java
@@ -20,7 +20,7 @@
 
 import java.util.Collections;
 import java.util.Map;
-import java.util.WeakHashMap;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
 
 import static org.mockito.internal.handler.MockHandlerFactory.createMockHandler;
@@ -29,35 +29,26 @@
 public class MockUtil {
 
     private static final MockMaker defaultMockMaker = Plugins.getMockMaker();
-    private static final Map<Class<? extends MockMaker>, MockMaker> mockMakers;
-
-    static {
-        mockMakers = Collections.synchronizedMap(new WeakHashMap<>());
-        mockMakers.put(defaultMockMaker.getClass(), defaultMockMaker);
-    }
+    private static final Map<Class<? extends MockMaker>, MockMaker> mockMakers =
+            new ConcurrentHashMap<>(
+                    Collections.singletonMap(defaultMockMaker.getClass(), defaultMockMaker));
 
     private MockUtil() {}
 
     private static MockMaker getMockMaker(Class<? extends MockMaker> type) {
         if (type == null) {
             return defaultMockMaker;
-        } else {
-            return mockMakers.computeIfAbsent(
-                    type,
-                    t -> {
-                        try {
-                            return t.getDeclaredConstructor().newInstance();
-                        } catch (Exception e) {
-                            throw new IllegalStateException(
-                                    "Failed to construct MockMaker: " + t, e);
-                        }
-                    });
         }
-    }
-
-    public static TypeMockability typeMockabilityOf(Class<?> type) {
-        // TODO: Maybe we should replace all usages of this method with the method below
-        return defaultMockMaker.isTypeMockable(type);
+        return mockMakers.computeIfAbsent(
+                type,
+                t -> {
+                    try {
+                        return t.getDeclaredConstructor().newInstance();
+                    } catch (Exception e) {
+                        throw new IllegalStateException(
+                                "Failed to construct MockMaker: " + t.getName(), e);
+                    }
+                });
     }
 
     public static TypeMockability typeMockabilityOf(
@@ -99,7 +90,7 @@ public static void resetMock(Object mock) {
     }
 
     public static MockHandler<?> getMockHandler(Object mock) {
-        MockHandler handler = getMockHandler0(mock);
+        MockHandler handler = getMockHandlerOrNull(mock);
         if (handler != null) {
             return handler;
         } else {
@@ -131,10 +122,10 @@ public static boolean isMock(Object mock) {
         if (mock == null) {
             return false;
         }
-        return getMockHandler0(mock) != null;
+        return getMockHandlerOrNull(mock) != null;
     }
 
-    private static MockHandler<?> getMockHandler0(Object mock) {
+    private static MockHandler<?> getMockHandlerOrNull(Object mock) {
         if (mock == null) {
             throw new NotAMockException("Argument should be a mock, but is null!");
         }

diff --git a/src/main/java/org/mockito/internal/util/reflection/FieldInitializer.java b/src/main/java/org/mockito/internal/util/reflection/FieldInitializer.java
@@ -259,8 +259,11 @@ public int compare(Constructor<?> constructorA, Constructor<?> constructorB) {
                     private int countMockableParams(Constructor<?> constructor) {
                         int constructorMockableParamsSize = 0;
                         for (Class<?> aClass : constructor.getParameterTypes()) {
-                            // TODO: Should we somehow use the mockMaker from the context?
-                            if (MockUtil.typeMockabilityOf(aClass).mockable()) {
+                            // TODO: I don't really understand the reason for this check here.
+                            //  It looks like we are not actually creating any mocks for these
+                            //  parameters, but might only use mocks which are already created
+                            //  and stored in some fields.
+                            if (MockUtil.typeMockabilityOf(aClass, null).mockable()) {
                                 constructorMockableParamsSize++;
                             }
                         }