Merge pull request #1745 from edx/ned/lms-1492

LMS-1492: Convert between webob's cgi.FieldStorage uploaded files to pure file objects.

common/lib/xmodule/xmodule/tests/test_capa_module.py

@@ -8,16 +8,21 @@ Tests of the Capa XModule
 #pylint: disable=C0302
 
 import datetime
-import unittest
-import random
 import json
+import random
+import os
+import textwrap
+import unittest
 
 from mock import Mock, patch
+import webob
 from webob.multidict import MultiDict
 
 import xmodule
+from xmodule.tests import DATA_DIR
 from capa.responsetypes import (StudentInputError, LoncapaProblemError,
                                 ResponseError)
+from capa.xqueue_interface import XQueueInterface
 from xmodule.capa_module import CapaModule, ComplexEncoder
 from xmodule.modulestore import Location
 from xblock.field_data import DictFieldData
@@ -33,42 +38,47 @@ class CapaFactory(object):
     A helper class to create problem modules with various parameters for testing.
     """
 
-    sample_problem_xml = """<?xml version="1.0"?>
-<problem>
+    sample_problem_xml = textwrap.dedent("""\
+        <?xml version="1.0"?>
+        <problem>
             <text>
-    <p>What is pi, to two decimal placs?</p>
+                <p>What is pi, to two decimal places?</p>
             </text>
-<numericalresponse answer="3.14">
-<textline math="1" size="30"/>
-</numericalresponse>
-</problem>
-"""
+        <numericalresponse answer="3.14">
+        <textline math="1" size="30"/>
+        </numericalresponse>
+        </problem>
+    """)
 
     num = 0
 
-    @staticmethod
-    def next_num():
-        CapaFactory.num += 1
-        return CapaFactory.num
+    @classmethod
+    def next_num(cls):
+        cls.num += 1
+        return cls.num
 
-    @staticmethod
-    def input_key():
+    @classmethod
+    def input_key(cls, input_num=2):
         """
         Return the input key to use when passing GET parameters
         """
-        return ("input_" + CapaFactory.answer_key())
+        return ("input_" + cls.answer_key(input_num))
 
-    @staticmethod
-    def answer_key():
+    @classmethod
+    def answer_key(cls, input_num=2):
         """
         Return the key stored in the capa problem answer dict
         """
-        return ("-".join(['i4x', 'edX', 'capa_test', 'problem',
-                         'SampleProblem%d' % CapaFactory.num]) +
-                "_2_1")
+        return (
+            "%s_%d_1" % (
+                "-".join(['i4x', 'edX', 'capa_test', 'problem', 'SampleProblem%d' % cls.num]),
+                input_num,
+            )
+        )
 
-    @staticmethod
-    def create(graceperiod=None,
+    @classmethod
+    def create(cls,
+               graceperiod=None,
                due=None,
                max_attempts=None,
                showanswer=None,
@@ -97,8 +107,8 @@ class CapaFactory(object):
             attempts: also added to instance state.  Will be converted to an int.
         """
         location = Location(["i4x", "edX", "capa_test", "problem",
-                             "SampleProblem{0}".format(CapaFactory.next_num())])
-        field_data = {'data': CapaFactory.sample_problem_xml}
+                             "SampleProblem{0}".format(cls.next_num())])
+        field_data = {'data': cls.sample_problem_xml}
 
         if graceperiod is not None:
             field_data['graceperiod'] = graceperiod
@@ -144,6 +154,47 @@ class CapaFactory(object):
         return module
 
 
+class CapaFactoryWithFiles(CapaFactory):
+    """
+    A factory for creating a Capa problem with files attached.
+    """
+    sample_problem_xml = textwrap.dedent("""\
+        <problem>
+            <coderesponse queuename="BerkeleyX-cs188x">
+                <!-- actual filenames here don't matter for server-side tests,
+                     they are only acted upon in the browser. -->
+                <filesubmission
+                    points="25"
+                    allowed_files="prog1.py prog2.py prog3.py"
+                    required_files="prog1.py prog2.py prog3.py"
+                />
+                <codeparam>
+                    <answer_display>
+                        If you're having trouble with this Project,
+                        please refer to the Lecture Slides and attend office hours.
+                    </answer_display>
+                    <grader_payload>{"project": "p3"}</grader_payload>
+                </codeparam>
+            </coderesponse>
+
+            <customresponse>
+                <text>
+                    If you worked with a partner, enter their username or email address. If you
+                    worked alone, enter None.
+                </text>
+
+                <textline points="0" size="40" correct_answer="Your partner's username or 'None'"/>
+                <answer type="loncapa/python">
+correct=['correct']
+s = str(submission[0]).strip()
+if submission[0] == '':
+    correct[0] = 'incorrect'
+                </answer>
+            </customresponse>
+        </problem>
+    """)
+
+
 class CapaModuleTest(unittest.TestCase):
 
     def setUp(self):
@@ -491,6 +542,88 @@ class CapaModuleTest(unittest.TestCase):
         # Expect that the number of attempts is NOT incremented
         self.assertEqual(module.attempts, 1)
 
+    def test_check_problem_with_files(self):
+        # Check a problem with uploaded files, using the check_problem API.
+        # pylint: disable=W0212
+
+        # The files we'll be uploading.
+        fnames = ["prog1.py", "prog2.py", "prog3.py"]
+        fpaths = [os.path.join(DATA_DIR, "capa", fname) for fname in fnames]
+        fileobjs = [open(fpath) for fpath in fpaths]
+        for fileobj in fileobjs:
+            self.addCleanup(fileobj.close)
+
+        module = CapaFactoryWithFiles.create()
+
+        # Mock the XQueueInterface.
+        xqueue_interface = XQueueInterface("http://example.com/xqueue", Mock())
+        xqueue_interface._http_post = Mock(return_value=(0, "ok"))
+        module.system.xqueue['interface'] = xqueue_interface
+
+        # Create a request dictionary for check_problem.
+        get_request_dict = {
+            CapaFactoryWithFiles.input_key(input_num=2): fileobjs,
+            CapaFactoryWithFiles.input_key(input_num=3): 'None',
+        }
+
+        module.check_problem(get_request_dict)
+
+        # _http_post is called like this:
+        #   _http_post(
+        #       'http://example.com/xqueue/xqueue/submit/',
+        #       {
+        #           'xqueue_header': '{"lms_key": "df34fb702620d7ae892866ba57572491", "lms_callback_url": "/", "queue_name": "BerkeleyX-cs188x"}',
+        #           'xqueue_body': '{"student_info": "{\\"anonymous_student_id\\": \\"student\\", \\"submission_time\\": \\"20131117183318\\"}", "grader_payload": "{\\"project\\": \\"p3\\"}", "student_response": ""}',
+        #       },
+        #       files={
+        #           path(u'/home/ned/edx/edx-platform/common/test/data/uploads/asset.html'):
+        #               <open file u'/home/ned/edx/edx-platform/common/test/data/uploads/asset.html', mode 'r' at 0x49c5f60>,
+        #           path(u'/home/ned/edx/edx-platform/common/test/data/uploads/image.jpg'):
+        #               <open file u'/home/ned/edx/edx-platform/common/test/data/uploads/image.jpg', mode 'r' at 0x49c56f0>,
+        #           path(u'/home/ned/edx/edx-platform/common/test/data/uploads/textbook.pdf'):
+        #               <open file u'/home/ned/edx/edx-platform/common/test/data/uploads/textbook.pdf', mode 'r' at 0x49c5a50>,
+        #       },
+        #   )
+
+        self.assertEqual(xqueue_interface._http_post.call_count, 1)
+        _, kwargs = xqueue_interface._http_post.call_args
+        self.assertItemsEqual(fpaths, kwargs['files'].keys())
+        for fpath, fileobj in kwargs['files'].iteritems():
+            self.assertEqual(fpath, fileobj.name)
+
+    def test_check_problem_with_files_as_xblock(self):
+        # Check a problem with uploaded files, using the XBlock API.
+        # pylint: disable=W0212
+
+        # The files we'll be uploading.
+        fnames = ["prog1.py", "prog2.py", "prog3.py"]
+        fpaths = [os.path.join(DATA_DIR, "capa", fname) for fname in fnames]
+        fileobjs = [open(fpath) for fpath in fpaths]
+        for fileobj in fileobjs:
+            self.addCleanup(fileobj.close)
+
+        module = CapaFactoryWithFiles.create()
+
+        # Mock the XQueueInterface.
+        xqueue_interface = XQueueInterface("http://example.com/xqueue", Mock())
+        xqueue_interface._http_post = Mock(return_value=(0, "ok"))
+        module.system.xqueue['interface'] = xqueue_interface
+
+        # Create a webob Request with the files uploaded.
+        post_data = []
+        for fname, fileobj in zip(fnames, fileobjs):
+            post_data.append((CapaFactoryWithFiles.input_key(input_num=2), (fname, fileobj)))
+        post_data.append((CapaFactoryWithFiles.input_key(input_num=3), 'None'))
+        request = webob.Request.blank("/some/fake/url", POST=post_data, content_type='multipart/form-data')
+
+        module.handle('xmodule_handler', request, 'problem_check')
+
+        self.assertEqual(xqueue_interface._http_post.call_count, 1)
+        _, kwargs = xqueue_interface._http_post.call_args
+        self.assertItemsEqual(fnames, kwargs['files'].keys())
+        for fpath, fileobj in kwargs['files'].iteritems():
+            self.assertEqual(fpath, fileobj.name)
+
     def test_check_problem_error(self):
 
         # Try each exception that capa_module should handle

common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py

@@ -257,7 +257,7 @@ class TestXModuleHandler(TestXBlockWrapper):
     def setUp(self):
         self.module = XModule(descriptor=Mock(), field_data=Mock(), runtime=Mock(), scope_ids=Mock())
         self.module.handle_ajax = Mock(return_value='{}')
-        self.request = Mock()
+        self.request = webob.Request({})
 
     def test_xmodule_handler_passed_data(self):
         self.module.xmodule_handler(self.request)

common/lib/xmodule/xmodule/x_module.py

@@ -8,6 +8,7 @@ from lxml import etree
 from collections import namedtuple
 from pkg_resources import resource_listdir, resource_string, resource_isdir
 from webob import Response
+from webob.multidict import MultiDict
 
 from xmodule.modulestore import Location
 from xmodule.modulestore.exceptions import ItemNotFoundError, InsufficientSpecificationError, InvalidLocationError
@@ -405,7 +406,31 @@ class XModule(XModuleMixin, HTMLSnippet, XBlock):  # pylint: disable=abstract-me
         """
         XBlock handler that wraps `handle_ajax`
         """
-        response_data = self.handle_ajax(suffix, request.POST)
+        class FileObjForWebobFiles(object):
+            """
+            Turn Webob cgi.FieldStorage uploaded files into pure file objects.
+
+            Webob represents uploaded files as cgi.FieldStorage objects, which
+            have a .file attribute.  We wrap the FieldStorage object, delegating
+            attribute access to the .file attribute.  But the files have no
+            name, so we carry the FieldStorage .filename attribute as the .name.
+
+            """
+            def __init__(self, webob_file):
+                self.file = webob_file.file
+                self.name = webob_file.filename
+
+            def __getattr__(self, name):
+                return getattr(self.file, name)
+
+        # WebOb requests have multiple entries for uploaded files.  handle_ajax
+        # expects a single entry as a list.
+        request_post = MultiDict(request.POST)
+        for key in set(request.POST.iterkeys()):
+            if hasattr(request.POST[key], "file"):
+                request_post[key] = map(FileObjForWebobFiles, request.POST.getall(key))
+
+        response_data = self.handle_ajax(suffix, request_post)
         return Response(response_data, content_type='application/json')
 
     def get_children(self):

common/test/data/capa/prog1.py

+# prog1

common/test/data/capa/prog3.py

+# prog3

requirements/edx/github.txt

@@ -15,7 +15,7 @@
 -e git+https://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk
 
 # Our libraries:
--e git+https://github.com/edx/XBlock.git@2daa4e54#egg=XBlock
+-e git+https://github.com/edx/XBlock.git@d6d2fc91#egg=XBlock
 -e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail
 -e git+https://github.com/edx/diff-cover.git@v0.2.6#egg=diff_cover
 -e git+https://github.com/edx/js-test-tool.git@v0.1.4#egg=js_test_tool