Separate ResponseActions from ClientHttpRequest

Before this commit RequestMatcherClientHttpRequest served both as
API to define request expectations, i.e. ResponseActions, as well as
the implementation of ClientHttpRequest representing actual requests.

DefaultResponseActions replaces this class as a simple holder of
expected requests and mock responses. MockRestServiceServer is then
responsible to match request expectations and create a mock response.

Issue: SPR-11365

spring-test/src/main/java/org/springframework/test/web/client/DefaultResponseActions.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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,28 +20,26 @@ import java.io.IOException;
 import java.util.LinkedList;
 import java.util.List;
 
+import org.springframework.http.client.ClientHttpRequest;
 import org.springframework.http.client.ClientHttpResponse;
-import org.springframework.mock.http.client.MockAsyncClientHttpRequest;
 import org.springframework.util.Assert;
 
 /**
- * A specialization of {@code MockClientHttpRequest} that matches the request
- * against a set of expectations, via {@link RequestMatcher} instances. The
- * expectations are checked when the request is executed. This class also uses a
- * {@link ResponseCreator} to create the response.
+ * Default implementation of {@code ResponseActions} that is also a composite
+ * {@code RequestMatcher}, invoking all request matchers it contains, as well as
+ * a {@code ResponseCreator} delegating to the response creator it contains.
  *
- * @author Craig Walls
  * @author Rossen Stoyanchev
- * @since 3.2
+ * @since 4.3
  */
-class RequestMatcherClientHttpRequest extends MockAsyncClientHttpRequest implements ResponseActions {
+class DefaultResponseActions implements ResponseActions, RequestMatcher, ResponseCreator {
 
 	private final List<RequestMatcher> requestMatchers = new LinkedList<RequestMatcher>();
 
 	private ResponseCreator responseCreator;
 
 
-	public RequestMatcherClientHttpRequest(RequestMatcher requestMatcher) {
+	public DefaultResponseActions(RequestMatcher requestMatcher) {
 		Assert.notNull(requestMatcher, "RequestMatcher is required");
 		this.requestMatchers.add(requestMatcher);
 	}
@@ -61,21 +59,18 @@ class RequestMatcherClientHttpRequest extends MockAsyncClientHttpRequest impleme
 	}
 
 	@Override
-	public ClientHttpResponse executeInternal() throws IOException {
-		if (this.requestMatchers.isEmpty()) {
-			throw new AssertionError("No request expectations to execute");
+	public void match(ClientHttpRequest request) throws IOException {
+		for (RequestMatcher matcher : this.requestMatchers) {
+			matcher.match(request);
 		}
+	}
 
+	@Override
+	public ClientHttpResponse createResponse(ClientHttpRequest request) throws IOException {
 		if (this.responseCreator == null) {
-			throw new AssertionError("No ResponseCreator was set up. Add it after request expectations, " +
-					"e.g. MockRestServiceServer.expect(requestTo(\"/foo\")).andRespond(withSuccess())");
-		}
-
-		for (RequestMatcher requestMatcher : this.requestMatchers) {
-			requestMatcher.match(this);
+			throw new IllegalStateException("createResponse called before ResponseCreator was set.");
 		}
-		setResponse(this.responseCreator.createResponse(this));
-		return super.executeInternal();
+		return this.responseCreator.createResponse(request);
 	}
 
 }

spring-test/src/main/java/org/springframework/test/web/client/MockRestServiceServer.java

@@ -27,6 +27,8 @@ import org.springframework.http.client.AsyncClientHttpRequest;
 import org.springframework.http.client.AsyncClientHttpRequestFactory;
 import org.springframework.http.client.ClientHttpRequest;
 import org.springframework.http.client.ClientHttpRequestFactory;
+import org.springframework.http.client.ClientHttpResponse;
+import org.springframework.mock.http.client.MockAsyncClientHttpRequest;
 import org.springframework.test.web.client.match.MockRestRequestMatchers;
 import org.springframework.test.web.client.response.MockRestResponseCreators;
 import org.springframework.util.Assert;
@@ -96,11 +98,11 @@ import org.springframework.web.client.support.RestGatewaySupport;
  */
 public class MockRestServiceServer {
 
-	private final List<RequestMatcherClientHttpRequest> expectedRequests =
-			new LinkedList<RequestMatcherClientHttpRequest>();
+	private final List<DefaultResponseActions> responseActions =
+			new LinkedList<DefaultResponseActions>();
 
-	private final List<RequestMatcherClientHttpRequest> actualRequests =
-			new LinkedList<RequestMatcherClientHttpRequest>();
+	private final List<MockAsyncClientHttpRequest> requests =
+			new LinkedList<MockAsyncClientHttpRequest>();
 
 
 	/**
@@ -161,9 +163,9 @@ public class MockRestServiceServer {
 	 * @return used to set up further expectations or to define a response
 	 */
 	public ResponseActions expect(RequestMatcher requestMatcher) {
-		Assert.state(this.actualRequests.isEmpty(), "Can't add more expected requests with test already underway");
-		RequestMatcherClientHttpRequest request = new RequestMatcherClientHttpRequest(requestMatcher);
-		this.expectedRequests.add(request);
+		Assert.state(this.requests.isEmpty(), "Can't add more expected requests with test already underway");
+		DefaultResponseActions request = new DefaultResponseActions(requestMatcher);
+		this.responseActions.add(request);
 		return request;
 	}
 
@@ -173,7 +175,7 @@ public class MockRestServiceServer {
 	 * @throws AssertionError when some expectations were not met
 	 */
 	public void verify() {
-		if (this.expectedRequests.isEmpty() || this.expectedRequests.equals(this.actualRequests)) {
+		if (this.responseActions.isEmpty() || this.responseActions.size() == this.requests.size()) {
 			return;
 		}
 		throw new AssertionError(getVerifyMessage());
@@ -181,15 +183,15 @@ public class MockRestServiceServer {
 
 	private String getVerifyMessage() {
 		StringBuilder sb = new StringBuilder("Further request(s) expected\n");
-		if (this.actualRequests.size() > 0) {
+		if (this.requests.size() > 0) {
 			sb.append("The following ");
 		}
-		sb.append(this.actualRequests.size()).append(" out of ");
-		sb.append(this.expectedRequests.size()).append(" were executed");
+		sb.append(this.requests.size()).append(" out of ");
+		sb.append(this.responseActions.size()).append(" were executed");
 
-		if (this.actualRequests.size() > 0) {
+		if (this.requests.size() > 0) {
 			sb.append(":\n");
-			for (RequestMatcherClientHttpRequest request : this.actualRequests) {
+			for (MockAsyncClientHttpRequest request : this.requests) {
 				sb.append(request.toString()).append("\n");
 			}
 		}
@@ -199,12 +201,12 @@ public class MockRestServiceServer {
 
 	/**
 	 * Mock ClientHttpRequestFactory that creates requests by iterating
-	 * over the list of expected {@link RequestMatcherClientHttpRequest}'s.
+	 * over the list of expected {@link DefaultResponseActions}'s.
 	 */
 	private class RequestMatcherClientHttpRequestFactory
 			implements ClientHttpRequestFactory, AsyncClientHttpRequestFactory {
 
-		private Iterator<RequestMatcherClientHttpRequest> requestIterator;
+		private Iterator<DefaultResponseActions> requestIterator;
 
 		@Override
 		public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException {
@@ -216,23 +218,38 @@ public class MockRestServiceServer {
 			return createRequestInternal(uri, httpMethod);
 		}
 
-		private RequestMatcherClientHttpRequest createRequestInternal(URI uri, HttpMethod httpMethod) {
+		private MockAsyncClientHttpRequest createRequestInternal(URI uri, HttpMethod httpMethod) {
 			Assert.notNull(uri, "'uri' must not be null");
 			Assert.notNull(httpMethod, "'httpMethod' must not be null");
 
+			MockAsyncClientHttpRequest request = new MockAsyncClientHttpRequest(httpMethod, uri) {
+				@Override
+				protected ClientHttpResponse executeInternal() throws IOException {
+					ClientHttpResponse response = validateRequest(this);
+					setResponse(response);
+					return response;
+				}
+			};
+
+			MockRestServiceServer.this.requests.add(request);
+			return request;
+		}
+
+		private ClientHttpResponse validateRequest(MockAsyncClientHttpRequest request)
+				throws IOException {
+
 			if (this.requestIterator == null) {
-				this.requestIterator = MockRestServiceServer.this.expectedRequests.iterator();
+				this.requestIterator = MockRestServiceServer.this.responseActions.iterator();
 			}
 			if (!this.requestIterator.hasNext()) {
-				throw new AssertionError("No further requests expected: HTTP " + httpMethod + " " + uri);
+				throw new AssertionError("No further requests expected: HTTP " +
+						request.getMethod() + " " + request.getURI());
 			}
 
-			RequestMatcherClientHttpRequest request = this.requestIterator.next();
-			request.setURI(uri);
-			request.setMethod(httpMethod);
+			DefaultResponseActions responseActions = this.requestIterator.next();
+			responseActions.match(request);
 
-			MockRestServiceServer.this.actualRequests.add(request);
-			return request;
+			return responseActions.createResponse(request);
 		}
 	}
 

spring-test/src/test/java/org/springframework/test/web/client/MockClientHttpRequestFactoryTests.java

@@ -52,10 +52,8 @@ public class MockClientHttpRequestFactoryTests {
 	@Test
 	public void createRequest() throws Exception {
 		URI uri = new URI("/foo");
-		ClientHttpRequest expected = (ClientHttpRequest) this.server.expect(anything());
 		ClientHttpRequest actual = this.factory.createRequest(uri, HttpMethod.GET);
 
-		assertSame(expected, actual);
 		assertEquals(uri, actual.getURI());
 		assertEquals(HttpMethod.GET, actual.getMethod());
 	}