Opened 11 months ago
Closed 11 months ago
#35071 closed New feature (invalid)
Resolve lazy objects inside dictionaries when saving JSONFields
Reported by: | Jacob Walls | Owned by: | Jacob Walls |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I have a model Notification
like this:
class Notification(models.Model): message = models.TextField(null=True) context = models.JSONField(null=True)
Here is the lazy object...
>>> from django.utils.translation import gettext_lazy as _ >>> lazy = _("lazy")
... that I can use in a TextField ...
>>> models.Notification(message=lazy).save() >>>
... but not for the JSONField without string-ifying it myself. Suggesting an implementation that resolves these in one of the methods that prepares values for database insertion. Tested with psycopg2 2.9.9.
>>> models.Notification(context={'lazy': lazy}).save() Traceback (most recent call last): File "<console>", line 1, in <module> File "/Users/jwalls/venv311/lib/python3.11/site-packages/django/db/models/base.py", line 814, in save self.save_base( File "/Users/jwalls/venv311/lib/python3.11/site-packages/django/db/models/base.py", line 877, in save_base updated = self._save_table( ^^^^^^^^^^^^^^^^^ File "/Users/jwalls/venv311/lib/python3.11/site-packages/django/db/models/base.py", line 990, in _save_table updated = self._do_update( ^^^^^^^^^^^^^^^^ File "/Users/jwalls/venv311/lib/python3.11/site-packages/django/db/models/base.py", line 1054, in _do_update return filtered._update(values) > 0 ^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/jwalls/venv311/lib/python3.11/site-packages/django/db/models/query.py", line 1231, in _update return query.get_compiler(self.db).execute_sql(CURSOR) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/jwalls/venv311/lib/python3.11/site-packages/django/db/models/sql/compiler.py", line 1982, in execute_sql cursor = super().execute_sql(result_type) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/jwalls/venv311/lib/python3.11/site-packages/django/db/models/sql/compiler.py", line 1560, in execute_sql cursor.execute(sql, params) File "/Users/jwalls/venv311/lib/python3.11/site-packages/django/db/backends/utils.py", line 102, in execute return super().execute(sql, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/jwalls/venv311/lib/python3.11/site-packages/django/db/backends/utils.py", line 67, in execute return self._execute_with_wrappers( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/jwalls/venv311/lib/python3.11/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers return executor(sql, params, many, context) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/jwalls/venv311/lib/python3.11/site-packages/django/db/backends/utils.py", line 89, in _execute return self.cursor.execute(sql, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/jwalls/venv311/lib/python3.11/site-packages/psycopg2/_json.py", line 78, in getquoted s = self.dumps(self.adapted) ^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/jwalls/venv311/lib/python3.11/site-packages/psycopg2/_json.py", line 72, in dumps return self._dumps(obj) ^^^^^^^^^^^^^^^^ File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/json/__init__.py", line 231, in dumps return _default_encoder.encode(obj) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/json/encoder.py", line 200, in encode chunks = self.iterencode(o, _one_shot=True) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/json/encoder.py", line 258, in iterencode return _iterencode(o, 0) ^^^^^^^^^^^^^^^^^ File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/json/encoder.py", line 180, in default raise TypeError(f'Object of type {o.__class__.__name__} ' TypeError: Object of type __proxy__ is not JSON serializable
Change History (13)
comment:1 by , 11 months ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:2 by , 11 months ago
I was thinking of something like this:
-
django/db/models/fields/json.py
diff --git a/django/db/models/fields/json.py b/django/db/models/fields/json.py index 7b9b2ae6b2..2df4f8ee12 100644
a b 1 from collections.abc import Container 1 2 import json 2 3 3 4 from django import forms 5 from django.conf import settings 4 6 from django.core import checks, exceptions 5 7 from django.db import NotSupportedError, connections, router 6 8 from django.db.models import expressions, lookups … … from django.db.models.lookups import ( 11 13 PostgresOperatorLookup, 12 14 Transform, 13 15 ) 16 from django.utils.functional import Promise 14 17 from django.utils.translation import gettext_lazy as _ 15 18 16 19 from . import Field … … class JSONField(CheckFieldDefaultMixin, Field): 96 99 def get_internal_type(self): 97 100 return "JSONField" 98 101 102 def get_prep_value(self, value): 103 if settings.USE_I18N: 104 match value: 105 case dict(): 106 value = { 107 self.get_prep_value(k): self.get_prep_value(v) 108 for k, v in value.items() 109 } 110 case str() | Promise(): 111 pass 112 case Container(): 113 value = value.__class__(self.get_prep_value(v) for v in value) 114 115 return super().get_prep_value(value) 116 99 117 def get_db_prep_value(self, value, connection, prepared=False): 100 118 if not prepared: 101 119 value = self.get_prep_value(value)
comment:4 by , 11 months ago
Yeah, I hadn't yet looked into whether lazy translation objects get resolved when USE_I18N = False. Looks like they do, so I should remove the guard.
comment:5 by , 11 months ago
Resolution: | wontfix |
---|---|
Status: | closed → new |
Triage Stage: | Unreviewed → Accepted |
Tentatively accepted.
comment:6 by , 11 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Version: | 4.2 → dev |
comment:7 by , 11 months ago
Summary: | Resolve lazy translation objects inside dictionaries when saving JSONFields → Resolve lazy objects inside dictionaries when saving JSONFields |
---|
comment:8 by , 11 months ago
So I have something sort of ready to go here, but then I noticed that what I should probably be doing is using DjangoJSONEncoder
in the encoder
arg of JSONField
, which solves my use case. Is that enough? Should that be the default?
comment:9 by , 11 months ago
I guess it would still be nice to separate this concern (resolving the promise) from the choice of serializer.
comment:11 by , 11 months ago
I think using the encoder
arg of JSONField
is the right answer to this issue. The get_prep_value
of your suggested patch would add too much overhead in my opinion.
comment:12 by , 11 months ago
I completely agree with Claude here.
As a matter of fact DjangoJSONEncoder
already works perfectly for this use case so I don't understand why we should treat promises differently given we already document using it for such cases and that the latter is documented to work with promises.
import json from django.core.serializers.json import DjangoJSONEncoder from django.utils.translation import gettext_lazy as _ json.dumps({"key": _("foobar")}) # TypeError: Object of type __proxy__ is not JSON serializable json.dumps({"key": _("foobar")}, cls=DjangoJSONEncoder) >>> '{"key": "foobar"}'
We also can't make encoder
default to DjangoJSONEncoder
as it elides the original type so you'd be able to serialize uuid, datetimes, etc but not get them back which breaks the principle of least astonishment IMO. DjangoJSONEncoder
works well for the serialization framework as it is equipped with a deserialization schema (model definition) so type information can be restored but JSONField
are schema less so it's not a good default.
comment:13 by , 11 months ago
Has patch: | unset |
---|---|
Resolution: | → invalid |
Status: | assigned → closed |
Triage Stage: | Accepted → Unreviewed |
Makes sense. Docs links are very helpful. Sorry for not using a support channel to check my understanding first!
It's easier to resolve a type for string-based fields, however,
JSONField
's values can contain different types of data. I don't think it's worth adding extra guess-like complexity for this. We can reconsider this decision if you provide a PoC.