Required and optional properties are not enforced by the property models

[Fresa]

When creating an instance of a model using the static Create method, optional, and I guess nullable, properties are modeled in the method signature to be nullable/optional C# types. This is however not enforced by the property field's signatures, so if a model is read from json there is no way to determine if a property is required (and hence never null/undefined) unless one look at the static create method, the schema it self or check for undefined/null for every property. This makes it difficult to work with these properties. Would be great to get help from the compiler here.

Would be great if nullable and/or optional properties could be modeled as nullable using C# nullability.

Maybe this would render Undefined and Null values redundant completely, or at least only interesting internally, meaning all the extension methods that uses them could be removed, to further simplify using these objects. I can't figure out any scenario where one need to distinguish between them, except when serializing/deserializing, and that's internal built-in concern.

What do you think?

[mwadams]

This is a good area for discussion.

So - first undefined and null. We deliberately preserve this information in the public interface as there are plenty of use cases - often around 'patching' or 'setting state' scenarios where 'undefined' means 'leave it alone' and 'null' means 'set it to null'.

However, there's also a more philosophical point behind this. We are building models over the JSON rather than mapping from JSON to 'not JSON', and we want a full-fidelity two-way mapping. But with 'as convenient as possible' representation in the target domain (C#).

The scenario we found where we most often care about the optionality of a property (as opposed to the nullability of a property) is in creation. Here was use the presence or absence of C# nullability to indicate (non-)optionality of the value. Note that you can still pass an instance of an IJsonValue<T> whose value is JsonValueKind.Null to one of these optional parameters, and then validate appropriately once it is constructed.

So this is a slightly different problem: how does a developer know, given just the object model, whether the value of the instance returned by the property is allowed to be null, or not?

Once you have a property value, we have an assortment of ways of dealing with potentially nullable/undefined types (e.g. the extension method .AsOptional<T>() gives a nullable instance of the type that is null if the value provided by the property is null or undefined, plus the family .IsNullOrUndefined(), IsNotNull(), IsNotUndefined() etc.

This is common in a variety of scenarios. e.g. 'Dealing with the garbage you get in the real world' where you find you have an interop issue where someone claims that they conform to the schema, but you have to filter out half of their messages and transform them to deal with 'numbers as strings' or things like that; or when we are dealing with extensibility on open types.

However, for the well-known properties; is there anything we can do to help you determine whether you need to test for null or undefined, or not?

This then breaks down into two problems - whether they are allowed to be undefined and whether they are allowed to be null.

The undefined thing is, properly, the responsibility of the containing type that declares the property, and one way to deal with this would be to make optional properties an instance of a readonly record struct JsonOptional<T> where T : IJsonValue<T> which is very, very similar in intent to Nullable<T> but handles the undefined case instead of nullability (perhaps with HasValue: bool and Value: T properties); this would force you to consider the not-required case.

The nullability case is perhaps more complex, and I haven't fully thought it through. It may well be possible to look at a family of JsonNullable<T> and JsonOptionalNullable<T> which gives you a strong indicator that you need to be careful with these types - but I'm not yet sure about that.

@idg10 may have some thoughts on this.

[idg10]

This is not exactly saying anything @mwadams hasn't already said but possibly offering a different perspective.

There's a thing we could conceivably do, but currently don't: in principle we could effectively define two parallel object models:

1. definitely known to be valid
2. might be invalid

An extreme version of this design philosophy might say: we won't give you any sort of statically typed accessors until it has been proven that everything is valid according to the schema. So perhaps you might write:

UnvalidatedModel um = UnvalidatedModel.FromJson(json); if (um.TryValidate(out ValidatedModel vm)) { Console.WriteLine($"Id: {vm.Id}, name: {vm.Name}"); }

And if the schema were to be defined in such a way that these Id and Name must be present and must have string values, then you could imagine that being somehow reflected in the generated .NET types. (A simple approach would be for them to be available as properties of type string, although there are low-allocation scenarios for which that's not ideal.)

But if you go down this extreme path, it rules out the "This isn't valid according to the schema, but we're going to do give it our best effort to process the thing anyway." There are certainly scenarios where that's a bad idea, but there are also reasonable scenarios where you do want to support this.

An all-or-nothing approach where you don't get any wrapper properties if validation didn't succeed doesn't work for this scenario. Now you could imagine that maybe the UnvalidatedModel offers access to everything described in the schema too. But now, would you ever use the ValidatedModel? If you're able to work with the UnvalidatedModel for the best effort case, it probably doesn't make sense to write the code twice, once for that and once for ValidatedModel. Sure, apps that aren't going to attempt this will just use ValidatedModel, and perhaps ones that are will just use UnvalidatedModel, so it might not be entirely mad to offer this kind of parallel model. But it would probably be confusing.

So we currently have this system where we generate one model, it will perform validation if you ask it to, but it won't actually stop you using it if validation fails (or if you didn't even ask it to validate).

Given that design choice, you can't usefully project a "not null" guarantee made by the schema into C#, because it's actually a case of "The schema says this won't be null but it might be because the document might not conform to the schema."

Admittedly, that's not actually so far off from string? vs string. The latter means "shouldn't ever be null but there are no guarantees". But that's a very specific C#ism, and it doesn't feel like a great idea to me to try to identify that with the notions of "might be there" that you get in the world of JSON. There are similarities but they aren't the same.

So I think that's why, if we were to make this kind of schema-derived knowledge available, it would probably need to be expressed through specific .NET types defined by Corvus with known semantics (such as the JsonOptionalNullable<T> Matthew suggested). The whole "it should be this but it might not be" nature of everything in this world, combined with the fact that JSON has more flavours of "this isn't here" than C# does means you can't really express these things with what you might think of as "natural" C# types.

I do sometimes wonder if there might be mileage in an "If it doesn't validate I'm not even going to look at it" mode, and whether that might enable simpler, more natural mappings. But I think it would be quite hard to make that a switchable thing, because the assumption that you are at the end of the day dealing with JSON goes quite deep.

[mwadams]

I'm thinking of something like this: Optional:

// <copyright file="JsonOptional{T}.cs" company="Endjin Limited">
// Copyright (c) Endjin Limited. All rights reserved.
// </copyright>

using System.Text.Json;

namespace Corvus.Json;

/// <summary>
/// Represents a JSON property value that might be optional.
/// </summary>
/// <typeparam name="T">The type of the value.</typeparam>
public readonly struct JsonOptional<T>
    where T : struct, IJsonValue<T>
{
    private readonly T backing;

    /// <summary>
    /// Initializes a new instance of the <see cref="JsonOptional{T}"/> class.
    /// </summary>
    /// <param name="value">The potentially optional value.</param>
    public JsonOptional(in T value)
    {
        this.backing = value;
    }

    /// <summary>
    /// Gets a value indicating whether this optional value is present.
    /// </summary>
    /// <remarks>
    /// Note that this determines that the value is not <see cref="JsonValueKind.Undefined"/>. If the value
    /// may be <see cref="JsonValueKind.Null"/> then this will still return <see langword="true"/>.
    /// </remarks>
    public bool HasValue => this.backing.ValueKind != JsonValueKind.Undefined;

    /// <summary>
    /// Gets the value.
    /// </summary>
    /// <exception cref="InvalidOperationException">Thrown if the optional value is undefined.</exception>
    public T Value
    {
        get
        {
            if (!this.HasValue)
            {
                throw new InvalidOperationException("The optional value is undefined.");
            }

            return this.backing;
        }
    }

    /// <summary>
    /// Explicit conversion to the value.
    /// </summary>
    /// <param name="value">The optional backing value to convert.</param>
    public static explicit operator T(JsonOptional<T> value) => value.backing;

    /// <summary>
    /// Implicit conversion to an optional value.
    /// </summary>
    /// <param name="value">The value to convert.</param>
    public static implicit operator JsonOptional<T>(T value) => new(value);

    /// <summary>
    /// Gets the value of the current instance, or a default value.
    /// </summary>
    /// <returns>The value of the <see cref="Value"/> property if the <see cref="HasValue"/> property is <see langword="true"/>; otherwise, the default value of the underlying type.</returns>
    public T GetValueOrDefault()
    {
        return this.HasValue ? this.backing : default;
    }

    /// <summary>
    /// Gets the value of the current instance, or a default value.
    /// </summary>
    /// <param name="defaultValue">The default value to use if <see cref="HasValue"/> is <see langword="false"/>.</param>
    /// <returns>The value of the <see cref="Value"/> property if the <see cref="HasValue"/> property is <see langword="true"/>; otherwise, the supplied <paramref name="defaultValue"/>.</returns>
    public readonly T GetValueOrDefault(in T defaultValue) => this.HasValue ? this.backing : defaultValue;

    /// <inheritdoc/>
    public override bool Equals(object? other)
    {
        if (!this.HasValue)
        {
            return other == null;
        }

        if (other is T value)
        {
            return this.backing.Equals(value);
        }

        return false;
    }

    /// <inheritdoc/>
    public override int GetHashCode() => this.HasValue ? this.backing.GetHashCode() : 0;

    /// <inheritdoc/>
    public override string? ToString() => this.HasValue ? this.backing.ToString() : string.Empty;
}

Nullable:

// <copyright file="JsonNullable{T}.cs" company="Endjin Limited">
// Copyright (c) Endjin Limited. All rights reserved.
// </copyright>

using System.Text.Json;

namespace Corvus.Json;

/// <summary>
/// Represents a JSON property value that might be nullable.
/// </summary>
/// <typeparam name="T">The type of the value.</typeparam>
public readonly struct JsonNullable<T>
    where T : struct, IJsonValue<T>
{
    private readonly T backing;

    /// <summary>
    /// Initializes a new instance of the <see cref="JsonNullable{T}"/> class.
    /// </summary>
    /// <param name="value">The potentially nullable value.</param>
    public JsonNullable(in T value)
    {
        this.backing = value;
    }

    /// <summary>
    /// Gets a value indicating whether this nullable value is not null.
    /// </summary>
    /// <remarks>
    /// Note that this determines that the value is not <see cref="JsonValueKind.Null"/>. If the value
    /// may be <see cref="JsonValueKind.Null"/> then this will still return <see langword="true"/>.
    /// </remarks>
    public bool HasValue => this.backing.ValueKind != JsonValueKind.Null;

    /// <summary>
    /// Gets the value.
    /// </summary>
    /// <exception cref="InvalidOperationException">Thrown if the nullable value is undefined.</exception>
    public T Value
    {
        get
        {
            if (!this.HasValue)
            {
                throw new InvalidOperationException("The nullable value is undefined.");
            }

            return this.backing;
        }
    }

    /// <summary>
    /// Explicit conversion to the value.
    /// </summary>
    /// <param name="value">The nullable backing value to convert.</param>
    public static explicit operator T(JsonNullable<T> value) => value.backing;

    /// <summary>
    /// Implicit conversion to an nullable value.
    /// </summary>
    /// <param name="value">The value to convert.</param>
    public static implicit operator JsonNullable<T>(T value) => new(value);

    /// <summary>
    /// Gets the value of the current instance, or a default value.
    /// </summary>
    /// <param name="defaultValue">The default value to use if <see cref="HasValue"/> is <see langword="false"/>.</param>
    /// <returns>The value of the <see cref="Value"/> property if the <see cref="HasValue"/> property is <see langword="true"/>; otherwise, the supplied <paramref name="defaultValue"/>.</returns>
    public readonly T GetValueOrDefault(in T defaultValue) => this.HasValue ? this.backing : defaultValue;

    /// <inheritdoc/>
    public override bool Equals(object? other)
    {
        if (!this.HasValue)
        {
            return other == null;
        }

        if (other is T value)
        {
            return this.backing.Equals(value);
        }

        return false;
    }

    /// <inheritdoc/>
    public override int GetHashCode() => this.HasValue ? this.backing.GetHashCode() : 0;

    /// <inheritdoc/>
    public override string? ToString() => this.HasValue ? this.backing.ToString() : string.Empty;
}

OptionalNullable:

// <copyright file="JsonOptionalNullable{T}.cs" company="Endjin Limited">
// Copyright (c) Endjin Limited. All rights reserved.
// </copyright>

using System.Text.Json;

namespace Corvus.Json;

/// <summary>
/// Represents a JSON property value that might be nullable.
/// </summary>
/// <typeparam name="T">The type of the value.</typeparam>
public readonly struct JsonOptionalNullable<T>
    where T : struct, IJsonValue<T>
{
    private readonly T backing;

    /// <summary>
    /// Initializes a new instance of the <see cref="JsonOptionalNullable{T}"/> class.
    /// </summary>
    /// <param name="value">The potentially nullable value.</param>
    public JsonOptionalNullable(in T value)
    {
        this.backing = value;
    }

    /// <summary>
    /// Gets a value indicating whether this nullable value is not null or undefined.
    /// </summary>
    /// <remarks>
    /// Note that this determines that the value is not <see cref="JsonValueKind.Null"/> or <see cref="JsonValueKind.Undefined"/>.
    /// </remarks>
    public bool HasValue
    {
        get
        {
            JsonValueKind kind = this.backing.ValueKind;
            return kind != JsonValueKind.Undefined && kind != JsonValueKind.Null;
        }
    }

    /// <summary>
    /// Gets the value.
    /// </summary>
    /// <exception cref="InvalidOperationException">Thrown if the nullable value is undefined.</exception>
    public T Value
    {
        get
        {
            if (!this.HasValue)
            {
                throw new InvalidOperationException("The nullable value is undefined.");
            }

            return this.backing;
        }
    }

    /// <summary>
    /// Explicit conversion to the value.
    /// </summary>
    /// <param name="value">The nullable backing value to convert.</param>
    public static explicit operator T(JsonOptionalNullable<T> value) => value.backing;

    /// <summary>
    /// Implicit conversion to an nullable value.
    /// </summary>
    /// <param name="value">The value to convert.</param>
    public static implicit operator JsonOptionalNullable<T>(T value) => new(value);

    /// <summary>
    /// Gets the value of the current instance, or a default value.
    /// </summary>
    /// <param name="defaultValue">The default value to use if <see cref="HasValue"/> is <see langword="false"/>.</param>
    /// <returns>The value of the <see cref="Value"/> property if the <see cref="HasValue"/> property is <see langword="true"/>; otherwise, the supplied <paramref name="defaultValue"/>.</returns>
    public readonly T GetValueOrDefault(in T defaultValue) => this.HasValue ? this.backing : defaultValue;

    /// <inheritdoc/>
    public override bool Equals(object? other)
    {
        if (!this.HasValue)
        {
            return other == null;
        }

        if (other is T value)
        {
            return this.backing.Equals(value);
        }

        return false;
    }

    /// <inheritdoc/>
    public override int GetHashCode() => this.HasValue ? this.backing.GetHashCode() : 0;

    /// <inheritdoc/>
    public override string? ToString() => this.HasValue ? this.backing.ToString() : string.Empty;
}

We would then declare optional (or nullable, or optional-and-nullable) properties like this:

    /// <summary>
    /// Gets AllOf.
    /// </summary>
    public JsonOptional<Corvus.Json.JsonSchema.Draft202012.Schema.SchemaArray> AllOf
    {
        get
        {
            if ((this.backing & Backing.JsonElement) != 0)
            {
                if (this.jsonElementBacking.ValueKind != JsonValueKind.Object)
                {
                    return default;
                }

                if (this.jsonElementBacking.TryGetProperty(JsonPropertyNames.AllOfUtf8, out JsonElement result))
                {
                    return new Corvus.Json.JsonSchema.Draft202012.Schema.SchemaArray(result);
                }
            }

            if ((this.backing & Backing.Object) != 0)
            {
                if (this.objectBacking.TryGetValue(JsonPropertyNames.AllOf, out JsonAny result))
                {
                    return result.As<Corvus.Json.JsonSchema.Draft202012.Schema.SchemaArray>();
                }
            }

            return default;
        }
    }

[mwadams]

Note that this still means you need to ensure validity and deal with those scenarios appropriately, but you get much better discoverability of what the schema intends, and perhaps a simpler programming model for those cases.

[Fresa]

I can see this work.

What would happen to the Null and Undefined instances that are defined on all types, would they be redundant and removed?

What about required properties, would those properties be implicitly required if they aren't wrapped by JsonOptional<T> etc, or would they wrap something like JsonRequired<T>?

[mwadams]

They would be implicitly "required". There is no need for a wrapper if required and not nullable.

The static Undefined could always be removed as it is identical to using default - but it is a semantic hint (rather like the types elsewhere in the dotnet framework that provide a static Empty property).

Null could be provided only if nullable.

[mwadams]

If we were to do this (which is quite a simple change) I would put it behind a command line switch as it is a source-breaking breaking change (i.e. you would have to rewrite not just recompile) with the default being 'as is'.

Tragically, this would make running the specs take twice as long which is why I've resisted optionality up to now 🤣

[Fresa]

Have you had any chance trying this out? Would be really great to have!

[mwadams]

I'm still working on an API that isn't horrible. I don't like the asymmetry between a property that could be undefined because you simply haven't validated it and it is missing, and a property that is optional, and therefore is undefined even if the document is valid.

Since V2.014, intellisense provides you with that information, but we already have a mechanism for checking that the value.IsNotUndefined() and the additional type just adds another one, which confuses things.

So this may take a bit more thinking.

[mwadams]

I've not yet found a mechanism that looks better than:

if (someObject.MyProperty.IsNotUndefined())
{
   // Use the property...
}

Now that we have

and

Another mechanism for doing this seems like overkill to me, but YMMV.

[mwadams]

I would love to hear any suggestions that add to the overall intuition of the API, which don't lose the notion that you can always get any named property from an object - but that the value may be undefined (or null, string, object, array, number, boolean, integer.)

We really get that this area of mismatch between C# and JSON is a tricky one (especially from the C# developer side), and are keen to make it as easy to understand as possible.

[Fresa]

What about generating a Nullable and NonNullable property according to the schema? Nullable would be generated if the schema states that the property is not required or if it explicitly is of type NULL. NonNullable would only be generated if the property is required and not of type NULL. This would cause a compilation error if these constraints where to change, and there would be a way to explicitly declare that a property will have a value, as long as it has gone through validation.

It might be possible to do this via explicit/implicit operators as well that marshall to the C# represented value by generating it's return type as nullable or not, or some similar methods (AsInt(), AsNullableInt() etc).

[mwadams]

I will continue to have a think and then offer a new suggestion! We might be on to something but it would be a bit of a shift.

[mwadams]

Unfortunately, this approach does not work because:

1. there is no way of specifying that a member of an instance property is not null if a member of the containing type is returning true.
2. Implicit conversions do not help because you can't write e.g.

            return BuiltInTypes.GetTypeNameFor(
                schema.Type.AsSimpleTypes.GetString(),
                schema.Format.GetString(),
                schema.ContentEncoding.GetString(),
                schema.ContentMediaType.GetString(),
                (validateAs & ValidationSemantics.Draft201909) != 0);

if e.g. the property schema.Type does not return the actual required type. It has no way of inferring the type that would offer the AsSimpleTypes().

[Fresa]

It would need to be declared on the type that declares the properties I recon, as that's where the required constraint is defined. And it would need to be combined with the properties' NULL type constraint.

Something like this as an example: public JsonString Foo { get; } public string FooAsString();

That would mean that Foo is defined as a required property and is not a null type. If it were either, it would be generated something like: public string? FooAsString();

What do you think?

[shuebner]

I am a big fan of the "validated model" approach mentioned in a previous comment. That way, the library can stay true to its lazy fault-tolerant duck-typing approach and at the same time provide strongly typed and thus discoverable and compiler-checked C# models.

[shuebner]

Any news? Corvus is so promising. Having compile-time safety on optional/nullable properties of a validated model would make it a killer library. Conversely, working with optional/nullable properties is a real drag right now.

Is there anything I could help you with?

I don't even care how that API looks like or if it is an optional quick-fix extension method on something.

I still have to use Corvus, because no other library seems to even be able to generate proper C# code from our schemas (quicktype, NJsonSchema, OpenAPIGenerator). But not having proper NRTs on a validated model requires so much schema knowledge in the code that it is not too far off from just using plain JsonElement.

[mwadams]

OK :-)

I'm going to implement this behind a command line switch. --optionalAsNullable with the options None (the default, current behaviour), Undefined (which returns null if the value is not present), and NullOrUndefined (which returns null when the optional value is either not present, or null),

Optional properties will then be emitted as MyPropertyValue? (where MyPropertyValue is the usual IJsonValue<MyPropertyValue> of the property type.

HasProperty() will continue to report that the property is present or absent, so you will be able to distinguish between JsonValueKind.Null and JsonValueKind.Undefined if you specify --optionalAsNullable NullOrUndefined.

I will also add a --useImplicitString flag, which makes the operator string() conversion implicit rather than explicit. While this is potentially problematic in high-performance scenarios, it makes it much simpler if you don't mind the implicit allocations.

Let me know what you think about this approach.

[shuebner]

@mwadams That sounds great, you are the best. Just to confirm I understood you correctly:

I don't care about the difference between property not there or property is null once I know the data is valid. I would therefore generate with --optionalAsNullable NullOrUndefined. I will then get POCOs generated with e. g. JsonString? MyNonRequiredString, JsonString MyRequiredString and JsonDateTime? MyNonRequiredDateTime, as well as MySchema.Foo? MaybeFoo for subschemas.

Because JsonString's GetString method will still always return string?, I should then use the implicit operator instead which I get by using --useImplicitString when generating.

And because string is the only primitive value returned by Corvus that is not a struct, this covers reference type nullability. All the other methods like AsInt32 already return non-null and are covered by the nullability of their containing IJsonValue<T>.

If that is correct, I will get my compile-time safety and be very happy :-).

[mwadams]

@shuebner - that is the behaviour I intend for that combination of options, so hopefully, yes!

[mwadams]

@shuebner

Having implemented it, I think I now actually prefer this behaviour for optional properties.

The number of cases where you as a developer actually care about the distinction between null and undefined is vanishingly small - not least because the number of cases where you bother to say e.g. "type": ["string", "null"] is equally small (except where foolish code->schema generators have interpreted optionality as JSON nullability in that direction)

And the way we've established here, you can still find out if you .NET null was really an explicit JsonValueKind.Null by inspecting the HasProperty() value. So the slight awkwardness will be with the less-used case.

Thanks for persisting with this!

It's still going to be behind a switch for V3 and I'll document that it will become the default for V4.

[shuebner]

@mwadams I agree that as a C# developer I probably don't care.

I probably don't need to tell you this, but what makes Corvus stand out is that it has a "JSON Schema first, C# second" approach, which allows it to support all the JSON schema features. All JSON schema features that can be represented by a compile-time mechanism in C#, should be. But there should be no compromises on the JSON-side of the model. Developers are rightfully forced to deal with the complexities of the particular JSON schema with which they work.

Some JSON schema features natively map to C# language features (e. g. primitive integer values). Some of them do not, but can be represented in a compile-time-safe way by generating code (e. g. matcher function for oneOf) And finally some cannot be mapped.

Developers should always be able to look at the data from a JSON perspective. They should be able to work with a compile-time C# representation wherever that representation truthfully represents the schema, but switch to JSON perspective when there is no such representation.

There are a few C# representations of great value and NRTs are one of them. The decision to represent both undefined and null by C# null is IMHO still truthful.

Thank you for taking this on.

I am looking forward to get rid of my hand-written partial struct filler code ;-)

[mwadams]

Fixed in #393 - please try the Preview packages on nuget.

[shuebner]

I am so gonna try this immediately. Thank you so much.

[shuebner]

I just tried it with our existing complex schema and what do you know, immediately found some inconsistencies between code expectations and schema definition regarding which properties are required and which are not. Mission accomplished there.

However, I noticed two things:

The CLI option --outputRootTypeName does not work anymore, it seems to be ignored now. This also happens when disabling all optional naming heuristics (I suspected those may be meddling with the root type name).

The CLI option --useImplicitString apparently does not do anything, I have to use the explicitly cast

[mwadams]

@shuebner Can you open an issue for --outputRootTypeName with a repro?

My test schema works correctly with the preview build.

generatejsonschematypes.exe --rootNamespace JsonSchemaSample.Api --outputRootTypeName FlimFlam --optionalAsNullable NullOrUndefined --outputPath Model test.json

Schema:

{
    "type": "array",
    "prefixItems": [
        {
            "type": "integer",
            "format": "int32",
            "minimum": 0
        },
        { "type": "string" },
        {
            "type": "string",
            "format": "date-time"
        }
    ],
    "unevaluatedItems": false
}

[shuebner]

what about --useImplicitString? Is that just not implemented yet?

[mwadams]

--useImplicitString is not yet implemented because, sadly, it would require a breaking change (the explicit cast is required in the interface in NET8.0 and implicit is not treated a "superset" case - it's just a different member).

So it will require a bit of thought as to how to sort out people upgrading from V3.