1 2 Previous Next 16 Replies Latest reply on Jul 6, 2007 12:02 PM by Juha Lindfors

    AbstractCollectionMetaData fails with optional add() operati

    Juha Lindfors Master

      I'm mapping a bean with a java.util.List type property where the bean getter returns a read-only view of the list via Collections.unmodifiableList(original).

      org.jboss.beans.metadata.plugins.AbstractCollectionMetaData#getValue fails here with a deployment exception since the method does not ensure the collection instance supports the optional Collection.add() operation (and doesn't fall back to default collection)

      *** DEPLOYMENTS IN ERROR: Name -> Error
      Manager -> java.lang.UnsupportedOperationException

      It seems rather unintuitive that the MC requires the read-method to provide a mutable collection reference and proceeds to make changes to my collection via read-method access?

        • 1. Re: AbstractCollectionMetaData fails with optional add() ope
          Ales Justin Master

          Can you add your XML example?

          But I guess this is a cause of this feature:
          - http://jira.jboss.com/jira/browse/JBMICROCONT-35

          You can do a workaround with 'lazy' getter:

          List getMyList()
          {
           return mylist != null ? Collections.unmodifiableList(mylist) : null;
          }
          

          If that still works for you.

          • 2. Re: AbstractCollectionMetaData fails with optional add() ope
            Juha Lindfors Master

             

            <?xml version="1.0" encoding="UTF-8"?>
            
            <deployment xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
             xsi:schemaLocation="urn:jboss:bean-deployer bean-deployer_2_0.xsd"
             xmlns="urn:jboss:bean-deployer:2.0">
            
            
             <bean name = "Manager" class = "PluginManager">
             <property name = "bootstrapPlugins">
             <list elementClass = "java.lang.String">
             <value>Test</value>
             </list>
             </property>
             </bean>
            
            </deployment>
            


            public class PluginManager
            {
            
             private List bootstrapPlugins = new ArrayList(10);
            
             public void setBootstrapPlugins(List listOfPluginNames)
             {
             this.bootstrapPlugins = listOfPluginNames;
             }
            
             public List getBootstrapPlugins()
             {
             return Collections.unmodifiableList(bootstrapPlugins);
             }
            }
            



            • 3. Re: AbstractCollectionMetaData fails with optional add() ope
              Adrian Brock Master

              There's no way to check whether a collections is modifiable or otherwise.

              But it should certainly be possible to turn this feature off,
              with something like the requiresNew attribute on the collection below.

              i.e. don't use the getter as the source of the collection to modify

              <?xml version="1.0" encoding="UTF-8"?>
              
              <deployment xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
               xsi:schemaLocation="urn:jboss:bean-deployer bean-deployer_2_0.xsd"
               xmlns="urn:jboss:bean-deployer:2.0">
              
              
               <bean name = "Manager" class = "PluginManager">
               <property name = "bootstrapPlugins">
               <list elementClass = "java.lang.String" requiresNew="true">
               <value>Test</value>
               </list>
               </property>
               </bean>
              
              </deployment>
              


              • 4. Re: AbstractCollectionMetaData fails with optional add() ope
                Ales Justin Master

                I added 'preinstantiate' attribute on the property element.
                So you can turn preinstantiate lookup on/off for any property, not just collections.

                Just writing the tests now.

                • 5. Re: AbstractCollectionMetaData fails with optional add() ope
                  Adrian Brock Master

                  In fact, the solution should really be that the type specified on the collection
                  overrides this feature. But currently this is not the case.

                  i.e. the following should always create a new array list not use the getter.

                  <?xml version="1.0" encoding="UTF-8"?>
                  
                  <deployment xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                   xsi:schemaLocation="urn:jboss:bean-deployer bean-deployer_2_0.xsd"
                   xmlns="urn:jboss:bean-deployer:2.0">
                  
                  
                   <bean name = "Manager" class = "PluginManager">
                   <property name = "bootstrapPlugins">
                   <list elementClass = "java.lang.String" class="java.util.ArrayList">
                   <value>Test</value>
                   </list>
                   </property>
                   </bean>
                  
                  </deployment>
                  


                  • 6. Re: AbstractCollectionMetaData fails with optional add() ope
                    Ales Justin Master

                     

                    "adrian@jboss.org" wrote:
                    In fact, the solution should really be that the type specified on the collection
                    overrides this feature. But currently this is not the case.

                    i.e. the following should always create a new array list not use the getter.

                    
                    <?xml version="1.0" encoding="UTF-8"?>
                    
                    <deployment xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                     xsi:schemaLocation="urn:jboss:bean-deployer bean-deployer_2_0.xsd"
                     xmlns="urn:jboss:bean-deployer:2.0">
                    
                    
                     <bean name = "Manager" class = "PluginManager">
                     <property name = "bootstrapPlugins">
                     <list elementClass = "java.lang.String" class="java.util.ArrayList">
                     <value>Test</value>
                     </list>
                     </property>
                     </bean>
                    
                    </deployment>
                    


                    I'll turn the 'preinstantiate' to false by default.
                    So this will then work out-of-the-box.
                    And if you explicitly tell to use preinstantiated, then the class attribute will be ignored in the case getter returns non null instance.

                    • 7. Re: AbstractCollectionMetaData fails with optional add() ope
                      Ales Justin Master

                       

                      "alesj" wrote:

                      I'll turn the 'preinstantiate' to false by default.
                      So this will then work out-of-the-box.
                      And if you explicitly tell to use preinstantiated, then the class attribute will be ignored in the case getter returns non null instance.


                      The only issue is that the behaviour will change.
                      Those who used preinstantiated instances before by default, should now explicitly mark them preinstantiate='true'.

                      I can issue a big fat log warning. :-)

                      • 8. Re: AbstractCollectionMetaData fails with optional add() ope
                        Adrian Brock Master

                        There's no need to start issuing warnings.
                        This stuff has never been released GA.

                        The correct order for this processing is
                        (and should always have been - if the user is explicit, do what they want):

                        1) Any class specified on the list element (like above) - new collection
                        2) Any class specified on the parent element (the typeInfo passed into getValue),
                        e.g.

                         <property name="blah" class="java.util.ArrayList"><list/></property>
                        
                        - new collection
                        3) The getter - use the returned collection (if any)
                        4) the default collection type - new collection

                        The only reason for the preinstantiated flag would be to turn off (3)
                        but you can always do this anyway by using (1) or (2).

                        • 9. Re: AbstractCollectionMetaData fails with optional add() ope
                          Adrian Brock Master

                          Actually (2) is probably not correct in the above, it should come between (3) and (4)

                          It doesn't actually exist for properties
                          (except as a hack/short hand for string value properties - not relevant here)
                          and actually defines which constructor/method to use for parameters
                          so it would always override the preinstantiated which isn't what is wanted
                          and sometimes it will just represent an interface anyway (because that is the parameter type).

                          So, the correct order should be:

                          1) AbstractTypeMetaData.getClassInfo(ClassInfo);
                          2) preinstantiated
                          3) TypeInfo in getValue() parameters
                          4) Default collection type

                          But can still stop the getter being used by adding a class= to the list element.

                          • 10. Re: AbstractCollectionMetaData fails with optional add() ope
                            Ales Justin Master

                            Yes, but this means people have to explicitly write class="java.util.ArrayList" if they don't want to use preinstantiated instance. All they really want is to write (list)...elements(/list).
                            I leave the preinstantiated attribute - false by default.
                            I think it is simple enough.

                            What we really wanted is to have Juha's case resolved. ;-)

                            • 11. Re: AbstractCollectionMetaData fails with optional add() ope
                              Adrian Brock Master

                              Yes, but the whole point of pre-instantiated is to reduce the amount users have
                              to write in xml - not least because it can also become inconsistent if they have
                              to do it two places.

                              Imagine if Juha's class was:

                              public class PluginManager
                              {
                               private List bootstrapPlugins = new CopyOnWriteArrayList(10);
                              
                               public void setBootstrapPlugins(List listOfPluginNames)
                               {
                               this.bootstrapPlugins = listOfPluginNames;
                               }
                              
                               public List getBootstrapPlugins()
                               {
                               return bootstrapPlugins;
                               }
                              }
                              


                              The user doesn't want to have to say "preinstantiated=true" or
                              class="java.util.concurrent.CopyOnWriteArrayList" to make this work
                              how they would expect!

                              It should just work out of the box.

                              It's only in classes like Juha's where you have to turn off preinstantiated
                              because the getter is returning an immutable.
                              Therefore you need to be explicit about what implementation you really want,
                              we can't work it out.

                              We should optimize for the common cases.

                              • 12. Re: AbstractCollectionMetaData fails with optional add() ope
                                Juha Lindfors Master

                                 

                                "adrian@jboss.org" wrote:
                                There's no way to check whether a collections is modifiable or otherwise.


                                True. You could address the scenario by catching the UnsupportedOperationException on Collection.add() and falling back to a default mechanism.

                                It's not a complete solution but depending on how common you think the use case is, would address it without extra burden of configuration on user's part.

                                • 13. Re: AbstractCollectionMetaData fails with optional add() ope
                                  Adrian Brock Master

                                   

                                  "juha@jboss.org" wrote:
                                  "adrian@jboss.org" wrote:
                                  There's no way to check whether a collections is modifiable or otherwise.


                                  True. You could address the scenario by catching the UnsupportedOperationException on Collection.add() and falling back to a default mechanism.

                                  It's not a complete solution but depending on how common you think the use case is, would address it without extra burden of configuration on user's part.


                                  It doesn't quite work the way you describe.
                                  The collection is first instantiated and then the values added.

                                  We could hack it like you describe, but it wouldn't be very clean.
                                  Maybe it is worth the ugliness for the ease of use? :-)

                                  • 14. Re: AbstractCollectionMetaData fails with optional add() ope
                                    Juha Lindfors Master

                                    I'm solidly in the camp of accepting ugliness in implementation for ease of use :-)

                                    1 2 Previous Next