diff --git a/README.md b/README.md index a4c56103a..1bc9628f0 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,14 @@ To run the tests. # Changelog +## 2.1.9 + +**Date**: 11th Dec 2012 + +* Bugfix: Fix broken nested serialization. +* Bugfix: Fix `Meta.fields` only working as tuple not as list. +* Bugfix: Edge case if unnecessarily specifying `required=False` on read only field. + ## 2.1.8 **Date**: 8th Dec 2012 diff --git a/docs/topics/credits.md b/docs/topics/credits.md index dfa1ee0f8..cdf57f7ec 100644 --- a/docs/topics/credits.md +++ b/docs/topics/credits.md @@ -74,6 +74,9 @@ The following people have helped make REST framework great. * Reinout van Rees - [reinout] * Michael Richards - [justanotherbody] * Ben Roberts - [roberts81] +* Venkata Subramanian Mahalingam - [annacoder] +* George Kappel - [gkappel] +* Colin Murtaugh - [cmurtaugh] Many thanks to everyone who's contributed to the project. @@ -183,3 +186,6 @@ To contact the author directly: [reinout]: https://github.com/reinout [justanotherbody]: https://github.com/justanotherbody [roberts81]: https://github.com/roberts81 +[annacoder]: https://github.com/annacoder +[gkappel]: https://github.com/gkappel +[cmurtaugh]: https://github.com/cmurtaugh diff --git a/docs/topics/release-notes.md b/docs/topics/release-notes.md index 46eb1494d..4f83cfd8f 100644 --- a/docs/topics/release-notes.md +++ b/docs/topics/release-notes.md @@ -4,6 +4,14 @@ > > — Eric S. Raymond, [The Cathedral and the Bazaar][cite]. +## 2.1.9 + +**Date**: 11th Dec 2012 + +* Bugfix: Fix broken nested serialization. +* Bugfix: Fix `Meta.fields` only working as tuple not as list. +* Bugfix: Edge case if unnecessarily specifying `required=False` on read only field. + ## 2.1.8 **Date**: 8th Dec 2012 diff --git a/docs/tutorial/3-class-based-views.md b/docs/tutorial/3-class-based-views.md index a3a18060f..b115b0229 100644 --- a/docs/tutorial/3-class-based-views.md +++ b/docs/tutorial/3-class-based-views.md @@ -109,7 +109,7 @@ The base class provides the core functionality, and the mixin classes provide th class SnippetDetail(mixins.RetrieveModelMixin, mixins.UpdateModelMixin, mixins.DestroyModelMixin, - generics.SingleObjectBaseView): + generics.SingleObjectAPIView): model = Snippet serializer_class = SnippetSerializer @@ -122,7 +122,7 @@ The base class provides the core functionality, and the mixin classes provide th def delete(self, request, *args, **kwargs): return self.destroy(request, *args, **kwargs) -Pretty similar. This time we're using the `SingleObjectBaseView` class to provide the core functionality, and adding in mixins to provide the `.retrieve()`, `.update()` and `.destroy()` actions. +Pretty similar. This time we're using the `SingleObjectAPIView` class to provide the core functionality, and adding in mixins to provide the `.retrieve()`, `.update()` and `.destroy()` actions. ## Using generic class based views diff --git a/rest_framework/__init__.py b/rest_framework/__init__.py index 02a606754..83a6f3022 100644 --- a/rest_framework/__init__.py +++ b/rest_framework/__init__.py @@ -1,3 +1,3 @@ -__version__ = '2.1.8' +__version__ = '2.1.9' VERSION = __version__ # synonym diff --git a/rest_framework/compat.py b/rest_framework/compat.py index 09b763681..d4901437d 100644 --- a/rest_framework/compat.py +++ b/rest_framework/compat.py @@ -19,6 +19,16 @@ except ImportError: import StringIO +# Try to import PIL in either of the two ways it can end up installed. +try: + from PIL import Image +except ImportError: + try: + import Image + except ImportError: + Image = None + + def get_concrete_model(model_cls): try: return model_cls._meta.concrete_model diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 285ec9be1..75ce1b9f9 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -133,7 +133,7 @@ class WritableField(Field): if required is None: self.required = not(read_only) else: - assert not read_only, "Cannot set required=True and read_only=True" + assert not (read_only and required), "Cannot set required=True and read_only=True" self.required = required messages = {} @@ -1056,11 +1056,8 @@ class ImageField(FileField): if f is None: return None - # Try to import PIL in either of the two ways it can end up installed. - try: - from PIL import Image - except ImportError: - import Image + from compat import Image + assert Image is not None, 'PIL must be installed for ImageField support' # We need to get a file object for PIL. We might have a path or we might # have to read the data into memory. diff --git a/rest_framework/runtests/runtests.py b/rest_framework/runtests/runtests.py index 1bd0a5fc8..729ef26ac 100755 --- a/rest_framework/runtests/runtests.py +++ b/rest_framework/runtests/runtests.py @@ -5,6 +5,11 @@ # http://code.djangoproject.com/svn/django/trunk/tests/runtests.py import os import sys +""" +Need to fix sys path so following works without specifically messing with PYTHONPATH +python ./rest_framework/runtests/runtests.py +""" +sys.path.append(os.path.join(os.path.dirname(__file__), "../..")) os.environ['DJANGO_SETTINGS_MODULE'] = 'rest_framework.runtests.settings' from django.conf import settings diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 7eab98606..caa7c980f 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -100,7 +100,8 @@ class BaseSerializer(Field): _options_class = SerializerOptions _dict_class = SortedDictWithMetadata # Set to unsorted dict for backwards compatibility with unsorted implementations. - def __init__(self, instance=None, data=None, files=None, context=None, partial=False, **kwargs): + def __init__(self, instance=None, data=None, files=None, + context=None, partial=False, **kwargs): super(BaseSerializer, self).__init__(**kwargs) self.opts = self._options_class(self.Meta) self.parent = None @@ -132,9 +133,9 @@ class BaseSerializer(Field): Returns the fieldnames that should not be validated. """ excluded_fields = list(self.opts.exclude) - for field in self.fields.keys() + self.get_default_fields().keys(): - if self.opts.fields: - if field not in self.opts.fields + self.opts.exclude: + if self.opts.fields: + for field in self.fields.keys() + self.get_default_fields().keys(): + if field not in list(self.opts.fields) + excluded_fields: excluded_fields.append(field) return excluded_fields @@ -151,8 +152,6 @@ class BaseSerializer(Field): base_fields = copy.deepcopy(self.base_fields) for key, field in base_fields.items(): ret[key] = field - # Set up the field - field.initialize(parent=self, field_name=key) # Add in the default fields default_fields = self.get_default_fields() @@ -172,6 +171,10 @@ class BaseSerializer(Field): for key in self.opts.exclude: ret.pop(key, None) + # Initialize the fields + for key, field in ret.items(): + field.initialize(parent=self, field_name=key) + return ret ##### @@ -186,6 +189,13 @@ class BaseSerializer(Field): if parent.opts.depth: self.opts.depth = parent.opts.depth - 1 + # We need to call initialize here to ensure any nested + # serializers that will have already called initialize on their + # descendants get updated with *their* parent. + # We could be a bit more smart about this, but it'll do for now. + for key, field in self.fields.items(): + field.initialize(parent=self, field_name=key) + ##### # Methods to convert or revert from objects <--> primitive representations. @@ -237,7 +247,8 @@ class BaseSerializer(Field): except ValidationError as err: self._errors[field_name] = self._errors.get(field_name, []) + list(err.messages) - # We don't run .validate() because field-validation failed and thus `attrs` may not be complete. + # If there are already errors, we don't run .validate() because + # field-validation failed and thus `attrs` may not be complete. # which in turn can cause inconsistent validation errors. if not self._errors: try: @@ -299,17 +310,14 @@ class BaseSerializer(Field): Override default so that we can apply ModelSerializer as a nested field to relationships. """ - if self.source: - value = obj for component in self.source.split('.'): - value = getattr(value, component) - if is_simple_callable(value): - value = value() - obj = value + obj = getattr(obj, component) + if is_simple_callable(obj): + obj = obj() else: - value = getattr(obj, field_name) - if is_simple_callable(value): + obj = getattr(obj, field_name) + if is_simple_callable(obj): obj = value() # If the object has an "all" method, assume it's a relationship @@ -486,16 +494,6 @@ class ModelSerializer(Serializer): except KeyError: return ModelField(model_field=model_field, **kwargs) - def validate(self, attrs): - copied_attrs = copy.deepcopy(attrs) - restored_object = self.restore_object(copied_attrs, instance=getattr(self, 'object', None)) - self.perform_model_validation(restored_object) - return attrs - - def perform_model_validation(self, restored_object): - # Call Django's full_clean() which in turn calls: Model.clean_fields(), Model.clean(), Model.validat_unique() - restored_object.full_clean(exclude=list(self.get_excluded_fieldnames())) - def restore_object(self, attrs, instance=None): """ Restore the model instance. @@ -517,7 +515,14 @@ class ModelSerializer(Serializer): for field in self.opts.model._meta.many_to_many: if field.name in attrs: self.m2m_data[field.name] = attrs.pop(field.name) - return self.opts.model(**attrs) + + instance = self.opts.model(**attrs) + try: + instance.full_clean(exclude=list(self.get_excluded_fieldnames())) + except ValidationError, err: + self._errors = err.message_dict + return None + return instance def save(self, save_m2m=True): """ diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 455fa270b..780177aa0 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -1,9 +1,10 @@ -import datetime, pickle +import datetime +import pickle from django.test import TestCase from rest_framework import serializers from rest_framework.tests.models import (Album, ActionItem, Anchor, BasicModel, BlankFieldModel, BlogPost, Book, CallableDefaultValueModel, DefaultValueModel, - ManyToManyModel, Person, ReadOnlyManyToManyModel) + ManyToManyModel, Person, ReadOnlyManyToManyModel, Photo) class SubComment(object): @@ -66,6 +67,7 @@ class AlbumsSerializer(serializers.ModelSerializer): class Meta: model = Album + fields = ['title'] # lists are also valid options class BasicTests(TestCase): @@ -282,9 +284,11 @@ class ValidationTests(TestCase): self.assertEquals(serializer.is_valid(), False) self.assertEquals(serializer.errors, {'info': [u'Ensure this value has at most 12 characters (it has 13).']}) + +class ModelValidationTests(TestCase): def test_validate_unique(self): """ - Just check if serializers.ModelSerializer.perform_model_validation() handles unique checks via .full_clean() + Just check if serializers.ModelSerializer handles unique checks via .full_clean() """ serializer = AlbumsSerializer(data={'title': 'a'}) serializer.is_valid() @@ -723,3 +727,92 @@ class SerializerPickleTests(TestCase): model = Person fields = ('name', 'age') pickle.dumps(InnerPersonSerializer(Person(name="Noah", age=950)).data) + + +class DepthTest(TestCase): + def test_implicit_nesting(self): + writer = Person.objects.create(name="django", age=1) + post = BlogPost.objects.create(title="Test blog post", writer=writer) + + class BlogPostSerializer(serializers.ModelSerializer): + class Meta: + model = BlogPost + depth = 1 + + serializer = BlogPostSerializer(instance=post) + expected = {'id': 1, 'title': u'Test blog post', + 'writer': {'id': 1, 'name': u'django', 'age': 1}} + + self.assertEqual(serializer.data, expected) + + def test_explicit_nesting(self): + writer = Person.objects.create(name="django", age=1) + post = BlogPost.objects.create(title="Test blog post", writer=writer) + + class PersonSerializer(serializers.ModelSerializer): + class Meta: + model = Person + + class BlogPostSerializer(serializers.ModelSerializer): + writer = PersonSerializer() + + class Meta: + model = BlogPost + + serializer = BlogPostSerializer(instance=post) + expected = {'id': 1, 'title': u'Test blog post', + 'writer': {'id': 1, 'name': u'django', 'age': 1}} + + self.assertEqual(serializer.data, expected) + + +class NestedSerializerContextTests(TestCase): + + def test_nested_serializer_context(self): + """ + Regression for #497 + + https://github.com/tomchristie/django-rest-framework/issues/497 + """ + class PhotoSerializer(serializers.ModelSerializer): + class Meta: + model = Photo + fields = ("description", "callable") + + callable = serializers.SerializerMethodField('_callable') + + def _callable(self, instance): + if not 'context_item' in self.context: + raise RuntimeError("context isn't getting passed into 2nd level nested serializer") + return "success" + + class AlbumSerializer(serializers.ModelSerializer): + class Meta: + model = Album + fields = ("photo_set", "callable") + + photo_set = PhotoSerializer(source="photo_set") + callable = serializers.SerializerMethodField("_callable") + + def _callable(self, instance): + if not 'context_item' in self.context: + raise RuntimeError("context isn't getting passed into 1st level nested serializer") + return "success" + + class AlbumCollection(object): + albums = None + + class AlbumCollectionSerializer(serializers.Serializer): + albums = AlbumSerializer(source="albums") + + album1 = Album.objects.create(title="album 1") + album2 = Album.objects.create(title="album 2") + Photo.objects.create(description="Bigfoot", album=album1) + Photo.objects.create(description="Unicorn", album=album1) + Photo.objects.create(description="Yeti", album=album2) + Photo.objects.create(description="Sasquatch", album=album2) + album_collection = AlbumCollection() + album_collection.albums = [album1, album2] + + # This will raise RuntimeError if context doesn't get passed correctly to the nested Serializers + AlbumCollectionSerializer(album_collection, context={'context_item': 'album context'}).data