Merge pull request #782 from opengisch/QF-3044_debugging_improving_qg…

…is_xml_validation

docker-qgis/process_projectfile.py

@@ -1,56 +1,40 @@
 import logging
 from pathlib import Path
-from typing import Dict
 from xml.etree import ElementTree
 
-from qfieldcloud.qgis.utils import BaseException, get_layers_data, layers_data_to_string
+from qfieldcloud.qgis.utils import (
+    FailedThumbnailGenerationException,
+    InvalidFileExtensionException,
+    InvalidXmlFileException,
+    ProjectFileNotFoundException,
+    get_layers_data,
+    get_qgis_xml_error_context,
+    layers_data_to_string,
+)
 from qgis.core import QgsMapRendererParallelJob, QgsMapSettings, QgsProject
 from qgis.PyQt.QtCore import QEventLoop, QSize
 from qgis.PyQt.QtGui import QColor
 
 logger = logging.getLogger("PROCPRJ")
 
 
-class ProjectFileNotFoundException(BaseException):
-    message = 'Project file "%(project_filename)s" does not exist'
-
-
-class InvalidFileExtensionException(BaseException):
-    message = (
-        'Project file "%(project_filename)s" has unknown file extension "%(extension)s"'
-    )
-
-
-class InvalidXmlFileException(BaseException):
-    message = "Project file is an invalid XML document:\n%(xml_error)s"
-
-
-class InvalidQgisFileException(BaseException):
-    message = 'Project file "%(project_filename)s" is invalid QGIS file:\n%(error)s'
-
-
-class InvalidLayersException(BaseException):
-    message = 'Project file "%(project_filename)s" contains invalid layers'
-
-
-class FailedThumbnailGenerationException(BaseException):
-    message = "Failed to generate project thumbnail:\n%(reason)s"
-
-
 def check_valid_project_file(project_filename: Path) -> None:
     logger.info("Check QGIS project file validity…")
 
     if not project_filename.exists():
         raise ProjectFileNotFoundException(project_filename=project_filename)
 
     if project_filename.suffix == ".qgs":
-        try:
-            with open(project_filename) as f:
-                ElementTree.fromstring(f.read())
-        except ElementTree.ParseError as err:
-            raise InvalidXmlFileException(
-                project_filename=project_filename, xml_error=err
-            )
+        with open(project_filename, "rb") as fh:
+            try:
+                for event, elem in ElementTree.iterparse(fh):
+                    continue
+            except ElementTree.ParseError as error:
+                error_msg = str(error)
+                raise InvalidXmlFileException(
+                    xml_error=get_qgis_xml_error_context(error_msg, fh) or error_msg,
+                    project_filename=project_filename,
+                )
     elif project_filename.suffix != ".qgz":
         raise InvalidFileExtensionException(
             project_filename=project_filename, extension=project_filename.suffix
@@ -71,7 +55,7 @@ def load_project_file(project_filename: Path) -> QgsProject:
     return project
 
 
-def extract_project_details(project: QgsProject) -> Dict[str, str]:
+def extract_project_details(project: QgsProject) -> dict[str, str]:
     """Extract project details"""
     logger.info("Extract project details…")
 

docker-qgis/utils.py

@@ -15,7 +15,7 @@
 from contextlib import contextmanager
 from datetime import datetime
 from pathlib import Path
-from typing import IO, Any, Callable, Dict, List, Optional, Union
+from typing import IO, Any, Callable, NamedTuple, Optional
 
 from libqfieldsync.layer import LayerSource
 from qfieldcloud_sdk import sdk
@@ -39,6 +39,44 @@
 qgs_msglog_logger.setLevel(logging.DEBUG)
 
 
+class QfcWorkerException(Exception):
+    """QFieldCloud Exception"""
+
+    message = ""
+
+    def __init__(self, message: str = None, **kwargs):
+        self.message = (message or self.message) % kwargs
+        self.details = kwargs
+
+        super().__init__(self.message)
+
+
+class ProjectFileNotFoundException(QfcWorkerException):
+    message = 'Project file "%(project_filename)s" does not exist'
+
+
+class InvalidFileExtensionException(QfcWorkerException):
+    message = (
+        'Project file "%(project_filename)s" has unknown file extension "%(extension)s"'
+    )
+
+
+class InvalidXmlFileException(QfcWorkerException):
+    message = "Project file is an invalid XML document:\n%(xml_error)s"
+
+
+class InvalidQgisFileException(QfcWorkerException):
+    message = 'Project file "%(project_filename)s" is invalid QGIS file:\n%(error)s'
+
+
+class InvalidLayersException(QfcWorkerException):
+    message = 'Project file "%(project_filename)s" contains invalid layers'
+
+
+class FailedThumbnailGenerationException(QfcWorkerException):
+    message = "Failed to generate project thumbnail:\n%(reason)s"
+
+
 def _qt_message_handler(mode, context, message):
     log_level = logging.DEBUG
     if mode == QtCore.QtDebugMsg:
@@ -268,7 +306,7 @@ def __init__(
         id: str,
         version: str,
         name: str,
-        steps: List["Step"],
+        steps: list["Step"],
         description: str = "",
     ):
         self.id = id
@@ -331,9 +369,9 @@ def __init__(
         id: str,
         name: str,
         method: Callable,
-        arguments: Dict[str, Any] = {},
-        return_names: List[str] = [],
-        outputs: List[str] = [],
+        arguments: dict[str, Any] = {},
+        return_names: list[str] = [],
+        outputs: list[str] = [],
     ):
         self.id = id
         self.name = name
@@ -366,18 +404,6 @@ def eval(self, root: Path) -> Path:
         return path
 
 
-class BaseException(Exception):
-    """QFieldCloud Exception"""
-
-    message = ""
-
-    def __init__(self, message: str = None, **kwargs):
-        self.message = (message or self.message) % kwargs
-        self.details = kwargs
-
-        super().__init__(self.message)
-
-
 @contextmanager
 def logger_context(step: Step):
     log_uuid = uuid.uuid4()
@@ -437,7 +463,7 @@ def get_layer_filename(layer: QgsMapLayer) -> Optional[str]:
     return None
 
 
-def extract_project_details(project: QgsProject) -> Dict[str, str]:
+def extract_project_details(project: QgsProject) -> dict[str, str]:
     """Extract project details"""
     map_settings = QgsMapSettings()
     details = {}
@@ -487,8 +513,8 @@ def json_default(obj):
 
 def run_workflow(
     workflow: Workflow,
-    feedback_filename: Optional[Union[IO, Path]],
-) -> Dict:
+    feedback_filename: Optional[Path | IO],
+) -> dict[str, Any]:
     """Executes the steps required to run a task and return structured feedback from the execution
 
     Each step has a method that is executed.
@@ -499,9 +525,9 @@ def run_workflow(
 
     Args:
         workflow (Workflow): workflow to be executed
-        feedback_filename (Optional[Union[IO, Path]]): write feedback to an IO device, to Path filename, or don't write it
+        feedback_filename (IO | Path): write feedback to an IO device, to Path filename, or don't write it
     """
-    feedback: Dict[str, Any] = {
+    feedback: dict[str, Any] = {
         "feedback_version": "2.0",
         "workflow_version": workflow.version,
         "workflow_id": workflow.id,
@@ -552,10 +578,12 @@ def run_workflow(
                 feedback["error_type"] = "API_OTHER"
         elif isinstance(err, FileNotFoundError):
             feedback["error_type"] = "FILE_NOT_FOUND"
+        elif isinstance(err, InvalidXmlFileException):
+            feedback["error_type"] = "INVALID_PROJECT_FILE"
         else:
             feedback["error_type"] = "UNKNOWN"
 
-        (_type, _value, tb) = sys.exc_info()
+        _type, _value, tb = sys.exc_info()
         feedback["error_class"] = type(err).__name__
         feedback["error_stack"] = traceback.format_tb(tb)
     finally:
@@ -596,7 +624,7 @@ def run_workflow(
         return feedback
 
 
-def get_layers_data(project: QgsProject) -> Dict[str, Dict]:
+def get_layers_data(project: QgsProject) -> dict[str, dict]:
     layers_by_id = {}
 
     for layer in project.mapLayers().values():
@@ -711,7 +739,7 @@ def get_file_md5sum(filename: str) -> str:
     return hasher.hexdigest()
 
 
-def files_list_to_string(files: List[Dict[str, Any]]) -> str:
+def files_list_to_string(files: list[dict[str, Any]]) -> str:
     table = [
         [
             d["name"],
@@ -814,3 +842,42 @@ def setup_basic_logging_config():
 
     for handler in logging.root.handlers:
         handler.setFormatter(formatter)
+
+
+class XmlErrorLocation(NamedTuple):
+    line: int
+    column: int
+
+
+def get_qgis_xml_error_location(
+    invalid_token_error_msg: str,
+) -> Optional[XmlErrorLocation]:
+    """Get column and line numbers from the provided error message."""
+    if "invalid token" not in invalid_token_error_msg.casefold():
+        return None
+
+    _, details = invalid_token_error_msg.split(":")
+    line, column = details.split(",")
+    _, line_number = line.strip().split(" ")
+    _, column_number = column.strip().split(" ")
+
+    return XmlErrorLocation(int(line_number), int(column_number))
+
+
+def get_qgis_xml_error_context(
+    invalid_token_error_msg: str, fh: io.BufferedReader
+) -> Optional[str]:
+    """Get a slice of the line where the exception occurred, with all faulty occurrences sanitized."""
+    location = get_qgis_xml_error_location(invalid_token_error_msg)
+    if location:
+        substitute = "?"
+        fh.seek(0)
+        for cursor_pos, line in enumerate(fh, start=1):
+            if location.line == cursor_pos:
+                faulty_char = line[location.column]
+                suffix_slice = line[: location.column - 1]
+                clean_safe_slice = suffix_slice.decode("utf-8").strip() + substitute
+
+                return f"Unable to parse character: {repr(faulty_char)}. Replaced by '{substitute}' on line {location.line} that starts with: {clean_safe_slice}"
+
+    return None