change m2m traversing default, performance & accessing secrets doesn'…

…t trigger load (#266) Changes: - Change the default traversal behaviour of manytomany fields - Fix handling unknown fields via the generic field - exclude secrets from triggering load - add debugging and performance tips - allow excluding attrs from triggering loads - fully initialized models by queryset are marked as loaded
dymmond · Jan 21, 2025 · ea6a5b8 · ea6a5b8
1 parent 6444489
commit ea6a5b8
Show file tree

Hide file tree

Showing 24 changed files with 330 additions and 74 deletions.
diff --git a/docs/debugging.md b/docs/debugging.md
@@ -0,0 +1,51 @@
+# Debugging & Performance
+
+Edgy has several debug features, also through databasez. It tries also to keep it's eventloop and use the most smartest way
+to execute a query performant.
+For example asyncio pools are thread protected in databasez so it is possible to keep the connections to the database open.
+
+But this requires that databases and registries are not just thrown away but kept open during the operation. For getting a
+sane lifespan a reference counter are used.
+
+When dropped to 0 the database is uninitialized and drops the connections.
+
+There is no problem re-opening the database but it is imperformant and can have side-effects especcially with the `DatabaseTestClient`.
+For this the `DatabaseNotConnectedWarning` warning exist.
+
+## `DatabaseNotConnectedWarning` warning
+
+The most common warning in edgy is probably the `DatabaseNotConnectedWarning` warning.
+
+It is deliberate and shall guide the user to improve his code so he doesn't throws away engines unneccessarily.
+Also it could lead in test environments to hard to debug errors because of a missing database (drop_database parameter).
+
+## Many connections
+
+If the database is slow due to many connections by edgy and no `DatabaseNotConnectedWarning` warning was raised
+it indicates that deferred fields are accessed.
+This includes ForeignKey, which models are not prefetched via `select_related`.
+
+### Debugging deferred loads
+
+For debugging purposes (but sacrificing deferred loads with it) you can set the ContextVariable
+`edgy.core.context_vars.MODEL_GETATTR_BEHAVIOR` to `"passdown"` instead of `"load"`.
+
+This will lead to crashes in case an implicit loaded variable is accessed.
+
+### Optimizing ReflectedModel
+
+ReflectedModel have the problem that not all database fields are known. Therefor testing if an optional attribute
+is available via `getattr`/`hasattr` will lead to a load first.
+
+There are two ways to work around:
+
+1. Use the model instance dict instead (e.g. `model.__dict__.get("foo")` or `"foo" in model.__dict__`).
+2. Add the optional available attributes to `__no_load_trigger_attrs__`. They won't trigger an load anymore.
+
+## Hangs
+
+Hangs typical occur when there is only **one** connection available or the database is blocked.
+This is normally easily debuggable often with the same ways like mentioned before because of the same reasons.
+If it has hard to debug stack traces, it seems that threads and asyncio are mixed.
+
+Here you can enforce hard timeouts via the `DATABASEZ_RESULT_TIMEOUT` environment variable.
diff --git a/docs/fields.md b/docs/fields.md
@@ -595,8 +595,8 @@ class MyModel(edgy.Model):
 * `through_registry` - Registry where the model callback is installed if `through` is a string or empty. Defaults to the field owner registry.
 * `through_tablename` - Custom tablename for `through`. E.g. when special characters are used in model names.
 * `embed_through` - When traversing, embed the through object in this attribute. Otherwise it is not accessable from the result.
-                      if an empty string was provided, the old behaviour is used to query from the through model as base (default).
-                      if False, the base is transformed to the target and source model (full proxying). You cannot select the through model via path traversal anymore (except from the through model itself).
+                      if an empty string was provided, the old behaviour is used to query from the through model as base.
+                      if False (the new default), the base is transformed to the target and source model (full proxying). You cannot select the through model via path traversal anymore (except from the through model itself).
                       If not an empty string, the same behaviour like with False applies except that you can select the through model fields via path traversal with the provided name.
 
 

diff --git a/docs/release-notes.md b/docs/release-notes.md
@@ -19,6 +19,7 @@ hide:
 - Add `no_copy` to models MetaInfo.
 - Add `ModelCollisionError` exception.
 - Add keyword only hook function `real_add_to_registry`. It can be used to customize the `add_to_registry` behaviour.
+- Add `__no_load_trigger_attrs__` to edgy base model to prevent some attrs from causing a deferred load.
 
 ### Changed
 
@@ -29,6 +30,8 @@ hide:
 - Instead of silent replacing models with the same `__name__` now an error is raised.
 - `skip_registry` has now also an allowed literal value: `"allow_search"`. It enables the search of the registry but doesn't register the model.
 - Move `testclient` to `testing` but keep a forward reference.
+- Change the default for ManyToMany `embed_through` from "" to `False` which affected traversing ManyToMany.
+- Better protect secrets from leaking. Prevent load when accessing a secret field or column.
 
 ### Fixed
 
@@ -41,11 +44,19 @@ hide:
 - Fix transaction method to work on instance and class.
 - Fix missing file conversion in File. Move from ContentFile.
 - Fix mypy crashing after the cache was build (cause ChoiceField annotation).
+- Fix handling unknown fields via the generic_field.
+- Sanify default for embed_through which affected traversing ManyToMany.
+  It defaulted to the backward compatible "" which requires the user to traverse the m2m model first.
+- Prevent fully initialized models from triggering a deferred load.
+- Prevent accessing excluded secrets from triggering a deferred load.
 
 ### BREAKING
 
 - Instead of silent replacing models with the same `__name__` now an error is raised.
 - The return value of `add_to_registry` changed. If you customize the function you need to return now the actual model added to the registry.
+- The default for ManyToMany `embed_through` changed from "" to `False` which affected traversing ManyToMany. For keeping the old behaviour pass:
+  `embed_through=""`.
+- Accessing field values excluded by exclude_secrets doesn't trigger an implicit load anymore.
 
 ## 0.24.2
 

diff --git a/edgy/core/db/fields/many_to_many.py b/edgy/core/db/fields/many_to_many.py
@@ -32,7 +32,7 @@ def __init__(
         from_foreign_key: str = "",
         through: Union[str, type["BaseModelType"]] = "",
         through_tablename: str = "",
-        embed_through: Union[str, Literal[False]] = "",
+        embed_through: Union[str, Literal[False]] = False,
         **kwargs: Any,
     ) -> None:
         super().__init__(**kwargs)

diff --git a/edgy/core/db/models/base.py b/edgy/core/db/models/base.py
@@ -56,6 +56,7 @@ class EdgyBaseModel(BaseModel, BaseModelType):
     __reflected__: ClassVar[bool] = False
     __show_pk__: ClassVar[bool] = False
     __using_schema__: Union[str, None, Any] = Undefined
+    __no_load_trigger_attrs__: ClassVar[set[str]] = _empty
     # private attribute
     database: ClassVar[Database] = None
     _loaded_or_deleted: bool = False
@@ -66,6 +67,8 @@ def __init__(
         self.__show_pk__ = __show_pk__
         # always set them in __dict__ to prevent __getattr__ loop
         self._loaded_or_deleted = False
+        # per instance it is a mutable set with pk added
+        self.__no_load_trigger_attrs__ = {*type(self).__no_load_trigger_attrs__}
         # inject in relation fields anonymous ModelRef (without a Field)
         for arg in args:
             if isinstance(arg, ModelRef):
@@ -95,6 +98,8 @@ def __init__(
 
         kwargs = self.transform_input(kwargs, phase=__phase__, instance=self)
         super().__init__(**kwargs)
+        # per instance it is a mutable set
+        self.__no_load_trigger_attrs__ = set(type(self).__no_load_trigger_attrs__)
         # move to dict (e.g. reflected or subclasses which allow extra attributes)
         if self.__pydantic_extra__ is not None:
             # default was triggered
@@ -289,12 +294,12 @@ def model_dump(self, show_pk: Union[bool, None] = None, **kwargs: Any) -> dict[s
                     include=sub_include, exclude=sub_exclude, mode=mode, **kwargs
                 )
             else:
-                assert (
-                    sub_include is None
-                ), "sub include filters for CompositeField specified, but no Pydantic model is set"
-                assert (
-                    sub_exclude is None
-                ), "sub exclude filters for CompositeField specified, but no Pydantic model is set"
+                assert sub_include is None, (
+                    "sub include filters for CompositeField specified, but no Pydantic model is set"
+                )
+                assert sub_exclude is None, (
+                    "sub exclude filters for CompositeField specified, but no Pydantic model is set"
+                )
                 if mode == "json" and not getattr(field, "unsafe_json_serialization", False):
                     # skip field if it isn't a BaseModel and the mode is json and unsafe_json_serialization is not set
                     # currently unsafe_json_serialization exists only on CompositeFields
@@ -501,8 +506,12 @@ def __getattr__(self, name: str) -> Any:
         if (
             name not in self.__dict__
             and behavior != "passdown"
+            # is already loaded
             and not self.__dict__.get("_loaded_or_deleted", False)
+            # only load when it is a field except for reflected
             and (field is not None or self.__reflected__)
+            # exclude attr names from triggering load
+            and name not in self.__dict__.get("__no_load_trigger_attrs__", _empty)
             and name not in self.identifying_db_fields
             and self.can_load
         ):

diff --git a/edgy/core/db/models/mixins/row.py b/edgy/core/db/models/mixins/row.py
@@ -167,11 +167,14 @@ async def from_sqla_row(
                 database=proxy_database,
             )
             proxy_model.identifying_db_fields = foreign_key.related_columns
+            if exclude_secrets:
+                proxy_model.__no_load_trigger_attrs__.update(model_related.meta.secret_fields)
 
             item[related] = proxy_model
 
         # Check for the only_fields
         # Pull out the regular column values.
+        class_columns = cls.table.columns
         for column in table_columns:
             if (
                 only_fields
@@ -181,7 +184,8 @@ async def from_sqla_row(
                 continue
             if column.key in secret_columns:
                 continue
-            if column.key not in cls.meta.columns_to_field:
+            if column.key not in class_columns:
+                # for supporting reflected we cannot use columns_to_field
                 continue
             # set if not of an foreign key with one column
             if column.key in item:
@@ -197,6 +201,12 @@ async def from_sqla_row(
             if exclude_secrets or is_defer_fields or only_fields
             else cls(**item, __phase__="init_db")
         )
+        # mark a model as completely loaded when no deferred is active
+        if not is_defer_fields and not only_fields:
+            model._loaded_or_deleted = True
+        # hard exclude secrets from triggering load
+        if exclude_secrets:
+            model.__no_load_trigger_attrs__.update(cls.meta.secret_fields)
         # Apply the schema to the model
         model = apply_instance_extras(
             model,

diff --git a/edgy/core/db/querysets/base.py b/edgy/core/db/querysets/base.py
@@ -592,9 +592,9 @@ async def wrapper(
 
                 clauses.append(wrapper)
             else:
-                assert not isinstance(
-                    value, BaseModelType
-                ), f"should be parsed in clean: {key}: {value}"
+                assert not isinstance(value, BaseModelType), (
+                    f"should be parsed in clean: {key}: {value}"
+                )
 
                 async def wrapper(
                     queryset: "QuerySet",
@@ -604,12 +604,14 @@ async def wrapper(
                     _value: Any = value,
                     _op: Optional[str] = op,
                     _prefix: str = related_str,
+                    # generic field has no field name
+                    _field_name: str = field_name,
                 ) -> Any:
                     _value = await clauses_mod.parse_clause_arg(
                         _value, queryset, tables_and_models
                     )
                     table = tables_and_models[_prefix][0]
-                    return _field.operator_to_clause(_field.name, _op, table, _value)
+                    return _field.operator_to_clause(_field_name, _op, table, _value)
 
                 wrapper._edgy_force_callable_queryset_filter = True
                 clauses.append(wrapper)
@@ -893,9 +895,9 @@ def _filter_or_exclude(
                 else:
                     converted_clauses.extend(extracted_clauses)
             elif isinstance(raw_clause, QuerySet):
-                assert (
-                    raw_clause.model_class is queryset.model_class
-                ), f"QuerySet arg has wrong model_class {raw_clause.model_class}"
+                assert raw_clause.model_class is queryset.model_class, (
+                    f"QuerySet arg has wrong model_class {raw_clause.model_class}"
+                )
                 converted_clauses.append(raw_clause.build_where_clause)
                 if not queryset._select_related.issuperset(raw_clause._select_related):
                     queryset._select_related.update(raw_clause._select_related)

diff --git a/edgy/core/db/relationships/relation.py b/edgy/core/db/relationships/relation.py
@@ -58,7 +58,7 @@ def get_queryset(self) -> "QuerySet":
         queryset = queryset.filter(**{self.from_foreign_key: query})
         # now set embed_parent
         queryset.embed_parent = (self.to_foreign_key, self.embed_through or "")
-        if self.embed_through:
+        if self.embed_through != "":
             queryset.embed_parent_filters = queryset.embed_parent
         return queryset
 

diff --git a/mkdocs.yml b/mkdocs.yml
@@ -119,6 +119,7 @@ nav:
       - "testing/index.md"
       - Test Client: "testing/test-client.md"
       - Model Factory: "testing/model-factory.md"
+  - Debugging: "debugging.md"
   - API Reference:
       - "references/index.md"
       - Model: "references/models.md"

diff --git a/tests/exclude_secrets/test_exclude.py b/tests/exclude_secrets/test_exclude.py
@@ -43,4 +43,6 @@ async def test_exclude_secrets_query():
         profile=profile, email="user@dev.com", password="dasrq3213", name="edgy"
     )
 
-    await User.query.exclude_secrets().get()
+    user = await User.query.exclude_secrets().get()
+    assert not hasattr(user, "password")
+    assert not hasattr(user.profile, "is_enabled")
diff --git a/tests/foreign_keys/m2m_string_old/test_many_to_many.py b/tests/foreign_keys/m2m_string_old/test_many_to_many.py
@@ -30,16 +30,16 @@ class Meta:
 class Album(edgy.StrictModel):
     id = edgy.IntegerField(primary_key=True, autoincrement=True)
     name = edgy.CharField(max_length=100)
-    tracks = edgy.ManyToManyField("Track")
+    tracks = edgy.ManyToManyField("Track", embed_through="")
 
     class Meta:
         registry = models
 
 
 class Studio(edgy.StrictModel):
     name = edgy.CharField(max_length=255)
-    users = edgy.ManyToManyField("User")
-    albums = edgy.ManyToManyField("Album")
+    users = edgy.ManyToManyField("User", embed_through="")
+    albums = edgy.ManyToManyField("Album", embed_through="")
 
     class Meta:
         registry = models

diff --git a/tests/foreign_keys/m2m_string_old/test_many_to_many_field.py b/tests/foreign_keys/m2m_string_old/test_many_to_many_field.py
@@ -30,16 +30,16 @@ class Meta:
 class Album(edgy.StrictModel):
     id = edgy.IntegerField(primary_key=True, autoincrement=True)
     name = edgy.CharField(max_length=100)
-    tracks = edgy.ManyToMany("Track")
+    tracks = edgy.ManyToMany("Track", embed_through="")
 
     class Meta:
         registry = models
 
 
 class Studio(edgy.StrictModel):
     name = edgy.CharField(max_length=255)
-    users = edgy.ManyToMany("User")
-    albums = edgy.ManyToMany("Album")
+    users = edgy.ManyToMany("User", embed_through="")
+    albums = edgy.ManyToMany("Album", embed_through="")
 
     class Meta:
         registry = models

diff --git a/tests/foreign_keys/m2m_string_old/test_many_to_many_field_related_name.py b/tests/foreign_keys/m2m_string_old/test_many_to_many_field_related_name.py
@@ -29,16 +29,16 @@ class Meta:
 class Album(edgy.StrictModel):
     id = edgy.IntegerField(primary_key=True, autoincrement=True)
     name = edgy.CharField(max_length=100)
-    tracks = edgy.ManyToManyField("Track", related_name="album_tracks")
+    tracks = edgy.ManyToManyField("Track", related_name="album_tracks", embed_through="")
 
     class Meta:
         registry = models
 
 
 class Studio(edgy.StrictModel):
     name = edgy.CharField(max_length=255)
-    users = edgy.ManyToManyField("User", related_name="studio_users")
-    albums = edgy.ManyToManyField("Album", related_name="studio_albums")
+    users = edgy.ManyToManyField("User", related_name="studio_users", embed_through="")
+    albums = edgy.ManyToManyField("Album", related_name="studio_albums", embed_through="")
 
     class Meta:
         registry = models

diff --git a/tests/foreign_keys/m2m_string_old/test_many_to_many_no_related_name.py b/tests/foreign_keys/m2m_string_old/test_many_to_many_no_related_name.py
@@ -22,7 +22,7 @@ class Meta:
 class Album(edgy.StrictModel):
     id = edgy.IntegerField(primary_key=True, autoincrement=True)
     name = edgy.CharField(max_length=100)
-    tracks = edgy.ManyToMany("Track", related_name=False)
+    tracks = edgy.ManyToMany("Track", related_name=False, embed_through="")
 
     class Meta:
         registry = models

diff --git a/tests/foreign_keys/m2m_string_old/test_many_to_many_related_name.py b/tests/foreign_keys/m2m_string_old/test_many_to_many_related_name.py
@@ -29,16 +29,16 @@ class Meta:
 class Album(edgy.StrictModel):
     id = edgy.IntegerField(primary_key=True, autoincrement=True)
     name = edgy.CharField(max_length=100)
-    tracks = edgy.ManyToMany("Track", related_name="album_tracks")
+    tracks = edgy.ManyToMany("Track", related_name="album_tracks", embed_through="")
 
     class Meta:
         registry = models
 
 
 class Studio(edgy.StrictModel):
     name = edgy.CharField(max_length=255)
-    users = edgy.ManyToMany("User", related_name="studio_users")
-    albums = edgy.ManyToMany("Album", related_name="studio_albums")
+    users = edgy.ManyToMany("User", related_name="studio_users", embed_through="")
+    albums = edgy.ManyToMany("Album", related_name="studio_albums", embed_through="")
 
     class Meta:
         registry = models

diff --git a/tests/foreign_keys/test_many_to_many_field_old.py b/tests/foreign_keys/test_many_to_many_field_old.py
@@ -30,7 +30,7 @@ class Meta:
 class Album(edgy.StrictModel):
     id = edgy.IntegerField(primary_key=True, autoincrement=True)
     name = edgy.CharField(max_length=100)
-    tracks = edgy.ManyToMany(Track)
+    tracks = edgy.ManyToMany(Track, embed_through="")
 
     class Meta:
         registry = models
@@ -39,7 +39,7 @@ class Meta:
 class Studio(edgy.StrictModel):
     name = edgy.CharField(max_length=255)
     users = edgy.ManyToMany(User)
-    albums = edgy.ManyToMany(Album)
+    albums = edgy.ManyToMany(Album, embed_through="")
 
     class Meta:
         registry = models

diff --git a/tests/foreign_keys/test_many_to_many_field_related_name_old.py b/tests/foreign_keys/test_many_to_many_field_related_name_old.py
@@ -29,16 +29,16 @@ class Meta:
 class Album(edgy.StrictModel):
     id = edgy.IntegerField(primary_key=True, autoincrement=True)
     name = edgy.CharField(max_length=100)
-    tracks = edgy.ManyToManyField(Track, related_name="album_tracks")
+    tracks = edgy.ManyToManyField(Track, related_name="album_tracks", embed_through="")
 
     class Meta:
         registry = models
 
 
 class Studio(edgy.StrictModel):
     name = edgy.CharField(max_length=255)
-    users = edgy.ManyToManyField(User, related_name="studio_users")
-    albums = edgy.ManyToManyField(Album, related_name="studio_albums")
+    users = edgy.ManyToManyField(User, related_name="studio_users", embed_through="")
+    albums = edgy.ManyToManyField(Album, related_name="studio_albums", embed_through="")
 
     class Meta:
         registry = models