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

Returning an object in Serializer.to_internal_value is not working any more since 3.8.2 #6053

Closed
5 of 6 tasks
slide333333 opened this issue Jun 22, 2018 · 9 comments
Closed
5 of 6 tasks
Milestone

Comments

@slide333333
Copy link

slide333333 commented Jun 22, 2018

Returning an object in Serializer.to_internal_value is not working any more since 3.8.2 (because of #5922). I don't know if it was ever intended to be working (since obviously there are no tests for it) but it did in 3.8.1. Now Serializer.to_internal_value always has to return a dict. Otherwise a type error is raised.

Checklist

  • I have verified that that issue exists against the master branch of Django REST framework.
  • I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
  • This is not a usage question. (Those should be directed to the discussion group instead.)
  • This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
  • I have reduced the issue to the simplest possible case.
  • I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

class NestedPointSerializer(serializers.Serializer):
    """
    Serialize `django.contrib.gis.geos.point.Point` instances
    """

    longitude = serializers.FloatField(source='x')
    latitude = serializers.FloatField(source='y')

    def to_internal_value(self, data):
        kwargs = super(NestedPointSerializer, self).to_internal_value(data)
        return Point(srid=4326, **kwargs)

NestedPointSerializer(data={'longitude': 6.958307, 'latitude': 50.941357}).is_valid()

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/IPython/core/interactiveshell.py", line 2910, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-4-fd7c8e4a6eab>", line 1, in <module>
    NestedPointSerializer(data={'longitude': 6.958307, 'latitude': 50.941357}).is_valid()
  File "/usr/local/lib/python3.7/site-packages/rest_framework/serializers.py", line 236, in is_valid
    self._validated_data = self.run_validation(self.initial_data)
  File "/usr/local/lib/python3.7/site-packages/rest_framework/serializers.py", line 436, in run_validation
    self.run_validators(value)
  File "/usr/local/lib/python3.7/site-packages/rest_framework/serializers.py", line 465, in run_validators
    to_validate.update(value)
TypeError: 'float' object is not iterable

Expected behavior

Everything is working like it did in 3.8.1.

Actual behavior

TypeError: 'float' object is not iterable raised in

to_validate.update(value)

@shulcsm
Copy link

shulcsm commented Aug 21, 2018

Bump,
Is the failing test necessary to proceed? Issue seems obvious

@rpkilby
Copy link
Member

rpkilby commented Aug 23, 2018

Typically, instance creation is handled in the create and update methods, which are called when saveing the serializer. Per the docstring, to_internal_value should return a dict, not an instance.

def to_internal_value(self, data):
"""
Dict of native values <- Dict of primitive datatypes.
"""

Usage would typically look like:

my_serializer.is_valid(raise_exception=True)
instance = my_serializer.save()

What are you expecting to do instead?

@shulcsm
Copy link

shulcsm commented Aug 23, 2018

What if there is no saving and you want to return your own type in validated_data

@rpkilby
Copy link
Member

rpkilby commented Aug 23, 2018

validated_data is still intended to be a dictionary of validated/native values. e.g., the kwargs passed to save() are used to update the validated_data dict. This wouldn't work if your validated_data is another type.

If you don't want to use the builtin save() method, you can always write your own instead. e.g.,

class PointSerializer:
    def save(self):
        self.instance = Point(self.validated_data)
        return self.instance

s = PointSerializer(...)
s.is_valid(raise_exception=True)
instance = s.save()

@shulcsm
Copy link

shulcsm commented Aug 23, 2018

To me it seemed like save was meant for persistence? And to_internal_value mean "normal form", if that is not the case it should be named to_scalar_dict or something. Perhaps my confusion is caused by this: http://www.django-rest-framework.org/api-guide/fields/#custom-fields

This ruins composablity since save is just a non-recursive stub.

This used to work prior to 3.8.2 and i have lot of code doing similar things.

class TwoPoints(serializer.Serializer):
    a = PointSerializer()
    b = PointSerializer()

s = TwoPoints(...)
s.is_valid(True)
assertDictEqual(s.validated_data, {'a': Point(...), 'b': Point(...) })

@rpkilby
Copy link
Member

rpkilby commented Aug 23, 2018

Good point. That is a discrepancy between the to_internal_value behavior of serializers and fields.

@rpkilby rpkilby added this to the 3.9 Release milestone Aug 23, 2018
@rpkilby
Copy link
Member

rpkilby commented Aug 23, 2018

I've added it to the milestone to help ensure this is addressed in some capacity.

The temporary "fix" here may just be to override run_validation instead of to_internal_value.

vdel added a commit to Deepomatic/django-rest-framework that referenced this issue Oct 11, 2018
vdel added a commit to Deepomatic/django-rest-framework that referenced this issue Oct 15, 2018
vdel added a commit to Deepomatic/django-rest-framework that referenced this issue Oct 15, 2018
@tomchristie tomchristie modified the milestones: 3.9 Release, 3.10 Release Oct 16, 2018
@mofr
Copy link

mofr commented Nov 20, 2018

Is there any workaround exists? I really need to update to 3.9 version and can't do it because of the problem. In our product most of serializers and fields return DTO (implemented with dataclasses) from to_internal_value.

@shulcsm
Copy link

shulcsm commented Nov 21, 2018

@mofr
You can get old behavior back by overloading run_validators so it doesn't mess with default values

def run_validators(self, value):
    super(Serializer, self).run_validators(value)

carltongibson added a commit to carltongibson/django-rest-framework that referenced this issue Dec 19, 2018
Fixes encode#6053.

Original test case thanks to Vincent Delaitre in encode#6242.
carltongibson added a commit to carltongibson/django-rest-framework that referenced this issue Dec 19, 2018
Fixes encode#6053.

Original test case thanks to Vincent Delaitre in encode#6242.
tomchristie pushed a commit that referenced this issue Jan 8, 2019
Fixes #6053.

Original test case thanks to Vincent Delaitre in #6242.
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this issue Nov 17, 2020
Fixes encode#6053.

Original test case thanks to Vincent Delaitre in encode#6242.
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

6 participants