Re: [1.2.5] Error during cascading saves when adding new entity to existing entity's cascaded persistent collection

[Marcel Klemenz]

I have not the time to read the hole post but I know there was a problem in cascaded saving. You should save the child first.

Am Donnerstag, 20. September 2012 21:10:40 UTC+2 schrieb Julian:

I have filled out a bug report for this problem but just in case someone more clever than I can spot something I'm doing wrong I'll repost it here.

Say I have two entities called Parent and Child. Parent has a persistent collection of children that is marked to cascade all operations.

@Entity
public class Parent extends Model {
    @OneToMany(mappedBy="parent", cascade=CascadeType.ALL)
    public List<Child> children = new ArrayList<Child>();
}

@Entity
public class Child extends Model {
    @ManyToOne
    public Parent parent;
}

If I look up an existing parent and add a new child to it...

Parent parent = Parent.findById(id);

Child child = new Child();
child.parent = parent;
parent.children.add(child)

and then save the parent...

parent.save()

This causes an UnexpectedOperationException to be thrown in JPABase.saveAndCascade(). The cause is a TransientObjectException down the stack in hibernate library code.

play.exceptions.UnexpectedException: During cascading save()
    at play.db.jpa.JPABase.saveAndCascade(JPABase.java:173)
    at play.db.jpa.JPABase._save(JPABase.java:36)
    at play.db.jpa.GenericModel.save(GenericModel.java:204)
        [etc]...
Caused by: org.hibernate.TransientObjectException: object references an unsaved transient instance - save the transient instance before flushing: Child
    at org.hibernate.engine.ForeignKeys.getEntityIdentifierIfNotUnsaved(ForeignKeys.java:243)
    at org.hibernate.collection.AbstractPersistentCollection.getOrphans(AbstractPersistentCollection.java:930)
    at org.hibernate.collection.PersistentBag.getOrphans(PersistentBag.java:143)
    at org.hibernate.engine.CollectionEntry.getOrphans(CollectionEntry.java:373)
    at play.db.jpa.JPABase.cascadeOrphans(JPABase.java:187)
    at play.db.jpa.JPABase.saveAndCascade(JPABase.java:152)
    ... 57 more

This isn't a problem if the Parent itself is new or was initially persisted in the current transaction. Explicitly calling save on the child first also avoid the problem but that defeats the purpose of cascading. I would normally expect to be able to persist a new Child entity in a cascaded relationship by calling save on the Parent. This problem doesn't seem to affect version 1.2.4 of the framework.

I've attached a simple test class that replicates the problem.

[Fabio Falci Rodrigues]

Take a look at this link:
http://pareis.com/2011/04/26/beware-of-this-play-frameworks-cascaded-save-only-works-on-loaded-collections/

Isn't your situation?

> --
> You received this message because you are subscribed to the Google Groups
> "play-framework" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/play-framework/-/9Es4lGjuj6wJ.
>
> To post to this group, send email to play-fr...@googlegroups.com.
> To unsubscribe from this group, send email to
> play-framewor...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/play-framework?hl=en.

[John]

To fill in some detail about the underlying hibernate behavior.

Parent parent = Parent.findById(1L);     //loads the object.  Relationship properties are PersistentBag's under the hood.

parent.children.add(new Child());           //This calls PersistentBag.add().  Child() will be queued to be added, not actually added

parent.children.size();                           //this or any other read operation, triggers AbstractPersistentCollection.initialize().  

                                                         //Initialization incorrectly(?) adds the queued items to the collection's snapshot (Loader::endLoadingCollections() calls endRead() before postInitialize()... endRead() 

                                                        //performs the queued operations.  postInitialize() makes the snapshot.

parent.save();                                    // save calls getOrphans() on the collection, which cannot handle Transient entries in the snapshot.   An exception is thrown.


Here's what PersistentBag::add() looks like:

/**

* @see java.util.Collection#add(Object)

*/

public boolean add(Object object) {

if ( !isOperationQueueEnabled() ) {

write();

return bag.add(object);

}

else {

queueOperation( new SimpleAdd(object) );

return true;

}

}

For some reason the code goes down the else fork and queues the add without initializing.  (write() would trigger initialization).  If this is the correct behavior, then flushing the queue before taking the CollectionEntry snapshot seems wrong.   Alternately if flushing before snapshoting is right, it seems like allowing these transient entries to be queued is wrong.

Note:  If you're trying to replicate the issue in an IDE, be careful about watches and whanot.  I found that in InteilliJ IDEA, the default watches would actually trigger initialization.  (For example if I added a break point in PresistentBag, the automatic watches would trigger read()'s)

-John

On Friday, September 21, 2012 9:24:52 AM UTC-7, Julian wrote:

It's semi-relevant, but not quite the same thing. The linked example deals with a set of 3 model objects.

Item has 1=>N Positions with CascadeType.ALL

Position has 1=>N Orders with CascadeType.ALL

The example has the following steps:

• Load an item, whose positions collections is lazy-loaded and not yet initialized.
  
• Load a single order whose position's item is the original item. This is done without initializing the item instance's positions collection
• Starting from the order, navigate to the loaded position and update a value
• Save the item. Because the positions collection is still not initialized (it hasn't been touched and thus lazy-loading has not been done on it) the save will not cascade to the position, even though it is part of the relationship.

The end result of this is essentially unexpected behavior (changes are not persisted to the Position), but not an exception. Furthermore, this example applies to play version 1.2.4 whereas the exception in my example only occurs in 1.2.5.

The problem does seem to have a bit to do with collection initialization though. I have since discovered that if the collection is fully initialized BEFORE the child is added to it, the exception does not occur. (One way of doing this being to call .size() on the collection as in the example).

The weird thing is that I would expect adding an object to the collection to initialize it. See here for a discussion on that behavior (the discussion seems to apply to the version of hibernate in use by play 1.2.4 and 1.2.5): 

http://stackoverflow.com/questions/5256537/add-and-remove-operations-on-lazily-initialized-collection-behaviour#answer-5256976 

A colleague found out that hibernate's persistent bag implementation contains a snapshot of the bag's contents that is set when the collection is initialized. The purpose of the snapshot is to detect orphans, e.g. whether the bag contained an item when it was initialized and no longer does when it is being persisted. The problem seems to be that the calling .add() doesn't initialize the bag, so when it is initialized later, the snapshot contains the transient object, which causes an exception during the orphan detection process. We haven't figured out of this is expected behavior, a bug of hibernate, or a bug of play itself (or its patched version of hibernate).

[John]

Additional note:

The problem only occurs if you're adding a transient entity to the inverse side of a bidirectional relationship.  It does not happen if the relationship is unidirectional.  (Because with a unidirectional relationship PersistentBag::add() performs the write immediately rather than queuing it.)

So:

1. Add transient entity to the inverse side of a bidirectional entity.  (Doesn't matter whether you set owning side.)

2. Call a read operation on the collection: children.size() or whatever

3. Try to .save().   Result will be a Hibernate exception. 

-John