14 Replies Latest reply on Aug 10, 2007 5:22 AM by alesj

    ClassCastException for char[] MetaValue creation

    starksm64

      This org.jboss.test.metatype.values.factory.test.ArrayValueFactoryUnitTestCase.testCharArray test is failing with the shown CCE. Should simple type arrays be handled by the DefaultMetaValueFactory.createArrayValue?

       public void testCharArray()
       throws Exception
       {
       char[] array = "Hello".toCharArray();
       ArrayMetaType arrayType = assertInstanceOf(resolve(array.getClass()), ArrayMetaType.class);
       MetaValue[] metaArray = { SimpleValueSupport.wrap('H'),
       SimpleValueSupport.wrap('e'),
       SimpleValueSupport.wrap('l'),
       SimpleValueSupport.wrap('l'),
       SimpleValueSupport.wrap('o')
       };
       ArrayValueSupport expected = new ArrayValueSupport(arrayType, metaArray);
      
       MetaValue result = createMetaValue(array);
       ArrayValue actual = assertInstanceOf(result, ArrayValue.class);
       getLog().debug("Array Value: " + actual);
       assertEquals(expected, actual);
       }
      


      java.lang.ClassCastException: [C
       at org.jboss.metatype.plugins.values.DefaultMetaValueFactory.createArrayValue(DefaultMetaValueFactory.java:201)
       at org.jboss.metatype.plugins.values.DefaultMetaValueFactory.internalCreate(DefaultMetaValueFactory.java:411)
       at org.jboss.metatype.plugins.values.DefaultMetaValueFactory.create(DefaultMetaValueFactory.java:344)
       at org.jboss.test.metatype.values.factory.test.AbstractMetaValueFactoryTest.createMetaValue(AbstractMetaValueFactoryTest.java:89)
       at org.jboss.test.metatype.values.factory.test.ArrayValueFactoryUnitTestCase.testCharArray(ArrayValueFactoryUnitTestCase.java:95)
      ...
      


        • 1. Re: ClassCastException for char[] MetaValue creation
          starksm64

          In looking into this the type info subsystem in the container module also looks broken for primitive array types. I added a primitive char[] test and its failing due to char vs Character mismatches:

           public void testCharArray()
           throws Throwable
           {
           char[] array = {'h', 'e', 'l', 'l', 'o'};
           testArray(array);
           }
          
          ...
          
          junit.framework.ComparisonFailure: expected:<...C> but was:<...char>
           at junit.framework.Assert.assertEquals(Assert.java:81)
           at junit.framework.Assert.assertEquals(Assert.java:87)
           at org.jboss.test.classinfo.test.AbstractClassInfoTest.testBasics(AbstractClassInfoTest.java:83)
           at org.jboss.test.classinfo.test.ClassInfoArrayTest.testArray(ClassInfoArrayTest.java:94)
           at org.jboss.test.classinfo.test.ClassInfoArrayTest.testCharArray(ClassInfoArrayTest.java:52)
          


          I also ran across JBMICROCONT-123 which is about improving the implementation, but this is actually a bug as this method does not work at all. The depth argument to Array.newInstance is the array length, not a depth for the array dimension.
           public static Class getArrayClass(Class clazz, int depth)
           {
           return Array.newInstance(clazz, depth).getClass();
           }
          


          This simple test in org.jboss.test.classinfo.test.ClassInfoArrayTest of getArrayClass fails because getArrayClass is returning an array of the input type, not selecting the type of the indicated array dimension:
           public void testArrayType()
           {
           String[] array = {"hello", "world"};
           TypeInfoFactory factory = getTypeInfoFactory();
           TypeInfo info = factory.getTypeInfo(array.getClass());
          
           TypeInfo info0 = info.getArrayType(0);
           assertEquals(info0.getName(), "[java.lang.String;", info0.getName());
           }
          
          ...
          junit.framework.ComparisonFailure: [[Ljava.lang.String; expected:<......> but was:<...[L...>
           at junit.framework.Assert.assertEquals(Assert.java:81)
           at org.jboss.test.classinfo.test.ClassInfoArrayTest.testArrayType(ClassInfoArrayTest.java:61)
          



          • 2. Re: ClassCastException for char[] MetaValue creation
            starksm64

            Based on other tests like org.jboss.test.classinfo.test.AbstractClassInfoTest.testArray:

             protected void testArray(Class<?> clazz, TypeInfo info) throws Throwable
             {
             TypeInfo arrayType = info.getArrayType(1);
             getLog().debug("ArrayType(1): " + arrayType);
             assertTrue(arrayType.isArray());
             Class<?> arrayClass = Array.newInstance(clazz, 1).getClass();
             assertEquals(arrayClass, arrayType.getType());
            
             arrayType = info.getArrayType(5);
             getLog().debug("ArrayType(5): " + arrayType);
             assertTrue(arrayType.isArray());
             arrayClass = Array.newInstance(clazz, 5).getClass();
             assertEquals(arrayClass, arrayType.getType());
             }
            


            the getArrayType is supposed to return a TypeInfo for a TypeInfo that corresponds to TypeInfo[depth]? I thought this was returning the TypeInfo for the given dimension of the array.
             /**
             * Whether this type is an array
             *
             * @param depth the array depth
             * @return the array type
             */
             TypeInfo getArrayType(int depth);
            


            This needs to be clarified.


            • 3. Re: ClassCastException for char[] MetaValue creation
              starksm64

              So the testCharArray failure comes down to this conflict mentioned in the org.jboss.metadata.spi.signature.Signature.stringsToClasses comment

               //For arrays we will want to load the class, the ArrayInfoImpl generates names in an invalid format, resolve this here
               String primitiveCandidate = param.substring(index + 1);
               String componentType = primitiveArrayTypes.get(primitiveCandidate);
              
              


              The same encoded names as found in primitiveArrayTypes should be going into the ArrayInfoImpl name when a primative type is the element type:

               public ArrayInfoImpl(TypeInfo componentType)
               {
               this.componentType = componentType;
               StringBuilder builder = new StringBuilder();
               builder.append("[");
               TypeInfo temp = componentType;
               while (temp.isArray())
               {
               builder.append("[");
               temp = ((ArrayInfo) temp).getComponentType();
               }
               if (temp.getClass().equals(PrimitiveInfo.class))
               {
               //builder.append(temp.getName());
               // Use the signature encoded name for the primitive element type
               String encodedName = Signature.getPrimativeArrayType(temp.getName());
               builder.append(encodedName);
               }
               else
               {
               builder.append("L").append(temp.getName()).append(";");
               }
               name = builder.toString();
               calculateHash();
               }
              


              with that change, the IntrospectionArrayUnitTestCase.testCharArray passes, but the JavassistArrayUnitTestCase.testCharArray fails with the following. The JavassistArrayInfoImpl also needs to be updated. If I do that all container unit tests are passing.

              junit.framework.AssertionFailedError: expected:<JavassistArrayInfoImpl@75843a75{name=[Lchar;}> but was:<ArrayInfoImpl@3c1a1399{name=[C}>
               at junit.framework.Assert.fail(Assert.java:47)
               at junit.framework.Assert.failNotEquals(Assert.java:282)
               at junit.framework.Assert.assertEquals(Assert.java:64)
               at junit.framework.Assert.assertEquals(Assert.java:71)
               at org.jboss.test.classinfo.test.AbstractClassInfoTest.testBasics(AbstractClassInfoTest.java:74)
               at org.jboss.test.classinfo.test.ClassInfoArrayTest.testArray(ClassInfoArrayTest.java:96)
               at org.jboss.test.classinfo.test.ClassInfoArrayTest.testCharArray(ClassInfoArrayTest.java:52)
              



              • 4. Re: ClassCastException for char[] MetaValue creation
                starksm64

                I created JBMICROCONT-202 for these issues and have checked in some changes for these that has the tests passing. There is a real mismatch in the type system for primitive arrays vs the object forms of such arrays. I played around with a few different things to try to better preserve the primitive arrays, but since you can't create generic arrays, the ArrayMetaValue always has to be the Object form (Character[] instead of char[]).

                • 5. Re: ClassCastException for char[] MetaValue creation

                  There was some work done on this for OpenMBeans in Java6
                  http://java.sun.com/javase/6/docs/technotes/guides/jmx/enhancements.html#Open
                  I haven't looked at the details.

                  • 6. Re: ClassCastException for char[] MetaValue creation
                    starksm64

                    Its templatized, and has a couple more methods to support primitive arrays. I'll look at adding these changes.

                    • 7. Re: ClassCastException for char[] MetaValue creation
                      starksm64

                      The ArrayMetaType seems easy enough and is mostly done, but not checked in. Its the ArrayValue that is more problematic since a primitive array (char[]) is not an Object[]. The jmx stuff does not deal with OpenType value representations. The only way I can see to do this is to drop the Object[] accessors and instead go to something more like the java.lang.reflect.Array.set/get style accessors as this would have to be used to deal with the primitive arrays anyway. I'll play with that more tomorrow.

                      • 8. Re: ClassCastException for char[] MetaValue creation
                        alesj

                         

                        "scott.stark@jboss.org" wrote:

                        I also ran across JBMICROCONT-123 which is about improving the implementation, but this is actually a bug as this method does not work at all. The depth argument to Array.newInstance is the array length, not a depth for the array dimension.
                         public static Class getArrayClass(Class clazz, int depth)
                         {
                         return Array.newInstance(clazz, depth).getClass();
                         }
                        



                        What's the deal with this one then?
                        I saw you did an assignment ping-pong. :-)
                        Do you need it fixed right away?
                        How come we didn't see this (The depth argument to Array.newInstance is the array length, not a depth for the array dimension.) before? ;-)

                        • 9. Re: ClassCastException for char[] MetaValue creation
                          starksm64

                          I have the following changes to the ArrayValue interface working and can preserve the primitive type arrays:

                          public interface ArrayValue<T extends Serializable>
                           extends MetaValue, Iterable<T>
                          {
                           ArrayMetaType<T> getMetaType();
                          
                           /**
                           * Get the underlying array value. This will not be an
                           * Object[] in general.
                           * @see #getValue(int)
                           *
                           * @return the underlying value
                           */
                           public Object getValue();
                           /**
                           * Get the length of the array.
                           * @return length of the array.
                           */
                           public int getLength();
                           /**
                           * Get the array element at index.
                           * @param index - index into the array.
                           * @return element at index.
                           */
                           public Object getValue(int index);
                          }
                          


                          As this ArrayValueSupportUnitTestCase test shows, you can use the getValue(int) accessor, the type safe foreach Iterable, or the raw array:
                           public void testCharArray() throws Exception
                           {
                           ArrayMetaType<Character> type = new ArrayMetaType<Character>(1, SimpleMetaType.CHARACTER);
                           char[] value = {'h', 'e', 'l', 'l', 'o'};
                           ArrayValueSupport<Character> avs = new ArrayValueSupport<Character>(type, value);
                           // Use getValue(int) accessor
                           for(int n = 0; n < avs.getLength(); n ++)
                           {
                           Object element = avs.getValue(n);
                           assertEquals(value[n], element);
                           }
                           // Use typesafe foreach Iterable
                           int i = 0;
                           for(Character c : avs)
                           {
                           assertEquals("["+i+"]", (Character) value[i++], c);
                           }
                           // Validate the primative array
                           char[] raw = (char[]) avs.getValue();
                           for(int n = 0; n < value.length; n ++)
                           {
                           assertEquals(value[n], raw[n]);
                           }
                           }
                          


                          There are quirks for multi-dimensional primitive arrays though. This 2d char[][] test shows them:
                           public void test2DCharArray() throws Exception
                           {
                           ArrayMetaType<Character[]> type = new ArrayMetaType<Character[]>(2, SimpleMetaType.CHARACTER, true);
                           char[][] value = {{'h', 'e'}, {'l', 'l', 'o'}};
                           ArrayValueSupport<Character[]> avs = new ArrayValueSupport<Character[]>(type, value);
                           assertEquals(value.length, avs.getLength());
                           for(int m = 0; m < value.length; m ++)
                           {
                           Object arraym = avs.getValue(m);
                           for(int n = 0; n < value[m].length; n ++)
                           {
                           // Have to use the java.lang.reflect.Array to access nested elements
                           Object valuenm = Array.get(arraym, n);
                           assertEquals("["+m+"]["+n+"]", value[m][n], valuenm);
                           }
                           }
                           // Use typesafe foreach Iterable: current broken with CCE on [C
                           int i = 0, j = 0;
                           for(Character[] carray : avs) // CCE here
                           {
                           for(Character c : carray)
                           {
                           Character cij = value[i ++][j ++];
                           assertEquals("["+i+"], ["+j+"]", cij , c);
                           }
                           }
                           // Validate the primitive 2d array
                           char[][] raw = (char[][]) avs.getValue();
                           for(int m = 0; m < value.length; m ++)
                           {
                           for(int n = 0; n < value[m].length; n ++)
                           {
                           assertEquals("["+m+"]["+n+"]", value[m][n], raw[m][n]);
                           }
                           }
                           }
                          


                          The first is that you have to use java.lang.reflect.Array.get(Object, int) to access the elements of the nested arrays obtained via the get(int) accessor.

                          The second is that the Iterable implementation on ArrayValueSupport needs to track whether the underlying value is a primitive array, and covert it to the primitive wrapper form to avoid the CCE.

                          Before going further I want to see if we agree that this is the way to go.


                          • 10. Re: ClassCastException for char[] MetaValue creation
                            starksm64

                             

                            "alesj" wrote:
                            "scott.stark@jboss.org" wrote:

                            I also ran across JBMICROCONT-123 which is about improving the implementation, but this is actually a bug as this method does not work at all. The depth argument to Array.newInstance is the array length, not a depth for the array dimension.
                             public static Class getArrayClass(Class clazz, int depth)
                             {
                             return Array.newInstance(clazz, depth).getClass();
                             }
                            



                            What's the deal with this one then?
                            I saw you did an assignment ping-pong. :-)
                            Do you need it fixed right away?
                            How come we didn't see this (The depth argument to Array.newInstance is the array length, not a depth for the array dimension.) before? ;-)


                            The question is what the getArrayClass(Class, int) method is supposed to do. If its supposed to create a Class[] from Class, then the depth argument is irrelevant, and misleading. There is no such type as Class[int length].

                            If its supposed to create a depth dimensional array of Class, then its broken. Based on the existing tests, it seems to be the former and it should just be:
                             public static Class getArrayClass(Class clazz)
                             {
                             return Array.newInstance(clazz, 0).getClass();
                             }
                            


                            In terms of JBMICROCONT-123, I don't think there is a better way to do this as I see the same logic in the jdk code. So either the blind are leading the blind and that is where we got this construct from, or there is no other way to create an array Class.


                            • 11. Re: ClassCastException for char[] MetaValue creation

                              It sounds ok.

                              I'm not sure I would spend much time on this though.
                              In general the collection interfaces should be preferable to arrays for metadata.

                              Not least because of the current difficultly in intercepting array changes using aop
                              that Bela is always complaining about. ;-)

                              • 12. Re: ClassCastException for char[] MetaValue creation
                                alesj

                                 

                                "adrian@jboss.org" wrote:
                                It sounds ok.

                                This falls to JBMICROCONT-123 or something else?

                                Should I remove depth parameter from the impl?
                                Or at least change it to length to be exact?

                                • 13. Re: ClassCastException for char[] MetaValue creation
                                  starksm64

                                   

                                  "alesj" wrote:
                                  "adrian@jboss.org" wrote:
                                  It sounds ok.

                                  This falls to JBMICROCONT-123 or something else?

                                  Should I remove depth parameter from the impl?
                                  Or at least change it to length to be exact?

                                  Just drop the depth parameter.


                                  • 14. Re: ClassCastException for char[] MetaValue creation
                                    alesj

                                     

                                    "scott.stark@jboss.org" wrote:

                                    Just drop the depth parameter.

                                    Done.
                                    Removed 'depth' from getArrayClass and interface method getArrayType.
                                    Changed tests to use zero instead.