diff --git a/openedx/core/djangoapps/content/block_structure/migrations/0004_blockstructuremodel_usagekeywithrun.py b/openedx/core/djangoapps/content/block_structure/migrations/0004_blockstructuremodel_usagekeywithrun.py new file mode 100644 index 0000000000..670a8ae8bd --- /dev/null +++ b/openedx/core/djangoapps/content/block_structure/migrations/0004_blockstructuremodel_usagekeywithrun.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import openedx.core.djangoapps.xmodule_django.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('block_structure', '0003_blockstructuremodel_storage'), + ] + + operations = [ + migrations.AlterField( + model_name='blockstructuremodel', + name='data_usage_key', + field=openedx.core.djangoapps.xmodule_django.models.UsageKeyWithRunField(unique=True, max_length=255, verbose_name='Identifier of the data being collected.'), + ), + ] diff --git a/openedx/core/djangoapps/content/block_structure/models.py b/openedx/core/djangoapps/content/block_structure/models.py index 3ae9b67273..b24010a4bb 100644 --- a/openedx/core/djangoapps/content/block_structure/models.py +++ b/openedx/core/djangoapps/content/block_structure/models.py @@ -11,7 +11,7 @@ from django.db import models, transaction from logging import getLogger from model_utils.models import TimeStampedModel -from openedx.core.djangoapps.xmodule_django.models import UsageKeyField +from openedx.core.djangoapps.xmodule_django.models import UsageKeyWithRunField from openedx.core.storage import get_storage from . import config @@ -33,9 +33,12 @@ def _directory_name(data_usage_key): Returns the directory name for the given data_usage_key. """ + # replace any '/' in the usage key so they aren't interpreted + # as folder separators. + encoded_usage_key = unicode(data_usage_key).replace('/', '_') return '{}{}'.format( settings.BLOCK_STRUCTURES_SETTINGS.get('DIRECTORY_PREFIX', ''), - unicode(data_usage_key), + encoded_usage_key, ) @@ -133,7 +136,7 @@ class BlockStructureModel(TimeStampedModel): class Meta(object): db_table = 'block_structure' - data_usage_key = UsageKeyField( + data_usage_key = UsageKeyWithRunField( u'Identifier of the data being collected.', blank=False, max_length=255, diff --git a/openedx/core/djangoapps/content/block_structure/tests/test_models.py b/openedx/core/djangoapps/content/block_structure/tests/test_models.py index a987ddc761..c455115167 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_models.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_models.py @@ -14,7 +14,7 @@ from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator from ..config import PRUNE_OLD_VERSIONS from ..exceptions import BlockStructureNotFound -from ..models import BlockStructureModel, _storage_error_handling +from ..models import BlockStructureModel, _directory_name, _storage_error_handling from .helpers import override_config_setting @@ -44,7 +44,7 @@ class BlockStructureModelTestCase(TestCase): self.assertEqual(field_value, getattr(bsm, field_name)) self.assertEqual(bsm.get_serialized_data(), expected_serialized_data) - self.assertIn(unicode(self.usage_key), bsm.data.name) + self.assertIn(_directory_name(self.usage_key), bsm.data.name) def _assert_file_count_equal(self, expected_count): """ @@ -160,3 +160,17 @@ class BlockStructureModelTestCase(TestCase): with self.assertRaises(expected_error_raised): with _storage_error_handling(bs_model, 'operation', is_read_operation): raise error_raised_in_operation + + @patch('openedx.core.djangoapps.content.block_structure.models.log') + def test_old_mongo_keys(self, mock_log): + self.course_key = CourseLocator('org2', 'course2', unicode(uuid4()), deprecated=True) + self.usage_key = BlockUsageLocator(course_key=self.course_key, block_type='course', block_id='course') + serialized_data = 'test data for old course' + self.params['data_usage_key'] = self.usage_key + + with patch('xmodule.modulestore.mixed.MixedModuleStore.fill_in_run') as mock_fill_in_run: + mock_fill_in_run.return_value = self.usage_key.course_key + self._verify_update_or_create_call(serialized_data, mock_log, expect_created=True) + found_bsm = BlockStructureModel.get(self.usage_key) + + self._assert_bsm_fields(found_bsm, serialized_data) diff --git a/openedx/core/djangoapps/xmodule_django/models.py b/openedx/core/djangoapps/xmodule_django/models.py index fe32953342..2987ca41e7 100644 --- a/openedx/core/djangoapps/xmodule_django/models.py +++ b/openedx/core/djangoapps/xmodule_django/models.py @@ -7,6 +7,8 @@ import logging from django.db import models from django.core.exceptions import ValidationError from opaque_keys.edx.keys import CourseKey, UsageKey, BlockTypeKey +from xmodule.modulestore.django import modulestore + log = logging.getLogger(__name__) @@ -181,6 +183,18 @@ class UsageKeyField(OpaqueKeyField): KEY_CLASS = UsageKey +class UsageKeyWithRunField(UsageKeyField): + """ + Subclass of UsageKeyField that automatically fills in + missing `run` values, for old Mongo courses. + """ + def to_python(self, value): + value = super(UsageKeyWithRunField, self).to_python(value) + if value is not None and value.run is None: + value = value.replace(course_key=modulestore().fill_in_run(value.course_key)) + return value + + class LocationKeyField(UsageKeyField): """ A django Field that stores a UsageKey object as a string.