Skip to content

E
edx-platform
Commit 577749ff by Vik Paruchuri
Options

Merge pull request #1163 from edx/fix/vik/image-upload 

Streamling, test, and fix issues with image uploading
parents 1c6e0057 19fb2017
Showing with 175 additions and 293 deletions
common/lib/xmodule/xmodule/combined_open_ended_module.py
......@@ -306,7 +306,7 @@ class CombinedOpenEndedFields(object):
)
peer_grade_finished_submissions_when_none_pending = Boolean(
display_name='Allow "overgrading" of peer submissions',
help=("Allow students to peer grade submissions that already have the requisite number of graders, "
help=("EXPERIMENTAL FEATURE. Allow students to peer grade submissions that already have the requisite number of graders, "
"but ONLY WHEN all submissions they are eligible to grade already have enough graders. "
"This is intended for use when settings for `Required Peer Grading` > `Peer Graders per Response`"),
default=False,
......
common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee
......@@ -372,7 +372,8 @@ class @CombinedOpenEnded
answer_area_div = @$(@answer_area_div_sel)
answer_area_div.html(response.student_response)
else
@can_upload_files = pre_can_upload_files
@submit_button.show()
@submit_button.attr('disabled', false)
@gentle_alert response.error
confirm_save_answer: (event) =>
......@@ -385,23 +386,27 @@ class @CombinedOpenEnded
event.preventDefault()
@answer_area.attr("disabled", true)
max_filesize = 2*1000*1000 #2MB
pre_can_upload_files = @can_upload_files
if @child_state == 'initial'
files = ""
valid_files_attached = false
if @can_upload_files == true
files = @$(@file_upload_box_sel)[0].files[0]
if files != undefined
valid_files_attached = true
if files.size > max_filesize
@can_upload_files = false
files = ""
else
@can_upload_files = false
# Don't submit the file in the case of it being too large, deal with the error locally.
@submit_button.show()
@submit_button.attr('disabled', false)
@gentle_alert "You are trying to upload a file that is too large for our system. Please choose a file under 2MB or paste a link to it into the answer box."
return
fd = new FormData()
fd.append('student_answer', @answer_area.val())
fd.append('student_file', files)
fd.append('can_upload_files', @can_upload_files)
fd.append('valid_files_attached', valid_files_attached)
that=this
settings =
type: "POST"
data: fd
......
common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_image_submission.py deleted 100644 → 0
"""
This contains functions and classes used to evaluate if images are acceptable (do not show improper content, etc), and
to send them to S3.
"""
try:
from PIL import Image
ENABLE_PIL = True
except:
ENABLE_PIL = False
from urlparse import urlparse
import requests
from boto.s3.connection import S3Connection
from boto.s3.key import Key
import logging
log = logging.getLogger(__name__)
#Domains where any image linked to can be trusted to have acceptable content.
TRUSTED_IMAGE_DOMAINS = [
'wikipedia',
'edxuploads.s3.amazonaws.com',
'wikimedia',
]
#Suffixes that are allowed in image urls
ALLOWABLE_IMAGE_SUFFIXES = [
'jpg',
'png',
'gif',
'jpeg'
]
#Maximum allowed dimensions (x and y) for an uploaded image
MAX_ALLOWED_IMAGE_DIM = 2000
#Dimensions to which image is resized before it is evaluated for color count, etc
MAX_IMAGE_DIM = 150
#Maximum number of colors that should be counted in ImageProperties
MAX_COLORS_TO_COUNT = 16
#Maximum number of colors allowed in an uploaded image
MAX_COLORS = 400
class ImageProperties(object):
"""
Class to check properties of an image and to validate if they are allowed.
"""
def __init__(self, image_data):
"""
Initializes class variables
@param image: Image object (from PIL)
@return: None
"""
self.image = Image.open(image_data)
image_size = self.image.size
self.image_too_large = False
if image_size[0] > MAX_ALLOWED_IMAGE_DIM or image_size[1] > MAX_ALLOWED_IMAGE_DIM:
self.image_too_large = True
if image_size[0] > MAX_IMAGE_DIM or image_size[1] > MAX_IMAGE_DIM:
self.image = self.image.resize((MAX_IMAGE_DIM, MAX_IMAGE_DIM))
self.image_size = self.image.size
def count_colors(self):
"""
Counts the number of colors in an image, and matches them to the max allowed
@return: boolean true if color count is acceptable, false otherwise
"""
colors = self.image.getcolors(MAX_COLORS_TO_COUNT)
if colors is None:
color_count = MAX_COLORS_TO_COUNT
else:
color_count = len(colors)
too_many_colors = (color_count <= MAX_COLORS)
return too_many_colors
def check_if_rgb_is_skin(self, rgb):
"""
Checks if a given input rgb tuple/list is a skin tone
@param rgb: RGB tuple
@return: Boolean true false
"""
colors_okay = False
try:
r = rgb[0]
g = rgb[1]
b = rgb[2]
check_r = (r > 60)
check_g = (r * 0.4) < g < (r * 0.85)
check_b = (r * 0.2) < b < (r * 0.7)
colors_okay = check_r and check_b and check_g
except:
pass
return colors_okay
def get_skin_ratio(self):
"""
Gets the ratio of skin tone colors in an image
@return: True if the ratio is low enough to be acceptable, false otherwise
"""
colors = self.image.getcolors(MAX_COLORS_TO_COUNT)
is_okay = True
if colors is not None:
skin = sum([count for count, rgb in colors if self.check_if_rgb_is_skin(rgb)])
total_colored_pixels = sum([count for count, rgb in colors])
bad_color_val = float(skin) / total_colored_pixels
if bad_color_val > .4:
is_okay = False
return is_okay
def run_tests(self):
"""
Does all available checks on an image to ensure that it is okay (size, skin ratio, colors)
@return: Boolean indicating whether or not image passes all checks
"""
image_is_okay = False
try:
#image_is_okay = self.count_colors() and self.get_skin_ratio() and not self.image_too_large
image_is_okay = not self.image_too_large
except:
log.exception("Could not run image tests.")
if not ENABLE_PIL:
image_is_okay = True
#log.debug("Image OK: {0}".format(image_is_okay))
return image_is_okay
class URLProperties(object):
"""
Checks to see if a URL points to acceptable content. Added to check if students are submitting reasonable
links to the peer grading image functionality of the external grading service.
"""
def __init__(self, url_string):
self.url_string = url_string
def check_if_parses(self):
"""
Check to see if a URL parses properly
@return: success (True if parses, false if not)
"""
success = False
try:
self.parsed_url = urlparse(self.url_string)
success = True
except:
pass
return success
def check_suffix(self):
"""
Checks the suffix of a url to make sure that it is allowed
@return: True if suffix is okay, false if not
"""
good_suffix = False
for suffix in ALLOWABLE_IMAGE_SUFFIXES:
if self.url_string.endswith(suffix):
good_suffix = True
break
return good_suffix
def run_tests(self):
"""
Runs all available url tests
@return: True if URL passes tests, false if not.
"""
url_is_okay = self.check_suffix() and self.check_if_parses()
return url_is_okay
def check_domain(self):
"""
Checks to see if url is from a trusted domain
"""
success = False
for domain in TRUSTED_IMAGE_DOMAINS:
if domain in self.url_string:
success = True
return success
return success
def run_url_tests(url_string):
"""
Creates a URLProperties object and runs all tests
@param url_string: A URL in string format
@return: Boolean indicating whether or not URL has passed all tests
"""
url_properties = URLProperties(url_string)
return url_properties.run_tests()
def run_image_tests(image):
"""
Runs all available image tests
@param image: PIL Image object
@return: Boolean indicating whether or not all tests have been passed
"""
success = False
try:
image_properties = ImageProperties(image)
success = image_properties.run_tests()
except:
log.exception("Cannot run image tests in combined open ended xmodule. May be an issue with a particular image,"
"or an issue with the deployment configuration of PIL/Pillow")
return success
def upload_to_s3(file_to_upload, keyname, s3_interface):
'''
Upload file to S3 using provided keyname.
Returns:
public_url: URL to access uploaded file
'''
#This commented out code is kept here in case we change the uploading method and require images to be
#converted before they are sent to S3.
#TODO: determine if commented code is needed and remove
#im = Image.open(file_to_upload)
#out_im = cStringIO.StringIO()
#im.save(out_im, 'PNG')
try:
conn = S3Connection(s3_interface['access_key'], s3_interface['secret_access_key'])
bucketname = str(s3_interface['storage_bucket_name'])
bucket = conn.create_bucket(bucketname.lower())
k = Key(bucket)
k.key = keyname
k.set_metadata('filename', file_to_upload.name)
k.set_contents_from_file(file_to_upload)
#This commented out code is kept here in case we change the uploading method and require images to be
#converted before they are sent to S3.
#k.set_contents_from_string(out_im.getvalue())
#k.set_metadata("Content-Type", 'images/png')
k.set_acl("public-read")
public_url = k.generate_url(60 * 60 * 24 * 365) # URL timeout in seconds.
return True, public_url
except:
#This is a dev_facing_error
error_message = "Could not connect to S3 to upload peer grading image. Trying to utilize bucket: {0}".format(
bucketname.lower())
log.error(error_message)
return False, error_message
def get_from_s3(s3_public_url):
"""
Gets an image from a given S3 url
@param s3_public_url: The URL where an image is located
@return: The image data
"""
r = requests.get(s3_public_url, timeout=2)
data = r.text
return data
common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py
......@@ -651,15 +651,12 @@ class OpenEndedModule(openendedchild.OpenEndedChild):
return self.out_of_sync_error(data)
# add new history element with answer and empty score and hint.
success, data = self.append_image_to_student_answer(data)
success, error_message, data = self.append_file_link_to_student_answer(data)
if success:
data['student_answer'] = OpenEndedModule.sanitize_html(data['student_answer'])
self.new_history_entry(data['student_answer'])
self.send_to_grader(data['student_answer'], system)
self.change_state(self.ASSESSING)
else:
# This is a student_facing_error
error_message = "There was a problem saving the image in your submission. Please try a different image, or try pasting a link to an image into the answer box."
return {
'success': success,
......
common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py
......@@ -179,14 +179,11 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
error_message = ""
# add new history element with answer and empty score and hint.
success, data = self.append_image_to_student_answer(data)
success, error_message, data = self.append_file_link_to_student_answer(data)
if success:
data['student_answer'] = SelfAssessmentModule.sanitize_html(data['student_answer'])
self.new_history_entry(data['student_answer'])
self.change_state(self.ASSESSING)
else:
# This is a student_facing_error
error_message = "There was a problem saving the image in your submission. Please try a different image, or try pasting a link to an image into the answer box."
return {
'success': success,
......
common/lib/xmodule/xmodule/tests/test_combined_open_ended.py
......@@ -12,7 +12,7 @@ import logging
import unittest
from lxml import etree
from mock import Mock, MagicMock, ANY
from mock import Mock, MagicMock, ANY, patch
from pytz import UTC
from xmodule.open_ended_grading_classes.openendedchild import OpenEndedChild
......@@ -26,7 +26,7 @@ from xmodule.progress import Progress
from xmodule.tests.test_util_open_ended import (
MockQueryDict, DummyModulestore, TEST_STATE_SA_IN,
MOCK_INSTANCE_STATE, TEST_STATE_SA, TEST_STATE_AI, TEST_STATE_AI2, TEST_STATE_AI2_INVALID,
TEST_STATE_SINGLE, TEST_STATE_PE_SINGLE
TEST_STATE_SINGLE, TEST_STATE_PE_SINGLE, MockUploadedFile
)
from xblock.field_data import DictFieldData
......@@ -374,7 +374,7 @@ class OpenEndedModuleTest(unittest.TestCase):
# Submit a student response to the question.
test_module.handle_ajax(
"save_answer",
{"student_answer": submitted_response, "can_upload_files": False, "student_file": None},
{"student_answer": submitted_response},
get_test_system()
)
# Submitting an answer should clear the stored answer.
......@@ -753,7 +753,7 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore):
#Simulate a student saving an answer
html = module.handle_ajax("get_html", {})
module.handle_ajax("save_answer", {"student_answer": self.answer, "can_upload_files": False, "student_file": None})
module.handle_ajax("save_answer", {"student_answer": self.answer})
html = module.handle_ajax("get_html", {})
#Mock a student submitting an assessment
......@@ -902,3 +902,78 @@ class OpenEndedModuleXmlAttemptTest(unittest.TestCase, DummyModulestore):
#Try to reset, should fail because only 1 attempt is allowed
reset_data = json.loads(module.handle_ajax("reset", {}))
self.assertEqual(reset_data['success'], False)
class OpenEndedModuleXmlImageUploadTest(unittest.TestCase, DummyModulestore):
"""
Test if student is able to upload images properly.
"""
problem_location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestionImageUpload"])
answer_text = "Hello, this is my amazing answer."
file_text = "Hello, this is my amazing file."
file_name = "Student file 1"
answer_link = "http://www.edx.org"
autolink_tag = "<a href="
def setUp(self):
self.test_system = get_test_system()
self.test_system.open_ended_grading_interface = None
self.test_system.s3_interface = test_util_open_ended.S3_INTERFACE
self.test_system.xqueue['interface'] = Mock(
send_to_queue=Mock(side_effect=[1, "queued"])
)
self.setup_modulestore(COURSE)
def test_file_upload_fail(self):
"""
Test to see if a student submission without a file attached fails.
"""
module = self.get_module_from_location(self.problem_location, COURSE)
#Simulate a student saving an answer
response = module.handle_ajax("save_answer", {"student_answer": self.answer_text})
response = json.loads(response)
self.assertFalse(response['success'])
self.assertIn('error', response)
@patch(
'xmodule.open_ended_grading_classes.openendedchild.S3Connection',
test_util_open_ended.MockS3Connection
)
@patch(
'xmodule.open_ended_grading_classes.openendedchild.Key',
test_util_open_ended.MockS3Key
)
def test_file_upload_success(self):
"""
Test to see if a student submission with a file is handled properly.
"""
module = self.get_module_from_location(self.problem_location, COURSE)
#Simulate a student saving an answer with a file
response = module.handle_ajax("save_answer", {
"student_answer": self.answer_text,
"valid_files_attached": True,
"student_file": [MockUploadedFile(self.file_name, self.file_text)],
})
response = json.loads(response)
self.assertTrue(response['success'])
self.assertIn(self.file_name, response['student_response'])
self.assertIn(self.autolink_tag, response['student_response'])
def test_link_submission_success(self):
"""
Students can submit links instead of files. Check that the link is properly handled.
"""
module = self.get_module_from_location(self.problem_location, COURSE)
# Simulate a student saving an answer with a link.
response = module.handle_ajax("save_answer", {
"student_answer": "{0} {1}".format(self.answer_text, self.answer_link)
})
response = json.loads(response)
self.assertTrue(response['success'])
self.assertIn(self.answer_link, response['student_response'])
self.assertIn(self.autolink_tag, response['student_response'])
common/lib/xmodule/xmodule/tests/test_self_assessment.py
......@@ -133,7 +133,7 @@ class SelfAssessmentTest(unittest.TestCase):
self.assertEqual(test_module.get_display_answer(), saved_response)
# Submit a student response to the question.
test_module.handle_ajax("save_answer", {"student_answer": submitted_response, "can_upload_files": False, "student_file": None}, get_test_system())
test_module.handle_ajax("save_answer", {"student_answer": submitted_response}, get_test_system())
# Submitting an answer should clear the stored answer.
self.assertEqual(test_module.stored_answer, None)
# Confirm that the answer is stored properly.
......
common/lib/xmodule/xmodule/tests/test_util_open_ended.py
......@@ -2,6 +2,8 @@ from xmodule.modulestore import Location
from xmodule.modulestore.xml import XMLModuleStore
from xmodule.tests import DATA_DIR, get_test_system
from StringIO import StringIO
OPEN_ENDED_GRADING_INTERFACE = {
'url': 'blah/',
'username': 'incorrect',
......@@ -12,11 +14,61 @@ OPEN_ENDED_GRADING_INTERFACE = {
}
S3_INTERFACE = {
'aws_access_key': "",
'aws_secret_key': "",
"aws_bucket_name": "",
'access_key': "",
'secret_access_key': "",
"storage_bucket_name": "",
}
class MockS3Key(object):
"""
Mock an S3 Key object from boto. Used for file upload testing.
"""
def __init__(self, bucket):
pass
def set_metadata(self, key, value):
setattr(self, key, value)
def set_contents_from_file(self, fileobject):
self.data = fileobject.read()
def set_acl(self, acl):
self.set_metadata("acl", acl)
def generate_url(self, timeout):
return "http://www.edx.org/sample_url"
class MockS3Connection(object):
"""
Mock boto S3Connection for testing image uploads.
"""
def __init__(self, access_key, secret_key, **kwargs):
"""
Mock the init call. S3Connection has a lot of arguments, but we don't need them.
"""
pass
def create_bucket(self, bucket_name, **kwargs):
return "edX Bucket"
class MockUploadedFile(object):
"""
Create a mock uploaded file for image submission tests.
value - String data to place into the mock file.
return - A StringIO object that behaves like a file.
"""
def __init__(self, name, value):
self.mock_file = StringIO()
self.mock_file.write(value)
self.name = name
def seek(self, index):
return self.mock_file.seek(index)
def read(self):
return self.mock_file.read()
class MockQueryDict(dict):
"""
......
common/test/data/open_ended/combinedopenended/SampleQuestionImageUpload.xml 0 → 100644
<combinedopenended attempts="1" display_name = "Humanities Question -- Machine Assessed" accept_file_upload="True">
<rubric>
<rubric>
<category>
<description>Writing Applications</description>
<option> The essay loses focus, has little information or supporting details, and the organization makes it difficult to follow.</option>
<option> The essay presents a mostly unified theme, includes sufficient information to convey the theme, and is generally organized well.</option>
</category>
<category>
<description> Language Conventions </description>
<option> The essay demonstrates a reasonable command of proper spelling and grammar. </option>
<option> The essay demonstrates superior command of proper spelling and grammar.</option>
</category>
</rubric>
</rubric>
<prompt>
<h4>Censorship in the Libraries</h4>
<p>"All of us can think of a book that we hope none of our children or any other children have taken off the shelf. But if I have the right to remove that book from the shelf -- that work I abhor -- then you also have exactly the same right and so does everyone else. And then we have no books left on the shelf for any of us." --Katherine Paterson, Author</p>
<p>Write a persuasive essay to a newspaper reflecting your views on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading.</p>
</prompt>
<task>
<selfassessment/>
</task>
</combinedopenended>
\ No newline at end of file
common/test/data/open_ended/course/2012_Fall.xml
......@@ -2,6 +2,7 @@
<chapter url_name="Overview">
<combinedopenended url_name="SampleQuestion"/>
<combinedopenended url_name="SampleQuestion1Attempt"/>
<combinedopenended url_name="SampleQuestionImageUpload"/>
<peergrading url_name="PeerGradingSample"/>
<peergrading url_name="PeerGradingScored"/>
<peergrading url_name="PeerGradingLinked"/>
......
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