From commits-return-19709-archive-asf-public=cust-asf.ponee.io@struts.apache.org Tue May 4 19:34:28 2021 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mxout1-ec2-va.apache.org (mxout1-ec2-va.apache.org [3.227.148.255]) by mx-eu-01.ponee.io (Postfix) with ESMTPS id B053218065E for ; Tue, 4 May 2021 21:34:28 +0200 (CEST) Received: from mail.apache.org (mailroute1-lw-us.apache.org [207.244.88.153]) by mxout1-ec2-va.apache.org (ASF Mail Server at mxout1-ec2-va.apache.org) with SMTP id 3F99F3FC32 for ; Tue, 4 May 2021 19:34:25 +0000 (UTC) Received: (qmail 44703 invoked by uid 500); 4 May 2021 19:34:24 -0000 Mailing-List: contact commits-help@struts.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@struts.apache.org Delivered-To: mailing list commits@struts.apache.org Received: (qmail 44548 invoked by uid 99); 4 May 2021 19:34:24 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 04 May 2021 19:34:24 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 492F6818A9; Tue, 4 May 2021 19:34:24 +0000 (UTC) Date: Tue, 04 May 2021 19:34:24 +0000 To: "commits@struts.apache.org" Subject: [struts] branch master updated: [WW-5126] use == instead of .equals in ModelDrivenInterceptor MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <162015686405.29721.10675506281971487046@gitbox.apache.org> From: lukaszlenart@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: struts X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 05528157f0725707a512aa4dc2b9054fb4a4467c X-Git-Newrev: fe656eae21a7a287b2143fad638234314f858178 X-Git-Rev: fe656eae21a7a287b2143fad638234314f858178 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated 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 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 }