Merge pull request #17698 from open-craft/clemente/implement-djangouserstateclient-iter-all

Implement iter_all_for_block and iter_all_for_course for DjangoUserStateClient

lms/djangoapps/courseware/tests/test_user_state_client.py

@@ -11,11 +11,13 @@ from edx_user_state_client.tests import UserStateClientTestBase
 
 from courseware.tests.factories import UserFactory
 from courseware.user_state_client import DjangoXBlockUserStateClient
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 
 
-class TestDjangoUserStateClient(UserStateClientTestBase, TestCase):
+class TestDjangoUserStateClient(UserStateClientTestBase, ModuleStoreTestCase):
     """
     Tests of the DjangoUserStateClient backend.
+    It reuses all tests from :class:`~UserStateClientTestBase`.
     """
     __test__ = True
     # Tell Django to clean out all databases, not just default
@@ -33,37 +35,3 @@ class TestDjangoUserStateClient(UserStateClientTestBase, TestCase):
         super(TestDjangoUserStateClient, self).setUp()
         self.client = DjangoXBlockUserStateClient()
         self.users = defaultdict(UserFactory.create)
-
-    # We're skipping these tests because the iter_all_by_block and iter_all_by_course
-    # are not implemented in the DjangoXBlockUserStateClient
-    @skip("Not supported by DjangoXBlockUserStateClient")
-    def test_iter_blocks_deleted_block(self):
-        pass
-
-    @skip("Not supported by DjangoXBlockUserStateClient")
-    def test_iter_blocks_empty(self):
-        pass
-
-    @skip("Not supported by DjangoXBlockUserStateClient")
-    def test_iter_blocks_many_users(self):
-        pass
-
-    @skip("Not supported by DjangoXBlockUserStateClient")
-    def test_iter_blocks_single_user(self):
-        pass
-
-    @skip("Not supported by DjangoXBlockUserStateClient")
-    def test_iter_course_deleted_block(self):
-        pass
-
-    @skip("Not supported by DjangoXBlockUserStateClient")
-    def test_iter_course_empty(self):
-        pass
-
-    @skip("Not supported by DjangoXBlockUserStateClient")
-    def test_iter_course_single_user(self):
-        pass
-
-    @skip("Not supported by DjangoXBlockUserStateClient")
-    def test_iter_course_many_users(self):
-        pass

lms/djangoapps/courseware/user_state_client.py

@@ -8,6 +8,8 @@ import logging
 from operator import attrgetter
 from time import time
 
+from django.conf import settings
+from django.core.paginator import Paginator
 from django.contrib.auth.models import User
 from django.db import transaction
 from django.db.utils import IntegrityError
@@ -439,22 +441,69 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
 
             yield XBlockUserState(username, block_key, state, history_entry.created, scope)
 
-    def iter_all_for_block(self, block_key, scope=Scope.user_state, batch_size=None):
+    def iter_all_for_block(self, block_key, scope=Scope.user_state):
         """
-        You get no ordering guarantees. Fetching will happen in batch_size
-        increments. If you're using this method, you should be running in an
-        async task.
-        """
-        if scope != Scope.user_state:
-            raise ValueError("Only Scope.user_state is supported")
-        raise NotImplementedError()
+        Return an iterator over the data stored in the block (e.g. a problem block).
 
-    def iter_all_for_course(self, course_key, block_type=None, scope=Scope.user_state, batch_size=None):
-        """
-        You get no ordering guarantees. Fetching will happen in batch_size
-        increments. If you're using this method, you should be running in an
+        You get no ordering guarantees.If you're using this method, you should be running in an
         async task.
+
+        Arguments:
+            block_key: an XBlock's locator (e.g. :class:`~BlockUsageLocator`)
+            scope (Scope): must be `Scope.user_state`
+
+        Returns:
+            an iterator over all data. Each invocation returns the next :class:`~XBlockUserState`
+                object, which includes the block's contents.
         """
         if scope != Scope.user_state:
             raise ValueError("Only Scope.user_state is supported")
-        raise NotImplementedError()
+
+        results = StudentModule.objects.filter(module_state_key=block_key)
+        p = Paginator(results, settings.USER_STATE_BATCH_SIZE)
+
+        for page_number in p.page_range:
+            page = p.page(page_number)
+
+            for sm in page.object_list:
+                state = json.loads(sm.state)
+
+                if state == {}:
+                    continue
+
+                yield XBlockUserState(sm.student.username, sm.module_state_key, state, sm.modified, scope)
+
+    def iter_all_for_course(self, course_key, block_type=None, scope=Scope.user_state):
+        """
+        Return an iterator over all data stored in a course's blocks.
+
+        You get no ordering guarantees. If you're using this method, you should be running in an
+        async task.
+
+        Arguments:
+            course_key: a course locator
+            scope (Scope): must be `Scope.user_state`
+
+        Returns:
+            an iterator over all data. Each invocation returns the next :class:`~XBlockUserState`
+                object, which includes the block's contents.
+        """
+        if scope != Scope.user_state:
+            raise ValueError("Only Scope.user_state is supported")
+
+        results = StudentModule.objects.filter(course_id=course_key)
+        if block_type:
+            results = results.filter(module_type=block_type)
+
+        p = Paginator(results, settings.USER_STATE_BATCH_SIZE)
+
+        for page_number in p.page_range:
+            page = p.page(page_number)
+
+            for sm in page.object_list:
+                state = json.loads(sm.state)
+
+                if state == {}:
+                    continue
+
+                yield XBlockUserState(sm.student.username, sm.module_state_key, state, sm.modified, scope)

lms/envs/common.py

@@ -3419,6 +3419,11 @@ FERNET_KEYS = [
     'DUMMY KEY CHANGE BEFORE GOING TO PRODUCTION',
 ]
 
+############### Settings for user-state-client ##################
+# Maximum number of rows to fetch in XBlockUserStateClient calls. Adjust for performance
+USER_STATE_BATCH_SIZE = 5000
+
+
 ############## Plugin Django Apps #########################
 
 from openedx.core.djangoapps.plugins import plugin_apps, plugin_settings, constants as plugin_constants