Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: replace XmlMixin with XmlParserMixin

Browse Source 
Most of the methods in `XmlMixin` act as wrappers for the official API for
serialization and deserialization (parse_xml() and add_xml_to_node()).
`XmlParserMixin` contains the code which does the actual serialization and
deserialization.
This commit is contained in:
Agrendalath
2022-03-15 23:54:10 +01:00
committed by Piotr Surowiec
parent 499f09e2fb
commit 8127d19115
9 changed files with 34 additions and 131 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
openedx/core/djangoapps/olx_rest_api/adapters.py
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

2
openedx/core/djangoapps/xblock/runtime/blockstore_runtime.py
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()

10
openedx/core/djangoapps/xblock/runtime/serializer.py
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

2
openedx/core/djangoapps/xblock/runtime/shims.py
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.

4
xmodule/discussion_block.py
View File

@@ -18,7 +18,7 @@ from xblockutils.studio_editable import StudioEditableXBlockMixin
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider
from openedx.core.djangolib.markup import HTML, Text
from openedx.core.lib.xblock_utils import get_css_dependencies, get_js_dependencies
from xmodule.xml_module import XmlParserMixin
from xmodule.xml_module import XmlMixin
log = logging.getLogger(__name__)
loader = ResourceLoader(__name__) # pylint: disable=invalid-name
@@ -34,7 +34,7 @@ def _(text):
@XBlock.needs('user') # pylint: disable=abstract-method
@XBlock.needs('i18n')
@XBlock.needs('mako')
class DiscussionXBlock(XBlock, StudioEditableXBlockMixin, XmlParserMixin): # lint-amnesty, pylint: disable=abstract-method
class DiscussionXBlock(XBlock, StudioEditableXBlockMixin, XmlMixin): # lint-amnesty, pylint: disable=abstract-method
"""
Provides a discussion forum that is inline with other content in the courseware.
"""

11
xmodule/tests/test_poll.py
View File

@@ -2,14 +2,14 @@
import json
import unittest
from unittest.mock import Mock
from opaque_keys.edx.keys import CourseKey
from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds
from xmodule.poll_module import PollBlock
from openedx.core.lib.safe_lxml import etree
from xmodule.poll_module import PollBlock
from . import get_test_system
from .test_import import DummySystem
@@ -29,9 +29,9 @@ class PollBlockTest(unittest.TestCase):
self.system = get_test_system(course_key)
usage_key = course_key.make_usage_key(PollBlock.category, 'test_loc')
# ScopeIds has 4 fields: user_id, block_type, def_id, usage_id
scope_ids = ScopeIds(1, PollBlock.category, usage_key, usage_key)
self.scope_ids = ScopeIds(1, PollBlock.category, usage_key, usage_key)
self.xmodule = PollBlock(
self.system, DictFieldData(self.raw_field_data), scope_ids
self.system, DictFieldData(self.raw_field_data), self.scope_ids
)
def ajax_request(self, dispatch, data):
@@ -70,8 +70,9 @@ class PollBlockTest(unittest.TestCase):
<answer id="less18">18</answer>
</poll_question>
'''
node = etree.fromstring(sample_poll_xml)
output = PollBlock.from_xml(sample_poll_xml, module_system, id_generator)
output = PollBlock.parse_xml(node, module_system, self.scope_ids, id_generator)
# Update the answer with invalid character.
invalid_characters_poll_answer = output.answers[0]
# Invalid less-than character.

4
xmodule/vertical_block.py
View File

@@ -21,7 +21,7 @@ from xmodule.studio_editable import StudioEditableBlock
from xmodule.util.misc import is_xblock_an_assignment
from xmodule.util.xmodule_django import add_webpack_to_fragment
from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW, XModuleFields
from xmodule.xml_module import XmlParserMixin
from xmodule.xml_module import XmlMixin
log = logging.getLogger(__name__)
@@ -55,7 +55,7 @@ class VerticalBlock(
VerticalFields,
XModuleFields,
StudioEditableBlock,
XmlParserMixin,
XmlMixin,
MakoTemplateBlockBase,
XBlock
):

2
xmodule/x_module.py
View File

@@ -1169,7 +1169,7 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime):
raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls")
def add_block_as_child_node(self, block, node):
child = etree.SubElement(node, "unknown")
child = etree.SubElement(node, block.category)
child.set('url_name', block.url_name)
block.add_xml_to_node(child)

104
xmodule/xml_module.py
View File

@@ -7,7 +7,7 @@ import logging
import os
from lxml import etree
from lxml.etree import Element, ElementTree, XMLParser
from lxml.etree import ElementTree, XMLParser
from xblock.core import XBlock, XML_NAMESPACES
from xblock.fields import Dict, Scope, ScopeIds
from xblock.runtime import KvsFieldData
@@ -103,7 +103,7 @@ def deserialize_field(field, value):
return value
class XmlParserMixin:
class XmlMixin:
"""
Class containing XML parsing functionality shared between XBlock and XModuleDescriptor.
"""
@@ -503,108 +503,10 @@ class XmlParserMixin:
Return a list of all metadata fields that cannot be edited.
"""
non_editable_fields = super().non_editable_metadata_fields
non_editable_fields.append(XmlParserMixin.xml_attributes)
non_editable_fields.append(XmlMixin.xml_attributes)
return non_editable_fields
class XmlMixin(XmlParserMixin): # lint-amnesty, pylint: disable=abstract-method
"""
Mixin class for standardized parsing of XModule xml.
"""
@classmethod
def from_xml(cls, xml_data, system, id_generator):
"""
Creates an instance of this descriptor from the supplied xml_data.
This may be overridden by subclasses.
Args:
xml_data (str): A string of xml that will be translated into data and children
for this module
system (:class:`.XMLParsingSystem):
id_generator (:class:`xblock.runtime.IdGenerator`): Used to generate the
usage_ids and definition_ids when loading this xml
"""
# Shim from from_xml to the parse_xml defined in XmlParserMixin.
# This only exists to satisfy subclasses that both:
# a) define from_xml themselves
# b) call super(..).from_xml(..)
return super().parse_xml(
etree.fromstring(xml_data),
system,
None, # This is ignored by XmlParserMixin
id_generator,
)
@classmethod
def parse_xml(cls, node, runtime, keys, id_generator):
"""
Interpret the parsed XML in `node`, creating an XModuleDescriptor.
"""
if cls.from_xml != XmlMixin.from_xml:
xml = etree.tostring(node).decode('utf-8')
block = cls.from_xml(xml, runtime, id_generator)
return block
else:
return super().parse_xml(node, runtime, keys, id_generator)
@classmethod
def parse_xml_new_runtime(cls, node, runtime, keys):
"""
This XML lives within Blockstore and the new runtime doesn't need this
legacy XModule code. Use the "normal" XBlock parsing code.
"""
try:
return super().parse_xml_new_runtime(node, runtime, keys)
except AttributeError:
return super().parse_xml(node, runtime, keys, id_generator=None)
def export_to_xml(self, resource_fs): # lint-amnesty, pylint: disable=unused-argument
"""
Returns an xml string representing this module, and all modules
underneath it. May also write required resources out to resource_fs.
Assumes that modules have single parentage (that no module appears twice
in the same course), and that it is thus safe to nest modules as xml
children as appropriate.
The returned XML should be able to be parsed back into an identical
XModuleDescriptor using the from_xml method with the same system, org,
and course
"""
# Shim from export_to_xml to the add_xml_to_node defined in XmlParserMixin.
# This only exists to satisfy subclasses that both:
# a) define export_to_xml themselves
# b) call super(..).export_to_xml(..)
node = Element(self.category)
super().add_xml_to_node(node)
return etree.tostring(node)
def add_xml_to_node(self, node):
"""
Export this :class:`XModuleDescriptor` as XML, by setting attributes on the provided
`node`.
"""
if self.export_to_xml != XmlMixin.export_to_xml: # lint-amnesty, pylint: disable=comparison-with-callable
xml_string = self.export_to_xml(self.runtime.export_fs)
exported_node = etree.fromstring(xml_string)
node.tag = exported_node.tag
node.text = exported_node.text
node.tail = exported_node.tail
for key, value in exported_node.items():
if key == 'url_name' and value == 'course' and key in node.attrib:
# if url_name is set in ExportManager then do not override it here.
continue
node.set(key, value)
node.extend(list(exported_node))
else:
super().add_xml_to_node(node)
@XBlock.needs("i18n")
class XmlDescriptor(XmlMixin, XModuleMixin): # lint-amnesty, pylint: disable=abstract-method
pass