8000 Fixed #32770 -- Added system check to ensure `django.contrib.postgres` in `INSTALLED_APPS` when using its features. by cliff688 · Pull Request #19480 · django/django · GitHub
[go: up one dir, main page]

Skip to content

Fixed #32770 -- Added system check to ensure django.contrib.postgres in INSTALLED_APPS when using its features. #19480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions django/contrib/postgres/constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,16 @@
from django.db.models.lookups import PostgresOperatorLookup
from django.db.models.sql import Query

from .utils import CheckPostgresInstalledMixin

__all__ = ["ExclusionConstraint"]


class ExclusionConstraintExpression(IndexExpression):
template = "%(expressions)s WITH %(operator)s"


class ExclusionConstraint(BaseConstraint):
class ExclusionConstraint(CheckPostgresInstalledMixin, BaseConstraint):
template = (
"CONSTRAINT %(name)s EXCLUDE USING %(index_type)s "
"(%(expressions)s)%(include)s%(where)s%(deferrable)s"
Expand Down Expand Up @@ -77,12 +79,14 @@ def _get_expressions(self, schema_editor, query):
return ExpressionList(* 10000 expressions).resolve_expression(query)

def check(self, model, connection):
errors = super().check(model, connection)
references = set()
for expr, _ in self.expressions:
if isinstance(expr, str):
expr = F(expr)
references.update(model._get_expr_references(expr))
return self._check_references(model, references)
errors.extend(self._check_references(model, references))
return errors

def _get_condition_sql(self, compiler, schema_editor, query):
if self.condition is None:
Expand Down
6 changes: 4 additions & 2 deletions django/contrib/postgres/fields/array.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
from django.db.models.lookups import Exact, In
from django.utils.translation import gettext_lazy as _

from ..utils import prefix_validation_error
from ..utils import CheckPostgresInstalledMixin, prefix_validation_error
from .utils import AttributeSetter

__all__ = ["ArrayField"]


class ArrayField(CheckFieldDefaultMixin, Field):
class ArrayField(CheckPostgresInstalledMixin, CheckFieldDefaultMixin, Field):
empty_strings_allowed = False
default_error_messages = {
"item_invalid": _("Item %(nth)s in the array did not validate:"),
Expand Down Expand Up @@ -73,6 +73,8 @@ def check(self, **kwargs):
"%s (%s)" % (base_check.msg, base_check.id)
for base_check in base_checks
if isinstance(base_check, checks.Error)
# Prevent duplication of E005 in an E001 check.
and not base_check.id == "postgres.E005"
)
if error_messages:
errors.append(
Expand Down
4 changes: 3 additions & 1 deletion django/contrib/postgres/fields/hstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
from django.db.models.fields.mixins import CheckFieldDefaultMixin
from django.utils.translation import gettext_lazy as _

from ..utils import CheckPostgresInstalledMixin

__all__ = ["HStoreField"]


class HStoreField(CheckFieldDefaultMixin, Field):
class HStoreField(CheckPostgresInstalledMixin, CheckFieldDefaultMixin, Field):
empty_strings_allowed = False
description = _("Map of strings to strings/nulls")
default_error_messages = {
Expand Down
3 changes: 2 additions & 1 deletion django/contrib/postgres/fields/ranges.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from django.db.models.functions import Cast
from django.db.models.lookups import PostgresOperatorLookup

from ..utils import CheckPostgresInstalledMixin
from .utils import AttributeSetter

__all__ = [
Expand Down Expand Up @@ -51,7 +52,7 @@ class RangeOperators:
ADJACENT_TO = "-|-"


class RangeField(models.Field):
class RangeField(CheckPostgresInstalledMixin, models.Field):
empty_strings_allowed = False

def __init__(self, *args, **kwargs):
Expand Down
4 changes: 3 additions & 1 deletion django/contrib/postgres/indexes.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from django.db.models import Func, Index
from django.utils.functional import cached_property

from .utils import CheckPostgresInstalledMixin

__all__ = [
"BloomIndex",
"BrinIndex",
Expand All @@ -12,7 +14,7 @@
]


class PostgresIndex(Index):
class PostgresIndex(CheckPostgresInstalledMixin, Index):
@cached_property
def max_name_length(self):
# Allow an index name longer than 30 characters when the suffix is
Expand Down
4 changes: 3 additions & 1 deletion django/contrib/postgres/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from django.db.models.expressions import CombinedExpression, register_combinable_fields
from django.db.models.functions import Cast, Coalesce

from .utils import CheckPostgresInstalledMixin


class SearchVectorExact(Lookup):
lookup_name = "exact"
Expand All @@ -29,7 +31,7 @@ def as_sql(self, qn, connection):
return "%s @@ %s" % (lhs, rhs), params


class SearchVectorField(Field):
class SearchVectorField(CheckPostgresInstalledMixin, Field):
def db_type(self, connection):
return "tsvector"

Expand Down
24 changes: 24 additions & 0 deletions django/contrib/postgres/utils.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from django.apps import apps
from django.core import checks
from django.core.exceptions import ValidationError
from django.utils.functional import SimpleLazyObject
from django.utils.text import format_lazy
Expand Down Expand Up @@ -27,3 +29,25 @@ def prefix_validation_error(error, prefix, code, params):
return ValidationError(
[prefix_validation_error(e, prefix, code, params) for e in error.error_list]
)


class CheckPostgresInstalledMixin:
def _check_postgres_installed(self, *args):
# When subclassed by Index or BaseConstraint subclasses, args is
# (model, connection).
obj = args[0] if args else self
if not apps.is_installed("django.contrib.postgres"):
return [
checks.Error(
"'django.contrib.postgres' must be in INSTALLED_APPS in "
"order to use %s." % self.__class__.__name__,
obj=obj,
id="postgres.E005",
)
]
return []

def check(self, *args, **kwargs):
errors = super().check(*args, **kwargs)
errors.extend(self._check_postgres_installed(*args))
return errors
4 changes: 3 additions & 1 deletion docs/ref/checks.txt
Original file line number Diff line number Diff line change
Expand Up @@ -912,14 +912,16 @@ The following checks are performed when a model contains a
------------

The following checks are performed on :mod:`django.contrib.postgres` model
fields:
fields, indexes, and constraints:

* **postgres.E001**: Base field for array has errors: ...
* **postgres.E002**: Base field for array cannot be a related field.
* **postgres.E003**: ``<field>`` default should be a callable instead of an
instance so that it's not shared between all field instances. *This check was
changed to* ``fields.E010`` *in Django 3.1*.
* **postgres.W004**: Base field for array has warnings: ...
* **postgres.E005**: 'django.contrib.postgres' must be in ``INSTALLED_APPS`` in
order to use ``<django.contrib.postgres feature>``.

.. _sites-system-checks:

Expand Down
4 changes: 3 additions & 1 deletion docs/releases/6.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ Minor features
:mod:`django.contrib.postgres`
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

* ...
* Model fields, indexes, and constraints from :mod:`django.contrib.postgres`
now include system checks to verify that ``django.contrib.postgres`` is an
installed app.

:mod:`django.contrib.redirects`
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down
3 changes: 2 additions & 1 deletion tests/invalid_models_tests/test_deprecated_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from django.core import checks
from django.db import connection, models
from django.test import SimpleTestCase
from django.test.utils import isolate_apps
from django.test.utils import isolate_apps, modify_settings


@isolate_apps("invalid_models_tests")
Expand Down Expand Up @@ -87,6 +87,7 @@ class PostgresJSONFieldModel(models.Model):
)

@skipUnless(connection.vendor == "postgresql", "PostgreSQL specific SQL")
@modify_settings(INSTALLED_APPS={"append": "django.contrib.postgres"})
def test_postgres_ci_fields_deprecated(self):
from django.contrib.postgres.fields import (
ArrayField,
Expand Down
138 changes: 138 additions & 0 deletions tests/postgres_tests/test_app_installed_check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
from django.core import checks
from django.db import models
from django.test import modify_settings
from django.test.utils import isolate_apps

from . import PostgreSQLTestCase
from .fields import (
BigIntegerRangeField,
DateRangeField,
DateTimeRangeField,
DecimalRangeField,
HStoreField,
IntegerRangeField,
SearchVectorField,
)
from .models import IntegerArrayModel, NestedIntegerArrayModel, PostgreSQLModel

try:
from django.contrib.postgres.constraints import ExclusionConstraint
from django.contrib.postgres.fields.ranges import RangeOperators
from django.contrib.postgres.indexes import GinIndex, PostgresIndex
except ImportError:
pass


@isolate_apps("postgres_tests")
class TestPostgresAppInstalledCheck(PostgreSQLTestCase):

def _make_error(self, obj, klass_name):
"""Helper to create postgres.E005 error for specific objects."""
return checks.Error(
"'django.contrib.postgres' must be in INSTALLED_APPS in order to "
f"use {klass_name}.",
obj=obj,
id="postgres.E005",
)

def assert_model_check_errors(self, model_class, expected_errors):
errors = model_class.check(databases=self.databases)
self.assertEqual(errors, [])
with modify_settings(INSTALLED_APPS={"remove": "django.contrib.postgres"}):
errors = model_class.check(databases=self.databases)
self.assertEqual(errors, expected_errors)

def test_indexes(self):
class IndexModel(PostgreSQLModel):
field = models.IntegerField()

class Meta:
indexes = [
PostgresIndex(fields=["id"], name="postgres_index_test"),
GinIndex(fields=["field"], name="gin_index_test"),
]

self.assert_model_check_errors(
IndexModel,
[
self._make_error(IndexModel, "PostgresIndex"),
self._make_error(IndexModel, "GinIndex"),
],
)

def test_exclusion_constraint(self):
class ExclusionModel(PostgreSQLModel):
value = models.IntegerField()

class Meta:
constraints = [
ExclusionConstraint(
name="exclude_equal",
expressions=[("value", RangeOperators.EQUAL)],
)
]

self.assert_model_check_errors(
ExclusionModel, [self._make_error(ExclusionModel, "ExclusionConstraint")]
)

def test_array_field(self):
field = IntegerArrayModel._meta.get_field("field")
self.assert_model_check_errors(
IntegerArrayModel,
[self._make_error(field, "ArrayField")],
)

def test_nested_array_field(self):
"""Inner ArrayField does not cause a postgres.E001 error."""
field = NestedIntegerArrayModel._meta.get_field("field")
self.assert_model_check_errors(
NestedIntegerArrayModel,
[
self._make_error(field, "ArrayField"),
],
)

def test_hstore_field(self):
class HStoreFieldModel(PostgreSQLModel):
field = HStoreField()

field = HStoreFieldModel._meta.get_field("field")
self.assert_model_check_errors(
HStoreFieldModel,
[
self._make_error(field, "HStoreField"),
],
)

def test_range_fields(self):
class RangeFieldsModel(PostgreSQLModel):
int_range = IntegerRangeField()
bigint_range = BigIntegerRangeField()
decimal_range = DecimalRangeField()
datetime_range = DateTimeRangeField()
date_range = DateRangeField()

expected_errors = [
self._make_error(field, field.__class__.__name__)
for field in [
RangeFieldsModel._meta.get_field("int_range"),
RangeFieldsModel._meta.get_field("bigint_range"),
RangeFieldsModel._meta.get_field("decimal_range"),
RangeFieldsModel._meta.get_field("datetime_range"),
RangeFieldsModel._meta.get_field("date_range"),
]
]
self.assert_model_check_errors(RangeFieldsModel, expected_errors)

def test_search_vector_field(self):
class SearchModel(PostgreSQLModel):
search_field = SearchVectorField()

field = SearchModel._meta.get_field("search_field")
self.assert_model_check_errors(
SearchModel,
[
self._make_error(field, "SearchVectorField"),
],
)
1 change: 1 addition & 0 deletions tests/runtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ def setup_collect_tests(start_at, start_after, test_labels=None):
settings.LOGGING = log_config
settings.SILENCED_SYSTEM_CHECKS = [
"fields.W342", # ForeignKey(unique=True) -> OneToOneField
"postgres.E005", # django.contrib.postgres must be installed to use feature.
]

# Load all the ALWAYS_INSTALLED_APPS.
Expand Down
0