From d0ff83c62ec0901cdf827c889197760491e5d1e7 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Mon, 25 Jan 2021 19:15:26 +0100 Subject: [PATCH] #118 - Use MVC ConversionService for parameter binding in link creation. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../hateoas/server/core/WebHandler.java | 31 ++++---- .../server/mvc/WebMvcLinkBuilderFactory.java | 34 ++++++++- .../server/reactive/WebFluxLinkBuilder.java | 74 ++++++++++++------- .../HypermediaWebFluxConfigurerTest.java | 56 +++++++++++++- .../HypermediaWebMvcConfigurerTest.java | 45 +++++++++++ 5 files changed, 195 insertions(+), 45 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/server/core/WebHandler.java b/src/main/java/org/springframework/hateoas/server/core/WebHandler.java index adec03b2..d12f0c99 100644 --- 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 conclude(Function finisher); + T conclude(Function finisher, ConversionService conversionService); } public static PreparedWebHandler linkTo(Object invocationValue, @@ -82,9 +82,9 @@ public class WebHandler { public static T linkTo(Object invocationValue, LinkBuilderCreator creator, @Nullable BiFunction additionalUriHandler, - Function finisher) { + Function finisher, Supplier conversionService) { - return linkTo(invocationValue, creator, additionalUriHandler).conclude(finisher); + return linkTo(invocationValue, creator, additionalUriHandler).conclude(finisher, conversionService.get()); } private static PreparedWebHandler 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 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, Function> 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)); diff --git a/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderFactory.java b/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderFactory.java index b1e34fe1..7a113adc 100644 --- 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 { + private static ConversionService FALLBACK_CONVERSION_SERVICE = new DefaultFormattingConversionService(); + private List uriComponentsContributors = new ArrayList<>(); /** @@ -124,7 +136,7 @@ public class WebMvcLinkBuilderFactory implements MethodLinkBuilderFactory 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); + }; + } } diff --git a/src/main/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilder.java b/src/main/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilder.java index 05057d70..50598b83 100644 --- 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 methodOn(Class 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 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 linkToInternal(Object invocation, Mono builder) { + private static Mono linkToInternal(Object invocation, Mono builder) { PreparedWebHandler 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 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 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); + } } } diff --git a/src/test/java/org/springframework/hateoas/config/HypermediaWebFluxConfigurerTest.java b/src/test/java/org/springframework/hateoas/config/HypermediaWebFluxConfigurerTest.java index 4764b9c4..078d6d2d 100644 --- 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> 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)); + } + } + } } diff --git a/src/test/java/org/springframework/hateoas/config/HypermediaWebMvcConfigurerTest.java b/src/test/java/org/springframework/hateoas/config/HypermediaWebMvcConfigurerTest.java index 3e7fa986..4d295bad 100644 --- 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); + } + } + } }