Skip to content

Commit

Permalink
Refs #34944 -- Propagated system checks for GeneratedField.output_field.
Browse files Browse the repository at this point in the history
  • Loading branch information
felixxm committed Nov 14, 2023
1 parent 5875f03 commit c705625
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 2 deletions.
2 changes: 2 additions & 0 deletions django/db/models/fields/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,8 @@ def _check_field_name(self):
Check if field name is valid, i.e. 1) does not end with an
underscore, 2) does not contain "__" and 3) is not "pk".
"""
if self.name is None:
return []
if self.name.endswith("_"):
return [
checks.Error(
Expand Down
36 changes: 35 additions & 1 deletion django/db/models/fields/generated.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,45 @@ def generated_sql(self, connection):

def check(self, **kwargs):
databases = kwargs.get("databases") or []
return [
errors = [
*super().check(**kwargs),
*self._check_supported(databases),
*self._check_persistence(databases),
]
output_field_clone = self.output_field.clone()
output_field_clone.model = self.model
output_field_checks = output_field_clone.check(databases=databases)
if output_field_checks:
separator = "\n "
error_messages = separator.join(
f"{output_check.msg} ({output_check.id})"
for output_check in output_field_checks
if isinstance(output_check, checks.Error)
)
if error_messages:
errors.append(
checks.Error(
"GeneratedField.output_field has errors:"
f"{separator}{error_messages}",
obj=self,
id="fields.E223",
)
)
warning_messages = separator.join(
f"{output_check.msg} ({output_check.id})"
for output_check in output_field_checks
if isinstance(output_check, checks.Warning)
)
if warning_messages:
errors.append(
checks.Warning(
"GeneratedField.output_field has warnings:"
f"{separator}{warning_messages}",
obj=self,
id="fields.W224",
)
)
return errors

def _check_supported(self, databases):
errors = []
Expand Down
2 changes: 2 additions & 0 deletions docs/ref/checks.txt
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,8 @@ Model fields
``GeneratedField``\s.
* **fields.E222**: ``<database>`` does not support persisted
``GeneratedField``\s.
* **fields.E223**: ``GeneratedField.output_field`` has errors: ...
* **fields.W224**: ``GeneratedField.output_field`` has warnings: ...
* **fields.E900**: ``IPAddressField`` has been removed except for support in
historical migrations.
* **fields.W900**: ``IPAddressField`` has been deprecated. Support for it
Expand Down
78 changes: 77 additions & 1 deletion tests/invalid_models_tests/test_ordinary_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from django.core.checks import Error
from django.core.checks import Warning as DjangoWarning
from django.db import connection, models
from django.db.models.functions import Coalesce, Pi
from django.db.models.functions import Coalesce, LPad, Pi
from django.test import SimpleTestCase, TestCase, skipIfDBFeature, skipUnlessDBFeature
from django.test.utils import isolate_apps, override_settings
from django.utils.functional import lazy
Expand Down Expand Up @@ -1336,3 +1336,79 @@ class Model(models.Model):
Model._meta.get_field("field").check(databases={"default"}),
expected_errors,
)

@skipUnlessDBFeature("supports_stored_generated_columns")
def test_output_field_check_error(self):
class Model(models.Model):
value = models.DecimalField(max_digits=5, decimal_places=2)
field = models.GeneratedField(
expression=models.F("value") * 2,
output_field=models.DecimalField(max_digits=-1, decimal_places=-1),
db_persist=True,
)

expected_errors = [
Error(
"GeneratedField.output_field has errors:"
"\n 'decimal_places' must be a non-negative integer. (fields.E131)"
"\n 'max_digits' must be a positive integer. (fields.E133)",
obj=Model._meta.get_field("field"),
id="fields.E223",
),
]
self.assertEqual(
Model._meta.get_field("field").check(databases={"default"}),
expected_errors,
)

@skipUnlessDBFeature("supports_stored_generated_columns")
def test_output_field_charfield_unlimited_error(self):
class Model(models.Model):
name = models.CharField(max_length=255)
field = models.GeneratedField(
expression=LPad("name", 7, models.Value("xy")),
output_field=models.CharField(),
db_persist=True,
)

expected_errors = (
[]
if connection.features.supports_unlimited_charfield
else [
Error(
"GeneratedField.output_field has errors:"
"\n CharFields must define a 'max_length' attribute. "
"(fields.E120)",
obj=Model._meta.get_field("field"),
id="fields.E223",
),
]
)
self.assertEqual(
Model._meta.get_field("field").check(databases={"default"}),
expected_errors,
)

@skipUnlessDBFeature("supports_stored_generated_columns")
def test_output_field_check_warning(self):
class Model(models.Model):
value = models.IntegerField()
field = models.GeneratedField(
expression=models.F("value") * 2,
output_field=models.IntegerField(max_length=40),
db_persist=True,
)

expected_warnings = [
DjangoWarning(
"GeneratedField.output_field has warnings:"
"\n 'max_length' is ignored when used with IntegerField. "
"(fields.W122)",
obj=Model._meta.get_field("field"),
id="fields.W224",
),
]
self.assertEqual(
Model._meta.get_field("field").check(databases={"default"}),
expected_warnings,
)

0 comments on commit c705625

Please sign in to comment.