Fixed #30673 -- Relaxed system check for db_table collision when database routers are installed by turning the error into a warning.

This commit is contained in:
Adnan Umer 2019-08-03 16:22:27 +05:00 committed by Mariusz Felisiak
parent 65e86948b8
commit 8d3519071e
5 changed files with 80 additions and 7 deletions

View File

@ -18,6 +18,7 @@ answer newbie questions, and generally made Django that much better:
Adam Malinowski <https://adammalinowski.co.uk/> Adam Malinowski <https://adammalinowski.co.uk/>
Adam Vandenberg Adam Vandenberg
Adiyat Mubarak <adiyatmubarak@gmail.com> Adiyat Mubarak <adiyatmubarak@gmail.com>
Adnan Umer <u.adnan@outlook.com>
Adrian Holovaty <adrian@holovaty.com> Adrian Holovaty <adrian@holovaty.com>
Adrien Lemaire <lemaire.adrien@gmail.com> Adrien Lemaire <lemaire.adrien@gmail.com>
Afonso Fernández Nogueira <fonzzo.django@gmail.com> Afonso Fernández Nogueira <fonzzo.django@gmail.com>

View File

@ -4,7 +4,8 @@ from collections import defaultdict
from itertools import chain from itertools import chain
from django.apps import apps from django.apps import apps
from django.core.checks import Error, Tags, register from django.conf import settings
from django.core.checks import Error, Tags, Warning, register
@register(Tags.models) @register(Tags.models)
@ -35,14 +36,25 @@ def check_all_models(app_configs=None, **kwargs):
indexes[model_index.name].append(model._meta.label) indexes[model_index.name].append(model._meta.label)
for model_constraint in model._meta.constraints: for model_constraint in model._meta.constraints:
constraints[model_constraint.name].append(model._meta.label) constraints[model_constraint.name].append(model._meta.label)
if settings.DATABASE_ROUTERS:
error_class, error_id = Warning, 'models.W035'
error_hint = (
'You have configured settings.DATABASE_ROUTERS. Verify that %s '
'are correctly routed to separate databases.'
)
else:
error_class, error_id = Error, 'models.E028'
error_hint = None
for db_table, model_labels in db_table_models.items(): for db_table, model_labels in db_table_models.items():
if len(model_labels) != 1: if len(model_labels) != 1:
model_labels_str = ', '.join(model_labels)
errors.append( errors.append(
Error( error_class(
"db_table '%s' is used by multiple models: %s." "db_table '%s' is used by multiple models: %s."
% (db_table, ', '.join(db_table_models[db_table])), % (db_table, model_labels_str),
obj=db_table, obj=db_table,
id='models.E028', hint=(error_hint % model_labels_str) if error_hint else None,
id=error_id,
) )
) )
for index_name, model_labels in indexes.items(): for index_name, model_labels in indexes.items():

View File

@ -317,6 +317,8 @@ Models
or a number. or a number.
* **models.E034**: The index name ``<index>`` cannot be longer than * **models.E034**: The index name ``<index>`` cannot be longer than
``<max_length>`` characters. ``<max_length>`` characters.
* **models.W035**: ``db_table`` ``<db_table>`` is used by multiple models:
``<model list>``.
Security Security
-------- --------

View File

@ -9,4 +9,6 @@ Django 2.2.5 fixes several bugs in 2.2.4.
Bugfixes Bugfixes
======== ========
* ... * Relaxed the system check added in Django 2.2 for models to reallow use of the
same ``db_table`` by multiple models when database routers are installed
(:ticket:`30673`).

View File

@ -1,12 +1,16 @@
from django.core import checks from django.core import checks
from django.core.checks import Error from django.core.checks import Error, Warning
from django.db import models from django.db import models
from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature
from django.test.utils import ( from django.test.utils import (
isolate_apps, modify_settings, override_system_checks, isolate_apps, modify_settings, override_settings, override_system_checks,
) )
class EmptyRouter:
pass
@isolate_apps('check_framework', attr_name='apps') @isolate_apps('check_framework', attr_name='apps')
@override_system_checks([checks.model_checks.check_all_models]) @override_system_checks([checks.model_checks.check_all_models])
class DuplicateDBTableTests(SimpleTestCase): class DuplicateDBTableTests(SimpleTestCase):
@ -28,6 +32,30 @@ class DuplicateDBTableTests(SimpleTestCase):
) )
]) ])
@override_settings(DATABASE_ROUTERS=['check_framework.test_model_checks.EmptyRouter'])
def test_collision_in_same_app_database_routers_installed(self):
class Model1(models.Model):
class Meta:
db_table = 'test_table'
class Model2(models.Model):
class Meta:
db_table = 'test_table'
self.assertEqual(checks.run_checks(app_configs=self.apps.get_app_configs()), [
Warning(
"db_table 'test_table' is used by multiple models: "
"check_framework.Model1, check_framework.Model2.",
hint=(
'You have configured settings.DATABASE_ROUTERS. Verify '
'that check_framework.Model1, check_framework.Model2 are '
'correctly routed to separate databases.'
),
obj='test_table',
id='models.W035',
)
])
@modify_settings(INSTALLED_APPS={'append': 'basic'}) @modify_settings(INSTALLED_APPS={'append': 'basic'})
@isolate_apps('basic', 'check_framework', kwarg_name='apps') @isolate_apps('basic', 'check_framework', kwarg_name='apps')
def test_collision_across_apps(self, apps): def test_collision_across_apps(self, apps):
@ -50,6 +78,34 @@ class DuplicateDBTableTests(SimpleTestCase):
) )
]) ])
@modify_settings(INSTALLED_APPS={'append': 'basic'})
@override_settings(DATABASE_ROUTERS=['check_framework.test_model_checks.EmptyRouter'])
@isolate_apps('basic', 'check_framework', kwarg_name='apps')
def test_collision_across_apps_database_routers_installed(self, apps):
class Model1(models.Model):
class Meta:
app_label = 'basic'
db_table = 'test_table'
class Model2(models.Model):
class Meta:
app_label = 'check_framework'
db_table = 'test_table'
self.assertEqual(checks.run_checks(app_configs=apps.get_app_configs()), [
Warning(
"db_table 'test_table' is used by multiple models: "
"basic.Model1, check_framework.Model2.",
hint=(
'You have configured settings.DATABASE_ROUTERS. Verify '
'that basic.Model1, check_framework.Model2 are correctly '
'routed to separate databases.'
),
obj='test_table',
id='models.W035',
)
])
def test_no_collision_for_unmanaged_models(self): def test_no_collision_for_unmanaged_models(self):
class Unmanaged(models.Model): class Unmanaged(models.Model):
class Meta: class Meta: