Refactor update creation api

This merge request does the following:

• Raise DRF errors rather than returning response to take advantage of the DRF exception handling mechanism
• Create serialisers for PUT request to simplify the code
• Improve validation checks in several places
• Fixes several issues in DRF infrastructure uses
• Fixes JSON fields handling
• Remove routes that do no fully qualify objects
• Implement a custom DRF exception handler that will allow to log errors
• Improve testing
• Improve permission management

beat/web/common/api.py

@@ -37,8 +37,14 @@ from .responses import BadRequestResponse, ForbiddenResponse
 from .models import Contribution, Versionable
 from .permissions import IsAuthor
 from .exceptions import ShareError, BaseCreationError
-from .serializers import SharingSerializer, ContributionSerializer, CheckNameSerializer, DiffSerializer
+from .serializers import (
+    SharingSerializer,
+    ContributionSerializer,
+    CheckNameSerializer,
+    DiffSerializer,
+)
 from .mixins import CommonContextMixin, SerializerFieldsMixin, IsAuthorOrReadOnlyMixin
+from .utils import py3_cmp
 
 from . import is_true
 
@@ -49,7 +55,7 @@ class CheckContributionNameView(CommonContextMixin, generics.CreateAPIView):
 
     def get_serializer_context(self):
         context = super(CheckContributionNameView, self).get_serializer_context()
-        context['model'] = self.model
+        context["model"] = self.model
         return context
 
     def post(self, request):
@@ -63,17 +69,19 @@ class ShareView(CommonContextMixin, generics.CreateAPIView):
     serializer_class = SharingSerializer
 
     def get_queryset(self):
-        author_name = self.kwargs.get('author_name')
-        object_name = self.kwargs.get('object_name')
-        version = self.kwargs.get('version')
-        return get_object_or_404(self.model,
-                                 author__username__iexact=author_name,
-                                 name__iexact=object_name,
-                                 version=version)
+        author_name = self.kwargs.get("author_name")
+        object_name = self.kwargs.get("object_name")
+        version = self.kwargs.get("version")
+        return get_object_or_404(
+            self.model,
+            author__username__iexact=author_name,
+            name__iexact=object_name,
+            version=version,
+        )
 
     def do_share(self, obj, data):
-        users = data.get('users', None)
-        teams = data.get('teams', None)
+        users = data.get("users", None)
+        teams = data.get("teams", None)
         obj.share(users=users, teams=teams)
 
     def post(self, request, *args, **kwargs):
@@ -93,7 +101,9 @@ class ShareView(CommonContextMixin, generics.CreateAPIView):
         return Response(object_db.sharing_preferences())
 
 
-class ListContributionView(CommonContextMixin, SerializerFieldsMixin, generics.ListAPIView):
+class ListContributionView(
+    CommonContextMixin, SerializerFieldsMixin, generics.ListAPIView
+):
     model = Contribution
     serializer_class = ContributionSerializer
     permission_classes = [permissions.AllowAny]
@@ -103,33 +113,42 @@ class ListContributionView(CommonContextMixin, SerializerFieldsMixin, generics.L
 
     def get(self, request, *args, **kwargs):
         fields_to_return = self.get_serializer_fields(request)
-        limit_to_latest_versions = is_true(request.query_params.get('latest_versions', False))
+        limit_to_latest_versions = is_true(
+            request.query_params.get("latest_versions", False)
+        )
 
         all_contributions = self.get_queryset().select_related()
-        if hasattr(self.model, 'author'):
-            all_contributions = all_contributions.order_by('author__username', 'name', '-version')
+        if hasattr(self.model, "author"):
+            all_contributions = all_contributions.order_by(
+                "author__username", "name", "-version"
+            )
         else:
-            all_contributions = all_contributions.order_by('name', '-version')
+            all_contributions = all_contributions.order_by("name", "-version")
 
         if limit_to_latest_versions:
             all_contributions = self.model.filter_latest_versions(all_contributions)
             # Sort the data formats and sends the response
-            all_contributions.sort(lambda x, y: cmp(x.fullname(), y.fullname()))
+            all_contributions.sort(lambda x, y: py3_cmp(x.fullname(), y.fullname()))
 
-        serializer = self.get_serializer(all_contributions, many=True, fields=fields_to_return)
+        serializer = self.get_serializer(
+            all_contributions, many=True, fields=fields_to_return
+        )
         return Response(serializer.data)
 
 
-class ListCreateBaseView(CommonContextMixin, SerializerFieldsMixin, generics.ListCreateAPIView):
-
+class ListCreateBaseView(
+    CommonContextMixin, SerializerFieldsMixin, generics.ListCreateAPIView
+):
     def get_serializer(self, *args, **kwargs):
-        if self.request.method == 'POST':
+        if self.request.method == "POST":
             self.serializer_class = self.writing_serializer_class
         return super(ListCreateBaseView, self).get_serializer(*args, **kwargs)
 
     def get(self, request, *args, **kwargs):
         fields_to_return = self.get_serializer_fields(request)
-        limit_to_latest_versions = is_true(request.query_params.get('latest_versions', False))
+        limit_to_latest_versions = is_true(
+            request.query_params.get("latest_versions", False)
+        )
 
         objects = self.get_queryset().select_related()
 
@@ -143,7 +162,7 @@ class ListCreateBaseView(CommonContextMixin, SerializerFieldsMixin, generics.Lis
         serializer = self.get_serializer(data=request.data)
         if serializer.is_valid():
             try:
-                if hasattr(self.model, 'author'):
+                if hasattr(self.model, "author"):
                     db_object = serializer.save(author=request.user)
                 else:
                     db_object = serializer.save()
@@ -157,22 +176,25 @@ class ListCreateBaseView(CommonContextMixin, SerializerFieldsMixin, generics.Lis
             html_view_args.insert(0, db_object.author.username)
 
         result = {
-            'name': db_object.name,
-            'full_name': db_object.fullname(),
-            'url': reverse('{}:all'.format(self.namespace)) + db_object.fullname() + '/',
-            'object_view': reverse('{}:view'.format(self.namespace.split('_')[1]), args=html_view_args),
+            "name": db_object.name,
+            "full_name": db_object.fullname(),
+            "url": reverse("{}:all".format(self.namespace))
+            + db_object.fullname()
+            + "/",
+            "object_view": reverse(
+                "{}:view".format(self.namespace.split("_")[1]), args=html_view_args
+            ),
         }
 
         response = Response(result, status=201)
-        response['Location'] = result['url']
+        response["Location"] = result["url"]
         return response
 
 
 class ListCreateContributionView(IsAuthorOrReadOnlyMixin, ListCreateBaseView):
-
     def get_queryset(self):
         user = self.request.user
-        author_name = self.kwargs.get('author_name')
+        author_name = self.kwargs.get("author_name")
         return self.model.objects.from_author_and_public(user, author_name)
 
 
@@ -183,19 +205,22 @@ class DiffView(generics.RetrieveAPIView):
     def get(self, request, author1, name1, version1, author2, name2, version2):
         # Retrieve the objects
         try:
-            object1 = self.model.objects.get(author__username__iexact=author1,
-                                             name__iexact=name1,
-                                             version=int(version1))
-        except:
-            return Response('%s/%s/%s' % (author1, name1, version1), status=404)
+            object1 = self.model.objects.get(
+                author__username__iexact=author1,
+                name__iexact=name1,
+                version=int(version1),
+            )
+        except Exception:
+            return Response("%s/%s/%s" % (author1, name1, version1), status=404)
 
         try:
-            object2 = self.model.objects.get(author__username__iexact=author2,
-                                             name__iexact=name2,
-                                             version=int(version2))
-        except:
-            return Response('%s/%s/%s' % (author2, name2, version2), status=404)
-
+            object2 = self.model.objects.get(
+                author__username__iexact=author2,
+                name__iexact=name2,
+                version=int(version2),
+            )
+        except Exception:
+            return Response("%s/%s/%s" % (author2, name2, version2), status=404)
 
         # Check that the user can access them
         accessibility = object1.accessibility_for(request.user)
@@ -207,27 +232,39 @@ class DiffView(generics.RetrieveAPIView):
             return ForbiddenResponse(object2.fullname())
 
         # Compute the diff
-        serializer = self.get_serializer({'object1': object1,
-                                          'object2': object2})
+        serializer = self.get_serializer({"object1": object1, "object2": object2})
         return Response(serializer.data)
 
 
-class RetrieveUpdateDestroyContributionView(CommonContextMixin, SerializerFieldsMixin, IsAuthorOrReadOnlyMixin, generics.RetrieveUpdateDestroyAPIView):
+class RetrieveUpdateDestroyContributionView(
+    CommonContextMixin,
+    SerializerFieldsMixin,
+    IsAuthorOrReadOnlyMixin,
+    generics.RetrieveUpdateDestroyAPIView,
+):
     model = Contribution
 
     def get_queryset(self):
-        version = self.kwargs.get('version', None)
-        author_name = self.kwargs.get('author_name')
-        object_name = self.kwargs.get('object_name')
+        version = self.kwargs.get("version", None)
+        author_name = self.kwargs.get("author_name")
+        object_name = self.kwargs.get("object_name")
         user = self.request.user
         if version is not None:
-            queryset = self.model.objects.for_user(user, True).filter(author__username__iexact=author_name,
-                                                                      name__iexact=object_name,
-                                                                      version__gte=version)\
-                .order_by('version')
+            queryset = (
+                self.model.objects.for_user(user, True)
+                .filter(
+                    author__username__iexact=author_name,
+                    name__iexact=object_name,
+                    version__gte=version,
+                )
+                .order_by("version")
+            )
         else:
-            queryset = self.model.objects.for_user(user, True).filter(author__username__iexact=author_name,
-                                                                      name__iexact=object_name).order_by('-version')
+            queryset = (
+                self.model.objects.for_user(user, True)
+                .filter(author__username__iexact=author_name, name__iexact=object_name)
+                .order_by("-version")
+            )
 
         return queryset
 
@@ -237,40 +274,47 @@ class RetrieveUpdateDestroyContributionView(CommonContextMixin, SerializerFields
         if db_objects.count() == 0:
             return Response(status=404)
 
-
         db_object = db_objects[0]
-        version = int(self.kwargs.get('version', -1))
+        version = int(self.kwargs.get("version", -1))
 
         if version != -1 and db_object.version != version:
             return Response(status=404)
 
         # Process the query string
-        allow_sharing = hasattr(db_object, 'author') and (request.user == db_object.author)
+        allow_sharing = hasattr(db_object, "author") and (
+            request.user == db_object.author
+        )
 
-        fields_to_return = self.get_serializer_fields(request, allow_sharing=allow_sharing)
+        fields_to_return = self.get_serializer_fields(
+            request, allow_sharing=allow_sharing
+        )
 
         serializer = self.get_serializer(db_object, fields=fields_to_return)
         return Response(serializer.data)
 
-
     def delete(self, request, *args, **kwargs):
-        author_name = self.kwargs.get('author_name')
-        object_name = self.kwargs.get('object_name')
-        version = self.kwargs.get('version', None)
+        author_name = self.kwargs.get("author_name")
+        object_name = self.kwargs.get("object_name")
+        version = self.kwargs.get("version", None)
 
         # Retrieve the object
         if version is None:
-            return BadRequestResponse('A version number must be provided')
+            return BadRequestResponse("A version number must be provided")
 
-        db_object = get_object_or_404(self.model,
-                                      author__username__iexact=author_name,
-                                      name__iexact=object_name,
-                                      version=version)
+        db_object = get_object_or_404(
+            self.model,
+            author__username__iexact=author_name,
+            name__iexact=object_name,
+            version=version,
+        )
 
         # Check that the object can be deleted
-        if not(db_object.deletable()):
-            return ForbiddenResponse("The {} can't be deleted anymore (needed by an attestation, an algorithm or another data format)".format(db_object.model_name()))
-
+        if not (db_object.deletable()):
+            return ForbiddenResponse(
+                "The {} can't be deleted anymore (needed by an attestation, an algorithm or another data format)".format(
+                    db_object.model_name()
+                )
+            )
 
         # Deletion of the object
         db_object.delete()