Ticket #3615: allow_forward_refs_in_fixtures_v4.diff

docs/ref/databases.txt

@@ -142 +142 @@
 The InnoDB_ engine is fully transactional and supports foreign key references
 and is probably the best choice at this point in time.
 
+.. versionchanged:: 1.4
+
+In previous versions of Django, fixtures with forward references (i.e.
+relations to rows that have not yet been inserted into the database) would fail
+to load when using the InnoDB storage engine. This was due to the fact that InnoDB
+deviates from the SQL standard by checking foreign key constraints immediately
+instead of deferring the check until the transaction is committed. This
+problem has been resolved in Django 1.4. Fixture data is now loaded with foreign key
+checks turned off; foreign key checks are then re-enabled when the data has
+finished loading, at which point the entire table is checked for invalid foreign
+key references and an `IntegrityError` is raised if any are found.
+
 .. _storage engines: http://dev.mysql.com/doc/refman/5.5/en/storage-engines.html
 .. _MyISAM: http://dev.mysql.com/doc/refman/5.5/en/myisam-storage-engine.html
 .. _InnoDB: http://dev.mysql.com/doc/refman/5.5/en/innodb.html

django/db/backends/mysql/base.py

@@ -25 +25 @@
 from MySQLdb.converters import conversions
 from MySQLdb.constants import FIELD_TYPE, CLIENT
 
-from django.db import utils
+from django.db import utils, IntegrityError
 from django.db.backends import *
 from django.db.backends.signals import connection_created
 from django.db.backends.mysql.client import DatabaseClient
@@ -349 +349 @@
                 raise Exception('Unable to determine MySQL version from version string %r' % self.connection.get_server_info())
             self.server_version = tuple([int(x) for x in m.groups()])
         return self.server_version
+
+    def disable_foreign_key_checks(self):
+        """
+        Disable foreign key checks, primarily for us in adding rows with forward references. Always returns True,
+        to indicate checks have been disabled.
+        """
+        self.foreign_key_checks_disabled = True
+        self.cursor().execute('SET foreign_key_checks=0')
+        return True
+
+    def enable_foreign_key_checks(self):
+        """
+        Re-enable foreign key checks after they have been disabled. Always returns True,
+        to indicate checks have been re-enabled.
+        """
+        self.foreign_key_checks_disabled = False
+        self.cursor().execute('SET foreign_key_checks=1')
+        return True
+    
+    def check_for_invalid_foreign_keys(self, table_name):
+        """
+        Checks given table for rows with invalid foreign key references. This method is intended to be used in
+        conjunction with `disable_foreign_keys()` and `enable_foreign_keys()`, to determine if rows with
+        invalid references were entered while foreign key checks were off.
+        
+        Raises an IntegrityError on the first invalid foreign key reference encountered (if any) and provides
+        detailed information about the invalid reference in the error message.
+        """
+        cursor = self.cursor()
+        key_columns = self.introspection.get_key_columns(cursor, table_name)
+        for column_name, referenced_table_name, referenced_column_name in key_columns:
+            primary_key_column_name = [c[0] for c in self.introspection.get_indexes(cursor, table_name).iteritems() if c[1]['primary_key']][0]
+            cursor.execute("""
+                SELECT REFERRING.`%s`, REFERRING.`%s` FROM `%s` as REFERRING
+                LEFT JOIN `%s` as REFERRED
+                ON (REFERRING.`%s` = REFERRED.`%s`)
+                WHERE REFERRING.`%s` IS NOT NULL
+                    AND REFERRED.`%s` IS NULL"""
+                % (primary_key_column_name, column_name, table_name, referenced_table_name,
+                   column_name, referenced_column_name, column_name, referenced_column_name))
+            for bad_row in cursor.fetchall():
+                raise IntegrityError("The row in table '%s' with primary key '%s' has an invalid \
+foreign key: %s.%s contains a value '%s' that does not have a corresponding value in %s.%s."
+                                     % (table_name, bad_row[0], table_name, column_name, bad_row[1],
+                                        referenced_table_name, referenced_column_name))

django/db/backends/mysql/introspection.py

@@ -51 +51 @@
         representing all relationships to the given table. Indexes are 0-based.
         """
         my_field_dict = self._name_to_index(cursor, table_name)
-        constraints = []
+        constraints = self.get_key_columns(cursor, table_name)
         relations = {}
+        for my_fieldname, other_table, other_field in constraints:
+            other_field_index = self._name_to_index(cursor, other_table)[other_field]
+            my_field_index = my_field_dict[my_fieldname]
+            relations[my_field_index] = (other_field_index, other_table)
+        return relations
+
+    def get_key_columns(self, cursor, table_name):
+        """
+        Returns a list of [column_name, referenced_table_name, referenced_column_name] for all
+        key columns in given table.
+        """
+        key_columns = []
         try:
-            # This should work for MySQL 5.0.
             cursor.execute("""
                 SELECT column_name, referenced_table_name, referenced_column_name
                 FROM information_schema.key_column_usage
@@ -62 +73 @@
                     AND table_schema = DATABASE()
                     AND referenced_table_name IS NOT NULL
                     AND referenced_column_name IS NOT NULL""", [table_name])
-            constraints.extend(cursor.fetchall())
+            key_columns.extend(cursor.fetchall())
         except (ProgrammingError, OperationalError):
             # Fall back to "SHOW CREATE TABLE", for previous MySQL versions.
             # Go through all constraints and save the equal matches.
@@ -74 +85 @@
                     if match == None:
                         break
                     pos = match.end()
-                    constraints.append(match.groups())
+                    key_columns.append(match.groups())
+        return key_columns
 
-        for my_fieldname, other_table, other_field in constraints:
-            other_field_index = self._name_to_index(cursor, other_table)[other_field]
-            my_field_index = my_field_dict[my_fieldname]
-            relations[my_field_index] = (other_field_index, other_table)
-
-        return relations
-
     def get_indexes(self, cursor, table_name):
         """
         Returns a dictionary of fieldname -> infodict for the given table,

django/db/backends/__init__.py

@@ -34 +34 @@
         self.transaction_state = []
         self.savepoint_state = 0
         self._dirty = None
+        
+        self.foreign_key_checks_disabled = False
 
     def __eq__(self, other):
         return self.alias == other.alias
@@ -239 +241 @@
         if self.savepoint_state:
             self._savepoint_commit(sid)
 
+    def disable_foreign_key_checks(self):
+        """
+        Backends can implement as needed to temporarily disable foreign key constraint
+        checking.
+        """
+        pass
+
+    def enable_foreign_key_checks(self):
+        """
+        Backends can implement as needed to re-enable foreign key constraint
+        checking.
+        """
+        pass
+    
+    def check_for_invalid_foreign_keys(self, table_name):
+        """
+        Backends can implement as needed to raise IntegrityError if invalid foreign keys
+        are found in a table (for use when adding rows with foreign key checks disabled)
+        """
+        pass
+
     def close(self):
         if self.connection is not None:
             self.connection.close()
@@ -870 +893 @@
 
         return sequence_list
 
+    def get_key_columns(self, cursor, table_name):
+        raise NotImplementedError
+
 class BaseDatabaseClient(object):
     """
     This class encapsulates all backend-specific methods for opening a

django/db/backends/dummy/base.py

@@ -34 +34 @@
     get_table_description = complain
     get_relations = complain
     get_indexes = complain
+    get_key_columns = complain
 
 class DatabaseWrapper(BaseDatabaseWrapper):
     operators = {}

django/core/management/commands/loaddata.py

@@ -8 +8 @@
 from django.core import serializers
 from django.core.management.base import BaseCommand
 from django.core.management.color import no_style
-from django.db import connections, router, transaction, DEFAULT_DB_ALIAS
+from django.db import connections, router, transaction, DEFAULT_DB_ALIAS, models as db_models
 from django.db.models import get_apps
 from django.utils.itercompat import product
 
@@ -166 +166 @@
                                     (format, fixture_name, humanize(fixture_dir)))
                             try:
                                 objects = serializers.deserialize(format, fixture, using=using)
+                                foreign_key_checks_disabled = connection.disable_foreign_key_checks()
                                 for obj in objects:
                                     objects_in_fixture += 1
                                     if router.allow_syncdb(using, obj.object.__class__):
                                         loaded_objects_in_fixture += 1
                                         models.add(obj.object.__class__)
                                         obj.save(using=using)
+                                        
+                                # If we disabled foreign key checks, then we should re-enable them and check for
+                                # any invalid keys that might have been added
+                                if foreign_key_checks_disabled:
+                                    connection.enable_foreign_key_checks()
+                                    for model in models:
+                                        connection.check_for_invalid_foreign_keys(table_name=model._meta.db_table)
+                                    
                                 loaded_object_count += loaded_objects_in_fixture
                                 fixture_object_count += objects_in_fixture
                                 label_found = True

tests/modeltests/serializers/tests.py

@@ -5 +5 @@
 
 from django.conf import settings
 from django.core import serializers
-from django.db import transaction
+from django.db import transaction, connection
 from django.test import TestCase, TransactionTestCase, Approximate
 from django.utils import simplejson, unittest
 
@@ -252 +252 @@
         transaction.enter_transaction_management()
         transaction.managed(True)
         objs = serializers.deserialize(self.serializer_name, self.fwd_ref_str)
+        connection.disable_foreign_key_checks()
         for obj in objs:
             obj.save()
+        connection.enable_foreign_key_checks()
         transaction.commit()
         transaction.leave_transaction_management()
 

tests/regressiontests/serializers_regress/tests.py

@@ -383 +383 @@
     objects = []
     instance_count = {}
     for (func, pk, klass, datum) in test_data:
+        connection.disable_foreign_key_checks()
         objects.extend(func[0](pk, klass, datum))
+        connection.enable_foreign_key_checks()
 
     # Get a count of the number of objects created for each class
     for klass in instance_count:

tests/regressiontests/introspection/tests.py

@@ -96 +96 @@
             # That's {field_index: (field_index_other_table, other_table)}
             self.assertEqual(relations, {3: (0, Reporter._meta.db_table)})
 
+    def test_get_key_columns(self):
+        cursor = connection.cursor()
+        key_columns = connection.introspection.get_key_columns(cursor, Article._meta.db_table)
+        self.assertEqual(key_columns, [(u'reporter_id', Reporter._meta.db_table, u'id')])
+
     def test_get_indexes(self):
         cursor = connection.cursor()
         indexes = connection.introspection.get_indexes(cursor, Article._meta.db_table)

tests/regressiontests/fixtures_regress/fixtures/forward_ref_bad_data.json

@@ +1 @@
+[
+    {
+        "pk": 1,
+        "model": "fixtures_regress.book",
+        "fields": {
+            "name": "Cryptonomicon",
+            "author": 3
+        }
+    },
+    {
+        "pk": "4",
+        "model": "fixtures_regress.person",
+        "fields": {
+            "name": "Neal Stephenson"
+        }
+    }
+]
+ No newline at end of file

tests/regressiontests/fixtures_regress/fixtures/forward_ref.json

@@ +1 @@
+[
+    {
+        "pk": 1,
+        "model": "fixtures_regress.book",
+        "fields": {
+            "name": "Cryptonomicon",
+            "author": 4
+        }
+    },
+    {
+        "pk": "4",
+        "model": "fixtures_regress.person",
+        "fields": {
+            "name": "Neal Stephenson"
+        }
+    }
+]
+ No newline at end of file

tests/regressiontests/fixtures_regress/tests.py

@@ -12 +12 @@
 from django.core.management.commands.dumpdata import sort_dependencies
 from django.core.management.base import CommandError
 from django.db.models import signals
-from django.db import transaction
+from django.db import transaction, IntegrityError
 from django.test import TestCase, TransactionTestCase, skipIfDBFeature, \
     skipUnlessDBFeature
 
@@ -362 +362 @@
             """[{"pk": %d, "model": "fixtures_regress.widget", "fields": {"name": "grommet"}}]"""
             % widget.pk
             )
+    
+    def test_loaddata_works_when_fixture_has_forward_refs(self):
+        """
+        Regression for #3615 - Forward references cause fixtures not to load in MySQL (InnoDB)
+        """
+        management.call_command(
+            'loaddata',
+            'forward_ref.json',
+            verbosity=0,
+            commit=False
+        )
+        self.assertEqual(Book.objects.all()[0].id, 1)
+        self.assertEqual(Person.objects.all()[0].id, 4)
+    
+    def test_loaddata_raise_when_fixture_has_forward_refs(self):
+        """
+        Regression for #3615 - Ensure data with nonexistent child key references raises error
+        """
+        stderr = StringIO()
+        management.call_command(
+            'loaddata',
+            'forward_ref_bad_data.json',
+            verbosity=0,
+            commit=False,
+            stderr=stderr,
+        )
+        self.assertTrue(
+            stderr.getvalue().startswith('Problem installing fixture')
+        )
 
 
 class NaturalKeyFixtureTests(TestCase):