From 8e78cf2275a5a2baad772a2bdbcaa71026a5e0d7 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Mon, 8 Mar 2021 14:26:08 +0100 Subject: [PATCH] #1484 - Fix performance regression in WebHandler. Moved the affordance metadata caching into SpringAffordanceBuilder and separate the caching of the metadata lookup from the assembly of the affordance. The latter is based on the URI resulting from expanding the request mapping with the given value and thus is likely to produce a lot of different values so that they're rather unsuitable as cache key. That means that we now return fresh Affordance instances for every request which probably causes a tiny hit on performance but still does not need additional metadata lookups. Also, it opens up the door to enrich the affordances with property value providers that would allow us to render affordance fields that refer to a instances property, e.g. to render edit forms. --- .../server/core/SpringAffordanceBuilder.java | 106 ++++++++++++++++-- .../hateoas/server/core/WebHandler.java | 83 +++----------- .../hateoas/server/mvc/WebMvcLinkBuilder.java | 8 +- 3 files changed, 113 insertions(+), 84 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/server/core/SpringAffordanceBuilder.java b/src/main/java/org/springframework/hateoas/server/core/SpringAffordanceBuilder.java index 152635d0..c310c065 100644 --- a/src/main/java/org/springframework/hateoas/server/core/SpringAffordanceBuilder.java +++ b/src/main/java/org/springframework/hateoas/server/core/SpringAffordanceBuilder.java @@ -16,7 +16,10 @@ package org.springframework.hateoas.server.core; import java.lang.reflect.Method; +import java.util.Collection; import java.util.List; +import java.util.Objects; +import java.util.function.Function; import java.util.stream.Collectors; import org.springframework.core.ResolvableType; @@ -24,9 +27,13 @@ import org.springframework.hateoas.Affordance; import org.springframework.hateoas.Link; import org.springframework.hateoas.LinkRelation; import org.springframework.hateoas.QueryParameter; -import org.springframework.hateoas.mediatype.AffordanceModelFactory; import org.springframework.hateoas.mediatype.Affordances; +import org.springframework.http.HttpMethod; +import org.springframework.http.MediaType; +import org.springframework.lang.Nullable; +import org.springframework.util.ConcurrentLruCache; import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; /** @@ -37,21 +44,50 @@ import org.springframework.web.bind.annotation.RequestParam; */ public class SpringAffordanceBuilder { + @SuppressWarnings("deprecation") // + public static final MappingDiscoverer DISCOVERER = CachingMappingDiscoverer + .of(new PropertyResolvingMappingDiscoverer(new AnnotationMappingDiscoverer(RequestMapping.class))); + + private static final ConcurrentLruCache>> AFFORDANCES_CACHE = new ConcurrentLruCache<>( + 256, key -> SpringAffordanceBuilder.create(key.type, key.method)); + /** - * Use the attributes of the current method call along with a collection of {@link AffordanceModelFactory}'s to create - * a set of {@link Affordance}s. + * Returns all {@link Affordance}s for the given type's method and base URI. * * @param type must not be {@literal null}. * @param method must not be {@literal null}. - * @param href must not be {@literal null}. - * @param discoverer must not be {@literal null}. + * @param href must not be {@literal null} or empty. * @return */ - public static List create(Class type, Method method, String href, MappingDiscoverer discoverer) { + public static List getAffordances(Class type, Method method, String href) { String methodName = method.getName(); Link affordanceLink = Link.of(href, LinkRelation.of(methodName)); + return AFFORDANCES_CACHE + .get(new AffordanceKey(type, method)) + .apply(Affordances.of(affordanceLink)); + } + + /** + * Returns the mapping for the given type's method. + * + * @param type must not be {@literal null}. + * @param method must not be {@literal null}. + * @return + */ + @Nullable + public static String getMapping(Class type, Method method) { + return DISCOVERER.getMapping(type, method); + } + + private static Function> create(Class type, Method method) { + + String methodName = method.getName(); + ResolvableType outputType = ResolvableType.forMethodReturnType(method); + Collection requestMethods = DISCOVERER.getRequestMethod(type, method); + List inputMediaTypes = DISCOVERER.getConsumes(method); + MethodParameters parameters = MethodParameters.of(method); ResolvableType inputType = parameters.getParametersWith(RequestBody.class).stream() // @@ -63,18 +99,66 @@ public class SpringAffordanceBuilder { .map(QueryParameter::of) // .collect(Collectors.toList()); - ResolvableType outputType = ResolvableType.forMethodReturnType(method); - Affordances affordances = Affordances.of(affordanceLink); - - return discoverer.getRequestMethod(type, method).stream() // + return affordances -> requestMethods.stream() // .flatMap(it -> affordances.afford(it) // .withInput(inputType) // .withOutput(outputType) // .withParameters(queryMethodParameters) // .withName(methodName) // - .withInputMediaTypes(discoverer.getConsumes(method)) // + .withInputMediaTypes(inputMediaTypes) // .build() // .stream()) // .collect(Collectors.toList()); } + + private static final class AffordanceKey { + + private final Class type; + private final Method method; + + AffordanceKey(Class type, Method method) { + + this.type = type; + this.method = method; + } + + /* + * (non-Javadoc) + * @see java.lang.Object#equals(java.lang.Object) + */ + @Override + public boolean equals(@Nullable Object o) { + + if (this == o) { + return true; + } + + if (!(o instanceof AffordanceKey)) { + return false; + } + + AffordanceKey that = (AffordanceKey) o; + + return Objects.equals(this.type, that.type) // + && Objects.equals(this.method, that.method); + } + + /* + * (non-Javadoc) + * @see java.lang.Object#hashCode() + */ + @Override + public int hashCode() { + return Objects.hash(this.type, this.method); + } + + /* + * (non-Javadoc) + * @see java.lang.Object#toString() + */ + @Override + public String toString() { + return "WebHandler.AffordanceKey(type=" + this.type + ", method=" + this.method + ")"; + } + } } 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 d12f0c99..ff96e8fe 100644 --- a/src/main/java/org/springframework/hateoas/server/core/WebHandler.java +++ b/src/main/java/org/springframework/hateoas/server/core/WebHandler.java @@ -22,7 +22,15 @@ import static org.springframework.web.util.UriComponents.UriTemplateVariables.*; import java.lang.annotation.Annotation; import java.lang.reflect.Method; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.function.BiFunction; import java.util.function.Function; @@ -38,13 +46,11 @@ import org.springframework.hateoas.TemplateVariables; import org.springframework.hateoas.server.LinkBuilder; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.ConcurrentLruCache; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.PathVariable; -import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.ValueConstants; import org.springframework.web.util.UriComponents; @@ -59,13 +65,6 @@ import org.springframework.web.util.UriTemplate; */ public class WebHandler { - @SuppressWarnings("deprecation") // - public static final MappingDiscoverer DISCOVERER = CachingMappingDiscoverer - .of(new PropertyResolvingMappingDiscoverer(new AnnotationMappingDiscoverer(RequestMapping.class))); - - private static final ConcurrentLruCache> AFFORDANCES_CACHE = new ConcurrentLruCache<>( - 256, key -> SpringAffordanceBuilder.create(key.type, key.method, key.href.toUriString(), DISCOVERER)); - public interface LinkBuilderCreator { T createBuilder(UriComponents components, TemplateVariables variables, List affordances); } @@ -101,8 +100,7 @@ public class WebHandler { } MethodInvocation invocation = invocations.getLastInvocation(); - - String mapping = DISCOVERER.getMapping(invocation.getTargetType(), invocation.getMethod()); + String mapping = SpringAffordanceBuilder.getMapping(invocation.getTargetType(), invocation.getMethod()); return (finisher, conversionService) -> { @@ -118,7 +116,8 @@ public class WebHandler { values.put(names.next(), encodePath(classMappingParameters.next())); } - HandlerMethodParameters parameters = HandlerMethodParameters.of(invocation.getMethod()); + Method method = invocation.getMethod(); + HandlerMethodParameters parameters = HandlerMethodParameters.of(method); Object[] arguments = invocation.getArguments(); ConversionService resolved = conversionService; @@ -163,8 +162,8 @@ public class WebHandler { variables = variables.concat(variable); } - List affordances = AFFORDANCES_CACHE - .get(new AffordanceKey(invocation.getTargetType(), invocation.getMethod(), components)); + List affordances = SpringAffordanceBuilder.getAffordances(invocation.getTargetType(), method, + components.toUriString()); return creator.createBuilder(components, variables, affordances); }; @@ -230,60 +229,6 @@ public class WebHandler { } } - private static final class AffordanceKey { - - private final Class type; - private final Method method; - private final UriComponents href; - - AffordanceKey(Class type, Method method, UriComponents href) { - - this.type = type; - this.method = method; - this.href = href; - } - - /* - * (non-Javadoc) - * @see java.lang.Object#equals(java.lang.Object) - */ - @Override - public boolean equals(@Nullable Object o) { - - if (this == o) { - return true; - } - - if (!(o instanceof AffordanceKey)) { - return false; - } - - AffordanceKey that = (AffordanceKey) o; - - return Objects.equals(this.type, that.type) // - && Objects.equals(this.method, that.method) // - && Objects.equals(this.href, that.href); - } - - /* - * (non-Javadoc) - * @see java.lang.Object#hashCode() - */ - @Override - public int hashCode() { - return Objects.hash(this.type, this.method, this.href); - } - - /* - * (non-Javadoc) - * @see java.lang.Object#toString() - */ - @Override - public String toString() { - return "WebHandler.AffordanceKey(type=" + this.type + ", method=" + this.method + ", href=" + this.href + ")"; - } - } - private static class HandlerMethodParameters { private static final List> ANNOTATIONS = Arrays.asList(RequestParam.class, diff --git a/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilder.java b/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilder.java index f61d7055..2d8b59a8 100644 --- a/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilder.java +++ b/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilder.java @@ -25,9 +25,9 @@ import org.springframework.hateoas.Affordance; import org.springframework.hateoas.Link; import org.springframework.hateoas.TemplateVariables; import org.springframework.hateoas.server.core.DummyInvocationUtils; +import org.springframework.hateoas.server.core.SpringAffordanceBuilder; import org.springframework.hateoas.server.core.TemplateVariableAwareLinkBuilderSupport; import org.springframework.hateoas.server.core.UriTemplateFactory; -import org.springframework.hateoas.server.core.WebHandler; import org.springframework.util.Assert; import org.springframework.web.util.DefaultUriTemplateHandler; import org.springframework.web.util.UriComponents; @@ -89,7 +89,7 @@ public class WebMvcLinkBuilder extends TemplateVariableAwareLinkBuilderSupport