Commits

Anonymous committed 0dc4327

Refactored ActionProxy/ActionInvocation/ActionContext interrelationships... ActionProxy now defines lifecycle of ActionContext so that new ActionContext is only applied during execute() of proxy.

git-svn-id: http://svn.opensymphony.com/svn/xwork/trunk/src@70e221344d-f017-0410-9bd5-d282ab1896d7

  • Participants
  • Parent commits 7d34bcd

Comments (0)

Files changed (10)

java/com/opensymphony/xwork/ActionInvocation.java

 public interface ActionInvocation extends Serializable {
     //~ Methods ////////////////////////////////////////////////////////////////
 
+    /**
+     * Get the Action associated with this ActionInvocation
+     * @return
+     */
     Action getAction();
 
     boolean isExecuted();
 
+    /**
+     * Gets the ActionContext associated with this ActionInvocation. The ActionProxy is
+     * responsible for setting this ActionContext onto the ThreadLocal before invoking
+     * the ActionInvocation and resetting the old ActionContext afterwards.
+     * @return
+     */
+    ActionContext getInvocationContext();
+
     ActionProxy getProxy();
 
     /**
      */
     Result getResult() throws Exception;
 
+    /**
+     * Gets the result code returned from this ActionInvocation
+     * @return
+     */
     String getResultCode();
 
     OgnlValueStack getStack();
 
+    /**
+     * Invokes the next step in processing this ActionInvocation. If there are more
+     * Interceptors, this will call the next one. If Interceptors choose not to short-circuit
+     * ActionInvocation processing and return their own return code, they will call
+     * invoke() to allow the next Interceptor to execute. If there are no more Interceptors
+     * to be applied, the Action is executed.
+     * @return
+     * @throws Exception
+     */
     String invoke() throws Exception;
 }

java/com/opensymphony/xwork/ActionProxy.java

 
     String getNamespace();
 
-    OgnlValueStack getValueStack();
-
     String execute() throws Exception;
 }

java/com/opensymphony/xwork/DefaultActionInvocation.java

 
     protected Action action;
     protected ActionProxy proxy;
-    ActionContext nestedContext;
+    ActionContext invocationContext;
     Iterator interceptors;
     Map extraContext;
     OgnlValueStack stack;
         return executed;
     }
 
+    public ActionContext getInvocationContext() {
+        return invocationContext;
+    }
+
     public ActionProxy getProxy() {
         return proxy;
     }
 
     /**
-     * If the DefaultActionInvocation has been executed before and the Result is an instance of ActionChainResult, this method
-     * will walk down the chain of ActionChainResults until it finds a non-chain result, which will be returned. If the
-     * DefaultActionInvocation's result has not been executed before, the Result instance will be created and populated with
-     * the result params.
-     * @return a Result instance
-     * @throws Exception
-     */
+ * If the DefaultActionInvocation has been executed before and the Result is an instance of ActionChainResult, this method
+ * will walk down the chain of ActionChainResults until it finds a non-chain result, which will be returned. If the
+ * DefaultActionInvocation's result has not been executed before, the Result instance will be created and populated with
+ * the result params.
+ * @return a Result instance
+ * @throws Exception
+ */
     public Result getResult() throws Exception {
         if (result != null) {
             Result returnResult = result;
             action = (Action) proxy.getConfig().getClazz().newInstance();
         } catch (Exception e) {
             String gripe = "";
+
             if (proxy == null) {
                 gripe = "Whoa!  No ActionProxy instance found in current ActionInvocation.  This is bad ... very bad";
             } else if (proxy.getConfig() == null) {
             } else if (proxy.getConfig().getClazz() == null) {
                 gripe = "No Action defined for '" + proxy.getActionName() + "' in namespace '" + proxy.getNamespace() + "'";
             } else {
-                gripe = "Unable to instantiate Action, " + proxy.getConfig().getClazz().getName() +
-                        ",  defined for '" + proxy.getActionName() +
-                        "' in namespace '" + proxy.getNamespace() + "'";
+                gripe = "Unable to instantiate Action, " + proxy.getConfig().getClazz().getName() + ",  defined for '" + proxy.getActionName() + "' in namespace '" + proxy.getNamespace() + "'";
             }
 
-            gripe += " -- " + e.getMessage();
+            gripe += (" -- " + e.getMessage());
             throw new IllegalArgumentException(gripe);
         }
     }
             contextMap = stack.getContext();
         } else {
             // create the value stack
+            // this also adds the ValueStack to its context
             stack = new OgnlValueStack();
 
             // create the action context
     }
 
     /**
-     * Uses getResult to get the final Result and executes it
-     */
+ * Uses getResult to get the final Result and executes it
+ */
     private void executeResult() throws Exception {
         Result aResult = getResult();
 
         createAction();
 
         Map contextMap = createContextMap();
-        ActionContext context = new ActionContext(contextMap);
-        ActionContext.setContext(context);
+        invocationContext = new ActionContext(contextMap);
+        invocationContext.setName(proxy.getActionName());
 
         // get a new List so we don't get problems with the iterator if someone changes the list
         List interceptorList = new ArrayList(proxy.getConfig().getInterceptors());

java/com/opensymphony/xwork/DefaultActionProxy.java

 import com.opensymphony.xwork.config.ConfigurationManager;
 import com.opensymphony.xwork.config.entities.ActionConfig;
 import com.opensymphony.xwork.util.LocalizedTextUtil;
-import com.opensymphony.xwork.util.OgnlValueStack;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
     String actionName;
     String namespace;
     boolean executeResult;
-    private transient ActionContext nestedContext;
 
     //~ Constructors ///////////////////////////////////////////////////////////
 
     /**
-* This constructor is private so the builder methods (create*) should be used to create an DefaultActionProxy.
-*
-* The reason for the builder methods is so that you can use a subclass to create your own DefaultActionProxy instance
-* (like a RMIActionProxy).
-*/
+    * This constructor is private so the builder methods (create*) should be used to create an DefaultActionProxy.
+    *
+    * The reason for the builder methods is so that you can use a subclass to create your own DefaultActionProxy instance
+    * (like a RMIActionProxy).
+    */
     protected DefaultActionProxy(String namespace, String actionName, Map extraContext, boolean executeResult) throws Exception {
         if (LOG.isDebugEnabled()) {
             LOG.debug("Creating an DefaultActionProxy for namespace " + namespace + " and action name " + actionName);
         return namespace;
     }
 
-    public OgnlValueStack getValueStack() {
-        return invocation.getStack();
-    }
-
     public String execute() throws Exception {
+        ActionContext nestedContext = ActionContext.getContext();
+        ActionContext.setContext(invocation.getInvocationContext());
+
         String retCode = null;
         retCode = invocation.invoke();
 
     }
 
     protected void prepare() throws Exception {
-        nestedContext = ActionContext.getContext();
-
-        // this will set up a new ActionContext on the ThreadLocal
         invocation = ActionProxyFactory.getFactory().createActionInvocation(this, extraContext);
-        ActionContext.getContext().setName(actionName);
     }
 }

java/com/opensymphony/xwork/XworkException.java

             return super.toString();
         }
 
-        return super.toString() + " with nested exception " + throwable.toString();
+        return super.toString() + "\n    with nested exception \n" + throwable.toString();
     }
 }

test/com/opensymphony/xwork/ActionInvocationTest.java

 public class ActionInvocationTest extends TestCase {
     //~ Methods ////////////////////////////////////////////////////////////////
 
-    public void testActionContextHasActionInvocation() {
-        try {
-            ActionProxy proxy = ActionProxyFactory.getFactory().createActionProxy("", "Foo", null);
-            assertEquals(ActionContext.getContext().getActionInvocation(), proxy.getInvocation());
-
-            // do some cleanup
-            ActionContext.setContext(null);
-        } catch (Exception e) {
-            e.printStackTrace();
-            fail();
-        }
-    }
-
     public void testCommandInvocation() throws Exception {
         ActionProxy baseActionProxy = ActionProxyFactory.getFactory().createActionProxy("baz", "commandTest", null);
         assertEquals("success", baseActionProxy.execute());
         try {
             ActionProxy proxy = ActionProxyFactory.getFactory().createActionProxy("", "Foo", extraContext);
             proxy.execute();
-            assertEquals("this is blah", proxy.getValueStack().findValue("blah"));
+            assertEquals("this is blah", proxy.getInvocation().getStack().findValue("blah"));
         } catch (Exception e) {
             e.printStackTrace();
             fail();

test/com/opensymphony/xwork/DefaultActionProxyTest.java

-/*
- * Copyright (c) 2002-2003 by OpenSymphony
- * All rights reserved.
- */
-package com.opensymphony.xwork;
-
-import com.opensymphony.xwork.config.ConfigurationManager;
-import com.opensymphony.xwork.config.providers.MockConfigurationProvider;
-import junit.framework.TestCase;
-
-
-/**
- * DefaultActionProxyTest
- * @author Jason Carreira
- * Created Aug 8, 2003 1:41:45 AM
- */
-public class DefaultActionProxyTest extends TestCase {
-    //~ Methods ////////////////////////////////////////////////////////////////
-
-    public void setUp() {
-        ConfigurationManager.destroyConfiguration();
-        ConfigurationManager.addConfigurationProvider(new MockConfigurationProvider());
-    }
-
-    public void testActionNameIsSet() throws Exception {
-        ActionProxy proxy = ActionProxyFactory.getFactory().createActionProxy("", MockConfigurationProvider.FOO_ACTION_NAME, null);
-        assertEquals(MockConfigurationProvider.FOO_ACTION_NAME, ActionContext.getContext().getName());
-    }
-}

test/com/opensymphony/xwork/ModelDrivenAction.properties

+invalid.fieldvalue.birth=Invalid date for birth.

test/com/opensymphony/xwork/config/ConfigurationTest.java

  * Copyright (c) 2002-2003 by OpenSymphony
  * All rights reserved.
  */
-
 package com.opensymphony.xwork.config;
 
 import com.opensymphony.xwork.*;
 import com.opensymphony.xwork.config.providers.MockConfigurationProvider;
 import com.opensymphony.xwork.config.providers.XmlConfigurationProvider;
+
 import junit.framework.TestCase;
 
 import java.util.HashMap;
         try {
             ActionProxy proxy = ActionProxyFactory.getFactory().createActionProxy("/does/not/exist", "Foo", extraContext);
             proxy.execute();
-            assertEquals("this is blah", proxy.getValueStack().findValue("blah"));
+            assertEquals("this is blah", proxy.getInvocation().getStack().findValue("blah"));
         } catch (Exception e) {
             e.printStackTrace();
             fail();

test/com/opensymphony/xwork/util/XWorkConverterTest.java

  */
 package com.opensymphony.xwork.util;
 
+import com.opensymphony.xwork.ActionContext;
+import com.opensymphony.xwork.ModelDrivenAction;
 import com.opensymphony.xwork.SimpleAction;
 
 import junit.framework.TestCase;
         SimpleAction action = new SimpleAction();
         action.setDate(null);
 
-        //        context = Ognl.createDefaultContext(action);
+        OgnlValueStack stack = new OgnlValueStack();
+        stack.push(action);
+        ActionContext.getContext().setValueStack(stack);
         assertNull("Conversion should have failed.", converter.convertValue(context, action, null, "date", new String[] {
                     "invalid date"
                 }, Date.class));
+        stack.pop();
         assertTrue(action.hasFieldErrors());
         assertNotNull(action.getFieldErrors().get("date"));
+        assertEquals("Invalid field value for field \"date\".", ((List) action.getFieldErrors().get("date")).get(0));
+    }
+
+    public void testFieldErrorMessageAddedWhenConversionFailsOnModelDriven() {
+        ModelDrivenAction action = new ModelDrivenAction();
+        OgnlValueStack stack = new OgnlValueStack();
+        stack.push(action);
+        stack.push(action.getModel());
+        ActionContext.getContext().setValueStack(stack);
+        assertNull("Conversion should have failed.", converter.convertValue(context, action, null, "birth", new String[] {
+                    "invalid date"
+                }, Date.class));
+        stack.pop();
+        stack.pop();
+        assertTrue(action.hasFieldErrors());
+        assertNotNull(action.getFieldErrors().get("birth"));
+        assertEquals("Invalid date for birth.", ((List) action.getFieldErrors().get("birth")).get(0));
+    }
+
+    public void testNoFieldErrorAddedForEmptyStringParameter() {
+        SimpleAction action = new SimpleAction();
+
+        OgnlValueStack stack = new OgnlValueStack();
+        stack.push(action);
+        ActionContext.getContext().setValueStack(stack);
+        assertNull("Conversion should have failed.", converter.convertValue(context, action, null, "bar", new String[] {
+                    ""
+                }, Integer.class));
+        stack.pop();
+        assertFalse(action.hasFieldErrors());
     }
 
     public void testStringArrayToCollection() {