From d0935d1fbb87711b0ffda8655c44ede29ee4208a Mon Sep 17 00:00:00 2001 From: Marko Tibold Date: Mon, 10 Dec 2012 23:10:04 +0100 Subject: [PATCH 01/11] get_excluded_fieldnames() should respect Meta options' ability to be either a tuple or list. Fixes #490. Refactored `if self.opt.fields` out of the for loop. Updated and cleaned up the validation-tests. --- rest_framework/serializers.py | 6 +++--- rest_framework/tests/serializer.py | 5 ++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 7eab98606..c3f260c77 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -132,9 +132,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 diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 455fa270b..a16f6abd8 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -66,6 +66,7 @@ class AlbumsSerializer(serializers.ModelSerializer): class Meta: model = Album + fields = ['title'] # lists are also valid options class BasicTests(TestCase): @@ -282,9 +283,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() From 1815cdd24732e8102ccdf7d28cb5f0cc400c7eaf Mon Sep 17 00:00:00 2001 From: Venkat Date: Mon, 10 Dec 2012 17:46:21 -0800 Subject: [PATCH 02/11] Making sure the assert does not fail when required=False, read_only=True --- rest_framework/fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 285ec9be1..827459738 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 = {} From 80adaecc4307ba802fcb7e45b2f178d2102a41e9 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 11 Dec 2012 09:04:47 +0000 Subject: [PATCH 03/11] Added @annacoder. Thanks! --- docs/topics/credits.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/topics/credits.md b/docs/topics/credits.md index dfa1ee0f8..dc98dccba 100644 --- a/docs/topics/credits.md +++ b/docs/topics/credits.md @@ -74,6 +74,7 @@ The following people have helped make REST framework great. * Reinout van Rees - [reinout] * Michael Richards - [justanotherbody] * Ben Roberts - [roberts81] +* Venkata Subramanian Mahalingam - [annacoder] Many thanks to everyone who's contributed to the project. @@ -183,3 +184,4 @@ 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 From 80f15c598afe138df4170ceb2198484889511d0c Mon Sep 17 00:00:00 2001 From: George Kappel Date: Tue, 11 Dec 2012 09:14:52 -0600 Subject: [PATCH 04/11] Added depth test --- rest_framework/runtests/runtests.py | 5 +++++ rest_framework/tests/serializer.py | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+) 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/tests/serializer.py b/rest_framework/tests/serializer.py index a16f6abd8..f80762f0f 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -726,3 +726,24 @@ class SerializerPickleTests(TestCase): model = Person fields = ('name', 'age') pickle.dumps(InnerPersonSerializer(Person(name="Noah", age=950)).data) + +class DepthTest(TestCase): + def test_depth(self): + user = Person.objects.create(name="django",age=1) + post = BlogPost.objects.create(title="Test blog post", writer=user) + + class PersonSerializer(serializers.ModelSerializer): + class Meta: + model = Person + fields = ("name", "age") + + 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) From 17b77fc446df29e7708c210eade8369c7babc466 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 11 Dec 2012 21:07:11 +0000 Subject: [PATCH 05/11] Added @gkrappel. Thank you! --- docs/topics/credits.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/topics/credits.md b/docs/topics/credits.md index dc98dccba..674c83783 100644 --- a/docs/topics/credits.md +++ b/docs/topics/credits.md @@ -75,6 +75,7 @@ The following people have helped make REST framework great. * Michael Richards - [justanotherbody] * Ben Roberts - [roberts81] * Venkata Subramanian Mahalingam - [annacoder] +* George Kappel - [gkappel] Many thanks to everyone who's contributed to the project. @@ -185,3 +186,4 @@ To contact the author directly: [justanotherbody]: https://github.com/justanotherbody [roberts81]: https://github.com/roberts81 [annacoder]: https://github.com/annacoder +[gkappel]: https://github.com/gkappel From 405822330958c5432dde56b07a61b223c03ca4c7 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 11 Dec 2012 21:07:25 +0000 Subject: [PATCH 06/11] Fix broken nested fields --- rest_framework/compat.py | 10 ++++++++ rest_framework/fields.py | 7 ++---- rest_framework/serializers.py | 38 +++++++++++++++--------------- rest_framework/tests/serializer.py | 37 +++++++++++++++++++++-------- 4 files changed, 58 insertions(+), 34 deletions(-) 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 827459738..75ce1b9f9 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -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/serializers.py b/rest_framework/serializers.py index c3f260c77..ebeb43e83 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -237,7 +237,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 +300,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,15 +484,10 @@ 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 validate(self, attrs): + # restored_object = self.restore_object(attrs, instance=getattr(self, 'object', None)) + # restored_object.full_clean(exclude=list(self.get_excluded_fieldnames())) + # return attrs def restore_object(self, attrs, instance=None): """ @@ -517,7 +510,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 f80762f0f..50a5f5a48 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -1,4 +1,5 @@ -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, @@ -727,15 +728,11 @@ class SerializerPickleTests(TestCase): fields = ('name', 'age') pickle.dumps(InnerPersonSerializer(Person(name="Noah", age=950)).data) -class DepthTest(TestCase): - def test_depth(self): - user = Person.objects.create(name="django",age=1) - post = BlogPost.objects.create(title="Test blog post", writer=user) - class PersonSerializer(serializers.ModelSerializer): - class Meta: - model = Person - fields = ("name", "age") +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: @@ -744,6 +741,26 @@ class DepthTest(TestCase): serializer = BlogPostSerializer(instance=post) expected = {'id': 1, 'title': u'Test blog post', - 'writer': {'id': 1, 'name': u'django', 'age':1}} + '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) From 0824761f471ee5130af980acc9fdbb2758a3a92a Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 11 Dec 2012 21:07:48 +0000 Subject: [PATCH 07/11] Version 2.1.9 --- README.md | 8 ++++++++ docs/topics/release-notes.md | 8 ++++++++ rest_framework/__init__.py | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-) 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/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/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 From 85bf4164ddef2ad6d2f58457d6621cb807ab4d29 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 11 Dec 2012 22:09:04 +0000 Subject: [PATCH 08/11] Drop left over code --- rest_framework/serializers.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index ebeb43e83..5465d7b72 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -484,11 +484,6 @@ class ModelSerializer(Serializer): except KeyError: return ModelField(model_field=model_field, **kwargs) - # def validate(self, attrs): - # restored_object = self.restore_object(attrs, instance=getattr(self, 'object', None)) - # restored_object.full_clean(exclude=list(self.get_excluded_fieldnames())) - # return attrs - def restore_object(self, attrs, instance=None): """ Restore the model instance. From 9188d487c3a0465b2a3e0d1c47f76c3df844b7d0 Mon Sep 17 00:00:00 2001 From: Colin Murtaugh Date: Tue, 11 Dec 2012 17:26:08 -0500 Subject: [PATCH 09/11] Replaced SingleObjectBaseView with SingleObjectAPIView --- docs/tutorial/3-class-based-views.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 From 628e3bf001ca71da48a6f3c7bbdf209f2e20b223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20Gro=C3=9F?= Date: Wed, 12 Dec 2012 08:59:19 +0100 Subject: [PATCH 10/11] Added @cmurtaugh. Thanks! --- docs/topics/credits.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/topics/credits.md b/docs/topics/credits.md index 674c83783..cdf57f7ec 100644 --- a/docs/topics/credits.md +++ b/docs/topics/credits.md @@ -76,6 +76,7 @@ The following people have helped make REST framework great. * Ben Roberts - [roberts81] * Venkata Subramanian Mahalingam - [annacoder] * George Kappel - [gkappel] +* Colin Murtaugh - [cmurtaugh] Many thanks to everyone who's contributed to the project. @@ -187,3 +188,4 @@ To contact the author directly: [roberts81]: https://github.com/roberts81 [annacoder]: https://github.com/annacoder [gkappel]: https://github.com/gkappel +[cmurtaugh]: https://github.com/cmurtaugh From 497da7fc699b9e88c966e37bc48739865336683d Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 12 Dec 2012 20:44:55 +0000 Subject: [PATCH 11/11] Clean up field initialization. Fixes #497 --- rest_framework/serializers.py | 16 +++++++-- rest_framework/tests/serializer.py | 54 +++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 5465d7b72..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 @@ -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. diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 50a5f5a48..780177aa0 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -4,7 +4,7 @@ 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): @@ -764,3 +764,55 @@ class DepthTest(TestCase): '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