Skip to content

OpenApi ignores JsonNumberHandling.WriteAsString #58882

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

Open
1 task done
adrianm64 opened this issue Nov 11, 2024 · 3 comments
Open
1 task done

OpenApi ignores JsonNumberHandling.WriteAsString #58882

adrianm64 opened this issue Nov 11, 2024 · 3 comments
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-openapi
Milestone

Comments

@adrianm64
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

When a property has the attribute JsonNumberHandling(JsonNumberHandling.AllowReadingFromString|JsonNumberHandling.WriteAsString), OpenApi still describes the property with the native type and not a string

Expected Behavior

When using JsonNumberHandling.WriteAsString I expect the OpenApi specification to report the propery as string

"prop": {
    "type": "string",
 }

Steps To Reproduce

  1. Create a new default asp.net webapi for dotnet 9.
  2. Run the project and navigate to /openapi/v1.json
  3. The temperatureC property is listed as integer:
"temperatureC": {
    "type": "integer",
     "format": "int32"
},  
  1. In weatherforcast.cs add an attribute to the temperatureC property
[JsonNumberHandling(JsonNumberHandling.AllowReadingFromString|JsonNumberHandling.WriteAsString)]
public int TemperatureC { get; set; }
  1. Run the project and navigate to /weatherforecast and see temperatureC is a string.
[{"date":"2024-11-12","temperatureC":"11","temperatureF":51,"summary":"Balmy"},{"date":"2024-11-13","temperatureC":"3","temperatureF":37,"summary":"Warm"}}
  1. Navigate to /openapi/v1.json
  2. The temperatureC property is still listed as an integer:
"temperatureC": {
    "type": "integer",
    "format": "int32"
},  

Exceptions (if any)

No response

.NET Version

9.0.100-rc.1.24452.12

Anything else?

A workaround is to add a SchemaTransformer like

internal sealed class JsonNumberHandlerSchemaTransformer : IOpenApiSchemaTransformer
{
    public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext context, CancellationToken cancellationToken)
    {
        var properties = context.JsonTypeInfo.Type.GetProperties(BindingFlags.Public | BindingFlags.Instance)
                                .Where(prop => prop.GetCustomAttribute<JsonNumberHandlingAttribute>()?.Handling.HasFlag(JsonNumberHandling.WriteAsString) ?? false)
                                .Select(prop => prop.GetCustomAttribute<JsonPropertyNameAttribute>()?.Name ?? 
                                                context.JsonTypeInfo.Options.PropertyNamingPolicy?.ConvertName(prop.Name) ??
                                                prop.Name)
                                .Where(schema.Properties.ContainsKey);

        foreach (var property in properties)
        {
            schema.Properties[property].Type = "string";
        }

        return Task.CompletedTask;
    }
}

but this is using reflection and probably missing some other details.

@ghost ghost added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Nov 11, 2024
@martincostello martincostello added feature-openapi area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Nov 11, 2024
@captainsafia
Copy link
Member

@adrianm64 Thanks for filing this issue and sharing the workaround that you had success with!

@mikekistler This is one of those scenarios where our explicit type mappings for primitives gets us into trouble. Should we exempt integer types from this? We might be able to do so in .NET 10 with the JSON schema generated directly from STJ.

@mikekistler
Copy link
Contributor

mikekistler commented Dec 18, 2024

I'm observing the following behavior:

Property has JsonNumberHandling produces a schema with type
AllowReadingFromString [ "string", "integer" ]
AllowReadingFromString|WriteAsString [ "string", "integer" ]
Strict "integer"

Note that "AllowReadingFromString" is the default for Web applications.

@captainsafia I don't think exempting integer is the solution, because the same behavior occurs for double and likely any other "number" type.

I'm not really sure what the solution is here, since OpenAPI v3.0.x does not permit type to be an array. Perhaps the only "correct" thing to do is to omit "type" whenever the type from STJ is an array with more than one element (still handling "null" specially). But that might be more painful than the current behavior.

@captainsafia
Copy link
Member

Thanks for verifying the current behavior!

@captainsafia I don't think exempting integer is the solution, because the same behavior occurs for double and likely any other "number" type.

Correct -- if we went down this route, we'd have to avoid our primitive-type based lookup for everything.

I'm not really sure what the solution is here, since OpenAPI v3.0.x does not permit type to be an array. Perhaps the only "correct" thing to do is to omit "type" whenever the type from STJ is an array with more than one element (still handling "null" specially). But that might be more painful than the current behavior.

Perhaps this is something we can bundle around the 3.1 support issue (#58619)? We should be able to use array types there and generate a more accurate schema.

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 feature-openapi
Projects
None yet
Development

No branches or pull requests

4 participants