Merge pull request #199 from yasserzamani/WW-4906

fix conversion by annotation and WW-4906
apache · Jan 15, 2018 · 29e1847 · 29e1847
2 parents 29b29a9 + 9739291
commit 29e1847
Show file tree

Hide file tree

Showing 13 changed files with 206 additions and 25 deletions.
diff --git a/core/src/main/java/com/opensymphony/xwork2/conversion/annotations/TypeConversion.java b/core/src/main/java/com/opensymphony/xwork2/conversion/annotations/TypeConversion.java
@@ -82,15 +82,15 @@
  * </tr>
  * <tr>
  * <td>converter</td>
- * <td>DEPRECATED: either this or value</td>
+ * <td>either this or value</td>
  * <td>&nbsp;</td>
- * <td>The class name of the TypeConverter to be used as converter.</td>
+ * <td>The class or bean name of the TypeConverter to be used as converter.</td>
  * </tr>
  * <tr>
  * <td>converterClass</td>
  * <td>either this or value</td>
- * <td>&nbsp;</td>
- * <td>The class of the TypeConverter to be used as converter. XWorkBasicConverter by default.</td>
+ * <td>XWorkBasicConverter</td>
+ * <td>The class of the TypeConverter to be used as converter.</td>
  * </tr>
  * <tr>
  * <td>value</td>
@@ -181,14 +181,13 @@
     ConversionRule rule() default ConversionRule.PROPERTY;
 
     /**
-     * The class of the TypeConverter to be used as converter.
+     * The class or bean name of the TypeConverter to be used as converter.
      *
      * Note: This can not be used with ConversionRule.KEY_PROPERTY!
      *
-     * @return class of the TypeConverter to be used as converter
-     * @deprecated user {@link #converterClass()} instead
+     * @return class or bean name of the TypeConverter to be used as converter
+     * @see {@link #converterClass()}
      */
-    @Deprecated
     String converter() default "";
 
     /**

diff --git a/...in/java/com/opensymphony/xwork2/conversion/impl/DefaultConversionAnnotationProcessor.java b/...in/java/com/opensymphony/xwork2/conversion/impl/DefaultConversionAnnotationProcessor.java
@@ -26,6 +26,7 @@
 import com.opensymphony.xwork2.conversion.annotations.ConversionType;
 import com.opensymphony.xwork2.conversion.annotations.TypeConversion;
 import com.opensymphony.xwork2.inject.Inject;
+import com.opensymphony.xwork2.util.ClassLoaderUtil;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -69,7 +70,7 @@ public void process(Map<String, Object> mapping, TypeConversion tc, String key)
                     mapping.put(key, tc.value());
                 }
                 //for properties of classes
-                else if (tc.rule() != ConversionRule.ELEMENT || tc.rule() == ConversionRule.KEY || tc.rule() == ConversionRule.COLLECTION) {
+                else if (tc.rule() != ConversionRule.ELEMENT && tc.rule() != ConversionRule.KEY && tc.rule() != ConversionRule.COLLECTION) {
                     if (StringUtils.isNoneEmpty(tc.converter())) {
                         mapping.put(key, converterCreator.createTypeConverter(tc.converter()));
                     } else {
@@ -80,18 +81,22 @@ else if (tc.rule() != ConversionRule.ELEMENT || tc.rule() == ConversionRule.KEY
                 else if (tc.rule() == ConversionRule.KEY) {
                     Class<?> converterClass;
                     if (StringUtils.isNoneEmpty(tc.converter())) {
-                        converterClass = Thread.currentThread().getContextClassLoader().loadClass(tc.converter());
-                        //check if the converter is a type converter if it is one
-                        //then just put it in the map as is. Otherwise
-                        //put a value in for the type converter of the class
+                        converterClass = ClassLoaderUtil.loadClass(tc.converter(), this.getClass());
                     } else {
                         converterClass = tc.converterClass();
                     }
 
                     LOG.debug("Converter class: [{}]", converterClass);
 
+                    //check if the converter is a type converter if it is one
+                    //then just put it in the map as is. Otherwise
+                    //put a value in for the type converter of the class
                     if (converterClass.isAssignableFrom(TypeConverter.class)) {
-                        mapping.put(key, converterCreator.createTypeConverter(tc.converter()));
+                        if (StringUtils.isNoneEmpty(tc.converter())) {
+                            mapping.put(key, converterCreator.createTypeConverter(tc.converter()));
+                        } else {
+                            mapping.put(key, converterCreator.createTypeConverter(tc.converterClass()));
+                        }
                     } else {
                         mapping.put(key, converterClass);
                         LOG.debug("Object placed in mapping for key [{}] is [{}]", key, mapping.get(key));
@@ -100,7 +105,7 @@ else if (tc.rule() == ConversionRule.KEY) {
                 //elements(values) of maps / lists
                 else {
                     if (StringUtils.isNoneEmpty(tc.converter())) {
-                        mapping.put(key, Thread.currentThread().getContextClassLoader().loadClass(tc.converter()));
+                        mapping.put(key, ClassLoaderUtil.loadClass(tc.converter(), this.getClass()));
                     } else {
                         mapping.put(key, tc.converterClass());
                     }

diff --git a/...src/main/java/com/opensymphony/xwork2/conversion/impl/DefaultConversionFileProcessor.java b/...src/main/java/com/opensymphony/xwork2/conversion/impl/DefaultConversionFileProcessor.java
@@ -86,7 +86,7 @@ else if (!(key.startsWith(DefaultObjectTypeDeterminer.ELEMENT_PREFIX) ||
                     //for keys of Maps
                     else if (key.startsWith(DefaultObjectTypeDeterminer.KEY_PREFIX)) {
 
-                        Class converterClass = Thread.currentThread().getContextClassLoader().loadClass((String) entry.getValue());
+                        Class converterClass = ClassLoaderUtil.loadClass((String) entry.getValue(), this.getClass());
 
                         //check if the converter is a type converter if it is one
                         //then just put it in the map as is. Otherwise
@@ -102,7 +102,7 @@ else if (key.startsWith(DefaultObjectTypeDeterminer.KEY_PREFIX)) {
                     }
                     //elements(values) of maps / lists
                     else {
-                        Class _c = Thread.currentThread().getContextClassLoader().loadClass((String) entry.getValue());
+                        Class _c = ClassLoaderUtil.loadClass((String) entry.getValue(), this.getClass());
                         LOG.debug("\t{}:{} [treated as Class {}]", key, entry.getValue(), _c);
                         mapping.put(key, _c);
                     }

diff --git a/core/src/main/java/com/opensymphony/xwork2/conversion/impl/XWorkConverter.java b/core/src/main/java/com/opensymphony/xwork2/conversion/impl/XWorkConverter.java
@@ -500,15 +500,32 @@ protected void addConverterMapping(Map<String, Object> mapping, Class clazz) {
             for (Annotation annotation : annotations) {
                 if (annotation instanceof TypeConversion) {
                     TypeConversion tc = (TypeConversion) annotation;
-                    if (mapping.containsKey(tc.key())) {
-                        break;
-                    }
                     String key = tc.key();
-                    // Default to the property name
+                    // Default to the property name with prefix
                     if (StringUtils.isEmpty(key)) {
                         key = AnnotationUtils.resolvePropertyName(method);
+                        switch (tc.rule()) {
+                            case COLLECTION:
+                                key = DefaultObjectTypeDeterminer.DEPRECATED_ELEMENT_PREFIX + key;
+                                break;
+                            case CREATE_IF_NULL:
+                                key = DefaultObjectTypeDeterminer.CREATE_IF_NULL_PREFIX + key;
+                                break;
+                            case ELEMENT:
+                                key = DefaultObjectTypeDeterminer.ELEMENT_PREFIX + key;
+                                break;
+                            case KEY:
+                                key = DefaultObjectTypeDeterminer.KEY_PREFIX + key;
+                                break;
+                            case KEY_PROPERTY:
+                                key = DefaultObjectTypeDeterminer.KEY_PROPERTY_PREFIX + key;
+                                break;
+                        }
                         LOG.debug("Retrieved key [{}] from method name [{}]", key, method.getName());
                     }
+                    if (mapping.containsKey(key)) {
+                        break;
+                    }
                     annotationProcessor.process(mapping, tc, key);
                 }
             }

diff --git a/core/src/test/java/com/opensymphony/xwork2/AnnotatedTestBean.java b/core/src/test/java/com/opensymphony/xwork2/AnnotatedTestBean.java
@@ -18,6 +18,9 @@
  */
 package com.opensymphony.xwork2;
 
+import com.opensymphony.xwork2.conversion.annotations.TypeConversion;
+import com.opensymphony.xwork2.conversion.impl.FooBarConverter;
+import com.opensymphony.xwork2.util.Bar;
 import com.opensymphony.xwork2.validator.annotations.IntRangeFieldValidator;
 import com.opensymphony.xwork2.validator.annotations.RequiredStringValidator;
 import com.opensymphony.xwork2.validator.annotations.Validations;
@@ -37,6 +40,7 @@ public class AnnotatedTestBean {
     private Date birth;
     private String name;
     private int count;
+    private Bar bar;
 
     //~ Constructors ///////////////////////////////////////////////////////////
 
@@ -76,4 +80,13 @@ public void setName(String name) {
     public String getName() {
         return name;
     }
+
+    public Bar getSupperBarObj() {
+        return bar;
+    }
+
+    @TypeConversion(converter = "com.opensymphony.xwork2.conversion.impl.FooBarConverter")
+    public void setSupperBarObj(Bar b) {
+        bar = b;
+    }
 }
diff --git a/core/src/test/java/com/opensymphony/xwork2/conversion/impl/AnnotationXWorkConverterTest.java b/core/src/test/java/com/opensymphony/xwork2/conversion/impl/AnnotationXWorkConverterTest.java
@@ -177,7 +177,7 @@ public void testFindConversionErrorMessage() {
         assertEquals("Invalid field value for field \"foo\".", message);
     }
 
-    public void testFindConversionMappingForInterface() {
+    public void testFindConversionMappingForInterfaceAndSuperclass() {
         ModelDrivenAnnotationAction2 action = new ModelDrivenAnnotationAction2();
         ValueStack stack = ActionContext.getContext().getValueStack();
         stack.push(action);
@@ -193,6 +193,14 @@ public void testFindConversionMappingForInterface() {
 
         Bar b = (Bar) o;
         assertEquals(value, b.getTitle() + ":" + b.getSomethingElse());
+
+        String value2 = "qwer:456";
+        Object o2 = converter.convertValue(ognlStackContext, action.getModel(), null, "supperBarObj", value2, Bar.class);
+        assertNotNull(o2);
+        assertTrue("class is: " + o.getClass(), o2 instanceof Bar);
+
+        Bar b2 = (Bar) o2;
+        assertEquals(value2, b2.getTitle() + ":" + b2.getSomethingElse());
     }
 
     public void testLocalizedDateConversion() throws Exception {

diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
@@ -582,7 +582,7 @@ public void testGetBeanMap() throws Exception {
         // just do some of the 15 tests
         Map beans = ognlUtil.getBeanMap(foo);
         assertNotNull(beans);
-        assertEquals(19, beans.size());
+        assertEquals(21, beans.size());
         assertEquals("Hello Santa", beans.get("title"));
         assertEquals(new Long("123"), beans.get("ALong"));
         assertEquals(new Integer("44"), beans.get("number"));

diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
@@ -733,11 +733,24 @@ public void testSetNullList() {
         assertEquals("Cat One", ((Cat) foo.getCats().get(0)).getName());
         assertEquals("Cat Two", ((Cat) foo.getCats().get(1)).getName());
 
+        vs.setValue("annotatedCats[0].name", "Cat One By Annotation");
+        vs.setValue("annotatedCats[1].name", "Cat Two By Annotation");
+        assertNotNull(foo.getAnnotatedCats());
+        assertEquals(2, foo.getAnnotatedCats().size());
+        assertEquals("Cat One By Annotation", ((Cat) foo.getAnnotatedCats().get(0)).getName());
+        assertEquals("Cat Two By Annotation", ((Cat) foo.getAnnotatedCats().get(1)).getName());
+
         vs.setValue("cats[0].foo.cats[1].name", "Deep null cat");
         assertNotNull(((Cat) foo.getCats().get(0)).getFoo());
         assertNotNull(((Cat) foo.getCats().get(0)).getFoo().getCats());
         assertNotNull(((Cat) foo.getCats().get(0)).getFoo().getCats().get(1));
         assertEquals("Deep null cat", ((Cat) ((Cat) foo.getCats().get(0)).getFoo().getCats().get(1)).getName());
+
+        vs.setValue("annotatedCats[0].foo.annotatedCats[1].name", "Deep null cat by annotation");
+        assertNotNull(((Cat) foo.getAnnotatedCats().get(0)).getFoo());
+        assertNotNull(((Cat) foo.getAnnotatedCats().get(0)).getFoo().getAnnotatedCats());
+        assertNotNull(((Cat) foo.getAnnotatedCats().get(0)).getFoo().getAnnotatedCats().get(1));
+        assertEquals("Deep null cat by annotation", ((Cat) ((Cat) foo.getAnnotatedCats().get(0)).getFoo().getAnnotatedCats().get(1)).getName());
     }
 
     public void testSetMultiple() {

diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/SetPropertiesTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/SetPropertiesTest.java
@@ -248,6 +248,7 @@ public void doTestAddingAndModifyingCollectionWithObjects(Collection barColl) {
         Bar bar2 = new Bar();
         bar2.setId(new Long(22));
         barColl.add(bar2);
+        foo.setAnnotatedBarCollection(barColl);
         //try modifying bar1 and bar2
         //check the logs here to make sure
         //the Map is being created
@@ -266,13 +267,31 @@ public void doTestAddingAndModifyingCollectionWithObjects(Collection barColl) {
                 assertEquals(bar1Title, next.getTitle());
             }
         }
+        Bar bar3 = new Bar();
+        bar3.setId(new Long(33));
+        barColl.add(bar3);
+        Bar bar4 = new Bar();
+        bar4.setId(new Long(44));
+        barColl.add(bar4);
+        String bar1TitleByAnnotation = "The Phantom Menace By Annotation";
+        String bar2TitleByAnnotation = "The Clone Wars By Annotation";
+        vs.setValue("annotatedBarCollection(44).title", bar2TitleByAnnotation);
+        vs.setValue("annotatedBarCollection(33).title", bar1TitleByAnnotation);
+        for (Object aBarColl : barColl) {
+            Bar next = (Bar) aBarColl;
+            if (next.getId().intValue() == 44) {
+                assertEquals(bar2TitleByAnnotation, next.getTitle());
+            } else if (next.getId().intValue() == 33) {
+                assertEquals(bar1TitleByAnnotation, next.getTitle());
+            }
+        }
         //now test adding to a collection
         String bar3Title = "Revenge of the Sith";
         String bar4Title = "A New Hope";
         vs.setValue("barCollection.makeNew[4].title", bar4Title, true);
         vs.setValue("barCollection.makeNew[0].title", bar3Title, true);
 
-        assertEquals(4, barColl.size());
+        assertEquals(6, barColl.size());
 
         for (Object aBarColl : barColl) {
             Bar next = (Bar) aBarColl;
@@ -283,6 +302,24 @@ public void doTestAddingAndModifyingCollectionWithObjects(Collection barColl) {
             }
         }
 
+        //now test adding to a collection by annotation
+        String bar3TitleByAnnotation = "Revenge of the Sith By Annotation";
+        String bar4TitleByAnnotation = "A New Hope By Annotation";
+        vs.setValue("annotatedBarCollection.makeNew[5].title", bar4TitleByAnnotation, true);
+        vs.setValue("annotatedBarCollection.makeNew[1].title", bar3TitleByAnnotation, true);
+
+        assertEquals(8, barColl.size());
+
+        for (Object aBarColl : barColl) {
+            Bar next = (Bar) aBarColl;
+            if (next.getId() == null) {
+                assertNotNull(next.getTitle());
+                assertTrue(next.getTitle().equals(bar4TitleByAnnotation)
+                        || next.getTitle().equals(bar3TitleByAnnotation)
+                        || next.getTitle().equals(bar4Title)
+                        || next.getTitle().equals(bar3Title));
+            }
+        }
     }
     public void testAddingToCollectionBasedOnPermission() {
         final MockObjectTypeDeterminer determiner = new MockObjectTypeDeterminer(Long.class,Bar.class,"id",true);

diff --git a/core/src/test/java/com/opensymphony/xwork2/test/annotations/PersonAction.java b/core/src/test/java/com/opensymphony/xwork2/test/annotations/PersonAction.java
@@ -32,7 +32,7 @@
 						converterClass=AddressTypeConverter.class),
 		@TypeConversion(type=ConversionType.APPLICATION,
 						key="com.opensymphony.xwork2.test.annotations.Person",
-						converterClass=PersonTypeConverter.class)})
+						converter="com.opensymphony.xwork2.test.annotations.PersonTypeConverter")})
 public class PersonAction {
 	List<Person> users;
 	private List<Address> address;

diff --git a/core/src/test/java/com/opensymphony/xwork2/util/Foo.java b/core/src/test/java/com/opensymphony/xwork2/util/Foo.java
@@ -18,6 +18,10 @@
  */
 package com.opensymphony.xwork2.util;
 
+import com.opensymphony.xwork2.conversion.annotations.ConversionRule;
+import com.opensymphony.xwork2.conversion.annotations.ConversionType;
+import com.opensymphony.xwork2.conversion.annotations.TypeConversion;
+
 import java.util.*;
 
 
@@ -34,9 +38,11 @@ public class Foo {
     Date meeting;
     Foo child;
     List cats;
+    List annotatedCats;
     List moreCats;
     List strings;
     Collection barCollection;
+    Collection annotatedBarCollection;
     Map catMap;
     Map anotherCatMap;
     String title;
@@ -96,6 +102,15 @@ public List getCats() {
         return cats;
     }
 
+    public void setAnnotatedCats(List annotatedCats) {
+        this.annotatedCats = annotatedCats;
+    }
+
+    @TypeConversion(rule = ConversionRule.ELEMENT, converterClass = Cat.class)
+    public List getAnnotatedCats() {
+        return annotatedCats;
+    }
+
     public void setChild(Foo child) {
         this.child = child;
     }
@@ -154,6 +169,16 @@ public void setBarCollection(Collection barCollection) {
         this.barCollection = barCollection;
     }
 
+    @TypeConversion(rule = ConversionRule.KEY_PROPERTY, value = "id")
+    public void setAnnotatedBarCollection(Collection annotatedBarCollection) {
+        this.annotatedBarCollection = annotatedBarCollection;
+    }
+
+    @TypeConversion(rule = ConversionRule.ELEMENT, converter = "com.opensymphony.xwork2.util.Bar")
+    public Collection getAnnotatedBarCollection() {
+        return annotatedBarCollection;
+    }
+
     public void setPoints(long[] points) {
         this.points = points;
     }