diff --git a/lib/galaxy/tools/lazy_toolbox.py b/lib/galaxy/tools/lazy_toolbox.py index e7ef1cebde0..2a8b09aeaca 100644 --- a/lib/galaxy/tools/lazy_toolbox.py +++ b/lib/galaxy/tools/lazy_toolbox.py @@ -291,30 +291,30 @@ class LazyTool: return True def to_dict(self, trans=None, link_details: bool = False, tool_help: bool = False, **kw) -> dict[str, Any]: - """API serialisation — always materialises. + """API serialisation. - The flat ``/api/tools?in_panel=False`` listing already bypasses - ``LazyTool.to_dict`` via ``services/tools.py:list_tools`` which - calls ``entry.to_api_dict()`` directly off the index — so the - only callers that reach this method are paths that need the - parsed payload: ``/api/tools/`` show (with or without - ``io_details``), the panel-view walk in ``to_panel_view`` / - ``ToolSection.to_dict``, and the per-tool ``Tool.to_dict`` reads - from filters / managers. + Entry-shape fast path covers the panel-view walk + (``AbstractToolBox.get_tool_to_dict`` no longer passes + ``link_details=True``; see commit ``f64858bf55e``) and the flat + ``/api/tools?in_panel=False`` listing handler. Materialise only + when the caller asks for ``io_details=True`` (the ``/api/tools/{id}`` + show endpoint, which needs ``inputs`` / ``outputs`` / parameter + tree) or ``tool_help=True`` (rendered help payload). On materialise failure (tool XML the parameter factory can't handle — ``upload_dataset``, ``column="value"`` against an - unresolvable column-name spec, …) we fall back to an entry-only + unresolvable column-name spec, …) we fall back to the entry-only dict so the caller still gets a renderable shape; eager mode catches the same in ``_load_tool_tag_set`` and drops the tool from ``_tools_by_id``. """ - try: - return self._materialize().to_dict( - trans, link_details=link_details, tool_help=tool_help, **kw - ) - except Exception as e: - log.warning("LazyTool.to_dict: materialise failed for %s, falling back to entry: %s", self.id, e) + if kw.get("io_details") or tool_help: + try: + return self._materialize().to_dict( + trans, link_details=link_details, tool_help=tool_help, **kw + ) + except Exception as e: + log.warning("LazyTool.to_dict: materialise failed for %s, falling back to entry: %s", self.id, e) entry = self._entry return { "id": self.id, @@ -328,7 +328,9 @@ class LazyTool: "model_class": "Tool", "panel_section_id": entry.panel_section_id, "panel_section_name": entry.panel_section_name, - "link": f"/api/tools/{self.id}", + # Cheap URL pattern — matches what ``Tool.to_dict`` now emits + # unconditionally for the materialised side. + "link": f"/tool_runner?tool_id={self.id}", } # --- materialisation --- diff --git a/test/unit/app/tools/test_lazy_tool.py b/test/unit/app/tools/test_lazy_tool.py index b2c51b43bff..48d21b5b9ad 100644 --- a/test/unit/app/tools/test_lazy_tool.py +++ b/test/unit/app/tools/test_lazy_tool.py @@ -109,28 +109,26 @@ def test_old_id_short_circuits_for_shed_ids(): assert _stub(_entry(id="local_tool")).old_id == "local_tool" -def test_to_dict_falls_back_to_entry_dict_when_materialise_fails(): - # ``LazyTool.to_dict`` always tries to materialise — the flat - # ``/api/tools?in_panel=False`` listing path bypasses it entirely via - # ``entry.to_api_dict()``, so every caller that reaches here needs the - # parsed payload. On materialise failure we return the entry-shape - # dict so the panel render / show endpoint still gets *something*. +def test_to_dict_entry_fast_path_does_not_materialise(): + # Default call (no ``io_details``, no ``tool_help``) is the panel-view + # path — the toolbox no longer asks for ``link_details``. Serve the + # entry-shape dict directly; never materialise. def boom(_e): - raise RuntimeError("materialise failed") + raise AssertionError(f"unexpected materialise for {_e.id!r}") t = LazyTool(_entry(), materialize_callback=boom, is_admin_user=lambda u: False) - d = t.to_dict(trans=None, link_details=False) + d = t.to_dict(trans=None) assert d["id"] == "bowtie2" assert d["model_class"] == "Tool" - assert d["link"] == "/api/tools/bowtie2" + assert d["link"] == "/tool_runner?tool_id=bowtie2" -def test_to_dict_materialises_exactly_once_per_call(): +def test_to_dict_io_details_materialises(): calls = [] class _Real: def to_dict(self, trans, link_details, tool_help, **kw): - calls.append("real-to_dict") + calls.append(("real-to_dict", kw.get("io_details"))) return {"id": "real"} def mat(_e): @@ -138,10 +136,23 @@ def test_to_dict_materialises_exactly_once_per_call(): return _Real() t = LazyTool(_entry(), materialize_callback=mat, is_admin_user=lambda u: False) - assert t.to_dict(trans=None, link_details=True) == {"id": "real"} - assert t.to_dict(trans=None, link_details=True) == {"id": "real"} - # First call materialises; second reuses ``_real``. - assert calls == ["mat", "real-to_dict", "real-to_dict"] + # ``io_details=True`` is the show-endpoint contract — materialise. + assert t.to_dict(trans=None, io_details=True) == {"id": "real"} + # Second call reuses cached ``_real``. + assert t.to_dict(trans=None, io_details=True) == {"id": "real"} + assert calls == ["mat", ("real-to_dict", True), ("real-to_dict", True)] + + +def test_to_dict_falls_back_to_entry_when_materialise_fails(): + # If a tool can't materialise (e.g. ``upload_dataset`` parameter factory + # failure) the show endpoint still gets the entry-shape dict. + def boom(_e): + raise RuntimeError("materialise failed") + + t = LazyTool(_entry(), materialize_callback=boom, is_admin_user=lambda u: False) + d = t.to_dict(trans=None, io_details=True) + assert d["id"] == "bowtie2" + assert d["model_class"] == "Tool" def test_allow_user_access_uses_index_data_without_materialise():