Merge pull request #2117 from edx/dhm/memory_definition

Set definition_locator on in-memory xblocks

cms/djangoapps/contentstore/tests/test_crud.py

@@ -5,7 +5,7 @@ from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore.django import modulestore, loc_mapper, clear_existing_modulestores
 from xmodule.seq_module import SequenceDescriptor
 from xmodule.capa_module import CapaDescriptor
-from xmodule.modulestore.locator import CourseLocator, BlockUsageLocator
+from xmodule.modulestore.locator import CourseLocator, BlockUsageLocator, LocalId
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.html_module import HtmlDescriptor
 from xmodule.modulestore import inheritance
@@ -103,13 +103,17 @@ class TemplateTests(unittest.TestCase):
             test_course.system, parent_xblock=test_course)
         test_def_content = '<problem>boo</problem>'
         # create child
-        self.load_from_json({
+        new_block = self.load_from_json({
             'category': 'problem',
             'fields': {
                 'data': test_def_content,
                 'display_name': 'problem'
             }},
-            test_course.system, parent_xblock=test_chapter)
+            test_course.system,
+            parent_xblock=test_chapter
+        )
+        self.assertIsNotNone(new_block.definition_locator)
+        self.assertTrue(isinstance(new_block.definition_locator.definition_id, LocalId))
         # better to pass in persisted parent over the subdag so
         # subdag gets the parent pointer (otherwise 2 ops, persist dag, update parent children,
         # persist parent

common/lib/xmodule/xmodule/modulestore/locator.py

@@ -502,11 +502,16 @@ class DefinitionLocator(Locator):
 
     URL_RE = re.compile(r'^defx://' + VERSION_PREFIX + '([^/]+)$', re.IGNORECASE)
     def __init__(self, definition_id):
-        if isinstance(definition_id, basestring):
+        if isinstance(definition_id, LocalId):
+            self.definition_id = definition_id
+        elif isinstance(definition_id, basestring):
             regex_match = self.URL_RE.match(definition_id)
             if regex_match is not None:
-                definition_id = self.as_object_id(regex_match.group(1))
-        self.definition_id = self.as_object_id(definition_id)
+                self.definition_id = self.as_object_id(regex_match.group(1))
+            else:
+                self.definition_id = self.as_object_id(definition_id)
+        else:
+            self.definition_id = self.as_object_id(definition_id)
 
     def __unicode__(self):
         '''

common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py

@@ -131,7 +131,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         module.edited_on = edit_info.get('edited_on')
         module.previous_version = edit_info.get('previous_version')
         module.update_version = edit_info.get('update_version')
-        module.definition_locator = self.modulestore.definition_locator(definition)
+        module.definition_locator = definition_id
         # decache any pending field settings
         module.save()
 

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -706,7 +706,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         a new version. Setting force to True conflicts with setting this to True and will cause a VersionConflictError
 
         :param definition_locator: should either be None to indicate this is a brand new definition or
-        a pointer to the existing definition to which this block should point or from which this was derived.
+        a pointer to the existing definition to which this block should point or from which this was derived
+        or a LocalId to indicate that it's new.
         If fields does not contain any Scope.content, then definition_locator must have a value meaning that this
         block points
         to the existing definition. If fields contains Scope.content and definition_locator is not None, then
@@ -741,7 +742,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         partitioned_fields = self._partition_fields_by_scope(category, fields)
         new_def_data = partitioned_fields.get(Scope.content, {})
         # persist the definition if persisted != passed
-        if (definition_locator is None or definition_locator.definition_id is None):
+        if (definition_locator is None or isinstance(definition_locator.definition_id, LocalId)):
             definition_locator = self.create_definition_from_data(new_def_data, category, user_id)
         elif new_def_data is not None:
             definition_locator, _ = self.update_definition_from_data(definition_locator, new_def_data, user_id)
@@ -1031,7 +1032,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
     def _persist_subdag(self, xblock, user_id, structure_blocks, new_id):
         # persist the definition if persisted != passed
         new_def_data = self._filter_special_fields(xblock.get_explicitly_set_fields_by_scope(Scope.content))
-        if (xblock.definition_locator is None or xblock.definition_locator.definition_id is None):
+        if xblock.definition_locator is None or isinstance(xblock.definition_locator.definition_id, LocalId):
             xblock.definition_locator = self.create_definition_from_data(
                 new_def_data, xblock.category, user_id)
             is_updated = True
@@ -1338,7 +1339,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         if isinstance(definition, DefinitionLazyLoader):
             return definition.definition_locator
         elif '_id' not in definition:
-            return None
+            return DefinitionLocator(LocalId())
         else:
             return DefinitionLocator(definition['_id'])