From 6517517ca9867be846112edd17b7da907a2d04de Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Wed, 5 Sep 2012 21:55:41 +0200 Subject: [PATCH] Refactor to lazy Environment creation where possible This commit avoids eager creation of Environment instances, favoring delegation of already existing Environment objects where possible. For example, FrameworkServlet creates an ApplicationContext; both require a StandardServletEnvironment instance, and prior to this change, two instances were created where one would suffice - indeed these two instances may reasonably be expected to be the same. Now, the FrameworkServlet defers creation of its Environment, allowing users to supply a custom instance via its #setEnvironment method (e.g. within a WebApplicationInitializer); the FrameworkServlet then takes care to delegate that instance to the ApplicationContext created in #createWebApplicationContext. This behavior produces more consistent behavior with regard to delegation of the environment, saves unnecessary cycles by avoiding needless instantiation and calls to methods like StandardServletEnvironment#initPropertySources and leads to better logging output, as the user sees only one Environment created and initialized when working with the FrameworkServlet/DispatcherServlet. This commit also mirrors these changes across the corresponding Portlet* classes. Issue: SPR-9763 --- .../support/AbstractApplicationContext.java | 16 ++++++-- ...tractRefreshableWebApplicationContext.java | 3 +- .../web/portlet/FrameworkPortlet.java | 3 +- .../web/portlet/GenericPortletBean.java | 40 +++++++++++++++---- .../context/StandardPortletEnvironment.java | 3 +- .../web/servlet/FrameworkServlet.java | 1 + .../web/servlet/HttpServletBean.java | 40 +++++++++++++++---- .../web/servlet/DispatcherServletTests.java | 34 +++++++++++++++- 8 files changed, 117 insertions(+), 23 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java b/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java index 726a172aa9..dead77434a 100644 --- a/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java +++ b/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java @@ -73,6 +73,7 @@ import org.springframework.core.Ordered; import org.springframework.core.PriorityOrdered; import org.springframework.core.convert.ConversionService; import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.Environment; import org.springframework.core.env.StandardEnvironment; import org.springframework.core.io.DefaultResourceLoader; import org.springframework.core.io.Resource; @@ -224,7 +225,6 @@ public abstract class AbstractApplicationContext extends DefaultResourceLoader public AbstractApplicationContext(ApplicationContext parent) { this.parent = parent; this.resourcePatternResolver = getResourcePatternResolver(); - this.environment = this.createEnvironment(); } @@ -276,7 +276,15 @@ public abstract class AbstractApplicationContext extends DefaultResourceLoader return this.parent; } + /** + * {@inheritDoc} + *

If {@code null}, a new environment will be initialized via + * {@link #createEnvironment()}. + */ public ConfigurableEnvironment getEnvironment() { + if (this.environment == null) { + this.environment = this.createEnvironment(); + } return this.environment; } @@ -387,9 +395,9 @@ public abstract class AbstractApplicationContext extends DefaultResourceLoader public void setParent(ApplicationContext parent) { this.parent = parent; if (parent != null) { - Object parentEnvironment = parent.getEnvironment(); + Environment parentEnvironment = parent.getEnvironment(); if (parentEnvironment instanceof ConfigurableEnvironment) { - this.environment.merge((ConfigurableEnvironment)parentEnvironment); + this.getEnvironment().merge((ConfigurableEnvironment)parentEnvironment); } } } @@ -505,7 +513,7 @@ public abstract class AbstractApplicationContext extends DefaultResourceLoader // Validate that all properties marked as required are resolvable // see ConfigurablePropertyResolver#setRequiredProperties - this.environment.validateRequiredProperties(); + this.getEnvironment().validateRequiredProperties(); } /** diff --git a/spring-web/src/main/java/org/springframework/web/context/support/AbstractRefreshableWebApplicationContext.java b/spring-web/src/main/java/org/springframework/web/context/support/AbstractRefreshableWebApplicationContext.java index c5e30e9f95..de5e078ec7 100644 --- a/spring-web/src/main/java/org/springframework/web/context/support/AbstractRefreshableWebApplicationContext.java +++ b/spring-web/src/main/java/org/springframework/web/context/support/AbstractRefreshableWebApplicationContext.java @@ -134,7 +134,8 @@ public abstract class AbstractRefreshableWebApplicationContext extends AbstractR } /** - * Create and return a new {@link StandardServletEnvironment}. + * Create and return a new {@link StandardServletEnvironment}. Subclasses may override + * in order to configure the environment or specialize the environment type returned. */ @Override protected ConfigurableEnvironment createEnvironment() { diff --git a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/FrameworkPortlet.java b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/FrameworkPortlet.java index 3ddac18c10..e236a1d226 100644 --- a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/FrameworkPortlet.java +++ b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/FrameworkPortlet.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -345,6 +345,7 @@ public abstract class FrameworkPortlet extends GenericPortletBean pac.setId(ConfigurablePortletApplicationContext.APPLICATION_CONTEXT_ID_PREFIX + getPortletName()); } + pac.setEnvironment(this.getEnvironment()); pac.setParent(parent); pac.setPortletContext(getPortletContext()); pac.setPortletConfig(getPortletConfig()); diff --git a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/GenericPortletBean.java b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/GenericPortletBean.java index bc0b8fead8..6351a90d83 100644 --- a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/GenericPortletBean.java +++ b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/GenericPortletBean.java @@ -34,13 +34,16 @@ import org.springframework.beans.PropertyAccessorFactory; import org.springframework.beans.PropertyValue; import org.springframework.beans.PropertyValues; import org.springframework.context.EnvironmentAware; +import org.springframework.core.env.ConfigurableEnvironment; 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.StringUtils; -import org.springframework.web.portlet.context.StandardPortletEnvironment; import org.springframework.web.portlet.context.PortletContextResourceLoader; +import org.springframework.web.portlet.context.StandardPortletEnvironment; /** * Simple extension of javax.portlet.GenericPortlet that treats @@ -65,7 +68,8 @@ import org.springframework.web.portlet.context.PortletContextResourceLoader; * @see #processAction * @see FrameworkPortlet */ -public abstract class GenericPortletBean extends GenericPortlet implements EnvironmentAware { +public abstract class GenericPortletBean extends GenericPortlet + implements EnvironmentCapable, EnvironmentAware { /** Logger available to subclasses */ protected final Log logger = LogFactory.getLog(getClass()); @@ -76,7 +80,7 @@ public abstract class GenericPortletBean extends GenericPortlet implements Envir */ private final Set requiredProperties = new HashSet(); - private Environment environment = new StandardPortletEnvironment(); + private ConfigurableEnvironment environment; /** @@ -107,7 +111,7 @@ public abstract class GenericPortletBean extends GenericPortlet implements Envir PropertyValues pvs = new PortletConfigPropertyValues(getPortletConfig(), this.requiredProperties); BeanWrapper bw = PropertyAccessorFactory.forBeanPropertyAccess(this); ResourceLoader resourceLoader = new PortletContextResourceLoader(getPortletContext()); - bw.registerCustomEditor(Resource.class, new ResourceEditor(resourceLoader, this.environment)); + bw.registerCustomEditor(Resource.class, new ResourceEditor(resourceLoader, this.getEnvironment())); initBeanWrapper(bw); bw.setPropertyValues(pvs, true); } @@ -167,17 +171,39 @@ public abstract class GenericPortletBean extends GenericPortlet implements Envir /** * {@inheritDoc} - *

Any environment set here overrides the {@link StandardPortletEnvironment} - * provided by default. + * @throws IllegalArgumentException if environment is not assignable to + * {@code ConfigurableEnvironment}. */ public void setEnvironment(Environment environment) { - this.environment = environment; + Assert.isInstanceOf(ConfigurableEnvironment.class, environment); + this.environment = (ConfigurableEnvironment)environment; + } + + /** + * {@inheritDoc} + *

If {@code null}, a new environment will be initialized via + * {@link #createEnvironment()}. + */ + public ConfigurableEnvironment getEnvironment() { + if (this.environment == null) { + this.environment = this.createEnvironment(); + } + return this.environment; + } + + /** + * Create and return a new {@link StandardPortletEnvironment}. Subclasses may override + * in order to configure the environment or specialize the environment type returned. + */ + protected ConfigurableEnvironment createEnvironment() { + return new StandardPortletEnvironment(); } /** * PropertyValues implementation created from PortletConfig init parameters. */ + @SuppressWarnings("serial") private static class PortletConfigPropertyValues extends MutablePropertyValues { /** diff --git a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/context/StandardPortletEnvironment.java b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/context/StandardPortletEnvironment.java index 137d54dc41..7a2afb00b7 100644 --- a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/context/StandardPortletEnvironment.java +++ b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/context/StandardPortletEnvironment.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -75,7 +75,6 @@ public class StandardPortletEnvironment extends StandardEnvironment { * @see org.springframework.core.env.AbstractEnvironment#customizePropertySources * @see PortletConfigPropertySource * @see PortletContextPropertySource - * @see AbstractRefreshablePortletApplicationContext#initPropertySources * @see PortletApplicationContextUtils#initPortletPropertySources */ @Override diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java index 9995e3696e..f4616752a9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java @@ -588,6 +588,7 @@ public abstract class FrameworkServlet extends HttpServletBean { ConfigurableWebApplicationContext wac = (ConfigurableWebApplicationContext) BeanUtils.instantiateClass(contextClass); + wac.setEnvironment(this.getEnvironment()); wac.setParent(parent); wac.setConfigLocation(getContextConfigLocation()); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/HttpServletBean.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/HttpServletBean.java index 127fbae308..ce0df647e0 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/HttpServletBean.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/HttpServletBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -35,11 +35,15 @@ import org.springframework.beans.PropertyAccessorFactory; import org.springframework.beans.PropertyValue; import org.springframework.beans.PropertyValues; import org.springframework.context.EnvironmentAware; +import org.springframework.core.env.ConfigurableEnvironment; 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.StringUtils; +import org.springframework.web.context.ConfigurableWebEnvironment; import org.springframework.web.context.support.StandardServletEnvironment; import org.springframework.web.context.support.ServletContextResourceLoader; @@ -76,7 +80,8 @@ import org.springframework.web.context.support.ServletContextResourceLoader; * @see #doPost */ @SuppressWarnings("serial") -public abstract class HttpServletBean extends HttpServlet implements EnvironmentAware { +public abstract class HttpServletBean extends HttpServlet + implements EnvironmentCapable, EnvironmentAware { /** Logger available to subclasses */ protected final Log logger = LogFactory.getLog(getClass()); @@ -87,7 +92,7 @@ public abstract class HttpServletBean extends HttpServlet implements Environment */ private final Set requiredProperties = new HashSet(); - private Environment environment = new StandardServletEnvironment(); + private ConfigurableWebEnvironment environment; /** @@ -120,7 +125,7 @@ public abstract class HttpServletBean extends HttpServlet implements Environment PropertyValues pvs = new ServletConfigPropertyValues(getServletConfig(), this.requiredProperties); BeanWrapper bw = PropertyAccessorFactory.forBeanPropertyAccess(this); ResourceLoader resourceLoader = new ServletContextResourceLoader(getServletContext()); - bw.registerCustomEditor(Resource.class, new ResourceEditor(resourceLoader, this.environment)); + bw.registerCustomEditor(Resource.class, new ResourceEditor(resourceLoader, this.getEnvironment())); initBeanWrapper(bw); bw.setPropertyValues(pvs, true); } @@ -182,11 +187,32 @@ public abstract class HttpServletBean extends HttpServlet implements Environment /** * {@inheritDoc} - *

Any environment set here overrides the {@link StandardServletEnvironment} - * provided by default. + * @throws IllegalArgumentException if environment is not assignable to + * {@code ConfigurableWebEnvironment}. */ public void setEnvironment(Environment environment) { - this.environment = environment; + Assert.isInstanceOf(ConfigurableWebEnvironment.class, environment); + this.environment = (ConfigurableWebEnvironment)environment; + } + + /** + * {@inheritDoc} + *

If {@code null}, a new environment will be initialized via + * {@link #createEnvironment()}. + */ + public ConfigurableWebEnvironment 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 ConfigurableWebEnvironment createEnvironment() { + return new StandardServletEnvironment(); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java index 92dbe8729b..6a5ec06388 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2012 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. @@ -18,6 +18,7 @@ package org.springframework.web.servlet; import java.io.IOException; import java.util.Locale; + import javax.servlet.Servlet; import javax.servlet.ServletConfig; import javax.servlet.ServletContext; @@ -33,14 +34,18 @@ import org.springframework.beans.PropertyValue; import org.springframework.beans.TestBean; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.support.DefaultMessageSourceResolvable; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.StandardEnvironment; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockServletConfig; import org.springframework.mock.web.MockServletContext; import org.springframework.web.bind.EscapedErrors; +import org.springframework.web.context.ConfigurableWebEnvironment; import org.springframework.web.context.ServletConfigAwareBean; import org.springframework.web.context.ServletContextAwareBean; import org.springframework.web.context.WebApplicationContext; +import org.springframework.web.context.support.StandardServletEnvironment; import org.springframework.web.context.support.StaticWebApplicationContext; import org.springframework.web.multipart.MaxUploadSizeExceededException; import org.springframework.web.multipart.MultipartHttpServletRequest; @@ -55,6 +60,9 @@ import org.springframework.web.servlet.theme.AbstractThemeResolver; import org.springframework.web.servlet.view.InternalResourceViewResolver; import org.springframework.web.util.WebUtils; +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + /** * @author Rod Johnson * @author Juergen Hoeller @@ -823,6 +831,30 @@ public class DispatcherServletTests extends TestCase { servlet.destroy(); } + public void testEnvironmentOperations() { + DispatcherServlet servlet = new DispatcherServlet(); + ConfigurableWebEnvironment defaultEnv = servlet.getEnvironment(); + assertThat(defaultEnv, notNullValue()); + ConfigurableEnvironment env1 = new StandardServletEnvironment(); + servlet.setEnvironment(env1); // should succeed + assertThat(servlet.getEnvironment(), sameInstance(env1)); + try { + servlet.setEnvironment(new StandardEnvironment()); + fail("expected exception"); + } + catch (IllegalArgumentException ex) { + } + class CustomServletEnvironment extends StandardServletEnvironment { } + @SuppressWarnings("serial") + DispatcherServlet custom = new DispatcherServlet() { + @Override + protected ConfigurableWebEnvironment createEnvironment() { + return new CustomServletEnvironment(); + } + }; + assertThat(custom.getEnvironment(), instanceOf(CustomServletEnvironment.class)); + } + public static class ControllerFromParent implements Controller {