Nullable readonly structs are not deserialized with the corresponding constructor call

[feO2x]

We've successfully used ExtendedXmlSerializer in one of our projects with quite a complicated object graph. During testing, I found another issue that might be a bug. Please consider the following minimal, complete, verifiable example (MCVE):

public sealed class NullableReadonlyStructSerializationTests
{
    private readonly IExtendedXmlSerializer _serializer =
        new ConfigurationContainer().UseOptimizedNamespaces()
                                    .EnableParameterizedContentWithPropertyAssignments()
                                    .UseAutoFormatting()
                                    .Create();
                                        
    [Fact]
    public void SerializeAndDeserializeInstanceA()
    {
        var a = new A { Point = new Point(2, 3)} ;

        var xml = _serializer.Serialize(a);
        var deserializedA = _serializer.Deserialize<A>(xml);

        Assert.Equal(a.Point, deserializedA.Point); // This test works fine!
    }

    [Fact]
    public void SerializeAndDeserializeInstanceB()
    {
        var b = new B { NullablePoint = new Point(24, 7)};

        var xml = _serializer.Serialize(b);
        var deserializedB = _serializer.Deserialize<B>(xml);

        Assert.Equal(b.NullablePoint, deserializedB.NullablePoint);
        // This test breaks because the deserialized point is (0,0) instead of (24, 7)
    }
}

public sealed class A
{
    public Point Point { get; set; }
}

public sealed class B
{
    public Point? NullablePoint { get; set; }
}

public readonly struct Point : IEquatable<Point>
{
    public Point(int x, int y)
    {
        X = x;
        Y = y;
    }

    public int X { get; }

    public int Y { get; }

    public bool Equals(Point other) => X == other.X && Y == other.Y;

    public override bool Equals(object? obj) => obj is Point other && Equals(other);

    public override int GetHashCode()
    {
        unchecked
        {
            return (X * 397) ^ Y;
        }
    }

    public override string ToString() => $"({X}, {Y})";

    public static bool operator ==(Point left, Point right) => left.Equals(right);

    public static bool operator !=(Point left, Point right) => !left.Equals(right);
}

In the example above, Point is implemented as an immutable struct. When I use this struct as a regular property in a class (as can be seen in class A), everything works fine. When I use it as a Point?, then the corresponding value is deserialized as (0,0) instead of (24, 7). On the IExtendedXmlSerializer instance, EnableParameterizedContentWithPropertyAssignments is configured among some other settings (but I don't think they are the cause of this behavior).

I suspect that when the XML is deserialized as Point?, the default object initializer of the struct is called instead of the constructor that takes two arguments. Might this be the case? Or did I miss something when configuring the serializer?

Thank you so much for your help in advance!

[issue-label-bot]

Issue-Label Bot is automatically applying the label bug to this issue, with a confidence of 0.77. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

[create-issue-branch]

Branch issues/fix/i477 created!

[Mike-E-angelo]

WOW @feO2x I am astounded that this doesn't work as expected! Actually I am not sure what is more astounding, the fact that this doesn't work, or that it's taken someone this long to point it out. 😅

I will look into this now and see if I can track down what is going on. Thank you for reporting it. 👍

[Mike-E-angelo]

Gosh, I have been spending more time trying to figure out how we could have missed this issue up until this point than actually seeing what's wrong.

It looks like we cover:

• Nullable primitives.
• Structures (e.g. writable Point)

But we do not cover:

• Nullable Structures.

To further summarize, this is what we have tests against:

class NullableSubject
{
	public int? Number { get; set; }
}

class PointProperty
{
	public System.Windows.Point Point { get; set; }
}

But we do not have something like this:

class NullablePointProperty
{
	public System.Windows.Point? Point { get; set; }
}

Or basically what was submitted here in the original issue.

This somewhat solves my confusion/astonishment over how this hasn't been tracked yet. Now onto seeing if we can actually fix this.

Here goes nothing. :P

[Mike-E-angelo]

I should also add that the above is based on the default serializer, without any extensions applied to it. The reason why this class works in the provided issue:

public sealed class A
{
    public Point Point { get; set; }
}

Is that the Point class just so happens to abide by the rules for EnableParameterizedContent* and gets picked up there. Otherwise, it would fail much like the public sealed class B does.

Sorry, it's seriously tripping me out that this isn't covered. 😅 I guess part of my surprise is that I use a lot of readonly structs -- like constantly -- so I am assuming that they would work. I guess they do, it's their nullable cohorts that are providing the friction here. Regardless, I was fully under the very wrong impression that they, too, are supported.

[feO2x]

No worries! I'm very happy that ExtendedXmlSerializer actually supports these scenarios that we have. Most other serializers don't, and that's why there is a lot of programming against dumb DTOs.

[Mike-E-angelo]

Alrighty @feO2x check out this build:

#478 (comment)

If it works for you we'll get this deployed to NuGet on Tuesday. Otherwise please let me know of any issues you find and I will look into them for you. 👍

[feO2x]

Thanks for the quick fix of the problem. On a side note, I tried to install this package into a fresh project, but I did get the following error:

Do I need to use a specific project type?

It's not that important, I can also wait until Tuesday for the official update on NuGet.

Thanks for your help!

[Mike-E-angelo]

Doh @feO2x looks like you are running into a known issue here?

NuGet/Home#7189

I've had transient problems with that command as well. Sometimes it works, sometimes it doesn't. 🤷‍♂️ I'll update the template with the above issue.

[feO2x]

Well then, no worries. I updated the NuGet issue. Thanks for your help!

[Mike-E-angelo]

OK cool... I wanted to verify here that you were able to get to the preview build. If the Package Manager Console command doesn't work, you can put in the preview feed manually in the NuGet interface and that should work for you.

My motivation here is that, if possible, I would prefer to get confirmation from you with the preview build before deploying it to NuGet. :)

[robertsynoradzki]

I wanna throw my three cents here.

I have my serializer set up like this, because I need a specific date format:

IExtendedXmlSerializer serializer = new ConfigurationContainer()
   .Type<DateTime>()
      .Register()
      .Serializer()
      .ByCalling((writer, date) => writer.Content(date.ToString("yyyy-MM-dd")), null)
   .Create();

But while serializing a class like below I found disappointing result when using nullables:

class Test
{
   public DateTime  Date1 { get; } = DateTime.Now; // Formatted OK
   public DateTime? Date2 { get; } = null;         // Is not emitted = OK
   public DateTime? Date3 { get; } = DateTime.Now; // Completety ignores configured formatting
}

I thought "Okay, this is probably my fault, this library might be very type-strict and I need to define a serializer for nullable DateTime as well". So I did:

IExtendedXmlSerializer serializer = new ConfigurationContainer()
   .Type<DateTime>()
      .Register()
      .Serializer()
      .ByCalling((writer, date) => writer.Content(date.ToString("yyyy-MM-dd")), null)
   .Type<DateTime?>()
      .Register()
      .Serializer()
      .ByCalling((writer, date) => writer.Content(date?.ToString("yyyy-MM-dd") ?? ""), null)
   .Create();

And guess what? DateTime and DateTime? instances did format correctly, but now I got bazilion errors about ALL other nullable types (like decimal?, enum?) not being able to cast into... DateTime (sic)!! So, now I've found a bug - all generic nullable types are treated as one type Nullable class. Crazy, IMO. This needs to be fixed, and also I would check if this extends to all generic types, not only nullables.

I believe custom serializer set up for DateTime should take effect not only on DateTime instances, but also on those DateTime?, which are not null.

[Mike-E-angelo]

Doh are you saying this is occurring with the preview build @ensisnoctis or is this from using the current NuGet release (3.4.2)? We do not provide any sophisticated registration but that error does seem suspicious. I will look into it for you.

[feO2x]

@Mike-E-angelo I can now confirm that version 3.4.2.1-xufywhoa is fixing this issue.

[Mike-E-angelo]

Great, thank you for the update, @feO2x!

@ensisnoctis I took a look into your scenario and with the preview build above I was able to confirm that it works as expected with the following test:

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

Lines 11 to 39 in 431c00e

	[Fact]
	public void Verify()
	{
	var serializer = new ConfigurationContainer().Type<DateTime>()
	.Register()
	.Serializer()
	.ByCalling((writer, date) => writer.Content(date.ToString("yyyy-MM-dd")),
	null)
	.Type<DateTime?>()
	.Register()
	.Serializer()
	.ByCalling((writer, date) => writer.Content(date?.ToString("yyyy-MM-dd") ?? string.Empty), null)
	.Create()
	.ForTesting();
	var instance = new Test();
	var content = serializer.Serialize(instance);
	content.Should().Be(@"<?xml version=""1.0"" encoding=""utf-8""?><Issue477Tests_Extended-Test xmlns=""clr-namespace:ExtendedXmlSerializer.Tests.ReportedIssues;assembly=ExtendedXmlSerializer.Tests.ReportedIssues""><Date1>2020-11-23</Date1><Date3>2020-11-23</Date3></Issue477Tests_Extended-Test>");
	}
	class Test
	{
	public DateTime Date1 { get; set; } = DateTime.Now; // Formatted OK
	public DateTime? Date2 { get; set; } = null; // Is not emitted = OK
	public DateTime? Date3 { get; set; } = DateTime.Now; // Completety ignores configured formatting
	}

Do note that I had to modify your Test class to have writable properties, otherwise they are not regarded as eligible for serialization.

If you are still encountering problems, please open up a new issue with a verifiable test that demonstrates a failure and I will look into it further for you.

As for registrations, I agree that having to register for both structures and nullable structures is a bit burdensome. I would recommend creating an extension method for this that allows for two commands in one. If you do create something that you feel would be of value please create a PR and we can discuss on integrating into the repro for a future release. 👍

[Mike-E-angelo]

Alrighty, this has been deployed to NuGet:

https://www.nuget.org/packages/ExtendedXmlSerializer/

Please do let me know of any further issues you encounter and I will take a look into it for you. Closing this issue for now.

ExtendedXmlSerializer 3.4.3

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