[Django] #34397: Subclasses of JSONField call `get_prep_value` with field as value

[Django]

#34397: Subclasses of JSONField call `get_prep_value` with field as value
-------------------------------------+-------------------------------------
Reporter: pheki | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 3.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Hi!

Let's say I have a class called PhoneNumber:

{{{
@dataclass
class PhoneNumber:
country_iso_code: str
national_number: int
}}}

I create a field I use to use PhoneNumber in my database:

{{{
class PhoneNumberField(JSONField):
def get_prep_value(
self, value: Optional[PhoneNumber]
) -> Optional[JsonAdapter]:
if isinstance(value, PhoneNumber):
return JsonAdapter(dataclasses.asdict(value),
encoder=CustomJSONEncoder)
elif value is None and self.null:
return None
else:
raise Exception("Fetching phone number with unexpected value")

# Similar from_db_value ommited
def from_db_value(...):
...
}}}

The problem is that when I do a KeyTextTransform to get a field, it will
call get_prep_value with that field and AFAIK there's no way to know which
field is it getting. For from_db_value the same happens, but it's fine as
you can inspect the expression column. Example:

{{{
User.objects.annotate(
national_number=KeyTextTransform("national_number", "phone_number")
).filter(
national_number="12345678"
)[0]
}}}

Will call `field.get_prep_value("12345678")`, forcing the only way to make
it work correctly to silently ignore unexpected types. This didn't happen
on django 2.2 for `django.contrib.postgres.fields.jsonb.JSONField`.

--
Ticket URL: <https://code.djangoproject.com/ticket/34397>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#34397: Subclasses of JSONField call `get_prep_value` with field as value
-------------------------------------+-------------------------------------
Reporter: pheki | Owner: nobody

Type: Bug | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* status: new => closed
* resolution: => invalid

Comment:

If I understand correctly, you have a custom `JSONField` with your own
implementation of `get_prep_value()`. `JSONField` no longer use
`get_prep_value()` in Django 4.2+ (see
5c23d9f0c32f166c81ecb6f3f01d5077a6084318) so you check if it works for
you, however, even if it doesn't, you can always implement your own
expression and use it instead of `KeyTextTransform`.

--
Ticket URL: <https://code.djangoproject.com/ticket/34397#comment:1>

[Django]

#34397: Subclasses of JSONField call `get_prep_value` with field as value
-------------------------------------+-------------------------------------
Reporter: pheki | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Julie Rymer):

This issue was filed for django 3.2 so the recent change in `JSONField`
behaviour has no impact.
However, from what I understand @pheki, wants two different behaviour
depending on the field it is called on. For that the solution is to create
two custom classes, each implementing the behaviour that you want and
assign it to the corresponding field.

--
Ticket URL: <https://code.djangoproject.com/ticket/34397#comment:2>

[Django]

#34397: Subclasses of JSONField call `get_prep_value` with field as value
-------------------------------------+-------------------------------------
Reporter: pheki | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by pheki):

Sorry for the very-very-late response, but I never took the time to
revisit this before and it may be useful for future reference.

> This issue was filed for django 3.2 so the recent change in JSONField
behaviour has no impact.

Yes, that's true, this is was a difference in behavior between Django
2.2's Postgres JSONField and 3.2's JSONField, which I realized as I was
working on a codebase update. At the same time it was useful information
as, even though 5.0 re-introduced get_prep_value, 4.2 is a kind of
necessary stepping stone.

> However, from what I understand @pheki, wants two different behaviour
depending on the field it is called on. For that the solution is to create
two custom classes, each implementing the behaviour that you want and
assign it to the corresponding field.

Not exactly, I explained pretty badly but here's a second try: I have a
single field that inherits JSONField and I'm using get_prep_value and
from_db_value to autoconvert the JSON to/from a dataclass. The issue was
that, when using KeyTextTransform and a filter or just a field lookup, it
sends the value to `get_prep_value` without any way to know its a field
lookup, so the conversion fails. You may be receiving a value that's
really internal in the JSON, not the whole dict.

My first solution was just to ignore errors on get_prep_value, but that
opens a margin for users using the field incorrectly. I later changed it
to use the encoder directly. In the end, I think that even though it makes
it harder to use correctly, at least it's consistent with other field
types. e.g. TextField lookups (e.g. `__contains`) also calls `get_prep_db`
with the lookup value and no way to check if it's a "field value" or a
"lookup value".
--
Ticket URL: <https://code.djangoproject.com/ticket/34397#comment:3>