From c86796069f91f3b4fdece83b7b2ea92581dd48e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Kautler?= Date: Thu, 7 Mar 2024 00:11:12 +0100 Subject: [PATCH 1/6] Properly fix possible deadlock, when blocking in mock response generators and fix fallout of #1885 (#583, #1899) --- docs/release_notes.adoc | 20 ++++++- .../mock/DefaultCompareToInteraction.java | 6 +- .../mock/DefaultEqualsInteraction.java | 6 +- .../mock/DefaultFinalizeInteraction.java | 6 +- .../mock/DefaultHashCodeInteraction.java | 6 +- .../mock/DefaultToStringInteraction.java | 28 +++++---- .../mock/EmptyOrDummyResponse.java | 2 +- .../spockframework/mock/IMockInteraction.java | 4 +- .../mock/ZeroOrNullResponse.java | 2 +- .../mock/response/CodeResponseGenerator.java | 3 + .../response/IterableResponseGenerator.java | 11 +++- .../mock/response/ResponseGeneratorChain.java | 25 +++++--- .../response/SingleResponseGenerator.java | 2 +- .../mock/runtime/InteractionScope.java | 28 +++++---- .../mock/runtime/MockController.java | 45 ++++++++------- .../mock/runtime/MockInteraction.java | 46 +++++---------- .../runtime/MockInteractionDecorator.java | 3 +- .../mock/runtime/ObjectMethodInvoker.java | 2 +- .../smoke/mock/MultiThreaded.groovy | 57 ++++++++++++++----- 19 files changed, 184 insertions(+), 118 deletions(-) diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc index e268228d5e..1f231d9a0c 100644 --- a/docs/release_notes.adoc +++ b/docs/release_notes.adoc @@ -5,6 +5,22 @@ include::include.adoc[] == 2.4 (tbd) +=== Breaking Changes + +* Calculated responses for interactions (`>> { ... }`) previously were all executed synchronized on the respective mock + controller instance, so could safely mutate shared state to a certain degree, even if the invocations were happening + on different threads. This also caused that one response calculation could not wait on something happening + in another response calculation, as they were all executed sequentially due to the synchronization. + Starting with this release, the response calculations are no longer happening synchronized. + If you depend on shared state in such calculations, you now have to make sure yourself, that this is done in a + thread-safe manner. Most users will probably be unaffected by this change as it only becomes relevant in a + multi-threaded situation where multiple threads do interaction invocations that care about shared state. + +=== Misc + +* Properly fix possible deadlock, when blocking in mock response generators and fix fallout of spockPull:1885[] +** This actually fixes the issues: spockIssue:583[], spockIssue:1882[], spockIssue:1899[] + == 2.4-M2 (2024-02-26) === Highlights @@ -70,8 +86,8 @@ include::include.adoc[] * Fix exception when configured `baseDir` was not existing, now `@TempDir` will create the baseDir directory if it is missing. * Fix bad error message for collection conditions, when one of the operands is `null` * Fix docs about initializer method interceptors spockPull:1666[] -* Fix possible deadlock, when blocking in mock response generators spockPull:1885[] -** This fixes the issues: spockIssue:583[], spockIssue:1882[] +* [.line-through]#Fix possible deadlock, when blocking in mock response generators spockPull:1885[]# +** [.line-through]#This fixes the issues: spockIssue:583[], spockIssue:1882[]# * Fix SpringSpy not working with `DirtiesContext.ClassMode.BEFORE_EACH_TEST_METHOD` spockPull:1869[] * Fix null handling for collection conditions spockPull:1858[] * Fix interceptor contexts spockPull:1676[] diff --git a/spock-core/src/main/java/org/spockframework/mock/DefaultCompareToInteraction.java b/spock-core/src/main/java/org/spockframework/mock/DefaultCompareToInteraction.java index ff91d9ab84..1065393084 100644 --- a/spock-core/src/main/java/org/spockframework/mock/DefaultCompareToInteraction.java +++ b/spock-core/src/main/java/org/spockframework/mock/DefaultCompareToInteraction.java @@ -1,5 +1,7 @@ package org.spockframework.mock; +import java.util.function.Supplier; + public class DefaultCompareToInteraction extends DefaultInteraction { public static final DefaultCompareToInteraction INSTANCE = new DefaultCompareToInteraction(); @@ -21,7 +23,7 @@ public boolean matches(IMockInvocation invocation) { } @Override - public Object accept(IMockInvocation invocation) { - return Integer.compare(System.identityHashCode(invocation.getMockObject().getInstance()), System.identityHashCode(invocation.getArguments().get(0))); + public Supplier accept(IMockInvocation invocation) { + return () -> Integer.compare(System.identityHashCode(invocation.getMockObject().getInstance()), System.identityHashCode(invocation.getArguments().get(0))); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/DefaultEqualsInteraction.java b/spock-core/src/main/java/org/spockframework/mock/DefaultEqualsInteraction.java index 823c4b58bf..ee55f970d8 100644 --- a/spock-core/src/main/java/org/spockframework/mock/DefaultEqualsInteraction.java +++ b/spock-core/src/main/java/org/spockframework/mock/DefaultEqualsInteraction.java @@ -13,6 +13,8 @@ */ package org.spockframework.mock; +import java.util.function.Supplier; + class DefaultEqualsInteraction extends DefaultInteraction { public static final DefaultEqualsInteraction INSTANCE = new DefaultEqualsInteraction(); @@ -33,7 +35,7 @@ public boolean matches(IMockInvocation invocation) { } @Override - public Object accept(IMockInvocation invocation) { - return invocation.getMockObject().getInstance() == invocation.getArguments().get(0); + public Supplier accept(IMockInvocation invocation) { + return () -> invocation.getMockObject().getInstance() == invocation.getArguments().get(0); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/DefaultFinalizeInteraction.java b/spock-core/src/main/java/org/spockframework/mock/DefaultFinalizeInteraction.java index a7f71a7d31..b753b704d9 100644 --- a/spock-core/src/main/java/org/spockframework/mock/DefaultFinalizeInteraction.java +++ b/spock-core/src/main/java/org/spockframework/mock/DefaultFinalizeInteraction.java @@ -13,6 +13,8 @@ */ package org.spockframework.mock; +import java.util.function.Supplier; + public class DefaultFinalizeInteraction extends DefaultInteraction { public static final DefaultFinalizeInteraction INSTANCE = new DefaultFinalizeInteraction(); @@ -31,7 +33,7 @@ public boolean matches(IMockInvocation invocation) { } @Override - public Object accept(IMockInvocation invocation) { - return null; + public Supplier accept(IMockInvocation invocation) { + return () -> null; } } diff --git a/spock-core/src/main/java/org/spockframework/mock/DefaultHashCodeInteraction.java b/spock-core/src/main/java/org/spockframework/mock/DefaultHashCodeInteraction.java index 3bb38ce417..653c45ebfc 100644 --- a/spock-core/src/main/java/org/spockframework/mock/DefaultHashCodeInteraction.java +++ b/spock-core/src/main/java/org/spockframework/mock/DefaultHashCodeInteraction.java @@ -13,6 +13,8 @@ */ package org.spockframework.mock; +import java.util.function.Supplier; + class DefaultHashCodeInteraction extends DefaultInteraction { public static final DefaultHashCodeInteraction INSTANCE = new DefaultHashCodeInteraction(); @@ -30,7 +32,7 @@ public boolean matches(IMockInvocation invocation) { } @Override - public Object accept(IMockInvocation invocation) { - return System.identityHashCode(invocation.getMockObject().getInstance()); + public Supplier accept(IMockInvocation invocation) { + return () -> System.identityHashCode(invocation.getMockObject().getInstance()); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/DefaultToStringInteraction.java b/spock-core/src/main/java/org/spockframework/mock/DefaultToStringInteraction.java index 490a102aa3..04aa700b14 100644 --- a/spock-core/src/main/java/org/spockframework/mock/DefaultToStringInteraction.java +++ b/spock-core/src/main/java/org/spockframework/mock/DefaultToStringInteraction.java @@ -14,6 +14,8 @@ package org.spockframework.mock; +import java.util.function.Supplier; + class DefaultToStringInteraction extends DefaultInteraction { public static final DefaultToStringInteraction INSTANCE = new DefaultToStringInteraction(); @@ -31,19 +33,21 @@ public boolean matches(IMockInvocation invocation) { } @Override - public Object accept(IMockInvocation invocation) { - StringBuilder builder = new StringBuilder(); - builder.append("Mock for type '"); - builder.append(invocation.getMockObject().getType().getSimpleName()); - builder.append("'"); - - String name = invocation.getMockObject().getName(); - if (name != null) { - builder.append(" named '"); - builder.append(name); + public Supplier accept(IMockInvocation invocation) { + return () -> { + StringBuilder builder = new StringBuilder(); + builder.append("Mock for type '"); + builder.append(invocation.getMockObject().getType().getSimpleName()); builder.append("'"); - } - return builder.toString(); + String name = invocation.getMockObject().getName(); + if (name != null) { + builder.append(" named '"); + builder.append(name); + builder.append("'"); + } + + return builder.toString(); + }; } } diff --git a/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java b/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java index 4f5a0df67c..b0ebcd569b 100644 --- a/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java @@ -40,7 +40,7 @@ private EmptyOrDummyResponse() {} @SuppressWarnings("rawtypes") public Object respond(IMockInvocation invocation) { IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); - if (interaction != null) return interaction.accept(invocation); + if (interaction != null) return interaction.accept(invocation).get(); Class returnType = invocation.getMethod().getReturnType(); diff --git a/spock-core/src/main/java/org/spockframework/mock/IMockInteraction.java b/spock-core/src/main/java/org/spockframework/mock/IMockInteraction.java index 103a8e0315..0d345bf451 100644 --- a/spock-core/src/main/java/org/spockframework/mock/IMockInteraction.java +++ b/spock-core/src/main/java/org/spockframework/mock/IMockInteraction.java @@ -19,6 +19,7 @@ import org.spockframework.util.Nullable; import java.util.List; +import java.util.function.Supplier; /** * An anticipated interaction between the SUT and one or more mock objects. @@ -34,8 +35,7 @@ public interface IMockInteraction { boolean matches(IMockInvocation invocation); - @Nullable - Object accept(IMockInvocation invocation); + Supplier accept(IMockInvocation invocation); List getAcceptedInvocations(); diff --git a/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java b/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java index 1f2a5ed9b6..873d05b3fc 100644 --- a/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java @@ -28,7 +28,7 @@ private ZeroOrNullResponse() {} @Override public Object respond(IMockInvocation invocation) { IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); - if (interaction != null) return interaction.accept(invocation); + if (interaction != null) return interaction.accept(invocation).get(); Class returnType = invocation.getMethod().getReturnType(); return ReflectionUtil.getDefaultValue(returnType); diff --git a/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java index 53d60f449d..b1206d063e 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java @@ -21,6 +21,8 @@ import groovy.lang.Closure; +import static org.spockframework.util.ObjectUtil.uncheckedCast; + /** * * @author Peter Niederwieser @@ -49,6 +51,7 @@ private Object invokeClosure(IMockInvocation invocation) { return GroovyRuntimeUtil.invokeClosure(code, invocation); } + Closure code = uncheckedCast(this.code.clone()); code.setDelegate(invocation); code.setResolveStrategy(Closure.DELEGATE_FIRST); return GroovyRuntimeUtil.invokeClosure(code, invocation.getArguments()); diff --git a/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java index 6c39406d6b..a7ea6ad634 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java @@ -28,6 +28,7 @@ * @author Peter Niederwieser */ public class IterableResponseGenerator implements IChainableResponseGenerator { + private final Object lock = new Object(); private final Iterator iterator; private Object nextValue; @@ -37,12 +38,16 @@ public IterableResponseGenerator(Object iterable) { @Override public boolean isAtEndOfCycle() { - return !iterator.hasNext(); + synchronized (lock) { + return !iterator.hasNext(); + } } @Override public Object respond(IMockInvocation invocation) { - if (iterator.hasNext()) nextValue = iterator.next(); - return GroovyRuntimeUtil.coerce(nextValue, invocation.getMethod().getReturnType()); + synchronized (lock) { + if (iterator.hasNext()) nextValue = iterator.next(); + return GroovyRuntimeUtil.coerce(nextValue, invocation.getMethod().getReturnType()); + } } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java b/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java index ba80fa47c5..4f84c1a706 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java @@ -22,25 +22,34 @@ import java.util.LinkedList; public class ResponseGeneratorChain implements IResponseGenerator { + private final Object lock = new Object(); private IChainableResponseGenerator current; private final LinkedList remaining = new LinkedList<>(); public void addFirst(IChainableResponseGenerator generator) { - if (current != null) remaining.addFirst(current); - current = generator; + synchronized (lock) { + if (current != null) remaining.addFirst(current); + current = generator; + } } public boolean isEmpty() { - return current == null; + synchronized (lock) { + return current == null; + } } @Override public Object respond(IMockInvocation invocation) { - if (isEmpty()) throw new InternalSpockError("ResultGeneratorChain should never be empty"); - - while (current.isAtEndOfCycle() && !remaining.isEmpty()) { - current = remaining.removeFirst(); + IChainableResponseGenerator responseGenerator; + synchronized (lock) { + if (isEmpty()) throw new InternalSpockError("ResultGeneratorChain should never be empty"); + + while (current.isAtEndOfCycle() && !remaining.isEmpty()) { + current = remaining.removeFirst(); + } + responseGenerator = current; } - return current.respond(invocation); + return responseGenerator.respond(invocation); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java index 3794b7f6cb..9e86379218 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java @@ -19,7 +19,7 @@ import org.spockframework.mock.*; public abstract class SingleResponseGenerator implements IChainableResponseGenerator { - private boolean endOfCycle = false; + private volatile boolean endOfCycle = false; @Override public boolean isAtEndOfCycle() { diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/InteractionScope.java b/spock-core/src/main/java/org/spockframework/mock/runtime/InteractionScope.java index 5332a0fca4..fe8931f020 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/InteractionScope.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/InteractionScope.java @@ -20,6 +20,7 @@ import org.spockframework.util.ExceptionUtil; import java.util.*; +import java.util.function.Supplier; /** * A scope for interactions defined outside a then-block @@ -40,10 +41,12 @@ public void addInteraction(final IMockInteraction interaction) { final int myRegistrationZone = currentRegistrationZone; @Override - public Object accept(IMockInvocation invocation) { - WrongInvocationOrderError wrongInvocationOrderError = null; + public Supplier accept(IMockInvocation invocation) { + WrongInvocationOrderError wrongInvocationOrderError; if (currentExecutionZone > myRegistrationZone) { wrongInvocationOrderError = new WrongInvocationOrderError(decorated, invocation, previousInvocationsInReverseOrder); + } else { + wrongInvocationOrderError = null; } currentExecutionZone = myRegistrationZone; if (previousInvocationsInReverseOrder.size() == MAX_PREVIOUS_INVOCATIONS) { @@ -51,15 +54,18 @@ public Object accept(IMockInvocation invocation) { } previousInvocationsInReverseOrder.addFirst(invocation); - Object result = null; - Throwable invocationException = null; - try { - result = super.accept(invocation); - } catch (AssertionError | Exception e) { - invocationException = e; - } - ExceptionUtil.throwWithSuppressed(invocationException, wrongInvocationOrderError); - return result; + Supplier supplierFromDecorated = super.accept(invocation); + return () -> { + Object result = null; + Throwable invocationException = null; + try { + result = supplierFromDecorated.get(); + } catch (AssertionError | Exception e) { + invocationException = e; + } + ExceptionUtil.throwWithSuppressed(invocationException, wrongInvocationOrderError); + return result; + }; } @Override diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java b/spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java index f380b1d7ec..26ad1270b9 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java @@ -17,10 +17,13 @@ package org.spockframework.mock.runtime; import org.spockframework.mock.*; +import org.spockframework.util.Checks; import org.spockframework.util.ExceptionUtil; import java.util.*; import java.util.concurrent.Callable; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.function.Supplier; import static java.util.Objects.requireNonNull; @@ -29,7 +32,7 @@ */ public class MockController implements IMockController, IThreadAwareMockController { private final Deque scopes = new LinkedList<>(); - private final List errors = new ArrayList<>(); + private final List errors = new CopyOnWriteArrayList<>(); private final List staticMocks = new ArrayList<>(); public MockController() { @@ -38,31 +41,31 @@ public MockController() { @Override public Object handle(IMockInvocation invocation) { - Optional interaction = findInteraction(invocation); - if (interaction.isPresent()) { - try { - return interaction.get().accept(invocation); - } catch (InteractionNotSatisfiedError e) { - synchronized (this) { - errors.add(e); + Supplier resultSupplier = null; + synchronized (this) { + for (IInteractionScope scope : scopes) { + IMockInteraction interaction = scope.match(invocation); + if (interaction != null) { + resultSupplier = requireNonNull(interaction.accept(invocation), "interaction must not return null"); + break; } - throw e; } - } - return invocation.getMockObject().getDefaultResponse().respond(invocation); - } - - private synchronized Optional findInteraction(IMockInvocation invocation) { - for (IInteractionScope scope : scopes) { - IMockInteraction interaction = scope.match(invocation); - if (interaction != null) { - return Optional.of(interaction); + if (resultSupplier == null) { + for (IInteractionScope scope : scopes) { + scope.addUnmatchedInvocation(invocation); + } } } - for (IInteractionScope scope : scopes) { - scope.addUnmatchedInvocation(invocation); + if (resultSupplier == null) { + return invocation.getMockObject().getDefaultResponse().respond(invocation); + } else { + try { + return resultSupplier.get(); + } catch (InteractionNotSatisfiedError e) { + errors.add(e); + throw e; + } } - return Optional.empty(); } public static final String ADD_INTERACTION = "addInteraction"; 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 f0662700cb..77161856ae 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 @@ -20,7 +20,8 @@ import org.spockframework.util.TextUtil; import java.util.*; -import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.Supplier; +import java.util.stream.Collectors; /** * An anticipated interaction between the SUT and one or more mock objects. @@ -35,8 +36,8 @@ public class MockInteraction implements IMockInteraction { private final int maxCount; private final List constraints; private final IResponseGenerator responseGenerator; + private final List acceptedInvocations = new ArrayList<>(); - private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); public MockInteraction(int line, int column, String text, int minCount, int maxCount, List constraints, @@ -65,18 +66,16 @@ public boolean matches(IMockInvocation invocation) { } @Override - public Object accept(IMockInvocation invocation) { - lock.writeLock().lock(); - try { - acceptedInvocations.add(invocation); - if (acceptedInvocations.size() > maxCount) { - throw new TooManyInvocationsError(this, acceptedInvocations); - } - - return responseGenerator == null ? null : responseGenerator.respond(invocation); - } finally { - lock.writeLock().unlock(); + public Supplier accept(IMockInvocation invocation) { + acceptedInvocations.add(invocation); + if (acceptedInvocations.size() > maxCount) { + List acceptedInvocationsCopy = new ArrayList<>(acceptedInvocations); + return () -> { + throw new TooManyInvocationsError(this, acceptedInvocationsCopy); + }; } + + return () -> responseGenerator == null ? null : responseGenerator.respond(invocation); } @Override @@ -127,22 +126,12 @@ public String describeMismatch(IMockInvocation invocation) { @Override public boolean isSatisfied() { - lock.readLock().lock(); - try { - return acceptedInvocations.size() >= minCount; - } finally { - lock.readLock().unlock(); - } + return acceptedInvocations.size() >= minCount; } @Override public boolean isExhausted() { - lock.readLock().lock(); - try { - return acceptedInvocations.size() >= maxCount; - } finally { - lock.readLock().unlock(); - } + return acceptedInvocations.size() >= maxCount; } @Override @@ -166,11 +155,6 @@ public String getText() { } public String toString() { - lock.readLock().lock(); - try { - return String.format("%s (%d %s)", text, acceptedInvocations.size(), acceptedInvocations.size() == 1 ? "invocation" : "invocations"); - } finally { - lock.readLock().unlock(); - } + return String.format("%s (%d %s)", text, acceptedInvocations.size(), acceptedInvocations.size() == 1 ? "invocation" : "invocations"); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/MockInteractionDecorator.java b/spock-core/src/main/java/org/spockframework/mock/runtime/MockInteractionDecorator.java index 4ef7eb8c05..5c8c7a3f32 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/MockInteractionDecorator.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/MockInteractionDecorator.java @@ -17,6 +17,7 @@ import org.spockframework.mock.*; import java.util.List; +import java.util.function.Supplier; public abstract class MockInteractionDecorator implements IMockInteraction { protected final IMockInteraction decorated; @@ -46,7 +47,7 @@ public boolean matches(IMockInvocation invocation) { } @Override - public Object accept(IMockInvocation invocation) { + public Supplier accept(IMockInvocation invocation) { return decorated.accept(invocation); } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java index 458463f419..fc8dc7f916 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java @@ -13,7 +13,7 @@ private ObjectMethodInvoker() { public Object respond(IMockInvocation invocation) { IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); if (interaction != null) { - return interaction.accept(invocation); + return interaction.accept(invocation).get(); } return null; diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MultiThreaded.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MultiThreaded.groovy index f6e9c75216..5b4550179f 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MultiThreaded.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MultiThreaded.groovy @@ -2,33 +2,60 @@ package org.spockframework.smoke.mock import spock.lang.Issue import spock.lang.Specification +import spock.lang.Timeout import java.util.concurrent.CountDownLatch -import java.util.concurrent.Executors -import java.util.concurrent.TimeUnit -class MultiThreaded extends Specification { +import static java.util.concurrent.TimeUnit.SECONDS +class MultiThreaded extends Specification { @Issue("https://github.com/spockframework/spock/issues/583") def "mocks can block and wait on conditions"() { given: - CountDownLatch latch = new CountDownLatch(2) - List aList = Mock () - def exec = Executors.newFixedThreadPool(2) + CountDownLatch latch1 = new CountDownLatch(1) + CountDownLatch latch2 = new CountDownLatch(1) + CountDownLatch latch3 = new CountDownLatch(2) + List aList = Mock() when: - exec.submit { aList.get(0) } - exec.submit { aList.get(0) } - exec.shutdown() - latch.await() + Thread.start { aList.get(0); latch3.countDown() } + Thread.start { aList.get(0); latch3.countDown() } + assert latch1.await(30, SECONDS) + assert latch2.await(30, SECONDS) then: - exec.awaitTermination(30, TimeUnit.SECONDS) + latch3.await(10, SECONDS) + + 1 * aList.get(_) >> { + latch1.countDown() + assert latch2.await(10, SECONDS) + } + + 1 * aList.get(_) >> { + latch2.countDown() + assert latch1.await(10, SECONDS) + } + } - 2 * aList.get(_) >> { - latch.countDown() - assert latch.await(30, TimeUnit.SECONDS) - 42 + @Issue("https://github.com/spockframework/spock/issues/1899") + @Timeout(30) + def "mock response generators do not deadlock when self-referencing"() { + given: + def called = false + Runnable foo + foo = Mock { + run() >> { + if (!called) { + called = true + Thread.start { foo.run() }.join() + } + } } + + when: + foo.run() + + then: + noExceptionThrown() } } From cfc3c54bfbda83c742a118057186ef9b020b2f5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Kautler?= Date: Fri, 8 Mar 2024 02:26:59 +0100 Subject: [PATCH 2/6] Review Feedback Vol. 1 --- .../spockframework/mock/CallRealMethodResponse.java | 2 ++ .../spockframework/mock/EmptyOrDummyResponse.java | 2 ++ .../org/spockframework/mock/ZeroOrNullResponse.java | 1 + .../mock/response/CodeResponseGenerator.java | 2 ++ .../mock/response/ConstantResponseGenerator.java | 2 ++ .../mock/response/DefaultResponseGenerator.java | 2 ++ .../mock/response/IterableResponseGenerator.java | 8 +++++--- .../mock/response/ResponseGeneratorChain.java | 10 ++++++---- .../mock/response/SingleResponseGenerator.java | 2 ++ .../mock/response/WildcardResponseGenerator.java | 2 ++ .../mock/runtime/ByteBuddyMethodInvoker.java | 2 ++ .../mock/runtime/CglibRealMethodInvoker.java | 2 ++ .../mock/runtime/DefaultMethodInvoker.java | 2 ++ .../mock/runtime/FailingRealMethodInvoker.java | 2 ++ .../mock/runtime/GroovyRealMethodInvoker.java | 2 ++ .../spockframework/mock/runtime/MockInteraction.java | 8 +++++++- .../mock/runtime/ObjectMethodInvoker.java | 2 ++ .../spockframework/smoke/mock/MultiThreaded.groovy | 12 +++++------- 18 files changed, 50 insertions(+), 15 deletions(-) diff --git a/spock-core/src/main/java/org/spockframework/mock/CallRealMethodResponse.java b/spock-core/src/main/java/org/spockframework/mock/CallRealMethodResponse.java index 5c23e0f34e..c4e1b67783 100644 --- a/spock-core/src/main/java/org/spockframework/mock/CallRealMethodResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/CallRealMethodResponse.java @@ -15,11 +15,13 @@ package org.spockframework.mock; import org.spockframework.util.Beta; +import org.spockframework.util.ThreadSafe; /** * A response strategy that delegates method calls to the real object underlying the mock (if any). */ @Beta +@ThreadSafe public class CallRealMethodResponse implements IDefaultResponse { public static final CallRealMethodResponse INSTANCE = new CallRealMethodResponse(); diff --git a/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java b/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java index b0ebcd569b..4faf562230 100644 --- a/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java @@ -15,6 +15,7 @@ package org.spockframework.mock; import org.spockframework.util.ReflectionUtil; +import org.spockframework.util.ThreadSafe; import spock.lang.Specification; import java.lang.reflect.*; @@ -31,6 +32,7 @@ * A response strategy that returns zero, an "empty" object, or a "dummy" object, * depending on the method's declared return type. */ +@ThreadSafe public class EmptyOrDummyResponse implements IDefaultResponse { public static final EmptyOrDummyResponse INSTANCE = new EmptyOrDummyResponse(); diff --git a/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java b/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java index 873d05b3fc..15464f00ef 100644 --- a/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java @@ -20,6 +20,7 @@ * A response strategy that returns zero, false, or null, depending on the method's return type. */ @Beta +@ThreadSafe public class ZeroOrNullResponse implements IDefaultResponse { public static final ZeroOrNullResponse INSTANCE = new ZeroOrNullResponse(); diff --git a/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java index b1206d063e..75d1e69696 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java @@ -20,6 +20,7 @@ import org.spockframework.runtime.GroovyRuntimeUtil; import groovy.lang.Closure; +import org.spockframework.util.ThreadSafe; import static org.spockframework.util.ObjectUtil.uncheckedCast; @@ -27,6 +28,7 @@ * * @author Peter Niederwieser */ +@ThreadSafe public class CodeResponseGenerator extends SingleResponseGenerator { private final Closure code; diff --git a/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java index 7ba1e1c7f3..8328a269c9 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java @@ -18,11 +18,13 @@ import org.spockframework.mock.IMockInvocation; import org.spockframework.runtime.GroovyRuntimeUtil; +import org.spockframework.util.ThreadSafe; /** * * @author Peter Niederwieser */ +@ThreadSafe public class ConstantResponseGenerator extends SingleResponseGenerator { private final Object constant; diff --git a/spock-core/src/main/java/org/spockframework/mock/response/DefaultResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/DefaultResponseGenerator.java index 715f0b6a8d..138ff4032b 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/DefaultResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/DefaultResponseGenerator.java @@ -17,7 +17,9 @@ package org.spockframework.mock.response; import org.spockframework.mock.*; +import org.spockframework.util.ThreadSafe; +@ThreadSafe public class DefaultResponseGenerator implements IResponseGenerator { @Override public Object respond(IMockInvocation invocation) { diff --git a/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java index a7ea6ad634..e3e319f543 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java @@ -18,6 +18,7 @@ import org.spockframework.mock.*; import org.spockframework.runtime.GroovyRuntimeUtil; +import org.spockframework.util.ThreadSafe; import java.util.Iterator; @@ -27,8 +28,9 @@ * * @author Peter Niederwieser */ +@ThreadSafe public class IterableResponseGenerator implements IChainableResponseGenerator { - private final Object lock = new Object(); + private final Object LOCK = new Object(); private final Iterator iterator; private Object nextValue; @@ -38,14 +40,14 @@ public IterableResponseGenerator(Object iterable) { @Override public boolean isAtEndOfCycle() { - synchronized (lock) { + synchronized (LOCK) { return !iterator.hasNext(); } } @Override public Object respond(IMockInvocation invocation) { - synchronized (lock) { + synchronized (LOCK) { if (iterator.hasNext()) nextValue = iterator.next(); return GroovyRuntimeUtil.coerce(nextValue, invocation.getMethod().getReturnType()); } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java b/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java index 4f84c1a706..fbe6b281ac 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java @@ -18,23 +18,25 @@ import org.spockframework.mock.*; import org.spockframework.util.InternalSpockError; +import org.spockframework.util.ThreadSafe; import java.util.LinkedList; +@ThreadSafe public class ResponseGeneratorChain implements IResponseGenerator { - private final Object lock = new Object(); + private final Object LOCK = new Object(); private IChainableResponseGenerator current; private final LinkedList remaining = new LinkedList<>(); public void addFirst(IChainableResponseGenerator generator) { - synchronized (lock) { + synchronized (LOCK) { if (current != null) remaining.addFirst(current); current = generator; } } public boolean isEmpty() { - synchronized (lock) { + synchronized (LOCK) { return current == null; } } @@ -42,7 +44,7 @@ public boolean isEmpty() { @Override public Object respond(IMockInvocation invocation) { IChainableResponseGenerator responseGenerator; - synchronized (lock) { + synchronized (LOCK) { if (isEmpty()) throw new InternalSpockError("ResultGeneratorChain should never be empty"); while (current.isAtEndOfCycle() && !remaining.isEmpty()) { diff --git a/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java index 9e86379218..2fcb599244 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java @@ -17,7 +17,9 @@ package org.spockframework.mock.response; import org.spockframework.mock.*; +import org.spockframework.util.ThreadSafe; +@ThreadSafe public abstract class SingleResponseGenerator implements IChainableResponseGenerator { private volatile boolean endOfCycle = false; diff --git a/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java index 4f98256641..fb3f4c7e71 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java @@ -17,6 +17,7 @@ package org.spockframework.mock.response; import org.spockframework.mock.*; +import org.spockframework.util.ThreadSafe; /** * Returns a value using {@code Stub} semantics (see {@link EmptyOrDummyResponse}). @@ -27,6 +28,7 @@ * @author Peter Niederwieser * @author Leonard Brünings */ +@ThreadSafe public class WildcardResponseGenerator extends SingleResponseGenerator { @Override public Object doRespond(IMockInvocation invocation) { diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java index c345bae852..5a93466c11 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java @@ -2,7 +2,9 @@ import org.spockframework.mock.*; import org.spockframework.util.ExceptionUtil; +import org.spockframework.util.ThreadSafe; +@ThreadSafe public class ByteBuddyMethodInvoker implements IResponseGenerator { private final ByteBuddyInvoker superCall; diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java index 687138ccb3..b12e2c091d 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java @@ -18,7 +18,9 @@ import org.spockframework.util.ExceptionUtil; import net.sf.cglib.proxy.MethodProxy; +import org.spockframework.util.ThreadSafe; +@ThreadSafe public class CglibRealMethodInvoker implements IResponseGenerator { private final MethodProxy methodProxy; diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java index 4ec594d790..f2747a29ef 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java @@ -20,6 +20,7 @@ import org.spockframework.util.ExceptionUtil; import org.spockframework.util.Nullable; import org.spockframework.util.ReflectionUtil; +import org.spockframework.util.ThreadSafe; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; @@ -27,6 +28,7 @@ import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; +@ThreadSafe public class DefaultMethodInvoker implements IResponseGenerator { @Nullable // Available since Java 17 private static final Method INVOKE_DEFAULT = ReflectionUtil.getDeclaredMethodByName(InvocationHandler.class, "invokeDefault"); diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java index 24ac94e92c..9cc0d05dc9 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java @@ -15,7 +15,9 @@ package org.spockframework.mock.runtime; import org.spockframework.mock.*; +import org.spockframework.util.ThreadSafe; +@ThreadSafe public class FailingRealMethodInvoker implements IResponseGenerator { private final String message; diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java index 4ec2ca1cc0..6ea43702a8 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java @@ -17,7 +17,9 @@ import org.spockframework.mock.*; import groovy.lang.MetaClass; +import org.spockframework.util.ThreadSafe; +@ThreadSafe public class GroovyRealMethodInvoker implements IResponseGenerator { private final MetaClass metaClass; 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 77161856ae..14d19825e7 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 @@ -17,17 +17,23 @@ package org.spockframework.mock.runtime; import org.spockframework.mock.*; +import org.spockframework.util.NotThreadSafe; import org.spockframework.util.TextUtil; import java.util.*; import java.util.function.Supplier; -import java.util.stream.Collectors; /** * An anticipated interaction between the SUT and one or more mock objects. * + *

An instance of this class is confined to a {@link MockController} and all method calls + * should be done in {@code synchronized} blocks that synchronize on that Mock controller to + * ensure proper interaction matching and invocation counting. Due to that this class is not + * thread-safe. + * * @author Peter Niederwieser */ +@NotThreadSafe public class MockInteraction implements IMockInteraction { private final int line; private final int column; diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java index fc8dc7f916..c4658e2d73 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java @@ -1,7 +1,9 @@ package org.spockframework.mock.runtime; import org.spockframework.mock.*; +import org.spockframework.util.ThreadSafe; +@ThreadSafe public class ObjectMethodInvoker implements IResponseGenerator { public static final ObjectMethodInvoker INSTANCE = new ObjectMethodInvoker(); diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MultiThreaded.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MultiThreaded.groovy index 5b4550179f..2142490fe5 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MultiThreaded.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MultiThreaded.groovy @@ -42,13 +42,11 @@ class MultiThreaded extends Specification { def "mock response generators do not deadlock when self-referencing"() { given: def called = false - Runnable foo - foo = Mock { - run() >> { - if (!called) { - called = true - Thread.start { foo.run() }.join() - } + Runnable foo = Mock() + foo.run() >> { + if (!called) { + called = true + Thread.start { foo.run() }.join() } } From ae20b7de397ecd8a84f6570fb65063908526537a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Kautler?= Date: Fri, 8 Mar 2024 20:50:11 +0100 Subject: [PATCH 3/6] Review Feedback Vol. 2 --- docs/release_notes.adoc | 7 +- .../mock/CallRealMethodResponse.java | 6 +- .../mock/EmptyOrDummyResponse.java | 103 +++++++++--------- .../mock/IResponseGenerator.java | 4 +- .../mock/ZeroOrNullResponse.java | 14 ++- .../mock/response/CodeResponseGenerator.java | 16 ++- .../response/ConstantResponseGenerator.java | 6 +- .../response/DefaultResponseGenerator.java | 4 +- .../response/IterableResponseGenerator.java | 6 +- .../mock/response/ResponseGeneratorChain.java | 7 +- .../response/SingleResponseGenerator.java | 6 +- .../response/WildcardResponseGenerator.java | 4 +- .../mock/runtime/ByteBuddyMethodInvoker.java | 24 ++-- .../mock/runtime/CglibRealMethodInvoker.java | 18 +-- .../mock/runtime/DefaultMethodInvoker.java | 15 ++- .../runtime/FailingRealMethodInvoker.java | 8 +- .../mock/runtime/GroovyRealMethodInvoker.java | 22 ++-- .../mock/runtime/MockController.java | 2 +- .../mock/runtime/MockInteraction.java | 2 +- .../mock/runtime/MockInvocation.java | 4 +- .../mock/runtime/ObjectMethodInvoker.java | 16 ++- .../runtime/mockito/MockitoMockMakerImpl.java | 2 +- .../DetachedMockFactoryDocSpec.groovy | 15 ++- .../docs/interaction/StaticSpyDocSpec.groovy | 2 +- .../IterableResponseGeneratorSpec.groovy | 20 ++-- .../smoke/mock/MultiThreaded.groovy | 54 +++++++++ 26 files changed, 248 insertions(+), 139 deletions(-) diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc index 1f231d9a0c..396c0a9785 100644 --- a/docs/release_notes.adoc +++ b/docs/release_notes.adoc @@ -14,7 +14,12 @@ include::include.adoc[] Starting with this release, the response calculations are no longer happening synchronized. If you depend on shared state in such calculations, you now have to make sure yourself, that this is done in a thread-safe manner. Most users will probably be unaffected by this change as it only becomes relevant in a - multi-threaded situation where multiple threads do interaction invocations that care about shared state. + multi-threaded situation where multiple threads do interaction invocations that care about shared state. spockPull:1910[] + +* `IResponseGenerator#respond` no longer returns `Object` but `Supplier`. To get the actual object you now + have to additionally call `get()` on the supplied response, for example when you are using `EmptyOrDummyResponse` + somewhere directly. On the other hand, if you are implementing a custom `IResponseGenerator`, you have go change + your implementation accordingly. spockPull:1910[] === Misc diff --git a/spock-core/src/main/java/org/spockframework/mock/CallRealMethodResponse.java b/spock-core/src/main/java/org/spockframework/mock/CallRealMethodResponse.java index c4e1b67783..5ec11698e7 100644 --- a/spock-core/src/main/java/org/spockframework/mock/CallRealMethodResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/CallRealMethodResponse.java @@ -17,6 +17,8 @@ import org.spockframework.util.Beta; import org.spockframework.util.ThreadSafe; +import java.util.function.Supplier; + /** * A response strategy that delegates method calls to the real object underlying the mock (if any). */ @@ -28,7 +30,7 @@ public class CallRealMethodResponse implements IDefaultResponse { private CallRealMethodResponse() {} @Override - public Object respond(IMockInvocation invocation) { - return invocation.callRealMethod(); + public Supplier respond(IMockInvocation invocation) { + return invocation::callRealMethod; } } diff --git a/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java b/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java index 4faf562230..8f3076f9f4 100644 --- a/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java @@ -22,6 +22,7 @@ import java.math.*; import java.util.*; import java.util.concurrent.CompletableFuture; +import java.util.function.Supplier; import java.util.stream.*; import groovy.lang.*; @@ -40,68 +41,70 @@ private EmptyOrDummyResponse() {} @Override @SuppressWarnings("rawtypes") - public Object respond(IMockInvocation invocation) { - IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); - if (interaction != null) return interaction.accept(invocation).get(); + public Supplier respond(IMockInvocation invocation) { + return () -> { + IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); + if (interaction != null) return interaction.accept(invocation).get(); - Class returnType = invocation.getMethod().getReturnType(); + Class returnType = invocation.getMethod().getReturnType(); - if (returnType == void.class || returnType == Void.class) { - return null; - } + if (returnType == void.class || returnType == Void.class) { + return null; + } - if (returnType.isPrimitive()) { - return ReflectionUtil.getDefaultValue(returnType); - } + if (returnType.isPrimitive()) { + return ReflectionUtil.getDefaultValue(returnType); + } - if (returnType != Object.class && returnType.isAssignableFrom(invocation.getMockObject().getType())) { - return invocation.getMockObject().getInstance(); - } + if (returnType != Object.class && returnType.isAssignableFrom(invocation.getMockObject().getType())) { + return invocation.getMockObject().getInstance(); + } - if (returnType.isInterface()) { - if (returnType == Iterable.class) return new ArrayList(); - if (returnType == Collection.class) return new ArrayList(); - if (returnType == List.class) return new ArrayList(); - if (returnType == Set.class) return new HashSet(); - if (returnType == Map.class) return new HashMap(); - if (returnType == Queue.class) return new LinkedList(); - if (returnType == SortedSet.class) return new TreeSet(); - if (returnType == SortedMap.class) return new TreeMap(); - if (returnType == CharSequence.class) return ""; - if (returnType == Stream.class) return Stream.empty(); - if (returnType == IntStream.class) return IntStream.empty(); - if (returnType == DoubleStream.class) return DoubleStream.empty(); - if (returnType == LongStream.class) return LongStream.empty(); - return createDummy(invocation); - } + if (returnType.isInterface()) { + if (returnType == Iterable.class) return new ArrayList(); + if (returnType == Collection.class) return new ArrayList(); + if (returnType == List.class) return new ArrayList(); + if (returnType == Set.class) return new HashSet(); + if (returnType == Map.class) return new HashMap(); + if (returnType == Queue.class) return new LinkedList(); + if (returnType == SortedSet.class) return new TreeSet(); + if (returnType == SortedMap.class) return new TreeMap(); + if (returnType == CharSequence.class) return ""; + if (returnType == Stream.class) return Stream.empty(); + if (returnType == IntStream.class) return IntStream.empty(); + if (returnType == DoubleStream.class) return DoubleStream.empty(); + if (returnType == LongStream.class) return LongStream.empty(); + return createDummy(invocation); + } - if (returnType.isArray()) { - return Array.newInstance(returnType.getComponentType(), 0); - } + if (returnType.isArray()) { + return Array.newInstance(returnType.getComponentType(), 0); + } - if (returnType.isEnum()) { - Object[] enumConstants = returnType.getEnumConstants(); - return enumConstants.length > 0 ? enumConstants[0] : null; // null is only permissible value - } + if (returnType.isEnum()) { + Object[] enumConstants = returnType.getEnumConstants(); + return enumConstants.length > 0 ? enumConstants[0] : null; // null is only permissible value + } - if (CharSequence.class.isAssignableFrom(returnType)) { - if (returnType == String.class) return ""; - if (returnType == StringBuilder.class) return new StringBuilder(); - if (returnType == StringBuffer.class) return new StringBuffer(); - if (returnType == GString.class) return GString.EMPTY; - // continue on - } + if (CharSequence.class.isAssignableFrom(returnType)) { + if (returnType == String.class) return ""; + if (returnType == StringBuilder.class) return new StringBuilder(); + if (returnType == StringBuffer.class) return new StringBuffer(); + if (returnType == GString.class) return GString.EMPTY; + // continue on + } - if (returnType == Optional.class) return Optional.empty(); - if (returnType == CompletableFuture.class) return CompletableFuture.completedFuture(null); + if (returnType == Optional.class) return Optional.empty(); + if (returnType == CompletableFuture.class) return CompletableFuture.completedFuture(null); - Object emptyWrapper = createEmptyWrapper(returnType); - if (emptyWrapper != null) return emptyWrapper; + Object emptyWrapper = createEmptyWrapper(returnType); + if (emptyWrapper != null) return emptyWrapper; - Object emptyObject = createEmptyObject(returnType); - if (emptyObject != null) return emptyObject; + Object emptyObject = createEmptyObject(returnType); + if (emptyObject != null) return emptyObject; - return createDummy(invocation); + return createDummy(invocation); + }; } // also handles some numeric types which aren't primitive wrapper types diff --git a/spock-core/src/main/java/org/spockframework/mock/IResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/IResponseGenerator.java index a43e3ce95b..7b9e0618aa 100644 --- a/spock-core/src/main/java/org/spockframework/mock/IResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/IResponseGenerator.java @@ -16,10 +16,12 @@ import org.spockframework.util.Beta; +import java.util.function.Supplier; + /** * Generates responses to mock invocations. */ @Beta public interface IResponseGenerator { - Object respond(IMockInvocation invocation); + Supplier respond(IMockInvocation invocation); } diff --git a/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java b/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java index 15464f00ef..49e1de462b 100644 --- a/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java @@ -16,6 +16,8 @@ import org.spockframework.util.*; +import java.util.function.Supplier; + /** * A response strategy that returns zero, false, or null, depending on the method's return type. */ @@ -27,11 +29,13 @@ public class ZeroOrNullResponse implements IDefaultResponse { private ZeroOrNullResponse() {} @Override - public Object respond(IMockInvocation invocation) { - IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); - if (interaction != null) return interaction.accept(invocation).get(); + public Supplier respond(IMockInvocation invocation) { + return () -> { + IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); + if (interaction != null) return interaction.accept(invocation).get(); - Class returnType = invocation.getMethod().getReturnType(); - return ReflectionUtil.getDefaultValue(returnType); + Class returnType = invocation.getMethod().getReturnType(); + return ReflectionUtil.getDefaultValue(returnType); + }; } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java index 75d1e69696..cff06fedf0 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java @@ -22,6 +22,8 @@ import groovy.lang.Closure; import org.spockframework.util.ThreadSafe; +import java.util.function.Supplier; + import static org.spockframework.util.ObjectUtil.uncheckedCast; /** @@ -37,14 +39,16 @@ public CodeResponseGenerator(Closure code) { } @Override - public Object doRespond(IMockInvocation invocation) { - Object result = invokeClosure(invocation); - Class returnType = invocation.getMethod().getReturnType(); + public Supplier doRespond(IMockInvocation invocation) { + return () -> { + Object result = invokeClosure(invocation); + Class returnType = invocation.getMethod().getReturnType(); - // don't attempt cast for void methods (closure could be an action that accidentally returns a value) - if (returnType == void.class || returnType == Void.class) return null; + // don't attempt cast for void methods (closure could be an action that accidentally returns a value) + if (returnType == void.class || returnType == Void.class) return null; - return GroovyRuntimeUtil.coerce(result, invocation.getMethod().getReturnType()); + return GroovyRuntimeUtil.coerce(result, invocation.getMethod().getReturnType()); + }; } private Object invokeClosure(IMockInvocation invocation) { diff --git a/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java index 8328a269c9..5fd9dc673e 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java @@ -20,6 +20,8 @@ import org.spockframework.runtime.GroovyRuntimeUtil; import org.spockframework.util.ThreadSafe; +import java.util.function.Supplier; + /** * * @author Peter Niederwieser @@ -33,7 +35,7 @@ public ConstantResponseGenerator(Object constant) { } @Override - public Object doRespond(IMockInvocation invocation) { - return GroovyRuntimeUtil.coerce(constant, invocation.getMethod().getReturnType()); + public Supplier doRespond(IMockInvocation invocation) { + return () -> GroovyRuntimeUtil.coerce(constant, invocation.getMethod().getReturnType()); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/DefaultResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/DefaultResponseGenerator.java index 138ff4032b..9c2fc02bb9 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/DefaultResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/DefaultResponseGenerator.java @@ -19,10 +19,12 @@ import org.spockframework.mock.*; import org.spockframework.util.ThreadSafe; +import java.util.function.Supplier; + @ThreadSafe public class DefaultResponseGenerator implements IResponseGenerator { @Override - public Object respond(IMockInvocation invocation) { + public Supplier respond(IMockInvocation invocation) { return invocation.getMockObject().getDefaultResponse().respond(invocation); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java index e3e319f543..1fd7ceaeb9 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java @@ -21,6 +21,7 @@ import org.spockframework.util.ThreadSafe; import java.util.Iterator; +import java.util.function.Supplier; /** * Generates result values from an iterable object. If the iterator has no more @@ -46,10 +47,11 @@ public boolean isAtEndOfCycle() { } @Override - public Object respond(IMockInvocation invocation) { + public Supplier respond(IMockInvocation invocation) { synchronized (LOCK) { if (iterator.hasNext()) nextValue = iterator.next(); - return GroovyRuntimeUtil.coerce(nextValue, invocation.getMethod().getReturnType()); + Object response = GroovyRuntimeUtil.coerce(nextValue, invocation.getMethod().getReturnType()); + return () -> response; } } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java b/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java index fbe6b281ac..67a9ce717a 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java @@ -21,6 +21,7 @@ import org.spockframework.util.ThreadSafe; import java.util.LinkedList; +import java.util.function.Supplier; @ThreadSafe public class ResponseGeneratorChain implements IResponseGenerator { @@ -42,16 +43,14 @@ public boolean isEmpty() { } @Override - public Object respond(IMockInvocation invocation) { - IChainableResponseGenerator responseGenerator; + public Supplier respond(IMockInvocation invocation) { synchronized (LOCK) { if (isEmpty()) throw new InternalSpockError("ResultGeneratorChain should never be empty"); while (current.isAtEndOfCycle() && !remaining.isEmpty()) { current = remaining.removeFirst(); } - responseGenerator = current; + return current.respond(invocation); } - return responseGenerator.respond(invocation); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java index 2fcb599244..1d6af76486 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java @@ -19,6 +19,8 @@ import org.spockframework.mock.*; import org.spockframework.util.ThreadSafe; +import java.util.function.Supplier; + @ThreadSafe public abstract class SingleResponseGenerator implements IChainableResponseGenerator { private volatile boolean endOfCycle = false; @@ -29,10 +31,10 @@ public boolean isAtEndOfCycle() { } @Override - public final Object respond(IMockInvocation invocation) { + public final Supplier respond(IMockInvocation invocation) { endOfCycle = true; return doRespond(invocation); } - public abstract Object doRespond(IMockInvocation invocation); + public abstract Supplier doRespond(IMockInvocation invocation); } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java index fb3f4c7e71..d63569cc87 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java @@ -19,6 +19,8 @@ import org.spockframework.mock.*; import org.spockframework.util.ThreadSafe; +import java.util.function.Supplier; + /** * Returns a value using {@code Stub} semantics (see {@link EmptyOrDummyResponse}). * @@ -31,7 +33,7 @@ @ThreadSafe public class WildcardResponseGenerator extends SingleResponseGenerator { @Override - public Object doRespond(IMockInvocation invocation) { + public Supplier doRespond(IMockInvocation invocation) { return EmptyOrDummyResponse.INSTANCE.respond(invocation); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java index 5a93466c11..06952b8484 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java @@ -4,6 +4,8 @@ import org.spockframework.util.ExceptionUtil; import org.spockframework.util.ThreadSafe; +import java.util.function.Supplier; + @ThreadSafe public class ByteBuddyMethodInvoker implements IResponseGenerator { @@ -14,15 +16,17 @@ public ByteBuddyMethodInvoker(ByteBuddyInvoker superCall) { } @Override - public Object respond(IMockInvocation invocation) { - if (superCall == null) { - throw new IllegalStateException("Cannot invoke abstract method " + invocation.getMethod()); - } - try { - return superCall.call(invocation.getArguments().toArray()); - } catch (Throwable t) { - // Byte Buddy doesn't wrap exceptions in InvocationTargetException, so no need to unwrap - return ExceptionUtil.sneakyThrow(t); - } + public Supplier respond(IMockInvocation invocation) { + return () -> { + if (superCall == null) { + throw new IllegalStateException("Cannot invoke abstract method " + invocation.getMethod()); + } + try { + return superCall.call(invocation.getArguments().toArray()); + } catch (Throwable t) { + // Byte Buddy doesn't wrap exceptions in InvocationTargetException, so no need to unwrap + return ExceptionUtil.sneakyThrow(t); + } + }; } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java index b12e2c091d..8ab6c32cd4 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java @@ -20,6 +20,8 @@ import net.sf.cglib.proxy.MethodProxy; import org.spockframework.util.ThreadSafe; +import java.util.function.Supplier; + @ThreadSafe public class CglibRealMethodInvoker implements IResponseGenerator { private final MethodProxy methodProxy; @@ -29,12 +31,14 @@ public CglibRealMethodInvoker(MethodProxy methodProxy) { } @Override - public Object respond(IMockInvocation invocation) { - try { - return methodProxy.invokeSuper(invocation.getMockObject().getInstance(), invocation.getArguments().toArray()); - } catch (Throwable t) { - // MethodProxy doesn't wrap exceptions in InvocationTargetException, so no need to unwrap - return ExceptionUtil.sneakyThrow(t); - } + public Supplier respond(IMockInvocation invocation) { + return () -> { + try { + return methodProxy.invokeSuper(invocation.getMockObject().getInstance(), invocation.getArguments().toArray()); + } catch (Throwable t) { + // MethodProxy doesn't wrap exceptions in InvocationTargetException, so no need to unwrap + return ExceptionUtil.sneakyThrow(t); + } + }; } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java index f2747a29ef..362eb8c3c9 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java @@ -27,6 +27,7 @@ import java.lang.reflect.Field; import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; +import java.util.function.Supplier; @ThreadSafe public class DefaultMethodInvoker implements IResponseGenerator { @@ -43,12 +44,14 @@ public DefaultMethodInvoker(Object target, Method method, Object[] arguments) { } @Override - public Object respond(IMockInvocation invocation) { - if (INVOKE_DEFAULT == null) { - return useInternalMethodHandle(); - } - Object[] args = new Object[]{target, method, arguments}; - return ReflectionUtil.invokeMethod(null, INVOKE_DEFAULT, args); + public Supplier respond(IMockInvocation invocation) { + return () -> { + if (INVOKE_DEFAULT == null) { + return useInternalMethodHandle(); + } + Object[] args = new Object[]{target, method, arguments}; + return ReflectionUtil.invokeMethod(null, INVOKE_DEFAULT, args); + }; } private Object useInternalMethodHandle() { diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java index 9cc0d05dc9..0b58b7ac9f 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java @@ -17,6 +17,8 @@ import org.spockframework.mock.*; import org.spockframework.util.ThreadSafe; +import java.util.function.Supplier; + @ThreadSafe public class FailingRealMethodInvoker implements IResponseGenerator { private final String message; @@ -26,7 +28,9 @@ public FailingRealMethodInvoker(String message) { } @Override - public Object respond(IMockInvocation invocation) { - throw new CannotInvokeRealMethodException(message); + public Supplier respond(IMockInvocation invocation) { + return () -> { + throw new CannotInvokeRealMethodException(message); + }; } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java index 6ea43702a8..2e83b5cc97 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java @@ -19,6 +19,8 @@ import groovy.lang.MetaClass; import org.spockframework.util.ThreadSafe; +import java.util.function.Supplier; + @ThreadSafe public class GroovyRealMethodInvoker implements IResponseGenerator { private final MetaClass metaClass; @@ -28,15 +30,17 @@ public GroovyRealMethodInvoker(MetaClass metaClass) { } @Override - public Object respond(IMockInvocation invocation) { - Object instance = invocation.getMockObject().getInstance(); - Object[] arguments = invocation.getArguments().toArray(); - if (invocation.getMethod().isStatic()) { - if ("".equals(invocation.getMethod().getName())) { - return metaClass.invokeConstructor(arguments); + public Supplier respond(IMockInvocation invocation) { + return () -> { + Object instance = invocation.getMockObject().getInstance(); + Object[] arguments = invocation.getArguments().toArray(); + if (invocation.getMethod().isStatic()) { + if ("".equals(invocation.getMethod().getName())) { + return metaClass.invokeConstructor(arguments); + } + return metaClass.invokeStaticMethod(instance, invocation.getMethod().getName(), arguments); } - return metaClass.invokeStaticMethod(instance, invocation.getMethod().getName(), arguments); - } - return metaClass.invokeMethod(instance, invocation.getMethod().getName(), arguments); + return metaClass.invokeMethod(instance, invocation.getMethod().getName(), arguments); + }; } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java b/spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java index 26ad1270b9..49d3d5eaf8 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java @@ -57,7 +57,7 @@ public Object handle(IMockInvocation invocation) { } } if (resultSupplier == null) { - return invocation.getMockObject().getDefaultResponse().respond(invocation); + return invocation.getMockObject().getDefaultResponse().respond(invocation).get(); } else { try { return resultSupplier.get(); 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 14d19825e7..9e46ac3a73 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 @@ -81,7 +81,7 @@ public Supplier accept(IMockInvocation invocation) { }; } - return () -> responseGenerator == null ? null : responseGenerator.respond(invocation); + return responseGenerator == null ? () -> null : responseGenerator.respond(invocation); } @Override diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/MockInvocation.java b/spock-core/src/main/java/org/spockframework/mock/runtime/MockInvocation.java index 5007ff6d75..9c192586e8 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/MockInvocation.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/MockInvocation.java @@ -58,7 +58,7 @@ public List getArguments() { @Override public Object callRealMethod() { - return realMethodInvoker.respond(this); + return realMethodInvoker.respond(this).get(); } @Override @@ -68,7 +68,7 @@ public Object callRealMethodWithArgs(final Object... arguments) { public List getArguments() { return asList(arguments); } - }); + }).get(); } @Override diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java index c4658e2d73..8742b95319 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java @@ -3,6 +3,8 @@ import org.spockframework.mock.*; import org.spockframework.util.ThreadSafe; +import java.util.function.Supplier; + @ThreadSafe public class ObjectMethodInvoker implements IResponseGenerator { @@ -12,12 +14,14 @@ private ObjectMethodInvoker() { } @Override - public Object respond(IMockInvocation invocation) { - IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); - if (interaction != null) { - return interaction.accept(invocation).get(); - } + public Supplier respond(IMockInvocation invocation) { + return () -> { + IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); + if (interaction != null) { + return interaction.accept(invocation).get(); + } - return null; + return null; + }; } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java b/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java index 55ae77f23c..a9232d04bd 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java @@ -265,7 +265,7 @@ public SpockMockHandler(IProxyBasedMockInterceptor mockInterceptor) { @Override public Object handle(Invocation invocation) { Object mock = invocation.getMock(); - return mockInterceptor.intercept(mock, invocation.getMethod(), invocation.getArguments(), spockInvocation -> { + return mockInterceptor.intercept(mock, invocation.getMethod(), invocation.getArguments(), spockInvocation -> () -> { try { return invocation.callRealMethod(); } catch (Throwable e) { diff --git a/spock-specs/src/test/groovy/org/spockframework/docs/interaction/DetachedMockFactoryDocSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/docs/interaction/DetachedMockFactoryDocSpec.groovy index c56645f147..09347b8d4a 100644 --- a/spock-specs/src/test/groovy/org/spockframework/docs/interaction/DetachedMockFactoryDocSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/docs/interaction/DetachedMockFactoryDocSpec.groovy @@ -11,6 +11,7 @@ import spock.mock.AutoAttach import spock.mock.DetachedMockFactory import java.util.concurrent.ThreadLocalRandom +import java.util.function.Supplier import static org.spockframework.docs.interaction.DetachedMockFactoryDocSpec.EngineMockCreator.StartMode.ALWAYS_STARTED import static org.spockframework.docs.interaction.DetachedMockFactoryDocSpec.EngineMockCreator.StartMode.ALWAYS_STOPPED @@ -189,12 +190,14 @@ class DetachedMockFactoryDocSpec extends Specification { StartMode startMode @Override - Object respond(IMockInvocation invocation) { - if (invocation.method.name != 'isStarted') - return ZeroOrNullResponse.INSTANCE.respond(invocation) - startMode == RANDOMLY_STARTED - ? ThreadLocalRandom.current().nextBoolean() - : startMode == ALWAYS_STARTED + Supplier respond(IMockInvocation invocation) { + return { + if (invocation.method.name != 'isStarted') + return ZeroOrNullResponse.INSTANCE.respond(invocation).get() + startMode == RANDOMLY_STARTED + ? ThreadLocalRandom.current().nextBoolean() + : startMode == ALWAYS_STARTED + } } } diff --git a/spock-specs/src/test/groovy/org/spockframework/docs/interaction/StaticSpyDocSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/docs/interaction/StaticSpyDocSpec.groovy index a5d8ef885c..cdb523b48f 100644 --- a/spock-specs/src/test/groovy/org/spockframework/docs/interaction/StaticSpyDocSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/docs/interaction/StaticSpyDocSpec.groovy @@ -54,7 +54,7 @@ class StaticSpyDocSpec extends Specification { def "SpyStatic mock all static methods"() { given: SpyStatic(StaticClass) - StaticClass._ >> { ZeroOrNullResponse.INSTANCE.respond(delegate) } + StaticClass._ >> { ZeroOrNullResponse.INSTANCE.respond(delegate).get() } expect: StaticClass.staticMethod() == null diff --git a/spock-specs/src/test/groovy/org/spockframework/mock/response/IterableResponseGeneratorSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/mock/response/IterableResponseGeneratorSpec.groovy index 64b50c6cd8..fd9352effc 100644 --- a/spock-specs/src/test/groovy/org/spockframework/mock/response/IterableResponseGeneratorSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/mock/response/IterableResponseGeneratorSpec.groovy @@ -31,10 +31,10 @@ class IterableResponseGeneratorSpec extends Specification { inv.getMethod() >> new StaticMockMethod(method, Object) expect: - gen.respond(inv) == 1 - gen.respond(inv) == 2 - gen.respond(inv) == 3 - gen.respond(inv) == 3 + gen.respond(inv).get() == 1 + gen.respond(inv).get() == 2 + gen.respond(inv).get() == 3 + gen.respond(inv).get() == 3 } def "iterate over empty list"() { @@ -43,8 +43,8 @@ class IterableResponseGeneratorSpec extends Specification { inv.getMethod() >> new StaticMockMethod(method, Object) expect: - gen.respond(inv) == null - gen.respond(inv) == null + gen.respond(inv).get() == null + gen.respond(inv).get() == null } def "iterate over string"() { @@ -53,9 +53,9 @@ class IterableResponseGeneratorSpec extends Specification { inv.getMethod() >> new StaticMockMethod(method, Object) expect: - gen.respond(inv) == "a" - gen.respond(inv) == "b" - gen.respond(inv) == "c" - gen.respond(inv) == "c" + gen.respond(inv).get() == "a" + gen.respond(inv).get() == "b" + gen.respond(inv).get() == "c" + gen.respond(inv).get() == "c" } } diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MultiThreaded.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MultiThreaded.groovy index 2142490fe5..cb24ce54b7 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MultiThreaded.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MultiThreaded.groovy @@ -37,6 +37,60 @@ class MultiThreaded extends Specification { } } + @Issue("https://github.com/spockframework/spock/issues/583") + def "mocks can block and wait on conditions in a response chain"() { + given: + CountDownLatch latch1 = new CountDownLatch(1) + CountDownLatch latch2 = new CountDownLatch(1) + CountDownLatch latch3 = new CountDownLatch(2) + List aList = Mock() + + when: + Thread.start { aList.get(0); latch3.countDown() } + Thread.start { aList.get(0); latch3.countDown() } + assert latch1.await(30, SECONDS) + assert latch2.await(30, SECONDS) + + then: + latch3.await(10, SECONDS) + + 2 * aList.get(_) >> { + latch1.countDown() + assert latch2.await(10, SECONDS) + } >> { + latch2.countDown() + assert latch1.await(10, SECONDS) + } + } + + @Issue("https://github.com/spockframework/spock/issues/583") + def "mocks can block and wait on conditions in the contents of a response chain result"() { + given: + CountDownLatch latch1 = new CountDownLatch(1) + CountDownLatch latch2 = new CountDownLatch(1) + CountDownLatch latch3 = new CountDownLatch(2) + List aList = Mock() + + when: + Thread.start { aList.get(0)(); latch3.countDown() } + Thread.start { aList.get(0)(); latch3.countDown() } + assert latch1.await(30, SECONDS) + assert latch2.await(30, SECONDS) + + then: + latch3.await(10, SECONDS) + + 2 * aList.get(_) >>> [ + { + latch1.countDown() + assert latch2.await(10, SECONDS) + }, { + latch2.countDown() + assert latch1.await(10, SECONDS) + } + ] + } + @Issue("https://github.com/spockframework/spock/issues/1899") @Timeout(30) def "mock response generators do not deadlock when self-referencing"() { From aa33d71085e437a2fbc756b4ec78128abece5676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Kautler?= Date: Sun, 10 Mar 2024 23:15:40 +0100 Subject: [PATCH 4/6] Review Feedback Vol. 3 --- docs/release_notes.adoc | 13 ++++-- .../mock/CallRealMethodResponse.java | 2 +- .../mock/EmptyOrDummyResponse.java | 2 +- .../mock/IResponseGenerator.java | 44 +++++++++++++++++- .../mock/ZeroOrNullResponse.java | 2 +- .../mock/response/CodeResponseGenerator.java | 2 +- .../response/ConstantResponseGenerator.java | 2 +- .../response/DefaultResponseGenerator.java | 4 +- .../response/IterableResponseGenerator.java | 2 +- .../mock/response/ResponseGeneratorChain.java | 4 +- .../response/SingleResponseGenerator.java | 46 +++++++++++++++++-- .../response/WildcardResponseGenerator.java | 4 +- .../mock/runtime/ByteBuddyMethodInvoker.java | 2 +- .../mock/runtime/CglibRealMethodInvoker.java | 2 +- .../mock/runtime/DefaultMethodInvoker.java | 2 +- .../runtime/FailingRealMethodInvoker.java | 2 +- .../mock/runtime/GroovyRealMethodInvoker.java | 2 +- .../mock/runtime/MockController.java | 2 +- .../mock/runtime/MockInteraction.java | 2 +- .../mock/runtime/MockInvocation.java | 4 +- .../mock/runtime/ObjectMethodInvoker.java | 2 +- .../runtime/mockito/MockitoMockMakerImpl.java | 20 ++++---- .../DetachedMockFactoryDocSpec.groovy | 4 +- .../docs/interaction/StaticSpyDocSpec.groovy | 2 +- .../IterableResponseGeneratorSpec.groovy | 20 ++++---- 25 files changed, 141 insertions(+), 52 deletions(-) diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc index 396c0a9785..765d3d3a56 100644 --- a/docs/release_notes.adoc +++ b/docs/release_notes.adoc @@ -16,10 +16,15 @@ include::include.adoc[] thread-safe manner. Most users will probably be unaffected by this change as it only becomes relevant in a multi-threaded situation where multiple threads do interaction invocations that care about shared state. spockPull:1910[] -* `IResponseGenerator#respond` no longer returns `Object` but `Supplier`. To get the actual object you now - have to additionally call `get()` on the supplied response, for example when you are using `EmptyOrDummyResponse` - somewhere directly. On the other hand, if you are implementing a custom `IResponseGenerator`, you have go change - your implementation accordingly. spockPull:1910[] +* `IResponseGenerator#respond` should no longer be called, but instead `getResponseSupplier`, which returns a + `Supplier` instead. This supplier can then be called outside of mock controller synchronization and thus + prevents deadlocks that could happen for example from response calculation closures otherwise. + If you are implementing a custom `IResponseGenerator`, you might consider implementing the new method instead of the + old `respond` one, which is mainly kept for backwards compatibility. spockPull:1910[] + +* Like the previous point it is the same for `SingleResponseGenerator#doRespond` and the new + `SingleResponseGenerator#doGetResponseSupplier` if you are sublcassing or calling `SingleResponseGenerator` + somewhere. spockPull:1910[] === Misc diff --git a/spock-core/src/main/java/org/spockframework/mock/CallRealMethodResponse.java b/spock-core/src/main/java/org/spockframework/mock/CallRealMethodResponse.java index 5ec11698e7..bbcae1fd3f 100644 --- a/spock-core/src/main/java/org/spockframework/mock/CallRealMethodResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/CallRealMethodResponse.java @@ -30,7 +30,7 @@ public class CallRealMethodResponse implements IDefaultResponse { private CallRealMethodResponse() {} @Override - public Supplier respond(IMockInvocation invocation) { + public Supplier getResponseSupplier(IMockInvocation invocation) { return invocation::callRealMethod; } } diff --git a/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java b/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java index 8f3076f9f4..3389317076 100644 --- a/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java @@ -41,7 +41,7 @@ private EmptyOrDummyResponse() {} @Override @SuppressWarnings("rawtypes") - public Supplier respond(IMockInvocation invocation) { + public Supplier getResponseSupplier(IMockInvocation invocation) { return () -> { IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); if (interaction != null) return interaction.accept(invocation).get(); diff --git a/spock-core/src/main/java/org/spockframework/mock/IResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/IResponseGenerator.java index 7b9e0618aa..8a0f3e8cfc 100644 --- a/spock-core/src/main/java/org/spockframework/mock/IResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/IResponseGenerator.java @@ -15,6 +15,7 @@ package org.spockframework.mock; import org.spockframework.util.Beta; +import org.spockframework.util.Nullable; import java.util.function.Supplier; @@ -23,5 +24,46 @@ */ @Beta public interface IResponseGenerator { - Supplier respond(IMockInvocation invocation); + /** + * Do not call this method from Spock 2.4 on! Some implementations of this interface have the need to overwrite + * {@link #getResponseSupplier(IMockInvocation)} instead and throw an exception if this method is called directly. + * Always use {@code getResponseSupplier} instead to retrieve a response from this response generator. + * + *

If you implement this interface and can directly provide the generated response, you can overwrite this + * method and use the default implementation of {@code getResponseSupplier} which uses this method. If you are in + * need to manage some state, like advancing an iterator or similar, or the actual supplying can deadlock, + * or you might throw an exception, you might want to implement {@code getResponseSupplier} instead and keep + * the default implementation of this method which throws an {@code UnsupportedOperationException}. + * + *

This method is mainly kept for backwards compatibility with code that implements this interface. + * Most usages where this method is called will most likely fail to execute now. + * + * @param invocation The invocation to generate a response for + * @return The generated response + * @throws UnsupportedOperationException if {@code getResponseSupplier} must be used instead + * @deprecated Use {@link #getResponseSupplier(IMockInvocation)} instead + */ + @Nullable + @Deprecated + default Object respond(IMockInvocation invocation) { + throw new UnsupportedOperationException("Call getResponseSupplier(invocation) instead"); + } + + /** + * This method should be used from Spock 2.4 on, to get a response generated by this response generator. + * Calling {@link #respond(IMockInvocation)} directly might throw an {@code UnsupportedOperationException}, if + * this response generator needs to do some state maintenance or other things in this method's implementation. + * + *

If you implement this interface and can directly provide the generated response, you can overwrite + * {@code respond} and use the default implementation of this method which uses that method. If you are in + * need to manage some state, like advancing an iterator or similar, or the actual supplying can deadlock, + * or you might throw an exception, you might want to implement this method instead and keep the default + * implementation of {@code respond} which throws an {@code UnsupportedOperationException}. + * + * @param invocation The invocation to generate a response for + * @return A supplier that provides the generated response + */ + default Supplier getResponseSupplier(IMockInvocation invocation) { + return () -> respond(invocation); + } } diff --git a/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java b/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java index 49e1de462b..1f6d621dc3 100644 --- a/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java @@ -29,7 +29,7 @@ public class ZeroOrNullResponse implements IDefaultResponse { private ZeroOrNullResponse() {} @Override - public Supplier respond(IMockInvocation invocation) { + public Supplier getResponseSupplier(IMockInvocation invocation) { return () -> { IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); if (interaction != null) return interaction.accept(invocation).get(); diff --git a/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java index cff06fedf0..7391018b9d 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java @@ -39,7 +39,7 @@ public CodeResponseGenerator(Closure code) { } @Override - public Supplier doRespond(IMockInvocation invocation) { + public Supplier doGetResponseSupplier(IMockInvocation invocation) { return () -> { Object result = invokeClosure(invocation); Class returnType = invocation.getMethod().getReturnType(); diff --git a/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java index 5fd9dc673e..fe36917b63 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java @@ -35,7 +35,7 @@ public ConstantResponseGenerator(Object constant) { } @Override - public Supplier doRespond(IMockInvocation invocation) { + public Supplier doGetResponseSupplier(IMockInvocation invocation) { return () -> GroovyRuntimeUtil.coerce(constant, invocation.getMethod().getReturnType()); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/DefaultResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/DefaultResponseGenerator.java index 9c2fc02bb9..bff5d13409 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/DefaultResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/DefaultResponseGenerator.java @@ -24,7 +24,7 @@ @ThreadSafe public class DefaultResponseGenerator implements IResponseGenerator { @Override - public Supplier respond(IMockInvocation invocation) { - return invocation.getMockObject().getDefaultResponse().respond(invocation); + public Supplier getResponseSupplier(IMockInvocation invocation) { + return invocation.getMockObject().getDefaultResponse().getResponseSupplier(invocation); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java index 1fd7ceaeb9..28672dea97 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/IterableResponseGenerator.java @@ -47,7 +47,7 @@ public boolean isAtEndOfCycle() { } @Override - public Supplier respond(IMockInvocation invocation) { + public Supplier getResponseSupplier(IMockInvocation invocation) { synchronized (LOCK) { if (iterator.hasNext()) nextValue = iterator.next(); Object response = GroovyRuntimeUtil.coerce(nextValue, invocation.getMethod().getReturnType()); diff --git a/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java b/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java index 67a9ce717a..1087d37f3a 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/ResponseGeneratorChain.java @@ -43,14 +43,14 @@ public boolean isEmpty() { } @Override - public Supplier respond(IMockInvocation invocation) { + public Supplier getResponseSupplier(IMockInvocation invocation) { synchronized (LOCK) { if (isEmpty()) throw new InternalSpockError("ResultGeneratorChain should never be empty"); while (current.isAtEndOfCycle() && !remaining.isEmpty()) { current = remaining.removeFirst(); } - return current.respond(invocation); + return current.getResponseSupplier(invocation); } } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java index 1d6af76486..f25e30a275 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java @@ -31,10 +31,50 @@ public boolean isAtEndOfCycle() { } @Override - public final Supplier respond(IMockInvocation invocation) { + public final Supplier getResponseSupplier(IMockInvocation invocation) { endOfCycle = true; - return doRespond(invocation); + return doGetResponseSupplier(invocation); } - public abstract Supplier doRespond(IMockInvocation invocation); + /** + * Do not call this method from Spock 2.4 on! Some subclasses of this class have the need to overwrite + * {@link #doGetResponseSupplier(IMockInvocation)} instead and throw an exception if this method is called directly. + * Always use {@code doGetResponseSupplier} instead to retrieve a response from this single response generator. + * + *

If you subclass this class and can directly provide the generated response, you can overwrite this + * method and use the default implementation of {@code doGetResponseSupplier} which uses this method. If you are in + * need to manage some state, like advancing an iterator or similar, or the actual supplying can deadlock, + * or you might throw an exception, you might want to overwrite {@code doGetResponseSupplier} instead and keep + * the default implementation of this method which throws an {@code UnsupportedOperationException}. + * + *

This method is mainly kept for backwards compatibility with code that subclasses this class. + * Most usages where this method is called will most likely fail to execute now. + * + * @param invocation The invocation to generate a response for + * @return The generated response + * @throws UnsupportedOperationException if {@code doGetResponseSupplier} must be used instead + * @deprecated Use {@link #doGetResponseSupplier(IMockInvocation)} instead + */ + @Deprecated + public Object doRespond(IMockInvocation invocation) { + throw new UnsupportedOperationException("Call doGetResponseSupplier(invocation) instead"); + } + + /** + * This method should be used from Spock 2.4 on, to get a response generated by this single response generator. + * Calling {@link #doRespond(IMockInvocation)} directly might throw an {@code UnsupportedOperationException}, if + * this single response generator needs to do some state maintenance or other things in this method's implementation. + * + *

If you subclass this class and can directly provide the generated response, you can overwrite + * {@code doRespond} and use the default implementation of this method which uses that method. If you are in + * need to manage some state, like advancing an iterator or similar, or the actual supplying can deadlock, + * or you might throw an exception, you might want to override this method instead and keep the default + * implementation of {@code doRespond} which throws an {@code UnsupportedOperationException}. + * + * @param invocation The invocation to generate a response for + * @return A supplier that provides the generated response + */ + public Supplier doGetResponseSupplier(IMockInvocation invocation) { + return () -> doRespond(invocation); + } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java index d63569cc87..86d9ef7534 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java @@ -33,7 +33,7 @@ @ThreadSafe public class WildcardResponseGenerator extends SingleResponseGenerator { @Override - public Supplier doRespond(IMockInvocation invocation) { - return EmptyOrDummyResponse.INSTANCE.respond(invocation); + public Supplier doGetResponseSupplier(IMockInvocation invocation) { + return EmptyOrDummyResponse.INSTANCE.getResponseSupplier(invocation); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java index 06952b8484..dae84eca3f 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java @@ -16,7 +16,7 @@ public ByteBuddyMethodInvoker(ByteBuddyInvoker superCall) { } @Override - public Supplier respond(IMockInvocation invocation) { + public Supplier getResponseSupplier(IMockInvocation invocation) { return () -> { if (superCall == null) { throw new IllegalStateException("Cannot invoke abstract method " + invocation.getMethod()); diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java index 8ab6c32cd4..4a97f0d979 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java @@ -31,7 +31,7 @@ public CglibRealMethodInvoker(MethodProxy methodProxy) { } @Override - public Supplier respond(IMockInvocation invocation) { + public Supplier getResponseSupplier(IMockInvocation invocation) { return () -> { try { return methodProxy.invokeSuper(invocation.getMockObject().getInstance(), invocation.getArguments().toArray()); diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java index 362eb8c3c9..5fb2b0094b 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java @@ -44,7 +44,7 @@ public DefaultMethodInvoker(Object target, Method method, Object[] arguments) { } @Override - public Supplier respond(IMockInvocation invocation) { + public Supplier getResponseSupplier(IMockInvocation invocation) { return () -> { if (INVOKE_DEFAULT == null) { return useInternalMethodHandle(); diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java index 0b58b7ac9f..8aff365caa 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java @@ -28,7 +28,7 @@ public FailingRealMethodInvoker(String message) { } @Override - public Supplier respond(IMockInvocation invocation) { + public Supplier getResponseSupplier(IMockInvocation invocation) { return () -> { throw new CannotInvokeRealMethodException(message); }; diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java index 2e83b5cc97..7458ae76f9 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java @@ -30,7 +30,7 @@ public GroovyRealMethodInvoker(MetaClass metaClass) { } @Override - public Supplier respond(IMockInvocation invocation) { + public Supplier getResponseSupplier(IMockInvocation invocation) { return () -> { Object instance = invocation.getMockObject().getInstance(); Object[] arguments = invocation.getArguments().toArray(); diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java b/spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java index 49d3d5eaf8..4bb8e4351d 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java @@ -57,7 +57,7 @@ public Object handle(IMockInvocation invocation) { } } if (resultSupplier == null) { - return invocation.getMockObject().getDefaultResponse().respond(invocation).get(); + return invocation.getMockObject().getDefaultResponse().getResponseSupplier(invocation).get(); } else { try { return resultSupplier.get(); 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 9e46ac3a73..4971e59033 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 @@ -81,7 +81,7 @@ public Supplier accept(IMockInvocation invocation) { }; } - return responseGenerator == null ? () -> null : responseGenerator.respond(invocation); + return responseGenerator == null ? () -> null : responseGenerator.getResponseSupplier(invocation); } @Override diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/MockInvocation.java b/spock-core/src/main/java/org/spockframework/mock/runtime/MockInvocation.java index 9c192586e8..cc08144232 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/MockInvocation.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/MockInvocation.java @@ -58,12 +58,12 @@ public List getArguments() { @Override public Object callRealMethod() { - return realMethodInvoker.respond(this).get(); + return realMethodInvoker.getResponseSupplier(this).get(); } @Override public Object callRealMethodWithArgs(final Object... arguments) { - return realMethodInvoker.respond(new DelegatingMockInvocation(this) { + return realMethodInvoker.getResponseSupplier(new DelegatingMockInvocation(this) { @Override public List getArguments() { return asList(arguments); diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java index 8742b95319..5d062d5f60 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java @@ -14,7 +14,7 @@ private ObjectMethodInvoker() { } @Override - public Supplier respond(IMockInvocation invocation) { + public Supplier getResponseSupplier(IMockInvocation invocation) { return () -> { IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); if (interaction != null) { diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java b/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java index a9232d04bd..2f75df0c32 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java @@ -29,10 +29,7 @@ import org.mockito.mock.MockCreationSettings; import org.mockito.plugins.MockMaker; import org.mockito.plugins.MockitoPlugins; -import org.spockframework.mock.CannotCreateMockException; -import org.spockframework.mock.IMockObject; -import org.spockframework.mock.ISpockMockObject; -import org.spockframework.mock.MockNature; +import org.spockframework.mock.*; import org.spockframework.mock.runtime.IMockMaker; import org.spockframework.mock.runtime.IProxyBasedMockInterceptor; import org.spockframework.runtime.GroovyRuntimeUtil; @@ -265,11 +262,16 @@ public SpockMockHandler(IProxyBasedMockInterceptor mockInterceptor) { @Override public Object handle(Invocation invocation) { Object mock = invocation.getMock(); - return mockInterceptor.intercept(mock, invocation.getMethod(), invocation.getArguments(), spockInvocation -> () -> { - try { - return invocation.callRealMethod(); - } catch (Throwable e) { - return ExceptionUtil.sneakyThrow(e); + return mockInterceptor.intercept(mock, invocation.getMethod(), invocation.getArguments(), new IResponseGenerator() { + @Override + public Supplier getResponseSupplier(IMockInvocation __) { + return () -> { + try { + return invocation.callRealMethod(); + } catch (Throwable e) { + return ExceptionUtil.sneakyThrow(e); + } + }; } }); } diff --git a/spock-specs/src/test/groovy/org/spockframework/docs/interaction/DetachedMockFactoryDocSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/docs/interaction/DetachedMockFactoryDocSpec.groovy index 09347b8d4a..a322fdbd0b 100644 --- a/spock-specs/src/test/groovy/org/spockframework/docs/interaction/DetachedMockFactoryDocSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/docs/interaction/DetachedMockFactoryDocSpec.groovy @@ -190,10 +190,10 @@ class DetachedMockFactoryDocSpec extends Specification { StartMode startMode @Override - Supplier respond(IMockInvocation invocation) { + Supplier getResponseSupplier(IMockInvocation invocation) { return { if (invocation.method.name != 'isStarted') - return ZeroOrNullResponse.INSTANCE.respond(invocation).get() + return ZeroOrNullResponse.INSTANCE.getResponseSupplier(invocation).get() startMode == RANDOMLY_STARTED ? ThreadLocalRandom.current().nextBoolean() : startMode == ALWAYS_STARTED diff --git a/spock-specs/src/test/groovy/org/spockframework/docs/interaction/StaticSpyDocSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/docs/interaction/StaticSpyDocSpec.groovy index cdb523b48f..0d284ca3b9 100644 --- a/spock-specs/src/test/groovy/org/spockframework/docs/interaction/StaticSpyDocSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/docs/interaction/StaticSpyDocSpec.groovy @@ -54,7 +54,7 @@ class StaticSpyDocSpec extends Specification { def "SpyStatic mock all static methods"() { given: SpyStatic(StaticClass) - StaticClass._ >> { ZeroOrNullResponse.INSTANCE.respond(delegate).get() } + StaticClass._ >> { ZeroOrNullResponse.INSTANCE.getResponseSupplier(delegate).get() } expect: StaticClass.staticMethod() == null diff --git a/spock-specs/src/test/groovy/org/spockframework/mock/response/IterableResponseGeneratorSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/mock/response/IterableResponseGeneratorSpec.groovy index fd9352effc..fe3658cdc9 100644 --- a/spock-specs/src/test/groovy/org/spockframework/mock/response/IterableResponseGeneratorSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/mock/response/IterableResponseGeneratorSpec.groovy @@ -31,10 +31,10 @@ class IterableResponseGeneratorSpec extends Specification { inv.getMethod() >> new StaticMockMethod(method, Object) expect: - gen.respond(inv).get() == 1 - gen.respond(inv).get() == 2 - gen.respond(inv).get() == 3 - gen.respond(inv).get() == 3 + gen.getResponseSupplier(inv).get() == 1 + gen.getResponseSupplier(inv).get() == 2 + gen.getResponseSupplier(inv).get() == 3 + gen.getResponseSupplier(inv).get() == 3 } def "iterate over empty list"() { @@ -43,8 +43,8 @@ class IterableResponseGeneratorSpec extends Specification { inv.getMethod() >> new StaticMockMethod(method, Object) expect: - gen.respond(inv).get() == null - gen.respond(inv).get() == null + gen.getResponseSupplier(inv).get() == null + gen.getResponseSupplier(inv).get() == null } def "iterate over string"() { @@ -53,9 +53,9 @@ class IterableResponseGeneratorSpec extends Specification { inv.getMethod() >> new StaticMockMethod(method, Object) expect: - gen.respond(inv).get() == "a" - gen.respond(inv).get() == "b" - gen.respond(inv).get() == "c" - gen.respond(inv).get() == "c" + gen.getResponseSupplier(inv).get() == "a" + gen.getResponseSupplier(inv).get() == "b" + gen.getResponseSupplier(inv).get() == "c" + gen.getResponseSupplier(inv).get() == "c" } } From 4a4a4afcb593fbde97325b757eb8e283e44e5570 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Kautler?= Date: Mon, 11 Mar 2024 13:00:29 +0100 Subject: [PATCH 5/6] Review Feedback Vol. 4 --- docs/release_notes.adoc | 13 +-- .../mock/EmptyOrDummyResponse.java | 103 +++++++++--------- .../mock/IChainableResponseGenerator.java | 16 +++ .../mock/IResponseGenerator.java | 44 ++++---- .../mock/ZeroOrNullResponse.java | 14 +-- .../mock/response/CodeResponseGenerator.java | 16 +-- .../response/ConstantResponseGenerator.java | 6 +- .../response/SingleResponseGenerator.java | 42 +------ .../response/WildcardResponseGenerator.java | 6 +- .../mock/runtime/ByteBuddyMethodInvoker.java | 24 ++-- .../mock/runtime/CglibRealMethodInvoker.java | 18 ++- .../mock/runtime/DefaultMethodInvoker.java | 15 +-- .../runtime/FailingRealMethodInvoker.java | 8 +- .../mock/runtime/GroovyRealMethodInvoker.java | 22 ++-- .../mock/runtime/ObjectMethodInvoker.java | 16 +-- .../runtime/mockito/MockitoMockMakerImpl.java | 15 +-- .../DetachedMockFactoryDocSpec.groovy | 15 +-- 17 files changed, 163 insertions(+), 230 deletions(-) diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc index 765d3d3a56..c1a173b930 100644 --- a/docs/release_notes.adoc +++ b/docs/release_notes.adoc @@ -16,15 +16,10 @@ include::include.adoc[] thread-safe manner. Most users will probably be unaffected by this change as it only becomes relevant in a multi-threaded situation where multiple threads do interaction invocations that care about shared state. spockPull:1910[] -* `IResponseGenerator#respond` should no longer be called, but instead `getResponseSupplier`, which returns a - `Supplier` instead. This supplier can then be called outside of mock controller synchronization and thus - prevents deadlocks that could happen for example from response calculation closures otherwise. - If you are implementing a custom `IResponseGenerator`, you might consider implementing the new method instead of the - old `respond` one, which is mainly kept for backwards compatibility. spockPull:1910[] - -* Like the previous point it is the same for `SingleResponseGenerator#doRespond` and the new - `SingleResponseGenerator#doGetResponseSupplier` if you are sublcassing or calling `SingleResponseGenerator` - somewhere. spockPull:1910[] +* `SingleResponseGenerator#isAtEndOfCycle` is now `final` and `SingleResponseGenerator#doRespond` is now `protected`. + When subclassing `SingleResponseGenerator` it does not make sense to override the first method, and it does not make + sense to call the second method from somewhere else. This should not affect most users, only if you were + subclassing `SingleResponseGenerator` and doing unusual things. spockPull:1910[] === Misc diff --git a/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java b/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java index 3389317076..4faf562230 100644 --- a/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/EmptyOrDummyResponse.java @@ -22,7 +22,6 @@ import java.math.*; import java.util.*; import java.util.concurrent.CompletableFuture; -import java.util.function.Supplier; import java.util.stream.*; import groovy.lang.*; @@ -41,70 +40,68 @@ private EmptyOrDummyResponse() {} @Override @SuppressWarnings("rawtypes") - public Supplier getResponseSupplier(IMockInvocation invocation) { - return () -> { - IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); - if (interaction != null) return interaction.accept(invocation).get(); + public Object respond(IMockInvocation invocation) { + IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); + if (interaction != null) return interaction.accept(invocation).get(); - Class returnType = invocation.getMethod().getReturnType(); + Class returnType = invocation.getMethod().getReturnType(); - if (returnType == void.class || returnType == Void.class) { - return null; - } + if (returnType == void.class || returnType == Void.class) { + return null; + } - if (returnType.isPrimitive()) { - return ReflectionUtil.getDefaultValue(returnType); - } + if (returnType.isPrimitive()) { + return ReflectionUtil.getDefaultValue(returnType); + } - if (returnType != Object.class && returnType.isAssignableFrom(invocation.getMockObject().getType())) { - return invocation.getMockObject().getInstance(); - } + if (returnType != Object.class && returnType.isAssignableFrom(invocation.getMockObject().getType())) { + return invocation.getMockObject().getInstance(); + } - if (returnType.isInterface()) { - if (returnType == Iterable.class) return new ArrayList(); - if (returnType == Collection.class) return new ArrayList(); - if (returnType == List.class) return new ArrayList(); - if (returnType == Set.class) return new HashSet(); - if (returnType == Map.class) return new HashMap(); - if (returnType == Queue.class) return new LinkedList(); - if (returnType == SortedSet.class) return new TreeSet(); - if (returnType == SortedMap.class) return new TreeMap(); - if (returnType == CharSequence.class) return ""; - if (returnType == Stream.class) return Stream.empty(); - if (returnType == IntStream.class) return IntStream.empty(); - if (returnType == DoubleStream.class) return DoubleStream.empty(); - if (returnType == LongStream.class) return LongStream.empty(); - return createDummy(invocation); - } + if (returnType.isInterface()) { + if (returnType == Iterable.class) return new ArrayList(); + if (returnType == Collection.class) return new ArrayList(); + if (returnType == List.class) return new ArrayList(); + if (returnType == Set.class) return new HashSet(); + if (returnType == Map.class) return new HashMap(); + if (returnType == Queue.class) return new LinkedList(); + if (returnType == SortedSet.class) return new TreeSet(); + if (returnType == SortedMap.class) return new TreeMap(); + if (returnType == CharSequence.class) return ""; + if (returnType == Stream.class) return Stream.empty(); + if (returnType == IntStream.class) return IntStream.empty(); + if (returnType == DoubleStream.class) return DoubleStream.empty(); + if (returnType == LongStream.class) return LongStream.empty(); + return createDummy(invocation); + } - if (returnType.isArray()) { - return Array.newInstance(returnType.getComponentType(), 0); - } + if (returnType.isArray()) { + return Array.newInstance(returnType.getComponentType(), 0); + } - if (returnType.isEnum()) { - Object[] enumConstants = returnType.getEnumConstants(); - return enumConstants.length > 0 ? enumConstants[0] : null; // null is only permissible value - } + if (returnType.isEnum()) { + Object[] enumConstants = returnType.getEnumConstants(); + return enumConstants.length > 0 ? enumConstants[0] : null; // null is only permissible value + } - if (CharSequence.class.isAssignableFrom(returnType)) { - if (returnType == String.class) return ""; - if (returnType == StringBuilder.class) return new StringBuilder(); - if (returnType == StringBuffer.class) return new StringBuffer(); - if (returnType == GString.class) return GString.EMPTY; - // continue on - } + if (CharSequence.class.isAssignableFrom(returnType)) { + if (returnType == String.class) return ""; + if (returnType == StringBuilder.class) return new StringBuilder(); + if (returnType == StringBuffer.class) return new StringBuffer(); + if (returnType == GString.class) return GString.EMPTY; + // continue on + } - if (returnType == Optional.class) return Optional.empty(); - if (returnType == CompletableFuture.class) return CompletableFuture.completedFuture(null); + if (returnType == Optional.class) return Optional.empty(); + if (returnType == CompletableFuture.class) return CompletableFuture.completedFuture(null); - Object emptyWrapper = createEmptyWrapper(returnType); - if (emptyWrapper != null) return emptyWrapper; + Object emptyWrapper = createEmptyWrapper(returnType); + if (emptyWrapper != null) return emptyWrapper; - Object emptyObject = createEmptyObject(returnType); - if (emptyObject != null) return emptyObject; + Object emptyObject = createEmptyObject(returnType); + if (emptyObject != null) return emptyObject; - return createDummy(invocation); - }; + return createDummy(invocation); } // also handles some numeric types which aren't primitive wrapper types diff --git a/spock-core/src/main/java/org/spockframework/mock/IChainableResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/IChainableResponseGenerator.java index 3d9c43d604..a841a8aa6c 100644 --- a/spock-core/src/main/java/org/spockframework/mock/IChainableResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/IChainableResponseGenerator.java @@ -22,5 +22,21 @@ * @author Peter Niederwieser */ public interface IChainableResponseGenerator extends IResponseGenerator { + /** + * Whether this chainable response generator is at the end of its cycle. If this method returns {@code true}, + * the next response generator in the chain is used if one is available. If this method returns {@code false} or + * there is no next response generator in the chain, the {@link #getResponseSupplier(IMockInvocation)} method will + * be called further on for all matched invocations. + * + *

The {@code getResponseSupplier} method and this method will be called under a common lock to make sure code + * in {@code getResponseSupplier} can update state like setting a boolean or advancing an iterator, that is then + * checked in the implementation of this method. Such state updates should therefore be done within + * {@code getResponseSupplier} directly and not within the returned {@code Supplier} or in + * {@link #respond(IMockInvocation)}, otherwise the iteration matching algorithm will break. All other logic + * should preferably be done within the {@code Supplier}, especially if the code could deadlock like closures + * waiting for a common condition. + * + * @return Whether this chainable response generator is at the end of its cycle + */ boolean isAtEndOfCycle(); } diff --git a/spock-core/src/main/java/org/spockframework/mock/IResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/IResponseGenerator.java index 8a0f3e8cfc..8f4bee70e8 100644 --- a/spock-core/src/main/java/org/spockframework/mock/IResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/IResponseGenerator.java @@ -25,40 +25,40 @@ @Beta public interface IResponseGenerator { /** - * Do not call this method from Spock 2.4 on! Some implementations of this interface have the need to overwrite - * {@link #getResponseSupplier(IMockInvocation)} instead and throw an exception if this method is called directly. - * Always use {@code getResponseSupplier} instead to retrieve a response from this response generator. + * Returns the response to be made for the given invocation. It is preferable to call + * {@link #getResponseSupplier(IMockInvocation)} instead, as that method can separate immediate actions that might + * need to be done under some lock from actions that can be done later in the returned supplier. * - *

If you implement this interface and can directly provide the generated response, you can overwrite this - * method and use the default implementation of {@code getResponseSupplier} which uses this method. If you are in - * need to manage some state, like advancing an iterator or similar, or the actual supplying can deadlock, - * or you might throw an exception, you might want to implement {@code getResponseSupplier} instead and keep - * the default implementation of this method which throws an {@code UnsupportedOperationException}. + *

The default implementation of this method uses {@code getResponseSupplier}, the default implementation of which + * uses this method. Implementations of this interface should override exactly one of these two methods. If they + * override both, the result could be different depending on which method is called, if they override none, + * any call will result in a {@code StackOverflowError}. Calls to the two methods must always behave consistent, + * and the easiest way to achieve that is to override only one of them. * - *

This method is mainly kept for backwards compatibility with code that implements this interface. - * Most usages where this method is called will most likely fail to execute now. + *

If you implement this interface and do not need to update shared state immediately like when implementing + * {@link IChainableResponseGenerator}, you can just override this method. If you need the separation of immediate + * actions done under some lock and actions that can be done delayed, better override {@code getResponseSupplier}. * * @param invocation The invocation to generate a response for * @return The generated response - * @throws UnsupportedOperationException if {@code getResponseSupplier} must be used instead - * @deprecated Use {@link #getResponseSupplier(IMockInvocation)} instead */ @Nullable - @Deprecated default Object respond(IMockInvocation invocation) { - throw new UnsupportedOperationException("Call getResponseSupplier(invocation) instead"); + return getResponseSupplier(invocation).get(); } /** - * This method should be used from Spock 2.4 on, to get a response generated by this response generator. - * Calling {@link #respond(IMockInvocation)} directly might throw an {@code UnsupportedOperationException}, if - * this response generator needs to do some state maintenance or other things in this method's implementation. + * Returns a supplier with the response to be made for the given invocation. * - *

If you implement this interface and can directly provide the generated response, you can overwrite - * {@code respond} and use the default implementation of this method which uses that method. If you are in - * need to manage some state, like advancing an iterator or similar, or the actual supplying can deadlock, - * or you might throw an exception, you might want to implement this method instead and keep the default - * implementation of {@code respond} which throws an {@code UnsupportedOperationException}. + *

The default implementation of this method uses {@link #respond(IMockInvocation)}, the default implementation + * of which uses this method. Implementations of this interface should override exactly one of these two methods. + * If they override both, the result could be different depending on which method is called, if they override none, + * any call will result in a {@code StackOverflowError}. Calls to the two methods must always behave consistent, + * and the easiest way to achieve that is to override only one of them. + * + *

If you implement this interface and do need to update shared state immediately like when implementing + * {@link IChainableResponseGenerator}, you should prefer overriding this method. If not, then you might consider + * just overriding {@code respond} instead. * * @param invocation The invocation to generate a response for * @return A supplier that provides the generated response diff --git a/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java b/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java index 1f6d621dc3..15464f00ef 100644 --- a/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java +++ b/spock-core/src/main/java/org/spockframework/mock/ZeroOrNullResponse.java @@ -16,8 +16,6 @@ import org.spockframework.util.*; -import java.util.function.Supplier; - /** * A response strategy that returns zero, false, or null, depending on the method's return type. */ @@ -29,13 +27,11 @@ public class ZeroOrNullResponse implements IDefaultResponse { private ZeroOrNullResponse() {} @Override - public Supplier getResponseSupplier(IMockInvocation invocation) { - return () -> { - IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); - if (interaction != null) return interaction.accept(invocation).get(); + public Object respond(IMockInvocation invocation) { + IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); + if (interaction != null) return interaction.accept(invocation).get(); - Class returnType = invocation.getMethod().getReturnType(); - return ReflectionUtil.getDefaultValue(returnType); - }; + Class returnType = invocation.getMethod().getReturnType(); + return ReflectionUtil.getDefaultValue(returnType); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java index 7391018b9d..75d1e69696 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/CodeResponseGenerator.java @@ -22,8 +22,6 @@ import groovy.lang.Closure; import org.spockframework.util.ThreadSafe; -import java.util.function.Supplier; - import static org.spockframework.util.ObjectUtil.uncheckedCast; /** @@ -39,16 +37,14 @@ public CodeResponseGenerator(Closure code) { } @Override - public Supplier doGetResponseSupplier(IMockInvocation invocation) { - return () -> { - Object result = invokeClosure(invocation); - Class returnType = invocation.getMethod().getReturnType(); + public Object doRespond(IMockInvocation invocation) { + Object result = invokeClosure(invocation); + Class returnType = invocation.getMethod().getReturnType(); - // don't attempt cast for void methods (closure could be an action that accidentally returns a value) - if (returnType == void.class || returnType == Void.class) return null; + // don't attempt cast for void methods (closure could be an action that accidentally returns a value) + if (returnType == void.class || returnType == Void.class) return null; - return GroovyRuntimeUtil.coerce(result, invocation.getMethod().getReturnType()); - }; + return GroovyRuntimeUtil.coerce(result, invocation.getMethod().getReturnType()); } private Object invokeClosure(IMockInvocation invocation) { diff --git a/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java index fe36917b63..8328a269c9 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/ConstantResponseGenerator.java @@ -20,8 +20,6 @@ import org.spockframework.runtime.GroovyRuntimeUtil; import org.spockframework.util.ThreadSafe; -import java.util.function.Supplier; - /** * * @author Peter Niederwieser @@ -35,7 +33,7 @@ public ConstantResponseGenerator(Object constant) { } @Override - public Supplier doGetResponseSupplier(IMockInvocation invocation) { - return () -> GroovyRuntimeUtil.coerce(constant, invocation.getMethod().getReturnType()); + public Object doRespond(IMockInvocation invocation) { + return GroovyRuntimeUtil.coerce(constant, invocation.getMethod().getReturnType()); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java index f25e30a275..ba7a4a5782 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/SingleResponseGenerator.java @@ -26,55 +26,21 @@ public abstract class SingleResponseGenerator implements IChainableResponseGener private volatile boolean endOfCycle = false; @Override - public boolean isAtEndOfCycle() { + public final boolean isAtEndOfCycle() { return endOfCycle; } @Override public final Supplier getResponseSupplier(IMockInvocation invocation) { endOfCycle = true; - return doGetResponseSupplier(invocation); + return () -> doRespond(invocation); } /** - * Do not call this method from Spock 2.4 on! Some subclasses of this class have the need to overwrite - * {@link #doGetResponseSupplier(IMockInvocation)} instead and throw an exception if this method is called directly. - * Always use {@code doGetResponseSupplier} instead to retrieve a response from this single response generator. - * - *

If you subclass this class and can directly provide the generated response, you can overwrite this - * method and use the default implementation of {@code doGetResponseSupplier} which uses this method. If you are in - * need to manage some state, like advancing an iterator or similar, or the actual supplying can deadlock, - * or you might throw an exception, you might want to overwrite {@code doGetResponseSupplier} instead and keep - * the default implementation of this method which throws an {@code UnsupportedOperationException}. - * - *

This method is mainly kept for backwards compatibility with code that subclasses this class. - * Most usages where this method is called will most likely fail to execute now. + * Returns the response to be made for the given invocation. * * @param invocation The invocation to generate a response for * @return The generated response - * @throws UnsupportedOperationException if {@code doGetResponseSupplier} must be used instead - * @deprecated Use {@link #doGetResponseSupplier(IMockInvocation)} instead - */ - @Deprecated - public Object doRespond(IMockInvocation invocation) { - throw new UnsupportedOperationException("Call doGetResponseSupplier(invocation) instead"); - } - - /** - * This method should be used from Spock 2.4 on, to get a response generated by this single response generator. - * Calling {@link #doRespond(IMockInvocation)} directly might throw an {@code UnsupportedOperationException}, if - * this single response generator needs to do some state maintenance or other things in this method's implementation. - * - *

If you subclass this class and can directly provide the generated response, you can overwrite - * {@code doRespond} and use the default implementation of this method which uses that method. If you are in - * need to manage some state, like advancing an iterator or similar, or the actual supplying can deadlock, - * or you might throw an exception, you might want to override this method instead and keep the default - * implementation of {@code doRespond} which throws an {@code UnsupportedOperationException}. - * - * @param invocation The invocation to generate a response for - * @return A supplier that provides the generated response */ - public Supplier doGetResponseSupplier(IMockInvocation invocation) { - return () -> doRespond(invocation); - } + protected abstract Object doRespond(IMockInvocation invocation); } diff --git a/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java b/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java index 86d9ef7534..fb3f4c7e71 100644 --- a/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java +++ b/spock-core/src/main/java/org/spockframework/mock/response/WildcardResponseGenerator.java @@ -19,8 +19,6 @@ import org.spockframework.mock.*; import org.spockframework.util.ThreadSafe; -import java.util.function.Supplier; - /** * Returns a value using {@code Stub} semantics (see {@link EmptyOrDummyResponse}). * @@ -33,7 +31,7 @@ @ThreadSafe public class WildcardResponseGenerator extends SingleResponseGenerator { @Override - public Supplier doGetResponseSupplier(IMockInvocation invocation) { - return EmptyOrDummyResponse.INSTANCE.getResponseSupplier(invocation); + public Object doRespond(IMockInvocation invocation) { + return EmptyOrDummyResponse.INSTANCE.respond(invocation); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java index dae84eca3f..5a93466c11 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMethodInvoker.java @@ -4,8 +4,6 @@ import org.spockframework.util.ExceptionUtil; import org.spockframework.util.ThreadSafe; -import java.util.function.Supplier; - @ThreadSafe public class ByteBuddyMethodInvoker implements IResponseGenerator { @@ -16,17 +14,15 @@ public ByteBuddyMethodInvoker(ByteBuddyInvoker superCall) { } @Override - public Supplier getResponseSupplier(IMockInvocation invocation) { - return () -> { - if (superCall == null) { - throw new IllegalStateException("Cannot invoke abstract method " + invocation.getMethod()); - } - try { - return superCall.call(invocation.getArguments().toArray()); - } catch (Throwable t) { - // Byte Buddy doesn't wrap exceptions in InvocationTargetException, so no need to unwrap - return ExceptionUtil.sneakyThrow(t); - } - }; + public Object respond(IMockInvocation invocation) { + if (superCall == null) { + throw new IllegalStateException("Cannot invoke abstract method " + invocation.getMethod()); + } + try { + return superCall.call(invocation.getArguments().toArray()); + } catch (Throwable t) { + // Byte Buddy doesn't wrap exceptions in InvocationTargetException, so no need to unwrap + return ExceptionUtil.sneakyThrow(t); + } } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java index 4a97f0d979..b12e2c091d 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/CglibRealMethodInvoker.java @@ -20,8 +20,6 @@ import net.sf.cglib.proxy.MethodProxy; import org.spockframework.util.ThreadSafe; -import java.util.function.Supplier; - @ThreadSafe public class CglibRealMethodInvoker implements IResponseGenerator { private final MethodProxy methodProxy; @@ -31,14 +29,12 @@ public CglibRealMethodInvoker(MethodProxy methodProxy) { } @Override - public Supplier getResponseSupplier(IMockInvocation invocation) { - return () -> { - try { - return methodProxy.invokeSuper(invocation.getMockObject().getInstance(), invocation.getArguments().toArray()); - } catch (Throwable t) { - // MethodProxy doesn't wrap exceptions in InvocationTargetException, so no need to unwrap - return ExceptionUtil.sneakyThrow(t); - } - }; + public Object respond(IMockInvocation invocation) { + try { + return methodProxy.invokeSuper(invocation.getMockObject().getInstance(), invocation.getArguments().toArray()); + } catch (Throwable t) { + // MethodProxy doesn't wrap exceptions in InvocationTargetException, so no need to unwrap + return ExceptionUtil.sneakyThrow(t); + } } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java index 5fb2b0094b..f2747a29ef 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/DefaultMethodInvoker.java @@ -27,7 +27,6 @@ import java.lang.reflect.Field; import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; -import java.util.function.Supplier; @ThreadSafe public class DefaultMethodInvoker implements IResponseGenerator { @@ -44,14 +43,12 @@ public DefaultMethodInvoker(Object target, Method method, Object[] arguments) { } @Override - public Supplier getResponseSupplier(IMockInvocation invocation) { - return () -> { - if (INVOKE_DEFAULT == null) { - return useInternalMethodHandle(); - } - Object[] args = new Object[]{target, method, arguments}; - return ReflectionUtil.invokeMethod(null, INVOKE_DEFAULT, args); - }; + public Object respond(IMockInvocation invocation) { + if (INVOKE_DEFAULT == null) { + return useInternalMethodHandle(); + } + Object[] args = new Object[]{target, method, arguments}; + return ReflectionUtil.invokeMethod(null, INVOKE_DEFAULT, args); } private Object useInternalMethodHandle() { diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java index 8aff365caa..9cc0d05dc9 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/FailingRealMethodInvoker.java @@ -17,8 +17,6 @@ import org.spockframework.mock.*; import org.spockframework.util.ThreadSafe; -import java.util.function.Supplier; - @ThreadSafe public class FailingRealMethodInvoker implements IResponseGenerator { private final String message; @@ -28,9 +26,7 @@ public FailingRealMethodInvoker(String message) { } @Override - public Supplier getResponseSupplier(IMockInvocation invocation) { - return () -> { - throw new CannotInvokeRealMethodException(message); - }; + public Object respond(IMockInvocation invocation) { + throw new CannotInvokeRealMethodException(message); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java index 7458ae76f9..6ea43702a8 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyRealMethodInvoker.java @@ -19,8 +19,6 @@ import groovy.lang.MetaClass; import org.spockframework.util.ThreadSafe; -import java.util.function.Supplier; - @ThreadSafe public class GroovyRealMethodInvoker implements IResponseGenerator { private final MetaClass metaClass; @@ -30,17 +28,15 @@ public GroovyRealMethodInvoker(MetaClass metaClass) { } @Override - public Supplier getResponseSupplier(IMockInvocation invocation) { - return () -> { - Object instance = invocation.getMockObject().getInstance(); - Object[] arguments = invocation.getArguments().toArray(); - if (invocation.getMethod().isStatic()) { - if ("".equals(invocation.getMethod().getName())) { - return metaClass.invokeConstructor(arguments); - } - return metaClass.invokeStaticMethod(instance, invocation.getMethod().getName(), arguments); + public Object respond(IMockInvocation invocation) { + Object instance = invocation.getMockObject().getInstance(); + Object[] arguments = invocation.getArguments().toArray(); + if (invocation.getMethod().isStatic()) { + if ("".equals(invocation.getMethod().getName())) { + return metaClass.invokeConstructor(arguments); } - return metaClass.invokeMethod(instance, invocation.getMethod().getName(), arguments); - }; + return metaClass.invokeStaticMethod(instance, invocation.getMethod().getName(), arguments); + } + return metaClass.invokeMethod(instance, invocation.getMethod().getName(), arguments); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java index 5d062d5f60..c4658e2d73 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ObjectMethodInvoker.java @@ -3,8 +3,6 @@ import org.spockframework.mock.*; import org.spockframework.util.ThreadSafe; -import java.util.function.Supplier; - @ThreadSafe public class ObjectMethodInvoker implements IResponseGenerator { @@ -14,14 +12,12 @@ private ObjectMethodInvoker() { } @Override - public Supplier getResponseSupplier(IMockInvocation invocation) { - return () -> { - IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); - if (interaction != null) { - return interaction.accept(invocation).get(); - } + public Object respond(IMockInvocation invocation) { + IMockInteraction interaction = DefaultJavaLangObjectInteractions.INSTANCE.match(invocation); + if (interaction != null) { + return interaction.accept(invocation).get(); + } - return null; - }; + return null; } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java b/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java index 2f75df0c32..dea6f9b283 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java @@ -32,7 +32,6 @@ import org.spockframework.mock.*; import org.spockframework.mock.runtime.IMockMaker; import org.spockframework.mock.runtime.IProxyBasedMockInterceptor; -import org.spockframework.runtime.GroovyRuntimeUtil; import org.spockframework.util.ExceptionUtil; import org.spockframework.util.ReflectionUtil; import org.spockframework.util.ThreadSafe; @@ -264,14 +263,12 @@ public Object handle(Invocation invocation) { Object mock = invocation.getMock(); return mockInterceptor.intercept(mock, invocation.getMethod(), invocation.getArguments(), new IResponseGenerator() { @Override - public Supplier getResponseSupplier(IMockInvocation __) { - return () -> { - try { - return invocation.callRealMethod(); - } catch (Throwable e) { - return ExceptionUtil.sneakyThrow(e); - } - }; + public Object respond(IMockInvocation __) { + try { + return invocation.callRealMethod(); + } catch (Throwable e) { + return ExceptionUtil.sneakyThrow(e); + } } }); } diff --git a/spock-specs/src/test/groovy/org/spockframework/docs/interaction/DetachedMockFactoryDocSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/docs/interaction/DetachedMockFactoryDocSpec.groovy index a322fdbd0b..c56645f147 100644 --- a/spock-specs/src/test/groovy/org/spockframework/docs/interaction/DetachedMockFactoryDocSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/docs/interaction/DetachedMockFactoryDocSpec.groovy @@ -11,7 +11,6 @@ import spock.mock.AutoAttach import spock.mock.DetachedMockFactory import java.util.concurrent.ThreadLocalRandom -import java.util.function.Supplier import static org.spockframework.docs.interaction.DetachedMockFactoryDocSpec.EngineMockCreator.StartMode.ALWAYS_STARTED import static org.spockframework.docs.interaction.DetachedMockFactoryDocSpec.EngineMockCreator.StartMode.ALWAYS_STOPPED @@ -190,14 +189,12 @@ class DetachedMockFactoryDocSpec extends Specification { StartMode startMode @Override - Supplier getResponseSupplier(IMockInvocation invocation) { - return { - if (invocation.method.name != 'isStarted') - return ZeroOrNullResponse.INSTANCE.getResponseSupplier(invocation).get() - startMode == RANDOMLY_STARTED - ? ThreadLocalRandom.current().nextBoolean() - : startMode == ALWAYS_STARTED - } + Object respond(IMockInvocation invocation) { + if (invocation.method.name != 'isStarted') + return ZeroOrNullResponse.INSTANCE.respond(invocation) + startMode == RANDOMLY_STARTED + ? ThreadLocalRandom.current().nextBoolean() + : startMode == ALWAYS_STARTED } } From 3e7babbddc519229601937f063365fa42f0e140e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20Br=C3=BCnings?= Date: Sun, 17 Mar 2024 17:36:40 +0100 Subject: [PATCH 6/6] Polish Breaking Changes section --- docs/release_notes.adoc | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc index c1a173b930..91b6d3841c 100644 --- a/docs/release_notes.adoc +++ b/docs/release_notes.adoc @@ -7,19 +7,18 @@ include::include.adoc[] === Breaking Changes -* Calculated responses for interactions (`>> { ... }`) previously were all executed synchronized on the respective mock - controller instance, so could safely mutate shared state to a certain degree, even if the invocations were happening - on different threads. This also caused that one response calculation could not wait on something happening - in another response calculation, as they were all executed sequentially due to the synchronization. +* _Most users will probably be unaffected by this change as it only becomes relevant in a multithreaded situation where multiple threads do interaction invocations that care about shared state._ + + Calculated responses for interactions (`>> { ... }`) previously were all executed synchronized on the respective mock controller instance, so could safely mutate shared state to a certain degree, even if the invocations were happening + on different threads. + This also caused that one response calculation could not wait on something happening in another response calculation, as they were all executed sequentially due to the synchronization. Starting with this release, the response calculations are no longer happening synchronized. - If you depend on shared state in such calculations, you now have to make sure yourself, that this is done in a - thread-safe manner. Most users will probably be unaffected by this change as it only becomes relevant in a - multi-threaded situation where multiple threads do interaction invocations that care about shared state. spockPull:1910[] - -* `SingleResponseGenerator#isAtEndOfCycle` is now `final` and `SingleResponseGenerator#doRespond` is now `protected`. - When subclassing `SingleResponseGenerator` it does not make sense to override the first method, and it does not make - sense to call the second method from somewhere else. This should not affect most users, only if you were - subclassing `SingleResponseGenerator` and doing unusual things. spockPull:1910[] + If you depend on shared state in such calculations, you now have to make sure yourself, that this is done in a thread-safe manner. + spockPull:1910[] + +* _This should not affect most users, only if you were subclassing `SingleResponseGenerator` and doing unusual things._ + + `SingleResponseGenerator#isAtEndOfCycle` is now `final` and `SingleResponseGenerator#doRespond` is now `protected`. + When subclassing `SingleResponseGenerator` it does not make sense to override the first method, and it does not make sense to call the second method from somewhere else. + spockPull:1910[] === Misc