Skip to content

Commit

Permalink
Refactor to lazy Environment creation where possible
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cbeams committed Sep 5, 2012
1 parent 9f8d219 commit 6517517
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
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;
Expand Down Expand Up @@ -224,7 +225,6 @@ public AbstractApplicationContext() {
public AbstractApplicationContext(ApplicationContext parent) {
this.parent = parent;
this.resourcePatternResolver = getResourcePatternResolver();
this.environment = this.createEnvironment();
}


Expand Down Expand Up @@ -276,7 +276,15 @@ public ApplicationContext getParent() {
return this.parent;
}

/**
* {@inheritDoc}
* <p>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;
}

Expand Down Expand Up @@ -387,9 +395,9 @@ protected ResourcePatternResolver getResourcePatternResolver() {
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);
}
}
}
Expand Down Expand Up @@ -505,7 +513,7 @@ protected void prepareRefresh() {

// Validate that all properties marked as required are resolvable
// see ConfigurablePropertyResolver#setRequiredProperties
this.environment.validateRequiredProperties();
this.getEnvironment().validateRequiredProperties();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ public String[] getConfigLocations() {
}

/**
* 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() {
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -345,6 +345,7 @@ protected ApplicationContext createPortletApplicationContext(ApplicationContext
pac.setId(ConfigurablePortletApplicationContext.APPLICATION_CONTEXT_ID_PREFIX + getPortletName());
}

pac.setEnvironment(this.getEnvironment());
pac.setParent(parent);
pac.setPortletContext(getPortletContext());
pac.setPortletConfig(getPortletConfig());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,16 @@
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 <code>javax.portlet.GenericPortlet</code> that treats
Expand All @@ -65,7 +68,8 @@
* @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());
Expand All @@ -76,7 +80,7 @@ public abstract class GenericPortletBean extends GenericPortlet implements Envir
*/
private final Set<String> requiredProperties = new HashSet<String>();

private Environment environment = new StandardPortletEnvironment();
private ConfigurableEnvironment environment;


/**
Expand Down Expand Up @@ -107,7 +111,7 @@ public final void init() throws PortletException {
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);
}
Expand Down Expand Up @@ -167,17 +171,39 @@ protected void initPortletBean() throws PortletException {

/**
* {@inheritDoc}
* <p>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}
* <p>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 {

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,7 @@ protected WebApplicationContext createWebApplicationContext(ApplicationContext p
ConfigurableWebApplicationContext wac =
(ConfigurableWebApplicationContext) BeanUtils.instantiateClass(contextClass);

wac.setEnvironment(this.getEnvironment());
wac.setParent(parent);
wac.setConfigLocation(getContextConfigLocation());

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -35,11 +35,15 @@
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;

Expand Down Expand Up @@ -76,7 +80,8 @@
* @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());
Expand All @@ -87,7 +92,7 @@ public abstract class HttpServletBean extends HttpServlet implements Environment
*/
private final Set<String> requiredProperties = new HashSet<String>();

private Environment environment = new StandardServletEnvironment();
private ConfigurableWebEnvironment environment;


/**
Expand Down Expand Up @@ -120,7 +125,7 @@ public final void init() throws ServletException {
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);
}
Expand Down Expand Up @@ -182,11 +187,32 @@ protected void initServletBean() throws ServletException {

/**
* {@inheritDoc}
* <p>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}
* <p>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();
}


Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -18,6 +18,7 @@

import java.io.IOException;
import java.util.Locale;

import javax.servlet.Servlet;
import javax.servlet.ServletConfig;
import javax.servlet.ServletContext;
Expand All @@ -33,14 +34,18 @@
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;
Expand All @@ -55,6 +60,9 @@
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
Expand Down Expand Up @@ -823,6 +831,30 @@ public void testDispatcherServletContextRefresh() throws ServletException {
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 {

Expand Down

0 comments on commit 6517517

Please sign in to comment.