mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-21 13:50:20 +08:00
LazyTool.to_dict: always materialise; fast path was incomplete
The entry-only fast path was missing fields the ``/api/tools/<id>`` show endpoint serves (``xrefs``, ``inputs`` when ``io_details=False``, …). The flat ``/api/tools?in_panel=False`` listing — the only caller that benefits from the fast path — already bypasses this method: ``services/tools.py:list_tools`` walks ``tool_index.list_all()`` and calls ``entry.to_api_dict()`` directly. Always materialise here; the entry-only dict survives as the fallback when materialise itself raises (a tool whose XML the parameter factory chokes on — ``upload_dataset``, ``column="value"``, …) so the caller still gets *something* renderable rather than a 500. Tests updated: - ``test_to_dict_fast_path_does_not_materialise`` → renamed to ``test_to_dict_falls_back_to_entry_dict_when_materialise_fails``. - ``test_to_dict_link_details_materialises_exactly_once`` → renamed for accuracy; behaviour unchanged.
This commit is contained in:
@@ -287,30 +287,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.
|
||||
"""API serialisation — always materialises.
|
||||
|
||||
The entry-only fast path (id, name, version, panel_section, …) only
|
||||
covers the ``/api/tools?in_panel=False`` listing — every other
|
||||
consumer needs the parsed payload. Materialise when:
|
||||
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.
|
||||
|
||||
- ``link_details=True`` — ``/api/tools/<id>/build`` and the panel-view
|
||||
walk (``ToolSection`` rendering).
|
||||
- ``io_details=True`` (in ``kw``) — ``/api/tools/<id>?io_details=true``
|
||||
show endpoint, which the integration suite uses to assert ``inputs``.
|
||||
|
||||
Materialise failures (tool XML the parameter factory can't handle —
|
||||
``upload_dataset``, ``column="value"`` against an unresolvable
|
||||
column-name spec, …) fall back to the entry-only dict so the panel
|
||||
listing still renders; eager mode catches the same in
|
||||
``_load_tool_tag_set`` and drops the tool from ``_tools_by_id``.
|
||||
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
|
||||
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``.
|
||||
"""
|
||||
if link_details or kw.get("io_details"):
|
||||
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)
|
||||
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,
|
||||
|
||||
@@ -109,15 +109,23 @@ def test_old_id_short_circuits_for_shed_ids():
|
||||
assert _stub(_entry(id="local_tool")).old_id == "local_tool"
|
||||
|
||||
|
||||
def test_to_dict_fast_path_does_not_materialise():
|
||||
t = _stub()
|
||||
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 boom(_e):
|
||||
raise RuntimeError("materialise failed")
|
||||
|
||||
t = LazyTool(_entry(), materialize_callback=boom, is_admin_user=lambda u: False)
|
||||
d = t.to_dict(trans=None, link_details=False)
|
||||
assert d["id"] == "bowtie2"
|
||||
assert d["model_class"] == "Tool"
|
||||
assert d["link"] == "/api/tools/bowtie2"
|
||||
|
||||
|
||||
def test_to_dict_link_details_materialises_exactly_once():
|
||||
def test_to_dict_materialises_exactly_once_per_call():
|
||||
calls = []
|
||||
|
||||
class _Real:
|
||||
@@ -132,6 +140,7 @@ def test_to_dict_link_details_materialises_exactly_once():
|
||||
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"]
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user