EditLevelMismatch when cloning object

[xal1983]

Describe the bug
When cloning an object graph that contains children, the edit level of the children is not correctly preserved.
Example scenario is Invoice with Cost line items. There is a BindingSource for the invoice and one for the child cost list.
Creating a clone and showing a new form with that cloned object triggers an EditLevelMismatch exception.

Version and Platform
CSLA version: 9.1.0
OS: Windows
Platform: WinForms

Code that Fails
I created a simple demo project to showcase the issue here: https://github.com/xal1983/CslaEditLevelMismatch/blob/master/EditLevelMismatch/Form1.cs

But the gist of it is, the child cost and it's field manager have an EditLevel = 1 at the time of cloning the invoice.
The cloned cost however ends up with EditLevel = 0 but it's FieldManager has an EditLevel = 1.
When setting the binding source, the binding source will automaticall call IEditableObject.BeginEdit() on the cost and Csla will raise the exception because the FieldManager and the Cost's EditLevels do not match.

Stack Trace or Exception Detail
at Csla.Core.FieldManager.FieldDataManager.Csla.Core.IUndoableObject.CopyState(Int32 parentEditLevel, Boolean parentBindingEdit) in //Source/Csla/Core/FieldManager/FieldDataManager.cs:line 512
at Csla.Core.UndoableBase.CopyState(Int32 parentEditLevel) in //Source/Csla/Core/UndoableBase.cs:line 167
at Csla.Core.BusinessBase.System.ComponentModel.IEditableObject.BeginEdit() in /_/Source/Csla/Core/BusinessBase.cs:line 778
at System.Windows.Forms.CurrencyManager.OnCurrentChanged(EventArgs e)
at System.Windows.Forms.CurrencyManager.ChangeRecordState(Int32 newPosition, Boolean validating, Boolean endCurrentEdit, Boolean firePositionChange, Boolean pullData)
at System.Windows.Forms.CurrencyManager.List_ListChanged(Object sender, ListChangedEventArgs e)
at System.Windows.Forms.BindingSource.OnListChanged(ListChangedEventArgs e)
at System.Windows.Forms.BindingSource.ResetBindings(Boolean metadataChanged)
at System.Windows.Forms.BindingSource.SetList(IList list, Boolean metaDataChanged, Boolean applySortAndFilter)
at System.Windows.Forms.BindingSource.ResetList()
at System.Windows.Forms.BindingSource.ParentCurrencyManager_MetaDataChanged(Object sender, EventArgs e)
at System.Windows.Forms.CurrencyManager.OnMetaDataChanged(EventArgs e)
at System.Windows.Forms.CurrencyManager.List_ListChanged(Object sender, ListChangedEventArgs e)
at System.Windows.Forms.BindingSource.OnListChanged(ListChangedEventArgs e)
at System.Windows.Forms.BindingSource.ResetBindings(Boolean metadataChanged)
at System.Windows.Forms.BindingSource.SetList(IList list, Boolean metaDataChanged, Boolean applySortAndFilter)
at System.Windows.Forms.BindingSource.ResetList()
at System.Windows.Forms.BindingSource.set_DataSource(Object value)
at EditLevelMismatch.Form1.SetDataSource(Invoice i) in E:\Devel\Tests\EditLevelMismatch\EditLevelMismatch\Form1.cs:line 27
at EditLevelMismatch.Form1.button1_Click(Object sender, EventArgs e) in E:\Devel\Tests\EditLevelMismatch\EditLevelMismatch\Form1.cs:line 46

Additional context
So, you may suggest "Call EndEdit or CancelEdit before cloning". however, this is precisely what I can't do. The use case is:
-User sees a list of invoices with the editor
-User start editing an invoice
-User needs to look up information from another invoice while editing the invoice
-At this point the app creates a clone of the in progress edit of the invoice and shows a popup with just the editor for this invoice, any edits being made so far need to be preserved, but the user needs to be able to cancel if they want to discard changes.

[rockfordlhotka]

@xal1983 I'm not 100% sure this is a supported scenario.

You are in the middle of editing an object graph, so various edit levels within the graph could be all over the place. And you clone the graph, so those edit levels could stay the same - but unless you can figure out the exact edit states of the whole graph how would you bind the clone to a UI? Or do anything else with it in such an indeterminate state?

[xal1983]

Hi @rockfordlhotka,
I'm not sure why this wouldn't be supported, this app has been relying on this behavior for 20 years (migrating from Csla 4 to 9 atm), it's possible it works because when the BeginEdit is called from a BindingSource it only ever gets to level 1 because of the special handling for System.ComponentModel.IEditableObject.
Regardless, the fact is that cloning the object clears the state stack.
I was able to finally track down where this happens. The state stack is preserved and correctly deserialized on the item, but when added to the deserailized collection, InsertItem calls "UndoableBase.ResetChildEditLevel(item, EditLevel, false);"
The collection EditLevel was always 0, so this forces the item to clear it's internal statestack.

Here's the code that triggers this behavior:

// set child edit level UndoableBase.ResetChildEditLevel(item, EditLevel, false); // when an object is inserted we assume it is // a new object and so the edit level when it was // added must be set item.EditLevelAdded = EditLevel;

I think we should probably skip this block if deserializing? Maybe using a flag to indicate that we're calling InsertItem from within OnSetChildren?

Thoughts?

PS: Interestingly, this was the default behavior we always did before AutoClone on save. Because this app predates that, it still uses that strategy:

var temp = _EditObject.Clone(); temp.ApplyEdit(); _EditObject = temp.Save();

The idea behind this approach is that if save fails, you still have the option of continuing to edit the original object to fix any data issues or undoing your changes and roll back to it's original state. In my mind that is one of the main uses for the undo functionality.

Cheers!

[rockfordlhotka]

Interesting. This is outside of what I remember as the design parameters for winforms data binding back in the 3.0-3.5 timeframe when I do remember pissing off a lot of people by making the edit level much more strict 😀

My recollection is that the object graph needed to be unbound from the UI, then you could safely call Save, then you'd have to bind the UI to the returned result graph.

I imagine the reason you saw the behavior you did is because the BinaryFormatter just blindly serialized everything that wasn't marked with NonSerialized. With MobileFormatter (and other serializers), we actually have to reinsert the child objects as we rebuild the graph, because we're not "cheating" with reflection.

So I'm not saying your issue isn't valid - but I do think it is an "accidental" side-effect of BinaryFormatter.

If there's a way to restore the necessary behavior without breaking existing v9 behaviors outside this specific scenario, then I'm totally willing to accept the change.