You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ibatis.apache.org by cb...@apache.org on 2007/03/20 06:32:04 UTC

svn commit: r520283 - in /ibatis/trunk/java/mapper/mapper2: build/ src/com/ibatis/common/beans/ test/badbeans/ test/com/ibatis/common/beans/

Author: cbegin
Date: Mon Mar 19 22:32:03 2007
New Revision: 520283

URL: http://svn.apache.org/viewvc?view=rev&rev=520283
Log:
Made ClassInfo more tolerant of oddball JavaBeans, and now throws exceptions instead of just logging.  

Supported Cases:
  * Proper JavaBeans no overloaded getter/setter, same type returned/taken respectively.
  * Weird JavaBeans with no overloads, but mismatched types for getter/setter.
  * Weird JavaBeans with overloaded setter, at least one matches the retun type of getter.

Unsupported (exception thrown)
  * Bad write only JavaBeans with ambiguous type -- No getter, multiple setter overloads 
  * Bad read/write JavaBeans with no setter that matches the return type of getter

Added:
    ibatis/trunk/java/mapper/mapper2/test/badbeans/
    ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithDifferentTypeGetterSetter.java
    ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithDifferentTypeOverloadedSetter.java
    ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithNoGetterOverloadedSetters.java
    ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithOverloadedSetter.java
    ibatis/trunk/java/mapper/mapper2/test/badbeans/GoodBean.java
    ibatis/trunk/java/mapper/mapper2/test/com/ibatis/common/beans/BadBeanTest.java
Modified:
    ibatis/trunk/java/mapper/mapper2/build/version.properties
    ibatis/trunk/java/mapper/mapper2/src/com/ibatis/common/beans/ClassInfo.java

Modified: ibatis/trunk/java/mapper/mapper2/build/version.properties
URL: http://svn.apache.org/viewvc/ibatis/trunk/java/mapper/mapper2/build/version.properties?view=diff&rev=520283&r1=520282&r2=520283
==============================================================================
--- ibatis/trunk/java/mapper/mapper2/build/version.properties (original)
+++ ibatis/trunk/java/mapper/mapper2/build/version.properties Mon Mar 19 22:32:03 2007
@@ -1,5 +1,5 @@
 #Build version info
-#Fri Mar 02 17:32:54 MST 2007
+#Mon Mar 19 23:25:50 MDT 2007
 version=2.3.1
-buildDate=2007/03/02 17\:32
-buildNum=687
+buildDate=2007/03/19 23\:25
+buildNum=689

Modified: ibatis/trunk/java/mapper/mapper2/src/com/ibatis/common/beans/ClassInfo.java
URL: http://svn.apache.org/viewvc/ibatis/trunk/java/mapper/mapper2/src/com/ibatis/common/beans/ClassInfo.java?view=diff&rev=520283&r1=520282&r2=520283
==============================================================================
--- ibatis/trunk/java/mapper/mapper2/src/com/ibatis/common/beans/ClassInfo.java (original)
+++ ibatis/trunk/java/mapper/mapper2/src/com/ibatis/common/beans/ClassInfo.java Mon Mar 19 22:32:03 2007
@@ -15,12 +15,8 @@
  */
 package com.ibatis.common.beans;
 
-import com.ibatis.common.logging.Log;
-import com.ibatis.common.logging.LogFactory;
-
 import java.lang.reflect.*;
-import java.math.BigDecimal;
-import java.math.BigInteger;
+import java.math.*;
 import java.util.*;
 
 /**
@@ -29,8 +25,6 @@
  */
 public class ClassInfo {
   
-  private static final Log log = LogFactory.getLog(ClassInfo.class);
-
   private static boolean cacheEnabled = true;
   private static final String[] EMPTY_STRING_ARRAY = new String[0];
   private static final Set SIMPLE_TYPE_SET = new HashSet();
@@ -78,7 +72,8 @@
   private ClassInfo(Class clazz) {
     className = clazz.getName();
     addDefaultConstructor(clazz);
-    addMethods(clazz);
+    addGetMethods(clazz);
+    addSetMethods(clazz);
     addFields (clazz);
     readablePropertyNames = (String[]) getMethods.keySet().toArray(new String[getMethods.keySet().size()]);
     writeablePropertyNames = (String[]) setMethods.keySet().toArray(new String[setMethods.keySet().size()]);
@@ -103,6 +98,98 @@
     }
   }
 
+  private void addGetMethods(Class cls) {
+    Method[] methods = getAllMethodsForClass(cls);
+    for (int i = 0; i < methods.length; i++) {
+      Method method = methods[i];
+      String name = method.getName();
+      if (name.startsWith("get") && name.length() > 3) {
+        if (method.getParameterTypes().length == 0) {
+          name = dropCase(name);
+          addGetMethod(name, method);
+        }
+      } else if (name.startsWith("is") && name.length() > 2) {
+        if (method.getParameterTypes().length == 0) {
+          name = dropCase(name);
+          addGetMethod(name, method);
+        }
+      }
+    }
+  }
+
+  private void addGetMethod(String name, Method method) {
+    getMethods.put(name, new MethodInvoker(method));
+    getTypes.put(name, method.getReturnType());
+  }
+
+  private void addSetMethods(Class cls) {
+    Map conflictingSetters = new HashMap();
+    Method[] methods = getAllMethodsForClass(cls);
+    for (int i = 0; i < methods.length; i++) {
+      Method method = methods[i];
+      String name = method.getName();
+      if (name.startsWith("set") && name.length() > 3) {
+        if (method.getParameterTypes().length == 1) {
+          name = dropCase(name);
+          ///------------
+          addSetterConflict(conflictingSetters, name, method);
+          // addSetMethod(name, method);
+          ///------------
+        }
+      }
+    }
+    resolveSetterConflicts(conflictingSetters);
+  }
+
+  private void addSetterConflict(Map conflictingSetters, String name, Method method) {
+    List list = (List)conflictingSetters.get(name);
+    if (list == null) {
+      list = new ArrayList();
+      conflictingSetters.put(name,list);
+    }
+    list.add(method);
+  }
+
+  private void resolveSetterConflicts(Map conflictingSetters) {
+    for (Iterator propNames = conflictingSetters.keySet().iterator(); propNames.hasNext();) {
+      String propName = (String) propNames.next();
+      List setters = (List) conflictingSetters.get(propName);
+      Method firstMethod = (Method) setters.get(0);
+      if (setters.size() == 1) {
+        addSetMethod(propName, firstMethod);
+      } else {
+        Class expectedType = (Class) getTypes.get(propName);
+        if (expectedType == null) {
+          throw new RuntimeException ("Illegal overloaded setter method with ambiguous type for property "
+              + propName + " in class " + firstMethod.getDeclaringClass() + ".  This breaks the JavaBeans " +
+              "specification and can cause unpredicatble results.");
+        } else {
+          Iterator methods = setters.iterator();
+          Method setter = null;
+          while (methods.hasNext()) {
+            Method method = (Method) methods.next();
+            if (method.getParameterTypes().length == 1
+                && expectedType.equals(method.getParameterTypes()[0])) {
+              setter = method;
+              break;
+            }
+          }
+          if (setter == null) {
+            throw new RuntimeException ("Illegal overloaded setter method with ambiguous type for property "
+                + propName + " in class " + firstMethod.getDeclaringClass() + ".  This breaks the JavaBeans " +
+                "specification and can cause unpredicatble results.");
+          }
+          addSetMethod(propName, setter);
+        }
+      }
+    }
+  }
+
+  private void addSetMethod(String name, Method method) {
+    setMethods.put(name, new MethodInvoker(method));
+    setTypes.put(name, method.getParameterTypes()[0]);
+  }
+
   private void addFields(Class clazz) {
     Field[] fields = clazz.getDeclaredFields();
     for (int i=0; i < fields.length; i++) {
@@ -116,12 +203,10 @@
       }
       if (field.isAccessible()) {
         if (!setMethods.containsKey(field.getName())) {
-          setMethods.put(field.getName(), new SetFieldInvoker(field));
-          setTypes.put(field.getName(), field.getType());
+          addSetField(field);
         }
         if (!getMethods.containsKey(field.getName())) {
-          getMethods.put(field.getName(), new GetFieldInvoker(field));
-          getTypes.put(field.getName(), field.getType());
+          addGetField(field);
         }
       }
     }
@@ -130,37 +215,14 @@
     }
   }
 
-  private void addMethods(Class cls) {
-    Method[] methods = getAllMethodsForClass(cls);
-    for (int i = 0; i < methods.length; i++) {
-      Method method = methods[i];
-      String name = method.getName();
-      if (name.startsWith("set") && name.length() > 3) {
-        if (method.getParameterTypes().length == 1) {
-          name = dropCase(name);
-          if (setMethods.containsKey(name)) {
-            // TODO(JGB) - this should probably be a RuntimeException at some point???
-            log.error("Illegal overloaded setter method for property " + name + " in class " + cls.getName() +
-                ".  This breaks the JavaBeans specification and can cause unpredicatble results.");
-          }
-          setMethods.put(name, new MethodInvoker(method));
-          setTypes.put(name, method.getParameterTypes()[0]);
-        }
-      } else if (name.startsWith("get") && name.length() > 3) {
-        if (method.getParameterTypes().length == 0) {
-          name = dropCase(name);
-          getMethods.put(name, new MethodInvoker(method));
-          getTypes.put(name, method.getReturnType());
-        }
-      } else if (name.startsWith("is") && name.length() > 2) {
-        if (method.getParameterTypes().length == 0) {
-          name = dropCase(name);
-          getMethods.put(name, new MethodInvoker(method));
-          getTypes.put(name, method.getReturnType());
-        }
-      }
-      name = null;
-    }
+  private void addSetField(Field field) {
+    setMethods.put(field.getName(), new SetFieldInvoker(field));
+    setTypes.put(field.getName(), field.getType());
+  }
+
+  private void addGetField(Field field) {
+    getMethods.put(field.getName(), new GetFieldInvoker(field));
+    getTypes.put(field.getName(), field.getType());
   }
 
   private Method[] getAllMethodsForClass(Class cls) {

Added: ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithDifferentTypeGetterSetter.java
URL: http://svn.apache.org/viewvc/ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithDifferentTypeGetterSetter.java?view=auto&rev=520283
==============================================================================
--- ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithDifferentTypeGetterSetter.java (added)
+++ ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithDifferentTypeGetterSetter.java Mon Mar 19 22:32:03 2007
@@ -0,0 +1,15 @@
+package badbeans;
+
+public class BeanWithDifferentTypeGetterSetter {
+
+  private String value;
+
+
+  public String getValue() {
+    return value;
+  }
+
+  public void setValue(Integer value) {
+    this.value = value.toString();
+  }
+}

Added: ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithDifferentTypeOverloadedSetter.java
URL: http://svn.apache.org/viewvc/ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithDifferentTypeOverloadedSetter.java?view=auto&rev=520283
==============================================================================
--- ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithDifferentTypeOverloadedSetter.java (added)
+++ ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithDifferentTypeOverloadedSetter.java Mon Mar 19 22:32:03 2007
@@ -0,0 +1,19 @@
+package badbeans;
+
+public class BeanWithDifferentTypeOverloadedSetter {
+
+  private String value;
+
+
+  public String getValue() {
+    return value;
+  }
+
+  public void setValue(Double value) {
+    this.value = value.toString();
+  }
+
+  public void setValue(Integer value) {
+    this.value = value.toString();
+  }
+}

Added: ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithNoGetterOverloadedSetters.java
URL: http://svn.apache.org/viewvc/ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithNoGetterOverloadedSetters.java?view=auto&rev=520283
==============================================================================
--- ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithNoGetterOverloadedSetters.java (added)
+++ ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithNoGetterOverloadedSetters.java Mon Mar 19 22:32:03 2007
@@ -0,0 +1,16 @@
+package badbeans;
+
+public class BeanWithNoGetterOverloadedSetters {
+
+  private String value;
+
+
+  public void setValue(String value) {
+    this.value = value;
+  }
+
+  public void setValue(Integer value) {
+    this.value = value.toString();
+  }
+
+}

Added: ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithOverloadedSetter.java
URL: http://svn.apache.org/viewvc/ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithOverloadedSetter.java?view=auto&rev=520283
==============================================================================
--- ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithOverloadedSetter.java (added)
+++ ibatis/trunk/java/mapper/mapper2/test/badbeans/BeanWithOverloadedSetter.java Mon Mar 19 22:32:03 2007
@@ -0,0 +1,19 @@
+package badbeans;
+
+public class BeanWithOverloadedSetter {
+
+  private String value;
+
+
+  public String getValue() {
+    return value;
+  }
+
+  public void setValue(String value) {
+    this.value = value;
+  }
+
+  public void setValue(Integer value) {
+    this.value = value.toString();
+  }
+}

Added: ibatis/trunk/java/mapper/mapper2/test/badbeans/GoodBean.java
URL: http://svn.apache.org/viewvc/ibatis/trunk/java/mapper/mapper2/test/badbeans/GoodBean.java?view=auto&rev=520283
==============================================================================
--- ibatis/trunk/java/mapper/mapper2/test/badbeans/GoodBean.java (added)
+++ ibatis/trunk/java/mapper/mapper2/test/badbeans/GoodBean.java Mon Mar 19 22:32:03 2007
@@ -0,0 +1,15 @@
+package badbeans;
+
+public class GoodBean {
+
+  private String value;
+
+
+  public String getValue() {
+    return value;
+  }
+
+  public void setValue(String value) {
+    this.value = value;
+  }
+}

Added: ibatis/trunk/java/mapper/mapper2/test/com/ibatis/common/beans/BadBeanTest.java
URL: http://svn.apache.org/viewvc/ibatis/trunk/java/mapper/mapper2/test/com/ibatis/common/beans/BadBeanTest.java?view=auto&rev=520283
==============================================================================
--- ibatis/trunk/java/mapper/mapper2/test/com/ibatis/common/beans/BadBeanTest.java (added)
+++ ibatis/trunk/java/mapper/mapper2/test/com/ibatis/common/beans/BadBeanTest.java Mon Mar 19 22:32:03 2007
@@ -0,0 +1,70 @@
+package com.ibatis.common.beans;
+
+import junit.framework.TestCase;
+import badbeans.*;
+
+public class BadBeanTest extends TestCase {
+
+  private static final String PROPNAME = "value";
+  private static final Object STRING_VALUE = "1";
+  private static final Object INT_VALUE = new Integer(1);
+  private static final Object[] NO_VALUE = new Object[]{};
+  private static final Object[] STRING_PARAMS = new Object[]{STRING_VALUE};
+  private static final Object[] INT_PARAMS = new Object[]{INT_VALUE};
+
+  public void testShouldSuccessfullyGetAndSetValueOnGoodBean() throws Exception {
+    GoodBean bean = new GoodBean();
+    ClassInfo info = ClassInfo.getInstance(GoodBean.class);
+    info.getSetter(PROPNAME).invoke(bean, STRING_PARAMS);
+    assertEquals(STRING_VALUE, info.getGetter(PROPNAME).invoke(bean, NO_VALUE));
+    assertEquals(String.class, info.getSetterType(PROPNAME));
+    assertEquals(String.class, info.getGetterType(PROPNAME));
+  }
+
+  public void testShouldSuccessfullyGetAndSetValueOnBeanWithDifferentTypeGetterSetter() throws Exception {
+    BeanWithDifferentTypeGetterSetter bean = new BeanWithDifferentTypeGetterSetter();
+    ClassInfo info = ClassInfo.getInstance(BeanWithDifferentTypeGetterSetter.class);
+    info.getSetter(PROPNAME).invoke(bean, INT_PARAMS);
+    assertEquals(INT_VALUE.toString(), info.getGetter(PROPNAME).invoke(bean, NO_VALUE));
+    assertEquals(Integer.class, info.getSetterType(PROPNAME));
+    assertEquals(String.class, info.getGetterType(PROPNAME));
+  }
+
+  public void testShouldSuccessfullyGetAndSetValueOnBeanWithOverloadedSetter() throws Exception {
+    BeanWithOverloadedSetter bean = new BeanWithOverloadedSetter();
+    ClassInfo info = ClassInfo.getInstance(BeanWithOverloadedSetter.class);
+    info.getSetter(PROPNAME).invoke(bean, STRING_PARAMS);
+    assertEquals(STRING_VALUE, info.getGetter(PROPNAME).invoke(bean, NO_VALUE));
+    assertEquals(String.class, info.getSetterType(PROPNAME));
+    assertEquals(String.class, info.getGetterType(PROPNAME));
+  }
+
+  public void testShouldFailInitializingClassInfoForBeanWithNoGetterOverloadedSetter() {
+    try {
+      try {
+        ClassInfo.getInstance(BeanWithNoGetterOverloadedSetters.class);
+      } catch (Exception e) {
+        assertTrue(e.getMessage().indexOf("Illegal overloaded setter method with ambiguous type for property") == 0);
+        throw e;
+      }
+      fail("Expected exception.");
+    } catch (Exception e) {
+      //ignore
+    }
+  }
+
+  public void testShouldFailInitializingClassInfoForBeanWithDifferentTypeOverloadedSetter() {
+    try {
+      try {
+        ClassInfo.getInstance(BeanWithDifferentTypeOverloadedSetter.class);
+      } catch (Exception e) {
+        assertTrue(e.getMessage().indexOf("Illegal overloaded setter method with ambiguous type for property") == 0);
+        throw e;
+      }
+      fail("Expected exception.");
+    } catch (Exception e) {
+      //ignore
+    }
+  }
+
+}