#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.

Author: odrotbohm

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 static PropertyResolvingMappingDiscoverer of(MappingDiscoverer delegate)
 	@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 String getMapping(Class<?> type) {
 	@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 String getMapping(Method method) {
 	@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 Collection<HttpMethod> getRequestMethod(Class<?> type, Method method) {
 		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);
 	}
 }

src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilder.java

@@ -53,7 +53,7 @@
 public class WebMvcLinkBuilder extends TemplateVariableAwareLinkBuilderSupport<WebMvcLinkBuilder> {
 
 	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();
 

src/test/java/org/springframework/hateoas/TestUtils.java

@@ -23,7 +23,6 @@
 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 @@
 
 /**
  * Utility class to ease testing.
- * 
+ *
  * @author Oliver Gierke
  * @author Greg Turnquist
  */
@@ -87,7 +86,4 @@ public static void assertNotEqualAndDifferentHashCode(Object left, Object right)
 		assertThat(left.hashCode()).isNotEqualTo(right.hashCode());
 		assertThat(left.toString()).isNotEqualTo(right.toString());
 	}
-
-	@Configuration
-	public static class Config {}
 }

src/test/java/org/springframework/hateoas/server/core/ControllerEntityLinksUnitTest.java

@@ -17,7 +17,7 @@
 
 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,46 +26,30 @@
 
 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;
 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}.
  *
  * @author Oliver Gierke
  */
 @ExtendWith(MockitoExtension.class)
-@SpringJUnitWebConfig(classes = TestUtils.Config.class)
 class ControllerEntityLinksUnitTest extends TestUtils {
 
 	@Mock LinkBuilderFactory<LinkBuilder> linkBuilderFactory;
 
-	@Autowired
-	WebApplicationContext context;
-
-	@BeforeEach
-	void contextLoading() {
-		ContextLoader contextLoader = new ContextLoader(context);
-		contextLoader.initWebApplicationContext(new MockServletContext());
-	}
-
 	@Test
 	void rejectsUnannotatedController() {
 

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 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,25 +36,25 @@
  * 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());
 	}
 
 	/**
 	 * @see #361
 	 */
 	@Test
 	void resolvesVariablesInMappings() throws NoSuchMethodException {
+
 		Method method = ResolveMethodEndpointController.class.getMethod("method");
 		AnnotationMappingDiscoverer annotationMappingDiscoverer = new AnnotationMappingDiscoverer(RequestMapping.class);
 
@@ -62,10 +63,11 @@ void resolvesVariablesInMappings() throws NoSuchMethodException {
 		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 @@ interface ResolveMethodEndpointController {
 		@RequestMapping("/${test.child}")
 		void method();
 	}
+
+	@Configuration
+	public static class Config {}
 }

src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderFactoryUnitTest.java

@@ -25,9 +25,7 @@
 
 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.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 @@
  * @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

src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java

@@ -23,27 +23,21 @@
 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;
 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;
 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 @@
  * @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() {