- Fixed bug where :class:`.AbstractConcreteBase` would fail to be

fully usable within declarative relationship configuration, as its
string classname would not be available in the registry of classnames
at mapper configuration time.   The class now explicitly adds itself
to the class regsitry, and additionally both :class:`.AbstractConcreteBase`
as well as :class:`.ConcreteBase` set themselves up *before* mappers
are configured within the :func:`.configure_mappers` setup, using
the new :meth:`.MapperEvents.before_configured` event. [ticket:2950]

- Added new :meth:`.MapperEvents.before_configured` event which allows
an event at the start of :func:`.configure_mappers`, as well
as ``__declare_first__()`` hook within declarative to complement
``__declare_last__()``.

- modified how after_configured is invoked; we just make a dispatch()
not actually connected to any mapper.  this makes it easier
to also invoke before_configured correctly.

- improved the ComparableEntity fixture to handle collections that are sets.

8 files changed, 168 insertions, 10 deletions

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index 312d24bec..b57ebdc92 100644
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -15,6 +15,27 @@
     :version: 0.9.3
 
     .. change::
+        :tags: bug, orm, declarative
+        :tickets: 2950
+
+        Fixed bug where :class:`.AbstractConcreteBase` would fail to be
+        fully usable within declarative relationship configuration, as its
+        string classname would not be available in the registry of classnames
+        at mapper configuration time.   The class now explicitly adds itself
+        to the class regsitry, and additionally both :class:`.AbstractConcreteBase`
+        as well as :class:`.ConcreteBase` set themselves up *before* mappers
+        are configured within the :func:`.configure_mappers` setup, using
+        the new :meth:`.MapperEvents.before_configured` event.
+
+    .. change::
+        :tags: feature, orm
+
+        Added new :meth:`.MapperEvents.before_configured` event which allows
+        an event at the start of :func:`.configure_mappers`, as well
+        as ``__declare_first__()`` hook within declarative to complement
+        ``__declare_last__()``.
+
+    .. change::
         :tags: bug, mysql, cymysql
         :tickets: 2934
         :pullreq: github:69
diff --git a/lib/sqlalchemy/ext/declarative/__init__.py b/lib/sqlalchemy/ext/declarative/__init__.py
index e878e362d..ebd613093 100644
--- a/lib/sqlalchemy/ext/declarative/__init__.py
+++ b/lib/sqlalchemy/ext/declarative/__init__.py
@@ -656,7 +656,7 @@ Using the Concrete Helpers
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Helper classes provides a simpler pattern for concrete inheritance.
-With these objects, the ``__declare_last__`` helper is used to configure the
+With these objects, the ``__declare_first__`` helper is used to configure the
 "polymorphic" loader for the mapper after all subclasses have been declared.
 
 .. versionadded:: 0.7.3
@@ -702,6 +702,26 @@ Either ``Employee`` base can be used in the normal fashion::
                         'concrete':True}
 
 
+The :class:`.AbstractConcreteBase` class is itself mapped, and can be
+used as a target of relationships::
+
+    class Company(Base):
+        __tablename__ = 'company'
+
+        id = Column(Integer, primary_key=True)
+        employees = relationship("Employee",
+                        primaryjoin="Company.id == Employee.company_id")
+
+
+.. versionchanged:: 0.9.3 Support for use of :class:`.AbstractConcreteBase`
+   as the target of a :func:`.relationship` has been improved.
+
+It can also be queried directly::
+
+    for employee in session.query(Employee).filter(Employee.name == 'qbert'):
+        print(employee)
+
+
 .. _declarative_mixins:
 
 Mixin and Custom Base Classes
@@ -1192,6 +1212,20 @@ assumed to be completed and the 'configure' step has finished::
 
 .. versionadded:: 0.7.3
 
+``__declare_first__()``
+~~~~~~~~~~~~~~~~~~~~~~
+
+Like ``__declare_last__()``, but is called at the beginning of mapper configuration
+via the :meth:`.MapperEvents.before_configured` event::
+
+    class MyClass(Base):
+        @classmethod
+        def __declare_first__(cls):
+            ""
+            # do something before mappings are configured
+
+.. versionadded:: 0.9.3
+
 .. _declarative_abstract:
 
 ``__abstract__``
diff --git a/lib/sqlalchemy/ext/declarative/api.py b/lib/sqlalchemy/ext/declarative/api.py
index 2418c6e50..84b97f629 100644
--- a/lib/sqlalchemy/ext/declarative/api.py
+++ b/lib/sqlalchemy/ext/declarative/api.py
@@ -20,7 +20,7 @@ from .base import _as_declarative, \
                 _declarative_constructor,\
                 _DeferredMapperConfig, _add_attribute
 from .clsregistry import _class_resolver
-
+from . import clsregistry
 
 def instrument_declarative(cls, registry, metadata):
     """Given a class, configure the class declaratively,
@@ -325,7 +325,7 @@ class ConcreteBase(object):
          ), 'type', 'pjoin')
 
     @classmethod
-    def __declare_last__(cls):
+    def __declare_first__(cls):
         m = cls.__mapper__
         if m.with_polymorphic:
             return
@@ -370,10 +370,11 @@ class AbstractConcreteBase(ConcreteBase):
     __abstract__ = True
 
     @classmethod
-    def __declare_last__(cls):
+    def __declare_first__(cls):
         if hasattr(cls, '__mapper__'):
             return
 
+        clsregistry.add_class(cls.__name__, cls)
         # can't rely on 'self_and_descendants' here
         # since technically an immediate subclass
         # might not be mapped, but a subclass
diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py
index 4fda9c734..eb66f12b6 100644
--- a/lib/sqlalchemy/ext/declarative/base.py
+++ b/lib/sqlalchemy/ext/declarative/base.py
@@ -52,6 +52,10 @@ def _as_declarative(cls, classname, dict_):
             @event.listens_for(mapper, "after_configured")
             def go():
                 cls.__declare_last__()
+        if '__declare_first__' in base.__dict__:
+            @event.listens_for(mapper, "before_configured")
+            def go():
+                cls.__declare_first__()
         if '__abstract__' in base.__dict__:
             if (base is cls or
                 (base in cls.__bases__ and not _is_declarative_inherits)
diff --git a/lib/sqlalchemy/orm/events.py b/lib/sqlalchemy/orm/events.py
index b2c356f24..52dcca232 100644
--- a/lib/sqlalchemy/orm/events.py
+++ b/lib/sqlalchemy/orm/events.py
@@ -583,6 +583,23 @@ class MapperEvents(event.Events):
         """
         # TODO: need coverage for this event
 
+    def before_configured(self):
+        """Called before a series of mappers have been configured.
+
+        This corresponds to the :func:`.orm.configure_mappers` call, which
+        note is usually called automatically as mappings are first
+        used.
+
+        Theoretically this event is called once per
+        application, but is actually called any time new mappers
+        are to be affected by a :func:`.orm.configure_mappers`
+        call.   If new mappings are constructed after existing ones have
+        already been used, this event can be called again.
+
+        .. versionadded:: 0.9.3
+
+        """
+
     def after_configured(self):
         """Called after a series of mappers have been configured.
 
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 26f105bec..70df9a679 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -2512,7 +2512,6 @@ def configure_mappers():
     if not Mapper._new_mappers:
         return
 
-    _call_configured = None
     _CONFIGURE_MUTEX.acquire()
     try:
         global _already_compiling
@@ -2525,10 +2524,12 @@ def configure_mappers():
             if not Mapper._new_mappers:
                 return
 
+            Mapper.dispatch(Mapper).before_configured()
             # initialize properties on all mappers
             # note that _mapper_registry is unordered, which
             # may randomly conceal/reveal issues related to
             # the order of mapper compilation
+
             for mapper in list(_mapper_registry):
                 if getattr(mapper, '_configure_failed', False):
                     e = sa_exc.InvalidRequestError(
@@ -2544,7 +2545,6 @@ def configure_mappers():
                         mapper._expire_memoizations()
                         mapper.dispatch.mapper_configured(
                                 mapper, mapper.class_)
-                        _call_configured = mapper
                     except:
                         exc = sys.exc_info()[1]
                         if not hasattr(exc, '_configure_failed'):
@@ -2556,8 +2556,7 @@ def configure_mappers():
             _already_compiling = False
     finally:
         _CONFIGURE_MUTEX.release()
-    if _call_configured is not None:
-        _call_configured.dispatch.after_configured()
+    Mapper.dispatch(Mapper).after_configured()
 
 
 def reconstructor(fn):
diff --git a/lib/sqlalchemy/testing/entities.py b/lib/sqlalchemy/testing/entities.py
index 0553e0e22..9309abfd8 100644
--- a/lib/sqlalchemy/testing/entities.py
+++ b/lib/sqlalchemy/testing/entities.py
@@ -85,8 +85,12 @@ class ComparableEntity(BasicEntity):
                     return False
 
                 if hasattr(value, '__iter__'):
-                    if list(value) != list(battr):
-                        return False
+                    if hasattr(value, '__getitem__') and not hasattr(value, 'keys'):
+                        if list(value) != list(battr):
+                            return False
+                    else:
+                        if set(value) != set(battr):
+                            return False
                 else:
                     if value is not None and value != battr:
                         return False
diff --git a/test/ext/declarative/test_inheritance.py b/test/ext/declarative/test_inheritance.py
index 01bf3f3f6..22dd32e45 100644
--- a/test/ext/declarative/test_inheritance.py
+++ b/test/ext/declarative/test_inheritance.py
@@ -1201,3 +1201,81 @@ class ConcreteInhTest(_RemoveListeners, DeclarativeTestBase):
             __mapper_args__ = {'polymorphic_identity': "engineer",
                             'concrete': True}
         self._roundtrip(Employee, Manager, Engineer, Boss, explicit_type=True)
+
+class ConcreteExtensionConfigTest(_RemoveListeners, testing.AssertsCompiledSQL, DeclarativeTestBase):
+    __dialect__ = 'default'
+
+    def test_classreg_setup(self):
+        class A(Base, fixtures.ComparableEntity):
+            __tablename__ = 'a'
+            id = Column(Integer, primary_key=True, test_needs_autoincrement=True)
+            data = Column(String(50))
+            collection = relationship("BC", primaryjoin="BC.a_id == A.id",
+                            collection_class=set)
+
+        class BC(AbstractConcreteBase, Base, fixtures.ComparableEntity):
+            pass
+
+        class B(BC):
+            __tablename__ = 'b'
+            id = Column(Integer, primary_key=True, test_needs_autoincrement=True)
+
+            a_id = Column(Integer, ForeignKey('a.id'))
+            data = Column(String(50))
+            b_data = Column(String(50))
+            __mapper_args__ = {
+                "polymorphic_identity": "b",
+                "concrete": True
+            }
+
+        class C(BC):
+            __tablename__ = 'c'
+            id = Column(Integer, primary_key=True, test_needs_autoincrement=True)
+            a_id = Column(Integer, ForeignKey('a.id'))
+            data = Column(String(50))
+            c_data = Column(String(50))
+            __mapper_args__ = {
+                "polymorphic_identity": "c",
+                "concrete": True
+            }
+
+        Base.metadata.create_all()
+        sess = Session()
+        sess.add_all([
+            A(data='a1', collection=set([
+                B(data='a1b1', b_data='a1b1'),
+                C(data='a1b2', c_data='a1c1'),
+                B(data='a1b2', b_data='a1b2'),
+                C(data='a1c2', c_data='a1c2'),
+            ])),
+            A(data='a2', collection=set([
+                B(data='a2b1', b_data='a2b1'),
+                C(data='a2c1', c_data='a2c1'),
+                B(data='a2b2', b_data='a2b2'),
+                C(data='a2c2', c_data='a2c2'),
+            ]))
+        ])
+        sess.commit()
+        sess.expunge_all()
+
+        eq_(
+            sess.query(A).filter_by(data='a2').all(),
+            [
+                A(data='a2', collection=set([
+                    B(data='a2b1', b_data='a2b1'),
+                    B(data='a2b2', b_data='a2b2'),
+                    C(data='a2c1', c_data='a2c1'),
+                    C(data='a2c2', c_data='a2c2'),
+                ]))
+            ]
+        )
+
+        self.assert_compile(
+            sess.query(A).join(A.collection),
+            "SELECT a.id AS a_id, a.data AS a_data FROM a JOIN "
+            "(SELECT c.id AS id, c.a_id AS a_id, c.data AS data, "
+                "c.c_data AS c_data, CAST(NULL AS VARCHAR(50)) AS b_data, "
+                "'c' AS type FROM c UNION ALL SELECT b.id AS id, b.a_id AS a_id, "
+                "b.data AS data, CAST(NULL AS VARCHAR(50)) AS c_data, "
+                "b.b_data AS b_data, 'b' AS type FROM b) AS pjoin ON pjoin.a_id = a.id"
+        )