Merge pull request #30072 from open-craft/agrendalath/bd-13-simplify_xmodule_serialization

reafactor: simplify XModule serialization/deserialization layer [BD-13]
This commit is contained in:
Piotr Surowiec
2022-12-07 18:01:57 +01:00
committed by GitHub
17 changed files with 127 additions and 230 deletions

View File

@@ -1,22 +1,22 @@
"""
Helpers required to adapt to differing APIs
"""
from contextlib import contextmanager
import logging
import re
from contextlib import contextmanager
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import AssetKey, CourseKey
from fs.memoryfs import MemoryFS
from fs.wrapfs import WrapFS
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import AssetKey, CourseKey
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_module import XmlMixin
from common.djangoapps.static_replace import replace_static_urls
from xmodule.contentstore.content import StaticContent # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.assetstore.assetmgr import AssetManager # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore as store # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.exceptions import NotFoundError # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.xml_module import XmlParserMixin # lint-amnesty, pylint: disable=wrong-import-order
log = logging.getLogger(__name__)
@@ -105,7 +105,7 @@ def override_export_fs(block):
XmlSerializationMixin.add_xml_to_node() method.
This method temporarily replaces a block's runtime's
'export_fs' system with an in-memory filesystem.
This method also abuses the XmlParserMixin.export_to_file()
This method also abuses the XmlMixin.export_to_file()
API to prevent the XModule export code from exporting each
block as two files (one .olx pointing to one .xml file).
The export_to_file was meant to be used only by the
@@ -120,10 +120,10 @@ def override_export_fs(block):
if hasattr(block, 'export_to_file'):
old_export_to_file = block.export_to_file
block.export_to_file = lambda: False
old_global_export_to_file = XmlParserMixin.export_to_file
XmlParserMixin.export_to_file = lambda _: False # So this applies to child blocks that get loaded during export
old_global_export_to_file = XmlMixin.export_to_file
XmlMixin.export_to_file = lambda _: False # So this applies to child blocks that get loaded during export
yield fs
block.runtime.export_fs = old_export_fs
if hasattr(block, 'export_to_file'):
block.export_to_file = old_export_to_file
XmlParserMixin.export_to_file = old_global_export_to_file
XmlMixin.export_to_file = old_global_export_to_file

View File

@@ -67,7 +67,7 @@ class BlockstoreXBlockRuntime(XBlockRuntime):
# This is a (former) XModule with messy XML parsing code; let its parse_xml() method continue to work
# as it currently does in the old runtime, but let this parse_xml_new_runtime() method parse the XML in
# a simpler way that's free of tech debt, if defined.
# In particular, XmlParserMixin doesn't play well with this new runtime, so this is mostly about
# In particular, XmlMixin doesn't play well with this new runtime, so this is mostly about
# bypassing that mixin's code.
# When a former XModule no longer needs to support the old runtime, its parse_xml_new_runtime method
# should be removed and its parse_xml() method should be simplified to just call the super().parse_xml()

View File

@@ -12,7 +12,7 @@ from fs.wrapfs import WrapFS
from lxml.etree import Element
from lxml.etree import tostring as etree_tostring
from xmodule.xml_module import XmlParserMixin
from xmodule.xml_module import XmlMixin
log = logging.getLogger(__name__)
@@ -104,7 +104,7 @@ def override_export_fs(block):
This method temporarily replaces a block's runtime's 'export_fs' system with
an in-memory filesystem. This method also abuses the
XmlParserMixin.export_to_file()
XmlMixin.export_to_file()
API to prevent the XModule export code from exporting each block as two
files (one .olx pointing to one .xml file). The export_to_file was meant to
be used only by the customtag XModule but it makes our lives here much
@@ -119,8 +119,8 @@ def override_export_fs(block):
if hasattr(block, 'export_to_file'):
old_export_to_file = block.export_to_file
block.export_to_file = lambda: False
old_global_export_to_file = XmlParserMixin.export_to_file
XmlParserMixin.export_to_file = lambda _: False # So this applies to child blocks that get loaded during export
old_global_export_to_file = XmlMixin.export_to_file
XmlMixin.export_to_file = lambda _: False # So this applies to child blocks that get loaded during export
try:
yield fs
except: # lint-amnesty, pylint: disable=try-except-raise
@@ -129,4 +129,4 @@ def override_export_fs(block):
block.runtime.export_fs = old_export_fs
if hasattr(block, 'export_to_file'):
block.export_to_file = old_export_to_file
XmlParserMixin.export_to_file = old_global_export_to_file
XmlMixin.export_to_file = old_global_export_to_file

View File

@@ -154,7 +154,7 @@ class RuntimeShim:
def process_xml(self, xml):
"""
Code to handle parsing of child XML for old blocks that use XmlParserMixin.
Code to handle parsing of child XML for old blocks that use XmlMixin.
"""
# We can't parse XML in a vacuum - we need to know the parent block and/or the
# OLX file that holds this XML in order to generate useful definition keys etc.