Skip to content

Commit

Permalink
Fixed #35233 -- Moved template engine system checks to backend methods.
Browse files Browse the repository at this point in the history
Thanks Adam Johnson for reviews.
  • Loading branch information
g-nie authored and felixxm committed Mar 27, 2024
1 parent b98271a commit d658a31
Show file tree
Hide file tree
Showing 7 changed files with 172 additions and 184 deletions.
75 changes: 6 additions & 69 deletions django/core/checks/templates.py
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions django/template/backends/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ def __init__(self, params):
"Unknown parameters: {}".format(", ".join(params))
)

def check(self, **kwargs):
return []

@property
def app_dirname(self):
raise ImproperlyConfigured(
Expand Down
46 changes: 46 additions & 0 deletions django/template/backends/django.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -25,6 +27,50 @@ def __init__(self, params):
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)

Expand Down
4 changes: 3 additions & 1 deletion docs/ref/checks.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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}``).
Expand Down
3 changes: 3 additions & 0 deletions docs/releases/5.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
~~~~~

Expand Down
20 changes: 12 additions & 8 deletions docs/topics/checks.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
-------------

Expand Down
Loading

0 comments on commit d658a31

Please sign in to comment.