From cc55a7b64310cdd4b8b96e8270a48fd994ede90c Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz Date: Tue, 13 Nov 2012 18:00:41 +0100 Subject: [PATCH 1/7] Returning a Location Header on Create when creating a Resource with HyperlinkedIdentityField of any name --- rest_framework/mixins.py | 19 +++++++++++++++++-- .../tests/hyperlinkedserializers.py | 8 ++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index c3625a88e..f54b5b1f6 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -7,6 +7,7 @@ which allows mixin classes to be composed in interesting ways. from django.http import Http404 from rest_framework import status from rest_framework.response import Response +from rest_framework.fields import HyperlinkedIdentityField class CreateModelMixin(object): @@ -19,9 +20,23 @@ class CreateModelMixin(object): if serializer.is_valid(): self.pre_save(serializer.object) self.object = serializer.save() - return Response(serializer.data, status=status.HTTP_201_CREATED) + headers = self.get_success_headers(serializer) + return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) - + + def get_success_headers(self,serializer): + headers = [] + identity_field = identity_name = None + for name,field in serializer.fields.iteritems(): + if isinstance(field,HyperlinkedIdentityField): + identity_name, identity_field = name, field + if identity_field: + #identity_field.initialize(serializer,"url") + headers.append( + ("Location",identity_field.field_to_native(self.object,identity_name)) + ) + return headers + def pre_save(self, obj): pass diff --git a/rest_framework/tests/hyperlinkedserializers.py b/rest_framework/tests/hyperlinkedserializers.py index 5ab850afa..cc5a19c16 100644 --- a/rest_framework/tests/hyperlinkedserializers.py +++ b/rest_framework/tests/hyperlinkedserializers.py @@ -8,6 +8,7 @@ factory = RequestFactory() class BlogPostCommentSerializer(serializers.ModelSerializer): + custom_identity_field = serializers.HyperlinkedIdentityField(view_name='blogpostcomment-detail') text = serializers.CharField() blog_post_url = serializers.HyperlinkedRelatedField(source='blog_post', view_name='blogpost-detail') @@ -17,6 +18,7 @@ class BlogPostCommentSerializer(serializers.ModelSerializer): class PhotoSerializer(serializers.Serializer): + """ When Adding a HyperlinkedIdentityField to this serializer, the TestCreateWithForeignKeysAndCustomSlug will fail """ description = serializers.CharField() album_url = serializers.HyperlinkedRelatedField(source='album', view_name='album-detail', queryset=Album.objects.all(), slug_field='title', slug_url_kwarg='title') @@ -53,6 +55,9 @@ class BlogPostCommentListCreate(generics.ListCreateAPIView): model = BlogPostComment serializer_class = BlogPostCommentSerializer +class BlogPostCommentDetail(generics.RetrieveAPIView): + model = BlogPostComment + serializer_class = BlogPostCommentSerializer class BlogPostDetail(generics.RetrieveAPIView): model = BlogPost @@ -80,6 +85,7 @@ urlpatterns = patterns('', url(r'^manytomany/(?P\d+)/$', ManyToManyDetail.as_view(), name='manytomanymodel-detail'), url(r'^posts/(?P\d+)/$', BlogPostDetail.as_view(), name='blogpost-detail'), url(r'^comments/$', BlogPostCommentListCreate.as_view(), name='blogpostcomment-list'), + url(r'^comments/(?P\d+)/$', BlogPostCommentDetail.as_view(), name='blogpostcomment-detail'), url(r'^albums/(?P\w[\w-]*)/$', AlbumDetail.as_view(), name='album-detail'), url(r'^photos/$', PhotoListCreate.as_view(), name='photo-list'), url(r'^optionalrelation/(?P<pk>\d+)/$', OptionalRelationDetail.as_view(), name='optionalrelationmodel-detail'), @@ -191,6 +197,7 @@ class TestCreateWithForeignKeys(TestCase): request = factory.post('/comments/', data=data) response = self.create_view(request).render() self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(response["Location"], 'http://testserver/comments/1/') self.assertEqual(self.post.blogpostcomment_set.count(), 1) self.assertEqual(self.post.blogpostcomment_set.all()[0].text, 'A test comment') @@ -215,6 +222,7 @@ class TestCreateWithForeignKeysAndCustomSlug(TestCase): request = factory.post('/photos/', data=data) response = self.list_create_view(request).render() self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertNotIn("Location",response,msg="A Serializer without HyperlinkedIdentityField can not produce a valid Location header (for now). Thats why there shouldn'd be one") self.assertEqual(self.post.photo_set.count(), 1) self.assertEqual(self.post.photo_set.all()[0].description, 'A test photo') From 573de11b233a85347456a4d7e50fd7345d13db03 Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz <ludi182@hotmail.com> Date: Tue, 13 Nov 2012 18:07:38 +0100 Subject: [PATCH 2/7] changed buggy response + code ploishing reponse didnt handle any headers at all. Accepts now a dict of headers and sets those properly --- rest_framework/mixins.py | 11 +++-------- rest_framework/response.py | 5 ++++- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index f54b5b1f6..eddd8f49e 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -25,16 +25,11 @@ class CreateModelMixin(object): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) def get_success_headers(self,serializer): - headers = [] - identity_field = identity_name = None + headers = {} for name,field in serializer.fields.iteritems(): if isinstance(field,HyperlinkedIdentityField): - identity_name, identity_field = name, field - if identity_field: - #identity_field.initialize(serializer,"url") - headers.append( - ("Location",identity_field.field_to_native(self.object,identity_name)) - ) + headers["Location"] = field.field_to_native(self.object,name) + break return headers def pre_save(self, obj): diff --git a/rest_framework/response.py b/rest_framework/response.py index 0de01204d..88f0019fc 100644 --- a/rest_framework/response.py +++ b/rest_framework/response.py @@ -20,9 +20,12 @@ class Response(SimpleTemplateResponse): """ super(Response, self).__init__(None, status=status) self.data = data - self.headers = headers and headers[:] or [] self.template_name = template_name self.exception = exception + + if headers: + for name,value in headers.iteritems(): + self[name] = value @property def rendered_content(self): From 851dff1644f6dafd29838a997a788df51c0570a3 Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz <ludi182@hotmail.com> Date: Tue, 13 Nov 2012 18:39:07 +0100 Subject: [PATCH 3/7] fixed a bug on testing throttling headers after changing the headers storing of reponse --- rest_framework/tests/throttling.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/tests/throttling.py b/rest_framework/tests/throttling.py index 0b94c25ba..4b98b9414 100644 --- a/rest_framework/tests/throttling.py +++ b/rest_framework/tests/throttling.py @@ -106,7 +106,7 @@ class ThrottlingTests(TestCase): if expect is not None: self.assertEquals(response['X-Throttle-Wait-Seconds'], expect) else: - self.assertFalse('X-Throttle-Wait-Seconds' in response.headers) + self.assertFalse('X-Throttle-Wait-Seconds' in response) def test_seconds_fields(self): """ From b341dc70af828d066eb3891e8eafb6337cdd2d04 Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz <ludi182@hotmail.com> Date: Tue, 13 Nov 2012 19:15:42 +0100 Subject: [PATCH 4/7] fixed ugly code Location header is set just, if there is a Location field on the serializer. --- rest_framework/mixins.py | 14 ++++++-------- rest_framework/tests/hyperlinkedserializers.py | 7 +++---- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index eddd8f49e..832365e56 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -20,17 +20,15 @@ class CreateModelMixin(object): if serializer.is_valid(): self.pre_save(serializer.object) self.object = serializer.save() - headers = self.get_success_headers(serializer) + headers = self.get_success_headers(serializer.data) return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) - def get_success_headers(self,serializer): - headers = {} - for name,field in serializer.fields.iteritems(): - if isinstance(field,HyperlinkedIdentityField): - headers["Location"] = field.field_to_native(self.object,name) - break - return headers + def get_success_headers(self,data): + if "url" in data: + return {'Location':data.get("url")} + else: + return {} def pre_save(self, obj): pass diff --git a/rest_framework/tests/hyperlinkedserializers.py b/rest_framework/tests/hyperlinkedserializers.py index cc5a19c16..5fc935ae4 100644 --- a/rest_framework/tests/hyperlinkedserializers.py +++ b/rest_framework/tests/hyperlinkedserializers.py @@ -8,17 +8,16 @@ factory = RequestFactory() class BlogPostCommentSerializer(serializers.ModelSerializer): - custom_identity_field = serializers.HyperlinkedIdentityField(view_name='blogpostcomment-detail') + url = serializers.HyperlinkedIdentityField(view_name='blogpostcomment-detail') text = serializers.CharField() blog_post_url = serializers.HyperlinkedRelatedField(source='blog_post', view_name='blogpost-detail') class Meta: model = BlogPostComment - fields = ('text', 'blog_post_url') + fields = ('text', 'blog_post_url', 'url') class PhotoSerializer(serializers.Serializer): - """ When Adding a HyperlinkedIdentityField to this serializer, the TestCreateWithForeignKeysAndCustomSlug will fail """ description = serializers.CharField() album_url = serializers.HyperlinkedRelatedField(source='album', view_name='album-detail', queryset=Album.objects.all(), slug_field='title', slug_url_kwarg='title') @@ -222,7 +221,7 @@ class TestCreateWithForeignKeysAndCustomSlug(TestCase): request = factory.post('/photos/', data=data) response = self.list_create_view(request).render() self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertNotIn("Location",response,msg="A Serializer without HyperlinkedIdentityField can not produce a valid Location header (for now). Thats why there shouldn'd be one") + self.assertNotIn("Location", response, msg="Location should only be included if there is a 'url' field on the serializer") self.assertEqual(self.post.photo_set.count(), 1) self.assertEqual(self.post.photo_set.all()[0].description, 'A test photo') From 3a30a9b1cbb4444adf3cbb1d3d80c637b5f4f2ca Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz <ludi182@hotmail.com> Date: Tue, 13 Nov 2012 20:30:18 +0100 Subject: [PATCH 5/7] removed useless line after polishing code added it in first commit but after third it became useless. --- rest_framework/mixins.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index 832365e56..f6119aa18 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -7,7 +7,6 @@ which allows mixin classes to be composed in interesting ways. from django.http import Http404 from rest_framework import status from rest_framework.response import Response -from rest_framework.fields import HyperlinkedIdentityField class CreateModelMixin(object): From 8b999c6bb500a045c6c32412009cbd3b1cd5a56b Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz <ludi182@hotmail.com> Date: Wed, 14 Nov 2012 11:46:16 +0100 Subject: [PATCH 6/7] polishing code and adding myself to auhtors file --- docs/topics/credits.md | 2 ++ rest_framework/mixins.py | 4 ++-- rest_framework/tests/hyperlinkedserializers.py | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/topics/credits.md b/docs/topics/credits.md index 22d08df7a..8e71c937a 100644 --- a/docs/topics/credits.md +++ b/docs/topics/credits.md @@ -59,6 +59,7 @@ The following people have helped make REST framework great. * Toni Michel - [tonimichel] * Ben Konrath - [benkonrath] * Marc Aymerich - [glic3rinu] +* Ludwig Kraatz - [ludwigkraatz] Many thanks to everyone who's contributed to the project. @@ -153,3 +154,4 @@ To contact the author directly: [tonimichel]: https://github.com/tonimichel [benkonrath]: https://github.com/benkonrath [glic3rinu]: https://github.com/glic3rinu +[ludwigkraatz]: https://github.com/ludwigkraatz diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index f6119aa18..089425d55 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -24,8 +24,8 @@ class CreateModelMixin(object): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) def get_success_headers(self,data): - if "url" in data: - return {'Location':data.get("url")} + if 'url' in data: + return {'Location': data.get('url')} else: return {} diff --git a/rest_framework/tests/hyperlinkedserializers.py b/rest_framework/tests/hyperlinkedserializers.py index 5fc935ae4..d7effce70 100644 --- a/rest_framework/tests/hyperlinkedserializers.py +++ b/rest_framework/tests/hyperlinkedserializers.py @@ -196,7 +196,7 @@ class TestCreateWithForeignKeys(TestCase): request = factory.post('/comments/', data=data) response = self.create_view(request).render() self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(response["Location"], 'http://testserver/comments/1/') + self.assertEqual(response['Location'], 'http://testserver/comments/1/') self.assertEqual(self.post.blogpostcomment_set.count(), 1) self.assertEqual(self.post.blogpostcomment_set.all()[0].text, 'A test comment') @@ -221,7 +221,7 @@ class TestCreateWithForeignKeysAndCustomSlug(TestCase): request = factory.post('/photos/', data=data) response = self.list_create_view(request).render() self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertNotIn("Location", response, msg="Location should only be included if there is a 'url' field on the serializer") + self.assertNotIn('Location', response, msg='Location should only be included if there is a "url" field on the serializer') self.assertEqual(self.post.photo_set.count(), 1) self.assertEqual(self.post.photo_set.all()[0].description, 'A test photo') From d9c62c20a7a025d8e94fb881641731b340088f98 Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz <ludi182@hotmail.com> Date: Wed, 14 Nov 2012 13:24:20 +0100 Subject: [PATCH 7/7] once more polished --- rest_framework/mixins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index 089425d55..cd104a7c9 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -23,7 +23,7 @@ class CreateModelMixin(object): return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) - def get_success_headers(self,data): + def get_success_headers(self, data): if 'url' in data: return {'Location': data.get('url')} else: