From 1f5c95ba9af3afff63b62ce0cde4a3c2425a092c Mon Sep 17 00:00:00 2001 From: Alexander Kryklia Date: Mon, 16 Jun 2014 13:13:53 +0300 Subject: [PATCH] Remove unused code when download from youtube. --- .../tests/test_transcripts_utils.py | 88 +++++++++---------- .../contentstore/views/transcripts_ajax.py | 2 +- .../xmodule/video_module/transcripts_utils.py | 66 +++----------- 3 files changed, 58 insertions(+), 98 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py index f0272764b4..8f60ea6af0 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py @@ -164,16 +164,27 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase): number = '999' display_name = 'Test course' + + def clear_sub_content(self, subs_id): + """ + Remove, if subtitle content exists. + """ + filename = 'subs_{0}.srt.sjson'.format(subs_id) + content_location = StaticContent.compute_location(self.course.id, filename) + try: + content = contentstore().find(content_location) + contentstore().delete(content.location) + except NotFoundError: + pass + def clear_subs_content(self, youtube_subs): - """Remove, if subtitles content exists.""" + """ + Remove, if subtitles content exists. + + youtube_subs: dict of '{speed: youtube_id}' format for different speeds. + """ for subs_id in youtube_subs.values(): - filename = 'subs_{0}.srt.sjson'.format(subs_id) - content_location = StaticContent.compute_location(self.course.id, filename) - try: - content = contentstore().find(content_location) - contentstore().delete(content.location) - except NotFoundError: - pass + self.clear_sub_content(subs_id) def setUp(self): self.course = CourseFactory.create( @@ -193,29 +204,22 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase): Test text 3. """) - good_youtube_subs = { - 0.5: 'good_id_1', - 1.0: 'good_id_2', - 2.0: 'good_id_3' - } - self.clear_subs_content(good_youtube_subs) + good_youtube_sub = 'good_id_2' + self.clear_sub_content(good_youtube_sub) with patch('xmodule.video_module.transcripts_utils.requests.get') as mock_get: mock_get.return_value = Mock(status_code=200, text=response, content=response) # Check transcripts_utils.GetTranscriptsFromYouTubeException not thrown - transcripts_utils.download_youtube_subs(good_youtube_subs, self.course, settings) + 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_1'}) mock_get.assert_any_call('http://video.google.com/timedtext', params={'lang': 'en', 'v': 'good_id_2'}) - mock_get.assert_any_call('http://video.google.com/timedtext', params={'lang': 'en', 'v': 'good_id_3'}) - # Check assets status after importing subtitles. - for subs_id in good_youtube_subs.values(): - filename = 'subs_{0}.srt.sjson'.format(subs_id) - content_location = StaticContent.compute_location(self.course.id, filename) - self.assertTrue(contentstore().find(content_location)) + # Check asset status after import of transcript. + filename = 'subs_{0}.srt.sjson'.format(good_youtube_sub) + content_location = StaticContent.compute_location(self.course.id, filename) + self.assertTrue(contentstore().find(content_location)) - self.clear_subs_content(good_youtube_subs) + self.clear_sub_content(good_youtube_sub) def test_subs_for_html5_vid_with_periods(self): """ @@ -235,25 +239,21 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase): mock_get.return_value = Mock(status_code=404, text='Error 404') - bad_youtube_subs = { - 0.5: 'BAD_YOUTUBE_ID1', - 1.0: 'BAD_YOUTUBE_ID2', - 2.0: 'BAD_YOUTUBE_ID3' - } - self.clear_subs_content(bad_youtube_subs) + bad_youtube_sub = 'BAD_YOUTUBE_ID2' + self.clear_sub_content(bad_youtube_sub) + with self.assertRaises(transcripts_utils.GetTranscriptsFromYouTubeException): - transcripts_utils.download_youtube_subs(bad_youtube_subs, self.course, settings) + transcripts_utils.download_youtube_subs(bad_youtube_sub, self.course, settings) - # Check assets status after importing subtitles. - for subs_id in bad_youtube_subs.values(): - filename = 'subs_{0}.srt.sjson'.format(subs_id) - content_location = StaticContent.compute_location( - self.course.id, filename - ) - with self.assertRaises(NotFoundError): - contentstore().find(content_location) + # Check asset status after import of transcript. + filename = 'subs_{0}.srt.sjson'.format(bad_youtube_sub) + content_location = StaticContent.compute_location( + self.course.id, filename + ) + with self.assertRaises(NotFoundError): + contentstore().find(content_location) - self.clear_subs_content(bad_youtube_subs) + self.clear_sub_content(bad_youtube_sub) def test_success_downloading_chinese_transcripts(self): @@ -262,13 +262,11 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase): # Re-enable when `requests.get` is patched using `mock.patch` raise SkipTest - good_youtube_subs = { - 1.0: 'j_jEn79vS3g', # Chinese, utf-8 - } - self.clear_subs_content(good_youtube_subs) + good_youtube_sub = 'j_jEn79vS3g' # Chinese, utf-8 + self.clear_sub_content(good_youtube_sub) # Check transcripts_utils.GetTranscriptsFromYouTubeException not thrown - transcripts_utils.download_youtube_subs(good_youtube_subs, self.course, settings) + transcripts_utils.download_youtube_subs(good_youtube_sub, self.course, settings) # Check assets status after importing subtitles. for subs_id in good_youtube_subs.values(): @@ -278,7 +276,7 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase): ) self.assertTrue(contentstore().find(content_location)) - self.clear_subs_content(good_youtube_subs) + self.clear_sub_content(good_youtube_sub) class TestGenerateSubsFromSource(TestDownloadYoutubeSubs): diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index d3a0add4b7..59b3c9be7b 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -402,7 +402,7 @@ def replace_transcripts(request): return error_response(response, 'YouTube id {} is not presented in request data.'.format(youtube_id)) try: - download_youtube_subs({1.0: youtube_id}, item, settings) + download_youtube_subs(youtube_id, item, settings) except GetTranscriptsFromYouTubeException as e: return error_response(response, e.message) diff --git a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py index 2ec9ce4ec5..1d80f56945 100644 --- a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py +++ b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py @@ -132,67 +132,29 @@ def get_transcripts_from_youtube(youtube_id, settings, i18n): return {'start': sub_starts, 'end': sub_ends, 'text': sub_texts} -def download_youtube_subs(youtube_subs, item, settings): +def download_youtube_subs(youtube_id, video_descriptor, settings): """ Download transcripts from Youtube and save them to assets. Args: - youtube_subs: dictionary of `speed: youtube_id` key:value pairs. - item: video module instance. + youtube_id: str, actual youtube_id of the video. + video_descriptor: video descriptor instance. - Returns: None, if transcripts were successfully downloaded and saved. - Otherwise raises GetTranscriptsFromYouTubeException. + We save transcripts for 1.0 speed, as for other speed conversion is done on front-end. + + Returns: + None, if transcripts were successfully downloaded and saved. + + Raises: + GetTranscriptsFromYouTubeException, if fails. """ - i18n = item.runtime.service(item, "i18n") + i18n = video_descriptor.runtime.service(video_descriptor, "i18n") _ = i18n.ugettext - highest_speed = highest_speed_subs = None - missed_speeds = [] - # Iterate from lowest to highest speed and try to do download transcripts - # from the Youtube service. - for speed, youtube_id in sorted(youtube_subs.iteritems()): - if not youtube_id: - continue - try: - subs = get_transcripts_from_youtube(youtube_id, settings, i18n) - if not subs: # if empty subs are returned - raise GetTranscriptsFromYouTubeException - except GetTranscriptsFromYouTubeException: - missed_speeds.append(speed) - continue + subs = get_transcripts_from_youtube(youtube_id, settings, i18n) + save_subs_to_store(subs, youtube_id, video_descriptor) - save_subs_to_store(subs, youtube_id, item) - - log.info( - "Transcripts for YouTube id %s (speed %s)" - "are downloaded and saved.", youtube_id, speed - ) - - highest_speed = speed - highest_speed_subs = subs - - if not highest_speed: - raise GetTranscriptsFromYouTubeException(_("Can't find any transcripts on the Youtube service.")) - - # When we exit from the previous loop, `highest_speed` and `highest_speed_subs` - # are the transcripts data for the highest speed available on the - # Youtube service. We use the highest speed as main speed for the - # generation other transcripts, cause during calculation timestamps - # for lower speeds we just use multiplication instead of division. - for speed in missed_speeds: # Generate transcripts for missed speeds. - save_subs_to_store( - generate_subs(speed, highest_speed, highest_speed_subs), - youtube_subs[speed], - item - ) - - log.info( - "Transcripts for YouTube id %s (speed %s)" - "are generated from YouTube id %s (speed %s) and saved", - youtube_subs[speed], speed, - youtube_subs[highest_speed], - highest_speed - ) + log.info("Transcripts for youtube_id %s for 1.0 speed are downloaded and saved.", youtube_id) def remove_subs_from_store(subs_id, item, lang='en'):