diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 1e464d063a..ea4753a51f 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -49,6 +49,12 @@ Blades: Fix comparison of float numbers. BLD-434.
Blades: Allow regexp strings as the correct answer to a string response question. BLD-475.
+Common: MixedModulestore is now the only approved access to the persistence layer
+ - takes a new parameter 'reference_type' which can be 'Location' or 'Locator'. Mixed
+ then tries to ensure that every reference in any xblock gets converted to that type on
+ retrieval. Because we're moving to Locators, the default is Locator; so, you should change
+ all existing configurations to 'Location' (unless you're using split)
+
Common: Add feature flags to allow developer use of pure XBlocks
- ALLOW_ALL_ADVANCED_COMPONENTS disables the hard-coded list of advanced
components in Studio, and allows any xblock to be added as an
diff --git a/cms/djangoapps/contentstore/course_info_model.py b/cms/djangoapps/contentstore/course_info_model.py
index 314793db72..0e2551cac8 100644
--- a/cms/djangoapps/contentstore/course_info_model.py
+++ b/cms/djangoapps/contentstore/course_info_model.py
@@ -102,7 +102,7 @@ def update_course_updates(location, update, passed_id=None):
# update db record
course_updates.data = html.tostring(course_html_parsed)
- modulestore('direct').update_item(location, course_updates.data)
+ modulestore('direct').update_item(course_updates, 'course_info_model')
return {
"id": idx,
@@ -158,7 +158,7 @@ def delete_course_update(location, update, passed_id):
# update db record
course_updates.data = html.tostring(course_html_parsed)
store = modulestore('direct')
- store.update_item(location, course_updates.data)
+ store.update_item(course_updates, 'course_info_model')
return get_course_updates(location, None)
diff --git a/cms/djangoapps/contentstore/tests/test_checklists.py b/cms/djangoapps/contentstore/tests/test_checklists.py
index 864fe23c83..14164680fc 100644
--- a/cms/djangoapps/contentstore/tests/test_checklists.py
+++ b/cms/djangoapps/contentstore/tests/test_checklists.py
@@ -54,7 +54,7 @@ class ChecklistTestCase(CourseTestCase):
# Save the changed `checklists` to the underlying KeyValueStore before updating the modulestore
self.course.save()
modulestore = get_modulestore(self.course.location)
- modulestore.update_metadata(self.course.location, own_metadata(self.course))
+ modulestore.update_item(self.course, self.user.pk)
self.assertEqual(self.get_persisted_checklists(), None)
response = self.client.get(self.checklists_url)
self.assertEqual(payload, response.content)
diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index d28a3ab3de..c7f4b9339c 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -47,8 +47,6 @@ from xmodule.exceptions import NotFoundError
from django_comment_common.utils import are_permissions_roles_seeded
from xmodule.exceptions import InvalidVersionError
-import datetime
-from pytz import UTC
from uuid import uuid4
from pymongo import MongoClient
from student.models import CourseEnrollment
@@ -126,11 +124,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
course.advanced_modules = component_types
- # Save the data that we've just changed to the underlying
- # MongoKeyValueStore before we update the mongo datastore.
- course.save()
-
- store.update_metadata(course.location, own_metadata(course))
+ store.update_item(course, self.user.username)
# just pick one vertical
descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0]
@@ -269,7 +263,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
self.assertIn('graceperiod', own_metadata(html_module))
self.assertEqual(html_module.graceperiod, new_graceperiod)
- draft_store.update_metadata(html_module.location, own_metadata(html_module))
+ draft_store.update_item(html_module, self.user.username)
# read back to make sure it reads as 'own-metadata'
html_module = draft_store.get_item(Location('i4x', 'edX', 'simple', 'html', 'test_html', None))
@@ -385,8 +379,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
self.assertEqual(course.tabs, expected_tabs)
item.display_name = 'Updated'
- item.save()
- module_store.update_metadata(item.location, own_metadata(item))
+ module_store.update_item(item, self.user.username)
course = module_store.get_item(course_location)
@@ -834,9 +827,9 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
html_module = module_store.get_instance(source_location.course_id, html_module_location)
self.assertIsInstance(html_module.data, basestring)
- new_data = html_module.data.replace('/static/', '/c4x/{0}/{1}/asset/'.format(
+ new_data = html_module.data = html_module.data.replace('/static/', '/c4x/{0}/{1}/asset/'.format(
source_location.org, source_location.course))
- module_store.update_item(html_module_location, new_data)
+ module_store.update_item(html_module, None)
html_module = module_store.get_instance(source_location.course_id, html_module_location)
self.assertEqual(new_data, html_module.data)
@@ -858,22 +851,18 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
draft_store = modulestore('draft')
direct_store = modulestore('direct')
- CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
+ course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
location = Location('i4x://MITx/999/chapter/neuvo')
# Ensure draft mongo store does not allow us to create chapters either directly or via convert to draft
self.assertRaises(InvalidVersionError, draft_store.create_and_save_xmodule, location)
direct_store.create_and_save_xmodule(location)
self.assertRaises(InvalidVersionError, draft_store.convert_to_draft, location)
+ chapter = draft_store.get_instance(course.id, location)
+ chapter.data = 'chapter data'
- self.assertRaises(InvalidVersionError, draft_store.update_item, location, 'chapter data')
-
- # taking advantage of update_children and other functions never checking that the ids are valid
- self.assertRaises(InvalidVersionError, draft_store.update_children, location,
- ['i4x://MITx/999/problem/doesntexist'])
-
- self.assertRaises(InvalidVersionError, draft_store.update_metadata, location,
- {'due': datetime.datetime.now(UTC)})
+ with self.assertRaises(InvalidVersionError):
+ draft_store.update_item(chapter, 'user')
self.assertRaises(InvalidVersionError, draft_store.unpublish, location)
@@ -992,8 +981,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
sequential = module_store.get_item(Location(['i4x', 'edX', 'toy',
'sequential', 'vertical_sequential', None]))
private_location_no_draft = private_vertical.location.replace(revision=None)
- module_store.update_children(sequential.location, sequential.children +
- [private_location_no_draft.url()])
+ sequential.children.append(private_location_no_draft.url())
+ module_store.update_item(sequential, 'user')
# read back the sequential, to make sure we have a pointer to
sequential = module_store.get_item(Location(['i4x', 'edX', 'toy',
@@ -1285,31 +1274,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
self.assertFalse(Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None])
in course.system.module_data)
- def test_export_course_with_unknown_metadata(self):
- module_store = modulestore('direct')
- content_store = contentstore()
-
- import_from_xml(module_store, 'common/test/data/', ['toy'])
- location = CourseDescriptor.id_to_location('edX/toy/2012_Fall')
-
- root_dir = path(mkdtemp_clean())
-
- course = module_store.get_item(location)
-
- metadata = own_metadata(course)
- # add a bool piece of unknown metadata so we can verify we don't throw an exception
- metadata['new_metadata'] = True
-
- # Save the data that we've just changed to the underlying
- # MongoKeyValueStore before we update the mongo datastore.
- course.save()
- module_store.update_metadata(location, metadata)
-
- print 'Exporting to tempdir = {0}'.format(root_dir)
-
- # export out to a tempdir
- export_to_xml(module_store, content_store, location, root_dir, 'test_export')
-
def test_export_course_without_content_store(self):
module_store = modulestore('direct')
content_store = contentstore()
@@ -1319,16 +1283,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
import_from_xml(module_store, 'common/test/data/', ['toy'])
location = CourseDescriptor.id_to_location('edX/toy/2012_Fall')
- # Add a sequence
-
stub_location = Location(['i4x', 'edX', 'toy', 'sequential', 'vertical_sequential'])
- sequential = module_store.get_item(stub_location)
- module_store.update_children(sequential.location, sequential.children)
-
- # Get course and export it without a content_store
-
- course = module_store.get_item(location)
- course.save()
root_dir = path(mkdtemp_clean())
@@ -1343,7 +1298,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
module_store, root_dir, ['test_export_no_content_store'],
draft_store=None,
static_content_store=None,
- target_location_namespace=course.location
+ target_location_namespace=location
)
# Verify reimported course
@@ -1810,7 +1765,8 @@ class ContentStoreTest(ModuleStoreTestCase):
# crate a new module and add it as a child to a vertical
module_store.create_and_save_xmodule(new_component_location)
parent = verticals[0]
- module_store.update_children(parent.location, parent.children + [new_component_location.url()])
+ parent.children.append(new_component_location.url())
+ module_store.update_item(parent, 'user')
# flush the cache
module_store.refresh_cached_metadata_inheritance_tree(new_component_location)
@@ -1827,8 +1783,7 @@ class ContentStoreTest(ModuleStoreTestCase):
# now let's define an override at the leaf node level
#
new_module.graceperiod = timedelta(1)
- new_module.save()
- module_store.update_metadata(new_module.location, own_metadata(new_module))
+ module_store.update_item(new_module, self.user.username)
# flush the cache and refetch
module_store.refresh_cached_metadata_inheritance_tree(new_component_location)
@@ -1942,10 +1897,7 @@ class MetadataSaveTestCase(ModuleStoreTestCase):
delattr(self.video_descriptor, field_name)
self.assertNotIn('html5_sources', own_metadata(self.video_descriptor))
- get_modulestore(location).update_metadata(
- location,
- own_metadata(self.video_descriptor)
- )
+ get_modulestore(location).update_item(self.video_descriptor, 'testuser')
module = get_modulestore(location).get_item(location)
self.assertNotIn('html5_sources', own_metadata(module))
@@ -2001,7 +1953,7 @@ def _course_factory_create_course():
Creates a course via the CourseFactory and returns the locator for it.
"""
course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
- return loc_mapper().translate_location(course.location.course_id, course.location, True, True)
+ return loc_mapper().translate_location(course.id, course.location, False, True)
def _get_course_id(test_course_data):
diff --git a/cms/djangoapps/contentstore/tests/test_course_updates.py b/cms/djangoapps/contentstore/tests/test_course_updates.py
index 5ee5f1289b..1a112f87e1 100644
--- a/cms/djangoapps/contentstore/tests/test_course_updates.py
+++ b/cms/djangoapps/contentstore/tests/test_course_updates.py
@@ -123,7 +123,7 @@ class CourseUpdateTest(CourseTestCase):
modulestore('direct').create_and_save_xmodule(location)
course_updates = modulestore('direct').get_item(location)
course_updates.data = 'bad news'
- modulestore('direct').update_item(location, course_updates.data)
+ modulestore('direct').update_item(course_updates, 'test_course_updates')
init_content = ''
diff --git a/cms/djangoapps/contentstore/tests/test_orphan.py b/cms/djangoapps/contentstore/tests/test_orphan.py
index e7babd25fd..3585d81e15 100644
--- a/cms/djangoapps/contentstore/tests/test_orphan.py
+++ b/cms/djangoapps/contentstore/tests/test_orphan.py
@@ -4,7 +4,6 @@ Test finding orphans via the view and django config
import json
from contentstore.tests.utils import CourseTestCase
from xmodule.modulestore.django import editable_modulestore, loc_mapper
-from django.core.urlresolvers import reverse
from student.models import CourseEnrollment
class TestOrphan(CourseTestCase):
@@ -35,7 +34,7 @@ class TestOrphan(CourseTestCase):
parent_location = self.course.location.replace(category=parent_category, name=parent_name)
parent = editable_modulestore('direct').get_item(parent_location)
parent.children.append(location.url())
- editable_modulestore('direct').update_children(parent_location, parent.children)
+ editable_modulestore('direct').update_item(parent, self.user.pk)
def test_mongo_orphan(self):
"""
diff --git a/cms/djangoapps/contentstore/tests/test_textbooks.py b/cms/djangoapps/contentstore/tests/test_textbooks.py
index d1fe606354..aab4ccc399 100644
--- a/cms/djangoapps/contentstore/tests/test_textbooks.py
+++ b/cms/djangoapps/contentstore/tests/test_textbooks.py
@@ -58,11 +58,8 @@ class TextbookIndexTestCase(CourseTestCase):
}
]
self.course.pdf_textbooks = content
- # Save the data that we've just changed to the underlying
- # MongoKeyValueStore before we update the mongo datastore.
- self.course.save()
store = get_modulestore(self.course.location)
- store.update_metadata(self.course.location, own_metadata(self.course))
+ store.update_item(self.course, self.user.pk)
resp = self.client.get(
self.url,
@@ -200,7 +197,7 @@ class TextbookDetailTestCase(CourseTestCase):
# MongoKeyValueStore before we update the mongo datastore.
self.course.save()
self.store = get_modulestore(self.course.location)
- self.store.update_metadata(self.course.location, own_metadata(self.course))
+ self.store.update_item(self.course, self.user.pk)
self.url_nonexist = self.course_locator.url_reverse("textbooks", "20")
def test_get_1(self):
diff --git a/cms/djangoapps/contentstore/tests/test_transcripts.py b/cms/djangoapps/contentstore/tests/test_transcripts.py
index f4c7f773ad..57e30ed9db 100644
--- a/cms/djangoapps/contentstore/tests/test_transcripts.py
+++ b/cms/djangoapps/contentstore/tests/test_transcripts.py
@@ -63,13 +63,13 @@ class Basetranscripts(CourseTestCase):
self.item_locator, self.item_location = self._get_locator(resp)
self.assertEqual(resp.status_code, 200)
+ self.item = modulestore().get_item(self.item_location)
# hI10vDNYz4M - valid Youtube ID with transcripts.
# JMD_ifUUfsU, AKqURZnYqpk, DYpADpL7jAY - valid Youtube IDs without transcripts.
- data = ''
- modulestore().update_item(self.item_location, data)
+ self.item.data = ''
+ modulestore().update_item(self.item, 'test_transcripts')
self.item = modulestore().get_item(self.item_location)
-
# Remove all transcripts for current module.
self.clear_subs_content()
@@ -130,14 +130,14 @@ class TestUploadtranscripts(Basetranscripts):
self.bad_name_srt_file.seek(0)
def test_success_video_module_source_subs_uploading(self):
- data = textwrap.dedent("""
+ self.item.data = textwrap.dedent("""
""")
- modulestore().update_item(self.item_location, data)
+ modulestore().update_item(self.item, 'test_transcripts')
link = reverse('upload_transcripts')
filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0]
@@ -212,8 +212,9 @@ class TestUploadtranscripts(Basetranscripts):
}
resp = self.client.ajax_post('/xblock', data)
item_locator, item_location = self._get_locator(resp)
- data = ''
- modulestore().update_item(item_location, data)
+ item = modulestore().get_item(item_location)
+ item.data = ''
+ modulestore().update_item(item, 'test_transcripts')
# non_video module: testing
@@ -232,8 +233,8 @@ class TestUploadtranscripts(Basetranscripts):
self.assertEqual(json.loads(resp.content).get('status'), 'Transcripts are supported only for "video" modules.')
def test_fail_bad_xml(self):
- data = '<<'
- modulestore().update_item(self.item_location, data)
+ self.item.data = '<<'
+ modulestore().update_item(self.item, 'test_transcripts')
link = reverse('upload_transcripts')
filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0]
@@ -344,8 +345,8 @@ class TestDownloadtranscripts(Basetranscripts):
pass
def test_success_download_youtube(self):
- data = ''
- modulestore().update_item(self.item_location, data)
+ self.item.data = ''
+ modulestore().update_item(self.item, 'test_transcripts')
subs = {
'start': [100, 200, 240],
@@ -365,14 +366,14 @@ class TestDownloadtranscripts(Basetranscripts):
def test_success_download_nonyoutube(self):
subs_id = str(uuid4())
- data = textwrap.dedent("""
+ self.item.data = textwrap.dedent("""
""".format(subs_id))
- modulestore().update_item(self.item_location, data)
+ modulestore().update_item(self.item, 'test_transcripts')
subs = {
'start': [100, 200, 240],
@@ -424,14 +425,15 @@ class TestDownloadtranscripts(Basetranscripts):
resp = self.client.ajax_post('/xblock', data)
item_locator, item_location = self._get_locator(resp)
subs_id = str(uuid4())
- data = textwrap.dedent("""
+ item = modulestore().get_item(item_location)
+ item.data = textwrap.dedent("""
""".format(subs_id))
- modulestore().update_item(item_location, data)
+ modulestore().update_item(item, 'test_transcripts')
subs = {
'start': [100, 200, 240],
@@ -449,28 +451,28 @@ class TestDownloadtranscripts(Basetranscripts):
self.assertEqual(resp.status_code, 404)
def test_fail_nonyoutube_subs_dont_exist(self):
- data = textwrap.dedent("""
+ self.item.data = textwrap.dedent("""
""")
- modulestore().update_item(self.item_location, data)
+ modulestore().update_item(self.item, 'test_transcripts')
link = reverse('download_transcripts')
resp = self.client.get(link, {'locator': self.item_locator})
self.assertEqual(resp.status_code, 404)
def test_empty_youtube_attr_and_sub_attr(self):
- data = textwrap.dedent("""
+ self.item.data = textwrap.dedent("""
""")
- modulestore().update_item(self.item_location, data)
+ modulestore().update_item(self.item, 'test_transcripts')
link = reverse('download_transcripts')
resp = self.client.get(link, {'locator': self.item_locator})
@@ -479,14 +481,14 @@ class TestDownloadtranscripts(Basetranscripts):
def test_fail_bad_sjson_subs(self):
subs_id = str(uuid4())
- data = textwrap.dedent("""
+ self.item.data = textwrap.dedent("""
""".format(subs_id))
- modulestore().update_item(self.item_location, data)
+ modulestore().update_item(self.item, 'test_transcripts')
subs = {
'start': [100, 200, 240],
@@ -532,14 +534,14 @@ class TestChecktranscripts(Basetranscripts):
def test_success_download_nonyoutube(self):
subs_id = str(uuid4())
- data = textwrap.dedent("""
+ self.item.data = textwrap.dedent("""
""".format(subs_id))
- modulestore().update_item(self.item_location, data)
+ modulestore().update_item(self.item, 'test_transcripts')
subs = {
'start': [100, 200, 240],
@@ -582,8 +584,8 @@ class TestChecktranscripts(Basetranscripts):
transcripts_utils.remove_subs_from_store(subs_id, self.item)
def test_check_youtube(self):
- data = ''
- modulestore().update_item(self.item_location, data)
+ self.item.data = ''
+ modulestore().update_item(self.item, 'test_transcripts')
subs = {
'start': [100, 200, 240],
@@ -674,14 +676,15 @@ class TestChecktranscripts(Basetranscripts):
resp = self.client.ajax_post('/xblock', data)
item_locator, item_location = self._get_locator(resp)
subs_id = str(uuid4())
- data = textwrap.dedent("""
+ item = modulestore().get_item(item_location)
+ item.data = textwrap.dedent("""
""".format(subs_id))
- modulestore().update_item(item_location, data)
+ modulestore().update_item(item, 'test_transcript')
subs = {
'start': [100, 200, 240],
diff --git a/cms/djangoapps/contentstore/transcripts_utils.py b/cms/djangoapps/contentstore/transcripts_utils.py
index 79024bc4fa..03aa46fcd5 100644
--- a/cms/djangoapps/contentstore/transcripts_utils.py
+++ b/cms/djangoapps/contentstore/transcripts_utils.py
@@ -286,7 +286,7 @@ def save_module(item):
"""
item.save()
store = get_modulestore(Location(item.id))
- store.update_metadata(item.id, own_metadata(item))
+ store.update_item(item, 'save_module')
def copy_or_rename_transcript(new_name, old_name, item, delete_old=False):
diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py
index fbe04186c4..29874583c4 100644
--- a/cms/djangoapps/contentstore/utils.py
+++ b/cms/djangoapps/contentstore/utils.py
@@ -222,16 +222,6 @@ def compute_unit_state(unit):
return UnitState.public
-def update_item(location, value):
- """
- If value is None, delete the db entry. Otherwise, update it using the correct modulestore.
- """
- if value is None:
- get_modulestore(location).delete_item(location)
- else:
- get_modulestore(location).update_item(location, value)
-
-
def add_extra_panel_tab(tab_type, course):
"""
Used to add the panel tab to a course if it does not exist.
diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py
index 123ff08f9a..2068d3ace6 100644
--- a/cms/djangoapps/contentstore/views/checklist.py
+++ b/cms/djangoapps/contentstore/views/checklist.py
@@ -51,8 +51,7 @@ def checklists_handler(request, tag=None, package_id=None, branch=None, version_
# from the template.
if not course_module.checklists:
course_module.checklists = CourseDescriptor.checklists.default
- course_module.save()
- modulestore.update_metadata(old_location, own_metadata(course_module))
+ modulestore.update_item(course_module, request.user.username)
expanded_checklists = expand_all_action_urls(course_module)
if json_request:
@@ -81,7 +80,7 @@ def checklists_handler(request, tag=None, package_id=None, branch=None, version_
# not default
course_module.checklists = course_module.checklists
course_module.save()
- modulestore.update_metadata(old_location, own_metadata(course_module))
+ modulestore.update_item(course_module, request.user.username)
expanded_checklist = expand_checklist_action_url(course_module, persisted_checklist)
return JsonResponse(expanded_checklist)
else:
diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py
index e930e33c1a..f06ed19501 100644
--- a/cms/djangoapps/contentstore/views/course.py
+++ b/cms/djangoapps/contentstore/views/course.py
@@ -163,8 +163,7 @@ def course_listing(request):
"""
List all courses available to the logged in user
"""
- # there's an index on category which will be used if none of its antecedents are set
- courses = modulestore('direct').get_items(Location(None, None, None, 'course', None))
+ courses = modulestore('direct').get_courses()
# filter out courses that we don't have access too
def course_filter(course):
@@ -743,10 +742,7 @@ def textbooks_list_handler(request, tag=None, package_id=None, branch=None, vers
if not any(tab['type'] == 'pdf_textbooks' for tab in course.tabs):
course.tabs.append({"type": "pdf_textbooks"})
course.pdf_textbooks = textbooks
- store.update_metadata(
- course.location,
- own_metadata(course)
- )
+ store.update_item(course, request.user.username)
return JsonResponse(course.pdf_textbooks)
elif request.method == 'POST':
# create a new textbook for the course
@@ -764,7 +760,7 @@ def textbooks_list_handler(request, tag=None, package_id=None, branch=None, vers
tabs = course.tabs
tabs.append({"type": "pdf_textbooks"})
course.tabs = tabs
- store.update_metadata(course.location, own_metadata(course))
+ store.update_item(course, request.user.username)
resp = JsonResponse(textbook, status=201)
resp["Location"] = locator.url_reverse('textbooks', textbook["id"])
return resp
@@ -815,10 +811,7 @@ def textbooks_detail_handler(request, tid, tag=None, package_id=None, branch=Non
course.pdf_textbooks = new_textbooks
else:
course.pdf_textbooks.append(new_textbook)
- store.update_metadata(
- course.location,
- own_metadata(course)
- )
+ store.update_item(course, request.user.username)
return JsonResponse(new_textbook, status=201)
elif request.method == 'DELETE':
if not textbook:
@@ -827,10 +820,7 @@ def textbooks_detail_handler(request, tid, tag=None, package_id=None, branch=Non
new_textbooks = course.pdf_textbooks[0:i]
new_textbooks.extend(course.pdf_textbooks[i + 1:])
course.pdf_textbooks = new_textbooks
- store.update_metadata(
- course.location,
- own_metadata(course)
- )
+ store.update_item(course, request.user.username)
return JsonResponse()
diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py
index 8d80d9e0bb..de81779749 100644
--- a/cms/djangoapps/contentstore/views/item.py
+++ b/cms/djangoapps/contentstore/views/item.py
@@ -232,7 +232,8 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta
modulestore().convert_to_draft(item_location)
if data:
- store.update_item(item_location, data)
+ # TODO Allow any scope.content fields not just "data" (exactly like the get below this)
+ existing_item.data = data
else:
data = existing_item.get_explicitly_set_fields_by_scope(Scope.content)
@@ -242,9 +243,9 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta
for child_locator
in children
]
- store.update_children(item_location, children_ids)
+ existing_item.children = children_ids
- # cdodge: also commit any metadata which might have been passed along
+ # also commit any metadata which might have been passed along
if nullout is not None or metadata is not None:
# the postback is not the complete metadata, as there's system metadata which is
# not presented to the end-user for editing. So let's use the original (existing_item) and
@@ -269,15 +270,12 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta
return JsonResponse({"error": "Invalid data"}, 400)
field.write_to(existing_item, value)
- # Save the data that we've just changed to the underlying
- # MongoKeyValueStore before we update the mongo datastore.
- existing_item.save()
- # commit to datastore
- store.update_metadata(item_location, own_metadata(existing_item))
-
if existing_item.category == 'video':
manage_video_subtitles_save(existing_item, existing_item)
+ # commit to datastore
+ store.update_item(existing_item, request.user)
+
result = {
'id': unicode(usage_loc),
'data': data,
@@ -339,7 +337,8 @@ def _create_item(request):
# TODO replace w/ nicer accessor
if not 'detached' in parent.runtime.load_block_type(category)._class_tags:
- get_modulestore(parent.location).update_children(parent_location, parent.children + [dest_location.url()])
+ parent.children.append(dest_location.url())
+ get_modulestore(parent.location).update_item(parent, request.user)
course_location = loc_mapper().translate_locator_to_location(parent_locator, get_course=True)
locator = loc_mapper().translate_location(course_location.course_id, dest_location, False, True)
@@ -373,13 +372,14 @@ def _duplicate_item(parent_location, duplicate_source_location, display_name=Non
system=source_item.runtime,
)
+ dest_module = get_modulestore(category).get_item(dest_location)
# Children are not automatically copied over (and not all xblocks have a 'children' attribute).
# Because DAGs are not fully supported, we need to actually duplicate each child as well.
if source_item.has_children:
- copied_children = []
+ dest_module.children = []
for child in source_item.children:
- copied_children.append(_duplicate_item(dest_location, Location(child)).url())
- get_modulestore(dest_location).update_children(dest_location, copied_children)
+ dest_module.children.append(_duplicate_item(dest_location, Location(child)).url())
+ get_modulestore(dest_location).update_item(dest_module, 'user')
if not 'detached' in source_item.runtime.load_block_type(category)._class_tags:
parent = get_modulestore(parent_location).get_item(parent_location)
@@ -390,7 +390,7 @@ def _duplicate_item(parent_location, duplicate_source_location, display_name=Non
parent.children.insert(source_index + 1, dest_location.url())
else:
parent.children.append(dest_location.url())
- get_modulestore(parent_location).update_children(parent_location, parent.children)
+ get_modulestore(parent_location).update_item(parent, 'user')
return dest_location
@@ -406,22 +406,19 @@ def _delete_item_at_location(item_location, delete_children=False, delete_all_ve
item = store.get_item(item_location)
if delete_children:
- _xmodule_recurse(item, lambda i: store.delete_item(i.location, delete_all_versions))
+ _xmodule_recurse(item, lambda i: store.delete_item(i.location, delete_all_versions=delete_all_versions))
else:
- store.delete_item(item.location, delete_all_versions)
+ store.delete_item(item.location, delete_all_versions=delete_all_versions)
# cdodge: we need to remove our parent's pointer to us so that it is no longer dangling
if delete_all_versions:
parent_locs = modulestore('direct').get_parent_locations(item_location, None)
+ item_url = item_location.url()
for parent_loc in parent_locs:
parent = modulestore('direct').get_item(parent_loc)
- item_url = item_location.url()
- if item_url in parent.children:
- children = parent.children
- children.remove(item_url)
- parent.children = children
- modulestore('direct').update_children(parent.location, parent.children)
+ parent.children.remove(item_url)
+ modulestore('direct').update_item(parent, 'user')
return JsonResponse()
@@ -452,7 +449,7 @@ def orphan_handler(request, tag=None, package_id=None, branch=None, version_guid
if request.user.is_staff:
items = modulestore().get_orphans(old_location, 'draft')
for item in items:
- modulestore('draft').delete_item(item, True)
+ modulestore('draft').delete_item(item, delete_all_versions=True)
return JsonResponse({'deleted': items})
else:
raise PermissionDenied()
diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py
index 4a34886ebc..f0c99ededb 100644
--- a/cms/djangoapps/contentstore/views/tabs.py
+++ b/cms/djangoapps/contentstore/views/tabs.py
@@ -47,7 +47,7 @@ def initialize_course_tabs(course):
{"type": "progress", "name": _("Progress")},
]
- modulestore('direct').update_metadata(course.location.url(), own_metadata(course))
+ modulestore('direct').update_item(course, 'system')
@expect_json
@login_required
@@ -123,7 +123,7 @@ def tabs_handler(request, tag=None, package_id=None, branch=None, version_guid=N
# OK, re-assemble the static tabs in the new order
course_item.tabs = reordered_tabs
- modulestore('direct').update_metadata(course_item.location, own_metadata(course_item))
+ modulestore('direct').update_item(course_item, request.user.username)
return JsonResponse()
else:
raise NotImplementedError('Creating or changing tab content is not supported.')
@@ -179,7 +179,7 @@ def primitive_delete(course, num):
# Note for future implementations: if you delete a static_tab, then Chris Dodge
# points out that there's other stuff to delete beyond this element.
# This code happens to not delete static_tab so it doesn't come up.
- modulestore('direct').update_metadata(course.location, own_metadata(course))
+ modulestore('direct').update_item(course, 'primitive delete')
def primitive_insert(course, num, tab_type, name):
@@ -188,5 +188,5 @@ def primitive_insert(course, num, tab_type, name):
new_tab = {u'type': unicode(tab_type), u'name': unicode(name)}
tabs = course.tabs
tabs.insert(num, new_tab)
- modulestore('direct').update_metadata(course.location, own_metadata(course))
+ modulestore('direct').update_item(course, 'primitive insert')
diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py
index dd8582ba76..dc842780ce 100644
--- a/cms/djangoapps/models/settings/course_details.py
+++ b/cms/djangoapps/models/settings/course_details.py
@@ -6,10 +6,8 @@ from json.encoder import JSONEncoder
from xmodule.modulestore import Location
from xmodule.modulestore.exceptions import ItemNotFoundError
-from xmodule.modulestore.inheritance import own_metadata
from contentstore.utils import get_modulestore, course_image_url
from models.settings import course_grading
-from contentstore.utils import update_item
from xmodule.fields import Date
from xmodule.modulestore.django import loc_mapper
@@ -74,6 +72,24 @@ class CourseDetails(object):
return course
+ @classmethod
+ def update_about_item(cls, course_old_location, about_key, data, course):
+ """
+ Update the about item with the new data blob. If data is None, then
+ delete the about item.
+ """
+ temploc = Location(course_old_location).replace(category='about', name=about_key)
+ store = get_modulestore(temploc)
+ if data is None:
+ store.delete_item(temploc)
+ else:
+ try:
+ about_item = store.get_item(temploc)
+ except ItemNotFoundError:
+ about_item = store.create_xmodule(temploc, system=course.runtime)
+ about_item.data = data
+ store.update_item(about_item, 'update_about_item')
+
@classmethod
def update_from_json(cls, course_locator, jsondict):
"""
@@ -130,26 +146,15 @@ class CourseDetails(object):
dirty = True
if dirty:
- # Save the data that we've just changed to the underlying
- # MongoKeyValueStore before we update the mongo datastore.
- descriptor.save()
-
- get_modulestore(course_old_location).update_metadata(course_old_location, own_metadata(descriptor))
+ get_modulestore(course_old_location).update_item(descriptor, 'course_details')
# NOTE: below auto writes to the db w/o verifying that any of the fields actually changed
# to make faster, could compare against db or could have client send over a list of which fields changed.
- temploc = Location(course_old_location).replace(category='about', name='syllabus')
- update_item(temploc, jsondict['syllabus'])
+ for about_type in ['syllabus', 'overview', 'effort']:
+ cls.update_about_item(course_old_location, about_type, jsondict[about_type], descriptor)
- temploc = temploc.replace(name='overview')
- update_item(temploc, jsondict['overview'])
-
- temploc = temploc.replace(name='effort')
- update_item(temploc, jsondict['effort'])
-
- temploc = temploc.replace(name='video')
recomposed_video_tag = CourseDetails.recompose_video_tag(jsondict['intro_video'])
- update_item(temploc, recomposed_video_tag)
+ cls.update_about_item(course_old_location, 'video', recomposed_video_tag, descriptor)
# Could just return jsondict w/o doing any db reads, but I put the reads in as a means to confirm
# it persisted correctly
diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py
index 519323298b..5123b52f8a 100644
--- a/cms/djangoapps/models/settings/course_grading.py
+++ b/cms/djangoapps/models/settings/course_grading.py
@@ -65,9 +65,7 @@ class CourseGradingModel(object):
descriptor.raw_grader = graders_parsed
descriptor.grade_cutoffs = jsondict['grade_cutoffs']
- get_modulestore(course_old_location).update_item(
- course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content)
- )
+ get_modulestore(course_old_location).update_item(descriptor, 'course_grading')
CourseGradingModel.update_grace_period_from_json(course_locator, jsondict['grace_period'])
@@ -91,9 +89,7 @@ class CourseGradingModel(object):
else:
descriptor.raw_grader.append(grader)
- get_modulestore(course_old_location).update_item(
- course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content)
- )
+ get_modulestore(course_old_location).update_item(descriptor, 'course_grading')
return CourseGradingModel.jsonize_grader(index, descriptor.raw_grader[index])
@@ -107,9 +103,7 @@ class CourseGradingModel(object):
descriptor = get_modulestore(course_old_location).get_item(course_old_location)
descriptor.grade_cutoffs = cutoffs
- get_modulestore(course_old_location).update_item(
- course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content)
- )
+ get_modulestore(course_old_location).update_item(descriptor, 'course_grading')
return cutoffs
@@ -132,9 +126,7 @@ class CourseGradingModel(object):
grace_timedelta = timedelta(**graceperiodjson)
descriptor.graceperiod = grace_timedelta
- get_modulestore(course_old_location).update_metadata(
- course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.settings)
- )
+ get_modulestore(course_old_location).update_item(descriptor, 'update_grace_period')
@staticmethod
def delete_grader(course_location, index):
@@ -150,9 +142,7 @@ class CourseGradingModel(object):
# force propagation to definition
descriptor.raw_grader = descriptor.raw_grader
- get_modulestore(course_old_location).update_item(
- course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content)
- )
+ get_modulestore(course_old_location).update_item(descriptor, 'delete_grader')
@staticmethod
def delete_grace_period(course_location):
@@ -164,9 +154,7 @@ class CourseGradingModel(object):
del descriptor.graceperiod
- get_modulestore(course_old_location).update_metadata(
- course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.settings)
- )
+ get_modulestore(course_old_location).update_item(descriptor, 'delete_grace_period')
@staticmethod
def get_section_grader_type(location):
@@ -186,9 +174,7 @@ class CourseGradingModel(object):
del descriptor.format
del descriptor.graded
- get_modulestore(descriptor.location).update_metadata(
- descriptor.location, descriptor.get_explicitly_set_fields_by_scope(Scope.settings)
- )
+ get_modulestore(descriptor.location).update_item(descriptor, 'update_grader')
return {'graderType': grader_type}
@staticmethod
diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py
index 3c561b6047..88aa3c04e9 100644
--- a/cms/djangoapps/models/settings/course_metadata.py
+++ b/cms/djangoapps/models/settings/course_metadata.py
@@ -1,7 +1,6 @@
from xblock.fields import Scope
from contentstore.utils import get_modulestore
-from xmodule.modulestore.inheritance import own_metadata
from cms.lib.xblock.mixin import CmsBlockMixin
@@ -78,6 +77,6 @@ class CourseMetadata(object):
setattr(descriptor, key, value)
if dirty:
- get_modulestore(descriptor.location).update_metadata(descriptor.location, own_metadata(descriptor))
+ get_modulestore(descriptor.location).update_item(descriptor, 'update_settings')
return cls.fetch(descriptor)
diff --git a/common/djangoapps/external_auth/tests/test_shib.py b/common/djangoapps/external_auth/tests/test_shib.py
index 0ef912464b..6397685c11 100644
--- a/common/djangoapps/external_auth/tests/test_shib.py
+++ b/common/djangoapps/external_auth/tests/test_shib.py
@@ -330,9 +330,7 @@ class ShibSPTest(ModuleStoreTestCase):
for domain in ["", "shib:https://idp.stanford.edu/"]:
# set domains
course.enrollment_domain = domain
- metadata = own_metadata(course)
- metadata['enrollment_domain'] = domain
- self.store.update_metadata(course.location.url(), metadata)
+ self.store.update_item(course, 'test_shib')
# setting location to test that GET params get passed through
login_request = self.request_factory.get('/course_specific_login/MITx/999/Robot_Super_Course' +
@@ -401,15 +399,11 @@ class ShibSPTest(ModuleStoreTestCase):
# create 2 course, one with limited enrollment one without
shib_course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only')
shib_course.enrollment_domain = 'shib:https://idp.stanford.edu/'
- metadata = own_metadata(shib_course)
- metadata['enrollment_domain'] = shib_course.enrollment_domain
- self.store.update_metadata(shib_course.location.url(), metadata)
+ self.store.update_item(shib_course, 'test_enroll_limit')
open_enroll_course = CourseFactory.create(org='MITx', number='999', display_name='Robot Super Course')
open_enroll_course.enrollment_domain = ''
- metadata = own_metadata(open_enroll_course)
- metadata['enrollment_domain'] = open_enroll_course.enrollment_domain
- self.store.update_metadata(open_enroll_course.location.url(), metadata)
+ self.store.update_item(open_enroll_course, 'test_enroll_limit')
# create 3 kinds of students, external_auth matching shib_course, external_auth not matching, no external auth
shib_student = UserFactory.create()
@@ -475,9 +469,7 @@ class ShibSPTest(ModuleStoreTestCase):
course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only')
course.enrollment_domain = 'shib:https://idp.stanford.edu/'
- metadata = own_metadata(course)
- metadata['enrollment_domain'] = course.enrollment_domain
- self.store.update_metadata(course.location.url(), metadata)
+ self.store.update_item(course, 'test_shib')
# use django test client for sessions and url processing
# no enrollment before trying
diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py
index 9be1e7de04..b50d80d1ad 100644
--- a/common/djangoapps/student/tests/test_login.py
+++ b/common/djangoapps/student/tests/test_login.py
@@ -202,9 +202,7 @@ class ExternalAuthShibTest(ModuleStoreTestCase):
self.course = CourseFactory.create(org='Stanford', number='456', display_name='NO SHIB')
self.shib_course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only')
self.shib_course.enrollment_domain = 'shib:https://idp.stanford.edu/'
- metadata = own_metadata(self.shib_course)
- metadata['enrollment_domain'] = self.shib_course.enrollment_domain
- self.store.update_metadata(self.shib_course.location.url(), metadata)
+ self.store.update_item(self.shib_course, 'test_shib')
self.user_w_map = UserFactory.create(email='withmap@stanford.edu')
self.extauth = ExternalAuthMap(external_id='withmap@stanford.edu',
external_email='withmap@stanford.edu',
diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py
index 5d241731a7..f764ce6d96 100644
--- a/common/lib/xmodule/xmodule/modulestore/__init__.py
+++ b/common/lib/xmodule/xmodule/modulestore/__init__.py
@@ -339,7 +339,7 @@ class ModuleStoreRead(object):
pass
@abstractmethod
- def get_items(self, location, course_id=None, depth=0):
+ def get_items(self, location, course_id=None, depth=0, qualifiers=None):
"""
Returns a list of XModuleDescriptor instances for the items
that match location. Any element of location that is None is treated
@@ -378,6 +378,15 @@ class ModuleStoreRead(object):
'''
pass
+ @abstractmethod
+ def get_orphans(self, course_location, branch):
+ """
+ Get all of the xblocks in the given course which have no parents and are not of types which are
+ usually orphaned. NOTE: may include xblocks which still have references via xblocks which don't
+ use children to point to their dependents.
+ """
+ pass
+
@abstractmethod
def get_errored_courses(self):
"""
@@ -404,44 +413,16 @@ class ModuleStoreWrite(ModuleStoreRead):
__metaclass__ = ABCMeta
@abstractmethod
- def update_item(self, location, data, allow_not_found=False):
+ def update_item(self, xblock, user, allow_not_found=False):
"""
- Set the data in the item specified by the location to
- data
-
- location: Something that can be passed to Location
- data: A nested dictionary of problem data
+ Update the given xblock's persisted repr
"""
pass
@abstractmethod
- def update_children(self, location, children):
+ def delete_item(self, location, user_id=None, delete_all_versions=False, delete_children=False, force=False):
"""
- Set the children for the item specified by the location to
- children
-
- location: Something that can be passed to Location
- children: A list of child item identifiers
- """
- pass
-
- @abstractmethod
- def update_metadata(self, location, metadata):
- """
- Set the metadata for the item specified by the location to
- metadata
-
- location: Something that can be passed to Location
- metadata: A nested dictionary of module metadata
- """
- pass
-
- @abstractmethod
- def delete_item(self, location):
- """
- Delete an item from this modulestore
-
- location: Something that can be passed to Location
+ Delete an item from persistence
"""
pass
diff --git a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py
index 37853d0789..9ada7fe970 100644
--- a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py
+++ b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py
@@ -163,8 +163,14 @@ class LocMapperStore(object):
else:
raise ItemNotFoundError(location)
elif isinstance(block_id, dict):
- # name is not unique, look through for the right category
- if location.category in block_id:
+ # jump_to_id uses a None category.
+ if location.category is None:
+ if len(block_id) == 1:
+ # unique match (most common case)
+ block_id = block_id.values()[0]
+ else:
+ raise InvalidLocationError()
+ elif location.category in block_id:
block_id = block_id[location.category]
elif add_entry_if_missing:
block_id = self._add_to_block_map(location, location_id, entry['block_map'])
diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py
index 2b58797532..01c7d4155c 100644
--- a/common/lib/xmodule/xmodule/modulestore/locator.py
+++ b/common/lib/xmodule/xmodule/modulestore/locator.py
@@ -488,6 +488,19 @@ class BlockUsageLocator(CourseLocator):
raise ValueError('Could not parse "%s" as a package_id' % package_id)
self._set_value(parse, 'block', self.set_block_id)
+ @classmethod
+ def make_relative(cls, course_locator, block_id):
+ """
+ Return a new instance which has the given block_id in the given course
+ :param course_locator: may be a BlockUsageLocator in the same snapshot
+ """
+ return BlockUsageLocator(
+ package_id=course_locator.package_id,
+ version_guid=course_locator.version_guid,
+ branch=course_locator.branch,
+ block_id=block_id
+ )
+
def __unicode__(self):
"""
Return a string representing this location.
diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py
index 46da67cb65..009c8c95f7 100644
--- a/common/lib/xmodule/xmodule/modulestore/mixed.py
+++ b/common/lib/xmodule/xmodule/modulestore/mixed.py
@@ -3,29 +3,42 @@ MixedModuleStore allows for aggregation between multiple modulestores.
In this way, courses can be served up both - say - XMLModuleStore or MongoModuleStore
-IMPORTANT: This modulestore only supports READONLY applications, e.g. LMS
"""
from . import ModuleStoreWriteBase
-from xmodule.modulestore.django import create_modulestore_instance
+from xmodule.modulestore.django import create_modulestore_instance, loc_mapper
import logging
+from xmodule.modulestore import Location
+from xblock.fields import Reference, ReferenceList, String
+from xmodule.modulestore.locator import CourseLocator, Locator, BlockUsageLocator
+from xmodule.modulestore.exceptions import InsufficientSpecificationError, ItemNotFoundError
+from xmodule.modulestore.parsers import ALLOWED_ID_CHARS
+import re
log = logging.getLogger(__name__)
class MixedModuleStore(ModuleStoreWriteBase):
"""
- ModuleStore that can be backed by either XML or Mongo
+ ModuleStore knows how to route requests to the right persistence ms and how to convert any
+ references in the xblocks to the type required by the app and the persistence layer.
"""
- def __init__(self, mappings, stores, **kwargs):
+ def __init__(self, mappings, stores, reference_type=None, **kwargs):
"""
Initialize a MixedModuleStore. Here we look into our passed in kwargs which should be a
collection of other modulestore configuration informations
+ :param reference_type: either Location or Locator to indicate what type of reference this app
+ uses.
"""
super(MixedModuleStore, self).__init__(**kwargs)
self.modulestores = {}
self.mappings = mappings
+ # temporary code for transition period
+ if reference_type is None:
+ log.warn("reference_type not specified in MixedModuleStore settings.",
+ "Will default temporarily to the to-be-deprecated Location.")
+ self.use_locations = (reference_type != 'Locator')
if 'default' not in stores:
raise Exception('Missing a default modulestore in the MixedModuleStore __init__ method.')
@@ -45,8 +58,136 @@ class MixedModuleStore(ModuleStoreWriteBase):
mapping = self.mappings.get(course_id, 'default')
return self.modulestores[mapping]
- def has_item(self, course_id, location):
- return self._get_modulestore_for_courseid(course_id).has_item(course_id, location)
+ def _incoming_reference_adaptor(self, store, course_id, reference):
+ """
+ Convert the reference to the type the persistence layer wants
+ """
+ if issubclass(store.reference_type, Location if self.use_locations else Locator):
+ return reference
+ stringify = isinstance(reference, basestring)
+ if store.reference_type == Location:
+ if stringify:
+ reference = BlockUsageLocator(url=reference)
+ location = loc_mapper().translate_locator_to_location(reference)
+ return location.url() if stringify else location
+ if stringify:
+ reference = Location(reference)
+ locator = loc_mapper().translate_location(course_id, reference, reference.revision == 'draft', True)
+ return unicode(locator) if stringify else locator
+
+ def _outgoing_reference_adaptor(self, store, course_id, reference):
+ """
+ Convert the reference to the type the application wants
+ """
+ if issubclass(store.reference_type, Location if self.use_locations else Locator):
+ return reference
+ stringify = isinstance(reference, basestring)
+ if store.reference_type == Location:
+ if stringify:
+ reference = Location(reference)
+ locator = loc_mapper().translate_location(
+ course_id, reference, reference.revision == 'draft', True
+ )
+ return unicode(locator) if stringify else locator
+ else:
+ if stringify:
+ reference = BlockUsageLocator(url=reference)
+ location = loc_mapper().translate_locator_to_location(reference)
+ return location.url() if stringify else location
+
+ def _incoming_xblock_adaptor(self, store, course_id, xblock):
+ """
+ Change all reference fields in this xblock to the type expected by the persistence layer
+ """
+ string_converter = self._get_string_converter(
+ course_id, store.reference_type, xblock.location
+ )
+ for field in xblock.fields.itervalues():
+ if field.is_set_on(xblock):
+ if isinstance(field, Reference):
+ field.write_to(
+ xblock,
+ self._incoming_reference_adaptor(store, course_id, field.read_from(xblock)))
+ elif isinstance(field, ReferenceList):
+ field.write_to(
+ xblock,
+ [self._incoming_reference_adaptor(store, course_id, ref)
+ for ref in field.read_from(xblock)]
+ )
+ elif string_converter is not None and isinstance(field, String):
+ # replace links within the string
+ string_converter(field, xblock)
+ return xblock
+
+ def _outgoing_xblock_adaptor(self, store, course_id, xblock):
+ """
+ Change all reference fields in this xblock to the type expected by the persistence layer
+ """
+ string_converter = self._get_string_converter(
+ course_id, xblock.location.__class__, xblock.location
+ )
+ for field in xblock.fields.itervalues():
+ if field.is_set_on(xblock):
+ if isinstance(field, Reference):
+ field.write_to(
+ xblock,
+ self._outgoing_reference_adaptor(store, course_id, field.read_from(xblock)))
+ elif isinstance(field, ReferenceList):
+ field.write_to(
+ xblock,
+ [self._outgoing_reference_adaptor(store, course_id, ref)
+ for ref in field.read_from(xblock)]
+ )
+ elif string_converter is not None and isinstance(field, String):
+ # replace links within the string
+ string_converter(field, xblock)
+ return xblock
+
+ CONVERT_RE = re.compile(r"/jump_to_id/({}+)".format(ALLOWED_ID_CHARS))
+
+ def _get_string_converter(self, course_id, reference_type, from_base_addr):
+ """
+ Return a closure which finds and replaces all embedded links in a string field
+ with the correct rewritten link for the target type
+ """
+ if self.use_locations and reference_type == Location:
+ return None
+ if not self.use_locations and issubclass(reference_type, Locator):
+ return None
+ if isinstance(from_base_addr, Location):
+ def mapper(found_id):
+ """
+ Convert the found id to BlockUsageLocator block_id
+ """
+ location = from_base_addr.replace(category=None, name=found_id)
+ # NOTE without category, it cannot create a new mapping if there's not one already
+ return loc_mapper().translate_location(course_id, location).block_id
+ else:
+ def mapper(found_id):
+ """
+ Convert the found id to Location block_id
+ """
+ locator = BlockUsageLocator.make_relative(from_base_addr, found_id)
+ return loc_mapper().translate_locator_to_location(locator).name
+
+ def converter(field, xblock):
+ """
+ Find all of the ids in the block and replace them w/ their mapped values
+ """
+ value = field.read_from(xblock)
+ self.CONVERT_RE.sub(mapper, value)
+ field.write_to(xblock, value)
+ return converter
+
+ def has_item(self, course_id, reference):
+ """
+ Does the course include the xblock who's id is reference?
+ :param course_id: a course_id or package_id (slashed or dotted)
+ :param reference: a Location or BlockUsageLocator
+ """
+ store = self._get_modulestore_for_courseid(course_id)
+ decoded_ref = self._incoming_reference_adaptor(store, course_id, reference)
+ return store.has_item(course_id, decoded_ref)
def get_item(self, location, depth=0):
"""
@@ -56,49 +197,62 @@ class MixedModuleStore(ModuleStoreWriteBase):
raise NotImplementedError
def get_instance(self, course_id, location, depth=0):
- return self._get_modulestore_for_courseid(course_id).get_instance(course_id, location, depth)
+ store = self._get_modulestore_for_courseid(course_id)
+ decoded_ref = self._incoming_reference_adaptor(store, course_id, location)
+ xblock = store.get_instance(course_id, decoded_ref, depth)
+ return self._outgoing_xblock_adaptor(store, course_id, xblock)
- def get_items(self, location, course_id=None, depth=0):
+ def get_items(self, location, course_id=None, depth=0, qualifiers=None):
"""
Returns a list of XModuleDescriptor instances for the items
that match location. Any element of location that is None is treated
- as a wildcard that matches any value
+ as a wildcard that matches any value. NOTE: don't use this to look for courses
+ as the course_id is required. Use get_courses.
- location: Something that can be passed to Location
+ location: either a Location possibly w/ None as wildcards for category or name or
+ a Locator with at least a package_id and branch but possibly no block_id.
depth: An argument that some module stores may use to prefetch
descendents of the queried modules for more efficient results later
in the request. The depth is counted in the number of calls to
get_children() to cache. None indicates to cache all descendents
"""
- if not course_id:
- raise Exception("Must pass in a course_id when calling get_items() with MixedModuleStore")
+ if not (course_id or hasattr(location, 'package_id')):
+ raise Exception("Must pass in a course_id when calling get_items()")
- return self._get_modulestore_for_courseid(course_id).get_items(location, course_id, depth)
-
- def update_item(self, location, data, allow_not_found=False):
- """
- MixedModuleStore is for read-only (aka LMS)
- """
- raise NotImplementedError
-
- def update_children(self, location, children):
- """
- MixedModuleStore is for read-only (aka LMS)
- """
- raise NotImplementedError
-
- def update_metadata(self, location, metadata):
- """
- MixedModuleStore is for read-only (aka LMS)
- """
- raise NotImplementedError
-
- def delete_item(self, location):
- """
- MixedModuleStore is for read-only (aka LMS)
- """
- raise NotImplementedError
+ store = self._get_modulestore_for_courseid(course_id or getattr(location, 'package_id'))
+ # translate won't work w/ missing fields so work around it
+ if store.reference_type == Location:
+ if not self.use_locations:
+ if getattr(location, 'block_id', False):
+ location = self._incoming_reference_adaptor(store, course_id, location)
+ else:
+ # get the course's location
+ location = loc_mapper().translate_locator_to_location(location, get_course=True)
+ # now remove the unknowns
+ location = location.replace(
+ category=qualifiers.get('category', None),
+ name=None
+ )
+ else:
+ if self.use_locations:
+ if not isinstance(location, Location):
+ location = Location(location)
+ try:
+ location.ensure_fully_specified()
+ location = loc_mapper().translate_location(
+ course_id, location, location.revision == 'published', True
+ )
+ except InsufficientSpecificationError:
+ # construct the Locator by hand
+ if location.category is not None and qualifiers.get('category', False):
+ qualifiers['category'] = location.category
+ location = loc_mapper().translate_location_to_course_locator(
+ course_id, location, location.revision == 'published'
+ )
+ xblocks = store.get_items(location, course_id, depth, qualifiers)
+ xblocks = [self._outgoing_xblock_adaptor(store, course_id, xblock) for xblock in xblocks]
+ return xblocks
def get_courses(self):
'''
@@ -126,15 +280,49 @@ class MixedModuleStore(ModuleStoreWriteBase):
def get_course(self, course_id):
"""
- returns the course module associated with the course_id
+ returns the course module associated with the course_id. If no such course exists,
+ it returns None
+ :param course_id: must be either a string course_id or a CourseLocator
"""
- return self._get_modulestore_for_courseid(course_id).get_course(course_id)
+ store = self._get_modulestore_for_courseid(
+ course_id.package_id if hasattr(course_id, 'package_id') else course_id)
+ try:
+ # translate won't work w/ missing fields so work around it
+ if store.reference_type == Location:
+ # takes the course_id: figure out if this is old or new style
+ if not self.use_locations:
+ if isinstance(course_id, basestring):
+ course_id = CourseLocator(package_id=course_id, branch='published')
+ course_location = loc_mapper().translate_locator_to_location(course_id, get_course=True)
+ course_id = course_location.course_id
+ xblock = store.get_course(course_id)
+ else:
+ # takes a courseLocator
+ if isinstance(course_id, CourseLocator):
+ location = course_id
+ course_id = None # not an old style course_id; so, don't use it further
+ elif '/' in course_id:
+ location = loc_mapper().translate_location_to_course_locator(course_id, None, True)
+ else:
+ location = CourseLocator(package_id=course_id, branch='published')
+ course_id = None # not an old style course_id; so, don't use it further
+ xblock = store.get_course(location)
+ except ItemNotFoundError:
+ return None
+ if xblock is not None:
+ return self._outgoing_xblock_adaptor(store, course_id, xblock)
+ else:
+ return None
def get_parent_locations(self, location, course_id):
"""
- returns the parent locations for a given lcoation and course_id
+ returns the parent locations for a given location and course_id
"""
- return self._get_modulestore_for_courseid(course_id).get_parent_locations(location, course_id)
+ store = self._get_modulestore_for_courseid(course_id)
+ decoded_ref = self._incoming_reference_adaptor(store, course_id, location)
+ parents = store.get_parent_locations(decoded_ref, course_id)
+ return [self._outgoing_reference_adaptor(store, course_id, reference)
+ for reference in parents]
def get_modulestore_type(self, course_id):
"""
@@ -146,6 +334,17 @@ class MixedModuleStore(ModuleStoreWriteBase):
"""
return self._get_modulestore_for_courseid(course_id).get_modulestore_type(course_id)
+ def get_orphans(self, course_location, branch):
+ """
+ Get all of the xblocks in the given course which have no parents and are not of types which are
+ usually orphaned. NOTE: may include xblocks which still have references via xblocks which don't
+ use children to point to their dependents.
+ """
+ course_id = getattr(course_location, 'course_id', getattr(course_location, 'package_id', None))
+ store = self._get_modulestore_for_courseid(course_id)
+ decoded_ref = self._incoming_reference_adaptor(store, course_id, course_location)
+ return store.get_orphans(decoded_ref, branch)
+
def get_errored_courses(self):
"""
Return a dictionary of course_dir -> [(msg, exception_str)], for each
@@ -155,3 +354,32 @@ class MixedModuleStore(ModuleStoreWriteBase):
for store in self.modulestores.values():
errs.update(store.get_errored_courses())
return errs
+
+ def update_item(self, xblock, user_id, allow_not_found=False):
+ """
+ Update the xblock persisted to be the same as the given for all types of fields
+ (content, children, and metadata) attribute the change to the given user.
+ """
+ if self.use_locations:
+ raise NotImplementedError
+
+ locator = xblock.location
+ course_id = locator.package_id
+ store = self._get_modulestore_for_courseid(course_id)
+
+ # if an xblock, convert its contents to correct addr scheme
+ xblock = self._incoming_xblock_adaptor(store, course_id, xblock)
+ xblock = store.update_item(xblock, user_id)
+
+ return self._outgoing_xblock_adaptor(store, course_id, xblock)
+
+ def delete_item(self, location, **kwargs):
+ """
+ Delete the given item from persistence.
+ """
+ if self.use_locations:
+ raise NotImplementedError
+
+ store = self._get_modulestore_for_courseid(location.package_id)
+ decoded_ref = self._incoming_reference_adaptor(store, location.package_id, location)
+ return store.delete_item(decoded_ref, **kwargs)
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
index 9870a1d3d8..7c77956aed 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
@@ -38,10 +38,6 @@ from xblock.core import XBlock
log = logging.getLogger(__name__)
-# TODO (cpennington): This code currently operates under the assumption that
-# there is only one revision for each item. Once we start versioning inside the CMS,
-# that assumption will have to change
-
def get_course_id_no_run(location):
'''
@@ -224,8 +220,10 @@ def namedtuple_to_son(namedtuple, prefix=''):
Converts a namedtuple into a SON object with the same key order
"""
son = SON()
+ # pylint: disable=protected-access
for idx, field_name in enumerate(namedtuple._fields):
son[prefix + field_name] = namedtuple[idx]
+ # pylint: enable=protected-access
return son
@@ -299,9 +297,11 @@ class MongoModuleStore(ModuleStoreWriteBase):
# Force mongo to maintain an index over _id.* that is in the same order
# that is used when querying by a location
+ # pylint: disable=no-member, protected_access
self.collection.ensure_index(
zip(('_id.' + field for field in Location._fields), repeat(1)),
)
+ # pylint: enable=no-member, protected_access
if default_class is not None:
module_path, _, class_name = default_class.rpartition('.')
@@ -313,6 +313,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
self.error_tracker = error_tracker
self.render_template = render_template
self.ignore_write_events_on_courses = []
+ self.reference_type = Location
def compute_metadata_inheritance_tree(self, location):
'''
@@ -363,11 +364,6 @@ class MongoModuleStore(ModuleStoreWriteBase):
"""
Helper method for computing inherited metadata for a specific location url
"""
- # check for presence of metadata key. Note that a given module may not yet be fully formed.
- # example: update_item -> update_children -> update_metadata sequence on new item create
- # if we get called here without update_metadata called first then 'metadata' hasn't been set
- # as we're not fully transactional at the DB layer. Same comment applies to below key name
- # check
my_metadata = results_by_url[url].get('metadata', {})
# go through all the children and recurse, but only if we have
@@ -443,6 +439,9 @@ class MongoModuleStore(ModuleStoreWriteBase):
del item['_id']
def _query_children_for_cache_children(self, items):
+ """
+ Generate a pymongo in query for finding the items and return the payloads
+ """
# first get non-draft in a round-trip
query = {
'_id': {'$in': [namedtuple_to_son(Location(item)) for item in items]}
@@ -531,8 +530,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
'''
Returns a list of course descriptors.
'''
- # TODO (vshnayder): Why do I have to specify i4x here?
- course_filter = Location("i4x", category="course")
+ course_filter = Location(category="course")
return [
course
for course
@@ -556,6 +554,16 @@ class MongoModuleStore(ModuleStoreWriteBase):
raise ItemNotFoundError(location)
return item
+ def get_course(self, course_id):
+ """
+ Get the course with the given courseid (org/course/run)
+ """
+ id_components = course_id.split('/')
+ try:
+ return self.get_item(Location('i4x', id_components[0], id_components[1], 'course', id_components[2]))
+ except ItemNotFoundError:
+ return None
+
def has_item(self, course_id, location):
"""
Returns True if location exists in this ModuleStore.
@@ -599,7 +607,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
"""
return self.get_item(location, depth=depth)
- def get_items(self, location, course_id=None, depth=0):
+ def get_items(self, location, course_id=None, depth=0, qualifiers=None):
items = self.collection.find(
location_to_query(location),
sort=[('revision', pymongo.ASCENDING)],
@@ -664,13 +672,13 @@ class MongoModuleStore(ModuleStoreWriteBase):
# Save any changes to the xmodule to the MongoKeyValueStore
xmodule.save()
self.collection.save({
- '_id': xmodule.location.dict(),
- 'metadata': own_metadata(xmodule),
- 'definition': {
- 'data': xmodule.get_explicitly_set_fields_by_scope(Scope.content),
- 'children': xmodule.children if xmodule.has_children else []
- }
- })
+ '_id': namedtuple_to_son(xmodule.location),
+ 'metadata': own_metadata(xmodule),
+ 'definition': {
+ 'data': xmodule.get_explicitly_set_fields_by_scope(Scope.content),
+ 'children': xmodule.children if xmodule.has_children else []
+ }
+ })
# recompute (and update) the metadata inheritance tree which is cached
self.refresh_cached_metadata_inheritance_tree(xmodule.location)
self.fire_updated_modulestore_signal(get_course_id_no_run(xmodule.location), xmodule.location)
@@ -708,7 +716,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
course.tabs = existing_tabs
# Save any changes to the course to the MongoKeyValueStore
course.save()
- self.update_metadata(course.location, course.get_explicitly_set_fields_by_scope(Scope.settings))
+ self.update_item(course, 'create_and_save_xmodule')
def fire_updated_modulestore_signal(self, course_id, location):
"""
@@ -754,7 +762,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
# See http://www.mongodb.org/display/DOCS/Updating for
# atomic update syntax
result = self.collection.update(
- {'_id': Location(location).dict()},
+ {'_id': namedtuple_to_son(Location(location))},
{'$set': update},
multi=False,
upsert=True,
@@ -765,73 +773,56 @@ class MongoModuleStore(ModuleStoreWriteBase):
if result['n'] == 0:
raise ItemNotFoundError(location)
- def update_item(self, location, data, allow_not_found=False):
+ def update_item(self, xblock, user, allow_not_found=False):
"""
- Set the data in the item specified by the location to
- data
+ Update the persisted version of xblock to reflect its current values.
location: Something that can be passed to Location
data: A nested dictionary of problem data
"""
try:
- self._update_single_item(location, {'definition.data': data})
+ definition_data = xblock.get_explicitly_set_fields_by_scope()
+ if len(definition_data) == 1 and 'data' in definition_data:
+ definition_data = definition_data['data']
+ payload = {
+ 'definition.data': definition_data,
+ 'metadata': own_metadata(xblock),
+ }
+ if xblock.has_children:
+ # convert all to urls
+ xblock.children = [child.url() if isinstance(child, Location) else child
+ for child in xblock.children]
+ payload.update({'definition.children': xblock.children})
+ self._update_single_item(xblock.location, payload)
+ if xblock.category == 'static_tab':
+ course = self._get_course_for_item(xblock.location)
+ existing_tabs = course.tabs or []
+ for tab in existing_tabs:
+ if tab.get('url_slug') == xblock.location.name:
+ if xblock.fields['display_name'].is_set_on(xblock):
+ tab['name'] = xblock.display_name
+ break
+ course.tabs = existing_tabs
+ # Save the updates to the course to the MongoKeyValueStore
+ self.update_item(course, user)
+
+ # recompute (and update) the metadata inheritance tree which is cached
+ # was conditional on children or metadata having changed before dhm made one update to rule them all
+ self.refresh_cached_metadata_inheritance_tree(xblock.location)
+ # fire signal that we've written to DB
+ self.fire_updated_modulestore_signal(get_course_id_no_run(xblock.location), xblock.location)
except ItemNotFoundError:
if not allow_not_found:
raise
- def update_children(self, location, children):
- """
- Set the children for the item specified by the location to
- children
-
- location: Something that can be passed to Location
- children: A list of child item identifiers
- """
- # Normalize the children to urls
- children = [Location(child).url() for child in children]
-
- self._update_single_item(location, {'definition.children': children})
- # recompute (and update) the metadata inheritance tree which is cached
- self.refresh_cached_metadata_inheritance_tree(Location(location))
- # fire signal that we've written to DB
- self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location))
-
- def update_metadata(self, location, metadata):
- """
- Set the metadata for the item specified by the location to
- metadata
-
- location: Something that can be passed to Location
- metadata: A nested dictionary of module metadata
- """
- # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so
- # if we add one then we need to also add it to the policy information (i.e. metadata)
- # we should remove this once we can break this reference from the course to static tabs
- loc = Location(location)
- if loc.category == 'static_tab':
- course = self._get_course_for_item(loc)
- existing_tabs = course.tabs or []
- for tab in existing_tabs:
- if tab.get('url_slug') == loc.name:
- tab['name'] = metadata.get('display_name', tab.get('name'))
- break
- course.tabs = existing_tabs
- # Save the updates to the course to the MongoKeyValueStore
- course.save()
- self.update_metadata(course.location, own_metadata(course))
-
- self._update_single_item(location, {'metadata': metadata})
- # recompute (and update) the metadata inheritance tree which is cached
- self.refresh_cached_metadata_inheritance_tree(loc)
- self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location))
-
- def delete_item(self, location, delete_all_versions=False):
+ # pylint: disable=unused-argument
+ def delete_item(self, location, **kwargs):
"""
Delete an item from this modulestore
location: Something that can be passed to Location
- delete_all_versions: is here because the DraftMongoModuleStore needs it and we need to keep the interface the same. It is unused.
"""
+ # pylint: enable=unused-argument
# VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so
# if we add one then we need to also add it to the policy information (i.e. metadata)
# we should remove this once we can break this reference from the course to static tabs
@@ -842,7 +833,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
course.tabs = [tab for tab in existing_tabs if tab.get('url_slug') != location.name]
# Save the updates to the course to the MongoKeyValueStore
course.save()
- self.update_metadata(course.location, own_metadata(course))
+ self.update_item(course, 'delete_item')
# Must include this to avoid the django debug toolbar (which defines the deprecated "safe=False")
# from overriding our default value set in the init method.
@@ -889,7 +880,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
item_locs -= all_reachable
return list(item_locs)
- def _create_new_field_data(self, category, location, definition_data, metadata):
+ def _create_new_field_data(self, _category, _location, definition_data, metadata):
"""
To instantiate a new xmodule which will be saved latter, set up the dbModel and kvs
"""
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
index f7a0250155..abb59e4983 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
@@ -11,11 +11,9 @@ from datetime import datetime
from xmodule.exceptions import InvalidVersionError
from xmodule.modulestore import Location
from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateItemError
-from xmodule.modulestore.inheritance import own_metadata
from xmodule.modulestore.mongo.base import location_to_query, namedtuple_to_son, get_course_id_no_run, MongoModuleStore
import pymongo
from pytz import UTC
-from xblock.fields import Scope
DRAFT = 'draft'
# Things w/ these categories should never be marked as version='draft'
@@ -109,7 +107,7 @@ class DraftModuleStore(MongoModuleStore):
return super(DraftModuleStore, self).create_xmodule(draft_loc, definition_data, metadata, system)
- def get_items(self, location, course_id=None, depth=0):
+ def get_items(self, location, course_id=None, depth=0, qualifiers=None):
"""
Returns a list of XModuleDescriptor instances for the items
that match location. Any element of location that is None is treated
@@ -146,7 +144,9 @@ class DraftModuleStore(MongoModuleStore):
draft_location = as_draft(source_location)
if draft_location.category in DIRECT_ONLY_CATEGORIES:
raise InvalidVersionError(source_location)
- original['_id'] = draft_location.dict()
+ if not original:
+ raise ItemNotFoundError
+ original['_id'] = namedtuple_to_son(draft_location)
try:
self.collection.insert(original)
except pymongo.errors.DuplicateKeyError:
@@ -157,60 +157,27 @@ class DraftModuleStore(MongoModuleStore):
return self._load_items([original])[0]
- def update_item(self, location, data, allow_not_found=False):
+ def update_item(self, xblock, user, allow_not_found=False):
"""
- Set the data in the item specified by the location to
- data
+ Save the current values to persisted version of the xblock
location: Something that can be passed to Location
data: A nested dictionary of problem data
"""
- draft_loc = as_draft(location)
+ draft_loc = as_draft(xblock.location)
try:
- draft_item = self.get_item(location)
- if not getattr(draft_item, 'is_draft', False):
- self.convert_to_draft(location)
+ if not self.has_item(None, draft_loc):
+ self.convert_to_draft(xblock.location)
except ItemNotFoundError, e:
if not allow_not_found:
raise e
- return super(DraftModuleStore, self).update_item(draft_loc, data)
+ xblock.location = draft_loc
+ super(DraftModuleStore, self).update_item(xblock, user)
+ # don't allow locations to truly represent thenmselves as draft outside of this file
+ xblock.location = as_published(xblock.location)
- def update_children(self, location, children):
- """
- Set the children for the item specified by the location to
- children
-
- location: Something that can be passed to Location
- children: A list of child item identifiers
- """
- draft_loc = as_draft(location)
- draft_item = self.get_item(location)
- if not getattr(draft_item, 'is_draft', False):
- self.convert_to_draft(location)
-
- return super(DraftModuleStore, self).update_children(draft_loc, children)
-
- def update_metadata(self, location, metadata):
- """
- Set the metadata for the item specified by the location to
- metadata
-
- location: Something that can be passed to Location
- metadata: A nested dictionary of module metadata
- """
- draft_loc = as_draft(location)
- draft_item = self.get_item(location)
-
- if not getattr(draft_item, 'is_draft', False):
- self.convert_to_draft(location)
-
- if 'is_draft' in metadata:
- del metadata['is_draft']
-
- return super(DraftModuleStore, self).update_metadata(draft_loc, metadata)
-
- def delete_item(self, location, delete_all_versions=False):
+ def delete_item(self, location, delete_all_versions=False, **kwargs):
"""
Delete an item from this modulestore
@@ -243,7 +210,6 @@ class DraftModuleStore(MongoModuleStore):
draft.published_date = datetime.now(UTC)
draft.published_by = published_by_id
- super(DraftModuleStore, self).update_item(location, draft.get_explicitly_set_fields_by_scope(Scope.content))
if draft.has_children:
if original_published is not None:
# see if children were deleted. 2 reasons for children lists to differ:
@@ -254,8 +220,7 @@ class DraftModuleStore(MongoModuleStore):
rents = [Location(mom) for mom in self.get_parent_locations(child, None)]
if (len(rents) == 1 and rents[0] == Location(location)): # the 1 is this original_published
self.delete_item(child, True)
- super(DraftModuleStore, self).update_children(location, draft.children)
- super(DraftModuleStore, self).update_metadata(location, own_metadata(draft))
+ super(DraftModuleStore, self).update_item(draft, None)
self.delete_item(location)
def unpublish(self, location):
diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
index 279002e473..ea89f7b4b7 100644
--- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
+++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
@@ -57,7 +57,8 @@ from pytz import UTC
from xmodule.errortracker import null_error_tracker
from xmodule.x_module import prefer_xmodules
-from xmodule.modulestore.locator import BlockUsageLocator, DefinitionLocator, CourseLocator, VersionTree, LocalId
+from xmodule.modulestore.locator import BlockUsageLocator, DefinitionLocator, CourseLocator, VersionTree, LocalId, \
+ Locator
from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError, DuplicateItemError
from xmodule.modulestore import inheritance, ModuleStoreWriteBase, Location, SPLIT_MONGO_MODULESTORE_TYPE
@@ -109,6 +110,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
super(SplitMongoModuleStore, self).__init__(**kwargs)
self.loc_mapper = loc_mapper
+ self.reference_type = Locator
self.db_connection = MongoConnection(**doc_store_config)
self.db = self.db_connection.database
@@ -1115,14 +1117,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
if key not in new_keys or original_fields[key] != settings[key]:
return True
- def update_children(self, location, children):
- '''Deprecated, use update_item.'''
- raise NotImplementedError('use update_item')
-
- def update_metadata(self, location, metadata):
- '''Deprecated, use update_item.'''
- raise NotImplementedError('use update_item')
-
def xblock_publish(self, user_id, source_course, destination_course, subtree_list, blacklist):
"""
Publishes each xblock in subtree_list and those blocks descendants excluding blacklist
@@ -1211,7 +1205,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
"""
self.db_connection.update_course_index(updated_index_entry)
- def delete_item(self, usage_locator, user_id, delete_children=False, force=False):
+ # TODO impl delete_all_versions
+ def delete_item(self, usage_locator, user_id, delete_all_versions=False, delete_children=False, force=False):
"""
Delete the block or tree rooted at block (if delete_children) and any references w/in the course to the block
from a new version of the course structure.
@@ -1361,6 +1356,16 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
else:
return DefinitionLocator(definition['_id'])
+ def get_modulestore_type(self, course_id):
+ """
+ Returns an enumeration-like type reflecting the type of this modulestore
+ The return can be one of:
+ "xml" (for XML based courses),
+ "mongo" for old-style MongoDB backed courses,
+ "split" for new-style split MongoDB backed courses.
+ """
+ return SPLIT_MONGO_MODULESTORE_TYPE
+
def internal_clean_children(self, course_locator):
"""
Only intended for rather low level methods to use. Goes through the children attrs of
@@ -1509,16 +1514,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
del fields['category']
return fields
- def get_modulestore_type(self, course_id):
- """
- Returns an enumeration-like type reflecting the type of this modulestore
- The return can be one of:
- "xml" (for XML based courses),
- "mongo" for old-style MongoDB backed courses,
- "split" for new-style split MongoDB backed courses.
- """
- return SPLIT_MONGO_MODULESTORE_TYPE
-
def _new_structure(self, user_id, root_block_id,
root_category=None, block_fields=None, definition_id=None):
"""
diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py
index 9408dc14fd..43e1b4ec0f 100644
--- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py
+++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py
@@ -1,7 +1,6 @@
import re
from xmodule.contentstore.content import StaticContent
from xmodule.modulestore import Location
-from xmodule.modulestore.inheritance import own_metadata
import logging
@@ -125,13 +124,9 @@ def _clone_modules(modulestore, modules, source_location, dest_location):
print "Cloning module {0} to {1}....".format(original_loc, module.location)
- # NOTE: usage of the the internal module.xblock_kvs._data does not include any 'default' values for the fields
- data = module.xblock_kvs._data
- if isinstance(data, basestring):
- data = rewrite_nonportable_content_links(
- source_location.course_id, dest_location.course_id, data)
-
- modulestore.update_item(module.location, data)
+ if 'data' in module.fields and module.fields['data'].is_set_on(module) and isinstance(module.data, basestring):
+ module.data = rewrite_nonportable_content_links(
+ source_location.course_id, dest_location.course_id, module.data)
# repoint children
if module.has_children:
@@ -145,10 +140,9 @@ def _clone_modules(modulestore, modules, source_location, dest_location):
)
new_children.append(child_loc.url())
- modulestore.update_children(module.location, new_children)
+ module.children = new_children
- # save metadata
- modulestore.update_metadata(module.location, own_metadata(module))
+ modulestore.update_item(module, '_clone_modules')
def clone_course(modulestore, contentstore, source_location, dest_location, delete_original=False):
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
index 88bc6c87d9..8211d71e2d 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
@@ -33,6 +33,7 @@ def mixed_store_config(data_dir, mappings):
'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore',
'OPTIONS': {
'mappings': mappings,
+ 'reference_type': 'Location',
'stores': {
'default': mongo_config['default'],
'xml': xml_config['default']
@@ -196,18 +197,15 @@ class ModuleStoreTestCase(TestCase):
"""
@staticmethod
- def update_course(course, data):
+ def update_course(course):
"""
Updates the version of course in the modulestore
- with the metadata in 'data' and returns the updated version.
'course' is an instance of CourseDescriptor for which we want
to update metadata.
-
- 'data' is a dictionary with an entry for each CourseField we want to update.
"""
store = editable_modulestore('direct')
- store.update_metadata(course.location, data)
+ store.update_item(course, 'testuser')
updated_course = store.get_instance(course.id, course.location)
return updated_course
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
index 70b580b69d..793792dd8d 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
@@ -1,9 +1,6 @@
-import datetime
-
from factory import Factory, lazy_attribute_sequence, lazy_attribute
from factory.containers import CyclicDefinitionError
from uuid import uuid4
-from pytz import UTC
from xmodule.modulestore import Location
from xmodule.x_module import prefer_xmodules
@@ -127,7 +124,6 @@ class ItemFactory(XModuleFactory):
# passed in via **kwargs. However, some of those aren't actual field values,
# so pop those off for use separately
- DETACHED_CATEGORIES = ['about', 'static_tab', 'course_info']
# catch any old style users before they get into trouble
assert 'template' not in kwargs
parent_location = Location(kwargs.pop('parent_location', None))
@@ -165,8 +161,8 @@ class ItemFactory(XModuleFactory):
store.save_xmodule(module)
- if location.category not in DETACHED_CATEGORIES:
+ if 'detached' not in module._class_tags:
parent.children.append(location.url())
- store.update_children(parent_location, parent.children)
+ store.update_item(parent, 'factory')
return store.get_item(location)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py
index a0e869d9fe..0848e5c707 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py
@@ -7,7 +7,7 @@ import unittest
import uuid
from xmodule.modulestore import Location
from xmodule.modulestore.locator import BlockUsageLocator
-from xmodule.modulestore.exceptions import ItemNotFoundError
+from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError
from xmodule.modulestore.loc_mapper_store import LocMapperStore
from mock import Mock
@@ -114,7 +114,7 @@ class TestLocationMapper(unittest.TestCase):
new_style_package_id = '{}.geek_dept.{}.baz_run'.format(org, course)
block_map = {
- 'abc123': {'problem': 'problem2'},
+ 'abc123': {'problem': 'problem2', 'vertical': 'vertical2'},
'def456': {'problem': 'problem4'},
'ghi789': {'problem': 'problem7'},
}
@@ -136,6 +136,14 @@ class TestLocationMapper(unittest.TestCase):
Location('i4x', org, course, 'problem', '1def23'),
add_entry_if_missing=False
)
+ test_no_cat_locn = test_problem_locn.replace(category=None)
+ with self.assertRaises(InvalidLocationError):
+ loc_mapper().translate_location(
+ old_style_course_id, test_no_cat_locn, False, False
+ )
+ test_no_cat_locn = test_no_cat_locn.replace(name='def456')
+ # only one course matches
+ self.translate_n_check(test_no_cat_locn, old_style_course_id, new_style_package_id, 'problem4', 'published')
# add a distractor course (note that abc123 has a different translation in this one)
distractor_block_map = {
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py
index 5922f13afc..5ec34ae801 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py
@@ -259,6 +259,24 @@ class LocatorTest(TestCase):
testobj = BlockUsageLocator(package_id=package_id, branch=branch, block_id=block_id)
self.check_block_locn_fields(testobj, 'Cannot handle colon', package_id=package_id, branch=branch, block=block_id)
+ def test_relative(self):
+ """
+ It seems we used to use colons in names; so, ensure they're acceptable.
+ """
+ package_id = 'mit.eecs-1'
+ branch = 'foo'
+ baseobj = CourseLocator(package_id=package_id, branch=branch)
+ block_id = 'problem:with-colon~2'
+ testobj = BlockUsageLocator.make_relative(baseobj, block_id)
+ self.check_block_locn_fields(
+ testobj, 'Cannot make relative to course', package_id=package_id, branch=branch, block=block_id
+ )
+ block_id = 'completely_different'
+ testobj = BlockUsageLocator.make_relative(testobj, block_id)
+ self.check_block_locn_fields(
+ testobj, 'Cannot make relative to block usage', package_id=package_id, branch=branch, block=block_id
+ )
+
def test_repr(self):
testurn = 'mit.eecs.6002x/' + BRANCH_PREFIX + 'published/' + BLOCK_PREFIX + 'HW3'
testobj = BlockUsageLocator(package_id=testurn)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
index 834e55398c..339b662db5 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
@@ -37,6 +37,7 @@ OPTIONS = {
XML_COURSEID2: 'xml',
IMPORT_COURSEID: 'default'
},
+ 'reference_type': 'Location',
'stores': {
'xml': {
'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore',
@@ -182,6 +183,7 @@ class TestMixedModuleStore(object):
)
def test_get_items(self):
+ # NOTE: use get_course if you just want the course. get_items only allows wildcarding of category and name
modules = self.store.get_items(Location('i4x', None, None, 'course', None), IMPORT_COURSEID)
assert_equals(len(modules), 1)
assert_equals(modules[0].location.course, self.import_course)
@@ -190,22 +192,15 @@ class TestMixedModuleStore(object):
assert_equals(len(modules), 1)
assert_equals(modules[0].location.course, 'toy')
- modules = self.store.get_items(Location('i4x', None, None, 'course', None), XML_COURSEID2)
+ modules = self.store.get_items(Location('i4x', 'edX', 'simple', 'course', None), XML_COURSEID2)
assert_equals(len(modules), 1)
assert_equals(modules[0].location.course, 'simple')
def test_update_item(self):
+ # FIXME update
with assert_raises(NotImplementedError):
self.store.update_item(self.fake_location, None)
- def test_update_children(self):
- with assert_raises(NotImplementedError):
- self.store.update_children(self.fake_location, None)
-
- def test_update_metadata(self):
- with assert_raises(NotImplementedError):
- self.store.update_metadata(self.fake_location, None)
-
def test_delete_item(self):
with assert_raises(NotImplementedError):
self.store.delete_item(self.fake_location)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py
index 16c70ea634..bd199cc02b 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py
@@ -82,7 +82,7 @@ class TestOrphan(unittest.TestCase):
parent_location = Location('i4x', 'test_org', 'test_course', parent_category, parent_name)
parent = self.old_mongo.get_item(parent_location)
parent.children.append(location.url())
- self.old_mongo.update_children(parent_location, parent.children)
+ self.old_mongo.update_item(parent, self.userid)
# create pointer for split
course_or_parent_locator = BlockUsageLocator(
package_id=self.split_package_id,
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py
index 867351370c..40a7d1a88f 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py
@@ -71,7 +71,7 @@ class TestPublish(unittest.TestCase):
mongo = self.old_mongo
else:
mongo = self.draft_mongo
- mongo.update_children(parent_location, parent.children)
+ mongo.update_item(parent, '_create_item')
def _create_course(self):
"""
@@ -174,8 +174,8 @@ class TestPublish(unittest.TestCase):
draft_vert.children.remove(other_child_loc.url())
other_vert = self.draft_mongo.get_item(self.course_location.replace(category='vertical', name='Vert2'), 0)
other_vert.children.append(other_child_loc.url())
- self.draft_mongo.update_children(draft_vert.location, draft_vert.children)
- self.draft_mongo.update_children(other_vert.location, other_vert.children)
+ self.draft_mongo.update_item(draft_vert, 'test_publish_delete')
+ self.draft_mongo.update_item(other_vert, 'test_publish_delete')
# publish
self._xmodule_recurse(
draft_vert,
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py
index 0341d43436..1e662c197b 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py
@@ -102,7 +102,7 @@ class TestMigration(unittest.TestCase):
location = location.replace(category='chapter', name=uuid.uuid4().hex)
chapter2 = self._create_and_get_item(self.old_mongo, location, {}, {'display_name': 'Chapter 2'}, runtime)
course_root.children.append(chapter2.location.url())
- self.old_mongo.update_children(course_root.location, course_root.children)
+ self.old_mongo.update_item(course_root, 'test_split_migrate')
# vertical in live only
location = location.replace(category='vertical', name=uuid.uuid4().hex)
live_vert = self._create_and_get_item(self.old_mongo, location, {}, {'display_name': 'Live vertical'}, runtime)
@@ -140,7 +140,7 @@ class TestMigration(unittest.TestCase):
self.create_random_units(self.old_mongo, live_vert)
# update the chapter
- self.old_mongo.update_children(chapter1.location, chapter1.children)
+ self.old_mongo.update_item(chapter1, 'test_split_migrator')
# now the other one w/ the conditional
# first create some show children
@@ -169,7 +169,7 @@ class TestMigration(unittest.TestCase):
# add direct children
self.create_random_units(self.old_mongo, conditional)
chapter2.children.append(conditional.location.url())
- self.old_mongo.update_children(chapter2.location, chapter2.children)
+ self.old_mongo.update_item(chapter2, 'test_split_migrator')
# and the ancillary docs (not children)
location = location.replace(category='static_tab', name=uuid.uuid4().hex)
@@ -207,9 +207,9 @@ class TestMigration(unittest.TestCase):
cc_store, location, data, {'display_name': str(uuid.uuid4())}, parent.runtime
)
cc_parent.children.append(element.location.url())
- store.update_children(parent.location, parent.children)
+ store.update_item(parent, 'test_split_migrator')
if cc_store is not None:
- cc_store.update_children(cc_parent.location, cc_parent.children)
+ cc_store.update_item(cc_parent, 'test_split_migrator')
def compare_courses(self, presplit, published):
# descend via children to do comparison
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index 0cdaf7f7e3..d78a4dcf1e 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -377,6 +377,7 @@ class XMLModuleStore(ModuleStoreReadBase):
self.default_class = class_
self.parent_trackers = defaultdict(ParentTracker)
+ self.reference_type = Location
# All field data will be stored in an inheriting field data.
self.field_data = inheriting_field_data(kvs=DictKeyValueStore())
@@ -640,7 +641,7 @@ class XMLModuleStore(ModuleStoreReadBase):
raise NotImplementedError("XMLModuleStores can't guarantee that definitions"
" are unique. Use get_instance.")
- def get_items(self, location, course_id=None, depth=0):
+ def get_items(self, location, course_id=None, depth=0, qualifiers=None):
items = []
def _add_get_items(self, location, modules):
@@ -672,7 +673,16 @@ class XMLModuleStore(ModuleStoreReadBase):
"""
return dict((k, self.errored_courses[k].errors) for k in self.errored_courses)
- def update_item(self, location, data):
+ def get_orphans(self, course_location, _branch):
+ """
+ Get all of the xblocks in the given course which have no parents and are not of types which are
+ usually orphaned. NOTE: may include xblocks which still have references via xblocks which don't
+ use children to point to their dependents.
+ """
+ # here just to quell the abstractmethod. someone could write the impl if needed
+ raise NotImplementedError
+
+ def update_item(self, xblock, user, **kwargs):
"""
Set the data in the item specified by the location to
data
@@ -682,26 +692,6 @@ class XMLModuleStore(ModuleStoreReadBase):
"""
raise NotImplementedError("XMLModuleStores are read-only")
- def update_children(self, location, children):
- """
- Set the children for the item specified by the location to
- data
-
- location: Something that can be passed to Location
- children: A list of child item identifiers
- """
- raise NotImplementedError("XMLModuleStores are read-only")
-
- def update_metadata(self, location, metadata):
- """
- Set the metadata for the item specified by the location to
- metadata
-
- location: Something that can be passed to Location
- metadata: A nested dictionary of module metadata
- """
- raise NotImplementedError("XMLModuleStores are read-only")
-
def get_parent_locations(self, location, course_id):
'''Find all locations that are the parents of this location in this
course. Needed for path_to_location().
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index 47dedecc6b..5815a459bc 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -312,43 +312,13 @@ def import_module(
logging.debug('processing import of module {}...'.format(module.location.url()))
- content = {}
- for field in module.fields.values():
- if field.scope != Scope.content:
- continue
- try:
- content[field.name] = module._field_data.get(module, field.name)
- except KeyError:
- # Ignore any missing keys in _field_data
- pass
-
- module_data = {}
- if 'data' in content:
- module_data = content['data']
- else:
- module_data = content
-
- if isinstance(module_data, basestring) and do_import_static:
+ if 'data' in module.fields and do_import_static:
# we want to convert all 'non-portable' links in the module_data
# (if it is a string) to portable strings (e.g. /static/)
- module_data = rewrite_nonportable_content_links(
+ module.data = rewrite_nonportable_content_links(
source_course_location.course_id,
- dest_course_location.course_id, module_data
+ dest_course_location.course_id, module.data
)
-
- if allow_not_found:
- store.update_item(
- module.location, module_data, allow_not_found=allow_not_found
- )
- else:
- store.update_item(module.location, module_data)
-
- if hasattr(module, 'children') and module.children != []:
- store.update_children(module.location, module.children)
-
- # NOTE: It's important to use own_metadata here to avoid writing
- # inherited metadata everywhere.
-
# remove any export/import only xml_attributes
# which are used to wire together draft imports
if 'parent_sequential_url' in getattr(module, 'xml_attributes', []):
@@ -356,9 +326,8 @@ def import_module(
if 'index_in_children_list' in getattr(module, 'xml_attributes', []):
del module.xml_attributes['index_in_children_list']
- module.save()
- store.update_metadata(module.location, dict(own_metadata(module)))
+ store.update_item(module, None, allow_not_found=allow_not_found)
def import_course_draft(
@@ -490,7 +459,7 @@ def import_course_draft(
if non_draft_location.url() not in sequential.children:
sequential.children.insert(index, non_draft_location.url())
- store.update_children(sequential.location, sequential.children)
+ store.update_item(sequential, 'importer')
import_module(
module, draft_store, course_data_path,
diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py
index 6221211e44..662aec3910 100644
--- a/lms/djangoapps/courseware/tests/test_submitting_problems.py
+++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py
@@ -229,9 +229,9 @@ class TestCourseGrader(TestSubmittingProblems):
Add a grading policy to the course.
"""
- course_data = {'grading_policy': grading_policy}
+ self.course.grading_policy = grading_policy
store = editable_modulestore('direct')
- store.update_item(self.course.location, course_data)
+ store.update_item(self.course, 'add_grading_policy')
self.refresh_course()
def get_grade_summary(self):
diff --git a/lms/djangoapps/courseware/tests/test_view_authentication.py b/lms/djangoapps/courseware/tests/test_view_authentication.py
index acc39c5001..e7fadb006f 100644
--- a/lms/djangoapps/courseware/tests/test_view_authentication.py
+++ b/lms/djangoapps/courseware/tests/test_view_authentication.py
@@ -25,6 +25,7 @@ from courseware.tests.factories import (
OrgStaffFactory,
OrgInstructorFactory,
)
+from xmodule.modulestore.django import modulestore
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@@ -84,7 +85,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
urls.extend([
reverse('book', kwargs={'course_id': course.id,
'book_index': index})
- for index, book in enumerate(course.textbooks)
+ for index, _book in enumerate(course.textbooks)
])
for url in urls:
check_for_get_code(self, 200, url)
@@ -112,6 +113,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.course = CourseFactory.create(number='999', display_name='Robot_Super_Course')
self.overview_chapter = ItemFactory.create(display_name='Overview')
self.courseware_chapter = ItemFactory.create(display_name='courseware')
+ self.course = modulestore().get_course(self.course.id)
self.test_course = CourseFactory.create(number='666', display_name='Robot_Sub_Course')
self.other_org_course = CourseFactory.create(org='Other_Org_Course')
@@ -126,6 +128,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
parent_location=self.overview_chapter.location,
display_name='Welcome'
)
+ self.test_course = modulestore().get_course(self.test_course.id)
self.global_staff_user = GlobalStaffFactory()
self.unenrolled_user = UserFactory(last_name="Unenrolled")
@@ -264,10 +267,10 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
# Make courses start in the future
now = datetime.datetime.now(pytz.UTC)
tomorrow = now + datetime.timedelta(days=1)
- course_data = {'start': tomorrow}
- test_course_data = {'start': tomorrow}
- self.course = self.update_course(self.course, course_data)
- self.test_course = self.update_course(self.test_course, test_course_data)
+ self.course.start = tomorrow
+ self.test_course.start = tomorrow
+ self.course = self.update_course(self.course)
+ self.test_course = self.update_course(self.test_course)
self.assertFalse(self.course.has_started())
self.assertFalse(self.test_course.has_started())
@@ -289,10 +292,10 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
"""
now = datetime.datetime.now(pytz.UTC)
tomorrow = now + datetime.timedelta(days=1)
- course_data = {'start': tomorrow}
- test_course_data = {'start': tomorrow}
- self.course = self.update_course(self.course, course_data)
- self.test_course = self.update_course(self.test_course, test_course_data)
+ self.course.start = tomorrow
+ self.test_course.start = tomorrow
+ self.course = self.update_course(self.course)
+ self.test_course = self.update_course(self.test_course)
self.login(self.instructor_user)
# Enroll in the classes---can't see courseware otherwise.
@@ -312,10 +315,10 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
"""
now = datetime.datetime.now(pytz.UTC)
tomorrow = now + datetime.timedelta(days=1)
- course_data = {'start': tomorrow}
- test_course_data = {'start': tomorrow}
- self.course = self.update_course(self.course, course_data)
- self.test_course = self.update_course(self.test_course, test_course_data)
+ self.course.start = tomorrow
+ self.test_course.start = tomorrow
+ self.course = self.update_course(self.course)
+ self.test_course = self.update_course(self.test_course)
self.login(self.global_staff_user)
self.enroll(self.course, True)
@@ -336,13 +339,14 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
nextday = tomorrow + datetime.timedelta(days=1)
yesterday = now - datetime.timedelta(days=1)
- course_data = {'enrollment_start': tomorrow, 'enrollment_end': nextday}
- test_course_data = {'enrollment_start': yesterday, 'enrollment_end': tomorrow}
-
# self.course's enrollment period hasn't started
- self.course = self.update_course(self.course, course_data)
+ self.course.enrollment_start = tomorrow
+ self.course.enrollment_end = nextday
# test_course course's has
- self.test_course = self.update_course(self.test_course, test_course_data)
+ self.test_course.enrollment_start = yesterday
+ self.test_course.enrollment_end = tomorrow
+ self.course = self.update_course(self.course)
+ self.test_course = self.update_course(self.test_course)
# First, try with an enrolled student
self.login(self.unenrolled_user)
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index a47ebac017..d9b1bbf825 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -234,13 +234,13 @@ def index(request, course_id, chapter=None, section=None,
- HTTPresponse
"""
user = User.objects.prefetch_related("groups").get(id=request.user.id)
- request.user = user # keep just one instance of User
+ request.user = user # keep just one instance of User
course = get_course_with_access(user, course_id, 'load', depth=2)
staff_access = has_access(user, course, 'staff')
registered = registered_for_course(course, user)
if not registered:
# TODO (vshnayder): do course instructors need to be registered to see course?
- log.debug(u'User {0} tried to view course {1} but is not enrolled'.format(user, course.location.url()))
+ log.debug('User %s tried to view course %s but is not enrolled', user, course.location.url())
return redirect(reverse('about_course', args=[course.id]))
masq = setup_masquerade(request, staff_access)
diff --git a/lms/djangoapps/instructor_task/tests/test_base.py b/lms/djangoapps/instructor_task/tests/test_base.py
index 6ee8d3dc3d..b6f9b9df8d 100644
--- a/lms/djangoapps/instructor_task/tests/test_base.py
+++ b/lms/djangoapps/instructor_task/tests/test_base.py
@@ -208,7 +208,9 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase):
'num_responses': 2}
problem_xml = factory.build_xml(**factory_args)
location = InstructorTaskTestCase.problem_location(problem_url_name)
- self.module_store.update_item(location, problem_xml)
+ item = self.module_store.get_instance(self.course.id, location)
+ item.data = problem_xml
+ self.module_store.update_item(item, 'redefine_option_problem')
def get_student_module(self, username, descriptor):
"""Get StudentModule object for test course, given the `username` and the problem's `descriptor`."""
diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py
index e6cdb92183..056a89b202 100644
--- a/lms/djangoapps/instructor_task/tests/test_integration.py
+++ b/lms/djangoapps/instructor_task/tests/test_integration.py
@@ -288,7 +288,11 @@ class TestRescoringTask(TestIntegrationTask):
""" % ('!=' if redefine else '=='))
problem_xml = factory.build_xml(script=script, cfn="check_func", expect="42", num_responses=1)
if redefine:
- self.module_store.update_item(InstructorTaskModuleTestCase.problem_location(problem_url_name), problem_xml)
+ descriptor = self.module_store.get_instance(
+ self.course.id, InstructorTaskModuleTestCase.problem_location(problem_url_name)
+ )
+ descriptor.data = problem_xml
+ self.module_store.update_item(descriptor, 'define_randomized_custom_response_problem')
else:
# Use "per-student" rerandomization so that check-problem can be called more than once.
# Using "always" means we cannot check a problem twice, but we want to call once to get the
diff --git a/lms/envs/acceptance.py b/lms/envs/acceptance.py
index b1d49b5237..a63aba0a2c 100644
--- a/lms/envs/acceptance.py
+++ b/lms/envs/acceptance.py
@@ -44,6 +44,7 @@ MODULESTORE = {
'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore',
'OPTIONS': {
'mappings': {},
+ 'reference_type': 'Location',
'stores': {
'default': {
'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
diff --git a/lms/envs/bok_choy.auth.json b/lms/envs/bok_choy.auth.json
index 931963c6d0..8f9e757490 100644
--- a/lms/envs/bok_choy.auth.json
+++ b/lms/envs/bok_choy.auth.json
@@ -51,6 +51,7 @@
"ENGINE": "xmodule.modulestore.mixed.MixedModuleStore",
"OPTIONS": {
"mappings": {},
+ "reference_type": "Location",
"stores": {
"default": {
"DOC_STORE_CONFIG": {
diff --git a/lms/envs/cms/mixed_dev.py b/lms/envs/cms/mixed_dev.py
index 0454c25409..35c5e0b82c 100644
--- a/lms/envs/cms/mixed_dev.py
+++ b/lms/envs/cms/mixed_dev.py
@@ -12,6 +12,7 @@ MODULESTORE = {
'default': {
'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore',
'OPTIONS': {
+ 'reference_type': 'Location',
'mappings': {
'MITx/2.01x/2013_Spring': 'xml'
},