diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index a8098aa8a01..bfcaff35b86 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -14,6 +14,7 @@ from collections.abc import ( MutableMapping, Sequence, ) +from datetime import datetime from pathlib import Path from typing import ( Any, @@ -700,20 +701,22 @@ class ToolBox(AbstractToolBox): if store is None: return None - stored = None + # Resolve by the on-disk file path first. ``source_path`` is recorded + # at populate time and the populator updates that per-path row in place, + # so a hit here is content-current and exact (no regex shortcut, no + # ``sources[0]`` fallback that could silently return a different tool's + # source). + stored = store.get_by_source_path(os.path.abspath(str(config_file))) - # Shed installs hand us the full guid; look up directly. - if tool_id: + # Shed installs hand us the full guid; fall back to it only when the + # path lookup misses. ``get_by_tool_id`` has no ORDER BY and the + # append-only store accumulates superseded content rows for the same + # tool_id, so pick the most recently stored one rather than an + # arbitrary row. + if stored is None and tool_id: sources = store.get_by_tool_id(tool_id) if sources: - stored = sources[0] - - # Local tools have no tool_id at this point — resolve by the on-disk - # file path. ``source_path`` is recorded at populate time, so a hit - # here is exact (no regex shortcut, no ``sources[0]`` fallback that - # could silently return a different tool's source). - if stored is None: - stored = store.get_by_source_path(os.path.abspath(str(config_file))) + stored = max(sources, key=lambda s: (s.stored_at is not None, s.stored_at or datetime.min)) if stored is None: return None diff --git a/test/unit/app/tools/test_tool_source_store_selection.py b/test/unit/app/tools/test_tool_source_store_selection.py new file mode 100644 index 00000000000..ce2bdb81bad --- /dev/null +++ b/test/unit/app/tools/test_tool_source_store_selection.py @@ -0,0 +1,92 @@ +from datetime import ( + datetime, + timedelta, + timezone, +) +from types import SimpleNamespace +from unittest.mock import MagicMock + +from galaxy import tools +from galaxy.tools import ToolBox +from galaxy.tools.source_store.interface import StoredToolSource + + +def _stored(tool_id=None, source_path=None, raw="", stored_at=None): + return StoredToolSource( + hash=f"{tool_id}:{source_path}:{raw}", + tool_source_class="XmlToolSource", + raw_source=raw, + tool_id=tool_id, + source_path=source_path, + stored_at=stored_at, + ) + + +def _box(store): + box = ToolBox.__new__(ToolBox) + box.app = SimpleNamespace(config=SimpleNamespace(use_lazy_toolbox=True), tool_source_store=store) # type: ignore[assignment] + return box + + +def _patch_get_tool_source(monkeypatch): + monkeypatch.setattr(tools, "get_tool_source", lambda raw_tool_source, tool_source_class: raw_tool_source) + + +def test_path_row_preferred_over_tool_id_rows(monkeypatch): + _patch_get_tool_source(monkeypatch) + store = MagicMock() + store.get_by_source_path.return_value = _stored(tool_id="t1", source_path="/tools/t1.xml", raw="path_row") + store.get_by_tool_id.return_value = [_stored(tool_id="t1", raw="guid_row")] + box = _box(store) + + assert box._get_tool_source_from_store("/tools/t1.xml", tool_id="t1") == "path_row" + store.get_by_tool_id.assert_not_called() + + +def test_tool_id_fallback_when_path_misses(monkeypatch): + _patch_get_tool_source(monkeypatch) + store = MagicMock() + store.get_by_source_path.return_value = None + store.get_by_tool_id.return_value = [_stored(tool_id="t1", raw="guid_row")] + box = _box(store) + + assert box._get_tool_source_from_store("/tools/t1.xml", tool_id="t1") == "guid_row" + + +def test_tool_id_fallback_picks_latest_stored_at(monkeypatch): + _patch_get_tool_source(monkeypatch) + now = datetime.now(timezone.utc) + store = MagicMock() + store.get_by_source_path.return_value = None + store.get_by_tool_id.return_value = [ + _stored(tool_id="t1", raw="old", stored_at=now - timedelta(hours=1)), + _stored(tool_id="t1", raw="new", stored_at=now), + _stored(tool_id="t1", raw="mid", stored_at=now - timedelta(minutes=30)), + ] + box = _box(store) + + assert box._get_tool_source_from_store("/tools/t1.xml", tool_id="t1") == "new" + + +def test_tool_id_fallback_prefers_dated_row_over_undated(monkeypatch): + _patch_get_tool_source(monkeypatch) + now = datetime.now(timezone.utc) + store = MagicMock() + store.get_by_source_path.return_value = None + store.get_by_tool_id.return_value = [ + _stored(tool_id="t1", raw="dated", stored_at=now), + _stored(tool_id="t1", raw="undated", stored_at=None), + ] + box = _box(store) + + assert box._get_tool_source_from_store("/tools/t1.xml", tool_id="t1") == "dated" + + +def test_returns_none_when_lazy_toolbox_disabled(): + store = MagicMock() + box = ToolBox.__new__(ToolBox) + box.app = SimpleNamespace(config=SimpleNamespace(use_lazy_toolbox=False), tool_source_store=store) # type: ignore[assignment] + + assert box._get_tool_source_from_store("/tools/t1.xml", tool_id="t1") is None + store.get_by_source_path.assert_not_called() + store.get_by_tool_id.assert_not_called()