Skip to content

Commit

Permalink
Test that dependencies only get re-used if they are identical
Browse files Browse the repository at this point in the history
  • Loading branch information
rwb27 committed Nov 23, 2023
1 parent e2942f0 commit a0dac41
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 23 deletions.
43 changes: 22 additions & 21 deletions src/labthings_fastapi/client/in_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,24 +60,22 @@ class P(PropertyClientDescriptor):
type = model
path = property_path or property_name

if readable:

def __get__(
self,
obj: Optional[DirectThingClient] = None,
_objtype: Optional[type[DirectThingClient]] = None,
):
if obj is None:
return self
return getattr(obj._wrapped_thing, self.name)
def __get__(
self,
obj: Optional[DirectThingClient] = None,
_objtype: Optional[type[DirectThingClient]] = None,
):
if obj is None:
return self
return getattr(obj._wrapped_thing, self.name)

def __set__(self, obj: DirectThingClient, value: Any):
setattr(obj, self.name, value)

if readable:
__get__.__annotations__["return"] = model
P.__get__ = __get__ # type: ignore[attr-defined]
if writeable:

def __set__(self, obj: DirectThingClient, value: Any):
setattr(obj, self.name, value)

__set__.__annotations__["value"] = model
P.__set__ = __set__ # type: ignore[attr-defined]
if description:
Expand All @@ -91,7 +89,10 @@ def add_action(
name: str,
action: ActionDescriptor,
) -> None:
"""Add an action to a DirectThingClient subclass"""
"""Generates an action method and adds it to an attrs dict
FastAPI Dependencies are appended to the `dependencies` list.
"""

@wraps(action.func)
def action_method(self, **kwargs):
Expand Down Expand Up @@ -156,7 +157,6 @@ def init_proxy(self, request: Request, **dependencies: Mapping[str, Any]):
client_attrs = {
"thing_class": thing_class,
"thing_path": thing_path,
"_dependency_params": [],
"__doc__": f"A client for {thing_class} at {thing_path}",
"__init__": init_proxy,
}
Expand All @@ -165,15 +165,16 @@ def init_proxy(self, request: Request, **dependencies: Mapping[str, Any]):
if isinstance(item, PropertyDescriptor):
# TODO: What about properties that don't use descriptors? Fall back to http?
add_property(client_attrs, name, item)
elif isinstance(item, ActionDescriptor) and (
actions is None or name in actions
):
add_action(client_attrs, dependencies, name, item)
elif isinstance(item, ActionDescriptor):
if actions is None or name in actions:
add_action(client_attrs, dependencies, name, item)
else:
continue # Ignore actions that aren't in the list
else:
for affordance in ["property", "action", "event"]:
if hasattr(item, f"{affordance}_affordance"):
logging.warning(
f"DirectThingClient doesn't support custom afforcances, "
f"DirectThingClient doesn't support custom affordances, "
f"ignoring {name}"
)
# This block of code makes dependencies show up in __init__ so
Expand Down
5 changes: 3 additions & 2 deletions src/labthings_fastapi/dependencies/thing.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from __future__ import annotations
from typing import Annotated, TypeVar
from typing import Annotated, Optional, TypeVar

from fastapi import Depends

Expand All @@ -13,7 +13,8 @@
def direct_thing_client_dependency(
thing_class: type[Thing],
thing_path: str,
actions: Optional[list[str]] = None,
) -> type[Thing]:
"""A type annotation that causes FastAPI to supply a direct thing client"""
C = direct_thing_client_class(thing_class, thing_path)
C = direct_thing_client_class(thing_class, thing_path, actions=actions)
return Annotated[C, Depends()] # type: ignore[return-value]
24 changes: 24 additions & 0 deletions tests/test_thing_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import inspect
from fastapi.testclient import TestClient
from fastapi import Request
import pytest
from labthings_fastapi.thing_server import ThingServer
from temp_client import poll_task
from labthings_fastapi.thing import Thing
Expand Down Expand Up @@ -135,6 +136,29 @@ def action_two(self, thing_one: ThingOneDep) -> str:
assert invocation["output"] == ThingOne.ACTION_ONE_RESULT


def test_conflicting_dependencies():
"""Dependencies are stored by argument name, and can't be duplicated.
We check here that an error is raised if the same argument name is used
for two different dependencies.
This also checks that dependencies on the same class but different
actions are recognised as "different".
"""
ThingTwoDepNoActions = direct_thing_client_dependency(ThingTwo, "/thing_two/", [])

class ThingFour(Thing):
@thing_action
def action_four(self, thing_two: ThingTwoDepNoActions) -> str:
return str(thing_two)

@thing_action
def action_five(self, thing_two: ThingTwoDep) -> str:
return thing_two.action_two()

with pytest.raises(ValueError):
direct_thing_client_dependency(ThingFour, "/thing_four/")


def check_request():
"""Check that the `Request` object has the same `app` as the server
Expand Down

0 comments on commit a0dac41

Please sign in to comment.