- Fixed regression regarding the declarative ``__declare_first__``

and ``__declare_last__`` accessors where these would no longer be
called on the superclass of the declarative base.
fixes #3383

7 files changed, 124 insertions, 13 deletions

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index 29e686391..319e58b78 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -16,6 +16,17 @@
         :start-line: 5
 
 .. changelog::
+    :version: 1.0.2
+
+    .. change::
+        :tags: bug, ext, declarative
+        :tickets: 3383
+
+        Fixed regression regarding the declarative ``__declare_first__``
+        and ``__declare_last__`` accessors where these would no longer be
+        called on the superclass of the declarative base.
+
+.. changelog::
     :version: 1.0.1
     :released: April 23, 2015
 
diff --git a/lib/sqlalchemy/__init__.py b/lib/sqlalchemy/__init__.py
index 3ef97ef90..69ab88772 100644
--- a/lib/sqlalchemy/__init__.py
+++ b/lib/sqlalchemy/__init__.py
@@ -120,7 +120,7 @@ from .schema import (
 from .inspection import inspect
 from .engine import create_engine, engine_from_config
 
-__version__ = '1.0.1'
+__version__ = '1.0.2'
 
 
 def __go(lcls):
diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py
index 7d4020b24..d5fc76ed1 100644
--- a/lib/sqlalchemy/ext/declarative/base.py
+++ b/lib/sqlalchemy/ext/declarative/base.py
@@ -50,7 +50,7 @@ def _resolve_for_abstract(cls):
         return cls
 
 
-def _get_immediate_cls_attr(cls, attrname):
+def _get_immediate_cls_attr(cls, attrname, strict=False):
     """return an attribute of the class that is either present directly
     on the class, e.g. not on a superclass, or is from a superclass but
     this superclass is a mixin, that is, not a descendant of
@@ -66,11 +66,12 @@ def _get_immediate_cls_attr(cls, attrname):
 
     for base in cls.__mro__:
         _is_declarative_inherits = hasattr(base, '_decl_class_registry')
-        if attrname in base.__dict__:
-            value = getattr(base, attrname)
-            if (base is cls or
-                    (base in cls.__bases__ and not _is_declarative_inherits)):
-                return value
+        if attrname in base.__dict__ and (
+            base is cls or
+            ((base in cls.__bases__ if strict else True)
+                and not _is_declarative_inherits)
+        ):
+            return getattr(base, attrname)
     else:
         return None
 
@@ -92,7 +93,7 @@ class _MapperConfig(object):
     @classmethod
     def setup_mapping(cls, cls_, classname, dict_):
         defer_map = _get_immediate_cls_attr(
-            cls_, '_sa_decl_prepare_nocascade') or \
+            cls_, '_sa_decl_prepare_nocascade', strict=True) or \
             hasattr(cls_, '_sa_decl_prepare')
 
         if defer_map:
@@ -158,7 +159,8 @@ class _MapperConfig(object):
         for base in cls.__mro__:
             class_mapped = base is not cls and \
                 _declared_mapping_info(base) is not None and \
-                not _get_immediate_cls_attr(base, '_sa_decl_prepare_nocascade')
+                not _get_immediate_cls_attr(
+                    base, '_sa_decl_prepare_nocascade', strict=True)
 
             if not class_mapped and base is not cls:
                 self._produce_column_copies(base)
@@ -412,7 +414,7 @@ class _MapperConfig(object):
                 continue
             if _declared_mapping_info(c) is not None and \
                     not _get_immediate_cls_attr(
-                        c, '_sa_decl_prepare_nocascade'):
+                        c, '_sa_decl_prepare_nocascade', strict=True):
                 self.inherits = c
                 break
         else:
diff --git a/lib/sqlalchemy/testing/__init__.py b/lib/sqlalchemy/testing/__init__.py
index bf83e9673..adfbe85e3 100644
--- a/lib/sqlalchemy/testing/__init__.py
+++ b/lib/sqlalchemy/testing/__init__.py
@@ -24,7 +24,8 @@ from .assertions import emits_warning, emits_warning_on, uses_deprecated, \
     AssertsExecutionResults, expect_deprecated, expect_warnings
 
 from .util import run_as_contextmanager, rowset, fail, \
-    provide_metadata, adict, force_drop_names
+    provide_metadata, adict, force_drop_names, \
+    teardown_events
 
 crashes = skip
 
diff --git a/lib/sqlalchemy/testing/util.py b/lib/sqlalchemy/testing/util.py
index 1842e58a5..e9437948a 100644
--- a/lib/sqlalchemy/testing/util.py
+++ b/lib/sqlalchemy/testing/util.py
@@ -267,3 +267,14 @@ def drop_all_tables(engine, inspector, schema=None, include_names=None):
                         ForeignKeyConstraint(
                             [tb.c.x], [tb.c.y], name=fkc)
                     ))
+
+
+def teardown_events(event_cls):
+    @decorator
+    def decorate(fn, *arg, **kw):
+        try:
+            return fn(*arg, **kw)
+        finally:
+            event_cls._clear()
+    return decorate
+
diff --git a/test/ext/declarative/test_inheritance.py b/test/ext/declarative/test_inheritance.py
index 2ecee99fd..3e6980190 100644
--- a/test/ext/declarative/test_inheritance.py
+++ b/test/ext/declarative/test_inheritance.py
@@ -1451,4 +1451,5 @@ class ConcreteExtensionConfigTest(
             "actual_documents.send_method AS send_method, "
             "actual_documents.id AS id, 'actual' AS type "
             "FROM actual_documents) AS pjoin"
-        )
\ No newline at end of file
+        )
+
diff --git a/test/ext/declarative/test_mixin.py b/test/ext/declarative/test_mixin.py
index 45b881671..b9e40421c 100644
--- a/test/ext/declarative/test_mixin.py
+++ b/test/ext/declarative/test_mixin.py
@@ -9,7 +9,8 @@ from sqlalchemy.orm import relationship, create_session, class_mapper, \
     configure_mappers, clear_mappers, \
     deferred, column_property, Session, base as orm_base
 from sqlalchemy.util import classproperty
-from sqlalchemy.ext.declarative import declared_attr
+from sqlalchemy.ext.declarative import declared_attr, declarative_base
+from sqlalchemy.orm import events as orm_events
 from sqlalchemy.testing import fixtures, mock
 from sqlalchemy.testing.util import gc_collect
 
@@ -438,6 +439,90 @@ class DeclarativeMixinTest(DeclarativeTestBase):
 
         eq_(MyModel.__table__.kwargs, {'mysql_engine': 'InnoDB'})
 
+    @testing.teardown_events(orm_events.MapperEvents)
+    def test_declare_first_mixin(self):
+        canary = mock.Mock()
+
+        class MyMixin(object):
+            @classmethod
+            def __declare_first__(cls):
+                canary.declare_first__(cls)
+
+            @classmethod
+            def __declare_last__(cls):
+                canary.declare_last__(cls)
+
+        class MyModel(Base, MyMixin):
+            __tablename__ = 'test'
+            id = Column(Integer, primary_key=True)
+
+        configure_mappers()
+
+        eq_(
+            canary.mock_calls,
+            [
+                mock.call.declare_first__(MyModel),
+                mock.call.declare_last__(MyModel),
+            ]
+        )
+
+    @testing.teardown_events(orm_events.MapperEvents)
+    def test_declare_first_base(self):
+        canary = mock.Mock()
+
+        class MyMixin(object):
+            @classmethod
+            def __declare_first__(cls):
+                canary.declare_first__(cls)
+
+            @classmethod
+            def __declare_last__(cls):
+                canary.declare_last__(cls)
+
+        class Base(MyMixin):
+            pass
+        Base = declarative_base(cls=Base)
+
+        class MyModel(Base):
+            __tablename__ = 'test'
+            id = Column(Integer, primary_key=True)
+
+        configure_mappers()
+
+        eq_(
+            canary.mock_calls,
+            [
+                mock.call.declare_first__(MyModel),
+                mock.call.declare_last__(MyModel),
+            ]
+        )
+
+    @testing.teardown_events(orm_events.MapperEvents)
+    def test_declare_first_direct(self):
+        canary = mock.Mock()
+
+        class MyOtherModel(Base):
+            __tablename__ = 'test2'
+            id = Column(Integer, primary_key=True)
+
+            @classmethod
+            def __declare_first__(cls):
+                canary.declare_first__(cls)
+
+            @classmethod
+            def __declare_last__(cls):
+                canary.declare_last__(cls)
+
+        configure_mappers()
+
+        eq_(
+            canary.mock_calls,
+            [
+                mock.call.declare_first__(MyOtherModel),
+                mock.call.declare_last__(MyOtherModel)
+            ]
+        )
+
     def test_mapper_args_declared_attr(self):
 
         class ComputedMapperArgs: