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.

11 files changed, 230 insertions, 30 deletions

diff --git a/django/contrib/postgres/aggregates/general.py b/django/contrib/postgres/aggregates/general.py
index f8b40fb709..3de59dfcfd 100644
--- a/django/contrib/postgres/aggregates/general.py
+++ b/django/contrib/postgres/aggregates/general.py
@@ -1,8 +1,9 @@
+import json
 import warnings
 
 from django.contrib.postgres.fields import ArrayField
 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
 
@@ -31,6 +32,14 @@ class DeprecatedConvertValueMixin:
             self._default_provided = True
         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):
         if value is None and not self._default_provided:
             warnings.warn(self.deprecation_msg, category=RemovedInDjango50Warning)
@@ -48,8 +57,7 @@ class ArrayAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate):
     deprecation_msg = (
         "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 "
-        "and silence this warning or default=Value([]) to keep the previous "
-        "behavior."
+        "and silence this warning or default=[] to keep the previous behavior."
     )
 
     @property
@@ -87,13 +95,46 @@ class JSONBAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate):
 
     # RemovedInDjango50Warning
     deprecation_value = "[]"
+    deprecation_empty_result_set_value = property(lambda self: [])
     deprecation_msg = (
         "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 "
-        "and silence this warning or default=Value('[]') to keep the previous "
+        "and silence this warning or default=[] to keep the previous "
         "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):
     function = "STRING_AGG"
@@ -106,8 +147,7 @@ class StringAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate):
     deprecation_msg = (
         "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 "
-        "behavior and silence this warning or default=Value('') to keep the "
-        "previous behavior."
+        'behavior and silence this warning or default="" to keep the previous behavior.'
     )
 
     def __init__(self, expression, delimiter, **extra):
diff --git a/django/db/models/aggregates.py b/django/db/models/aggregates.py
index e2e0ef762b..e672f0aeb0 100644
--- a/django/db/models/aggregates.py
+++ b/django/db/models/aggregates.py
@@ -85,6 +85,8 @@ class Aggregate(Func):
             return c
         if hasattr(default, "resolve_expression"):
             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:
             default = Value(default, c._output_field_or_none)
         c.default = None  # Reset the default argument before wrapping.
diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py
index 2a98396cad..cd995de80f 100644
--- a/django/db/models/fields/__init__.py
+++ b/django/db/models/fields/__init__.py
@@ -921,6 +921,8 @@ class Field(RegisterLookupMixin):
 
     def get_db_prep_save(self, value, connection):
         """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)
 
     def has_default(self):
@@ -1715,6 +1717,8 @@ class DecimalField(Field):
     def get_db_prep_value(self, value, connection, prepared=False):
         if not prepared:
             value = self.get_prep_value(value)
+        if hasattr(value, "as_sql"):
+            return value
         return connection.ops.adapt_decimalfield_value(
             value, self.max_digits, self.decimal_places
         )
diff --git a/django/db/models/fields/json.py b/django/db/models/fields/json.py
index c0242bd7be..eb2d35f100 100644
--- a/django/db/models/fields/json.py
+++ b/django/db/models/fields/json.py
@@ -1,9 +1,10 @@
 import json
+import warnings
 
 from django import forms
 from django.core import checks, exceptions
 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.fields import TextField
 from django.db.models.lookups import (
@@ -11,6 +12,7 @@ from django.db.models.lookups import (
     PostgresOperatorLookup,
     Transform,
 )
+from django.utils.deprecation import RemovedInDjango51Warning
 from django.utils.translation import gettext_lazy as _
 
 from . import Field
@@ -97,7 +99,32 @@ class JSONField(CheckFieldDefaultMixin, Field):
         return "JSONField"
 
     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 connection.ops.adapt_json_value(value, self.encoder)
 
diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py
index caf36382b5..2b66ab12b4 100644
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -1637,9 +1637,7 @@ class SQLInsertCompiler(SQLCompiler):
                     "Window expressions are not allowed in this query (%s=%r)."
                     % (field.name, value)
                 )
-        else:
-            value = field.get_db_prep_save(value, connection=self.connection)
-        return value
+        return field.get_db_prep_save(value, connection=self.connection)
 
     def pre_save_val(self, field, obj):
         """
@@ -1893,18 +1891,14 @@ class SQLUpdateCompiler(SQLCompiler):
                     )
             elif hasattr(val, "prepare_database_save"):
                 if field.remote_field:
-                    val = field.get_db_prep_save(
-                        val.prepare_database_save(field),
-                        connection=self.connection,
-                    )
+                    val = val.prepare_database_save(field)
                 else:
                     raise TypeError(
                         "Tried to update field %s with a model instance, %r. "
                         "Use a value compatible with %s."
                         % (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.
             if hasattr(field, "get_placeholder"):
diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index 521054f69e..9e70aadca1 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -522,9 +522,9 @@ class Query(BaseExpression):
         result = compiler.execute_sql(SINGLE)
         if result is None:
             result = empty_set_result
-
-        converters = compiler.get_converters(outer_query.annotation_select.values())
-        result = next(compiler.apply_converters((result,), converters))
+        else:
+            converters = compiler.get_converters(outer_query.annotation_select.values())
+            result = next(compiler.apply_converters((result,), converters))
 
         return dict(zip(outer_query.annotation_select, result))
 
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index abd5a363bc..02a12f0823 100644
--- a/docs/internals/deprecation.txt
+++ b/docs/internals/deprecation.txt
@@ -39,6 +39,9 @@ details on these changes.
 
 * 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:
 
 5.0
diff --git a/docs/releases/4.2.txt b/docs/releases/4.2.txt
index 8153eefebc..054f8d6621 100644
--- a/docs/releases/4.2.txt
+++ b/docs/releases/4.2.txt
@@ -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.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
 -------------
 
diff --git a/docs/topics/db/queries.txt b/docs/topics/db/queries.txt
index 5114efb57d..977e287c53 100644
--- a/docs/topics/db/queries.txt
+++ b/docs/topics/db/queries.txt
@@ -971,7 +971,7 @@ Storing and querying for ``None``
 
 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
-``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>`.
 
 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: 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.objects.filter(data=None)
     <QuerySet [<Dog: Archie>]>
-    >>> Dog.objects.filter(data=Value('null'))
+    >>> Dog.objects.filter(data=Value(None, JSONField())
     <QuerySet [<Dog: Archie>]>
     >>> Dog.objects.filter(data__isnull=True)
     <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
     <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
 
 Key, index, and path transforms
diff --git a/tests/model_fields/test_jsonfield.py b/tests/model_fields/test_jsonfield.py
index 277e8aaa3c..05816817ef 100644
--- a/tests/model_fields/test_jsonfield.py
+++ b/tests/model_fields/test_jsonfield.py
@@ -19,6 +19,7 @@ from django.db.models import (
     ExpressionWrapper,
     F,
     IntegerField,
+    JSONField,
     OuterRef,
     Q,
     Subquery,
@@ -36,6 +37,7 @@ from django.db.models.fields.json import (
 from django.db.models.functions import Cast
 from django.test import SimpleTestCase, TestCase, skipIfDBFeature, skipUnlessDBFeature
 from django.test.utils import CaptureQueriesContext
+from django.utils.deprecation import RemovedInDjango51Warning
 
 from .models import CustomJSONDecoder, JSONModel, NullableJSONModel, RelatedJSONModel
 
@@ -191,15 +193,40 @@ class TestSaveLoad(TestCase):
         obj.refresh_from_db()
         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")
     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()
         sql_null = NullableJSONModel.objects.create(value=None)
         sql_null.refresh_from_db()
         # 'null' is not equal to NULL in the database.
         self.assertSequenceEqual(
-            NullableJSONModel.objects.filter(value=Value("null")),
+            NullableJSONModel.objects.filter(value=Value(None, JSONField())),
             [json_null],
         )
         self.assertSequenceEqual(
diff --git a/tests/postgres_tests/test_aggregates.py b/tests/postgres_tests/test_aggregates.py
index 629493b78f..183cff10c9 100644
--- a/tests/postgres_tests/test_aggregates.py
+++ b/tests/postgres_tests/test_aggregates.py
@@ -4,6 +4,7 @@ from django.db.models import (
     F,
     Func,
     IntegerField,
+    JSONField,
     OuterRef,
     Q,
     Subquery,
@@ -15,7 +16,7 @@ from django.db.models.functions import Cast, Concat, Substr
 from django.test import skipUnlessDBFeature
 from django.test.utils import Approximate, ignore_warnings
 from django.utils import timezone
-from django.utils.deprecation import RemovedInDjango50Warning
+from django.utils.deprecation import RemovedInDjango50Warning, RemovedInDjango51Warning
 
 from . import PostgreSQLTestCase
 from .models import AggregateTestModel, HotelReservation, Room, StatTestModel
@@ -124,7 +125,11 @@ class TestGeneralAggregate(PostgreSQLTestCase):
             (BitOr("integer_field", default=0), 0),
             (BoolAnd("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=Value("<empty>")),
@@ -189,9 +194,7 @@ class TestGeneralAggregate(PostgreSQLTestCase):
             {"aggregation": []},
         )
         self.assertEqual(
-            queryset.aggregate(
-                aggregation=JSONBAgg("integer_field", default=Value("[]"))
-            ),
+            queryset.aggregate(aggregation=JSONBAgg("integer_field", default=[])),
             {"aggregation": []},
         )
         self.assertEqual(
@@ -201,6 +204,59 @@ class TestGeneralAggregate(PostgreSQLTestCase):
             {"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):
         values = AggregateTestModel.objects.aggregate(arrayagg=ArrayAgg("char_field"))
         self.assertEqual(values, {"arrayagg": ["Foo1", "Foo2", "Foo4", "Foo3"]})