Properly handle follow-up request after failure

Issue: SPR-16132

Author: jhoeller

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

@@ -72,24 +72,28 @@ public ClientHttpResponse validateRequest(ClientHttpRequest request) throws IOEx
 			if (requests.isEmpty()) {
 				afterExpectationsDeclared();
 			}
-			ClientHttpResponse response = validateRequestInternal(request);
-			requests.add(request);
-			return response;
+			try {
+				return validateRequestInternal(request);
+			}
+			finally {
+				requests.add(request);
+			}
 		}
 	}
 
 	/**
-	 * Invoked after the phase of declaring expected requests is over. This is
-	 * detected from {@link #validateRequest} on the first actual request.
+	 * Invoked at the time of the first actual request, which effectively means
+	 * the expectations declaration phase is over.
 	 */
 	protected void afterExpectationsDeclared() {
 	}
 
 	/**
 	 * Subclasses must implement the actual validation of the request
-	 * matching it to a declared expectation.
+	 * matching to declared expectations.
 	 */
-	protected abstract ClientHttpResponse validateRequestInternal(ClientHttpRequest request) throws IOException;
+	protected abstract ClientHttpResponse validateRequestInternal(ClientHttpRequest request)
+			throws IOException;
 
 	@Override
 	public void verify() {
@@ -145,9 +149,7 @@ public void reset() {
 
 
 	/**
-	 * Helper class to manage a group of request expectations. It helps with
-	 * operations against the entire group such as finding a match and updating
-	 * (add or remove) based on expected request count.
+	 * Helper class to manage a group of remaining expectations.
 	 */
 	protected static class RequestExpectationGroup {
 
@@ -157,6 +159,24 @@ public Set<RequestExpectation> getExpectations() {
 			return this.expectations;
 		}
 
+		public RequestExpectation findExpectation(ClientHttpRequest request) throws IOException {
+			for (RequestExpectation expectation : getExpectations()) {
+				try {
+					expectation.match(request);
+					return expectation;
+				}
+				catch (AssertionError error) {
+					// Ignore
+				}
+			}
+			return null;
+		}
+
+		/**
+		 * 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.
+		 */
 		public void update(RequestExpectation expectation) {
 			if (expectation.hasRemainingCount()) {
 				getExpectations().add(expectation);
@@ -166,25 +186,16 @@ public void update(RequestExpectation expectation) {
 			}
 		}
 
+		/**
+		 * Collection variant of {@link #update(RequestExpectation)} that can
+		 * be used to insert expectations.
+		 */
 		public void updateAll(Collection<RequestExpectation> expectations) {
 			for (RequestExpectation expectation : expectations) {
 				update(expectation);
 			}
 		}
 
-		public RequestExpectation findExpectation(ClientHttpRequest request) throws IOException {
-			for (RequestExpectation expectation : getExpectations()) {
-				try {
-					expectation.match(request);
-					return expectation;
-				}
-				catch (AssertionError error) {
-					// Ignore
-				}
-			}
-			return null;
-		}
-
 		public void reset() {
 			getExpectations().clear();
 		}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -13,27 +13,32 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.test.web.client;
 
+import java.net.SocketException;
+
 import org.junit.Test;
 
 import org.springframework.test.web.client.MockRestServiceServer.MockRestServiceServerBuilder;
 import org.springframework.web.client.RestTemplate;
 
-import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo;
-import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess;
+import static org.springframework.http.HttpMethod.*;
+import static org.springframework.test.web.client.match.MockRestRequestMatchers.*;
+import static org.springframework.test.web.client.response.MockRestResponseCreators.*;
 
 /**
  * Unit tests for {@link MockRestServiceServer}.
+ *
  * @author Rossen Stoyanchev
  */
 public class MockRestServiceServerTests {
 
-	private RestTemplate restTemplate = new RestTemplate();
+	private final RestTemplate restTemplate = new RestTemplate();
 
 
 	@Test
-	public void buildMultipleTimes() throws Exception {
+	public void buildMultipleTimes() {
 		MockRestServiceServerBuilder builder = MockRestServiceServer.bindTo(this.restTemplate);
 
 		MockRestServiceServer server = builder.build();
@@ -55,7 +60,7 @@ public void buildMultipleTimes() throws Exception {
 	}
 
 	@Test(expected = AssertionError.class)
-	public void exactExpectOrder() throws Exception {
+	public void exactExpectOrder() {
 		MockRestServiceServer server = MockRestServiceServer.bindTo(this.restTemplate)
 				.ignoreExpectOrder(false).build();
 
@@ -65,7 +70,7 @@ public void exactExpectOrder() throws Exception {
 	}
 
 	@Test
-	public void ignoreExpectOrder() throws Exception {
+	public void ignoreExpectOrder() {
 		MockRestServiceServer server = MockRestServiceServer.bindTo(this.restTemplate)
 				.ignoreExpectOrder(true).build();
 
@@ -77,7 +82,7 @@ public void ignoreExpectOrder() throws Exception {
 	}
 
 	@Test
-	public void resetAndReuseServer() throws Exception {
+	public void resetAndReuseServer() {
 		MockRestServiceServer server = MockRestServiceServer.bindTo(this.restTemplate).build();
 
 		server.expect(requestTo("/foo")).andRespond(withSuccess());
@@ -91,7 +96,7 @@ public void resetAndReuseServer() throws Exception {
 	}
 
 	@Test
-	public void resetAndReuseServerWithUnorderedExpectationManager() throws Exception {
+	public void resetAndReuseServerWithUnorderedExpectationManager() {
 		MockRestServiceServer server = MockRestServiceServer.bindTo(this.restTemplate)
 				.ignoreExpectOrder(true).build();
 
@@ -107,4 +112,24 @@ public void resetAndReuseServerWithUnorderedExpectationManager() throws Exceptio
 		server.verify();
 	}
 
+	@Test  // SPR-16132
+	public void followUpRequestAfterFailure() {
+		MockRestServiceServer server = MockRestServiceServer.bindTo(this.restTemplate).build();
+
+		server.expect(requestTo("/some-service/some-endpoint"))
+				.andRespond(request -> { throw new SocketException("pseudo network error"); });
+
+		server.expect(requestTo("/reporting-service/report-error"))
+				.andExpect(method(POST)).andRespond(withSuccess());
+
+		try {
+			this.restTemplate.getForEntity("/some-service/some-endpoint", String.class);
+		}
+		catch (Exception ex) {
+			this.restTemplate.postForEntity("/reporting-service/report-error", ex.toString(), String.class);
+		}
+
+		server.verify();
+	}
+
 }

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

@@ -16,6 +16,7 @@
 
 package org.springframework.test.web.client;
 
+import java.net.SocketException;
 import java.net.URI;
 import java.net.URISyntaxException;
 
@@ -168,8 +169,36 @@ public void sequentialRequestsWithDifferentCount() throws Exception {
 		this.manager.validateRequest(createRequest(GET, "/bar"));
 	}
 
+	@Test  // SPR-15719
+	public void repeatedRequestsInSequentialOrder() throws Exception {
+		this.manager.expectRequest(times(2), requestTo("/foo")).andExpect(method(GET)).andRespond(withSuccess());
+		this.manager.expectRequest(times(2), requestTo("/bar")).andExpect(method(GET)).andRespond(withSuccess());
+
+		this.manager.validateRequest(createRequest(GET, "/foo"));
+		this.manager.validateRequest(createRequest(GET, "/foo"));
+		this.manager.validateRequest(createRequest(GET, "/bar"));
+		this.manager.validateRequest(createRequest(GET, "/bar"));
+	}
+
+	@Test  // SPR-16132
+	public void sequentialRequestsWithFirstFailing() throws Exception {
+		this.manager.expectRequest(once(), requestTo("/foo")).
+				andExpect(method(GET)).andRespond(request -> { throw new SocketException("pseudo network error"); });
+		this.manager.expectRequest(once(), requestTo("/handle-error")).
+				andExpect(method(POST)).andRespond(withSuccess());
+
+		try {
+			this.manager.validateRequest(createRequest(GET, "/foo"));
+			fail("Expected SocketException");
+		}
+		catch (SocketException ex) {
+			// expected
+		}
+		this.manager.validateRequest(createRequest(POST, "/handle-error"));
+		this.manager.verify();
+	}
+
 
-	@SuppressWarnings("deprecation")
 	private ClientHttpRequest createRequest(HttpMethod method, String url) {
 		try {
 			return new MockClientHttpRequest(method, new URI(url));

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.test.web.client;
 
 import java.net.URI;
@@ -26,18 +27,15 @@
 import org.springframework.http.client.ClientHttpRequest;
 import org.springframework.mock.http.client.MockAsyncClientHttpRequest;
 
-import static org.junit.Assert.assertEquals;
-import static org.springframework.http.HttpMethod.GET;
-import static org.springframework.test.web.client.ExpectedCount.max;
-import static org.springframework.test.web.client.ExpectedCount.min;
-import static org.springframework.test.web.client.ExpectedCount.once;
-import static org.springframework.test.web.client.ExpectedCount.times;
-import static org.springframework.test.web.client.match.MockRestRequestMatchers.method;
-import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo;
-import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess;
+import static org.junit.Assert.*;
+import static org.springframework.http.HttpMethod.*;
+import static org.springframework.test.web.client.ExpectedCount.*;
+import static org.springframework.test.web.client.match.MockRestRequestMatchers.*;
+import static org.springframework.test.web.client.response.MockRestResponseCreators.*;
 
 /**
  * Unit tests for {@link UnorderedRequestExpectationManager}.
+ *
  * @author Rossen Stoyanchev
  */
 public class UnorderedRequestExpectationManagerTests {