WW-5382 Rework Dispatcher injections

core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java

@@ -64,6 +64,7 @@
 import org.apache.struts2.config.StrutsJavaConfiguration;
 import org.apache.struts2.config.StrutsJavaConfigurationProvider;
 import org.apache.struts2.config.StrutsXmlConfigurationProvider;
+import org.apache.struts2.dispatcher.mapper.ActionMapper;
 import org.apache.struts2.dispatcher.mapper.ActionMapping;
 import org.apache.struts2.dispatcher.multipart.MultiPartRequest;
 import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper;
@@ -119,6 +120,10 @@ public class Dispatcher {
      */
     private static final List<DispatcherListener> dispatcherListeners = new CopyOnWriteArrayList<>();
 
+    /**
+     * This field exists so {@link #getContainer()} can determine whether to (re-)inject this instance in the case of
+     * a {@link ConfigurationManager} reload.
+     */
     private Container injectedContainer;
 
     /**
@@ -146,11 +151,6 @@ public class Dispatcher {
      */
     private String multipartSaveDir;
 
-    /**
-     * Stores the value of {@link StrutsConstants#STRUTS_MULTIPART_PARSER} setting
-     */
-    private String multipartHandlerName;
-
     /**
      * Stores the value of {@link StrutsConstants#STRUTS_MULTIPART_ENABLED}
      */
@@ -194,6 +194,11 @@ public class Dispatcher {
      * Store ConfigurationManager instance, set on init.
      */
     protected ConfigurationManager configurationManager;
+    private ObjectFactory objectFactory;
+    private ActionProxyFactory actionProxyFactory;
+    private LocaleProviderFactory localeProviderFactory;
+    private StaticContentLoader staticContentLoader;
+    private ActionMapper actionMapper;
 
     /**
      * Provide the dispatcher instance for the current thread.
@@ -213,6 +218,13 @@ public static void setInstance(Dispatcher instance) {
         Dispatcher.instance.set(instance);
     }
 
+    /**
+     * Removes the dispatcher instance for this thread.
+     */
+    public static void clearInstance() {
+        Dispatcher.instance.remove();
+    }
+
     /**
      * Add a dispatcher lifecycle listener.
      *
@@ -308,9 +320,12 @@ public void setMultipartSaveDir(String val) {
         multipartSaveDir = val;
     }
 
-    @Inject(StrutsConstants.STRUTS_MULTIPART_PARSER)
+    /**
+     * @deprecated since 6.4.0, no replacement.
+     */
+    @Deprecated
     public void setMultipartHandler(String val) {
-        multipartHandlerName = val;
+        // no-op
     }
 
     @Inject(value = StrutsConstants.STRUTS_MULTIPART_ENABLED, required = false)
@@ -328,6 +343,10 @@ public void setValueStackFactory(ValueStackFactory valueStackFactory) {
         this.valueStackFactory = valueStackFactory;
     }
 
+    public ValueStackFactory getValueStackFactory() {
+        return valueStackFactory;
+    }
+
     @Inject(StrutsConstants.STRUTS_HANDLE_EXCEPTION)
     public void setHandleException(String handleException) {
         this.handleException = Boolean.parseBoolean(handleException);
@@ -348,12 +367,48 @@ public void setDispatcherErrorHandler(DispatcherErrorHandler errorHandler) {
         this.errorHandler = errorHandler;
     }
 
+    @Inject
+    public void setObjectFactory(ObjectFactory objectFactory) {
+        this.objectFactory = objectFactory;
+    }
+
+    @Inject
+    public void setActionProxyFactory(ActionProxyFactory actionProxyFactory) {
+        this.actionProxyFactory = actionProxyFactory;
+    }
+
+    public ActionProxyFactory getActionProxyFactory() {
+        return actionProxyFactory;
+    }
+
+    @Inject
+    public void setLocaleProviderFactory(LocaleProviderFactory localeProviderFactory) {
+        this.localeProviderFactory = localeProviderFactory;
+    }
+
+    @Inject
+    public void setStaticContentLoader(StaticContentLoader staticContentLoader) {
+        this.staticContentLoader = staticContentLoader;
+    }
+
+    public StaticContentLoader getStaticContentLoader() {
+        return staticContentLoader;
+    }
+
+    @Inject
+    public void setActionMapper(ActionMapper actionMapper) {
+        this.actionMapper = actionMapper;
+    }
+
+    public ActionMapper getActionMapper() {
+        return actionMapper;
+    }
+
     /**
      * Releases all instances bound to this dispatcher instance.
      */
     public void cleanup() {
         // clean up ObjectFactory
-        ObjectFactory objectFactory = getContainer().getInstance(ObjectFactory.class);
         if (objectFactory == null) {
             LOG.warn("Object Factory is null, something is seriously wrong, no clean up will be performed");
         }
@@ -540,10 +595,6 @@ private void init_DeferredXmlConfigurations() {
         loadConfigPaths("struts-deferred.xml");
     }
 
-    private Container init_PreloadConfiguration() {
-        return getContainer();
-    }
-
     /**
      * Load configurations, including both XML and zero-configuration strategies,
      * and update optional settings, including whether to reload configurations and resource files.
@@ -684,7 +735,6 @@ protected ActionProxy prepareActionProxy(Map<String, Object> extraContext, Strin
     }
 
     protected ActionProxy createActionProxy(String namespace, String name, String method, Map<String, Object> extraContext) {
-        ActionProxyFactory actionProxyFactory = getContainer().getInstance(ActionProxyFactory.class);
         return actionProxyFactory.createActionProxy(namespace, name, method, extraContext, true, false);
     }
 
@@ -860,6 +910,7 @@ protected String getSaveDir() {
      * @param response The response
      */
     public void prepare(HttpServletRequest request, HttpServletResponse response) {
+        getContainer(); // Init ContainerHolder and reinject this instance IF ConfigurationManager was reloaded
         String encoding = null;
         if (defaultEncoding != null) {
             encoding = defaultEncoding;
@@ -932,15 +983,12 @@ public HttpServletRequest wrapRequest(HttpServletRequest request) throws IOExcep
         }
 
         if (isMultipartSupportEnabled(request) && isMultipartRequest(request)) {
-            MultiPartRequest multiPartRequest = getMultiPartRequest();
-            LocaleProviderFactory localeProviderFactory = getContainer().getInstance(LocaleProviderFactory.class);
-
             request = new MultiPartRequestWrapper(
-                multiPartRequest,
-                request,
-                getSaveDir(),
-                localeProviderFactory.createLocaleProvider(),
-                disableRequestAttributeValueStackLookup
+                    getMultiPartRequest(),
+                    request,
+                    getSaveDir(),
+                    localeProviderFactory.createLocaleProvider(),
+                    disableRequestAttributeValueStackLookup
             );
         } else {
             request = new StrutsRequestWrapper(request, disableRequestAttributeValueStackLookup);
@@ -1065,5 +1113,4 @@ public Container getContainer() {
         }
         return ContainerHolder.get();
     }
-
 }

core/src/main/java/org/apache/struts2/dispatcher/ExecuteOperations.java

@@ -18,8 +18,8 @@
  */
 package org.apache.struts2.dispatcher;
 
-import org.apache.struts2.dispatcher.mapper.ActionMapping;
 import org.apache.struts2.RequestUtils;
+import org.apache.struts2.dispatcher.mapper.ActionMapping;
 
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -54,7 +54,7 @@ public boolean executeStaticResourceRequest(HttpServletRequest request, HttpServ
             resourcePath = request.getPathInfo();
         }
 
-        StaticContentLoader staticResourceLoader = dispatcher.getContainer().getInstance(StaticContentLoader.class);
+        StaticContentLoader staticResourceLoader = dispatcher.getStaticContentLoader();
         if (staticResourceLoader.canHandle(resourcePath)) {
             staticResourceLoader.findStaticResource(resourcePath, request, response);
             // The framework did its job here

core/src/main/java/org/apache/struts2/dispatcher/InitOperations.java

@@ -57,7 +57,7 @@ public Dispatcher initDispatcher(HostConfig filterConfig) {
      * @return the static content loader
      */
     public StaticContentLoader initStaticContentLoader(HostConfig filterConfig, Dispatcher dispatcher) {
-        StaticContentLoader loader = dispatcher.getContainer().getInstance(StaticContentLoader.class);
+        StaticContentLoader loader = dispatcher.getStaticContentLoader();
         loader.setHostConfig(filterConfig);
         return loader;
     }

core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java

@@ -20,13 +20,11 @@
 
 import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.util.ValueStack;
-import com.opensymphony.xwork2.util.ValueStackFactory;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.struts2.RequestUtils;
 import org.apache.struts2.ServletActionContext;
 import org.apache.struts2.StrutsException;
-import org.apache.struts2.dispatcher.mapper.ActionMapper;
 import org.apache.struts2.dispatcher.mapper.ActionMapping;
 
 import javax.servlet.ServletException;
@@ -78,7 +76,7 @@ public void cleanupRequest(final HttpServletRequest request) {
                 dispatcher.cleanUpRequest(request);
             } finally {
                 ActionContext.clear();
-                Dispatcher.setInstance(null);
+                Dispatcher.clearInstance();
                 devModeOverride.remove();
             }
         });
@@ -101,7 +99,7 @@ public ActionContext createActionContext(HttpServletRequest request, HttpServlet
         } else {
             ctx = ServletActionContext.getActionContext(request);   //checks if we are probably in an async
             if (ctx == null) {
-                ValueStack stack = dispatcher.getContainer().getInstance(ValueStackFactory.class).createValueStack();
+                ValueStack stack = dispatcher.getValueStackFactory().createValueStack();
                 stack.getContext().putAll(dispatcher.createContextMap(request, response, null));
                 ctx = ActionContext.of(stack.getContext()).bind();
             }
@@ -188,7 +186,7 @@ public ActionMapping findActionMapping(HttpServletRequest request, HttpServletRe
         Object mappingAttr = request.getAttribute(STRUTS_ACTION_MAPPING_KEY);
         if (mappingAttr == null || forceLookup) {
             try {
-                mapping = dispatcher.getContainer().getInstance(ActionMapper.class).getMapping(request, dispatcher.getConfigurationManager());
+                mapping = dispatcher.getActionMapper().getMapping(request, dispatcher.getConfigurationManager());
                 if (mapping != null) {
                     request.setAttribute(STRUTS_ACTION_MAPPING_KEY, mapping);
                 } else {

core/src/main/java/org/apache/struts2/util/StrutsTestCaseHelper.java

@@ -59,7 +59,7 @@ public static void tearDown(Dispatcher dispatcher) {
 
     public static void tearDown() {
         (new Dispatcher(null, null)).cleanUpAfterInit(); // Clear ContainerHolder
-        Dispatcher.setInstance(null);
+        Dispatcher.clearInstance();
         ActionContext.clear();
     }
 

core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java

@@ -251,12 +251,8 @@ public void testInitLoadsDefaultConfig() {
 
     @Test
     public void testObjectFactoryDestroy() {
-        dispatcher = spy(dispatcher);
-        Container spiedContainer = spy(container);
-        doReturn(spiedContainer).when(dispatcher).getContainer();
-
         InnerDestroyableObjectFactory destroyedObjectFactory = new InnerDestroyableObjectFactory();
-        doReturn(destroyedObjectFactory).when(spiedContainer).getInstance(ObjectFactory.class);
+        dispatcher.setObjectFactory(destroyedObjectFactory);
 
         assertFalse(destroyedObjectFactory.destroyed);
         dispatcher.cleanup();

plugins/dwr/src/main/java/org/apache/struts2/validators/DWRValidator.java

@@ -18,31 +18,26 @@
  */
 package org.apache.struts2.validators;
 
-import java.util.HashMap;
-import java.util.Map;
-
-import javax.servlet.ServletContext;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
-import org.apache.struts2.ServletActionContext;
-import org.apache.struts2.dispatcher.ApplicationMap;
-import org.apache.struts2.dispatcher.Dispatcher;
-import org.apache.struts2.dispatcher.HttpParameters;
-import org.apache.struts2.dispatcher.RequestMap;
-import org.apache.struts2.dispatcher.SessionMap;
-
-import org.directwebremoting.WebContextFactory;
-
 import com.opensymphony.xwork2.Action;
 import com.opensymphony.xwork2.ActionProxy;
 import com.opensymphony.xwork2.ActionProxyFactory;
 import com.opensymphony.xwork2.DefaultActionInvocation;
-import com.opensymphony.xwork2.interceptor.ValidationAware;
 import com.opensymphony.xwork2.ValidationAwareSupport;
 import com.opensymphony.xwork2.config.entities.ActionConfig;
-import org.apache.logging.log4j.Logger;
+import com.opensymphony.xwork2.interceptor.ValidationAware;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.struts2.dispatcher.ApplicationMap;
+import org.apache.struts2.dispatcher.Dispatcher;
+import org.apache.struts2.dispatcher.HttpParameters;
+import org.apache.struts2.dispatcher.RequestMap;
+import org.apache.struts2.dispatcher.SessionMap;
+import org.directwebremoting.WebContextFactory;
+
+import javax.servlet.ServletContext;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.util.Map;
 
 /**
  * <p>
@@ -87,7 +82,7 @@ public ValidationAwareSupport doPost(String namespace, String actionName, Map pa
                 res);
 
         try {
-            ActionProxyFactory actionProxyFactory = du.getContainer().getInstance(ActionProxyFactory.class);
+            ActionProxyFactory actionProxyFactory = du.getActionProxyFactory();
             ActionProxy proxy = actionProxyFactory.createActionProxy(namespace, actionName, null, ctx, true, true);
             proxy.execute();
             Object action = proxy.getAction();