Skip to content

Commit

Permalink
Backport JsonSchemaExporter bugfix. (#5671)
Browse files Browse the repository at this point in the history
* Backport JsonSchemaExporter bugfix.

* Address feedback.
  • Loading branch information
eiriktsarpalis authored Nov 19, 2024
1 parent 9b61daa commit 9cfd5ff
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ internal static partial class JsonSchemaExporter
// https://github.com/dotnet/runtime/blob/50d6cad649aad2bfa4069268eddd16fd51ec5cf3/src/libraries/System.Text.Json/src/System/Text/Json/Schema/JsonSchema.cs
private sealed class JsonSchema
{
public static JsonSchema False { get; } = new(false);
public static JsonSchema True { get; } = new(true);
public static JsonSchema CreateFalseSchema() => new(false);
public static JsonSchema CreateTrueSchema() => new(true);

public JsonSchema()
{
Expand Down Expand Up @@ -467,7 +467,7 @@ public static void EnsureMutable(ref JsonSchema schema)
switch (schema._trueOrFalse)
{
case false:
schema = new JsonSchema { Not = JsonSchema.True };
schema = new JsonSchema { Not = JsonSchema.CreateTrueSchema() };
break;
case true:
schema = new JsonSchema();
Expand Down
16 changes: 8 additions & 8 deletions src/Shared/JsonSchemaExporter/JsonSchemaExporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ private static JsonSchema MapJsonSchemaCore(
if (!ReflectionHelpers.IsBuiltInConverter(effectiveConverter))
{
// Return a `true` schema for types with user-defined converters.
return CompleteSchema(ref state, JsonSchema.True);
return CompleteSchema(ref state, JsonSchema.CreateTrueSchema());
}

if (parentPolymorphicTypeInfo is null && typeInfo.PolymorphismOptions is { DerivedTypes.Count: > 0 } polyOptions)
Expand Down Expand Up @@ -245,7 +245,7 @@ private static JsonSchema MapJsonSchemaCore(
if (effectiveUnmappedMemberHandling is JsonUnmappedMemberHandling.Disallow)
{
// Disallow unspecified properties.
additionalProperties = JsonSchema.False;
additionalProperties = JsonSchema.CreateFalseSchema();
}

if (typeDiscriminator is { } typeDiscriminatorPair)
Expand Down Expand Up @@ -435,7 +435,7 @@ private static JsonSchema MapJsonSchemaCore(
}
else
{
schema = JsonSchema.True;
schema = JsonSchema.CreateTrueSchema();
}

return CompleteSchema(ref state, schema);
Expand Down Expand Up @@ -578,7 +578,7 @@ private static string FormatJsonPointer(ReadOnlySpan<string> path)

private static readonly Dictionary<Type, Func<JsonNumberHandling, JsonSchema>> _simpleTypeSchemaFactories = new()
{
[typeof(object)] = _ => JsonSchema.True,
[typeof(object)] = _ => JsonSchema.CreateTrueSchema(),
[typeof(bool)] = _ => new JsonSchema { Type = JsonSchemaType.Boolean },
[typeof(byte)] = numberHandling => GetSchemaForNumericType(JsonSchemaType.Integer, numberHandling),
[typeof(ushort)] = numberHandling => GetSchemaForNumericType(JsonSchemaType.Integer, numberHandling),
Expand Down Expand Up @@ -625,10 +625,10 @@ private static string FormatJsonPointer(ReadOnlySpan<string> path)
Pattern = @"^\d+(\.\d+){1,3}$",
},

[typeof(JsonDocument)] = _ => JsonSchema.True,
[typeof(JsonElement)] = _ => JsonSchema.True,
[typeof(JsonNode)] = _ => JsonSchema.True,
[typeof(JsonValue)] = _ => JsonSchema.True,
[typeof(JsonDocument)] = _ => JsonSchema.CreateTrueSchema(),
[typeof(JsonElement)] = _ => JsonSchema.CreateTrueSchema(),
[typeof(JsonNode)] = _ => JsonSchema.CreateTrueSchema(),
[typeof(JsonValue)] = _ => JsonSchema.CreateTrueSchema(),
[typeof(JsonObject)] = _ => new JsonSchema { Type = JsonSchemaType.Object },
[typeof(JsonArray)] = _ => new JsonSchema { Type = JsonSchemaType.Array },
};
Expand Down
33 changes: 33 additions & 0 deletions test/Shared/JsonSchemaExporter/JsonSchemaExporterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using System.Xml.Linq;
#endif
using Xunit;
using static Microsoft.Extensions.AI.JsonSchemaExporter.TestTypes;

#pragma warning disable SA1402 // File may only contain a single type

Expand Down Expand Up @@ -86,6 +87,38 @@ public void CanGenerateXElementSchema()
}
#endif

#if !NET9_0 // Disable until https://github.com/dotnet/runtime/pull/109954 gets backported
[Fact]
public void TransformSchemaNode_PropertiesWithCustomConverters()
{
// Regression test for https://github.com/dotnet/runtime/issues/109868
List<(Type? parentType, string? propertyName, Type type)> visitedNodes = new();
JsonSchemaExporterOptions exporterOptions = new()
{
TransformSchemaNode = (ctx, schema) =>
{
#if NET9_0_OR_GREATER
visitedNodes.Add((ctx.PropertyInfo?.DeclaringType, ctx.PropertyInfo?.Name, ctx.TypeInfo.Type));
#else
visitedNodes.Add((ctx.DeclaringType, ctx.PropertyInfo?.Name, ctx.TypeInfo.Type));
#endif
return schema;
}
};

List<(Type? parentType, string? propertyName, Type type)> expectedNodes =
[
(typeof(ClassWithPropertiesUsingCustomConverters), "Prop1", typeof(ClassWithPropertiesUsingCustomConverters.ClassWithCustomConverter1)),
(typeof(ClassWithPropertiesUsingCustomConverters), "Prop2", typeof(ClassWithPropertiesUsingCustomConverters.ClassWithCustomConverter2)),
(null, null, typeof(ClassWithPropertiesUsingCustomConverters)),
];

Options.GetJsonSchemaAsNode(typeof(ClassWithPropertiesUsingCustomConverters), exporterOptions);

Assert.Equal(expectedNodes, visitedNodes);
}
#endif

[Fact]
public void TreatNullObliviousAsNonNullable_True_DoesNotImpactObjectType()
{
Expand Down
24 changes: 24 additions & 0 deletions test/Shared/JsonSchemaExporter/TestTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1164,6 +1164,29 @@ public readonly struct StructDictionary<TKey, TValue>(IEnumerable<KeyValuePair<T
IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable)_dictionary).GetEnumerator();
}

public class ClassWithPropertiesUsingCustomConverters
{
[JsonPropertyOrder(0)]
public ClassWithCustomConverter1? Prop1 { get; set; }
[JsonPropertyOrder(1)]
public ClassWithCustomConverter2? Prop2 { get; set; }

[JsonConverter(typeof(CustomConverter<ClassWithCustomConverter1>))]
public class ClassWithCustomConverter1;

[JsonConverter(typeof(CustomConverter<ClassWithCustomConverter2>))]
public class ClassWithCustomConverter2;

public sealed class CustomConverter<T> : JsonConverter<T>
{
public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
=> default;

public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
=> writer.WriteNullValue();
}
}

[JsonSerializable(typeof(object))]
[JsonSerializable(typeof(bool))]
[JsonSerializable(typeof(byte))]
Expand Down Expand Up @@ -1248,6 +1271,7 @@ public readonly struct StructDictionary<TKey, TValue>(IEnumerable<KeyValuePair<T
[JsonSerializable(typeof(PocoCombiningPolymorphicTypeAndDerivedTypes))]
[JsonSerializable(typeof(ClassWithComponentModelAttributes))]
[JsonSerializable(typeof(ClassWithOptionalObjectParameter))]
[JsonSerializable(typeof(ClassWithPropertiesUsingCustomConverters))]
// Collection types
[JsonSerializable(typeof(int[]))]
[JsonSerializable(typeof(List<bool>))]
Expand Down

0 comments on commit 9cfd5ff

Please sign in to comment.