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

PrimaryKeyRelatedField from property method wrongly generates PKOnlyObject #6643

Closed
5 of 6 tasks
claradaia opened this issue May 2, 2019 · 4 comments · Fixed by #7142
Closed
5 of 6 tasks

PrimaryKeyRelatedField from property method wrongly generates PKOnlyObject #6643

claradaia opened this issue May 2, 2019 · 4 comments · Fixed by #7142

Comments

@claradaia
Copy link

claradaia commented May 2, 2019

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.
    It seems related to PKOnlyObject optimization may break HyperlinkedRelatedField #4653, but that one was closed with no action taken.
  • 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

Define the following models and serializer:

class Foo(models.Model):
    pass

class Bar(models.Model):
    @property
    def foo(self):
        return Foo.objects.first()

class BarSerializer(serializers.ModelSerializer):
    foo = serializers.PrimaryKeyRelatedField(
        queryset=Foo.objects.all()
    )

    class Meta:
        model = Bar
        fields = ('id', 'foo')

Create Bar instance and attempt to render it:

>>> Foo.objects.create()
<Foo: Foo object (2)>
>>> bar = Bar.objects.create()
>>> bar.foo
<Foo: Foo object (1)>
>>> serializer = BarSerializer(bar)
>>> serializer.data
{'foo': <Foo: Foo object (1)>, 'id': 2}
>>> JSONRenderer().render(serializer.data)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/clara/.virtualenvs/django_example_project/lib/python3.5/site-packages/rest_framework/renderers.py", line 107, in render
    allow_nan=not self.strict, separators=separators
  File "/home/clara/.virtualenvs/django_example_project/lib/python3.5/site-packages/rest_framework/utils/json.py", line 28, in dumps
    return json.dumps(*args, **kwargs)
  File "/usr/lib/python3.5/json/__init__.py", line 237, in dumps
    **kw).encode(obj)
  File "/usr/lib/python3.5/json/encoder.py", line 198, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/usr/lib/python3.5/json/encoder.py", line 256, in iterencode
    return _iterencode(o, 0)
  File "/home/clara/.virtualenvs/django_example_project/lib/python3.5/site-packages/rest_framework/utils/encoders.py", line 68, in default
    return super(JSONEncoder, self).default(obj)
  File "/usr/lib/python3.5/json/encoder.py", line 179, in default
    raise TypeError(repr(o) + " is not JSON serializable")
TypeError: <Foo: Foo object (1)> is not JSON serializable

Expected behavior

The serialization and deserialization should behave the same way they would if the field was a real FK in the model, that is, extract the PK from the related object in the serialization, and obtain the related object from the PK in the deserialization.

serializer.data should be

{
    'id': 2,
    'foo': 1
}

Which can be rendered to JSON.

Actual behavior

serializer.data is

{
    'id': 2,
    'foo': <Foo: Foo object (1)>
}

Which causes the error in the JSON rendering.


If I understand it correctly, this situation should fall into the edge case handled in RelatedField.get_attribute():

value = instance.serializable_value(self.source_attrs[-1])
if is_simple_callable(value):
    # Handle edge case where the relationship `source` argument
    # points to a `get_relationship()` method on the model
    value = value().pk

but because it is a @property method, it does not return a callable and causes the method to assume it is a PK.

I tried changing the source to foo.pk, that corrects the serialization behaviour, but messes up the deserialization behaviour (which was correct).

@rpkilby
Copy link
Member

rpkilby commented May 3, 2019

I'm going to close this off as unintended usage. Related fields are designed to work across ORM relationships, and this is just a regular Python property that happens to look kind of like a relationship. However, all of the ORM metadata that backs these fields is not present (since it's not an actual model field), so naturally the related serializer fields may not work.

That said, you should be able to achieve the desired result with the following:

class BarSerializer(serializers.ModelSerializer):
    foo = serializers.IntegerField(source='foo.pk')

    class Meta:
        model = Bar
        fields = ['id', 'foo']

@gabn88
Copy link
Contributor

gabn88 commented Jan 6, 2020

It would be nice if this worked.

@Mossop
Copy link

Mossop commented Jan 13, 2020

I just hit this too and spent a bunch of time tearing my hair out trying to figure out why. Might be nice if the framework could spit out an error or something when folks make this mistake.

@rpkilby
Copy link
Member

rpkilby commented Jan 15, 2020

After taking a second look, I was incorrect. As pointed out in #2690 (comment), there isn't anything explicitly stating that related field sources are incompatible with properties and callables.

I've opened #7142 to fix this.

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

Successfully merging a pull request may close this issue.

4 participants