base all_orm_descriptors ordering on cls.__dict__ + cls.__mro__

Adjusted the workings of the :meth:`_orm.Mapper.all_orm_descriptors`
accessor to represent the attributes in the order that they are located in
a deterministic way, assuming the use of Python 3.6 or higher which
maintains the sorting order of class attributes based on how they were
declared.   This sorting is not guaranteed to match the declared order of
attributes in all cases however; see the method documentation for the exact
scheme.

Fixes: #5494
Change-Id: I6ee8d4ace3eb8b3f7c9c0f2a3d7e27b5f62abfd3
(cherry picked from commit 9a3fee2cb6b608eb5c0263cf5a7e9085f74f2e73)

diff --git a/doc/build/changelog/unreleased_13/5494.rst b/doc/build/changelog/unreleased_13/5494.rst
new file mode 100644 (file)
index 0000000..a9b594e
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/5494.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: usecase, orm
+    :tickets: 5494
+
+    Adjusted the workings of the :meth:`_orm.Mapper.all_orm_descriptors`
+    accessor to represent the attributes in the order that they are located in
+    a deterministic way, assuming the use of Python 3.6 or higher which
+    maintains the sorting order of class attributes based on how they were
+    declared.   This sorting is not guaranteed to match the declared order of
+    attributes in all cases however; see the method documentation for the exact
+    scheme.
+
+

diff --git a/lib/sqlalchemy/orm/instrumentation.py b/lib/sqlalchemy/orm/instrumentation.py
index d6a47576d75e05a599a9716d2293b6020999e1fe..03a95d156bb45927210e0e301e6a090132f0083a 100644 (file)
--- a/lib/sqlalchemy/orm/instrumentation.py
+++ b/lib/sqlalchemy/orm/instrumentation.py
@@ -137,12 +137,24 @@ class ClassManager(dict):
         :class:`.AssociationProxy`.
 
         """
-        if exclude is None:
-            exclude = set()
-        for supercls in self.class_.__mro__:
-            for key in set(supercls.__dict__).difference(exclude):
-                exclude.add(key)
-                val = supercls.__dict__[key]
+
+        found = {}
+
+        # constraints:
+        # 1. yield keys in cls.__dict__ order
+        # 2. if a subclass has the same key as a superclass, include that
+        #    key as part of the ordering of the superclass, because an
+        #    overridden key is usually installed by the mapper which is going
+        #    on a different ordering
+        # 3. don't use getattr() as this fires off descriptors
+
+        for supercls in self.class_.__mro__[0:-1]:
+            inherits = supercls.__mro__[1]
+            for key in supercls.__dict__:
+                found.setdefault(key, supercls)
+                if key in inherits.__dict__:
+                    continue
+                val = found[key].__dict__[key]
                 if (
                     isinstance(val, interfaces.InspectionAttr)
                     and val.is_attribute

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 1bcf0c9d410ecbee7a98cad27e0eb1331d238410..86c63df9499d9995c0c804136d29ca65f25eea04 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -2405,6 +2405,21 @@ class Mapper(InspectionAttr):
         the attribute :attr:`.InspectionAttr.extension_type` will refer
         to a constant that distinguishes between different extension types.
 
+        The sorting of the attributes is based on what is located in
+        the ``__dict__`` of the mapped class as well as its mapped
+        superclasses.    The sorting will be all those attribute names
+        that appear in the ``__dict__`` of the immediate class and not
+        any of its superclasses, then the names which appear in the
+        ``__dict__`` of the superclass and not any of the further superclasses,
+        all the way down.   This will produce a deterministic ordering on
+        Python 3.6 and above.   It is not guaranteed to match the declared
+        ordering of attributes on the class, however, as the mapping process
+        itself populates Python descriptors into the ``__dict__`` of a mapped
+        class which are not always explicit in a declarative mapping.
+
+        .. versionchanged:: 1.4 ensured deterministic ordering for
+           :meth:`_orm.Mapper.all_orm_descriptors`.
+
         When dealing with a :class:`.QueryableAttribute`, the
         :attr:`.QueryableAttribute.property` attribute refers to the
         :class:`.MapperProperty` property, which is what you get when

diff --git a/lib/sqlalchemy/testing/requirements.py b/lib/sqlalchemy/testing/requirements.py
index c49c7be52b3cf6908a4b42b0fa6e904d3098869e..df3aa4613b38da69114cb24de2e9c303188cdf78 100644 (file)
--- a/lib/sqlalchemy/testing/requirements.py
+++ b/lib/sqlalchemy/testing/requirements.py
@@ -1021,6 +1021,10 @@ class SuiteRequirements(Requirements):
             lambda: sys.version_info < (3,), "Python version 3.xx is required."
         )
 
+    @property
+    def pep520(self):
+        return self.python36
+
     @property
     def python36(self):
         return exclusions.skip_if(

diff --git a/test/orm/test_inspect.py b/test/orm/test_inspect.py
index 4a39fc87d0af7047f32b24852a74be4bd61238b4..3130269b153912fd5ccce41c4a2ae48ebb671e0b 100644 (file)
--- a/test/orm/test_inspect.py
+++ b/test/orm/test_inspect.py
@@ -432,6 +432,139 @@ class TestORMInspection(_fixtures.FixtureTest):
             set(["id", "name", "q", "foob"]),
         )
 
+    def _random_names(self):
+        import random
+
+        return [
+            "".join(
+                random.choice("abcdegfghijklmnopqrstuvwxyz")
+                for i in range(random.randint(3, 15))
+            )
+            for j in range(random.randint(4, 12))
+        ]
+
+    def _ordered_name_fixture(self, glbls, clsname, base, supercls):
+        import random
+        from sqlalchemy import Integer, Column
+        import textwrap
+
+        names = self._random_names()
+
+        if base is supercls:
+            pk_names = set(
+                random.choice(names) for i in range(random.randint(1, 3))
+            )
+            fk_name = random.choice(
+                [name for name in names if name not in pk_names]
+            )
+        else:
+            pk_names = []
+            fk_name = None
+
+        def _make_name(name):
+            if name in pk_names:
+                return "%s = Column(Integer, primary_key=True)" % name
+            elif name == fk_name:
+                return "%s = Column(ForeignKey('myotherclass.id'))" % name
+            else:
+                type_ = random.choice(["relationship", "column", "hybrid"])
+                if type_ == "relationship":
+                    return "%s = relationship('MyOtherClass')" % name
+                elif type_ == "column":
+                    return "%s = Column(Integer)" % name
+                elif type_ == "hybrid":
+                    return (
+                        "@hybrid_property\ndef %s(self):\n    return None"
+                        % name
+                    )
+
+        glbls["Base"] = base
+        glbls["SuperCls"] = supercls
+
+        if base is supercls:
+
+            class MyOtherClass(base):
+                __tablename__ = "myotherclass"
+                id = Column(Integer, primary_key=True)
+
+            glbls["MyOtherClass"] = MyOtherClass
+        code = """
+
+from sqlalchemy import Column, Integer, ForeignKey
+from sqlalchemy.orm import relationship
+from sqlalchemy.ext.hybrid import hybrid_property
+
+class %s(SuperCls):
+    %s
+
+%s
+""" % (
+            clsname,
+            "__tablename__ = 'mytable'" if base is supercls else "",
+            "\n".join(
+                textwrap.indent(_make_name(name), "    ") for name in names
+            ),
+        )
+
+        exec(code, glbls)
+        return names, glbls[clsname]
+
+    @testing.requires.pep520
+    def test_all_orm_descriptors_pep520_noinh(self):
+        from sqlalchemy.ext.declarative import declarative_base
+
+        Base = declarative_base()
+
+        glbls = {}
+        names, MyClass = self._ordered_name_fixture(
+            glbls, "MyClass", Base, Base
+        )
+
+        eq_(MyClass.__mapper__.all_orm_descriptors.keys(), names)
+
+    @testing.requires.pep520
+    def test_all_orm_descriptors_pep520_onelevel_inh(self):
+        from sqlalchemy.ext.declarative import declarative_base
+
+        Base = declarative_base()
+
+        glbls = {}
+
+        base_names, MyClass = self._ordered_name_fixture(
+            glbls, "MyClass", Base, Base
+        )
+
+        sub_names, SubClass = self._ordered_name_fixture(
+            glbls, "SubClass", Base, MyClass
+        )
+
+        eq_(
+            SubClass.__mapper__.all_orm_descriptors.keys(),
+            sub_names + base_names,
+        )
+
+    @testing.requires.pep520
+    def test_all_orm_descriptors_pep520_classical(self):
+        class MyClass(object):
+            pass
+
+        from sqlalchemy.orm import mapper
+        from sqlalchemy import Table, MetaData, Column, Integer
+
+        names = self._random_names()
+
+        m = MetaData()
+        t = Table(
+            "t",
+            m,
+            Column("id", Integer, primary_key=True),
+            *[Column(name, Integer) for name in names]
+        )
+
+        m = mapper(MyClass, t)
+
+        eq_(m.all_orm_descriptors.keys(), ["id"] + names)
+
     def test_instance_state_ident_transient(self):
         User = self.classes.User
         u1 = User(name="ed")