From fcb530e48a0c965a7bd91a9f9ae77a7b963530a3 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Mon, 8 Mar 2021 14:26:08 +0100 Subject: [PATCH] #1486 - 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 | 102 ++++++++++++++++-- .../hateoas/server/core/WebHandler.java | 83 +++----------- .../hateoas/server/mvc/WebMvcLinkBuilder.java | 8 +- 3 files changed, 110 insertions(+), 83 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 c5e6b175..b2c54fa4 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,12 @@ 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.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 +43,49 @@ 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); + MethodParameters parameters = MethodParameters.of(method); ResolvableType inputType = parameters.getParametersWith(RequestBody.class).stream() // @@ -63,10 +97,7 @@ 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) // @@ -76,4 +107,55 @@ public class SpringAffordanceBuilder { .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 adec03b2..02b85bc5 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 -> { @@ -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(); for (HandlerMethodParameter parameter : parameters.getParameterAnnotatedWith(PathVariable.class, arguments)) { @@ -161,8 +160,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); }; @@ -228,60 +227,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