mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-21 13:50:20 +08:00
LazyTool: restore entry-only to_dict fast path (no panel materialise)
With ``link_details`` no longer gating any field, the toolbox-internal
panel / listing callers stopped passing it (``f64858bf55e``).
``LazyTool.to_dict`` can now return the entry-shape dict for the
default-call case — every panel-view tool serves from the index, zero
materialise.
Materialise still fires when the caller asks for the parsed-Tool
payload: ``io_details=True`` (the ``/api/tools/{id}`` show endpoint
contract) or ``tool_help=True`` (rendered help). Failure-to-materialise
still falls back to the entry-shape dict so the show endpoint never
returns 500 when a specific tool's XML chokes the parameter factory.
The ``link`` field is derived from ``self.id`` matching what
``Tool.to_dict`` now emits unconditionally — ``/tool_runner?tool_id={id}``
for the standard tool runner.
Tests updated:
- ``test_to_dict_entry_fast_path_does_not_materialise`` asserts the
default call serves the entry dict and never invokes the callback.
- ``test_to_dict_io_details_materialises`` covers the show contract.
- ``test_to_dict_falls_back_to_entry_when_materialise_fails`` covers the
parameter-factory-failure case.
23/23 unit tests pass.
This commit is contained in:
@@ -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/<id>`` 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 ---
|
||||
|
||||
@@ -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():
|
||||
|
||||
Reference in New Issue
Block a user