Merge pull request #358 from yasserzamani/WW-4999

fix logMissingProperties (WW-4999)
This commit is contained in:
Lukasz Lenart
2019-06-03 08:25:32 +02:00
committed by GitHub
3 changed files with 173 additions and 16 deletions
@@ -182,7 +182,7 @@ 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 || logMissingProperties ? Boolean.TRUE : Boolean.FALSE);
ognlUtil.setValue(expr, context, root, value);
}
@@ -246,7 +246,7 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS
}
protected void setupExceptionOnFailure(boolean throwExceptionOnFailure) {
if (throwExceptionOnFailure) {
if (throwExceptionOnFailure || logMissingProperties) {
context.put(THROW_EXCEPTION_ON_FAILURE, true);
}
}
@@ -339,8 +339,9 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS
}
protected boolean shouldLogMissingPropertyWarning(OgnlException e) {
return (e instanceof NoSuchPropertyException || e instanceof MethodFailedException)
&& devMode && logMissingProperties;
return (e instanceof NoSuchPropertyException ||
(e instanceof MethodFailedException && e.getReason() instanceof NoSuchMethodException))
&& logMissingProperties;
}
private Object tryFindValue(String expr, Class asType) throws OgnlException {
@@ -217,13 +217,14 @@ public class CompoundRootAccessor implements PropertyAccessor, MethodAccessor, C
return null;
}
Throwable reason = null;
Class[] argTypes = getArgTypes(objects);
for (Object o : root) {
if (o == null) {
continue;
}
Class clazz = o.getClass();
Class[] argTypes = getArgTypes(objects);
MethodCall mc = null;
@@ -233,24 +234,27 @@ 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)) {
invalidMethods.put(mc, Boolean.TRUE);
} else if (reason != null) {
throw new MethodFailedException(o, name, e.getReason());
if (reason != null && !(reason instanceof NoSuchMethodException)) {
// method has found but thrown an exception
break;
}
if ((mc != null) && (reason != null)) {
invalidMethods.put(mc, Boolean.TRUE);
}
// continue and try the next one
}
}
}
if (context.containsKey(OgnlValueStack.THROW_EXCEPTION_ON_FAILURE)) {
throw new MethodFailedException(target, name, reason);
}
return null;
}
@@ -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,95 @@ public class OgnlValueStackTest extends XWorkTestCase {
}
}
/**
* monitors the resolution of WW-4999
* @since 2.5.21
*/
public void testLogMissingProperties() {
testLogMissingProperties(true);
testLogMissingProperties(false);
}
private void testLogMissingProperties(boolean logMissingProperties) {
OgnlValueStack vs = createValueStack();
vs.setLogMissingProperties("" + logMissingProperties);
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);
if (logMissingProperties) {
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());
} else {
assertEquals(0, testAppender.logEvents.size());
}
} finally {
testAppender.stop();
logger.removeAppender(testAppender);
}
}
/**
* tests the correctness of distinguishing between user exception and NoSuchMethodException
* @since 2.5.21
*/
public void testNotLogUserExceptionsAsMissingProperties() {
OgnlValueStack vs = createValueStack();
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("exception", "exceptionValue", false);
vs.findValue("exception", false);
vs.findValue("exception", String.class, false);
vs.findValue("getException()", false);
vs.findValue("getException()", String.class, false);
vs.findValue("bite", false);
vs.findValue("bite", void.class, false);
vs.findValue("getBite()", false);
vs.findValue("getBite()", void.class, false);
vs.setLogMissingProperties("false");
vs.setValue("exception", "exceptionValue", false);
vs.findValue("exception", false);
vs.findValue("exception", String.class, false);
vs.findValue("getException()", false);
vs.findValue("getException()", String.class, false);
vs.findValue("bite", false);
vs.findValue("bite", void.class, false);
vs.findValue("getBite()", false);
vs.findValue("getBite()", void.class, false);
assertEquals(0, testAppender.logEvents.size());
} finally {
testAppender.stop();
logger.removeAppender(testAppender);
}
}
public void testFailOnMissingMethod() {
OgnlValueStack vs = createValueStack();
@@ -879,6 +975,49 @@ public class OgnlValueStackTest extends XWorkTestCase {
assertNull(vs.findValue("@com.nothing.here.Nothing@BLAH"));
}
/**
* Fails on 2.5.20 and earlier - tested on 2.5 (5/5/2016) and failed
* @since 2.5.21
*/
public void testNotThrowExceptionOnTopMissingProperty() {
OgnlValueStack vs = createValueStack();
Dog dog = new Dog();
dog.setName("Rover");
vs.push(dog);
Cat cat = new Cat();
vs.push(cat);
vs.setValue("age", 12, true);
assertEquals(12, vs.findValue("age", true));
assertEquals(12, vs.findValue("age", Integer.class, true));
assertEquals(12, vs.findValue("getAge()", true));
assertEquals(12, vs.findValue("getAge()", Integer.class, true));
}
/**
* Fails on 2.5.20 and earlier - tested on 2.5 (5/5/2016) and failed
* @since 2.5.21
*/
public void testNotSkipUserReturnedNullValues() {
OgnlValueStack vs = createValueStack();
Dog dog = new Dog();
dog.setName("Rover");
vs.push(dog);
Cat cat = new Cat();
vs.push(cat);
// should not skip returned null values from cat.name
assertNull(vs.findValue("name", true));
assertNull(vs.findValue("name", String.class, true));
assertNull(vs.findValue("getName()", true));
assertNull(vs.findValue("getName()", String.class, true));
}
public void testTop() {
OgnlValueStack vs = createValueStack();
@@ -1293,6 +1432,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 {