diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 2bab345b83..4bb7a129b1 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -53,7 +53,6 @@ from xmodule.util.sandboxing import SandboxService from xmodule.services import EventPublishingService, RebindUserService, SettingsService, TeamsConfigurationService from common.djangoapps.static_replace.services import ReplaceURLService from common.djangoapps.static_replace.wrapper import replace_urls_wrapper -from xmodule.capa.xqueue_interface import XQueueService # lint-amnesty, pylint: disable=wrong-import-order 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 ( @@ -439,44 +438,12 @@ def prepare_runtime_for_user( KvsFieldData: student_data bound to, primarily, the user and block """ - def make_xqueue_callback(dispatch='score_update'): - """ - Returns fully qualified callback URL for external queueing system - """ - relative_xqueue_callback_url = reverse( - 'xqueue_callback', - kwargs=dict( - course_id=str(course_id), - userid=str(user.id), - mod_id=str(block.location), - 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 - # contains the current block. - # TODO: Queuename should be derived from 'course_settings.json' of each course - xqueue_default_queuename = block.location.org + '-' + block.location.course - - 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_block(block): """ Delegate to get_block_for_descriptor_internal() with all values except `block` set. Because it does an access check, it may return None. """ - # TODO: fix this so that make_xqueue_callback uses the block passed into - # inner_get_block, not the parent's callback. Add it as an argument.... return get_block_for_descriptor_internal( user=user, block=block, @@ -579,7 +546,6 @@ def prepare_runtime_for_user( 'content_type_gating': ContentTypeGatingService(), 'cache': CacheService(cache), 'sandbox': SandboxService(contentstore=contentstore, course_id=course_id), - 'xqueue': xqueue_service, 'replace_urls': replace_url_service, # Rebind module service to deal with noauth modules getting attached to users. 'rebind_user': RebindUserService( diff --git a/lms/djangoapps/courseware/tests/test_block_render.py b/lms/djangoapps/courseware/tests/test_block_render.py index dc654a3995..e0f97f5418 100644 --- a/lms/djangoapps/courseware/tests/test_block_render.py +++ b/lms/djangoapps/courseware/tests/test_block_render.py @@ -41,7 +41,6 @@ from xblock.runtime import DictKeyValueStore, KvsFieldData # lint-amnesty, pyli from xblock.test.tools import TestRuntime # lint-amnesty, pylint: disable=wrong-import-order from xmodule.capa.tests.response_xml_factory import OptionResponseXMLFactory # lint-amnesty, pylint: disable=reimported -from xmodule.capa.xqueue_interface import XQueueInterface from xmodule.capa_block import ProblemBlock from xmodule.contentstore.django import contentstore from xmodule.html_block import AboutBlock, CourseInfoBlock, HtmlBlock, StaticTabBlock @@ -121,7 +120,6 @@ TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT @XBlock.needs('content_type_gating') @XBlock.needs('cache') @XBlock.needs('sandbox') -@XBlock.needs('xqueue') @XBlock.needs('replace_urls') @XBlock.needs('rebind_user') @XBlock.needs('completion') @@ -2333,7 +2331,6 @@ class LMSXBlockServiceBindingTest(LMSXBlockServiceMixin): 'content_type_gating', 'cache', 'sandbox', - 'xqueue', 'replace_urls', 'rebind_user', 'completion', @@ -2865,47 +2862,6 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): rendered = self.block.runtime.render_template('templates/edxmako.html', {'element_id': 'hi'}) # pylint: disable=not-callable assert rendered == '
Testing the MakoService
\n' - def test_xqueue(self): - xqueue = self.block.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 = f'http://localhost:8000/courses/{self.course.id}/xqueue/232/{self.block.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): - _ = render.prepare_runtime_for_user( - self.user, - self.student_data, - self.block, - self.course.id, - self.track_function, - self.request_token, - course=self.course, - ) - xqueue = self.block.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/{self.course.id}/xqueue/232/{self.block.location}' - assert xqueue['construct_callback']() == f'{callback_url}/score_update' - assert xqueue['construct_callback']('mock_dispatch') == f'{callback_url}/mock_dispatch' - @override_settings(COURSES_WITH_UNSAFE_CODE=[r'course-v1:edX\+LmsModuleShimTest\+2021_Fall']) def test_can_execute_unsafe_code_when_allowed(self): assert self.block.runtime.can_execute_unsafe_code() diff --git a/xmodule/capa/tests/test_xqueue_interface.py b/xmodule/capa/tests/test_xqueue_interface.py index 315e0c0b0a..5fd913a15e 100644 --- a/xmodule/capa/tests/test_xqueue_interface.py +++ b/xmodule/capa/tests/test_xqueue_interface.py @@ -1,41 +1,52 @@ -# -*- coding: utf-8 -*- -""" -Tests the xqueue service interface. -""" +"""Test the XQueue service and interface.""" from unittest import TestCase +from unittest.mock import Mock + from django.conf import settings +from django.test.utils import override_settings +from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator +from xblock.fields import ScopeIds from xmodule.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' - + """Test the XQueue service methods.""" 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, - ) + location = BlockUsageLocator(CourseLocator("test_org", "test_course", "test_run"), "problem", "ExampleProblem") + self.block = Mock(scope_ids=ScopeIds('user1', 'mock_problem', location, location)) + self.service = XQueueService(self.block) def test_interface(self): + """Test that the `XQUEUE_INTERFACE` settings are passed from the service to the interface.""" assert isinstance(self.service.interface, XQueueInterface) + assert self.service.interface.url == 'http://sandbox-xqueue.edx.org' + assert self.service.interface.auth['username'] == 'lms' + assert self.service.interface.auth['password'] == '***REMOVED***' + assert self.service.interface.session.auth.username == 'anant' + assert self.service.interface.session.auth.password == 'agarwal' def test_construct_callback(self): - assert self.service.construct_callback() == 'https://lms.url/callback' + """Test that the XQueue callback is initialized correctly, and can be altered through the settings.""" + usage_id = self.block.scope_ids.usage_id + callback_url = f'courses/{usage_id.context_key}/xqueue/user1/{usage_id}' + + assert self.service.construct_callback() == f'{settings.LMS_ROOT_URL}/{callback_url}/score_update' + assert self.service.construct_callback('alt_dispatch') == f'{settings.LMS_ROOT_URL}/{callback_url}/alt_dispatch' + + custom_callback_url = 'http://alt.url' + with override_settings(XQUEUE_INTERFACE={**settings.XQUEUE_INTERFACE, 'callback_url': custom_callback_url}): + assert self.service.construct_callback() == f'{custom_callback_url}/{callback_url}/score_update' def test_default_queuename(self): - assert self.service.default_queuename == 'my-very-own-queue' + """Check the format of the default queue name.""" + assert self.service.default_queuename == 'test_org-test_course' def test_waittime(self): + """Check that the time between requests is retrieved correctly from the settings.""" assert self.service.waittime == 5 + + with override_settings(XQUEUE_WAITTIME_BETWEEN_REQUESTS=15): + assert self.service.waittime == 15 diff --git a/xmodule/capa/xqueue_interface.py b/xmodule/capa/xqueue_interface.py index f035d12201..aee7232a41 100644 --- a/xmodule/capa/xqueue_interface.py +++ b/xmodule/capa/xqueue_interface.py @@ -1,13 +1,19 @@ """ LMS Interface to external queueing system (xqueue) """ +from typing import Dict, Optional, TYPE_CHECKING import hashlib import json import logging import requests -import six +from django.conf import settings +from django.urls import reverse +from requests.auth import HTTPBasicAuth + +if TYPE_CHECKING: + from xmodule.capa_block import ProblemBlock log = logging.getLogger(__name__) dateformat = '%Y%m%d%H%M%S' @@ -25,7 +31,7 @@ def make_hashkey(seed): Generate a string key by hashing """ h = hashlib.md5() - h.update(six.b(str(seed))) + h.update(str(seed).encode('latin-1')) return h.hexdigest() @@ -65,13 +71,13 @@ def parse_xreply(xreply): return (return_code, content) -class XQueueInterface(object): +class XQueueInterface: """ Interface to the external grading system """ - def __init__(self, url, django_auth, requests_auth=None): - self.url = six.text_type(url) + def __init__(self, url: str, django_auth: Dict[str, str], requests_auth: Optional[HTTPBasicAuth] = None): + self.url = url self.auth = django_auth self.session = requests.Session() self.session.auth = requests_auth @@ -155,21 +161,17 @@ 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. + block: The `ProblemBlock` instance. """ - 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) + def __init__(self, block: 'ProblemBlock'): + basic_auth = settings.XQUEUE_INTERFACE.get('basic_auth') + requests_auth = HTTPBasicAuth(*basic_auth) if basic_auth else None + self._interface = XQueueInterface( + settings.XQUEUE_INTERFACE['url'], settings.XQUEUE_INTERFACE['django_auth'], requests_auth + ) - self._construct_callback = construct_callback - self._default_queuename = default_queuename.replace(' ', '_') - self._waittime = waittime + self._block = block @property def interface(self): @@ -178,23 +180,33 @@ class XQueueService: """ return self._interface - @property - def construct_callback(self): + def construct_callback(self, dispatch: str = 'score_update') -> str: """ - Returns the function to construct the XQueue callback. + Return a fully qualified callback URL for external queueing system. """ - return self._construct_callback + relative_xqueue_callback_url = reverse( + 'xqueue_callback', + kwargs=dict( + course_id=str(self._block.scope_ids.usage_id.context_key), + userid=str(self._block.scope_ids.user_id), + mod_id=str(self._block.scope_ids.usage_id), + 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 @property - def default_queuename(self): + def default_queuename(self) -> str: """ Returns the default queue name for the current course. """ - return self._default_queuename + course_id = self._block.scope_ids.usage_id.context_key + return f'{course_id.org}-{course_id.course}'.replace(' ', '_') @property - def waittime(self): + def waittime(self) -> int: """ Returns the number of seconds to wait in between calls to XQueue. """ - return self._waittime + return settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS diff --git a/xmodule/capa_block.py b/xmodule/capa_block.py index c70e619f27..9100077127 100644 --- a/xmodule/capa_block.py +++ b/xmodule/capa_block.py @@ -52,6 +52,7 @@ from common.djangoapps.xblock_django.constants import ( ATTR_KEY_USER_ID, ) from openedx.core.djangolib.markup import HTML, Text +from .capa.xqueue_interface import XQueueService from .fields import Date, ScoreField, Timedelta from .progress import Progress @@ -123,8 +124,6 @@ class Randomization(String): @XBlock.needs('cache') @XBlock.needs('sandbox') @XBlock.needs('replace_urls') -# Studio doesn't provide XQueueService, but the LMS does. -@XBlock.wants('xqueue') @XBlock.wants('call_to_action') class ProblemBlock( ScorableXBlockMixin, @@ -814,6 +813,8 @@ class ProblemBlock( sandbox_service = self.runtime.service(self, 'sandbox') cache_service = self.runtime.service(self, 'cache') + is_studio = getattr(self.runtime, 'is_author_mode', False) + capa_system = LoncapaSystem( ajax_url=self.ajax_url, anonymous_student_id=anonymous_student_id, @@ -825,7 +826,7 @@ class ProblemBlock( render_template=self.runtime.service(self, 'mako').render_template, resources_fs=self.runtime.resources_fs, seed=seed, # Why do we do this if we have self.seed? - xqueue=self.runtime.service(self, 'xqueue'), + xqueue=None if is_studio else XQueueService(self), matlab_api_key=self.matlab_api_key ) @@ -1736,7 +1737,7 @@ class ProblemBlock( if self.lcp.is_queued(): prev_submit_time = self.lcp.get_recentmost_queuetime() - xqueue_service = self.runtime.service(self, 'xqueue') + xqueue_service = self.lcp.capa_system.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( diff --git a/xmodule/tests/__init__.py b/xmodule/tests/__init__.py index a68c8161c2..b2cdd67b71 100644 --- a/xmodule/tests/__init__.py +++ b/xmodule/tests/__init__.py @@ -161,14 +161,6 @@ def get_test_system( services = { 'user': user_service, 'mako': mako_service, - '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="/"), - ), 'replace_urls': replace_url_service, 'cache': CacheService(DoNothingCache()), 'field-data': DictFieldData({}), @@ -224,14 +216,6 @@ def prepare_block_runtime( services = { 'user': user_service, 'mako': mako_service, - '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="/"), - ), 'replace_urls': replace_url_service, 'cache': CacheService(DoNothingCache()), 'field-data': DictFieldData({}), diff --git a/xmodule/x_module.py b/xmodule/x_module.py index b1c8d5bfb0..fbea2b2896 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -1239,31 +1239,6 @@ class ModuleSystemShim: """ self._deprecated_render_template = render_template - @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._runtime_services.get('xqueue') or 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 - @property def can_execute_unsafe_code(self): """