Polish MockRestRequestMatchers query param support

Issue: SPR-14995

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

@@ -19,13 +19,13 @@ package org.springframework.test.web.client.match;
 import java.io.IOException;
 import java.net.URI;
 import java.nio.charset.Charset;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import javax.xml.xpath.XPathExpressionException;
 
 import org.hamcrest.Matcher;
 
-import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.client.ClientHttpRequest;
 import org.springframework.test.util.AssertionErrors;
@@ -38,6 +38,7 @@ import org.springframework.web.util.UriUtils;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.hamcrest.MatcherAssert.*;
+import static org.junit.Assert.assertNotNull;
 import static org.springframework.test.util.AssertionErrors.*;
 
 /**
@@ -65,6 +66,21 @@ public abstract class MockRestRequestMatchers {
 		};
 	}
 
+	/**
+	 * Assert the {@link HttpMethod} of the request.
+	 * @param method the HTTP method
+	 * @return the request matcher
+	 */
+	public static RequestMatcher method(final HttpMethod method) {
+		Assert.notNull(method, "'method' must not be null");
+		return new RequestMatcher() {
+			@Override
+			public void match(ClientHttpRequest request) throws AssertionError {
+				AssertionErrors.assertEquals("Unexpected HttpMethod", method, request.getMethod());
+			}
+		};
+	}
+
 	/**
 	 * Assert the request URI string with the given matcher.
 	 * @param matcher String matcher for the expected URI
@@ -95,21 +111,6 @@ public abstract class MockRestRequestMatchers {
 		};
 	}
 
-	/**
-	 * Assert the {@link HttpMethod} of the request.
-	 * @param method the HTTP method
-	 * @return the request matcher
-	 */
-	public static RequestMatcher method(final HttpMethod method) {
-		Assert.notNull(method, "'method' must not be null");
-		return new RequestMatcher() {
-			@Override
-			public void match(ClientHttpRequest request) throws AssertionError {
-				AssertionErrors.assertEquals("Unexpected HttpMethod", method, request.getMethod());
-			}
-		};
-	}
-
 	/**
 	 * Expect a request to the given URI.
 	 * @param uri the expected URI
@@ -126,110 +127,86 @@ public abstract class MockRestRequestMatchers {
 	}
 
 	/**
-	 * Assert request header values with the given Hamcrest matcher.
+	 * Assert request query parameter values with the given Hamcrest matcher.
 	 */
 	@SafeVarargs
-	public static RequestMatcher header(final String name, final Matcher<? super String>... matchers) {
+	public static RequestMatcher queryParam(final String name, final Matcher<? super String>... matchers) {
 		return new RequestMatcher() {
 			@Override
 			public void match(ClientHttpRequest request) {
-				assertHeaderValueCount(name, request.getHeaders(), matchers.length);
+				MultiValueMap<String, String> params = getQueryParams(request);
+				assertValueCount("query param", name, params, matchers.length);
 				for (int i = 0 ; i < matchers.length; i++) {
-					assertThat("Request header", request.getHeaders().get(name).get(i), matchers[i]);
+					assertThat("Query param", params.get(name).get(i), matchers[i]);
 				}
 			}
 		};
 	}
 
 	/**
-	 * Assert request header values.
+	 * Assert request query parameter values.
 	 */
-	public static RequestMatcher header(final String name, final String... expectedValues) {
+	public static RequestMatcher queryParam(final String name, final String... expectedValues) {
 		return new RequestMatcher() {
 			@Override
 			public void match(ClientHttpRequest request) {
-				assertHeaderValueCount(name, request.getHeaders(), expectedValues.length);
+				MultiValueMap<String, String> params = getQueryParams(request);
+				assertValueCount("query param", name, params, expectedValues.length);
 				for (int i = 0 ; i < expectedValues.length; i++) {
-					assertEquals("Request header + [" + name + "]",
-							expectedValues[i], request.getHeaders().get(name).get(i));
+					assertEquals("Query param + [" + name + "]", expectedValues[i], params.get(name).get(i));
 				}
 			}
 		};
 	}
 
-	private static void assertHeaderValueCount(final String name, HttpHeaders headers, int expectedCount) {
-		assertMultiValueMapCount("header", name, headers, expectedCount);
+	private static MultiValueMap<String, String> getQueryParams(ClientHttpRequest request) {
+		return UriComponentsBuilder.fromUri(request.getURI()).build().getQueryParams();
 	}
 
+	private static void assertValueCount(String valueType, final String name,
+			MultiValueMap<String, String> map, int count) {
 
-	/**
-	 * Assert request query parameter values with the given Hamcrest matcher.
-	 */
-	@SafeVarargs
-	public static RequestMatcher queryParameter(final String name, final Matcher<? super String>... matchers) {
-		return queryParameter(name, UTF_8, matchers);
+		List<String> values = map.get(name);
+
+		String message = "Expected " + valueType + " <" + name + ">";
+		assertNotNull(message, values);
+
+		assertTrue(message + " to have at least <" + count + "> values but found " + values,
+				count <= values.size());
 	}
 
 	/**
-	 * Assert request query parameter values with the given Hamcrest matcher.
+	 * Assert request header values with the given Hamcrest matcher.
 	 */
 	@SafeVarargs
-	public static RequestMatcher queryParameter(final String name, final Charset charset,
-												final Matcher<? super String>... matchers) {
+	public static RequestMatcher header(final String name, final Matcher<? super String>... matchers) {
 		return new RequestMatcher() {
 			@Override
 			public void match(ClientHttpRequest request) {
-				MultiValueMap<String, String> queryParameters = getQueryParameters(request.getURI().toString(), charset);
-				assertQueryParameterValueCount(name, queryParameters, matchers.length);
+				assertValueCount("header", name, request.getHeaders(), matchers.length);
 				for (int i = 0 ; i < matchers.length; i++) {
-					assertThat("Request query parameter", queryParameters.get(name).get(i), matchers[i]);
+					assertThat("Request header", request.getHeaders().get(name).get(i), matchers[i]);
 				}
 			}
 		};
 	}
 
 	/**
-	 * Assert request query parameter values.
-	 */
-	public static RequestMatcher queryParameter(final String name, final String... expectedValues) {
-		return queryParameter(name, UTF_8, expectedValues);
-	}
-
-	/**
-	 * Assert request query parameter values.
+	 * Assert request header values.
 	 */
-	public static RequestMatcher queryParameter(final String name, final Charset charset, final String... expectedValues) {
+	public static RequestMatcher header(final String name, final String... expectedValues) {
 		return new RequestMatcher() {
 			@Override
 			public void match(ClientHttpRequest request) {
-				MultiValueMap<String, String> queryParameters = getQueryParameters(request.getURI().toString(), charset);
-				assertQueryParameterValueCount(name, queryParameters, expectedValues.length);
+				assertValueCount("header", name, request.getHeaders(), expectedValues.length);
 				for (int i = 0 ; i < expectedValues.length; i++) {
-					assertEquals("Request query parameter + [" + name + "]",
-							expectedValues[i], queryParameters.get(name).get(i));
+					assertEquals("Request header + [" + name + "]",
+							expectedValues[i], request.getHeaders().get(name).get(i));
 				}
 			}
 		};
 	}
 
-	private static MultiValueMap<String, String> getQueryParameters(String uri, Charset charset) {
-		String decodeUri = UriUtils.decode(uri, charset);
-		MultiValueMap<String, String> queryParameters = UriComponentsBuilder.fromUriString(decodeUri)
-				.build().getQueryParams();
-		return queryParameters;
-	}
-
-	private static void assertQueryParameterValueCount(final String name, MultiValueMap<String, String> queryParameters, int expectedCount) {
-		assertMultiValueMapCount("query parameter", name, queryParameters, expectedCount);
-	}
-
-	private static void assertMultiValueMapCount(String type, final String name, MultiValueMap<String, String> multiValueMap, int expectedCount) {
-		List<String> actualValues = multiValueMap.get(name);
-		assertTrue("Expected " + type + " <" + name + ">", actualValues != null);
-		assertTrue("Expected " + type + " <" + name + "> to have at least <" + expectedCount +
-				"> values but found " + actualValues, expectedCount <= actualValues.size());
-	}
-
 	/**
 	 * Access to request body matchers.
 	 */

spring-test/src/test/java/org/springframework/test/web/client/match/MockRestRequestMatchersTests.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.
@@ -16,20 +16,16 @@
 
 package org.springframework.test.web.client.match;
 
+import java.net.URI;
+import java.util.Arrays;
+import java.util.Collections;
+
 import org.junit.Test;
+
 import org.springframework.http.HttpMethod;
 import org.springframework.mock.http.client.MockClientHttpRequest;
-import org.springframework.test.web.client.RequestMatcher;
-import org.springframework.util.LinkedMultiValueMap;
-import org.springframework.util.MultiValueMap;
-import org.springframework.web.util.UriComponentsBuilder;
-
-import java.net.URI;
-import java.util.Arrays;
 
 import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.junit.Assert.assertThat;
 
 /**
  * Unit tests for {@link MockRestRequestMatchers}.
@@ -129,79 +125,39 @@ public class MockRestRequestMatchersTests {
 
 	@Test(expected = AssertionError.class)
 	public void headersWithMissingValue() throws Exception {
-		this.request.getHeaders().put("foo", Arrays.asList("bar"));
+		this.request.getHeaders().put("foo", Collections.singletonList("bar"));
 
 		MockRestRequestMatchers.header("foo", "bar", "baz").match(this.request);
 	}
 
 	@Test
-	public void queryParameter() throws Exception {
-		this.request.setURI(createUriWithQueryParameters("foo", "bar", "baz"));
-
-		RequestMatcher requestMatcher = MockRestRequestMatchers.queryParameter("foo", "bar", "baz");
-		assertThat("Factory method did not create any request matcher", requestMatcher, notNullValue());
-		requestMatcher.match(this.request);
+	public void queryParam() throws Exception {
+		this.request.setURI(new URI("http://foo.com/a?foo=bar&foo=baz"));
+		MockRestRequestMatchers.queryParam("foo", "bar", "baz").match(this.request);
 	}
 
 	@Test(expected = AssertionError.class)
-	public void queryParameterMissing() throws Exception {
-		this.request.setURI(UriComponentsBuilder
-				.fromUriString("http://foo.com")
-				.path("/bar")
-				.build().encode()
-				.toUri());
-
-		MockRestRequestMatchers.queryParameter("foo", "bar").match(this.request);
+	public void queryParamMissing() throws Exception {
+		this.request.setURI(new URI("http://foo.com/a"));
+		MockRestRequestMatchers.queryParam("foo", "bar").match(this.request);
 	}
 
 	@Test(expected = AssertionError.class)
-	public void queryParameterMissingValue() throws Exception {
-		this.request.setURI(createUriWithQueryParameters("foo", "bar", "baz"));
-
-		MockRestRequestMatchers.queryParameter("foo", "bad").match(this.request);
+	public void queryParamMissingValue() throws Exception {
+		this.request.setURI(new URI("http://foo.com/a?foo=bar&foo=baz"));
+		MockRestRequestMatchers.queryParam("foo", "bad").match(this.request);
 	}
 
 	@Test
-	public void queryParameterContains() throws Exception {
-		this.request.setURI(createUriWithQueryParameters("foo", "bar", "baz"));
-
-        RequestMatcher requestMatcher = MockRestRequestMatchers.queryParameter("foo", containsString("ba"));
-        assertThat("Factory method did not create any request matcher", requestMatcher, notNullValue());
-        requestMatcher.match(this.request);
+	public void queryParamContains() throws Exception {
+		this.request.setURI(new URI("http://foo.com/a?foo=bar&foo=baz"));
+		MockRestRequestMatchers.queryParam("foo", containsString("ba")).match(this.request);
 	}
 
 	@Test(expected = AssertionError.class)
-	public void queryParameterContainsWithMissingValue() throws Exception {
-		this.request.setURI(createUriWithQueryParameters("foo", "bar", "baz"));
-
-		MockRestRequestMatchers.queryParameter("foo", containsString("bx")).match(this.request);
-	}
-
-	@Test
-	public void queryParameters() throws Exception {
-		this.request.setURI(createUriWithQueryParameters("foo", "bar", "baz"));
-
-        RequestMatcher requestMatcher = MockRestRequestMatchers.queryParameter("foo", "bar", "baz");
-        assertThat("Factory method did not create any request matcher", requestMatcher, notNullValue());
-        requestMatcher.match(this.request);
-	}
-
-	@Test(expected = AssertionError.class)
-	public void queryParametersWithMissingValue() throws Exception {
-		this.request.setURI(createUriWithQueryParameters("foo", "bar"));
-
-		MockRestRequestMatchers.queryParameter("foo", "bar", "baz").match(this.request);
-	}
-
-
-	private URI createUriWithQueryParameters(String key, String ... values) {
-		MultiValueMap<String, String> params = new LinkedMultiValueMap<>();
-		params.put(key, Arrays.asList(values));
-		return UriComponentsBuilder
-				.fromUriString("http://foo.com")
-				.path("/bar")
-				.queryParams(params)
-				.build().encode().toUri();
+	public void queryParamContainsWithMissingValue() throws Exception {
+		this.request.setURI(new URI("http://foo.com/a?foo=bar&foo=baz"));
+		MockRestRequestMatchers.queryParam("foo", containsString("bx")).match(this.request);
 	}
 
 }