From 39cd31613b60ba5759808f0774e6fa0261cccf87 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 10 Apr 2024 18:06:32 +0200 Subject: [PATCH] Polishing --- .../aop/framework/CglibAopProxy.java | 10 ++-- .../aop/framework/JdkDynamicAopProxy.java | 4 +- .../springframework/aop/support/AopUtils.java | 9 ++-- ...eflectionBeanRegistrationAotProcessor.java | 3 +- .../server/ServletServerHttpRequestTests.java | 9 ++-- ...questParamMethodArgumentResolverTests.java | 52 +++---------------- .../annotation/ControllerAdviceTests.java | 9 ++-- ...questParamMethodArgumentResolverTests.java | 29 +++++------ 8 files changed, 43 insertions(+), 82 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java index b227b90d191..44bdf08db7b 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java @@ -94,17 +94,17 @@ class CglibAopProxy implements AopProxy, Serializable { private static final int INVOKE_HASHCODE = 6; + private static final String COROUTINES_FLOW_CLASS_NAME = "kotlinx.coroutines.flow.Flow"; + + private static final boolean coroutinesReactorPresent = ClassUtils.isPresent( + "kotlinx.coroutines.reactor.MonoKt", CglibAopProxy.class.getClassLoader()); + /** Logger available to subclasses; static to optimize serialization. */ protected static final Log logger = LogFactory.getLog(CglibAopProxy.class); /** Keeps track of the Classes that we have validated for final methods. */ private static final Map, Boolean> validatedClasses = new WeakHashMap<>(); - private static final String COROUTINES_FLOW_CLASS_NAME = "kotlinx.coroutines.flow.Flow"; - - private static final boolean coroutinesReactorPresent = ClassUtils.isPresent("kotlinx.coroutines.reactor.MonoKt", - CglibAopProxy.class.getClassLoader());; - /** The configuration used to configure this proxy. */ protected final AdvisedSupport advised; diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java b/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java index a1cd6539be0..3ab70ee9e87 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java @@ -75,8 +75,8 @@ final class JdkDynamicAopProxy implements AopProxy, InvocationHandler, Serializa private static final String COROUTINES_FLOW_CLASS_NAME = "kotlinx.coroutines.flow.Flow"; - private static final boolean coroutinesReactorPresent = ClassUtils.isPresent("kotlinx.coroutines.reactor.MonoKt", - JdkDynamicAopProxy.class.getClassLoader());; + private static final boolean coroutinesReactorPresent = ClassUtils.isPresent( + "kotlinx.coroutines.reactor.MonoKt", JdkDynamicAopProxy.class.getClassLoader()); /** We use a static Log to avoid serialization issues. */ private static final Log logger = LogFactory.getLog(JdkDynamicAopProxy.class); diff --git a/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java b/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java index 0df9a367407..da5e4eefb25 100644 --- a/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java +++ b/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java @@ -64,8 +64,9 @@ import org.springframework.util.ReflectionUtils; */ public abstract class AopUtils { - private static final boolean coroutinesReactorPresent = ClassUtils.isPresent("kotlinx.coroutines.reactor.MonoKt", - AopUtils.class.getClassLoader());; + private static final boolean coroutinesReactorPresent = ClassUtils.isPresent( + "kotlinx.coroutines.reactor.MonoKt", AopUtils.class.getClassLoader()); + /** * Check whether the given object is a JDK dynamic proxy or a CGLIB proxy. @@ -349,8 +350,8 @@ public abstract class AopUtils { // Use reflection to invoke the method. try { ReflectionUtils.makeAccessible(method); - return coroutinesReactorPresent && KotlinDetector.isSuspendingFunction(method) ? - KotlinDelegate.invokeSuspendingFunction(method, target, args) : method.invoke(target, args); + return (coroutinesReactorPresent && KotlinDetector.isSuspendingFunction(method) ? + KotlinDelegate.invokeSuspendingFunction(method, target, args) : method.invoke(target, args)); } catch (InvocationTargetException ex) { // Invoked method threw a checked exception. diff --git a/spring-context/src/main/java/org/springframework/context/aot/KotlinReflectionBeanRegistrationAotProcessor.java b/spring-context/src/main/java/org/springframework/context/aot/KotlinReflectionBeanRegistrationAotProcessor.java index 48341f736e9..e8804416eee 100644 --- a/spring-context/src/main/java/org/springframework/context/aot/KotlinReflectionBeanRegistrationAotProcessor.java +++ b/spring-context/src/main/java/org/springframework/context/aot/KotlinReflectionBeanRegistrationAotProcessor.java @@ -35,8 +35,8 @@ import org.springframework.lang.Nullable; */ class KotlinReflectionBeanRegistrationAotProcessor implements BeanRegistrationAotProcessor { - @Nullable @Override + @Nullable public BeanRegistrationAotContribution processAheadOfTime(RegisteredBean registeredBean) { Class beanClass = registeredBean.getBeanClass(); if (KotlinDetector.isKotlinType(beanClass)) { @@ -45,6 +45,7 @@ class KotlinReflectionBeanRegistrationAotProcessor implements BeanRegistrationAo return null; } + private static class AotContribution implements BeanRegistrationAotContribution { private final Class beanClass; diff --git a/spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java b/spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java index 2e1792f6da6..dcb98b14515 100644 --- a/spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java @@ -124,6 +124,7 @@ class ServletServerHttpRequestTests { assertThat(headers).as("No HttpHeaders returned").isNotNull(); assertThat(headers.containsKey(headerName)).as("Invalid headers returned").isTrue(); List headerValues = headers.get(headerName); + assertThat(headerValues).as("No header values returned").isNotNull(); assertThat(headerValues.size()).as("Invalid header values returned").isEqualTo(2); assertThat(headerValues.contains(headerValue1)).as("Invalid header values returned").isTrue(); assertThat(headerValues.contains(headerValue2)).as("Invalid header values returned").isTrue(); @@ -150,7 +151,7 @@ class ServletServerHttpRequestTests { assertThat(headers.getContentType()).isNull(); } - @Test // gh-27957 + @Test // gh-27957 void getHeadersWithWildcardContentType() { mockRequest.setContentType("*/*"); mockRequest.removeHeader("Content-Type"); @@ -166,7 +167,7 @@ class ServletServerHttpRequestTests { assertThat(result).as("Invalid content returned").isEqualTo(content); } - @Test // gh-13318 + @Test // gh-13318 void getFormBody() throws IOException { mockRequest.setContentType("application/x-www-form-urlencoded; charset=UTF-8"); mockRequest.setMethod("POST"); @@ -189,7 +190,7 @@ class ServletServerHttpRequestTests { assertThat(result).as("Invalid content returned").isEqualTo(content); } - @Test // gh-31327 + @Test // gh-31327 void getFormBodyWhenQueryParamsAlsoPresent() throws IOException { mockRequest.setContentType("application/x-www-form-urlencoded; charset=UTF-8"); mockRequest.setMethod("POST"); @@ -203,7 +204,7 @@ class ServletServerHttpRequestTests { assertThat(result).as("Invalid content returned").isEqualTo(content); } - @Test // gh-32471 + @Test // gh-32471 void getFormBodyWhenNotEncodedCharactersPresent() throws IOException { mockRequest.setContentType("application/x-www-form-urlencoded; charset=UTF-8"); mockRequest.setMethod("POST"); diff --git a/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java index 2205d6b1f0e..3b61880c631 100644 --- a/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java @@ -150,8 +150,6 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annot(requestParam().notRequired("bar")).arg(String.class); Object result = resolver.resolveArgument(param, null, webRequest, null); - boolean condition = result instanceof String; - assertThat(condition).isTrue(); assertThat(result).as("Invalid result").isEqualTo(expected); } @@ -162,12 +160,10 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(String[].class); Object result = resolver.resolveArgument(param, null, webRequest, null); - boolean condition = result instanceof String[]; - assertThat(condition).isTrue(); - assertThat((String[]) result).as("Invalid result").isEqualTo(expected); + assertThat(result).as("Invalid result").isEqualTo(expected); } - @Test // gh-32577 + @Test // gh-32577 void resolveStringArrayWithEmptyArraySuffix() throws Exception { String[] expected = new String[] {"foo", "bar"}; request.addParameter("name[]", expected[0]); @@ -175,9 +171,7 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(String[].class); Object result = resolver.resolveArgument(param, null, webRequest, null); - boolean condition = result instanceof String[]; - assertThat(condition).isTrue(); - assertThat((String[]) result).isEqualTo(expected); + assertThat(result).isEqualTo(expected); } @Test @@ -189,8 +183,6 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(MultipartFile.class); Object result = resolver.resolveArgument(param, null, webRequest, null); - boolean condition = result instanceof MultipartFile; - assertThat(condition).isTrue(); assertThat(result).as("Invalid result").isEqualTo(expected); } @@ -206,9 +198,6 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(List.class, MultipartFile.class); Object result = resolver.resolveArgument(param, null, webRequest, null); - - boolean condition = result instanceof List; - assertThat(condition).isTrue(); assertThat(result).isEqualTo(Arrays.asList(expected1, expected2)); } @@ -235,9 +224,7 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(MultipartFile[].class); Object result = resolver.resolveArgument(param, null, webRequest, null); - - boolean condition = result instanceof MultipartFile[]; - assertThat(condition).isTrue(); + assertThat(result instanceof MultipartFile[]).isTrue(); MultipartFile[] parts = (MultipartFile[]) result; assertThat(parts).hasSize(2); assertThat(expected1).isEqualTo(parts[0]); @@ -266,9 +253,6 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(Part.class); Object result = resolver.resolveArgument(param, null, webRequest, null); - - boolean condition = result instanceof Part; - assertThat(condition).isTrue(); assertThat(result).as("Invalid result").isEqualTo(expected); } @@ -286,9 +270,6 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(List.class, Part.class); Object result = resolver.resolveArgument(param, null, webRequest, null); - - boolean condition = result instanceof List; - assertThat(condition).isTrue(); assertThat(result).isEqualTo(Arrays.asList(expected1, expected2)); } @@ -319,9 +300,7 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(Part[].class); Object result = resolver.resolveArgument(param, null, webRequest, null); - - boolean condition = result instanceof Part[]; - assertThat(condition).isTrue(); + assertThat(result instanceof Part[]).isTrue(); Part[] parts = (Part[]) result; assertThat(parts).hasSize(2); assertThat(expected1).isEqualTo(parts[0]); @@ -350,8 +329,6 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annotNotPresent().arg(MultipartFile.class); Object result = resolver.resolveArgument(param, null, webRequest, null); - boolean condition = result instanceof MultipartFile; - assertThat(condition).isTrue(); assertThat(result).as("Invalid result").isEqualTo(expected); } @@ -368,8 +345,6 @@ class RequestParamMethodArgumentResolverTests { .annotNotPresent(RequestParam.class).arg(List.class, MultipartFile.class); Object result = resolver.resolveArgument(param, null, webRequest, null); - boolean condition = result instanceof List; - assertThat(condition).isTrue(); assertThat(result).isEqualTo(Arrays.asList(expected1, expected2)); } @@ -424,8 +399,6 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annotNotPresent(RequestParam.class).arg(Part.class); Object result = resolver.resolveArgument(param, null, webRequest, null); - boolean condition = result instanceof Part; - assertThat(condition).isTrue(); assertThat(result).as("Invalid result").isEqualTo(expected); } @@ -433,8 +406,6 @@ class RequestParamMethodArgumentResolverTests { void resolveDefaultValue() throws Exception { MethodParameter param = this.testMethod.annot(requestParam().notRequired("bar")).arg(String.class); Object result = resolver.resolveArgument(param, null, webRequest, null); - boolean condition = result instanceof String; - assertThat(condition).isTrue(); assertThat(result).as("Invalid result").isEqualTo("bar"); } @@ -452,7 +423,6 @@ class RequestParamMethodArgumentResolverTests { WebDataBinderFactory binderFactory = mock(); given(binderFactory.createBinder(webRequest, null, "stringNotAnnot")).willReturn(binder); - request.addParameter("stringNotAnnot", ""); MethodParameter param = this.testMethod.annotNotPresent(RequestParam.class).arg(String.class); @@ -466,7 +436,6 @@ class RequestParamMethodArgumentResolverTests { WebDataBinderFactory binderFactory = mock(); given(binderFactory.createBinder(webRequest, null, "booleanParam")).willReturn(binder); - request.addParameter("booleanParam", " "); MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(Boolean.class); @@ -481,7 +450,6 @@ class RequestParamMethodArgumentResolverTests { WebDataBinderFactory binderFactory = mock(); given(binderFactory.createBinder(webRequest, null, "name")).willReturn(binder); - request.addParameter("name", ""); MethodParameter param = this.testMethod.annot(requestParam().notRequired()).arg(String.class); @@ -489,14 +457,13 @@ class RequestParamMethodArgumentResolverTests { assertThat(arg).isNull(); } - @Test // gh-29550 + @Test // gh-29550 public void missingRequestParamEmptyValueNotRequiredWithDefaultValue() throws Exception { WebDataBinder binder = new WebRequestDataBinder(null); binder.registerCustomEditor(String.class, new StringTrimmerEditor(true)); WebDataBinderFactory binderFactory = mock(); given(binderFactory.createBinder(webRequest, null, "name")).willReturn(binder); - request.addParameter("name", " "); MethodParameter param = this.testMethod.annot(requestParam().notRequired("bar")).arg(String.class); @@ -509,9 +476,6 @@ class RequestParamMethodArgumentResolverTests { request.setParameter("stringNotAnnot", "plainValue"); MethodParameter param = this.testMethod.annotNotPresent(RequestParam.class).arg(String.class); Object result = resolver.resolveArgument(param, null, webRequest, null); - - boolean condition = result instanceof String; - assertThat(condition).isTrue(); assertThat(result).isEqualTo("plainValue"); } @@ -658,9 +622,7 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(Optional.class, MultipartFile.class); Object result = resolver.resolveArgument(param, null, webRequest, binderFactory); - - boolean condition = result instanceof Optional; - assertThat(condition).isTrue(); + assertThat(result instanceof Optional).isTrue(); assertThat(((Optional) result).get()).as("Invalid result").isEqualTo(expected); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ControllerAdviceTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ControllerAdviceTests.java index 0cbd20e9091..d0925de205b 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ControllerAdviceTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ControllerAdviceTests.java @@ -188,8 +188,8 @@ class ControllerAdviceTests { TestController controller = context.getBean(TestController.class); controller.setException(exception); - Object actual = handle(adapter, controller, this.postExchange, Duration.ofMillis(100) - , "threadWithArg", String.class).getReturnValue(); + Object actual = handle(adapter, controller, this.postExchange, Duration.ofMillis(1000), + "threadWithArg", String.class).getReturnValue(); assertThat(actual).isEqualTo(expected); } @@ -242,6 +242,7 @@ class ControllerAdviceTests { } } + @Controller static class TestController { @@ -249,7 +250,6 @@ class ControllerAdviceTests { private Throwable exception; - void setValidator(Validator validator) { this.validator = validator; } @@ -258,7 +258,6 @@ class ControllerAdviceTests { this.exception = exception; } - @InitBinder public void initDataBinder(WebDataBinder dataBinder) { if (this.validator != null) { @@ -291,6 +290,7 @@ class ControllerAdviceTests { } } + @ControllerAdvice @Order(1) static class OneControllerAdvice { @@ -323,6 +323,7 @@ class ControllerAdviceTests { } } + @ControllerAdvice @Order(2) static class SecondControllerAdvice { diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverTests.java index c42bb55f151..bb6ca0e3d38 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverTests.java @@ -28,6 +28,7 @@ import reactor.test.StepVerifier; import org.springframework.core.MethodParameter; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.format.support.DefaultFormattingConversionService; +import org.springframework.lang.Nullable; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.support.ConfigurableWebBindingInitializer; import org.springframework.web.reactive.BindingContext; @@ -58,7 +59,6 @@ class RequestParamMethodArgumentResolverTests { @BeforeEach void setup() throws Exception { - ReactiveAdapterRegistry adapterRegistry = ReactiveAdapterRegistry.getSharedInstance(); this.resolver = new RequestParamMethodArgumentResolver(null, adapterRegistry, true); @@ -70,7 +70,6 @@ class RequestParamMethodArgumentResolverTests { @Test void supportsParameter() { - MethodParameter param = this.testMethod.annot(requestParam().notRequired("bar")).arg(String.class); assertThat(this.resolver.supportsParameter(param)).isTrue(); @@ -105,12 +104,12 @@ class RequestParamMethodArgumentResolverTests { @Test void doesNotSupportReactiveWrapper() { - assertThatIllegalStateException().isThrownBy(() -> - this.resolver.supportsParameter(this.testMethod.annot(requestParam()).arg(Mono.class, String.class))) - .withMessageStartingWith("RequestParamMethodArgumentResolver does not support reactive type wrapper"); - assertThatIllegalStateException().isThrownBy(() -> - this.resolver.supportsParameter(this.testMethod.annotNotPresent(RequestParam.class).arg(Mono.class, String.class))) - .withMessageStartingWith("RequestParamMethodArgumentResolver does not support reactive type wrapper"); + assertThatIllegalStateException() + .isThrownBy(() -> this.resolver.supportsParameter(this.testMethod.annot(requestParam()).arg(Mono.class, String.class))) + .withMessageStartingWith("RequestParamMethodArgumentResolver does not support reactive type wrapper"); + assertThatIllegalStateException() + .isThrownBy(() -> this.resolver.supportsParameter(this.testMethod.annotNotPresent(RequestParam.class).arg(Mono.class, String.class))) + .withMessageStartingWith("RequestParamMethodArgumentResolver does not support reactive type wrapper"); } @Test @@ -125,19 +124,15 @@ class RequestParamMethodArgumentResolverTests { MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(String[].class); MockServerHttpRequest request = MockServerHttpRequest.get("/path?name=foo&name=bar").build(); Object result = resolve(param, MockServerWebExchange.from(request)); - boolean condition = result instanceof String[]; - assertThat(condition).isTrue(); - assertThat((String[]) result).isEqualTo(new String[] {"foo", "bar"}); + assertThat(result).isEqualTo(new String[] {"foo", "bar"}); } - @Test // gh-32577 + @Test // gh-32577 void resolveStringArrayWithEmptyArraySuffix() { MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(String[].class); MockServerHttpRequest request = MockServerHttpRequest.get("/path?name[]=foo&name[]=bar").build(); Object result = resolve(param, MockServerWebExchange.from(request)); - boolean condition = result instanceof String[]; - assertThat(condition).isTrue(); - assertThat((String[]) result).isEqualTo(new String[] {"foo", "bar"}); + assertThat(result).isEqualTo(new String[] {"foo", "bar"}); } @Test @@ -146,7 +141,7 @@ class RequestParamMethodArgumentResolverTests { assertThat(resolve(param, MockServerWebExchange.from(MockServerHttpRequest.get("/")))).isEqualTo("bar"); } - @Test // SPR-17050 + @Test // SPR-17050 public void resolveAndConvertNullValue() { MethodParameter param = this.testMethod .annot(requestParam().notRequired()) @@ -156,7 +151,6 @@ class RequestParamMethodArgumentResolverTests { @Test void missingRequestParam() { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(String[].class); Mono mono = this.resolver.resolveArgument(param, this.bindContext, exchange); @@ -221,6 +215,7 @@ class RequestParamMethodArgumentResolverTests { } + @Nullable private Object resolve(MethodParameter parameter, ServerWebExchange exchange) { return this.resolver.resolveArgument(parameter, this.bindContext, exchange).block(Duration.ZERO); }