Serve download redirects from a dedicated /download route

Redirecting /display to a presigned object-store URL was a non-obvious
breaking change: curl does not follow redirects by default and requests
does not follow them for HEAD, and it only happened for certain object
stores. Instead, leave /display untouched (it always streams through
Galaxy) and add an explicit /download route whose contract is that
clients must follow a 302.

- New GET/HEAD /api/datasets/{id}/download and
  /api/histories/{history_id}/contents/{id}/download. GET redirects (302)
  to a presigned URL when the backend supports it, otherwise streams the
  whole file (zipping composite datatypes) exactly as before. HEAD answers
  from object-store metadata (size, filename) without redirecting or
  pulling into cache, so HEAD clients still get the size.
- Revert the redirect from /display; drop the DirectDownloadUrl sentinel.
- Point the web UI download actions and the HDA download_url field at the
  new route. (bioblend will move to it in a follow-up in its own repo.)

Refs galaxyproject/galaxy#19255
This commit is contained in:
Nuwan Goonasekera
2026-07-07 23:11:02 +05:30
parent 6779e15e6e
commit 8324eb268f
10 changed files with 394 additions and 48 deletions
+248 -14
View File
@@ -983,6 +983,30 @@ export interface paths {
patch?: never;
trace?: never;
};
"/api/datasets/{history_content_id}/download": {
parameters: {
query?: never;
header?: never;
path?: never;
cookie?: never;
};
/**
* Downloads the dataset, redirecting to the object store when possible.
* @description Downloads the whole dataset file. Clients must follow the 302 redirect this route may return.
*/
get: operations["download_api_datasets__history_content_id__download_get"];
put?: never;
post?: never;
delete?: never;
options?: never;
/**
* Returns download metadata (size, filename) for the dataset.
* @description Downloads the whole dataset file. Clients must follow the 302 redirect this route may return.
*/
head: operations["download_api_datasets__history_content_id__download_head"];
patch?: never;
trace?: never;
};
"/api/datasets/{history_content_id}/metadata_file": {
parameters: {
query?: never;
@@ -2529,6 +2553,30 @@ export interface paths {
patch?: never;
trace?: never;
};
"/api/histories/{history_id}/contents/{history_content_id}/download": {
parameters: {
query?: never;
header?: never;
path?: never;
cookie?: never;
};
/**
* Downloads the dataset, redirecting to the object store when possible.
* @description Downloads the whole dataset file. Clients must follow the 302 redirect this route may return.
*/
get: operations["history_contents_download_api_histories__history_id__contents__history_content_id__download_get"];
put?: never;
post?: never;
delete?: never;
options?: never;
/**
* Returns download metadata (size, filename) for the dataset.
* @description Downloads the whole dataset file. Clients must follow the 302 redirect this route may return.
*/
head: operations["history_contents_download_api_histories__history_id__contents__history_content_id__download_head"];
patch?: never;
trace?: never;
};
"/api/histories/{history_id}/contents/{history_content_id}/extra_files": {
parameters: {
query?: never;
@@ -33754,13 +33802,6 @@ export interface operations {
};
content?: never;
};
/** @description Redirect to a URL serving the dataset directly from the backing object store. Only returned for whole-file downloads when the dataset's object store has `enable_direct_download` set. */
302: {
headers: {
[name: string]: unknown;
};
content?: never;
};
/** @description Request Error */
"4XX": {
headers: {
@@ -33838,6 +33879,105 @@ export interface operations {
};
};
};
download_api_datasets__history_content_id__download_get: {
parameters: {
query?: {
/** @description The file extension when downloading the display data. Use the value `data` to let the server infer it from the data type. */
to_ext?: string | null;
};
header?: {
/** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
"run-as"?: string | null;
};
path: {
/** @description The ID of the History Dataset. */
history_content_id: string;
};
cookie?: never;
};
requestBody?: never;
responses: {
/** @description Successful Response */
200: {
headers: {
[name: string]: unknown;
};
content?: never;
};
/** @description Redirect to a URL serving the dataset directly from the backing object store. Only returned for whole-file downloads when the dataset's object store has `enable_direct_download` set. */
302: {
headers: {
[name: string]: unknown;
};
content?: never;
};
/** @description Request Error */
"4XX": {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["MessageExceptionModel"];
};
};
/** @description Server Error */
"5XX": {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["MessageExceptionModel"];
};
};
};
};
download_api_datasets__history_content_id__download_head: {
parameters: {
query?: {
/** @description The file extension when downloading the display data. Use the value `data` to let the server infer it from the data type. */
to_ext?: string | null;
};
header?: {
/** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
"run-as"?: string | null;
};
path: {
/** @description The ID of the History Dataset. */
history_content_id: string;
};
cookie?: never;
};
requestBody?: never;
responses: {
/** @description Successful Response */
200: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": unknown;
};
};
/** @description Request Error */
"4XX": {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["MessageExceptionModel"];
};
};
/** @description Server Error */
"5XX": {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["MessageExceptionModel"];
};
};
};
};
datasets__get_metadata_file: {
parameters: {
query: {
@@ -38906,13 +39046,6 @@ export interface operations {
};
content?: never;
};
/** @description Redirect to a URL serving the dataset directly from the backing object store. Only returned for whole-file downloads when the dataset's object store has `enable_direct_download` set. */
302: {
headers: {
[name: string]: unknown;
};
content?: never;
};
/** @description Request Error */
"4XX": {
headers: {
@@ -38991,6 +39124,107 @@ export interface operations {
};
};
};
history_contents_download_api_histories__history_id__contents__history_content_id__download_get: {
parameters: {
query?: {
/** @description The file extension when downloading the display data. Use the value `data` to let the server infer it from the data type. */
to_ext?: string | null;
};
header?: {
/** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
"run-as"?: string | null;
};
path: {
/** @description The ID of the History Dataset. */
history_content_id: string;
history_id: string | null;
};
cookie?: never;
};
requestBody?: never;
responses: {
/** @description Successful Response */
200: {
headers: {
[name: string]: unknown;
};
content?: never;
};
/** @description Redirect to a URL serving the dataset directly from the backing object store. Only returned for whole-file downloads when the dataset's object store has `enable_direct_download` set. */
302: {
headers: {
[name: string]: unknown;
};
content?: never;
};
/** @description Request Error */
"4XX": {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["MessageExceptionModel"];
};
};
/** @description Server Error */
"5XX": {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["MessageExceptionModel"];
};
};
};
};
history_contents_download_api_histories__history_id__contents__history_content_id__download_head: {
parameters: {
query?: {
/** @description The file extension when downloading the display data. Use the value `data` to let the server infer it from the data type. */
to_ext?: string | null;
};
header?: {
/** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
"run-as"?: string | null;
};
path: {
/** @description The ID of the History Dataset. */
history_content_id: string;
history_id: string | null;
};
cookie?: never;
};
requestBody?: never;
responses: {
/** @description Successful Response */
200: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": unknown;
};
};
/** @description Request Error */
"4XX": {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["MessageExceptionModel"];
};
};
/** @description Server Error */
"5XX": {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["MessageExceptionModel"];
};
};
};
};
extra_files_history_api_histories__history_id__contents__history_content_id__extra_files_get: {
parameters: {
query?: never;
@@ -38,7 +38,9 @@ const { isAdmin } = storeToRefs(useUserStore());
const dataset = computed(() => getDataset(props.datasetId));
const datasetUrl = computed(() => `/datasets/${props.datasetId}/display/`);
const downloadUrl = computed(() => withPrefix(`${datasetUrl.value}?to_ext=${dataset.value?.file_ext}`));
const downloadUrl = computed(() =>
withPrefix(`/api/datasets/${props.datasetId}/download?to_ext=${dataset.value?.file_ext}`),
);
const isLoading = computed(() => isLoadingDataset(props.datasetId));
const previewUrl = computed(() => `${datasetUrl.value}?preview=True`);
@@ -43,7 +43,7 @@ const iframeLoading = ref(true);
const dataset = computed(() => datasetStore.getDataset(props.datasetId));
const loadError = computed(() => datasetStore.getDatasetError(props.datasetId));
const downloadUrl = computed(() => withPrefix(`/datasets/${props.datasetId}/display`));
const downloadUrl = computed(() => withPrefix(`/api/datasets/${props.datasetId}/download`));
const headerState = computed(() => (headerCollapsed.value ? "closed" : "open"));
// Track datatype loading state
@@ -58,7 +58,7 @@ const rerunUrl = computed(() => {
return prependPath(props.itemUrls.rerun!);
});
const downloadUrl = computed(() => {
return prependPath(`api/datasets/${props.item.id}/display?to_ext=${props.item.extension}`);
return prependPath(`api/datasets/${props.item.id}/download?to_ext=${props.item.extension}`);
});
function onCopyLink() {
@@ -38,9 +38,9 @@ describe("DatasetDownload", () => {
expect(foundItems).toBe(false);
await wrapper.trigger("click");
const emitted = wrapper.emitted()["on-download"];
expect(emitted?.[0]?.[0]).toBe(`/api/datasets/item_id/display?to_ext=ext`);
expect(emitted?.[0]?.[0]).toBe(`/api/datasets/item_id/download?to_ext=ext`);
expect(emitted?.[1]?.[0]).toBe(`/api/datasets/item_id/metadata_file?metadata_file=a`);
expect(emitted?.[2]?.[0]).toBe(`/api/datasets/item_id/metadata_file?metadata_file=b`);
expect(emitted?.[3]?.[0]).toBe(`/api/datasets/item_id/display?to_ext=ext`);
expect(emitted?.[3]?.[0]).toBe(`/api/datasets/item_id/download?to_ext=ext`);
});
});
@@ -26,7 +26,7 @@ const metaDownloadUrl = computed(() => {
return prependPath(`api/datasets/${props.item.id}/metadata_file?metadata_file=`);
});
const downloadUrl = computed(() => {
return prependPath(`api/datasets/${props.item.id}/display?to_ext=${props.item.extension}`);
return prependPath(`api/datasets/${props.item.id}/download?to_ext=${props.item.extension}`);
});
const downloadTitle = computed(() => {
const size = props.item.file_size;
+1 -1
View File
@@ -628,7 +628,7 @@ class HDASerializer( # datasets._UnflattenedMetadataDatasetAssociationSerialize
),
# TODO: backwards compat: need to go away
"download_url": lambda item, key, **context: self.url_for(
"history_contents_display",
"history_contents_download",
history_id=self.app.security.encode_id(item.history.id),
history_content_id=self.app.security.encode_id(item.id),
context=context,
+58 -5
View File
@@ -70,7 +70,6 @@ from galaxy.webapps.galaxy.services.datasets import (
DatasetTextContentDetails,
DeleteDatasetBatchPayload,
DeleteDatasetBatchResult,
DirectDownloadUrl,
RequestDataType,
UpdateObjectStoreIdPayload,
)
@@ -310,7 +309,6 @@ class FastAPIDatasets:
summary="Displays (preview) or downloads dataset content.",
tags=["histories"],
response_class=StreamingResponse,
responses={302: DIRECT_DOWNLOAD_REDIRECT_RESPONSE},
)
@router.head(
"/api/histories/{history_id}/contents/{history_content_id}/display",
@@ -338,7 +336,6 @@ class FastAPIDatasets:
"/api/datasets/{history_content_id}/display",
summary="Displays (preview) or downloads dataset content.",
response_class=StreamingResponse,
responses={302: DIRECT_DOWNLOAD_REDIRECT_RESPONSE},
)
@router.head(
"/api/datasets/{history_content_id}/display",
@@ -359,6 +356,64 @@ class FastAPIDatasets:
"""Streams the dataset for download or the contents preview to be displayed in a browser."""
return self._display(request, trans, history_content_id, preview, filename, to_ext, raw, offset, ck_size)
@router.get(
"/api/histories/{history_id}/contents/{history_content_id}/download",
name="history_contents_download",
summary="Downloads the dataset, redirecting to the object store when possible.",
tags=["histories"],
response_class=StreamingResponse,
responses={302: DIRECT_DOWNLOAD_REDIRECT_RESPONSE},
)
@router.head(
"/api/histories/{history_id}/contents/{history_content_id}/download",
name="history_contents_download",
summary="Returns download metadata (size, filename) for the dataset.",
tags=["histories"],
)
def download_history_content(
self,
request: Request,
history_content_id: HistoryDatasetIDPathParam,
history_id: Optional[HistoryIDPathParam] = None,
trans=DependsOnTrans,
to_ext: Optional[str] = ToExtQueryParam,
):
"""Downloads the whole dataset file. Clients must follow the 302 redirect this route may return."""
return self._download(request, trans, history_content_id, to_ext)
@router.get(
"/api/datasets/{history_content_id}/download",
summary="Downloads the dataset, redirecting to the object store when possible.",
response_class=StreamingResponse,
responses={302: DIRECT_DOWNLOAD_REDIRECT_RESPONSE},
)
@router.head(
"/api/datasets/{history_content_id}/download",
summary="Returns download metadata (size, filename) for the dataset.",
)
def download(
self,
request: Request,
history_content_id: HistoryDatasetIDPathParam,
trans=DependsOnTrans,
to_ext: Optional[str] = ToExtQueryParam,
):
"""Downloads the whole dataset file. Clients must follow the 302 redirect this route may return."""
return self._download(request, trans, history_content_id, to_ext)
def _download(self, request: Request, trans, dataset_id: DecodedDatabaseIdField, to_ext: Optional[str]):
# Default to_ext to "data" so the route always behaves as a whole-file download (server infers
# the extension from the datatype) rather than a preview.
to_ext = to_ext or "data"
if request.method == "HEAD":
headers = self.service.download_head_headers(trans, dataset_id, to_ext)
return Response(status_code=200, headers=headers)
url = self.service.direct_download_url(trans, dataset_id, to_ext)
if url is not None:
return RedirectResponse(url, status_code=302)
# No direct URL available: stream the file through Galaxy, exactly as a /display download would.
return self._display(request, trans, dataset_id, preview=False, filename=None, to_ext=to_ext, raw=False)
def _display(
self,
request: Request,
@@ -385,8 +440,6 @@ class FastAPIDatasets:
ck_size=ck_size,
**extra_params,
)
if isinstance(display_data, DirectDownloadUrl):
return RedirectResponse(display_data.url, status_code=302, headers=headers)
if isinstance(display_data, IOBase):
file_name = getattr(display_data, "name", None)
if file_name:
+43 -14
View File
@@ -4,7 +4,6 @@ API operations on the contents of a history dataset.
import logging
import os
from dataclasses import dataclass
from enum import Enum
from typing import (
Any,
@@ -95,13 +94,6 @@ log = logging.getLogger(__name__)
DEFAULT_LIMIT = 500
@dataclass(frozen=True)
class DirectDownloadUrl:
"""Sentinel returned by `DatasetsService.display` to signal a redirect to a backing-store URL."""
url: str
def is_direct_download_candidate(filename, to_ext, raw, offset, ck_size, is_archive) -> bool:
"""Whether a display request is a plain whole-file download eligible for a direct backing-store link.
@@ -654,11 +646,23 @@ class DatasetsService(ServiceBase, UsesVisualizationMixin):
return rval
def _direct_download_url(self, trans, dataset_instance, filename, to_ext, raw, offset, ck_size) -> Optional[str]:
"""Return a backing-store URL the client can be redirected to, or None to stream as usual."""
def direct_download_url(
self,
trans: ProvidesHistoryContext,
dataset_id: DecodedDatabaseIdField,
to_ext: Optional[str] = None,
hda_ldda: DatasetSourceType = DatasetSourceType.hda,
) -> Optional[str]:
"""Return a backing-store URL a whole-file download can be redirected to, or None to stream.
Used by the dedicated download route; the regular display route never redirects.
"""
dataset_manager = self.dataset_manager_by_type[hda_ldda]
dataset_instance = dataset_manager.get_accessible(dataset_id, trans.user)
dataset_manager.ensure_dataset_on_disk(trans, dataset_instance)
datatype = dataset_instance.datatype
is_archive = datatype.is_archive_download(trans.app.datatypes_registry, dataset_instance.extension)
if not is_direct_download_candidate(filename, to_ext, raw, offset, ck_size, is_archive):
if not is_direct_download_candidate(None, to_ext, False, None, None, is_archive):
return None
content_disposition = None
content_type = None
@@ -670,6 +674,34 @@ class DatasetsService(ServiceBase, UsesVisualizationMixin):
dataset_instance.dataset, content_disposition=content_disposition, content_type=content_type
)
def download_head_headers(
self,
trans: ProvidesHistoryContext,
dataset_id: DecodedDatabaseIdField,
to_ext: Optional[str] = None,
hda_ldda: DatasetSourceType = DatasetSourceType.hda,
) -> dict[str, str]:
"""Build response headers for a HEAD download request from object-store metadata.
Answers without redirecting or pulling the object into cache, so clients (which may not follow
redirects on HEAD) can learn the size and filename of a download.
"""
dataset_manager = self.dataset_manager_by_type[hda_ldda]
dataset_instance = dataset_manager.get_accessible(dataset_id, trans.user)
dataset_manager.ensure_dataset_on_disk(trans, dataset_instance)
datatype = dataset_instance.datatype
headers = {
"content-type": "application/octet-stream",
"Content-Disposition": datatype.download_content_disposition(dataset_instance, to_ext),
"accept-ranges": "bytes",
}
# Composite/archived downloads are zipped on the fly, so their size is not known up front.
if not datatype.is_archive_download(trans.app.datatypes_registry, dataset_instance.extension):
size = trans.app.object_store.size(dataset_instance.dataset)
if size:
headers["Content-Length"] = str(size)
return headers
def display(
self,
trans: ProvidesHistoryContext,
@@ -699,9 +731,6 @@ class DatasetsService(ServiceBase, UsesVisualizationMixin):
if filename and filename.startswith("/"):
# Path needs to relative to extra files path
filename = filename.lstrip("/")
direct_url = self._direct_download_url(trans, dataset_instance, filename, to_ext, raw, offset, ck_size)
if direct_url is not None:
return DirectDownloadUrl(direct_url), headers
if raw:
if filename and filename != "index":
object_store = trans.app.object_store
@@ -1,8 +1,9 @@
"""Integration test for direct-download redirects (presigned URLs) from a remote object store.
Datasets stored in a backing object store with ``enable_direct_download`` set should be served via a
302 redirect to a URL the client fetches directly from the store, instead of being pulled through
Galaxy's cache. Uses a boto3 object store backed by a disposable minio container.
Whole-file downloads of datasets stored in a backing object store with ``enable_direct_download`` set
are served from the dedicated ``/download`` route via a 302 redirect to a URL the client fetches
directly from the store, instead of being pulled through Galaxy's cache. The ``/display`` route keeps
streaming through Galaxy (no redirect). Uses a boto3 object store backed by a disposable minio container.
"""
import os
@@ -77,10 +78,13 @@ class TestDirectDownloadRedirectIntegration(BaseObjectStoreIntegrationTestCase):
super().setUp()
self.dataset_populator = DatasetPopulator(self.galaxy_interactor)
def _download_url(self, hda_id, **params):
return self._api_url(f"datasets/{hda_id}/download", params=params, use_key=True)
def _display_url(self, hda_id, **params):
return self._api_url(f"datasets/{hda_id}/display", params=params, use_key=True)
def test_download_redirects_to_presigned_url(self):
def test_download_route_redirects_to_presigned_url(self):
history_id = self.dataset_populator.new_history()
hda = self.dataset_populator.new_dataset(history_id, content="123", wait=True)
@@ -88,7 +92,7 @@ class TestDirectDownloadRedirectIntegration(BaseObjectStoreIntegrationTestCase):
self._reset_cache()
assert files_count(self.object_store_cache_path) == 0
url = self._display_url(hda["id"], to_ext="txt")
url = self._download_url(hda["id"], to_ext="txt")
response = requests.get(url, allow_redirects=False)
assert response.status_code == 302
location = response.headers["Location"]
@@ -104,15 +108,39 @@ class TestDirectDownloadRedirectIntegration(BaseObjectStoreIntegrationTestCase):
# Galaxy served the download without pulling the object into its cache.
assert files_count(self.object_store_cache_path) == 0
def test_raw_download_redirects(self):
def test_download_route_redirects_with_inferred_extension(self):
history_id = self.dataset_populator.new_history()
hda = self.dataset_populator.new_dataset(history_id, content="raw-bytes", wait=True)
hda = self.dataset_populator.new_dataset(history_id, content="123", wait=True)
url = self._display_url(hda["id"], raw="True")
# No to_ext: the download route infers it and still redirects for a single-file dataset.
url = self._download_url(hda["id"])
response = requests.get(url, allow_redirects=False)
assert response.status_code == 302
assert OBJECT_STORE_HOST in response.headers["Location"]
def test_head_download_returns_metadata_without_redirect(self):
history_id = self.dataset_populator.new_history()
hda = self.dataset_populator.new_dataset(history_id, content="123", wait=True)
self._reset_cache()
url = self._download_url(hda["id"], to_ext="txt")
response = requests.head(url, allow_redirects=False)
# HEAD answers from object-store metadata: 200, real size, no redirect, no cache pull.
assert response.status_code == 200
assert "Location" not in response.headers
assert response.headers["Content-Length"] == str(len(b"123\n"))
assert files_count(self.object_store_cache_path) == 0
def test_display_does_not_redirect(self):
history_id = self.dataset_populator.new_history()
hda = self.dataset_populator.new_dataset(history_id, content="display-me", wait=True)
# The legacy /display route is unchanged: it streams through Galaxy, never redirects.
url = self._display_url(hda["id"], to_ext="txt")
response = requests.get(url, allow_redirects=False)
assert response.status_code == 200
assert "display-me" in response.text
def test_preview_is_not_redirected(self):
history_id = self.dataset_populator.new_history()
hda = self.dataset_populator.new_dataset(history_id, content="hello", wait=True)