Generating a scheme with both nullable and not nullable value types, marks both as nullable

[tjochums]

Nuget Package

JsonSchema.Net.Generation

Package Version

3.5.0

Operating System

None

.Net Target (if relevant, please specify the version in the description)

.Net (5 or after)

A clear and concise description of the bug.

I had some classes that were using either int or long as ids, and in some cases when the schema was getting generated it was marking them as null, but not all cases. After running some tests I came to the conclusion that it has to do with the case where the Nullable property isn't defined at all on the id, but another property later on in the file is marked as Nullable(true). The most simple possible example is:

    public class TestAsset
    {
        public int AssetId { get; set; }

        [Nullable(true)]
        public int? OtherIntProperty{ get; set; }
    }

The output json schema looks like:

{
	"type": "object",
	"properties": {
		"AssetId": {
			"type": [
				"integer",
				"null"
			]
		},
		"OtherIntProperty": {
			"type": [
				"integer",
				"null"
			]
		}
	}
}

What did you expect?

I would've expected in this case that the type for the AssetId property would be only integer, and not allow nullable, like so:

{
	"type": "object",
	"properties": {
		"AssetId": {
			"type": "integer"
		},
		"OtherIntProperty": {
			"type": [
				"integer",
				"null"
			]
		}
	}
}

Please add test code or steps to reproduce the behavior.

Just a test console app, here showing how the schemas are being generated.

namespace MyApp 
{
    using System;
    using System.Text.Json;
    using System.Text;
    using Json.Schema;
    using Json.Schema.Generation;
    using Json.More;

    internal class Program
    {
        static void Main(string[] args)
        {
            var testSchema = GetJsonSchema(typeof(TestAsset));

            var schema = ToJsonString(testSchema);
            Console.WriteLine(schema);
        }

        private static string ToJsonString(JsonDocument jdoc)
        {
            // I didn't do much on this, but a cleaner way to deal with writing these to JSON would be nice.
            using (var stream = new MemoryStream())
            {
                Utf8JsonWriter writer = new Utf8JsonWriter(stream, new JsonWriterOptions { Indented = false });
                jdoc.WriteTo(writer);
                writer.Flush();
                return Encoding.UTF8.GetString(stream.ToArray());
            }
        }

        private static JsonDocument GetJsonSchema(Type objectType)
        {
            //var blah = JsonSchema.FromType();
            var jsonSchema = GetJsonSchemaObject(objectType);
            var jsonDoc = jsonSchema.ToJsonDocument();
            return jsonDoc;
        }

        public static JsonSchema GetJsonSchemaObject(Type objectType)
        {
            // The Schema Builder tracks all keywords, and only replaces ones that exist for the new type. So we need to use a new instance each time.
            var jsonBuilder = new JsonSchemaBuilder();
            return jsonBuilder.FromType(objectType, new SchemaGeneratorConfiguration() { Optimize = false }).Build();
        }
    }

    public class TestAsset
    {
        //[Nullable(false)]
        public int AssetId { get; set; }

        [Nullable(true)]
        public int? OtherIntProperty { get; set; }
    }
}

Is there any other information you'd like to share regarding this bug?

I did some work poking around to try to determine what I could do about it, and what was the root cause of the problem, and I came up with a few things.

1. It can be worked around by marking the the required one with Nullable(false) (which I commented out in the code sample), so there is a work around , but my issue is primarily that I'm expecting others to be building these kinds of classes and I think that it would make sense if it behaved as I was expecting it to in the case described.
2. After debugging through the JsonSchema.Net.Generation library, what appeared to causing it to happen was in the NullableValueTypeSchemaGenerator, when we'd already cached the int type and it's context with a TypeIntent that is just Integer, It copies the intent object from the int into the list of intents for the int? type.

	public void AddConstraints(SchemaGenerationContextBase context)
	{
		var underlyingType = Nullable.GetUnderlyingType(context.Type);

		if (underlyingType == null) return;
		var underlyingContext = SchemaGenerationContextCache.Get(underlyingType);

		context.Intents.AddRange(underlyingContext.Intents);
	}

After it adds the reference to that intent, then when the NullabilityRefiner to deal with the Nullable attribute, then it modifies the intent to be "Integer | Null"

		if (nullabilityOverride.HasValue)
		{
			if (nullabilityOverride.Value)
				typeIntent.Type |= SchemaValueType.Null;
			else
				typeIntent.Type &= ~SchemaValueType.Null;
			return;
		}

But because it's the same object reference now cached value for the int and the int? both have the "Integer | Null" intent.

I did determine I could override that particular generator and do some extra handling on the intents to effectively clone any TypeIntent when the context , though I don't know what if any consequences that would have, and/or if it would break something else that I'm not expecting.

        public void AddConstraints(SchemaGenerationContextBase context)
        {
            var underlyingType = Nullable.GetUnderlyingType(context.Type);

            if (underlyingType == null) return;
            var underlyingContext = SchemaGenerationContextCache.Get(underlyingType);

            foreach (var intent in underlyingContext.Intents)
            {
                var typeIntent = intent as TypeIntent;
                if (typeIntent != null)
                {
                    context.Intents.Add(new TypeIntent(typeIntent.Type));
                    continue;
                }

                context.Intents.Add(intent);
            }
        }

Code of Conduct

• I agree to follow this project's Code of Conduct

[gregsdennis]

Thanks for the report. I'll set up a test and have a look.

[gregsdennis]

// I didn't do much on this, but a cleaner way to deal with writing these to JSON would be nice.

Your entire program could be written better using the serializer.

var jsonBuilder = new JsonSchemaBuilder();
// implicit conversion
JsonSchema jsonSchema = jsonBuilder.FromType(objectType, new SchemaGeneratorConfiguration{ Optimize = false });
var asString = JsonSerializer.Serialize(jsonSchema);
Console.WriteLine(asString);

.ToJsonDocument() just calls the serializer and then parses the result into a JsonDocument. So your code is really converting to a string twice.

[tjochums]

Thanks for the feedback on that and the fix, looks much better in my use case!