feat: allow callbacks as strings (#18)

* feat: allow callbacks as strings

* test: more tests

* test: coverage

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:

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

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 ".<locals>." in name:  # pragma: no cover
+            *_, a, b = name.split(".<locals>.")
+            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")
+    <function some_function at 0x...>
+    >>> import_python_name('pydantic:BaseModel')
+    <class 'pydantic.main.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}"
+    )

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")

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"

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"

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.<locals>:asdf' is not a valid"):
+        Action(id="test", title="test", callback="x.<locals>:asdf")