Add backend video image validations - EDUCATOR-45

cms/djangoapps/contentstore/views/tests/test_videos.py

@@ -12,6 +12,7 @@ import dateutil.parser
 import ddt
 import pytz
 from django.conf import settings
+from django.core.files.uploadedfile import UploadedFile
 from django.test.utils import override_settings
 from edxval.api import create_profile, create_video, get_video_info, get_course_video_image_url
 from mock import Mock, patch
@@ -23,7 +24,8 @@ from contentstore.views.videos import (
     KEY_EXPIRATION_IN_SECONDS,
     StatusDisplayStrings,
     convert_video_status,
-    _get_default_video_image_url
+    _get_default_video_image_url,
+    validate_video_image
 )
 from contentstore.views.videos import KEY_EXPIRATION_IN_SECONDS, StatusDisplayStrings, convert_video_status
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -532,6 +534,7 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase):
         self.assert_video_status(url, edx_video_id, 'Failed')
 
 
+@ddt.ddt
 @patch.dict('django.conf.settings.FEATURES', {'ENABLE_VIDEO_UPLOAD_PIPELINE': True})
 @override_settings(VIDEO_UPLOAD_PIPELINE={'BUCKET': 'test_bucket', 'ROOT_PATH': 'test_root'})
 class VideoImageTestCase(VideoUploadTestBase, CourseTestCase):
@@ -560,18 +563,35 @@ class VideoImageTestCase(VideoUploadTestBase, CourseTestCase):
 
         return val_image_url
 
+    def verify_error_message(self, response, error_message):
+        """
+        Verify that image upload failure gets proper error message.
+
+        Arguments:
+            response: Response object.
+            error_message: Expected error message.
+        """
+        self.assertEqual(response.status_code, 400)
+        response = json.loads(response.content)
+        self.assertIn('error', response)
+        self.assertEqual(response['error'], error_message)
+
     def test_video_image(self):
         """
         Test video image is saved.
         """
         edx_video_id = 'test1'
         video_image_upload_url = self.get_url_for_course_key(self.course.id, {'edx_video_id': edx_video_id})
-        with make_image_file() as image_file:
+        with make_image_file(
+            dimensions=(settings.VIDEO_IMAGE_MIN_WIDTH, settings.VIDEO_IMAGE_MIN_HEIGHT),
+        ) as image_file:
             response = self.client.post(video_image_upload_url, {'file': image_file}, format='multipart')
             image_url1 = self.verify_image_upload_reponse(self.course.id, edx_video_id, response)
 
         # upload again to verify that new image is uploaded successfully
-        with make_image_file() as image_file:
+        with make_image_file(
+            dimensions=(settings.VIDEO_IMAGE_MIN_WIDTH, settings.VIDEO_IMAGE_MIN_HEIGHT),
+        ) as image_file:
             response = self.client.post(video_image_upload_url, {'file': image_file}, format='multipart')
             image_url2 = self.verify_image_upload_reponse(self.course.id, edx_video_id, response)
 
@@ -583,9 +603,27 @@ class VideoImageTestCase(VideoUploadTestBase, CourseTestCase):
         """
         video_image_upload_url = self.get_url_for_course_key(self.course.id, {'edx_video_id': 'test1'})
         response = self.client.post(video_image_upload_url, {})
-        self.assertEqual(response.status_code, 400)
-        response = json.loads(response.content)
-        self.assertEqual(response['error'], 'No file provided for video image')
+        self.verify_error_message(response, 'No file provided for video image')
+
+    def test_invalid_image_file_info(self):
+        """
+        Test that when no file information is provided to validate_video_image, it gives proper error message.
+        """
+        error = validate_video_image({})
+        self.assertEquals(error, 'The image must have name, content type, and size information.')
+
+    def test_currupt_image_file(self):
+        """
+        Test that when corrupt file is provided to validate_video_image, it gives proper error message.
+        """
+        with open(settings.MEDIA_ROOT + '/test-corrupt-image.png', 'w+') as file:
+            image_file = UploadedFile(
+                file,
+                content_type='image/png',
+                size=settings.VIDEO_IMAGE_SETTINGS['VIDEO_IMAGE_MIN_BYTES']
+            )
+            error = validate_video_image(image_file)
+            self.assertEquals(error, 'This image file is corrupted.')
 
     def test_no_video_image(self):
         """
@@ -594,7 +632,9 @@ class VideoImageTestCase(VideoUploadTestBase, CourseTestCase):
         edx_video_id = 'test1'
         get_videos_url = reverse_course_url('videos_handler', self.course.id)
         video_image_upload_url = self.get_url_for_course_key(self.course.id, {'edx_video_id': edx_video_id})
-        with make_image_file() as image_file:
+        with make_image_file(
+            dimensions=(settings.VIDEO_IMAGE_MIN_WIDTH, settings.VIDEO_IMAGE_MIN_HEIGHT),
+        ) as image_file:
             self.client.post(video_image_upload_url, {'file': image_file}, format='multipart')
 
         val_image_url = get_course_video_image_url(course_id=self.course.id, edx_video_id=edx_video_id)
@@ -608,6 +648,171 @@ class VideoImageTestCase(VideoUploadTestBase, CourseTestCase):
             else:
                 self.assertEqual(response_video['course_video_image_url'], None)
 
+    @ddt.data(
+        # Image file type validation
+        (
+            {
+                'extension': '.png'
+            },
+            None
+        ),
+        (
+            {
+                'extension': '.gif'
+            },
+            None
+        ),
+        (
+            {
+                'extension': '.bmp'
+            },
+            None
+        ),
+        (
+            {
+                'extension': '.jpg'
+            },
+            None
+        ),
+        (
+            {
+                'extension': '.jpeg'
+            },
+            None
+        ),
+        (
+            {
+                'extension': '.PNG'
+            },
+            None
+        ),
+        (
+            {
+                'extension': '.tiff'
+            },
+            'This image file type is not supported. Supported file types are {supported_file_formats}.'.format(
+                supported_file_formats=settings.VIDEO_IMAGE_SUPPORTED_FILE_FORMATS.keys()
+            )
+        ),
+        # Image file size validation
+        (
+            {
+                'size': settings.VIDEO_IMAGE_SETTINGS['VIDEO_IMAGE_MAX_BYTES'] + 10
+            },
+            'This image file must be smaller than {image_max_size}.'.format(
+                image_max_size=settings.VIDEO_IMAGE_MAX_FILE_SIZE_MB
+            )
+        ),
+        (
+            {
+                'size': settings.VIDEO_IMAGE_SETTINGS['VIDEO_IMAGE_MIN_BYTES'] - 10
+            },
+            'This image file must be larger than {image_min_size}.'.format(
+                image_min_size=settings.VIDEO_IMAGE_MIN_FILE_SIZE_KB
+            )
+        ),
+        # Image file resolution validation
+        (
+            {
+                'width': settings.VIDEO_IMAGE_MAX_WIDTH,  # 1280x720
+                'height': settings.VIDEO_IMAGE_MAX_HEIGHT
+            },
+            None
+        ),
+        (
+            {
+                'width': 850,  # 16:9
+                'height': 478
+            },
+            None
+        ),
+        (
+            {
+                'width': 940,  # 1.67 ratio, applicable aspect ratio margin of .01
+                'height': 560
+            },
+            None
+        ),
+        (
+            {
+                'width': 1200,  # not 16:9
+                'height': 100
+            },
+            'This image file must have an aspect ratio of {video_image_aspect_ratio_text}.'.format(
+                video_image_aspect_ratio_text=settings.VIDEO_IMAGE_ASPECT_RATIO_TEXT
+            )
+        ),
+        (
+            {
+                'width': settings.VIDEO_IMAGE_MIN_WIDTH + 100,
+                'height': settings.VIDEO_IMAGE_MIN_HEIGHT + 200
+            },
+            'This image file must have an aspect ratio of {video_image_aspect_ratio_text}.'.format(
+                video_image_aspect_ratio_text=settings.VIDEO_IMAGE_ASPECT_RATIO_TEXT
+            )
+        ),
+        # Image file minimum width / height
+        (
+            {
+                'width': 16,  # 16x9
+                'height': 9
+            },
+            'The minimum allowed image resolution is {image_file_min_width}x{image_file_min_height}.'.format(
+                image_file_min_width=settings.VIDEO_IMAGE_MIN_WIDTH,
+                image_file_min_height=settings.VIDEO_IMAGE_MIN_HEIGHT
+            )
+        ),
+        (
+            {
+                'width': settings.VIDEO_IMAGE_MIN_WIDTH - 10,
+                'height': settings.VIDEO_IMAGE_MIN_HEIGHT
+            },
+            'The minimum allowed image resolution is {image_file_min_width}x{image_file_min_height}.'.format(
+                image_file_min_width=settings.VIDEO_IMAGE_MIN_WIDTH,
+                image_file_min_height=settings.VIDEO_IMAGE_MIN_HEIGHT
+            )
+        ),
+        (
+            {
+                'width': settings.VIDEO_IMAGE_MIN_WIDTH,
+                'height': settings.VIDEO_IMAGE_MIN_HEIGHT - 10
+            },
+            'The minimum allowed image resolution is {image_file_min_width}x{image_file_min_height}.'.format(
+                image_file_min_width=settings.VIDEO_IMAGE_MIN_WIDTH,
+                image_file_min_height=settings.VIDEO_IMAGE_MIN_HEIGHT
+            )
+        ),
+        # Image file name validation
+        (
+            {
+                'prefix': u'nøn-åßç¡¡'
+            },
+            'The image file name can only contain letters, numbers, hyphens (-), and underscores (_).'
+        )
+    )
+    @ddt.unpack
+    def test_video_image_validation_message(self, image_data, error_message):
+        """
+        Test video image validation gives proper error message.
+
+        Arguments:
+            image_data (Dict): Specific data to create image file.
+            error_message (String): Error message
+        """
+        edx_video_id = 'test1'
+        video_image_upload_url = self.get_url_for_course_key(self.course.id, {'edx_video_id': edx_video_id})
+        with make_image_file(
+            dimensions=(image_data.get('width', settings.VIDEO_IMAGE_MIN_WIDTH), image_data.get('height', settings.VIDEO_IMAGE_MIN_HEIGHT)),
+            prefix=image_data.get('prefix', 'videoimage'),
+            extension=image_data.get('extension', '.png'),
+            force_size=image_data.get('size', settings.VIDEO_IMAGE_SETTINGS['VIDEO_IMAGE_MIN_BYTES'])
+        ) as image_file:
+            response = self.client.post(video_image_upload_url, {'file': image_file}, format='multipart')
+            if error_message:
+                self.verify_error_message(response, error_message)
+            else:
+                self.verify_image_upload_reponse(self.course.id, edx_video_id, response)
+
 
 @patch.dict("django.conf.settings.FEATURES", {"ENABLE_VIDEO_UPLOAD_PIPELINE": True})
 @override_settings(VIDEO_UPLOAD_PIPELINE={"BUCKET": "test_bucket", "ROOT_PATH": "test_root"})

cms/djangoapps/contentstore/views/videos.py

@@ -16,6 +16,7 @@ from boto import s3
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
 from django.contrib.staticfiles.storage import staticfiles_storage
+from django.core.files.images import get_image_dimensions
 from django.http import HttpResponse, HttpResponseNotFound
 from django.utils.translation import ugettext as _
 from django.utils.translation import ugettext_noop
@@ -153,19 +154,69 @@ def videos_handler(request, course_key_string, edx_video_id=None):
         return videos_post(course, request)
 
 
+def validate_video_image(image_file):
+    """
+    Validates video image file.
+
+    Arguments:
+        image_file: The selected image file.
+
+   Returns:
+        error (String or None): If there is error returns error message otherwise None.
+    """
+    error = None
+
+    if not all(hasattr(image_file, attr) for attr in ['name', 'content_type', 'size']):
+        error = _('The image must have name, content type, and size information.')
+    elif image_file.content_type not in settings.VIDEO_IMAGE_SUPPORTED_FILE_FORMATS.values():
+        error = _('This image file type is not supported. Supported file types are {supported_file_formats}.').format(
+            supported_file_formats=settings.VIDEO_IMAGE_SUPPORTED_FILE_FORMATS.keys()
+        )
+    elif image_file.size > settings.VIDEO_IMAGE_SETTINGS['VIDEO_IMAGE_MAX_BYTES']:
+        error = _('This image file must be smaller than {image_max_size}.').format(
+            image_max_size=settings.VIDEO_IMAGE_MAX_FILE_SIZE_MB
+        )
+    elif image_file.size < settings.VIDEO_IMAGE_SETTINGS['VIDEO_IMAGE_MIN_BYTES']:
+        error = _('This image file must be larger than {image_min_size}.').format(
+            image_min_size=settings.VIDEO_IMAGE_MIN_FILE_SIZE_KB
+        )
+    else:
+        try:
+            image_file_width, image_file_height = get_image_dimensions(image_file)
+        except TypeError:
+            return _('This image file is corrupted.')
+        image_file_aspect_ratio = abs(image_file_width / float(image_file_height) - settings.VIDEO_IMAGE_ASPECT_RATIO)
+        if image_file_aspect_ratio > settings.VIDEO_IMAGE_ASPECT_RATIO_ERROR_MARGIN:
+            error = _('This image file must have an aspect ratio of {video_image_aspect_ratio_text}.').format(
+                video_image_aspect_ratio_text=settings.VIDEO_IMAGE_ASPECT_RATIO_TEXT
+            )
+        elif image_file_width < settings.VIDEO_IMAGE_MIN_WIDTH or image_file_height < settings.VIDEO_IMAGE_MIN_HEIGHT:
+            error = _('The minimum allowed image resolution is {image_file_min_width}x{image_file_min_height}.').format(
+                image_file_min_width=settings.VIDEO_IMAGE_MIN_WIDTH,
+                image_file_min_height=settings.VIDEO_IMAGE_MIN_HEIGHT
+            )
+        else:
+            try:
+                image_file.name.encode('ascii')
+            except UnicodeEncodeError:
+                error = _('The image file name can only contain letters, numbers, hyphens (-), and underscores (_).')
+    return error
+
+
 @expect_json
 @login_required
 @require_POST
 def video_images_handler(request, course_key_string, edx_video_id=None):
     if 'file' not in request.FILES:
-        return JsonResponse({"error": _(u'No file provided for video image')}, status=400)
+        return JsonResponse({'error': _(u'No file provided for video image')}, status=400)
 
     image_file = request.FILES['file']
-    file_name = request.FILES['file'].name
+    error = validate_video_image(image_file)
+    if error:
+        return JsonResponse({'error': error}, status=400)
 
-    # TODO: Image file validation
     with closing(image_file):
-        image_url = update_video_image(edx_video_id, course_key_string, image_file, file_name)
+        image_url = update_video_image(edx_video_id, course_key_string, image_file, image_file.name)
         LOGGER.info(
             'VIDEOS: Video image uploaded for edx_video_id [%s] in course [%s]', edx_video_id, course_key_string
         )

cms/envs/common.py

@@ -1350,3 +1350,20 @@ PROFILE_IMAGE_SIZES_MAP = {
 ###################### VIDEO IMAGE STORAGE ######################
 
 VIDEO_IMAGE_DEFAULT_FILENAME = 'images/video-images/default_video_image.png'
+VIDEO_IMAGE_SUPPORTED_FILE_FORMATS = {
+    '.bmp': 'image/bmp',
+    '.bmp2': 'image/x-ms-bmp',   # PIL gives x-ms-bmp format
+    '.gif': 'image/gif',
+    '.jpg': 'image/jpeg',
+    '.jpeg': 'image/jpeg',
+    '.png': 'image/png'
+}
+VIDEO_IMAGE_MAX_FILE_SIZE_MB = '2 MB'
+VIDEO_IMAGE_MIN_FILE_SIZE_KB = '2 KB'
+VIDEO_IMAGE_MAX_WIDTH = 1280
+VIDEO_IMAGE_MAX_HEIGHT = 720
+VIDEO_IMAGE_MIN_WIDTH = 640
+VIDEO_IMAGE_MIN_HEIGHT = 360
+VIDEO_IMAGE_ASPECT_RATIO = 16 / 9.0
+VIDEO_IMAGE_ASPECT_RATIO_TEXT = '16:9'
+VIDEO_IMAGE_ASPECT_RATIO_ERROR_MARGIN = 0.1

cms/envs/test.py

@@ -338,6 +338,8 @@ INSTALLED_APPS += ('openedx.core.djangoapps.api_admin',)
 
 ########################## VIDEO IMAGE STORAGE ############################
 VIDEO_IMAGE_SETTINGS = dict(
+    VIDEO_IMAGE_MAX_BYTES=2 * 1024 * 1024,    # 2 MB
+    VIDEO_IMAGE_MIN_BYTES=2 * 1024,       # 2 KB
     STORAGE_KWARGS=dict(
         location=MEDIA_ROOT,
         base_url=MEDIA_URL,

lms/envs/common.py

@@ -2567,6 +2567,8 @@ TIME_ZONE_DISPLAYED_FOR_DEADLINES = 'UTC'
 ########################## VIDEO IMAGE STORAGE ############################
 
 VIDEO_IMAGE_SETTINGS = dict(
+    VIDEO_IMAGE_MAX_BYTES=2 * 1024 * 1024,    # 2 MB
+    VIDEO_IMAGE_MIN_BYTES=2 * 1024,       # 2 KB
     # Backend storage
     # STORAGE_CLASS='storages.backends.s3boto.S3BotoStorage',
     # STORAGE_KWARGS=dict(bucket='video-image-bucket'),

openedx/core/djangoapps/profile_images/tests/helpers.py

@@ -11,7 +11,7 @@ from PIL import Image
 
 
 @contextmanager
-def make_image_file(dimensions=(320, 240), extension=".jpeg", force_size=None, orientation=None):
+def make_image_file(dimensions=(320, 240), prefix='tmp', extension='.jpeg', force_size=None, orientation=None):
     """
     Yields a named temporary file created with the specified image type and
     options.
@@ -21,9 +21,13 @@ def make_image_file(dimensions=(320, 240), extension=".jpeg", force_size=None, o
 
     The temporary file will be closed and deleted automatically upon exiting
     the `with` block.
+
+    prefix - To add prefix to random image file name, after adding will be like <custom-prefix><random-name>.png
+            otherwise by default `tmp` is added making file name tmp<random-name>.png.
+
     """
     image = Image.new('RGB', dimensions, "green")
-    image_file = NamedTemporaryFile(suffix=extension)
+    image_file = NamedTemporaryFile(prefix=prefix, suffix=extension)
     try:
         if orientation and orientation in xrange(1, 9):
             exif_bytes = piexif.dump({'0th': {piexif.ImageIFD.Orientation: orientation}})