From 436618dde98a14a49fd19f357cb6280ecb1926e1 Mon Sep 17 00:00:00 2001 From: Richard Bowman Date: Mon, 2 Mar 2026 22:45:31 +0000 Subject: [PATCH 1/4] Validate assignment to BaseProperty.constraints I've upgraded this to a property, and added basic validation to check the dictionary keys. --- src/labthings_fastapi/properties.py | 48 +++++++++++++++++++---------- tests/test_properties.py | 10 ++++++ 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py index 9c374a04..1ccafc02 100644 --- a/src/labthings_fastapi/properties.py +++ b/src/labthings_fastapi/properties.py @@ -350,27 +350,43 @@ def __init__(self, constraints: Mapping[str, Any] | None = None) -> None: super().__init__() self._model: type[BaseModel] | None = None self.readonly: bool = False - self.constraints = constraints or {} - for key in self.constraints: + self._constraints = {} + try: + self.constraints = constraints or {} + except UnsupportedConstraintError: + raise + + @builtins.property + def constraints(self) -> Mapping[str, Any]: # 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 + + @constraints.setter + def constraints(self, new_constraints: Mapping[str, Any]) -> None: + r"""Set the constraints added to the model. + + :param new_constraints: the new value of ``constraints``\ . + + :raises UnsupportedConstraintError: if invalid dictionary keys are present. + """ + for key in new_constraints: if key not in CONSTRAINT_ARGS: raise UnsupportedConstraintError( f"Unknown constraint argument: {key}. \n" f"Supported arguments are: {', '.join(CONSTRAINT_ARGS)}." ) - - constraints: Mapping[str, Any] - """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. - """ + self._constraints = new_constraints @builtins.property def model(self) -> type[BaseModel]: diff --git a/tests/test_properties.py b/tests/test_properties.py index 2d4fd9e9..bb608aa8 100644 --- a/tests/test_properties.py +++ b/tests/test_properties.py @@ -456,6 +456,16 @@ 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} + def test_propertyinfo(): """Check the PropertyInfo class is generated correctly.""" From d603349e4ea60755f16ce765fdf37366425b6e0b Mon Sep 17 00:00:00 2001 From: Richard Bowman Date: Tue, 3 Mar 2026 12:47:13 +0000 Subject: [PATCH 2/4] Use a typeddict for stricter validation and helpful autocompletion The `constraints` property is now a typed dictionary. Assigning dictionary literals to it still ought to work, but it should flag type errors if the keys are incorrect. The method `BaseProperty._validate_constraints` is provided to convert untyped dictionaries with appropriate validation. This now uses `pydantic` to validate the typeddict. It is stricter than what I did before, as it also checks the type of the keys, not just their names. Pydantic was less ugly than coming up with my own logic to coerce an untyped dictionary into a typeddict. I've added a unit test on validation to check it does what I expect. It would be lovely to deduplicate the typeddict and the constant with key names in it - but this is hard to do neatly. --- src/labthings_fastapi/properties.py | 64 +++++++++++++++++++++++------ tests/test_properties.py | 38 +++++++++++++++++ typing_tests/thing_properties.py | 24 +++++++++++ 3 files changed, 114 insertions(+), 12 deletions(-) diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py index 1ccafc02..b6315ed7 100644 --- a/src/labthings_fastapi/properties.py +++ b/src/labthings_fastapi/properties.py @@ -55,6 +55,7 @@ class attribute. Documentation is in strings immediately following the Callable, Generic, TypeVar, + TypedDict, overload, TYPE_CHECKING, ) @@ -62,7 +63,15 @@ class attribute. Documentation is in strings immediately following the 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 +131,21 @@ class attribute. Documentation is in strings immediately following the """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,14 +374,33 @@ def __init__(self, constraints: Mapping[str, Any] | None = None) -> None: super().__init__() self._model: type[BaseModel] | None = None self.readonly: bool = False - self._constraints = {} + self._constraints: FieldConstraints = {} try: - self.constraints = constraints or {} + 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 + @builtins.property - def constraints(self) -> Mapping[str, Any]: # noqa[DOC201] + def constraints(self) -> FieldConstraints: # noqa[DOC201] """Validation constraints applied to this property. This mapping contains keyword arguments that will be passed to @@ -373,20 +416,17 @@ def constraints(self) -> Mapping[str, Any]: # noqa[DOC201] return self._constraints @constraints.setter - def constraints(self, new_constraints: Mapping[str, Any]) -> None: + def constraints(self, new_constraints: FieldConstraints) -> None: r"""Set the constraints added to the model. :param new_constraints: the new value of ``constraints``\ . :raises UnsupportedConstraintError: if invalid dictionary keys are present. """ - for key in new_constraints: - if key not in CONSTRAINT_ARGS: - raise UnsupportedConstraintError( - f"Unknown constraint argument: {key}. \n" - f"Supported arguments are: {', '.join(CONSTRAINT_ARGS)}." - ) - self._constraints = new_constraints + try: + self._constraints = self._validate_constraints(new_constraints) + except UnsupportedConstraintError: + raise @builtins.property def model(self) -> type[BaseModel]: diff --git a/tests/test_properties.py b/tests/test_properties.py index bb608aa8..eef64408 100644 --- a/tests/test_properties.py +++ b/tests/test_properties.py @@ -467,6 +467,44 @@ def functional_bad_prop(self) -> str: 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.""" diff --git a/typing_tests/thing_properties.py b/typing_tests/thing_properties.py index 0a5962ad..5dbadb5b 100644 --- a/typing_tests/thing_properties.py +++ b/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] From ec732db127df01896499541ea08cc3fdaa92852c Mon Sep 17 00:00:00 2001 From: Richard Bowman Date: Tue, 3 Mar 2026 13:40:52 +0000 Subject: [PATCH 3/4] Import TypedDict from typing_extensions While typing has a TypedDict class from Python 3.8, pydantic requires the use of typing_extensions prior to 3.12. --- src/labthings_fastapi/properties.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py index b6315ed7..467bb3b7 100644 --- a/src/labthings_fastapi/properties.py +++ b/src/labthings_fastapi/properties.py @@ -55,11 +55,10 @@ class attribute. Documentation is in strings immediately following the Callable, Generic, TypeVar, - TypedDict, overload, TYPE_CHECKING, ) -from typing_extensions import Self +from typing_extensions import Self, TypedDict from weakref import WeakSet from fastapi import Body, FastAPI From 523f7e1761ec1db86742c30a5c2d16a9a37a0a9c Mon Sep 17 00:00:00 2001 From: Richard Bowman Date: Thu, 26 Mar 2026 13:39:47 +0000 Subject: [PATCH 4/4] Fix a confusing comment --- tests/test_properties.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_properties.py b/tests/test_properties.py index eef64408..fcaaa917 100644 --- a/tests/test_properties.py +++ b/tests/test_properties.py @@ -456,7 +456,7 @@ 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 + # Invalid constraints on functional properties should also raise errors. with pytest.raises(UnsupportedConstraintError): class FunctionalBadConstraintThing(lt.Thing):