Skip to content

E
edx-platform
Commit abbfa95e by Nimisha Asthagiri
Options

LMS-11168 Support for removing versions and branch in Split, Mixed, and SQL. 

Make default_store thread-safe.
parent e9db4ad1
Showing with 450 additions and 224 deletions
cms/djangoapps/contentstore/tests/test_contentstore.py
...@@ -28,7 +28,7 @@ from xmodule.exceptions import NotFoundError, InvalidVersionError ...@@ -28,7 +28,7 @@ from xmodule.exceptions import NotFoundError, InvalidVersionError
from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.inheritance import own_metadata
from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.keys import UsageKey, CourseKey
from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation, CourseLocator from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation, CourseLocator
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_exporter import export_to_xml
...@@ -47,6 +47,7 @@ from student.roles import CourseCreatorRole, CourseInstructorRole ...@@ -47,6 +47,7 @@ from student.roles import CourseCreatorRole, CourseInstructorRole
from opaque_keys import InvalidKeyError from opaque_keys import InvalidKeyError
from contentstore.tests.utils import get_url from contentstore.tests.utils import get_url
from course_action_state.models import CourseRerunState, CourseRerunUIStateManager from course_action_state.models import CourseRerunState, CourseRerunUIStateManager
from course_action_state.managers import CourseActionStateItemNotFoundError
TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
...@@ -1115,8 +1116,7 @@ class ContentStoreTest(ContentStoreTestCase): ...@@ -1115,8 +1116,7 @@ class ContentStoreTest(ContentStoreTestCase):
def test_create_course_with_bad_organization(self): def test_create_course_with_bad_organization(self):
"""Test new course creation - error path for bad organization name""" """Test new course creation - error path for bad organization name"""
self.course_data['org'] = 'University of California, Berkeley' self.course_data['org'] = 'University of California, Berkeley'
self.assert_course_creation_failed( self.assert_course_creation_failed(r"(?s)Unable to create course 'Robot Super Course'.*")
r"(?s)Unable to create course 'Robot Super Course'.*: Invalid characters in u'University of California, Berkeley'")
def test_create_course_with_course_creation_disabled_staff(self): def test_create_course_with_course_creation_disabled_staff(self):
"""Test new course creation -- course creation disabled, but staff access.""" """Test new course creation -- course creation disabled, but staff access."""
...@@ -1572,31 +1572,35 @@ class RerunCourseTest(ContentStoreTestCase): ...@@ -1572,31 +1572,35 @@ class RerunCourseTest(ContentStoreTestCase):
'display_name': 'Robot Super Course', 'display_name': 'Robot Super Course',
'run': '2013_Spring' 'run': '2013_Spring'
} }
self.destination_course_key = _get_course_id(self.destination_course_data)
def post_rerun_request(self, source_course_key, response_code=200): def post_rerun_request(
self, source_course_key, destination_course_data=None, response_code=200, expect_error=False
):
"""Create and send an ajax post for the rerun request""" """Create and send an ajax post for the rerun request"""
# create data to post # create data to post
rerun_course_data = {'source_course_key': unicode(source_course_key)} rerun_course_data = {'source_course_key': unicode(source_course_key)}
rerun_course_data.update(self.destination_course_data) if not destination_course_data:
destination_course_data = self.destination_course_data
rerun_course_data.update(destination_course_data)
destination_course_key = _get_course_id(destination_course_data)
# post the request # post the request
course_url = get_url('course_handler', self.destination_course_key, 'course_key_string') course_url = get_url('course_handler', destination_course_key, 'course_key_string')
response = self.client.ajax_post(course_url, rerun_course_data) response = self.client.ajax_post(course_url, rerun_course_data)
# verify response # verify response
self.assertEqual(response.status_code, response_code) self.assertEqual(response.status_code, response_code)
if response_code == 200: if not expect_error:
self.assertNotIn('ErrMsg', parse_json(response)) json_resp = parse_json(response)
self.assertNotIn('ErrMsg', json_resp)
destination_course_key = CourseKey.from_string(json_resp['destination_course_key'])
def create_course_listing_html(self, course_key): return destination_course_key
"""Creates html fragment that is created for the given course_key in the course listing section"""
return '<a class="course-link" href="/course/{}"'.format(course_key)
def create_unsucceeded_course_action_html(self, course_key): def create_unsucceeded_course_action_html(self, course_key):
"""Creates html fragment that is created for the given course_key in the unsucceeded course action section""" """Creates html fragment that is created for the given course_key in the unsucceeded course action section"""
# TODO LMS-11011 Update this once the Rerun UI is implemented. # TODO Update this once the Rerun UI LMS-11011 is implemented.
return '<div class="unsucceeded-course-action" href="/course/{}"'.format(course_key) return '<div class="unsucceeded-course-action" href="/course/{}"'.format(course_key)
def assertInCourseListing(self, course_key): def assertInCourseListing(self, course_key):
...@@ -1605,7 +1609,7 @@ class RerunCourseTest(ContentStoreTestCase): ...@@ -1605,7 +1609,7 @@ class RerunCourseTest(ContentStoreTestCase):
and NOT in the unsucceeded course action section of the html. and NOT in the unsucceeded course action section of the html.
""" """
course_listing_html = self.client.get_html('/course/') course_listing_html = self.client.get_html('/course/')
self.assertIn(self.create_course_listing_html(course_key), course_listing_html.content) self.assertIn(course_key.run, course_listing_html.content)
self.assertNotIn(self.create_unsucceeded_course_action_html(course_key), course_listing_html.content) self.assertNotIn(self.create_unsucceeded_course_action_html(course_key), course_listing_html.content)
def assertInUnsucceededCourseActions(self, course_key): def assertInUnsucceededCourseActions(self, course_key):
...@@ -1614,32 +1618,39 @@ class RerunCourseTest(ContentStoreTestCase): ...@@ -1614,32 +1618,39 @@ class RerunCourseTest(ContentStoreTestCase):
and NOT in the accessible course listing section of the html. and NOT in the accessible course listing section of the html.
""" """
course_listing_html = self.client.get_html('/course/') course_listing_html = self.client.get_html('/course/')
self.assertNotIn(self.create_course_listing_html(course_key), course_listing_html.content) self.assertNotIn(course_key.run, course_listing_html.content)
# TODO Uncomment this once LMS-11011 is implemented. # TODO Verify the course is in the unsucceeded listing once LMS-11011 is implemented.
# self.assertIn(self.create_unsucceeded_course_action_html(course_key), course_listing_html.content)
def test_rerun_course_success(self): def test_rerun_course_success(self):
source_course = CourseFactory.create()
self.post_rerun_request(source_course.id)
# Verify that the course rerun action is marked succeeded source_course = CourseFactory.create()
rerun_state = CourseRerunState.objects.find_first(course_key=self.destination_course_key) destination_course_key = self.post_rerun_request(source_course.id)
self.assertEquals(rerun_state.state, CourseRerunUIStateManager.State.SUCCEEDED)
# Verify the contents of the course rerun action
rerun_state = CourseRerunState.objects.find_first(course_key=destination_course_key)
expected_states = {
'state': CourseRerunUIStateManager.State.SUCCEEDED,
'source_course_key': source_course.id,
'course_key': destination_course_key,
'should_display': True,
}
for field_name, expected_value in expected_states.iteritems():
self.assertEquals(getattr(rerun_state, field_name), expected_value)
# Verify that the creator is now enrolled in the course. # Verify that the creator is now enrolled in the course.
self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.destination_course_key)) self.assertTrue(CourseEnrollment.is_enrolled(self.user, destination_course_key))
# Verify both courses are in the course listing section # Verify both courses are in the course listing section
self.assertInCourseListing(source_course.id) self.assertInCourseListing(source_course.id)
self.assertInCourseListing(self.destination_course_key) self.assertInCourseListing(destination_course_key)
def test_rerun_course_fail(self): def test_rerun_course_fail_no_source_course(self):
existent_course_key = CourseFactory.create().id existent_course_key = CourseFactory.create().id
non_existent_course_key = CourseLocator("org", "non_existent_course", "run") non_existent_course_key = CourseLocator("org", "non_existent_course", "non_existent_run")
self.post_rerun_request(non_existent_course_key) destination_course_key = self.post_rerun_request(non_existent_course_key)
# Verify that the course rerun action is marked failed # Verify that the course rerun action is marked failed
rerun_state = CourseRerunState.objects.find_first(course_key=self.destination_course_key) rerun_state = CourseRerunState.objects.find_first(course_key=destination_course_key)
self.assertEquals(rerun_state.state, CourseRerunUIStateManager.State.FAILED) self.assertEquals(rerun_state.state, CourseRerunUIStateManager.State.FAILED)
self.assertIn("Cannot find a course at", rerun_state.message) self.assertIn("Cannot find a course at", rerun_state.message)
...@@ -1652,13 +1663,32 @@ class RerunCourseTest(ContentStoreTestCase): ...@@ -1652,13 +1663,32 @@ class RerunCourseTest(ContentStoreTestCase):
# Verify that the failed course is NOT in the course listings # Verify that the failed course is NOT in the course listings
self.assertInUnsucceededCourseActions(non_existent_course_key) self.assertInUnsucceededCourseActions(non_existent_course_key)
def test_rerun_course_fail_duplicate_course(self):
existent_course_key = CourseFactory.create().id
destination_course_data = {
'org': existent_course_key.org,
'number': existent_course_key.course,
'display_name': 'existing course',
'run': existent_course_key.run
}
destination_course_key = self.post_rerun_request(
existent_course_key, destination_course_data, expect_error=True
)
# Verify that the course rerun action doesn't exist
with self.assertRaises(CourseActionStateItemNotFoundError):
CourseRerunState.objects.find_first(course_key=destination_course_key)
# Verify that the existing course continues to be in the course listing
self.assertInCourseListing(existent_course_key)
def test_rerun_with_permission_denied(self): def test_rerun_with_permission_denied(self):
with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
source_course = CourseFactory.create() source_course = CourseFactory.create()
auth.add_users(self.user, CourseCreatorRole(), self.user) auth.add_users(self.user, CourseCreatorRole(), self.user)
self.user.is_staff = False self.user.is_staff = False
self.user.save() self.user.save()
self.post_rerun_request(source_course.id, 403) self.post_rerun_request(source_course.id, response_code=403, expect_error=True)
class EntryPageTestCase(TestCase): class EntryPageTestCase(TestCase):
...@@ -1705,6 +1735,6 @@ def _course_factory_create_course(): ...@@ -1705,6 +1735,6 @@ def _course_factory_create_course():
return CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') return CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
def _get_course_id(course_data): def _get_course_id(course_data, key_class=SlashSeparatedCourseKey):
"""Returns the course ID (org/number/run).""" """Returns the course ID (org/number/run)."""
return SlashSeparatedCourseKey(course_data['org'], course_data['number'], course_data['run']) return key_class(course_data['org'], course_data['number'], course_data['run'])
cms/djangoapps/contentstore/views/assets.py
...@@ -294,5 +294,5 @@ def _get_asset_json(display_name, date, location, thumbnail_location, locked): ...@@ -294,5 +294,5 @@ def _get_asset_json(display_name, date, location, thumbnail_location, locked):
def _add_slash(url): def _add_slash(url):
if not url.startswith('/'): if not url.startswith('/'):
url = '/' + url # TODO NAATODO - is there a better way to do this? url = '/' + url # TODO - re-address this once LMS-11198 is tackled.
return url return url
cms/djangoapps/contentstore/views/course.py
...@@ -24,10 +24,9 @@ from xmodule.contentstore.content import StaticContent ...@@ -24,10 +24,9 @@ from xmodule.contentstore.content import StaticContent
from xmodule.tabs import PDFTextbookTabs from xmodule.tabs import PDFTextbookTabs
from xmodule.partitions.partitions import UserPartition, Group from xmodule.partitions.partitions import UserPartition, Group
from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError
from opaque_keys import InvalidKeyError from opaque_keys import InvalidKeyError
from opaque_keys.edx.locations import Location from opaque_keys.edx.locations import Location
from opaque_keys.edx.locator import CourseLocator
from contentstore.course_info_model import get_course_updates, update_course_updates, delete_course_update from contentstore.course_info_model import get_course_updates, update_course_updates, delete_course_update
from contentstore.utils import ( from contentstore.utils import (
...@@ -442,7 +441,7 @@ def _create_or_rerun_course(request): ...@@ -442,7 +441,7 @@ def _create_or_rerun_course(request):
""" """
To be called by requests that create a new destination course (i.e., create_new_course and rerun_course) To be called by requests that create a new destination course (i.e., create_new_course and rerun_course)
Returns the destination course_key and overriding fields for the new course. Returns the destination course_key and overriding fields for the new course.
Raises InvalidLocationError and InvalidKeyError Raises DuplicateCourseError and InvalidKeyError
""" """
if not auth.has_access(request.user, CourseCreatorRole()): if not auth.has_access(request.user, CourseCreatorRole()):
raise PermissionDenied() raise PermissionDenied()
...@@ -461,15 +460,14 @@ def _create_or_rerun_course(request): ...@@ -461,15 +460,14 @@ def _create_or_rerun_course(request):
status=400 status=400
) )
course_key = CourseLocator(org, number, run)
fields = {'display_name': display_name} if display_name is not None else {} fields = {'display_name': display_name} if display_name is not None else {}
if 'source_course_key' in request.json: if 'source_course_key' in request.json:
return _rerun_course(request, course_key, fields) return _rerun_course(request, org, number, run, fields)
else: else:
return _create_new_course(request, course_key, fields) return _create_new_course(request, org, number, run, fields)
except InvalidLocationError: except DuplicateCourseError:
return JsonResponse({ return JsonResponse({
'ErrMsg': _( 'ErrMsg': _(
'There is already a course defined with the same ' 'There is already a course defined with the same '
...@@ -489,27 +487,27 @@ def _create_or_rerun_course(request): ...@@ -489,27 +487,27 @@ def _create_or_rerun_course(request):
) )
def _create_new_course(request, course_key, fields): def _create_new_course(request, org, number, run, fields):
""" """
Create a new course. Create a new course.
Returns the URL for the course overview page. Returns the URL for the course overview page.
Raises InvalidLocationError if the course already exists Raises DuplicateCourseError if the course already exists
""" """
# Set a unique wiki_slug for newly created courses. To maintain active wiki_slugs for # Set a unique wiki_slug for newly created courses. To maintain active wiki_slugs for
# existing xml courses this cannot be changed in CourseDescriptor. # existing xml courses this cannot be changed in CourseDescriptor.
# # TODO get rid of defining wiki slug in this org/course/run specific way and reconcile # # TODO get rid of defining wiki slug in this org/course/run specific way and reconcile
# w/ xmodule.course_module.CourseDescriptor.__init__ # w/ xmodule.course_module.CourseDescriptor.__init__
wiki_slug = u"{0}.{1}.{2}".format(course_key.org, course_key.course, course_key.run) wiki_slug = u"{0}.{1}.{2}".format(org, number, run)
definition_data = {'wiki_slug': wiki_slug} definition_data = {'wiki_slug': wiki_slug}
fields.update(definition_data) fields.update(definition_data)
store = modulestore() store = modulestore()
with store.default_store(settings.FEATURES.get('DEFAULT_STORE_FOR_NEW_COURSE', 'mongo')): with store.default_store(settings.FEATURES.get('DEFAULT_STORE_FOR_NEW_COURSE', 'mongo')):
# Creating the course raises InvalidLocationError if an existing course with this org/name is found # Creating the course raises DuplicateCourseError if an existing course with this org/name is found
new_course = modulestore().create_course( new_course = store.create_course(
course_key.org, org,
course_key.course, number,
course_key.run, run,
request.user.id, request.user.id,
fields=fields, fields=fields,
) )
...@@ -525,7 +523,7 @@ def _create_new_course(request, course_key, fields): ...@@ -525,7 +523,7 @@ def _create_new_course(request, course_key, fields):
}) })
def _rerun_course(request, destination_course_key, fields): def _rerun_course(request, org, number, run, fields):
""" """
Reruns an existing course. Reruns an existing course.
Returns the URL for the course listing page. Returns the URL for the course listing page.
...@@ -536,6 +534,15 @@ def _rerun_course(request, destination_course_key, fields): ...@@ -536,6 +534,15 @@ def _rerun_course(request, destination_course_key, fields):
if not has_course_access(request.user, source_course_key): if not has_course_access(request.user, source_course_key):
raise PermissionDenied() raise PermissionDenied()
# create destination course key
store = modulestore()
with store.default_store('split'):
destination_course_key = store.make_course_key(org, number, run)
# verify org course and run don't already exist
if store.has_course(destination_course_key, ignore_case=True):
raise DuplicateCourseError(source_course_key, destination_course_key)
# Make sure user has instructor and staff access to the destination course # Make sure user has instructor and staff access to the destination course
# so the user can see the updated status for that course # so the user can see the updated status for that course
add_instructor(destination_course_key, request.user, request.user) add_instructor(destination_course_key, request.user, request.user)
...@@ -544,10 +551,13 @@ def _rerun_course(request, destination_course_key, fields): ...@@ -544,10 +551,13 @@ def _rerun_course(request, destination_course_key, fields):
CourseRerunState.objects.initiated(source_course_key, destination_course_key, request.user) CourseRerunState.objects.initiated(source_course_key, destination_course_key, request.user)
# Rerun the course as a new celery task # Rerun the course as a new celery task
rerun_course.delay(source_course_key, destination_course_key, request.user.id, fields) rerun_course.delay(unicode(source_course_key), unicode(destination_course_key), request.user.id, fields)
# Return course listing page # Return course listing page
return JsonResponse({'url': reverse_url('course_handler')}) return JsonResponse({
'url': reverse_url('course_handler'),
'destination_course_key': unicode(destination_course_key)
})
# pylint: disable=unused-argument # pylint: disable=unused-argument
......
cms/djangoapps/contentstore/views/tasks.py
...@@ -5,17 +5,27 @@ This file contains celery tasks for contentstore views ...@@ -5,17 +5,27 @@ This file contains celery tasks for contentstore views
from celery.task import task from celery.task import task
from django.contrib.auth.models import User from django.contrib.auth.models import User
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import DuplicateCourseError, ItemNotFoundError
from course_action_state.models import CourseRerunState from course_action_state.models import CourseRerunState
from contentstore.utils import initialize_permissions from contentstore.utils import initialize_permissions
from opaque_keys.edx.keys import CourseKey
@task() @task()
def rerun_course(source_course_key, destination_course_key, user_id, fields=None): def rerun_course(source_course_key_string, destination_course_key_string, user_id, fields=None):
""" """
Reruns a course in a new celery task. Reruns a course in a new celery task.
""" """
try: try:
modulestore().clone_course(source_course_key, destination_course_key, user_id, fields=fields) # deserialize the keys
source_course_key = CourseKey.from_string(source_course_key_string)
destination_course_key = CourseKey.from_string(destination_course_key_string)
# use the split modulestore as the store for the rerun course,
# as the Mongo modulestore doesn't support multiple runs of the same course.
store = modulestore()
with store.default_store('split'):
store.clone_course(source_course_key, destination_course_key, user_id, fields=fields)
# set initial permissions for the user to access the course. # set initial permissions for the user to access the course.
initialize_permissions(destination_course_key, User.objects.get(id=user_id)) initialize_permissions(destination_course_key, User.objects.get(id=user_id))
...@@ -23,10 +33,24 @@ def rerun_course(source_course_key, destination_course_key, user_id, fields=None ...@@ -23,10 +33,24 @@ def rerun_course(source_course_key, destination_course_key, user_id, fields=None
# update state: Succeeded # update state: Succeeded
CourseRerunState.objects.succeeded(course_key=destination_course_key) CourseRerunState.objects.succeeded(course_key=destination_course_key)
return "succeeded"
except DuplicateCourseError as exc:
# do NOT delete the original course, only update the status
CourseRerunState.objects.failed(course_key=destination_course_key, exception=exc)
return "duplicate course"
# catch all exceptions so we can update the state and properly cleanup the course. # catch all exceptions so we can update the state and properly cleanup the course.
except Exception as exc: # pylint: disable=broad-except except Exception as exc: # pylint: disable=broad-except
# update state: Failed # update state: Failed
CourseRerunState.objects.failed(course_key=destination_course_key, exception=exc) CourseRerunState.objects.failed(course_key=destination_course_key, exception=exc)
# cleanup any remnants of the course try:
modulestore().delete_course(destination_course_key, user_id) # cleanup any remnants of the course
modulestore().delete_course(destination_course_key, user_id)
except ItemNotFoundError:
# it's possible there was an error even before the course module was created
pass
return "exception: " + unicode(exc)
cms/envs/bok_choy.auth.json
...@@ -51,7 +51,6 @@ ...@@ -51,7 +51,6 @@
"ENGINE": "xmodule.modulestore.mixed.MixedModuleStore", "ENGINE": "xmodule.modulestore.mixed.MixedModuleStore",
"OPTIONS": { "OPTIONS": {
"mappings": {}, "mappings": {},
"reference_type": "Location",
"stores": [ "stores": [
{ {
"NAME": "draft", "NAME": "draft",
......
common/djangoapps/cache_toolbox/core.py
...@@ -119,13 +119,19 @@ def get_cached_content(location): ...@@ -119,13 +119,19 @@ def get_cached_content(location):
def del_cached_content(location): def del_cached_content(location):
# delete content for the given location, as well as for content with run=None. """
# it's possible that the content could have been cached without knowing the delete content for the given location, as well as for content with run=None.
# course_key - and so without having the run. it's possible that the content could have been cached without knowing the
course_key - and so without having the run.
"""
def location_str(loc):
return unicode(loc).encode("utf-8")
locations = [location_str(location)]
try: try:
cache.delete_many( locations.append(location_str(location.replace(run=None)))
[unicode(loc).encode("utf-8") for loc in [location, location.replace(run=None)]]
)
except InvalidKeyError: except InvalidKeyError:
# although deprecated keys allowed run=None, new keys don't if there is no version. # although deprecated keys allowed run=None, new keys don't if there is no version.
pass pass
cache.delete_many(locations)
common/djangoapps/user_api/middleware.py
...@@ -5,7 +5,7 @@ Adds user's tags to tracking event context. ...@@ -5,7 +5,7 @@ Adds user's tags to tracking event context.
from eventtracking import tracker from eventtracking import tracker
from opaque_keys import InvalidKeyError from opaque_keys import InvalidKeyError
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.keys import CourseKey
from track.contexts import COURSE_REGEX from track.contexts import COURSE_REGEX
from user_api.models import UserCourseTag from user_api.models import UserCourseTag
...@@ -24,7 +24,7 @@ class UserTagsEventContextMiddleware(object): ...@@ -24,7 +24,7 @@ class UserTagsEventContextMiddleware(object):
if match: if match:
course_id = match.group('course_id') course_id = match.group('course_id')
try: try:
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) course_key = CourseKey.from_string(course_id)
except InvalidKeyError: except InvalidKeyError:
course_id = None course_id = None
course_key = None course_key = None
......
common/djangoapps/xmodule_django/models.py
...@@ -2,6 +2,7 @@ from django.db import models ...@@ -2,6 +2,7 @@ from django.db import models
from django.core.exceptions import ValidationError from django.core.exceptions import ValidationError
from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.keys import CourseKey, UsageKey
from opaque_keys.edx.locator import Locator
from south.modelsinspector import add_introspection_rules from south.modelsinspector import add_introspection_rules
add_introspection_rules([], ["^xmodule_django\.models\.CourseKeyField"]) add_introspection_rules([], ["^xmodule_django\.models\.CourseKeyField"])
...@@ -44,6 +45,28 @@ class NoneToEmptyQuerySet(models.query.QuerySet): ...@@ -44,6 +45,28 @@ class NoneToEmptyQuerySet(models.query.QuerySet):
return super(NoneToEmptyQuerySet, self)._filter_or_exclude(*args, **kwargs) return super(NoneToEmptyQuerySet, self)._filter_or_exclude(*args, **kwargs)
def _strip_object(key):
"""
Strips branch and version info if the given key supports those attributes.
"""
if hasattr(key, 'version_agnostic') and hasattr(key, 'for_branch'):
return key.for_branch(None).version_agnostic()
else:
return key
def _strip_value(value, lookup='exact'):
"""
Helper function to remove the branch and version information from the given value,
which could be a single object or a list.
"""
if lookup == 'in':
stripped_value = [_strip_object(el) for el in value]
else:
stripped_value = _strip_object(value)
return stripped_value
class CourseKeyField(models.CharField): class CourseKeyField(models.CharField):
description = "A SlashSeparatedCourseKey object, saved to the DB in the form of a string" description = "A SlashSeparatedCourseKey object, saved to the DB in the form of a string"
...@@ -69,14 +92,18 @@ class CourseKeyField(models.CharField): ...@@ -69,14 +92,18 @@ class CourseKeyField(models.CharField):
if lookup == 'isnull': if lookup == 'isnull':
raise TypeError('Use CourseKeyField.Empty rather than None to query for a missing CourseKeyField') raise TypeError('Use CourseKeyField.Empty rather than None to query for a missing CourseKeyField')
return super(CourseKeyField, self).get_prep_lookup(lookup, value) return super(CourseKeyField, self).get_prep_lookup(
lookup,
# strip key before comparing
_strip_value(value, lookup)
)
def get_prep_value(self, value): def get_prep_value(self, value):
if value is self.Empty or value is None: if value is self.Empty or value is None:
return '' # CharFields should use '' as their empty value, rather than None return '' # CharFields should use '' as their empty value, rather than None
assert isinstance(value, CourseKey) assert isinstance(value, CourseKey)
return value.to_deprecated_string() return unicode(_strip_value(value))
def validate(self, value, model_instance): def validate(self, value, model_instance):
"""Validate Empty values, otherwise defer to the parent""" """Validate Empty values, otherwise defer to the parent"""
...@@ -119,14 +146,19 @@ class LocationKeyField(models.CharField): ...@@ -119,14 +146,19 @@ class LocationKeyField(models.CharField):
if lookup == 'isnull': if lookup == 'isnull':
raise TypeError('Use LocationKeyField.Empty rather than None to query for a missing LocationKeyField') raise TypeError('Use LocationKeyField.Empty rather than None to query for a missing LocationKeyField')
return super(LocationKeyField, self).get_prep_lookup(lookup, value) # remove version and branch info before comparing keys
return super(LocationKeyField, self).get_prep_lookup(
lookup,
# strip key before comparing
_strip_value(value, lookup)
)
def get_prep_value(self, value): def get_prep_value(self, value):
if value is self.Empty: if value is self.Empty:
return '' return ''
assert isinstance(value, UsageKey) assert isinstance(value, UsageKey)
return value.to_deprecated_string() return unicode(_strip_value(value))
def validate(self, value, model_instance): def validate(self, value, model_instance):
"""Validate Empty values, otherwise defer to the parent""" """Validate Empty values, otherwise defer to the parent"""
......
common/lib/xmodule/xmodule/contentstore/content.py
...@@ -103,7 +103,8 @@ class StaticContent(object): ...@@ -103,7 +103,8 @@ class StaticContent(object):
return None return None
assert(isinstance(course_key, CourseKey)) assert(isinstance(course_key, CourseKey))
# create a dummy asset location and then strip off the last character: 'a' # create a dummy asset location and then strip off the last character: 'a',
# since the AssetLocator rejects the empty string as a legal value for the block_id.
return course_key.make_asset_key('asset', 'a').for_branch(None).to_deprecated_string()[:-1] return course_key.make_asset_key('asset', 'a').for_branch(None).to_deprecated_string()[:-1]
@staticmethod @staticmethod
......
common/lib/xmodule/xmodule/contentstore/mongo.py
...@@ -95,7 +95,8 @@ class MongoContentStore(ContentStore): ...@@ -95,7 +95,8 @@ class MongoContentStore(ContentStore):
thumbnail_location = getattr(fp, 'thumbnail_location', None) thumbnail_location = getattr(fp, 'thumbnail_location', None)
if thumbnail_location: if thumbnail_location:
thumbnail_location = location.course_key.make_asset_key( thumbnail_location = location.course_key.make_asset_key(
'thumbnail', thumbnail_location[4] 'thumbnail',
thumbnail_location[4]
) )
return StaticContentStream( return StaticContentStream(
location, fp.displayname, fp.content_type, fp, last_modified_at=fp.uploadDate, location, fp.displayname, fp.content_type, fp, last_modified_at=fp.uploadDate,
...@@ -108,7 +109,8 @@ class MongoContentStore(ContentStore): ...@@ -108,7 +109,8 @@ class MongoContentStore(ContentStore):
thumbnail_location = getattr(fp, 'thumbnail_location', None) thumbnail_location = getattr(fp, 'thumbnail_location', None)
if thumbnail_location: if thumbnail_location:
thumbnail_location = location.course_key.make_asset_key( thumbnail_location = location.course_key.make_asset_key(
'thumbnail', thumbnail_location[4] 'thumbnail',
thumbnail_location[4]
) )
return StaticContent( return StaticContent(
location, fp.displayname, fp.content_type, fp.read(), last_modified_at=fp.uploadDate, location, fp.displayname, fp.content_type, fp.read(), last_modified_at=fp.uploadDate,
......
common/lib/xmodule/xmodule/modulestore/__init__.py
...@@ -116,7 +116,7 @@ class ModuleStoreRead(object): ...@@ -116,7 +116,7 @@ class ModuleStoreRead(object):
pass pass
@abstractmethod @abstractmethod
def get_item(self, usage_key, depth=0): def get_item(self, usage_key, depth=0, **kwargs):
""" """
Returns an XModuleDescriptor instance for the item at location. Returns an XModuleDescriptor instance for the item at location.
...@@ -150,7 +150,7 @@ class ModuleStoreRead(object): ...@@ -150,7 +150,7 @@ class ModuleStoreRead(object):
pass pass
@abstractmethod @abstractmethod
def get_items(self, location, course_id=None, depth=0, qualifiers=None): def get_items(self, location, course_id=None, depth=0, qualifiers=None, **kwargs):
""" """
Returns a list of XModuleDescriptor instances for the items Returns a list of XModuleDescriptor instances for the items
that match location. Any element of location that is None is treated that match location. Any element of location that is None is treated
...@@ -202,6 +202,9 @@ class ModuleStoreRead(object): ...@@ -202,6 +202,9 @@ class ModuleStoreRead(object):
return True, field return True, field
for key, criteria in qualifiers.iteritems(): for key, criteria in qualifiers.iteritems():
if callable(criteria):
# skip over any optional fields that are functions
continue
is_set, value = _is_set_on(key) is_set, value = _is_set_on(key)
if not is_set: if not is_set:
return False return False
...@@ -228,7 +231,17 @@ class ModuleStoreRead(object): ...@@ -228,7 +231,17 @@ class ModuleStoreRead(object):
return criteria == target return criteria == target
@abstractmethod @abstractmethod
def get_courses(self): def make_course_key(self, org, course, run):
"""
Return a valid :class:`~opaque_keys.edx.keys.CourseKey` for this modulestore
that matches the supplied `org`, `course`, and `run`.
This key may represent a course that doesn't exist in this modulestore.
"""
pass
@abstractmethod
def get_courses(self, **kwargs):
''' '''
Returns a list containing the top level XModuleDescriptors of the courses Returns a list containing the top level XModuleDescriptors of the courses
in this modulestore. in this modulestore.
...@@ -236,7 +249,7 @@ class ModuleStoreRead(object): ...@@ -236,7 +249,7 @@ class ModuleStoreRead(object):
pass pass
@abstractmethod @abstractmethod
def get_course(self, course_id, depth=0): def get_course(self, course_id, depth=0, **kwargs):
''' '''
Look for a specific course by its id (:class:`CourseKey`). Look for a specific course by its id (:class:`CourseKey`).
Returns the course descriptor, or None if not found. Returns the course descriptor, or None if not found.
...@@ -244,7 +257,7 @@ class ModuleStoreRead(object): ...@@ -244,7 +257,7 @@ class ModuleStoreRead(object):
pass pass
@abstractmethod @abstractmethod
def has_course(self, course_id, ignore_case=False): def has_course(self, course_id, ignore_case=False, **kwargs):
''' '''
Look for a specific course id. Returns whether it exists. Look for a specific course id. Returns whether it exists.
Args: Args:
...@@ -256,13 +269,14 @@ class ModuleStoreRead(object): ...@@ -256,13 +269,14 @@ class ModuleStoreRead(object):
@abstractmethod @abstractmethod
def get_parent_location(self, location, **kwargs): def get_parent_location(self, location, **kwargs):
'''Find the location that is the parent of this location in this '''
Find the location that is the parent of this location in this
course. Needed for path_to_location(). course. Needed for path_to_location().
''' '''
pass pass
@abstractmethod @abstractmethod
def get_orphans(self, course_key): def get_orphans(self, course_key, **kwargs):
""" """
Get all of the xblocks in the given course which have no parents and are not of types which are 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 usually orphaned. NOTE: may include xblocks which still have references via xblocks which don't
...@@ -287,7 +301,7 @@ class ModuleStoreRead(object): ...@@ -287,7 +301,7 @@ class ModuleStoreRead(object):
pass pass
@abstractmethod @abstractmethod
def get_courses_for_wiki(self, wiki_slug): def get_courses_for_wiki(self, wiki_slug, **kwargs):
""" """
Return the list of courses which use this wiki_slug Return the list of courses which use this wiki_slug
:param wiki_slug: the course wiki root slug :param wiki_slug: the course wiki root slug
...@@ -325,7 +339,7 @@ class ModuleStoreWrite(ModuleStoreRead): ...@@ -325,7 +339,7 @@ class ModuleStoreWrite(ModuleStoreRead):
__metaclass__ = ABCMeta __metaclass__ = ABCMeta
@abstractmethod @abstractmethod
def update_item(self, xblock, user_id, allow_not_found=False, force=False): def update_item(self, xblock, user_id, allow_not_found=False, force=False, **kwargs):
""" """
Update the given xblock's persisted repr. Pass the user's unique id which the persistent store Update the given xblock's persisted repr. Pass the user's unique id which the persistent store
should save with the update if it has that ability. should save with the update if it has that ability.
...@@ -413,7 +427,7 @@ class ModuleStoreWrite(ModuleStoreRead): ...@@ -413,7 +427,7 @@ class ModuleStoreWrite(ModuleStoreRead):
pass pass
@abstractmethod @abstractmethod
def delete_course(self, course_key, user_id): def delete_course(self, course_key, user_id, **kwargs):
""" """
Deletes the course. It may be a soft or hard delete. It may or may not remove the xblock definitions Deletes the course. It may be a soft or hard delete. It may or may not remove the xblock definitions
depending on the persistence layer and how tightly bound the xblocks are to the course. depending on the persistence layer and how tightly bound the xblocks are to the course.
...@@ -480,19 +494,19 @@ class ModuleStoreReadBase(ModuleStoreRead): ...@@ -480,19 +494,19 @@ class ModuleStoreReadBase(ModuleStoreRead):
""" """
return {} return {}
def get_course(self, course_id, depth=0): def get_course(self, course_id, depth=0, **kwargs):
""" """
See ModuleStoreRead.get_course See ModuleStoreRead.get_course
Default impl--linear search through course list Default impl--linear search through course list
""" """
assert(isinstance(course_id, CourseKey)) assert(isinstance(course_id, CourseKey))
for course in self.get_courses(): for course in self.get_courses(**kwargs):
if course.id == course_id: if course.id == course_id:
return course return course
return None return None
def has_course(self, course_id, ignore_case=False): def has_course(self, course_id, ignore_case=False, **kwargs):
""" """
Returns the course_id of the course if it was found, else None Returns the course_id of the course if it was found, else None
Args: Args:
...@@ -577,7 +591,7 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): ...@@ -577,7 +591,7 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
result[field.scope][field_name] = value result[field.scope][field_name] = value
return result return result
def clone_course(self, source_course_id, dest_course_id, user_id, fields=None): def clone_course(self, source_course_id, dest_course_id, user_id, fields=None, **kwargs):
""" """
This base method just copies the assets. The lower level impls must do the actual cloning of This base method just copies the assets. The lower level impls must do the actual cloning of
content. content.
...@@ -587,7 +601,7 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): ...@@ -587,7 +601,7 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
self.contentstore.copy_all_course_assets(source_course_id, dest_course_id) self.contentstore.copy_all_course_assets(source_course_id, dest_course_id)
return dest_course_id return dest_course_id
def delete_course(self, course_key, user_id): def delete_course(self, course_key, user_id, **kwargs):
""" """
This base method just deletes the assets. The lower level impls must do the actual deleting of This base method just deletes the assets. The lower level impls must do the actual deleting of
content. content.
......
common/lib/xmodule/xmodule/modulestore/modulestore_settings.py
...@@ -43,7 +43,6 @@ def convert_module_store_setting_if_needed(module_store_setting): ...@@ -43,7 +43,6 @@ def convert_module_store_setting_if_needed(module_store_setting):
"ENGINE": "xmodule.modulestore.mixed.MixedModuleStore", "ENGINE": "xmodule.modulestore.mixed.MixedModuleStore",
"OPTIONS": { "OPTIONS": {
"mappings": {}, "mappings": {},
"reference_type": "Location",
"stores": [] "stores": []
} }
} }
...@@ -70,12 +69,12 @@ def convert_module_store_setting_if_needed(module_store_setting): ...@@ -70,12 +69,12 @@ def convert_module_store_setting_if_needed(module_store_setting):
# If Split is not defined but the DraftMongoModuleStore is configured, add Split as a copy of Draft # If Split is not defined but the DraftMongoModuleStore is configured, add Split as a copy of Draft
mixed_stores = module_store_setting['default']['OPTIONS']['stores'] mixed_stores = module_store_setting['default']['OPTIONS']['stores']
is_split_defined = any(('DraftVersioningModuleStore' in store['ENGINE']) for store in mixed_stores) is_split_defined = any((store['ENGINE'].endswith('.DraftVersioningModuleStore')) for store in mixed_stores)
if not is_split_defined: if not is_split_defined:
# find first setting of mongo store # find first setting of mongo store
mongo_store = next( mongo_store = next(
(store for store in mixed_stores if ( (store for store in mixed_stores if (
'DraftMongoModuleStore' in store['ENGINE'] or 'DraftModuleStore' in store['ENGINE'] store['ENGINE'].endswith('.DraftMongoModuleStore') or store['ENGINE'].endswith('.DraftModuleStore')
)), )),
None None
) )
......
common/lib/xmodule/xmodule/modulestore/mongo/base.py
...@@ -37,10 +37,11 @@ from xblock.fields import Scope, ScopeIds, Reference, ReferenceList, ReferenceVa ...@@ -37,10 +37,11 @@ from xblock.fields import Scope, ScopeIds, Reference, ReferenceList, ReferenceVa
from xmodule.modulestore import ModuleStoreWriteBase, ModuleStoreEnum from xmodule.modulestore import ModuleStoreWriteBase, ModuleStoreEnum
from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished, DIRECT_ONLY_CATEGORIES from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished, DIRECT_ONLY_CATEGORIES
from opaque_keys.edx.locations import Location from opaque_keys.edx.locations import Location
from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, ReferentialIntegrityError from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError, ReferentialIntegrityError
from xmodule.modulestore.inheritance import own_metadata, InheritanceMixin, inherit_metadata, InheritanceKeyValueStore from xmodule.modulestore.inheritance import own_metadata, InheritanceMixin, inherit_metadata, InheritanceKeyValueStore
from xblock.core import XBlock from xblock.core import XBlock
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
from opaque_keys.edx.locator import CourseLocator
from opaque_keys.edx.keys import UsageKey, CourseKey from opaque_keys.edx.keys import UsageKey, CourseKey
from xmodule.exceptions import HeartbeatFailure from xmodule.exceptions import HeartbeatFailure
...@@ -354,8 +355,6 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): ...@@ -354,8 +355,6 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
""" """
A Mongodb backed ModuleStore A Mongodb backed ModuleStore
""" """
reference_type = SlashSeparatedCourseKey
# TODO (cpennington): Enable non-filesystem filestores # TODO (cpennington): Enable non-filesystem filestores
# pylint: disable=C0103 # pylint: disable=C0103
# pylint: disable=W0201 # pylint: disable=W0201
...@@ -716,7 +715,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): ...@@ -716,7 +715,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
for item in items for item in items
] ]
def get_courses(self): def get_courses(self, **kwargs):
''' '''
Returns a list of course descriptors. Returns a list of course descriptors.
''' '''
...@@ -751,7 +750,16 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): ...@@ -751,7 +750,16 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
raise ItemNotFoundError(location) raise ItemNotFoundError(location)
return item return item
def get_course(self, course_key, depth=0): def make_course_key(self, org, course, run):
"""
Return a valid :class:`~opaque_keys.edx.keys.CourseKey` for this modulestore
that matches the supplied `org`, `course`, and `run`.
This key may represent a course that doesn't exist in this modulestore.
"""
return CourseLocator(org, course, run, deprecated=True)
def get_course(self, course_key, depth=0, **kwargs):
""" """
Get the course with the given courseid (org/course/run) Get the course with the given courseid (org/course/run)
""" """
...@@ -763,7 +771,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): ...@@ -763,7 +771,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
except ItemNotFoundError: except ItemNotFoundError:
return None return None
def has_course(self, course_key, ignore_case=False): def has_course(self, course_key, ignore_case=False, **kwargs):
""" """
Returns the course_id of the course if it was found, else None Returns the course_id of the course if it was found, else None
Note: we return the course_id instead of a boolean here since the found course may have Note: we return the course_id instead of a boolean here since the found course may have
...@@ -882,7 +890,10 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): ...@@ -882,7 +890,10 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
if 'children' in kwargs: if 'children' in kwargs:
query['definition.children'] = kwargs.pop('children') query['definition.children'] = kwargs.pop('children')
query.update(kwargs) # remove any callable kwargs for qualifiers
qualifiers = {key: val for key, val in kwargs.iteritems() if not callable(val)}
query.update(qualifiers)
items = self.collection.find( items = self.collection.find(
query, query,
sort=[SORT_REVISION_FAVOR_DRAFT], sort=[SORT_REVISION_FAVOR_DRAFT],
...@@ -919,10 +930,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): ...@@ -919,10 +930,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
]) ])
courses = self.collection.find(course_search_location, fields=('_id')) courses = self.collection.find(course_search_location, fields=('_id'))
if courses.count() > 0: if courses.count() > 0:
raise InvalidLocationError( raise DuplicateCourseError(course_id, courses[0]['_id'])
"There are already courses with the given org and course id: {}".format([
course['_id'] for course in courses
]))
location = course_id.make_usage_key('course', course_id.run) location = course_id.make_usage_key('course', course_id.run)
course = self.create_xmodule( course = self.create_xmodule(
...@@ -1253,7 +1261,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): ...@@ -1253,7 +1261,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
""" """
return ModuleStoreEnum.Type.mongo return ModuleStoreEnum.Type.mongo
def get_orphans(self, course_key): def get_orphans(self, course_key, **kwargs):
""" """
Return an array of all of the locations for orphans in the course. Return an array of all of the locations for orphans in the course.
""" """
...@@ -1274,7 +1282,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): ...@@ -1274,7 +1282,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
item_locs -= all_reachable item_locs -= all_reachable
return [course_key.make_usage_key_from_deprecated_string(item_loc) for item_loc in item_locs] return [course_key.make_usage_key_from_deprecated_string(item_loc) for item_loc in item_locs]
def get_courses_for_wiki(self, wiki_slug): def get_courses_for_wiki(self, wiki_slug, **kwargs):
""" """
Return the list of courses which use this wiki_slug Return the list of courses which use this wiki_slug
:param wiki_slug: the course wiki root slug :param wiki_slug: the course wiki root slug
......
common/lib/xmodule/xmodule/modulestore/mongo/draft.py
...@@ -47,7 +47,7 @@ class DraftModuleStore(MongoModuleStore): ...@@ -47,7 +47,7 @@ class DraftModuleStore(MongoModuleStore):
This module also includes functionality to promote DRAFT modules (and their children) This module also includes functionality to promote DRAFT modules (and their children)
to published modules. to published modules.
""" """
def get_item(self, usage_key, depth=0, revision=None): def get_item(self, usage_key, depth=0, revision=None, **kwargs):
""" """
Returns an XModuleDescriptor instance for the item at usage_key. Returns an XModuleDescriptor instance for the item at usage_key.
...@@ -155,7 +155,7 @@ class DraftModuleStore(MongoModuleStore): ...@@ -155,7 +155,7 @@ class DraftModuleStore(MongoModuleStore):
course_query = self._course_key_to_son(course_key) course_query = self._course_key_to_son(course_key)
self.collection.remove(course_query, multi=True) self.collection.remove(course_query, multi=True)
def clone_course(self, source_course_id, dest_course_id, user_id, fields=None): def clone_course(self, source_course_id, dest_course_id, user_id, fields=None, **kwargs):
""" """
Only called if cloning within this store or if env doesn't set up mixed. Only called if cloning within this store or if env doesn't set up mixed.
* copy the courseware * copy the courseware
...@@ -439,7 +439,7 @@ class DraftModuleStore(MongoModuleStore): ...@@ -439,7 +439,7 @@ class DraftModuleStore(MongoModuleStore):
# convert the subtree using the original item as the root # convert the subtree using the original item as the root
self._breadth_first(convert_item, [location]) self._breadth_first(convert_item, [location])
def update_item(self, xblock, user_id, allow_not_found=False, force=False, isPublish=False): def update_item(self, xblock, user_id, allow_not_found=False, force=False, isPublish=False, **kwargs):
""" """
See superclass doc. See superclass doc.
In addition to the superclass's behavior, this method converts the unit to draft if it's not In addition to the superclass's behavior, this method converts the unit to draft if it's not
...@@ -616,7 +616,7 @@ class DraftModuleStore(MongoModuleStore): ...@@ -616,7 +616,7 @@ class DraftModuleStore(MongoModuleStore):
else: else:
return False return False
def publish(self, location, user_id): def publish(self, location, user_id, **kwargs):
""" """
Publish the subtree rooted at location to the live course and remove the drafts. Publish the subtree rooted at location to the live course and remove the drafts.
Such publishing may cause the deletion of previously published but subsequently deleted Such publishing may cause the deletion of previously published but subsequently deleted
...@@ -690,7 +690,7 @@ class DraftModuleStore(MongoModuleStore): ...@@ -690,7 +690,7 @@ class DraftModuleStore(MongoModuleStore):
self.collection.remove({'_id': {'$in': to_be_deleted}}) self.collection.remove({'_id': {'$in': to_be_deleted}})
return self.get_item(as_published(location)) return self.get_item(as_published(location))
def unpublish(self, location, user_id): def unpublish(self, location, user_id, **kwargs):
""" """
Turn the published version into a draft, removing the published version. Turn the published version into a draft, removing the published version.
......
common/lib/xmodule/xmodule/modulestore/split_migrator.py
...@@ -25,7 +25,9 @@ class SplitMigrator(object): ...@@ -25,7 +25,9 @@ class SplitMigrator(object):
self.split_modulestore = split_modulestore self.split_modulestore = split_modulestore
self.source_modulestore = source_modulestore self.source_modulestore = source_modulestore
def migrate_mongo_course(self, source_course_key, user_id, new_org=None, new_course=None, new_run=None, fields=None): def migrate_mongo_course(
self, source_course_key, user_id, new_org=None, new_course=None, new_run=None, fields=None, **kwargs
):
""" """
Create a new course in split_mongo representing the published and draft versions of the course from the Create a new course in split_mongo representing the published and draft versions of the course from the
original mongo store. And return the new CourseLocator original mongo store. And return the new CourseLocator
...@@ -43,7 +45,7 @@ class SplitMigrator(object): ...@@ -43,7 +45,7 @@ class SplitMigrator(object):
# locations are in location, children, conditionals, course.tab # locations are in location, children, conditionals, course.tab
# create the course: set fields to explicitly_set for each scope, id_root = new_course_locator, master_branch = 'production' # create the course: set fields to explicitly_set for each scope, id_root = new_course_locator, master_branch = 'production'
original_course = self.source_modulestore.get_course(source_course_key) original_course = self.source_modulestore.get_course(source_course_key, **kwargs)
if new_org is None: if new_org is None:
new_org = source_course_key.org new_org = source_course_key.org
...@@ -60,17 +62,20 @@ class SplitMigrator(object): ...@@ -60,17 +62,20 @@ class SplitMigrator(object):
new_org, new_course, new_run, user_id, new_org, new_course, new_run, user_id,
fields=new_fields, fields=new_fields,
master_branch=ModuleStoreEnum.BranchName.published, master_branch=ModuleStoreEnum.BranchName.published,
**kwargs
) )
with self.split_modulestore.bulk_write_operations(new_course.id): with self.split_modulestore.bulk_write_operations(new_course.id):
self._copy_published_modules_to_course(new_course, original_course.location, source_course_key, user_id) self._copy_published_modules_to_course(
new_course, original_course.location, source_course_key, user_id, **kwargs
)
# create a new version for the drafts # create a new version for the drafts
with self.split_modulestore.bulk_write_operations(new_course.id): with self.split_modulestore.bulk_write_operations(new_course.id):
self._add_draft_modules_to_course(new_course.location, source_course_key, user_id) self._add_draft_modules_to_course(new_course.location, source_course_key, user_id, **kwargs)
return new_course.id return new_course.id
def _copy_published_modules_to_course(self, new_course, old_course_loc, source_course_key, user_id): def _copy_published_modules_to_course(self, new_course, old_course_loc, source_course_key, user_id, **kwargs):
""" """
Copy all of the modules from the 'direct' version of the course to the new split course. Copy all of the modules from the 'direct' version of the course to the new split course.
""" """
...@@ -79,7 +84,7 @@ class SplitMigrator(object): ...@@ -79,7 +84,7 @@ class SplitMigrator(object):
# iterate over published course elements. Wildcarding rather than descending b/c some elements are orphaned (e.g., # iterate over published course elements. Wildcarding rather than descending b/c some elements are orphaned (e.g.,
# course about pages, conditionals) # course about pages, conditionals)
for module in self.source_modulestore.get_items( for module in self.source_modulestore.get_items(
source_course_key, revision=ModuleStoreEnum.RevisionOption.published_only source_course_key, revision=ModuleStoreEnum.RevisionOption.published_only, **kwargs
): ):
# don't copy the course again. # don't copy the course again.
if module.location != old_course_loc: if module.location != old_course_loc:
...@@ -95,7 +100,8 @@ class SplitMigrator(object): ...@@ -95,7 +100,8 @@ class SplitMigrator(object):
fields=self._get_fields_translate_references( fields=self._get_fields_translate_references(
module, course_version_locator, new_course.location.block_id module, course_version_locator, new_course.location.block_id
), ),
continue_version=True continue_version=True,
**kwargs
) )
# after done w/ published items, add version for DRAFT pointing to the published structure # after done w/ published items, add version for DRAFT pointing to the published structure
index_info = self.split_modulestore.get_course_index_info(course_version_locator) index_info = self.split_modulestore.get_course_index_info(course_version_locator)
...@@ -107,7 +113,7 @@ class SplitMigrator(object): ...@@ -107,7 +113,7 @@ class SplitMigrator(object):
# children which meant some pointers were to non-existent locations in 'direct' # children which meant some pointers were to non-existent locations in 'direct'
self.split_modulestore.internal_clean_children(course_version_locator) self.split_modulestore.internal_clean_children(course_version_locator)
def _add_draft_modules_to_course(self, published_course_usage_key, source_course_key, user_id): def _add_draft_modules_to_course(self, published_course_usage_key, source_course_key, user_id, **kwargs):
""" """
update each draft. Create any which don't exist in published and attach to their parents. update each draft. Create any which don't exist in published and attach to their parents.
""" """
...@@ -117,11 +123,13 @@ class SplitMigrator(object): ...@@ -117,11 +123,13 @@ class SplitMigrator(object):
# to prevent race conditions of grandchilden being added before their parents and thus having no parent to # to prevent race conditions of grandchilden being added before their parents and thus having no parent to
# add to # add to
awaiting_adoption = {} awaiting_adoption = {}
for module in self.source_modulestore.get_items(source_course_key, revision=ModuleStoreEnum.RevisionOption.draft_only): for module in self.source_modulestore.get_items(
source_course_key, revision=ModuleStoreEnum.RevisionOption.draft_only, **kwargs
):
new_locator = new_draft_course_loc.make_usage_key(module.category, module.location.block_id) new_locator = new_draft_course_loc.make_usage_key(module.category, module.location.block_id)
if self.split_modulestore.has_item(new_locator): if self.split_modulestore.has_item(new_locator):
# was in 'direct' so draft is a new version # was in 'direct' so draft is a new version
split_module = self.split_modulestore.get_item(new_locator) split_module = self.split_modulestore.get_item(new_locator, **kwargs)
# need to remove any no-longer-explicitly-set values and add/update any now set values. # need to remove any no-longer-explicitly-set values and add/update any now set values.
for name, field in split_module.fields.iteritems(): for name, field in split_module.fields.iteritems():
if field.is_set_on(split_module) and not module.fields[name].is_set_on(module): if field.is_set_on(split_module) and not module.fields[name].is_set_on(module):
...@@ -131,7 +139,7 @@ class SplitMigrator(object): ...@@ -131,7 +139,7 @@ class SplitMigrator(object):
).iteritems(): ).iteritems():
field.write_to(split_module, value) field.write_to(split_module, value)
_new_module = self.split_modulestore.update_item(split_module, user_id) _new_module = self.split_modulestore.update_item(split_module, user_id, **kwargs)
else: else:
# only a draft version (aka, 'private'). # only a draft version (aka, 'private').
_new_module = self.split_modulestore.create_item( _new_module = self.split_modulestore.create_item(
...@@ -140,22 +148,23 @@ class SplitMigrator(object): ...@@ -140,22 +148,23 @@ class SplitMigrator(object):
block_id=new_locator.block_id, block_id=new_locator.block_id,
fields=self._get_fields_translate_references( fields=self._get_fields_translate_references(
module, new_draft_course_loc, published_course_usage_key.block_id module, new_draft_course_loc, published_course_usage_key.block_id
) ),
**kwargs
) )
awaiting_adoption[module.location] = new_locator awaiting_adoption[module.location] = new_locator
for draft_location, new_locator in awaiting_adoption.iteritems(): for draft_location, new_locator in awaiting_adoption.iteritems():
parent_loc = self.source_modulestore.get_parent_location( parent_loc = self.source_modulestore.get_parent_location(
draft_location, revision=ModuleStoreEnum.RevisionOption.draft_preferred draft_location, revision=ModuleStoreEnum.RevisionOption.draft_preferred, **kwargs
) )
if parent_loc is None: if parent_loc is None:
log.warn(u'No parent found in source course for %s', draft_location) log.warn(u'No parent found in source course for %s', draft_location)
continue continue
old_parent = self.source_modulestore.get_item(parent_loc) old_parent = self.source_modulestore.get_item(parent_loc, **kwargs)
split_parent_loc = new_draft_course_loc.make_usage_key( split_parent_loc = new_draft_course_loc.make_usage_key(
parent_loc.category, parent_loc.category,
parent_loc.block_id if parent_loc.category != 'course' else published_course_usage_key.block_id parent_loc.block_id if parent_loc.category != 'course' else published_course_usage_key.block_id
) )
new_parent = self.split_modulestore.get_item(split_parent_loc) new_parent = self.split_modulestore.get_item(split_parent_loc, **kwargs)
# this only occurs if the parent was also awaiting adoption: skip this one, go to next # this only occurs if the parent was also awaiting adoption: skip this one, go to next
if any(new_locator == child.version_agnostic() for child in new_parent.children): if any(new_locator == child.version_agnostic() for child in new_parent.children):
continue continue
......
common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py
...@@ -53,7 +53,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): ...@@ -53,7 +53,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
self.default_class = default_class self.default_class = default_class
self.local_modules = {} self.local_modules = {}
def _load_item(self, block_id, course_entry_override=None): def _load_item(self, block_id, course_entry_override=None, **kwargs):
if isinstance(block_id, BlockUsageLocator): if isinstance(block_id, BlockUsageLocator):
if isinstance(block_id.block_id, LocalId): if isinstance(block_id.block_id, LocalId):
try: try:
...@@ -77,7 +77,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): ...@@ -77,7 +77,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
raise ItemNotFoundError(block_id) raise ItemNotFoundError(block_id)
class_ = self.load_block_type(json_data.get('category')) class_ = self.load_block_type(json_data.get('category'))
return self.xblock_from_json(class_, block_id, json_data, course_entry_override) return self.xblock_from_json(class_, block_id, json_data, course_entry_override, **kwargs)
# xblock's runtime does not always pass enough contextual information to figure out # xblock's runtime does not always pass enough contextual information to figure out
# which named container (course x branch) or which parent is requesting an item. Because split allows # which named container (course x branch) or which parent is requesting an item. Because split allows
...@@ -90,7 +90,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): ...@@ -90,7 +90,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
# low; thus, the course_entry is most likely correct. If the thread is looking at > 1 named container # low; thus, the course_entry is most likely correct. If the thread is looking at > 1 named container
# pointing to the same structure, the access is likely to be chunky enough that the last known container # pointing to the same structure, the access is likely to be chunky enough that the last known container
# is the intended one when not given a course_entry_override; thus, the caching of the last branch/course id. # is the intended one when not given a course_entry_override; thus, the caching of the last branch/course id.
def xblock_from_json(self, class_, block_id, json_data, course_entry_override=None): def xblock_from_json(self, class_, block_id, json_data, course_entry_override=None, **kwargs):
if course_entry_override is None: if course_entry_override is None:
course_entry_override = self.course_entry course_entry_override = self.course_entry
else: else:
...@@ -126,6 +126,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): ...@@ -126,6 +126,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
definition, definition,
converted_fields, converted_fields,
json_data.get('_inherited_settings'), json_data.get('_inherited_settings'),
**kwargs
) )
field_data = KvsFieldData(kvs) field_data = KvsFieldData(kvs)
...@@ -151,8 +152,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem): ...@@ -151,8 +152,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
edit_info = json_data.get('edit_info', {}) edit_info = json_data.get('edit_info', {})
module.edited_by = edit_info.get('edited_by') module.edited_by = edit_info.get('edited_by')
module.edited_on = edit_info.get('edited_on') module.edited_on = edit_info.get('edited_on')
module.published_by = None # TODO module.published_by = None # TODO - addressed with LMS-11184
module.published_date = None # TODO module.published_date = None # TODO - addressed with LMS-11184
module.previous_version = edit_info.get('previous_version') module.previous_version = edit_info.get('previous_version')
module.update_version = edit_info.get('update_version') module.update_version = edit_info.get('update_version')
module.source_version = edit_info.get('source_version', None) module.source_version = edit_info.get('source_version', None)
......
common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py
...@@ -48,16 +48,22 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS ...@@ -48,16 +48,22 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
item = super(DraftVersioningModuleStore, self).create_course( item = super(DraftVersioningModuleStore, self).create_course(
org, course, run, user_id, master_branch=master_branch, **kwargs org, course, run, user_id, master_branch=master_branch, **kwargs
) )
self._auto_publish_no_children(item.location, item.location.category, user_id) self._auto_publish_no_children(item.location, item.location.category, user_id, **kwargs)
return item return item
def get_courses(self): def get_courses(self, **kwargs):
""" """
Returns all the courses on the Draft branch (which is a superset of the courses on the Published branch). Returns all the courses on the Draft or Published branch depending on the branch setting.
""" """
return super(DraftVersioningModuleStore, self).get_courses(ModuleStoreEnum.BranchName.draft) branch_setting = self.get_branch_setting()
if branch_setting == ModuleStoreEnum.Branch.draft_preferred:
return super(DraftVersioningModuleStore, self).get_courses(ModuleStoreEnum.BranchName.draft, **kwargs)
elif branch_setting == ModuleStoreEnum.Branch.published_only:
return super(DraftVersioningModuleStore, self).get_courses(ModuleStoreEnum.BranchName.published, **kwargs)
else:
raise InsufficientSpecificationError()
def _auto_publish_no_children(self, location, category, user_id): def _auto_publish_no_children(self, location, category, user_id, **kwargs):
""" """
Publishes item if the category is DIRECT_ONLY. This assumes another method has checked that Publishes item if the category is DIRECT_ONLY. This assumes another method has checked that
location points to the head of the branch and ignores the version. If you call this in any location points to the head of the branch and ignores the version. If you call this in any
...@@ -66,16 +72,17 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS ...@@ -66,16 +72,17 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
""" """
if location.branch == ModuleStoreEnum.BranchName.draft and category in DIRECT_ONLY_CATEGORIES: if location.branch == ModuleStoreEnum.BranchName.draft and category in DIRECT_ONLY_CATEGORIES:
# version_agnostic b/c of above assumption in docstring # version_agnostic b/c of above assumption in docstring
self.publish(location.version_agnostic(), user_id, blacklist=EXCLUDE_ALL) self.publish(location.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs)
def update_item(self, descriptor, user_id, allow_not_found=False, force=False): def update_item(self, descriptor, user_id, allow_not_found=False, force=False, **kwargs):
item = super(DraftVersioningModuleStore, self).update_item( item = super(DraftVersioningModuleStore, self).update_item(
descriptor, descriptor,
user_id, user_id,
allow_not_found=allow_not_found, allow_not_found=allow_not_found,
force=force force=force,
**kwargs
) )
self._auto_publish_no_children(item.location, item.location.category, user_id) self._auto_publish_no_children(item.location, item.location.category, user_id, **kwargs)
return item return item
def create_item( def create_item(
...@@ -88,7 +95,7 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS ...@@ -88,7 +95,7 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
definition_locator=definition_locator, fields=fields, definition_locator=definition_locator, fields=fields,
force=force, continue_version=continue_version, **kwargs force=force, continue_version=continue_version, **kwargs
) )
self._auto_publish_no_children(item.location, item.location.category, user_id) self._auto_publish_no_children(item.location, item.location.category, user_id, **kwargs)
return item return item
def create_child( def create_child(
...@@ -99,7 +106,7 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS ...@@ -99,7 +106,7 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
user_id, parent_usage_key, block_type, block_id=block_id, user_id, parent_usage_key, block_type, block_id=block_id,
fields=fields, continue_version=continue_version, **kwargs fields=fields, continue_version=continue_version, **kwargs
) )
self._auto_publish_no_children(parent_usage_key, item.location.category, user_id) self._auto_publish_no_children(parent_usage_key, item.location.category, user_id, **kwargs)
return item return item
def delete_item(self, location, user_id, revision=None, **kwargs): def delete_item(self, location, user_id, revision=None, **kwargs):
...@@ -134,8 +141,8 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS ...@@ -134,8 +141,8 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
for branch in branches_to_delete: for branch in branches_to_delete:
branched_location = location.for_branch(branch) branched_location = location.for_branch(branch)
parent_loc = self.get_parent_location(branched_location) parent_loc = self.get_parent_location(branched_location)
SplitMongoModuleStore.delete_item(self, branched_location, user_id, **kwargs) SplitMongoModuleStore.delete_item(self, branched_location, user_id)
self._auto_publish_no_children(parent_loc, parent_loc.category, user_id) self._auto_publish_no_children(parent_loc, parent_loc.category, user_id, **kwargs)
def _map_revision_to_branch(self, key, revision=None): def _map_revision_to_branch(self, key, revision=None):
""" """
...@@ -157,12 +164,12 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS ...@@ -157,12 +164,12 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
usage_key = self._map_revision_to_branch(usage_key, revision=revision) usage_key = self._map_revision_to_branch(usage_key, revision=revision)
return super(DraftVersioningModuleStore, self).has_item(usage_key) return super(DraftVersioningModuleStore, self).has_item(usage_key)
def get_item(self, usage_key, depth=0, revision=None): def get_item(self, usage_key, depth=0, revision=None, **kwargs):
""" """
Returns the item identified by usage_key and revision. Returns the item identified by usage_key and revision.
""" """
usage_key = self._map_revision_to_branch(usage_key, revision=revision) usage_key = self._map_revision_to_branch(usage_key, revision=revision)
return super(DraftVersioningModuleStore, self).get_item(usage_key, depth=depth) return super(DraftVersioningModuleStore, self).get_item(usage_key, depth=depth, **kwargs)
def get_items(self, course_locator, settings=None, content=None, revision=None, **kwargs): def get_items(self, course_locator, settings=None, content=None, revision=None, **kwargs):
""" """
...@@ -200,14 +207,18 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS ...@@ -200,14 +207,18 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
:param xblock: the block to check :param xblock: the block to check
:return: True if the draft and published versions differ :return: True if the draft and published versions differ
""" """
# TODO for better performance: lookup the courses and get the block entry, don't create the instances def get_block(branch_name):
draft = self.get_item(xblock.location.for_branch(ModuleStoreEnum.BranchName.draft)) course_structure = self._lookup_course(xblock.location.for_branch(branch_name))['structure']
try: return self._get_block_from_structure(course_structure, xblock.location.block_id)
published = self.get_item(xblock.location.for_branch(ModuleStoreEnum.BranchName.published))
except ItemNotFoundError: draft_block = get_block(ModuleStoreEnum.BranchName.draft)
published_block = get_block(ModuleStoreEnum.BranchName.published)
if not published_block:
return True return True
return draft.update_version != published.source_version # check if the draft has changed since the published was created
return draft_block['edit_info']['update_version'] != published_block['edit_info']['source_version']
def publish(self, location, user_id, blacklist=None, **kwargs): def publish(self, location, user_id, blacklist=None, **kwargs):
""" """
...@@ -224,15 +235,15 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS ...@@ -224,15 +235,15 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
[location], [location],
blacklist=blacklist blacklist=blacklist
) )
return self.get_item(location.for_branch(ModuleStoreEnum.BranchName.published)) return self.get_item(location.for_branch(ModuleStoreEnum.BranchName.published), **kwargs)
def unpublish(self, location, user_id): def unpublish(self, location, user_id, **kwargs):
""" """
Deletes the published version of the item. Deletes the published version of the item.
Returns the newly unpublished item. Returns the newly unpublished item.
""" """
self.delete_item(location, user_id, revision=ModuleStoreEnum.RevisionOption.published_only) self.delete_item(location, user_id, revision=ModuleStoreEnum.RevisionOption.published_only)
return self.get_item(location.for_branch(ModuleStoreEnum.BranchName.draft)) return self.get_item(location.for_branch(ModuleStoreEnum.BranchName.draft), **kwargs)
def revert_to_published(self, location, user_id): def revert_to_published(self, location, user_id):
""" """
......
common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py
...@@ -15,45 +15,52 @@ class SplitMongoKVS(InheritanceKeyValueStore): ...@@ -15,45 +15,52 @@ class SplitMongoKVS(InheritanceKeyValueStore):
known to the MongoModuleStore (data, children, and metadata) known to the MongoModuleStore (data, children, and metadata)
""" """
def __init__(self, definition, fields, inherited_settings): def __init__(self, definition, initial_values, inherited_settings, **kwargs):
""" """
:param definition: either a lazyloader or definition id for the definition :param definition: either a lazyloader or definition id for the definition
:param fields: a dictionary of the locally set fields :param initial_values: a dictionary of the locally set values
:param inherited_settings: the json value of each inheritable field from above this. :param inherited_settings: the json value of each inheritable field from above this.
Note, local fields may override and disagree w/ this b/c this says what the value Note, local fields may override and disagree w/ this b/c this says what the value
should be if the field is undefined. should be if the field is undefined.
""" """
# deepcopy so that manipulations of fields does not pollute the source # deepcopy so that manipulations of fields does not pollute the source
super(SplitMongoKVS, self).__init__(copy.deepcopy(fields), inherited_settings) super(SplitMongoKVS, self).__init__(copy.deepcopy(initial_values), inherited_settings)
self._definition = definition # either a DefinitionLazyLoader or the db id of the definition. self._definition = definition # either a DefinitionLazyLoader or the db id of the definition.
# if the db id, then the definition is presumed to be loaded into _fields # if the db id, then the definition is presumed to be loaded into _fields
# a decorator function for field values (to be called when a field is accessed)
self.field_decorator = kwargs.get('field_decorator', lambda x: x)
def get(self, key): def get(self, key):
# simplest case, field is directly set # load the field, if needed
if key.field_name not in self._fields:
# parent undefined in editing runtime (I think)
if key.scope == Scope.parent:
# see STUD-624. Right now copies MongoKeyValueStore.get's behavior of returning None
return None
if key.scope == Scope.children:
# didn't find children in _fields; so, see if there's a default
raise KeyError()
elif key.scope == Scope.settings:
# get default which may be the inherited value
raise KeyError()
elif key.scope == Scope.content:
if isinstance(self._definition, DefinitionLazyLoader):
self._load_definition()
else:
raise KeyError()
else:
raise InvalidScopeError(key)
if key.field_name in self._fields: if key.field_name in self._fields:
return self._fields[key.field_name] field_value = self._fields[key.field_name]
# parent undefined in editing runtime (I think) # return the "decorated" field value
if key.scope == Scope.parent: return self.field_decorator(field_value)
# see STUD-624. Right now copies MongoKeyValueStore.get's behavior of returning None
return None return None
if key.scope == Scope.children:
# didn't find children in _fields; so, see if there's a default
raise KeyError()
elif key.scope == Scope.settings:
# get default which may be the inherited value
raise KeyError()
elif key.scope == Scope.content:
if isinstance(self._definition, DefinitionLazyLoader):
self._load_definition()
if key.field_name in self._fields:
return self._fields[key.field_name]
raise KeyError()
else:
raise InvalidScopeError(key)
def set(self, key, value): def set(self, key, value):
# handle any special cases # handle any special cases
......
common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
import pymongo import pymongo
from uuid import uuid4 from uuid import uuid4
import ddt import ddt
import itertools
from importlib import import_module from importlib import import_module
from collections import namedtuple from collections import namedtuple
import unittest import unittest
...@@ -8,7 +9,6 @@ import datetime ...@@ -8,7 +9,6 @@ import datetime
from pytz import UTC from pytz import UTC
from xmodule.tests import DATA_DIR from xmodule.tests import DATA_DIR
from opaque_keys.edx.locations import Location
from xmodule.modulestore import ModuleStoreEnum, PublishState from xmodule.modulestore import ModuleStoreEnum, PublishState
from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.exceptions import InvalidVersionError from xmodule.exceptions import InvalidVersionError
...@@ -21,6 +21,7 @@ from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator ...@@ -21,6 +21,7 @@ from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
from django.conf import settings from django.conf import settings
from xmodule.modulestore.tests.factories import check_mongo_calls from xmodule.modulestore.tests.factories import check_mongo_calls
from xmodule.modulestore.search import path_to_location from xmodule.modulestore.search import path_to_location
from xmodule.modulestore.exceptions import DuplicateCourseError
if not settings.configured: if not settings.configured:
settings.configure() settings.configure()
from xmodule.modulestore.mixed import MixedModuleStore from xmodule.modulestore.mixed import MixedModuleStore
...@@ -192,6 +193,10 @@ class TestMixedModuleStore(unittest.TestCase): ...@@ -192,6 +193,10 @@ class TestMixedModuleStore(unittest.TestCase):
""" """
return self.course_locations[string].course_key return self.course_locations[string].course_key
def _initialize_mixed(self):
self.store = MixedModuleStore(None, create_modulestore_instance=create_modulestore_instance, **self.options)
self.addCleanup(self.store.close_all_connections)
def initdb(self, default): def initdb(self, default):
""" """
Initialize the database and create one test course in it Initialize the database and create one test course in it
...@@ -203,8 +208,7 @@ class TestMixedModuleStore(unittest.TestCase): ...@@ -203,8 +208,7 @@ class TestMixedModuleStore(unittest.TestCase):
if index > 0: if index > 0:
store_configs[index], store_configs[0] = store_configs[0], store_configs[index] store_configs[index], store_configs[0] = store_configs[0], store_configs[index]
break break
self.store = MixedModuleStore(None, create_modulestore_instance=create_modulestore_instance, **self.options) self._initialize_mixed()
self.addCleanup(self.store.close_all_connections)
# convert to CourseKeys # convert to CourseKeys
self.course_locations = { self.course_locations = {
...@@ -216,12 +220,9 @@ class TestMixedModuleStore(unittest.TestCase): ...@@ -216,12 +220,9 @@ class TestMixedModuleStore(unittest.TestCase):
course_id: course_key.make_usage_key('course', course_key.run) course_id: course_key.make_usage_key('course', course_key.run)
for course_id, course_key in self.course_locations.iteritems() # pylint: disable=maybe-no-member for course_id, course_key in self.course_locations.iteritems() # pylint: disable=maybe-no-member
} }
if default == 'split':
self.fake_location = CourseLocator( self.fake_location = self.course_locations[self.MONGO_COURSEID].course_key.make_usage_key('vertical', 'fake')
'foo', 'bar', 'slowly', branch=ModuleStoreEnum.BranchName.draft
).make_usage_key('vertical', 'baz')
else:
self.fake_location = Location('foo', 'bar', 'slowly', 'vertical', 'baz')
self.xml_chapter_location = self.course_locations[self.XML_COURSEID1].replace( self.xml_chapter_location = self.course_locations[self.XML_COURSEID1].replace(
category='chapter', name='Overview' category='chapter', name='Overview'
) )
...@@ -248,6 +249,23 @@ class TestMixedModuleStore(unittest.TestCase): ...@@ -248,6 +249,23 @@ class TestMixedModuleStore(unittest.TestCase):
SlashSeparatedCourseKey('foo', 'bar', '2012_Fall')), mongo_ms_type SlashSeparatedCourseKey('foo', 'bar', '2012_Fall')), mongo_ms_type
) )
@ddt.data(*itertools.product(
(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split),
(True, False)
))
@ddt.unpack
def test_duplicate_course_error(self, default_ms, reset_mixed_mappings):
"""
Make sure we get back the store type we expect for given mappings
"""
self._initialize_mixed()
with self.store.default_store(default_ms):
self.store.create_course('org_x', 'course_y', 'run_z', self.user_id)
if reset_mixed_mappings:
self.store.mappings = {}
with self.assertRaises(DuplicateCourseError):
self.store.create_course('org_x', 'course_y', 'run_z', self.user_id)
# split has one lookup for the course and then one for the course items # split has one lookup for the course and then one for the course items
@ddt.data(('draft', 1, 0), ('split', 2, 0)) @ddt.data(('draft', 1, 0), ('split', 2, 0))
@ddt.unpack @ddt.unpack
...@@ -512,7 +530,7 @@ class TestMixedModuleStore(unittest.TestCase): ...@@ -512,7 +530,7 @@ class TestMixedModuleStore(unittest.TestCase):
with check_mongo_calls(mongo_store, max_find, max_send): with check_mongo_calls(mongo_store, max_find, max_send):
self.store.delete_item(private_leaf.location, self.user_id) self.store.delete_item(private_leaf.location, self.user_id)
@ddt.data(('draft', 3, 0), ('split', 6, 0)) @ddt.data(('draft', 2, 0), ('split', 3, 0))
@ddt.unpack @ddt.unpack
def test_get_courses(self, default_ms, max_find, max_send): def test_get_courses(self, default_ms, max_find, max_send):
self.initdb(default_ms) self.initdb(default_ms)
...@@ -1189,6 +1207,48 @@ class TestMixedModuleStore(unittest.TestCase): ...@@ -1189,6 +1207,48 @@ class TestMixedModuleStore(unittest.TestCase):
# there should be no published problems with the old name # there should be no published problems with the old name
assertNumProblems(problem_original_name, 0) assertNumProblems(problem_original_name, 0)
def verify_default_store(self, store_type):
# verify default_store property
self.assertEquals(self.store.default_modulestore.get_modulestore_type(), store_type)
# verify internal helper method
store = self.store._get_modulestore_for_courseid()
self.assertEquals(store.get_modulestore_type(), store_type)
# verify store used for creating a course
try:
course = self.store.create_course("org", "course{}".format(uuid4().hex[:3]), "run", self.user_id)
self.assertEquals(course.system.modulestore.get_modulestore_type(), store_type)
except NotImplementedError:
self.assertEquals(store_type, ModuleStoreEnum.Type.xml)
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.xml)
def test_default_store(self, default_ms):
"""
Test the default store context manager
"""
# initialize the mixed modulestore
self._initialize_mixed()
with self.store.default_store(default_ms):
self.verify_default_store(default_ms)
def test_nested_default_store(self):
"""
Test the default store context manager, nested within one another
"""
# initialize the mixed modulestore
self._initialize_mixed()
with self.store.default_store(ModuleStoreEnum.Type.mongo):
self.verify_default_store(ModuleStoreEnum.Type.mongo)
with self.store.default_store(ModuleStoreEnum.Type.split):
self.verify_default_store(ModuleStoreEnum.Type.split)
with self.store.default_store(ModuleStoreEnum.Type.xml):
self.verify_default_store(ModuleStoreEnum.Type.xml)
self.verify_default_store(ModuleStoreEnum.Type.split)
self.verify_default_store(ModuleStoreEnum.Type.mongo)
#============================================================================================================= #=============================================================================================================
# General utils for not using django settings # General utils for not using django settings
......
common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py
...@@ -45,7 +45,6 @@ class ModuleStoreSettingsMigration(TestCase): ...@@ -45,7 +45,6 @@ class ModuleStoreSettingsMigration(TestCase):
"ENGINE": "xmodule.modulestore.mixed.MixedModuleStore", "ENGINE": "xmodule.modulestore.mixed.MixedModuleStore",
"OPTIONS": { "OPTIONS": {
"mappings": {}, "mappings": {},
"reference_type": "Location",
"stores": { "stores": {
"an_old_mongo_store": { "an_old_mongo_store": {
"DOC_STORE_CONFIG": {}, "DOC_STORE_CONFIG": {},
...@@ -82,7 +81,6 @@ class ModuleStoreSettingsMigration(TestCase): ...@@ -82,7 +81,6 @@ class ModuleStoreSettingsMigration(TestCase):
'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore', 'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore',
'OPTIONS': { 'OPTIONS': {
'mappings': {}, 'mappings': {},
'reference_type': 'Location',
'stores': [ 'stores': [
{ {
'NAME': 'split', 'NAME': 'split',
...@@ -146,7 +144,7 @@ class ModuleStoreSettingsMigration(TestCase): ...@@ -146,7 +144,7 @@ class ModuleStoreSettingsMigration(TestCase):
Tests whether the split module store is configured in the given setting. Tests whether the split module store is configured in the given setting.
""" """
stores = self._get_mixed_stores(mixed_setting) stores = self._get_mixed_stores(mixed_setting)
split_settings = [store for store in stores if 'DraftVersioningModuleStore' in store['ENGINE']] split_settings = [store for store in stores if store['ENGINE'].endswith('.DraftVersioningModuleStore')]
if len(split_settings): if len(split_settings):
# there should only be one setting for split # there should only be one setting for split
self.assertEquals(len(split_settings), 1) self.assertEquals(len(split_settings), 1)
......
common/lib/xmodule/xmodule/modulestore/xml.py
...@@ -24,6 +24,7 @@ from xmodule.modulestore import ModuleStoreEnum, ModuleStoreReadBase ...@@ -24,6 +24,7 @@ from xmodule.modulestore import ModuleStoreEnum, ModuleStoreReadBase
from xmodule.tabs import CourseTabList from xmodule.tabs import CourseTabList
from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
from opaque_keys.edx.locator import CourseLocator
from xblock.field_data import DictFieldData from xblock.field_data import DictFieldData
from xblock.runtime import DictKeyValueStore, IdGenerator from xblock.runtime import DictKeyValueStore, IdGenerator
...@@ -403,7 +404,6 @@ class XMLModuleStore(ModuleStoreReadBase): ...@@ -403,7 +404,6 @@ class XMLModuleStore(ModuleStoreReadBase):
self.default_class = class_ self.default_class = class_
self.parent_trackers = defaultdict(ParentTracker) self.parent_trackers = defaultdict(ParentTracker)
self.reference_type = Location
# All field data will be stored in an inheriting field data. # All field data will be stored in an inheriting field data.
self.field_data = inheriting_field_data(kvs=DictKeyValueStore()) self.field_data = inheriting_field_data(kvs=DictKeyValueStore())
...@@ -700,7 +700,7 @@ class XMLModuleStore(ModuleStoreReadBase): ...@@ -700,7 +700,7 @@ class XMLModuleStore(ModuleStoreReadBase):
""" """
return usage_key in self.modules[usage_key.course_key] return usage_key in self.modules[usage_key.course_key]
def get_item(self, usage_key, depth=0): def get_item(self, usage_key, depth=0, **kwargs):
""" """
Returns an XBlock instance for the item for this UsageKey. Returns an XBlock instance for the item for this UsageKey.
...@@ -766,7 +766,16 @@ class XMLModuleStore(ModuleStoreReadBase): ...@@ -766,7 +766,16 @@ class XMLModuleStore(ModuleStoreReadBase):
return items return items
def get_courses(self, depth=0): def make_course_key(self, org, course, run):
"""
Return a valid :class:`~opaque_keys.edx.keys.CourseKey` for this modulestore
that matches the supplied `org`, `course`, and `run`.
This key may represent a course that doesn't exist in this modulestore.
"""
return CourseLocator(org, course, run, deprecated=True)
def get_courses(self, depth=0, **kwargs):
""" """
Returns a list of course descriptors. If there were errors on loading, Returns a list of course descriptors. If there were errors on loading,
some of these may be ErrorDescriptors instead. some of these may be ErrorDescriptors instead.
...@@ -780,7 +789,7 @@ class XMLModuleStore(ModuleStoreReadBase): ...@@ -780,7 +789,7 @@ class XMLModuleStore(ModuleStoreReadBase):
""" """
return dict((k, self.errored_courses[k].errors) for k in self.errored_courses) return dict((k, self.errored_courses[k].errors) for k in self.errored_courses)
def get_orphans(self, course_key): def get_orphans(self, course_key, **kwargs):
""" """
Get all of the xblocks in the given course which have no parents and are not of types which are 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 usually orphaned. NOTE: may include xblocks which still have references via xblocks which don't
...@@ -806,7 +815,7 @@ class XMLModuleStore(ModuleStoreReadBase): ...@@ -806,7 +815,7 @@ class XMLModuleStore(ModuleStoreReadBase):
""" """
return ModuleStoreEnum.Type.xml return ModuleStoreEnum.Type.xml
def get_courses_for_wiki(self, wiki_slug): def get_courses_for_wiki(self, wiki_slug, **kwargs):
""" """
Return the list of courses which use this wiki_slug Return the list of courses which use this wiki_slug
:param wiki_slug: the course wiki root slug :param wiki_slug: the course wiki root slug
......
common/lib/xmodule/xmodule/modulestore/xml_importer.py
...@@ -17,7 +17,7 @@ from .store_utilities import rewrite_nonportable_content_links ...@@ -17,7 +17,7 @@ from .store_utilities import rewrite_nonportable_content_links
import xblock import xblock
from xmodule.tabs import CourseTabList from xmodule.tabs import CourseTabList
from xmodule.modulestore.django import ASSET_IGNORE_REGEX from xmodule.modulestore.django import ASSET_IGNORE_REGEX
from xmodule.modulestore.exceptions import InvalidLocationError from xmodule.modulestore.exceptions import DuplicateCourseError
from xmodule.modulestore.mongo.base import MongoRevisionKey from xmodule.modulestore.mongo.base import MongoRevisionKey
from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore import ModuleStoreEnum
...@@ -174,7 +174,7 @@ def import_from_xml( ...@@ -174,7 +174,7 @@ def import_from_xml(
if create_new_course_if_not_present and not store.has_course(dest_course_id, ignore_case=True): if create_new_course_if_not_present and not store.has_course(dest_course_id, ignore_case=True):
try: try:
store.create_course(dest_course_id.org, dest_course_id.course, dest_course_id.run, user_id) store.create_course(dest_course_id.org, dest_course_id.course, dest_course_id.run, user_id)
except InvalidLocationError: except DuplicateCourseError:
# course w/ same org and course exists # course w/ same org and course exists
log.debug( log.debug(
"Skipping import of course with id, {0}," "Skipping import of course with id, {0},"
......
lms/envs/bok_choy.auth.json
...@@ -51,7 +51,6 @@ ...@@ -51,7 +51,6 @@
"ENGINE": "xmodule.modulestore.mixed.MixedModuleStore", "ENGINE": "xmodule.modulestore.mixed.MixedModuleStore",
"OPTIONS": { "OPTIONS": {
"mappings": {}, "mappings": {},
"reference_type": "Location",
"stores": [ "stores": [
{ {
"NAME": "draft", "NAME": "draft",
......
lms/envs/common.py
...@@ -504,7 +504,6 @@ MODULESTORE = { ...@@ -504,7 +504,6 @@ MODULESTORE = {
'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore', 'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore',
'OPTIONS': { 'OPTIONS': {
'mappings': {}, 'mappings': {},
'reference_type': 'Location',
'stores': [ 'stores': [
{ {
'NAME': 'draft', 'NAME': 'draft',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment