From 6d01fbc4188bfe07acfe21669c921ad2c4a3588b Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 20 Apr 2023 17:01:06 -0700 Subject: [PATCH] chore: consolidate two different implementations for serializing XBlocks --- .../core/djangoapps/content_libraries/api.py | 2 +- .../content_staging/block_serializer.py | 99 ----------- .../content_staging/tests/test_clipboard.py | 2 +- .../core/djangoapps/content_staging/views.py | 4 +- openedx/core/djangoapps/olx_rest_api/api.py | 21 +-- .../djangoapps/olx_rest_api/test_views.py | 6 +- openedx/core/djangoapps/olx_rest_api/views.py | 12 +- .../core/lib/xblock_serializer/__init__.py | 0 openedx/core/lib/xblock_serializer/api.py | 27 +++ .../xblock_serializer}/block_serializer.py | 155 ++++++++-------- .../core/lib/xblock_serializer/test_api.py | 167 ++++++++++++++++++ .../xblock_serializer/test_utils.py} | 10 +- .../xblock_serializer/utils.py} | 30 +++- setup.cfg | 1 + 14 files changed, 321 insertions(+), 215 deletions(-) delete mode 100644 openedx/core/djangoapps/content_staging/block_serializer.py create mode 100644 openedx/core/lib/xblock_serializer/__init__.py create mode 100644 openedx/core/lib/xblock_serializer/api.py rename openedx/core/{djangoapps/olx_rest_api => lib/xblock_serializer}/block_serializer.py (52%) create mode 100644 openedx/core/lib/xblock_serializer/test_api.py rename openedx/core/{djangoapps/olx_rest_api/test_adapters.py => lib/xblock_serializer/test_utils.py} (87%) rename openedx/core/{djangoapps/olx_rest_api/adapters.py => lib/xblock_serializer/utils.py} (85%) diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index ecb18d681729..e796beef1201 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -92,13 +92,13 @@ LIBRARY_BLOCK_UPDATED, LIBRARY_BLOCK_DELETED, ) -from openedx.core.djangoapps.olx_rest_api.api import serialize_modulestore_block_for_blockstore from openedx.core.djangoapps.xblock.api import ( get_block_display_name, get_learning_context_impl, load_block, XBlockInclude, ) +from openedx.core.lib.xblock_serializer.api import serialize_modulestore_block_for_blockstore from openedx.core.lib.blockstore_api import ( get_bundle, get_bundles, diff --git a/openedx/core/djangoapps/content_staging/block_serializer.py b/openedx/core/djangoapps/content_staging/block_serializer.py deleted file mode 100644 index e1d6d353df36..000000000000 --- a/openedx/core/djangoapps/content_staging/block_serializer.py +++ /dev/null @@ -1,99 +0,0 @@ -""" -Code for serializing a modulestore XBlock to OLX suitable for import into -Blockstore. -""" -import logging -import os -from collections import namedtuple - -from lxml import etree - -from openedx.core.djangoapps.olx_rest_api.api import adapters - -log = logging.getLogger(__name__) - -# A static file required by an XBlock -StaticFile = namedtuple('StaticFile', ['name', 'url', 'data']) - - -class XBlockSerializer: - """ - A class that can serializer an XBlock to OLX - """ - # TEMP: this needs to be consolidated with the XBlockSerializer in olx_rest_api. - # i.e. have one base serializer, and a derived blockstore serializer - - def __init__(self, block): - """ - Serialize an XBlock to an OLX string + supporting files, and store the - resulting data in this object. - """ - self.orig_block_key = block.scope_ids.usage_id - self.static_files = [] - olx_node = self.serialize_block(block) - self.olx_str = etree.tostring(olx_node, encoding="unicode", pretty_print=True) - - course_key = self.orig_block_key.course_key - # Search the OLX for references to files stored in the course's - # "Files & Uploads" (contentstore): - self.olx_str = adapters.rewrite_absolute_static_urls(self.olx_str, course_key) - for asset in adapters.collect_assets_from_text(self.olx_str, course_key): - path = asset['path'] - if path not in [sf.name for sf in self.static_files]: - self.static_files.append(StaticFile(name=path, url=asset['url'], data=None)) - - def serialize_block(self, block) -> etree.Element: - if self.orig_block_key.block_type == 'html': - return self.serialize_html_block(block) - else: - return self.serialize_normal_block(block) - - def serialize_normal_block(self, block) -> etree.Element: - """ - Serialize an XBlock to XML. - - This method is used for every block type except HTML, which uses - serialize_html_block() instead. - """ - # Create an XML node to hold the exported data - olx_node = etree.Element("root") # The node name doesn't matter: add_xml_to_node will change it - # ^ Note: We could pass nsmap=xblock.core.XML_NAMESPACES here, but the - # resulting XML namespace attributes don't seem that useful? - with adapters.override_export_fs(block) as filesystem: # Needed for XBlocks that inherit XModuleDescriptor - # Tell the block to serialize itself as XML/OLX: - if not block.has_children: - block.add_xml_to_node(olx_node) - else: - # We don't want the children serialized at this time, because - # otherwise we can't tell which files in 'filesystem' belong to - # this block and which belong to its children. So, temporarily - # disable any children: - children = block.children - block.children = [] - block.add_xml_to_node(olx_node) - block.children = children - - # Now the block may have exported addtional data as files in - # 'filesystem'. If so, store them: - for item in filesystem.walk(): # pylint: disable=not-callable - for unit_file in item.files: - file_path = os.path.join(item.path, unit_file.name) - with filesystem.open(file_path, 'rb') as fh: - data = fh.read() - self.static_files.append(StaticFile(name=unit_file.name, data=data, url=None)) - # Recursively serialize the children: - if block.has_children: - for child in block.get_children(): - child_node = self.serialize_block(child) - olx_node.append(child_node) - return olx_node - - def serialize_html_block(self, block) -> etree.Element: - """ - Special case handling for HTML blocks - """ - olx_node = etree.Element("html") - if block.display_name: - olx_node.attrib["display_name"] = block.display_name - olx_node.text = etree.CDATA("\n" + block.data + "\n") - return olx_node diff --git a/openedx/core/djangoapps/content_staging/tests/test_clipboard.py b/openedx/core/djangoapps/content_staging/tests/test_clipboard.py index 4e042199a692..1313f05d7ce6 100644 --- a/openedx/core/djangoapps/content_staging/tests/test_clipboard.py +++ b/openedx/core/djangoapps/content_staging/tests/test_clipboard.py @@ -122,7 +122,7 @@ def test_copy_html(self): # For HTML, we really want to be sure that the OLX is serialized in this exact format (using CDATA), so we check # the actual string directly rather than using assertXmlEqual(): self.assertEqual(olx_response.content.decode(), dedent(""" - Sample ]]> """).lstrip()) diff --git a/openedx/core/djangoapps/content_staging/views.py b/openedx/core/djangoapps/content_staging/views.py index 96eae3db7156..3afed9352ba0 100644 --- a/openedx/core/djangoapps/content_staging/views.py +++ b/openedx/core/djangoapps/content_staging/views.py @@ -17,11 +17,11 @@ from common.djangoapps.student.auth import has_studio_read_access from openedx.core.lib.api.view_utils import view_auth_classes +from openedx.core.lib.xblock_serializer.api import serialize_xblock_to_olx from xmodule import block_metadata_utils from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError -from .block_serializer import XBlockSerializer from .models import StagedContent, UserClipboard from .serializers import UserClipboardSerializer, PostToClipboardSerializer from .tasks import delete_expired_clipboards @@ -110,7 +110,7 @@ def post(self, request): block = modulestore().get_item(usage_key) except ItemNotFoundError as exc: raise NotFound("The requested usage key does not exist.") from exc - block_data = XBlockSerializer(block) + block_data = serialize_xblock_to_olx(block) expired_ids = [] with transaction.atomic(): diff --git a/openedx/core/djangoapps/olx_rest_api/api.py b/openedx/core/djangoapps/olx_rest_api/api.py index 752e1e9ccab8..0703dfab3cac 100644 --- a/openedx/core/djangoapps/olx_rest_api/api.py +++ b/openedx/core/djangoapps/olx_rest_api/api.py @@ -1,22 +1,5 @@ """ Public Python API for the OLX REST API app """ -from .block_serializer import XBlockSerializer as _XBlockSerializer -# pylint: disable=unused-import -# 'adapters' are _temporarily_ part of the public API to keep the code DRY until -# we can consolidate the two different block_serializer implementations in -# content_staging and olx_rest_api. -from . import adapters - - -def serialize_modulestore_block_for_blockstore(block): - """ - Given a modulestore XBlock (e.g. loaded using - modulestore.get_item(block_key) - ), produce: - (1) A new definition ID for use in Blockstore - (2) an XML string defining the XBlock and referencing the IDs of its - children (but not containing the actual XML of its children) - (3) a list of any static files required by the XBlock and their URL - """ - return _XBlockSerializer(block) +# Currently there is no python API here. See openedx.core.lib.xblock_serializer +# for a python API to serialize XBlocks to OLX. diff --git a/openedx/core/djangoapps/olx_rest_api/test_views.py b/openedx/core/djangoapps/olx_rest_api/test_views.py index ba790f9d5c87..24b318ca1229 100644 --- a/openedx/core/djangoapps/olx_rest_api/test_views.py +++ b/openedx/core/djangoapps/olx_rest_api/test_views.py @@ -7,9 +7,9 @@ from openedx.core.djangolib.testing.utils import skip_unless_cms from common.djangoapps.student.roles import CourseStaffRole from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory -from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.factories import ToyCourseFactory # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import ToyCourseFactory @skip_unless_cms diff --git a/openedx/core/djangoapps/olx_rest_api/views.py b/openedx/core/djangoapps/olx_rest_api/views.py index bb1caa306194..8977b131530b 100644 --- a/openedx/core/djangoapps/olx_rest_api/views.py +++ b/openedx/core/djangoapps/olx_rest_api/views.py @@ -11,9 +11,9 @@ from common.djangoapps.student.auth import has_studio_read_access from openedx.core.lib.api.view_utils import view_auth_classes +from xmodule.modulestore.django import modulestore -from . import adapters -from .block_serializer import XBlockSerializer +from openedx.core.lib.xblock_serializer.api import serialize_modulestore_block_for_blockstore @api_view(['GET']) @@ -47,8 +47,8 @@ def serialize_block(block_key): if block_key in serialized_blocks: return - block = adapters.get_block(block_key) - serialized_blocks[block_key] = XBlockSerializer(block) + block = modulestore().get_item(block_key) + serialized_blocks[block_key] = serialize_modulestore_block_for_blockstore(block) if block.has_children: for child_id in block.children: @@ -102,8 +102,8 @@ def get_block_exportfs_file(request, usage_key_str, path): if not has_studio_read_access(request.user, course_key): raise PermissionDenied("You must be a member of the course team in Studio to export OLX using this API.") - block = adapters.get_block(usage_key) - serialized = XBlockSerializer(block) + block = modulestore().get_item(usage_key) + serialized = serialize_modulestore_block_for_blockstore(block) static_file = None for f in serialized.static_files: if f.name == path: diff --git a/openedx/core/lib/xblock_serializer/__init__.py b/openedx/core/lib/xblock_serializer/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/core/lib/xblock_serializer/api.py b/openedx/core/lib/xblock_serializer/api.py new file mode 100644 index 000000000000..97d04580f24b --- /dev/null +++ b/openedx/core/lib/xblock_serializer/api.py @@ -0,0 +1,27 @@ +""" +Public python API for serializing XBlocks to OLX +""" +# pylint: disable=unused-import +from .block_serializer import StaticFile, XBlockSerializer, XBlockSerializerForBlockstore + + +def serialize_xblock_to_olx(block): + """ + This class will serialize an XBlock, producing: + (1) an XML string defining the XBlock and all of its children (inline) + (2) a list of any static files required by the XBlock and their URL + """ + return XBlockSerializer(block) + + +def serialize_modulestore_block_for_blockstore(block): + """ + This class will serialize an XBlock, producing: + (1) A new definition ID for use in Blockstore + (2) an XML string defining the XBlock and referencing the IDs of its + children using syntax (which doesn't actually + contain the OLX of its children, just refers to them, so you have to + separately serialize them.) + (3) a list of any static files required by the XBlock and their URL + """ + return XBlockSerializerForBlockstore(block) diff --git a/openedx/core/djangoapps/olx_rest_api/block_serializer.py b/openedx/core/lib/xblock_serializer/block_serializer.py similarity index 52% rename from openedx/core/djangoapps/olx_rest_api/block_serializer.py rename to openedx/core/lib/xblock_serializer/block_serializer.py index 4e063601aebe..92bb7475c3d6 100644 --- a/openedx/core/djangoapps/olx_rest_api/block_serializer.py +++ b/openedx/core/lib/xblock_serializer/block_serializer.py @@ -1,6 +1,5 @@ """ -Code for serializing a modulestore XBlock to OLX suitable for import into -Blockstore. +Code for serializing a modulestore XBlock to OLX. """ import logging import os @@ -8,7 +7,7 @@ from lxml import etree -from . import adapters +from . import utils log = logging.getLogger(__name__) @@ -16,33 +15,9 @@ StaticFile = namedtuple('StaticFile', ['name', 'url', 'data']) -def blockstore_def_key_from_modulestore_usage_key(usage_key): - """ - In modulestore, the "definition key" is a MongoDB ObjectID kept in split's - definitions table, which theoretically allows the same block to be used in - many places (each with a unique usage key). However, that functionality is - not exposed in Studio (other than via content libraries). So when we import - into Blockstore, we assume that each usage is unique, don't generate a usage - key, and create a new "definition key" from the original usage key. - So modulestore usage key - block-v1:A+B+C+type@html+block@introduction - will become Blockstore definition key - html/introduction - """ - block_type = usage_key.block_type - if block_type == 'vertical': - # We transform to - block_type = "unit" - return block_type + "/" + usage_key.block_id - - class XBlockSerializer: """ - This class will serialize an XBlock, producing: - (1) A new definition ID for use in Blockstore - (2) an XML string defining the XBlock and referencing the IDs of its - children (but not containing the actual XML of its children) - (3) a list of any static files required by the XBlock and their URL + A class that can serialize an XBlock to OLX. """ def __init__(self, block): @@ -52,24 +27,26 @@ def __init__(self, block): """ self.orig_block_key = block.scope_ids.usage_id self.static_files = [] - self.def_id = blockstore_def_key_from_modulestore_usage_key(self.orig_block_key) - - # Special cases: - if self.orig_block_key.block_type == 'html': - self.serialize_html_block(block) - else: - self.serialize_normal_block(block) + olx_node = self._serialize_block(block) + self.olx_str = etree.tostring(olx_node, encoding="unicode", pretty_print=True) course_key = self.orig_block_key.course_key # Search the OLX for references to files stored in the course's # "Files & Uploads" (contentstore): - self.olx_str = adapters.rewrite_absolute_static_urls(self.olx_str, course_key) - for asset in adapters.collect_assets_from_text(self.olx_str, course_key): + self.olx_str = utils.rewrite_absolute_static_urls(self.olx_str, course_key) + for asset in utils.collect_assets_from_text(self.olx_str, course_key): path = asset['path'] if path not in [sf.name for sf in self.static_files]: self.static_files.append(StaticFile(name=path, url=asset['url'], data=None)) - def serialize_normal_block(self, block): + def _serialize_block(self, block) -> etree.Element: + """ Serialize an XBlock to OLX/XML. """ + if block.scope_ids.usage_id.block_type == 'html': + return self._serialize_html_block(block) + else: + return self._serialize_normal_block(block) + + def _serialize_normal_block(self, block) -> etree.Element: """ Serialize an XBlock to XML. @@ -80,7 +57,7 @@ def serialize_normal_block(self, block): olx_node = etree.Element("root") # The node name doesn't matter: add_xml_to_node will change it # ^ Note: We could pass nsmap=xblock.core.XML_NAMESPACES here, but the # resulting XML namespace attributes don't seem that useful? - with adapters.override_export_fs(block) as filesystem: # Needed for XBlocks that inherit XModuleDescriptor + with utils.override_export_fs(block) as filesystem: # Needed for XBlocks that inherit XModuleDescriptor # Tell the block to serialize itself as XML/OLX: if not block.has_children: block.add_xml_to_node(olx_node) @@ -102,47 +79,85 @@ def serialize_normal_block(self, block): with filesystem.open(file_path, 'rb') as fh: data = fh.read() self.static_files.append(StaticFile(name=unit_file.name, data=data, url=None)) - # Apply some transformations to the OLX: - self.transform_olx(olx_node, usage_id=block.scope_ids.usage_id) - # Add tags for each child (XBlock XML export - # normally puts children inline as e.g. tags, but we want - # references to them only.) if block.has_children: - for child_id in block.children: - # In modulestore, the "definition key" is a MongoDB ObjectID - # kept in split's definitions table, which theoretically allows - # the same block to be used in many places (each with a unique - # usage key). However, that functionality is not exposed in - # Studio (other than via content libraries). So when we import - # into Blockstore, we assume that each usage is unique, don't - # generate a usage key, and create a new "definition key" from - # the original usage key. - # So modulestore usage key - # block-v1:A+B+C+type@html+block@introduction - # will become Blockstore definition key - # html+introduction - # - # If we needed the real definition key, we could get it via - # child = block.runtime.get_block(child_id) - # child_def_id = str(child.scope_ids.def_id) - # and then use - # - def_id = blockstore_def_key_from_modulestore_usage_key(child_id) - olx_node.append(olx_node.makeelement("xblock-include", {"definition": def_id})) - # Store the resulting XML as a string: - self.olx_str = etree.tostring(olx_node, encoding="unicode", pretty_print=True) + self._serialize_children(block, olx_node) + return olx_node - def serialize_html_block(self, block): + def _serialize_children(self, block, parent_olx_node): + """ + Recursively serialize the children of XBlock 'block'. + Subclasses may override this. + """ + for child in block.get_children(): + child_node = self._serialize_block(child) + parent_olx_node.append(child_node) + + def _serialize_html_block(self, block) -> etree.Element: """ Special case handling for HTML blocks """ olx_node = etree.Element("html") + olx_node.attrib["url_name"] = block.scope_ids.usage_id.block_id if block.display_name: olx_node.attrib["display_name"] = block.display_name olx_node.text = etree.CDATA("\n" + block.data + "\n") - self.olx_str = etree.tostring(olx_node, encoding="unicode", pretty_print=True) + return olx_node - def transform_olx(self, olx_node, usage_id): + +class XBlockSerializerForBlockstore(XBlockSerializer): + """ + This class will serialize an XBlock, producing: + (1) A new definition ID for use in Blockstore + (2) an XML string defining the XBlock and referencing the IDs of its + children using syntax (which doesn't actually + contain the OLX of its children, just refers to them, so you have to + separately serialize them.) + (3) a list of any static files required by the XBlock and their URL + """ + + def __init__(self, block): + """ + Serialize an XBlock to an OLX string + supporting files, and store the + resulting data in this object. + """ + super().__init__(block) + self.def_id = utils.blockstore_def_key_from_modulestore_usage_key(self.orig_block_key) + + def _serialize_block(self, block) -> etree.Element: + """ Serialize an XBlock to OLX/XML. """ + olx_node = super()._serialize_block(block) + # Apply some transformations to the OLX: + self._transform_olx(olx_node, usage_id=block.scope_ids.usage_id) + return olx_node + + def _serialize_children(self, block, parent_olx_node): + """ + Recursively serialize the children of XBlock 'block'. + Subclasses may override this. + """ + for child_id in block.children: + # In modulestore, the "definition key" is a MongoDB ObjectID + # kept in split's definitions table, which theoretically allows + # the same block to be used in many places (each with a unique + # usage key). However, that functionality is not exposed in + # Studio (other than via content libraries). So when we import + # into Blockstore, we assume that each usage is unique, don't + # generate a usage key, and create a new "definition key" from + # the original usage key. + # So modulestore usage key + # block-v1:A+B+C+type@html+block@introduction + # will become Blockstore definition key + # html+introduction + # + # If we needed the real definition key, we could get it via + # child = block.runtime.get_block(child_id) + # child_def_id = str(child.scope_ids.def_id) + # and then use + # + def_id = utils.blockstore_def_key_from_modulestore_usage_key(child_id) + parent_olx_node.append(parent_olx_node.makeelement("xblock-include", {"definition": def_id})) + + def _transform_olx(self, olx_node, usage_id): """ Apply transformations to the given OLX etree Node. """ diff --git a/openedx/core/lib/xblock_serializer/test_api.py b/openedx/core/lib/xblock_serializer/test_api.py new file mode 100644 index 000000000000..f9258f18551a --- /dev/null +++ b/openedx/core/lib/xblock_serializer/test_api.py @@ -0,0 +1,167 @@ +""" +Test for the XBlock serialization lib's API +""" +from xml.etree import ElementTree + +from openedx.core.djangolib.testing.utils import skip_unless_cms +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import ToyCourseFactory + +from . import api + + +# The expected OLX string for the 'Toy_Videos' sequential in the toy course +EXPECTED_SEQUENTIAL_OLX = """ + + Lab 2A: Superposition Experiment + + +

Isn't the toy course great?

+ +

Let's add some markup that uses non-ascii characters. +'For example, we should be able to write words like encyclopædia, or foreign words like français. +Looking beyond latin-1, we should handle math symbols: πr² ≤ ∞. +And it shouldn't matter if we use entities or numeric codes — Ω ≠ π ≡ Ω ≠ π. +

+ + +]]> + This is a link to another page and some Chinese 四節比分和七年前

Some more Chinese 四節比分和七年前

+ +]]> + Sample +]]> + link + +]]> + link + + +]]> + + +]]> + Red text here

+]]> + +]]> + +""" + + +@skip_unless_cms +class XBlockSerializationTestCase(SharedModuleStoreTestCase): + """ + Test for the XBlock serialization library's python API + """ + + @classmethod + def setUpClass(cls): + """ + Set up a course for use in these tests + """ + super().setUpClass() + cls.course = ToyCourseFactory.create() + + def assertXmlEqual(self, xml_str_a: str, xml_str_b: str) -> bool: + """ Assert that the given XML strings are equal, ignoring attribute order and some whitespace variations. """ + self.assertEqual( + ElementTree.canonicalize(xml_str_a, strip_text=True), + ElementTree.canonicalize(xml_str_b, strip_text=True), + ) + + def test_html_with_static_asset(self): + """ + Test that HTML gets converted to use CDATA and static assets are + handled. + """ + block_id = self.course.id.make_usage_key('html', 'just_img') # see sample_courses.py + html_block = modulestore().get_item(block_id) + serialized = api.serialize_xblock_to_olx(html_block) + + self.assertXmlEqual( + serialized.olx_str, + """ + + ]]> + """ + ) + self.assertIn("CDATA", serialized.olx_str) + self.assertEqual(serialized.static_files, [ + api.StaticFile( + name="foo_bar.jpg", + url="/asset-v1:edX+toy+2012_Fall+type@asset+block@foo_bar.jpg", + data=None, + ), + ]) + + def test_html_with_static_asset_blockstore(self): + """ + Test the blockstore-specific serialization of an HTML block + """ + block_id = self.course.id.make_usage_key('html', 'just_img') # see sample_courses.py + html_block = modulestore().get_item(block_id) + serialized = api.serialize_xblock_to_olx(html_block) + serialized_blockstore = api.serialize_modulestore_block_for_blockstore(html_block) + self.assertXmlEqual( + serialized_blockstore.olx_str, + # For blockstore, OLX should never contain "url_name" as that ID is specified by the filename: + """ + + ]]> + """ + ) + self.assertIn("CDATA", serialized.olx_str) + # Static files should be identical: + self.assertEqual(serialized.static_files, serialized_blockstore.static_files) + # This is the only other difference - an extra field with the blockstore-specific definition ID: + self.assertEqual(serialized_blockstore.def_id, "html/just_img") + + def test_export_sequential(self): + """ + Export a sequential from the toy course, including all of its children. + """ + sequential_id = self.course.id.make_usage_key('sequential', 'Toy_Videos') # see sample_courses.py + sequential = modulestore().get_item(sequential_id) + serialized = api.serialize_xblock_to_olx(sequential) + + self.assertXmlEqual(serialized.olx_str, EXPECTED_SEQUENTIAL_OLX) + + def test_export_sequential_blockstore(self): + """ + Export a sequential from the toy course, formatted for blockstore. + """ + sequential_id = self.course.id.make_usage_key('sequential', 'Toy_Videos') # see sample_courses.py + sequential = modulestore().get_item(sequential_id) + serialized = api.serialize_modulestore_block_for_blockstore(sequential) + + self.assertXmlEqual(serialized.olx_str, """ + + + + + + + + + + + + """) diff --git a/openedx/core/djangoapps/olx_rest_api/test_adapters.py b/openedx/core/lib/xblock_serializer/test_utils.py similarity index 87% rename from openedx/core/djangoapps/olx_rest_api/test_adapters.py rename to openedx/core/lib/xblock_serializer/test_utils.py index ec029758c3c2..5517a692732b 100644 --- a/openedx/core/djangoapps/olx_rest_api/test_adapters.py +++ b/openedx/core/lib/xblock_serializer/test_utils.py @@ -1,16 +1,16 @@ """ -Test the OLX REST API adapters code +Test the OLX serialization utils """ import unittest from opaque_keys.edx.keys import CourseKey -from openedx.core.djangoapps.olx_rest_api import adapters +from . import utils -class TestAdapters(unittest.TestCase): +class TestUtils(unittest.TestCase): """ - Test the OLX REST API adapters code + Test the OLX serialization utils """ def test_rewrite_absolute_static_urls(self): @@ -45,5 +45,5 @@ def test_rewrite_absolute_static_urls(self): """ - olx_out = adapters.rewrite_absolute_static_urls(olx_in, course_id) + olx_out = utils.rewrite_absolute_static_urls(olx_in, course_id) assert olx_out == olx_expected diff --git a/openedx/core/djangoapps/olx_rest_api/adapters.py b/openedx/core/lib/xblock_serializer/utils.py similarity index 85% rename from openedx/core/djangoapps/olx_rest_api/adapters.py rename to openedx/core/lib/xblock_serializer/utils.py index cc5c59f896a4..fd2253ccad44 100644 --- a/openedx/core/djangoapps/olx_rest_api/adapters.py +++ b/openedx/core/lib/xblock_serializer/utils.py @@ -1,5 +1,5 @@ """ -Helpers required to adapt to differing APIs +Helper functions for XBlock serialization """ import logging import re @@ -12,7 +12,6 @@ from xmodule.assetstore.assetmgr import AssetManager from xmodule.contentstore.content import StaticContent from xmodule.exceptions import NotFoundError -from xmodule.modulestore.django import modulestore as store from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.xml_block import XmlMixin @@ -21,13 +20,6 @@ log = logging.getLogger(__name__) -def get_block(usage_key): - """ - Return an XBlock from modulestore. - """ - return store().get_item(usage_key) - - def get_asset_content_from_path(course_key, asset_path): """ Locate the given asset content, load it into memory, and return it. @@ -137,3 +129,23 @@ def override_export_fs(block): if hasattr(block, 'export_to_file'): block.export_to_file = old_export_to_file XmlMixin.export_to_file = old_global_export_to_file + + +def blockstore_def_key_from_modulestore_usage_key(usage_key): + """ + In modulestore, the "definition key" is a MongoDB ObjectID kept in split's + definitions table, which theoretically allows the same block to be used in + many places (each with a unique usage key). However, that functionality is + not exposed in Studio (other than via content libraries). So when we import + into Blockstore, we assume that each usage is unique, don't generate a usage + key, and create a new "definition key" from the original usage key. + So modulestore usage key + block-v1:A+B+C+type@html+block@introduction + will become Blockstore definition key + html/introduction + """ + block_type = usage_key.block_type + if block_type == 'vertical': + # We transform to + block_type = "unit" + return block_type + "/" + usage_key.block_id diff --git a/setup.cfg b/setup.cfg index 7fcb03ff1530..e588252266cb 100644 --- a/setup.cfg +++ b/setup.cfg @@ -164,6 +164,7 @@ isolated_apps = openedx.core.djangoapps.content_libraries openedx.core.djangoapps.olx_rest_api openedx.core.djangoapps.xblock + openedx.core.lib.xblock_serializer allowed_modules = # Only imports from api.py are allowed elsewhere in the code # See https://open-edx-proposals.readthedocs.io/en/latest/best-practices/oep-0049-django-app-patterns.html#api-py