Revert "refactor: reuse services and wrappers between XBlocks" (#32730)

This reverts commit 36cc415fc2.
This commit is contained in:
Piotr Surowiec
2023-07-13 16:05:34 +02:00
committed by GitHub
parent f6071490e8
commit c6bd98e51a
19 changed files with 247 additions and 253 deletions

View File

@@ -157,13 +157,19 @@ class RebindUserService(Service):
user (User) - A Django User object
course_id (str) - Course ID
course (Course) - Course Object
prepare_runtime_for_user (function) - The helper function that will be called to create a module system
for a specfic user. This is the parent function from which this service was reactored out.
`lms.djangoapps.courseware.block_render.prepare_runtime_for_user`
kwargs (dict) - all the keyword arguments that need to be passed to the `prepare_runtime_for_user`
function when it is called during rebinding
"""
def __init__(self, user, course_id, **kwargs):
def __init__(self, user, course_id, prepare_runtime_for_user, **kwargs):
super().__init__(**kwargs)
self.user = user
self.course_id = course_id
self._ref = {
"prepare_runtime_for_user": prepare_runtime_for_user
}
self._kwargs = kwargs
def rebind_noauth_module_to_user(self, block, real_user):
@@ -195,11 +201,10 @@ class RebindUserService(Service):
with modulestore().bulk_operations(self.course_id):
course = modulestore().get_course(course_key=self.course_id)
from lms.djangoapps.courseware.block_render import prepare_runtime_for_user
prepare_runtime_for_user(
self._ref["prepare_runtime_for_user"](
user=real_user,
student_data=student_data_real_user, # These have implicit user bindings, rest of args considered not to
runtime=block.runtime,
block=block,
course_id=self.course_id,
course=course,
**self._kwargs

View File

@@ -59,7 +59,7 @@ class LibraryContentTest(MixedSplitTestCase):
Bind a block (part of self.course) so we can access student-specific data.
"""
prepare_block_runtime(block.runtime, course_id=block.location.course_key)
block.runtime._services.update({'library_tools': self.tools}) # lint-amnesty, pylint: disable=protected-access
block.runtime._runtime_services.update({'library_tools': self.tools}) # lint-amnesty, pylint: disable=protected-access
def get_block(descriptor):
"""Mocks module_system get_block function"""

View File

@@ -370,7 +370,7 @@ class LTI20RESTResultServiceTest(unittest.TestCase):
Test that we get a 404 when the supplied user does not exist
"""
self.setup_system_xblock_mocks_for_lti20_request_test()
self.runtime._services['user'] = StubUserService(user=None) # pylint: disable=protected-access
self.runtime._runtime_services['user'] = StubUserService(user=None) # pylint: disable=protected-access
mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT)
response = self.xblock.lti_2_0_result_rest_handler(mock_request, "user/abcd")
assert response.status_code == 404

View File

@@ -63,16 +63,16 @@ class LTIBlockTest(TestCase):
</imsx_POXEnvelopeRequest>
""")
self.course_id = CourseKey.from_string('org/course/run')
self.runtime = get_test_system(self.course_id)
self.runtime.publish = Mock()
self.runtime._services['rebind_user'] = Mock() # pylint: disable=protected-access
self.system = get_test_system(self.course_id)
self.system.publish = Mock()
self.system._runtime_services['rebind_user'] = Mock() # pylint: disable=protected-access
self.xblock = LTIBlock(
self.runtime,
self.system,
DictFieldData({}),
ScopeIds(None, None, None, BlockUsageLocator(self.course_id, 'lti', 'name'))
)
current_user = self.runtime.service(self.xblock, 'user').get_current_user()
current_user = self.system.service(self.xblock, 'user').get_current_user()
self.user_id = current_user.opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
self.lti_id = self.xblock.lti_id
@@ -178,7 +178,7 @@ class LTIBlockTest(TestCase):
"""
If we have no real user, we should send back failure response.
"""
self.runtime._services['user'] = StubUserService(user=None) # pylint: disable=protected-access
self.system._runtime_services['user'] = StubUserService(user=None) # pylint: disable=protected-access
self.xblock.verify_oauth_body_sign = Mock()
self.xblock.has_score = True
request = Request(self.environ)

View File

@@ -94,8 +94,8 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
self._set_up_module_system(block)
block.runtime._services['bookmarks'] = Mock() # pylint: disable=protected-access
block.runtime._services['user'] = StubUserService(user=Mock()) # pylint: disable=protected-access
block.runtime._runtime_services['bookmarks'] = Mock() # pylint: disable=protected-access
block.runtime._runtime_services['user'] = StubUserService(user=Mock()) # pylint: disable=protected-access
block.parent = parent.location
return block
@@ -368,7 +368,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
def test_xblock_handler_get_completion_success(self):
"""Test that the completion data is returned successfully on targeted vertical through ajax call"""
self.sequence_3_1.runtime._services['completion'] = Mock( # pylint: disable=protected-access
self.sequence_3_1.runtime._runtime_services['completion'] = Mock( # pylint: disable=protected-access
return_value=Mock(vertical_is_complete=Mock(return_value=True))
)
for child in self.sequence_3_1.get_children():
@@ -380,7 +380,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
)
completion_return = self.sequence_3_1.handle('get_completion', request)
assert completion_return.json == {'complete': True}
self.sequence_3_1.runtime._services['completion'] = None # pylint: disable=protected-access
self.sequence_3_1.runtime._runtime_services['completion'] = None # pylint: disable=protected-access
def test_xblock_handler_get_completion_bad_key(self):
"""Test that the completion data is returned as False when usage key is None through ajax call"""
@@ -394,14 +394,14 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
def test_handle_ajax_get_completion_success(self):
"""Test that the old-style ajax handler for completion still works"""
self.sequence_3_1.runtime._services['completion'] = Mock( # pylint: disable=protected-access
self.sequence_3_1.runtime._runtime_services['completion'] = Mock( # pylint: disable=protected-access
return_value=Mock(vertical_is_complete=Mock(return_value=True))
)
for child in self.sequence_3_1.get_children():
usage_key = str(child.location)
completion_return = self.sequence_3_1.handle_ajax('get_completion', {'usage_key': usage_key})
assert json.loads(completion_return) == {'complete': True}
self.sequence_3_1.runtime._services['completion'] = None # pylint: disable=protected-access
self.sequence_3_1.runtime._runtime_services['completion'] = None # pylint: disable=protected-access
def test_xblock_handler_goto_position_success(self):
"""Test that we can set position through ajax call"""

View File

@@ -106,7 +106,7 @@ class SplitTestBlockTest(XModuleXmlImportTest, PartitionTestCase):
self.course,
course_id=self.course.id,
)
self.course.runtime._services['partitions'] = partitions_service # pylint: disable=protected-access
self.course.runtime._runtime_services['partitions'] = partitions_service # pylint: disable=protected-access
# Mock user_service user
user_service = Mock()

View File

@@ -212,17 +212,17 @@ class VerticalBlockTestCase(BaseVerticalBlockTest):
"""
Test the rendering of the student and public view.
"""
self.course.runtime._services['bookmarks'] = Mock()
self.course.runtime._runtime_services['bookmarks'] = Mock()
now = datetime.now(pytz.UTC)
self.vertical.due = now + timedelta(days=days)
if view == STUDENT_VIEW:
self.course.runtime._services['user'] = StubUserService(user=Mock(username=self.username))
self.course.runtime._services['completion'] = StubCompletionService(
self.course.runtime._runtime_services['user'] = StubUserService(user=Mock(username=self.username))
self.course.runtime._runtime_services['completion'] = StubCompletionService(
enabled=True,
completion_value=completion_value
)
elif view == PUBLIC_VIEW:
self.course.runtime._services['user'] = StubUserService(user=AnonymousUser())
self.course.runtime._runtime_services['user'] = StubUserService(user=AnonymousUser())
html = self.course.runtime.render(
self.vertical, view, self.default_context if context is None else context

View File

@@ -1077,7 +1077,7 @@ class ModuleSystemShim:
'runtime.anonymous_student_id is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=3,
)
user_service = self._services.get('user')
user_service = self._runtime_services.get('user') or self._services.get('user')
if user_service:
return user_service.get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
return None
@@ -1107,7 +1107,7 @@ class ModuleSystemShim:
'runtime.user_id is deprecated. Use block.scope_ids.user_id or the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._services.get('user')
user_service = self._runtime_services.get('user') or self._services.get('user')
if user_service:
return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_ID)
return None
@@ -1123,7 +1123,7 @@ class ModuleSystemShim:
'runtime.user_is_staff is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._services.get('user')
user_service = self._runtime_services.get('user') or self._services.get('user')
if user_service:
return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
return None
@@ -1139,7 +1139,7 @@ class ModuleSystemShim:
'runtime.user_location is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._services.get('user')
user_service = self._runtime_services.get('user') or self._services.get('user')
if user_service:
return user_service.get_current_user().opt_attrs.get(ATTR_KEY_REQUEST_COUNTRY_CODE)
return None
@@ -1159,7 +1159,7 @@ class ModuleSystemShim:
'runtime.get_real_user is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._services.get('user')
user_service = self._runtime_services.get('user') or self._services.get('user')
if user_service:
return user_service.get_user_by_anonymous_id
return None
@@ -1177,7 +1177,7 @@ class ModuleSystemShim:
'runtime.get_user_role is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._services.get('user')
user_service = self._runtime_services.get('user') or self._services.get('user')
if user_service:
return partial(user_service.get_current_user().opt_attrs.get, ATTR_KEY_USER_ROLE)
@@ -1192,7 +1192,7 @@ class ModuleSystemShim:
'runtime.user_is_beta_tester is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._services.get('user')
user_service = self._runtime_services.get('user') or self._services.get('user')
if user_service:
return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_BETA_TESTER)
@@ -1207,7 +1207,7 @@ class ModuleSystemShim:
'runtime.user_is_admin is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._services.get('user')
user_service = self._runtime_services.get('user') or self._services.get('user')
if user_service:
return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_GLOBAL_STAFF)
@@ -1225,7 +1225,7 @@ class ModuleSystemShim:
)
if hasattr(self, '_deprecated_render_template'):
return self._deprecated_render_template
render_service = self._services.get('mako')
render_service = self._runtime_services.get('mako') or self._services.get('mako')
if render_service:
return render_service.render_template
return None
@@ -1251,7 +1251,7 @@ class ModuleSystemShim:
'runtime.can_execute_unsafe_code is deprecated. Please use the sandbox service instead.',
DeprecationWarning, stacklevel=2,
)
sandbox_service = self._services.get('sandbox')
sandbox_service = self._runtime_services.get('sandbox') or self._services.get('sandbox')
if sandbox_service:
return sandbox_service.can_execute_unsafe_code
# Default to saying "no unsafe code".
@@ -1271,7 +1271,7 @@ class ModuleSystemShim:
'runtime.get_python_lib_zip is deprecated. Please use the sandbox service instead.',
DeprecationWarning, stacklevel=2,
)
sandbox_service = self._services.get('sandbox')
sandbox_service = self._runtime_services.get('sandbox') or self._services.get('sandbox')
if sandbox_service:
return sandbox_service.get_python_lib_zip
# Default to saying "no lib data"
@@ -1290,7 +1290,7 @@ class ModuleSystemShim:
'runtime.cache is deprecated. Please use the cache service instead.',
DeprecationWarning, stacklevel=2,
)
return self._services.get('cache') or DoNothingCache()
return self._runtime_services.get('cache') or self._services.get('cache') or DoNothingCache()
@property
def filestore(self):
@@ -1341,7 +1341,7 @@ class ModuleSystemShim:
"rebind_noauth_module_to_user is deprecated. Please use the 'rebind_user' service instead.",
DeprecationWarning, stacklevel=2,
)
rebind_user_service = self._services.get('rebind_user')
rebind_user_service = self._runtime_services.get('rebind_user') or self._services.get('rebind_user')
if rebind_user_service:
return partial(rebind_user_service.rebind_noauth_module_to_user)
@@ -1421,6 +1421,7 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh
self.get_policy = lambda u: {}
self.disabled_xblock_types = disabled_xblock_types
self._runtime_services = {}
def get(self, attr):
""" provide uniform access to attributes (like etree)."""
@@ -1518,7 +1519,7 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh
Publish events through the `EventPublishingService`.
This ensures that the correct track method is used for Instructor tasks.
"""
if publish_service := self._services.get('publish'):
if publish_service := self._runtime_services.get('publish') or self._services.get('publish'):
publish_service.publish(block, event_type, event)
def service(self, block, service_name):
@@ -1535,8 +1536,11 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh
Returns:
An object implementing the requested service, or None.
"""
# Getting the service from parent module. making sure of block service declarations.
service = super().service(block=block, service_name=service_name)
declaration = block.service_declaration(service_name)
service = self._runtime_services.get(service_name)
if declaration is None or service is None:
# getting the service from parent module. making sure of block service declarations.
service = super().service(block=block, service_name=service_name)
# Passing the block to service if it is callable e.g. XBlockI18nService. It is the responsibility of calling
# service to handle the passing argument.
if callable(service):