From ffa1dc35398dc3e276298b18b0ab08f37f477314 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Mon, 27 Jul 2020 16:55:08 +0200 Subject: [PATCH] #361 - Polishing. Made implementation class package protected to not expose it. Avoid the use of Optional for hot code paths. Removed factory method as we only use the type internally anyway. Removed test customizations as the discoverer is now just leniently opting out if not ApplicationContext can be found. Original pull request: #1328. --- .../PropertyResolvingMappingDiscoverer.java | 43 +++++++++++-------- .../hateoas/server/mvc/WebMvcLinkBuilder.java | 2 +- .../springframework/hateoas/TestUtils.java | 6 +-- .../core/ControllerEntityLinksUnitTest.java | 18 +------- ...rtyResolvingMappingDiscovererUnitTest.java | 25 ++++++----- .../mvc/WebMvcLinkBuilderFactoryUnitTest.java | 16 ------- .../server/mvc/WebMvcLinkBuilderUnitTest.java | 16 ------- 7 files changed, 44 insertions(+), 82 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/server/mvc/PropertyResolvingMappingDiscoverer.java b/src/main/java/org/springframework/hateoas/server/mvc/PropertyResolvingMappingDiscoverer.java index db1fcea7..18c4faa0 100644 --- a/src/main/java/org/springframework/hateoas/server/mvc/PropertyResolvingMappingDiscoverer.java +++ b/src/main/java/org/springframework/hateoas/server/mvc/PropertyResolvingMappingDiscoverer.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2020 the original author or authors. + * Copyright 2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,32 +15,31 @@ */ package org.springframework.hateoas.server.mvc; -import static java.util.Optional.ofNullable; +import java.lang.reflect.Method; +import java.util.Collection; -import org.springframework.core.env.PropertyResolver; import org.springframework.hateoas.server.core.MappingDiscoverer; import org.springframework.http.HttpMethod; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; import org.springframework.web.context.ContextLoader; - -import java.lang.reflect.Method; -import java.util.Collection; +import org.springframework.web.context.WebApplicationContext; /** * Property resolving adapter of {@link MappingDiscoverer}. * * @author Lars Michele + * @author Oliver Drotbohm */ -public class PropertyResolvingMappingDiscoverer implements MappingDiscoverer { +class PropertyResolvingMappingDiscoverer implements MappingDiscoverer { private final MappingDiscoverer delegate; - private PropertyResolvingMappingDiscoverer(MappingDiscoverer delegate) { - this.delegate = delegate; - } + PropertyResolvingMappingDiscoverer(MappingDiscoverer delegate) { - public static PropertyResolvingMappingDiscoverer of(MappingDiscoverer delegate) { - return new PropertyResolvingMappingDiscoverer(delegate); + Assert.notNull(delegate, "Delegate MappingDiscoverer must not be null!"); + + this.delegate = delegate; } /* @@ -50,7 +49,7 @@ public class PropertyResolvingMappingDiscoverer implements MappingDiscoverer { @Nullable @Override public String getMapping(Class type) { - return ofNullable(delegate.getMapping(type)).map(getPropertyResolver()::resolvePlaceholders).orElse(null); + return resolveProperties(delegate.getMapping(type)); } /* @@ -60,7 +59,7 @@ public class PropertyResolvingMappingDiscoverer implements MappingDiscoverer { @Nullable @Override public String getMapping(Method method) { - return ofNullable(delegate.getMapping(method)).map(getPropertyResolver()::resolvePlaceholders).orElse(null); + return resolveProperties(delegate.getMapping(method)); } /* @@ -70,7 +69,7 @@ public class PropertyResolvingMappingDiscoverer implements MappingDiscoverer { @Nullable @Override public String getMapping(Class type, Method method) { - return ofNullable(delegate.getMapping(type, method)).map(getPropertyResolver()::resolvePlaceholders).orElse(null); + return resolveProperties(delegate.getMapping(type, method)); } /* @@ -82,7 +81,17 @@ public class PropertyResolvingMappingDiscoverer implements MappingDiscoverer { return delegate.getRequestMethod(type, method); } - private static PropertyResolver getPropertyResolver() { - return ContextLoader.getCurrentWebApplicationContext().getEnvironment(); + @Nullable + private static String resolveProperties(@Nullable String mapping) { + + if (mapping == null) { + return mapping; + } + + WebApplicationContext context = ContextLoader.getCurrentWebApplicationContext(); + + return context == null // + ? mapping // + : context.getEnvironment().resolvePlaceholders(mapping); } } 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 921aaa66..2a5fadab 100644 --- a/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilder.java +++ b/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilder.java @@ -53,7 +53,7 @@ import org.springframework.web.util.UriTemplate; public class WebMvcLinkBuilder extends TemplateVariableAwareLinkBuilderSupport { private static final MappingDiscoverer DISCOVERER = CachingMappingDiscoverer - .of(PropertyResolvingMappingDiscoverer.of(new AnnotationMappingDiscoverer(RequestMapping.class))); + .of(new PropertyResolvingMappingDiscoverer(new AnnotationMappingDiscoverer(RequestMapping.class))); private static final WebMvcLinkBuilderFactory FACTORY = new WebMvcLinkBuilderFactory(); private static final CustomUriTemplateHandler HANDLER = new CustomUriTemplateHandler(); diff --git a/src/test/java/org/springframework/hateoas/TestUtils.java b/src/test/java/org/springframework/hateoas/TestUtils.java index db206857..a7cf6b5d 100644 --- a/src/test/java/org/springframework/hateoas/TestUtils.java +++ b/src/test/java/org/springframework/hateoas/TestUtils.java @@ -23,7 +23,6 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import org.junit.jupiter.api.BeforeEach; -import org.springframework.context.annotation.Configuration; import org.springframework.mock.web.MockFilterChain; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; @@ -33,7 +32,7 @@ import org.springframework.web.filter.ForwardedHeaderFilter; /** * Utility class to ease testing. - * + * * @author Oliver Gierke * @author Greg Turnquist */ @@ -87,7 +86,4 @@ public class TestUtils { assertThat(left.hashCode()).isNotEqualTo(right.hashCode()); assertThat(left.toString()).isNotEqualTo(right.toString()); } - - @Configuration - public static class Config {} } diff --git a/src/test/java/org/springframework/hateoas/server/core/ControllerEntityLinksUnitTest.java b/src/test/java/org/springframework/hateoas/server/core/ControllerEntityLinksUnitTest.java index 245e649f..fac96a1f 100755 --- a/src/test/java/org/springframework/hateoas/server/core/ControllerEntityLinksUnitTest.java +++ b/src/test/java/org/springframework/hateoas/server/core/ControllerEntityLinksUnitTest.java @@ -17,7 +17,7 @@ package org.springframework.hateoas.server.core; import static java.util.Collections.*; import static org.assertj.core.api.Assertions.*; -import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.*; @@ -26,12 +26,10 @@ import lombok.Value; import java.util.Arrays; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.hateoas.TestUtils; import org.springframework.hateoas.server.EntityLinks; import org.springframework.hateoas.server.ExposesResourceFor; @@ -39,12 +37,8 @@ import org.springframework.hateoas.server.LinkBuilder; import org.springframework.hateoas.server.LinkBuilderFactory; import org.springframework.hateoas.server.TypedEntityLinks; import org.springframework.hateoas.server.TypedEntityLinks.ExtendedTypedEntityLinks; -import org.springframework.mock.web.MockServletContext; import org.springframework.stereotype.Controller; -import org.springframework.test.context.junit.jupiter.web.SpringJUnitWebConfig; import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.context.ContextLoader; -import org.springframework.web.context.WebApplicationContext; /** * Unit tests for {@link ControllerEntityLinks}. @@ -52,20 +46,10 @@ import org.springframework.web.context.WebApplicationContext; * @author Oliver Gierke */ @ExtendWith(MockitoExtension.class) -@SpringJUnitWebConfig(classes = TestUtils.Config.class) class ControllerEntityLinksUnitTest extends TestUtils { @Mock LinkBuilderFactory linkBuilderFactory; - @Autowired - WebApplicationContext context; - - @BeforeEach - void contextLoading() { - ContextLoader contextLoader = new ContextLoader(context); - contextLoader.initWebApplicationContext(new MockServletContext()); - } - @Test void rejectsUnannotatedController() { diff --git a/src/test/java/org/springframework/hateoas/server/mvc/PropertyResolvingMappingDiscovererUnitTest.java b/src/test/java/org/springframework/hateoas/server/mvc/PropertyResolvingMappingDiscovererUnitTest.java index a2f97205..d70640dc 100755 --- a/src/test/java/org/springframework/hateoas/server/mvc/PropertyResolvingMappingDiscovererUnitTest.java +++ b/src/test/java/org/springframework/hateoas/server/mvc/PropertyResolvingMappingDiscovererUnitTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,7 @@ import java.lang.reflect.Method; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Configuration; import org.springframework.hateoas.TestUtils; import org.springframework.hateoas.server.core.AnnotationMappingDiscoverer; import org.springframework.mock.web.MockServletContext; @@ -35,18 +36,17 @@ import org.springframework.web.context.WebApplicationContext; * Unit tests for {@link PropertyResolvingMappingDiscoverer}. * * @author Lars Michele + * @author Oliver Drotbohm */ -@SpringJUnitWebConfig(classes = TestUtils.Config.class) -@TestPropertySource(properties = {"test.parent=resolvedparent", "test.child=resolvedchild"}) +@SpringJUnitWebConfig(classes = PropertyResolvingMappingDiscovererUnitTest.Config.class) +@TestPropertySource(properties = { "test.parent=resolvedparent", "test.child=resolvedchild" }) class PropertyResolvingMappingDiscovererUnitTest extends TestUtils { - @Autowired - WebApplicationContext context; + @Autowired WebApplicationContext context; @BeforeEach void contextLoading() { - ContextLoader contextLoader = new ContextLoader(context); - contextLoader.initWebApplicationContext(new MockServletContext()); + new ContextLoader(context).initWebApplicationContext(new MockServletContext()); } /** @@ -54,6 +54,7 @@ class PropertyResolvingMappingDiscovererUnitTest extends TestUtils { */ @Test void resolvesVariablesInMappings() throws NoSuchMethodException { + Method method = ResolveMethodEndpointController.class.getMethod("method"); AnnotationMappingDiscoverer annotationMappingDiscoverer = new AnnotationMappingDiscoverer(RequestMapping.class); @@ -62,10 +63,11 @@ class PropertyResolvingMappingDiscovererUnitTest extends TestUtils { assertThat(annotationMappingDiscoverer.getMapping(ResolveMethodEndpointController.class, method)) .isEqualTo("/${test.parent}/${test.child}"); - PropertyResolvingMappingDiscoverer propertyResolvingMappingDiscoverer = PropertyResolvingMappingDiscoverer - .of(annotationMappingDiscoverer); + PropertyResolvingMappingDiscoverer propertyResolvingMappingDiscoverer = new PropertyResolvingMappingDiscoverer( + annotationMappingDiscoverer); - assertThat(propertyResolvingMappingDiscoverer.getMapping(ResolveEndpointController.class)).isEqualTo("/resolvedparent"); + assertThat(propertyResolvingMappingDiscoverer.getMapping(ResolveEndpointController.class)) + .isEqualTo("/resolvedparent"); assertThat(propertyResolvingMappingDiscoverer.getMapping(method)).isEqualTo("/resolvedparent/resolvedchild"); assertThat(propertyResolvingMappingDiscoverer.getMapping(ResolveMethodEndpointController.class, method)) .isEqualTo("/resolvedparent/resolvedchild"); @@ -80,4 +82,7 @@ class PropertyResolvingMappingDiscovererUnitTest extends TestUtils { @RequestMapping("/${test.child}") void method(); } + + @Configuration + public static class Config {} } diff --git a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderFactoryUnitTest.java b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderFactoryUnitTest.java index da8aa3c0..a61cfeb9 100644 --- a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderFactoryUnitTest.java +++ b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderFactoryUnitTest.java @@ -25,9 +25,7 @@ import java.util.Map; import org.joda.time.DateTime; import org.joda.time.format.ISODateTimeFormat; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.MethodParameter; import org.springframework.format.annotation.DateTimeFormat; import org.springframework.format.annotation.DateTimeFormat.ISO; @@ -38,15 +36,11 @@ import org.springframework.hateoas.server.mvc.WebMvcLinkBuilderUnitTest.Controll import org.springframework.hateoas.server.mvc.WebMvcLinkBuilderUnitTest.PersonControllerImpl; import org.springframework.hateoas.server.mvc.WebMvcLinkBuilderUnitTest.PersonsAddressesController; import org.springframework.http.HttpEntity; -import org.springframework.mock.web.MockServletContext; -import org.springframework.test.context.junit.jupiter.web.SpringJUnitWebConfig; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; 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.context.ContextLoader; -import org.springframework.web.context.WebApplicationContext; import org.springframework.web.util.UriComponentsBuilder; /** @@ -57,18 +51,8 @@ import org.springframework.web.util.UriComponentsBuilder; * @author Kamill Sokol * @author Ross Turner */ -@SpringJUnitWebConfig(classes = TestUtils.Config.class) class WebMvcLinkBuilderFactoryUnitTest extends TestUtils { - @Autowired - WebApplicationContext context; - - @BeforeEach - void contextLoading() { - ContextLoader contextLoader = new ContextLoader(context); - contextLoader.initWebApplicationContext(new MockServletContext()); - } - WebMvcLinkBuilderFactory factory = new WebMvcLinkBuilderFactory(); @Test diff --git a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java index 2fee4f86..90999f54 100644 --- a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java +++ b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java @@ -23,9 +23,7 @@ import java.util.Arrays; import java.util.List; import java.util.Optional; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.hateoas.IanaLinkRelations; import org.springframework.hateoas.Link; import org.springframework.hateoas.TemplateVariable; @@ -33,8 +31,6 @@ import org.springframework.hateoas.TemplateVariable.VariableType; import org.springframework.hateoas.TestUtils; import org.springframework.http.HttpEntity; import org.springframework.http.ResponseEntity; -import org.springframework.mock.web.MockServletContext; -import org.springframework.test.context.junit.jupiter.web.SpringJUnitWebConfig; import org.springframework.util.MultiValueMap; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; @@ -42,8 +38,6 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; -import org.springframework.web.context.ContextLoader; -import org.springframework.web.context.WebApplicationContext; import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; @@ -59,18 +53,8 @@ import org.springframework.web.util.UriComponentsBuilder; * @author Oliver Trosien * @author Greg Turnquist */ -@SpringJUnitWebConfig(classes = TestUtils.Config.class) class WebMvcLinkBuilderUnitTest extends TestUtils { - @Autowired - WebApplicationContext context; - - @BeforeEach - void contextLoading() { - ContextLoader contextLoader = new ContextLoader(context); - contextLoader.initWebApplicationContext(new MockServletContext()); - } - @Test void createsLinkToControllerRoot() {