Skip to content

Commit e1c4513

Browse files
tomchristietomchristie
committed
Fix NamespaceVersioning with hyperlinked serializer fields
1 parent 83673e8 commit e1c4513

File tree

6 files changed

+72
-61
lines changed

6 files changed

+72
-61
lines changed

‎rest_framework/relations.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
# coding: utf-8
22
from __future__ import unicode_literals
33
from django.core.exceptions import ObjectDoesNotExist, ImproperlyConfigured
4-
from django.core.urlresolvers import get_script_prefix, NoReverseMatch, Resolver404
4+
from django.core.urlresolvers import get_script_prefix, resolve, NoReverseMatch, Resolver404
55
from django.db.models.query import QuerySet
66
from django.utils import six
77
from django.utils.encoding import smart_text
88
from django.utils.six.moves.urllib import parse as urlparse
99
from django.utils.translation import ugettext_lazy as _
1010
from rest_framework.compat import OrderedDict
1111
from rest_framework.fields import get_attribute, empty, Field
12-
from rest_framework.reverse import reverse, resolve
12+
from rest_framework.reverse import reverse
1313
from rest_framework.utils import html
1414

1515

@@ -167,11 +167,10 @@ def __init__(self, view_name=None, **kwargs):
167167
self.lookup_url_kwarg = kwargs.pop('lookup_url_kwarg', self.lookup_field)
168168
self.format = kwargs.pop('format', None)
169169

170-
# We include these simply for dependency injection in tests.
171-
# We can't add them as class attributes or they would expect an
170+
# We include this simply for dependency injection in tests.
171+
# We can't add it as a class attributes or it would expect an
172172
# implicit `self` argument to be passed.
173173
self.reverse = reverse
174-
self.resolve = resolve
175174

176175
super(HyperlinkedRelatedField, self).__init__(**kwargs)
177176

@@ -219,11 +218,18 @@ def to_internal_value(self, data):
219218
data = '/' + data[len(prefix):]
220219

221220
try:
222-
match = self.resolve(data, request=request)
221+
match = resolve(data)
223222
except Resolver404:
224223
self.fail('no_match')
225224

226-
if match.view_name != self.view_name:
225+
try:
226+
expected_viewname = request.versioning_scheme.get_versioned_viewname(
227+
self.view_name, request
228+
)
229+
except AttributeError:
230+
expected_viewname = self.view_name
231+
232+
if match.view_name != expected_viewname:
227233
self.fail('incorrect_match')
228234

229235
try:

‎rest_framework/reverse.py

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,10 @@
33
"""
44
from __future__ import unicode_literals
55
from django.core.urlresolvers import reverse as django_reverse
6-
from django.core.urlresolvers import resolve as django_resolve
76
from django.utils import six
87
from django.utils.functional import lazy
98

109

11-
def resolve(path, urlconf=None, request=None):
12-
"""
13-
If versioning is being used then we pass any `resolve` calls through
14-
to the versioning scheme instance, so that the resulting view name
15-
can be modified if needed.
16-
"""
17-
scheme = getattr(request, 'versioning_scheme', None)
18-
if scheme is not None:
19-
return scheme.resolve(path, urlconf, request)
20-
return django_resolve(path, urlconf)
21-
22-
2310
def reverse(viewname, args=None, kwargs=None, request=None, format=None, **extra):
2411
"""
2512
If versioning is being used then we pass any `reverse` calls through

‎rest_framework/versioning.py

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
# coding: utf-8
22
from __future__ import unicode_literals
33
from django.utils.translation import ugettext_lazy as _
4-
from django.core.urlresolvers import resolve as django_resolve
5-
from django.core.urlresolvers import ResolverMatch
64
from rest_framework import exceptions
75
from rest_framework.compat import unicode_http_header
86
from rest_framework.reverse import _reverse
@@ -26,9 +24,6 @@ def determine_version(self, request, *args, **kwargs):
2624
def reverse(self, viewname, args=None, kwargs=None, request=None, format=None, **extra):
2725
return _reverse(viewname, args, kwargs, request, format, **extra)
2826

29-
def resolve(self, path, urlconf=None):
30-
return django_resolve(path, urlconf)
31-
3227
def is_allowed_version(self, version):
3328
if not self.allowed_versions:
3429
return True
@@ -127,21 +122,13 @@ def determine_version(self, request, *args, **kwargs):
127122

128123
def reverse(self, viewname, args=None, kwargs=None, request=None, format=None, **extra):
129124
if request.version is not None:
130-
viewname = request.version + ':' + viewname
125+
viewname = self.get_versioned_viewname(viewname, request)
131126
return super(NamespaceVersioning, self).reverse(
132127
viewname, args, kwargs, request, format, **extra
133128
)
134129

135-
def resolve(self, path, urlconf=None, request=None):
136-
match = django_resolve(path, urlconf)
137-
if match.namespace:
138-
_, view_name = match.view_name.split(':')
139-
return ResolverMatch(func=match.func,
140-
args=match.args,
141-
kwargs=match.kwargs,
142-
url_name=view_name,
143-
app_name=match.app_name)
144-
return match
130+
def get_versioned_viewname(self, viewname, request):
131+
return request.version + ':' + viewname
145132

146133

147134
class HostNameVersioning(BaseVersioning):

‎tests/test_relations_hyperlink.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from __future__ import unicode_literals
2-
from django.conf.urls import patterns, url
2+
from django.conf.urls import url
33
from django.test import TestCase
44
from rest_framework import serializers
55
from rest_framework.test import APIRequestFactory
@@ -14,8 +14,7 @@
1414

1515
dummy_view = lambda request, pk: None
1616

17-
urlpatterns = patterns(
18-
'',
17+
urlpatterns = [
1918
url(r'^dummyurl/(?P<pk>[0-9]+)/$', dummy_view, name='dummy-url'),
2019
url(r'^manytomanysource/(?P<pk>[0-9]+)/$', dummy_view, name='manytomanysource-detail'),
2120
url(r'^manytomanytarget/(?P<pk>[0-9]+)/$', dummy_view, name='manytomanytarget-detail'),
@@ -24,7 +23,7 @@
2423
url(r'^nullableforeignkeysource/(?P<pk>[0-9]+)/$', dummy_view, name='nullableforeignkeysource-detail'),
2524
url(r'^onetoonetarget/(?P<pk>[0-9]+)/$', dummy_view, name='onetoonetarget-detail'),
2625
url(r'^nullableonetoonesource/(?P<pk>[0-9]+)/$', dummy_view, name='nullableonetoonesource-detail'),
27-
)
26+
]
2827

2928

3029
# ManyToMany

‎tests/test_versioning.py

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
1-
from .utils import MockObject, MockQueryset
1+
from .utils import MockObject, MockQueryset, UsingURLPatterns
22
from django.conf.urls import include, url
33
from django.core.exceptions import ObjectDoesNotExist
44
from rest_framework import serializers
55
from rest_framework import status, versioning
66
from rest_framework.decorators import APIView
77
from rest_framework.response import Response
88
from rest_framework.reverse import reverse
9-
from rest_framework.test import APIRequestFactory, APITestCase, APISimpleTestCase
9+
from rest_framework.test import APIRequestFactory, APITestCase
1010
from rest_framework.versioning import NamespaceVersioning
11+
import pytest
1112

1213

1314
class RequestVersionView(APIView):
@@ -35,18 +36,6 @@ def get(self, request, *args, **kwargs):
3536
mock_view = lambda request: None
3637
dummy_view = lambda request, pk: None
3738

38-
included_patterns = [
39-
url(r'^namespaced/$', mock_view, name='another'),
40-
url(r'^example/(?P<pk>\d+)/$', dummy_view, name='example-detail')
41-
]
42-
43-
urlpatterns = [
44-
url(r'^v1/', include(included_patterns, namespace='v1')),
45-
url(r'^another/$', mock_view, name='another'),
46-
url(r'^(?P<version>[^/]+)/another/$', mock_view, name='another'),
47-
url(r'^example/(?P<pk>\d+)/$', dummy_view, name='example-detail')
48-
]
49-
5039

5140
class TestRequestVersion:
5241
def test_unversioned(self):
@@ -121,8 +110,17 @@ class FakeResolverMatch:
121110
assert response.data == {'version': None}
122111

123112

124-
class TestURLReversing(APITestCase):
125-
urls = 'tests.test_versioning'
113+
class TestURLReversing(UsingURLPatterns, APITestCase):
114+
included = [
115+
url(r'^namespaced/$', mock_view, name='another'),
116+
url(r'^example/(?P<pk>\d+)/$', dummy_view, name='example-detail')
117+
]
118+
119+
urlpatterns = [
120+
url(r'^v1/', include(included, namespace='v1')),
121+
url(r'^another/$', mock_view, name='another'),
122+
url(r'^(?P<version>[^/]+)/another/$', mock_view, name='another'),
123+
]
126124

127125
def test_reverse_unversioned(self):
128126
view = ReverseView.as_view()
@@ -230,10 +228,18 @@ class FakeResolverMatch:
230228
assert response.status_code == status.HTTP_404_NOT_FOUND
231229

232230

233-
class TestHyperlinkedRelatedField(APISimpleTestCase):
234-
urls = 'tests.test_versioning'
231+
class TestHyperlinkedRelatedField(UsingURLPatterns, APITestCase):
232+
included = [
233+
url(r'^namespaced/(?P<pk>\d+)/$', mock_view, name='namespaced'),
234+
]
235+
236+
urlpatterns = [
237+
url(r'^v1/', include(included, namespace='v1')),
238+
url(r'^v2/', include(included, namespace='v2'))
239+
]
235240

236241
def setUp(self):
242+
super(TestHyperlinkedRelatedField, self).setUp()
237243

238244
class HyperlinkedMockQueryset(MockQueryset):
239245
def get(self, **lookup):
@@ -248,13 +254,15 @@ def get(self, **lookup):
248254
MockObject(pk=3, name='baz')
249255
])
250256
self.field = serializers.HyperlinkedRelatedField(
251-
view_name='example-detail',
257+
view_name='namespaced',
252258
queryset=self.queryset
253259
)
254260
request = factory.post('/', urlconf='tests.test_versioning')
255261
request.versioning_scheme = NamespaceVersioning()
262+
request.version = 'v1'
256263
self.field._context = {'request': request}
257264

258265
def test_bug_2489(self):
259-
self.field.to_internal_value('/example/3/')
260-
self.field.to_internal_value('/v1/example/3/')
266+
self.field.to_internal_value('/v1/namespaced/3/')
267+
with pytest.raises(serializers.ValidationError):
268+
self.field.to_internal_value('/v2/namespaced/3/')

‎tests/utils.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,30 @@
22
from django.core.urlresolvers import NoReverseMatch
33

44

5+
class UsingURLPatterns(object):
6+
"""
7+
Isolates URL patterns used during testing on the test class itself.
8+
For example:
9+
10+
class MyTestCase(UsingURLPatterns, TestCase):
11+
urlpatterns = [
12+
...
13+
]
14+
15+
def test_something(self):
16+
...
17+
"""
18+
urls = __name__
19+
20+
def setUp(self):
21+
global urlpatterns
22+
urlpatterns = self.urlpatterns
23+
24+
def tearDown(self):
25+
global urlpatterns
26+
urlpatterns = []
27+
28+
529
class MockObject(object):
630
def __init__(self, **kwargs):
731
self._kwargs = kwargs

0 commit comments

Comments
 (0)