Skip to content

Commit 51e70a1

Browse files
authored
Merge pull request #341 from bjester/make-deferrable-pls
Add deferrable FK utility, test case, and migration
2 parents 03bbfc2 + 4de5f43 commit 51e70a1

5 files changed

Lines changed: 251 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
List of the most important changes for each release.
44

5+
## 0.8.14
6+
- Adds utility for addressing immediate FK constraints caused by Django upgrade, automatically performed for morango models in a Django migration.
7+
58
## 0.8.13
69
- Removes multiprocessing fallback for RSA key generation to avoid leaving zombie processes; key generation now stays in-process
710

morango/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
__version__ = "0.8.13"
1+
__version__ = "0.8.14"

morango/deferrable_foreign_keys.py

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
"""
2+
Helpers for ensuring that existing SQLite foreign key constraints are
3+
``DEFERRABLE INITIALLY DEFERRED``.
4+
5+
Prior to Django 3.1, SQLite tables were created with immediate foreign key constraints, e.g.::
6+
7+
"store_model_id" char(32) NOT NULL REFERENCES "morango_store" ("id")
8+
9+
Since Django 3.1 the same column is created as::
10+
11+
"store_model_id" char(32) NOT NULL REFERENCES "morango_store" ("id") DEFERRABLE INITIALLY DEFERRED
12+
13+
Django relies on deferred constraint checking for correct cascade-deletion ordering. Databases
14+
created before Morango 0.8 (with Django 1.11) therefore retain immediate constraints, which can
15+
raise ``IntegrityError: FOREIGN KEY constraint failed`` during operations such as sync
16+
deserialization once foreign key enforcement is enabled at the database level (the default since
17+
Django 3.2).
18+
19+
The migration framework does not regenerate these constraints on its own because the deferrable
20+
clause is not part of the field definition that migrations track. This module rebuilds the affected
21+
tables so that a migrated database ends up with the same schema as a freshly created one.
22+
"""
23+
import logging
24+
from typing import List
25+
from typing import Optional
26+
27+
from django.apps import apps as global_apps
28+
from django.apps.registry import Apps
29+
from django.db import connection
30+
from django.db import DatabaseError
31+
from django.db import router
32+
from django.db.backends.base.schema import BaseDatabaseSchemaEditor
33+
from django.db.backends.sqlite3.schema import DatabaseSchemaEditor
34+
35+
logger = logging.getLogger(__name__)
36+
37+
38+
def _get_table_sql(conn, table_name: str) -> Optional[str]:
39+
"""
40+
Return the ``CREATE TABLE`` statement stored by SQLite for ``table_name``, or ``None`` if the
41+
table does not exist.
42+
"""
43+
with conn.cursor() as cursor:
44+
cursor.execute(
45+
"SELECT sql FROM sqlite_master WHERE type = 'table' AND name = %s",
46+
[table_name],
47+
)
48+
row = cursor.fetchone()
49+
return row[0] if row else None
50+
51+
52+
def _table_has_immediate_foreign_key(conn, table_name: str) -> bool:
53+
"""
54+
Determine whether ``table_name`` has at least one foreign key constraint that is not deferrable.
55+
Tables are created with all constraints in the same style, so the presence of a ``REFERENCES``
56+
clause without ``DEFERRABLE`` indicates an immediate constraint that needs to be rebuilt.
57+
"""
58+
sql = _get_table_sql(conn, table_name)
59+
if not sql:
60+
return False
61+
references_count = sql.count("REFERENCES")
62+
if references_count == 0:
63+
return False
64+
# make sure that all FK columns (identified by references) have a deferrable constraint
65+
return references_count != sql.count("DEFERRABLE")
66+
67+
68+
class MakeForeignKeysDeferrable:
69+
"""
70+
Invokable utility class that rebuilds of any table belonging to one of ``*app_labels`` whose
71+
foreign key constraints are still immediate, so that they become
72+
``DEFERRABLE INITIALLY DEFERRED``.
73+
74+
This can be utilized by passing this class to ``RunPython`` in a Django migration, or by
75+
manually invoking its ``run`` method.
76+
77+
This is a no-op on non-SQLite backends (PostgreSQL already creates deferrable
78+
constraints) and on tables that already use deferrable constraints, so it is
79+
safe to apply to any database.
80+
"""
81+
82+
def __init__(
83+
self,
84+
include_app_labels: Optional[List[str]] = None,
85+
exclude_app_labels: Optional[List[str]] = None
86+
):
87+
self.include_app_labels = include_app_labels
88+
self.exclude_app_labels = exclude_app_labels
89+
90+
def _iter_apps(self, apps: Apps):
91+
for app_config in apps.get_app_configs():
92+
if self.exclude_app_labels is not None and app_config.label in self.exclude_app_labels:
93+
continue
94+
if self.include_app_labels is None or app_config.label in self.include_app_labels:
95+
yield app_config
96+
97+
def __call__(self, apps: Apps, schema_editor: BaseDatabaseSchemaEditor):
98+
"""
99+
Loops through all relevant django apps and their models, checking if the model's table has
100+
FKs, and if so, runs the schema editor's utility that synchronizes the schema to the model,
101+
which will make FKs deferrable
102+
"""
103+
conn = schema_editor.connection
104+
# these should be in sync, but since we call `_remake_table`, specific to this schema editor
105+
# class, we ensure we have the correct instance to begin with
106+
if conn.vendor != "sqlite" or not isinstance(schema_editor, DatabaseSchemaEditor):
107+
return
108+
109+
for app_config in self._iter_apps(apps):
110+
for model in app_config.get_models(include_auto_created=True):
111+
if not router.allow_migrate_model(conn.alias, model):
112+
continue
113+
table_name = model._meta.db_table
114+
if not _table_has_immediate_foreign_key(conn, table_name):
115+
continue
116+
logger.info(
117+
"Rebuilding table %s to make foreign key constraints deferrable",
118+
table_name,
119+
)
120+
try:
121+
# Rebuilding the table from the current model state regenerates the column
122+
# definitions, which SQLite always emits with the deferrable clause (see
123+
# DatabaseSchemaEditor.sql_create_inline_fk). Only tested with Django 3.2
124+
schema_editor._remake_table(model)
125+
except DatabaseError as e:
126+
logger.error("Failed to rebuild table %s", table_name)
127+
logger.exception(e)
128+
129+
def run(self):
130+
"""
131+
Invoke this utility outside of Django migrations, manually passing in Django's app registry
132+
and schema editor.
133+
"""
134+
# since this method is meant for manual invocation, we let the editor create a transaction
135+
with connection.schema_editor(atomic=True) as schema_editor:
136+
self.__call__(global_apps, schema_editor)
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
"""
2+
Make existing foreign key constraints ``DEFERRABLE INITIALLY DEFERRED``.
3+
4+
Databases created before Morango 0.8 (with Django 1.11) have immediate SQLite foreign key
5+
constraints, which can break cascade deletions (e.g. during sync) now that foreign keys are enforced
6+
at the database level. See https://github.com/learningequality/kolibri/issues/14884.
7+
"""
8+
from django.db import migrations
9+
10+
from morango.deferrable_foreign_keys import MakeForeignKeysDeferrable
11+
12+
13+
class Migration(migrations.Migration):
14+
15+
dependencies = [
16+
("morango", "0003_store_deserialization_errors"),
17+
]
18+
19+
operations = [
20+
migrations.RunPython(
21+
MakeForeignKeysDeferrable(include_app_labels=["morango"]),
22+
migrations.RunPython.noop,
23+
),
24+
]
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import pytest
2+
from django.apps import apps as global_apps
3+
from django.conf import settings
4+
from django.db import connection
5+
from django.test import TransactionTestCase
6+
from facility_profile.models import Facility
7+
8+
from morango.deferrable_foreign_keys import _get_table_sql
9+
from morango.deferrable_foreign_keys import MakeForeignKeysDeferrable
10+
11+
12+
BASE_APPS = [
13+
'django.contrib.admin',
14+
'django.contrib.auth',
15+
'django.contrib.contenttypes',
16+
'django.contrib.sessions',
17+
'django.contrib.messages',
18+
'django.contrib.staticfiles',
19+
'rest_framework',
20+
]
21+
22+
23+
class MakeDeferrableForeignKeysTestCase(TransactionTestCase):
24+
def _rewrite_model_fk_immediate(self):
25+
table = Facility._meta.db_table
26+
sql = _get_table_sql(connection, table)
27+
immediate = sql.replace(" DEFERRABLE INITIALLY DEFERRED", "")
28+
assert "DEFERRABLE" not in immediate
29+
with connection.cursor() as cursor:
30+
cursor.execute(
31+
"SELECT sql FROM sqlite_master WHERE type='index' AND tbl_name=%s AND sql IS NOT NULL",
32+
[table],
33+
)
34+
indexes = [r[0] for r in cursor.fetchall()]
35+
cursor.execute("PRAGMA foreign_keys = OFF")
36+
cursor.execute("DROP TABLE {}".format(connection.ops.quote_name(table)))
37+
cursor.execute(immediate)
38+
for idx in indexes:
39+
cursor.execute(idx)
40+
cursor.execute("PRAGMA foreign_keys = ON")
41+
42+
@pytest.mark.skipif(
43+
getattr(settings, "MORANGO_TEST_POSTGRESQL", False), reason="SQLite only"
44+
)
45+
def test_remake_makes_deferrable_and_preserves_data(self):
46+
table = Facility._meta.db_table
47+
# fresh schema should be deferrable
48+
self.assertIn("DEFERRABLE", _get_table_sql(connection, table))
49+
50+
# downgrade schema to immediate FK (table is rebuilt empty)
51+
self._rewrite_model_fk_immediate()
52+
self.assertNotIn("DEFERRABLE", _get_table_sql(connection, table))
53+
54+
# create some data on the immediate-FK schema
55+
f = Facility.objects.create(name="testfac")
56+
f_id = f.id
57+
self.assertTrue(Facility.objects.filter(id=f_id).exists())
58+
59+
# run the helper with the real app registry + a schema editor
60+
op = MakeForeignKeysDeferrable(include_app_labels=["facility_profile"])
61+
with connection.schema_editor(atomic=False) as schema_editor:
62+
op(global_apps, schema_editor)
63+
64+
# now deferrable again, and data preserved
65+
self.assertIn("DEFERRABLE", _get_table_sql(connection, table))
66+
self.assertTrue(Facility.objects.filter(id=f_id).exists())
67+
68+
def test_iter_apps__all(self):
69+
op = MakeForeignKeysDeferrable()
70+
self.assertEqual([app.name for app in op._iter_apps(global_apps)], [
71+
*BASE_APPS,
72+
'morango',
73+
'facility_profile'
74+
])
75+
76+
def test_iter_apps__exclude(self):
77+
op = MakeForeignKeysDeferrable(exclude_app_labels=['facility_profile'])
78+
self.assertEqual([app.name for app in op._iter_apps(global_apps)], [
79+
*BASE_APPS,
80+
'morango',
81+
])
82+
83+
def test_iter_apps__include(self):
84+
op = MakeForeignKeysDeferrable(include_app_labels=['morango'])
85+
self.assertEqual([app.name for app in op._iter_apps(global_apps)], [
86+
'morango',
87+
])

0 commit comments

Comments
 (0)