Deserialization of inherited class with different namespaces not possible with Migration

[imperiobadgo]

Hello and thank you again for continous great work. I have found a problem with the migration, which does not allow current saves to be loaded. This problem is only invoked with different namespaces of base and inherited classes. I have created a small example showing the problem:

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

namespace ExtendedXmlSerializer.Tests.ReportedIssues
{
    public sealed class MigrationTest
    {
        [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 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 { }

}

I think is has to do something with the inner workings of the MigrationsExtensions of reading the subtree of the current element and Namespaces that are already known are ignored.

[create-issue-branch[bot]]

Branch issues/fix/i612 created!

[Mike-E-angelo]

Hi @imperiobadgo thank you for reporting this. I can confirm there appears to be an issue here and have reproduced it on my side.

Unfortunately, my time these days for this project are next to nothing so I will not be able to look into this with the care it deserves. However, if you or someone want to create a PR that addresses this, I would be happy to work with you/them to do so. :)

FWIW as a potential workaround, I was able to get to work by enabling implicit typing on the type in question. Not sure if this is viable but wanted to mention it:

var serializer = new ConfigurationContainer().Type<BaseNamespace.Container>()
                                                             .AddMigration(EmptyMigration.Default)
                                                             .EnableImplicitTypingFromNamespace(typeof(InheritNamespace.InheritCheck))
                                                             .Create()
                                                             .ForTesting();

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

[imperiobadgo]

Thank you for your answer. I was able to solve my current problem with your workaround, but I would still like to tackle the actual problem with the migration.

I think I have found the problem in IdentityMapper because it does not go through all the existing namespaces. At the moment, it only checks whether there is an instance in the specified namespace. However, if no namespace is specified (which is the case here), it produces an incorrect result.

using System.Xml;
using ExtendedXmlSerializer.ContentModel.Identification;

namespace ExtendedXmlSerializer.ExtensionModel.Xml
{
    sealed class IdentityMapper : IIdentityStore
    {
        readonly IIdentityStore        _store;
        readonly IXmlNamespaceResolver _manager;

        public IdentityMapper(IIdentityStore store, IXmlNamespaceResolver manager)
        {
            _store   = store;
            _manager = manager;
        }

        public IIdentity Get(string name, string identifier) => _store.Get(name, _manager.LookupNamespace(identifier));
    }
}

The contents of the parameter of the get function: name: "InheritCheck" identifier: ""

So the identity inside the TypePartReflector contains; Identifier: clr-namespace:BaseNamespace;assembly=ExtendedXmlSerializer.Tests.ReportedIssues Name: "InheritCheck"

The Namespace is the wrong one, so we get the ParseException...

sealed class TypePartReflector : CacheBase<TypeParts, TypeInfo>, ITypePartReflector
{
    readonly IIdentityStore _identities;
    readonly ITypes         _types;

    public TypePartReflector(IIdentityStore identities, ITypes types) : base(TypePartsEqualityComparer.Default)
    {
        _identities = identities;
        _types      = types;
    }

    protected override TypeInfo Create(TypeParts parameter)
    {
        var identity  = _identities.Get(parameter.Name, parameter.Identifier);
        var typeInfo  = _types.Get(identity);
        var arguments = parameter.GetArguments();
        var type = arguments.HasValue
                       ? typeInfo.MakeGenericType(Arguments(arguments.Value))
                                 .GetTypeInfo()
                       : typeInfo;
        if (type == null)
        {
            throw new ParseException(
                                     $"An attempt was made to parse the identity '{IdentityFormatter.Default.Get(identity)}', but no type could be located with that name.");
        }

        var result = parameter.Dimensions.HasValue
                         ? new DimensionsAlteration(parameter.Dimensions.Value).Get(type)
                         : type;
        return result;
    }

    Type[] Arguments(ImmutableArray<TypeParts> names)
    {
        var length = names.Length;
        var result = new Type[length];
        for (var i = 0; i < length; i++)
        {
            result[i] = Get(names[i])
                .AsType();
        }

        return result;
    }
}

Can you help me with how to go through all the namespaces already found to find the correct namespace for the type if none is specified?

[Mike-E-angelo]

Awesome, thank you @imperiobadgo for taking the time to investigate! I might actually be able to dive into that and see if I can figure out what is wrong there. I'll see if I can carve some time today and will update you here. 👍

[Mike-E-angelo]

Alright @imperiobadgo I've tracked it down. The issue is indeed with XmlNamespaceManager and processing that is applied when migrations are enabled.

To start, this is what is emitted:

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

In particular, note the xmlns="" for both the root element and Inherit element.

When doing migrations, we actually go through and pull all detected namespaces out and add them to the new subreader. You can find this code here:

https://github.com/ExtendedXmlSerializer/home/blob/master/src/ExtendedXmlSerializer/ExtensionModel/Xml/MigrationsExtension.cs#L118-L149

The problem is that xmlns="" is declared twice and to assign it a second time would override the definition applied at the root element.

There may be a way to address this, and I am open to exploring this. However, it got me wondering if we have anything that forces new namespace prefixes and indeed this occurs when UseOptimizedNamespaces is applied. This will pull up all detected namespaces and apply them on the root element:

var serializer = new ConfigurationContainer().UseOptimizedNamespaces()
                                                             .Type<BaseNamespace.Container>()
                                                             .AddMigration(EmptyMigration.Default)
                                                             .Create()
                                                             .ForTesting();

Emits:

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

As there are no longer conflicts, the proper prefix is added to the namespace manager and the namespaces are resolved as expected.

This doesn't attend to the core issue of the migration extension. I am open to suggestions on how to proceed with this. For some awareness/context it is a v1 feature that was pulled over to v2 and my understanding of it is limited. My basic exposure to this extension was to get existing tests passing from v1 to v2 (and v3). 😇

[imperiobadgo]

After a lot of trial and error, I finally understood this XmlNamespaceManager at least a bit and was able to find a way to fix the problem. I also tried to map other namespace scenarios with the unit tests so that I don't accidentally break another constellation. Please have a look if the small change is ok -> #613

[Mike-E-angelo]

Awesome! Thank you @imperiobadgo for your efforts here. I am currently slammed with my main project but I should have some time today or tomorrow to check this out. The biggest part will be to get our connection back online with NuGet so we can publish a new release as its been so long our API key has expired. 😅

[Mike-E-angelo]

Thank you again for your contributions!