HV-1853 Fix inconsistency of getter method detection in GetterPropertySelectionStrategy

rsvasyliev · gsmet · commit 9bb5db3e620d · 2022-06-15T14:26:20.000+01:00
diff --git a/documentation/src/test/java/org/hibernate/validator/referenceguide/chapter12/getterselectionstrategy/FluentGetterPropertySelectionStrategy.java b/documentation/src/test/java/org/hibernate/validator/referenceguide/chapter12/getterselectionstrategy/FluentGetterPropertySelectionStrategy.java
@@ -7,6 +7,7 @@
 import java.util.Collections;
 import java.util.Optional;
 import java.util.Set;
+import java.util.List;
 import java.util.stream.Collectors;
 
 import org.hibernate.validator.spi.properties.ConstrainableExecutable;
@@ -36,9 +37,9 @@ public Optional<String> getProperty(ConstrainableExecutable executable) {
 	}
 
 	@Override
-	public Set<String> getGetterMethodNameCandidates(String propertyName) {
+	public List<String> getGetterMethodNameCandidates(String propertyName) {
 		// As method name == property name, there always is just one possible name for a method
-		return Collections.singleton( propertyName );
+		return Collections.singletonList( propertyName );
 	}
 }
 //end::include[]
diff --git a/engine/src/main/java/org/hibernate/validator/internal/properties/DefaultGetterPropertySelectionStrategy.java b/engine/src/main/java/org/hibernate/validator/internal/properties/DefaultGetterPropertySelectionStrategy.java
@@ -7,8 +7,8 @@
 package org.hibernate.validator.internal.properties;
 
 import java.lang.invoke.MethodHandles;
+import java.util.List;
 import java.util.Optional;
-import java.util.Set;
 
 import org.hibernate.validator.internal.util.CollectionHelper;
 import org.hibernate.validator.internal.util.Contracts;
@@ -54,10 +54,10 @@ public Optional<String> getProperty(ConstrainableExecutable executable) {
 	}
 
 	@Override
-	public Set<String> getGetterMethodNameCandidates(String propertyName) {
+	public List<String> getGetterMethodNameCandidates(String propertyName) {
 		Contracts.assertNotEmpty( propertyName, "Name of a property must not be empty" );
 
-		Set<String> nameCandidates = CollectionHelper.newHashSet( GETTER_PREFIXES.length );
+		List<String> nameCandidates = CollectionHelper.newArrayList( GETTER_PREFIXES.length );
 		for ( String prefix : GETTER_PREFIXES ) {
 			nameCandidates.add( prefix + Character.toUpperCase( propertyName.charAt( 0 ) ) + propertyName.substring( 1 ) );
 		}
diff --git a/engine/src/main/java/org/hibernate/validator/internal/util/privilegedactions/GetMethodFromGetterNameCandidates.java b/engine/src/main/java/org/hibernate/validator/internal/util/privilegedactions/GetMethodFromGetterNameCandidates.java
@@ -8,7 +8,7 @@
 
 import java.lang.reflect.Method;
 import java.security.PrivilegedAction;
-import java.util.Set;
+import java.util.List;
 
 /**
  * Returns the method with the specified property name or {@code null} if it does not exist. This action will
@@ -23,20 +23,20 @@
 public final class GetMethodFromGetterNameCandidates implements PrivilegedAction<Method> {
 
 	private final Class<?> clazz;
-	private final Set<String> getterNameCandidates;
+	private final List<String> getterNameCandidates;
 	private final boolean lookForMethodsInHierarchy;
 
-	private GetMethodFromGetterNameCandidates(Class<?> clazz, Set<String> getterNameCandidates, boolean lookForMethodsInHierarchy) {
+	private GetMethodFromGetterNameCandidates(Class<?> clazz, List<String> getterNameCandidates, boolean lookForMethodsInHierarchy) {
 		this.clazz = clazz;
 		this.getterNameCandidates = getterNameCandidates;
 		this.lookForMethodsInHierarchy = lookForMethodsInHierarchy;
 	}
 
-	public static GetMethodFromGetterNameCandidates action(Class<?> clazz, Set<String> getterNameCandidates) {
+	public static GetMethodFromGetterNameCandidates action(Class<?> clazz, List<String> getterNameCandidates) {
 		return new GetMethodFromGetterNameCandidates( clazz, getterNameCandidates, false );
 	}
 
-	public static GetMethodFromGetterNameCandidates action(Class<?> clazz, Set<String> possibleMethodNames, boolean lookForMethodsInHierarchy) {
+	public static GetMethodFromGetterNameCandidates action(Class<?> clazz, List<String> possibleMethodNames, boolean lookForMethodsInHierarchy) {
 		return new GetMethodFromGetterNameCandidates( clazz, possibleMethodNames, lookForMethodsInHierarchy );
 	}
 
diff --git a/engine/src/main/java/org/hibernate/validator/spi/properties/GetterPropertySelectionStrategy.java b/engine/src/main/java/org/hibernate/validator/spi/properties/GetterPropertySelectionStrategy.java
@@ -6,6 +6,7 @@
  */
 package org.hibernate.validator.spi.properties;
 
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 
@@ -42,6 +43,6 @@ public interface GetterPropertySelectionStrategy {
 	 *
 	 * @return the {@link Set} of possible getter names
 	 */
-	Set<String> getGetterMethodNameCandidates(String propertyName);
+	List<String> getGetterMethodNameCandidates(String propertyName);
 
 }
diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/xml/MultipleGetterCandidates.java b/engine/src/test/java/org/hibernate/validator/test/internal/xml/MultipleGetterCandidates.java
@@ -0,0 +1,33 @@
+/*
+ * 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-1853.
+ *
+ * @author Roman Vasilyev
+ */
+public class MultipleGetterCandidates {
+	private Integer property1 = 1;
+	private Integer property2 = 1;
+
+	public boolean hasProperty1() {
+		return property1 != null;
+	}
+
+	public Integer getProperty1() {
+		return property1;
+	}
+
+	public boolean hasProperty2() {
+		return property2 != null;
+	}
+
+	public Integer getProperty2() {
+		return property2;
+	}
+}
diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/xml/XmlMappingTest.java b/engine/src/test/java/org/hibernate/validator/test/internal/xml/XmlMappingTest.java
@@ -24,6 +24,7 @@
 import jakarta.validation.ValidationException;
 import jakarta.validation.Validator;
 import jakarta.validation.ValidatorFactory;
+import jakarta.validation.constraints.Min;
 import jakarta.validation.constraints.NotNull;
 import jakarta.validation.constraints.Size;
 import jakarta.validation.executable.ExecutableType;
@@ -431,4 +432,19 @@ public void test_constraint_is_applied_to_type_argument_of_inherited_getter() {
 		);
 	}
 
+	@Test
+	public void test_right_getter_used_when_there_are_multiple_getter_candidates() {
+		final Configuration<?> configuration = ValidatorUtil.getConfiguration();
+		configuration.addMapping( XmlMappingTest.class.getResourceAsStream( "hv-1853-mapping.xml" ) );
+
+		final ValidatorFactory validatorFactory = configuration.buildValidatorFactory();
+		final Validator validator = validatorFactory.getValidator();
+
+		Set<ConstraintViolation<MultipleGetterCandidates>> violations = validator.validate( new MultipleGetterCandidates() );
+		assertThat( violations ).containsOnlyViolations(
+				violationOf( Min.class ).withProperty( "property1" ),
+				violationOf( Min.class ).withProperty( "property2" )
+		);
+	}
+
 }
diff --git a/engine/src/test/java/org/hibernate/validator/test/properties/GetterPropertySelectionStrategyTest.java b/engine/src/test/java/org/hibernate/validator/test/properties/GetterPropertySelectionStrategyTest.java
@@ -232,8 +232,8 @@ public Optional<String> getProperty(ConstrainableExecutable executable) {
 		}
 
 		@Override
-		public Set<String> getGetterMethodNameCandidates(String propertyName) {
-			return Collections.singleton( propertyName );
+		public List<String> getGetterMethodNameCandidates(String propertyName) {
+			return Collections.singletonList( propertyName );
 		}
 	}
 
@@ -252,8 +252,8 @@ public Optional<String> getProperty(ConstrainableExecutable executable) {
 		}
 
 		@Override
-		public Set<String> getGetterMethodNameCandidates(String propertyName) {
-			return Collections.singleton( "foo" + Character.toUpperCase( propertyName.charAt( 0 ) ) + propertyName.substring( 1 ) );
+		public List<String> getGetterMethodNameCandidates(String propertyName) {
+			return Collections.singletonList( "foo" + Character.toUpperCase( propertyName.charAt( 0 ) ) + propertyName.substring( 1 ) );
 		}
 	}
 
diff --git a/engine/src/test/resources/org/hibernate/validator/test/internal/xml/hv-1853-mapping.xml b/engine/src/test/resources/org/hibernate/validator/test/internal/xml/hv-1853-mapping.xml
@@ -0,0 +1,29 @@
+<?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="https://jakarta.ee/xml/ns/validation/mapping"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:schemaLocation="https://jakarta.ee/xml/ns/validation/mapping
+            https://jakarta.ee/xml/ns/validation/validation-mapping-3.0.xsd"
+        version="3.0">
+
+    <default-package>org.hibernate.validator.internal.xml</default-package>
+
+    <bean class="org.hibernate.validator.test.internal.xml.MultipleGetterCandidates">
+        <getter name="property1">
+            <constraint annotation="jakarta.validation.constraints.Min">
+                <element name="value">2</element>
+            </constraint>
+        </getter>
+        <getter name="property2">
+            <constraint annotation="jakarta.validation.constraints.Min">
+                <element name="value">2</element>
+            </constraint>
+        </getter>
+    </bean>
+</constraint-mappings>

Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,7 @@`
`7`	`7`	`import java.util.Collections;`
`8`	`8`	`import java.util.Optional;`
`9`	`9`	`import java.util.Set;`
	`10`	`+import java.util.List;`
`10`	`11`	`import java.util.stream.Collectors;`
`11`	`12`
`12`	`13`	`import org.hibernate.validator.spi.properties.ConstrainableExecutable;`
`@@ -36,9 +37,9 @@ public Optional<String> getProperty(ConstrainableExecutable executable) {`
`36`	`37`	`}`
`37`	`38`
`38`	`39`	`@Override`
`39`		`- public Set<String> getGetterMethodNameCandidates(String propertyName) {`
	`40`	`+ public List<String> getGetterMethodNameCandidates(String propertyName) {`
`40`	`41`	`// As method name == property name, there always is just one possible name for a method`
`41`		`- return Collections.singleton( propertyName );`
	`42`	`+ return Collections.singletonList( propertyName );`
`42`	`43`	`}`
`43`	`44`	`}`
`44`	`45`	`//end::include[]`
Original file line number	Diff line number	Diff line change
`@@ -6,6 +6,7 @@`
`6`	`6`	`*/`
`7`	`7`	`package org.hibernate.validator.spi.properties;`
`8`	`8`
	`9`	`+import java.util.List;`
`9`	`10`	`import java.util.Optional;`
`10`	`11`	`import java.util.Set;`
`11`	`12`
`@@ -42,6 +43,6 @@ public interface GetterPropertySelectionStrategy {`
`42`	`43`	`*`
`43`	`44`	`* @return the {@link Set} of possible getter names`
`44`	`45`	`*/`
`45`		`- Set<String> getGetterMethodNameCandidates(String propertyName);`
	`46`	`+ List<String> getGetterMethodNameCandidates(String propertyName);`
`46`	`47`
`47`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -232,8 +232,8 @@ public Optional<String> getProperty(ConstrainableExecutable executable) {`
`232`	`232`	`}`
`233`	`233`
`234`	`234`	`@Override`
`235`		`- public Set<String> getGetterMethodNameCandidates(String propertyName) {`
`236`		`- return Collections.singleton( propertyName );`
	`235`	`+ public List<String> getGetterMethodNameCandidates(String propertyName) {`
	`236`	`+ return Collections.singletonList( propertyName );`
`237`	`237`	`}`
`238`	`238`	`}`
`239`	`239`
`@@ -252,8 +252,8 @@ public Optional<String> getProperty(ConstrainableExecutable executable) {`
`252`	`252`	`}`
`253`	`253`
`254`	`254`	`@Override`
`255`		`- public Set<String> getGetterMethodNameCandidates(String propertyName) {`
`256`		`- return Collections.singleton( "foo" + Character.toUpperCase( propertyName.charAt( 0 ) ) + propertyName.substring( 1 ) );`
	`255`	`+ public List<String> getGetterMethodNameCandidates(String propertyName) {`
	`256`	`+ return Collections.singletonList( "foo" + Character.toUpperCase( propertyName.charAt( 0 ) ) + propertyName.substring( 1 ) );`
`257`	`257`	`}`
`258`	`258`	`}`
`259`	`259`