Refactor HttpServiceProxyFactory for use as a bean

Closes gh-28505

Author: rstoyanchev

spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceProxyFactory.java

@@ -28,6 +28,8 @@
 import org.aopalliance.intercept.MethodInvocation;
 
 import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.beans.factory.InitializingBean;
+import org.springframework.context.EmbeddedValueResolverAware;
 import org.springframework.core.MethodIntrospector;
 import org.springframework.core.ReactiveAdapterRegistry;
 import org.springframework.core.annotation.AnnotatedElementUtils;
@@ -42,189 +44,161 @@
  * Factory for creating a client proxy given an HTTP service interface with
  * {@link HttpExchange @HttpExchange} methods.
  *
+ * <p>This class is intended to be declared as a bean in a Spring configuration.
+ *
  * @author Rossen Stoyanchev
  * @since 6.0
  */
-public final class HttpServiceProxyFactory {
+public final class HttpServiceProxyFactory implements InitializingBean, EmbeddedValueResolverAware {
 
 	private final HttpClientAdapter clientAdapter;
 
-	private final List<HttpServiceArgumentResolver> argumentResolvers;
+	@Nullable
+	private List<HttpServiceArgumentResolver> customArgumentResolvers;
+
+	@Nullable
+	private List<HttpServiceArgumentResolver> argumentResolvers;
 
 	@Nullable
-	private final StringValueResolver embeddedValueResolver;
+	private ConversionService conversionService;
 
-	private final ReactiveAdapterRegistry reactiveAdapterRegistry;
+	@Nullable
+	private StringValueResolver embeddedValueResolver;
 
-	private final Duration blockTimeout;
+	private ReactiveAdapterRegistry reactiveAdapterRegistry = ReactiveAdapterRegistry.getSharedInstance();
 
+	private Duration blockTimeout = Duration.ofSeconds(5);
 
-	private HttpServiceProxyFactory(
-			HttpClientAdapter clientAdapter, List<HttpServiceArgumentResolver> argumentResolvers,
-			@Nullable StringValueResolver embeddedValueResolver, ReactiveAdapterRegistry reactiveAdapterRegistry,
-			Duration blockTimeout) {
 
+	/**
+	 * Create an instance with the underlying HTTP client to use.
+	 * @param clientAdapter an adapter for the client
+	 */
+	public HttpServiceProxyFactory(HttpClientAdapter clientAdapter) {
+		Assert.notNull(clientAdapter, "HttpClientAdapter is required");
 		this.clientAdapter = clientAdapter;
-		this.argumentResolvers = argumentResolvers;
-		this.embeddedValueResolver = embeddedValueResolver;
-		this.reactiveAdapterRegistry = reactiveAdapterRegistry;
-		this.blockTimeout = blockTimeout;
 	}
 
 
 	/**
-	 * Return a proxy that implements the given HTTP service interface to perform
-	 * HTTP requests and retrieve responses through an HTTP client.
-	 * @param serviceType the HTTP service to create a proxy for
-	 * @param <S> the HTTP service type
-	 * @return the created proxy
+	 * Register a custom argument resolver, invoked ahead of default resolvers.
+	 * @param resolver the resolver to add
 	 */
-	public <S> S createClient(Class<S> serviceType) {
-
-		List<HttpServiceMethod> methods =
-				MethodIntrospector.selectMethods(serviceType, this::isExchangeMethod)
-						.stream()
-						.map(method ->
-								new HttpServiceMethod(
-										method, serviceType, this.argumentResolvers, this.clientAdapter,
-										this.embeddedValueResolver, this.reactiveAdapterRegistry, this.blockTimeout))
-						.toList();
-
-		return ProxyFactory.getProxy(serviceType, new HttpServiceMethodInterceptor(methods));
+	public void addCustomArgumentResolver(HttpServiceArgumentResolver resolver) {
+		if (this.customArgumentResolvers == null) {
+			this.customArgumentResolvers = new ArrayList<>();
+		}
+		this.customArgumentResolvers.add(resolver);
 	}
 
-	private boolean isExchangeMethod(Method method) {
-		return AnnotatedElementUtils.hasAnnotation(method, HttpExchange.class);
+	/**
+	 * Set the custom argument resolvers to use, ahead of default resolvers.
+	 * @param resolvers the resolvers to use
+	 */
+	public void setCustomArgumentResolvers(List<HttpServiceArgumentResolver> resolvers) {
+		this.customArgumentResolvers = new ArrayList<>(resolvers);
 	}
 
-
 	/**
-	 * Return a builder for an {@link HttpServiceProxyFactory}.
-	 * @param adapter an adapter for the underlying HTTP client
-	 * @return the builder
+	 * Set the {@link ConversionService} to use where input values need to
+	 * be formatted as Strings.
+	 * <p>By default this is {@link DefaultFormattingConversionService}.
 	 */
-	public static Builder builder(HttpClientAdapter adapter) {
-		return new Builder(adapter);
+	public void setConversionService(ConversionService conversionService) {
+		this.conversionService = conversionService;
 	}
 
-
 	/**
-	 * Builder for {@link HttpServiceProxyFactory}.
+	 * Set the StringValueResolver to use for resolving placeholders and
+	 * expressions in {@link HttpExchange#url()}.
+	 * @param resolver the resolver to use
 	 */
-	public final static class Builder {
-
-		private final HttpClientAdapter clientAdapter;
-
-		private final List<HttpServiceArgumentResolver> customResolvers = new ArrayList<>();
-
-		@Nullable
-		private ConversionService conversionService;
-
-		@Nullable
-		private StringValueResolver embeddedValueResolver;
+	@Override
+	public void setEmbeddedValueResolver(StringValueResolver resolver) {
+		this.embeddedValueResolver = resolver;
+	}
 
-		private ReactiveAdapterRegistry reactiveAdapterRegistry = ReactiveAdapterRegistry.getSharedInstance();
+	/**
+	 * Set the {@link ReactiveAdapterRegistry} to use to support different
+	 * asynchronous types for HTTP service method return values.
+	 * <p>By default this is {@link ReactiveAdapterRegistry#getSharedInstance()}.
+	 */
+	public void setReactiveAdapterRegistry(ReactiveAdapterRegistry registry) {
+		this.reactiveAdapterRegistry = registry;
+	}
 
-		private Duration blockTimeout = Duration.ofSeconds(5);
+	/**
+	 * Configure how long to wait for a response for an HTTP service method
+	 * with a synchronous (blocking) method signature.
+	 * <p>By default this is 5 seconds.
+	 * @param blockTimeout the timeout value
+	 */
+	public void setBlockTimeout(Duration blockTimeout) {
+		this.blockTimeout = blockTimeout;
+	}
 
-		private Builder(HttpClientAdapter clientAdapter) {
-			Assert.notNull(clientAdapter, "HttpClientAdapter is required");
-			this.clientAdapter = clientAdapter;
-		}
 
-		/**
-		 * Register a custom argument resolver. This will be inserted ahead of
-		 * default resolvers.
-		 * @return the same builder instance
-		 */
-		public Builder addCustomResolver(HttpServiceArgumentResolver resolver) {
-			this.customResolvers.add(resolver);
-			return this;
-		}
+	@Override
+	public void afterPropertiesSet() throws Exception {
 
-		/**
-		 * Set the {@link ConversionService} to use where input values need to
-		 * be formatted as Strings.
-		 * <p>By default this is {@link DefaultFormattingConversionService}.
-		 * @return the same builder instance
-		 */
-		public Builder setConversionService(ConversionService conversionService) {
-			this.conversionService = conversionService;
-			return this;
-		}
+		this.conversionService = (this.conversionService != null ?
+				this.conversionService : new DefaultFormattingConversionService());
 
-		/**
-		 * Set the StringValueResolver to use for resolving placeholders and
-		 * expressions in {@link HttpExchange#url()}.
-		 * @param embeddedValueResolver the resolver to use
-		 * @return the same builder instance
-		 * @see org.springframework.context.EmbeddedValueResolverAware
-		 */
-		public Builder setEmbeddedValueResolver(@Nullable StringValueResolver embeddedValueResolver) {
-			this.embeddedValueResolver = embeddedValueResolver;
-			return this;
-		}
+		this.argumentResolvers = initArgumentResolvers(this.conversionService);
+	}
 
-		/**
-		 * Set the {@link ReactiveAdapterRegistry} to use to support different
-		 * asynchronous types for HTTP service method return values.
-		 * <p>By default this is {@link ReactiveAdapterRegistry#getSharedInstance()}.
-		 * @return the same builder instance
-		 */
-		public Builder setReactiveAdapterRegistry(ReactiveAdapterRegistry registry) {
-			this.reactiveAdapterRegistry = registry;
-			return this;
-		}
+	private List<HttpServiceArgumentResolver> initArgumentResolvers(ConversionService conversionService) {
+		List<HttpServiceArgumentResolver> resolvers = new ArrayList<>();
 
-		/**
-		 * Configure how long to wait for a response for an HTTP service method
-		 * with a synchronous (blocking) method signature.
-		 * <p>By default this is 5 seconds.
-		 * @param blockTimeout the timeout value
-		 * @return the same builder instance
-		 */
-		public Builder setBlockTimeout(Duration blockTimeout) {
-			this.blockTimeout = blockTimeout;
-			return this;
+		// Custom
+		if (this.customArgumentResolvers != null) {
+			resolvers.addAll(this.customArgumentResolvers);
 		}
 
-		/**
-		 * Build and return the {@link HttpServiceProxyFactory} instance.
-		 */
-		public HttpServiceProxyFactory build() {
+		// Annotation-based
+		resolvers.add(new RequestHeaderArgumentResolver(conversionService));
+		resolvers.add(new RequestBodyArgumentResolver(this.reactiveAdapterRegistry));
+		resolvers.add(new PathVariableArgumentResolver(conversionService));
+		resolvers.add(new RequestParamArgumentResolver(conversionService));
+		resolvers.add(new CookieValueArgumentResolver(conversionService));
+		resolvers.add(new RequestAttributeArgumentResolver());
 
-			ConversionService conversionService = initConversionService();
-			List<HttpServiceArgumentResolver> resolvers = initArgumentResolvers(conversionService);
+		// Specific type
+		resolvers.add(new UrlArgumentResolver());
+		resolvers.add(new HttpMethodArgumentResolver());
 
-			return new HttpServiceProxyFactory(
-					this.clientAdapter, resolvers, this.embeddedValueResolver, this.reactiveAdapterRegistry,
-					this.blockTimeout);
-		}
+		return resolvers;
+	}
 
-		private ConversionService initConversionService() {
-			return (this.conversionService != null ?
-					this.conversionService : new DefaultFormattingConversionService());
-		}
 
-		private List<HttpServiceArgumentResolver> initArgumentResolvers(ConversionService conversionService) {
+	/**
+	 * Return a proxy that implements the given HTTP service interface to perform
+	 * HTTP requests and retrieve responses through an HTTP client.
+	 * @param serviceType the HTTP service to create a proxy for
+	 * @param <S> the HTTP service type
+	 * @return the created proxy
+	 */
+	public <S> S createClient(Class<S> serviceType) {
 
-			List<HttpServiceArgumentResolver> resolvers = new ArrayList<>(this.customResolvers);
+		List<HttpServiceMethod> httpServiceMethods =
+				MethodIntrospector.selectMethods(serviceType, this::isExchangeMethod).stream()
+						.map(method -> createHttpServiceMethod(serviceType, method))
+						.toList();
 
-			// Annotation-based
-			resolvers.add(new RequestHeaderArgumentResolver(conversionService));
-			resolvers.add(new RequestBodyArgumentResolver(this.reactiveAdapterRegistry));
-			resolvers.add(new PathVariableArgumentResolver(conversionService));
-			resolvers.add(new RequestParamArgumentResolver(conversionService));
-			resolvers.add(new CookieValueArgumentResolver(conversionService));
-			resolvers.add(new RequestAttributeArgumentResolver());
+		return ProxyFactory.getProxy(serviceType, new HttpServiceMethodInterceptor(httpServiceMethods));
+	}
 
-			// Specific type
-			resolvers.add(new UrlArgumentResolver());
-			resolvers.add(new HttpMethodArgumentResolver());
+	private boolean isExchangeMethod(Method method) {
+		return AnnotatedElementUtils.hasAnnotation(method, HttpExchange.class);
+	}
 
-			return resolvers;
-		}
+	private <S> HttpServiceMethod createHttpServiceMethod(Class<S> serviceType, Method method) {
+		Assert.notNull(this.argumentResolvers,
+				"No argument resolvers: afterPropertiesSet was not called");
 
+		return new HttpServiceMethod(
+				method, serviceType, this.argumentResolvers, this.clientAdapter,
+				this.embeddedValueResolver, this.reactiveAdapterRegistry, this.blockTimeout);
 	}
 
 

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

@@ -18,6 +18,7 @@
 
 import java.util.List;
 
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.util.ObjectUtils;
@@ -36,7 +37,15 @@ class CookieValueArgumentResolverTests {
 
 	private final TestHttpClientAdapter client = new TestHttpClientAdapter();
 
-	private final Service service = HttpServiceProxyFactory.builder(this.client).build().createClient(Service.class);
+	private Service service;
+
+
+	@BeforeEach
+	void setUp() throws Exception {
+		HttpServiceProxyFactory proxyFactory = new HttpServiceProxyFactory(this.client);
+		proxyFactory.afterPropertiesSet();
+		this.service = proxyFactory.createClient(Service.class);
+	}
 
 
 	// Base class functionality should be tested in NamedValueArgumentResolverTests.

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

@@ -16,6 +16,7 @@
 
 package org.springframework.web.service.invoker;
 
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.http.HttpMethod;
@@ -34,7 +35,15 @@ public class HttpMethodArgumentResolverTests {
 
 	private final TestHttpClientAdapter client = new TestHttpClientAdapter();
 
-	private final Service service = HttpServiceProxyFactory.builder(this.client).build().createClient(Service.class);
+	private Service service;
+
+
+	@BeforeEach
+	void setUp() throws Exception {
+		HttpServiceProxyFactory proxyFactory = new HttpServiceProxyFactory(this.client);
+		proxyFactory.afterPropertiesSet();
+		this.service = proxyFactory.createClient(Service.class);
+	}
 
 
 	@Test