From a0dac41b2f261ce9d59907b2747d317c3f096405 Mon Sep 17 00:00:00 2001 From: Richard Bowman Date: Thu, 23 Nov 2023 10:21:23 +0000 Subject: [PATCH] Test that dependencies only get re-used if they are identical --- src/labthings_fastapi/client/in_server.py | 43 +++++++++++---------- src/labthings_fastapi/dependencies/thing.py | 5 ++- tests/test_thing_dependencies.py | 24 ++++++++++++ 3 files changed, 49 insertions(+), 23 deletions(-) diff --git a/src/labthings_fastapi/client/in_server.py b/src/labthings_fastapi/client/in_server.py index bbdd936..dbdb597 100644 --- a/src/labthings_fastapi/client/in_server.py +++ b/src/labthings_fastapi/client/in_server.py @@ -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: @@ -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): @@ -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, } @@ -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 diff --git a/src/labthings_fastapi/dependencies/thing.py b/src/labthings_fastapi/dependencies/thing.py index db9ff64..37c881e 100644 --- a/src/labthings_fastapi/dependencies/thing.py +++ b/src/labthings_fastapi/dependencies/thing.py @@ -1,5 +1,5 @@ from __future__ import annotations -from typing import Annotated, TypeVar +from typing import Annotated, Optional, TypeVar from fastapi import Depends @@ -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] diff --git a/tests/test_thing_dependencies.py b/tests/test_thing_dependencies.py index 6abb9c1..6f483e9 100644 --- a/tests/test_thing_dependencies.py +++ b/tests/test_thing_dependencies.py @@ -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 @@ -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