Skip to content

Commit

Permalink
Defining responses for additionalInterfaces doesn't work
Browse files Browse the repository at this point in the history
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 spockframework#1405
  • Loading branch information
AndreasTu committed Aug 7, 2023
1 parent a691632 commit ab3f9e7
Show file tree
Hide file tree
Showing 7 changed files with 165 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
Expand All @@ -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.");
}
Expand All @@ -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<Class<?>> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import spock.lang.Specification;

import java.lang.reflect.Method;
import java.util.Arrays;

import groovy.lang.*;

Expand All @@ -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
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@

package org.spockframework.mock.runtime;

import groovy.lang.GroovyObject;
import org.spockframework.mock.*;
import org.spockframework.runtime.GroovyRuntimeUtil;
import org.spockframework.util.ReflectionUtil;
import org.spockframework.util.SpockDocLinks;
import spock.lang.Specification;

import java.lang.reflect.Modifier;
import java.util.List;

import groovy.lang.MetaClass;

Expand All @@ -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<Class<?>> 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import spock.lang.Specification;

import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Objects;

import groovy.lang.*;

Expand All @@ -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
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 <code>true</code>, 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;
}
}
Original file line number Diff line number Diff line change
@@ -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()
}
}

0 comments on commit ab3f9e7

Please sign in to comment.