diff --git a/docs/web3.main.rst b/docs/web3.main.rst index 3feeefb684..f85840c62d 100644 --- a/docs/web3.main.rst +++ b/docs/web3.main.rst @@ -419,17 +419,13 @@ web3.py library. External Modules ~~~~~~~~~~~~~~~~ -External modules can be used to introduce custom or third-party APIs to your ``Web3`` instance. Adding external modules -can occur either at instantiation of the ``Web3`` instance or by making use of the ``attach_modules()`` method. - -Unlike the native modules, external modules need not inherit from the ``web3.module.Module`` class. The only requirement -is that a Module must be a class and, if you'd like to make use of the parent ``Web3`` instance, it must be passed into -the ``__init__`` function. For example: +External modules can be used to introduce custom or third-party APIs to your ``Web3`` instance. External modules are simply +classes whose methods and properties can be made available within the ``Web3`` instance. Optionally, the external module may +make use of the parent ``Web3`` instance by accepting it as the first argument within the ``__init__`` function: .. code-block:: python - >>> class ExampleModule(): - ... + >>> class ExampleModule: ... def __init__(self, w3): ... self.w3 = w3 ... @@ -440,7 +436,9 @@ the ``__init__`` function. For example: .. warning:: Given the flexibility of external modules, use caution and only import modules from trusted third parties and open source code you've vetted! -To instantiate the ``Web3`` instance with external modules: +Configuring external modules can occur either at instantiation of the ``Web3`` instance or by making use of the +``attach_modules()`` method. To instantiate the ``Web3`` instance with external modules use the ``external_modules`` +keyword argument: .. code-block:: python @@ -466,11 +464,11 @@ To instantiate the ``Web3`` instance with external modules: ... ) # `return_zero`, in this case, is an example attribute of the `ModuleClass1` object - >>> w3.module1.return_zero + >>> w3.module1.return_zero() 0 - >>> w3.module2.submodule1.return_one + >>> w3.module2.submodule1.return_one() 1 - >>> w3.module2.submodule2.submodule2a.return_two + >>> w3.module2.submodule2.submodule2a.return_two() 2 @@ -504,9 +502,9 @@ To instantiate the ``Web3`` instance with external modules: ... }) ... }) ... }) - >>> w3.module1.return_zero + >>> w3.module1.return_zero() 0 - >>> w3.module2.submodule1.return_one + >>> w3.module2.submodule1.return_one() 1 - >>> w3.module2.submodule2.submodule2a.return_two + >>> w3.module2.submodule2.submodule2a.return_two() 2 diff --git a/newsfragments/2328.bugfix.rst b/newsfragments/2328.bugfix.rst new file mode 100644 index 0000000000..ef7bbad1d1 --- /dev/null +++ b/newsfragments/2328.bugfix.rst @@ -0,0 +1 @@ +Properly initialize external modules that do not inherit from the ``web3.module.Module`` class \ No newline at end of file diff --git a/tests/core/conftest.py b/tests/core/conftest.py index cdaa362fce..6947f8d334 100644 --- a/tests/core/conftest.py +++ b/tests/core/conftest.py @@ -48,31 +48,52 @@ class Module4(Module): @pytest.fixture(scope='module') def module1_unique(): + # uses ``Web3`` instance by accepting it as first arg in the ``__init__()`` method class Module1: a = 'a' + + def __init__(self, w3): + self._b = "b" + self.w3 = w3 + + def b(self): + return self._b + + @property + def return_eth_chain_id(self): + return self.w3.eth.chain_id return Module1 @pytest.fixture(scope='module') def module2_unique(): class Module2: - b = 'b' + c = 'c' @staticmethod - def c(): - return 'c' + def d(): + return 'd' return Module2 @pytest.fixture(scope='module') def module3_unique(): class Module3: - d = 'd' + e = 'e' return Module3 @pytest.fixture(scope='module') def module4_unique(): class Module4: - e = 'e' + f = 'f' return Module4 + + +@pytest.fixture(scope='module') +def module_many_init_args(): + class ModuleManyArgs: + def __init__(self, a, b): + self.a = a + self.b = b + return ModuleManyArgs diff --git a/tests/core/utilities/test_attach_modules.py b/tests/core/utilities/test_attach_modules.py index 60914dc21f..4d151cc73a 100644 --- a/tests/core/utilities/test_attach_modules.py +++ b/tests/core/utilities/test_attach_modules.py @@ -1,3 +1,6 @@ +from io import ( + UnsupportedOperation, +) import pytest from eth_utils import ( @@ -147,18 +150,34 @@ def test_attach_external_modules_that_do_not_inherit_from_module_class( # assert module1 attached assert hasattr(w3, 'module1') assert w3.module1.a == 'a' + assert w3.module1.b() == 'b' + assert w3.module1.return_eth_chain_id == w3.eth.chain_id # assert module2 + submodules attached assert hasattr(w3, 'module2') - assert w3.module2.b == 'b' - assert w3.module2.c() == 'c' + assert w3.module2.c == 'c' + assert w3.module2.d() == 'd' assert hasattr(w3.module2, 'submodule1') - assert w3.module2.submodule1.d == 'd' + assert w3.module2.submodule1.e == 'e' assert hasattr(w3.module2.submodule1, 'submodule2') - assert w3.module2.submodule1.submodule2.e == 'e' + assert w3.module2.submodule1.submodule2.f == 'f' # assert default modules intact assert hasattr(w3, 'geth') assert hasattr(w3, 'eth') assert is_integer(w3.eth.chain_id) + + +def test_attach_modules_for_module_with_more_than_one_init_argument(web3, module_many_init_args): + with pytest.raises( + UnsupportedOperation, + match=( + "A module class may accept a single `Web3` instance as the first argument of its " + "__init__\\(\\) method. More than one argument found for ModuleManyArgs: \\['a', 'b']" + ) + ): + Web3( + EthereumTesterProvider(), + external_modules={'module_should_fail': module_many_init_args} + ) diff --git a/tests/core/web3-module/test_attach_modules.py b/tests/core/web3-module/test_attach_modules.py index 58b3d5aca3..35f575cb06 100644 --- a/tests/core/web3-module/test_attach_modules.py +++ b/tests/core/web3-module/test_attach_modules.py @@ -1,3 +1,8 @@ +from io import ( + UnsupportedOperation, +) +import pytest + from eth_utils import ( is_integer, ) @@ -51,18 +56,31 @@ def test_attach_modules_that_do_not_inherit_from_module_class( # assert module1 attached assert hasattr(web3, 'module1') assert web3.module1.a == 'a' + assert web3.module1.b() == 'b' + assert web3.module1.return_eth_chain_id == web3.eth.chain_id # assert module2 + submodules attached assert hasattr(web3, 'module2') - assert web3.module2.b == 'b' - assert web3.module2.c() == 'c' + assert web3.module2.c == 'c' + assert web3.module2.d() == 'd' assert hasattr(web3.module2, 'submodule1') - assert web3.module2.submodule1.d == 'd' + assert web3.module2.submodule1.e == 'e' assert hasattr(web3.module2.submodule1, 'submodule2') - assert web3.module2.submodule1.submodule2.e == 'e' + assert web3.module2.submodule1.submodule2.f == 'f' # assert default modules intact assert hasattr(web3, 'geth') assert hasattr(web3, 'eth') assert is_integer(web3.eth.chain_id) + + +def test_attach_modules_for_module_with_more_than_one_init_argument(web3, module_many_init_args): + with pytest.raises( + UnsupportedOperation, + match=( + "A module class may accept a single `Web3` instance as the first argument of its " + "__init__\\(\\) method. More than one argument found for ModuleManyArgs: \\['a', 'b']" + ) + ): + web3.attach_modules({'module_should_fail': module_many_init_args}) diff --git a/web3/_utils/module.py b/web3/_utils/module.py index 89142583de..7bba1a86db 100644 --- a/web3/_utils/module.py +++ b/web3/_utils/module.py @@ -1,7 +1,12 @@ +import inspect +from io import ( + UnsupportedOperation, +) from typing import ( TYPE_CHECKING, Any, Dict, + List, Optional, Sequence, Union, @@ -18,6 +23,22 @@ from web3 import Web3 # noqa: F401 +def _validate_init_params_and_return_if_found(module_class: Any) -> List[str]: + init_params_raw = list(inspect.signature(module_class.__init__).parameters) + module_init_params = [ + param for param in init_params_raw if param not in ['self', 'args', 'kwargs'] + ] + + if len(module_init_params) > 1: + raise UnsupportedOperation( + "A module class may accept a single `Web3` instance as the first argument of its " + f"__init__() method. More than one argument found for {module_class.__name__}: " + f"{module_init_params}" + ) + + return module_init_params + + def attach_modules( parent_module: Union["Web3", "Module"], module_definitions: Dict[str, Any], @@ -34,18 +55,20 @@ def attach_modules( "already has an attribute with that name" ) - if issubclass(module_class, Module): - # If the `module_class` inherits from the `web3.module.Module` class, it has access to - # caller functions internal to the web3.py library and sets up a proper codec. This - # is likely important for all modules internal to the library. - if w3 is None: - setattr(parent_module, module_name, module_class(parent_module)) - w3 = parent_module - else: - setattr(parent_module, module_name, module_class(w3)) + # The parent module is the ``Web3`` instance on first run of the loop + if type(parent_module).__name__ == 'Web3': + w3 = parent_module + + module_init_params = _validate_init_params_and_return_if_found(module_class) + if len(module_init_params) == 1: + # Modules that need access to the ``Web3`` instance may accept the instance as the first + # arg in their ``__init__()`` method. This is the case for any module that inherits from + # ``web3.module.Module``. + # e.g. def __init__(self, w3): + setattr(parent_module, module_name, module_class(w3)) else: - # An external `module_class` need not inherit from the `web3.module.Module` class. - setattr(parent_module, module_name, module_class) + # Modules need not take in a ``Web3`` instance in their ``__init__()`` if not needed + setattr(parent_module, module_name, module_class()) if module_info_is_list_like: if len(module_info) == 2: