Refactor resolver tests

Extract NamedValueArgumentResolverTests.
Move form data vs query params tests into HttpRequestValues.

See gh-28386

spring-web/src/test/java/org/springframework/web/service/invoker/CookieValueArgumentResolverTests.java

@@ -17,125 +17,39 @@
 package org.springframework.web.service.invoker;
 
 import java.util.List;
-import java.util.Map;
-import java.util.Optional;
 
-import org.apache.groovy.util.Maps;
 import org.junit.jupiter.api.Test;
 
-import org.springframework.lang.Nullable;
 import org.springframework.util.ObjectUtils;
 import org.springframework.web.bind.annotation.CookieValue;
 import org.springframework.web.service.annotation.GetExchange;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 
 
 /**
  * Unit tests for {@link RequestHeaderArgumentResolver}.
+ * <p>For base class functionality, see {@link NamedValueArgumentResolverTests}.
  *
  * @author Rossen Stoyanchev
  */
 class CookieValueArgumentResolverTests {
 
-	private final TestHttpClientAdapter clientAdapter = new TestHttpClientAdapter();
+	private final TestHttpClientAdapter client = new TestHttpClientAdapter();
 
-	private final Service service = this.clientAdapter.createService(Service.class);
+	private final Service service = HttpServiceProxyFactory.builder(this.client).build().createClient(Service.class);
 
 
+	// Base class functionality should be tested in NamedValueArgumentResolverTests.
+
 	@Test
-	void stringCookie() {
-		this.service.executeString("test");
-		assertCookie("cookie", "test");
-	}
-
-	@Test
-	void objectCookie() {
-		this.service.execute(Boolean.TRUE);
-		assertCookie("cookie", "true");
-	}
-
-	@Test
-	void listCookie() {
-		this.service.executeList(List.of("test1", Boolean.TRUE, "test3"));
-		assertCookie("multiValueCookie", "test1", "true", "test3");
-	}
-
-	@Test
-	void arrayCookie() {
-		this.service.executeArray("test1", Boolean.FALSE, "test3");
-		assertCookie("multiValueCookie", "test1", "false", "test3");
-	}
-
-	@Test
-	void namedCookie() {
-		this.service.executeNamed("test");
-		assertCookie("cookieRenamed", "test");
-	}
-
-	@SuppressWarnings("ConstantConditions")
-	@Test
-	void nullCookieRequired() {
-		assertThatIllegalArgumentException().isThrownBy(() -> this.service.executeString(null));
-	}
-
-	@Test
-	void nullCookieNotRequired() {
-		this.service.executeNotRequired(null);
-		assertCookie("cookie");
-	}
-
-	@Test
-	void nullCookieWithDefaultValue() {
-		this.service.executeWithDefaultValue(null);
-		assertCookie("cookie", "default");
-	}
-
-	@Test
-	void optionalStringCookie() {
-		this.service.executeOptional(Optional.of("test"));
-		assertCookie("cookie", "test");
-	}
-
-	@Test
-	void optionalObjectCookie() {
-		this.service.executeOptional(Optional.of(Boolean.TRUE));
-		assertCookie("cookie", "true");
-	}
-
-	@Test
-	void optionalEmpty() {
-		this.service.executeOptional(Optional.empty());
-		assertCookie("cookie");
-	}
-
-	@Test
-	void optionalEmpthyWithDefaultValue() {
-		this.service.executeOptionalWithDefaultValue(Optional.empty());
-		assertCookie("cookie", "default");
-	}
-
-	@Test
-	void mapOfCookies() {
-		this.service.executeMap(Maps.of("cookie1", "true", "cookie2", "false"));
-		assertCookie("cookie1", "true");
-		assertCookie("cookie2", "false");
-	}
-
-	@Test
-	void mapOfCookiesIsNull() {
-		assertThatIllegalArgumentException().isThrownBy(() -> this.service.executeMap(null));
-	}
-
-	@Test
-	void mapOfCookiesHasOptionalValue() {
-		this.service.executeMapWithOptionalValue(Map.of("cookie", Optional.of("test")));
+	void cookieValue() {
+		this.service.execute("test");
 		assertCookie("cookie", "test");
 	}
 
 	private void assertCookie(String key, String... values) {
-		List<String> actualValues = this.clientAdapter.getRequestValues().getCookies().get(key);
+		List<String> actualValues = this.client.getRequestValues().getCookies().get(key);
 		if (ObjectUtils.isEmpty(values)) {
 			assertThat(actualValues).isNull();
 		}
@@ -145,41 +59,10 @@ class CookieValueArgumentResolverTests {
 	}
 
 
-	@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
 	private interface Service {
 
 		@GetExchange
-		void executeString(@CookieValue String cookie);
-
-		@GetExchange
-		void execute(@CookieValue Object cookie);
-
-		@GetExchange
-		void executeList(@CookieValue List<Object> multiValueCookie);
-
-		@GetExchange
-		void executeArray(@CookieValue Object... multiValueCookie);
-
-		@GetExchange
-		void executeNamed(@CookieValue(name = "cookieRenamed") String cookie);
-
-		@GetExchange
-		void executeNotRequired(@Nullable @CookieValue(required = false) String cookie);
-
-		@GetExchange
-		void executeWithDefaultValue(@Nullable @CookieValue(defaultValue = "default") String cookie);
-
-		@GetExchange
-		void executeOptional(@CookieValue Optional<Object> cookie);
-
-		@GetExchange
-		void executeOptionalWithDefaultValue(@CookieValue(defaultValue = "default") Optional<Object> cookie);
-
-		@GetExchange
-		void executeMap(@Nullable @CookieValue Map<String, String> cookie);
-
-		@GetExchange
-		void executeMapWithOptionalValue(@CookieValue Map<String, Optional<String>> cookies);
+		void execute(@CookieValue String cookie);
 
 	}
 

spring-web/src/test/java/org/springframework/web/service/invoker/HttpMethodArgumentResolverTests.java

@@ -19,7 +19,6 @@ package org.springframework.web.service.invoker;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.http.HttpMethod;
-import org.springframework.lang.Nullable;
 import org.springframework.web.service.annotation.GetExchange;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -29,40 +28,35 @@ import static org.assertj.core.api.Assertions.assertThat;
  * Tests for {@link HttpMethodArgumentResolver}.
  *
  * @author Olga Maciaszek-Sharma
+ * @author Rossen Stoyanchev
  */
 public class HttpMethodArgumentResolverTests {
 
-	private final TestHttpClientAdapter clientAdapter = new TestHttpClientAdapter();
+	private final TestHttpClientAdapter client = new TestHttpClientAdapter();
 
-	private final Service service = this.clientAdapter.createService(Service.class);
+	private final Service service = HttpServiceProxyFactory.builder(this.client).build().createClient(Service.class);
 
 
 	@Test
-	void shouldResolveRequestMethodFromArgument() {
-		this.service.execute(HttpMethod.GET);
-		assertThat(getActualMethod()).isEqualTo(HttpMethod.GET);
+	void requestMethodOverride() {
+		this.service.execute(HttpMethod.POST);
+		assertThat(getActualMethod()).isEqualTo(HttpMethod.POST);
 	}
 
 	@Test
-	void shouldIgnoreArgumentsNotMatchingType() {
+	void ignoreOtherArgumentTypes() {
 		this.service.execute("test");
 		assertThat(getActualMethod()).isEqualTo(HttpMethod.GET);
 	}
 
 	@Test
-	void shouldOverrideMethodAnnotation() {
-		this.service.executeGet(HttpMethod.POST);
-		assertThat(getActualMethod()).isEqualTo(HttpMethod.POST);
-	}
-
-	@Test
-	void shouldIgnoreNullValue() {
-		this.service.executeForNull(null);
+	void ignoreNull() {
+		this.service.execute((HttpMethod) null);
 		assertThat(getActualMethod()).isEqualTo(HttpMethod.GET);
 	}
 
 	private HttpMethod getActualMethod() {
-		return this.clientAdapter.getRequestValues().getHttpMethod();
+		return this.client.getRequestValues().getHttpMethod();
 	}
 
 
@@ -71,17 +65,9 @@ public class HttpMethodArgumentResolverTests {
 		@GetExchange
 		void execute(HttpMethod method);
 
-		@GetExchange
-		void executeGet(HttpMethod method);
-
 		@GetExchange
 		void execute(String test);
 
-		@GetExchange
-		void execute(HttpMethod firstMethod, HttpMethod secondMethod);
-
-		@GetExchange
-		void executeForNull(@Nullable HttpMethod method);
 	}
 
 }

spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java

@@ -0,0 +1,111 @@
+/*
+ * Copyright 2002-2022 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.web.service.invoker;
+
+
+import java.net.URI;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import org.springframework.http.HttpMethod;
+import org.springframework.http.MediaType;
+import org.springframework.web.util.UriComponentsBuilder;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.assertj.core.api.Assertions.assertThat;
+
+
+/**
+ * Unit tests for {@link HttpRequestValues}.
+ *
+ * @author Rossen Stoyanchev
+ */
+public class HttpRequestValuesTests {
+
+	@Test
+	void defaultUri() {
+		HttpRequestValues requestValues = HttpRequestValues.builder(HttpMethod.GET).build();
+
+		assertThat(requestValues.getUri()).isNull();
+		assertThat(requestValues.getUriTemplate()).isEqualTo("");
+	}
+
+	@ParameterizedTest
+	@ValueSource(strings = {"POST", "PUT", "PATCH"})
+	void requestParamAsFormData(String httpMethod) {
+
+		HttpRequestValues requestValues = HttpRequestValues.builder(HttpMethod.valueOf(httpMethod))
+				.setContentType(MediaType.APPLICATION_FORM_URLENCODED)
+				.addRequestParameter("param1", "1st value")
+				.addRequestParameter("param2", "2nd value A", "2nd value B")
+				.build();
+
+		Object body = requestValues.getBodyValue();
+		assertThat(body).isNotNull().isInstanceOf(byte[].class);
+		assertThat(new String((byte[]) body, UTF_8)).isEqualTo("param1=1st+value&param2=2nd+value+A&param2=2nd+value+B");
+	}
+
+	@Test
+	void requestParamAsQueryParamsInUriTemplate() {
+
+		HttpRequestValues requestValues = HttpRequestValues.builder(HttpMethod.POST)
+				.setUriTemplate("/path")
+				.addRequestParameter("param1", "1st value")
+				.addRequestParameter("param2", "2nd value A", "2nd value B")
+				.build();
+
+		String uriTemplate = requestValues.getUriTemplate();
+		assertThat(uriTemplate).isNotNull();
+
+		assertThat(uriTemplate)
+				.isEqualTo("/path?" +
+						"{queryParam0}={queryParam0[0]}&" +
+						"{queryParam1}={queryParam1[0]}&" +
+						"{queryParam1}={queryParam1[1]}");
+
+		assertThat(requestValues.getUriVariables())
+				.containsOnlyKeys("queryParam0", "queryParam1", "queryParam0[0]", "queryParam1[0]", "queryParam1[1]")
+				.containsEntry("queryParam0", "param1")
+				.containsEntry("queryParam1", "param2")
+				.containsEntry("queryParam0[0]", "1st value")
+				.containsEntry("queryParam1[0]", "2nd value A")
+				.containsEntry("queryParam1[1]", "2nd value B");
+
+		URI uri = UriComponentsBuilder.fromUriString(uriTemplate)
+				.encode()
+				.build(requestValues.getUriVariables());
+
+		assertThat(uri.toString())
+				.isEqualTo("/path?param1=1st%20value&param2=2nd%20value%20A&param2=2nd%20value%20B");
+	}
+
+	@Test
+	void requestParamAsQueryParamsInUri() {
+
+		HttpRequestValues requestValues = HttpRequestValues.builder(HttpMethod.POST)
+				.setUri(URI.create("/path"))
+				.addRequestParameter("param1", "1st value")
+				.addRequestParameter("param2", "2nd value A", "2nd value B")
+				.build();
+
+		assertThat(requestValues.getUri().toString())
+				.isEqualTo("/path?param1=1st%20value&param2=2nd%20value%20A&param2=2nd%20value%20B");
+	}
+
+}

spring-web/src/test/java/org/springframework/web/service/invoker/HttpServiceMethodTests.java

@@ -58,10 +58,12 @@ public class HttpServiceMethodTests {
 
 	private final TestHttpClientAdapter clientAdapter = new TestHttpClientAdapter();
 
+	private final HttpServiceProxyFactory proxyFactory = HttpServiceProxyFactory.builder(this.clientAdapter).build();
+
 
 	@Test
 	void reactorService() {
-		ReactorService service = this.clientAdapter.createService(ReactorService.class);
+		ReactorService service = this.proxyFactory.createClient(ReactorService.class);
 
 		Mono<Void> voidMono = service.execute();
 		StepVerifier.create(voidMono).verifyComplete();
@@ -94,7 +96,7 @@ public class HttpServiceMethodTests {
 
 	@Test
 	void rxJavaService() {
-		RxJavaService service = this.clientAdapter.createService(RxJavaService.class);
+		RxJavaService service = this.proxyFactory.createClient(RxJavaService.class);
 		Completable completable = service.execute();
 		assertThat(completable).isNotNull();
 
@@ -121,7 +123,7 @@ public class HttpServiceMethodTests {
 	@Test
 	void blockingService() {
 
-		BlockingService service = this.clientAdapter.createService(BlockingService.class);
+		BlockingService service = this.proxyFactory.createClient(BlockingService.class);
 
 		service.execute();
 
@@ -141,7 +143,7 @@ public class HttpServiceMethodTests {
 	@Test
 	void methodAnnotatedService() {
 
-		MethodLevelAnnotatedService service = this.clientAdapter.createService(MethodLevelAnnotatedService.class);
+		MethodLevelAnnotatedService service = this.proxyFactory.createClient(MethodLevelAnnotatedService.class);
 
 		service.performGet();
 
@@ -163,7 +165,7 @@ public class HttpServiceMethodTests {
 	@Test
 	void typeAndMethodAnnotatedService() {
 
-		MethodLevelAnnotatedService service = this.clientAdapter.createService(TypeAndMethodLevelAnnotatedService.class);
+		MethodLevelAnnotatedService service = this.proxyFactory.createClient(TypeAndMethodLevelAnnotatedService.class);
 
 		service.performGet();
 

spring-web/src/test/java/org/springframework/web/service/invoker/HttpUrlArgumentResolverTests.java

@@ -32,9 +32,9 @@ import static org.assertj.core.api.Assertions.assertThat;
  */
 public class HttpUrlArgumentResolverTests {
 
-	private final TestHttpClientAdapter clientAdapter = new TestHttpClientAdapter();
+	private final TestHttpClientAdapter client = new TestHttpClientAdapter();
 
-	private final Service service = this.clientAdapter.createService(Service.class);
+	private final Service service = HttpServiceProxyFactory.builder(this.client).build().createClient(Service.class);
 
 
 	@Test
@@ -47,7 +47,7 @@ public class HttpUrlArgumentResolverTests {
 	}
 
 	private HttpRequestValues getRequestValues() {
-		return this.clientAdapter.getRequestValues();
+		return this.client.getRequestValues();
 	}
 
 

spring-web/src/test/java/org/springframework/web/service/invoker/NamedValueArgumentResolverTests.java

@@ -0,0 +1,247 @@
+/*
+ * Copyright 2002-2022 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.web.service.invoker;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+import org.apache.groovy.util.Maps;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.core.MethodParameter;
+import org.springframework.core.annotation.AliasFor;
+import org.springframework.format.support.DefaultFormattingConversionService;
+import org.springframework.lang.Nullable;
+import org.springframework.util.LinkedMultiValueMap;
+import org.springframework.util.MultiValueMap;
+import org.springframework.util.ObjectUtils;
+import org.springframework.web.bind.annotation.ValueConstants;
+import org.springframework.web.service.annotation.GetExchange;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+
+
+/**
+ * Unit tests for {@link AbstractNamedValueArgumentResolver} through a
+ * {@link TestValue @TestValue} annotation and {@link TestNamedValueArgumentResolver}.
+ *
+ * @author Rossen Stoyanchev
+ */
+class NamedValueArgumentResolverTests {
+
+	private final TestHttpClientAdapter client = new TestHttpClientAdapter();
+
+	private final TestNamedValueArgumentResolver argumentResolver = new TestNamedValueArgumentResolver();
+
+	private final Service service =
+			HttpServiceProxyFactory.builder(this.client)
+					.addCustomResolver(this.argumentResolver)
+					.build()
+					.createClient(Service.class);
+
+
+	@Test
+	void stringTestValue() {
+		this.service.executeString("test");
+		assertTestValue("value", "test");
+	}
+
+	@Test
+	void objectTestValue() {
+		this.service.execute(Boolean.TRUE);
+		assertTestValue("value", "true");
+	}
+
+	@Test
+	void listTestValue() {
+		this.service.executeList(List.of("test1", Boolean.TRUE, "test3"));
+		assertTestValue("testValues", "test1", "true", "test3");
+	}
+
+	@Test
+	void arrayTestValue() {
+		this.service.executeArray("test1", Boolean.FALSE, "test3");
+		assertTestValue("testValues", "test1", "false", "test3");
+	}
+
+	@Test
+	void namedTestValue() {
+		this.service.executeNamed("test");
+		assertTestValue("valueRenamed", "test");
+	}
+
+	@SuppressWarnings("ConstantConditions")
+	@Test
+	void nullTestValueRequired() {
+		assertThatIllegalArgumentException().isThrownBy(() -> this.service.executeString(null));
+	}
+
+	@Test
+	void nullTestValueNotRequired() {
+		this.service.executeNotRequired(null);
+		assertTestValue("value");
+	}
+
+	@Test
+	void nullTestValueWithDefaultValue() {
+		this.service.executeWithDefaultValue(null);
+		assertTestValue("value", "default");
+	}
+
+	@Test
+	void optionalStringTestValue() {
+		this.service.executeOptional(Optional.of("test"));
+		assertTestValue("value", "test");
+	}
+
+	@Test
+	void optionalObjectTestValue() {
+		this.service.executeOptional(Optional.of(Boolean.TRUE));
+		assertTestValue("value", "true");
+	}
+
+	@Test
+	void optionalEmpty() {
+		this.service.executeOptional(Optional.empty());
+		assertTestValue("value");
+	}
+
+	@Test
+	void optionalEmpthyWithDefaultValue() {
+		this.service.executeOptionalWithDefaultValue(Optional.empty());
+		assertTestValue("value", "default");
+	}
+
+	@Test
+	void mapOfTestValues() {
+		this.service.executeMap(Maps.of("value1", "true", "value2", "false"));
+		assertTestValue("value1", "true");
+		assertTestValue("value2", "false");
+	}
+
+	@Test
+	void mapOfTestValuesIsNull() {
+		assertThatIllegalArgumentException().isThrownBy(() -> this.service.executeMap(null));
+	}
+
+	@Test
+	void mapOfTestValuesHasOptionalValue() {
+		this.service.executeMapWithOptionalValue(Map.of("value", Optional.of("test")));
+		assertTestValue("value", "test");
+	}
+
+	private void assertTestValue(String key, String... values) {
+		List<String> actualValues = this.argumentResolver.getTestValues().get(key);
+		if (ObjectUtils.isEmpty(values)) {
+			assertThat(actualValues).isNull();
+		}
+		else {
+			assertThat(actualValues).containsOnly(values);
+		}
+	}
+
+
+	@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
+	private interface Service {
+
+		@GetExchange
+		void executeString(@TestValue String value);
+
+		@GetExchange
+		void execute(@TestValue Object value);
+
+		@GetExchange
+		void executeList(@TestValue List<Object> testValues);
+
+		@GetExchange
+		void executeArray(@TestValue Object... testValues);
+
+		@GetExchange
+		void executeNamed(@TestValue(name = "valueRenamed") String value);
+
+		@GetExchange
+		void executeNotRequired(@Nullable @TestValue(required = false) String value);
+
+		@GetExchange
+		void executeWithDefaultValue(@Nullable @TestValue(defaultValue = "default") String value);
+
+		@GetExchange
+		void executeOptional(@TestValue Optional<Object> value);
+
+		@GetExchange
+		void executeOptionalWithDefaultValue(@TestValue(defaultValue = "default") Optional<Object> value);
+
+		@GetExchange
+		void executeMap(@Nullable @TestValue Map<String, String> value);
+
+		@GetExchange
+		void executeMapWithOptionalValue(@TestValue Map<String, Optional<String>> values);
+
+	}
+
+
+	private static class TestNamedValueArgumentResolver extends AbstractNamedValueArgumentResolver {
+
+		private final MultiValueMap<String, String> testValues = new LinkedMultiValueMap<>();
+
+		TestNamedValueArgumentResolver() {
+			super(new DefaultFormattingConversionService());
+		}
+
+		public MultiValueMap<String, String> getTestValues() {
+			return this.testValues;
+		}
+
+		@Override
+		protected NamedValueInfo createNamedValueInfo(MethodParameter parameter) {
+			TestValue annot = parameter.getParameterAnnotation(TestValue.class);
+			return (annot == null ? null :
+					new NamedValueInfo(annot.name(), annot.required(), annot.defaultValue(), "test value", true));
+		}
+
+		@Override
+		protected void addRequestValue(String name, String value, HttpRequestValues.Builder requestValues) {
+			this.testValues.add(name, value);
+		}
+	}
+
+
+	@Target(ElementType.PARAMETER)
+	@Retention(RetentionPolicy.RUNTIME)
+	@Documented
+	public @interface TestValue {
+
+		@AliasFor("name")
+		String value() default "";
+
+		@AliasFor("value")
+		String name() default "";
+
+		boolean required() default true;
+
+		String defaultValue() default ValueConstants.DEFAULT_NONE;
+
+	}
+
+}

spring-web/src/test/java/org/springframework/web/service/invoker/PathVariableArgumentResolverTests.java

@@ -16,9 +16,6 @@
 
 package org.springframework.web.service.invoker;
 
-import java.util.Map;
-import java.util.Optional;
-
 import org.junit.jupiter.api.Test;
 
 import org.springframework.lang.Nullable;
@@ -26,128 +23,42 @@ import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.service.annotation.GetExchange;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 
 
 /**
  * Tests for {@link PathVariableArgumentResolver}.
+ * <p>For base class functionality, see {@link NamedValueArgumentResolverTests}.
  *
  * @author Olga Maciaszek-Sharma
+ * @author Rossen Stoyanchev
  */
 class PathVariableArgumentResolverTests {
 
-	private final TestHttpClientAdapter clientAdapter = new TestHttpClientAdapter();
+	private final TestHttpClientAdapter client = new TestHttpClientAdapter();
 
-	private final Service service = this.clientAdapter.createService(Service.class);
+	private final Service service = HttpServiceProxyFactory.builder(this.client).build().createClient(Service.class);
 
 
+	// Base class functionality should be tested in NamedValueArgumentResolverTests.
+
 	@Test
-	void stringVariable() {
+	void pathVariable() {
 		this.service.execute("test");
 		assertPathVariable("id", "test");
 	}
 
-	@Test
-	void objectVariable() {
-		this.service.execute(Boolean.TRUE);
-		assertPathVariable("id", "true");
-	}
-
-	@Test
-	void namedVariable() {
-		this.service.executeNamed("test");
-		assertPathVariable("id", "test");
-	}
-
-	@SuppressWarnings("ConstantConditions")
-	@Test
-	void nullVariableRequired() {
-		assertThatIllegalArgumentException().isThrownBy(() -> this.service.execute(null));
-	}
-
-	@Test
-	void nullVariableNotRequired() {
-		this.service.executeNotRequired(null);
-		assertPathVariable("id", null);
-	}
-
-	@Test
-	void optionalStringVariable() {
-		this.service.execute(Optional.of("test"));
-		assertPathVariable("id", "test");
-	}
-
-	@Test
-	void optionalObjectVariable() {
-		this.service.executeOptional(Optional.of(Boolean.TRUE));
-		assertPathVariable("id", "true");
-	}
-
-	@Test
-	void optionalEmpty() {
-		this.service.executeOptional(Optional.empty());
-		assertPathVariable("id", null);
-	}
-
-	@Test
-	void optionalEmptyOnObjectArgument() {
-		assertThatIllegalArgumentException().isThrownBy(() -> this.service.execute(Optional.empty()));
-	}
-
-	@Test
-	void mapOfVariables() {
-		this.service.executeMap(Map.of("id", "test"));
-		assertPathVariable("id", "test");
-	}
-
-	@Test
-	void mapOfVariablesIsNull() {
-		assertThatIllegalArgumentException().isThrownBy(() -> this.service.executeMap(null));
-	}
-
-	@Test
-	void mapOfVariablesHasOptionalValue() {
-		this.service.executeMapWithOptionalValue(Map.of("id", Optional.of("test")));
-		assertPathVariable("id", "test");
-	}
-
-
-	@Test
-	void mapOfVariablesHasOptionalEmpty() {
-		this.service.executeMapWithOptionalValue(Map.of("id", Optional.empty()));
-		assertPathVariable("id", null);
-	}
-
 	@SuppressWarnings("SameParameterValue")
 	private void assertPathVariable(String name, @Nullable String expectedValue) {
-		assertThat(this.clientAdapter.getRequestValues().getUriVariables().get(name))
+		assertThat(this.client.getRequestValues().getUriVariables().get(name))
 				.isEqualTo(expectedValue);
 	}
 
 
-	@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
 	private interface Service {
 
 		@GetExchange
 		void execute(@PathVariable String id);
 
-		@GetExchange
-		void execute(@PathVariable Object id);
-
-		@GetExchange
-		void executeNamed(@PathVariable(name = "id") String employeeId);
-
-		@GetExchange
-		void executeNotRequired(@Nullable @PathVariable(required = false) String id);
-
-		@GetExchange
-		void executeOptional(@PathVariable Optional<Boolean> id);
-
-		@GetExchange
-		void executeMap(@Nullable @PathVariable Map<String, String> map);
-
-		@GetExchange
-		void executeMapWithOptionalValue(@PathVariable Map<String, Optional<String>> map);
 	}
 
 }

spring-web/src/test/java/org/springframework/web/service/invoker/RequestHeaderArgumentResolverTests.java

@@ -17,125 +17,40 @@
 package org.springframework.web.service.invoker;
 
 import java.util.List;
-import java.util.Map;
-import java.util.Optional;
 
-import org.apache.groovy.util.Maps;
 import org.junit.jupiter.api.Test;
 
-import org.springframework.lang.Nullable;
 import org.springframework.util.ObjectUtils;
 import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.service.annotation.GetExchange;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 
 
 /**
  * Unit tests for {@link RequestHeaderArgumentResolver}.
+ * <p>For base class functionality, see {@link NamedValueArgumentResolverTests}.
  *
  * @author Olga Maciaszek-Sharma
+ * @author Rossen Stoyanchev
  */
 class RequestHeaderArgumentResolverTests {
 
-	private final TestHttpClientAdapter clientAdapter = new TestHttpClientAdapter();
+	private final TestHttpClientAdapter client = new TestHttpClientAdapter();
 
-	private final Service service = this.clientAdapter.createService(Service.class);
+	private final Service service = HttpServiceProxyFactory.builder(this.client).build().createClient(Service.class);
 
 
+	// Base class functionality should be tested in NamedValueArgumentResolverTests.
+
 	@Test
-	void stringHeader() {
-		this.service.executeString("test");
-		assertRequestHeaders("id", "test");
-	}
-
-	@Test
-	void objectHeader() {
-		this.service.execute(Boolean.TRUE);
-		assertRequestHeaders("id", "true");
-	}
-
-	@Test
-	void listHeader() {
-		this.service.executeList(List.of("test1", Boolean.TRUE, "test3"));
-		assertRequestHeaders("multiValueHeader", "test1", "true", "test3");
-	}
-
-	@Test
-	void arrayHeader() {
-		this.service.executeArray("test1", Boolean.FALSE, "test3");
-		assertRequestHeaders("multiValueHeader", "test1", "false", "test3");
-	}
-
-	@Test
-	void namedHeader() {
-		this.service.executeNamed("test");
-		assertRequestHeaders("id", "test");
-	}
-
-	@SuppressWarnings("ConstantConditions")
-	@Test
-	void nullHeaderRequired() {
-		assertThatIllegalArgumentException().isThrownBy(() -> this.service.executeString(null));
-	}
-
-	@Test
-	void nullHeaderNotRequired() {
-		this.service.executeNotRequired(null);
-		assertRequestHeaders("id");
-	}
-
-	@Test
-	void nullHeaderWithDefaultValue() {
-		this.service.executeWithDefaultValue(null);
-		assertRequestHeaders("id", "default");
-	}
-
-	@Test
-	void optionalStringHeader() {
-		this.service.executeOptional(Optional.of("test"));
-		assertRequestHeaders("id", "test");
-	}
-
-	@Test
-	void optionalObjectHeader() {
-		this.service.executeOptional(Optional.of(Boolean.TRUE));
-		assertRequestHeaders("id", "true");
-	}
-
-	@Test
-	void optionalEmpty() {
-		this.service.executeOptional(Optional.empty());
-		assertRequestHeaders("id");
-	}
-
-	@Test
-	void optionalEmpthyWithDefaultValue() {
-		this.service.executeOptionalWithDefaultValue(Optional.empty());
-		assertRequestHeaders("id", "default");
-	}
-
-	@Test
-	void mapOfHeaders() {
-		this.service.executeMap(Maps.of("header1", "true", "header2", "false"));
-		assertRequestHeaders("header1", "true");
-		assertRequestHeaders("header2", "false");
-	}
-
-	@Test
-	void mapOfHeadersIsNull() {
-		assertThatIllegalArgumentException().isThrownBy(() -> this.service.executeMap(null));
-	}
-
-	@Test
-	void mapOfHeadersHasOptionalValue() {
-		this.service.executeMapWithOptionalValue(Map.of("id", Optional.of("test")));
+	void header() {
+		this.service.execute("test");
 		assertRequestHeaders("id", "test");
 	}
 
 	private void assertRequestHeaders(String key, String... values) {
-		List<String> actualValues = this.clientAdapter.getRequestValues().getHeaders().get(key);
+		List<String> actualValues = this.client.getRequestValues().getHeaders().get(key);
 		if (ObjectUtils.isEmpty(values)) {
 			assertThat(actualValues).isNull();
 		}
@@ -145,41 +60,10 @@ class RequestHeaderArgumentResolverTests {
 	}
 
 
-	@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
 	private interface Service {
 
 		@GetExchange
-		void executeString(@RequestHeader String id);
-
-		@GetExchange
-		void execute(@RequestHeader Object id);
-
-		@GetExchange
-		void executeList(@RequestHeader List<Object> multiValueHeader);
-
-		@GetExchange
-		void executeArray(@RequestHeader Object... multiValueHeader);
-
-		@GetExchange
-		void executeNamed(@RequestHeader(name = "id") String employeeId);
-
-		@GetExchange
-		void executeNotRequired(@Nullable @RequestHeader(required = false) String id);
-
-		@GetExchange
-		void executeWithDefaultValue(@Nullable @RequestHeader(defaultValue = "default") String id);
-
-		@GetExchange
-		void executeOptional(@RequestHeader Optional<Object> id);
-
-		@GetExchange
-		void executeOptionalWithDefaultValue(@RequestHeader(defaultValue = "default") Optional<Object> id);
-
-		@GetExchange
-		void executeMap(@Nullable @RequestHeader Map<String, String> id);
-
-		@GetExchange
-		void executeMapWithOptionalValue(@RequestHeader Map<String, Optional<String>> headers);
+		void execute(@RequestHeader String id);
 
 	}
 

spring-web/src/test/java/org/springframework/web/service/invoker/RequestParamArgumentResolverTests.java

@@ -16,16 +16,10 @@
 
 package org.springframework.web.service.invoker;
 
-import java.net.URI;
-import java.util.Arrays;
-import java.util.List;
-
 import org.junit.jupiter.api.Test;
 
 import org.springframework.web.bind.annotation.RequestParam;
-import org.springframework.web.service.annotation.GetExchange;
 import org.springframework.web.service.annotation.PostExchange;
-import org.springframework.web.util.UriComponentsBuilder;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -34,71 +28,39 @@ import static org.assertj.core.api.Assertions.assertThat;
 /**
  * Unit tests for {@link RequestParamArgumentResolver}.
  *
+ * <p>Additional tests for this resolver:
+ * <ul>
+ * <li>Base class functionality in {@link NamedValueArgumentResolverTests}
+ * <li>Form data vs query params in {@link HttpRequestValuesTests}
+ * </ul>
+ *
  * @author Rossen Stoyanchev
  */
 public class RequestParamArgumentResolverTests {
 
-	private final TestHttpClientAdapter clientAdapter = new TestHttpClientAdapter();
+	private final TestHttpClientAdapter client = new TestHttpClientAdapter();
 
-	private final Service service = this.clientAdapter.createService(Service.class);
+	private final Service service = HttpServiceProxyFactory.builder(this.client).build().createClient(Service.class);
 
 
+	// Base class functionality should be tested in NamedValueArgumentResolverTests.
+	// Form data vs query params tested in HttpRequestValuesTests.
+
 	@Test
-	void formData() {
+	void requestParam() {
 		this.service.postForm("value 1", "value 2");
 
-		Object body = this.clientAdapter.getRequestValues().getBodyValue();
+		Object body = this.client.getRequestValues().getBodyValue();
 		assertThat(body).isNotNull().isInstanceOf(byte[].class);
 		assertThat(new String((byte[]) body, UTF_8)).isEqualTo("param1=value+1&param2=value+2");
 	}
 
-	@Test
-	void uriTemplate() {
-		this.service.search("1st value", Arrays.asList("2nd value A", "2nd value B"));
-
-		HttpRequestValues requestValues = this.clientAdapter.getRequestValues();
-
-		assertThat(requestValues.getUriTemplate())
-				.isEqualTo("/path?" +
-						"{queryParam0}={queryParam0[0]}&" +
-						"{queryParam1}={queryParam1[0]}&" +
-						"{queryParam1}={queryParam1[1]}");
-
-		assertThat(requestValues.getUriVariables())
-				.containsOnlyKeys("queryParam0", "queryParam1", "queryParam0[0]", "queryParam1[0]", "queryParam1[1]")
-				.containsEntry("queryParam0", "param1")
-				.containsEntry("queryParam1", "param2")
-				.containsEntry("queryParam0[0]", "1st value")
-				.containsEntry("queryParam1[0]", "2nd value A")
-				.containsEntry("queryParam1[1]", "2nd value B");
-
-		URI uri = UriComponentsBuilder.fromUriString(requestValues.getUriTemplate())
-				.encode().build(requestValues.getUriVariables());
-
-		assertThat(uri.toString())
-				.isEqualTo("/path?param1=1st%20value&param2=2nd%20value%20A&param2=2nd%20value%20B");
-	}
-
-	@Test
-	void uri() {
-		URI baseUrl = URI.create("http://localhost:8080/path");
-		this.service.searchWithDynamicUri(baseUrl, "1st value", Arrays.asList("2nd value A", "2nd value B"));
-
-		assertThat(this.clientAdapter.getRequestValues().getUri().toString())
-				.isEqualTo(baseUrl + "?param1=1st%20value&param2=2nd%20value%20A&param2=2nd%20value%20B");
-	}
-
 
 	private interface Service {
 
 		@PostExchange(contentType = "application/x-www-form-urlencoded")
 		void postForm(@RequestParam String param1, @RequestParam String param2);
 
-		@GetExchange("/path")
-		void search(@RequestParam String param1, @RequestParam List<String> param2);
-
-		@GetExchange
-		void searchWithDynamicUri(URI uri, @RequestParam String param1, @RequestParam List<String> param2);
 	}
 
 }

spring-web/src/test/java/org/springframework/web/service/invoker/TestHttpClientAdapter.java

@@ -46,15 +46,6 @@ class TestHttpClientAdapter implements HttpClientAdapter {
 	private ParameterizedTypeReference<?> bodyType;
 
 
-	/**
-	 * Create the proxy for the give service type.
-	 */
-	public <S> S createService(Class<S> serviceType) {
-		HttpServiceProxyFactory factory = HttpServiceProxyFactory.builder(this).build();
-		return factory.createClient(serviceType);
-	}
-
-
 	public String getInvokedMethodName() {
 		assertThat(this.invokedMethodName).isNotNull();
 		return this.invokedMethodName;