Nested routes allow creation of objects for another parent object

[felipe-prenholato]

So, consider two models as example:

class User(AbstractUser):
    pass

class Account(Model):
    user = ForeignKey(settings.AUTH_USER_MODEL)
    avatar = URLField()

This router:

router.register(r"users", UserViewSet) \
      .register(r"accounts", AccountViewSet, base_name="users-accounts",
                parents_query_lookups=["user"])

Now if you issue a POST to /users/1/accounts/ with {"user": 2, "avatar": "www.example.com"} the account is created for user with ID 2 instead ID 1.

Serializers (which create the stuff in DB) will use request.data and request.data have {"user": "2"}. DRF-Extensions need to force the correct user in this situations.

One workaround to this problem is inject correct user in request.data, something like:

class NestedAccountViewSet(NestedViewSetMixin, AccountViewSet):

    def initialize_request(self, request, *args, **kwargs):
        """
        Inject user from url in request.data
        """
        request = super(NestedAccountViewSet, self).initialize_request(
            request, *args, **kwargs
        )
        if request.data:
            request.data["user"] = kwargs["parent_loookup_user"]

        return request

Change the request.data like code above is easy, but I'm sure that isn't the best way to do it. Maybe DRF-Extensions can offer some custom serializer that receive extra information from url regex or do some magic in views so we don't need to do this injection?

[timb07]

I'm facing a similar problem, which is that the parent object shouldn't have to be specified in the request data, given it's in the URL already. I'm addressing it by adding to NestedViewSetMixin:

class NestedViewSetCreateMixin(NestedViewSetMixin):
    def perform_create(self, serializer):
        kwargs_id = {key + '_id': value for key, value in self.get_parents_query_dict().items()}
        serializer.save(**kwargs_id)

This probably doesn't correctly handle all possible configurations, but it's working for mine.

[auvipy]

@timb07 could you send a pr? we will then review and figure out proper work around?

[raphendyr]

Does #153 fix this?

[auvipy]

i didn't verify that, so im not sure

[Honorato]

I fixed my viewset by doing this:

class LendingViewSet(NestedViewSetMixin, ModelViewSet):
    serializer_class = serializers.LendingSerializer
    queryset = Lending.objects.all()

    def create(self, request, *args, **kwargs):
        parents_query_dict = self.get_parents_query_dict()
        if parents_query_dict:
            request.data.update(parents_query_dict)
        return super(LendingViewSet, self).create(request, *args, **kwargs)

    def update(self, request, *args, **kwargs):
        parents_query_dict = self.get_parents_query_dict()
        if parents_query_dict:
            request.data.update(parents_query_dict)
        return super(LendingViewSet, self).update(request, *args, **kwargs)

I believe this could be done by the mixin.

[Place1]

I've actually gone ahead and implemented this feature in my app like this:
This implementation also throws a 404, if the parent doesn't exist, before running the business logic on the viewset because I try to resolve the parents in initial(). One draw back of this approach is that serializers can't implement a queryset for the field.

from django.shortcuts import get_object_or_404


class NestedViewSetMixin:
    def initial(self, request, *args, **kwargs):
        # Before every request to this nested viewset we
        # want to resolve all the parent lookup kwargs into
        # the actual model instances.
        # We do this so that if they don't exist a 404 will
        # be raised.
        # We also cache the result on `self` so that
        # if the request is a POST, PUT or PATCH the parent
        # models can be reused in our perform_create and
        # perform_update handlers to avoid accessing the DB
        # twice.
        self.resolved_parents = self.resolve_parent_lookup_fields()
        return super().initial(request, *args, **kwargs)

    def get_queryset(self):
        return super().get_queryset().filter(**self.get_parent_lookup_fields())

    def perform_create(self, serializer):
        serializer.save(**self.resolved_parents)

    def perform_update(self, serializer):
        serializer.save(**self.resolved_parents)

    def get_parent_lookup_fields(self):
        lookup_fields = {}
        for key, value in self.kwargs.items():
            # For every kwargs the view receives we want to
            # find all the keys that begin with 'parent_lookup_'
            # because that's what our 'NestedRouterMixin' registers
            # parent lookup kwargs with.
            # Then for each of the parent lookups we want to remove
            # the 'parent_lookup_' prefix and return a new dictionary
            # with only the modified parent lookup fields
            if key.startswith('parent_lookup_'):
                parent_field = key.replace('parent_lookup_', '', 1)
                lookup_fields[parent_field] = value
        return lookup_fields

    def resolve_parent_lookup_fields(self):
        parent_lookups = self.get_parent_lookup_fields()
        resolved_parents = {}
        for key, value in parent_lookups.items():
            # the lookup key can be a django ORM query string like
            # 'project__slug' so we want to split on the first '__'
            # to get the related field's name, followed by the lookup
            # string for the related model. Using the given example
            # the related field will be 'project' and the 'slug' property
            # will be the lookup on that related model
            field, lookup = key.split('__', 1)
            related_model = self.queryset.model._meta.get_field(field).related_model # pylint: disable=protected-access
            resolved_parents[field] = get_object_or_404(related_model, **{lookup: value})
        return resolved_parents

[derek-adair]

OOOOF! This is a massive security issue and should be addressed. This issue is enough reason to not use this feature, which happens to be why I would use this project.

Workarounds are nice and all, but how about a fix on this?

[Misairu-G]

The other thing is, users that don't have permission on the parent, can still access child (by default).

[Misairu-G]

from collections import OrderedDict

from django.utils import six
from django.db.models import Model
from django.db.models.fields.related_descriptors import ForwardManyToOneDescriptor
from rest_framework.exceptions import NotFound
from rest_framework.viewsets import GenericViewSet
from rest_framework_extensions.settings import extensions_api_settings


class NestedGenericViewSet(GenericViewSet):
    """
    This ViewSet is a re-write of the original NestedViewSetMixin from rest_framework_extensions

    The original ViewSet is a little bit buggy regarding the following points:
    - It would return 200 even if the parent lookup does not exist
    - It would allow creation of objects for another parent object
    - FIXME: It does not check whether requester has the permission to access parent object

    The rewrite is based on https://github.com/chibisov/drf-extensions/issues/142,
    credit to @Place1 for the ideas and sample implementation.
    """
    resolved_parents = OrderedDict()

    def initial(self, request, *args, **kwargs) -> None:
        """
        Resolve parent objects.

        Before every request to this nested viewset we want to resolve all the parent
        lookup kwargs into the actual model instances.

        We do this so that if they don't exist a 404 will be raised.

        We also cache the result on `self` so that if the request is a POST, PUT or
        PATCH the parent models can be reused in our perform_create and perform_update
        handlers to avoid accessing the DB twice.
        """
        super(NestedGenericViewSet, self).initial(request, *args, **kwargs)
        self.resolve_parent_lookup_fields()

    def get_queryset(self):
        return self.filter_queryset_by_parents_lookups(
                super(NestedGenericViewSet, self).get_queryset()
        )

    def filter_queryset_by_parents_lookups(self, queryset):
        parents_query_dict = self.get_parents_query_dict()
        if parents_query_dict:
            try:
                return queryset.filter(**parents_query_dict)
            except ValueError:
                raise NotFound()
        else:
            return queryset

    def get_parents_query_dict(self) -> OrderedDict:
        result = OrderedDict()
        for kwarg_name, kwarg_value in six.iteritems(self.kwargs):
            if kwarg_name.startswith(
                    extensions_api_settings.DEFAULT_PARENT_LOOKUP_KWARG_NAME_PREFIX):
                query_lookup = kwarg_name.replace(
                        extensions_api_settings.DEFAULT_PARENT_LOOKUP_KWARG_NAME_PREFIX,
                        '',
                        1
                )
                query_value = kwarg_value
                result[query_lookup] = query_value
        return result

    def resolve_parent_lookup_fields(self) -> None:
        """Update resolved parents to the instance variable"""

        parents_query_dict = self.get_parents_query_dict()

        keys = list(parents_query_dict.keys())

        for i in range(len(keys)):
            # the lookup key can be a django ORM query string like 'project__slug'
            # so we want to split on the first '__' to get the related field's name,
            # followed by the lookup string for the related model. Using the given
            # example the related field will be 'project' and the 'slug' property
            # will be the lookup on that related model

            # TODO: support django ORM query string, like 'project__slug'
            field = keys[i]
            value = parents_query_dict[keys[i]]

            related_descriptor: ForwardManyToOneDescriptor = getattr(self.queryset.model, field)
            related_model: Model = related_descriptor.field.related_model

            # The request must have all previous parents matched, for example
            # /contracts/2/assessment-methods/1/examine/ must satisfies assessment-method=1
            # can be query by contract=2
            previous_parents = {k: self.resolved_parents[k] for k in keys[:i]}

            try:
                self.resolved_parents[field] = related_model.objects.get(
                        **{'pk': value, **previous_parents})
            except related_model.DoesNotExist:
                raise NotFound()

Improved solution based on @Place1 's, work under Python 3.7 with Django 2.2 and DRF 3.10.

The line field, lookup = key.split('__', 1) is deleted as would cause error if no slug is given, since I currently only use primary key for index, it fits my minimal requirement.

Feel free to use it and add your own modification.

At the end, I don't feel like nested router is a good idea, nested view set make more sense for me. But I don't really have time to switch to other solution right now.

---

Edit: If you're reading this, please keep in mind that nested route/viewset for resources is a bad practice when designing RESTful API, and you should generally avoid doing this. Nesting resources would result unnecessary complication for dependencies, instead, you should use hyperlink for related resources.

Refer here for details about best practice of RESTful API design.

[derek-adair]

What do you mean nested viewset?

Meaning just manually coded routes?

[Misairu-G]

@derek-adair Means once the parent ViewSet is being registered in the router, all its child ViewSet would be automatically registered. Similar case is the action decorator of rest framework.

[derek-adair]

Anyone got a link to an example? I'm assuming that it's outside of this project as I dont see any references to nested viewsets here.