Fix #26: Missing checking <ref/> in XML config (#41)

In former times, the reference checking was done with the
`global-check-ref-list.xsl` stylesheet. Although this works, it
may be easier and can be customized further to implement it
in pure Python code.

This change allows to check the stitchfile against any <ref/>
which point to a non-existing target.

TODO: Make it more async?

Author: tomschr

changelog.d/41.bugfix.rst

@@ -0,0 +1 @@
+Fix #26 and add missing checks for references in stitchfile

src/docbuild/cli/cmd_validate/process.py

@@ -2,6 +2,7 @@
 
 import asyncio
 from collections.abc import Iterator
+from datetime import date
 import logging
 from pathlib import Path
 import tempfile
@@ -292,7 +293,15 @@ async def process(
     if successful_files_paths:
         try:
             log.info('Performing stitch-file validation...')
-            await create_stitchfile(successful_files_paths)
+            cachedir = paths.get('base_server_cache_dir', None)
+            tree = await create_stitchfile(successful_files_paths)
+            if cachedir is not None:
+                stitchfile = (
+                    Path(cachedir) / f'stitchfile-{date.today().isoformat()}.xml'
+                )
+                stitchfile.write_text(etree.tostring(tree, encoding='unicode'))
+                log.debug("Wrote stitchfile to %s", str(stitchfile))
+
             log.info('Stitch-file validation successful.')
 
         except ValueError as e:
@@ -309,7 +318,7 @@ async def process(
     failed_part = f'[red]{failed_files} file(s)[/red]'
     summary_msg = f'{successful_part} successfully validated, {failed_part} failed.'
 
-    if context.verbose > 0:
+    if context.verbose > 0:  # pragma: no cover
         console_out.print(f'Result: {summary_msg}')
 
     final_success = (failed_files == 0) and stitch_success

src/docbuild/config/xml/data/global-check-ref-list.xsl

@@ -0,0 +1,188 @@
+"""Check <ref/> links on a stitched XML configuration file."""
+
+from lxml import etree
+
+
+def check_ref_to_subdeliverable(
+    ref: etree._Element,
+    attrs: dict[str, str],
+) -> str | None:
+    """Check reference to a subdeliverable."""
+    product = attrs.get('product')
+    docset = attrs.get('docset')
+    dc = attrs.get('dc')
+    subdeliverable = attrs.get('subdeliverable')
+    # Build the XPath
+    xpath = (  # Use // to be more robust against nesting
+        f'//product[@productid = {product!r}]/docset[@setid = {docset!r}]'
+        f"/builddocs/language[@default = 'true' or @default = 1]"
+        f"/deliverable[dc = '{dc}'][subdeliverable = '{subdeliverable}']"
+    )
+
+    if not ref.getroottree().xpath(xpath):
+        origin = ref.xpath(
+            'concat(ancestor::product/@productid, "/",  ancestor::docset/@setid)'
+        )
+        return (
+            f'Failed reference from {origin!r} to '
+            f'{product}/{docset}:{dc}#{subdeliverable}: '
+            'Referenced subdeliverable does not exist.'
+        )
+
+
+def check_ref_to_deliverable(ref: etree._Element, attrs: dict[str, str]) -> str | None:
+    """Check reference to a deliverable."""
+    product = attrs.get('product')
+    docset = attrs.get('docset')
+    dc = attrs.get('dc')
+
+    # Build the XPath
+    base_xpath = (  # Use // to be more robust against nesting
+        f'//product[@productid = {product!r}]/docset[@setid = {docset!r}]'
+        f"/builddocs/language[@default = 'true' or @default = 1]"
+        f"/deliverable[dc = '{dc}']"
+    )
+    xpath_has_sub = f'{base_xpath}[subdeliverable]'
+    xpath_hasnot_sub = f'{base_xpath}[not(subdeliverable)]'
+    tree = ref.getroottree()
+
+    origin = ref.xpath(
+        'concat(ancestor::product/@productid, "/",  ancestor::docset/@setid)'
+    )
+    # A reference to a deliverable is only valid if it exists and
+    # has no subdeliverables.
+    if tree.xpath(xpath_hasnot_sub):
+        return None  # Valid reference found
+
+    # If we are here, it's an invalid reference. We need to find out why.
+    if tree.xpath(xpath_has_sub):
+        return (
+            f'Failed reference from {origin!r} to {product}/{docset}:{dc}: '
+            'Referenced deliverable has subdeliverables, '
+            'you must choose a subdeliverable in your reference.'
+        )
+    else:
+        return (
+            f'Failed reference from {origin!r} to {product}/{docset}:{dc}: '
+            'Referenced deliverable does not exist.'
+        )
+
+
+def check_ref_to_link(ref: etree._Element, attrs: dict[str, str]) -> str | None:
+    """Check ref to an external link."""
+    product = attrs.get('product')
+    docset = attrs.get('docset')
+    link = attrs.get('link')
+
+    xpath = (  # Use // to be more robust against nesting
+        f'//product[@productid = {product!r}]/docset[@setid = {docset!r}]'
+        f"/builddocs/language[@default = 'true' or @default = 1]"
+        f"/external/link[@linkid = '{link}']"
+    )
+
+    tree = ref.getroottree()
+    if not tree.xpath(xpath):
+        origin = ref.xpath(
+            'concat(ancestor::product/@productid, "/",  ancestor::docset/@setid)'
+        )
+        return (
+            f'Failed reference from {origin!r} to {product}/{docset}@{link}: '
+            'Referenced external link does not exist.'
+        )
+
+
+def check_ref_to_docset(ref: etree._Element, attrs: dict[str, str]) -> str | None:
+    """Check reference to a docset."""
+    product = attrs.get('product')
+    docset = attrs.get('docset')
+    xpath = (  # Use // to be more robust against nesting
+        f'//product[@productid = {product!r}]/docset[@setid = {docset!r}]'
+    )
+
+    tree = ref.getroottree()
+    if not tree.xpath(xpath):
+        origin = ref.xpath(
+            'concat(ancestor::product/@productid, "/",  ancestor::docset/@setid)'
+        )
+        return (
+            f'Failed reference from {origin!r} to {product}/{docset}: '
+            'Referenced docset does not exist.'
+        )
+
+
+def check_ref_to_product(ref: etree._Element, attrs: dict[str, str]) -> str | None:
+    """Check reference to a product."""
+    product = attrs.get('product')
+
+    xpath = f'//product[@productid = {product!r}]'  # Use // to be more robust
+    tree = ref.getroottree()
+    if not tree.xpath(xpath):
+        origin = ref.xpath(
+            'concat(ancestor::product/@productid, "/",  ancestor::docset/@setid)'
+        )
+        return (
+            f'Failed reference from {origin!r} to {product}: '
+            'Referenced product does not exist.'
+        )
+
+
+def check_stitched_references(tree: etree._ElementTree) -> list[str]:
+    """Check <ref> elements for broken references.
+
+    This function is a Python equivalent of the
+    ``global-check-ref-list.xsl`` stylesheet.
+
+    :param tree: The tree of the stitched XML file.
+    :returns: A list of error messages for any broken references found.
+    """
+    errors: list[str] = []
+
+    # TODO: Make it concurrent using Python's `concurrent.futures` module.
+    #       Consider using ThreadPoolExecutor or ProcessPoolExecutor to parallelize
+    #       the processing of <ref> elements. Ensure thread safety when appending
+    #       to the `errors` list, and handle exceptions raised during concurrent
+    #       execution to avoid losing error messages.
+    for ref in tree.iter('ref'):
+        attrs = ref.attrib
+        product = attrs.get('product')
+        docset = attrs.get('docset')
+        dc = attrs.get('dc')
+        subdeliverable = attrs.get('subdeliverable')
+        link = attrs.get('link')
+
+        result = None
+        # Case 1: Reference to a subdeliverable
+        if subdeliverable and dc and docset and product:
+            result = check_ref_to_subdeliverable(ref, attrs)
+
+        # Case 2: Reference to a deliverable
+        elif dc and docset and product:
+            result = check_ref_to_deliverable(ref, attrs)
+
+        # Case 3: Reference to an external link
+        elif link and docset and product:
+            result = check_ref_to_link(ref, attrs)
+
+        # Case 4: Reference to a docset
+        elif docset and product:
+            result = check_ref_to_docset(ref, attrs)
+
+        # Case 5: Reference to a product
+        elif product:
+            result = check_ref_to_product(ref, attrs)
+
+        # Case 6: Invalid <ref> element
+        else:
+            origin = ref.xpath(
+                'concat(ancestor::product/@productid, "/",  ancestor::docset/@setid)'
+            )
+            result = (
+                f'Reference failed in {origin!r}. '
+                'This issue should have been caught by the RNC validation: '
+                'This is a docserv-stitch bug.'
+            )
+
+        if result:
+            errors.append(result)
+
+    return errors

src/docbuild/config/xml/references.py

@@ -6,11 +6,15 @@
 from copy import deepcopy
 import importlib
 import inspect
+import logging
 from pathlib import Path
 
 from lxml import etree
 
 from ...constants import XMLDATADIR
+from .references import check_stitched_references
+
+log = logging.getLogger(__name__)
 
 
 def load_check_functions() -> list[Callable]:
@@ -30,21 +34,36 @@ def load_check_functions() -> list[Callable]:
 #     print(f'Memory usage: {process.memory_info().rss / 1024**2:.2f} MB')
 
 
+def check_stitchfile(tree: etree._Element | etree._ElementTree) -> bool:
+    """Check the stitchfile for unresolved references.
+
+    :param tree: The tree of the stitched XML file.
+    :returns: True if no unresolved references are found, False otherwise.
+    """
+    result = check_stitched_references(tree)
+
+    for err in result:
+        log.error(err)
+
+    return not bool(result)
+
+
 async def create_stitchfile(
     xmlfiles: Sequence[str | Path],
     *,
     xmlparser: etree.XMLParser | None = None,
+    with_ref_check: bool = True,
 ) -> etree._ElementTree:
     """Asynchronously create a stitch file from a list of XML files.
 
     :param xmlfiles: A sequence of XML file paths to stitch together.
     :param xmlparser: An optional XML parser to use.
+    :param with_ref_check: Whether to perform a reference check (=True) or not (=False).
     :return: all XML file stitched together into a
         :class:`lxml.etree.ElementTree` object.
     """
     # TODO: Should we retrieve them from the config file?
-    simplify_xslt = XMLDATADIR / 'simplify.xsl'
-    reference_xslt = XMLDATADIR / 'global-check-ref-list.xsl'
+    # simplify_xslt = XMLDATADIR / 'simplify.xsl'
 
     # TODO: Do we need the MD5 hashes? If so maybe store them
 
@@ -74,5 +93,9 @@ async def parse_and_xinclude(file_path: Path) -> etree._ElementTree:
         raise ValueError(f'Duplicate product IDs found: {", ".join(duplicates)}')
 
     # Step 3: Check references
+    if with_ref_check:
+        result = check_stitchfile(docservconfig)
+        if not result:
+            raise ValueError('Unresolved references found in stitch file')
 
     return etree.ElementTree(docservconfig)