[OJB-developers] Serious flaws with Collections of interfaces + new MtoNQueries

[Georg S. <ge...@me...>]

Hi Thomas and Jacob (and all others :-) ),

I think the following two issues are extremely important and should be
fixed as soon as possible. Since it might take some time to explain them
in detail I will first give a short introduction to both issues:

1.) Collections of Interfaces are not working: Currently there is no
testcase which checks this. If you look at the mapping for
test.ojb.broker.ProductGroup it contains a collection-descriptor with an
element-class-ref="test.ojb.broker.Article" which is wrong because if
you look at the ProductGroup class, it takes objects of interface
InterfaceArticle so the line should actually be

element-class-ref="test.ojb.broker.InterfaceArticle"

Ones you do that the whole junit testsuite blows up. I'll explain the
details and a possible fix further down.

2.) M:N relationships aren't working as they should. IMHO an M:N
relationship means that an object can be part of various "different"
collections. This was possible up to release 0.9 but has now been
changed. Let me be more specific. Let's say we have a class Article, a
Class ShoppingCart which contains a collection of Articles and another
class Inventory which also contains a collection of Articles. I'll
provide a TestCase for this + an explanation in my next mail.



ad 1.)
Please contact me if anything I write seems to be confusing or doesn't
seem clear (my thoughts might get carried away :-) )


The basic problem somewhat converges on one function in
ObjectReferenceDescriptor

public FieldDescriptor[] getForeignKeyFieldDescriptors(ClassDescriptor
mif)

this function is called on various occassions to obtain an array of
FieldDescriptors that contain the fields pointing to the 1 part of a 1:n
relationship.

The first problem is that, when the collection items are of an
"Interface" type (i.e. mif describes an interface) there are no
FieldDescriptors and a NullPointerException is thrown further down when
calling

..mif.getFieldDescriptorByIndex(...

so what we need is a concrete class which does have the fields. The
easiest way is to put the following code at the beginning of the
function:

	if (mif.isInterface()) {
	Vector extentClasses = mif.getExtentClasses();
	Class itemClass = (Class) extentClasses.get(0);
	mif = mif.getRepository().getDescriptorFor(itemClass);
	}

This simply takes the first concrete class and uses their fields.

Unfortunately this is not the end of the story. The problem is that with
interfaces (as opposed to base classes) the "Fields" of one concrete
class can not be used to set the foreign-key "Fields" of another class
implementing the same Interface.

This is a rather subtle problem and it becomes easier to understand when
you look at the set(..) method in PersistentFieldDefaultImpl. There it
says:

            try
            {
                f.set(obj, value);
            }
            catch (IllegalArgumentException ex)

f would be the the field obtained via the getForeignKeyFieldDescriptors
method mentioned above, obj is the concrete object for which the field
is set (the fk-field of the n-side) , and value is the value it is set
to (the primary key value of the 1-side).

As long as the item-class of the collection is a concrete class or a
base-class, but not an interface everything is fine. Java lets you use
the "Field"-objects of a base class, when setting them on a derived
class.
The same doesn't hold true if we are talking of different classes
implementing a common interface. The same "Field"-object obtained for
one class can not be used to set the field in another class also
implementing the interface. The consequence is an
IllegalArgumentException being thrown.

Going back to ObjectReferenceDescriptor the consequence is, that a new
array of FieldDescriptors has to be computed and returned for each
concrete classdescriptor (mif). This means the following if statements
has to be taken out (just the if, not the block itself)

	if (foreignKeyFieldDescriptors == null)
	{
	......
	}

because it computes the array only the first time and returns it
directly on subsequent calls (for performance reasons I suppose). For
performance reasons one could use a Hashtable to store already computed
FieldDescriptor arrays for concrete ClassDescriptors.

Now that we have a function that always returns the right array of
FieldDescriptors we have to make sure that it is also called with the
right class-descriptors.

The one place where that has to be changed is in TransactionImpl

In the method lockCollections(....
the following lines

ClassDescriptor itemCld =
this.getBroker().getClassDescriptor(cds.getItemClass());
FieldDescriptor[] itemFkFields = cds.getForeignKeyFieldDescriptors(itemCld);

are being called before the while (colIterator.hasNext()) loop

They have to be moved into the loop to work correctly:

	      while (colIterator.hasNext())
                {
                    Object item = colIterator.next();
		    ClassDescriptor itemCld =
			this.getBroker().getClassDescriptor(item.getClass());
		    FieldDescriptor[] itemFkFields =
			cds.getForeignKeyFieldDescriptors(itemCld);
	.......


So to come to a conclusion, in order to see the problem and its possible
fix you have to do the following:

1.) change the collection-descriptor in the mapping for ProductGroup to
	element-class-ref="test.ojb.broker.InterfaceArticle"

	this will make you see the problem when running the unit-tests

2.)

	In ObjectReferenceDescriptor
	 in the method getForeignKeyFieldDescriptors(...

	a) take out the if
	b) put
	        if (mif.isInterface()) {
        	Vector extentClasses = mif.getExtentClasses();
        	Class itemClass = (Class) extentClasses.get(0);
        	mif = mif.getRepository().getDescriptorFor(itemClass);
        	}
	at the beginning

3.) in TransactionImpl move the 2 lines into the loop as explained
above.

Cheers

Georg