This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/master by this push:
new fe656ea [WW-5126] use == instead of .equals in ModelDrivenInterceptor
new cb318cd Merge pull request #486 from yasserzamani/ww_5126
fe656ea is described below
commit fe656eae21a7a287b2143fad638234314f858178
Author: Yasser Zamani <[email protected]>
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
}
