From ab3f9e76b038620990a8ba7955087dcedf7375ab Mon Sep 17 00:00:00 2001 From: AndreasTu Date: Mon, 31 Jul 2023 14:45:32 +0200 Subject: [PATCH] Defining responses for additionalInterfaces doesn't work The metaClass used in JavaMocks of Groovy classes did not contain the methods of additionalInterfaces, so the fix is to change the MetaClass, if we are a GroovyObject and have additionalInterfaces to the mocked MetaClass. It is not the best solution, because we now use the MetaClass of the Mock, which also contains methods and interfaces of Spock internal, but there is currently no class implementing mockType + additionalInterfaces. This would be a bigger rework. Moved the metaClass field down to BaseMockInterceptor to use it in both cases: GroovyMockInterceptor and JavaMockInterceptor The second fix is for the JavaMock of a Java class with additionalInterfaces. Previously the mockType of the Java class was used also for the methods of the additionalInterfaces, which lead to a NullPointerException. This fixes #1405 --- .../mock/runtime/BaseMockInterceptor.java | 15 +++ .../mock/runtime/GroovyMockFactory.java | 16 ++- .../mock/runtime/GroovyMockInterceptor.java | 6 +- .../mock/runtime/JavaMockFactory.java | 25 +++-- .../mock/runtime/JavaMockInterceptor.java | 7 +- .../mock/runtime/StaticMockMethod.java | 17 +++- .../AdditionalInterfaceResponseSpec.groovy | 98 +++++++++++++++++++ 7 files changed, 165 insertions(+), 19 deletions(-) create mode 100644 spock-specs/src/test/groovy/org/spockframework/mock/AdditionalInterfaceResponseSpec.groovy diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java b/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java index 69836790c6..5a4494d1c8 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java @@ -4,11 +4,26 @@ import java.lang.reflect.Method; import java.util.Arrays; +import java.util.Objects; import groovy.lang.*; import org.jetbrains.annotations.Nullable; public abstract class BaseMockInterceptor implements IProxyBasedMockInterceptor { + private MetaClass mockMetaClass; + + BaseMockInterceptor(MetaClass mockMetaClass) { + this.mockMetaClass = mockMetaClass; + } + + protected MetaClass getMockMetaClass() { + return mockMetaClass; + } + + public void setMetaClass(MetaClass mockMetaClass) { + this.mockMetaClass = Objects.requireNonNull(mockMetaClass); + } + @Nullable protected String handleGetProperty(GroovyObject target, Object[] args) { // Another hack It should be replaced with something more reliable: https://github.com/spockframework/spock/issues/1076 diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockFactory.java b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockFactory.java index b134734546..efec25bd01 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockFactory.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockFactory.java @@ -39,12 +39,13 @@ public Object create(IMockConfiguration configuration, Specification specificati GroovyMockMetaClass newMetaClass = new GroovyMockMetaClass(configuration, specification, oldMetaClass); final Class type = configuration.getType(); + boolean hasAdditionalInterfaces = !configuration.getAdditionalInterfaces().isEmpty(); if (configuration.isGlobal()) { if (type.isInterface()) { throw new CannotCreateMockException(type, ". Global mocking is only possible for classes, but not for interfaces."); } - if (!configuration.getAdditionalInterfaces().isEmpty()) { + if (hasAdditionalInterfaces) { throw new CannotCreateMockException(type, ". Global cannot add additionalInterfaces."); } @@ -54,7 +55,7 @@ public Object create(IMockConfiguration configuration, Specification specificati } if (isFinalClass(type)) { - if (!configuration.getAdditionalInterfaces().isEmpty()) { + if (hasAdditionalInterfaces) { throw new CannotCreateMockException(type, ". Cannot add additionalInterfaces to final classes."); } @@ -65,12 +66,21 @@ public Object create(IMockConfiguration configuration, Specification specificati return instance; } - IProxyBasedMockInterceptor mockInterceptor = new GroovyMockInterceptor(configuration, specification, newMetaClass); + GroovyMockInterceptor mockInterceptor = new GroovyMockInterceptor(configuration, specification, newMetaClass); List> additionalInterfaces = new ArrayList<>(configuration.getAdditionalInterfaces()); additionalInterfaces.add(GroovyObject.class); Object proxy = ProxyBasedMockFactory.INSTANCE.create(type, additionalInterfaces, configuration.getConstructorArgs(), mockInterceptor, specification.getClass().getClassLoader(), configuration.isUseObjenesis()); + + if (hasAdditionalInterfaces) { + //Ticket #1405: We need to update the mockMetaClass to reflect the methods of the additional interfaces + // The MetaClass of the mock is a bit too much, but we do not have a class representing the hierarchy without the internal Spock interfaces like ISpockMockObject + MetaClass oldMetaClassOfProxy = GroovyRuntimeUtil.getMetaClass(proxy.getClass()); + GroovyMockMetaClass mockMetaClass = new GroovyMockMetaClass(configuration, specification, oldMetaClassOfProxy); + mockInterceptor.setMetaClass(mockMetaClass); + } + if ((configuration.getNature() == MockNature.SPY) && (configuration.getInstance() != null)) { try { ReflectionUtil.deepCopyFields(configuration.getInstance(), proxy); diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockInterceptor.java b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockInterceptor.java index 217d19d853..48d46fd64b 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockInterceptor.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockInterceptor.java @@ -19,7 +19,6 @@ import spock.lang.Specification; import java.lang.reflect.Method; -import java.util.Arrays; import groovy.lang.*; @@ -28,12 +27,11 @@ public class GroovyMockInterceptor extends BaseMockInterceptor { private final IMockConfiguration mockConfiguration; private final Specification specification; - private final MetaClass mockMetaClass; public GroovyMockInterceptor(IMockConfiguration mockConfiguration, Specification specification, MetaClass mockMetaClass) { + super(mockMetaClass); this.mockConfiguration = mockConfiguration; this.specification = specification; - this.mockMetaClass = mockMetaClass; } @Override @@ -50,7 +48,7 @@ public Object intercept(Object target, Method method, Object[] arguments, IRespo Object[] args = GroovyRuntimeUtil.asUnwrappedArgumentArray(arguments); if (isMethod(method, "getMetaClass")) { - return mockMetaClass; + return getMockMetaClass(); } if (isMethod(method, "invokeMethod", String.class, Object.class)) { return GroovyRuntimeUtil.invokeMethod(target, diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/JavaMockFactory.java b/spock-core/src/main/java/org/spockframework/mock/runtime/JavaMockFactory.java index 8306978883..a0aa76ac5c 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/JavaMockFactory.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/JavaMockFactory.java @@ -16,6 +16,7 @@ package org.spockframework.mock.runtime; +import groovy.lang.GroovyObject; import org.spockframework.mock.*; import org.spockframework.runtime.GroovyRuntimeUtil; import org.spockframework.util.ReflectionUtil; @@ -23,6 +24,7 @@ import spock.lang.Specification; import java.lang.reflect.Modifier; +import java.util.List; import groovy.lang.MetaClass; @@ -45,25 +47,34 @@ public Object createDetached(IMockConfiguration configuration, ClassLoader class } private Object createInternal(IMockConfiguration configuration, Specification specification, ClassLoader classLoader) { - if (Modifier.isFinal(configuration.getType().getModifiers())) { - throw new CannotCreateMockException(configuration.getType(), + final Class type = configuration.getType(); + if (Modifier.isFinal(type.getModifiers())) { + throw new CannotCreateMockException(type, " because Java mocks cannot mock final classes. If the code under test is written in Groovy, use a Groovy mock."); } if (configuration.isGlobal()) { - throw new CannotCreateMockException(configuration.getType(), + throw new CannotCreateMockException(type, " because Java mocks cannot mock globally. If the code under test is written in Groovy, use a Groovy mock."); } - MetaClass mockMetaClass = GroovyRuntimeUtil.getMetaClass(configuration.getType()); - IProxyBasedMockInterceptor interceptor = new JavaMockInterceptor(configuration, specification, mockMetaClass); - Object proxy = ProxyBasedMockFactory.INSTANCE.create(configuration.getType(), configuration.getAdditionalInterfaces(), + MetaClass mockMetaClass = GroovyRuntimeUtil.getMetaClass(type); + JavaMockInterceptor interceptor = new JavaMockInterceptor(configuration, specification, mockMetaClass); + List> additionalInterfaces = configuration.getAdditionalInterfaces(); + Object proxy = ProxyBasedMockFactory.INSTANCE.create(type, additionalInterfaces, configuration.getConstructorArgs(), interceptor, classLoader, configuration.isUseObjenesis()); + + if (!additionalInterfaces.isEmpty() && GroovyObject.class.isAssignableFrom(type)) { + //Ticket #1405: We need to update the mockMetaClass to reflect the methods of the additional interfaces + // The MetaClass of the mock is a bit too much, but we do not have a class representing the hierarchy without the internal Spock interfaces like ISpockMockObject + interceptor.setMetaClass(GroovyRuntimeUtil.getMetaClass(proxy.getClass())); + } + if ((configuration.getNature() == MockNature.SPY) && (configuration.getInstance() != null)) { try { ReflectionUtil.deepCopyFields(configuration.getInstance(), proxy); } catch (Exception e) { - throw new CannotCreateMockException(configuration.getType(), + throw new CannotCreateMockException(type, ". Cannot copy fields.\n" + SpockDocLinks.SPY_ON_JAVA_17.getLink(), e); } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/JavaMockInterceptor.java b/spock-core/src/main/java/org/spockframework/mock/runtime/JavaMockInterceptor.java index 649f211f19..44ffba3bb8 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/JavaMockInterceptor.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/JavaMockInterceptor.java @@ -19,7 +19,7 @@ import spock.lang.Specification; import java.lang.reflect.Method; -import java.util.Arrays; +import java.util.Objects; import groovy.lang.*; @@ -29,12 +29,11 @@ public class JavaMockInterceptor extends BaseMockInterceptor { private final IMockConfiguration mockConfiguration; private Specification specification; private MockController fallbackMockController; - private final MetaClass mockMetaClass; public JavaMockInterceptor(IMockConfiguration mockConfiguration, Specification specification, MetaClass mockMetaClass) { + super(mockMetaClass); this.mockConfiguration = mockConfiguration; this.specification = specification; - this.mockMetaClass = mockMetaClass; } @Override @@ -56,7 +55,7 @@ public Object intercept(Object target, Method method, Object[] arguments, IRespo if (target instanceof GroovyObject) { if (isMethod(method, "getMetaClass")) { - return mockMetaClass; + return getMockMetaClass(); } if (isMethod(method, "setProperty", String.class, Object.class)) { Throwable throwable = new Throwable(); diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/StaticMockMethod.java b/spock-core/src/main/java/org/spockframework/mock/runtime/StaticMockMethod.java index 5c4f04d7d2..efee0d4360 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/StaticMockMethod.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/StaticMockMethod.java @@ -29,7 +29,12 @@ public class StaticMockMethod implements IMockMethod { public StaticMockMethod(Method method, Type mockType) { this.method = method; - this.mockType = mockType; + if (isMethodFromAdditionalInterfaces(mockType, method)) { + //We need to switch the mockType to the additional interface type, because otherwise the type resolution will fail with NPE + this.mockType = method.getDeclaringClass(); + } else { + this.mockType = mockType; + } } @Override @@ -61,4 +66,14 @@ public Type getExactReturnType() { public boolean isStatic() { return Modifier.isStatic(method.getModifiers()); } + + /** + * @param mockType the mocked type + * @param method the intercepted method + * @return true, if the method is not part of the hierarchy of the mocked type. + */ + private static boolean isMethodFromAdditionalInterfaces(Type mockType, Method method) { + //We have a method from additional interfaces of the mock, because there was no common type found. + return GenericTypeReflector.getExactSuperType(GenericTypeReflector.capture(mockType), method.getDeclaringClass()) == null; + } } diff --git a/spock-specs/src/test/groovy/org/spockframework/mock/AdditionalInterfaceResponseSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/mock/AdditionalInterfaceResponseSpec.groovy new file mode 100644 index 0000000000..263aab1faf --- /dev/null +++ b/spock-specs/src/test/groovy/org/spockframework/mock/AdditionalInterfaceResponseSpec.groovy @@ -0,0 +1,98 @@ +package org.spockframework.mock + +import spock.lang.Issue +import spock.lang.Specification + +import java.util.function.IntSupplier + +class AdditionalInterfaceResponseSpec extends Specification { + + @Issue("https://github.com/spockframework/spock/issues/1405") + def "Defining responses for additionalInterfaces for Groovy class Ticket #1405"() { + setup: + def a = Stub(A, additionalInterfaces: [B]) { + it.methodFromA() >> { "MockedA" } + it.methodFromIf() >> { "Result" } + } + expect: + a instanceof A + a.methodFromA() == "MockedA" + a instanceof B + B ifType = a as B + ifType.methodFromIf() == "Result" + a.methodFromIf() == "Result" + } + + def "Defining responses for additionalInterfaces for Groovy class with GroovyStub"() { + setup: + def a = GroovyStub(A, additionalInterfaces: [B]) { + it.methodFromA() >> { "MockedA" } + it.methodFromIf() >> { "Result" } + } + expect: + a instanceof A + a.methodFromA() == "MockedA" + a instanceof B + a.methodFromIf() == "Result" + B ifType = (B) a + ifType.methodFromIf() == "Result" + } + + @Issue("https://github.com/spockframework/spock/issues/1405") + def "Defining responses for additionalInterfaces for Groovy interface Ticket #1405"() { + setup: + def c = Stub(C, additionalInterfaces: [B]) { + it.methodFromIfC() >> { "ResultC" } + it.methodFromIf() >> { "ResultB" } + } + expect: + c instanceof C + (c as C).methodFromIfC() == "ResultC" + c instanceof B + B ifType = c as B + ifType.methodFromIf() == "ResultB" + c.methodFromIf() == "ResultB" + } + + def "Defining responses for additionalInterfaces for Groovy interface with GroovyStub"() { + setup: + def c = GroovyStub(C, additionalInterfaces: [B]) { + it.methodFromIfC() >> { "ResultC" } + it.methodFromIf() >> { "ResultB" } + } + expect: + c instanceof C + c.methodFromIfC() == "ResultC" + c instanceof B + c.methodFromIf() == "ResultB" + B ifType = (B) c + ifType.methodFromIf() == "ResultB" + } + + @Issue("https://github.com/spockframework/spock/issues/1405") + def "Defining responses for additionalInterfaces for Java classes Ticket #1405"() { + setup: + def a = Stub(ArrayList, additionalInterfaces: [IntSupplier]) { + it.getAsInt() >> { 5 } + } + expect: + a instanceof ArrayList + a instanceof IntSupplier + a.getAsInt() == 5 + } + + static class A { + @SuppressWarnings('GrMethodMayBeStatic') + String methodFromA() { + return "RealA" + } + } + + interface B { + String methodFromIf() + } + + interface C { + String methodFromIfC() + } +}