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 25, 2023
1 parent 2fff484 commit 5858cf6
Show file tree
Hide file tree
Showing 9 changed files with 211 additions and 20 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 GenericTypeReflectorUtil.getExactSuperType(mockType, method.getDeclaringClass()) == null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,19 @@ public static Class<?> erase(Type type) {
}

/**
* Resolves the parameter types of the given method/constructor in the given type, with the method {@link GenericTypeReflector#getParameterTypes(Executable, Type)}.
* Finds the most specific supertype of {@code subType} whose erasure is {@code searchSuperClass}, with
* the method {@link GenericTypeReflector#getExactSuperType(Type, Class)}.
*
* @param subType the subType which we search the super type for
* @param searchSuperClass the super type to search for
* @return the common super type or {@code null} if non was found
*/
public static Type getExactSuperType(Type subType, Class<?> searchSuperClass) {
return GenericTypeReflector.getExactSuperType(subType, searchSuperClass);
}

/**
* Resolves the parameter types of the given method/constructor in the given type, with the method {@link GenericTypeReflector#getParameterTypes(Executable, Type)}.
*
* <p>In difference to the method of the library, this method will resolve unknown {@link java.lang.reflect.TypeVariable}s to the upper bound of the type variable.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* Copyright 2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

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:
A a = Stub(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:
A a = GroovyStub(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:
C c = Stub(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:
C c = GroovyStub(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:
ArrayList a = Stub(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()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import java.util.function.Function
class GenericTypeReflectorUtilSpec extends Specification {
private static final Type FUNC_STRING_INT_TYPE = new TypeToken<Function<String, Integer>>() {}.type
private static final Type LIST_STRING_TYPE = new TypeToken<List<String>>() {}.type
private static final Type ARRAY_LIST_STRING_TYPE = new TypeToken<ArrayList<String>>() {}.type
private static final Type LIST_LIST_STRING_TYPE = new TypeToken<List<List<String>>>() {}.type
private static final Type CLASS_SERIALIZABLE = new TypeToken<Class<Serializable>>() {}.type

Expand All @@ -30,6 +31,22 @@ class GenericTypeReflectorUtilSpec extends Specification {
LIST_LIST_STRING_TYPE | List
}

def "getExactSuperType"(Type inputSubType, Class<?> inputSuperClass, @Nullable Type expectedType) {
expect:
GenericTypeReflectorUtil.getExactSuperType(inputSubType, inputSuperClass) == expectedType

where:
inputSubType | inputSuperClass | expectedType
Object | Object | Object
StringBuilder | StringBuilder | StringBuilder
StringBuilder | Appendable | Appendable
Runnable | ArrayList | null
ArrayList | AbstractList | AbstractList
List | AbstractList | null
LIST_STRING_TYPE | AbstractList | null
ARRAY_LIST_STRING_TYPE | List | LIST_STRING_TYPE
}

def "getTypeName"(Type inputType, String expectedName) {
expect:
GenericTypeReflectorUtil.getTypeName(inputType) == expectedName
Expand Down

0 comments on commit 5858cf6

Please sign in to comment.