feat: more error handling

`ExpectedAncestorError`, `InvalidContentError`, and `UnknownAttributeError`

questionpy_sdk/webserver/question_ui/__init__.py

@@ -15,10 +15,13 @@
 
 from questionpy_sdk.webserver.question_ui.errors import (
     ConversionError,
+    ExpectedAncestorError,
     InvalidAttributeValueError,
     InvalidCleanOptionError,
+    InvalidContentError,
     PlaceholderReferenceError,
     RenderErrorCollection,
+    UnknownAttributeError,
     UnknownElementError,
     XMLSyntaxError,
 )
@@ -598,9 +601,9 @@ def collect(self) -> RenderErrorCollection:
         self._validate_placeholders()
         self._validate_feedback()
         self._validate_if_role()
-        self._validate_shuffle_contents()
+        self._validate_shuffle_contents_and_shuffled_index()
         self._validate_format_floats()
-        self._look_for_unknown_qpy_elements()
+        self._look_for_unknown_qpy_elements_and_attributes()
 
         return self.errors
 
@@ -655,11 +658,8 @@ def _validate_if_role(self) -> None:
                     )
                     self.errors.insert(error)
 
-    def _validate_shuffle_contents(self) -> None:
+    def _validate_shuffle_contents_and_shuffled_index(self) -> None:
         """Validates elements marked with `qpy:shuffle-contents`."""
-        # TODO: - check if shuffle-contents has children
-        #       - check if shuffled-index elements have a parent element marked with qpy:shuffle-contests
-
         for element in _assert_element_list(self._xpath("//*[@qpy:shuffle-contents]")):
             child_elements = [child for child in element if isinstance(child, etree._Element)]
             for child in child_elements:
@@ -668,7 +668,24 @@ def _validate_shuffle_contents(self) -> None:
                 ):
                     format_style = index_element.get("format", "123")
                     if format_style not in {"123", "abc", "ABC", "iii", "III"}:
-                        self.errors.insert(InvalidAttributeValueError(index_element, "format", format_style))
+                        attribute_error = InvalidAttributeValueError(
+                            element=index_element, attribute="format", value=format_style
+                        )
+                        self.errors.insert(attribute_error)
+
+        # Gather every qpy:shuffle-contents with direct text nodes or processing instructions.
+        for element in _assert_element_list(
+            self._xpath("//*[@qpy:shuffle-contents and (text()[normalize-space()] != '' or processing-instruction())]")
+        ):
+            placement_error = InvalidContentError(element=element, attribute="qpy:shuffle-contents")
+            self.errors.insert(placement_error)
+
+        # Gather every qpy:shuffled-index without qpy:shuffle-contents ancestor.
+        for element in _assert_element_list(
+            self._xpath("//qpy:shuffled-index[not(ancestor::*[@qpy:shuffle-contents])]")
+        ):
+            ancestor_error = ExpectedAncestorError(element=element, expected_ancestor_attribute="qpy:shuffle-contents")
+            self.errors.insert(ancestor_error)
 
     def _validate_format_floats(self) -> None:
         """Validates the `qpy:format-float` element."""
@@ -708,13 +725,28 @@ def _validate_format_floats(self) -> None:
                 )
                 self.errors.insert(thousands_sep_error)
 
-    def _look_for_unknown_qpy_elements(self) -> None:
-        """Checks if there are any unknown qpy-elements.
-
-        TODO: also look for unknown qpy-attributes
-        """
+    def _look_for_unknown_qpy_elements_and_attributes(self) -> None:
+        """Checks if there are any unknown qpy-elements or -attributes."""
+        # Gather unknown elements.
         known_elements = ["shuffled-index", "format-float"]
-        xpath = " and ".join([f"name() != 'qpy:{element}'" for element in known_elements])
-        for element in _assert_element_list(self._xpath(f"//*[starts-with(name(), 'qpy:') and {xpath}]")):
-            error = UnknownElementError(element=element)
-            self.errors.insert(error)
+        xpath_elements = " and ".join(f"local-name() != '{element}'" for element in known_elements)
+        xpath_query = f"//qpy:*[{xpath_elements}]"
+
+        for element in _assert_element_list(self._xpath(xpath_query)):
+            unknown_element_error = UnknownElementError(element=element)
+            self.errors.insert(unknown_element_error)
+
+        # Gather unknown attributes.
+        known_attrs = ["feedback", "if-role", "shuffle-contents", "correct-response"]
+        xpath_attrs = " and ".join(f"local-name() != '{attr}'" for attr in known_attrs)
+        xpath_query = f"//*[@qpy:*[{xpath_attrs}]]"
+
+        for element in _assert_element_list(self._xpath(xpath_query)):
+            unknown_attributes: list[str] = [
+                attr.replace(f"{{{_QPY_NAMESPACE}}}", "qpy:")
+                for attr in map(str, element.attrib)
+                if attr.startswith(f"{{{_QPY_NAMESPACE}}}") and attr.split("}")[1] not in known_attrs
+            ]
+            if unknown_attributes:
+                unknown_attribute_error = UnknownAttributeError(element=element, attributes=unknown_attributes)
+                self.errors.insert(unknown_attribute_error)

questionpy_sdk/webserver/question_ui/errors.py

@@ -180,6 +180,31 @@ def __init__(self, element: etree._Element, option: str, expected: Collection[st
         )
 
 
+@dataclass(frozen=True)
+class InvalidContentError(RenderElementError):
+    """Invalid content placement."""
+
+    def __init__(self, element: etree._Element, attribute: str):
+        super().__init__(
+            element=element,
+            template="Avoid placing text or processing instructions directly inside {element} with the {attribute} "
+            "attribute. Wrap the content in an element instead.",
+            template_kwargs={"attribute": attribute},
+        )
+
+
+@dataclass(frozen=True)
+class ExpectedAncestorError(RenderElementError):
+    """Invalid element placement."""
+
+    def __init__(self, element: etree._Element, expected_ancestor_attribute: str):
+        super().__init__(
+            element=element,
+            template="{element} must be placed inside an element with the {expected_ancestor_attribute} attribute.",
+            template_kwargs={"expected_ancestor_attribute": expected_ancestor_attribute},
+        )
+
+
 @dataclass(frozen=True)
 class UnknownElementError(RenderElementError):
     """Unknown element with qpy-namespace."""
@@ -191,6 +216,19 @@ def __init__(self, element: etree._Element):
         )
 
 
+@dataclass(frozen=True)
+class UnknownAttributeError(RenderElementError):
+    """Unknown attribute with qpy-namespace."""
+
+    def __init__(self, element: etree._Element, attributes: Collection[str]):
+        s = "" if len(attributes) == 1 else "s"
+        super().__init__(
+            element=element,
+            template=f"Unknown attribute{s} {{attributes}} on element {{element}}.",
+            template_kwargs={"attributes": attributes},
+        )
+
+
 @dataclass(frozen=True)
 class XMLSyntaxError(RenderError):
     """Syntax error while parsing the XML."""
@@ -247,5 +285,5 @@ def log_render_errors(render_errors: RenderErrorCollections) -> None:
             line = f"Line {error.line}: " if error.line else ""
             errors_string += f"\n\t- {line}{error.type} - {error.message}"
         error_count = len(errors)
-        s = "s" if error_count > 1 else ""
+        s = "" if error_count == 1 else "s"
         _log.warning(f"{error_count} error{s} occurred while rendering {section}:{errors_string}")

tests/questionpy_sdk/webserver/test_data/faulty.xhtml

@@ -8,8 +8,11 @@
             Invalid shuffle format.
             <qpy:shuffled-index format="invalid"/>. A
         </label>
+        Invalid text placement.
     </fieldset>
     <div>Missing placeholder.<?p missing invalid-cleaning-option?></div>
     <div>Empty placeholder.<?p      ?></div>
+    <qpy:shuffled-index/> <!-- Not inside a qpy:shuffle-contents element. -->
+    <div qpy:unknown-attribute="">Unknown attribute.</div>
     <span no-attribute-value>Missing attribute value.</span>
 </div>

tests/questionpy_sdk/webserver/test_question_ui.py

@@ -7,20 +7,23 @@
 import pytest
 
 from questionpy_sdk.webserver.question_ui import (
-    InvalidAttributeValueError,
     QuestionDisplayOptions,
     QuestionDisplayRole,
     QuestionFormulationUIRenderer,
     QuestionMetadata,
     QuestionUIRenderer,
-    XMLSyntaxError,
 )
 from questionpy_sdk.webserver.question_ui.errors import (
     ConversionError,
+    ExpectedAncestorError,
+    InvalidAttributeValueError,
     InvalidCleanOptionError,
+    InvalidContentError,
     PlaceholderReferenceError,
     RenderError,
+    UnknownAttributeError,
     UnknownElementError,
+    XMLSyntaxError,
 )
 from tests.questionpy_sdk.webserver.conftest import assert_html_is_equal
 
@@ -354,7 +357,7 @@ def test_clean_up(renderer: QuestionUIRenderer) -> None:
         </div>
     """
     html, errors = renderer.render()
-    assert len(errors) == 0
+    assert len(errors) == 1  # Undefined attribute.
     assert_html_is_equal(html, expected)
 
 
@@ -381,27 +384,34 @@ def test_errors_should_be_collected(renderer: QuestionUIRenderer) -> None:
     expected = """
         <div>
             <span>&lt;qpy:format-float xmlns:qpy="http://questionpy.org/ns/question" xmlns="http://www.w3.org/1999/xhtml" thousands-separator="maybe" precision="invalid"&gt;Unknown value.&lt;/qpy:format-float&gt;</span>
-            <fieldset><label>Invalid shuffle format.<span>1</span>. A</label></fieldset>
+            <fieldset>
+                <label>Invalid shuffle format.<span>1</span>. A</label>
+                Invalid text placement.
+            </fieldset>
             <div>Missing placeholder.</div>
             <div>Empty placeholder.</div>
+            <div>Unknown attribute.</div>
             <span>Missing attribute value.</span>
         </div>
     """  # noqa: E501
     html, errors = renderer.render()
 
     expected_errors: list[tuple[type[RenderError], int]] = [
         # Even though the syntax error occurs after all the other errors, it should be listed first.
-        (XMLSyntaxError, 14),
+        (XMLSyntaxError, 17),
         (InvalidAttributeValueError, 2),
         (UnknownElementError, 3),
         (InvalidAttributeValueError, 4),
         (ConversionError, 5),
         (ConversionError, 5),
         (InvalidAttributeValueError, 5),
+        (InvalidContentError, 6),
         (InvalidAttributeValueError, 9),
-        (InvalidCleanOptionError, 12),
-        (PlaceholderReferenceError, 12),
+        (InvalidCleanOptionError, 13),
         (PlaceholderReferenceError, 13),
+        (PlaceholderReferenceError, 14),
+        (ExpectedAncestorError, 15),
+        (UnknownAttributeError, 16),
     ]
 
     assert len(errors) == len(expected_errors)