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
35.45k stars 10.03k forks source link

OpenApi Flags enum not handled correctly #57980

Open dnv-kimbell opened 1 month ago

dnv-kimbell commented 1 month ago

Is there an existing issue for this?

Describe the bug

I have two operations that uses enums; one has [Flags] applied to it, the other does not. The one without flags is handled correctly, the one with the attribute is not.

[Flags]
public enum EnumWithFlags
{
    None = 0,
    Value2 = 1,
    Value3 = 2,
    Value4 = 4,
    Value5 = 8,
    Value6 = 16
}
public enum EnumWithNoFlags
{
    None = 0,
    Value2 = 1,
    Value3 = 2,
    Value4 = 4,
    Value5 = 8,
    Value6 = 16
}
"EnumWithFlags": {
    "type": "string"
},
"EnumWithNoFlags": {
    "enum": [
        "None",
        "Value2",
        "Value3",
        "Value4",
        "Value5",
        "Value6"
    ]

Expected Behavior

Enums with flags should be handled the same as without flags.

[Flags] is a .NET specific thing and one could consider this an edge case since it doesn't have a counterpart in OpenApi. I work on a set of old apps that have been upgraded from asmx->WCF->WebApi. These use flag enums and we have .NET on both sides.

The STJ Enum converter has support for Flags https://github.com/dotnet/runtime/blob/f96898084d7e4fadd8679f280daef979d60e10cf/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs#L23

Steps To Reproduce

https://github.com/dnv-kimbell/openapi-inlineschema

Exceptions (if any)

No response

.NET Version

9.0 RC1

Anything else?

No response

dnv-kimbell commented 1 month ago

One workaround for this is to create your own schema transformer. This might be good enough for most scenarios.

public class EnumFlagsTransformer : IOpenApiSchemaTransformer
{
    public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext context, CancellationToken cancellationToken)
    {
        var type = context.JsonTypeInfo.Type;
        if (type.IsEnum)
        {
            var flags = type.GetCustomAttribute<FlagsAttribute>();
            if (flags is not null)
            {
                var values = Enum.GetValues(type);

                foreach (var e in values)
                {
                    schema.Enum.Add(new OpenApiString(e.ToString()));
                }
            }
        }

        return Task.CompletedTask;
    }
}
captainsafia commented 1 month ago

@dnv-kimbell Thanks for reporting this issue!

It looks like this is intentional behavior of the JsonSchemaExporter API's given the implementation defined here.

I suspect that this is because bitwise enums don't have a definition that maps cleanly into JSON schema since there's no way to explicitly label the combined values of a bitwise combination. The representation that you're modeling via the schema transformer above totally works, but for the purposes of .NET <-> .NET intro, the fact that FlagsWithEnums supports bitwise combination has been lost when reduced to this form.

For this particular case, I'm inclined to say that we wouldn't modify the default behavior in the implementation to support actually emitting the enum values given the constraints of the schema.

I'll poke around to see if there's any guidance at the schema level about the best way to model bitwise enums.

dnv-kimbell commented 1 month ago

System.Text..Json serializes bitwise enums as comma separated list of values. If the schema listed the possible values as it does with normal enums, wouldn't the use of bitwise become an implementation detail between client an server? Some consuming languages could potentially have limited support for bitwise values; only use it when you know the details of each end.

captainsafia commented 1 month ago

System.Text..Json serializes bitwise enums as comma separated list of values.

Can you clarify what you mean by this? Given the following sample code:

using System;
using System.Text.Json;
using System.Text.Json.Serialization;

JsonSerializerOptions options = new(JsonSerializerDefaults.Web) { Converters = { new JsonStringEnumConverter() } };

Console.WriteLine(JsonSerializer.Serialize(Role.Editor | Role.Author));
Console.WriteLine(JsonSerializer.Serialize(Role.Editor | Role.Author, options));

[Flags]
public enum Role
{
    Admin = 0,
    Editor = 1,
    Author = 2,
}

You'll get the following output:

3
"Editor, Author"

Both of these values are not in the set of supported enums that are generated in the schema, so by JSON schema's definition, they would fail validation:

"Role": {
    "enum": [
        0,
        1,
        2
    ]
}
// OR
"Role": {
    "enum": [
        "Admin",
        "Editor",
        "Author"
    ]
}

Some consuming languages could potentially have limited support for bitwise values; only use it when you know the details of each end.

How would any arbitrary client/server know that an enum actually supports bitwise combination given the information you're currently able to convey in the schema?

dnv-kimbell commented 1 month ago

As you have pointed out, bitwise combinations is not something OpenApi currently support. If you are building a public API consumable by anybody, bitwise is probably not an option. If you are developing an internal API where you have control over both ends, bitwise becomes an option.

If the default schema generation handled flag enums the same way as other enums, we would have enough information to generate an enum type in C# and let STJ handle the bitwise parsing.

Is there a downside of having flag enums handled the same way as other enums? STJ handles flag enums and if you have started using them, you probably have made some assumptions. It would be nice if the schema generator handled these automatically, but since there seems to be simple workaround, not the most important thing to fix.

captainsafia commented 1 month ago

Is there a downside of having flag enums handled the same way as other enums? STJ handles flag enums and if you have started using them, you probably have made some assumptions.

The biggest downside is STJ handles them using assumptions that aren't easy to encode in a schema (for example, the fact that bitwise enums are comma separated strings when JsonStringEnumConverter is enabled). The most round-trippable way to convey this in the schema is to use a more flexible type like "integer" or "string".

Your situation is unique because the round-tripping is between two .NET services, but that seems like a less common case compared to say client SDKs in any language and a backing .NET API.

dnv-kimbell commented 1 month ago

I agree that this case is unique compared to client SDKs that are consumed by any language. On the other hand, there are probably a bunch of legacy systems out there from the dawn of .NET (like ours) that are gradually modernizing their code. Big enterprises seem to favor packages from Microsoft, so when this is released people will start looking into it.

The workaround I posted seems to be a good enough workaround for most cases. This issue might be resolved just by posting a sample transformer as part of the documentation. If the system could generate a warning when it encounters a flags enum, it would give people something to investigate from.