dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
34.9k stars 9.86k forks source link

Exception from dotnet build for project using JsonPolymorphic with TypeDiscriminatorPropertyName #56575

Open mikekistler opened 4 weeks ago

mikekistler commented 4 weeks ago

Description

Build for a project that uses the JsonPolymorphic attribute with TypeDiscriminatorPropertyName throws an exception if 1) the project is configured to generate an OpenAPI document at build time, and b) the discriminator property is already declared in the class.

Note that a similar project in .NET 8 builds cleanly.

Reproduction Steps

I have created a minimal repro for this problem in a branch in my aspnet-openapi-examples project.

https://github.com/mikekistler/aspnet-openapi-examples/tree/bug-104064

The branch has a series of commits that illustrate that a) the project builds cleanly on .NET 8, and b) the exception comes after the final commit which adds the .NET 9 feature for generating OpenAPI documents.

The exception message indicates that the crash occurs when attempting to a field that already exists. Most likely this is attempting to add the discriminator field.

Expected behavior

Rather than crash, I think logic should simply bypass adding the discriminator field if it is already present in the class, as seems to be the behavior in .NET 8.

Actual behavior

Here's the full stack trace for the exception:

> dotnet build
Restore complete (0.7s)
You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
  Bug failed with 28 error(s) and 2 warning(s) (2.2s) → bin\Debug\net9.0\Bug.dll
    C:\Users\mikekistler\Projects\mikekistler\aspnet-openapi-examples\Shapes.cs(14,19): warning CS8618: Non-nullable property 'ShapeType' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
    C:\Users\mikekistler\Projects\mikekistler\aspnet-openapi-examples\Program.cs(26,19): warning CS8600: Converting null literal or possible null value to non-nullable type.
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error : System.AggregateException: One or more errors occurred. (An item with the same key has already been added. Key: shapeType (Parameter 'key'))
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :  ---> System.ArgumentException: An item with the same key has already been added. Key: shapeType (Parameter 'key')
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at System.Collections.ThrowHelper.ThrowDuplicateKey[TKey](TKey key)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at System.Collections.Generic.OrderedDictionary`2.TryInsert(Int32 index, TKey key, TValue value, InsertionBehavior behavior)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at System.Collections.Generic.OrderedDictionary`2.Add(TKey key, TValue value)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at System.Text.Json.Nodes.JsonObject.Add(String propertyName, JsonNode value)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at JsonSchemaMapper.JsonSchemaMapper.MapJsonSchemaCore(GenerationState& state, JsonTypeInfo typeInfo, Type parentType, JsonPropertyInfo parentPropertyInfo, ParameterInfo parentParameterInfo, String description, Boolean isNullableReferenceType, Boolean isNullableOfTElement, JsonConverter customConverter, Nullable`1 customNumberHandling, Boolean hasDefaultValue, JsonNode defaultValue, Type parentPolymorphicType, Nullable`1 typeDiscriminator)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at JsonSchemaMapper.JsonSchemaMapper.MapJsonSchemaCore(GenerationState& state, JsonTypeInfo typeInfo, Type parentType, JsonPropertyInfo parentPropertyInfo, ParameterInfo parentParameterInfo, String description, Boolean isNullableReferenceType, Boolean isNullableOfTElement, JsonConverter customConverter, Nullable`1 customNumberHandling, Boolean hasDefaultValue, JsonNode defaultValue, Type parentPolymorphicType, Nullable`1 typeDiscriminator)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at JsonSchemaMapper.JsonSchemaMapper.GetJsonSchema(JsonSerializerOptions options, Type type, JsonSchemaMapperConfiguration configuration)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.AspNetCore.OpenApi.OpenApiComponentService.CreateSchema(ValueTuple`2 key)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.AspNetCore.OpenApi.OpenApiComponentService.GetOrCreateSchema(Type type, ApiParameterDescription parameterDescription)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.AspNetCore.OpenApi.OpenApiDocumentService.GetResponse(ApiDescription apiDescription, Int32 statusCode, ApiResponseType apiResponseType)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.AspNetCore.OpenApi.OpenApiDocumentService.GetResponses(ApiDescription description)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.AspNetCore.OpenApi.OpenApiDocumentService.GetOperation(ApiDescription description, HashSet`1 capturedTags)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.AspNetCore.OpenApi.OpenApiDocumentService.GetOperations(IGrouping`2 descriptions, HashSet`1 capturedTags)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.AspNetCore.OpenApi.OpenApiDocumentService.GetOpenApiPaths(HashSet`1 capturedTags)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.AspNetCore.OpenApi.OpenApiDocumentService.GetOpenApiDocumentAsync(CancellationToken cancellationToken)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.Extensions.ApiDescriptions.OpenApiDocumentProvider.GenerateAsync(String documentName, TextWriter writer, OpenApiSpecVersion openApiSpecVersion)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    --- End of inner exception stack trace ---
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at System.Threading.Tasks.Task.Wait(TimeSpan timeout, CancellationToken cancellationToken)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at System.Threading.Tasks.Task.Wait(TimeSpan timeout)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.Extensions.ApiDescription.Tool.Commands.GetDocumentCommandWorker.GetDocument(String documentName, String projectName, String outputDirectory, MethodInfo generateMethod, Object service, MethodInfo generateWithVersionMethod)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.Extensions.ApiDescription.Tool.Commands.GetDocumentCommandWorker.GetDocuments(IServiceProvider services)
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.Extensions.ApiDescription.Tool.Commands.GetDocumentCommandWorker.Process()
    C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error MSB3073: The command "dotnet "C:\.tools\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.5.24306.11\build\../tools/dotnet-getdocument.dll" --assembly "C:\Users\mikekistler\Projects\mikekistler\aspnet-openapi-examples\bin\Debug\net9.0\Bug.dll" --file-list "obj\Bug.OpenApiFiles.cache" --framework ".NETCoreApp,Version=v9.0" --output "C:\Users\mikekistler\Projects\mikekistler\aspnet-openapi-examples" --project "Bug" --assets-file "C:\Users\mikekistler\Projects\mikekistler\aspnet-openapi-examples\obj\project.assets.json" --platform "AnyCPU" " exited with code 11.

Build failed with 28 error(s) and 2 warning(s) in 3.2s

Regression?

While technically not a regression, I think some might view it that way because the application code causing the crash is unchanged from the .NET 8 version that builds cleanly.

Known Workarounds

The workaround seems to be removing the ShapeType property from the class and removing any references to it -- such as setting it to specific values -- elsewhere in the code.

Configuration

dotnet --version: 9.0.100-preview.6.24320.9 OS and version: Windows 11 Architecture: x64 (dev box)

Other information

No response

dotnet-policy-service[bot] commented 4 weeks ago

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis See info in area-owners.md if you want to be subscribed.

eiriktsarpalis commented 3 weeks ago

I suspect this is related to the discriminator id conflicting with a property of a similar name, although I couldn't reproduce the error in the standalone JsonSchemaMapper component. @captainsafia I'm assuming this could be caused by the fixup logic performed by the callback, but I defer to you on that. It may well be that the issue has resolved itself in Preview 6 since JsonSchemaMapper was replaced with the built-in STJ exporter.

captainsafia commented 3 weeks ago

@eiriktsarpalis I was actually able to repro this in the new codebase that doesn't use the JsconSchemaMapper prototype.

I was able to repro this without the custom TransformSchemaNode callback registered in the callback:

[JsonPolymorphic(TypeDiscriminatorPropertyName = "shapeType")]
[JsonDerivedType(typeof(AnotherTriangle), typeDiscriminator: "triangle")]
[JsonDerivedType(typeof(AnotherSquare), typeDiscriminator: "square")]
internal abstract class AnotherShape
{
    public required string ShapeType { get; set; }
    public string Color { get; set; } = string.Empty;
    public int Sides { get; set; }
}

internal class AnotherTriangle : AnotherShape
{
    public double Hypotenuse { get; set; }
}
internal class AnotherSquare : AnotherShape
{
    public double Area { get; set; }
}

// key.Type == AnotherShape
JsonSchemaExporter.GetJsonSchemaAsNode(_jsonSerializerOptions, key.Type); // throws here

I think the issue might be happening when the schema generator inserts the discriminator property into a child schema. The modifications that we make don't touch the child schema.

although I couldn't reproduce the error in the standalone JsonSchemaMapper component.

What did your repro look like?

eiriktsarpalis commented 3 weeks ago

I used the repro provided here but with JsonSchemaMapper instead of JsonSchemaExporter. Could you please transfer the issue back to runtime if it repros for JsonSchemaExporter?

captainsafia commented 3 weeks ago

OK -- I'll send this back to the runtime repo. The repro here uses JsonSchemaExporter.

eiriktsarpalis commented 3 weeks ago

I still wasn't able to reproduce the issue as-is, but it did occur once I changed the type discriminator property name so that it matches the casing of the ShapeType property. In that case however the type discriminator property name conflicts with a property name, so the type is configured incorrectly and the generated schema would be ambiguous.

Just to confirm that I'm looking at the same manifestation of the problem, how is the _jsonSerializerOptions in the repro being configured?

captainsafia commented 3 weeks ago

It's resolved from ASP.NET Core's JsonOptions service in DI. The default configuration for it is here.

eiriktsarpalis commented 3 weeks ago

I still can't repro. Adapting from ASP.NET Core defaults here's what I got:

[Fact]
public void Repro()
{
    JsonSerializerOptions options = new()
    {
        TypeInfoResolver = new DefaultJsonTypeInfoResolver(),
        Encoder = Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping
    };

    JsonNode schema = options.GetJsonSchemaAsNode(typeof(Shape));
}

[JsonPolymorphic(TypeDiscriminatorPropertyName = "shapeType")]
[JsonDerivedType(typeof(Triangle), typeDiscriminator: "triangle")]
[JsonDerivedType(typeof(Square), typeDiscriminator: "square")]
[JsonDerivedType(typeof(Circle), typeDiscriminator: "circle")]
public abstract class Shape
{
    public string ShapeType { get; set; }
    public string Color { get; set; } = "";
    public int Sides { get; set; }
}

public class Triangle : Shape
{
    public double Hypotenuse { get; set; }
}

public class Square : Shape
{
    public double Area { get; set; }
}

public class Circle : Shape
{
    public double Radius { get; set; }
}
captainsafia commented 3 weeks ago

@eiriktsarpalis Thanks for trying this out. I suppose there's something missing between the minimal repro and the repro with our JSON serializer options. I'll transfer this back to us and take another look.