I'm finding that the FieldDataManager (FieldManager property) seems to be returning the wrong type of data in certain undo scenarios.
Basically, I have a managed child object (lazy loaded) associated with a particular PropertyInfo. It was probably null when I called BeginEdit() on the object, but was loaded soon afterwards as soon a data binding requested data from an associated property. (Basically it's a child object in a 1:1 relationship versus a collection).
I then took an action in the user interface that cleared it. The UI responds correctly -- the object key and description disappear, just what I want.
Now I call UndoChanges. My FieldData slot is now occupied by a Boolean value rather than either null or a value of the correct type, and I get an invalid cast exception when I try to call GetFieldData() in my property getter. This happens after UndoChangesComplete() is fired, and the EditLevel is 0 at this point, so where once was null, now appears to be a FieldData<Boolean>.
Unable to cast object of type 'Csla.Core.FieldManager.FieldData`1[System.Boolean]' to type 'Csla.WORMapper.DependencyPropertyData`2[Company.Application.Data.ChildType,System.Nullable`1[System.Int32]]'.
This is a custom PropertyInfo<T>/FieldData<T> situation, so maybe I overlooked a step. But I think you might have been storing booleans in the state stack to indicate the presence/absence of a value and this is being left behind somehow?
EDIT: OK, I see from your use GetFieldData() that this may be by design, as you are using "fieldData as ChildType" rather than forcibly casting the return value to the property info type -- e.g. (ChildType)fieldData. I think I can work around this.
I had edited my original post with some additional information, but your code appears to dynamically tests the type of the returned value versus my forced cast, so it may not be a bug after all.
I definitely need to be aware of it, though... :)
Hmmmm. I think there may be a bug after all.
I solved my original problem, but in continued editing, I can't seem to set the property to a new value once this has happened.
It's trying to stuff the new child object into the FieldData<Boolean> placeholder that is left behind.
EDIT: I have found where it is doing it, but not sure what the best fix would be.
In FieldDataManager.UndoChanges, you have these two lines of code:
var oldItem = state[index]; var item = _fieldData[index]; The oldItem value from the state stack is the boolean, indicating there wasn't a value. The current item value is the correct FieldData<T> type, but it's value is null, which causes the code following if (undoable != null) to be bypassed, and then it just puts the boolean in as the state: if (item != null){
var undoable = item.Value as IUndoableObject; if (undoable != null){
// current value is undoable if (oldItem != null)undoable.UndoChanges(parentEditLevel, parentBindingEdit);
else_fieldData[index] =
null; continue;}
}
// restore IFieldData object into field collection_fieldData[index] = oldItem; <---------HERE IS WHERE IT HAPPENS
rsbaker0:
Basically, I have a managed child object (lazy loaded) associated with a particular PropertyInfo. It was probably null when I called BeginEdit() on the object, but was loaded soon afterwards as soon a data binding requested data from an associated property. (Basically it's a child object in a 1:1 relationship versus a collection).
ajj3085:
Ok, I can't comment on whether or not you've found a bug, but this seems odd to me. If you have a lazy loaded child, but databinding always ends up loading almost immediately... that seems like a poor choice to implement the child as lazy loaded.
Quick on this -- the UI consists of a multi-tabbed interface and the property where I first an into this is only shown on an seldom used "Advanced" tab.
I've researched further, and it turns out it has nothing to do with lazy loading or my custom field implementation. The undo implementation seems to be incapable of restoring a managed child property set to null after BeginEdit() is called back to its previous non-null value:
This simple but albeit constrived sequence will break: (STATION.Employee is a completely standard managed property implementation, no lazy loading, CSLA 3.5.1)
EMPLOYEE emp = EMPLOYEE.CreateChild(); STATION s = STATION.GetFirst(string.Empty);s.Employee = emp;
s.BeginEdit();
s.Employee =
null;s.CancelEdit();
s.Employee = emp;
// Exception occurs here since FieldData is now FieldData<Boolean>Unable to cast object of type 'System.Boolean' to type 'Company.Application.Data.EMPLOYEE'.
OK, now I understand the issue. This has never worked in any version of CSLA, and isn't currently supported in 3.6 either. There's a wish list item to add this capability, and the field manager should make such a change possible, but I haven't had time to really dig in and make it happen as of yet.
Ah, I see.
In the meantime, I'd argue that the code sequence I have provided silently breaks the object in an unfixable way.
If the behavior isn't supported, then I should either get an exception when I try to do an in-place replacement like this, or perhaps less onerously Undo should just leave the object null rather than putting a value of the incorrect type in it's place.
Why not just put the object itself on the state stack instead of FieldData<bool> -- maybe only if it's not a collection. That way, you could put the original value back if Undo was called and the current reference was null (or perhaps even if the reference has changed, although that seems trickier). I may even have to implement it as a workaround unless you tell me I shouldn't ;)
As I say, this is something that the field manager allows to be
addressed, but it is relatively complex.
Child objects can never be put on the state stack, as they are
independent entities that manage their own state. So what has to happen is that
the FieldData object for child objects needs to maintain a list (stack) of
child object references, and it must coordinate all the begin/cancel/apply
operations on the parent so they cascade down to the child objects
appropriately.
You are looking at it from the perspective of one child and one
null. But the general case is one where numerous child references could be used
at different edit levels, and those references must be restored or discarded as
stacked apply/cancel methods are called. And at the same time the list of
non-active child objects can’t get out of sync with the overall edit
level of the object graph, because they could become active based on the
parent.
This is absolutely solvable, but it isn’t a 30 minute
thing to do. N-level undo is the second most complex part of CSLA (following
data binding), and the burden of testing is very high due to the numerous
permutations.
I don’t disagree that a better exception might make sense –
but I do plan to enable the scenario and so I didn’t put energy into
smoothing over a “temporary” problem.
Rocky
Thanks for the more detailed insight.
Correct me if I'm wrong, but if this were a field that wasn't managed, the code snippet I supplied would just silently leave the field null, right? Changes to the child object contents are undoable, but setting the actual field holding the child object to null isn't?
Non-managed fields that reference non-CSLA objects are directly serialized
onto the parent’s state stack, and so they will be subject to undo at all
levels. But you’ll get a new object instance on any CancelEdit() call,
because the stacked state (byte stream) is deserialized.
Any field referencing a CSLA object (one that implements
IUndoableObject) is never serialized, and the undo call is cascaded to that child.
But (outside BLB) there’s no tracking of previous reference values other
than null.
In other words, if the value starts as null, then becomes a
reference, a cancel will restore the null. But the reverse is not true. The
field manager design includes the ability to solve this – I just ran out
of time to address the issue in 3.6.
3.6.1 is primarily a bug-fix release, with some low-hanging
fruit as a bonus. Hopefully I’ll be able to address more complex issues (like
this one) in 3.6.2 or .3.
Rocky
From: rsbaker0
[mailto:cslanet@lhotka.net]
Sent: Tuesday, January 27, 2009 10:02 AM
To: rocky@lhotka.net
Subject: Re: [CSLA .NET] RE: Undo mischief in CSLA 3.5? GetFieldData()
returning a FieldData<Boolean> instead of FieldData<ChildType>?
Thanks for the more detailed insight.
Correct me if I'm wrong, but if this were a field that wasn't managed, the
code snippet I supplied would just silently leave the field null,
right? Changes to the child object are undoable, but setting the actual
field holding the child object to null isn't?
Thanks for your patience.
Here is a simple two line "fix" to FieldDataManager.UndoChanges() that will at least leave the value null rather leaving the FieldData<bool> around to cause trouble. There may be a better way to do this, but it seemed safe to me -- it only does something different if the FieldData types don't match. I'm leaving the current FieldData with correct type but null value in place rather than setting the whole thing to null (not sure which would be preferable in this case, but it works either way for me).
void Core.IUndoableObject.UndoChanges(int parentEditLevel, bool parentBindingEdit)
{
if (EditLevel > 0)
{
if (this.EditLevel - 1 < parentEditLevel)
throw new UndoException(string.Format(Properties.Resources.EditLevelMismatchException, "UndoChanges"));
IFieldData[] state = null;
using (MemoryStream buffer = new MemoryStream(_stateStack.Pop()))
{
buffer.Position = 0;
var formatter = SerializationFormatterFactory.GetFormatter();
state = (IFieldData[])(formatter.Deserialize(buffer));
}
for (var index = 0; index < _fieldData.Length; index++)
{
var oldItem = state[index];
var item = _fieldData[index];
if (item != null)
{
var undoable = item.Value as IUndoableObject;
if (undoable != null)
{
// current value is undoable
if (oldItem != null)
undoable.UndoChanges(parentEditLevel, parentBindingEdit);
else
_fieldData[index] = null;
continue;
}
else
{
// CHANGE TO CSLA HERE
// Workaround for previously existing child object being set to NULL
// NOTE: This does not make change to null undoable, but avoids leaving mismatching FieldData
// type in the slot formerly occupied by an object
if (item.Value == null && (oldItem != null && oldItem.GetType() != item.GetType()))
continue; // Leave existing FieldData with null value for now
}
}
// restore IFieldData object into field collection
_fieldData[index] = oldItem;
}
}
}
Quick on this -- the UI consists of a multi-tabbed interface and the property where I first an into this is only shown on an seldom used "Advanced" tab.
Copyright (c) Marimer LLC