persistence@glassfish.java.net

Re: [Fwd: [Issue 1021] OneToMany Cascade Persist Failure due to Insert Order]

From: Markus Fuchs <Markus.Fuchs_at_Sun.COM>
Date: Mon, 16 Oct 2006 11:33:11 -0700
Alright. I'll run the tests again and check in, ok?

Thanks!

-- markus.

Gordon Yorke wrote:
No Markus,
   There is no need to add it to the interface.
--Gordon
 
-----Original Message-----
From: Markus.Fuchs@Sun.COM [mailto:Markus.Fuchs@Sun.COM]On Behalf Of Markus Fuchs
Sent: Monday, October 16, 2006 2:00 PM
To: persistence@glassfish.dev.java.net
Subject: Re: [Fwd: [Issue 1021] OneToMany Cascade Persist Failure due to Insert Order]

Hi Gordon,

I've added a new method to ObjectChangeSet:

public Class getClassType()

This method is used to get the class type from the tofind ObjectChangeSet. Shall I add this method to the ObjectChangeSet interface?

Thanks!

-- markus.

Gordon Yorke wrote:
Hello Markus,
    There is no reason not to use the ClassType from the toFind ObjectChangeSet.  The Clone is being used from that ObjectChangeSet so the ClassType can be used as well.
--Gordon
-----Original Message-----
From: Markus.Fuchs@Sun.COM [mailto:Markus.Fuchs@Sun.COM]On Behalf Of Markus Fuchs
Sent: Friday, October 13, 2006 5:15 PM
To: persistence@glassfish.dev.java.net
Subject: Re: [Fwd: [Issue 1021] OneToMany Cascade Persist Failure due to Insert Order]

Hi Gordon,

    Your code looks great.  Looking at the code I see there is really no need to reload the class.  The code uses the clone from the toFind changeset so we can continue to use the class from that changeset as well.
 
    public ObjectChangeSet findOrIntegrateObjectChangeSet(ObjectChangeSet tofind, UnitOfWorkChangeSet mergeFromChangeSet) {
        if (tofind == null) {
            return tofind;
        }
        ObjectChangeSet localChangeSet = this.findObjectChangeSet(tofind, mergeFromChangeSet);
        if (localChangeSet == null) {//not found locally then replace it with the one from the merging changeset
            localChangeSet = new ObjectChangeSet(tofind.getPrimaryKeys(), toFind.getClassType(), tofind.getUnitOfWorkClone(), this, tofind.isNew());
            this.addObjectChangeSetForIdentity(localChangeSet, localChangeSet.getUnitOfWorkClone());
        }
        return localChangeSet;
    }
The only way for clients to access the classType field is ObjectChangeSet.getClassType(Session session). This method takes a Session parameter, which is not available at this time. getClassType converts the className into the Class object, if classType == null. I've changed the implementation not to reload the class for the constructor, and only set the className here. The disadvantage is that the classType field will be null until ObjectChangeSet.getClassType(Session session) is called.

What do you think?

Thanks,

-- markus.
--Gordon
-----Original Message-----
From: Markus.Fuchs@Sun.COM [mailto:Markus.Fuchs@Sun.COM]On Behalf Of Markus Fuchs
Sent: Thursday, October 12, 2006 6:26 PM
To: persistence@glassfish.dev.java.net
Subject: Re: [Fwd: [Issue 1021] OneToMany Cascade Persist Failure due to Insert Order]

Hi Gordon,

Please find my new version of the fix for issue 1021
attached. Following is a summary of the changes:

ObjectBuilder: passing the class type to the ObjectChangeSet constructor

AggregateObjectChangeSet: added class type parameter to the constructor

ObjectChangeSet: added class type parameter to the constructor

UnitOfWorkChangeSet: determinating the class type by using the
ConvertionManager, if the tofind clone was loaded with the current
class loader; *Question*: What to do if tofind clone was laoded with
different class loader? - Please see inside class.

SuperLargeProject: added non-entity subclass for testing purpose

EntityManagerJUnitTestSuite: the test associates an SuperLargeProject
to an Employee instance, which is then persisted. Since saving the
Employee also creates a join table entry for the relationship, the
test would fail, if SuperLargeProject wasn't stored as Project.

Thanks again for your previous suggestions and please let me know your
opinion on the question about the UnitOfWorkChangeSet implementation!

-- markus.

Gordon Yorke wrote:
Hello Markus,
    First answer to your second question, Aggregate Collection objects use the regular ChangeSet they do not need the special AggregateChangeSet.
    Second, Passing the class type from the descriptor would be best for the ObjectBuilder.  For the UnitOfWorkChangeSet.findOrIntegrateObjectChangeSet, You can use the className from the "toFind" ObjectChangeSet, which you can assume is correct.
--Gordon
-----Original Message-----
From: Markus.Fuchs@Sun.COM [mailto:Markus.Fuchs@Sun.COM]On Behalf Of Markus Fuchs
Sent: Monday, October 09, 2006 7:02 PM
To: Gordon Yorke; persistence@glassfish.dev.java.net
Subject: Re: [Fwd: [Issue 1021] OneToMany Cascade Persist Failure due to Insert Order]

Hi Gordon,
     Perhaps a more effecient way of handling this issue is to have the ObjectBuilder set the Class type in the ObjectChangeSet from the Descriptor in the ObjectBuilder.  Then there would be no need to perform a Descriptor lookup later in UnitOfWorkChangeSet.
Good idea! That would either involve

* passing the classType to the constructor, or
* not setting the classType and className in the constructor and setting it from outside.

The two methods creating ObjectChangeSets are

ObjectBuilder.createObjectChangeSet
UnitOfWorkChangeSet.findOrIntegrateObjectChangeSet

Looks like the descriptor is not available in the UnitOfWorkChangeSet.findOrIntegrateObjectChangeSet call...

Another question:

in ObjectBuilder.createObjectChangeSet

            if (getDescriptor().isAggregateDescriptor()) {
                changes = new AggregateObjectChangeSet(new Vector(0), clone, uowChangeSet, isNew);
            } else {
                changes = new ObjectChangeSet(extractPrimaryKeyFromObject(clone, session), clone, uowChangeSet, isNew);
            }
            changes.setIsAggregate(getDescriptor().isAggregateDescriptor() || getDescriptor().isAggregateCollectionDescriptor());

meaning that a "regular" ObjectChangeSet is created, if getDescriptor().isAggregateCollectionDescriptor() is true. Is that intended?

Thanks,

-- markus.
--Gordon
-----Original Message-----
From: Markus.Fuchs@Sun.COM [mailto:Markus.Fuchs@Sun.COM]On Behalf Of Markus Fuchs
Sent: Wednesday, October 04, 2006 2:48 PM
To: Gordon Yorke
Subject: Re: [Fwd: [Issue 1021] OneToMany Cascade Persist Failure due to Insert Order]

Hi Gordon,

did you get a chance to look at this?

Thanks!

-- markus.

Markus Fuchs wrote:
Hi Gordon,

could you please give me your opinion?

Thanks!

-- markus.



Subject:
[Issue 1021] OneToMany Cascade Persist Failure due to Insert Order
From:
mf125085@dev.java.net
Date:
Mon, 02 Oct 2006 19:56:41 +0000
To:
issues@glassfish.dev.java.net
To:
issues@glassfish.dev.java.net

https://glassfish.dev.java.net/issues/show_bug.cgi?id=1021






------- Additional comments from mf125085@dev.java.net Mon Oct  2 19:56:41 +0000 2006 -------
I'm attaching a possible fix for issue 1021. The fix basically
registers and removes new instances for non-entity classes in the
Hashtable of the closest entity super class. During flush, the
instances are written to the database from there.

I think that we have to do this twist only for new instance, since
TopLink will never return non-entity instances for any entity manager
operation or queries.

What do you think?


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@glassfish.dev.java.net
For additional commands, e-mail: issues-help@glassfish.dev.java.net