From d1dbeb44fcccff3573c48c26d7a01d1b561af107 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 7 Mar 2023 08:57:37 -0500 Subject: [PATCH 1/8] Common: Handle default arguments in DIContainer --- monkey/common/di_container.py | 17 +++++++++++--- .../unit_tests/common/test_di_container.py | 22 +++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/monkey/common/di_container.py b/monkey/common/di_container.py index eb5361769ba..fe6eeedfbed 100644 --- a/monkey/common/di_container.py +++ b/monkey/common/di_container.py @@ -111,8 +111,8 @@ def __init__(self, hostname: str): def resolve(self, type_: Type[T]) -> T: """ Resolves all dependencies and returns a new instance of `type_` using constructor dependency - injection. Note that only positional arguments are resolved. Varargs, keyword-only args, and - default values are ignored. + injection. Note that only positional arguments or arguments with defaults are resolved. + Varargs and keyword-only args are ignored. Dependencies are resolved with the following precedence @@ -147,7 +147,13 @@ def resolve_dependencies(self, type_: Type[T]) -> Sequence[Any]: try: instance = self._resolve_convention(arg_type, arg_name) except UnregisteredConventionError: - instance = self._resolve_type(arg_type) + try: + instance = self._resolve_type(arg_type) + except UnresolvableDependencyError as err: + if DIContainer._has_default_argument(type_, arg_name): + continue + + raise err args.append(instance) @@ -183,6 +189,11 @@ def _construct_new_instance(self, arg_type: Type[T]) -> T: def _retrieve_registered_instance(self, arg_type: Type[T]) -> T: return self._instance_registry[arg_type] + @staticmethod + def _has_default_argument(type_: Type[T], arg_name: str) -> bool: + parameters = inspect.signature(type_).parameters + return parameters[arg_name].default is not inspect.Parameter.empty + def release(self, interface: Type[T]): """ Deregister an interface diff --git a/monkey/tests/unit_tests/common/test_di_container.py b/monkey/tests/unit_tests/common/test_di_container.py index 857168f82aa..4163d1f2987 100644 --- a/monkey/tests/unit_tests/common/test_di_container.py +++ b/monkey/tests/unit_tests/common/test_di_container.py @@ -374,3 +374,25 @@ def test_release_convention(container): with pytest.raises(ValueError): container.release_convention(str, "my_str") container.resolve(TestClass6) + + +class Dependency: + def __init__(self, my_int=42): + self.my_int = my_int + + +class HasDefault: + def __init__(self, dependency: Dependency = Dependency(99)): + self.dependency = dependency + + +def test_handle_default_parameter__no_dependency_registered(container): + has_default = container.resolve(HasDefault) + assert has_default.dependency.my_int == 99 + + +def test_handle_default_parameter__dependency_registered(container): + container.register(Dependency, Dependency) + + has_default = container.resolve(HasDefault) + assert has_default.dependency.my_int == 42 From 47e325561fe6f9d8b7b14623b1e38246b72188d4 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 7 Mar 2023 08:59:20 -0500 Subject: [PATCH 2/8] Common: Flatten logic in DIContainer.resolve_dependencies() --- monkey/common/di_container.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/monkey/common/di_container.py b/monkey/common/di_container.py index fe6eeedfbed..bac6d9ca8e0 100644 --- a/monkey/common/di_container.py +++ b/monkey/common/di_container.py @@ -144,18 +144,17 @@ def resolve_dependencies(self, type_: Type[T]) -> Sequence[Any]: args = [] for arg_name, arg_type in inspect.getfullargspec(type_).annotations.items(): - try: - instance = self._resolve_convention(arg_type, arg_name) - except UnregisteredConventionError: - try: - instance = self._resolve_type(arg_type) - except UnresolvableDependencyError as err: - if DIContainer._has_default_argument(type_, arg_name): - continue + with suppress(UnregisteredConventionError): + args.append(self._resolve_convention(arg_type, arg_name)) + continue - raise err + try: + args.append(self._resolve_type(arg_type)) + except UnresolvableDependencyError as err: + if DIContainer._has_default_argument(type_, arg_name): + continue - args.append(instance) + raise err return tuple(args) From e28044b0532be887a47f9952c014a1935265239c Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 7 Mar 2023 09:11:54 -0500 Subject: [PATCH 3/8] Common: Improve logic in DIContainer.resolve_dependencies() It's cleaner to use the inspect.Parameter class to evaluate parameters as opposed to inspect.FullArgSpec. --- monkey/common/di_container.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/monkey/common/di_container.py b/monkey/common/di_container.py index bac6d9ca8e0..d134eaef26c 100644 --- a/monkey/common/di_container.py +++ b/monkey/common/di_container.py @@ -143,15 +143,16 @@ def resolve_dependencies(self, type_: Type[T]) -> Sequence[Any]: """ args = [] - for arg_name, arg_type in inspect.getfullargspec(type_).annotations.items(): + for parameter in inspect.signature(type_).parameters.values(): with suppress(UnregisteredConventionError): - args.append(self._resolve_convention(arg_type, arg_name)) + args.append(self._resolve_convention(parameter.annotation, parameter.name)) continue try: - args.append(self._resolve_type(arg_type)) + args.append(self._resolve_type(parameter.annotation)) except UnresolvableDependencyError as err: - if DIContainer._has_default_argument(type_, arg_name): + if parameter.default is not inspect.Parameter.empty: + # Default value will be used to construct the object. No need to add it to args. continue raise err @@ -188,11 +189,6 @@ def _construct_new_instance(self, arg_type: Type[T]) -> T: def _retrieve_registered_instance(self, arg_type: Type[T]) -> T: return self._instance_registry[arg_type] - @staticmethod - def _has_default_argument(type_: Type[T], arg_name: str) -> bool: - parameters = inspect.signature(type_).parameters - return parameters[arg_name].default is not inspect.Parameter.empty - def release(self, interface: Type[T]): """ Deregister an interface From 3a69d6d20ee0e71640f00a4b06e6d07ee8ad4411 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 7 Mar 2023 09:20:18 -0500 Subject: [PATCH 4/8] Common: Improve default handling in DIContainer --- monkey/common/di_container.py | 7 +++---- .../tests/unit_tests/common/test_di_container.py | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/monkey/common/di_container.py b/monkey/common/di_container.py index d134eaef26c..c0dbee5dc67 100644 --- a/monkey/common/di_container.py +++ b/monkey/common/di_container.py @@ -151,11 +151,10 @@ def resolve_dependencies(self, type_: Type[T]) -> Sequence[Any]: try: args.append(self._resolve_type(parameter.annotation)) except UnresolvableDependencyError as err: - if parameter.default is not inspect.Parameter.empty: - # Default value will be used to construct the object. No need to add it to args. - continue + if parameter.default is inspect.Parameter.empty: + raise err - raise err + args.append(parameter.default) return tuple(args) diff --git a/monkey/tests/unit_tests/common/test_di_container.py b/monkey/tests/unit_tests/common/test_di_container.py index 4163d1f2987..476ed703b70 100644 --- a/monkey/tests/unit_tests/common/test_di_container.py +++ b/monkey/tests/unit_tests/common/test_di_container.py @@ -396,3 +396,17 @@ def test_handle_default_parameter__dependency_registered(container): has_default = container.resolve(HasDefault) assert has_default.dependency.my_int == 42 + + +def test_handle_default_parameter__skip_default(container): + class HasDefault_2_Parameters: + def __init__(self, dependency: Dependency = Dependency(99), my_str: str = "hello"): + self.dependency = dependency + self.my_str = my_str + + container.register_instance(str, "goodbye") + + has_default = container.resolve(HasDefault_2_Parameters) + + assert has_default.dependency.my_int == 99 + assert has_default.my_str == "goodbye" From 0439d63bc3c80b97959b8aa1c3fae9f1cd6f7691 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 7 Mar 2023 09:28:37 -0500 Subject: [PATCH 5/8] Common: Extract method DIContainer._resolve_parameter --- monkey/common/di_container.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/monkey/common/di_container.py b/monkey/common/di_container.py index c0dbee5dc67..ff9f38f259d 100644 --- a/monkey/common/di_container.py +++ b/monkey/common/di_container.py @@ -148,13 +148,7 @@ def resolve_dependencies(self, type_: Type[T]) -> Sequence[Any]: args.append(self._resolve_convention(parameter.annotation, parameter.name)) continue - try: - args.append(self._resolve_type(parameter.annotation)) - except UnresolvableDependencyError as err: - if parameter.default is inspect.Parameter.empty: - raise err - - args.append(parameter.default) + args.append(self._resolve_parameter(parameter)) return tuple(args) @@ -167,6 +161,15 @@ def _resolve_convention(self, type_: Type[T], name: str) -> T: f"Failed to resolve unregistered convention {convention_identifier}" ) + def _resolve_parameter(self, parameter: inspect.Parameter) -> Any: + try: + return self._resolve_type(parameter.annotation) + except UnresolvableDependencyError as err: + if parameter.default is inspect.Parameter.empty: + raise err + + return parameter.default + def _resolve_type(self, type_: Type[T]) -> T: if type_ in self._type_registry: return self._construct_new_instance(type_) From 5d3779d420986b442b6e19b1fb8c5ce07cd039a2 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 7 Mar 2023 09:51:41 -0500 Subject: [PATCH 6/8] Common: Remove DIContainer._resolve_parameter() --- monkey/common/di_container.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/monkey/common/di_container.py b/monkey/common/di_container.py index ff9f38f259d..df4e79a1ab5 100644 --- a/monkey/common/di_container.py +++ b/monkey/common/di_container.py @@ -124,7 +124,7 @@ def resolve(self, type_: Type[T]) -> T: :raises UnresolvableDependencyError: If any dependencies could not be successfully resolved """ with suppress(UnresolvableDependencyError): - return self._resolve_type(type_) + return self._resolve_type(type_, inspect.Parameter.empty) args = self.resolve_dependencies(type_) return type_(*args) @@ -148,7 +148,7 @@ def resolve_dependencies(self, type_: Type[T]) -> Sequence[Any]: args.append(self._resolve_convention(parameter.annotation, parameter.name)) continue - args.append(self._resolve_parameter(parameter)) + args.append(self._resolve_type(parameter.annotation, parameter.default)) return tuple(args) @@ -161,21 +161,16 @@ def _resolve_convention(self, type_: Type[T], name: str) -> T: f"Failed to resolve unregistered convention {convention_identifier}" ) - def _resolve_parameter(self, parameter: inspect.Parameter) -> Any: - try: - return self._resolve_type(parameter.annotation) - except UnresolvableDependencyError as err: - if parameter.default is inspect.Parameter.empty: - raise err - - return parameter.default - - def _resolve_type(self, type_: Type[T]) -> T: + def _resolve_type(self, type_: Type[T], default: T) -> T: if type_ in self._type_registry: return self._construct_new_instance(type_) - elif type_ in self._instance_registry: + + if type_ in self._instance_registry: return self._retrieve_registered_instance(type_) + if default is not inspect.Parameter.empty: + return default + raise UnresolvableDependencyError( f'Failed to resolve unregistered type "{DIContainer._format_type_name(type)}"' ) From 2c75d178f60447f10cce43f86af07cf8c57305e6 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 7 Mar 2023 10:12:40 -0500 Subject: [PATCH 7/8] Common: Extract method DIContainer._resolve_default() --- monkey/common/di_container.py | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/monkey/common/di_container.py b/monkey/common/di_container.py index df4e79a1ab5..f6daed5c4e9 100644 --- a/monkey/common/di_container.py +++ b/monkey/common/di_container.py @@ -124,7 +124,7 @@ def resolve(self, type_: Type[T]) -> T: :raises UnresolvableDependencyError: If any dependencies could not be successfully resolved """ with suppress(UnresolvableDependencyError): - return self._resolve_type(type_, inspect.Parameter.empty) + return self._resolve_type(type_) args = self.resolve_dependencies(type_) return type_(*args) @@ -148,7 +148,18 @@ def resolve_dependencies(self, type_: Type[T]) -> Sequence[Any]: args.append(self._resolve_convention(parameter.annotation, parameter.name)) continue - args.append(self._resolve_type(parameter.annotation, parameter.default)) + with suppress(UnresolvableDependencyError): + args.append(self._resolve_type(parameter.annotation)) + continue + + with suppress(UnresolvableDependencyError): + args.append(self._resolve_default(parameter)) + continue + + raise UnresolvableDependencyError( + f"Failed to resolve dependency {parameter.name} of type " + f"{DIContainer._format_type_name(parameter.annotation)}" + ) return tuple(args) @@ -161,16 +172,13 @@ def _resolve_convention(self, type_: Type[T], name: str) -> T: f"Failed to resolve unregistered convention {convention_identifier}" ) - def _resolve_type(self, type_: Type[T], default: T) -> T: + def _resolve_type(self, type_: Type[T]) -> T: if type_ in self._type_registry: return self._construct_new_instance(type_) if type_ in self._instance_registry: return self._retrieve_registered_instance(type_) - if default is not inspect.Parameter.empty: - return default - raise UnresolvableDependencyError( f'Failed to resolve unregistered type "{DIContainer._format_type_name(type)}"' ) @@ -186,6 +194,14 @@ def _construct_new_instance(self, arg_type: Type[T]) -> T: def _retrieve_registered_instance(self, arg_type: Type[T]) -> T: return self._instance_registry[arg_type] + def _resolve_default(self, parameter: inspect.Parameter) -> Any: + if parameter.default is not inspect.Parameter.empty: + return parameter.default + + raise UnresolvableDependencyError( + f'No default found for "{parameter.name}:{DIContainer._format_type_name(type)}"' + ) + def release(self, interface: Type[T]): """ Deregister an interface From cb20874b0736ea0890f9317afabdd0702fdee553 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 7 Mar 2023 10:16:43 -0500 Subject: [PATCH 8/8] Common: Change parameters to DiContainer._resolve_default() --- monkey/common/di_container.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/monkey/common/di_container.py b/monkey/common/di_container.py index f6daed5c4e9..898295dcd75 100644 --- a/monkey/common/di_container.py +++ b/monkey/common/di_container.py @@ -153,7 +153,7 @@ def resolve_dependencies(self, type_: Type[T]) -> Sequence[Any]: continue with suppress(UnresolvableDependencyError): - args.append(self._resolve_default(parameter)) + args.append(self._resolve_default(parameter.name, parameter.default)) continue raise UnresolvableDependencyError( @@ -194,13 +194,11 @@ def _construct_new_instance(self, arg_type: Type[T]) -> T: def _retrieve_registered_instance(self, arg_type: Type[T]) -> T: return self._instance_registry[arg_type] - def _resolve_default(self, parameter: inspect.Parameter) -> Any: - if parameter.default is not inspect.Parameter.empty: - return parameter.default + def _resolve_default(self, name: str, default: T) -> T: + if default is not inspect.Parameter.empty: + return default - raise UnresolvableDependencyError( - f'No default found for "{parameter.name}:{DIContainer._format_type_name(type)}"' - ) + raise UnresolvableDependencyError(f'No default found for "{name}"') def release(self, interface: Type[T]): """