Merge pull request #2724 from delinhabit/namespaced-versioning-non-api-reversal

Handle reversal of non-API view_name in HyperLinkedRelatedField

Author: tomchristie

rest_framework/reverse.py

@@ -3,6 +3,7 @@
 """
 from __future__ import unicode_literals
 from django.core.urlresolvers import reverse as django_reverse
+from django.core.urlresolvers import NoReverseMatch
 from django.utils import six
 from django.utils.functional import lazy
 
@@ -15,7 +16,13 @@ def reverse(viewname, args=None, kwargs=None, request=None, format=None, **extra
     """
     scheme = getattr(request, 'versioning_scheme', None)
     if scheme is not None:
-        return scheme.reverse(viewname, args, kwargs, request, format, **extra)
+        try:
+            return scheme.reverse(viewname, args, kwargs, request, format, **extra)
+        except NoReverseMatch:
+            # In case the versioning scheme reversal fails, fallback to the
+            # default implementation
+            pass
+
     return _reverse(viewname, args, kwargs, request, format, **extra)
 
 

tests/test_reverse.py

@@ -1,5 +1,6 @@
 from __future__ import unicode_literals
 from django.conf.urls import patterns, url
+from django.core.urlresolvers import NoReverseMatch
 from django.test import TestCase
 from rest_framework.reverse import reverse
 from rest_framework.test import APIRequestFactory
@@ -16,6 +17,18 @@ def null_view(request):
 )
 
 
+class MockVersioningScheme(object):
+
+    def __init__(self, raise_error=False):
+        self.raise_error = raise_error
+
+    def reverse(self, *args, **kwargs):
+        if self.raise_error:
+            raise NoReverseMatch()
+
+        return 'http://scheme-reversed/view'
+
+
 class ReverseTests(TestCase):
     """
     Tests for fully qualified URLs when using `reverse`.
@@ -26,3 +39,17 @@ def test_reversed_urls_are_fully_qualified(self):
         request = factory.get('/view')
         url = reverse('view', request=request)
         self.assertEqual(url, 'http://testserver/view')
+
+    def test_reverse_with_versioning_scheme(self):
+        request = factory.get('/view')
+        request.versioning_scheme = MockVersioningScheme()
+
+        url = reverse('view', request=request)
+        self.assertEqual(url, 'http://scheme-reversed/view')
+
+    def test_reverse_with_versioning_scheme_fallback_to_default_on_error(self):
+        request = factory.get('/view')
+        request.versioning_scheme = MockVersioningScheme(raise_error=True)
+
+        url = reverse('view', request=request)
+        self.assertEqual(url, 'http://testserver/view')

tests/test_versioning.py

@@ -7,6 +7,7 @@
 from rest_framework.reverse import reverse
 from rest_framework.test import APIRequestFactory, APITestCase
 from rest_framework.versioning import NamespaceVersioning
+from rest_framework.relations import PKOnlyObject
 import pytest
 
 
@@ -234,7 +235,7 @@ class FakeResolverMatch:
 
 class TestHyperlinkedRelatedField(UsingURLPatterns, APITestCase):
     included = [
-        url(r'^namespaced/(?P<pk>\d+)/$', dummy_view, name='namespaced'),
+        url(r'^namespaced/(?P<pk>\d+)/$', dummy_pk_view, name='namespaced'),
     ]
 
     urlpatterns = [
@@ -262,3 +263,44 @@ def test_bug_2489(self):
         assert self.field.to_internal_value('/v1/namespaced/3/') == 'object 3'
         with pytest.raises(serializers.ValidationError):
             self.field.to_internal_value('/v2/namespaced/3/')
+
+
+class TestNamespaceVersioningHyperlinkedRelatedFieldScheme(UsingURLPatterns, APITestCase):
+    included = [
+        url(r'^namespaced/(?P<pk>\d+)/$', dummy_pk_view, name='namespaced'),
+    ]
+
+    urlpatterns = [
+        url(r'^v1/', include(included, namespace='v1')),
+        url(r'^v2/', include(included, namespace='v2')),
+        url(r'^non-api/(?P<pk>\d+)/$', dummy_pk_view, name='non-api-view')
+    ]
+
+    def _create_field(self, view_name, version):
+        request = factory.get("/")
+        request.versioning_scheme = NamespaceVersioning()
+        request.version = version
+
+        field = serializers.HyperlinkedRelatedField(
+            view_name=view_name,
+            read_only=True)
+        field._context = {'request': request}
+        return field
+
+    def test_api_url_is_properly_reversed_with_v1(self):
+        field = self._create_field('namespaced', 'v1')
+        assert field.to_representation(PKOnlyObject(3)) == 'http://testserver/v1/namespaced/3/'
+
+    def test_api_url_is_properly_reversed_with_v2(self):
+        field = self._create_field('namespaced', 'v2')
+        assert field.to_representation(PKOnlyObject(5)) == 'http://testserver/v2/namespaced/5/'
+
+    def test_non_api_url_is_properly_reversed_regardless_of_the_version(self):
+        """
+        Regression test for #2711
+        """
+        field = self._create_field('non-api-view', 'v1')
+        assert field.to_representation(PKOnlyObject(10)) == 'http://testserver/non-api/10/'
+
+        field = self._create_field('non-api-view', 'v2')
+        assert field.to_representation(PKOnlyObject(10)) == 'http://testserver/non-api/10/'