I'm having what seems to be a related (but different) problem from the one I described a few weeks ago, and this is admittedly related to undo support for child objects which isn't currently fully supported. Still, it seems like this isn't quite right. I'm using CSLA 3.5.1, DisableIEditableObject=true.
This is a somewhat unusual lazy child scenario. The child isn't automatically loaded just because you call GetProperty(). The lazy loading occurs elsewhere, so a call to GetProperty() returns a null value if the object isn't loaded.
At the time I called BeginEdit(), it evidently pushed an IFieldData value onto the state stack for which the Value is null, so the object didn't exist before.
When the child is lazily loaded, it syncs up it's edit level correctly and every thing seems fine.
Now I call CancelEdit().
What I expected (and want) to happen is for this child to now disappear. It wasn't there when I called BeginEdit(), and now I want it to go away.
However, the undo code in FieldDataManager sees that there was previously an item on the state stack (not looking closer to see that it was really null, e.g. IFieldData.Value==null), so calls UndoChanges on the the lazy child instead of removing it.
I think in this case a reasonable course of action is to just restore the previous item from the state stack, e.g. just put back the IFieldData with the null value.
I think I can program around it for now, but it seems to me that there is clearly enough information on the state stack for UndoChanges to detect that the previous object value was really null. However, I can see how this might be an edge case.
I'm not sure it does have enough information to do what you want.
Remember this is n-level undo. I don't think it has enough information to know that the value was null at level n1, but you are now at n3 and so it will take three CancelEdit() calls to get back to the null.
If the child object was null at n1 but present at n2 and n3, here is what I think happens: (and how a possible fix might work)
When you CancelEdit on the Parent at n3, the state stack has one of your "boolean placeholders" that just indicates the child object was present at n2, so you cascade the UndoChanges call to the current child value. Then you pop the state stack.
Then you CancelEdit on the Parent at n2. The prior state on the stack for the child now has IFieldData(null value) rather than the boolean. This is not tested for currently but I think it could be. So, instead of cascading the call to the child like you do now, you could instead discard the child object entirely and restore the IFieldData reference as the child value in the FieldDataManager.
Now the Parent object is at n1, and it would (with the fix) look just like it did before the BeginEdit() BeginEdit() CancelEdit() CancelEdit() sequence.
You miss my point.
var root = Root.NewRoot();
root.BeginEdit();
root.Child = Child.NewChild();
root.BeginEdit();
root.BeginEdit();
root.BeginEdit(); // edit level of root and child are now
at 4
root.CancelEdit();
This is where the problem occurs. At this point, how do we know
whether to revert to the null value that the child started with, or to cascade
the CancelEdit() call to the child?
I agree that this should be resolved. I’m just saying that
it isn’t a trivial thing.
There’s already an item in the wish list that basically
addresses this question – which is that a direct managed reference to a
child object needs to act much more like a single item BusinessListBase
reference to that child.
I mean really what you want is more than you are asking. Really
what you want is to be able to do this:
var root = Root.GetRoot(123);
root.BeginEdit();
root.RemoveChild(); // removes existing child, which is marked
for deletion and stored in a “deletedList”
root.Child = Child.NewChild();
// … (any number of nested Begin/Cancel/Apply edit
operations
root.CancelEdit(); // removes new child, replacing it with the “deleted”
child
This is the same problem as your null problem – just with
an initial non-null value.
The problem is absolutely solvable. Either by having the
FieldData object use a BusinessListBase internally to handle all the complexity,
or by creating a ChildFieldData class that does much of what BLB does.
Rocky
I think I did understand the point. I'm not asking for a perfect solution -- I think this is a 2-3 line fix. Also, this example works the way I want it to with a non-managed field, it just doesn't work with a managed field.
Using your example:
var root = Root.NewRoot();
root.BeginEdit();
root.Child = Child.NewChild();
root.BeginEdit();
root.BeginEdit();
root.BeginEdit(); // edit level of root and child are now at 4
root.CancelEdit();
Non-managed fields work correctly because the null value is
stacked into the state – so CSLA actually knows at which point to switch
back to the null.
I may address this in 3.6.2 – I just don’t know for
sure – it is a matter of time and priority. But I need to fix it
correctly, to address not only the null issue, but the delete issue too –
because if I fix the delete issue the null issue is resolved by definition.
Rocky
From: rsbaker0
[mailto:cslanet@lhotka.net]
Sent: Saturday, February 14, 2009 3:51 PM
To: rocky@lhotka.net
Subject: Re: [CSLA .NET] RE: Can't undo lazy-loaded child loaded after
BeginEdit (managed property)?
I think I did understand the point. I'm not asking for a perfect solution --
I think this is a 2-3 line fix. Also, this example works the way I
want it to with a non-managed field, it just doesn't work with a managed field.
Using your example:
var root = Root.NewRoot();
root.BeginEdit();
root.Child
= Child.NewChild();
root.BeginEdit();
root.BeginEdit();
root.BeginEdit();
// edit level of root and child are now at 4
root.CancelEdit();
At
this point I don't see a problem -- the state stack in the FieldManager
indicates a previous value was present, so you call UndoChanges() on the
root.Child value. Then you pop the state stack.
The
same would be true on the 2nd and 3rd calls to CancelEdit(). So,
again you call UndoChanges() on the Child.
On
a 4th call to CancelEdit(), matching the first BeginEdit(), however, the state
stack tells you there wasn't a child value, so you would remove it.
RockfordLhotka:
Non-managed fields work correctly because the null value is stacked into the state – so CSLA actually knows at which point to switch back to the null.
I'm not trying to quibble, but (unless I'm missing something and I've waded in pretty deeply already) your managed field implementation has the same information. However, your test in the FieldDataManager to see if a previous value was in the state stack only looks to see if it there is *any* non-null value. This test gets confused if the previous "value" is an IFieldData w/null data value. It works fine if the field was actually never previously accessed and there is no IFieldData at all on the stack.
I went to managed fields for child objects per your recommendation because they are supposed to simplify the programming and you automatically get whatever fixes or improvements you put into the infrastructure. In this case, my old code worked as I expected it to (with the known limitations), and then suddenly broke just because I changed it to a managed property.
Until you get around to a complete solution, I'm just asking that they work the same way.
Don’t worry about quibbling :)
I have not looked at that code for (probably) months and am
going from memory.
If you have a 2-3 line fix, post the idea and that’ll help me
when I get time to look at the issue.
Rocky
From: rsbaker0
[mailto:cslanet@lhotka.net]
Sent: Saturday, February 14, 2009 4:14 PM
To: rocky@lhotka.net
Subject: Re: [CSLA .NET] RE: RE: Can't undo lazy-loaded child loaded
after BeginEdit (managed property)?
RockfordLhotka:
Non-managed fields work correctly because the null value is stacked into the state – so CSLA actually knows at which point to switch back to the null.
I'm not trying to quibble, but (unless I'm missing something and I've waded in pretty deeply already) your managed field implementation has the same information. However, your test in the FieldDataManager to see if a previous value was in the state stack only looks to see if it there is *any* non-null value. This test gets confused if the previous "value" is an IFieldData w/null data value. It works fine if the field was actually never previously accessed.
I went to managed fields for child objects per your recommendation because they are supposed to simplify the programming and you automatically get whatever fixes or improvements you put into the infrastructure. In this case, my old code worked as I expected it to (with the known limitations), and then suddenly broke just because I changed it to a managed property.