Issue Details (XML | Word | Printable)

Key: CASTOR-2592
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: Werner Guttmann
Reporter: No Matter
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
castor

Unmarshalling fails for constructor with primitive type optional arguments - null-s are assigned as values for missing primitive type arguments.

Created: 09/Dec/08 03:54 AM   Updated: 24/Jan/09 11:12 AM   Resolved: 21/Jan/09 02:47 PM
Return to search
Component/s: XML
Affects Version/s: 1.2
Fix Version/s: 1.3

Time Tracking:
Original Estimate: Not Specified
Remaining Estimate: 0 minutes
Remaining Estimate - 0 minutes
Time Spent: 1 hour, 45 minutes
Time Spent - 1 hour, 45 minutes


 Description  « Hide

When set-method maps a primitive type field (type="integer" for example) into the constructor argument of this primitive type and this field is optional and missed in XML file which is unmarshalling - the null is passed as value for a constructor's primitive type parameter. That results in IllegalArgumentException.

Really the problem is in UnmarshalHandler.java, line: 3207:

...
                if ((valueType != null) && (valueType.equals(QNAME_NAME))) {
                        value = resolveNamespace(value);
                }
                args.values[argIndex] = value;
            }
            else {
                args.values[argIndex] = null;             <-------- Here
            }

I think here should be the same or similar processing as a little above:

if (isPrimitive(args.types[argIndex])) {
                    value = toPrimitiveObject(args.types[argIndex], (String)value, descriptor);
                } else {
                    // check whether we are looking at an enum-style object, and if so,
                    // convert the (string) value
                    value = convertToEnumObject(descriptor, value);
                }

but use null or "" instead of value.
So the right default values (0 - for example for int) should be used for constructor parameters of primitive types.



Werner Guttmann added a comment - 12/Dec/08 03:22 PM

Can you please attach a (junit) test case so that I can easily replay the problem at hand ?


Werner Guttmann added a comment - 12/Dec/08 03:38 PM

Possible patch, but I'd still need a test case first.

Index: xml/src/main/java/org/exolab/castor/xml/UnmarshalHandler.java
===================================================================
--- xml/src/main/java/org/exolab/castor/xml/UnmarshalHandler.java	(revision 7993)
+++ xml/src/main/java/org/exolab/castor/xml/UnmarshalHandler.java	(working copy)
@@ -3149,20 +3149,21 @@
 
     /**
      * Processes the given attribute set, and creates the
-     * constructor arguments
+     * constructor arguments.
      *
      * @param atts the AttributeSet to process
      * @param classDesc the XMLClassDescriptor of the objec
      * @return the array of constructor argument values.
+     * @throws SAXException If there's a problem creating the constructor argument set. 
      */
     private Arguments processConstructorArgs
-        (AttributeSet atts, XMLClassDescriptor classDesc)
-        throws org.xml.sax.SAXException
-    {
+        (final AttributeSet atts, final XMLClassDescriptor classDesc)
+        throws SAXException {
         
-        if (classDesc == null) return new Arguments();
+        if (classDesc == null) {
+            return new Arguments();
+        }
 
-
         //-- Loop through Attribute Descriptors and build
         //-- the argument array
 
@@ -3170,27 +3171,35 @@
         //-- un-yet unmarshalled object, we cannot handle
         //-- references as constructor arguments. 
         //-- kvisco - 20030421
+        int count = 0;
         XMLFieldDescriptor[] descriptors = classDesc.getAttributeDescriptors();
-        int count = 0;
-        for (int i = 0; i < descriptors.length; i++) {
-            XMLFieldDescriptor descriptor = descriptors[i];
-            if (descriptor == null) continue;
-            if (descriptor.isConstructorArgument()) ++count;
+        for (XMLFieldDescriptor fieldDescriptor : descriptors) {
+            if (fieldDescriptor == null) {
+                continue;
+            }
+            if (fieldDescriptor.isConstructorArgument()) {
+                ++count;
+            }
         }
         
         Arguments args = new Arguments();
         
-        if (count == 0) return args;
+        if (count == 0) {
+            return args;
+        }
         
         args.values = new Object[count];
         args.types  = new Class[count];
         
-        for (int i = 0; i < descriptors.length; i++) {
-
-            XMLFieldDescriptor descriptor = descriptors[i];
-            if (descriptor == null) continue;
-            if (!descriptor.isConstructorArgument()) continue;
+        for (XMLFieldDescriptor descriptor : descriptors) {
             
+            if (descriptor == null) {
+                continue;
+            }
+            if (!descriptor.isConstructorArgument()) {
+                continue;
+            }
+            
             int argIndex = descriptor.getConstructorArgumentIndex();
             if (argIndex >= count) {
                 String err = "argument index out of bounds: " + argIndex;
@@ -3198,7 +3207,7 @@
             }
 
             args.types[argIndex] = descriptor.getFieldType();
-            String name      = descriptor.getXMLName();
+            String name = descriptor.getXMLName();
             String namespace = descriptor.getNameSpaceURI();
 
             int index = atts.getIndex(name, namespace);
@@ -3208,7 +3217,7 @@
                 //-- check for proper type and do type
                 //-- conversion
                 if (isPrimitive(args.types[argIndex])) {
-                    value = toPrimitiveObject(args.types[argIndex], (String)value, descriptor);
+                    value = toPrimitiveObject(args.types[argIndex], (String) value, descriptor);
                 } else {
                     // check whether we are looking at an enum-style object, and if so,
                     // convert the (string) value
@@ -3222,17 +3231,22 @@
                         value = resolveNamespace(value);
                 }
                 args.values[argIndex] = value;
+            } else {
+                if (isPrimitive(args.types[argIndex])) {
+                    args.values[argIndex] = 
+                        toPrimitiveObject(args.types[argIndex], null, descriptor);
+                } else {
+                    args.values[argIndex] = null;
+                }
             }
-            else {
-                args.values[argIndex] = null;
-            }
         }
         return args;
-    } //-- processConstructorArgs
+    }
 
     /**
      * Checks whether the actual value passed in should be converted to an enum-style class
-     * instance
+     * instance.
+     * 
      * @param descriptor The {@link XMLFieldDescriptor} instance in question.
      * @param value The actual value (which might need conversion).
      * @return The value, potentially converted to an enum-style class.
@@ -3663,24 +3677,24 @@
     }
 
     /**
-     * Converts a String to the given primitive object type
+     * Converts a String to the given primitive object type.
      *
      * @param type the class type of the primitive in which
      * to convert the String to
      * @param value the String to convert to a primitive
+     * @param fieldDesc Descriptor for the given field (value)
      * @return the new primitive Object
+     * @exception SAXException If the String cannot be converted to a primitive object type
      */
     private Object toPrimitiveObject
-        (Class type, String value, XMLFieldDescriptor fieldDesc) 
-        throws SAXException
-    {
+        (final Class type, final String value, final XMLFieldDescriptor fieldDesc) 
+        throws SAXException {
         try {
             return toPrimitiveObject(type, value);
-        }
-        catch(Exception ex) {
+        } catch (Exception ex) {
             String err = "The following error occured while trying to ";
             err += "unmarshal field " + fieldDesc.getFieldName();
-            UnmarshalState state = (UnmarshalState)_stateInfo.peek();
+            UnmarshalState state = (UnmarshalState) _stateInfo.peek();
             if (state != null) {
                 if (state.object != null) {
                     err += " of class " + state.object.getClass().getName();
@@ -3689,32 +3703,31 @@
             err += "\n";
             err += ex.getMessage();
             
-            
             throw new SAXException(err, ex);
         }
-    } //-- toPrimitiveObject
+    }
 
 
     /**
-     * Converts a String to the given primitive object type
+     * Converts a String to the given primitive object type.
      *
      * @param type the class type of the primitive in which
      * to convert the String to
      * @param value the String to convert to a primitive
      * @return the new primitive Object
      */
-    public static Object toPrimitiveObject(Class type, String value) {
+    public static Object toPrimitiveObject(final Class type, String value) {
 
         Object primitive = value;
 
         if (value != null) {
             //-- trim any numeric values
-            if ((type != Character.TYPE) && (type != Character.class))
+            if ((type != Character.TYPE) && (type != Character.class)) {
                 value = value.trim();
+            }
         }
         
         boolean isNull = ((value == null) || (value.length() == 0));
-
         
         //-- I tried to order these in the order in which
         //-- (I think) types are used more frequently

Please note that the patch improves code quality as well (checkstyle, etc.) in the areas where I touched things.