Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: deprecates runtime.xqueue in favor of the XQueueService

Browse Source 
* Deprecates ModuleSystem.xqueue property
* Adds new XQueueService to provide the deprecated property values to the LMS runtime
  (Studio does not need the XQueueService.)
* Adds tests for new service and updates the ModuleSystemShim tests in LMS and Studio
* Fixes existing tests.
This commit is contained in:
Jillian Vogel
2021-11-10 18:13:53 +10:30
committed by Piotr Surowiec
parent f828d89feb
commit 1974bacadd
11 changed files with 201 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
cms/djangoapps/contentstore/views/tests/test_preview.py
View File

@@ -246,3 +246,13 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase):
descriptor = ItemFactory(category="pure", parent=self.course)
html = get_preview_fragment(self.request, descriptor, {'element_id': 142}).content
assert '<div id="142" ns="main">Testing the MakoService</div>' in html
def test_xqueue_is_not_available_in_studio(self):
descriptor = ItemFactory(category="problem", parent=self.course)
runtime = _preview_module_system(
self.request,
descriptor=descriptor,
field_data=mock.Mock(),
)
assert runtime.xqueue is None
assert runtime.service(descriptor, 'xqueue') is None

1
cms/envs/common.py
View File

@@ -725,6 +725,7 @@ CROSS_DOMAIN_CSRF_COOKIE_NAME = ''
CSRF_TRUSTED_ORIGINS = []
#################### CAPA External Code Evaluation #############################
XQUEUE_WAITTIME_BETWEEN_REQUESTS = 5 # seconds
XQUEUE_INTERFACE = {
'url': 'http://localhost:18040',
'basic_auth': ['edx', 'edx'],

41
common/lib/capa/capa/tests/test_xqueue_interface.py Normal file
View File

@@ -0,0 +1,41 @@
# -*- coding: utf-8 -*-
"""
Tests the xqueue service interface.
"""
from unittest import TestCase
from django.conf import settings
from capa.xqueue_interface import XQueueInterface, XQueueService
class XQueueServiceTest(TestCase):
"""
Tests the XQueue service methods.
"""
@staticmethod
def construct_callback(*args, **kwargs):
return 'https://lms.url/callback'
def setUp(self):
super().setUp()
self.service = XQueueService(
url=settings.XQUEUE_INTERFACE['url'],
django_auth=settings.XQUEUE_INTERFACE['django_auth'],
basic_auth=settings.XQUEUE_INTERFACE['basic_auth'],
construct_callback=self.construct_callback,
default_queuename='my-very-own-queue',
waittime=settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS,
)
def test_interface(self):
assert isinstance(self.service.interface, XQueueInterface)
def test_construct_callback(self):
assert self.service.construct_callback() == 'https://lms.url/callback'
def test_default_queuename(self):
assert self.service.default_queuename == 'my-very-own-queue'
def test_waittime(self):
assert self.service.waittime == 5

57
common/lib/capa/capa/xqueue_interface.py
View File

@@ -1,7 +1,6 @@
# lint-amnesty, pylint: disable=missing-module-docstring
# LMS Interface to external queueing system (xqueue)
#
"""
LMS Interface to external queueing system (xqueue)
"""
import hashlib
import json
@@ -149,3 +148,53 @@ class XQueueInterface(object):
return 1, 'unexpected HTTP status code [%d]' % response.status_code
return parse_xreply(response.text)
class XQueueService:
"""
XBlock service providing an interface to the XQueue service.
Args:
construct_callback(callable): function which constructs a fully-qualified callback URL to make xqueue requests.
default_queuename(string): course-specific queue name.
waittime(int): number of seconds to wait between xqueue requests
url(string): base URL for the XQueue service.
django_auth(dict): username and password for the XQueue service.
basic_auth(array or None): basic authentication credentials, if needed.
"""
def __init__(self, construct_callback, default_queuename, waittime, url, django_auth, basic_auth=None):
requests_auth = requests.auth.HTTPBasicAuth(*basic_auth) if basic_auth else None
self._interface = XQueueInterface(url, django_auth, requests_auth)
self._construct_callback = construct_callback
self._default_queuename = default_queuename.replace(' ', '_')
self._waittime = waittime
@property
def interface(self):
"""
Returns the XQueueInterface instance.
"""
return self._interface
@property
def construct_callback(self):
"""
Returns the function to construct the XQueue callback.
"""
return self._construct_callback
@property
def default_queuename(self):
"""
Returns the default queue name for the current course.
"""
return self._default_queuename
@property
def waittime(self):
"""
Returns the number of seconds to wait in between calls to XQueue.
"""
return self._waittime

2
common/lib/xmodule/xmodule/capa_module.py
View File

@@ -121,6 +121,8 @@ class Randomization(String):
@XBlock.needs('user')
@XBlock.needs('i18n')
@XBlock.needs('mako')
# Studio doesn't provide XQueueService, but the LMS does.
@XBlock.wants('xqueue')
@XBlock.wants('call_to_action')
class ProblemBlock(
ScorableXBlockMixin,

16
common/lib/xmodule/xmodule/tests/__init__.py
View File

@@ -26,6 +26,7 @@ from xblock.core import XBlock
from xblock.field_data import DictFieldData
from xblock.fields import Reference, ReferenceList, ReferenceValueDict, ScopeIds
from capa.xqueue_interface import XQueueService
from xmodule.assetstore import AssetMetadata
from xmodule.error_module import ErrorBlock
from xmodule.mako_module import MakoDescriptorSystem
@@ -144,13 +145,14 @@ def get_test_system(
services={
'user': user_service,
'mako': mako_service,
},
xqueue={
'interface': None,
'callback_url': '/',
'default_queuename': 'testqueue',
'waittime': 10,
'construct_callback': Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"),
'xqueue': XQueueService(
url='http://xqueue.url',
django_auth={},
basic_auth=[],
default_queuename='testqueue',
waittime=10,
construct_callback=Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"),
),
},
node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"),
course_id=course_id,

35
common/lib/xmodule/xmodule/tests/test_capa_module.py
View File

@@ -817,7 +817,8 @@ class ProblemBlockTest(unittest.TestCase): # lint-amnesty, pylint: disable=miss
# Expect that the number of attempts is NOT incremented
assert module.attempts == 1
def test_submit_problem_with_files(self):
@patch.object(XQueueInterface, '_http_post')
def test_submit_problem_with_files(self, mock_xqueue_post):
# Check a problem with uploaded files, using the submit_problem API.
# pylint: disable=protected-access
@@ -830,10 +831,8 @@ class ProblemBlockTest(unittest.TestCase): # lint-amnesty, pylint: disable=miss
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
# Mock the XQueueInterface post method
mock_xqueue_post.return_value = (0, "ok")
# Create a request dictionary for submit_problem.
get_request_dict = {
@@ -862,13 +861,14 @@ class ProblemBlockTest(unittest.TestCase): # lint-amnesty, pylint: disable=miss
# )
# pylint: enable=line-too-long
assert xqueue_interface._http_post.call_count == 1
_, kwargs = xqueue_interface._http_post.call_args
assert mock_xqueue_post.call_count == 1
_, kwargs = mock_xqueue_post.call_args
self.assertCountEqual(fpaths, list(kwargs['files'].keys()))
for fpath, fileobj in kwargs['files'].items():
assert fpath == fileobj.name
def test_submit_problem_with_files_as_xblock(self):
@patch.object(XQueueInterface, '_http_post')
def test_submit_problem_with_files_as_xblock(self, mock_xqueue_post):
# Check a problem with uploaded files, using the XBlock API.
# pylint: disable=protected-access
@@ -881,10 +881,8 @@ class ProblemBlockTest(unittest.TestCase): # lint-amnesty, pylint: disable=miss
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
# Mock the XQueueInterface post method
mock_xqueue_post.return_value = (0, "ok")
# Create a webob Request with the files uploaded.
post_data = []
@@ -895,8 +893,8 @@ class ProblemBlockTest(unittest.TestCase): # lint-amnesty, pylint: disable=miss
module.handle('xmodule_handler', request, 'problem_check')
assert xqueue_interface._http_post.call_count == 1
_, kwargs = xqueue_interface._http_post.call_args
assert mock_xqueue_post.call_count == 1
_, kwargs = mock_xqueue_post.call_args
self.assertCountEqual(fnames, list(kwargs['files'].keys()))
for fpath, fileobj in kwargs['files'].items():
assert fpath == fileobj.name
@@ -3101,7 +3099,8 @@ class ProblemCheckTrackingTest(unittest.TestCase):
'group_label': '',
'variant': module.seed}}
def test_file_inputs(self):
@patch.object(XQueueInterface, '_http_post')
def test_file_inputs(self, mock_xqueue_post):
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]
@@ -3111,10 +3110,8 @@ class ProblemCheckTrackingTest(unittest.TestCase):
factory = CapaFactoryWithFiles
module = factory.create()
# Mock the XQueueInterface.
xqueue_interface = XQueueInterface("http://example.com/xqueue", Mock())
xqueue_interface._http_post = Mock(return_value=(0, "ok")) # pylint: disable=protected-access
module.system.xqueue['interface'] = xqueue_interface
# Mock the XQueueInterface post method
mock_xqueue_post.return_value = (0, "ok")
answer_input_dict = {
CapaFactoryWithFiles.input_key(response_num=2): fileobjs,

34
common/lib/xmodule/xmodule/x_module.py
View File

@@ -1881,6 +1881,31 @@ class ModuleSystemShim:
return render_service.render_template
return None
@property
def xqueue(self):
"""
Returns a dict containing the XQueueInterface object, as well as parameters for the specific StudentModule:
* interface: XQueueInterface object
* construct_callback: function to construct the fully-qualified LMS callback URL.
* default_queuename: default queue name for the course in XQueue
* waittime: number of seconds to wait in between calls to XQueue
Deprecated in favor of the xqueue service.
"""
warnings.warn(
'runtime.xqueue is deprecated. Please use the xqueue service instead.',
DeprecationWarning, stacklevel=3,
)
xqueue_service = self._services.get('xqueue')
if xqueue_service:
return {
'interface': xqueue_service.interface,
'construct_callback': xqueue_service.construct_callback,
'default_queuename': xqueue_service.default_queuename,
'waittime': xqueue_service.waittime,
}
return None
class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime):
"""
@@ -1898,7 +1923,7 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
def __init__(
self, static_url, track_function, get_module,
replace_urls, descriptor_runtime, filestore=None,
debug=False, hostname="", xqueue=None, publish=None, node_path="",
debug=False, hostname="", publish=None, node_path="",
course_id=None,
cache=None, can_execute_unsafe_code=None, replace_course_urls=None,
replace_jump_to_id_urls=None, error_descriptor_class=None,
@@ -1921,12 +1946,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
filestore - A filestore ojbect. Defaults to an instance of OSFS based
at settings.DATA_DIR.
xqueue - Dict containing XqueueInterface object, as well as parameters
for the specific StudentModule:
xqueue = {'interface': XQueueInterface object,
'callback_url': Callback into the LMS,
'queue_name': Target queuename in Xqueue}
replace_urls - TEMPORARY - A function like static_replace.replace_urls
that capa_module can use to fix up the static urls in
ajax results.
@@ -1963,7 +1982,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
super().__init__(field_data=field_data, **kwargs)
self.STATIC_URL = static_url
self.xqueue = xqueue
self.track_function = track_function
self.filestore = filestore
self.get_module = get_module

30
lms/djangoapps/courseware/module_render.py
View File

@@ -31,7 +31,6 @@ from edx_when.field_data import DateLookupFieldData
from eventtracking import tracker
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey
from requests.auth import HTTPBasicAuth
from rest_framework.decorators import api_view
from rest_framework.exceptions import APIException
from web_fragments.fragment import Fragment
@@ -42,7 +41,7 @@ from xblock.runtime import KvsFieldData
from common.djangoapps import static_replace
from common.djangoapps.xblock_django.constants import ATTR_KEY_USER_ID
from capa.xqueue_interface import XQueueInterface # lint-amnesty, pylint: disable=wrong-import-order
from capa.xqueue_interface import XQueueService
from lms.djangoapps.courseware.access import get_user_role, has_access
from lms.djangoapps.courseware.entrance_exams import user_can_skip_entrance_exam, user_has_passed_entrance_exam
from lms.djangoapps.courseware.masquerade import (
@@ -100,17 +99,6 @@ from xmodule.util.sandboxing import can_execute_unsafe_code, get_python_lib_zip
log = logging.getLogger(__name__)
if settings.XQUEUE_INTERFACE.get('basic_auth') is not None:
REQUESTS_AUTH = HTTPBasicAuth(*settings.XQUEUE_INTERFACE['basic_auth'])
else:
REQUESTS_AUTH = None
XQUEUE_INTERFACE = XQueueInterface(
settings.XQUEUE_INTERFACE['url'],
settings.XQUEUE_INTERFACE['django_auth'],
REQUESTS_AUTH,
)
# TODO: course_id and course_key are used interchangeably in this file, which is wrong.
# Some brave person should make the variable names consistently someday, but the code's
# coupled enough that it's kind of tricky--you've been warned!
@@ -490,12 +478,14 @@ def get_module_system_for_user(
# TODO: Queuename should be derived from 'course_settings.json' of each course
xqueue_default_queuename = descriptor.location.org + '-' + descriptor.location.course
xqueue = {
'interface': XQUEUE_INTERFACE,
'construct_callback': make_xqueue_callback,
'default_queuename': xqueue_default_queuename.replace(' ', '_'),
'waittime': settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS
}
xqueue_service = XQueueService(
construct_callback=make_xqueue_callback,
default_queuename=xqueue_default_queuename,
url=settings.XQUEUE_INTERFACE['url'],
django_auth=settings.XQUEUE_INTERFACE['django_auth'],
basic_auth=settings.XQUEUE_INTERFACE.get('basic_auth'),
waittime=settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS,
)
def inner_get_module(descriptor):
"""
@@ -774,7 +764,6 @@ def get_module_system_for_user(
system = LmsModuleSystem(
track_function=track_function,
static_url=settings.STATIC_URL,
xqueue=xqueue,
# TODO (cpennington): Figure out how to share info between systems
filestore=descriptor.runtime.resources_fs,
get_module=inner_get_module,
@@ -822,6 +811,7 @@ def get_module_system_for_user(
'grade_utils': GradesUtilService(course_id=course_id),
'user_state': UserStateService(),
'content_type_gating': ContentTypeGatingService(),
'xqueue': xqueue_service,
},
descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access
rebind_noauth_module_to_user=rebind_noauth_module_to_user,

26
lms/djangoapps/courseware/tests/test_module_render.py
View File

@@ -41,6 +41,7 @@ from xblock.runtime import DictKeyValueStore, KvsFieldData, Runtime # lint-amne
from xblock.test.tools import TestRuntime # lint-amnesty, pylint: disable=wrong-import-order
from capa.tests.response_xml_factory import OptionResponseXMLFactory # lint-amnesty, pylint: disable=reimported
from capa.xqueue_interface import XQueueInterface
from common.djangoapps.course_modes.models import CourseMode # lint-amnesty, pylint: disable=reimported
from common.djangoapps.student.tests.factories import GlobalStaffFactory
from common.djangoapps.student.tests.factories import RequestFactoryNoCsrf
@@ -2567,6 +2568,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
"""
Tests that the deprecated attributes in the LMS Module System (XBlock Runtime) return the expected values.
"""
COURSE_ID = 'edX/LmsModuleShimTest/2021_Fall'
@classmethod
def setUpClass(cls):
@@ -2574,7 +2576,8 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
Set up the course and descriptor used to instantiate the runtime.
"""
super().setUpClass()
cls.course = CourseFactory.create()
org, number, run = cls.COURSE_ID.split('/')
cls.course = CourseFactory.create(org=org, number=number, run=run)
cls.descriptor = ItemFactory(category="vertical", parent=cls.course)
cls.problem_descriptor = ItemFactory(category="problem", parent=cls.course)
@@ -2586,7 +2589,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
self.user = UserFactory(id=232)
self.student_data = Mock()
self.track_function = Mock()
self.xqueue_callback_url_prefix = Mock()
self.xqueue_callback_url_prefix = 'https://lms.url'
self.request_token = Mock()
@ddt.data(
@@ -2737,3 +2740,22 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
)
rendered = runtime.render_template('templates/edxmako.html', {'element_id': 'hi'}) # pylint: disable=not-callable
assert rendered == '<div id="hi" ns="main">Testing the MakoService</div>\n'
def test_xqueue(self):
runtime, _ = render.get_module_system_for_user(
self.user,
self.student_data,
self.descriptor,
self.course.id,
self.track_function,
self.xqueue_callback_url_prefix,
self.request_token,
course=self.course,
)
xqueue = runtime.xqueue
assert isinstance(xqueue['interface'], XQueueInterface)
assert xqueue['default_queuename'] == 'edX-LmsModuleShimTest'
assert xqueue['waittime'] == 5
callback_url = f'https://lms.url/courses/edX/LmsModuleShimTest/2021_Fall/xqueue/232/{self.descriptor.location}'
assert xqueue['construct_callback']() == f'{callback_url}/score_update'
assert xqueue['construct_callback']('mock_dispatch') == f'{callback_url}/mock_dispatch'

17
lms/djangoapps/courseware/tests/test_submitting_problems.py
View File

@@ -26,6 +26,7 @@ from capa.tests.response_xml_factory import (
OptionResponseXMLFactory,
SchematicResponseXMLFactory
)
from capa.xqueue_interface import XQueueInterface
from common.djangoapps.course_modes.models import CourseMode
from lms.djangoapps.courseware.models import BaseStudentModuleHistory, StudentModule
from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase
@@ -776,7 +777,8 @@ class ProblemWithUploadedFilesTest(TestSubmittingProblems):
# re-fetch the course from the database so the object is up to date
self.refresh_course()
def test_three_files(self):
@patch.object(XQueueInterface, '_http_post')
def test_three_files(self, mock_xqueue_post):
# Open the test files, and arrange to close them later.
filenames = "prog1.py prog2.py prog3.py"
fileobjs = [
@@ -787,20 +789,19 @@ class ProblemWithUploadedFilesTest(TestSubmittingProblems):
self.addCleanup(fileobj.close)
self.problem_setup("the_problem", filenames)
with patch('lms.djangoapps.courseware.module_render.XQUEUE_INTERFACE.session') as mock_session:
resp = self.submit_question_answer("the_problem", {'2_1': fileobjs})
mock_xqueue_post.return_value = (0, "ok")
resp = self.submit_question_answer("the_problem", {'2_1': fileobjs})
assert resp.status_code == 200
json_resp = json.loads(resp.content.decode('utf-8'))
assert json_resp['success'] == 'incorrect'
# See how post got called.
name, args, kwargs = mock_session.mock_calls[0]
assert name == 'post'
assert len(args) == 1
assert mock_xqueue_post.call_count == 1
args, kwargs = mock_xqueue_post.call_args
assert len(args) == 2
assert args[0].endswith('/submit/')
self.assertCountEqual(list(kwargs.keys()), ["files", "data", "timeout"])
self.assertCountEqual(list(kwargs['files'].keys()), filenames.split())
self.assertEqual(list(kwargs['files'].keys()), filenames.split())
class TestPythonGradedResponse(TestSubmittingProblems):