Rename base_name => basename for consistency's sake (#5990)

* Rename base_name => basename for consistency

* Update tests to use basename
This commit is contained in:
Ryan P Kilby 2018-07-06 05:03:12 -04:00 committed by Carlton Gibson
parent f89cc066bc
commit 7095021db7
7 changed files with 134 additions and 34 deletions

View File

@ -28,7 +28,7 @@ There are two mandatory arguments to the `register()` method:
Optionally, you may also specify an additional argument: Optionally, you may also specify an additional argument:
* `base_name` - The base to use for the URL names that are created. If unset the basename will be automatically generated based on the `queryset` attribute of the viewset, if it has one. Note that if the viewset does not include a `queryset` attribute then you must set `base_name` when registering the viewset. * `basename` - The base to use for the URL names that are created. If unset the basename will be automatically generated based on the `queryset` attribute of the viewset, if it has one. Note that if the viewset does not include a `queryset` attribute then you must set `basename` when registering the viewset.
The example above would generate the following URL patterns: The example above would generate the following URL patterns:
@ -39,13 +39,13 @@ The example above would generate the following URL patterns:
--- ---
**Note**: The `base_name` argument is used to specify the initial part of the view name pattern. In the example above, that's the `user` or `account` part. **Note**: The `basename` argument is used to specify the initial part of the view name pattern. In the example above, that's the `user` or `account` part.
Typically you won't *need* to specify the `base_name` argument, but if you have a viewset where you've defined a custom `get_queryset` method, then the viewset may not have a `.queryset` attribute set. If you try to register that viewset you'll see an error like this: Typically you won't *need* to specify the `basename` argument, but if you have a viewset where you've defined a custom `get_queryset` method, then the viewset may not have a `.queryset` attribute set. If you try to register that viewset you'll see an error like this:
'base_name' argument not specified, and could not automatically determine the name from the viewset, as it does not have a '.queryset' attribute. 'basename' argument not specified, and could not automatically determine the name from the viewset, as it does not have a '.queryset' attribute.
This means you'll need to explicitly set the `base_name` argument when registering the viewset, as it could not be automatically determined from the model name. This means you'll need to explicitly set the `basename` argument when registering the viewset, as it could not be automatically determined from the model name.
--- ---
@ -53,7 +53,7 @@ This means you'll need to explicitly set the `base_name` argument when registeri
The `.urls` attribute on a router instance is simply a standard list of URL patterns. There are a number of different styles for how you can include these URLs. The `.urls` attribute on a router instance is simply a standard list of URL patterns. There are a number of different styles for how you can include these URLs.
For example, you can append `router.urls` to a list of existing views For example, you can append `router.urls` to a list of existing views...
router = routers.SimpleRouter() router = routers.SimpleRouter()
router.register(r'users', UserViewSet) router.register(r'users', UserViewSet)
@ -65,7 +65,7 @@ For example, you can append `router.urls` to a list of existing views…
urlpatterns += router.urls urlpatterns += router.urls
Alternatively you can use Django's `include` function, like so Alternatively you can use Django's `include` function, like so...
urlpatterns = [ urlpatterns = [
url(r'^forgot-password/$', ForgotPasswordFormView.as_view()), url(r'^forgot-password/$', ForgotPasswordFormView.as_view()),
@ -302,7 +302,7 @@ For another example of setting the `.routes` attribute, see the source code for
If you want to provide totally custom behavior, you can override `BaseRouter` and override the `get_urls(self)` method. The method should inspect the registered viewsets and return a list of URL patterns. The registered prefix, viewset and basename tuples may be inspected by accessing the `self.registry` attribute. If you want to provide totally custom behavior, you can override `BaseRouter` and override the `get_urls(self)` method. The method should inspect the registered viewsets and return a list of URL patterns. The registered prefix, viewset and basename tuples may be inspected by accessing the `self.registry` attribute.
You may also want to override the `get_default_base_name(self, viewset)` method, or else always explicitly set the `base_name` argument when registering your viewsets with the router. You may also want to override the `get_default_basename(self, viewset)` method, or else always explicitly set the `basename` argument when registering your viewsets with the router.
# Third Party Packages # Third Party Packages

View File

@ -51,7 +51,7 @@ Typically we wouldn't do this, but would instead register the viewset with a rou
from rest_framework.routers import DefaultRouter from rest_framework.routers import DefaultRouter
router = DefaultRouter() router = DefaultRouter()
router.register(r'users', UserViewSet, base_name='user') router.register(r'users', UserViewSet, basename='user')
urlpatterns = router.urls urlpatterns = router.urls
Rather than writing your own viewsets, you'll often want to use the existing base classes that provide a default set of behavior. For example: Rather than writing your own viewsets, you'll often want to use the existing base classes that provide a default set of behavior. For example:
@ -269,7 +269,7 @@ Note that you can use any of the standard attributes or method overrides provide
def get_queryset(self): def get_queryset(self):
return self.request.user.accounts.all() return self.request.user.accounts.all()
Note however that upon removal of the `queryset` property from your `ViewSet`, any associated [router][routers] will be unable to derive the base_name of your Model automatically, and so you will have to specify the `base_name` kwarg as part of your [router registration][routers]. Note however that upon removal of the `queryset` property from your `ViewSet`, any associated [router][routers] will be unable to derive the basename of your Model automatically, and so you will have to specify the `basename` kwarg as part of your [router registration][routers].
Also note that although this class provides the complete set of create/list/retrieve/update/destroy actions by default, you can restrict the available operations by using the standard permission classes. Also note that although this class provides the complete set of create/list/retrieve/update/destroy actions by default, you can restrict the available operations by using the standard permission classes.

View File

@ -38,6 +38,16 @@ You can determine your currently installed version using `pip show`:
--- ---
## 3.9.x series
### 3.9.0
**Date**: Unreleased
* Deprecate the `Router.register` `base_name` argument in favor of `basename`. [#5990][gh5990]
* Deprecate the `Router.get_default_base_name` method in favor of `Router.get_default_basename`. [#5990][gh5990]
## 3.8.x series ## 3.8.x series
### 3.8.2 ### 3.8.2
@ -1961,3 +1971,6 @@ For older release notes, [please see the version 2.x documentation][old-release-
[gh5922]: https://github.com/encode/django-rest-framework/issues/5922 [gh5922]: https://github.com/encode/django-rest-framework/issues/5922
[gh5921]: https://github.com/encode/django-rest-framework/issues/5921 [gh5921]: https://github.com/encode/django-rest-framework/issues/5921
[gh5920]: https://github.com/encode/django-rest-framework/issues/5920 [gh5920]: https://github.com/encode/django-rest-framework/issues/5920
<!-- 3.9.0 -->
[gh5990]: https://github.com/encode/django-rest-framework/issues/5990

View File

@ -22,6 +22,8 @@ from collections import OrderedDict, namedtuple
from django.conf.urls import url from django.conf.urls import url
from django.core.exceptions import ImproperlyConfigured from django.core.exceptions import ImproperlyConfigured
from django.urls import NoReverseMatch from django.urls import NoReverseMatch
from django.utils import six
from django.utils.deprecation import RenameMethodsBase
from rest_framework import views from rest_framework import views
from rest_framework.response import Response from rest_framework.response import Response
@ -73,21 +75,37 @@ def flatten(list_of_lists):
return itertools.chain(*list_of_lists) return itertools.chain(*list_of_lists)
class BaseRouter(object): class RenameRouterMethods(RenameMethodsBase):
renamed_methods = (
('get_default_base_name', 'get_default_basename', DeprecationWarning),
)
class BaseRouter(six.with_metaclass(RenameRouterMethods)):
def __init__(self): def __init__(self):
self.registry = [] self.registry = []
def register(self, prefix, viewset, base_name=None): def register(self, prefix, viewset, basename=None, base_name=None):
if base_name is None: if base_name is not None:
base_name = self.get_default_base_name(viewset) msg = "The `base_name` argument has been deprecated in favor of `basename`."
self.registry.append((prefix, viewset, base_name)) warnings.warn(msg, DeprecationWarning, 2)
def get_default_base_name(self, viewset): assert not (basename and base_name), (
"Do not provide both the `basename` and `base_name` arguments.")
if basename is None:
basename = base_name
if basename is None:
basename = self.get_default_basename(viewset)
self.registry.append((prefix, viewset, basename))
def get_default_basename(self, viewset):
""" """
If `base_name` is not specified, attempt to automatically determine If `basename` is not specified, attempt to automatically determine
it from the viewset. it from the viewset.
""" """
raise NotImplementedError('get_default_base_name must be overridden') raise NotImplementedError('get_default_basename must be overridden')
def get_urls(self): def get_urls(self):
""" """
@ -151,14 +169,14 @@ class SimpleRouter(BaseRouter):
self.trailing_slash = '/' if trailing_slash else '' self.trailing_slash = '/' if trailing_slash else ''
super(SimpleRouter, self).__init__() super(SimpleRouter, self).__init__()
def get_default_base_name(self, viewset): def get_default_basename(self, viewset):
""" """
If `base_name` is not specified, attempt to automatically determine If `basename` is not specified, attempt to automatically determine
it from the viewset. it from the viewset.
""" """
queryset = getattr(viewset, 'queryset', None) queryset = getattr(viewset, 'queryset', None)
assert queryset is not None, '`base_name` argument not specified, and could ' \ assert queryset is not None, '`basename` argument not specified, and could ' \
'not automatically determine the name from the viewset, as ' \ 'not automatically determine the name from the viewset, as ' \
'it does not have a `.queryset` attribute.' 'it does not have a `.queryset` attribute.'

View File

@ -1,5 +1,6 @@
from __future__ import unicode_literals from __future__ import unicode_literals
import warnings
from collections import namedtuple from collections import namedtuple
import pytest import pytest
@ -86,13 +87,13 @@ kwarged_notes_router = SimpleRouter()
kwarged_notes_router.register(r'notes', KWargedNoteViewSet) kwarged_notes_router.register(r'notes', KWargedNoteViewSet)
namespaced_router = DefaultRouter() namespaced_router = DefaultRouter()
namespaced_router.register(r'example', MockViewSet, base_name='example') namespaced_router.register(r'example', MockViewSet, basename='example')
empty_prefix_router = SimpleRouter() empty_prefix_router = SimpleRouter()
empty_prefix_router.register(r'', EmptyPrefixViewSet, base_name='empty_prefix') empty_prefix_router.register(r'', EmptyPrefixViewSet, basename='empty_prefix')
regex_url_path_router = SimpleRouter() regex_url_path_router = SimpleRouter()
regex_url_path_router.register(r'', RegexUrlPathViewSet, base_name='regex') regex_url_path_router.register(r'', RegexUrlPathViewSet, basename='regex')
class BasicViewSet(viewsets.ViewSet): class BasicViewSet(viewsets.ViewSet):
@ -319,7 +320,7 @@ class TestActionKeywordArgs(TestCase):
}) })
self.router = SimpleRouter() self.router = SimpleRouter()
self.router.register(r'test', TestViewSet, base_name='test') self.router.register(r'test', TestViewSet, basename='test')
self.view = self.router.urls[-1].callback self.view = self.router.urls[-1].callback
def test_action_kwargs(self): def test_action_kwargs(self):
@ -344,7 +345,7 @@ class TestActionAppliedToExistingRoute(TestCase):
}) })
self.router = SimpleRouter() self.router = SimpleRouter()
self.router.register(r'test', TestViewSet, base_name='test') self.router.register(r'test', TestViewSet, basename='test')
with pytest.raises(ImproperlyConfigured): with pytest.raises(ImproperlyConfigured):
self.router.urls self.router.urls
@ -481,3 +482,71 @@ class TestViewInitkwargs(URLPatternsTestCase, TestCase):
initkwargs = match.func.initkwargs initkwargs = match.func.initkwargs
assert initkwargs['basename'] == 'routertestmodel' assert initkwargs['basename'] == 'routertestmodel'
class TestBaseNameRename(TestCase):
def test_base_name_and_basename_assertion(self):
router = SimpleRouter()
msg = "Do not provide both the `basename` and `base_name` arguments."
with warnings.catch_warnings(record=True) as w, \
self.assertRaisesMessage(AssertionError, msg):
warnings.simplefilter('always')
router.register('mock', MockViewSet, 'mock', base_name='mock')
msg = "The `base_name` argument has been deprecated in favor of `basename`."
assert len(w) == 1
assert str(w[0].message) == msg
def test_base_name_argument_deprecation(self):
router = SimpleRouter()
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
router.register('mock', MockViewSet, base_name='mock')
msg = "The `base_name` argument has been deprecated in favor of `basename`."
assert len(w) == 1
assert str(w[0].message) == msg
assert router.registry == [
('mock', MockViewSet, 'mock'),
]
def test_basename_argument_no_warnings(self):
router = SimpleRouter()
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
router.register('mock', MockViewSet, basename='mock')
assert len(w) == 0
assert router.registry == [
('mock', MockViewSet, 'mock'),
]
def test_get_default_base_name_deprecation(self):
msg = "`CustomRouter.get_default_base_name` method should be renamed `get_default_basename`."
# Class definition should raise a warning
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
class CustomRouter(SimpleRouter):
def get_default_base_name(self, viewset):
return 'foo'
assert len(w) == 1
assert str(w[0].message) == msg
# Deprecated method implementation should still be called
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
router = CustomRouter()
router.register('mock', MockViewSet)
assert len(w) == 0
assert router.registry == [
('mock', MockViewSet, 'foo'),
]

View File

@ -122,7 +122,7 @@ else:
pass pass
router = DefaultRouter() router = DefaultRouter()
router.register('example', ExampleViewSet, base_name='example') router.register('example', ExampleViewSet, basename='example')
urlpatterns = [ urlpatterns = [
url(r'^$', schema_view), url(r'^$', schema_view),
url(r'^', include(router.urls)) url(r'^', include(router.urls))
@ -509,7 +509,7 @@ class TestSchemaGeneratorNotAtRoot(TestCase):
class TestSchemaGeneratorWithMethodLimitedViewSets(TestCase): class TestSchemaGeneratorWithMethodLimitedViewSets(TestCase):
def setUp(self): def setUp(self):
router = DefaultRouter() router = DefaultRouter()
router.register('example1', MethodLimitedViewSet, base_name='example1') router.register('example1', MethodLimitedViewSet, basename='example1')
self.patterns = [ self.patterns = [
url(r'^', include(router.urls)) url(r'^', include(router.urls))
] ]
@ -566,8 +566,8 @@ class TestSchemaGeneratorWithMethodLimitedViewSets(TestCase):
class TestSchemaGeneratorWithRestrictedViewSets(TestCase): class TestSchemaGeneratorWithRestrictedViewSets(TestCase):
def setUp(self): def setUp(self):
router = DefaultRouter() router = DefaultRouter()
router.register('example1', Http404ExampleViewSet, base_name='example1') router.register('example1', Http404ExampleViewSet, basename='example1')
router.register('example2', PermissionDeniedExampleViewSet, base_name='example2') router.register('example2', PermissionDeniedExampleViewSet, basename='example2')
self.patterns = [ self.patterns = [
url('^example/?$', ExampleListView.as_view()), url('^example/?$', ExampleListView.as_view()),
url(r'^', include(router.urls)) url(r'^', include(router.urls))
@ -1042,7 +1042,7 @@ class NamingCollisionViewSet(GenericViewSet):
naming_collisions_router = SimpleRouter() naming_collisions_router = SimpleRouter()
naming_collisions_router.register(r'collision', NamingCollisionViewSet, base_name="collision") naming_collisions_router.register(r'collision', NamingCollisionViewSet, basename="collision")
@pytest.mark.skipif(not coreapi, reason='coreapi is not installed') @pytest.mark.skipif(not coreapi, reason='coreapi is not installed')

View File

@ -65,7 +65,7 @@ class ActionViewSet(GenericViewSet):
router = SimpleRouter() router = SimpleRouter()
router.register(r'actions', ActionViewSet) router.register(r'actions', ActionViewSet)
router.register(r'actions-alt', ActionViewSet, base_name='actions-alt') router.register(r'actions-alt', ActionViewSet, basename='actions-alt')
urlpatterns = [ urlpatterns = [
@ -177,7 +177,7 @@ class GetExtraActionUrlMapTests(TestCase):
class ReverseActionTests(TestCase): class ReverseActionTests(TestCase):
def test_default_basename(self): def test_default_basename(self):
view = ActionViewSet() view = ActionViewSet()
view.basename = router.get_default_base_name(ActionViewSet) view.basename = router.get_default_basename(ActionViewSet)
view.request = None view.request = None
assert view.reverse_action('list') == '/api/actions/' assert view.reverse_action('list') == '/api/actions/'
@ -203,7 +203,7 @@ class ReverseActionTests(TestCase):
def test_request_passing(self): def test_request_passing(self):
view = ActionViewSet() view = ActionViewSet()
view.basename = router.get_default_base_name(ActionViewSet) view.basename = router.get_default_basename(ActionViewSet)
view.request = factory.get('/') view.request = factory.get('/')
# Passing the view's request object should result in an absolute URL. # Passing the view's request object should result in an absolute URL.