Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialization of generics - Xml error #1073

Closed
mennodeij opened this issue Jul 25, 2017 · 5 comments
Closed

Serialization of generics - Xml error #1073

mennodeij opened this issue Jul 25, 2017 · 5 comments
Assignees
Milestone

Comments

@mennodeij
Copy link
Contributor

When I run the test in the code below, I get an exception when I use the XmlSerializer (using the BinarySerializer - no problem). The message is

Expected: No Exception to be thrown
But was:  <System.Xml.XmlException: The '`' character, hexadecimal value 0x60, cannot be included in a name.

It looks like the back-tick that is used in generic names is not compatible with the XmlSerializer.

The test code:

using System;
using System.IO;
using System.Runtime.Serialization;
using Catel.Data;
using Catel.Logging;
using Catel.Runtime.Serialization;
using NUnit.Framework;


namespace CatelTest
{
    [TestFixture]
    public class SerializationTest
    {
        [OneTimeSetUp]
        public void StartLogging()
        {
            LogManager.AddListener(new ConsoleLogListener());
            LogManager.IsDebugEnabled = false;
        }

        [Test]
        public void Test()
        {
            A a = new A {Count = 3};
            B<A> ba = new B<A>(a);

            var ser = SerializationFactory.GetXmlSerializer();

            MemoryStream ms = new MemoryStream();
            Assert.DoesNotThrow(() => ser.Serialize(ba, ms, new SerializationConfiguration()));

            Console.WriteLine("{0} bytes in stream", ms.Length);

            ms.Position = 0L;

            B<A> deserialized = null;
            Assert.DoesNotThrow(() => deserialized = ser.Deserialize<B<A>>(ms, new SerializationConfiguration()));

            Assert.IsNotNull(deserialized?.Item);
            Assert.That(deserialized.Item.Count, Is.EqualTo(3));
        }
    }

    [Serializable]
    public class A : SavableModelBase<A>
    {
        protected A(SerializationInfo info, StreamingContext context)
        {
            Count = info.GetInt32("Count");
        }

        public override void GetObjectData(SerializationInfo info, StreamingContext context)
        {
            info.AddValue("Count", Count);
        }

        public A()
        {
        
        }

        [IncludeInSerialization]
        public int Count { get; set; }
    }

    [Serializable]
    public class B<T> : SavableModelBase<B<T>>
        where T : A
    {
        protected B(SerializationInfo info, StreamingContext context)
        {
            Item = info.GetValue("Item", typeof(T)) as T;
        }

        public override void GetObjectData(SerializationInfo info, StreamingContext context)
        {
            info.AddValue("Item", Item);
            base.GetObjectData(info, context);
        }

        public B()
        {
            ;
        }

        [IncludeInSerialization]
        public T Item { get; }

        public B(T item)
        {
            Item = item;
        }
    }
}
@GeertvanHorrik
Copy link
Member

Good point, so when storing the type name, we should use replace ` with something else. Interested in doing a PR?

@mennodeij
Copy link
Contributor Author

I'm not going to do a PR, I can only partially build Catel here and my git-fu is weak. Instead, I propose the following changes below. The assumption, that seems valid from the test passing, is that using XmlConvert.EncodeName on the element is that the type is not deduced from the element name once it has been serialized to XML. The type is given to the serializer and the parameterless constructor is used to create a new instance of the type to deserialize. Therefore it seems safe to convert B`1 to B_0x0060_1 and not worry about anyone needing to decode this at a later point.

XmlSerializer.GetXmlElementName replacement of function by

protected string GetXmlElementName(Type modelType, object model, string memberName)
{
    string xmlElementName = null;
    try
    {
        XmlRootAttribute xmlRootAttribute;
        if (ShouldSerializeAsCollection(modelType))
        {
            xmlElementName = CollectionName;
        }

        else if (modelType.TryGetAttribute(out xmlRootAttribute))
        {
            xmlElementName = xmlRootAttribute.ElementName;
        }

        else if (!string.IsNullOrWhiteSpace(memberName))
        {
            var propertyDataManager = PropertyDataManager.Default;
            if (propertyDataManager.IsPropertyNameMappedToXmlElement(modelType, memberName))
            {
                xmlElementName = propertyDataManager.MapPropertyNameToXmlElementName(modelType, memberName);
            }
        }
        else
        {
                
            xmlElementName = modelType.Name;
        }
    }
    finally
    {
        if (null != xmlElementName)
            xmlElementName = XmlConvert.EncodeName(xmlElementName);
    }
    return xmlElementName;
}

And a unit test to test this based on the unit test described earlier, but formatted similarly to other unit tests in the Catel project.

namespace Catel.Test.Runtime.Serialization.XmlSerialization
{
    using System.IO;
    using Catel.Data;
    using Catel.Runtime.Serialization;
    using NUnit.Framework;

    public class GenericSerializationFacts
    {
        public class A : SavableModelBase<A>
        {
            public A(int count)
            {
                _count = count;
            }

            public A()
            {
                ; //empty for deserialization
            }

            [IncludeInSerialization]
            private readonly int _count;
        
            public int Count
            {
                get { return _count; }
            }
        }

        public class B<T> : SavableModelBase<B<T>>
            where T : A
        {
            public B(T item)
            {
                _item = item;
            }

            public B()
            {
                ; //empty for deserialization
            }

            [IncludeInSerialization]
            private readonly T _item;

            public T Item
            {
                get { return _item; }
            }
        }


        [TestFixture]
        class GenericSerialization
        {
            [TestCase]
            public void TestGenericSerialization()
            {
                var a = new A(3);
                var b = new B<A>(a);

                using (var memoryStream = new MemoryStream())
                {
                    var ser = SerializationFactory.GetXmlSerializer();
                    Assert.DoesNotThrow(() => b.Save(memoryStream, ser));

                    memoryStream.Position = 0L;
                
                    B<A> deserialized = null;
                    Assert.DoesNotThrow(() => deserialized = ser.Deserialize<B<A>>(memoryStream));

                    Assert.IsNotNull(deserialized?.Item);
                    Assert.That(deserialized.Item.Count, Is.EqualTo(3));
                }
            }
        }
    }
}

@GeertvanHorrik
Copy link
Member

Thanks for the code. Currently adding the unit tests (converting it a bit so it gets tested against all serializers).

@GeertvanHorrik
Copy link
Member

Works like a charm now, will include this in v5 (beta 16). Thanks a lot for the contribution.

@GeertvanHorrik GeertvanHorrik modified the milestones: 5.0.0, Up for grabs Jul 26, 2017
GeertvanHorrik added a commit that referenced this issue Jul 26, 2017
…character, hexadecimal value 0x60, cannot be included in a name)
@GeertvanHorrik GeertvanHorrik self-assigned this Jul 26, 2017
@lock
Copy link

lock bot commented Aug 27, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants