MarimerLLC / cslaforum

Discussion forum for CSLA .NET
https://cslanet.com
Other
31 stars 6 forks source link

Blazor ViewModel GraphMerger error on loading managed property #940

Closed JacoJordaan closed 4 years ago

JacoJordaan commented 4 years ago

Question I have not used the GraphMerger before and with the Blazor ViewModel this is build into the save. With my current business objects I now get the following error on loading of the property. "Attempt to read/load private field property in managed properties."

Not sure if this is a bug or something wrong with my property declaration?

Error are thrown here:

        protected override void LoadProperty(IPropertyInfo propertyInfo, object newValue)
        {
            if (propertyInfo == CustomerIdProperty)
            {
                if (ReadProperty(CustomerIdProperty) != (string)newValue)
                    PreviousCustomerId = ReadProperty(CustomerIdProperty);
            }

            base.LoadProperty(propertyInfo, newValue); //<--Error are thrown here
        }

Property that is causing the error, declaration as follows:

        public static readonly PropertyInfo<string> ApprovalResultDisplayProperty = RegisterProperty<string>(c => c.ApprovalResultDisplay, RelationshipTypes.PrivateField);
        [NotUndoable, NonSerialized]
        private string _approvalResultDisplay = string.Empty;
        public string ApprovalResultDisplay
        {
            get
            {
                _approvalResultDisplay = IntegrationApprovalResultList.GetItemByKey(((int)ApprovalResult).ToString()).Value;
                return GetProperty(ApprovalResultDisplayProperty, _approvalResultDisplay);
            }            
        }

Viewmodel code:

protected virtual async Task<T> DoSaveAsync()
        {
            if (Model is Csla.Core.ISavable savable)
            {
                var result = (T)await savable.SaveAsync();
                if (Model is Csla.Core.IEditableBusinessObject editable)
                    new Csla.Core.GraphMerger().MergeGraph(editable, (Csla.Core.IEditableBusinessObject)result); //<-- error on MergeGraph
                else
                    Model = result;
            }
            return Model;
        }

Version and Platform CSLA version: 5.1.0 OS: Windows, Linux Platform: Blazor

rockfordlhotka commented 4 years ago

This is because MobileFormatter has no concept of the NonSerialized attribute, nor any way to flag a property so it is not serialized.

Right now your workaround is to use a managed property, not a private backing field, and allow it to serialize.

JacoJordaan commented 4 years ago

Thanks, that explains it. Although, I am a bit confused how the normal serialization for saving of object then works which is also using the MobileFormatter?

rockfordlhotka commented 4 years ago

No, you are right - this isn't a MobileFormatter thing at all.

The problem is this line that calls LoadProperty.

Or more accurately a limitation of the overloads of LoadProperty in the ObjectFactory class:

https://github.com/MarimerLLC/csla/blob/master/Source/Csla.Shared/Server/ObjectFactory.cs#L120

Your property is truly read-only - it has no setter, and it uses a private backing field, so the LoadProperty method has absolutely no way to set the value of the property.

I do think this is related somewhat to the limitation of MobileFormatter where we have no way to mark a property as "nonserialized". No way to indicate that the property should be ignored.

The workaround thus far has been that such a property shouldn't be managed at all. If there's no registered IPropertyInfo for a property it is ignored by pretty much everything (except BinaryFormatter and NDCS, which follow their own rules).

JacoJordaan commented 4 years ago

Does this mean that with the MobileFormatter that this property will be serialized when doing a normal save operation? Or is that determined by the OnGetState and OnSetState methods for private fields?

I just want to make sure that there is no unnecessary data serialized and at the same time want to have the csla property info fields for binding support.

rockfordlhotka commented 4 years ago

@JacoJordaan regarding serialization, I'm pretty confident that the private backing field is not serialized unless you explicitly implement the onget/onset state overrides and manually move the field value through the serialization stream.

This code establishes that the City value is not cloned.

using Csla;
using System;

namespace ConsoleApp2
{
  class Program
  {
    static void Main(string[] args)
    {
      var obj1 = DataPortal.Create<PersonEdit>();
      obj1.Name = "Rocky";
      obj1.City = "Eden Prairie";
      var obj2 = obj1.Clone();
      Console.WriteLine($"clone Name {obj2.Name}");
      Console.WriteLine($"clone City {obj2.City}");

      Console.ReadLine();
    }
  }

  [Serializable]
  public class PersonEdit : BusinessBase<PersonEdit>
  {
    public static readonly PropertyInfo<string> NameProperty = RegisterProperty<string>(nameof(Name));
    public string Name
    {
      get => GetProperty(NameProperty);
      set => SetProperty(NameProperty, value);
    }

    public static readonly PropertyInfo<string> CityProperty = RegisterProperty<string>(nameof(City), RelationshipTypes.PrivateField);
    [NonSerialized]
    private string _city = CityProperty.DefaultValue;
    public string City
    {
      get => GetProperty(CityProperty, _city);
      set => _city = value;
    }
  }
}
rockfordlhotka commented 4 years ago

Back to the original question, I think maybe the answer is for GraphMerger to handle properties defined with RelationshipTypes.PrivateField differently.

Specifically, I can change the behavior to invoke the property setter directly. This would require you implement at least a private setter.

I think this is probably the right answer.

With this change the following code works

      var obj1 = DataPortal.Create<PersonEdit>("Rocky", "Eden Prairie");
      var obj2 = DataPortal.Create<PersonEdit>();
      new Csla.Core.GraphMerger().MergeGraph(obj2, obj1);
      Console.WriteLine($"clone Name {obj2.Name}");
      Console.WriteLine($"clone City {obj2.City}");

Resulting in this output

clone Name Rocky
clone City Eden Prairie

So we can see it did merge obj1 into obj2, including the City property.

This is with the City property declared like this:

    public static readonly PropertyInfo<string> CityProperty = RegisterProperty<string>(nameof(City), RelationshipTypes.PrivateField);
    [NonSerialized]
    private string _city = CityProperty.DefaultValue;
    public string City
    {
      get => GetProperty(CityProperty, _city);
      private set => _city = value;
    }
JacoJordaan commented 4 years ago

@rockfordlhotka Yes, this will resolve the issue.

Thank you for also clarifying the serialization question.