From ae16394ee5903ddeea8ce91110cfc17d855ace07 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 21 Jan 2021 15:38:53 -0800 Subject: [PATCH] Fix: update new runtime's runtime.anonymous_student_id to store in DB This addresses a longstanding TODO item to make runtime.anonymous_student_id for content libraries v2 work the same way as it does for XBlocks in regular courses, persisting the "context ID" (equivalent to course ID) to the database. This way, if SECRET KEY is changed, existing anonymous IDs will continue to work unchanged. This is a potentially breaking change, but should mostly affect capa problems using external code graders or Matlab code input, and I'm not aware of any such usage of the new runtime / libraries v2. --- .../migrations/0039_anon_id_context.py | 30 +++++++++++++++++++ common/djangoapps/student/models.py | 4 +-- .../core/djangoapps/xblock/runtime/shims.py | 18 ++--------- 3 files changed, 35 insertions(+), 17 deletions(-) create mode 100644 common/djangoapps/student/migrations/0039_anon_id_context.py diff --git a/common/djangoapps/student/migrations/0039_anon_id_context.py b/common/djangoapps/student/migrations/0039_anon_id_context.py new file mode 100644 index 0000000000..f241d3623c --- /dev/null +++ b/common/djangoapps/student/migrations/0039_anon_id_context.py @@ -0,0 +1,30 @@ +# Convert the student.models.AnonymousUserId.course_id field from CourseKey to +# the more generic LearningContextKey. +# +# This migration does not produce any changes at the database level. + +from django.db import migrations +import opaque_keys.edx.django.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('student', '0038_auto_20201021_1256'), + ] + + operations = [ + migrations.SeparateDatabaseAndState( + # Do not actually make any changes to the database; the fields are identical string fields with the same + # database properties. + database_operations=[], + # But update the migrator's view of the field to reflect the new field type. + state_operations=[ + migrations.AlterField( + model_name='anonymoususerid', + name='course_id', + field=opaque_keys.edx.django.models.LearningContextKeyField(blank=True, db_index=True, max_length=255), + ), + ], + ), + ] diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 2c58927ee9..15c8c7ec78 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -46,7 +46,7 @@ from edx_django_utils.cache import RequestCache from edx_rest_api_client.exceptions import SlumberBaseException from eventtracking import tracker from model_utils.models import TimeStampedModel -from opaque_keys.edx.django.models import CourseKeyField +from opaque_keys.edx.django.models import CourseKeyField, LearningContextKeyField from opaque_keys.edx.keys import CourseKey from pytz import UTC from simple_history.models import HistoricalRecords @@ -143,7 +143,7 @@ class AnonymousUserId(models.Model): user = models.ForeignKey(User, db_index=True, on_delete=models.CASCADE) anonymous_user_id = models.CharField(unique=True, max_length=32) - course_id = CourseKeyField(db_index=True, max_length=255, blank=True) + course_id = LearningContextKeyField(db_index=True, max_length=255, blank=True) def anonymous_id_for_user(user, course_id, save=True): diff --git a/openedx/core/djangoapps/xblock/runtime/shims.py b/openedx/core/djangoapps/xblock/runtime/shims.py index a5afd95357..0cc43b6cff 100644 --- a/openedx/core/djangoapps/xblock/runtime/shims.py +++ b/openedx/core/djangoapps/xblock/runtime/shims.py @@ -3,7 +3,6 @@ Code to implement backwards compatibility """ # pylint: disable=no-member -import hashlib import warnings from django.conf import settings @@ -12,9 +11,9 @@ from django.template import TemplateDoesNotExist from django.utils.functional import cached_property from fs.memoryfs import MemoryFS from openedx.core.djangoapps.xblock.apps import get_xblock_app_config -import six from common.djangoapps.edxmako.shortcuts import render_to_string +from common.djangoapps.student.models import anonymous_id_for_user class RuntimeShim(object): @@ -71,19 +70,8 @@ class RuntimeShim(object): # really care since this user's XBlock data is ephemeral and is only # kept around for a day or two anyways. return self.user_id - #### TEMPORARY IMPLEMENTATION: - # TODO: Update student.models.AnonymousUserId to have a 'context_key' - # column instead of 'course_key' (no DB migration needed). Then change - # this to point to the existing student.models.anonymous_id_for_user - # code. (Or do we want a separate new table for the new runtime?) - # The reason we can't do this now is that ContextKey doesn't yet exist - # in the opaque-keys repo, so there is no working 'ContextKeyField' - # class in opaque-keys that accepts either old CourseKeys or new - # LearningContextKeys. - hasher = hashlib.md5() - hasher.update(settings.SECRET_KEY.encode('utf-8')) - hasher.update(six.text_type(self.user_id).encode('utf-8')) - digest = hasher.hexdigest() + context_key = self._active_block.scope_ids.usage_id.context_key + digest = anonymous_id_for_user(self.user, course_id=context_key) return digest @property