From b20b07fe11baffbcaedaccf35c0459cd331dac1a Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Wed, 8 Mar 2023 09:13:59 -0500 Subject: [PATCH 1/4] Refactor history `query_filters` Related filter code in backend --- lib/galaxy/managers/history_contents.py | 71 ++++++++++++++----------- lib/galaxy/managers/job_connections.py | 13 ++--- 2 files changed, 46 insertions(+), 38 deletions(-) diff --git a/lib/galaxy/managers/history_contents.py b/lib/galaxy/managers/history_contents.py index d5734aa2aa7..ba1e9dd4bb1 100644 --- a/lib/galaxy/managers/history_contents.py +++ b/lib/galaxy/managers/history_contents.py @@ -531,36 +531,11 @@ class HistoryContentsFilters( def parse_query_filters_with_relations(self, query_filters: ValueFilterQueryParams, history_id): """Parse query filters but consider case where related filter is included.""" - if query_filters.q and query_filters.qv and "related-eq" in query_filters.q: - qv_index = query_filters.q.index("related-eq") - qv_hid = query_filters.qv[qv_index] - - # Type check whether hid is int - if not qv_hid.isdigit(): - raise glx_exceptions.RequestParameterInvalidException( - "unparsable value for filter", - column="related", - operation="eq", - value=qv_hid, - ValueError="invalid type in filter", - ) - - # Make new q and qv excluding related filter - new_q = [x for i, x in enumerate(query_filters.q) if i != qv_index] - new_qv = [x for i, x in enumerate(query_filters.qv) if i != qv_index] - - # Get list of related item hids from job_connections manager - job_connections_manager = JobConnectionsManager(self.app.model.session) - related = job_connections_manager.get_related_hids(history_id, qv_hid) - - # Make new query_filters with updated list of related hids for given hid - new_q.append("related-eq") - new_qv.append(json.dumps(related)) - query_filters_with_relations = ValueFilterQueryParams( - q=new_q, - qv=new_qv, - ) - return super().parse_query_filters(query_filters_with_relations) + if query_filters.q and query_filters.qv: + has_related_q = [q for q in ('related-eq', 'related') if q in query_filters.q] + if has_related_q: + query_filters_with_relations = self.get_query_filters_with_relations(query_filters=query_filters, related_q=has_related_q[0], history_id=history_id) + return super().parse_query_filters(query_filters_with_relations) return super().parse_query_filters(query_filters) def _parse_orm_filter(self, attr, op, val): @@ -617,6 +592,40 @@ class HistoryContentsFilters( return self.parsed_filter(filter_type="orm", filter=column_filter) return super()._parse_orm_filter(attr, op, val) + def get_query_filters_with_relations(self, query_filters: ValueFilterQueryParams, related_q: str, history_id): + """Return `query_filters_with_relations` changing `related:hid` to `related:[hid1, hid2, ...]`.""" + if query_filters.q and query_filters.qv: + qv_index = query_filters.q.index(related_q) + qv_hid = query_filters.qv[qv_index] + + # Type check whether hid is int + if not qv_hid.isdigit(): + raise glx_exceptions.RequestParameterInvalidException( + "unparsable value for related filter", + column="related", + operation="eq", + value=qv_hid, + ValueError="invalid type in filter", + ) + + # Make new q and qv excluding related filter + new_q = [x for i, x in enumerate(query_filters.q) if i != qv_index] + new_qv = [x for i, x in enumerate(query_filters.qv) if i != qv_index] + + # Get list of related item hids from job_connections manager + job_connections_manager = JobConnectionsManager(self.app.model.session) + related = job_connections_manager.get_related_hids(history_id, int(qv_hid)) + + # Make new query_filters with updated list of related hids for given hid + new_q.append("related-eq") + new_qv.append(json.dumps(related)) + query_filters_with_relations = ValueFilterQueryParams( + q=new_q, + qv=new_qv, + ) + return query_filters_with_relations + return query_filters + def decode_type_id(self, type_id): TYPE_ID_SEP = "-" split = type_id.split(TYPE_ID_SEP, 1) @@ -638,8 +647,6 @@ class HistoryContentsFilters( self.orm_filter_parsers.update( { "history_content_type": {"op": ("eq")}, - # maybe remove related from here, as there's no corresponding field? - "related": {"op": ("eq")}, "type_id": {"op": ("eq", "in"), "val": self.parse_type_id_list}, "hid": {"op": ("eq", "ge", "le", "gt", "lt"), "val": int}, # TODO: needs a different val parser - but no way to add to the above diff --git a/lib/galaxy/managers/job_connections.py b/lib/galaxy/managers/job_connections.py index df3363ceee5..cd9f3500307 100644 --- a/lib/galaxy/managers/job_connections.py +++ b/lib/galaxy/managers/job_connections.py @@ -10,6 +10,7 @@ from sqlalchemy.sql import ( from galaxy import model from galaxy.managers.base import get_class from galaxy.model.scoped_session import galaxy_scoped_session +from galaxy.schema.fields import DecodedDatabaseIdField class JobConnectionsManager: @@ -18,7 +19,7 @@ class JobConnectionsManager: def __init__(self, sa_session: galaxy_scoped_session): self.sa_session = sa_session - def get_connections_graph(self, id, src): + def get_connections_graph(self, id: DecodedDatabaseIdField, src: str): """Get connections graph of inputs and outputs for given item id""" if src == "HistoryDatasetAssociation": output_selects = self.outputs_derived_from_input_hda(id) @@ -34,7 +35,7 @@ class JobConnectionsManager: result["inputs"] = self._get_union_results(*input_selects) return result - def get_related_hids(self, history_id, hid): + def get_related_hids(self, history_id: DecodedDatabaseIdField, hid: int): """Get connections graph of inputs and outputs for given item hid from the given history_id""" # Get id(s) and src(s) for the given hid items_by_hid = self.sa_session.execute( @@ -63,7 +64,7 @@ class JobConnectionsManager: result.append({"src": row.src, "id": row.id}) return result - def outputs_derived_from_input_hda(self, input_hda_id): + def outputs_derived_from_input_hda(self, input_hda_id: DecodedDatabaseIdField): hda_select = ( select( [ @@ -94,7 +95,7 @@ class JobConnectionsManager: ) return hda_select, hdca_select - def outputs_derived_from_input_hdca(self, input_hdca_id): + def outputs_derived_from_input_hdca(self, input_hdca_id: DecodedDatabaseIdField): hda_select = ( select( [ @@ -126,7 +127,7 @@ class JobConnectionsManager: ) return hda_select, hdca_select - def inputs_for_hda(self, input_hda_id): + def inputs_for_hda(self, input_hda_id: DecodedDatabaseIdField): input_hdas = ( select( [ @@ -157,7 +158,7 @@ class JobConnectionsManager: ) return input_hdas, input_hdcas - def inputs_for_hdca(self, input_hdca_id): + def inputs_for_hdca(self, input_hdca_id: DecodedDatabaseIdField): input_hdas = ( select( [ From b72486f03319992af2ffcf50201afe2dac4b99a3 Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Mon, 13 Mar 2023 17:03:13 -0400 Subject: [PATCH 2/4] Add backend test for `related:hid` filter --- lib/galaxy/managers/history_contents.py | 9 ++--- lib/galaxy/managers/job_connections.py | 13 +++---- lib/galaxy_test/api/test_history_contents.py | 40 ++++++++++++++++++++ 3 files changed, 50 insertions(+), 12 deletions(-) diff --git a/lib/galaxy/managers/history_contents.py b/lib/galaxy/managers/history_contents.py index ba1e9dd4bb1..cca7fa8589f 100644 --- a/lib/galaxy/managers/history_contents.py +++ b/lib/galaxy/managers/history_contents.py @@ -531,11 +531,10 @@ class HistoryContentsFilters( def parse_query_filters_with_relations(self, query_filters: ValueFilterQueryParams, history_id): """Parse query filters but consider case where related filter is included.""" - if query_filters.q and query_filters.qv: - has_related_q = [q for q in ('related-eq', 'related') if q in query_filters.q] - if has_related_q: - query_filters_with_relations = self.get_query_filters_with_relations(query_filters=query_filters, related_q=has_related_q[0], history_id=history_id) - return super().parse_query_filters(query_filters_with_relations) + has_related_q = [q for q in ('related-eq', 'related') if query_filters.q and q in query_filters.q] + if has_related_q: + query_filters_with_relations = self.get_query_filters_with_relations(query_filters=query_filters, related_q=has_related_q[0], history_id=history_id) + return super().parse_query_filters(query_filters_with_relations) return super().parse_query_filters(query_filters) def _parse_orm_filter(self, attr, op, val): diff --git a/lib/galaxy/managers/job_connections.py b/lib/galaxy/managers/job_connections.py index cd9f3500307..e387f36527a 100644 --- a/lib/galaxy/managers/job_connections.py +++ b/lib/galaxy/managers/job_connections.py @@ -10,7 +10,6 @@ from sqlalchemy.sql import ( from galaxy import model from galaxy.managers.base import get_class from galaxy.model.scoped_session import galaxy_scoped_session -from galaxy.schema.fields import DecodedDatabaseIdField class JobConnectionsManager: @@ -19,7 +18,7 @@ class JobConnectionsManager: def __init__(self, sa_session: galaxy_scoped_session): self.sa_session = sa_session - def get_connections_graph(self, id: DecodedDatabaseIdField, src: str): + def get_connections_graph(self, id: int, src: str): """Get connections graph of inputs and outputs for given item id""" if src == "HistoryDatasetAssociation": output_selects = self.outputs_derived_from_input_hda(id) @@ -35,7 +34,7 @@ class JobConnectionsManager: result["inputs"] = self._get_union_results(*input_selects) return result - def get_related_hids(self, history_id: DecodedDatabaseIdField, hid: int): + def get_related_hids(self, history_id, hid: int): """Get connections graph of inputs and outputs for given item hid from the given history_id""" # Get id(s) and src(s) for the given hid items_by_hid = self.sa_session.execute( @@ -64,7 +63,7 @@ class JobConnectionsManager: result.append({"src": row.src, "id": row.id}) return result - def outputs_derived_from_input_hda(self, input_hda_id: DecodedDatabaseIdField): + def outputs_derived_from_input_hda(self, input_hda_id: int): hda_select = ( select( [ @@ -95,7 +94,7 @@ class JobConnectionsManager: ) return hda_select, hdca_select - def outputs_derived_from_input_hdca(self, input_hdca_id: DecodedDatabaseIdField): + def outputs_derived_from_input_hdca(self, input_hdca_id: int): hda_select = ( select( [ @@ -127,7 +126,7 @@ class JobConnectionsManager: ) return hda_select, hdca_select - def inputs_for_hda(self, input_hda_id: DecodedDatabaseIdField): + def inputs_for_hda(self, input_hda_id: int): input_hdas = ( select( [ @@ -158,7 +157,7 @@ class JobConnectionsManager: ) return input_hdas, input_hdcas - def inputs_for_hdca(self, input_hdca_id: DecodedDatabaseIdField): + def inputs_for_hdca(self, input_hdca_id: int): input_hdas = ( select( [ diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index 427f6178053..a6ac0053d3e 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -737,6 +737,46 @@ class TestHistoryContentsApi(ApiTestCase): ).json() assert len(contents_response) == 0 + @skip_without_tool("cat_data_and_sleep") + def test_index_filter_by_related_items(self, history_id): + # initialise history with 2 datasets + input_hda_id = self.dataset_populator.new_dataset(history_id)["id"] + unrelated_hid = self.dataset_populator.new_dataset(history_id)["hid"] + self.dataset_populator.wait_for_history_jobs(history_id) + + # Run tool on first dataset to get 3rd, related dataset + inputs = { + "input1": {"src": "hda", "id": input_hda_id}, + "sleep_time": 10, + } + run_response = self.dataset_populator.run_tool_raw( + "cat_data_and_sleep", + inputs, + history_id, + ) + self.dataset_populator.wait_for_history_jobs(history_id) + related_hid = run_response.json()["outputs"][0]["hid"] + + # Test q = related-eq, for related items + contents_response = self._get( + f"histories/{history_id}/contents?v=dev&q=related-eq&qv={related_hid}" + ).json() + assert len(contents_response) == 2 + + # Test q = related, for unrelated item + contents_response = self._get( + f"histories/{history_id}/contents?v=dev&q=related&qv={unrelated_hid}" + ).json() + assert len(contents_response) == 1 + + # Test error case: qv is string + related_hid = "one" + contents_response = self._get( + f"histories/{history_id}/contents?v=dev&q=related-eq&qv={related_hid}" + ) + assert contents_response.status_code == 400 + assert contents_response.json()["err_msg"] == "unparsable value for related filter" + def test_elements_datatypes_field(self, history_id): collection_name = "homogeneous" expected_datatypes = ["txt"] From 84405e102aa57e85e06b1f37c817767068ba2430 Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Mon, 13 Mar 2023 17:30:40 -0400 Subject: [PATCH 3/4] Remove waits from related-hid filter api test, lint fixes --- lib/galaxy/managers/history_contents.py | 6 ++++-- lib/galaxy_test/api/test_history_contents.py | 16 ++++------------ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/managers/history_contents.py b/lib/galaxy/managers/history_contents.py index cca7fa8589f..7de47a459d9 100644 --- a/lib/galaxy/managers/history_contents.py +++ b/lib/galaxy/managers/history_contents.py @@ -531,9 +531,11 @@ class HistoryContentsFilters( def parse_query_filters_with_relations(self, query_filters: ValueFilterQueryParams, history_id): """Parse query filters but consider case where related filter is included.""" - has_related_q = [q for q in ('related-eq', 'related') if query_filters.q and q in query_filters.q] + has_related_q = [q for q in ("related-eq", "related") if query_filters.q and q in query_filters.q] if has_related_q: - query_filters_with_relations = self.get_query_filters_with_relations(query_filters=query_filters, related_q=has_related_q[0], history_id=history_id) + query_filters_with_relations = self.get_query_filters_with_relations( + query_filters=query_filters, related_q=has_related_q[0], history_id=history_id + ) return super().parse_query_filters(query_filters_with_relations) return super().parse_query_filters(query_filters) diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index a6ac0053d3e..8f5cc22883b 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -742,7 +742,6 @@ class TestHistoryContentsApi(ApiTestCase): # initialise history with 2 datasets input_hda_id = self.dataset_populator.new_dataset(history_id)["id"] unrelated_hid = self.dataset_populator.new_dataset(history_id)["hid"] - self.dataset_populator.wait_for_history_jobs(history_id) # Run tool on first dataset to get 3rd, related dataset inputs = { @@ -754,26 +753,19 @@ class TestHistoryContentsApi(ApiTestCase): inputs, history_id, ) - self.dataset_populator.wait_for_history_jobs(history_id) related_hid = run_response.json()["outputs"][0]["hid"] # Test q = related-eq, for related items - contents_response = self._get( - f"histories/{history_id}/contents?v=dev&q=related-eq&qv={related_hid}" - ).json() + contents_response = self._get(f"histories/{history_id}/contents?v=dev&q=related-eq&qv={related_hid}").json() assert len(contents_response) == 2 # Test q = related, for unrelated item - contents_response = self._get( - f"histories/{history_id}/contents?v=dev&q=related&qv={unrelated_hid}" - ).json() + contents_response = self._get(f"histories/{history_id}/contents?v=dev&q=related&qv={unrelated_hid}").json() assert len(contents_response) == 1 # Test error case: qv is string - related_hid = "one" - contents_response = self._get( - f"histories/{history_id}/contents?v=dev&q=related-eq&qv={related_hid}" - ) + related_qv = "one" + contents_response = self._get(f"histories/{history_id}/contents?v=dev&q=related-eq&qv={related_qv}") assert contents_response.status_code == 400 assert contents_response.json()["err_msg"] == "unparsable value for related filter" From 7e11c57bd73a34cd729ae278ea779809b1155e33 Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Tue, 14 Mar 2023 11:25:31 -0400 Subject: [PATCH 4/4] move type checking back to `parse_query_filters_with_relations` --- lib/galaxy/managers/history_contents.py | 25 +++++++++++--------- lib/galaxy_test/api/test_history_contents.py | 2 +- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/galaxy/managers/history_contents.py b/lib/galaxy/managers/history_contents.py index 7de47a459d9..068020b63d5 100644 --- a/lib/galaxy/managers/history_contents.py +++ b/lib/galaxy/managers/history_contents.py @@ -532,7 +532,20 @@ class HistoryContentsFilters( def parse_query_filters_with_relations(self, query_filters: ValueFilterQueryParams, history_id): """Parse query filters but consider case where related filter is included.""" has_related_q = [q for q in ("related-eq", "related") if query_filters.q and q in query_filters.q] - if has_related_q: + if query_filters.q and query_filters.qv and has_related_q: + qv_index = query_filters.q.index(has_related_q[0]) + qv_hid = query_filters.qv[qv_index] + + # Type check whether hid is int + if not qv_hid.isdigit(): + raise glx_exceptions.RequestParameterInvalidException( + "unparsable value for related filter", + column="related", + operation="eq", + value=qv_hid, + ValueError="invalid type in filter", + ) + query_filters_with_relations = self.get_query_filters_with_relations( query_filters=query_filters, related_q=has_related_q[0], history_id=history_id ) @@ -599,16 +612,6 @@ class HistoryContentsFilters( qv_index = query_filters.q.index(related_q) qv_hid = query_filters.qv[qv_index] - # Type check whether hid is int - if not qv_hid.isdigit(): - raise glx_exceptions.RequestParameterInvalidException( - "unparsable value for related filter", - column="related", - operation="eq", - value=qv_hid, - ValueError="invalid type in filter", - ) - # Make new q and qv excluding related filter new_q = [x for i, x in enumerate(query_filters.q) if i != qv_index] new_qv = [x for i, x in enumerate(query_filters.qv) if i != qv_index] diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index 8f5cc22883b..f92bdd4577f 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -746,7 +746,7 @@ class TestHistoryContentsApi(ApiTestCase): # Run tool on first dataset to get 3rd, related dataset inputs = { "input1": {"src": "hda", "id": input_hda_id}, - "sleep_time": 10, + "sleep_time": 0, } run_response = self.dataset_populator.run_tool_raw( "cat_data_and_sleep",