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/code/api.py

@@ -30,7 +30,6 @@ from django.shortcuts import get_object_or_404
 from django.core.exceptions import ValidationError
 
 from rest_framework import generics
-from rest_framework import permissions
 from rest_framework.response import Response
 from rest_framework.exceptions import PermissionDenied, ParseError
 from rest_framework import serializers
@@ -45,13 +44,14 @@ from .serializers import CodeSharingSerializer, CodeSerializer
 
 import simplejson as json
 
+
 class ShareCodeView(ShareView):
     serializer_class = CodeSharingSerializer
 
     def do_share(self, obj, data):
-        users = data.get('users', None)
-        teams = data.get('teams', None)
-        public = data.get('status') == 'public'
+        users = data.get("users", None)
+        teams = data.get("teams", None)
+        public = data.get("status") == "public"
 
         obj.share(public=public, users=users, teams=teams)
 
@@ -63,34 +63,38 @@ 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
         has_access, open_source, _ = object1.accessibility_for(request.user)
-        if not ((request.user == object1.author) or \
-                (has_access and open_source)):
-            return ForbiddenResponse("You cannot access the source-code of \"%s\"" % object1.fullname())
+        if not ((request.user == object1.author) or (has_access and open_source)):
+            return ForbiddenResponse(
+                'You cannot access the source-code of "%s"' % object1.fullname()
+            )
 
         has_access, open_source, _ = object2.accessibility_for(request.user)
-        if not ((request.user == object2.author) or \
-                (has_access and open_source)):
-            return ForbiddenResponse("You cannot access the source-code of \"%s\"" % object2.fullname())
+        if not ((request.user == object2.author) or (has_access and open_source)):
+            return ForbiddenResponse(
+                'You cannot access the source-code of "%s"' % 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)
 
 
@@ -100,99 +104,120 @@ class RetrieveUpdateDestroyCodeView(RetrieveUpdateDestroyContributionView):
 
     def do_update(self, request, author_name, object_name, version=None):
         if version is None:
-            raise ValidationError({'version': 'A version number must be provided'})
+            raise ValidationError({"version": "A version number must be provided"})
 
         try:
             data = request.data
         except ParseError as e:
-            raise serializers.ValidationError({'data': str(e)})
+            raise serializers.ValidationError({"data": str(e)})
         else:
             if not data:
-                raise serializers.ValidationError({'data': 'Empty'})
-
-        if 'short_description' in data:
-            if not(isinstance(data['short_description'], six.string_types)):
-                raise ValidationError({'short_description': 'Invalid short_description data'})
-            short_description = data['short_description']
+                raise serializers.ValidationError({"data": "Empty"})
+
+        if "short_description" in data:
+            if not (isinstance(data["short_description"], six.string_types)):
+                raise ValidationError(
+                    {"short_description": "Invalid short_description data"}
+                )
+            short_description = data["short_description"]
         else:
             short_description = None
 
-        if 'description' in data:
-            if not(isinstance(data['description'], six.string_types)):
-                raise serializers.ValidationError({'description': 'Invalid description data'})
-            description = data['description']
+        if "description" in data:
+            if not (isinstance(data["description"], six.string_types)):
+                raise serializers.ValidationError(
+                    {"description": "Invalid description data"}
+                )
+            description = data["description"]
             try:
                 validate_restructuredtext(description)
             except ValidationError as errors:
-                raise serializers.ValidationError({'description': [error for error in errors]})
+                raise serializers.ValidationError(
+                    {"description": [error for error in errors]}
+                )
         else:
             description = None
 
-        if 'declaration' in data:
-            if isinstance(data['declaration'], dict):
-                json_declaration = data['declaration']
+        if "declaration" in data:
+            if isinstance(data["declaration"], dict):
+                json_declaration = data["declaration"]
                 declaration = json.dumps(json_declaration, indent=4)
-            elif isinstance(data['declaration'], six.string_types):
-                declaration = data['declaration']
+            elif isinstance(data["declaration"], six.string_types):
+                declaration = data["declaration"]
                 try:
                     json_declaration = json.loads(declaration)
-                except:
-                    raise serializers.ValidationError({'declaration': 'Invalid declaration data'})
+                except Exception:
+                    raise serializers.ValidationError(
+                        {"declaration": "Invalid declaration data"}
+                    )
             else:
-                raise serializers.ValidationError({'declaration': 'Invalid declaration data'})
+                raise serializers.ValidationError(
+                    {"declaration": "Invalid declaration data"}
+                )
 
-            if 'description' in json_declaration:
+            if "description" in json_declaration:
                 if short_description is not None:
-                    raise serializers.ValidationError({'short_description': 'A short description is already provided in the declaration'})
+                    raise serializers.ValidationError(
+                        {
+                            "short_description": "A short description is already provided in the declaration"
+                        }
+                    )
 
-                short_description = json_declaration['description']
+                short_description = json_declaration["description"]
             elif short_description is not None:
-                json_declaration['description'] = short_description
+                json_declaration["description"] = short_description
                 declaration = json.dumps(json_declaration, indent=4)
         else:
             declaration = None
             json_declaration = None
 
-        if (short_description is not None) and (len(short_description) > self.model._meta.get_field('short_description').max_length):
-            raise ValidationError({'short_description': 'Short description too long'})
+        if (short_description is not None) and (
+            len(short_description)
+            > self.model._meta.get_field("short_description").max_length
+        ):
+            raise ValidationError({"short_description": "Short description too long"})
 
-        if 'code' in data:
-            if not(isinstance(data['code'], six.string_types)):
-                raise ValidationError({'code': 'Invalid code data'})
-            code = data['code']
+        if "code" in data:
+            if not (isinstance(data["code"], six.string_types)):
+                raise ValidationError({"code": "Invalid code data"})
+            code = data["code"]
         else:
             code = None
 
         # Retrieve the object
-        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 still be modified (if applicable, the
         # documentation can always be modified)
-        if ((declaration is not None) or (code is not None)) and \
-                not(db_object.modifiable()):
-            raise PermissionDenied("The {} isn't modifiable anymore (either shared with someone else, or needed by an attestation)".format(db_object.model_name()))
-
+        if ((declaration is not None) or (code is not None)) and not (
+            db_object.modifiable()
+        ):
+            raise PermissionDenied(
+                "The {} isn't modifiable anymore (either shared with someone else, or needed by an attestation)".format(
+                    db_object.model_name()
+                )
+            )
 
         # Modification of the documentation
         if (short_description is not None) and (declaration is None):
             tmp_declaration = db_object.declaration
-            tmp_declaration['description'] = short_description
+            tmp_declaration["description"] = short_description
             db_object.declaration = tmp_declaration
 
         if description is not None:
             db_object.description = description
 
-
         # Modification of the declaration
         modified = False
         if declaration is not None:
             db_object.declaration = declaration
             modified = True
 
-
         # Modification of the source code
         if code is not None:
             db_object.source_code = code
@@ -202,7 +227,6 @@ class RetrieveUpdateDestroyCodeView(RetrieveUpdateDestroyContributionView):
 
         return modified, db_object
 
-
     def get(self, request, *args, **kwargs):
         db_objects = self.get_queryset()
 
@@ -210,13 +234,15 @@ class RetrieveUpdateDestroyCodeView(RetrieveUpdateDestroyContributionView):
             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)
 
         # Check that the user can access it
-        (has_access, open_source, accessibility) = db_object.accessibility_for(request.user)
+        (has_access, open_source, accessibility) = db_object.accessibility_for(
+            request.user
+        )
 
         # Process the query string
         # Other available fields (not returned by default):
@@ -226,34 +252,40 @@ class RetrieveUpdateDestroyCodeView(RetrieveUpdateDestroyContributionView):
         #    - needed_dataformats
         #    - attestations
         fields_to_remove = []
-        if ((request.user != db_object.author) and not(open_source)) or db_object.is_binary():
-            fields_to_remove = ['code']
+        if (
+            (request.user != db_object.author) and not (open_source)
+        ) or db_object.is_binary():
+            fields_to_remove = ["code"]
 
-        fields_to_return = self.get_serializer_fields(request, allow_sharing=(request.user == db_object.author),
-                                                      exclude_fields=fields_to_remove)
+        fields_to_return = self.get_serializer_fields(
+            request,
+            allow_sharing=(request.user == db_object.author),
+            exclude_fields=fields_to_remove,
+        )
 
         serializer = self.get_serializer(db_object, fields=fields_to_return)
         return Response(serializer.data)
 
-
     def put(self, request, author_name, object_name, version=None):
-        (modified, db_object) = self.do_update(request, author_name, object_name, version)
+        (modified, db_object) = self.do_update(
+            request, author_name, object_name, version
+        )
 
         # Available fields (not returned by default):
         #    - html_description
-        if 'fields' in request.GET:
-            fields_to_return = request.GET['fields'].split(',')
+        if "fields" in request.GET:
+            fields_to_return = request.GET["fields"].split(",")
         else:
             return Response(status=204)
 
         result = {}
 
         # Retrieve the description in HTML format
-        if 'html_description' in fields_to_return:
+        if "html_description" in fields_to_return:
             description = db_object.description
             if len(description) > 0:
-                result['html_description'] = ensure_html(description)
+                result["html_description"] = ensure_html(description)
             else:
-                result['html_description'] = ''
+                result["html_description"] = ""
 
         return Response(result)