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

Implement a BaseStudentModuleHistory

Browse Source 
This abstract class contains most of the fields (aside from the id and
foreign key to StudentModule that the subclasses need to manage).  It
also provides a get_history method that abstracts searching across
multiple backends.

Move router code to openedx/core
We need to use it from cms and lms.
Ensure aws_migrate can be used for migrating both the lms and cms.

Handle queries directed to student_module_history vs default and the
extra queries generated by Django 1.8 (SAVEPOINTS, etc).

Additionally, flag testing classes as multi_db so that Django will
flush the non-default database between unit tests.

Further decouple the foreignkey relation between csm and csmhe

When calling StudentModule().delete() Django will try to delete CSMHE
objects, but naively does so in the database, not by consulting the
database router.

Instead, we disable django cascading deletes and listen for post_delete
signals and clean up CSMHE by hand.

Add feature flags for CSMHE
One to turn it on/off so we can control the deploy.
The other will control whether or not we read from two database tables
or one when searching.

Update tests to explicitly use this get_history method rather than
looking directly into StudentModuleHistory or
StudentModuleHistoryExtended.

Inform lettuce to avoid the coursewarehistoryextended app

Otherwise it fails when it can't find features/ in that app.

Add Pg support, this is not tested automatically.
This commit is contained in:
Kevin Falcone
2016-01-28 10:32:28 -05:00
parent dd90c55602
commit 6f9a3911e5
25 changed files with 366 additions and 149 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cms/envs/aws_migrate.py
View File

@@ -26,5 +26,5 @@ if DB_OVERRIDES['PASSWORD'] is None:
raise ImproperlyConfigured("No database password was provided for running "
"migrations. This is fatal.")
for override, value in DB_OVERRIDES.iteritems():
DATABASES['default'][override] = value
DATABASES['default'].update(DB_OVERRIDES)
DATABASES['student_module_history'].update(DB_OVERRIDES)

5
cms/envs/common.py
View File

@@ -1114,6 +1114,11 @@ PROCTORING_BACKEND_PROVIDER = {
}
PROCTORING_SETTINGS = {}
############################ Global Database Configuration #####################
DATABASE_ROUTERS = [
'openedx.core.lib.django_courseware_routers.StudentModuleHistoryExtendedRouter',
]
############################ OAUTH2 Provider ###################################

4
common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
View File

@@ -265,6 +265,8 @@ class SharedModuleStoreTestCase(TestCase):
for Django ORM models that will get cleaned up properly.
"""
MODULESTORE = mixed_store_config(mkdtemp_clean(), {}, include_xml=False)
# Tell Django to clean out all databases, not just default
multi_db = True
@classmethod
def setUpClass(cls):
@@ -392,6 +394,8 @@ class ModuleStoreTestCase(TestCase):
"""
MODULESTORE = mixed_store_config(mkdtemp_clean(), {}, include_xml=False)
# Tell Django to clean out all databases, not just default
multi_db = True
def setUp(self, **kwargs):
"""

74
lms/djangoapps/ccx/tests/test_field_override_performance.py
View File

@@ -46,6 +46,8 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
providers.
"""
__test__ = False
# Tell Django to clean out all databases, not just default
multi_db = True
# TEST_DATA must be overridden by subclasses
TEST_DATA = None
@@ -227,24 +229,24 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
# # of mongo queries,
# # of xblocks
# )
('no_overrides', 1, True, False): (23, 1, 6, 13),
('no_overrides', 2, True, False): (53, 16, 6, 84),
('no_overrides', 3, True, False): (183, 81, 6, 335),
('ccx', 1, True, False): (23, 1, 6, 13),
('ccx', 2, True, False): (53, 16, 6, 84),
('ccx', 3, True, False): (183, 81, 6, 335),
('ccx', 1, True, True): (23, 1, 6, 13),
('ccx', 2, True, True): (53, 16, 6, 84),
('ccx', 3, True, True): (183, 81, 6, 335),
('no_overrides', 1, False, False): (23, 1, 6, 13),
('no_overrides', 2, False, False): (53, 16, 6, 84),
('no_overrides', 3, False, False): (183, 81, 6, 335),
('ccx', 1, False, False): (23, 1, 6, 13),
('ccx', 2, False, False): (53, 16, 6, 84),
('ccx', 3, False, False): (183, 81, 6, 335),
('ccx', 1, False, True): (23, 1, 6, 13),
('ccx', 2, False, True): (53, 16, 6, 84),
('ccx', 3, False, True): (183, 81, 6, 335),
('no_overrides', 1, True, False): (47, 1, 6, 13),
('no_overrides', 2, True, False): (119, 16, 6, 84),
('no_overrides', 3, True, False): (399, 81, 6, 335),
('ccx', 1, True, False): (47, 1, 6, 13),
('ccx', 2, True, False): (119, 16, 6, 84),
('ccx', 3, True, False): (399, 81, 6, 335),
('ccx', 1, True, True): (47, 1, 6, 13),
('ccx', 2, True, True): (119, 16, 6, 84),
('ccx', 3, True, True): (399, 81, 6, 335),
('no_overrides', 1, False, False): (47, 1, 6, 13),
('no_overrides', 2, False, False): (119, 16, 6, 84),
('no_overrides', 3, False, False): (399, 81, 6, 335),
('ccx', 1, False, False): (47, 1, 6, 13),
('ccx', 2, False, False): (119, 16, 6, 84),
('ccx', 3, False, False): (399, 81, 6, 335),
('ccx', 1, False, True): (47, 1, 6, 13),
('ccx', 2, False, True): (119, 16, 6, 84),
('ccx', 3, False, True): (399, 81, 6, 335),
}
@@ -256,22 +258,22 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
__test__ = True
TEST_DATA = {
('no_overrides', 1, True, False): (23, 1, 4, 9),
('no_overrides', 2, True, False): (53, 16, 19, 54),
('no_overrides', 3, True, False): (183, 81, 84, 215),
('ccx', 1, True, False): (23, 1, 4, 9),
('ccx', 2, True, False): (53, 16, 19, 54),
('ccx', 3, True, False): (183, 81, 84, 215),
('ccx', 1, True, True): (25, 1, 4, 13),
('ccx', 2, True, True): (55, 16, 19, 84),
('ccx', 3, True, True): (185, 81, 84, 335),
('no_overrides', 1, False, False): (23, 1, 4, 9),
('no_overrides', 2, False, False): (53, 16, 19, 54),
('no_overrides', 3, False, False): (183, 81, 84, 215),
('ccx', 1, False, False): (23, 1, 4, 9),
('ccx', 2, False, False): (53, 16, 19, 54),
('ccx', 3, False, False): (183, 81, 84, 215),
('ccx', 1, False, True): (23, 1, 4, 9),
('ccx', 2, False, True): (53, 16, 19, 54),
('ccx', 3, False, True): (183, 81, 84, 215),
('no_overrides', 1, True, False): (47, 1, 4, 9),
('no_overrides', 2, True, False): (119, 16, 19, 54),
('no_overrides', 3, True, False): (399, 81, 84, 215),
('ccx', 1, True, False): (47, 1, 4, 9),
('ccx', 2, True, False): (119, 16, 19, 54),
('ccx', 3, True, False): (399, 81, 84, 215),
('ccx', 1, True, True): (49, 1, 4, 13),
('ccx', 2, True, True): (121, 16, 19, 84),
('ccx', 3, True, True): (401, 81, 84, 335),
('no_overrides', 1, False, False): (47, 1, 4, 9),
('no_overrides', 2, False, False): (119, 16, 19, 54),
('no_overrides', 3, False, False): (399, 81, 84, 215),
('ccx', 1, False, False): (47, 1, 4, 9),
('ccx', 2, False, False): (119, 16, 19, 54),
('ccx', 3, False, False): (399, 81, 84, 215),
('ccx', 1, False, True): (47, 1, 4, 9),
('ccx', 2, False, True): (119, 16, 19, 54),
('ccx', 3, False, True): (399, 81, 84, 215),
}

116
lms/djangoapps/courseware/models.py
View File

@@ -24,9 +24,9 @@ from django.dispatch import receiver, Signal
from model_utils.models import TimeStampedModel
from student.models import user_by_anonymous_id
from submissions.models import score_set, score_reset
import coursewarehistoryextended
from xmodule_django.models import CourseKeyField, LocationKeyField, BlockTypeKeyField
from courseware.fields import UnsignedBigIntAutoField
log = logging.getLogger("edx.courseware")
@@ -149,18 +149,15 @@ class StudentModule(models.Model):
return unicode(repr(self))
class StudentModuleHistory(models.Model):
"""Keeps a complete history of state changes for a given XModule for a given
Student. Right now, we restrict this to problems so that the table doesn't
explode in size."""
class BaseStudentModuleHistory(models.Model):
"""Abstract class containing most fields used by any class
storing Student Module History"""
objects = ChunkingManager()
HISTORY_SAVING_TYPES = {'problem'}
class Meta(object):
app_label = "courseware"
get_latest_by = "created"
abstract = True
student_module = models.ForeignKey(StudentModule, db_index=True)
version = models.CharField(max_length=255, null=True, blank=True, db_index=True)
# This should be populated from the modified field in StudentModule
@@ -169,11 +166,59 @@ class StudentModuleHistory(models.Model):
grade = models.FloatField(null=True, blank=True)
max_grade = models.FloatField(null=True, blank=True)
@receiver(post_save, sender=StudentModule)
@property
def csm(self):
"""
Finds the StudentModule object for this history record, even if our data is split
across multiple data stores. Django does not handle this correctly with the built-in
student_module property.
"""
return StudentModule.objects.get(pk=self.student_module_id)
@staticmethod
def get_history(student_modules):
"""
Find history objects across multiple backend stores for a given StudentModule
"""
history_entries = []
if settings.FEATURES.get('ENABLE_CSMH_EXTENDED'):
history_entries += coursewarehistoryextended.models.StudentModuleHistoryExtended.objects.filter(
# Django will sometimes try to join to courseware_studentmodule
# so just do an in query
student_module__in=[module.id for module in student_modules]
).order_by('-id')
# If we turn off reading from multiple history tables, then we don't want to read from
# StudentModuleHistory anymore, we believe that all history is in the Extended table.
if settings.FEATURES.get('ENABLE_READING_FROM_MULTIPLE_HISTORY_TABLES'):
# we want to save later SQL queries on the model which allows us to prefetch
history_entries += StudentModuleHistory.objects.prefetch_related('student_module').filter(
student_module__in=student_modules
).order_by('-id')
return history_entries
class StudentModuleHistory(BaseStudentModuleHistory):
"""Keeps a complete history of state changes for a given XModule for a given
Student. Right now, we restrict this to problems so that the table doesn't
explode in size."""
class Meta(object):
app_label = "courseware"
get_latest_by = "created"
student_module = models.ForeignKey(StudentModule, db_index=True)
def __unicode__(self):
return unicode(repr(self))
def save_history(sender, instance, **kwargs): # pylint: disable=no-self-argument, unused-argument
"""
Checks the instance's module_type, and creates & saves a
StudentModuleHistory entry if the module_type is one that
StudentModuleHistoryExtended entry if the module_type is one that
we save.
"""
if instance.module_type in StudentModuleHistory.HISTORY_SAVING_TYPES:
@@ -185,52 +230,11 @@ class StudentModuleHistory(models.Model):
max_grade=instance.max_grade)
history_entry.save()
def __unicode__(self):
return unicode(repr(self))
class StudentModuleHistoryExtended(models.Model):
"""Keeps a complete history of state changes for a given XModule for a given
Student. Right now, we restrict this to problems so that the table doesn't
explode in size.
This new extended CSMH has a larger primary key that won't run out of space
so quickly."""
objects = ChunkingManager()
HISTORY_SAVING_TYPES = {'problem'}
class Meta(object):
app_label = "courseware"
get_latest_by = "created"
id = UnsignedBigIntAutoField(primary_key=True) # pylint: disable=invalid-name
student_module = models.ForeignKey(StudentModule, db_index=True, db_constraint=False)
version = models.CharField(max_length=255, null=True, blank=True, db_index=True)
# This should be populated from the modified field in StudentModule
created = models.DateTimeField(db_index=True)
state = models.TextField(null=True, blank=True)
grade = models.FloatField(null=True, blank=True)
max_grade = models.FloatField(null=True, blank=True)
@receiver(post_save, sender=StudentModule)
def save_history(sender, instance, **kwargs): # pylint: disable=no-self-argument, unused-argument
"""
Checks the instance's module_type, and creates & saves a
StudentModuleHistory entry if the module_type is one that
we save.
"""
if instance.module_type in StudentModuleHistoryExtended.HISTORY_SAVING_TYPES:
history_entry = StudentModuleHistoryExtended(student_module=instance,
version=None,
created=instance.modified,
state=instance.state,
grade=instance.grade,
max_grade=instance.max_grade)
history_entry.save()
def __unicode__(self):
return unicode(repr(self))
# When the extended studentmodulehistory table exists, don't save
# duplicate history into courseware_studentmodulehistory, just retain
# data for reading.
if not settings.FEATURES.get('ENABLE_CSMH_EXTENDED'):
post_save.connect(save_history, sender=StudentModule)
class XBlockFieldBase(models.Model):

1
lms/djangoapps/courseware/tests/test_access.py
View File

@@ -520,6 +520,7 @@ class UserRoleTestCase(TestCase):
"""
Tests for user roles.
"""
def setUp(self):
super(UserRoleTestCase, self).setUp()
self.course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')

3
lms/djangoapps/courseware/tests/test_grades.py
View File

@@ -240,6 +240,9 @@ class TestProgressSummary(TestCase):
(2/5) (3/5) (0/1) - (1/3) - (3/10)
"""
# Tell Django to clean out all databases, not just default
multi_db = True
def setUp(self):
super(TestProgressSummary, self).setUp()
self.course_key = CourseLocator(

1
lms/djangoapps/courseware/tests/test_i18n.py
View File

@@ -20,6 +20,7 @@ class BaseI18nTestCase(TestCase):
"""
Base utilities for i18n test classes to derive from
"""
def assert_tag_has_attr(self, content, tag, attname, value):
"""Assert that a tag in `content` has a certain value in a certain attribute."""
regex = r"""<{tag} [^>]*\b{attname}=['"]([\w\d\- ]+)['"][^>]*>""".format(tag=tag, attname=attname)

30
lms/djangoapps/courseware/tests/test_model_data.py
View File

@@ -103,6 +103,8 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase):
"""Tests for user_state storage via StudentModule"""
other_key_factory = partial(DjangoKeyValueStore.Key, Scope.user_state, 2, location('usage_id')) # user_id=2, not 1
existing_field_name = "a_field"
# Tell Django to clean out all databases, not just default
multi_db = True
def setUp(self):
super(TestStudentModuleStorage, self).setUp()
@@ -137,8 +139,9 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase):
# to discover if something other than the DjangoXBlockUserStateClient
# has written to the StudentModule (such as UserStateCache setting the score
# on the StudentModule).
with self.assertNumQueries(3):
self.kvs.set(user_state_key('a_field'), 'new_value')
with self.assertNumQueries(2, using='default'):
with self.assertNumQueries(1, using='student_module_history'):
self.kvs.set(user_state_key('a_field'), 'new_value')
self.assertEquals(1, StudentModule.objects.all().count())
self.assertEquals({'b_field': 'b_value', 'a_field': 'new_value'}, json.loads(StudentModule.objects.all()[0].state))
@@ -149,8 +152,9 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase):
# to discover if something other than the DjangoXBlockUserStateClient
# has written to the StudentModule (such as UserStateCache setting the score
# on the StudentModule).
with self.assertNumQueries(3):
self.kvs.set(user_state_key('not_a_field'), 'new_value')
with self.assertNumQueries(2, using='default'):
with self.assertNumQueries(1, using='student_module_history'):
self.kvs.set(user_state_key('not_a_field'), 'new_value')
self.assertEquals(1, StudentModule.objects.all().count())
self.assertEquals({'b_field': 'b_value', 'a_field': 'a_value', 'not_a_field': 'new_value'}, json.loads(StudentModule.objects.all()[0].state))
@@ -161,8 +165,9 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase):
# to discover if something other than the DjangoXBlockUserStateClient
# has written to the StudentModule (such as UserStateCache setting the score
# on the StudentModule).
with self.assertNumQueries(3):
self.kvs.delete(user_state_key('a_field'))
with self.assertNumQueries(2, using='default'):
with self.assertNumQueries(1, using='student_module_history'):
self.kvs.delete(user_state_key('a_field'))
self.assertEquals(1, StudentModule.objects.all().count())
self.assertRaises(KeyError, self.kvs.get, user_state_key('not_a_field'))
@@ -201,8 +206,9 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase):
# We also need to read the database to discover if something other than the
# DjangoXBlockUserStateClient has written to the StudentModule (such as
# UserStateCache setting the score on the StudentModule).
with self.assertNumQueries(3):
self.kvs.set_many(kv_dict)
with self.assertNumQueries(2, using="default"):
with self.assertNumQueries(1, using="student_module_history"):
self.kvs.set_many(kv_dict)
for key in kv_dict:
self.assertEquals(self.kvs.get(key), kv_dict[key])
@@ -223,6 +229,9 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase):
@attr('shard_1')
class TestMissingStudentModule(TestCase):
# Tell Django to clean out all databases, not just default
multi_db = True
def setUp(self):
super(TestMissingStudentModule, self).setUp()
@@ -244,12 +253,13 @@ class TestMissingStudentModule(TestCase):
self.assertEquals(0, len(self.field_data_cache))
self.assertEquals(0, StudentModule.objects.all().count())
# We are updating a problem, so we write to courseware_studentmodulehistory
# We are updating a problem, so we write to courseware_studentmodulehistoryextended
# as well as courseware_studentmodule. We also need to read the database
# to discover if something other than the DjangoXBlockUserStateClient
# has written to the StudentModule (such as UserStateCache setting the score
# on the StudentModule).
with self.assertNumQueries(2, using='default'):
# Django 1.8 also has a number of other BEGIN and SAVESTATE queries.
with self.assertNumQueries(4, using='default'):
with self.assertNumQueries(1, using='student_module_history'):
self.kvs.set(user_state_key('a_field'), 'a_value')

27
lms/djangoapps/courseware/tests/test_submitting_problems.py
View File

@@ -19,7 +19,7 @@ from capa.tests.response_xml_factory import (
CodeResponseXMLFactory,
)
from courseware import grades
from courseware.models import StudentModule, StudentModuleHistory
from courseware.models import StudentModule, BaseStudentModuleHistory
from courseware.tests.helpers import LoginEnrollmentTestCase
from lms.djangoapps.lms_xblock.runtime import quote_slashes
from student.tests.factories import UserFactory
@@ -121,6 +121,8 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl
Check that a course gets graded properly.
"""
# Tell Django to clean out all databases, not just default
multi_db = True
# arbitrary constant
COURSE_SLUG = "100"
COURSE_NAME = "test_course"
@@ -319,6 +321,9 @@ class TestCourseGrader(TestSubmittingProblems):
"""
Suite of tests for the course grader.
"""
# Tell Django to clean out all databases, not just default
multi_db = True
def basic_setup(self, late=False, reset=False, showanswer=False):
"""
Set up a simple course for testing basic grading functionality.
@@ -451,26 +456,20 @@ class TestCourseGrader(TestSubmittingProblems):
self.submit_question_answer('p1', {'2_1': u'Correct'})
# Now fetch the state entry for that problem.
student_module = StudentModule.objects.get(
student_module = StudentModule.objects.filter(
course_id=self.course.id,
student=self.student_user
)
# count how many state history entries there are
baseline = StudentModuleHistory.objects.filter(
student_module=student_module
)
baseline_count = baseline.count()
self.assertEqual(baseline_count, 3)
baseline = BaseStudentModuleHistory.get_history(student_module)
self.assertEqual(len(baseline), 3)
# now click "show answer"
self.show_question_answer('p1')
# check that we don't have more state history entries
csmh = StudentModuleHistory.objects.filter(
student_module=student_module
)
current_count = csmh.count()
self.assertEqual(current_count, 3)
csmh = BaseStudentModuleHistory.get_history(student_module)
self.assertEqual(len(csmh), 3)
def test_grade_with_max_score_cache(self):
"""
@@ -713,6 +712,8 @@ class TestCourseGrader(TestSubmittingProblems):
@attr('shard_1')
class ProblemWithUploadedFilesTest(TestSubmittingProblems):
"""Tests of problems with uploaded files."""
# Tell Django to clean out all databases, not just default
multi_db = True
def setUp(self):
super(ProblemWithUploadedFilesTest, self).setUp()
@@ -768,6 +769,8 @@ class TestPythonGradedResponse(TestSubmittingProblems):
"""
Check that we can submit a schematic and custom response, and it answers properly.
"""
# Tell Django to clean out all databases, not just default
multi_db = True
SCHEMATIC_SCRIPT = dedent("""
# for a schematic response, submission[i] is the json representation

2
lms/djangoapps/courseware/tests/test_user_state_client.py
View File

@@ -18,6 +18,8 @@ class TestDjangoUserStateClient(UserStateClientTestBase, TestCase):
Tests of the DjangoUserStateClient backend.
"""
__test__ = True
# Tell Django to clean out all databases, not just default
multi_db = True
def _user(self, user_idx):
return self.users[user_idx].username

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

@@ -15,7 +15,7 @@ except ImportError:
import dogstats_wrapper as dog_stats_api
from django.contrib.auth.models import User
from xblock.fields import Scope, ScopeBase
from courseware.models import StudentModule, StudentModuleHistory
from courseware.models import StudentModule, BaseStudentModuleHistory
from edx_user_state_client.interface import XBlockUserStateClient, XBlockUserState
@@ -312,9 +312,7 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
if len(student_modules) == 0:
raise self.DoesNotExist()
history_entries = StudentModuleHistory.objects.prefetch_related('student_module').filter(
student_module__in=student_modules
).order_by('-id')
history_entries = BaseStudentModuleHistory.get_history(student_modules)
# If no history records exist, raise an error
if not history_entries:
@@ -332,9 +330,9 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
if state == {}:
state = None
block_key = history_entry.student_module.module_state_key
block_key = history_entry.csm.module_state_key
block_key = block_key.map_into_course(
history_entry.student_module.course_id
history_entry.csm.course_id
)
yield XBlockUserState(username, block_key, state, history_entry.created, scope)

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

@@ -59,7 +59,7 @@ from courseware.courses import (
)
from courseware.masquerade import setup_masquerade
from courseware.model_data import FieldDataCache, ScoresClient
from courseware.models import StudentModuleHistory
from courseware.models import StudentModule, BaseStudentModuleHistory
from courseware.url_helpers import get_redirect_url
from courseware.user_state_client import DjangoXBlockUserStateClient
from edxmako.shortcuts import render_to_response, render_to_string, marketing_link
@@ -1173,11 +1173,12 @@ def submission_history(request, course_id, student_username, location):
# This is ugly, but until we have a proper submissions API that we can use to provide
# the scores instead, it will have to do.
scores = list(StudentModuleHistory.objects.filter(
student_module__module_state_key=usage_key,
student_module__student__username=student_username,
student_module__course_id=course_key
).order_by('-id'))
csm = StudentModule.objects.filter(
module_state_key=usage_key,
student__username=student_username,
course_id=course_key)
scores = BaseStudentModuleHistory.get_history(csm)
if len(scores) != len(history_entries):
log.warning(

0
lms/djangoapps/coursewarehistoryextended/__init__.py Normal file
View File

6
lms/djangoapps/courseware/fields.py → lms/djangoapps/coursewarehistoryextended/fields.py
View File

@@ -1,5 +1,5 @@
"""
Custom fields for use in the courseware django app.
Custom fields for use in the coursewarehistoryextended django app.
"""
from django.db.models.fields import AutoField
@@ -17,5 +17,9 @@ class UnsignedBigIntAutoField(AutoField):
# is an alias for that (https://www.sqlite.org/autoinc.html). An unsigned integer
# isn't an alias for ROWID, so we have to give up on the unsigned part.
return "integer"
elif connection.settings_dict['ENGINE'] == 'django.db.backends.postgresql_psycopg2':
# Pg's bigserial is implicitly unsigned (doesn't allow negative numbers) and
# goes 1-9.2x10^18
return "BIGSERIAL"
else:
return None

18
lms/djangoapps/courseware/migrations/0002_csmh-extended-keyspace.py → lms/djangoapps/coursewarehistoryextended/migrations/0001_initial.py
View File

@@ -2,18 +2,21 @@
from __future__ import unicode_literals
from django.db import migrations, models
import courseware.fields
from django.conf import settings
import django.db.models.deletion
import coursewarehistoryextended.fields
def bump_pk_start(apps, schema_editor):
if not schema_editor.connection.alias == 'student_module_history':
return
StudentModuleHistory = apps.get_model("courseware", "StudentModuleHistory")
initial_id = settings.STUDENTMODULEHISTORYEXTENDED_OFFSET + StudentModuleHistory.objects.all().latest('id').id
biggest_id = StudentModuleHistory.objects.all().order_by('-id').first()
initial_id = settings.STUDENTMODULEHISTORYEXTENDED_OFFSET
if biggest_id is not None:
initial_id += biggest_id.id
if schema_editor.connection.vendor == 'mysql':
schema_editor.execute('ALTER TABLE courseware_studentmodulehistoryextended AUTO_INCREMENT=%s', [initial_id])
schema_editor.execute('ALTER TABLE coursewarehistoryextended_studentmodulehistoryextended AUTO_INCREMENT=%s', [initial_id])
elif schema_editor.connection.vendor == 'sqlite3':
# This is a hack to force sqlite to add new rows after the earlier rows we
# want to migrate.
@@ -25,7 +28,8 @@ def bump_pk_start(apps, schema_editor):
version="",
created=datetime.datetime.now(),
).save()
elif schema_editor.connection.vendor == 'postgresql':
schema_editor.execute("SELECT setval('coursewarehistoryextended_studentmodulehistoryextended_seq', %s)", [initial_id])
class Migration(migrations.Migration):
@@ -37,13 +41,13 @@ class Migration(migrations.Migration):
migrations.CreateModel(
name='StudentModuleHistoryExtended',
fields=[
('id', courseware.fields.UnsignedBigIntAutoField(serialize=False, primary_key=True)),
('version', models.CharField(db_index=True, max_length=255, null=True, blank=True)),
('created', models.DateTimeField(db_index=True)),
('state', models.TextField(null=True, blank=True)),
('grade', models.FloatField(null=True, blank=True)),
('max_grade', models.FloatField(null=True, blank=True)),
('student_module', models.ForeignKey(to='courseware.StudentModule', db_constraint=False)),
('id', coursewarehistoryextended.fields.UnsignedBigIntAutoField(serialize=False, primary_key=True)),
('student_module', models.ForeignKey(to='courseware.StudentModule', on_delete=django.db.models.deletion.DO_NOTHING, db_constraint=False)),
],
options={
'get_latest_by': 'created',

0
lms/djangoapps/coursewarehistoryextended/migrations/__init__.py Normal file
View File

64
lms/djangoapps/coursewarehistoryextended/models.py Normal file
View File

@@ -0,0 +1,64 @@
"""
WE'RE USING MIGRATIONS!
If you make changes to this model, be sure to create an appropriate migration
file and check it in at the same time as your model changes. To do that,
1. Go to the edx-platform dir
2. ./manage.py schemamigration courseware --auto description_of_your_change
3. Add the migration file created in edx-platform/lms/djangoapps/coursewarehistoryextended/migrations/
ASSUMPTIONS: modules have unique IDs, even across different module_types
"""
from django.db import models
from django.db.models.signals import post_save, post_delete
from django.dispatch import receiver
from coursewarehistoryextended.fields import UnsignedBigIntAutoField
from courseware.models import StudentModule, BaseStudentModuleHistory
class StudentModuleHistoryExtended(BaseStudentModuleHistory):
"""Keeps a complete history of state changes for a given XModule for a given
Student. Right now, we restrict this to problems so that the table doesn't
explode in size.
This new extended CSMH has a larger primary key that won't run out of space
so quickly."""
class Meta(object):
app_label = 'coursewarehistoryextended'
get_latest_by = "created"
id = UnsignedBigIntAutoField(primary_key=True) # pylint: disable=invalid-name
student_module = models.ForeignKey(StudentModule, db_index=True, db_constraint=False, on_delete=models.DO_NOTHING)
@receiver(post_save, sender=StudentModule)
def save_history(sender, instance, **kwargs): # pylint: disable=no-self-argument, unused-argument
"""
Checks the instance's module_type, and creates & saves a
StudentModuleHistoryExtended entry if the module_type is one that
we save.
"""
if instance.module_type in StudentModuleHistoryExtended.HISTORY_SAVING_TYPES:
history_entry = StudentModuleHistoryExtended(student_module=instance,
version=None,
created=instance.modified,
state=instance.state,
grade=instance.grade,
max_grade=instance.max_grade)
history_entry.save()
@receiver(post_delete, sender=StudentModule)
def delete_history(sender, instance, **kwargs): # pylint: disable=no-self-argument, unused-argument
"""
Django can't cascade delete across databases, so we tell it at the model level to
on_delete=DO_NOTHING and then listen for post_delete so we can clean up the CSMHE rows.
"""
StudentModuleHistoryExtended.objects.filter(student_module=instance).all().delete()
def __unicode__(self):
return unicode(repr(self))

81
lms/djangoapps/coursewarehistoryextended/tests.py Normal file
View File

@@ -0,0 +1,81 @@
"""
Tests for coursewarehistoryextended
Many aspects of this app are covered by the courseware tests,
but these are specific to the new storage model with multiple
backend tables.
"""
import json
from mock import patch
from django.test import TestCase
from django.conf import settings
from unittest import skipUnless
from nose.plugins.attrib import attr
from courseware.models import BaseStudentModuleHistory, StudentModuleHistory, StudentModule
from courseware.tests.factories import StudentModuleFactory, location, course_id
@attr('shard_1')
@skipUnless(settings.FEATURES["ENABLE_CSMH_EXTENDED"], "CSMH Extended needs to be enabled")
class TestStudentModuleHistoryBackends(TestCase):
""" Tests of data in CSMH and CSMHE """
# Tell Django to clean out all databases, not just default
multi_db = True
def setUp(self):
super(TestStudentModuleHistoryBackends, self).setUp()
for record in (1, 2, 3):
# This will store into CSMHE via the post_save signal
csm = StudentModuleFactory.create(module_state_key=location('usage_id'),
course_id=course_id,
state=json.dumps({'type': 'csmhe', 'order': record}))
# This manually gets us a CSMH record to compare
csmh = StudentModuleHistory(student_module=csm,
version=None,
created=csm.modified,
state=json.dumps({'type': 'csmh', 'order': record}),
grade=csm.grade,
max_grade=csm.max_grade)
csmh.save()
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_CSMH_EXTENDED": True})
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_READING_FROM_MULTIPLE_HISTORY_TABLES": True})
def test_get_history_true_true(self):
student_module = StudentModule.objects.all()
history = BaseStudentModuleHistory.get_history(student_module)
self.assertEquals(len(history), 6)
self.assertEquals({'type': 'csmhe', 'order': 3}, json.loads(history[0].state))
self.assertEquals({'type': 'csmhe', 'order': 2}, json.loads(history[1].state))
self.assertEquals({'type': 'csmhe', 'order': 1}, json.loads(history[2].state))
self.assertEquals({'type': 'csmh', 'order': 3}, json.loads(history[3].state))
self.assertEquals({'type': 'csmh', 'order': 2}, json.loads(history[4].state))
self.assertEquals({'type': 'csmh', 'order': 1}, json.loads(history[5].state))
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_CSMH_EXTENDED": True})
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_READING_FROM_MULTIPLE_HISTORY_TABLES": False})
def test_get_history_true_false(self):
student_module = StudentModule.objects.all()
history = BaseStudentModuleHistory.get_history(student_module)
self.assertEquals(len(history), 3)
self.assertEquals({'type': 'csmhe', 'order': 3}, json.loads(history[0].state))
self.assertEquals({'type': 'csmhe', 'order': 2}, json.loads(history[1].state))
self.assertEquals({'type': 'csmhe', 'order': 1}, json.loads(history[2].state))
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_CSMH_EXTENDED": False})
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_READING_FROM_MULTIPLE_HISTORY_TABLES": True})
def test_get_history_false_true(self):
student_module = StudentModule.objects.all()
history = BaseStudentModuleHistory.get_history(student_module)
self.assertEquals(len(history), 3)
self.assertEquals({'type': 'csmh', 'order': 3}, json.loads(history[0].state))
self.assertEquals({'type': 'csmh', 'order': 2}, json.loads(history[1].state))
self.assertEquals({'type': 'csmh', 'order': 1}, json.loads(history[2].state))
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_CSMH_EXTENDED": False})
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_READING_FROM_MULTIPLE_HISTORY_TABLES": False})
def test_get_history_false_false(self):
student_module = StudentModule.objects.all()
history = BaseStudentModuleHistory.get_history(student_module)
self.assertEquals(len(history), 0)

4
lms/envs/acceptance.py
View File

@@ -153,7 +153,9 @@ LETTUCE_APPS = ('courseware', 'instructor')
# This causes some pretty cryptic errors as lettuce tries
# to parse files in `instructor_task` as features.
# As a quick workaround, explicitly exclude the `instructor_task` app.
LETTUCE_AVOID_APPS = ('instructor_task',)
# The coursewarehistoryextended app also falls prey to this fuzzy
# for the courseware app.
LETTUCE_AVOID_APPS = ('instructor_task', 'coursewarehistoryextended')
LETTUCE_BROWSER = os.environ.get('LETTUCE_BROWSER', 'chrome')

4
lms/envs/aws.py
View File

@@ -771,3 +771,7 @@ if ENV_TOKENS.get('AUDIT_CERT_CUTOFF_DATE', None):
################################ Settings for Credentials Service ################################
CREDENTIALS_GENERATION_ROUTING_KEY = HIGH_PRIORITY_QUEUE
# The extended StudentModule history table
if FEATURES.get('ENABLE_CSMH_EXTENDED'):
INSTALLED_APPS += ('coursewarehistoryextended',)

5
lms/envs/bok_choy.py
View File

@@ -178,6 +178,11 @@ PROFILE_IMAGE_BACKEND = {
'base_url': os.path.join(MEDIA_URL, 'profile-images/'),
},
}
# Make sure we test with the extended history table
FEATURES['ENABLE_CSMH_EXTENDED'] = True
INSTALLED_APPS += ('coursewarehistoryextended',)
#####################################################################
# Lastly, see if the developer has any local overrides.
try:

19
lms/envs/common.py
View File

@@ -363,6 +363,18 @@ FEATURES = {
# Show Language selector.
'SHOW_LANGUAGE_SELECTOR': False,
# Write new CSM history to the extended table.
# This will eventually default to True and may be
# removed since all installs should have the separate
# extended history table.
'ENABLE_CSMH_EXTENDED': False,
# Read from both the CSMH and CSMHE history tables.
# This is the default, but can be disabled if all history
# lives in the Extended table, saving the frontend from
# making multiple queries.
'ENABLE_READING_FROM_MULTIPLE_HISTORY_TABLES': True
}
# Ignore static asset files on import which match this pattern
@@ -416,7 +428,7 @@ STATUS_MESSAGE_PATH = ENV_ROOT / "status_message.json"
############################ Global Database Configuration #####################
DATABASE_ROUTERS = [
'courseware.routers.StudentModuleHistoryRouter',
'openedx.core.lib.django_courseware_routers.StudentModuleHistoryExtendedRouter',
]
############################ OpenID Provider ##################################
@@ -2766,7 +2778,10 @@ MOBILE_APP_USER_AGENT_REGEXES = [
]
# Offset for courseware.StudentModuleHistoryExtended which is used to
# calculate the starting primary key for the underlying table.
# calculate the starting primary key for the underlying table. This gap
# should be large enough that you do not generate more than N courseware.StudentModuleHistory
# records before you have deployed the app to write to coursewarehistoryextended.StudentModuleHistoryExtended
# if you want to avoid an overlap in ids while searching for history across the two tables.
STUDENTMODULEHISTORYEXTENDED_OFFSET = 10000
# Deprecated xblock types

4
lms/envs/test.py
View File

@@ -197,6 +197,10 @@ if os.environ.get('DISABLE_MIGRATIONS'):
# to Django 1.9, which allows setting MIGRATION_MODULES to None in order to skip migrations.
MIGRATION_MODULES = NoOpMigrationModules()
# Make sure we test with the extended history table
FEATURES['ENABLE_CSMH_EXTENDED'] = True
INSTALLED_APPS += ('coursewarehistoryextended',)
CACHES = {
# This is the cache used for most things.
# In staging/prod envs, the sessions also live here.

20
lms/djangoapps/courseware/routers.py → openedx/core/lib/django_courseware_routers.py
View File

@@ -1,27 +1,27 @@
"""
Database Routers for use with the courseware django app.
Database Routers for use with the coursewarehistoryextended django app.
"""
class StudentModuleHistoryRouter(object):
class StudentModuleHistoryExtendedRouter(object):
"""
A Database Router that separates StudentModuleHistory into its own database.
A Database Router that separates StudentModuleHistoryExtended into its own database.
"""
DATABASE_NAME = 'student_module_history'
def _is_csmh(self, model):
"""
Return True if ``model`` is courseware.StudentModuleHistory.
Return True if ``model`` is courseware.StudentModuleHistoryExtended.
"""
return (
model._meta.app_label == 'courseware' and # pylint: disable=protected-access
model.__name__ == 'StudentModuleHistory'
model._meta.app_label == 'coursewarehistoryextended' and # pylint: disable=protected-access
model.__name__ == 'StudentModuleHistoryExtended'
)
def db_for_read(self, model, **hints): # pylint: disable=unused-argument
"""
Use the StudentModuleHistoryRouter.DATABASE_NAME if the model is StudentModuleHistory.
Use the StudentModuleHistoryExtendedRouter.DATABASE_NAME if the model is StudentModuleHistoryExtended.
"""
if self._is_csmh(model):
return self.DATABASE_NAME
@@ -30,7 +30,7 @@ class StudentModuleHistoryRouter(object):
def db_for_write(self, model, **hints): # pylint: disable=unused-argument
"""
Use the StudentModuleHistoryRouter.DATABASE_NAME if the model is StudentModuleHistory.
Use the StudentModuleHistoryExtendedRouter.DATABASE_NAME if the model is StudentModuleHistoryExtended.
"""
if self._is_csmh(model):
return self.DATABASE_NAME
@@ -39,7 +39,7 @@ class StudentModuleHistoryRouter(object):
def allow_relation(self, obj1, obj2, **hints): # pylint: disable=unused-argument
"""
Disable relations if the model is StudentModuleHistory.
Disable relations if the model is StudentModuleHistoryExtended.
"""
if self._is_csmh(obj1) or self._is_csmh(obj2):
return False
@@ -47,7 +47,7 @@ class StudentModuleHistoryRouter(object):
def allow_migrate(self, db, model): # pylint: disable=unused-argument
"""
Only sync StudentModuleHistory to StudentModuleHistoryRouter.DATABASE_Name
Only sync StudentModuleHistoryExtended to StudentModuleHistoryExtendedRouter.DATABASE_Name
"""
if self._is_csmh(model):
return db == self.DATABASE_NAME