From 0707ff6d366de6fbe08c8f70dd9d483344077711 Mon Sep 17 00:00:00 2001 From: Mike Yusko Date: Sat, 21 Dec 2019 22:18:14 +0200 Subject: [PATCH 01/34] Renamed set_language()'s next variable to avoid clash with builtin. --- django/views/i18n.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/django/views/i18n.py b/django/views/i18n.py index cbe61bcf885f..17eb4f9f6140 100644 --- a/django/views/i18n.py +++ b/django/views/i18n.py @@ -31,26 +31,31 @@ def set_language(request): redirect to the page in the request (the 'next' parameter) without changing any state. """ - next = request.POST.get('next', request.GET.get('next')) + next_url = request.POST.get('next', request.GET.get('next')) if ( - (next or not request.is_ajax()) and + (next_url or not request.is_ajax()) and not url_has_allowed_host_and_scheme( - url=next, allowed_hosts={request.get_host()}, require_https=request.is_secure(), + url=next_url, + allowed_hosts={request.get_host()}, + require_https=request.is_secure(), ) ): - next = request.META.get('HTTP_REFERER') - next = next and unquote(next) # HTTP_REFERER may be encoded. + next_url = request.META.get('HTTP_REFERER') + # HTTP_REFERER may be encoded. + next_url = next_url and unquote(next_url) if not url_has_allowed_host_and_scheme( - url=next, allowed_hosts={request.get_host()}, require_https=request.is_secure(), + url=next_url, + allowed_hosts={request.get_host()}, + require_https=request.is_secure(), ): - next = '/' - response = HttpResponseRedirect(next) if next else HttpResponse(status=204) + next_url = '/' + response = HttpResponseRedirect(next_url) if next_url else HttpResponse(status=204) if request.method == 'POST': lang_code = request.POST.get(LANGUAGE_QUERY_PARAMETER) if lang_code and check_for_language(lang_code): - if next: - next_trans = translate_url(next, lang_code) - if next_trans != next: + if next_url: + next_trans = translate_url(next_url, lang_code) + if next_trans != next_url: response = HttpResponseRedirect(next_trans) if hasattr(request, 'session'): # Storing the language in the session is deprecated. From 6c9c823e7a58827af9f3127d884aef4ac0a7251e Mon Sep 17 00:00:00 2001 From: Someoneece <54041974+Someoneece@users.noreply.github.com> Date: Thu, 9 Jan 2020 03:36:55 +0530 Subject: [PATCH 02/34] Renamed docs/README to README.rst. --- docs/{README => README.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename docs/{README => README.rst} (100%) diff --git a/docs/README b/docs/README.rst similarity index 100% rename from docs/README rename to docs/README.rst From 66e16dcc99f7442b135697989b79a8bd12c63272 Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Sun, 10 Nov 2019 20:02:16 +0100 Subject: [PATCH 03/34] Removed unused lines in ImageFieldTests.test_pickle(). --- tests/model_fields/test_imagefield.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/model_fields/test_imagefield.py b/tests/model_fields/test_imagefield.py index 59a1836bdb9e..5ac25fbe2680 100644 --- a/tests/model_fields/test_imagefield.py +++ b/tests/model_fields/test_imagefield.py @@ -178,9 +178,6 @@ def test_pickle(self): p.mugshot.save("mug", self.file1) dump = pickle.dumps(p) - p2 = Person(name="Bob") - p2.mugshot = self.file1 - loaded_p = pickle.loads(dump) self.assertEqual(p.mugshot, loaded_p.mugshot) From aaea9deac4dea08ada934a930cfc27765358d8da Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Thu, 9 Jan 2020 09:16:26 +0100 Subject: [PATCH 04/34] Refs #21238 -- Added more tests for pickling FileField and ImageField. --- tests/model_fields/test_filefield.py | 21 ++++++++++++++++++++- tests/model_fields/test_imagefield.py | 8 ++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/model_fields/test_filefield.py b/tests/model_fields/test_filefield.py index 815026047633..d25868d7cd52 100644 --- a/tests/model_fields/test_filefield.py +++ b/tests/model_fields/test_filefield.py @@ -1,10 +1,11 @@ import os +import pickle import sys import tempfile import unittest from pathlib import Path -from django.core.files import temp +from django.core.files import File, temp from django.core.files.base import ContentFile from django.core.files.uploadedfile import TemporaryUploadedFile from django.db.utils import IntegrityError @@ -103,3 +104,21 @@ def test_media_root_pathlib(self): with TemporaryUploadedFile('foo.txt', 'text/plain', 1, 'utf-8') as tmp_file: Document.objects.create(myfile=tmp_file) self.assertTrue(os.path.exists(os.path.join(tmp_dir, 'unused', 'foo.txt'))) + + def test_pickle(self): + with open(__file__, 'rb') as fp: + file1 = File(fp, name='test_file.py') + document = Document(myfile='test_file.py') + document.myfile.save('test_file.py', file1) + + dump = pickle.dumps(document) + loaded_document = pickle.loads(dump) + self.assertEqual(document.myfile, loaded_document.myfile) + self.assertEqual(document.myfile.url, loaded_document.myfile.url) + self.assertEqual(document.myfile.storage, loaded_document.myfile.storage) + self.assertEqual(document.myfile.instance, loaded_document.myfile.instance) + self.assertEqual(document.myfile.field, loaded_document.myfile.field) + + myfile_dump = pickle.dumps(document.myfile) + loaded_myfile = pickle.loads(myfile_dump) + self.assertEqual(document.myfile, loaded_myfile) diff --git a/tests/model_fields/test_imagefield.py b/tests/model_fields/test_imagefield.py index 5ac25fbe2680..e4390a2507ec 100644 --- a/tests/model_fields/test_imagefield.py +++ b/tests/model_fields/test_imagefield.py @@ -180,6 +180,14 @@ def test_pickle(self): loaded_p = pickle.loads(dump) self.assertEqual(p.mugshot, loaded_p.mugshot) + self.assertEqual(p.mugshot.url, loaded_p.mugshot.url) + self.assertEqual(p.mugshot.storage, loaded_p.mugshot.storage) + self.assertEqual(p.mugshot.instance, loaded_p.mugshot.instance) + self.assertEqual(p.mugshot.field, loaded_p.mugshot.field) + + mugshot_dump = pickle.dumps(p.mugshot) + loaded_mugshot = pickle.loads(mugshot_dump) + self.assertEqual(p.mugshot, loaded_mugshot) def test_defer(self): self.PersonModel.objects.create(name='Joe', mugshot=self.file1) From f600e3fad6e92d9fe1ad8b351dc8446415f24345 Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Sun, 10 Nov 2019 22:55:48 +0100 Subject: [PATCH 05/34] Fixed #21238 -- Fixed restoring attributes when pickling FileField and ImageField. --- django/db/models/fields/files.py | 20 +++++++++++++++----- tests/model_fields/test_filefield.py | 4 ++++ tests/model_fields/test_imagefield.py | 4 ++++ 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/django/db/models/fields/files.py b/django/db/models/fields/files.py index b03d54366aec..7ee38d937f67 100644 --- a/django/db/models/fields/files.py +++ b/django/db/models/fields/files.py @@ -123,11 +123,21 @@ def close(self): file.close() def __getstate__(self): - # FieldFile needs access to its associated model field and an instance - # it's attached to in order to work properly, but the only necessary - # data to be pickled is the file's name itself. Everything else will - # be restored later, by FileDescriptor below. - return {'name': self.name, 'closed': False, '_committed': True, '_file': None} + # FieldFile needs access to its associated model field, an instance and + # the file's name. Everything else will be restored later, by + # FileDescriptor below. + return { + 'name': self.name, + 'closed': False, + '_committed': True, + '_file': None, + 'instance': self.instance, + 'field': self.field, + } + + def __setstate__(self, state): + self.__dict__.update(state) + self.storage = self.field.storage class FileDescriptor: diff --git a/tests/model_fields/test_filefield.py b/tests/model_fields/test_filefield.py index d25868d7cd52..5600051a3566 100644 --- a/tests/model_fields/test_filefield.py +++ b/tests/model_fields/test_filefield.py @@ -122,3 +122,7 @@ def test_pickle(self): myfile_dump = pickle.dumps(document.myfile) loaded_myfile = pickle.loads(myfile_dump) self.assertEqual(document.myfile, loaded_myfile) + self.assertEqual(document.myfile.url, loaded_myfile.url) + self.assertEqual(document.myfile.storage, loaded_myfile.storage) + self.assertEqual(document.myfile.instance, loaded_myfile.instance) + self.assertEqual(document.myfile.field, loaded_myfile.field) diff --git a/tests/model_fields/test_imagefield.py b/tests/model_fields/test_imagefield.py index e4390a2507ec..99831cb0a3a8 100644 --- a/tests/model_fields/test_imagefield.py +++ b/tests/model_fields/test_imagefield.py @@ -188,6 +188,10 @@ def test_pickle(self): mugshot_dump = pickle.dumps(p.mugshot) loaded_mugshot = pickle.loads(mugshot_dump) self.assertEqual(p.mugshot, loaded_mugshot) + self.assertEqual(p.mugshot.url, loaded_mugshot.url) + self.assertEqual(p.mugshot.storage, loaded_mugshot.storage) + self.assertEqual(p.mugshot.instance, loaded_mugshot.instance) + self.assertEqual(p.mugshot.field, loaded_mugshot.field) def test_defer(self): self.PersonModel.objects.create(name='Joe', mugshot=self.file1) From eef3ea847e12f23db9a0af8efdc9543f24b780c4 Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Thu, 9 Jan 2020 11:10:25 +0100 Subject: [PATCH 06/34] Fixed #31148 -- Added error messages on update()/delete() operations following union(), intersection(), and difference(). --- django/db/models/query.py | 2 ++ tests/queries/test_qs_combinators.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/django/db/models/query.py b/django/db/models/query.py index 38c13584d133..f8be008a62ea 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -711,6 +711,7 @@ def in_bulk(self, id_list=None, *, field_name='pk'): def delete(self): """Delete the records in the current QuerySet.""" + self._not_support_combined_queries('delete') assert not self.query.is_sliced, \ "Cannot use 'limit' or 'offset' with delete." @@ -756,6 +757,7 @@ def update(self, **kwargs): Update all elements in the current QuerySet, setting all the given fields to the appropriate values. """ + self._not_support_combined_queries('update') assert not self.query.is_sliced, \ "Cannot update a query once a slice has been taken." self._for_write = True diff --git a/tests/queries/test_qs_combinators.py b/tests/queries/test_qs_combinators.py index 668d5e6ad6f0..71309ecf1b90 100644 --- a/tests/queries/test_qs_combinators.py +++ b/tests/queries/test_qs_combinators.py @@ -272,12 +272,14 @@ def test_unsupported_operations_on_combined_qs(self): for operation in ( 'annotate', 'defer', + 'delete', 'exclude', 'extra', 'filter', 'only', 'prefetch_related', 'select_related', + 'update', ): with self.subTest(combinator=combinator, operation=operation): with self.assertRaisesMessage( From ceecd0556dc6f013b5b62fedb12453b8ae3b8067 Mon Sep 17 00:00:00 2001 From: KHMANJUNATH Date: Thu, 9 Jan 2020 18:40:28 +0530 Subject: [PATCH 07/34] Improved ReST formatting in docs/README.rst. Co-authored-by: Mariusz Felisiak --- docs/README.rst | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/README.rst b/docs/README.rst index f34aa8782ad4..c9ed46ddfcba 100644 --- a/docs/README.rst +++ b/docs/README.rst @@ -1,7 +1,7 @@ The documentation in this tree is in plain text files and can be viewed using any text file viewer. -It uses ReST (reStructuredText) [1], and the Sphinx documentation system [2]. +It uses `ReST`_ (reStructuredText), and the `Sphinx`_ documentation system. This allows it to be built into other forms for easier viewing and browsing. To create an HTML version of the docs: @@ -11,7 +11,8 @@ To create an HTML version of the docs: * In this docs/ directory, type ``make html`` (or ``make.bat html`` on Windows) at a shell prompt. -The documentation in _build/html/index.html can then be viewed in a web browser. +The documentation in ``_build/html/index.html`` can then be viewed in a web +browser. -[1] http://docutils.sourceforge.net/rst.html -[2] http://sphinx-doc.org/ +.. _ReST: https://docutils.sourceforge.io/rst.html +.. _Sphinx: http://sphinx-doc.org From eb629f4c028ae220084904db84d633d7b3f0af20 Mon Sep 17 00:00:00 2001 From: Jack Cushman Date: Sat, 21 Dec 2019 13:22:18 -0500 Subject: [PATCH 08/34] Fixed #30995 -- Allowed converter.to_url() to raise ValueError to indicate no match. --- django/urls/resolvers.py | 9 ++++++++- docs/releases/3.1.txt | 3 ++- docs/topics/http/urls.txt | 13 ++++++++++-- tests/urlpatterns/path_same_name_urls.py | 17 ++++++++++++++-- tests/urlpatterns/tests.py | 25 ++++++++++++++++++++---- 5 files changed, 57 insertions(+), 10 deletions(-) diff --git a/django/urls/resolvers.py b/django/urls/resolvers.py index 120e0396af99..6e3f2443c974 100644 --- a/django/urls/resolvers.py +++ b/django/urls/resolvers.py @@ -632,11 +632,18 @@ def _reverse_with_prefix(self, lookup_view, _prefix, *args, **kwargs): candidate_subs = kwargs # Convert the candidate subs to text using Converter.to_url(). text_candidate_subs = {} + match = True for k, v in candidate_subs.items(): if k in converters: - text_candidate_subs[k] = converters[k].to_url(v) + try: + text_candidate_subs[k] = converters[k].to_url(v) + except ValueError: + match = False + break else: text_candidate_subs[k] = str(v) + if not match: + continue # WSGI provides decoded URLs, without %xx escapes, and the URL # resolver operates on such URLs. First substitute arguments # without quoting to build a decoded URL and look for a match. diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index cdacbd71cda9..3def8920cc6c 100644 --- a/docs/releases/3.1.txt +++ b/docs/releases/3.1.txt @@ -294,7 +294,8 @@ Tests URLs ~~~~ -* ... +* :ref:`Path converters ` can now raise + ``ValueError`` in ``to_url()`` to indicate no match when reversing URLs. Utilities ~~~~~~~~~ diff --git a/docs/topics/http/urls.txt b/docs/topics/http/urls.txt index ef9d4c007cd8..efe61d041cf1 100644 --- a/docs/topics/http/urls.txt +++ b/docs/topics/http/urls.txt @@ -156,7 +156,14 @@ A converter is a class that includes the following: user unless another URL pattern matches. * A ``to_url(self, value)`` method, which handles converting the Python type - into a string to be used in the URL. + into a string to be used in the URL. It should raise ``ValueError`` if it + can't convert the given value. A ``ValueError`` is interpreted as no match + and as a consequence :func:`~django.urls.reverse` will raise + :class:`~django.urls.NoReverseMatch` unless another URL pattern matches. + + .. versionchanged:: 3.1 + + Support for raising ``ValueError`` to indicate no match was added. For example:: @@ -666,7 +673,9 @@ included at all). You may also use the same name for multiple URL patterns if they differ in their arguments. In addition to the URL name, :func:`~django.urls.reverse()` -matches the number of arguments and the names of the keyword arguments. +matches the number of arguments and the names of the keyword arguments. Path +converters can also raise ``ValueError`` to indicate no match, see +:ref:`registering-custom-path-converters` for details. .. _topics-http-defining-url-namespaces: diff --git a/tests/urlpatterns/path_same_name_urls.py b/tests/urlpatterns/path_same_name_urls.py index 8eee949316cb..7b4fd2e62705 100644 --- a/tests/urlpatterns/path_same_name_urls.py +++ b/tests/urlpatterns/path_same_name_urls.py @@ -1,6 +1,8 @@ -from django.urls import path, re_path +from django.urls import path, re_path, register_converter -from . import views +from . import converters, views + +register_converter(converters.DynamicConverter, 'to_url_value_error') urlpatterns = [ # Different number of arguments. @@ -18,4 +20,15 @@ # Different regular expressions. re_path(r'^regex/uppercase/([A-Z]+)/', views.empty_view, name='regex'), re_path(r'^regex/lowercase/([a-z]+)/', views.empty_view, name='regex'), + # converter.to_url() raises ValueError (no match). + path( + 'converter_to_url/int//', + views.empty_view, + name='converter_to_url', + ), + path( + 'converter_to_url/tiny_int//', + views.empty_view, + name='converter_to_url', + ), ] diff --git a/tests/urlpatterns/tests.py b/tests/urlpatterns/tests.py index 95bea583394e..0de0f6f05da8 100644 --- a/tests/urlpatterns/tests.py +++ b/tests/urlpatterns/tests.py @@ -3,7 +3,7 @@ from django.core.exceptions import ImproperlyConfigured from django.test import SimpleTestCase from django.test.utils import override_settings -from django.urls import Resolver404, path, resolve, reverse +from django.urls import NoReverseMatch, Resolver404, path, resolve, reverse from .converters import DynamicConverter from .views import empty_view @@ -203,6 +203,12 @@ def test_nonmatching_urls(self): @override_settings(ROOT_URLCONF='urlpatterns.path_same_name_urls') class SameNameTests(SimpleTestCase): def test_matching_urls_same_name(self): + @DynamicConverter.register_to_url + def requires_tiny_int(value): + if value > 5: + raise ValueError + return value + tests = [ ('number_of_args', [ ([], {}, '0/'), @@ -227,6 +233,10 @@ def test_matching_urls_same_name(self): (['ABC'], {}, 'uppercase/ABC/'), (['abc'], {}, 'lowercase/abc/'), ]), + ('converter_to_url', [ + ([6], {}, 'int/6/'), + ([1], {}, 'tiny_int/1/'), + ]), ] for url_name, cases in tests: for args, kwargs, url_suffix in cases: @@ -272,9 +282,16 @@ def raises_type_error(value): with self.assertRaisesMessage(TypeError, 'This type error propagates.'): resolve('/dynamic/abc/') - def test_reverse_value_error_propagates(self): + def test_reverse_value_error_means_no_match(self): @DynamicConverter.register_to_url def raises_value_error(value): - raise ValueError('This value error propagates.') - with self.assertRaisesMessage(ValueError, 'This value error propagates.'): + raise ValueError + with self.assertRaises(NoReverseMatch): + reverse('dynamic', kwargs={'value': object()}) + + def test_reverse_type_error_propagates(self): + @DynamicConverter.register_to_url + def raises_type_error(value): + raise TypeError('This type error propagates.') + with self.assertRaisesMessage(TypeError, 'This type error propagates.'): reverse('dynamic', kwargs={'value': object()}) From aa6c620249bc8c2a6245c8d7b928b05e7e5e78fc Mon Sep 17 00:00:00 2001 From: Kal Sze Date: Thu, 18 Apr 2019 10:58:13 +0800 Subject: [PATCH 09/34] More accurate terminology ("logger" instead of "logging handler") in logging documentation. --- docs/topics/logging.txt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/topics/logging.txt b/docs/topics/logging.txt index b8b3162fe5e9..80348e360b53 100644 --- a/docs/topics/logging.txt +++ b/docs/topics/logging.txt @@ -164,10 +164,9 @@ is a parent of the ``project.interesting`` logger. Why is the hierarchy important? Well, because loggers can be set to *propagate* their logging calls to their parents. In this way, you can define a single set of handlers at the root of a logger tree, and -capture all logging calls in the subtree of loggers. A logging handler -defined in the ``project`` namespace will catch all logging messages -issued on the ``project.interesting`` and -``project.interesting.stuff`` loggers. +capture all logging calls in the subtree of loggers. A logger defined +in the ``project`` namespace will catch all logging messages issued on +the ``project.interesting`` and ``project.interesting.stuff`` loggers. This propagation can be controlled on a per-logger basis. If you don't want a particular logger to propagate to its parents, you From 4c1b401e8250f9f520b3c7dc369554477ce8b15a Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Thu, 9 Jan 2020 20:43:50 +0100 Subject: [PATCH 10/34] Added file cleanup in FileFieldTests.test_pickle(). --- tests/model_fields/test_filefield.py | 40 +++++++++++++++++----------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/tests/model_fields/test_filefield.py b/tests/model_fields/test_filefield.py index 5600051a3566..67c08c984c1d 100644 --- a/tests/model_fields/test_filefield.py +++ b/tests/model_fields/test_filefield.py @@ -110,19 +110,27 @@ def test_pickle(self): file1 = File(fp, name='test_file.py') document = Document(myfile='test_file.py') document.myfile.save('test_file.py', file1) - - dump = pickle.dumps(document) - loaded_document = pickle.loads(dump) - self.assertEqual(document.myfile, loaded_document.myfile) - self.assertEqual(document.myfile.url, loaded_document.myfile.url) - self.assertEqual(document.myfile.storage, loaded_document.myfile.storage) - self.assertEqual(document.myfile.instance, loaded_document.myfile.instance) - self.assertEqual(document.myfile.field, loaded_document.myfile.field) - - myfile_dump = pickle.dumps(document.myfile) - loaded_myfile = pickle.loads(myfile_dump) - self.assertEqual(document.myfile, loaded_myfile) - self.assertEqual(document.myfile.url, loaded_myfile.url) - self.assertEqual(document.myfile.storage, loaded_myfile.storage) - self.assertEqual(document.myfile.instance, loaded_myfile.instance) - self.assertEqual(document.myfile.field, loaded_myfile.field) + try: + dump = pickle.dumps(document) + loaded_document = pickle.loads(dump) + self.assertEqual(document.myfile, loaded_document.myfile) + self.assertEqual(document.myfile.url, loaded_document.myfile.url) + self.assertEqual( + document.myfile.storage, + loaded_document.myfile.storage, + ) + self.assertEqual( + document.myfile.instance, + loaded_document.myfile.instance, + ) + self.assertEqual(document.myfile.field, loaded_document.myfile.field) + + myfile_dump = pickle.dumps(document.myfile) + loaded_myfile = pickle.loads(myfile_dump) + self.assertEqual(document.myfile, loaded_myfile) + self.assertEqual(document.myfile.url, loaded_myfile.url) + self.assertEqual(document.myfile.storage, loaded_myfile.storage) + self.assertEqual(document.myfile.instance, loaded_myfile.instance) + self.assertEqual(document.myfile.field, loaded_myfile.field) + finally: + document.myfile.delete() From 5166097d7c80cab757e44f2d02f3d148fbbc2ff6 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Thu, 9 Jan 2020 15:41:54 +0000 Subject: [PATCH 11/34] Fixed #31154 -- Added support for using enumeration types in templates. Enumeration helpers are callables, so the template system tried to call them with no arguments. Thanks Rupert Baker for helping discover this. --- django/db/models/enums.py | 1 + docs/releases/3.0.3.txt | 3 +++ tests/model_enums/tests.py | 6 ++++++ 3 files changed, 10 insertions(+) diff --git a/django/db/models/enums.py b/django/db/models/enums.py index f48143ddaf52..51821a2b458f 100644 --- a/django/db/models/enums.py +++ b/django/db/models/enums.py @@ -31,6 +31,7 @@ def __new__(metacls, classname, bases, classdict): # that is passed in as "self" as the value to use when looking up the # label in the choices. cls.label = property(lambda self: cls._value2label_map_.get(self.value)) + cls.do_not_call_in_templates = True return enum.unique(cls) def __contains__(cls, member): diff --git a/docs/releases/3.0.3.txt b/docs/releases/3.0.3.txt index 27ace475d155..ab5a2fd84bfc 100644 --- a/docs/releases/3.0.3.txt +++ b/docs/releases/3.0.3.txt @@ -20,3 +20,6 @@ Bugfixes * Relaxed the system check added in Django 3.0 to reallow use of a sublanguage in the :setting:`LANGUAGE_CODE` setting, when a base language is available in Django but the sublanguage is not (:ticket:`31141`). + +* Added support for using enumeration types ``TextChoices``, + ``IntegerChoices``, and ``Choices`` in templates (:ticket:`31154`). diff --git a/tests/model_enums/tests.py b/tests/model_enums/tests.py index b9356479de95..ffc199ce424d 100644 --- a/tests/model_enums/tests.py +++ b/tests/model_enums/tests.py @@ -4,6 +4,7 @@ import uuid from django.db import models +from django.template import Context, Template from django.test import SimpleTestCase from django.utils.functional import Promise from django.utils.translation import gettext_lazy as _ @@ -149,6 +150,11 @@ def test_str(self): with self.subTest(member=member): self.assertEqual(str(test[member.name]), str(member.value)) + def test_templates(self): + template = Template('{{ Suit.DIAMOND.label }}|{{ Suit.DIAMOND.value }}') + output = template.render(Context({'Suit': Suit})) + self.assertEqual(output, 'Diamond|1') + class Separator(bytes, models.Choices): FS = b'\x1c', 'File Separator' From 581ba5a9486ed73cb81031d85b3ce1b27a960109 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Thu, 9 Jan 2020 10:00:07 +0100 Subject: [PATCH 12/34] Refs #23004 -- Allowed exception reporter filters to customize settings filtering. Thanks to Tim Graham for the original implementation idea. Co-authored-by: Daniel Maxson --- django/views/debug.py | 93 +++++++++++++--------------- docs/howto/error-reporting.txt | 43 +++++++++---- docs/releases/3.1.txt | 11 ++++ tests/view_tests/tests/test_debug.py | 83 +++++++++++++++++++------ 4 files changed, 152 insertions(+), 78 deletions(-) diff --git a/django/views/debug.py b/django/views/debug.py index 3b37e8da1a5f..608282c23277 100644 --- a/django/views/debug.py +++ b/django/views/debug.py @@ -25,10 +25,6 @@ libraries={'i18n': 'django.templatetags.i18n'}, ) -HIDDEN_SETTINGS = _lazy_re_compile('API|TOKEN|KEY|SECRET|PASS|SIGNATURE', flags=re.IGNORECASE) - -CLEANSED_SUBSTITUTE = '********************' - CURRENT_DIR = Path(__file__).parent @@ -46,42 +42,6 @@ def __repr__(self): return repr(self._wrapped) -def cleanse_setting(key, value): - """ - Cleanse an individual setting key/value of sensitive content. If the value - is a dictionary, recursively cleanse the keys in that dictionary. - """ - try: - if HIDDEN_SETTINGS.search(key): - cleansed = CLEANSED_SUBSTITUTE - else: - if isinstance(value, dict): - cleansed = {k: cleanse_setting(k, v) for k, v in value.items()} - else: - cleansed = value - except TypeError: - # If the key isn't regex-able, just return as-is. - cleansed = value - - if callable(cleansed): - # For fixing #21345 and #23070 - cleansed = CallableSettingWrapper(cleansed) - - return cleansed - - -def get_safe_settings(): - """ - Return a dictionary of the settings module with values of sensitive - settings replaced with stars (*********). - """ - settings_dict = {} - for k in dir(settings): - if k.isupper(): - settings_dict[k] = cleanse_setting(k, getattr(settings, k)) - return settings_dict - - def technical_500_response(request, exc_type, exc_value, tb, status_code=500): """ Create a technical server error response. The last three arguments are @@ -128,6 +88,40 @@ class SafeExceptionReporterFilter(ExceptionReporterFilter): Use annotations made by the sensitive_post_parameters and sensitive_variables decorators to filter out sensitive information. """ + cleansed_substitute = '********************' + hidden_settings = _lazy_re_compile('API|TOKEN|KEY|SECRET|PASS|SIGNATURE', flags=re.I) + + def cleanse_setting(self, key, value): + """ + Cleanse an individual setting key/value of sensitive content. If the + value is a dictionary, recursively cleanse the keys in that dictionary. + """ + try: + if self.hidden_settings.search(key): + cleansed = self.cleansed_substitute + elif isinstance(value, dict): + cleansed = {k: self.cleanse_setting(k, v) for k, v in value.items()} + else: + cleansed = value + except TypeError: + # If the key isn't regex-able, just return as-is. + cleansed = value + + if callable(cleansed): + cleansed = CallableSettingWrapper(cleansed) + + return cleansed + + def get_safe_settings(self): + """ + Return a dictionary of the settings module with values of sensitive + settings replaced with stars (*********). + """ + settings_dict = {} + for k in dir(settings): + if k.isupper(): + settings_dict[k] = self.cleanse_setting(k, getattr(settings, k)) + return settings_dict def is_active(self, request): """ @@ -149,7 +143,7 @@ def get_cleansed_multivaluedict(self, request, multivaluedict): multivaluedict = multivaluedict.copy() for param in sensitive_post_parameters: if param in multivaluedict: - multivaluedict[param] = CLEANSED_SUBSTITUTE + multivaluedict[param] = self.cleansed_substitute return multivaluedict def get_post_parameters(self, request): @@ -166,13 +160,13 @@ def get_post_parameters(self, request): if sensitive_post_parameters == '__ALL__': # Cleanse all parameters. for k in cleansed: - cleansed[k] = CLEANSED_SUBSTITUTE + cleansed[k] = self.cleansed_substitute return cleansed else: # Cleanse only the specified parameters. for param in sensitive_post_parameters: if param in cleansed: - cleansed[param] = CLEANSED_SUBSTITUTE + cleansed[param] = self.cleansed_substitute return cleansed else: return request.POST @@ -215,12 +209,12 @@ def get_traceback_frame_variables(self, request, tb_frame): if sensitive_variables == '__ALL__': # Cleanse all variables for name in tb_frame.f_locals: - cleansed[name] = CLEANSED_SUBSTITUTE + cleansed[name] = self.cleansed_substitute else: # Cleanse specified variables for name, value in tb_frame.f_locals.items(): if name in sensitive_variables: - value = CLEANSED_SUBSTITUTE + value = self.cleansed_substitute else: value = self.cleanse_special_types(request, value) cleansed[name] = value @@ -236,8 +230,8 @@ def get_traceback_frame_variables(self, request, tb_frame): # the sensitive_variables decorator's frame, in case the variables # associated with those arguments were meant to be obfuscated from # the decorated function's frame. - cleansed['func_args'] = CLEANSED_SUBSTITUTE - cleansed['func_kwargs'] = CLEANSED_SUBSTITUTE + cleansed['func_args'] = self.cleansed_substitute + cleansed['func_kwargs'] = self.cleansed_substitute return cleansed.items() @@ -304,7 +298,7 @@ def get_traceback_data(self): 'request': self.request, 'user_str': user_str, 'filtered_POST_items': list(self.filter.get_post_parameters(self.request).items()), - 'settings': get_safe_settings(), + 'settings': self.filter.get_safe_settings(), 'sys_executable': sys.executable, 'sys_version_info': '%d.%d.%d' % sys.version_info[0:3], 'server_time': timezone.now(), @@ -506,6 +500,7 @@ def technical_404_response(request, exception): with Path(CURRENT_DIR, 'templates', 'technical_404.html').open(encoding='utf-8') as fh: t = DEBUG_ENGINE.from_string(fh.read()) + reporter_filter = get_default_exception_reporter_filter() c = Context({ 'urlconf': urlconf, 'root_urlconf': settings.ROOT_URLCONF, @@ -513,7 +508,7 @@ def technical_404_response(request, exception): 'urlpatterns': tried, 'reason': str(exception), 'request': request, - 'settings': get_safe_settings(), + 'settings': reporter_filter.get_safe_settings(), 'raising_view_name': caller, }) return HttpResponseNotFound(t.render(c), content_type='text/html') diff --git a/docs/howto/error-reporting.txt b/docs/howto/error-reporting.txt index 8521e01c617f..a4cb5d2a1aae 100644 --- a/docs/howto/error-reporting.txt +++ b/docs/howto/error-reporting.txt @@ -262,25 +262,46 @@ attribute:: Your custom filter class needs to inherit from :class:`django.views.debug.SafeExceptionReporterFilter` and may override the -following methods: +following attributes and methods: .. class:: SafeExceptionReporterFilter -.. method:: SafeExceptionReporterFilter.is_active(request) + .. attribute:: cleansed_substitute - Returns ``True`` to activate the filtering operated in the other methods. - By default the filter is active if :setting:`DEBUG` is ``False``. + .. versionadded:: 3.1 -.. method:: SafeExceptionReporterFilter.get_post_parameters(request) + The string value to replace sensitive value with. By default it + replaces the values of sensitive variables with stars (`**********`). - Returns the filtered dictionary of POST parameters. By default it replaces - the values of sensitive parameters with stars (`**********`). + .. attribute:: hidden_settings -.. method:: SafeExceptionReporterFilter.get_traceback_frame_variables(request, tb_frame) + .. versionadded:: 3.1 - Returns the filtered dictionary of local variables for the given traceback - frame. By default it replaces the values of sensitive variables with stars - (`**********`). + A compiled regular expression object used to match settings considered + as sensitive. By default equivalent to:: + + import re + + re.compile(r'API|TOKEN|KEY|SECRET|PASS|SIGNATURE', flags=re.IGNORECASE) + + .. method:: is_active(request) + + Returns ``True`` to activate the filtering in + :meth:`get_post_parameters` and :meth:`get_traceback_frame_variables`. + By default the filter is active if :setting:`DEBUG` is ``False``. Note + that sensitive settings are always filtered, as described in the + :setting:`DEBUG` documentation. + + .. method:: get_post_parameters(request) + + Returns the filtered dictionary of POST parameters. Sensitive values + are replaced with :attr:`cleansed_substitute`. + + .. method:: get_traceback_frame_variables(request, tb_frame) + + Returns the filtered dictionary of local variables for the given + traceback frame. Sensitive values are replaced with + :attr:`cleansed_substitute`. .. seealso:: diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index 3def8920cc6c..48df0a0d66b6 100644 --- a/docs/releases/3.1.txt +++ b/docs/releases/3.1.txt @@ -158,6 +158,17 @@ Email * The :setting:`EMAIL_FILE_PATH` setting, used by the :ref:`file email backend `, now supports :class:`pathlib.Path`. +Error Reporting +~~~~~~~~~~~~~~~ + +* The new :attr:`.SafeExceptionReporterFilter.cleansed_substitute` and + :attr:`.SafeExceptionReporterFilter.hidden_settings` attributes allow + customization of sensitive settings filtering in exception reports. + +* The technical 404 debug view now respects + :setting:`DEFAULT_EXCEPTION_REPORTER_FILTER` when applying settings + filtering. + File Storage ~~~~~~~~~~~~ diff --git a/tests/view_tests/tests/test_debug.py b/tests/view_tests/tests/test_debug.py index 9b6f66e98997..9becfc77d061 100644 --- a/tests/view_tests/tests/test_debug.py +++ b/tests/view_tests/tests/test_debug.py @@ -20,11 +20,13 @@ from django.urls import path, reverse from django.urls.converters import IntConverter from django.utils.functional import SimpleLazyObject +from django.utils.regex_helper import _lazy_re_compile from django.utils.safestring import mark_safe from django.views.debug import ( - CLEANSED_SUBSTITUTE, CallableSettingWrapper, ExceptionReporter, - Path as DebugPath, cleanse_setting, default_urlconf, - technical_404_response, technical_500_response, + CallableSettingWrapper, ExceptionReporter, Path as DebugPath, + SafeExceptionReporterFilter, default_urlconf, + get_default_exception_reporter_filter, technical_404_response, + technical_500_response, ) from django.views.decorators.debug import ( sensitive_post_parameters, sensitive_variables, @@ -1199,6 +1201,66 @@ def test_settings_with_sensitive_keys(self): response = self.client.get('/raises500/') self.assertNotContains(response, 'should not be displayed', status_code=500) + def test_cleanse_setting_basic(self): + reporter_filter = SafeExceptionReporterFilter() + self.assertEqual(reporter_filter.cleanse_setting('TEST', 'TEST'), 'TEST') + self.assertEqual( + reporter_filter.cleanse_setting('PASSWORD', 'super_secret'), + reporter_filter.cleansed_substitute, + ) + + def test_cleanse_setting_ignore_case(self): + reporter_filter = SafeExceptionReporterFilter() + self.assertEqual( + reporter_filter.cleanse_setting('password', 'super_secret'), + reporter_filter.cleansed_substitute, + ) + + def test_cleanse_setting_recurses_in_dictionary(self): + reporter_filter = SafeExceptionReporterFilter() + initial = {'login': 'cooper', 'password': 'secret'} + self.assertEqual( + reporter_filter.cleanse_setting('SETTING_NAME', initial), + {'login': 'cooper', 'password': reporter_filter.cleansed_substitute}, + ) + + +class CustomExceptionReporterFilter(SafeExceptionReporterFilter): + cleansed_substitute = 'XXXXXXXXXXXXXXXXXXXX' + hidden_settings = _lazy_re_compile('API|TOKEN|KEY|SECRET|PASS|SIGNATURE|DATABASE_URL', flags=re.I) + + +@override_settings( + ROOT_URLCONF='view_tests.urls', + DEFAULT_EXCEPTION_REPORTER_FILTER='%s.CustomExceptionReporterFilter' % __name__, +) +class CustomExceptionReporterFilterTests(SimpleTestCase): + def setUp(self): + get_default_exception_reporter_filter.cache_clear() + + def tearDown(self): + get_default_exception_reporter_filter.cache_clear() + + def test_setting_allows_custom_subclass(self): + self.assertIsInstance( + get_default_exception_reporter_filter(), + CustomExceptionReporterFilter, + ) + + def test_cleansed_substitute_override(self): + reporter_filter = get_default_exception_reporter_filter() + self.assertEqual( + reporter_filter.cleanse_setting('password', 'super_secret'), + reporter_filter.cleansed_substitute, + ) + + def test_hidden_settings_override(self): + reporter_filter = get_default_exception_reporter_filter() + self.assertEqual( + reporter_filter.cleanse_setting('database_url', 'super_secret'), + reporter_filter.cleansed_substitute, + ) + class AjaxResponseExceptionReporterFilter(ExceptionReportTestMixin, LoggingCaptureMixin, SimpleTestCase): """ @@ -1262,21 +1324,6 @@ def test_ajax_response_encoding(self): self.assertEqual(response['Content-Type'], 'text/plain; charset=utf-8') -class HelperFunctionTests(SimpleTestCase): - - def test_cleanse_setting_basic(self): - self.assertEqual(cleanse_setting('TEST', 'TEST'), 'TEST') - self.assertEqual(cleanse_setting('PASSWORD', 'super_secret'), CLEANSED_SUBSTITUTE) - - def test_cleanse_setting_ignore_case(self): - self.assertEqual(cleanse_setting('password', 'super_secret'), CLEANSED_SUBSTITUTE) - - def test_cleanse_setting_recurses_in_dictionary(self): - initial = {'login': 'cooper', 'password': 'secret'} - expected = {'login': 'cooper', 'password': CLEANSED_SUBSTITUTE} - self.assertEqual(cleanse_setting('SETTING_NAME', initial), expected) - - class DecoratorsTests(SimpleTestCase): def test_sensitive_variables_not_called(self): msg = ( From e2d9d66a22f9004c0349f6aa9f8762fa558bdee8 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Thu, 9 Jan 2020 11:37:19 +0100 Subject: [PATCH 13/34] Fixed #23004 -- Added request.META filtering to SafeExceptionReporterFilter. Co-authored-by: Ryan Castner --- django/views/debug.py | 9 +++++++++ django/views/templates/technical_500.html | 2 +- django/views/templates/technical_500.txt | 2 +- docs/howto/error-reporting.txt | 10 ++++++---- docs/releases/3.1.txt | 6 +++++- tests/view_tests/tests/test_debug.py | 18 ++++++++++++++++++ 6 files changed, 40 insertions(+), 7 deletions(-) diff --git a/django/views/debug.py b/django/views/debug.py index 608282c23277..ae2b68ab7006 100644 --- a/django/views/debug.py +++ b/django/views/debug.py @@ -123,6 +123,14 @@ def get_safe_settings(self): settings_dict[k] = self.cleanse_setting(k, getattr(settings, k)) return settings_dict + def get_safe_request_meta(self, request): + """ + Return a dictionary of request.META with sensitive values redacted. + """ + if not hasattr(request, 'META'): + return {} + return {k: self.cleanse_setting(k, v) for k, v in request.META.items()} + def is_active(self, request): """ This filter is to add safety in production environments (i.e. DEBUG @@ -296,6 +304,7 @@ def get_traceback_data(self): 'unicode_hint': unicode_hint, 'frames': frames, 'request': self.request, + 'request_meta': self.filter.get_safe_request_meta(self.request), 'user_str': user_str, 'filtered_POST_items': list(self.filter.get_post_parameters(self.request).items()), 'settings': self.filter.get_safe_settings(), diff --git a/django/views/templates/technical_500.html b/django/views/templates/technical_500.html index b01b00c8e6b6..a63c7183e6f8 100644 --- a/django/views/templates/technical_500.html +++ b/django/views/templates/technical_500.html @@ -438,7 +438,7 @@

META

- {% for var in request.META.items|dictsort:0 %} + {% for var in request_meta.items|dictsort:0 %} {{ var.0 }} 
{{ var.1|pprint }}
diff --git a/django/views/templates/technical_500.txt b/django/views/templates/technical_500.txt index c9f70af79701..f06a1a499ec1 100644 --- a/django/views/templates/technical_500.txt +++ b/django/views/templates/technical_500.txt @@ -50,7 +50,7 @@ FILES:{% for k, v in request_FILES_items %} COOKIES:{% for k, v in request_COOKIES_items %} {{ k }} = {{ v|stringformat:"r" }}{% empty %} No cookie data{% endfor %} -META:{% for k, v in request.META.items|dictsort:0 %} +META:{% for k, v in request_meta.items|dictsort:0 %} {{ k }} = {{ v|stringformat:"r" }}{% endfor %} {% else %}Request data not supplied {% endif %} diff --git a/docs/howto/error-reporting.txt b/docs/howto/error-reporting.txt index a4cb5d2a1aae..e145897b2a38 100644 --- a/docs/howto/error-reporting.txt +++ b/docs/howto/error-reporting.txt @@ -277,8 +277,9 @@ following attributes and methods: .. versionadded:: 3.1 - A compiled regular expression object used to match settings considered - as sensitive. By default equivalent to:: + A compiled regular expression object used to match settings and + ``request.META`` values considered as sensitive. By default equivalent + to:: import re @@ -289,8 +290,9 @@ following attributes and methods: Returns ``True`` to activate the filtering in :meth:`get_post_parameters` and :meth:`get_traceback_frame_variables`. By default the filter is active if :setting:`DEBUG` is ``False``. Note - that sensitive settings are always filtered, as described in the - :setting:`DEBUG` documentation. + that sensitive ``request.META`` values are always filtered along with + sensitive setting values, as described in the :setting:`DEBUG` + documentation. .. method:: get_post_parameters(request) diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index 48df0a0d66b6..29e5e223140b 100644 --- a/docs/releases/3.1.txt +++ b/docs/releases/3.1.txt @@ -161,9 +161,13 @@ Email Error Reporting ~~~~~~~~~~~~~~~ +* :class:`django.views.debug.SafeExceptionReporterFilter` now filters sensitive + values from ``request.META`` in exception reports. + * The new :attr:`.SafeExceptionReporterFilter.cleansed_substitute` and :attr:`.SafeExceptionReporterFilter.hidden_settings` attributes allow - customization of sensitive settings filtering in exception reports. + customization of sensitive settings and ``request.META`` filtering in + exception reports. * The technical 404 debug view now respects :setting:`DEFAULT_EXCEPTION_REPORTER_FILTER` when applying settings diff --git a/tests/view_tests/tests/test_debug.py b/tests/view_tests/tests/test_debug.py index 9becfc77d061..020af1a19c0e 100644 --- a/tests/view_tests/tests/test_debug.py +++ b/tests/view_tests/tests/test_debug.py @@ -1224,6 +1224,24 @@ def test_cleanse_setting_recurses_in_dictionary(self): {'login': 'cooper', 'password': reporter_filter.cleansed_substitute}, ) + def test_request_meta_filtering(self): + request = self.rf.get('/', HTTP_SECRET_HEADER='super_secret') + reporter_filter = SafeExceptionReporterFilter() + self.assertEqual( + reporter_filter.get_safe_request_meta(request)['HTTP_SECRET_HEADER'], + reporter_filter.cleansed_substitute, + ) + + def test_exception_report_uses_meta_filtering(self): + response = self.client.get('/raises500/', HTTP_SECRET_HEADER='super_secret') + self.assertNotIn(b'super_secret', response.content) + response = self.client.get( + '/raises500/', + HTTP_SECRET_HEADER='super_secret', + HTTP_X_REQUESTED_WITH='XMLHttpRequest', + ) + self.assertNotIn(b'super_secret', response.content) + class CustomExceptionReporterFilter(SafeExceptionReporterFilter): cleansed_substitute = 'XXXXXXXXXXXXXXXXXXXX' From 8b3e714ecf409ed6c9628c3f2a4e033cbfa4253b Mon Sep 17 00:00:00 2001 From: Adam Donaghy Date: Mon, 6 Jan 2020 22:10:40 +1100 Subject: [PATCH 14/34] Fixed #30980 -- Improved error message when checking uniqueness of admin actions' __name__. Thanks Keshav Kumar for the initial patch. --- django/contrib/admin/checks.py | 24 +++++++++++++++--------- docs/ref/checks.txt | 2 +- tests/modeladmin/test_checks.py | 5 ++--- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py index 0c32301284b4..ba754bd873b7 100644 --- a/django/contrib/admin/checks.py +++ b/django/contrib/admin/checks.py @@ -1,3 +1,4 @@ +import collections from itertools import chain from django.apps import apps @@ -985,15 +986,20 @@ def _check_action_permission_methods(self, obj): def _check_actions_uniqueness(self, obj): """Check that every action has a unique __name__.""" - names = [name for _, name, _ in obj._get_base_actions()] - if len(names) != len(set(names)): - return [checks.Error( - '__name__ attributes of actions defined in %s must be ' - 'unique.' % obj.__class__, - obj=obj.__class__, - id='admin.E130', - )] - return [] + errors = [] + names = collections.Counter(name for _, name, _ in obj._get_base_actions()) + for name, count in names.items(): + if count > 1: + errors.append(checks.Error( + '__name__ attributes of actions defined in %s must be ' + 'unique. Name %r is not unique.' % ( + obj.__class__.__name__, + name, + ), + obj=obj.__class__, + id='admin.E130', + )) + return errors class InlineModelAdminChecks(BaseModelAdminChecks): diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index fcbfec98cb17..a080b5bdf51c 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -633,7 +633,7 @@ with the admin site: * **admin.E129**: ```` must define a ``has__permission()`` method for the ```` action. * **admin.E130**: ``__name__`` attributes of actions defined in - ```` must be unique. + ```` must be unique. Name ```` is not unique. ``InlineModelAdmin`` ~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/modeladmin/test_checks.py b/tests/modeladmin/test_checks.py index 98cc56d67c3d..308f4a19eb67 100644 --- a/tests/modeladmin/test_checks.py +++ b/tests/modeladmin/test_checks.py @@ -1441,9 +1441,8 @@ class BandAdmin(ModelAdmin): self.assertIsInvalid( BandAdmin, Band, - "__name__ attributes of actions defined in " - ".BandAdmin'> must be unique.", + "__name__ attributes of actions defined in BandAdmin must be " + "unique. Name 'action' is not unique.", id='admin.E130', ) From 6f7998adc784032f4b8918ca2eea27537ea4cbbe Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Sat, 11 Jan 2020 19:47:36 +0100 Subject: [PATCH 15/34] Fixed #31155 -- Fixed a system check for the longest choice when a named group contains only non-string values. Regression in b6251956b69512bf230322bd7a49b629ca8455c6. Thanks Murat Guchetl for the report. --- django/db/models/fields/__init__.py | 4 ++-- docs/releases/3.0.3.txt | 3 +++ .../test_ordinary_fields.py | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index aa21a151bc15..1a55d3d41789 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -269,10 +269,10 @@ def _check_choices(self): ): break if self.max_length is not None and group_choices: - choice_max_length = max( + choice_max_length = max([ choice_max_length, *(len(value) for value, _ in group_choices if isinstance(value, str)), - ) + ]) except (TypeError, ValueError): # No groups, choices in the form [value, display] value, human_name = group_name, group_choices diff --git a/docs/releases/3.0.3.txt b/docs/releases/3.0.3.txt index ab5a2fd84bfc..6d2098d78249 100644 --- a/docs/releases/3.0.3.txt +++ b/docs/releases/3.0.3.txt @@ -23,3 +23,6 @@ Bugfixes * Added support for using enumeration types ``TextChoices``, ``IntegerChoices``, and ``Choices`` in templates (:ticket:`31154`). + +* Fixed a system check to ensure the ``max_length`` attribute fits the longest + choice, when a named group contains only non-string values (:ticket:`31155`). diff --git a/tests/invalid_models_tests/test_ordinary_fields.py b/tests/invalid_models_tests/test_ordinary_fields.py index 813f97e5515b..ce7a67ca51dc 100644 --- a/tests/invalid_models_tests/test_ordinary_fields.py +++ b/tests/invalid_models_tests/test_ordinary_fields.py @@ -1,4 +1,5 @@ import unittest +import uuid from django.core.checks import Error, Warning as DjangoWarning from django.db import connection, models @@ -769,3 +770,20 @@ class Model(models.Model): id='fields.W162', ) ]) + + +@isolate_apps('invalid_models_tests') +class UUIDFieldTests(TestCase): + def test_choices_named_group(self): + class Model(models.Model): + field = models.UUIDField( + choices=[ + ['knights', [ + [uuid.UUID('5c859437-d061-4847-b3f7-e6b78852f8c8'), 'Lancelot'], + [uuid.UUID('c7853ec1-2ea3-4359-b02d-b54e8f1bcee2'), 'Galahad'], + ]], + [uuid.UUID('25d405be-4895-4d50-9b2e-d6695359ce47'), 'Other'], + ], + ) + + self.assertEqual(Model._meta.get_field('field').check(), []) From 1f4b9f4f1f17542e8af755de7aa139196ae440c1 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Thu, 9 Jan 2020 15:54:05 +0100 Subject: [PATCH 16/34] Removed unused ExceptionReporterFilter class. Unused since 8f8c54f70bfa3aa8e311514297f1eeded2c32593. --- django/views/debug.py | 18 +----------------- docs/releases/3.1.txt | 5 +++++ 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/django/views/debug.py b/django/views/debug.py index ae2b68ab7006..13dabf165bc0 100644 --- a/django/views/debug.py +++ b/django/views/debug.py @@ -67,23 +67,7 @@ def get_exception_reporter_filter(request): return getattr(request, 'exception_reporter_filter', default_filter) -class ExceptionReporterFilter: - """ - Base for all exception reporter filter classes. All overridable hooks - contain lenient default behaviors. - """ - - def get_post_parameters(self, request): - if request is None: - return {} - else: - return request.POST - - def get_traceback_frame_variables(self, request, tb_frame): - return list(tb_frame.f_locals.items()) - - -class SafeExceptionReporterFilter(ExceptionReporterFilter): +class SafeExceptionReporterFilter: """ Use annotations made by the sensitive_post_parameters and sensitive_variables decorators to filter out sensitive information. diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index 29e5e223140b..9e8dff3456e6 100644 --- a/docs/releases/3.1.txt +++ b/docs/releases/3.1.txt @@ -425,6 +425,11 @@ Miscellaneous * The :class:`~django.forms.FileInput` widget no longer renders with the ``required`` HTML attribute when initial data exists. +* The undocumented ``django.views.debug.ExceptionReporterFilter`` class is + removed. As per the :ref:`custom-error-reports` documentation, classes to be + used with :setting:`DEFAULT_EXCEPTION_REPORTER_FILTER` needs to inherit from + :class:`django.views.debug.SafeExceptionReporterFilter`. + .. _deprecated-features-3.1: Features deprecated in 3.1 From 77d335e5abec889b15323975187a8d5b10bfcb0f Mon Sep 17 00:00:00 2001 From: "Owen T. Heisler" Date: Mon, 13 Jan 2020 02:13:33 -0600 Subject: [PATCH 17/34] Fixed #31160 -- Fixed admin CSS for ordered lists' descendants in unordered list. --- django/contrib/admin/static/admin/css/base.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django/contrib/admin/static/admin/css/base.css b/django/contrib/admin/static/admin/css/base.css index 3e27dd41f06c..cb527f1618ab 100644 --- a/django/contrib/admin/static/admin/css/base.css +++ b/django/contrib/admin/static/admin/css/base.css @@ -94,7 +94,7 @@ h5 { letter-spacing: 1px; } -ul li { +ul > li { list-style-type: square; padding: 1px 0; } From 20debf01bd3b58ba28ffc6606333851a7ef18bea Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 13 Jan 2020 12:39:14 +0100 Subject: [PATCH 18/34] Fixed typo in docs/ref/django-admin.txt. --- docs/ref/django-admin.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ref/django-admin.txt b/docs/ref/django-admin.txt index cbb4a2d8eda4..3f95d8e1ce56 100644 --- a/docs/ref/django-admin.txt +++ b/docs/ref/django-admin.txt @@ -218,7 +218,7 @@ Specifies the database onto which to open a shell. Defaults to ``default``. .. note:: - Be aware that not all options set it in the :setting:`OPTIONS` part of your + Be aware that not all options set in the :setting:`OPTIONS` part of your database configuration in :setting:`DATABASES` are passed to the command-line client, e.g. ``'isolation_level'``. From 4fe486520f9275d88f25a867706b974886ad1b54 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 14 Jan 2020 10:08:27 +0100 Subject: [PATCH 19/34] Fixed nesting in technical 500 template. --- django/views/templates/technical_500.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django/views/templates/technical_500.html b/django/views/templates/technical_500.html index a63c7183e6f8..3e3d3b332527 100644 --- a/django/views/templates/technical_500.html +++ b/django/views/templates/technical_500.html @@ -208,7 +208,7 @@

{{ template_info.message }}

{% endif %} {% if frames %}
-

Traceback {% if not is_email %} +

Traceback{% if not is_email %} Switch to copy-and-paste view{% endif %}

From 927c903f3cd25c817c21738328b53991c035b415 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Tue, 14 Jan 2020 09:59:23 +0100 Subject: [PATCH 20/34] Refs #31097 -- Added release notes for 2f565f84aca136d9cc4e4d061f3196ddf9358ab8. . --- docs/releases/3.0.3.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/releases/3.0.3.txt b/docs/releases/3.0.3.txt index 6d2098d78249..c2cac9203d8a 100644 --- a/docs/releases/3.0.3.txt +++ b/docs/releases/3.0.3.txt @@ -26,3 +26,8 @@ Bugfixes * Fixed a system check to ensure the ``max_length`` attribute fits the longest choice, when a named group contains only non-string values (:ticket:`31155`). + +* Fixed a regression in Django 2.2 that caused a crash of + :class:`~django.contrib.postgres.aggregates.ArrayAgg` and + :class:`~django.contrib.postgres.aggregates.StringAgg` with ``filter`` + argument when used in a ``Subquery`` (:ticket:`31097`). From 63e6ee1f996e16a1a6238fed16fdb28bce156bc6 Mon Sep 17 00:00:00 2001 From: chetan22 Date: Mon, 28 Oct 2019 10:58:40 +0530 Subject: [PATCH 21/34] Fixed #29871 -- Allowed setting pk=None on a child model to create a copy. Thanks Simon Charette and Tim Graham for the initial patch. --- django/db/models/base.py | 3 ++ tests/model_inheritance_regress/tests.py | 37 +++++++++++++++++++++--- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/django/db/models/base.py b/django/db/models/base.py index 844c01e95eeb..8ea6c05ef948 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -569,6 +569,9 @@ def _get_pk_val(self, meta=None): return getattr(self, meta.pk.attname) def _set_pk_val(self, value): + for parent_link in self._meta.parents.values(): + if parent_link and parent_link != self._meta.pk: + setattr(self, parent_link.target_field.attname, value) return setattr(self, self._meta.pk.attname, value) pk = property(_get_pk_val, _set_pk_val) diff --git a/tests/model_inheritance_regress/tests.py b/tests/model_inheritance_regress/tests.py index b0156ff9ac41..2c15925da5f7 100644 --- a/tests/model_inheritance_regress/tests.py +++ b/tests/model_inheritance_regress/tests.py @@ -10,10 +10,11 @@ from .models import ( ArticleWithAuthor, BachelorParty, BirthdayParty, BusStation, Child, - DerivedM, InternalCertificationAudit, ItalianRestaurant, M2MChild, - MessyBachelorParty, ParkingLot, ParkingLot3, ParkingLot4A, ParkingLot4B, - Person, Place, Profile, QualityControl, Restaurant, SelfRefChild, - SelfRefParent, Senator, Supplier, TrainStation, User, Wholesaler, + Congressman, DerivedM, InternalCertificationAudit, ItalianRestaurant, + M2MChild, MessyBachelorParty, ParkingLot, ParkingLot3, ParkingLot4A, + ParkingLot4B, Person, Place, Politician, Profile, QualityControl, + Restaurant, SelfRefChild, SelfRefParent, Senator, Supplier, TrainStation, + User, Wholesaler, ) @@ -558,3 +559,31 @@ def test_id_field_update_on_ancestor_change(self): italian_restaurant.restaurant_ptr = None self.assertIsNone(italian_restaurant.pk) self.assertIsNone(italian_restaurant.id) + + def test_create_new_instance_with_pk_equals_none(self): + p1 = Profile.objects.create(username='john') + p2 = User.objects.get(pk=p1.user_ptr_id).profile + # Create a new profile by setting pk = None. + p2.pk = None + p2.user_ptr_id = None + p2.username = 'bill' + p2.save() + self.assertEqual(Profile.objects.count(), 2) + self.assertEqual(User.objects.get(pk=p1.user_ptr_id).username, 'john') + + def test_create_new_instance_with_pk_equals_none_multi_inheritance(self): + c1 = Congressman.objects.create(state='PA', name='John', title='senator 1') + c2 = Person.objects.get(pk=c1.pk).congressman + # Create a new congressman by setting pk = None. + c2.pk = None + c2.id = None + c2.politician_ptr_id = None + c2.name = 'Bill' + c2.title = 'senator 2' + c2.save() + self.assertEqual(Congressman.objects.count(), 2) + self.assertEqual(Person.objects.get(pk=c1.pk).name, 'John') + self.assertEqual( + Politician.objects.get(pk=c1.politician_ptr_id).title, + 'senator 1', + ) From 59b4e99dd00b9c36d56055b889f96885995e4240 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Wed, 15 Jan 2020 09:32:42 +0100 Subject: [PATCH 22/34] Refs #31136 -- Made QuerySet.values()/values_list() group only by selected annotation. Regression in 0f843fdd5b9b2f2307148465cd60f4e1b2befbb4. --- django/db/models/sql/query.py | 15 ++++++++------- tests/aggregation/tests.py | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index f96a0a6e2d8b..78c4f47b5bfa 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -2093,13 +2093,6 @@ def set_values(self, fields): self.clear_deferred_loading() self.clear_select_fields() - if self.group_by is True: - self.add_fields((f.attname for f in self.model._meta.concrete_fields), False) - # Disable GROUP BY aliases to avoid orphaning references to the - # SELECT clause which is about to be cleared. - self.set_group_by(allow_aliases=False) - self.clear_select_fields() - if fields: field_names = [] extra_names = [] @@ -2121,6 +2114,14 @@ def set_values(self, fields): self.set_annotation_mask(annotation_names) else: field_names = [f.attname for f in self.model._meta.concrete_fields] + # Selected annotations must be known before setting the GROUP BY + # clause. + if self.group_by is True: + self.add_fields((f.attname for f in self.model._meta.concrete_fields), False) + # Disable GROUP BY aliases to avoid orphaning references to the + # SELECT clause which is about to be cleared. + self.set_group_by(allow_aliases=False) + self.clear_select_fields() self.values_select = tuple(field_names) self.add_fields(field_names, True) diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index f523b3ca35a9..06c4cc7ccb69 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -1165,6 +1165,31 @@ def test_aggregation_exists_annotation(self): 'Sams', ]) + def test_aggregation_subquery_annotation_values(self): + """ + Subquery annotations and external aliases are excluded from the GROUP + BY if they are not selected. + """ + books_qs = Book.objects.annotate( + first_author_the_same_age=Subquery( + Author.objects.filter( + age=OuterRef('contact__friends__age'), + ).order_by('age').values('id')[:1], + ) + ).filter( + publisher=self.p1, + first_author_the_same_age__isnull=False, + ).annotate( + min_age=Min('contact__friends__age'), + ).values('name', 'min_age').order_by('name') + self.assertEqual(list(books_qs), [ + {'name': 'Practical Django Projects', 'min_age': 34}, + { + 'name': 'The Definitive Guide to Django: Web Development Done Right', + 'min_age': 29, + }, + ]) + @skipUnlessDBFeature('supports_subqueries_in_group_by') def test_group_by_subquery_annotation(self): """ From b5a62bd17db0e28e6842111034fa6714d0701edb Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Wed, 8 Jan 2020 16:23:08 +0100 Subject: [PATCH 23/34] Refs #27468 -- Added explicit tests for django.utils.crypto.salted_hmac() --- tests/utils_tests/test_crypto.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/utils_tests/test_crypto.py b/tests/utils_tests/test_crypto.py index c2045fc4abb8..1c3868f4caf7 100644 --- a/tests/utils_tests/test_crypto.py +++ b/tests/utils_tests/test_crypto.py @@ -1,7 +1,7 @@ import hashlib import unittest -from django.utils.crypto import constant_time_compare, pbkdf2 +from django.utils.crypto import constant_time_compare, pbkdf2, salted_hmac class TestUtilsCryptoMisc(unittest.TestCase): @@ -13,6 +13,25 @@ def test_constant_time_compare(self): self.assertTrue(constant_time_compare('spam', 'spam')) self.assertFalse(constant_time_compare('spam', 'eggs')) + def test_salted_hmac(self): + tests = [ + ((b'salt', b'value'), {}, 'b51a2e619c43b1ca4f91d15c57455521d71d61eb'), + (('salt', 'value'), {}, 'b51a2e619c43b1ca4f91d15c57455521d71d61eb'), + ( + ('salt', 'value'), + {'secret': 'abcdefg'}, + '8bbee04ccddfa24772d1423a0ba43bd0c0e24b76', + ), + ( + ('salt', 'value'), + {'secret': 'x' * hashlib.sha1().block_size}, + 'bd3749347b412b1b0a9ea65220e55767ac8e96b0', + ), + ] + for args, kwargs, digest in tests: + with self.subTest(args=args, kwargs=kwargs): + self.assertEqual(salted_hmac(*args, **kwargs).hexdigest(), digest) + class TestUtilsCryptoPBKDF2(unittest.TestCase): From c5e373d48cbdd923575956fed477b63d66d9603f Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Wed, 15 Jan 2020 11:58:01 +0100 Subject: [PATCH 24/34] Fixed obsolete comment in django.utils.crypto.salted_hmac(). Obsolete since 13864703bc1d5dd4dac63c96c6a4b42a392bc57f. --- django/utils/crypto.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/django/utils/crypto.py b/django/utils/crypto.py index eeb55af0667d..4ec1cfcf77a7 100644 --- a/django/utils/crypto.py +++ b/django/utils/crypto.py @@ -26,8 +26,7 @@ def salted_hmac(key_salt, value, secret=None): # passing the key_salt and our base key through a pseudo-random function and # SHA1 works nicely. key = hashlib.sha1(key_salt + secret).digest() - - # If len(key_salt + secret) > sha_constructor().block_size, the above + # If len(key_salt + secret) > block size of the hash algorithm, the above # line is redundant and could be replaced by key = key_salt + secret, since # the hmac module does the same thing for keys longer than the block size. # However, we need to ensure that we *always* do this. From d202846ced2f58d7a34ad80bfe2bde8a542a70b9 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Wed, 15 Jan 2020 15:07:07 +0100 Subject: [PATCH 25/34] Refs #29998 -- Corrected auto-created OneToOneField parent_link in MTI docs. --- docs/topics/db/models.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/topics/db/models.txt b/docs/topics/db/models.txt index a047f58b7883..75a46485acdc 100644 --- a/docs/topics/db/models.txt +++ b/docs/topics/db/models.txt @@ -1092,6 +1092,7 @@ The automatically-created :class:`~django.db.models.OneToOneField` on place_ptr = models.OneToOneField( Place, on_delete=models.CASCADE, parent_link=True, + primary_key=True, ) You can override that field by declaring your own From 29c126bb349526b5f1cd78facbe9f25906f18563 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Tue, 7 Jan 2020 11:52:09 +0100 Subject: [PATCH 26/34] Fixed #31124 -- Fixed setting of get_FOO_display() when overriding inherited choices. Regression in 2d38eb0ab9f78d68c083a5b78b1eca39027b279a --- django/db/models/fields/__init__.py | 6 +++++- docs/ref/models/instances.txt | 9 +++++++++ docs/releases/3.0.3.txt | 4 ++++ tests/model_fields/tests.py | 13 +++++++++++++ 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 1a55d3d41789..6bd95f542ce8 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -764,7 +764,11 @@ def contribute_to_class(self, cls, name, private_only=False): if not getattr(cls, self.attname, None): setattr(cls, self.attname, self.descriptor_class(self)) if self.choices is not None: - if not hasattr(cls, 'get_%s_display' % self.name): + # Don't override a get_FOO_display() method defined explicitly on + # this class, but don't check methods derived from inheritance, to + # allow overriding inherited choices. For more complex inheritance + # structures users should override contribute_to_class(). + if 'get_%s_display' % self.name not in cls.__dict__: setattr( cls, 'get_%s_display' % self.name, diff --git a/docs/ref/models/instances.txt b/docs/ref/models/instances.txt index 1524ad2fcd79..6adcb979ba58 100644 --- a/docs/ref/models/instances.txt +++ b/docs/ref/models/instances.txt @@ -834,6 +834,15 @@ Note that in the case of identical date values, these methods will use the primary key as a tie-breaker. This guarantees that no records are skipped or duplicated. That also means you cannot use those methods on unsaved objects. +.. admonition:: Overriding extra instance methods + + In most cases overriding or inheriting ``get_FOO_display()``, + ``get_next_by_FOO()``, and ``get_previous_by_FOO()` should work as + expected. Since they are added by the metaclass however, it is not + practical to account for all possible inheritance structures. In more + complex cases you should override ``Field.contribute_to_class()`` to set + the methods you need. + Other attributes ================ diff --git a/docs/releases/3.0.3.txt b/docs/releases/3.0.3.txt index c2cac9203d8a..2726e2d3ab16 100644 --- a/docs/releases/3.0.3.txt +++ b/docs/releases/3.0.3.txt @@ -31,3 +31,7 @@ Bugfixes :class:`~django.contrib.postgres.aggregates.ArrayAgg` and :class:`~django.contrib.postgres.aggregates.StringAgg` with ``filter`` argument when used in a ``Subquery`` (:ticket:`31097`). + +* Fixed a regression in Django 2.2.7 that caused + :meth:`~django.db.models.Model.get_FOO_display` to work incorrectly when + overriding inherited choices (:ticket:`31124`). diff --git a/tests/model_fields/tests.py b/tests/model_fields/tests.py index a3b805409c55..b97c99d42d06 100644 --- a/tests/model_fields/tests.py +++ b/tests/model_fields/tests.py @@ -178,6 +178,19 @@ def get_foo_bar_display(self): f = FooBar(foo_bar=1) self.assertEqual(f.get_foo_bar_display(), 'something') + def test_overriding_inherited_FIELD_display(self): + class Base(models.Model): + foo = models.CharField(max_length=254, choices=[('A', 'Base A')]) + + class Meta: + abstract = True + + class Child(Base): + foo = models.CharField(max_length=254, choices=[('A', 'Child A'), ('B', 'Child B')]) + + self.assertEqual(Child(foo='A').get_foo_display(), 'Child A') + self.assertEqual(Child(foo='B').get_foo_display(), 'Child B') + def test_iterator_choices(self): """ get_choices() works with Iterators. From 7400da49a5d0ec5a087c3cf05e0b2d15048fc93f Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Tue, 14 Jan 2020 10:13:20 +0100 Subject: [PATCH 27/34] Clarified backport policy for regressions. --- docs/internals/release-process.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/internals/release-process.txt b/docs/internals/release-process.txt index ce600395eb71..e86554ea151b 100644 --- a/docs/internals/release-process.txt +++ b/docs/internals/release-process.txt @@ -141,7 +141,8 @@ page for the current state of support for each version. * Major functionality bugs in new features of the latest stable release. - * Regressions from older versions of Django. + * Regressions from older versions of Django introduced in the current release + series. The rule of thumb is that fixes will be backported to the last feature release for bugs that would have prevented a release in the first place From bf77669453b9e9f64291da6701fe06fd5ba47e29 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Thu, 16 Jan 2020 08:06:16 +0100 Subject: [PATCH 28/34] Fixed #29998 -- Allowed multiple OneToOneFields to the parent model. We assumed that any OneToOneField's in a child model must be the parent link and raised an error when parent_link=True was not specified. This patch allows to specify multiple OneToOneField's to the parent model. OneToOneField's without a custom related_name will raise fields.E304 and fields.E305 so this should warn users when they try to override the auto-created OneToOneField. --- django/db/models/base.py | 2 +- django/db/models/options.py | 6 +---- tests/invalid_models_tests/test_models.py | 25 +++++++++++------ .../test_relative_fields.py | 27 +++++++++++++++++++ tests/migrations/test_state.py | 1 + 5 files changed, 47 insertions(+), 14 deletions(-) diff --git a/django/db/models/base.py b/django/db/models/base.py index 8ea6c05ef948..24453e218a43 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -202,7 +202,7 @@ def __new__(cls, name, bases, attrs, **kwargs): continue # Locate OneToOneField instances. for field in base._meta.local_fields: - if isinstance(field, OneToOneField): + if isinstance(field, OneToOneField) and field.remote_field.parent_link: related = resolve_relation(new_class, field.remote_field.model) parent_links[make_model_tuple(related)] = field diff --git a/django/db/models/options.py b/django/db/models/options.py index a375f6ba1dd8..08c80bb6c852 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -5,7 +5,7 @@ from django.apps import apps from django.conf import settings -from django.core.exceptions import FieldDoesNotExist, ImproperlyConfigured +from django.core.exceptions import FieldDoesNotExist from django.db import connections from django.db.models import Manager from django.db.models.fields import AutoField @@ -251,10 +251,6 @@ def _prepare(self, model): field = already_created[0] field.primary_key = True self.setup_pk(field) - if not field.remote_field.parent_link: - raise ImproperlyConfigured( - 'Add parent_link=True to %s.' % field, - ) else: auto = AutoField(verbose_name='ID', primary_key=True, auto_created=True) model.add_to_class('id', auto) diff --git a/tests/invalid_models_tests/test_models.py b/tests/invalid_models_tests/test_models.py index 60b89b6f2ec4..ec2d345d5a9c 100644 --- a/tests/invalid_models_tests/test_models.py +++ b/tests/invalid_models_tests/test_models.py @@ -3,7 +3,6 @@ from django.conf import settings from django.core.checks import Error, Warning from django.core.checks.model_checks import _check_lazy_references -from django.core.exceptions import ImproperlyConfigured from django.db import connection, connections, models from django.db.models.functions import Lower from django.db.models.signals import post_init @@ -1006,14 +1005,24 @@ class ShippingMethodPrice(models.Model): self.assertEqual(ShippingMethod.check(), []) - def test_missing_parent_link(self): - msg = 'Add parent_link=True to invalid_models_tests.ParkingLot.parent.' - with self.assertRaisesMessage(ImproperlyConfigured, msg): - class Place(models.Model): - pass + def test_onetoone_with_parent_model(self): + class Place(models.Model): + pass + + class ParkingLot(Place): + other_place = models.OneToOneField(Place, models.CASCADE, related_name='other_parking') + + self.assertEqual(ParkingLot.check(), []) + + def test_onetoone_with_explicit_parent_link_parent_model(self): + class Place(models.Model): + pass + + class ParkingLot(Place): + place = models.OneToOneField(Place, models.CASCADE, parent_link=True, primary_key=True) + other_place = models.OneToOneField(Place, models.CASCADE, related_name='other_parking') - class ParkingLot(Place): - parent = models.OneToOneField(Place, models.CASCADE) + self.assertEqual(ParkingLot.check(), []) def test_m2m_table_name_clash(self): class Foo(models.Model): diff --git a/tests/invalid_models_tests/test_relative_fields.py b/tests/invalid_models_tests/test_relative_fields.py index 4ac0c547a9e3..786573672f38 100644 --- a/tests/invalid_models_tests/test_relative_fields.py +++ b/tests/invalid_models_tests/test_relative_fields.py @@ -1291,6 +1291,33 @@ class Model(models.Model): ), ]) + def test_clash_parent_link(self): + class Parent(models.Model): + pass + + class Child(Parent): + other_parent = models.OneToOneField(Parent, models.CASCADE) + + errors = [ + ('fields.E304', 'accessor', 'parent_ptr', 'other_parent'), + ('fields.E305', 'query name', 'parent_ptr', 'other_parent'), + ('fields.E304', 'accessor', 'other_parent', 'parent_ptr'), + ('fields.E305', 'query name', 'other_parent', 'parent_ptr'), + ] + self.assertEqual(Child.check(), [ + Error( + "Reverse %s for 'Child.%s' clashes with reverse %s for " + "'Child.%s'." % (attr, field_name, attr, clash_name), + hint=( + "Add or change a related_name argument to the definition " + "for 'Child.%s' or 'Child.%s'." % (field_name, clash_name) + ), + obj=Child._meta.get_field(field_name), + id=error_id, + ) + for error_id, attr, field_name, clash_name in errors + ]) + @isolate_apps('invalid_models_tests') class M2mThroughFieldsTests(SimpleTestCase): diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py index 267b75c811c2..ef198eb0b687 100644 --- a/tests/migrations/test_state.py +++ b/tests/migrations/test_state.py @@ -345,6 +345,7 @@ def test_render(self): 'migrations.Tag', models.CASCADE, auto_created=True, + parent_link=True, primary_key=True, to_field='id', serialize=False, From 1e0dcd6c8bfa4519c21014c73eb510620dd1a000 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Thu, 16 Jan 2020 08:27:04 +0100 Subject: [PATCH 29/34] Used constant instead of hard-coded value for recursive relationship. --- django/db/models/fields/related.py | 34 ++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index f6c5ae258597..75b533b0a86c 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -606,8 +606,11 @@ def resolve_related_fields(self): for index in range(len(self.from_fields)): from_field_name = self.from_fields[index] to_field_name = self.to_fields[index] - from_field = (self if from_field_name == 'self' - else self.opts.get_field(from_field_name)) + from_field = ( + self + if from_field_name == RECURSIVE_RELATIONSHIP_CONSTANT + else self.opts.get_field(from_field_name) + ) to_field = (self.remote_field.model._meta.pk if to_field_name is None else self.remote_field.model._meta.get_field(to_field_name)) related_fields.append((from_field, to_field)) @@ -810,8 +813,13 @@ def __init__(self, to, on_delete, related_name=None, related_query_name=None, ) kwargs.setdefault('db_index', True) - super().__init__(to, on_delete, from_fields=['self'], to_fields=[to_field], **kwargs) - + super().__init__( + to, + on_delete, + from_fields=[RECURSIVE_RELATIONSHIP_CONSTANT], + to_fields=[to_field], + **kwargs, + ) self.db_constraint = db_constraint def check(self, **kwargs): @@ -1276,8 +1284,11 @@ def _check_relationship_model(self, from_model=None, **kwargs): "through_fields keyword argument.") % (self, from_model_name), hint=( 'If you want to create a recursive relationship, ' - 'use ForeignKey("self", symmetrical=False, through="%s").' - ) % relationship_model_name, + 'use ForeignKey("%s", symmetrical=False, through="%s").' + ) % ( + RECURSIVE_RELATIONSHIP_CONSTANT, + relationship_model_name, + ), obj=self, id='fields.E334', ) @@ -1293,8 +1304,11 @@ def _check_relationship_model(self, from_model=None, **kwargs): "through_fields keyword argument." % (self, to_model_name), hint=( 'If you want to create a recursive relationship, ' - 'use ForeignKey("self", symmetrical=False, through="%s").' - ) % relationship_model_name, + 'use ForeignKey("%s", symmetrical=False, through="%s").' + ) % ( + RECURSIVE_RELATIONSHIP_CONSTANT, + relationship_model_name, + ), obj=self, id='fields.E335', ) @@ -1561,7 +1575,9 @@ def contribute_to_class(self, cls, name, **kwargs): # automatically. The funky name reduces the chance of an accidental # clash. if self.remote_field.symmetrical and ( - self.remote_field.model == "self" or self.remote_field.model == cls._meta.object_name): + self.remote_field.model == RECURSIVE_RELATIONSHIP_CONSTANT or + self.remote_field.model == cls._meta.object_name + ): self.remote_field.related_name = "%s_rel_+" % name elif self.remote_field.is_hidden(): # If the backwards relation is disabled, replace the original From d08d4f464ab11cc226d4e197c0f43e26a7fd2961 Mon Sep 17 00:00:00 2001 From: Flavio Curella Date: Thu, 26 Sep 2019 11:41:38 -0700 Subject: [PATCH 30/34] Fixed #30765 -- Made cache_page decorator take precedence over max-age Cache-Control directive. --- django/middleware/cache.py | 23 ++++++++++++++--------- django/views/decorators/cache.py | 2 +- docs/releases/3.1.txt | 4 ++++ docs/topics/cache.txt | 8 ++++++++ tests/cache/tests.py | 23 +++++++++++++++++++++++ 5 files changed, 50 insertions(+), 10 deletions(-) diff --git a/django/middleware/cache.py b/django/middleware/cache.py index 6b320f1db5ad..9705270b5927 100644 --- a/django/middleware/cache.py +++ b/django/middleware/cache.py @@ -63,6 +63,7 @@ class UpdateCacheMiddleware(MiddlewareMixin): """ def __init__(self, get_response=None): self.cache_timeout = settings.CACHE_MIDDLEWARE_SECONDS + self.page_timeout = None self.key_prefix = settings.CACHE_MIDDLEWARE_KEY_PREFIX self.cache_alias = settings.CACHE_MIDDLEWARE_ALIAS self.cache = caches[self.cache_alias] @@ -89,15 +90,18 @@ def process_response(self, request, response): if 'private' in response.get('Cache-Control', ()): return response - # Try to get the timeout from the "max-age" section of the "Cache- - # Control" header before reverting to using the default cache_timeout - # length. - timeout = get_max_age(response) + # Page timeout takes precedence over the "max-age" and the default + # cache timeout. + timeout = self.page_timeout if timeout is None: - timeout = self.cache_timeout - elif timeout == 0: - # max-age was set to 0, don't bother caching. - return response + # The timeout from the "max-age" section of the "Cache-Control" + # header takes precedence over the default cache timeout. + timeout = get_max_age(response) + if timeout is None: + timeout = self.cache_timeout + elif timeout == 0: + # max-age was set to 0, don't cache. + return response patch_response_headers(response, timeout) if timeout and response.status_code == 200: cache_key = learn_cache_key(request, response, timeout, self.key_prefix, cache=self.cache) @@ -160,7 +164,7 @@ class CacheMiddleware(UpdateCacheMiddleware, FetchFromCacheMiddleware): Also used as the hook point for the cache decorator, which is generated using the decorator-from-middleware utility. """ - def __init__(self, get_response=None, cache_timeout=None, **kwargs): + def __init__(self, get_response=None, cache_timeout=None, page_timeout=None, **kwargs): self.get_response = get_response # We need to differentiate between "provided, but using default value", # and "not provided". If the value is provided using a default, then @@ -186,4 +190,5 @@ def __init__(self, get_response=None, cache_timeout=None, **kwargs): if cache_timeout is None: cache_timeout = settings.CACHE_MIDDLEWARE_SECONDS self.cache_timeout = cache_timeout + self.page_timeout = page_timeout self.cache = caches[self.cache_alias] diff --git a/django/views/decorators/cache.py b/django/views/decorators/cache.py index 9658bd6ba257..773cf0c2c674 100644 --- a/django/views/decorators/cache.py +++ b/django/views/decorators/cache.py @@ -20,7 +20,7 @@ def cache_page(timeout, *, cache=None, key_prefix=None): into account on caching -- just like the middleware does. """ return decorator_from_middleware_with_args(CacheMiddleware)( - cache_timeout=timeout, cache_alias=cache, key_prefix=key_prefix + page_timeout=timeout, cache_alias=cache, key_prefix=key_prefix, ) diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index 9e8dff3456e6..064aadd34ace 100644 --- a/docs/releases/3.1.txt +++ b/docs/releases/3.1.txt @@ -430,6 +430,10 @@ Miscellaneous used with :setting:`DEFAULT_EXCEPTION_REPORTER_FILTER` needs to inherit from :class:`django.views.debug.SafeExceptionReporterFilter`. +* The cache timeout set by :func:`~django.views.decorators.cache.cache_page` + decorator now takes precedence over the ``max-age`` directive from the + ``Cache-Control`` header. + .. _deprecated-features-3.1: Features deprecated in 3.1 diff --git a/docs/topics/cache.txt b/docs/topics/cache.txt index 29ea8fb001f2..bc6b96ecfe14 100644 --- a/docs/topics/cache.txt +++ b/docs/topics/cache.txt @@ -570,6 +570,9 @@ minutes. (Note that we've written it as ``60 * 15`` for the purpose of readability. ``60 * 15`` will be evaluated to ``900`` -- that is, 15 minutes multiplied by 60 seconds per minute.) +The cache timeout set by ``cache_page`` takes precedence over the ``max-age`` +directive from the ``Cache-Control`` header. + The per-view cache, like the per-site cache, is keyed off of the URL. If multiple URLs point at the same view, each URL will be cached separately. Continuing the ``my_view`` example, if your URLconf looks like this:: @@ -605,6 +608,11 @@ The ``key_prefix`` and ``cache`` arguments may be specified together. The ``key_prefix`` argument and the :setting:`KEY_PREFIX ` specified under :setting:`CACHES` will be concatenated. +.. versionchanged:: 3.1 + + In older versions, the ``max-age`` directive from the ``Cache-Control`` + header had precedence over the cache timeout set by ``cache_page``. + Specifying per-view cache in the URLconf ---------------------------------------- diff --git a/tests/cache/tests.py b/tests/cache/tests.py index e99ab408a15e..141d782203b9 100644 --- a/tests/cache/tests.py +++ b/tests/cache/tests.py @@ -2188,6 +2188,29 @@ def test_view_decorator(self): response = other_with_prefix_view(request, '16') self.assertEqual(response.content, b'Hello World 16') + def test_cache_page_timeout(self): + # Page timeout takes precedence over the "max-age" section of the + # "Cache-Control". + tests = [ + (1, 3), # max_age < page_timeout. + (3, 1), # max_age > page_timeout. + ] + for max_age, page_timeout in tests: + with self.subTest(max_age=max_age, page_timeout=page_timeout): + view = cache_page(timeout=page_timeout)( + cache_control(max_age=max_age)(hello_world_view) + ) + request = self.factory.get('/view/') + response = view(request, '1') + self.assertEqual(response.content, b'Hello World 1') + time.sleep(1) + response = view(request, '2') + self.assertEqual( + response.content, + b'Hello World 1' if page_timeout > max_age else b'Hello World 2', + ) + cache.clear() + def test_cached_control_private_not_cached(self): """Responses with 'Cache-Control: private' are not cached.""" view_with_private_cache = cache_page(3)(cache_control(private=True)(hello_world_view)) From 266c853e10de50ce75af8be834647fda2c5fc2a0 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Thu, 16 Jan 2020 14:34:54 +0100 Subject: [PATCH 31/34] Fixed #31162 -- Prevented error logs when using WKT strings in lookups. Thanks dbxnr for the initial patch. Regression in 6f44f714c92d2966dca390ebd3054e5fb0bb0c80. --- django/contrib/gis/gdal/raster/source.py | 9 ++++++++- tests/gis_tests/gdal_tests/test_raster.py | 5 +++++ tests/gis_tests/geoapp/tests.py | 6 ++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/django/contrib/gis/gdal/raster/source.py b/django/contrib/gis/gdal/raster/source.py index 05475434c426..33d8b3069f08 100644 --- a/django/contrib/gis/gdal/raster/source.py +++ b/django/contrib/gis/gdal/raster/source.py @@ -73,6 +73,13 @@ def __init__(self, ds_input, write=False): # If input is a valid file path, try setting file as source. if isinstance(ds_input, str): + if ( + not ds_input.startswith(VSI_FILESYSTEM_BASE_PATH) and + not os.path.exists(ds_input) + ): + raise GDALException( + 'Unable to read raster source input "%s".' % ds_input + ) try: # GDALOpen will auto-detect the data source type. self._ptr = capi.open_ds(force_bytes(ds_input), self._write) @@ -225,7 +232,7 @@ def vsi_buffer(self): @cached_property def is_vsi_based(self): - return self.name.startswith(VSI_FILESYSTEM_BASE_PATH) + return self._ptr and self.name.startswith(VSI_FILESYSTEM_BASE_PATH) @property def name(self): diff --git a/tests/gis_tests/gdal_tests/test_raster.py b/tests/gis_tests/gdal_tests/test_raster.py index f3477181b49b..018871efc3a2 100644 --- a/tests/gis_tests/gdal_tests/test_raster.py +++ b/tests/gis_tests/gdal_tests/test_raster.py @@ -156,6 +156,11 @@ def test_file_based_raster_creation(self): else: self.assertEqual(restored_raster.bands[0].data(), self.rs.bands[0].data()) + def test_nonexistent_file(self): + msg = 'Unable to read raster source input "nonexistent.tif".' + with self.assertRaisesMessage(GDALException, msg): + GDALRaster('nonexistent.tif') + def test_vsi_raster_creation(self): # Open a raster as a file object. with open(self.rs_path, 'rb') as dat: diff --git a/tests/gis_tests/geoapp/tests.py b/tests/gis_tests/geoapp/tests.py index aab83a161a82..f66e315e73b1 100644 --- a/tests/gis_tests/geoapp/tests.py +++ b/tests/gis_tests/geoapp/tests.py @@ -430,6 +430,12 @@ def test_null_geometries_excluded_in_lookups(self): with self.subTest(lookup=lookup): self.assertNotIn(null, State.objects.filter(**{'poly__%s' % lookup: geom})) + def test_wkt_string_in_lookup(self): + # Valid WKT strings don't emit error logs. + with self.assertRaisesMessage(AssertionError, 'no logs'): + with self.assertLogs('django.contrib.gis', 'ERROR'): + State.objects.filter(poly__intersects='LINESTRING(0 0, 1 1, 5 5)') + @skipUnlessDBFeature("supports_relate_lookup") def test_relate_lookup(self): "Testing the 'relate' lookup type." From a5a28de89dabfa03302a5893102b6f1a7c7861a1 Mon Sep 17 00:00:00 2001 From: Anael Mobilia Date: Thu, 16 Jan 2020 14:51:27 +0100 Subject: [PATCH 32/34] Added apps.py to project from tutorials in reusable apps docs. --- docs/intro/reusable-apps.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/intro/reusable-apps.txt b/docs/intro/reusable-apps.txt index 58069dc10a1f..c5e94debc6e9 100644 --- a/docs/intro/reusable-apps.txt +++ b/docs/intro/reusable-apps.txt @@ -66,6 +66,7 @@ After the previous tutorials, our project should look like this:: polls/ __init__.py admin.py + apps.py migrations/ __init__.py 0001_initial.py From 13e4abf83e5129b44c771b2204809792087abda4 Mon Sep 17 00:00:00 2001 From: Pavel Lysak Date: Sat, 7 Sep 2019 20:08:12 +0300 Subject: [PATCH 33/34] Fixed #30752 -- Allowed using ExceptionReporter subclasses in error reports. --- django/conf/global_settings.py | 4 ++ django/utils/log.py | 2 +- django/views/debug.py | 7 +++- docs/howto/error-reporting.txt | 56 ++++++++++++++++++++++++++++ docs/ref/settings.txt | 14 +++++++ docs/releases/3.1.txt | 4 ++ tests/view_tests/tests/test_debug.py | 9 +++++ tests/view_tests/urls.py | 1 + tests/view_tests/views.py | 18 ++++++++- 9 files changed, 112 insertions(+), 3 deletions(-) diff --git a/django/conf/global_settings.py b/django/conf/global_settings.py index 830ba25408b4..09c9b95d26d5 100644 --- a/django/conf/global_settings.py +++ b/django/conf/global_settings.py @@ -567,6 +567,10 @@ def gettext_noop(s): # Custom logging configuration. LOGGING = {} +# Default exception reporter class used in case none has been +# specifically assigned to the HttpRequest instance. +DEFAULT_EXCEPTION_REPORTER = 'django.views.debug.ExceptionReporter' + # Default exception reporter filter class used in case none has been # specifically assigned to the HttpRequest instance. DEFAULT_EXCEPTION_REPORTER_FILTER = 'django.views.debug.SafeExceptionReporterFilter' diff --git a/django/utils/log.py b/django/utils/log.py index e40d87159c68..717c15814cdb 100644 --- a/django/utils/log.py +++ b/django/utils/log.py @@ -86,7 +86,7 @@ def __init__(self, include_html=False, email_backend=None, reporter_class=None): super().__init__() self.include_html = include_html self.email_backend = email_backend - self.reporter_class = import_string(reporter_class or 'django.views.debug.ExceptionReporter') + self.reporter_class = import_string(reporter_class or settings.DEFAULT_EXCEPTION_REPORTER) def emit(self, record): try: diff --git a/django/views/debug.py b/django/views/debug.py index 13dabf165bc0..1761d6904a1c 100644 --- a/django/views/debug.py +++ b/django/views/debug.py @@ -47,7 +47,7 @@ def technical_500_response(request, exc_type, exc_value, tb, status_code=500): Create a technical server error response. The last three arguments are the values returned from sys.exc_info() and friends. """ - reporter = ExceptionReporter(request, exc_type, exc_value, tb) + reporter = get_exception_reporter_class(request)(request, exc_type, exc_value, tb) if request.is_ajax(): text = reporter.get_traceback_text() return HttpResponse(text, status=status_code, content_type='text/plain; charset=utf-8') @@ -67,6 +67,11 @@ def get_exception_reporter_filter(request): return getattr(request, 'exception_reporter_filter', default_filter) +def get_exception_reporter_class(request): + default_exception_reporter_class = import_string(settings.DEFAULT_EXCEPTION_REPORTER) + return getattr(request, 'exception_reporter_class', default_exception_reporter_class) + + class SafeExceptionReporterFilter: """ Use annotations made by the sensitive_post_parameters and diff --git a/docs/howto/error-reporting.txt b/docs/howto/error-reporting.txt index e145897b2a38..13043cf3878f 100644 --- a/docs/howto/error-reporting.txt +++ b/docs/howto/error-reporting.txt @@ -305,6 +305,62 @@ following attributes and methods: traceback frame. Sensitive values are replaced with :attr:`cleansed_substitute`. +.. versionadded:: 3.1 + +If you need to customize error reports beyond filtering you may specify a +custom error reporter class by defining the +:setting:`DEFAULT_EXCEPTION_REPORTER` setting:: + + DEFAULT_EXCEPTION_REPORTER = 'path.to.your.CustomExceptionReporter' + +The exception reporter is responsible for compiling the exception report data, +and formatting it as text or HTML appropriately. (The exception reporter uses +:setting:`DEFAULT_EXCEPTION_REPORTER_FILTER` when preparing the exception +report data.) + +Your custom reporter class needs to inherit from +:class:`django.views.debug.ExceptionReporter`. + +.. class:: ExceptionReporter + + .. method:: get_traceback_data() + + Return a dictionary containing traceback information. + + This is the main extension point for customizing exception reports, for + example:: + + from django.views.debug import ExceptionReporter + + + class CustomExceptionReporter(ExceptionReporter): + def get_traceback_data(self): + data = super().get_traceback_data() + # ... remove/add something here ... + return data + + .. method:: get_traceback_html() + + Return HTML version of exception report. + + Used for HTML version of debug 500 HTTP error page. + + .. method:: get_traceback_text() + + Return plain text version of exception report. + + Used for plain text version of debug 500 HTTP error page and email + reports. + +As with the filter class, you may control which exception reporter class to use +within any given view by setting the ``HttpRequest``’s +``exception_reporter_class`` attribute:: + + def my_view(request): + if request.user.is_authenticated: + request.exception_reporter_class = CustomExceptionReporter() + ... + .. seealso:: You can also set up custom error reporting by writing a custom piece of diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index 51bbba35f0fd..917ffeebb463 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -1250,6 +1250,19 @@ Default: ``'utf-8'`` Default charset to use for all ``HttpResponse`` objects, if a MIME type isn't manually specified. Used when constructing the ``Content-Type`` header. +.. setting:: DEFAULT_EXCEPTION_REPORTER + +``DEFAULT_EXCEPTION_REPORTER`` +------------------------------ + +.. versionadded:: 3.1 + +Default: ``'``:class:`django.views.debug.ExceptionReporter`\ ``'`` + +Default exception reporter class to be used if none has been assigned to the +:class:`~django.http.HttpRequest` instance yet. See +:ref:`custom-error-reports`. + .. setting:: DEFAULT_EXCEPTION_REPORTER_FILTER ``DEFAULT_EXCEPTION_REPORTER_FILTER`` @@ -3537,6 +3550,7 @@ Email Error reporting --------------- +* :setting:`DEFAULT_EXCEPTION_REPORTER` * :setting:`DEFAULT_EXCEPTION_REPORTER_FILTER` * :setting:`IGNORABLE_404_URLS` * :setting:`MANAGERS` diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index 064aadd34ace..ced98c2e470a 100644 --- a/docs/releases/3.1.txt +++ b/docs/releases/3.1.txt @@ -173,6 +173,10 @@ Error Reporting :setting:`DEFAULT_EXCEPTION_REPORTER_FILTER` when applying settings filtering. +* The new :setting:`DEFAULT_EXCEPTION_REPORTER` allows providing a + :class:`django.views.debug.ExceptionReporter` subclass to customize exception + report generation. See :ref:`custom-error-reports` for details. + File Storage ~~~~~~~~~~~~ diff --git a/tests/view_tests/tests/test_debug.py b/tests/view_tests/tests/test_debug.py index 020af1a19c0e..07d8208e19d9 100644 --- a/tests/view_tests/tests/test_debug.py +++ b/tests/view_tests/tests/test_debug.py @@ -249,6 +249,15 @@ def test_technical_404_converter_raise_404(self): response = self.client.get('/path-post/1/') self.assertContains(response, 'Page not found', status_code=404) + def test_exception_reporter_from_request(self): + response = self.client.get('/custom_reporter_class_view/') + self.assertContains(response, 'custom traceback text', status_code=500) + + @override_settings(DEFAULT_EXCEPTION_REPORTER='view_tests.views.CustomExceptionReporter') + def test_exception_reporter_from_settings(self): + response = self.client.get('/raises500/') + self.assertContains(response, 'custom traceback text', status_code=500) + class DebugViewQueriesAllowedTests(SimpleTestCase): # May need a query to initialize MySQL connection diff --git a/tests/view_tests/urls.py b/tests/view_tests/urls.py index 34415b06e0af..6c6f73467ad1 100644 --- a/tests/view_tests/urls.py +++ b/tests/view_tests/urls.py @@ -26,6 +26,7 @@ path('raises403/', views.raises403), path('raises404/', views.raises404), path('raises500/', views.raises500), + path('custom_reporter_class_view/', views.custom_reporter_class_view), path('technical404/', views.technical404, name='my404'), path('classbased404/', views.Http404View.as_view()), diff --git a/tests/view_tests/views.py b/tests/view_tests/views.py index ce0079a355d4..36c7bda4b4cb 100644 --- a/tests/view_tests/views.py +++ b/tests/view_tests/views.py @@ -10,7 +10,7 @@ from django.urls import get_resolver from django.views import View from django.views.debug import ( - SafeExceptionReporterFilter, technical_500_response, + ExceptionReporter, SafeExceptionReporterFilter, technical_500_response, ) from django.views.decorators.debug import ( sensitive_post_parameters, sensitive_variables, @@ -227,6 +227,22 @@ def custom_exception_reporter_filter_view(request): return technical_500_response(request, *exc_info) +class CustomExceptionReporter(ExceptionReporter): + custom_traceback_text = 'custom traceback text' + + def get_traceback_html(self): + return self.custom_traceback_text + + +def custom_reporter_class_view(request): + request.exception_reporter_class = CustomExceptionReporter + try: + raise Exception + except Exception: + exc_info = sys.exc_info() + return technical_500_response(request, *exc_info) + + class Klass: @sensitive_variables('sauce') From 2f9bde30e59cf74bd642f380c81c1afed1847412 Mon Sep 17 00:00:00 2001 From: matt ferrante Date: Mon, 6 Jan 2020 16:44:32 -0700 Subject: [PATCH 34/34] fix nested filtered relations --- django/db/models/sql/query.py | 26 ++++- docs/ref/models/querysets.txt | 11 -- tests/filtered_relation/tests.py | 173 +++++++++++++++++++++++++++---- 3 files changed, 174 insertions(+), 36 deletions(-) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 78c4f47b5bfa..cd304ea2b2c7 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -968,7 +968,6 @@ def join(self, join, reuse=None, reuse_with_filtered_relation=False): reuse_alias = reuse_aliases[-1] self.ref_alias(reuse_alias) return reuse_alias - # No reuse is possible, so we need a new alias. alias, _ = self.table_alias(join.table_name, create=True, filtered_relation=join.filtered_relation) if join.join_type: @@ -1408,13 +1407,23 @@ def build_filtered_relation_q(self, q_object, reuse, branch_negated=False, curre def add_filtered_relation(self, filtered_relation, alias): filtered_relation.alias = alias lookups = dict(get_children_from_q(filtered_relation.condition)) - for lookup in chain((filtered_relation.relation_name,), lookups): + relation_lookup_parts, relation_field_parts, _ = self.solve_lookup_type(filtered_relation.relation_name) + if relation_lookup_parts: + raise ValueError( + "FilteredRelation's relation_name doesn't support lookups " + "(got %r)." % filtered_relation.relation_name + ) + for lookup in chain(lookups): lookup_parts, field_parts, _ = self.solve_lookup_type(lookup) shift = 2 if not lookup_parts else 1 - if len(field_parts) > (shift + len(lookup_parts)): + lookup_path = field_parts[:-shift] + for _ in relation_field_parts: + if lookup_path: + lookup_path = lookup_path[1:] + if lookup_path: raise ValueError( "FilteredRelation's condition doesn't support nested " - "relations (got %r)." % lookup + "on clauses deeper than the relation (got %r for %r)." % (lookup, filtered_relation.relation_name) ) self._filtered_relations[filtered_relation.alias] = filtered_relation @@ -1448,7 +1457,14 @@ def names_to_path(self, names, opts, allow_many=True, fail_on_missing=False): field = self.annotation_select[name].output_field elif name in self._filtered_relations and pos == 0: filtered_relation = self._filtered_relations[name] - field = opts.get_field(filtered_relation.relation_name) + if LOOKUP_SEP in filtered_relation.relation_name: + parts = filtered_relation.relation_name.split(LOOKUP_SEP) + filtered_relation_path, field, _, _ = self.names_to_path( + parts, opts, allow_many, fail_on_missing + ) + path.extend(filtered_relation_path[:-1]) + else: + field = opts.get_field(filtered_relation.relation_name) if field is not None: # Fields that contain one-to-many relations with a generic # model (like a GenericForeignKey) cannot generate reverse diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index ba20c7516a57..02557878930f 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -3666,17 +3666,6 @@ operate on vegetarian pizzas. ``FilteredRelation`` doesn't support: -* Conditions that span relational fields. For example:: - - >>> Restaurant.objects.annotate( - ... pizzas_with_toppings_startswith_n=FilteredRelation( - ... 'pizzas__toppings', - ... condition=Q(pizzas__toppings__name__startswith='n'), - ... ), - ... ) - Traceback (most recent call last): - ... - ValueError: FilteredRelation's condition doesn't support nested relations (got 'pizzas__toppings__name__startswith'). * :meth:`.QuerySet.only` and :meth:`~.QuerySet.prefetch_related`. * A :class:`~django.contrib.contenttypes.fields.GenericForeignKey` inherited from a parent model. diff --git a/tests/filtered_relation/tests.py b/tests/filtered_relation/tests.py index 05869a2a957c..d37561cf809f 100644 --- a/tests/filtered_relation/tests.py +++ b/tests/filtered_relation/tests.py @@ -156,8 +156,15 @@ def test_with_multiple_filter(self): def test_multiple_times(self): self.assertSequenceEqual( Author.objects.annotate( - book_title_alice=FilteredRelation('book', condition=Q(book__title__icontains='alice')), - ).filter(book_title_alice__isnull=False).filter(book_title_alice__isnull=False).distinct(), + book_title_alice=FilteredRelation( + 'book', + condition=Q(book__title__icontains='alice') + ), + ).filter( + book_title_alice__isnull=False + ).filter( + book_title_alice__isnull=False + ).distinct(), [self.author1] ) @@ -248,10 +255,11 @@ def test_difference(self): self.assertSequenceEqual(qs1.difference(qs2), [self.author1]) def test_select_for_update(self): + qs = Author.objects.annotate( + book_jane=FilteredRelation('book', condition=Q(book__title__iexact='the book by jane a')), + ).filter(book_jane__isnull=False).select_for_update() self.assertSequenceEqual( - Author.objects.annotate( - book_jane=FilteredRelation('book', condition=Q(book__title__iexact='the book by jane a')), - ).filter(book_jane__isnull=False).select_for_update(), + qs, [self.author2] ) @@ -279,28 +287,153 @@ def test_as_subquery(self): qs = Author.objects.filter(id__in=inner_qs) self.assertSequenceEqual(qs, [self.author1]) - def test_with_foreign_key_error(self): + def test_with_nested_foreign_key(self): + qs = Author.objects.annotate( + book_editor_worked_with=FilteredRelation('book__editor', condition=Q(book__title__icontains='book')), + ).filter( + book_editor_worked_with__isnull=False + ).select_related( + 'book_editor_worked_with' + ).order_by( + 'pk', 'book_editor_worked_with__pk' + ).distinct() + + with self.assertNumQueries(1): + self.assertQuerysetEqual(qs, [ + (self.author1, self.editor_a), + (self.author2, self.editor_b), + ], lambda x: (x, x.book_editor_worked_with)) + + def test_with_nested_field(self): + qs = Author.objects.annotate( + book_editor_worked_with=FilteredRelation('book__editor', condition=Q(book__title__icontains='book')), + ).filter( + book_editor_worked_with__isnull=False + ).values( + 'name', 'book_editor_worked_with__name' + ).order_by( + 'name', 'book_editor_worked_with__name' + ).distinct() + + self.assertSequenceEqual(qs, [ + {'name': self.author1.name, 'book_editor_worked_with__name': self.editor_a.name}, + {'name': self.author2.name, 'book_editor_worked_with__name': self.editor_b.name}, + ]) + + def test_with_deep_nested_foreign_key(self): + qs = Book.objects.annotate( + author_favorite_book_editor=FilteredRelation( + 'author__favorite_books__editor', + condition=Q(author__favorite_books__title__icontains='Jane A') + ) + ).filter( + author_favorite_book_editor__isnull=False + ).select_related( + 'author_favorite_book_editor' + ).order_by( + 'pk', 'author_favorite_book_editor__pk' + ) + + with self.assertNumQueries(1): + self.assertQuerysetEqual(qs, [ + (self.book1, self.editor_b), + (self.book4, self.editor_b), + ], lambda x: (x, x.author_favorite_book_editor)) + + def test_with_foreign_key_on_condition_deeper_than_relationship_error(self): msg = ( - "FilteredRelation's condition doesn't support nested relations " - "(got 'author__favorite_books__author')." + "FilteredRelation's condition doesn't support nested " + "on clauses deeper than the relation (got 'book__editor__name__icontains' for 'book')." ) with self.assertRaisesMessage(ValueError, msg): - list(Book.objects.annotate( - alice_favorite_books=FilteredRelation( - 'author__favorite_books', - condition=Q(author__favorite_books__author=self.author1), - ) - )) + qs = Author.objects.annotate( + book_edited_by_b=FilteredRelation('book', condition=Q(book__editor__name__icontains='b')), + ).filter(book_edited_by_b__isnull=False).distinct() + list(qs) - def test_with_foreign_key_on_condition_error(self): + def test_with_foreign_key_relation_name_lookup(self): msg = ( - "FilteredRelation's condition doesn't support nested relations " - "(got 'book__editor__name__icontains')." + "FilteredRelation's relation_name doesn't support lookups " + "(got 'book__title__icontains')." ) with self.assertRaisesMessage(ValueError, msg): - list(Author.objects.annotate( - book_edited_by_b=FilteredRelation('book', condition=Q(book__editor__name__icontains='b')), - )) + qs = Author.objects.annotate( + book_edited_by_b=FilteredRelation( + 'book__title__icontains', + condition=Q(book__editor__name__icontains='b') + ), + ).filter(book_edited_by_b__isnull=False).distinct() + list(qs) + + def test_with_nested_filtered_relationship(self): + qs = Book.objects.annotate( + author_favorite_books=FilteredRelation( + 'author__favorite_books', + condition=Q(author__favorite_books__title__icontains='book') + ) + ) + qs = qs.values( + 'title', 'author__name', 'author_favorite_books__title' + ).order_by( + 'title', 'author__name', 'author_favorite_books__title' + ) + + self.assertSequenceEqual(qs, [ + { + 'title': self.book1.title, 'author__name': self.author1.name, + 'author_favorite_books__title': self.book2.title, + }, + { + 'title': self.book1.title, 'author__name': self.author1.name, + 'author_favorite_books__title': self.book3.title, + }, + { + 'title': self.book4.title, 'author__name': self.author1.name, + 'author_favorite_books__title': self.book2.title, + }, + { + 'title': self.book4.title, 'author__name': self.author1.name, + 'author_favorite_books__title': self.book3.title, + }, + { + 'title': self.book2.title, 'author__name': self.author2.name, + 'author_favorite_books__title': None, + }, + { + 'title': self.book3.title, 'author__name': self.author2.name, + 'author_favorite_books__title': None, + }, + ]) + + def test_with_composed_relations(self): + qs = Author.objects.annotate( + my_books=FilteredRelation( + 'book', condition=Q(book__title__icontains='book') + ) + ) + qs = qs.annotate( + preferred_by_authors=FilteredRelation( + 'my_books__preferred_by_authors', + condition=Q( + my_books__preferred_by_authors__name__icontains='e' + ) + ) + ) + qs = qs.annotate( + author=F('name'), + book_title=F("my_books__title"), + preferred_by_author_name=F("preferred_by_authors__name"), + ) + qs = qs.values( + 'author', 'book_title', 'preferred_by_author_name' + ).order_by( + 'author', 'book_title', 'preferred_by_author_name' + ).distinct() + self.assertSequenceEqual(qs, [ + {'author': 'Alice', 'book_title': 'The book by Alice', 'preferred_by_author_name': None}, + {'author': 'Jane', 'book_title': 'The book by Jane A', 'preferred_by_author_name': 'Alice'}, + {'author': 'Jane', 'book_title': 'The book by Jane B', 'preferred_by_author_name': 'Alice'} + ]) def test_with_empty_relation_name_error(self): with self.assertRaisesMessage(ValueError, 'relation_name cannot be empty.'):