Skip to content

Commit

Permalink
Fix defining responses for additionalInterfaces (#1730)
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.

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

Co-authored-by: Leonard Brünings <leonard.bruenings@gradle.com>
  • Loading branch information
AndreasTu and leonard84 authored Oct 3, 2023
1 parent efc6213 commit 8d1e654
Show file tree
Hide file tree
Showing 9 changed files with 252 additions and 17 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);
IMockMaker.IMockCreationSettings mockCreationSettings = MockCreationSettings.settingsFromMockConfiguration(configuration,
mockInterceptor,
specification.getClass().getClassLoader());
mockCreationSettings.getAdditionalInterface().add(GroovyObject.class);
Object proxy = RunContext.get().getMockMakerRegistry().makeMock(mockCreationSettings);

if (hasAdditionalInterfaces) {
//Issue #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,16 @@

package org.spockframework.mock.runtime;

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

import java.util.List;

import groovy.lang.MetaClass;

public class JavaMockFactory implements IMockFactory {
Expand All @@ -44,19 +47,27 @@ public Object createDetached(IMockConfiguration configuration, ClassLoader class
}

private Object createInternal(IMockConfiguration configuration, Specification specification, ClassLoader classLoader) {
Class<?> type = configuration.getType();
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);
MetaClass mockMetaClass = GroovyRuntimeUtil.getMetaClass(type);
JavaMockInterceptor interceptor = new JavaMockInterceptor(configuration, specification, mockMetaClass);
Object proxy = RunContext.get().getMockMakerRegistry().makeMock(MockCreationSettings.settingsFromMockConfiguration(configuration, interceptor, classLoader));
List<Class<?>> additionalInterfaces = configuration.getAdditionalInterfaces();
if (!additionalInterfaces.isEmpty() && GroovyObject.class.isAssignableFrom(type)) {
//Issue #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,6 @@
import spock.lang.Specification;

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

import groovy.lang.*;

Expand All @@ -29,12 +28,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 +54,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
@@ -1,3 +1,19 @@
/*
* 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.util;

import io.leangen.geantyref.GenericTypeReflector;
Expand Down Expand Up @@ -28,7 +44,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,127 @@
/*
* 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"() {
given:
A a = Stub(additionalInterfaces: [B]) {
it.methodFromA() >> { "MockedA" }
it.methodFromIf() >> { "Result" }
}

expect:
a instanceof A
a.methodFromA() == "MockedA"
a instanceof B

and:
B ifType = a as B
ifType.methodFromIf() == "Result"
a.methodFromIf() == "Result"
}

def "Defining responses for additionalInterfaces for Groovy class with GroovyStub"() {
given:
A a = GroovyStub(additionalInterfaces: [B]) {
it.methodFromA() >> { "MockedA" }
it.methodFromIf() >> { "Result" }
}

expect:
a instanceof A
a.methodFromA() == "MockedA"
a instanceof B
a.methodFromIf() == "Result"

and:
B ifType = (B) a
ifType.methodFromIf() == "Result"
}

@Issue("https://github.com/spockframework/spock/issues/1405")
def "Defining responses for additionalInterfaces for Groovy interface"() {
given:
C c = Stub(additionalInterfaces: [B]) {
it.methodFromIfC() >> { "ResultC" }
it.methodFromIf() >> { "ResultB" }
}

expect:
c instanceof C
(c as C).methodFromIfC() == "ResultC"
c instanceof B

and:
B ifType = c as B
ifType.methodFromIf() == "ResultB"
c.methodFromIf() == "ResultB"
}

def "Defining responses for additionalInterfaces for Groovy interface with GroovyStub"() {
given:
C c = GroovyStub(additionalInterfaces: [B]) {
it.methodFromIfC() >> { "ResultC" }
it.methodFromIf() >> { "ResultB" }
}

expect:
c instanceof C
c.methodFromIfC() == "ResultC"
c instanceof B
c.methodFromIf() == "ResultB"

and:
B ifType = (B) c
ifType.methodFromIf() == "ResultB"
}

@Issue("https://github.com/spockframework/spock/issues/1405")
def "Defining responses for additionalInterfaces for Java classes"() {
given:
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()
}
}
Loading

0 comments on commit 8d1e654

Please sign in to comment.