From 509bbb8486ca706e7a34db0b02a8b4a1a272d4b4 Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Wed, 26 Sep 2018 15:37:13 +0200 Subject: [PATCH] Small cleanups for django best practices. Changes include: * Custom admin app config instead of monkey patching attributes. * DRF Routers & Viewsets instead of manual view configuration. * nested viewsets from drf-extensions (for now) to scope files to playbooks. * Removed media serving since runserver does this alreay in dev; deployment will need another option anyways. Change-Id: I409bd3f7faa5f133fcb204613baebf2512d34c1a --- ara/api/urls.py | 36 ++++++---------- ara/api/views.py | 93 ++++++++---------------------------------- ara/server/admin.py | 6 +++ ara/server/apps.py | 5 +++ ara/server/settings.py | 6 +-- ara/server/urls.py | 13 ++---- requirements.txt | 1 + 7 files changed, 48 insertions(+), 112 deletions(-) create mode 100644 ara/server/admin.py create mode 100644 ara/server/apps.py diff --git a/ara/api/urls.py b/ara/api/urls.py index b51aa1ba..af647d48 100644 --- a/ara/api/urls.py +++ b/ara/api/urls.py @@ -15,29 +15,19 @@ # You should have received a copy of the GNU General Public License # along with ARA. If not, see . -from django.conf.urls import url -from rest_framework.urlpatterns import format_suffix_patterns +from rest_framework_extensions.routers import ExtendedDefaultRouter from ara.api import views -urlpatterns = [ - url(r'^/$', views.api_root), - url(r'^/labels$', views.LabelList.as_view(), name='label-list'), - url(r'^/labels/(?P[0-9]+)$', views.LabelDetail.as_view(), name='label-detail'), - url(r'^/playbooks$', views.PlaybookList.as_view(), name='playbook-list'), - url(r'^/playbooks/(?P[0-9]+)$', views.PlaybookDetail.as_view(), name='playbook-detail'), - url(r'^/playbooks/(?P[0-9]+)/files$', views.PlaybookFilesDetail.as_view(), name='playbook-file-detail'), - url(r'^/plays$', views.PlayList.as_view(), name='play-list'), - url(r'^/plays/(?P[0-9]+)$', views.PlayDetail.as_view(), name='play-detail'), - url(r'^/tasks$', views.TaskList.as_view(), name='task-list'), - url(r'^/tasks/(?P[0-9]+)$', views.TaskDetail.as_view(), name='task-detail'), - url(r'^/hosts$', views.HostList.as_view(), name='host-list'), - url(r'^/hosts/(?P[0-9]+)$', views.HostDetail.as_view(), name='host-detail'), - url(r'^/results$', views.ResultList.as_view(), name='result-list'), - url(r'^/results/(?P[0-9]+)$', views.ResultDetail.as_view(), name='result-detail'), - url(r'^/files$', views.FileList.as_view(), name='file-list'), - url(r'^/files/(?P[0-9]+)$', views.FileDetail.as_view(), name='file-detail'), - url(r'^/stats$', views.StatsList.as_view(), name='stats-list'), - url(r'^/stats/(?P[0-9]+)$', views.StatsDetail.as_view(), name='stats-detail'), -] +router = ExtendedDefaultRouter(trailing_slash=False) +router.register('labels', views.LabelViewSet, base_name='label') +router.register('plays', views.PlayViewSet, base_name='play') +router.register('tasks', views.TaskViewSet, base_name='task') +router.register('hosts', views.HostViewSet, base_name='host') +router.register('results', views.ResultViewSet, base_name='result') +router.register('files', views.FileViewSet, base_name='file') +router.register('stats', views.StatsViewSet, base_name='stats') -urlpatterns = format_suffix_patterns(urlpatterns) +playbook_routes = router.register('playbooks', views.PlaybookViewSet, base_name='playbook') +playbook_routes.register('files', views.PlaybookFilesDetail, base_name='file', parents_query_lookups=['playbooks']) + +urlpatterns = router.urls diff --git a/ara/api/views.py b/ara/api/views.py index b3928bb0..b32d3d27 100644 --- a/ara/api/views.py +++ b/ara/api/views.py @@ -14,118 +14,57 @@ # # You should have received a copy of the GNU General Public License # along with ARA. If not, see . -from rest_framework.decorators import api_view -from rest_framework.response import Response -from rest_framework.reverse import reverse +from rest_framework import viewsets +from rest_framework_extensions.mixins import NestedViewSetMixin from ara.api import models, serializers -from rest_framework import generics, status - -@api_view(['GET']) -def api_root(request, format=None): - return Response({ - 'labels': reverse('label-list', request=request, format=format), - 'playbooks': reverse('playbook-list', request=request, format=format), - 'plays': reverse('play-list', request=request, format=format), - 'tasks': reverse('task-list', request=request, format=format), - 'files': reverse('file-list', request=request, format=format), - 'hosts': reverse('host-list', request=request, format=format), - 'results': reverse('result-list', request=request, format=format), - 'stats': reverse('stats-list', request=request, format=format) - }) - - -class LabelList(generics.ListCreateAPIView): +class LabelViewSet(viewsets.ModelViewSet): queryset = models.Label.objects.all() serializer_class = serializers.LabelSerializer -class LabelDetail(generics.RetrieveUpdateDestroyAPIView): - queryset = models.Label.objects.all() - serializer_class = serializers.LabelSerializer - - -class PlaybookList(generics.ListCreateAPIView): +class PlaybookViewSet(viewsets.ModelViewSet): queryset = models.Playbook.objects.all() serializer_class = serializers.PlaybookSerializer -class PlaybookDetail(generics.RetrieveUpdateDestroyAPIView): - queryset = models.Playbook.objects.all() - serializer_class = serializers.PlaybookSerializer - - -class PlaybookFilesDetail(generics.CreateAPIView): - queryset = models.Playbook.objects.all() +class PlaybookFilesDetail(NestedViewSetMixin, viewsets.ModelViewSet): + queryset = models.File.objects.all() serializer_class = serializers.FileSerializer - def post(self, request, *args, **kwargs): - playbook = self.get_object() - serializer = self.get_serializer(data=request.data) - serializer.is_valid(raise_exception=True) - self.perform_create(serializer) - headers = self.get_success_headers(serializer.data) - playbook.files.add(serializer.data['id']) - return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) + def perform_create(self, serializer): + playbook = models.Playbook.objects.get(pk=self.get_parents_query_dict()['playbooks']) + instance = serializer.save() + playbook.files.add(instance) -class PlayList(generics.ListCreateAPIView): +class PlayViewSet(viewsets.ModelViewSet): queryset = models.Play.objects.all() serializer_class = serializers.PlaySerializer -class PlayDetail(generics.RetrieveUpdateDestroyAPIView): - queryset = models.Play.objects.all() - serializer_class = serializers.PlaySerializer - - -class TaskList(generics.ListCreateAPIView): +class TaskViewSet(viewsets.ModelViewSet): queryset = models.Task.objects.all() serializer_class = serializers.TaskSerializer -class TaskDetail(generics.RetrieveUpdateDestroyAPIView): - queryset = models.Task.objects.all() - serializer_class = serializers.TaskSerializer - - -class HostList(generics.ListCreateAPIView): +class HostViewSet(viewsets.ModelViewSet): queryset = models.Host.objects.all() serializer_class = serializers.HostSerializer -class HostDetail(generics.RetrieveUpdateDestroyAPIView): - queryset = models.Host.objects.all() - serializer_class = serializers.HostSerializer - - -class ResultList(generics.ListCreateAPIView): +class ResultViewSet(viewsets.ModelViewSet): queryset = models.Result.objects.all() serializer_class = serializers.ResultSerializer -class ResultDetail(generics.RetrieveUpdateDestroyAPIView): - queryset = models.Result.objects.all() - serializer_class = serializers.ResultSerializer - - -class FileList(generics.ListCreateAPIView): +class FileViewSet(viewsets.ModelViewSet): queryset = models.File.objects.all() serializer_class = serializers.FileSerializer -class FileDetail(generics.RetrieveUpdateDestroyAPIView): - queryset = models.File.objects.all() - serializer_class = serializers.FileSerializer - - -class StatsList(generics.ListCreateAPIView): - queryset = models.Stats.objects.all() - serializer_class = serializers.StatsSerializer - - -class StatsDetail(generics.RetrieveUpdateDestroyAPIView): +class StatsViewSet(viewsets.ModelViewSet): queryset = models.Stats.objects.all() serializer_class = serializers.StatsSerializer diff --git a/ara/server/admin.py b/ara/server/admin.py new file mode 100644 index 00000000..ef32333b --- /dev/null +++ b/ara/server/admin.py @@ -0,0 +1,6 @@ +from django.contrib import admin + + +class AraAdminSite(admin.AdminSite): + site_header = 'Administration' + index_title = 'Administration Ara' diff --git a/ara/server/apps.py b/ara/server/apps.py new file mode 100644 index 00000000..07a4fc57 --- /dev/null +++ b/ara/server/apps.py @@ -0,0 +1,5 @@ +from django.contrib.admin.apps import AdminConfig + + +class AraAdminConfig(AdminConfig): + default_site = 'ara.server.admin.AraAdminSite' diff --git a/ara/server/settings.py b/ara/server/settings.py index 0f1102fa..297bed51 100644 --- a/ara/server/settings.py +++ b/ara/server/settings.py @@ -19,10 +19,9 @@ DEBUG = env.bool('DJANGO_DEBUG', default=False) ALLOWED_HOSTS = env('ALLOWED_HOSTS', cast=list, default=['localhost', '127.0.0.1', 'testserver']) -ADMINS = (('Guillaume Vincent', 'gvincent@redhat.com'),) +ADMINS = () INSTALLED_APPS = [ - 'django.contrib.admin', 'django.contrib.auth', 'django.contrib.contenttypes', 'django.contrib.sessions', @@ -30,7 +29,8 @@ INSTALLED_APPS = [ 'django.contrib.staticfiles', 'corsheaders', 'rest_framework', - 'ara.api' + 'ara.api', + 'ara.server.apps.AraAdminConfig', ] MIDDLEWARE = [ diff --git a/ara/server/urls.py b/ara/server/urls.py index f82fd400..df0a7cab 100644 --- a/ara/server/urls.py +++ b/ara/server/urls.py @@ -1,13 +1,8 @@ -from django.conf.urls import include, url +from django.urls import include, path from django.contrib import admin -from django.conf import settings -from django.conf.urls.static import static -admin.site.site_header = 'Administration' -admin.site.index_title = 'Administration Ara' -routes = [ - url(r'^api/v1', include('ara.api.urls')), - url(r'^admin', admin.site.urls), +urlpatterns = [ + path('api/v1/', include('ara.api.urls')), + path('admin/', admin.site.urls), ] -urlpatterns = routes + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) diff --git a/requirements.txt b/requirements.txt index 4d893098..73f1d822 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,4 +3,5 @@ pbr!=2.1.0,>=2.0.0 # Apache-2.0 Django>=2 djangorestframework django-cors-headers +drf-extensions envparse