From 54fad998c500bd1befeb66b28b9ad06c3ec5d3c6 Mon Sep 17 00:00:00 2001 From: Richard Bowman Date: Mon, 2 Mar 2026 22:45:31 +0000 Subject: [PATCH 1/3] 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 f7182440..286cf3ab 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 2f9457eb4565651460562a1d59a6aa34aaa457b6 Mon Sep 17 00:00:00 2001 From: Richard Bowman Date: Tue, 3 Mar 2026 12:47:13 +0000 Subject: [PATCH 2/3] 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 286cf3ab..caef56b3 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 fb961c37590b51603ec35380155a99fb27bb800e Mon Sep 17 00:00:00 2001 From: Richard Bowman Date: Tue, 3 Mar 2026 13:40:52 +0000 Subject: [PATCH 3/3] 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