Fix handling of ALLOWED_VERSIONS and no DEFAULT_VERSION (#4370)

When only `ALLOWED_VERSIONS` but no `DEFAULT_VERSION` is specified, a
version should be enforced.

docs/api-guide/versioning.md

@@ -71,8 +71,8 @@ You can also set the versioning scheme on an individual view. Typically you won'
 The following settings keys are also used to control versioning:
 
 * `DEFAULT_VERSION`. The value that should be used for `request.version` when no versioning information is present. Defaults to `None`.
-* `ALLOWED_VERSIONS`. If set, this value will restrict the set of versions that may be returned by the versioning scheme, and will raise an error if the provided version if not in this set. Note that the value used for the `DEFAULT_VERSION` setting is always considered to be part of the `ALLOWED_VERSIONS` set. Defaults to `None`.
-* `VERSION_PARAM`. The string that should used for any versioning parameters, such as in the media type or URL query parameters. Defaults to `'version'`.
+* `ALLOWED_VERSIONS`. If set, this value will restrict the set of versions that may be returned by the versioning scheme, and will raise an error if the provided version is not in this set. Note that the value used for the `DEFAULT_VERSION` setting is always considered to be part of the `ALLOWED_VERSIONS` set (unless it is `None`). Defaults to `None`.
+* `VERSION_PARAM`. The string that should be used for any versioning parameters, such as in the media type or URL query parameters. Defaults to `'version'`.
 
 You can also set your versioning class plus those three values on a per-view or a per-viewset basis by defining your own versioning scheme and using the `default_version`, `allowed_versions` and `version_param` class variables. For example, if you want to use `URLPathVersioning`:
 

rest_framework/versioning.py

@@ -30,7 +30,8 @@ def reverse(self, viewname, args=None, kwargs=None, request=None, format=None, *
     def is_allowed_version(self, version):
         if not self.allowed_versions:
             return True
-        return (version == self.default_version) or (version in self.allowed_versions)
+        return ((version is not None and version == self.default_version) or
+                (version in self.allowed_versions))
 
 
 class AcceptHeaderVersioning(BaseVersioning):

tests/test_versioning.py

@@ -44,14 +44,34 @@ def get(self, request, *args, **kwargs):
         return Response({'url': reverse('another', request=request)})
 
 
-class RequestInvalidVersionView(APIView):
+class AllowedVersionsView(RequestVersionView):
     def determine_version(self, request, *args, **kwargs):
         scheme = self.versioning_class()
         scheme.allowed_versions = ('v1', 'v2')
         return (scheme.determine_version(request, *args, **kwargs), scheme)
 
-    def get(self, request, *args, **kwargs):
-        return Response({'version': request.version})
+
+class AllowedAndDefaultVersionsView(RequestVersionView):
+    def determine_version(self, request, *args, **kwargs):
+        scheme = self.versioning_class()
+        scheme.allowed_versions = ('v1', 'v2')
+        scheme.default_version = 'v2'
+        return (scheme.determine_version(request, *args, **kwargs), scheme)
+
+
+class AllowedWithNoneVersionsView(RequestVersionView):
+    def determine_version(self, request, *args, **kwargs):
+        scheme = self.versioning_class()
+        scheme.allowed_versions = ('v1', 'v2', None)
+        return (scheme.determine_version(request, *args, **kwargs), scheme)
+
+
+class AllowedWithNoneAndDefaultVersionsView(RequestVersionView):
+    def determine_version(self, request, *args, **kwargs):
+        scheme = self.versioning_class()
+        scheme.allowed_versions = ('v1', 'v2', None)
+        scheme.default_version = 'v2'
+        return (scheme.determine_version(request, *args, **kwargs), scheme)
 
 
 factory = APIRequestFactory()
@@ -219,7 +239,7 @@ class FakeResolverMatch:
 class TestInvalidVersion:
     def test_invalid_query_param_versioning(self):
         scheme = versioning.QueryParameterVersioning
-        view = RequestInvalidVersionView.as_view(versioning_class=scheme)
+        view = AllowedVersionsView.as_view(versioning_class=scheme)
 
         request = factory.get('/endpoint/?version=v3')
         response = view(request)
@@ -228,23 +248,23 @@ def test_invalid_query_param_versioning(self):
     @override_settings(ALLOWED_HOSTS=['*'])
     def test_invalid_host_name_versioning(self):
         scheme = versioning.HostNameVersioning
-        view = RequestInvalidVersionView.as_view(versioning_class=scheme)
+        view = AllowedVersionsView.as_view(versioning_class=scheme)
 
         request = factory.get('/endpoint/', HTTP_HOST='v3.example.org')
         response = view(request)
         assert response.status_code == status.HTTP_404_NOT_FOUND
 
     def test_invalid_accept_header_versioning(self):
         scheme = versioning.AcceptHeaderVersioning
-        view = RequestInvalidVersionView.as_view(versioning_class=scheme)
+        view = AllowedVersionsView.as_view(versioning_class=scheme)
 
         request = factory.get('/endpoint/', HTTP_ACCEPT='application/json; version=v3')
         response = view(request)
         assert response.status_code == status.HTTP_406_NOT_ACCEPTABLE
 
     def test_invalid_url_path_versioning(self):
         scheme = versioning.URLPathVersioning
-        view = RequestInvalidVersionView.as_view(versioning_class=scheme)
+        view = AllowedVersionsView.as_view(versioning_class=scheme)
 
         request = factory.get('/v3/endpoint/')
         response = view(request, version='v3')
@@ -255,14 +275,60 @@ class FakeResolverMatch:
             namespace = 'v3'
 
         scheme = versioning.NamespaceVersioning
-        view = RequestInvalidVersionView.as_view(versioning_class=scheme)
+        view = AllowedVersionsView.as_view(versioning_class=scheme)
 
         request = factory.get('/v3/endpoint/')
         request.resolver_match = FakeResolverMatch
         response = view(request, version='v3')
         assert response.status_code == status.HTTP_404_NOT_FOUND
 
 
+class TestAllowedAndDefaultVersion:
+    def test_missing_without_default(self):
+        scheme = versioning.AcceptHeaderVersioning
+        view = AllowedVersionsView.as_view(versioning_class=scheme)
+
+        request = factory.get('/endpoint/', HTTP_ACCEPT='application/json')
+        response = view(request)
+        assert response.status_code == status.HTTP_406_NOT_ACCEPTABLE
+
+    def test_missing_with_default(self):
+        scheme = versioning.AcceptHeaderVersioning
+        view = AllowedAndDefaultVersionsView.as_view(versioning_class=scheme)
+
+        request = factory.get('/endpoint/', HTTP_ACCEPT='application/json')
+        response = view(request)
+        assert response.status_code == status.HTTP_200_OK
+        assert response.data == {'version': 'v2'}
+
+    def test_with_default(self):
+        scheme = versioning.AcceptHeaderVersioning
+        view = AllowedAndDefaultVersionsView.as_view(versioning_class=scheme)
+
+        request = factory.get('/endpoint/',
+                              HTTP_ACCEPT='application/json; version=v2')
+        response = view(request)
+        assert response.status_code == status.HTTP_200_OK
+
+    def test_missing_without_default_but_none_allowed(self):
+        scheme = versioning.AcceptHeaderVersioning
+        view = AllowedWithNoneVersionsView.as_view(versioning_class=scheme)
+
+        request = factory.get('/endpoint/', HTTP_ACCEPT='application/json')
+        response = view(request)
+        assert response.status_code == status.HTTP_200_OK
+        assert response.data == {'version': None}
+
+    def test_missing_with_default_and_none_allowed(self):
+        scheme = versioning.AcceptHeaderVersioning
+        view = AllowedWithNoneAndDefaultVersionsView.as_view(versioning_class=scheme)
+
+        request = factory.get('/endpoint/', HTTP_ACCEPT='application/json')
+        response = view(request)
+        assert response.status_code == status.HTTP_200_OK
+        assert response.data == {'version': 'v2'}
+
+
 class TestHyperlinkedRelatedField(URLPatternsTestCase):
     included = [
         url(r'^namespaced/(?P<pk>\d+)/$', dummy_pk_view, name='namespaced'),