oxan / djangorestframework-dataclasses

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

Cannot use with PATCH method #23

Closed HosseyNJF closed 3 years ago

HosseyNJF commented 4 years ago

The update method will not work with the PATCH method and partially updating. Error: TypeError: __init__() missing 3 required positional arguments: 'id', 'port', and 'cipher' It lists all the missing attributes and says they're required to init the dataclass.

oxan commented 4 years ago

Can you post the whole stacktrace? Thanks!

HosseyNJF commented 4 years ago

@oxan Here it is:

Traceback (most recent call last):
  File "/home/...../project/.venv/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/home/...../project/.venv/lib/python3.8/site-packages/django/core/handlers/base.py", line 179, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/...../project/.venv/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "/home/...../project/.venv/lib/python3.8/site-packages/rest_framework/viewsets.py", line 114, in view
    return self.dispatch(request, *args, **kwargs)
  File "/home/...../project/.venv/lib/python3.8/site-packages/rest_framework/views.py", line 505, in dispatch
    response = self.handle_exception(exc)
  File "/home/...../project/.venv/lib/python3.8/site-packages/rest_framework/views.py", line 465, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/home/...../project/.venv/lib/python3.8/site-packages/rest_framework/views.py", line 476, in raise_uncaught_exception
    raise exc
  File "/home/...../project/.venv/lib/python3.8/site-packages/rest_framework/views.py", line 502, in dispatch
    response = handler(request, *args, **kwargs)
  File "/home/...../project/.venv/lib/python3.8/site-packages/rest_framework/mixins.py", line 82, in partial_update
    return self.update(request, *args, **kwargs)
  File "/home/...../project/.venv/lib/python3.8/site-packages/rest_framework/mixins.py", line 67, in update
    serializer.is_valid(raise_exception=True)
  File "/home/...../project/.venv/lib/python3.8/site-packages/rest_framework/serializers.py", line 234, in is_valid
    self._validated_data = self.run_validation(self.initial_data)
  File "/home/...../project/.venv/lib/python3.8/site-packages/rest_framework/serializers.py", line 433, in run_validation
    value = self.to_internal_value(data)
  File "/home/...../project/.venv/lib/python3.8/site-packages/rest_framework_dataclasses/serializers.py", line 502, in to_internal_value
    instance = dataclass_type(**native_values)

Exception Type: TypeError at /api/v1/user/1/
Exception Value: __init__() missing 3 required positional arguments: 'id', 'port', and 'cipher'  # In this situation I only provided the 'secret' field in my request; the other 3 were absent
oxan commented 4 years ago

Thanks.

This is a bit tricky problem. In a4cc1c6, I converted the internal representation to use a dataclass instance instead of a dictionary. This eliminated a lot of complexity and subtle bugs in the save() implementation, where the dictionary had to be converted back to dataclasses (recursively). It also made it possible to nest dataclass serializers inside non-dataclass serializers. However, that also created this bug, as it tries to create a dataclass instance with only the supplied values (missing the required values). I'm not sure what the best way to fix this is yet.

oxan commented 3 years ago

Okay, for the root-level serializer this can be fixed by taking values for the missing properties from self.initial_data and instantiate the dataclass that way. For nested serializers it's a bit more tricky, but we might be able to pass the initial_data down to them as well.

HosseyNJF commented 3 years ago

@oxan Are all fields guaranteed to have the initial_data filled in properly? and also probably the consumer doesn't need those other fields anyway (because it is a PATCH method and it is supposed to have partial data).

I guess there is another way, which is having a special hollow class (some placeholder class named like MissingFieldValue), and all missing fields of the dataclass will get filled with an instance of this class. This method also has the benefit of the consumer knowing exactly which fields are provided and which are not.

oxan commented 3 years ago

@HosseyNJF Regarding initial_data, I'm not sure, that's something I would need to check. But I think your MissingFieldValue idea is pretty good, and it makes more sense to implement it like that.

tolomea commented 3 years ago

@oxan Did patching into nested serializers happen? and if so is it released?

It'd be handy if the docs had some info about the supported patch/post features.

oxan commented 3 years ago

Patching into nested serializers works in so far that you can overwrite the nested object, but not patch the nested object itself. I suppose it might be possible to implement that (at least for the simple cases), though I struggle to think of a usecase. Please open a new issue if you're interested in that.

I'll see if I can add something to the docs.