diff --git a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py index bd85b52222..3f6af8512e 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py @@ -1,8 +1,8 @@ """ Tests for transcripts_utils. """ - import copy import json +import re import tempfile import textwrap import unittest @@ -15,7 +15,7 @@ from django.conf import settings from django.test.utils import override_settings from django.utils import translation -from cms.djangoapps.contentstore.tests.utils import mock_requests_get +from cms.djangoapps.contentstore.tests.utils import setup_caption_responses from common.djangoapps.student.tests.factories import UserFactory from xmodule.contentstore.content import StaticContent # lint-amnesty, pylint: disable=wrong-import-order from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order @@ -222,7 +222,7 @@ class TestDownloadYoutubeSubs(TestYoutubeSubsBase): def test_success_downloading_subs(self): - response = textwrap.dedent(""" + caption_response_string = textwrap.dedent(""" Test text 1. @@ -233,12 +233,16 @@ class TestDownloadYoutubeSubs(TestYoutubeSubsBase): good_youtube_sub = 'good_id_2' self.clear_sub_content(good_youtube_sub) + language_code = 'en' with patch('xmodule.video_module.transcripts_utils.requests.get') as mock_get: - mock_get.return_value = Mock(status_code=200, text=response, content=response.encode('utf-8')) - # Check transcripts_utils.GetTranscriptsFromYouTubeException not thrown + setup_caption_responses(mock_get, language_code, caption_response_string) transcripts_utils.download_youtube_subs(good_youtube_sub, self.course, settings) - mock_get.assert_any_call('http://video.google.com/timedtext', params={'lang': 'en', 'v': 'good_id_2'}) + self.assertEqual(2, len(mock_get.mock_calls)) + args, kwargs = mock_get.call_args_list[0] + self.assertEqual(args[0], 'https://www.youtube.com/watch?v=good_id_2') + args, kwargs = mock_get.call_args_list[1] + self.assertTrue(re.match(r"^https://www\.youtube\.com/api/timedtext.*", args[0])) def test_subs_for_html5_vid_with_periods(self): """ @@ -256,7 +260,8 @@ class TestDownloadYoutubeSubs(TestYoutubeSubsBase): @patch('xmodule.video_module.transcripts_utils.requests.get') def test_fail_downloading_subs(self, mock_get): - mock_get.return_value = Mock(status_code=404, text='Error 404') + track_status_code = 404 + setup_caption_responses(mock_get, 'en', 'Error 404', track_status_code) bad_youtube_sub = 'BAD_YOUTUBE_ID2' self.clear_sub_content(bad_youtube_sub) @@ -287,20 +292,6 @@ class TestDownloadYoutubeSubs(TestYoutubeSubsBase): self.clear_sub_content(good_youtube_sub) - @patch('xmodule.video_module.transcripts_utils.requests.get', side_effect=mock_requests_get) - def test_downloading_subs_using_transcript_name(self, mock_get): - """ - Download transcript using transcript name in url - """ - good_youtube_sub = 'good_id_2' - self.clear_sub_content(good_youtube_sub) - - transcripts_utils.download_youtube_subs(good_youtube_sub, self.course, settings) - mock_get.assert_any_call( - 'http://video.google.com/timedtext', - params={'lang': 'en', 'v': 'good_id_2', 'name': 'Custom'} - ) - class TestGenerateSubsFromSource(TestDownloadYoutubeSubs): # lint-amnesty, pylint: disable=test-inherits-tests """Tests for `generate_subs_from_source` function.""" @@ -469,20 +460,21 @@ class TestYoutubeTranscripts(unittest.TestCase): """ @patch('xmodule.video_module.transcripts_utils.requests.get') def test_youtube_bad_status_code(self, mock_get): - mock_get.return_value = Mock(status_code=404, text='test') + track_status_code = 404 + setup_caption_responses(mock_get, 'en', 'test', track_status_code) youtube_id = 'bad_youtube_id' with self.assertRaises(transcripts_utils.GetTranscriptsFromYouTubeException): transcripts_utils.get_transcripts_from_youtube(youtube_id, settings, translation) @patch('xmodule.video_module.transcripts_utils.requests.get') def test_youtube_empty_text(self, mock_get): - mock_get.return_value = Mock(status_code=200, text='') + setup_caption_responses(mock_get, 'en', '') youtube_id = 'bad_youtube_id' with self.assertRaises(transcripts_utils.GetTranscriptsFromYouTubeException): transcripts_utils.get_transcripts_from_youtube(youtube_id, settings, translation) def test_youtube_good_result(self): - response = textwrap.dedent(""" + caption_response_string = textwrap.dedent(""" Test text 1. @@ -496,11 +488,17 @@ class TestYoutubeTranscripts(unittest.TestCase): 'text': ['Test text 1.', 'Test text 2.', 'Test text 3.'] } youtube_id = 'good_youtube_id' + language_code = 'en' with patch('xmodule.video_module.transcripts_utils.requests.get') as mock_get: - mock_get.return_value = Mock(status_code=200, text=response, content=response.encode('utf-8')) + setup_caption_responses(mock_get, language_code, caption_response_string) transcripts = transcripts_utils.get_transcripts_from_youtube(youtube_id, settings, translation) + self.assertEqual(transcripts, expected_transcripts) - mock_get.assert_called_with('http://video.google.com/timedtext', params={'lang': 'en', 'v': 'good_youtube_id'}) + self.assertEqual(2, len(mock_get.mock_calls)) + args, kwargs = mock_get.call_args_list[0] + self.assertEqual(args[0], f'https://www.youtube.com/watch?v={youtube_id}') + args, kwargs = mock_get.call_args_list[1] + self.assertTrue(re.match(r"^https://www\.youtube\.com/api/timedtext.*", args[0])) class TestTranscript(unittest.TestCase): diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index fabfb2c75f..b2f6356a2a 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -4,8 +4,6 @@ Utilities for contentstore tests import json -import textwrap -from unittest.mock import Mock from django.conf import settings from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user @@ -19,6 +17,7 @@ from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_MODULESTORE, from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.utils import ProceduralCourseTestMixin from xmodule.modulestore.xml_importer import import_course_from_xml +from xmodule.tests.test_transcripts_utils import YoutubeVideoHTMLResponse from cms.djangoapps.contentstore.utils import reverse_url from common.djangoapps.student.models import Registration @@ -365,33 +364,31 @@ class CourseTestCase(ProceduralCourseTestMixin, ModuleStoreTestCase): self.assertEqual(value, course2_asset_attrs[key]) -def mock_requests_get(*args, **kwargs): +class HTTPGetResponse: """ - Returns mock responses for the youtube API. + Generic object used to return results from a mock patch to an HTTP GET request """ - # pylint: disable=unused-argument - response_transcript_list = """ - - - - - """ - response_transcript = textwrap.dedent(""" - - subs #1 - subs #2 - subs #3 - - """) + def __init__(self, status_code, response_string): + self.status_code = status_code + self.text = response_string + self.content = response_string.encode('utf-8') - if kwargs == {'params': {'lang': 'en', 'v': 'good_id_2'}}: - return Mock(status_code=200, text='') - elif kwargs == {'params': {'type': 'list', 'v': 'good_id_2'}}: - return Mock(status_code=200, text=response_transcript_list, content=response_transcript_list) - elif kwargs == {'params': {'lang': 'en', 'v': 'good_id_2', 'name': 'Custom'}}: - return Mock(status_code=200, text=response_transcript, content=response_transcript) - return Mock(status_code=404, text='') +def setup_caption_responses(mock_get, language_code, caption_response_string, track_status_code=200): + """ + When fetching youtube captions, two calls to requests.get() are required. The first fetches a + captions URL (link) from the video page, applicable to the selected language track. The second + fetches caption timing information from that track's captions URL. + + This helper method assumes that the two operations are performed in order, and is used in conjunction + with mock patch() operations to return appropriate results for each of the two get operations. + """ + caption_link_response = YoutubeVideoHTMLResponse.with_caption_track(language_code) + caption_track_response = HTTPGetResponse(track_status_code, caption_response_string) + mock_get.side_effect = [ + caption_link_response, + caption_track_response, + ] def get_url(handler_name, key_value, key_name='usage_key_string', kwargs=None): diff --git a/cms/djangoapps/contentstore/views/tests/test_transcripts.py b/cms/djangoapps/contentstore/views/tests/test_transcripts.py index b39535856a..fd130917d2 100644 --- a/cms/djangoapps/contentstore/views/tests/test_transcripts.py +++ b/cms/djangoapps/contentstore/views/tests/test_transcripts.py @@ -16,7 +16,7 @@ from django.urls import reverse from edxval.api import create_video from opaque_keys.edx.keys import UsageKey -from cms.djangoapps.contentstore.tests.utils import CourseTestCase, mock_requests_get +from cms.djangoapps.contentstore.tests.utils import CourseTestCase, setup_caption_responses from openedx.core.djangoapps.contentserver.caching import del_cached_content from xmodule.contentstore.content import StaticContent # lint-amnesty, pylint: disable=wrong-import-order from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order @@ -940,7 +940,7 @@ class TestCheckTranscripts(BaseTranscripts): } ) - @patch('xmodule.video_module.transcripts_utils.requests.get', side_effect=mock_requests_get) + @patch('xmodule.video_module.transcripts_utils.requests.get') def test_check_youtube_with_transcript_name(self, mock_get): """ Test that the transcripts are fetched correctly when the the transcript name is set @@ -958,6 +958,7 @@ class TestCheckTranscripts(BaseTranscripts): ] } self.save_subs_to_store(subs, 'good_id_2') + setup_caption_responses(mock_get, 'en', 'caption_response_string') link = reverse('check_transcripts') data = { 'locator': str(self.video_usage_key), @@ -969,10 +970,9 @@ class TestCheckTranscripts(BaseTranscripts): } resp = self.client.get(link, {'data': json.dumps(data)}) - mock_get.assert_any_call( - 'http://video.google.com/timedtext', - params={'lang': 'en', 'v': 'good_id_2', 'name': 'Custom'} - ) + self.assertEqual(2, len(mock_get.mock_calls)) + args, kwargs = mock_get.call_args_list[0] + self.assertEqual(args[0], 'https://www.youtube.com/watch?v=good_id_2') self.assertEqual(resp.status_code, 200) diff --git a/xmodule/tests/test_transcripts_utils.py b/xmodule/tests/test_transcripts_utils.py index 37e63e45fc..f36c22ef32 100644 --- a/xmodule/tests/test_transcripts_utils.py +++ b/xmodule/tests/test_transcripts_utils.py @@ -1,5 +1,11 @@ ''' Tests mechanism for obtaining language-specific transcript links from youtube video pages Note that tests that work with these links are located elsewhere (test_video.py) + +These tests follow the following nomenclature: + - a youtube video page supports one or more caption languages (aka 'tracks') + - embedded in the page's HTML are track descriptors + - among the fields found in a track descriptor is a caption URL (aka caption link) + - use this link to obtain the track's caption data ''' from ..video_module.transcripts_utils import get_transcript_link_from_youtube @@ -15,7 +21,7 @@ YOUTUBE_VIDEO_ID = "z-LoKnweV6w" # {0} - The youtube video ID whose captions you want # {1} - Either \u0026 for use with UTF-8 encoded HTML, or '&' for use with json # {2} - Language code (e.g., "en") -CAPTION_URL_TEMPLATE = "https: //www.youtube.com/api/timedtext?v = {0}{1}caps = asr{1}xoaf = 5{1}hl = {2}{1}\ +CAPTION_URL_TEMPLATE = "https://www.youtube.com/api/timedtext?v = {0}{1}caps = asr{1}xoaf = 5{1}hl = {2}{1}\ ip = 0.0.0.0{1}ipbits = 0{1}expire = 1667281544{1}sparams = ip, ipbits, expire, v, caps, xoaf{1}\ signature = 3A2A34F0A1FB11B3825FF54D4238B6CC415877E8.058892{1}key = yt8{1}kind = asr{1}lang = {2}" @@ -29,6 +35,9 @@ CAPTION_URL_UTF8_ENCODED_TEMPLATE = CAPTION_URL_TEMPLATE.format(YOUTUBE_VIDEO_ID CAPTION_URL_UTF8_DECODED_TEMPLATE = CAPTION_URL_TEMPLATE.format(YOUTUBE_VIDEO_ID, "&", "{0}") # Macro providing the HTML returned by our mock GET operation on the youtube video page +# +# This template is hard-wired for a video with a single language track +# # This is not valid HTML, but that's OK, as we'll only be using it to confirm the regex # search on the 'playerCaptionsTrackListRenderer' subtree. # @@ -38,66 +47,105 @@ CAPTION_URL_UTF8_DECODED_TEMPLATE = CAPTION_URL_TEMPLATE.format(YOUTUBE_VIDEO_ID # Parameterized with # {0} - the URL that obtains the selected video's caption # {1} - Language code (e.g., "en") -YOUTUBE_HTML_TEMPLATE = "HTML content that comes before the captions..." \ +YOUTUBE_HTML_TEMPLATE = "HTML content that comes before the caption tracks..." \ "\"captions\":{{\"playerCaptionsTracklistRenderer\":" \ "{{\"captionTracks\":[{{\"baseUrl\":\"{0}\"," \ "\"name\":{{\"simpleText\":\"(Caption language name in local language)\"}}," \ "\"vssId\":\".{1}\",\"languageCode\":\"{1}\"," \ - "\"isTranslatable\":true}}]}}}}HTML content that comes after the captions..." + "\"isTranslatable\":true}}]}}}}HTML content that comes after the caption tracks ..." class YoutubeVideoHTMLResponse: - '''Generates substitute HTTP GET responses used when mocking the GET operation to a youtube video page''' + """ + Generates substitute HTTP GET responses used when mocking the GET operation to a youtube video page + """ @classmethod - def with_caption_link(cls, language_code): - '''Generates a GET response of HTML with a single caption of the specified language code - language_code = "en" for english - ''' + def with_caption_track(cls, language_code): + """ + Generates a GET response of HTML with a single caption track for the specified + language code language_code = "en" for english + """ caption_link = CAPTION_URL_UTF8_ENCODED_TEMPLATE.format(language_code) - html_with_embedded_link = YOUTUBE_HTML_TEMPLATE.format(caption_link, language_code) - return cls.MockResponse(html_with_embedded_link) + html_with_single_caption_track = YOUTUBE_HTML_TEMPLATE.format(caption_link, language_code) + return cls.MockResponse(html_with_single_caption_track) @classmethod - def with_no_caption_links(cls): - '''Generates a GET response of (invalid) HTML lacking any captions within it. + def with_no_caption_tracks(cls): + """ + Generates a GET response of (invalid) HTML lacking any caption tracks within it. This fake HTML is nevered rendered; it's only intended as a source for a regex search - ''' - return cls.MockResponse("No caption URL info for regex to find here") + """ + html_with_no_caption_tracks = "No caption URL info for regex to find here" + return cls.MockResponse(html_with_no_caption_tracks) + + @classmethod + def with_malformed_caption_track(cls, language_code): + """ + Generates a GET response of HTML with a single caption of the specified + language code language_code = "en" for english + """ + caption_link = CAPTION_URL_UTF8_ENCODED_TEMPLATE.format(language_code) + html_with_single_valid_caption_track = YOUTUBE_HTML_TEMPLATE.format(caption_link, language_code) + html_with_single_malformed_caption_track = \ + html_with_single_valid_caption_track.replace('languageCode', 'bogus_key') + return cls.MockResponse(html_with_single_malformed_caption_track) class MockResponse: - '''An object fit to be returned from a an HTTP GET operation, exposing - a UTF-8 encoded version of the youtube_html input string in its content attribute''' + """ + An object fit to be returned from a an HTTP GET operation, exposing + a UTF-8 encoded version of the youtube_html input string in its content attribute + """ def __init__(self, youtube_html): + self.status_code = 200 self.content = bytearray(youtube_html, 'UTF-8') class TranscriptsUtilsTest(TestCase): - ''' Tests utility fucntions for transcripts (in video_module)''' + """ + Tests utility fucntions for transcripts (in video_module) + """ @mock.patch('requests.get') def test_get_transcript_link_from_youtube(self, mock_get): - '''Happy path test: english caption link returned when video page HTML has one english caption''' + """ + Happy path test: english caption link returned when video page HTML has one english caption + """ language_code = 'en' - mock_get.return_value = YoutubeVideoHTMLResponse.with_caption_link(language_code) + mock_get.return_value = YoutubeVideoHTMLResponse.with_caption_track(language_code) language_specific_caption_link = get_transcript_link_from_youtube(YOUTUBE_VIDEO_ID) self.assertEqual(language_specific_caption_link, CAPTION_URL_UTF8_DECODED_TEMPLATE.format(language_code)) @ mock.patch('requests.get') def test_get_caption_no_english_caption(self, mock_get): - '''No caption link returned when video page HTML contains no caption in English''' + """ + No caption link returned when video page HTML contains no caption in English + """ language_code = 'fr' - mock_get.return_value = YoutubeVideoHTMLResponse.with_caption_link(language_code) + mock_get.return_value = YoutubeVideoHTMLResponse.with_caption_track(language_code) + + english_language_caption_link = get_transcript_link_from_youtube(YOUTUBE_VIDEO_ID) + self.assertIsNone(english_language_caption_link) + + @ mock.patch('requests.get') + def test_get_caption_no_captions_in_HTML(self, mock_get): + """ + No caption link returned when video page HTML contains no captions at all + """ + mock_get.return_value = YoutubeVideoHTMLResponse.with_no_caption_tracks() english_language_caption_link = get_transcript_link_from_youtube(YOUTUBE_VIDEO_ID) self.assertEqual(english_language_caption_link, None) @ mock.patch('requests.get') - def test_get_caption_no_captions_in_HTML(self, mock_get): - ''' No caption link returned when video page HTML contains no captions at all''' - mock_get.return_value = YoutubeVideoHTMLResponse.with_no_caption_links() + def test_get_caption_malformed_caption_locator(self, mock_get): + """ + Caption track provided on video page for the selected language, but with broken syntax + """ + language_code = 'en' + mock_get.return_value = YoutubeVideoHTMLResponse.with_malformed_caption_track(language_code) english_language_caption_link = get_transcript_link_from_youtube(YOUTUBE_VIDEO_ID) - self.assertEqual(english_language_caption_link, None) + self.assertIsNone(english_language_caption_link) diff --git a/xmodule/video_module/transcripts_utils.py b/xmodule/video_module/transcripts_utils.py index ed1b66579e..0b61b97116 100644 --- a/xmodule/video_module/transcripts_utils.py +++ b/xmodule/video_module/transcripts_utils.py @@ -186,7 +186,7 @@ def get_transcript_link_from_youtube(youtube_id): if caption_matched: caption_tracks = json.loads(f'[{caption_matched.group("caption_tracks")}]') for caption in caption_tracks: - if caption["languageCode"] == "en": + if "languageCode" in caption.keys() and caption["languageCode"] == "en": return caption["baseUrl"] return None except ConnectionError: