2025-01-03

Parameterless Constructors and the Invalid Field

Do you know what is the difference between this record...

record MyAmazingRecord(
    string Id,
    SomeInnerData Data
) {
    public static readonly MyAmazingRecord Invalid = new("Invalid", new SomeInnerData(null));
    
    public MyAmazingRecord() : this(Invalid)
    {}
}

record SomeInnerData(string? Data);

...and this record...?

record MyAmazingRecord(
    string Id,
    SomeInnerData Data
) {
    public static readonly MyAmazingRecord Invalid = new();
    
    public MyAmazingRecord() : this("Invalid", new SomeInnerData(null))
    {}
}

record SomeInnerData(string? Data);

Turns out... quite a big one.

In the first case, every instance of MyAmazingRecord has the same instance of SomeInnerData in its Data property, because the auto-generated cloning constructor of records does a shallow clone. On the other hand, in the second case, each instance of MyAmazingRecord has its own SomeInnerData instance.

You might think "It shouldn't matter because SomeInnerData is immutable." or even "The first version is better. It reduces meaningless instancing of SomeInnerData." And you might be right...

...unless you JSON-deserialize instances of MyAmazingRecord!

If you use Newtonsoft.Json and rely on default behavior, the SomeInnerData serialization contract will have ObjectCreationHandling set to Auto. That means that, during deserialization, Newtonsoft.Json won't create a new instance of SomeInnerData for MyAmazingRecord.Data if one already got assigned to it in the parameterless constructor. Therefore, in the first version of MyAmazingRecord, it uses the instance from the Invalid field. Newtonsoft.Json doesn't care that MyAmazingRecord is a record type because records are just C# syntax sugar. Underneath, SomeInnerData.Data still has a setter and Newtonsoft.Json will use it to populate the instance with deserialized data. So... it changes it to whatever happens to be deserialized right now.

What this all means is that, in the first version, ALL deserialized instances of MyAmazingRecord have the same SomeInnerData.

How bad can that be? Oh boy... I found out about this in EntityPermissionEventProjection because users had permissions they were not supposed to have. And if that isn't bad enough, consider that I used the same parameter-less ctor cloning Invalid pattern for all of KAFE's records. I hope it didn't cause too much havoc because I have not way of finding out what this caused.

Glad that's fixed though.