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.

core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java

@@ -182,7 +182,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);
     }
 
@@ -246,7 +247,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);
         }
     }

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;
     }
 

core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java

@@ -34,9 +34,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;
 
 
@@ -238,6 +245,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();
 
@@ -1293,6 +1331,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 {