From: Mike Bayer Date: Tue, 26 Mar 2019 16:58:42 +0000 (-0400) Subject: Refine ambiguous access for unknown attribute types X-Git-Tag: rel_1_3_2~2^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=51f81d00dc6103c1dea939513a3437a5ab433e75;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Refine ambiguous access for unknown attribute types 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. Fixes: #4574 Change-Id: I787a22806b5530c146ae6ee66b588e5b191ae689 --- diff --git a/doc/build/changelog/unreleased_13/4574.rst b/doc/build/changelog/unreleased_13/4574.rst new file mode 100644 index 0000000000..675e2567bc --- /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 ea3b8fadeb..b8672e7398 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 6f701416cc..683a84fcdf 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, )