Merge pull request #147 from python-odin/development

timsavage · web-flow · commit 5590b7cef1cf · 2023-03-09T10:39:07.000+11:00
Relese 2.5
diff --git a/HISTORY b/HISTORY
@@ -1,3 +1,12 @@
+2.5
+===
+
+- Add ValidationErrorCollection helper for simplifying collection of errors in custom validation code.
+- Change behaviour of to_python method on a CompositeField to not do a full clean.
+  This is not required as this should be completed during the validation stage.
+  This prevents double validation and solves the issue of resources not being populated at all if
+  any contained field contains an error.
+
 2.4
 ===
 
diff --git a/docs/ref/helpers.rst b/docs/ref/helpers.rst
@@ -0,0 +1,2 @@
+.. automodule:: odin.helpers
+    :members:
diff --git a/docs/ref/index.rst b/docs/ref/index.rst
@@ -14,4 +14,5 @@ API Reference
    validators
    traversal
    decorators
+   helpers
    utils
diff --git a/poetry.lock b/poetry.lock
diff --git a/pyproject.toml b/pyproject.toml
@@ -4,7 +4,7 @@ build-backend = "poetry.core.masonry.api"
 
 [tool.poetry]
 name = "odin"
-version = "2.4"
+version = "2.5"
 description = "Data-structure definition/validation/traversal, mapping and serialisation toolkit for Python"
 authors = ["Tim Savage <tim@savage.company>"]
 license = "BSD-3-Clause"
diff --git a/src/odin/__init__.py b/src/odin/__init__.py
@@ -13,6 +13,7 @@
 from odin.proxy import ResourceProxy  # noqa
 from odin.annotated_resource import *  # noqa
 from odin.annotated_resource import type_aliases as types  # noqa
+from odin.helpers import *  # noqa
 
 __authors__ = "Tim Savage <tim@savage.company>"
 __copyright__ = "Copyright (C) 2021 Tim Savage"
diff --git a/src/odin/contrib/rich/theme.py b/src/odin/contrib/rich/theme.py
@@ -0,0 +1,22 @@
+"""Rich Theme definition."""
+from typing import Dict
+
+from rich import get_console
+from rich.theme import Theme
+from rich.style import Style
+
+ODIN_STYLES: Dict[str, Style] = {
+    "odin.resource.name": Style(color="bright_cyan"),
+    "odin.resource.error": Style(color="red", underline=True),
+    "odin.field.name": Style(color="bright_blue"),
+    "odin.field.error": Style(color="red", italic=True),
+    "odin.field.type": Style(color="magenta"),
+    "odin.field.doc": Style(),
+}
+
+odin_theme = Theme(ODIN_STYLES, inherit=False)
+
+
+def add_odin_theme():
+    """Add odin to builtin theme."""
+    get_console().push_theme(odin_theme)
diff --git a/src/odin/contrib/rich/validation_tree.py b/src/odin/contrib/rich/validation_tree.py
@@ -1,9 +1,11 @@
 """Integration with Rich for nicer CLI's!"""
+from rich.text import Text
 from typing import Iterable, Union
 
 from rich.tree import Tree
 
 from odin.exceptions import ValidationError, NON_FIELD_ERRORS
+from .theme import odin_theme
 
 
 def _all_str(iterable: Iterable) -> bool:
@@ -15,18 +17,20 @@ def _validation_error_to_tree(error_messages: Union[list, dict], tree: Tree):
     """Internal recursive method."""
 
     if isinstance(error_messages, dict):
-        for key, value in error_messages.items():
+        for name, value in error_messages.items():
 
             node = tree.add(
-                f"[yellow]:memo:" if key == NON_FIELD_ERRORS else f"[green]{key}"
+                f"[odin.resource.name]+"
+                if name == NON_FIELD_ERRORS
+                else f"[odin.field.name]{name}"
             )
 
             _validation_error_to_tree(value, node)
 
     elif isinstance(error_messages, list):
         if _all_str(error_messages):
             for message in error_messages:
-                tree.add(f"[italic]{message}", guide_style="bold")
+                tree.add(f"[odin.field.error]{message}", guide_style="bold")
 
         else:
             for idx, value in enumerate(error_messages):
@@ -47,6 +51,8 @@ def validation_error_tree(error: ValidationError, *, tree: Tree = None) -> Tree:
         print(tree)
 
     """
-    tree = tree or Tree("[red bold]Validation Errors")
+    tree = tree or Tree(
+        "[red bold]Validation Errors",
+    )
     _validation_error_to_tree(error.error_messages, tree)
     return tree
diff --git a/src/odin/contrib/sphinx/__init__.py b/src/odin/contrib/sphinx/__init__.py
@@ -153,44 +153,46 @@ def document_members(self, all_members: bool = False) -> None:
             for f in field_iter(self.object, self.options.include_virtual)
         ]
 
-        # Calculate table column widths
-        name_len = 4
-        data_type_len = 9
-        details_len = 7
-        for name, data_type, details in data_table:
-            name_len = max(len(name), name_len)
-            data_type_len = max(len(data_type), data_type_len)
-            details_len = max(max(len(l) for l in details), details_len)
-        name_len += 2  # Padding
-        data_type_len += 2  # Padding
-        details_len += 2  # Padding
-
-        def add_separator(char="-"):
-            self.add_line(
-                f"+{char * name_len}+{char * data_type_len}+{char * details_len}+",
-                "<odin_sphinx>",
-            )
-
-        def add_row_line(name, data_type, details):
-            self.add_line(
-                f"| {name}{' ' * (name_len - len(name) - 2)} "
-                f"| {data_type}{' ' * (data_type_len - len(data_type) - 2)} "
-                f"| {details}{' ' * (details_len - len(details) - 2)} |",
-                "<odin_sphinx>",
-            )
-
-        def add_row(name, data_type, details):
-            add_row_line(name, data_type, details.pop(0))
-            for line in details:
-                add_row_line("", "", line)
-
-        # Generate table
-        add_separator()
-        add_row("Name", "Data type", ["Details"])
-        add_separator("=")
-        for row in data_table:
-            add_row(*row)
+        # Generate output if there is any.
+        if data_table:
+            # Calculate table column widths
+            name_len = 4
+            data_type_len = 9
+            details_len = 7
+            for name, data_type, details in data_table:
+                name_len = max(len(name), name_len)
+                data_type_len = max(len(data_type), data_type_len)
+                details_len = max(max(len(l) for l in details), details_len)
+            name_len += 2  # Padding
+            data_type_len += 2  # Padding
+            details_len += 2  # Padding
+
+            def add_separator(char="-"):
+                self.add_line(
+                    f"+{char * name_len}+{char * data_type_len}+{char * details_len}+",
+                    "<odin_sphinx>",
+                )
+
+            def add_row_line(name, data_type, details):
+                self.add_line(
+                    f"| {name}{' ' * (name_len - len(name) - 2)} "
+                    f"| {data_type}{' ' * (data_type_len - len(data_type) - 2)} "
+                    f"| {details}{' ' * (details_len - len(details) - 2)} |",
+                    "<odin_sphinx>",
+                )
+
+            def add_row(name, data_type, details):
+                add_row_line(name, data_type, details.pop(0))
+                for line in details:
+                    add_row_line("", "", line)
+
+            # Generate table
             add_separator()
+            add_row("Name", "Data type", ["Details"])
+            add_separator("=")
+            for row in data_table:
+                add_row(*row)
+                add_separator()
 
 
 def setup(app: Sphinx):
diff --git a/src/odin/fields/__init__.py b/src/odin/fields/__init__.py
@@ -649,6 +649,10 @@ def __init__(self, **options):
         options.setdefault("default", dict)
         super().__init__(**options)
 
+    def __iter__(self):
+        # This does nothing but it does prevent inspections from complaining.
+        return None  # NoQA
+
     def to_python(self, value):
         if value is None:
             return value
@@ -674,6 +678,10 @@ def __init__(self, **options):
         options.setdefault("default", list)
         super().__init__(**options)
 
+    def __iter__(self):
+        # This does nothing but it does prevent inspections from complaining.
+        return None  # NoQA
+
     def to_python(self, value):
         if value is None:
             return value
diff --git a/src/odin/fields/composite.py b/src/odin/fields/composite.py
@@ -49,7 +49,7 @@ def to_python(self, value):
         if isinstance(value, self.of):
             return value
         if isinstance(value, dict):
-            return create_resource_from_dict(value, self.of)
+            return create_resource_from_dict(value, self.of, full_clean=False)
         msg = self.error_messages["invalid"] % self.of
         raise exceptions.ValidationError(msg)
 
diff --git a/src/odin/helpers.py b/src/odin/helpers.py
@@ -0,0 +1,64 @@
+"""
+Helpers
+~~~~~~~
+
+A collection of useful convenience methods.
+"""
+
+from typing import Union, List, Dict, DefaultDict
+
+from odin import BaseField
+from odin.exceptions import NON_FIELD_ERRORS, ValidationError
+
+__all__ = ("ValidationErrorCollection",)
+
+
+class ValidationErrorCollection:
+    """Helper collection for collecting validation error messages and generating or raising an exception.
+
+    Usage:
+
+    .. code-block:: python
+
+        errors = ValidationErrorCollection()
+        ... # Perform validation
+        errors.add_message("name", "Value is required")
+
+        if errors:
+            raise errors.validation_error()
+
+    """
+
+    def __init__(self):
+        """Initialise collection."""
+        self.error_messages = DefaultDict[str, List[str]](list)
+
+    def __bool__(self):
+        return bool(self.messages)
+
+    @property
+    def messages(self) -> Dict[str, List[str]]:
+        """Filtered messages that strips out empty messages."""
+        return {
+            field_name: messages
+            for field_name, messages in self.error_messages.items()
+            if messages
+        }
+
+    def add_message(self, field: Union[str, BaseField], *messages):
+        """Append validation error message(s)."""
+        field_name = field if isinstance(field, str) else field.attname
+        self.error_messages[field_name].extend(messages)
+
+    def add_resource_message(self, *messages):
+        """Append resource level validation error message(s)."""
+        self.error_messages[NON_FIELD_ERRORS].extend(messages)
+
+    def raise_if_defined(self):
+        """Raise an exception if any are defined."""
+        if self:
+            raise self.validation_error()
+
+    def validation_error(self) -> ValidationError:
+        """Generate an exception based on the validation messages added."""
+        return ValidationError(self.messages)
diff --git a/tests/test_helpers.py b/tests/test_helpers.py
@@ -0,0 +1,57 @@
+import pytest
+
+from odin import helpers
+from odin.exceptions import ValidationError
+from odin.utils import getmeta
+from .resources import Author
+
+
+class TestValidationErrorCollection:
+    def test_messages__with_valid_message(self):
+        target = helpers.ValidationErrorCollection()
+
+        target.add_message("name", "this is required")
+
+        assert target.messages == {"name": ["this is required"]}
+
+    def test_messages__with_empty_messages(self):
+        target = helpers.ValidationErrorCollection()
+
+        target.add_message("name")
+
+        assert target.messages == {}
+
+    def test_add_message__with_string_field_name(self):
+        target = helpers.ValidationErrorCollection()
+
+        target.add_message("name", "this is required")
+
+        assert target.messages == {"name": ["this is required"]}
+
+    def test_add_message__with_field_instance(self):
+        field = getmeta(Author).field_map["name"]
+        target = helpers.ValidationErrorCollection()
+
+        target.add_message(field, "this is required")
+
+        assert target.messages == {"name": ["this is required"]}
+
+    def test_add_resource_message(self):
+        target = helpers.ValidationErrorCollection()
+
+        target.add_resource_message("this is required")
+
+        assert target.messages == {"__all__": ["this is required"]}
+
+    def test_raise_if_defined__with_no_errors(self):
+        target = helpers.ValidationErrorCollection()
+
+        # Nothing should happen
+        target.raise_if_defined()
+
+    def test_raise_if_defined__with_errors(self):
+        target = helpers.ValidationErrorCollection()
+        target.add_message("name", "this is required")
+
+        with pytest.raises(ValidationError):
+            target.raise_if_defined()

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+.. automodule:: odin.helpers`
	`2`	`+ :members:`