From a4267e78e09de4706ad79bbdc700efeef139988e Mon Sep 17 00:00:00 2001 From: Dimitri Justeau Date: Mon, 22 Aug 2016 19:16:30 +1100 Subject: [PATCH 1/6] Enhance api_endpoint model to track the tree structure of an api, and add router settings for a better integration of viewsets. --- rest_framework_docs/api_docs.py | 128 +++++++++++++++--- rest_framework_docs/api_endpoint.py | 75 ++++++++-- rest_framework_docs/settings.py | 5 +- .../templates/rest_framework_docs/home.html | 2 +- 4 files changed, 178 insertions(+), 32 deletions(-) diff --git a/rest_framework_docs/api_docs.py b/rest_framework_docs/api_docs.py index d22dd4c..bd1f667 100644 --- a/rest_framework_docs/api_docs.py +++ b/rest_framework_docs/api_docs.py @@ -1,9 +1,15 @@ from importlib import import_module +from types import ModuleType from django.conf import settings from django.core.urlresolvers import RegexURLResolver, RegexURLPattern from django.utils.module_loading import import_string from rest_framework.views import APIView -from rest_framework_docs.api_endpoint import ApiEndpoint +from rest_framework.routers import BaseRouter +from rest_framework_docs.api_endpoint import ApiNode, ApiEndpoint +from rest_framework_docs.settings import DRFSettings + + +drf_settings = DRFSettings().settings class ApiDocumentation(object): @@ -11,6 +17,7 @@ class ApiDocumentation(object): def __init__(self, drf_router=None): self.endpoints = [] self.drf_router = drf_router + try: root_urlconf = import_string(settings.ROOT_URLCONF) except ImportError: @@ -21,26 +28,113 @@ def __init__(self, drf_router=None): else: self.get_all_view_names(root_urlconf.urlpatterns) - def get_all_view_names(self, urlpatterns, parent_pattern=None): + def get_all_view_names(self, urlpatterns, parent_api_node=None): for pattern in urlpatterns: if isinstance(pattern, RegexURLResolver): - parent_pattern = None if pattern._regex == "^" else pattern - self.get_all_view_names(urlpatterns=pattern.url_patterns, parent_pattern=parent_pattern) - elif isinstance(pattern, RegexURLPattern) and self._is_drf_view(pattern) and not self._is_format_endpoint(pattern): - api_endpoint = ApiEndpoint(pattern, parent_pattern, self.drf_router) + # Try to get router from settings, if no router is found, + # Use the instance drf_router property. + router = get_router(pattern) + if router is None: + parent_router = None + if parent_api_node is not None: + parent_router = parent_api_node.drf_router + if parent_router is not None: + router = parent_router + else: + router = self.drf_router + if pattern._regex == "^": + parent = parent_api_node + else: + parent = ApiNode( + pattern, + parent_node=parent_api_node, + drf_router=router + ) + self.get_all_view_names(urlpatterns=pattern.url_patterns, parent_api_node=parent) + elif isinstance(pattern, RegexURLPattern) and _is_drf_view(pattern) and not _is_format_endpoint(pattern): + router = parent_api_node.drf_router + router = self.drf_router if router is None else router + api_endpoint = ApiEndpoint(pattern, parent_api_node, router) self.endpoints.append(api_endpoint) - def _is_drf_view(self, pattern): - """ - Should check whether a pattern inherits from DRF's APIView - """ - return hasattr(pattern.callback, 'cls') and issubclass(pattern.callback.cls, APIView) + def get_endpoints(self): + return self.endpoints - def _is_format_endpoint(self, pattern): - """ - Exclude endpoints with a "format" parameter + +def _is_drf_view(pattern): + """ + Should check whether a pattern inherits from DRF's APIView + """ + return hasattr(pattern.callback, 'cls') and issubclass(pattern.callback.cls, + APIView) + + +def _is_format_endpoint(pattern): + """ + Exclude endpoints with a "format" parameter + """ + return '?P' in pattern._regex + + +def get_router(pattern): + urlconf = pattern.urlconf_name + router = None + if isinstance(urlconf, ModuleType): + # First: try MODULE_ROUTERS setting - Don't ignore errors + router = get_module_router(urlconf) + if router is not None: + return router + # Second: try DEFAULT_MODULE_ROUTER setting - Ignore errors + try: + router = get_default_module_router(urlconf) + if router is not None: + return router + except: + pass + # Third: try DEFAULT_ROUTER setting - Don't ignore errors + router = get_default_router() + if router is not None: + return router + return router + + +def get_module_router(module): + routers = drf_settings['MODULE_ROUTERS'] + if routers is None: + return None + if module.__name__ in routers: + router_name = routers[module.__name__] + router = getattr(module, router_name) + assert isinstance(router, BaseRouter), \ + """ + drfdocs 'ROUTERS' setting does not correspond to + a router instance for module {}. + """.format(module.__name__) + return router + return None + + +def get_default_module_router(module): + default_module_router = drf_settings['DEFAULT_MODULE_ROUTER'] + if default_module_router is None: + return None + router = getattr(module, default_module_router) + assert isinstance(router, BaseRouter), \ """ - return '?P' in pattern._regex + drfdocs 'DEFAULT_MODULE_ROUTER' setting does not correspond to + a router instance for module {}. + """.format(module.__name__) + return router - def get_endpoints(self): - return self.endpoints + +def get_default_router(): + default_router_path = drf_settings['DEFAULT_ROUTER'] + if default_router_path is None: + return None + router = import_string(default_router_path) + assert isinstance(router, BaseRouter), \ + """ + drfdocs 'DEFAULT_ROUTER_NAME' setting does not correspond to + a router instance {}. + """.format(router.__name__) + return router diff --git a/rest_framework_docs/api_endpoint.py b/rest_framework_docs/api_endpoint.py index 89a33f8..7fb116e 100644 --- a/rest_framework_docs/api_endpoint.py +++ b/rest_framework_docs/api_endpoint.py @@ -5,18 +5,73 @@ from rest_framework.serializers import BaseSerializer -class ApiEndpoint(object): +class ApiNode(object): - def __init__(self, pattern, parent_pattern=None, drf_router=None): - self.drf_router = drf_router + def __init__(self, pattern, parent_node=None, drf_router=None): self.pattern = pattern + self.parent_node = parent_node + self.drf_router = drf_router + + @property + def parent_pattern(self): + if self.parent_node is None: + return None + return self.parent_node.pattern + + @property + def name_parent(self): + if self.parent_pattern is None: + return None + return simplify_regex(self.parent_pattern.regex.pattern).strip('/') + + @property + def name_parent_full(self): + name_parent_full = self.name_parent + if name_parent_full is None: + return None + parent_node = self.parent_node + parent_name = parent_node.name_parent + while parent_name is not None: + name_parent_full = "{}/{}".format(parent_name, name_parent_full) + parent_node = parent_node.parent_node + parent_name = parent_node.name_parent + return name_parent_full + + @property + def path(self): + pattern = self.__get_path__(self.parent_pattern) + if pattern is None: + return None + parent = self.parent_node + name_parent = None if parent is None else parent.name_parent + while name_parent is not None: + pattern = "{}/{}".format(name_parent, pattern) + parent = parent.parent_node + name_parent = parent.name_parent + if pattern[0] != "/": + pattern = "/{}".format(pattern) + return pattern + + def __get_path__(self, parent_pattern): + if parent_pattern: + return "{0}{1}".format( + self.name_parent, + simplify_regex(self.pattern.regex.pattern) + ) + return simplify_regex(self.pattern.regex.pattern) + + +class ApiEndpoint(ApiNode): + + def __init__(self, pattern, parent_node=None, drf_router=None): + super(ApiEndpoint, self).__init__( + pattern, + parent_node=parent_node, + drf_router=drf_router + ) self.callback = pattern.callback - # self.name = pattern.name self.docstring = self.__get_docstring__() - self.name_parent = simplify_regex(parent_pattern.regex.pattern).strip('/') if parent_pattern else None - self.path = self.__get_path__(parent_pattern) self.allowed_methods = self.__get_allowed_methods__() - # self.view_name = pattern.callback.__name__ self.errors = None self.serializer_class = self.__get_serializer_class__() if self.serializer_class: @@ -26,13 +81,7 @@ def __init__(self, pattern, parent_pattern=None, drf_router=None): self.permissions = self.__get_permissions_class__() - def __get_path__(self, parent_pattern): - if parent_pattern: - return "/{0}{1}".format(self.name_parent, simplify_regex(self.pattern.regex.pattern)) - return simplify_regex(self.pattern.regex.pattern) - def __get_allowed_methods__(self): - viewset_methods = [] if self.drf_router: for prefix, viewset, basename in self.drf_router.registry: diff --git a/rest_framework_docs/settings.py b/rest_framework_docs/settings.py index 2853a7b..e27070a 100644 --- a/rest_framework_docs/settings.py +++ b/rest_framework_docs/settings.py @@ -5,7 +5,10 @@ class DRFSettings(object): def __init__(self): self.drf_settings = { - "HIDE_DOCS": self.get_setting("HIDE_DOCS") or False + "HIDE_DOCS": self.get_setting("HIDE_DOCS") or False, + "MODULE_ROUTERS": self.get_setting("MODULE_ROUTERS"), + "DEFAULT_MODULE_ROUTER": self.get_setting("DEFAULT_MODULE_ROUTER"), + "DEFAULT_ROUTER": self.get_setting("DEFAULT_ROUTER") } def get_setting(self, name): diff --git a/rest_framework_docs/templates/rest_framework_docs/home.html b/rest_framework_docs/templates/rest_framework_docs/home.html index 235a6ee..0ba24d5 100644 --- a/rest_framework_docs/templates/rest_framework_docs/home.html +++ b/rest_framework_docs/templates/rest_framework_docs/home.html @@ -15,7 +15,7 @@ {% block content %} - {% regroup endpoints by name_parent as endpoints_grouped %} + {% regroup endpoints by name_parent_full as endpoints_grouped %} {% if endpoints_grouped %} {% for group in endpoints_grouped %} From 4470670383e01981fce344183e88a0358f46bfee Mon Sep 17 00:00:00 2001 From: Dimitri Justeau Date: Wed, 24 Aug 2016 12:34:29 +1100 Subject: [PATCH 2/6] Add viewsets in demo project --- demo/project/accounts/urls.py | 12 +++++++++++- demo/project/accounts/views.py | 6 ++++++ demo/project/organisations/urls.py | 13 +++++++++++-- demo/project/organisations/views.py | 9 ++++++++- demo/project/settings.py | 8 ++++++++ demo/project/urls.py | 4 ++++ 6 files changed, 48 insertions(+), 4 deletions(-) diff --git a/demo/project/accounts/urls.py b/demo/project/accounts/urls.py index 1486675..9fb4c43 100644 --- a/demo/project/accounts/urls.py +++ b/demo/project/accounts/urls.py @@ -1,15 +1,25 @@ -from django.conf.urls import url +from django.conf.urls import url, include +from rest_framework import routers from project.accounts import views +router = routers.DefaultRouter() +router.register( + r'user_viewset', + views.UserModelViewSet, +) + + urlpatterns = [ url(r'^test/$', views.TestView.as_view(), name="test-view"), url(r'^login/$', views.LoginView.as_view(), name="login"), url(r'^register/$', views.UserRegistrationView.as_view(), name="register"), + url(r'^register/$', views.UserRegistrationView.as_view(), name="register"), url(r'^reset-password/$', view=views.PasswordResetView.as_view(), name="reset-password"), url(r'^reset-password/confirm/$', views.PasswordResetConfirmView.as_view(), name="reset-password-confirm"), url(r'^user/profile/$', views.UserProfileView.as_view(), name="profile"), + url(r'^viewset_test/', include(router.urls), name="user_viewset"), ] diff --git a/demo/project/accounts/views.py b/demo/project/accounts/views.py index e1bd9c0..6c1bb48 100644 --- a/demo/project/accounts/views.py +++ b/demo/project/accounts/views.py @@ -6,6 +6,7 @@ from rest_framework.permissions import AllowAny from rest_framework.response import Response from rest_framework.views import APIView +from rest_framework.viewsets import ModelViewSet from project.accounts.models import User from project.accounts.serializers import ( UserRegistrationSerializer, UserProfileSerializer, ResetPasswordSerializer @@ -81,3 +82,8 @@ def post(self, request, *args, **kwargs): if not serializer.is_valid(): return Response({'errors': serializer.errors}, status=status.HTTP_400_BAD_REQUEST) return Response({"msg": "Password updated successfully."}, status=status.HTTP_200_OK) + + +class UserModelViewSet(ModelViewSet): + queryset = User.objects.all() + serializer_class = UserProfileSerializer diff --git a/demo/project/organisations/urls.py b/demo/project/organisations/urls.py index 4d0311e..0f57c91 100644 --- a/demo/project/organisations/urls.py +++ b/demo/project/organisations/urls.py @@ -1,12 +1,21 @@ -from django.conf.urls import url +from django.conf.urls import url, include +from rest_framework import routers from project.organisations import views +router = routers.DefaultRouter() +router.register( + r'organisation_viewset', + views.OrganisationViewSet, +) + + urlpatterns = [ url(r'^create/$', view=views.CreateOrganisationView.as_view(), name="create"), url(r'^(?P[\w-]+)/$', view=views.RetrieveOrganisationView.as_view(), name="organisation"), url(r'^(?P[\w-]+)/members/$', view=views.OrganisationMembersView.as_view(), name="members"), - url(r'^(?P[\w-]+)/leave/$', view=views.LeaveOrganisationView.as_view(), name="leave") + url(r'^(?P[\w-]+)/leave/$', view=views.LeaveOrganisationView.as_view(), name="leave"), + url(r'^', include(router.urls), name="organisation_viewset"), ] diff --git a/demo/project/organisations/views.py b/demo/project/organisations/views.py index 1e2d5fb..c40f630 100644 --- a/demo/project/organisations/views.py +++ b/demo/project/organisations/views.py @@ -1,8 +1,10 @@ from rest_framework import generics, status from rest_framework.response import Response +from rest_framework.viewsets import ModelViewSet from project.organisations.models import Organisation, Membership from project.organisations.serializers import ( - CreateOrganisationSerializer, OrganisationMembersSerializer, RetrieveOrganisationSerializer + CreateOrganisationSerializer, OrganisationMembersSerializer, + RetrieveOrganisationSerializer, OrganisationDetailSerializer ) @@ -34,3 +36,8 @@ def delete(self, request, *args, **kwargs): instance = self.get_object() self.perform_destroy(instance) return Response(status=status.HTTP_204_NO_CONTENT) + + +class OrganisationViewSet(ModelViewSet): + queryset = Organisation.objects.all() + serializer_class = OrganisationDetailSerializer diff --git a/demo/project/settings.py b/demo/project/settings.py index 5e06207..0b34c30 100644 --- a/demo/project/settings.py +++ b/demo/project/settings.py @@ -101,6 +101,14 @@ ) } +REST_FRAMEWORK_DOCS = { + 'HIDE_DOCS': False, + 'MODULE_ROUTERS': { + 'project.accounts.urls': 'router', + }, + 'DEFAULT_MODULE_ROUTER': 'router', +} + # Internationalization # https://docs.djangoproject.com/en/1.8/topics/i18n/ diff --git a/demo/project/urls.py b/demo/project/urls.py index d8e049f..f764dcf 100644 --- a/demo/project/urls.py +++ b/demo/project/urls.py @@ -16,11 +16,15 @@ from django.conf.urls import include, url from django.contrib import admin + urlpatterns = [ url(r'^admin/', include(admin.site.urls)), url(r'^docs/', include('rest_framework_docs.urls')), # API url(r'^accounts/', view=include('project.accounts.urls', namespace='accounts')), + + url(r'^organisations/', view=include('project.organisations.urls', namespace='organisations')), + ] From f33aa98050bbe94b075ce20f26c518ac552c57c5 Mon Sep 17 00:00:00 2001 From: Dimitri Justeau Date: Wed, 24 Aug 2016 14:59:08 +1100 Subject: [PATCH 3/6] Fix case when RegexURLPattern has no parent --- rest_framework_docs/api_docs.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rest_framework_docs/api_docs.py b/rest_framework_docs/api_docs.py index bd1f667..99fad59 100644 --- a/rest_framework_docs/api_docs.py +++ b/rest_framework_docs/api_docs.py @@ -52,8 +52,10 @@ def get_all_view_names(self, urlpatterns, parent_api_node=None): ) self.get_all_view_names(urlpatterns=pattern.url_patterns, parent_api_node=parent) elif isinstance(pattern, RegexURLPattern) and _is_drf_view(pattern) and not _is_format_endpoint(pattern): - router = parent_api_node.drf_router - router = self.drf_router if router is None else router + router = self.drf_router + if parent_api_node is not None: + if parent_api_node.drf_router is not None: + router = parent_api_node.drf_router api_endpoint = ApiEndpoint(pattern, parent_api_node, router) self.endpoints.append(api_endpoint) From ab5e821a682bf47db125d7ec0b851ae6b883873f Mon Sep 17 00:00:00 2001 From: Dimitri Justeau Date: Wed, 24 Aug 2016 15:36:54 +1100 Subject: [PATCH 4/6] Document new router settings --- docs/settings.md | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/docs/settings.md b/docs/settings.md index abd2106..b0c5643 100755 --- a/docs/settings.md +++ b/docs/settings.md @@ -6,7 +6,12 @@ source_filename: settings To set DRF docs' settings just include the dictionary below in Django's `settings.py` file. REST_FRAMEWORK_DOCS = { - 'HIDE_DOCS': True + 'HIDE_DOCS': True, + 'MODULE_ROUTERS': { + 'project.accounts.urls': 'accounts_router', + }, + 'DEFAULT_MODULE_ROUTER': 'router', + 'DEFAULT_ROUTER': 'project.urls.default_router', } @@ -21,9 +26,29 @@ You can use hidden to prevent your docs from showing up in different environment Then set the value of the environment variable `HIDE_DRFDOCS` for each environment (ie. Use `.env` files) +##### MODULE_ROUTERS +Use this setting to manually bind url modules to a router instance. The router must be defined in the module, or imported in the module. +For instance, if the router of an app called 'gifts' is 'gifts_router', and the router of another app called 'scuba_diving' is 'scuba_diving_router', the MODULE_ROUTERS setting should look like: + + 'MODULE_ROUTERS': { + 'gifts.urls': 'gift_router', + 'scuba_diving.urls': 'scuba_diving_router' + } + +If there is no entry for a given module, if this setting is not set, or if it is set to None, the value of the DEFAULT_MODULE_ROUTER setting is used. + +##### DEFAULT_MODULE_ROUTER +When set, the value of this setting will be used to find a router for a urls module. If there is no router having the DEFAULT_MODULE_ROUTER name, the setting is ignored and the value of DEFAULT_ROUTER is used. + +##### DEFAULT_ROUTER +When defined, this setting must describe a python dotted path leading to the router that should be used when MODULE_ROUTERS and DEFAULT_MODULE_ROUTER are not set. +This parameter is useful when there is only one router for your whole API. + ### List of Settings -| Setting | Type | Options | Default | -|---------|---------|-----------------|---------| -|HIDE_DOCS| Boolean | `True`, `False` | `False` | -| | | | | +| Setting | Type | Options | Default | +|---------------------|-----------------------------------------------------------|-----------------|---------| +|HIDE_DOCS | Boolean | `True`, `False` | `False` | +|MODULE_ROUTERS | dict of python dotted paths -> router instance name | | `None` | +|DEFAULT_MODULE_ROUTER| str representing a default router instance name | | `None` | +|DEFAULT_ROUTER | str representing a python dotted path to a router instance| | `None` | From 77dc48978764240a5e3af51e6ae23239ad0d8555 Mon Sep 17 00:00:00 2001 From: Dimitri Justeau Date: Wed, 24 Aug 2016 16:23:21 +1100 Subject: [PATCH 5/6] Update README.md --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 6c82768..50b0382 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,9 @@ You can find detailed information about the package's settings at [the docs](htt REST_FRAMEWORK_DOCS = { 'HIDE_DOCS': True # Default: False + 'MODULE_ROUTERS': {}, + 'DEFAULT_MODULE_ROUTER': 'router', + 'DEFAULT_ROUTER': None, } From 7a561755c2b1b4ab2757c46be601e720ab1d7f2c Mon Sep 17 00:00:00 2001 From: Dimitri Justeau Date: Thu, 25 Aug 2016 16:50:54 +1100 Subject: [PATCH 6/6] Fix empty doctstring for viewsets when docstring is only defined at class-level --- rest_framework_docs/api_endpoint.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rest_framework_docs/api_endpoint.py b/rest_framework_docs/api_endpoint.py index 7fb116e..0e74d25 100644 --- a/rest_framework_docs/api_endpoint.py +++ b/rest_framework_docs/api_endpoint.py @@ -110,8 +110,9 @@ def __get_allowed_methods__(self): ) viewset_methods = list(viewset_methods) if len(set(funcs)) == 1: - self.docstring = inspect.getdoc(getattr(self.callback.cls, funcs[0])) - + func_docstring = inspect.getdoc(getattr(self.callback.cls, funcs[0])) + if func_docstring is not None: + self.docstring = func_docstring view_methods = [force_str(m).upper() for m in self.callback.cls.http_method_names if hasattr(self.callback.cls, m)] return viewset_methods + view_methods