Commits

cameronbraid  committed 75cbaf0

Issue number: WW-271
Bug in converter that used String.class instead of sourceObject.class when converting to string, causing sourceObject.toString() to be called instead of using the sourceObject.class converter.
Addition of programatic registration of converters.
Addition of programatic specification of a default converter to use as the fallback converter - when a converter isn't found in the registry.

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

  • Participants
  • Parent commits 912a450

Comments (0)

Files changed (4)

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

 import ognl.OgnlContext;
 import ognl.TypeConverter;
 
-import java.io.IOException;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
 import java.io.InputStream;
 
 import java.lang.reflect.Member;
 
-import java.util.*;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Properties;
 
 
 /**
     //~ Static fields/initializers /////////////////////////////////////////////
 
     private static XWorkConverter instance;
+    private static final Log LOG = LogFactory.getLog(XWorkConverter.class);
 
     //~ Instance fields ////////////////////////////////////////////////////////
 
     HashMap defaultMappings = new HashMap();
     HashMap mappings = new HashMap();
     HashSet noMapping = new HashSet();
+    TypeConverter defaultTypeConverter = null;
 
     //~ Constructors ///////////////////////////////////////////////////////////
 
         instance = null;
     }
 
-    public Object convertValue(Map context, Object o, Member member, String s, Object value, Class aClass) {
+    public void setDefaultConverter(TypeConverter defaultTypeConverter) {
+        this.defaultTypeConverter = defaultTypeConverter;
+    }
+
+    public Object convertValue(Map context, Object object, Member member, String property, Object value, Class toClass) {
         if (value == null) {
             return null;
         }
 
-        Class clazz = 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)) {
+            Class clazz = null;
 
-        try {
             OgnlContext ognlContext = (OgnlContext) context;
             Evaluation eval = ognlContext.getCurrentEvaluation();
-            String property;
 
             if (eval == null) {
                 eval = ognlContext.getLastEvaluation();
-                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();
             }
 
+            //	
+            //
+            // 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)) {
-                Map mapping = (Map) mappings.get(clazz);
+                TypeConverter tc = null;
+
+                try {
+                    Map mapping = (Map) mappings.get(clazz);
 
-                if (mapping == null) {
-                    mapping = new HashMap();
-                    mappings.put(clazz, mapping);
+                    if (mapping == null) {
+                        mapping = new HashMap();
+                        mappings.put(clazz, mapping);
 
-                    String className = clazz.getName();
-                    String resource = className.replace('.', '/') + "-conversion.properties";
-                    InputStream is = Thread.currentThread().getContextClassLoader().getResourceAsStream(resource);
+                        String className = clazz.getName();
+                        String resource = className.replace('.', '/') + "-conversion.properties";
+                        InputStream is = Thread.currentThread().getContextClassLoader().getResourceAsStream(resource);
 
-                    Properties props = new Properties();
-                    props.load(is);
-                    mapping.putAll(props);
+                        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()));
+                        for (Iterator iterator = mapping.entrySet().iterator();
+                                iterator.hasNext();) {
+                            Map.Entry entry = (Map.Entry) iterator.next();
+                            entry.setValue(createTypeConverter((String) entry.getValue()));
+                        }
                     }
-                }
 
-                TypeConverter converter = (TypeConverter) mapping.get(property);
+                    tc = (TypeConverter) mapping.get(property);
+                } catch (Throwable t) {
+                    noMapping.add(clazz);
+                }
 
-                if (converter != null) {
-                    return converter.convertValue(context, o, member, s, value, aClass);
+                if (tc != null) {
+                    return tc.convertValue(context, object, member, property, value, toClass);
                 }
             }
-        } catch (Throwable t) {
-            if (clazz != null) {
-                noMapping.add(clazz);
-            }
         }
 
-        if (defaultMappings.containsKey(aClass.getName())) {
-            try {
-                TypeConverter tc = (TypeConverter) defaultMappings.get(aClass.getName());
+        //
+        // Process the conversion using the default mappings, if one exists
+        //
+        if (defaultMappings.containsKey(toClass.getName())) {
+            TypeConverter tc = null;
 
-                return tc.convertValue(context, o, member, s, value, aClass);
-            } catch (Exception e) {
-                e.printStackTrace();
+            if (toClass.equals(String.class) && !(value.getClass().equals(String.class) || value.getClass().equals(String[].class))) {
+                // converting ToString from NON String 
+                tc = (TypeConverter) defaultMappings.get(value.getClass().getName());
+            } else {
+                //	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);
+                    }
+
+                    return null;
+                }
             }
         }
 
-        return super.convertValue(context, o, member, s, value, aClass);
+        if (defaultTypeConverter != null) {
+            return defaultTypeConverter.convertValue(context, object, member, property, value, toClass);
+        } else {
+            return super.convertValue(context, object, member, property, value, toClass);
+        }
+    }
+
+    public TypeConverter lookup(String className) {
+        return (TypeConverter) defaultMappings.get(className);
+    }
+
+    public TypeConverter lookup(Class clazz) {
+        return lookup(clazz.getName());
+    }
+
+    public void registerConverter(String className, TypeConverter converter) {
+        defaultMappings.put(className, converter);
     }
 
     private TypeConverter createTypeConverter(String className) throws Exception, InstantiationException {
             Map.Entry entry = (Map.Entry) iterator.next();
             String key = (String) entry.getKey();
 
-            defaultMappings.put(key, createTypeConverter((String) entry.getValue()));
+            try {
+                defaultMappings.put(key, createTypeConverter((String) entry.getValue()));
+            } catch (Exception e) {
+                LOG.error("Conversion registration error", e);
+            }
         }
     }
 }

File test/com/opensymphony/xwork/util/FooBarConverter.java

 
 import ognl.DefaultTypeConverter;
 
+import java.lang.reflect.Member;
+
 import java.util.Map;
 
 
 
         return null;
     }
+
+    /* (non-Javadoc)
+ * @see ognl.TypeConverter#convertValue(java.util.Map, java.lang.Object, java.lang.reflect.Member, java.lang.String, java.lang.Object, java.lang.Class)
+ */
+    public Object convertValue(Map context, Object source, Member member, String property, Object value, Class toClass) {
+        return convertValue(context, value, toClass);
+    }
 }

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

 package com.opensymphony.xwork.util;
 
 import junit.framework.TestCase;
+
 import ognl.Ognl;
 
 import java.util.Calendar;
 
         foo1.setTitle("blah");
         foo1.setNumber(1);
-        foo1.setPoints(new long[]{1, 2, 3});
+        foo1.setPoints(new long[] {1, 2, 3});
         foo1.setBirthday(cal.getTime());
         foo1.setUseful(false);
 
 
         foo.setTitle("blah");
         foo.setNumber(1);
-        foo.setPoints(new long[]{1, 2, 3});
+        foo.setPoints(new long[] {1, 2, 3});
         foo.setBirthday(cal.getTime());
         foo.setUseful(false);
 
         props.put("number", "2");
         OgnlUtil.setProperties(props, foo, context);
 
-        assertEquals(foo.getNumber(), 2);
+        assertEquals(2, foo.getNumber());
     }
 
     public void testSetPropertiesLongArray() {
         Map context = Ognl.createDefaultContext(foo);
 
         HashMap props = new HashMap();
-        props.put("points", new String[]{"1", "2"});
+        props.put("points", new String[] {"1", "2"});
         OgnlUtil.setProperties(props, foo, context);
 
         assertNotNull(foo.getPoints());
-        assertEquals(foo.getPoints().length, 2);
-        assertEquals(foo.getPoints()[0], 1);
-        assertEquals(foo.getPoints()[1], 2);
+        assertEquals(2, foo.getPoints().length);
+        assertEquals(1, foo.getPoints()[0]);
+        assertEquals(2, foo.getPoints()[1]);
     }
 
     public void testSetPropertiesString() {
         Map context = Ognl.createDefaultContext(foo);
         assertFalse(123456 == foo.getNumber());
         OgnlUtil.setProperty("number", "123456", foo, context);
-        assertTrue(123456 == foo.getNumber());
+        assertEquals(123456, foo.getNumber());
     }
 
     public void testStringToLong() {
         OgnlUtil.setProperties(props, foo, context);
         assertEquals(123, foo.getaLong());
 
-        props.put("aLong", new String[]{"123"});
+        props.put("aLong", new String[] {"123"});
 
         foo.setaLong(0);
         OgnlUtil.setProperties(props, foo, context);

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

 package com.opensymphony.xwork.util;
 
 import junit.framework.TestCase;
+
 import ognl.Ognl;
 import ognl.OgnlException;
+import ognl.TypeConverter;
+
+import java.lang.reflect.Member;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
 
 /**
         list.add("foo");
         list.add("bar");
         list.add("baz");
-        assertEquals(list, converter.convertValue(context, null, null, null, new String[]{
-            "foo", "bar", "baz"
-        }, Collection.class));
+        assertEquals(list, converter.convertValue(context, null, null, null, new String[] {
+                    "foo", "bar", "baz"
+                }, Collection.class));
     }
 
     public void testStringArrayToList() {
         list.add("foo");
         list.add("bar");
         list.add("baz");
-        assertEquals(list, converter.convertValue(context, null, null, null, new String[]{
-            "foo", "bar", "baz"
-        }, List.class));
+        assertEquals(list, converter.convertValue(context, null, null, null, new String[] {
+                    "foo", "bar", "baz"
+                }, List.class));
     }
 
     public void testStringArrayToPrimitiveWrappers() {
-        Long[] longs = (Long[]) converter.convertValue(context, null, null, null, new String[]{
-            "123", "456"
-        }, Long[].class);
+        Long[] longs = (Long[]) converter.convertValue(context, null, null, null, new String[] {
+                "123", "456"
+            }, Long[].class);
         assertNotNull(longs);
-        assertTrue(Arrays.equals(new Long[]{new Long(123), new Long(456)}, longs));
+        assertTrue(Arrays.equals(new Long[] {new Long(123), new Long(456)}, longs));
 
-        Integer[] ints = (Integer[]) converter.convertValue(context, null, null, null, new String[]{
-            "123", "456"
-        }, Integer[].class);
+        Integer[] ints = (Integer[]) converter.convertValue(context, null, null, null, new String[] {
+                "123", "456"
+            }, Integer[].class);
         assertNotNull(ints);
-        assertTrue(Arrays.equals(new Integer[]{
-            new Integer(123), new Integer(456)
-        }, ints));
+        assertTrue(Arrays.equals(new Integer[] {
+                    new Integer(123), new Integer(456)
+                }, ints));
 
-        Double[] doubles = (Double[]) converter.convertValue(context, null, null, null, new String[]{
-            "123", "456"
-        }, Double[].class);
+        Double[] doubles = (Double[]) converter.convertValue(context, null, null, null, new String[] {
+                "123", "456"
+            }, Double[].class);
         assertNotNull(doubles);
-        assertTrue(Arrays.equals(new Double[]{new Double(123), new Double(456)}, doubles));
+        assertTrue(Arrays.equals(new Double[] {new Double(123), new Double(456)}, doubles));
 
-        Float[] floats = (Float[]) converter.convertValue(context, null, null, null, new String[]{
-            "123", "456"
-        }, Float[].class);
+        Float[] floats = (Float[]) converter.convertValue(context, null, null, null, new String[] {
+                "123", "456"
+            }, Float[].class);
         assertNotNull(floats);
-        assertTrue(Arrays.equals(new Float[]{new Float(123), new Float(456)}, floats));
+        assertTrue(Arrays.equals(new Float[] {new Float(123), new Float(456)}, floats));
 
-        Boolean[] booleans = (Boolean[]) converter.convertValue(context, null, null, null, new String[]{
-            "true", "false"
-        }, Boolean[].class);
+        Boolean[] booleans = (Boolean[]) converter.convertValue(context, null, null, null, new String[] {
+                "true", "false"
+            }, Boolean[].class);
         assertNotNull(booleans);
-        assertTrue(Arrays.equals(new Boolean[]{Boolean.TRUE, Boolean.FALSE}, booleans));
+        assertTrue(Arrays.equals(new Boolean[] {Boolean.TRUE, Boolean.FALSE}, booleans));
     }
 
     public void testStringArrayToPrimitives() throws OgnlException {
-        long[] longs = (long[]) converter.convertValue(context, null, null, null, new String[]{
-            "123", "456"
-        }, long[].class);
+        long[] longs = (long[]) converter.convertValue(context, null, null, null, new String[] {
+                "123", "456"
+            }, long[].class);
         assertNotNull(longs);
-        assertTrue(Arrays.equals(new long[]{123, 456}, longs));
+        assertTrue(Arrays.equals(new long[] {123, 456}, longs));
 
-        int[] ints = (int[]) converter.convertValue(context, null, null, null, new String[]{
-            "123", "456"
-        }, int[].class);
+        int[] ints = (int[]) converter.convertValue(context, null, null, null, new String[] {
+                "123", "456"
+            }, int[].class);
         assertNotNull(ints);
-        assertTrue(Arrays.equals(new int[]{123, 456}, ints));
+        assertTrue(Arrays.equals(new int[] {123, 456}, ints));
 
-        double[] doubles = (double[]) converter.convertValue(context, null, null, null, new String[]{
-            "123", "456"
-        }, double[].class);
+        double[] doubles = (double[]) converter.convertValue(context, null, null, null, new String[] {
+                "123", "456"
+            }, double[].class);
         assertNotNull(doubles);
-        assertTrue(Arrays.equals(new double[]{123, 456}, doubles));
+        assertTrue(Arrays.equals(new double[] {123, 456}, doubles));
 
-        float[] floats = (float[]) converter.convertValue(context, null, null, null, new String[]{
-            "123", "456"
-        }, float[].class);
+        float[] floats = (float[]) converter.convertValue(context, null, null, null, new String[] {
+                "123", "456"
+            }, float[].class);
         assertNotNull(floats);
-        assertTrue(Arrays.equals(new float[]{123, 456}, floats));
+        assertTrue(Arrays.equals(new float[] {123, 456}, floats));
 
-        boolean[] booleans = (boolean[]) converter.convertValue(context, null, null, null, new String[]{
-            "true", "false"
-        }, boolean[].class);
+        boolean[] booleans = (boolean[]) converter.convertValue(context, null, null, null, new String[] {
+                "true", "false"
+            }, boolean[].class);
         assertNotNull(booleans);
-        assertTrue(Arrays.equals(new boolean[]{true, false}, booleans));
+        assertTrue(Arrays.equals(new boolean[] {true, false}, booleans));
     }
 
     public void testStringArrayToSet() {
         list.add("foo");
         list.add("bar");
         list.add("baz");
-        assertEquals(list, converter.convertValue(context, null, null, null, new String[]{
-            "foo", "bar", "bar", "baz"
-        }, Set.class));
+        assertEquals(list, converter.convertValue(context, null, null, null, new String[] {
+                    "foo", "bar", "bar", "baz"
+                }, Set.class));
+    }
+
+    public void testStringToCustomTypeUsingCustomConverter() {
+        // the converter needs to be registered as the Bar.class converter 
+        // it won't be detected from the Foo-conversion.properties
+        // because the Foo-conversion.properties file is only used when converting a property of Foo
+        converter.registerConverter(Bar.class.getName(), new FooBarConverter());
+
+        Bar bar = null;
+
+        bar = (Bar) converter.convertValue(null, null, null, null, "blah:123", Bar.class);
+        assertNotNull("conversion failed", bar);
+        assertEquals(123, bar.getSomethingElse());
+        assertEquals("blah", bar.getTitle());
     }
 
     public void testStringToPrimitiveWrappers() {