Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenApi Flags enum not handled correctly #57980

Open
1 task done
dnv-kimbell opened this issue Sep 20, 2024 · 7 comments
Open
1 task done

OpenApi Flags enum not handled correctly #57980

dnv-kimbell opened this issue Sep 20, 2024 · 7 comments
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi
Milestone

Comments

@dnv-kimbell
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

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

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Sep 20, 2024
@martincostello martincostello added feature-openapi area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Sep 20, 2024
@dnv-kimbell
Copy link
Author

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
Copy link
Member

@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.

@captainsafia captainsafia added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Sep 22, 2024
@captainsafia captainsafia added this to the Discussions milestone Sep 22, 2024
@dnv-kimbell
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi
Projects
None yet
Development

No branches or pull requests

3 participants