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

Deserialization issue with inherited class with different namespaces and Migration #614

Closed
imperiobadgo opened this issue Mar 20, 2024 · 2 comments

Comments

@imperiobadgo
Copy link

imperiobadgo commented Mar 20, 2024

In the course of testing the migration feature, I found another problem. In the following test setup, the first "Check" property is not set:

using ExtendedXmlSerializer.Configuration;
using ExtendedXmlSerializer.Tests.ReportedIssues.Support;
using FluentAssertions;
using System;
using System.Collections;
using System.Collections.Generic;
using System.Xml.Linq;
using Xunit;

namespace ExtendedXmlSerializer.Tests.ReportedIssues
{
    public sealed class Issue613Tests
    {
        [Fact]
        public void Verify()
        {
            var serializer = new ConfigurationContainer().Type<BaseNamespace.Container>()
                                                         .AddMigration(EmptyMigration.Default)
                                                         .Create()
                                                         .ForTesting();

            var container = new BaseNamespace.Container();
            container.Content.Add(new InheritNamespace.Inherit() { Check = new BaseNamespace.BaseCheck() });
            container.Content.Add(new BaseNamespace.Base() { Check = new InheritNamespace.InheritCheck() });
            serializer.Cycle(container).Should().BeEquivalentTo(container);
        }

    }

    sealed class EmptyMigration : IEnumerable<Action<XElement>>
    {
        public static EmptyMigration Default { get; } = new EmptyMigration();

        EmptyMigration() { }

        public IEnumerator<Action<XElement>> GetEnumerator()
        {
            yield break;
        }

        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
    }
}

namespace BaseNamespace
{

    public class Container
    {
        public List<Base> Content { get; set; } = new List<Base>();
    }

    public class Base
    {
        public BaseCheck Check { get; set; }
    }
    public class BaseCheck { }
}

namespace InheritNamespace
{

    public class Inherit : BaseNamespace.Base { }


    public class InheritCheck : BaseNamespace.BaseCheck { }

}

This is the resulting xml:

<?xml version="1.0" encoding="UTF-8"?>
<Container xmlns="clr-namespace:BaseNamespace;assembly=ExtendedXmlSerializer.Tests.ReportedIssues" exs:version="0" xmlns:exs="https://extendedxmlserializer.github.io/v2">
	<Content>
		<Capacity>4</Capacity>
		<Inherit xmlns="clr-namespace:InheritNamespace;assembly=ExtendedXmlSerializer.Tests.ReportedIssues">
			<Check/>
		</Inherit>
		<Base>
			<Check exs:type="ns1:InheritCheck" xmlns:ns1="clr-namespace:InheritNamespace;assembly=ExtendedXmlSerializer.Tests.ReportedIssues"/>
		</Base>
	</Content>
</Container>

The problem seems to come from the reader formatter, which returns "ns1:Check" as the name of the object, although only "Check" would be correct. I have updated my pull request #613 with this fix.

@Mike-E-angelo
Copy link
Member

Wow @imperiobadgo I am really amazed that this works and all tests pass. That's a pretty significant change, but I guess it's only used for member reading which expect a local name. It makes me wonder if this will address the hacky exs:member attribute we have lurking in our code but I am firmly in the "if it ain't broke don't fix it" mentality with this project. 😁

As all tests pass I will accept/merge/deploy these changes. Thank you your contributions!

@Mike-E-angelo
Copy link
Member

Oof... that was brutal. I was able to get NuGet deployed but apparently our documentation cannot be deployed now as the deploy key we used has expired and it is immediately unclear how to update it. I am deploying an attempt now let's hope that works. Closing these issues for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants