Django-rest-framework: OneToOneField should be PrimaryKeyRelatedField, not ModelField, raises exception

[mikofski]

Checklist

• [ ] I have verified that that issue exists against the master branch of Django REST framework.
• [x] I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
• [x] This is not a usage question. (Those should be directed to the discussion group instead.)
• [x] 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.)
• [x] I have reduced the issue to the simplest possible case.
• [x] 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

create a model with a OneToOneField

class SpecialUser(models.Model):
    special_user = models.OneToOneField(
    User,
    on_delete=models.CASCADE,
    primary_key=True,
    related_name="+")

Then create a mode serializer.

class SpecialUserSerializer(ModelSerializer):
    class Meta:
        model = SpecialUser
        fields = '__all__'

Now view the serializer

SpecialUserSerializer():
    special_user = ModelField(
        model_field=<django.db.models.fields.related.OneToOneField: special_user>,
        validators=[<UniqueValidator(queryset=SpecialUser.objects.all())>]
    )

Now try to serialize a new user:

s = SpecialUserSerializer(data={'special_user': 2})
s.is_valid()  # True
s.save()

ValueError: Cannot assign "2": "SpecialUser.special_user" must be a "User" instance.

for full stacktrace see pastebin

Expected behavior

I would expect that the model serializer would treat the one-to-one field same as a foreign key and create a PrimaryKeyRelatedField instead of the model field. If I do this explicitly then the serializer works as expected:

class SpecialUserSerializer(ModelSerializer):
    special_user = PrimaryKeyRelatedField(queryset=User.objects.all())
    class Meta:
        model = SpecialUser
        fields = '__all__'

Actual behavior

If you don't explicitly create the primary key related field in the serializer then you cannot use the primary key to serialize the object because Django complains that you must use a model not an integer, nor can you use a nested serialization of the one-to-one related model because then DRF complains that you must use an integer.

I would like to post a pull request to fix this myself, but it will take a little time. I don't think this is so urgent, since the work around is so easy. In fact, maybe I'm missing something totally obvious and this works as intended?

[tomchristie]

First thing to do is probably to double check which versions you see this behavior against.

I'm surprised to see a breakage in such a typical relationship, so it's possible that it's either been broken in our latest release, or it's something that's since been fixed that's breaking for you in an older release.

It's also possible that this...

primary_key=True,

Could be the source of the issue - perhaps this only fails for 1:1 relationships that also happen to be the primary key on a model.

[mikofski]

• I ran this test in a fresh virtualenv with the latest versions of Django (1.11.1) and DRF (3.6.3).
• I originally discovered this behavior (OneToOneField <-> ModelField instead of PrimaryKeyRelatedField) on Django-1.10 and DRF-3.5.

• @tomchristie your intuition was correct re: primary_key=True - if I set this to False then the serializer is created with a PrimaryKeyRelatedField for the OneToOneField and a separate primary key as an integer field called "ID".

  SpecialUserSerializer():
      id = IntegerField(label='ID', read_only=True)
      special_user = PrimaryKeyRelatedField(
          queryset=User.objects.all(),
          validators=[<UniqueValidator(queryset=SpecialUser.objects.all())>]
      )
  

So is this the expected behavior then? My concern then is that the Django documentation on OneToOneField seem to suggest that One-To-One relations are most often used as a primary key:

This is most useful as the primary key of a model which “extends” another model in some way; ...

I left out the next sentence on MTI as an example of one-to-one relations to avoid confusion, since AFAICT that is working fine.

Is this a non-issue then? Perhaps I can add something to the documentation to explain that if primary_key=True in the OneToOneField then explicitly bind the field to a PrimaryKeyRelatedField in the associated serializer.

[tomchristie]

No we should deal with this case automatically. Nice work on pinpointing it.

[matteius]

You can assign this to me, I started working on determining the fix but I have a test harness that demos the issue so I will work on it from that angle.

[mikofski]

Thanks @matteius

[matteius]

@mikofski I have a Pull Request out that should solve the issue you are describing.

[mikofski]

@matteius it works! I just tried it with the example I originally gave above.

I installed your branch using

pip install -e git+https://github.com/matteius/django-rest-framework@DRF-5135-one-to-one-pk#egg=djangorestframework`

then in django shell:

import SpecialUserSerializer
s = SpecialUserSerializer(data={'special_user': 2})
s.is_valid()
s.save()
s.instance.special_user
# returns special user #2 YAY!

[mikofski]

Also just tested the browsable API and it also works, first time, nothing fancy!

in urls.py

from django.conf.urls import include, url
import views
from rest_framework.routers import DefaultRouter

# Create a router and register our viewsets with it.
router = DefaultRouter()
router.register(r'special_user', views.SpecialUserViewSet)

# The API URLs are now determined automatically by the router.
# Additionally, we include the login URLs for the browsable API.
urlpatterns = [
    url(r'^', include(router.urls)),
    url(r'^api-auth/', include('rest_framework.urls',
                           namespace='rest_framework'))
]

and in views.py

class SpecialUserViewSet(viewsets.ModelViewSet):
    """
    This viewset automatically provides `list` and `detail` actions.
    """
    queryset = SpecialUser.objects.all()
    serializer_class = SpecialUserSerializer

just works!