Fix non-deterministic configuration for retrieving parameter names fr…

…om Spring MVC annotations (#137)

Fixes #136.

* Move AnnotationParameterNameDiscoverer and
ProblemValidatorConfiguration
to belgif-rest-problem-spring-boot-2 module
* Add `@AutoConfigureBefore(ValidationAutoConfiguration.class)` to
ProblemValidatorConfiguration
* Add AnnotationParameterNameProvider and
ProblemValidationConfigurationCustomizer
to belgif-rest-problem-spring-boot-3 module
* Introduce CachedAnnotationParameterNameSupport for shared code between
AnnotationParameterNameDiscoverer and AnnotationParameterNameProvider

belgif-rest-problem-spring-boot-2/pom.xml

@@ -71,6 +71,11 @@
       <artifactId>spring-boot-starter-test</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.springframework.boot</groupId>
+      <artifactId>spring-boot-starter-validation</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter</artifactId>

belgif-rest-problem-spring-boot-2/src/main/java/io/github/belgif/rest/problem/spring/AnnotationParameterNameDiscoverer.java

@@ -0,0 +1,20 @@
+package io.github.belgif.rest.problem.spring;
+
+import java.util.stream.Stream;
+
+import org.springframework.core.ParameterNameDiscoverer;
+
+import io.github.belgif.rest.problem.internal.CachedAnnotationParameterNameSupport;
+
+/**
+ * ParameterNameDiscoverer that retrieves the parameter name from Spring MVC annotations (if present).
+ */
+public class AnnotationParameterNameDiscoverer extends CachedAnnotationParameterNameSupport<String[]>
+        implements ParameterNameDiscoverer {
+
+    @Override
+    protected String[] toResult(Stream<String> parameterNames) {
+        return parameterNames.toArray(String[]::new);
+    }
+
+}

belgif-rest-problem-spring/src/main/java/io/github/belgif/rest/problem/spring/ProblemValidatorConfiguration.java → belgif-rest-problem-spring-boot-2/src/main/java/io/github/belgif/rest/problem/spring/ProblemValidatorConfiguration.java

@@ -3,8 +3,10 @@
 import javax.validation.ConstraintViolationException;
 
 import org.springframework.beans.factory.config.BeanDefinition;
+import org.springframework.boot.autoconfigure.AutoConfigureBefore;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
+import org.springframework.boot.autoconfigure.validation.ValidationAutoConfiguration;
 import org.springframework.boot.validation.MessageInterpolatorFactory;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
@@ -21,6 +23,7 @@
  * @see org.springframework.boot.autoconfigure.validation.ValidationAutoConfiguration
  */
 @Configuration
+@AutoConfigureBefore(ValidationAutoConfiguration.class)
 public class ProblemValidatorConfiguration {
 
     @Bean

belgif-rest-problem-spring-boot-2/src/test/java/io/github/belgif/rest/problem/spring/AnnotationParameterNameDiscovererTest.java

@@ -0,0 +1,18 @@
+package io.github.belgif.rest.problem.spring;
+
+import static org.assertj.core.api.Assertions.*;
+
+import java.util.stream.Stream;
+
+import org.junit.jupiter.api.Test;
+
+class AnnotationParameterNameDiscovererTest {
+
+    private final AnnotationParameterNameDiscoverer discoverer = new AnnotationParameterNameDiscoverer();
+
+    @Test
+    void toResult() {
+        assertThat(discoverer.toResult(Stream.of("A", "B"))).containsExactly("A", "B");
+    }
+
+}

belgif-rest-problem-spring-boot-3/src/main/java/io/github/belgif/rest/problem/spring/AnnotationParameterNameProvider.java

@@ -0,0 +1,22 @@
+package io.github.belgif.rest.problem.spring;
+
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import jakarta.validation.ParameterNameProvider;
+
+import io.github.belgif.rest.problem.internal.CachedAnnotationParameterNameSupport;
+
+/**
+ * ParameterNameProvider that retrieves the parameter name from Spring MVC annotations (if present).
+ */
+public class AnnotationParameterNameProvider extends CachedAnnotationParameterNameSupport<List<String>>
+        implements ParameterNameProvider {
+
+    @Override
+    protected List<String> toResult(Stream<String> parameterNames) {
+        return parameterNames.collect(Collectors.toList());
+    }
+
+}

belgif-rest-problem-spring-boot-3/src/main/java/io/github/belgif/rest/problem/spring/ProblemValidationConfigurationCustomizer.java

@@ -0,0 +1,21 @@
+package io.github.belgif.rest.problem.spring;
+
+import jakarta.validation.Configuration;
+
+import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
+import org.springframework.boot.autoconfigure.validation.ValidationConfigurationCustomizer;
+import org.springframework.stereotype.Component;
+
+/**
+ * ValidationConfigurationCustomizer that registers the AnnotationParameterNameProvider.
+ */
+@Component
+@ConditionalOnClass(Configuration.class)
+public class ProblemValidationConfigurationCustomizer implements ValidationConfigurationCustomizer {
+
+    @Override
+    public void customize(Configuration<?> configuration) {
+        configuration.parameterNameProvider(new AnnotationParameterNameProvider());
+    }
+
+}

belgif-rest-problem-spring-boot-3/src/test/java/io/github/belgif/rest/problem/spring/AnnotationParameterNameProviderTest.java

@@ -0,0 +1,18 @@
+package io.github.belgif.rest.problem.spring;
+
+import static org.assertj.core.api.Assertions.*;
+
+import java.util.stream.Stream;
+
+import org.junit.jupiter.api.Test;
+
+class AnnotationParameterNameProviderTest {
+
+    private final AnnotationParameterNameProvider provider = new AnnotationParameterNameProvider();
+
+    @Test
+    void toResult() {
+        assertThat(provider.toResult(Stream.of("A", "B"))).containsExactly("A", "B");
+    }
+
+}

belgif-rest-problem-spring-boot-3/src/test/java/io/github/belgif/rest/problem/spring/ProblemValidationConfigurationCustomizerTest.java

@@ -0,0 +1,34 @@
+package io.github.belgif.rest.problem.spring;
+
+import static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+import jakarta.validation.Configuration;
+import jakarta.validation.ParameterNameProvider;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class ProblemValidationConfigurationCustomizerTest {
+
+    private final ProblemValidationConfigurationCustomizer customizer = new ProblemValidationConfigurationCustomizer();
+
+    @Mock
+    private Configuration configuration;
+
+    @Captor
+    private ArgumentCaptor<ParameterNameProvider> parameterNameProviderCaptor;
+
+    @Test
+    void customize() {
+        when(configuration.parameterNameProvider(parameterNameProviderCaptor.capture())).thenReturn(configuration);
+        customizer.customize(configuration);
+        assertThat(parameterNameProviderCaptor.getValue()).isInstanceOf(AnnotationParameterNameProvider.class);
+    }
+
+}

belgif-rest-problem-spring/src/main/java/io/github/belgif/rest/problem/spring/AnnotationParameterNameDiscoverer.java → belgif-rest-problem-spring/src/main/java/io/github/belgif/rest/problem/internal/CachedAnnotationParameterNameSupport.java

@@ -1,4 +1,4 @@
-package io.github.belgif.rest.problem.spring;
+package io.github.belgif.rest.problem.internal;
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Constructor;
@@ -7,35 +7,35 @@
 import java.lang.reflect.Parameter;
 import java.util.Arrays;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Stream;
 
-import org.springframework.core.ParameterNameDiscoverer;
+import org.springframework.lang.NonNull;
 import org.springframework.web.bind.annotation.CookieValue;
 import org.springframework.web.bind.annotation.MatrixVariable;
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestParam;
 
 /**
- * ParameterNameDiscoverer that retrieves the parameter name from Spring MVC annotations (if present).
+ * Helper class for cached retrieval of parameter names from Spring MVC annotations.
+ *
+ * @param <T> the result type
  */
-public class AnnotationParameterNameDiscoverer implements ParameterNameDiscoverer {
+public abstract class CachedAnnotationParameterNameSupport<T> {
 
-    private final ConcurrentHashMap<Executable, String[]> parameterNameCache = new ConcurrentHashMap<>();
+    private final ConcurrentHashMap<Executable, T> parameterNameCache = new ConcurrentHashMap<>();
 
-    @Override
-    public String[] getParameterNames(Constructor<?> constructor) {
+    public T getParameterNames(@NonNull Constructor<?> constructor) {
         return parameterNameCache.computeIfAbsent(constructor, this::getParameterNames);
     }
 
-    @Override
-    public String[] getParameterNames(Method method) {
+    public T getParameterNames(@NonNull Method method) {
         return parameterNameCache.computeIfAbsent(method, this::getParameterNames);
     }
 
-    private String[] getParameterNames(Executable executable) {
-        return Arrays.stream(executable.getParameters())
-                .map(this::getParameterName)
-                .toArray(String[]::new);
+    private T getParameterNames(Executable executable) {
+        return toResult(Arrays.stream(executable.getParameters())
+                .map(this::getParameterName));
     }
 
     private String getParameterName(Parameter parameter) {
@@ -64,4 +64,6 @@ private String getParameterNameFromAnnotations(Parameter parameter) {
         return null;
     }
 
+    protected abstract T toResult(Stream<String> parameterNames);
+
 }

belgif-rest-problem-spring/src/test/java/io/github/belgif/rest/problem/spring/AnnotationParameterNameDiscovererTest.java → belgif-rest-problem-spring/src/test/java/io/github/belgif/rest/problem/internal/CachedAnnotationParameterNameSupportTest.java

@@ -1,7 +1,11 @@
-package io.github.belgif.rest.problem.spring;
+package io.github.belgif.rest.problem.internal;
 
 import static org.assertj.core.api.Assertions.*;
 
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
@@ -11,28 +15,34 @@
 import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestParam;
 
-class AnnotationParameterNameDiscovererTest {
+class CachedAnnotationParameterNameSupportTest {
 
-    private final AnnotationParameterNameDiscoverer discoverer = new AnnotationParameterNameDiscoverer();
+    private final CachedAnnotationParameterNameSupport<List<String>> support =
+            new CachedAnnotationParameterNameSupport<List<String>>() {
+                @Override
+                protected List<String> toResult(Stream<String> parameterNames) {
+                    return parameterNames.collect(Collectors.toList());
+                }
+            };
 
     @Test
     void getParameterNameConstructor() throws Exception {
-        assertThat(discoverer.getParameterNames(Tested.class.getDeclaredConstructor(String.class, Long.class)))
+        assertThat(support.getParameterNames(Tested.class.getDeclaredConstructor(String.class, Long.class)))
                 .containsExactly("name", "other");
     }
 
     @Test
     void getParameterNamesMethod() throws Exception {
         assertThat(
-                discoverer.getParameterNames(Tested.class.getDeclaredMethod("plainMethod", String.class, Long.class)))
+                support.getParameterNames(Tested.class.getDeclaredMethod("plainMethod", String.class, Long.class)))
                         .containsExactly("name", "other");
     }
 
     @ParameterizedTest
     @ValueSource(
             strings = { "requestParam", "pathVariable", "requestHeader", "cookieValue", "matrixVariable", "fallback" })
     void getParameterNamesMethodWithJaxRsAnnotation(String method) throws Exception {
-        assertThat(discoverer.getParameterNames(Tested.class.getDeclaredMethod(method, String.class)))
+        assertThat(support.getParameterNames(Tested.class.getDeclaredMethod(method, String.class)))
                 .containsExactly("name");
     }
 

src/main/asciidoc/index.adoc

@@ -544,7 +544,6 @@ io.github.belgif.rest.problem.scan-additional-problem-packages=com.acme.custom
 * *RoutingExceptionsHandler:* an exception handler for RestControllers that converts routing related validation exceptions to HTTP 400 BadRequestProblem.
 * *ProblemResponseErrorHandler:* a RestTemplate error handler that converts problem responses to Problem exceptions.
 * *ProblemRestTemplateCustomizer:* a RestTemplateCustomizer that registers the ProblemResponseErrorHandler.
-* *AnnotationParameterNameDiscoverer:* a bean validation ParameterNameDiscoverer that retrieves parameter names from Spring MVC annotations
 
 In general, these components make it possible to use standard java exception handling (throw and try-catch) for dealing with problems in Spring Boot REST APIs.
 
@@ -555,6 +554,8 @@ Rather than depending on <<belgif-rest-problem-spring>> directly, Spring Boot 2.
 
 * *ProblemWebClientCustomizer:* a WebClientCustomizer that registers a filter that converts problem responses to Problem exceptions.
 This handles integration with the https://docs.spring.io/spring-framework/reference/web/webflux-webclient.html[Reactive WebClient].
+* *AnnotationParameterNameDiscoverer:* a bean validation ParameterNameDiscoverer that retrieves parameter names from Spring MVC annotations
+* *ProblemValidatorConfiguration:* registers a LocalValidatorFactoryBean with the AnnotationParameterNameDiscoverer
 
 [[belgif-rest-problem-spring-boot-3]]
 ==== belgif-rest-problem-spring-boot-3
@@ -565,3 +566,5 @@ Rather than depending on <<belgif-rest-problem-spring>> directly, Spring Boot 3.
 This handles integration with the https://docs.spring.io/spring-framework/reference/web/webflux-webclient.html[Reactive WebClient].
 * *NoResourceFoundExceptionHandler:* an exception handler for RestControllers that converts NoResourceFoundException to HTTP 404 ResourceNotFoundProblem.
 * *ProblemRestClientCustomizer:* a RestClientCustomizer that registers the ProblemResponseErrorHandler.
+* *AnnotationParameterNameProvider:* a bean validation ParameterNameProvider that retrieves parameter names from Spring MVC annotations
+* *ProblemValidationConfigurationCustomizer:* a ValidationConfigurationCustomizer that registers the AnnotationParameterNameProvider

src/main/asciidoc/release-notes.adoc

@@ -16,7 +16,11 @@
 
 *belgif-rest-problem:*
 
-* Add in(Input<?>) in InputValidationIssue for a fluent single input setter
+* Add in(Input<?>) to InputValidationIssue for a fluent single input setter
+
+*belgif-rest-problem-spring:*
+
+* Fix non-deterministic configuration for retrieving parameter names from Spring MVC annotations
 
 == Version 0.10