Fixed #35233 -- Moved template engine system checks to backend methods.

Thanks Adam Johnson for reviews.
This commit is contained in:
Giannis Terzopoulos 2024-03-18 14:50:32 +01:00 committed by Mariusz Felisiak
parent b98271a6e4
commit d658a3162f
7 changed files with 171 additions and 183 deletions

View File

@ -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 []
)
def check_templates(app_configs, **kwargs):
"""Check all registered template engines."""
from django.template import engines
@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
@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

View File

@ -23,6 +23,9 @@ class BaseEngine:
"Unknown parameters: {}".format(", ".join(params))
)
def check(self, **kwargs):
return []
@property
def app_dirname(self):
raise ImproperlyConfigured(

View File

@ -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)

View File

@ -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}``).

View File

@ -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
~~~~~

View File

@ -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
-------------

View File

@ -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()), [])