From 1c58834e11b11c3b4a49607d4b351f83c8be4afb Mon Sep 17 00:00:00 2001
From: Miroslav Shubernetskiy <miroslav@miki725.com>
Date: Wed, 14 Nov 2018 22:41:14 -0500
Subject: [PATCH] only running distinct on one-to-many filters (fixes #26)

---
 HISTORY.rst                   |  7 +++++++
 tests/backends/test_django.py | 21 ++++++++++++++++++---
 tox.ini                       |  6 ++++--
 url_filter/__init__.py        |  2 +-
 url_filter/backends/django.py | 16 +++++++++++++++-
 url_filter/utils.py           | 19 +++++++++++++++++++
 6 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/HISTORY.rst b/HISTORY.rst
index 68c3a2e..74b2075 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -3,6 +3,13 @@
 History
 -------
 
+0.3.10 (2018-11-14)
+~~~~~~~~~~~~~~~~~~
+
+* Only running ``distinct`` on queryset when one of filters is on one-to-many relation.
+  This should help with performance.
+  See `#26 <https://github.com/miki725/django-url-filter/issues/26>`_.
+
 0.3.9 (2018-11-12)
 ~~~~~~~~~~~~~~~~~~
 
diff --git a/tests/backends/test_django.py b/tests/backends/test_django.py
index 2186d77..97d4d69 100644
--- a/tests/backends/test_django.py
+++ b/tests/backends/test_django.py
@@ -70,6 +70,7 @@ def test_filter(self):
         qs = mock.Mock()
 
         backend = DjangoFilterBackend(qs)
+        backend.model = Place
         backend.bind([
             FilterSpec(['name'], 'exact', 'value', False),
             FilterSpec(['address'], 'contains', 'value', True),
@@ -77,10 +78,24 @@ def test_filter(self):
 
         result = backend.filter()
 
-        assert result == qs.filter.return_value.exclude.return_value.distinct.return_value
+        assert result == qs.filter.return_value.exclude.return_value
         qs.filter.assert_called_once_with(name__exact='value')
         qs.filter.return_value.exclude.assert_called_once_with(address__contains='value')
 
+    def test_filter_to_many(self):
+        qs = mock.Mock()
+
+        backend = DjangoFilterBackend(qs)
+        backend.model = Place
+        backend.bind([
+            FilterSpec(['restaurant', 'waiter', 'name'], 'exact', 'value', False),
+        ])
+
+        result = backend.filter()
+
+        assert result == qs.filter.return_value.distinct.return_value
+        qs.filter.assert_called_once_with(restaurant__waiter__name__exact='value')
+
     def test_filter_callable_specs(self):
         qs = mock.Mock()
 
@@ -93,5 +108,5 @@ def foo(queryset, spec):
 
         result = backend.filter()
 
-        assert result == qs.distinct.return_value.filter.return_value
-        qs.distinct.return_value.filter.assert_called_once_with(spec)
+        assert result == qs.filter.return_value
+        qs.filter.assert_called_once_with(spec)
diff --git a/tox.ini b/tox.ini
index 854ae19..1638959 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,12 +1,13 @@
 [tox]
 envlist =
-    {py27,py36,pypy,pypy3}-django{18,11}
-    {py36,pypy3}-django20
+    {py27,py36,py37,pypy,pypy3}-django{18,11}
+    {py36,py37,pypy3}-django{20,latest}
 
 [testenv]
 basepython =
     py27: python2.7
     py36: python3.6
+    py37: python3.7
     pypy: pypy
     pypy3: pypy3
 setenv =
@@ -20,6 +21,7 @@ deps =
     django18: djangorestframework<3.7
     django11: django<2
     django20: django<2.1
+    djangolatest: django
 whitelist_externals =
     make
 
diff --git a/url_filter/__init__.py b/url_filter/__init__.py
index c98f4f8..122b3c8 100644
--- a/url_filter/__init__.py
+++ b/url_filter/__init__.py
@@ -4,4 +4,4 @@
 
 __author__ = 'Miroslav Shubernetskiy'
 __email__ = 'miroslav@miki725.com'
-__version__ = '0.3.9'
+__version__ = '0.3.10'
diff --git a/url_filter/backends/django.py b/url_filter/backends/django.py
index ffeeab2..cf6bee1 100644
--- a/url_filter/backends/django.py
+++ b/url_filter/backends/django.py
@@ -1,8 +1,10 @@
 # -*- coding: utf-8 -*-
 from __future__ import absolute_import, print_function, unicode_literals
 
+from django.core.exceptions import FieldDoesNotExist
 from django.db.models.constants import LOOKUP_SEP
 
+from ..utils import suppress
 from .base import BaseFilterBackend
 
 
@@ -110,4 +112,16 @@ def filter_by_specs(self, queryset):
         for lookup, value in exclude.items():
             queryset = queryset.exclude(**{lookup: value})
 
-        return queryset.distinct()
+        to_many = self._is_any_to_many()
+        return queryset.distinct() if to_many else queryset
+
+    def _is_any_to_many(self):
+        return any(self._is_to_many(self.model, i.components) for i in self.regular_specs)
+
+    def _is_to_many(self, model, components):
+        if not components:
+            return False
+
+        with suppress(FieldDoesNotExist):
+            f = model._meta.get_field(components[0])
+            return f.one_to_many or f.many_to_many or self._is_to_many(f.related_model, components[1:])
diff --git a/url_filter/utils.py b/url_filter/utils.py
index 7eaf351..07683a7 100644
--- a/url_filter/utils.py
+++ b/url_filter/utils.py
@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 from __future__ import absolute_import, print_function, unicode_literals
 import inspect
+from contextlib import contextmanager
 
 
 class FilterSpec(object):
@@ -242,3 +243,21 @@ def dict_pop(key, d):
     """
     d.pop(key, None)
     return d
+
+
+@contextmanager
+def suppress(e):
+    """
+    Suppress given exception type
+
+    For example::
+
+        >>> with suppress(ValueError):
+        ...     print('test')
+        ...     raise ValueError
+        test
+    """
+    try:
+        yield
+    except e:
+        pass