From b46b1ae282998402f78ba2150f9aa60c88e70c78 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 9 Aug 2022 11:58:14 +0200 Subject: [PATCH 1/4] Add more API tests for errors in dataset details --- lib/galaxy_test/api/test_history_contents.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index 55cb716b102..511f58a4046 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -9,13 +9,13 @@ from typing import ( ) from galaxy.webapps.galaxy.services.history_contents import DirectionOptions +from galaxy_test.api._framework import ApiTestCase from galaxy_test.base.populators import ( DatasetCollectionPopulator, DatasetPopulator, LibraryPopulator, skip_without_tool, ) -from ._framework import ApiTestCase TEST_SOURCE_URI = "http://google.com/dataset.txt" TEST_HASH_FUNCTION = "MD5" @@ -166,10 +166,15 @@ class HistoryContentsApiTestCase(ApiTestCase): def test_index_detail_parameter_error(self): hda1 = self.dataset_populator.new_dataset(self.history_id) - wrong_details_query_with_empty_ids = f"details=,,{hda1['id']}" - contents_response = self._get( - f"histories/{self.history_id}/contents?v=dev&{wrong_details_query_with_empty_ids}" - ) + # Invalid details should return 400 + contents_response = self._get(f"histories/{self.history_id}/contents?v=dev&details= ") + self._assert_status_code_is(contents_response, 400) + # Empty IDs should return 400 + contents_response = self._get(f"histories/{self.history_id}/contents?v=dev&details=,,{hda1['id']}") + self._assert_status_code_is(contents_response, 400) + + # Invalid IDs should return 400 + contents_response = self._get(f"histories/{self.history_id}/contents?v=dev&details={hda1['id']}, ,{hda1['id']}") self._assert_status_code_is(contents_response, 400) def test_show_hda(self): From ef9da3426c6d7b4f3cf048d6b7adb3d2bb094832 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 9 Aug 2022 12:01:48 +0200 Subject: [PATCH 2/4] Handle history index params validation properly Instead of trying to raise an error for each possible case, just wrap the ValidationError with our custom exceptions. This will finally return a 400 instead of 500 error code when any query parameter is invalid. --- .../webapps/galaxy/api/history_contents.py | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/api/history_contents.py b/lib/galaxy/webapps/galaxy/api/history_contents.py index 6a3816883ff..414e7a83abc 100644 --- a/lib/galaxy/webapps/galaxy/api/history_contents.py +++ b/lib/galaxy/webapps/galaxy/api/history_contents.py @@ -20,6 +20,7 @@ from fastapi import ( Query, Request, ) +from pydantic.error_wrappers import ValidationError from starlette import status from starlette.responses import ( Response, @@ -27,7 +28,6 @@ from starlette.responses import ( ) from galaxy import util -from galaxy.exceptions import RequestParameterInvalidException from galaxy.managers.context import ProvidesHistoryContext from galaxy.schema import ( FilterQueryParams, @@ -58,6 +58,7 @@ from galaxy.schema.schema import ( UpdateHistoryContentsPayload, WriteStoreToPayload, ) +from galaxy.web.framework.decorators import validation_error_to_message_exception from galaxy.webapps.galaxy.api.common import ( get_filter_query_params, get_update_permission_payload, @@ -151,10 +152,13 @@ def parse_index_query_params( ) -> HistoryContentsIndexParams: """Parses query parameters for the history contents `index` operation and returns a model containing the values in the correct type.""" - return HistoryContentsIndexParams( - v=v, - dataset_details=parse_dataset_details(dataset_details), - ) + try: + return HistoryContentsIndexParams( + v=v, + dataset_details=parse_dataset_details(dataset_details), + ) + except ValidationError as e: + raise validation_error_to_message_exception(e) def get_legacy_index_query_params( @@ -238,23 +242,24 @@ def parse_legacy_index_query_params( else: dataset_details = parse_dataset_details(details) - return LegacyHistoryContentsIndexParams( - types=content_types, - ids=id_list, - deleted=deleted, - visible=visible, - dataset_details=dataset_details, - ) + try: + return LegacyHistoryContentsIndexParams( + types=content_types, + ids=id_list, + deleted=deleted, + visible=visible, + dataset_details=dataset_details, + ) + except ValidationError as e: + raise validation_error_to_message_exception(e) def parse_dataset_details(details: Optional[str]): """Parses the different values that the `dataset_details` parameter can have from a string.""" dataset_details: Optional[DatasetDetailsType] = None - if details and details != "all": + if details is not None and details != "all": dataset_details = set(util.listify(details)) - if "" in dataset_details: - raise RequestParameterInvalidException("Invalid empty IDs found in dataset details parameter") else: # either None or 'all' dataset_details = details # type: ignore return dataset_details From 9c63cb6ad7cadbeb570762b10396b573327ce727 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 9 Aug 2022 12:05:04 +0200 Subject: [PATCH 3/4] Avoid possible empty IDs in expanded datasets In some unknown situations, the expanded items IDs contains just blank spaces and provoke a Bad Request. --- client/src/store/historyStore/model/watchHistory.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/store/historyStore/model/watchHistory.js b/client/src/store/historyStore/model/watchHistory.js index c1ec1fa6376..df0b3fd7035 100644 --- a/client/src/store/historyStore/model/watchHistory.js +++ b/client/src/store/historyStore/model/watchHistory.js @@ -122,7 +122,7 @@ function getCurrentlyExpandedHistoryContentIds() { expandedItems.forEach((key) => { // Items have the format: - const itemId = key.split("-")[1]; - if (itemId) { + if (itemId?.trim()) { expandedItemIds.push(itemId); } }); From 55bb5e0f6f39b8463b27f25e0c4cf8b316e39ee1 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 9 Aug 2022 08:53:50 -0400 Subject: [PATCH 4/4] Fix number of jobs submitted display in tool success. --- client/src/components/Tool/ToolSuccess.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/components/Tool/ToolSuccess.vue b/client/src/components/Tool/ToolSuccess.vue index eeb9e8e3ed6..2a0a71089e3 100644 --- a/client/src/components/Tool/ToolSuccess.vue +++ b/client/src/components/Tool/ToolSuccess.vue @@ -66,7 +66,7 @@ export default { return this.jobResponse && this.jobResponse.jobs ? this.jobResponse.jobs.length : 0; }, nJobsText() { - return this.nJobs > 1 ? `${this.njobs} jobs` : `1 job`; + return this.nJobs > 1 ? `${this.nJobs} jobs` : `1 job`; }, nInputsText() { return this.nInputs > 1 ? `${this.nInputs} inputs` : `this input`;