From 0e699a2cf8fc096635f2602194759f8187656144 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 25 Feb 2025 11:31:13 +0100 Subject: [PATCH] Polishing. Align nullness/nullability wording. Use template methods instead of functions to create exceptions. Original Pull Request: #3244 --- .../projection/ProxyProjectionFactory.java | 13 ++-- .../support/MethodInvocationValidator.java | 13 ++-- .../support/RepositoryFactorySupport.java | 4 +- ...=> NullnessMethodInvocationValidator.java} | 78 +++++++++++-------- 4 files changed, 62 insertions(+), 46 deletions(-) rename src/main/java/org/springframework/data/util/{NullabilityMethodInvocationValidator.java => NullnessMethodInvocationValidator.java} (75%) diff --git a/src/main/java/org/springframework/data/projection/ProxyProjectionFactory.java b/src/main/java/org/springframework/data/projection/ProxyProjectionFactory.java index 36b0e77b1..c7b001cad 100644 --- a/src/main/java/org/springframework/data/projection/ProxyProjectionFactory.java +++ b/src/main/java/org/springframework/data/projection/ProxyProjectionFactory.java @@ -24,6 +24,7 @@ import java.util.concurrent.ConcurrentHashMap; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; + import org.springframework.aop.framework.Advised; import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.factory.BeanClassLoaderAware; @@ -31,8 +32,8 @@ import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.convert.Jsr310Converters; import org.springframework.data.util.Lazy; -import org.springframework.data.util.NullabilityMethodInvocationValidator; import org.springframework.data.util.NullableWrapperConverters; +import org.springframework.data.util.NullnessMethodInvocationValidator; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -67,8 +68,8 @@ class ProxyProjectionFactory implements ProjectionFactory, BeanClassLoaderAware private final Lazy defaultMethodInvokingMethodInterceptor = Lazy .of(DefaultMethodInvokingMethodInterceptor::new); - private final Lazy nullabilityValidator = Lazy - .of(NullabilityMethodInvocationValidator::new); + private final Lazy nullabilityValidator = Lazy + .of(NullnessMethodInvocationValidator::new); /** * Creates a new {@link ProxyProjectionFactory}. @@ -124,7 +125,7 @@ class ProxyProjectionFactory implements ProjectionFactory, BeanClassLoaderAware factory.addAdvice(new TargetAwareMethodInterceptor(source.getClass())); - if(NullabilityMethodInvocationValidator.supports(projectionType)) { + if (projectionMetadata.definesNullness) { factory.addAdvice(nullabilityValidator.get()); } @@ -300,10 +301,12 @@ class ProxyProjectionFactory implements ProjectionFactory, BeanClassLoaderAware * * @since 3.1.1 */ - record ProjectionMetadata(boolean hasDefaultMethods, ProjectionInformation projectionInformation) { + record ProjectionMetadata(boolean hasDefaultMethods, boolean definesNullness, + ProjectionInformation projectionInformation) { public static ProjectionMetadata create(Class projectionType, ProjectionInformation projectionInformation) { return new ProjectionMetadata(DefaultMethodInvokingMethodInterceptor.hasDefaultMethods(projectionType), + NullnessMethodInvocationValidator.supports(projectionType), projectionInformation); } } diff --git a/src/main/java/org/springframework/data/repository/core/support/MethodInvocationValidator.java b/src/main/java/org/springframework/data/repository/core/support/MethodInvocationValidator.java index 9241b6f2b..99c6068e7 100644 --- a/src/main/java/org/springframework/data/repository/core/support/MethodInvocationValidator.java +++ b/src/main/java/org/springframework/data/repository/core/support/MethodInvocationValidator.java @@ -15,8 +15,10 @@ */ package org.springframework.data.repository.core.support; +import java.lang.reflect.Method; + import org.springframework.dao.EmptyResultDataAccessException; -import org.springframework.data.util.NullabilityMethodInvocationValidator; +import org.springframework.data.util.NullnessMethodInvocationValidator; /** * Interceptor enforcing required return value and method parameter constraints declared on repository query methods. @@ -29,12 +31,11 @@ import org.springframework.data.util.NullabilityMethodInvocationValidator; * @see org.springframework.lang.NonNull * @see org.springframework.data.util.ReflectionUtils#isNullable(org.springframework.core.MethodParameter) * @see org.springframework.data.util.NullableUtils - * @deprecated use {@link NullabilityMethodInvocationValidator} instead. */ -@Deprecated // TODO: do we want to remove this with next major -public class MethodInvocationValidator extends NullabilityMethodInvocationValidator { +public class MethodInvocationValidator extends NullnessMethodInvocationValidator { - public MethodInvocationValidator() { - super((invocation) -> new EmptyResultDataAccessException("Result must not be null", 1)); + @Override + protected RuntimeException returnValueIsNull(Method method) { + return new EmptyResultDataAccessException("Result must not be null", 1); } } diff --git a/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java b/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java index b60b53304..b5f51c395 100644 --- a/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java +++ b/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java @@ -73,7 +73,7 @@ import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.data.repository.util.QueryExecutionConverters; import org.springframework.data.spel.EvaluationContextProvider; import org.springframework.data.util.Lazy; -import org.springframework.data.util.NullabilityMethodInvocationValidator; +import org.springframework.data.util.NullnessMethodInvocationValidator; import org.springframework.data.util.ReflectionUtils; import org.springframework.expression.ExpressionParser; import org.springframework.expression.spel.standard.SpelExpressionParser; @@ -400,7 +400,7 @@ public abstract class RepositoryFactorySupport result.setTarget(target); result.setInterfaces(repositoryInterface, Repository.class, TransactionalProxy.class); - if (NullabilityMethodInvocationValidator.supports(repositoryInterface)) { + if (NullnessMethodInvocationValidator.supports(repositoryInterface)) { if (logger.isTraceEnabled()) { logger.trace(LogMessage.format("Register MethodInvocationValidator for %s…", repositoryInterface.getName())); } diff --git a/src/main/java/org/springframework/data/util/NullabilityMethodInvocationValidator.java b/src/main/java/org/springframework/data/util/NullnessMethodInvocationValidator.java similarity index 75% rename from src/main/java/org/springframework/data/util/NullabilityMethodInvocationValidator.java rename to src/main/java/org/springframework/data/util/NullnessMethodInvocationValidator.java index 8aef5ce85..67f17250d 100644 --- a/src/main/java/org/springframework/data/util/NullabilityMethodInvocationValidator.java +++ b/src/main/java/org/springframework/data/util/NullnessMethodInvocationValidator.java @@ -15,15 +15,16 @@ */ package org.springframework.data.util; +import kotlin.reflect.KFunction; + import java.lang.annotation.ElementType; import java.lang.reflect.Method; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Function; -import kotlin.reflect.KFunction; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; + import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.KotlinDetector; import org.springframework.core.MethodParameter; @@ -34,8 +35,8 @@ import org.springframework.util.ObjectUtils; /** * Interceptor enforcing required return value and method parameter constraints declared on repository query methods. - * Supports Kotlin nullability markers and JSR-305 Non-null annotations. - * Originally implemented via {@link org.springframework.data.repository.core.support.MethodInvocationValidator}. + * Supports Kotlin nullness markers and JSR-305 Non-null annotations. Originally implemented via + * {@link org.springframework.data.repository.core.support.MethodInvocationValidator}. * * @author Mark Paluch * @author Johannes Englmeier @@ -44,23 +45,12 @@ import org.springframework.util.ObjectUtils; * @see org.springframework.lang.NonNull * @see ReflectionUtils#isNullable(MethodParameter) * @see NullableUtils + * @link Nullness */ -public class NullabilityMethodInvocationValidator implements MethodInterceptor { +public class NullnessMethodInvocationValidator implements MethodInterceptor { private final ParameterNameDiscoverer discoverer = new DefaultParameterNameDiscoverer(); - private final Map nullabilityCache = new ConcurrentHashMap<>(16); - private final Function errorFunction; - - public NullabilityMethodInvocationValidator() { - this((invocation) -> new NullPointerException("Method marked non nullable used with null value. If this is by design consider providing additional metadata using @Nullable annotations.")); - } - - /** - * @param errorFunction custom function creating the error in case of failure. - */ - protected NullabilityMethodInvocationValidator(Function errorFunction) { - this.errorFunction = errorFunction; - } + private final Map nullabilityCache = new ConcurrentHashMap<>(16); /** * Returns {@literal true} if the {@code type} is supported by this interceptor. @@ -80,51 +70,73 @@ public class NullabilityMethodInvocationValidator implements MethodInterceptor { public Object invoke(@SuppressWarnings("null") MethodInvocation invocation) throws Throwable { Method method = invocation.getMethod(); - Nullability nullability = nullabilityCache.get(method); + MethodNullness nullness = nullabilityCache.get(method); - if (nullability == null) { + if (nullness == null) { - nullability = Nullability.of(method, discoverer); - nullabilityCache.put(method, nullability); + nullness = MethodNullness.of(method, discoverer); + nullabilityCache.put(method, nullness); } Object[] arguments = invocation.getArguments(); for (int i = 0; i < method.getParameterCount(); i++) { - if (nullability.isNullableParameter(i)) { + if (nullness.isNullableParameter(i)) { continue; } if ((arguments.length < i) || (arguments[i] == null)) { - throw new IllegalArgumentException( - String.format("Parameter %s in %s.%s must not be null", nullability.getMethodParameterName(i), - ClassUtils.getShortName(method.getDeclaringClass()), method.getName())); + throw argumentIsNull(method, nullness.getMethodParameterName(i)); } } Object result = invocation.proceed(); - if ((result == null) && !nullability.isNullableReturn()) { - throw errorFunction.apply(invocation); + if ((result == null) && !nullness.isNullableReturn()) { + throw returnValueIsNull(method); } return result; } - static final class Nullability { + /** + * Template method to construct a {@link RuntimeException} indicating failure to provide a non-{@literal null} value + * for a method parameter. + * + * @param method + * @param parameterName + * @return + */ + protected RuntimeException argumentIsNull(Method method, String parameterName) { + return new IllegalArgumentException(String.format("Parameter %s in %s.%s must not be null", parameterName, + ClassUtils.getShortName(method.getDeclaringClass()), method.getName())); + } + + /** + * Template method to construct a {@link RuntimeException} indicating failure to return a non-{@literal null} return + * value. + * + * @param method + * @return + */ + protected RuntimeException returnValueIsNull(Method method) { + return new NullPointerException("Return value is null but must not be null"); + } + + static final class MethodNullness { private final boolean nullableReturn; private final boolean[] nullableParameters; private final MethodParameter[] methodParameters; - private Nullability(boolean nullableReturn, boolean[] nullableParameters, MethodParameter[] methodParameters) { + private MethodNullness(boolean nullableReturn, boolean[] nullableParameters, MethodParameter[] methodParameters) { this.nullableReturn = nullableReturn; this.nullableParameters = nullableParameters; this.methodParameters = methodParameters; } - static Nullability of(Method method, ParameterNameDiscoverer discoverer) { + static MethodNullness of(Method method, ParameterNameDiscoverer discoverer) { boolean nullableReturn = isNullableParameter(new MethodParameter(method, -1)); boolean[] nullableParameters = new boolean[method.getParameterCount()]; @@ -138,7 +150,7 @@ public class NullabilityMethodInvocationValidator implements MethodInterceptor { methodParameters[i] = parameter; } - return new Nullability(nullableReturn, nullableParameters, methodParameters); + return new MethodNullness(nullableReturn, nullableParameters, methodParameters); } String getMethodParameterName(int index) { @@ -203,7 +215,7 @@ public class NullabilityMethodInvocationValidator implements MethodInterceptor { return true; } - if (!(o instanceof Nullability that)) { + if (!(o instanceof MethodNullness that)) { return false; }