From 11e327a3ff84e16ceace13ea6ec408a93ca9e72c Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Fri, 15 Nov 2019 16:20:07 -0500 Subject: [PATCH] Fixed #30988 -- Deprecated the InvalidQuery exception. It was barely documented without pointers at its defining location and was abused to prevent misuse of the QuerySet field deferring feature. --- django/db/models/query.py | 6 +++-- django/db/models/query_utils.py | 37 ++++++++++++++++++++++++++----- docs/internals/deprecation.txt | 3 +++ docs/releases/3.1.txt | 5 +++++ docs/topics/db/sql.txt | 5 +++-- tests/defer/tests.py | 6 ++--- tests/queries/test_deprecation.py | 30 +++++++++++++++++++++++++ tests/raw_query/tests.py | 5 +++-- 8 files changed, 82 insertions(+), 15 deletions(-) create mode 100644 tests/queries/test_deprecation.py diff --git a/django/db/models/query.py b/django/db/models/query.py index bb0bc4da63..08a6f421d1 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -21,7 +21,7 @@ from django.db.models.deletion import Collector from django.db.models.expressions import Case, Expression, F, Value, When from django.db.models.fields import AutoField from django.db.models.functions import Cast, Trunc -from django.db.models.query_utils import FilteredRelation, InvalidQuery, Q +from django.db.models.query_utils import FilteredRelation, Q from django.db.models.sql.constants import CURSOR, GET_ITERATOR_CHUNK_SIZE from django.db.utils import NotSupportedError from django.utils import timezone @@ -1455,7 +1455,9 @@ class RawQuerySet: try: model_init_names, model_init_pos, annotation_fields = self.resolve_model_init_order() if self.model._meta.pk.attname not in model_init_names: - raise InvalidQuery('Raw query must include the primary key') + raise exceptions.FieldDoesNotExist( + 'Raw query must include the primary key' + ) model_cls = self.model fields = [self.model_fields.get(c) for c in self.columns] converters = compiler.get_converters([ diff --git a/django/db/models/query_utils.py b/django/db/models/query_utils.py index a9abf8d025..a6b7154541 100644 --- a/django/db/models/query_utils.py +++ b/django/db/models/query_utils.py @@ -8,10 +8,13 @@ circular import difficulties. import copy import functools import inspect +import warnings from collections import namedtuple +from django.core.exceptions import FieldDoesNotExist, FieldError from django.db.models.constants import LOOKUP_SEP from django.utils import tree +from django.utils.deprecation import RemovedInDjango40Warning # PathInfo is used when converting lookups (fk__somecol). The contents # describe the relation in Model terms (model Options and Fields for both @@ -19,8 +22,29 @@ from django.utils import tree PathInfo = namedtuple('PathInfo', 'from_opts to_opts target_fields join_field m2m direct filtered_relation') -class InvalidQuery(Exception): - """The query passed to raw() isn't a safe query to use with raw().""" +class InvalidQueryType(type): + @property + def _subclasses(self): + return (FieldDoesNotExist, FieldError) + + def __warn(self): + warnings.warn( + 'The InvalidQuery exception class is deprecated. Use ' + 'FieldDoesNotExist or FieldError instead.', + category=RemovedInDjango40Warning, + stacklevel=4, + ) + + def __instancecheck__(self, instance): + self.__warn() + return isinstance(instance, self._subclasses) or super().__instancecheck__(instance) + + def __subclasscheck__(self, subclass): + self.__warn() + return issubclass(subclass, self._subclasses) or super().__subclasscheck__(subclass) + + +class InvalidQuery(Exception, metaclass=InvalidQueryType): pass @@ -233,10 +257,11 @@ def select_related_descend(field, restricted, requested, load_fields, reverse=Fa if load_fields: if field.attname not in load_fields: if restricted and field.name in requested: - raise InvalidQuery("Field %s.%s cannot be both deferred" - " and traversed using select_related" - " at the same time." % - (field.model._meta.object_name, field.name)) + msg = ( + 'Field %s.%s cannot be both deferred and traversed using ' + 'select_related at the same time.' + ) % (field.model._meta.object_name, field.name) + raise FieldError(msg) return True diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index b2a21b12a9..044d35bfd5 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -39,6 +39,9 @@ details on these changes. * The undocumented usage of the :lookup:`isnull` lookup with non-boolean values as the right-hand side will no longer be allowed. +* The ``django.db.models.query_utils.InvalidQuery`` exception class will be + removed. + See the :ref:`Django 3.1 release notes ` for more details on these changes. diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index a7c9caa31e..00d620b045 100644 --- a/docs/releases/3.1.txt +++ b/docs/releases/3.1.txt @@ -330,6 +330,11 @@ Miscellaneous * The undocumented usage of the :lookup:`isnull` lookup with non-boolean values as the right-hand side is deprecated, use ``True`` or ``False`` instead. +* The barely documented ``django.db.models.query_utils.InvalidQuery`` exception + class is deprecated in favor of + :class:`~django.core.exceptions.FieldDoesNotExist` and + :class:`~django.core.exceptions.FieldError`. + .. _removed-features-3.1: Features removed in 3.1 diff --git a/docs/topics/db/sql.txt b/docs/topics/db/sql.txt index f0af7a4b44..355c7366ca 100644 --- a/docs/topics/db/sql.txt +++ b/docs/topics/db/sql.txt @@ -170,8 +170,9 @@ last names were both retrieved on demand when they were printed. There is only one field that you can't leave out - the primary key field. Django uses the primary key to identify model instances, so it -must always be included in a raw query. An ``InvalidQuery`` exception -will be raised if you forget to include the primary key. +must always be included in a raw query. A +:class:`~django.core.exceptions.FieldDoesNotExist` exception will be raised if +you forget to include the primary key. Adding annotations ------------------ diff --git a/tests/defer/tests.py b/tests/defer/tests.py index b67c5d9a5e..b85475f74f 100644 --- a/tests/defer/tests.py +++ b/tests/defer/tests.py @@ -1,4 +1,4 @@ -from django.db.models.query_utils import InvalidQuery +from django.core.exceptions import FieldError from django.test import TestCase from .models import ( @@ -113,7 +113,7 @@ class DeferTests(AssertionMixin, TestCase): 'Field Primary.related cannot be both deferred and traversed ' 'using select_related at the same time.' ) - with self.assertRaisesMessage(InvalidQuery, msg): + with self.assertRaisesMessage(FieldError, msg): Primary.objects.defer("related").select_related("related")[0] def test_only_select_related_raises_invalid_query(self): @@ -121,7 +121,7 @@ class DeferTests(AssertionMixin, TestCase): 'Field Primary.related cannot be both deferred and traversed using ' 'select_related at the same time.' ) - with self.assertRaisesMessage(InvalidQuery, msg): + with self.assertRaisesMessage(FieldError, msg): Primary.objects.only("name").select_related("related")[0] def test_defer_foreign_keys_are_deferred_and_not_traversed(self): diff --git a/tests/queries/test_deprecation.py b/tests/queries/test_deprecation.py new file mode 100644 index 0000000000..1dc8031fa7 --- /dev/null +++ b/tests/queries/test_deprecation.py @@ -0,0 +1,30 @@ +from contextlib import contextmanager + +from django.core.exceptions import FieldDoesNotExist, FieldError +from django.db.models.query_utils import InvalidQuery +from django.test import SimpleTestCase +from django.utils.deprecation import RemovedInDjango40Warning + + +class InvalidQueryTests(SimpleTestCase): + @contextmanager + def assert_warns(self): + msg = ( + 'The InvalidQuery exception class is deprecated. Use ' + 'FieldDoesNotExist or FieldError instead.' + ) + with self.assertWarnsMessage(RemovedInDjango40Warning, msg): + yield + + def test_type(self): + self.assertIsInstance(InvalidQuery(), InvalidQuery) + + def test_isinstance(self): + for exception in (FieldError, FieldDoesNotExist): + with self.assert_warns(), self.subTest(exception.__name__): + self.assertIsInstance(exception(), InvalidQuery) + + def test_issubclass(self): + for exception in (FieldError, FieldDoesNotExist, InvalidQuery): + with self.assert_warns(), self.subTest(exception.__name__): + self.assertIs(issubclass(exception, InvalidQuery), True) diff --git a/tests/raw_query/tests.py b/tests/raw_query/tests.py index 703a6b311e..bc9ed47c81 100644 --- a/tests/raw_query/tests.py +++ b/tests/raw_query/tests.py @@ -1,8 +1,8 @@ from datetime import date from decimal import Decimal +from django.core.exceptions import FieldDoesNotExist from django.db.models.query import RawQuerySet -from django.db.models.query_utils import InvalidQuery from django.test import TestCase, skipUnlessDBFeature from .models import ( @@ -235,7 +235,8 @@ class RawQueryTests(TestCase): def test_missing_fields_without_PK(self): query = "SELECT first_name, dob FROM raw_query_author" - with self.assertRaisesMessage(InvalidQuery, 'Raw query must include the primary key'): + msg = 'Raw query must include the primary key' + with self.assertRaisesMessage(FieldDoesNotExist, msg): list(Author.objects.raw(query)) def test_annotations(self):