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

@@ -15,29 +15,19 @@
 #  You should have received a copy of the GNU General Public License
 #  along with ARA.  If not, see <http://www.gnu.org/licenses/>.
 
-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<pk>[0-9]+)$', views.LabelDetail.as_view(), name='label-detail'),
-    url(r'^/playbooks$', views.PlaybookList.as_view(), name='playbook-list'),
-    url(r'^/playbooks/(?P<pk>[0-9]+)$', views.PlaybookDetail.as_view(), name='playbook-detail'),
-    url(r'^/playbooks/(?P<pk>[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<pk>[0-9]+)$', views.PlayDetail.as_view(), name='play-detail'),
-    url(r'^/tasks$', views.TaskList.as_view(), name='task-list'),
-    url(r'^/tasks/(?P<pk>[0-9]+)$', views.TaskDetail.as_view(), name='task-detail'),
-    url(r'^/hosts$', views.HostList.as_view(), name='host-list'),
-    url(r'^/hosts/(?P<pk>[0-9]+)$', views.HostDetail.as_view(), name='host-detail'),
-    url(r'^/results$', views.ResultList.as_view(), name='result-list'),
-    url(r'^/results/(?P<pk>[0-9]+)$', views.ResultDetail.as_view(), name='result-detail'),
-    url(r'^/files$', views.FileList.as_view(), name='file-list'),
-    url(r'^/files/(?P<pk>[0-9]+)$', views.FileDetail.as_view(), name='file-detail'),
-    url(r'^/stats$', views.StatsList.as_view(), name='stats-list'),
-    url(r'^/stats/(?P<pk>[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

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 <http://www.gnu.org/licenses/>.
-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

ara/server/admin.py

@@ -0,0 +1,6 @@
+from django.contrib import admin
+
+
+class AraAdminSite(admin.AdminSite):
+    site_header = 'Administration'
+    index_title = 'Administration Ara'

ara/server/apps.py

@@ -0,0 +1,5 @@
+from django.contrib.admin.apps import AdminConfig
+
+
+class AraAdminConfig(AdminConfig):
+    default_site = 'ara.server.admin.AraAdminSite'

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 = [

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)

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