Make ObservableObject serializable

[popejustice]

I was utilizing serialization for some easy copy/paste, but discovered after pulling this project in that observable object isn't serializable. That seems like something that wouldn't cause too much pain, but would make the class more flexible. Am I crazy?

[michael-hawker]

FYI @Sergio0694

Not sure if the new Source Generators help with this either if they're pulling in the same properties.

@popejustice can you describe a bit more the issue/error you're getting when trying to serialize your classes? What serializer are you using (newtonsoft, system.text.json, something else?)? Do you have an example class that can't be serialized?

[popejustice]

Hey, sorry for the delay, so I'm utilizing the built in System.Runtime.Serialization library to serialize objects into a binary stream. On most of the objects I'm utilizing the "Serializable" attribute (from the System namespace). The serialization library doesn't seem to play nice with events (which makes sense it would be a mess to include those references) so you have to explicitly mark events with the [ field: NonSerialized ] attribute, but I can't do that for observable object so the serializer chokes up when it hits propertychanged and propertychanging.

I've just copied observableobject into my project and made the modifications since the dependencies weren't bad. Seems to be working, but if I remove the nonserializable attribute from one of the events I get the following:

System.Runtime.Serialization.SerializationException: 'Type 'HiveAppModel.A429DefValidationService' in Assembly 'HiveAppModel, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' is not marked as serializable.'

thrown from the format.Serialize call here:

    /// <summary>
    /// Utilizes serialization to create a deep clone of the target object {source} the target object
    /// must implement the ISerializable interface or have the Serializable attribute
    /// </summary>
    /// <typeparam name="T">The type of the object to clone</typeparam>
    /// <param name="source">The object to clone from</param>
    /// <returns>A deep cloned instance of {source}</returns>
    public static T DeepClone< T >( this T source )
    {
        if( !typeof( T ).IsSerializable )
        {
            throw new ArgumentException( "The type must be serializable.", nameof( source ) );
        }

        if( ReferenceEquals( source, null ) )
        {
            return default;
        }

        IFormatter format = new BinaryFormatter();
        Stream stream = new MemoryStream();
        using( stream )
        {
            format.Serialize( stream, source );
            stream.Seek( 0, SeekOrigin.Begin );
            return (T)format.Deserialize( stream );
        }
    }

anyways, I've got a work around, but that seemed like a low impact change (just add the attributes?) and I'd prefer not have my jank 1-off version. Plus, it's the included serializer so that didn't seem that out there. Let me know if that helps.

[popejustice]

also sorry, for replication you could call that DeepClone function on any object that inherits from ObservableObject and that should recreate it.

[michael-hawker]

Thanks for the info @popejustice! It's these two events that need the [NonSerialized] attribute you're saying, right?

WindowsCommunityToolkit/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs

Lines 33 to 36 in ee93385

	public event PropertyChangedEventHandler? PropertyChanged;
	/// <inheritdoc cref="INotifyPropertyChanging.PropertyChanging"/>
	public event PropertyChangingEventHandler? PropertyChanging;

@Sergio0694 this seems straight-forward if it's just a matter of adding attributes on methods right? We'd just want to check the final binary size impact.

[Sergio0694]

BinaryFormatter is a fundamentally unsafe type that can expose your program to a number of security vulnerabilities, and as such has been marked as obsolete and it will be removed gradually as we move to .NET 8. You should never, ever use it in production code, and you should migrate away from it as well if possible. We don't plan on adding support for it in the MVVM Toolkit or in any other parts of the Community Toolkit as a whole at any time in the future. For more details on this, see here.

[popejustice]

@michael-hawker those were the ones, but it sounds like maybe my own deep clone implementation is the better route. Thanks @Sergio0694 I wasn't aware of the security vulnerability, but i guess that makes a good amount of sense. Appreciate both of you taking a look at this and the justification. Thanks!