From 8740567076f0b342c85d87483b23fb4264330363 Mon Sep 17 00:00:00 2001 From: Abdul-Muqadim-Arbisoft Date: Mon, 14 Sep 2026 17:36:53 +0500 Subject: [PATCH 1/3] feat: standardize Course Videos API into authoring v1 Adds a conforming REST surface for a course's video assets at /api/authoring/v1/courses/{course_key}/videos/ and .../{edx_video_id}/, as one ViewSet with list, retrieve, create and destroy. The legacy GET at /api/contentstore/v0/videos/uploads/{course_id}/{edx_video_id} declares a video id and ignores it, returning the whole course listing. The new version splits that in two: the collection endpoint succeeds the behaviour the legacy route really has, and the member endpoint finally honours the id. Three defects are fixed on the new addresses only, because fixing them in place would break anyone parsing the current responses: - The legacy GET and HEAD write. They flip every video stuck in 'upload' for more than 24 hours to 'upload_failed'. The new read path computes the same display status without the write; the legacy route still reconciles. - Three incompatible error shapes, including a 400 with an empty body whose payload rides in the HTTP reason phrase, become one error envelope. - A non-JSON Accept header returns a redirect to the authoring MFE, or a 500 when COURSE_AUTHORING_MICROFRONTEND_URL is unset, which is the platform default. The new addresses are JSON only. Business logic is reused by importing the existing video_storage_handlers and edxval.api functions; none of it is duplicated or edited. Authentication drops to the platform default, which refuses inactive users on the new addresses while the legacy addresses are unchanged. Addresses under a course that no endpoint serves are answered with the error envelope rather than the site's HTML error page, so a client can read them. Part of #39060. --- .../rest_api/v1/authoring_urls.py | 47 + .../contentstore/rest_api/v1/error_types.py | 30 + .../rest_api/v1/serializers/video_uploads.py | 204 ++ .../rest_api/v1/video_uploads_service.py | 292 +++ .../v1/views/tests/test_video_uploads.py | 1738 +++++++++++++++++ .../rest_api/v1/views/unknown_route.py | 26 + .../rest_api/v1/views/video_uploads.py | 399 ++++ cms/urls.py | 7 + 8 files changed, 2743 insertions(+) create mode 100644 cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py create mode 100644 cms/djangoapps/contentstore/rest_api/v1/error_types.py create mode 100644 cms/djangoapps/contentstore/rest_api/v1/serializers/video_uploads.py create mode 100644 cms/djangoapps/contentstore/rest_api/v1/video_uploads_service.py create mode 100644 cms/djangoapps/contentstore/rest_api/v1/views/tests/test_video_uploads.py create mode 100644 cms/djangoapps/contentstore/rest_api/v1/views/unknown_route.py create mode 100644 cms/djangoapps/contentstore/rest_api/v1/views/video_uploads.py diff --git a/cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py b/cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py new file mode 100644 index 000000000000..a3cb45753f7c --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py @@ -0,0 +1,47 @@ +"""Authoring API v1 URLs.""" + +from django.urls import path + +from cms.djangoapps.contentstore.rest_api.v1.error_types import register_request_error_types +from cms.djangoapps.contentstore.rest_api.v1.views.unknown_route import UnknownRouteView +from cms.djangoapps.contentstore.rest_api.v1.views.video_uploads import CourseVideoUploadsViewSet + +app_name = "authoring_v1" + +register_request_error_types() + +urlpatterns = [ + path( + "courses//videos/", + CourseVideoUploadsViewSet.as_view({"get": "list", "post": "create"}), + name="course_video_list", + ), + path( + "courses//videos//", + CourseVideoUploadsViewSet.as_view({"get": "retrieve", "delete": "destroy"}), + name="course_video_detail", + ), + # The three routes below stand behind the two above and are reached only by a + # request the ones above turned down: a course key the converter refuses + # because it is malformed or in the deprecated slash-separated form, or an + # address under a course that this API does not serve. They answer with the + # API error body, which a client can read, instead of the site's HTML error + # page. Order is what keeps them out of the way of real requests, and each + # accepts only addresses ending in a slash so that the redirect to the + # slash-terminated form keeps working. + path( + "courses//videos/", + UnknownRouteView.as_view(), + name="course_video_list_unmatched", + ), + path( + "courses//videos//", + UnknownRouteView.as_view(), + name="course_video_detail_unmatched", + ), + path( + "courses//", + UnknownRouteView.as_view(), + name="course_unmatched", + ), +] diff --git a/cms/djangoapps/contentstore/rest_api/v1/error_types.py b/cms/djangoapps/contentstore/rest_api/v1/error_types.py new file mode 100644 index 000000000000..91e5d0b24d39 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/error_types.py @@ -0,0 +1,30 @@ +"""Error-type catalog entries for request-level refusals.""" + +from edx_rest_framework_extensions.errors import register_error_type +from rest_framework.exceptions import ( + MethodNotAllowed, + NotAcceptable, + ParseError, + UnsupportedMediaType, +) + +#: Refusals of the request itself - its body, its method, its media type - with +#: the error type and title each is answered with. +_REQUEST_ERROR_TYPES = ( + (ParseError, "validation", "Malformed Request"), + (MethodNotAllowed, "method-not-allowed", "Method Not Allowed"), + (NotAcceptable, "not-acceptable", "Not Acceptable"), + (UnsupportedMediaType, "unsupported-media-type", "Unsupported Media Type"), +) + + +def register_request_error_types(): + """ + Give each request-level refusal its own error type. + + Uncataloged errors are answered with the type and title of an internal + server error while keeping their 4xx status, which tells a client that its + own unparseable body or unsupported method was a fault of the server. + """ + for exception_class, slug, title in _REQUEST_ERROR_TYPES: + register_error_type(exception_class, slug, title) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/video_uploads.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/video_uploads.py new file mode 100644 index 000000000000..81f6fbd72348 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/video_uploads.py @@ -0,0 +1,204 @@ +"""API Serializers for course video uploads.""" + +from edx_rest_framework_extensions.shaping import MinimalViewMixin +from rest_framework import serializers +from rest_framework.settings import api_settings + +from cms.djangoapps.contentstore.rest_api.serializers.common import StrictSerializer +from cms.djangoapps.contentstore.rest_api.v1.video_uploads_service import VIDEO_ORDERING_CHOICES +from cms.djangoapps.contentstore.video_storage_handlers import VIDEO_SUPPORTED_FILE_FORMATS + +SUPPORTED_UPLOAD_CONTENT_TYPES = sorted(set(VIDEO_SUPPORTED_FILE_FORMATS.values())) + +NON_FIELD_ERRORS_KEY = api_settings.NON_FIELD_ERRORS_KEY + +#: The fields one video is reduced to under the minimal response preset. +MINIMAL_VIDEO_FIELDS = ("edx_video_id", "client_video_id", "status", "created", "duration") + + +def _flat_messages(detail, path=""): + """ + Return validation ``detail`` as a map of field path to messages. + + Errors reported for the entries of a list of objects are nested one or two + levels below the field they belong to. Flattening them to + ``{"files[0].content_type": ["..."]}`` gives a client the message of every + invalid field without walking the containers it is reported inside. + """ + if isinstance(detail, dict): + flat = {} + for field, value in detail.items(): + nested = path if field == NON_FIELD_ERRORS_KEY and path else _join(path, field) + flat.update(_flat_messages(value, nested)) + return flat + if isinstance(detail, list) and any(isinstance(entry, (dict, list)) for entry in detail): + flat = {} + for index, entry in enumerate(detail): + if entry: + flat.update(_flat_messages(entry, f"{path}[{index}]")) + return flat + messages = detail if isinstance(detail, list) else [detail] + return {path or NON_FIELD_ERRORS_KEY: [str(message) for message in messages]} + + +def _join(path, field): + """Return the path of ``field`` inside ``path``.""" + return f"{path}.{field}" if path else str(field) + + +class VideoUploadFileSerializer(StrictSerializer): + """One requested upload slot: the file the caller intends to PUT.""" + + file_name = serializers.CharField( + help_text="Name of the video file being uploaded. ASCII characters only.", + ) + content_type = serializers.ChoiceField( + choices=SUPPORTED_UPLOAD_CONTENT_TYPES, + help_text="MIME type of the video file. Only these types can be stored.", + ) + + def validate_file_name(self, value): + """Reject names the storage backend cannot carry in its object metadata.""" + try: + value.encode("ascii") + except UnicodeEncodeError as error: + raise serializers.ValidationError( + "The file name must contain only ASCII characters." + ) from error + return value + + +class VideoUploadRequestSerializer(StrictSerializer): + """Request body for creating upload slots.""" + + files = VideoUploadFileSerializer( + many=True, + allow_empty=False, + help_text="Files to create upload slots for. One slot is returned per entry, in order.", + ) + + def run_validation(self, data=serializers.empty): + """Validate the body, reporting every invalid field as a list of messages.""" + try: + return super().run_validation(data) + except serializers.ValidationError as error: + raise serializers.ValidationError(_flat_messages(error.detail)) from error + + +class VideoUploadLinkSerializer(serializers.Serializer): # pylint: disable=abstract-method + """One created upload slot.""" + + file_name = serializers.CharField( + help_text="Name of the video file, echoed from the request.", + ) + upload_url = serializers.CharField( + help_text="Short-lived pre-signed URL to PUT the video file to. Expires after 24 hours.", + ) + edx_video_id = serializers.CharField( + help_text="Identifier assigned to the video. Use it to address the video afterwards.", + ) + + +class VideoUploadResponseSerializer(serializers.Serializer): # pylint: disable=abstract-method + """Response body for creating upload slots.""" + + files = VideoUploadLinkSerializer( + many=True, + help_text="Created upload slots, one per requested file, in request order.", + ) + + +class CourseVideoSerializer(serializers.Serializer): # pylint: disable=abstract-method + """One video asset attached to a course.""" + + edx_video_id = serializers.CharField( + help_text="Identifier of the video.", + ) + client_video_id = serializers.CharField( + allow_blank=True, + help_text="Original file name the video was uploaded under.", + ) + created = serializers.DateTimeField( + allow_null=True, + help_text="Time the video record was created.", + ) + duration = serializers.FloatField( + allow_null=True, + help_text="Length of the video in seconds; 0 until the video has been processed.", + ) + status = serializers.CharField( + help_text=( + "Processing state of the video, in stable English: for example Uploading, " + "In Progress, Ready, Failed, YouTube Duplicate." + ), + ) + error_description = serializers.CharField( + allow_null=True, + allow_blank=True, + help_text="Details of the processing failure, when the video failed to process.", + ) + course_video_image_url = serializers.CharField( + allow_null=True, + help_text="Thumbnail image URL for this video in this course, or null when none is set.", + ) + download_link = serializers.CharField( + allow_blank=True, + help_text="URL of the desktop MP4 encoding, or an empty string when it is not ready.", + ) + file_size = serializers.IntegerField( + help_text="Size of the desktop MP4 encoding in bytes; 0 when it is not ready.", + ) + transcripts = serializers.ListField( + child=serializers.CharField(), + help_text="Language codes the video has transcripts for.", + ) + transcription_status = serializers.CharField( + allow_blank=True, + help_text=( + "Transcription state for courses on the current video workflow, in stable English; " + "empty for courses still using a course video upload token." + ), + ) + transcript_urls = serializers.DictField( + child=serializers.CharField(), + help_text="Transcript download URL keyed by language code.", + ) + + +class CourseVideoMinimalSerializer(CourseVideoSerializer): + """One video asset, reduced to the fields that identify it.""" + + def get_fields(self): + """Return only the fields the minimal preset keeps.""" + fields = super().get_fields() + return {name: field for name, field in fields.items() if name in MINIMAL_VIDEO_FIELDS} + + +class CourseVideoQuerySerializer(serializers.Serializer): # pylint: disable=abstract-method + """Query parameters accepted by every course video address.""" + + view = serializers.ChoiceField( + choices=[MinimalViewMixin.minimal_view_value], + required=False, + help_text="Response preset to return. Omit it for the full representation.", + ) + + +class CourseVideoListQuerySerializer(CourseVideoQuerySerializer): + """Query parameters accepted by the course video collection.""" + + ordering = serializers.ChoiceField( + choices=VIDEO_ORDERING_CHOICES, + required=False, + help_text="Field to sort by. Prefix the field name with '-' to sort descending.", + ) + page = serializers.IntegerField( + required=False, + min_value=1, + help_text="Number of the page to return.", + ) + page_size = serializers.IntegerField( + required=False, + min_value=1, + help_text="Number of videos per page; values above the maximum are reduced to it.", + ) diff --git a/cms/djangoapps/contentstore/rest_api/v1/video_uploads_service.py b/cms/djangoapps/contentstore/rest_api/v1/video_uploads_service.py new file mode 100644 index 000000000000..7001a435cdef --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/video_uploads_service.py @@ -0,0 +1,292 @@ +"""Service layer for the course video uploads resource.""" + +import logging +from datetime import datetime, timedelta + +from django.core.exceptions import ObjectDoesNotExist +from django.core.exceptions import PermissionDenied as DjangoPermissionDenied +from django.http import Http404 +from edx_rest_framework_extensions.errors import register_error_type +from edxval.api import ( + SortDirection, + VideoSortField, + get_available_transcript_languages, + get_course_videos_qset, + get_video_info, + get_video_transcript_url, + get_videos_for_course, + remove_video_for_course, +) +from edxval.exceptions import ValVideoNotFoundError +from pytz import UTC +from rest_framework.exceptions import ( + APIException, + NotFound, + PermissionDenied, + ValidationError, +) +from rest_framework.status import ( + HTTP_200_OK, + HTTP_400_BAD_REQUEST, + HTTP_403_FORBIDDEN, + HTTP_404_NOT_FOUND, +) + +from cms.djangoapps.contentstore.toggles import use_mock_video_uploads +from cms.djangoapps.contentstore.video_storage_handlers import ( + MAX_UPLOAD_HOURS, + StatusDisplayStrings, + _get_and_validate_course, + videos_post, +) + +log = logging.getLogger(__name__) + +#: Error type of a course whose video uploads are switched off or unconfigured. +VIDEO_UPLOADS_NOT_CONFIGURED_SLUG = "videos/uploads-not-configured" + +#: Sort fields the video store can order by, ascending and descending. +VIDEO_ORDERING_CHOICES = tuple( + sorted( + [field.value for field in VideoSortField] + + [f"-{field.value}" for field in VideoSortField] + ) +) +DEFAULT_ORDERING = "-created" + +#: VAL statuses that mean transcription has started, which only happens once +#: every encoding is complete on the current video workflow. +_TRANSCRIPTION_STATUSES = frozenset({ + "transcription_in_progress", + "transcript_ready", + "partial_failure", + "transcript_failed", +}) + + +class VideoUploadsNotConfigured(NotFound): + """The course cannot serve video uploads because the upload pipeline is off or unconfigured.""" + + default_detail = "Video uploads are not configured for this course." + default_code = "video_uploads_not_configured" + + +register_error_type( + VideoUploadsNotConfigured, + VIDEO_UPLOADS_NOT_CONFIGURED_SLUG, + "Video Uploads Not Configured", +) + + +class _JsonBody: + """ + Carrier for a parsed JSON body. + + ``videos_post`` reads the request's already-parsed JSON and nothing else, so + it is handed this instead of the live request, which keeps the request + object out of the storage layer. + """ + + def __init__(self, json_body): + self.json = json_body + + +def get_course_for_uploads(course_key, user): + """ + Return the course a video upload operation applies to. + + Enforces the caller's Studio access to the course and the course's upload + pipeline configuration; a course the caller may not read is refused, and a + course that does not accept uploads is reported as missing. Returns + ``None`` only when uploads are mocked for local development, in which case + no real course is needed. + """ + try: + course = _get_and_validate_course(str(course_key), user) + except DjangoPermissionDenied as error: + log.info("Course [%s] is not readable in Studio by user [%s]", course_key, user.id) + raise PermissionDenied() from error + except Http404 as error: + log.info("No course [%s] to serve video uploads for", course_key) + raise NotFound() from error + if course is None and not use_mock_video_uploads(): + raise VideoUploadsNotConfigured() + return course + + +def _require_course(course): + """Return ``course``, or refuse a read that has no course behind it.""" + if course is None: + raise VideoUploadsNotConfigured() + return course + + +def _parse_ordering(ordering): + """Split an ordering value into the sort field and direction the video store expects.""" + ordering = ordering or DEFAULT_ORDERING + descending = ordering.startswith("-") + return ( + VideoSortField(ordering.lstrip("-")), + SortDirection.desc if descending else SortDirection.asc, + ) + + +def list_course_videos(course, ordering=DEFAULT_ORDERING): + """ + Return every non-hidden video attached to ``course``, in the requested order. + + Rows are returned unenriched: the per-video transcript lookups happen in + :func:`enrich_course_videos`, so a caller that only shows one page pays for + one page. + """ + sort_field, sort_direction = _parse_ordering(ordering) + videos, __ = get_videos_for_course(str(_require_course(course).id), sort_field, sort_direction) + return list(videos) + + +def get_course_video(course, edx_video_id): + """Return the one video attached to ``course`` under ``edx_video_id``, unenriched.""" + course = _require_course(course) + attached = get_course_videos_qset(course.id).filter(video__edx_video_id=edx_video_id).exists() + if not attached: + raise NotFound() + try: + return get_video_info(edx_video_id) + except ValVideoNotFoundError as error: + log.info("No video record for edx_video_id [%s] in course [%s]", edx_video_id, course.id) + raise NotFound() from error + + +def enrich_course_videos(course, videos): + """ + Return the API representation of ``videos`` for ``course``. + + Each row gains its display status, its course-specific thumbnail, its + downloadable encoding and its transcript languages and URLs. + """ + course = _require_course(course) + course_id = str(course.id) + upload_token = course.video_upload_pipeline.get("course_video_upload_token") + return [_enrich_video(video, course_id, upload_token) for video in videos] + + +def _enrich_video(video, course_id, upload_token): + """Return the API representation of one video row.""" + encodes_ready = not upload_token and video["status"] in _TRANSCRIPTION_STATUSES + status = _display_status(video, encodes_ready) + transcripts = get_available_transcript_languages(video_id=video["edx_video_id"]) + download_link, file_size = _desktop_encoding(video) + return { + "edx_video_id": video["edx_video_id"], + "client_video_id": video["client_video_id"], + "created": video["created"], + "duration": video["duration"], + "status": status, + "error_description": video["error_description"], + "course_video_image_url": _course_video_image_url(video, course_id), + "download_link": download_link, + "file_size": file_size, + "transcripts": transcripts, + "transcription_status": _transcription_status(video, encodes_ready), + "transcript_urls": { + language_code: get_video_transcript_url( + video_id=video["edx_video_id"], + language_code=language_code, + ) + for language_code in transcripts + }, + } + + +def _transcription_status(video, encodes_ready): + """ + Return how far transcription has got for one video. + + Only courses on the current video workflow report it: elsewhere the stored + status never reaches a transcription state. It names the transcription + state itself, which the display status no longer shows once the encodes are + complete. + """ + if not encodes_ready: + return "" + return StatusDisplayStrings.get(video["status"]) + + +def _display_status(video, encodes_ready): + """ + Return the display status of one video. + + A video left in ``upload`` for longer than the upload window is reported as + failed. Reporting it does not reconcile the stored record. + """ + created = video.get("created") + now = datetime.now(created.tzinfo if created else UTC) + if video["status"] == "upload" and created and (now - created) > timedelta(hours=MAX_UPLOAD_HOURS): + return StatusDisplayStrings.get("upload_failed") + if video["status"] == "invalid_token": + return StatusDisplayStrings.get("youtube_duplicate") + if encodes_ready: + return StatusDisplayStrings.get("file_complete") + return StatusDisplayStrings.get(video["status"]) + + +def _course_video_image_url(video, course_id): + """Return the thumbnail URL recorded for this video in this course, or None.""" + for course in video["courses"]: + if course_id in course: + return course[course_id] + return None + + +def _desktop_encoding(video): + """Return the desktop MP4 download URL and file size, or empty values when absent.""" + for encoding in video["encoded_videos"]: + if encoding["profile"] == "desktop_mp4": + return encoding["url"], encoding["file_size"] + return "", 0 + + +#: The message every refused upload request is answered with. The video store's +#: own wording can name internal storage detail, so it is logged instead. +_UPLOAD_REFUSED = "The requested files cannot be uploaded." + +#: How a refusal from the video store is reported, by the status it refused with. +_UPLOAD_REFUSAL_ERRORS = { + HTTP_403_FORBIDDEN: PermissionDenied, + HTTP_404_NOT_FOUND: NotFound, +} + + +def _upload_refusal(status_code): + """Return the error answering a refusal the video store reported with ``status_code``.""" + if status_code == HTTP_400_BAD_REQUEST: + return ValidationError({"files": [_UPLOAD_REFUSED]}) + return _UPLOAD_REFUSAL_ERRORS.get(status_code, APIException)(_UPLOAD_REFUSED) + + +def create_video_uploads(course, files): + """ + Create one upload slot per entry in ``files`` and return them in request order. + + Each slot carries a newly assigned video identifier and a short-lived + pre-signed URL the caller PUTs the file to. + """ + data, status_code = videos_post(course, _JsonBody({"files": files})) + if status_code == HTTP_200_OK: + return data + log.warning( + "Video upload slots refused with status [%s] for course [%s]: %s", + status_code, + getattr(course, "id", None), + data.get("error"), + ) + raise _upload_refusal(status_code) + + +def delete_course_video(course_key, edx_video_id): + """Detach ``edx_video_id`` from the course; the video itself is kept for other courses.""" + try: + remove_video_for_course(str(course_key), edx_video_id) + except ObjectDoesNotExist as error: + log.info("No video [%s] attached to course [%s] to remove", edx_video_id, course_key) + raise NotFound() from error diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_video_uploads.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_video_uploads.py new file mode 100644 index 000000000000..89e654226fbe --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_video_uploads.py @@ -0,0 +1,1738 @@ +""" +Unit tests for the course video uploads API. +""" +import json +import os +import subprocess +import sys +import tempfile +from datetime import datetime, timedelta +from pathlib import Path +from unittest.mock import Mock, patch + +import ddt +import pytest +import pytz +from django.core.exceptions import PermissionDenied as DjangoPermissionDenied +from django.http import Http404 +from django.test import TestCase, override_settings +from django.urls import resolve, reverse +from drf_spectacular.generators import SchemaGenerator +from drf_spectacular.settings import patched_settings +from edx_django_utils.cache import TieredCache +from edx_rest_framework_extensions.auth.jwt.tests.utils import generate_jwt +from edx_rest_framework_extensions.testing import assert_error_envelope +from edxval.api import create_profile, create_video +from edxval.models import CourseVideo, EncodedVideo, Video +from opaque_keys.edx.keys import CourseKey +from rest_framework import status +from rest_framework.test import APIClient + +import cms.envs +from cms.djangoapps.contentstore.rest_api.v0.views.authoring_videos import ( + VideosCreateUploadView, + VideosUploadsView, +) +from cms.djangoapps.contentstore.rest_api.v1.serializers.video_uploads import CourseVideoSerializer +from cms.djangoapps.contentstore.rest_api.v1.views.unknown_route import UnknownRouteView +from cms.djangoapps.contentstore.rest_api.v1.views.video_uploads import CourseVideoUploadsViewSet +from cms.djangoapps.contentstore.tests.utils import CourseTestCase +from cms.lib.spectacular import cms_api_filter, cms_mark_superseded_paths +from common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseLimitedStaffRole, + CourseStaffRole, +) +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.video_pipeline.models import VideoUploadsEnabledByDefault +from xmodule.modulestore.tests.factories import CourseFactory + +LIST_URL_NAME = "authoring_v1:course_video_list" +DETAIL_URL_NAME = "authoring_v1:course_video_detail" +LEGACY_LIST_URL_NAME = "cms.djangoapps.contentstore:v0:cms_api_create_videos_upload" +LEGACY_DETAIL_URL_NAME = "cms.djangoapps.contentstore:v0:cms_api_videos_uploads" + +ENUM_POSTPROCESSING_HOOK = "drf_spectacular.hooks.postprocess_schema_enums" +SUPERSEDED_PATHS_HOOK = "cms.lib.spectacular.cms_mark_superseded_paths" + +# The post-processing hooks the CMS registers. Registering any hook replaces +# drf-spectacular's own list, so the enum hook has to be named again alongside. +REGISTERED_POSTPROCESSING_HOOKS = [ENUM_POSTPROCESSING_HOOK, SUPERSEDED_PATHS_HOOK] + +PRODUCTION_SETTINGS = "cms.envs.production" +DEVSTACK_SETTINGS = "cms.envs.devstack" +SETTINGS_DIRECTORY = Path(cms.envs.__file__).resolve().parent +REPO_ROOT = SETTINGS_DIRECTORY.parents[1] +MOCK_CONFIG = SETTINGS_DIRECTORY / "mock.yml" + +# The schema settings that decide the addresses the document publishes, and the +# subset of them a document generated in-process has to be given. +SCHEMA_SETTING_KEYS = ( + "PREPROCESSING_HOOKS", + "POSTPROCESSING_HOOKS", + "SCHEMA_PATH_PREFIX", + "SCHEMA_PATH_PREFIX_TRIM", + "SERVERS", +) +GENERATION_SETTING_KEYS = SCHEMA_SETTING_KEYS[:-1] +SETTINGS_MARKER = "schema settings: " + + +def schema_settings(module, config_file=MOCK_CONFIG): + """ + Return the schema settings of the deployment settings module ``module``. + + Deployment settings share their mutable defaults with the settings the test + process runs under and would alter them on import, so they are read in a + separate process. + """ + script = ( + "import importlib, json, sys\n" + "values = importlib.import_module(sys.argv[1]).SPECTACULAR_SETTINGS\n" + "print(sys.argv[2] + json.dumps({key: values[key] for key in sys.argv[3:]}))\n" + ) + completed = subprocess.run( + [sys.executable, "-c", script, module, SETTINGS_MARKER, *SCHEMA_SETTING_KEYS], + capture_output=True, + check=True, + cwd=REPO_ROOT, + env={**os.environ, "CMS_CFG": str(config_file), "SERVICE_VARIANT": "cms"}, + text=True, + ) + reported = next( + line for line in completed.stdout.splitlines() if line.startswith(SETTINGS_MARKER) + ) + return json.loads(reported[len(SETTINGS_MARKER):]) + + +VIDEO_UPLOAD_PIPELINE = { + "BUCKET": "test_bucket", + "ROOT_PATH": "test_root", + "CONCURRENT_UPLOAD_LIMIT": 4, + "VEM_S3_BUCKET": "vem_test_bucket", +} + +# Every way one video row of this version may differ from the same row of the +# legacy listing, as the JSON path it shows up at and why it is accepted. The +# row parity tests suppress exactly these paths and assert that each of them +# genuinely occurs, so a row that diverges anywhere else, or that stops +# diverging here, fails. Differences that belong to a whole response rather +# than to a row - the pagination envelope, the created status code, the error +# bodies, the Accept-header redirect and the accepted course key forms - are +# each proven by their own test. +ROW_DIFFERENCES = [ + ( + "status_nontranslated", + "the untranslated companion field is gone; 'status' carries that value", + ), + ( + "created", + "timestamps keep the microsecond precision the video store records, which the legacy " + "encoder truncated to milliseconds", + ), +] + +# Fields that legitimately differ per request and are ignored when diffing. +VOLATILE_FIELDS = {"instance", "next", "previous"} + + +def _normalize(obj, path=""): + """Flatten ``obj`` to a {json_path: value} map, dropping volatile fields.""" + flat = {} + if isinstance(obj, dict): + for key in sorted(obj): + if key in VOLATILE_FIELDS: + continue + flat.update(_normalize(obj[key], f"{path}.{key}" if path else key)) + elif isinstance(obj, list): + for index, item in enumerate(obj): + flat.update(_normalize(item, f"{path}[{index}]")) + else: + flat[path] = obj + return flat + + +def _matches(json_path, declared): + return json_path == declared or json_path.startswith(declared + ".") or json_path.startswith(declared + "[") + + +def _diff(legacy, new, declared=()): + """Return (changed paths, paths not covered by a declared difference).""" + left, right = _normalize(legacy), _normalize(new) + changed = {key for key in set(left) | set(right) if left.get(key, "") != right.get(key, "")} + unexpected = {key for key in changed if not any(_matches(key, path) for path in declared)} + return changed, unexpected + + +@override_settings(ENABLE_VIDEO_UPLOAD_PIPELINE=True, VIDEO_UPLOAD_PIPELINE=VIDEO_UPLOAD_PIPELINE) +class CourseVideoUploadsTestBase(CourseTestCase): + """Fixtures shared by every course video uploads test.""" + + def setUp(self): + super().setUp() + self.course.video_upload_pipeline = {"course_video_upload_token": "test_token"} + self.save_course() + + self.api_client = APIClient() + self.api_client.force_authenticate(user=self.user) + + self.list_url = reverse(LIST_URL_NAME, kwargs={"course_key": self.course.id}) + # Microsecond-bearing, as every stored creation time is, so that the + # precision the two versions publish is compared rather than assumed. + self.created = datetime(2026, 1, 2, 3, 4, 5, 123456, tzinfo=pytz.utc) + + def detail_url(self, edx_video_id, course_key=None): + """Return the member address of one video.""" + return reverse( + DETAIL_URL_NAME, + kwargs={"course_key": course_key or self.course.id, "edx_video_id": edx_video_id}, + ) + + def create_videos(self, count=2, status_value="file_complete", course=None): + """Create ``count`` videos attached to the course and return their ids.""" + course = course or self.course + video_ids = [] + for index in range(count): + edx_video_id = f"video-{status_value}-{index}" + create_video({ + "edx_video_id": edx_video_id, + "client_video_id": f"{edx_video_id}.mp4", + "duration": 42.0 + index, + "status": status_value, + "courses": [str(course.id)], + "encoded_videos": [], + }) + video_ids.append(edx_video_id) + self.set_created(video_ids) + return video_ids + + def set_created(self, video_ids, base=None, age=None): + """ + Pin the creation time of ``video_ids``. + + ``created`` is assigned by the database on insert, so it is rewritten + here to make ordering deterministic and, with ``age``, to age a video + past the upload window. + """ + base = base or self.created + for offset, edx_video_id in enumerate(video_ids): + created = base + timedelta(seconds=offset) + if age: + created = datetime.now(pytz.utc) - age + Video.objects.filter(edx_video_id=edx_video_id).update(created=created) + + def create_course_author(self, role_class, course=None): + """Create a user holding ``role_class`` on the course and return them.""" + user = UserFactory.create(is_staff=False) + role_class((course or self.course).id).add_users(user) + return user + + def client_for(self, user): + """Return an API client authenticated as ``user``.""" + client = APIClient() + if user is not None: + client.force_authenticate(user=user) + return client + + def legacy_detail_url(self, edx_video_id, course=None): + """Return the legacy v0 member address.""" + return reverse( + LEGACY_DETAIL_URL_NAME, + kwargs={"course_id": str((course or self.course).id), "edx_video_id": edx_video_id}, + ) + + def legacy_list_url(self, course=None): + """Return the legacy v0 create address.""" + return reverse(LEGACY_LIST_URL_NAME, kwargs={"course_id": str((course or self.course).id)}) + + +class CurrentWorkflowTestBase(CourseVideoUploadsTestBase): + """ + Fixtures for a course on the current video workflow. + + Such a course holds no course video upload token; video uploads are enabled + for the whole platform instead. This is the only configuration in which a + video reaches a transcription status, so it is the only one in which the + encodes-ready branch of the display status and the transcription status are + reachable at all. + """ + + def setUp(self): + super().setUp() + self.course.video_upload_pipeline = {} + self.save_course() + VideoUploadsEnabledByDefault.objects.create(enabled=True, enabled_for_all_courses=True) + self.addCleanup(TieredCache.delete_all_tiers, VideoUploadsEnabledByDefault.cache_key_name()) + + +def _mock_bucket(mock_boto3_resource, upload_url="http://example.com/put_video"): + """Point ``boto3.resource`` at a bucket whose pre-signed URLs are predictable.""" + mock_s3_client = Mock() + mock_s3_client.generate_presigned_url.return_value = upload_url + mock_bucket = Mock() + mock_bucket.name = "vem_test_bucket" + mock_bucket.meta.client = mock_s3_client + mock_resource = Mock() + mock_resource.Bucket.return_value = mock_bucket + mock_boto3_resource.return_value = mock_resource + return mock_s3_client + + +@ddt.ddt +class CourseVideoListTest(CourseVideoUploadsTestBase): + """Tests for listing a course's videos.""" + + def test_list_returns_pagination_envelope(self): + self.create_videos(count=2) + response = self.api_client.get(self.list_url) + assert response.status_code == status.HTTP_200_OK + assert set(response.data) == { + "count", "num_pages", "current_page", "start", "next", "previous", "results", + } + assert response.data["count"] == 2 + + def test_list_row_shape(self): + self.create_videos(count=1) + response = self.api_client.get(self.list_url) + assert set(response.data["results"][0]) == { + "edx_video_id", + "client_video_id", + "created", + "duration", + "status", + "error_description", + "course_video_image_url", + "download_link", + "file_size", + "transcripts", + "transcription_status", + "transcript_urls", + } + + def test_list_excludes_other_courses(self): + other_course = CourseFactory.create() + other_course.video_upload_pipeline = {"course_video_upload_token": "test_token"} + other_course.save() + self.store.update_item(other_course, self.user.id) + self.create_videos(count=1, status_value="elsewhere", course=other_course) + self.create_videos(count=1) + + response = self.api_client.get(self.list_url) + assert [row["edx_video_id"] for row in response.data["results"]] == ["video-file_complete-0"] + + def test_list_excludes_removed_videos(self): + video_ids = self.create_videos(count=2) + CourseVideo.objects.filter( + course_id=str(self.course.id), video__edx_video_id=video_ids[0] + ).update(is_hidden=True) + + response = self.api_client.get(self.list_url) + assert [row["edx_video_id"] for row in response.data["results"]] == [video_ids[1]] + + def test_list_defaults_to_newest_first(self): + video_ids = self.create_videos(count=3) + response = self.api_client.get(self.list_url) + assert [row["edx_video_id"] for row in response.data["results"]] == list(reversed(video_ids)) + + def test_list_honours_ordering(self): + video_ids = self.create_videos(count=3) + response = self.api_client.get(self.list_url, {"ordering": "created"}) + assert [row["edx_video_id"] for row in response.data["results"]] == video_ids + + def test_list_rejects_unknown_ordering(self): + response = self.api_client.get(self.list_url, {"ordering": "status"}) + assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert "ordering" in response.data["errors"] + + def test_list_reduces_an_oversized_page_size_to_the_maximum(self): + self.create_videos(count=101) + response = self.api_client.get(self.list_url, {"page_size": 500}) + assert response.status_code == status.HTTP_200_OK + assert len(response.data["results"]) == 100 + assert response.data["count"] == 101 + assert response.data["num_pages"] == 2 + + def test_list_accepts_the_maximum_page_size(self): + self.create_videos(count=11) + response = self.api_client.get(self.list_url, {"page_size": 100}) + assert response.status_code == status.HTTP_200_OK + assert len(response.data["results"]) == 11 + assert response.data["num_pages"] == 1 + + def test_list_returns_ten_videos_a_page_by_default(self): + self.create_videos(count=11) + response = self.api_client.get(self.list_url) + assert response.status_code == status.HTTP_200_OK + assert len(response.data["results"]) == 10 + assert response.data["count"] == 11 + assert response.data["num_pages"] == 2 + + @ddt.data("abc", "0", "-1") + def test_list_rejects_an_unusable_page(self, page): + response = self.api_client.get(self.list_url, {"page": page}) + assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert "page" in response.data["errors"] + + @ddt.data("0", "-1", "abc") + def test_list_rejects_an_unusable_page_size(self, page_size): + response = self.api_client.get(self.list_url, {"page_size": page_size}) + assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert "page_size" in response.data["errors"] + + @ddt.data("full", "miniml", "MINIMAL") + def test_list_rejects_an_unknown_view(self, view): + response = self.api_client.get(self.list_url, {"view": view}) + assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert "view" in response.data["errors"] + + def test_list_paginates(self): + self.create_videos(count=3) + response = self.api_client.get(self.list_url, {"page": 2, "page_size": 1}) + assert response.status_code == status.HTTP_200_OK + assert response.data["count"] == 3 + assert response.data["num_pages"] == 3 + assert response.data["current_page"] == 2 + assert response.data["start"] == 1 + assert len(response.data["results"]) == 1 + + def test_list_minimal_view_is_opt_in(self): + self.create_videos(count=1) + full = self.api_client.get(self.list_url) + minimal = self.api_client.get(self.list_url, {"view": "minimal"}) + assert set(minimal.data["results"][0]) == { + "edx_video_id", "client_video_id", "status", "created", "duration", + } + assert set(minimal.data["results"][0]) < set(full.data["results"][0]) + + def test_the_serializer_is_built_with_the_request_context(self): + captured = {} + + class ContextCapturingSerializer(CourseVideoSerializer): + """Record the context the view builds its serializers with.""" + + def __init__(self, *args, **kwargs): + captured.update(kwargs.get("context") or {}) + super().__init__(*args, **kwargs) + + self.create_videos(count=1) + with patch.object( + CourseVideoUploadsViewSet, "serializer_class", ContextCapturingSerializer + ): + response = self.api_client.get(self.list_url) + + assert response.status_code == status.HTTP_200_OK + assert captured["request"].path == self.list_url + assert isinstance(captured["view"], CourseVideoUploadsViewSet) + assert "format" in captured + + def test_list_404_when_pipeline_disabled(self): + with override_settings(ENABLE_VIDEO_UPLOAD_PIPELINE=False): + response = self.api_client.get(self.list_url) + assert_error_envelope(response, expected_status=404, expected_type_slug="videos/uploads-not-configured") + + def test_list_404_when_course_not_configured(self): + self.course.video_upload_pipeline = {} + self.save_course() + response = self.api_client.get(self.list_url) + assert_error_envelope(response, expected_status=404, expected_type_slug="videos/uploads-not-configured") + + def test_list_404_for_a_page_past_the_end(self): + self.create_videos(count=1) + response = self.api_client.get(self.list_url, {"page": 5}) + assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + def test_list_404_for_unknown_course(self): + unknown = CourseKey.from_string("course-v1:no+such+course") + response = self.api_client.get(reverse(LIST_URL_NAME, kwargs={"course_key": unknown})) + assert_error_envelope(response, expected_status=404) + + +class CourseVideoRetrieveTest(CourseVideoUploadsTestBase): + """Tests for retrieving one of a course's videos.""" + + def test_retrieve_returns_the_addressed_video(self): + video_ids = self.create_videos(count=2) + response = self.api_client.get(self.detail_url(video_ids[0])) + assert response.status_code == status.HTTP_200_OK + assert response.data["edx_video_id"] == video_ids[0] + + def test_retrieve_reports_encoding_and_transcripts(self): + create_profile("desktop_mp4") + create_video({ + "edx_video_id": "encoded", + "client_video_id": "encoded.mp4", + "duration": 12.0, + "status": "file_complete", + "courses": [str(self.course.id)], + "encoded_videos": [{ + "profile": "desktop_mp4", + "url": "http://example.com/encoded.mp4", + "file_size": 1600, + "bitrate": 100, + }], + }) + response = self.api_client.get(self.detail_url("encoded")) + assert response.data["download_link"] == "http://example.com/encoded.mp4" + assert response.data["file_size"] == 1600 + assert response.data["transcripts"] == [] + assert response.data["transcript_urls"] == {} + + def test_retrieve_minimal_view(self): + video_ids = self.create_videos(count=1) + response = self.api_client.get(self.detail_url(video_ids[0]), {"view": "minimal"}) + assert set(response.data) == { + "edx_video_id", "client_video_id", "status", "created", "duration", + } + + def test_retrieve_rejects_an_unknown_view(self): + video_ids = self.create_videos(count=1) + response = self.api_client.get(self.detail_url(video_ids[0]), {"view": "full"}) + assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert "view" in response.data["errors"] + + def test_retrieve_404_for_unknown_video(self): + response = self.api_client.get(self.detail_url("no-such-video")) + assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + def test_retrieve_404_when_pipeline_disabled(self): + video_ids = self.create_videos(count=1) + with override_settings(ENABLE_VIDEO_UPLOAD_PIPELINE=False): + response = self.api_client.get(self.detail_url(video_ids[0])) + assert_error_envelope(response, expected_status=404, expected_type_slug="videos/uploads-not-configured") + + def test_retrieve_404_for_video_of_another_course(self): + other_course = CourseFactory.create() + other_course.video_upload_pipeline = {"course_video_upload_token": "test_token"} + other_course.save() + self.store.update_item(other_course, self.user.id) + video_ids = self.create_videos(count=1, status_value="elsewhere", course=other_course) + + response = self.api_client.get(self.detail_url(video_ids[0])) + assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + def test_retrieve_404_for_removed_video(self): + video_ids = self.create_videos(count=1) + CourseVideo.objects.filter( + course_id=str(self.course.id), video__edx_video_id=video_ids[0] + ).update(is_hidden=True) + response = self.api_client.get(self.detail_url(video_ids[0])) + assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + +@ddt.ddt +class CurrentWorkflowStatusTest(CurrentWorkflowTestBase): + """What a course on the current video workflow reports for each stored status.""" + + def row(self, status_value): + """Create one video in ``status_value`` and return its row from the listing.""" + self.create_videos(count=1, status_value=status_value) + response = self.api_client.get(self.list_url) + assert response.status_code == status.HTTP_200_OK + result, = response.data["results"] + return result + + @ddt.data( + ("transcription_in_progress", "Transcription in Progress"), + ("transcript_ready", "Transcript Ready"), + ("partial_failure", "Partial Failure"), + ("transcript_failed", "Transcript Failed"), + ) + @ddt.unpack + def test_transcription_status_names_the_transcription_state(self, status_value, expected): + row = self.row(status_value) + assert row["transcription_status"] == expected + assert row["status"] == "Ready" + + @ddt.data( + ("ingest", "In Progress"), + ("file_complete", "Ready"), + ("upload_completed", "Uploaded"), + ("pipeline_error", "Failed"), + ) + @ddt.unpack + def test_transcription_status_is_empty_before_transcription_starts(self, status_value, expected): + row = self.row(status_value) + assert row["transcription_status"] == "" + assert row["status"] == expected + + def test_an_invalid_token_is_reported_as_a_youtube_duplicate(self): + row = self.row("invalid_token") + assert row["status"] == "YouTube Duplicate" + assert row["transcription_status"] == "" + + def test_the_member_reports_the_same_transcription_state(self): + video_ids = self.create_videos(count=1, status_value="transcript_failed") + response = self.api_client.get(self.detail_url(video_ids[0])) + assert response.data["transcription_status"] == "Transcript Failed" + assert response.data["status"] == "Ready" + + +class UploadTokenStatusTest(CourseVideoUploadsTestBase): + """A course still holding an upload token reports no transcription state.""" + + def test_a_transcription_status_is_reported_as_the_display_status_only(self): + self.create_videos(count=1, status_value="transcription_in_progress") + response = self.api_client.get(self.list_url) + result, = response.data["results"] + assert result["status"] == "Transcription in Progress" + assert result["transcription_status"] == "" + + +@ddt.ddt +class CourseVideoCreateTest(CourseVideoUploadsTestBase): + """Tests for creating upload slots.""" + + @patch("cms.djangoapps.contentstore.video_storage_handlers.boto3.resource") + def test_create_returns_one_slot_per_file(self, mock_boto3_resource): + _mock_bucket(mock_boto3_resource) + response = self.api_client.post( + self.list_url, + {"files": [ + {"file_name": "first.mp4", "content_type": "video/mp4"}, + {"file_name": "second.mov", "content_type": "video/quicktime"}, + ]}, + format="json", + ) + assert response.status_code == status.HTTP_201_CREATED + assert [entry["file_name"] for entry in response.data["files"]] == ["first.mp4", "second.mov"] + for entry in response.data["files"]: + assert set(entry) == {"file_name", "upload_url", "edx_video_id"} + assert entry["upload_url"] == "http://example.com/put_video" + assert Video.objects.filter(edx_video_id=entry["edx_video_id"], status="upload").exists() + assert CourseVideo.objects.filter( + course_id=str(self.course.id), video__edx_video_id=entry["edx_video_id"] + ).exists() + + @patch("cms.djangoapps.contentstore.video_storage_handlers.boto3.resource") + def test_create_presigns_with_the_requested_content_type(self, mock_boto3_resource): + mock_s3_client = _mock_bucket(mock_boto3_resource) + self.api_client.post( + self.list_url, + {"files": [{"file_name": "first.mov", "content_type": "video/quicktime"}]}, + format="json", + ) + params = mock_s3_client.generate_presigned_url.call_args[1]["Params"] + assert params["ContentType"] == "video/quicktime" + assert params["Metadata"]["client_video_id"] == "first.mov" + assert params["Metadata"]["course_key"] == str(self.course.id) + + def test_create_rejects_unsupported_content_type(self): + response = self.api_client.post( + self.list_url, + {"files": [{"file_name": "first.webm", "content_type": "video/webm"}]}, + format="json", + ) + assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert response.data["errors"]["files[0].content_type"] == [ + '"video/webm" is not a valid choice.' + ] + + def test_create_names_the_entry_each_message_belongs_to(self): + response = self.api_client.post( + self.list_url, + {"files": [ + {"file_name": "first.mp4", "content_type": "video/mp4"}, + {"content_type": "video/avi"}, + ]}, + format="json", + ) + assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert response.data["errors"] == { + "files[1].file_name": ["This field is required."], + "files[1].content_type": ['"video/avi" is not a valid choice.'], + } + + def test_create_rejects_non_ascii_file_name(self): + response = self.api_client.post( + self.list_url, + {"files": [{"file_name": "nón-ascii-näme.mp4", "content_type": "video/mp4"}]}, + format="json", + ) + assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert response.data["errors"]["files[0].file_name"] == [ + "The file name must contain only ASCII characters." + ] + + def test_create_rejects_missing_files(self): + response = self.api_client.post(self.list_url, {}, format="json") + assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert response.data["errors"]["files"] == ["This field is required."] + + def test_create_rejects_empty_files(self): + response = self.api_client.post(self.list_url, {"files": []}, format="json") + assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert response.data["errors"]["files"] == ["This list may not be empty."] + + def test_create_reports_every_message_as_a_string(self): + response = self.api_client.post(self.list_url, {"files": []}, format="json") + errors = response.data["errors"] + assert errors + for messages in errors.values(): + assert all(isinstance(message, str) for message in messages) + assert "ErrorDetail" not in json.dumps(errors) + + def test_create_rejects_unexpected_fields(self): + response = self.api_client.post( + self.list_url, + {"files": [{"file_name": "first.mp4", "content_type": "video/mp4"}], "extra": 1}, + format="json", + ) + assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert "extra" in response.data["errors"] + + def post_with_store_refusal(self, refusal_status): + """Create an upload slot against a video store that refuses with ``refusal_status``.""" + with patch( + "cms.djangoapps.contentstore.rest_api.v1.video_uploads_service.videos_post", + return_value=({"error": "internal bucket detail"}, refusal_status), + ): + return self.api_client.post( + self.list_url, + {"files": [{"file_name": "first.mp4", "content_type": "video/mp4"}]}, + format="json", + ) + + def test_create_reports_a_stable_message_when_the_store_refuses(self): + response = self.post_with_store_refusal(400) + assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert "internal bucket detail" not in json.dumps(response.data) + + @ddt.data( + (403, "authz"), + (404, "not-found"), + ) + @ddt.unpack + def test_create_keeps_the_status_the_store_refused_with(self, refusal_status, expected_slug): + response = self.post_with_store_refusal(refusal_status) + assert_error_envelope( + response, expected_status=refusal_status, expected_type_slug=expected_slug + ) + assert "internal bucket detail" not in json.dumps(response.data) + + def test_create_reports_an_unexpected_refusal_as_a_server_error(self): + response = self.post_with_store_refusal(418) + assert_error_envelope(response, expected_status=500, expected_type_slug="internal") + + def test_create_404_when_pipeline_disabled(self): + with override_settings(ENABLE_VIDEO_UPLOAD_PIPELINE=False): + response = self.api_client.post( + self.list_url, + {"files": [{"file_name": "first.mp4", "content_type": "video/mp4"}]}, + format="json", + ) + assert_error_envelope(response, expected_status=404, expected_type_slug="videos/uploads-not-configured") + + +class CourseVideoDestroyTest(CourseVideoUploadsTestBase): + """Tests for removing a video from a course.""" + + def test_destroy_detaches_the_video(self): + video_ids = self.create_videos(count=1) + response = self.api_client.delete(self.detail_url(video_ids[0])) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not response.data + course_video = CourseVideo.objects.get( + course_id=str(self.course.id), video__edx_video_id=video_ids[0] + ) + assert course_video.is_hidden is True + assert Video.objects.filter(edx_video_id=video_ids[0]).exists() + + def test_destroy_leaves_other_courses_alone(self): + other_course = CourseFactory.create() + create_video({ + "edx_video_id": "shared", + "client_video_id": "shared.mp4", + "duration": 1.0, + "status": "file_complete", + "courses": [str(self.course.id), str(other_course.id)], + "encoded_videos": [], + }) + self.api_client.delete(self.detail_url("shared")) + assert CourseVideo.objects.get( + course_id=str(other_course.id), video__edx_video_id="shared" + ).is_hidden is False + + def test_destroy_accepts_a_video_already_detached_from_the_course(self): + # A detached video is invisible to the reads but still removable, as it + # is on the legacy endpoints. + video_ids = self.create_videos(count=1) + self.api_client.delete(self.detail_url(video_ids[0])) + + assert self.api_client.get(self.detail_url(video_ids[0])).status_code == ( + status.HTTP_404_NOT_FOUND + ) + again = self.api_client.delete(self.detail_url(video_ids[0])) + assert again.status_code == status.HTTP_204_NO_CONTENT + assert CourseVideo.objects.get( + course_id=str(self.course.id), video__edx_video_id=video_ids[0] + ).is_hidden is True + + def test_destroy_404_for_unknown_video(self): + response = self.api_client.delete(self.detail_url("no-such-video")) + assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + def test_destroy_404_when_pipeline_disabled(self): + video_ids = self.create_videos(count=1) + with override_settings(ENABLE_VIDEO_UPLOAD_PIPELINE=False): + response = self.api_client.delete(self.detail_url(video_ids[0])) + assert_error_envelope(response, expected_status=404, expected_type_slug="videos/uploads-not-configured") + assert CourseVideo.objects.get( + course_id=str(self.course.id), video__edx_video_id=video_ids[0] + ).is_hidden is False + + +class CourseVideoRequestErrorTest(CourseVideoUploadsTestBase): + """A request the API refuses to read is reported as the caller's error, not the server's.""" + + def test_a_malformed_body_is_reported_as_a_client_error(self): + response = self.api_client.post( + self.list_url, data="{not json", content_type="application/json" + ) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["title"] == "Malformed Request" + + def test_an_unsupported_method_is_reported_as_such(self): + response = self.api_client.put(self.list_url, {}, format="json") + envelope = assert_error_envelope( + response, expected_status=405, expected_type_slug="method-not-allowed" + ) + assert envelope["title"] == "Method Not Allowed" + + def test_an_unsupported_media_type_is_reported_as_such(self): + response = self.api_client.post( + self.list_url, data="file_name,content_type", content_type="text/csv" + ) + envelope = assert_error_envelope( + response, expected_status=415, expected_type_slug="unsupported-media-type" + ) + assert envelope["title"] == "Unsupported Media Type" + + def test_an_unsatisfiable_accept_header_is_reported_as_such(self): + response = self.api_client.get(self.list_url, HTTP_ACCEPT="application/xml") + envelope = assert_error_envelope( + response, expected_status=406, expected_type_slug="not-acceptable" + ) + assert envelope["title"] == "Not Acceptable" + + +@ddt.ddt +class CourseVideoAuthorizationTest(CourseVideoUploadsTestBase): + """Who may reach each operation, and who may not.""" + + def setUp(self): + super().setUp() + self.video_id = self.create_videos(count=1)[0] + + def call(self, client, operation, course_key=None): + """Issue ``operation`` against the course video endpoints as ``client``.""" + course_key = course_key or self.course.id + list_url = reverse(LIST_URL_NAME, kwargs={"course_key": course_key}) + detail_url = reverse( + DETAIL_URL_NAME, kwargs={"course_key": course_key, "edx_video_id": self.video_id} + ) + if operation == "list": + return client.get(list_url) + if operation == "retrieve": + return client.get(detail_url) + if operation == "create": + with patch("cms.djangoapps.contentstore.video_storage_handlers.boto3.resource") as resource: + _mock_bucket(resource) + return client.post( + list_url, + {"files": [{"file_name": "first.mp4", "content_type": "video/mp4"}]}, + format="json", + ) + return client.delete(detail_url) + + @ddt.data( + ("list", status.HTTP_200_OK), + ("retrieve", status.HTTP_200_OK), + ("create", status.HTTP_201_CREATED), + ("destroy", status.HTTP_204_NO_CONTENT), + ) + @ddt.unpack + def test_global_staff_is_allowed(self, operation, expected_status): + assert self.call(self.client_for(self.user), operation).status_code == expected_status + + @ddt.data( + ("list", status.HTTP_200_OK), + ("retrieve", status.HTTP_200_OK), + ("create", status.HTTP_201_CREATED), + ("destroy", status.HTTP_204_NO_CONTENT), + ) + @ddt.unpack + def test_course_staff_is_allowed(self, operation, expected_status): + author = self.create_course_author(CourseStaffRole) + assert self.call(self.client_for(author), operation).status_code == expected_status + + @ddt.data( + ("list", status.HTTP_200_OK), + ("retrieve", status.HTTP_200_OK), + ("create", status.HTTP_201_CREATED), + ("destroy", status.HTTP_204_NO_CONTENT), + ) + @ddt.unpack + def test_course_instructor_is_allowed(self, operation, expected_status): + author = self.create_course_author(CourseInstructorRole) + assert self.call(self.client_for(author), operation).status_code == expected_status + + @ddt.data("list", "retrieve", "create", "destroy") + def test_anonymous_is_refused(self, operation): + response = self.call(self.client_for(None), operation) + assert_error_envelope(response, expected_status=401, expected_type_slug="authn") + + @ddt.data("list", "retrieve", "create", "destroy") + def test_user_without_a_role_is_refused(self, operation): + stranger = UserFactory.create(is_staff=False) + response = self.call(self.client_for(stranger), operation) + assert_error_envelope(response, expected_status=403, expected_type_slug="authz") + + @ddt.data("list", "retrieve", "create", "destroy") + def test_limited_staff_is_refused(self, operation): + limited = self.create_course_author(CourseLimitedStaffRole) + response = self.call(self.client_for(limited), operation) + assert_error_envelope(response, expected_status=403, expected_type_slug="authz") + + @ddt.data("list", "retrieve", "create", "destroy") + def test_author_of_another_course_is_refused(self, operation): + other_course = CourseFactory.create() + author = self.create_course_author(CourseStaffRole, course=other_course) + response = self.call(self.client_for(author), operation) + assert_error_envelope(response, expected_status=403, expected_type_slug="authz") + + @ddt.data("list", "retrieve", "create", "destroy") + def test_ccx_course_key_is_refused(self, operation): + course_id = self.course.id + ccx_key = CourseKey.from_string( + f"ccx-v1:{course_id.org}+{course_id.course}+{course_id.run}+ccx@1" + ) + response = self.call(self.client_for(self.user), operation, course_key=ccx_key) + assert_error_envelope(response, expected_status=403, expected_type_slug="authz") + + @ddt.data("list", "retrieve", "create", "destroy") + def test_a_course_the_caller_cannot_read_in_studio_is_refused(self, operation): + # Studio's own read check refuses with its framework-level error, which + # carries no status of its own; the operation must still answer 403. + with patch( + "cms.djangoapps.contentstore.rest_api.v1.video_uploads_service._get_and_validate_course", + side_effect=DjangoPermissionDenied(), + ): + response = self.call(self.client_for(self.user), operation) + assert_error_envelope(response, expected_status=403, expected_type_slug="authz") + + @ddt.data("list", "retrieve", "create", "destroy") + def test_a_course_that_cannot_be_loaded_is_reported_as_missing(self, operation): + with patch( + "cms.djangoapps.contentstore.rest_api.v1.video_uploads_service._get_and_validate_course", + side_effect=Http404(), + ): + response = self.call(self.client_for(self.user), operation) + assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + @ddt.data("list", "retrieve", "create", "destroy") + def test_no_domain_record_is_touched_when_refused(self, operation): + stranger = UserFactory.create(is_staff=False) + before = (Video.objects.count(), CourseVideo.objects.count()) + self.call(self.client_for(stranger), operation) + assert (Video.objects.count(), CourseVideo.objects.count()) == before + + +class CourseVideoReadOnlyTest(CourseVideoUploadsTestBase): + """Reading the course's videos must not change any of them.""" + + def setUp(self): + super().setUp() + self.create_videos(count=1, status_value="upload") + self.stuck_id = "video-upload-0" + self.set_created([self.stuck_id], age=timedelta(hours=48)) + + def row_counts(self): + return (Video.objects.count(), CourseVideo.objects.count(), EncodedVideo.objects.count()) + + def test_list_reports_a_stuck_upload_as_failed_without_changing_it(self): + before = Video.objects.get(edx_video_id=self.stuck_id).status + counts = self.row_counts() + + response = self.api_client.get(self.list_url) + + assert response.data["results"][0]["status"] == "Failed" + assert Video.objects.get(edx_video_id=self.stuck_id).status == before + assert self.row_counts() == counts + + def test_head_on_the_list_changes_nothing(self): + before = Video.objects.get(edx_video_id=self.stuck_id).status + counts = self.row_counts() + + response = self.api_client.head(self.list_url) + + assert response.status_code == status.HTTP_200_OK + assert Video.objects.get(edx_video_id=self.stuck_id).status == before + assert self.row_counts() == counts + + def test_head_on_the_member_changes_nothing(self): + before = Video.objects.get(edx_video_id=self.stuck_id).status + counts = self.row_counts() + + response = self.api_client.head(self.detail_url(self.stuck_id)) + + assert response.status_code == status.HTTP_200_OK + assert Video.objects.get(edx_video_id=self.stuck_id).status == before + assert self.row_counts() == counts + + def test_retrieve_reports_a_stuck_upload_as_failed_without_changing_it(self): + before = Video.objects.get(edx_video_id=self.stuck_id).status + counts = self.row_counts() + + response = self.api_client.get(self.detail_url(self.stuck_id)) + + assert response.data["status"] == "Failed" + assert Video.objects.get(edx_video_id=self.stuck_id).status == before + assert self.row_counts() == counts + + def test_the_legacy_route_still_reconciles_a_stuck_upload(self): + from cms.djangoapps.contentstore.utils import reverse_course_url + + assert Video.objects.get(edx_video_id=self.stuck_id).status == "upload" + response = self.client.get_json(reverse_course_url("videos_handler", str(self.course.id))) + assert response.status_code == status.HTTP_200_OK + assert Video.objects.get(edx_video_id=self.stuck_id).status == "upload_failed" + + +class CourseVideoAuthenticationTest(CourseVideoUploadsTestBase): + """The endpoints use the platform's default authentication.""" + + def test_session_caller_succeeds(self): + client = APIClient() + client.login(username=self.user.username, password=self.user_password) + assert client.get(self.list_url).status_code == status.HTTP_200_OK + + def test_jwt_caller_succeeds(self): + client = APIClient() + token = generate_jwt(self.user) + assert client.get( + self.list_url, HTTP_AUTHORIZATION=f"JWT {token}" + ).status_code == status.HTTP_200_OK + + def deactivated_session_client(self, is_staff=False): + """Log a course author in, then deactivate the account behind the live session.""" + password = "password-12345" + author = UserFactory.create(is_staff=is_staff, password=password) + CourseStaffRole(self.course.id).add_users(author) + client = APIClient() + assert client.login(username=author.username, password=password) + author.is_active = False + author.save() + return client + + def test_inactive_author_is_refused(self): + response = self.deactivated_session_client().get(self.list_url) + + assert_error_envelope(response, expected_status=401, expected_type_slug="authn") + + def test_inactive_author_is_refused_by_the_legacy_address_too(self): + video_ids = self.create_videos(count=1) + response = self.deactivated_session_client().get( + self.legacy_detail_url(video_ids[0]), HTTP_ACCEPT="application/json" + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_inactive_global_staff_is_refused(self): + response = self.deactivated_session_client(is_staff=True).get(self.list_url) + + assert_error_envelope(response, expected_status=401, expected_type_slug="authn") + + def test_inactive_global_staff_still_reaches_the_legacy_address(self): + video_ids = self.create_videos(count=1) + response = self.deactivated_session_client(is_staff=True).get( + self.legacy_detail_url(video_ids[0]), HTTP_ACCEPT="application/json" + ) + + assert response.status_code == status.HTTP_200_OK + + def test_the_viewset_does_not_declare_authentication_classes(self): + assert "authentication_classes" not in CourseVideoUploadsViewSet.__dict__ + assert not any( + "Bearer" in klass.__name__ or "OAuth2" in klass.__name__ + for klass in CourseVideoUploadsViewSet.authentication_classes + ) + + +class CourseVideoUrlContractTest(CourseVideoUploadsTestBase): + """The addresses, their names, and what happens to keys they do not accept.""" + + def test_conforming_addresses(self): + course_key = "course-v1:edX+DemoX+Demo_Course" + assert reverse(LIST_URL_NAME, kwargs={"course_key": course_key}) == ( + f"/api/authoring/v1/courses/{course_key}/videos/" + ) + assert reverse( + DETAIL_URL_NAME, kwargs={"course_key": course_key, "edx_video_id": "abc-123"} + ) == f"/api/authoring/v1/courses/{course_key}/videos/abc-123/" + + def test_conforming_addresses_resolve_to_the_viewset(self): + assert resolve(self.list_url).func.cls is CourseVideoUploadsViewSet + assert resolve(self.detail_url("abc-123")).func.cls is CourseVideoUploadsViewSet + + def test_a_usable_course_key_reaches_the_viewset_and_not_the_standby_routes(self): + """A key the converter accepts must be served by the endpoints, whatever stands behind them.""" + list_match, detail_match = resolve(self.list_url), resolve(self.detail_url("abc-123")) + + assert (list_match.url_name, detail_match.url_name) == ("course_video_list", "course_video_detail") + assert list_match.kwargs["course_key"] == self.course.id + assert detail_match.kwargs["course_key"] == self.course.id + assert isinstance(detail_match.kwargs["course_key"], CourseKey) + + def test_legacy_addresses_are_unchanged(self): + course_id = str(self.course.id) + assert self.legacy_list_url() == f"/api/contentstore/v0/videos/uploads/{course_id}" + assert self.legacy_detail_url("abc-123") == ( + f"/api/contentstore/v0/videos/uploads/{course_id}/abc-123" + ) + assert resolve(self.legacy_list_url()).func.cls is VideosCreateUploadView + assert resolve(self.legacy_detail_url("abc-123")).func.cls is VideosUploadsView + + def assert_json_not_found(self, url): + """Assert that ``url`` answers with the API error body rather than an error page.""" + response = self.api_client.get(url) + assert response["Content-Type"].startswith("application/json") + return assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + def test_deprecated_course_key_is_not_routed(self): + self.assert_json_not_found("/api/authoring/v1/courses/Org/Course/Run/videos/") + + def test_deprecated_course_key_is_not_routed_on_the_member_address(self): + self.assert_json_not_found("/api/authoring/v1/courses/Org/Course/Run/videos/abc-123/") + + def test_the_legacy_addresses_still_accept_a_deprecated_course_key(self): + legacy_match = resolve("/api/contentstore/v0/videos/uploads/Org/Course/Run/abc-123") + assert legacy_match.func.cls is VideosUploadsView + assert legacy_match.kwargs["course_id"] == "Org/Course/Run" + + def test_the_legacy_list_address_still_accepts_a_deprecated_course_key(self): + legacy_match = resolve("/api/contentstore/v0/videos/uploads/Org/Course/Run") + assert legacy_match.func.cls is VideosCreateUploadView + assert legacy_match.kwargs["course_id"] == "Org/Course/Run" + + def test_malformed_course_key_is_not_routed(self): + self.assert_json_not_found("/api/authoring/v1/courses/a+b+c/videos/") + + def test_an_unknown_address_under_a_course_is_not_routed(self): + self.assert_json_not_found("/api/authoring/v1/courses/course-v1:edX+DemoX+Demo_Course/no_such_thing/") + + def test_an_unknown_member_of_a_course_is_not_routed(self): + self.assert_json_not_found("/api/authoring/v1/courses/a+b+c/videos/abc-123/") + + def test_an_address_outside_a_course_is_left_to_the_site(self): + """ + Only addresses under a course carry the API error body. + + An address that names no course at all is beyond what this API claims, and + is answered by the site the same way as any other address it does not serve. + """ + from django.urls import Resolver404 + + with pytest.raises(Resolver404): + resolve("/api/authoring/v1/no/such/thing/") + assert self.api_client.get("/api/authoring/v1/no/such/thing/").status_code == 404 + + def test_unmatched_addresses_are_claimed_by_routes_that_can_be_reversed(self): + """Each address the endpoints turn down is claimed by a named route of its own.""" + unusable_key = "a+b+c" + addresses = { + "authoring_v1:course_video_list_unmatched": + f"/api/authoring/v1/courses/{unusable_key}/videos/", + "authoring_v1:course_video_detail_unmatched": + f"/api/authoring/v1/courses/{unusable_key}/videos/abc-123/", + "authoring_v1:course_unmatched": + f"/api/authoring/v1/courses/{unusable_key}/", + } + kwargs = { + "authoring_v1:course_video_detail_unmatched": {"edx_video_id": "abc-123"}, + } + for name, address in addresses.items(): + reversed_address = reverse(name, kwargs={"course_key": unusable_key, **kwargs.get(name, {})}) + assert reversed_address == address, name + assert resolve(address).func.cls is UnknownRouteView, name + + def test_an_unclaimed_address_needs_no_authentication(self): + response = self.client_for(None).get("/api/authoring/v1/courses/a+b+c/videos/") + assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + def test_addresses_require_the_trailing_slash(self): + from django.urls import Resolver404 + + with pytest.raises(Resolver404): + resolve(self.list_url.rstrip("/")) + with pytest.raises(Resolver404): + resolve(self.detail_url("abc-123").rstrip("/")) + + def test_the_trailing_slash_is_still_appended(self): + response = self.api_client.get(self.list_url.rstrip("/")) + assert response.status_code == status.HTTP_301_MOVED_PERMANENTLY + assert response["Location"] == self.list_url + + def test_url_names_are_unique_to_the_authoring_namespace(self): + from django.urls import NoReverseMatch + + for name in ("course_video_list", "course_video_detail"): + with pytest.raises(NoReverseMatch): + reverse(f"cms.djangoapps.contentstore:v1:{name}", kwargs={"course_key": self.course.id}) + + +class CourseVideoQueryCountTest(CourseVideoUploadsTestBase): + """The cost of a page must not grow with the size of the course.""" + + #: Set from the pytest fixture below, which class-based tests cannot request directly. + django_assert_num_queries = None + + @pytest.fixture(autouse=True) + def _assert_num_queries(self, django_assert_num_queries): + self.django_assert_num_queries = django_assert_num_queries + + def warm_caches(self): + """Issue requests first so per-process caches do not skew the measured count.""" + for __ in range(2): + self.api_client.get(self.list_url, {"page_size": 1}) + + def test_list_query_count(self): + self.create_videos(count=10) + self.warm_caches() + with self.django_assert_num_queries(21): + self.api_client.get(self.list_url, {"page_size": 10}) + + def test_list_query_count_is_independent_of_course_size(self): + self.create_videos(count=100) + self.warm_caches() + with self.django_assert_num_queries(21): + self.api_client.get(self.list_url, {"page_size": 10}) + + def test_list_query_count_grows_only_with_the_page(self): + self.create_videos(count=10) + self.warm_caches() + with self.django_assert_num_queries(13): + self.api_client.get(self.list_url, {"page_size": 2}) + + def test_retrieve_query_count(self): + video_ids = self.create_videos(count=10) + self.warm_caches() + with self.django_assert_num_queries(13): + self.api_client.get(self.detail_url(video_ids[0])) + + @patch("cms.djangoapps.contentstore.video_storage_handlers.boto3.resource") + def test_create_query_count(self, mock_boto3_resource): + _mock_bucket(mock_boto3_resource) + body = {"files": [ + {"file_name": "first.mp4", "content_type": "video/mp4"}, + {"file_name": "second.mp4", "content_type": "video/mp4"}, + ]} + self.warm_caches() + self.api_client.post(self.list_url, body, format="json") + with self.django_assert_num_queries(16): + self.api_client.post(self.list_url, body, format="json") + + +class RowParityMixin: + """Comparing one row of the legacy listing against the same row of this version.""" + + def legacy_and_new_row(self, edx_video_id): + """Fetch both listings and return the (legacy, new) row for one video.""" + legacy = self.api_client.get( + self.legacy_detail_url(edx_video_id), HTTP_ACCEPT="application/json" + ) + new = self.api_client.get(self.list_url, {"page_size": 100}) + + assert legacy.status_code == status.HTTP_200_OK + assert new.status_code == status.HTTP_200_OK + legacy_rows = {row["edx_video_id"]: row for row in json.loads(legacy.content)["videos"]} + new_rows = {row["edx_video_id"]: row for row in json.loads(new.content)["results"]} + assert set(legacy_rows) == set(new_rows) + return legacy_rows[edx_video_id], new_rows[edx_video_id] + + def assert_row_parity(self, legacy_row, new_row): + """Assert the two rows differ in every declared way, and in no other way.""" + declared = [path for path, __ in ROW_DIFFERENCES] + changed, unexpected = _diff(legacy_row, new_row, declared) + assert not unexpected, f"undeclared differences: {sorted(unexpected)}" + for path in declared: + assert any(_matches(key, path) for key in changed), ( + f"declared difference did not occur: {path}" + ) + assert new_row["status"] == legacy_row["status_nontranslated"] + + +class CourseVideoParityTest(RowParityMixin, CourseVideoUploadsTestBase): + """The new addresses return what the legacy ones do, apart from the declared differences.""" + + def test_list_rows_match_the_legacy_listing(self): + create_profile("desktop_mp4") + create_video({ + "edx_video_id": "parity", + "client_video_id": "parity.mp4", + "duration": 42.0, + "status": "file_complete", + "courses": [str(self.course.id)], + "encoded_videos": [{ + "profile": "desktop_mp4", + "url": "http://example.com/parity.mp4", + "file_size": 1600, + "bitrate": 100, + }], + }) + self.set_created(["parity"]) + + self.assert_row_parity(*self.legacy_and_new_row("parity")) + + def test_created_keeps_the_precision_the_legacy_encoder_dropped(self): + video_ids = self.create_videos(count=1) + legacy_row, new_row = self.legacy_and_new_row(video_ids[0]) + + assert legacy_row["created"] == "2026-01-02T03:04:05.123Z" + assert new_row["created"] == "2026-01-02T03:04:05.123456Z" + + def test_the_status_is_never_locale_translated(self): + video_ids = self.create_videos(count=1) + with patch( + "cms.djangoapps.contentstore.video_storage_handlers._", side_effect="<{}>".format + ): + legacy_row, new_row = self.legacy_and_new_row(video_ids[0]) + + assert legacy_row["status"] == "" + assert new_row["status"] == "Ready" + + @patch("cms.djangoapps.contentstore.video_storage_handlers.boto3.resource") + def test_create_matches_the_legacy_create(self, mock_boto3_resource): + mock_s3_client = _mock_bucket(mock_boto3_resource) + body = {"files": [{"file_name": "parity.mp4", "content_type": "video/mp4"}]} + + legacy = self.api_client.post(self.legacy_list_url(), body, format="json") + legacy_params = mock_s3_client.generate_presigned_url.call_args[1] + new = self.api_client.post(self.list_url, body, format="json") + new_params = mock_s3_client.generate_presigned_url.call_args[1] + + assert legacy.status_code == status.HTTP_200_OK + assert new.status_code == status.HTTP_201_CREATED + + legacy_entry, = json.loads(legacy.content)["files"] + new_entry, = json.loads(new.content)["files"] + assert set(legacy_entry) == set(new_entry) + assert legacy_entry["file_name"] == new_entry["file_name"] + assert legacy_entry["upload_url"] == new_entry["upload_url"] + assert legacy_entry["edx_video_id"] != new_entry["edx_video_id"] + + assert legacy_params["ExpiresIn"] == new_params["ExpiresIn"] + assert legacy_params["Params"]["Bucket"] == new_params["Params"]["Bucket"] + assert legacy_params["Params"]["ContentType"] == new_params["Params"]["ContentType"] + assert legacy_params["Params"]["Metadata"] == new_params["Params"]["Metadata"] + + for entry in (legacy_entry, new_entry): + video = Video.objects.get(edx_video_id=entry["edx_video_id"]) + assert video.status == "upload" + assert video.client_video_id == "parity.mp4" + assert CourseVideo.objects.filter( + course_id=str(self.course.id), video=video + ).exists() + + def test_delete_matches_the_legacy_delete(self): + video_ids = self.create_videos(count=2) + + legacy = self.api_client.delete(self.legacy_detail_url(video_ids[0])) + new = self.api_client.delete(self.detail_url(video_ids[1])) + + assert legacy.status_code == new.status_code == status.HTTP_204_NO_CONTENT + for edx_video_id in video_ids: + assert CourseVideo.objects.get( + course_id=str(self.course.id), video__edx_video_id=edx_video_id + ).is_hidden is True + + def test_unauthenticated_error_shapes(self): + client = self.client_for(None) + legacy = client.get(self.legacy_detail_url("any"), HTTP_ACCEPT="application/json") + new = client.get(self.list_url) + + assert legacy.status_code == new.status_code == status.HTTP_401_UNAUTHORIZED + assert set(json.loads(legacy.content)) == {"developer_message"} + assert_error_envelope(new, expected_status=401, expected_type_slug="authn") + + def test_forbidden_error_shapes(self): + client = self.client_for(UserFactory.create(is_staff=False)) + legacy = client.get(self.legacy_detail_url("any"), HTTP_ACCEPT="application/json") + new = client.get(self.list_url) + + assert legacy.status_code == new.status_code == status.HTTP_403_FORBIDDEN + assert json.loads(legacy.content)["error_code"] == "user_permissions" + assert_error_envelope(new, expected_status=403, expected_type_slug="authz") + + def test_pipeline_disabled_error_shapes(self): + with override_settings(ENABLE_VIDEO_UPLOAD_PIPELINE=False): + legacy = self.api_client.get(self.legacy_detail_url("any"), HTTP_ACCEPT="application/json") + new = self.api_client.get(self.list_url) + + assert legacy.status_code == new.status_code == status.HTTP_404_NOT_FOUND + assert legacy.content == b"" + assert_error_envelope(new, expected_status=404, expected_type_slug="videos/uploads-not-configured") + + def test_invalid_request_body_error_shapes(self): + body = {"files": [{"file_name": "first.mp4"}]} + legacy = self.api_client.post(self.legacy_list_url(), body, format="json") + new = self.api_client.post(self.list_url, body, format="json") + + assert legacy.status_code == new.status_code == status.HTTP_400_BAD_REQUEST + assert legacy.content == b"" + assert_error_envelope(new, expected_status=400, expected_type_slug="validation") + + def test_missing_video_error_shapes(self): + legacy = self.api_client.delete(self.legacy_detail_url("no-such-video")) + new = self.api_client.delete(self.detail_url("no-such-video")) + + assert legacy.status_code == new.status_code == status.HTTP_404_NOT_FOUND + assert "developer_message" in json.loads(legacy.content) + assert_error_envelope(new, expected_status=404, expected_type_slug="not-found") + + def test_the_legacy_accept_header_branch_is_not_carried_over(self): + video_ids = self.create_videos(count=1) + + legacy = self.api_client.get(self.legacy_detail_url(video_ids[0]), HTTP_ACCEPT="*/*") + new = self.api_client.get(self.list_url, HTTP_ACCEPT="*/*") + + assert legacy.status_code == status.HTTP_302_FOUND + assert legacy["Location"] + assert new.status_code == status.HTTP_200_OK + assert "Location" not in new + assert set(json.loads(new.content)) == { + "count", "num_pages", "current_page", "start", "next", "previous", "results", + } + + def test_the_listing_is_wrapped_in_the_page_envelope(self): + self.create_videos(count=3) + + legacy = self.api_client.get( + self.legacy_detail_url("video-file_complete-0"), HTTP_ACCEPT="application/json" + ) + new = self.api_client.get(self.list_url, {"page": 2, "page_size": 1}) + + assert set(json.loads(legacy.content)) == {"videos"} + assert set(json.loads(new.content)) == { + "count", "num_pages", "current_page", "start", "next", "previous", "results", + } + + def test_minimal_view_is_a_subset_of_the_default(self): + video_ids = self.create_videos(count=1) + default = self.api_client.get(self.detail_url(video_ids[0])) + minimal = self.api_client.get(self.detail_url(video_ids[0]), {"view": "minimal"}) + assert set(minimal.data) < set(default.data) + for field in minimal.data: + assert minimal.data[field] == default.data[field] + + +@ddt.ddt +class CurrentWorkflowParityTest(RowParityMixin, CurrentWorkflowTestBase): + """Row parity for a course whose videos reach the encodes-ready and transcription states.""" + + @ddt.data( + "ingest", + "file_complete", + "upload_completed", + "invalid_token", + "transcription_in_progress", + "transcript_ready", + "partial_failure", + "transcript_failed", + ) + def test_rows_match_the_legacy_listing(self, status_value): + video_ids = self.create_videos(count=1, status_value=status_value) + + self.assert_row_parity(*self.legacy_and_new_row(video_ids[0])) + + def test_transcription_status_matches_the_legacy_listing(self): + video_ids = self.create_videos(count=1, status_value="partial_failure") + + legacy_row, new_row = self.legacy_and_new_row(video_ids[0]) + + assert legacy_row["transcription_status"] == "Partial Failure" + assert new_row["transcription_status"] == legacy_row["transcription_status"] + + +class CourseVideoSchemaTest(CourseVideoUploadsTestBase): + """What the generated schema promises, measured against what the endpoints return.""" + + def generated_schema(self): + """Generate the OpenAPI document for the course video addresses.""" + from cms.djangoapps.contentstore.rest_api.v1 import authoring_urls + + return SchemaGenerator(patterns=authoring_urls.urlpatterns).get_schema(request=None, public=True) + + def resolve(self, schema, body): + """Return the component ``body`` refers to, or ``body`` itself.""" + if "$ref" in body: + return schema["components"]["schemas"][body["$ref"].rsplit("/", 1)[-1]] + return body + + def response_schema(self, schema, path_suffix, method="get", code="200"): + """Return the declared response body for one operation, following any reference.""" + path = next(path for path in schema["paths"] if path.endswith(path_suffix)) + body = schema["paths"][path][method]["responses"][code]["content"]["application/json"]["schema"] + return self.resolve(schema, body) + + def representations(self, schema, declared): + """Return the video representations ``declared`` allows, by component name.""" + variant = self.resolve(schema, declared) + return { + member["$ref"].rsplit("/", 1)[-1]: self.resolve(schema, member) + for member in variant["oneOf"] + } + + def test_only_the_two_video_addresses_are_documented(self): + """The routes that stand behind the endpoints serve no operation and must stay unpublished.""" + assert set(self.generated_schema()["paths"]) == { + "/courses/{course_key}/videos/", + "/courses/{course_key}/videos/{edx_video_id}/", + } + + def test_the_list_response_is_declared_as_the_page_envelope(self): + envelope = self.response_schema(self.generated_schema(), "/videos/") + assert envelope["type"] == "object" + assert set(envelope["properties"]) == { + "count", "num_pages", "current_page", "start", "next", "previous", "results", + } + assert set(envelope["required"]) == {"count", "num_pages", "current_page", "start", "results"} + assert envelope["properties"]["results"]["type"] == "array" + + def test_the_declared_list_envelope_is_the_one_returned(self): + self.create_videos(count=1) + envelope = self.response_schema(self.generated_schema(), "/videos/") + response = self.api_client.get(self.list_url) + assert set(response.data) == set(envelope["properties"]) + + def test_both_video_representations_are_declared(self): + schema = self.generated_schema() + envelope = self.response_schema(schema, "/videos/") + declared = self.representations(schema, envelope["properties"]["results"]["items"]) + assert set(declared) == {"CourseVideo", "CourseVideoMinimal"} + + def test_the_declared_full_representation_is_the_one_returned(self): + self.create_videos(count=1) + schema = self.generated_schema() + envelope = self.response_schema(schema, "/videos/") + declared = self.representations(schema, envelope["properties"]["results"]["items"])["CourseVideo"] + row = self.api_client.get(self.list_url).data["results"][0] + assert set(row) == set(declared["properties"]) == set(declared["required"]) + + def test_the_declared_minimal_representation_is_the_one_returned(self): + self.create_videos(count=1) + schema = self.generated_schema() + envelope = self.response_schema(schema, "/videos/") + declared = self.representations( + schema, envelope["properties"]["results"]["items"] + )["CourseVideoMinimal"] + row = self.api_client.get(self.list_url, {"view": "minimal"}).data["results"][0] + assert set(row) == set(declared["properties"]) == set(declared["required"]) + + def test_the_member_response_declares_both_representations(self): + schema = self.generated_schema() + video_ids = self.create_videos(count=1) + declared = self.representations( + schema, schema["paths"]["/courses/{course_key}/videos/{edx_video_id}/"]["get"][ + "responses"]["200"]["content"]["application/json"]["schema"] + ) + assert set(declared) == {"CourseVideo", "CourseVideoMinimal"} + full = self.api_client.get(self.detail_url(video_ids[0])).data + minimal = self.api_client.get(self.detail_url(video_ids[0]), {"view": "minimal"}).data + assert set(full) == set(declared["CourseVideo"]["properties"]) + assert set(minimal) == set(declared["CourseVideoMinimal"]["properties"]) + + def test_the_minimal_representation_is_named_by_the_view_parameter(self): + schema = self.generated_schema() + parameters = schema["paths"]["/courses/{course_key}/videos/"]["get"]["parameters"] + view_parameter = next(item for item in parameters if item["name"] == "view") + assert view_parameter["schema"]["enum"] == ["minimal"] + declared = self.representations( + schema, + self.response_schema(schema, "/videos/")["properties"]["results"]["items"], + )["CourseVideoMinimal"] + for field in declared["properties"]: + assert field in view_parameter["description"] + + def declared_examples(self, schema, path, method, code): + """Return the declared response examples of one operation, by title.""" + content = schema["paths"][path][method]["responses"][code]["content"]["application/json"] + return {name: example["value"] for name, example in content["examples"].items()} + + def test_the_member_404_names_both_causes(self): + schema = self.generated_schema() + declared = self.declared_examples( + schema, "/courses/{course_key}/videos/{edx_video_id}/", "get", "404" + ) + assert {example["type"] for example in declared.values()} == { + "https://docs.openedx.org/errors/videos/uploads-not-configured", + "https://docs.openedx.org/errors/not-found", + } + + def test_the_declared_404_types_are_the_ones_returned(self): + schema = self.generated_schema() + declared = { + example["type"] + for example in self.declared_examples( + schema, "/courses/{course_key}/videos/{edx_video_id}/", "get", "404" + ).values() + } + video_ids = self.create_videos(count=1) + unknown = self.api_client.get(self.detail_url("no-such-video")) + with override_settings(ENABLE_VIDEO_UPLOAD_PIPELINE=False): + unconfigured = self.api_client.get(self.detail_url(video_ids[0])) + assert {unknown.data["type"], unconfigured.data["type"]} == declared + + def test_the_create_404_names_only_the_cause_it_can_have(self): + schema = self.generated_schema() + declared = self.declared_examples(schema, "/courses/{course_key}/videos/", "post", "404") + assert {example["type"] for example in declared.values()} == { + "https://docs.openedx.org/errors/videos/uploads-not-configured", + } + + def test_the_create_response_is_declared_as_the_upload_slots(self): + slots = self.response_schema(self.generated_schema(), "/videos/", method="post", code="201") + assert set(slots["properties"]) == {"files"} + + +class CmsSchemaHookTest(CourseVideoUploadsTestBase): + """The schema hooks admit the new paths and flag the superseded ones.""" + + def endpoint(self, path): + return (path, path, "get", None) + + def test_filter_admits_the_authoring_prefix(self): + admitted = cms_api_filter([ + self.endpoint("/api/authoring/v1/courses/course-v1:a+b+c/videos/"), + self.endpoint("/api/contentstore/v0/videos/uploads/course-v1:a+b+c"), + self.endpoint("/login"), + ]) + assert [entry[0] for entry in admitted] == [ + "/api/authoring/v1/courses/course-v1:a+b+c/videos/", + "/api/contentstore/v0/videos/uploads/course-v1:a+b+c", + ] + + def test_post_processing_marks_only_the_superseded_operations(self): + schema = { + "paths": { + "/v0/videos/uploads/{course_id}": {"post": {}}, + "/v0/videos/uploads/{course_id}/{edx_video_id}": {"get": {}, "delete": {}}, + "/v0/videos/images/{course_id}/{edx_video_id}": {"post": {}}, + "/api/authoring/v1/courses/{course_key}/videos/": {"get": {}, "post": {}}, + } + } + result = cms_mark_superseded_paths(schema, None, None, False) + + deprecated = { + (path, method) + for path, item in result["paths"].items() + for method, operation in item.items() + if operation.get("deprecated") + } + assert deprecated == { + ("/v0/videos/uploads/{course_id}", "post"), + ("/v0/videos/uploads/{course_id}/{edx_video_id}", "get"), + ("/v0/videos/uploads/{course_id}/{edx_video_id}", "delete"), + } + + def test_post_processing_also_matches_untrimmed_paths(self): + schema = {"paths": {"/api/contentstore/v0/videos/uploads/{course_id}": {"post": {}}}} + result = cms_mark_superseded_paths(schema, None, None, False) + assert result["paths"]["/api/contentstore/v0/videos/uploads/{course_id}"]["post"]["deprecated"] is True + + def authoring_schema(self, hooks): + """Generate the authoring document with ``hooks`` as the post-processing list.""" + from cms.djangoapps.contentstore.rest_api.v1 import authoring_urls + + with patched_settings({"POSTPROCESSING_HOOKS": hooks}): + return SchemaGenerator(patterns=authoring_urls.urlpatterns).get_schema( + request=None, public=True + ) + + def test_enum_components_survive_the_registered_post_processing(self): + deprecation_only = self.authoring_schema([SUPERSEDED_PATHS_HOOK]) + registered = self.authoring_schema(REGISTERED_POSTPROCESSING_HOOKS) + + assert not [ + name for name in deprecation_only["components"]["schemas"] if name.endswith("Enum") + ] + assert "ContentTypeEnum" in registered["components"]["schemas"] + + def test_post_processing_leaves_other_schema_members_alone(self): + schema = {"paths": {"/v0/videos/uploads/{course_id}": {"post": {}, "parameters": []}}} + result = cms_mark_superseded_paths(schema, None, None, False) + assert not result["paths"]["/v0/videos/uploads/{course_id}"]["parameters"] + + +@ddt.ddt +class CmsSchemaAddressTest(TestCase): + """The published document addresses every operation the way it is mounted.""" + + # One operation of every Studio API version the document covers, with the + # address a client reaches by joining the published path to a server, plus + # both new addresses. + PUBLISHED_PATHS = { + "/api/contentstore/v0/videos/uploads/{course_id}": + "/api/contentstore/v0/videos/uploads/course-v1:a+b+c", + "/api/contentstore/v1/videos/{course_id}": + "/api/contentstore/v1/videos/course-v1:a+b+c", + "/api/contentstore/v2/home/courses": + "/api/contentstore/v2/home/courses", + "/api/contentstore/v3/course_details/{course_id}/": + "/api/contentstore/v3/course_details/course-v1:a+b+c/", + "/api/contentstore/v4/home/courses/": + "/api/contentstore/v4/home/courses/", + "/api/authoring/v1/courses/{course_key}/videos/": + "/api/authoring/v1/courses/course-v1:a+b+c/videos/", + "/api/authoring/v1/courses/{course_key}/videos/{edx_video_id}/": + "/api/authoring/v1/courses/course-v1:a+b+c/videos/video-1/", + } + + def cms_schema(self, module=PRODUCTION_SETTINGS): + """Generate the Studio document with the schema settings of ``module``.""" + read = schema_settings(module) + with patched_settings({key: read[key] for key in GENERATION_SETTING_KEYS}): + return SchemaGenerator().get_schema(request=None, public=True) + + def config_without_public_host(self): + """Write a deployment configuration that leaves the public host unset.""" + directory = tempfile.TemporaryDirectory() + self.addCleanup(directory.cleanup) + config = Path(directory.name) / "cms.yml" + config.write_text("".join( + line for line in MOCK_CONFIG.read_text().splitlines(keepends=True) + if not line.startswith("AUTHORING_API_URL:") + )) + return config + + def test_the_document_publishes_the_mounted_addresses(self): + assert set(self.PUBLISHED_PATHS) <= set(self.cms_schema()["paths"]) + + def test_the_document_publishes_no_other_authoring_address(self): + published = self.cms_schema()["paths"] + authoring = {path for path in published if path.startswith("/api/authoring/")} + assert authoring == {path for path in self.PUBLISHED_PATHS if path.startswith("/api/authoring/")} + + def test_no_published_path_is_shortened(self): + published = sorted(self.cms_schema()["paths"]) + assert published, "the pre-processing hook admitted no path" + assert [path for path in published if not path.startswith("/api/")] == [] + + def test_every_published_path_is_an_address_that_resolves(self): + for published, address in self.PUBLISHED_PATHS.items(): + assert resolve(address).func, published + + @ddt.data(PRODUCTION_SETTINGS, DEVSTACK_SETTINGS) + def test_the_settings_publish_paths_in_full(self, module): + assert schema_settings(module)["SCHEMA_PATH_PREFIX_TRIM"] is False + + @ddt.data(PRODUCTION_SETTINGS, DEVSTACK_SETTINGS) + def test_the_servers_serve_every_published_path(self, module): + servers = schema_settings(module)["SERVERS"] + assert [server["description"] for server in servers] == ["Public", "Local"] + assert [server for server in servers if "/api/contentstore" in server["url"]] == [] + + @ddt.data(PRODUCTION_SETTINGS, DEVSTACK_SETTINGS) + def test_an_unset_public_host_is_left_out_of_the_servers(self, module): + servers = schema_settings(module, config_file=self.config_without_public_host())["SERVERS"] + assert [server["description"] for server in servers] == ["Local"] + assert all(server["url"] for server in servers) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/unknown_route.py b/cms/djangoapps/contentstore/rest_api/v1/views/unknown_route.py new file mode 100644 index 000000000000..dd089c0163fe --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/unknown_route.py @@ -0,0 +1,26 @@ +"""API view answering addresses that no endpoint serves.""" + +from drf_spectacular.utils import extend_schema +from edx_rest_framework_extensions.mixins import StandardizedErrorMixin +from rest_framework.exceptions import NotFound +from rest_framework.permissions import AllowAny +from rest_framework.views import APIView + + +@extend_schema(exclude=True) +class UnknownRouteView(StandardizedErrorMixin, APIView): + """ + Answer an address under a course that no endpoint of this API serves. + + A course key that is malformed or in the deprecated slash-separated form, or + a misspelt address beneath a course, would otherwise be answered by the + site's HTML error page, which an API client cannot read. Nothing here needs + the caller's identity, so the answer is the same for everyone: the address + holds no resource. + """ + + permission_classes = (AllowAny,) + + def initial(self, request, *args, **kwargs): + """Refuse the request before any handler runs; the address holds nothing.""" + raise NotFound() diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/video_uploads.py b/cms/djangoapps/contentstore/rest_api/v1/views/video_uploads.py new file mode 100644 index 000000000000..25e308b7e16a --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/video_uploads.py @@ -0,0 +1,399 @@ +"""API Views for the video assets of a course.""" + +import logging + +from drf_spectacular.utils import ( + OpenApiExample, + OpenApiParameter, + OpenApiRequest, + OpenApiResponse, + PolymorphicProxySerializer, + extend_schema, +) +from edx_rest_framework_extensions.errors import ErrorResponseSerializer, error_type_uri +from edx_rest_framework_extensions.mixins import StandardizedErrorMixin +from edx_rest_framework_extensions.paginators import DefaultPagination, IterablePaginationMixin +from edx_rest_framework_extensions.shaping import MinimalViewMixin +from rest_framework import status, viewsets +from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response + +from cms.djangoapps.contentstore.rest_api.v1.serializers.video_uploads import ( + MINIMAL_VIDEO_FIELDS, + CourseVideoListQuerySerializer, + CourseVideoMinimalSerializer, + CourseVideoQuerySerializer, + CourseVideoSerializer, + VideoUploadRequestSerializer, + VideoUploadResponseSerializer, +) +from cms.djangoapps.contentstore.rest_api.v1.video_uploads_service import ( + DEFAULT_ORDERING, + VIDEO_ORDERING_CHOICES, + VIDEO_UPLOADS_NOT_CONFIGURED_SLUG, + create_video_uploads, + delete_course_video, + enrich_course_videos, + get_course_for_uploads, + get_course_video, + list_course_videos, +) +from cms.djangoapps.contentstore.rest_api.v1.views.permissions import HasCourseAuthorAccess + +log = logging.getLogger(__name__) + + +class CourseVideoPagination(DefaultPagination): + """ + The platform's page envelope, described in full to schema consumers. + + ``DefaultPagination`` answers with seven members, but the response schema it + inherits names only four of them, so a generated client would not know that + ``num_pages``, ``current_page`` and ``start`` are there. + """ + + def get_paginated_response_schema(self, schema): + """Describe every member of the page envelope this paginator returns.""" + inherited = super().get_paginated_response_schema(schema)["properties"] + return { + "type": "object", + "required": ["count", "num_pages", "current_page", "start", "results"], + "properties": { + "count": inherited["count"], + "num_pages": { + "type": "integer", + "description": "Number of pages the results are divided into.", + "example": 13, + }, + "current_page": { + "type": "integer", + "description": "Number of the page returned.", + "example": 4, + }, + "start": { + "type": "integer", + "description": "Position of this page's first result within the whole list.", + "example": 30, + }, + "next": inherited["next"], + "previous": inherited["previous"], + "results": inherited["results"], + }, + } + + +_COURSE_KEY_PARAMETER = OpenApiParameter( + name="course_key", + description="Key of the course the videos belong to, for example course-v1:edX+DemoX+Demo_Course.", + required=True, + type=str, + location=OpenApiParameter.PATH, +) + +_EDX_VIDEO_ID_PARAMETER = OpenApiParameter( + name="edx_video_id", + description="Identifier of the video, as returned when its upload slot was created.", + required=True, + type=str, + location=OpenApiParameter.PATH, +) + +_VIEW_PARAMETER = OpenApiParameter( + name="view", + description=( + "Response preset. 'minimal' reduces each video to " + f"{', '.join(MINIMAL_VIDEO_FIELDS)}, omitting every other field. " + "Omit the parameter to receive the full representation." + ), + required=False, + type=str, + location=OpenApiParameter.QUERY, + enum=[MinimalViewMixin.minimal_view_value], +) + +_ORDERING_PARAMETER = OpenApiParameter( + name="ordering", + description=( + "Field to sort the videos by. Prefix the field name with '-' to sort descending. " + f"Defaults to {DEFAULT_ORDERING}." + ), + required=False, + type=str, + location=OpenApiParameter.QUERY, + enum=list(VIDEO_ORDERING_CHOICES), +) + +_PAGE_PARAMETER = OpenApiParameter( + name="page", + description="Number of the page to return. Defaults to the first page.", + required=False, + type=int, + location=OpenApiParameter.QUERY, +) + +_PAGE_SIZE_PARAMETER = OpenApiParameter( + name="page_size", + description=( + f"Number of videos per page. Defaults to {CourseVideoPagination.page_size}; a larger " + f"value is reduced to the maximum of {CourseVideoPagination.max_page_size}." + ), + required=False, + type=int, + location=OpenApiParameter.QUERY, +) + +_RESPONSE_BAD_REQUEST = OpenApiResponse( + response=ErrorResponseSerializer, + description="The request body or a query parameter is invalid.", +) +_RESPONSE_UNAUTHENTICATED = OpenApiResponse( + response=ErrorResponseSerializer, + description="The requester is not authenticated.", +) +_RESPONSE_FORBIDDEN = OpenApiResponse( + response=ErrorResponseSerializer, + description="The requester does not have authoring access to the course.", +) +_EXAMPLE_UPLOADS_NOT_CONFIGURED = OpenApiExample( + "The course does not accept video uploads", + value={ + "type": error_type_uri(VIDEO_UPLOADS_NOT_CONFIGURED_SLUG), + "title": "Video Uploads Not Configured", + "status": 404, + "detail": "Video uploads are not configured for this course.", + "instance": "/api/authoring/v1/courses/course-v1:edX+DemoX+Demo_Course/videos/", + }, + response_only=True, + status_codes=["404"], +) + +_EXAMPLE_VIDEO_NOT_FOUND = OpenApiExample( + "No such video in this course", + value={ + "type": error_type_uri("not-found"), + "title": "Not Found", + "status": 404, + "detail": "Not found.", + "instance": ( + "/api/authoring/v1/courses/course-v1:edX+DemoX+Demo_Course/videos/" + "8f2a6d9c-2f2e-4a7e-9d6b-1f0c9e2a4b31/" + ), + }, + response_only=True, + status_codes=["404"], +) + +_RESPONSE_NOT_FOUND = OpenApiResponse( + response=ErrorResponseSerializer, + description="The course does not accept video uploads, or the video is not attached to it.", + examples=[_EXAMPLE_UPLOADS_NOT_CONFIGURED, _EXAMPLE_VIDEO_NOT_FOUND], +) +_RESPONSE_CREATE_NOT_FOUND = OpenApiResponse( + response=ErrorResponseSerializer, + description="The course does not accept video uploads.", + examples=[_EXAMPLE_UPLOADS_NOT_CONFIGURED], +) +_RESPONSE_LIST_NOT_FOUND = OpenApiResponse( + response=ErrorResponseSerializer, + description=( + "The course does not accept video uploads, or the requested page is past the end of " + "the list." + ), + examples=[_EXAMPLE_UPLOADS_NOT_CONFIGURED], +) + +_CREATE_EXAMPLE_REQUEST = OpenApiExample( + "Two files", + value={ + "files": [ + {"file_name": "lecture-01.mp4", "content_type": "video/mp4"}, + {"file_name": "lecture-02.mov", "content_type": "video/quicktime"}, + ] + }, + request_only=True, +) + +_CREATE_EXAMPLE_RESPONSE = OpenApiExample( + "Upload slots to PUT the files to", + value={ + "files": [ + { + "file_name": "lecture-01.mp4", + "upload_url": "https://video-uploads.example.com/videos/8f2a...?X-Amz-Signature=...", + "edx_video_id": "8f2a6d9c-2f2e-4a7e-9d6b-1f0c9e2a4b31", + }, + { + "file_name": "lecture-02.mov", + "upload_url": "https://video-uploads.example.com/videos/1c7b...?X-Amz-Signature=...", + "edx_video_id": "1c7b40f5-6b62-4a51-8f10-0b4d2a9f7c58", + }, + ] + }, + response_only=True, +) + + +def _video_response(description, many=False): + """Describe a response carrying videos in whichever representation was asked for.""" + return OpenApiResponse( + response=PolymorphicProxySerializer( + component_name="CourseVideoRepresentation", + serializers=[CourseVideoSerializer, CourseVideoMinimalSerializer], + resource_type_field_name=None, + many=many, + ), + description=description, + ) + + +@extend_schema(tags=["openedx-platform-sdk"]) +class CourseVideoUploadsViewSet( + StandardizedErrorMixin, MinimalViewMixin, IterablePaginationMixin, viewsets.ViewSet +): + """ + The video assets of a single course. + + Lists the videos attached to a course, returns one of them, creates upload + slots for new ones, and detaches a video from the course. Reads and writes + go through the video store rather than the database directly, so this is a + plain ViewSet with no queryset of its own. + + Listing a video reports an upload that has been stuck past the upload window + as failed without changing the stored record; a stuck upload is reconciled + by the upload pipeline, not by reading the list. + """ + + permission_classes = (IsAuthenticated, HasCourseAuthorAccess) + serializer_class = CourseVideoSerializer + pagination_class = CourseVideoPagination + minimal_fields = MINIMAL_VIDEO_FIELDS + + def __init__(self, **kwargs): + super().__init__(**kwargs) + self.course_key = None + + def initial(self, request, *args, **kwargs): + """Expose the course key for the permission check that runs next.""" + self.course_key = kwargs.get("course_key") + super().initial(request, *args, **kwargs) + + def get_serializer_context(self): + """Return the context every serializer of this view is built with.""" + return {"request": self.request, "view": self, "format": self.format_kwarg} + + def get_serializer(self, *args, **kwargs): + """Instantiate and return the configured serializer class.""" + kwargs.setdefault("context", self.get_serializer_context()) + return self.serializer_class(*args, **kwargs) + + @extend_schema( + summary="List the videos of a course", + description=( + "Returns a paginated list of the videos attached to the course, newest first by " + "default. Transcript details are resolved only for the videos on the requested page." + ), + parameters=[ + _COURSE_KEY_PARAMETER, + _ORDERING_PARAMETER, + _PAGE_PARAMETER, + _PAGE_SIZE_PARAMETER, + _VIEW_PARAMETER, + ], + responses={ + 200: _video_response( + "A page of videos, in the representation the view parameter selected.", + many=True, + ), + 400: _RESPONSE_BAD_REQUEST, + 401: _RESPONSE_UNAUTHENTICATED, + 403: _RESPONSE_FORBIDDEN, + 404: _RESPONSE_LIST_NOT_FOUND, + }, + ) + def list(self, request, course_key): + """Return a page of the course's videos.""" + query = CourseVideoListQuerySerializer(data=request.query_params) + query.is_valid(raise_exception=True) + course = get_course_for_uploads(course_key, request.user) + videos = list_course_videos(course, query.validated_data.get("ordering", DEFAULT_ORDERING)) + return self.paginate_iterable( + request, + videos, + serialize=lambda page: self.shape_minimal( + self.get_serializer(enrich_course_videos(course, page), many=True).data, + request, + ), + ) + + @extend_schema( + summary="Get one video of a course", + description="Returns the video attached to the course under the given identifier.", + parameters=[_COURSE_KEY_PARAMETER, _EDX_VIDEO_ID_PARAMETER, _VIEW_PARAMETER], + responses={ + 200: _video_response("The video, in the representation the view parameter selected."), + 400: _RESPONSE_BAD_REQUEST, + 401: _RESPONSE_UNAUTHENTICATED, + 403: _RESPONSE_FORBIDDEN, + 404: _RESPONSE_NOT_FOUND, + }, + ) + def retrieve(self, request, course_key, edx_video_id): + """Return one of the course's videos.""" + CourseVideoQuerySerializer(data=request.query_params).is_valid(raise_exception=True) + course = get_course_for_uploads(course_key, request.user) + video = get_course_video(course, edx_video_id) + enriched, = enrich_course_videos(course, [video]) + data = self.get_serializer(enriched).data + return Response(self.shape_minimal(data, request)) + + @extend_schema( + summary="Create upload slots for new course videos", + description=( + "Registers one video per requested file and returns a short-lived pre-signed URL for " + "each. Upload the file itself with a PUT to that URL; this endpoint does not receive " + "file content. The returned identifiers address the videos from then on." + ), + parameters=[_COURSE_KEY_PARAMETER], + request=OpenApiRequest(request=VideoUploadRequestSerializer), + responses={ + 201: OpenApiResponse( + response=VideoUploadResponseSerializer, + description="Upload slots created, one per requested file, in request order.", + ), + 400: _RESPONSE_BAD_REQUEST, + 401: _RESPONSE_UNAUTHENTICATED, + 403: _RESPONSE_FORBIDDEN, + 404: _RESPONSE_CREATE_NOT_FOUND, + }, + examples=[_CREATE_EXAMPLE_REQUEST, _CREATE_EXAMPLE_RESPONSE], + ) + def create(self, request, course_key): + """Create an upload slot for each requested file.""" + request_serializer = VideoUploadRequestSerializer(data=request.data) + request_serializer.is_valid(raise_exception=True) + course = get_course_for_uploads(course_key, request.user) + created = create_video_uploads(course, request_serializer.validated_data["files"]) + return Response( + VideoUploadResponseSerializer(created).data, + status=status.HTTP_201_CREATED, + ) + + @extend_schema( + summary="Remove a video from a course", + description=( + "Detaches the video from the course. The video itself is kept, so it stays available " + "in any other course it is attached to." + ), + parameters=[_COURSE_KEY_PARAMETER, _EDX_VIDEO_ID_PARAMETER], + responses={ + 204: OpenApiResponse(description="The video is no longer attached to the course."), + 401: _RESPONSE_UNAUTHENTICATED, + 403: _RESPONSE_FORBIDDEN, + 404: _RESPONSE_NOT_FOUND, + }, + ) + def destroy(self, request, course_key, edx_video_id): + """Detach one video from the course.""" + get_course_for_uploads(course_key, request.user) + delete_course_video(course_key, edx_video_id) + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/cms/urls.py b/cms/urls.py index c0f96f489bb8..9bf84dc7ea86 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -13,6 +13,7 @@ from django.views.generic import RedirectView from drf_spectacular.views import SpectacularAPIView, SpectacularSwaggerView from edx_api_doc_tools import make_docs_urls +from edx_rest_framework_extensions.url_converters import register_url_converters import openedx.core.djangoapps.common_views.xblock import openedx.core.djangoapps.debug.views @@ -27,6 +28,7 @@ from openedx.core.djangoapps.password_policy.forms import PasswordPolicyAwareAdminAuthForm django_autodiscover() +register_url_converters() admin.site.site_header = _('Studio Administration') admin.site.site_title = admin.site.site_header @@ -356,6 +358,11 @@ path('api/contentstore/', include('cms.djangoapps.contentstore.rest_api.urls')) ] +# Authoring REST APIs +urlpatterns += [ + path('api/authoring/v1/', include('cms.djangoapps.contentstore.rest_api.v1.authoring_urls')), +] + # Content tagging urlpatterns += [ path('api/content_tagging/', include(('openedx.core.djangoapps.content_tagging.urls', 'content_tagging'))), From fedc4befe2ee4600649a0156099b3f94be8abeda Mon Sep 17 00:00:00 2001 From: Abdul-Muqadim-Arbisoft Date: Mon, 14 Sep 2026 17:37:04 +0500 Subject: [PATCH 2/3] fix: publish CMS schema paths in full so every operation is reachable The CMS schema trimmed '/api/contentstore' from every path and advertised a matching server entry. Once the document also carries paths under another prefix, that arrangement has no server that serves both: a client generated from the advertised server builds https://{CMS_BASE}/api/contentstore/api/authoring/v1/courses/{key}/videos/ and gets a 404. Trimming is therefore off and the server list is reduced to the public host and the Studio host, so one server URL serves every operation in the document. Path strings change from /v0/... to /api/contentstore/v0/..., but no effective URL moves and no operation is added or removed by this commit. Two consequences worth knowing. The schema comparison gate reads a path-key rename as a removal, so it reports a breaking change for each renamed path; the compensating addition is invisible to it. And AUTHORING_API_URL now joins full paths rather than shortened ones, so it must be the Studio service root rather than a gateway base. These settings predate this work; the defect is latent on master and is surfaced, not introduced, by adding a second prefix to the document. --- cms/envs/devstack.py | 22 +++++++++++++++++----- cms/envs/production.py | 22 +++++++++++++++++----- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 0576a35272af..a2e5282309ea 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -356,12 +356,24 @@ def should_show_debug_toolbar(request): # pylint: disable=missing-function-docs 'SERVE_INCLUDE_SCHEMA': False, # restrict spectacular to CMS API endpoints (cms/lib/spectacular.py): 'PREPROCESSING_HOOKS': ['cms.lib.spectacular.cms_api_filter'], - # remove the default schema path prefix to replace it with server-specific base paths: + # Setting this replaces drf-spectacular's default hook list, so the default + # enum post-processing has to be listed again to keep running: + 'POSTPROCESSING_HOOKS': [ + 'drf_spectacular.hooks.postprocess_schema_enums', + 'cms.lib.spectacular.cms_mark_superseded_paths', + ], + # Paths are published in full, so one server URL serves every operation of the + # document. The prefix below is not removed from the paths; it only keeps the + # service prefix out of generated operation ids and tags: 'SCHEMA_PATH_PREFIX': '/api/contentstore', - 'SCHEMA_PATH_PREFIX_TRIM': '/api/contentstore', + 'SCHEMA_PATH_PREFIX_TRIM': False, + # The public host is optional, so an unset URL is left out of the list: 'SERVERS': [ - {'url': AUTHORING_API_URL, 'description': 'Public'}, # noqa: F405 - {'url': f'http://{CMS_BASE}', 'description': 'Local'}, - {'url': f'http://{CMS_BASE}/api/contentstore', 'description': 'CMS-contentstore'} + {'url': url, 'description': description} + for url, description in [ + (AUTHORING_API_URL, 'Public'), # noqa: F405 + (f'http://{CMS_BASE}', 'Local'), + ] + if url ], } diff --git a/cms/envs/production.py b/cms/envs/production.py index 604d2753bccd..d3ad492662de 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -416,13 +416,25 @@ def get_env_setting(setting): 'SERVE_INCLUDE_SCHEMA': False, # restrict spectacular to CMS API endpoints (cms/lib/spectacular.py): 'PREPROCESSING_HOOKS': ['cms.lib.spectacular.cms_api_filter'], - # remove the default schema path prefix to replace it with server-specific base paths: + # Setting this replaces drf-spectacular's default hook list, so the default + # enum post-processing has to be listed again to keep running: + 'POSTPROCESSING_HOOKS': [ + 'drf_spectacular.hooks.postprocess_schema_enums', + 'cms.lib.spectacular.cms_mark_superseded_paths', + ], + # Paths are published in full, so one server URL serves every operation of the + # document. The prefix below is not removed from the paths; it only keeps the + # service prefix out of generated operation ids and tags: 'SCHEMA_PATH_PREFIX': '/api/contentstore', - 'SCHEMA_PATH_PREFIX_TRIM': '/api/contentstore', + 'SCHEMA_PATH_PREFIX_TRIM': False, + # The public host is optional, so an unset URL is left out of the list: 'SERVERS': [ - {'url': AUTHORING_API_URL, 'description': 'Public'}, # noqa: F405 - {'url': f'https://{CMS_BASE}', 'description': 'Local'}, # noqa: F405 - {'url': f'https://{CMS_BASE}/api/contentstore', 'description': 'CMS-contentstore'} # noqa: F405 + {'url': url, 'description': description} + for url, description in [ + (AUTHORING_API_URL, 'Public'), # noqa: F405 + (f'https://{CMS_BASE}', 'Local'), # noqa: F405 + ] + if url ], } From 92408b87e0289ea38818d9f1f8c22d2dbfcbfb1c Mon Sep 17 00:00:00 2001 From: Abdul-Muqadim-Arbisoft Date: Mon, 14 Sep 2026 17:37:15 +0500 Subject: [PATCH 3/3] docs: mark the v0 video upload endpoints deprecated The three video upload operations under /api/contentstore/v0/videos/uploads/ now have conforming successors, so they are marked deprecated in the published schema by a post-processing hook, and their docstrings name the successor address for each operation. The hook lives outside the frozen version directory, and the change inside it is docstring text only: no import, no statement, no decorator, so the v0 endpoints behave exactly as before. No runtime Deprecation header is emitted, since that would be a response change on a live endpoint. The pre-processing filter is widened at the same time so the authoring paths reach the document at all; without it they are dropped silently. Still owed before the deprecation window can formally open: a DEPR issue in openedx/public-engineering, and a named removal release. Both need someone who knows the current release train. Part of #39060. --- .../rest_api/v0/views/authoring_videos.py | 28 ++++++++++++++++ cms/lib/spectacular.py | 32 +++++++++++++++++-- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/authoring_videos.py b/cms/djangoapps/contentstore/rest_api/v0/views/authoring_videos.py index 5a7563c2372e..e5618a05b6cc 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/authoring_videos.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/authoring_videos.py @@ -1,5 +1,12 @@ """ Public rest API endpoints for the Authoring API video assets. + +.. deprecated:: + The video upload endpoints in this module are superseded by + ``CourseVideoUploadsViewSet`` in + ``cms.djangoapps.contentstore.rest_api.v1.views.video_uploads``. + Use ``/api/authoring/v1/courses/{course_key}/videos/`` going forward. + These v0 endpoints will be removed in a future release. """ import logging @@ -27,6 +34,10 @@ @view_auth_classes() class VideosUploadsView(DeveloperErrorViewMixin, RetrieveAPIView, DestroyAPIView): """ + **DEPRECATED** — use ``GET /api/authoring/v1/courses/{course_key}/videos/`` for the listing + this GET returns, and ``DELETE /api/authoring/v1/courses/{course_key}/videos/{edx_video_id}/`` + to remove one video from the course. These v0 endpoints will be removed in a future release. + public rest API endpoints for the CMS API video assets. course_key: required argument, needed to authorize course authors and identify the video. video_id: required argument, needed to identify the video. @@ -35,17 +46,34 @@ class VideosUploadsView(DeveloperErrorViewMixin, RetrieveAPIView, DestroyAPIView @course_author_access_required def retrieve(self, request, course_key, edx_video_id=None): # pylint: disable=arguments-differ + """ + **DEPRECATED** — use ``GET /api/authoring/v1/courses/{course_key}/videos/`` instead. + + This GET ignores ``edx_video_id`` and returns the whole course listing; + ``/api/authoring/v1/courses/{course_key}/videos/`` is the successor for that listing. + It will be removed in a future release. + """ return handle_videos(request, course_key.html_id(), edx_video_id) @course_author_access_required @expect_json_in_class_view def destroy(self, request, course_key, edx_video_id): # pylint: disable=arguments-differ + """ + **DEPRECATED** — use + ``DELETE /api/authoring/v1/courses/{course_key}/videos/{edx_video_id}/`` instead. + + It will be removed in a future release. + """ return handle_videos(request, course_key.html_id(), edx_video_id) @view_auth_classes() class VideosCreateUploadView(DeveloperErrorViewMixin, CreateAPIView): """ + **DEPRECATED** — use ``POST /api/authoring/v1/courses/{course_key}/videos/`` instead. + + These v0 endpoints will be removed in a future release. + public rest API endpoints for the CMS API video assets. course_key: required argument, needed to authorize course authors and identify the video. """ diff --git a/cms/lib/spectacular.py b/cms/lib/spectacular.py index 90bce5668fec..af60402915b2 100644 --- a/cms/lib/spectacular.py +++ b/cms/lib/spectacular.py @@ -2,14 +2,26 @@ import re +CMS_PATH_PATTERN = re.compile(r"^/api/(contentstore|authoring)/v\d+/") + +# Path prefixes of operations that have a conforming successor and are kept only +# for their deprecation window. The mounted prefix is listed next to the +# shortened form, so the hook still matches where a document is published +# without the service prefix. +SUPERSEDED_PATH_PREFIXES = ( + "/api/contentstore/v0/videos/uploads/", + "/v0/videos/uploads/", +) + +_OPERATION_KEYS = frozenset({"get", "put", "post", "delete", "options", "head", "patch", "trace"}) + def cms_api_filter(endpoints): """ - Pre-processing hook: keep only contentstore versioned endpoints and select - course-level endpoints. + Pre-processing hook: keep the versioned authoring and contentstore + endpoints, and the course-level endpoints named below. """ filtered = [] - CMS_PATH_PATTERN = re.compile(r"^/api/contentstore/v\d+/") for path, path_regex, method, callback in endpoints: if ( @@ -22,3 +34,17 @@ def cms_api_filter(endpoints): filtered.append((path, path_regex, method, callback)) return filtered + + +def cms_mark_superseded_paths(result, generator, request, public): # pylint: disable=unused-argument + """ + Post-processing hook: mark every operation of a superseded path deprecated. + """ + for path, path_item in (result.get("paths") or {}).items(): + if not path.startswith(SUPERSEDED_PATH_PREFIXES): + continue + for key, operation in path_item.items(): + if key in _OPERATION_KEYS and isinstance(operation, dict): + operation["deprecated"] = True + + return result