Merge pull request #34398 from openedx/xblock2

upgrade xblock==2.0
References:
- openedx/public-engineering#15
- openedx/XBlock#680

cms/djangoapps/contentstore/helpers.py

@@ -319,10 +319,15 @@ def _import_xml_node_to_parent(
     parent_key = parent_xblock.scope_ids.usage_id
     block_type = node.tag
 
+    # Modulestore's IdGenerator here is SplitMongoIdManager which is assigned
+    # by CachingDescriptorSystem Runtime and since we need our custom ImportIdGenerator
+    # here we are temporaraliy swtiching it.
+    original_id_generator = runtime.id_generator
+
     # Generate the new ID:
-    id_generator = ImportIdGenerator(parent_key.context_key)
-    def_id = id_generator.create_definition(block_type, slug_hint)
-    usage_id = id_generator.create_usage(def_id)
+    runtime.id_generator = ImportIdGenerator(parent_key.context_key)
+    def_id = runtime.id_generator.create_definition(block_type, slug_hint)
+    usage_id = runtime.id_generator.create_usage(def_id)
     keys = ScopeIds(None, block_type, def_id, usage_id)
     # parse_xml is a really messy API. We pass both 'keys' and 'id_generator' and, depending on the XBlock, either
     # one may be used to determine the new XBlock's usage key, and the other will be ignored. e.g. video ignores
@@ -348,7 +353,7 @@ def _import_xml_node_to_parent(
 
     if not xblock_class.has_children:
         # No children to worry about. The XML may contain child nodes, but they're not XBlocks.
-        temp_xblock = xblock_class.parse_xml(node, runtime, keys, id_generator)
+        temp_xblock = xblock_class.parse_xml(node, runtime, keys)
     else:
         # We have to handle the children ourselves, because there are lots of complex interactions between
         #    * the vanilla XBlock parse_xml() method, and its lack of API for "create and save a new XBlock"
@@ -358,8 +363,12 @@ def _import_xml_node_to_parent(
         # serialization of a child block, in order. For blocks that don't support children, their XML content/nodes
         # could be anything (e.g. HTML, capa)
         node_without_children = etree.Element(node.tag, **node.attrib)
-        temp_xblock = xblock_class.parse_xml(node_without_children, runtime, keys, id_generator)
+        temp_xblock = xblock_class.parse_xml(node_without_children, runtime, keys)
         child_nodes = list(node)
+
+    # Restore the original id_generator
+    runtime.id_generator = original_id_generator
+
     if xblock_class.has_children and temp_xblock.children:
         raise NotImplementedError("We don't yet support pasting XBlocks with children")
     temp_xblock.parent = parent_key

lms/djangoapps/courseware/tests/test_video_mongo.py

@@ -2025,16 +2025,16 @@ def test_import_val_data_internal(self):
             val_transcript_provider=val_transcript_provider
         )
         xml_object = etree.fromstring(xml_data)
-        id_generator = Mock()
-        id_generator.target_course_id = "test_course_id"
-        video = self.block.parse_xml(xml_object, module_system, None, id_generator)
+        module_system.id_generator.target_course_id = "test_course_id"
+
+        video = self.block.parse_xml(xml_object, module_system, None)
 
         assert video.edx_video_id == 'test_edx_video_id'
         video_data = get_video_info(video.edx_video_id)
         assert video_data['client_video_id'] == 'test_client_video_id'
         assert video_data['duration'] == 111.0
         assert video_data['status'] == 'imported'
-        assert video_data['courses'] == [{id_generator.target_course_id: None}]
+        assert video_data['courses'] == [{module_system.id_generator.target_course_id: None}]
         assert video_data['encoded_videos'][0]['profile'] == 'mobile'
         assert video_data['encoded_videos'][0]['url'] == 'http://example.com/video'
         assert video_data['encoded_videos'][0]['file_size'] == 222
@@ -2075,12 +2075,11 @@ def test_import_no_video_id(self):
         xml_data = """<video><video_asset></video_asset></video>"""
         xml_object = etree.fromstring(xml_data)
         module_system = DummySystem(load_error_blocks=True)
-        id_generator = Mock()
 
         # Verify edx_video_id is empty before.
         assert self.block.edx_video_id == ''
 
-        video = self.block.parse_xml(xml_object, module_system, None, id_generator)
+        video = self.block.parse_xml(xml_object, module_system, None)
 
         # Verify edx_video_id is populated after the import.
         assert video.edx_video_id != ''
@@ -2112,7 +2111,6 @@ def test_import_val_transcript(self):
         )
         xml_object = etree.fromstring(xml_data)
         module_system = DummySystem(load_error_blocks=True)
-        id_generator = Mock()
 
         # Create static directory in import file system and place transcript files inside it.
         module_system.resources_fs.makedirs(EXPORT_IMPORT_STATIC_DIR, recreate=True)
@@ -2128,7 +2126,7 @@ def test_import_val_transcript(self):
         # Verify edx_video_id is empty before.
         assert self.block.edx_video_id == ''
 
-        video = self.block.parse_xml(xml_object, module_system, None, id_generator)
+        video = self.block.parse_xml(xml_object, module_system, None)
 
         # Verify edx_video_id is populated after the import.
         assert video.edx_video_id != ''
@@ -2218,7 +2216,6 @@ def test_import_val_transcript_priority(self, sub_id, external_transcripts, val_
         language_code = 'en'
 
         module_system = DummySystem(load_error_blocks=True)
-        id_generator = Mock()
 
         # Create static directory in import file system and place transcript files inside it.
         module_system.resources_fs.makedirs(EXPORT_IMPORT_STATIC_DIR, recreate=True)
@@ -2270,7 +2267,7 @@ def test_import_val_transcript_priority(self, sub_id, external_transcripts, val_
         # Verify edx_video_id is empty before import.
         assert self.block.edx_video_id == ''
 
-        video = self.block.parse_xml(xml_object, module_system, None, id_generator)
+        video = self.block.parse_xml(xml_object, module_system, None)
 
         # Verify edx_video_id is not empty after import.
         assert video.edx_video_id != ''
@@ -2298,7 +2295,7 @@ def test_import_val_data_invalid(self):
         """
         xml_object = etree.fromstring(xml_data)
         with pytest.raises(ValCannotCreateError):
-            VideoBlock.parse_xml(xml_object, module_system, None, id_generator=Mock())
+            VideoBlock.parse_xml(xml_object, module_system, None)
         with pytest.raises(ValVideoNotFoundError):
             get_video_info("test_edx_video_id")
 

openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py

@@ -205,7 +205,7 @@ def get_block(self, usage_key, for_parent=None):
             # plus some minor additional lines of code as needed.
             block = block_class.parse_xml_new_runtime(xml_node, runtime=self, keys=keys)
         else:
-            block = block_class.parse_xml(xml_node, runtime=self, keys=keys, id_generator=None)
+            block = block_class.parse_xml(xml_node, runtime=self, keys=keys)
 
         # Update field data with parsed values. We can't call .save() because it will call save_block(), below.
         block.force_save_fields(block._get_fields_to_save())  # pylint: disable=protected-access

openedx/core/djangoapps/xblock/runtime/runtime.py

@@ -217,11 +217,11 @@ def applicable_aside_types(self, block: XBlock):
         """ Disable XBlock asides in this runtime """
         return []
 
-    def parse_xml_file(self, fileobj, id_generator=None):
+    def parse_xml_file(self, fileobj):
         # Deny access to the inherited method
         raise NotImplementedError("XML Serialization is only supported with BlockstoreXBlockRuntime")
 
-    def add_node_as_child(self, block, node, id_generator=None):
+    def add_node_as_child(self, block, node):
         """
         Called by XBlock.parse_xml to treat a child node as a child block.
         """

requirements/edx/base.txt

@@ -6,7 +6,7 @@
 #
 -e git+https://github.com/anupdhabarde/edx-proctoring-proctortrack.git@31c6c9923a51c903ae83760ecbbac191363aa2a2#egg=edx_proctoring_proctortrack
     # via -r requirements/edx/github.in
-acid-xblock==0.2.1
+acid-xblock==0.3.0
     # via -r requirements/edx/kernel.in
 aiohttp==3.9.3
     # via
@@ -528,7 +528,7 @@ edx-rest-api-client==5.6.1
     #   edx-proctoring
 edx-search==3.9.1
     # via -r requirements/edx/kernel.in
-edx-sga==0.23.1
+edx-sga==0.24.1
     # via -r requirements/edx/bundled.in
 edx-submissions==3.7.0
     # via
@@ -804,7 +804,7 @@ optimizely-sdk==4.1.1
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/bundled.in
-ora2==6.5.1
+ora2==6.6.1
     # via -r requirements/edx/bundled.in
 packaging==23.2
     # via
@@ -1222,7 +1222,7 @@ webob==1.8.7
     #   xblock
 wrapt==1.16.0
     # via -r requirements/edx/paver.txt
-xblock[django]==1.10.0
+xblock[django]==2.0.0
     # via
     #   -r requirements/edx/kernel.in
     #   acid-xblock

requirements/edx/development.txt

@@ -12,7 +12,7 @@ accessible-pygments==0.0.4
     # via
     #   -r requirements/edx/doc.txt
     #   pydata-sphinx-theme
-acid-xblock==0.2.1
+acid-xblock==0.3.0
     # via
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
@@ -825,7 +825,7 @@ edx-search==3.9.1
     # via
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
-edx-sga==0.23.1
+edx-sga==0.24.1
     # via
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
@@ -1333,7 +1333,7 @@ optimizely-sdk==4.1.1
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
-ora2==6.5.1
+ora2==6.6.1
     # via
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
@@ -2185,7 +2185,7 @@ wrapt==1.16.0
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
     #   astroid
-xblock[django]==1.10.0
+xblock[django]==2.0.0
     # via
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt

requirements/edx/doc.txt

@@ -8,7 +8,7 @@
     # via -r requirements/edx/base.txt
 accessible-pygments==0.0.4
     # via pydata-sphinx-theme
-acid-xblock==0.2.1
+acid-xblock==0.3.0
     # via -r requirements/edx/base.txt
 aiohttp==3.9.3
     # via
@@ -607,7 +607,7 @@ edx-rest-api-client==5.6.1
     #   edx-proctoring
 edx-search==3.9.1
     # via -r requirements/edx/base.txt
-edx-sga==0.23.1
+edx-sga==0.24.1
     # via -r requirements/edx/base.txt
 edx-submissions==3.7.0
     # via
@@ -942,7 +942,7 @@ optimizely-sdk==4.1.1
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/base.txt
-ora2==6.5.1
+ora2==6.6.1
     # via -r requirements/edx/base.txt
 packaging==23.2
     # via
@@ -1488,7 +1488,7 @@ webob==1.8.7
     #   xblock
 wrapt==1.16.0
     # via -r requirements/edx/base.txt
-xblock[django]==1.10.0
+xblock[django]==2.0.0
     # via
     #   -r requirements/edx/base.txt
     #   acid-xblock

requirements/edx/testing.txt

@@ -6,7 +6,7 @@
 #
 -e git+https://github.com/anupdhabarde/edx-proctoring-proctortrack.git@31c6c9923a51c903ae83760ecbbac191363aa2a2#egg=edx_proctoring_proctortrack
     # via -r requirements/edx/base.txt
-acid-xblock==0.2.1
+acid-xblock==0.3.0
     # via -r requirements/edx/base.txt
 aiohttp==3.9.3
     # via
@@ -633,7 +633,7 @@ edx-rest-api-client==5.6.1
     #   edx-proctoring
 edx-search==3.9.1
     # via -r requirements/edx/base.txt
-edx-sga==0.23.1
+edx-sga==0.24.1
     # via -r requirements/edx/base.txt
 edx-submissions==3.7.0
     # via
@@ -997,7 +997,7 @@ optimizely-sdk==4.1.1
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/base.txt
-ora2==6.5.1
+ora2==6.6.1
     # via -r requirements/edx/base.txt
 packaging==23.2
     # via
@@ -1604,7 +1604,7 @@ wrapt==1.16.0
     # via
     #   -r requirements/edx/base.txt
     #   astroid
-xblock[django]==1.10.0
+xblock[django]==2.0.0
     # via
     #   -r requirements/edx/base.txt
     #   acid-xblock

xmodule/course_block.py

@@ -1166,8 +1166,8 @@ def read_grading_policy(cls, paths, system):
         return policy_str
 
     @classmethod
-    def parse_xml(cls, node, runtime, keys, id_generator):
-        instance = super().parse_xml(node, runtime, keys, id_generator)
+    def parse_xml(cls, node, runtime, keys):
+        instance = super().parse_xml(node, runtime, keys)
 
         policy_dir = None
         url_name = node.get('url_name')

xmodule/discussion_block.py

@@ -233,7 +233,7 @@ def student_view_data(self):
         return {'topic_id': self.discussion_id}
 
     @classmethod
-    def parse_xml(cls, node, runtime, keys, id_generator):
+    def parse_xml(cls, node, runtime, keys):
         """
         Parses OLX into XBlock.
 
@@ -246,7 +246,7 @@ def parse_xml(cls, node, runtime, keys, id_generator):
         XBlock.parse_xml. Otherwise this method parses file in "discussion" folder (known as definition_xml), applies
         policy.json and updates fields accordingly.
         """
-        block = super().parse_xml(node, runtime, keys, id_generator)
+        block = super().parse_xml(node, runtime, keys)
 
         cls._apply_metadata_and_policy(block, node, runtime)
 

xmodule/error_block.py

@@ -178,13 +178,13 @@ def from_xml(cls, xml_data, system, id_generator,  # pylint: disable=arguments-d
         return cls._construct(system, xml_data, error_msg, location=id_generator.create_definition('error'))
 
     @classmethod
-    def parse_xml(cls, node, runtime, keys, id_generator):  # lint-amnesty, pylint: disable=unused-argument
+    def parse_xml(cls, node, runtime, keys):  # lint-amnesty, pylint: disable=unused-argument
         """
         Interpret the parsed XML in `node`, creating an XModuleDescriptor.
         """
         # It'd be great to not reserialize and deserialize the xml
         xml = etree.tostring(node).decode('utf-8')
-        block = cls.from_xml(xml, runtime, id_generator)
+        block = cls.from_xml(xml, runtime, runtime.id_generator)
         return block
 
     def export_to_xml(self, resource_fs):

xmodule/modulestore/xml.py

@@ -259,7 +259,7 @@ def construct_xblock_from_class(self, cls, scope_ids, field_data=None, *args, **
 
     # id_generator is ignored, because each ImportSystem is already local to
     # a course, and has it's own id_generator already in place
-    def add_node_as_child(self, block, node, id_generator):  # lint-amnesty, pylint: disable=signature-differs
+    def add_node_as_child(self, block, node):  # lint-amnesty, pylint: disable=signature-differs
         child_block = self.process_xml(etree.tostring(node))
         block.children.append(child_block.scope_ids.usage_id)
 

xmodule/raw_block.py

@@ -78,7 +78,7 @@ def parse_xml_new_runtime(cls, node, runtime, keys):
         try:
             block = super().parse_xml_new_runtime(node, runtime, keys)
         except AttributeError:
-            block = super().parse_xml(node, runtime, keys, id_generator=None)
+            block = super().parse_xml(node, runtime, keys)
         block.data = data_field_value
         return block
 

xmodule/template_block.py

@@ -130,7 +130,7 @@ class TranslateCustomTagBlock(  # pylint: disable=abstract-method
     resources_dir = None
 
     @classmethod
-    def parse_xml(cls, node, runtime, _keys, _id_generator):
+    def parse_xml(cls, node, runtime, _keys):
         """
         Transforms the xml_data from <$custom_tag attr="" attr=""/> to
         <customtag attr="" attr="" impl="$custom_tag"/>

xmodule/tests/test_discussion.py

@@ -69,7 +69,6 @@ def setUp(self):
         self.runtime_mock = mock.Mock(spec=Runtime)
         self.runtime_mock.construct_xblock_from_class = mock.Mock(side_effect=self._construct_xblock_mock)
         self.runtime_mock.get_policy = mock.Mock(return_value={})
-        self.id_gen_mock = mock.Mock()
 
     def _construct_xblock_mock(self, cls, keys):  # pylint: disable=unused-argument
         """
@@ -102,7 +101,7 @@ def test_xblock_export_format(self, id_pair, category_pair, target_pair, patched
 
         patched_load_definition_xml.side_effect = Exception("Irrelevant")
 
-        block = DiscussionXBlock.parse_xml(node, self.runtime_mock, self.keys, self.id_gen_mock)
+        block = DiscussionXBlock.parse_xml(node, self.runtime_mock, self.keys)
         try:
             assert block.discussion_id == id_pair.value
             assert block.discussion_category == category_pair.value
@@ -134,7 +133,7 @@ def test_legacy_export_format(self, id_pair, category_pair, target_pair, patched
 
         patched_load_definition_xml.return_value = (definition_node, "irrelevant")
 
-        block = DiscussionXBlock.parse_xml(node, self.runtime_mock, self.keys, self.id_gen_mock)
+        block = DiscussionXBlock.parse_xml(node, self.runtime_mock, self.keys)
         try:
             assert block.discussion_id == id_pair.value
             assert block.discussion_category == category_pair.value