From 51c2f90b8e2cb58df55473320b47a42773444b0d Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Sun, 3 Jul 2022 14:03:57 -0400 Subject: [PATCH] feat: allow callbacks as strings (#18) * feat: allow callbacks as strings * test: more tests * test: coverage --- src/app_model/registries/_commands_reg.py | 30 ++++++++- src/app_model/types/_action.py | 23 +++++-- src/app_model/types/_utils.py | 46 ++++++++++++++ tests/conftest.py | 75 +++++++++++++++++++---- tests/fixtures/fake_module.py | 11 ++++ tests/test_app.py | 38 ++++++++++++ tests/test_types.py | 16 ++++- 7 files changed, 219 insertions(+), 20 deletions(-) create mode 100644 src/app_model/types/_utils.py create mode 100644 tests/fixtures/fake_module.py diff --git a/src/app_model/registries/_commands_reg.py b/src/app_model/registries/_commands_reg.py index 2ef5152..b552de3 100644 --- a/src/app_model/registries/_commands_reg.py +++ b/src/app_model/registries/_commands_reg.py @@ -2,7 +2,7 @@ from concurrent.futures import Future, ThreadPoolExecutor from functools import cached_property -from typing import TYPE_CHECKING, Any, Callable +from typing import TYPE_CHECKING, Any, Callable, Union from psygnal import Signal @@ -12,7 +12,7 @@ from ..types import CommandIdStr DisposeCallable = Callable[[], None] - CommandCallable = TypeVar("CommandCallable", bound=Callable) + CommandCallable = TypeVar("CommandCallable", bound=Union[Callable, str]) class _RegisteredCommand: @@ -28,12 +28,36 @@ def __init__(self, id: CommandIdStr, callback: CommandCallable, title: str) -> N self.id = id self.callback = callback self.title = title + self._resolved_callback = callback if callable(callback) else None + + @property + def resolved_callback(self) -> Callable: + if self._resolved_callback is None: + from ..types._utils import import_python_name + + try: + self._resolved_callback = import_python_name(str(self.callback)) + except ImportError as e: + self._resolved_callback = lambda *a, **k: None + raise type(e)( + f"Command pointer {self.callback!r} registered for Command " + f"{self.id!r} was not importable: {e}" + ) from e + + if not callable(self._resolved_callback): + # don't try to import again, just create a no-op + self._resolved_callback = lambda *a, **k: None + raise TypeError( + f"Command pointer {self.callback!r} registered for Command " + f"{self.id!r} did not resolve to a callble object." + ) + return self._resolved_callback @cached_property def run_injected(self) -> Callable: # from .._injection import inject_dependencies # return inject_dependencies(self.run) - return self.callback + return self.resolved_callback class CommandsRegistry: diff --git a/src/app_model/types/_action.py b/src/app_model/types/_action.py index 5540fd5..0bcd9fe 100644 --- a/src/app_model/types/_action.py +++ b/src/app_model/types/_action.py @@ -1,11 +1,12 @@ -from typing import Callable, Generic, List, Optional, TypeVar +from typing import Callable, Generic, List, Optional, TypeVar, Union -from pydantic import Field +from pydantic import Field, validator from typing_extensions import ParamSpec from ._command_rule import CommandRule from ._keybinding_rule import KeyBindingRule from ._menu import MenuRule +from ._utils import _validate_python_name P = ParamSpec("P") R = TypeVar("R") @@ -20,10 +21,13 @@ class Action(CommandRule, Generic[P, R]): `register_action`. """ - # TODO: this could also be a string - callback: Callable[P, R] = Field( + callback: Union[Callable[P, R], str] = Field( ..., - description="A function to call when the associated command id is executed.", + description="A function to call when the associated command id is executed. " + "If a string is provided, it must be a fully qualified name to a callable " + "python object. This usually takes the form of " + "`{obj.__module__}:{obj.__qualname__}` " + "(e.g. `my_package.a_module:some_function`)", ) menus: Optional[List[MenuRule]] = Field( None, @@ -38,3 +42,12 @@ class Action(CommandRule, Generic[P, R]): description="Whether to add this command to the global Command Palette " "during registration.", ) + + @validator("callback") + def _validate_callback(callback: object) -> Union[Callable, str]: + """Assert that `callback` is a callable or valid fully qualified name.""" + if callable(callback): + return callback + elif isinstance(callback, str): + return _validate_python_name(str(callback)) + raise TypeError("callback must be a callable or a string") # pragma: no cover diff --git a/src/app_model/types/_utils.py b/src/app_model/types/_utils.py new file mode 100644 index 0000000..459d5ac --- /dev/null +++ b/src/app_model/types/_utils.py @@ -0,0 +1,46 @@ +import re +from importlib import import_module +from typing import Any + +_identifier_plus_dash = "(?:[a-zA-Z_][a-zA-Z_0-9-]+)" +_dotted_name = f"(?:(?:{_identifier_plus_dash}\\.)*{_identifier_plus_dash})" +PYTHON_NAME_PATTERN = re.compile(f"^({_dotted_name}):({_dotted_name})$") + + +def _validate_python_name(name: str) -> str: + """Assert that `name` is a valid python name: e.g. `module.submodule:funcname`.""" + if name and not PYTHON_NAME_PATTERN.match(name): + msg = ( + f"{name!r} is not a valid python_name. A python_name must " + "be of the form '{obj.__module__}:{obj.__qualname__}' (e.g. " + "'my_package.a_module:some_function')." + ) + if ".." in name: # pragma: no cover + *_, a, b = name.split("..") + a = a.split(":")[-1] + msg += ( + " Note: functions defined in local scopes are not yet supported. " + f"Please move function {b!r} to the global scope of module {a!r}" + ) + raise ValueError(msg) + return name + + +def import_python_name(python_name: str) -> Any: + """Import object from a fully qualified python name. + + Examples + -------- + >>> import_python_name("my_package.a_module:some_function") + + >>> import_python_name('pydantic:BaseModel') + + """ + _validate_python_name(python_name) # shows the best error message + if match := PYTHON_NAME_PATTERN.match(python_name): + module_name, funcname = match.groups() + mod = import_module(module_name) + return getattr(mod, funcname) + raise ValueError( # pragma: no cover + f"Could not parse python_name: {python_name!r}" + ) diff --git a/tests/conftest.py b/tests/conftest.py index 6001c51..5b62c2f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,5 @@ +import sys +from pathlib import Path from unittest.mock import Mock import pytest @@ -12,6 +14,8 @@ except ImportError: UNDO_ICON = "fa5s.undo" +FIXTURES = Path(__file__).parent / "fixtures" + class Menus: FILE = "file" @@ -28,18 +32,45 @@ class Commands: PASTE = "paste" OPEN_FROM_A = "open.from_a" OPEN_FROM_B = "open.from_b" + UNIMPORTABLE = "unimportable" + NOT_CALLABLE = "not.callable" + RAISES = "raises.error" + + +def _raise_an_error(): + raise ValueError("This is an error") class Mocks: def __init__(self) -> None: - self.open = Mock(name=Commands.OPEN, side_effect=lambda: print("open")) - self.undo = Mock(name=Commands.UNDO, side_effect=lambda: print("undo")) - self.redo = Mock(name=Commands.REDO, side_effect=lambda: print("redo")) - self.copy = Mock(name=Commands.COPY, side_effect=lambda: print("copy")) - self.paste = Mock(name=Commands.PASTE, side_effect=lambda: print("paste")) + self.open = Mock(name=Commands.OPEN) + self.undo = Mock(name=Commands.UNDO) + self.copy = Mock(name=Commands.COPY) + self.paste = Mock(name=Commands.PASTE) self.open_from_a = Mock(name=Commands.OPEN_FROM_A) self.open_from_b = Mock(name=Commands.OPEN_FROM_B) + @property + def redo(self) -> Mock: + """This tests that we can lazily import a callback. + + There is a function called `run_me` in fixtures/fake_module.py that calls the + global mock in that module. In the redo action below, we declare: + `callback="fake_module:run_me"` + + So, whenever the redo action is triggered, it should import that module, and + then call the mock. We can also access it here at `mocks.redo`... but the + fixtures directory must be added to sys path during the test (as we do below) + """ + try: + from fake_module import GLOBAL_MOCK # noqa + + return GLOBAL_MOCK + except ImportError as e: + raise ImportError( + "This mock must be run with the fixutres directory added to sys.path." + ) from e + class FullApp(Application): Menus = Menus @@ -50,8 +81,8 @@ def __init__(self, name: str) -> None: self.mocks = Mocks() -def build_app() -> Application: - app = FullApp("complete_test_app") +def build_app(name: str = "complete_test_app") -> FullApp: + app = FullApp(name) app.menus.append_menu_items( [ ( @@ -99,7 +130,7 @@ def build_app() -> Application: tooltip="Redo it!", icon="fa5s.redo", enablement="allow_undo_redo", - callback=app.mocks.redo, + callback="fake_module:run_me", # this is a function in fixtures keybindings=[{"primary": "Ctrl+Shift+Z"}], menus=[ { @@ -147,6 +178,21 @@ def build_app() -> Application: callback=app.mocks.open_from_b, menus=[{"id": Menus.FILE_OPEN_FROM}], ), + Action( + id=Commands.UNIMPORTABLE, + title="Can't be found", + callback="unresolvable:function", + ), + Action( + id=Commands.NOT_CALLABLE, + title="Will Never Work", + callback="fake_module:attr", + ), + Action( + id=Commands.RAISES, + title="Will raise an error", + callback=_raise_an_error, + ), ] for action in actions: app.register_action(action) @@ -155,10 +201,17 @@ def build_app() -> Application: @pytest.fixture -def full_app() -> Application: +def full_app(monkeypatch) -> Application: """Premade application.""" - app = build_app() try: - yield app + app = build_app() + with monkeypatch.context() as m: + # mock path to add fake_module + m.setattr(sys, "path", [str(FIXTURES)] + sys.path) + # make sure it's not already in sys.modules + sys.modules.pop("fake_module", None) + yield app + # clear the global mock if it's been called + app.mocks.redo.reset_mock() finally: Application.destroy("complete_test_app") diff --git a/tests/fixtures/fake_module.py b/tests/fixtures/fake_module.py new file mode 100644 index 0000000..88b05a8 --- /dev/null +++ b/tests/fixtures/fake_module.py @@ -0,0 +1,11 @@ +from unittest.mock import Mock + +GLOBAL_MOCK = Mock(name="GLOBAL") + + +def run_me() -> bool: + GLOBAL_MOCK() + return True + + +attr = "not a callble" diff --git a/tests/test_app.py b/tests/test_app.py index 4bc8219..4a352e1 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1,5 +1,6 @@ from __future__ import annotations +import sys from typing import TYPE_CHECKING import pytest @@ -43,3 +44,40 @@ def test_sorting(full_app: FullApp): assert [i.command.title for i in g1] == ["Undo", "Redo"] assert [i.command.title for i in g2] == ["Copy", "Paste"] + + +def test_action_import_by_string(full_app: FullApp): + """the REDO command is declared as a string in the conftest.py file + + This tests that it can be lazily imported at callback runtime and executed + """ + assert "fake_module" not in sys.modules + assert full_app.commands.execute_command(full_app.Commands.REDO).result() + assert "fake_module" in sys.modules + full_app.mocks.redo.assert_called_once() + + # tests what happens when the module cannot be found + with pytest.raises( + ModuleNotFoundError, match="Command 'unimportable' was not importable" + ): + full_app.commands.execute_command(full_app.Commands.UNIMPORTABLE) + # the second time we try within a session, nothing should happen + full_app.commands.execute_command(full_app.Commands.UNIMPORTABLE) + + # tests what happens when the object is not callable cannot be found + with pytest.raises( + TypeError, + match="Command 'not.callable' did not resolve to a callble object", + ): + full_app.commands.execute_command(full_app.Commands.NOT_CALLABLE) + # the second time we try within a session, nothing should happen + full_app.commands.execute_command(full_app.Commands.NOT_CALLABLE) + + +def test_action_raises_exception(full_app: FullApp): + result = full_app.commands.execute_command(full_app.Commands.RAISES) + with pytest.raises(ValueError): + result.result() + + # the function that raised the exception is `_raise_an_error` in conftest.py + assert str(result.exception()) == "This is an error" diff --git a/tests/test_types.py b/tests/test_types.py index 6c91358..4c1ae50 100644 --- a/tests/test_types.py +++ b/tests/test_types.py @@ -1,7 +1,21 @@ -from app_model.types import Icon +import pytest +from pydantic import ValidationError + +from app_model.types import Action, Icon def test_icon_validate(): assert Icon.validate('"fa5s.arrow_down"') == Icon( dark='"fa5s.arrow_down"', light='"fa5s.arrow_down"' ) + + +def test_action_validation(): + with pytest.raises(ValidationError, match="'s!adf' is not a valid python_name"): + Action(id="test", title="test", callback="s!adf") + + with pytest.raises(ValidationError): + Action(id="test", title="test", callback=list()) + + with pytest.raises(ValidationError, match="'x.:asdf' is not a valid"): + Action(id="test", title="test", callback="x.:asdf")