Add support to secondary tables relationships

.devcontainer/devcontainer.json

@@ -10,7 +10,6 @@
     "python.pythonPath": "/usr/local/bin/python",
     "python.linting.enabled": true,
     "python.linting.pylintEnabled": true,
-    "python.formatting.autopep8Path": "/usr/local/py-utils/bin/autopep8",
     "python.formatting.blackPath": "/usr/local/py-utils/bin/black",
     "python.formatting.yapfPath": "/usr/local/py-utils/bin/yapf",
     "python.linting.banditPath": "/usr/local/py-utils/bin/bandit",

.pre-commit-config.yaml

@@ -1,12 +1,12 @@
 repos:
   - repo: https://github.com/psf/black
-    rev: 24.4.2
+    rev: 24.8.0 # Do not update this repository; it is pinned for compatibility with python 3.8
     hooks:
       - id: black
         exclude: ^tests/\w+/snapshots/
 
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.4.5
+    rev: v0.9.3
     hooks:
       - id: ruff
         exclude: ^tests/\w+/snapshots/
@@ -24,7 +24,7 @@ repos:
         files: '^docs/.*\.mdx?$'
 
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.6.0
+    rev: v5.0.0
     hooks:
       - id: trailing-whitespace
       - id: check-merge-conflict
@@ -33,7 +33,7 @@ repos:
       - id: check-toml
 
   - repo: https://github.com/adamchainz/blacken-docs
-    rev: 1.16.0
+    rev: 1.18.0 # Do not update this repository; it is pinned for compatibility with python 3.8
     hooks:
       - id: blacken-docs
         args: [--skip-errors]

RELEASE.md

@@ -0,0 +1,3 @@
+Release type: patch
+
+Add support for secondary table relationships in SQLAlchemy mapper, addressing a bug and enhancing the loader to handle these relationships efficiently.

src/strawberry_sqlalchemy_mapper/exc.py

@@ -36,3 +36,11 @@ def __init__(self, model):
             f"Model `{model}` is not polymorphic or is not the base model of its "
             + "inheritance chain, and thus cannot be used as an interface."
         )
+
+
+class InvalidLocalRemotePairs(Exception):
+    def __init__(self, relationship_name):
+        super().__init__(
+            f"The `local_remote_pairs` for the relationship `{relationship_name}` is invalid or missing. "
+            + "This is likely an issue with the library. Please report this error to the maintainers."
+        )

src/strawberry_sqlalchemy_mapper/loader.py

@@ -18,6 +18,8 @@
 from sqlalchemy.orm import RelationshipProperty, Session
 from strawberry.dataloader import DataLoader
 
+from strawberry_sqlalchemy_mapper.exc import InvalidLocalRemotePairs
+
 
 class StrawberrySQLAlchemyLoader:
     """
@@ -45,13 +47,22 @@ def __init__(
                 "One of bind or async_bind_factory must be set for loader to function properly."
             )
 
-    async def _scalars_all(self, *args, **kwargs):
+    async def _scalars_all(self, *args, query_secondary_tables=False, **kwargs):
+        # query_secondary_tables explanation:
+        # We need to retrieve values from both the self_model and related_model.
+        # To achieve this, we must disable the default SQLAlchemy optimization
+        # that returns only related_model values.
+        # This is necessary because we use the keys variable
+        # to match both related_model and self_model.
         if self._async_bind_factory:
             async with self._async_bind_factory() as bind:
+                if query_secondary_tables:
+                    return (await bind.execute(*args, **kwargs)).all()
                 return (await bind.scalars(*args, **kwargs)).all()
-        else:
-            assert self._bind is not None
-            return self._bind.scalars(*args, **kwargs).all()
+        assert self._bind is not None
+        if query_secondary_tables:
+            return self._bind.execute(*args, **kwargs).all()
+        return self._bind.scalars(*args, **kwargs).all()
 
     def loader_for(self, relationship: RelationshipProperty) -> DataLoader:
         """
@@ -63,14 +74,81 @@ def loader_for(self, relationship: RelationshipProperty) -> DataLoader:
             related_model = relationship.entity.entity
 
             async def load_fn(keys: List[Tuple]) -> List[Any]:
-                query = select(related_model).filter(
-                    tuple_(
-                        *[remote for _, remote in relationship.local_remote_pairs or []]
-                    ).in_(keys)
+                def _build_normal_relationship_query(related_model, relationship, keys):
+                    return select(related_model).filter(
+                        tuple_(
+                            *[
+                                remote
+                                for _, remote in relationship.local_remote_pairs or []
+                            ]
+                        ).in_(keys)
+                    )
+
+                def _build_relationship_with_secondary_table_query(
+                    related_model, relationship, keys
+                ):
+                    # Use another query when relationship uses a secondary table
+                    self_model = relationship.parent.entity
+
+                    if not relationship.local_remote_pairs:
+                        raise InvalidLocalRemotePairs(
+                            f"{related_model.__name__} -- {self_model.__name__}"
+                        )
+
+                    self_model_key_label = str(
+                        relationship.local_remote_pairs[0][1].key
+                    )
+                    related_model_key_label = str(
+                        relationship.local_remote_pairs[1][1].key
+                    )
+
+                    self_model_key = str(relationship.local_remote_pairs[0][0].key)
+                    related_model_key = str(relationship.local_remote_pairs[1][0].key)
+
+                    remote_to_use = relationship.local_remote_pairs[0][1]
+                    query_keys = tuple([item[0] for item in keys])
+
+                    # This query returns rows in this format -> (self_model.key, related_model)
+                    return (
+                        select(
+                            getattr(self_model, self_model_key).label(
+                                self_model_key_label
+                            ),
+                            related_model,
+                        )
+                        .join(
+                            relationship.secondary,
+                            getattr(relationship.secondary.c, related_model_key_label)
+                            == getattr(related_model, related_model_key),
+                        )
+                        .join(
+                            self_model,
+                            getattr(relationship.secondary.c, self_model_key_label)
+                            == getattr(self_model, self_model_key),
+                        )
+                        .filter(remote_to_use.in_(query_keys))
+                    )
+
+                query = (
+                    _build_normal_relationship_query(related_model, relationship, keys)
+                    if relationship.secondary is None
+                    else _build_relationship_with_secondary_table_query(
+                        related_model, relationship, keys
+                    )
                 )
+
                 if relationship.order_by:
                     query = query.order_by(*relationship.order_by)
-                rows = await self._scalars_all(query)
+
+                if relationship.secondary is not None:
+                    # We need to retrieve values from both the self_model and related_model.
+                    # To achieve this, we must disable the default SQLAlchemy optimization
+                    # that returns only related_model values.
+                    # This is necessary because we use the keys variable
+                    # to match both related_model and self_model.
+                    rows = await self._scalars_all(query, query_secondary_tables=True)
+                else:
+                    rows = await self._scalars_all(query)
 
                 def group_by_remote_key(row: Any) -> Tuple:
                     return tuple(
@@ -82,8 +160,13 @@ def group_by_remote_key(row: Any) -> Tuple:
                     )
 
                 grouped_keys: Mapping[Tuple, List[Any]] = defaultdict(list)
-                for row in rows:
-                    grouped_keys[group_by_remote_key(row)].append(row)
+                if relationship.secondary is None:
+                    for row in rows:
+                        grouped_keys[group_by_remote_key(row)].append(row)
+                else:
+                    for row in rows:
+                        grouped_keys[(row[0],)].append(row[1])
+
                 if relationship.uselist:
                     return [grouped_keys[key] for key in keys]
                 else:

src/strawberry_sqlalchemy_mapper/mapper.py

@@ -82,6 +82,7 @@
 from strawberry_sqlalchemy_mapper.exc import (
     HybridPropertyNotAnnotated,
     InterfaceModelNotPolymorphic,
+    InvalidLocalRemotePairs,
     UnsupportedAssociationProxyTarget,
     UnsupportedColumnType,
     UnsupportedDescriptorType,
@@ -387,7 +388,7 @@ def _convert_relationship_to_strawberry_type(
         if relationship.uselist:
             # Use list if excluding relay pagination
             if use_list:
-                return List[ForwardRef(type_name)] # type: ignore
+                return List[ForwardRef(type_name)]  # type: ignore
 
             return self._connection_type_for(type_name)
         else:
@@ -500,13 +501,30 @@ async def resolve(self, info: Info):
             if relationship.key not in instance_state.unloaded:
                 related_objects = getattr(self, relationship.key)
             else:
-                relationship_key = tuple(
-                    [
+                if relationship.secondary is None:
+                    relationship_key = tuple(
                         getattr(self, local.key)
                         for local, _ in relationship.local_remote_pairs or []
                         if local.key
-                    ]
-                )
+                    )
+                else:
+                    # If has a secondary table, gets only the first ID as additional IDs require a separate query
+                    if not relationship.local_remote_pairs:
+                        raise InvalidLocalRemotePairs(
+                            f"{relationship.entity.entity.__name__} -- {relationship.parent.entity.__name__}"
+                        )
+
+                    local_remote_pairs_secondary_table_local = (
+                        relationship.local_remote_pairs[0][0]
+                    )
+                    relationship_key = tuple(
+                        [
+                            getattr(
+                                self, str(local_remote_pairs_secondary_table_local.key)
+                            ),
+                        ]
+                    )
+
                 if any(item is None for item in relationship_key):
                     if relationship.uselist:
                         return []
@@ -536,7 +554,9 @@ def connection_resolver_for(
         if relationship.uselist and not use_list:
             return self.make_connection_wrapper_resolver(
                 relationship_resolver,
-                self.model_to_type_or_interface_name(relationship.entity.entity),  # type: ignore[arg-type]
+                self.model_to_type_or_interface_name(
+                    relationship.entity.entity  # type: ignore[arg-type]
+                ),
             )
         else:
             return relationship_resolver
@@ -785,7 +805,8 @@ def convert(type_: Any) -> Any:
             # ignore inherited `is_type_of`
             if "is_type_of" not in type_.__dict__:
                 type_.is_type_of = (
-                    lambda obj, info: type(obj) == model or type(obj) == type_
+                    lambda obj, info: type(obj) == model  # noqa: E721
+                    or type(obj) == type_  # noqa: E721
                 )
 
             # Default querying methods for relay