Merge "Refine ambiguous access for unknown attribute types"

3 files changed, 109 insertions, 6 deletions

diff --git a/doc/build/changelog/unreleased_13/4574.rst b/doc/build/changelog/unreleased_13/4574.rst
new file mode 100644
index 000000000..675e2567b
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4574.rst
@@ -0,0 +1,14 @@
+.. change::
+   :tags: bug, orm, ext
+   :tickets: 4574, 4573
+
+   Restored instance-level support for plain Python descriptors, e.g.
+   ``@property`` objects, in conjunction with association proxies, in that if
+   the proxied object is not within ORM scope at all, it gets classified as
+   "ambiguous" but is proxed directly.  For class level access, a basic class
+   level``__get__()`` now returns the
+   :class:`.AmbiguousAssociationProxyInstance` directly, rather than raising
+   its exception, which is the closest approximation to the previous behavior
+   that returned the :class:`.AssociationProxy` itself that's possible.  Also
+   improved the stringification of these objects to be more descriptive of
+   current state.
diff --git a/lib/sqlalchemy/ext/associationproxy.py b/lib/sqlalchemy/ext/associationproxy.py
index ea3b8fade..b8672e739 100644
--- a/lib/sqlalchemy/ext/associationproxy.py
+++ b/lib/sqlalchemy/ext/associationproxy.py
@@ -295,6 +295,12 @@ class AssociationProxy(interfaces.InspectionAttrInfo):
 
         return getter, setter
 
+    def __repr__(self):
+        return "AssociationProxy(%r, %r)" % (
+            self.target_collection,
+            self.value_attr,
+        )
+
 
 class AssociationProxyInstance(object):
     """A per-class object that serves class- and object-specific results.
@@ -385,7 +391,11 @@ class AssociationProxyInstance(object):
             )
 
         attr = getattr(target_class, value_attr)
-        if attr._is_internal_proxy and not hasattr(attr, "impl"):
+        if (
+            not hasattr(attr, "_is_internal_proxy")
+            or attr._is_internal_proxy
+            and not hasattr(attr, "impl")
+        ):
             return AmbiguousAssociationProxyInstance(
                 parent, owning_class, target_class, value_attr
             )
@@ -724,6 +734,9 @@ class AssociationProxyInstance(object):
             criterion=criterion, is_has=True, **kwargs
         )
 
+    def __repr__(self):
+        return "%s(%r)" % (self.__class__.__name__, self.parent)
+
 
 class AmbiguousAssociationProxyInstance(AssociationProxyInstance):
     """an :class:`.AssociationProxyInstance` where we cannot determine
@@ -748,10 +761,16 @@ class AmbiguousAssociationProxyInstance(AssociationProxyInstance):
 
     def get(self, obj):
         if obj is None:
-            self._ambiguous()
+            return self
         else:
             return super(AmbiguousAssociationProxyInstance, self).get(obj)
 
+    def __eq__(self, obj):
+        self._ambiguous()
+
+    def __ne__(self, obj):
+        self._ambiguous()
+
     def any(self, criterion=None, **kwargs):
         self._ambiguous()
 
diff --git a/test/ext/test_associationproxy.py b/test/ext/test_associationproxy.py
index 6f701416c..683a84fcd 100644
--- a/test/ext/test_associationproxy.py
+++ b/test/ext/test_associationproxy.py
@@ -3141,7 +3141,7 @@ class ProxyOfSynonymTest(AssertsCompiledSQL, fixtures.DeclarativeMappedTest):
         )
 
 
-class ProxyAttrTest(fixtures.DeclarativeMappedTest):
+class ProxyHybridTest(fixtures.DeclarativeMappedTest):
     @classmethod
     def setup_classes(cls):
         from sqlalchemy.ext.hybrid import hybrid_property
@@ -3225,6 +3225,15 @@ class ProxyAttrTest(fixtures.DeclarativeMappedTest):
             "a.id = b.aid AND b.data = :data_1)",
         )
 
+    def test_get_classlevel_ambiguous(self):
+        A, B = self.classes("A", "B")
+
+        eq_(
+            str(A.b_data),
+            "AmbiguousAssociationProxyInstance"
+            "(AssociationProxy('bs', 'value'))",
+        )
+
     def test_expr_ambiguous(self):
         A, B = self.classes("A", "B")
 
@@ -3232,9 +3241,70 @@ class ProxyAttrTest(fixtures.DeclarativeMappedTest):
             AttributeError,
             "Association proxy A.bs refers to an attribute "
             "'value' that is not directly mapped",
-            getattr,
-            A,
-            "b_data",
+            A.b_data.any,
+        )
+
+
+class ProxyPlainPropertyTest(fixtures.DeclarativeMappedTest):
+    @classmethod
+    def setup_classes(cls):
+
+        Base = cls.DeclarativeBasic
+
+        class A(Base):
+            __tablename__ = "a"
+
+            id = Column(Integer, primary_key=True)
+            bs = relationship("B")
+
+            b_data = association_proxy("bs", "value")
+
+        class B(Base):
+            __tablename__ = "b"
+
+            id = Column(Integer, primary_key=True)
+            aid = Column(ForeignKey("a.id"))
+            data = Column(String(50))
+
+            @property
+            def value(self):
+                return self.data
+
+            @value.setter
+            def value(self, value):
+                self.data = value
+
+    def test_get_ambiguous(self):
+        A, B = self.classes("A", "B")
+
+        a1 = A(bs=[B(data="b1")])
+        eq_(a1.b_data[0], "b1")
+
+    def test_set_ambiguous(self):
+        A, B = self.classes("A", "B")
+
+        a1 = A(bs=[B()])
+
+        a1.b_data[0] = "b1"
+        eq_(a1.b_data[0], "b1")
+
+    def test_get_classlevel_ambiguous(self):
+        A, B = self.classes("A", "B")
+
+        eq_(
+            str(A.b_data),
+            "AmbiguousAssociationProxyInstance"
+            "(AssociationProxy('bs', 'value'))",
+        )
+
+    def test_expr_ambiguous(self):
+        A, B = self.classes("A", "B")
+
+        assert_raises_message(
+            AttributeError,
+            "Association proxy A.bs refers to an attribute "
+            "'value' that is not directly mapped",
+            lambda: A.b_data == 5,
         )