Skip to content

Add support for GenericRelations #319

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

Merged
merged 7 commits into from
Feb 23, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion example/factories/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import factory
from faker import Factory as FakerFactory
from example.models import Blog, Author, AuthorBio, Entry, Comment
from example.models import Blog, Author, AuthorBio, Entry, Comment, TaggedItem

faker = FakerFactory.create()
faker.seed(983843)
Expand Down Expand Up @@ -58,3 +58,11 @@ class Meta:
body = factory.LazyAttribute(lambda x: faker.text())
author = factory.SubFactory(AuthorFactory)


class EntryTaggedItemFactory(factory.django.DjangoModelFactory):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you call this TaggedItemFactory instead of EntryTaggedItemFactory? That seems more in line with the other factory naming conventions here.


class Meta:
model = TaggedItem

content_object = factory.SubFactory(EntryFactory)
tag = factory.LazyAttribute(lambda x: faker.word())
31 changes: 31 additions & 0 deletions example/migrations/0002_taggeditem.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.10.5 on 2017-02-01 08:34
from __future__ import unicode_literals

from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('contenttypes', '0002_remove_content_type_name'),
('example', '0001_initial'),
]

operations = [
migrations.CreateModel(
name='TaggedItem',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('created_at', models.DateTimeField(auto_now_add=True)),
('modified_at', models.DateTimeField(auto_now=True)),
('tag', models.SlugField()),
('object_id', models.PositiveIntegerField()),
('content_type', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType')),
],
options={
'abstract': False,
},
),
]
15 changes: 15 additions & 0 deletions example/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

from django.db import models
from django.utils.encoding import python_2_unicode_compatible
from django.contrib.contenttypes.models import ContentType
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reviewing this package's code, imports are usually alphabetically sorted. Could you move these contrib imports about the db import?

from django.contrib.contenttypes.fields import GenericForeignKey
from django.contrib.contenttypes.fields import GenericRelation


class BaseModel(models.Model):
Expand All @@ -16,10 +19,21 @@ class Meta:
abstract = True


class TaggedItem(BaseModel):
tag = models.SlugField()
content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
object_id = models.PositiveIntegerField()
content_object = GenericForeignKey('content_type', 'object_id')

def __str__(self):
return self.tag


@python_2_unicode_compatible
class Blog(BaseModel):
name = models.CharField(max_length=100)
tagline = models.TextField()
tags = GenericRelation(TaggedItem)

def __str__(self):
return self.name
Expand Down Expand Up @@ -54,6 +68,7 @@ class Entry(BaseModel):
n_comments = models.IntegerField(default=0)
n_pingbacks = models.IntegerField(default=0)
rating = models.IntegerField(default=0)
tags = GenericRelation(TaggedItem)

def __str__(self):
return self.headline
Expand Down
22 changes: 19 additions & 3 deletions example/serializers.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
from datetime import datetime
from rest_framework_json_api import serializers, relations
from example.models import Blog, Entry, Author, AuthorBio, Comment
from example.models import Blog, Entry, Author, AuthorBio, Comment, TaggedItem


class TaggedItemSerializer(serializers.ModelSerializer):

class Meta:
model = TaggedItem
fields = ('tag', )


class BlogSerializer(serializers.ModelSerializer):

copyright = serializers.SerializerMethodField()
tags = TaggedItemSerializer(many=True, read_only=True)

include_serializers = {
'tags': 'example.serializers.TaggedItemSerializer',
}

def get_copyright(self, resource):
return datetime.now().year
Expand All @@ -17,7 +29,8 @@ def get_root_meta(self, resource, many):

class Meta:
model = Blog
fields = ('name', 'url',)
fields = ('name', 'url', 'tags')
read_only_fields = ('tags', )
meta_fields = ('copyright',)


Expand All @@ -36,6 +49,7 @@ def __init__(self, *args, **kwargs):
'comments': 'example.serializers.CommentSerializer',
'featured': 'example.serializers.EntrySerializer',
'suggested': 'example.serializers.EntrySerializer',
'tags': 'example.serializers.TaggedItemSerializer',
}

body_format = serializers.SerializerMethodField()
Expand All @@ -52,6 +66,7 @@ def __init__(self, *args, **kwargs):
# single related from serializer
featured = relations.SerializerMethodResourceRelatedField(
source='get_featured', model=Entry, read_only=True)
tags = TaggedItemSerializer(many=True, read_only=True)

def get_suggested(self, obj):
return Entry.objects.exclude(pk=obj.pk)
Expand All @@ -65,7 +80,8 @@ def get_body_format(self, obj):
class Meta:
model = Entry
fields = ('blog', 'headline', 'body_text', 'pub_date', 'mod_date',
'authors', 'comments', 'featured', 'suggested',)
'authors', 'comments', 'featured', 'suggested', 'tags')
read_only_fields = ('tags', )
meta_fields = ('body_format',)


Expand Down
7 changes: 5 additions & 2 deletions example/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
import pytest
from pytest_factoryboy import register

from example.factories import BlogFactory, AuthorFactory, AuthorBioFactory, EntryFactory, CommentFactory
from example.factories import BlogFactory, AuthorFactory, AuthorBioFactory, EntryFactory, CommentFactory, \
EntryTaggedItemFactory

register(BlogFactory)
register(AuthorFactory)
register(AuthorBioFactory)
register(EntryFactory)
register(CommentFactory)
register(EntryTaggedItemFactory)


@pytest.fixture
def single_entry(blog, author, entry_factory, comment_factory):
def single_entry(blog, author, entry_factory, comment_factory, entry_tagged_item_factory):

entry = entry_factory(blog=blog, authors=(author,))
comment_factory(entry=entry)
entry_tagged_item_factory(content_object=entry)
return entry


Expand Down
10 changes: 10 additions & 0 deletions example/tests/integration/test_meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ def test_top_level_meta_for_list_view(blog, client):
"links": {
"self": 'http://testserver/blogs/1'
},
"relationships": {
"tags": {
"data": []
}
},
"meta": {
"copyright": datetime.now().year
},
Expand Down Expand Up @@ -51,6 +56,11 @@ def test_top_level_meta_for_detail_view(blog, client):
"attributes": {
"name": blog.name
},
"relationships": {
"tags": {
"data": []
}
},
"links": {
"self": "http://testserver/blogs/1"
},
Expand Down
6 changes: 6 additions & 0 deletions example/tests/integration/test_non_paginated_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ def test_multiple_entries_no_pagination(multiple_entries, rf):
"related": "http://testserver/entries/1/suggested/",
"self": "http://testserver/entries/1/relationships/suggested"
}
},
"tags": {
"data": []
}
}
},
Expand Down Expand Up @@ -83,6 +86,9 @@ def test_multiple_entries_no_pagination(multiple_entries, rf):
"related": "http://testserver/entries/2/suggested/",
"self": "http://testserver/entries/2/relationships/suggested"
}
},
"tags": {
"data": []
}
}
},
Expand Down
8 changes: 8 additions & 0 deletions example/tests/integration/test_pagination.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ def test_pagination_with_single_entry(single_entry, client):
"related": "http://testserver/entries/1/suggested/",
"self": "http://testserver/entries/1/relationships/suggested"
}
},
"tags": {
"data": [
{
"id": "1",
"type": "taggedItems"
}
]
}
}
}],
Expand Down
2 changes: 1 addition & 1 deletion requirements-development.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
pytest>=2.9.0,<3.0
pytest-django
pytest-factoryboy
fake-factory
Faker
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking up this package name change. This change has bit me on a few projects already.

tox
mock
16 changes: 12 additions & 4 deletions rest_framework_json_api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,14 @@
if django.VERSION >= (1, 9):
from django.db.models.fields.related_descriptors import ManyToManyDescriptor, ReverseManyToOneDescriptor
ReverseManyRelatedObjectsDescriptor = type(None)
from django.contrib.contenttypes.fields import ReverseGenericManyToOneDescriptor
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method of having two mirrored imports that set one to the None type is something I found confusing. After digging through the Django contenttypes source, I see that what is happening is trying to deal with a rename of the descriptor from 1.8 to 1.9. Unfortunately, this method leads to extra branches in an already large if/elif section.

How about using an import alias instead?

if django.VERSION >= (1, 9):
    # ... snip unmodified lines ...
    from django.contrib.contenttypes.fields import ReverseGenericManyToOneDescriptor
else:
    from django.contrib.contenttypes.fields import ReverseGenericRelatedObjectsDescriptor as ReverseGenericManyToOneDescriptor

I believe this would clean up the code, be more efficient, and capture the fact that the descriptor was renamed. It seems this pattern was already followed for the first two imports that follow the else.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

ReverseGenericRelatedObjectsDescriptor = type(None)
else:
from django.db.models.fields.related import ManyRelatedObjectsDescriptor as ManyToManyDescriptor
from django.db.models.fields.related import ForeignRelatedObjectsDescriptor as ReverseManyToOneDescriptor
from django.db.models.fields.related import ReverseManyRelatedObjectsDescriptor

from django.contrib.contenttypes.fields import ReverseGenericRelatedObjectsDescriptor
ReverseGenericManyToOneDescriptor = type(None)

def get_resource_name(context):
"""
Expand Down Expand Up @@ -203,17 +206,22 @@ def get_related_resource_type(relation):
else:
parent_model_relation = getattr(parent_model, parent_serializer.field_name)

if type(parent_model_relation) is ReverseManyToOneDescriptor:
parent_model_relation_type = type(parent_model_relation)
if parent_model_relation_type is ReverseManyToOneDescriptor:
if django.VERSION >= (1, 9):
relation_model = parent_model_relation.rel.related_model
elif django.VERSION >= (1, 8):
relation_model = parent_model_relation.related.related_model
else:
relation_model = parent_model_relation.related.model
elif type(parent_model_relation) is ManyToManyDescriptor:
elif parent_model_relation_type is ManyToManyDescriptor:
relation_model = parent_model_relation.field.remote_field.model
elif type(parent_model_relation) is ReverseManyRelatedObjectsDescriptor:
elif parent_model_relation_type is ReverseManyRelatedObjectsDescriptor:
relation_model = parent_model_relation.field.related.model
elif parent_model_relation_type is ReverseGenericManyToOneDescriptor:
relation_model = parent_model_relation.rel.model
elif parent_model_relation_type is ReverseGenericRelatedObjectsDescriptor:
relation_model = parent_model_relation.field.related_model
else:
return get_related_resource_type(parent_model_relation)

Expand Down