chore: add pointer tag parsing logic to LTIBlock

Author: ttqureshi

xblocks_contrib/lti/lti.py

@@ -84,7 +84,7 @@
     from xblockutils.resources import ResourceLoader
     from xblockutils.studio_editable import StudioEditableXBlockMixin
 
-from .lti_2_util import LTI20BlockMixin, LTIError
+from .lti_2_util import LTI20BlockMixin, LTIError, LTIXmlMixin
 
 # The anonymous user ID for the user in the course.
 ATTR_KEY_ANONYMOUS_USER_ID = 'edx-platform.anonymous_user_id'
@@ -290,6 +290,7 @@ class LTIFields:
 class LTIBlock(
     LTIFields,
     LTI20BlockMixin,
+    LTIXmlMixin,
     StudioEditableXBlockMixin,
     XBlock,
 ):

xblocks_contrib/lti/lti_2_util.py

@@ -5,6 +5,7 @@
 
 
 import base64
+import copy
 import hashlib
 import json
 import logging
@@ -14,9 +15,22 @@
 from urllib import parse
 
 from django.conf import settings
+from lxml import etree
 from oauthlib.oauth1 import Client
 from webob import Response
 from xblock.core import XBlock
+from xblock.fields import Dict, Scope, ScopeIds
+
+from xblocks_contrib.common.xml_utils import (
+    apply_pointer_attributes,
+    deserialize_field,
+    format_filepath,
+    is_pointer_tag,
+    load_definition_xml,
+    name_to_pathname,
+    own_metadata,
+    serialize_field,
+)
 
 log = logging.getLogger(__name__)
 
@@ -391,3 +405,219 @@ def parse_lti_2_0_result_json(self, json_str):
             raise LTIError(msg)  # lint-amnesty, pylint: disable=raise-missing-from
 
         return score, json_obj.get('comment', "")
+
+
+class LTIXmlMixin:
+    """
+    A mixin class to add XML parsing functionality to LTIBlock.
+    """
+
+    metadata_to_strip = (
+        'data_dir',
+        'tabs', 'grading_policy',
+        'discussion_blackouts',
+        # VS[compat]
+        # These attributes should have been removed from here once all 2012-fall courses imported into
+        # the CMS and "inline" OLX format deprecated. But, it never got deprecated. Moreover, it's
+        # widely used to this date. So, we still have to strip them. Also, removing of "filename"
+        # changes OLX returned by `/api/olx-export/v1/xblock/{block_id}/`, which indicates that some
+        # places in the platform rely on it.
+        'course', 'org', 'url_name', 'filename',
+        # Used for storing xml attributes between import and export, for roundtrips
+        'xml_attributes',
+        # Used by _import_xml_node_to_parent in cms/djangoapps/contentstore/helpers.py to prevent
+        # XmlMixin from treating some XML nodes as "pointer nodes".
+        "x-is-pointer-node",
+    )
+
+    xml_attributes = Dict(
+        help="Map of unhandled xml attributes, used only for storage between import and export",
+        default={},
+        scope=Scope.settings
+    )
+
+    @classmethod
+    def apply_policy(cls, metadata, policy):
+        """
+        Add the keys in policy to metadata, after processing them
+        through the attrmap.  Updates the metadata dict in place.
+        """
+        for attr, value in policy.items():
+            if attr not in cls.fields:
+                # Store unknown attributes coming from policy.json
+                # in such a way that they will export to xml unchanged
+                metadata['xml_attributes'][attr] = value
+            else:
+                metadata[attr] = value
+
+    @staticmethod
+    def _get_metadata_from_xml(xml_object, remove=True):
+        """
+        Extract the metadata from the XML.
+        """
+        meta = xml_object.find('meta')
+        if meta is None:
+            return ''
+        dmdata = meta.text
+        if remove:
+            xml_object.remove(meta)
+        return dmdata
+
+    @classmethod
+    def clean_metadata_from_xml(cls, xml_object, excluded_fields=()):
+        """
+        Remove any attribute named for a field with scope Scope.settings from the supplied
+        xml_object
+        """
+        for field_name, field in cls.fields.items():
+            if (field.scope == Scope.settings
+                    and field_name not in excluded_fields
+                    and xml_object.get(field_name) is not None):
+                del xml_object.attrib[field_name]
+
+    @classmethod
+    def load_definition(cls, xml_object):
+        """
+        Load a block from the specified xml_object.
+
+        Args:
+            xml_object: an lxml.etree._Element containing the definition to load
+        """
+
+        filename = xml_object.get('filename')
+        definition_xml = copy.deepcopy(xml_object)
+        filepath = ''
+
+        definition_metadata = cls._get_metadata_from_xml(definition_xml)
+        cls.clean_metadata_from_xml(definition_xml)
+
+        if len(xml_object) == 0 and len(list(xml_object.items())) == 0:
+            definition, children = {'data': ''}, []
+        else:
+            definition, children = {'data': etree.tostring(xml_object, pretty_print=True, encoding='unicode')}, []
+
+        if definition_metadata:
+            definition['definition_metadata'] = definition_metadata
+        definition['filename'] = [filepath, filename]
+
+        return definition, children
+
+    @classmethod
+    def load_metadata(cls, xml_object):
+        """
+        Read the metadata attributes from this xml_object.
+
+        Returns a dictionary {key: value}.
+        """
+        metadata = {'xml_attributes': {}}
+        for attr, val in xml_object.attrib.items():
+
+            if attr in cls.metadata_to_strip:
+                # don't load these
+                continue
+
+            if attr not in cls.fields:
+                metadata['xml_attributes'][attr] = val
+            else:
+                metadata[attr] = deserialize_field(cls.fields[attr], val)
+        return metadata
+
+    @classmethod
+    def parse_xml(cls, node, runtime, keys):
+        """
+        Use `node` to construct a new block.
+
+        Arguments:
+            node (etree.Element): The xml node to parse into an xblock.
+
+            runtime (:class:`.Runtime`): The runtime to use while parsing.
+
+            keys (:class:`.ScopeIds`): The keys identifying where this block
+                will store its data.
+
+        Returns (XBlock): The newly parsed XBlock
+
+        """
+        import pdb ; pdb.set_trace()
+        if keys is None:
+            # Passing keys=None is against the XBlock API but some platform tests do it.
+            def_id = runtime.id_generator.create_definition(node.tag, node.get("url_name"))
+            keys = ScopeIds(None, node.tag, def_id, runtime.id_generator.create_usage(def_id))
+        aside_children = []
+
+        # Let the runtime construct the block. It will have a proper, inheritance-aware field data store.
+        block = runtime.construct_xblock_from_class(cls, keys)
+
+        # VS[compat]
+        # In 2012, when the platform didn't have CMS, and all courses were handwritten XML files, problem tags
+        # contained XML problem descriptions withing themselves. Later, when Studio has been created, and "pointer" tags
+        # became the preferred problem format, edX has to add this compatibility code to 1) support both pre- and
+        # post-Studio course formats simulteneously, and 2) be able to migrate 2012-fall courses to Studio. Old style
+        # support supposed to be removed, but the deprecation process have never been initiated, so this
+        # compatibility must stay, probably forever.
+        if is_pointer_tag(node):
+            # new style:
+            # read the actual definition file--named using url_name.replace(':','/')
+            definition_xml, filepath = load_definition_xml(node, runtime, keys.def_id)
+            aside_children = runtime.parse_asides(definition_xml, keys.def_id, keys.usage_id, runtime.id_generator)
+        else:
+            filepath = None
+            definition_xml = node
+
+        # Removes metadata
+        definition, children = cls.load_definition(definition_xml)
+
+        # VS[compat]
+        # Make Ike's github preview links work in both old and new file layouts.
+        if is_pointer_tag(node):
+            # new style -- contents actually at filepath
+            definition["filename"] = [filepath, filepath]
+
+        metadata = cls.load_metadata(definition_xml)
+
+        # move definition metadata into dict
+        dmdata = definition.get("definition_metadata", "")
+        if dmdata:
+            metadata["definition_metadata_raw"] = dmdata
+            try:
+                metadata.update(json.loads(dmdata))
+            except Exception as err:  # lint-amnesty, pylint: disable=broad-except
+                log.debug("Error in loading metadata %r", dmdata, exc_info=True)
+                metadata["definition_metadata_err"] = str(err)
+
+        definition_aside_children = definition.pop("aside_children", None)
+        if definition_aside_children:
+            aside_children.extend(definition_aside_children)
+
+        # Set/override any metadata specified by policy
+        cls.apply_policy(metadata, runtime.get_policy(keys.usage_id))
+
+        field_data = {**metadata, **definition}
+
+        for field_name, value in field_data.items():
+            # The 'xml_attributes' field has a special setter logic in its Field class,
+            # so we must handle it carefully to avoid duplicating data.
+            if field_name == "xml_attributes":
+                # The 'filename' attribute is specially handled for git links.
+                value["filename"] = definition.get("filename", ["", None])
+                block.xml_attributes.update(value)
+            elif field_name in block.fields:
+                setattr(block, field_name, value)
+
+        block.children = children
+
+        if aside_children:
+            cls.add_applicable_asides_to_block(block, runtime, aside_children)
+
+        return block
+
+    @classmethod
+    def add_applicable_asides_to_block(cls, block, runtime, aside_children):
+        """
+        Add asides to the block. Moved this out of the parse_xml method to use it in the VideoBlock.parse_xml
+        """
+        asides_tags = [aside_child.tag for aside_child in aside_children]
+        asides = runtime.get_asides(block)
+        for aside in asides:
+            if aside.scope_ids.block_type in asides_tags:
+                block.add_aside(aside)