Django-rest-framework: DjangoFilterBackend deprecation breaks mro

[mattjmorrison]

Checklist

• [X] I have verified that that issue exists against the master branch of Django REST framework.
• [X] I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
• [X] This is not a usage question. (Those should be directed to the discussion group instead.)
• [X] This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
• [X] I have reduced the issue to the simplest possible case.
• [ ] I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

I ran into a really nasty bug today because of how we are using DjangoFilterBackend. Here is our scenario... we have a FilterBackend as well as a parent filter backend (that is really a mixin). Our backend extends our mixin as well as the rest_framework.filters.DjangoFilterBackend.... so it looks something like this

class NewDjangoFilterBackend:

    def filter_queryset(self):
        print("Inside django-filters")


class DefaultDRFBackwardsCompat:

    def __new__(*args, **kwargs):
        return NewDjangoFilterBackend()


class OurCustomerFilterBackendParent:

    def filter_queryset(self):
        print("Inside Our Parent Object")


class OurFilterBackend(OurCustomerFilterBackendParent, DefaultDRFBackwardsCompat):
    pass

OurFilterBackend().filter_queryset()

Expected behavior

(this is in Python 3.4.6 btw)

I expect, because of Python's normal MRO that filter_queryset on OurCustomerFilterBackendParent will be called.

Actual behavior

Due to the __new__ in DefaultDRFBackwardsCompat the filter_queryset inside of django-filters is called instead.

It is not very clear to my why this would happen... but there has to be a way to deprecate a class without breaking Python's MRO.

In our case, the fix to our bug was to just use the DjangoFilterBackend from django-filters instead of the one in rest_framework... but this just seems like a potentially dangerous way to make something be backwards compatible.

[xordoquy]

Interesting.

[mattjmorrison]

Here is another example that illuminates what is actually happening...

class NewDjangoFilterBackend:

    def filter_queryset(self):
        print("Inside django-filters")


class DefaultDRFBackwardsCompat:

    def __new__(*args, **kwargs):
        return NewDjangoFilterBackend()


class OurCustomerFilterBackendParent:

    def filter_queryset(self):
        print("Inside Our Parent Object")


class OurFilterBackend(OurCustomerFilterBackendParent, DefaultDRFBackwardsCompat):

    def x(self):
        print("in x")

OurFilterBackend().x()

>>> Traceback (most recent call last):
>>>   File "<stdin>", line 25, in <module>
>>> AttributeError: 'NewDjangoFilterBackend' object has no attribute 'x'

It looks like, because of the way __new__ works OurFilterBackend is just NewDjangoFilterBackend instead.

[carltongibson]

@mattjmorrison Can I just check, if you update to use django_filters.rest_framework.DjangoFilterBackend this issue goes away right? i.e. it's an issue with the deprecation wrapper.

[carltongibson]

Sorry, didn't get to the end:

In our case, the fix to our bug was to just use the DjangoFilterBackend from django-filters instead of the one in rest_framework... but this just seems like a potentially dangerous way to make something be backwards compatible.

The issue is here. There's no effort at all to handle a custom backend class.

I'm not sure of the cost/benefit of bullet-proofing this at point. Obviously a PR would be worth merging but it's a bit of an edge-case that'll drop out shortly. (It's quite interesting though)

[rpkilby]

It should be straightforward to fix this:

• The deprecation backend should inherit from django_filters.rest_framework.DjangoFilterBackend
• The __new__ implementation should raise the warnings and then just call super().__new__(...)

[xordoquy]

The deprecation backend should inherit from django_filters.rest_framework.DjangoFilterBackend

not going to work as it'll require django_filters to be installed in any case.

[rpkilby]

This could work with

if django_filters:
    DFBase = django_filters.rest_framework.DjangoFilterBackend
else:
    DFBase = BaseFilterBackend

class DjangoFilterBackend(DFBase):

[xordoquy]

yup could do.

[tomchristie]

Closed via #5117.