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

@@ -37,8 +37,12 @@ from rest_framework import serializers
 from rest_framework.exceptions import PermissionDenied, ParseError
 
 from ..common.responses import BadRequestResponse
-from ..common.api import (CheckContributionNameView, ShareView,
-                          ListCreateContributionView, RetrieveUpdateDestroyContributionView)
+from ..common.api import (
+    CheckContributionNameView,
+    ShareView,
+    ListCreateContributionView,
+    RetrieveUpdateDestroyContributionView,
+)
 
 from .models import Toolchain
 from .serializers import ToolchainSerializer
@@ -52,7 +56,7 @@ from ..experiments.models import Experiment
 import beat.core.toolchain
 
 
-#----------------------------------------------------------
+# ----------------------------------------------------------
 
 
 class CheckToolchainNameView(CheckContributionNameView):
@@ -60,10 +64,11 @@ class CheckToolchainNameView(CheckContributionNameView):
     This view sanitizes a toolchain name and
     checks whether it is already used.
     """
+
     model = Toolchain
 
 
-#----------------------------------------------------------
+# ----------------------------------------------------------
 
 
 class ShareToolchainView(ShareView):
@@ -71,21 +76,23 @@ class ShareToolchainView(ShareView):
     This view allows to share a toolchain with
     other users and/or teams
     """
+
     model = Toolchain
 
 
-#----------------------------------------------------------
+# ----------------------------------------------------------
 
 
 class ListToolchainView(ListContributionView):
     """
     List all available toolchains
     """
+
     model = Toolchain
     serializer_class = ToolchainSerializer
 
 
-#----------------------------------------------------------
+# ----------------------------------------------------------
 
 
 class ListCreateToolchainsView(ListCreateContributionView):
@@ -93,122 +100,147 @@ class ListCreateToolchainsView(ListCreateContributionView):
     Read/Write end point that list the toolchains available
     from a given author and allows the creation of new toolchains
     """
+
     model = Toolchain
     serializer_class = ToolchainSerializer
     writing_serializer_class = ToolchainCreationSerializer
-    namespace = 'api_toolchains'
+    namespace = "api_toolchains"
 
 
-#----------------------------------------------------------
+# ----------------------------------------------------------
 
 
 class RetrieveUpdateDestroyToolchainsView(RetrieveUpdateDestroyContributionView):
     """
     Read/Write/Delete endpoint for a given toolchain
     """
+
     model = Toolchain
     serializer_class = FullToolchainSerializer
 
-
     def put(self, request, author_name, object_name, version=None):
         if version is None:
-            return BadRequestResponse('A version number must be provided')
+            return BadRequestResponse("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 serializers.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 serializers.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 'strict' in data:
-            strict = data['strict']
+        if "strict" in data:
+            strict = data["strict"]
         else:
             strict = True
 
-        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 json.decoder.JSONDecodeError:
+                    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 toolchain declaration'})
+                    raise serializers.ValidationError(
+                        {
+                            "short_description": "A short description is already provided in the toolchain 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)
 
             toolchain_declaration = beat.core.toolchain.Toolchain(
-                settings.PREFIX, json_declaration)
+                settings.PREFIX, json_declaration
+            )
 
             if not toolchain_declaration.valid:
                 if strict:
-                    raise serializers.ValidationError({'declaration': toolchain_declaration.errors})
+                    raise serializers.ValidationError(
+                        {"declaration": toolchain_declaration.errors}
+                    )
 
         else:
             declaration = None
 
-        if (short_description is not None) and (len(short_description) > self.model._meta.get_field('short_description').max_length):
-            raise serializers.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 serializers.ValidationError(
+                {"short_description": "Short description too long"}
+            )
 
         # Process the query string
-        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:
             # Available fields (not returned by default):
             #    - html_description
-            fields_to_return = ['errors']
-
+            fields_to_return = ["errors"]
 
         # Retrieve the toolchain
-        dbtoolchain = get_object_or_404(Toolchain,
-                                        author__username__iexact=author_name,
-                                        name__iexact=object_name,
-                                        version=version)
+        dbtoolchain = get_object_or_404(
+            Toolchain,
+            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 and not dbtoolchain.modifiable():
-            raise PermissionDenied("The {} isn't modifiable anymore (either shared with someone else, or needed by an attestation)".format(dbtoolchain.model_name()))
+            raise PermissionDenied(
+                "The {} isn't modifiable anymore (either shared with someone else, or needed by an attestation)".format(
+                    dbtoolchain.model_name()
+                )
+            )
 
         errors = None
 
-
         # Modification of the short_description
         if (short_description is not None) and (declaration is None):
             tmp_declaration = dbtoolchain.declaration
-            tmp_declaration['description'] = short_description
+            tmp_declaration["description"] = short_description
             dbtoolchain.declaration = tmp_declaration
 
         # Modification of the description
@@ -217,9 +249,9 @@ class RetrieveUpdateDestroyToolchainsView(RetrieveUpdateDestroyContributionView)
 
         # Modification of the declaration
         if declaration is not None:
-            errors = ''
+            errors = ""
             if not toolchain_declaration.valid:
-                errors = ' * %s' % '\n * '.join(toolchain_declaration.errors)
+                errors = " * %s" % "\n * ".join(toolchain_declaration.errors)
 
             dbtoolchain.errors = errors
             dbtoolchain.declaration = declaration
@@ -240,19 +272,18 @@ class RetrieveUpdateDestroyToolchainsView(RetrieveUpdateDestroyContributionView)
         result = {}
 
         # Retrieve the errors (if necessary)
-        if 'errors' in fields_to_return:
+        if "errors" in fields_to_return:
             if errors:
-                result['errors'] = errors
+                result["errors"] = errors
             else:
-                result['errors'] = ''
-
+                result["errors"] = ""
 
         # Retrieve the description in HTML format (if necessary)
-        if 'html_description' in fields_to_return:
+        if "html_description" in fields_to_return:
             description = dbtoolchain.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)