From aeef95938e11ce0025a0721f0e3d277946d65cc3 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 5 Jul 2019 17:07:01 +0200 Subject: [PATCH 1/6] SpringValidatorAdapter's ObjectError subclasses are serializable Closes gh-23181 --- .../SpringValidatorAdapterTests.java | 11 +- .../SpringValidatorAdapter.java | 135 +++++++++++++----- .../SpringValidatorAdapterTests.java | 11 +- src/checkstyle/checkstyle.xml | 5 +- 4 files changed, 113 insertions(+), 49 deletions(-) diff --git a/spring-context-support/src/test/java/org/springframework/validation/beanvalidation2/SpringValidatorAdapterTests.java b/spring-context-support/src/test/java/org/springframework/validation/beanvalidation2/SpringValidatorAdapterTests.java index e488036d403..86bcdd675db 100644 --- a/spring-context-support/src/test/java/org/springframework/validation/beanvalidation2/SpringValidatorAdapterTests.java +++ b/spring-context-support/src/test/java/org/springframework/validation/beanvalidation2/SpringValidatorAdapterTests.java @@ -50,6 +50,7 @@ import org.springframework.beans.BeanWrapper; import org.springframework.beans.BeanWrapperImpl; import org.springframework.context.support.StaticMessageSource; import org.springframework.util.ObjectUtils; +import org.springframework.util.SerializationTestUtils; import org.springframework.validation.BeanPropertyBindingResult; import org.springframework.validation.FieldError; import org.springframework.validation.beanvalidation.SpringValidatorAdapter; @@ -89,7 +90,7 @@ public class SpringValidatorAdapterTests { } @Test // SPR-13406 - public void testNoStringArgumentValue() { + public void testNoStringArgumentValue() throws Exception { TestBean testBean = new TestBean(); testBean.setPassword("pass"); testBean.setConfirmPassword("pass"); @@ -104,10 +105,11 @@ public class SpringValidatorAdapterTests { assertThat(messageSource.getMessage(error, Locale.ENGLISH), is("Size of Password must be between 8 and 128")); assertTrue(error.contains(ConstraintViolation.class)); assertThat(error.unwrap(ConstraintViolation.class).getPropertyPath().toString(), is("password")); + assertThat(SerializationTestUtils.serializeAndDeserialize(error.toString()), is(error.toString())); } @Test // SPR-13406 - public void testApplyMessageSourceResolvableToStringArgumentValueWithResolvedLogicalFieldName() { + public void testApplyMessageSourceResolvableToStringArgumentValueWithResolvedLogicalFieldName() throws Exception { TestBean testBean = new TestBean(); testBean.setPassword("password"); testBean.setConfirmPassword("PASSWORD"); @@ -122,6 +124,7 @@ public class SpringValidatorAdapterTests { assertThat(messageSource.getMessage(error, Locale.ENGLISH), is("Password must be same value as Password(Confirm)")); assertTrue(error.contains(ConstraintViolation.class)); assertThat(error.unwrap(ConstraintViolation.class).getPropertyPath().toString(), is("password")); + assertThat(SerializationTestUtils.serializeAndDeserialize(error.toString()), is(error.toString())); } @Test // SPR-13406 @@ -518,10 +521,10 @@ public class SpringValidatorAdapterTests { .addPropertyNode(f.getName()) .addConstraintViolation(); } - } catch (IllegalAccessException ex) { + } + catch (IllegalAccessException ex) { throw new IllegalStateException(ex); } - }); return fieldsErros.isEmpty(); } diff --git a/spring-context/src/main/java/org/springframework/validation/beanvalidation/SpringValidatorAdapter.java b/spring-context/src/main/java/org/springframework/validation/beanvalidation/SpringValidatorAdapter.java index a55fff3d690..a6b6b5e46f1 100644 --- a/spring-context/src/main/java/org/springframework/validation/beanvalidation/SpringValidatorAdapter.java +++ b/spring-context/src/main/java/org/springframework/validation/beanvalidation/SpringValidatorAdapter.java @@ -165,27 +165,15 @@ public class SpringValidatorAdapter implements SmartValidator, javax.validation. String nestedField = bindingResult.getNestedPath() + field; if (nestedField.isEmpty()) { String[] errorCodes = bindingResult.resolveMessageCodes(errorCode); - ObjectError error = new ObjectError( - errors.getObjectName(), errorCodes, errorArgs, violation.getMessage()) { - @Override - public boolean shouldRenderDefaultMessage() { - return requiresMessageFormat(violation); - } - }; - error.wrap(violation); + ObjectError error = new ViolationObjectError( + errors.getObjectName(), errorCodes, errorArgs, violation, this); bindingResult.addError(error); } else { Object rejectedValue = getRejectedValue(field, violation, bindingResult); String[] errorCodes = bindingResult.resolveMessageCodes(errorCode, field); - FieldError error = new FieldError(errors.getObjectName(), nestedField, - rejectedValue, false, errorCodes, errorArgs, violation.getMessage()) { - @Override - public boolean shouldRenderDefaultMessage() { - return requiresMessageFormat(violation); - } - }; - error.wrap(violation); + FieldError error = new ViolationFieldError(errors.getObjectName(), nestedField, + rejectedValue, errorCodes, errorArgs, violation, this); bindingResult.addError(error); } } @@ -307,29 +295,6 @@ public class SpringValidatorAdapter implements SmartValidator, javax.validation. return new DefaultMessageSourceResolvable(codes, field); } - /** - * Indicate whether this violation's interpolated message has remaining - * placeholders and therefore requires {@link java.text.MessageFormat} - * to be applied to it. Called for a Bean Validation defined message - * (coming out {@code ValidationMessages.properties}) when rendered - * as the default message in Spring's MessageSource. - *

The default implementation considers a Spring-style "{0}" placeholder - * for the field name as an indication for {@link java.text.MessageFormat}. - * Any other placeholder or escape syntax occurrences are typically a - * mismatch, coming out of regex pattern values or the like. Note that - * standard Bean Validation does not support "{0}" style placeholders at all; - * this is a feature typically used in Spring MessageSource resource bundles. - * @param violation the Bean Validation constraint violation, including - * BV-defined interpolation of named attribute references in its message - * @return {@code true} if {@code java.text.MessageFormat} is to be applied, - * or {@code false} if the violation's message should be used as-is - * @since 5.1.8 - * @see #getArgumentsForConstraint - */ - protected boolean requiresMessageFormat(ConstraintViolation violation) { - return violation.getMessage().contains("{0}"); - } - /** * Extract the rejected value behind the given constraint violation, * for exposure through the Spring errors representation. @@ -354,6 +319,33 @@ public class SpringValidatorAdapter implements SmartValidator, javax.validation. return invalidValue; } + /** + * Indicate whether this violation's interpolated message has remaining + * placeholders and therefore requires {@link java.text.MessageFormat} + * to be applied to it. Called for a Bean Validation defined message + * (coming out {@code ValidationMessages.properties}) when rendered + * as the default message in Spring's MessageSource. + *

The default implementation considers a Spring-style "{0}" placeholder + * for the field name as an indication for {@link java.text.MessageFormat}. + * Any other placeholder or escape syntax occurrences are typically a + * mismatch, coming out of regex pattern values or the like. Note that + * standard Bean Validation does not support "{0}" style placeholders at all; + * this is a feature typically used in Spring MessageSource resource bundles. + * @param violation the Bean Validation constraint violation, including + * BV-defined interpolation of named attribute references in its message + * @return {@code true} if {@code java.text.MessageFormat} is to be applied, + * or {@code false} if the violation's message should be used as-is + * @since 5.1.8 + * @see #getArgumentsForConstraint + */ + protected boolean requiresMessageFormat(ConstraintViolation violation) { + return containsSpringStylePlaceholder(violation.getMessage()); + } + + private static boolean containsSpringStylePlaceholder(@Nullable String message) { + return (message != null && message.contains("{0}")); + } + //--------------------------------------------------------------------- // Implementation of JSR-303 Validator interface @@ -436,6 +428,71 @@ public class SpringValidatorAdapter implements SmartValidator, javax.validation. public String getDefaultMessage() { return this.resolvableString; } + + @Override + public String toString() { + return this.resolvableString; + } + } + + + /** + * Subclass of {@code ObjectError} with Spring-style default message rendering. + */ + @SuppressWarnings("serial") + private static class ViolationObjectError extends ObjectError implements Serializable { + + @Nullable + private transient SpringValidatorAdapter adapter; + + @Nullable + private transient ConstraintViolation violation; + + public ViolationObjectError(String objectName, String[] codes, Object[] arguments, + ConstraintViolation violation, SpringValidatorAdapter adapter) { + + super(objectName, codes, arguments, violation.getMessage()); + this.adapter = adapter; + this.violation = violation; + wrap(violation); + } + + @Override + public boolean shouldRenderDefaultMessage() { + return (this.adapter != null && this.violation != null ? + this.adapter.requiresMessageFormat(this.violation) : + containsSpringStylePlaceholder(getDefaultMessage())); + } + } + + + /** + * Subclass of {@code FieldError} with Spring-style default message rendering. + */ + @SuppressWarnings("serial") + private static class ViolationFieldError extends FieldError implements Serializable { + + @Nullable + private transient SpringValidatorAdapter adapter; + + @Nullable + private transient ConstraintViolation violation; + + public ViolationFieldError(String objectName, String field, @Nullable Object rejectedValue, String[] codes, + Object[] arguments, ConstraintViolation violation, SpringValidatorAdapter adapter) { + + super(objectName, field, rejectedValue, false, codes, arguments, violation.getMessage()); + this.adapter = adapter; + this.violation = violation; + wrap(violation); + } + + @Override + public boolean shouldRenderDefaultMessage() { + return (this.adapter != null && this.violation != null ? + this.adapter.requiresMessageFormat(this.violation) : + containsSpringStylePlaceholder(getDefaultMessage())); + } } } diff --git a/spring-context/src/test/java/org/springframework/validation/beanvalidation/SpringValidatorAdapterTests.java b/spring-context/src/test/java/org/springframework/validation/beanvalidation/SpringValidatorAdapterTests.java index 482eba19a89..414769cba4b 100644 --- a/spring-context/src/test/java/org/springframework/validation/beanvalidation/SpringValidatorAdapterTests.java +++ b/spring-context/src/test/java/org/springframework/validation/beanvalidation/SpringValidatorAdapterTests.java @@ -48,6 +48,7 @@ import org.springframework.beans.BeanWrapper; import org.springframework.beans.BeanWrapperImpl; import org.springframework.context.support.StaticMessageSource; import org.springframework.util.ObjectUtils; +import org.springframework.util.SerializationTestUtils; import org.springframework.validation.BeanPropertyBindingResult; import org.springframework.validation.FieldError; @@ -86,7 +87,7 @@ public class SpringValidatorAdapterTests { } @Test // SPR-13406 - public void testNoStringArgumentValue() { + public void testNoStringArgumentValue() throws Exception { TestBean testBean = new TestBean(); testBean.setPassword("pass"); testBean.setConfirmPassword("pass"); @@ -101,10 +102,11 @@ public class SpringValidatorAdapterTests { assertThat(messageSource.getMessage(error, Locale.ENGLISH), is("Size of Password must be between 8 and 128")); assertTrue(error.contains(ConstraintViolation.class)); assertThat(error.unwrap(ConstraintViolation.class).getPropertyPath().toString(), is("password")); + assertThat(SerializationTestUtils.serializeAndDeserialize(error.toString()), is(error.toString())); } @Test // SPR-13406 - public void testApplyMessageSourceResolvableToStringArgumentValueWithResolvedLogicalFieldName() { + public void testApplyMessageSourceResolvableToStringArgumentValueWithResolvedLogicalFieldName() throws Exception { TestBean testBean = new TestBean(); testBean.setPassword("password"); testBean.setConfirmPassword("PASSWORD"); @@ -119,6 +121,7 @@ public class SpringValidatorAdapterTests { assertThat(messageSource.getMessage(error, Locale.ENGLISH), is("Password must be same value as Password(Confirm)")); assertTrue(error.contains(ConstraintViolation.class)); assertThat(error.unwrap(ConstraintViolation.class).getPropertyPath().toString(), is("password")); + assertThat(SerializationTestUtils.serializeAndDeserialize(error.toString()), is(error.toString())); } @Test // SPR-13406 @@ -473,10 +476,10 @@ public class SpringValidatorAdapterTests { .addPropertyNode(f.getName()) .addConstraintViolation(); } - } catch (IllegalAccessException ex) { + } + catch (IllegalAccessException ex) { throw new IllegalStateException(ex); } - }); return fieldsErros.isEmpty(); } diff --git a/src/checkstyle/checkstyle.xml b/src/checkstyle/checkstyle.xml index ccd01ad779b..c9fcc6195e2 100644 --- a/src/checkstyle/checkstyle.xml +++ b/src/checkstyle/checkstyle.xml @@ -1,7 +1,6 @@ - @@ -46,7 +45,9 @@ - + + + From 56cc0d02e980f3a6b1fd8dff2c10077620ea20d5 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 5 Jul 2019 17:07:22 +0200 Subject: [PATCH 2/6] Bean destruction exceptions consistently logged at warn level Closes gh-23200 --- .../factory/support/DefaultSingletonBeanRegistry.java | 6 +++--- .../beans/factory/support/DisposableBeanAdapter.java | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java index 6cec5d5014f..185a5807631 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -571,8 +571,8 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements bean.destroy(); } catch (Throwable ex) { - if (logger.isInfoEnabled()) { - logger.info("Destroy method on bean with name '" + beanName + "' threw an exception", ex); + if (logger.isWarnEnabled()) { + logger.warn("Destruction of bean with name '" + beanName + "' threw an exception", ex); } } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java index 9d1ce3466cb..36768418cdd 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java @@ -261,10 +261,10 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable { catch (Throwable ex) { String msg = "Invocation of destroy method failed on bean with name '" + this.beanName + "'"; if (logger.isDebugEnabled()) { - logger.info(msg, ex); + logger.warn(msg, ex); } else { - logger.info(msg + ": " + ex); + logger.warn(msg + ": " + ex); } } } @@ -343,14 +343,14 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable { String msg = "Destroy method '" + this.destroyMethodName + "' on bean with name '" + this.beanName + "' threw an exception"; if (logger.isDebugEnabled()) { - logger.info(msg, ex.getTargetException()); + logger.warn(msg, ex.getTargetException()); } else { - logger.info(msg + ": " + ex.getTargetException()); + logger.warn(msg + ": " + ex.getTargetException()); } } catch (Throwable ex) { - logger.info("Failed to invoke destroy method '" + this.destroyMethodName + + logger.warn("Failed to invoke destroy method '" + this.destroyMethodName + "' on bean with name '" + this.beanName + "'", ex); } } From a1eae42fd08243e727790734e93c3fe4cc5e4ff7 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 5 Jul 2019 17:07:44 +0200 Subject: [PATCH 3/6] Expose implementation method for annotation introspection purposes Closes gh-23210 --- .../support/ReflectivePropertyAccessor.java | 32 +++++++++------ .../expression/spel/SpelReproTests.java | 41 +++++++++++++++---- 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java index 3ef76790e9b..99c763da52b 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java @@ -138,6 +138,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { // The readerCache will only contain gettable properties (let's not worry about setters for now). Property property = new Property(type, method, null); TypeDescriptor typeDescriptor = new TypeDescriptor(property); + method = ClassUtils.getInterfaceMethodIfPossible(method); this.readerCache.put(cacheKey, new InvokerPair(method, typeDescriptor)); this.typeDescriptorCache.put(cacheKey, typeDescriptor); return true; @@ -180,6 +181,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { // The readerCache will only contain gettable properties (let's not worry about setters for now). Property property = new Property(type, method, null); TypeDescriptor typeDescriptor = new TypeDescriptor(property); + method = ClassUtils.getInterfaceMethodIfPossible(method); invoker = new InvokerPair(method, typeDescriptor); this.lastReadInvokerPair = invoker; this.readerCache.put(cacheKey, invoker); @@ -239,6 +241,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { // Treat it like a property Property property = new Property(type, null, method); TypeDescriptor typeDescriptor = new TypeDescriptor(property); + method = ClassUtils.getInterfaceMethodIfPossible(method); this.writerCache.put(cacheKey, method); this.typeDescriptorCache.put(cacheKey, typeDescriptor); return true; @@ -287,6 +290,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { if (method == null) { method = findSetterForProperty(name, type, target); if (method != null) { + method = ClassUtils.getInterfaceMethodIfPossible(method); cachedMember = method; this.writerCache.put(cacheKey, cachedMember); } @@ -414,13 +418,24 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { method.getParameterCount() == numberOfParams && (!mustBeStatic || Modifier.isStatic(method.getModifiers())) && (requiredReturnTypes.isEmpty() || requiredReturnTypes.contains(method.getReturnType()))) { - return ClassUtils.getInterfaceMethodIfPossible(method); + return method; } } } return null; } + /** + * Return class methods ordered with non-bridge methods appearing higher. + */ + private Method[] getSortedMethods(Class clazz) { + return this.sortedMethodsCache.computeIfAbsent(clazz, key -> { + Method[] methods = key.getMethods(); + Arrays.sort(methods, (o1, o2) -> (o1.isBridge() == o2.isBridge() ? 0 : (o1.isBridge() ? 1 : -1))); + return methods; + }); + } + /** * Determine whether the given {@code Method} is a candidate for property access * on an instance of the given target class. @@ -434,17 +449,6 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { return true; } - /** - * Return class methods ordered with non-bridge methods appearing higher. - */ - private Method[] getSortedMethods(Class clazz) { - return this.sortedMethodsCache.computeIfAbsent(clazz, key -> { - Method[] methods = key.getMethods(); - Arrays.sort(methods, (o1, o2) -> (o1.isBridge() == o2.isBridge() ? 0 : (o1.isBridge() ? 1 : -1))); - return methods; - }); - } - /** * Return the method suffixes for a given property name. The default implementation * uses JavaBean conventions with additional support for properties of the form 'xY' @@ -536,7 +540,9 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { if (method == null) { method = findGetterForProperty(name, clazz, target); if (method != null) { - invocationTarget = new InvokerPair(method, new TypeDescriptor(new MethodParameter(method, -1))); + TypeDescriptor typeDescriptor = new TypeDescriptor(new MethodParameter(method, -1)); + method = ClassUtils.getInterfaceMethodIfPossible(method); + invocationTarget = new InvokerPair(method, typeDescriptor); ReflectionUtils.makeAccessible(method); this.readerCache.put(cacheKey, invocationTarget); } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java index eba067e0c1e..e37867e5465 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -60,10 +60,18 @@ import org.springframework.expression.spel.support.ReflectivePropertyAccessor; import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.expression.spel.support.StandardTypeLocator; import org.springframework.expression.spel.testresources.le.div.mod.reserved.Reserver; +import org.springframework.lang.Nullable; import org.springframework.util.ObjectUtils; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.*; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * Reproduction tests cornering various reported SpEL issues. @@ -1213,9 +1221,13 @@ public class SpelReproTests extends AbstractExpressionTests { public void SPR9994_bridgeMethods() throws Exception { ReflectivePropertyAccessor accessor = new ReflectivePropertyAccessor(); StandardEvaluationContext context = new StandardEvaluationContext(); - Object target = new GenericImplementation(); + GenericImplementation target = new GenericImplementation(); + accessor.write(context, target, "property", "1"); + assertEquals(1, target.value); TypedValue value = accessor.read(context, target, "property"); + assertEquals(1, value.getValue()); assertEquals(Integer.class, value.getTypeDescriptor().getType()); + assertTrue(value.getTypeDescriptor().getAnnotations().length > 0); } @Test @@ -1224,6 +1236,7 @@ public class SpelReproTests extends AbstractExpressionTests { StandardEvaluationContext context = new StandardEvaluationContext(); Object target = new OnlyBridgeMethod(); TypedValue value = accessor.read(context, target, "property"); + assertNull(value.getValue()); assertEquals(Integer.class, value.getTypeDescriptor().getType()); } @@ -1232,7 +1245,7 @@ public class SpelReproTests extends AbstractExpressionTests { ExpressionParser parser = new SpelExpressionParser(); StandardEvaluationContext evaluationContext = new StandardEvaluationContext(new BooleanHolder()); Class valueType = parser.parseExpression("simpleProperty").getValueType(evaluationContext); - assertNotNull(valueType); + assertEquals(Boolean.class, valueType); } @Test @@ -1240,7 +1253,7 @@ public class SpelReproTests extends AbstractExpressionTests { ExpressionParser parser = new SpelExpressionParser(); StandardEvaluationContext evaluationContext = new StandardEvaluationContext(new BooleanHolder()); Object value = parser.parseExpression("simpleProperty").getValue(evaluationContext); - assertNotNull(value); + assertEquals(Boolean.class, value.getClass()); } @Test @@ -1248,7 +1261,7 @@ public class SpelReproTests extends AbstractExpressionTests { ExpressionParser parser = new SpelExpressionParser(); StandardEvaluationContext evaluationContext = new StandardEvaluationContext(new BooleanHolder()); Class valueType = parser.parseExpression("primitiveProperty").getValueType(evaluationContext); - assertNotNull(valueType); + assertEquals(Boolean.class, valueType); } @Test @@ -1256,7 +1269,7 @@ public class SpelReproTests extends AbstractExpressionTests { ExpressionParser parser = new SpelExpressionParser(); StandardEvaluationContext evaluationContext = new StandardEvaluationContext(new BooleanHolder()); Object value = parser.parseExpression("primitiveProperty").getValue(evaluationContext); - assertNotNull(value); + assertEquals(Boolean.class, value.getClass()); } @Test @@ -2220,15 +2233,25 @@ public class SpelReproTests extends AbstractExpressionTests { private interface GenericInterface { + void setProperty(T value); + T getProperty(); } private static class GenericImplementation implements GenericInterface { + int value; + + @Override + public void setProperty(Integer value) { + this.value = value; + } + @Override + @Nullable public Integer getProperty() { - return null; + return this.value; } } From 16deb3c50f4f7d5a55d3e10482bda7bba6587742 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 5 Jul 2019 17:08:08 +0200 Subject: [PATCH 4/6] Defensively register ReactiveReturnValueHandler for messaging methods Closes gh-23207 --- .../HandlerMethodReturnValueHandlerComposite.java | 10 +++++----- .../invocation/ReactiveReturnValueHandler.java | 7 ++++--- .../support/SimpAnnotationMethodMessageHandler.java | 11 +++++++++-- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/HandlerMethodReturnValueHandlerComposite.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/HandlerMethodReturnValueHandlerComposite.java index 25665a816e0..645538e8273 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/HandlerMethodReturnValueHandlerComposite.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/HandlerMethodReturnValueHandlerComposite.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -26,7 +26,6 @@ import org.apache.commons.logging.LogFactory; import org.springframework.core.MethodParameter; import org.springframework.lang.Nullable; import org.springframework.messaging.Message; -import org.springframework.util.Assert; import org.springframework.util.concurrent.ListenableFuture; /** @@ -139,9 +138,10 @@ public class HandlerMethodReturnValueHandlerComposite implements AsyncHandlerMet @Nullable public ListenableFuture toListenableFuture(Object returnValue, MethodParameter returnType) { HandlerMethodReturnValueHandler handler = getReturnValueHandler(returnType); - Assert.state(handler instanceof AsyncHandlerMethodReturnValueHandler, - "AsyncHandlerMethodReturnValueHandler required"); - return ((AsyncHandlerMethodReturnValueHandler) handler).toListenableFuture(returnValue, returnType); + if (handler instanceof AsyncHandlerMethodReturnValueHandler) { + return ((AsyncHandlerMethodReturnValueHandler) handler).toListenableFuture(returnValue, returnType); + } + return null; } } diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/ReactiveReturnValueHandler.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/ReactiveReturnValueHandler.java index 7fb0f669649..c71e8293875 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/ReactiveReturnValueHandler.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/ReactiveReturnValueHandler.java @@ -21,7 +21,6 @@ import reactor.core.publisher.Mono; import org.springframework.core.MethodParameter; import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; -import org.springframework.util.Assert; import org.springframework.util.concurrent.ListenableFuture; import org.springframework.util.concurrent.MonoToListenableFutureAdapter; @@ -60,8 +59,10 @@ public class ReactiveReturnValueHandler extends AbstractAsyncReturnValueHandler @Override public ListenableFuture toListenableFuture(Object returnValue, MethodParameter returnType) { ReactiveAdapter adapter = this.adapterRegistry.getAdapter(returnType.getParameterType(), returnValue); - Assert.state(adapter != null, () -> "No ReactiveAdapter found for " + returnType.getParameterType()); - return new MonoToListenableFutureAdapter<>(Mono.from(adapter.toPublisher(returnValue))); + if (adapter != null) { + return new MonoToListenableFutureAdapter<>(Mono.from(adapter.toPublisher(returnValue))); + } + return null; } } diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/annotation/support/SimpAnnotationMethodMessageHandler.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/annotation/support/SimpAnnotationMethodMessageHandler.java index c7a1c9c6c32..f1070381307 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/annotation/support/SimpAnnotationMethodMessageHandler.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/annotation/support/SimpAnnotationMethodMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -74,6 +74,7 @@ import org.springframework.messaging.support.MessageHeaderInitializer; import org.springframework.stereotype.Controller; import org.springframework.util.AntPathMatcher; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.util.CollectionUtils; import org.springframework.util.PathMatcher; import org.springframework.util.StringValueResolver; @@ -93,6 +94,10 @@ import org.springframework.validation.Validator; public class SimpAnnotationMethodMessageHandler extends AbstractMethodMessageHandler implements EmbeddedValueResolverAware, SmartLifecycle { + private static final boolean reactorPresent = ClassUtils.isPresent( + "reactor.core.publisher.Flux", SimpAnnotationMethodMessageHandler.class.getClassLoader()); + + private final SubscribableChannel clientInboundChannel; private final SimpMessageSendingOperations clientMessagingTemplate; @@ -328,7 +333,9 @@ public class SimpAnnotationMethodMessageHandler extends AbstractMethodMessageHan handlers.add(new ListenableFutureReturnValueHandler()); handlers.add(new CompletableFutureReturnValueHandler()); - handlers.add(new ReactiveReturnValueHandler()); + if (reactorPresent) { + handlers.add(new ReactiveReturnValueHandler()); + } // Annotation-based return value types From 53ebbb20f909de10033230cd51a223ccec7d4b41 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 5 Jul 2019 17:08:46 +0200 Subject: [PATCH 5/6] Upgrade to Netty 4.1.37 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 94a16fb30fd..b5383583a99 100644 --- a/build.gradle +++ b/build.gradle @@ -37,7 +37,7 @@ ext { junit5Version = "5.3.2" kotlinVersion = "1.2.71" log4jVersion = "2.11.2" - nettyVersion = "4.1.36.Final" + nettyVersion = "4.1.37.Final" reactorVersion = "Californium-SR9" rxjavaVersion = "1.3.8" rxjavaAdapterVersion = "1.2.1" From 7a7d4109ac00fdad8f2e7e9fa56d4d4712956e9e Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 5 Jul 2019 17:47:36 +0200 Subject: [PATCH 6/6] Polishing --- .../SpringValidatorAdapterTests.java | 32 +++++++++---------- .../SpringValidatorAdapterTests.java | 32 +++++++++---------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/spring-context-support/src/test/java/org/springframework/validation/beanvalidation2/SpringValidatorAdapterTests.java b/spring-context-support/src/test/java/org/springframework/validation/beanvalidation2/SpringValidatorAdapterTests.java index 86bcdd675db..cc42203a672 100644 --- a/spring-context-support/src/test/java/org/springframework/validation/beanvalidation2/SpringValidatorAdapterTests.java +++ b/spring-context-support/src/test/java/org/springframework/validation/beanvalidation2/SpringValidatorAdapterTests.java @@ -17,9 +17,11 @@ package org.springframework.validation.beanvalidation2; import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; import java.lang.annotation.Inherited; import java.lang.annotation.Repeatable; import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.Field; import java.util.ArrayList; @@ -55,8 +57,6 @@ import org.springframework.validation.BeanPropertyBindingResult; import org.springframework.validation.FieldError; import org.springframework.validation.beanvalidation.SpringValidatorAdapter; -import static java.lang.annotation.ElementType.*; -import static java.lang.annotation.RetentionPolicy.*; import static org.hamcrest.core.Is.*; import static org.hamcrest.core.StringContains.*; import static org.junit.Assert.*; @@ -327,8 +327,8 @@ public class SpringValidatorAdapterTests { @Documented @Constraint(validatedBy = {SameValidator.class}) - @Target({TYPE, ANNOTATION_TYPE}) - @Retention(RUNTIME) + @Target({ElementType.TYPE, ElementType.ANNOTATION_TYPE}) + @Retention(RetentionPolicy.RUNTIME) @Repeatable(SameGroup.class) @interface Same { @@ -342,8 +342,8 @@ public class SpringValidatorAdapterTests { String comparingField(); - @Target({TYPE, ANNOTATION_TYPE}) - @Retention(RUNTIME) + @Target({ElementType.TYPE, ElementType.ANNOTATION_TYPE}) + @Retention(RetentionPolicy.RUNTIME) @Documented @interface List { Same[] value(); @@ -353,8 +353,8 @@ public class SpringValidatorAdapterTests { @Documented @Inherited - @Retention(RUNTIME) - @Target({TYPE, ANNOTATION_TYPE}) + @Target({ElementType.TYPE, ElementType.ANNOTATION_TYPE}) + @Retention(RetentionPolicy.RUNTIME) @interface SameGroup { Same[] value(); @@ -490,7 +490,7 @@ public class SpringValidatorAdapterTests { @Constraint(validatedBy = AnythingValidator.class) - @Retention(RUNTIME) + @Retention(RetentionPolicy.RUNTIME) public @interface AnythingValid { String message() default "{AnythingValid.message}"; @@ -511,14 +511,14 @@ public class SpringValidatorAdapterTests { @Override public boolean isValid(Object value, ConstraintValidatorContext context) { - List fieldsErros = new ArrayList<>(); - Arrays.asList(value.getClass().getDeclaredFields()).forEach(f -> { - f.setAccessible(true); + List fieldsErrors = new ArrayList<>(); + Arrays.asList(value.getClass().getDeclaredFields()).forEach(field -> { + field.setAccessible(true); try { - if (!f.getName().equals(ID) && f.get(value) == null) { - fieldsErros.add(f); + if (!field.getName().equals(ID) && field.get(value) == null) { + fieldsErrors.add(field); context.buildConstraintViolationWithTemplate(context.getDefaultConstraintMessageTemplate()) - .addPropertyNode(f.getName()) + .addPropertyNode(field.getName()) .addConstraintViolation(); } } @@ -526,7 +526,7 @@ public class SpringValidatorAdapterTests { throw new IllegalStateException(ex); } }); - return fieldsErros.isEmpty(); + return fieldsErrors.isEmpty(); } } diff --git a/spring-context/src/test/java/org/springframework/validation/beanvalidation/SpringValidatorAdapterTests.java b/spring-context/src/test/java/org/springframework/validation/beanvalidation/SpringValidatorAdapterTests.java index 414769cba4b..92228522d50 100644 --- a/spring-context/src/test/java/org/springframework/validation/beanvalidation/SpringValidatorAdapterTests.java +++ b/spring-context/src/test/java/org/springframework/validation/beanvalidation/SpringValidatorAdapterTests.java @@ -17,9 +17,11 @@ package org.springframework.validation.beanvalidation; import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; import java.lang.annotation.Inherited; import java.lang.annotation.Repeatable; import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.Field; import java.util.ArrayList; @@ -52,8 +54,6 @@ import org.springframework.util.SerializationTestUtils; import org.springframework.validation.BeanPropertyBindingResult; import org.springframework.validation.FieldError; -import static java.lang.annotation.ElementType.*; -import static java.lang.annotation.RetentionPolicy.*; import static org.hamcrest.core.Is.*; import static org.hamcrest.core.StringContains.*; import static org.junit.Assert.*; @@ -282,8 +282,8 @@ public class SpringValidatorAdapterTests { @Documented @Constraint(validatedBy = {SameValidator.class}) - @Target({TYPE, ANNOTATION_TYPE}) - @Retention(RUNTIME) + @Target({ElementType.TYPE, ElementType.ANNOTATION_TYPE}) + @Retention(RetentionPolicy.RUNTIME) @Repeatable(SameGroup.class) @interface Same { @@ -297,8 +297,8 @@ public class SpringValidatorAdapterTests { String comparingField(); - @Target({TYPE, ANNOTATION_TYPE}) - @Retention(RUNTIME) + @Target({ElementType.TYPE, ElementType.ANNOTATION_TYPE}) + @Retention(RetentionPolicy.RUNTIME) @Documented @interface List { Same[] value(); @@ -308,8 +308,8 @@ public class SpringValidatorAdapterTests { @Documented @Inherited - @Retention(RUNTIME) - @Target({TYPE, ANNOTATION_TYPE}) + @Target({ElementType.TYPE, ElementType.ANNOTATION_TYPE}) + @Retention(RetentionPolicy.RUNTIME) @interface SameGroup { Same[] value(); @@ -445,7 +445,7 @@ public class SpringValidatorAdapterTests { @Constraint(validatedBy = AnythingValidator.class) - @Retention(RUNTIME) + @Retention(RetentionPolicy.RUNTIME) public @interface AnythingValid { String message() default "{AnythingValid.message}"; @@ -466,14 +466,14 @@ public class SpringValidatorAdapterTests { @Override public boolean isValid(Object value, ConstraintValidatorContext context) { - List fieldsErros = new ArrayList<>(); - Arrays.asList(value.getClass().getDeclaredFields()).forEach(f -> { - f.setAccessible(true); + List fieldsErrors = new ArrayList<>(); + Arrays.asList(value.getClass().getDeclaredFields()).forEach(field -> { + field.setAccessible(true); try { - if (!f.getName().equals(ID) && f.get(value) == null) { - fieldsErros.add(f); + if (!field.getName().equals(ID) && field.get(value) == null) { + fieldsErrors.add(field); context.buildConstraintViolationWithTemplate(context.getDefaultConstraintMessageTemplate()) - .addPropertyNode(f.getName()) + .addPropertyNode(field.getName()) .addConstraintViolation(); } } @@ -481,7 +481,7 @@ public class SpringValidatorAdapterTests { throw new IllegalStateException(ex); } }); - return fieldsErros.isEmpty(); + return fieldsErrors.isEmpty(); } }