Update Django rest framework

* Upgrade Django Rest Framework to version 3.1
* Add an external library for OAuth authentication (moved out of DRF core)
* Add a test case for updating course IDs associated with a video.
* Point to fork of oauth2 lib

edxval/serializers.py

@@ -5,7 +5,7 @@ Serialization is usually sent through the VideoSerializer which uses the
 EncodedVideoSerializer which uses the profile_name as it's profile field.
 """
 from rest_framework import serializers
-from django.core.exceptions import ValidationError
+from rest_framework.fields import IntegerField, DateTimeField
 
 from edxval.models import Profile, Video, EncodedVideo, Subtitle, CourseVideo
 
@@ -16,7 +16,20 @@ class EncodedVideoSerializer(serializers.ModelSerializer):
 
     Uses the profile_name as it's profile value instead of a Profile object.
     """
-    profile = serializers.SlugRelatedField(slug_field="profile_name")
+    profile = serializers.SlugRelatedField(
+        slug_field="profile_name",
+        queryset=Profile.objects.all()
+    )
+
+    # Django Rest Framework v3 doesn't enforce minimum values for
+    # PositiveIntegerFields, so we need to specify the min value explicitly.
+    bitrate = IntegerField(min_value=0)
+    file_size = IntegerField(min_value=0)
+
+    # Django Rest Framework v3 converts datetimes to unicode by default.
+    # Specify format=None to leave them as datetimes.
+    created = DateTimeField(required=False, format=None)
+    modified = DateTimeField(required=False, format=None)
 
     class Meta:  # pylint: disable=C1001, C0111
         model = EncodedVideo
@@ -44,20 +57,20 @@ class SubtitleSerializer(serializers.ModelSerializer):
     content_url = serializers.CharField(source='get_absolute_url', read_only=True)
     content = serializers.CharField(write_only=True)
 
-    def validate_content(self, attrs, source):
+    def validate(self, data):
         """
         Validate that the subtitle is in the correct format
         """
-        value = attrs[source]
-        if attrs.get('fmt') == 'sjson':
+        value = data.get("content")
+        if data.get("fmt") == "sjson":
             import json
             try:
                 loaded = json.loads(value)
             except ValueError:
                 raise serializers.ValidationError("Not in JSON format")
             else:
-                attrs[source] = json.dumps(loaded)
-        return attrs
+                data["content"] = json.dumps(loaded)
+        return data
 
     class Meta:  # pylint: disable=C1001, C0111
         model = Subtitle
@@ -74,10 +87,10 @@ class CourseSerializer(serializers.RelatedField):
     """
     Field for CourseVideo
     """
-    def to_native(self, value):
+    def to_representation(self, value):
         return value.course_id
 
-    def from_native(self, data):
+    def to_internal_value(self, data):
         if data:
             course_video = CourseVideo(course_id=data)
             course_video.full_clean(exclude=["video"])
@@ -90,10 +103,19 @@ class VideoSerializer(serializers.ModelSerializer):
 
     encoded_videos takes a list of dicts EncodedVideo data.
     """
-    encoded_videos = EncodedVideoSerializer(many=True, allow_add_remove=True)
-    subtitles = SubtitleSerializer(many=True, allow_add_remove=True, required=False)
-    courses = CourseSerializer(many=True, read_only=False)
-    url = serializers.SerializerMethodField('get_url')
+    encoded_videos = EncodedVideoSerializer(many=True)
+    subtitles = SubtitleSerializer(many=True, required=False)
+    courses = CourseSerializer(
+        many=True,
+        read_only=False,
+        required=False,
+        queryset=CourseVideo.objects.all()
+    )
+    url = serializers.SerializerMethodField()
+
+    # Django Rest Framework v3 converts datetimes to unicode by default.
+    # Specify format=None to leave them as datetimes.
+    created = DateTimeField(required=False, format=None)
 
     class Meta:  # pylint: disable=C1001, C0111
         model = Video
@@ -106,33 +128,80 @@ class VideoSerializer(serializers.ModelSerializer):
         """
         return obj.get_absolute_url()
 
-    def restore_fields(self, data, files):
+    def validate(self, data):
         """
-        Overridden function used to check against duplicate profile names.
-
-        Converts a dictionary of data into a dictionary of deserialized fields. Also
-        checks if there are duplicate profile_name(s). If there is, the deserialization
-        is rejected.
+        Check that the video data is valid.
         """
-        reverted_data = {}
-
         if data is not None and not isinstance(data, dict):
-            self._errors['non_field_errors'] = ['Invalid data']
-            return None
+            raise serializers.ValidationError("Invalid data")
 
         try:
             profiles = [ev["profile"] for ev in data.get("encoded_videos", [])]
             if len(profiles) != len(set(profiles)):
-                self._errors['non_field_errors'] = ['Invalid data: duplicate profiles']
+                raise serializers.ValidationError("Invalid data: duplicate profiles")
         except KeyError:
-            raise ValidationError("profile required for deserializing")
+            raise serializers.ValidationError("profile required for deserializing")
         except TypeError:
-            raise ValidationError("profile field needs to be a profile_name (str)")
+            raise serializers.ValidationError("profile field needs to be a profile_name (str)")
 
-        for field_name, field in self.fields.items():
-            field.initialize(parent=self, field_name=field_name)
-            try:
-                field.field_from_native(data, files, field_name, reverted_data)
-            except ValidationError as err:
-                self._errors[field_name] = list(err.messages)
-        return reverted_data
+        return data
+
+    def create(self, validated_data):
+        """
+        Create the video and its nested resources.
+        """
+        courses = validated_data.pop("courses", [])
+        encoded_videos = validated_data.pop("encoded_videos", [])
+        subtitles = validated_data.pop("subtitles", [])
+
+        video = Video.objects.create(**validated_data)
+
+        EncodedVideo.objects.bulk_create(
+            EncodedVideo(video=video, **video_data)
+            for video_data in encoded_videos
+        )
+
+        Subtitle.objects.bulk_create(
+            Subtitle(video=video, **subtitle_data)
+            for subtitle_data in subtitles
+        )
+
+        # 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:
+            course_video.video = video
+            course_video.save()
+
+        return video
+
+    def update(self, instance, validated_data):
+        """
+        Update an existing video resource.
+        """
+        instance.status = validated_data["status"]
+        instance.client_video_id = validated_data["client_video_id"]
+        instance.duration = validated_data["duration"]
+        instance.save()
+
+        # Set encoded videos
+        instance.encoded_videos.all().delete()
+        EncodedVideo.objects.bulk_create(
+            EncodedVideo(video=instance, **video_data)
+            for video_data in validated_data.get("encoded_videos", [])
+        )
+
+        # Set subtitles
+        instance.subtitles.all().delete()
+        Subtitle.objects.bulk_create(
+            Subtitle(video=instance, **subtitle_data)
+            for subtitle_data in validated_data.get("subtitles", [])
+        )
+
+        # 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", []):
+            course_video.video = instance
+            course_video.save()
+
+        return instance

edxval/tests/test_serializers.py

@@ -6,11 +6,7 @@ Tests the serializers for the Video Abstraction Layer
 
 from django.test import TestCase
 
-from edxval.serializers import (
-    EncodedVideoSerializer,
-    VideoSerializer,
-    ValidationError,
-)
+from edxval.serializers import EncodedVideoSerializer, VideoSerializer
 from edxval.models import Profile, Video, EncodedVideo
 from edxval.tests import constants
 
@@ -36,14 +32,19 @@ class SerializerTests(TestCase):
 
         Tests negative inputs for bitrate, file_size in EncodedVideo
         """
-        errors = EncodedVideoSerializer(  # pylint: disable=E1101
-            data=constants.ENCODED_VIDEO_DICT_NEGATIVE_BITRATE).errors
-        self.assertEqual(errors.get('bitrate')[0],
-                         u"Ensure this value is greater than or equal to 0.")
-        errors = EncodedVideoSerializer(  # pylint: disable=E1101
-            data=constants.ENCODED_VIDEO_DICT_NEGATIVE_FILESIZE).errors
-        self.assertEqual(errors.get('file_size')[0],
-                         u"Ensure this value is greater than or equal to 0.")
+        serializer = EncodedVideoSerializer(data=constants.ENCODED_VIDEO_DICT_NEGATIVE_BITRATE)
+        self.assertFalse(serializer.is_valid())
+        self.assertEqual(
+            serializer.errors.get('bitrate')[0],
+            u"Ensure this value is greater than or equal to 0."
+        )
+
+        serializer = EncodedVideoSerializer(data=constants.ENCODED_VIDEO_DICT_NEGATIVE_FILESIZE)
+        self.assertFalse(serializer.is_valid())
+        self.assertEqual(
+            serializer.errors.get('file_size')[0],
+            u"Ensure this value is greater than or equal to 0."
+        )
 
     def test_negative_fields_for_video_serializer(self):
         """
@@ -51,10 +52,12 @@ class SerializerTests(TestCase):
 
         Tests negative inputs for duration in model Video
         """
-        errors = VideoSerializer(  # pylint: disable=E1101
-            data=constants.VIDEO_DICT_NEGATIVE_DURATION).errors
-        self.assertEqual(errors.get('duration')[0],
-                         u"Ensure this value is greater than or equal to 0.")
+        serializer = VideoSerializer(data=constants.VIDEO_DICT_NEGATIVE_DURATION)
+        self.assertFalse(serializer.is_valid())
+        self.assertEqual(
+            serializer.errors.get('duration')[0],
+            u"Ensure this value is greater than or equal to 0."
+        )
 
     def test_non_latin_serialization(self):
         """
@@ -72,15 +75,16 @@ class SerializerTests(TestCase):
         """
         Test the Video model regex validation for edx_video_id field
         """
-        error = VideoSerializer(data=constants.VIDEO_DICT_INVALID_ID).errors  # pylint: disable=E1101
-        message = error.get("edx_video_id")[0]
+        serializer = VideoSerializer(data=constants.VIDEO_DICT_INVALID_ID)
+        self.assertFalse(serializer.is_valid())
+        message = serializer.errors.get("edx_video_id")[0]
         self.assertEqual(
             message,
             u"edx_video_id has invalid characters"
         )
 
     def test_invalid_course_id(self):
-        errors = VideoSerializer(
+        serializer = VideoSerializer(
             data={
                 "edx_video_id": "dummy",
                 "client_video_id": "dummy",
@@ -89,9 +93,10 @@ class SerializerTests(TestCase):
                 "encoded_videos": [],
                 "courses": ["x" * 300],
             }
-        ).errors
+        )
+        self.assertFalse(serializer.is_valid())
         self.assertEqual(
-            errors,
+            serializer.errors,
             {"courses": ["Ensure this value has at most 255 characters (it has 300)."]}
         )
 
@@ -128,8 +133,11 @@ class SerializerTests(TestCase):
             **constants.VIDEO_DICT_FISH
         )
         serializer = VideoSerializer(data=data)
-        with self.assertRaises(ValidationError):
-            serializer.is_valid()
+        self.assertFalse(serializer.is_valid())
+        self.assertEqual(
+            serializer.errors.get("encoded_videos"),
+            [{"profile": ["This field is required."]}]
+        )
 
     def test_wrong_input_type(self):
         """
@@ -137,7 +145,8 @@ class SerializerTests(TestCase):
         """
         data = "hello"
         serializer = VideoSerializer(data=data)
+        self.assertFalse(serializer.is_valid())
         self.assertEqual(
             serializer.errors.get("non_field_errors")[0],
-            "Invalid data"
+            "Invalid data. Expected a dictionary, but got str."
         )

edxval/tests/test_views.py

@@ -486,7 +486,7 @@ class VideoListTest(APIAuthTestCase):
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         self.assertEqual(
             response.data.get("edx_video_id")[0],
-            "Video with this Edx video id already exists."
+            "This field must be unique."
         )
         videos = len(self.client.get("/edxval/videos/").data)
         self.assertEqual(videos, 1)
@@ -641,7 +641,7 @@ class VideoListTest(APIAuthTestCase):
         Tests number of queries for a Video/EncodedVideo(2) pair
         """
         url = reverse('video-list')
-        with self.assertNumQueries(21):
+        with self.assertNumQueries(17):
             self.client.post(url, constants.COMPLETE_SET_FISH, format='json')
 
     def test_queries_for_single_encoded_videos(self):
@@ -649,7 +649,7 @@ class VideoListTest(APIAuthTestCase):
         Tests number of queries for a Video/EncodedVideo(1) pair
                 """
         url = reverse('video-list')
-        with self.assertNumQueries(15):
+        with self.assertNumQueries(14):
             self.client.post(url, constants.COMPLETE_SET_STAR, format='json')
 
 

edxval/views.py

@@ -2,7 +2,8 @@
 Views file for django app edxval.
 """
 from rest_framework import generics
-from rest_framework.authentication import OAuth2Authentication, SessionAuthentication
+from rest_framework.authentication import SessionAuthentication
+from rest_framework_oauth.authentication import OAuth2Authentication
 from rest_framework.permissions import DjangoModelPermissions
 from django.http import HttpResponse
 from django.shortcuts import get_object_or_404

requirements.txt

 django>=1.4,<1.5
-djangorestframework<2.4
+djangorestframework>=3.1,<3.2
 enum34==1.0.4
 lxml==3.3.6
 South==1.0.1
--e git+https://github.com/edx/django-oauth2-provider.git@0.2.7-fork-edx-1#egg=django-oauth2-provider
+-e git+https://github.com/edx/django-oauth2-provider.git@0.2.7-fork-edx-5#egg=django-oauth2-provider
+-e git+https://github.com/edx/django-rest-framework-oauth.git@f0b503fda8c254a38f97fef802ded4f5fe367f7a#egg=djangorestframework-oauth

setup.py

@@ -37,7 +37,7 @@ def load_requirements(*requirements_paths):
 
 setup(
     name='edxval',
-    version='0.0.5',
+    version='0.0.6',
     author='edX',
     url='http://github.com/edx/edx-val',
     description='edx-val',