From 53f5d34198a591be805e24b44d6d046440a6a364 Mon Sep 17 00:00:00 2001 From: Alexey Trofimov Date: Mon, 11 Nov 2024 13:37:34 +0700 Subject: [PATCH 1/2] Fix error message when introspection is missing and method has generic signature Fixed #417 --- .../validation/PojoOptionalGetterSpec.java | 3 +- .../docs/validation/WrongValidationTest.java | 48 +++++++++++++++++++ .../validator/DefaultValidator.java | 10 ++-- 3 files changed, 56 insertions(+), 5 deletions(-) create mode 100644 test-suite/src/test/java/io/micronaut/docs/validation/WrongValidationTest.java diff --git a/test-suite/src/test/java/io/micronaut/docs/validation/PojoOptionalGetterSpec.java b/test-suite/src/test/java/io/micronaut/docs/validation/PojoOptionalGetterSpec.java index 6f31c804..4970c167 100644 --- a/test-suite/src/test/java/io/micronaut/docs/validation/PojoOptionalGetterSpec.java +++ b/test-suite/src/test/java/io/micronaut/docs/validation/PojoOptionalGetterSpec.java @@ -6,13 +6,12 @@ import jakarta.inject.Singleton; import jakarta.validation.Valid; import org.junit.jupiter.api.Test; -import spock.lang.Specification; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @Property(name = "spec.name", value = "PojoOptionalGetterSpec") @MicronautTest -class PojoOptionalGetterTest extends Specification { +class PojoOptionalGetterSpec { @Test void pojoCanHaveGetterWhichReturnsAnOptional(MockService service) { diff --git a/test-suite/src/test/java/io/micronaut/docs/validation/WrongValidationTest.java b/test-suite/src/test/java/io/micronaut/docs/validation/WrongValidationTest.java new file mode 100644 index 00000000..f00999ff --- /dev/null +++ b/test-suite/src/test/java/io/micronaut/docs/validation/WrongValidationTest.java @@ -0,0 +1,48 @@ +package io.micronaut.docs.validation; + +import io.micronaut.context.annotation.Property; +import io.micronaut.context.annotation.Requires; +import io.micronaut.core.annotation.Introspected; +import io.micronaut.test.extensions.junit5.annotation.MicronautTest; +import io.micronaut.validation.Validated; +import jakarta.inject.Inject; +import jakarta.inject.Singleton; +import jakarta.validation.ConstraintViolationException; +import jakarta.validation.Valid; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@Property(name = "spec.name", value = "WrongValidationTest") +@MicronautTest +class WrongValidationTest { + + @Inject + ValidatedService validatedService; + + @Test + void validateEvent() { + assertDoesNotThrow(() -> validatedService.send(new IntrospectedEvent())); + + var ex = assertThrows(ConstraintViolationException.class, () -> validatedService.send(new NonIntrospectedEvent())); + + assertTrue(ex.getMessage().contains("NonIntrospectedEvent")); + } + + @Requires(property = "spec.name", value = "WrongValidationTest") + @Singleton + @Validated + static class ValidatedService { + + void send(@Valid E event) { + + } + } + + @Introspected + static class IntrospectedEvent {} + + static class NonIntrospectedEvent {} +} \ No newline at end of file diff --git a/validation/src/main/java/io/micronaut/validation/validator/DefaultValidator.java b/validation/src/main/java/io/micronaut/validation/validator/DefaultValidator.java index 8141e639..f3f35814 100644 --- a/validation/src/main/java/io/micronaut/validation/validator/DefaultValidator.java +++ b/validation/src/main/java/io/micronaut/validation/validator/DefaultValidator.java @@ -1357,7 +1357,7 @@ private void propagateValidation(DefaultConstraintValidatorContext con final BeanIntrospection beanIntrospection = getBeanIntrospection(elementValue, elementType.getType()); if (beanIntrospection == null) { // Error if not introspected - ConstraintDescriptor constraintDescriptor = notIntrospectedConstraint(elementType); + ConstraintDescriptor constraintDescriptor = notIntrospectedConstraint(elementType, elementValue); DefaultConstraintViolation violation = createConstraintViolation(context, leftBean, elementValue, constraintDescriptor); context.addViolation(violation); return; @@ -1584,7 +1584,7 @@ public static String requireNonEmpty(String name, String value) { return value; } - private static ConstraintDescriptor notIntrospectedConstraint(Argument notIntrospectedArgument) { + private static ConstraintDescriptor notIntrospectedConstraint(Argument notIntrospectedArgument, E elementValue) { return new ConstraintDescriptor<>() { @Override @@ -1619,7 +1619,11 @@ public ConstraintTarget getValidationAppliesTo() { @Override public Map getAttributes() { - return Collections.singletonMap("type", notIntrospectedArgument.getType().getName()); + var argType = notIntrospectedArgument.getType().getName(); + if (notIntrospectedArgument.isTypeVariable()) { + argType = elementValue.getClass().getName(); + } + return Collections.singletonMap("type", argType); } @Override From 78d5f7d45e21d3a29dcb18db21f1146356d76e50 Mon Sep 17 00:00:00 2001 From: Alexey Trofimov Date: Tue, 12 Nov 2024 17:15:53 +0700 Subject: [PATCH 2/2] Add ability to disable automatically prepend property path in exception message. Fixed #286 --- .../ConstraintViolationExceptionUtil.java | 34 ++++++++++++ .../validation/ValidatingInterceptor.java | 17 ++++-- .../ConstraintExceptionHandler.java | 55 +++++++++++-------- .../validator/DefaultValidator.java | 10 +++- .../DefaultValidatorConfiguration.java | 21 ++++++- .../validator/ValidatorConfiguration.java | 9 +++ .../DisabledPrependPropertyPathSpec.groovy | 42 ++++++++++++++ 7 files changed, 154 insertions(+), 34 deletions(-) create mode 100644 validation/src/main/java/io/micronaut/validation/ConstraintViolationExceptionUtil.java create mode 100644 validation/src/test/groovy/io/micronaut/validation/validator/DisabledPrependPropertyPathSpec.groovy diff --git a/validation/src/main/java/io/micronaut/validation/ConstraintViolationExceptionUtil.java b/validation/src/main/java/io/micronaut/validation/ConstraintViolationExceptionUtil.java new file mode 100644 index 00000000..6b1568fb --- /dev/null +++ b/validation/src/main/java/io/micronaut/validation/ConstraintViolationExceptionUtil.java @@ -0,0 +1,34 @@ +package io.micronaut.validation; + +import jakarta.validation.ConstraintViolation; +import jakarta.validation.ConstraintViolationException; + +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Utility class for ConstraintViolationException. + * + * @since 4.9.0 + */ +public final class ConstraintViolationExceptionUtil { + + private ConstraintViolationExceptionUtil() { + } + + /** + * Method to create ConstraintViolationException with custom message + * + * @param prependPropertyPath prependPropertyPath configuration flag + * @param constraintViolations constraint violations + * @return ConstraintViolationException + */ + public static ConstraintViolationException createConstraintViolationException(boolean prependPropertyPath, Set> constraintViolations) { + var message = constraintViolations.stream() + .map(cv -> cv == null ? "null" : (prependPropertyPath ? cv.getPropertyPath() + ": " : "") + cv.getMessage()) + .collect(Collectors.joining(", ")); + + return new ConstraintViolationException(message, constraintViolations); + } + +} diff --git a/validation/src/main/java/io/micronaut/validation/ValidatingInterceptor.java b/validation/src/main/java/io/micronaut/validation/ValidatingInterceptor.java index c645280b..aa10a8ab 100644 --- a/validation/src/main/java/io/micronaut/validation/ValidatingInterceptor.java +++ b/validation/src/main/java/io/micronaut/validation/ValidatingInterceptor.java @@ -26,9 +26,9 @@ import io.micronaut.validation.validator.ExecutableMethodValidator; import io.micronaut.validation.validator.ReactiveValidator; import io.micronaut.validation.validator.Validator; +import io.micronaut.validation.validator.ValidatorConfiguration; import jakarta.inject.Singleton; import jakarta.validation.ConstraintViolation; -import jakarta.validation.ConstraintViolationException; import jakarta.validation.ValidatorFactory; import jakarta.validation.executable.ExecutableValidator; @@ -36,6 +36,8 @@ import java.util.Set; import java.util.concurrent.CompletionStage; +import static io.micronaut.validation.ConstraintViolationExceptionUtil.createConstraintViolationException; + /** * A {@link MethodInterceptor} that validates method invocations. * @@ -53,6 +55,7 @@ public class ValidatingInterceptor implements MethodInterceptor private final @Nullable ExecutableValidator executableValidator; private final @Nullable ExecutableMethodValidator micronautValidator; private final ConversionService conversionService; + private final boolean isPrependPropertyPath; /** * Creates ValidatingInterceptor from the validatorFactory. @@ -63,8 +66,10 @@ public class ValidatingInterceptor implements MethodInterceptor */ public ValidatingInterceptor(@Nullable Validator micronautValidator, @Nullable ValidatorFactory validatorFactory, - ConversionService conversionService) { + ConversionService conversionService, + ValidatorConfiguration validatorConfiguration) { this.conversionService = conversionService; + isPrependPropertyPath = validatorConfiguration.isPrependPropertyPath(); if (validatorFactory != null) { jakarta.validation.Validator validator = validatorFactory.getValidator(); @@ -103,7 +108,7 @@ public Object intercept(MethodInvocationContext context) { getValidationGroups(context) ); if (!constraintViolations.isEmpty()) { - throw new ConstraintViolationException(constraintViolations); + throw createConstraintViolationException(isPrependPropertyPath, constraintViolations); } } return validateReturnExecutableValidator(context, targetMethod); @@ -116,7 +121,7 @@ public Object intercept(MethodInvocationContext context) { context.getParameterValues(), getValidationGroups(context)); if (!constraintViolations.isEmpty()) { - throw new ConstraintViolationException(constraintViolations); + throw createConstraintViolationException(isPrependPropertyPath, constraintViolations); } } if (micronautValidator instanceof ReactiveValidator reactiveValidator) { @@ -157,7 +162,7 @@ private Object validateReturnMicronautValidator(MethodInvocationContext> { private final ErrorResponseProcessor responseProcessor; + private final ValidatorConfiguration validatorConfiguration; /** * Constructor. * @param responseProcessor Error Response Processor */ @Inject - public ConstraintExceptionHandler(ErrorResponseProcessor responseProcessor) { + public ConstraintExceptionHandler(ErrorResponseProcessor responseProcessor, ValidatorConfiguration validatorConfiguration) { this.responseProcessor = responseProcessor; + this.validatorConfiguration = validatorConfiguration; } @Override @@ -85,38 +88,42 @@ public HttpResponse handle(HttpRequest request, ConstraintViolationException */ protected String buildMessage(ConstraintViolation violation) { Path propertyPath = violation.getPropertyPath(); - StringBuilder message = new StringBuilder(); - Iterator i = propertyPath.iterator(); + var message = new StringBuilder(); - boolean firstNode = true; + if (validatorConfiguration.isPrependPropertyPath()) { + Iterator i = propertyPath.iterator(); - while (i.hasNext()) { - Path.Node node = i.next(); + boolean firstNode = true; - if (node.getKind() == ElementKind.METHOD || node.getKind() == ElementKind.CONSTRUCTOR) { - continue; - } + while (i.hasNext()) { + Path.Node node = i.next(); - if (node.isInIterable()) { - message.append('['); - if (node.getKey() != null) { - message.append(node.getKey()); - } else if (node.getIndex() != null) { - message.append(node.getIndex()); + if (node.getKind() == ElementKind.METHOD || node.getKind() == ElementKind.CONSTRUCTOR) { + continue; } - message.append(']'); - } - if (node.getKind() != ElementKind.CONTAINER_ELEMENT && node.getName() != null) { - if (!firstNode) { - message.append('.'); + + if (node.isInIterable()) { + message.append('['); + if (node.getKey() != null) { + message.append(node.getKey()); + } else if (node.getIndex() != null) { + message.append(node.getIndex()); + } + message.append(']'); } - message.append(node.getName()); + if (node.getKind() != ElementKind.CONTAINER_ELEMENT && node.getName() != null) { + if (!firstNode) { + message.append('.'); + } + message.append(node.getName()); + } + + firstNode = false; } - firstNode = false; + message.append(": "); } - - message.append(": ").append(violation.getMessage()); + message.append(violation.getMessage()); return message.toString(); } diff --git a/validation/src/main/java/io/micronaut/validation/validator/DefaultValidator.java b/validation/src/main/java/io/micronaut/validation/validator/DefaultValidator.java index f3f35814..f2f55e07 100644 --- a/validation/src/main/java/io/micronaut/validation/validator/DefaultValidator.java +++ b/validation/src/main/java/io/micronaut/validation/validator/DefaultValidator.java @@ -92,6 +92,8 @@ import java.util.function.Function; import java.util.stream.Collectors; +import static io.micronaut.validation.ConstraintViolationExceptionUtil.createConstraintViolationException; + /** * Default implementation of the {@link Validator} interface. * @@ -122,6 +124,7 @@ public class DefaultValidator implements private final ConversionService conversionService; private final BeanIntrospector beanIntrospector; private final InternalConstraintValidatorFactory constraintValidatorFactory; + private final boolean isPrependPropertyPath; /** * Default constructor. @@ -139,6 +142,7 @@ public DefaultValidator(@NonNull ValidatorConfiguration configuration) { this.conversionService = configuration.getConversionService(); this.beanIntrospector = configuration.getBeanIntrospector(); this.constraintValidatorFactory = (InternalConstraintValidatorFactory) configuration.getConstraintValidatorFactory(); + this.isPrependPropertyPath = configuration.isPrependPropertyPath(); } /** @@ -339,7 +343,7 @@ public T createValid(@NonNull Class beanType, Object... arguments) throws final Set> constraintViolations = validateConstructorParameters(introspection, arguments); if (!constraintViolations.isEmpty()) { - throw new ConstraintViolationException(constraintViolations); + throw createConstraintViolationException(isPrependPropertyPath, constraintViolations); } final T instance = introspection.instantiate(arguments); @@ -347,7 +351,7 @@ public T createValid(@NonNull Class beanType, Object... arguments) throws if (errors.isEmpty()) { return instance; } - throw new ConstraintViolationException(errors); + throw createConstraintViolationException(isPrependPropertyPath, errors); } @Override @@ -912,7 +916,7 @@ private CompletionStage instrumentCompletionStage(DefaultConstraintVal } if (!context.getOverallViolations().isEmpty()) { - throw new ConstraintViolationException(context.getOverallViolations()); + throw createConstraintViolationException(isPrependPropertyPath, context.getOverallViolations()); } return value; diff --git a/validation/src/main/java/io/micronaut/validation/validator/DefaultValidatorConfiguration.java b/validation/src/main/java/io/micronaut/validation/validator/DefaultValidatorConfiguration.java index ac3d561d..4a653884 100644 --- a/validation/src/main/java/io/micronaut/validation/validator/DefaultValidatorConfiguration.java +++ b/validation/src/main/java/io/micronaut/validation/validator/DefaultValidatorConfiguration.java @@ -38,8 +38,8 @@ import io.micronaut.validation.validator.extractors.DefaultValueExtractors; import io.micronaut.validation.validator.extractors.ValueExtractorDefinition; import io.micronaut.validation.validator.extractors.ValueExtractorRegistry; -import io.micronaut.validation.validator.messages.DefaultMessages; import io.micronaut.validation.validator.messages.DefaultMessageInterpolator; +import io.micronaut.validation.validator.messages.DefaultMessages; import jakarta.inject.Inject; import jakarta.validation.ClockProvider; import jakarta.validation.ConstraintValidatorFactory; @@ -113,6 +113,7 @@ public class DefaultValidatorConfiguration implements ValidatorConfiguration, To private BeanIntrospector beanIntrospector = BeanIntrospector.SHARED; private boolean enabled = true; + private boolean prependPropertyPath = true; /** * Sets the conversion service. @@ -173,6 +174,24 @@ public DefaultValidatorConfiguration setEnabled(boolean enabled) { return this; } + @Override + public boolean isPrependPropertyPath() { + return prependPropertyPath; + } + + /** + * If true, then the path to the property will be automatically added to the error message. + *

+ * Default: true + * + * @param prependPropertyPath If true, then the path to the property will be automatically added to the error message. + * @return this configuration + */ + public DefaultValidatorConfiguration setPrependPropertyPath(boolean prependPropertyPath) { + this.prependPropertyPath = prependPropertyPath; + return this; + } + /** * Sets the constraint validator registry to use. * diff --git a/validation/src/main/java/io/micronaut/validation/validator/ValidatorConfiguration.java b/validation/src/main/java/io/micronaut/validation/validator/ValidatorConfiguration.java index bf5e8b4b..e4c7981a 100644 --- a/validation/src/main/java/io/micronaut/validation/validator/ValidatorConfiguration.java +++ b/validation/src/main/java/io/micronaut/validation/validator/ValidatorConfiguration.java @@ -106,6 +106,15 @@ public interface ValidatorConfiguration extends ConversionServiceProvider { @NonNull ExecutionHandleLocator getExecutionHandleLocator(); + /** + * If true, then the path to the property will be automatically added to the error message. + *

+ * Default: true + * + * @return prependPropertyPath flag value + */ + boolean isPrependPropertyPath(); + /** * The bean introspector. * @return The introspector diff --git a/validation/src/test/groovy/io/micronaut/validation/validator/DisabledPrependPropertyPathSpec.groovy b/validation/src/test/groovy/io/micronaut/validation/validator/DisabledPrependPropertyPathSpec.groovy new file mode 100644 index 00000000..00931f45 --- /dev/null +++ b/validation/src/test/groovy/io/micronaut/validation/validator/DisabledPrependPropertyPathSpec.groovy @@ -0,0 +1,42 @@ +package io.micronaut.validation.validator + +import io.micronaut.context.ApplicationContext +import io.micronaut.validation.Validated +import jakarta.inject.Singleton +import jakarta.validation.Valid +import jakarta.validation.constraints.Max +import jakarta.validation.constraints.Min +import spock.lang.AutoCleanup +import spock.lang.Shared +import spock.lang.Specification + +class DisabledPrependPropertyPathSpec extends Specification { + + @Shared + @AutoCleanup + ApplicationContext applicationContext = ApplicationContext.run(["micronaut.validator.prepend-property-path": false]) + + void "test disabled prependPropertyPath"() { + when: + def service = applicationContext.getBean(MyService3) + def bean = new MyBeanWithPrimitives() + bean.number = 100 + service.myMethod2(bean) + then: + Exception e = thrown() + e.message == 'must be less than or equal to 20' + } +} + +@Validated +@Singleton +class MyService3 { + + @Min(10) + int myMethod1(@Max(5) int a) { + return a + } + + void myMethod2(@Valid MyBeanWithPrimitives bean) { + } +}