Skip to content

Commit

Permalink
Plug JsonSchemaExporter test data to the AIJsonUtilities tests (#5590)
Browse files Browse the repository at this point in the history
* Plug JsonSchemaExporter test data to the AIJsonUtilities tests

* Update src/LegacySupport/DiagnosticAttributes/README.md

* Update src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.cs

* Address feedback.
  • Loading branch information
eiriktsarpalis authored and joperezr committed Nov 5, 2024
1 parent 365f33c commit ce9a807
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 94 deletions.
1 change: 0 additions & 1 deletion eng/packages/TestOnly.props
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
<PackageVersion Include="Xunit.Combinatorial" Version="1.6.24" />
<PackageVersion Include="xunit.extensibility.execution" Version="2.4.2" />
<PackageVersion Include="JsonSchema.Net" Version="7.2.3" />
<PackageVersion Include="JsonSchema.Net.Generation" Version="4.3.0.2" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'net462'">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using System.Text.Json;
using System.Text.Json.Nodes;
using System.Text.Json.Schema;
using System.Text.Json.Serialization;
using Microsoft.Shared.Diagnostics;

#pragma warning disable S1121 // Assignments should not be made from within sub-expressions
Expand Down Expand Up @@ -282,7 +283,7 @@ JsonNode TransformSchemaNode(JsonSchemaExporterContext ctx, JsonNode schema)
// Some consumers of the JSON schema, including Ollama as of v0.3.13, don't understand
// schemas with "type": [...], and only understand "type" being a single value.
// STJ represents .NET integer types as ["string", "integer"], which will then lead to an error.
if (TypeIsArrayContainingInteger(objSchema))
if (TypeIsIntegerWithStringNumberHandling(ctx, objSchema))
{
// We don't want to emit any array for "type". In this case we know it contains "integer"
// so reduce the type to that alone, assuming it's the most specific type.
Expand Down Expand Up @@ -351,17 +352,21 @@ static JsonObject ConvertSchemaToObject(ref JsonNode schema)
}
}

private static bool TypeIsArrayContainingInteger(JsonObject schema)
private static bool TypeIsIntegerWithStringNumberHandling(JsonSchemaExporterContext ctx, JsonObject schema)
{
if (schema["type"] is JsonArray typeArray)
if (ctx.TypeInfo.NumberHandling is not JsonNumberHandling.Strict && schema["type"] is JsonArray typeArray)
{
foreach (var entry in typeArray)
int count = 0;
foreach (JsonNode? entry in typeArray)
{
if (entry?.GetValueKind() == JsonValueKind.String && entry.GetValue<string>() == "integer")
if (entry?.GetValueKind() is JsonValueKind.String &&
entry.GetValue<string>() is "integer" or "string")
{
return true;
count++;
}
}

return count == typeArray.Count;
}

return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,27 @@
</PropertyGroup>

<PropertyGroup>
<NoWarn>$(NoWarn);CA1063;CA1861;CA2201;VSTHRD003</NoWarn>
<NoWarn>$(NoWarn);CA1063;CA1861;CA2201;VSTHRD003;S104</NoWarn>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
</PropertyGroup>

<PropertyGroup>
<InjectDiagnosticAttributesOnLegacy>true</InjectDiagnosticAttributesOnLegacy>
<InjectCompilerFeatureRequiredOnLegacy>true</InjectCompilerFeatureRequiredOnLegacy>
<InjectRequiredMemberOnLegacy>true</InjectRequiredMemberOnLegacy>
<InjectIsExternalInitOnLegacy>true</InjectIsExternalInitOnLegacy>
<InjectStringSyntaxAttributeOnLegacy>true</InjectStringSyntaxAttributeOnLegacy>
</PropertyGroup>

<ItemGroup>
<Compile Include="..\..\Shared\JsonSchemaExporter\SchemaTestHelpers.cs" Link="Utilities\SchemaTestHelpers.cs" />
<Compile Include="..\..\Shared\JsonSchemaExporter\TestData.cs" Link="Utilities\TestData.cs" />
<Compile Include="..\..\Shared\JsonSchemaExporter\TestTypes.cs" Link="Utilities\TestTypes.cs" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="System.Memory.Data" />
<PackageReference Include="JsonSchema.Net" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@

using System.ComponentModel;
using System.Text.Json;
using System.Text.Json.Nodes;
using System.Text.Json.Serialization;
using Microsoft.Extensions.AI.JsonSchemaExporter;
using Xunit;

namespace Microsoft.Extensions.AI;
Expand Down Expand Up @@ -130,7 +132,7 @@ public static void ResolveParameterJsonSchema_ReturnsExpectedValue()
}

[Fact]
public static void ResolveParameterJsonSchema_TreatsIntegralTypesAsInteger_EvenWithAllowReadingFromString()
public static void CreateParameterJsonSchema_TreatsIntegralTypesAsInteger_EvenWithAllowReadingFromString()
{
JsonElement expected = JsonDocument.Parse("""
{
Expand Down Expand Up @@ -160,9 +162,36 @@ public enum MyEnumValue
}

[Fact]
public static void ResolveJsonSchema_CanBeBoolean()
public static void CreateJsonSchema_CanBeBoolean()
{
JsonElement schema = AIJsonUtilities.CreateJsonSchema(typeof(object));
Assert.Equal(JsonValueKind.True, schema.ValueKind);
}

[Theory]
[MemberData(nameof(TestTypes.GetTestDataUsingAllValues), MemberType = typeof(TestTypes))]
public static void CreateJsonSchema_ValidateWithTestData(ITestData testData)
{
// Stress tests the schema generation method using types from the JsonSchemaExporter test battery.

JsonSerializerOptions options = testData.Options is { } opts
? new(opts) { TypeInfoResolver = TestTypes.TestTypesContext.Default }
: TestTypes.TestTypesContext.Default.Options;

JsonElement schema = AIJsonUtilities.CreateJsonSchema(testData.Type, serializerOptions: options);
JsonNode? schemaAsNode = JsonSerializer.SerializeToNode(schema, options);

Assert.NotNull(schemaAsNode);
Assert.Equal(testData.ExpectedJsonSchema.GetValueKind(), schemaAsNode.GetValueKind());

if (testData.Value is null || testData.WritesNumbersAsStrings)
{
// By design, our generated schema does not accept null root values
// or numbers formatted as strings, so we skip schema validation.
return;
}

JsonNode? serializedValue = JsonSerializer.SerializeToNode(testData.Value, testData.Type, options);
SchemaTestHelpers.AssertDocumentMatchesSchema(schemaAsNode, serializedValue);
}
}
6 changes: 3 additions & 3 deletions test/Shared/JsonSchemaExporter/JsonSchemaExporterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public void TestTypes_GeneratesExpectedJsonSchema(ITestData testData)
: Options;

JsonNode schema = options.GetJsonSchemaAsNode(testData.Type, (JsonSchemaExporterOptions?)testData.ExporterOptions);
Helpers.AssertValidJsonSchema(testData.Type, testData.ExpectedJsonSchema, schema);
SchemaTestHelpers.AssertEqualJsonSchema(testData.ExpectedJsonSchema, schema);
}

[Theory]
Expand All @@ -45,7 +45,7 @@ public void TestTypes_SerializedValueMatchesGeneratedSchema(ITestData testData)

JsonNode schema = options.GetJsonSchemaAsNode(testData.Type, (JsonSchemaExporterOptions?)testData.ExporterOptions);
JsonNode? instance = JsonSerializer.SerializeToNode(testData.Value, testData.Type, options);
Helpers.AssertDocumentMatchesSchema(schema, instance);
SchemaTestHelpers.AssertDocumentMatchesSchema(schema, instance);
}

[Theory]
Expand Down Expand Up @@ -100,7 +100,7 @@ public void TypeWithDisallowUnmappedMembers_AdditionalPropertiesFailValidation()
{
JsonNode schema = Options.GetJsonSchemaAsNode(typeof(TestTypes.PocoDisallowingUnmappedMembers));
JsonNode? jsonWithUnmappedProperties = JsonNode.Parse("""{ "UnmappedProperty" : {} }""");
Helpers.AssertDoesNotMatchSchema(schema, jsonWithUnmappedProperties);
SchemaTestHelpers.AssertDoesNotMatchSchema(schema, jsonWithUnmappedProperties);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,20 @@
using System.Text.Json.Nodes;
using System.Text.Json.Serialization;
using Json.Schema;
using Json.Schema.Generation;
using Xunit.Sdk;

namespace Microsoft.Extensions.AI.JsonSchemaExporter;

internal static partial class Helpers
internal static partial class SchemaTestHelpers
{
public static void AssertValidJsonSchema(Type type, string? expectedJsonSchema, JsonNode actualJsonSchema)
public static void AssertEqualJsonSchema(JsonNode expectedJsonSchema, JsonNode actualJsonSchema)
{
// If an expected schema is provided, use that. Otherwise, generate a schema from the type.
JsonNode? expectedJsonSchemaNode = expectedJsonSchema != null
? JsonNode.Parse(expectedJsonSchema, documentOptions: new() { CommentHandling = JsonCommentHandling.Skip })
: JsonSerializer.SerializeToNode(new JsonSchemaBuilder().FromType(type), Context.Default.JsonSchema);

// Trim the $schema property from actual schema since it's not included by the generator.
(actualJsonSchema as JsonObject)?.Remove("$schema");

if (!JsonNode.DeepEquals(expectedJsonSchemaNode, actualJsonSchema))
if (!JsonNode.DeepEquals(expectedJsonSchema, actualJsonSchema))
{
throw new XunitException($"""
Generated schema does not match the expected specification.
Expected:
{FormatJson(expectedJsonSchemaNode)}
{FormatJson(expectedJsonSchema)}
Actual:
{FormatJson(actualJsonSchema)}
""");
Expand Down
26 changes: 19 additions & 7 deletions test/Shared/JsonSchemaExporter/TestData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,40 @@
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Text.Json;
using System.Text.Json.Nodes;
using System.Text.Json.Schema;

namespace Microsoft.Extensions.AI.JsonSchemaExporter;

internal sealed record TestData<T>(
T? Value,
[StringSyntax(StringSyntaxAttribute.Json)] string ExpectedJsonSchema,
IEnumerable<T?>? AdditionalValues = null,
[StringSyntax("Json")] string? ExpectedJsonSchema = null,
JsonSchemaExporterOptions? ExporterOptions = null,
JsonSerializerOptions? Options = null)
JsonSerializerOptions? Options = null,
bool WritesNumbersAsStrings = false)
: ITestData
{
private static readonly JsonDocumentOptions _schemaParseOptions = new() { CommentHandling = JsonCommentHandling.Skip };

public Type Type => typeof(T);
object? ITestData.Value => Value;
object? ITestData.ExporterOptions => ExporterOptions;
JsonNode ITestData.ExpectedJsonSchema { get; } =
JsonNode.Parse(ExpectedJsonSchema, documentOptions: _schemaParseOptions)
?? throw new ArgumentNullException("schema must not be null");

IEnumerable<ITestData> ITestData.GetTestDataForAllValues()
{
yield return this;

if (default(T) is null &&
ExporterOptions is { TreatNullObliviousAsNonNullable: false } &&
Value is not null)
{
yield return this with { Value = default };
}

if (AdditionalValues != null)
{
foreach (T? value in AdditionalValues)
Expand All @@ -41,15 +55,13 @@ public interface ITestData

object? Value { get; }

/// <summary>
/// Gets the expected JSON schema for the value.
/// Fall back to JsonSchemaGenerator as the source of truth if null.
/// </summary>
string? ExpectedJsonSchema { get; }
JsonNode ExpectedJsonSchema { get; }

object? ExporterOptions { get; }

JsonSerializerOptions? Options { get; }

bool WritesNumbersAsStrings { get; }

IEnumerable<ITestData> GetTestDataForAllValues();
}
Loading

0 comments on commit ce9a807

Please sign in to comment.