From c2293e9f251b1f215825186a7bcbf5a006df0cb0 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 29 May 2019 21:32:03 +0300 Subject: [PATCH] Improve performance of lazy validation message formatting (#6709) --- rest_framework/compat.py | 30 ------------------ rest_framework/fields.py | 51 ++++++++++-------------------- rest_framework/utils/formatting.py | 26 +++++++++++++++ tests/test_utils.py | 19 +++++++++++ 4 files changed, 62 insertions(+), 64 deletions(-) diff --git a/rest_framework/compat.py b/rest_framework/compat.py index 2e6a8adda..4bee21f91 100644 --- a/rest_framework/compat.py +++ b/rest_framework/compat.py @@ -5,7 +5,6 @@ versions of Django/Python, and compatibility wrappers around optional packages. import sys from django.conf import settings -from django.core import validators from django.views.generic import View try: @@ -238,34 +237,5 @@ LONG_SEPARATORS = (', ', ': ') INDENT_SEPARATORS = (',', ': ') -class CustomValidatorMessage: - """ - We need to avoid evaluation of `lazy` translated `message` in `django.core.validators.BaseValidator.__init__`. - https://github.com/django/django/blob/75ed5900321d170debef4ac452b8b3cf8a1c2384/django/core/validators.py#L297 - - Ref: https://github.com/encode/django-rest-framework/pull/5452 - """ - - def __init__(self, *args, **kwargs): - self.message = kwargs.pop('message', self.message) - super().__init__(*args, **kwargs) - - -class MinValueValidator(CustomValidatorMessage, validators.MinValueValidator): - pass - - -class MaxValueValidator(CustomValidatorMessage, validators.MaxValueValidator): - pass - - -class MinLengthValidator(CustomValidatorMessage, validators.MinLengthValidator): - pass - - -class MaxLengthValidator(CustomValidatorMessage, validators.MaxLengthValidator): - pass - - # Version Constants. PY36 = sys.version_info >= (3, 6) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 7da3d2b8e..a68cd6ecc 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -12,7 +12,8 @@ from django.conf import settings from django.core.exceptions import ObjectDoesNotExist from django.core.exceptions import ValidationError as DjangoValidationError from django.core.validators import ( - EmailValidator, RegexValidator, URLValidator, ip_address_validators + EmailValidator, MaxLengthValidator, MaxValueValidator, MinLengthValidator, + MinValueValidator, RegexValidator, URLValidator, ip_address_validators ) from django.forms import FilePathField as DjangoFilePathField from django.forms import ImageField as DjangoImageField @@ -23,20 +24,17 @@ from django.utils.dateparse import ( from django.utils.duration import duration_string from django.utils.encoding import is_protected_type, smart_text from django.utils.formats import localize_input, sanitize_separators -from django.utils.functional import lazy from django.utils.ipv6 import clean_ipv6_address from django.utils.timezone import utc from django.utils.translation import gettext_lazy as _ from pytz.exceptions import InvalidTimeError from rest_framework import ISO_8601 -from rest_framework.compat import ( - MaxLengthValidator, MaxValueValidator, MinLengthValidator, - MinValueValidator, ProhibitNullCharactersValidator -) +from rest_framework.compat import ProhibitNullCharactersValidator from rest_framework.exceptions import ErrorDetail, ValidationError from rest_framework.settings import api_settings from rest_framework.utils import html, humanize_datetime, json, representation +from rest_framework.utils.formatting import lazy_format class empty: @@ -749,12 +747,11 @@ class CharField(Field): self.min_length = kwargs.pop('min_length', None) super().__init__(**kwargs) if self.max_length is not None: - message = lazy(self.error_messages['max_length'].format, str)(max_length=self.max_length) + message = lazy_format(self.error_messages['max_length'], max_length=self.max_length) self.validators.append( MaxLengthValidator(self.max_length, message=message)) if self.min_length is not None: - message = lazy( - self.error_messages['min_length'].format, str)(min_length=self.min_length) + message = lazy_format(self.error_messages['min_length'], min_length=self.min_length) self.validators.append( MinLengthValidator(self.min_length, message=message)) @@ -915,13 +912,11 @@ class IntegerField(Field): self.min_value = kwargs.pop('min_value', None) super().__init__(**kwargs) if self.max_value is not None: - message = lazy( - self.error_messages['max_value'].format, str)(max_value=self.max_value) + message = lazy_format(self.error_messages['max_value'], max_value=self.max_value) self.validators.append( MaxValueValidator(self.max_value, message=message)) if self.min_value is not None: - message = lazy( - self.error_messages['min_value'].format, str)(min_value=self.min_value) + message = lazy_format(self.error_messages['min_value'], min_value=self.min_value) self.validators.append( MinValueValidator(self.min_value, message=message)) @@ -953,15 +948,11 @@ class FloatField(Field): self.min_value = kwargs.pop('min_value', None) super().__init__(**kwargs) if self.max_value is not None: - message = lazy( - self.error_messages['max_value'].format, - str)(max_value=self.max_value) + message = lazy_format(self.error_messages['max_value'], max_value=self.max_value) self.validators.append( MaxValueValidator(self.max_value, message=message)) if self.min_value is not None: - message = lazy( - self.error_messages['min_value'].format, - str)(min_value=self.min_value) + message = lazy_format(self.error_messages['min_value'], min_value=self.min_value) self.validators.append( MinValueValidator(self.min_value, message=message)) @@ -1012,14 +1003,11 @@ class DecimalField(Field): super().__init__(**kwargs) if self.max_value is not None: - message = lazy( - self.error_messages['max_value'].format, - str)(max_value=self.max_value) + message = lazy_format(self.error_messages['max_value'], max_value=self.max_value) self.validators.append( MaxValueValidator(self.max_value, message=message)) if self.min_value is not None: - message = lazy( - self.error_messages['min_value'].format, str)(min_value=self.min_value) + message = lazy_format(self.error_messages['min_value'], min_value=self.min_value) self.validators.append( MinValueValidator(self.min_value, message=message)) @@ -1357,15 +1345,11 @@ class DurationField(Field): self.min_value = kwargs.pop('min_value', None) super().__init__(**kwargs) if self.max_value is not None: - message = lazy( - self.error_messages['max_value'].format, - str)(max_value=self.max_value) + message = lazy_format(self.error_messages['max_value'], max_value=self.max_value) self.validators.append( MaxValueValidator(self.max_value, message=message)) if self.min_value is not None: - message = lazy( - self.error_messages['min_value'].format, - str)(min_value=self.min_value) + message = lazy_format(self.error_messages['min_value'], min_value=self.min_value) self.validators.append( MinValueValidator(self.min_value, message=message)) @@ -1610,10 +1594,10 @@ class ListField(Field): super().__init__(*args, **kwargs) self.child.bind(field_name='', parent=self) if self.max_length is not None: - message = lazy(self.error_messages['max_length'].format, str)(max_length=self.max_length) + message = lazy_format(self.error_messages['max_length'], max_length=self.max_length) self.validators.append(MaxLengthValidator(self.max_length, message=message)) if self.min_length is not None: - message = lazy(self.error_messages['min_length'].format, str)(min_length=self.min_length) + message = lazy_format(self.error_messages['min_length'], min_length=self.min_length) self.validators.append(MinLengthValidator(self.min_length, message=message)) def get_value(self, dictionary): @@ -1887,8 +1871,7 @@ class ModelField(Field): max_length = kwargs.pop('max_length', None) super().__init__(**kwargs) if max_length is not None: - message = lazy( - self.error_messages['max_length'].format, str)(max_length=self.max_length) + message = lazy_format(self.error_messages['max_length'], max_length=self.max_length) self.validators.append( MaxLengthValidator(self.max_length, message=message)) diff --git a/rest_framework/utils/formatting.py b/rest_framework/utils/formatting.py index 4e003f614..e96cc6719 100644 --- a/rest_framework/utils/formatting.py +++ b/rest_framework/utils/formatting.py @@ -65,3 +65,29 @@ def markup_description(description): description = escape(description).replace('\n', '
') description = '

' + description + '

' return mark_safe(description) + + +class lazy_format: + """ + Delay formatting until it's actually needed. + + Useful when the format string or one of the arguments is lazy. + + Not using Django's lazy because it is too slow. + """ + __slots__ = ('format_string', 'args', 'kwargs', 'result') + + def __init__(self, format_string, *args, **kwargs): + self.result = None + self.format_string = format_string + self.args = args + self.kwargs = kwargs + + def __str__(self): + if self.result is None: + self.result = self.format_string.format(*self.args, **self.kwargs) + self.format_string, self.args, self.kwargs = None, None, None + return self.result + + def __mod__(self, value): + return str(self) % value diff --git a/tests/test_utils.py b/tests/test_utils.py index 0d61cc2f9..885b5a58d 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,3 +1,5 @@ +from unittest import mock + from django.conf.urls import url from django.test import TestCase, override_settings @@ -6,6 +8,7 @@ from rest_framework.routers import SimpleRouter from rest_framework.serializers import ModelSerializer from rest_framework.utils import json from rest_framework.utils.breadcrumbs import get_breadcrumbs +from rest_framework.utils.formatting import lazy_format from rest_framework.utils.urls import remove_query_param, replace_query_param from rest_framework.views import APIView from rest_framework.viewsets import ModelViewSet @@ -257,3 +260,19 @@ class UrlsRemoveQueryParamTests(TestCase): removed_key = 'page' assert key in remove_query_param(q, removed_key) + + +class LazyFormatTests(TestCase): + def test_it_formats_correctly(self): + formatted = lazy_format('Does {} work? {answer}: %s', 'it', answer='Yes') + assert str(formatted) == 'Does it work? Yes: %s' + assert formatted % 'it does' == 'Does it work? Yes: it does' + + def test_it_formats_lazily(self): + message = mock.Mock(wraps='message') + formatted = lazy_format(message) + assert message.format.call_count == 0 + str(formatted) + assert message.format.call_count == 1 + str(formatted) + assert message.format.call_count == 1