From c3a9c67ad374989275daa25957ac72115cbca9a3 Mon Sep 17 00:00:00 2001 From: Richard Bowman Date: Tue, 3 Mar 2026 14:36:51 +0000 Subject: [PATCH 1/2] Validate properties in __set__. This uses the existing validation logic in `PropertyInfo` to check assignments made in Python as well as set operations over HTTP. --- src/labthings_fastapi/properties.py | 9 ++++++++- tests/test_properties.py | 27 +++++++++++++++++++++++++-- tests/test_property.py | 4 ++-- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py index 9c374a04..7cc07cb9 100644 --- a/src/labthings_fastapi/properties.py +++ b/src/labthings_fastapi/properties.py @@ -608,7 +608,8 @@ def __set__( :param value: the new value for the property. :param emit_changed_event: whether to emit a changed event. """ - obj.__dict__[self.name] = value + property_info = self.descriptor_info(obj) + obj.__dict__[self.name] = property_info.validate(value) if emit_changed_event: self.emit_changed_event(obj, value) @@ -811,6 +812,9 @@ def instance_get(self, obj: Owner) -> Value: def __set__(self, obj: Owner, value: Value) -> None: """Set the value of the property. + This will validate the value against the property's model, and an error + will be raised if the value is not valid. + :param obj: the `.Thing` on which the attribute is accessed. :param value: the value of the property. @@ -818,6 +822,9 @@ def __set__(self, obj: Owner, value: Value) -> None: """ if self.fset is None: raise ReadOnlyPropertyError(f"Property {self.name} of {obj} has no setter.") + property_info = self.descriptor_info(obj) + value = property_info.validate(value) + self.fset(obj, value) diff --git a/tests/test_properties.py b/tests/test_properties.py index f7182440..6012f717 100644 --- a/tests/test_properties.py +++ b/tests/test_properties.py @@ -378,21 +378,44 @@ def test_setting_without_event_loop(): @pytest.mark.parametrize("prop_info", CONSTRAINED_PROPS) -def test_constrained_properties(prop_info): - """Test that constraints on property values generate correct models.""" +def test_constrained_properties(prop_info, mocker): + """Test that constraints on property values generate correct models. + + This also tests the `validate` method and checks validation happens + on assignment to the property in Python. Further checks over http + are made in later tests. + """ prop = prop_info.prop assert prop.value_type is prop_info.value_type m = prop.model assert issubclass(m, RootModel) + mock_thing = mocker.Mock(spec=PropertyTestThing) + mock_thing._thing_server_interface = mocker.Mock() + descriptorinfo = prop.descriptor_info(mock_thing) + assert isinstance(descriptorinfo, PropertyInfo) for ann in prop_info.constraints: assert any(meta == ann for meta in m.model_fields["root"].metadata) for valid in prop_info.valid_values: + # Check the model can be created instance = m(root=valid) + # Check the value passes through the model validated = instance.model_dump() assert validated == valid or validated is valid # `is` for NaN + # Check the descriptorinfo object also validates + # (this is what we get from thing.properties["name"]) + validated = descriptorinfo.validate(valid) + assert validated == valid or validated is valid # `is` for NaN + # Check that assignment works + prop.__set__(mock_thing, valid) + validated = prop.__get__(mock_thing) + assert validated == valid or validated is valid # `is` for NaN for invalid in prop_info.invalid_values: with pytest.raises(ValidationError): _ = m(root=invalid) + with pytest.raises(ValidationError): + descriptorinfo.validate(invalid) + with pytest.raises(ValidationError): + prop.__set__(mock_thing, invalid) def convert_inf_nan(value): diff --git a/tests/test_property.py b/tests/test_property.py index fe285dbd..fdc2abca 100644 --- a/tests/test_property.py +++ b/tests/test_property.py @@ -365,8 +365,8 @@ class BadIntModel(pydantic.BaseModel): # Check that a broken `_model` raises the right error # See above for where we manually set badprop._model to something that's # not a rootmodel. - example.badprop = 3 - assert example.badprop == 3 + with pytest.raises(TypeError): + example.badprop = 3 # Validation will fail here because of the bad model. with pytest.raises(TypeError): _ = example.properties["badprop"].model_instance with pytest.raises(TypeError): From 5a37ae63722c76e0721cce5704c22051d9387ece Mon Sep 17 00:00:00 2001 From: Richard Bowman Date: Tue, 3 Mar 2026 14:49:04 +0000 Subject: [PATCH 2/2] Add more tests for property validation logic. --- tests/test_property.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_property.py b/tests/test_property.py index fdc2abca..88460253 100644 --- a/tests/test_property.py +++ b/tests/test_property.py @@ -391,12 +391,24 @@ class BadIntModel(pydantic.BaseModel): # Check validation for a model modelprop = example.properties["modelprop"] assert modelprop.validate(MyModel(a=3, b="four")) == MyModel(a=3, b="four") + # Check that a valid model passes through unchanged: this should indicate that + # we're not unnecessarily re-validating already-valid models. m = MyModel(a=3, b="four") assert modelprop.validate(m) is m assert modelprop.validate({"a": 5, "b": "six"}) == MyModel(a=5, b="six") for invalid in [{"c": 5}, (4, "f"), None]: with pytest.raises(pydantic.ValidationError): modelprop.validate(invalid) + # Check that an invalid model doesn't get re-validated. This is intended behaviour: + # it is another test that we're not unnecessarily re-validating a model that + # should already have been validated when it was created. + # Creating models with `model_construct` intentionally allows invalid models: + # if this is used downstream, the downstream code should accept responsibility! + bad_m = MyModel.model_construct(a="not an int", b=6) + assert modelprop.validate(bad_m) is bad_m + with pytest.raises(pydantic.ValidationError): + # Check that passing the same data in as a dict fails validation. + modelprop.validate(bad_m.model_dump()) # Check again for an odd rootmodel rootmodelprop = example.properties["rootmodelprop"] @@ -409,6 +421,11 @@ class BadIntModel(pydantic.BaseModel): for invalid in ["seven", {"root": None}, 14.5, pydantic.RootModel[int](root=0)]: with pytest.raises(pydantic.ValidationError): modelprop.validate(invalid) + # Tty constructing a model with an invalid root value, skipping validation + invalid_model = rootmodelprop.model.model_construct(invalid) + # The RootModel gets re-validated, in contrast to the BaseModel above. + with pytest.raises(pydantic.ValidationError): + assert modelprop.validate(invalid_model) == invalid def test_readonly_metadata():