Index: src/test/com/opensymphony/xwork2/util/OgnlValueStackTest.java =================================================================== --- src/test/com/opensymphony/xwork2/util/OgnlValueStackTest.java (revision 1615) +++ src/test/com/opensymphony/xwork2/util/OgnlValueStackTest.java (working copy) @@ -722,6 +722,26 @@ assertEquals(null, stack.findValue("unknown", String.class)); } + public void testWarnAboutInvalidProperties() { + OgnlValueStack stack = new OgnlValueStack(); + MyAction action = new MyAction(); + action.setName("Don"); + stack.push(action); + + // how to test the warning was logged? + assertEquals("Don", stack.findValue("name", String.class)); + assertEquals(null, stack.findValue("address", String.class)); + // should log warning + assertEquals(null, stack.findValue("address.invalidProperty", String.class)); + + // if country is null, OGNL throws an exception + /*action.setAddress(new Address()); + stack.push(action);*/ + // should log warning + assertEquals(null, stack.findValue("address.country.id", String.class)); + assertEquals(null, stack.findValue("address.country.name", String.class)); + } + class BadJavaBean { private int count; private int count2; @@ -742,4 +762,94 @@ return count2; } } + + class MyAction { + private Long id; + private String name; + private Address address; + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public Address getAddress() { + return address; + } + + public void setAddress(Address address) { + this.address = address; + } + } + + class Address { + private String address; + private Country country; + private String city; + + public String getAddress() { + return address; + } + + public void setAddress(String address) { + this.address = address; + } + + public String getCity() { + return city; + } + + public void setCity(String city) { + this.city = city; + } + + public Country getCountry() { + return country; + } + + public void setCountry(Country country) { + this.country = country; + } + } + + class Country { + private String iso; + private String name; + private String displayName; + + public String getIso() { + return iso; + } + + public void setIso(String iso) { + this.iso = iso; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getDisplayName() { + return displayName; + } + + public void setDisplayName(String displayName) { + this.displayName = displayName; + } + } } Index: src/java/com/opensymphony/xwork2/util/OgnlValueStack.java =================================================================== --- src/java/com/opensymphony/xwork2/util/OgnlValueStack.java (revision 1615) +++ src/java/com/opensymphony/xwork2/util/OgnlValueStack.java (working copy) @@ -4,14 +4,18 @@ */ package com.opensymphony.xwork2.util; +import com.opensymphony.xwork2.ActionSupport; import com.opensymphony.xwork2.DefaultTextProvider; import com.opensymphony.xwork2.XWorkException; import com.opensymphony.xwork2.inject.Inject; - +import ognl.*; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import ognl.*; + +import java.beans.PropertyDescriptor; +import java.beans.IntrospectionException; import java.io.Serializable; +import java.lang.reflect.Method; import java.util.*; /** @@ -239,9 +243,13 @@ if (value != null) { return value; } else { + // Raible: check for invalid properties on actions + checkForInvalidProperties(expr); return findInContext(expr); } } catch (OgnlException e) { + // Raible: check for invalid properties on actions + checkForInvalidProperties(expr); return findInContext(expr); } catch (Exception e) { logLookupFailure(expr, e); @@ -257,6 +265,69 @@ } /** + * 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) { + if (expr.contains("(") && expr.contains(")")) { + LOG.warn("Could not find method [" + expr + "]"); + } else if (findInContext(expr) == null) { + // find objects with Action in them and inspect matching getters + Set availableProperties = new LinkedHashSet(); + for (Object o : root) { + if (o instanceof ActionSupport || o.getClass().getSimpleName().endsWith("Action")) { + try { + findAvailableProperties(o.getClass(), expr, availableProperties, null); + } catch (IntrospectionException ise) { + // ignore + } + } + } + if (!availableProperties.contains(expr)) { + LOG.warn("Could not find property [" + expr + "]"); + } + } + } + + /** + * Look for available properties on an existing class. + * @param c the class to search on + * @param expr the property expression + * @param availableProperties a set of properties found + * @param parent a parent property + * @throws IntrospectionException when Ognl can't get property descriptors + */ + private void findAvailableProperties(Class c, String expr, Set availableProperties, String parent) throws IntrospectionException { + PropertyDescriptor[] descriptors = OgnlUtil.getPropertyDescriptors(c); + for (PropertyDescriptor pd : descriptors) { + String name = pd.getDisplayName(); + if (parent != null && expr.indexOf(".") > -1) { + name = expr.substring(0, expr.indexOf(".") + 1) + name; + } + if (expr.startsWith(name)) { + availableProperties.add((parent != null) ? parent + "." + name : name); + if (expr.equals(name)) break; // no need to go any further + if (expr.indexOf(".") > -1) { + String property = expr.substring(expr.indexOf(".") + 1); + // if there is a nested property (indicated by a dot), chop it off so we can look for method name + String rawProperty = (property.indexOf(".") > -1) ? property.substring(0, property.indexOf(".")) : property; + String methodToLookFor = "get" + rawProperty.substring(0, 1).toUpperCase() + rawProperty.substring(1); + Method[] methods = pd.getPropertyType().getDeclaredMethods(); + for (Method method : methods) { + if (method.getName().equals(methodToLookFor)) { + availableProperties.add(name + "." + rawProperty); + Class returnType = method.getReturnType(); + findAvailableProperties(returnType, property, availableProperties, name); + } + } + + } + } + } + } + + /** * Log a failed lookup, being more verbose when devMode=true. * * @param expr The failed expression Index: src/java/com/opensymphony/xwork2/util/OgnlUtil.java =================================================================== --- src/java/com/opensymphony/xwork2/util/OgnlUtil.java (revision 1615) +++ src/java/com/opensymphony/xwork2/util/OgnlUtil.java (working copy) @@ -307,6 +307,18 @@ } /** + * Get's the java beans property descriptors for the given class. + * + * @param clazz the source object. + * @return property descriptors. + * @throws IntrospectionException is thrown if an exception occurs during introspection. + */ + public static PropertyDescriptor[] getPropertyDescriptors(Class clazz) throws IntrospectionException { + BeanInfo beanInfo = getBeanInfo(clazz); + return beanInfo.getPropertyDescriptors(); + } + + /** * Creates a Map with read properties for the given source object. *

* If the source object does not have a read property (i.e. write-only) then @@ -337,19 +349,30 @@ } /** - * Get's the java bean info for the given source. + * Get's the java bean info for the given source object. Calls getBeanInfo(Class c). * * @param from the source object. * @return java bean info. * @throws IntrospectionException is thrown if an exception occurs during introspection. */ public static BeanInfo getBeanInfo(Object from) throws IntrospectionException { + return getBeanInfo(from.getClass()); + } + + /** + * Get's the java bean info for the given source. + * + * @param clazz the source class. + * @return java bean info. + * @throws IntrospectionException is thrown if an exception occurs during introspection. + */ + public static BeanInfo getBeanInfo(Class clazz) throws IntrospectionException { synchronized (beanInfoCache) { BeanInfo beanInfo; - beanInfo = (BeanInfo) beanInfoCache.get(from.getClass()); + beanInfo = (BeanInfo) beanInfoCache.get(clazz); if (beanInfo == null) { - beanInfo = Introspector.getBeanInfo(from.getClass(), Object.class); - beanInfoCache.put(from.getClass(), beanInfo); + beanInfo = Introspector.getBeanInfo(clazz, Object.class); + beanInfoCache.put(clazz, beanInfo); } return beanInfo; }