Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DataclassSerializer fails with source="*" embedded serialiser fields because it requires Meta.dataclass #77

Closed
txomon opened this issue Apr 25, 2023 · 7 comments

Comments

@txomon
Copy link
Contributor

txomon commented Apr 25, 2023

According to https://www.django-rest-framework.org/api-guide/fields/#using-source, the serialisers should allow for use of source="*".

The issue can be reproduced in the following way. Copy the following snippet on the bottom of tests/test_functional.py:

class PetEmbeddedDefaultSerializer(DataclassSerializer):
    class PetInformationSerializer(DataclassSerializer):
        animal = fields.CharField()

    class Meta:
        dataclass = Pet
        fields = ['name', 'information']

    name = fields.CharField()
    information = PetInformationSerializer(source="*")


class SourceStartTest(TestCase):
    DATA = {'name': 'Milo', 'information': {'animal': 'cat'}}
    INSTANCE = Pet(name='Milo', animal='cat')

    def test_default_serialization(self):
        serializer = PetEmbeddedDefaultSerializer(instance=self.INSTANCE)
        self.assertDictEqual(serializer.data, self.DATA)

    def test_default_deserialization(self):
        serializer = PetEmbeddedDefaultSerializer(data=self.DATA)
        serializer.is_valid(raise_exception=True)
        self.assertEqual(serializer.instance, self.INSTANCE)
@txomon
Copy link
Contributor Author

txomon commented Apr 26, 2023

I have submitted a couple of options to solve the problem. Please have a look and see which solution works best for you.

@oxan
Copy link
Owner

oxan commented Apr 28, 2023

Is there any reason why you need your inner serializer (PetInformationSerializer) to subclass from DataclassSerializer? The whole thing of DataclassSerializer is that it autogenerates the serializer fields for you, but if you suppress that (like your PRs do), I don't see why you cannot use a regular Serializer instead (and that indeed seems to work fine for your example).

@txomon
Copy link
Contributor Author

txomon commented Apr 28, 2023 via email

@oxan
Copy link
Owner

oxan commented Apr 28, 2023

Maybe I'm overlooking something, but with vanilla drf-dataclasses the below standalone script seems to work as it should to me:

from rest_framework_dataclasses.serializers import DataclassSerializer
from rest_framework import fields, serializers
from typing import Literal
import dataclasses


@dataclasses.dataclass
class Pet:
    animal: Literal['cat', 'dog']
    name: str


class TestSerializer(DataclassSerializer):
    class NestedSerializer(serializers.Serializer):
       animal = fields.CharField()

    class Meta:
       dataclass = Pet
       fields = ['name', 'information']

    name = fields.CharField()
    information = NestedSerializer(source='*')


ser = TestSerializer(instance=Pet(animal='cat', name='Milo'))
print(ser.data)  # {'name': 'Milo', 'information': OrderedDict([('animal', 'cat')])}

dser = TestSerializer(data={'name': 'Milo', 'information': {'animal': 'dog'}})
dser.is_valid(raise_exception=True)
print(dser.validated_data)  # Pet(animal='dog', name='Milo')

@txomon
Copy link
Contributor Author

txomon commented May 1, 2023 via email

@txomon
Copy link
Contributor Author

txomon commented May 3, 2023

OK, I have refactored the PR to showcase the missing usecase, where an embedded dataclass fails to build because of the source="*" passed as arguments. Besides updating #79, here you have a snippet that surfaces the problem:

import dataclasses

from rest_framework import fields

from rest_framework_dataclasses.serializers import DataclassSerializer
from rest_framework_dataclasses.types import Literal


@dataclasses.dataclass
class Pet:
    animal: Literal['cat', 'dog']
    name: str


@dataclasses.dataclass
class PersonPet:
    name: str
    pet: Pet


class PersonEmbeddedDataclassSerializer(DataclassSerializer):
    class PetSerializer(DataclassSerializer):
        class Meta:
            dataclass = Pet
            fields = ['name', 'owner_name', 'animal']

        name = fields.CharField(source='pet.name')
        animal = fields.CharField(source='pet.animal')
        owner_name = fields.CharField(source="name")

    class Meta:
        dataclass = PersonPet
        fields = ['name', 'pet']

    name = fields.CharField()
    pet = PetSerializer(source="*")


DATA = {'name': 'Milo', 'pet': {'name': 'Katsu', 'animal': 'cat', 'owner_name': 'Milo'}}
serializer = PersonEmbeddedDataclassSerializer(data=DATA)
serializer.is_valid(raise_exception=True)
# Traceback (most recent call last):
#   File "/opt/.pycharm_helpers/pydev/pydevconsole.py", line 364, in runcode
#     coro = func()
#   File "<input>", line 1, in <module>
#   File "/usr/local/lib/python3.10/site-packages/rest_framework/serializers.py", line 227, in is_valid
#     self._validated_data = self.run_validation(self.initial_data)
#   File "/usr/local/lib/python3.10/site-packages/rest_framework/serializers.py", line 426, in run_validation
#     value = self.to_internal_value(data)
#   File "/opt/project/rest_framework_dataclasses/serializers.py", line 618, in to_internal_value
#     native_values = super(DataclassSerializer, self).to_internal_value(data)
#   File "/usr/local/lib/python3.10/site-packages/rest_framework/serializers.py", line 483, in to_internal_value
#     validated_value = field.run_validation(primitive_value)
#   File "/usr/local/lib/python3.10/site-packages/rest_framework/serializers.py", line 426, in run_validation
#     value = self.to_internal_value(data)
#   File "/opt/project/rest_framework_dataclasses/serializers.py", line 628, in to_internal_value
#     instance = dataclass_type(**native_values, **empty_values)
# TypeError: Pet.__init__() got an unexpected keyword argument 'pet'

@oxan
Copy link
Owner

oxan commented May 4, 2023

Ah, that's a good example.

It's a tricky problem to solve though. On deserialization, normally each DataclassSerializer returns a dataclass instance. That won't work here, as the inner serializer has fields that update the dataclass of its parent serializer. To do this properly, we would need to first process all the serializers all the way up to the root serializer, and then recurse down from the top to convert all value into dataclasses. However, that recursing down is problematic, as we can't assume that all dataclass fields have typehints, and in that case we don't know which dataclass to instantiate...

oxan added a commit that referenced this issue Aug 21, 2023
oxan added a commit that referenced this issue Aug 21, 2023
@oxan oxan closed this as completed in 34baaae Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants