WW-4518 Makes Dispatcher immutable

EddyZhan · Jun 23, 2015 · 3c03b49 · 3c03b49
1 parent 0fada66
commit 3c03b49
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 31 deletions.
diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java
@@ -91,11 +91,6 @@ public class Dispatcher {
      */
     private static List<DispatcherListener> dispatcherListeners = new CopyOnWriteArrayList<>();
 
-    /**
-     * Store ConfigurationManager instance, set on init.
-     */
-    private ConfigurationManager configurationManager;
-
     /**
      * Store state of StrutsConstants.STRUTS_DEVMODE setting.
      */
@@ -151,6 +146,11 @@ public class Dispatcher {
      */
     private DispatcherErrorHandler errorHandler;
 
+    /**
+     * Store ConfigurationManager instance, set on init.
+     */
+    protected ConfigurationManager configurationManager;
+
     /**
      * Provide the dispatcher instance for the current thread.
      *
@@ -867,17 +867,6 @@ public ConfigurationManager getConfigurationManager() {
         return configurationManager;
     }
 
-    /**
-     * Modify the ConfigurationManager instance
-     *
-     * @param mgr The configuration manager
-     * @deprecated should be removed as is used only in tests
-     */
-    public void setConfigurationManager(ConfigurationManager mgr) {
-        ContainerHolder.clear();
-        this.configurationManager = mgr;
-    }
-
     /**
      * Expose the dependency injection container.
      * @return Our dependency injection container

diff --git a/core/src/main/java/org/apache/struts2/dispatcher/MockDispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/MockDispatcher.java
@@ -0,0 +1,24 @@
+package org.apache.struts2.dispatcher;
+
+import com.opensymphony.xwork2.config.ConfigurationManager;
+
+import javax.servlet.ServletContext;
+import java.util.HashMap;
+import java.util.Map;
+
+public class MockDispatcher extends Dispatcher {
+
+    private final ConfigurationManager copyConfigurationManager;
+
+    public MockDispatcher(ServletContext servletContext, Map<String, String> context, ConfigurationManager configurationManager) {
+        super(servletContext, context);
+        this.copyConfigurationManager = configurationManager;
+    }
+
+    @Override
+    public void init() {
+        super.init();
+        ContainerHolder.clear();
+        this.configurationManager = copyConfigurationManager;
+    }
+}
diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java
@@ -176,13 +176,10 @@ public void dispatcherInitialized(Dispatcher du) {
 
     public void testConfigurationManager() {
     	Dispatcher du;
-    	InternalConfigurationManager configurationManager = new InternalConfigurationManager();
+    	final InternalConfigurationManager configurationManager = new InternalConfigurationManager();
     	try {
-    		du = new Dispatcher(new MockServletContext(), new HashMap<String, String>());
-    		du.setConfigurationManager(configurationManager);
-
+    		du = new MockDispatcher(new MockServletContext(), new HashMap<String, String>(), configurationManager);
     		du.init();
-
             Dispatcher.setInstance(du);
 
             assertFalse(configurationManager.destroyConfiguration);
@@ -200,8 +197,8 @@ public void testConfigurationManager() {
     public void testObjectFactoryDestroy() throws Exception {
 
         final InnerDestroyableObjectFactory destroyedObjectFactory = new InnerDestroyableObjectFactory();
-        Dispatcher du = new Dispatcher(new MockServletContext(), new HashMap<String, String>());
         ConfigurationManager cm = new ConfigurationManager();
+        Dispatcher du = new MockDispatcher(new MockServletContext(), new HashMap<String, String>(), cm);
         Mock mockConfiguration = new Mock(Configuration.class);
         cm.setConfiguration((Configuration)mockConfiguration.proxy());
 
@@ -216,7 +213,7 @@ public void testObjectFactoryDestroy() throws Exception {
         mockConfiguration.expect("destroy");
         mockConfiguration.matchAndReturn("getPackageConfigs", new HashMap<String, PackageConfig>());
 
-        du.setConfigurationManager(cm);
+        du.init();
         assertFalse(destroyedObjectFactory.destroyed);
         du.cleanup();
         assertTrue(destroyedObjectFactory.destroyed);
@@ -252,8 +249,8 @@ public void testInterceptorDestroy() throws Exception {
         ConfigurationManager configurationManager = new ConfigurationManager();
         configurationManager.setConfiguration((Configuration) mockConfiguration.proxy());
 
-        Dispatcher dispatcher = new Dispatcher(new MockServletContext(), new HashMap<String, String>());
-        dispatcher.setConfigurationManager(configurationManager);
+        Dispatcher dispatcher = new MockDispatcher(new MockServletContext(), new HashMap<String, String>(), configurationManager);
+        dispatcher.init();
         dispatcher.cleanup();
 
         mockInterceptor.verify();

diff --git a/core/src/test/java/org/apache/struts2/dispatcher/FilterDispatcherTest.java b/core/src/test/java/org/apache/struts2/dispatcher/FilterDispatcherTest.java
@@ -56,10 +56,10 @@ public void testIfActionMapperIsNullDontServiceAction() throws Exception {
         MockHttpServletRequest req = new MockHttpServletRequest(servletContext);
         MockHttpServletResponse res = new MockHttpServletResponse();
         MockFilterChain chain = new MockFilterChain();
-        final NoOpDispatcher _dispatcher = new NoOpDispatcher(servletContext);
         ConfigurationManager confManager = new ConfigurationManager();
         confManager.setConfiguration(new DefaultConfiguration());
-        _dispatcher.setConfigurationManager(confManager);
+
+        final NoOpDispatcher _dispatcher = new NoOpDispatcher(servletContext, confManager);
         Dispatcher.setInstance(_dispatcher);
 
 
@@ -112,8 +112,9 @@ public static class NoOpDispatcher extends Dispatcher {
         protected boolean wrappedRequest = false;
         protected boolean serviceRequest = false;
 
-        public NoOpDispatcher(ServletContext servletContext) {
+        public NoOpDispatcher(ServletContext servletContext, ConfigurationManager cm) {
             super(servletContext, new HashMap());
+            this.configurationManager = cm;
         }
 
         @Override

diff --git a/core/src/test/java/org/apache/struts2/views/jsp/AbstractTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/AbstractTagTest.java
@@ -34,6 +34,7 @@
 import org.apache.struts2.TestAction;
 import org.apache.struts2.dispatcher.ApplicationMap;
 import org.apache.struts2.dispatcher.Dispatcher;
+import org.apache.struts2.dispatcher.MockDispatcher;
 import org.apache.struts2.dispatcher.RequestMap;
 import org.apache.struts2.dispatcher.SessionMap;
 
@@ -108,10 +109,9 @@ protected void createMocks() throws Exception {
         pageContext.setServletContext(servletContext);
 
         mockContainer = new Mock(Container.class);
-        Dispatcher du = new Dispatcher(pageContext.getServletContext(), new HashMap());
+        MockDispatcher du = new MockDispatcher(pageContext.getServletContext(), new HashMap<String, String>(), configurationManager);
         du.init();
         Dispatcher.setInstance(du);
-        du.setConfigurationManager(configurationManager);
         session = new SessionMap(request);
         Map<String, Object> extraContext = du.createContextMap(new RequestMap(request),
                 request.getParameterMap(),

diff --git a/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java
@@ -548,7 +548,6 @@ public void testEmptyActionCustomMapper() throws Exception {
 
         mockContainer = new Mock(Container.class);
 
-        du.setConfigurationManager(configurationManager);
         session = new SessionMap(request);
         Map<String, Object> extraContext = du.createContextMap(new RequestMap(request),
                 request.getParameterMap(),