From 6996c56ce99d1b3fe76ab6208010b76bb1d7af59 Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Mon, 25 Mar 2024 13:18:12 -0400 Subject: [PATCH 1/5] add mypy to pre-commit --- .pre-commit-config.yaml | 4 +++ mypy.ini | 52 +++++++++++++++++++++++++++++++ requirements/dev-requirements.txt | 3 ++ 3 files changed, 59 insertions(+) create mode 100644 mypy.ini diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index deb808d2..94df02b1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,3 +11,7 @@ repos: rev: 5.12.0 hooks: - id: isort + - repo: https://github.com/pre-commit/mirrors-mypy + rev: '9db9854e3041219b1eb619872a2dfaf58adfb20b' # v1.9.0 + hooks: + - id: mypy diff --git a/mypy.ini b/mypy.ini new file mode 100644 index 00000000..3c47cc7c --- /dev/null +++ b/mypy.ini @@ -0,0 +1,52 @@ +[mypy] +# start with an ideal: enable strict type-checking, then loosen in module-specific config +# see https://mypy.readthedocs.io/en/stable/existing_code.html#introduce-stricter-options +strict = True +## BEGIN aspects of `strict` that could be loosened: +# disallow_subclassing_any = False +# warn_unused_configs = False +# warn_redundant_casts = False +# warn_unused_ignores = False +# strict_equality = False +# strict_concatenate = False +# check_untyped_defs = False +# disallow_untyped_decorators = False +# disallow_any_generics = False +# disallow_untyped_calls = False +# disallow_incomplete_defs = False +# disallow_untyped_defs = False +# no_implicit_reexport = False +# warn_return_any = False +## END aspects of `strict` + +# prefer types that can be understood by reading code in only one place +local_partial_types = True +# avoid easily-avoidable dead code +warn_unreachable = True + +### +# module-specific config + +[mypy-addon_toolkit.*] +## loosen strict: +disallow_any_generics = False +disallow_untyped_calls = False +disallow_incomplete_defs = False +disallow_untyped_defs = False + +[mypy-addon_service.*,app.*,manage] +# got untyped dependencies -- this is fine +ignore_missing_imports = True +disable_error_code = import-untyped,import-not-found +## loosen strict: +disallow_subclassing_any = False +disallow_untyped_decorators = False +disallow_any_generics = False +disallow_untyped_calls = False +disallow_incomplete_defs = False +disallow_untyped_defs = False +warn_return_any = False + +[mypy-addon_service.migrations.*] +strict = False +disable_error_code = var-annotated diff --git a/requirements/dev-requirements.txt b/requirements/dev-requirements.txt index 51f3d502..4db647c7 100644 --- a/requirements/dev-requirements.txt +++ b/requirements/dev-requirements.txt @@ -11,6 +11,9 @@ black==24.* isort pre-commit +# Type checking +mypy + # ASGI server for local use # https://docs.djangoproject.com/en/4.2/howto/deployment/asgi/daphne/#integration-with-runserver daphne From 195bf60ce02e61355e6189500cdfa927832225a6 Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Mon, 25 Mar 2024 13:19:36 -0400 Subject: [PATCH 2/5] fix type errors --- addon_service/addon_operation/models.py | 7 +++++-- addon_service/addon_operation/serializers.py | 2 +- addon_toolkit/operation.py | 3 ++- addon_toolkit/tests/test_addon_protocol.py | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/addon_service/addon_operation/models.py b/addon_service/addon_operation/models.py index 8246eb99..9fd279c8 100644 --- a/addon_service/addon_operation/models.py +++ b/addon_service/addon_operation/models.py @@ -7,7 +7,10 @@ get_imp_name, ) from addon_service.common.static_dataclass_model import StaticDataclassModel -from addon_toolkit import AddonOperationImp +from addon_toolkit import ( + AddonCapabilities, + AddonOperationImp, +) from addon_toolkit.json_arguments import jsonschema_for_signature_params from addon_toolkit.operation import AddonOperationType @@ -42,7 +45,7 @@ def implementation_docstring(self) -> str: return self.operation_imp.imp_function.__doc__ or "" @cached_property - def capability(self) -> str: + def capability(self) -> AddonCapabilities: return self.operation_imp.declaration.capability @cached_property diff --git a/addon_service/addon_operation/serializers.py b/addon_service/addon_operation/serializers.py index 875b1f47..fcc39981 100644 --- a/addon_service/addon_operation/serializers.py +++ b/addon_service/addon_operation/serializers.py @@ -1,11 +1,11 @@ from rest_framework_json_api import serializers from rest_framework_json_api.utils import get_resource_type_from_model -from addon_service.addon_imp.models import AddonImp from addon_service.common import view_names from addon_service.common.enums.serializers import EnumNameChoiceField from addon_service.common.serializer_fields import DataclassRelatedDataField from addon_toolkit import AddonCapabilities +from addon_toolkit.imp import AddonImp from .models import AddonOperationModel diff --git a/addon_toolkit/operation.py b/addon_toolkit/operation.py index 75efe879..76ec4aaf 100644 --- a/addon_toolkit/operation.py +++ b/addon_toolkit/operation.py @@ -7,6 +7,7 @@ Iterator, ) +from .capabilities import AddonCapabilities from .declarator import Declarator @@ -35,7 +36,7 @@ class AddonOperationDeclaration: """ operation_type: AddonOperationType - capability: enum.Enum + capability: AddonCapabilities operation_fn: Callable # the decorated function return_type: type = dataclasses.field( default=type(None), # if not provided, inferred by __post_init__ diff --git a/addon_toolkit/tests/test_addon_protocol.py b/addon_toolkit/tests/test_addon_protocol.py index a39d2eec..7ccb4b1e 100644 --- a/addon_toolkit/tests/test_addon_protocol.py +++ b/addon_toolkit/tests/test_addon_protocol.py @@ -21,7 +21,7 @@ class TestAddonProtocol(unittest.TestCase): # the basics of an addon protocol ### - # shared test env (on `self`) + # shared test env (initialized by setUpClass) _MyProtocol: type # typing.Protocol subclass decorated with `@addon_protocol` _MyImplementation: type # subclass of _MyProtocol _my_imp: AddonImp From 6ac6cfd8a8d25823d988a07215eb32f8aee282d9 Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Fri, 3 May 2024 09:46:05 -0400 Subject: [PATCH 3/5] fix more type errors --- addon_service/addon_operation/models.py | 7 ++- addon_service/common/hmac.py | 32 ++++++---- addon_service/common/jsonapi.py | 12 ++-- .../test_authorized_storage_account.py | 7 ++- addon_toolkit/credentials.py | 5 +- addon_toolkit/declarator.py | 4 +- addon_toolkit/imp.py | 9 ++- addon_toolkit/json_arguments.py | 26 +++++--- addon_toolkit/operation.py | 61 ++++++++----------- mypy.ini | 1 - 10 files changed, 91 insertions(+), 73 deletions(-) diff --git a/addon_service/addon_operation/models.py b/addon_service/addon_operation/models.py index 9fd279c8..563e26a3 100644 --- a/addon_service/addon_operation/models.py +++ b/addon_service/addon_operation/models.py @@ -11,7 +11,10 @@ AddonCapabilities, AddonOperationImp, ) -from addon_toolkit.json_arguments import jsonschema_for_signature_params +from addon_toolkit.json_arguments import ( + JsonableDict, + jsonschema_for_signature_params, +) from addon_toolkit.operation import AddonOperationType @@ -49,7 +52,7 @@ def capability(self) -> AddonCapabilities: return self.operation_imp.declaration.capability @cached_property - def params_jsonschema(self) -> dict: + def params_jsonschema(self) -> JsonableDict: return jsonschema_for_signature_params( self.operation_imp.declaration.call_signature ) diff --git a/addon_service/common/hmac.py b/addon_service/common/hmac.py index a64e993c..ff48af92 100644 --- a/addon_service/common/hmac.py +++ b/addon_service/common/hmac.py @@ -2,7 +2,7 @@ import hashlib import hmac import re -import urllib +import urllib.parse from datetime import ( UTC, datetime, @@ -16,7 +16,7 @@ ) -def _sign_message(message: str, hmac_key: str = None) -> str: +def _sign_message(message: str, hmac_key: str | None = None) -> str: key = hmac_key or settings.DEFAULT_HMAC_KEY encoded_message = base64.b64encode(message.encode()) return hmac.new( @@ -30,25 +30,31 @@ def _get_signed_components( parsed_url = urllib.parse.urlparse(request_url) if isinstance(body, str): body = body.encode() - content_hash = hashlib.sha256(body).hexdigest if body else None - auth_timestamp = datetime.now(UTC) + content_hash = hashlib.sha256(body).hexdigest() if body else None + auth_timestamp = str(datetime.now(UTC)) + # Filter out query string and content_hash if none present signed_segments = [ - request_method, - parsed_url.path, - parsed_url.query, - str(auth_timestamp), - content_hash, + segment + for segment in [ + request_method, + parsed_url.path, + parsed_url.query, + auth_timestamp, + content_hash, + ] + if segment ] - # Filter out query string and content_hash if none present - signed_segments = [segment for segment in signed_segments if segment] - signed_headers = {"X-Authorization-Timestamp": auth_timestamp} + signed_headers: dict[str, str] = {"X-Authorization-Timestamp": auth_timestamp} if content_hash: signed_headers["X-Content-SHA256"] = content_hash return signed_segments, signed_headers def make_signed_headers( - request_url: str, request_method: str, body: str | bytes = "", hmac_key: str = None + request_url: str, + request_method: str, + body: str | bytes = "", + hmac_key: str | None = None, ) -> dict: signed_string_segments, signed_headers = _get_signed_components( request_url, request_method, body diff --git a/addon_service/common/jsonapi.py b/addon_service/common/jsonapi.py index 71f0177a..81bbcaf5 100644 --- a/addon_service/common/jsonapi.py +++ b/addon_service/common/jsonapi.py @@ -13,7 +13,7 @@ class JSONAPIQueryParam: """Dataclass for describing the contents of a JSON:API-compliant Query Parameter.""" family: str - args: tuple[str] = () + args: tuple[str, ...] = () value: str = "" # Matches any alphanumeric string followed by an open bracket or end of input @@ -29,7 +29,7 @@ def from_key_value_pair(cls, query_param_name: str, query_param_value: str) -> S return cls(family, args, query_param_value) @classmethod - def parse_param_name(cls, query_param_name: str) -> tuple[str, tuple[str]]: + def parse_param_name(cls, query_param_name: str) -> tuple[str, tuple[str, ...]]: """Parses a query parameter name into its family and bracketed args. >>> JSONAPIQueryParam.parse_param_name('filter') @@ -43,7 +43,9 @@ def parse_param_name(cls, query_param_name: str) -> tuple[str, tuple[str]]: """ if not cls._param_name_is_valid(query_param_name): raise ValueError(f"Invalid query param name: {query_param_name}") - family = cls.FAMILY_REGEX.match(query_param_name).group() + family_match = cls.FAMILY_REGEX.match(query_param_name) + assert family_match is not None + family = family_match.group() args = cls.ARG_REGEX.findall(query_param_name) return (family, tuple(args)) @@ -82,7 +84,7 @@ def __str__(self): return f"{self.family}{args}={self.value}" -QueryParamFamilies = dict[str, Iterable[JSONAPIQueryParam]] +QueryParamFamilies = dict[str, list[JSONAPIQueryParam]] def group_query_params_by_family( @@ -93,7 +95,7 @@ def group_query_params_by_family( Data should be pre-normalized before calling, such as by using the results of `urllib.parse.parse_qs(...).items()` or `django.utils.QueryDict.lists()` """ - grouped_query_params = QueryParamFamilies() + grouped_query_params: QueryParamFamilies = {} for _unparsed_name, _param_values in query_items: # Handle wsgiref.headers.Headers-style multi-dicts if isinstance(_param_values, str): 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 3de6f087..31d4ced3 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 @@ -68,7 +68,7 @@ def _make_post_payload( } credentials = credentials or MOCK_CREDENTIALS[external_service.credentials_format] if credentials: - payload["data"]["attributes"]["credentials"] = credentials.asdict() + payload["data"]["attributes"]["credentials"] = credentials.asdict() # type: ignore return payload @@ -143,10 +143,12 @@ def test_post__sets_credentials(self): self.assertEqual(_resp.status_code, HTTPStatus.CREATED) account = db.AuthorizedStorageAccount.objects.get(id=_resp.data["id"]) + mock_credentials = MOCK_CREDENTIALS[creds_format] + assert mock_credentials is not None with self.subTest(creds_format=creds_format): self.assertEqual( account._credentials.credentials_blob, - MOCK_CREDENTIALS[creds_format].asdict(), + mock_credentials.asdict(), ) def test_post__sets_auth_url(self): @@ -390,6 +392,7 @@ def test_set_credentials__create(self): ) self.assertIsNone(account._credentials) mock_credentials = MOCK_CREDENTIALS[creds_format] + assert mock_credentials is not None account.credentials = mock_credentials account.save() with self.subTest(creds_format=creds_format): diff --git a/addon_toolkit/credentials.py b/addon_toolkit/credentials.py index d5375bd9..913d804c 100644 --- a/addon_toolkit/credentials.py +++ b/addon_toolkit/credentials.py @@ -2,11 +2,14 @@ import typing +@dataclasses.dataclass(frozen=True) class Credentials(typing.Protocol): def asdict(self): return dataclasses.asdict(self) - def iter_headers(self) -> typing.Iterator[tuple[str, str]]: ... + def iter_headers(self) -> typing.Iterator[tuple[str, str]]: + return + yield @dataclasses.dataclass(frozen=True, kw_only=True) diff --git a/addon_toolkit/declarator.py b/addon_toolkit/declarator.py index 416525c4..d3eca8a2 100644 --- a/addon_toolkit/declarator.py +++ b/addon_toolkit/declarator.py @@ -79,7 +79,7 @@ def _decorator(decorator_target: DecoratorTarget) -> DecoratorTarget: return _decorator - def with_kwargs(self, **static_kwargs) -> "Declarator": + def with_kwargs(self, **static_kwargs) -> "Declarator[DeclarationDataclass]": """convenience for decorators that differ only by static field values""" # note: shared __declarations_by_target return dataclasses.replace(self, static_kwargs=static_kwargs) @@ -105,7 +105,7 @@ def get_declaration(self, target) -> DeclarationDataclass: raise ValueError(f"no declaration found for {target}") -class ClassDeclarator(Declarator): +class ClassDeclarator(Declarator[DeclarationDataclass]): """add declarative metadata to python classes using decorators (same as Declarator but with additional methods that only make diff --git a/addon_toolkit/imp.py b/addon_toolkit/imp.py index 0b026187..c2023dd2 100644 --- a/addon_toolkit/imp.py +++ b/addon_toolkit/imp.py @@ -11,7 +11,10 @@ sync_to_async, ) -from .json_arguments import kwargs_from_json +from .json_arguments import ( + JsonableDict, + kwargs_from_json, +) from .operation import AddonOperationDeclaration from .protocol import ( AddonProtocolDeclaration, @@ -88,7 +91,9 @@ def __post_init__(self): def imp_function(self): return getattr(self.addon_imp.imp_cls, self.declaration.name) - async def invoke_thru_addon(self, addon_instance: object, json_kwargs: dict): + async def invoke_thru_addon( + self, addon_instance: object, json_kwargs: JsonableDict + ): _method = self._get_instance_method(addon_instance) _kwargs = kwargs_from_json(self.declaration.call_signature, json_kwargs) if not inspect.iscoroutinefunction(_method): diff --git a/addon_toolkit/json_arguments.py b/addon_toolkit/json_arguments.py index 889759f4..c1ccb122 100644 --- a/addon_toolkit/json_arguments.py +++ b/addon_toolkit/json_arguments.py @@ -13,8 +13,13 @@ "jsonschema_for_signature_params", ) +JsonableScalar = str | int | float | None | bool | enum.Enum +JsonableList = typing.Iterable["JsonableValue"] +JsonableDict = typing.Mapping[str, "JsonableValue"] +JsonableValue = JsonableScalar | JsonableList | JsonableDict -def jsonschema_for_signature_params(signature: inspect.Signature) -> dict: + +def jsonschema_for_signature_params(signature: inspect.Signature) -> JsonableDict: """build jsonschema corresponding to parameters from a function signature >>> def _foo(a: str, b: int = 7): ... @@ -40,13 +45,13 @@ def jsonschema_for_signature_params(signature: inspect.Signature) -> dict: } -def jsonschema_for_dataclass(dataclass: type) -> dict: +def jsonschema_for_dataclass(dataclass: type) -> JsonableDict: """build jsonschema corresponding to the constructor signature of a dataclass""" assert dataclasses.is_dataclass(dataclass) and isinstance(dataclass, type) return jsonschema_for_signature_params(inspect.signature(dataclass)) -def jsonschema_for_annotation(annotation: type) -> dict: +def jsonschema_for_annotation(annotation: type) -> JsonableDict: """build jsonschema for a python type annotation""" if dataclasses.is_dataclass(annotation): return jsonschema_for_dataclass(annotation) @@ -105,8 +110,8 @@ def json_for_typed_value(type_annotation: typing.Any, value: typing.Any): def kwargs_from_json( signature: inspect.Signature, - args_from_json: dict, -) -> dict: + args_from_json: JsonableDict, +) -> JsonableDict: try: _kwargs = { _param_name: arg_value_from_json( @@ -142,7 +147,7 @@ def arg_value_from_json( raise NotImplementedError(f"what do with `{json_arg_value}` (value for {param})") -def json_for_dataclass(dataclass_instance) -> dict: +def json_for_dataclass(dataclass_instance) -> JsonableDict: """return json-serializable representation of the dataclass instance""" _field_value_pairs = ( (_field, getattr(dataclass_instance, _field.name)) @@ -155,7 +160,7 @@ def json_for_dataclass(dataclass_instance) -> dict: } -def dataclass_from_json(dataclass: type, dataclass_json: dict): +def dataclass_from_json(dataclass: type, dataclass_json: JsonableDict): return dataclass( **{ _field.name: field_value_from_json(_field, dataclass_json) @@ -164,7 +169,9 @@ def dataclass_from_json(dataclass: type, dataclass_json: dict): ) -def field_value_from_json(field: dataclasses.Field, dataclass_json: dict): +def field_value_from_json( + field: dataclasses.Field[JsonableValue], dataclass_json: JsonableDict +): _json_value = dataclass_json.get(field.name) if _json_value is None: return None # TODO: check optional @@ -174,7 +181,8 @@ def field_value_from_json(field: dataclasses.Field, dataclass_json: dict): if issubclass(field.type, enum.Enum): return field.type(_json_value) if field.type in (tuple, list, set, frozenset): - return field.type(_json_value) + # type is narrowed but mypy doesn't understand + return field.type(_json_value) # type: ignore if field.type in (str, int, float): assert isinstance(_json_value, field.type) return _json_value diff --git a/addon_toolkit/operation.py b/addon_toolkit/operation.py index 76ec4aaf..6ce30096 100644 --- a/addon_toolkit/operation.py +++ b/addon_toolkit/operation.py @@ -1,11 +1,9 @@ import dataclasses import enum +import functools import inspect +import typing from http import HTTPMethod -from typing import ( - Callable, - Iterator, -) from .capabilities import AddonCapabilities from .declarator import Declarator @@ -37,34 +35,33 @@ class AddonOperationDeclaration: operation_type: AddonOperationType capability: AddonCapabilities - operation_fn: Callable # the decorated function - return_type: type = dataclasses.field( - default=type(None), # if not provided, inferred by __post_init__ - compare=False, - ) + operation_fn: typing.Callable[..., typing.Any] # the decorated function + required_return_type: type | None = dataclasses.field(default=None, compare=False) @classmethod - def for_function(self, fn: Callable) -> "AddonOperationDeclaration": + def for_function( + self, fn: typing.Callable[..., typing.Any] + ) -> "AddonOperationDeclaration": return addon_operation.get_declaration(fn) def __post_init__(self): + if self.required_return_type and not issubclass( + self.return_type, self.required_return_type + ): + raise ValueError( + f"expected return type {self.return_type} on operation function {self.operation_fn} (got {self.return_type})" + ) + + @functools.cached_property + def return_type(self) -> type: _return_type = self.call_signature.return_annotation - if self.return_type is type(None): - # no return_type declared; infer from type annotation - assert dataclasses.is_dataclass( - _return_type - ), f"operation methods must return a dataclass (got {_return_type} on {self.operation_fn})" - # use object.__setattr__ to bypass dataclass frozenness (only here in __post_init__) - object.__setattr__(self, "return_type", _return_type) - else: - # return_type declared; enforce it - assert dataclasses.is_dataclass( - self.return_type - ), f"return_type must be a dataclass (got {self.return_type})" - if not issubclass(_return_type, self.return_type): - raise ValueError( - f"expected return type {self.return_type} on operation function {self.operation_fn} (got {_return_type})" - ) + if not ( + isinstance(_return_type, type) and dataclasses.is_dataclass(_return_type) + ): + raise ValueError( + f"operation methods must return a dataclass (got {_return_type} on {self.operation_fn})" + ) + return _return_type @property def name(self): @@ -77,18 +74,10 @@ def docstring(self) -> str: # TODO: docstring/description param on operation decorators, since __doc__ is removed on -O return self.operation_fn.__doc__ or "" - @property + @functools.cached_property def call_signature(self) -> inspect.Signature: return inspect.signature(self.operation_fn) - def param_dataclasses(self) -> Iterator[type]: - for _param_name, _param in self.call_signature.parameters.items(): - if not dataclasses.is_dataclass(_param.annotation): - raise ValueError( - f"operation parameters must have dataclass annotations (TODO: decorator to infer dataclass from annotated kwargs), got `{_param_name}: {_param.annotation}`" - ) - yield _param.annotation - # declarator for all types of operations -- use operation_type-specific decorators below addon_operation = Declarator( @@ -106,7 +95,7 @@ class RedirectResult: # decorator for operations that may be performed by a client request (e.g. redirect to waterbutler) redirect_operation = addon_operation.with_kwargs( operation_type=AddonOperationType.REDIRECT, - return_type=RedirectResult, + required_return_type=RedirectResult, # TODO: consider adding `save_invocation: bool = True`, set False here ) diff --git a/mypy.ini b/mypy.ini index 3c47cc7c..6d83679c 100644 --- a/mypy.ini +++ b/mypy.ini @@ -29,7 +29,6 @@ warn_unreachable = True [mypy-addon_toolkit.*] ## loosen strict: -disallow_any_generics = False disallow_untyped_calls = False disallow_incomplete_defs = False disallow_untyped_defs = False From 59253d24fdd7bbc22070882d8ef7157835200d3e Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Fri, 3 May 2024 10:10:03 -0400 Subject: [PATCH 4/5] fix test error --- addon_service/common/permissions.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/addon_service/common/permissions.py b/addon_service/common/permissions.py index 5b2f2dff..84d344cf 100644 --- a/addon_service/common/permissions.py +++ b/addon_service/common/permissions.py @@ -78,10 +78,20 @@ def has_permission(self, request, view): expiration_time = datetime.now(UTC) - timedelta( seconds=self.REQUEST_EXPIRATION_SECONDS ) - request_timestamp = request.headers.get("X-Authorization-Timestamp") - if not request_timestamp or request_timestamp < expiration_time: + request_timestamp_str = request.headers.get("X-Authorization-Timestamp") + if not request_timestamp_str: + raise exceptions.PermissionDenied( + "Missing required 'X-Authorization-Timestamp' value" + ) + try: + _request_timestamp = datetime.fromisoformat(request_timestamp_str) + except ValueError: + raise exceptions.PermissionDenied( + "Invalid 'X-Authorization-Timestamp' value" + ) + if _request_timestamp < expiration_time: raise exceptions.PermissionDenied("HMAC Signed Request is expired") - elif request_timestamp > datetime.now(UTC): + if _request_timestamp > datetime.now(UTC): raise exceptions.PermissionDenied( "HMAC Signed Request provided a timestamp from the future" ) From bce59e43562e50ee561a5d699001e6b829111861 Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Fri, 3 May 2024 11:08:18 -0400 Subject: [PATCH 5/5] tighten addon_toolkit types --- addon_imps/storage/box_dot_com.py | 2 +- addon_service/common/network.py | 2 +- addon_toolkit/constrained_network/http.py | 22 ++++++++-------- addon_toolkit/credentials.py | 7 +++-- addon_toolkit/cursor.py | 24 +++++++++++------- addon_toolkit/declarator.py | 31 +++++++++++++---------- addon_toolkit/imp.py | 25 ++++++++---------- addon_toolkit/iri_utils.py | 6 ++--- addon_toolkit/json_arguments.py | 28 ++++++++++++++------ addon_toolkit/operation.py | 4 +-- addon_toolkit/tests/_doctest.py | 29 ++++++++++++++++++--- addon_toolkit/typing.py | 8 ++++++ mypy.ini | 6 ++--- 13 files changed, 120 insertions(+), 74 deletions(-) create mode 100644 addon_toolkit/typing.py diff --git a/addon_imps/storage/box_dot_com.py b/addon_imps/storage/box_dot_com.py index abf788d6..0b5a9e8b 100644 --- a/addon_imps/storage/box_dot_com.py +++ b/addon_imps/storage/box_dot_com.py @@ -61,7 +61,7 @@ def _params_from_cursor(self, cursor: str = "") -> dict[str, str]: # https://developer.box.com/guides/api-calls/pagination/offset-based/ try: _cursor = OffsetCursor.from_str(cursor) - return {"offset": _cursor.offset, "limit": _cursor.limit} + return {"offset": str(_cursor.offset), "limit": str(_cursor.limit)} except ValueError: return {} diff --git a/addon_service/common/network.py b/addon_service/common/network.py index a804eeed..4160d03c 100644 --- a/addon_service/common/network.py +++ b/addon_service/common/network.py @@ -66,7 +66,7 @@ def __init__( # abstract method from HttpRequestor: @contextlib.asynccontextmanager - async def do_send(self, request: HttpRequestInfo): + async def send_request(self, request: HttpRequestInfo): try: async with self._try_send(request) as _response: yield _response diff --git a/addon_toolkit/constrained_network/http.py b/addon_toolkit/constrained_network/http.py index 1ad34ffd..282d404d 100644 --- a/addon_toolkit/constrained_network/http.py +++ b/addon_toolkit/constrained_network/http.py @@ -61,25 +61,25 @@ class HttpRequestor(typing.Protocol): def response_info_cls(self) -> type[HttpResponseInfo]: ... # abstract method for subclasses - def do_send( + def send_request( self, request: HttpRequestInfo ) -> contextlib.AbstractAsyncContextManager[HttpResponseInfo]: ... @contextlib.asynccontextmanager - async def request( + async def _request( self, http_method: HTTPMethod, uri_path: str, query: Multidict | KeyValuePairs | None = None, headers: Multidict | KeyValuePairs | None = None, - ): + ) -> typing.Any: # loose type; method-specific methods below are more accurate _request_info = HttpRequestInfo( http_method=http_method, uri_path=uri_path, query=(query if isinstance(query, Multidict) else Multidict(query)), headers=(headers if isinstance(headers, Multidict) else Multidict(headers)), ) - async with self.do_send(_request_info) as _response: + async with self.send_request(_request_info) as _response: yield _response # TODO: streaming send/receive (only if/when needed) @@ -88,10 +88,10 @@ async def request( # convenience methods for http methods # (same call signature as self.request, minus `http_method`) - OPTIONS: _MethodRequestMethod = partialmethod(request, HTTPMethod.OPTIONS) - HEAD: _MethodRequestMethod = partialmethod(request, HTTPMethod.HEAD) - GET: _MethodRequestMethod = partialmethod(request, HTTPMethod.GET) - PATCH: _MethodRequestMethod = partialmethod(request, HTTPMethod.PATCH) - POST: _MethodRequestMethod = partialmethod(request, HTTPMethod.POST) - PUT: _MethodRequestMethod = partialmethod(request, HTTPMethod.PUT) - DELETE: _MethodRequestMethod = partialmethod(request, HTTPMethod.DELETE) + OPTIONS: _MethodRequestMethod = partialmethod(_request, HTTPMethod.OPTIONS) + HEAD: _MethodRequestMethod = partialmethod(_request, HTTPMethod.HEAD) + GET: _MethodRequestMethod = partialmethod(_request, HTTPMethod.GET) + PATCH: _MethodRequestMethod = partialmethod(_request, HTTPMethod.PATCH) + POST: _MethodRequestMethod = partialmethod(_request, HTTPMethod.POST) + PUT: _MethodRequestMethod = partialmethod(_request, HTTPMethod.PUT) + DELETE: _MethodRequestMethod = partialmethod(_request, HTTPMethod.DELETE) diff --git a/addon_toolkit/credentials.py b/addon_toolkit/credentials.py index 913d804c..bce71036 100644 --- a/addon_toolkit/credentials.py +++ b/addon_toolkit/credentials.py @@ -4,19 +4,18 @@ @dataclasses.dataclass(frozen=True) class Credentials(typing.Protocol): - def asdict(self): + def asdict(self) -> dict[str, typing.Any]: return dataclasses.asdict(self) def iter_headers(self) -> typing.Iterator[tuple[str, str]]: - return - yield + yield from () # no headers unless implemented by subclass @dataclasses.dataclass(frozen=True, kw_only=True) class AccessTokenCredentials(Credentials): access_token: str - def iter_headers(self): + def iter_headers(self) -> typing.Iterator[tuple[str, str]]: yield ("Authorization", f"Bearer {self.access_token}") diff --git a/addon_toolkit/cursor.py b/addon_toolkit/cursor.py index d92c09e9..6bc3f7a0 100644 --- a/addon_toolkit/cursor.py +++ b/addon_toolkit/cursor.py @@ -1,26 +1,32 @@ import base64 import dataclasses import json -from typing import ( - ClassVar, - Protocol, -) +import typing -def encode_cursor_dataclass(dataclass_instance) -> str: +class DataclassInstance(typing.Protocol): + __dataclass_fields__: typing.ClassVar[dict[str, typing.Any]] + + +SomeDataclassInstance = typing.TypeVar("SomeDataclassInstance", bound=DataclassInstance) + + +def encode_cursor_dataclass(dataclass_instance: DataclassInstance) -> str: _as_json = json.dumps(dataclasses.astuple(dataclass_instance)) _cursor_bytes = base64.b64encode(_as_json.encode()) return _cursor_bytes.decode() -def decode_cursor_dataclass(cursor: str, dataclass_class): +def decode_cursor_dataclass( + cursor: str, dataclass_class: type[SomeDataclassInstance] +) -> SomeDataclassInstance: _as_list = json.loads(base64.b64decode(cursor)) return dataclass_class(*_as_list) -class Cursor(Protocol): +class Cursor(DataclassInstance, typing.Protocol): @classmethod - def from_str(cls, cursor: str): + def from_str(cls, cursor: str) -> typing.Self: return decode_cursor_dataclass(cursor, cls) @property @@ -52,7 +58,7 @@ class OffsetCursor(Cursor): limit: int total_count: int # use -1 to mean "many more" - MAX_INDEX: ClassVar[int] = 9999 + MAX_INDEX: typing.ClassVar[int] = 9999 @property def next_cursor_str(self) -> str | None: diff --git a/addon_toolkit/declarator.py b/addon_toolkit/declarator.py index d3eca8a2..c6929e6d 100644 --- a/addon_toolkit/declarator.py +++ b/addon_toolkit/declarator.py @@ -7,13 +7,14 @@ TypeVar, ) +from addon_toolkit.typing import DataclassInstance + DecoratorTarget = TypeVar("DecoratorTarget") -DeclarationDataclass = TypeVar("DeclarationDataclass") @dataclasses.dataclass -class Declarator(Generic[DeclarationDataclass]): +class Declarator(Generic[DataclassInstance]): """Declarator: add declarative metadata in python using decorators and dataclasses define a dataclass with fields you want declared in your decorator, plus a field @@ -48,15 +49,15 @@ class Declarator(Generic[DeclarationDataclass]): TwoPartGreetingDeclaration(a='kia', b='ora', on=) """ - declaration_dataclass: type[DeclarationDataclass] + declaration_dataclass: type[DataclassInstance] field_for_target: str static_kwargs: dict[str, Any] | None = None # private storage linking a decorated class or function to data gleaned from its decorator - __declarations_by_target: weakref.WeakKeyDictionary[ - object, DeclarationDataclass - ] = dataclasses.field( - default_factory=weakref.WeakKeyDictionary, + __declarations_by_target: weakref.WeakKeyDictionary[object, DataclassInstance] = ( + dataclasses.field( + default_factory=weakref.WeakKeyDictionary, + ) ) def __post_init__(self) -> None: @@ -69,7 +70,7 @@ def __post_init__(self) -> None: ), f'expected field "{self.field_for_target}" on dataclass "{self.declaration_dataclass}"' def __call__( - self, **declaration_dataclass_kwargs + self, **declaration_dataclass_kwargs: Any ) -> Callable[[DecoratorTarget], DecoratorTarget]: """for using a Declarator as a decorator""" @@ -79,13 +80,13 @@ def _decorator(decorator_target: DecoratorTarget) -> DecoratorTarget: return _decorator - def with_kwargs(self, **static_kwargs) -> "Declarator[DeclarationDataclass]": + def with_kwargs(self, **static_kwargs: Any) -> "Declarator[DataclassInstance]": """convenience for decorators that differ only by static field values""" # note: shared __declarations_by_target return dataclasses.replace(self, static_kwargs=static_kwargs) def set_declaration( - self, declaration_target: DecoratorTarget, **declaration_dataclass_kwargs + self, declaration_target: DecoratorTarget, **declaration_dataclass_kwargs: Any ) -> None: """create a declaration associated with the target @@ -98,14 +99,14 @@ def set_declaration( **{self.field_for_target: declaration_target}, ) - def get_declaration(self, target) -> DeclarationDataclass: + def get_declaration(self, target: DecoratorTarget) -> DataclassInstance: try: return self.__declarations_by_target[target] except KeyError: raise ValueError(f"no declaration found for {target}") -class ClassDeclarator(Declarator[DeclarationDataclass]): +class ClassDeclarator(Declarator[DataclassInstance]): """add declarative metadata to python classes using decorators (same as Declarator but with additional methods that only make @@ -157,13 +158,15 @@ class ClassDeclarator(Declarator[DeclarationDataclass]): SemanticVersionDeclaration(major=4, minor=2, patch=9, subj=) """ - def get_declaration_for_class_or_instance(self, type_or_object: type | object): + def get_declaration_for_class_or_instance( + self, type_or_object: type | object + ) -> DataclassInstance: _cls = ( type_or_object if isinstance(type_or_object, type) else type(type_or_object) ) return self.get_declaration_for_class(_cls) - def get_declaration_for_class(self, cls: type): + def get_declaration_for_class(self, cls: type) -> DataclassInstance: for _cls in cls.__mro__: try: return self.get_declaration(_cls) diff --git a/addon_toolkit/imp.py b/addon_toolkit/imp.py index c2023dd2..ea4f47eb 100644 --- a/addon_toolkit/imp.py +++ b/addon_toolkit/imp.py @@ -1,10 +1,7 @@ import dataclasses import enum import inspect -from typing import ( - Iterable, - Iterator, -) +import typing from asgiref.sync import ( async_to_sync, @@ -37,7 +34,7 @@ class AddonImp: imp_number: int addon_protocol: AddonProtocolDeclaration = dataclasses.field(init=False) - def __post_init__(self, addon_protocol_cls): + def __post_init__(self, addon_protocol_cls: type) -> None: object.__setattr__( # using __setattr__ to bypass dataclass frozenness self, "addon_protocol", @@ -45,8 +42,8 @@ def __post_init__(self, addon_protocol_cls): ) def get_operation_imps( - self, *, capabilities: Iterable[enum.Enum] = () - ) -> Iterator["AddonOperationImp"]: + self, *, capabilities: typing.Iterable[enum.Enum] = () + ) -> typing.Iterator["AddonOperationImp"]: for _declaration in self.addon_protocol.get_operation_declarations( capabilities=capabilities ): @@ -74,13 +71,13 @@ class AddonOperationImp: addon_imp: AddonImp declaration: AddonOperationDeclaration - def __post_init__(self): + def __post_init__(self) -> None: _protocol_fn = getattr( self.addon_imp.addon_protocol.protocol_cls, self.declaration.name ) try: _imp_fn = self.imp_function - except AttributeError: + except Exception: _imp_fn = _protocol_fn if _imp_fn is _protocol_fn: raise NotImplementedError( # TODO: helpful exception type @@ -88,12 +85,12 @@ def __post_init__(self): ) @property - def imp_function(self): + def imp_function(self) -> typing.Any: # TODO: less typing.Any return getattr(self.addon_imp.imp_cls, self.declaration.name) async def invoke_thru_addon( self, addon_instance: object, json_kwargs: JsonableDict - ): + ) -> typing.Any: # TODO: less typing.Any _method = self._get_instance_method(addon_instance) _kwargs = kwargs_from_json(self.declaration.call_signature, json_kwargs) if not inspect.iscoroutinefunction(_method): @@ -104,7 +101,7 @@ async def invoke_thru_addon( invoke_thru_addon__blocking = async_to_sync(invoke_thru_addon) - def _get_instance_method(self, addon_instance: object): + def _get_instance_method( + self, addon_instance: object + ) -> typing.Any: # TODO: less typing.Any return getattr(addon_instance, self.declaration.name) - - # TODO: async def async_call_with_json_kwargs(self, addon_instance: object, json_kwargs: dict): diff --git a/addon_toolkit/iri_utils.py b/addon_toolkit/iri_utils.py index c8b9230f..2250a3a3 100644 --- a/addon_toolkit/iri_utils.py +++ b/addon_toolkit/iri_utils.py @@ -63,14 +63,14 @@ def __init__(self, key_value_pairs: KeyValuePairs | None = None): _headerslist = list(key_value_pairs) super().__init__(_headerslist) - def add(self, key: str, value: str, **mediatype_params): + def add(self, key: str, value: str) -> None: """add a key-value pair (allowing other values to exist) alias of `wsgiref.headers.Headers.add_header` """ - super().add_header(key, value, **mediatype_params) + super().add_header(key, value) - def add_many(self, pairs: Iterable[tuple[str, str]]): + def add_many(self, pairs: Iterable[tuple[str, str]]) -> None: for _key, _value in pairs: self.add(_key, _value) diff --git a/addon_toolkit/json_arguments.py b/addon_toolkit/json_arguments.py index c1ccb122..4c79c781 100644 --- a/addon_toolkit/json_arguments.py +++ b/addon_toolkit/json_arguments.py @@ -4,6 +4,11 @@ import types import typing +from addon_toolkit.typing import ( + AnyDataclassInstance, + DataclassInstance, +) + __all__ = ( "kwargs_from_json", @@ -13,7 +18,7 @@ "jsonschema_for_signature_params", ) -JsonableScalar = str | int | float | None | bool | enum.Enum +JsonableScalar = str | int | float | None | bool | enum.Enum | AnyDataclassInstance JsonableList = typing.Iterable["JsonableValue"] JsonableDict = typing.Mapping[str, "JsonableValue"] JsonableValue = JsonableScalar | JsonableList | JsonableDict @@ -45,7 +50,7 @@ def jsonschema_for_signature_params(signature: inspect.Signature) -> JsonableDic } -def jsonschema_for_dataclass(dataclass: type) -> JsonableDict: +def jsonschema_for_dataclass(dataclass: type[DataclassInstance]) -> JsonableDict: """build jsonschema corresponding to the constructor signature of a dataclass""" assert dataclasses.is_dataclass(dataclass) and isinstance(dataclass, type) return jsonschema_for_signature_params(inspect.signature(dataclass)) @@ -67,7 +72,9 @@ def jsonschema_for_annotation(annotation: type) -> JsonableDict: # TODO generic type: def json_for_typed_value[_ValueType: object](type_annotation: type[_ValueType], value: _ValueType): -def json_for_typed_value(type_annotation: typing.Any, value: typing.Any): +def json_for_typed_value( + type_annotation: typing.Any, value: typing.Any +) -> JsonableValue: """return json-serializable representation of field value >>> json_for_typed_value(int, 13) @@ -87,11 +94,12 @@ def json_for_typed_value(type_annotation: typing.Any, value: typing.Any): raise ValueError(f"expected instance of {_type}, got {value}") return json_for_dataclass(value) if issubclass(_type, enum.Enum): + assert isinstance(value, enum.Enum) if value not in _type: raise ValueError(f"expected member of enum {_type}, got {value}") return value.name if _type in (str, int, float): # check str before Iterable - return _type(value) + return _type(value) # type: ignore[no-any-return] if isinstance(_type, types.GenericAlias): # parameterized generic like `list[int]` if issubclass(_type.__origin__, typing.Iterable): @@ -147,7 +155,7 @@ def arg_value_from_json( raise NotImplementedError(f"what do with `{json_arg_value}` (value for {param})") -def json_for_dataclass(dataclass_instance) -> JsonableDict: +def json_for_dataclass(dataclass_instance: DataclassInstance) -> JsonableDict: """return json-serializable representation of the dataclass instance""" _field_value_pairs = ( (_field, getattr(dataclass_instance, _field.name)) @@ -160,7 +168,10 @@ def json_for_dataclass(dataclass_instance) -> JsonableDict: } -def dataclass_from_json(dataclass: type, dataclass_json: JsonableDict): +def dataclass_from_json( + dataclass: type[DataclassInstance], + dataclass_json: JsonableDict, +) -> DataclassInstance: return dataclass( **{ _field.name: field_value_from_json(_field, dataclass_json) @@ -170,8 +181,9 @@ def dataclass_from_json(dataclass: type, dataclass_json: JsonableDict): def field_value_from_json( - field: dataclasses.Field[JsonableValue], dataclass_json: JsonableDict -): + field: dataclasses.Field[JsonableValue], + dataclass_json: JsonableDict, +) -> JsonableValue: _json_value = dataclass_json.get(field.name) if _json_value is None: return None # TODO: check optional diff --git a/addon_toolkit/operation.py b/addon_toolkit/operation.py index 6ce30096..de7f6a4c 100644 --- a/addon_toolkit/operation.py +++ b/addon_toolkit/operation.py @@ -44,7 +44,7 @@ def for_function( ) -> "AddonOperationDeclaration": return addon_operation.get_declaration(fn) - def __post_init__(self): + def __post_init__(self) -> None: if self.required_return_type and not issubclass( self.return_type, self.required_return_type ): @@ -64,7 +64,7 @@ def return_type(self) -> type: return _return_type @property - def name(self): + def name(self) -> str: # TODO: language tag (kwarg for tagged string?) return self.operation_fn.__name__ diff --git a/addon_toolkit/tests/_doctest.py b/addon_toolkit/tests/_doctest.py index 60239610..8d96f7dd 100644 --- a/addon_toolkit/tests/_doctest.py +++ b/addon_toolkit/tests/_doctest.py @@ -1,7 +1,26 @@ import doctest +import typing +import unittest +from types import ModuleType -def load_doctests(*modules): +class LoadTestsFunction(typing.Protocol): + """structural type for the function expected by the "load_tests protocol" + https://docs.python.org/3/library/unittest.html#load-tests-protocol + """ + + def __call__( + _, # implicit, nonexistent "self" + loader: unittest.TestLoader, + tests: unittest.TestSuite, + pattern: str | None, + ) -> unittest.TestSuite: ... + + +def load_doctests( + *modules: ModuleType, + doctestflags: int = doctest.ELLIPSIS | doctest.NORMALIZE_WHITESPACE, +) -> LoadTestsFunction: """shorthand for unittests from doctests meant for implementing the "load_tests protocol" @@ -18,12 +37,16 @@ def load_doctests(*modules): (if there's a need, could support pass-thru kwargs to DocTestSuite) """ - def _load_tests(loader, tests, pattern): + def _load_tests( + loader: unittest.TestLoader, + tests: unittest.TestSuite, + pattern: str | None, + ) -> unittest.TestSuite: for _module in modules: tests.addTests( doctest.DocTestSuite( _module, - optionflags=doctest.ELLIPSIS | doctest.NORMALIZE_WHITESPACE, + optionflags=doctestflags, ) ) return tests diff --git a/addon_toolkit/typing.py b/addon_toolkit/typing.py new file mode 100644 index 00000000..baa68e30 --- /dev/null +++ b/addon_toolkit/typing.py @@ -0,0 +1,8 @@ +import typing + + +class AnyDataclassInstance(typing.Protocol): + __dataclass_fields__: typing.ClassVar[dict[str, typing.Any]] + + +DataclassInstance = typing.TypeVar("DataclassInstance", bound=AnyDataclassInstance) diff --git a/mypy.ini b/mypy.ini index 6d83679c..1789d58a 100644 --- a/mypy.ini +++ b/mypy.ini @@ -28,10 +28,7 @@ warn_unreachable = True # module-specific config [mypy-addon_toolkit.*] -## loosen strict: -disallow_untyped_calls = False -disallow_incomplete_defs = False -disallow_untyped_defs = False +# addon_toolkit fully typed! [mypy-addon_service.*,app.*,manage] # got untyped dependencies -- this is fine @@ -47,5 +44,6 @@ disallow_untyped_defs = False warn_return_any = False [mypy-addon_service.migrations.*] +# django migrations are whatever strict = False disable_error_code = var-annotated