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

Looking for a way to use a schema to define acceptable query params #45

Open
BenC14 opened this issue Oct 17, 2019 · 9 comments
Open

Looking for a way to use a schema to define acceptable query params #45

BenC14 opened this issue Oct 17, 2019 · 9 comments
Labels
enhancement New feature or request

Comments

@BenC14
Copy link

BenC14 commented Oct 17, 2019

I have a case where I defined a marshmallow schema that is being used in both @accepts and @responds for different methods. I would like to use the schema object in an @accepts for a get_all route but instead of the fields being defined in the body of the request they should be query params. For example a UserSchema might have 3 fields.

class UserSchema(Schema):
    userId = ma_fields.Integer(dump_only=True, attribute='user_id')
    firstName = ma_fields.String(attribute='first_name')
    emailAddress = ma_fields.Email(attribute='email')

and we might have a get_all route like this

    @accepts(dict(name='limit', type=int), dict(name='page', type=int), api=api)
    @responds(schema=UserSchema(many=True), api=api)
    def get(self):
        return UserService.get_all(**request.parsed_args)

I would like to have a route with documented parameters based on the schema
Perhaps something like

    @accepts(dict(name='limit', type=int), dict(name='page', type=int), UserSchema(location='values'), api=api)
    @responds(schema=UserSchema(many=True), api=api)
    def get(self):
        return UserService.get_all(**request.parsed_args)

It seems like if we are returning a UserSchema object/s with this request we should be able to easily make all of that schemas dump-able fields queryable.

End result would document the existence of ?userId ?firstName and ?emailAddress.
and request.parsed_args would be a dict with 'user_id', 'first_name', 'email' as well as the other query arg dicts 'limit' and 'page'

@apryor6
Copy link
Owner

apryor6 commented Oct 17, 2019

I personally have been down this path and totally understand what you are after; however, there are some philosophical concerns with wanting to provide a body with a GET request. This SO post summarizes it.

In my experience, the use cases where you think you want this behavior boil down to largely three cases:

  1. I want to GET a particular entity, such as by ID. In this case you don't need (or even want) to provide the rest of the object and you would instead setup an endpoint that takes the ID either by URL param or query param.

  2. You want to create a new entity or modify/replace an existing one, in which case you should be providing a JSON body, but with a POST, PUT, or PATCH.

  3. You want to GET all entities that fit a certain set of parameters, such as a template. In this case the query params should define filters, groupings, etc.

Happy to discuss further.

@apryor6
Copy link
Owner

apryor6 commented Oct 24, 2019

Closing

@apryor6 apryor6 closed this as completed Oct 24, 2019
@BenC14
Copy link
Author

BenC14 commented Dec 2, 2019

I agree with you that a GET request cannot accept a body. In this case I am trying to accomplish case #3 more easily. I want to use Marshmallow schemas to add query params for several reasons:

  1. I don't want to write out a new dict object for every possible queryable field.
  2. I want to be able to define preload functions on both POSTs and GETs.
  3. I want create a schema that allows for the same param name in two possible location. For GETs obviously the location=['args'] for POST I could require the same location but preferably it would be location=['values','args'] https://flask-restplus.readthedocs.io/en/stable/parsing.html#multiple-locations

With the current implementation of accepts if you pass a schema object it assumes everything in that schema goes in the payload. Unfortunately that means you cannot define a GET with a schema in the request because the swagger example that gets generated by flask-restplus requires a body and most browsers will throw an error just for attempting to pass a body with a GET.

I am proposing we allow the passing of a schema to @accepts on GETs. By default all the fields in the schema are assumed to be location='args' if you want to support multiple locations you can override the schema field location in the decorator with a dict(name=schema_field_name, location=['headers', 'args', 'values'...etc])
This way we can support multiple locations for POSTs and PUTs, one location for GETs, plus we gain support for @pre_load, @post_load, @pre_dump, @post_dump for all methods in the schema.

@apryor6 apryor6 reopened this Dec 3, 2019
@ErickStoneUSU
Copy link

I second BenC14s request. Being able to use a Schema for uri parameters for a GET would be very welcomed.

@calvinwyoung
Copy link
Contributor

+1 on this — I have a few GET endpoints with a complicated set of parameters. It'd be great if I could declare the structure as a Marsmallow schema

@apryor6 apryor6 added the enhancement New feature or request label Apr 1, 2020
@apryor6
Copy link
Owner

apryor6 commented Apr 1, 2020

Marking this as a feature request. This could be implemented by adding an additional parameter to the decorators that accepted a query_param_schema or header_schema that would use the same internal conversions but apply the result accordingly to swagger and the route.

@calvinwyoung
Copy link
Contributor

@apryor6 Awesome thanks for the response. Adding those parameters would vastly improve things on my end — thank you.

@calvinwyoung
Copy link
Contributor

@apryor6 Do you have a sense for your bandwidth to complete this request? If you're slammed (understandably so during these times), then I'd be happy to take a whack at it.

@apryor6
Copy link
Owner

apryor6 commented Apr 12, 2020

My bandwidth is pretty nonexistent, unfortunately. I would be happy to review and accept a backwards compatible PR with tests. I would think we could accomplish this with an additional keyword parameter to the accepts decorator like query_param_schema and/or header_schema that used the same type conversion machinery but applied the model to a separate part of the route than the body, as it does now with all Schemas

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

No branches or pull requests

4 participants