Re: AW: Bugs in 1.0.5rc1

Armin Waibel <[email protected]> Fri, 07 Mar 2008 18:49:41 +0100
Newsgroups gmane.comp.jakarta.ojb.user
Message-ID <[email protected]>
Hi Sascha,

Sascha Broich wrote:
> Hi Armin,
> 
>> The method CollectionTypes#getCollectionClass is correctly
> implemented.
>> First this method checks for user specific collection class
>> implementations (implementations of ManageableCollection), then this
>> method try to resolve the collection class implementations for 1:n and
>> m:n collection-fields (of type Collection, List, Set or array types)
> of
>> persistence capable classes.
>>
>> The "real bug" is a typo in the OJB.properties file:
>> replace
>>
> CollectionTypes.OneToManySet=org.apache.ojb.broker.util.collections.Man
>> ageableSet
>> with
>>
> CollectionTypes.OneToManySet=org.apache.ojb.broker.util.collections.Man
>> ageableHashSet
>>
>> I will fix this ASAP. Thanks again.
> 
> I'm sorry, but the change in the OJB.properties file didn't solve the
> problem.
> I still get the MetadataException "Cannot determine a collection type
> for collection/list/set/array field 'devices' of type 'class
> java.util.HashSet' in persistence capable class 'Abc'"
> 
> As I wrote: HashSet.class.isAssignableFrom(java.util.Set.class) will
> ALWAYS fail.

yep, this is the intended behavior. This method (beside user specific 
ManageableCollection implementations) only allow base collection classes 
(List, Set,...).
The mentioned line is similar to:
Set.class.equals(fieldClass)


> Please remember, the method can only be successful in the direction
> superclass.isAssignableFrom(subclass).
> The only superclass of java.util.Set is java.util.Collection.
> So as long as there is a HashSet instead of a Set used as field the
> check fails, despite the OJB.properties file.
> 
> In 1.0.4 this assignable check was against RemovalAwareSet, which is a
> subclass of HashSet.

yep, you are right. But now the collection types are configurable in 
OJB.properties file e.g.
CollectionTypes.OneToManySet=org.apache.ojb.broker.util.collections.ManageableHashSet
So we can't check against an implementation class.

Currently RC1 automatic resolve only base collection types like 
Collection, List, Vector and Set.
Two workarounds:
1.) If you change field 'devices' to type 'Set' it should work (with 
patched OJB.properties file).
2.) Directly set a collection implementation class in the 
collection-descriptor (you can use a shortcut-name for shipped 
implementations - see documentation):

<collection-descriptor
name="allArticlesInGroup"
element-class-ref="org.apache.ojb.broker.Article"
collection-class="set"
...
</collection-descriptor>


I have to admit that the current behavior isn't perfect ;-), so I will 
improve class CollectionTypes to be backward compatible with 1.0.4:
First check the collection field base type (e.g. subclass of class Set) 
and then make sure that the ManageableCollection implemetation class is 
a subclass of the field collection type.

<snip>
if (ManageableCollection.class.isAssignableFrom(fieldType))
{
     result = fieldType;
}
else if(Set.class.isAssignableFrom(fieldType))
{
     result = cds.isMtoNRelation() ? getManyToManySet() : getOneToManySet();
     if(!fieldType.isAssignableFrom(result))
     {
         failed = true;
     }
}
...
</snip>



> 
>>> Another bug is in the statement creation when a subclass is
> involved.
>>> Note: A0 is the subclass table from A1, PHB_USRID exists only in A1
>>>
>>> 1.0.4 creates something like
>>>
>>> SELECT
>>>         A0.PHB_ID         ,
>>>         A1.PHB_USRID      ,
>>> FROM
>>>         IPT_PHONEBOOKCSV A0
>>>         INNER JOIN IPT_PHONEBOOK A1
>>>         ON
>>>                 A0.PHB_ID = A1.PHB_ID
>>> WHERE
>>>         PHB_USRID IS NULL
>>>
>>>
>>>
>>> 1.0.5rc1 creates
>>>
>>> SELECT
>>>         A0.PHB_ID         ,
>>>         A1.PHB_ID         ,
>>>         A1.PHB_USRID      ,
>>> FROM
>>>         IPT_PHONEBOOKCSV A0
>>>         INNER JOIN IPT_PHONEBOOK A1
>>>         ON
>>>                 A0.PHB_ID = A1.PHB_ID
>>> WHERE
>>>         A0.PHB_USRID IS NULL
>>>
>>>
>>> The WHERE criteria assignment to A0 the statement produces an
>> exception
>>> for the nonexisting column A0.PHB_USRID.
>>>
>>> Note, that in the SELECT of 1.0.5rc1 is also A1.PHB_ID, which is not
>> in
>>> the statement of 1.0.4.
>> If PHB_USRID only exists in A1 then A1 is a subclass of (super) class
>> A0. In this case it's not allowed to build a query with target class
> A0
>> and fields only existing in A1.
>> Is this the case? If not, please post some more details (java pseudo
>> code, class mapping).
> 
> class A1
> {
>   PHB_ID;
>   PHB_USRID;
>   PHB_NAME;
> }
> 
> class A0 extends A1
> {
>   PHB_CSV;
> }
> 
> And the tables are
> Table A1
>   PHB_ID
>   PHB_USRID
>   PHB_NAME
> 
> Table A0
>   PHB_ID
>   PHB_CSV
> 
> Because of the inheritance there shouldn't be a need for the column
> PHB_USRID in the table for the derived class A0.
> 
> The correct statement should be
> 
> SELECT
>   A0.PHB_ID   ,
>   A0.PHB_CSV  ,
>   A1.PHB_USRID,
>   A1.PHB_NAME
> FROM
>   A0 INNER JOIN A1 ON A0.PHB_ID = A1.PHB_ID
> WHERE 
>   A1.PHB_USRID IS NULL
> 
> I'm not sure if the 1.0.4 resolved this correct but it didn't add the
> alias to the where clause so the database assigned the correct column by
> itself.

You are right, this should not happen. In the OJB test-suite we do 
several tests using table-per-subclass inheritance and this never happens.
Could you post the mapping for class A1, A0 and the source of the query 
then I can try to reproduce your issue.

regards,
Armin

> 
> 
> Regards,
> Sascha Broich
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
> 
>