oxan / djangorestframework-dataclasses

Dataclasses serializer for Django REST framework
BSD 3-Clause "New" or "Revised" License
429 stars 28 forks source link

Nesting dataclass serializer inside regular serializer can result in <class 'rest_framework.fields.empty'> for optional fields #59

Closed henribru closed 1 year ago

henribru commented 2 years ago
from rest_framework import serializers
from rest_framework_dataclasses.serializers import DataclassSerializer

@dataclass
class Foo:
    foo: str | None = None

class FooSerializer(DataclassSerializer):
    class Meta:
        dataclass = Foo

class BarSerializer(serializers.Serializer):
    foo = FooSerializer()

serializer = BarSerializer(data={"foo": {}})

serializer.is_valid()

print(serializer.validated_data)  # OrderedDict([('foo', Foo(foo=<class 'rest_framework.fields.empty'>))])
oxan commented 2 years ago

Hmm, this is unfortunate but I don't immediately see how we can fix this. We need the empty sentinel values to distinguish between fields that aren't supplied and fields that are supplied but have the default value (the distinction matters for partial updates). On a DataclassSerializer they are stripped when accessing validated_data or calling save(), but they don't get called when you nest inside a regular serializer, and I don't see any other way to hook into that process.

adonig commented 1 year ago

I worked around it like this in my code:

@dataclass(frozen=True)
class Foo:
    foo: int | None = None
    bar: Bar | None = None
    baz: str | None = None

    def __post_init__(self):
        # XXX: Necessary until this issue gets fixed: https://github.com/oxan/djangorestframework-dataclasses/issues/59
        if type(self.foo) != int and self.foo is not None:
            object.__setattr__(self, "foo", None)
        if type(self.bar) != Bar and self.bar is not None:
            object.__setattr__(self, "bar", None)
        if type(self.baz) != str and self.baz is not None:
            object.__setattr__(self, "baz", None)
oxan commented 1 year ago

On second thought, we could put in the empty sentinel values only when partial updates are used (partial=True). I bet the combination of partial updates with nested serializers isn't be that common. I'll see if I can find some time to try that out soon...