HV-1261 Loading value extractors via service loader

engine/src/main/java/org/hibernate/validator/internal/engine/ConfigurationImpl.java

@@ -14,9 +14,13 @@
 import java.io.InputStream;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.ServiceLoader;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import javax.validation.BootstrapConfiguration;
 import javax.validation.ClockProvider;
@@ -34,6 +38,7 @@
 import org.hibernate.validator.HibernateValidatorConfiguration;
 import org.hibernate.validator.cfg.ConstraintMapping;
 import org.hibernate.validator.internal.cfg.context.DefaultConstraintMapping;
+import org.hibernate.validator.internal.engine.cascading.ValueExtractorDescriptor;
 import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorFactoryImpl;
 import org.hibernate.validator.internal.engine.resolver.DefaultTraversableResolver;
 import org.hibernate.validator.internal.util.Contracts;
@@ -92,6 +97,7 @@ public class ConfigurationImpl implements HibernateValidatorConfiguration, Confi
 	private boolean failFast;
 	private ClassLoader externalClassLoader;
 	private final MethodValidationConfiguration.Builder methodValidationConfigurationBuilder = new MethodValidationConfiguration.Builder();
+	private final Map<ValueExtractorDescriptor.Key, ValueExtractorDescriptor> valueExtractors = new HashMap<>();
 
 	public ConfigurationImpl(BootstrapState state) {
 		this();
@@ -199,11 +205,14 @@ public HibernateValidatorConfiguration clockProvider(ClockProvider clockProvider
 	public HibernateValidatorConfiguration addValueExtractor(ValueExtractor<?> extractor) {
 		Contracts.assertNotNull( extractor, MESSAGES.parameterMustNotBeNull( "extractor" ) );
 
+		// TODO prevent duplicates
+
 		if ( log.isDebugEnabled() ) {
 			log.debug( "Adding value extractor " + extractor );
 		}
 
-		validationBootstrapParameters.addValueExtractor( extractor );
+		ValueExtractorDescriptor descriptor = new ValueExtractorDescriptor( extractor );
+		valueExtractors.put( descriptor.getKey(), descriptor );
 
 		return this;
 	}
@@ -291,6 +300,7 @@ public HibernateValidatorConfiguration externalClassLoader(ClassLoader externalC
 
 	@Override
 	public final ValidatorFactory buildValidatorFactory() {
+		loadValueExtractorsFromServiceLoader();
 		parseValidationXml();
 		ValidatorFactory factory = null;
 		try {
@@ -392,7 +402,11 @@ public ClockProvider getClockProvider() {
 
 	@Override
 	public Set<ValueExtractor<?>> getValueExtractors() {
-		return validationBootstrapParameters.getValueExtractors();
+		return validationBootstrapParameters.getValueExtractors()
+				.values()
+				.stream()
+				.map( ValueExtractorDescriptor::getValueExtractor )
+				.collect( Collectors.toSet() );
 	}
 
 	@Override
@@ -472,6 +486,24 @@ private void parseValidationXml() {
 			);
 			applyXmlSettings( xmlParameters );
 		}
+
+		for ( ValueExtractorDescriptor valueExtractorDescriptor : valueExtractors.values() ) {
+			validationBootstrapParameters.addValueExtractor( valueExtractorDescriptor.getValueExtractor() );
+		}
+	}
+
+	private void loadValueExtractorsFromServiceLoader() {
+		@SuppressWarnings("rawtypes")
+		ServiceLoader<ValueExtractor> loader = ServiceLoader.load(
+				ValueExtractor.class,
+				externalClassLoader != null ? externalClassLoader : getClass().getClassLoader()
+		);
+
+		@SuppressWarnings("rawtypes")
+		Iterator<ValueExtractor> extractors = loader.iterator();
+		while ( extractors.hasNext() ) {
+			validationBootstrapParameters.addValueExtractor( extractors.next() );
+		}
 	}
 
 	private void applyXmlSettings(ValidationBootstrapParameters xmlParameters) {
@@ -521,7 +553,9 @@ private void applyXmlSettings(ValidationBootstrapParameters xmlParameters) {
 			}
 		}
 
-		validationBootstrapParameters.addAllNonExistingValueExtractors( xmlParameters.getValueExtractors() );
+		for ( ValueExtractorDescriptor extractor : xmlParameters.getValueExtractors().values() ) {
+			validationBootstrapParameters.addValueExtractor( extractor.getValueExtractor() );
+		}
 
 		validationBootstrapParameters.addAllMappings( xmlParameters.getMappings() );
 		configurationStreams.addAll( xmlParameters.getMappings() );

engine/src/main/java/org/hibernate/validator/internal/xml/ValidationBootstrapParameters.java

@@ -16,7 +16,6 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 import javax.validation.BootstrapConfiguration;
 import javax.validation.ClockProvider;
@@ -29,6 +28,7 @@
 import javax.validation.valueextraction.ValueExtractor;
 
 import org.hibernate.validator.internal.engine.cascading.ValueExtractorDescriptor;
+import org.hibernate.validator.internal.engine.cascading.ValueExtractorDescriptor.Key;
 import org.hibernate.validator.internal.util.logging.Log;
 import org.hibernate.validator.internal.util.logging.LoggerFactory;
 import org.hibernate.validator.internal.util.privilegedactions.LoadClass;
@@ -142,32 +142,15 @@ public void setClockProvider(ClockProvider clockProvider) {
 		this.clockProvider = clockProvider;
 	}
 
-	public Set<ValueExtractor<?>> getValueExtractors() {
-		return valueExtractors.values()
-				.stream()
-				.map( ValueExtractorDescriptor::getValueExtractor )
-				.collect( Collectors.toSet() );
+	public Map<Key, ValueExtractorDescriptor> getValueExtractors() {
+		return valueExtractors;
 	}
 
 	public void addValueExtractor(ValueExtractor<?> valueExtractor) {
-		// TODO raise error if already there
 		ValueExtractorDescriptor descriptor = new ValueExtractorDescriptor( valueExtractor );
 		valueExtractors.put( descriptor.getKey(), descriptor );
 	}
 
-	/**
-	 * Adds all those extractors from the given input which are not for a type and type parameter handled by any of the
-	 * already added extractors.
-	 */
-	public void addAllNonExistingValueExtractors(Iterable<ValueExtractor<?>> valueExtractors) {
-		for ( ValueExtractor<?> valueExtractor : valueExtractors ) {
-			ValueExtractorDescriptor descriptor = new ValueExtractorDescriptor( valueExtractor );
-			if ( !this.valueExtractors.containsKey( descriptor.getKey() ) ) {
-				this.valueExtractors.put( descriptor.getKey(), descriptor );
-			}
-		}
-	}
-
 	@SuppressWarnings("unchecked")
 	private void setProviderClass(String providerFqcn, ClassLoader externalClassLoader) {
 		if ( providerFqcn != null ) {

engine/src/test/java/org/hibernate/validator/test/internal/engine/cascaded/CustomValueExtractorTest.java

@@ -106,6 +106,19 @@ public void canUseCustomValueExtractorPerValidatorForMultimaps() throws Exceptio
 		assertCorrectPropertyPaths( violations, "addressByType[work].multimap_value", "addressByType[work].multimap_value" );
 	}
 
+	@Test
+	@TestForIssue(jiraKey = "HV-1261")
+	public void canUseValueExtractorGivenViaServiceLoader() {
+		CustomerWithOptionalAddress bob = new CustomerWithOptionalAddress();
+
+		Validator validator = Validation.buildDefaultValidatorFactory()
+				.getValidator();
+
+		Set<ConstraintViolation<CustomerWithOptionalAddress>> violations = validator.validate( bob );
+
+		assertCorrectPropertyPaths( violations, "address" );
+	}
+
 	@Test(expectedExceptions = ConstraintDeclarationException.class, expectedExceptionsMessageRegExp = "HV000197.*")
 	public void missingCustomExtractorThrowsException() throws Exception {
 		Cinema cinema = new Cinema();
@@ -126,14 +139,22 @@ public void valueExtractorPrecedenceIsAppliedCorrectly() {
 			() -> {
 				CustomerWithOptionalAddress bob = new CustomerWithOptionalAddress();
 
+				Validator validator = Validation.buildDefaultValidatorFactory()
+						.getValidator();
+
+				Set<ConstraintViolation<CustomerWithOptionalAddress>> violations = validator.validate( bob );
+
+				// validation.xml overrides service loader
+				assertCorrectPropertyPaths( violations, "address.1" );
+
 				ValidatorFactory validatorFactory = Validation.byDefaultProvider()
 						.configure()
 						.addValueExtractor( new GuavaOptionalValueExtractor2() )
 						.buildValidatorFactory();
 
-				Validator validator = validatorFactory.getValidator();
+				validator = validatorFactory.getValidator();
 
-				Set<ConstraintViolation<CustomerWithOptionalAddress>> violations = validator.validate( bob );
+				violations = validator.validate( bob );
 
 				// VF overrides validation.xml
 				assertCorrectPropertyPaths( violations, "address.2" );

engine/src/test/java/org/hibernate/validator/test/internal/engine/cascaded/GuavaOptionalValueExtractor.java

@@ -0,0 +1,23 @@
+/*
+ * 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.engine.cascaded;
+
+import javax.validation.valueextraction.ExtractedValue;
+import javax.validation.valueextraction.ValueExtractor;
+
+import com.google.common.base.Optional;
+
+/**
+ * @author Gunnar Morling
+ */
+public class GuavaOptionalValueExtractor implements ValueExtractor<Optional<@ExtractedValue ?>> {
+
+	@Override
+	public void extractValues(Optional<@ExtractedValue ?> originalValue, ValueExtractor.ValueReceiver receiver) {
+		receiver.value( null, originalValue.isPresent() ? originalValue.get() : null );
+	}
+}

engine/src/test/resources/META-INF/services/javax.validation.valueextraction.ValueExtractor

@@ -0,0 +1 @@
+org.hibernate.validator.test.internal.engine.cascaded.GuavaOptionalValueExtractor