Merge pull request #192 from yasserzamani/WW-4873_2

WW-4873 Makes ActionInvocation not serializable and InvocationSessionStore$InvocationContext transient

core/src/main/java/com/opensymphony/xwork2/ActionInvocation.java

@@ -21,8 +21,6 @@
 import com.opensymphony.xwork2.interceptor.PreResultListener;
 import com.opensymphony.xwork2.util.ValueStack;
 
-import java.io.Serializable;
-
 /**
  * An {@link ActionInvocation} represents the execution state of an {@link Action}. It holds the Interceptors and the Action instance.
  * By repeated re-entrant execution of the <code>invoke()</code> method, initially by the {@link ActionProxy}, then by the Interceptors, the
@@ -31,7 +29,7 @@
  * @author Jason Carreira
  * @see com.opensymphony.xwork2.ActionProxy
  */
-public interface ActionInvocation extends Serializable {
+public interface ActionInvocation {
 
     /**
      * Get the Action associated with this ActionInvocation.
@@ -178,20 +176,4 @@ public interface ActionInvocation extends Serializable {
 
     void init(ActionProxy proxy) ;
 
-    /**
-     * Prepares instance of ActionInvocation to be serializable,
-     * which simple means removing all unserializable fields, eg. Container
-     *
-     * @return ActionInvocation which can be serialize (eg. into HttpSession)
-     */
-    ActionInvocation serialize();
-
-    /**
-     * Performs opposite process to restore back ActionInvocation after deserialisation
-     *
-     * @param actionContext current {@link ActionContext}
-     * @return fully operational ActionInvocation
-     */
-    ActionInvocation deserialize(ActionContext actionContext);
-
 }

core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java

@@ -500,27 +500,4 @@ protected String saveResult(ActionConfig actionConfig, Object methodResult) {
         }
     }
 
-    /**
-     * Version ready to be serialize
-     *
-     * @return instance without reference to {@link Container}
-     */
-    public ActionInvocation serialize() {
-        DefaultActionInvocation that = this;
-        that.container = null;
-        return that;
-    }
-
-    /**
-     * Restoring Container
-     *
-     * @param actionContext current {@link ActionContext}
-     * @return instance which can be used to invoke action
-     */
-    public ActionInvocation deserialize(ActionContext actionContext) {
-        DefaultActionInvocation that = this;
-        that.container = actionContext.getContainer();
-        return that;
-    }
-
 }

core/src/main/java/com/opensymphony/xwork2/mock/MockActionInvocation.java

@@ -124,12 +124,4 @@ public ActionEventListener getActionEventListener() {
     public void init(ActionProxy proxy) {
     }
 
-    public ActionInvocation serialize() {
-        return this;
-    }
-
-    public ActionInvocation deserialize(ActionContext actionContext) {
-        return this;
-    }
-
 }

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

@@ -20,7 +20,6 @@
 
 import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.ActionInvocation;
-import com.opensymphony.xwork2.util.ValueStack;
 
 import java.io.Serializable;
 import java.util.HashMap;
@@ -56,10 +55,14 @@ public static ActionInvocation loadInvocation(String key, String token) {
             return null;
         }
 
-        ValueStack stack = invocationContext.invocation.getStack();
-        ActionContext.getContext().setValueStack(stack);
+        ActionInvocation savedInvocation = null;
+        if (invocationContext.invocation != null) {
+            savedInvocation = invocationContext.invocation;
+            ActionContext.setContext(savedInvocation.getInvocationContext());
+            ActionContext.getContext().setValueStack(savedInvocation.getStack());
+        }
 
-        return invocationContext.invocation.deserialize(ActionContext.getContext());
+        return savedInvocation;
     }
 
     /**
@@ -71,7 +74,7 @@ public static ActionInvocation loadInvocation(String key, String token) {
      * @param invocation the action invocation
      */
     public static void storeInvocation(String key, String token, ActionInvocation invocation) {
-        InvocationContext invocationContext = new InvocationContext(invocation.serialize(), token);
+        InvocationContext invocationContext = new InvocationContext(invocation, token);
         Map invocationMap = getInvocationMap();
         invocationMap.put(key, invocationContext);
         setInvocationMap(invocationMap);
@@ -109,7 +112,9 @@ private static class InvocationContext implements Serializable {
 
         private static final long serialVersionUID = -286697666275777888L;
 
-        ActionInvocation invocation;
+        //WW-4873 transient since 2.5.15
+        transient ActionInvocation invocation;
+
         String token;
 
         public InvocationContext(ActionInvocation invocation, String token) {

core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java

@@ -75,34 +75,6 @@ public void testInvoke() throws Exception {
         assertTrue(mockInterceptor3.isExecuted());
     }
 
-    public void testSerialization() throws Exception {
-        // given
-        DefaultActionInvocation actionInvocation = new DefaultActionInvocation(new HashMap<String, Object>(), false);
-        actionInvocation.setContainer(new MockContainer());
-
-        // when
-        DefaultActionInvocation serializable = (DefaultActionInvocation) actionInvocation.serialize();
-
-        // then
-        assertNull(actionInvocation.container);
-        assertNull(serializable.container);
-    }
-
-    public void testDeserialization() throws Exception {
-        // given
-        DefaultActionInvocation actionInvocation = new DefaultActionInvocation(new HashMap<String, Object>(), false);
-        MockContainer mockContainer = new MockContainer();
-        ActionContext.getContext().setContainer(mockContainer);
-
-        // when
-        DefaultActionInvocation deserializable = (DefaultActionInvocation) actionInvocation.deserialize(ActionContext.getContext());
-
-        // then
-        assertNotNull(actionInvocation.container);
-        assertNotNull(deserializable.container);
-        assertEquals(mockContainer, deserializable.container);
-    }
-
     public void testInvokingExistingExecuteMethod() throws Exception {
         // given
         DefaultActionInvocation dai = new DefaultActionInvocation(new HashMap<String, Object>(), false) {

core/src/test/java/com/opensymphony/xwork2/spring/interceptor/TestActionInvocation.java

@@ -86,12 +86,4 @@ public void setActionEventListener(ActionEventListener listener) {
     public void init(ActionProxy proxy)  {
     }
 
-    public ActionInvocation serialize() {
-        return this;
-    }
-
-    public ActionInvocation deserialize(ActionContext actionContext) {
-        return this;
-    }
-
 }

core/src/test/java/org/apache/struts2/util/InvocationSessionStoreTest.java

@@ -25,6 +25,10 @@
 import com.opensymphony.xwork2.util.ValueStack;
 import org.apache.struts2.StrutsInternalTestCase;
 
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -40,7 +44,7 @@ public class InvocationSessionStoreTest extends StrutsInternalTestCase {
 
 
     private ActionInvocation invocation;
-    private Map session;
+    private Map<String, Object> session;
     private Mock invocationMock;
     private ValueStack stack;
 
@@ -62,20 +66,53 @@ public void testValueStackReset() {
         assertEquals(stack, actionContext.getValueStack());
     }
 
+    public void testActionContextReset() {
+        ActionContext actionContext = ActionContext.getContext();
+        InvocationSessionStore.storeInvocation(INVOCATION_KEY, TOKEN_VALUE, invocation);
+
+        ActionContext actionContext2 = new ActionContext(new HashMap<String, Object>());
+        actionContext2.setSession(session);
+        ActionContext.setContext(actionContext2);
+        assertEquals(actionContext2, ActionContext.getContext());
+
+        InvocationSessionStore.loadInvocation(INVOCATION_KEY, TOKEN_VALUE);
+        assertEquals(actionContext, ActionContext.getContext());
+    }
+
+    public void testStoreAndLoadFromDeserializedSession() throws Exception {
+        InvocationSessionStore.storeInvocation(INVOCATION_KEY, TOKEN_VALUE, invocation);
+
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        ObjectOutputStream oos = new ObjectOutputStream(baos);
+        oos.writeObject(session);//WW-4873 invocation is not serializable but we should not fail at this line
+        oos.close();
+        byte b[] = baos.toByteArray();
+        baos.close();
+
+        ByteArrayInputStream bais = new ByteArrayInputStream(b);
+        ObjectInputStream ois = new ObjectInputStream(bais);
+        session = (Map) ois.readObject();
+        ActionContext.getContext().setSession(session);
+        ois.close();
+        bais.close();
+
+        ActionInvocation savedInvocation = InvocationSessionStore.loadInvocation(INVOCATION_KEY, TOKEN_VALUE);
+        assertNull(savedInvocation);//Currently we don't support invocation restore from serialized session
+    }
+
     protected void setUp() throws Exception {
         super.setUp();
         stack = ActionContext.getContext().getValueStack();
 
         ActionContext actionContext = new ActionContext(stack.getContext());
         ActionContext.setContext(actionContext);
 
-        session = new HashMap();
+        session = new HashMap<>();
         actionContext.setSession(session);
 
         invocationMock = new Mock(ActionInvocation.class);
         invocation = (ActionInvocation) invocationMock.proxy();
-        invocationMock.matchAndReturn("serialize", invocation);
-        invocationMock.matchAndReturn("deserialize", actionContext, invocation);
+        invocationMock.matchAndReturn("getInvocationContext", actionContext);
 
         actionContext.setValueStack(stack);
         invocationMock.matchAndReturn("getStack", stack);

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

@@ -110,7 +110,6 @@ public ValidationAwareSupport doPost(String namespace, String actionName, Map pa
     }
 
     public static class ValidatorActionInvocation extends DefaultActionInvocation {
-        private static final long serialVersionUID = -7645433725470191275L;
 
         protected ValidatorActionInvocation(Map<String, Object> extraContext, boolean pushAction) throws Exception {
             super(extraContext, pushAction);

plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java

@@ -556,8 +556,6 @@ protected void setUp() throws Exception {
 
 class MockActionInvocationEx extends MockActionInvocation {
 
-    private static final long serialVersionUID = 3057703805130170757L;
-
     private boolean invoked;
 
     @Override

plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java

@@ -46,8 +46,6 @@
  */
 public class RestActionInvocation extends DefaultActionInvocation {
 
-    private static final long serialVersionUID = 3485701178946428716L;
-
     private static final Logger LOG = LogManager.getLogger(RestActionInvocation.class);
 
     private ContentTypeHandlerManager handlerSelector;

plugins/sitemesh/src/main/java/org/apache/struts2/sitemesh/OldDecorator2NewStrutsDecorator.java

@@ -140,8 +140,6 @@ protected String getEncoding() {
 
     static class DummyActionInvocation implements ActionInvocation {
 
-        private static final long serialVersionUID = -4808072199157363028L;
-
         ActionSupport action;
 
         public DummyActionInvocation(ActionSupport action) {
@@ -196,14 +194,6 @@ public void setActionEventListener(ActionEventListener listener) {
         public void init(ActionProxy proxy) {
         }
 
-        public ActionInvocation serialize() {
-            return null;
-        }
-
-        public ActionInvocation deserialize(ActionContext actionContext) {
-            return null;
-        }
-
     }
 
 }