#34539: `get_prep_value` no longer called for JSONField
-------------------------------------+-------------------------------------
Reporter: Julie Rymer | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Julie Rymer):
Replying to [comment:10 Mariusz Felisiak]:
> This will required adding LOC (probably many) that are not used by
Django itself and aren't tested (unless we will add many tests with custom
subclasses of built-in fields). This can also cause a lot of backward
incompatible changes in the behavior. I'm strongly against it as it'll set
a dangerous precedent and increase the maintenance burden.
I'll try to address Mariusz' concerns.
From what I understand, the change would only be a matter of adding
`value=self.get_prep_value(value)` is the concerned Field classes'
`get_db_prep_value` (`JSONField`, `ArrayField`, `DurationField`, other
?). The default `Field.get_prep_value` definition is the following:
{{{
def get_prep_value(self, value):
"""Perform preliminary non-db specific value checks and
conversions."""
if isinstance(value, Promise):
value = value._proxy____cast()
return value
}}}
So it doesn't seems to do much so I don't see what backward incompatible
change that would cause.
But if there is concern about it, `get_prep_value` could be overridden in
the concerned Field classes to directly return the value instead of doing
any operation, hence guaranteed no behaviour change.
This addition to the code **will be tested**, because `get_db_prep_value`
is already tested (right?) and we are not adding any behaviour that should
involve any explicit test. We are just adding an easier entrypoint for
customisation, the customisation itself is not for django to test so I
don't see what additional maintenance burden it would add.
The only potential burden I see is a new rule for future changes done on
`get_db_prep_value` of any standard field class to take care to call
`get_prep_value` even if it does nothing.
Perhaps I missed something, do tell me if that is the case.
On the contrary, updating the documentation to keep track of any Field
class not following the rule and explaining how to do customisation for
each of them seems like a lot more involved effort.
--
Ticket URL: <https://code.djangoproject.com/ticket/34539#comment:11>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/01070187e6ace96a-64a408fd-44e1-4457-8b1c-c8043b4188d9-000000%40eu-central-1.amazonses.com.
