feat: cache qactions [wip] (#35)

* feat: cache qactions [wip]

* test: fix test

* feat: add findAction and test

* fix: fix for pyqt

* fix: change destroyed

src/app_model/_app.py

@@ -1,9 +1,9 @@
 from __future__ import annotations
 
-import contextlib
 from typing import TYPE_CHECKING, ClassVar, Dict, List, Tuple
 
 import in_n_out as ino
+from psygnal import Signal
 
 from .registries import (
     CommandsRegistry,
@@ -20,6 +20,7 @@
 class Application:
     """Full application model."""
 
+    destroyed = Signal(str)
     _instances: ClassVar[Dict[str, Application]] = {}
 
     def __init__(self, name: str) -> None:
@@ -51,11 +52,7 @@ def destroy(cls, name: str) -> None:
         app = cls._instances.pop(name)
         app.dispose()
         app.injection_store.destroy(name)
-
-    def __del__(self) -> None:
-        """Remove the app from the registry when it is garbage collected."""
-        with contextlib.suppress(KeyError):
-            Application.destroy(self.name)
+        app.destroyed.emit(app.name)
 
     @property
     def name(self) -> str:

src/app_model/backends/qt/_qaction.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Mapping, Optional, Union
+import contextlib
+from typing import TYPE_CHECKING, Any, Dict, Mapping, Optional, Tuple, Type, Union, cast
 
 from qtpy.QtWidgets import QAction
 
@@ -37,6 +38,7 @@ def __init__(
         super().__init__(parent)
         self._app = Application.get_or_create(app) if isinstance(app, str) else app
         self._command_id = command_id
+        self.setObjectName(command_id)
         if kb := self._app.keybindings.get_keybinding(command_id):
             self.setShortcut(QKeyBindingSequence(kb.keybinding))
         self.triggered.connect(self._on_triggered)
@@ -68,7 +70,6 @@ def __init__(
     ):
         super().__init__(command_rule.id, app, parent)
         self._cmd_rule = command_rule
-        self.setObjectName(command_rule.id)
         if use_short_title and command_rule.short_title:
             self.setText(command_rule.short_title)  # pragma: no cover
         else:
@@ -78,8 +79,6 @@ def __init__(
         if command_rule.tooltip:
             self.setToolTip(command_rule.tooltip)
 
-        self.setIconVisibleInMenu(False)
-
     def update_from_context(self, ctx: Mapping[str, object]) -> None:
         """Update the enabled state of this menu item from `ctx`."""
         self.setEnabled(expr.eval(ctx) if (expr := self._cmd_rule.enablement) else True)
@@ -92,16 +91,51 @@ class QMenuItemAction(QCommandRuleAction):
     to toggle visibility.
     """
 
+    _cache: Dict[Tuple[int, int], QMenuItemAction] = {}
+
+    def __new__(
+        cls: Type[QMenuItemAction],
+        menu_item: MenuItem,
+        app: Union[str, Application],
+        **kwargs: Any,
+    ) -> QMenuItemAction:
+        """Create and cache a QMenuItemAction for the given menu item."""
+        cache = kwargs.pop("cache", True)
+        app = Application.get_or_create(app) if isinstance(app, str) else app
+        key = (id(app), hash(menu_item))
+        if cache and key in cls._cache:
+            return cls._cache[key]
+
+        self = cast(QMenuItemAction, super().__new__(cls))
+        if cache:
+            cls._cache[key] = self
+        return self
+
     def __init__(
         self,
         menu_item: MenuItem,
         app: Union[str, Application],
         parent: Optional[QObject] = None,
+        *,
+        cache: bool = True,
     ):
-        super().__init__(menu_item.command, app, parent)
-        self._menu_item = menu_item
+        initialized = False
+        with contextlib.suppress(RuntimeError):
+            initialized = getattr(self, "_initialized", False)
+
+        if not initialized:
+            super().__init__(menu_item.command, app, parent)
+            self._menu_item = menu_item
+            key = (id(self._app), hash(menu_item))
+            self.destroyed.connect(lambda: QMenuItemAction._cache.pop(key, None))
+            self._app.destroyed.connect(lambda: QMenuItemAction._cache.pop(key, None))
+            self._initialized = True
 
     def update_from_context(self, ctx: Mapping[str, object]) -> None:
         """Update the enabled/visible state of this menu item from `ctx`."""
         super().update_from_context(ctx)
         self.setVisible(expr.eval(ctx) if (expr := self._menu_item.when) else True)
+
+    def __repr__(self) -> str:
+        name = self.__class__.__name__
+        return f"{name}({self._menu_item!r}, app={self._app.name!r})"

src/app_model/backends/qt/_qmainwindow.py

@@ -16,7 +16,8 @@ def __init__(
         super().__init__(parent)
         self._app = Application.get_or_create(app) if isinstance(app, str) else app
 
-    def setModelMenuBar(self, menu_ids: List[str]) -> None:
+    def setModelMenuBar(self, menu_ids: List[str]) -> QModelMenuBar:
         """Set the menu bar to a list of menu ids."""
         menu_bar = QModelMenuBar(menu_ids, self._app, self)
         self.setMenuBar(menu_bar)
+        return menu_bar

src/app_model/backends/qt/_qmenu.py

@@ -12,7 +12,7 @@
 from ._util import to_qicon
 
 if TYPE_CHECKING:
-    from qtpy.QtWidgets import QWidget
+    from qtpy.QtWidgets import QAction, QWidget
 
 
 class QModelMenu(QMenu):
@@ -62,7 +62,7 @@ def rebuild(self) -> None:
         for n, group in enumerate(groups):
             for item in group:
                 if isinstance(item, SubmenuItem):
-                    submenu = QModelSubmenu(item, self._app, self)
+                    submenu = QModelSubmenu(item, self._app, parent=self)
                     self.addMenu(submenu)
                 else:
                     action = QMenuItemAction(item, app=self._app, parent=self)
@@ -102,6 +102,17 @@ def update_from_context(
                 if _recurse:
                     parent.update_from_context(ctx, _recurse=False)
 
+    def findAction(self, object_name: str) -> Union[QAction, QModelMenu, None]:
+        """Find an action by its ObjectName.
+
+        Parameters
+        ----------
+        object_name : str
+            Action ID to find. Note that `QCommandAction` have `ObjectName` set
+            to their `command.id`
+        """
+        return next((a for a in self.actions() if a.objectName() == object_name), None)
+
 
 class QModelSubmenu(QModelMenu):
     """QMenu for a menu_id in an `app_model` MenusRegistry.

src/app_model/expressions/_expressions.py

@@ -320,6 +320,11 @@ def validate(cls, v: Any) -> Expr:
         """Validator for Expr. For use with Pydantic."""
         return v if isinstance(v, Expr) else parse_expression(v)
 
+    def __hash__(self) -> int:
+        return hash(self.__class__) + hash(
+            tuple(getattr(self, f) for f in self._fields)
+        )
+
 
 LOAD = ast.Load()
 

src/app_model/types/_command_rule.py

@@ -30,6 +30,11 @@ class CommandRule(_StrictModel):
     tooltip: Optional[str] = Field(
         None, description="(Optional) Tooltip to show when hovered."
     )
+    status_tip: Optional[str] = Field(
+        None,
+        description="(Optional) Help message to show in the status bar when a "
+        "button representing this command is hovered (For backends that support it).",
+    )
     icon: Optional[Icon] = Field(
         None,
         description="(Optional) Icon used to represent this command, e.g. on buttons "
@@ -47,3 +52,7 @@ class CommandRule(_StrictModel):
         "the UI. Menus pick either `title` or `short_title` depending on the context "
         "in which they show commands.",
     )
+
+    def _as_command_rule(self) -> "CommandRule":
+        """Simplify (subclasses) to a plain CommandRule."""
+        return CommandRule(**{f: getattr(self, f) for f in CommandRule.__fields__})

src/app_model/types/_icon.py

@@ -31,6 +31,8 @@ def __get_validators__(cls) -> Generator[Callable[..., Any], None, None]:
     def validate(cls, v: Any) -> "Icon":
         """Validate icon."""
         # if a single string is passed, use it for both light and dark.
+        if isinstance(v, Icon):
+            return v
         if isinstance(v, str):
             v = {"dark": v, "light": v}
         return cls(**v)

src/app_model/types/_menu_rule.py

@@ -1,6 +1,6 @@
 from typing import Any, Callable, Generator, Optional, Type, TypedDict, Union
 
-from pydantic import Field
+from pydantic import Field, validator
 
 from .. import expressions
 from ._base import _StrictModel
@@ -74,6 +74,12 @@ class MenuItem(_MenuItemBase):
         "selected, (e.g. when the Alt-key is held when opening the menu)",
     )
 
+    @validator("command")
+    def _simplify_command_rule(cls, v: Any) -> CommandRule:
+        if isinstance(v, CommandRule):
+            return v._as_command_rule()
+        raise TypeError("command must be a CommandRule")
+
 
 class SubmenuItem(_MenuItemBase):
     """Point to another Menu that will be displayed as a submenu."""

tests/conftest.py

@@ -206,7 +206,7 @@ def build_app(name: str = "complete_test_app") -> FullApp:
 def full_app(monkeypatch) -> Application:
     """Premade application."""
     try:
-        app = build_app()
+        app = build_app("complete_test_app")
         with monkeypatch.context() as m:
             # mock path to add fake_module
             m.setattr(sys, "path", [str(FIXTURES)] + sys.path)

tests/test_app.py

@@ -22,6 +22,7 @@ def test_app_create():
         Application("my_app")
 
     assert repr(app) == "Application('my_app')"
+    Application.destroy("my_app")
 
 
 def test_app(full_app: FullApp):

tests/test_qt/test_qmenu.py

@@ -5,7 +5,8 @@
 from qtpy.QtCore import Qt
 from qtpy.QtWidgets import QAction, QMainWindow
 
-from app_model.backends.qt import QModelMenu
+from app_model.backends.qt import QMenuItemAction, QModelMenu
+from app_model.types import MenuItem
 
 if TYPE_CHECKING:
     from pytestqt.plugin import QtBot
@@ -28,12 +29,13 @@ def test_menu(qtbot: "QtBot", full_app: "FullApp") -> None:
 
     # check that triggering the actions calls the associated commands
     for cmd in (app.Commands.UNDO, app.Commands.REDO):
-        action: QAction = menu.findChild(QAction, cmd)
+        action: QAction = menu.findAction(cmd)
         with qtbot.waitSignal(action.triggered):
             action.trigger()
             getattr(app.mocks, cmd).assert_called_once()
 
-    redo_action: QAction = menu.findChild(QAction, app.Commands.REDO)
+    redo_action: QAction = menu.findAction(app.Commands.REDO)
+
     assert redo_action.isVisible()
     assert redo_action.isEnabled()
 
@@ -68,7 +70,8 @@ def test_submenu(qtbot: "QtBot", full_app: "FullApp") -> None:
     menu_texts = [a.text() for a in menu.actions()]
     assert menu_texts == ["Open From...", "Open..."]
 
-    submenu: QModelMenu = menu.findChild(QModelMenu, app.Menus.FILE_OPEN_FROM)
+    submenu = menu.findChild(QModelMenu, app.Menus.FILE_OPEN_FROM)
+    assert isinstance(submenu, QModelMenu)
     submenu.setVisible(True)
     assert submenu.isVisible()
     assert submenu.isEnabled()
@@ -107,10 +110,20 @@ def test_shortcuts(qtbot: "QtBot", full_app: "FullApp") -> None:
     with qtbot.waitExposed(win):
         win.show()
 
-    copy_action = menu.findChild(QAction, app.Commands.COPY)
+    copy_action = menu.findAction(app.Commands.COPY)
+
     with qtbot.waitSignal(copy_action.triggered, timeout=1000):
         qtbot.keyClicks(win, "C", Qt.KeyboardModifier.ControlModifier)
 
-    paste_action = menu.findChild(QAction, app.Commands.PASTE)
+    paste_action = menu.findAction(app.Commands.PASTE)
     with qtbot.waitSignal(paste_action.triggered, timeout=1000):
         qtbot.keyClicks(win, "V", Qt.KeyboardModifier.ControlModifier)
+
+
+def test_cache_action(full_app: "FullApp") -> None:
+    action = next(
+        i for k, items in full_app.menus for i in items if isinstance(i, MenuItem)
+    )
+    a1 = QMenuItemAction(action, full_app)
+    a2 = QMenuItemAction(action, full_app)
+    assert a1 is a2