GenericFilterBean lazily initializes its default environment

Alignment with HttpServletBean; consistent use of EnvironmentCapable.

Issue: SPR-15469

spring-context/src/main/java/org/springframework/context/ConfigurableApplicationContext.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-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.
@@ -103,18 +103,19 @@ public interface ConfigurableApplicationContext extends ApplicationContext, Life
 	void setParent(ApplicationContext parent);
 
 	/**
-	 * Return the Environment for this application context in configurable form.
+	 * Set the {@code Environment} for this application context.
+	 * @param environment the new environment
 	 * @since 3.1
 	 */
-	@Override
-	ConfigurableEnvironment getEnvironment();
+	void setEnvironment(ConfigurableEnvironment environment);
 
 	/**
-	 * Set the {@code Environment} for this application context.
-	 * @param environment the new environment
+	 * Return the {@code Environment} for this application context in configurable
+	 * form, allowing for further customization.
 	 * @since 3.1
 	 */
-	void setEnvironment(ConfigurableEnvironment environment);
+	@Override
+	ConfigurableEnvironment getEnvironment();
 
 	/**
 	 * Add a new BeanFactoryPostProcessor that will get applied to the internal

spring-context/src/main/java/org/springframework/context/EnvironmentAware.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-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.
@@ -25,11 +25,12 @@ import org.springframework.core.env.Environment;
  *
  * @author Chris Beams
  * @since 3.1
+ * @see org.springframework.core.env.EnvironmentCapable
  */
 public interface EnvironmentAware extends Aware {
 
 	/**
-	 * Set the {@code Environment} that this object runs in.
+	 * Set the {@code Environment} that this component runs in.
 	 */
 	void setEnvironment(Environment environment);
 

spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java

@@ -280,8 +280,22 @@ public abstract class AbstractApplicationContext extends DefaultResourceLoader
 	}
 
 	/**
-	 * {@inheritDoc}
-	 * <p>If {@code null}, a new environment will be initialized via
+	 * Set the {@code Environment} for this application context.
+	 * <p>Default value is determined by {@link #createEnvironment()}. Replacing the
+	 * default with this method is one option but configuration through {@link
+	 * #getEnvironment()} should also be considered. In either case, such modifications
+	 * should be performed <em>before</em> {@link #refresh()}.
+	 * @see org.springframework.context.support.AbstractApplicationContext#createEnvironment
+	 */
+	@Override
+	public void setEnvironment(ConfigurableEnvironment environment) {
+		this.environment = environment;
+	}
+
+	/**
+	 * Return the {@code Environment} for this application context in configurable
+	 * form, allowing for further customization.
+	 * <p>If none specified, a default environment will be initialized via
 	 * {@link #createEnvironment()}.
 	 */
 	@Override
@@ -293,16 +307,12 @@ public abstract class AbstractApplicationContext extends DefaultResourceLoader
 	}
 
 	/**
-	 * {@inheritDoc}
-	 * <p>Default value is determined by {@link #createEnvironment()}. Replacing the
-	 * default with this method is one option but configuration through {@link
-	 * #getEnvironment()} should also be considered. In either case, such modifications
-	 * should be performed <em>before</em> {@link #refresh()}.
-	 * @see org.springframework.context.support.AbstractApplicationContext#createEnvironment
+	 * Create and return a new {@link StandardEnvironment}.
+	 * <p>Subclasses may override this method in order to supply
+	 * a custom {@link ConfigurableEnvironment} implementation.
 	 */
-	@Override
-	public void setEnvironment(ConfigurableEnvironment environment) {
-		this.environment = environment;
+	protected ConfigurableEnvironment createEnvironment() {
+		return new StandardEnvironment();
 	}
 
 	/**
@@ -443,7 +453,7 @@ public abstract class AbstractApplicationContext extends DefaultResourceLoader
 	//---------------------------------------------------------------------
 
 	/**
-	 * {@inheritDoc}
+	 * Set the parent of this application context.
 	 * <p>The parent {@linkplain ApplicationContext#getEnvironment() environment} is
 	 * {@linkplain ConfigurableEnvironment#merge(ConfigurableEnvironment) merged} with
 	 * this (child) application context environment if the parent is non-{@code null} and
@@ -494,15 +504,6 @@ public abstract class AbstractApplicationContext extends DefaultResourceLoader
 		return this.applicationListeners;
 	}
 
-	/**
-	 * Create and return a new {@link StandardEnvironment}.
-	 * <p>Subclasses may override this method in order to supply
-	 * a custom {@link ConfigurableEnvironment} implementation.
-	 */
-	protected ConfigurableEnvironment createEnvironment() {
-		return new StandardEnvironment();
-	}
-
 	@Override
 	public void refresh() throws BeansException, IllegalStateException {
 		synchronized (this.startupShutdownMonitor) {

spring-core/src/main/java/org/springframework/core/env/EnvironmentCapable.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-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.
@@ -41,7 +41,8 @@ package org.springframework.core.env;
 public interface EnvironmentCapable {
 
 	/**
-	 * Return the {@link Environment} associated with this component.
+	 * Return the {@link Environment} associated with this component
+	 * (may be {@code null} or a default environment).
 	 */
 	Environment getEnvironment();
 

spring-web/src/main/java/org/springframework/web/filter/GenericFilterBean.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-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.
@@ -38,10 +38,12 @@ import org.springframework.beans.factory.DisposableBean;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.context.EnvironmentAware;
 import org.springframework.core.env.Environment;
+import org.springframework.core.env.EnvironmentCapable;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.ResourceEditor;
 import org.springframework.core.io.ResourceLoader;
 import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.web.context.ServletContextAware;
 import org.springframework.web.context.support.ServletContextResourceLoader;
@@ -75,26 +77,22 @@ import org.springframework.web.util.NestedServletException;
  * @see #initFilterBean
  * @see #doFilter
  */
-public abstract class GenericFilterBean implements
-		Filter, BeanNameAware, EnvironmentAware, ServletContextAware, InitializingBean, DisposableBean {
+public abstract class GenericFilterBean implements Filter, BeanNameAware, EnvironmentAware,
+		EnvironmentCapable, ServletContextAware, InitializingBean, DisposableBean {
 
 	/** Logger available to subclasses */
 	protected final Log logger = LogFactory.getLog(getClass());
 
-	/**
-	 * Set of required properties (Strings) that must be supplied as
-	 * config parameters to this filter.
-	 */
-	private final Set<String> requiredProperties = new HashSet<>();
-
-	private FilterConfig filterConfig;
-
 	private String beanName;
 
-	private Environment environment = new StandardServletEnvironment();
+	private Environment environment;
 
 	private ServletContext servletContext;
 
+	private FilterConfig filterConfig;
+
+	private final Set<String> requiredProperties = new HashSet<>(4);
+
 
 	/**
 	 * Stores the bean name as defined in the Spring bean factory.
@@ -104,24 +102,47 @@ public abstract class GenericFilterBean implements
 	 * @see #getFilterName()
 	 */
 	@Override
-	public final void setBeanName(String beanName) {
+	public void setBeanName(String beanName) {
 		this.beanName = beanName;
 	}
 
 	/**
-	 * {@inheritDoc}
+	 * Set the {@code Environment} that this filter runs in.
 	 * <p>Any environment set here overrides the {@link StandardServletEnvironment}
 	 * provided by default.
 	 * <p>This {@code Environment} object is used only for resolving placeholders in
 	 * resource paths passed into init-parameters for this filter. If no init-params are
 	 * used, this {@code Environment} can be essentially ignored.
-	 * @see #init(FilterConfig)
 	 */
 	@Override
 	public void setEnvironment(Environment environment) {
 		this.environment = environment;
 	}
 
+	/**
+	 * Return the {@link Environment} associated with this filter.
+	 * <p>If none specified, a default environment will be initialized via
+	 * {@link #createEnvironment()}.
+	 * @since 4.3.9
+	 */
+	@Override
+	public Environment getEnvironment() {
+		if (this.environment == null) {
+			this.environment = createEnvironment();
+		}
+		return this.environment;
+	}
+
+	/**
+	 * Create and return a new {@link StandardServletEnvironment}.
+	 * <p>Subclasses may override this in order to configure the environment or
+	 * specialize the environment type returned.
+	 * @since 4.3.9
+	 */
+	protected Environment createEnvironment() {
+		return new StandardServletEnvironment();
+	}
+
 	/**
 	 * Stores the ServletContext that the bean factory runs in.
 	 * <p>Only relevant in case of initialization as bean, to have a ServletContext
@@ -130,7 +151,7 @@ public abstract class GenericFilterBean implements
 	 * @see #getServletContext()
 	 */
 	@Override
-	public final void setServletContext(ServletContext servletContext) {
+	public void setServletContext(ServletContext servletContext) {
 		this.servletContext = servletContext;
 	}
 
@@ -147,6 +168,16 @@ public abstract class GenericFilterBean implements
 		initFilterBean();
 	}
 
+	/**
+	 * Subclasses may override this to perform custom filter shutdown.
+	 * <p>Note: This method will be called from standard filter destruction
+	 * as well as filter bean destruction in a Spring application context.
+	 * <p>This default implementation is empty.
+	 */
+	@Override
+	public void destroy() {
+	}
+
 
 	/**
 	 * Subclasses can invoke this method to specify that this property
@@ -180,19 +211,25 @@ public abstract class GenericFilterBean implements
 		this.filterConfig = filterConfig;
 
 		// Set bean properties from init parameters.
-		try {
-			PropertyValues pvs = new FilterConfigPropertyValues(filterConfig, this.requiredProperties);
-			BeanWrapper bw = PropertyAccessorFactory.forBeanPropertyAccess(this);
-			ResourceLoader resourceLoader = new ServletContextResourceLoader(filterConfig.getServletContext());
-			bw.registerCustomEditor(Resource.class, new ResourceEditor(resourceLoader, this.environment));
-			initBeanWrapper(bw);
-			bw.setPropertyValues(pvs, true);
-		}
-		catch (BeansException ex) {
-			String msg = "Failed to set bean properties on filter '" +
-				filterConfig.getFilterName() + "': " + ex.getMessage();
-			logger.error(msg, ex);
-			throw new NestedServletException(msg, ex);
+		PropertyValues pvs = new FilterConfigPropertyValues(filterConfig, this.requiredProperties);
+		if (!pvs.isEmpty()) {
+			try {
+				BeanWrapper bw = PropertyAccessorFactory.forBeanPropertyAccess(this);
+				ResourceLoader resourceLoader = new ServletContextResourceLoader(filterConfig.getServletContext());
+				Environment env = this.environment;
+				if (env == null) {
+					env = new StandardServletEnvironment();
+				}
+				bw.registerCustomEditor(Resource.class, new ResourceEditor(resourceLoader, env));
+				initBeanWrapper(bw);
+				bw.setPropertyValues(pvs, true);
+			}
+			catch (BeansException ex) {
+				String msg = "Failed to set bean properties on filter '" +
+					filterConfig.getFilterName() + "': " + ex.getMessage();
+				logger.error(msg, ex);
+				throw new NestedServletException(msg, ex);
+			}
 		}
 
 		// Let subclasses do whatever initialization they like.
@@ -214,6 +251,20 @@ public abstract class GenericFilterBean implements
 	protected void initBeanWrapper(BeanWrapper bw) throws BeansException {
 	}
 
+	/**
+	 * Subclasses may override this to perform custom initialization.
+	 * All bean properties of this filter will have been set before this
+	 * method is invoked.
+	 * <p>Note: This method will be called from standard filter initialization
+	 * as well as filter bean initialization in a Spring application context.
+	 * Filter name and ServletContext will be available in both cases.
+	 * <p>This default implementation is empty.
+	 * @throws ServletException if subclass initialization fails
+	 * @see #getFilterName()
+	 * @see #getServletContext()
+	 */
+	protected void initFilterBean() throws ServletException {
+	}
 
 	/**
 	 * Make the FilterConfig of this filter available, if any.
@@ -258,32 +309,6 @@ public abstract class GenericFilterBean implements
 	}
 
 
-	/**
-	 * Subclasses may override this to perform custom initialization.
-	 * All bean properties of this filter will have been set before this
-	 * method is invoked.
-	 * <p>Note: This method will be called from standard filter initialization
-	 * as well as filter bean initialization in a Spring application context.
-	 * Filter name and ServletContext will be available in both cases.
-	 * <p>This default implementation is empty.
-	 * @throws ServletException if subclass initialization fails
-	 * @see #getFilterName()
-	 * @see #getServletContext()
-	 */
-	protected void initFilterBean() throws ServletException {
-	}
-
-	/**
-	 * Subclasses may override this to perform custom filter shutdown.
-	 * <p>Note: This method will be called from standard filter destruction
-	 * as well as filter bean destruction in a Spring application context.
-	 * <p>This default implementation is empty.
-	 */
-	@Override
-	public void destroy() {
-	}
-
-
 	/**
 	 * PropertyValues implementation created from FilterConfig init parameters.
 	 */
@@ -298,14 +323,14 @@ public abstract class GenericFilterBean implements
 		 * @throws ServletException if any required properties are missing
 		 */
 		public FilterConfigPropertyValues(FilterConfig config, Set<String> requiredProperties)
-			throws ServletException {
+				throws ServletException {
 
-			Set<String> missingProps = (requiredProperties != null && !requiredProperties.isEmpty()) ?
-					new HashSet<>(requiredProperties) : null;
+			Set<String> missingProps = (!CollectionUtils.isEmpty(requiredProperties) ?
+					new HashSet<>(requiredProperties) : null);
 
-			Enumeration<?> en = config.getInitParameterNames();
-			while (en.hasMoreElements()) {
-				String property = (String) en.nextElement();
+			Enumeration<String> paramNames = config.getInitParameterNames();
+			while (paramNames.hasMoreElements()) {
+				String property = paramNames.nextElement();
 				Object value = config.getInitParameter(property);
 				addPropertyValue(new PropertyValue(property, value));
 				if (missingProps != null) {
@@ -314,11 +339,11 @@ public abstract class GenericFilterBean implements
 			}
 
 			// Fail if we are still missing properties.
-			if (missingProps != null && missingProps.size() > 0) {
+			if (!CollectionUtils.isEmpty(missingProps)) {
 				throw new ServletException(
-					"Initialization from FilterConfig for filter '" + config.getFilterName() +
-					"' failed; the following required properties were missing: " +
-					StringUtils.collectionToDelimitedString(missingProps, ", "));
+						"Initialization from FilterConfig for filter '" + config.getFilterName() +
+						"' failed; the following required properties were missing: " +
+						StringUtils.collectionToDelimitedString(missingProps, ", "));
 			}
 		}
 	}

spring-webmvc/src/main/java/org/springframework/web/servlet/HttpServletBean.java

@@ -84,14 +84,10 @@ public abstract class HttpServletBean extends HttpServlet implements Environment
 	/** Logger available to subclasses */
 	protected final Log logger = LogFactory.getLog(getClass());
 
-	/**
-	 * Set of required properties (Strings) that must be supplied as
-	 * config parameters to this servlet.
-	 */
-	private final Set<String> requiredProperties = new HashSet<>();
-
 	private ConfigurableEnvironment environment;
 
+	private final Set<String> requiredProperties = new HashSet<>(4);
+
 
 	/**
 	 * Subclasses can invoke this method to specify that this property
@@ -106,6 +102,41 @@ public abstract class HttpServletBean extends HttpServlet implements Environment
 		this.requiredProperties.add(property);
 	}
 
+	/**
+	 * Set the {@code Environment} that this servlet runs in.
+	 * <p>Any environment set here overrides the {@link StandardServletEnvironment}
+	 * provided by default.
+	 * @throws IllegalArgumentException if environment is not assignable to
+	 * {@code ConfigurableEnvironment}
+	 */
+	@Override
+	public void setEnvironment(Environment environment) {
+		Assert.isInstanceOf(ConfigurableEnvironment.class, environment, "ConfigurableEnvironment required");
+		this.environment = (ConfigurableEnvironment) environment;
+	}
+
+	/**
+	 * Return the {@link Environment} associated with this servlet.
+	 * <p>If none specified, a default environment will be initialized via
+	 * {@link #createEnvironment()}.
+	 */
+	@Override
+	public ConfigurableEnvironment getEnvironment() {
+		if (this.environment == null) {
+			this.environment = createEnvironment();
+		}
+		return this.environment;
+	}
+
+	/**
+	 * Create and return a new {@link StandardServletEnvironment}.
+	 * <p>Subclasses may override this in order to configure the environment or
+	 * specialize the environment type returned.
+	 */
+	protected ConfigurableEnvironment createEnvironment() {
+		return new StandardServletEnvironment();
+	}
+
 	/**
 	 * Map config parameters onto bean properties of this servlet, and
 	 * invoke subclass initialization.
@@ -119,19 +150,21 @@ public abstract class HttpServletBean extends HttpServlet implements Environment
 		}
 
 		// Set bean properties from init parameters.
-		try {
-			PropertyValues pvs = new ServletConfigPropertyValues(getServletConfig(), this.requiredProperties);
-			BeanWrapper bw = PropertyAccessorFactory.forBeanPropertyAccess(this);
-			ResourceLoader resourceLoader = new ServletContextResourceLoader(getServletContext());
-			bw.registerCustomEditor(Resource.class, new ResourceEditor(resourceLoader, getEnvironment()));
-			initBeanWrapper(bw);
-			bw.setPropertyValues(pvs, true);
-		}
-		catch (BeansException ex) {
-			if (logger.isErrorEnabled()) {
-				logger.error("Failed to set bean properties on servlet '" + getServletName() + "'", ex);
+		PropertyValues pvs = new ServletConfigPropertyValues(getServletConfig(), this.requiredProperties);
+		if (!pvs.isEmpty()) {
+			try {
+				BeanWrapper bw = PropertyAccessorFactory.forBeanPropertyAccess(this);
+				ResourceLoader resourceLoader = new ServletContextResourceLoader(getServletContext());
+				bw.registerCustomEditor(Resource.class, new ResourceEditor(resourceLoader, getEnvironment()));
+				initBeanWrapper(bw);
+				bw.setPropertyValues(pvs, true);
+			}
+			catch (BeansException ex) {
+				if (logger.isErrorEnabled()) {
+					logger.error("Failed to set bean properties on servlet '" + getServletName() + "'", ex);
+				}
+				throw ex;
 			}
-			throw ex;
 		}
 
 		// Let subclasses do whatever initialization they like.
@@ -153,6 +186,15 @@ public abstract class HttpServletBean extends HttpServlet implements Environment
 	protected void initBeanWrapper(BeanWrapper bw) throws BeansException {
 	}
 
+	/**
+	 * Subclasses may override this to perform custom initialization.
+	 * All bean properties of this servlet will have been set before this
+	 * method is invoked.
+	 * <p>This default implementation is empty.
+	 * @throws ServletException if subclass initialization fails
+	 */
+	protected void initServletBean() throws ServletException {
+	}
 
 	/**
 	 * Overridden method that simply returns {@code null} when no
@@ -175,49 +217,6 @@ public abstract class HttpServletBean extends HttpServlet implements Environment
 	}
 
 
-	/**
-	 * Subclasses may override this to perform custom initialization.
-	 * All bean properties of this servlet will have been set before this
-	 * method is invoked.
-	 * <p>This default implementation is empty.
-	 * @throws ServletException if subclass initialization fails
-	 */
-	protected void initServletBean() throws ServletException {
-	}
-
-	/**
-	 * {@inheritDoc}
-	 * @throws IllegalArgumentException if environment is not assignable to
-	 * {@code ConfigurableEnvironment}.
-	 */
-	@Override
-	public void setEnvironment(Environment environment) {
-		Assert.isInstanceOf(ConfigurableEnvironment.class, environment, "ConfigurableEnvironment required");
-		this.environment = (ConfigurableEnvironment) environment;
-	}
-
-	/**
-	 * {@inheritDoc}
-	 * <p>If {@code null}, a new environment will be initialized via
-	 * {@link #createEnvironment()}.
-	 */
-	@Override
-	public ConfigurableEnvironment getEnvironment() {
-		if (this.environment == null) {
-			this.environment = this.createEnvironment();
-		}
-		return this.environment;
-	}
-
-	/**
-	 * Create and return a new {@link StandardServletEnvironment}. Subclasses may override
-	 * in order to configure the environment or specialize the environment type returned.
-	 */
-	protected ConfigurableEnvironment createEnvironment() {
-		return new StandardServletEnvironment();
-	}
-
-
 	/**
 	 * PropertyValues implementation created from ServletConfig init parameters.
 	 */
@@ -231,9 +230,9 @@ public abstract class HttpServletBean extends HttpServlet implements Environment
 		 * @throws ServletException if any required properties are missing
 		 */
 		public ServletConfigPropertyValues(ServletConfig config, Set<String> requiredProperties)
-			throws ServletException {
+				throws ServletException {
 
-			Set<String> missingProps = (requiredProperties != null && !requiredProperties.isEmpty() ?
+			Set<String> missingProps = (!CollectionUtils.isEmpty(requiredProperties) ?
 					new HashSet<>(requiredProperties) : null);
 
 			Enumeration<String> paramNames = config.getInitParameterNames();
@@ -249,9 +248,9 @@ public abstract class HttpServletBean extends HttpServlet implements Environment
 			// Fail if we are still missing properties.
 			if (!CollectionUtils.isEmpty(missingProps)) {
 				throw new ServletException(
-					"Initialization from ServletConfig for servlet '" + config.getServletName() +
-					"' failed; the following required properties were missing: " +
-					StringUtils.collectionToDelimitedString(missingProps, ", "));
+						"Initialization from ServletConfig for servlet '" + config.getServletName() +
+						"' failed; the following required properties were missing: " +
+						StringUtils.collectionToDelimitedString(missingProps, ", "));
 			}
 		}
 	}