[BD-13][BB-6145] Move the rebind_noauth_module_to_user from ModuleSystem argument to service

common/lib/xmodule/xmodule/lti_2_util.py

@@ -79,7 +79,7 @@ def lti_2_0_result_rest_handler(self, request, suffix):
         except LTIError:
             return Response(status=401)  # Unauthorized in this case.  401 is right
 
-        real_user = self.system.service(self, 'user').get_user_by_anonymous_id(anon_id)
+        real_user = self.runtime.service(self, 'user').get_user_by_anonymous_id(anon_id)
         if not real_user:  # that means we can't save to database, as we do not have real user id.
             msg = f"[LTI]: Real user not found against anon_id: {anon_id}"
             log.info(msg)
@@ -171,7 +171,7 @@ def _lti_2_0_result_get_handler(self, request, real_user):
             "@context": "http://purl.imsglobal.org/ctx/lis/v2/Result",
             "@type": "Result"
         }
-        self.system.rebind_noauth_module_to_user(self, real_user)
+        self.runtime.service(self, 'rebind_user').rebind_noauth_module_to_user(self, real_user)
         if self.module_score is None:  # In this case, no score has been ever set
             return Response(json.dumps(base_json_obj).encode('utf-8'), content_type=LTI_2_0_JSON_CONTENT_TYPE)
 
@@ -254,10 +254,10 @@ def set_user_module_score(self, user, score, max_score, comment="", score_delete
         else:
             scaled_score = None
 
-        self.system.rebind_noauth_module_to_user(self, user)
+        self.runtime.service(self, 'rebind_user').rebind_noauth_module_to_user(self, user)
 
         # have to publish for the progress page...
-        self.system.publish(
+        self.runtime.publish(
             self,
             'grade',
             {

common/lib/xmodule/xmodule/lti_module.py

@@ -275,6 +275,7 @@ class LTIFields:
 @XBlock.needs("i18n")
 @XBlock.needs("mako")
 @XBlock.needs("user")
+@XBlock.needs("rebind_user")
 class LTIBlock(
     LTIFields,
     LTI20BlockMixin,

common/lib/xmodule/xmodule/services.py

@@ -4,10 +4,26 @@
 
 
 import inspect
+import logging
+
+from functools import partial
 
 from config_models.models import ConfigurationModel
 from django.conf import settings
+from edx_when.field_data import DateLookupFieldData
 from xmodule.modulestore.django import modulestore
+from xblock.reference.plugins import Service
+from xblock.runtime import KvsFieldData
+
+
+from lms.djangoapps.courseware.field_overrides import OverrideFieldData
+from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache
+from lms.djangoapps.lms_xblock.field_data import LmsFieldData
+from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig
+from openedx.core.lib.courses import get_course_by_id
+
+
+log = logging.getLogger(__name__)
 
 
 class SettingsService:
@@ -119,3 +135,91 @@ def get_teams_configuration(self, course_id):
         if not self._course:
             self._course = self.get_course(course_id)
         return self._course.teams_configuration
+
+
+class RebindUserServiceError(Exception):
+    pass
+
+
+class RebindUserService(Service):
+    """
+    An XBlock Service that allows modules to get rebound to real users if it was previously bound to an AnonymousUser.
+
+    This used to be a local function inside the `lms.djangoapps.courseware.module_render.get_module_system_for_user`
+    method, and was passed as a constructor argument to x_module.ModuleSystem. This has been refactored out into a
+    service to simplify the ModuleSystem and lives in this module temporarily.
+
+    TODO: Only the old LTI XBlock uses it in 2 places for LTI 2.0 integration. As the LTI XBlock is deprecated in
+    favour of the LTI Consumer XBlock, this should be removed when the LTI XBlock is removed.
+
+    Arguments:
+        user (User) - A Django User object
+        course_id (str) - Course ID
+        course (Course) - Course Object
+        get_module_system_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.module_render.get_module_system_for_user`
+        kwargs (dict) - all the keyword arguments that need to be passed to the `get_module_system_for_user`
+            function when it is called during rebinding
+    """
+    def __init__(self, user, course_id, get_module_system_for_user, **kwargs):
+        super().__init__(**kwargs)
+        self.user = user
+        self.course_id = course_id
+        self._ref = {
+            "get_module_system_for_user": get_module_system_for_user
+        }
+        self._kwargs = kwargs
+
+    def rebind_noauth_module_to_user(self, block, real_user):
+        """
+        Function that rebinds the module to the real_user.
+
+        Will only work within a module bound to an AnonymousUser, e.g. one that's instantiated by the noauth_handler.
+
+        Arguments:
+            block (any xblock type):  the module to rebind
+            real_user (django.contrib.auth.models.User):  the user to bind to
+
+        Returns:
+            nothing (but the side effect is that module is re-bound to real_user)
+        """
+        if self.user.is_authenticated:
+            err_msg = "rebind_noauth_module_to_user can only be called from a module bound to an anonymous user"
+            log.error(err_msg)
+            raise RebindUserServiceError(err_msg)
+
+        field_data_cache_real_user = FieldDataCache.cache_for_descriptor_descendents(
+            self.course_id,
+            real_user,
+            block,
+            asides=XBlockAsidesConfig.possible_asides(),
+        )
+        student_data_real_user = KvsFieldData(DjangoKeyValueStore(field_data_cache_real_user))
+
+        with modulestore().bulk_operations(self.course_id):
+            course = modulestore().get_course(course_key=self.course_id)
+
+        (inner_system, inner_student_data) = self._ref["get_module_system_for_user"](
+            user=real_user,
+            student_data=student_data_real_user,  # These have implicit user bindings, rest of args considered not to
+            descriptor=block,
+            course_id=self.course_id,
+            course=course,
+            **self._kwargs
+        )
+
+        block.bind_for_student(
+            inner_system,
+            real_user.id,
+            [
+                partial(DateLookupFieldData, course_id=self.course_id, user=self.user),
+                partial(OverrideFieldData.wrap, real_user, course),
+                partial(LmsFieldData, student_data=inner_student_data),
+            ],
+        )
+
+        block.scope_ids = block.scope_ids._replace(user_id=real_user.id)
+        # now bind the module to the new ModuleSystem instance and vice-versa
+        block.runtime = inner_system
+        inner_system.xmodule_instance = block

common/lib/xmodule/xmodule/tests/test_lti20_unit.py

@@ -24,12 +24,12 @@ class LTI20RESTResultServiceTest(unittest.TestCase):
 
     def setUp(self):
         super().setUp()
-        self.system = get_test_system(user=self.USER_STANDIN)
+        self.runtime = get_test_system(user=self.USER_STANDIN)
         self.environ = {'wsgi.url_scheme': 'http', 'REQUEST_METHOD': 'POST'}
-        self.system.publish = Mock()
-        self.system.rebind_noauth_module_to_user = Mock()
+        self.runtime.publish = Mock()
+        self.runtime._services['rebind_user'] = Mock()  # pylint: disable=protected-access
 
-        self.xmodule = LTIBlock(self.system, DictFieldData({}), Mock())
+        self.xmodule = LTIBlock(self.runtime, DictFieldData({}), Mock())
         self.lti_id = self.xmodule.lti_id
         self.xmodule.due = None
         self.xmodule.graceperiod = None
@@ -250,7 +250,7 @@ def test_lti20_put_like_delete_success(self):
         assert response.status_code == 200
         assert self.xmodule.module_score is None
         assert self.xmodule.score_comment == ''
-        (_, evt_type, called_grade_obj), _ = self.system.publish.call_args  # pylint: disable=unpacking-non-sequence
+        (_, evt_type, called_grade_obj), _ = self.runtime.publish.call_args  # pylint: disable=unpacking-non-sequence
         assert called_grade_obj ==\
                {'user_id': self.USER_STANDIN.id, 'value': None, 'max_value': None, 'score_deleted': True}
         assert evt_type == 'grade'
@@ -271,7 +271,7 @@ def test_lti20_delete_success(self):
         assert response.status_code == 200
         assert self.xmodule.module_score is None
         assert self.xmodule.score_comment == ''
-        (_, evt_type, called_grade_obj), _ = self.system.publish.call_args  # pylint: disable=unpacking-non-sequence
+        (_, evt_type, called_grade_obj), _ = self.runtime.publish.call_args  # pylint: disable=unpacking-non-sequence
         assert called_grade_obj ==\
                {'user_id': self.USER_STANDIN.id, 'value': None, 'max_value': None, 'score_deleted': True}
         assert evt_type == 'grade'
@@ -288,7 +288,7 @@ def test_lti20_put_set_score_success(self):
         assert response.status_code == 200
         assert self.xmodule.module_score == 0.1
         assert self.xmodule.score_comment == 'ಠ益ಠ'
-        (_, evt_type, called_grade_obj), _ = self.system.publish.call_args  # pylint: disable=unpacking-non-sequence
+        (_, evt_type, called_grade_obj), _ = self.runtime.publish.call_args  # pylint: disable=unpacking-non-sequence
         assert evt_type == 'grade'
         assert called_grade_obj ==\
                {'user_id': self.USER_STANDIN.id, 'value': 0.1, 'max_value': 1.0, 'score_deleted': False}
@@ -370,7 +370,7 @@ def test_lti20_request_handler_bad_user(self):
         Test that we get a 404 when the supplied user does not exist
         """
         self.setup_system_xmodule_mocks_for_lti20_request_test()
-        self.system._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)
         response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd")
         assert response.status_code == 404

common/lib/xmodule/xmodule/tests/test_lti_unit.py

@@ -63,7 +63,7 @@ def setUp(self):
             """)
         self.system = get_test_system()
         self.system.publish = Mock()
-        self.system.rebind_noauth_module_to_user = Mock()
+        self.system._services['rebind_user'] = Mock()  # pylint: disable=protected-access
 
         self.xmodule = LTIBlock(
             self.system,

common/lib/xmodule/xmodule/x_module.py

@@ -1634,6 +1634,22 @@ def hostname(self):
         from django.conf import settings
         return settings.LMS_BASE
 
+    @property
+    def rebind_noauth_module_to_user(self):
+        """
+        A function that was used to bind modules initialized by AnonymousUsers to real users. Mainly used
+        by the LTI Module to connect the right users with the requests from LTI tools.
+
+        Deprecated in favour of the "rebind_user" service.
+        """
+        warnings.warn(
+            "rebind_noauth_module_to_user is deprecated. Please use the 'rebind_user' service instead.",
+            DeprecationWarning, stacklevel=3
+        )
+        rebind_user_service = self._services.get('rebind_user')
+        if rebind_user_service:
+            return partial(rebind_user_service.rebind_noauth_module_to_user)
+
 
 class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime):
     """
@@ -1658,7 +1674,6 @@ def __init__(
         course_id=None,
         error_descriptor_class=None,
         field_data=None,
-        rebind_noauth_module_to_user=None,
         **kwargs,
     ):
         """
@@ -1684,9 +1699,6 @@ def __init__(
         error_descriptor_class - The class to use to render XModules with errors
 
         field_data - the `FieldData` to use for backing XBlock storage.
-
-        rebind_noauth_module_to_user - rebinds module bound to AnonymousUser to a real user...used in LTI
-           modules, which have an anonymous handler, to set legitimate users' data
         """
 
         # Usage_store is unused, and field_data is often supplanted with an
@@ -1706,7 +1718,6 @@ def __init__(
         self.xmodule_instance = None
 
         self.descriptor_runtime = descriptor_runtime
-        self.rebind_noauth_module_to_user = rebind_noauth_module_to_user
 
     def get(self, attr):
         """	provide uniform access to attributes (like etree)."""

lms/djangoapps/courseware/module_render.py

@@ -45,6 +45,7 @@
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.util.sandboxing import SandboxService
+from xmodule.services import RebindUserService
 from common.djangoapps.static_replace.services import ReplaceURLService
 from common.djangoapps.static_replace.wrapper import replace_urls_wrapper
 from common.djangoapps.xblock_django.constants import ATTR_KEY_USER_ID
@@ -63,7 +64,6 @@
 from lms.djangoapps.grades.api import GradesUtilService
 from lms.djangoapps.grades.api import signals as grades_signals
 from lms.djangoapps.lms_xblock.field_data import LmsFieldData
-from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig
 from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem
 from lms.djangoapps.verify_student.services import XBlockVerificationService
 from openedx.core.djangoapps.bookmarks.services import BookmarksService
@@ -483,12 +483,12 @@ def inner_get_module(descriptor):
             student_data=student_data,
             course_id=course_id,
             track_function=track_function,
+            request_token=request_token,
             position=position,
             wrap_xmodule_display=wrap_xmodule_display,
             grade_bucket_type=grade_bucket_type,
             static_asset_path=static_asset_path,
             user_location=user_location,
-            request_token=request_token,
             course=course,
             will_recheck_access=will_recheck_access,
         )
@@ -608,65 +608,20 @@ def handle_deprecated_progress_event(block, event):
                     completion=1.0,
                 )
 
-    def rebind_noauth_module_to_user(module, real_user):
-        """
-        A function that allows a module to get re-bound to a real user if it was previously bound to an AnonymousUser.
-
-        Will only work within a module bound to an AnonymousUser, e.g. one that's instantiated by the noauth_handler.
-
-        Arguments:
-            module (any xblock type):  the module to rebind
-            real_user (django.contrib.auth.models.User):  the user to bind to
-
-        Returns:
-            nothing (but the side effect is that module is re-bound to real_user)
-        """
-        if user.is_authenticated:
-            err_msg = ("rebind_noauth_module_to_user can only be called from a module bound to "
-                       "an anonymous user")
-            log.error(err_msg)
-            raise LmsModuleRenderError(err_msg)
-
-        field_data_cache_real_user = FieldDataCache.cache_for_descriptor_descendents(
-            course_id,
-            real_user,
-            module,
-            asides=XBlockAsidesConfig.possible_asides(),
-        )
-        student_data_real_user = KvsFieldData(DjangoKeyValueStore(field_data_cache_real_user))
-
-        (inner_system, inner_student_data) = get_module_system_for_user(
-            user=real_user,
-            student_data=student_data_real_user,  # These have implicit user bindings, rest of args considered not to
-            descriptor=module,
-            course_id=course_id,
-            track_function=track_function,
-            position=position,
-            wrap_xmodule_display=wrap_xmodule_display,
-            grade_bucket_type=grade_bucket_type,
-            static_asset_path=static_asset_path,
-            user_location=user_location,
-            request_token=request_token,
-            course=course,
-            will_recheck_access=will_recheck_access,
-        )
-
-        module.bind_for_student(
-            inner_system,
-            real_user.id,
-            [
-                partial(DateLookupFieldData, course_id=course_id, user=user),
-                partial(OverrideFieldData.wrap, real_user, course),
-                partial(LmsFieldData, student_data=inner_student_data),
-            ],
-        )
-
-        module.scope_ids = (
-            module.scope_ids._replace(user_id=real_user.id)
-        )
-        # now bind the module to the new ModuleSystem instance and vice-versa
-        module.runtime = inner_system
-        inner_system.xmodule_instance = module
+    # Rebind module service to deal with noauth modules getting attached to users
+    rebind_user_service = RebindUserService(
+        user,
+        course_id,
+        get_module_system_for_user,
+        track_function=track_function,
+        position=position,
+        wrap_xmodule_display=wrap_xmodule_display,
+        grade_bucket_type=grade_bucket_type,
+        static_asset_path=static_asset_path,
+        user_location=user_location,
+        request_token=request_token,
+        will_recheck_access=will_recheck_access,
+    )
 
     # Build a list of wrapping functions that will be applied in order
     # to the Fragment content coming out of the xblocks that are about to be rendered.
@@ -751,10 +706,10 @@ def rebind_noauth_module_to_user(module, real_user):
             'cache': CacheService(cache),
             'sandbox': SandboxService(contentstore=contentstore, course_id=course_id),
             'xqueue': xqueue_service,
-            'replace_urls': replace_url_service
+            'replace_urls': replace_url_service,
+            'rebind_user': rebind_user_service,
         },
         descriptor_runtime=descriptor._runtime,  # pylint: disable=protected-access
-        rebind_noauth_module_to_user=rebind_noauth_module_to_user,
         request_token=request_token,
     )