From 00c339d091a10883ed126d2d027813d3b2086624 Mon Sep 17 00:00:00 2001 From: Daniel Clemente Laboreo Date: Wed, 7 Mar 2018 08:32:57 +0200 Subject: [PATCH] Implement iter_all_for_block and iter_all_for_course for DjangoUserStateClient --- .../tests/test_user_state_client.py | 38 +--------- .../courseware/user_state_client.py | 75 +++++++++++++++---- lms/envs/common.py | 5 ++ 3 files changed, 70 insertions(+), 48 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_user_state_client.py b/lms/djangoapps/courseware/tests/test_user_state_client.py index 4891840a13..79652f67e6 100644 --- a/lms/djangoapps/courseware/tests/test_user_state_client.py +++ b/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 diff --git a/lms/djangoapps/courseware/user_state_client.py b/lms/djangoapps/courseware/user_state_client.py index ecf6647613..0419aa2dc5 100644 --- a/lms/djangoapps/courseware/user_state_client.py +++ b/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) diff --git a/lms/envs/common.py b/lms/envs/common.py index 8a1b384f85..1e838cd6ba 100644 --- a/lms/envs/common.py +++ b/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