From b45ed61b72bf55e0049473bea1ed9cd1f51b2e55 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Wed, 16 May 2018 12:48:46 -0400 Subject: [PATCH] Rename base_name => basename for consistency --- docs/api-guide/routers.md | 16 ++++----- docs/api-guide/viewsets.md | 4 +-- rest_framework/routers.py | 40 ++++++++++++++++------ tests/test_routers.py | 69 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 21 deletions(-) diff --git a/docs/api-guide/routers.md b/docs/api-guide/routers.md index 2609185ee..4aa0aeefc 100644 --- a/docs/api-guide/routers.md +++ b/docs/api-guide/routers.md @@ -28,7 +28,7 @@ There are two mandatory arguments to the `register()` method: 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: @@ -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. -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.register(r'users', UserViewSet) @@ -65,7 +65,7 @@ For example, you can append `router.urls` to a list of existing views… urlpatterns += router.urls -Alternatively you can use Django's `include` function, like so… +Alternatively you can use Django's `include` function, like so... urlpatterns = [ 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. -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 diff --git a/docs/api-guide/viewsets.md b/docs/api-guide/viewsets.md index 503459a96..d4c053127 100644 --- a/docs/api-guide/viewsets.md +++ b/docs/api-guide/viewsets.md @@ -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 router = DefaultRouter() - router.register(r'users', UserViewSet, base_name='user') + router.register(r'users', UserViewSet, basename='user') 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: @@ -251,7 +251,7 @@ Note that you can use any of the standard attributes or method overrides provide def get_queryset(self): 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. diff --git a/rest_framework/routers.py b/rest_framework/routers.py index 281bbde8a..f1034a65c 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -22,6 +22,8 @@ from collections import OrderedDict, namedtuple from django.conf.urls import url from django.core.exceptions import ImproperlyConfigured from django.urls import NoReverseMatch +from django.utils import six +from django.utils.deprecation import RenameMethodsBase from rest_framework import views from rest_framework.response import Response @@ -73,21 +75,37 @@ def flatten(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): self.registry = [] - def register(self, prefix, viewset, base_name=None): - if base_name is None: - base_name = self.get_default_base_name(viewset) - self.registry.append((prefix, viewset, base_name)) + def register(self, prefix, viewset, basename=None, base_name=None): + if base_name is not None: + msg = "The `base_name` argument has been deprecated in favor of `basename`." + 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. """ - raise NotImplementedError('get_default_base_name must be overridden') + raise NotImplementedError('get_default_basename must be overridden') def get_urls(self): """ @@ -151,14 +169,14 @@ class SimpleRouter(BaseRouter): self.trailing_slash = '/' if trailing_slash else '' 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. """ 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 ' \ 'it does not have a `.queryset` attribute.' diff --git a/tests/test_routers.py b/tests/test_routers.py index 36255f48f..dc2625659 100644 --- a/tests/test_routers.py +++ b/tests/test_routers.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import warnings from collections import namedtuple import pytest @@ -466,3 +467,71 @@ class TestViewInitkwargs(URLPatternsTestCase, TestCase): initkwargs = match.func.initkwargs 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'), + ]