From 1a103279afd10c4125f3e2a2a99ece6e6a2357b7 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Thu, 18 Aug 2022 18:37:10 +0200 Subject: [PATCH 1/4] Add `query_parameter_as_list` dependable Supports declaring query parameters as 'listified' --- lib/galaxy/webapps/galaxy/api/common.py | 42 +++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/lib/galaxy/webapps/galaxy/api/common.py b/lib/galaxy/webapps/galaxy/api/common.py index 6b53eab1877..bb7a26bcdb9 100644 --- a/lib/galaxy/webapps/galaxy/api/common.py +++ b/lib/galaxy/webapps/galaxy/api/common.py @@ -18,6 +18,7 @@ from galaxy.schema import ( ValueFilterQueryParams, ) from galaxy.schema.schema import UpdateDatasetPermissionsPayload +from galaxy.util import listify SerializationViewQueryParam: Optional[str] = Query( None, @@ -142,3 +143,44 @@ def get_query_parameters_from_request_excluding(request: Request, exclude: Set[s for param_name in exclude: extra_params.pop(param_name, None) return extra_params + + +def query_parameter_as_list(query): + """Used as FastAPI dependable for query parameters that need to behave as a list of values separated by comma + or as multiple instances of the same parameter. + + **Important**: the `query` annotation provided must define the `alias` exactly as the name of the actual parameter name. + + Usage example: + ```python + ValueQueryParam: Optional[str] = Query( + default=None, + alias="value", # Important! this is the parameter name that will be displayed in the API docs + title="My Value", + description="A single value, a comma-separated list of values or a list of values.", + ) + + @router.get("/api/my_route") + def index( + self, + values: Optional[List[str]] = Depends(query_parameter_as_list(ValueQueryParam)), + ): + ... + ``` + + This will render in the API docs as a single string query parameter but will make the following requests equivalent: + - `api/my_route?value=val1,val2,val3` + - `api/my_route?value=val1&value=val2&value=val3` + """ + + def parse_elements( + element: str = query, + elements: Optional[List[str]] = Query(default=None, alias=query.alias, include_in_schema=False), + ) -> Optional[List[Any]]: + if query.default != Ellipsis and element is None and not elements: + return query.default + if elements and len(elements) > 1: + return elements + return listify(element) + + return parse_elements From 05ed92688c41ed63a29d04c79c4029f5e7d32d0d Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Thu, 18 Aug 2022 18:39:34 +0200 Subject: [PATCH 2/4] Fix Jobs API index endpoint Use the `query_parameter_as_list` dependency to support multiple ways of providing lists of values while rendering the API docs as a single string query parameter. --- lib/galaxy/webapps/galaxy/api/jobs.py | 40 ++++++++++++--------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/api/jobs.py b/lib/galaxy/webapps/galaxy/api/jobs.py index 1eb13167b58..8c46228c9cb 100644 --- a/lib/galaxy/webapps/galaxy/api/jobs.py +++ b/lib/galaxy/webapps/galaxy/api/jobs.py @@ -17,7 +17,10 @@ from typing import ( Union, ) -from fastapi import Query +from fastapi import ( + Depends, + Query, +) from galaxy import ( exceptions, @@ -39,20 +42,13 @@ from galaxy.managers.jobs import ( from galaxy.schema.fields import EncodedDatabaseIdField from galaxy.schema.schema import JobIndexSortByEnum from galaxy.schema.types import OffsetNaiveDatetime -from galaxy.util import listify from galaxy.web import ( expose_api, expose_api_anonymous, require_admin, ) from galaxy.webapps.base.controller import UsesVisualizationMixin -from galaxy.webapps.galaxy.services.jobs import ( - JobIndexPayload, - JobIndexViewEnum, - JobsService, -) -from galaxy.work.context import WorkRequestContext -from . import ( +from galaxy.webapps.galaxy.api import ( BaseGalaxyAPIController, depends, DependsOnTrans, @@ -60,6 +56,13 @@ from . import ( Router, search_query_param, ) +from galaxy.webapps.galaxy.api.common import query_parameter_as_list +from galaxy.webapps.galaxy.services.jobs import ( + JobIndexPayload, + JobIndexViewEnum, + JobsService, +) +from galaxy.work.context import WorkRequestContext log = logging.getLogger(__name__) @@ -68,6 +71,7 @@ router = Router(tags=["jobs"]) StateQueryParam: Optional[str] = Query( default=None, + alias="state", title="States", description="Comma-separated list of states to filter job query on. If unspecified, jobs of any state may be returned.", ) @@ -93,6 +97,7 @@ ViewQueryParam: JobIndexViewEnum = Query( ToolIdQueryParam: Optional[str] = Query( default=None, + alias="tool_id", title="Tool ID(s)", description="Limit listing of jobs to those that match one of the included tool_ids. If none, all are returned", ) @@ -100,6 +105,7 @@ ToolIdQueryParam: Optional[str] = Query( ToolIdLikeQueryParam: Optional[str] = Query( default=None, + alias="tool_id_like", title="Tool ID Pattern(s)", description="Limit listing of jobs to those that match one of the included tool ID sql-like patterns. If none, all are returned", ) @@ -186,12 +192,12 @@ class FastAPIJobs: def index( self, trans: ProvidesUserContext = DependsOnTrans, - state: Optional[str] = StateQueryParam, + states: Optional[List[str]] = Depends(query_parameter_as_list(StateQueryParam)), user_details: bool = UserDetailsQueryParam, user_id: Optional[EncodedDatabaseIdField] = UserIdQueryParam, view: JobIndexViewEnum = ViewQueryParam, - tool_id: Optional[str] = ToolIdQueryParam, - tool_id_like: Optional[str] = ToolIdLikeQueryParam, + tool_ids: Optional[List[str]] = Depends(query_parameter_as_list(ToolIdQueryParam)), + tool_ids_like: Optional[List[str]] = Depends(query_parameter_as_list(ToolIdLikeQueryParam)), date_range_min: Optional[Union[datetime, date]] = DateRangeMinQueryParam, date_range_max: Optional[Union[datetime, date]] = DateRangeMaxQueryParam, history_id: Optional[EncodedDatabaseIdField] = HistoryIdQueryParam, @@ -202,16 +208,6 @@ class FastAPIJobs: limit: int = LimitQueryParam, offset: int = OffsetQueryParam, ) -> List[Dict[str, Any]]: - def optional_list(input: Optional[str]) -> Optional[List[str]]: - if input is None: - return None - else: - return listify(input) - - states = optional_list(state) - tool_ids = optional_list(tool_id) - tool_ids_like = optional_list(tool_id_like) - payload = JobIndexPayload( states=states, user_details=user_details, From 1d594873652e17119f2a31bd1f6dbb7dedff407b Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 19 Aug 2022 08:59:02 +0200 Subject: [PATCH 3/4] Add unit tests for `query_parameter_as_list` --- test/unit/webapps/test_query_params_lists.py | 56 ++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 test/unit/webapps/test_query_params_lists.py diff --git a/test/unit/webapps/test_query_params_lists.py b/test/unit/webapps/test_query_params_lists.py new file mode 100644 index 00000000000..6d50e07b6ab --- /dev/null +++ b/test/unit/webapps/test_query_params_lists.py @@ -0,0 +1,56 @@ +from typing import List + +from fastapi.applications import FastAPI +from fastapi.param_functions import ( + Depends, + Query, +) +from fastapi.testclient import TestClient + +from galaxy.webapps.galaxy.api.common import query_parameter_as_list + +app = FastAPI() + +client = TestClient(app) + + +@app.get("/test/get_value_as_list") +async def get_value_as_list( + values: List[str] = Depends(query_parameter_as_list(Query(alias="value"))), +): + return values + + +def test_single_value(): + query_params = "value=val" + result = _get_result_for_params(query_params) + assert len(result) == 1 + assert result[0] == "val" + + +def test_list_as_comma_separated_values(): + query_params = "value=val1,val2,val3" + result = _get_result_for_params(query_params) + + assert len(result) == 3 + assert result[0] == "val1" + assert result[1] == "val2" + assert result[2] == "val3" + + +def test_list_as_multiple_query_entries(): + query_params = "value=val1&value=val2&value=val3" + result = _get_result_for_params(query_params) + + assert len(result) == 3 + assert result[0] == "val1" + assert result[1] == "val2" + assert result[2] == "val3" + + +def _get_result_for_params(query_params: str): + response = client.get(f"/test/get_value_as_list?{query_params}") + + assert response.status_code == 200 + result = response.json() + return result From 1545b9da99bfe3218767e7f7fe234e8739171f89 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 19 Aug 2022 12:30:53 +0200 Subject: [PATCH 4/4] Simplify `query_parameter_as_list` And render the repeated parameter input in the docs. The first parameter can be a comma-separated list of values --- lib/galaxy/webapps/galaxy/api/common.py | 13 ++++++------- lib/galaxy/webapps/galaxy/api/jobs.py | 8 ++++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/api/common.py b/lib/galaxy/webapps/galaxy/api/common.py index bb7a26bcdb9..063d9a6d56d 100644 --- a/lib/galaxy/webapps/galaxy/api/common.py +++ b/lib/galaxy/webapps/galaxy/api/common.py @@ -153,7 +153,7 @@ def query_parameter_as_list(query): Usage example: ```python - ValueQueryParam: Optional[str] = Query( + ValueQueryParam = Query( default=None, alias="value", # Important! this is the parameter name that will be displayed in the API docs title="My Value", @@ -174,13 +174,12 @@ def query_parameter_as_list(query): """ def parse_elements( - element: str = query, - elements: Optional[List[str]] = Query(default=None, alias=query.alias, include_in_schema=False), + elements: Optional[List[str]] = query, ) -> Optional[List[Any]]: - if query.default != Ellipsis and element is None and not elements: + if query.default != Ellipsis and not elements: return query.default - if elements and len(elements) > 1: - return elements - return listify(element) + if elements and len(elements) == 1: + return listify(elements[0]) + return elements return parse_elements diff --git a/lib/galaxy/webapps/galaxy/api/jobs.py b/lib/galaxy/webapps/galaxy/api/jobs.py index 8c46228c9cb..46390412b7f 100644 --- a/lib/galaxy/webapps/galaxy/api/jobs.py +++ b/lib/galaxy/webapps/galaxy/api/jobs.py @@ -69,11 +69,11 @@ log = logging.getLogger(__name__) router = Router(tags=["jobs"]) -StateQueryParam: Optional[str] = Query( +StateQueryParam = Query( default=None, alias="state", title="States", - description="Comma-separated list of states to filter job query on. If unspecified, jobs of any state may be returned.", + description="A list or comma-separated list of states to filter job query on. If unspecified, jobs of any state may be returned.", ) UserDetailsQueryParam: bool = Query( @@ -95,7 +95,7 @@ ViewQueryParam: JobIndexViewEnum = Query( ) -ToolIdQueryParam: Optional[str] = Query( +ToolIdQueryParam = Query( default=None, alias="tool_id", title="Tool ID(s)", @@ -103,7 +103,7 @@ ToolIdQueryParam: Optional[str] = Query( ) -ToolIdLikeQueryParam: Optional[str] = Query( +ToolIdLikeQueryParam = Query( default=None, alias="tool_id_like", title="Tool ID Pattern(s)",