Allowing partial response with return fields specified through a query param

[zedrdave]

We are looking to make a number of endpoints allow a only (or select?) query param that essentially would forward the fields (with appropriate sanity checking) to the return schema as an only= argument. Allowing us for example to run things like GET /pets/123?select=status and get a compact response.

While this seems doable if we implement our own serialisation, or maybe even by running the response twice (first through our partial schema, then through smorest's default wrapper), it would of course be a lot nicer if we could somehow specify (at endpoint or api level) that a given query param can be used for that purpose.

In a way, this should probably be implemented similarly to pagination, but we are not entirely sure about the best way to go.

Is this something you have given thought to? any opinions on that? If we were to work on a PR, would you consider merging it?

[lafrech]

Hi. Thanks for opening this.

This is not something we ever needed to do but the use case sounds legit.

I have no advice to provide out of my head. I'd have to get deeper into it, which I can't right now.

There's a balance between functionality and ease of use and API surface.

If this can be achieved in a way that is not too clunky, I'm happy to merge. If not, I'm happy if the recipe is shared here.

If this can be made easier to achieve in user code thanks to a few changes in this lib (like splitting a function, adding a hook,...), we can do that, and even add the recipe as a test in https://github.com/marshmallow-code/flask-smorest/blob/main/tests/test_examples.py to ensure it is not broken inadvertently in the future.

Does that sound good?

Feel free to discuss the feature here or even send a draft PR to discuss.

[zedrdave]

@lafrech thanks for the quick reply.
We will also need to dig a bit deeper to see what can be done. The only observation so far, was that in order to modify the return schema (eg by passing it an only argument), we'd need to work around the fact that it it assigned at compile time (whereas the filter arguments happen at request time). But hopefully that could work through a wrapper.

Will look into it and possibly suggest a PR.

[zedrdave]

@lafrech after looking closer, I can't see a way to make this work nicely, without somehow modifying the logic that dumps from the Schema:

flask-smorest/src/flask_smorest/response.py

Line 103 in 91be341

result_dump = schema.dump(result_raw)

The two structural issues I see are:

1. we want to modify only, which is not possible since the schema has been instantiated. I reckon this could be done by modifying the attributes directly (?), but that seems rather dirty
2. additionally, we need a way to provide the only value through the argument wrapper. I guess looking in the kwargs for a predefined argument might work?

Any idea how we could work around that, with or without modifying the core?

[zedrdave]

@lafrech After digging a bit more, I was able to get something very close to working smoothly.

My main issue has to do with Marshmallow schema instantiation (so, not really directly related to Smorest, but I'm hoping you might have some insight): because we allow schema instances (and sometimes need it, eg when specifying many=True), it is afaik no longer possible to modify .only (see https://github.com/marshmallow-code/marshmallow/issues/1491)…

Is there any option beside introspecting the init args of that instance and instantiating a new one with the correct only value?

class QueryListArgs(ma.Schema):
    select = ma.fields.List(
        ma.fields.String, missing=[], required=False, description="Fields to select"
    )
   [...] # additional sanity checking (using the model), `filters` dict etc

def filter_response(*args, **kwargs):
    def decorator(func):
        def wrapper(*args2, **kwargs2):

            # Define a wrapper that overrides @bp.response and customises the schema:
            newargs = list(args)
            if "select" in kwargs2:
                newargs[1] = resolve_schema_instance(newargs[1])
                # FIXME: Won't work, because the schema is already instantiated:
                newargs[1].only = kwargs2["select"]
                del kwargs2["select"]
            return bp.response(*newargs, **kwargs)(func)(*args2, **kwargs2)

        return wrapper

    return decorator


@bp.route("/pets/", methods=["GET"])
class ExportPets(MethodView):

    @bp.arguments(QueryListArgs(model=Pet), location="query", as_kwargs=True)
    @filter_response(HTTPStatus.OK, PetSchema(many=True))
    def get(self, filters):
        ret = Pet.query.all()
        # todo: apply filters
        return ret

Aside from that issue with Marshmallow, I think the above could work (with some extra clean up and sanity checking) as an add-on. But it might also make more sense to add it as an option to the existing bp.response wrapper?

[lafrech]

Random thought : would it make sense to override `@response` to add schema modifiers such as `only` and declare them as parameters along the ones declared in `@parameters`, like `@pagination` does ?

[zedrdave]

Random thought : would it make sense to override @.to add schema modifiers such asonly and declare them as parameters along the ones declared in ***@***.***, like @.` does ?

It seems like the email-to-github interface is doing weird things to your message 😅

BTW, I got a working version by deriving Blueprint:

class QueryFilterBlueprint(Blueprint):

    def filtered_response(self, *args, **kwargs):
        schema_cls_or_instance = args[1]

        def decorator(func):
            def wrapper(*args2, **kwargs2):

                # Define a wrapper that overrides @bp.response and customises the schema:
                newargs = list(args)
                select = kwargs2.pop("select", None)
                if select is not None:
                    if isinstance(schema_cls_or_instance, type):
                        newargs[1] = schema_cls_or_instance(only=select)
                    else:
                        # We can't update `only` on an instantiated schema,
                        # so we need to clone it with the same init args and new `only` value:
                        schema_class = schema_cls_or_instance.__class__
                        init_kwargs = {"only": select}
                        for key in [
                            "many",
                            "context",
                            "load_only",
                            "dump_only",
                            "partial",
                        ]:
                            if hasattr(schema_cls_or_instance, key):
                                init_kwargs[key] = getattr(schema_cls_or_instance, key)
                        newargs[1] = schema_class(**init_kwargs)

                return super(Blueprint, self).response(*newargs, **kwargs)(func)(
                    *args2, **kwargs2
                )

            [...]
            # had to copy paste a bunch of code from the default @response decorator to 
            # attach all the API doc stuff to the wrapper. Not sure I see another any other option…
            [...]

            # Wrap in a bp.arguments decorator that will parse filter/select options:
            return self.arguments(
                QueryListArgs(
                    model=kwargs.pop("filter_model", None),
                    schema=schema_cls_or_instance,
                ),
                location="query",
                as_kwargs=True,
            )(wrapper)

        return decorator

[zedrdave]

@lafrech obviously, a cleaner way to do that, would be to have filtered_response a default decorator and factor out the API doc-related code between response and filtered_response… 😅

My QueryListArgs schema also does a bunch of things such as:

• validate that select fields are present in the model
• validate that filters parameters are valid queriable columns
• transform filters ({column, operator, value}) into valid SQLA predicate that can be fed directly to query.filter()…

I suppose we could actually try to apply the filters manually on the response (if it's a list), but then we would lose the advantage of filtering at SQLA level… 🤷 Also this would need to run before serialisation (since the select would potentially remove filterable fields)
I guess this could be configurable.

What do you think?

[lafrech]

It seems like the email-to-github interface is doing weird things to your message

Ah, ah. Fixed.

I guess you got the idea anyway.

Please allow me some time to review your proposal.

I suppose I could at least split @response in a sensible way to avoid custom decorators such as yours duplicating too much code.

[zedrdave]

@lafrech Sounds good. Have a look and let me know. One structural issue to keep in mind if we override/enhance the existing @response, is that this feature requires instantiating the response schema at request time (since it's based on the query args), which is very different from the current set-up.

IMHO, it would potentially make more sense to have both decorators, and a common add_doc_to_wrapper method that does the API doc stuff.

With access to that common method, our Blueprint-override implementation will perfectly suit our purposes, but I think it would be a nice addition to the core, as this seems a fairly typical use-case along the same line as pagination (of course, it would need to be made a bit sturdier and more configurable).

[lafrech]

this feature requires instantiating the response schema at request time (since it's based on the query args), which is very different from the current set-up.

Yes. Changing this could have a performance impact and perhaps generate other side-issues, so I'd rather keep things as is.

Considering the case, although legit, is not so common, and since your code seems to work for you, I'm leaning towards extracting the documentation related code from the response decorator into a private method. Not sure exactly which part should be split and how. Perhaps seeing your copied/adapted code would help.

[zedrdave]

@lafrech sure thing, here is the code we use:

    def filtered_response(self, *args, **kwargs):
        schema_cls_or_instance = args[1]

        def decorator(func):
            def wrapper(*args2, **kwargs2):

                # Define a wrapper that overrides @bp.response and customises the schema:
                newargs = list(args)
                select = kwargs2.pop("select", None)
                if select is not None:
                    if isinstance(schema_cls_or_instance, type):
                        newargs[1] = schema_cls_or_instance(only=select)
                    else:
                        # We can't update `only` on an instantiated schema,
                        # so we need to clone it with the same init args and new `only` value:
                        schema_class = schema_cls_or_instance.__class__
                        init_kwargs = {"only": select}
                        for key in [
                            "many",
                            "context",
                            "load_only",
                            "dump_only",
                            "partial",
                        ]:
                            if hasattr(schema_cls_or_instance, key):
                                init_kwargs[key] = getattr(schema_cls_or_instance, key)
                        newargs[1] = schema_class(**init_kwargs)

                return super(Blueprint, self).response(*newargs, **kwargs)(func)(
                    *args2, **kwargs2
                )

            status_code = kwargs.pop("status_code", args[0])
            description = kwargs.pop("description", None)
            schema = resolve_schema_instance(schema_cls_or_instance)
            # NOTE: Copy-pasting from default Blueprint.response decorator to add
            # Document response (schema, description,...) in the API doc
            doc_schema = self._make_doc_response_schema(schema)
            if description is None:
                description = http.HTTPStatus(int(status_code)).phrase
            resp_doc = remove_none(
                {
                    "schema": doc_schema,
                    "description": description,
                    "example": kwargs.get("example"),
                    "examples": kwargs.get("examples"),
                    "headers": kwargs.get("headers"),
                }
            )
            resp_doc["content_type"] = kwargs.get("content_type")

            # Store doc in wrapper function
            # The deepcopy avoids modifying the wrapped function doc
            # In OAS 3, there may be several responses for the same status code
            wrapper._apidoc = deepcopy(getattr(wrapper, "_apidoc", {}))
            (
                wrapper._apidoc.setdefault("response", {})
                .setdefault("responses", {})
                .setdefault(status_code, [])
                .append(resp_doc)
            )
            # Indicate this code is a success status code
            # Helps other decorators documenting success responses
            wrapper._apidoc.setdefault("success_status_codes", []).append(status_code)

            # Wrap in a bp.arguments decorator that will parse filter/select options:
            return self.arguments(
                QueryListArgs(
                    model=kwargs.pop("filter_model", None),
                    schema=schema_cls_or_instance,
                ),
                location="query",
                as_kwargs=True,
            )(wrapper)

        return decorator

[lafrech]

I didn't try to run any code but from a quick glance, I'm surprised you need to duplicate all the documentation code.

Couldn't you grab the one added as _apidoc to the wrapped function and just modify what you need (the schema) and add that to the wrapper?