Separate from expectations from response creation

This commit separates the creation of a response so that it is executed
after the synchronized block inside which requests need to be matched
and validated (for order and count).

This allows a ResponseCreator to be slow or block if it has to.

Issue: SPR-16319
This commit is contained in:
Rossen Stoyanchev 2018-01-17 12:55:52 -05:00
parent 7ab4d0ca08
commit 0c289283ff
6 changed files with 88 additions and 30 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2017 the original author or authors.
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -73,19 +73,29 @@ public abstract class AbstractRequestExpectationManager implements RequestExpect
return expectation;
}
@SuppressWarnings("deprecation")
@Override
public ClientHttpResponse validateRequest(ClientHttpRequest request) throws IOException {
RequestExpectation expectation = null;
synchronized (this.requests) {
if (this.requests.isEmpty()) {
afterExpectationsDeclared();
}
try {
return validateRequestInternal(request);
// Try this first for backwards compatibility
ClientHttpResponse response = validateRequestInternal(request);
if (response != null) {
return response;
}
else {
expectation = matchRequest(request);
}
}
finally {
this.requests.add(request);
}
}
return expectation.createResponse(request);
}
/**
@ -98,9 +108,34 @@ public abstract class AbstractRequestExpectationManager implements RequestExpect
/**
* Subclasses must implement the actual validation of the request
* matching to declared expectations.
* @deprecated as of 5.0.3 sub-classes should implement
* {@link #matchRequest(ClientHttpRequest)} instead and return only the matched
* expectation, leaving the call to create the response as a separate step
* (to be invoked by this class).
*/
protected abstract ClientHttpResponse validateRequestInternal(ClientHttpRequest request)
throws IOException;
@Deprecated
@Nullable
protected ClientHttpResponse validateRequestInternal(ClientHttpRequest request)
throws IOException {
return null;
}
/**
* As of 5.0.3 subclasses should implement this method instead of
* {@link #validateRequestInternal(ClientHttpRequest)} in order to match the
* request to an expectation, leaving the call to create the response as a separate step
* (to be invoked by this class).
* @param request the current request
* @return the matched expectation with its request count updated via
* {@link RequestExpectation#incrementAndValidate()}.
* @since 5.0.3
*/
protected RequestExpectation matchRequest(ClientHttpRequest request) throws IOException {
throw new java.lang.UnsupportedOperationException(
"It looks like neither the deprecated \"validateRequestInternal\"" +
"nor its replacement (this method) are implemented.");
}
@Override
public void verify() {
@ -162,10 +197,15 @@ public abstract class AbstractRequestExpectationManager implements RequestExpect
private final Set<RequestExpectation> expectations = new LinkedHashSet<>();
public Set<RequestExpectation> getExpectations() {
return this.expectations;
}
public void addAllExpectations(Collection<RequestExpectation> expectations) {
this.expectations.addAll(expectations);
}
/**
* Return a matching expectation, or {@code null} if none match.
@ -186,10 +226,15 @@ public abstract class AbstractRequestExpectationManager implements RequestExpect
/**
* Invoke this for an expectation that has been matched.
* <p>The given expectation will either be stored if it has a remaining
* count or it will be removed otherwise.
* <p>The count of the given expectation is incremented, then it is
* either stored if remainingCount > 0 or removed otherwise.
*/
public void update(RequestExpectation expectation) {
expectation.incrementAndValidate();
updateInternal(expectation);
}
private void updateInternal(RequestExpectation expectation) {
if (expectation.hasRemainingCount()) {
this.expectations.add(expectation);
}
@ -199,11 +244,12 @@ public abstract class AbstractRequestExpectationManager implements RequestExpect
}
/**
* Collection variant of {@link #update(RequestExpectation)} that can
* be used to insert expectations.
* Add expectations to this group.
* @deprecated as of 5.0.3 please use {@link #addAllExpectations(Collection)} instead.
*/
@Deprecated
public void updateAll(Collection<RequestExpectation> expectations) {
expectations.forEach(this::update);
expectations.forEach(this::updateInternal);
}
public void reset() {

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2017 the original author or authors.
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -88,11 +88,16 @@ public class DefaultRequestExpectation implements RequestExpectation {
}
}
/**
* Note that as of 5.0.3, the creation of the response, which may block
* intentionally, is separated from request count tracking, and this
* method no longer increments the count transparently. Instead
* {@link #incrementAndValidate()} must be invoked independently.
*/
@Override
public ClientHttpResponse createResponse(@Nullable ClientHttpRequest request) throws IOException {
ResponseCreator responseCreator = getResponseCreator();
Assert.state(responseCreator != null, "createResponse() called before ResponseCreator was set");
getRequestCount().incrementAndValidate();
return responseCreator.createResponse(request);
}
@ -101,6 +106,11 @@ public class DefaultRequestExpectation implements RequestExpectation {
return getRequestCount().hasRemainingCount();
}
@Override
public void incrementAndValidate() {
getRequestCount().incrementAndValidate();
}
@Override
public boolean isSatisfied() {
return getRequestCount().isSatisfied();

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -34,6 +34,12 @@ public interface RequestExpectation extends ResponseActions, RequestMatcher, Res
*/
boolean hasRemainingCount();
/**
* Increase the matched request count and check we haven't passed the max count.
* @since 5.0.3
*/
void incrementAndValidate();
/**
* Whether the requirements for this request expectation have been met.
*/

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2017 the original author or authors.
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -20,7 +20,6 @@ import java.io.IOException;
import java.util.Iterator;
import org.springframework.http.client.ClientHttpRequest;
import org.springframework.http.client.ClientHttpResponse;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
@ -53,7 +52,7 @@ public class SimpleRequestExpectationManager extends AbstractRequestExpectationM
}
@Override
public ClientHttpResponse validateRequestInternal(ClientHttpRequest request) throws IOException {
protected RequestExpectation matchRequest(ClientHttpRequest request) throws IOException {
RequestExpectation expectation = this.repeatExpectations.findExpectation(request);
if (expectation == null) {
if (this.expectationIterator == null || !this.expectationIterator.hasNext()) {
@ -62,9 +61,8 @@ public class SimpleRequestExpectationManager extends AbstractRequestExpectationM
expectation = this.expectationIterator.next();
expectation.match(request);
}
ClientHttpResponse response = expectation.createResponse(request);
this.repeatExpectations.update(expectation);
return response;
return expectation;
}
@Override

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -19,7 +19,6 @@ package org.springframework.test.web.client;
import java.io.IOException;
import org.springframework.http.client.ClientHttpRequest;
import org.springframework.http.client.ClientHttpResponse;
/**
* {@code RequestExpectationManager} that matches requests to expectations
@ -35,18 +34,17 @@ public class UnorderedRequestExpectationManager extends AbstractRequestExpectati
@Override
protected void afterExpectationsDeclared() {
this.remainingExpectations.updateAll(getExpectations());
this.remainingExpectations.addAllExpectations(getExpectations());
}
@Override
public ClientHttpResponse validateRequestInternal(ClientHttpRequest request) throws IOException {
public RequestExpectation matchRequest(ClientHttpRequest request) throws IOException {
RequestExpectation expectation = this.remainingExpectations.findExpectation(request);
if (expectation == null) {
throw createUnexpectedRequestError(request);
}
ClientHttpResponse response = expectation.createResponse(request);
this.remainingExpectations.update(expectation);
return response;
return expectation;
}
@Override

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -62,26 +62,26 @@ public class DefaultRequestExpectationTests {
}
@Test
public void hasRemainingCount() throws Exception {
public void hasRemainingCount() {
RequestExpectation expectation = new DefaultRequestExpectation(twice(), requestTo("/foo"));
expectation.andRespond(withSuccess());
expectation.createResponse(createRequest(GET, "/foo"));
expectation.incrementAndValidate();
assertTrue(expectation.hasRemainingCount());
expectation.createResponse(createRequest(GET, "/foo"));
expectation.incrementAndValidate();
assertFalse(expectation.hasRemainingCount());
}
@Test
public void isSatisfied() throws Exception {
public void isSatisfied() {
RequestExpectation expectation = new DefaultRequestExpectation(twice(), requestTo("/foo"));
expectation.andRespond(withSuccess());
expectation.createResponse(createRequest(GET, "/foo"));
expectation.incrementAndValidate();
assertFalse(expectation.isSatisfied());
expectation.createResponse(createRequest(GET, "/foo"));
expectation.incrementAndValidate();
assertTrue(expectation.isSatisfied());
}