From a88c47a120ea7e1e937921b6ca5655b16db6b7ed Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 14 Nov 2017 12:30:04 +0100 Subject: [PATCH] Properly handle follow-up request after failure Issue: SPR-16132 --- .../AbstractRequestExpectationManager.java | 61 +++++++++++-------- .../client/MockRestServiceServerTests.java | 43 ++++++++++--- .../SimpleRequestExpectationManagerTests.java | 31 +++++++++- ...orderedRequestExpectationManagerTests.java | 18 +++--- 4 files changed, 108 insertions(+), 45 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/web/client/AbstractRequestExpectationManager.java b/spring-test/src/main/java/org/springframework/test/web/client/AbstractRequestExpectationManager.java index ea6bbcb314..790a1e2d6e 100644 --- a/spring-test/src/main/java/org/springframework/test/web/client/AbstractRequestExpectationManager.java +++ b/spring-test/src/main/java/org/springframework/test/web/client/AbstractRequestExpectationManager.java @@ -72,24 +72,28 @@ public abstract class AbstractRequestExpectationManager implements RequestExpect 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 abstract class AbstractRequestExpectationManager implements RequestExpect /** - * 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,21 +159,6 @@ public abstract class AbstractRequestExpectationManager implements RequestExpect return this.expectations; } - public void update(RequestExpectation expectation) { - if (expectation.hasRemainingCount()) { - getExpectations().add(expectation); - } - else { - getExpectations().remove(expectation); - } - } - - public void updateAll(Collection expectations) { - for (RequestExpectation expectation : expectations) { - update(expectation); - } - } - public RequestExpectation findExpectation(ClientHttpRequest request) throws IOException { for (RequestExpectation expectation : getExpectations()) { try { @@ -185,6 +172,30 @@ public abstract class AbstractRequestExpectationManager implements RequestExpect return null; } + /** + * Invoke this for an expectation that has been matched. + *

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); + } + else { + getExpectations().remove(expectation); + } + } + + /** + * Collection variant of {@link #update(RequestExpectation)} that can + * be used to insert expectations. + */ + public void updateAll(Collection expectations) { + for (RequestExpectation expectation : expectations) { + update(expectation); + } + } + public void reset() { getExpectations().clear(); } diff --git a/spring-test/src/test/java/org/springframework/test/web/client/MockRestServiceServerTests.java b/spring-test/src/test/java/org/springframework/test/web/client/MockRestServiceServerTests.java index ddf8b26fdb..49c5c1b5f2 100644 --- a/spring-test/src/test/java/org/springframework/test/web/client/MockRestServiceServerTests.java +++ b/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 class MockRestServiceServerTests { } @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 class MockRestServiceServerTests { } @Test - public void ignoreExpectOrder() throws Exception { + public void ignoreExpectOrder() { MockRestServiceServer server = MockRestServiceServer.bindTo(this.restTemplate) .ignoreExpectOrder(true).build(); @@ -77,7 +82,7 @@ public class MockRestServiceServerTests { } @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 class MockRestServiceServerTests { } @Test - public void resetAndReuseServerWithUnorderedExpectationManager() throws Exception { + public void resetAndReuseServerWithUnorderedExpectationManager() { MockRestServiceServer server = MockRestServiceServer.bindTo(this.restTemplate) .ignoreExpectOrder(true).build(); @@ -107,4 +112,24 @@ public class MockRestServiceServerTests { 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(); + } + } diff --git a/spring-test/src/test/java/org/springframework/test/web/client/SimpleRequestExpectationManagerTests.java b/spring-test/src/test/java/org/springframework/test/web/client/SimpleRequestExpectationManagerTests.java index 3390d2df84..c350a88a91 100644 --- a/spring-test/src/test/java/org/springframework/test/web/client/SimpleRequestExpectationManagerTests.java +++ b/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 class SimpleRequestExpectationManagerTests { 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)); diff --git a/spring-test/src/test/java/org/springframework/test/web/client/UnorderedRequestExpectationManagerTests.java b/spring-test/src/test/java/org/springframework/test/web/client/UnorderedRequestExpectationManagerTests.java index 163f3865e6..80b673ef00 100644 --- a/spring-test/src/test/java/org/springframework/test/web/client/UnorderedRequestExpectationManagerTests.java +++ b/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.HttpMethod; 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 {