[prev in list] [next in list] [prev in thread] [next in thread] 

List:       struts-dev
Subject:    [struts] 01/05: fix logMissingProperties (WW-4999)
From:       yasserzamani () apache ! org
Date:       2019-06-03 7:48:15
Message-ID: 20190603074814.892408A886 () gitbox ! apache ! org
[Download RAW message or body]

This is an automated email from the ASF dual-hosted git repository.

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

commit b657a272d1d2ae93dc5fd0a1b7f81d6ae1772e89
Author: Yasser Zamani <yasserzamani@apache.org>
AuthorDate: Thu May 30 16:36:54 2019 +0430

    fix logMissingProperties (WW-4999)
    
    Moves checking OgnlValueStack.THROW_EXCEPTION_ON_FAILURE outside loop because it \
shouldn't throw exception on first failure while is trying all root objects.  
    Returns on first successful call because it's not rational and is confusing user \
to skip when user method successfully returns null as an actual result.  
    Fixes WW-4999 via honoring (devMode && logMissingProperties) for \
OgnlValueStack.THROW_EXCEPTION_ON_FAILURE and REPORT_ERRORS_ON_NO_PROP.  
    (cherry picked from commit 3ac6835)
---
 .../opensymphony/xwork2/ognl/OgnlValueStack.java   |  5 ++-
 .../xwork2/ognl/accessor/CompoundRootAccessor.java | 17 ++++----
 .../xwork2/ognl/OgnlValueStackTest.java            | 51 ++++++++++++++++++++++
 3 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java \
b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java index \
                6a0ef66..94ec450 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java
@@ -181,7 +181,8 @@ public class OgnlValueStack implements Serializable, ValueStack, \
ClearableValueS  
     private void trySetValue(String expr, Object value, boolean \
throwExceptionOnFailure, Map<String, Object> context) throws OgnlException {  \
                context.put(XWorkConverter.CONVERSION_PROPERTY_FULLNAME, expr);
-        context.put(REPORT_ERRORS_ON_NO_PROP, (throwExceptionOnFailure) ? \
Boolean.TRUE : Boolean.FALSE); +        context.put(REPORT_ERRORS_ON_NO_PROP, \
throwExceptionOnFailure || (devMode && logMissingProperties) +                ? \
Boolean.TRUE : Boolean.FALSE);  ognlUtil.setValue(expr, context, root, value);
     }
 
@@ -245,7 +246,7 @@ public class OgnlValueStack implements Serializable, ValueStack, \
ClearableValueS  }
 
     protected void setupExceptionOnFailure(boolean throwExceptionOnFailure) {
-        if (throwExceptionOnFailure) {
+        if (throwExceptionOnFailure || (devMode && logMissingProperties)) {
             context.put(THROW_EXCEPTION_ON_FAILURE, true);
         }
     }
diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java \
b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java \
                index 0403f61..2a4f5d2 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java
                
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java
 @@ -217,6 +217,7 @@ public class CompoundRootAccessor implements PropertyAccessor, \
MethodAccessor, C  return null;
         }
 
+        Throwable reason = null;
         for (Object o : root) {
             if (o == null) {
                 continue;
@@ -233,24 +234,22 @@ public class CompoundRootAccessor implements PropertyAccessor, \
MethodAccessor, C  
             if ((argTypes == null) || !invalidMethods.containsKey(mc)) {
                 try {
-                    Object value = OgnlRuntime.callMethod((OgnlContext) context, o, \
                name, objects);
-
-                    if (value != null) {
-                        return value;
-                    }
+                    return OgnlRuntime.callMethod((OgnlContext) context, o, name, \
objects);  } catch (OgnlException e) {
                     // try the next one
-                    Throwable reason = e.getReason();
+                    reason = e.getReason();
 
-                    if \
(!context.containsKey(OgnlValueStack.THROW_EXCEPTION_ON_FAILURE) && (mc != null) && \
(reason != null) && (reason.getClass() == NoSuchMethodException.class)) { +           \
if ((mc != null) && (reason != null) && (reason.getClass() == \
NoSuchMethodException.class)) {  invalidMethods.put(mc, Boolean.TRUE);
-                    } else if (reason != null) {
-                        throw new MethodFailedException(o, name, e.getReason());
                     }
                 }
             }
         }
 
+        if (context.containsKey(OgnlValueStack.THROW_EXCEPTION_ON_FAILURE)) {
+            throw new MethodFailedException(target, name, reason);
+        }
+
         return null;
     }
 
diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java \
b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java index \
                b10cba7..617f876 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
@@ -33,9 +33,16 @@ import ognl.PropertyAccessor;
 
 import java.io.*;
 import java.math.BigDecimal;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
 import org.apache.struts2.StrutsConstants;
 
 
@@ -239,6 +246,37 @@ public class OgnlValueStackTest extends XWorkTestCase {
         }
     }
 
+    public void testLogMissingProperties() {
+        OgnlValueStack vs = createValueStack();
+        vs.setDevMode("true");
+        vs.setLogMissingProperties("true");
+
+        Dog dog = new Dog();
+        vs.push(dog);
+
+        TestAppender testAppender = new TestAppender();
+        Logger logger = (Logger) LogManager.getLogger(OgnlValueStack.class);
+        logger.addAppender(testAppender);
+        testAppender.start();
+
+        try {
+            vs.setValue("missingProp1", "missingProp1Value", false);
+            vs.findValue("missingProp2", false);
+            vs.findValue("missingProp3", Integer.class, false);
+
+            assertEquals(3, testAppender.logEvents.size());
+            assertEquals("Error setting value [missingProp1Value] with expression \
[missingProp1]", +                    \
testAppender.logEvents.get(0).getMessage().getFormattedMessage()); +            \
assertEquals("Could not find property [missingProp2]!", +                    \
testAppender.logEvents.get(1).getMessage().getFormattedMessage()); +            \
assertEquals("Could not find property [missingProp3]!", +                    \
testAppender.logEvents.get(2).getMessage().getFormattedMessage()); +        } finally \
{ +            testAppender.stop();
+            logger.removeAppender(testAppender);
+        }
+    }
+
     public void testFailOnMissingMethod() {
         OgnlValueStack vs = createValueStack();
 
@@ -1374,6 +1412,19 @@ public class OgnlValueStackTest extends XWorkTestCase {
             this.displayName = displayName;
         }
     }
+
+    class TestAppender extends AbstractAppender {
+        List<LogEvent> logEvents = new ArrayList<>();
+
+        TestAppender() {
+            super("TestAppender", null, null, false, null);
+        }
+
+        @Override
+        public void append(LogEvent logEvent) {
+            logEvents.add(logEvent);
+        }
+    }
 }
 
 enum MyNumbers {


[prev in list] [next in list] [prev in thread] [next in thread]