Merge pull request #262 from labthings/readonly-metadata

Propagate read-only metadata to the Thing Description.

Author: rwb27

src/labthings_fastapi/client/__init__.py

@@ -180,7 +180,7 @@ def set_property(self, path: str, value: Any) -> None:
             ):
                 err_msg = detail[0].get("msg", "Unknown error")
 
-            raise ClientPropertyError(f"Failed to get property {path}: {err_msg}")
+            raise ClientPropertyError(f"Failed to set property {path}: {err_msg}")
 
     def invoke_action(self, path: str, **kwargs: Any) -> Any:
         r"""Invoke an action on the Thing.
@@ -358,18 +358,34 @@ def __get__(
             if obj is None:
                 return self
             return obj.get_property(self.name)
+    else:
+
+        def __get__(
+            self: PropertyClientDescriptor,
+            obj: Optional[ThingClient] = None,
+            _objtype: Optional[type[ThingClient]] = None,
+        ) -> Any:
+            raise ClientPropertyError("This property may not be read.")
+
+    __get__.__annotations__["return"] = model
+    P.__get__ = __get__  # type: ignore[attr-defined]
 
-        __get__.__annotations__["return"] = model
-        P.__get__ = __get__  # type: ignore[attr-defined]
+    # Set __set__ method based on whether writable
     if writeable:
 
         def __set__(
             self: PropertyClientDescriptor, obj: ThingClient, value: Any
         ) -> None:
             obj.set_property(self.name, value)
+    else:
+
+        def __set__(
+            self: PropertyClientDescriptor, obj: ThingClient, value: Any
+        ) -> None:
+            raise ClientPropertyError("This property may not be set.")
 
-        __set__.__annotations__["value"] = model
-        P.__set__ = __set__  # type: ignore[attr-defined]
+    __set__.__annotations__["value"] = model
+    P.__set__ = __set__  # type: ignore[attr-defined]
     if description:
         P.__doc__ = description
     return P()

src/labthings_fastapi/properties.py

@@ -467,6 +467,8 @@ def property_affordance(
             title=self.title,
             forms=forms,
             description=self.description,
+            readOnly=self.readonly,
+            writeOnly=False,  # write-only properties are not yet supported
         )
         # We merge the data schema with the property affordance (which subclasses the
         # DataSchema model) with the affordance second so its values take priority.

tests/test_property.py

@@ -344,3 +344,49 @@ class BadIntModel(pydantic.BaseModel):
     assert example.badprop == 3
     with pytest.raises(TypeError):
         _ = example.properties["badprop"].model_instance
+
+
+def test_readonly_metadata():
+    """Check read-only data propagates to the Thing Description."""
+
+    class Example(lt.Thing):
+        prop: int = lt.property(default=0)
+        ro_property: int = lt.property(default=0, readonly=True)
+
+        @lt.property
+        def ro_functional_property(self) -> int:
+            """This property should be read-only as there's no setter."""
+            return 42
+
+        @lt.property
+        def ro_functional_property_with_setter(self) -> int:
+            return 42
+
+        @ro_functional_property_with_setter.setter
+        def _set_ro_functional_property_with_setter(self, val: int) -> None:
+            pass
+
+        ro_functional_property_with_setter.readonly = True
+
+        @lt.property
+        def funcprop(self) -> int:
+            return 42
+
+        @funcprop.setter
+        def _set_funcprop(self, val: int) -> None:
+            pass
+
+    example = create_thing_without_server(Example)
+
+    td = example.thing_description()
+
+    # Check read-write properties are not read-only
+    for name in ["prop", "funcprop"]:
+        assert td.properties[name].readOnly is False
+
+    for name in [
+        "ro_property",
+        "ro_functional_property",
+        "ro_functional_property_with_setter",
+    ]:
+        assert td.properties[name].readOnly is True

tests/test_thing_client.py

@@ -1,11 +1,13 @@
-"""Test that Thing Client's can call actions and read properties."""
+"""Test that Thing Clients can call actions and read properties."""
 
 import re
 
 import pytest
 import labthings_fastapi as lt
 from fastapi.testclient import TestClient
 
+from labthings_fastapi.exceptions import ClientPropertyError, FailedToInvokeActionError
+
 
 class ThingToTest(lt.Thing):
     """A thing to be tested by using a ThingClient."""
@@ -59,94 +61,188 @@ def throw_value_error(self) -> None:
 
 
 @pytest.fixture
-def thing_client():
-    """Yield a test client connected to a ThingServer."""
+def thing_client_and_thing():
+    """Yield a test client connected to a ThingServer and the Thing itself."""
     server = lt.ThingServer({"test_thing": ThingToTest})
     with TestClient(server.app) as client:
-        yield lt.ThingClient.from_url("/test_thing/", client=client)
+        thing_client = lt.ThingClient.from_url("/test_thing/", client=client)
+        thing = server.things["test_thing"]
+        yield thing_client, thing
+
+
+@pytest.fixture
+def thing_client(thing_client_and_thing):
+    """Yield a test client connected to a ThingServer."""
+    return thing_client_and_thing[0]
 
 
-def test_reading_and_setting_properties(thing_client):
+def test_reading_and_setting_properties(thing_client_and_thing):
     """Test reading and setting properties."""
+    thing_client, thing = thing_client_and_thing
+
+    # Read the properties from the thing
     assert thing_client.int_prop == 1
     assert thing_client.float_prop == 0.1
     assert thing_client.str_prop == "foo"
 
+    # Update via thing client and check they change on the server and in the client
     thing_client.int_prop = 2
     thing_client.float_prop = 0.2
     thing_client.str_prop = "foo2"
 
+    # Check the server updated
+    assert thing.int_prop == 2
+    assert thing.float_prop == 0.2
+    assert thing.str_prop == "foo2"
+    # Check the client updated
     assert thing_client.int_prop == 2
     assert thing_client.float_prop == 0.2
     assert thing_client.str_prop == "foo2"
 
+    # Update them on the server side and read them again
+    thing.int_prop = 3
+    thing.float_prop = 0.3
+    thing.str_prop = "foo3"
+    assert thing_client.int_prop == 3
+    assert thing_client.float_prop == 0.3
+    assert thing_client.str_prop == "foo3"
+
     # Set a property that doesn't exist.
     err = "Failed to get property foobar: Not Found"
-    with pytest.raises(lt.exceptions.ClientPropertyError, match=err):
+    with pytest.raises(ClientPropertyError, match=err):
         thing_client.get_property("foobar")
 
     # Set a property with bad data type.
     err = (
-        "Failed to get property int_prop: Input should be a valid integer, unable to "
+        "Failed to set property int_prop: Input should be a valid integer, unable to "
         "parse string as an integer"
     )
-    with pytest.raises(lt.exceptions.ClientPropertyError, match=err):
+    with pytest.raises(ClientPropertyError, match=err):
         thing_client.int_prop = "Bad value!"
 
 
-def test_reading_and_not_setting_read_only_properties(thing_client):
+def test_reading_and_not_setting_read_only_properties(thing_client_and_thing):
     """Test reading read_only properties, but failing to set."""
+    thing_client, thing = thing_client_and_thing
     assert thing_client.int_prop_read_only == 1
     assert thing_client.float_prop_read_only == 0.1
     assert thing_client.str_prop_read_only == "foo"
 
-    with pytest.raises(lt.exceptions.ClientPropertyError, match="Method Not Allowed"):
+    with pytest.raises(ClientPropertyError, match="may not be set"):
         thing_client.int_prop_read_only = 2
-    with pytest.raises(lt.exceptions.ClientPropertyError, match="Method Not Allowed"):
+    with pytest.raises(ClientPropertyError, match="may not be set"):
         thing_client.float_prop_read_only = 0.2
-    with pytest.raises(lt.exceptions.ClientPropertyError, match="Method Not Allowed"):
+    with pytest.raises(ClientPropertyError, match="may not be set"):
         thing_client.str_prop_read_only = "foo2"
 
     assert thing_client.int_prop_read_only == 1
     assert thing_client.float_prop_read_only == 0.1
     assert thing_client.str_prop_read_only == "foo"
 
 
-def test_call_action(thing_client):
+def test_property_descriptor_errors(mocker):
+    """This checks that read/write-only properties raise an error when written/read.
+
+    Write only properties are not yet supported on the server side, so it's done with a
+    mocked up Thing Description.
+    """
+    thing_description = {
+        "title": "Example",
+        "properties": {
+            "readwrite": {
+                "title": "test",
+                "writeOnly": False,
+                "readOnly": False,
+                "type": "integer",
+                "forms": [],
+            },
+            "readonly": {
+                "title": "test",
+                "writeOnly": False,
+                "readOnly": True,
+                "type": "integer",
+                "forms": [],
+            },
+            "writeonly": {
+                "title": "test",
+                "writeOnly": True,
+                "readOnly": False,
+                "type": "integer",
+                "forms": [],
+            },
+        },
+        "actions": {},
+        "base": None,
+        "securityDefinitions": {"no_security": {}},
+        "security": "no_security",
+    }
+
+    # Create a client object
+    MyClient = lt.ThingClient.subclass_from_td(thing_description)
+    client = MyClient("/", mocker.Mock())
+    # Mock the underlying get/set functions so we don't need a server
+    mocker.patch.object(client, "get_property")
+    client.get_property.return_value = 42
+    mocker.patch.object(client, "set_property")
+
+    # Check which properties we can read
+    assert client.readwrite == 42
+    assert client.readonly == 42
+    with pytest.raises(ClientPropertyError, match="may not be read"):
+        _ = client.writeonly
+    assert client.get_property.call_count == 2
+
+    # The same check for writing
+    client.readwrite = 0
+    client.writeonly = 0
+    with pytest.raises(ClientPropertyError, match="may not be set"):
+        _ = client.readonly = 0
+    assert client.set_property.call_count == 2
+
+
+def test_call_action(thing_client_and_thing):
     """Test calling an action."""
+    thing_client, thing = thing_client_and_thing
     assert thing_client.int_prop == 1
     thing_client.increment()
     assert thing_client.int_prop == 2
+    assert thing.int_prop == 2
 
 
-def test_call_action_with_return(thing_client):
+def test_call_action_with_return(thing_client_and_thing):
     """Test calling an action with a return."""
+    thing_client, thing = thing_client_and_thing
     assert thing_client.int_prop == 1
     new_value = thing_client.increment_and_return()
     assert new_value == 2
     assert thing_client.int_prop == 2
+    assert thing.int_prop == 2
 
 
-def test_call_action_with_args(thing_client):
+def test_call_action_with_args(thing_client_and_thing):
     """Test calling an action."""
+    thing_client, thing = thing_client_and_thing
     assert thing_client.int_prop == 1
     thing_client.increment_by_input(value=5)
     assert thing_client.int_prop == 6
+    assert thing.int_prop == 6
 
 
-def test_call_action_with_args_and_return(thing_client):
+def test_call_action_with_args_and_return(thing_client_and_thing):
     """Test calling an action with a return."""
+    thing_client, thing = thing_client_and_thing
     assert thing_client.int_prop == 1
     new_value = thing_client.increment_by_input_and_return(value=5)
     assert new_value == 6
     assert thing_client.int_prop == 6
+    assert thing.int_prop == 6
 
 
 def test_call_action_wrong_arg(thing_client):
     """Test calling an action with wrong argument."""
     err = "Error when invoking action increment_by_input: 'value' - Field required"
 
-    with pytest.raises(lt.exceptions.FailedToInvokeActionError, match=err):
+    with pytest.raises(FailedToInvokeActionError, match=err):
         thing_client.increment_by_input(input=5)
 
 
@@ -156,7 +252,7 @@ def test_call_action_wrong_type(thing_client):
         "Error when invoking action increment_by_input: 'value' - Input should be a "
         "valid integer, unable to parse string as an integer"
     )
-    with pytest.raises(lt.exceptions.FailedToInvokeActionError, match=err):
+    with pytest.raises(FailedToInvokeActionError, match=err):
         thing_client.increment_by_input(value="foo")