Autogenerate now orders tables correctly. Dependency tables now appear before their...

diff --git a/alembic/autogenerate.py b/alembic/autogenerate.py
index d0a5e736cb8edd22b8a03ee6d1a7790cc8c47528..710da8f980e56a39b9ba7a1d58499d52b30ab2b8 100644 (file)
--- a/alembic/autogenerate.py
+++ b/alembic/autogenerate.py
@@ -3,6 +3,7 @@ automatically."""
 
 from alembic import util
 from sqlalchemy.engine.reflection import Inspector
+from sqlalchemy.util._collections import OrderedSet
 from sqlalchemy import schema, types as sqltypes
 import re
 
@@ -54,9 +55,10 @@ def _produce_net_changes(connection, metadata, diffs, autogen_context):
     # TODO: not hardcode alembic_version here ?
     conn_table_names = set(inspector.get_table_names()).\
                             difference(['alembic_version'])
-    metadata_table_names = set(metadata.tables)
+    metadata_table_names = OrderedSet([table.name for table in metadata.sorted_tables])
 
-    _compare_tables(conn_table_names, metadata_table_names, inspector, metadata, diffs, autogen_context)
+    _compare_tables(conn_table_names, metadata_table_names,
+                    inspector, metadata, diffs, autogen_context)
 
 def _compare_tables(conn_table_names, metadata_table_names, 
                     inspector, metadata, diffs, autogen_context):
@@ -235,7 +237,7 @@ def _produce_upgrade_commands(diffs, autogen_context):
 
 def _produce_downgrade_commands(diffs, autogen_context):
     buf = []
-    for diff in diffs:
+    for diff in reversed(diffs):
         buf.append(_invoke_command("downgrade", diff, autogen_context))
     if not buf:
         buf = ["pass"]

diff --git a/tests/test_autogenerate.py b/tests/test_autogenerate.py
index 497ceb210dc58307a8318c60c5f2944082e62384..e03ceead1488667206d8cfd84617164d01225d50 100644 (file)
--- a/tests/test_autogenerate.py
+++ b/tests/test_autogenerate.py
@@ -69,6 +69,23 @@ def _model_two():
     )
     return m
 
+def _model_three():
+    m = MetaData()
+    return m
+
+def _model_four():
+    m = MetaData()
+
+    Table('parent', m,
+        Column('id', Integer, primary_key=True)
+    )
+
+    Table('child', m,
+        Column('parent_id', Integer, ForeignKey('parent.id')),
+    )
+
+    return m
+
 class AutogenerateDiffTest(TestCase):
     @classmethod
     @requires_07
@@ -111,8 +128,8 @@ class AutogenerateDiffTest(TestCase):
         connection = self.context.bind
         diffs = []
         autogenerate._produce_net_changes(connection, metadata, diffs, 
-                                        self.autogen_context)
-
+                                          self.autogen_context)
+        
         eq_(
             diffs[0],
             ('add_table', metadata.tables['item'])
@@ -148,7 +165,6 @@ class AutogenerateDiffTest(TestCase):
         eq_(diffs[7][0][4], True)
         eq_(diffs[7][0][5], False)
 
-
     def test_render_nothing(self):
         context = MigrationContext.configure(
             connection = self.bind.connect(),
@@ -206,28 +222,28 @@ class AutogenerateDiffTest(TestCase):
     ### end Alembic commands ###""")
         eq_(re.sub(r"u'", "'", template_args['downgrades']),
 """### commands auto generated by Alembic - please adjust! ###
-    op.drop_table('item')
+    op.alter_column('user', 'name', 
+               existing_type=sa.VARCHAR(length=50), 
+               nullable=True)
+    op.alter_column('user', 'a1', 
+               existing_type=sa.TEXT(), 
+               server_default=None, 
+               existing_nullable=True)
+    op.add_column('user', sa.Column('pw', sa.VARCHAR(length=50), nullable=True))
+    op.alter_column('order', 'amount', 
+               existing_type=sa.Numeric(precision=10, scale=2), 
+               type_=sa.NUMERIC(precision=8, scale=2), 
+               nullable=False, 
+               existing_server_default='0')
+    op.drop_column('order', 'user_id')
+    op.drop_column('address', 'street')
     op.create_table('extra',
     sa.Column('x', sa.CHAR(), nullable=True),
     sa.Column('uid', sa.INTEGER(), nullable=True),
     sa.ForeignKeyConstraint(['uid'], ['user.id'], ),
     sa.PrimaryKeyConstraint()
     )
-    op.drop_column('address', 'street')
-    op.drop_column('order', 'user_id')
-    op.alter_column('order', 'amount', 
-               existing_type=sa.Numeric(precision=10, scale=2), 
-               type_=sa.NUMERIC(precision=8, scale=2), 
-               nullable=False, 
-               existing_server_default='0')
-    op.add_column('user', sa.Column('pw', sa.VARCHAR(length=50), nullable=True))
-    op.alter_column('user', 'a1', 
-               existing_type=sa.TEXT(), 
-               server_default=None, 
-               existing_nullable=True)
-    op.alter_column('user', 'name', 
-               existing_type=sa.VARCHAR(length=50), 
-               nullable=True)
+    op.drop_table('item')
     ### end Alembic commands ###""")
 
     def test_skip_null_type_comparison_reflected(self):
@@ -282,6 +298,59 @@ class AutogenerateDiffTest(TestCase):
             [('remove_table', 'extra'), ('remove_table', u'user')]
         )
 
+class AutogenerateDiffOrderTest(TestCase):
+    @classmethod
+    @requires_07
+    def setup_class(cls):
+        staging_env()
+        cls.bind = sqlite_db()
+        cls.m3 = _model_three()
+        cls.m3.create_all(cls.bind)
+        cls.m4 = _model_four()
+
+        cls.empty_context = empty_context = MigrationContext.configure(
+            connection = cls.bind.connect(),
+            opts = {
+                'compare_type':True,
+                'compare_server_default':True,
+                'target_metadata':cls.m3,
+                'upgrade_token':"upgrades",
+                'downgrade_token':"downgrades",
+                'alembic_module_prefix':'op.',
+                'sqlalchemy_module_prefix':'sa.'
+            }
+        )
+        
+        connection = empty_context.bind
+        cls.autogen_empty_context = {
+            'imports':set(),
+            'connection':connection,
+            'dialect':connection.dialect,
+            'context':empty_context
+            }
+
+    @classmethod
+    def teardown_class(cls):
+        clear_staging_env()
+    
+    def test_diffs_order(self):
+        """
+        Added in order to test that child tables(tables with FKs) are generated
+        before their parent tables
+        """
+
+        metadata = self.m4
+        connection = self.empty_context.bind
+        diffs = []
+        
+        autogenerate._produce_net_changes(connection, metadata, diffs, 
+                                          self.autogen_empty_context)
+        
+        eq_(diffs[0][0], 'add_table')
+        eq_(diffs[0][1].name, "parent")
+        eq_(diffs[1][0], 'add_table')
+        eq_(diffs[1][1].name, "child")
+
 class AutogenRenderTest(TestCase):
     """test individual directives"""