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

refactor: reuse services and wrappers between XBlocks

Browse Source
This commit is contained in:
Agrendalath
2023-06-22 12:11:29 +02:00
committed by Piotr Surowiec
parent 80374ed1ce
commit 36cc415fc2
19 changed files with 253 additions and 247 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
cms/djangoapps/contentstore/views/preview.py
View File

@@ -220,7 +220,7 @@ def _prepare_runtime_for_preview(request, block):
# Set up functions to modify the fragment produced by student_view # Set up functions to modify the fragment produced by student_view
block.runtime.wrappers = wrappers block.runtime.wrappers = wrappers
block.runtime.wrappers_asides = wrappers_asides block.runtime.wrappers_asides = wrappers_asides
block.runtime._runtime_services.update(services) # lint-amnesty, pylint: disable=protected-access block.runtime._services.update(services) # pylint: disable=protected-access
# xmodules can check for this attribute during rendering to determine if # xmodules can check for this attribute during rendering to determine if
# they are being rendered for preview (i.e. in Studio) # they are being rendered for preview (i.e. in Studio)

276
lms/djangoapps/courseware/block_render.py
View File

@@ -2,7 +2,7 @@
Block rendering Block rendering
""" """
from __future__ import annotations
import json import json
import logging import logging
import textwrap import textwrap
@@ -12,7 +12,7 @@ from functools import partial
from completion.services import CompletionService from completion.services import CompletionService
from django.conf import settings from django.conf import settings
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user
from django.core.cache import cache from django.core.cache import cache
from django.db import transaction from django.db import transaction
from django.http import Http404, HttpResponse, HttpResponseForbidden from django.http import Http404, HttpResponse, HttpResponseForbidden
@@ -33,6 +33,7 @@ from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.keys import CourseKey, UsageKey
from rest_framework.decorators import api_view from rest_framework.decorators import api_view
from rest_framework.exceptions import APIException from rest_framework.exceptions import APIException
from typing import Callable, TYPE_CHECKING
from web_fragments.fragment import Fragment from web_fragments.fragment import Fragment
from xblock.django.request import django_to_webob_request, webob_to_django_response from xblock.django.request import django_to_webob_request, webob_to_django_response
from xblock.exceptions import NoSuchHandlerError, NoSuchViewError from xblock.exceptions import NoSuchHandlerError, NoSuchViewError
@@ -97,6 +98,13 @@ from common.djangoapps.edxmako.services import MakoService
from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
from openedx.core.lib.cache_utils import CacheService from openedx.core.lib.cache_utils import CacheService
if TYPE_CHECKING:
from rest_framework.request import Request
from xblock.core import XBlock
from xblock.runtime import Runtime
from xmodule.course_block import CourseBlock
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
# TODO: course_id and course_key are used interchangeably in this file, which is wrong. # TODO: course_id and course_key are used interchangeably in this file, which is wrong.
@@ -289,8 +297,9 @@ def get_block(user, request, usage_key, field_data_cache, position=None, log_if_
and such works based on user. and such works based on user.
- usage_key : A UsageKey object identifying the module to load - usage_key : A UsageKey object identifying the module to load
- field_data_cache : a FieldDataCache - field_data_cache : a FieldDataCache
- position : extra information from URL for user-specified - position : Extra information from URL for user-specified position within module.
position within module It is used to determine the active tab within the `SequenceBlock`/subsection.
Once the legacy course experience is removed, it should be safe to remove this, too.
- log_if_not_found : If this is True, we log a debug message if we cannot find the requested xmodule. - log_if_not_found : If this is True, we log a debug message if we cannot find the requested xmodule.
- wrap_xblock_display : If this is True, wrap the output display in a single div to allow for the - wrap_xblock_display : If this is True, wrap the output display in a single div to allow for the
XModule javascript to be bound correctly XModule javascript to be bound correctly
@@ -364,10 +373,24 @@ def display_access_messages(user, block, view, frag, context): # pylint: disabl
# pylint: disable=too-many-statements # pylint: disable=too-many-statements
def get_block_for_descriptor(user, request, block, field_data_cache, course_key, def get_block_for_descriptor(
position=None, wrap_xblock_display=True, grade_bucket_type=None, user: User | AnonymousUser,
static_asset_path='', disable_staff_debug_info=False, request: Request | None,
course=None, will_recheck_access=False): block: XBlock,
field_data_cache: FieldDataCache | None,
course_key: CourseKey,
position: int | None = None,
wrap_xblock_display: bool = True,
grade_bucket_type: str | None = None,
static_asset_path: str = '',
disable_staff_debug_info: bool = False,
course: CourseBlock | None = None,
will_recheck_access: bool = False,
track_function: Callable[[str, dict], None] | None = None,
student_data: KvsFieldData | None = None,
request_token: str | None = None,
user_location: str | None = None,
) -> XBlock | None:
""" """
Implements get_block, extracting out the request-specific functionality. Implements get_block, extracting out the request-specific functionality.
@@ -375,49 +398,107 @@ def get_block_for_descriptor(user, request, block, field_data_cache, course_key,
See get_block() docstring for further details. See get_block() docstring for further details.
""" """
track_function = make_track_function(request) if request:
track_function = track_function or make_track_function(request)
user_location = user_location or getattr(request, 'session', {}).get('country_code')
request_token = request_token or xblock_request_token(request)
user_location = getattr(request, 'session', {}).get('country_code') if not student_data:
student_kvs = DjangoKeyValueStore(field_data_cache)
if is_masquerading_as_specific_student(user, course_key):
student_kvs = MasqueradingKeyValueStore(student_kvs, request.session)
student_data = KvsFieldData(student_kvs)
student_kvs = DjangoKeyValueStore(field_data_cache) # The runtime is already shared between XBlocks. If there are no wrappers, it is the first initialization.
if is_masquerading_as_specific_student(user, course_key): should_recreate_runtime = not block.runtime.wrappers
student_kvs = MasqueradingKeyValueStore(student_kvs, request.session)
student_data = KvsFieldData(student_kvs)
return get_block_for_descriptor_internal( # If the runtime was prepared for another user, it should be recreated.
user=user, # This part can be removed if we remove all user-specific handling from the runtime services and pull this
block=block, # information directly from XBlocks during the service initialization.
student_data=student_data, if not should_recreate_runtime:
course_id=course_key, # If the user service is absent (which should never happen), the runtime should be reinitialized.
track_function=track_function, # We retrieve this service directly to bypass service declaration checks.
position=position, if user_service := block.runtime._services.get('user'): # pylint: disable=protected-access
wrap_xblock_display=wrap_xblock_display, # Check the user ID bound to the runtime. This operation can run often for complex course structures, so we
grade_bucket_type=grade_bucket_type, # are accessing the protected attribute of the user service to reduce the number of queries.
static_asset_path=static_asset_path, should_recreate_runtime = user.id != user_service._django_user.id # pylint: disable=protected-access
user_location=user_location, else:
request_token=xblock_request_token(request), should_recreate_runtime = True
disable_staff_debug_info=disable_staff_debug_info,
course=course, if should_recreate_runtime:
will_recheck_access=will_recheck_access, prepare_runtime_for_user(
user=user,
student_data=student_data, # These have implicit user bindings, the rest of args are considered not to
runtime=block.runtime,
course_id=course_key,
track_function=track_function,
wrap_xblock_display=wrap_xblock_display,
grade_bucket_type=grade_bucket_type,
static_asset_path=static_asset_path,
user_location=user_location,
request_token=request_token,
disable_staff_debug_info=disable_staff_debug_info,
course=course,
will_recheck_access=will_recheck_access,
)
# Pass position specified in URL to runtime.
if position is not None:
try:
position = int(position)
except (ValueError, TypeError):
log.exception('Non-integer %r passed as position.', position)
position = None
block.runtime.set('position', position)
block.bind_for_student(
user.id,
[
partial(DateLookupFieldData, course_id=course_key, user=user),
partial(OverrideFieldData.wrap, user, course),
partial(LmsFieldData, student_data=student_data),
],
) )
# Do not check access when it's a noauth request.
# Not that the access check needs to happen after the block is bound
# for the student, since there may be field override data for the student
# that affects xblock visibility.
user_needs_access_check = getattr(user, 'known', True) and not isinstance(user, SystemUser)
if user_needs_access_check:
access = has_access(user, 'load', block, course_key)
# A block should only be returned if either the user has access, or the user doesn't have access, but
# the failed access has a message for the user and the caller of this function specifies it will check access
# again. This allows blocks to show specific error message or upsells when access is denied.
caller_will_handle_access_error = (
not access
and will_recheck_access
and (access.user_message or access.user_fragment)
)
if access or caller_will_handle_access_error:
block.has_access_error = bool(caller_will_handle_access_error)
return block
return None
return block
def prepare_runtime_for_user( def prepare_runtime_for_user(
user, user: User | AnonymousUser,
student_data, # TODO student_data: KvsFieldData,
# Arguments preceding this comment have user binding, those following don't # Arguments preceding this comment have user binding, those following don't
block, runtime: Runtime,
course_id, course_id: CourseKey,
track_function, track_function: Callable[[str, dict], None],
request_token, request_token: str,
position=None, position: int | None = None,
wrap_xblock_display=True, wrap_xblock_display: bool = True,
grade_bucket_type=None, grade_bucket_type: str | None = None,
static_asset_path='', static_asset_path: str = '',
user_location=None, user_location: str | None = None,
disable_staff_debug_info=False, disable_staff_debug_info: bool = False,
course=None, course: CourseBlock | None = None,
will_recheck_access=False, will_recheck_access: bool = False,
): ):
""" """
Helper function that binds the given xblock to a user and student_data to a user and the block. Helper function that binds the given xblock to a user and student_data to a user and the block.
@@ -427,25 +508,26 @@ def prepare_runtime_for_user(
The arguments fall into two categories: those that have explicit or implicit user binding, which are user The arguments fall into two categories: those that have explicit or implicit user binding, which are user
and student_data, and those don't and are used to instantiate the service required in LMS, which and student_data, and those don't and are used to instantiate the service required in LMS, which
are all the other arguments. Ultimately, this isn't too different than how get_block_for_descriptor_internal are all the other arguments.
was before refactoring.
Arguments: Arguments:
see arguments for get_block() see arguments for get_block()
request_token (str): A token unique to the request use by xblock initialization request_token (str): A token unique to the request use by xblock initialization
""" """
def inner_get_block(block): def inner_get_block(block: XBlock) -> XBlock | None:
""" """
Delegate to get_block_for_descriptor_internal() with all values except `block` set. Delegate to get_block_for_descriptor() with all values except `block` set.
Because it does an access check, it may return None. Because it does an access check, it may return None.
""" """
return get_block_for_descriptor_internal( return get_block_for_descriptor(
user=user, user=user,
request=None,
field_data_cache=None,
block=block, block=block,
student_data=student_data, student_data=student_data,
course_id=course_id, course_key=course_id,
track_function=track_function, track_function=track_function,
request_token=request_token, request_token=request_token,
position=position, position=position,
@@ -479,16 +561,6 @@ def prepare_runtime_for_user(
request_token=request_token, request_token=request_token,
)) ))
# HACK: The following test fails when we do not access this attribute.
# lms/djangoapps/courseware/tests/test_views.py::TestRenderXBlock::test_success_enrolled_staff
# This happens because accessing `field_data` caches the XBlock's parents through the inheritance mixin.
# If we do this operation before assigning `inner_get_block` to the `get_block_for_descriptor` attribute, these
# parents will be initialized without binding the user data (that's how the `get_block` works in `x_module.py`).
# If we retrieve the parents after this operation (e.g., in the `enclosing_sequence_for_gating_checks` function),
# they will have their runtimes initialized, which can lead to an altered behavior of the XBlock-specific
# parts of other runtimes. We will keep this workaround until we remove all XBlock-specific code from here.
block.static_asset_path # pylint: disable=pointless-statement
replace_url_service = partial( replace_url_service = partial(
ReplaceURLService, ReplaceURLService,
static_asset_path=static_asset_path, static_asset_path=static_asset_path,
@@ -553,7 +625,6 @@ def prepare_runtime_for_user(
'rebind_user': RebindUserService( 'rebind_user': RebindUserService(
user, user,
course_id, course_id,
prepare_runtime_for_user,
track_function=track_function, track_function=track_function,
position=position, position=position,
wrap_xblock_display=wrap_xblock_display, wrap_xblock_display=wrap_xblock_display,
@@ -576,86 +647,13 @@ def prepare_runtime_for_user(
'publish': EventPublishingService(user, course_id, track_function), 'publish': EventPublishingService(user, course_id, track_function),
} }
block.runtime.get_block_for_descriptor = inner_get_block runtime.get_block_for_descriptor = inner_get_block
block.runtime.wrappers = block_wrappers runtime.wrappers = block_wrappers
block.runtime._runtime_services.update(services) # lint-amnesty, pylint: disable=protected-access runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access
block.runtime.request_token = request_token runtime.request_token = request_token
block.runtime.wrap_asides_override = lms_wrappers_aside runtime.wrap_asides_override = lms_wrappers_aside
block.runtime.applicable_aside_types_override = lms_applicable_aside_types runtime.applicable_aside_types_override = lms_applicable_aside_types
# pass position specified in URL to runtime
if position is not None:
try:
position = int(position)
except (ValueError, TypeError):
log.exception('Non-integer %r passed as position.', position)
position = None
block.runtime.set('position', position)
# TODO: Find all the places that this method is called and figure out how to
# get a loaded course passed into it
def get_block_for_descriptor_internal(user, block, student_data, course_id, track_function, request_token,
position=None, wrap_xblock_display=True, grade_bucket_type=None,
static_asset_path='', user_location=None, disable_staff_debug_info=False,
course=None, will_recheck_access=False):
"""
Actually implement get_block, without requiring a request.
See get_block() docstring for further details.
Arguments:
request_token (str): A unique token for this request, used to isolate xblock rendering
"""
prepare_runtime_for_user(
user=user,
student_data=student_data, # These have implicit user bindings, the rest of args are considered not to
block=block,
course_id=course_id,
track_function=track_function,
position=position,
wrap_xblock_display=wrap_xblock_display,
grade_bucket_type=grade_bucket_type,
static_asset_path=static_asset_path,
user_location=user_location,
request_token=request_token,
disable_staff_debug_info=disable_staff_debug_info,
course=course,
will_recheck_access=will_recheck_access,
)
block.bind_for_student(
user.id,
[
partial(DateLookupFieldData, course_id=course_id, user=user),
partial(OverrideFieldData.wrap, user, course),
partial(LmsFieldData, student_data=student_data),
],
)
# Do not check access when it's a noauth request.
# Not that the access check needs to happen after the block is bound
# for the student, since there may be field override data for the student
# that affects xblock visibility.
user_needs_access_check = getattr(user, 'known', True) and not isinstance(user, SystemUser)
if user_needs_access_check:
access = has_access(user, 'load', block, course_id)
# A block should only be returned if either the user has access, or the user doesn't have access, but
# the failed access has a message for the user and the caller of this function specifies it will check access
# again. This allows blocks to show specific error message or upsells when access is denied.
caller_will_handle_access_error = (
not access
and will_recheck_access
and (access.user_message or access.user_fragment)
)
if access or caller_will_handle_access_error:
block.has_access_error = bool(caller_will_handle_access_error)
return block
return None
return block
def load_single_xblock(request, user_id, course_id, usage_key_string, course=None, will_recheck_access=False): def load_single_xblock(request, user_id, course_id, usage_key_string, course=None, will_recheck_access=False):

29
lms/djangoapps/courseware/tests/test_block_render.py
View File

@@ -949,7 +949,7 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas
request.user = self.mock_user request.user = self.mock_user
render.handle_xblock_callback(request, str(course.id), usage_id, handler) render.handle_xblock_callback(request, str(course.id), usage_id, handler)
assert mock_get_block.call_count == 1 assert mock_get_block.call_count == 2
assert mock_get_block.call_args[1]['will_recheck_access'] == will_recheck_access assert mock_get_block.call_args[1]['will_recheck_access'] == will_recheck_access
@@ -1922,14 +1922,16 @@ class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase)
if hasattr(xblock_class, 'module_class'): if hasattr(xblock_class, 'module_class'):
block.module_class = xblock_class.module_class block.module_class = xblock_class.module_class
rendered_block = render.get_block_for_descriptor_internal( rendered_block = render.get_block_for_descriptor(
user=self.user, user=self.user,
block=block, block=block,
student_data=Mock(spec=FieldData, name='student_data'), student_data=Mock(spec=FieldData, name='student_data'),
course_id=course_id, course_key=course_id,
track_function=Mock(name='track_function'), # Track Function track_function=Mock(name='track_function'), # Track Function
request_token='request_token', request_token='request_token',
course=self.course, course=self.course,
request=None,
field_data_cache=None,
) )
current_user = rendered_block.runtime.service(rendered_block, 'user').get_current_user() current_user = rendered_block.runtime.service(rendered_block, 'user').get_current_user()
@@ -2285,7 +2287,7 @@ class LMSXBlockServiceMixin(SharedModuleStoreTestCase):
render.prepare_runtime_for_user( render.prepare_runtime_for_user(
self.user, self.user,
self.student_data, self.student_data,
self.block, self.block.runtime,
self.course.id, self.course.id,
self.track_function, self.track_function,
self.request_token, self.request_token,
@@ -2439,7 +2441,6 @@ class TestI18nService(LMSXBlockServiceMixin):
Test: NoSuchServiceError should be raised if i18n service is none. Test: NoSuchServiceError should be raised if i18n service is none.
""" """
i18nService = self.block.runtime._services['i18n'] # pylint: disable=protected-access i18nService = self.block.runtime._services['i18n'] # pylint: disable=protected-access
self.block.runtime._runtime_services['i18n'] = None # pylint: disable=protected-access
self.block.runtime._services['i18n'] = None # pylint: disable=protected-access self.block.runtime._services['i18n'] = None # pylint: disable=protected-access
with pytest.raises(NoSuchServiceError): with pytest.raises(NoSuchServiceError):
self.block.runtime.service(self.block, 'i18n') self.block.runtime.service(self.block, 'i18n')
@@ -2656,7 +2657,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user( render.prepare_runtime_for_user(
self.user, self.user,
self.student_data, self.student_data,
self.block, self.block.runtime,
self.course.id, self.course.id,
self.track_function, self.track_function,
self.request_token, self.request_token,
@@ -2684,7 +2685,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user( render.prepare_runtime_for_user(
self.user, self.user,
self.student_data, self.student_data,
self.block, self.block.runtime,
self.course.id, self.course.id,
self.track_function, self.track_function,
self.request_token, self.request_token,
@@ -2706,7 +2707,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user( render.prepare_runtime_for_user(
self.user, self.user,
self.student_data, self.student_data,
self.block, self.block.runtime,
self.course.id, self.course.id,
self.track_function, self.track_function,
self.request_token, self.request_token,
@@ -2726,7 +2727,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user( render.prepare_runtime_for_user(
self.user, self.user,
self.student_data, self.student_data,
self.block, self.block.runtime,
self.course.id, self.course.id,
self.track_function, self.track_function,
self.request_token, self.request_token,
@@ -2747,7 +2748,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user( render.prepare_runtime_for_user(
self.user, self.user,
self.student_data, self.student_data,
self.block, self.block.runtime,
self.course.id, self.course.id,
self.track_function, self.track_function,
self.request_token, self.request_token,
@@ -2776,7 +2777,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user( render.prepare_runtime_for_user(
self.user, self.user,
self.student_data, self.student_data,
self.problem_block, self.problem_block.runtime,
self.course.id, self.course.id,
self.track_function, self.track_function,
self.request_token, self.request_token,
@@ -2790,7 +2791,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user( render.prepare_runtime_for_user(
self.user, self.user,
self.student_data, self.student_data,
self.block, self.block.runtime,
self.course.id, self.course.id,
self.track_function, self.track_function,
self.request_token, self.request_token,
@@ -2810,7 +2811,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user( render.prepare_runtime_for_user(
AnonymousUser(), AnonymousUser(),
self.student_data, self.student_data,
self.block, self.block.runtime,
self.course.id, self.course.id,
self.track_function, self.track_function,
self.request_token, self.request_token,
@@ -2839,7 +2840,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user( render.prepare_runtime_for_user(
self.user, self.user,
self.student_data, self.student_data,
self.block, self.block.runtime,
self.course.id, self.course.id,
self.track_function, self.track_function,
self.request_token, self.request_token,

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

@@ -21,7 +21,7 @@ from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE,
from xmodule.modulestore.tests.factories import BlockFactory, ToyCourseFactory from xmodule.modulestore.tests.factories import BlockFactory, ToyCourseFactory
from lms.djangoapps.course_api.blocks.tests.helpers import deserialize_usage_key from lms.djangoapps.course_api.blocks.tests.helpers import deserialize_usage_key
from lms.djangoapps.courseware.block_render import get_block_for_descriptor_internal from lms.djangoapps.courseware.block_render import get_block_for_descriptor
from lms.djangoapps.courseware.tests.helpers import XModuleRenderingTestBase from lms.djangoapps.courseware.tests.helpers import XModuleRenderingTestBase
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
@@ -293,13 +293,15 @@ class TestXBlockInCourse(SharedModuleStoreTestCase):
""" """
Test rendered DiscussionXBlock permissions. Test rendered DiscussionXBlock permissions.
""" """
discussion_xblock = get_block_for_descriptor_internal( discussion_xblock = get_block_for_descriptor(
user=self.user, user=self.user,
block=self.discussion, block=self.discussion,
student_data=mock.Mock(name='student_data'), student_data=mock.Mock(name='student_data'),
course_id=self.course.id, course_key=self.course.id,
track_function=mock.Mock(name='track_function'), track_function=mock.Mock(name='track_function'),
request_token='request_token', request_token='request_token',
request=None,
field_data_cache=None,
) )
fragment = discussion_xblock.render('student_view') fragment = discussion_xblock.render('student_view')
@@ -336,13 +338,15 @@ class TestXBlockInCourse(SharedModuleStoreTestCase):
assert orphan_sequential.location.block_id == root.location.block_id assert orphan_sequential.location.block_id == root.location.block_id
# Get xblock bound to a user and a block. # Get xblock bound to a user and a block.
discussion_xblock = get_block_for_descriptor_internal( discussion_xblock = get_block_for_descriptor(
user=self.user, user=self.user,
block=discussion, block=discussion,
student_data=mock.Mock(name='student_data'), student_data=mock.Mock(name='student_data'),
course_id=self.course.id, course_key=self.course.id,
track_function=mock.Mock(name='track_function'), track_function=mock.Mock(name='track_function'),
request_token='request_token', request_token='request_token',
request=None,
field_data_cache=None,
) )
fragment = discussion_xblock.render('student_view') fragment = discussion_xblock.render('student_view')
@@ -386,13 +390,15 @@ class TestXBlockInCourse(SharedModuleStoreTestCase):
provider_type=Provider.OPEN_EDX, provider_type=Provider.OPEN_EDX,
) )
discussion_xblock = get_block_for_descriptor_internal( discussion_xblock = get_block_for_descriptor(
user=self.user, user=self.user,
block=self.discussion, block=self.discussion,
student_data=mock.Mock(name='student_data'), student_data=mock.Mock(name='student_data'),
course_id=self.course.id, course_key=self.course.id,
track_function=mock.Mock(name='track_function'), track_function=mock.Mock(name='track_function'),
request_token='request_token', request_token='request_token',
request=None,
field_data_cache=None,
) )
fragment = discussion_xblock.render('student_view') fragment = discussion_xblock.render('student_view')
@@ -436,13 +442,15 @@ class TestXBlockQueryLoad(SharedModuleStoreTestCase):
num_queries = 6 num_queries = 6
for discussion in discussions: for discussion in discussions:
discussion_xblock = get_block_for_descriptor_internal( discussion_xblock = get_block_for_descriptor(
user=user, user=user,
block=discussion, block=discussion,
student_data=mock.Mock(name='student_data'), student_data=mock.Mock(name='student_data'),
course_id=course.id, course_key=course.id,
track_function=mock.Mock(name='track_function'), track_function=mock.Mock(name='track_function'),
request_token='request_token', request_token='request_token',
request=None,
field_data_cache=None,
) )
with self.assertNumQueries(num_queries): with self.assertNumQueries(num_queries):
fragment = discussion_xblock.render('student_view') fragment = discussion_xblock.render('student_view')

18
lms/djangoapps/courseware/testutils.py
View File

@@ -44,9 +44,8 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta):
'<div class="container"', '<div class="container"',
] ]
# DOM elements that appear in an xBlock, # DOM elements that should only be present when viewing the XBlock as staff.
# but are excluded from the xBlock-only rendering. XBLOCK_STAFF_DEBUG_INFO = [
XBLOCK_REMOVED_HTML_ELEMENTS = [
'<div class="wrap-instructor-info"', '<div class="wrap-instructor-info"',
] ]
@@ -143,14 +142,14 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta):
if login: if login:
self.login() self.login()
def verify_response(self, expected_response_code=200, url_params=None): def verify_response(self, expected_response_code=200, url_params=None, is_staff=False):
""" """
Helper method that calls the endpoint, verifies the expected response code, and returns the response. Helper method that calls the endpoint, verifies the expected response code, and returns the response.
Arguments: Arguments:
expected_response_code: The expected response code. expected_response_code: The expected response code.
url_params: URL parameters that will be encoded and passed to the request. url_params: URL parameters that will be encoded and passed to the request.
is_staff: Whether the user has staff permissions in the course.
""" """
if url_params: if url_params:
url_params = urlencode(url_params) url_params = urlencode(url_params)
@@ -158,8 +157,9 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta):
response = self.get_response(self.block_to_be_tested.location, url_params) response = self.get_response(self.block_to_be_tested.location, url_params)
if expected_response_code == 200: if expected_response_code == 200:
self.assertContains(response, self.html_block.data, status_code=expected_response_code) self.assertContains(response, self.html_block.data, status_code=expected_response_code)
unexpected_elements = self.block_specific_chrome_html_elements unexpected_elements = self.block_specific_chrome_html_elements + self.COURSEWARE_CHROME_HTML_ELEMENTS
unexpected_elements += self.COURSEWARE_CHROME_HTML_ELEMENTS + self.XBLOCK_REMOVED_HTML_ELEMENTS if not is_staff:
unexpected_elements += self.XBLOCK_STAFF_DEBUG_INFO
for chrome_element in unexpected_elements: for chrome_element in unexpected_elements:
self.assertNotContains(response, chrome_element) self.assertNotContains(response, chrome_element)
else: else:
@@ -202,12 +202,12 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta):
# (3) definition - HTML block # (3) definition - HTML block
# (4) definition - edx_notes decorator (original_get_html) # (4) definition - edx_notes decorator (original_get_html)
with check_mongo_calls(4): with check_mongo_calls(4):
self.verify_response() self.verify_response(is_staff=True)
def test_success_unenrolled_staff(self): def test_success_unenrolled_staff(self):
self.setup_course() self.setup_course()
self.setup_user(admin=True, enroll=False, login=True) self.setup_user(admin=True, enroll=False, login=True)
self.verify_response() self.verify_response(is_staff=True)
def test_success_unenrolled_staff_masquerading_as_student(self): def test_success_unenrolled_staff_masquerading_as_student(self):
self.setup_course() self.setup_course()

10
lms/djangoapps/courseware/views/views.py
View File

@@ -1521,7 +1521,7 @@ def _check_sequence_exam_access(request, location):
@xframe_options_exempt @xframe_options_exempt
@transaction.non_atomic_requests @transaction.non_atomic_requests
@ensure_csrf_cookie @ensure_csrf_cookie
def render_xblock(request, usage_key_string, check_if_enrolled=True): def render_xblock(request, usage_key_string, check_if_enrolled=True, disable_staff_debug_info=False):
""" """
Returns an HttpResponse with HTML content for the xBlock with the given usage_key. Returns an HttpResponse with HTML content for the xBlock with the given usage_key.
The returned HTML is a chromeless rendering of the xBlock (excluding content of the containing courseware). The returned HTML is a chromeless rendering of the xBlock (excluding content of the containing courseware).
@@ -1571,8 +1571,12 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True):
# get the block, which verifies whether the user has access to the block. # get the block, which verifies whether the user has access to the block.
recheck_access = request.GET.get('recheck_access') == '1' recheck_access = request.GET.get('recheck_access') == '1'
block, _ = get_block_by_usage_id( block, _ = get_block_by_usage_id(
request, str(course_key), str(usage_key), disable_staff_debug_info=True, course=course, request,
will_recheck_access=recheck_access str(course_key),
str(usage_key),
disable_staff_debug_info=disable_staff_debug_info,
course=course,
will_recheck_access=recheck_access,
) )
student_view_context = request.GET.dict() student_view_context = request.GET.dict()

24
lms/djangoapps/instructor_task/tasks_helper/module_state.py
View File

@@ -9,7 +9,6 @@ from time import time
from django.utils.translation import gettext_noop from django.utils.translation import gettext_noop
from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.keys import UsageKey
from xblock.runtime import KvsFieldData
from xblock.scorable import Score from xblock.scorable import Score
from xmodule.capa.responsetypes import LoncapaProblemError, ResponseError, StudentInputError from xmodule.capa.responsetypes import LoncapaProblemError, ResponseError, StudentInputError
@@ -18,9 +17,9 @@ from common.djangoapps.track.event_transaction_utils import create_new_event_tra
from common.djangoapps.track.views import task_track from common.djangoapps.track.views import task_track
from common.djangoapps.util.db import outer_atomic from common.djangoapps.util.db import outer_atomic
from lms.djangoapps.courseware.courses import get_problems_in_section from lms.djangoapps.courseware.courses import get_problems_in_section
from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache from lms.djangoapps.courseware.model_data import FieldDataCache
from lms.djangoapps.courseware.models import StudentModule from lms.djangoapps.courseware.models import StudentModule
from lms.djangoapps.courseware.block_render import get_block_for_descriptor_internal from lms.djangoapps.courseware.block_render import get_block_for_descriptor
from lms.djangoapps.grades.api import events as grades_events from lms.djangoapps.grades.api import events as grades_events
from openedx.core.lib.courses import get_course_by_id from openedx.core.lib.courses import get_course_by_id
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
@@ -334,16 +333,10 @@ def _get_module_instance_for_task(course_id, student, block, xblock_instance_arg
""" """
Fetches a StudentModule instance for a given `course_id`, `student` object, and `block`. Fetches a StudentModule instance for a given `course_id`, `student` object, and `block`.
`xblock_instance_args` is used to provide information for creating a track function and an XQueue callback. `xblock_instance_args` is used to provide information for creating a track function.
These are passed, along with `grade_bucket_type`, to get_block_for_descriptor_internal, which sidesteps It is passed, along with `grade_bucket_type`, to get_block_for_descriptor.
the need for a Request object when instantiating an xblock instance.
""" """
# reconstitute the problem's corresponding XBlock: # get request-related tracking information from args passthrough, and supplement with task-specific information:
field_data_cache = FieldDataCache.cache_for_block_descendents(course_id, student, block)
student_data = KvsFieldData(DjangoKeyValueStore(field_data_cache))
# get request-related tracking information from args passthrough, and supplement with task-specific
# information:
request_info = xblock_instance_args.get('request_info', {}) if xblock_instance_args is not None else {} request_info = xblock_instance_args.get('request_info', {}) if xblock_instance_args is not None else {}
task_info = {"student": student.username, "task_id": _get_task_id_from_xblock_args(xblock_instance_args)} task_info = {"student": student.username, "task_id": _get_task_id_from_xblock_args(xblock_instance_args)}
@@ -357,11 +350,12 @@ def _get_module_instance_for_task(course_id, student, block, xblock_instance_arg
''' '''
return lambda event_type, event: task_track(request_info, task_info, event_type, event, page='x_module_task') return lambda event_type, event: task_track(request_info, task_info, event_type, event, page='x_module_task')
return get_block_for_descriptor_internal( return get_block_for_descriptor(
user=student, user=student,
request=None,
block=block, block=block,
student_data=student_data, field_data_cache=FieldDataCache.cache_for_block_descendents(course_id, student, block),
course_id=course_id, course_key=course_id,
track_function=make_track_function(), track_function=make_track_function(),
grade_bucket_type=grade_bucket_type, grade_bucket_type=grade_bucket_type,
# This module isn't being used for front-end rendering # This module isn't being used for front-end rendering

12
lms/djangoapps/instructor_task/tests/test_tasks.py
View File

@@ -292,7 +292,7 @@ class TestOverrideScoreInstructorTask(TestInstructorTasks):
mock_instance = MagicMock() mock_instance = MagicMock()
del mock_instance.set_score del mock_instance.set_score
with patch( with patch(
'lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor_internal' 'lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor'
) as mock_get_block: ) as mock_get_block:
mock_get_block.return_value = mock_instance mock_get_block.return_value = mock_instance
with pytest.raises(UpdateProblemModuleStateError): with pytest.raises(UpdateProblemModuleStateError):
@@ -313,7 +313,7 @@ class TestOverrideScoreInstructorTask(TestInstructorTasks):
num_students = 1 num_students = 1
self._create_students_with_state(num_students, input_state) self._create_students_with_state(num_students, input_state)
task_entry = self._create_input_entry(score=0) task_entry = self._create_input_entry(score=0)
with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor_internal', with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor',
return_value=None): return_value=None):
self._run_task_with_mock_celery(override_problem_score, task_entry.id, task_entry.task_id) self._run_task_with_mock_celery(override_problem_score, task_entry.id, task_entry.task_id)
@@ -338,7 +338,7 @@ class TestOverrideScoreInstructorTask(TestInstructorTasks):
self._create_students_with_state(num_students) self._create_students_with_state(num_students)
task_entry = self._create_input_entry(score=0) task_entry = self._create_input_entry(score=0)
with patch( with patch(
'lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor_internal' 'lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor'
) as mock_get_block: ) as mock_get_block:
mock_get_block.return_value = mock_instance mock_get_block.return_value = mock_instance
mock_instance.max_score = MagicMock(return_value=99999.0) mock_instance.max_score = MagicMock(return_value=99999.0)
@@ -425,7 +425,7 @@ class TestRescoreInstructorTask(TestInstructorTasks):
mock_instance = MagicMock() mock_instance = MagicMock()
del mock_instance.rescore_problem del mock_instance.rescore_problem
del mock_instance.rescore del mock_instance.rescore
with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor_internal') as mock_get_block: # lint-amnesty, pylint: disable=line-too-long with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor') as mock_get_block: # lint-amnesty, pylint: disable=line-too-long
mock_get_block.return_value = mock_instance mock_get_block.return_value = mock_instance
with pytest.raises(UpdateProblemModuleStateError): with pytest.raises(UpdateProblemModuleStateError):
self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id) self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id)
@@ -448,7 +448,7 @@ class TestRescoreInstructorTask(TestInstructorTasks):
num_students = 1 num_students = 1
self._create_students_with_state(num_students, input_state) self._create_students_with_state(num_students, input_state)
task_entry = self._create_input_entry() task_entry = self._create_input_entry()
with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor_internal', return_value=None): # lint-amnesty, pylint: disable=line-too-long with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor', return_value=None): # lint-amnesty, pylint: disable=line-too-long
self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id) self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id)
self.assert_task_output( self.assert_task_output(
@@ -474,7 +474,7 @@ class TestRescoreInstructorTask(TestInstructorTasks):
self._create_students_with_state(num_students) self._create_students_with_state(num_students)
task_entry = self._create_input_entry() task_entry = self._create_input_entry()
with patch( with patch(
'lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor_internal' 'lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor'
) as mock_get_block: ) as mock_get_block:
mock_get_block.return_value = mock_instance mock_get_block.return_value = mock_instance
self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id) self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id)

12
lms/djangoapps/lti_provider/tests/test_views.py
View File

@@ -112,6 +112,18 @@ class LtiLaunchTest(LtiTestMixin, TestCase):
self.consumer self.consumer
) )
@patch('lms.djangoapps.courseware.views.views.render_xblock')
@patch('lms.djangoapps.lti_provider.views.authenticate_lti_user')
def test_render_xblock_params(self, _authenticate, render):
"""
Verifies that the LTI renders an XBlock without:
1. Checking the enrollment.
2. Displaying staff debug info.
"""
request = build_launch_request()
views.lti_launch(request, str(COURSE_KEY), str(USAGE_KEY))
render.assert_called_with(request, str(USAGE_KEY), check_if_enrolled=False, disable_staff_debug_info=True)
@patch('lms.djangoapps.lti_provider.views.render_courseware') @patch('lms.djangoapps.lti_provider.views.render_courseware')
@patch('lms.djangoapps.lti_provider.views.store_outcome_parameters') @patch('lms.djangoapps.lti_provider.views.store_outcome_parameters')
@patch('lms.djangoapps.lti_provider.views.authenticate_lti_user') @patch('lms.djangoapps.lti_provider.views.authenticate_lti_user')

2
lms/djangoapps/lti_provider/views.py
View File

@@ -145,7 +145,7 @@ def render_courseware(request, usage_key):
""" """
# return an HttpResponse object that contains the template and necessary context to render the courseware. # return an HttpResponse object that contains the template and necessary context to render the courseware.
from lms.djangoapps.courseware.views.views import render_xblock from lms.djangoapps.courseware.views.views import render_xblock
return render_xblock(request, str(usage_key), check_if_enrolled=False) return render_xblock(request, str(usage_key), check_if_enrolled=False, disable_staff_debug_info=True)
def parse_course_and_usage_keys(course_id, usage_id): def parse_course_and_usage_keys(course_id, usage_id):

2
openedx/core/djangoapps/xblock/runtime/runtime.py
View File

@@ -32,7 +32,6 @@ from common.djangoapps.track import contexts as track_contexts
from common.djangoapps.track import views as track_views from common.djangoapps.track import views as track_views
from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache
from lms.djangoapps.courseware import block_render
from lms.djangoapps.grades.api import signals as grades_signals from lms.djangoapps.grades.api import signals as grades_signals
from openedx.core.djangoapps.xblock.apps import get_xblock_app_config from openedx.core.djangoapps.xblock.apps import get_xblock_app_config
from openedx.core.djangoapps.xblock.runtime.blockstore_field_data import BlockstoreChildrenData, BlockstoreFieldData from openedx.core.djangoapps.xblock.runtime.blockstore_field_data import BlockstoreChildrenData, BlockstoreFieldData
@@ -270,7 +269,6 @@ class XBlockRuntime(RuntimeShim, Runtime):
return RebindUserService( return RebindUserService(
self.user, self.user,
context_key, context_key,
block_render.prepare_runtime_for_user,
track_function=make_track_function(), track_function=make_track_function(),
request_token=request_token(crum.get_current_request()), request_token=request_token(crum.get_current_request()),
) )

13
xmodule/services.py
View File

@@ -157,19 +157,13 @@ class RebindUserService(Service):
user (User) - A Django User object user (User) - A Django User object
course_id (str) - Course ID course_id (str) - Course ID
course (Course) - Course Object 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` kwargs (dict) - all the keyword arguments that need to be passed to the `prepare_runtime_for_user`
function when it is called during rebinding function when it is called during rebinding
""" """
def __init__(self, user, course_id, prepare_runtime_for_user, **kwargs): def __init__(self, user, course_id, **kwargs):
super().__init__(**kwargs) super().__init__(**kwargs)
self.user = user self.user = user
self.course_id = course_id self.course_id = course_id
self._ref = {
"prepare_runtime_for_user": prepare_runtime_for_user
}
self._kwargs = kwargs self._kwargs = kwargs
def rebind_noauth_module_to_user(self, block, real_user): def rebind_noauth_module_to_user(self, block, real_user):
@@ -201,10 +195,11 @@ class RebindUserService(Service):
with modulestore().bulk_operations(self.course_id): with modulestore().bulk_operations(self.course_id):
course = modulestore().get_course(course_key=self.course_id) course = modulestore().get_course(course_key=self.course_id)
self._ref["prepare_runtime_for_user"]( from lms.djangoapps.courseware.block_render import prepare_runtime_for_user
prepare_runtime_for_user(
user=real_user, user=real_user,
student_data=student_data_real_user, # These have implicit user bindings, rest of args considered not to student_data=student_data_real_user, # These have implicit user bindings, rest of args considered not to
block=block, runtime=block.runtime,
course_id=self.course_id, course_id=self.course_id,
course=course, course=course,
**self._kwargs **self._kwargs

2
xmodule/tests/test_library_content.py
View File

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

2
xmodule/tests/test_lti20_unit.py
View File

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

12
xmodule/tests/test_lti_unit.py
View File

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

12
xmodule/tests/test_sequence.py
View File

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

2
xmodule/tests/test_split_test_block.py
View File

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

8
xmodule/tests/test_vertical.py
View File

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

36
xmodule/x_module.py
View File

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