From abfa4619fb818ff694c22e962a280673e085239e Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Wed, 12 Nov 2025 19:47:16 +0000 Subject: [PATCH 1/4] Fixed #36730 -- Fixed constraint validation crash for excluded FK attnames. Regression in e44e8327d3d88d86895735c0e427102063ff5b55. Co-authored-by: Mariusz Felisiak --- django/db/models/constraints.py | 7 +++++-- tests/model_forms/models.py | 19 +++++++++++++++++++ tests/model_forms/tests.py | 15 +++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/django/db/models/constraints.py b/django/db/models/constraints.py index 73ab23bdfa94..93251ff322e3 100644 --- a/django/db/models/constraints.py +++ b/django/db/models/constraints.py @@ -51,9 +51,12 @@ def remove_sql(self, model, schema_editor): def _expression_refs_exclude(cls, model, expression, exclude): get_field = model._meta.get_field for field_name, *__ in model._get_expr_references(expression): - if field_name in exclude: + if field_name == "pk": + field = model._meta.pk + else: + field = get_field(field_name) + if field_name in exclude or field.name in exclude: return True - field = get_field(field_name) if field.generated and cls._expression_refs_exclude( model, field.expression, exclude ): diff --git a/tests/model_forms/models.py b/tests/model_forms/models.py index f6c34a35218a..83daa13d71f0 100644 --- a/tests/model_forms/models.py +++ b/tests/model_forms/models.py @@ -543,3 +543,22 @@ class Meta: violation_error_message="Price must be greater than zero.", ), ] + + +class AttnameConstraintsModel(models.Model): + left = models.ForeignKey( + "self", related_name="+", null=True, on_delete=models.SET_NULL + ) + right = models.ForeignKey( + "self", related_name="+", null=True, on_delete=models.SET_NULL + ) + + class Meta: + required_db_features = {"supports_table_check_constraints"} + constraints = [ + models.CheckConstraint( + name="%(app_label)s_%(class)s_left_not_right", + # right_id here is the ForeignKey's attname, not name. + condition=~models.Q(left=models.F("right_id")), + ), + ] diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index f0334e1e8699..129ce56c7ac1 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -30,6 +30,7 @@ from .models import ( Article, ArticleStatus, + AttnameConstraintsModel, Author, Author1, Award, @@ -3766,3 +3767,17 @@ def test_check_constraint_refs_excluded_field(self): self.assertEqual( full_form.errors, {"__all__": ["Price must be greater than zero."]} ) + + def test_check_constraint_refs_excluded_field_attname(self): + left = AttnameConstraintsModel.objects.create() + instance = AttnameConstraintsModel.objects.create(left=left) + data = { + "left": str(left.id), + "right": "", + } + AttnameConstraintsModelForm = modelform_factory( + AttnameConstraintsModel, fields="__all__" + ) + full_form = AttnameConstraintsModelForm(data, instance=instance) + self.assertFalse(full_form.is_valid()) + self.assertEqual(full_form.errors, {"right": ["This field is required."]}) From 1b539ef27e776bd6112e8f22e653feca5279c4fb Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sat, 8 Nov 2025 15:26:36 -0500 Subject: [PATCH 2/4] Refs #31055 -- Augmented regression tests for database system checks. We might want to change this in the future but it should be further discussed first. --- tests/check_framework/tests.py | 16 ++++++++++++++-- tests/migrations/test_commands.py | 21 ++++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/tests/check_framework/tests.py b/tests/check_framework/tests.py index f926308fd78e..c5f53c4789ce 100644 --- a/tests/check_framework/tests.py +++ b/tests/check_framework/tests.py @@ -1,11 +1,11 @@ import multiprocessing import sys from io import StringIO -from unittest import skipIf +from unittest import mock, skipIf from django.apps import apps from django.core import checks -from django.core.checks import Error, Warning +from django.core.checks import Error, Tags, Warning from django.core.checks.messages import CheckMessage from django.core.checks.registry import CheckRegistry from django.core.management import call_command @@ -268,6 +268,18 @@ def test_fail_level(self): with self.assertRaises(CommandError): call_command("check", fail_level="WARNING") + def test_database_system_checks(self): + database_check = mock.Mock(return_value=[], tags=[Tags.database]) + + with override_system_checks([database_check]): + call_command("check") + database_check.assert_called_once_with(app_configs=None, databases=None) + database_check.reset_mock() + call_command("check", databases=["default"]) + database_check.assert_called_once_with( + app_configs=None, databases=["default"] + ) + def custom_error_system_check(app_configs, **kwargs): return [Error("Error", id="myerrorcheck.E001")] diff --git a/tests/migrations/test_commands.py b/tests/migrations/test_commands.py index b5817081d28f..83c5575fd3a8 100644 --- a/tests/migrations/test_commands.py +++ b/tests/migrations/test_commands.py @@ -113,7 +113,7 @@ def test_migrate_with_system_checks(self, mocked_check): out = io.StringIO() call_command("migrate", skip_checks=False, no_color=True, stdout=out) self.assertIn("Apply all migrations: migrated_app", out.getvalue()) - mocked_check.assert_called_once() + mocked_check.assert_called_once_with(databases=["default"]) def test_migrate_with_custom_system_checks(self): original_checks = registry.registered_checks.copy() @@ -137,6 +137,25 @@ class CustomMigrateCommandWithSecurityChecks(MigrateCommand): command = CustomMigrateCommandWithSecurityChecks() call_command(command, skip_checks=False, stdout=io.StringIO()) + @override_settings( + INSTALLED_APPS=[ + "django.contrib.auth", + "django.contrib.contenttypes", + "migrations.migrations_test_apps.migrated_app", + ] + ) + def test_migrate_runs_database_system_checks(self): + original_checks = registry.registered_checks.copy() + self.addCleanup(setattr, registry, "registered_checks", original_checks) + + out = io.StringIO() + mock_check = mock.Mock(return_value=[]) + register(mock_check, Tags.database) + + call_command("migrate", skip_checks=False, no_color=True, stdout=out) + self.assertIn("Apply all migrations: migrated_app", out.getvalue()) + mock_check.assert_called_once_with(app_configs=None, databases=["default"]) + @override_settings( INSTALLED_APPS=[ "migrations", From 96ee097bd68a93e0f360326271ebe78553fae15b Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sat, 8 Nov 2025 16:40:26 -0500 Subject: [PATCH 3/4] Refs #31055 -- Adjusted passing of Field.check kwargs to ArrayField.base_field. This was missed when Field.check(databases) was introduced. --- django/contrib/postgres/fields/array.py | 2 +- tests/postgres_tests/test_array.py | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/django/contrib/postgres/fields/array.py b/django/contrib/postgres/fields/array.py index a76598a9bff4..f867571215f2 100644 --- a/django/contrib/postgres/fields/array.py +++ b/django/contrib/postgres/fields/array.py @@ -70,7 +70,7 @@ def check(self, **kwargs): ) ) else: - base_checks = self.base_field.check() + base_checks = self.base_field.check(**kwargs) if base_checks: error_messages = "\n ".join( "%s (%s)" % (base_check.msg, base_check.id) diff --git a/tests/postgres_tests/test_array.py b/tests/postgres_tests/test_array.py index f35211e8ed75..d4289d79dfb3 100644 --- a/tests/postgres_tests/test_array.py +++ b/tests/postgres_tests/test_array.py @@ -841,6 +841,25 @@ class MyModel(PostgreSQLModel): self.assertEqual(errors[0].id, "postgres.E001") self.assertIn("max_length", errors[0].msg) + def test_base_field_check_kwargs(self): + passed_kwargs = None + + class MyField(models.Field): + def check(self, **kwargs): + nonlocal passed_kwargs + passed_kwargs = kwargs + return [] + + class MyModel(PostgreSQLModel): + field = ArrayField(MyField()) + + self.assertEqual(MyModel.check(databases=["default"]), []) + self.assertEqual( + passed_kwargs, + {"databases": ["default"]}, + "ArrayField.check kwargs should be passed to its base_field.", + ) + def test_invalid_base_fields(self): class MyModel(PostgreSQLModel): field = ArrayField( From 3aba1fced8254435b947467739721ec6b4fb865c Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Mon, 29 Sep 2025 17:47:56 -0400 Subject: [PATCH 4/4] Fixed #36663 -- Defaulted to running checks against all databases. Regression in 0b83c8cc4db95812f1e15ca19d78614e94cf38dd which added support for running checks only against databases but also defaulted to running against no databases if not specified. Note that this continues to *not* default to runing database tagged checks for all management commands except the migrate one as whether or not we should change this must be discussed further. Thanks Tim Graham for surfacing how this was a problematic default. --- django/core/checks/registry.py | 9 +++++++++ tests/check_framework/tests.py | 37 +++++++++++++++++++++++++++------- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/django/core/checks/registry.py b/django/core/checks/registry.py index 3139fc3ef4d9..257dadac41b6 100644 --- a/django/core/checks/registry.py +++ b/django/core/checks/registry.py @@ -1,6 +1,7 @@ from collections.abc import Iterable from itertools import chain +from django.db import connections from django.utils.inspect import func_accepts_kwargs @@ -84,6 +85,14 @@ def run_checks( if tags is not None: checks = [check for check in checks if not set(check.tags).isdisjoint(tags)] + elif not databases: + # By default, 'database'-tagged checks are not run if an alias + # is not explicitly specified as they do more than mere static + # code analysis. + checks = [check for check in checks if Tags.database not in check.tags] + + if databases is None: + databases = list(connections) for check in checks: new_errors = check(app_configs=app_configs, databases=databases) diff --git a/tests/check_framework/tests.py b/tests/check_framework/tests.py index c5f53c4789ce..138d27aafddd 100644 --- a/tests/check_framework/tests.py +++ b/tests/check_framework/tests.py @@ -92,6 +92,21 @@ def return_non_iterable(**kwargs): with self.assertRaisesMessage(TypeError, msg): registry.run_checks() + def test_run_checks_database_exclusion(self): + registry = CheckRegistry() + + database_errors = [checks.Warning("Database Check")] + + @registry.register(Tags.database) + def database_system_check(**kwargs): + return database_errors + + errors = registry.run_checks() + self.assertEqual(errors, []) + + errors = registry.run_checks(databases=["default"]) + self.assertEqual(errors, database_errors) + class MessageTests(SimpleTestCase): def test_printing(self): @@ -190,10 +205,12 @@ def tearDown(self): def test_simple_call(self): call_command("check") self.assertEqual( - simple_system_check.kwargs, {"app_configs": None, "databases": None} + simple_system_check.kwargs, + {"app_configs": None, "databases": ["default", "other"]}, ) self.assertEqual( - tagged_system_check.kwargs, {"app_configs": None, "databases": None} + tagged_system_check.kwargs, + {"app_configs": None, "databases": ["default", "other"]}, ) @override_system_checks([simple_system_check, tagged_system_check]) @@ -203,11 +220,17 @@ def test_given_app(self): admin_config = apps.get_app_config("admin") self.assertEqual( simple_system_check.kwargs, - {"app_configs": [auth_config, admin_config], "databases": None}, + { + "app_configs": [auth_config, admin_config], + "databases": ["default", "other"], + }, ) self.assertEqual( tagged_system_check.kwargs, - {"app_configs": [auth_config, admin_config], "databases": None}, + { + "app_configs": [auth_config, admin_config], + "databases": ["default", "other"], + }, ) @override_system_checks([simple_system_check, tagged_system_check]) @@ -215,7 +238,8 @@ def test_given_tag(self): call_command("check", tags=["simpletag"]) self.assertIsNone(simple_system_check.kwargs) self.assertEqual( - tagged_system_check.kwargs, {"app_configs": None, "databases": None} + tagged_system_check.kwargs, + {"app_configs": None, "databases": ["default", "other"]}, ) @override_system_checks([simple_system_check, tagged_system_check]) @@ -273,8 +297,7 @@ def test_database_system_checks(self): with override_system_checks([database_check]): call_command("check") - database_check.assert_called_once_with(app_configs=None, databases=None) - database_check.reset_mock() + database_check.assert_not_called() call_command("check", databases=["default"]) database_check.assert_called_once_with( app_configs=None, databases=["default"]