List<T> with complex objects (referential integrity, parameterized content) is not deserialized properly

[feO2x]

Hey guys,
thanks for ExtendedXmlSerializer and the love and effort you put into it, I really think it's a great XML serializer.

I'm currently testing if ExtendedXmlSerializer is able to handle a complex object graph for a new project. This project uses a node graph that can be cyclic. Each node has a list of all linked nodes, and these links can either be unidirectional (if only one of two nodes links to the other one) or bidirectional (if both nodes link to each other).

I tried to serialize and deserialize this object graph with ExtendedXmlSerializer, but I encountered an issue. Please take a look at the following test (which uses xunit and FluentAssertions):

public sealed class MissingItemsInCollectionTests
{
    private readonly ITestOutputHelper _output;

    public MissingItemsInCollectionTests(ITestOutputHelper output) => _output = output;

    [Fact]
    public void NodesListIsMissingNodes()
    {
        var xmlSerializer = new ConfigurationContainer().UseOptimizedNamespaces()
                                                        .EnableParameterizedContentWithPropertyAssignments()
                                                        .UseAutoFormatting()
                                                        .Type<Node>()
                                                        .EnableReferences(node => node.Id)
                                                        .Create();
        var node1 = new Node(1);
        var node2 = new Node(2).AddNode(node1, isBidirectional: true);
        var node3 = new Node(3).AddNode(node2, isBidirectional: false);
        node1.AddNode(node3, isBidirectional: false);
        var nodesList = new List<Node> { node1, node2, node3 };

        var xml = xmlSerializer.Serialize(new XmlWriterSettings { Indent = true, IndentChars = "    " }, nodesList);
        _output.WriteLine(xml);

        var deserializedNodesList = xmlSerializer.Deserialize<List<Node>>(xml);
        deserializedNodesList.Should().BeEquivalentTo(nodesList,  config => config.IgnoringCyclicReferences());
    }

    public sealed class Node : IEquatable<Node>
    {
        private readonly List<Node> _linkedNodes;

        public Node(int id) : this(id, null) { }

        public Node(int id, List<Node> linkedNodes)
        {
            Id = id;
            _linkedNodes = linkedNodes ?? new List<Node>();
        }

        public int Id { get; }

        public IReadOnlyList<Node> LinkedNodes => _linkedNodes;

        public Node AddNode(Node otherNode, bool isBidirectional = true)
        {
            if (otherNode == null)
                throw new ArgumentNullException(nameof(otherNode));
            if (ReferenceEquals(this, otherNode))
                throw new ArgumentException($"You cannot link a node {Id} to itself.");

            _linkedNodes.Add(otherNode);
            if (isBidirectional)
                otherNode.AddNode(this, false);
            return this;
        }

        public bool Equals(Node other) => !ReferenceEquals(null, other) && Id == other.Id;

        public override bool Equals(object obj) => ReferenceEquals(this, obj) || obj is Node other && Equals(other);

        public override int GetHashCode() => Id;

        public override string ToString() => "Node " + Id;
    }
}

In this test, i create three instances of the Node class and interconnect them. Node 1 has a bidirectional link to node 2 and a unidirectional link to node 3, node 3 links unidirectionally to node 2.

When I run this test, the produced XML looks fine, but when the I deserialize the XML back to an object graph, then the following stuff happens:

• deserializedNodesList contains [ node 1, null, null ]
  
• Node 2 does not link back to node 1
  
• Node 3 does not link to node 2
  

What keeps me wondering is that the link collections contain the right amount of entries, but all of them are null instead of the corresponding deserialized node instance. Did I forget to configure something properly, or is this is a bug in ExtendedXmlSerializer? Does this have something to do with #360 (although no structs are involved)?

Thanks for your help in advance.

[feO2x]

I've tested a further scenario and when EnableParameterizedContentWithPropertyAssignments is not configured on the XML serializer, it works flawlessly.

[Fact]
public void NodesDto()
{
    var xmlSerializer = new ConfigurationContainer().UseOptimizedNamespaces()
                                                    .UseAutoFormatting()
                                                    .Type<NodeDto>()
                                                    .EnableReferences(node => node.Id)
                                                    .Create();
    var node1 = new NodeDto { Id = 1 };
    var node2 = new NodeDto { Id = 2 }.AddNode(node1, isBidirectional: true);
    var node3 = new NodeDto { Id = 3 }.AddNode(node2, isBidirectional: false);
    node1.AddNode(node3, isBidirectional: false);
    var nodesList = new List<NodeDto>{ node1, node2, node3 };

    var xml = xmlSerializer.Serialize(new XmlWriterSettings { Indent = true, IndentChars = "    " }, nodesList);
    _output.WriteLine(xml);

    var deserializedNodesList = xmlSerializer.Deserialize<List<NodeDto>>(xml);
    deserializedNodesList.Should().BeEquivalentTo(nodesList, config => config.IgnoringCyclicReferences());
}

public sealed class NodeDto
{
    public int Id { get; set; }

    public List<NodeDto> LinkedNodes { get; set; }

    public NodeDto AddNode(NodeDto otherNode, bool isBidirectional = true)
    {
        if (otherNode == null)
            throw new ArgumentNullException(nameof(otherNode));
        if (ReferenceEquals(this, otherNode))
            throw new ArgumentException($"You cannot link a node {Id} to itself.");

        LinkedNodes ??= new List<NodeDto>();
        LinkedNodes.Add(otherNode);
        if (isBidirectional)
            otherNode.AddNode(this, false);
        return this;
    }

    public override string ToString() => "Node " + Id;
}

Of course, this means that the ID cannot be read-only, which might be bad when these instances are used in a dictionary or hash set.

[Mike-E-angelo]

Wow! Thank you for your detailed report and kind words, @feO2x! It is much appreciated.

To be honest, I am amazed that anything worked at all for you. 😆 Combining these two features has been identified as an incompatible feature combination with details provided in #360.

This is one of those inevitable gotchas with building an extensible system. Well, rather, the v1 version of such (that is, the extension system in place now was built from scratch for ExtendedXmlSerializer v2 and continues to this day). In a future version a better approach should be made to ensure that extensions fit better together and aware of the things they do to a given graph along with a capability matrix system, but alas, we do not have that currently.

Are you able to get what you need via your second provided configuration (without the use of EnableParameterizedContent*? Otherwise, this is an identified issue and it, unfortunately, cannot be addressed without a significant amount of work. By me, at least. 😁 (more @ #383)

[feO2x]

Credit where credit is due. I've read a few issues in your repo and the way you handle them as a maintainer is very friendly and exemplary.

BTW, I realized that you can even turn EnableParameterizedContentWithPropertyAssignments on and it would not have any negative consequences as long as the serialized instances that require referential integrity are mutable.

So I will leave my model mutable for now and tell my devs that they shouldn't change the ID when the corresponding instances are used in dictionaries.

I'm closing this issue as the problem is already captured in #360.

Thanks for your help!

[Mike-E-angelo]

Ahh thank you @feO2x that means a lot! Happy to get this resolved, one way or another. :) Thanks for stopping by and let us know if you run into any additional problems, particularly if they are easy to fix. 😁

[Mike-E-angelo]

Hey there @feO2x ... good news! I managed to figure out what is wrong here and have your test above passing in our suite now:

home/test/ExtendedXmlSerializer.Tests.ReportedIssues/Issue396Tests.cs

Lines 12 to 32 in f3e47da

	[Fact]
	public void NodesListIsMissingNodes()
	{
	var serializer = new ConfigurationContainer().UseOptimizedNamespaces()
	.EnableParameterizedContentWithPropertyAssignments()
	.UseAutoFormatting()
	.Type<Node>()
	.EnableReferences()
	.Create()
	.ForTesting();
	var node1 = new Node(1);
	var node2 = new Node(2).AddNode(node1, isBidirectional: true);
	var node3 = new Node(3).AddNode(node2, isBidirectional: false);
	node1.AddNode(node3, isBidirectional: false);
	var nodesList = new List<Node> { node1, node2, node3 };
	var cycled = serializer.Cycle(nodesList);
	cycled.Should().BeEquivalentTo(nodesList, config => config.IgnoringCyclicReferences());
	}

If interested, you can check out the build here:

#421 (comment)

Thought I would let you know. Please let me know if you run into any further problems and I can look into it for you. Re-opening and will close when pushed to NuGet. 👍

[Mike-E-angelo]

I am actually astounded to see this work, considering the complexity of the graph! 😅 From first sight I definitely think "yep that should totally break." 😂

[Mike-E-angelo]

Guess I didn't need to really re-open this, but in any case, the fix has been deployed to NuGet here:
https://www.nuget.org/packages/ExtendedXmlSerializer/

If you encounter any further issues please let me know and I will take a look into it for you. Closing (again) for now.

ExtendedXmlSerializer 3.2.4

An extensible Xml Serializer for .NET that builds on the functionality of the classic XmlSerializer with a powerful and robust extension model.

[feO2x]

Thanks for your efforts, I will take a look at it tomorrow!