From 942c79e606a18cc6e1f1c05e03243d69a6a4e2ab Mon Sep 17 00:00:00 2001 From: DawoudSheraz Date: Thu, 5 Sep 2019 10:08:43 +0500 Subject: [PATCH] add video url transformer --- lms/djangoapps/course_api/blocks/api.py | 10 +- .../course_api/blocks/tests/test_api.py | 42 +++++- lms/djangoapps/course_api/blocks/toggles.py | 39 ++++++ .../blocks/transformers/blocks_api.py | 8 +- .../transformers/tests/test_video_urls.py | 123 ++++++++++++++++++ .../blocks/transformers/video_urls.py | 53 ++++++++ 6 files changed, 263 insertions(+), 12 deletions(-) create mode 100644 lms/djangoapps/course_api/blocks/toggles.py create mode 100644 lms/djangoapps/course_api/blocks/transformers/tests/test_video_urls.py create mode 100644 lms/djangoapps/course_api/blocks/transformers/video_urls.py diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index 35d0cd9db4..6d2c31fc51 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -9,10 +9,10 @@ from lms.djangoapps.course_blocks.transformers.access_denied_filter import Acces from lms.djangoapps.course_blocks.transformers.hidden_content import HiddenContentTransformer from lms.djangoapps.course_blocks.transformers.hide_empty import HideEmptyTransformer from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers -from openedx.core.djangoapps.waffle_utils import WaffleFlag, WaffleFlagNamespace from openedx.core.lib.mobile_utils import is_request_from_mobile_app from .serializers import BlockDictSerializer, BlockSerializer +from .toggles import HIDE_ACCESS_DENIALS_FLAG from .transformers.block_completion import BlockCompletionTransformer from .transformers.blocks_api import BlocksAPITransformer from .transformers.milestones import MilestonesAndSpecialExamsTransformer @@ -61,13 +61,7 @@ def get_blocks( attached. """ - course_blocks_namespace = WaffleFlagNamespace(name=u'course_blocks_api') - hide_access_denials_flag = WaffleFlag( - waffle_namespace=course_blocks_namespace, - flag_name=u'hide_access_denials', - flag_undefined_default=False - ) - if hide_access_denials_flag.is_enabled(): + if HIDE_ACCESS_DENIALS_FLAG.is_enabled(): hide_access_denials = True # create ordered list of transformers, adding BlocksAPITransformer at end. diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index af1b55491e..cc7ba99694 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -13,6 +13,7 @@ from mock import patch from openedx.core.djangoapps.content.block_structure.api import clear_course_from_cache from openedx.core.djangoapps.content.block_structure.config import STORAGE_BACKING_FOR_CACHE, waffle +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from student.tests.factories import UserFactory from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase @@ -20,6 +21,7 @@ from xmodule.modulestore.tests.factories import SampleCourseFactory, check_mongo from xmodule.modulestore.tests.sample_courses import BlockInfo from ..api import get_blocks +from ..toggles import ENABLE_VIDEO_URL_REWRITE class TestGetBlocks(SharedModuleStoreTestCase): @@ -131,6 +133,7 @@ class TestGetBlocksMobileHack(SharedModuleStoreTestCase): BlockInfo('full_sequential', 'sequential', {}, [ BlockInfo('full_vertical', 'vertical', {}, [ BlockInfo('html', 'html', {}, []), + BlockInfo('sample_video', 'video', {}, []) ]), ]), ]) @@ -156,6 +159,39 @@ class TestGetBlocksMobileHack(SharedModuleStoreTestCase): assert_containment = self.assertNotIn if is_mobile else self.assertIn assert_containment(str(empty_container_key), blocks['blocks']) + @patch('xmodule.video_module.VideoBlock.student_view_data') + @ddt.data( + True, False + ) + def test_video_urls_rewrite(self, waffle_flag_value, video_data_patch): + """ + Verify the video blocks returned have their URL re-written for + encoded videos. + """ + video_data_patch.return_value = { + 'encoded_videos': { + 'hls': { + 'url': 'https://xyz123.cloudfront.net/XYZ123ABC.mp4', + 'file_size': 0 + }, + 'mobile_low': { + 'url': 'https://1234abcd.cloudfront.net/ABCD1234abcd.mp4', + 'file_size': 0 + } + } + } + with override_waffle_flag(ENABLE_VIDEO_URL_REWRITE, waffle_flag_value): + blocks = get_blocks( + self.request, self.course.location, requested_fields=['student_view_data'], student_view_data=['video'] + ) + video_block_key = str(self.course.id.make_usage_key('video', 'sample_video')) + video_block_data = blocks['blocks'][video_block_key] + for video_data in six.itervalues(video_block_data['student_view_data']['encoded_videos']): + if waffle_flag_value: + self.assertNotIn('cloudfront', video_data['url']) + else: + self.assertIn('cloudfront', video_data['url']) + @ddt.ddt class TestGetBlocksQueryCountsBase(SharedModuleStoreTestCase): @@ -208,7 +244,7 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): self._get_blocks( course, expected_mongo_queries=0, - expected_sql_queries=12 if with_storage_backing else 11, + expected_sql_queries=14 if with_storage_backing else 13, ) @ddt.data( @@ -225,9 +261,9 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): clear_course_from_cache(course.id) if with_storage_backing: - num_sql_queries = 22 + num_sql_queries = 24 else: - num_sql_queries = 12 + num_sql_queries = 14 self._get_blocks( course, diff --git a/lms/djangoapps/course_api/blocks/toggles.py b/lms/djangoapps/course_api/blocks/toggles.py new file mode 100644 index 0000000000..ac62f803c2 --- /dev/null +++ b/lms/djangoapps/course_api/blocks/toggles.py @@ -0,0 +1,39 @@ +""" +Toggles for Course API. +""" + +from __future__ import absolute_import +from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag, WaffleFlag, WaffleFlagNamespace + + +COURSE_BLOCKS_API_NAMESPACE = WaffleFlagNamespace(name=u'course_blocks_api') + +# Waffle flag to hide access denial message. +# .. toggle_name: HIDE_ACCESS_DENIALS_FLAG +# .. toggle_type: waffle_flag +# .. toggle_default: False +# .. toggle_description: ?? +# .. toggle_category: course api +# .. toggle_use_cases: incremental_release, open_edx +# .. toggle_creation_date: 2019-04-10 +# .. toggle_expiration_date: ?? +HIDE_ACCESS_DENIALS_FLAG = WaffleFlag( + waffle_namespace=COURSE_BLOCKS_API_NAMESPACE, + flag_name=u'hide_access_denials', + flag_undefined_default=False +) + +# Waffle course override to rewrite video URLs for videos that have encodings available. +# .. toggle_name: ENABLE_VIDEO_URL_REWRITE +# .. toggle_type: waffle_flag +# .. toggle_default: False +# .. toggle_description: Controlled rollout for video URL re-write utility to serve videos from edX CDN. +# .. toggle_category: course api +# .. toggle_use_cases: incremental_release +# .. toggle_creation_date: 2019-09-24 +# .. toggle_expiration_date: ?? +ENABLE_VIDEO_URL_REWRITE = CourseWaffleFlag( + waffle_namespace=COURSE_BLOCKS_API_NAMESPACE, + flag_name="enable_video_url_rewrite", + flag_undefined_default=False +) diff --git a/lms/djangoapps/course_api/blocks/transformers/blocks_api.py b/lms/djangoapps/course_api/blocks/transformers/blocks_api.py index a85d043559..e90682fd76 100644 --- a/lms/djangoapps/course_api/blocks/transformers/blocks_api.py +++ b/lms/djangoapps/course_api/blocks/transformers/blocks_api.py @@ -9,6 +9,8 @@ from .block_counts import BlockCountsTransformer from .block_depth import BlockDepthTransformer from .navigation import BlockNavigationTransformer from .student_view import StudentViewTransformer +from .video_urls import VideoBlockURLTransformer +from ..toggles import ENABLE_VIDEO_URL_REWRITE class BlocksAPITransformer(BlockStructureTransformer): @@ -22,7 +24,9 @@ class BlocksAPITransformer(BlockStructureTransformer): BlockDepthTransformer BlockNavigationTransformer - Note: BlockDepthTransformer must be executed before BlockNavigationTransformer. + Note: + * BlockDepthTransformer must be executed before BlockNavigationTransformer. + * StudentViewTransformer must be executed before VideoBlockURLTransformer. """ WRITE_VERSION = 1 @@ -65,3 +69,5 @@ class BlocksAPITransformer(BlockStructureTransformer): BlockCountsTransformer(self.block_types_to_count).transform(usage_info, block_structure) BlockDepthTransformer(self.depth).transform(usage_info, block_structure) BlockNavigationTransformer(self.nav_depth).transform(usage_info, block_structure) + if ENABLE_VIDEO_URL_REWRITE.is_enabled(block_structure.root_block_usage_key.course_key): + VideoBlockURLTransformer().transform(usage_info, block_structure) diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_video_urls.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_video_urls.py new file mode 100644 index 0000000000..925654333c --- /dev/null +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_video_urls.py @@ -0,0 +1,123 @@ +""" +Tests for VideoBlockURLTransformer. +""" +from __future__ import absolute_import + +import mock +import six + +from openedx.core.djangoapps.content.block_structure.factory import BlockStructureFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import ToyCourseFactory + +from ..student_view import StudentViewTransformer +from ..video_urls import VideoBlockURLTransformer + + +class TestVideoBlockURLTransformer(ModuleStoreTestCase): + """ + Test the URL re-write for video URLs using VideoBlockURLTransformer. + """ + + def setUp(self): + super(TestVideoBlockURLTransformer, self).setUp() + self.course_key = ToyCourseFactory.create().id + self.course_usage_key = self.store.make_course_usage_key(self.course_key) + self.block_structure = BlockStructureFactory.create_from_modulestore(self.course_usage_key, self.store) + + def get_pre_transform_data(self, block_key): + """ + Return the student view data before the transformation for given video block. + """ + video_block = self.block_structure.get_xblock(block_key) + return video_block.student_view_data() + + def change_encoded_videos_presentation(self, encoded_videos): + """ + Relocate url data in new dictionary for pre & post transformation data comparison. + """ + video_urls = {} + for video_format, video_data in six.iteritems(encoded_videos): + video_urls[video_format] = video_data['url'] + return video_urls + + def get_post_transform_data(self, block_key): + """ + Return the block's student view data after transformation. + """ + return self.block_structure.get_transformer_block_field( + block_key, StudentViewTransformer, StudentViewTransformer.STUDENT_VIEW_DATA + ) + + def collect_and_transform(self): + """ + Perform transformer operations. + """ + StudentViewTransformer.collect(self.block_structure) + self.block_structure._collect_requested_xblock_fields() # pylint: disable=protected-access + StudentViewTransformer(['video']).transform( + usage_info=None, + block_structure=self.block_structure, + ) + VideoBlockURLTransformer().transform( + usage_info=None, + block_structure=self.block_structure, + ) + + @mock.patch('xmodule.video_module.VideoBlock.student_view_data') + def test_rewrite_for_encoded_videos(self, mock_video_data): + """ + Test that video URLs for videos with available encodings + are re-written successfully by VideoBlockURLTransformer. + """ + mock_video_data.return_value = { + 'encoded_videos': { + 'hls': { + 'url': 'https://xyz123.cloudfront.net/XYZ123ABC.mp4', + 'file_size': 0 + }, + 'mobile_low': { + 'url': 'https://1234abcd.cloudfront.net/ABCD1234abcd.mp4', + 'file_size': 0 + } + } + } + video_block_key = self.course_key.make_usage_key('video', 'sample_video') + pre_transform_data = self.get_pre_transform_data(video_block_key) + pre_transform_data = self.change_encoded_videos_presentation(pre_transform_data['encoded_videos']) + + self.collect_and_transform() + post_transform_data = self.get_post_transform_data(video_block_key) + post_transform_data = self.change_encoded_videos_presentation(post_transform_data['encoded_videos']) + + for video_format, video_url in six.iteritems(post_transform_data): + self.assertNotEqual(pre_transform_data[video_format], video_url) + + @mock.patch('xmodule.video_module.VideoBlock.student_view_data') + def test_no_rewrite_for_third_party_vendor(self, mock_video_data): + """ + Test that video URLs aren't re-written for the videos + being served from third party vendors or CDN. + """ + mock_video_data.return_value = { + 'encoded_videos': { + 'youtube': { + 'url': 'https://www.youtube.com/watch?v=abcd1234', + 'file_size': 0 + }, + 'fallback': { + 'url': 'https://1234abcd.third_part_cdn.com/ABCD1234abcd.mp4', + 'file_size': 0 + } + } + } + video_block_key = self.course_key.make_usage_key('video', 'sample_video') + pre_transform_data = self.get_pre_transform_data(video_block_key) + pre_transform_data = self.change_encoded_videos_presentation(pre_transform_data['encoded_videos']) + + self.collect_and_transform() + post_transform_data = self.get_post_transform_data(video_block_key) + post_transform_data = self.change_encoded_videos_presentation(post_transform_data['encoded_videos']) + + for video_format, video_url in six.iteritems(post_transform_data): + self.assertEqual(pre_transform_data[video_format], video_url) diff --git a/lms/djangoapps/course_api/blocks/transformers/video_urls.py b/lms/djangoapps/course_api/blocks/transformers/video_urls.py new file mode 100644 index 0000000000..6835cdb8d3 --- /dev/null +++ b/lms/djangoapps/course_api/blocks/transformers/video_urls.py @@ -0,0 +1,53 @@ +""" +Video block URL Transformer +""" +from __future__ import absolute_import + +import six +from django.conf import settings + +from xmodule.video_module.video_utils import rewrite_video_url +from openedx.core.djangoapps.content.block_structure.transformer import BlockStructureTransformer + +from .student_view import StudentViewTransformer + + +class VideoBlockURLTransformer(BlockStructureTransformer): + """ + Transformer to re-write video urls for the encoded videos + to server content from edx-video. + """ + + @classmethod + def name(cls): + return "video_url" + + WRITE_VERSION = 1 + READ_VERSION = 1 + CDN_URL = getattr(settings, 'VIDEO_CDN_URL', {}).get('default', 'https://edx-video.net') + VIDEO_FORMAT_EXCEPTIONS = ['youtube', 'fallback'] + + def transform(self, usage_info, block_structure): + """ + Re-write all the video blocks' encoded videos URLs. + + For the encoded_videos dictionary, all the available video format URLs + will be re-written to serve the videos from edx-video.net + with YouTube and fallback URL as an exception. Fallback URL is an exception + because when there is no video profile data in VAL, the user specified + data from all_sources is taken, which can be URL from any CDN. + """ + for block_key in block_structure.topological_traversal( + filter_func=lambda block_key: block_key.block_type == 'video', + yield_descendants_of_unyielded=True, + ): + student_view_data = block_structure.get_transformer_block_field( + block_key, StudentViewTransformer, StudentViewTransformer.STUDENT_VIEW_DATA + ) + if not student_view_data: + return + encoded_videos = student_view_data['encoded_videos'] + for video_format, video_data in six.iteritems(encoded_videos): + if video_format in self.VIDEO_FORMAT_EXCEPTIONS: + continue + video_data['url'] = rewrite_video_url(self.CDN_URL, video_data['url'])