[Django] #36986: Add a hook to Serializer._value_from_field() for complex fields like CompositePrimaryKey

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Type:
| Cleanup/optimization
Status: new | Component: Core
| (Serialization)
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
The Python serializer's `_value_from_field()` method using `isinstance`
for
[https://github.com/django/django/blob/main/django/core/serializers/python.py#L43-L44
special-case handling] of `CompositePrimaryKey`.

Custom model fields can't add similar logic without subclassing the
serializer, a cumbersome solution which doesn't scale well in the presence
of multiple custom fields.

My motivation for a generic hook is to add serialization support for
MongoDB's [https://django-mongodb-
backend.readthedocs.io/en/latest/ref/models/fields/#embeddedmodelfield
EmbeddedModelField], `EmbeddedModelArrayField`,
`PolymorphicEmbeddedModelField`, and `PolymorphicEmbeddedModelArrayField`.

An early prototype:
{{{#!diff
diff --git a/django/core/serializers/python.py
b/django/core/serializers/python.py
index 2929874b01..1a4f5a1336 100644
--- a/django/core/serializers/python.py
+++ b/django/core/serializers/python.py
@@ -39,9 +39,34 @@ class Serializer(base.Serializer):
data["fields"] = self._current
return data

+ def get_fields_from_model(self, obj, *, polymorphic=False):
+ data = {
+ field.name: self._value_from_field(obj, field)
+ for field in obj._meta.local_fields
+ }
+ if polymorphic:
+ data["_label"] = obj._meta.label
+ return data
+
def _value_from_field(self, obj, field):
if isinstance(field, CompositePrimaryKey):
return [self._value_from_field(obj, f) for f in field]
+ if hasattr(field, "embedded_model"):
+ sub_obj = getattr(obj, field.attname)
+ if sub_obj is None:
+ return None
+ if isinstance(sub_obj, list):
+ return [self.get_fields_from_model(sub) for sub in
sub_obj]
+ return self.get_fields_from_model(sub_obj)
+ if hasattr(field, "embedded_models"):
+ sub_obj = getattr(obj, field.attname)
+ if sub_obj is None:
+ return None
+ if isinstance(sub_obj, list):
+ return [
+ self.get_fields_from_model(sub, polymorphic=True) for
sub in sub_obj
+ ]
+ return self.get_fields_from_model(sub_obj, polymorphic=True)
value = field.value_from_object(obj)
# Protected types (i.e., primitives like None, numbers, dates,
# and Decimals) are passed through as is. All other values are
}}}
I don't have an immediate proposal for the API change to accomplish this.
--
Ticket URL: <https://code.djangoproject.com/ticket/36986>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Core | Version: dev
(Serialization) |
Severity: Normal | Resolution:

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 Vishy Algo):

Could we do a protocol-based hook similar to this?

{{{#!diff
diff --git a/django/core/serializers/python.py
b/django/core/serializers/python.py

index 73ba24368b..b2e61d1b72 100644
--- a/django/core/serializers/python.py
+++ b/django/core/serializers/python.py
@@ -40,6 +40,8 @@ class Serializer(base.Serializer):
return data

def _value_from_field(self, obj, field):
+ if hasattr(field, 'get_serializer_value'):
+ return field.get_serializer_value(obj)

if isinstance(field, CompositePrimaryKey):
return [self._value_from_field(obj, f) for f in field]

value = field.value_from_object(obj)
}}}

I guess, this gives custom fields full control while keeping it generic.
--
Ticket URL: <https://code.djangoproject.com/ticket/36986#comment:1>

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Core | Version: dev
(Serialization) |
Severity: Normal | Resolution:
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 Koushik Romel):

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

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Core | Version: dev
(Serialization) |
Severity: Normal | Resolution:
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 Tim Graham):

That's insufficient. Notice that both `CompositePrimaryKey` and my example
in the description require the ability to call `self._value_from_field`
recursively. I'm considering the possibility of a registry that would
allow registering a field's function with the serializer, similar to how
lookups are registered to model fields.

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: Vishy
Type: | Algo
Cleanup/optimization | Status: assigned

Component: Core | Version: dev
(Serialization) |
Severity: Normal | Resolution:
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 Vishy Algo):

* owner: (none) => Vishy Algo
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36986#comment:3>

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Vishy
Type: | Algo
Cleanup/optimization | Status: assigned
Component: Core | Version: dev
(Serialization) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

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

Changes (by Jacob Walls):

* stage: Unreviewed => Accepted

Comment:

Thanks, Tim. Would your field be able to implement `__iter__()`, like
`CompositePrimaryKey` does?
--
Ticket URL: <https://code.djangoproject.com/ticket/36986#comment:4>

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Vishy
Type: | Algo
Cleanup/optimization | Status: assigned
Component: Core | Version: dev
(Serialization) |
Severity: Normal | 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 Tim Graham):

Not for the polymorphic fields, where each field value may be a model
instance of a different class with different fields. The implementation in
the description uses `obj._meta.local_fields` rather than getting the list
of subfields from `field`.
--
Ticket URL: <https://code.djangoproject.com/ticket/36986#comment:5>

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Vishy
Type: | Algo
Cleanup/optimization | Status: assigned
Component: Core | Version: dev
(Serialization) |
Severity: Normal | 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 Tim Graham):

For another serializer API concern, see
47651eadb8ca7aacddad41da4df64fd2af11faae which fixed handling of JSONField
in the XML serializer by special casing it. I didn't intend to address XML
serialization in this ticket, but it could be useful if the new API were
generic enough to clean that up as well.

And while I only intended to address serialization in this ticket, I found
that a deserialization hook is also likely needed. Not only does the
JSONField/XML patch demonstrate a need, but for the various embedded model
fields, the
[https://github.com/django/django/blob/f2169ef3688422d394d36007e320bac839117f0b/django/core/serializers/python.py#L209
field.to_python()] handling in the Python `Deserializer._handle_object()`
almost works, except that embedded subfields are serialized to the
database using `Field.column` instead of `Field.name` as you would expect
to find in fixtures. Thus, the correct deserialization for embedded model
fields is [https://github.com/mongodb/django-mongodb-
backend/blob/3960d2ae2f6fc83b8c941db56195350412f68e1c/django_mongodb_backend/fields/embedded_model.py#L89-L106
EmbeddedModelField.to_python()] but with
`field.to_python(value[field.name])` instead of
`field.to_python(value[field.column])`.
--
Ticket URL: <https://code.djangoproject.com/ticket/36986#comment:6>

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Vishy
Type: | Algo
Cleanup/optimization | Status: assigned
Component: Core | Version: dev
(Serialization) |
Severity: Normal | 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 Vishy Algo):

We can Introduce a format-aware serialization registry on
{{{django.db.models.Field}}} mapping serializer formats ('python', 'xml',
etc.) to handler callables. To support recursive execution, the callable
signature must accept the active serializer instance as context:
{{{handler(field, obj, serializer)}}}.

This allows the removal of hardcoded isinstance checks within the core
serializers. For example, CompositePrimaryKey will encapsulate its
serialization compilation via the registry:
{{{CompositePrimaryKey.register_serializer_handler('python',
composite_python_serializer)}}}

Similarly, with the JSON field
{{{JSONField.register_serializer_handler('xml', json_xml_serializer)}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36986#comment:7>

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Vishy
Type: | Algo
Cleanup/optimization | Status: assigned
Component: Core | Version: dev
(Serialization) |
Severity: Normal | 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 Tim Graham):

The ideas seem plausible. My idea was to do the registrations on the
serializer rather than on the field, but either path could be viable.
Since the json, jsonl, and pyyaml serializers all subclass the Python
serializer, it's unclear to me that hooks are needed for them. Are these
your original ideas or are they from an LLM?
--
Ticket URL: <https://code.djangoproject.com/ticket/36986#comment:8>

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Vishy
Type: | Algo
Cleanup/optimization | Status: assigned
Component: Core | Version: dev
(Serialization) |
Severity: Normal | 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 Vishy Algo):

This, I followed up on the registry (lookup) approach as you mentioned
above, at implementing very similar pattern for serializer registration
with recursion support. Although, it is redundant hooks that's needed for
subclasses. Would making the registration on the serializers eliminate it?
--
Ticket URL: <https://code.djangoproject.com/ticket/36986#comment:9>

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Vishy
Type: | Algo
Cleanup/optimization | Status: assigned
Component: Core | Version: dev
(Serialization) |
Severity: Normal | 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 Vishy Algo):

I guess it would! We could simply register the Python serializer and
override serialization for specific fields in relevant serializers. Which
approach you think is more appropriate and extensible?
--
Ticket URL: <https://code.djangoproject.com/ticket/36986#comment:10>

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Vishy
Type: | Algo
Cleanup/optimization | Status: assigned
Component: Core | Version: dev
(Serialization) |
Severity: Normal | 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 Tim Graham):

The registration approach might be overcomplicated. In my
[https://github.com/mongodb-forks/django/pull/18 latest approach], I added
`Field.serialize_to_python()` and `deserialize_from_python()` hooks.
That's enough for MongoDB's embedded model fields and plays nicely with
composite fields as well. You could see if similar hooks for the XML
serializer would allow cleaning up the JSONField-specific code there.
--
Ticket URL: <https://code.djangoproject.com/ticket/36986#comment:11>

[Django]

#36986: Add a hook to Serializer._value_from_field() for complex fields like
CompositePrimaryKey
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: Tim
Type: | Graham

Cleanup/optimization | Status: assigned
Component: Core | Version: dev
(Serialization) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Tim Graham):

* owner: Vishy Algo => Tim Graham

Comment:

I added `Field.serialize_to_xml()` and `deserialize_from_xml()` to
[https://github.com/mongodb-forks/django/pull/18/changes my exploratory
branch] and it seems to work well. I'll refine it and make a PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/36986#comment:12>