Save user state for Blockstore XBlocks in CSM, clean up CSM a bit (#21630)
This commit introduces the changes needed for XBlocks in Blockstore to save their user state into CSM. Before this commit, all student state for Blockstore blocks was ephemeral (in-process dict store). Notes: * The main risk factor of this PR is that it adds non-course keys to the course_id field in CSM. If any code (like analytics?) reads course keys directly out of CSM and doesn't have graceful handling for key types it doesn't recognize, it could cause an issue. With the included changes to opaque-keys, calling CourseKey.from_string(...) on these values will raise InvalidKeyError since they're not CourseKeys. (But calling LearningContextKey.from_string(...) will work for both course and library keys.) * This commit introduces a slight regression for the Studio view of XBlocks in Blockstore content libraries: their state is now lost from request to request. I have a follow up PR to give them a proper studio-appropriate state store, but I want to review it separately so it doesn't hold up this PR and we can test this PR on its own.
This commit is contained in:
committed by
David Ormsbee
parent
742c254562
commit
1382bf8720
32
lms/djangoapps/courseware/migrations/0012_adjust_fields.py
Normal file
32
lms/djangoapps/courseware/migrations/0012_adjust_fields.py
Normal file
@@ -0,0 +1,32 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Generated by Django 1.11.23 on 2019-09-08 04:54
|
||||
#
|
||||
# This migration does not produce any actual database changes; it only affects
|
||||
# the python code. You can confirm this with:
|
||||
# ./manage.py lms sqlmigrate courseware 0012_adjust_fields
|
||||
from __future__ import unicode_literals
|
||||
|
||||
from django.conf import settings
|
||||
from django.db import migrations, models
|
||||
import opaque_keys.edx.django.models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
|
||||
('courseware', '0011_csm_id_bigint'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AlterField(
|
||||
model_name='studentmodule',
|
||||
name='course_id',
|
||||
field=opaque_keys.edx.django.models.LearningContextKeyField(db_index=True, max_length=255),
|
||||
),
|
||||
migrations.AlterField(
|
||||
model_name='studentmodule',
|
||||
name='module_type',
|
||||
field=models.CharField(db_index=True, max_length=32),
|
||||
),
|
||||
]
|
||||
@@ -33,7 +33,7 @@ from contracts import contract, new_contract
|
||||
from django.db import DatabaseError, IntegrityError, transaction
|
||||
from opaque_keys.edx.asides import AsideUsageKeyV1, AsideUsageKeyV2
|
||||
from opaque_keys.edx.block_types import BlockTypeKeyV1
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from opaque_keys.edx.keys import LearningContextKey
|
||||
from xblock.core import XBlockAside
|
||||
from xblock.exceptions import InvalidScopeError, KeyValueMultiSaveError
|
||||
from xblock.fields import Scope, UserScope
|
||||
@@ -703,7 +703,7 @@ class FieldDataCache(object):
|
||||
else:
|
||||
self.asides = asides
|
||||
|
||||
assert isinstance(course_id, CourseKey)
|
||||
assert isinstance(course_id, LearningContextKey)
|
||||
self.course_id = course_id
|
||||
self.user = user
|
||||
self.read_only = read_only
|
||||
@@ -997,13 +997,14 @@ def set_score(user_id, usage_key, score, max_score):
|
||||
Set the score and max_score for the specified user and xblock usage.
|
||||
"""
|
||||
created = False
|
||||
kwargs = {"student_id": user_id, "module_state_key": usage_key, "course_id": usage_key.course_key}
|
||||
kwargs = {"student_id": user_id, "module_state_key": usage_key, "course_id": usage_key.context_key}
|
||||
try:
|
||||
with transaction.atomic():
|
||||
student_module, created = StudentModule.objects.get_or_create(
|
||||
defaults={
|
||||
'grade': score,
|
||||
'max_grade': max_score,
|
||||
'module_type': usage_key.block_type,
|
||||
},
|
||||
**kwargs
|
||||
)
|
||||
@@ -1012,7 +1013,7 @@ def set_score(user_id, usage_key, score, max_score):
|
||||
log.exception(
|
||||
u'set_score: IntegrityError for student %s - course_id %s - usage_key %s having '
|
||||
u'score %d and max_score %d',
|
||||
str(user_id), usage_key.course_key, usage_key, score, max_score
|
||||
str(user_id), usage_key.context_key, usage_key, score, max_score
|
||||
)
|
||||
student_module = StudentModule.objects.get(**kwargs)
|
||||
|
||||
|
||||
@@ -25,7 +25,7 @@ from django.db import models
|
||||
from django.db.models.signals import post_save
|
||||
from django.utils.translation import ugettext_lazy as _
|
||||
from model_utils.models import TimeStampedModel
|
||||
from opaque_keys.edx.django.models import BlockTypeKeyField, CourseKeyField, UsageKeyField
|
||||
from opaque_keys.edx.django.models import BlockTypeKeyField, CourseKeyField, LearningContextKeyField, UsageKeyField
|
||||
from courseware.fields import UnsignedBigIntAutoField
|
||||
from six import text_type
|
||||
from six.moves import range
|
||||
@@ -79,33 +79,25 @@ class ChunkingManager(models.Manager):
|
||||
|
||||
class StudentModule(models.Model):
|
||||
"""
|
||||
Keeps student state for a particular module in a particular course.
|
||||
Keeps student state for a particular XBlock usage and particular student.
|
||||
|
||||
Called Module since it was originally used for XModule state.
|
||||
|
||||
.. no_pii:
|
||||
"""
|
||||
objects = ChunkingManager()
|
||||
MODEL_TAGS = ['course_id', 'module_type']
|
||||
|
||||
# For a homework problem, contains a JSON
|
||||
# object consisting of state
|
||||
MODULE_TYPES = (('problem', 'problem'),
|
||||
('video', 'video'),
|
||||
('html', 'html'),
|
||||
('course', 'course'),
|
||||
('chapter', 'Section'),
|
||||
('sequential', 'Subsection'),
|
||||
('library_content', 'Library Content'))
|
||||
|
||||
id = UnsignedBigIntAutoField(primary_key=True) # pylint: disable=invalid-name
|
||||
|
||||
## These three are the key for the object
|
||||
module_type = models.CharField(max_length=32, choices=MODULE_TYPES, default='problem', db_index=True)
|
||||
## The XBlock/XModule type (e.g. "problem")
|
||||
module_type = models.CharField(max_length=32, db_index=True)
|
||||
|
||||
# Key used to share state. This is the XBlock usage_id
|
||||
module_state_key = UsageKeyField(max_length=255, db_column='module_id')
|
||||
student = models.ForeignKey(User, db_index=True, db_constraint=False, on_delete=models.CASCADE)
|
||||
|
||||
course_id = CourseKeyField(max_length=255, db_index=True)
|
||||
# The learning context of the usage_key (usually a course ID, but may be a library or something else)
|
||||
course_id = LearningContextKeyField(max_length=255, db_index=True)
|
||||
|
||||
class Meta(object):
|
||||
app_label = "courseware"
|
||||
|
||||
@@ -499,6 +499,13 @@ DATABASE_ROUTERS = [
|
||||
############################ Cache Configuration ###############################
|
||||
|
||||
CACHES = {
|
||||
'blockstore': {
|
||||
'KEY_PREFIX': 'blockstore',
|
||||
'KEY_FUNCTION': 'util.memcache.safe_key',
|
||||
'LOCATION': ['localhost:11211'],
|
||||
'TIMEOUT': '86400', # This data should be long-lived for performance, BundleCache handles invalidation
|
||||
'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache',
|
||||
},
|
||||
'course_structure_cache': {
|
||||
'KEY_PREFIX': 'course_structure',
|
||||
'KEY_FUNCTION': 'util.memcache.safe_key',
|
||||
|
||||
@@ -233,6 +233,13 @@ CACHES = {
|
||||
'course_structure_cache': {
|
||||
'BACKEND': 'django.core.cache.backends.dummy.DummyCache',
|
||||
},
|
||||
# Blockstore caching tests require a cache that actually works:
|
||||
'blockstore': {
|
||||
'KEY_PREFIX': 'blockstore',
|
||||
'KEY_FUNCTION': 'util.memcache.safe_key',
|
||||
'LOCATION': 'edx_loc_mem_cache',
|
||||
'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
|
||||
},
|
||||
}
|
||||
|
||||
############################### BLOCKSTORE #####################################
|
||||
|
||||
Reference in New Issue
Block a user