HV-1534 Allow getter constraints to be specified for subclasses in XML configuration

It was possible in Hibernate Validator 4.3 and got somehow broken during
the 5 development cycle.

It's not exactly something we would support nowadays but as some users
were relying on it and it's not complicating the code, for now, we can
try to support it, hoping it won't be in the way later.

Author: RADickinson

engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/PropertyMetaData.java

@@ -33,6 +33,7 @@
 import org.hibernate.validator.internal.metadata.descriptor.PropertyDescriptorImpl;
 import org.hibernate.validator.internal.metadata.facets.Cascadable;
 import org.hibernate.validator.internal.metadata.location.ConstraintLocation;
+import org.hibernate.validator.internal.metadata.location.GetterConstraintLocation;
 import org.hibernate.validator.internal.metadata.location.TypeArgumentConstraintLocation;
 import org.hibernate.validator.internal.metadata.raw.ConstrainedElement;
 import org.hibernate.validator.internal.metadata.raw.ConstrainedElement.ConstrainedElementKind;
@@ -262,7 +263,13 @@ private MetaConstraint<?> withGetterLocation(ConstraintLocation getterConstraint
 
 			// fast track if it's a regular constraint
 			if ( !(constraint.getLocation() instanceof TypeArgumentConstraintLocation) ) {
-				converted = getterConstraintLocation;
+				// Change the constraint location to a GetterConstraintLocation if it is not already one
+				if ( constraint.getLocation() instanceof GetterConstraintLocation ) {
+					converted = constraint.getLocation();
+				}
+				else {
+					converted = getterConstraintLocation;
+				}
 			}
 			else {
 				Deque<ConstraintLocation> locationStack = new ArrayDeque<>();
@@ -283,7 +290,13 @@ private MetaConstraint<?> withGetterLocation(ConstraintLocation getterConstraint
 				// 2. beginning at the root, transform each location so it references the transformed delegate
 				for ( ConstraintLocation location : locationStack ) {
 					if ( !(location instanceof TypeArgumentConstraintLocation) ) {
-						converted = getterConstraintLocation;
+						// Change the constraint location to a GetterConstraintLocation if it is not already one
+						if ( location instanceof GetterConstraintLocation ) {
+							converted = location;
+						}
+						else {
+							converted = getterConstraintLocation;
+						}
 					}
 					else {
 						converted = ConstraintLocation.forTypeArgument(

engine/src/main/java/org/hibernate/validator/internal/metadata/location/ConstraintLocation.java

@@ -43,8 +43,29 @@ static ConstraintLocation forField(Field field) {
 		return new FieldConstraintLocation( field );
 	}
 
+	/**
+	 * Create a new {@link GetterConstraintLocation} for the given getter method.
+	 *
+	 * @param getter The getter method being constrained
+	 * @return A new GetterConstraintLocation
+	 */
 	static ConstraintLocation forGetter(Method getter) {
-		return new GetterConstraintLocation( getter );
+		return new GetterConstraintLocation( getter.getDeclaringClass(), getter );
+	}
+
+	/**
+	 * Create a new {@link GetterConstraintLocation} for the given declaring class and getter method.
+	 * <p>
+	 * This provides an alternative to {@link ConstraintLocation#forGetter(Method)} where the given declaring class is usually a sub-class of the
+	 * actual class on which the getter method is declared. This is provided to support XML mapping configurations used to specify constraints on
+	 * subclasses for inherited getter methods.
+	 *
+	 * @param declaringClass The class on which the constraint is defined.
+	 * @param getter The getter method being constrained.
+	 * @return A new GetterConstraintLocation
+	 */
+	static ConstraintLocation forGetter(Class<?> declaringClass, Method getter ) {
+		return new GetterConstraintLocation( declaringClass, getter );
 	}
 
 	static ConstraintLocation forTypeArgument(ConstraintLocation delegate, TypeVariable<?> typeParameter, Type typeOfAnnotatedElement) {

engine/src/main/java/org/hibernate/validator/internal/metadata/location/GetterConstraintLocation.java

@@ -44,17 +44,26 @@ public class GetterConstraintLocation implements ConstraintLocation {
 	 */
 	private final Type typeForValidatorResolution;
 
+	/**
+	 * The class of the method for which the constraint was defined.
+	 * <p>
+	 * It is usually the same as the declaring class of the method itself, except in the XML case when a user could
+	 * declare a constraint for a specific subclass.
+	 */
+	private final Class<?> declaringClass;
+
 
-	GetterConstraintLocation(Method method) {
+	GetterConstraintLocation( Class<?> declaringClass, Method method ) {
 		this.method = method;
 		this.accessibleMethod = getAccessible( method );
 		this.propertyName = ReflectionHelper.getPropertyName( method );
 		this.typeForValidatorResolution = ReflectionHelper.boxedType( ReflectionHelper.typeOf( method ) );
+		this.declaringClass = declaringClass;
 	}
 
 	@Override
 	public Class<?> getDeclaringClass() {
-		return method.getDeclaringClass();
+		return declaringClass;
 	}
 
 	@Override

engine/src/main/java/org/hibernate/validator/internal/xml/mapping/ConstrainedGetterStaxBuilder.java

@@ -71,7 +71,7 @@ ConstrainedExecutable build(Class<?> beanClass, List<String> alreadyProcessedGet
 			alreadyProcessedGetterNames.add( mainAttributeValue );
 		}
 		Method getter = findGetter( beanClass, mainAttributeValue );
-		ConstraintLocation constraintLocation = ConstraintLocation.forGetter( getter );
+		ConstraintLocation constraintLocation = ConstraintLocation.forGetter( beanClass, getter );
 
 		Set<MetaConstraint<?>> metaConstraints = constraintTypeStaxBuilders.stream()
 				.map( builder -> builder.build( constraintLocation, java.lang.annotation.ElementType.METHOD, null ) )

engine/src/test/java/org/hibernate/validator/test/internal/xml/Child.java

@@ -0,0 +1,19 @@
+/*
+ * Hibernate Validator, declare and validate application constraints
+ *
+ * License: Apache License, Version 2.0
+ * See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>.
+ */
+package org.hibernate.validator.test.internal.xml;
+
+/**
+ * Test class for HV-1534.
+ *
+ * @author Rob Dickinson
+ */
+public class Child extends Parent {
+
+	public Child( String parentAttribute ) {
+		super( parentAttribute );
+	}
+}

engine/src/test/java/org/hibernate/validator/test/internal/xml/Parent.java

@@ -0,0 +1,37 @@
+/*
+ * Hibernate Validator, declare and validate application constraints
+ *
+ * License: Apache License, Version 2.0
+ * See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>.
+ */
+package org.hibernate.validator.test.internal.xml;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Test class for HV-1534.
+ *
+ * @author Rob Dickinson
+ */
+public class Parent {
+
+	private String parentAttribute = null;
+	private List<String> parentListAttribute = new ArrayList<>();
+
+	public Parent( String parentAttribute ) {
+		this.parentAttribute = parentAttribute;
+	}
+
+	public final String getParentAttribute() {
+		return parentAttribute;
+	}
+
+	public void addToListAttribute(String element) {
+		parentListAttribute.add( element );
+	}
+
+	public final List<String> getParentListAttribute() {
+		return parentListAttribute;
+	}
+}

engine/src/test/java/org/hibernate/validator/test/internal/xml/XmlMappingTest.java

@@ -9,6 +9,7 @@
 import static org.hibernate.validator.internal.util.CollectionHelper.asSet;
 import static org.hibernate.validator.testutil.ConstraintViolationAssert.assertNoViolations;
 import static org.hibernate.validator.testutil.ConstraintViolationAssert.assertThat;
+import static org.hibernate.validator.testutil.ConstraintViolationAssert.pathWith;
 import static org.hibernate.validator.testutil.ConstraintViolationAssert.violationOf;
 import static org.testng.Assert.assertEquals;
 
@@ -376,4 +377,59 @@ public void testCascadedValidation() {
 				violationOf( NotNull.class )
 		);
 	}
+
+	@Test
+	@TestForIssue(jiraKey = "HV-1534")
+	public void test_constraint_is_applied_to_inherited_getter() {
+		final Configuration<?> configuration = ValidatorUtil.getConfiguration();
+		configuration.addMapping( XmlMappingTest.class.getResourceAsStream( "hv-1534-mapping.xml" ) );
+
+		final ValidatorFactory validatorFactory = configuration.buildValidatorFactory();
+		final Validator validator = validatorFactory.getValidator();
+
+		Parent parent = new Parent( null );
+
+		Set<ConstraintViolation<Parent>> parentViolations = validator.validate( parent );
+
+		assertNoViolations( parentViolations );
+
+		Child child = new Child( null );
+
+		Set<ConstraintViolation<Child>> childViolations = validator.validate( child );
+
+		assertThat( childViolations ).containsOnlyViolations(
+				violationOf( NotNull.class ).withProperty( "parentAttribute" )
+		);
+	}
+
+	@Test
+	@TestForIssue(jiraKey = "HV-1534")
+	public void test_constraint_is_applied_to_type_argument_of_inherited_getter() {
+		final Configuration<?> configuration = ValidatorUtil.getConfiguration();
+		configuration.addMapping( XmlMappingTest.class.getResourceAsStream( "hv-1534-mapping.xml" ) );
+
+		final ValidatorFactory validatorFactory = configuration.buildValidatorFactory();
+		final Validator validator = validatorFactory.getValidator();
+
+		Parent parent = new Parent( "someValue" );
+		parent.addToListAttribute( null );
+
+		Set<ConstraintViolation<Parent>> parentViolations = validator.validate( parent );
+
+		assertNoViolations( parentViolations );
+
+		Child child = new Child( "someValue" );
+		child.addToListAttribute( null );
+
+		Set<ConstraintViolation<Child>> childViolations = validator.validate( child );
+
+		assertThat( childViolations ).containsOnlyViolations(
+				violationOf( NotNull.class )
+					.withPropertyPath(
+						pathWith()
+							.property( "parentListAttribute" )
+							.containerElement( "<list element>", true, null, 0, List.class, 0 ) )
+		);
+	}
+
 }

engine/src/test/resources/org/hibernate/validator/test/internal/xml/hv-1534-mapping.xml

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Hibernate Validator, declare and validate application constraints
+  ~
+  ~ License: Apache License, Version 2.0
+  ~ See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>.
+  -->
+<constraint-mappings
+        xmlns="http://xmlns.jcp.org/xml/ns/validation/mapping"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/validation/mapping
+            http://xmlns.jcp.org/xml/ns/validation/mapping/validation-mapping-2.0.xsd"
+        version="2.0">
+        
+    <default-package>org.hibernate.validator.internal.xml</default-package>
+
+    <bean class="org.hibernate.validator.test.internal.xml.Child" ignore-annotations="false">
+        <getter name="parentAttribute">
+            <constraint annotation="javax.validation.constraints.NotNull"/>
+        </getter>
+        <getter name="parentListAttribute">
+            <container-element-type type-argument-index="0">
+                <constraint annotation="javax.validation.constraints.NotNull"/>
+            </container-element-type>
+        </getter>
+    </bean>
+
+</constraint-mappings>