Merge pull request #1846 from oakal510/configuration-error-document-t…

…ypes

Create validation for document types with error handling

changes/1846.bugfix.rst

@@ -0,0 +1 @@
+Document type declarations are now fully validated.

pyproject.toml

@@ -101,9 +101,8 @@ dev = [
     # Pre-commit 3.6.0 deprecated support for Python 3.8
     "pre-commit == 3.5.0 ; python_version < '3.9'",
     "pre-commit == 3.7.1 ; python_version >= '3.9'",
-    "pytest == 8.3.1",
-    # Having xdist in the pytest environment causes some weird failures on Windows with Py3.13
-    "pytest-xdist == 3.6.1 ; python_version < '3.13'",
+    "pytest == 8.3.2",
+    "pytest-xdist == 3.6.1",
     "setuptools_scm == 8.1.0",
     "tox == 4.16.0",
 ]

src/briefcase/commands/base.py

@@ -1035,7 +1035,6 @@ def _generate_template(self, template, branch, output_path, extra_context):
         )
 
         self.logger.configure_stdlib_logging("cookiecutter")
-
         try:
             # Unroll the template.
             self.tools.cookiecutter(
@@ -1059,6 +1058,8 @@ def _generate_template(self, template, branch, output_path, extra_context):
         except cookiecutter_exceptions.RepositoryCloneFailed as e:
             # Branch does not exist.
             raise TemplateUnsupportedVersion(branch) from e
+        except cookiecutter_exceptions.UndefinedVariableInTemplate as e:
+            raise BriefcaseConfigError(e.message) from e
 
     def generate_template(
         self,

src/briefcase/commands/convert.py

@@ -18,7 +18,7 @@
 else:  # pragma: no-cover-if-gte-py311
     import tomli as tomllib
 
-from briefcase.config import make_class_name
+from briefcase.config import make_class_name, validate_url
 from briefcase.exceptions import BriefcaseCommandError
 
 
@@ -299,7 +299,7 @@ def input_url(self, app_name, override_value: str | None) -> str:
                 ),
                 variable="application URL",
                 default=default,
-                validator=self.validate_url,
+                validator=validate_url,
                 override_value=override_value,
             )
 
@@ -321,7 +321,7 @@ def input_url(self, app_name, override_value: str | None) -> str:
                 intro="What website URL do you want to use for the application?",
                 variable="application URL",
                 default=self.make_project_url("com.example", app_name),
-                validator=self.validate_url,
+                validator=validate_url,
             )
 
         return url

src/briefcase/commands/new.py

@@ -7,7 +7,6 @@
 from collections.abc import Sequence
 from email.utils import parseaddr
 from typing import Iterable
-from urllib.parse import urlparse
 
 if sys.version_info >= (3, 10):  # pragma: no-cover-if-lt-py310
     from importlib.metadata import entry_points
@@ -21,6 +20,7 @@
     is_valid_app_name,
     is_valid_bundle_identifier,
     make_class_name,
+    validate_url,
 )
 from briefcase.console import MAX_TEXT_WIDTH
 from briefcase.console import select_option as _select_option
@@ -300,20 +300,6 @@ def make_project_url(self, bundle, app_name):
         """
         return f"https://{self.make_domain(bundle)}/{app_name}"
 
-    def validate_url(self, candidate):
-        """Determine if the URL is valid.
-
-        :param candidate: The candidate URL
-        :returns: True. If there are any validation problems, raises ValueError with a
-            diagnostic message.
-        """
-        result = urlparse(candidate)
-        if not all([result.scheme, result.netloc]):
-            raise ValueError("Not a valid URL!")
-        if result.scheme not in {"http", "https"}:
-            raise ValueError("Not a valid website URL!")
-        return True
-
     def prompt_divider(self, title: str = ""):
         """Writes a divider with an optional title."""
         title = f"-- {title} " if title else ""
@@ -576,7 +562,7 @@ def build_app_context(self, project_overrides: dict[str, str]) -> dict[str, str]
             ),
             variable="application URL",
             default=self.make_project_url(bundle, app_name),
-            validator=self.validate_url,
+            validator=validate_url,
             override_value=project_overrides.pop("url", None),
         )
         project_license = self.input_license(

src/briefcase/config.py

@@ -4,6 +4,7 @@
 import sys
 import unicodedata
 from types import SimpleNamespace
+from urllib.parse import urlparse
 
 if sys.version_info >= (3, 11):  # pragma: no-cover-if-lt-py311
     import tomllib
@@ -83,6 +84,68 @@ def make_class_name(formal_name):
     return class_name
 
 
+def validate_url(candidate):
+    """Determine if the URL is valid.
+
+    :param candidate: The candidate URL
+    :returns: True. If there are any validation problems, raises ValueError with a
+        diagnostic message.
+    """
+    result = urlparse(candidate)
+    if not all([result.scheme, result.netloc]):
+        raise ValueError("Not a valid URL!")
+    if result.scheme not in {"http", "https"}:
+        raise ValueError("Not a valid website URL!")
+    return True
+
+
+def validate_document_type_config(document_type_id, document_type):
+
+    try:
+        if not (
+            isinstance(document_type["extension"], str)
+            and document_type["extension"].isalnum()
+        ):
+            raise BriefcaseConfigError(
+                f"The extension provided for document type {document_type_id!r} is not alphanumeric."
+            )
+    except KeyError:
+        raise BriefcaseConfigError(
+            f"Document type {document_type_id!r} does not provide an extension."
+        )
+
+    try:
+        if not isinstance(document_type["icon"], str):
+            raise BriefcaseConfigError(
+                f"The icon definition associated with document type {document_type_id!r} is not a string."
+            )
+    except KeyError:
+        raise BriefcaseConfigError(
+            f"Document type {document_type_id!r} does not define an icon."
+        )
+
+    try:
+        if not isinstance(document_type["description"], str):
+            raise BriefcaseConfigError(
+                f"The description associated with document type {document_type_id!r} is not a string."
+            )
+    except KeyError:
+        raise BriefcaseConfigError(
+            f"Document type {document_type_id!r} does not provide a description."
+        )
+
+    try:
+        validate_url(document_type["url"])
+    except KeyError:
+        raise BriefcaseConfigError(
+            f"Document type {document_type_id!r} does not provide a URL."
+        )
+    except ValueError as e:
+        raise BriefcaseConfigError(
+            f"The URL associated with document type {document_type_id!r} is invalid: {e}"
+        )
+
+
 VALID_BUNDLE_RE = re.compile(r"[a-zA-Z0-9-]+(\.[a-zA-Z0-9-]+)+$")
 
 
@@ -247,6 +310,9 @@ def __init__(
                 "'switch', or 'while')."
             )
 
+        for document_type_id, document_type in self.document_types.items():
+            validate_document_type_config(document_type_id, document_type)
+
         # Version number is PEP440 compliant:
         if not is_pep440_canonical_version(self.version):
             raise BriefcaseConfigError(

tests/commands/convert/test_input_url.py

@@ -1,5 +1,7 @@
 from unittest.mock import MagicMock
 
+from briefcase.config import validate_url
+
 
 def test_multiple_pep621_options(convert_command, monkeypatch):
     mock_select_option = MagicMock()
@@ -64,7 +66,7 @@ def test_multiple_pep621_options_select_other(convert_command, monkeypatch):
         intro="What website URL do you want to use for the application?",
         variable="application URL",
         default="https://example.com/some-name",
-        validator=convert_command.validate_url,
+        validator=validate_url,
     )
 
 
@@ -81,7 +83,7 @@ def test_no_pep621_options(convert_command, monkeypatch):
         ),
         variable="application URL",
         default=default,
-        validator=convert_command.validate_url,
+        validator=validate_url,
         override_value=None,
     )
 

tests/commands/create/test_generate_app_template.py

@@ -11,6 +11,7 @@
 import briefcase
 from briefcase.exceptions import (
     BriefcaseCommandError,
+    BriefcaseConfigError,
     NetworkFailure,
     TemplateUnsupportedVersion,
 )
@@ -722,3 +723,25 @@ def test_x_permissions(
         extra_context=full_context,
         default_config={"replay_dir": str(tmp_path / "data/templates/.replay")},
     )
+
+
+def test_cookiecutter_undefined_variable_in_template(
+    myapp,
+    create_command,
+):
+    """If the cookiecutter template has an undefined variable, a breifcase configuration
+    error is raised."""
+
+    cookiecutter_exception_message = "undefined document type"
+    create_command.tools.cookiecutter.side_effect = (
+        cookiecutter_exceptions.UndefinedVariableInTemplate(
+            cookiecutter_exception_message, "error", "context"
+        )
+    )
+
+    with pytest.raises(
+        BriefcaseConfigError,
+        match=f"Briefcase configuration error: {cookiecutter_exception_message}",
+    ):
+        # Generating the template with an undefined cookiecutter variable generates an error
+        create_command.generate_app_template(myapp)

tests/commands/create/test_install_app_resources.py

@@ -254,9 +254,17 @@ def test_doctype_icon_target(create_command, tmp_path):
         description="This is a simple app",
         sources=["src/my_app"],
         document_type={
-            "mydoc": {"icon": "images/mydoc-icon"},
+            "mydoc": {
+                "icon": "images/mydoc-icon",
+                "description": "mydoc image",
+                "url": "https://testexample.com",
+                "extension": "mydoc",
+            },
             "other": {
                 "icon": "images/other-icon",
+                "description": "other image",
+                "url": "https://othertestexample.com",
+                "extension": "other",
             },
         },
         license={"file": "LICENSE"},

tests/commands/new/test_make_project_url.py

@@ -1,5 +1,7 @@
 import pytest
 
+from briefcase.config import validate_url
+
 
 @pytest.mark.parametrize(
     "app_name, bundle, candidate",
@@ -14,4 +16,4 @@ def test_make_project_url(new_command, app_name, bundle, candidate):
     url = new_command.make_project_url(bundle, app_name)
     assert url == candidate
     # Double check - the app name passes the validity check.
-    assert new_command.validate_url(url)
+    assert validate_url(url)

tests/config/test_AppConfig.py

@@ -56,8 +56,10 @@ def test_extra_attrs():
         requires=["first", "second", "third"],
         document_type={
             "document": {
+                "icon": "icon",
                 "extension": "doc",
                 "description": "A document",
+                "url": "https://testurl.com",
             }
         },
         first="value 1",
@@ -78,8 +80,10 @@ def test_extra_attrs():
     assert config.class_name == "MyApp"
     assert config.document_types == {
         "document": {
+            "icon": "icon",
             "extension": "doc",
             "description": "A document",
+            "url": "https://testurl.com",
         }
     }