From 5d25804eaf81795c7d457e5a2a9f0b9b0989136c Mon Sep 17 00:00:00 2001 From: Sanyam Khurana Date: Thu, 13 Dec 2018 01:41:50 +0530 Subject: [PATCH] Fixed #20098 -- Added a check for model Meta.db_table collisions. --- django/core/checks/model_checks.py | 14 ++++ docs/ref/checks.txt | 2 + tests/check_framework/test_model_checks.py | 75 ++++++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 tests/check_framework/test_model_checks.py diff --git a/django/core/checks/model_checks.py b/django/core/checks/model_checks.py index 2397fe3bb2..8514fb4d12 100644 --- a/django/core/checks/model_checks.py +++ b/django/core/checks/model_checks.py @@ -1,5 +1,6 @@ import inspect import types +from collections import defaultdict from itertools import chain from django.apps import apps @@ -8,12 +9,15 @@ from django.core.checks import Error, Tags, register @register(Tags.models) def check_all_models(app_configs=None, **kwargs): + db_table_models = defaultdict(list) errors = [] if app_configs is None: models = apps.get_models() else: models = chain.from_iterable(app_config.get_models() for app_config in app_configs) for model in models: + if model._meta.managed and not model._meta.proxy: + db_table_models[model._meta.db_table].append(model._meta.label) if not inspect.ismethod(model.check): errors.append( Error( @@ -25,6 +29,16 @@ def check_all_models(app_configs=None, **kwargs): ) else: errors.extend(model.check(**kwargs)) + for db_table, model_labels in db_table_models.items(): + if len(model_labels) != 1: + errors.append( + Error( + "db_table '%s' is used by multiple models: %s." + % (db_table, ', '.join(db_table_models[db_table])), + obj=db_table, + id='models.E028', + ) + ) return errors diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index acd860065f..6ec3028c35 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -302,6 +302,8 @@ Models * **models.E026**: The model cannot have more than one field with ``primary_key=True``. * **models.W027**: ```` does not support check constraints. +* **models.E028**: ``db_table`` ```` is used by multiple models: + ````. Security -------- diff --git a/tests/check_framework/test_model_checks.py b/tests/check_framework/test_model_checks.py new file mode 100644 index 0000000000..2e55ad637d --- /dev/null +++ b/tests/check_framework/test_model_checks.py @@ -0,0 +1,75 @@ +from django.core import checks +from django.core.checks import Error +from django.db import models +from django.test import SimpleTestCase +from django.test.utils import ( + isolate_apps, modify_settings, override_system_checks, +) + + +@isolate_apps('check_framework', attr_name='apps') +@override_system_checks([checks.model_checks.check_all_models]) +class DuplicateDBTableTests(SimpleTestCase): + def test_collision_in_same_app(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()), [ + Error( + "db_table 'test_table' is used by multiple models: " + "check_framework.Model1, check_framework.Model2.", + obj='test_table', + id='models.E028', + ) + ]) + + @modify_settings(INSTALLED_APPS={'append': 'basic'}) + @isolate_apps('basic', 'check_framework', kwarg_name='apps') + def test_collision_across_apps(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()), [ + Error( + "db_table 'test_table' is used by multiple models: " + "basic.Model1, check_framework.Model2.", + obj='test_table', + id='models.E028', + ) + ]) + + def test_no_collision_for_unmanaged_models(self): + class Unmanaged(models.Model): + class Meta: + db_table = 'test_table' + managed = False + + class Managed(models.Model): + class Meta: + db_table = 'test_table' + + self.assertEqual(checks.run_checks(app_configs=self.apps.get_app_configs()), []) + + def test_no_collision_for_proxy_models(self): + class Model(models.Model): + class Meta: + db_table = 'test_table' + + class ProxyModel(Model): + class Meta: + proxy = True + + self.assertEqual(Model._meta.db_table, ProxyModel._meta.db_table) + self.assertEqual(checks.run_checks(app_configs=self.apps.get_app_configs()), [])