Skip to content

Fix support for parameter names sourced from attributes (#45572) #45591

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

Merged
merged 2 commits into from
Jan 11, 2023

Conversation

captainsafia
Copy link
Member

Backport of #45572 to release/7.0.

Description

This change resolves an issue where OpenAPI generation via the WithOpenApi method did not factor for the Name argument in parameters sourced from routes or queries via the explicit FromRoute and FromQuery.

Fixes #45542

Customer Impact

This bug fix addresses a user-submitted bug report that is presented as a regression from .NET 6. Without this bug fix, users are not able to successfully test endpoints that use explicit binding source attributes with a name via the Swagger UI and the generated OpenAPI document.

Regression?

  • Yes
  • No

This is a regression in behavior that was available in .NET 6.

Risk

  • High
  • Medium
  • Low

Low risk because this change updates the new OpenAPI package to match the behavior of the existing ApiExplorer implementation.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@captainsafia captainsafia requested a review from a team as a code owner December 14, 2022 18:14
@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 Dec 14, 2022
@captainsafia captainsafia added the Servicing-consider Shiproom approval is required for the issue label Dec 14, 2022
@ghost
Copy link

ghost commented Dec 14, 2022

Hi @captainsafia. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@captainsafia captainsafia removed the Servicing-consider Shiproom approval is required for the issue label Jan 5, 2023
@captainsafia captainsafia added the Servicing-consider Shiproom approval is required for the issue label Jan 6, 2023
@ghost
Copy link

ghost commented Jan 6, 2023

Hi @captainsafia. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@captainsafia captainsafia added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Jan 7, 2023
@ghost
Copy link

ghost commented Jan 7, 2023

Hi @captainsafia. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed.

@captainsafia
Copy link
Member Author

Approved via email.

@dougbu
Copy link
Contributor

dougbu commented Jan 7, 2023

@captainsafia were you asking for a merge❔

One QQ on the changes: Don't IFormFile parameters use the name to find a specific attached file❔ I'm not looking at the binder but thought there was something like that…

@captainsafia
Copy link
Member Author

captainsafia commented Jan 10, 2023

@captainsafia were you asking for a merge❔

Yep! 😄

One QQ on the changes: Don't IFormFile parameters use the name to find a specific attached file❔ I'm not looking at the binder but thought there was something like that…

AFAIK, they just use the filename. However, with the OpenAPI specification, form uploads are treated as part of the request body and are documented separately from request parameters (sourced from routes, queries, headers) which support a unique name field. The request body of the OpenAPI schema doesn't support custom names in the same way AFAIK.

@dougbu dougbu merged commit 5fa89a8 into release/7.0 Jan 11, 2023
@dougbu dougbu deleted the safia/openapi-bp branch January 11, 2023 19:03
@dougbu dougbu added this to the 7.0.3 milestone Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants