Avoid exposing several javax.validaton.Validator beans

This commit makes sure that the Spring `Validator` used by the MVC
layer doesn't expose a JSR-303 contract, if any.

The default implementation of the `mvcValidator` is
`LocalValidatorFactoryBean`. While this object is exposed as a Spring
`Validator` only, its runtime capabilities expose that contract as well
as the standard `Validator` and `ValidatorFactory` ones.

Concretely, if an auto-configuration is checking if a
`javax.validation.Validator` bean is missing, the condition will match
since we only know about "advertized types": beans haven't been created
yet so we can't inspect their runtime capabilities. Since the condition
match, we will auto-configure a bean. At runtime though, we're no longer
ale to inject a `javax.validation.Validator` by type since two candidates
are available.

This commit introduces `SpringValidatorAdapterWrapper`, a wrapper class
on any `SpringValidatorAdapter` (`LocalValidatorFactoryBean` being one of
the available implementations) that only exposes the Spring contract.

Also, if a `javax.validation.Validator` bean is available, we will use it
for the MVC layer, rather than creating a new one.

Closes gh-8223

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/SpringValidatorAdapterWrapper.java

@@ -0,0 +1,83 @@
+/*
+ * Copyright 2012-2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.autoconfigure.web;
+
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.DisposableBean;
+import org.springframework.beans.factory.InitializingBean;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContextAware;
+import org.springframework.validation.Errors;
+import org.springframework.validation.Validator;
+import org.springframework.validation.beanvalidation.SpringValidatorAdapter;
+
+/**
+ * Wraps a {@link SpringValidatorAdapter} so that only the Spring's {@link Validator}
+ * type is exposed. This prevents such a bean to expose both the Spring and JSR-303
+ * validator contract at the same time.
+ *
+ * @author Stephane Nicoll
+ */
+class SpringValidatorAdapterWrapper
+		implements Validator, ApplicationContextAware, InitializingBean, DisposableBean {
+
+	private final SpringValidatorAdapter target;
+	private final boolean managed;
+
+	SpringValidatorAdapterWrapper(SpringValidatorAdapter target, boolean managed) {
+		this.target = target;
+		this.managed = managed;
+	}
+
+	public SpringValidatorAdapter getTarget() {
+		return this.target;
+	}
+
+	@Override
+	public boolean supports(Class<?> clazz) {
+		return this.target.supports(clazz);
+	}
+
+	@Override
+	public void validate(Object target, Errors errors) {
+		this.target.validate(target, errors);
+	}
+
+	@Override
+	public void setApplicationContext(ApplicationContext applicationContext)
+			throws BeansException {
+		if (!this.managed && this.target instanceof ApplicationContextAware) {
+			((ApplicationContextAware) this.target).setApplicationContext(
+					applicationContext);
+		}
+	}
+
+	@Override
+	public void afterPropertiesSet() throws Exception {
+		if (!this.managed && this.target instanceof InitializingBean) {
+			((InitializingBean) this.target).afterPropertiesSet();
+		}
+	}
+
+	@Override
+	public void destroy() throws Exception {
+		if (!this.managed && this.target instanceof DisposableBean) {
+			((DisposableBean) this.target).destroy();
+		}
+	}
+
+}

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfiguration.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2016 the original author or authors.
+ * Copyright 2012-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -42,11 +42,14 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
+import org.springframework.boot.autoconfigure.validation.ValidationAutoConfiguration;
 import org.springframework.boot.autoconfigure.web.ResourceProperties.Strategy;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
+import org.springframework.boot.validation.MessageInterpolatorFactory;
 import org.springframework.boot.web.filter.OrderedHiddenHttpMethodFilter;
 import org.springframework.boot.web.filter.OrderedHttpPutFormContentFilter;
 import org.springframework.boot.web.filter.OrderedRequestContextFilter;
+import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
@@ -61,9 +64,14 @@ import org.springframework.format.datetime.DateFormatter;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.MediaType;
 import org.springframework.http.converter.HttpMessageConverter;
+import org.springframework.util.ClassUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.validation.DefaultMessageCodesResolver;
 import org.springframework.validation.MessageCodesResolver;
+import org.springframework.validation.Validator;
+import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
+import org.springframework.validation.beanvalidation.OptionalValidatorFactoryBean;
+import org.springframework.validation.beanvalidation.SpringValidatorAdapter;
 import org.springframework.web.accept.ContentNegotiationManager;
 import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
 import org.springframework.web.context.request.RequestContextListener;
@@ -83,6 +91,7 @@ import org.springframework.web.servlet.config.annotation.ResourceChainRegistrati
 import org.springframework.web.servlet.config.annotation.ResourceHandlerRegistration;
 import org.springframework.web.servlet.config.annotation.ResourceHandlerRegistry;
 import org.springframework.web.servlet.config.annotation.WebMvcConfigurationSupport;
+import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
 import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter;
 import org.springframework.web.servlet.handler.AbstractHandlerExceptionResolver;
 import org.springframework.web.servlet.handler.AbstractUrlHandlerMapping;
@@ -118,13 +127,16 @@ import org.springframework.web.servlet.view.InternalResourceViewResolver;
 		WebMvcConfigurerAdapter.class })
 @ConditionalOnMissingBean(WebMvcConfigurationSupport.class)
 @AutoConfigureOrder(Ordered.HIGHEST_PRECEDENCE + 10)
-@AutoConfigureAfter(DispatcherServletAutoConfiguration.class)
+@AutoConfigureAfter({ DispatcherServletAutoConfiguration.class,
+		ValidationAutoConfiguration.class })
 public class WebMvcAutoConfiguration {
 
 	public static String DEFAULT_PREFIX = "";
 
 	public static String DEFAULT_SUFFIX = "";
 
+	private static final String JSR303_VALIDATOR_CLASS = "javax.validation.Validator";
+
 	@Bean
 	@ConditionalOnMissingBean(HiddenHttpMethodFilter.class)
 	public OrderedHiddenHttpMethodFilter hiddenHttpMethodFilter() {
@@ -148,6 +160,8 @@ public class WebMvcAutoConfiguration {
 		private static final Log logger = LogFactory
 				.getLog(WebMvcConfigurerAdapter.class);
 
+		private final ApplicationContext applicationContext;
+
 		private final ResourceProperties resourceProperties;
 
 		private final WebMvcProperties mvcProperties;
@@ -156,20 +170,39 @@ public class WebMvcAutoConfiguration {
 
 		private final HttpMessageConverters messageConverters;
 
+		private final Validator userDefinedValidator;
+
 		final ResourceHandlerRegistrationCustomizer resourceHandlerRegistrationCustomizer;
 
-		public WebMvcAutoConfigurationAdapter(ResourceProperties resourceProperties,
-				WebMvcProperties mvcProperties, ListableBeanFactory beanFactory,
-				HttpMessageConverters messageConverters,
+		public WebMvcAutoConfigurationAdapter(ApplicationContext applicationContext,
+				ResourceProperties resourceProperties, WebMvcProperties mvcProperties,
+				ListableBeanFactory beanFactory, HttpMessageConverters messageConverters,
+				ObjectProvider<List<WebMvcConfigurer>> webMvcConfigurers,
 				ObjectProvider<ResourceHandlerRegistrationCustomizer> resourceHandlerRegistrationCustomizerProvider) {
+			this.applicationContext = applicationContext;
 			this.resourceProperties = resourceProperties;
 			this.mvcProperties = mvcProperties;
 			this.beanFactory = beanFactory;
 			this.messageConverters = messageConverters;
+			this.userDefinedValidator = findUserDefinedValidator(
+					webMvcConfigurers.getIfAvailable());
 			this.resourceHandlerRegistrationCustomizer = resourceHandlerRegistrationCustomizerProvider
 					.getIfAvailable();
 		}
 
+		private static Validator findUserDefinedValidator(
+				List<WebMvcConfigurer> webMvcConfigurers) {
+			if (webMvcConfigurers != null) {
+				for (WebMvcConfigurer webMvcConfigurer : webMvcConfigurers) {
+					Validator validator = webMvcConfigurer.getValidator();
+					if (validator != null) {
+						return validator;
+					}
+				}
+			}
+			return null;
+		}
+
 		@Override
 		public void configureMessageConverters(List<HttpMessageConverter<?>> converters) {
 			converters.addAll(this.messageConverters.getConverters());
@@ -265,6 +298,23 @@ public class WebMvcAutoConfiguration {
 			}
 		}
 
+		@Override
+		public Validator getValidator() {
+			// We want to make sure that the exposed 'mvcValidator' bean isn't going to
+			// expose the standard JSR-303 type
+			if (isJsr303Present() && this.userDefinedValidator == null) {
+				return new Jsr303ValidatorHandler(this.applicationContext)
+						.wrapJsr303Validator();
+
+			}
+			return null; // Keep default or user defined, if any
+		}
+
+		private boolean isJsr303Present() {
+			return ClassUtils.isPresent(JSR303_VALIDATOR_CLASS,
+					this.applicationContext.getClassLoader());
+		}
+
 		private <T> Collection<T> getBeansOfType(Class<T> type) {
 			return this.beanFactory.getBeansOfType(type).values();
 		}
@@ -533,4 +583,35 @@ public class WebMvcAutoConfiguration {
 
 	}
 
+	static class Jsr303ValidatorHandler {
+
+		private final ApplicationContext applicationContext;
+
+		Jsr303ValidatorHandler(ApplicationContext applicationContext) {
+			this.applicationContext = applicationContext;
+		}
+
+		public Validator wrapJsr303Validator() {
+			try {
+				javax.validation.Validator validator = this.applicationContext
+						.getBean(javax.validation.Validator.class);
+				if (validator instanceof LocalValidatorFactoryBean) {
+					return new SpringValidatorAdapterWrapper(
+							(LocalValidatorFactoryBean) validator, true);
+				}
+				else {
+					return new SpringValidatorAdapterWrapper(
+							new SpringValidatorAdapter(validator), false);
+				}
+			}
+			catch (NoSuchBeanDefinitionException ex) {
+				OptionalValidatorFactoryBean factory = new OptionalValidatorFactoryBean();
+				MessageInterpolatorFactory interpolatorFactory = new MessageInterpolatorFactory();
+				factory.setMessageInterpolator(interpolatorFactory.getObject());
+				return new SpringValidatorAdapterWrapper(factory, false);
+			}
+
+		}
+	}
+
 }

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/SpringValidatorAdapterWrapperTests.java

@@ -0,0 +1,152 @@
+/*
+ * Copyright 2012-2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.autoconfigure.web;
+
+import java.util.HashMap;
+
+import javax.validation.constraints.Min;
+
+import org.junit.After;
+import org.junit.Test;
+
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.validation.MapBindingResult;
+import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Tests for {@link SpringValidatorAdapterWrapper}.
+ *
+ * @author Stephane Nicoll
+ */
+public class SpringValidatorAdapterWrapperTests {
+
+	private AnnotationConfigApplicationContext context;
+
+	@After
+	public void close() {
+		if (this.context != null) {
+			this.context.close();
+		}
+	}
+
+	@Test
+	public void wrapLocalValidatorFactoryBean() {
+		SpringValidatorAdapterWrapper wrapper = load(
+				LocalValidatorFactoryBeanConfig.class);
+		assertThat(wrapper.supports(SampleData.class)).isTrue();
+		MapBindingResult errors = new MapBindingResult(
+				new HashMap<String, Object>(), "test");
+		wrapper.validate(new SampleData(40), errors);
+		assertThat(errors.getErrorCount()).isEqualTo(1);
+	}
+
+	@Test
+	public void wrapperInvokesCallbackOnNonManagedBean() {
+		load(NonManagedBeanConfig.class);
+		LocalValidatorFactoryBean validator = this.context.getBean(
+				NonManagedBeanConfig.class).validator;
+		verify(validator, times(1)).setApplicationContext(any(ApplicationContext.class));
+		verify(validator, times(1)).afterPropertiesSet();
+		verify(validator, times(0)).destroy();
+		this.context.close();
+		this.context = null;
+		verify(validator, times(1)).destroy();
+	}
+
+	@Test
+	public void wrapperDoesNotInvokeCallbackOnManagedBean() {
+		load(ManagedBeanConfig.class);
+		LocalValidatorFactoryBean validator = this.context.getBean(
+				ManagedBeanConfig.class).validator;
+		verify(validator, times(0)).setApplicationContext(any(ApplicationContext.class));
+		verify(validator, times(0)).afterPropertiesSet();
+		verify(validator, times(0)).destroy();
+		this.context.close();
+		this.context = null;
+		verify(validator, times(0)).destroy();
+	}
+
+	private SpringValidatorAdapterWrapper load(Class<?> config) {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
+		ctx.register(config);
+		ctx.refresh();
+		this.context = ctx;
+		return this.context.getBean(SpringValidatorAdapterWrapper.class);
+	}
+
+	@Configuration
+	static class LocalValidatorFactoryBeanConfig {
+
+		@Bean
+		public LocalValidatorFactoryBean validator() {
+			return new LocalValidatorFactoryBean();
+		}
+
+		@Bean
+		public SpringValidatorAdapterWrapper wrapper() {
+			return new SpringValidatorAdapterWrapper(validator(), true);
+		}
+
+	}
+
+	@Configuration
+	static class NonManagedBeanConfig {
+
+		private final LocalValidatorFactoryBean validator
+				= mock(LocalValidatorFactoryBean.class);
+
+		@Bean
+		public SpringValidatorAdapterWrapper wrapper() {
+			return new SpringValidatorAdapterWrapper(this.validator, false);
+		}
+
+	}
+
+	@Configuration
+	static class ManagedBeanConfig {
+
+		private final LocalValidatorFactoryBean validator
+				= mock(LocalValidatorFactoryBean.class);
+
+		@Bean
+		public SpringValidatorAdapterWrapper wrapper() {
+			return new SpringValidatorAdapterWrapper(this.validator, true);
+		}
+
+	}
+
+	static class SampleData {
+
+		@Min(42)
+		private int counter;
+
+		SampleData(int counter) {
+			this.counter = counter;
+		}
+
+	}
+
+}

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfigurationTests.java

@@ -27,6 +27,7 @@ import java.util.Map;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.validation.ValidatorFactory;
 
 import org.assertj.core.api.Condition;
 import org.joda.time.DateTime;
@@ -60,6 +61,9 @@ import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.setup.MockMvcBuilders;
 import org.springframework.util.ReflectionUtils;
 import org.springframework.util.StringUtils;
+import org.springframework.validation.Validator;
+import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
+import org.springframework.validation.beanvalidation.SpringValidatorAdapter;
 import org.springframework.web.accept.ContentNegotiationManager;
 import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
 import org.springframework.web.filter.HttpPutFormContentFilter;
@@ -94,6 +98,7 @@ import org.springframework.web.servlet.view.AbstractView;
 import org.springframework.web.servlet.view.ContentNegotiatingViewResolver;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.forwardedUrl;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@@ -640,6 +645,55 @@ public class WebMvcAutoConfigurationTests {
 		}
 	}
 
+	@Test
+	public void validationNoJsr303ValidatorExposedByDefault() {
+		load();
+		assertThat(this.context.getBeansOfType(ValidatorFactory.class)).isEmpty();
+		assertThat(this.context.getBeansOfType(javax.validation.Validator.class))
+				.isEmpty();
+		assertThat(this.context.getBeansOfType(Validator.class)).hasSize(1);
+	}
+
+	@Test
+	public void validationCustomConfigurerTakesPrecedence() {
+		load(MvcValidator.class);
+		assertThat(this.context.getBeansOfType(ValidatorFactory.class)).isEmpty();
+		assertThat(this.context.getBeansOfType(javax.validation.Validator.class))
+				.isEmpty();
+		assertThat(this.context.getBeansOfType(Validator.class)).hasSize(1);
+		Validator validator = this.context.getBean(Validator.class);
+		assertThat(validator).isSameAs(this.context.getBean(MvcValidator.class)
+				.validator);
+	}
+
+	@Test
+	public void validationJsr303CustomValidatorReusedAsSpringValidator() {
+		load(CustomValidator.class);
+		assertThat(this.context.getBeansOfType(ValidatorFactory.class)).hasSize(1);
+		assertThat(this.context.getBeansOfType(javax.validation.Validator.class))
+				.hasSize(1);
+		assertThat(this.context.getBeansOfType(Validator.class)).hasSize(2);
+		Validator validator = this.context.getBean("mvcValidator", Validator.class);
+		assertThat(validator).isInstanceOf(SpringValidatorAdapterWrapper.class);
+		assertThat(((SpringValidatorAdapterWrapper) validator).getTarget())
+				.isSameAs(this.context.getBean(javax.validation.Validator.class));
+	}
+
+	@Test
+	public void validationJsr303ValidatorExposedAsSpringValidator() {
+		load(Jsr303Validator.class);
+		assertThat(this.context.getBeansOfType(ValidatorFactory.class)).isEmpty();
+		assertThat(this.context.getBeansOfType(javax.validation.Validator.class))
+				.hasSize(1);
+		assertThat(this.context.getBeansOfType(Validator.class)).hasSize(1);
+		Validator validator = this.context.getBean(Validator.class);
+		assertThat(validator).isInstanceOf(SpringValidatorAdapterWrapper.class);
+		SpringValidatorAdapter target = ((SpringValidatorAdapterWrapper) validator)
+				.getTarget();
+		assertThat(new DirectFieldAccessor(target).getPropertyValue("targetValidator"))
+				.isSameAs(this.context.getBean(javax.validation.Validator.class));
+	}
+
 	private void load(Class<?> config, String... environment) {
 		this.context = new AnnotationConfigEmbeddedWebApplicationContext();
 		EnvironmentTestUtils.addEnvironment(this.context, environment);
@@ -818,4 +872,36 @@ public class WebMvcAutoConfigurationTests {
 
 	}
 
+	@Configuration
+	protected static class MvcValidator extends WebMvcConfigurerAdapter {
+
+		private final Validator validator = mock(Validator.class);
+
+		@Override
+		public Validator getValidator() {
+			return this.validator;
+		}
+
+	}
+
+	@Configuration
+	static class Jsr303Validator {
+
+		@Bean
+		public javax.validation.Validator jsr303Validator() {
+			return mock(javax.validation.Validator.class);
+		}
+
+	}
+
+	@Configuration
+	static class CustomValidator {
+
+		@Bean
+		public Validator customValidator() {
+			return new LocalValidatorFactoryBean();
+		}
+
+	}
+
 }