From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 19 Apr 2021 00:30:14 +0000 (-0400)
Subject: Bypass declared_attr w/ a custom wrapper for lambda criteria
X-Git-Tag: rel_1_4_10~9^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=fa817cc0ea21a4b28b7a076eab0b42010279fbc9;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Bypass declared_attr w/ a custom wrapper for lambda criteria

Fixed bug in new :func:`_orm.with_loader_criteria` feature where using a
mixin class with :func:`_orm.declared_attr` on an attribute that were
accessed inside the custom lambda would emit a warning regarding using an
unmapped declared attr, when the lambda callable were first initialized.
This warning is now prevented using special instrumentation for this
lambda initialization step.

Fixes: #6320
Change-Id: I18ce0c662131f2e683b84caa38c01b2182eb210b
---

diff --git a/doc/build/changelog/unreleased_14/6320.rst b/doc/build/changelog/unreleased_14/6320.rst
new file mode 100644
index 0000000000..5a42a1985b
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6320.rst
@@ -0,0 +1,11 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 6320
+
+    Fixed bug in new :func:`_orm.with_loader_criteria` feature where using a
+    mixin class with :func:`_orm.declared_attr` on an attribute that were
+    accessed inside the custom lambda would emit a warning regarding using an
+    unmapped declared attr, when the lambda callable were first initialized.
+    This warning is now prevented using special instrumentation for this
+    lambda initialization step.
+
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index a30f085aa7..65fdcf9fc7 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -874,6 +874,32 @@ class AliasedInsp(
             return "aliased(%s)" % (self._target.__name__,)
 
 
+class _WrapUserEntity(object):
+    """A wrapper used within the loader_criteria lambda caller so that
+    we can bypass declared_attr descriptors on unmapped mixins, which
+    normally emit a warning for such use.
+
+    might also be useful for other per-lambda instrumentations should
+    the need arise.
+
+    """
+
+    def __init__(self, subject):
+        self.subject = subject
+
+    @util.preload_module("sqlalchemy.orm.decl_api")
+    def __getattribute__(self, name):
+        decl_api = util.preloaded.orm.decl_api
+
+        subject = object.__getattribute__(self, "subject")
+        if name in subject.__dict__ and isinstance(
+            subject.__dict__[name], decl_api.declared_attr
+        ):
+            return subject.__dict__[name].fget(subject)
+        else:
+            return getattr(subject, name)
+
+
 class LoaderCriteriaOption(CriteriaOption):
     """Add additional WHERE criteria to the load for all occurrences of
     a particular entity.
@@ -1033,9 +1059,11 @@ class LoaderCriteriaOption(CriteriaOption):
                 where_criteria,
                 roles.WhereHavingRole,
                 lambda_args=(
-                    self.root_entity
-                    if self.root_entity is not None
-                    else self.entity.entity,
+                    _WrapUserEntity(
+                        self.root_entity
+                        if self.root_entity is not None
+                        else self.entity.entity,
+                    ),
                 ),
                 opts=lambdas.LambdaOptions(
                     track_closure_variables=track_closure_variables
diff --git a/test/orm/test_relationship_criteria.py b/test/orm/test_relationship_criteria.py
index 04afe34779..df7c3bc0d9 100644
--- a/test/orm/test_relationship_criteria.py
+++ b/test/orm/test_relationship_criteria.py
@@ -22,6 +22,7 @@ from sqlalchemy.orm import selectinload
 from sqlalchemy.orm import Session
 from sqlalchemy.orm import subqueryload
 from sqlalchemy.orm import with_loader_criteria
+from sqlalchemy.orm.decl_api import declared_attr
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing.assertsql import CompiledSQL
 from test.orm import _fixtures
@@ -85,6 +86,24 @@ class _Fixtures(_fixtures.FixtureTest):
         )
         return HasFoob, UserWFoob
 
+    @testing.fixture
+    def declattr_mixin_fixture(self):
+        users = self.tables.users
+
+        class HasFoob(object):
+            @declared_attr
+            def name(cls):
+                return Column(String)
+
+        class UserWFoob(HasFoob, self.Comparable):
+            pass
+
+        mapper(
+            UserWFoob,
+            users,
+        )
+        return HasFoob, UserWFoob
+
     @testing.fixture
     def multi_mixin_fixture(self):
         orders, items = self.tables.orders, self.tables.items
@@ -649,6 +668,28 @@ class LoaderCriteriaTest(_Fixtures, testing.AssertsCompiledSQL):
                     [Order(description="order 3")],
                 )
 
+    def test_declared_attr_no_warning(self, declattr_mixin_fixture):
+        HasFoob, UserWFoob = declattr_mixin_fixture
+
+        statement = select(UserWFoob).filter(UserWFoob.id < 10)
+
+        def go(value):
+            return statement.options(
+                with_loader_criteria(
+                    HasFoob,
+                    lambda cls: cls.name == value,
+                    include_aliases=True,
+                )
+            )
+
+        s = Session(testing.db, future=True)
+
+        for i in range(10):
+            name = random.choice(["ed", "fred", "jack"])
+            stmt = go(name)
+
+            eq_(s.execute(stmt).scalars().all(), [UserWFoob(name=name)])
+
     def test_caching_and_binds_lambda_more_mixins(self, multi_mixin_fixture):
         # By including non-mapped mixin HasBat in the middle of the
         # hierarchy, we test issue #5766