From 0cf910ddac55233e006f98d80deaf968623dc18b Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 4 Jul 2026 12:33:18 +0200 Subject: [PATCH] =?UTF-8?q?persist=20tool=20removal=20in=20the=20index=20?= =?UTF-8?q?=E2=80=94=20uninstalls=20stayed=20in-memory=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit remove_tool_by_id popped the entry from every in-memory map, but the persisted singleton index still carried it. Every populate broadcasts a reload_tool_source_cache invalidation, so the very next reload handed the entry back and the uninstalled tool resolved again (test_repository_uninstall expected err_msg, got the full tool). ToolSourceStore grows remove_index_entry — the uninstall counterpart of update_index_entry — and remove_tool_by_id writes the removal through. --- lib/galaxy/tool_source_store/__init__.py | 21 +++++++++++++++++++++ lib/galaxy/tools/lazy_toolbox.py | 10 ++++++++++ lib/galaxy/tools/source_store/composite.py | 3 +++ test/unit/tool_source_store/test_stores.py | 19 +++++++++++++++++++ 4 files changed, 53 insertions(+) diff --git a/lib/galaxy/tool_source_store/__init__.py b/lib/galaxy/tool_source_store/__init__.py index b3bb29c8667..57c98ed5264 100644 --- a/lib/galaxy/tool_source_store/__init__.py +++ b/lib/galaxy/tool_source_store/__init__.py @@ -176,6 +176,27 @@ class ToolSourceStore(ABC): entry: The index entry to update. """ + def remove_index_entry(self, tool_id: str) -> None: + """Remove a tool's entry from the persisted index. + + Counterpart of :meth:`update_index_entry` for uninstalls: the lazy + toolbox pops the entry from its in-memory index, but the persisted + singleton would hand it right back on the next cache invalidation + unless the removal is written through. + """ + index = self.load_index() + if index is None: + return + removed = index.entries.pop(tool_id, None) + removed_versions = index.entries_by_version.pop(tool_id, None) + if removed is None and removed_versions is None: + return + for section_tool_ids in index.by_section.values(): + if tool_id in section_tool_ids: + section_tool_ids.remove(tool_id) + index.invalidate_caches() + self.store_index(index) + def invalidate_index_cache(self) -> None: # noqa: B027 — intentional empty default """Drop any in-memory cached index so the next load_index() reads fresh. diff --git a/lib/galaxy/tools/lazy_toolbox.py b/lib/galaxy/tools/lazy_toolbox.py index 9a037bb366e..836137a1674 100644 --- a/lib/galaxy/tools/lazy_toolbox.py +++ b/lib/galaxy/tools/lazy_toolbox.py @@ -1229,6 +1229,16 @@ class LazyToolBox(ToolBox): with self._cache_lock: for key in [k for k in self._tool_object_cache.keys() if k.startswith(f"{tool_id}:")]: self._tool_object_cache.pop(key, None) + if self._store is not None: + # The pops above are in-memory. The persisted singleton index + # still carries the entry, and any later cache invalidation + # (every populate broadcasts one) would reload it — + # resurrecting an uninstalled tool. + try: + self._store.remove_index_entry(tool_id) + self._store.commit() + except Exception as e: + log.warning("Persisting index removal of %s raised: %s", tool_id, e) return result # === Override has_tool to check index === diff --git a/lib/galaxy/tools/source_store/composite.py b/lib/galaxy/tools/source_store/composite.py index abaf5439ac2..7249eee784c 100644 --- a/lib/galaxy/tools/source_store/composite.py +++ b/lib/galaxy/tools/source_store/composite.py @@ -73,6 +73,9 @@ class CompositeToolSourceStore(ToolSourceStore): def update_index_entry(self, entry: ToolIndexEntry) -> None: self._default_store.update_index_entry(entry) + def remove_index_entry(self, tool_id: str) -> None: + self._default_store.remove_index_entry(tool_id) + # --- read ops: priority order -------------------------------------- def get(self, hash: str) -> StoredToolSource | None: diff --git a/test/unit/tool_source_store/test_stores.py b/test/unit/tool_source_store/test_stores.py index 782ae993a14..8f062cd0fc5 100644 --- a/test/unit/tool_source_store/test_stores.py +++ b/test/unit/tool_source_store/test_stores.py @@ -194,6 +194,25 @@ class TestDatabaseBackendPathRows: store.delete("edited_hash_v2") app.model.context.commit() + def test_remove_index_entry_persists_removal(self): + app = MockApp() + store = DatabaseToolSourceStore(app.model.context) # type: ignore[arg-type] + index = ToolIndex() + entry = ToolIndexEntry(id="removable", version="1.0", name="Removable", panel_section_id="sec1") + index.add_entry(entry) + index.by_section["sec1"] = ["removable"] + store.store_index(index) + app.model.context.commit() + + store.remove_index_entry("removable") + app.model.context.commit() + store.invalidate_index_cache() + + reloaded = store.load_index() + assert reloaded is not None + assert reloaded.get("removable") is None + assert "removable" not in reloaded.by_section.get("sec1", []) + def test_pathless_sources_dedupe_on_hash(self): app = MockApp() store = DatabaseToolSourceStore(app.model.context) # type: ignore[arg-type]