From 6e222dae5636f875c19ec66f730a4241abe33faa Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sun, 6 Nov 2016 13:52:07 +0100 Subject: [PATCH] Fixed #27453 -- Avoided unnecessary recompilation of non-translated URL regexes. --- django/urls/resolvers.py | 49 ++++++++++++------- .../test_localeregexprovider.py | 16 ++++++ 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/django/urls/resolvers.py b/django/urls/resolvers.py index 3c4f18c991..0bb5645dc1 100644 --- a/django/urls/resolvers.py +++ b/django/urls/resolvers.py @@ -79,6 +79,37 @@ def get_ns_resolver(ns_pattern, resolver): return RegexURLResolver(r'^/', [ns_resolver]) +class LocaleRegexDescriptor(object): + def __get__(self, instance, cls=None): + """ + Return a compiled regular expression based on the active language. + """ + if instance is None: + return self + # As a performance optimization, if the given regex string is a regular + # string (not a lazily-translated string proxy), compile it once and + # avoid per-language compilation. + if isinstance(instance._regex, six.string_types): + instance.__dict__['regex'] = self._compile(instance._regex) + return instance.__dict__['regex'] + language_code = get_language() + if language_code not in instance._regex_dict: + instance._regex_dict[language_code] = self._compile(force_text(instance._regex)) + return instance._regex_dict[language_code] + + def _compile(self, regex): + """ + Compile and return the given regular expression. + """ + try: + return re.compile(regex, re.UNICODE) + except re.error as e: + raise ImproperlyConfigured( + '"%s" is not a valid regular expression: %s' % + (regex, six.text_type(e)) + ) + + class LocaleRegexProvider(object): """ A mixin to provide a default regex property which can vary by active @@ -91,23 +122,7 @@ class LocaleRegexProvider(object): self._regex = regex self._regex_dict = {} - @property - def regex(self): - """ - Return a compiled regular expression based on the activate language. - """ - language_code = get_language() - if language_code not in self._regex_dict: - regex = self._regex if isinstance(self._regex, six.string_types) else force_text(self._regex) - try: - compiled_regex = re.compile(regex, re.UNICODE) - except re.error as e: - raise ImproperlyConfigured( - '"%s" is not a valid regular expression: %s' % - (regex, six.text_type(e)) - ) - self._regex_dict[language_code] = compiled_regex - return self._regex_dict[language_code] + regex = LocaleRegexDescriptor() def describe(self): """ diff --git a/tests/urlpatterns_reverse/test_localeregexprovider.py b/tests/urlpatterns_reverse/test_localeregexprovider.py index 9da8df656c..401e9a1ad0 100644 --- a/tests/urlpatterns_reverse/test_localeregexprovider.py +++ b/tests/urlpatterns_reverse/test_localeregexprovider.py @@ -5,6 +5,7 @@ import os from django.core.exceptions import ImproperlyConfigured from django.test import SimpleTestCase, mock, override_settings from django.urls import LocaleRegexProvider +from django.urls.resolvers import LocaleRegexDescriptor from django.utils import translation from django.utils._os import upath @@ -33,9 +34,24 @@ class LocaleRegexProviderTests(SimpleTestCase): self.assertEqual(de_compiled.pattern, '^foo-de/$') self.assertEqual(de_compiled, de_compiled_2) + def test_nontranslated_regex_compiled_once(self): + provider = LocaleRegexProvider('^foo/$') + with translation.override('de'): + de_compiled = provider.regex + with translation.override('fr'): + # compiled only once, regardless of language + error = AssertionError('tried to compile non-translated url regex twice') + with mock.patch('django.urls.resolvers.re.compile', side_effect=error): + fr_compiled = provider.regex + self.assertEqual(de_compiled.pattern, '^foo/$') + self.assertEqual(fr_compiled.pattern, '^foo/$') + def test_regex_compile_error(self): """Regex errors are re-raised as ImproperlyConfigured.""" provider = LocaleRegexProvider('*') msg = '"*" is not a valid regular expression: nothing to repeat' with self.assertRaisesMessage(ImproperlyConfigured, msg): provider.regex + + def test_access_locale_regex_descriptor(self): + self.assertIsInstance(LocaleRegexProvider.regex, LocaleRegexDescriptor)