From 27eb2100f66574299de6d9c881a2a1602f626626 Mon Sep 17 00:00:00 2001 From: Bart Vandendriessche Date: Mon, 10 Feb 2014 15:12:21 +0100 Subject: [PATCH 1/7] Remove **extra argument from reverse function django.core.urlresolvers.reverse doesn't accept a **kwargs attribute --- rest_framework/reverse.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rest_framework/reverse.py b/rest_framework/reverse.py index a51b07f54..07187765f 100644 --- a/rest_framework/reverse.py +++ b/rest_framework/reverse.py @@ -6,7 +6,7 @@ from django.core.urlresolvers import reverse as django_reverse from django.utils.functional import lazy -def reverse(viewname, args=None, kwargs=None, request=None, format=None, **extra): +def reverse(viewname, args=None, kwargs=None, request=None, format=None): """ Same as `django.core.urlresolvers.reverse`, but optionally takes a request and returns a fully qualified URL, using the request to get the base URL. @@ -14,7 +14,8 @@ def reverse(viewname, args=None, kwargs=None, request=None, format=None, **extra if format is not None: kwargs = kwargs or {} kwargs['format'] = format - url = django_reverse(viewname, args=args, kwargs=kwargs, **extra) + + url = django_reverse(viewname, args=args, kwargs=kwargs) if request: return request.build_absolute_uri(url) return url From 3a162b7e338c4d5eb55f24b7be7fd2b8d177b8fb Mon Sep 17 00:00:00 2001 From: Bart Vandendriessche Date: Mon, 10 Feb 2014 17:31:45 +0100 Subject: [PATCH 2/7] Make view kwargs available to HyperLinkedIdentityField and HyperlinkedRelatedField --- rest_framework/relations.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 02185c2ff..b3ecc6ccf 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -358,8 +358,13 @@ class HyperlinkedRelatedField(RelatedField): May raise a `NoReverseMatch` if the `view_name` and `lookup_field` attributes are not configured to correctly match the URL conf. """ + view_kwargs = {} + if self.context.get('view'): + view_kwargs = self.context['view'].kwargs + lookup_field = getattr(obj, self.lookup_field) - kwargs = {self.lookup_field: lookup_field} + kwargs = dict(view_kwargs.items() + {self.lookup_field: lookup_field}.items()) + try: return reverse(view_name, kwargs=kwargs, request=request, format=format) except NoReverseMatch: @@ -369,7 +374,8 @@ class HyperlinkedRelatedField(RelatedField): # Only try pk if it has been explicitly set. # Otherwise, the default `lookup_field = 'pk'` has us covered. pk = obj.pk - kwargs = {self.pk_url_kwarg: pk} + kwargs = dict(view_kwargs.items() + {self.pk_url_kwarg: pk}.items()) + try: return reverse(view_name, kwargs=kwargs, request=request, format=format) except NoReverseMatch: @@ -378,7 +384,8 @@ class HyperlinkedRelatedField(RelatedField): slug = getattr(obj, self.slug_field, None) if slug is not None: # Only try slug if it corresponds to an attribute on the object. - kwargs = {self.slug_url_kwarg: slug} + kwargs = dict(view_kwargs.items() + {self.slug_url_kwarg: slug}.items()) + try: ret = reverse(view_name, kwargs=kwargs, request=request, format=format) if self.slug_field == 'slug' and self.slug_url_kwarg == 'slug': @@ -565,12 +572,18 @@ class HyperlinkedIdentityField(Field): attributes are not configured to correctly match the URL conf. """ lookup_field = getattr(obj, self.lookup_field, None) - kwargs = {self.lookup_field: lookup_field} + # Handle unsaved object case if lookup_field is None: return None + view_kwargs = {} + if self.context.get('view'): + view_kwargs = self.context['view'].kwargs + + kwargs = dict(view_kwargs.items() + {self.lookup_field: lookup_field}.items()) + try: return reverse(view_name, kwargs=kwargs, request=request, format=format) except NoReverseMatch: @@ -579,7 +592,8 @@ class HyperlinkedIdentityField(Field): if self.pk_url_kwarg != 'pk': # Only try pk lookup if it has been explicitly set. # Otherwise, the default `lookup_field = 'pk'` has us covered. - kwargs = {self.pk_url_kwarg: obj.pk} + kwargs = dict(view_kwargs.items() + {self.pk_url_kwarg: obj.pk}.items()) + try: return reverse(view_name, kwargs=kwargs, request=request, format=format) except NoReverseMatch: @@ -588,7 +602,8 @@ class HyperlinkedIdentityField(Field): slug = getattr(obj, self.slug_field, None) if slug: # Only use slug lookup if a slug field exists on the model - kwargs = {self.slug_url_kwarg: slug} + kwargs = dict(view_kwargs.items() + {self.slug_url_kwarg: slug}.items()) + try: return reverse(view_name, kwargs=kwargs, request=request, format=format) except NoReverseMatch: From 2efa82d62b8637927107f94e4c1d0096e7104785 Mon Sep 17 00:00:00 2001 From: Bart Vandendriessche Date: Tue, 11 Feb 2014 08:55:56 +0100 Subject: [PATCH 3/7] Dictionary concatenation should now work with Python 3 --- rest_framework/relations.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index b3ecc6ccf..a33172b59 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -363,7 +363,7 @@ class HyperlinkedRelatedField(RelatedField): view_kwargs = self.context['view'].kwargs lookup_field = getattr(obj, self.lookup_field) - kwargs = dict(view_kwargs.items() + {self.lookup_field: lookup_field}.items()) + kwargs = dict(view_kwargs, **{self.lookup_field: lookup_field}) try: return reverse(view_name, kwargs=kwargs, request=request, format=format) @@ -374,7 +374,7 @@ class HyperlinkedRelatedField(RelatedField): # Only try pk if it has been explicitly set. # Otherwise, the default `lookup_field = 'pk'` has us covered. pk = obj.pk - kwargs = dict(view_kwargs.items() + {self.pk_url_kwarg: pk}.items()) + kwargs = dict(view_kwargs, **{self.pk_url_kwarg: pk}) try: return reverse(view_name, kwargs=kwargs, request=request, format=format) @@ -384,7 +384,7 @@ class HyperlinkedRelatedField(RelatedField): slug = getattr(obj, self.slug_field, None) if slug is not None: # Only try slug if it corresponds to an attribute on the object. - kwargs = dict(view_kwargs.items() + {self.slug_url_kwarg: slug}.items()) + kwargs = dict(view_kwargs, **{self.slug_url_kwarg: slug}) try: ret = reverse(view_name, kwargs=kwargs, request=request, format=format) @@ -582,7 +582,7 @@ class HyperlinkedIdentityField(Field): if self.context.get('view'): view_kwargs = self.context['view'].kwargs - kwargs = dict(view_kwargs.items() + {self.lookup_field: lookup_field}.items()) + kwargs = dict(view_kwargs, **{self.lookup_field: lookup_field}) try: return reverse(view_name, kwargs=kwargs, request=request, format=format) @@ -592,7 +592,7 @@ class HyperlinkedIdentityField(Field): if self.pk_url_kwarg != 'pk': # Only try pk lookup if it has been explicitly set. # Otherwise, the default `lookup_field = 'pk'` has us covered. - kwargs = dict(view_kwargs.items() + {self.pk_url_kwarg: obj.pk}.items()) + kwargs = dict(view_kwargs, **{self.pk_url_kwarg: obj.pk}) try: return reverse(view_name, kwargs=kwargs, request=request, format=format) @@ -602,7 +602,7 @@ class HyperlinkedIdentityField(Field): slug = getattr(obj, self.slug_field, None) if slug: # Only use slug lookup if a slug field exists on the model - kwargs = dict(view_kwargs.items() + {self.slug_url_kwarg: slug}.items()) + kwargs = dict(view_kwargs, **{self.slug_url_kwarg: slug}) try: return reverse(view_name, kwargs=kwargs, request=request, format=format) From 855a2eb3b1cbe95bb1ca31b053be6cd5aefbf498 Mon Sep 17 00:00:00 2001 From: Bart Vandendriessche Date: Tue, 11 Feb 2014 10:06:44 +0100 Subject: [PATCH 4/7] Add test for hyperlinkedserializers mapped to a url with a captured parameter --- rest_framework/tests/test_hyperlinkedserializers.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/rest_framework/tests/test_hyperlinkedserializers.py b/rest_framework/tests/test_hyperlinkedserializers.py index 83d460435..13ffe8656 100644 --- a/rest_framework/tests/test_hyperlinkedserializers.py +++ b/rest_framework/tests/test_hyperlinkedserializers.py @@ -104,6 +104,7 @@ urlpatterns = patterns('', 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'^(?P<scope>\w[\w-]*)/albums/(?P<title>\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'), ) @@ -211,6 +212,11 @@ class TestHyperlinkedIdentityFieldLookup(TestCase): 'bar': {'title': 'bar', 'url': 'http://testserver/albums/bar/'}, 'baz': {'title': 'baz', 'url': 'http://testserver/albums/baz/'} } + self.data_scoped = { + 'foo': {'title': 'foo', 'url': 'http://testserver/scope/albums/foo/'}, + 'bar': {'title': 'bar', 'url': 'http://testserver/can-be/albums/bar/'}, + 'baz': {'title': 'baz', 'url': 'http://testserver/random/albums/baz/'} + } def test_lookup_field(self): """ @@ -223,6 +229,13 @@ class TestHyperlinkedIdentityFieldLookup(TestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data, self.data[album.title]) + def test_lookup_field_scoped(self): + for album, scope in zip(Album.objects.all(),('scope', 'can-be', 'random')): + request = factory.get('/{0}/albums/{1}/'.format(scope, album.title)) + response = self.detail_view(request, title=album.title, scope=scope) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data, self.data_scoped[album.title]) + class TestCreateWithForeignKeys(TestCase): urls = 'rest_framework.tests.test_hyperlinkedserializers' From da40aacb3bd2dafa6759cbf064fcc878b9edbd1a Mon Sep 17 00:00:00 2001 From: Bart Vandendriessche <bart.vandendriessche@gmail.com> Date: Tue, 11 Feb 2014 10:14:36 +0100 Subject: [PATCH 5/7] Add test for scoped ManyToMany --- .../tests/test_hyperlinkedserializers.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/rest_framework/tests/test_hyperlinkedserializers.py b/rest_framework/tests/test_hyperlinkedserializers.py index 13ffe8656..3ae4856cf 100644 --- a/rest_framework/tests/test_hyperlinkedserializers.py +++ b/rest_framework/tests/test_hyperlinkedserializers.py @@ -98,8 +98,11 @@ urlpatterns = patterns('', url(r'^basic/$', BasicList.as_view(), name='basicmodel-list'), url(r'^basic/(?P<pk>\d+)/$', BasicDetail.as_view(), name='basicmodel-detail'), url(r'^anchor/(?P<pk>\d+)/$', AnchorDetail.as_view(), name='anchor-detail'), + url(r'^(?P<scope>\w[\w-]*)/anchor/(?P<pk>\d+)/$', AnchorDetail.as_view(), name='anchor-detail'), url(r'^manytomany/$', ManyToManyList.as_view(), name='manytomanymodel-list'), url(r'^manytomany/(?P<pk>\d+)/$', ManyToManyDetail.as_view(), name='manytomanymodel-detail'), + url(r'^(?P<scope>\w[\w-]*)/manytomany/$', ManyToManyList.as_view(), name='manytomanymodel-list'), + url(r'^(?P<scope>\w[\w-]*)/manytomany/(?P<pk>\d+)/$', ManyToManyDetail.as_view(), name='manytomanymodel-detail'), url(r'^posts/(?P<pk>\d+)/$', BlogPostDetail.as_view(), name='blogpost-detail'), url(r'^comments/$', BlogPostCommentListCreate.as_view(), name='blogpostcomment-list'), url(r'^comments/(?P<pk>\d+)/$', BlogPostCommentDetail.as_view(), name='blogpostcomment-detail'), @@ -173,6 +176,15 @@ class TestManyToManyHyperlinkedView(TestCase): 'http://testserver/anchor/3/', ] }] + + self.data_scoped = [{ + 'url': 'http://testserver/scope/manytomany/1/', + 'rel': [ + 'http://testserver/scope/anchor/1/', + 'http://testserver/scope/anchor/2/', + 'http://testserver/scope/anchor/3/', + ] + }] self.list_view = ManyToManyList.as_view() self.detail_view = ManyToManyDetail.as_view() @@ -185,6 +197,16 @@ class TestManyToManyHyperlinkedView(TestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data, self.data) + def test_get_list_view_scoped(self): + """ + Scoped GET requests to ListCreateAPIView should return list of objects + with scoped url properties. + """ + request = factory.get('scope/manytomany/') + response = self.list_view(request, scope='scope') + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data, self.data_scoped) + def test_get_detail_view(self): """ GET requests to ListCreateAPIView should return list of objects. @@ -194,6 +216,16 @@ class TestManyToManyHyperlinkedView(TestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data, self.data[0]) + def test_get_detail_view_scoped(self): + """ + Scoped GET requests to ListCreateAPIView should return list of objects + with scoped url properties + """ + request = factory.get('scope/manytomany/1/') + response = self.detail_view(request, pk=1, scope='scope') + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data, self.data_scoped[0]) + class TestHyperlinkedIdentityFieldLookup(TestCase): urls = 'rest_framework.tests.test_hyperlinkedserializers' From 024baf176548ac5edf34ebfdb2cb481aaca55236 Mon Sep 17 00:00:00 2001 From: Bart Vandendriessche <bart.vandendriessche@gmail.com> Date: Tue, 11 Feb 2014 10:30:51 +0100 Subject: [PATCH 6/7] Fix tests for django 1.3.7 and 1.4.10 --- rest_framework/tests/test_hyperlinkedserializers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rest_framework/tests/test_hyperlinkedserializers.py b/rest_framework/tests/test_hyperlinkedserializers.py index 3ae4856cf..1679813f9 100644 --- a/rest_framework/tests/test_hyperlinkedserializers.py +++ b/rest_framework/tests/test_hyperlinkedserializers.py @@ -202,7 +202,7 @@ class TestManyToManyHyperlinkedView(TestCase): Scoped GET requests to ListCreateAPIView should return list of objects with scoped url properties. """ - request = factory.get('scope/manytomany/') + request = factory.get('/scope/manytomany/') response = self.list_view(request, scope='scope') self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data, self.data_scoped) @@ -221,7 +221,7 @@ class TestManyToManyHyperlinkedView(TestCase): Scoped GET requests to ListCreateAPIView should return list of objects with scoped url properties """ - request = factory.get('scope/manytomany/1/') + request = factory.get('/scope/manytomany/1/') response = self.detail_view(request, pk=1, scope='scope') self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data, self.data_scoped[0]) From aa8908fef2f5bfb52892923cb516bafdb3368ee5 Mon Sep 17 00:00:00 2001 From: Bart Vandendriessche <bart.vandendriessche@gmail.com> Date: Tue, 11 Feb 2014 14:28:07 +0100 Subject: [PATCH 7/7] Allow DefaultRouter's api-root to be mounted on a url that includes captured parameters --- rest_framework/routers.py | 4 ++-- rest_framework/tests/test_routers.py | 22 +++++++++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/rest_framework/routers.py b/rest_framework/routers.py index 97b35c10a..2876dfa2a 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -254,10 +254,10 @@ class DefaultRouter(SimpleRouter): class APIRoot(views.APIView): _ignore_model_permissions = True - def get(self, request, format=None): + def get(self, request, format=None, *args, **kwargs): ret = {} for key, url_name in api_root_dict.items(): - ret[key] = reverse(url_name, request=request, format=format) + ret[key] = reverse(url_name, request=request, format=format, kwargs=kwargs) return Response(ret) return APIRoot.as_view() diff --git a/rest_framework/tests/test_routers.py b/rest_framework/tests/test_routers.py index e723f7d45..5eda19188 100644 --- a/rest_framework/tests/test_routers.py +++ b/rest_framework/tests/test_routers.py @@ -2,7 +2,7 @@ from __future__ import unicode_literals from django.db import models from django.test import TestCase from django.core.exceptions import ImproperlyConfigured -from rest_framework import serializers, viewsets, permissions +from rest_framework import serializers, viewsets, permissions, status from rest_framework.compat import include, patterns, url from rest_framework.decorators import link, action from rest_framework.response import Response @@ -164,6 +164,26 @@ class TestNameableRoot(TestCase): expected = 'nameable-root' self.assertEqual(expected, self.urls[0].name) +class TestScopedRoot(TestCase): + class NoteViewSet(viewsets.ModelViewSet): + model = RouterTestModel + + router = DefaultRouter() + router.register(r'notes', NoteViewSet) + + urls = patterns( + '', + url(r'^(?P<scope>\w[\w-]*)/', include(router.urls)), + ) + + def setUp(self): + self.view = self.router.get_api_root_view() + + def test_api_root_is_accessible(self): + request = factory.get('/scope/') # get the api-root + response = self.view(request, scope='scope') + self.assertEqual(response.status_code, status.HTTP_200_OK) + class TestActionKeywordArgs(TestCase): """