diff --git a/.ci/installed_startup.sh b/.ci/installed_startup.sh new file mode 100755 index 00000000000..b553cbf9521 --- /dev/null +++ b/.ci/installed_startup.sh @@ -0,0 +1,70 @@ +#!/bin/bash + +set -euo pipefail + +VENV=${1:?usage: $0 VENV} +PYTHON="$VENV/bin/python" +GALAXY="$VENV/bin/galaxy" +URL=http://localhost:8080 +TRIES=120 +WORK_DIR=$(mktemp -d -t galaxy-installed-startupXXXXXX) +LOG_FILE="$WORK_DIR/galaxy.log" +GALAXY_PID= + +cleanup() { + exit_code=$? + if [ -n "$GALAXY_PID" ]; then + kill "$GALAXY_PID" 2>/dev/null || true + wait "$GALAXY_PID" 2>/dev/null || true + fi + if [ "$exit_code" -ne 0 ] && [ -f "$LOG_FILE" ]; then + echo "Installed Galaxy startup failed; showing $LOG_FILE:" + cat "$LOG_FILE" + fi + rm -rf "$WORK_DIR" + exit "$exit_code" +} +trap cleanup EXIT + +"$PYTHON" - <<'PY' +import sys +from pathlib import Path + +import galaxy.web_client + +package_path = Path(galaxy.web_client.__file__).resolve() +venv_path = Path(sys.prefix).resolve() +if not package_path.is_relative_to(venv_path): + raise SystemExit(f"galaxy.web_client resolved outside the installed environment: {package_path}") +PY + +ASSET=$( + "$PYTHON" - <<'PY' +from importlib.resources import files + +dist = files("galaxy.web_client").joinpath("dist") +print(next(asset.name for asset in dist.iterdir() if asset.name.startswith("galaxy-app-") and asset.name.endswith(".js"))) +PY +) + +cd "$WORK_DIR" +export GALAXY_CONFIG_DATABASE_AUTO_MIGRATE=true +export PATH="$VENV/bin:$PATH" +"$GALAXY" >"$LOG_FILE" 2>&1 & +GALAXY_PID=$! + +for ((i = 0; i <= TRIES; i++)); do + if curl --fail --max-time 1 --silent --output /dev/null "$URL/api/version"; then + curl --fail --max-time 5 --silent --output /dev/null "$URL/" + curl --fail --max-time 5 --silent --output /dev/null "$URL/static/dist/$ASSET" + exit 0 + fi + if ! kill -0 "$GALAXY_PID" 2>/dev/null; then + echo "Installed Galaxy exited before becoming ready." >&2 + exit 1 + fi + sleep 1 +done + +echo "Installed Galaxy did not become ready after $TRIES seconds." >&2 +exit 1 diff --git a/.github/workflows/first_startup.yaml b/.github/workflows/first_startup.yaml index 9c901098f95..87d9fc6dcee 100644 --- a/.github/workflows/first_startup.yaml +++ b/.github/workflows/first_startup.yaml @@ -67,3 +67,30 @@ jobs: - name: run tests run: tox -e first_startup working-directory: 'galaxy root' + installed-test: + name: Installed startup test + runs-on: ubuntu-latest + defaults: + run: + shell: bash -l {0} + steps: + - uses: actions/checkout@v7.0.1 + with: + path: 'galaxy root' + persist-credentials: false + - name: Set up Python + uses: actions/setup-python@v7.0.0 + with: + python-version: '3.14' + - name: Install uv + uses: astral-sh/setup-uv@v10.2.0 + with: + version-file: galaxy root/.tool-versions + - name: Install release tooling + run: uv tool install galaxy-release-util==0.4.2 + - name: Build and install Galaxy packages + run: make install-packages VENV="$RUNNER_TEMP/galaxy-installed" + working-directory: 'galaxy root' + - name: Start installed Galaxy + run: .ci/installed_startup.sh "$RUNNER_TEMP/galaxy-installed" + working-directory: 'galaxy root' diff --git a/Makefile b/Makefile index da2c2ea437f..a526771ff32 100644 --- a/Makefile +++ b/Makefile @@ -190,6 +190,10 @@ update-navigation-schema: client-node-deps install-client: ## Install prebuilt client wheel from PyPI matching the current Galaxy version $(IN_VENV) pip install "galaxy-web-client==$$(PYTHONPATH=lib python -c 'from galaxy.version import VERSION; print(VERSION)')" +install-packages: VENV=packages/.venv +install-packages: ## Build and install Galaxy packages from this checkout into VENV + cd packages && VENV="$(abspath $(VENV))" ./package-build-install.sh -m + client: client-node-deps ## Rebuild client-side artifacts for local development. $(IN_VENV) cd client && $(NODE_ENV) pnpm run build diff --git a/client/src/components/Common/ExportRemoteSourceSelector.vue b/client/src/components/Common/ExportRemoteSourceSelector.vue index e15b63c9e48..7089e86a7a3 100644 --- a/client/src/components/Common/ExportRemoteSourceSelector.vue +++ b/client/src/components/Common/ExportRemoteSourceSelector.vue @@ -26,7 +26,7 @@ const props = withDefaults(defineProps(), { const emit = defineEmits<{ (e: "update:directory", value: string): void; - (e: "update:fileName", value: string): void; + (e: "update:file-name", value: string): void; (e: "navigated"): void; }>(); @@ -67,7 +67,7 @@ const fileNameDescription = localize("Give the exported file a name."); placeholder="Name" required data-test-id="export-file-name-input" - @input="emit('update:fileName', $event)" /> + @input="emit('update:file-name', $event)" /> .{{ props.fileExtension }} diff --git a/client/src/components/Panels/Common/ToolSearch.vue b/client/src/components/Panels/Common/ToolSearch.vue index dbb53b8560e..3e36a2b72b1 100644 --- a/client/src/components/Panels/Common/ToolSearch.vue +++ b/client/src/components/Panels/Common/ToolSearch.vue @@ -3,7 +3,7 @@ import { storeToRefs } from "pinia"; import { nextTick } from "vue"; import { onMounted, onUnmounted, type PropType, watch } from "vue"; -import { FAVORITES_KEYS, filterPanelByToolIds, searchTools } from "@/components/Panels/utilities"; +import { FAVORITES_KEYS, searchTools } from "@/components/Panels/utilities"; import { type Tool, type ToolPanelItem, type ToolSection, useToolStore } from "@/stores/toolStore"; import { useUserStore } from "@/stores/userStore"; import _l from "@/utils/localization"; @@ -64,6 +64,7 @@ interface RequestPayload { tools: Tool[]; query: string; currentPanel: Record; + favoriteToolIds: string[]; } interface SearchEventQuery { @@ -141,28 +142,29 @@ function parseFavoritesQuery(query: string) { function handlePost(event: SearchEvent) { const { type } = event.data; if (type === "searchTools") { - const { tools, query, currentPanel } = event.data.payload; + const { tools, query, currentPanel, favoriteToolIds } = event.data.payload; const favoritesQuery = parseFavoritesQuery(query); if (favoritesQuery && !favoritesQuery.isFavoritesOnly) { - const { results, resultPanel, closestTerm } = searchTools(tools, favoritesQuery.remainder, currentPanel); const favoriteToolIdSet = new Set(currentFavorites.value.tools); - const favoriteResults = results.filter((toolId) => favoriteToolIdSet.has(toolId)); - const favoriteResultPanel = - resultPanel && favoriteResults.length > 0 - ? filterPanelByToolIds(resultPanel, new Set(favoriteResults)) - : resultPanel; + // Narrow to favorites before searching, so lineage de-duplication cannot drop a favorited version. + const favoriteTools = tools.filter((tool) => favoriteToolIdSet.has(tool.id)); + const { results, resultPanel, closestTerm } = searchTools( + favoriteTools, + favoritesQuery.remainder, + currentPanel, + ); onMessage({ data: { type: "favoriteSearchToolsResult", - payload: favoriteResults, - sectioned: favoriteResultPanel, + payload: results, + sectioned: resultPanel, query, closestTerm, }, } as unknown as MessageEvent); return; } - const { results, resultPanel, closestTerm } = searchTools(tools, query, currentPanel); + const { results, resultPanel, closestTerm } = searchTools(tools, query, currentPanel, favoriteToolIds); // send the result back to the main thread onMessage({ data: { @@ -221,11 +223,12 @@ onUnmounted(() => { watch( () => currentFavorites.value.tools, () => { - const favoritesQuery = parseFavoritesQuery(props.query); - if (!favoritesQuery) { + // Plain searches re-run too, since favorites decide which tool version is shown. + if (!props.query || props.query.trim().length < MIN_QUERY_LENGTH) { return; } - if (favoritesQuery.isFavoritesOnly) { + const favoritesQuery = parseFavoritesQuery(props.query); + if (favoritesQuery?.isFavoritesOnly) { post({ type: "favoriteTools" }); } else { post({ @@ -234,6 +237,7 @@ watch( tools: props.toolsList, query: props.query, currentPanel: props.currentPanel, + favoriteToolIds: [...currentFavorites.value.tools], }, }); } @@ -253,6 +257,7 @@ function checkQuery(q: string) { tools: props.toolsList, query: q, currentPanel: props.currentPanel, + favoriteToolIds: [...currentFavorites.value.tools], }, }); } diff --git a/client/src/components/Panels/ToolBoxSearch.test.ts b/client/src/components/Panels/ToolBoxSearch.test.ts index 3346762742c..454ea42f865 100644 --- a/client/src/components/Panels/ToolBoxSearch.test.ts +++ b/client/src/components/Panels/ToolBoxSearch.test.ts @@ -208,6 +208,96 @@ describe("ToolBox search", () => { expect(toolIds).toEqual(["__FILTER_FAILED_DATASETS__", "__ZIP_COLLECTION__"]); }); + it("keeps a favorited older tool version in #favorites and plain searches", async () => { + const latestZipVersion = { + ...toolsList.find((tool) => tool.id === "__ZIP_COLLECTION__")!, + id: "toolshed.example.org/repos/dev/zip_collection/zip/1.0", + version: "1.0", + }; + const olderZipVersion = { + ...latestZipVersion, + id: "toolshed.example.org/repos/dev/zip_collection/zip/0.9", + version: "0.9", + }; + const { wrapper } = mountToolBox({ + tools: [...toolsList, latestZipVersion, olderZipVersion], + toolSections: { + default: { + ...toolsListInPanel, + collection_operations: { + ...(toolsListInPanel.collection_operations as ToolSection), + tools: [latestZipVersion.id, olderZipVersion.id], + }, + }, + }, + favorites: { tools: [olderZipVersion.id] }, + }); + await flushPromises(); + + const input = wrapper.find("input.search-query"); + await input.setValue("#favorites Zip"); + vi.advanceTimersByTime(250); + await flushPromises(); + + const toolIds = wrapper.findAll("a[data-tool-id]").map((item) => item.attributes("data-tool-id")); + expect(toolIds).toEqual([olderZipVersion.id]); + + await input.setValue("Zip"); + vi.advanceTimersByTime(250); + await flushPromises(); + + const plainToolIds = wrapper.findAll("a[data-tool-id]").map((item) => item.attributes("data-tool-id")); + expect(plainToolIds).toContain(olderZipVersion.id); + expect(plainToolIds).not.toContain(latestZipVersion.id); + }); + + it("updates the shown tool version in a plain search when favorites change", async () => { + const latestZipVersion = { + ...toolsList.find((tool) => tool.id === "__ZIP_COLLECTION__")!, + id: "toolshed.example.org/repos/dev/zip_collection/zip/1.0", + version: "1.0", + }; + const olderZipVersion = { + ...latestZipVersion, + id: "toolshed.example.org/repos/dev/zip_collection/zip/0.9", + version: "0.9", + }; + const { wrapper, userStore } = mountToolBox({ + tools: [...toolsList, latestZipVersion, olderZipVersion], + toolSections: { + default: { + ...toolsListInPanel, + collection_operations: { + ...(toolsListInPanel.collection_operations as ToolSection), + tools: [latestZipVersion.id, olderZipVersion.id], + }, + }, + }, + }); + await flushPromises(); + + const input = wrapper.find("input.search-query"); + await input.setValue("Zip"); + vi.advanceTimersByTime(250); + await flushPromises(); + + const shownToolIds = () => wrapper.findAll("a[data-tool-id]").map((item) => item.attributes("data-tool-id")); + expect(shownToolIds()).toContain(latestZipVersion.id); + expect(shownToolIds()).not.toContain(olderZipVersion.id); + + userStore.currentPreferences = { favorites: { tools: [olderZipVersion.id] } }; + await flushPromises(); + + expect(shownToolIds()).toContain(olderZipVersion.id); + expect(shownToolIds()).not.toContain(latestZipVersion.id); + + userStore.currentPreferences = { favorites: { tools: [] } }; + await flushPromises(); + + expect(shownToolIds()).toContain(latestZipVersion.id); + expect(shownToolIds()).not.toContain(olderZipVersion.id); + }); + it("collapses favorite results during search in My panel", async () => { const { wrapper } = mountToolBox({ favorites: { tools: ["__FILTER_FAILED_DATASETS__"] } }); await flushPromises(); diff --git a/client/src/components/Panels/toolSearch.worker.js b/client/src/components/Panels/toolSearch.worker.js index c51346005a0..58b2c3c584c 100644 --- a/client/src/components/Panels/toolSearch.worker.js +++ b/client/src/components/Panels/toolSearch.worker.js @@ -8,8 +8,8 @@ import { searchTools } from "./utilities"; self.addEventListener("message", (event) => { const { type, payload } = event.data; if (type === "searchTools") { - const { tools, query, currentPanel } = payload; - const { results, resultPanel, closestTerm } = searchTools(tools, query, currentPanel); + const { tools, query, currentPanel, favoriteToolIds } = payload; + const { results, resultPanel, closestTerm } = searchTools(tools, query, currentPanel, favoriteToolIds); // send the result back to the main thread self.postMessage({ type: "searchToolsByKeysResult", diff --git a/client/src/components/Panels/utilities.test.ts b/client/src/components/Panels/utilities.test.ts index 3b892f4ad03..aebd84d4bbb 100644 --- a/client/src/components/Panels/utilities.test.ts +++ b/client/src/components/Panels/utilities.test.ts @@ -12,6 +12,7 @@ import { getValidPanelItems, getValidToolsInEachSection, searchObjectsByKeys, + searchTools, type SearchCommonKeys, } from "./utilities"; import type { Tool, ToolPanelItem, ToolSection, ToolSectionLabel } from "@/stores/toolStore"; @@ -493,3 +494,102 @@ describe("getValidPanelItems", () => { expect(result["testlabel1"]).toBeDefined(); }); }); + +describe("searchTools tool-lineage de-duplication", () => { + // Regression for #23151: each placed version of a tool showed up as its own search result. + const pgt38 = "toolshed.g2.bx.psu.edu/repos/iuc/pygenometracks/pygenomeTracks/3.8+galaxy2"; + const pgt39 = "toolshed.g2.bx.psu.edu/repos/iuc/pygenometracks/pygenomeTracks/3.9+galaxy0"; + + const twoVersionTools = [ + { id: pgt38, name: "pyGenomeTracks", description: "Plot genomic tracks", version: "3.8+galaxy2" }, + { id: pgt39, name: "pyGenomeTracks", description: "Plot genomic tracks", version: "3.9+galaxy0" }, + ] as unknown as Tool[]; + + const twoSectionPanel = { + graph_display_data: { + model_class: "ToolSection", + id: "graph_display_data", + name: "Graph/Display Data", + tools: [pgt38], + }, + plots: { + model_class: "ToolSection", + id: "plots", + name: "Plots", + tools: [pgt39], + }, + } as unknown as Record; + + it.each([ + ["older version listed first", twoVersionTools], + ["newer version listed first", [...twoVersionTools].reverse()], + ])("collapses a tool placed at multiple versions to its latest version (%s)", (_order, tools) => { + const { results, resultPanel } = searchTools(tools, "pyGenomeTracks", twoSectionPanel); + + // Equally ranked versions of one lineage collapse to the latest, whatever the API order + expect(results).toEqual([pgt39]); + + // The section left empty by the dropped version is removed. + expect(Object.keys(resultPanel)).toEqual(["plots"]); + expect((resultPanel.plots as ToolSection).tools).toEqual([pgt39]); + }); + + it("compares versions numerically when picking the latest", () => { + const pgt310 = "toolshed.g2.bx.psu.edu/repos/iuc/pygenometracks/pygenomeTracks/3.10+galaxy0"; + const tools = [ + ...twoVersionTools, + { id: pgt310, name: "pyGenomeTracks", description: "Plot genomic tracks", version: "3.10+galaxy0" }, + ] as unknown as Tool[]; + const panel = { + plots: { model_class: "ToolSection", id: "plots", name: "Plots", tools: [pgt39, pgt310, pgt38] }, + } as unknown as Record; + + const { results } = searchTools(tools, "pyGenomeTracks", panel); + expect(results).toEqual([pgt310]); + }); + + it("keeps the better-ranked version when versions match the query differently", () => { + const tools = [ + { id: pgt38, name: "pyGenomeTracks", description: "Plot genomic tracks", version: "3.8+galaxy2" }, + { id: pgt39, name: "pyGenomeTracks plot", description: "Plot genomic tracks", version: "3.9+galaxy0" }, + ] as unknown as Tool[]; + + // 3.8 is an exact name match and outranks the newer 3.9, which only starts with the query + const { results } = searchTools(tools, "pyGenomeTracks", twoSectionPanel); + expect(results).toEqual([pgt38]); + + // Search rank still beats a favorited version. + const { results: withFavorite } = searchTools(tools, "pyGenomeTracks", twoSectionPanel, [pgt39]); + expect(withFavorite).toEqual([pgt38]); + }); + + it("keeps a favorited version over a newer, equally ranked one", () => { + const { results, resultPanel } = searchTools(twoVersionTools, "pyGenomeTracks", twoSectionPanel, [pgt38]); + + expect(results).toEqual([pgt38]); + expect(Object.keys(resultPanel)).toEqual(["graph_display_data"]); + }); + + it("keeps distinct tools that merely share a search term", () => { + const distinctTools = [ + { id: "pygenometracks/1.0", name: "pyGenomeTracks", description: "Plot tracks", version: "1.0" }, + { + id: "pygenometracks_utils/1.0", + name: "pyGenomeTracks utils", + description: "Track helpers", + version: "1.0", + }, + ] as unknown as Tool[]; + const panel = { + plots: { + model_class: "ToolSection", + id: "plots", + name: "Plots", + tools: ["pygenometracks/1.0", "pygenometracks_utils/1.0"], + }, + } as unknown as Record; + + const { results } = searchTools(distinctTools, "pyGenomeTracks", panel); + expect(results).toHaveLength(2); + }); +}); diff --git a/client/src/components/Panels/utilities.ts b/client/src/components/Panels/utilities.ts index fa6c34be948..3352784f5c5 100644 --- a/client/src/components/Panels/utilities.ts +++ b/client/src/components/Panels/utilities.ts @@ -23,6 +23,7 @@ import type { ToolSectionLabel, } from "@/stores/toolStore"; import levenshteinDistance from "@/utils/levenshtein"; +import { compareToolVersions } from "@/utils/tool-version"; export const FAVORITES_KEYS = ["#favs", "#favorites", "#favourites"]; @@ -407,6 +408,7 @@ export function getValidPanelItems( * @param tools - toolbox * @param query - a search query * @param currentPanel - current ToolPanel with { section_id: { tools: [tool ids] }, ... } + * @param favoriteToolIds - the user's favorite tool ids, preferred when collapsing tool versions * @returns an object containing * - results: array of tool ids that match the query * - resultPanel: a ToolPanel with only the results for the currentPanel @@ -418,6 +420,7 @@ export function searchTools( tools: Tool[], query: string, currentPanel: Record, + favoriteToolIds: string[] = [], ): { results: string[]; resultPanel: Record; @@ -428,7 +431,95 @@ export function searchTools( "description", ]); const { idResults, resultPanel } = createSortedResultPanel(matchedResults, currentPanel); - return { results: idResults, resultPanel: resultPanel, closestTerm: closestTerm }; + // Placed versions of one tool would otherwise render as duplicates (#23151). + const orderById = new Map(matchedResults.map((match) => [match.id, match.order])); + const { results, resultPanel: dedupedPanel } = dedupeResultsByToolLineage( + idResults, + resultPanel, + tools, + orderById, + new Set(favoriteToolIds), + ); + return { results, resultPanel: dedupedPanel, closestTerm: closestTerm }; +} + +/** + * Collapses search results that belong to the same tool lineage (the same tool + * installed at multiple versions) down to a single result. The best-ranked version + * wins; among equally ranked versions a favorited one wins, then the latest one, so + * the result does not depend on the order the tools API lists versions in. Also + * prunes the sectioned `resultPanel` so the dropped ids no longer render, removing + * any section left empty as a result. + * + * @param idResults ordered result tool ids from `createSortedResultPanel` + * @param resultPanel sectioned result panel from `createSortedResultPanel` + * @param tools the searched tools, used to resolve each id's version + * @param orderById search rank of each result id (higher ranks first) + * @param favoriteToolIds favorited tool ids, preferred over newer versions of equal rank + * @returns the de-duplicated `results` and matching `resultPanel` + */ +export function dedupeResultsByToolLineage( + idResults: string[], + resultPanel: Record, + tools: Tool[], + orderById: Map = new Map(), + favoriteToolIds: Set = new Set(), +): { results: string[]; resultPanel: Record } { + const toolById = new Map(tools.map((tool) => [tool.id, tool])); + const keptByLineage = new Map(); + for (const id of idResults) { + const lineage = getVersionlessToolId(id, toolById.get(id)); + const keptId = keptByLineage.get(lineage); + if (keptId === undefined || isPreferredLineageResult(id, keptId, toolById, orderById, favoriteToolIds)) { + keptByLineage.set(lineage, id); + } + } + // Map keeps first-insertion order, so each lineage stays at its best-ranked slot. + const results = Array.from(keptByLineage.values()); + + // Nothing collapsed: keep the original panel untouched. + if (results.length === idResults.length) { + return { results: idResults, resultPanel }; + } + + const keptIds = new Set(results); + const prunedPanel: Record = {}; + for (const [key, item] of Object.entries(resultPanel)) { + if (isToolSection(item)) { + const sectionTools = (item.tools ?? []).filter( + (toolId) => typeof toolId !== "string" || keptIds.has(toolId), + ); + if (sectionTools.length > 0) { + prunedPanel[key] = { ...item, tools: sectionTools }; + } + } else if (isTool(item)) { + if (keptIds.has(item.id)) { + prunedPanel[key] = item; + } + } else { + prunedPanel[key] = item; + } + } + return { results, resultPanel: prunedPanel }; +} + +function isPreferredLineageResult( + candidateId: string, + keptId: string, + toolById: Map, + orderById: Map, + favoriteToolIds: Set, +) { + const candidateOrder = orderById.get(candidateId) ?? 0; + const keptOrder = orderById.get(keptId) ?? 0; + if (candidateOrder !== keptOrder) { + return candidateOrder > keptOrder; + } + const candidateFavorite = favoriteToolIds.has(candidateId); + if (candidateFavorite !== favoriteToolIds.has(keptId)) { + return candidateFavorite; + } + return compareToolVersions(toolById.get(candidateId)?.version, toolById.get(keptId)?.version) > 0; } export function searchSections(sections: ToolSection[], query: string) { diff --git a/client/src/utils/tool-version.ts b/client/src/utils/tool-version.ts index 36bb4d2e5ae..e056b02830b 100644 --- a/client/src/utils/tool-version.ts +++ b/client/src/utils/tool-version.ts @@ -71,6 +71,14 @@ export function parseRequestedToolParts(toolId: string): RequestedToolParts { return { tool_shed_id: toolShedId, name: segments.join("/"), requested_version: requestedVersion }; } +/** + * Compares two tool versions with a natural (numeric-aware) sort; negative when `a` is older than `b`. + * A missing version counts as "0". + */ +export function compareToolVersions(a: string | undefined, b: string | undefined): number { + return (a || "0").localeCompare(b || "0", undefined, { numeric: true }); +} + /** * Filters a list of tools to show only the latest version from each lineage */ @@ -94,12 +102,7 @@ export function filterLatestToolVersions(tools: Tool[]): Tool[] { latestTools.push(toolGroup[0]); } else if (toolGroup.length > 1) { // Sort by version (descending) and take the first one - const sorted = toolGroup.sort((a, b) => { - // Compare versions using natural sort - const versionA = a.version || "0"; - const versionB = b.version || "0"; - return versionB.localeCompare(versionA, undefined, { numeric: true }); - }); + const sorted = toolGroup.sort((a, b) => compareToolVersions(b.version, a.version)); if (sorted[0]) { latestTools.push(sorted[0]); } diff --git a/lib/galaxy/authnz/psa_authnz.py b/lib/galaxy/authnz/psa_authnz.py index 84d7624f1a9..ade22beec9b 100644 --- a/lib/galaxy/authnz/psa_authnz.py +++ b/lib/galaxy/authnz/psa_authnz.py @@ -369,6 +369,9 @@ class PSAAuthnz(IdentityProvider): strategy = Strategy(trans.request, {}, Storage, self.config) backend = self._load_backend(strategy, self.config["redirect_uri"]) redirect = do_auth(backend) + # The post-login destination travels in the validated login-next cookie. + # Do not persist social-core's own copy of ``next``. + strategy.session.pop("next", None) state = strategy.session_get(f"{backend.name}_state") flow_state.save("authentication", strategy.session, identifier=authentication_id(state)) return redirect diff --git a/lib/galaxy/managers/datatypes.py b/lib/galaxy/managers/datatypes.py index f49dec233e4..76101f28666 100644 --- a/lib/galaxy/managers/datatypes.py +++ b/lib/galaxy/managers/datatypes.py @@ -165,8 +165,11 @@ def get_preferred_visualization(datatypes_registry: Registry, datatype_extension datatypes_conf.xml configuration file. These mappings determine which visualization plugin should be used by default when viewing datasets of a specific type. - If no direct mapping exists for the extension, this method will walk up the inheritance - chain to find a preferred visualization from a parent datatype class. + If no direct mapping exists for the extension, this method walks the datatype's class + inheritance chain (its MRO) and returns the mapping of the nearest ancestor class that + has one. Datatypes declared with ``subclass="true"`` each get their own class derived + from the parent, so they inherit the parent's mapping but not the mappings of sibling + ``subclass="true"`` datatypes of the same parent. Example configuration: @@ -181,22 +184,15 @@ def get_preferred_visualization(datatypes_registry: Registry, datatype_extension if not current_datatype: return None - # Use the same mapping approach as the datatypes API for consistency - mapping_data = view_mapping(datatypes_registry) - - current_class_name = mapping_data.ext_to_class_name.get(datatype_extension) - if not current_class_name: - return None - - current_class_mappings = mapping_data.class_to_classes.get(current_class_name, {}) - + mapping_by_class: dict[type, dict[str, str]] = {} for ext, visualization_mapping in datatypes_registry.visualization_mappings.items(): - if ext == datatype_extension: - continue + datatype = datatypes_registry.get_datatype_by_extension(ext) + if datatype: + mapping_by_class.setdefault(type(datatype), visualization_mapping) - parent_class_name = mapping_data.ext_to_class_name.get(ext) - if parent_class_name and parent_class_name in current_class_mappings: - return visualization_mapping + for cls in type(current_datatype).__mro__: + if cls in mapping_by_class: + return mapping_by_class[cls] return None diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index 0561a7f13a2..074791f6e70 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -89,6 +89,7 @@ from ._types import ( from .wait import ( DEFAULT_POLLING_BACKOFF, DEFAULT_POLLING_DELTA, + TimeoutAssertionError, wait_on, ) @@ -536,7 +537,20 @@ class GalaxyInteractorApi: compare_expected_metadata_to_api_response(metadata, dataset) def wait_for_job(self, job_id: str, history_id: str | None = None, maxseconds=DEFAULT_TOOL_TEST_WAIT) -> None: - self.wait_for(lambda: self.__job_ready(job_id, history_id), maxseconds=maxseconds) + try: + self.wait_for(lambda: self.__job_ready(job_id, history_id), maxseconds=maxseconds) + except TimeoutAssertionError: + # Nobody will wait for this job anymore, stop it so it doesn't keep + # occupying a job runner slot needed by subsequent tests. + self._cancel_job(job_id) + raise + + def _cancel_job(self, job_id: str) -> None: + try: + response = self._delete(f"jobs/{job_id}") + response.raise_for_status() + except Exception as e: + log.warning("Failed to cancel job %s after timeout: %s", job_id, e) def wait_on_tool_request(self, tool_request_id: str): def state(): diff --git a/lib/galaxy/tools/data_fetch.py b/lib/galaxy/tools/data_fetch.py index 767d0759690..02f36f669c8 100644 --- a/lib/galaxy/tools/data_fetch.py +++ b/lib/galaxy/tools/data_fetch.py @@ -710,7 +710,8 @@ def _has_src_to_path( metadata_out=source_metadata, ) except Exception as e: - raise Exception(f"Failed to fetch url {url}. {str(e)}") + # Reason before URL: the HDA info column keeps only the first 255 characters. + raise Exception(f"Failed to fetch url: {str(e)} ({url})") from e if not is_dataset: # Actual target dataset will validate and put results in dict diff --git a/packages/package-build-install.sh b/packages/package-build-install.sh index e9f7c191221..91ef6c84da3 100755 --- a/packages/package-build-install.sh +++ b/packages/package-build-install.sh @@ -8,7 +8,6 @@ set -euo pipefail : "${PACKAGE_LIST_FILE:=packages_by_dep_dag.txt}" -: "${PIP_EXTRA_ARGS:=--extra-index-url https://wheels.galaxyproject.org}" #: ${SETUP_VENV:=true} INSTALL=true @@ -16,18 +15,53 @@ EDITABLE=false META=false WHEELHOUSE= -if command -v uv >/dev/null; then - VENV_CMD="$(command -v uv) venv" - PIP_CMD="$(command -v uv) pip" -else - VENV_CMD='python3 -m venv' - PIP_CMD='pip' +# Use one environment for package build dependencies and the installed Galaxy. +: "${VENV:=.venv}" +case "$VENV" in + /*) ;; + *) VENV="$(pwd)/$VENV" ;; +esac +export VENV + +if [ -z "${PIP_EXTRA_ARGS:-}" ]; then + if command -v uv >/dev/null; then + PIP_EXTRA_ARGS="--index-strategy unsafe-best-match --extra-index-url https://wheels.galaxyproject.org/simple" + else + PIP_EXTRA_ARGS="--extra-index-url https://wheels.galaxyproject.org/simple" + fi fi -# Prevent making a venv for build deps for each package, Used inside the package dir, so refers to -# /packages/.venv -: "${VENV=../.venv}" -export VENV +ensure_venv() { + if [ ! -d "$VENV" ]; then + if command -v uv >/dev/null; then + uv venv "$VENV" + else + python3 -m venv "$VENV" + fi + fi +} + +pip_install() { + ensure_venv + if command -v uv >/dev/null; then + uv pip install --python "$VENV/bin/python" "$@" + else + "$VENV/bin/python" -m pip install "$@" + fi +} + +build_release_packages() { + if command -v galaxy-release-util >/dev/null && galaxy-release-util --help >/dev/null 2>&1; then + galaxy-release-util build --galaxy-root .. + elif command -v uvx >/dev/null; then + local release_util_requirement + release_util_requirement=$(grep '^galaxy-release-util==' ../lib/galaxy/dependencies/dev-requirements.txt) + uvx --from "$release_util_requirement" galaxy-release-util build --galaxy-root .. + else + echo "ERROR: galaxy-release-util is required to build the Galaxy metapackage." >&2 + exit 1 + fi +} trap_handler() { if [ -n "$WHEELHOUSE" ]; then @@ -70,30 +104,34 @@ if [ -n "$up_to" ] && [ ! -d "$up_to" ]; then exit 1 fi -while read -r package; do - [ -n "$package" ] || continue - if $INSTALL && [[ $package == meta ]] && ! $META; then continue; fi - printf "\n========= PACKAGE %s =========\n\n" "$package" - pushd "$package" - if $EDITABLE; then - ${PIP_CMD} install -e . - else - if [ ! -d "$VENV" ]; then - ${VENV_CMD} "$VENV" - VIRTUAL_ENV="${VENV}" ${PIP_CMD} install -r dev-requirements.txt +if $META && ! $EDITABLE; then + build_release_packages +else + while read -r package; do + [ -n "$package" ] || continue + if $INSTALL && [[ $package == meta ]] && ! $META; then continue; fi + printf "\n========= PACKAGE %s =========\n\n" "$package" + pushd "$package" + if $EDITABLE; then + pip_install -e . + else + if [ ! -d "$VENV" ]; then + ensure_venv + pip_install -r dev-requirements.txt + fi + make dist + if $INSTALL && ! $META; then + pip_install dist/*.whl + fi fi - make dist - if $INSTALL && ! $META; then - ${PIP_CMD} install dist/*.whl - fi - fi - popd - [ "$package" != "$up_to" ] || exit -done < "$PACKAGE_LIST_FILE" + popd + [ "$package" != "$up_to" ] || exit + done < "$PACKAGE_LIST_FILE" +fi if $INSTALL && $META && ! $EDITABLE; then WHEELHOUSE=$(mktemp -d -t gxpkgwheelhouseXXXXXX) cp ./*/dist/*.whl "$WHEELHOUSE" # shellcheck disable=SC2086 - ${PIP_CMD} install ${PIP_EXTRA_ARGS} --find-links "$WHEELHOUSE" meta/dist/*.whl + pip_install ${PIP_EXTRA_ARGS} --find-links "$WHEELHOUSE" meta/dist/*.whl fi diff --git a/test/unit/app/managers/test_datatypes.py b/test/unit/app/managers/test_datatypes.py new file mode 100644 index 00000000000..4d8ef5ead9c --- /dev/null +++ b/test/unit/app/managers/test_datatypes.py @@ -0,0 +1,23 @@ +from galaxy.datatypes.registry import example_datatype_registry_for_sample +from galaxy.managers.datatypes import get_preferred_visualization + + +def test_preferred_visualization_direct_mapping(): + registry = example_datatype_registry_for_sample() + mapping = get_preferred_visualization(registry, "hyphy_results.json") + assert mapping + assert mapping["visualization"] == "hyphyvision" + + +def test_preferred_visualization_not_inherited_by_sibling_subclass(): + registry = example_datatype_registry_for_sample() + for extension in ("anvio_state", "freq.json", "hivtrace"): + assert get_preferred_visualization(registry, extension) is None + + +def test_preferred_visualization_inherited_from_parent_datatype(): + registry = example_datatype_registry_for_sample() + for extension, visualization in (("mbi", "h5web"), ("ome.tiff", "tiffviewer"), ("probam", "igv")): + mapping = get_preferred_visualization(registry, extension) + assert mapping + assert mapping["visualization"] == visualization diff --git a/test/unit/app/tools/test_data_fetch.py b/test/unit/app/tools/test_data_fetch.py index db47cce20b4..95df6f6f60e 100644 --- a/test/unit/app/tools/test_data_fetch.py +++ b/test/unit/app/tools/test_data_fetch.py @@ -469,7 +469,8 @@ def test_hdas_single_url_error(test_http_server): assert "error_message" not in elements[0] assert "error_message" in elements[1] error = elements[1]["error_message"] - assert f"Failed to fetch url {url_12_bed}" in error + assert error.startswith("Failed to fetch url: 404 Client Error") + assert url_12_bed in error def test_hdca_collection_element_failed(test_http_server): @@ -503,7 +504,8 @@ def test_hdca_collection_element_failed(test_http_server): output = _unnamed_output(execute_context) assert "error_message" in output error = output["error_message"] - assert f"Failed to fetch url {url_12_bed}" in error + assert "- Failed to fetch url: 404 Client Error" in error + assert url_12_bed in error def test_hdca_allow_failed_collections(test_http_server): @@ -543,7 +545,43 @@ def test_hdca_allow_failed_collections(test_http_server): assert "error_message" not in elements[0] assert "error_message" in elements[1] error = elements[1]["error_message"] - assert f"Failed to fetch url {url_12_bed}" in error + assert error.startswith("Failed to fetch url: 404 Client Error") + assert url_12_bed in error + + +def test_hdas_long_url_error_reason_survives_truncation(test_http_server): + long_url = ( + test_http_server.get_url( + remote_url="https://raw.githubusercontent.com/galaxyproject/galaxy/dev/test-data/12.bed", + status=404, + body="Not Found", + ) + + "?" + + "&".join(f"param{i}=value{i}" for i in range(30)) + ) + assert len(long_url) > 255 + with _execute_context(allow_localhost=True) as execute_context: + request = { + "targets": [ + { + "destination": { + "type": "hdas", + }, + "elements": [ + { + "src": "url", + "url": long_url, + }, + ], + } + ] + } + execute_context.execute_request(request) + output = _unnamed_output(execute_context) + error = output["elements"][0]["error_message"] + assert long_url in error + # HDA info is a TrimmedString(255), so the failure reason must come before the URL. + assert "404 Client Error" in error[:255] def test_hdca_failed_expansion(): diff --git a/test/unit/app/tools/test_expression_basics.py b/test/unit/app/tools/test_expression_basics.py index 42ea7a8af96..592d23159dd 100644 --- a/test/unit/app/tools/test_expression_basics.py +++ b/test/unit/app/tools/test_expression_basics.py @@ -320,7 +320,8 @@ def test_timeout_kills_worker_under_waiting_wrapper(tmp_path, short_timeout_grac pid = int(pid_file.read_text()) try: child = psutil.Process(pid) - assert child.status() == psutil.STATUS_ZOMBIE + # The orphaned worker is reaped asynchronously by init or a subreaper. + assert child.status() in (psutil.STATUS_ZOMBIE, psutil.STATUS_DEAD) except psutil.NoSuchProcess: pass diff --git a/test/unit/authnz/test_deferred_user_creation.py b/test/unit/authnz/test_deferred_user_creation.py index 1c97396756a..69145fae215 100644 --- a/test/unit/authnz/test_deferred_user_creation.py +++ b/test/unit/authnz/test_deferred_user_creation.py @@ -265,6 +265,18 @@ def test_google_confirmation_preserves_backend_identity(trans, provider, monkeyp assert association.user.email == "verified@example.com" +def test_authentication_state_omits_social_core_next(trans, provider): + trans.request = Request.blank( + "https://galaxy.example/authnz/keycloak/login", POST={"next": "/workflows/login/start"} + ) + query = start(trans, provider) + record = trans.sa_session.query(model.PSAPartial).filter_by(next_step=-1).one() + session = json.loads(record.data)["payload"] + assert "next" not in session + assert session["keycloak_state"] == query["state"][0] + assert session["pkce_code_verifier"] + + @pytest.mark.parametrize("invalid", ["signature", "subject", "state", "policy"]) def test_failed_callback_never_issues_confirmation(trans, provider, invalid, monkeypatch): query = start(trans, provider) diff --git a/test/unit/tool_util/verify/test_interactor_wait.py b/test/unit/tool_util/verify/test_interactor_wait.py new file mode 100644 index 00000000000..3b7cc24f42a --- /dev/null +++ b/test/unit/tool_util/verify/test_interactor_wait.py @@ -0,0 +1,38 @@ +from collections.abc import Iterator +from contextlib import contextmanager +from unittest import mock + +import pytest + +from galaxy.tool_util.verify.interactor import GalaxyInteractorApi +from galaxy.tool_util.verify.wait import TimeoutAssertionError + + +@contextmanager +def _interactor(job_state: str) -> Iterator[tuple[GalaxyInteractorApi, mock.MagicMock]]: + interactor = GalaxyInteractorApi(galaxy_url="http://localhost:8080", master_api_key=None, api_key="key") + job_response = mock.Mock() + job_response.json.return_value = {"state": job_state} + with mock.patch.object(interactor, "_get", return_value=job_response): + with mock.patch.object(interactor, "_delete") as delete: + yield interactor, delete + + +def test_wait_for_job_cancels_job_on_timeout(): + with _interactor("running") as (interactor, delete): + with pytest.raises(TimeoutAssertionError): + interactor.wait_for_job("job1", maxseconds=1) + delete.assert_called_once_with("jobs/job1") + + +def test_wait_for_job_timeout_raised_if_cancel_fails(): + with _interactor("running") as (interactor, delete): + delete.side_effect = Exception("connection refused") + with pytest.raises(TimeoutAssertionError): + interactor.wait_for_job("job1", maxseconds=1) + + +def test_wait_for_job_does_not_cancel_finished_job(): + with _interactor("ok") as (interactor, delete): + interactor.wait_for_job("job1", maxseconds=1) + delete.assert_not_called()