diff --git a/xblocks_contrib/annotatable/annotatable.py b/xblocks_contrib/annotatable/annotatable.py index 332966a9..0c2c8755 100644 --- a/xblocks_contrib/annotatable/annotatable.py +++ b/xblocks_contrib/annotatable/annotatable.py @@ -12,18 +12,30 @@ import markupsafe from django.utils.translation import gettext_noop as _ from lxml import etree +from opaque_keys.edx.keys import UsageKey from web_fragments.fragment import Fragment from xblock.core import XBlock from xblock.fields import Scope, String, XMLString from xblock.utils.resources import ResourceLoader +from xblocks_contrib.common.xml_utils import LegacyXmlMixin + log = logging.getLogger(__name__) resource_loader = ResourceLoader(__name__) +class SerializationError(Exception): + """ + Thrown when a module cannot be exported to XML + """ + def __init__(self, location, msg): + super().__init__(msg) + self.location = location + + @XBlock.needs("i18n") -class AnnotatableBlock(XBlock): +class AnnotatableBlock(LegacyXmlMixin, XBlock): """ AnnotatableXBlock allows instructors to create annotated content that students can view interactively. Annotations can be styled and customized, with internationalization support for multilingual environments. @@ -84,6 +96,18 @@ class AnnotatableBlock(XBlock): # List of supported highlight colors for annotations HIGHLIGHT_COLORS = ["yellow", "orange", "purple", "blue", "green"] + @property + def location(self): + return self.scope_ids.usage_id + + @location.setter + def location(self, value): + assert isinstance(value, UsageKey) + self.scope_ids = self.scope_ids._replace( + def_id=value, # Note: assigning a UsageKey as def_id is OK in old mongo / import system but wrong in split + usage_id=value, + ) + def _get_annotation_class_attr(self, index, el): # pylint: disable=unused-argument """Returns a dict with the CSS class attribute to set on the annotation and an XML key to delete from the element. @@ -234,3 +258,45 @@ def workbench_scenarios(): """, ), ] + + @classmethod + def definition_from_xml(cls, xml_object, system): + if len(xml_object) == 0 and len(list(xml_object.items())) == 0: + return {'data': ''}, [] + return {'data': etree.tostring(xml_object, pretty_print=True, encoding='unicode')}, [] + + def definition_to_xml(self, resource_fs): + """ + Return an Element if we've kept the import OLX, or None otherwise. + """ + # If there's no self.data, it means that an XBlock/XModule originally + # existed for this data at the time of import/editing, but was later + # uninstalled. RawDescriptor therefore never got to preserve the + # original OLX that came in, and we have no idea how it should be + # serialized for export. It's possible that we could do some smarter + # fallback here and attempt to extract the data, but it's reasonable + # and simpler to just skip this node altogether. + if not self.data: + log.warning( + "Could not serialize %s: No XBlock installed for '%s' tag.", + self.location, + self.location.block_type, + ) + return None + + # Normal case: Just echo back the original OLX we saved. + try: + return etree.fromstring(self.data) + except etree.XMLSyntaxError as err: + # Can't recover here, so just add some info and + # re-raise + lines = self.data.split('\n') + line, offset = err.position # lint-amnesty, pylint: disable=unpacking-non-sequence + msg = ( + "Unable to create xml for block {loc}. " + "Context: '{context}'" + ).format( + context=lines[line - 1][offset - 40:offset + 40], + loc=self.location, + ) + raise SerializationError(self.location, msg) from err diff --git a/xblocks_contrib/common/xml_utils.py b/xblocks_contrib/common/xml_utils.py index 1b838e9b..4c86abb3 100644 --- a/xblocks_contrib/common/xml_utils.py +++ b/xblocks_contrib/common/xml_utils.py @@ -132,7 +132,7 @@ def own_metadata(block: XBlock) -> dict[str, Any]: keys, mapped to their serialized values """ result = {} - for field in block.fields.values(): # lint-amnesty, pylint: disable=no-member + for field in block.fields.values(): if field.scope == Scope.settings and field.is_set_on(block): try: result[field.name] = field.read_json(block) diff --git a/xblocks_contrib/html/html.py b/xblocks_contrib/html/html.py index 2896f421..ab1d9fa3 100644 --- a/xblocks_contrib/html/html.py +++ b/xblocks_contrib/html/html.py @@ -143,7 +143,7 @@ def stringify_children(node): # This makes our block more resilient. It won't crash in test environments # where the user service might not be available. @XBlock.wants("user") -class HtmlBlock(LegacyXmlMixin, XBlock): # pylint: disable=abstract-method +class HtmlBlock(LegacyXmlMixin, XBlock): """ The HTML XBlock. """ @@ -268,7 +268,7 @@ def get_html(self): data = data.replace("%%COURSE_ID%%", str(self.scope_ids.usage_id.context_key)) return data - def studio_view(self, context=None): # pylint: disable=unused-argument + def studio_view(self, context=None): """Return a fragment that contains the html for the studio view.""" # Only the ReactJS editor is supported for this block. # See https://github.com/openedx/frontend-app-authoring/tree/master/src/editors/containers/TextEditor diff --git a/xblocks_contrib/lti/lti.py b/xblocks_contrib/lti/lti.py index ca80b3eb..0fbe3f9b 100644 --- a/xblocks_contrib/lti/lti.py +++ b/xblocks_contrib/lti/lti.py @@ -84,6 +84,8 @@ from xblockutils.resources import ResourceLoader from xblockutils.studio_editable import StudioEditableXBlockMixin +from xblocks_contrib.common.xml_utils import LegacyXmlMixin + from .lti_2_util import LTI20BlockMixin, LTIError # The anonymous user ID for the user in the course. @@ -122,6 +124,8 @@ class LTIFields: https://github.com/idan/oauthlib/blob/master/oauthlib/oauth1/rfc5849/signature.py#L136 """ + data = String(default='', scope=Scope.content) + display_name = String( display_name=_("Display Name"), help=_( @@ -290,6 +294,7 @@ class LTIFields: class LTIBlock( LTIFields, LTI20BlockMixin, + LegacyXmlMixin, StudioEditableXBlockMixin, XBlock, ): @@ -1011,3 +1016,14 @@ def is_past_due(self): else: close_date = due_date return close_date is not None and datetime.datetime.now(UTC) > close_date + + @classmethod + def definition_from_xml(cls, xml_object, system): + if len(xml_object) == 0 and len(list(xml_object.items())) == 0: + return {'data': ''}, [] + return {'data': etree.tostring(xml_object, pretty_print=True, encoding='unicode')}, [] + + def definition_to_xml(self, resource_fs): + if self.data: + return etree.fromstring(self.data) + return etree.Element(self.category) diff --git a/xblocks_contrib/poll/poll.py b/xblocks_contrib/poll/poll.py index ef28e0ab..76e7d46e 100644 --- a/xblocks_contrib/poll/poll.py +++ b/xblocks_contrib/poll/poll.py @@ -6,8 +6,6 @@ If student have answered - Question with statistics for each answers. """ -import copy -import datetime import json import logging from collections import OrderedDict @@ -18,98 +16,16 @@ from lxml import etree from opaque_keys.edx.keys import UsageKey from web_fragments.fragment import Fragment -from xblock.core import XML_NAMESPACES, XBlock -from xblock.fields import Boolean, Dict, List, Scope, ScopeIds, String +from xblock.core import XBlock +from xblock.fields import Boolean, Dict, List, Scope, String from xblock.utils.resources import ResourceLoader +from xblocks_contrib.common.xml_utils import LegacyXmlMixin + Text = markupsafe.escape resource_loader = ResourceLoader(__name__) log = logging.getLogger(__name__) -# 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") - - -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) - - -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 HTML(html): # pylint: disable=invalid-name """ @@ -165,7 +81,7 @@ def stringify_children(node): @XBlock.needs("i18n") -class PollBlock(XBlock): +class PollBlock(LegacyXmlMixin, XBlock): """Poll Block""" # Indicates that this XBlock has been extracted from edx-platform. @@ -187,47 +103,8 @@ class PollBlock(XBlock): question = String(help=_("Poll question"), scope=Scope.content, default="") - # This is a categories to fields map that contains the block category specific fields which should not be - # cleaned and/or override while adding xml to node. - metadata_to_not_to_clean = { - # A category `video` having `sub` and `transcripts` fields - # which should not be cleaned/override in an xml object. - "video": ("sub", "transcripts") - } - metadata_to_export_to_policy = ("discussion_topics",) js_module_name = "poll" - # Extension to append to filename paths - filename_extension = "xml" - - xml_attributes = Dict( - help="Map of unhandled xml attributes, used only for storage between import and export", - default={}, - scope=Scope.settings, - ) - - 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", - ) - _tag_name = "poll_question" _child_tag_name = "answer" @@ -426,23 +303,23 @@ def get_explicitly_set_fields_by_scope(self, scope=Scope.content): raise TypeError(exception_message) # pylint: disable=raise-missing-from return result - @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 definition_from_xml(cls, xml_object, system): # pylint: disable=unused-argument + def definition_from_xml(cls, xml_object, system): """ - Pull out the data into dictionary. + Pull out the data into a dictionary. + + Args: + xml_object: XML from file. + system: `system` object. + + Returns: + tuple: A tuple ``(definition, children)``. + + definition (dict): + A dictionary containing: + + - ``answers`` (list): List of answers. + - ``question`` (str): Question string. """ # Check for presense of required tags in xml. if len(xml_object.xpath(cls._child_tag_name)) == 0: @@ -463,268 +340,7 @@ def definition_from_xml(cls, xml_object, system): # pylint: disable=unused-argu children = [] return (definition, children) - @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 getattr(cls, "fields", {}).items(): # pylint: disable=no-member - 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] - - @staticmethod - 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() - - @classmethod - def load_file(cls, filepath, fs, def_id): # pylint: disable=invalid-name - """ - 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 - - @classmethod - def load_definition(cls, xml_object, system, def_id, id_generator): - """ - Load a block from the specified xml_object. - """ - - # VS[compat] - # The filename attr should have been removed once all 2012-fall courses imported into the CMS and "inline" OLX - # format deprecated. This never happened, and `filename` is still used, so we have too keep both formats. - filename = xml_object.get("filename") - if filename is None: - definition_xml = copy.deepcopy(xml_object) - filepath = "" - aside_children = [] - else: - filepath = cls._format_filepath(xml_object.tag, filename) - - definition_xml = cls.load_file(filepath, system.resources_fs, def_id) - usage_id = id_generator.create_usage(def_id) - aside_children = system.parse_asides(definition_xml, def_id, usage_id, id_generator) - - # Add the attributes from the pointer node - definition_xml.attrib.update(xml_object.attrib) - - definition_metadata = cls._get_metadata_from_xml(definition_xml) - cls.clean_metadata_from_xml(definition_xml) - definition, children = cls.definition_from_xml(definition_xml, system) - if definition_metadata: - definition["definition_metadata"] = definition_metadata - definition["filename"] = [filepath, filename] - - if aside_children: - definition["aside_children"] = aside_children - - 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 getattr(cls, "fields", {}): # pylint: disable=unsupported-membership-test - metadata["xml_attributes"][attr] = val - else: - metadata[attr] = deserialize_field(cls.fields[attr], val) # pylint: disable=unsubscriptable-object - return metadata - - @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: # pylint: disable=unsupported-membership-test - # 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 - - @classmethod - def parse_xml(cls, node, runtime, keys): - """ - Use `node` to construct a new block. - Returns (XBlock): The newly parsed XBlock - """ - - if keys is None: - 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 = [] - - if is_pointer_tag(node): - definition_xml, filepath = cls.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 - - definition, children = cls.load_definition(definition_xml, runtime, keys.def_id, runtime.id_generator) - - if is_pointer_tag(node): - definition["filename"] = [filepath, filepath] - - metadata = cls.load_metadata(definition_xml) - - dmdata = definition.get("definition_metadata", "") - if dmdata: - metadata["definition_metadata_raw"] = dmdata - try: - metadata.update(json.loads(dmdata)) - except Exception as err: # pylint: disable=broad-exception-caught - 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) - - field_data = {**metadata, **definition, "children": children} - field_data["xml_attributes"]["filename"] = definition.get("filename", ["", None]) - - if "filename" in field_data: - del field_data["filename"] - - # The "normal" / new way to set field data: - xblock = runtime.construct_xblock_from_class(cls, keys) - for key, value_jsonish in field_data.items(): - if key in cls.fields: # pylint: disable=unsupported-membership-test - setattr(xblock, key, cls.fields[key].from_json(value_jsonish)) # pylint: disable=unsubscriptable-object - elif key == "children": - xblock.children = value_jsonish - else: - log.warning( - "Imported %s XBlock does not have field %s found in XML.", - xblock.scope_ids.block_type, - key, - ) - - if aside_children: - asides_tags = [x.tag for x in aside_children] - asides = runtime.get_asides(xblock) - for asd in asides: - if asd.scope_ids.block_type in asides_tags: - xblock.add_aside(asd) - return xblock - - @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): - return f"{category}/{name}.{cls.filename_extension}" - - def add_xml_to_node(self, node): - """ - For exporting, set data on `node` from ourselves. - """ - xml_object = self.definition_to_xml() - - # If xml_object is None, we don't know how to serialize this node, but - # we shouldn't crash out the whole export for it. - if xml_object is None: - return - - for aside in self.runtime.get_asides(self): - if aside.needs_serialization(): - aside_node = etree.Element("unknown_root", nsmap=XML_NAMESPACES) - aside.add_xml_to_node(aside_node) - xml_object.append(aside_node) - - not_to_clean_fields = self.metadata_to_not_to_clean.get(self.category, ()) - self.clean_metadata_from_xml(xml_object, excluded_fields=not_to_clean_fields) - - # Set the tag on both nodes so we get the file path right. - xml_object.tag = self.category - node.tag = self.category - - # Add the non-inherited metadata - - for attr in sorted(self.get_explicitly_set_fields_by_scope(Scope.settings)): - # don't want e.g. data_dir - if ( - attr not in self.metadata_to_strip - and attr not in self.metadata_to_export_to_policy - and attr not in not_to_clean_fields - ): - # pylint: disable=unsubscriptable-object - val = serialize_field(self.fields[attr].to_json(getattr(self, attr))) - try: - xml_object.set(attr, val) - except Exception: # pylint: disable=broad-except - logging.exception( - ( - "Failed to serialize metadata attribute %s with value %s in module %s. " - "This could mean data loss!!!" - ), - attr, - val, - self.url_name, - ) - - for key, value in self.xml_attributes.items(): - if key not in self.metadata_to_strip: - xml_object.set(key, serialize_field(value)) - - node.clear() - node.tag = xml_object.tag - node.text = xml_object.text - node.tail = xml_object.tail - node.attrib.update(xml_object.attrib) - node.extend(xml_object) - - # Do not override an existing value for the course. - if not node.get("url_name"): - node.set("url_name", self.url_name) - - # Special case for course pointers: - if self.category == "course": - # add org and course attributes on the pointer tag - node.set("org", self.location.org) - node.set("course", self.location.course) - - def definition_to_xml(self, resource_fs=None): # pylint: disable=unused-argument + def definition_to_xml(self, resource_fs=None): """Return an xml element representing to this definition.""" poll_str = HTML("<{tag_name}>{text}").format(tag_name=self._tag_name, text=self.question) diff --git a/xblocks_contrib/word_cloud/word_cloud.py b/xblocks_contrib/word_cloud/word_cloud.py index 49e1f068..74ae656e 100644 --- a/xblocks_contrib/word_cloud/word_cloud.py +++ b/xblocks_contrib/word_cloud/word_cloud.py @@ -8,12 +8,15 @@ import uuid from django.utils.translation import gettext_noop as _ +from lxml import etree from web_fragments.fragment import Fragment from xblock.core import XBlock from xblock.fields import Boolean, Dict, Integer, List, Scope, String from xblock.utils.resources import ResourceLoader from xblock.utils.studio_editable import StudioEditableXBlockMixin +from xblocks_contrib.common.xml_utils import LegacyXmlMixin + resource_loader = ResourceLoader(__name__) @@ -28,7 +31,7 @@ def pretty_bool(value): @XBlock.needs("i18n") -class WordCloudBlock(StudioEditableXBlockMixin, XBlock): +class WordCloudBlock(StudioEditableXBlockMixin, LegacyXmlMixin, XBlock): """ Word Cloud XBlock. """ @@ -308,3 +311,14 @@ def index_dictionary(self): xblock_body["content_type"] = "Word Cloud" return xblock_body + + @classmethod + def definition_from_xml(cls, xml_object, system): + if len(xml_object) == 0 and len(list(xml_object.items())) == 0: + return {'data': ''}, [] + return {'data': etree.tostring(xml_object, pretty_print=True, encoding='unicode')}, [] + + def definition_to_xml(self, resource_fs): + if self.data: + return etree.fromstring(self.data) + return etree.Element(self.category)