Merge pull request #1244 from maaaks/1076

Fixed sort_models_topologically()

peewee.py

@@ -202,19 +202,26 @@ def sort_models_topologically(models):
         seen = set()
         ordering = []
         def dfs(model):
+            # Omit models which are already sorted
+            # or should not be in the list at all
             if model in models and model not in seen:
                 seen.add(model)
-                for foreign_key in model._meta.reverse_rel.values():
-                    dfs(foreign_key.model_class)
-                ordering.append(model)  # parent will follow descendants
+
+                # First create models on which current model depends
+                # (either through foreign keys or through depends_on),
+                # then create current model itself
+                for foreign_key in model._meta.rel.values():
+                    dfs(foreign_key.rel_model)
                 if model._meta.depends_on:
                     for dependency in model._meta.depends_on:
                         dfs(dependency)
+                ordering.append(model)
+
         # Order models by name and table initially to guarantee total ordering.
         names = lambda m: (m._meta.name, m._meta.db_table)
-        for m in sorted(models, key=names, reverse=True):
+        for m in sorted(models, key=names):
             dfs(m)
-        return list(reversed(ordering))
+        return ordering
 
     def strip_parens(s):
         # Quick sanity check.

playhouse/_speedups.pyx

@@ -329,20 +329,20 @@ cdef tuple _sort_key(model):
 cdef _sort_models(model, set model_set, set seen, list accum):
     if model in model_set and model not in seen:
         seen.add(model)
-        for foreign_key in model._meta.reverse_rel.values():
-            _sort_models(foreign_key.model_class, model_set, seen, accum)
-        accum.append(model)
+        for foreign_key in model._meta.rel.values():
+            _sort_models(foreign_key.rel_model, model_set, seen, accum)
         if model._meta.depends_on is not None:
             for dependency in model._meta.depends_on:
                 _sort_models(dependency, model_set, seen, accum)
+        accum.append(model)
 
 def sort_models_topologically(models):
     cdef:
         set model_set = set(models)
         set seen = set()
         list accum = []
 
-    for model in sorted(model_set, key=_sort_key, reverse=True):
+    for model in sorted(model_set, key=_sort_key):
         _sort_models(model, model_set, seen, accum)
 
-    return list(reversed(accum))
+    return accum

playhouse/tests/test_helpers.py

@@ -110,6 +110,23 @@ class Meta:
         for pmodels in permutations(models):
             ordering = sort_models_topologically(pmodels)
             self.assertEqual(ordering, ordered)
+
+    def test_declared_dependencies_2(self):
+        class C(Model):
+            pass
+        class B(Model):
+            c = ForeignKeyField(C)
+        class A(Model):
+            class Meta:
+                depends_on = B,
+            c = ForeignKeyField(C)
+
+        models = [ C, B, A ]
+
+        ordered = list(models)
+        for pmodels in permutations(models):
+            ordering = sort_models_topologically(pmodels)
+            self.assertEqual(ordering, ordered)
 
 
 def permutations(xs):