mplanchard / falcon-marshmallow

Marshmallow serialization/deserialization middleware for Falcon
MIT License
4 stars 2 forks source link

callable get_schema? #1

Open mplanchard opened 6 years ago

mplanchard commented 6 years ago

linked from ihiji/falcon-marshmallow#17 for further discussion

@timc13 raised the idea of having a more generic get_schema() functionality that would allow the dynamic selection of a schema depending on the request/response context as it relates to the application integrating this middleware library.

So, given the current architectural style and usage of "you define something on your resource class and it gets used automatically," I could definitely see us checking the Resource object for a method with a particular name, i.e. get_schema() and calling it if it's available to get the schema to use for loading/dumping.

This would put the entirety of the burden of selecting and possibly instantiating a schema on the integrating application, but it would allow virtually unlimited dynamism in schema selection.

One possible issue that springs to mind is ensuring that get_schema() knows whether it is being called from a request context or a response context, which seems like something you'd want to know when dynamically selecting a schema. We could maybe expand the special methods to get_request_schema() and get_response_schema(), with each being passed the request and response contexts. The request method could also include URL parameters, while the response method could include the req_succeeded boolean.

Thoughts?

timc13 commented 6 years ago
  1. get_schema may not be the best name given the existing naming convention of {http_method}_schema

  2. I think we will want the params argument in addition to context in case the app needs to get something from the url path.

In my little custom middleware (inspired by this project), I have something like:

def set_schema(self, req, resp, params):
    req.context['get_schema'] = MyGetSchema()
    req.context['post_request_schema'] = MyPostRequestSchema()
    req.context['post_response_schema'] = MyPostResponseSchema()
mplanchard commented 6 years ago

Ha! I didn't even think about the HTTP method names interfering with the get_schema name. Good catch.

From an API usage perspective, I'd like to keep the use of this method as simple as possible. I think that the presence of a method returning a schema should supersede any method_schema or method_request/response_schema defined on the resource.

This is easy for different HTTP methods, because they're included with the falcon.Request object.

Finding a way for the method to differentiate between being in a request or a response context is a bit tougher, though, and params are only available in a request context. As such, I am right now imagining an API like this:

class MyResource:

  def schema(req, resp):
    """Req/resp context independent. Superseded by specific methods."""

  def request_schema(req, resp, params):
    """Req-specific schema. If present, overrides schema()"""

  def response_schema(req, resp, req_succeeded):
    """Resp-specific schema. If present, overrides schema()"""

We could prevent the potential sacrifice of efficiency caused by checking twice by storing any retrieved schema during the request in the context. If it's present and there's no response_schema() method on the class, we could just use that schema again.

That way, if people don't need anything too complex, they can just use schema(), but like with the class attributes (e.g. get_schema, get_request_schema, etc.), they can customize more if needed.

jwestboston commented 4 years ago

I'm interested in this concept/discussion. There are times where I want to modify the schema based on query parameters on GET requests for example, such as a toggle of showing/returning additional data or not, along with the normal and originally requested data.

mplanchard commented 4 years ago

I did a minimal PoC of this a while back, but didn’t get it to the point where it was cleaned up and ready for public consumption.

I wound up bikeshedding quite a bit on method naming, particularly when it came to method-specific schemas, request vs response schemas, etc. I was thinking about checking for a variety of potential methods, e.g. get_post_schema() and the like, with maybe fallbacks to a generic one, similarly to how defining different schemas as class attributes works currently.

I think in retrospect the easiest thing to do might be to define some wrapper object that encapsulates the request/response context and flags whether we’re dealing with input or output data, and then just pass that object into a single get_schema() method if it exists. This would make the method a little more complicated to implement, but would keep it as flexible as possible.

What do you all think?

On Sun, Oct 13, 2019 at 11:36 AM Josh West notifications@github.com wrote:

I'm interested in this concept/discussion. There are times where I want to modify the schema based on query parameters on GET requests for example, such as a toggle of showing/returning additional data or not, along with the normal and originally requested data.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/mplanchard/falcon-marshmallow/issues/1?email_source=notifications&email_token=ABTQRYNATMMBHADM2GY6WI3QONFATA5CNFSM4FEVN7N2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBC2BII#issuecomment-541434017, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTQRYOFZ657HQF5Q3ZGGSDQONFATANCNFSM4FEVN7NQ .

jwestboston commented 4 years ago

That approach works for me!

mplanchard commented 4 years ago

Cool. I will try to find some time to work on this in the near future! Always happy to accept PRs in the meantime, too.