From 89d5317246be5650b263a38461d7f7aa3c52fb04 Mon Sep 17 00:00:00 2001 From: "M. Tayyab Tahir Qureshi" Date: Thu, 23 Oct 2025 16:05:58 +0500 Subject: [PATCH 1/4] refactor: move pointer tag common utility functions to `xml_utils.py` --- xblocks_contrib/common/xml_utils.py | 187 ++++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 xblocks_contrib/common/xml_utils.py diff --git a/xblocks_contrib/common/xml_utils.py b/xblocks_contrib/common/xml_utils.py new file mode 100644 index 00000000..4565f75f --- /dev/null +++ b/xblocks_contrib/common/xml_utils.py @@ -0,0 +1,187 @@ +"""Shared utilities for pointer tag handling across XBlocks. + +This module centralizes detection, path computation, and attribute application +for OLX "pointer" tags used by XBlocks during import/export. +Also provides helpers to load definition XML files referenced by pointer tags. + +Note: The functionality has been taken from the edx-platform's XmlMixin class. +https://github.com/openedx/edx-platform/blob/18d5abb2f641db7f364f7566187e57bebdae9fe9/xmodule/xml_block.py +""" + +import datetime +import json + +from django.core.serializers.json import DjangoJSONEncoder +from lxml import etree +from opaque_keys.edx.keys import CourseKey, UsageKey +from xblock.fields import Scope + +# Assume all XML files are persisted as utf-8. +EDX_XML_PARSER = etree.XMLParser(dtd_validation=False, load_dtd=False, remove_blank_text=True, encoding="utf-8") + +filename_extension = "xml" + + +class EdxJSONEncoder(DjangoJSONEncoder): + """ + Custom JSONEncoder that handles ``Location`` and ``datetime.datetime`` objects. + Encodes ``Location`` as its URL string form, and ``datetime.datetime`` as an ISO 8601 string. + """ + + def default(self, o): + if isinstance(o, (CourseKey, UsageKey)): + return str(o) + elif isinstance(o, datetime.datetime): + if o.tzinfo is not None: + if o.utcoffset() is None: + return o.isoformat() + "Z" + else: + return o.isoformat() + else: + return o.isoformat() + else: + return super().default(o) + + +def name_to_pathname(name): + """ + Convert a location name for use in a path: replace ':' with '/'. + This allows users of the xml format to organize content into directories + """ + return name.replace(":", "/") + + +def is_pointer_tag(xml_obj): + """ + Check if xml_obj is a pointer tag: . + No children, one attribute named url_name, no text. + + Special case for course roots: the pointer is + + + xml_obj: an etree Element + + Returns a bool. + """ + if xml_obj.tag != "course": + expected_attr = {"url_name"} + else: + expected_attr = {"url_name", "course", "org"} + + actual_attr = set(xml_obj.attrib.keys()) + + has_text = xml_obj.text is not None and len(xml_obj.text.strip()) > 0 + + return len(xml_obj) == 0 and actual_attr == expected_attr and not has_text + + +def serialize_field(value): + """ + Return a string version of the value (where value is the JSON-formatted, internally stored value). + + If the value is a string, then we simply return what was passed in. + Otherwise, we return json.dumps on the input value. + """ + if isinstance(value, str): + return value + elif isinstance(value, datetime.datetime): + if value.tzinfo is not None and value.utcoffset() is None: + return value.isoformat() + "Z" + return value.isoformat() + + return json.dumps(value, cls=EdxJSONEncoder) + + +def deserialize_field(field, value): + """ + Deserialize the string version to the value stored internally. + + Note that this is not the same as the value returned by from_json, as model types typically store + their value internally as JSON. By default, this method will return the result of calling json.loads + on the supplied value, unless json.loads throws a TypeError, or the type of the value returned by json.loads + is not supported for this class (from_json throws an Error). In either of those cases, this method returns + the input value. + """ + try: + deserialized = json.loads(value) + if deserialized is None: + return deserialized + try: + field.from_json(deserialized) + return deserialized + except (ValueError, TypeError): + # Support older serialized version, which was just a string, not result of json.dumps. + # If the deserialized version cannot be converted to the type (via from_json), + # just return the original value. For example, if a string value of '3.4' was + # stored for a String field (before we started storing the result of json.dumps), + # then it would be deserialized as 3.4, but 3.4 is not supported for a String + # field. Therefore field.from_json(3.4) will throw an Error, and we should + # actually return the original value of '3.4'. + return value + + except (ValueError, TypeError): + # Support older serialized version. + return value + + +def own_metadata(block): + """ + Return a JSON-friendly dictionary that contains only non-inherited field + keys, mapped to their serialized values + """ + return block.get_explicitly_set_fields_by_scope(Scope.settings) + + +def apply_pointer_attributes(node, block) -> None: + """Apply required pointer attributes to the export node for a block. + + Sets "url_name" for all blocks. For course blocks, also sets "org" and + "course" attributes. + """ + if not node.get("url_name"): + node.set("url_name", block.url_name) + + if getattr(block, "category", None) == "course": + # These attributes are required on course pointers + node.set("org", block.location.org) + node.set("course", block.location.course) + + +def format_filepath(category, name): + """Formats a path to an XML definition file.""" + return f"{category}/{name}.{filename_extension}" + + +def file_to_xml(file_object): + """ + Used when this module wants to parse a file object to xml + that will be converted to the definition. + + Returns an lxml Element + """ + return etree.parse(file_object, parser=EDX_XML_PARSER).getroot() + + +def load_file(filepath, fs, def_id): + """ + Open the specified file in fs, and call `file_to_xml` on it, + returning the lxml object. + + Add details and reraise on error. + """ + try: + with fs.open(filepath) as xml_file: + return file_to_xml(xml_file) + except Exception as err: + # Add info about where we are, but keep the traceback + raise Exception(f"Unable to load file contents at path {filepath} for item {def_id}: {err}") from err + + +def load_definition_xml(node, runtime, def_id): + """ + Loads definition_xml stored in a dedicated file + """ + url_name = node.get("url_name") + filepath = format_filepath(node.tag, name_to_pathname(url_name)) + definition_xml = load_file(filepath, runtime.resources_fs, def_id) + return definition_xml, filepath From c57e3b0fc7444df387c569fc79e90712937a5e11 Mon Sep 17 00:00:00 2001 From: "M. Tayyab Tahir Qureshi" Date: Tue, 28 Oct 2025 19:32:33 +0500 Subject: [PATCH 2/4] feat: add support for pointer tag OLX export/import format in HTML block --- xblocks_contrib/html/html.py | 174 +++-------------------------------- 1 file changed, 14 insertions(+), 160 deletions(-) diff --git a/xblocks_contrib/html/html.py b/xblocks_contrib/html/html.py index 39733114..90599d6f 100644 --- a/xblocks_contrib/html/html.py +++ b/xblocks_contrib/html/html.py @@ -4,7 +4,6 @@ """ import copy -import datetime import json import logging import os @@ -14,7 +13,6 @@ from html.parser import HTMLParser from django.conf import settings -from django.core.serializers.json import DjangoJSONEncoder from django.utils.translation import gettext_noop as _ from fs.errors import ResourceNotFound from lxml import etree @@ -27,6 +25,17 @@ from xblock.fields import Boolean, Dict, Scope, ScopeIds, String, UserScope from xblock.utils.resources import ResourceLoader +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__) resource_loader = ResourceLoader(__name__) @@ -37,27 +46,6 @@ EDX_XML_PARSER = XMLParser(dtd_validation=False, load_dtd=False, remove_blank_text=True, encoding="utf-8") -class EdxJSONEncoder(DjangoJSONEncoder): - """ - Custom JSONEncoder that handles ``Location`` and ``datetime.datetime`` objects. - Encodes ``Location`` as its URL string form, and ``datetime.datetime`` as an ISO 8601 string. - """ - - def default(self, o): - if isinstance(o, (CourseKey, UsageKey)): - return str(o) - elif isinstance(o, datetime.datetime): - if o.tzinfo is not None: - if o.utcoffset() is None: - return o.isoformat() + "Z" - else: - return o.isoformat() - else: - return o.isoformat() - else: - return super().default(o) - - class MLStripper(HTMLParser): "helper function for html_to_text below" @@ -164,95 +152,6 @@ def stringify_children(node): return "".join([part for part in parts if part]) -def name_to_pathname(name): - """ - Convert a location name for use in a path: replace ':' with '/'. - This allows users of the xml format to organize content into directories - """ - return name.replace(":", "/") - - -def is_pointer_tag(xml_obj): - """ - Check if xml_obj is a pointer tag: . - No children, one attribute named url_name, no text. - - Special case for course roots: the pointer is - - - xml_obj: an etree Element - - Returns a bool. - """ - if xml_obj.tag != "course": - expected_attr = {"url_name"} - else: - expected_attr = {"url_name", "course", "org"} - - actual_attr = set(xml_obj.attrib.keys()) - - has_text = xml_obj.text is not None and len(xml_obj.text.strip()) > 0 - - return len(xml_obj) == 0 and actual_attr == expected_attr and not has_text - - -def serialize_field(value): - """ - Return a string version of the value (where value is the JSON-formatted, internally stored value). - - If the value is a string, then we simply return what was passed in. - Otherwise, we return json.dumps on the input value. - """ - if isinstance(value, str): - return value - elif isinstance(value, datetime.datetime): - if value.tzinfo is not None and value.utcoffset() is None: - return value.isoformat() + "Z" - return value.isoformat() - - return json.dumps(value, cls=EdxJSONEncoder) - - -def deserialize_field(field, value): - """ - Deserialize the string version to the value stored internally. - - Note that this is not the same as the value returned by from_json, as model types typically store - their value internally as JSON. By default, this method will return the result of calling json.loads - on the supplied value, unless json.loads throws a TypeError, or the type of the value returned by json.loads - is not supported for this class (from_json throws an Error). In either of those cases, this method returns - the input value. - """ - try: - deserialized = json.loads(value) - if deserialized is None: - return deserialized - try: - field.from_json(deserialized) - return deserialized - except (ValueError, TypeError): - # Support older serialized version, which was just a string, not result of json.dumps. - # If the deserialized version cannot be converted to the type (via from_json), - # just return the original value. For example, if a string value of '3.4' was - # stored for a String field (before we started storing the result of json.dumps), - # then it would be deserialized as 3.4, but 3.4 is not supported for a String - # field. Therefore field.from_json(3.4) will throw an Error, and we should - # actually return the original value of '3.4'. - return value - - except (ValueError, TypeError): - # Support older serialized version. - return value - - -def own_metadata(block): - """ - Return a JSON-friendly dictionary that contains only non-inherited field - keys, mapped to their serialized values - """ - return block.get_explicitly_set_fields_by_scope(Scope.settings) - - @XBlock.needs("i18n") # We 'want' the user service, but we don't strictly 'need' it. # This makes our block more resilient. It won't crash in test environments @@ -600,31 +499,6 @@ def clean_metadata_from_xml(cls, xml_object, excluded_fields=()): ): del xml_object.attrib[field_name] - @classmethod - def file_to_xml(cls, file_object): - """ - Used when this module wants to parse a file object to xml - that will be converted to the definition. - - Returns an lxml Element - """ - return etree.parse(file_object, parser=EDX_XML_PARSER).getroot() # CHANGEE - - @classmethod - def load_file(cls, filepath, fs, def_id): - """ - Open the specified file in fs, and call cls.file_to_xml on it, - returning the lxml object. - - Add details and reraise on error. - """ - try: - with fs.open(filepath) as xml_file: - return cls.file_to_xml(xml_file) - except Exception as err: - # Add info about where we are, but keep the traceback - raise Exception(f"Unable to load file contents at path {filepath} for item {def_id}: {err}") from err - # NOTE: html descriptors are special. We do not want to parse and # export them ourselves, because that can break things (e.g. lxml # adds body tags when it exports, but they should just be html @@ -769,7 +643,7 @@ def parse_xml(cls, node, runtime, keys): if is_pointer_tag(node): # new style: # read the actual definition file--named using url_name.replace(':','/') - definition_xml, filepath = cls.load_definition_xml(node, runtime, keys.def_id) + 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 @@ -847,21 +721,6 @@ def parse_xml_new_runtime(cls, node, runtime, keys): cls._set_field_if_present(block, name, value, {}) return block - @classmethod - def load_definition_xml(cls, node, runtime, def_id): - """ - Loads definition_xml stored in a dedicated file - """ - url_name = node.get("url_name") - filepath = cls._format_filepath(node.tag, name_to_pathname(url_name)) - definition_xml = cls.load_file(filepath, runtime.resources_fs, def_id) - return definition_xml, filepath - - @classmethod - def _format_filepath(cls, category, name): - """Formats a path to an XML definition file.""" - return f"{category}/{name}.{cls.filename_extension}" - def export_to_file(self): """If this returns True, write the definition of this block to a separate file. @@ -907,7 +766,7 @@ def add_xml_to_node(self, node): if self.export_to_file(): url_path = name_to_pathname(self.url_name) - filepath = self._format_filepath( + filepath = format_filepath( self.category, self.location.run if self.category == "course" else url_path ) self.runtime.export_fs.makedirs(os.path.dirname(filepath), recreate=True) @@ -921,12 +780,7 @@ def add_xml_to_node(self, node): node.attrib.update(xml_object.attrib) node.extend(xml_object) - if not node.get("url_name"): - node.set("url_name", self.url_name) - - if self.category == "course": - node.set("org", self.location.org) - node.set("course", self.location.course) + apply_pointer_attributes(node, self) def definition_to_xml(self, resource_fs): """ From 610d236a63ac5c3968e5c30d4f56134dffb606d7 Mon Sep 17 00:00:00 2001 From: "M. Tayyab Tahir Qureshi" Date: Thu, 30 Oct 2025 15:49:36 +0500 Subject: [PATCH 3/4] chore: address PR changes --- xblocks_contrib/common/xml_utils.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/xblocks_contrib/common/xml_utils.py b/xblocks_contrib/common/xml_utils.py index 4565f75f..20499df4 100644 --- a/xblocks_contrib/common/xml_utils.py +++ b/xblocks_contrib/common/xml_utils.py @@ -1,6 +1,4 @@ -"""Shared utilities for pointer tag handling across XBlocks. - -This module centralizes detection, path computation, and attribute application +"""This module centralizes detection, path computation, and attribute application for OLX "pointer" tags used by XBlocks during import/export. Also provides helpers to load definition XML files referenced by pointer tags. @@ -133,10 +131,10 @@ def own_metadata(block): def apply_pointer_attributes(node, block) -> None: - """Apply required pointer attributes to the export node for a block. + """Apply required pointer attributes to the relevant node for a block. - Sets "url_name" for all blocks. For course blocks, also sets "org" and - "course" attributes. + Sets "url_name" for all blocks. For course blocks, additionally assigns + "org" and "course" attributes. """ if not node.get("url_name"): node.set("url_name", block.url_name) From 290d22d5b4507512683f20f0945cd91bee513ed3 Mon Sep 17 00:00:00 2001 From: "M. Tayyab Tahir Qureshi" Date: Wed, 5 Nov 2025 17:53:08 +0500 Subject: [PATCH 4/4] chore: add type annotations --- xblocks_contrib/common/xml_utils.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/xblocks_contrib/common/xml_utils.py b/xblocks_contrib/common/xml_utils.py index 20499df4..e7a69e46 100644 --- a/xblocks_contrib/common/xml_utils.py +++ b/xblocks_contrib/common/xml_utils.py @@ -8,11 +8,17 @@ import datetime import json +from typing import Any, TextIO from django.core.serializers.json import DjangoJSONEncoder +from fs.osfs import OSFS from lxml import etree +from lxml.etree import _Element as Element from opaque_keys.edx.keys import CourseKey, UsageKey -from xblock.fields import Scope +from opaque_keys.edx.locator import BlockUsageLocator +from xblock.core import XBlock +from xblock.fields import Field, Scope +from xblock.runtime import Runtime # Assume all XML files are persisted as utf-8. EDX_XML_PARSER = etree.XMLParser(dtd_validation=False, load_dtd=False, remove_blank_text=True, encoding="utf-8") @@ -41,7 +47,7 @@ def default(self, o): return super().default(o) -def name_to_pathname(name): +def name_to_pathname(name: str) -> str: """ Convert a location name for use in a path: replace ':' with '/'. This allows users of the xml format to organize content into directories @@ -49,7 +55,7 @@ def name_to_pathname(name): return name.replace(":", "/") -def is_pointer_tag(xml_obj): +def is_pointer_tag(xml_obj: Element) -> bool: """ Check if xml_obj is a pointer tag: . No children, one attribute named url_name, no text. @@ -73,7 +79,7 @@ def is_pointer_tag(xml_obj): return len(xml_obj) == 0 and actual_attr == expected_attr and not has_text -def serialize_field(value): +def serialize_field(value: Any) -> str: """ Return a string version of the value (where value is the JSON-formatted, internally stored value). @@ -90,7 +96,7 @@ def serialize_field(value): return json.dumps(value, cls=EdxJSONEncoder) -def deserialize_field(field, value): +def deserialize_field(field: Field, value: str) -> Any: """ Deserialize the string version to the value stored internally. @@ -122,7 +128,7 @@ def deserialize_field(field, value): return value -def own_metadata(block): +def own_metadata(block: XBlock) -> dict[str, Any]: """ Return a JSON-friendly dictionary that contains only non-inherited field keys, mapped to their serialized values @@ -130,7 +136,7 @@ def own_metadata(block): return block.get_explicitly_set_fields_by_scope(Scope.settings) -def apply_pointer_attributes(node, block) -> None: +def apply_pointer_attributes(node: Element, block: XBlock) -> None: """Apply required pointer attributes to the relevant node for a block. Sets "url_name" for all blocks. For course blocks, additionally assigns @@ -145,12 +151,12 @@ def apply_pointer_attributes(node, block) -> None: node.set("course", block.location.course) -def format_filepath(category, name): +def format_filepath(category: str, name: str) -> str: """Formats a path to an XML definition file.""" return f"{category}/{name}.{filename_extension}" -def file_to_xml(file_object): +def file_to_xml(file_object: TextIO) -> Element: """ Used when this module wants to parse a file object to xml that will be converted to the definition. @@ -160,7 +166,7 @@ def file_to_xml(file_object): return etree.parse(file_object, parser=EDX_XML_PARSER).getroot() -def load_file(filepath, fs, def_id): +def load_file(filepath: str, fs: OSFS, def_id: BlockUsageLocator) -> Element: """ Open the specified file in fs, and call `file_to_xml` on it, returning the lxml object. @@ -175,7 +181,7 @@ def load_file(filepath, fs, def_id): raise Exception(f"Unable to load file contents at path {filepath} for item {def_id}: {err}") from err -def load_definition_xml(node, runtime, def_id): +def load_definition_xml(node: Element, runtime: Runtime, def_id: BlockUsageLocator) -> tuple[Element, str]: """ Loads definition_xml stored in a dedicated file """