Initializing a bag after queued add causes incorrect behavior

Description

I haven't been able to reproduce the complete behavior in unit tests, but hopefully this will be enough to find the cause.

The sequence of actions is this:
1. Load an entity with a lazy one-to-many bag
2. Add a new (unsaved) object to the yet-uninitialized bag. The add is queued for later.
3. Initialize the bag with NHibernateUtil.Initialize()

At this point, the collection creates a snapshot of its contents, and the snapshot contains the unsaved object from step 2 - which I believe is wrong.

A consequence of this is that if I –
4. Remove the newly added unsaved object from collection
5. Flush the session

– I get an exception saying "object references an unsaved transient instance - save the transient instance before flushing or set cascade action for the property to something that would make it autosave." The exception is thrown by NHibernate.Engine.ForeignKeys.GetEntityIdentifierIfNotUnsaved(), and the reason why this happens is that the AbstractPersistentCollection.GetOrphans() method receives this unsaved object in the oldElements collection, which I believe should never happen (oldElements should contain the state present in the database?) This was caused by the incorrect snapshot in the collection in step 3.

I was able to reproduce only the incorrect snapshot in the unit test. The later exception appears in my application but I cannot figure out which combination of mappings triggers it, probably a multiple cascade of delete-orphan relations. In any case, when I reverse the order of steps 2 and 3 - that is, initialize the collection before adding the object - the problem disappears.

I suppose the solution would be to somehow modify the code to create the collection snapshot before the queued actions are replayed, but I have no idea if this is correct or how it could be properly done.

There are two files attached that reproduce the incorrect snapshot. It's a modified version of the existing generic bag test, include it in NHibernate.Test/GenericTest/BagGeneric and run the supplied method.

Environment: .Net framework 3.5 SP1, SQL Server 2005 dialect running on a SQL Server 2008.

As a side note, it seems to me that the exception in the GetEntityIdentifierIfNotUnsaved() method is a bit misplaced, I'm mentioning it here because this is a good illustration why: the method doesn't really know the reason for the error, the message is just a guess. The problem here is not that something needs to be saved or cascaded but that the collection state is corrupt. I think it would be better if the error is handled in a caller method (possibly by catching this exception and providing a better message). This part is related to - I can create a separate issue for this if appropriate.

Environment

None

Assignee

Unassigned

Reporter

Boris Drajer

Labels

None

Components

Affects versions

Priority

Major
Configure