search

manchenkoff / openapi3-parser

OpenAPI 3 parser to use a specification inside of the code in your projects
https://pypi.org/project/openapi3-parser/
MIT License
61 stars 33 forks source link

[Feature] Include Path.parameters in Operation.parameters #42

Closed sergei-maertens closed 6 months ago

sergei-maertens commented 2 years ago

What is a problem?

In https://github.com/manchenkoff/openapi3-parser/blob/aeb7185fb21de958b1d956be743dcd40e0019c21/src/openapi_parser/builders/path.py#L40

only the operation.parameters is passed to the operation builder, while the OpenAPI spec also supports specifying parameters at the path level that are shared by all operations for that path.

Considering that the method (under which an operation is keyed in the spec) is passed to the operation builder, it looks like the Operation instance is supposed to contain all information for itself, which should include the path parameters.

Describe the solution you'd like

Pass the extracted path parameters down to the operation builder, and have the operation builder do the resolution. Operations may override the path parameters according to the spec, but they cannot remove them. Note that a parameters is uniquely identified by the combination of name and location, so you can use those as dict keys for the override-behaviour check.

Describe alternatives you've considered

I can do the processing in my code of course, but ideally the code making use of openapi3-parser should not have to think about all the intricacies of the OpenAPI 3 specification.

Additional context

I'd like to be able to look at a single Operation instance and have all the necessary information at hand.

I'd also suggest including the URL path in the builder for that reason.

If you're open to this, then I can definitely submit a PR :-)

manchenkoff commented 2 years ago

@sergei-maertens feel free to open this PR 👍 but I guess these changes will break backward compatibility

sergei-maertens commented 2 years ago

@sergei-maertens feel free to open this PR +1 but I guess these changes will break backward compatibility

I might have some time tonight, otherwise it'll probably be next weekend.

We could put the functionality behind a flag like in #40, and then in the next major release switch the default value disabled to enabled by default. What's your opinion on this approach?

manchenkoff commented 2 years ago

@sergei-maertens, sounds good to me 👍