From cd2ce32d7315c467a7957340dd22da164d34eafe Mon Sep 17 00:00:00 2001 From: Jesper Hodge <19345795+jesperhodge@users.noreply.github.com> Date: Fri, 22 Sep 2023 17:49:49 -0400 Subject: [PATCH] CMS API serializers and validations (#33316) Internal ticket: https://2u-internal.atlassian.net/browse/TNL-11077 This adds validations to all new CMS API endpoints via serializers. Validations follow the "strong parameter" concept: https://www.mintbit.com/blog/securing-ruby-on-rails-applications-part-3-use-strong-parameters#:~:text=Strong%20parameters%20are%20a%20feature,parameters%20to%20a%20controller%20action. --- .../rest_api/v1/serializers/__init__.py | 4 + .../rest_api/v1/serializers/assets.py | 13 +++ .../rest_api/v1/serializers/common.py | 33 ++++++++ .../rest_api/v1/serializers/transcripts.py | 15 ++++ .../rest_api/v1/serializers/videos.py | 29 +++++++ .../rest_api/v1/serializers/xblock.py | 82 +++++++++++++++++-- .../contentstore/rest_api/v1/urls.py | 2 +- .../contentstore/rest_api/v1/views/assets.py | 9 ++ .../rest_api/v1/views/tests/test_assets.py | 8 +- .../rest_api/v1/views/tests/test_xblock.py | 21 +---- .../rest_api/v1/views/transcripts.py | 8 ++ .../contentstore/rest_api/v1/views/utils.py | 23 ++++++ .../contentstore/rest_api/v1/views/videos.py | 18 +++- .../contentstore/rest_api/v1/views/xblock.py | 9 +- .../contentstore/video_storage_handlers.py | 4 +- 15 files changed, 239 insertions(+), 39 deletions(-) create mode 100644 cms/djangoapps/contentstore/rest_api/v1/serializers/assets.py create mode 100644 cms/djangoapps/contentstore/rest_api/v1/serializers/transcripts.py create mode 100644 cms/djangoapps/contentstore/rest_api/v1/serializers/videos.py create mode 100644 cms/djangoapps/contentstore/rest_api/v1/views/utils.py diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/__init__.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/__init__.py index c60d33bc44..a5c3497998 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/__init__.py @@ -13,3 +13,7 @@ from .proctoring import ( ProctoringErrorsSerializer ) from .settings import CourseSettingsSerializer +from .xblock import XblockSerializer +from .videos import VideoUploadSerializer, VideoImageSerializer +from .transcripts import TranscriptSerializer +from .assets import AssetSerializer diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/assets.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/assets.py new file mode 100644 index 0000000000..e44c31b511 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/assets.py @@ -0,0 +1,13 @@ +""" +API Serializers for assets +""" +from rest_framework import serializers +from .common import StrictSerializer + + +class AssetSerializer(StrictSerializer): + """ + Strict Serializer for file assets. + """ + file = serializers.FileField(required=False, allow_null=True) + locked = serializers.BooleanField(required=False, allow_null=True) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/common.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/common.py index bc2f8d2da6..362504ccb0 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/common.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/common.py @@ -17,3 +17,36 @@ class CourseCommonSerializer(serializers.Serializer): rerun_link = serializers.CharField() run = serializers.CharField() url = serializers.CharField() + + +class StrictSerializer(serializers.Serializer): + """ + Serializers that validates strong parameters, i.e. that no extra fields are passed in. + The serializer inheriting from this may throw a ValidationError and can be called in a try/catch + block that will return a 400 response on ValidationError. + """ + def to_internal_value(self, data): + """ + raise validation error if there are any unexpected fields. + """ + # Transform and validate the expected fields + ret = super().to_internal_value(data) + + # Get the list of valid fields from the serializer + valid_fields = set(self.fields.keys()) + + # Check for unexpected fields + extra_fields = set(data.keys()) - valid_fields + if extra_fields: + # Check if these unexpected fields are due to nested serializers + for field_name in list(extra_fields): + if isinstance(self.fields.get(field_name), serializers.BaseSerializer): + extra_fields.remove(field_name) + + # If there are still unexpected fields left, raise an error + if extra_fields: + raise serializers.ValidationError( + {field: ["This field is not expected."] for field in extra_fields} + ) + + return ret diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/transcripts.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/transcripts.py new file mode 100644 index 0000000000..2b72f1ff44 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/transcripts.py @@ -0,0 +1,15 @@ +""" +API Serializers for transcripts +""" +from rest_framework import serializers +from .common import StrictSerializer + + +class TranscriptSerializer(StrictSerializer): + """ + Strict Serializer for video transcripts. + """ + file = serializers.FileField() + edx_video_id = serializers.CharField() + language_code = serializers.CharField(required=False, allow_null=True) + new_language_code = serializers.CharField(required=False, allow_null=True) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/videos.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/videos.py new file mode 100644 index 0000000000..c08856d1b5 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/videos.py @@ -0,0 +1,29 @@ +""" +API Serializers for videos +""" +from rest_framework import serializers +from .common import StrictSerializer + + +class FileSpecSerializer(StrictSerializer): + """ Strict Serializer for file specs """ + file_name = serializers.CharField() + content_type = serializers.ChoiceField(choices=['video/mp4', 'video/webm', 'video/ogg']) + + +class VideoUploadSerializer(StrictSerializer): + """ + Strict Serializer for video upload urls. + Note that these are not actually video uploads but endpoints to generate an upload url for AWS + and generating a video placeholder without performing an actual upload. + """ + files = serializers.ListField( + child=FileSpecSerializer() + ) + + +class VideoImageSerializer(StrictSerializer): + """ + Strict Serializer for video imgage files. + """ + file = serializers.ImageField() diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/xblock.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/xblock.py index 68d44af0ff..522a34e077 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/xblock.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/xblock.py @@ -1,15 +1,79 @@ """ API Serializers for xblocks """ +from rest_framework import serializers +from .common import StrictSerializer + +# The XblockSerializer is designed to be scalable and generic. As such, its structure +# should remain as general as possible. Avoid indiscriminately adding fields to it, +# especially those that are xblock-specific. In the future, we aim to develop a solution +# that can generate serializer fields dynamically based on the xblock definitions. -# TODO: implement and use serializer -# from rest_framework import serializers +class XblockSerializer(StrictSerializer): + """ + A serializer for xblocks that enforces strict validation. -# class XblockSerializer(serializers.Serializer): -# """Serializer for xblocks""" -# id=serializers.CharField(required=False) -# display_name=serializers.CharField(required=False) -# category=serializers.CharField(required=False) -# data=serializers.CharField(required=False) -# metadata=serializers.DictField(required=False) + The serializer ensures: + 1. All top-level fields have the expected data types. + 2. No unexpected fields are passed in. + + Note: The current list of fields is not exhaustive. It is primarily designed + to support the CMS API demo. While optional fields have been added, they were + chosen based on ease of discovery, not comprehensiveness. + """ + id = serializers.CharField(required=False, allow_null=True) + parent_locator = serializers.CharField(required=False, allow_null=True) + display_name = serializers.CharField(required=False, allow_null=True) + category = serializers.CharField(required=False, allow_null=True) + data = serializers.CharField(required=False, allow_null=True) + metadata = serializers.DictField(required=False, allow_null=True) + has_changes = serializers.BooleanField(required=False, allow_null=True) + children = serializers.ListField(required=False, allow_null=True) + fields = serializers.DictField(required=False, allow_null=True) + has_children = serializers.BooleanField(required=False, allow_null=True) + video_sharing_enabled = serializers.BooleanField(required=False, allow_null=True) + video_sharing_options = serializers.CharField(required=False, allow_null=True) + video_sharing_doc_url = serializers.CharField(required=False, allow_null=True) + edited_on = serializers.CharField(required=False, allow_null=True) + published = serializers.BooleanField(required=False, allow_null=True) + published_on = serializers.JSONField(required=False, allow_null=True) + studio_url = serializers.CharField(required=False, allow_null=True) + released_to_students = serializers.BooleanField(required=False, allow_null=True) + release_date = serializers.JSONField(required=False, allow_null=True) + nullout = serializers.JSONField(required=False, allow_null=True) + graderType = serializers.JSONField(required=False, allow_null=True) + visibility_state = serializers.CharField(required=False, allow_null=True) + has_explicit_staff_lock = serializers.BooleanField( + required=False, allow_null=True + ) + start = serializers.CharField(required=False, allow_null=True) + graded = serializers.BooleanField(required=False, allow_null=True) + due_date = serializers.CharField(required=False, allow_null=True) + due = serializers.JSONField(required=False, allow_null=True) + relative_weeks_due = serializers.JSONField(required=False, allow_null=True) + format = serializers.JSONField(required=False, allow_null=True) + course_graders = serializers.ListField(required=False, allow_null=True) + actions = serializers.DictField(required=False, allow_null=True) + explanatory_message = serializers.Field(required=False, allow_null=True) + group_access = serializers.DictField(required=False, allow_null=True) + user_partitions = serializers.ListField(required=False, allow_null=True) + show_correctness = serializers.CharField(required=False, allow_null=True) + discussion_enabled = serializers.BooleanField(required=False, allow_null=True) + ancestor_has_staff_lock = serializers.BooleanField(required=False, allow_null=True) + user_partition_info = serializers.DictField(required=False, allow_null=True) + summary_configuration_enabled = serializers.JSONField(required=False, allow_null=True) + isPrereq = serializers.BooleanField(required=False, allow_null=True) + prereqUsageKey = serializers.CharField(required=False, allow_null=True) + prereqMinScore = serializers.IntegerField(required=False, allow_null=True) + prereqMinCompletion = serializers.IntegerField(required=False, allow_null=True) + publish = serializers.ChoiceField( + required=False, + allow_null=True, + choices=['make_public', 'republish', 'discard_changes'] + ) + duplicate_source_locator = serializers.CharField(required=False, allow_null=True) + move_source_locator = serializers.CharField(required=False, allow_null=True) + target_index = serializers.IntegerField(required=False, allow_null=True) + boilerplate = serializers.JSONField(required=False, allow_null=True) + staged_content = serializers.CharField(required=False, allow_null=True) diff --git a/cms/djangoapps/contentstore/rest_api/v1/urls.py b/cms/djangoapps/contentstore/rest_api/v1/urls.py index 286424f5e1..9c1bef0c90 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v1/urls.py @@ -74,7 +74,7 @@ urlpatterns = [ videos.VideosView.as_view(), name='studio_content_videos_uploads' ), re_path( - fr'^videos/images/{settings.COURSE_ID_PATTERN}/{VIDEO_ID_PATTERN}?$', + fr'^videos/images/{settings.COURSE_ID_PATTERN}/{VIDEO_ID_PATTERN}$', videos.VideoImagesView.as_view(), name='studio_content_videos_images' ), re_path( diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/assets.py b/cms/djangoapps/contentstore/rest_api/v1/views/assets.py index dc95087a12..481d850ca8 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/assets.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/assets.py @@ -14,6 +14,11 @@ from ....api import course_author_access_required from cms.djangoapps.contentstore.asset_storage_handlers import handle_assets import cms.djangoapps.contentstore.toggles as contentstore_toggles +from cms.djangoapps.contentstore.rest_api.v1.serializers import AssetSerializer +from .utils import validate_request_with_serializer +from rest_framework.parsers import (MultiPartParser, FormParser, JSONParser) +from openedx.core.lib.api.parsers import TypedFileUploadParser + log = logging.getLogger(__name__) toggles = contentstore_toggles @@ -25,6 +30,8 @@ class AssetsView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView, CreateAP course_key: required argument, needed to authorize course authors and identify the asset. asset_key_string: required argument, needed to identify the asset. """ + serializer_class = AssetSerializer + parser_classes = (JSONParser, MultiPartParser, FormParser, TypedFileUploadParser) def dispatch(self, request, *args, **kwargs): # TODO: probably want to refactor this to a decorator. @@ -44,11 +51,13 @@ class AssetsView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView, CreateAP @csrf_exempt @course_author_access_required + @validate_request_with_serializer def create(self, request, course_key): # pylint: disable=arguments-differ return handle_assets(request, course_key.html_id()) @course_author_access_required @expect_json_in_class_view + @validate_request_with_serializer def update(self, request, course_key, asset_key_string): # pylint: disable=arguments-differ return handle_assets(request, course_key.html_id(), asset_key_string) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_assets.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_assets.py index 908af1d1bc..61fa331d6c 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_assets.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_assets.py @@ -6,9 +6,11 @@ not the underlying Xblock service. It checks that the assets_handler method of the Xblock service is called with the expected parameters. """ from unittest.mock import patch +from django.core.files import File from django.http import JsonResponse from django.urls import reverse +from mock import MagicMock from rest_framework import status from rest_framework.test import APITestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -17,6 +19,8 @@ from cms.djangoapps.contentstore.tests.test_utils import AuthorizeStaffTestCase ASSET_KEY_STRING = "asset-v1:dede+aba+weagi+type@asset+block@_0e37192a-42c4-441e-a3e1-8e40ec304e2e.jpg" +mock_image = MagicMock(file=File) +mock_image.name = "test.jpg" class AssetsViewTestCase(AuthorizeStaffTestCase): @@ -146,7 +150,7 @@ class AssetsViewPostTest(AssetsViewTestCase, ModuleStoreTestCase, APITestCase): def get_test_data(self): return { - "file": ASSET_KEY_STRING, + "file": mock_image, } def assert_assets_handler_called(self, *, mock_handle_assets, response): @@ -159,7 +163,7 @@ class AssetsViewPostTest(AssetsViewTestCase, ModuleStoreTestCase, APITestCase): course_id = self.get_course_key_string() - assert passed_args.data.get("file") == ASSET_KEY_STRING + assert passed_args.data.get("file").name == mock_image.name assert passed_args.method == "POST" assert passed_args.path == self.get_url() diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_xblock.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_xblock.py index f8d871a5be..64462a6ee2 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_xblock.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_xblock.py @@ -50,7 +50,7 @@ class XBlockViewTestCase(AuthorizeStaffTestCase): return_value=JsonResponse( { "locator": TEST_LOCATOR, - "courseKey": AuthorizeStaffTestCase.get_course_key_string(), + "course_key": AuthorizeStaffTestCase.get_course_key_string(), } ), ) @@ -128,7 +128,6 @@ class XBlockViewGetTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): assert response.status_code == status.HTTP_200_OK data = response.json() assert data["locator"] == TEST_LOCATOR - assert data["courseKey"] == self.get_course_key_string() class XBlockViewPostTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): @@ -150,7 +149,6 @@ class XBlockViewPostTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): return { "parent_locator": course_id, "category": "html", - "courseKey": course_id, } def assert_xblock_handler_called(self, *, mock_handle_xblock, response): @@ -161,9 +159,6 @@ class XBlockViewPostTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): mock_handle_xblock.assert_called_once() passed_args = mock_handle_xblock.call_args[0][0] - course_id = self.get_course_key_string() - - assert passed_args.data.get("courseKey") == course_id assert passed_args.method == "POST" assert passed_args.path == self.get_url() @@ -187,7 +182,6 @@ class XBlockViewPostTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): assert response.status_code == status.HTTP_200_OK data = response.json() assert data["locator"] == TEST_LOCATOR - assert data["courseKey"] == self.get_course_key_string() class XBlockViewPutTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): @@ -196,10 +190,8 @@ class XBlockViewPutTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): """ def get_test_data(self): - course_id = self.get_course_key_string() return { "category": "html", - "courseKey": course_id, "data": "

Updated block!

", "has_changes": True, "id": TEST_LOCATOR, @@ -216,9 +208,6 @@ class XBlockViewPutTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): mock_handle_xblock.assert_called_once() passed_args = mock_handle_xblock.call_args[0][0] - course_id = self.get_course_key_string() - - assert passed_args.data.get("courseKey") == course_id assert passed_args.data.get("data") == "

Updated block!

" assert passed_args.data.get("id") == TEST_LOCATOR assert passed_args.method == "PUT" @@ -244,7 +233,6 @@ class XBlockViewPutTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): assert response.status_code == status.HTTP_200_OK data = response.json() assert data["locator"] == TEST_LOCATOR - assert data["courseKey"] == self.get_course_key_string() class XBlockViewPatchTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): @@ -253,10 +241,8 @@ class XBlockViewPatchTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): """ def get_test_data(self): - course_id = self.get_course_key_string() return { "category": "html", - "courseKey": course_id, "data": "

Patched block!

", "has_changes": True, "id": TEST_LOCATOR, @@ -273,9 +259,6 @@ class XBlockViewPatchTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): mock_handle_xblock.assert_called_once() passed_args = mock_handle_xblock.call_args[0][0] - course_id = self.get_course_key_string() - - assert passed_args.data.get("courseKey") == course_id assert passed_args.data.get("data") == "

Patched block!

" assert passed_args.data.get("id") == TEST_LOCATOR assert passed_args.method == "PATCH" @@ -301,7 +284,6 @@ class XBlockViewPatchTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): assert response.status_code == status.HTTP_200_OK data = response.json() assert data["locator"] == TEST_LOCATOR - assert data["courseKey"] == self.get_course_key_string() class XBlockViewDeleteTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase): @@ -343,4 +325,3 @@ class XBlockViewDeleteTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase) assert response.status_code == status.HTTP_200_OK data = response.json() assert data["locator"] == TEST_LOCATOR - assert data["courseKey"] == self.get_course_key_string() diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/transcripts.py b/cms/djangoapps/contentstore/rest_api/v1/views/transcripts.py index e4e8ff8667..f621929f9c 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/transcripts.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/transcripts.py @@ -21,6 +21,11 @@ from cms.djangoapps.contentstore.transcript_storage_handlers import ( handle_transcript_download, ) import cms.djangoapps.contentstore.toggles as contentstore_toggles +from cms.djangoapps.contentstore.rest_api.v1.serializers import TranscriptSerializer +from rest_framework.parsers import (MultiPartParser, FormParser) +from openedx.core.lib.api.parsers import TypedFileUploadParser + +from .utils import validate_request_with_serializer log = logging.getLogger(__name__) toggles = contentstore_toggles @@ -34,6 +39,8 @@ class TranscriptView(DeveloperErrorViewMixin, CreateAPIView, RetrieveAPIView, De edx_video_id: optional query parameter, needed to identify the transcript. language_code: optional query parameter, needed to identify the transcript. """ + serializer_class = TranscriptSerializer + parser_classes = (MultiPartParser, FormParser, TypedFileUploadParser) def dispatch(self, request, *args, **kwargs): if not toggles.use_studio_content_api(): @@ -43,6 +50,7 @@ class TranscriptView(DeveloperErrorViewMixin, CreateAPIView, RetrieveAPIView, De @csrf_exempt @course_author_access_required @expect_json_in_class_view + @validate_request_with_serializer def create(self, request, course_key_string): # pylint: disable=arguments-differ return upload_transcript(request) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/utils.py b/cms/djangoapps/contentstore/rest_api/v1/views/utils.py new file mode 100644 index 0000000000..7175b76c7c --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/utils.py @@ -0,0 +1,23 @@ +""" +Utilities for the REST API views. +""" +from functools import wraps +from django.http import HttpResponseBadRequest + + +def validate_request_with_serializer(view_func): + """ + A decorator to validate request data using the view's serializer. + + Usage: + @validate_request_with_serializer + def my_view_function(self, request, ...): + ... + """ + @wraps(view_func) + def _wrapped_view(instance, request, *args, **kwargs): + serializer = instance.serializer_class(data=request.data) + if not serializer.is_valid(): + return HttpResponseBadRequest(reason=serializer.errors) + return view_func(instance, request, *args, **kwargs) + return _wrapped_view diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/videos.py b/cms/djangoapps/contentstore/rest_api/v1/views/videos.py index a6e776b77c..710dbb5df2 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/videos.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/videos.py @@ -7,10 +7,12 @@ from rest_framework.generics import ( RetrieveAPIView, DestroyAPIView ) +from rest_framework.parsers import (MultiPartParser, FormParser) from django.views.decorators.csrf import csrf_exempt from django.http import Http404 from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes +from openedx.core.lib.api.parsers import TypedFileUploadParser from common.djangoapps.util.json_request import expect_json_in_class_view from ....api import course_author_access_required @@ -19,10 +21,12 @@ from cms.djangoapps.contentstore.video_storage_handlers import ( handle_videos, get_video_encodings_download, handle_video_images, - enabled_video_features, - handle_generate_video_upload_link + enabled_video_features ) +from cms.djangoapps.contentstore.rest_api.v1.serializers import VideoUploadSerializer, VideoImageSerializer import cms.djangoapps.contentstore.toggles as contentstore_toggles +from .utils import validate_request_with_serializer + log = logging.getLogger(__name__) toggles = contentstore_toggles @@ -35,6 +39,7 @@ class VideosView(DeveloperErrorViewMixin, CreateAPIView, RetrieveAPIView, Destro course_key: required argument, needed to authorize course authors and identify the video. video_id: required argument, needed to identify the video. """ + serializer_class = VideoUploadSerializer def dispatch(self, request, *args, **kwargs): # TODO: probably want to refactor this to a decorator. @@ -50,7 +55,9 @@ class VideosView(DeveloperErrorViewMixin, CreateAPIView, RetrieveAPIView, Destro @csrf_exempt @course_author_access_required @expect_json_in_class_view + @validate_request_with_serializer def create(self, request, course_key): # pylint: disable=arguments-differ + """Deprecated. Use the upload_link endpoint instead.""" return handle_videos(request, course_key.html_id()) @course_author_access_required @@ -70,6 +77,8 @@ class VideoImagesView(DeveloperErrorViewMixin, CreateAPIView): course_key: required argument, needed to authorize course authors and identify the video. video_id: required argument, needed to identify the video. """ + serializer_class = VideoImageSerializer + parser_classes = (MultiPartParser, FormParser, TypedFileUploadParser) def dispatch(self, request, *args, **kwargs): # TODO: probably want to refactor this to a decorator. @@ -85,6 +94,7 @@ class VideoImagesView(DeveloperErrorViewMixin, CreateAPIView): @csrf_exempt @course_author_access_required @expect_json_in_class_view + @validate_request_with_serializer def create(self, request, course_key, edx_video_id=None): # pylint: disable=arguments-differ return handle_video_images(request, course_key.html_id(), edx_video_id) @@ -140,6 +150,7 @@ class UploadLinkView(DeveloperErrorViewMixin, CreateAPIView): """ public rest API endpoint providing a list of enabled video features. """ + serializer_class = VideoUploadSerializer def dispatch(self, request, *args, **kwargs): # TODO: probably want to refactor this to a decorator. @@ -155,5 +166,6 @@ class UploadLinkView(DeveloperErrorViewMixin, CreateAPIView): @csrf_exempt @course_author_access_required @expect_json_in_class_view + @validate_request_with_serializer def create(self, request, course_key): # pylint: disable=arguments-differ - return handle_generate_video_upload_link(request, course_key.html_id()) + return handle_videos(request, course_key.html_id()) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/xblock.py b/cms/djangoapps/contentstore/rest_api/v1/views/xblock.py index 13087c6cd3..0e5765d8a2 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/xblock.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/xblock.py @@ -13,7 +13,8 @@ from cms.djangoapps.contentstore.api import course_author_access_required from cms.djangoapps.contentstore.xblock_storage_handlers import view_handlers import cms.djangoapps.contentstore.toggles as contentstore_toggles -# from cms.djangoapps.contentstore.rest_api.v1.serializers import XblockSerializer +from cms.djangoapps.contentstore.rest_api.v1.serializers import XblockSerializer +from .utils import validate_request_with_serializer log = logging.getLogger(__name__) @@ -29,8 +30,7 @@ class XblockView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView, CreateAP usage_key_string (optional): xblock identifier, for example in the form of "block-v1:+type@+block@" """ - # TODO: uncomment next line after XblockSerializer is implemented - # serializer_class = XblockSerializer + serializer_class = XblockSerializer def dispatch(self, request, *args, **kwargs): # TODO: probably want to refactor this to a decorator. @@ -51,11 +51,13 @@ class XblockView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView, CreateAP @course_author_access_required @expect_json_in_class_view + @validate_request_with_serializer def update(self, request, course_key, usage_key_string=None): return handle_xblock(request, usage_key_string) @course_author_access_required @expect_json_in_class_view + @validate_request_with_serializer def partial_update(self, request, course_key, usage_key_string=None): return handle_xblock(request, usage_key_string) @@ -67,5 +69,6 @@ class XblockView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView, CreateAP @csrf_exempt @course_author_access_required @expect_json_in_class_view + @validate_request_with_serializer def create(self, request, course_key, usage_key_string=None): return handle_xblock(request, usage_key_string) diff --git a/cms/djangoapps/contentstore/video_storage_handlers.py b/cms/djangoapps/contentstore/video_storage_handlers.py index d1a7c55ac6..6e83a5b9e3 100644 --- a/cms/djangoapps/contentstore/video_storage_handlers.py +++ b/cms/djangoapps/contentstore/video_storage_handlers.py @@ -729,7 +729,9 @@ def videos_post(course, request): """ if use_mock_video_uploads(): - return {'files': [{'file_name': 'video.mp4', 'upload_url': 'http://example.com/put_video'}]}, 200 + return {'files': [{ + 'file_name': 'video.mp4', 'upload_url': 'http://example.com/put_video', 'edx_video_id': '1234' + }]}, 200 error = None data = request.json