From 63d537804bd97924bd02dc4a07b93b4cbdce8563 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 19 Nov 2024 18:41:33 +0000 Subject: [PATCH] Backport JsonSchemaExporter bugfix. (#5671) * Backport JsonSchemaExporter bugfix. * Address feedback. --- .../JsonSchemaExporter.JsonSchema.cs | 6 ++-- .../JsonSchemaExporter/JsonSchemaExporter.cs | 16 ++++----- .../JsonSchemaExporterTests.cs | 33 +++++++++++++++++++ test/Shared/JsonSchemaExporter/TestTypes.cs | 24 ++++++++++++++ 4 files changed, 68 insertions(+), 11 deletions(-) diff --git a/src/Shared/JsonSchemaExporter/JsonSchemaExporter.JsonSchema.cs b/src/Shared/JsonSchemaExporter/JsonSchemaExporter.JsonSchema.cs index 0f1044fc6e..a395c13398 100644 --- a/src/Shared/JsonSchemaExporter/JsonSchemaExporter.JsonSchema.cs +++ b/src/Shared/JsonSchemaExporter/JsonSchemaExporter.JsonSchema.cs @@ -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() { @@ -467,7 +467,7 @@ internal static partial class JsonSchemaExporter switch (schema._trueOrFalse) { case false: - schema = new JsonSchema { Not = JsonSchema.True }; + schema = new JsonSchema { Not = JsonSchema.CreateTrueSchema() }; break; case true: schema = new JsonSchema(); diff --git a/src/Shared/JsonSchemaExporter/JsonSchemaExporter.cs b/src/Shared/JsonSchemaExporter/JsonSchemaExporter.cs index 5c6ce6d9ab..2d8ffc5497 100644 --- a/src/Shared/JsonSchemaExporter/JsonSchemaExporter.cs +++ b/src/Shared/JsonSchemaExporter/JsonSchemaExporter.cs @@ -119,7 +119,7 @@ internal static partial class JsonSchemaExporter 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) @@ -245,7 +245,7 @@ internal static partial class JsonSchemaExporter if (effectiveUnmappedMemberHandling is JsonUnmappedMemberHandling.Disallow) { // Disallow unspecified properties. - additionalProperties = JsonSchema.False; + additionalProperties = JsonSchema.CreateFalseSchema(); } if (typeDiscriminator is { } typeDiscriminatorPair) @@ -435,7 +435,7 @@ internal static partial class JsonSchemaExporter } else { - schema = JsonSchema.True; + schema = JsonSchema.CreateTrueSchema(); } return CompleteSchema(ref state, schema); @@ -578,7 +578,7 @@ internal static partial class JsonSchemaExporter private static readonly Dictionary> _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), @@ -625,10 +625,10 @@ internal static partial class JsonSchemaExporter 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 }, }; diff --git a/test/Shared/JsonSchemaExporter/JsonSchemaExporterTests.cs b/test/Shared/JsonSchemaExporter/JsonSchemaExporterTests.cs index 2ec81987dc..70babf8133 100644 --- a/test/Shared/JsonSchemaExporter/JsonSchemaExporterTests.cs +++ b/test/Shared/JsonSchemaExporter/JsonSchemaExporterTests.cs @@ -13,6 +13,7 @@ using System.Text.Json.Serialization.Metadata; 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 @@ -86,6 +87,38 @@ public abstract class JsonSchemaExporterTests } #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() { diff --git a/test/Shared/JsonSchemaExporter/TestTypes.cs b/test/Shared/JsonSchemaExporter/TestTypes.cs index d21a40640d..0ac4ca2bf1 100644 --- a/test/Shared/JsonSchemaExporter/TestTypes.cs +++ b/test/Shared/JsonSchemaExporter/TestTypes.cs @@ -1164,6 +1164,29 @@ public static partial class TestTypes 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))] + public class ClassWithCustomConverter1; + + [JsonConverter(typeof(CustomConverter))] + public class ClassWithCustomConverter2; + + public sealed class CustomConverter : JsonConverter + { + 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))] @@ -1248,6 +1271,7 @@ public static partial class TestTypes [JsonSerializable(typeof(PocoCombiningPolymorphicTypeAndDerivedTypes))] [JsonSerializable(typeof(ClassWithComponentModelAttributes))] [JsonSerializable(typeof(ClassWithOptionalObjectParameter))] + [JsonSerializable(typeof(ClassWithPropertiesUsingCustomConverters))] // Collection types [JsonSerializable(typeof(int[]))] [JsonSerializable(typeof(List))]