Remove the tus chunk endpoints from the schema (#5961)

As explained in the comment in `_tus_chunk_action`, I don't think we need explicit documentation for these endpoints. Hiding them makes the documentation a bit cleaner, especially because these endpoints were indistinguishable from one another. It also removes the corresponding SDK code, which is good, since clients should follow the URL returned by the creation endpoint, not invoke the chunk endpoints directly.
cvat-ai · Apr 6, 2023 · 57b8766 · 57b8766
1 parent 5203119
commit 57b8766
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 378 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Cloud storage unique_together limitation (<https://github.com/opencv/cvat/pull/5855>)
 - Support for redundant request media types in the API
   (<https://github.com/opencv/cvat/pull/5874>)
+- Static URLs and direct SDK support for the tus chunk endpoints.
+  Clients must use the `Location` header from the response to the `Upload-Length` request,
+  as per the tus creation protocol
+  (<https://github.com/opencv/cvat/pull/5961>)
 
 ### Fixed
 - An invalid project/org handling in webhooks (<https://github.com/opencv/cvat/pull/5707>)

diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py
@@ -25,7 +25,7 @@
 
 from drf_spectacular.utils import OpenApiExample, extend_schema_field, extend_schema_serializer
 
-from cvat.apps.engine.view_utils import build_field_filter_params, get_list_view_name, reverse
+from cvat.apps.engine.utils import build_field_filter_params, get_list_view_name, reverse
 
 @extend_schema_field(serializers.URLField)
 class HyperlinkedEndpointSerializer(serializers.Serializer):

diff --git a/cvat/apps/engine/utils.py b/cvat/apps/engine/utils.py
@@ -9,10 +9,16 @@
 import importlib
 import sys
 import traceback
+from typing import Any, Dict, Optional
 import subprocess
 import os
 import urllib.parse
+
+from django.http.request import HttpRequest
 from django.utils import timezone
+from django.utils.http import urlencode
+
+from rest_framework.reverse import reverse as _reverse
 
 from av import VideoFrame
 from PIL import Image
@@ -176,3 +182,37 @@ def get_rq_job_meta(request, db_obj):
         'task_id': tid,
         'job_id': jid,
     }
+
+def reverse(viewname, *, args=None, kwargs=None,
+    query_params: Optional[Dict[str, str]] = None,
+    request: Optional[HttpRequest] = None,
+) -> str:
+    """
+    The same as rest_framework's reverse(), but adds custom query params support.
+    The original request can be passed in the 'request' parameter to
+    return absolute URLs.
+    """
+
+    url = _reverse(viewname, args, kwargs, request)
+
+    if query_params:
+        return f'{url}?{urlencode(query_params)}'
+
+    return url
+
+def build_field_filter_params(field: str, value: Any) -> Dict[str, str]:
+    """
+    Builds a collection filter query params for a single field and value.
+    """
+    return { field: value }
+
+def get_list_view_name(model):
+    # Implemented after
+    # rest_framework/utils/field_mapping.py.get_detail_view_name()
+    """
+    Given a model class, return the view name to use for URL relationships
+    that refer to instances of the model.
+    """
+    return '%(model_name)s-list' % {
+        'model_name': model._meta.object_name.lower()
+    }
diff --git a/cvat/apps/engine/view_utils.py b/cvat/apps/engine/view_utils.py
@@ -4,21 +4,22 @@
 
 # NOTE: importing in the utils.py header leads to circular importing
 
-from typing import Any, Dict, Optional, Type
+from typing import Optional, Type
 
 from django.db.models.query import QuerySet
 from django.http.request import HttpRequest
 from django.http.response import HttpResponse
 from django.shortcuts import get_object_or_404
-from django.utils.http import urlencode
 from rest_framework.decorators import action
 from rest_framework.exceptions import PermissionDenied
 from rest_framework.response import Response
-from rest_framework.reverse import reverse as _reverse
 from rest_framework.serializers import Serializer
 from rest_framework.viewsets import GenericViewSet
+from drf_spectacular.utils import extend_schema
 
+from cvat.apps.engine.mixins import UploadMixin
 from cvat.apps.engine.models import CloudStorage as CloudStorageModel
+from cvat.apps.engine.parsers import TusUploadParser
 from cvat.apps.iam.permissions import CloudStoragePermission
 
 
@@ -57,40 +58,6 @@ def make_paginated_response(
 
     return response_type(serializer.data)
 
-def reverse(viewname, *, args=None, kwargs=None,
-    query_params: Optional[Dict[str, str]] = None,
-    request: Optional[HttpRequest] = None,
-) -> str:
-    """
-    The same as rest_framework's reverse(), but adds custom query params support.
-    The original request can be passed in the 'request' parameter to
-    return absolute URLs.
-    """
-
-    url = _reverse(viewname, args, kwargs, request)
-
-    if query_params:
-        return f'{url}?{urlencode(query_params)}'
-
-    return url
-
-def build_field_filter_params(field: str, value: Any) -> Dict[str, str]:
-    """
-    Builds a collection filter query params for a single field and value.
-    """
-    return { field: value }
-
-def get_list_view_name(model):
-    # Implemented after
-    # rest_framework/utils/field_mapping.py.get_detail_view_name()
-    """
-    Given a model class, return the view name to use for URL relationships
-    that refer to instances of the model.
-    """
-    return '%(model_name)s-list' % {
-        'model_name': model._meta.object_name.lower()
-    }
-
 def list_action(serializer_class: Type[Serializer], **kwargs):
     params = dict(
         detail=True,
@@ -124,3 +91,23 @@ def get_cloud_storage_for_import_or_export(
         raise PermissionDenied(error_message)
 
     return get_object_or_404(CloudStorageModel, pk=storage_id)
+
+def tus_chunk_action(*, detail: bool, suffix_base: str):
+    def decorator(f):
+        f = action(detail=detail, methods=['HEAD', 'PATCH'],
+            url_path=f'{suffix_base}/{UploadMixin.file_id_regex}',
+            parser_classes=[TusUploadParser],
+            serializer_class=None,
+        )(f)
+
+        # tus chunk endpoints are never accessed directly (the client must
+        # access them by following the Location header from the response to
+        # the creation endpoint). Moreover, the details of how these endpoints
+        # work are already described by the tus specification. Since we don't
+        # need to document either where these points are or how they work,
+        # they don't need to be in the schema.
+        f = extend_schema(exclude=True)(f)
+
+        return f
+
+    return decorator
diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py
@@ -51,7 +51,6 @@
     CloudProviderChoice, Location
 )
 from cvat.apps.engine.models import CloudStorage as CloudStorageModel
-from cvat.apps.engine.parsers import TusUploadParser
 from cvat.apps.engine.serializers import (
     AboutSerializer, AnnotationFileSerializer, BasicUserSerializer,
     DataMetaReadSerializer, DataMetaWriteSerializer, DataSerializer,
@@ -80,6 +79,7 @@
     TaskPermission, UserPermission)
 from cvat.apps.engine.cache import MediaCache
 from cvat.apps.events.handlers import handle_annotations_patch
+from cvat.apps.engine.view_utils import tus_chunk_action
 
 _UPLOAD_PARSER_CLASSES = api_settings.DEFAULT_PARSER_CLASSES + [MultiPartParser]
 
@@ -362,18 +362,7 @@ def dataset(self, request, pk):
                     callback=dm.views.export_project_as_dataset
                 )
 
-    @extend_schema(methods=['PATCH'],
-        operation_id='projects_partial_update_dataset_file',
-        summary="Allows to upload a file chunk. "
-            "Implements TUS file uploading protocol.",
-        request=OpenApiTypes.BINARY,
-        responses={}
-    )
-    @extend_schema(methods=['HEAD'],
-        summary="Implements TUS file uploading protocol."
-    )
-    @action(detail=True, methods=['HEAD', 'PATCH'], url_path='dataset/'+UploadMixin.file_id_regex,
-        parser_classes=[TusUploadParser])
+    @tus_chunk_action(detail=True, suffix_base="dataset")
     def append_dataset_chunk(self, request, pk, file_id):
         self._object = self.get_object()
         return self.append_tus_chunk(request, file_id)
@@ -510,18 +499,7 @@ def export_backup(self, request, pk=None):
     def import_backup(self, request, pk=None):
         return self.deserialize(request, backup.import_project)
 
-    @extend_schema(methods=['PATCH'],
-        operation_id='projects_partial_update_backup_file',
-        summary="Allows to upload a file chunk. "
-            "Implements TUS file uploading protocol.",
-        request=OpenApiTypes.BINARY,
-        responses={}
-    )
-    @extend_schema(methods=['HEAD'],
-        summary="Implements TUS file uploading protocol."
-    )
-    @action(detail=False, methods=['HEAD', 'PATCH'], url_path='backup/'+UploadMixin.file_id_regex,
-        serializer_class=None, parser_classes=[TusUploadParser])
+    @tus_chunk_action(detail=False, suffix_base="backup")
     def append_backup_chunk(self, request, file_id):
         return self.append_tus_chunk(request, file_id)
 
@@ -744,18 +722,7 @@ def get_queryset(self):
     def import_backup(self, request, pk=None):
         return self.deserialize(request, backup.import_task)
 
-    @extend_schema(methods=['PATCH'],
-        operation_id='tasks_partial_update_backup_file',
-        summary="Allows to upload a file chunk. "
-            "Implements TUS file uploading protocol.",
-        request=OpenApiTypes.BINARY,
-        responses={}
-    )
-    @extend_schema(methods=['HEAD'],
-        summary="Implements TUS file uploading protocol."
-    )
-    @action(detail=False, methods=['HEAD', 'PATCH'], url_path='backup/'+UploadMixin.file_id_regex,
-        parser_classes=[TusUploadParser])
+    @tus_chunk_action(detail=False, suffix_base="backup")
     def append_backup_chunk(self, request, file_id):
         return self.append_tus_chunk(request, file_id)
 
@@ -946,18 +913,7 @@ def data(self, request, pk):
             return data_getter(request, self._object.data.start_frame,
                 self._object.data.stop_frame, self._object.data)
 
-    @extend_schema(methods=['PATCH'],
-        operation_id='tasks_partial_update_data_file',
-        summary="Allows to upload a file chunk. "
-            "Implements TUS file uploading protocol.",
-        request=OpenApiTypes.BINARY,
-        responses={}
-    )
-    @extend_schema(methods=['HEAD'],
-        summary="Implements TUS file uploading protocol."
-    )
-    @action(detail=True, methods=['HEAD', 'PATCH'], url_path='data/'+UploadMixin.file_id_regex,
-        parser_classes=[TusUploadParser])
+    @tus_chunk_action(detail=True, suffix_base="data")
     def append_data_chunk(self, request, pk, file_id):
         self._object = self.get_object()
         return self.append_tus_chunk(request, file_id)
@@ -1107,19 +1063,7 @@ def annotations(self, request, pk):
                     return Response(data=str(e), status=status.HTTP_400_BAD_REQUEST)
                 return Response(data)
 
-    @extend_schema(methods=['PATCH'],
-        operation_id='tasks_partial_update_annotations_file',
-        summary="Allows to upload an annotation file chunk. "
-            "Implements TUS file uploading protocol.",
-        request=OpenApiTypes.BINARY,
-        responses={}
-    )
-    @extend_schema(methods=['HEAD'],
-        operation_id='tasks_annotations_file_retrieve_status',
-        summary="Implements TUS file uploading protocol."
-    )
-    @action(detail=True, methods=['HEAD', 'PATCH'], url_path='annotations/'+UploadMixin.file_id_regex,
-        parser_classes=[TusUploadParser])
+    @tus_chunk_action(detail=True, suffix_base="annotations")
     def append_annotations_chunk(self, request, pk, file_id):
         self._object = self.get_object()
         return self.append_tus_chunk(request, file_id)
@@ -1509,18 +1453,7 @@ def annotations(self, request, pk):
                 return Response(data)
 
 
-    @extend_schema(methods=['PATCH'],
-        operation_id='jobs_partial_update_annotations_file',
-        summary="Allows to upload an annotation file chunk. "
-            "Implements TUS file uploading protocol.",
-        request=OpenApiTypes.BINARY,
-        responses={}
-    )
-    @extend_schema(methods=['HEAD'],
-        summary="Implements TUS file uploading protocol."
-    )
-    @action(detail=True, methods=['HEAD', 'PATCH'], url_path='annotations/'+UploadMixin.file_id_regex,
-        parser_classes=[TusUploadParser])
+    @tus_chunk_action(detail=True, suffix_base="annotations")
     def append_annotations_chunk(self, request, pk, file_id):
         self._object = self.get_object()
         return self.append_tus_chunk(request, file_id)