Skip to content

Commit 5b74441

Browse files
authored
Merge pull request #173 from labthings/read-only-properties-in-directhingclient
Fix read-only properties in `DirectThingClient`
2 parents 9814eaa + 22fa40c commit 5b74441

File tree

5 files changed

+175
-19
lines changed

5 files changed

+175
-19
lines changed

dev-requirements.txt

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,6 @@ email-validator==2.2.0
5555
# via
5656
# fastapi
5757
# pydantic
58-
exceptiongroup==1.3.0
59-
# via
60-
# anyio
61-
# pytest
6258
fastapi==0.116.1
6359
# via labthings-fastapi (pyproject.toml)
6460
fastapi-cli==0.0.8
@@ -170,8 +166,11 @@ pytest==7.4.4
170166
# via
171167
# labthings-fastapi (pyproject.toml)
172168
# pytest-cov
169+
# pytest-mock
173170
pytest-cov==6.2.1
174171
# via labthings-fastapi (pyproject.toml)
172+
pytest-mock==3.14.1
173+
# via labthings-fastapi (pyproject.toml)
175174
python-dotenv==1.1.1
176175
# via
177176
# pydantic-settings
@@ -242,14 +241,6 @@ sphinxcontrib-serializinghtml==2.0.0
242241
# via sphinx
243242
starlette==0.47.1
244243
# via fastapi
245-
tomli==2.2.1
246-
# via
247-
# coverage
248-
# flake8-pyproject
249-
# mypy
250-
# pydoclint
251-
# pytest
252-
# sphinx
253244
typer==0.16.0
254245
# via
255246
# fastapi-cli
@@ -260,20 +251,16 @@ typing-extensions==4.14.1
260251
# via
261252
# labthings-fastapi (pyproject.toml)
262253
# anyio
263-
# astroid
264-
# exceptiongroup
265254
# fastapi
266255
# mypy
267256
# pydantic
268257
# pydantic-core
269258
# pydantic-extra-types
270259
# referencing
271-
# rich
272260
# rich-toolkit
273261
# starlette
274262
# typer
275263
# typing-inspection
276-
# uvicorn
277264
typing-inspection==0.4.1
278265
# via pydantic-settings
279266
ujson==5.10.0

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ dependencies = [
2727
dev = [
2828
"pytest>=7.4.0, <8",
2929
"pytest-cov",
30+
"pytest-mock",
3031
"mypy>=1.6.1, <2",
3132
"ruff>=0.1.3",
3233
"types-jsonschema",

src/labthings_fastapi/client/in_server.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,18 @@ def __get__(
124124
def __set__(self, obj: DirectThingClient, value: Any):
125125
setattr(obj._wrapped_thing, self.name, value)
126126

127+
def set_readonly(self, obj: DirectThingClient, value: Any):
128+
raise AttributeError("This property is read-only.")
129+
127130
if readable:
128131
__get__.__annotations__["return"] = model
129132
P.__get__ = __get__ # type: ignore[attr-defined]
130133
if writeable:
131134
__set__.__annotations__["value"] = model
132135
P.__set__ = __set__ # type: ignore[attr-defined]
136+
else:
137+
set_readonly.__annotations__["value"] = model
138+
P.__set__ = set_readonly # type: ignore[attr-defined]
133139
if description:
134140
P.__doc__ = description
135141
return P()

tests/test_directthingclient.py

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
"""Test the DirectThingClient class.
2+
3+
This module tests inter-Thing interactions. It does not yet test exhaustively,
4+
and has been added primarily to fix #165.
5+
"""
6+
7+
from fastapi.testclient import TestClient
8+
import pytest
9+
import labthings_fastapi as lt
10+
from labthings_fastapi.deps import DirectThingClient, direct_thing_client_class
11+
from .temp_client import poll_task
12+
13+
14+
class Counter(lt.Thing):
15+
ACTION_ONE_RESULT = "Action one result!"
16+
17+
@lt.thing_action
18+
def increment(self) -> str:
19+
"""An action that takes no arguments"""
20+
return self.increment_internal()
21+
22+
def increment_internal(self) -> str:
23+
"""An action that increments the counter."""
24+
self.count += self.step
25+
return self.ACTION_ONE_RESULT
26+
27+
step: int = lt.property(default=1)
28+
count: int = lt.property(default=0, readonly=True)
29+
30+
31+
@pytest.fixture
32+
def counter_client(mocker) -> DirectThingClient:
33+
r"""Instantiate a Counter and wrap it in a DirectThingClient.
34+
35+
In order to make this work without a server, ``DirectThingClient`` is
36+
subclassed, and ``__init__`` is overridden.
37+
This could be done with ``mocker`` but it would be more verbose and
38+
less clear.
39+
40+
:param mocker: the mocker test fixture from ``pytest-mock``\ .
41+
:returns: a ``DirectThingClient`` subclass wrapping a ``Counter``\ .
42+
"""
43+
counter = Counter()
44+
counter._labthings_blocking_portal = mocker.Mock(["start_task_soon"])
45+
46+
CounterClient = direct_thing_client_class(Counter, "/counter")
47+
48+
class StandaloneCounterClient(CounterClient):
49+
def __init__(self, wrapped):
50+
self._dependencies = {}
51+
self._request = mocker.Mock()
52+
self._wrapped_thing = wrapped
53+
54+
return StandaloneCounterClient(counter)
55+
56+
57+
CounterDep = lt.deps.direct_thing_client_dependency(Counter, "/counter/")
58+
RawCounterDep = lt.deps.raw_thing_dependency(Counter)
59+
60+
61+
class Controller(lt.Thing):
62+
"""Controller is used to test a real DirectThingClient in a server.
63+
64+
This is used by ``test_directthingclient_in_server`` to verify the
65+
client works as expected when created normally, rather than by mocking
66+
the server.
67+
"""
68+
69+
@lt.thing_action
70+
def count_in_twos(self, counter: CounterDep) -> str:
71+
"""An action that needs a Counter and uses its affordances.
72+
73+
This only uses methods that are part of the HTTP API, so all
74+
of these commands should work.
75+
"""
76+
counter.step = 2
77+
assert counter.count == 0
78+
counter.increment()
79+
assert counter.count == 2
80+
return "success"
81+
82+
@lt.thing_action
83+
def count_internal(self, counter: CounterDep) -> str:
84+
"""An action that tries to access local-only attributes.
85+
86+
This previously used `pytest.raises` but that caused the test
87+
to hang, most likely because this will run in a background thread.
88+
"""
89+
try:
90+
counter.increment_internal()
91+
raise AssertionError("Expected error was not raised!")
92+
except AttributeError:
93+
# pytest.raises seems to hang.
94+
pass
95+
try:
96+
counter.count = 4
97+
raise AssertionError("Expected error was not raised!")
98+
except AttributeError:
99+
# pytest.raises seems to hang.
100+
pass
101+
return "success"
102+
103+
@lt.thing_action
104+
def count_raw(self, counter: RawCounterDep) -> str:
105+
"""Increment the counter using a method that is not an Action."""
106+
counter.count = 0
107+
counter.step = -1
108+
counter.increment_internal()
109+
assert counter.count == -1
110+
return "success"
111+
112+
113+
def test_readwrite_property(counter_client):
114+
"""Test a read/write property works as expected."""
115+
counter_client.step = 2
116+
assert counter_client.step == 2
117+
118+
119+
def test_readonly_property(counter_client):
120+
"""Test a read/write property works as expected."""
121+
assert counter_client.count == 0
122+
with pytest.raises(AttributeError):
123+
counter_client.count = 10
124+
125+
126+
def test_action(counter_client):
127+
"""Test we can run an action."""
128+
assert counter_client.count == 0
129+
counter_client.increment()
130+
assert counter_client.count == 1
131+
132+
133+
def test_method(counter_client):
134+
"""Methods that are not decorated as actions should be missing."""
135+
with pytest.raises(AttributeError):
136+
counter_client.increment_internal()
137+
# Just to double-check the line above isn't a typo...
138+
counter_client._wrapped_thing.increment_internal()
139+
140+
141+
@pytest.mark.parametrize("action", ["count_in_twos", "count_internal", "count_raw"])
142+
def test_directthingclient_in_server(action):
143+
"""Test that a Thing can depend on another Thing
144+
145+
This uses the internal thing client mechanism.
146+
"""
147+
server = lt.ThingServer()
148+
server.add_thing(Counter(), "/counter")
149+
server.add_thing(Controller(), "/controller")
150+
with TestClient(server.app) as client:
151+
r = client.post(f"/controller/{action}")
152+
invocation = poll_task(client, r.json())
153+
assert invocation["status"] == "completed"
154+
assert invocation["output"] == "success"

tests/test_settings.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,11 @@ def set_localonly_boolsetting(
112112
client: TestThingClientDep,
113113
val: bool,
114114
):
115-
"""Attempt to set a setting with a DirectThingClient."""
115+
"""Attempt to set a setting with a DirectThingClient.
116+
117+
This should fail with an error, as it's not writeable from a
118+
DirectThingClient.
119+
"""
116120
client.localonly_boolsetting = val
117121

118122
@lt.thing_action
@@ -130,7 +134,11 @@ def directly_set_localonly_boolsetting(
130134
test_thing: TestThingDep,
131135
val: bool,
132136
):
133-
"""Attempt to set a setting directly."""
137+
"""Attempt to set a setting directly.
138+
139+
This should work, even though the setting is read-only from clients.
140+
Using a raw thing dependency bypasses that restriction.
141+
"""
134142
test_thing.localonly_boolsetting = val
135143

136144

@@ -291,7 +299,7 @@ def test_readonly_setting(thing, client_thing, server, endpoint, value, method):
291299
# The setting is not changed (that's tested later), but the action
292300
# does complete. It should fail with an error, but this is expected
293301
# behaviour - see #165.
294-
assert invocation["status"] == "completed"
302+
assert invocation["status"] == "error"
295303

296304
# Check the setting hasn't changed over HTTP
297305
r = client.get(f"/thing/{endpoint}")

0 commit comments

Comments
 (0)