Add video image support for course videos

.gitignore

@@ -66,3 +66,5 @@ logs/*/*.log*
 .vagrant
 
 venv/
+
+video-images/

AUTHORS

 Christopher Lee <clee@edx.org>
 Mushtaq Ali <mushtaak@gmail.com>
+Muhammad Ammar <mammar@gmail.com>

edxval/admin.py

@@ -3,7 +3,7 @@ Admin file for django app edxval.
 """
 
 from django.contrib import admin
-from .models import Video, Profile, EncodedVideo, Subtitle, CourseVideo
+from .models import Video, Profile, EncodedVideo, Subtitle, CourseVideo, VideoImage
 
 
 class ProfileAdmin(admin.ModelAdmin):  # pylint: disable=C0111
@@ -11,15 +11,18 @@ class ProfileAdmin(admin.ModelAdmin):  # pylint: disable=C0111
     list_display_links = ('id', 'profile_name')
     admin_order_field = 'profile_name'
 
+
 class EncodedVideoInline(admin.TabularInline):  # pylint: disable=C0111
     model = EncodedVideo
 
+
 class CourseVideoInline(admin.TabularInline):  # pylint: disable=C0111
     model = CourseVideo
     extra = 0
     verbose_name = "Course"
     verbose_name_plural = "Courses"
 
+
 class VideoAdmin(admin.ModelAdmin):  # pylint: disable=C0111
     list_display = (
         'id', 'edx_video_id', 'client_video_id', 'duration', 'created', 'status'
@@ -30,6 +33,13 @@ class VideoAdmin(admin.ModelAdmin):  # pylint: disable=C0111
     admin_order_field = 'edx_video_id'
     inlines = [CourseVideoInline, EncodedVideoInline]
 
+
+class VideoImageAdmin(admin.ModelAdmin):
+    model = VideoImage
+    verbose_name = 'Video Image'
+    verbose_name_plural = 'Video Images'
+
 admin.site.register(Profile, ProfileAdmin)
 admin.site.register(Video, VideoAdmin)
 admin.site.register(Subtitle)
+admin.site.register(VideoImage, VideoImageAdmin)

edxval/api.py

 # pylint: disable=E1101
 # -*- coding: utf-8 -*-
 """
-The internal API for VAL. This is not yet stable
+The internal API for VAL.
 """
 import logging
 
 from lxml.etree import Element, SubElement
 from enum import Enum
 
-from django.core.exceptions import ValidationError
+from django.core.exceptions import ValidationError, ObjectDoesNotExist
+from django.core.files.base import ContentFile
 
-from edxval.models import Video, EncodedVideo, CourseVideo, Profile
+from edxval.models import Video, EncodedVideo, CourseVideo, Profile, VideoImage
 from edxval.serializers import VideoSerializer
+from edxval.exceptions import (  # pylint: disable=unused-import
+    ValError,
+    ValInternalError,
+    ValVideoNotFoundError,
+    ValCannotCreateError,
+    ValCannotUpdateError
+)
 
 logger = logging.getLogger(__name__)  # pylint: disable=C0103
 
 
-class ValError(Exception):
-    """
-    An error that occurs during VAL actions.
-
-    This error is raised when the VAL API cannot perform a requested
-    action.
-
-    """
-    pass
-
-
-class ValInternalError(ValError):
-    """
-    An error internal to the VAL API has occurred.
-
-    This error is raised when an error occurs that is not caused by incorrect
-    use of the API, but rather internal implementation of the underlying
-    services.
-
-    """
-    pass
-
-
-class ValVideoNotFoundError(ValError):
-    """
-    This error is raised when a video is not found
-
-    If a state is specified in a call to the API that results in no matching
-    entry in database, this error may be raised.
-
-    """
-    pass
-
-
-class ValCannotCreateError(ValError):
-    """
-    This error is raised when an object cannot be created
-    """
-    pass
-
-
-class ValCannotUpdateError(ValError):
-    """
-    This error is raised when an object cannot be updated
-    """
-    pass
-
-
 class VideoSortField(Enum):
     """An enum representing sortable fields in the Video model"""
     created = "created"
@@ -182,6 +142,40 @@ def update_video_status(edx_video_id, status):
     video.save()
 
 
+def get_course_video_image_url(course_id, edx_video_id):
+    """
+    Returns course video image url or None if no image found
+    """
+    try:
+        video_image = CourseVideo.objects.get(course_id=course_id, video__edx_video_id=edx_video_id).video_image
+        return video_image.image_url()
+    except ObjectDoesNotExist:
+        return None
+
+
+def update_video_image(edx_video_id, course_id, image_data, file_name):
+    """
+    Update video image for an existing video.
+
+    Arguments:
+        image_data (InMemoryUploadedFile): Image data to be saved for a course video.
+
+    Returns:
+        course video image url
+
+    Raises:
+        Raises ValVideoNotFoundError if the CourseVideo cannot be retrieved.
+    """
+    try:
+        course_video = CourseVideo.objects.get(course_id=course_id, video__edx_video_id=edx_video_id)
+    except ObjectDoesNotExist:
+        error_message = u'CourseVideo not found for edx_video_id: {0}'.format(edx_video_id)
+        raise ValVideoNotFoundError(error_message)
+
+    video_image, _ = VideoImage.create_or_update(course_video, file_name, image_data)
+    return video_image.image_url()
+
+
 def create_profile(profile_name):
     """
     Used to create Profile objects in the database
@@ -314,11 +308,7 @@ def get_url_for_profile(edx_video_id, profile):
     return get_urls_for_profiles(edx_video_id, [profile])[profile]
 
 
-def _get_videos_for_filter(
-        video_filter,
-        sort_field=None,
-        sort_dir=SortDirection.asc
-):
+def _get_videos_for_filter(video_filter, sort_field=None, sort_dir=SortDirection.asc):
     """
     Returns a generator expression that contains the videos found, sorted by
     the given field and direction, with ties broken by edx_video_id to ensure a
@@ -333,11 +323,7 @@ def _get_videos_for_filter(
     return (VideoSerializer(video).data for video in videos)
 
 
-def get_videos_for_course(
-    course_id,
-    sort_field=None,
-    sort_dir=SortDirection.asc,
-):
+def get_videos_for_course(course_id, sort_field=None, sort_dir=SortDirection.asc):
     """
     Returns an iterator of videos for the given course id.
 
@@ -352,7 +338,7 @@ def get_videos_for_course(
         total order.
     """
     return _get_videos_for_filter(
-        {"courses__course_id": unicode(course_id), "courses__is_hidden": False},
+        {'courses__course_id': unicode(course_id), 'courses__is_hidden': False},
         sort_field,
         sort_dir,
     )

edxval/exceptions.py

+"""
+VAL Exceptions.
+"""
+
+class ValError(Exception):
+    """
+    An error that occurs during VAL actions.
+
+    This error is raised when the VAL API cannot perform a requested
+    action.
+
+    """
+    pass
+
+
+class ValInternalError(ValError):
+    """
+    An error internal to the VAL API has occurred.
+
+    This error is raised when an error occurs that is not caused by incorrect
+    use of the API, but rather internal implementation of the underlying
+    services.
+
+    """
+    pass
+
+
+class ValVideoNotFoundError(ValError):
+    """
+    This error is raised when a video is not found
+
+    If a state is specified in a call to the API that results in no matching
+    entry in database, this error may be raised.
+
+    """
+    pass
+
+
+class ValCannotCreateError(ValError):
+    """
+    This error is raised when an object cannot be created
+    """
+    pass
+
+
+class ValCannotUpdateError(ValError):
+    """
+    This error is raised when an object cannot be updated
+    """
+    pass

edxval/migrations/0005_videoimage.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.utils.timezone
+import model_utils.fields
+import edxval.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('edxval', '0004_data__add_hls_profile'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='VideoImage',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, verbose_name='created', editable=False)),
+                ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, verbose_name='modified', editable=False)),
+                ('image', edxval.models.CustomizableImageField(null=True, blank=True)),
+                ('course_video', models.OneToOneField(related_name='video_image', to='edxval.CourseVideo')),
+            ],
+            options={
+                'abstract': False,
+            },
+        ),
+    ]

edxval/models.py

@@ -11,13 +11,20 @@ themselves. After these are resolved, errors such as a negative file_size or
 invalid profile_name will be returned.
 """
 
+from contextlib import closing
 import logging
+import os
+from uuid import uuid4
 
 from django.db import models
 from django.dispatch import receiver
 from django.core.validators import MinValueValidator, RegexValidator
 from django.core.urlresolvers import reverse
 
+from model_utils.models import TimeStampedModel
+
+from edxval.utils import video_image_path, get_video_image_storage
+
 logger = logging.getLogger(__name__)  # pylint: disable=C0103
 
 URL_REGEX = r'^[a-zA-Z0-9\-_]*$'
@@ -35,6 +42,7 @@ class ModelFactoryWithValidation(object):
         ret_val = cls(*args, **kwargs)
         ret_val.full_clean()
         ret_val.save()
+        return ret_val
 
     @classmethod
     def get_or_create_with_validation(cls, *args, **kwargs):
@@ -137,6 +145,13 @@ class CourseVideo(models.Model, ModelFactoryWithValidation):
         """
         unique_together = ("course_id", "video")
 
+    def image_url(self):
+        """
+        Return image url for a course video image or None if no image.
+        """
+        if hasattr(self, 'video_image'):
+            return self.video_image.image_url()
+
     def __unicode__(self):
         return self.course_id
 
@@ -155,6 +170,84 @@ class EncodedVideo(models.Model):
     video = models.ForeignKey(Video, related_name="encoded_videos")
 
 
+class CustomizableImageField(models.ImageField):
+    """
+    Subclass of ImageField that allows custom settings to not
+    be serialized (hard-coded) in migrations. Otherwise,
+    migrations include optional settings for storage (such as
+    the storage class and bucket name); we don't want to
+    create new migration files for each configuration change.
+    """
+    def __init__(self, *args, **kwargs):
+        kwargs.update(dict(
+            upload_to=video_image_path,
+            storage=get_video_image_storage(),
+            max_length=500,  # allocate enough for filepath
+            blank=True,
+            null=True
+        ))
+        super(CustomizableImageField, self).__init__(*args, **kwargs)
+
+    def deconstruct(self):
+        """
+        Override base class method.
+        """
+        name, path, args, kwargs = super(CustomizableImageField, self).deconstruct()
+        del kwargs['upload_to']
+        del kwargs['storage']
+        del kwargs['max_length']
+        return name, path, args, kwargs
+
+
+class VideoImage(TimeStampedModel):
+    """
+    Image model for course video.
+    """
+    course_video = models.OneToOneField(CourseVideo, related_name="video_image")
+    image = CustomizableImageField()
+
+    @classmethod
+    def create_or_update(cls, course_video, file_name, image_data=None):
+        """
+        Create a VideoImage object for a CourseVideo.
+
+        Arguments:
+            course_video (CourseVideo): CourseVideo instance
+            file_name (str): File name of the image
+            image_data (InMemoryUploadedFile): Image data to be saved.
+
+        Returns:
+            Returns a tuple of (video_image, created).
+        """
+        video_image, created = cls.objects.get_or_create(course_video=course_video)
+        if image_data:
+            with closing(image_data) as image_file:
+                file_name = '{uuid}{ext}'.format(uuid=uuid4().hex, ext=os.path.splitext(file_name)[1])
+                try:
+                    course_id = course_video.course_id
+                    edx_video_id = course_video.video.edx_video_id
+                    video_image.image.save(file_name, image_file)
+                except Exception:  # pylint: disable=broad-except
+                    logger.exception(
+                        'VAL: Video Image save failed to storage for course_id [%s] and video_id [%s]',
+                        course_id,
+                        edx_video_id
+                    )
+                    raise
+        else:
+            video_image.image.name = file_name
+
+        video_image.save()
+        return video_image, created
+
+    def image_url(self):
+        """
+        Return image url for a course video image.
+        """
+        storage = get_video_image_storage()
+        return storage.url(self.image.name)
+
+
 SUBTITLE_FORMATS = (
     ('srt', 'SubRip'),
     ('sjson', 'SRT JSON')

edxval/serializers.py

@@ -7,7 +7,7 @@ EncodedVideoSerializer which uses the profile_name as it's profile field.
 from rest_framework import serializers
 from rest_framework.fields import IntegerField, DateTimeField
 
-from edxval.models import Profile, Video, EncodedVideo, Subtitle, CourseVideo
+from edxval.models import Profile, Video, EncodedVideo, Subtitle, CourseVideo, VideoImage
 
 
 class EncodedVideoSerializer(serializers.ModelSerializer):
@@ -87,14 +87,27 @@ class CourseSerializer(serializers.RelatedField):
     """
     Field for CourseVideo
     """
-    def to_representation(self, value):
-        return value.course_id
+    def to_representation(self, course_video):
+        """
+        Returns a serializable representation of a CourseVideo instance.
+        """
+        return {
+            course_video.course_id: course_video.image_url()
+        }
 
     def to_internal_value(self, data):
-        if data:
-            course_video = CourseVideo(course_id=data)
-            course_video.full_clean(exclude=["video"])
-            return course_video
+        """
+        Convert data into CourseVideo instance and image filename tuple.
+        """
+        if isinstance(data, basestring):
+            course_id, image = data, None
+        elif  isinstance(data, dict):
+            (course_id, image), = data.items()
+
+        course_video = CourseVideo(course_id=course_id)
+        course_video.full_clean(exclude=["video"])
+
+        return course_video, image
 
 
 class VideoSerializer(serializers.ModelSerializer):
@@ -168,9 +181,12 @@ class VideoSerializer(serializers.ModelSerializer):
 
         # The CourseSerializer will already have converted the course data
         # to CourseVideo models, so we can just set the video and save.
-        for course_video in courses:
+        # Also create VideoImage objects if an image filename is present
+        for course_video, image in courses:
             course_video.video = video
             course_video.save()
+            if image:
+                VideoImage.create_or_update(course_video, image)
 
         return video
 
@@ -200,8 +216,11 @@ class VideoSerializer(serializers.ModelSerializer):
         # Set courses
         # NOTE: for backwards compatibility with the DRF v2 behavior,
         # we do NOT delete existing course videos during the update.
-        for course_video in validated_data.get("courses", []):
+        # Also update VideoImage objects if an image filename is present
+        for course_video, image in validated_data.get("courses", []):
             course_video.video = instance
             course_video.save()
+            if image:
+                VideoImage.create_or_update(course_video, image)
 
         return instance

edxval/settings.py

@@ -130,6 +130,7 @@ INSTALLED_APPS = (
     # Third Party
     'django_nose',
     'rest_framework',
+    'storages',
 
     # Our App
     'edxval',
@@ -178,3 +179,14 @@ LOGGING = {
 # copied from edx-platform
 COURSE_KEY_PATTERN = r'(?P<course_key_string>[^/+]+(/|\+)[^/+]+(/|\+)[^/]+)'
 COURSE_ID_PATTERN = COURSE_KEY_PATTERN.replace('course_key_string', 'course_id')
+
+VIDEO_IMAGE_SETTINGS = dict(
+    # Backend storage
+    # STORAGE_CLASS='storages.backends.s3boto.S3BotoStorage',
+    # STORAGE_KWARGS=dict(bucket='video-image-bucket'),
+    # If you are changing prefix value then update the .gitignore accordingly
+    # so that images created during tests due to upload should be ignored
+    VIDEO_IMAGE_MAX_BYTES=2097152,
+    VIDEO_IMAGE_MIN_BYTES=100,
+    DIRECTORY_PREFIX='video-images/',
+)

edxval/tests/test_api.py

@@ -7,13 +7,14 @@ import mock
 from mock import patch
 from lxml import etree
 
+from django.core.files.images import ImageFile
 from django.test import TestCase
 from django.db import DatabaseError
 from django.core.urlresolvers import reverse
 from rest_framework import status
 from ddt import ddt, data
 
-from edxval.models import Profile, Video, EncodedVideo, CourseVideo
+from edxval.models import Profile, Video, EncodedVideo, CourseVideo, VideoImage
 from edxval import api as api
 from edxval.api import (
     SortDirection,
@@ -1192,3 +1193,82 @@ class VideoStatusUpdateTest(TestCase):
             'fail',
             video.edx_video_id
         )
+
+
+class CourseVideoImageTest(TestCase):
+    """
+    Tests to check course video image related functions works correctly
+    """
+
+    def setUp(self):
+        """
+        Creates video objects for courses
+        """
+        self.course_id = 'test-course'
+        self.course_id2 = 'test-course2'
+        self.video = Video.objects.create(**constants.VIDEO_DICT_FISH)
+        self.edx_video_id = self.video.edx_video_id
+        self.course_video = CourseVideo.objects.create(video=self.video, course_id=self.course_id)
+        self.course_video2 = CourseVideo.objects.create(video=self.video, course_id=self.course_id2)
+        self.image_path1 = 'edxval/tests/data/image.jpg'
+        self.image_path2 = 'edxval/tests/data/edx.jpg'
+        self.image_url = api.update_video_image(
+            self.edx_video_id, self.course_id, ImageFile(open(self.image_path1)), 'image.jpg'
+        )
+        self.image_url2 = api.update_video_image(
+            self.edx_video_id, self.course_id2, ImageFile(open(self.image_path2)), 'image.jpg'
+        )
+
+    def test_update_video_image(self):
+        """
+        Verify that `update_video_image` api function works as expected.
+        """
+        self.assertEqual(self.course_video.video_image.image.name, self.image_url)
+        self.assertEqual(self.course_video2.video_image.image.name, self.image_url2)
+        self.assertEqual(ImageFile(open(self.image_path1)).size, ImageFile(open(self.image_url)).size)
+        self.assertEqual(ImageFile(open(self.image_path2)).size, ImageFile(open(self.image_url2)).size)
+
+    def test_get_course_video_image_url(self):
+        """
+        Verify that `get_course_video_image_url` api function works as expected.
+        """
+        image_url = api.get_course_video_image_url(self.course_id, self.edx_video_id)
+        self.assertEqual(self.image_url, image_url)
+
+    def test_get_course_video_image_url_no_image(self):
+        """
+        Verify that `get_course_video_image_url` api function returns None when no image is found.
+        """
+        self.course_video.video_image.delete()
+        image_url = api.get_course_video_image_url(self.course_id, self.edx_video_id)
+        self.assertIsNone(image_url)
+
+    def test_get_videos_for_course(self):
+        """
+        Verify that `get_videos_for_course` api function has correct course_video_image_url.
+        """
+        video_data_generator = api.get_videos_for_course(self.course_id)
+        video_data = list(video_data_generator)[0]
+        self.assertEqual(video_data['courses'][0]['test-course'], self.image_url)
+
+    def test_get_videos_for_ids(self):
+        """
+        Verify that `get_videos_for_ids` api function returns response with course_video_image_url set to None.
+        """
+        video_data_generator = api.get_videos_for_ids([self.edx_video_id])
+        video_data = list(video_data_generator)[0]
+        self.assertEqual(video_data['courses'][0]['test-course'], self.image_url)
+
+    @patch('edxval.models.logger')
+    def test_create_or_update_logging(self, mock_logger):
+        """
+        Tests correct message is logged when save to storge is failed in `create_or_update`
+        """
+        with self.assertRaises(Exception) as save_exception:  # pylint: disable=unused-variable
+            VideoImage.create_or_update(self.course_video, 'test.jpg', open(self.image_path2))
+
+        mock_logger.exception.assert_called_with(
+            'VAL: Video Image save failed to storage for course_id [%s] and video_id [%s]',
+            self.course_video.course_id,
+            self.course_video.video.edx_video_id
+        )

edxval/tests/test_views.py

@@ -580,7 +580,7 @@ class VideoListTest(APIAuthTestCase):
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
         videos = self.client.get("/edxval/videos/").data
         self.assertEqual(len(videos), 1)
-        self.assertEqual(videos[0]['courses'], [course1, course2])
+        self.assertEqual(videos[0]['courses'], [{course1: None}, {course2: None}])
 
         url = reverse('video-list') + '?course=%s' % course1
         videos = self.client.get(url).data

edxval/utils.py

+"""
+Util methods to be used in api and models.
+"""
+
+from django.conf import settings
+from django.core.files.storage import get_storage_class
+
+
+def video_image_path(video_image_instance, filename):  # pylint:disable=unused-argument
+    """
+    Returns video image path.
+
+    Arguments:
+        video_image_instance (VideoImage): This is passed automatically by models.CustomizableImageField
+        filename (str): name of image file
+    """
+    return u'{}{}'.format(settings.VIDEO_IMAGE_SETTINGS.get('DIRECTORY_PREFIX', ''), filename)
+
+
+def get_video_image_storage():
+    """
+    Return the configured django storage backend.
+    """
+    if hasattr(settings, 'VIDEO_IMAGE_SETTINGS'):
+        return get_storage_class(
+            settings.VIDEO_IMAGE_SETTINGS.get('STORAGE_CLASS'),
+        )(**settings.VIDEO_IMAGE_SETTINGS.get('STORAGE_KWARGS', {}))
+    else:
+        # during edx-platform loading this method gets called but settings are not ready yet
+        # so in that case we will return default(FileSystemStorage) storage class instance
+        return get_storage_class()()

requirements.txt

@@ -4,3 +4,6 @@ enum34==1.0.4
 lxml==3.3.6
 -e git+https://github.com/edx/django-oauth2-provider.git@0.2.7-fork-edx-6a#egg=django-oauth2-provider==0.2.7-fork-edx-6
 -e git+https://github.com/edx/django-rest-framework-oauth.git@f0b503fda8c254a38f97fef802ded4f5fe367f7a#egg=djangorestframework-oauth
+django-storages==1.5.2
+boto==2.46.1
+django-model-utils==2.3.1