Anonymous avatar Anonymous committed f10b8ec

WW-3306 Null value accepted from action property, but not model property when throwExceptionOnFailure=true

git-svn-id: http://svn.opensymphony.com/svn/xwork/trunk@2075 e221344d-f017-0410-9bd5-d282ab1896d7

Comments (0)

Files changed (5)

                                     <exclude>junit:junit</exclude>
                                     <exclude>commons-logging:commons-logging</exclude>
                                     <exclude>opensymphony:ognl</exclude>
+                                    <exclude>ognl:ognl</exclude>
                                     <exclude>org.springframework:spring-core</exclude>
                                     <exclude>org.springframework:spring-aop</exclude>
                                     <exclude>org.springframework:spring-aspects</exclude>

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

 import com.opensymphony.xwork2.util.logging.LoggerFactory;
 import com.opensymphony.xwork2.util.logging.LoggerUtils;
 import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
-import ognl.Ognl;
-import ognl.OgnlContext;
-import ognl.OgnlException;
-import ognl.PropertyAccessor;
+import ognl.*;
 
 import java.beans.IntrospectionException;
 import java.beans.PropertyDescriptor;
     private static Logger LOG = LoggerFactory.getLogger(OgnlValueStack.class);
     private boolean devMode;
     private boolean logMissingProperties;
+    public static final String THROW_EXCEPTION_ON_FAILURE = OgnlValueStack.class.getName() + ".throwExceptionOnFailure";
 
     public static void link(Map<String, Object> context, Class clazz, String name) {
         context.put("__link", new Object[]{clazz, name});
                 return findValue(expr, defaultType);
             }
 
-            Object value = ognlUtil.getValue(expr, context, root);
+            Object value = null;
+            try {
+                if (throwExceptionOnFailure)
+                    context.put(THROW_EXCEPTION_ON_FAILURE, true);
+                value = ognlUtil.getValue(expr, context, root);
+            } finally {
+                context.remove(THROW_EXCEPTION_ON_FAILURE);
+            }
+
             if (value != null) {
                 return value;
             } else {
-                checkForInvalidProperties(expr, throwExceptionOnFailure, throwExceptionOnFailure);
                 return findInContext(expr);
             }
         } catch (OgnlException e) {
-            checkForInvalidProperties(expr, throwExceptionOnFailure, throwExceptionOnFailure);
-
-            return findInContext(expr);
+            Object ret = findInContext(expr);
+
+            if (ret != null)
+                return ret;
+            else {
+                if (e instanceof NoSuchPropertyException && devMode && logMissingProperties)
+                    LOG.warn("Could not find property [" + ((NoSuchPropertyException)e).getName() + "]");
+
+                if (throwExceptionOnFailure)
+                    throw new XWorkException(e);
+                else
+                    return null;
+            }
         } catch (Exception e) {
             logLookupFailure(expr, e);
 
                 expr = (String) overrides.get(expr);
             }
 
-            Object value = ognlUtil.getValue(expr, context, root, asType);
+            Object value = null;
+            try {
+                if (throwExceptionOnFailure)
+                    context.put(THROW_EXCEPTION_ON_FAILURE, true);
+                value = ognlUtil.getValue(expr, context, root, asType);
+            } finally {
+                context.remove(THROW_EXCEPTION_ON_FAILURE);
+            }
+
             if (value != null) {
                 return value;
             } else {
-                checkForInvalidProperties(expr, throwExceptionOnFailure, throwExceptionOnFailure);
                 return findInContext(expr);
             }
         } catch (OgnlException e) {
-            checkForInvalidProperties(expr, throwExceptionOnFailure, throwExceptionOnFailure);
-            return findInContext(expr);
+            Object ret = findInContext(expr);
+
+            if (ret != null)
+                return ret;
+            else {
+                if (e instanceof NoSuchPropertyException && devMode && logMissingProperties)
+                    LOG.warn("Could not find property [" + ((NoSuchPropertyException)e).getName() + "]");
+                
+                if (throwExceptionOnFailure)
+                    throw new XWorkException(e);
+                else
+                    return null;
+            }
         } catch (Exception e) {
             logLookupFailure(expr, e);
 
     }
 
     /**
-     * This method looks for matching methods/properties in an action to warn the user if
-     * they specified a property that doesn't exist.
-     *
-     * @param expr the property expression
-     */
-    private void checkForInvalidProperties(String expr, boolean throwExceptionPropNotFound, boolean throwExceptionMethodFound) {
-        if (expr.contains("(") && expr.contains(")")) {
-            if (devMode)
-                LOG.warn("Could not find method [" + expr + "]");
-
-            if (throwExceptionMethodFound)
-                    throw new XWorkException("Could not find method [" + expr + "]");
-        } else if (findInContext(expr) == null) {
-            // find objects with Action in them and inspect matching getters
-            Set<String> availableProperties = new LinkedHashSet<String>();
-            for (Object o : root) {
-                try {
-                    findAvailableProperties(o.getClass(), expr, availableProperties, null);
-                } catch (IntrospectionException ise) {
-                    // ignore
-                }
-            }
-            if (!availableProperties.contains(expr)) {
-                if (devMode && logMissingProperties) {
-                    LOG.warn("Could not find property [" + expr + "]");
-                }
-
-                if (throwExceptionMethodFound)
-                    throw new XWorkException("Could not find property [" + expr + "]");
-            }
-        }
-    }
-
-    /**
      * Look for available properties on an existing class.
      *
      * @param c                   the class to search on

core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java

 package com.opensymphony.xwork2.ognl.accessor;
 
 import com.opensymphony.xwork2.XWorkException;
+import com.opensymphony.xwork2.ognl.OgnlValueStack;
 import com.opensymphony.xwork2.inject.Inject;
 import com.opensymphony.xwork2.util.CompoundRoot;
 import com.opensymphony.xwork2.util.ValueStack;
                 }
             }
 
-            return null;
+            //property was not found
+            if (context.containsKey(OgnlValueStack.THROW_EXCEPTION_ON_FAILURE))
+                throw new NoSuchPropertyException(target, name);
+            else
+                return null;
         } else {
             return null;
         }
                     // try the next one
                     Throwable reason = e.getReason();
 
-                    if ((mc != null) && (reason != null) && (reason.getClass() == NoSuchMethodException.class)) {
+                    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());

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

         // just do some of the 15 tests
         Map beans = ognlUtil.getBeanMap(foo);
         assertNotNull(beans);
-        assertEquals(18, beans.size());
+        assertEquals(19, beans.size());
         assertEquals("Hello Santa", beans.get("title"));
         assertEquals(new Long("123"), beans.get("ALong"));
         assertEquals(new Integer("44"), beans.get("number"));

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

         vs.findValue("barJunior.title", true);
     }
 
-     public void testFailOnErrorOnInheritedPropertiesWithMethods() {
+     public void testSuccessFailOnErrorOnInheritedPropertiesWithMethods() {
         //this shuld not fail as the property is defined on a parent class
-        /*OgnlValueStack vs = createValueStack();
+        OgnlValueStack vs = createValueStack();
+
+        Foo foo = new Foo();
+        BarJunior barjr = new BarJunior();
+        foo.setBarJunior(barjr);
+        vs.push(foo);
+
+        assertNull(barjr.getTitle());
+        vs.findValue("getBarJunior().title", true);
+    }
+
+    public void testFailFailOnErrorOnInheritedPropertiesWithMethods() {
+        OgnlValueStack vs = createValueStack();
 
         Foo foo = new Foo();
         BarJunior barjr = new BarJunior();
         vs.push(foo);
 
         assertNull(barjr.getTitle());
-        vs.findValue("getBarJunior().title", true);*/
+        try {
+            vs.findValue("getBarJunior().title2", true);
+            fail("should have failed on missing property");
+        } catch (Exception e) {
+        }
     }
 
     public void testFailOnMissingProperty() {
         }
     }
 
+    public void testFailOnMissingMethod() {
+        OgnlValueStack vs = createValueStack();
+
+        Dog dog = new Dog();
+        vs.push(dog);
+        try {
+            vs.findValue("someprop()", true);
+            fail("Failed to throw exception on EL missing method");
+        } catch (Exception ex) {
+            //ok
+        }
+    }
+
+    public void testFailsOnMethodThatThrowsException() {
+        SimpleAction action = new SimpleAction();
+        OgnlValueStack stack = createValueStack();
+        stack.push(action);
+
+        action.setThrowException(true);
+        try {
+            stack.findValue("exceptionMethod1()", true);
+            fail("Failed to throw exception on EL method exception");
+        } catch (Exception ex) {
+            //ok
+        }
+    }
+
+
     public void testDoesNotFailOnNonActionObjects() {
         //if a value is not found, then it will check for missing properties
         //it needs to check in all objects in the stack, not only actions, see WW-3306
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.