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

Added Explode and Style to SwaggerParameter #3065

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aberus
Copy link

@aberus aberus commented Sep 4, 2024

Pull Request

The issue or feature being addressed

It is currently not possible to set Explode and Style on the parameter.

Details on the issue fix or feature implementation

Added bool Explode and string Style with backing properties bool? ExplodeFlag and Microsoft.OpenApi.Models.ParameterStyle? ParameterStyle respectively. AnnotationsParameterFilter applies these two properties if set.

Copy link
Collaborator

@martincostello martincostello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add at least one integration/verification test that shows the rendered effect of the new properties in an OpenAPI document please?

@jgarciadelanoceda
Copy link
Contributor

This could close the issue #2248

@jgarciadelanoceda
Copy link
Contributor

@aberus, are you planning to continue with this PR?, because I find it interesting :). I also will like to add the required parameters to solve the issue #2115(That's why I contact you first, to avoid conflicts between us)

@martincostello
Copy link
Collaborator

@aberus Thanks for responding to the review suggestions. This still needs tests before we can merge it though.

@aberus
Copy link
Author

aberus commented Oct 11, 2024

@martincostello I plan to add tests. But also I think that generation need to be changed, to support these two scenarions.

The default ASP.NET model binding parses the query ?sizes=a,b,c,d as a parameter of type string.

[HttpGet("Products")]
public IActionResult Get([FromQuery(Name = "sizes")][SwaggerParameter(Explode = false)] string sizes)
{
    return Ok();
}

The custom model binding ([CommaSeparated]) mentioned in #2248 can be mapped to a collection;

[HttpGet("Products")]
public IActionResult Get([FromQuery(Name = "sizes")][SwaggerParameter(Explode = false)][CommaSeparated] IEnumerable<string> sizes)
{
    return Ok();
}

Both of these scenarios should produce this:

paths:
  /Products:
    get:
      parameters:
        - name: sizes
          in: query
          style: form
          explode: false
          schema:
            type: array
            items:
              type: string
      responses:
        '200':
          description: Success

Let me know, what do you think?

@martincostello
Copy link
Collaborator

How would you propose adding metadata (i.e. an attribute) indicating that the parameters should be treated that way to both ASP.NET Core and Swashbuckle? The issue points to a Stack Overflow answer, which points to a blog post, which mentions a custom implementation of ASP.NET Core model binding. We wouldn't want to be shipping model binders in Swashbuckle, because that's not specific to OpenAPI.

@aberus
Copy link
Author

aberus commented Oct 12, 2024

My suggestion in first example is that a non-collection parameter combination with explode set to false, should generate an array in the OpenAPI specification.
Second example show how currently works (with this change) if explode set to false, but external library need to used to work property. I didn't mean that some external library needs to be added, just that this scenario also needs to be supported if someone wants to use it with external library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants