Merge pull request #12382 from Nerdinacan/fix-history-contents-api

Fixes to contents/near api endpoint

client/src/components/History/caching/loadHistoryContents.js

@@ -22,7 +22,7 @@ export const clearHistoryDateStore = async () => {
  */
 // prettier-ignore
 export const loadHistoryContents = (cfg = {}) => (rawInputs$) => {
-    const { 
+    const {
         noInitial = false,
         windowSize = SearchParams.pageSize
     } = cfg;
@@ -39,7 +39,7 @@ export const loadHistoryContents = (cfg = {}) => (rawInputs$) => {
             return `${baseUrl}?${params.historyContentQueryString}`;
         }),
         map(prependPath),
-        requestWithUpdateTime({ dateStore, noInitial, responseQualifier }),
+        requestWithUpdateTime({ dateStore, noInitial, responseQualifier, dateFieldName: "since" }),
     );
 
     const validResponses$ = ajaxResponse$.pipe(
@@ -81,7 +81,7 @@ export const loadHistoryContents = (cfg = {}) => (rawInputs$) => {
 
             const matches = areDefined(matchesUp, matchesDown) ? matchesUp + matchesDown : undefined;
             const totalMatches = areDefined(totalMatchesUp, totalMatchesDown) ? totalMatchesUp + totalMatchesDown : undefined;
-            
+
             return {
                 summary,
                 matches,

lib/galaxy/model/migrate/versions/0177_update_job_state_summary.py

@@ -0,0 +1,24 @@
+"""
+Update job-state-summary view for hdca elements to include job directly tied with the hdca
+"""
+
+import logging
+
+from galaxy.model.view import HistoryDatasetCollectionJobStateSummary
+from galaxy.model.view.utils import CreateView, DropView
+
+log = logging.getLogger(__name__)
+
+
+def upgrade(migrate_engine):
+    print(__doc__)
+    # drop first because sqlite does not support or_replace
+    downgrade(migrate_engine)
+    view = HistoryDatasetCollectionJobStateSummary
+    create_view = CreateView(view.name, view.__view__)
+    migrate_engine.execute(create_view)
+
+
+def downgrade(migrate_engine):
+    drop_view = DropView(HistoryDatasetCollectionJobStateSummary.name)
+    migrate_engine.execute(drop_view)

lib/galaxy/model/view/__init__.py

@@ -9,7 +9,7 @@ from galaxy.model.view.utils import View
 
 AGGREGATE_STATE_QUERY = """
 SELECT
-    hdca.id as hdca_id,
+    hdca_id,
     SUM(CASE WHEN state = 'new' THEN 1 ELSE 0 END) AS new,
     SUM(CASE WHEN state = 'resubmitted' THEN 1 ELSE 0 END) AS resubmitted,
     SUM(CASE WHEN state = 'waiting' THEN 1 ELSE 0 END) AS waiting,
@@ -22,15 +22,25 @@ SELECT
     SUM(CASE WHEN state = 'deleted' THEN 1 ELSE 0 END) AS deleted,
     SUM(CASE WHEN state = 'deleted_new' THEN 1 ELSE 0 END) AS deleted_new,
     SUM(CASE WHEN state = 'upload' THEN 1 ELSE 0 END) AS upload,
-    SUM(CASE WHEN job.id IS NOT NULL THEN 1 ELSE 0 END) AS all_jobs
-FROM history_dataset_collection_association hdca
-LEFT JOIN implicit_collection_jobs icj
-    ON icj.id = hdca.implicit_collection_jobs_id
-LEFT JOIN implicit_collection_jobs_job_association icjja
-    ON icj.id = icjja.implicit_collection_jobs_id
-LEFT JOIN job
-    ON icjja.job_id = job.id
-GROUP BY hdca.id
+    SUM(CASE WHEN job_id IS NOT NULL THEN 1 ELSE 0 END) AS all_jobs
+FROM (
+    SELECT hdca.id AS hdca_id, job.id AS job_id, job.state as state
+    FROM history_dataset_collection_association hdca
+    LEFT JOIN implicit_collection_jobs icj
+        ON icj.id = hdca.implicit_collection_jobs_id
+    LEFT JOIN implicit_collection_jobs_job_association icjja
+        ON icj.id = icjja.implicit_collection_jobs_id
+    LEFT JOIN job
+        ON icjja.job_id = job.id
+
+    UNION
+
+    SELECT hdca.id AS hdca_id, job.id AS job_id, job.state AS state
+    FROM history_dataset_collection_association hdca
+    LEFT JOIN job
+        ON hdca.job_id = job.id
+) jobstates
+GROUP BY jobstates.hdca_id
 """
 
 

lib/galaxy/webapps/galaxy/api/history_contents.py

@@ -787,11 +787,11 @@ class HistoriesContentsService(ServiceBase):
         # while polling, check to see if the history has changed
         # if it hasn't then we can short-circuit the poll request
         if since:
-            # sqlalchemy DateTime columns are not timezone aware, so parse `since` into timezone-aware
-            # datetime and then convert to naive datetime object representing UTC,
-            # assuming history.update_time represents UTC time.
-            since_date = since.astimezone(datetime.timezone.utc).replace(tzinfo=None)
-            if history.update_time <= since_date:
+            # sqlalchemy DateTime columns are not timezone aware, but `since` may be a timezone aware
+            # DateTime object if a timezone offset is provided (https://github.com/samuelcolvin/pydantic/blob/5ccbdcb5904f35834300b01432a665c75dc02296/pydantic/datetime_parse.py#L179).
+            # If a timezone is provided (since.tzinfo is not None) we convert to UTC and remove tzinfo so that comparison with history.update_time is correct.
+            since = since if since.tzinfo is None else since.astimezone(datetime.timezone.utc).replace(tzinfo=None)
+            if history.update_time <= since:
                 trans.response.status = 204
                 return
 
@@ -1769,12 +1769,16 @@ class HistoryContentsController(BaseGalaxyAPIController, UsesLibraryMixinItems,
         * GET /api/histories/{history_id}/contents/near/{hid}/{limit}
         """
         serialization_params = parse_serialization_params(default_view='betawebclient', **kwd)
+
+        since_str = kwd.pop('since', None)
+        if since_str:
+            since = dateutil.parser.isoparse(since_str)
+        else:
+            since = None
+
         filter_params = self._parse_rest_params(kwd)
         hid = int(hid)
         limit = int(limit)
-        since = kwd.get('update_time-gt', None)
-        if since:
-            since = dateutil.parser.isoparse(since)
 
         return self.service.contents_near(
             trans, history_id, serialization_params, filter_params, hid, limit, since,

lib/galaxy_test/api/test_history_contents.py

@@ -1,5 +1,6 @@
 import json
 import time
+import urllib
 from datetime import datetime
 
 from requests import delete, put
@@ -585,7 +586,7 @@ class HistoryContentsApiTestCase(ApiTestCase):
             assert isinstance(c['job_state_summary'], dict)
 
     def _get_content(self, history_id, update_time):
-        return self._get(f"/api/histories/{history_id}/contents/near/100/100?update_time-ge={update_time}").json()
+        return self._get(f"/api/histories/{history_id}/contents/near/100/100?update_time-gt={update_time}").json()
 
     def test_history_contents_near_with_update_time(self):
         with self.dataset_populator.test_history() as history_id:
@@ -597,6 +598,63 @@ class HistoryContentsApiTestCase(ApiTestCase):
             all_datasets_finished = first_time = datetime.utcnow().isoformat()
             assert len(self._get_content(history_id, update_time=all_datasets_finished)) == 0
 
+    def test_history_contents_near_with_since(self):
+        with self.dataset_populator.test_history() as history_id:
+            original_history = self._get(f"/api/histories/{history_id}").json()
+            original_history_stamp = original_history['update_time']
+
+            # check empty contents, with no since flag, should return an empty 200 result
+            history_contents = self._get(f"/api/histories/{history_id}/contents/near/100/100")
+            assert history_contents.status_code == 200
+            assert len(history_contents.json()) == 0
+
+            # adding a since parameter, should return a 204 if history has not changed at all
+            history_contents = self._get(f"/api/histories/{history_id}/contents/near/100/100?since={original_history_stamp}")
+            assert history_contents.status_code == 204
+
+            # add some stuff
+            self.dataset_collection_populator.create_list_in_history(history_id=history_id)
+            self.dataset_populator.wait_for_history(history_id)
+
+            # check to make sure the added stuff is there
+            changed_history_contents = self._get(f"/api/histories/{history_id}/contents/near/100/100")
+            assert changed_history_contents.status_code == 200
+            assert len(changed_history_contents.json()) == 4
+
+            # check to make sure the history date has actually changed due to changing the contents
+            changed_history = self._get(f"/api/histories/{history_id}").json()
+            changed_history_stamp = changed_history['update_time']
+            assert original_history_stamp != changed_history_stamp
+
+            # a repeated contents request with since=original_history_stamp should now return data
+            # because we have added datasets and the update_time should have been changed
+            changed_content = self._get(f"/api/histories/{history_id}/contents/near/100/100?since={original_history_stamp}")
+            assert changed_content.status_code == 200
+            assert len(changed_content.json()) == 4
+
+    def test_history_contents_near_since_with_standard_iso8601_date(self):
+        with self.dataset_populator.test_history() as history_id:
+            original_history = self._get(f"/api/histories/{history_id}").json()
+            original_history_stamp = original_history['update_time']
+
+            # this is the standard date format that javascript will emit using .toISOString(), it
+            # should be the expected date format for any modern api
+            # https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toISOString
+
+            # checking to make sure that the same exact history.update_time returns a "not changed"
+            # result after date parsing
+            valid_iso8601_date = original_history_stamp + 'Z'
+            encoded_valid_date = urllib.parse.quote_plus(valid_iso8601_date)
+            history_contents = self._get(f"/api/histories/{history_id}/contents/near/100/100?since={encoded_valid_date}")
+            assert history_contents.status_code == 204
+
+            # test parsing for other standard is08601 formats
+            sample_formats = ['2021-08-26T15:53:02+00:00', '2021-08-26T15:53:02Z', '20210826T155302Z', '2002-10-10T12:00:00-05:00']
+            for date_str in sample_formats:
+                encoded_date = urllib.parse.quote_plus(date_str)  # handles pluses, minuses
+                history_contents = self._get(f"/api/histories/{history_id}/contents/near/100/100?since={encoded_date}")
+                self._assert_status_code_is_ok(history_contents)
+
     @skip_without_tool('cat_data_and_sleep')
     def test_history_contents_near_with_update_time_implicit_collection(self):
         with self.dataset_populator.test_history() as history_id: