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 inline enum for query parameters #57979

Closed
1 task done
dnv-kimbell opened this issue Sep 20, 2024 · 5 comments
Closed
1 task done

OpenApi inline enum for query parameters #57979

dnv-kimbell opened this issue Sep 20, 2024 · 5 comments
Assignees
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 Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update.
Milestone

Comments

@dnv-kimbell
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

I have an operation with that takes in an enum as query string parameter, and returns it back in the response.

In the parameter section, the enum is just defined as string with no information about what valid values are

"parameters": [
	{
		"name": "e",
		"in": "query",
		"schema": {
			"type": "string"
		}
	}
],

In the response the enum is referenced and it's possible to figure out valid values

"responses": {
	"200": {
		"description": "OK",
		"content": {
			"application/json": {
				"schema": {
					"$ref": "#/components/schemas/EnumWithNoFlags"
				}
			}
		}
	}
}

Expected Behavior

The query parameter should use a reference to the enum schema.

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 area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Sep 20, 2024
@captainsafia captainsafia added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi 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 21, 2024
@martincostello
Copy link
Member

Just spotted this one myself with version 9.0.100-rc.2.24468.2 of the SDK and 9.0.0-rc.2.24470.14 of the M.A.OpenApi library.

With martincostello/openapi-extensions@5742a21 I get an empty enum for CarType:

"CarType": {
  "type": "integer",
  "description": "The type of the car."
}

@captainsafia
Copy link
Member

The query parameter should use a reference to the enum schema.

The behavior that I find surprising here is that the schemas generated for the response and the query parameter are different. The schema generated by the query parameter uses a type: string with no enum list configured, whereas the schema configured for the response contains the correct enum listing. The difference in behavior between the two is the actual source of the issue.

Upon further debugging, the issue seemed to only manifest for endpoints that used controller actions (as opposed to minimal APIs) which is particularly strange given that the schema generation logic is shared between the two.

Some further debugging reveals that this might be an issue with the ApiExplorer layer for MVC because the ApiParameterDescription generates a default type of string for this argument:

image

Based on the logic here. My suspicion gravitated towards the recent changes we made to ModelMetadata to support native AoT since the referenced method interacts with the IsParsable and IsConvertibleType properties that we recently modified as a result of this work although that doesn't appear to be the case.

It seems like the actual root cause is some code that we added in 46b5580 that set the parameter type of simple types (including those that can be parsed from an enum) to string to support the introduction of TryParse binding support in MVC. Ultimately, that appears to be the issue.

Anyhow, I've figured out a bug fix for this that I hope to include in .NET 9 but figured I'd monologue a bit about the root case here for posterities sake.

@captainsafia captainsafia self-assigned this Sep 22, 2024
@captainsafia captainsafia added this to the 9.0.0 milestone Sep 22, 2024
wtgodbe pushed a commit that referenced this issue Sep 25, 2024
* Fix JsonUnmappedMemberHandling attribute handling to close #57981

* Fix enum handling for MVC actions to close #57979

* Fix self-referential schema handling to close #58006

* Fix concurrent request handling for OpenAPI documents (#57972)

* fix: Allow concurrent requests

* test: Update test

* test: Use Parallel.ForEachAsync

* feat: Use valueFactory overload

* feat: Pass valueFactory directly

* Harden self-referencing schema ID check

---------

Co-authored-by: Justin Lampe <xC0dex@users.noreply.github.com>
captainsafia added a commit that referenced this issue Sep 26, 2024
* Fix JsonUnmappedMemberHandling attribute handling to close #57981

* Fix enum handling for MVC actions to close #57979

* Fix self-referential schema handling to close #58006

* Fix concurrent request handling for OpenAPI documents (#57972)

* fix: Allow concurrent requests

* test: Update test

* test: Use Parallel.ForEachAsync

* feat: Use valueFactory overload

* feat: Pass valueFactory directly

* Harden self-referencing schema ID check

---------

Co-authored-by: Justin Lampe <xC0dex@users.noreply.github.com>
@captainsafia
Copy link
Member

@dnv-kimbell The fix for this has landed in nightly package version 9.0.0-rtm.24476.2.

You'll need to use the following PackageReference:

<PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="9.0.0-rtm.24476.2" />

And make sure that you have a reference to the nightly dotnet9 feed in your nuget.config file:

<add key="dotnet9" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet9/nuget/v3/index.json" />

Can you let me know if that works for you?

@captainsafia captainsafia added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Sep 26, 2024
@captainsafia captainsafia reopened this Sep 27, 2024
@dnv-kimbell
Copy link
Author

When I try that package, I get a reference to the enum from the query section, both with and without flags, so it seems to be working as expected.

"parameters": [
	{
		"name": "e",
		"in": "query",
		"schema": {
			"$ref": "#/components/schemas/EnumWithFlags"
		}
	}
],

"parameters": [
	{
		"name": "e",
		"in": "query",
		"schema": {
			"$ref": "#/components/schemas/EnumWithNoFlags"
		}
	}
],

@dotnet-policy-service dotnet-policy-service bot added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Sep 27, 2024
@captainsafia
Copy link
Member

@dnv-kimbell Great! Thanks for verifying. Closing this bug as fixed.

captainsafia added a commit that referenced this issue Dec 31, 2024
… (#58096)

* Fix JsonUnmappedMemberHandling attribute handling to close #57981

* Fix enum handling for MVC actions to close #57979

* Fix self-referential schema handling to close #58006

* Fix concurrent request handling for OpenAPI documents (#57972)

* fix: Allow concurrent requests

* test: Update test

* test: Use Parallel.ForEachAsync

* feat: Use valueFactory overload

* feat: Pass valueFactory directly

* Harden self-referencing schema ID check

---------

Co-authored-by: Justin Lampe <xC0dex@users.noreply.github.com>
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 Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update.
Projects
None yet
Development

No branches or pull requests

3 participants