Merge pull request #399 from geoadmin/develop

New Release v1.21.0 - #minor

app/stac_api/apps.py

@@ -4,7 +4,9 @@
 import django.core.exceptions
 from django.apps import AppConfig
 from django.conf import settings
+from django.http import Http404
 
+import rest_framework.exceptions
 from rest_framework import serializers
 from rest_framework.response import Response
 from rest_framework.views import exception_handler
@@ -25,6 +27,7 @@ def ready(self):
 def custom_exception_handler(exc, context):
     # NOTE: this exception handler is only called for REST Framework endpoints. Other endpoints
     # exception are handled via middleware.exception.
+
     if isinstance(exc, django.core.exceptions.ValidationError):
         # Translate django ValidationError to Rest Framework ValidationError,
         # this is required because some validation cannot be done in the Rest
@@ -34,6 +37,10 @@ def custom_exception_handler(exc, context):
         if exc.params:
             message %= exc.params
         exc = serializers.ValidationError(exc.message, exc.code)
+    elif isinstance(exc, Http404):
+        exc = rest_framework.exceptions.NotFound()
+    elif isinstance(exc, django.core.exceptions.PermissionDenied):
+        exc = rest_framework.exceptions.PermissionDenied()
 
     # Then call REST framework's default exception handler, to get the standard error response.
     response = exception_handler(exc, context)
@@ -42,7 +49,7 @@ def custom_exception_handler(exc, context):
         # pylint: disable=protected-access
         extra = {
             "request": context['request']._request,
-            "request.query": context['request']._request.GET.urlencode()
+            "request.query": context['request']._request.GET.urlencode(),
         }
 
         if (
@@ -53,7 +60,12 @@ def custom_exception_handler(exc, context):
             extra["request.payload"] = context['request'].data
 
         logger.error("Response %s: %s", response.status_code, response.data, extra=extra)
-        response.data = {'code': response.status_code, 'description': response.data}
+
+        data = {'code': response.status_code, 'description': exc.detail}
+        # Add any exception data object to the response if any (See StacAPIException)
+        if hasattr(exc, 'data'):
+            data.update(exc.data)
+        response.data = data
         return response
 
     # If we don't have a response that's means that we have an unhandled exception that needs to

app/stac_api/exceptions.py

@@ -0,0 +1,34 @@
+import logging
+
+from django.utils.translation import gettext_lazy as _
+
+import rest_framework.exceptions
+from rest_framework import status
+
+logger = logging.getLogger(__name__)
+
+
+class StacAPIException(rest_framework.exceptions.APIException):
+    '''STAC API custom exception
+
+    These exception can add additional data to the HTTP response.
+    '''
+
+    def __init__(self, detail=None, code=None, data=None):
+        super().__init__(detail, code)
+        if isinstance(data, dict):
+            self.data = data
+        elif data:
+            self.data = {'data': data}
+
+
+class UploadNotInProgressError(StacAPIException):
+    status_code = status.HTTP_409_CONFLICT
+    default_detail = _('No upload in progress')
+    default_code = 'conflict'
+
+
+class UploadInProgressError(StacAPIException):
+    status_code = status.HTTP_409_CONFLICT
+    default_detail = _('Upload already in progress')
+    default_code = 'conflict'

app/stac_api/s3_multipart_upload.py

@@ -11,7 +11,7 @@
 
 from rest_framework import serializers
 
-from stac_api.serializers import UploadNotInProgressError
+from stac_api.exceptions import UploadNotInProgressError
 from stac_api.utils import get_s3_cache_control_value
 from stac_api.utils import get_s3_client
 from stac_api.utils import isoformat

app/stac_api/serializers.py

@@ -7,7 +7,6 @@
 from django.utils.translation import gettext_lazy as _
 
 from rest_framework import serializers
-from rest_framework.exceptions import APIException
 from rest_framework.utils.serializer_helpers import ReturnDict
 from rest_framework.validators import UniqueValidator
 from rest_framework_gis import serializers as gis_serializers
@@ -828,9 +827,3 @@ class Meta:
     parts = serializers.ListField(
         source='Parts', child=UploadPartSerializer(), default=list, read_only=True
     )
-
-
-class UploadNotInProgressError(APIException):
-    status_code = 409
-    default_detail = 'Upload not in progress'
-    default_code = 'conflict'

app/stac_api/views.py

@@ -13,12 +13,15 @@
 from rest_framework import generics
 from rest_framework import mixins
 from rest_framework import serializers
+from rest_framework.exceptions import APIException
 from rest_framework.generics import get_object_or_404
 from rest_framework.permissions import AllowAny
 from rest_framework.response import Response
 from rest_framework_condition import etag
 
 from stac_api import views_mixins
+from stac_api.exceptions import UploadInProgressError
+from stac_api.exceptions import UploadNotInProgressError
 from stac_api.models import Asset
 from stac_api.models import AssetUpload
 from stac_api.models import Collection
@@ -35,7 +38,6 @@
 from stac_api.serializers import ConformancePageSerializer
 from stac_api.serializers import ItemSerializer
 from stac_api.serializers import LandingPageSerializer
-from stac_api.serializers import UploadNotInProgressError
 from stac_api.serializers_utils import get_relation_links
 from stac_api.utils import get_asset_path
 from stac_api.utils import harmonize_post_get_for_search
@@ -621,8 +623,8 @@ def _save_asset_upload(self, executor, serializer, key, asset, upload_id, urls):
                 }
             )
             if bool(self.get_in_progress_queryset()):
-                raise serializers.ValidationError(
-                    code='unique', detail=_('Upload already in progress')
+                raise UploadInProgressError(
+                    data={"upload_id": self.get_in_progress_queryset()[0].upload_id}
                 ) from None
             raise
 
@@ -647,7 +649,7 @@ def create_multipart_upload(self, executor, serializer, validated_data, asset):
                 )
 
             self._save_asset_upload(executor, serializer, key, asset, upload_id, urls)
-        except serializers.ValidationError as err:
+        except APIException as err:
             executor.abort_multipart_upload(key, asset, upload_id)
             raise
 

app/tests/test_asset_upload_endpoint.py

@@ -242,6 +242,7 @@ def test_asset_upload_create_multipart_duplicate(self):
         json_data = response.json()
         self.check_created_response(json_data)
         self.check_urls_response(json_data['urls'], number_parts)
+        initial_upload_id = json_data['upload_id']
 
         response = self.client.post(
             self.get_create_multipart_upload_path(),
@@ -252,8 +253,18 @@ def test_asset_upload_create_multipart_duplicate(self):
             },
             content_type="application/json"
         )
-        self.assertStatusCode(400, response)
-        self.assertEqual(response.json()['description'], ["Upload already in progress"])
+        self.assertStatusCode(409, response)
+        self.assertEqual(response.json()['description'], "Upload already in progress")
+        self.assertIn(
+            "upload_id",
+            response.json(),
+            msg="The upload id of the current upload is missing from response"
+        )
+        self.assertEqual(
+            initial_upload_id,
+            response.json()['upload_id'],
+            msg="Current upload ID not matching the one from the 409 Conflict response"
+        )
 
         self.assertEqual(
             self.get_asset_upload_queryset().filter(status=AssetUpload.Status.IN_PROGRESS).count(),
@@ -313,13 +324,13 @@ def asset_upload_atomic_create_test(worker):
         results, errors = self.run_parallel(workers, asset_upload_atomic_create_test)
 
         for _, response in results:
-            self.assertStatusCode([201, 400], response)
+            self.assertStatusCode([201, 409], response)
 
         ok_201 = [r for _, r in results if r.status_code == 201]
-        bad_400 = [r for _, r in results if r.status_code == 400]
+        bad_409 = [r for _, r in results if r.status_code == 409]
         self.assertEqual(len(ok_201), 1, msg="More than 1 parallel request was successful")
-        for response in bad_400:
-            self.assertEqual(response.json()['description'], ["Upload already in progress"])
+        for response in bad_409:
+            self.assertEqual(response.json()['description'], "Upload already in progress")
 
 
 class AssetUpload1PartEndpointTestCase(AssetUploadBaseTest):
@@ -986,7 +997,7 @@ def test_asset_upload_1_parts_duplicate_complete(self):
         )
         self.assertStatusCode(409, response)
         self.assertEqual(response.json()['code'], 409)
-        self.assertEqual(response.json()['description'], {'detail': 'Upload not in progress'})
+        self.assertEqual(response.json()['description'], 'No upload in progress')
 
 
 class AssetUploadDeleteInProgressEndpointTestCase(AssetUploadBaseTest):

spec/static/spec/v0.9/openapitransactional.yaml

@@ -1040,6 +1040,13 @@ paths:
           $ref: "#/components/responses/BadRequest"
         "404":
           $ref: "#/components/responses/NotFound"
+        "409":
+          description: Another upload is already in progress, you need first to complete
+            or abort it before creating a new upload.
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/uploadInProgress"
         5XX:
           $ref: "#/components/responses/ServerError"
   /collections/{collectionId}/items/{featureId}/assets/{assetId}/uploads/{uploadId}:
@@ -1254,8 +1261,7 @@ paths:
                 $ref: "#/components/schemas/exception"
               example:
                 code: 409
-                description:
-                  detail: Upload not in progress
+                description: No upload in progress
         "404":
           $ref: "#/components/responses/NotFound"
         5XX:
@@ -3950,6 +3956,25 @@ components:
       type: string
       description: The RFC7232 ETag for the specified uploaded part.
       example: "d01af8b8ebbf899e30095be8754b377ddb0f0ed0f7fddbc33ac23b0d1969736b"
+    uploadInProgress:
+      description: Another upload is already in progress
+      properties:
+        code:
+          type: integer
+          example: 409
+        description:
+          type: string
+          description: Description of the error
+          example: Upload already in progress
+        upload_id:
+          title: ID
+          type: string
+          description: Current Asset upload unique identifier
+          example: KrFTuglD.N8ireqry_w3.oQqNwrYI7SfSXpVRiusKah0YigDnuM06hfJNIUZg4R_No0MMW9FLU2UG5anTW0boTUYVxKfBZWCFXqnQTpjnQEo1K7la39MYpjSTvIbZgnG
+      required:
+      - code
+      - upload_id
+      type: object
   examples:
     inprogress:
       summary: In progress upload example

spec/transaction/components/schemas.yaml

@@ -554,3 +554,22 @@ components:
       type: string
       description: The RFC7232 ETag for the specified uploaded part.
       example: "d01af8b8ebbf899e30095be8754b377ddb0f0ed0f7fddbc33ac23b0d1969736b"
+    uploadInProgress:
+      description: Another upload is already in progress
+      properties:
+        code:
+          type: integer
+          example: 409
+        description:
+          type: string
+          description: Description of the error
+          example: Upload already in progress
+        upload_id:
+          title: ID
+          type: string
+          description: Current Asset upload unique identifier
+          example: KrFTuglD.N8ireqry_w3.oQqNwrYI7SfSXpVRiusKah0YigDnuM06hfJNIUZg4R_No0MMW9FLU2UG5anTW0boTUYVxKfBZWCFXqnQTpjnQEo1K7la39MYpjSTvIbZgnG
+      required:
+        - code
+        - upload_id
+      type: object

spec/transaction/paths.yaml

@@ -503,6 +503,12 @@ paths:
           $ref: "../components/responses.yaml#/components/responses/BadRequest"
         "404":
           $ref: "../components/responses.yaml#/components/responses/NotFound"
+        "409":
+          description: Another upload is already in progress, you need first to complete or abort it before creating a new upload.
+          content:
+            application/json:
+              schema:
+                $ref: "./schemas.yaml#/components/schemas/uploadInProgress"
         "5XX":
           $ref: "../components/responses.yaml#/components/responses/ServerError"
   "/collections/{collectionId}/items/{featureId}/assets/{assetId}/uploads/{uploadId}":
@@ -714,8 +720,7 @@ paths:
                 $ref: "./schemas.yaml#/components/schemas/exception"
               example:
                 code: 409
-                description:
-                  detail: Upload not in progress
+                description: No upload in progress
         "404":
           $ref: "../components/responses.yaml#/components/responses/NotFound"
         "5XX":