In some scenarios, Vogen will happily deserialize a JSON null into a null-valued Value Object

[Steve-OH]

Describe the bug

IDE: Visual Studio 2022 Build target: .NET 8 Language: C# 12 Vogen: 4.0.8

Using System.Text.Json, if you use class or record for your value object, Vogen will deserialize a JSON null without complaint. This won't happen if you use struct or record struct. The reason for this is that the base JsonConverter class has a bool property, HandleNull, that defaults to true for struct or record struct, but false for class or record. When HandleNull is false, the converter is bypassed completely, allowing the null to pass through.

The solution is to add the following line to the converter templates:

public override bool HandleNull => true;

This will fix the problem for class and record, and will have no effect on struct or record struct.

Steps to reproduce

Compile and run the following simple console program:

namespace MyNamespace;

using System.Text.Json;
using Vogen;

[ValueObject<string>(Conversions.SystemTextJson)]
internal partial class Foo;

internal class Program
{
    static void Main()
    {
        var foo = JsonSerializer.Deserialize<Foo>("null");
        Console.WriteLine(foo);
    }
}

No exception will be thrown, and the value object foo will have a value of null.

Expected behaviour

Vogen should throw a ValueObjectValidationException with message "Cannot create a value object with null."

NOTE: I've put together a pull request with the fix applied to the string converter template, and with one additional test to demonstrate the before and after behavior, but the fix needs to be applied to the templates for all of the pimitive types, and I think the issue may also affect the Newtonsoft converter, and I just don't have the time right now to try to fix everything. Also, my Visual Studio and ReSharper seem to be in cahoots to auto-reformat everything, even though I've tried to turn it off completely, so making the smallest change to a file causes a gazillion lines to change.

Anyway, if you'd like me to submit it as-is, I'd be happy to do so, but it's definitely WIP.

[SteveDunn]

Hi, many thanks for the bug report and the PR. I'll take a look today. I know that there are some users who want nullable instances when deserialization fails. I remember there's a DeserializationStrictness flag, but I'll have to refresh my memory on what it does.

[Steve-OH]

I don't think DeserializationStrictness applies in this case. Here, the converter is being bypassed upstream of any Vogen code.

[SteveDunn]

Thanks @Steve-OH - This is fixed in 4.0.11, which will be released in a couple of hours. There is a new flag in DeserializationStrictness which disallows nulls. I kept it allowing nulls for backwards compatibility.