Skip to content

E
edx-submissions
Commit 34d6987c by Will Daly
Options

Upgrade djangorestframework to v3.1 

* Upgrade DRF to v3.1
* Use JSONField on the model instead of custom serializer field for the submission answer.
parent 7fce3f8e
Showing with 146 additions and 83 deletions
requirements.txt
......@@ -2,6 +2,7 @@ dogapi==1.2.1
django>=1.4,<1.5
django-extensions==1.5.5
django-model-utils==2.3.1
djangorestframework<2.4
djangorestframework>=3.1,<3.2
jsonfield==1.0.3
pytz==2015.2
South==0.7.6
South==1.0.1
settings.py
......@@ -46,6 +46,7 @@ INSTALLED_APPS = (
# Third party
'django_extensions',
'south',
# Test
'django_nose',
......
setup.py
......@@ -33,7 +33,7 @@ def load_requirements(*requirements_paths):
setup(
name='edx-submissions',
version='0.0.8',
version='0.1.0',
author='edX',
description='An API for creating submissions and scores.',
url='http://github.com/edx/edx-submissions.git',
......
submissions/api.py
......@@ -14,7 +14,7 @@ from django.db import IntegrityError, DatabaseError
from dogapi import dog_stats_api
from submissions.serializers import (
SubmissionSerializer, StudentItemSerializer, ScoreSerializer, JsonFieldError
SubmissionSerializer, StudentItemSerializer, ScoreSerializer
)
from submissions.models import Submission, StudentItem, Score, ScoreSummary, score_set, score_reset
......@@ -177,11 +177,6 @@ def create_submission(student_item_dict, answer, submitted_at=None, attempt_numb
return sub_data
except JsonFieldError:
error_message = u"Could not serialize JSON field in submission {} for student item {}".format(
model_kwargs, student_item_dict
)
raise SubmissionRequestError(msg=error_message)
except DatabaseError:
error_message = u"An error occurred while creating submission {} for student item: {}".format(
model_kwargs,
......
submissions/migrations/0005_raw_answer_to_jsonfield.py 0 → 100644
# -*- coding: utf-8 -*-
from south.utils import datetime_utils as datetime
from south.db import db
from south.v2 import SchemaMigration
from django.db import models
class Migration(SchemaMigration):
def forwards(self, orm):
# For the upgrade to Django Rest Framework v3, we replaced the `raw_answer` TextField
# with an `answer` JsonField that maps to the `raw_answer` DB column.
# This doesn't require any changes to the database schema, but we generate a migration
# anyway so that South knows about the change.
pass
def backwards(self, orm):
pass
models = {
'submissions.score': {
'Meta': {'object_name': 'Score'},
'created_at': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'db_index': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'points_earned': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'}),
'points_possible': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'}),
'reset': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'student_item': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['submissions.StudentItem']"}),
'submission': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['submissions.Submission']", 'null': 'True'})
},
'submissions.scoresummary': {
'Meta': {'object_name': 'ScoreSummary'},
'highest': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'+'", 'to': "orm['submissions.Score']"}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'latest': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'+'", 'to': "orm['submissions.Score']"}),
'student_item': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['submissions.StudentItem']", 'unique': 'True'})
},
'submissions.studentitem': {
'Meta': {'unique_together': "(('course_id', 'student_id', 'item_id'),)", 'object_name': 'StudentItem'},
'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'item_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
'item_type': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'student_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'})
},
'submissions.submission': {
'Meta': {'ordering': "['-submitted_at', '-id']", 'object_name': 'Submission'},
'answer': ('jsonfield.fields.JSONField', [], {'db_column': "'raw_answer'", 'blank': 'True'}),
'attempt_number': ('django.db.models.fields.PositiveIntegerField', [], {}),
'created_at': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'db_index': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'student_item': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['submissions.StudentItem']"}),
'submitted_at': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'db_index': 'True'}),
'uuid': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '36', 'blank': 'True'})
}
}
complete_apps = ['submissions']
submissions/models.py
......@@ -9,7 +9,6 @@ need to then generate a matching migration for it using:
./manage.py schemamigration submissions --auto
"""
import json
import logging
from django.db import models, DatabaseError
......@@ -17,22 +16,23 @@ from django.db.models.signals import post_save
from django.dispatch import receiver, Signal
from django.utils.timezone import now
from django_extensions.db.fields import UUIDField
from jsonfield import JSONField
logger = logging.getLogger(__name__)
# Signal to inform listeners that a score has been changed
score_set = Signal(providing_args=[
'points_possible', 'points_earned', 'anonymous_user_id',
'course_id', 'item_id'
]
)
'points_possible', 'points_earned', 'anonymous_user_id',
'course_id', 'item_id'
])
# Signal to inform listeners that a score has been reset
score_reset = Signal(
providing_args=['anonymous_user_id', 'course_id', 'item_id']
)
class StudentItem(models.Model):
"""Represents a single item for a single course for a single user.
......@@ -99,7 +99,12 @@ class Submission(models.Model):
created_at = models.DateTimeField(editable=False, default=now, db_index=True)
# The answer (JSON-serialized)
raw_answer = models.TextField(blank=True)
# NOTE: previously, this field was a TextField named `raw_answer`.
# Since JSONField is a subclass of TextField, we can use it as a drop-in
# replacement for TextField that performs JSON serialization/deserialization.
# For backwards compatibility, we override the default database column
# name so it continues to use `raw_answer`.
answer = JSONField(blank=True, db_column="raw_answer")
def __repr__(self):
return repr(dict(
......@@ -108,7 +113,7 @@ class Submission(models.Model):
attempt_number=self.attempt_number,
submitted_at=self.submitted_at,
created_at=self.created_at,
raw_answer=self.raw_answer,
answer=self.answer,
))
def __unicode__(self):
......
submissions/serializers.py
"""
Serializers are created to ensure models do not have to be accessed outside the
scope of the Tim APIs.
scope of the submissions API.
"""
import json
from rest_framework import serializers
from rest_framework.fields import Field, DateTimeField, IntegerField
from submissions.models import StudentItem, Submission, Score
class JsonFieldError(Exception):
"""
An error occurred while serializing/deserializing JSON.
class RawField(Field):
"""
pass
Serializer field that does NOT modify its value.
This is useful when the Django model field is handling serialization/deserialization.
For example, `JsonField` already converts its value to JSON internally. If we use
the default DRF text field, the value would be converted to a string, which would then
be encoded as JSON:
class JsonField(serializers.WritableField):
"""
JSON-serializable field.
"""
def to_native(self, obj):
"""
Deserialize the JSON string.
Args:
obj (str): The JSON string stored in the database.
1) field value is {"foo": "bar"} (a dict)
2) DRF's default field implementation converts the dict to a string: "{'foo': 'bar'}"
3) JsonField encodes the string as JSON: '"{\'foo\': \'bar\'}"'
Returns:
JSON-serializable
This is a problem, because when we load the data back from the database, we'll end
up with a string instead of a dictionary!
Raises:
JsonFieldError: The field could not be deserialized.
"""
try:
return json.loads(obj)
except (TypeError, ValueError):
raise JsonFieldError(u"Could not deserialize as JSON: {}".format(obj))
def from_native(self, data):
"""
Serialize an object to JSON.
"""
def to_representation(self, obj):
return obj
Args:
data (JSON-serializable): The data to serialize.
Returns:
str
Raises:
ValueError: The data could not be serialized as JSON.
"""
try:
return json.dumps(data)
except (TypeError, ValueError):
raise JsonFieldError(u"Could not serialize as JSON: {}".format(data))
def to_internal_value(self, data):
return data
class StudentItemSerializer(serializers.ModelSerializer):
......@@ -63,14 +41,37 @@ class StudentItemSerializer(serializers.ModelSerializer):
class SubmissionSerializer(serializers.ModelSerializer):
answer = JsonField(source='raw_answer')
# Django Rest Framework v3 uses the Django setting `DATETIME_FORMAT`
# when serializing datetimes. This differs from v2, which always
# returned a datetime. To preserve the old behavior, we explicitly
# set `format` to None.
# http://www.django-rest-framework.org/api-guide/fields/#datetimefield
submitted_at = DateTimeField(format=None, required=False)
created_at = DateTimeField(format=None, required=False)
# Django Rest Framework v3 apparently no longer validates that
# `PositiveIntegerField`s are positive!
attempt_number = IntegerField(min_value=0)
# Prevent Django Rest Framework from converting the answer (dict or str)
# to a string.
answer = RawField()
def validate_answer(self, value):
"""
Check that the answer is JSON-serializable and not too long.
"""
# Check that the answer is JSON-serializable
try:
serialized = json.dumps(value)
except (ValueError, TypeError):
raise serializers.ValidationError("Answer value must be JSON-serializable")
def validate_answer(self, attrs, source):
"""Check that the answer is within an acceptable size range."""
value = attrs[source]
if len(value) > Submission.MAXSIZE:
# Check the length of the serialized representation
if len(serialized) > Submission.MAXSIZE:
raise serializers.ValidationError("Maximum answer size exceeded.")
return attrs
return value
class Meta:
model = Submission
......@@ -80,15 +81,14 @@ class SubmissionSerializer(serializers.ModelSerializer):
'attempt_number',
'submitted_at',
'created_at',
# Computed
'answer',
)
class ScoreSerializer(serializers.ModelSerializer):
submission_uuid = serializers.Field(source='submission_uuid')
# Ensure that the created_at datetime is not converted to a string.
created_at = DateTimeField(format=None, required=False)
class Meta:
model = Score
......@@ -102,4 +102,3 @@ class ScoreSerializer(serializers.ModelSerializer):
# Computed
'submission_uuid',
)
submissions/tests/test_api.py
import datetime
import copy
from ddt import ddt, file_data
from django.db import DatabaseError
from django.dispatch import Signal
import ddt
from django.db import DatabaseError, connection, transaction
from django.core.cache import cache
from django.test import TestCase
from nose.tools import raises
......@@ -11,7 +10,7 @@ from mock import patch
import pytz
from submissions import api as api
from submissions.models import ScoreSummary, Submission, StudentItem, Score, score_set
from submissions.models import ScoreSummary, Submission, StudentItem, score_set
from submissions.serializers import StudentItemSerializer
STUDENT_ITEM = dict(
......@@ -32,8 +31,11 @@ ANSWER_ONE = u"this is my answer!"
ANSWER_TWO = u"this is my other answer!"
ANSWER_THREE = u'' + 'c' * (Submission.MAXSIZE + 1)
# Test a non-string JSON-serializable answer
ANSWER_DICT = {"text": "foobar"}
@ddt
@ddt.ddt
class TestSubmissionsApi(TestCase):
"""
Testing Submissions
......@@ -45,10 +47,11 @@ class TestSubmissionsApi(TestCase):
"""
cache.clear()
def test_create_submission(self):
submission = api.create_submission(STUDENT_ITEM, ANSWER_ONE)
@ddt.data(ANSWER_ONE, ANSWER_DICT)
def test_create_submission(self, answer):
submission = api.create_submission(STUDENT_ITEM, answer)
student_item = self._get_student_item(STUDENT_ITEM)
self._assert_submission(submission, ANSWER_ONE, student_item.pk, 1)
self._assert_submission(submission, answer, student_item.pk, 1)
def test_create_huge_submission_fails(self):
with self.assertRaises(api.SubmissionRequestError):
......@@ -121,7 +124,6 @@ class TestSubmissionsApi(TestCase):
mock_get.side_effect = DatabaseError("Kaboom!")
api.get_submission("000000000000000")
def test_two_students(self):
api.create_submission(STUDENT_ITEM, ANSWER_ONE)
api.create_submission(SECOND_STUDENT_ITEM, ANSWER_TWO)
......@@ -136,8 +138,7 @@ class TestSubmissionsApi(TestCase):
student_item = self._get_student_item(SECOND_STUDENT_ITEM)
self._assert_submission(submissions[0], ANSWER_TWO, student_item.pk, 1)
@file_data('data/valid_student_items.json')
@ddt.file_data('data/valid_student_items.json')
def test_various_student_items(self, valid_student_item):
api.create_submission(valid_student_item, ANSWER_ONE)
student_item = self._get_student_item(valid_student_item)
......@@ -164,12 +165,13 @@ class TestSubmissionsApi(TestCase):
self._assert_submission(submissions[0], ANSWER_ONE, student_item.pk, 2)
@raises(api.SubmissionRequestError)
@file_data('data/bad_student_items.json')
@ddt.file_data('data/bad_student_items.json')
def test_error_checking(self, bad_student_item):
api.create_submission(bad_student_item, -100)
@raises(api.SubmissionRequestError)
def test_error_checking_submissions(self):
# Attempt number should be >= 0
api.create_submission(STUDENT_ITEM, ANSWER_ONE, None, -1)
@patch.object(Submission.objects, 'filter')
......@@ -183,12 +185,14 @@ class TestSubmissionsApi(TestCase):
api.create_submission(STUDENT_ITEM, datetime.datetime.now())
def test_load_non_json_answer(self):
# This should never happen, if folks are using the public API.
# Create a submission with a raw answer that is NOT valid JSON
submission = api.create_submission(STUDENT_ITEM, ANSWER_ONE)
sub_model = Submission.objects.get(uuid=submission['uuid'])
sub_model.raw_answer = ''
sub_model.save()
# This should never happen, if folks are using the public API.
# Create a submission with a raw answer that is NOT valid JSON
query = "UPDATE submissions_submission SET raw_answer = '}' WHERE id = %s"
connection.cursor().execute(query, [str(sub_model.id)])
transaction.commit_unless_managed()
with self.assertRaises(api.SubmissionInternalError):
api.get_submission(sub_model.uuid)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment