From b894b3c15d54a97643ddb96d08c1203f7294f5c4 Mon Sep 17 00:00:00 2001 From: lukasbuenger Date: Wed, 27 Mar 2013 19:15:16 +0100 Subject: [PATCH] refactored GenericRelatedField with dict argument as discussed here: https://github.com/tomchristie/django-rest-framework/pull/755 --- rest_framework/genericrelations.py | 218 ++++------------- rest_framework/tests/relations_generic.py | 277 ++++++++++------------ 2 files changed, 168 insertions(+), 327 deletions(-) diff --git a/rest_framework/genericrelations.py b/rest_framework/genericrelations.py index c30ebccac..3c27e8009 100644 --- a/rest_framework/genericrelations.py +++ b/rest_framework/genericrelations.py @@ -1,197 +1,50 @@ -""" -# Generic relations - -## Introduction - -This is an attempt to implement generic relation foreign keys as provided by Django's -[contenttype framework](https://docs.djangoproject.com/en/dev/ref/contrib/contenttypes/) and I tried to follow the -suggestions and requirements described here: -* https://github.com/tomchristie/django-rest-framework/issues/606 -* http://stackoverflow.com/a/14454629/1326146 - -The `GenericRelatedField` field enables you to read and write `GenericForeignKey`s within the scope of Django REST -Framework. While you can decide whether to output `GenericForeignKey`s as nested objects (`ModelSerializer`) or valid -resource URLs (`HyperlinkedRelatedField`), the input is restricted to resource URLs mainly because of two reasons: -* The sheer impossibility of deriving a valid `Model` class from a simple data dictionary. -* My inner storm of indignation when thinking of exposing the actual `ContentType`s to the public scope. - -## Disclaimer - -Although I'm pretty experienced with Django and REST etc, please note that this piece of code is also my first -experience with Django REST framework at all. I am the maintainer of a pretty large REST service application based on -[tastypie](tastypieapi.org), but Daniel's recent -[blog post](http://toastdriven.com/blog/2013/feb/05/committers-needed-tastypie-haystack/) made me feel a little -uncomfortable. As much I'd like to spend more time contributing, I fear that my current work-life balance doesn't allow -me to do so at the level I'd expect from myself. So I thought it would be a good thing to reach out for other solutions. -It's a good thing anyway, I guess. But a sane and approved way of representing `GenericForeignKeys` is just essential to -the data structure I'm working with. So that's basically why I got in the mix. - - -## Minimalist example - -This is based on the models mentioned in -[this article](http://django-rest-framework.org/api-guide/relations.html#generic-relationships). The examples are also -based on working URL patterns for each model fitting the pattern `-detail`. - -A minimalist but still working example of a `TagSerializer` with `GenericRelatedField` would look like this: - - class TagSerializer(serializers.ModelSerializer): - tagged_item = GenericRelatedField([ - GenericRelationOption(Bookmark, 'bookmark-detail'), - GenericRelationOption(Note, 'note-detail'), - ], source='tagged_item') - - class Meta: - model = Tag - exclude = ('id', 'content_type', 'object_id', ) - -## ``GenericRelationOption`` - -Constructor: - - GenericRelationOption(model, view_name, as_hyperlink=True, related_field=None, serializer=None) - -**`model`** -The model class. - -**`view_name`** -The view name as used in url patterns. - -**`as_hyperlink`** -Decides whether the output of the `GenericForeignKey` should be as end point or nested object. In the case of the -latter a generic serializer will be used unless you pass a specific one. - -**`related_field`** -A specific subclass of `HyperlinkedRelatedField` that should be used for resolving input data and resolving output data -in case of `as_hyperlink` is `True`. - -**`serializer`** -A specific subclass of `ModelSerializer` that should be used for resolving output data -in case of `as_hyperlink` is `True`. - -""" from __future__ import unicode_literals - from django.core.exceptions import ValidationError from django.core.urlresolvers import get_script_prefix, resolve from django.utils.translation import ugettext_lazy as _ from django import forms from rest_framework.compat import urlparse +from rest_framework import six from rest_framework import serializers -from rest_framework.settings import api_settings - - -class GenericRelationOption(object): - """ - This object is responsible for setting up the components needed for providing a generic relation with a given model. - """ - - #TODO: Far more strict evaluation of custom related_field and serializer objects - - # Trying to be inline with common practices - model_serializer_class = api_settings.DEFAULT_MODEL_SERIALIZER_CLASS - - def __init__(self, model, view_name, as_hyperlink=True, related_field=None, serializer=None): - self.model = model - self.view_name = view_name - self.as_hyperlink = as_hyperlink - self.related_field = related_field or self.get_default_related_field() - self.serializer = serializer or self.get_default_serializer() - - def get_output_resolver(self): - """ - Should return a class that implements the `to_native` method, i.e. `HyperlinkedRelatedField` or `ModelSerializer`. - """ - if self.as_hyperlink: - return self.get_prepared_related_field() - else: - return self.serializer - - def get_input_resolver(self): - """ - Should return a class that implements the `from_native` method that can handle URL values, - i.e. `HyperlinkedRelatedField`. - """ - return self.get_prepared_related_field() - - def get_prepared_related_field(self): - """ - Provides the related field with a queryset if not present, based on `self.model`. - """ - if self.related_field.queryset is None: - self.related_field.queryset = self.model.objects.all() - return self.related_field - - def get_default_related_field(self): - """ - Creates and returns a minimalist ``HyperlinkedRelatedField` instance if none has been passed to the constructor. - """ - return serializers.HyperlinkedRelatedField(view_name=self.view_name) - - def get_default_serializer(self): - """ - Creates and returns a minimalist ``ModelSerializer` instance if none has been passed to the constructor. - """ - class DefaultSerializer(self.model_serializer_class): - class Meta: - model = self.model - return DefaultSerializer() +from rest_framework.exceptions import ConfigurationError class GenericRelatedField(serializers.WritableField): """ Represents a generic relation foreign key. - - It's actually more of a wrapper, that delegates the logic to registered fields / serializers based on some - contenttype framework criteria. + It's actually more of a wrapper, that delegates the logic to registered serializers based on the `Model` class. """ default_error_messages = { 'no_model_match': _('Invalid model - model not available.'), - 'no_match': _('Invalid hyperlink - No URL match'), - 'incorrect_match': _('Invalid hyperlink - view name not available'), + 'no_url_match': _('Invalid hyperlink - No URL match'), + 'incorrect_url_match': _('Invalid hyperlink - view name not available'), } form_field_class = forms.URLField - def __init__(self, options, *args, **kwargs): + def __init__(self, serializers, *args, **kwargs): """ - Needs an extra parameter ``options`` which has to be a list of `GenericRelationOption` objects. + Needs an extra parameter `serializers` which has to be a dict key: value being `Model`: serializer. """ super(GenericRelatedField, self).__init__(*args, **kwargs) - - # Map for option identifying based on a `Model` class (deserialization cycle) - self._model_map = dict() - # Map for option identifying based on a `view_name` (serialization cycle) - self._view_name_map = dict() - - # Adding the options to the maps. - for option in options: - self._model_map[option.model] = option - self._view_name_map[option.view_name] = option + self.serializers = serializers + for model, serializer in six.iteritems(self.serializers): + # We have to do it, because the serializer can't access a explicit manager through the + # GenericForeignKey field on the model. + if hasattr(serializer, 'queryset') and serializer.queryset is None: + serializer.queryset = model._default_manager.all() def field_to_native(self, obj, field_name): """ - Identifies the option object that is responsible for this `value.__class__` (a model) object and returns - its output serializer's `to_native` method. + Delegates to the `to_native` method of the serializer registered under obj.__class__ """ value = super(GenericRelatedField, self).field_to_native(obj, field_name) - - # Retrieving the model class. - model = value.__class__ - - try: - option = self._model_map[model] - except KeyError: - raise ValidationError(self.error_messages['no_model_match']) - - # Get the serializer responsible for output formatting - serializer = option.get_output_resolver() + serializer = self.determine_deserializer_for_data(value) # Necessary because of context, field resolving etc. serializer.initialize(self.parent, field_name) - return serializer.to_native(value) def to_native(self, value): @@ -199,6 +52,24 @@ class GenericRelatedField(serializers.WritableField): return value def from_native(self, value): + # Get the serializer responsible for input resolving + serializer = self.determine_serializer_for_data(value) + serializer.initialize(self.parent, self.source) + return serializer.from_native(value) + + def determine_deserializer_for_data(self, value): + try: + model = value.__class__ + serializer = self.serializers[model] + except KeyError: + raise ValidationError(self.error_messages['no_model_match']) + return serializer + + def determine_serializer_for_data(self, value): + for serializer in six.itervalues(self.serializers): + if not isinstance(serializer, serializers.HyperlinkedRelatedField): + raise ConfigurationError('Please use HyperlinkedRelatedField as serializers on GenericRelatedField \ + instances with read_only=False or set read_only=True.') # This excerpt is an exact copy of ``rest_framework.relations.HyperlinkedRelatedField``, Line 363 # From here until ... @@ -217,20 +88,15 @@ class GenericRelatedField(serializers.WritableField): try: match = resolve(value) except Exception: - raise ValidationError(self.error_messages['no_match']) + raise ValidationError(self.error_messages['no_url_match']) - # ... here. Thinking about putting that in ``rest_framework.utils.py``. Of course With more appropriate exceptions. + # ... here - # Try to find the derived `view_name` in the map. - try: - view_name = match.url_name - option = self._view_name_map[view_name] - except KeyError: - raise ValidationError(self.error_messages['incorrect_match']) + matched_serializer = None + for serializer in six.itervalues(self.serializers): + if serializer.view_name == match.url_name: + matched_serializer = serializer - # Get the serializer responsible for input resolving - serializer = option.get_input_resolver() - - # Necessary because of context, field resolving etc. - serializer.initialize(self.parent, self.source) - return serializer.from_native(value) + if matched_serializer is None: + raise ValidationError(self.error_messages['incorrect_url_match']) + return matched_serializer \ No newline at end of file diff --git a/rest_framework/tests/relations_generic.py b/rest_framework/tests/relations_generic.py index 2705bec05..9f991648f 100644 --- a/rest_framework/tests/relations_generic.py +++ b/rest_framework/tests/relations_generic.py @@ -1,12 +1,14 @@ from __future__ import unicode_literals from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.generic import GenericRelation, GenericForeignKey +from django.core.exceptions import ValidationError from django.db import models from django.test import TestCase, RequestFactory from rest_framework import serializers from rest_framework.compat import patterns, url -from rest_framework.genericrelations import GenericRelationOption, GenericRelatedField +from rest_framework.exceptions import ConfigurationError +from rest_framework.genericrelations import GenericRelatedField from rest_framework.reverse import reverse factory = RequestFactory() @@ -58,75 +60,19 @@ class Note(models.Model): return 'Note: %s' % self.text -class Contact(models.Model): - """ - A textual note that may have multiple tags attached. - """ - name = models.TextField() - slug = models.SlugField() - tags = GenericRelation(Tag) - - def __unicode__(self): - return 'Contact: %s' % self.name +class BookmarkSerializer(serializers.ModelSerializer): + class Meta: + model = Bookmark + exclude = ('id', ) -class TestGenericRelationOptions(TestCase): - - def test_default_related_field(self): - option = GenericRelationOption(Bookmark, 'bookmark-detail') - self.assertIsInstance(option.related_field, serializers.HyperlinkedRelatedField) - - def test_default_related_field_view_name(self): - option = GenericRelationOption(Bookmark, 'bookmark-detail') - self.assertEqual(option.related_field.view_name, 'bookmark-detail') - - def test_default_serializer(self): - option = GenericRelationOption(Bookmark, 'bookmark-detail') - self.assertIsInstance(option.serializer, serializers.ModelSerializer) - - def test_default_serializer_meta_model(self): - option = GenericRelationOption(Bookmark, 'bookmark-detail') - self.assertEqual(option.serializer.Meta.model, Bookmark) - - def test_get_url_output_resolver(self): - option = GenericRelationOption(Bookmark, 'bookmark-detail') - self.assertIsInstance(option.get_output_resolver(), serializers.HyperlinkedRelatedField) - - def test_get_url_output_resolver_with_queryset(self): - option = GenericRelationOption(Bookmark, 'bookmark-detail') - self.assertIsNotNone(option.get_output_resolver().queryset) - - def test_get_input_resolver(self): - option = GenericRelationOption(Bookmark, 'bookmark-detail') - self.assertIsInstance(option.get_input_resolver(), serializers.HyperlinkedRelatedField) - - def test_get_input_resolver_with_queryset(self): - option = GenericRelationOption(Bookmark, 'bookmark-detail') - self.assertIsNotNone(option.get_output_resolver().queryset) - - def test_get_serializer_resolver(self): - option = GenericRelationOption(Bookmark, 'bookmark-detail', as_hyperlink=False) - self.assertIsInstance(option.get_output_resolver(), serializers.ModelSerializer) - - def test_custom_related_field(self): - related_field = serializers.HyperlinkedRelatedField(view_name='bookmark-detail', format='xml') - option = GenericRelationOption(Bookmark, 'bookmark-detail', - related_field=related_field) - self.assertEqual(option.related_field.format, 'xml') - - def test_custom_serializer(self): - - class BookmarkSerializer(serializers.ModelSerializer): - class Meta: - model = Bookmark - exclude = ('id', ) - - serializer = BookmarkSerializer() - option = GenericRelationOption(Bookmark, 'bookmark-detail', as_hyperlink=False, serializer=serializer) - self.assertIn('id', option.get_output_resolver().Meta.exclude) +class NoteSerializer(serializers.ModelSerializer): + class Meta: + model = Note + exclude = ('id', ) -class TestGenericRelatedFieldToNative(TestCase): +class TestGenericRelatedFieldDeserialization(TestCase): urls = 'rest_framework.tests.relations_generic' @@ -140,10 +86,10 @@ class TestGenericRelatedFieldToNative(TestCase): def test_relations_as_hyperlinks(self): class TagSerializer(serializers.ModelSerializer): - tagged_item = GenericRelatedField([ - GenericRelationOption(Bookmark, 'bookmark-detail'), - GenericRelationOption(Note, 'note-detail'), - ], source='tagged_item') + tagged_item = GenericRelatedField({ + Bookmark: serializers.HyperlinkedRelatedField(view_name='bookmark-detail'), + Note: serializers.HyperlinkedRelatedField(view_name='note-detail'), + }, source='tagged_item', read_only=True) class Meta: model = Tag @@ -169,10 +115,10 @@ class TestGenericRelatedFieldToNative(TestCase): def test_relations_as_nested(self): class TagSerializer(serializers.ModelSerializer): - tagged_item = GenericRelatedField([ - GenericRelationOption(Bookmark, 'bookmark-detail'), - GenericRelationOption(Note, 'note-detail', as_hyperlink=False), - ], source='tagged_item') + tagged_item = GenericRelatedField({ + Bookmark: BookmarkSerializer(), + Note: NoteSerializer(), + }, source='tagged_item', read_only=True) class Meta: model = Tag @@ -181,16 +127,19 @@ class TestGenericRelatedFieldToNative(TestCase): serializer = TagSerializer(Tag.objects.all(), many=True) expected = [ { - 'tagged_item': '/bookmark/1/', + 'tagged_item': { + 'url': 'https://www.djangoproject.com/' + }, 'tag': 'django' }, { - 'tagged_item': '/bookmark/1/', + 'tagged_item': { + 'url': 'https://www.djangoproject.com/' + }, 'tag': 'python' }, { 'tagged_item': { - 'id': 1, 'text': 'Remember the milk', }, 'tag': 'reminder' @@ -198,19 +147,12 @@ class TestGenericRelatedFieldToNative(TestCase): ] self.assertEqual(serializer.data, expected) - def test_custom_related_field(self): - contact = Contact.objects.create(name='Lukas Buenger', slug='lukas-buenger') - Tag.objects.create(tagged_item=contact, tag='developer') - - contact_related_field = serializers.HyperlinkedRelatedField(view_name='contact-detail', - slug_url_kwarg='my_own_slug') - + def test_mixed_serializers(self): class TagSerializer(serializers.ModelSerializer): - tagged_item = GenericRelatedField([ - GenericRelationOption(Bookmark, 'bookmark-detail'), - GenericRelationOption(Note, 'note-detail', as_hyperlink=False), - GenericRelationOption(Contact, 'contact-detail', related_field=contact_related_field), - ], source='tagged_item') + tagged_item = GenericRelatedField({ + Bookmark: BookmarkSerializer(), + Note: serializers.HyperlinkedRelatedField(view_name='note-detail'), + }, source='tagged_item', read_only=True) class Meta: model = Tag @@ -219,80 +161,42 @@ class TestGenericRelatedFieldToNative(TestCase): serializer = TagSerializer(Tag.objects.all(), many=True) expected = [ { - 'tagged_item': '/bookmark/1/', + 'tagged_item': { + 'url': 'https://www.djangoproject.com/' + }, 'tag': 'django' }, { - 'tagged_item': '/bookmark/1/', + 'tagged_item': { + 'url': 'https://www.djangoproject.com/' + }, 'tag': 'python' }, { - 'tagged_item': { - 'id': 1, - 'text': 'Remember the milk', - }, + 'tagged_item': '/note/1/', 'tag': 'reminder' - }, - { - 'tagged_item': '/contact/lukas-buenger/', - 'tag': 'developer' } ] self.assertEqual(serializer.data, expected) - def test_custom_serializer(self): - contact = Contact.objects.create(name='Lukas Buenger', slug='lukas-buenger') - Tag.objects.create(tagged_item=contact, tag='developer') - - contact_related_field = serializers.HyperlinkedRelatedField(view_name='contact-detail', - slug_url_kwarg='my_own_slug') - - class ContactSerializer(serializers.ModelSerializer): - class Meta: - model = Contact - exclude = ('id', 'slug', ) - - + def test_invalid_model(self): + # Leaving out the Note model should result in a ValidationError class TagSerializer(serializers.ModelSerializer): - tagged_item = GenericRelatedField([ - GenericRelationOption(Bookmark, 'bookmark-detail'), - GenericRelationOption(Note, 'note-detail', as_hyperlink=False), - GenericRelationOption(Contact, 'contact-detail', as_hyperlink=False, related_field=contact_related_field, - serializer=ContactSerializer()), - ], source='tagged_item') + tagged_item = GenericRelatedField({ + Bookmark: BookmarkSerializer(), + }, source='tagged_item', read_only=True) class Meta: model = Tag exclude = ('id', 'content_type', 'object_id', ) - serializer = TagSerializer(Tag.objects.all(), many=True) - expected = [ - { - 'tagged_item': '/bookmark/1/', - 'tag': 'django' - }, - { - 'tagged_item': '/bookmark/1/', - 'tag': 'python' - }, - { - 'tagged_item': { - 'id': 1, - 'text': 'Remember the milk', - }, - 'tag': 'reminder' - }, - { - 'tagged_item': { - 'name': 'Lukas Buenger' - }, - 'tag': 'developer' - } - ] - self.assertEqual(serializer.data, expected) + + def call_data(): + return serializer.data + self.assertRaises(ValidationError, call_data) -class TestGenericRelatedFieldFromNative(TestCase): +class TestGenericRelatedFieldSerialization(TestCase): urls = 'rest_framework.tests.relations_generic' @@ -302,13 +206,12 @@ class TestGenericRelatedFieldFromNative(TestCase): Tag.objects.create(tagged_item=self.bookmark, tag='python') self.note = Note.objects.create(text='Remember the milk') - def test_default(self): - + def test_hyperlink_serialization(self): class TagSerializer(serializers.ModelSerializer): - tagged_item = GenericRelatedField([ - GenericRelationOption(Bookmark, 'bookmark-detail'), - GenericRelationOption(Note, 'note-detail'), - ], source='tagged_item') + tagged_item = GenericRelatedField({ + Bookmark: serializers.HyperlinkedRelatedField(view_name='bookmark-detail'), + Note: serializers.HyperlinkedRelatedField(view_name='note-detail'), + }, source='tagged_item', read_only=False) class Meta: model = Tag @@ -318,10 +221,82 @@ class TestGenericRelatedFieldFromNative(TestCase): 'tag': 'reminder', 'tagged_item': reverse('note-detail', kwargs={'pk': self.note.pk}) }) - serializer.is_valid() expected = { 'tagged_item': '/note/1/', 'tag': 'reminder' } self.assertEqual(serializer.data, expected) + + def test_configuration_error(self): + class TagSerializer(serializers.ModelSerializer): + tagged_item = GenericRelatedField({ + Bookmark: BookmarkSerializer(), + Note: serializers.HyperlinkedRelatedField(view_name='note-detail'), + }, source='tagged_item', read_only=False) + + class Meta: + model = Tag + exclude = ('id', 'content_type', 'object_id', ) + + serializer = TagSerializer(data={ + 'tag': 'reminder', + 'tagged_item': reverse('note-detail', kwargs={'pk': self.note.pk}) + }) + self.assertRaises(ConfigurationError, serializer.is_valid) + + def test_not_registered_view_name(self): + class TagSerializer(serializers.ModelSerializer): + tagged_item = GenericRelatedField({ + Bookmark: serializers.HyperlinkedRelatedField(view_name='bookmark-detail'), + }, source='tagged_item', read_only=False) + + class Meta: + model = Tag + exclude = ('id', 'content_type', 'object_id', ) + + serializer = TagSerializer(data={ + 'tag': 'reminder', + 'tagged_item': reverse('note-detail', kwargs={'pk': self.note.pk}) + }) + self.assertFalse(serializer.is_valid()) + + def test_invalid_url(self): + class TagSerializer(serializers.ModelSerializer): + tagged_item = GenericRelatedField({ + Bookmark: serializers.HyperlinkedRelatedField(view_name='bookmark-detail'), + }, source='tagged_item', read_only=False) + + class Meta: + model = Tag + exclude = ('id', 'content_type', 'object_id', ) + + serializer = TagSerializer(data={ + 'tag': 'reminder', + 'tagged_item': 'foo-bar' + }) + self.assertFalse(serializer.is_valid()) + + def test_serializer_save(self): + class TagSerializer(serializers.ModelSerializer): + tagged_item = GenericRelatedField({ + Bookmark: serializers.HyperlinkedRelatedField(view_name='bookmark-detail'), + Note: serializers.HyperlinkedRelatedField(view_name='note-detail'), + }, source='tagged_item', read_only=False) + + class Meta: + model = Tag + exclude = ('id', 'content_type', 'object_id', ) + + serializer = TagSerializer(data={ + 'tag': 'reminder', + 'tagged_item': reverse('note-detail', kwargs={'pk': self.note.pk}) + }) + serializer.is_valid() + expected = { + 'tagged_item': '/note/1/', + 'tag': 'reminder' + } + serializer.save() + tag = Tag.objects.get(pk=3) + self.assertEqual(tag.tagged_item, self.note) \ No newline at end of file