Change tracking of entity properties

I'm trying to implement an Undo/Redo system for the data controlled by Lightspeed. This Undo/Redo system registers the changes (there are actually three: property change, object create and object delete).

To connect my Undo system I set generation="FieldOnly" in the .lsmodel file and then I write my partial classes which register changes in property setters. I'm completely successful with regular fields, but I cannot get it working with relation fields. What bothers me:

1. Let's assume I have Parent-Children relation (1_to_Many). When I use the following access:
   
   child.Parent = parent;
   
   it works perfect - my property setter is called (I can register change) and both child.Parent property and parent.Children lists are correct.
2. When I use the opposite access:
   
   parent.Children.Add(child);
   
   it doesn't work because my property setter isn't called (however both child.Parent property and parent.Children lists are correct).
3. When I try to remove item from relation using:
   
   child.Parent = null;
   
   I get the strange situation - my property setter is called, child.Parent == null but parent.Children still contains 'child'.
   
   
4. Similar problems occur for 1_to_1 relations.

I know that there's the ChangeTracker class, but it's too primitive for me. I also tried using Entity.PropertyChanged event, but the event doesn't contain oldValue and newValue.

Could you help me somehow?

I see the following solutions:

1. Make an interface:
   
   interface IChangeTracker
   {
      void PropertyChanged(Entity anEntity, string aPropertyName, object anOldValue, object aNewValue);
   }
   
   or even better:
   interface IChangeTracker
   {
      void PropertyChanged<T>(Entity anEntity, string aPropertyName, T anOldValue, T aNewValue);
   }
   
   allow me to write my class which implements such interface, and assign instance of this class to UnitOfWork or Entity.
2. If Lightspeed called my setter while calling "parent.Children.Add(child);" and behaved correctly while calling "child.Parent = null;" -  it would be the solution for me.
3. If the Entity.PropertyChanged event or Entity.OnPropertyChanged method contained oldValue and newValue it would also help me.

I'm currently using nightly build from 20110519.

LightSpeed can't call a setter when you do a Children.Add because there's no setter involved.  However it does raise the EntityCollection<T>.EntityAdded event.  You could subscribe to this event in your constructor and hook your change tracking to that.  (Same for Children.Remove and the EntityRemoved event.)

We'll look into the issue where setting child.Parent == null doesn't remove it from the parent.Children collection.  However since your setter is being called you should still be able to record this as an undoable action.  You may just need to take a bit more care in the Redo stage because the entity to be reinstated into the collection may still be present.

Thank you for your answer.

I can use EntityAdded and EntityRemoved events, however they are very inconvenient for me.

I noticed that Lightspeed allows for changing relations by changing foreign key property (in the previous example I can use: child.ParentId = parent.Id). I appreciate it very much, because tracking changes of relations are exactly the same as tracking changes of regular fields.

When I try to track changes in EntityAdded and EntityRemoved event handlers I'm on the 'wrong' side of the relation - I have to make special support for this. I also have to write a separate handler for every relation.

Anyway - it's hard to code, but it's possible to code. The final problem lies in 1_to_1 relations - I didn't notice any events that are invoked for such relations.

I think myself, that you already have a change tracker - accessible through Entity.ChangeTracker property. Why not allowing me to write my own change tracker and replace the built-in one?

One more comment to the previous post:

The EntityAdded and EntityRemoved events also seem to be insufficient. In the event handler argument I have only the new state of the entity - I haven't got the previous state. The problem appears when I move a child from one parent to another:

• child.Parent = parent1;
• child.Parent = parent2;

In the event handler invoked by 2nd line I have no information that child was previously in parent1's Children list. I could rely on EntityRemoved event for parent1.Children list (registering 2 undo actions instead of one), but it isn't called in this case.

All this makes my Undo system very complicated :-(

We can certainly look at opening up some of the methods on ChangeTracker to allow overriding or interception.  Actually, since you don't need to modify the behaviour of the existing ChangeTracker, just augment it, the easiest thing would be for ChangeTracker to raise an event with the old and new values.  Would this suffice for you?

Definitely it would be sufficient :-)

The only thing I'm afraid of, is the ChangeTracker mode. I kindly ask you to call this event in TrackingMode = ChangeTrackingMode.None or to add new TrackingMode for the event. If you required to set TrackingMode = ChangeTrackingMode.ChangesOnly to have event called, I would get heaps of ChangedValue objects which are not interesting for me (I cannot get rid of them, and they might be memory consuming).

Another subject - please add Entity as one of the properties of the event args. I will have only one EventHandler which would handle changes of all the entities (one per UOW not per entity), so I have to distinguish entities.

The necessary properties for me are:

• Entity
• OldValue (I think they will be of type 'object' - it would be even better if they could be generics)
• NewValue (comment as above)
• PropertyName (I assume that there will be regular fields as well as relation identifiers).

I'd also like to thank you for taking our needs seriously and helping us every time we ask for help. You're great!

Okay, I'll take a look at this.  I can't promise that we won't keep ChangedValue objects around, but if this is an interactive application (as I assume it is from the fact that you're implementing undo/redo), there shouldn't be *that* many of them hanging around.  I'm still thinking about the implementation options though!

However, before I jump into this, one of your concerns was intercepting entity collection add and remove actions.  The change tracker doesn't record these -- they are implicitly recorded through the entity holder change at the other end.  As far as I can tell, this *should* suffice for your undo system, but can you confirm it?  I don't want to embark on this and then find it still doesn't do what you need!

I understand that change tracker doesn't record add and remove actions - instead it records changes of identifiers. But the mentioned Lightspeed feature allowing me to change relation by changing its identifier does the job.

I understand it this way:

1. Lets' assume 'child' belongs to parent1's Children collection.
2. In the code I write: parent1.Children.Remove(child); parent2.Children.Add(child);
3. The ChangeTracker records two changes: 1st: "child.ParentId changes from parent1.Id to null"; 2nd: "child.ParentId changes from null to parent2.Id".
4. While undoing these changes I perform the opposite: 1st: "set child.ParentId to null", 2nd: "set child.ParentId to the recorded Id of parent1".
5. Assuming that parent1 exists (hasn't been deleted or has been undeleted with the same Id - I can achieve that), the relation is rebuilt correctly and child again belongs to parent1.Children collection.
6. The point 2 can also be achieved by: child.Parent = parent2;. In this case the ChangeTracker records only: "child.ParentId changes from parent1.Id to parent2.Id". Undoing such operation should also give correct result.
7. I assume that 1_to_1 relations will also be recorded as their identifiers
8. The Many_to_Many relations are done using "through" objects, so the mechanism is exactly the same as for 1_to_Many. I assume that ThroughAssociation is updated accordingly by Lightspeed after updating any of its component relations.

A comment to ChangedValues objects - we are using Lightspeed to store data for a CAE desktop application. That's why we treat UOW as long lived - once an object is created, it lives until either user deletes it from the project or user closes the project. That's why all the changes made to this object would remain in memory without a chance to dispose. I'm afraid of volume of ChangedValues objects recorded for all properties during a few hours work. You may say that our Undo/Redo objects are also stored, but we can control their lifetime, and cut Undo list as the ultimate solution if memory gets low.

Sweet, thanks for the clarification.  I'll take a look at this now.

Honestly, I wouldn't worry too much about blowing out the number of ChangedValue objects.  You're right that the entity and its ChangeTracker remain in memory, but the list of changes is cleared when the unit of work is saved.  So unless the user has literally tens or hundreds of thousands of unsaved edits, it just shouldn't be an issue.  And users will surely be saving more often than every ten thousand changes...?

Okay, this will be in the next nightly build (available from about 1200 GMT, assuming our build server doesn't have another fit).

The ChangeTracker object now has a ChangeTracked event.  This is raised only if change tracking is enabled.  The ChangeTracked event indicates:

* Which entity the change happened on
* The kind of change
* If appropriate, the property name and before and after values

The kind of change can be Immediate, Pending, Commit or Cancel.  The simple case is Immediate, and unless you (or one of your controls) use IEditableObject, that is all you will ever see.

However if you have something like a data grid which allows back-out, then you may see Pending, Commit and Cancel changes.  Pending means a change under the aegis of a BeginEdit.  Commit means EndEdit (all pending changes committed to the entity -- note this is not the same as a commit to database).  Cancel means CancelEdit (all pending changes reverted; you will not get separate change notifications for the properties that are being put back the way they were).  Commit and Cancel events do not include property name and values, because they are about applying/reverting changes that have already been made, and may involve multiple properties and values.

For the time being, I have not provided a mode to turn off the Changes collection (see previous message).  Please give it a go the way it is, and if you really do find that large numbers of ChangedValue objects are backing up, then let me know and we can take a look.

I have downloaded the nightly build and bound my Undo system to the ChangeTracked event. Generally it works :-), but I face some serious problems:

1. Changes are tracked incorrectly for 1_to_Many relations when removing item from list:
   1. parent.Children.Remove(child) - works ok, one change is tracked
   2. child.Parent = null - change is tracked twice
   3. child.ParentId = null - change is tracked once, however item remains in parent.Children list
2. Changes are tracked incorrectly for 1_to_1 relations:
   1. Let's assume we have obj1.RelToObj2 and obj2.RelToObj1 - objects are bound to each other (1_to_1 nullable relation)
   2. When I delete obj2 by uow.Remove(obj2) - obj1.RelToObj2 is correctly set to null, but no change is tracked
3. The changes for 1_to_1 relations are tracked for the object, which actually has been changed, not for the one which holds the relation Id. It makes registering Undo very hard, because I don't know the other's side property name - please see my comment below and the Undo scenario in one of my previous posts.
4. Assigning null to Id field doesn't remove Entity from the other side list (mentioned previously). It makes undoing of changes impossible.

General comment to points 3 and 4: In my Undo objects I store the object Id's, not the object references, because objects can be deleted and undeleted over time. That's why recording and restoring of relation Ids is critical for me :-(

There are also some good news: I noticed that ChangedValue objects are disposed after uow.SaveChanges. That means that I don't have to bother about them anymore :-) Is that true?

The next nightly build will include fixes for the item remaining in the parent.Children list (1.3) and the change in a one-to-one on object removal not getting tracked (2.2).

We will try to look into 1.2 but for now I think you will just need to deduplicate the events as you process them into your Undo structure.

Regarding 3, correct, the changes are tracked for the object which is changing.  You can get the other side's property name from the ReverseAssociationAttribute.  If you are hand-coding then you may not be applying RAA, instead letting LightSpeed infer reverse associations: in that case you will need to apply RAA by hand, use heuristics, or switch to LightSpeed 4 Beta where you can get the info direct from the metadata API.

Regarding 4, this seems to be the same as 1.3 and should therefore be fixed in the next nightly build.

Yes, the Changes list is cleared after uow.SaveChanges -- see previous post on this thread.

Thank you very much for the implementation!

It works much better, but I still have problems :-(

1. 1_to_1 relations are still incorrect (previous point 2):
   
   When I delete obj2 by uow.Remove(obj2) - obj1.RelToObj2 is now correctly set to null, but obj1.RelToObj2Id still has previous value. It means that when I assign another object to obj1.RelToObj2, I get incorrect change (change.OriginalValue isn't null)
2. Another problem arose, which IMO wasn't present before:
   When I detach a deleted object from the UOW (according to http://www.mindscapehq.com/forums/Thread.aspx?ThreadID=3832), Lightspeed also detaches (but not deletes) objects which were in relation to that object. 

Could you provide us with repro code for #1?  Our tests for this are passing but maybe we haven't understood the desired behaviour.

We will look into #2 -- I don't think anything has changed here though, and we may need to double check what the desired behaviour is.

I attach an example of both cases.

Case1 - shows that RelationId is incorrectly changed for 1_to_1 relations after removing an object. It causes that changes are also tracked incorrectly.

Case2 - shows that RelationId is incorrectly changed for 1_to_Many relations after removing a "parent" object (new subject in this thread). Additionaly detaching parent from UOW detaches children, what is unexpected IMHO.

I've made some updates in the latest nightly build which partially address these issues:

* "Original value" for 1-to-1 relationships is now correct (it was not being reset on save).

* There is now a flag which will remove deleted items from the unit of work automatically on save.  This removes the need to detach them (and prevents a cascading detach if you do).  To set this flag:

context.CompatibilityOptions |= LightSpeedCompatibilityOptions.ClearDeletedEntitiesOnSave;

I have investigated fixing the foreign key to clear it after a parent entity is removed.  Unfortunately this is not an easy fix.  We were able to get it to work for your specific test case, but there is a problem in cases where there have been changes made to the child entity and the relationship is dependent.  If there have been changes made to the child entity, LightSpeed issues a (redundant) UPDATE for the child entity, and if the relationship is dependent and the foreign key has been cleared, then this redundant UPDATE fails with an "association must be present" validation error.  We realise this is a bug -- the entity is going to be cascade-deleted anyway so we should not be issuing the UPDATE -- but it is difficult to fix in the current architecture.  Therefore for now we are going to have to leave the obsolete FK in place.

For the undo system, you should be able to work around this by noting when a to-one association is changing to null, and recording that the FK is spurious and should be deemed 0 or null for the next change's revert purposes.  Sorry we can't offer you a cleaner solution at the moment.

I found another bug.

After binding items in Many_to_Many relations, ChangeTracker unexpectedly resets.

You may find it in the attached example, Case3.

BTW: If you wish, I may prepare these examples as NUnit test fixtures.

Concerning cases 1 and 2:

I've downloaded the 20110608 build and it works much better :-) Thank you - especially for ClearDeletedEntitiesOnSave.

I understand the identifiers subject. I rely on ChangeTracker, so I may probably live with it (I will check it out).

However I noticed that ChangeTracker behaviour depends on SaveChanges. I attach an appropriate example. Please find the two comments "!!! Here SaveChanges modifies the ChangeTracker behaviour." in Case1.cs and Case2.cs. They denote SaveChanges lines which, if commented out, change the ChangeTracker notifications.

Please also look at Case3 (my previous comment).

Thanks, I'll take a look.

NUnit test fixtures would be awesome -- thanks!

Just to let you know, a fix for the many-to-many bug (Case 3) should be in tonight's nightly build.  We don't have a fix for the SaveChanges issue yet, but hopefully soon!

Thank you for the correction!

I've prepared a test library. It is suited to our build environment (libraries are expected in "..\..\..\bin" folder). I hope you can easily fit it to your needs.

In the attached tests I added a test which shows another bug, related to object inheritance - the SQL script generated for deleting objects is incorrect - it references not existing field of the base class. Please look into Case4Test() for details.

Quick update: the build server had a bit of a hiccup last night so the nightly build with the Case 3 fix is not up yet after all.  Sorry for the inconvenience.

Regarding the behavioural changes caused by SaveChanges: looking at the output from Case 2, I believe it is working as expected.  When you create a new entBParent and add entBChild to its Children collection, entBChild joins the entBParent's unit of work.  So one would indeed expect the (if ch.UOW == null) check to fail and the "Child has no unit of work" line not to execute.

I did notice that you have a comment saying it "modifies the ChangeTracker behaviour," but the output doesn't seem to relate to the ChangeTracker.  So maybe I am looking at the wrong thing.  If so please let me know how to reproduce the thing that's behaving unexpectedly and I will take a look.

In Case 4, could you try changing the one-to-one association to a one-to-many association?  (You can do this by right-clicking the association arrow and choosing Convert to One-to-Many Association.)  If this fixes the issue, then we can provide you with a workaround while we investigate the problem.

It's funny, but Case3 looks to be solved :-)

Case 4 - I have plenty one-to-one relations, so I'm not eager to change them all to one-to-many. I'll rather wait. All the cases affect my undo/redo system, so I can live with them until we finish the project (I have them marked as bugs).

Anyway - my previous post contains attached NUnit tests, which check all the cases. I hope they are self-describing - if not - please let me know.

There'll be a candidate fix for Case 4 in the next nightly build.  There may be some situations it doesn't address, so let me know if you run into any of these.

It looks like recent correction spoilt entity removing when relation is not nullable (both for 1_to_1 and 1_to_Many). I attach expanded tests.

Could you let me know which of the tests I need to look at?  I'm translating your tests into our test environment, so I would prefer to just bring across the new tests rather than re-translate the whole suite.  Thanks!

OK, I understand.

I extended Case4 to check both manual and automatic removing of dependent (not nullable) entities.

If I've understood Case 4 correctly, LightSpeed is behaving correctly and the assertion in the aManuallyDeleteSubrecord=false case is incorrect.  (For me the test passes in the aManuallyDeleteSubrecord=true case, so hopefully we're all good there.)

The EntityDSubrecord does not have a dependent foreign key to EntityDInherited.  Despite the class names, it is perfectly legitimate to have an EntityDSubrecord floating around with no EntityDInherited.  Therefore, LightSpeed does not automatically cascade the delete to the EntityDSubrecord, and it remains in the unit of work.

So for the aManuallyDeleteSubrecord=false case, the assert should read Assert.AreEqual(1, uow.Count()), and for me this passes.

If you want EntityDSubrecord to be cascade deleted, you need to do two things:

1. Make the association non-nullable (it is currently nullable).

2. Reverse the direction of the association so that the foreign key is on EntityDSubrecord instead of on EntityDInherited.

(Yes, directionality of one-to-one associations is confusing -- sorry!)

There are two relations from EntityDInherited:

1. 1_to_1 relation to EntityDSubrecord, which is "Source nullable" = false, but "Target nullable" = true. I understand it as follows: EntityDSubrecord cannot exist without EntityDInherited, but EntityDInherited can exist without EntityDSubrecord. So I expect EntityDSubrecord to be cascade deleted when EntityDInherited is deleted.
   Nevertheless even if I change "Target nullable" = false, nothing improves - EntityDSubrecord is still not cascade deleted.
2. 1_to_many relation to EntDArrayItem, which "Is nullable" = false. So I expect EntDArrayItem to be cascade deleted when EntityDInherited is deleted.

These two cascade deletes are tested by Case4, when aManuallyDeleteSubrecord is set to false. My last test was against 20010616 nightly build.

1. Please try reversing your association.  At the moment, EntityDInherited has a foreign key to EntityDSubrecord, meaning that EntityDSubrecord can exist without EntityDInherited, but EntityDInherited cannot exist without EntityDSubrecord -- the exact reverse of what you want.  Reverse the association so that the foreign key is on EntityDSubrecord.  Then you should see the cascade delete as desired (but see below).

Regarding "Source nullable," unfortunately this option should not have been shown as it has no effect in current versions of LightSpeed.  Nullability applies to the FK and the FK of a one-to-one association is represented by the target id.  There is no concept of nullability in the 'source' direction.  We apologise for the confusion.

2. EntDArrayItem is cascade deleted when EntityDInherited is deleted, but cascade deleted items were not being removed from the unit of work.  There will be a fix for this in the next nightly build.  This issue will also affect one-to-one associations -- again fixed in the next nightly.

I will try to reverse 1_to_1 relations.

Summarizing: from my point of view, the only points left is the Case1 and Case2 without SaveChanges (incorrect change tracking in these cases).

We had a hiccup in the overnight build so those fixes aren't up at the moment -- should be up tonight, and I will look at Case1 and Case2 now.

I have implemented a fix for Case1 and Case2 without SaveChanges and it will be in tonight's nightly build (assuming our build server doesn't throw a tizzy again).  Let us know how you get on!

It all works fine now. Thank you!