Anonymous avatar Anonymous committed b2f358f

XW-98: Type conversion errors add field error messags to the Actions

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

Comments (0)

Files changed (5)

src/java/com/opensymphony/xwork/util/CompoundRootAccessor.java

 package com.opensymphony.xwork.util;
 
 import ognl.*;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import java.beans.IntrospectionException;
+
 import java.util.Iterator;
 import java.util.Map;
 
                 Object o = iterator.next();
 
                 try {
-                    if ((OgnlRuntime.hasGetProperty(ognlContext, o, name)) || ((o instanceof Map) && ((Map)o).containsKey(name))) {
+                    if ((OgnlRuntime.hasGetProperty(ognlContext, o, name)) || ((o instanceof Map) && ((Map) o).containsKey(name))) {
                         Object value = OgnlRuntime.getProperty(ognlContext, o, name);
 
                         //Ognl.getValue(OgnlUtil.compile((String) name), context, o);

src/java/com/opensymphony/xwork/util/XWorkConverter.java

  */
 package com.opensymphony.xwork.util;
 
+import com.opensymphony.xwork.LocaleAware;
+import com.opensymphony.xwork.ValidationAware;
+
 import ognl.DefaultTypeConverter;
 import ognl.Evaluation;
 import ognl.OgnlContext;
         this.defaultTypeConverter = defaultTypeConverter;
     }
 
-    public Object convertValue(Map context, Object object, Member member, String property, Object value, Class toClass) {
+    public Object convertValue(Map context, Object target, Member member, String property, Object value, Class toClass) {
         if (value == null) {
             return null;
         }
 
+        //
+        // Process the conversion using the default mappings, if one exists
+        //
+        TypeConverter tc = null;
+
         // allow this method to be called without any context
         // i.e. it can be called with as little as "Object value" and "Class toClass"
-        if ((context != null) && (object != null)) {
+        if (target != null) {
             Class clazz = null;
 
-            OgnlContext ognlContext = (OgnlContext) context;
-            Evaluation eval = ognlContext.getCurrentEvaluation();
+            clazz = target.getClass();
+
+            // this is to handle weird issues with setValue with a different type
+            if ((target instanceof CompoundRoot) && (context != null)) {
+                OgnlContext ognlContext = (OgnlContext) context;
+                Evaluation eval = ognlContext.getCurrentEvaluation();
 
-            if (eval == null) {
-                eval = ognlContext.getLastEvaluation();
+                if (eval == null) {
+                    eval = ognlContext.getLastEvaluation();
 
-                // since the upgrade to ognl-2.6.3.jar, eval is null here
-                // and this null check was being caoucht by an outer try/catch which ignored it !
-                if (eval != null) {
-                    clazz = eval.getResult().getClass();
-                    property = (String) eval.getLastChild().getResult();
+                    // since the upgrade to ognl-2.6.3.jar, eval is null here
+                    // and this null check was being caoucht by an outer try/catch which ignored it !
+                    if (eval != null) {
+                        clazz = eval.getResult().getClass();
+                        property = (String) eval.getLastChild().getResult();
+                    }
+                } else {
+                    clazz = eval.getLastChild().getSource().getClass();
+                    property = (String) eval.getFirstChild().getResult();
                 }
-            } else {
-                clazz = eval.getLastChild().getSource().getClass();
-                property = (String) eval.getFirstChild().getResult();
             }
 
-            //	
-            //
-            // I refactored this bit, as when runtime exceptions were occuring within a custom TypeConverter
-            // the clazz was being added to noMappings.
-            //
             if (!noMapping.contains(clazz)) {
-                TypeConverter tc = null;
-
                 try {
                     Map mapping = (Map) mappings.get(clazz);
 
                         String resource = className.replace('.', '/') + "-conversion.properties";
                         InputStream is = Thread.currentThread().getContextClassLoader().getResourceAsStream(resource);
 
-                        Properties props = new Properties();
-                        props.load(is);
-                        mapping.putAll(props);
-
-                        for (Iterator iterator = mapping.entrySet().iterator();
-                                iterator.hasNext();) {
-                            Map.Entry entry = (Map.Entry) iterator.next();
-                            entry.setValue(createTypeConverter((String) entry.getValue()));
+                        if (is != null) {
+                            Properties props = new Properties();
+                            props.load(is);
+                            mapping.putAll(props);
+
+                            for (Iterator iterator = mapping.entrySet().iterator();
+                                    iterator.hasNext();) {
+                                Map.Entry entry = (Map.Entry) iterator.next();
+                                entry.setValue(createTypeConverter((String) entry.getValue()));
+                            }
+                        } else {
+                            noMapping.add(clazz);
                         }
                     }
 
                 } catch (Throwable t) {
                     noMapping.add(clazz);
                 }
-
-                if (tc != null) {
-                    return tc.convertValue(context, object, member, property, value, toClass);
-                }
             }
         }
 
-        //
-        // Process the conversion using the default mappings, if one exists
-        //
-        TypeConverter tc = null;
-
-        if (toClass.equals(String.class) && !(value.getClass().equals(String.class) || value.getClass().equals(String[].class))) {
-            // when converting to a string, use the source object's class's converter
-            if (defaultMappings.containsKey(value.getClass().getName())) {
-                tc = (TypeConverter) defaultMappings.get(value.getClass().getName());
-            }
-        } else {
-            // when converting from a string, use the toClass's converter
-            if (defaultMappings.containsKey(toClass.getName())) {
-                //	converting from String
-                tc = (TypeConverter) defaultMappings.get(toClass.getName());
+        if (tc == null) {
+            if (toClass.equals(String.class) && !(value.getClass().equals(String.class) || value.getClass().equals(String[].class))) {
+                // when converting to a string, use the source target's class's converter
+                if (defaultMappings.containsKey(value.getClass().getName())) {
+                    tc = (TypeConverter) defaultMappings.get(value.getClass().getName());
+                }
+            } else {
+                // when converting from a string, use the toClass's converter
+                if (defaultMappings.containsKey(toClass.getName())) {
+                    //	converting from String
+                    tc = (TypeConverter) defaultMappings.get(toClass.getName());
+                }
             }
         }
 
         if (tc != null) {
             try {
-                return tc.convertValue(context, object, member, property, value, toClass);
-            } catch (Exception e) {
-                if (LOG.isDebugEnabled()) {
-                    LOG.error("Conversion failed", e);
+                Object returnVal = tc.convertValue(context, target, member, property, value, toClass);
+
+                if (returnVal == null) {
+                    handleConversionException(property, value, target);
                 }
 
+                return returnVal;
+            } catch (Exception e) {
+                handleConversionException(property, value, target);
+
                 return null;
             }
         }
 
         if (defaultTypeConverter != null) {
-            return defaultTypeConverter.convertValue(context, object, member, property, value, toClass);
+            try {
+                Object returnVal = defaultTypeConverter.convertValue(context, target, member, property, value, toClass);
+
+                if (returnVal == null) {
+                    handleConversionException(property, value, target);
+                }
+
+                return returnVal;
+            } catch (Exception e) {
+                handleConversionException(property, value, target);
+
+                return null;
+            }
         } else {
-            return super.convertValue(context, object, member, property, value, toClass);
+            try {
+                Object returnVal = super.convertValue(context, target, member, property, value, toClass);
+
+                if (returnVal == null) {
+                    handleConversionException(property, value, target);
+                }
+
+                return returnVal;
+            } catch (Exception e) {
+                handleConversionException(property, value, target);
+
+                return null;
+            }
         }
     }
 
         defaultMappings.put(className, converter);
     }
 
+    protected void handleConversionException(String property, Object value, Object object) {
+        String defaultMessage = "Invalid field value for field " + property + ": " + value;
+
+        if ((object != null) && (object instanceof ValidationAware)) {
+            String message;
+
+            if (object instanceof LocaleAware) {
+                message = ((LocaleAware) object).getText("invalid.fieldvalue." + property, defaultMessage);
+            } else {
+                message = defaultMessage;
+            }
+
+            ((ValidationAware) object).addFieldError(property, message);
+        } else {
+            LOG.warn(defaultMessage);
+        }
+    }
+
     private TypeConverter createTypeConverter(String className) throws Exception, InstantiationException {
         Class conversionClass = Thread.currentThread().getContextClassLoader().loadClass(className);
 

src/java/com/opensymphony/xwork/validator/validators/ValidatorSupport.java

             message = validatorContext.getText(messageKey, defaultMessage);
         } else {
             message = defaultMessage;
-            message = TextParseUtil.translateVariables(message, stack);
         }
 
+        message = TextParseUtil.translateVariables(message, stack);
+
         stack.pop();
 
         return message;

src/test/com/opensymphony/xwork/util/OgnlUtilTest.java

 
 import junit.framework.TestCase;
 
-import ognl.Ognl;
-import ognl.OgnlRuntime;
 import ognl.NullHandler;
+import ognl.Ognl;
 import ognl.OgnlException;
+import ognl.OgnlRuntime;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 
 import java.util.Calendar;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.List;
-import java.lang.reflect.Method;
-import java.lang.reflect.InvocationTargetException;
 
 
 /**
 public class OgnlUtilTest extends TestCase {
     //~ Methods ////////////////////////////////////////////////////////////////
 
+    public void testCanSetADependentObject() {
+        String dogName = "fido";
+
+        OgnlRuntime.setNullHandler(Owner.class, new NullHandler() {
+                public Object nullMethodResult(Map map, Object o, String s, Object[] objects) {
+                    System.out.println("nullMethodResult");
+
+                    return null;
+                }
+
+                public Object nullPropertyValue(Map map, Object o, Object o1) {
+                    System.out.println("nullPropertyValue");
+                    System.out.println("map -- " + map);
+                    System.out.println("o   -- " + o);
+                    System.out.println("o1  -- " + o1.getClass().getName());
+
+                    String methodName = o1.toString();
+                    String getter = "set" + methodName.substring(0, 1).toUpperCase() + methodName.substring(1);
+                    Method[] methods = o.getClass().getDeclaredMethods();
+                    System.out.println(getter);
+
+                    for (int i = 0; i < methods.length; i++) {
+                        String name = methods[i].getName();
+
+                        if (!getter.equals(name) || (methods[i].getParameterTypes().length != 1)) {
+                            continue;
+                        } else {
+                            Class clazz = methods[i].getParameterTypes()[0];
+
+                            try {
+                                Object param = clazz.newInstance();
+                                methods[i].invoke(o, new Object[] {param});
+
+                                return param;
+                            } catch (InstantiationException e) {
+                                e.printStackTrace(); //To change body of catch statement use Options | File Templates.
+                            } catch (IllegalAccessException e) {
+                                e.printStackTrace(); //To change body of catch statement use Options | File Templates.
+                            } catch (IllegalArgumentException e) {
+                                e.printStackTrace(); //To change body of catch statement use Options | File Templates.
+                            } catch (InvocationTargetException e) {
+                                e.printStackTrace(); //To change body of catch statement use Options | File Templates.
+                            }
+                        }
+                    }
+
+                    return null;
+                }
+            });
+
+        Owner owner = new Owner();
+        Map context = Ognl.createDefaultContext(owner);
+        HashMap props = new HashMap();
+        props.put("dog.name", dogName);
+
+        OgnlUtil.setProperties(props, owner, context);
+        assertNotNull("expected Ognl to create an instance of Dog", owner.getDog());
+        assertEquals(dogName, owner.getDog().getName());
+
+        try {
+            System.out.println("dog.name == " + Ognl.getValue("dog.name", new Owner()));
+        } catch (OgnlException e) {
+            e.printStackTrace(); //To change body of catch statement use Options | File Templates.
+        }
+    }
+
     public void testCopySameType() {
         Foo foo1 = new Foo();
         Foo foo2 = new Foo();
         assertEquals(foo.getBar().getTitle(), "i am barbaz");
     }
 
+    public void testOgnlHandlesCrapAtTheEndOfANumber() {
+        Foo foo = new Foo();
+        Map context = Ognl.createDefaultContext(foo);
+
+        HashMap props = new HashMap();
+        props.put("aLong", "123a");
+
+        OgnlUtil.setProperties(props, foo, context);
+        assertEquals(0, foo.getaLong());
+    }
+
     public void testSetPropertiesBoolean() {
         Foo foo = new Foo();
 
         OgnlUtil.setProperties(props, foo, context);
         assertEquals(123, foo.getaLong());
     }
-
-    public void testOgnlHandlesCrapAtTheEndOfANumber() {
-        Foo foo= new Foo();
-        Map context = Ognl.createDefaultContext(foo);
-
-        HashMap props = new HashMap();
-        props.put("aLong", "123a");
-
-        OgnlUtil.setProperties(props, foo, context);
-        assertEquals(0, foo.getaLong());
-    }
-
-    public void testCanSetADependentObject() {
-        String dogName = "fido";
-
-        OgnlRuntime.setNullHandler(Owner.class, new NullHandler() {
-            public Object nullMethodResult(Map map, Object o, String s, Object[] objects) {
-                System.out.println("nullMethodResult");
-                return null;
-            }
-
-            public Object nullPropertyValue(Map map, Object o, Object o1) {
-                System.out.println("nullPropertyValue");
-                System.out.println("map -- " + map);
-                System.out.println("o   -- " + o);
-                System.out.println("o1  -- " + o1.getClass().getName());
-                String methodName = o1.toString();
-                String getter = "set" + methodName.substring(0, 1).toUpperCase() + methodName.substring(1);
-                Method[] methods = o.getClass().getDeclaredMethods();
-                System.out.println(getter);
-                for (int i = 0; i < methods.length; i++) {
-                    String name = methods[i].getName();
-                    if( !getter.equals(name) || methods[i].getParameterTypes().length != 1 ) {
-                        continue;
-                    } else {
-                        Class clazz = methods[i].getParameterTypes()[0];
-                        try {
-                            Object param = clazz.newInstance();
-                            methods[i].invoke(o, new Object[]{param});
-                            return param;
-                        } catch (InstantiationException e) {
-                            e.printStackTrace();  //To change body of catch statement use Options | File Templates.
-                        } catch (IllegalAccessException e) {
-                            e.printStackTrace();  //To change body of catch statement use Options | File Templates.
-                        } catch (IllegalArgumentException e) {
-                            e.printStackTrace();  //To change body of catch statement use Options | File Templates.
-                        } catch (InvocationTargetException e) {
-                            e.printStackTrace();  //To change body of catch statement use Options | File Templates.
-                        }
-                    }
-                }
-                return null;
-            }
-        });
-
-        Owner owner = new Owner();
-        Map context = Ognl.createDefaultContext(owner);
-        HashMap props = new HashMap();
-        props.put("dog.name", dogName);
-
-        OgnlUtil.setProperties(props, owner, context);
-        assertNotNull("expected Ognl to create an instance of Dog", owner.getDog());
-        assertEquals(dogName, owner.getDog().getName());
-
-        try {
-            System.out.println("dog.name == " + Ognl.getValue("dog.name", new Owner()));
-        } catch (OgnlException e) {
-            e.printStackTrace();  //To change body of catch statement use Options | File Templates.
-        }
-    }
 }

src/test/com/opensymphony/xwork/util/XWorkConverterTest.java

  */
 package com.opensymphony.xwork.util;
 
+import com.opensymphony.xwork.SimpleAction;
+
 import junit.framework.TestCase;
 
 import ognl.Ognl;
 import ognl.OgnlException;
-import ognl.TypeConverter;
-
-import java.lang.reflect.Member;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 
     //~ Methods ////////////////////////////////////////////////////////////////
 
+    public void testFieldErrorMessageAddedWhenConversionFails() {
+        SimpleAction action = new SimpleAction();
+        action.setDate(null);
+
+        //        context = Ognl.createDefaultContext(action);
+        assertNull("Conversion should have failed.", converter.convertValue(context, action, null, "date", new String[] {
+                    "invalid date"
+                }, Date.class));
+        assertTrue(action.hasFieldErrors());
+        assertNotNull(action.getFieldErrors().get("date"));
+    }
+
     public void testStringArrayToCollection() {
         ArrayList list = new ArrayList();
         list.add("foo");
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.