From 2b544420497acc69a881fa62525613f5135fe474 Mon Sep 17 00:00:00 2001 From: AndreasTu Date: Thu, 7 Nov 2024 13:01:29 +0100 Subject: [PATCH] Best-effort error reporting for interactions on final methods Add error reporting code to the byte-buddy` mock maker to report interaction on final methods, which can't be handled. The IMockInteractionValidation interface allows IMockMakers to implement different kinds of validations on mock interactions. Fixes #2039 --- docs/release_notes.adoc | 1 + .../org/spockframework/compiler/AstUtil.java | 1 - .../spockframework/compiler/SpecRewriter.java | 7 +- .../org/spockframework/mock/IMockObject.java | 9 ++ .../spockframework/mock/ISpockMockObject.java | 22 ++- .../org/spockframework/mock/MockUtil.java | 13 +- .../constraint/EqualMethodNameConstraint.java | 4 + .../EqualPropertyNameConstraint.java | 4 + .../mock/constraint/TargetConstraint.java | 21 ++- .../mock/runtime/BaseMockInterceptor.java | 18 ++- .../mock/runtime/ByteBuddyMockFactory.java | 11 ++ .../ByteBuddyMockInteractionValidation.java | 135 ++++++++++++++++++ .../mock/runtime/GroovyMockInterceptor.java | 7 +- .../mock/runtime/GroovyMockMetaClass.java | 12 +- .../runtime/IMockInteractionValidation.java | 47 ++++++ .../mock/runtime/JavaMockInterceptor.java | 8 +- .../mock/runtime/MockInteraction.java | 4 + .../mock/runtime/MockObject.java | 46 +++--- .../runtime/GroovyRuntimeUtil.java | 8 +- .../runtime/GroovyRuntimeUtilSpec.groovy | 3 +- .../smoke/mock/JavaSpies.groovy | 12 +- .../smoke/mock/JavaStubs.groovy | 83 ++++++++++- .../smoke/mock/MockDefaultResponses.groovy | 12 ++ .../spring/mock/DelegatingInterceptor.java | 8 +- 24 files changed, 442 insertions(+), 54 deletions(-) create mode 100644 spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockInteractionValidation.java create mode 100644 spock-core/src/main/java/org/spockframework/mock/runtime/IMockInteractionValidation.java diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc index d52a9c7324..5722942ff4 100644 --- a/docs/release_notes.adoc +++ b/docs/release_notes.adoc @@ -26,6 +26,7 @@ include::include.adoc[] * Size of data providers is no longer calculated multiple times but only once * Fix exception when using `@RepeatUntilFailure` with a data provider with unknown iteration amount. spockPull:2031[] * Clarified documentation about data providers and `size()` calls spockIssue:2022[] +* Add best-effort error reporting for interactions on final methods when using the `byte-buddy` mock maker spockIssue:2039[] == 2.4-M4 (2024-03-21) diff --git a/spock-core/src/main/java/org/spockframework/compiler/AstUtil.java b/spock-core/src/main/java/org/spockframework/compiler/AstUtil.java index 61c1287c40..9ada98adba 100644 --- a/spock-core/src/main/java/org/spockframework/compiler/AstUtil.java +++ b/spock-core/src/main/java/org/spockframework/compiler/AstUtil.java @@ -40,7 +40,6 @@ */ public abstract class AstUtil { private static final Pattern DATA_TABLE_SEPARATOR = Pattern.compile("_{2,}+"); - private static final String GET_METHOD_NAME = "get"; private static final String GET_AT_METHOD_NAME = new IntegerArrayGetAtMetaMethod().getName(); /** diff --git a/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java b/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java index 3b02908707..7ae3edefd4 100644 --- a/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java +++ b/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java @@ -33,6 +33,9 @@ import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.spockframework.compiler.AstUtil.createDirectMethodCall; +import static org.spockframework.runtime.GroovyRuntimeUtil.GET; +import static org.spockframework.runtime.GroovyRuntimeUtil.propertyToMethodName; +import static org.spockframework.runtime.GroovyRuntimeUtil.SET; /** * A Spec visitor responsible for most of the rewriting of a Spec's AST. @@ -135,7 +138,7 @@ private void createSharedFieldGetter(Field field) { } private void createFinalFieldGetter(Field field) { - String getterName = "get" + MetaClassHelper.capitalize(field.getName()); + String getterName = propertyToMethodName(GET, field.getName()); MethodNode getter = spec.getAst().getMethod(getterName, Parameter.EMPTY_ARRAY); if (getter != null) { errorReporter.error(field.getAst(), @@ -158,7 +161,7 @@ private void createFinalFieldGetter(Field field) { } private void createSharedFieldSetter(Field field) { - String setterName = "set" + MetaClassHelper.capitalize(field.getName()); + String setterName = propertyToMethodName(SET, field.getName()); Parameter[] params = new Parameter[] { new Parameter(field.getAst().getType(), "$spock_value") }; MethodNode setter = spec.getAst().getMethod(setterName, params); if (setter != null) { diff --git a/spock-core/src/main/java/org/spockframework/mock/IMockObject.java b/spock-core/src/main/java/org/spockframework/mock/IMockObject.java index 44407da2e8..77ad216114 100644 --- a/spock-core/src/main/java/org/spockframework/mock/IMockObject.java +++ b/spock-core/src/main/java/org/spockframework/mock/IMockObject.java @@ -15,6 +15,7 @@ package org.spockframework.mock; import org.spockframework.mock.runtime.SpecificationAttachable; +import org.spockframework.util.Beta; import org.spockframework.util.Nullable; import spock.lang.Specification; @@ -81,4 +82,12 @@ public interface IMockObject extends SpecificationAttachable { * @return whether this mock object matches the target of the specified interaction */ boolean matches(Object target, IMockInteraction interaction); + + /** + * Returns the used mock configuration which created this mock. + * + * @return the mock configuration + */ + @Beta + IMockConfiguration getConfiguration(); } diff --git a/spock-core/src/main/java/org/spockframework/mock/ISpockMockObject.java b/spock-core/src/main/java/org/spockframework/mock/ISpockMockObject.java index c9e1595960..41ac0dc6b0 100644 --- a/spock-core/src/main/java/org/spockframework/mock/ISpockMockObject.java +++ b/spock-core/src/main/java/org/spockframework/mock/ISpockMockObject.java @@ -14,10 +14,28 @@ package org.spockframework.mock; +import org.spockframework.mock.runtime.IMockInteractionValidation; +import org.spockframework.util.Beta; +import org.spockframework.util.Nullable; + /** - * Marker-like interface implemented by all mock objects that allows - * {@link MockUtil} to detect mock objects. Not intended for direct use. + * MockObject interface implemented by some {@link spock.mock.MockMakers} that allows the {@link org.spockframework.mock.runtime.MockMakerRegistry} + * to detect mock objects. + * + *

Not intended for direct use. */ public interface ISpockMockObject { + IMockObject $spock_get(); + + /** + * @return the {@link IMockInteractionValidation} used to verify {@link IMockInteraction} + * @see IMockInteractionValidation + * @since 2.4 + */ + @Nullable + @Beta + default IMockInteractionValidation $spock_mockInteractionValidation() { + return null; + } } diff --git a/spock-core/src/main/java/org/spockframework/mock/MockUtil.java b/spock-core/src/main/java/org/spockframework/mock/MockUtil.java index 745525cd80..aa2e943fd1 100644 --- a/spock-core/src/main/java/org/spockframework/mock/MockUtil.java +++ b/spock-core/src/main/java/org/spockframework/mock/MockUtil.java @@ -35,7 +35,7 @@ public class MockUtil { * @return whether the given object is a (Spock) mock object */ public boolean isMock(Object object) { - return getMockMakerRegistry().asMockOrNull(object) != null; + return asMockOrNull(object) != null; } /** @@ -69,6 +69,17 @@ public IMockObject asMock(Object object) { return mockOrNull; } + /** + * Returns information about a mock object or {@code null}, if the object is not a mock. + * + * @param object a mock object + * @return information about the mock object or {@code null}, if the object is not a mock. + */ + @Nullable + public IMockObject asMockOrNull(Object object) { + return getMockMakerRegistry().asMockOrNull(object); + } + /** * Attaches mock to a Specification. * diff --git a/spock-core/src/main/java/org/spockframework/mock/constraint/EqualMethodNameConstraint.java b/spock-core/src/main/java/org/spockframework/mock/constraint/EqualMethodNameConstraint.java index 020120e600..82196a538f 100644 --- a/spock-core/src/main/java/org/spockframework/mock/constraint/EqualMethodNameConstraint.java +++ b/spock-core/src/main/java/org/spockframework/mock/constraint/EqualMethodNameConstraint.java @@ -31,6 +31,10 @@ public EqualMethodNameConstraint(String methodName) { this.methodName = methodName; } + public String getMethodName() { + return methodName; + } + @Override public boolean isSatisfiedBy(IMockInvocation invocation) { return invocation.getMethod().getName().equals(methodName); diff --git a/spock-core/src/main/java/org/spockframework/mock/constraint/EqualPropertyNameConstraint.java b/spock-core/src/main/java/org/spockframework/mock/constraint/EqualPropertyNameConstraint.java index 737916baef..eb21660dff 100644 --- a/spock-core/src/main/java/org/spockframework/mock/constraint/EqualPropertyNameConstraint.java +++ b/spock-core/src/main/java/org/spockframework/mock/constraint/EqualPropertyNameConstraint.java @@ -29,6 +29,10 @@ public EqualPropertyNameConstraint(String propertyName) { this.propertyName = propertyName; } + public String getPropertyName() { + return propertyName; + } + @Override public boolean isSatisfiedBy(IMockInvocation invocation) { return propertyName.equals(getPropertyName(invocation)); diff --git a/spock-core/src/main/java/org/spockframework/mock/constraint/TargetConstraint.java b/spock-core/src/main/java/org/spockframework/mock/constraint/TargetConstraint.java index 5df131f027..80525414f7 100644 --- a/spock-core/src/main/java/org/spockframework/mock/constraint/TargetConstraint.java +++ b/spock-core/src/main/java/org/spockframework/mock/constraint/TargetConstraint.java @@ -17,6 +17,7 @@ package org.spockframework.mock.constraint; import org.spockframework.mock.*; +import org.spockframework.mock.runtime.IMockInteractionValidation; import org.spockframework.runtime.Condition; import org.spockframework.runtime.InvalidSpecException; import org.spockframework.util.CollectionUtil; @@ -50,8 +51,15 @@ public String describeMismatch(IMockInvocation invocation) { @Override public void setInteraction(IMockInteraction interaction) { this.interaction = interaction; - if (interaction.isRequired() && MOCK_UTIL.isMock(target)) { - IMockObject mockObject = MOCK_UTIL.asMock(target); + IMockObject mockObject = MOCK_UTIL.asMockOrNull(target); + if (mockObject != null) { + checkRequiredInteraction(mockObject, interaction); + validateMockInteraction(mockObject, interaction); + } + } + + private void checkRequiredInteraction(IMockObject mockObject, IMockInteraction interaction) { + if (interaction.isRequired()) { if (!mockObject.isVerified()) { String mockName = mockObject.getName() != null ? mockObject.getName() : "unnamed"; throw new InvalidSpecException("Stub '%s' matches the following required interaction:" + @@ -59,4 +67,13 @@ public void setInteraction(IMockInteraction interaction) { } } } + + private void validateMockInteraction(IMockObject mockObject, IMockInteraction interaction) { + if (target instanceof ISpockMockObject) { + IMockInteractionValidation validation = ((ISpockMockObject) target).$spock_mockInteractionValidation(); + if (validation != null) { + validation.validateMockInteraction(mockObject, interaction); + } + } + } } 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 3af8948528..1c0de26b8f 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 @@ -1,5 +1,7 @@ package org.spockframework.mock.runtime; +import org.spockframework.mock.IMockObject; +import org.spockframework.mock.ISpockMockObject; import org.spockframework.runtime.GroovyRuntimeUtil; import java.lang.reflect.Method; @@ -8,8 +10,13 @@ import groovy.lang.*; import org.jetbrains.annotations.Nullable; +import org.spockframework.util.ReflectionUtil; + +import static java.util.Objects.requireNonNull; public abstract class BaseMockInterceptor implements IProxyBasedMockInterceptor { + static final Method MOCK_INTERACTION_VALIDATION_METHOD = requireNonNull(ReflectionUtil.getMethodByName(ISpockMockObject.class, "$spock_mockInteractionValidation")); + private MetaClass mockMetaClass; BaseMockInterceptor(MetaClass mockMetaClass) { @@ -39,11 +46,11 @@ protected String handleGetProperty(GroovyObject target, Object[] args) { MetaClass metaClass = target.getMetaClass(); //First try the isXXX before getXXX, because this is the expected behavior, if it is boolean property. MetaMethod booleanVariant = metaClass - .getMetaMethod(GroovyRuntimeUtil.propertyToMethodName("is", propertyName), GroovyRuntimeUtil.EMPTY_ARGUMENTS); + .getMetaMethod(GroovyRuntimeUtil.propertyToMethodName(GroovyRuntimeUtil.IS, propertyName), GroovyRuntimeUtil.EMPTY_ARGUMENTS); if (booleanVariant != null && booleanVariant.getReturnType() == boolean.class) { methodName = booleanVariant.getName(); } else { - methodName = GroovyRuntimeUtil.propertyToMethodName("get", propertyName); + methodName = GroovyRuntimeUtil.propertyToMethodName(GroovyRuntimeUtil.GET, propertyName); } } return methodName; @@ -52,4 +59,11 @@ protected String handleGetProperty(GroovyObject target, Object[] args) { protected boolean isMethod(Method method, String name, Class... parameterTypes) { return method.getName().equals(name) && Arrays.equals(method.getParameterTypes(), parameterTypes); } + + public static Object handleSpockMockInterface(Method method, IMockObject mockObject) { + if (method.getName().equals(MOCK_INTERACTION_VALIDATION_METHOD.getName())) { + return null; //This should be handled in the corresponding MockMakers. + } + return mockObject; + } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java index 0e4086aa5f..3dc319781f 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java @@ -22,6 +22,7 @@ import net.bytebuddy.TypeCache; import net.bytebuddy.description.annotation.AnnotationDescription; import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.modifier.MethodManifestation; import net.bytebuddy.description.modifier.SynchronizationState; import net.bytebuddy.description.modifier.SyntheticState; import net.bytebuddy.description.modifier.Visibility; @@ -31,6 +32,7 @@ import net.bytebuddy.dynamic.loading.MultipleParentClassLoader; import net.bytebuddy.dynamic.scaffold.TypeValidation; import net.bytebuddy.implementation.FieldAccessor; +import net.bytebuddy.implementation.FixedValue; import net.bytebuddy.implementation.MethodDelegation; import net.bytebuddy.implementation.bind.annotation.Morph; import org.codehaus.groovy.runtime.callsite.AbstractCallSite; @@ -45,6 +47,7 @@ import java.util.concurrent.ThreadLocalRandom; import static net.bytebuddy.matcher.ElementMatchers.none; +import static org.spockframework.mock.runtime.BaseMockInterceptor.MOCK_INTERACTION_VALIDATION_METHOD; class ByteBuddyMockFactory { /** @@ -136,6 +139,10 @@ Object createMock(IMockMaker.IMockCreationSettings settings) { .method(m -> !isGroovyMOPMethod(type, m)) .intercept(mockInterceptor()) .transform(mockTransformer()) + .method(m -> m.represents(MOCK_INTERACTION_VALIDATION_METHOD)) + // Implement the $spock_mockInteractionValidation() method which returns the static field below, so we have a validation instance for every mock class + .intercept(FixedValue.reference(new ByteBuddyMockInteractionValidation(), MOCK_INTERACTION_VALIDATION_METHOD.getName())) + .transform(validateMockInteractionTransformer()) .implement(ByteBuddyInterceptorAdapter.InterceptorAccess.class) .intercept(FieldAccessor.ofField("$spock_interceptor")) .defineField("$spock_interceptor", IProxyBasedMockInterceptor.class, Visibility.PRIVATE, SyntheticState.SYNTHETIC) @@ -149,6 +156,10 @@ Object createMock(IMockMaker.IMockCreationSettings settings) { return proxy; } + private static Transformer validateMockInteractionTransformer() { + return Transformer.ForMethod.withModifiers(SynchronizationState.PLAIN, Visibility.PUBLIC, MethodManifestation.FINAL); + } + private static Transformer mockTransformer() { return Transformer.ForMethod.withModifiers(SynchronizationState.PLAIN, Visibility.PUBLIC); //Overridden methods should be public and non-synchronized. } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockInteractionValidation.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockInteractionValidation.java new file mode 100644 index 0000000000..0022d13f7b --- /dev/null +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockInteractionValidation.java @@ -0,0 +1,135 @@ +/* + * Copyright 2024 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.runtime; + +import org.junit.platform.commons.support.ModifierSupport; +import org.spockframework.mock.IInvocationConstraint; +import org.spockframework.mock.IMockInteraction; +import org.spockframework.mock.IMockObject; +import org.spockframework.mock.MockImplementation; +import org.spockframework.mock.constraint.EqualMethodNameConstraint; +import org.spockframework.mock.constraint.EqualPropertyNameConstraint; +import org.spockframework.mock.constraint.NamedArgumentListConstraint; +import org.spockframework.mock.constraint.PositionalArgumentListConstraint; +import org.spockframework.runtime.InvalidSpecException; +import org.spockframework.util.ThreadSafe; + +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +import static java.util.Objects.requireNonNull; +import static org.spockframework.runtime.GroovyRuntimeUtil.GET; +import static org.spockframework.runtime.GroovyRuntimeUtil.IS; +import static org.spockframework.runtime.GroovyRuntimeUtil.SET; +import static org.spockframework.runtime.GroovyRuntimeUtil.propertyToMethodName; + +/** + * {@link ByteBuddyMockInteractionValidation} validates the {@link IMockInteraction} for {@link ByteBuddyMockMaker} mocks. + * + *

+ * + *

Implementation note: The {@link ByteBuddyMockFactory} create a single instance for each mocked class + * and the same validation is used for multiple mocks of the same class.

+ * + * @author Andreas Turban + */ +@ThreadSafe +final class ByteBuddyMockInteractionValidation implements IMockInteractionValidation { + + private volatile Class mockClass; + private volatile Set finalMethods; + + ByteBuddyMockInteractionValidation() { + } + + @Override + public void validateMockInteraction(IMockObject mockObject, IMockInteraction mockInteractionParam) { + requireNonNull(mockObject); + requireNonNull(mockInteractionParam); + Object instance = requireNonNull(mockObject.getInstance()); + + if (mockObject.getConfiguration().getImplementation() == MockImplementation.GROOVY) { + //We do not validate final methods for Groovy mocks, because final mocking can be done with the Groovy MOP. + return; + } + + initializeClassData(instance); + + validate((MockInteraction) mockInteractionParam); + } + + private void validate(MockInteraction mockInteraction) { + for (IInvocationConstraint constraint : mockInteraction.getConstraints()) { + if (constraint instanceof EqualMethodNameConstraint) { + EqualMethodNameConstraint methodConstraint = (EqualMethodNameConstraint) constraint; + String methodName = methodConstraint.getMethodName(); + validateNonFinalMethod(methodName); + } + if (constraint instanceof EqualPropertyNameConstraint) { + EqualPropertyNameConstraint propNameConstraint = (EqualPropertyNameConstraint) constraint; + validateProperty(propNameConstraint); + } + } + } + + private void validateProperty(EqualPropertyNameConstraint propNameConstraint) { + if (propNameConstraint != null) { + String propName = propNameConstraint.getPropertyName(); + //We do not need to check for setters, because a property access like x.prop = value has not mock interaction syntax + validateNonFinalMethod(propertyToMethodName(GET, propName)); + validateNonFinalMethod(propertyToMethodName(IS, propName)); + } + } + + private void validateNonFinalMethod(String methodName) { + if (finalMethods.contains(methodName)) { + throw new InvalidSpecException("The final method '" + methodName + "' can't be mocked by the " + ByteBuddyMockMaker.ID + " mock maker. Please use another mock maker supporting final methods."); + } + } + + private void initializeClassData(Object mock) { + Class mockClassOfMockObj = mock.getClass(); + if (mockClass == null) { + synchronized (this) { + if (mockClass == null) { + finalMethods = Arrays.stream(mockClassOfMockObj.getMethods()) + .filter(m -> !ModifierSupport.isStatic(m)) + .collect(Collectors.groupingBy(Method::getName)) + .entrySet() + .stream() + .filter(e -> e.getValue() + .stream() + .allMatch(ModifierSupport::isFinal)) + .map(Map.Entry::getKey) + .collect(Collectors.toSet()); + + mockClass = mockClassOfMockObj; + } + } + } + + if (mockClass != mockClassOfMockObj) { + throw new IllegalStateException(); + } + } +} 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 48d46fd64b..4a071a1374 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 @@ -36,11 +36,10 @@ public GroovyMockInterceptor(IMockConfiguration mockConfiguration, Specification @Override public Object intercept(Object target, Method method, Object[] arguments, IResponseGenerator realMethodInvoker) { - IMockObject mockObject = new MockObject(mockConfiguration.getName(), mockConfiguration.getExactType(), target, - mockConfiguration.isVerified(), mockConfiguration.isGlobal(), mockConfiguration.getDefaultResponse(), specification, this); + IMockObject mockObject = new MockObject(mockConfiguration, target, specification, this); if (method.getDeclaringClass() == ISpockMockObject.class) { - return mockObject; + return handleSpockMockInterface(method, mockObject); } // we do not need the cast information from the wrappers here, the method selection @@ -61,7 +60,7 @@ public Object intercept(Object target, Method method, Object[] arguments, IRespo } } if (isMethod(method, "setProperty", String.class, Object.class)) { - String methodName = GroovyRuntimeUtil.propertyToMethodName("set", (String) args[0]); + String methodName = GroovyRuntimeUtil.propertyToMethodName(GroovyRuntimeUtil.SET, (String) args[0]); return GroovyRuntimeUtil.invokeMethod(target, methodName, args[1]); } if (isMethod(method, "methodMissing", String.class, Object.class)) { diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockMetaClass.java b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockMetaClass.java index b8ef479d79..9e6fb3d798 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockMetaClass.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockMetaClass.java @@ -25,6 +25,9 @@ import groovy.lang.*; import static java.util.Arrays.asList; +import static org.spockframework.runtime.GroovyRuntimeUtil.GET; +import static org.spockframework.runtime.GroovyRuntimeUtil.IS; +import static org.spockframework.runtime.GroovyRuntimeUtil.SET; public class GroovyMockMetaClass extends DelegatingMetaClass implements SpecificationAttachable { private final IMockConfiguration configuration; @@ -53,17 +56,17 @@ public Object invokeConstructor(Object[] arguments) { @Override public Object getProperty(Object target, String property) { - String methodName = GroovyRuntimeUtil.propertyToMethodName("is", property); + String methodName = GroovyRuntimeUtil.propertyToMethodName(IS, property); MetaMethod metaMethod = delegate.getMetaMethod(methodName, GroovyRuntimeUtil.EMPTY_ARGUMENTS); if (metaMethod == null || metaMethod.getReturnType() != boolean.class) { - methodName = GroovyRuntimeUtil.propertyToMethodName("get", property); + methodName = GroovyRuntimeUtil.propertyToMethodName(GET, property); } return invokeMethod(target, methodName, GroovyRuntimeUtil.EMPTY_ARGUMENTS); } @Override public void setProperty(Object target, String property, Object newValue) { - String methodName = GroovyRuntimeUtil.propertyToMethodName("set", property); + String methodName = GroovyRuntimeUtil.propertyToMethodName(SET, property); invokeMethod(target, methodName, new Object[] {newValue}); } @@ -121,8 +124,7 @@ private boolean isGetMetaClassCallOnGroovyObject(Object target, String method, O private IMockInvocation createMockInvocation(MetaMethod metaMethod, Object target, String methodName, Object[] arguments, boolean isStatic) { - IMockObject mockObject = new MockObject(configuration.getName(), configuration.getExactType(), target, - configuration.isVerified(), configuration.isGlobal(), configuration.getDefaultResponse(), specification, this); + IMockObject mockObject = new MockObject(configuration, target, specification, this); IMockMethod mockMethod; if (metaMethod != null) { List parameterTypes = asList(metaMethod.getNativeParameterTypes()); diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/IMockInteractionValidation.java b/spock-core/src/main/java/org/spockframework/mock/runtime/IMockInteractionValidation.java new file mode 100644 index 0000000000..b103f240cd --- /dev/null +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/IMockInteractionValidation.java @@ -0,0 +1,47 @@ +/* + * Copyright 2024 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.runtime; + +import org.spockframework.mock.IMockInteraction; +import org.spockframework.mock.IMockObject; +import org.spockframework.mock.ISpockMockObject; +import org.spockframework.util.Beta; +import org.spockframework.util.ThreadSafe; + +/** + * The {@link IMockInteractionValidation} interface allows {@link IMockMaker} implementations + * to implement different kinds of validations on mock interactions. + * + *

The instance is registered and the {@link ISpockMockObject#$spock_mockInteractionValidation()} method. The {@code IMockMaker} shall return the validation instance to use.

+ * + * @author Andreas Turban + * @since 2.4 + */ +@ThreadSafe +@Beta +public interface IMockInteractionValidation { + /** + * Is call by the {@link IMockInteraction} on creation to allow the underlying {@link IMockMaker} to validate the interaction. + * + * @param mockObject the spock mock object + * @param interaction the interaction to validate + * @throws org.spockframework.runtime.InvalidSpecException if the interface is invalid + * @since 2.4 + */ + @Beta + void validateMockInteraction(IMockObject mockObject, IMockInteraction interaction); +} 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 a90eacb7dc..79a077cf26 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 @@ -23,6 +23,7 @@ import groovy.lang.*; import static java.util.Arrays.asList; +import static org.spockframework.runtime.GroovyRuntimeUtil.SET; public class JavaMockInterceptor extends BaseMockInterceptor { private final IMockConfiguration mockConfiguration; @@ -37,11 +38,10 @@ public JavaMockInterceptor(IMockConfiguration mockConfiguration, Specification s @Override public Object intercept(Object target, Method method, Object[] arguments, IResponseGenerator realMethodInvoker) { - IMockObject mockObject = new MockObject(mockConfiguration.getName(), mockConfiguration.getExactType(), - target, mockConfiguration.isVerified(), false, mockConfiguration.getDefaultResponse(), specification, this); + IMockObject mockObject = new MockObject(mockConfiguration, target, specification, this); if (method.getDeclaringClass() == ISpockMockObject.class) { - return mockObject; + return handleSpockMockInterface(method, mockObject); } // here no instances of org.codehaus.groovy.runtime.wrappers.Wrapper subclasses @@ -64,7 +64,7 @@ public Object intercept(Object target, Method method, Object[] arguments, IRespo // HACK: for some reason, runtime dispatches direct property access on mock classes via ScriptBytecodeAdapter // delegate to the corresponding setter method // for abstract groovy classes and interfaces it uses InvokerHelper - String methodName = GroovyRuntimeUtil.propertyToMethodName("set", (String)args[0]); + String methodName = GroovyRuntimeUtil.propertyToMethodName(SET, (String) args[0]); return GroovyRuntimeUtil.invokeMethod(target, methodName, GroovyRuntimeUtil.asArgumentArray(args[1])); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.java b/spock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.java index 4971e59033..c197e86b43 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.java @@ -71,6 +71,10 @@ public boolean matches(IMockInvocation invocation) { return true; } + List getConstraints() { + return constraints; + } + @Override public Supplier accept(IMockInvocation invocation) { acceptedInvocations.add(invocation); diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/MockObject.java b/spock-core/src/main/java/org/spockframework/mock/runtime/MockObject.java index ab98ebc0a8..a6a641f17e 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/MockObject.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/MockObject.java @@ -23,25 +23,18 @@ import java.lang.reflect.Type; +import static java.util.Objects.*; + public class MockObject implements IMockObject { - private final String name; - private final Type type; + private final IMockConfiguration configuration; private final Object instance; - private final boolean verified; - private final boolean global; - private final IDefaultResponse defaultResponse; private final SpecificationAttachable mockInterceptor; - + @Nullable private Specification specification; - public MockObject(@Nullable String name, Type type, Object instance, boolean verified, boolean global, - IDefaultResponse defaultResponse, Specification specification, SpecificationAttachable mockInterceptor) { - this.name = name; - this.type = type; + public MockObject(IMockConfiguration configuration, Object instance, Specification specification, SpecificationAttachable mockInterceptor) { + this.configuration = requireNonNull(configuration); this.instance = instance; - this.verified = verified; - this.global = global; - this.defaultResponse = defaultResponse; this.specification = specification; this.mockInterceptor = mockInterceptor; } @@ -49,17 +42,17 @@ public MockObject(@Nullable String name, Type type, Object instance, boolean ver @Override @Nullable public String getName() { - return name; + return configuration.getName(); } @Override public Class getType() { - return GenericTypeReflectorUtil.erase(type); + return GenericTypeReflectorUtil.erase(getExactType()); } @Override public Type getExactType() { - return type; + return configuration.getExactType(); } @Override @@ -69,12 +62,16 @@ public Object getInstance() { @Override public boolean isVerified() { - return verified; + return configuration.isVerified(); + } + + private boolean isGlobal() { + return configuration.isGlobal(); } @Override public IDefaultResponse getDefaultResponse() { - return defaultResponse; + return configuration.getDefaultResponse(); } @Override @@ -84,9 +81,9 @@ public Specification getSpecification() { @Override public boolean matches(Object target, IMockInteraction interaction) { - if (target instanceof Wildcard) return verified || !interaction.isRequired(); + if (target instanceof Wildcard) return isVerified() || !interaction.isRequired(); - boolean match = global ? matchGlobal(target) : instance == target; + boolean match = isGlobal() ? matchGlobal(target) : instance == target; if (match) { checkRequiredInteractionAllowed(interaction); } @@ -102,8 +99,8 @@ private boolean isMockOfClass() { } private void checkRequiredInteractionAllowed(IMockInteraction interaction) { - if (!verified && interaction.isRequired()) { - String mockName = name != null ? name : "unnamed"; + if (!isVerified() && interaction.isRequired()) { + String mockName = getName() != null ? getName() : "unnamed"; throw new InvalidSpecException("Stub '%s' matches the following required interaction:" + "\n\n%s\n\nRemove the cardinality (e.g. '1 *'), or turn the stub into a mock.\n").withArgs(mockName, interaction); } @@ -120,4 +117,9 @@ public void detach() { this.specification = null; this.mockInterceptor.detach(); } + + @Override + public IMockConfiguration getConfiguration() { + return configuration; + } } diff --git a/spock-core/src/main/java/org/spockframework/runtime/GroovyRuntimeUtil.java b/spock-core/src/main/java/org/spockframework/runtime/GroovyRuntimeUtil.java index c9a7cd34fa..7cba5c62ab 100644 --- a/spock-core/src/main/java/org/spockframework/runtime/GroovyRuntimeUtil.java +++ b/spock-core/src/main/java/org/spockframework/runtime/GroovyRuntimeUtil.java @@ -37,6 +37,10 @@ * @author Peter Niederwieser */ public abstract class GroovyRuntimeUtil { + public static final String SET = "set"; + public static final String GET = "get"; + public static final String IS = "is"; + public static Object[] EMPTY_ARGUMENTS = new Object[0]; public static boolean isTruthy(Object obj) { @@ -115,10 +119,10 @@ public static String getterMethodToPropertyName(String methodName, List if (!parameterTypes.isEmpty()) return null; if (returnType == void.class) return null; // Void.class is allowed - if (methodName.startsWith("get")) + if (methodName.startsWith(GET)) return getPropertyName(methodName, 3); - if (methodName.startsWith("is") + if (methodName.startsWith(IS) && returnType == boolean.class) // Boolean.class is not allowed return getPropertyName(methodName, 2); diff --git a/spock-specs/src/test/groovy/org/spockframework/runtime/GroovyRuntimeUtilSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/runtime/GroovyRuntimeUtilSpec.groovy index 6b7b2a148f..d578c015b9 100644 --- a/spock-specs/src/test/groovy/org/spockframework/runtime/GroovyRuntimeUtilSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/runtime/GroovyRuntimeUtilSpec.groovy @@ -15,7 +15,7 @@ package org.spockframework.runtime import org.codehaus.groovy.runtime.typehandling.GroovyCastException -import spock.lang.* +import spock.lang.Specification class GroovyRuntimeUtilSpec extends Specification { def "getterMethodToPropertyName"() { @@ -39,6 +39,7 @@ class GroovyRuntimeUtilSpec extends Specification { "get" | Integer | null "is" | boolean | null "foo" | String | null + "isfoo" | String | null "setFoo" | void | null } diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaSpies.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaSpies.groovy index 30f451a896..ae037b3512 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaSpies.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaSpies.groovy @@ -202,10 +202,17 @@ class JavaSpies extends Specification { person.phoneNumber == "6789" } + @Issue("https://github.com/spockframework/spock/issues/2039") def "cannot spy final methods with byteBuddy"() { FinalMethodPerson person = Spy(mockMaker: MockMakers.byteBuddy) + + when: person.phoneNumber >> 6789 + then: + def ex = thrown(InvalidSpecException) + ex.message == "The final method 'getPhoneNumber' can't be mocked by the byte-buddy mock maker. Please use another mock maker supporting final methods." + expect: person.phoneNumber == "12345" } @@ -214,10 +221,11 @@ class JavaSpies extends Specification { FinalMethodPerson person = Spy() when: - person.phoneNumber + person.phoneNumber >> 6789 then: - 0 * person.phoneNumber + def ex = thrown(InvalidSpecException) + ex.message == "The final method 'getPhoneNumber' can't be mocked by the byte-buddy mock maker. Please use another mock maker supporting final methods." } def "cannot spy globally"() { diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy index 9e70d29cd9..9a02bbde79 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy @@ -160,20 +160,84 @@ class JavaStubs extends Specification { person.phoneNumber == "6789" } + @Issue("https://github.com/spockframework/spock/issues/2039") def "cannot stub final methods with byteBuddy"() { + given: + FinalMethodPerson person = Stub(mockMaker: MockMakers.byteBuddy) + + when: + person.getPhoneNumber() >> 6789 + + then: + def ex = thrown(InvalidSpecException) + ex.message == "The final method 'getPhoneNumber' can't be mocked by the byte-buddy mock maker. Please use another mock maker supporting final methods." + + expect: + person.getPhoneNumber() == "12345" + } + + @Issue("https://github.com/spockframework/spock/issues/2039") + def "cannot stub final methods with byteBuddy without error message when one overload is non final"() { + given: + FinalMethodPerson person = Stub(mockMaker: MockMakers.byteBuddy) + + person.finalAndNonFinalOverload() >> "B" + + expect: + person.finalAndNonFinalOverload() == "final" + } + + @Issue("https://github.com/spockframework/spock/issues/2039") + def "non final method overload shall be mockable"() { + given: FinalMethodPerson person = Stub(mockMaker: MockMakers.byteBuddy) + + person.finalAndNonFinalOverload("A") >> "B" + + expect: + person.finalAndNonFinalOverload("A") == "B" + } + + def "cannot stub final method as property with byteBuddy"() { + given: + FinalMethodPerson person = Stub(mockMaker: MockMakers.byteBuddy) + + when: person.phoneNumber >> 6789 + then: + def ex = thrown(InvalidSpecException) + ex.message == "The final method 'getPhoneNumber' can't be mocked by the byte-buddy mock maker. Please use another mock maker supporting final methods." + expect: person.phoneNumber == "12345" } + def "cannot stub final is getter as property with byteBuddy"() { + given: + FinalMethodPerson person = Stub(mockMaker: MockMakers.byteBuddy) + + when: + person.finalPerson >> false + + then: + def ex = thrown(InvalidSpecException) + ex.message == "The final method 'isFinalPerson' can't be mocked by the byte-buddy mock maker. Please use another mock maker supporting final methods." + + expect: + person.finalPerson + } + def "cannot stub final methods without specifying mockMaker"() { + given: FinalMethodPerson person = Stub() - person.phoneNumber >> 6789 - expect: - person.phoneNumber == "12345" + when: + person.getPhoneNumber() >> 6789 + + then: + def ex = thrown(InvalidSpecException) + ex.message == "The final method 'getPhoneNumber' can't be mocked by the byte-buddy mock maker. Please use another mock maker supporting final methods." } def "cannot stub globally"() { @@ -208,11 +272,24 @@ class JavaStubs extends Specification { List children } + @SuppressWarnings('GrMethodMayBeStatic') static final class FinalPerson extends Person { String getPhoneNumber() { "12345" } } + @SuppressWarnings('GrMethodMayBeStatic') static class FinalMethodPerson extends Person { + final String getPhoneNumber() { "12345" } + + final boolean isFinalPerson() { return true } + + final String finalAndNonFinalOverload() { + return "final" + } + + String finalAndNonFinalOverload(String arg) { + return arg + } } } diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MockDefaultResponses.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MockDefaultResponses.groovy index 4a075816bc..c60a248c7f 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MockDefaultResponses.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MockDefaultResponses.groovy @@ -16,6 +16,8 @@ package org.spockframework.smoke.mock +import org.spockframework.mock.IMockObject +import org.spockframework.mock.ISpockMockObject import spock.lang.Specification class MockDefaultResponses extends Specification { @@ -72,6 +74,16 @@ class MockDefaultResponses extends Specification { cmock.getDouble().getClass() == Double.class } + def "default impl of ISpockMockObject.spock_mockInteractionValidation"() { + given: + def obj = new ISpockMockObject() { + @Override + IMockObject $spock_get() { return null } + } + expect: + obj.$spock_mockInteractionValidation() == null + } + interface IMockable { boolean getBoolean() byte getByte() diff --git a/spock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.java b/spock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.java index 29eae58721..77b8d58a2e 100644 --- a/spock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.java +++ b/spock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.java @@ -17,6 +17,7 @@ package org.spockframework.spring.mock; import org.spockframework.mock.*; +import org.spockframework.mock.runtime.BaseMockInterceptor; import org.spockframework.mock.runtime.IProxyBasedMockInterceptor; import org.spockframework.runtime.model.FieldInfo; import org.spockframework.util.*; @@ -35,7 +36,7 @@ public DelegatingInterceptor(FieldInfo fieldInfo) { @Override public Object intercept(Object target, Method method, Object[] arguments, IResponseGenerator realMethodInvoker) { if (method.getDeclaringClass() == ISpockMockObject.class) { - return new MockObjectAdapter(this); + return BaseMockInterceptor.handleSpockMockInterface(method, new MockObjectAdapter(this)); } if (delegate == null) { @@ -125,5 +126,10 @@ public Specification getSpecification() { public boolean matches(Object target, IMockInteraction interaction) { return false; } + + @Override + public IMockConfiguration getConfiguration() { + throw new UnsupportedOperationException(); + } } }