[multi-db] Fixed bugs in handling of pending references. Fixed dropping of test database, and ensured that it drops even if syncdb() fails.

git-svn-id: http://code.djangoproject.com/svn/django/branches/multiple-db-support@3760 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/core/management.py

@@ -73,11 +73,14 @@ def get_sql_create(app):
     # final output will be divided by comments into sections for each
     # named connection, if there are any named connections
     connection_output = {}
-    pending_references = {}
+    pending = {}
     final_output = []
     
     app_models = models.get_models(app, creation_order=True)
     for model in app_models:
+
+        print "Get create sql for model", model
+        
         opts = model._meta
         connection_name = model_connection_name(model)
         output = connection_output.setdefault(connection_name, [])
@@ -109,21 +112,14 @@ def get_sql_create(app):
             # table list.
             tables = []
 
-        installed_models = [ model for model in
+        installed_models = [ m for m in
                              manager.get_installed_models(tables)
-                             if model not in app_models ]
+                             if m not in app_models ]
         models_output = set(installed_models) 
         builder = creation.builder
         builder.models_already_seen.update(models_output)
-        model_output, references = builder.get_create_table(model, style)
+        model_output, pending = builder.get_create_table(model, style, pending)
         output.extend(model_output)
-        for refto, refs in references.items():
-            try:
-                pending_references[refto].extend(refs)
-            except KeyError:
-                pending_references[refto] = refs
-        if model in pending_references:
-            output.extend(pending_references.pop(model))
 
         # Create the many-to-many join tables.
         many_many = builder.get_create_many_to_many(model, style)
@@ -131,14 +127,18 @@ def get_sql_create(app):
             output.extend(statements)
 
     final_output = _collate(connection_output)
+    
     # Handle references to tables that are from other apps
     # but don't exist physically
-    not_installed_models = set(pending_references.keys())
+    not_installed_models = set(pending.keys())
     if not_installed_models:
         alter_sql = []
         for model in not_installed_models:
-            alter_sql.extend(['-- ' + sql
+            builder = model._default_manager.db.builder.get_creation_module().builder
-                              for sql in pending_references.pop(model)])
+            
+            for rel_class, f in pending[model]:
+                sql = builder._ref_sql(model, rel_class, f, style)
+                alter_sql.extend(['-- ', str(sql)])
         if alter_sql:
             final_output.append('-- The following references should be added '
                                 'but depend on non-existent tables:')
@@ -406,22 +406,20 @@ def _install(app, commit=True, initial_data=True):
             models_installed = manager.get_installed_models(tables)
             # Don't re-install already-installed models
             if not model in models_installed:
-                new_pending = manager.install(initial_data=initial_data)
+                pending = manager.install(initial_data=initial_data,
+                                          pending=pending)
                 created_models.append(model)
-                for dep_model, statements in new_pending.items():
+                
-                    pending.setdefault(dep_model, []).extend(statements)
-            # Execute any pending statements that were waiting for this model
-            if model in pending:
-                for statement in pending.pop(model):
-                    statement.execute()                
         if pending:            
-            for model, statements in pending.items():
-                manager = model._default_manager 
-                tables = manager.get_table_list()
             models_installed = manager.get_installed_models(tables)
+
+            for model in pending.keys():
                 if model in models_installed:
-                    for statement in statements:
+                    for rel_class, f in pending[model]:
+                        manager = model._default_manager 
+                        for statement in manager.get_pending(rel_class, f):
                             statement.execute()
+                    pending.pop(model)
                 else:
                     raise Exception("%s is not installed, but there are "
                                     "pending statements that need it: %s"

django/db/backends/ansi/sql.py

@@ -50,7 +50,7 @@ class SchemaBuilder(object):
         # table cache; set to short-circuit table lookups
         self.tables = None
         
-    def get_create_table(self, model, style=None):
+    def get_create_table(self, model, style=None, pending=None):
         """Construct and return the SQL expression(s) needed to create the
         table for the given model, and any constraints on that
         table. The return value is a 2-tuple. The first element of the tuple
@@ -61,6 +61,8 @@ class SchemaBuilder(object):
         """
         if style is None:
             style = default_style
+        if pending is None:
+            pending = {}
         self.models_already_seen.add(model)
         
         opts = model._meta
@@ -70,13 +72,6 @@ class SchemaBuilder(object):
         data_types = db.get_creation_module().DATA_TYPES
         table_output = []
 
-        # pending field references, keyed by the model class
-        # they reference
-        pending_references = {}
-
-        # pending statements to execute, keyed by
-        # the model class they reference
-        pending = {}
         for f in opts.fields:
             if isinstance(f, models.ForeignKey):
                 rel_field = f.rel.get_related_field()
@@ -108,7 +103,7 @@ class SchemaBuilder(object):
                     else:
                         # We haven't yet created the table to which this field
                         # is related, so save it for later.
-                        pending_references.setdefault(f.rel.to, []).append(f)
+                        pending.setdefault(f.rel.to, []).append((model, f))
                 table_output.append(' '.join(field_output))
         if opts.order_with_respect_to:
             table_output.append(style.SQL_FIELD(quote_name('_order')) + ' ' + \
@@ -128,24 +123,15 @@ class SchemaBuilder(object):
         full_statement.append(');')
         create = [BoundStatement('\n'.join(full_statement), db.connection)]
 
-        if (pending_references and
+        # Pull out any pending statements for me
+        if (pending and
             backend.supports_constraints):
-            for rel_class, cols in pending_references.items():
+            if model in pending:
-                for f in cols:
+                for rel_class, f in pending[model]:
-                    rel_opts = rel_class._meta
+                    create.append(self.get_ref_sql(model, rel_class, f,
-                    r_table = rel_opts.db_table
+                                                   style=style))
-                    r_col = f.column
+                # What was pending for me is now no longer pending
-                    table = opts.db_table
+                pending.pop(model)
-                    col = opts.get_field(f.rel.field_name).column
-                    # For MySQL, r_name must be unique in the first 64 
-                    # characters. So we are careful with character usage here.
-                    r_name = '%s_refs_%s_%x' % (col, r_col,
-                                                abs(hash((r_table, table))))
-                    sql = style.SQL_KEYWORD('ALTER TABLE') + ' %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s);' % \
-                        (quote_name(table), quote_name(r_name),
-                        quote_name(r_col), quote_name(r_table), quote_name(col))
-                    pending.setdefault(rel_class, []).append(
-                        BoundStatement(sql, db.connection))
         return (create, pending)    
 
     def get_create_indexes(self, model, style=None):
@@ -323,6 +309,31 @@ class SchemaBuilder(object):
                                           and 'IntegerField' \
                                           or f.get_internal_type()
 
+    def get_ref_sql(self, model, rel_class, f, style=None):
+        """Get sql statement for a reference between model and rel_class on
+        field f.
+        """
+        if style is None:
+            style = default_style
+        
+        db = model._default_manager.db
+        qn = db.backend.quote_name
+        opts = model._meta
+        rel_opts = rel_class._meta
+        table = rel_opts.db_table
+        r_col = f.column
+        r_table = opts.db_table
+        col = opts.get_field(f.rel.field_name).column
+        # For MySQL, r_name must be unique in the first 64 
+        # characters. So we are careful with character usage here.
+        r_name = '%s_refs_%s_%x' % (col, r_col,
+                                    abs(hash((r_table, table))))
+        sql = style.SQL_KEYWORD('ALTER TABLE') + \
+              ' %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s);' % \
+              (qn(table), qn(r_name),
+               qn(r_col), qn(r_table), qn(col))
+        return BoundStatement(sql, db.connection)
+        
     def get_references(self):
         """Fill (if needed) and return the reference cache.
         """

django/db/models/manager.py

@@ -118,7 +118,7 @@ class Manager(object):
     # SCHEMA MANIPULATION #
     #######################
 
-    def install(self, initial_data=False):
+    def install(self, initial_data=False, pending=None):
         """Install my model's table, indexes and (if requested) initial data.
 
         Returns a dict of pending statements, keyed by the model that
@@ -127,8 +127,10 @@ class Manager(object):
         such as foreign key constraints for tables that don't exist at
         install time.)
         """
+        if pending is None:
+            pending = {}
         builder = self.db.get_creation_module().builder
-        run, pending = builder.get_create_table(self.model)
+        run, pending = builder.get_create_table(self.model, pending=pending)
         run += builder.get_create_indexes(self.model)
         many_many = builder.get_create_many_to_many(self.model)
 
@@ -144,6 +146,10 @@ class Manager(object):
             self.load_initial_data()
         return pending
 
+    def get_pending(self, rel_class, f):
+        builder = self.db.get_creation_module().builder
+        return builder.get_ref_sql(self.model, rel_class, f)
+
     def load_initial_data(self):
         """Install initial data for model in db, Returns statements executed.
         """

django/test/simple.py

@@ -78,8 +78,9 @@ def run_tests(module_list, verbosity=1, extra_tests=[]):
 
     old_name = settings.DATABASE_NAME
     create_test_db(verbosity)
+    try:
         management.syncdb(verbosity, interactive=False)        
         unittest.TextTestRunner(verbosity=verbosity).run(suite)
+    finally:
         destroy_test_db(old_name, verbosity)
-    
         teardown_test_environment()

django/test/utils.py

@@ -66,19 +66,19 @@ def create_test_db(verbosity=1, autoclobber=False):
         cursor = connection.cursor()
         _set_autocommit(connection)
         try:
-            cursor.execute("CREATE DATABASE %s" % qn(db_name))
+            cursor.execute("CREATE DATABASE %s" % qn(TEST_DATABASE_NAME))
         except Exception, e:            
             sys.stderr.write("Got an error creating the test database: %s\n" % e)
             if not autoclobber:
-                confirm = raw_input("It appears the test database, %s, already exists. Type 'yes' to delete it, or 'no' to cancel: " % db_name)
+                confirm = raw_input("It appears the test database, %s, already exists. Type 'yes' to delete it, or 'no' to cancel: " % TEST_DATABASE_NAME)
             if autoclobber or confirm == 'yes':
                 try:
                     if verbosity >= 1:
                         print "Destroying old test database..."                
-                    cursor.execute("DROP DATABASE %s" % qn(db_name))
+                    cursor.execute("DROP DATABASE %s" % qn(TEST_DATABASE_NAME))
                     if verbosity >= 1:
                         print "Creating test database..."
-                    cursor.execute("CREATE DATABASE %s" % qn(db_name))
+                    cursor.execute("CREATE DATABASE %s" % qn(TEST_DATABASE_NAME))
                 except Exception, e:
                     sys.stderr.write("Got an error recreating the test database: %s\n" % e)
                     sys.exit(2)
@@ -118,12 +118,15 @@ def destroy_test_db(old_database_name, old_databases, verbosity=1):
     # connected to it.
     if verbosity >= 1:
         print "Destroying test database..."
-    connection.close()
+    for cnx in connections.keys():
+        connections[cnx].close()
     TEST_DATABASE_NAME = settings.DATABASE_NAME
     settings.DATABASE_NAME = old_database_name
 
     if settings.DATABASE_ENGINE != "sqlite3":
         settings.OTHER_DATABASES = old_databases
+        for cnx in connections.keys():
+            connections[cnx].connection.cursor()
         cursor = connection.cursor()
         _set_autocommit(connection)
         time.sleep(1) # To avoid "database is being accessed by other users" errors.

tests/modeltests/multiple_databases/models.py

@@ -78,7 +78,14 @@ __test__ = {'API_TESTS': """
 >>> from django.db import connection, connections, _default, model_connection_name
 >>> from django.conf import settings
 
-# The default connection is in there, but let's ignore it
+# Connections are referenced by name
+>>> connections['_a']
+Connection: ...
+>>> connections['_b']
+Connection: ...
+
+# Let's see what connections are available.The default connection is
+# in there, but let's ignore it
 
 >>> non_default = connections.keys()
 >>> non_default.remove(_default)
@@ -86,12 +93,6 @@ __test__ = {'API_TESTS': """
 >>> non_default
 ['_a', '_b']
     
-# Each connection references its settings
->>> connections['_a'].settings.DATABASE_NAME == settings.OTHER_DATABASES['_a']['DATABASE_NAME']
-True
->>> connections['_b'].settings.DATABASE_NAME == settings.OTHER_DATABASES['_b']['DATABASE_NAME']
-True
-    
 # Invalid connection names raise ImproperlyConfigured
 >>> connections['bad']
 Traceback (most recent call last):

tests/regressiontests/ansi_sql/models.py

@@ -20,7 +20,7 @@ Set([<class 'regressiontests.ansi_sql.models.Car'>])
 # test pending relationships
 >>> builder.models_already_seen = set()
 >>> builder.get_create_table(Mod)
-([BoundStatement('CREATE TABLE "ansi_sql_mod" (..."car_id" integer NOT NULL,...);')], {<class 'regressiontests.ansi_sql.models.Car'>: [BoundStatement('ALTER TABLE "ansi_sql_mod" ADD CONSTRAINT ... FOREIGN KEY ("car_id") REFERENCES "ansi_sql_car" ("id");')]})
+([BoundStatement('CREATE TABLE "ansi_sql_mod" (..."car_id" integer NOT NULL,...);')], {<class 'regressiontests.ansi_sql.models.Car'>: [(<class 'regressiontests.ansi_sql.models.Mod'>, <django.db.models.fields.related.ForeignKey...>)]})
 >>> builder.models_already_seen = set()
 >>> builder.get_create_table(Car)
 ([BoundStatement('CREATE TABLE "ansi_sql_car" (...);')], {})
@@ -45,7 +45,7 @@ Set([<class 'regressiontests.ansi_sql.models.Car'>])
 # patch builder so that it looks for initial data where we want it to
 # >>> builder.get_initialdata_path = othertests_sql
 >>> builder.get_initialdata(Car)
-[BoundStatement('insert into ansi_sql_car (...)...values (...);')]
+[BoundStatement("insert into ansi_sql_car (...)...values (...);...")]
 
 # test drop
 >>> builder.get_drop_table(Mod)

tests/regressiontests/ansi_sql/sql/car.sql

@@ -1,2 +1,2 @@
 insert into ansi_sql_car (make, model, year, condition)
-       values ("Chevy", "Impala", 1966, "mint");
+       values ('Chevy', 'Impala', 1966, 'mint');

tests/regressiontests/manager_schema_manipulation/tests.py

@@ -123,7 +123,7 @@
 >>> PA._default_manager.db.backend.supports_constraints = True
 >>> result = PA.objects.install()
 >>> result
-{<class 'regressiontests.manager_schema_manipulation.tests.PC'>: [BoundStatement('ALTER TABLE "msm_pa" ADD CONSTRAINT "id_refs_c_id..." FOREIGN KEY ("c_id") REFERENCES "msm_pc" ("id");')]}
+{<class 'regressiontests.manager_schema_manipulation.tests.PC'>: [(<class 'regressiontests.manager_schema_manipulation.tests.PA'>, <django.db.models.fields.related.ForeignKey ...>)]}
 
 # NOTE: restore real constraint flag
 >>> PA._default_manager.db.backend.supports_constraints = real_cnst