From 6fb8335b632e01fa128ac9c1d6aa6f5aafb34019 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Tue, 27 Oct 2015 21:45:46 -0400 Subject: [PATCH] Add make_course_usage_key method to modulestore Different modulestores create different usage_keys for the root course block. This commit introduces a new method on the modulestore so each store type can implement its own. This is required by the new Course Blocks API since it wants to derive a usage key from a client-supplied course key. --- .../xmodule/xmodule/modulestore/__init__.py | 8 +++++++ .../lib/xmodule/xmodule/modulestore/mixed.py | 9 ++++++++ .../xmodule/xmodule/modulestore/mongo/base.py | 7 +++++++ .../xmodule/modulestore/split_mongo/split.py | 9 ++++++++ .../xmodule/modulestore/tests/test_mongo.py | 7 +++++++ .../tests/test_split_modulestore.py | 21 ++++++++++++++++++- common/lib/xmodule/xmodule/modulestore/xml.py | 9 +++++++- 7 files changed, 68 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 32a04f1083..35894c3d0f 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -906,6 +906,14 @@ class ModuleStoreRead(ModuleStoreAssetBase): """ pass + @abstractmethod + def make_course_usage_key(self, course_key): + """ + Return a valid :class:`~opaque_keys.edx.keys.UsageKey` for this modulestore + that matches the supplied course_key. + """ + pass + @abstractmethod def get_courses(self, **kwargs): ''' diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 65bb345e1d..ac20f7ad64 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -313,6 +313,15 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): # Otherwise, return the key created by the default store return self.default_modulestore.make_course_key(org, course, run) + def make_course_usage_key(self, course_key): + """ + Return a valid :class:`~opaque_keys.edx.keys.UsageKey` for the modulestore + that matches the supplied course_key. + """ + assert isinstance(course_key, CourseKey) + store = self._get_modulestore_for_courselike(course_key) + return store.make_course_usage_key(course_key) + @strip_key def get_course(self, course_key, depth=0, **kwargs): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 6b554554a2..10f37f9239 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -1033,6 +1033,13 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo """ return CourseLocator(org, course, run, deprecated=True) + def make_course_usage_key(self, course_key): + """ + Return a valid :class:`~opaque_keys.edx.keys.UsageKey` for this modulestore + that matches the supplied course_key. + """ + return BlockUsageLocator(course_key, 'course', course_key.run) + def get_course(self, course_key, depth=0, **kwargs): """ Get the course with the given courseid (org/course/run) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index cad0e2a918..8018613aaf 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -70,6 +70,7 @@ from xmodule.errortracker import null_error_tracker from opaque_keys.edx.locator import ( BlockUsageLocator, DefinitionLocator, CourseLocator, LibraryLocator, VersionTree, LocalId, ) +from ccx_keys.locator import CCXLocator, CCXBlockUsageLocator from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError, DuplicateItemError, \ DuplicateCourseError from xmodule.modulestore import ( @@ -949,6 +950,14 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): """ return CourseLocator(org, course, run) + def make_course_usage_key(self, course_key): + """ + Return a valid :class:`~opaque_keys.edx.keys.UsageKey` for this modulestore + that matches the supplied course_key. + """ + locator_cls = CCXBlockUsageLocator if isinstance(course_key, CCXLocator) else BlockUsageLocator + return locator_cls(course_key, 'course', 'course') + def _get_structure(self, structure_id, depth, head_validation=True, **kwargs): """ Gets Course or Library by locator diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 6cc48c4d15..b635a67110 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -758,6 +758,13 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): # Clean up the data so we don't break other tests which apparently expect a particular state self.draft_store.delete_course(course.id, self.dummy_user) + def test_make_course_usage_key(self): + """Test that we get back the appropriate usage key for the root of a course key.""" + course_key = CourseLocator(org="edX", course="101", run="2015") + root_block_key = self.draft_store.make_course_usage_key(course_key) + self.assertEqual(root_block_key.block_type, "course") + self.assertEqual(root_block_key.name, "2015") + class TestMongoModuleStoreWithNoAssetCollection(TestMongoModuleStore): ''' diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 3b09ac10b9..5e10752fa7 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -10,6 +10,7 @@ import re import unittest import uuid +import ddt from contracts import contract from nose.plugins.attrib import attr from django.core.cache import get_cache, InvalidCacheBackendError @@ -23,7 +24,8 @@ from xmodule.modulestore.exceptions import ( DuplicateItemError, DuplicateCourseError, InsufficientSpecificationError ) -from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator, VersionTree, LocalId +from opaque_keys.edx.locator import CourseKey, CourseLocator, BlockUsageLocator, VersionTree, LocalId +from ccx_keys.locator import CCXBlockUsageLocator from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.x_module import XModuleMixin from xmodule.fields import Date, Timedelta @@ -596,6 +598,7 @@ class TestHasChildrenAtDepth(SplitModuleTest): self.assertFalse(ch3.has_children_at_depth(1)) +@ddt.ddt class SplitModuleCourseTests(SplitModuleTest): ''' Course CRUD operation tests @@ -908,6 +911,22 @@ class SplitModuleCourseTests(SplitModuleTest): version_history = modulestore().get_block_generations(second_problem.location) self.assertNotEqual(version_history.locator.version_guid, first_problem.location.version_guid) + @ddt.data( + ("course-v1:edx+test_course+test_run", BlockUsageLocator), + ("ccx-v1:edX+test_course+test_run+ccx@1", CCXBlockUsageLocator), + ) + @ddt.unpack + def test_make_course_usage_key(self, course_id, root_block_cls): + """Test that we get back the appropriate usage key for the root of a course key. + + In particular, we want to make sure that it properly handles CCX courses. + """ + course_key = CourseKey.from_string(course_id) + root_block_key = modulestore().make_course_usage_key(course_key) + self.assertIsInstance(root_block_key, root_block_cls) + self.assertEqual(root_block_key.block_type, "course") + self.assertEqual(root_block_key.name, "course") + class TestCourseStructureCache(SplitModuleTest): """Tests for the CourseStructureCache""" diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 3341e35040..7cd2c615fe 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -27,7 +27,7 @@ from xmodule.modulestore.xml_exporter import DEFAULT_CONTENT_FIELDS from xmodule.modulestore import ModuleStoreEnum, ModuleStoreReadBase, LIBRARY_ROOT, COURSE_ROOT from xmodule.tabs import CourseTabList from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location -from opaque_keys.edx.locator import CourseLocator, LibraryLocator +from opaque_keys.edx.locator import CourseLocator, LibraryLocator, BlockUsageLocator from xblock.field_data import DictFieldData from xblock.runtime import DictKeyValueStore @@ -821,6 +821,13 @@ class XMLModuleStore(ModuleStoreReadBase): """ return CourseLocator(org, course, run, deprecated=True) + def make_course_usage_key(self, course_key): + """ + Return a valid :class:`~opaque_keys.edx.keys.UsageKey` for this modulestore + that matches the supplied course_key. + """ + return BlockUsageLocator(course_key, 'course', course_key.run) + def get_courses(self, **kwargs): """ Returns a list of course descriptors. If there were errors on loading,