From b608bb0175892194f4721dcb721896aaffc5add4 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Mon, 4 Mar 2019 19:08:00 +0100 Subject: [PATCH] #728 - Make sure invocation mappings are resolved eagerly. Tweaked WebFluxLinkBuilder to make sure the controller invocations are resolved eagerly as otherwise, the controller proxy created would override the invocations due to the lazy evaluation of the links and for multiple method references, only the last one would have been used. --- .../server/core/DummyInvocationUtils.java | 24 ++--- .../hateoas/server/core/WebHandler.java | 88 ++++++++++--------- .../mvc/ControllerLinkBuilderFactory.java | 25 +++--- .../server/mvc/WebMvcLinkBuilderFactory.java | 5 +- .../server/reactive/WebFluxLinkBuilder.java | 24 +++-- .../support/WebFluxEmployeeController.java | 18 ++-- 6 files changed, 99 insertions(+), 85 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/server/core/DummyInvocationUtils.java b/src/main/java/org/springframework/hateoas/server/core/DummyInvocationUtils.java index 3cd36330..add47a6e 100644 --- a/src/main/java/org/springframework/hateoas/server/core/DummyInvocationUtils.java +++ b/src/main/java/org/springframework/hateoas/server/core/DummyInvocationUtils.java @@ -38,19 +38,18 @@ import org.springframework.util.ReflectionUtils; /** * Utility methods to capture dummy method invocations. - * + * * @author Oliver Gierke */ public class DummyInvocationUtils { private static final ObjenesisStd OBJENESIS = new ObjenesisStd(); - private static final Map, Class> CLASS_CACHE = new ConcurrentReferenceHashMap<>(16, - ReferenceType.WEAK); + private static final Map, Class> CLASS_CACHE = new ConcurrentReferenceHashMap<>(16, ReferenceType.WEAK); /** * Method interceptor that records the last method invocation and creates a proxy for the return value that exposes * the method invocation. - * + * * @author Oliver Gierke */ private static class InvocationRecordingMethodInterceptor @@ -71,7 +70,7 @@ public class DummyInvocationUtils { /** * Creates a new {@link InvocationRecordingMethodInterceptor} carrying the given parameters forward that might be * needed to populate the class level mapping. - * + * * @param targetType must not be {@literal null}. * @param parameters must not be {@literal null}. */ @@ -95,7 +94,7 @@ public class DummyInvocationUtils { return getLastInvocation(); } else if (GET_OBJECT_PARAMETERS.equals(method)) { return getObjectParameters(); - } else if (Object.class.equals(method.getDeclaringClass())) { + } else if (ReflectionUtils.isObjectMethod(method)) { return ReflectionUtils.invokeMethod(method, obj, args); } @@ -105,7 +104,7 @@ public class DummyInvocationUtils { return returnType.cast(getProxyWithInterceptor(returnType, this, obj.getClass().getClassLoader())); } - /* + /* * (non-Javadoc) * @see org.aopalliance.intercept.MethodInterceptor#invoke(org.aopalliance.intercept.MethodInvocation) */ @@ -123,7 +122,7 @@ public class DummyInvocationUtils { return invocation; } - /* + /* * (non-Javadoc) * @see org.springframework.hateoas.core.DummyInvocationUtils.LastInvocationAware#getObjectParameters() */ @@ -138,9 +137,10 @@ public class DummyInvocationUtils { * equips it with an {@link InvocationRecordingMethodInterceptor}. The interceptor records the last invocation and * returns a proxy of the return type that also implements {@link LastInvocationAware} so that the last method * invocation can be inspected. Parameters passed to the subsequent method invocation are generally neglected except - * the ones that might be mapped into the URI translation eventually, e.g. {@link org.springframework.web.bind.annotation.PathVariable} in the case of Spring - * MVC. Note, that the return types of the methods have to be capable to be proxied. - * + * the ones that might be mapped into the URI translation eventually, e.g. + * {@link org.springframework.web.bind.annotation.PathVariable} in the case of Spring MVC. Note, that the return types + * of the methods have to be capable to be proxied. + * * @param type must not be {@literal null}. * @param parameters parameters to extend template variables in the type level mapping. * @return @@ -174,7 +174,7 @@ public class DummyInvocationUtils { /** * Returns the already created proxy class for the given source type or creates a new one. - * + * * @param type must not be {@literal null}. * @param classLoader must not be {@literal null}. * @return 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 f6388215..a4e670ab 100644 --- a/src/main/java/org/springframework/hateoas/server/core/WebHandler.java +++ b/src/main/java/org/springframework/hateoas/server/core/WebHandler.java @@ -73,13 +73,13 @@ public class WebHandler { T createBuilder(UriComponents components, TemplateVariables variables, List affordances); } - public static T linkTo(Object invocationValue, - Function mappingToUriComponentsBuilder, LinkBuilderCreator creator) { - return linkTo(invocationValue, mappingToUriComponentsBuilder, creator, null); + public static Function, T> linkTo( + Object invocationValue, LinkBuilderCreator creator) { + return linkTo(invocationValue, creator, null); } - public static T linkTo(Object invocationValue, - Function mappingToUriComponentsBuilder, LinkBuilderCreator creator, + public static Function, T> linkTo( + Object invocationValue, LinkBuilderCreator creator, BiFunction additionalUriHandler) { Assert.isInstanceOf(LastInvocationAware.class, invocationValue); @@ -87,64 +87,66 @@ public class WebHandler { LastInvocationAware invocations = (LastInvocationAware) invocationValue; MethodInvocation invocation = invocations.getLastInvocation(); - String mapping = DISCOVERER.getMapping(invocation.getTargetType(), invocation.getMethod()); + return mappingToUriComponentsBuilder -> { + String mapping = DISCOVERER.getMapping(invocation.getTargetType(), invocation.getMethod()); - UriComponentsBuilder builder = mappingToUriComponentsBuilder.apply(mapping); - UriTemplate template = UriTemplateFactory.templateFor(mapping == null ? "/" : mapping); - Map values = new HashMap<>(); + UriComponentsBuilder builder = mappingToUriComponentsBuilder.apply(mapping); + UriTemplate template = UriTemplateFactory.templateFor(mapping == null ? "/" : mapping); + Map values = new HashMap<>(); - Iterator names = template.getVariableNames().iterator(); - Iterator classMappingParameters = invocations.getObjectParameters(); + Iterator names = template.getVariableNames().iterator(); + Iterator classMappingParameters = invocations.getObjectParameters(); - while (classMappingParameters.hasNext()) { - values.put(names.next(), encodePath(classMappingParameters.next())); - } + while (classMappingParameters.hasNext()) { + values.put(names.next(), encodePath(classMappingParameters.next())); + } - for (AnnotatedParametersParameterAccessor.BoundMethodParameter parameter : PATH_VARIABLE_ACCESSOR - .getBoundParameters(invocation)) { - values.put(parameter.getVariableName(), encodePath(parameter.asString())); - } + for (AnnotatedParametersParameterAccessor.BoundMethodParameter parameter : PATH_VARIABLE_ACCESSOR + .getBoundParameters(invocation)) { + values.put(parameter.getVariableName(), encodePath(parameter.asString())); + } - List optionalEmptyParameters = new ArrayList<>(); + List optionalEmptyParameters = new ArrayList<>(); - for (AnnotatedParametersParameterAccessor.BoundMethodParameter parameter : REQUEST_PARAM_ACCESSOR - .getBoundParameters(invocation)) { + for (AnnotatedParametersParameterAccessor.BoundMethodParameter parameter : REQUEST_PARAM_ACCESSOR + .getBoundParameters(invocation)) { - bindRequestParameters(builder, parameter); + bindRequestParameters(builder, parameter); - if (SKIP_VALUE.equals(parameter.getValue())) { + if (SKIP_VALUE.equals(parameter.getValue())) { - values.put(parameter.getVariableName(), SKIP_VALUE); + values.put(parameter.getVariableName(), SKIP_VALUE); - if (!parameter.isRequired()) { - optionalEmptyParameters.add(parameter.getVariableName()); + if (!parameter.isRequired()) { + optionalEmptyParameters.add(parameter.getVariableName()); + } } } - } - for (String variable : template.getVariableNames()) { - if (!values.containsKey(variable)) { - values.put(variable, SKIP_VALUE); + for (String variable : template.getVariableNames()) { + if (!values.containsKey(variable)) { + values.put(variable, SKIP_VALUE); + } } - } - UriComponents components = additionalUriHandler == null // - ? builder.buildAndExpand(values) - : additionalUriHandler.apply(builder, invocation).buildAndExpand(values); + UriComponents components = additionalUriHandler == null // + ? builder.buildAndExpand(values) + : additionalUriHandler.apply(builder, invocation).buildAndExpand(values); - TemplateVariables variables = NONE; + TemplateVariables variables = NONE; - for (String parameter : optionalEmptyParameters) { + for (String parameter : optionalEmptyParameters) { - boolean previousRequestParameter = components.getQueryParams().isEmpty() && variables.equals(NONE); - TemplateVariable variable = new TemplateVariable(parameter, - previousRequestParameter ? REQUEST_PARAM : REQUEST_PARAM_CONTINUED); - variables = variables.concat(variable); - } + boolean previousRequestParameter = components.getQueryParams().isEmpty() && variables.equals(NONE); + TemplateVariable variable = new TemplateVariable(parameter, + previousRequestParameter ? REQUEST_PARAM : REQUEST_PARAM_CONTINUED); + variables = variables.concat(variable); + } - List affordances = SpringAffordanceBuilder.create(invocation, DISCOVERER, components); + List affordances = SpringAffordanceBuilder.create(invocation, DISCOVERER, components); - return creator.createBuilder(components, variables, affordances); + return creator.createBuilder(components, variables, affordances); + }; } /** diff --git a/src/main/java/org/springframework/hateoas/server/mvc/ControllerLinkBuilderFactory.java b/src/main/java/org/springframework/hateoas/server/mvc/ControllerLinkBuilderFactory.java index f16a570d..df538711 100644 --- a/src/main/java/org/springframework/hateoas/server/mvc/ControllerLinkBuilderFactory.java +++ b/src/main/java/org/springframework/hateoas/server/mvc/ControllerLinkBuilderFactory.java @@ -103,25 +103,24 @@ public class ControllerLinkBuilderFactory implements MethodLinkBuilderFactory ControllerLinkBuilder.getBuilder().path(mapping), - ControllerLinkBuilder::new, (builder, invocation) -> { + return WebHandler.linkTo(invocationValue, ControllerLinkBuilder::new, (builder, invocation) -> { - MethodParameters parameters = new MethodParameters(invocation.getMethod()); - Iterator parameterValues = Arrays.asList(invocation.getArguments()).iterator(); + MethodParameters parameters = new MethodParameters(invocation.getMethod()); + Iterator parameterValues = Arrays.asList(invocation.getArguments()).iterator(); - for (MethodParameter parameter : parameters.getParameters()) { - Object parameterValue = parameterValues.next(); + for (MethodParameter parameter : parameters.getParameters()) { + Object parameterValue = parameterValues.next(); - for (UriComponentsContributor contributor : this.uriComponentsContributors) { + for (UriComponentsContributor contributor : this.uriComponentsContributors) { - if (contributor.supportsParameter(parameter)) { - contributor.enhance(builder, parameter, parameterValue); - } - } + if (contributor.supportsParameter(parameter)) { + contributor.enhance(builder, parameter, parameterValue); } + } + } - return builder; - }); + return builder; + }).apply(mapping -> ControllerLinkBuilder.getBuilder().path(mapping)); } /* 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 cff5af86..3805c801 100644 --- a/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderFactory.java +++ b/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderFactory.java @@ -106,7 +106,7 @@ public class WebMvcLinkBuilderFactory implements MethodLinkBuilderFactory builderFactory = mapping -> UriComponentsBuilderFactory.getBuilder() .path(mapping); - return WebHandler.linkTo(invocationValue, builderFactory, WebMvcLinkBuilder::new, (builder, invocation) -> { + return WebHandler.linkTo(invocationValue, WebMvcLinkBuilder::new, (builder, invocation) -> { MethodParameters parameters = new MethodParameters(invocation.getMethod()); Iterator parameterValues = Arrays.asList(invocation.getArguments()).iterator(); @@ -121,7 +121,8 @@ public class WebMvcLinkBuilderFactory implements MethodLinkBuilderFactory linkToInternal(invocation) // + Mono builder = linkToInternal(invocation); + + return new WebFluxLink(link.flatMap(it -> builder // .flatMapIterable(WebFluxLinkBuilder::getAffordances) // .singleOrEmpty() // .map(it::andAffordance))); @@ -237,14 +239,20 @@ public class WebFluxLinkBuilder extends TemplateVariableAwareLinkBuilderSupport< private static Mono linkToInternal(Object invocation) { - return Mono.subscriberContext() // - .flatMap(context -> linkToInternal(invocation, context.getOrDefault(SERVER_WEB_EXCHANGE, null))); + return linkToInternal(invocation, + Mono.subscriberContext().map(context -> getBuilder(context.getOrDefault(SERVER_WEB_EXCHANGE, null)))); } - private static Mono linkToInternal(Object invocation, ServerWebExchange exchange) { + private static Mono linkToInternal(Object invocation, Mono exchange) { - return Mono.just(WebHandler.linkTo(invocation, // - path -> getBuilder(exchange).replacePath(path == null ? "/" : path), // - WebFluxLinkBuilder::new)); + Function, WebFluxLinkBuilder> linkTo = // + WebHandler.linkTo(invocation, WebFluxLinkBuilder::new); + + return exchange.map(WebFluxLinkBuilder::getBuilderCreator) // + .map(linkTo::apply); + } + + private static Function getBuilderCreator(UriComponentsBuilder exchange) { + return path -> exchange.replacePath(path == null ? "/" : path); } } diff --git a/src/test/java/org/springframework/hateoas/support/WebFluxEmployeeController.java b/src/test/java/org/springframework/hateoas/support/WebFluxEmployeeController.java index fb82edd7..43d8387e 100644 --- a/src/test/java/org/springframework/hateoas/support/WebFluxEmployeeController.java +++ b/src/test/java/org/springframework/hateoas/support/WebFluxEmployeeController.java @@ -82,7 +82,7 @@ public class WebFluxEmployeeController { @RequestParam Optional name, // @RequestParam Optional role) { - Class controller = WebFluxEmployeeController.class; + WebFluxEmployeeController controller = methodOn(WebFluxEmployeeController.class); return Flux.fromIterable(EMPLOYEES.keySet()) // .flatMap(id -> findOne(id)) // @@ -96,11 +96,13 @@ public class WebFluxEmployeeController { .orElse(true); return nameMatches && roleMatches; - }).collectList().flatMap(resources -> linkTo(methodOn(controller).all()).withSelfRel() // - .andAffordance(methodOn(controller).newEmployee(null)) // - .andAffordance(methodOn(controller).search(null, null)) // - .toMono() // - .map(selfLink -> new CollectionModel<>(resources, selfLink))); + }).collectList().flatMap(resources -> { + return linkTo(controller.all()).withSelfRel() // + .andAffordance(controller.newEmployee(null)) // + .andAffordance(controller.search(null, null)) // + .toMono() // + .map(selfLink -> new CollectionModel<>(resources, selfLink)); + }); } @GetMapping("/employees/{id}") @@ -118,7 +120,9 @@ public class WebFluxEmployeeController { return selfLink.zipWith(employeesLink) // .map(function((left, right) -> Links.of(left, right))) // - .map(links -> new EntityModel<>(EMPLOYEES.get(id), links)); + .map(links -> { + return new EntityModel<>(EMPLOYEES.get(id), links); + }); } @PostMapping("/employees")