From efb735aa9989e9c3337daf9b91b8eab7e4e1f08a Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 16 Jun 2017 13:32:43 +0200 Subject: [PATCH] SimpleRequestExpectationManager properly handles sequential requests with different count Issue: SPR-15672 --- .../AbstractRequestExpectationManager.java | 12 +++---- .../web/client/RequestExpectationManager.java | 4 +-- .../SimpleRequestExpectationManager.java | 23 ++++--------- .../SimpleRequestExpectationManagerTests.java | 34 +++++++++++-------- 4 files changed, 35 insertions(+), 38 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 d9390b3a7c1..f91b1c77e1c 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 @@ -39,6 +39,7 @@ import org.springframework.util.Assert; * to expectations following the order of declaration or not. * * @author Rossen Stoyanchev + * @author Juergen Hoeller * @since 4.3 */ public abstract class AbstractRequestExpectationManager implements RequestExpectationManager { @@ -47,8 +48,6 @@ public abstract class AbstractRequestExpectationManager implements RequestExpect private final List requests = new LinkedList<>(); - private final Object lock = new Object(); - protected List getExpectations() { return this.expectations; @@ -69,12 +68,13 @@ public abstract class AbstractRequestExpectationManager implements RequestExpect @Override public ClientHttpResponse validateRequest(ClientHttpRequest request) throws IOException { - synchronized (this.lock) { - if (getRequests().isEmpty()) { + List requests = getRequests(); + synchronized (requests) { + if (requests.isEmpty()) { afterExpectationsDeclared(); } ClientHttpResponse response = validateRequestInternal(request); - getRequests().add(request); + requests.add(request); return response; } } @@ -188,7 +188,7 @@ public abstract class AbstractRequestExpectationManager implements RequestExpect } public void reset() { - this.expectations.clear(); + getExpectations().clear(); } } diff --git a/spring-test/src/main/java/org/springframework/test/web/client/RequestExpectationManager.java b/spring-test/src/main/java/org/springframework/test/web/client/RequestExpectationManager.java index a79f9210179..4495ffa0adb 100644 --- a/spring-test/src/main/java/org/springframework/test/web/client/RequestExpectationManager.java +++ b/spring-test/src/main/java/org/springframework/test/web/client/RequestExpectationManager.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. @@ -45,7 +45,7 @@ public interface RequestExpectationManager { * @param request the request * @return the response to return if the request was validated. * @throws AssertionError when some expectations were not met - * @throws IOException + * @throws IOException in case of any validation errors */ ClientHttpResponse validateRequest(ClientHttpRequest request) throws IOException; diff --git a/spring-test/src/main/java/org/springframework/test/web/client/SimpleRequestExpectationManager.java b/spring-test/src/main/java/org/springframework/test/web/client/SimpleRequestExpectationManager.java index 89d91f1e62a..f76449e3982 100644 --- a/spring-test/src/main/java/org/springframework/test/web/client/SimpleRequestExpectationManager.java +++ b/spring-test/src/main/java/org/springframework/test/web/client/SimpleRequestExpectationManager.java @@ -32,6 +32,7 @@ import org.springframework.util.Assert; * Subsequent request executions may be inserted anywhere thereafter. * * @author Rossen Stoyanchev + * @author Juergen Hoeller * @since 4.3 */ public class SimpleRequestExpectationManager extends AbstractRequestExpectationManager { @@ -49,29 +50,19 @@ public class SimpleRequestExpectationManager extends AbstractRequestExpectationM @Override public ClientHttpResponse validateRequestInternal(ClientHttpRequest request) throws IOException { - RequestExpectation expectation; - try { - expectation = next(request); - expectation.match(request); - } - catch (AssertionError error) { - expectation = this.repeatExpectations.findExpectation(request); - if (expectation == null) { - throw error; + RequestExpectation expectation = this.repeatExpectations.findExpectation(request); + if (expectation == null) { + if (!this.expectationIterator.hasNext()) { + throw createUnexpectedRequestError(request); } + expectation = this.expectationIterator.next(); + expectation.match(request); } ClientHttpResponse response = expectation.createResponse(request); this.repeatExpectations.update(expectation); return response; } - private RequestExpectation next(ClientHttpRequest request) { - if (this.expectationIterator.hasNext()) { - return this.expectationIterator.next(); - } - throw createUnexpectedRequestError(request); - } - @Override public void reset() { super.reset(); 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 2542fbf311b..555353bb206 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 @@ -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. @@ -25,26 +25,22 @@ import org.junit.rules.ExpectedException; import org.springframework.http.HttpMethod; import org.springframework.http.client.ClientHttpRequest; +import org.springframework.mock.http.client.MockClientHttpRequest; -import static org.junit.Assert.assertEquals; -import static org.springframework.http.HttpMethod.GET; -import static org.springframework.http.HttpMethod.POST; -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.ExpectedCount.twice; -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 SimpleRequestExpectationManager}. + * * @author Rossen Stoyanchev */ public class SimpleRequestExpectationManagerTests { - private SimpleRequestExpectationManager manager = new SimpleRequestExpectationManager(); + private final SimpleRequestExpectationManager manager = new SimpleRequestExpectationManager(); @Rule public ExpectedException thrown = ExpectedException.none(); @@ -162,11 +158,21 @@ public class SimpleRequestExpectationManagerTests { this.manager.validateRequest(createRequest(POST, "/foo")); } + @Test // SPR-15672 + public void sequentialRequestsWithDifferentCount() throws Exception { + this.manager.expectRequest(times(2), requestTo("/foo")).andExpect(method(GET)).andRespond(withSuccess()); + this.manager.expectRequest(once(), 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")); + } + @SuppressWarnings("deprecation") private ClientHttpRequest createRequest(HttpMethod method, String url) { try { - return new org.springframework.mock.http.client.MockAsyncClientHttpRequest(method, new URI(url)); + return new MockClientHttpRequest(method, new URI(url)); } catch (URISyntaxException ex) { throw new IllegalStateException(ex);