Merge pull request #18 from elmkarami/17-using-filter-on-query-with-a…

…-join-creates-a-duplicate-join

using filter on query with a join creates a duplicate join

.pre-commit-config.yaml

@@ -8,7 +8,7 @@ minimum_pre_commit_version: 2.6.0
 repos:
 
   - repo: https://github.com/psf/black
-    rev: 20.8b1
+    rev: 22.3.0
     hooks:
       - id: black
         args:

setup.py

@@ -3,7 +3,7 @@
 
 HERE = pathlib.Path(__file__).parent
 
-version = "1.1.2"
+version = "1.1.3"
 
 
 setup(

sqlalchemy_filters/filters.py

@@ -32,7 +32,7 @@
 from sqlalchemy_filters.mixins import MarshmallowValidatorFilterMixin
 from sqlalchemy_filters.operators import AndOperator
 from sqlalchemy_filters.operators import BaseOperator
-from sqlalchemy_filters.utils import empty_sql, is_none
+from sqlalchemy_filters.utils import empty_sql, is_none, is_already_joined
 from sqlalchemy_filters.fields import (
     Field,
     MethodField,
@@ -361,6 +361,8 @@ def _apply_join(self, query):
         # remove duplicates and preserve order
         joins = list(OrderedDict.fromkeys(self._get_joins()))
         for join in joins:
+            if is_already_joined(query, join[0]):
+                continue
             query = query.join(join)
         return query
 

sqlalchemy_filters/utils.py

@@ -3,12 +3,15 @@
 from typing import Any
 
 from sqlalchemy import text  # type: ignore
-from sqlalchemy.orm import Query  # type: ignore
 from sqlalchemy import __version__
 
-
 SQLALCHEMY_VERSION = __version__
-IS_SQLALCHEMY_1_4 = __version__.startswith("1.4")
+IS_SQLALCHEMY_1_4 = __version__[0] == "1" and __version__[2] >= "4"
+
+
+if IS_SQLALCHEMY_1_4:
+    from sqlalchemy.sql import coercions
+    from sqlalchemy.sql import roles
 
 
 class Empty:
@@ -23,10 +26,15 @@ def empty_sql():
     return text("")
 
 
-def get_already_joined_tables(query: Query) -> list:
-    if IS_SQLALCHEMY_1_4:
-        return [joins[0].parent.entity for joins in query._setup_joins]
-    return [mapper.class_ for mapper in query._join_entities]
+def is_already_joined(query, model):
+    if not IS_SQLALCHEMY_1_4:
+        return model in [mapper.class_ for mapper in query._join_entities]
+    if hasattr(query, "_setup_joins"):
+        if model in [joins[0].parent.entity for joins in query._setup_joins]:
+            return True
+    if hasattr(query, "_legacy_setup_joins"):
+        join_table = coercions.expect(roles.JoinTargetRole, model, legacy=True)
+        return join_table in [_[0] for _ in query._legacy_setup_joins]
 
 
 def to_timezone(func):

tests/test_filters.py

@@ -259,6 +259,18 @@ def test_foreign_key_contains_operator():
     assert result[0] == a1
 
 
+def test_foreign_key_filter_with_query_contains_join(db_session):
+    user = UserFactory(first_name="First Name")
+    user2 = UserFactory(first_name="Name")
+    a1 = ArticleFactory(user=user)
+    ArticleFactory(user=user2)
+    query = db_session.query(Article).join(User)
+    filter_obj = ContainsFKFilter(data={"author_first_name": "First"}, query=query)
+    result = filter_obj.apply().all()
+    assert len(result) == 1
+    assert result[0] == a1
+
+
 def test_2_foreign_keys():
     user = UserFactory(first_name="First Name", last_name="some last name")
     user2 = UserFactory(first_name="Name", last_name="test name")
@@ -591,14 +603,11 @@ class Meta:
     user2 = UserFactory(first_name="John", last_name="Jack", age=20)
 
     assert set(MyFilter(data={"custom_key": "John"}).apply()) == {user, user2}
-    assert (
-        set(
-            MyFilter(
-                data={"custom_key": "John", "test2": "J"},
-            ).apply()
-        )
-        == {user2}
-    )
+    assert set(
+        MyFilter(
+            data={"custom_key": "John", "test2": "J"},
+        ).apply()
+    ) == {user2}
     assert set(
         MyFilter(data={"custom_key": "John", "test2": "J"}, operator=OrOperator).apply()
     ) == {user2, user}