From 6197c83953db0762f42cd29f853c9b7407770472 Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Thu, 6 Apr 2023 16:28:18 +0300 Subject: [PATCH] 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. --- CHANGELOG.md | 4 + cvat/apps/engine/serializers.py | 2 +- cvat/apps/engine/utils.py | 40 +++++ cvat/apps/engine/view_utils.py | 61 +++----- cvat/apps/engine/views.py | 81 +--------- cvat/schema.yml | 266 -------------------------------- 6 files changed, 76 insertions(+), 378 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 953ea1c3009..578c13ac98c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Cloud storage unique_together limitation () - Support for redundant request media types in the API () +- 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 + () ### Fixed - An invalid project/org handling in webhooks () diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index 179623eba15..7237fe0507a 100644 --- 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 index 8903a93f5eb..4f65af6bd11 100644 --- 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 index 8c767b03b42..457f07b5465 100644 --- 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 index 933a74962b6..686fce20ade 100644 --- 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) diff --git a/cvat/schema.yml b/cvat/schema.yml index 50de2027d7b..84004fe8757 100644 --- a/cvat/schema.yml +++ b/cvat/schema.yml @@ -2238,53 +2238,6 @@ paths: responses: '204': description: The annotation has been deleted - /api/jobs/{id}/annotations/{file_id}: - patch: - operationId: jobs_partial_update_annotations_file - summary: Allows to upload an annotation file chunk. Implements TUS file uploading - protocol. - parameters: - - in: header - name: X-Organization - schema: - type: string - - in: path - name: file_id - schema: - type: string - pattern: ^\b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b$ - required: true - - in: path - name: id - schema: - type: integer - description: A unique integer value identifying this job. - required: true - - in: query - name: org - schema: - type: string - description: Organization unique slug - - in: query - name: org_id - schema: - type: integer - description: Organization identifier - tags: - - jobs - requestBody: - content: - application/offset+octet-stream: - schema: - type: string - format: binary - security: - - sessionAuth: [] - csrfAuth: [] - tokenAuth: [] - - signatureAuth: [] - - basicAuth: [] - responses: {} /api/jobs/{id}/data: get: operationId: jobs_retrieve_data @@ -4026,52 +3979,6 @@ paths: description: Failed to import dataset '405': description: Format is not available - /api/projects/{id}/dataset/{file_id}: - patch: - operationId: projects_partial_update_dataset_file - summary: Allows to upload a file chunk. Implements TUS file uploading protocol. - parameters: - - in: header - name: X-Organization - schema: - type: string - - in: path - name: file_id - schema: - type: string - pattern: ^\b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b$ - required: true - - in: path - name: id - schema: - type: integer - description: A unique integer value identifying this project. - required: true - - in: query - name: org - schema: - type: string - description: Organization unique slug - - in: query - name: org_id - schema: - type: integer - description: Organization identifier - tags: - - projects - requestBody: - content: - application/offset+octet-stream: - schema: - type: string - format: binary - security: - - sessionAuth: [] - csrfAuth: [] - tokenAuth: [] - - signatureAuth: [] - - basicAuth: [] - responses: {} /api/projects/{id}/preview: get: operationId: projects_retrieve_preview @@ -4169,46 +4076,6 @@ paths: description: The project has been imported '202': description: Importing a backup file has been started - /api/projects/backup/{file_id}: - patch: - operationId: projects_partial_update_backup_file - summary: Allows to upload a file chunk. Implements TUS file uploading protocol. - parameters: - - in: header - name: X-Organization - schema: - type: string - - in: path - name: file_id - schema: - type: string - pattern: ^\b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b$ - required: true - - in: query - name: org - schema: - type: string - description: Organization unique slug - - in: query - name: org_id - schema: - type: integer - description: Organization identifier - tags: - - projects - requestBody: - content: - application/offset+octet-stream: - schema: - type: string - format: binary - security: - - sessionAuth: [] - csrfAuth: [] - tokenAuth: [] - - signatureAuth: [] - - basicAuth: [] - responses: {} /api/schema/: get: operationId: schema_retrieve @@ -5113,53 +4980,6 @@ paths: responses: '204': description: The annotation has been deleted - /api/tasks/{id}/annotations/{file_id}: - patch: - operationId: tasks_partial_update_annotations_file - summary: Allows to upload an annotation file chunk. Implements TUS file uploading - protocol. - parameters: - - in: header - name: X-Organization - schema: - type: string - - in: path - name: file_id - schema: - type: string - pattern: ^\b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b$ - required: true - - in: path - name: id - schema: - type: integer - description: A unique integer value identifying this task. - required: true - - in: query - name: org - schema: - type: string - description: Organization unique slug - - in: query - name: org_id - schema: - type: integer - description: Organization identifier - tags: - - tasks - requestBody: - content: - application/offset+octet-stream: - schema: - type: string - format: binary - security: - - sessionAuth: [] - csrfAuth: [] - tokenAuth: [] - - signatureAuth: [] - - basicAuth: [] - responses: {} /api/tasks/{id}/backup: get: operationId: tasks_retrieve_backup @@ -5352,52 +5172,6 @@ paths: responses: '202': description: No response body - /api/tasks/{id}/data/{file_id}: - patch: - operationId: tasks_partial_update_data_file - summary: Allows to upload a file chunk. Implements TUS file uploading protocol. - parameters: - - in: header - name: X-Organization - schema: - type: string - - in: path - name: file_id - schema: - type: string - pattern: ^\b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b$ - required: true - - in: path - name: id - schema: - type: integer - description: A unique integer value identifying this task. - required: true - - in: query - name: org - schema: - type: string - description: Organization unique slug - - in: query - name: org_id - schema: - type: integer - description: Organization identifier - tags: - - tasks - requestBody: - content: - application/offset+octet-stream: - schema: - type: string - format: binary - security: - - sessionAuth: [] - csrfAuth: [] - tokenAuth: [] - - signatureAuth: [] - - basicAuth: [] - responses: {} /api/tasks/{id}/data/meta: get: operationId: tasks_retrieve_data_meta @@ -5713,46 +5487,6 @@ paths: description: The task has been imported '202': description: Importing a backup file has been started - /api/tasks/backup/{file_id}: - patch: - operationId: tasks_partial_update_backup_file - summary: Allows to upload a file chunk. Implements TUS file uploading protocol. - parameters: - - in: header - name: X-Organization - schema: - type: string - - in: path - name: file_id - schema: - type: string - pattern: ^\b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b$ - required: true - - in: query - name: org - schema: - type: string - description: Organization unique slug - - in: query - name: org_id - schema: - type: integer - description: Organization identifier - tags: - - tasks - requestBody: - content: - application/offset+octet-stream: - schema: - type: string - format: binary - security: - - sessionAuth: [] - csrfAuth: [] - tokenAuth: [] - - signatureAuth: [] - - basicAuth: [] - responses: {} /api/users: get: operationId: users_list