CSLA for Silverlight - Minor Bug in Undo Deserialisation

CSLA for Silverlight - Minor Bug in Undo Deserialisation

Old forum URL: forums.lhotka.net/forums/t/6413.aspx


paupdb posted on Thursday, February 12, 2009

Hi Rocky and Magenic guys

I believe I've found a minor bug in the undo deserialisation in Silverlight - specifically in Csla.Core.FieldManager.FieldDataManager.

The problem comes up in the OnSetState method, at the section below in the part I highlighted:
   
      foreach (IPropertyInfo property in _propertyList)
      {
        if (info.Values.ContainsKey(property.Name))
        {
          SerializationInfo.FieldData value = info.Values[property.Name];

          IFieldData data = GetOrCreateFieldData(property);
          if (mode == StateMode.Undo &&
            typeof(IMobileObject).IsAssignableFrom(property.Type) &&
            !typeof(IUndoableObject).IsAssignableFrom(property.Type))
          {
            data.Value = MobileFormatter.Deserialize((byte[])value.Value);
          }

          else data.Value = value.Value;

          if (!value.IsDirty)
            data.MarkClean();
        }


The problem I'm getting is an error coming out of the MobileFormatter.Deserialize call - this is because the value.Value is null.

Its kind of hard to duplicate this error, but I'll try explain whats happening.

I have an editable root with a property that is of type readonly child.
On fetch, I am not loading any value into this property, so the business object comes back to Silverlight with that readonly property as null.
At this point I can call BeginEdit and then CancelEdit on the root object with no problems, but this is because the readonly property is being excluded from the _propertyList array inside the FieldDataManager.

However, I then hook up the root object to some controls via data binding and I think somewhere along the way, the readonly child property gets a Set called on it.  Either way, the property is suddenly included in _propertyList.
Note though that the readonly property remains null when I check its value.

So basically if I then try to do a BeginEdit, then a CancelEdit after databinding I get the Deserialize error.

These are my findings from stepping through the code quite a bit.

Good news is the fix is really easy:

Change:
         if (mode == StateMode.Undo &&
            typeof(IMobileObject).IsAssignableFrom(property.Type) &&
            !typeof(IUndoableObject).IsAssignableFrom(property.Type))
          {
            data.Value = MobileFormatter.Deserialize((byte[])value.Value);
          }


To:
         if (value.Value != null &&
             mode == StateMode.Undo &&
             typeof(IMobileObject).IsAssignableFrom(property.Type) &&
             !typeof(IUndoableObject).IsAssignableFrom(property.Type))
          {
            data.Value = MobileFormatter.Deserialize((byte[])value.Value);
          }


That way you never have a case where a null value is being fed into the Deserialize call.

Any chance this could be included in the next CSLA release as a fix?

RockfordLhotka replied on Friday, February 13, 2009

Thanks, added to bug list

http://www.lhotka.net/cslabugs/edit_bug.aspx?id=346 

Killian35 replied on Monday, April 02, 2012

Hello,

In Silverlight, this minor bug will still creep in if the property type is a Nullable IMobileObject. For instance having a nullable SmartDate will cause the undo to fail if this property is not set to null. It is easy to replicate. I created this Root class:

    [Serializable]
    public class Root : BusinessBase<Root>
    {
        public static readonly PropertyInfo<SmartDate?> TodayProperty = RegisterProperty<SmartDate?>(c => c.Today);
        public SmartDate? Today
        {
            get { return GetProperty(TodayProperty); }
            set { SetProperty(TodayProperty, value); }
        }
 
        public override void DataPortal_Create(Csla.DataPortalClient.LocalProxy<Root>.CompletedHandler handler)
        {
            Today = new SmartDate();
            base.DataPortal_Create(handler);
        }
    }

and in a Silverlight button code-behind I had this code:
        private void button1_Click(object sender, RoutedEventArgs e)
        {
            DataPortal.BeginCreate<Root>((s, r) =>
                                             {
                                                 var root = r.Object;
 
                                                 root.BeginEdit();
                                                 root.CancelEdit();
                                             },
                                         DataPortal.ProxyModes.LocalOnly);
        }

It seems to error in the same error as the previous code was failing during deserialization of Undo.

Killian35 replied on Tuesday, April 03, 2012

Hello,

After further investigation, I found where the problem lies. By changing the following code in the FieldDataManager.OnSetState

                    if (value.Value != null &&
                      mode == StateMode.Undo &&
                      typeof(IMobileObject).IsAssignableFrom(property.Type) &&
                      !typeof(IUndoableObject).IsAssignableFrom(property.Type))
                    {
                        data.Value = MobileFormatter.Deserialize((byte[])value.Value);
                    }
                    else data.Value = value.Value;
to
                    if (value.Value != null &&
                      mode == StateMode.Undo &&
                      typeof(IMobileObject).IsAssignableFrom(Nullable.GetUnderlyingType(property.Type) ?? property.Type) &&
                      !typeof(IUndoableObject).IsAssignableFrom(Nullable.GetUnderlyingType(property.Type) ?? property.Type))
                    {
                        data.Value = MobileFormatter.Deserialize((byte[])value.Value);
                    }
                    else data.Value = value.Value;
the issue of InvalidCastException with nullable types is resolved. Is this a fix that can be incorporated in a future release?

Thanks.

sergeyb replied on Friday, April 06, 2012

Could you guys please specify what version of CSLA you are using?  I wanted to take care of this issue in the latest at lease, but I would like to investigate the problem some more and write up a test case that fails.

 

Thank you.

Killian35 replied on Friday, April 06, 2012

Sorry for not including the versions of everything. I'm using CSLA 4.3.10 and Silverlight 5.

sergeyb replied on Sunday, April 08, 2012

Thanks.

I added unit test to verify the bug and the fix.  THe fix has been checked into 4.3.x branch along with th tests.  I wrote up new bug - http://www.lhotka.net/cslabugs/edit_bug.aspx?id=1039.  I am planning to make the same fix in the trunk as well, as soon I set up environment on my Win 8 box for CSLA work.

sergeyb replied on Sunday, April 08, 2012

Now the same change is in trunk as well.

Copyright (c) Marimer LLC