Pass whole DeploymentConfiguration to ApplicationStartEvent and Application; add accessors to supported servlet init params to DeploymentConfiguration; move init param checks and validation to AbstractDeploymentConfiguration (#9340,#9341)

7 files changed, 160 insertions, 148 deletions

diff --git a/src/com/vaadin/Application.java b/src/com/vaadin/Application.java
index 086caa5509..6f4dccd028 100644
--- a/src/com/vaadin/Application.java
+++ b/src/com/vaadin/Application.java
@@ -337,31 +337,24 @@ public class Application implements Terminal.ErrorListener, Serializable {
     public static class ApplicationStartEvent implements Serializable {
         private final URL applicationUrl;
 
-        private final Properties applicationProperties;
+        private final DeploymentConfiguration configuration;
 
         private final ApplicationContext context;
 
-        private final boolean productionMode;
-
         /**
          * @param applicationUrl
          *            the URL the application should respond to.
-         * @param applicationProperties
-         *            the Application properties as specified by the deployment
-         *            configuration.
+         * @param configuration
+         *            the deployment configuration for the application.
          * @param context
          *            the context application will be running in.
-         * @param productionMode
-         *            flag indicating whether the application is running in
-         *            production mode.
          */
         public ApplicationStartEvent(URL applicationUrl,
-                Properties applicationProperties, ApplicationContext context,
-                boolean productionMode) {
+                DeploymentConfiguration configuration,
+                ApplicationContext context) {
             this.applicationUrl = applicationUrl;
-            this.applicationProperties = applicationProperties;
+            this.configuration = configuration;
             this.context = context;
-            this.productionMode = productionMode;
         }
 
         /**
@@ -377,15 +370,12 @@ public class Application implements Terminal.ErrorListener, Serializable {
         }
 
         /**
-         * Gets the Application properties as specified by the deployment
-         * configuration.
-         * 
-         * @return the properties configured for the applciation.
+         * Returns the deployment configuration used by this application.
          * 
-         * @see Application#getProperty(String)
+         * @return the deployment configuration.
          */
-        public Properties getApplicationProperties() {
-            return applicationProperties;
+        public DeploymentConfiguration getConfiguration() {
+            return configuration;
         }
 
         /**
@@ -398,18 +388,6 @@ public class Application implements Terminal.ErrorListener, Serializable {
         public ApplicationContext getContext() {
             return context;
         }
-
-        /**
-         * Checks whether the application is running in production mode.
-         * 
-         * @return <code>true</code> if in production mode, else
-         *         <code>false</code>
-         * 
-         * @see Application#isProductionMode()
-         */
-        public boolean isProductionMode() {
-            return productionMode;
-        }
     }
 
     private final static Logger logger = Logger.getLogger(Application.class
@@ -421,6 +399,11 @@ public class Application implements Terminal.ErrorListener, Serializable {
     private ApplicationContext context;
 
     /**
+     * Deployment configuration for the application.
+     */
+    private DeploymentConfiguration configuration;
+
+    /**
      * The current user or <code>null</code> if no user has logged in.
      */
     private Object user;
@@ -436,11 +419,6 @@ public class Application implements Terminal.ErrorListener, Serializable {
     private volatile boolean applicationIsRunning = false;
 
     /**
-     * Application properties.
-     */
-    private Properties properties;
-
-    /**
      * Default locale of the application.
      */
     private Locale locale;
@@ -488,8 +466,6 @@ public class Application implements Terminal.ErrorListener, Serializable {
     private int nextRootId = 0;
     private Map<Integer, Root> roots = new HashMap<Integer, Root>();
 
-    private boolean productionMode = true;
-
     private final Map<String, Integer> retainOnRefreshRoots = new HashMap<String, Integer>();
 
     /**
@@ -612,8 +588,7 @@ public class Application implements Terminal.ErrorListener, Serializable {
      */
     public void start(ApplicationStartEvent event) {
         applicationUrl = event.getApplicationUrl();
-        productionMode = event.isProductionMode();
-        properties = event.getApplicationProperties();
+        configuration = event.getConfiguration();
         context = event.getContext();
         init();
         applicationIsRunning = true;
@@ -647,6 +622,16 @@ public class Application implements Terminal.ErrorListener, Serializable {
     }
 
     /**
+     * Returns the properties of this application as specified in the deployment
+     * configuration.
+     * 
+     * @return Application properties
+     */
+    protected Properties getProperties() {
+        return configuration.getInitParameters();
+    }
+
+    /**
      * Returns an enumeration of all the names in this application.
      * 
      * <p>
@@ -659,7 +644,7 @@ public class Application implements Terminal.ErrorListener, Serializable {
      * 
      */
     public Enumeration<?> getPropertyNames() {
-        return properties.propertyNames();
+        return getProperties().propertyNames();
     }
 
     /**
@@ -674,7 +659,7 @@ public class Application implements Terminal.ErrorListener, Serializable {
      * @return the value in this property list with the specified key value.
      */
     public String getProperty(String name) {
-        return properties.getProperty(name);
+        return getProperties().getProperty(name);
     }
 
     /**
@@ -1908,7 +1893,7 @@ public class Application implements Terminal.ErrorListener, Serializable {
      * @since 7.0
      */
     protected String getRootClassName(WrappedRequest request) {
-        Object rootClassNameObj = properties.get(ROOT_PARAMETER);
+        Object rootClassNameObj = getProperties().get(ROOT_PARAMETER);
         if (rootClassNameObj instanceof String) {
             return (String) rootClassNameObj;
         } else {
@@ -2159,7 +2144,7 @@ public class Application implements Terminal.ErrorListener, Serializable {
      * @since 7.0
      */
     public boolean isProductionMode() {
-        return productionMode;
+        return configuration.isProductionMode();
     }
 
     /**
diff --git a/src/com/vaadin/terminal/DeploymentConfiguration.java b/src/com/vaadin/terminal/DeploymentConfiguration.java
index 160350ca24..6847072523 100644
--- a/src/com/vaadin/terminal/DeploymentConfiguration.java
+++ b/src/com/vaadin/terminal/DeploymentConfiguration.java
@@ -120,4 +120,25 @@ public interface DeploymentConfiguration extends Serializable {
     public VaadinContext getVaadinContext();
 
     public void setVaadinContext(VaadinContext vaadinContext);
+
+    /**
+     * Returns whether Vaadin is in production mode.
+     * 
+     * @return true if in production mode, false otherwise.
+     */
+    public boolean isProductionMode();
+
+    /**
+     * Returns whether cross-site request forgery protection is enabled.
+     * 
+     * @return true if XSRF protection is enabled, false otherwise.
+     */
+    public boolean isXsrfProtectionEnabled();
+
+    /**
+     * Returns the time resources can be cached in the browsers, in seconds.
+     * 
+     * @return The resource cache time.
+     */
+    public int getResourceCacheTime();
 }
diff --git a/src/com/vaadin/terminal/gwt/server/AbstractApplicationPortlet.java b/src/com/vaadin/terminal/gwt/server/AbstractApplicationPortlet.java
index bb4e8ba0a7..bfd35fd034 100644
--- a/src/com/vaadin/terminal/gwt/server/AbstractApplicationPortlet.java
+++ b/src/com/vaadin/terminal/gwt/server/AbstractApplicationPortlet.java
@@ -316,9 +316,6 @@ public abstract class AbstractApplicationPortlet extends GenericPortlet
                     config.getInitParameter(name));
         }
 
-        checkProductionMode();
-        checkCrossSiteProtection();
-
         vaadinContext.init();
     }
 
@@ -329,34 +326,6 @@ public abstract class AbstractApplicationPortlet extends GenericPortlet
         vaadinContext.destroy();
     }
 
-    private void checkCrossSiteProtection() {
-        if (getDeploymentConfiguration().getApplicationOrSystemProperty(
-                SERVLET_PARAMETER_DISABLE_XSRF_PROTECTION, "false").equals(
-                "true")) {
-            /*
-             * Print an information/warning message about running with xsrf
-             * protection disabled
-             */
-            getLogger().warning(WARNING_XSRF_PROTECTION_DISABLED);
-        }
-    }
-
-    private void checkProductionMode() {
-        // TODO Identical code in AbstractApplicationServlet -> refactor
-        // Check if the application is in production mode.
-        // We are in production mode if productionMode=true
-        if (getDeploymentConfiguration().getApplicationOrSystemProperty(
-                SERVLET_PARAMETER_PRODUCTION_MODE, "false").equals("true")) {
-            productionMode = true;
-        }
-
-        if (!productionMode) {
-            /* Print an information/warning message about running in debug mode */
-            // TODO Maybe we need a different message for portlets?
-            getLogger().warning(NOT_PRODUCTION_MODE_INFO);
-        }
-    }
-
     protected enum RequestType {
         FILE_UPLOAD, UIDL, RENDER, STATIC_FILE, APPLICATION_RESOURCE, DUMMY, EVENT, ACTION, UNKNOWN, BROWSER_DETAILS, CONNECTOR_RESOURCE;
     }
@@ -799,8 +768,7 @@ public abstract class AbstractApplicationPortlet extends GenericPortlet
             application.setLocale(locale);
             // No application URL when running inside a portlet
             application.start(new ApplicationStartEvent(null,
-                    getDeploymentConfiguration().getInitParameters(), context,
-                    isProductionMode()));
+                    getDeploymentConfiguration(), context));
         }
     }
 
diff --git a/src/com/vaadin/terminal/gwt/server/AbstractApplicationServlet.java b/src/com/vaadin/terminal/gwt/server/AbstractApplicationServlet.java
index c2e1d2d3e7..2ba19b0cf6 100644
--- a/src/com/vaadin/terminal/gwt/server/AbstractApplicationServlet.java
+++ b/src/com/vaadin/terminal/gwt/server/AbstractApplicationServlet.java
@@ -87,12 +87,8 @@ public abstract class AbstractApplicationServlet extends HttpServlet implements
     // TODO Move some (all?) of the constants to a separate interface (shared
     // with portlet)
 
-    private boolean productionMode = false;
-
     private final String resourcePath = null;
 
-    private int resourceCacheTime = 3600;
-
     private DeploymentConfiguration deploymentConfiguration = new AbstractDeploymentConfiguration(
             getClass()) {
 
@@ -166,10 +162,6 @@ public abstract class AbstractApplicationServlet extends HttpServlet implements
                     servletConfig.getInitParameter(name));
         }
 
-        checkProductionMode();
-        checkCrossSiteProtection();
-        checkResourceCacheTime();
-
         vaadinContext.init();
     }
 
@@ -180,47 +172,6 @@ public abstract class AbstractApplicationServlet extends HttpServlet implements
         vaadinContext.destroy();
     }
 
-    private void checkCrossSiteProtection() {
-        if (getDeploymentConfiguration().getApplicationOrSystemProperty(
-                SERVLET_PARAMETER_DISABLE_XSRF_PROTECTION, "false").equals(
-                "true")) {
-            /*
-             * Print an information/warning message about running with xsrf
-             * protection disabled
-             */
-            getLogger().warning(WARNING_XSRF_PROTECTION_DISABLED);
-        }
-    }
-
-    private void checkProductionMode() {
-        // Check if the application is in production mode.
-        // We are in production mode if productionMode=true
-        if (getDeploymentConfiguration().getApplicationOrSystemProperty(
-                SERVLET_PARAMETER_PRODUCTION_MODE, "false").equals("true")) {
-            productionMode = true;
-        }
-
-        if (!productionMode) {
-            /* Print an information/warning message about running in debug mode */
-            getLogger().warning(NOT_PRODUCTION_MODE_INFO);
-        }
-
-    }
-
-    private void checkResourceCacheTime() {
-        // Check if the browser caching time has been set in web.xml
-        try {
-            String rct = getDeploymentConfiguration()
-                    .getApplicationOrSystemProperty(
-                            SERVLET_PARAMETER_RESOURCE_CACHE_TIME, "3600");
-            resourceCacheTime = Integer.parseInt(rct);
-        } catch (NumberFormatException nfe) {
-            // Default is 1h
-            resourceCacheTime = 3600;
-            getLogger().warning(WARNING_RESOURCE_CACHING_TIME_NOT_NUMERIC);
-        }
-    }
-
     /**
      * Returns true if the servlet is running in production mode. Production
      * mode disables all debug facilities.
@@ -228,17 +179,17 @@ public abstract class AbstractApplicationServlet extends HttpServlet implements
      * @return true if in production mode, false if in debug mode
      */
     public boolean isProductionMode() {
-        return productionMode;
+        return getDeploymentConfiguration().isProductionMode();
     }
 
     /**
-     * Returns the amount of milliseconds the browser should cache a file.
-     * Default is 1 hour (3600 ms).
+     * Returns the number of seconds the browser should cache a file. Default is
+     * 1 hour (3600 s).
      * 
-     * @return The amount of milliseconds files are cached in the browser
+     * @return The number of seconds files are cached in the browser
      */
     public int getResourceCacheTime() {
-        return resourceCacheTime;
+        return getDeploymentConfiguration().getResourceCacheTime();
     }
 
     /**
@@ -899,8 +850,7 @@ public abstract class AbstractApplicationServlet extends HttpServlet implements
             Locale locale = request.getLocale();
             application.setLocale(locale);
             application.start(new ApplicationStartEvent(applicationUrl,
-                    getDeploymentConfiguration().getInitParameters(),
-                    webApplicationContext, isProductionMode()));
+                    getDeploymentConfiguration(), webApplicationContext));
         }
     }
 
@@ -1045,7 +995,7 @@ public abstract class AbstractApplicationServlet extends HttpServlet implements
              * parameter in web.xml
              */
             response.setHeader("Cache-Control",
-                    "max-age= " + String.valueOf(resourceCacheTime));
+                    "max-age= " + String.valueOf(getResourceCacheTime()));
         }
 
         // Write the resource to the client.
diff --git a/src/com/vaadin/terminal/gwt/server/AbstractDeploymentConfiguration.java b/src/com/vaadin/terminal/gwt/server/AbstractDeploymentConfiguration.java
index 47bf5ecc60..1382c1ed53 100644
--- a/src/com/vaadin/terminal/gwt/server/AbstractDeploymentConfiguration.java
+++ b/src/com/vaadin/terminal/gwt/server/AbstractDeploymentConfiguration.java
@@ -8,6 +8,7 @@ import java.lang.reflect.Constructor;
 import java.util.Iterator;
 import java.util.Properties;
 import java.util.ServiceLoader;
+import java.util.logging.Logger;
 
 import com.vaadin.terminal.DeploymentConfiguration;
 
@@ -17,9 +18,16 @@ public abstract class AbstractDeploymentConfiguration implements
     private final Class<?> systemPropertyBaseClass;
     private final Properties applicationProperties = new Properties();
     private VaadinContext vaadinContext;
+    private boolean productionMode;
+    private boolean xsrfProtectionEnabled;
+    private int resourceCacheTime;
 
     public AbstractDeploymentConfiguration(Class<?> systemPropertyBaseClass) {
         this.systemPropertyBaseClass = systemPropertyBaseClass;
+
+        checkProductionMode();
+        checkXsrfProtection();
+        checkResourceCacheTime();
     }
 
     @Override
@@ -140,4 +148,63 @@ public abstract class AbstractDeploymentConfiguration implements
     public VaadinContext getVaadinContext() {
         return vaadinContext;
     }
+
+    @Override
+    public boolean isProductionMode() {
+        return productionMode;
+    }
+
+    @Override
+    public boolean isXsrfProtectionEnabled() {
+        return xsrfProtectionEnabled;
+    }
+
+    @Override
+    public int getResourceCacheTime() {
+        return resourceCacheTime;
+    }
+
+    /**
+     * Log a warning if Vaadin is not running in production mode.
+     */
+    private void checkProductionMode() {
+        productionMode = getApplicationOrSystemProperty(
+                Constants.SERVLET_PARAMETER_PRODUCTION_MODE, "false").equals(
+                "true");
+        if (!productionMode) {
+            getLogger().warning(Constants.NOT_PRODUCTION_MODE_INFO);
+        }
+    }
+
+    /**
+     * Log a warning if cross-site request forgery protection is disabled.
+     */
+    private void checkXsrfProtection() {
+        xsrfProtectionEnabled = getApplicationOrSystemProperty(
+                Constants.SERVLET_PARAMETER_DISABLE_XSRF_PROTECTION, "false")
+                .equals("true");
+        if (!xsrfProtectionEnabled) {
+            getLogger().warning(Constants.WARNING_XSRF_PROTECTION_DISABLED);
+        }
+    }
+
+    /**
+     * Log a warning if resource cache time is set but is not an integer.
+     */
+    private void checkResourceCacheTime() {
+        try {
+            resourceCacheTime = Integer
+                    .parseInt(getApplicationOrSystemProperty(
+                            Constants.SERVLET_PARAMETER_RESOURCE_CACHE_TIME,
+                            "3600"));
+        } catch (NumberFormatException e) {
+            getLogger().warning(
+                    Constants.WARNING_RESOURCE_CACHING_TIME_NOT_NUMERIC);
+            resourceCacheTime = 3600;
+        }
+    }
+
+    private Logger getLogger() {
+        return Logger.getLogger(getClass().getName());
+    }
 }
diff --git a/tests/server-side/com/vaadin/tests/server/TransactionListenersConcurrency.java b/tests/server-side/com/vaadin/tests/server/TransactionListenersConcurrency.java
index b567617fdd..f7ac55b6da 100644
--- a/tests/server-side/com/vaadin/tests/server/TransactionListenersConcurrency.java
+++ b/tests/server-side/com/vaadin/tests/server/TransactionListenersConcurrency.java
@@ -17,14 +17,15 @@ import javax.servlet.http.HttpSession;
 
 import junit.framework.TestCase;
 
-import org.easymock.EasyMock;
-
 import com.vaadin.Application;
 import com.vaadin.Application.ApplicationStartEvent;
 import com.vaadin.service.ApplicationContext.TransactionListener;
+import com.vaadin.terminal.DeploymentConfiguration;
 import com.vaadin.terminal.gwt.server.AbstractWebApplicationContext;
 import com.vaadin.terminal.gwt.server.WebApplicationContext;
 
+import org.easymock.EasyMock;
+
 public class TransactionListenersConcurrency extends TestCase {
 
     /**
@@ -71,10 +72,15 @@ public class TransactionListenersConcurrency extends TestCase {
                     // Start the application so the transaction listener is
                     // called later on.
                     try {
+                        DeploymentConfiguration dc = EasyMock
+                                .createMock(DeploymentConfiguration.class);
+                        EasyMock.expect(dc.isProductionMode()).andReturn(true);
+                        EasyMock.expect(dc.getInitParameters()).andReturn(
+                                new Properties());
+                        EasyMock.replay(dc);
 
                         app.start(new ApplicationStartEvent(new URL(
-                                "http://localhost/"), new Properties(),
-                                context, true));
+                                "http://localhost/"), dc, context));
                     } catch (MalformedURLException e) {
                         // TODO Auto-generated catch block
                         e.printStackTrace();
@@ -101,7 +107,9 @@ public class TransactionListenersConcurrency extends TestCase {
 
                 @Override
                 public void uncaughtException(Thread t, Throwable e) {
-                    e = e.getCause();
+                    if (e.getCause() != null) {
+                        e = e.getCause();
+                    }
                     exceptions.add(e);
                 }
             });
@@ -127,8 +135,10 @@ public class TransactionListenersConcurrency extends TestCase {
             if (t instanceof InvocationTargetException) {
                 t = t.getCause();
             }
-            t.printStackTrace(System.err);
-            fail(t.getClass().getName());
+            if (t != null) {
+                t.printStackTrace(System.err);
+                fail(t.getClass().getName());
+            }
         }
 
         System.out.println("Done, all ok");
diff --git a/tests/server-side/com/vaadin/tests/server/component/root/CustomRootClassLoader.java b/tests/server-side/com/vaadin/tests/server/component/root/CustomRootClassLoader.java
index aa9753ebcc..fa730515a2 100644
--- a/tests/server-side/com/vaadin/tests/server/component/root/CustomRootClassLoader.java
+++ b/tests/server-side/com/vaadin/tests/server/component/root/CustomRootClassLoader.java
@@ -6,8 +6,6 @@ import java.util.Properties;
 
 import junit.framework.TestCase;
 
-import org.easymock.EasyMock;
-
 import com.vaadin.Application;
 import com.vaadin.Application.ApplicationStartEvent;
 import com.vaadin.RootRequiresMoreInformationException;
@@ -15,6 +13,8 @@ import com.vaadin.terminal.DeploymentConfiguration;
 import com.vaadin.terminal.WrappedRequest;
 import com.vaadin.ui.Root;
 
+import org.easymock.EasyMock;
+
 public class CustomRootClassLoader extends TestCase {
 
     /**
@@ -52,13 +52,24 @@ public class CustomRootClassLoader extends TestCase {
      */
     public void testWithNullClassLoader() throws Exception {
         Application application = createStubApplication();
-        application.start(new ApplicationStartEvent(null, new Properties(),
-                null, false));
+        application.start(new ApplicationStartEvent(null,
+                createConfigurationMock(), null));
 
         Root root = application.getRootForRequest(createRequestMock(null));
         assertTrue(root instanceof MyRoot);
     }
 
+    private static DeploymentConfiguration createConfigurationMock() {
+        DeploymentConfiguration configurationMock = EasyMock
+                .createMock(DeploymentConfiguration.class);
+        EasyMock.expect(configurationMock.isProductionMode()).andReturn(false);
+        EasyMock.expect(configurationMock.getInitParameters()).andReturn(
+                new Properties());
+
+        EasyMock.replay(configurationMock);
+        return configurationMock;
+    }
+
     private static WrappedRequest createRequestMock(ClassLoader classloader) {
         // Mock a DeploymentConfiguration to give the passed classloader
         DeploymentConfiguration configurationMock = EasyMock
@@ -86,8 +97,8 @@ public class CustomRootClassLoader extends TestCase {
         LoggingClassLoader loggingClassLoader = new LoggingClassLoader();
 
         Application application = createStubApplication();
-        application.start(new ApplicationStartEvent(null, new Properties(),
-                null, false));
+        application.start(new ApplicationStartEvent(null,
+                createConfigurationMock(), null));
 
         Root root = application
                 .getRootForRequest(createRequestMock(loggingClassLoader));