You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by ya...@apache.org on 2021/04/25 14:35:27 UTC
[struts] 01/01: [WW-5126] use == instead of .equals in
ModelDrivenInterceptor
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 <ya...@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
}