From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 6 Jun 2017 21:13:54 +0000 (-0400)
Subject: Warn when declared_attr.cascading detected on mapped class
X-Git-Tag: rel_1_2_0b1~32
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4e1f5377d1c5443016cc6c5ae86b8053ead41647;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Warn when declared_attr.cascading detected on mapped class

A warning is emitted if the :attr:`.declared_attr.cascading` modifier
is used with a declarative attribute that is itself declared on
a class that is to be mapped, as opposed to a declarative mixin
class or ``__abstract__`` class.  The :attr:`.declared_attr.cascading`
modifier currently only applies to mixin/abstract classes.

Also add a test for @declared_attr.cascading when used on an attribute
on __abstract__.

Change-Id: Ib1b9dbe373e8be1cf24eadfed224a8988b3cd95d
Fixes: #3847
---

diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst
index 5ea6b513ed..219742a316 100644
--- a/doc/build/changelog/changelog_12.rst
+++ b/doc/build/changelog/changelog_12.rst
@@ -27,6 +27,16 @@
 
             :ref:`change_3853`
 
+    .. change:: 3847
+        :tags: bug, declarative
+        :tickets: 3847
+
+        A warning is emitted if the :attr:`.declared_attr.cascading` modifier
+        is used with a declarative attribute that is itself declared on
+        a class that is to be mapped, as opposed to a declarative mixin
+        class or ``__abstract__`` class.  The :attr:`.declared_attr.cascading`
+        modifier currently only applies to mixin/abstract classes.
+
     .. change:: 4003
         :tags: feature, oracle
 
diff --git a/lib/sqlalchemy/ext/declarative/api.py b/lib/sqlalchemy/ext/declarative/api.py
index 7c503d471b..ee13a90f3c 100644
--- a/lib/sqlalchemy/ext/declarative/api.py
+++ b/lib/sqlalchemy/ext/declarative/api.py
@@ -199,6 +199,12 @@ class declared_attr(interfaces._MappedAttribute, property):
         or MapperProperty-based declared attribute should be configured
         distinctly per mapped subclass, within a mapped-inheritance scenario.
 
+        .. note::
+
+            The :attr:`.declared_attr.cascading` modifier **only** applies
+            to the use of :class:`.declared_attr` on declarative mixin classes
+            and ``__abstract__`` classes.
+
         Below, both MyClass as well as MySubClass will have a distinct
         ``id`` Column object established::
 
diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py
index d9433e6928..3a2ac66aee 100644
--- a/lib/sqlalchemy/ext/declarative/base.py
+++ b/lib/sqlalchemy/ext/declarative/base.py
@@ -283,6 +283,13 @@ class _MapperConfig(object):
 
             value = dict_[k]
             if isinstance(value, declarative_props):
+                if isinstance(value, declared_attr) and value._cascading:
+                    util.warn(
+                        "Use of @declared_attr.cascading only applies to "
+                        "Declarative 'mixin' and 'abstract' classes.  "
+                        "Currently, this flag is ignored on mapped class "
+                        "%s" % self.cls)
+
                 value = getattr(cls, k)
 
             elif isinstance(value, QueryableAttribute) and \
diff --git a/test/ext/declarative/test_basic.py b/test/ext/declarative/test_basic.py
index d91a88276a..16fef5128d 100644
--- a/test/ext/declarative/test_basic.py
+++ b/test/ext/declarative/test_basic.py
@@ -1,6 +1,6 @@
 
 from sqlalchemy.testing import eq_, assert_raises, \
-    assert_raises_message
+    assert_raises_message, expect_warnings
 from sqlalchemy.ext import declarative as decl
 from sqlalchemy import exc
 import sqlalchemy as sa
@@ -1230,6 +1230,21 @@ class DeclarativeTest(DeclarativeTestBase):
         assert 'somecol' in MyBase.__table__.c
         assert 'somecol' not in MyClass.__table__.c
 
+    def test_decl_cascading_warns_non_mixin(self):
+        with expect_warnings(
+                "Use of @declared_attr.cascading only applies to "
+                "Declarative 'mixin' and 'abstract' classes.  "
+                "Currently, this flag is ignored on mapped class "
+                "<class '.*.MyBase'>"
+        ):
+            class MyBase(Base):
+                __tablename__ = 'foo'
+                id = Column(Integer, primary_key=True)
+
+                @declared_attr.cascading
+                def somecol(cls):
+                    return Column(Integer)
+
     def test_column(self):
 
         class User(Base, fixtures.ComparableEntity):
diff --git a/test/ext/declarative/test_mixin.py b/test/ext/declarative/test_mixin.py
index 3272e0753f..be169bcda1 100644
--- a/test/ext/declarative/test_mixin.py
+++ b/test/ext/declarative/test_mixin.py
@@ -1593,7 +1593,7 @@ class DeclaredAttrTest(DeclarativeTestBase, testing.AssertsCompiledSQL):
 
         eq_(counter.mock_calls, [mock.call(A)])
 
-    def test_property_cascade(self):
+    def test_property_cascade_mixin(self):
         counter = mock.Mock()
 
         class Mixin(object):
@@ -1613,6 +1613,28 @@ class DeclaredAttrTest(DeclarativeTestBase, testing.AssertsCompiledSQL):
 
         eq_(counter.mock_calls, [mock.call(A), mock.call(B)])
 
+    def test_property_cascade_abstract(self):
+        counter = mock.Mock()
+
+        class Abs(Base):
+            __abstract__ = True
+
+            @declared_attr.cascading
+            def my_prop(cls):
+                counter(cls)
+                return column_property(cls.x + 2)
+
+        class A(Abs):
+            __tablename__ = 'a'
+
+            id = Column(Integer, primary_key=True)
+            x = Column(Integer)
+
+        class B(A):
+            pass
+
+        eq_(counter.mock_calls, [mock.call(A), mock.call(B)])
+
     def test_col_prop_attrs_associated_w_class_for_mapper_args(self):
         from sqlalchemy import Column
         import collections