diff --git a/AUTHORS b/AUTHORS index 8b90f9c028..b2ea40e565 100644 --- a/AUTHORS +++ b/AUTHORS @@ -931,6 +931,7 @@ answer newbie questions, and generally made Django that much better: Wilson Miner Wim Glenn wojtek + Xavier Francisco Xia Kai Yann Fouillat Yann Malet diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 17a08fa931..f5fdaa55ee 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -4,7 +4,7 @@ from functools import partial from django import forms from django.apps import apps -from django.conf import SettingsReference +from django.conf import SettingsReference, settings from django.core import checks, exceptions from django.db import connection, router from django.db.backends import utils @@ -1436,12 +1436,23 @@ class ManyToManyField(RelatedField): clashing_obj = '%s.%s' % (opts.label, _get_field_name(model)) else: clashing_obj = model._meta.label + if settings.DATABASE_ROUTERS: + error_class, error_id = checks.Warning, 'fields.W344' + error_hint = ( + 'You have configured settings.DATABASE_ROUTERS. Verify ' + 'that the table of %r is correctly routed to a separate ' + 'database.' % clashing_obj + ) + else: + error_class, error_id = checks.Error, 'fields.E340' + error_hint = None return [ - checks.Error( + error_class( "The field's intermediary table '%s' clashes with the " "table name of '%s'." % (m2m_db_table, clashing_obj), obj=self, - id='fields.E340', + hint=error_hint, + id=error_id, ) ] return [] diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 99d5fccfaa..56a6ac3b55 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -261,6 +261,8 @@ Related fields effect as using a ``OneToOneField``. * **fields.W343**: ``limit_choices_to`` has no effect on ``ManyToManyField`` with a ``through`` model. +* **fields.W344**: The field's intermediary table ```` clashes with + the table name of ````/``.``. Models ------ diff --git a/tests/invalid_models_tests/test_models.py b/tests/invalid_models_tests/test_models.py index 0df41c2952..438c327003 100644 --- a/tests/invalid_models_tests/test_models.py +++ b/tests/invalid_models_tests/test_models.py @@ -10,6 +10,10 @@ from django.test import SimpleTestCase, TestCase from django.test.utils import isolate_apps, override_settings, register_lookup +class EmptyRouter: + pass + + def get_max_column_name_length(): allowed_len = None db_alias = None @@ -1044,6 +1048,32 @@ class OtherModelTests(SimpleTestCase): ) ]) + @override_settings(DATABASE_ROUTERS=['invalid_models_tests.test_models.EmptyRouter']) + def test_m2m_table_name_clash_database_routers_installed(self): + class Foo(models.Model): + bar = models.ManyToManyField('Bar', db_table='myapp_bar') + + class Meta: + db_table = 'myapp_foo' + + class Bar(models.Model): + class Meta: + db_table = 'myapp_bar' + + self.assertEqual(Foo.check(), [ + Warning( + "The field's intermediary table 'myapp_bar' clashes with the " + "table name of 'invalid_models_tests.Bar'.", + obj=Foo._meta.get_field('bar'), + hint=( + "You have configured settings.DATABASE_ROUTERS. Verify " + "that the table of 'invalid_models_tests.Bar' is " + "correctly routed to a separate database." + ), + id='fields.W344', + ), + ]) + def test_m2m_field_table_name_clash(self): class Foo(models.Model): pass @@ -1069,6 +1099,32 @@ class OtherModelTests(SimpleTestCase): ) ]) + @override_settings(DATABASE_ROUTERS=['invalid_models_tests.test_models.EmptyRouter']) + def test_m2m_field_table_name_clash_database_routers_installed(self): + class Foo(models.Model): + pass + + class Bar(models.Model): + foos = models.ManyToManyField(Foo, db_table='clash') + + class Baz(models.Model): + foos = models.ManyToManyField(Foo, db_table='clash') + + self.assertEqual(Bar.check() + Baz.check(), [ + Warning( + "The field's intermediary table 'clash' clashes with the " + "table name of 'invalid_models_tests.%s.foos'." + % clashing_model, + obj=model_cls._meta.get_field('foos'), + hint=( + "You have configured settings.DATABASE_ROUTERS. Verify " + "that the table of 'invalid_models_tests.%s.foos' is " + "correctly routed to a separate database." % clashing_model + ), + id='fields.W344', + ) for model_cls, clashing_model in [(Bar, 'Baz'), (Baz, 'Bar')] + ]) + def test_m2m_autogenerated_table_name_clash(self): class Foo(models.Model): class Meta: @@ -1090,6 +1146,33 @@ class OtherModelTests(SimpleTestCase): ) ]) + @override_settings(DATABASE_ROUTERS=['invalid_models_tests.test_models.EmptyRouter']) + def test_m2m_autogenerated_table_name_clash_database_routers_installed(self): + class Foo(models.Model): + class Meta: + db_table = 'bar_foos' + + class Bar(models.Model): + # The autogenerated db_table is bar_foos. + foos = models.ManyToManyField(Foo) + + class Meta: + db_table = 'bar' + + self.assertEqual(Bar.check(), [ + Warning( + "The field's intermediary table 'bar_foos' clashes with the " + "table name of 'invalid_models_tests.Foo'.", + obj=Bar._meta.get_field('foos'), + hint=( + "You have configured settings.DATABASE_ROUTERS. Verify " + "that the table of 'invalid_models_tests.Foo' is " + "correctly routed to a separate database." + ), + id='fields.W344', + ), + ]) + def test_m2m_unmanaged_shadow_models_not_checked(self): class A1(models.Model): pass