From 03489910baa1caac92a4fdd80fcad2108957cc6d Mon Sep 17 00:00:00 2001 From: Usama Sadiq Date: Mon, 19 Jun 2023 15:15:48 +0500 Subject: [PATCH] fix: update s3 connection method (#32492) * fix: update s3 connection method and fixing tests. --- .../contentstore/views/tests/test_videos.py | 49 ++++++++++++++----- cms/djangoapps/contentstore/views/videos.py | 4 +- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_videos.py b/cms/djangoapps/contentstore/views/tests/test_videos.py index c321129e95..44d1ee177b 100644 --- a/cms/djangoapps/contentstore/views/tests/test_videos.py +++ b/cms/djangoapps/contentstore/views/tests/test_videos.py @@ -14,6 +14,7 @@ from unittest.mock import Mock, patch import dateutil.parser import ddt import pytz +from django.test import TestCase from django.conf import settings from django.test.utils import override_settings from django.urls import reverse @@ -27,7 +28,6 @@ from edxval.api import ( get_transcript_preferences, get_video_info ) - from cms.djangoapps.contentstore.models import VideoUploadConfig from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.contentstore.utils import reverse_course_url @@ -47,7 +47,7 @@ from ..videos import ( StatusDisplayStrings, TranscriptProvider, _get_default_video_image_url, - convert_video_status + convert_video_status, storage_service_bucket, storage_service_key ) @@ -210,7 +210,7 @@ class VideoUploadPostTestsMixin: """ @override_settings(AWS_ACCESS_KEY_ID='test_key_id', AWS_SECRET_ACCESS_KEY='test_secret') @patch('boto.s3.key.Key') - @patch('boto.s3.connection.S3Connection') + @patch('cms.djangoapps.contentstore.views.videos.S3Connection') def test_post_success(self, mock_conn, mock_key): files = [ { @@ -467,7 +467,7 @@ class VideosHandlerTestCase( @override_settings(AWS_ACCESS_KEY_ID="test_key_id", AWS_SECRET_ACCESS_KEY="test_secret") @patch("boto.s3.key.Key") - @patch("boto.s3.connection.S3Connection") + @patch("cms.djangoapps.contentstore.views.videos.S3Connection") @ddt.data( ( [ @@ -501,8 +501,7 @@ class VideosHandlerTestCase( """ Test that video upload works correctly against supported and unsupported file formats. """ - bucket = Mock() - mock_conn.return_value = Mock(get_bucket=Mock(return_value=bucket)) + mock_conn.get_bucket = Mock() mock_key_instances = [ Mock( generate_url=Mock( @@ -530,11 +529,12 @@ class VideosHandlerTestCase( self.assertEqual(response['error'], "Request 'files' entry contain unsupported content_type") @override_settings(AWS_ACCESS_KEY_ID='test_key_id', AWS_SECRET_ACCESS_KEY='test_secret') - @patch('boto.s3.connection.S3Connection') + @patch('cms.djangoapps.contentstore.views.videos.S3Connection') def test_upload_with_non_ascii_charaters(self, mock_conn): """ Test that video uploads throws error message when file name contains special characters. """ + mock_conn.get_bucket = Mock() file_name = 'test\u2019_file.mp4' files = [{'file_name': file_name, 'content_type': 'video/mp4'}] @@ -552,10 +552,11 @@ class VideosHandlerTestCase( @override_settings(AWS_ACCESS_KEY_ID='test_key_id', AWS_SECRET_ACCESS_KEY='test_secret', AWS_SECURITY_TOKEN='token') @patch('boto.s3.key.Key') - @patch('boto.s3.connection.S3Connection') + @patch('cms.djangoapps.contentstore.views.videos.S3Connection') @override_waffle_flag(ENABLE_DEVSTACK_VIDEO_UPLOADS, active=True) def test_devstack_upload_connection(self, mock_conn, mock_key): files = [{'file_name': 'first.mp4', 'content_type': 'video/mp4'}] + mock_conn.get_bucket = Mock() mock_key_instances = [ Mock( generate_url=Mock( @@ -579,11 +580,12 @@ class VideosHandlerTestCase( ) @patch('boto.s3.key.Key') - @patch('boto.s3.connection.S3Connection') + @patch('cms.djangoapps.contentstore.views.videos.S3Connection') def test_send_course_to_vem_pipeline(self, mock_conn, mock_key): """ Test that uploads always go to VEM S3 bucket by default. """ + mock_conn.get_bucket = Mock() files = [{'file_name': 'first.mp4', 'content_type': 'video/mp4'}] mock_key_instances = [ Mock( @@ -608,7 +610,7 @@ class VideosHandlerTestCase( @override_settings(AWS_ACCESS_KEY_ID='test_key_id', AWS_SECRET_ACCESS_KEY='test_secret') @patch('boto.s3.key.Key') - @patch('boto.s3.connection.S3Connection') + @patch('cms.djangoapps.contentstore.views.videos.S3Connection') @ddt.data( { 'global_waffle': True, @@ -646,7 +648,7 @@ class VideosHandlerTestCase( 'file_name': 'first.mp4', 'content_type': 'video/mp4', } - mock_conn.return_value = Mock(get_bucket=Mock(return_value=Mock())) + mock_conn.get_bucket = Mock() mock_key_instance = Mock( generate_url=Mock( return_value='http://example.com/url_{}'.format(file_data['file_name']) @@ -1445,7 +1447,7 @@ class TranscriptPreferencesTestCase(VideoUploadTestBase, CourseTestCase): @ddt.unpack @override_settings(AWS_ACCESS_KEY_ID='test_key_id', AWS_SECRET_ACCESS_KEY='test_secret') @patch('boto.s3.key.Key') - @patch('boto.s3.connection.S3Connection') + @patch('cms.djangoapps.contentstore.views.videos.S3Connection') @patch('cms.djangoapps.contentstore.views.videos.get_transcript_preferences') def test_transcript_preferences_metadata(self, transcript_preferences, is_video_transcript_enabled, mock_transcript_preferences, mock_conn, mock_key): @@ -1626,3 +1628,26 @@ class GetVideoFeaturesTestCase( self.assertEqual(response.status_code, 200) self.assertEqual(response.json()[key], is_enabled) + + +class GetStorageBucketTestCase(TestCase): + """ This test just check that connection works and returns the bucket. + It does not involve any mocking and triggers errors if has any import issue. + """ + @override_settings(AWS_ACCESS_KEY_ID='test_key_id', AWS_SECRET_ACCESS_KEY='test_secret') + @override_settings(VIDEO_UPLOAD_PIPELINE={ + "VEM_S3_BUCKET": "vem_test_bucket", "BUCKET": "test_bucket", "ROOT_PATH": "test_root" + }) + def test_storage_bucket(self): + """ get bucket and generate url. It will not hit actual s3.""" + bucket = storage_service_bucket() + edx_video_id = 'dummy_video' + key = storage_service_key(bucket, file_name=edx_video_id) + upload_url = key.generate_url( + KEY_EXPIRATION_IN_SECONDS, + 'PUT', + headers={'Content-Type': 'mp4'} + ) + + self.assertIn("https://vem_test_bucket.s3.amazonaws.com:443/test_root/", upload_url) + self.assertIn(edx_video_id, upload_url) diff --git a/cms/djangoapps/contentstore/views/videos.py b/cms/djangoapps/contentstore/views/videos.py index bc880de4cf..1410faa7a7 100644 --- a/cms/djangoapps/contentstore/views/videos.py +++ b/cms/djangoapps/contentstore/views/videos.py @@ -11,7 +11,7 @@ import logging from contextlib import closing from datetime import datetime, timedelta from uuid import uuid4 - +from boto.s3.connection import S3Connection from boto import s3 from django.conf import settings from django.contrib.auth.decorators import login_required @@ -834,7 +834,7 @@ def storage_service_bucket(): 'aws_secret_access_key': settings.AWS_SECRET_ACCESS_KEY } - conn = s3.connection.S3Connection(**params) + conn = S3Connection(**params) # We don't need to validate our bucket, it requires a very permissive IAM permission # set since behind the scenes it fires a HEAD request that is equivalent to get_all_keys()