Validate constraint keys

src/labthings_fastapi/properties.py

@@ -58,11 +58,19 @@
     overload,
     TYPE_CHECKING,
 )
-from typing_extensions import Self
+from typing_extensions import Self, TypedDict
 from weakref import WeakSet
 
 from fastapi import Body, FastAPI
-from pydantic import BaseModel, ConfigDict, RootModel, ValidationError, create_model
+from pydantic import (
+    BaseModel,
+    ConfigDict,
+    RootModel,
+    TypeAdapter,
+    ValidationError,
+    create_model,
+    with_config,
+)
 
 from .thing_description import type_to_dataschema
 from .thing_description._model import (
@@ -122,6 +130,21 @@
 """The set of supported constraint arguments for properties."""
 
 
+@with_config(ConfigDict(extra="forbid"))
+class FieldConstraints(TypedDict, total=False):
+    r"""Constraints that may be applied to a `.property`\ ."""
+
+    gt: int | float
+    ge: int | float
+    lt: int | float
+    le: int | float
+    multiple_of: int | float
+    allow_inf_nan: bool
+    min_length: int
+    max_length: int
+    pattern: str
+
+
 # The following exceptions are raised only when creating/setting up properties.
 class OverspecifiedDefaultError(ValueError):
     """The default value has been specified more than once.
@@ -350,27 +373,59 @@
         super().__init__()
         self._model: type[BaseModel] | None = None
         self.readonly: bool = False
-        self.constraints = constraints or {}
-        for key in self.constraints:
-            if key not in CONSTRAINT_ARGS:
-                raise UnsupportedConstraintError(
-                    f"Unknown constraint argument: {key}. \n"
-                    f"Supported arguments are: {', '.join(CONSTRAINT_ARGS)}."
-                )
+        self._constraints: FieldConstraints = {}
+        try:
+            self.constraints = self._validate_constraints(constraints or {})
+        except UnsupportedConstraintError:
+            raise
+
+    @staticmethod
+    def _validate_constraints(constraints: Mapping[str, Any]) -> FieldConstraints:
+        """Validate an untyped dictionary of constraints.
+
+        :param constraints: A mapping that will be validated against the
+            `.FieldConstraints` typed dictionary.
+        :return: A `.FieldConstraints` instance.
+        :raises UnsupportedConstraintError: if the input is not valid.
+        """
+        validator = TypeAdapter(FieldConstraints)
+        try:
+            return validator.validate_python(constraints)
+        except ValidationError as e:
+            raise UnsupportedConstraintError(
+                f"Bad constraint arguments were supplied ({constraints}). \n"
+                f"Supported arguments are: {', '.join(CONSTRAINT_ARGS)}.\n"
+                f"Validation error details are below: \n\n{e}"
+            ) from e
 
-    constraints: Mapping[str, Any]
-    """Validation constraints applied to this property.
+    @builtins.property
+    def constraints(self) -> FieldConstraints:  # noqa[DOC201]
+        """Validation constraints applied to this property.
+
+        This mapping contains keyword arguments that will be passed to
+        `pydantic.Field` to add validation constraints to the property.
+        See `pydantic.Field` for details. The module-level constant
+        `CONSTRAINT_ARGS` lists the supported constraint arguments.
+
+        Note that these constraints will be enforced when values are
+        received over HTTP, but they are not automatically enforced
+        when setting the property directly on the `.Thing` instance
+        from Python code.
+        """
+        return self._constraints
 
-    This mapping contains keyword arguments that will be passed to
-    `pydantic.Field` to add validation constraints to the property.
-    See `pydantic.Field` for details. The module-level constant
-    `CONSTRAINT_ARGS` lists the supported constraint arguments.
+    @constraints.setter
+    def constraints(self, new_constraints: FieldConstraints) -> None:
+        r"""Set the constraints added to the model.
 
-    Note that these constraints will be enforced when values are
-    received over HTTP, but they are not automatically enforced
-    when setting the property directly on the `.Thing` instance
-    from Python code.
-    """
+        :param new_constraints: the new value of ``constraints``\ .
+
+        :raises UnsupportedConstraintError: if invalid dictionary keys are present.
+        """
+        try:
+            self._constraints = self._validate_constraints(new_constraints)
+        except UnsupportedConstraintError:
+            raise
 
     @builtins.property
     def model(self) -> type[BaseModel]:
@@ -696,11 +751,11 @@
         self._fget = fget
         self._type = return_type(self._fget)
         if self._type is None:
             msg = (
                 f"{fget} does not have a valid type. "
                 "Return type annotations are required for property getters."
             )
             raise MissingTypeError(msg)
         self._fset: Callable[[Owner, Value], None] | None = None
         self.readonly: bool = True
 
@@ -723,10 +778,10 @@
         :param fget: The new getter function.
         :return: this descriptor (i.e. ``self``). This allows use as a decorator.
         """
         self._fget = fget
         self._type = return_type(self._fget)
         self.__doc__ = fget.__doc__
         return self
 
     def setter(self, fset: Callable[[Owner, Value], None]) -> Self:
         r"""Set the setter function of the property.
@@ -798,7 +853,7 @@
             # Don't return the descriptor if it's named differently.
             # see typing notes in docstring.
             return fset  # type: ignore[return-value]
         return self
 
     def instance_get(self, obj: Owner) -> Value:
         """Get the value of the property.
@@ -817,7 +872,7 @@
         :raises ReadOnlyPropertyError: if the property cannot be set.
         """
         if self.fset is None:
             raise ReadOnlyPropertyError(f"Property {self.name} of {obj} has no setter.")
         self.fset(obj, value)
 
 
@@ -1044,7 +1099,7 @@
 
         :raises NotImplementedError: this method should be implemented in subclasses.
         """
         raise NotImplementedError("This method should be implemented in subclasses.")
 
     def descriptor_info(self, owner: Owner | None = None) -> SettingInfo[Owner, Value]:
         r"""Return an object that allows access to this descriptor's metadata.

tests/test_properties.py

@@ -456,6 +456,54 @@ class AnotherBadConstraintThing(lt.Thing):
     # as metadata if used on the wrong type. We don't currently raise errors
     # for these.
 
+    # We should also raise errors if constraints are set after a property is defined
+    with pytest.raises(UnsupportedConstraintError):
+
+        class FunctionalBadConstraintThing(lt.Thing):
+            @lt.property
+            def functional_bad_prop(self) -> str:
+                return "foo"
+
+            functional_bad_prop.constraints = {"bad_constraint": 2}
+
+
+GOOD_CONSTRAINTS = []
+# Single numeric constraints (test float and int)
+GOOD_CONSTRAINTS += [
+    {k: v} for k in ["ge", "gt", "le", "lt", "multiple_of"] for v in [3, 3.4]
+]
+# Max/min length
+GOOD_CONSTRAINTS += [{k: 10} for k in ["max_length", "min_length"]]
+# Allow_inf_nan
+GOOD_CONSTRAINTS += [{"allow_inf_nan": v} for v in [True, False]]
+# Pattern
+GOOD_CONSTRAINTS += [{"pattern": v} for v in ["test", r"[0-9]+"]]
+
+
+BAD_CONSTRAINTS = []
+# These should be numerics
+BAD_CONSTRAINTS += [
+    {k: "str"}
+    for k in ["ge", "gt", "le", "lt", "multiple_of", "max_length", "min_length"]
+]
+# pattern must be a string
+BAD_CONSTRAINTS += [{"pattern": 152}]
+# other keys should not be allowed
+BAD_CONSTRAINTS += [{"invalid": None}]
+
+
+@pytest.mark.parametrize("constraints", GOOD_CONSTRAINTS)
+def test_successful_constraint_validation(constraints):
+    """Check valid constraints values are passed through."""
+    assert BaseProperty._validate_constraints(constraints) == constraints
+
+
+@pytest.mark.parametrize("constraints", BAD_CONSTRAINTS)
+def test_unsuccessful_constraint_validation(constraints):
+    """Check invalid constraints values are flagged."""
+    with pytest.raises(UnsupportedConstraintError):
+        BaseProperty._validate_constraints(constraints)
+
 
 def test_propertyinfo():
     """Check the PropertyInfo class is generated correctly."""

typing_tests/thing_properties.py

@@ -288,3 +288,27 @@ def strprop(self, val: str) -> None:
 assert_type(test_functional_property.intprop3, int)
 assert_type(test_functional_property.fprop, int)
 # ``strprop`` will be ``Any`` because of the ``[no-redef]`` error.
+
+
+class TestConstrainedProperties(lt.Thing):
+    """A class with some correctly and incorrectly-defined constraints."""
+
+    # Constraints can be passed as kwargs to `lt.property` but currently
+    # aren't explicit, so don't get checked by mypy.
+    # The line below is valid
+    positiveint: int = lt.property(default=0, ge=0)
+
+    # The line below is not valid but doesn't bother mypy.
+    # This would get picked up at runtime, as we validate the kwargs.
+    negativeint: int = lt.property(default=0, sign="negative")
+
+    @lt.property
+    def positivefloat(self) -> float:
+        """A functional property."""
+        return 42
+
+    positivefloat.constraints = {"gt": 0.0}  # This is OK
+
+    # The typed dict checks the name and type of constraints, so the line
+    # below should be flagged. This is also validated at runtime by pydantic
+    positivefloat.constraints = {"gt": "zero"}  # type:ignore[typeddict-item]