Circular dependencies detection problem

[UladimirLameyko]

I got exception about circular dependencies, but there are no circular dependencies just same property types and same object. Just show code instead of 1000 words

using ExtendedXmlSerializer;
using ExtendedXmlSerializer.Configuration;

var sameInnerObject = new InnerObject { Id = 100 };
var testRootObject = new RootObject
{
    Item1 = sameInnerObject,
    Item2 = null,
    List = new List<InnerObject>
    {
        sameInnerObject
    }
};

var serialized = new ConfigurationContainer().Create().Serialize(testRootObject);
Console.WriteLine("Success");

public class RootObject
{
    // This is the same object type as Item2 but can have other value
    public InnerObject? Item1 { get; set; }

    // This is the same object type as Item1 but can have other value
    public InnerObject? Item2 { get; set; }

    //Should have the same object as Item1 or Item2
    public List<InnerObject> List { get; set; } = new();
}

public class InnerObject
{
    public int Id { get; set; }
}

[Mike-E-angelo]

Thank you for reporting this @UladimirLameyko I can confirm I am seeing the same thing here. It's weird too as if I remove Item2 the error goes away. 🤔

I will take a look into it. However, I am not sure how much of a priority I can make it ATM with both the holidays here and the fact that the last time we deployed something was over a year ago. 😬

[UladimirLameyko]

@Mike-E-angelo Thanks for your response It is strange, why only by having empty field is enough to create a problem. It is ok that it can take a while to fix, just want to help others do not face this definitely unique situation on production.😎

[Mike-E-angelo]

OK I've managed to take a look at this. There are a few issues here:

1. Before we run an instance (reference) graph check on the provided instance, we run a static type check. The static type check looks at all the properties on the type and determines if any of the properties can contain a circular reference. If this is true then the instance reference graph check is added to the serializer when instances are serialized. We check for member properties, but collections should be checked as well and they are not. This is a bug.
2. The way we are doing circular reference checks here seems to be inaccurate. As your graph clearly shows, there is no danger with serializing objects that appear more than once. However, our reference extension optimizes this process and will only emit the reference once in the graph, and then reference back to it whenever it does occur again.

1 is tricky but I was able to fix it pretty quickly. #2 is a bit of a problem. There are two scenarios here conflated into one:

1. Circular references
2. Multiple occurrences of the same reference

This is tricky because one (the first) is really bad and will cycle forever (hence the exception). The second isn't as bad but our reference extension optimizes the serialization/deserialization process by writing/reading the reference once and using "pointers" for subsequent occurrences. As such, it is a recommendation to do this rather than having the serializer/deserializer write/read the same instance multiple times in the graph.

Unfortunately, it does not appear that our circular reference detector actually checks for circular references 😭 It appears to only detect the same instance being found in a graph more than once.

I am currently eyeballing how much work it would take to make a true circular reference detector in a short amount of time. If I can do that then I will attempt this. Otherwise, I am a bit perplexed on how to handle this one. 🤔

[create-issue-branch[bot]]

Branch issues/fix/i583 created!

[Mike-E-angelo]

OK @UladimirLameyko this was sort of involved but I landed on a solution I am happy with. We now check to see if both Cyclical References are encountered as well as Multiple References.

These still both throw exceptions by default to stay with current design. However, if you would like to utilize multiple references without enabling references, you can do so with the new AllowMultipleReferences configuration method as demonstrated here: https://github.com/ExtendedXmlSerializer/home/blob/master/test/ExtendedXmlSerializer.Tests.ReportedIssues/Issue583Tests.cs#L46-L61

Now is the tricky part... to get this deployed. All sorts of expected values are broken in such regard ATM because keys and the like have expired. So I have to spend some time brushing off the dust and getting that in order.

However, in the meantime you can try the latest preview version as outlined here: https://github.com/ExtendedXmlSerializer/home/pull/586#issuecomment-1365446421

Please let me know if you have any questions/issues and I will do my best to assist. 👍

GitHub

home/Issue583Tests.cs at master · ExtendedXmlSerializer/home

A configurable and eXtensible Xml serializer for .NET. - home/Issue583Tests.cs at master · ExtendedXmlSerializer/home

[UladimirLameyko]

It is actually incredible to see your fix so fast. Thank you very much. 😎 Will you create a new release with that fix?

[Mike-E-angelo]

Thank you @UladimirLameyko it is appreciated. I barely have enough time to spend on this project now but there just so happened to be a window here with the holidays, so took advantage of it. I also figure that since it's been over a year now since anything has been reported it's better to attend to this now than never. 😄

Will you create a new release with that fix?

I will attempt to brush off the dust here with the deployments today and see if I can get something going here. I will let you know where if/when it is ready. 👍

[Mike-E-angelo]

Alright @UladimirLameyko you should be able to get the latest NuGet with this change here: https://www.nuget.org/packages/ExtendedXmlSerializer

Thank you for improving ExtendedXmlSerializer. Happy Holidays out there 🎄⛄🌟

ExtendedXmlSerializer 3.7.9

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