Opened 5 months ago

Closed 5 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 Mariusz Felisiak, 5 months ago

Resolution: wontfix
Status: newclosed

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.

comment:2 by Jacob Walls, 5 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  
     1from collections.abc import Container
    12import json
    23
    34from django import forms
     5from django.conf import settings
    46from django.core import checks, exceptions
    57from django.db import NotSupportedError, connections, router
    68from django.db.models import expressions, lookups
    from django.db.models.lookups import (  
    1113    PostgresOperatorLookup,
    1214    Transform,
    1315)
     16from django.utils.functional import Promise
    1417from django.utils.translation import gettext_lazy as _
    1518
    1619from . import Field
    class JSONField(CheckFieldDefaultMixin, Field):  
    9699    def get_internal_type(self):
    97100        return "JSONField"
    98101
     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
    99117    def get_db_prep_value(self, value, connection, prepared=False):
    100118        if not prepared:
    101119            value = self.get_prep_value(value)

comment:3 by Mariusz Felisiak, 5 months ago

Why do you want to put it behind the USE_I18N?

comment:4 by Jacob Walls, 5 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 Mariusz Felisiak, 5 months ago

Resolution: wontfix
Status: closednew
Triage Stage: UnreviewedAccepted

Tentatively accepted.

comment:6 by Mariusz Felisiak, 5 months ago

Owner: changed from nobody to Jacob Walls
Status: newassigned
Version: 4.2dev

comment:7 by Mariusz Felisiak, 5 months ago

Summary: Resolve lazy translation objects inside dictionaries when saving JSONFieldsResolve lazy objects inside dictionaries when saving JSONFields

comment:8 by Jacob Walls, 5 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 Jacob Walls, 5 months ago

I guess it would still be nice to separate this concern (resolving the promise) from the choice of serializer.

comment:10 by Jacob Walls, 5 months ago

Has patch: set

comment:11 by Claude Paroz, 5 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 Simon Charette, 5 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.

Last edited 5 months ago by Simon Charette (previous) (diff)

comment:13 by Jacob Walls, 5 months ago

Has patch: unset
Resolution: invalid
Status: assignedclosed
Triage Stage: AcceptedUnreviewed

Makes sense. Docs links are very helpful. Sorry for not using a support channel to check my understanding first!

Note: See TracTickets for help on using tickets.
Back to Top