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

Make studio case-insensitive for course keys for creating courses.

Browse Source 
ECOM-4890
This commit is contained in:
Awais Jibran
2016-07-27 16:47:04 +05:00
parent e3397671a2
commit 4bc5ded768
10 changed files with 117 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
cms/djangoapps/contentstore/management/commands/tests/test_create_course.py
View File

@@ -61,3 +61,35 @@ class TestCreateCourse(ModuleStoreTestCase):
)
# pylint: disable=protected-access
self.assertEqual(store, modulestore()._get_modulestore_for_courselike(new_key).get_modulestore_type())
@ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
def test_get_course_with_different_case(self, default_store):
"""
Tests that course can not be accessed with different case.
Scenario:
Create a course with lower case keys inside `bulk_operations` with `ignore_case=True`.
Verify that course is created.
Verify that get course from store using same course id but different case is not accessible.
"""
org = 'org1'
number = 'course1'
run = 'run1'
with self.store.default_store(default_store):
lowercase_course_id = self.store.make_course_key(org, number, run)
with self.store.bulk_operations(lowercase_course_id, ignore_case=True):
# Create course with lowercase key & Verify that store returns course.
self.store.create_course(
lowercase_course_id.org,
lowercase_course_id.course,
lowercase_course_id.run,
self.user.id
)
course = self.store.get_course(lowercase_course_id)
self.assertIsNotNone(course, 'Course not found using lowercase course key.')
self.assertEqual(unicode(course.id), unicode(lowercase_course_id))
# Verify store does not return course with different case.
uppercase_course_id = self.store.make_course_key(org.upper(), number.upper(), run.upper())
course = self.store.get_course(uppercase_course_id)
self.assertIsNone(course, 'Course should not be accessed with uppercase course id.')

62
cms/djangoapps/contentstore/tests/test_contentstore.py
View File

@@ -1151,6 +1151,9 @@ class ContentStoreTest(ContentStoreTestCase, XssTestMixin):
"""
Tests for the CMS ContentStore application.
"""
duplicate_course_error = ("There is already a course defined with the same organization and course number. "
"Please change either organization or course number to be unique.")
def setUp(self):
super(ContentStoreTest, self).setUp()
@@ -1203,6 +1206,22 @@ class ContentStoreTest(ContentStoreTestCase, XssTestMixin):
self.course_data['run'] = 'run.name'
self.assert_created_course()
@ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
def test_course_with_different_cases(self, default_store):
"""
Tests that course can not be created with different case using an AJAX request to
course handler.
"""
course_number = '99x'
with self.store.default_store(default_store):
# Verify create a course passes with lower case.
self.course_data['number'] = course_number.lower()
self.assert_created_course()
# Verify create a course fail when same course number is provided with different case.
self.course_data['number'] = course_number.upper()
self.assert_course_creation_failed(self.duplicate_course_error)
def test_create_course_check_forum_seeding(self):
"""Test new course creation and verify forum seeding """
test_course_data = self.assert_created_course(number_suffix=uuid4().hex)
@@ -1289,7 +1308,7 @@ class ContentStoreTest(ContentStoreTestCase, XssTestMixin):
def test_create_course_duplicate_course(self):
"""Test new course creation - error path"""
self.client.ajax_post('/course/', self.course_data)
self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change either organization or course number to be unique.')
self.assert_course_creation_failed(self.duplicate_course_error)
def assert_course_creation_failed(self, error_message):
"""
@@ -1318,21 +1337,38 @@ class ContentStoreTest(ContentStoreTestCase, XssTestMixin):
self.course_data['display_name'] = 'Robot Super Course Two'
self.course_data['run'] = '2013_Summer'
self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change either organization or course number to be unique.')
self.assert_course_creation_failed(self.duplicate_course_error)
def test_create_course_case_change(self):
@ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
def test_create_course_case_change(self, default_store):
"""Test new course creation - error path due to case insensitive name equality"""
self.course_data['number'] = 'capital'
self.client.ajax_post('/course/', self.course_data)
cache_current = self.course_data['org']
self.course_data['org'] = self.course_data['org'].lower()
self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change either organization or course number to be unique.')
self.course_data['org'] = cache_current
self.course_data['number'] = '99x'
self.client.ajax_post('/course/', self.course_data)
cache_current = self.course_data['number']
self.course_data['number'] = self.course_data['number'].upper()
self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change either organization or course number to be unique.')
with self.store.default_store(default_store):
# Verify that the course was created properly.
self.assert_created_course()
# Keep the copy of original org
cache_current = self.course_data['org']
# Change `org` to lower case and verify that course did not get created
self.course_data['org'] = self.course_data['org'].lower()
self.assert_course_creation_failed(self.duplicate_course_error)
# Replace the org with its actual value, and keep the copy of course number.
self.course_data['org'] = cache_current
cache_current = self.course_data['number']
self.course_data['number'] = self.course_data['number'].upper()
self.assert_course_creation_failed(self.duplicate_course_error)
# Replace the org with its actual value, and keep the copy of course number.
self.course_data['number'] = cache_current
__ = self.course_data['run']
self.course_data['run'] = self.course_data['run'].upper()
self.assert_course_creation_failed(self.duplicate_course_error)
def test_course_substring(self):
"""

28
common/lib/xmodule/xmodule/modulestore/__init__.py
View File

@@ -178,7 +178,7 @@ class BulkOperationsMixin(object):
self.signal_handler = None
@contextmanager
def bulk_operations(self, course_id, emit_signals=True):
def bulk_operations(self, course_id, emit_signals=True, ignore_case=False):
"""
A context manager for notifying the store of bulk operations. This affects only the current thread.
@@ -186,10 +186,10 @@ class BulkOperationsMixin(object):
until the bulk operation is completed.
"""
try:
self._begin_bulk_operation(course_id)
self._begin_bulk_operation(course_id, ignore_case)
yield
finally:
self._end_bulk_operation(course_id, emit_signals)
self._end_bulk_operation(course_id, emit_signals, ignore_case)
# the relevant type of bulk_ops_record for the mixin (overriding classes should override
# this variable)
@@ -206,10 +206,10 @@ class BulkOperationsMixin(object):
if ignore_case:
for key, record in self._active_bulk_ops.records.iteritems():
# Shortcut: check basic equivalence for cases where org/course/run might be None.
if key == course_key or (
key.org.lower() == course_key.org.lower() and
key.course.lower() == course_key.course.lower() and
key.run.lower() == course_key.run.lower()
if (key == course_key) or (
(key.org and key.org.lower() == course_key.org.lower()) and
(key.course and key.course.lower() == course_key.course.lower()) and
(key.run and key.run.lower() == course_key.run.lower())
):
return record
@@ -231,7 +231,7 @@ class BulkOperationsMixin(object):
if course_key.for_branch(None) in self._active_bulk_ops.records:
del self._active_bulk_ops.records[course_key.for_branch(None)]
def _start_outermost_bulk_operation(self, bulk_ops_record, course_key):
def _start_outermost_bulk_operation(self, bulk_ops_record, course_key, ignore_case=False):
"""
The outermost nested bulk_operation call: do the actual begin of the bulk operation.
@@ -239,11 +239,11 @@ class BulkOperationsMixin(object):
"""
pass
def _begin_bulk_operation(self, course_key):
def _begin_bulk_operation(self, course_key, ignore_case=False):
"""
Begin a bulk operation on course_key.
"""
bulk_ops_record = self._get_bulk_ops_record(course_key)
bulk_ops_record = self._get_bulk_ops_record(course_key, ignore_case)
# Increment the number of active bulk operations (bulk operations
# on the same course can be nested)
@@ -251,7 +251,7 @@ class BulkOperationsMixin(object):
# If this is the highest level bulk operation, then initialize it
if bulk_ops_record.is_root:
self._start_outermost_bulk_operation(bulk_ops_record, course_key)
self._start_outermost_bulk_operation(bulk_ops_record, course_key, ignore_case)
def _end_outermost_bulk_operation(self, bulk_ops_record, structure_key):
"""
@@ -261,12 +261,12 @@ class BulkOperationsMixin(object):
"""
pass
def _end_bulk_operation(self, structure_key, emit_signals=True):
def _end_bulk_operation(self, structure_key, emit_signals=True, ignore_case=False):
"""
End the active bulk operation on structure_key (course or library key).
"""
# If no bulk op is active, return
bulk_ops_record = self._get_bulk_ops_record(structure_key)
bulk_ops_record = self._get_bulk_ops_record(structure_key, ignore_case)
if not bulk_ops_record.active:
return
@@ -1017,7 +1017,7 @@ class ModuleStoreRead(ModuleStoreAssetBase):
pass
@contextmanager
def bulk_operations(self, course_id, emit_signals=True): # pylint: disable=unused-argument
def bulk_operations(self, course_id, emit_signals=True, ignore_case=False): # pylint: disable=unused-argument
"""
A context manager for notifying the store of bulk operations. This affects only the current thread.
"""

4
common/lib/xmodule/xmodule/modulestore/mixed.py
View File

@@ -984,13 +984,13 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
yield
@contextmanager
def bulk_operations(self, course_id, emit_signals=True):
def bulk_operations(self, course_id, emit_signals=True, ignore_case=False):
"""
A context manager for notifying the store of bulk operations.
If course_id is None, the default store is used.
"""
store = self._get_modulestore_for_courselike(course_id)
with store.bulk_operations(course_id, emit_signals):
with store.bulk_operations(course_id, emit_signals, ignore_case):
yield
def ensure_indexes(self):

2
common/lib/xmodule/xmodule/modulestore/mongo/base.py
View File

@@ -482,7 +482,7 @@ class MongoBulkOpsMixin(BulkOperationsMixin):
"""
_bulk_ops_record_type = MongoBulkOpsRecord
def _start_outermost_bulk_operation(self, bulk_ops_record, course_key):
def _start_outermost_bulk_operation(self, bulk_ops_record, course_key, ignore_case=False):
"""
Prevent updating the meta-data inheritance cache for the given course
"""

6
common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
View File

@@ -226,11 +226,11 @@ class SplitBulkWriteMixin(BulkOperationsMixin):
course_key.replace(org=None, course=None, run=None, branch=None)
]
def _start_outermost_bulk_operation(self, bulk_write_record, course_key):
def _start_outermost_bulk_operation(self, bulk_write_record, course_key, ignore_case=False):
"""
Begin a bulk write operation on course_key.
"""
bulk_write_record.initial_index = self.db_connection.get_course_index(course_key)
bulk_write_record.initial_index = self.db_connection.get_course_index(course_key, ignore_case=ignore_case)
# Ensure that any edits to the index don't pollute the initial_index
bulk_write_record.index = copy.deepcopy(bulk_write_record.initial_index)
bulk_write_record.course_key = course_key
@@ -1884,7 +1884,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
"""
Internal code for creating a course or library
"""
index = self.get_course_index(locator)
index = self.get_course_index(locator, ignore_case=True)
if index is not None:
raise DuplicateCourseError(locator, index)

2
common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py
View File

@@ -33,7 +33,7 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
Returns: a CourseDescriptor
"""
master_branch = kwargs.pop('master_branch', ModuleStoreEnum.BranchName.draft)
with self.bulk_operations(CourseLocator(org, course, run)):
with self.bulk_operations(CourseLocator(org, course, run), ignore_case=True):
item = super(DraftVersioningModuleStore, self).create_course(
org, course, run, user_id, master_branch=master_branch, **kwargs
)

12
common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
View File

@@ -357,6 +357,18 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
with self.assertRaises(DuplicateCourseError):
self.store.create_course('org_x', 'course_y', 'run_z', self.user_id)
@ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
def test_duplicate_course_error_with_different_case_ids(self, default_store):
"""
Verify that course can not be created with same course_id with different case.
"""
self._initialize_mixed(mappings={})
with self.store.default_store(default_store):
self.store.create_course('org_x', 'course_y', 'run_z', self.user_id)
with self.assertRaises(DuplicateCourseError):
self.store.create_course('ORG_X', 'COURSE_Y', 'RUN_Z', self.user_id)
# Draft:
# problem: One lookup to locate an item that exists
# fake: one w/ wildcard version

2
common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py
View File

@@ -704,7 +704,7 @@ class TestBulkWriteMixinOpen(TestBulkWriteMixin):
from_index=self.conn.get_course_index.return_value,
course_context=self.course_key,
)
self.conn.get_course_index.assert_called_once_with(self.course_key)
self.conn.get_course_index.assert_called_once_with(self.course_key, ignore_case=False)
class TestBulkWriteMixinOpenAfterPrevTransaction(TestBulkWriteMixinOpen, TestBulkWriteMixinPreviousTransaction):

4
lms/djangoapps/ccx/modulestore.py
View File

@@ -296,8 +296,8 @@ class CCXModulestoreWrapper(object):
yield
@contextmanager
def bulk_operations(self, course_id, emit_signals=True):
def bulk_operations(self, course_id, emit_signals=True, ignore_case=False):
"""See the docs for xmodule.modulestore.mixed.MixedModuleStore"""
course_id, _ = strip_ccx(course_id)
with self._modulestore.bulk_operations(course_id, emit_signals=emit_signals):
with self._modulestore.bulk_operations(course_id, emit_signals=emit_signals, ignore_case=ignore_case):
yield