wimglenn / djangorestframework-queryfields

Allows clients to control which fields will be sent in the API response
http://djangorestframework-queryfields.readthedocs.io/
MIT License
216 stars 16 forks source link

New Feature - Nested Fields #8

Open stunaz opened 7 years ago

stunaz commented 7 years ago

Hi, Thank you for the lib. Would it be possible to add a mechanism for nested fields? i.e. some how : Given GET http://127.0.0.1:8000/things/

[
  {
    "id": 1,
    "key1": "val1",
    "key2": "val2",
    "key3":{
      "id": 2,
      "key1": "valA",
      "key2": "valB",
      "key3": "valC",
    },
  }
]

then we could do :

GET http://127.0.0.1:8000/things/?fields=id,key3__id, key3__key2

[
  {
    "id": 1,
    "key3":{
      "id": 2,
      "key2": "valB"
    },
  }
]
wimglenn commented 7 years ago

That's a good idea. I assume you didn't intend to include "key1": "val1", in the example response here? I'll have a look and see what I can do. Handling exclusion queries will need some design decisions - could you add examples for what you think makes most sense for those?

stunaz commented 7 years ago

@wimglenn yes indeed, meant to exclude "key1": "val1", removed it. exclusions would work the same way: i.e.: with initial example from my first post

GET http://127.0.0.1:8000/things/?fields!=id,key3__id, key3__key2

[
  {
    "key1": "val1",
    "key2": "val2",
    "key3":{
      "id": 2,
      "key3": "valC",
    },
  }
]
wimglenn commented 7 years ago

Why not like this?

[
  {
    "key1": "val1",
    "key2": "val2",
    "key3":{
      "key1": "valA",
      "key3": "valC",
    },
  }
]
stunaz commented 7 years ago

@wimglenn exactly

mlissner commented 7 years ago

FWIW, the other project claims to have this functionality: https://github.com/dbrgn/drf-dynamic-fields/blob/master/drf_dynamic_fields/__init__.py

wimglenn commented 7 years ago

@mlissner Could you clarify? I don't see that claim anywhere there, nor code to support that feature. And there is no test coverage showing nested filtering working.

mlissner commented 7 years ago

Yeah, I didn't see the code either, but I also didn't understand it all. Here's where they claim it supports nested fields:

https://github.com/dbrgn/drf-dynamic-fields/blob/master/CHANGELOG.md#020---2017-04-07

And this seems to (maybe) be where it was implemented (judging by commit msg):

https://github.com/dbrgn/drf-dynamic-fields/pull/14

Not sure. Maybe I'm misunderstanding something?

adinvadim commented 6 years ago

@wimglenn Any plans to implement this feature?

jsmedmar commented 6 years ago

Hey this would be so amazing!

johncpang commented 6 years ago

Yeah, I didn't see the code either, but I also didn't understand it all. Here's where they claim it supports nested fields:

https://github.com/dbrgn/drf-dynamic-fields/blob/master/CHANGELOG.md#020---2017-04-07

And this seems to (maybe) be where it was implemented (judging by commit msg):

dbrgn/drf-dynamic-fields#14

Not sure. Maybe I'm misunderstanding something?

From my understanding, they are fixing a mistake that the filter ALSO apply to subserializer. For example,

{
    "id": 1,
    "key1": "val1",
    "key2": {
        "id": 2,
        "key1": "valA"
    }
}

If you supply ?omit=key1, the expect result should be:

{
    "id": 1,
    "key2": {
        "id": 2,
        "key1": "valA"
    }
}

But actual result was:

{
    "id": 1,
    "key2": {
        "id": 2,
    }
}

The OP here, however, want to able to filter the nested serializer, and (I believe) should be explicit. So, ?fields!=key1 will only apply to first level but not sub-levels, and ?fields!=key2__key1 will only apply to second level but not root level.

johncpang commented 6 years ago

There is another project that said to have this functionality: drf_tweaks. Just have a brief look in their source and it's much bigger than djangorestframework-queryfields. Some of us prefer smaller better.

Here is a short summary after I tried both of them.

djangorestframework-queryfields works on first level only. drf_tweaks works on multiple levels (nested-serializer).

djangorestframework-queryfields allows you to define the list of fields you want. drf_tweaks allows you to define the list of fields you want.

djangorestframework-queryfields allows you to define the list of fields you don't want (and you get all the remainings). If you want all fields, do nothing. drf_tweaks can do similar but the other way: first excluding them using 'on_demand_fields', and bring them back using '?include_fields='. If you want all fields, you need to use '?include_fields'.

Maybe we can include both mixins on serializer (umm the serializer getting fat). Since both of them are available in mixins, we selectively apply one or both per serializer.

yezyilomo commented 5 years ago

I think django-restql is more advanced and simpler, it works much like GraphQL.

shafiyal commented 5 years ago

@mlissner I have checked 'drf-dynamic-fields' package but still it is also not doing fields filtering on nested fields

jsmedmar commented 5 years ago

I found a good intersection between djangorestframework-queryfields and django-restql that the owner of this repo may want to look at (or any other person). Here I propose a QueryFieldsMixin that supports both functionalities, using both projects, and motivated by these two ideas:

This solution supports queries in this format:

GET http://127.0.0.1:8000/things/?fields=id,key3__id, key3__key2

Query Syntax

First we need to be able to convert between djangorestframework-queryfields query syntax to django-restql syntax.

from collections import defaultdict

def fields2restql(include_str):
    """Convert include string to restql query format."""
    fields_list = [i for i in (include_str or "").split(",") if i]

    def _f2q(fields_list):
        query, fields_dict = "", defaultdict(list)

        for i in sorted(set(fields_list)):
            split = i.split("__", 1)

            if len(split) == 2:
                fields_dict[split[0].strip()].append(split[1].strip())
            else:
                fields_dict[split[0].strip()] = []

        for field, nested in sorted(fields_dict.items()):
            new_query = (field + " " + _f2q(nested)) if nested else field
            query = ", ".join(i for i in [query, new_query] if i)

        return "{{{0}}}".format(query) if query else query

    return _f2q(fields_list)

A new QueryFieldsMixin

This mixin can be used interchangeably with that from djangorestframework-queryfields.

from django_restql.exceptions import FieldNotFound
from django_restql.exceptions import FormatError
from django_restql.exceptions import InvalidField
from django_restql.mixins import get_formatted_query
from django_restql.mixins import parse_query
from drf_queryfields import QueryFieldsMixin
from rest_framework.exceptions import NotFound
from rest_framework.exceptions import ParseError
import dictfier

class CombinedQueryFieldsMixin(QueryFieldsMixin):
    def __init__(self, *args, **kwargs):
        """Add suport for restql filtering."""
        super(QueryFieldsMixin, self).__init__(  # pylint: disable=bad-super-call
            *args, **kwargs
        )

        try:
            request = self.context["request"]
        except (AttributeError, TypeError, KeyError):
            # The serializer was not initialized with request context.
            return

        query_params = getattr(request, "query_params", {})
        include_str = query_params.get(self.include_arg_name, "")

        if "__" in include_str:
            self.context["restql_query"] = self.get_dictfier_query(include_str)
            request._request.GET = request._request.GET.copy()
            request.query_params[self.include_arg_name] = ",".join(
                [i.split("__")[0] for i in include_str.split(",")]
            )

        super().__init__(*args, **kwargs)

    @property
    def data(self):
        ret = super().data
        query = self.context.get("restql_query")
        return dictfier.filter(ret, query) if query else ret

    def get_dictfier_query(self, include_str):
        try:
            raw_query = parse_query(fields2restql(include_str))
            schema = self.get_fields()
            return get_formatted_query(raw_query, schema)
        except FormatError as error:
            raise ParseError({"error": error})
        except FieldNotFound as error:
            raise NotFound({"error": error})
        except InvalidField as error:
            raise ParseError({"error": error})

    @classmethod
    def many_init(cls, *args, **kwargs):
        meta = getattr(cls, "Meta", None)

        if meta:
            list_serializer_class = getattr(
                meta, "list_serializer_class", serializers.ListSerializer
            )

            class ListSerializer(  # pylint:disable=abstract-method
                list_serializer_class
            ):
                @property
                def data(self):
                    ret = super().data
                    query = self.context.get("restql_query")
                    return dictfier.filter(ret, [query]) if query else ret

            meta.list_serializer_class = ListSerializer

        return super().many_init(*args, **kwargs)

Testing

For those interested I wrote some tests here https://github.com/yezyilomo/django-restql/issues/26.

Conclusions

Wanted to share this solution here in case other people may benefit from it. I think it also provides some hints into how djangorestframework-queryfields could be extended to support nested fields by using something like dictfier as in django-restql.