From f2baa74cdc01b29a95d584dec857945cce23caf9 Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Mon, 4 Mar 2024 15:10:43 -0500 Subject: [PATCH] wip (fixed tests) --- addon_service/tests/_factories.py | 2 + .../test_authorized_storage_account.py | 3 +- .../test_configured_storage_addon.py | 3 +- .../test_external_storage_service.py | 25 ++----- addon_toolkit/__init__.py | 2 + addon_toolkit/interface.py | 4 +- addon_toolkit/operation.py | 60 +++++++++++----- addon_toolkit/storage.py | 8 +-- addon_toolkit/tests/test_addon_interface.py | 69 +++++++++++++------ 9 files changed, 106 insertions(+), 70 deletions(-) diff --git a/addon_service/tests/_factories.py b/addon_service/tests/_factories.py index d08e3dd3..d8cb7f00 100644 --- a/addon_service/tests/_factories.py +++ b/addon_service/tests/_factories.py @@ -2,6 +2,7 @@ from factory.django import DjangoModelFactory from addon_service import models as db +from addon_service.addon_imp.known import IntAddonImp from addon_service.common.capability import IntStorageCapability @@ -52,6 +53,7 @@ class Meta: max_upload_mb = factory.Faker("pyint") auth_uri = factory.Sequence(lambda n: f"http://auth.example/{n}") credentials_issuer = factory.SubFactory(CredentialsIssuerFactory) + int_addon_imp = IntAddonImp.blarg class AuthorizedStorageAccountFactory(DjangoModelFactory): diff --git a/addon_service/tests/test_by_type/test_authorized_storage_account.py b/addon_service/tests/test_by_type/test_authorized_storage_account.py index f8518e32..d58f91f5 100644 --- a/addon_service/tests/test_by_type/test_authorized_storage_account.py +++ b/addon_service/tests/test_by_type/test_authorized_storage_account.py @@ -39,7 +39,7 @@ def _related_path(self, related_field): }, ) - def test_get(self): + def test_get_detail(self): _resp = self.client.get(self._detail_path) self.assertEqual(_resp.status_code, HTTPStatus.OK) self.assertEqual( @@ -123,6 +123,7 @@ def test_get(self): "account_owner", "external_storage_service", "configured_storage_addons", + "authorized_operations", }, ) diff --git a/addon_service/tests/test_by_type/test_configured_storage_addon.py b/addon_service/tests/test_by_type/test_configured_storage_addon.py index 8acadd0a..0daf52fb 100644 --- a/addon_service/tests/test_by_type/test_configured_storage_addon.py +++ b/addon_service/tests/test_by_type/test_configured_storage_addon.py @@ -107,8 +107,9 @@ def test_get(self): self.assertEqual( set(_content["data"]["relationships"].keys()), { - "base_account", "authorized_resource", + "base_account", + "connected_operations", }, ) diff --git a/addon_service/tests/test_by_type/test_external_storage_service.py b/addon_service/tests/test_by_type/test_external_storage_service.py index 4d258b47..59acd339 100644 --- a/addon_service/tests/test_by_type/test_external_storage_service.py +++ b/addon_service/tests/test_by_type/test_external_storage_service.py @@ -114,7 +114,7 @@ def test_get(self): self.assertEqual( set(_content["data"]["relationships"].keys()), { - "authorized_storage_accounts", + "addon_imp", }, ) @@ -141,28 +141,11 @@ def setUpTestData(cls): {"get": "retrieve_related"}, ) - def test_get_related__empty(self): + def test_get_related(self): _resp = self._related_view( get_test_request(), pk=self._ess.pk, - related_field="authorized_storage_accounts", + related_field="addon_imp", ) self.assertEqual(_resp.status_code, HTTPStatus.OK) - self.assertEqual(_resp.data, []) - - def test_get_related__several(self): - _accounts = _factories.AuthorizedStorageAccountFactory.create_batch( - size=5, - external_storage_service=self._ess, - ) - _resp = self._related_view( - get_test_request(), - pk=self._ess.pk, - related_field="authorized_storage_accounts", - ) - self.assertEqual(_resp.status_code, HTTPStatus.OK) - _content = json.loads(_resp.rendered_content) - self.assertEqual( - {_datum["id"] for _datum in _content["data"]}, - {str(_account.pk) for _account in _accounts}, - ) + self.assertEqual(_resp.data["name"], self._ess.addon_imp.name) diff --git a/addon_toolkit/__init__.py b/addon_toolkit/__init__.py index 6137f262..c4656e7a 100644 --- a/addon_toolkit/__init__.py +++ b/addon_toolkit/__init__.py @@ -5,6 +5,7 @@ ) from .operation import ( AddonOperationDeclaration, + RedirectResult, immediate_operation, redirect_operation, ) @@ -14,6 +15,7 @@ "AddonInterfaceDeclaration", "AddonOperationDeclaration", "AddonOperationImplementation", + "RedirectResult", "addon_interface", "immediate_operation", "redirect_operation", diff --git a/addon_toolkit/interface.py b/addon_toolkit/interface.py index 9b6bf035..f6d473b4 100644 --- a/addon_toolkit/interface.py +++ b/addon_toolkit/interface.py @@ -19,7 +19,7 @@ _logger = logging.getLogger(__name__) -@dataclasses.dataclass +@dataclasses.dataclass(frozen=True) class AddonInterfaceDeclaration: """dataclass for the operations declared on a class decorated with `addon_interface`""" @@ -27,11 +27,13 @@ class AddonInterfaceDeclaration: capability_enum: type[enum.Enum] method_name_by_op: dict[AddonOperationDeclaration, str] = dataclasses.field( default_factory=dict, + compare=False, ) ops_by_capability: dict[ enum.Enum, set[AddonOperationDeclaration] ] = dataclasses.field( default_factory=dict, + compare=False, ) @classmethod diff --git a/addon_toolkit/operation.py b/addon_toolkit/operation.py index 3d5a9ed2..eed38cbd 100644 --- a/addon_toolkit/operation.py +++ b/addon_toolkit/operation.py @@ -1,6 +1,11 @@ import dataclasses import enum -from typing import Callable +import inspect +from http import HTTPMethod +from typing import ( + Any, + Callable, +) from .declarator import Declarator @@ -30,8 +35,8 @@ class AddonOperationDeclaration: operation_type: AddonOperationType capability: enum.Enum operation_fn: Callable # the decorated function - params_dataclass: type = dataclasses.field(init=False) - success_dataclass: type = dataclasses.field(init=False) + return_dataclass: type = dataclasses.field(default=type(None), compare=False) + params_dataclass: type = dataclasses.field(init=False, compare=False) @classmethod def for_function(self, fn: Callable) -> "AddonOperationDeclaration": @@ -40,7 +45,19 @@ def for_function(self, fn: Callable) -> "AddonOperationDeclaration": def __post_init__(self): # use __setattr__ to ignore dataclass frozenness super().__setattr__("params_dataclass", self._build_params_dataclass()) - super().__setattr__("success_dataclass", self._get_success_dataclass()) + _return_type = self.get_return_type() + if self.return_dataclass is type(None): + assert dataclasses.is_dataclass( + _return_type + ), f"operation methods must return a dataclass (got {_return_type} on {self.operation_fn})" + super().__setattr__("return_dataclass", _return_type) + else: + assert dataclasses.is_dataclass( + self.return_dataclass + ), f"return_dataclass must be a dataclass (got {self.return_dataclass})" + assert issubclass( + _return_type, self.return_dataclass + ), f"expected return type {self.return_dataclass} on operation function {self.operation_fn} (got {_return_type})" @property def name(self): @@ -53,23 +70,22 @@ def docstring(self) -> str: # TODO: consider docstring param on operation decorators, allow overriding __doc__ return self.operation_fn.__doc__ or "" + def get_return_type(self) -> type: + return inspect.get_annotations(self.operation_fn)["return"] + + def get_param_types(self) -> tuple[tuple[str, Any], ...]: + return tuple( + (_name, _type) + for _name, _type in inspect.get_annotations(self.operation_fn).items() + if _name != "return" + ) + def _build_params_dataclass(self): - _fields = [ - (_param_name, _param_type) - for _param_name, _param_type in self.operation_fn.__annotations__.items() - if _param_name != "return" - ] return dataclasses.make_dataclass( - f"ParamsDataclass__{self.__class__.__qualname__}__{self.name}", - _fields, + f"ParamsDataclass__{self.__class__.__name__}__{self.name}", + fields=self.get_param_types(), ) - def _get_success_dataclass(self): - _return_type = self.operation_fn.__annotations__["return"] - # TODO: except KeyError - assert dataclasses.is_dataclass(_return_type) and isinstance(_return_type, type) - return _return_type - # declarator for all types of operations -- use operation_type-specific decorators below _operation_declarator = Declarator( @@ -77,13 +93,21 @@ def _get_success_dataclass(self): object_field="operation_fn", ) + +@dataclasses.dataclass +class RedirectResult: + url: str + method: HTTPMethod = HTTPMethod.GET + + # decorator for operations that may be performed by a client request (e.g. redirect to waterbutler) redirect_operation = _operation_declarator.with_kwargs( operation_type=AddonOperationType.REDIRECT, + return_dataclass=RedirectResult, # TODO: consider adding `save_invocation: bool = True`, set False here ) -# decorator for operations that must be performed by the server but will take only +# decorator for operations that must be performed by the server but should take only # a short time, so a server may wait on the operation before responding to a request # (e.g. get a metadata description of an item, list items in a given folder) immediate_operation = _operation_declarator.with_kwargs( diff --git a/addon_toolkit/storage.py b/addon_toolkit/storage.py index fb68d890..e512a506 100644 --- a/addon_toolkit/storage.py +++ b/addon_toolkit/storage.py @@ -1,8 +1,8 @@ import dataclasses import enum -from http import HTTPMethod from addon_toolkit import ( + RedirectResult, addon_interface, immediate_operation, redirect_operation, @@ -17,12 +17,6 @@ class StorageCapability(enum.Enum): UPDATE = "update" -@dataclasses.dataclass -class RedirectResult: - url: str - method: HTTPMethod = HTTPMethod.GET - - @dataclasses.dataclass class PagedResult: items: list diff --git a/addon_toolkit/tests/test_addon_interface.py b/addon_toolkit/tests/test_addon_interface.py index c2408aa5..3d8df41d 100644 --- a/addon_toolkit/tests/test_addon_interface.py +++ b/addon_toolkit/tests/test_addon_interface.py @@ -1,9 +1,11 @@ +import dataclasses import enum import unittest from addon_toolkit import ( AddonOperationDeclaration, AddonOperationImplementation, + RedirectResult, addon_interface, immediate_operation, redirect_operation, @@ -12,32 +14,50 @@ class TestAddonInterface(unittest.TestCase): + # the basics of an addon interface + class _MyCapability(enum.Enum): + GET_IT = "get-it" + PUT_IT = "put-it" + UNUSED = "unused" # for testing when a capability has no operations + + _MyInterface: type + _MyImplementation: type # subclass of _MyInterface + + # shared expectations across tests: + _expected_get_op: AddonOperationDeclaration + _expected_put_op: AddonOperationDeclaration + _expected_query_op: AddonOperationDeclaration + _expected_get_imp: AddonOperationImplementation + _expected_put_imp: AddonOperationImplementation + @classmethod def setUpClass(cls): ### # declare the capabilities and interface for a category of addons - class _MyCapability(enum.Enum): - GET_IT = "get-it" - PUT_IT = "put-it" - UNUSED = "unused" # for testing when a capability has no operations + @dataclasses.dataclass + class _MyCustomOperationResult: + url: str + flibbly: int - @addon_interface(capability_enum=_MyCapability) + @addon_interface(capability_enum=cls._MyCapability) class _MyInterface: """this _MyInterface docstring should find its way to browsable docs somewhere""" - @redirect_operation(capability=_MyCapability.GET_IT) - def url_for_get(self, checksum_iri) -> str: + @redirect_operation(capability=cls._MyCapability.GET_IT) + def url_for_get(self, checksum_iri) -> RedirectResult: """this url_for_get docstring should find its way to docs""" raise NotImplementedError - @immediate_operation(capability=_MyCapability.GET_IT) - async def query_relations(self, checksum_iri, query=None): + @immediate_operation(capability=cls._MyCapability.GET_IT) + async def query_relations( + self, checksum_iri, query=None + ) -> _MyCustomOperationResult: """this query_relations docstring should find its way to docs""" raise NotImplementedError - @redirect_operation(capability=_MyCapability.PUT_IT) - def url_for_put(self, checksum_iri): + @redirect_operation(capability=cls._MyCapability.PUT_IT) + def url_for_put(self, checksum_iri) -> RedirectResult: """this url_for_put docstring should find its way to docs""" raise NotImplementedError @@ -58,31 +78,38 @@ def url_for_put(self, checksum_iri): # )? return f"https://myarchive.example///{checksum_iri}" - cls._MyCapability = _MyCapability + ### + # shared test interface (on `self`) + + # shared static types cls._MyInterface = _MyInterface cls._MyImplementation = _MyImplementation + # shared operations cls._expected_get_op = AddonOperationDeclaration( operation_type=AddonOperationType.REDIRECT, - capability=_MyCapability.GET_IT, + capability=cls._MyCapability.GET_IT, operation_fn=_MyInterface.url_for_get, ) cls._expected_put_op = AddonOperationDeclaration( operation_type=AddonOperationType.REDIRECT, - capability=_MyCapability.PUT_IT, + capability=cls._MyCapability.PUT_IT, operation_fn=_MyInterface.url_for_put, ) cls._expected_query_op = AddonOperationDeclaration( operation_type=AddonOperationType.IMMEDIATE, - capability=_MyCapability.GET_IT, + capability=cls._MyCapability.GET_IT, operation_fn=_MyInterface.query_relations, ) + # a specific implementation of some of those shared operations cls._expected_get_imp = AddonOperationImplementation( + interface=addon_interface.get_declaration(_MyInterface), operation=cls._expected_get_op, implementation_cls=_MyImplementation, ) cls._expected_put_imp = AddonOperationImplementation( + interface=addon_interface.get_declaration(_MyInterface), operation=cls._expected_put_op, implementation_cls=_MyImplementation, ) @@ -96,7 +123,7 @@ def test_get_operation_declarations(self): self.assertEqual( set( _interface_dec.get_operation_declarations( - capability=self._MyCapability.GET_IT + capabilities=[self._MyCapability.GET_IT] ) ), {self._expected_get_op, self._expected_query_op}, @@ -104,7 +131,7 @@ def test_get_operation_declarations(self): self.assertEqual( set( _interface_dec.get_operation_declarations( - capability=self._MyCapability.PUT_IT + capabilities=[self._MyCapability.PUT_IT] ) ), {self._expected_put_op}, @@ -112,7 +139,7 @@ def test_get_operation_declarations(self): self.assertEqual( set( _interface_dec.get_operation_declarations( - capability=self._MyCapability.UNUSED + capabilities=[self._MyCapability.UNUSED] ) ), set(), @@ -131,7 +158,7 @@ def test_get_implemented_operations(self): set( AddonOperationImplementation.on_implementation_cls( self._MyImplementation, - capability=self._MyCapability.GET_IT, + capabilities=[self._MyCapability.GET_IT], ) ), {self._expected_get_imp}, @@ -140,7 +167,7 @@ def test_get_implemented_operations(self): set( AddonOperationImplementation.on_implementation_cls( self._MyImplementation, - capability=self._MyCapability.PUT_IT, + capabilities=[self._MyCapability.PUT_IT], ) ), {self._expected_put_imp}, @@ -149,7 +176,7 @@ def test_get_implemented_operations(self): set( AddonOperationImplementation.on_implementation_cls( self._MyImplementation, - capability=self._MyCapability.UNUSED, + capabilities=[self._MyCapability.UNUSED], ) ), set(),