From 658bcc16f1b814b3a063d3fa16fabaea8b471863 Mon Sep 17 00:00:00 2001 From: Tom Forbes Date: Thu, 5 Nov 2020 12:18:20 +0100 Subject: [PATCH] Fixed #25791 -- Implement autoreload behaviour for cached template loader. --- django/template/__init__.py | 3 + django/template/autoreload.py | 50 ++++++++++++ django/utils/autoreload.py | 20 ++++- django/utils/translation/reloader.py | 8 +- docs/releases/3.2.txt | 3 + tests/template_tests/test_autoreloader.py | 92 +++++++++++++++++++++++ tests/utils_tests/test_autoreload.py | 22 ++++++ 7 files changed, 191 insertions(+), 7 deletions(-) create mode 100644 django/template/autoreload.py create mode 100644 tests/template_tests/test_autoreloader.py diff --git a/django/template/__init__.py b/django/template/__init__.py index e5ac524a5a..7414d0fef5 100644 --- a/django/template/__init__.py +++ b/django/template/__init__.py @@ -64,5 +64,8 @@ from .base import ( # NOQA i # Library management from .library import Library # NOQA isort:skip +# Import the .autoreload module to trigger the registrations of signals. +from . import autoreload # NOQA isort:skip + __all__ += ('Template', 'Context', 'RequestContext') diff --git a/django/template/autoreload.py b/django/template/autoreload.py new file mode 100644 index 0000000000..36952ef9aa --- /dev/null +++ b/django/template/autoreload.py @@ -0,0 +1,50 @@ +from django.dispatch import receiver +from django.template import engines +from django.template.backends.django import DjangoTemplates +from django.utils.autoreload import ( + autoreload_started, file_changed, is_django_path, +) + + +def get_template_directories(): + # Iterate through each template backend and find + # any template_loader that has a 'get_dirs' method. + # Collect the directories, filtering out Django templates. + items = set() + for backend in engines.all(): + if not isinstance(backend, DjangoTemplates): + continue + + items.update(backend.engine.dirs) + + for loader in backend.engine.template_loaders: + if not hasattr(loader, 'get_dirs'): + continue + items.update( + directory + for directory in loader.get_dirs() + if not is_django_path(directory) + ) + return items + + +def reset_loaders(): + for backend in engines.all(): + if not isinstance(backend, DjangoTemplates): + continue + for loader in backend.engine.template_loaders: + loader.reset() + + +@receiver(autoreload_started, dispatch_uid='template_loaders_watch_changes') +def watch_for_template_changes(sender, **kwargs): + for directory in get_template_directories(): + sender.watch_dir(directory, '**/*') + + +@receiver(file_changed, dispatch_uid='template_loaders_file_changed') +def template_changed(sender, file_path, **kwargs): + for template_dir in get_template_directories(): + if template_dir in file_path.parents: + reset_loaders() + return True diff --git a/django/utils/autoreload.py b/django/utils/autoreload.py index 442ece1f6d..bb5bdf107e 100644 --- a/django/utils/autoreload.py +++ b/django/utils/autoreload.py @@ -14,6 +14,7 @@ from pathlib import Path from types import ModuleType from zipimport import zipimporter +import django from django.apps import apps from django.core.signals import request_finished from django.dispatch import Signal @@ -45,6 +46,16 @@ except ImportError: pywatchman = None +def is_django_module(module): + """Return True if the given module is nested under Django.""" + return module.__name__.startswith('django.') + + +def is_django_path(path): + """Return True if the given file path is nested under Django.""" + return Path(django.__file__).parent in Path(path).parents + + def check_errors(fn): @functools.wraps(fn) def wrapper(*args, **kwargs): @@ -431,8 +442,15 @@ class WatchmanReloader(BaseReloader): def _subscribe(self, directory, name, expression): root, rel_path = self._watch_root(directory) + # Only receive notifications of files changing, filtering out other types + # like special files: https://facebook.github.io/watchman/docs/type + only_files_expression = [ + 'allof', + ['anyof', ['type', 'f'], ['type', 'l']], + expression + ] query = { - 'expression': expression, + 'expression': only_files_expression, 'fields': ['name'], 'since': self._get_clock(root), 'dedup_results': True, diff --git a/django/utils/translation/reloader.py b/django/utils/translation/reloader.py index 695f769e5f..d8afa89270 100644 --- a/django/utils/translation/reloader.py +++ b/django/utils/translation/reloader.py @@ -3,11 +3,7 @@ from pathlib import Path from asgiref.local import Local from django.apps import apps - - -def _is_django_module(module): - """Return True if the given module is nested under Django.""" - return module.__name__.startswith('django.') +from django.utils.autoreload import is_django_module def watch_for_translation_changes(sender, **kwargs): @@ -19,7 +15,7 @@ def watch_for_translation_changes(sender, **kwargs): directories.extend( Path(config.path) / 'locale' for config in apps.get_app_configs() - if not _is_django_module(config.module) + if not is_django_module(config.module) ) directories.extend(Path(p) for p in settings.LOCALE_PATHS) for path in directories: diff --git a/docs/releases/3.2.txt b/docs/releases/3.2.txt index f684486687..a460f96eef 100644 --- a/docs/releases/3.2.txt +++ b/docs/releases/3.2.txt @@ -394,6 +394,9 @@ Templates * :tfilter:`floatformat` template filter now allows using the ``g`` suffix to force grouping by the :setting:`THOUSAND_SEPARATOR` for the active locale. +* Templates cached with :ref:`Cached template loaders` are + now correctly reloaded in development. + Tests ~~~~~ diff --git a/tests/template_tests/test_autoreloader.py b/tests/template_tests/test_autoreloader.py new file mode 100644 index 0000000000..7af6729b38 --- /dev/null +++ b/tests/template_tests/test_autoreloader.py @@ -0,0 +1,92 @@ +from pathlib import Path +from unittest import mock + +from django.template import autoreload +from django.test import SimpleTestCase, override_settings +from django.test.utils import require_jinja2 + +ROOT = Path(__file__).parent.absolute() +EXTRA_TEMPLATES_DIR = ROOT / "templates_extra" + + +@override_settings( + INSTALLED_APPS=['template_tests'], + TEMPLATES=[{ + 'BACKEND': 'django.template.backends.dummy.TemplateStrings', + 'APP_DIRS': True, + }, { + 'BACKEND': 'django.template.backends.django.DjangoTemplates', + 'DIRS': [EXTRA_TEMPLATES_DIR], + 'OPTIONS': { + 'context_processors': [ + 'django.template.context_processors.request', + ], + 'loaders': [ + 'django.template.loaders.filesystem.Loader', + 'django.template.loaders.app_directories.Loader', + ] + }, + }]) +class TemplateReloadTests(SimpleTestCase): + @mock.patch('django.template.autoreload.reset_loaders') + def test_template_changed(self, mock_reset): + template_path = Path(__file__).parent / 'templates' / 'index.html' + self.assertTrue(autoreload.template_changed(None, template_path)) + mock_reset.assert_called_once() + + @mock.patch('django.template.autoreload.reset_loaders') + def test_non_template_changed(self, mock_reset): + self.assertIsNone(autoreload.template_changed(None, Path(__file__))) + mock_reset.assert_not_called() + + def test_watch_for_template_changes(self): + mock_reloader = mock.MagicMock() + autoreload.watch_for_template_changes(mock_reloader) + self.assertSequenceEqual( + sorted(mock_reloader.watch_dir.call_args_list), + [ + mock.call(ROOT / 'templates', '**/*'), + mock.call(ROOT / 'templates_extra', '**/*') + ] + ) + + def test_get_template_directories(self): + self.assertSetEqual( + autoreload.get_template_directories(), + { + ROOT / 'templates_extra', + ROOT / 'templates', + } + ) + + @mock.patch('django.template.loaders.base.Loader.reset') + def test_reset_all_loaders(self, mock_reset): + autoreload.reset_loaders() + self.assertEqual(mock_reset.call_count, 2) + + +@require_jinja2 +@override_settings(INSTALLED_APPS=['template_tests']) +class Jinja2TemplateReloadTests(SimpleTestCase): + def test_watch_for_template_changes(self): + mock_reloader = mock.MagicMock() + autoreload.watch_for_template_changes(mock_reloader) + self.assertSequenceEqual( + sorted(mock_reloader.watch_dir.call_args_list), + [ + mock.call(ROOT / 'templates', '**/*'), + ] + ) + + def test_get_template_directories(self): + self.assertSetEqual( + autoreload.get_template_directories(), + { + ROOT / 'templates', + } + ) + + @mock.patch('django.template.loaders.base.Loader.reset') + def test_reset_all_loaders(self, mock_reset): + autoreload.reset_loaders() + self.assertEqual(mock_reset.call_count, 0) diff --git a/tests/utils_tests/test_autoreload.py b/tests/utils_tests/test_autoreload.py index 5b1a733b47..e21414f702 100644 --- a/tests/utils_tests/test_autoreload.py +++ b/tests/utils_tests/test_autoreload.py @@ -14,6 +14,8 @@ from pathlib import Path from subprocess import CompletedProcess from unittest import mock, skip, skipIf +import pytz + import django.__main__ from django.apps.registry import Apps from django.test import SimpleTestCase @@ -201,6 +203,26 @@ class TestChildArguments(SimpleTestCase): autoreload.get_child_arguments() +class TestUtilities(SimpleTestCase): + def test_is_django_module(self): + for module, expected in ( + (pytz, False), + (sys, False), + (autoreload, True) + ): + with self.subTest(module=module): + self.assertIs(autoreload.is_django_module(module), expected) + + def test_is_django_path(self): + for module, expected in ( + (pytz.__file__, False), + (contextlib.__file__, False), + (autoreload.__file__, True) + ): + with self.subTest(module=module): + self.assertIs(autoreload.is_django_path(module), expected) + + class TestCommonRoots(SimpleTestCase): def test_common_roots(self): paths = (