#118 - Use MVC ConversionService for parameter binding in link creation.

We now look up the ConversionService available in the ApplicationContext from Web(Mvc|Flux)LinkBuilder. Some API tweaks to WebHandler to allow the lookup from the current request. The general fallback is now the invocation of …toString() on the parameter value. Fixes #118, #352, #144, #149.
2021-01-25 19:15:26 +01:00
parent 3da23a7d76
commit d0ff83c62e
5 changed files with 195 additions and 45 deletions
--- a/src/main/java/org/springframework/hateoas/server/core/WebHandler.java
+++ b/src/main/java/org/springframework/hateoas/server/core/WebHandler.java
@@ -26,12 +26,12 @@ import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.BiFunction;
 import java.util.function.Function;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;

 import org.springframework.core.MethodParameter;
 import org.springframework.core.convert.ConversionService;
 import org.springframework.core.convert.TypeDescriptor;
-import org.springframework.format.support.DefaultFormattingConversionService;
 import org.springframework.hateoas.Affordance;
 import org.springframework.hateoas.TemplateVariable;
 import org.springframework.hateoas.TemplateVariables;
@@ -71,7 +71,7 @@ public class WebHandler {
 	}

 	public interface PreparedWebHandler<T extends LinkBuilder> {
-		T conclude(Function<String, UriComponentsBuilder> finisher);
+		T conclude(Function<String, UriComponentsBuilder> finisher, ConversionService conversionService);
 	}

 	public static <T extends LinkBuilder> PreparedWebHandler<T> linkTo(Object invocationValue,
@@ -82,9 +82,9 @@ public class WebHandler {

 	public static <T extends LinkBuilder> T linkTo(Object invocationValue, LinkBuilderCreator<T> creator,
 			@Nullable BiFunction<UriComponentsBuilder, MethodInvocation, UriComponentsBuilder> additionalUriHandler,
-			Function<String, UriComponentsBuilder> finisher) {
+			Function<String, UriComponentsBuilder> finisher, Supplier<ConversionService> conversionService) {

-		return linkTo(invocationValue, creator, additionalUriHandler).conclude(finisher);
+		return linkTo(invocationValue, creator, additionalUriHandler).conclude(finisher, conversionService.get());
 	}

 	private static <T extends LinkBuilder> PreparedWebHandler<T> linkTo(Object invocationValue,
@@ -104,7 +104,7 @@ public class WebHandler {

 		String mapping = DISCOVERER.getMapping(invocation.getTargetType(), invocation.getMethod());

-		return finisher -> {
+		return (finisher, conversionService) -> {

 			UriComponentsBuilder builder = finisher.apply(mapping);
 			UriTemplate template = UriTemplateFactory.templateFor(mapping == null ? "/" : mapping);
@@ -120,16 +120,18 @@ public class WebHandler {

 			HandlerMethodParameters parameters = HandlerMethodParameters.of(invocation.getMethod());
 			Object[] arguments = invocation.getArguments();
+			ConversionService resolved = conversionService;

 			for (HandlerMethodParameter parameter : parameters.getParameterAnnotatedWith(PathVariable.class, arguments)) {
-				values.put(parameter.getVariableName(), encodePath(parameter.getValueAsString(arguments)));
+				values.put(parameter.getVariableName(),
+						encodePath(parameter.getValueAsString(arguments, resolved)));
 			}

 			List<String> optionalEmptyParameters = new ArrayList<>();

 			for (HandlerMethodParameter parameter : parameters.getParameterAnnotatedWith(RequestParam.class, arguments)) {

-				bindRequestParameters(builder, parameter, arguments);
+				bindRequestParameters(builder, parameter, arguments, conversionService);

 				if (SKIP_VALUE.equals(parameter.getVerifiedValue(arguments))) {

@@ -177,7 +179,7 @@ public class WebHandler {
 	 */
 	@SuppressWarnings("unchecked")
 	private static void bindRequestParameters(UriComponentsBuilder builder, HandlerMethodParameter parameter,
-			Object[] arguments) {
+			Object[] arguments, ConversionService conversionService) {

 		Object value = parameter.getVerifiedValue(arguments);

@@ -223,7 +225,7 @@ public class WebHandler {

 		} else {
 			if (key != null) {
-				builder.queryParam(key, encodeParameter(parameter.getValueAsString(arguments)));
+				builder.queryParam(key, encodeParameter(parameter.getValueAsString(arguments, conversionService)));
 			}
 		}
 	}
@@ -335,7 +337,6 @@ public class WebHandler {

 	private abstract static class HandlerMethodParameter {

-		private static final ConversionService CONVERSION_SERVICE = new DefaultFormattingConversionService();
 		private static final TypeDescriptor STRING_DESCRIPTOR = TypeDescriptor.valueOf(String.class);
 		private static final Map<Class<? extends Annotation>, Function<MethodParameter, ? extends HandlerMethodParameter>> FACTORY;
 		private static final String NO_PARAMETER_NAME = "Could not determine name of parameter %s! Make sure you compile with parameter information or explicitly define a parameter name in %s.";
@@ -400,7 +401,7 @@ public class WebHandler {
 			return variableName;
 		}

-		public String getValueAsString(Object[] values) {
+		public String getValueAsString(Object[] values, ConversionService conversionService) {

 			Object value = values[parameter.getParameterIndex()];

@@ -414,11 +415,9 @@ public class WebHandler {

 			value = ObjectUtils.unwrapOptional(value);

-			// Try to lookup ConversionService from the request's context
-
-			// Guard with ….canConvert(…)
-			// if not, fall back to ….toString();
-			Object result = CONVERSION_SERVICE.convert(value, typeDescriptor, STRING_DESCRIPTOR);
+			Object result = conversionService.canConvert(typeDescriptor, STRING_DESCRIPTOR)
+					? conversionService.convert(value, typeDescriptor, STRING_DESCRIPTOR)
+					: value == null ? null : value.toString();

 			if (result == null) {
 				throw new IllegalArgumentException(String.format("Conversion of value %s resulted in null!", value));
--- a/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderFactory.java
+++ b/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderFactory.java
@@ -23,13 +23,23 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
+import java.util.function.Supplier;
+
+import javax.servlet.ServletContext;

 import org.springframework.core.MethodParameter;
+import org.springframework.core.convert.ConversionService;
+import org.springframework.format.support.DefaultFormattingConversionService;
 import org.springframework.hateoas.Link;
 import org.springframework.hateoas.server.MethodLinkBuilderFactory;
 import org.springframework.hateoas.server.core.LinkBuilderSupport;
 import org.springframework.hateoas.server.core.MethodParameters;
 import org.springframework.hateoas.server.core.WebHandler;
+import org.springframework.web.context.WebApplicationContext;
+import org.springframework.web.context.request.RequestAttributes;
+import org.springframework.web.context.request.RequestContextHolder;
+import org.springframework.web.context.request.ServletRequestAttributes;
+import org.springframework.web.context.support.WebApplicationContextUtils;
 import org.springframework.web.util.UriComponentsBuilder;

 /**
@@ -47,6 +57,8 @@ import org.springframework.web.util.UriComponentsBuilder;
 */
 public class WebMvcLinkBuilderFactory implements MethodLinkBuilderFactory<WebMvcLinkBuilder> {

+	private static ConversionService FALLBACK_CONVERSION_SERVICE = new DefaultFormattingConversionService();
+
 	private List<UriComponentsContributor> uriComponentsContributors = new ArrayList<>();

 	/**
@@ -124,7 +136,7 @@ public class WebMvcLinkBuilderFactory implements MethodLinkBuilderFactory<WebMvc

 			return builder;

-		}, builderFactory);
+		}, builderFactory, getConversionService());
 	}

 	/*
@@ -135,4 +147,24 @@ public class WebMvcLinkBuilderFactory implements MethodLinkBuilderFactory<WebMvc
 	public WebMvcLinkBuilder linkTo(Method method, Object... parameters) {
 		return WebMvcLinkBuilder.linkTo(method, parameters);
 	}
+
+	@SuppressWarnings("null")
+	private Supplier<ConversionService> getConversionService() {
+
+		return () -> {
+
+			RequestAttributes attributes = RequestContextHolder.getRequestAttributes();
+
+			if (!ServletRequestAttributes.class.isInstance(attributes)) {
+				return null;
+			}
+
+			ServletContext servletContext = ((ServletRequestAttributes) attributes).getRequest().getServletContext();
+			WebApplicationContext context = WebApplicationContextUtils.getWebApplicationContext(servletContext);
+
+			return context == null || !context.containsBean("mvcConversionService")
+					? FALLBACK_CONVERSION_SERVICE
+					: context.getBean("mvcConversionService", ConversionService.class);
+		};
+	}
 }
--- a/src/main/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilder.java
+++ b/src/main/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilder.java
@@ -22,6 +22,9 @@ import reactor.core.publisher.Mono;
 import java.util.List;
 import java.util.function.Function;

+import org.springframework.context.ApplicationContext;
+import org.springframework.core.convert.ConversionService;
+import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.hateoas.Affordance;
 import org.springframework.hateoas.IanaLinkRelations;
 import org.springframework.hateoas.Link;
@@ -75,7 +78,7 @@ public class WebFluxLinkBuilder extends TemplateVariableAwareLinkBuilderSupport<
 	 * @param exchange must not be {@literal null}.
 	 */
 	public static WebFluxBuilder linkTo(Object invocation, ServerWebExchange exchange) {
-		return new WebFluxBuilder(linkToInternal(invocation, Mono.just(getBuilder(exchange))));
+		return new WebFluxBuilder(linkToInternal(invocation, CurrentRequest.of(exchange)));
 	}

 	/**
@@ -87,7 +90,6 @@ public class WebFluxLinkBuilder extends TemplateVariableAwareLinkBuilderSupport<
 	 * @return
 	 */
 	public static <T> T methodOn(Class<T> controller, Object... parameters) {
-
 		return DummyInvocationUtils.methodOn(controller, parameters);
 	}

@@ -245,40 +247,58 @@ public class WebFluxLinkBuilder extends TemplateVariableAwareLinkBuilderSupport<
 		}
 	}

-	/**
-	 * Returns a {@link UriComponentsBuilder} obtained from the {@link ServerWebExchange}.
-	 *
-	 * @param exchange
-	 */
-	private static UriComponentsBuilder getBuilder(@Nullable ServerWebExchange exchange) {
-
-		if (exchange == null) {
-			return UriComponentsBuilder.fromPath("/");
-		}
-
-		ServerHttpRequest request = exchange.getRequest();
-		PathContainer contextPath = request.getPath().contextPath();
-
-		return UriComponentsBuilder.fromHttpRequest(request) //
-				.replacePath(contextPath.toString()) //
-				.replaceQuery("");
-	}
-
 	private static Mono<WebFluxLinkBuilder> linkToInternal(Object invocation) {

 		return linkToInternal(invocation,
-				Mono.subscriberContext().map(context -> getBuilder(context.getOrDefault(EXCHANGE_CONTEXT_ATTRIBUTE, null))));
+				Mono.deferContextual(
+						context -> CurrentRequest.of(context.getOrDefault(EXCHANGE_CONTEXT_ATTRIBUTE, null))));
 	}

-	private static Mono<WebFluxLinkBuilder> linkToInternal(Object invocation, Mono<UriComponentsBuilder> builder) {
+	private static Mono<WebFluxLinkBuilder> linkToInternal(Object invocation, Mono<CurrentRequest> builder) {

 		PreparedWebHandler<WebFluxLinkBuilder> handler = WebHandler.linkTo(invocation, WebFluxLinkBuilder::new);

-		return builder.map(WebFluxLinkBuilder::getBuilderCreator) //
-				.map(handler::conclude);
+		return builder.map(it -> handler.conclude(path -> it.builder.path(path), it.conversionService));
 	}

-	private static Function<String, UriComponentsBuilder> getBuilderCreator(UriComponentsBuilder builder) {
-		return path -> builder.path(path);
+	/**
+	 * Access to components we can obtain from the current request or fallbacks in case no current request is available.
+	 *
+	 * @author Oliver Drotbohm
+	 */
+	private static class CurrentRequest {
+
+		private static final ConversionService FALLBACK_CONVERSION_SERVICE = new DefaultConversionService();
+
+		private UriComponentsBuilder builder;
+		private ConversionService conversionService;
+
+		public static Mono<CurrentRequest> of(@Nullable ServerWebExchange exchange) {
+
+			CurrentRequest result = new CurrentRequest();
+
+			if (exchange == null) {
+
+				result.builder = UriComponentsBuilder.fromPath("/");
+				result.conversionService = FALLBACK_CONVERSION_SERVICE;
+
+				return Mono.just(result);
+			}
+
+			ServerHttpRequest request = exchange.getRequest();
+			PathContainer contextPath = request.getPath().contextPath();
+
+			result.builder = UriComponentsBuilder.fromHttpRequest(request) //
+					.replacePath(contextPath.toString()) //
+					.replaceQuery("");
+
+			ApplicationContext context = exchange.getApplicationContext();
+
+			result.conversionService = context != null && context.containsBean("webFluxConversionService")
+					? context.getBean("webFluxConversionService", ConversionService.class)
+					: FALLBACK_CONVERSION_SERVICE;
+
+			return Mono.just(result);
+		}
 	}
 }
--- a/src/test/java/org/springframework/hateoas/config/HypermediaWebFluxConfigurerTest.java
+++ b/src/test/java/org/springframework/hateoas/config/HypermediaWebFluxConfigurerTest.java
@@ -18,6 +18,7 @@ package org.springframework.hateoas.config;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
 import static org.springframework.hateoas.config.EnableHypermediaSupport.HypermediaType.*;
+import static org.springframework.hateoas.server.reactive.WebFluxLinkBuilder.*;

 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
@@ -31,6 +32,7 @@ import org.junit.jupiter.api.Test;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.format.FormatterRegistry;
 import org.springframework.hateoas.CollectionModel;
 import org.springframework.hateoas.EntityModel;
 import org.springframework.hateoas.IanaLinkRelations;
@@ -41,7 +43,11 @@ import org.springframework.hateoas.server.SimpleRepresentationModelAssembler;
 import org.springframework.hateoas.server.core.TypeReferences.CollectionModelType;
 import org.springframework.hateoas.server.core.TypeReferences.EntityModelType;
 import org.springframework.hateoas.support.Employee;
+import org.springframework.http.HttpEntity;
+import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
+import org.springframework.http.ResponseEntity;
+import org.springframework.stereotype.Controller;
 import org.springframework.test.web.reactive.server.WebTestClient;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
@@ -50,6 +56,7 @@ import org.springframework.web.bind.annotation.PutMapping;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.reactive.config.EnableWebFlux;
+import org.springframework.web.reactive.config.WebFluxConfigurer;

 /**
 * @author Greg Turnquist
@@ -67,7 +74,7 @@ class HypermediaWebFluxConfigurerTest {
 		ctx.register(context);
 		ctx.refresh();

-        HypermediaWebTestClientConfigurer configurer = ctx.getBean(HypermediaWebTestClientConfigurer.class);
+		HypermediaWebTestClientConfigurer configurer = ctx.getBean(HypermediaWebTestClientConfigurer.class);

 		this.testClient = WebTestClient.bindToApplicationContext(ctx).build().mutateWith(configurer);
 	}
@@ -322,6 +329,24 @@ class HypermediaWebFluxConfigurerTest {
 				}).verifyComplete();
 	}

+	@Test // #118
+	void linkCreationConsidersRegisteredConverters() throws Exception {
+
+		setUp(WithConversionService.class);
+
+		this.testClient.get().uri("/sample/4711").exchange() //
+				.expectStatus().isEqualTo(HttpStatus.I_AM_A_TEAPOT) //
+				.returnResult(String.class).getResponseBody()
+				.as(StepVerifier::create)
+				.expectNextMatches(it -> {
+
+					assertThat(it).isEqualTo("/sample/sample");
+
+					return true;
+				})
+				.verifyComplete();
+	}
+
 	private void verifyRootUriServesHypermedia(MediaType mediaType) {
 		verifyRootUriServesHypermedia(mediaType, mediaType);
 	}
@@ -555,4 +580,33 @@ class HypermediaWebFluxConfigurerTest {
 		}
 	}

+	// #118
+
+	@Configuration
+	static class WithConversionService extends HalWebFluxConfig implements WebFluxConfigurer {
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.web.servlet.config.annotation.WebMvcConfigurer#addFormatters(org.springframework.format.FormatterRegistry)
+		 */
+		@Override
+		public void addFormatters(FormatterRegistry registry) {
+			registry.addConverter(Sample.class, String.class, source -> "sample");
+			registry.addConverter(String.class, Sample.class, source -> new Sample());
+		}
+
+		static class Sample {}
+
+		@Controller
+		static class SampleController {
+
+			@GetMapping("/sample/{sample}")
+			Mono<HttpEntity<?>> sample(@PathVariable Sample sample) {
+
+				return linkTo(methodOn(SampleController.class).sample(new Sample())).withSelfRel()
+						.toMono()
+						.map(it -> new ResponseEntity<>(it.getHref(), HttpStatus.I_AM_A_TEAPOT));
+			}
+		}
+	}
 }
--- a/src/test/java/org/springframework/hateoas/config/HypermediaWebMvcConfigurerTest.java
+++ b/src/test/java/org/springframework/hateoas/config/HypermediaWebMvcConfigurerTest.java
@@ -18,6 +18,7 @@ package org.springframework.hateoas.config;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
 import static org.springframework.hateoas.config.EnableHypermediaSupport.HypermediaType.*;
+import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.*;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
 import static org.springframework.test.web.servlet.setup.MockMvcBuilders.*;
@@ -29,6 +30,7 @@ import java.util.List;
 import org.junit.jupiter.api.Test;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.format.FormatterRegistry;
 import org.springframework.hateoas.CollectionModel;
 import org.springframework.hateoas.EntityModel;
 import org.springframework.hateoas.IanaLinkRelations;
@@ -41,9 +43,13 @@ import org.springframework.hateoas.mediatype.hal.forms.Jackson2HalFormsModule;
 import org.springframework.hateoas.mediatype.uber.Jackson2UberModule;
 import org.springframework.hateoas.server.SimpleRepresentationModelAssembler;
 import org.springframework.hateoas.support.Employee;
+import org.springframework.http.HttpEntity;
 import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
+import org.springframework.http.ResponseEntity;
 import org.springframework.mock.web.MockServletContext;
+import org.springframework.stereotype.Controller;
 import org.springframework.test.context.web.WebAppConfiguration;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.web.bind.annotation.GetMapping;
@@ -54,6 +60,7 @@ import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
 import org.springframework.web.servlet.config.annotation.EnableWebMvc;
+import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;

 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.JavaType;
@@ -256,6 +263,15 @@ class HypermediaWebMvcConfigurerTest {
 				.isEqualTo("{\"links\":[{\"rel\":\"self\",\"href\":\"/\"},{\"rel\":\"employees\",\"href\":\"/employees\"}]}");
 	}

+	@Test // #118
+	void linkCreationConsidersRegisteredConverters() throws Exception {
+
+		setUp(WithConversionService.class);
+
+		this.mockMvc.perform(get("/sample/4711"))
+				.andExpect(status().isIAmATeapot());
+	}
+
 	private void verifyRootUriServesHypermedia(MediaType mediaType) throws Exception {
 		verifyRootUriServesHypermedia(mediaType, mediaType);
 	}
@@ -478,4 +494,33 @@ class HypermediaWebMvcConfigurerTest {
 		}
 	}

+	// #118
+
+	@Configuration
+	static class WithConversionService extends BaseConfig implements WebMvcConfigurer {
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.web.servlet.config.annotation.WebMvcConfigurer#addFormatters(org.springframework.format.FormatterRegistry)
+		 */
+		@Override
+		public void addFormatters(FormatterRegistry registry) {
+			registry.addConverter(Sample.class, String.class, source -> "sample");
+			registry.addConverter(String.class, Sample.class, source -> new Sample());
+		}
+
+		static class Sample {}
+
+		@Controller
+		static class SampleController {
+
+			@GetMapping("/sample/{sample}")
+			HttpEntity<?> sample(@PathVariable Sample sample) {
+
+				linkTo(methodOn(SampleController.class).sample(new Sample())).withSelfRel();
+
+				return new ResponseEntity<>(HttpStatus.I_AM_A_TEAPOT);
+			}
+		}
+	}
 }