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

@@ -25,33 +25,26 @@
 #                                                                             #
 ###############################################################################
 
-from django.utils import six
 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
+from rest_framework import exceptions as drf_exceptions
 
-from ..common.responses import ForbiddenResponse
 from ..common.api import ShareView, RetrieveUpdateDestroyContributionView
-from ..common.utils import validate_restructuredtext, ensure_html
 from ..common.serializers import DiffSerializer
 
 from ..code.models import Code
 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)
 
@@ -62,35 +55,36 @@ 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)
 
-        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)
+        object1 = get_object_or_404(
+            self.model,
+            author__username__iexact=author1,
+            name__iexact=name1,
+            version=int(version1),
+        )
 
+        object2 = get_object_or_404(
+            self.model,
+            author__username__iexact=author2,
+            name__iexact=name2,
+            version=int(version2),
+        )
 
         # 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)):
+            raise drf_exceptions.PermissionDenied(
+                '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)):
+            raise drf_exceptions.PermissionDenied(
+                '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)
 
 
@@ -98,125 +92,13 @@ class RetrieveUpdateDestroyCodeView(RetrieveUpdateDestroyContributionView):
     model = Code
     serializer_class = CodeSerializer
 
-    def do_update(self, request, author_name, object_name, version=None):
-        if version is None:
-            raise ValidationError({'version': 'A version number must be provided'})
-
-        try:
-            data = request.data
-        except ParseError as 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']
-        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']
-            try:
-                validate_restructuredtext(description)
-            except ValidationError as 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']
-                declaration = json.dumps(json_declaration, indent=4)
-            elif isinstance(data['declaration'], six.string_types):
-                declaration = data['declaration']
-                try:
-                    json_declaration = json.loads(declaration)
-                except:
-                    raise serializers.ValidationError({'declaration': 'Invalid declaration data'})
-            else:
-                raise serializers.ValidationError({'declaration': 'Invalid declaration data'})
-
-            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'})
-
-                short_description = json_declaration['description']
-            elif short_description is not None:
-                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 '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)
-
-        # 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()))
-
-
-        # Modification of the documentation
-        if (short_description is not None) and (declaration is None):
-            tmp_declaration = db_object.declaration
-            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
-            modified = True
-
-        db_object.save()
-
-        return modified, db_object
-
-
     def get(self, request, *args, **kwargs):
-        db_objects = self.get_queryset()
-
-        if db_objects.count() == 0:
-            return Response(status=404)
-
-        db_object = db_objects[0]
-        version = int(self.kwargs.get('version', -1))
-
-        if version != -1 and db_object.version != version:
-            return Response(status=404)
+        db_object = self.get_object()
 
         # 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 +108,16 @@ 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)
-
-        # Available fields (not returned by default):
-        #    - html_description
-        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:
-            description = db_object.description
-            if len(description) > 0:
-                result['html_description'] = ensure_html(description)
-            else:
-                result['html_description'] = ''
-
-        return Response(result)