Skip to content

Nested serializers only work with Django Model objects #249

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

Closed
knaperek opened this issue Jun 2, 2016 · 9 comments
Closed

Nested serializers only work with Django Model objects #249

knaperek opened this issue Jun 2, 2016 · 9 comments

Comments

@knaperek
Copy link
Contributor

knaperek commented Jun 2, 2016

While DRF works perfectly fine with simple non-model instances, DJA crashes and can't be made to work correctly.

For example, this code works fine with DRF:

import random
from rest_framework.response import Response
from rest_framework.views import APIView
from rest_framework_json_api import serializers
from rest_framework_json_api.renderers import JSONRenderer as jsonapi_renderer


class Post(object):
    def __init__(self, message):
        super(Post, self).__init__()
        self.pk = 'tmp-%s' % random.randint(1, 2**20)  # Added to support json API referencing
        self.message = message


class User(object):
    def __init__(self, name, posts):
        super(User, self).__init__()
        self.pk = 'tmp-%s' % random.randint(1, 2**20)  # Added to support json API referencing
        self.name = name
        self.posts = posts


class PostSerializer(serializers.Serializer):
    message = serializers.CharField(max_length=200)


class UserSerializer(serializers.Serializer):
    name = serializers.CharField(max_length=200)
    posts = PostSerializer(many=True, read_only=True)


class UserView(APIView):
    resource_name = 'users'
    renderer_classes = (jsonapi_renderer,)  # ! Uncommenting this causes failure !

    def get(self, request, format=None):
        p1 = Post("First message")
        p2 = Post("Second message")
        user = User('Bob', [p1, p2])
        s = UserSerializer(user)
        return Response(s.data)

However, after uncommenting the line that specifies the JSON API renderer class to be used, the view crashes with:

'NoneType' object has no attribute 'Meta'

If DJA does not support Model-less references, it could at least be clearly stated in the documentation, mentioning also some workarounds if such exist. Though, I have not found any way to make this working yet.

@jerel
Copy link
Member

jerel commented Jun 2, 2016

DJA uses a model's JSONAPIMeta class to determine the type of a resource and I believe it falls back to Meta for backwards compatibility. That could be where the error is coming from. You could try defining:

class JSONAPIMeta:
    resource_name = 'post'

in your Post class. Otherwise I'm not sure, would need to see more of the stack trace to figure out what's going wrong.

@knaperek
Copy link
Contributor Author

knaperek commented Jun 2, 2016

Thanks for the quick answer. I did try to define that resource_name inside JSONAPIMeta class before, had the same result though.

This is the stack trace I'm getting:

2016-06-02 18:20:34,464 ERROR django.request Internal Server Error: /users/
Traceback (most recent call last):
  File "/home/vagrant/env/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 164, in get_response
    response = response.render()
  File "/home/vagrant/env/local/lib/python2.7/site-packages/django/template/response.py", line 158, in render
    self.content = self.rendered_content
  File "/home/vagrant/env/local/lib/python2.7/site-packages/rest_framework/response.py", line 71, in rendered_content
    ret = renderer.render(self.data, media_type, context)
  File "/home/vagrant/env/local/lib/python2.7/site-packages/rest_framework_json_api/renderers.py", line 463, in render
    json_api_data = self.build_json_resource_obj(fields, serializer_data, resource_instance, resource_name)
  File "/home/vagrant/env/local/lib/python2.7/site-packages/rest_framework_json_api/renderers.py", line 369, in build_json_resource_obj
    relationships = JSONRenderer.extract_relationships(fields, resource, resource_instance)
  File "/home/vagrant/env/local/lib/python2.7/site-packages/rest_framework_json_api/renderers.py", line 100, in extract_relationships
    relation_type = utils.get_related_resource_type(field)
  File "/home/vagrant/env/local/lib/python2.7/site-packages/rest_framework_json_api/utils.py", line 181, in get_related_resource_type
    parent_model = parent_serializer.parent.Meta.model
AttributeError: 'NoneType' object has no attribute 'Meta'
[02/Jun/2016 18:20:34] "GET /users/ HTTP/1.0" 500 146083

The parent_serializer object inside the get_related_resource_type method is an instance of my UserSerializer class (which seems OK). The problem is, that its parent attribute is None, and I don't know why it should have any parent for proper functioning; and what kind of parent should that be.

@rmmr
Copy link
Contributor

rmmr commented Jun 2, 2016

Just came across the same issue. I will fix this in the coming hours. Pull request incoming.

@rmmr
Copy link
Contributor

rmmr commented Jun 2, 2016

Ok I made an initial fix, but I had to change more than I expected.

https://github.com/adaptivdesign/django-rest-framework-json-api/commit/127f4b7128d6db4150422715a1345f01a4a7b265

@knaperek

This should work now:

class PostListSerializer(serializers.ListSerializer):
    class JSONAPIMeta:
        resource_name = 'post'


class PostSerializer(serializers.Serializer):
    message = serializers.CharField(max_length=200)

    class Meta:
        list_serializer_class = PostListSerializer

    class JSONAPIMeta:
        resource_name = 'post'

class UserSerializer(serializers.Serializer):
    name = serializers.CharField(max_length=200)
    posts = PostSerializer(many=True, read_only=True)

    class JSONAPIMeta:
        resource_name = 'user'

@jerel

  • As it seems resource_name would have never been used for relations because 'get_related_resource_type' would never look for it.
  • If resource_name was looked up by 'get_resource_type_from_serializer' it would use the regular Meta class, instead of JSONAPIMeta. For now I allowed both, but I would suggest using only JSONAPIMeta in the future.
  • After this was fixed, I had to make some changes in the renderer. It didn't like anything else besides QuerySet's ans Manager's .

All current tests are passed. But I didn't write any new tests, because modelles Serializer's aren't really tested right now. What should we do next?

@knaperek
Copy link
Contributor Author

knaperek commented Jun 3, 2016

@adaptivdesign Thanks for such a prompt fix, I confirm that it no longer crashes.

However, I'm still missing the included top level key (with its data). Is this a missing feature in general, or is it pertinent to this Model-less setup? UPDATE: OK, included works when added as a query param inside the request. Would be nice to have the ability to set the default included types though.

Anyway, many thanks for help and quick fixing! You guys rock 👍

This is my output currently:

{
  "data": {
    "type": "users",
    "id": "tmp-953157",
    "attributes": {
      "name": "Bob"
    },
    "relationships": {
      "posts": {
        "data": [
          {
            "type": "post",
            "id": "tmp-545551"
          },
          {
            "type": "post",
            "id": "tmp-612679"
          }
        ]
      }
    }
  }
}

UPDATE 2: I have created PR #250 that implements this new option to specify resources to be included by default.

@scottfisk
Copy link
Contributor

scottfisk commented Jun 24, 2016

@adaptivdesign RE

If resource_name was looked up by 'get_resource_type_from_serializer' it would use the regular Meta class, instead of JSONAPIMeta. For now I allowed both, but I would suggest using only JSONAPIMeta in the future.

We did add JSONAPIMeta to the serializer as part of #250 so it seems reasonable to move jsonapi specific options to the new Meta class. We should update the tests and docs of this change and possibly throw a deprecation if people still have resource_name in the Meta class on serializers.

@rmmr
Copy link
Contributor

rmmr commented Jul 3, 2016

Okay, do you want me to do these? I'll have to find some spare time for it. Regarding the tests and docs, besides mentioning the JSONAPIMeta changes, what about the changes to the serialization behaviour. Right now the example resources "Blog, Entry, etc" are all models, in order to test regular serializers, we would need to add (or possibly rewrite) quite some tests.

@jerel
Copy link
Member

jerel commented Jul 7, 2016

I'd be in favor of moving jsonapi options to the JSONAPIMeta class although it wouldn't have to be done in this PR. No need to rewrite existing tests, just keep the model serializers as is and add another test or two for non-model serializers.

@jerel
Copy link
Member

jerel commented Aug 9, 2016

Closed by #265

@jerel jerel closed this as completed Aug 9, 2016
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

4 participants