Browse Source

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.
Simon Charette 5 years ago
parent
commit
11e327a3ff

+ 4 - 2
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([

+ 31 - 6
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
 
 

+ 3 - 0
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 <deprecated-features-3.1>` for more
 details on these changes.
 

+ 5 - 0
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

+ 3 - 2
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
 ------------------

+ 3 - 3
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):

+ 30 - 0
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)

+ 3 - 2
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):