struts-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From yasserzam...@apache.org
Subject [struts] 01/01: [WW-5126] use == instead of .equals in ModelDrivenInterceptor
Date Sun, 25 Apr 2021 14:35:27 GMT
This is an automated email from the ASF dual-hosted git repository.

yasserzamani pushed a commit to branch ww_5126_2_5
in repository https://gitbox.apache.org/repos/asf/struts.git

commit 509663717a955669d987ebd3792a0049145a235f
Author: Yasser Zamani <yasserzamani@apache.org>
AuthorDate: Sun Apr 25 19:04:55 2021 +0430

    [WW-5126] use == instead of .equals in ModelDrivenInterceptor
    
    due to refreshing model regardless of potential overridden Object.equals
---
 .../xwork2/interceptor/ModelDrivenInterceptor.java | 12 ++--
 .../interceptor/ModelDrivenInterceptorTest.java    | 80 ++++++++++++++++++++--
 2 files changed, 81 insertions(+), 11 deletions(-)

diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/ModelDrivenInterceptor.java
b/core/src/main/java/com/opensymphony/xwork2/interceptor/ModelDrivenInterceptor.java
index a70a5e3..fa90a31 100644
--- a/core/src/main/java/com/opensymphony/xwork2/interceptor/ModelDrivenInterceptor.java
+++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/ModelDrivenInterceptor.java
@@ -105,7 +105,7 @@ public class ModelDrivenInterceptor extends AbstractInterceptor {
      * Refreshes the model instance on the value stack, if it has changed
      */
     protected static class RefreshModelBeforeResult implements PreResultListener {
-        private Object originalModel = null;
+        private Object originalModel;
         protected ModelDriven action;
 
 
@@ -122,10 +122,12 @@ public class ModelDrivenInterceptor extends AbstractInterceptor {
             Object newModel = action.getModel();
 
             // Check to see if the new model instance is already on the stack
-            for (Object item : root) {
-                if (item.equals(newModel)) {
-                    needsRefresh = false;
-                    break;
+            if (newModel != null) {
+                for (Object item : root) {
+                    if (item == newModel) {
+                        needsRefresh = false;
+                        break;
+                    }
                 }
             }
 
diff --git a/core/src/test/java/com/opensymphony/xwork2/interceptor/ModelDrivenInterceptorTest.java
b/core/src/test/java/com/opensymphony/xwork2/interceptor/ModelDrivenInterceptorTest.java
index 71a8876..4f3589f 100644
--- a/core/src/test/java/com/opensymphony/xwork2/interceptor/ModelDrivenInterceptorTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/interceptor/ModelDrivenInterceptorTest.java
@@ -37,10 +37,10 @@ public class ModelDrivenInterceptorTest extends XWorkTestCase {
     ModelDrivenInterceptor modelDrivenInterceptor;
     Object model;
     PreResultListener preResultListener;
+    ValueStack stack;
 
 
     public void testModelDrivenGetsPushedOntoStack() throws Exception {
-        ValueStack stack = ActionContext.getContext().getValueStack();
         action = new ModelDrivenAction();
         mockActionInvocation.expectAndReturn("getAction", action);
         mockActionInvocation.expectAndReturn("getStack", stack);
@@ -52,8 +52,7 @@ public class ModelDrivenInterceptorTest extends XWorkTestCase {
         assertEquals("our model should be on the top of the stack", model, topOfStack);
     }
 
-    public void testModelDrivenUpdatedAndGetsPushedOntoStack() throws Exception {
-        ValueStack stack = ActionContext.getContext().getValueStack();
+    private void setupRefreshModelBeforeResult() {
         action = new ModelDrivenAction();
         mockActionInvocation.expectAndReturn("getAction", action);
         mockActionInvocation.matchAndReturn("getStack", stack);
@@ -70,18 +69,86 @@ public class ModelDrivenInterceptorTest extends XWorkTestCase {
             }
         });
         modelDrivenInterceptor.setRefreshModelBeforeResult(true);
+    }
+
+    public void testModelDrivenUpdatedAndGetsPushedOntoStack() throws Exception {
+        setupRefreshModelBeforeResult();
 
         modelDrivenInterceptor.intercept((ActionInvocation) mockActionInvocation.proxy());
         assertNotNull(preResultListener);
-        model = "this is my model";
+        model = "this is my new model";
         preResultListener.beforeResult((ActionInvocation) mockActionInvocation.proxy(), "success");
 
         Object topOfStack = stack.pop();
-        assertEquals("our model should be on the top of the stack", model, topOfStack);
+        assertSame("our new model should be on the top of the stack", model, topOfStack);
+        assertEquals(1, stack.getRoot().size());
+    }
+
+    public void testWW5126() throws Exception {
+        model = new Object() {
+            @Override
+            public boolean equals(Object obj) {
+                return true;
+            }
+        };
+        setupRefreshModelBeforeResult();
+
+        modelDrivenInterceptor.intercept((ActionInvocation) mockActionInvocation.proxy());
+        assertNotNull(preResultListener);
+        model = "this is my new model";
+        preResultListener.beforeResult((ActionInvocation) mockActionInvocation.proxy(), "success");
+
+        Object topOfStack = stack.pop();
+        assertSame("our new model should be on the top of the stack regardless of Object.equals",
model, topOfStack);
         assertEquals(1, stack.getRoot().size());
     }
 
-    public void testStackNotModifedForNormalAction() throws Exception {
+    public void testPrimitiveModelDrivenUpdatedAndGetsPushedOntoStack() throws Exception
{
+        model = 123;
+        setupRefreshModelBeforeResult();
+
+        modelDrivenInterceptor.intercept((ActionInvocation) mockActionInvocation.proxy());
+        assertNotNull(preResultListener);
+        model = new Integer("123");
+        preResultListener.beforeResult((ActionInvocation) mockActionInvocation.proxy(), "success");
+
+        Object topOfStack = stack.pop();
+        assertSame("our new primitive model should be on the top of the stack", model, topOfStack);
+        assertEquals(1, stack.getRoot().size());
+    }
+
+    public void testNotNeedsRefresh() throws Exception {
+        model = new Object() {
+            @Override
+            public boolean equals(Object obj) {
+                return false;
+            }
+        };
+        setupRefreshModelBeforeResult();
+
+        modelDrivenInterceptor.intercept((ActionInvocation) mockActionInvocation.proxy());
+        assertNotNull(preResultListener);
+        preResultListener.beforeResult((ActionInvocation) mockActionInvocation.proxy(), "success");
+
+        Object topOfStack = stack.pop();
+        assertSame("our original model should be on the top of the stack", model, topOfStack);
+        assertEquals(1, stack.getRoot().size());
+    }
+
+    public void testNullNewModel() throws Exception {
+        setupRefreshModelBeforeResult();
+
+        modelDrivenInterceptor.intercept((ActionInvocation) mockActionInvocation.proxy());
+        assertNotNull(preResultListener);
+        model = null;
+        preResultListener.beforeResult((ActionInvocation) mockActionInvocation.proxy(), "success");
+
+        Object topOfStack = stack.pop();
+        assertNotSame("our model should be removed from the stack", model, topOfStack);
+        assertEquals(0, stack.getRoot().size());
+    }
+
+    public void testStackNotModifiedForNormalAction() throws Exception {
         action = new ActionSupport();
         mockActionInvocation.expectAndReturn("getAction", action);
         mockActionInvocation.expectAndReturn("invoke", "foo");
@@ -95,6 +162,7 @@ public class ModelDrivenInterceptorTest extends XWorkTestCase {
         super.setUp();
         mockActionInvocation = new Mock(ActionInvocation.class);
         modelDrivenInterceptor = new ModelDrivenInterceptor();
+        stack = ActionContext.getContext().getValueStack();
         model = new Date(); // any object will do
     }
 

Mime
View raw message