#34397 closed Bug (invalid)
Subclasses of JSONField call `get_prep_value` with field as value
Reported by: | pheki | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 3.2 |
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
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
.
Change History (3)
comment:1 by , 2 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 22 months ago
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.
comment:3 by , 8 days ago
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".
If I understand correctly, you have a custom
JSONField
with your own implementation ofget_prep_value()
.JSONField
no longer useget_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 ofKeyTextTransform
.