diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py index 3a7f29472f2f..1c8ddc860b14 100644 --- a/cms/djangoapps/contentstore/views/tests/test_preview.py +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -236,3 +236,13 @@ def test_render_template(self): descriptor = ItemFactory(category="pure", parent=self.course) html = get_preview_fragment(self.request, descriptor, {'element_id': 142}).content assert '
Testing the MakoService
' 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 diff --git a/cms/envs/common.py b/cms/envs/common.py index 23e3073c00ca..6f4ab636a4ca 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -725,6 +725,7 @@ CSRF_TRUSTED_ORIGINS = [] #################### CAPA External Code Evaluation ############################# +XQUEUE_WAITTIME_BETWEEN_REQUESTS = 5 # seconds XQUEUE_INTERFACE = { 'url': 'http://localhost:18040', 'basic_auth': ['edx', 'edx'], diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index ab72f4da4d63..f88ec187e68a 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -994,9 +994,9 @@ def _plot_data(self, data): response = data['submission'] # construct xqueue headers - qinterface = self.capa_system.xqueue['interface'] + qinterface = self.capa_system.xqueue.interface qtime = datetime.utcnow().strftime(xqueue_interface.dateformat) - callback_url = self.capa_system.xqueue['construct_callback']('ungraded_response') + callback_url = self.capa_system.xqueue.construct_callback('ungraded_response') anonymous_student_id = self.capa_system.anonymous_student_id # TODO: Why is this using self.capa_system.seed when we have self.seed??? queuekey = xqueue_interface.make_hashkey(str(self.capa_system.seed) + qtime + diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 86347de26f78..3e4d6fc937da 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -2586,14 +2586,14 @@ class CodeResponse(LoncapaResponse): """ Grade student code using an external queueing server, called 'xqueue'. - Expects 'xqueue' dict in LoncapaSystem with the following keys that are + Expects 'xqueue' dict in LoncapaSystem with the following properties that are needed by CodeResponse:: - capa_system.xqueue = { - 'interface': XQueueInterface object. - 'construct_callback': Per-StudentModule callback URL constructor, + capa_system.xqueue = object with properties: + interface: XQueueInterface object. + construct_callback: Per-StudentModule callback URL constructor, defaults to using 'score_update' as the correct dispatch (function). - 'default_queuename': Default queue name to submit request (string). + default_queuename: Default queue name to submit request (string). } External requests are only submitted for student submission grading, not @@ -2623,7 +2623,7 @@ def setup_response(self): # We do not support xqueue within Studio. if self.capa_system.xqueue is not None: - default_queuename = self.capa_system.xqueue['default_queuename'] + default_queuename = self.capa_system.xqueue.default_queuename else: default_queuename = None self.queue_name = xml.get('queuename', default_queuename) @@ -2684,7 +2684,7 @@ def get_score(self, student_answers): # Prepare xqueue request #------------------------------------------------------------ - qinterface = self.capa_system.xqueue['interface'] + qinterface = self.capa_system.xqueue.interface qtime = datetime.strftime(datetime.now(UTC), xqueue_interface.dateformat) anonymous_student_id = self.capa_system.anonymous_student_id @@ -2693,7 +2693,7 @@ def get_score(self, student_answers): queuekey = xqueue_interface.make_hashkey( str(self.capa_system.seed) + qtime + anonymous_student_id + self.answer_id ) - callback_url = self.capa_system.xqueue['construct_callback']() + callback_url = self.capa_system.xqueue.construct_callback() xheader = xqueue_interface.make_xheader( lms_callback_url=callback_url, lms_key=queuekey, diff --git a/common/lib/capa/capa/tests/helpers.py b/common/lib/capa/capa/tests/helpers.py index f5b59200f86d..c923d0bdd93b 100644 --- a/common/lib/capa/capa/tests/helpers.py +++ b/common/lib/capa/capa/tests/helpers.py @@ -44,12 +44,19 @@ def tst_render_template(template, context): # pylint: disable=unused-argument return '
{0}
'.format(saxutils.escape(repr(context))) -def calledback_url(dispatch='score_update'): - """A callback url method to use in tests.""" - return dispatch +class StubXQueueService: + """ + Stubs out the XQueueService for Capa problem tests. + """ + def __init__(self): + self.interface = MagicMock() + self.interface.send_to_queue.return_value = (0, 'Success!') + self.default_queuename = 'testqueue' + self.waittime = 10 -xqueue_interface = MagicMock() # pylint: disable=invalid-name -xqueue_interface.send_to_queue.return_value = (0, 'Success!') + def construct_callback(self, dispatch='score_update'): + """A callback url method to use in tests.""" + return dispatch def test_capa_system(render_template=None): @@ -72,12 +79,7 @@ def test_capa_system(render_template=None): seed=0, STATIC_URL='/dummy-static/', STATUS_CLASS=Status, - xqueue={ - 'interface': xqueue_interface, - 'construct_callback': calledback_url, - 'default_queuename': 'testqueue', - 'waittime': 10 - }, + xqueue=StubXQueueService(), ) return the_system diff --git a/common/lib/capa/capa/tests/test_inputtypes.py b/common/lib/capa/capa/tests/test_inputtypes.py index 783a09be751a..0d2ce8c93c7c 100644 --- a/common/lib/capa/capa/tests/test_inputtypes.py +++ b/common/lib/capa/capa/tests/test_inputtypes.py @@ -643,9 +643,7 @@ def test_rendering_while_queued(self, time): # lint-amnesty, pylint: disable=un def test_plot_data(self): data = {'submission': 'x = 1234;'} response = self.the_input.handle_ajax("plot", data) - - test_capa_system().xqueue['interface'].send_to_queue.assert_called_with(header=ANY, body=ANY) - + self.the_input.capa_system.xqueue.interface.send_to_queue.assert_called_with(header=ANY, body=ANY) assert response['success'] assert self.the_input.input_state['queuekey'] is not None assert self.the_input.input_state['queuestate'] == 'queued' @@ -653,7 +651,7 @@ def test_plot_data(self): def test_plot_data_failure(self): data = {'submission': 'x = 1234;'} error_message = 'Error message!' - test_capa_system().xqueue['interface'].send_to_queue.return_value = (1, error_message) + self.the_input.capa_system.xqueue.interface.send_to_queue.return_value = (1, error_message) response = self.the_input.handle_ajax("plot", data) assert not response['success'] assert response['message'] == error_message @@ -740,7 +738,7 @@ def test_matlab_api_key(self): data = {'submission': 'x = 1234;'} response = the_input.handle_ajax("plot", data) # lint-amnesty, pylint: disable=unused-variable - body = system.xqueue['interface'].send_to_queue.call_args[1]['body'] + body = system.xqueue.interface.send_to_queue.call_args[1]['body'] payload = json.loads(body) assert 'test_api_key' == payload['token'] assert '2' == payload['endpoint_version'] diff --git a/common/lib/capa/capa/tests/test_xqueue_interface.py b/common/lib/capa/capa/tests/test_xqueue_interface.py new file mode 100644 index 000000000000..77cd6ccad0c4 --- /dev/null +++ b/common/lib/capa/capa/tests/test_xqueue_interface.py @@ -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 diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index 3deff6a3dd3c..f035d122014b 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -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 @@ def _http_post(self, url, data, files=None): # lint-amnesty, pylint: disable=mi 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 diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 26a42d59d6a6..f2d5575c16bb 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -121,6 +121,8 @@ def from_json(self, value): @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, @@ -825,7 +827,7 @@ def new_lcp(self, state, text=None): render_template=self.runtime.service(self, 'mako').render_template, seed=seed, # Why do we do this if we have self.seed? STATIC_URL=self.runtime.STATIC_URL, - xqueue=self.runtime.xqueue, + xqueue=self.runtime.service(self, 'xqueue'), matlab_api_key=self.matlab_api_key ) @@ -1744,7 +1746,8 @@ def submit_problem(self, data, override_time=False): if self.lcp.is_queued(): prev_submit_time = self.lcp.get_recentmost_queuetime() - waittime_between_requests = self.runtime.xqueue['waittime'] + xqueue_service = self.runtime.service(self, 'xqueue') + waittime_between_requests = xqueue_service.waittime if xqueue_service else 0 if (current_time - prev_submit_time).total_seconds() < waittime_between_requests: msg = _("You must wait at least {wait} seconds between submissions.").format( wait=waittime_between_requests) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 533bff5c6952..50717772f62f 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -26,6 +26,7 @@ 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 @@ -140,13 +141,14 @@ def get_module(descriptor): 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, diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 4121cac007c8..2d3f2a4e42bb 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -817,7 +817,8 @@ def test_submit_problem_queued(self): # 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 @@ def test_submit_problem_with_files(self): 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 @@ def test_submit_problem_with_files(self): # ) # 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 @@ def test_submit_problem_with_files_as_xblock(self): 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 @@ def test_submit_problem_with_files_as_xblock(self): 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 @@ def test_rerandomized_inputs(self): '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 @@ def test_file_inputs(self): 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, diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 056528ebd4ba..6d1fa68bb748 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1826,6 +1826,31 @@ def render_template(self): 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): """ @@ -1843,7 +1868,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, get_real_user=None, @@ -1866,12 +1891,6 @@ def __init__( 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. @@ -1914,7 +1933,6 @@ def __init__( 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 diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 6929cea8a8c3..9a3d3e1c1c3c 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -31,7 +31,6 @@ 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 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 @@ 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! @@ -376,21 +364,6 @@ def display_access_messages(user, block, view, frag, context): # pylint: disabl return msg_fragment -def get_xqueue_callback_url_prefix(request): - """ - Calculates default prefix based on request, but allows override via settings - - This is separated from get_module_for_descriptor so that it can be called - by the LMS before submitting background tasks to run. The xqueue callbacks - should go back to the LMS, not to the worker. - """ - prefix = '{proto}://{host}'.format( - proto=request.META.get('HTTP_X_FORWARDED_PROTO', 'https' if request.is_secure() else 'http'), - host=request.get_host() - ) - return settings.XQUEUE_INTERFACE.get('callback_url', prefix) - - # pylint: disable=too-many-statements def get_module_for_descriptor(user, request, descriptor, field_data_cache, course_key, position=None, wrap_xmodule_display=True, grade_bucket_type=None, @@ -404,7 +377,6 @@ def get_module_for_descriptor(user, request, descriptor, field_data_cache, cours See get_module() docstring for further details. """ track_function = make_track_function(request) - xqueue_callback_url_prefix = get_xqueue_callback_url_prefix(request) user_location = getattr(request, 'session', {}).get('country_code') @@ -419,7 +391,6 @@ def get_module_for_descriptor(user, request, descriptor, field_data_cache, cours student_data=student_data, course_id=course_key, track_function=track_function, - xqueue_callback_url_prefix=xqueue_callback_url_prefix, position=position, wrap_xmodule_display=wrap_xmodule_display, grade_bucket_type=grade_bucket_type, @@ -439,7 +410,6 @@ def get_module_system_for_user( descriptor, course_id, track_function, - xqueue_callback_url_prefix, request_token, position=None, wrap_xmodule_display=True, @@ -483,6 +453,7 @@ def make_xqueue_callback(dispatch='score_update'): dispatch=dispatch ), ) + xqueue_callback_url_prefix = settings.XQUEUE_INTERFACE.get('callback_url', settings.LMS_ROOT_URL) return xqueue_callback_url_prefix + relative_xqueue_callback_url # Default queuename is course-specific and is derived from the course that @@ -490,12 +461,14 @@ def make_xqueue_callback(dispatch='score_update'): # 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): """ @@ -511,7 +484,6 @@ def inner_get_module(descriptor): student_data=student_data, course_id=course_id, track_function=track_function, - xqueue_callback_url_prefix=xqueue_callback_url_prefix, position=position, wrap_xmodule_display=wrap_xmodule_display, grade_bucket_type=grade_bucket_type, @@ -668,7 +640,6 @@ def rebind_noauth_module_to_user(module, real_user): descriptor=module, course_id=course_id, track_function=track_function, - xqueue_callback_url_prefix=xqueue_callback_url_prefix, position=position, wrap_xmodule_display=wrap_xmodule_display, grade_bucket_type=grade_bucket_type, @@ -772,7 +743,6 @@ def rebind_noauth_module_to_user(module, real_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, @@ -821,6 +791,7 @@ def rebind_noauth_module_to_user(module, real_user): 'grade_utils': GradesUtilService(course_id=course_id), 'user_state': UserStateService(), 'content_type_gating': ContentTypeGatingService(), + 'xqueue': xqueue_service, }, get_user_role=lambda: get_user_role(user, course_id), descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access @@ -856,7 +827,7 @@ def rebind_noauth_module_to_user(module, real_user): # TODO: Find all the places that this method is called and figure out how to # get a loaded course passed into it def get_module_for_descriptor_internal(user, descriptor, student_data, course_id, - track_function, xqueue_callback_url_prefix, request_token, + track_function, request_token, position=None, wrap_xmodule_display=True, grade_bucket_type=None, static_asset_path='', user_location=None, disable_staff_debug_info=False, course=None, will_recheck_access=False): @@ -875,7 +846,6 @@ def get_module_for_descriptor_internal(user, descriptor, student_data, course_id descriptor=descriptor, course_id=course_id, track_function=track_function, - xqueue_callback_url_prefix=xqueue_callback_url_prefix, position=position, wrap_xmodule_display=wrap_xmodule_display, grade_bucket_type=grade_bucket_type, diff --git a/lms/djangoapps/courseware/tests/test_discussion_xblock.py b/lms/djangoapps/courseware/tests/test_discussion_xblock.py index 6d6420a9df99..385ec26fef06 100644 --- a/lms/djangoapps/courseware/tests/test_discussion_xblock.py +++ b/lms/djangoapps/courseware/tests/test_discussion_xblock.py @@ -300,7 +300,6 @@ def test_html_with_user(self): student_data=mock.Mock(name='student_data'), course_id=self.course.id, track_function=mock.Mock(name='track_function'), - xqueue_callback_url_prefix=mock.Mock(name='xqueue_callback_url_prefix'), request_token='request_token', ) @@ -321,7 +320,6 @@ def test_embed_mfe_in_course(self): student_data=mock.Mock(name='student_data'), course_id=self.course.id, track_function=mock.Mock(name='track_function'), - xqueue_callback_url_prefix=mock.Mock(name='xqueue_callback_url_prefix'), request_token='request_token', ) @@ -375,7 +373,6 @@ def test_discussion_render_successfully_with_orphan_parent(self, default_store): student_data=mock.Mock(name='student_data'), course_id=self.course.id, track_function=mock.Mock(name='track_function'), - xqueue_callback_url_prefix=mock.Mock(name='xqueue_callback_url_prefix'), request_token='request_token', ) @@ -447,7 +444,6 @@ def test_permissions_query_load(self): student_data=mock.Mock(name='student_data'), course_id=course.id, track_function=mock.Mock(name='track_function'), - xqueue_callback_url_prefix=mock.Mock(name='xqueue_callback_url_prefix'), request_token='request_token', ) with self.assertNumQueries(num_queries): diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 5f76c0580328..eaa3dfa54a1b 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -41,6 +41,7 @@ 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 @@ -1930,7 +1931,6 @@ def _get_anonymous_id(self, course_id, xblock_class): # lint-amnesty, pylint: d student_data=Mock(spec=FieldData, name='student_data'), course_id=course_id, track_function=Mock(name='track_function'), # Track Function - xqueue_callback_url_prefix=Mock(name='xqueue_callback_url_prefix'), # XQueue Callback Url Prefix request_token='request_token', course=self.course, ) @@ -2290,7 +2290,6 @@ def setUp(self): self.user = UserFactory() self.student_data = Mock() self.track_function = Mock() - self.xqueue_callback_url_prefix = Mock() self.request_token = Mock() @XBlock.register_temp_plugin(PureXBlock, identifier='pure') @@ -2306,7 +2305,6 @@ def test_expected_services_exist(self, expected_service): descriptor, self.course.id, self.track_function, - self.xqueue_callback_url_prefix, self.request_token, course=self.course ) @@ -2325,7 +2323,6 @@ def test_beta_tester_fields_added(self): descriptor, self.course.id, self.track_function, - self.xqueue_callback_url_prefix, self.request_token, course=self.course ) @@ -2567,6 +2564,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 +2572,8 @@ def setUpClass(cls): 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 +2585,6 @@ def setUp(self): self.user = UserFactory(id=232) self.student_data = Mock() self.track_function = Mock() - self.xqueue_callback_url_prefix = Mock() self.request_token = Mock() @ddt.data( @@ -2605,7 +2603,6 @@ def test_user_service_attributes(self, attribute, expected_value): self.descriptor, self.course.id, self.track_function, - self.xqueue_callback_url_prefix, self.request_token, course=self.course, ) @@ -2619,7 +2616,6 @@ def test_user_is_staff(self): self.descriptor, self.course.id, self.track_function, - self.xqueue_callback_url_prefix, self.request_token, course=self.course, ) @@ -2632,7 +2628,6 @@ def test_anonymous_student_id(self): self.descriptor, self.course.id, self.track_function, - self.xqueue_callback_url_prefix, self.request_token, course=self.course, ) @@ -2649,7 +2644,6 @@ def test_anonymous_student_id_bug(self): self.problem_descriptor, self.course.id, self.track_function, - self.xqueue_callback_url_prefix, self.request_token, course=self.course, ) @@ -2662,7 +2656,6 @@ def test_anonymous_student_id_bug(self): self.descriptor, self.course.id, self.track_function, - self.xqueue_callback_url_prefix, self.request_token, course=self.course, ) @@ -2679,7 +2672,6 @@ def test_user_service_with_anonymous_user(self): self.descriptor, self.course.id, self.track_function, - self.xqueue_callback_url_prefix, self.request_token, course=self.course, ) @@ -2695,9 +2687,59 @@ def test_render_template(self): self.descriptor, self.course.id, self.track_function, - self.xqueue_callback_url_prefix, self.request_token, course=self.course, ) rendered = runtime.render_template('templates/edxmako.html', {'element_id': 'hi'}) # pylint: disable=not-callable assert rendered == '
Testing the MakoService
\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.request_token, + course=self.course, + ) + xqueue = runtime.xqueue + assert isinstance(xqueue['interface'], XQueueInterface) + assert xqueue['interface'].url == 'http://sandbox-xqueue.edx.org' + assert xqueue['default_queuename'] == 'edX-LmsModuleShimTest' + assert xqueue['waittime'] == 5 + callback_url = ('http://localhost:8000/courses/edX/LmsModuleShimTest/2021_Fall/xqueue/232/' + + str(self.descriptor.location)) + assert xqueue['construct_callback']() == f'{callback_url}/score_update' + assert xqueue['construct_callback']('mock_dispatch') == f'{callback_url}/mock_dispatch' + + @override_settings( + XQUEUE_INTERFACE={ + 'callback_url': 'http://alt.url', + 'url': 'http://xqueue.url', + 'django_auth': { + 'username': 'user', + 'password': 'password', + }, + 'basic_auth': ('basic', 'auth'), + }, + XQUEUE_WAITTIME_BETWEEN_REQUESTS=15, + ) + def test_xqueue_settings(self): + runtime, _ = render.get_module_system_for_user( + self.user, + self.student_data, + self.descriptor, + self.course.id, + self.track_function, + self.request_token, + course=self.course, + ) + xqueue = runtime.xqueue + assert isinstance(xqueue['interface'], XQueueInterface) + assert xqueue['interface'].url == 'http://xqueue.url' + assert xqueue['default_queuename'] == 'edX-LmsModuleShimTest' + assert xqueue['waittime'] == 15 + callback_url = f'http://alt.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' diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index a9b15db3b13f..2757bd77dba0 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -26,6 +26,7 @@ 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 @@ def problem_setup(self, name, files): # 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 @@ def test_three_files(self): 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): diff --git a/lms/djangoapps/instructor_task/api_helper.py b/lms/djangoapps/instructor_task/api_helper.py index 54345f182168..dee85ef0f207 100644 --- a/lms/djangoapps/instructor_task/api_helper.py +++ b/lms/djangoapps/instructor_task/api_helper.py @@ -17,7 +17,6 @@ from common.djangoapps.util.db import outer_atomic from lms.djangoapps.courseware.courses import get_problems_in_section -from lms.djangoapps.courseware.module_render import get_xqueue_callback_url_prefix from lms.djangoapps.instructor_task.models import PROGRESS, InstructorTask from xmodule.modulestore.django import modulestore @@ -131,8 +130,7 @@ def _get_xmodule_instance_args(request, task_id): Calculate parameters needed for instantiating xmodule instances. The `request_info` will be passed to a tracking log function, to provide information - about the source of the task request. The `xqueue_callback_url_prefix` is used to - permit old-style xqueue callbacks directly to the appropriate module in the LMS. + about the source of the task request. The `task_id` is also passed to the tracking log function. """ request_info = {'username': request.user.username, @@ -142,8 +140,7 @@ def _get_xmodule_instance_args(request, task_id): 'host': request.META['SERVER_NAME'], } - xmodule_instance_args = {'xqueue_callback_url_prefix': get_xqueue_callback_url_prefix(request), - 'request_info': request_info, + xmodule_instance_args = {'request_info': request_info, 'task_id': task_id, } return xmodule_instance_args diff --git a/lms/djangoapps/instructor_task/tasks_helper/module_state.py b/lms/djangoapps/instructor_task/tasks_helper/module_state.py index 73c94af40bc2..5a63836089a3 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/module_state.py +++ b/lms/djangoapps/instructor_task/tasks_helper/module_state.py @@ -357,16 +357,12 @@ def make_track_function(): ''' return lambda event_type, event: task_track(request_info, task_info, event_type, event, page='x_module_task') - xqueue_callback_url_prefix = xmodule_instance_args.get('xqueue_callback_url_prefix', '') \ - if xmodule_instance_args is not None else '' - return get_module_for_descriptor_internal( user=student, descriptor=module_descriptor, student_data=student_data, course_id=course_id, track_function=make_track_function(), - xqueue_callback_url_prefix=xqueue_callback_url_prefix, grade_bucket_type=grade_bucket_type, # This module isn't being used for front-end rendering request_token=None, diff --git a/lms/djangoapps/instructor_task/tests/test_tasks.py b/lms/djangoapps/instructor_task/tests/test_tasks.py index ead71b5c7a9a..589a4d631647 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks.py @@ -83,7 +83,6 @@ def _get_xmodule_instance_args(self): Calculate dummy values for parameters needed for instantiating xmodule instances. """ return { - 'xqueue_callback_url_prefix': 'dummy_value', 'request_info': { 'username': 'dummy_username', 'user_id': 'dummy_id',