allow extended from_kwargs calls & small fixes (#218)

Changes: - from_kwargs supports now complex kwargs like in queryset - failure querying when using proxy model table and kwargs - proxy and main model use now the same tables - model_class in query is always not a proxy model - fix stacklevel of performance warning - deprecate passing an object for from_kwargs - add dedicated warning for unconnected databases - document and test the performance warning - bump version
dymmond · Oct 30, 2024 · 35e2fe2 · 35e2fe2
1 parent 8634aea
commit 35e2fe2
Show file tree

Hide file tree

Showing 14 changed files with 450 additions and 142 deletions.
diff --git a/docs/queries/queries.md b/docs/queries/queries.md
@@ -275,10 +275,10 @@ For this there is an extension of edgy's `and_` and `or_` which takes a model or
 matches kwargs against:
 
 ```python
-users = await User.query.filter(and_.from_kwargs(User, name="foo", email="[email protected]"))
+users = await User.query.filter(and_.from_kwargs(name="foo", email="[email protected]"))
 # or
 
-users = await User.query.filter(and_.from_kwargs(User, **my_dict))
+users = await User.query.filter(and_.from_kwargs(**my_dict))
 ```
 
 #### OR
@@ -1126,8 +1126,36 @@ sql types because otherwise some features are not supported or cause warnings.
 
 ## Debugging
 
+### Getting the SQL query
+
 QuerySet contains a cached debug property named `sql` which contains the QuerySet as query with inserted blanks.
 
+### Performance warnings
+
+Edgy issues a `DatabaseNotConnectedWarning` when using edgy without a connected database. To silence it, wrap the affected
+code in a database scope
+
+``` python
+await model.save()
+# becomes
+async with model.database:
+    await model.save()
+```
+
+If the warning is completely unwanted despite the performance impact, you can filter:
+
+``` python
+import warnings
+from edgy.exceptions import DatabaseNotConnectedWarning
+with warnings.catch_warnings(action="ignore", category=DatabaseNotConnectedWarning):
+    await model.save()
+```
+
+It inherits from `UserWarning` so it is possible to filter UserWarnings.
+
+However the silencing way is not recommended.
+
+
 [model]: ../models.md
 [managers]: ../managers.md
 [lambda statements](https://docs.sqlalchemy.org/en/20/core/sqlelement.html#sqlalchemy.sql.expression.lambda_stmt)
diff --git a/docs/release-notes.md b/docs/release-notes.md
@@ -6,6 +6,21 @@ hide:
 
 # Release Notes
 
+## 0.21.1
+
+### Changed
+
+- Breaking: from_kwargs doesn't require model or table anymore. It is simply ignored.
+
+### Fixed
+
+- Q, and_, or_ support now complex kwargs like querysets.
+- Failure querying when using proxy model table and kwargs.
+- Proxy and main model use now the same tables.
+  This could have been a problem when filtering against table columns of the proxy table in a query from the main table.
+- Queries operate now always on the main model not the proxy model.
+- Stacklevel of performance warning was wrong.
+
 ## 0.21.0
 
 ### Added

diff --git a/edgy/__init__.py b/edgy/__init__.py
@@ -1,4 +1,4 @@
-__version__ = "0.21.0"
+__version__ = "0.21.1"
 
 from .cli.base import Migrate
 from .conf import settings

diff --git a/edgy/core/db/models/metaclasses.py b/edgy/core/db/models/metaclasses.py
@@ -784,6 +784,8 @@ def table(cls) -> sqlalchemy.Table:
         2. If a db_schema in the `registry` is passed, then it will use that as a default.
         3. If none is passed, defaults to the shared schema of the database connected.
         """
+        if cls.__is_proxy_model__:
+            return cls.__parent__.table  # type: ignore
         if not cls.meta.registry:
             # we cannot set the table without a registry
             # raising is required
@@ -841,6 +843,10 @@ def table_schema(
         Retrieve table for schema (nearly the same as build with scheme argument).
         Cache per class via a primitive LRU cache.
         """
+        if cls.__is_proxy_model__:
+            return cls.__parent__.table_schema(  # type: ignore
+                schema, metadata=metadata, update_cache=update_cache
+            )
         if schema is None or (cls.get_db_schema() or "") == schema:
             # sqlalchemy uses "" for empty schema
             table = getattr(cls, "_table", None)

diff --git a/edgy/core/db/models/mixins/db.py b/edgy/core/db/models/mixins/db.py
@@ -209,7 +209,7 @@ def table(self) -> sqlalchemy.Table:
             schema = self.get_active_instance_schema()
             return cast(
                 "sqlalchemy.Table",
-                self.table_schema(schema),
+                self.__class__.table_schema(schema),
             )
         return self._table
 
@@ -286,7 +286,7 @@ async def _update(self: "Model", **kwargs: Any) -> Any:
         token = CURRENT_INSTANCE.set(self)
         try:
             if column_values and clauses:
-                check_db_connection(self.database)
+                check_db_connection(self.database, stacklevel=4)
                 async with self.database as database, database.transaction():
                     # can update column_values
                     column_values.update(
@@ -393,7 +393,7 @@ async def _insert(self: "Model", **kwargs: Any) -> "Model":
             instance=self,
             model_instance=self,
         )
-        check_db_connection(self.database)
+        check_db_connection(self.database, stacklevel=4)
         token = CURRENT_INSTANCE.set(self)
         try:
             async with self.database as database, database.transaction():

diff --git a/edgy/core/db/querysets/base.py b/edgy/core/db/querysets/base.py
@@ -1,4 +1,3 @@
-import asyncio
 import warnings
 from collections.abc import AsyncIterator, Awaitable, Generator, Iterable, Sequence
 from collections.abc import Iterable as CollectionsIterable
@@ -19,7 +18,7 @@
 from edgy.core.db.context_vars import CURRENT_INSTANCE, MODEL_GETATTR_BEHAVIOR, get_schema
 from edgy.core.db.datastructures import QueryModelResultCache
 from edgy.core.db.fields import CharField, TextField
-from edgy.core.db.fields.base import BaseField, BaseForeignKey, RelationshipField
+from edgy.core.db.fields.base import BaseForeignKey, RelationshipField
 from edgy.core.db.models.model_reference import ModelRef
 from edgy.core.db.models.types import BaseModelType
 from edgy.core.db.models.utils import apply_instance_extras
@@ -41,7 +40,6 @@
     from edgy.core.db.fields.types import BaseFieldType
 
 
-generic_field = BaseField()
 _empty_set = cast(Sequence[Any], frozenset())
 
 
@@ -53,32 +51,6 @@ def get_table_key_or_name(table: Union[sqlalchemy.Table, sqlalchemy.Alias]) -> s
         return table.name
 
 
-def clean_query_kwargs(
-    model_class: type[BaseModelType],
-    kwargs: dict[str, Any],
-    embed_parent: Optional[tuple[str, str]] = None,
-    model_database: Optional["Database"] = None,
-) -> dict[str, Any]:
-    new_kwargs: dict[str, Any] = {}
-    for key, val in kwargs.items():
-        if embed_parent:
-            if embed_parent[1] and key.startswith(embed_parent[1]):
-                key = key.removeprefix(embed_parent[1]).removeprefix("__")
-            else:
-                key = f"{embed_parent[0]}__{key}"
-        sub_model_class, field_name, _, _, _, cross_db_remainder = crawl_relationship(
-            model_class, key, model_database=model_database
-        )
-        # we preserve the uncleaned argument
-        field = None if cross_db_remainder else sub_model_class.meta.fields.get(field_name)
-        if field is not None and not callable(val):
-            new_kwargs.update(field.clean(key, val, for_query=True))
-        else:
-            new_kwargs[key] = val
-    assert "pk" not in new_kwargs, "pk should be already parsed"
-    return new_kwargs
-
-
 class BaseQuerySet(
     TenancyMixin,
     QuerySetPropsMixin,
@@ -114,6 +86,11 @@ def __init__(
         table: Optional[sqlalchemy.Table] = None,
         exclude_secrets: bool = False,
     ) -> None:
+        # Making sure for queries we use the main class and not the proxy
+        # And enable the parent
+        if model_class.__is_proxy_model__:
+            model_class = model_class.__parent__
+
         super().__init__(model_class=model_class)
         self.filter_clauses: list[Any] = list(filter_clauses)
         self.or_clauses: list[Any] = []
@@ -236,17 +213,6 @@ def _build_group_by_expression(self, group_by: Any, expression: Any) -> Any:
         expression = expression.group_by(*(self._prepare_order_by(entry) for entry in group_by))
         return expression
 
-    async def _resolve_clause_args(
-        self, args: Any, tables_and_models: tables_and_models_type
-    ) -> Any:
-        result: list[Any] = []
-        for arg in args:
-            result.append(clauses_mod.parse_clause_arg(arg, self, tables_and_models))
-        if self.database.force_rollback:
-            return [await el for el in result]
-        else:
-            return await asyncio.gather(*result)
-
     async def build_where_clause(
         self, _: Any = None, tables_and_models: Optional[tables_and_models_type] = None
     ) -> Any:
@@ -257,19 +223,22 @@ async def build_where_clause(
         # ignored args for passing build_where_clause in filter_clauses
         where_clauses: list[Any] = []
         if self.or_clauses:
-            or_clauses = await self._resolve_clause_args(self.or_clauses, tables_and_models)
             where_clauses.append(
-                or_clauses[0] if len(or_clauses) == 1 else clauses_mod.or_(*or_clauses)
+                await clauses_mod.parse_clause_arg(
+                    clauses_mod.or_(*self.or_clauses, no_select_related=True),
+                    self,
+                    tables_and_models,
+                )
             )
 
         if self.filter_clauses:
             # we AND by default
             where_clauses.extend(
-                await self._resolve_clause_args(self.filter_clauses, tables_and_models)
+                await clauses_mod.parse_clause_args(self.filter_clauses, self, tables_and_models)
             )
         # for nicer unpacking
         if joins is None or len(tables_and_models) == 1:
-            return clauses_mod.and_(*where_clauses)
+            return clauses_mod.and_sqlalchemy(*where_clauses)
         expression = sqlalchemy.sql.select(
             *(
                 getattr(tables_and_models[""][0].c, col)
@@ -438,7 +407,7 @@ def _build_tables_join_from_relationship(
                         former_table = table
                         former_transition = transition_key
                         continue
-                    and_clause = clauses_mod.and_(
+                    and_clause = clauses_mod.and_sqlalchemy(
                         *self._select_from_relationship_clause_generator(
                             foreign_key, table, reverse, former_table
                         )
@@ -586,13 +555,7 @@ def _kwargs_to_clauses(
     ) -> tuple[list[Any], set[str]]:
         clauses = []
         select_related: set[str] = set()
-
-        # Making sure for queries we use the main class and not the proxy
-        # And enable the parent
-        if self.model_class.__is_proxy_model__:
-            self.model_class = self.model_class.__parent__
-
-        cleaned_kwargs = clean_query_kwargs(
+        cleaned_kwargs = clauses_mod.clean_query_kwargs(
             self.model_class, kwargs, self.embed_parent_filters, model_database=self.database
         )
 
@@ -602,9 +565,9 @@ def _kwargs_to_clauses(
             )
             if related_str:
                 select_related.add(related_str)
-            field = model_class.meta.fields.get(field_name, generic_field)
+            field = model_class.meta.fields.get(field_name, clauses_mod.generic_field)
             if cross_db_remainder:
-                assert field is not generic_field
+                assert field is not clauses_mod.generic_field
                 fk_field = cast(BaseForeignKey, field)
                 sub_query = (
                     fk_field.target.query.filter(**{cross_db_remainder: value})
@@ -648,6 +611,7 @@ async def wrapper(
                     table = tables_and_models[_prefix][0]
                     return _field.operator_to_clause(_field.name, _op, table, _value)
 
+                wrapper._edgy_force_callable_queryset_filter = True
                 clauses.append(wrapper)
 
         return clauses, select_related
@@ -833,7 +797,7 @@ async def _execute_iterate(
 
         counter = 0
         last_element: Optional[tuple[BaseModelType, BaseModelType]] = None
-        check_db_connection(queryset.database)
+        check_db_connection(queryset.database, stacklevel=4)
         if fetch_all_at_once:
             async with queryset.database as database:
                 batch = cast(Sequence[sqlalchemy.Row], await database.fetch_all(expression))
@@ -919,31 +883,7 @@ def _filter_or_exclude(
                     queryset._select_related.update(related)
                     queryset._cached_select_related_expression = None
                 if or_ and extracted_clauses:
-
-                    async def wrapper_and(
-                        queryset: "QuerySet",
-                        tables_and_models: tables_and_models_type,
-                        _extracted_clauses: Sequence[
-                            Union[
-                                "sqlalchemy.sql.expression.BinaryExpression",
-                                Callable[
-                                    ["QuerySetType"],
-                                    Union[
-                                        "sqlalchemy.sql.expression.BinaryExpression",
-                                        Awaitable["sqlalchemy.sql.expression.BinaryExpression"],
-                                    ],
-                                ],
-                            ]
-                        ] = extracted_clauses,
-                    ) -> Any:
-                        return clauses_mod.and_(
-                            *(
-                                await self._resolve_clause_args(
-                                    _extracted_clauses, tables_and_models
-                                )
-                            )
-                        )
-
+                    wrapper_and = clauses_mod.and_(*extracted_clauses, no_select_related=True)
                     if allow_global_or and len(clauses) == 1:
                         # add to global or
                         assert not exclude
@@ -960,33 +900,28 @@ async def wrapper_and(
                 if not queryset._select_related.issuperset(raw_clause._select_related):
                     queryset._select_related.update(raw_clause._select_related)
                     queryset._cached_select_related_expression = None
-
             else:
                 converted_clauses.append(raw_clause)
+                if hasattr(raw_clause, "_edgy_calculate_select_related"):
+                    select_related_calculated = raw_clause._edgy_calculate_select_related(queryset)
+                    if not queryset._select_related.issuperset(select_related_calculated):
+                        queryset._select_related.update(select_related_calculated)
+                        queryset._cached_select_related_expression = None
         if not converted_clauses:
             return queryset
 
         if exclude:
             op = clauses_mod.and_ if not or_ else clauses_mod.or_
 
-            async def wrapper(
-                queryset: "QuerySet", tables_and_models: tables_and_models_type
-            ) -> Any:
-                return clauses_mod.not_(
-                    op(*(await self._resolve_clause_args(converted_clauses, tables_and_models)))
+            queryset.filter_clauses.append(
+                clauses_mod.not_(
+                    op(*converted_clauses, no_select_related=True), no_select_related=True
                 )
-
-            queryset.filter_clauses.append(wrapper)
+            )
         elif or_:
-
-            async def wrapper(
-                queryset: "QuerySet", tables_and_models: tables_and_models_type
-            ) -> Any:
-                return clauses_mod.or_(
-                    *(await self._resolve_clause_args(converted_clauses, tables_and_models))
-                )
-
-            queryset.filter_clauses.append(wrapper)
+            queryset.filter_clauses.append(
+                clauses_mod.or_(*converted_clauses, no_select_related=True)
+            )
         else:
             # default to and
             queryset.filter_clauses.extend(converted_clauses)
@@ -1026,7 +961,7 @@ async def _get_raw(self, **kwargs: Any) -> tuple[BaseModelType, Any]:
             return await filter_query._get_raw()
 
         expression, tables_and_models = await self.as_select_with_tables()
-        check_db_connection(self.database)
+        check_db_connection(self.database, stacklevel=4)
         async with self.database as database:
             # we want no queryset copy, so use sqlalchemy limit(2)
             rows = await database.fetch_all(expression.limit(2))