MattBroach / DjangoRestMultipleModels

View (and mixin) for serializing multiple models or querysets in Django Rest Framework
MIT License
549 stars 67 forks source link

AssertionError with ViewSets #33

Closed chris-canipe closed 6 years ago

chris-canipe commented 6 years ago

I'm using Python 3.6.3, Django 1.11.6, DRF 3.7.7, version 2.0.1 of this library, and FlatMultipleModelAPIViewSet.

The issue I'm having is:

AssertionError at <URI>
<ViewSet>.get_queryset() returned None
...
Exception Location: <Path>/rest_framework/permissions.py in _queryset, line 126

This is in the code here:

class FlatMultipleModelAPIViewSet(FlatMultipleModelMixin, GenericViewSet):
    def get_queryset(self):
        return None

And it's running up against this check:

if hasattr(view, 'get_queryset'):
    queryset = view.get_queryset()
    assert queryset is not None, (
        '{}.get_queryset() returned None'.format(view.__class__.__name__)
    )
    return queryset
MattBroach commented 6 years ago

Thanks for the report. The usage of get_queryset there is mostly to trick previous DRF checks, since the FlatMultipleModelAPIViewSet bypasses the queryset attribute entirely. It looks like DRF changed it's queryset checks, though -- I should be able to return an empty Queryset instead of None. I'll roll out a fix in the next day or two.

MattBroach commented 6 years ago

Hey @epicyclist, sorry this took a little time, but I finally had a chance to look at this. My first read was incorrect -- the problem is not on the view/viewset, but, as you pointed out, in the DjangoModelPermission class. The short workaround that I referenced in my earlier comment won't work, because in order for the DjangoModelPermission to work, it needs some model on the queryset, but with the MultipleModelViewSet there are multiple models to check.

If you need to use a variant of that permission class, I'd recommend you write a new permission class that inherits from DjangoModelPermission but can handle ALL the querysets in the querylist. Something like this:

from rest_framework.permissions import DjangoModelPermission

class MultipleModelPermission(DjangoModelPermission):
    def has_permission(self, request, view):
        # Workaround to ensure DjangoModelPermissions are not applied
        # to the root view when using DefaultRouter.
        if getattr(view, '_ignore_model_permissions', False):
            return True

        if not request.user or (
           not request.user.is_authenticated and self.authenticated_users_only):
               return False

        querylist = self.get_querylist()
        has_perms = True
        for query in querylist:
            queryset = query['queryset']
            perms = self.get_required_permissions(request.method, queryset.model)
            has_perms = has_perms and request.user.has_perms(perms)

        return has_perms

You could then add that to the required viewset:

MyMultipleModelViewset(FlatMultipleModelAPIViewSet):
     permission_classes = (MultipleModelPermission,)
     ....

Please note that the above code is 100% UNTESTED and is not guaranteed to work -- it's merely meant to be guideposts to lead you in the right direction. I'm going to close this issue for now, but please feel free to post follow-up questions.

rlabrecque commented 5 years ago

This pointed me in the right direction, All I really needed was

from rest_framework import permissions

class MultipleModelPermissions(permissions.DjangoModelPermissions):
    def has_permission(self, request, view):
        # Workaround to ensure DjangoModelPermissions are not applied
        # to the root view when using DefaultRouter.
        return True

MyMultipleModelViewset(FlatMultipleModelAPIViewSet):
     permission_classes = (MultipleModelPermissions,)

The only note is that a trailing s was missing.

amar-muratovic commented 4 years ago

What could be possible reason why I'm seeing just first listed model? Second one is not showing up.

MattBroach commented 4 years ago

Hi @amar-muratovic, can you provide some sample code to reproduce the issue? It's very hard to answer without knowing the specifics of your code.

Also, this seems like a separate issue than the one under discussion here. I'd recommend opening a new issue rather than continuing this current (probably unrelated) discussion.