From 54fa1b24d332ac8ebbead12a8bd431811b2fa74f Mon Sep 17 00:00:00 2001 From: Eli Polonsky Date: Thu, 29 Aug 2024 12:04:13 +0300 Subject: [PATCH] fix(python): user defined `__jsii_proxy_class` attributes are not preserved (#4625) In https://github.com/aws/jsii/pull/4611, we added the `_jsii_proxy_class__` attributes to the `@jsii.interface` implementations. This was required in order to comply with `typeguard` protocol checking. We didn't implement it correctly, accidentally overriding user defined proxy classes. ## Note I have been wrecking my brain trying to understand if this bug has any runtime implications, and I couldn't find any. #### How so? At runtime, from what I could gather, the `__jsii_proxy_class__` attribute is only used when we try to instantiate a subclass of an abstract class: https://github.com/aws/jsii/blob/dc77d6c7016bcb7531f6e374243410f969ea1fbf/packages/%40jsii/python-runtime/src/jsii/_reference_map.py#L65-L70 However, for abstract classes, we assign an explicit value to `__jsii_proxy_class__`: https://github.com/aws/jsii/blob/dc77d6c7016bcb7531f6e374243410f969ea1fbf/packages/jsii-pacmak/lib/targets/python.ts#L1496-L1501 Luckily, this happens **AFTER** the `@jsii.implements` decorator has finished, thus overriding the mistake in the decorator. Presumably, this would still be a problem for user defined abstract classes (since they don't have this assignment). However, reference resolving for user defined classes is done via native reference lookup: https://github.com/aws/jsii/blob/dc77d6c7016bcb7531f6e374243410f969ea1fbf/packages/%40jsii/python-runtime/src/jsii/_reference_map.py#L48-L54 This is also why I couldn't come up with a real life test case, and had to resort to an artificial one. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- .../@jsii/python-runtime/src/jsii/_runtime.py | 2 +- .../@jsii/python-runtime/tests/test_python.py | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/@jsii/python-runtime/src/jsii/_runtime.py b/packages/@jsii/python-runtime/src/jsii/_runtime.py index 35c3385300..7d7cb1b469 100644 --- a/packages/@jsii/python-runtime/src/jsii/_runtime.py +++ b/packages/@jsii/python-runtime/src/jsii/_runtime.py @@ -168,7 +168,7 @@ def implements(*interfaces: Type[Any]) -> Callable[[T], T]: def deco(cls): cls.__jsii_type__ = getattr(cls, "__jsii_type__", None) cls.__jsii_ifaces__ = getattr(cls, "__jsii_ifaces__", []) + list(interfaces) - cls.__jsii_proxy_class__ = lambda: getattr(cls, "__jsii_proxy_class__", None) + cls.__jsii_proxy_class__ = getattr(cls, "__jsii_proxy_class__", lambda: None) # https://github.com/agronholm/typeguard/issues/479 cls.__protocol_attrs__ = getattr(cls, "__protocol_attrs__", []) diff --git a/packages/@jsii/python-runtime/tests/test_python.py b/packages/@jsii/python-runtime/tests/test_python.py index 1aaa087ae6..991185f2d0 100644 --- a/packages/@jsii/python-runtime/tests/test_python.py +++ b/packages/@jsii/python-runtime/tests/test_python.py @@ -27,6 +27,37 @@ def test_inheritance_maintained(self): assert base_names == ["DerivedStruct", "MyFirstStruct"] + +class TestImplementsInterface: + + def test_jsii_proxy_class_defaults_to_none(self) -> None: + @jsii.implements(IBaz) + class MyBaz: + pass + + klass = getattr(MyBaz, "__jsii_proxy_class__")() + assert klass == None + + def test_jsii_proxy_class_preserves_user_defined_attribute(self) -> None: + + class _MyBazProxy: + def baz_method(self) -> str: + return "_MyBazProxy" + + @jsii.implements(IBaz) + class MyBaz: + + @staticmethod + def __jsii_proxy_class__(): + return _MyBazProxy + + def baz_method(self) -> str: + return "MyBaz" + + klass = getattr(MyBaz, "__jsii_proxy_class__")() + instance = klass() + assert instance.baz_method() == "_MyBazProxy" + def test_implements_interface(self) -> None: """Checks that jsii-generated classes correctly implement the relevant jsii-generated interfaces."""