From 4f1f9f9710f7e1e2daad2539185af04a20922989 Mon Sep 17 00:00:00 2001
From: Kevin Wise <kevin.wise@keap.com>
Date: Fri, 22 Apr 2022 19:14:23 -0700
Subject: [PATCH] fix: validation group inheritance, group validation

per Bean validation specification:
- validation groups should consider inheritance
- validation groups must be interfaces
---
 .../validator/DefaultValidator.java           | 38 ++++++++++++++++---
 .../validator/ValidatorGroupsSpec.groovy      | 14 +++++++
 2 files changed, 47 insertions(+), 5 deletions(-)

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 7b713670330..e42d3313bb7 100644
--- a/validation/src/main/java/io/micronaut/validation/validator/DefaultValidator.java
+++ b/validation/src/main/java/io/micronaut/validation/validator/DefaultValidator.java
@@ -97,7 +97,7 @@
 @Requires(property = ValidatorConfiguration.ENABLED, value = StringUtils.TRUE, defaultValue = StringUtils.TRUE)
 public class DefaultValidator implements Validator, ExecutableMethodValidator, ReactiveValidator, AnnotatedElementValidator, BeanDefinitionValidator {
 
-    private static final List<Class> DEFAULT_GROUPS = Collections.singletonList(Default.class);
+    private static final List<Class<?>> DEFAULT_GROUPS = Collections.singletonList(Default.class);
     private final ConstraintValidatorRegistry constraintValidatorRegistry;
     private final ClockProvider clockProvider;
     private final ValueExtractorRegistry valueExtractorRegistry;
@@ -272,7 +272,6 @@ public Set<String> validatedAnnotatedElement(@NonNull AnnotatedElement element,
         final DefaultConstraintValidatorContext context = new DefaultConstraintValidatorContext();
         try {
             context.addPropertyNode(element.getName(), null);
-            //noinspection unchecked
             validatePropertyInternal(
                     null,
                     element,
@@ -286,7 +285,6 @@ public Set<String> validatedAnnotatedElement(@NonNull AnnotatedElement element,
             context.removeLast();
         }
 
-        //noinspection unchecked
         return Collections.unmodifiableSet(overallViolations.stream()
                 .map(ConstraintViolation::getMessage).collect(Collectors.toSet()));
     }
@@ -1807,7 +1805,7 @@ private <T> DefaultConstraintViolation<T> createIntrospectionConstraintViolation
     private final class DefaultConstraintValidatorContext implements ConstraintValidatorContext {
         final Set<Object> validatedObjects = new HashSet<>(20);
         final PathImpl currentPath;
-        final List<Class> groups;
+        final List<Class<?>> groups;
         String messageTemplate = null;
 
         private <T> DefaultConstraintValidatorContext(T object, Class<?>... groups) {
@@ -1819,7 +1817,13 @@ private <T> DefaultConstraintValidatorContext(T object, PathImpl path, Class<?>.
                 validatedObjects.add(object);
             }
             if (ArrayUtils.isNotEmpty(groups)) {
-                this.groups = Arrays.asList(groups);
+                sanityCheckGroups(groups);
+
+                List<Class<?>> groupList = new ArrayList<>();
+                for (Class<?> group: groups) {
+                    addInheritedGroups(group, groupList);
+                }
+                this.groups = Collections.unmodifiableList(groupList);
             } else {
                 this.groups = DEFAULT_GROUPS;
             }
@@ -1831,6 +1835,30 @@ private DefaultConstraintValidatorContext(Class<?>... groups) {
             this(null, groups);
         }
 
+        private void sanityCheckGroups(Class<?>[] groups) {
+            ArgumentUtils.requireNonNull("groups", groups);
+
+            for (Class<?> clazz : groups) {
+                if (clazz == null) {
+                    throw new IllegalArgumentException("Validation groups must be non-null");
+                }
+                if (!clazz.isInterface()) {
+                    throw new IllegalArgumentException(
+                        "Validation groups must be interfaces. " + clazz.getName() + " is not.");
+                }
+            }
+        }
+
+        private void addInheritedGroups(Class<?> group, List<Class<?>> groups) {
+            if (!groups.contains(group)) {
+                groups.add(group);
+            }
+
+            for (Class<?> inheritedGroup : group.getInterfaces()) {
+                addInheritedGroups(inheritedGroup, groups);
+            }
+        }
+
         @NonNull
         @Override
         public ClockProvider getClockProvider() {
diff --git a/validation/src/test/groovy/io/micronaut/validation/validator/ValidatorGroupsSpec.groovy b/validation/src/test/groovy/io/micronaut/validation/validator/ValidatorGroupsSpec.groovy
index 2e71e5f9055..6245d530afc 100644
--- a/validation/src/test/groovy/io/micronaut/validation/validator/ValidatorGroupsSpec.groovy
+++ b/validation/src/test/groovy/io/micronaut/validation/validator/ValidatorGroupsSpec.groovy
@@ -37,9 +37,21 @@ class ValidatorGroupsSpec extends AbstractTypeElementSpec {
 
         when:
         violations = validator.validate(address, GroupThree, GroupTwo)
+        List messageTemplates = violations*.messageTemplate
 
         then:
         violations.size() == 2
+        messageTemplates.contains('{javax.validation.constraints.Size.message}')
+        messageTemplates.contains('different message')
+
+        when:
+        violations = validator.validate(address, InheritedGroup)
+        messageTemplates = violations*.messageTemplate
+
+        then:
+        violations.size() == 2
+        messageTemplates.contains('{javax.validation.constraints.Size.message}')
+        messageTemplates.contains('message for default')
     }
 
     void "test validate with default group"() {
@@ -126,6 +138,7 @@ interface GroupThree {}
 class Address {
     @NotBlank(groups = GroupOne)
     @NotBlank(groups = GroupThree, message = "different message")
+    @NotBlank(message = "message for default")
     @Size(min = 5, max = 20, groups = GroupTwo)
     String street
 }
@@ -133,6 +146,7 @@ class Address {
 interface GroupOne {}
 interface GroupTwo {}
 interface GroupThree {}
+interface InheritedGroup extends Default, GroupTwo {}
 
 @Introspected
 class AddressTwo {