Refs #33308 -- Deprecated support for passing encoded JSON string literals to JSONField & co.

JSON should be provided as literal Python objects an not in their
encoded string literal forms.
This commit is contained in:
Simon Charette 2022-11-02 22:03:05 -04:00 committed by Mariusz Felisiak
parent d3e746ace5
commit 0ff46591ac
11 changed files with 230 additions and 30 deletions

View File

@ -1,8 +1,9 @@
import json
import warnings import warnings
from django.contrib.postgres.fields import ArrayField from django.contrib.postgres.fields import ArrayField
from django.db.models import Aggregate, BooleanField, JSONField, TextField, Value from django.db.models import Aggregate, BooleanField, JSONField, TextField, Value
from django.utils.deprecation import RemovedInDjango50Warning from django.utils.deprecation import RemovedInDjango50Warning, RemovedInDjango51Warning
from .mixins import OrderableAggMixin from .mixins import OrderableAggMixin
@ -31,6 +32,14 @@ class DeprecatedConvertValueMixin:
self._default_provided = True self._default_provided = True
super().__init__(*expressions, default=default, **extra) super().__init__(*expressions, default=default, **extra)
def resolve_expression(self, *args, **kwargs):
resolved = super().resolve_expression(*args, **kwargs)
if not self._default_provided:
resolved.empty_result_set_value = getattr(
self, "deprecation_empty_result_set_value", self.deprecation_value
)
return resolved
def convert_value(self, value, expression, connection): def convert_value(self, value, expression, connection):
if value is None and not self._default_provided: if value is None and not self._default_provided:
warnings.warn(self.deprecation_msg, category=RemovedInDjango50Warning) warnings.warn(self.deprecation_msg, category=RemovedInDjango50Warning)
@ -48,8 +57,7 @@ class ArrayAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate):
deprecation_msg = ( deprecation_msg = (
"In Django 5.0, ArrayAgg() will return None instead of an empty list " "In Django 5.0, ArrayAgg() will return None instead of an empty list "
"if there are no rows. Pass default=None to opt into the new behavior " "if there are no rows. Pass default=None to opt into the new behavior "
"and silence this warning or default=Value([]) to keep the previous " "and silence this warning or default=[] to keep the previous behavior."
"behavior."
) )
@property @property
@ -87,13 +95,46 @@ class JSONBAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate):
# RemovedInDjango50Warning # RemovedInDjango50Warning
deprecation_value = "[]" deprecation_value = "[]"
deprecation_empty_result_set_value = property(lambda self: [])
deprecation_msg = ( deprecation_msg = (
"In Django 5.0, JSONBAgg() will return None instead of an empty list " "In Django 5.0, JSONBAgg() will return None instead of an empty list "
"if there are no rows. Pass default=None to opt into the new behavior " "if there are no rows. Pass default=None to opt into the new behavior "
"and silence this warning or default=Value('[]') to keep the previous " "and silence this warning or default=[] to keep the previous "
"behavior." "behavior."
) )
# RemovedInDjango51Warning: When the deprecation ends, remove __init__().
#
# RemovedInDjango50Warning: When the deprecation ends, replace with:
# def __init__(self, *expressions, default=None, **extra):
def __init__(self, *expressions, default=NOT_PROVIDED, **extra):
super().__init__(*expressions, default=default, **extra)
if (
isinstance(default, Value)
and isinstance(default.value, str)
and not isinstance(default.output_field, JSONField)
):
value = default.value
try:
decoded = json.loads(value)
except json.JSONDecodeError:
warnings.warn(
"Passing a Value() with an output_field that isn't a JSONField as "
"JSONBAgg(default) is deprecated. Pass default="
f"Value({value!r}, output_field=JSONField()) instead.",
stacklevel=2,
category=RemovedInDjango51Warning,
)
self.default.output_field = self.output_field
else:
self.default = Value(decoded, self.output_field)
warnings.warn(
"Passing an encoded JSON string as JSONBAgg(default) is "
f"deprecated. Pass default={decoded!r} instead.",
stacklevel=2,
category=RemovedInDjango51Warning,
)
class StringAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate): class StringAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate):
function = "STRING_AGG" function = "STRING_AGG"
@ -106,8 +147,7 @@ class StringAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate):
deprecation_msg = ( deprecation_msg = (
"In Django 5.0, StringAgg() will return None instead of an empty " "In Django 5.0, StringAgg() will return None instead of an empty "
"string if there are no rows. Pass default=None to opt into the new " "string if there are no rows. Pass default=None to opt into the new "
"behavior and silence this warning or default=Value('') to keep the " 'behavior and silence this warning or default="" to keep the previous behavior.'
"previous behavior."
) )
def __init__(self, expression, delimiter, **extra): def __init__(self, expression, delimiter, **extra):

View File

@ -85,6 +85,8 @@ class Aggregate(Func):
return c return c
if hasattr(default, "resolve_expression"): if hasattr(default, "resolve_expression"):
default = default.resolve_expression(query, allow_joins, reuse, summarize) default = default.resolve_expression(query, allow_joins, reuse, summarize)
if default._output_field_or_none is None:
default.output_field = c._output_field_or_none
else: else:
default = Value(default, c._output_field_or_none) default = Value(default, c._output_field_or_none)
c.default = None # Reset the default argument before wrapping. c.default = None # Reset the default argument before wrapping.

View File

@ -921,6 +921,8 @@ class Field(RegisterLookupMixin):
def get_db_prep_save(self, value, connection): def get_db_prep_save(self, value, connection):
"""Return field's value prepared for saving into a database.""" """Return field's value prepared for saving into a database."""
if hasattr(value, "as_sql"):
return value
return self.get_db_prep_value(value, connection=connection, prepared=False) return self.get_db_prep_value(value, connection=connection, prepared=False)
def has_default(self): def has_default(self):
@ -1715,6 +1717,8 @@ class DecimalField(Field):
def get_db_prep_value(self, value, connection, prepared=False): def get_db_prep_value(self, value, connection, prepared=False):
if not prepared: if not prepared:
value = self.get_prep_value(value) value = self.get_prep_value(value)
if hasattr(value, "as_sql"):
return value
return connection.ops.adapt_decimalfield_value( return connection.ops.adapt_decimalfield_value(
value, self.max_digits, self.decimal_places value, self.max_digits, self.decimal_places
) )

View File

@ -1,9 +1,10 @@
import json import json
import warnings
from django import forms from django import forms
from django.core import checks, exceptions from django.core import checks, exceptions
from django.db import NotSupportedError, connections, router from django.db import NotSupportedError, connections, router
from django.db.models import lookups from django.db.models import expressions, lookups
from django.db.models.constants import LOOKUP_SEP from django.db.models.constants import LOOKUP_SEP
from django.db.models.fields import TextField from django.db.models.fields import TextField
from django.db.models.lookups import ( from django.db.models.lookups import (
@ -11,6 +12,7 @@ from django.db.models.lookups import (
PostgresOperatorLookup, PostgresOperatorLookup,
Transform, Transform,
) )
from django.utils.deprecation import RemovedInDjango51Warning
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext_lazy as _
from . import Field from . import Field
@ -97,7 +99,32 @@ class JSONField(CheckFieldDefaultMixin, Field):
return "JSONField" return "JSONField"
def get_db_prep_value(self, value, connection, prepared=False): def get_db_prep_value(self, value, connection, prepared=False):
if hasattr(value, "as_sql"): # RemovedInDjango51Warning: When the deprecation ends, replace with:
# if (
# isinstance(value, expressions.Value)
# and isinstance(value.output_field, JSONField)
# ):
# value = value.value
# elif hasattr(value, "as_sql"): ...
if isinstance(value, expressions.Value):
if isinstance(value.value, str) and not isinstance(
value.output_field, JSONField
):
try:
value = json.loads(value.value, cls=self.decoder)
except json.JSONDecodeError:
value = value.value
else:
warnings.warn(
"Providing an encoded JSON string via Value() is deprecated. "
f"Use Value({value!r}, output_field=JSONField()) instead.",
category=RemovedInDjango51Warning,
)
elif isinstance(value.output_field, JSONField):
value = value.value
else:
return value
elif hasattr(value, "as_sql"):
return value return value
return connection.ops.adapt_json_value(value, self.encoder) return connection.ops.adapt_json_value(value, self.encoder)

View File

@ -1637,9 +1637,7 @@ class SQLInsertCompiler(SQLCompiler):
"Window expressions are not allowed in this query (%s=%r)." "Window expressions are not allowed in this query (%s=%r)."
% (field.name, value) % (field.name, value)
) )
else: return field.get_db_prep_save(value, connection=self.connection)
value = field.get_db_prep_save(value, connection=self.connection)
return value
def pre_save_val(self, field, obj): def pre_save_val(self, field, obj):
""" """
@ -1893,18 +1891,14 @@ class SQLUpdateCompiler(SQLCompiler):
) )
elif hasattr(val, "prepare_database_save"): elif hasattr(val, "prepare_database_save"):
if field.remote_field: if field.remote_field:
val = field.get_db_prep_save( val = val.prepare_database_save(field)
val.prepare_database_save(field),
connection=self.connection,
)
else: else:
raise TypeError( raise TypeError(
"Tried to update field %s with a model instance, %r. " "Tried to update field %s with a model instance, %r. "
"Use a value compatible with %s." "Use a value compatible with %s."
% (field, val, field.__class__.__name__) % (field, val, field.__class__.__name__)
) )
else: val = field.get_db_prep_save(val, connection=self.connection)
val = field.get_db_prep_save(val, connection=self.connection)
# Getting the placeholder for the field. # Getting the placeholder for the field.
if hasattr(field, "get_placeholder"): if hasattr(field, "get_placeholder"):

View File

@ -522,9 +522,9 @@ class Query(BaseExpression):
result = compiler.execute_sql(SINGLE) result = compiler.execute_sql(SINGLE)
if result is None: if result is None:
result = empty_set_result result = empty_set_result
else:
converters = compiler.get_converters(outer_query.annotation_select.values()) converters = compiler.get_converters(outer_query.annotation_select.values())
result = next(compiler.apply_converters((result,), converters)) result = next(compiler.apply_converters((result,), converters))
return dict(zip(outer_query.annotation_select, result)) return dict(zip(outer_query.annotation_select, result))

View File

@ -39,6 +39,9 @@ details on these changes.
* The ``TransactionTestCase.assertQuerysetEqual()`` method will be removed. * The ``TransactionTestCase.assertQuerysetEqual()`` method will be removed.
* Support for passing encoded JSON string literals to ``JSONField`` and
associated lookups and expressions will be removed.
.. _deprecation-removed-in-5.0: .. _deprecation-removed-in-5.0:
5.0 5.0

View File

@ -444,6 +444,42 @@ but it should not be used for new migrations. Use
:class:`~django.db.migrations.operations.AddIndex` and :class:`~django.db.migrations.operations.AddIndex` and
:class:`~django.db.migrations.operations.RemoveIndex` operations instead. :class:`~django.db.migrations.operations.RemoveIndex` operations instead.
Passing encoded JSON string literals to ``JSONField`` is deprecated
-------------------------------------------------------------------
``JSONField`` and its associated lookups and aggregates use to allow passing
JSON encoded string literals which caused ambiguity on whether string literals
were already encoded from database backend's perspective.
During the deprecation period string literals will be attempted to be JSON
decoded and a warning will be emitted on success that points at passing
non-encoded forms instead.
Code that use to pass JSON encoded string literals::
Document.objects.bulk_create(
Document(data=Value("null")),
Document(data=Value("[]")),
Document(data=Value('"foo-bar"')),
)
Document.objects.annotate(
JSONBAgg("field", default=Value('[]')),
)
Should become::
Document.objects.bulk_create(
Document(data=Value(None, JSONField())),
Document(data=[]),
Document(data="foo-bar"),
)
Document.objects.annotate(
JSONBAgg("field", default=[]),
)
From Django 5.1+ string literals will be implicitly interpreted as JSON string
literals.
Miscellaneous Miscellaneous
------------- -------------

View File

@ -971,7 +971,7 @@ Storing and querying for ``None``
As with other fields, storing ``None`` as the field's value will store it as As with other fields, storing ``None`` as the field's value will store it as
SQL ``NULL``. While not recommended, it is possible to store JSON scalar SQL ``NULL``. While not recommended, it is possible to store JSON scalar
``null`` instead of SQL ``NULL`` by using :class:`Value('null') ``null`` instead of SQL ``NULL`` by using :class:`Value(None, JSONField())
<django.db.models.Value>`. <django.db.models.Value>`.
Whichever of the values is stored, when retrieved from the database, the Python Whichever of the values is stored, when retrieved from the database, the Python
@ -987,11 +987,13 @@ query for SQL ``NULL``, use :lookup:`isnull`::
>>> Dog.objects.create(name='Max', data=None) # SQL NULL. >>> Dog.objects.create(name='Max', data=None) # SQL NULL.
<Dog: Max> <Dog: Max>
>>> Dog.objects.create(name='Archie', data=Value('null')) # JSON null. >>> Dog.objects.create(
... name='Archie', data=Value(None, JSONField()) # JSON null.
... )
<Dog: Archie> <Dog: Archie>
>>> Dog.objects.filter(data=None) >>> Dog.objects.filter(data=None)
<QuerySet [<Dog: Archie>]> <QuerySet [<Dog: Archie>]>
>>> Dog.objects.filter(data=Value('null')) >>> Dog.objects.filter(data=Value(None, JSONField())
<QuerySet [<Dog: Archie>]> <QuerySet [<Dog: Archie>]>
>>> Dog.objects.filter(data__isnull=True) >>> Dog.objects.filter(data__isnull=True)
<QuerySet [<Dog: Max>]> <QuerySet [<Dog: Max>]>
@ -1007,6 +1009,15 @@ Unless you are sure you wish to work with SQL ``NULL`` values, consider setting
Storing JSON scalar ``null`` does not violate :attr:`null=False Storing JSON scalar ``null`` does not violate :attr:`null=False
<django.db.models.Field.null>`. <django.db.models.Field.null>`.
.. versionchanged:: 4.2
Support for expressing JSON ``null`` using ``Value(None, JSONField())`` was
added.
.. deprecated:: 4.2
Passing ``Value("null")`` to express JSON ``null`` is deprecated.
.. fieldlookup:: jsonfield.key .. fieldlookup:: jsonfield.key
Key, index, and path transforms Key, index, and path transforms

View File

@ -19,6 +19,7 @@ from django.db.models import (
ExpressionWrapper, ExpressionWrapper,
F, F,
IntegerField, IntegerField,
JSONField,
OuterRef, OuterRef,
Q, Q,
Subquery, Subquery,
@ -36,6 +37,7 @@ from django.db.models.fields.json import (
from django.db.models.functions import Cast from django.db.models.functions import Cast
from django.test import SimpleTestCase, TestCase, skipIfDBFeature, skipUnlessDBFeature from django.test import SimpleTestCase, TestCase, skipIfDBFeature, skipUnlessDBFeature
from django.test.utils import CaptureQueriesContext from django.test.utils import CaptureQueriesContext
from django.utils.deprecation import RemovedInDjango51Warning
from .models import CustomJSONDecoder, JSONModel, NullableJSONModel, RelatedJSONModel from .models import CustomJSONDecoder, JSONModel, NullableJSONModel, RelatedJSONModel
@ -191,15 +193,40 @@ class TestSaveLoad(TestCase):
obj.refresh_from_db() obj.refresh_from_db()
self.assertIsNone(obj.value) self.assertIsNone(obj.value)
def test_ambiguous_str_value_deprecation(self):
msg = (
"Providing an encoded JSON string via Value() is deprecated. Use Value([], "
"output_field=JSONField()) instead."
)
with self.assertWarnsMessage(RemovedInDjango51Warning, msg):
obj = NullableJSONModel.objects.create(value=Value("[]"))
obj.refresh_from_db()
self.assertEqual(obj.value, [])
@skipUnlessDBFeature("supports_primitives_in_json_field")
def test_value_str_primitives_deprecation(self):
msg = (
"Providing an encoded JSON string via Value() is deprecated. Use "
"Value(None, output_field=JSONField()) instead."
)
with self.assertWarnsMessage(RemovedInDjango51Warning, msg):
obj = NullableJSONModel.objects.create(value=Value("null"))
obj.refresh_from_db()
self.assertIsNone(obj.value)
obj = NullableJSONModel.objects.create(value=Value("invalid-json"))
obj.refresh_from_db()
self.assertEqual(obj.value, "invalid-json")
@skipUnlessDBFeature("supports_primitives_in_json_field") @skipUnlessDBFeature("supports_primitives_in_json_field")
def test_json_null_different_from_sql_null(self): def test_json_null_different_from_sql_null(self):
json_null = NullableJSONModel.objects.create(value=Value("null")) json_null = NullableJSONModel.objects.create(value=Value(None, JSONField()))
NullableJSONModel.objects.update(value=Value(None, JSONField()))
json_null.refresh_from_db() json_null.refresh_from_db()
sql_null = NullableJSONModel.objects.create(value=None) sql_null = NullableJSONModel.objects.create(value=None)
sql_null.refresh_from_db() sql_null.refresh_from_db()
# 'null' is not equal to NULL in the database. # 'null' is not equal to NULL in the database.
self.assertSequenceEqual( self.assertSequenceEqual(
NullableJSONModel.objects.filter(value=Value("null")), NullableJSONModel.objects.filter(value=Value(None, JSONField())),
[json_null], [json_null],
) )
self.assertSequenceEqual( self.assertSequenceEqual(

View File

@ -4,6 +4,7 @@ from django.db.models import (
F, F,
Func, Func,
IntegerField, IntegerField,
JSONField,
OuterRef, OuterRef,
Q, Q,
Subquery, Subquery,
@ -15,7 +16,7 @@ from django.db.models.functions import Cast, Concat, Substr
from django.test import skipUnlessDBFeature from django.test import skipUnlessDBFeature
from django.test.utils import Approximate, ignore_warnings from django.test.utils import Approximate, ignore_warnings
from django.utils import timezone from django.utils import timezone
from django.utils.deprecation import RemovedInDjango50Warning from django.utils.deprecation import RemovedInDjango50Warning, RemovedInDjango51Warning
from . import PostgreSQLTestCase from . import PostgreSQLTestCase
from .models import AggregateTestModel, HotelReservation, Room, StatTestModel from .models import AggregateTestModel, HotelReservation, Room, StatTestModel
@ -124,7 +125,11 @@ class TestGeneralAggregate(PostgreSQLTestCase):
(BitOr("integer_field", default=0), 0), (BitOr("integer_field", default=0), 0),
(BoolAnd("boolean_field", default=False), False), (BoolAnd("boolean_field", default=False), False),
(BoolOr("boolean_field", default=False), False), (BoolOr("boolean_field", default=False), False),
(JSONBAgg("integer_field", default=Value('["<empty>"]')), ["<empty>"]), (JSONBAgg("integer_field", default=["<empty>"]), ["<empty>"]),
(
JSONBAgg("integer_field", default=Value(["<empty>"], JSONField())),
["<empty>"],
),
(StringAgg("char_field", delimiter=";", default="<empty>"), "<empty>"), (StringAgg("char_field", delimiter=";", default="<empty>"), "<empty>"),
( (
StringAgg("char_field", delimiter=";", default=Value("<empty>")), StringAgg("char_field", delimiter=";", default=Value("<empty>")),
@ -189,9 +194,7 @@ class TestGeneralAggregate(PostgreSQLTestCase):
{"aggregation": []}, {"aggregation": []},
) )
self.assertEqual( self.assertEqual(
queryset.aggregate( queryset.aggregate(aggregation=JSONBAgg("integer_field", default=[])),
aggregation=JSONBAgg("integer_field", default=Value("[]"))
),
{"aggregation": []}, {"aggregation": []},
) )
self.assertEqual( self.assertEqual(
@ -201,6 +204,59 @@ class TestGeneralAggregate(PostgreSQLTestCase):
{"aggregation": ""}, {"aggregation": ""},
) )
@ignore_warnings(category=RemovedInDjango51Warning)
def test_jsonb_agg_default_str_value(self):
AggregateTestModel.objects.all().delete()
queryset = AggregateTestModel.objects.all()
self.assertEqual(
queryset.aggregate(
aggregation=JSONBAgg("integer_field", default=Value("<empty>"))
),
{"aggregation": "<empty>"},
)
def test_jsonb_agg_default_str_value_deprecation(self):
queryset = AggregateTestModel.objects.all()
msg = (
"Passing a Value() with an output_field that isn't a JSONField as "
"JSONBAgg(default) is deprecated. Pass default=Value('<empty>', "
"output_field=JSONField()) instead."
)
with self.assertWarnsMessage(RemovedInDjango51Warning, msg):
queryset.aggregate(
aggregation=JSONBAgg("integer_field", default=Value("<empty>"))
)
with self.assertWarnsMessage(RemovedInDjango51Warning, msg):
queryset.none().aggregate(
aggregation=JSONBAgg("integer_field", default=Value("<empty>"))
),
@ignore_warnings(category=RemovedInDjango51Warning)
def test_jsonb_agg_default_encoded_json_string(self):
AggregateTestModel.objects.all().delete()
queryset = AggregateTestModel.objects.all()
self.assertEqual(
queryset.aggregate(
aggregation=JSONBAgg("integer_field", default=Value("[]"))
),
{"aggregation": []},
)
def test_jsonb_agg_default_encoded_json_string_deprecation(self):
queryset = AggregateTestModel.objects.all()
msg = (
"Passing an encoded JSON string as JSONBAgg(default) is deprecated. Pass "
"default=[] instead."
)
with self.assertWarnsMessage(RemovedInDjango51Warning, msg):
queryset.aggregate(
aggregation=JSONBAgg("integer_field", default=Value("[]"))
)
with self.assertWarnsMessage(RemovedInDjango51Warning, msg):
queryset.none().aggregate(
aggregation=JSONBAgg("integer_field", default=Value("[]"))
)
def test_array_agg_charfield(self): def test_array_agg_charfield(self):
values = AggregateTestModel.objects.aggregate(arrayagg=ArrayAgg("char_field")) values = AggregateTestModel.objects.aggregate(arrayagg=ArrayAgg("char_field"))
self.assertEqual(values, {"arrayagg": ["Foo1", "Foo2", "Foo4", "Foo3"]}) self.assertEqual(values, {"arrayagg": ["Foo1", "Foo2", "Foo4", "Foo3"]})