refactorthis/GraphDiff — pull request #37 by andypelzer (2014-01-29 13:52:07+00:00)
Title: Associated entities and collections

Hi,

I added a failing test. The setup is actually quite simple: just an entity with an associate but both (entity and associate) also are part of a 'group' which in turn is associated to both of them (thus creating a cycle). And that's where it fails. The ObjectStateManager complains that it can't track two entities with the same key as soon as the second entity is attached (because it contains an indirect reference to the first which has already been attached). A proposed fix (based on your idea *g*) is to just attach a key only object.

There is one small drawback though: the persisted (and thus returned) object no longer equals (neither reference nor content) the one passed in because I had to replace the reference before setting it in the persisted parent or I'd still have duplicates in my set as soon as change tracking got turned back on. I also did that for the 2nd caller of AttachAndReload (associated collection elements) but there aren't any tests covering that yet (no test turned red when I had not replaced the added collection element).

I think the side effect of reference inequality should be okay as there are a couple of other places new'ing up "persisted" instances, e.g. missing owned associated entities, missing collections (both, owned and associated) and the root entity.

I'm not so sure if you're thinking about the side effect of not attaching the whole sub graph I'd anticipate code relying on it to attach cycle-free sub graphs without explicitly mapping them. On the other hand I don't believe such code to be correct (and safe) as it might attach parts of the graph that aren't mapped at all or are mapped as associated but without guaranteeing the behavior as changes in implicitly attached sub graphs will be saved as well.

As far as semver is concerned a new major version might be the right thing to do nonetheless.

I also found and fixed another bug for an edge case. I have an associated entity but the association is stored inside that entity (foreign key field). So I need to update the entity to update the association. So far everything is okay and working just fine. But my associated entity has another association which is [Required]. So if I want to update the association (and thus the associated entity) I've to attach that second association as well. Which is impossible to map with the current API because AssociatedCollections can't have associations. My proposed bug fix automatically detects such cases (required associations) and attaches them as well. I've also added a test to demonstrate my problem.