Schema for path-arguments duplicates input

[der-joel]

Using a schema to validate/process the path arguments seems to pass the argument twice. The data-dict of the marshmallow schema is passed as an argument and the value is passed as keyword-argument (probably by vanilla flask).

Note: This can be prevented by passing as_kwargs = True to the decorator

Code-Example:

from flask.views import MethodView
from marshmallow import Schema
from marshmallow.fields import String
from flask_smorest import Blueprint

bp = Blueprint("pets", "pets", description="pet related operations", url_prefix="/api/pets")

class PetIdSchema(Schema):
    pet_id = String(required=True)

@bp.route("/<string:pet_id>")
class SinglePet(MethodView):
    """route for operations on a single pet"""

    @bp.arguments(PetIdSchema(), location="path")
    def get(self, *args, **kwargs):
        """get information on this pet"""
        print(f"args: {args} | kwargs: {kwargs}")
        # prints: args: ({'pet_id': '600aed28e65f74c3a97f6f58'},) | kwargs: {'pet_id': '600aed28e65f74c3a97f6f58'}
        pet_id = args[0]
        return Pet.get_by_id(pet_id)

[lafrech]

Use of Schema for path parameters is poorly documented.

See this conversation: https://github.com/marshmallow-code/flask-smorest/issues/219.

Better doc would certainly not hurt.

Thinking out loud, perhaps we could catch the parameter injected by Flask from @arguments and solely rely on the schema. Might be worth investigating, as it could improve user experience.

[sirosen]

Thinking out loud, perhaps we could catch the parameter injected by Flask from @arguments and solely rely on the schema. Might be worth investigating, as it could improve user experience.

I'm thinking about how this could be done, either in flask-smorest or in webargs.

We could check request.view_args and filter any matching keys out of keyword args when location="path", use_kwargs=False. No path parameters would be passed through multiple stacked Blueprint.arguments decorators in that case.

Another option is for Blueprint.arguments to implicitly pass use_kwargs=True if location="path" and no use_kwargs parameter is explicitly passed.

I'm not sure if these are good ideas vs. docs and maybe a warning about path params specifically. Fancy special-case behaviors might be harder to understand or document.

[hellbe]

Ran into another issue relating to this. In the generated OpenAPI specification the path parameter is documented on the path object but it seems like the OpenAPI standard expects it to be documented on the method(s): https://swagger.io/docs/specification/describing-parameters/#path-parameters

I can create the desired output for the method by adding an argument schema with location="path" but a validator still complains about the unexpected parameters for the path. Any advice on how to get rid of that output or, even better, change so the path arguments output is added to the method(s) instead?

[lafrech]

@hellbe see the "common parameters" part of that page:

Common Parameters for All Methods of a Path Parameters shared by all operations of a path can be defined on the path level instead of the operation level. Path-level parameters are inherited by all operations of that path. A typical use case are the GET/PUT/PATCH/DELETE operations that manipulate a resource accessed via a path parameter.

[hellbe]

@lafrech yes I am well aware and not suggesting to change that, but the fact that OpenAPI/Swagger spec still expects those parameters to be described for each method. I.e. the definition should not change, the generated documentation should.

[lafrech]

My understanding is that parameters shared by all operations of a path can be defined on the path level instead of the operation level so our implementation conforms to the spec, doesn't it?

Are you saying that swagger-ui or another tool complains about it?

[hellbe]

@lafrech you are correct - looked more closely and also confirmed here: https://swagger.io/specification/#path-item-object

For some reason the validator I was using is complaining so I assumed it was out of spec. Sorry about the confusion!