summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGiannis Terzopoulos <terzo.giannis@gmail.com>2024-03-18 14:50:32 +0100
committerMariusz Felisiak <felisiak.mariusz@gmail.com>2024-03-27 08:14:54 +0100
commitd658a3162fbeb68d148d1b2fcf4da4fe1437eddb (patch)
tree2614270a87d4c178dfca714dc48b075a75c9676b
parentb98271a6e42107233311d17f5d7bc74fbb47f22c (diff)
Fixed #35233 -- Moved template engine system checks to backend methods.
Thanks Adam Johnson for reviews.
-rw-r--r--django/core/checks/templates.py75
-rw-r--r--django/template/backends/base.py3
-rw-r--r--django/template/backends/django.py46
-rw-r--r--docs/ref/checks.txt4
-rw-r--r--docs/releases/5.1.txt3
-rw-r--r--docs/topics/checks.txt20
-rw-r--r--tests/check_framework/test_templates.py203
7 files changed, 171 insertions, 183 deletions
diff --git a/django/core/checks/templates.py b/django/core/checks/templates.py
index 681aa1f317..72a3212e56 100644
--- a/django/core/checks/templates.py
+++ b/django/core/checks/templates.py
@@ -1,75 +1,12 @@
-import copy
-from collections import defaultdict
-
-from django.conf import settings
-from django.template.backends.django import get_template_tag_modules
-
-from . import Error, Tags, Warning, register
-
-E001 = Error(
- "You have 'APP_DIRS': True in your TEMPLATES but also specify 'loaders' "
- "in OPTIONS. Either remove APP_DIRS or remove the 'loaders' option.",
- id="templates.E001",
-)
-E002 = Error(
- "'string_if_invalid' in TEMPLATES OPTIONS must be a string but got: {} ({}).",
- id="templates.E002",
-)
-W003 = Warning(
- "{} is used for multiple template tag modules: {}",
- id="templates.E003",
-)
+from . import Tags, register
@register(Tags.templates)
-def check_setting_app_dirs_loaders(app_configs, **kwargs):
- return (
- [E001]
- if any(
- conf.get("APP_DIRS") and "loaders" in conf.get("OPTIONS", {})
- for conf in settings.TEMPLATES
- )
- else []
- )
-
-
-@register(Tags.templates)
-def check_string_if_invalid_is_string(app_configs, **kwargs):
- errors = []
- for conf in settings.TEMPLATES:
- string_if_invalid = conf.get("OPTIONS", {}).get("string_if_invalid", "")
- if not isinstance(string_if_invalid, str):
- error = copy.copy(E002)
- error.msg = error.msg.format(
- string_if_invalid, type(string_if_invalid).__name__
- )
- errors.append(error)
- return errors
-
+def check_templates(app_configs, **kwargs):
+ """Check all registered template engines."""
+ from django.template import engines
-@register(Tags.templates)
-def check_for_template_tags_with_the_same_name(app_configs, **kwargs):
errors = []
- libraries = defaultdict(set)
-
- for conf in settings.TEMPLATES:
- custom_libraries = conf.get("OPTIONS", {}).get("libraries", {})
- for module_name, module_path in custom_libraries.items():
- libraries[module_name].add(module_path)
-
- for module_name, module_path in get_template_tag_modules():
- libraries[module_name].add(module_path)
-
- for library_name, items in libraries.items():
- if len(items) > 1:
- errors.append(
- Warning(
- W003.msg.format(
- repr(library_name),
- ", ".join(repr(item) for item in sorted(items)),
- ),
- id=W003.id,
- )
- )
-
+ for engine in engines.all():
+ errors.extend(engine.check())
return errors
diff --git a/django/template/backends/base.py b/django/template/backends/base.py
index 991ce64cb7..f08eb2464b 100644
--- a/django/template/backends/base.py
+++ b/django/template/backends/base.py
@@ -23,6 +23,9 @@ class BaseEngine:
"Unknown parameters: {}".format(", ".join(params))
)
+ def check(self, **kwargs):
+ return []
+
@property
def app_dirname(self):
raise ImproperlyConfigured(
diff --git a/django/template/backends/django.py b/django/template/backends/django.py
index ba561bba9f..cf6874c408 100644
--- a/django/template/backends/django.py
+++ b/django/template/backends/django.py
@@ -1,8 +1,10 @@
+from collections import defaultdict
from importlib import import_module
from pkgutil import walk_packages
from django.apps import apps
from django.conf import settings
+from django.core.checks import Error, Warning
from django.template import TemplateDoesNotExist
from django.template.context import make_context
from django.template.engine import Engine
@@ -25,6 +27,50 @@ class DjangoTemplates(BaseEngine):
super().__init__(params)
self.engine = Engine(self.dirs, self.app_dirs, **options)
+ def check(self, **kwargs):
+ return [
+ *self._check_string_if_invalid_is_string(),
+ *self._check_for_template_tags_with_the_same_name(),
+ ]
+
+ def _check_string_if_invalid_is_string(self):
+ value = self.engine.string_if_invalid
+ if not isinstance(value, str):
+ return [
+ Error(
+ "'string_if_invalid' in TEMPLATES OPTIONS must be a string but "
+ "got: %r (%s)." % (value, type(value)),
+ obj=self,
+ id="templates.E002",
+ )
+ ]
+ return []
+
+ def _check_for_template_tags_with_the_same_name(self):
+ libraries = defaultdict(set)
+
+ for module_name, module_path in get_template_tag_modules():
+ libraries[module_name].add(module_path)
+
+ for module_name, module_path in self.engine.libraries.items():
+ libraries[module_name].add(module_path)
+
+ errors = []
+
+ for library_name, items in libraries.items():
+ if len(items) > 1:
+ items = ", ".join(repr(item) for item in sorted(items))
+ errors.append(
+ Warning(
+ f"{library_name!r} is used for multiple template tag modules: "
+ f"{items}",
+ obj=self,
+ id="templates.W003",
+ )
+ )
+
+ return errors
+
def from_string(self, template_code):
return Template(self.engine.from_string(template_code), self)
diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt
index cf0ab32efa..efc8cf666a 100644
--- a/docs/ref/checks.txt
+++ b/docs/ref/checks.txt
@@ -575,7 +575,9 @@ configured:
* **templates.E001**: You have ``'APP_DIRS': True`` in your
:setting:`TEMPLATES` but also specify ``'loaders'`` in ``OPTIONS``. Either
- remove ``APP_DIRS`` or remove the ``'loaders'`` option.
+ remove ``APP_DIRS`` or remove the ``'loaders'`` option. *This check is
+ removed in Django 5.1 as system checks may now raise*
+ ``ImproperlyConfigured`` *instead.*
* **templates.E002**: ``string_if_invalid`` in :setting:`TEMPLATES`
:setting:`OPTIONS <TEMPLATES-OPTIONS>` must be a string but got: ``{value}``
(``{type}``).
diff --git a/docs/releases/5.1.txt b/docs/releases/5.1.txt
index 2dbacf08f7..fbb05d7548 100644
--- a/docs/releases/5.1.txt
+++ b/docs/releases/5.1.txt
@@ -307,6 +307,9 @@ Templates
example, to generate a link to the next page while keeping any filtering
options in place.
+* :ref:`Template engines <field-checking>` now implement a ``check()`` method
+ that is already registered with the check framework.
+
Tests
~~~~~
diff --git a/docs/topics/checks.txt b/docs/topics/checks.txt
index 3e3bbe19d6..94ba66f0db 100644
--- a/docs/topics/checks.txt
+++ b/docs/topics/checks.txt
@@ -130,18 +130,18 @@ The code below is equivalent to the code above::
.. _field-checking:
-Field, model, manager, and database checks
-------------------------------------------
+Field, model, manager, template engine, and database checks
+-----------------------------------------------------------
In some cases, you won't need to register your check function -- you can
piggyback on an existing registration.
-Fields, models, model managers, and database backends all implement a
-``check()`` method that is already registered with the check framework. If you
-want to add extra checks, you can extend the implementation on the base class,
-perform any extra checks you need, and append any messages to those generated
-by the base class. It's recommended that you delegate each check to separate
-methods.
+Fields, models, model managers, template engines, and database backends all
+implement a ``check()`` method that is already registered with the check
+framework. If you want to add extra checks, you can extend the implementation
+on the base class, perform any extra checks you need, and append any messages
+to those generated by the base class. It's recommended that you delegate each
+check to separate methods.
Consider an example where you are implementing a custom field named
``RangedIntegerField``. This field adds ``min`` and ``max`` arguments to the
@@ -195,6 +195,10 @@ the only difference is that the check is a classmethod, not an instance method::
# ... your own checks ...
return errors
+.. versionchanged:: 5.1
+
+ In older versions, template engines didn't implement a ``check()`` method.
+
Writing tests
-------------
diff --git a/tests/check_framework/test_templates.py b/tests/check_framework/test_templates.py
index c8a2f83b8a..18f705ddb8 100644
--- a/tests/check_framework/test_templates.py
+++ b/tests/check_framework/test_templates.py
@@ -1,128 +1,105 @@
-from copy import copy, deepcopy
+from copy import deepcopy
+from itertools import chain
-from django.core.checks import Warning
-from django.core.checks.templates import (
- E001,
- E002,
- W003,
- check_for_template_tags_with_the_same_name,
- check_setting_app_dirs_loaders,
- check_string_if_invalid_is_string,
-)
+from django.core.checks import Error, Warning
+from django.core.checks.templates import check_templates
+from django.template import engines
+from django.template.backends.base import BaseEngine
from django.test import SimpleTestCase
from django.test.utils import override_settings
-class CheckTemplateSettingsAppDirsTest(SimpleTestCase):
- TEMPLATES_APP_DIRS_AND_LOADERS = [
- {
- "BACKEND": "django.template.backends.django.DjangoTemplates",
- "APP_DIRS": True,
- "OPTIONS": {
- "loaders": ["django.template.loaders.filesystem.Loader"],
- },
- },
- ]
+class ErrorEngine(BaseEngine):
+ def __init__(self, params):
+ params.pop("OPTIONS")
+ super().__init__(params)
- @override_settings(TEMPLATES=TEMPLATES_APP_DIRS_AND_LOADERS)
- def test_app_dirs_and_loaders(self):
- """
- Error if template loaders are specified and APP_DIRS is True.
- """
- self.assertEqual(check_setting_app_dirs_loaders(None), [E001])
+ def check(self, **kwargs):
+ return [Error("Example")]
- def test_app_dirs_removed(self):
- TEMPLATES = deepcopy(self.TEMPLATES_APP_DIRS_AND_LOADERS)
- del TEMPLATES[0]["APP_DIRS"]
- with self.settings(TEMPLATES=TEMPLATES):
- self.assertEqual(check_setting_app_dirs_loaders(None), [])
- def test_loaders_removed(self):
- TEMPLATES = deepcopy(self.TEMPLATES_APP_DIRS_AND_LOADERS)
- del TEMPLATES[0]["OPTIONS"]["loaders"]
- with self.settings(TEMPLATES=TEMPLATES):
- self.assertEqual(check_setting_app_dirs_loaders(None), [])
+class CheckTemplatesTests(SimpleTestCase):
+ @override_settings(
+ TEMPLATES=[
+ {"BACKEND": f"{__name__}.{ErrorEngine.__qualname__}", "NAME": "backend_1"},
+ {"BACKEND": f"{__name__}.{ErrorEngine.__qualname__}", "NAME": "backend_2"},
+ ]
+ )
+ def test_errors_aggregated(self):
+ errors = check_templates(None)
+ self.assertEqual(errors, [Error("Example")] * 2)
class CheckTemplateStringIfInvalidTest(SimpleTestCase):
TEMPLATES_STRING_IF_INVALID = [
{
"BACKEND": "django.template.backends.django.DjangoTemplates",
+ "NAME": "backend_1",
"OPTIONS": {
"string_if_invalid": False,
},
},
{
"BACKEND": "django.template.backends.django.DjangoTemplates",
+ "NAME": "backend_2",
"OPTIONS": {
"string_if_invalid": 42,
},
},
]
- @classmethod
- def setUpClass(cls):
- super().setUpClass()
- cls.error1 = copy(E002)
- cls.error2 = copy(E002)
- string_if_invalid1 = cls.TEMPLATES_STRING_IF_INVALID[0]["OPTIONS"][
- "string_if_invalid"
- ]
- string_if_invalid2 = cls.TEMPLATES_STRING_IF_INVALID[1]["OPTIONS"][
- "string_if_invalid"
- ]
- cls.error1.msg = cls.error1.msg.format(
- string_if_invalid1, type(string_if_invalid1).__name__
+ def _get_error_for_engine(self, engine):
+ value = engine.engine.string_if_invalid
+ return Error(
+ "'string_if_invalid' in TEMPLATES OPTIONS must be a string but got: %r "
+ "(%s)." % (value, type(value)),
+ obj=engine,
+ id="templates.E002",
)
- cls.error2.msg = cls.error2.msg.format(
- string_if_invalid2, type(string_if_invalid2).__name__
+
+ def _check_engines(self, engines):
+ return list(
+ chain.from_iterable(e._check_string_if_invalid_is_string() for e in engines)
)
@override_settings(TEMPLATES=TEMPLATES_STRING_IF_INVALID)
def test_string_if_invalid_not_string(self):
- self.assertEqual(
- check_string_if_invalid_is_string(None), [self.error1, self.error2]
- )
+ _engines = engines.all()
+ errors = [
+ self._get_error_for_engine(_engines[0]),
+ self._get_error_for_engine(_engines[1]),
+ ]
+ self.assertEqual(self._check_engines(_engines), errors)
def test_string_if_invalid_first_is_string(self):
TEMPLATES = deepcopy(self.TEMPLATES_STRING_IF_INVALID)
TEMPLATES[0]["OPTIONS"]["string_if_invalid"] = "test"
with self.settings(TEMPLATES=TEMPLATES):
- self.assertEqual(check_string_if_invalid_is_string(None), [self.error2])
+ _engines = engines.all()
+ errors = [self._get_error_for_engine(_engines[1])]
+ self.assertEqual(self._check_engines(_engines), errors)
def test_string_if_invalid_both_are_strings(self):
TEMPLATES = deepcopy(self.TEMPLATES_STRING_IF_INVALID)
TEMPLATES[0]["OPTIONS"]["string_if_invalid"] = "test"
TEMPLATES[1]["OPTIONS"]["string_if_invalid"] = "test"
with self.settings(TEMPLATES=TEMPLATES):
- self.assertEqual(check_string_if_invalid_is_string(None), [])
+ self.assertEqual(self._check_engines(engines.all()), [])
def test_string_if_invalid_not_specified(self):
TEMPLATES = deepcopy(self.TEMPLATES_STRING_IF_INVALID)
del TEMPLATES[1]["OPTIONS"]["string_if_invalid"]
with self.settings(TEMPLATES=TEMPLATES):
- self.assertEqual(check_string_if_invalid_is_string(None), [self.error1])
+ _engines = engines.all()
+ errors = [self._get_error_for_engine(_engines[0])]
+ self.assertEqual(self._check_engines(_engines), errors)
class CheckTemplateTagLibrariesWithSameName(SimpleTestCase):
- @classmethod
- def setUpClass(cls):
- super().setUpClass()
- cls.warning_same_tags = Warning(
- W003.msg.format(
- "'same_tags'",
- "'check_framework.template_test_apps.same_tags_app_1."
- "templatetags.same_tags', "
- "'check_framework.template_test_apps.same_tags_app_2."
- "templatetags.same_tags'",
- ),
- id=W003.id,
- )
-
- @staticmethod
- def get_settings(module_name, module_path):
+ def get_settings(self, module_name, module_path, name="django"):
return {
"BACKEND": "django.template.backends.django.DjangoTemplates",
+ "NAME": name,
"OPTIONS": {
"libraries": {
module_name: f"check_framework.template_test_apps.{module_path}",
@@ -130,6 +107,20 @@ class CheckTemplateTagLibrariesWithSameName(SimpleTestCase):
},
}
+ def _get_error_for_engine(self, engine, modules):
+ return Warning(
+ f"'same_tags' is used for multiple template tag modules: {modules}",
+ obj=engine,
+ id="templates.W003",
+ )
+
+ def _check_engines(self, engines):
+ return list(
+ chain.from_iterable(
+ e._check_for_template_tags_with_the_same_name() for e in engines
+ )
+ )
+
@override_settings(
INSTALLED_APPS=[
"check_framework.template_test_apps.same_tags_app_1",
@@ -137,26 +128,32 @@ class CheckTemplateTagLibrariesWithSameName(SimpleTestCase):
]
)
def test_template_tags_with_same_name(self):
- self.assertEqual(
- check_for_template_tags_with_the_same_name(None),
- [self.warning_same_tags],
+ _engines = engines.all()
+ modules = (
+ "'check_framework.template_test_apps.same_tags_app_1.templatetags"
+ ".same_tags', 'check_framework.template_test_apps.same_tags_app_2"
+ ".templatetags.same_tags'"
)
+ errors = [self._get_error_for_engine(_engines[0], modules)]
+ self.assertEqual(self._check_engines(_engines), errors)
- def test_template_tags_with_same_library_name(self):
+ def test_template_tags_for_separate_backends(self):
+ # The "libraries" names are the same, but the backends are different.
with self.settings(
TEMPLATES=[
self.get_settings(
- "same_tags", "same_tags_app_1.templatetags.same_tags"
+ "same_tags",
+ "same_tags_app_1.templatetags.same_tags",
+ name="backend_1",
),
self.get_settings(
- "same_tags", "same_tags_app_2.templatetags.same_tags"
+ "same_tags",
+ "same_tags_app_2.templatetags.same_tags",
+ name="backend_2",
),
]
):
- self.assertEqual(
- check_for_template_tags_with_the_same_name(None),
- [self.warning_same_tags],
- )
+ self.assertEqual(self._check_engines(engines.all()), [])
@override_settings(
INSTALLED_APPS=["check_framework.template_test_apps.same_tags_app_1"]
@@ -169,48 +166,44 @@ class CheckTemplateTagLibrariesWithSameName(SimpleTestCase):
),
]
):
- self.assertEqual(check_for_template_tags_with_the_same_name(None), [])
+ self.assertEqual(self._check_engines(engines.all()), [])
@override_settings(
INSTALLED_APPS=["check_framework.template_test_apps.same_tags_app_1"]
)
def test_template_tags_with_same_library_name_and_module_name(self):
+ modules = (
+ "'check_framework.template_test_apps.different_tags_app.templatetags"
+ ".different_tags', 'check_framework.template_test_apps.same_tags_app_1"
+ ".templatetags.same_tags'"
+ )
with self.settings(
TEMPLATES=[
self.get_settings(
- "same_tags",
- "different_tags_app.templatetags.different_tags",
+ "same_tags", "different_tags_app.templatetags.different_tags"
),
]
):
- self.assertEqual(
- check_for_template_tags_with_the_same_name(None),
- [
- Warning(
- W003.msg.format(
- "'same_tags'",
- "'check_framework.template_test_apps.different_tags_app."
- "templatetags.different_tags', "
- "'check_framework.template_test_apps.same_tags_app_1."
- "templatetags.same_tags'",
- ),
- id=W003.id,
- )
- ],
- )
+ _engines = engines.all()
+ errors = [self._get_error_for_engine(_engines[0], modules)]
+ self.assertEqual(self._check_engines(_engines), errors)
def test_template_tags_with_different_library_name(self):
with self.settings(
TEMPLATES=[
self.get_settings(
- "same_tags", "same_tags_app_1.templatetags.same_tags"
+ "same_tags",
+ "same_tags_app_1.templatetags.same_tags",
+ name="backend_1",
),
self.get_settings(
- "not_same_tags", "same_tags_app_2.templatetags.same_tags"
+ "not_same_tags",
+ "same_tags_app_2.templatetags.same_tags",
+ name="backend_2",
),
]
):
- self.assertEqual(check_for_template_tags_with_the_same_name(None), [])
+ self.assertEqual(self._check_engines(engines.all()), [])
@override_settings(
INSTALLED_APPS=[
@@ -219,4 +212,4 @@ class CheckTemplateTagLibrariesWithSameName(SimpleTestCase):
]
)
def test_template_tags_with_different_name(self):
- self.assertEqual(check_for_template_tags_with_the_same_name(None), [])
+ self.assertEqual(self._check_engines(engines.all()), [])