From 414156af8aa6f9dd42cc96abe5c183262cbdd44c Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 15 Aug 2022 17:55:24 +0200 Subject: [PATCH 1/4] Fix data_range_{min,max} parsing All datetime values established via our sqlalchemy models don't include `tzinfo`. Fixes https://github.com/galaxyproject/galaxy/issues/14094#issuecomment-1215166187. In general this is a bad thing, we should definitely be honoring timezone info, but the rest of the app just assumes UTC, so this will work. --- lib/galaxy/schema/schema.py | 9 ++++++--- lib/galaxy/schema/types.py | 15 +++++++++++++++ lib/galaxy/webapps/galaxy/api/jobs.py | 5 +++-- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index b863546c343..cbbad2608ea 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -39,7 +39,10 @@ from galaxy.schema.fields import ( EncodedDatabaseIdField, ModelClassField, ) -from galaxy.schema.types import RelativeUrl +from galaxy.schema.types import ( + OffsetNaiveDatetime, + RelativeUrl, +) USER_MODEL_CLASS_NAME = "User" GROUP_MODEL_CLASS_NAME = "Group" @@ -1112,8 +1115,8 @@ class JobIndexQueryPayload(Model): user_id: Optional[DecodedDatabaseIdField] = None tool_ids: Optional[List[str]] = None tool_ids_like: Optional[List[str]] = None - date_range_min: Optional[Union[datetime, date]] = None - date_range_max: Optional[Union[datetime, date]] = None + date_range_min: Optional[Union[OffsetNaiveDatetime, date]] = None + date_range_max: Optional[Union[OffsetNaiveDatetime, date]] = None history_id: Optional[DecodedDatabaseIdField] = None workflow_id: Optional[DecodedDatabaseIdField] = None invocation_id: Optional[DecodedDatabaseIdField] = None diff --git a/lib/galaxy/schema/types.py b/lib/galaxy/schema/types.py index cf39c391d3a..c78c57767c5 100644 --- a/lib/galaxy/schema/types.py +++ b/lib/galaxy/schema/types.py @@ -1,3 +1,6 @@ +from datetime import datetime + +from pydantic.datetime_parse import parse_datetime from typing_extensions import Literal # Relative URLs cannot be validated with AnyUrl, they need a scheme. @@ -5,3 +8,15 @@ from typing_extensions import Literal RelativeUrl = str LatestLiteral = Literal["latest"] + + +class OffsetNaiveDatetime(datetime): + @classmethod + def __get_validators__(cls): + yield cls.validate + + @classmethod + def validate(cls, v): + v = parse_datetime(v) + v = v.replace(tzinfo=None) + return v diff --git a/lib/galaxy/webapps/galaxy/api/jobs.py b/lib/galaxy/webapps/galaxy/api/jobs.py index d88ba5c40f8..1eb13167b58 100644 --- a/lib/galaxy/webapps/galaxy/api/jobs.py +++ b/lib/galaxy/webapps/galaxy/api/jobs.py @@ -38,6 +38,7 @@ 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, @@ -103,13 +104,13 @@ ToolIdLikeQueryParam: Optional[str] = Query( description="Limit listing of jobs to those that match one of the included tool ID sql-like patterns. If none, all are returned", ) -DateRangeMinQueryParam: Optional[Union[datetime, date]] = Query( +DateRangeMinQueryParam: Optional[Union[OffsetNaiveDatetime, date]] = Query( default=None, title="Date Range Minimum", description="Limit listing of jobs to those that are updated after specified date (e.g. '2014-01-01')", ) -DateRangeMaxQueryParam: Optional[Union[datetime, date]] = Query( +DateRangeMaxQueryParam: Optional[Union[OffsetNaiveDatetime, date]] = Query( default=None, title="Date Range Maximum", description="Limit listing of jobs to those that are updated before specified date (e.g. '2014-01-01')", From cd76fef415507e78fe1d1d48ff1099cd78f5fa7f Mon Sep 17 00:00:00 2001 From: Marius van den Beek Date: Tue, 16 Aug 2022 13:11:21 +0200 Subject: [PATCH 2/4] Convert to UTC instead of stripping timezone Co-authored-by: Nicola Soranzo --- lib/galaxy/schema/types.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/galaxy/schema/types.py b/lib/galaxy/schema/types.py index c78c57767c5..49b42126907 100644 --- a/lib/galaxy/schema/types.py +++ b/lib/galaxy/schema/types.py @@ -18,5 +18,4 @@ class OffsetNaiveDatetime(datetime): @classmethod def validate(cls, v): v = parse_datetime(v) - v = v.replace(tzinfo=None) - return v + return v.replace(tzinfo=None) - v.utcoffset() if v.tzinfo else v From b0d9cff6c4ae7052607131b156bcd46af5fd8edc Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 16 Aug 2022 13:11:54 +0200 Subject: [PATCH 3/4] Add unit test That was meant to go into the first commit ... --- test/unit/webapps/api/test_datetime_parsing.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 test/unit/webapps/api/test_datetime_parsing.py diff --git a/test/unit/webapps/api/test_datetime_parsing.py b/test/unit/webapps/api/test_datetime_parsing.py new file mode 100644 index 00000000000..7b979449ee5 --- /dev/null +++ b/test/unit/webapps/api/test_datetime_parsing.py @@ -0,0 +1,14 @@ +from pydantic import BaseModel + +from galaxy.schema.types import OffsetNaiveDatetime + + +class Time(BaseModel): + time: OffsetNaiveDatetime + + +def test_naive_datetime_parsing(): + with_zulu = Time(time="2022-08-15T11:29:32.853974Z") + without_zulu = Time(time="2022-08-15T11:29:32.853974") + assert with_zulu.time == without_zulu.time + assert not with_zulu.time.tzinfo From 973d0779b65993f0252531533717790fea759870 Mon Sep 17 00:00:00 2001 From: Marius van den Beek Date: Wed, 17 Aug 2022 18:43:57 +0200 Subject: [PATCH 4/4] Include timezone testing Co-authored-by: Nicola Soranzo --- test/unit/webapps/api/test_datetime_parsing.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/webapps/api/test_datetime_parsing.py b/test/unit/webapps/api/test_datetime_parsing.py index 7b979449ee5..29ea26b14ff 100644 --- a/test/unit/webapps/api/test_datetime_parsing.py +++ b/test/unit/webapps/api/test_datetime_parsing.py @@ -8,7 +8,7 @@ class Time(BaseModel): def test_naive_datetime_parsing(): - with_zulu = Time(time="2022-08-15T11:29:32.853974Z") - without_zulu = Time(time="2022-08-15T11:29:32.853974") - assert with_zulu.time == without_zulu.time - assert not with_zulu.time.tzinfo + with_tz = Time(time="2022-08-15T11:29:32.853974+02:00") + without_tz = Time(time="2022-08-15T09:29:32.853974") + assert with_tz.time == without_tz.time + assert with_tz.time.tzinfo is None