Add more detailed tests of settings

This has uncovered a bug in DirectThingClient (#165).
For now, the test reflects current behaviour, but it will need an update when #165 is fixed.

Author: rwb27

tests/__init__.py

@@ -8,6 +8,7 @@
 from fastapi.testclient import TestClient
 
 import labthings_fastapi as lt
+from .temp_client import poll_task
 
 
 class TestThing(lt.Thing):
@@ -17,6 +18,7 @@ def __init__(self) -> None:
         super().__init__()
         # Initialize functional settings with default values
         self._floatsetting: float = 1.0
+        self._localonlysetting = "Local-only default."
 
     boolsetting: bool = lt.setting(default=False)
     "A boolean setting"
@@ -37,15 +39,42 @@ def floatsetting(self, value: float):
         self._floatsetting = value
 
     @lt.setting
-    def readonlysetting(self) -> str:
-        """A read-only setting."""
-        return "This is read-only"
+    def localonlysetting(self) -> str:
+        """A setting that is not writeable from HTTP clients or DirectThingClients.
 
-    @readonlysetting.setter
-    def readonlysetting(self, value: str):
-        pass
+        This setting has a setter, so may be written to from this Thing, or
+        when settings are loaded. However, it's marked as read-only later, which
+        means HTTP clients or DirectThingClient subclasses can't write to it.
+        """
+        return self._localonlysetting
 
-    readonlysetting.readonly = True
+    @localonlysetting.setter
+    def localonlysetting(self, value: str):
+        self._localonlysetting = value
+
+    localonlysetting.readonly = True
+
+    localonly_boolsetting: bool = lt.setting(default=False, readonly=True)
+
+    @lt.thing_action
+    def write_localonly_setting(self, value: str) -> None:
+        """Change the value of the local-only setting.
+
+        This is allowed - the setting is only read-only for code running
+        over HTTP or via a DirectThingClient. By using this action, we can
+        check it's writeable for local code.
+        """
+        self.localonlysetting = value
+
+    @lt.thing_action
+    def toggle_localonly_boolsetting(self) -> None:
+        """Toggle the local-only bool setting.
+
+        Settings with `readonly=True` are read-only for client code via HTTP
+        or a DirectThingClient. This action checks they are still writeable
+        from within the Thing.
+        """
+        self.localonly_boolsetting = not self.localonly_boolsetting
 
     @lt.thing_action
     def toggle_boolsetting(self):
@@ -57,13 +86,66 @@ def toggle_boolsetting_from_thread(self):
         t.start()
 
 
+TestThingClientDep = lt.deps.direct_thing_client_dependency(TestThing, "/thing/")
+TestThingDep = lt.deps.raw_thing_dependency(TestThing)
+
+
+class ClientThing(lt.Thing):
+    """This Thing attempts to set read-only settings on TestThing.
+
+    Read-only settings may not be set by DirectThingClient wrappers,
+    which is what this class tests.
+    """
+
+    @lt.thing_action
+    def set_localonlysetting(
+        self,
+        client: TestThingClientDep,
+        val: str,
+    ):
+        """Attempt to set a setting with a DirectThingClient."""
+        client.localonlysetting = val
+
+    @lt.thing_action
+    def set_localonly_boolsetting(
+        self,
+        client: TestThingClientDep,
+        val: bool,
+    ):
+        """Attempt to set a setting with a DirectThingClient."""
+        client.localonly_boolsetting = val
+
+    @lt.thing_action
+    def directly_set_localonlysetting(
+        self,
+        test_thing: TestThingDep,
+        val: str,
+    ):
+        """Attempt to set a setting directly."""
+        test_thing.localonlysetting = val
+
+    @lt.thing_action
+    def directly_set_localonly_boolsetting(
+        self,
+        test_thing: TestThingDep,
+        val: bool,
+    ):
+        """Attempt to set a setting directly."""
+        test_thing.localonly_boolsetting = val
+
+
 def _get_setting_file(server, thingpath):
     path = os.path.join(server.settings_folder, thingpath.lstrip("/"), "settings.json")
     return os.path.normpath(path)
 
 
 def _settings_dict(
-    boolsetting=False, floatsetting=1.0, stringsetting="foo", dictsetting=None
+    boolsetting=False,
+    floatsetting=1.0,
+    stringsetting="foo",
+    dictsetting=None,
+    localonlysetting="Local-only default.",
+    localonly_boolsetting=False,
 ):
     """Return the expected settings dictionary
 
@@ -76,7 +158,8 @@ def _settings_dict(
         "floatsetting": floatsetting,
         "stringsetting": stringsetting,
         "dictsetting": dictsetting,
-        "readonlysetting": "This is read-only",
+        "localonlysetting": localonlysetting,
+        "localonly_boolsetting": localonly_boolsetting,
     }
 
 
@@ -85,6 +168,11 @@ def thing():
     return TestThing()
 
 
+@pytest.fixture
+def client_thing():
+    return ClientThing()
+
+
 @pytest.fixture
 def server():
     with tempfile.TemporaryDirectory() as tempdir:
@@ -98,7 +186,7 @@ def test_setting_available(thing):
     assert not thing.boolsetting
     assert thing.stringsetting == "foo"
     assert thing.floatsetting == 1.0
-    assert thing.readonlysetting == "This is read-only"
+    assert thing.localonlysetting == "Local-only default."
 
 
 def test_functional_settings_save(thing, server):
@@ -153,20 +241,77 @@ def test_data_settings_save(thing, server):
             assert json.load(file_obj) == _settings_dict(boolsetting=True)
 
 
-def test_readonly_setting(thing, server):
-    """Check read-only settings cannot be set remotely."""
+@pytest.mark.parametrize(
+    ("endpoint", "value"),
+    [
+        ("localonlysetting", "Other value"),
+        ("localonly_boolsetting", True),
+    ],
+)
+@pytest.mark.parametrize(
+    "method",
+    ["http", "direct_thing_client", "direct"],
+)
+def test_readonly_setting(thing, client_thing, server, endpoint, value, method):
+    """Check read-only functional settings cannot be set remotely.
+
+    Functional settings must always have a setter, and will be
+    writeable from within the Thing. However, they should not
+    be settable remotely or via a DirectThingClient.
+
+    This test is a bit complicated, but it checks both a
+    `.FunctionalSetting` and a `.DataSetting` via all three
+    methods: HTTP, DirectThingClient, and directly on the Thing.
+    Only the last method should work.
+
+    The test is parametrized so it will run 6 times, trying one
+    block of code inside the ``with`` block each time.
+    """
     setting_file = _get_setting_file(server, "/thing")
     server.add_thing(thing, "/thing")
+    server.add_thing(client_thing, "/client_thing")
     # No setting file created when first added
     assert not os.path.isfile(setting_file)
+
+    # Access it over "HTTP" with a TestClient
+    # This doesn't actually serve over the network but will use
+    # all the same codepaths.
     with TestClient(server.app) as client:
-        # Check we can read it over HTTP
-        r = client.get("/thing/readonlysetting")
-        assert r.json() == "This is read-only"
+        if method == "http":
+            # Attempt to set read-only setting
+            r = client.put(f"/thing/{endpoint}", json=value)
+            assert r.status_code == 405
+
+        if method == "direct_thing_client":
+            # Attempt to set read-only setting via a DirectThingClient
+            r = client.post(f"/client_thing/set_{endpoint}", json={"val": value})
+            assert r.status_code == 201
+            invocation = poll_task(client, r.json())
+            # The setting is not changed (that's tested later), but the action
+            # does complete. It should fail with an error, but this is expected
+            # behaviour - see #165.
+            assert invocation["status"] == "completed"
+
+        # Check the setting hasn't changed over HTTP
+        r = client.get(f"/thing/{endpoint}")
+        assert r.json() == _settings_dict()[endpoint]
         assert r.status_code == 200
-        # Attempt to set read-only setting
-        r = client.put("/thing/readonlysetting", json="new value")
-        assert r.status_code == 405
+
+        if method == "direct":
+            # Actually set read-only setting via raw_thing_dependency
+            r = client.post(
+                f"/client_thing/directly_set_{endpoint}", json={"val": value}
+            )
+            invocation = poll_task(client, r.json())
+            assert invocation["status"] == "completed"
+
+    if method == "direct":
+        # Setting directly should succeed, so the file should exist.
+        with open(setting_file, "r", encoding="utf-8") as file_obj:
+            # Check settings on file match expected dictionary
+            assert json.load(file_obj) == _settings_dict(**{endpoint: value})
+    else:
+        # Other methods fail, so there should be no file here.
         assert not os.path.isfile(setting_file)  # No file created