From a3c0e4bbce87433b2c856f1c6f0b3c172a545244 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 10 Mar 2021 14:13:21 -0500 Subject: [PATCH 1/8] An approach to copying collections. --- lib/galaxy/managers/collections.py | 25 ++++++++--- lib/galaxy/model/__init__.py | 18 ++++++-- lib/galaxy/tools/force_to_datatype.xml | 25 +++++++++++ .../webapps/galaxy/api/history_contents.py | 12 +++++- lib/galaxy_test/api/test_history_contents.py | 43 +++++++++++++++++++ 5 files changed, 113 insertions(+), 10 deletions(-) create mode 100644 lib/galaxy/tools/force_to_datatype.xml diff --git a/lib/galaxy/managers/collections.py b/lib/galaxy/managers/collections.py index c45942401e9..fa09788009e 100644 --- a/lib/galaxy/managers/collections.py +++ b/lib/galaxy/managers/collections.py @@ -105,7 +105,8 @@ class DatasetCollectionManager: def create(self, trans, parent, name, collection_type, element_identifiers=None, elements=None, implicit_collection_info=None, trusted_identifiers=None, hide_source_items=False, tags=None, copy_elements=False, history=None, - set_hid=True, flush=True, completed_job=None, output_name=None): + set_hid=True, flush=True, completed_job=None, output_name=None, + copy_dataset_instance_attributes=None): """ PRECONDITION: security checks on ability to add to parent occurred during load. @@ -128,6 +129,7 @@ class DatasetCollectionManager: elements=elements, hide_source_items=hide_source_items, copy_elements=copy_elements, + copy_dataset_instance_attributes=copy_dataset_instance_attributes, history=history, ) @@ -184,7 +186,7 @@ class DatasetCollectionManager: return self.__persist(dataset_collection_instance, flush=flush) def create_dataset_collection(self, trans, collection_type, element_identifiers=None, elements=None, - hide_source_items=None, copy_elements=False, history=None): + hide_source_items=None, copy_elements=False, history=None, copy_dataset_instance_attributes=None): # Make sure at least one of these is None. assert element_identifiers is None or elements is None @@ -203,6 +205,7 @@ class DatasetCollectionManager: element_identifiers=element_identifiers, hide_source_items=hide_source_items, copy_elements=copy_elements, + copy_dataset_instance_attributes=copy_dataset_instance_attributes, history=history) else: if has_subcollections: @@ -224,6 +227,7 @@ class DatasetCollectionManager: element_identifiers, hide_source_items=False, copy_elements=False, + copy_dataset_instance_attributes=None, history=None): if collection_type_description.has_subcollections(): # Nested collection - recursively create collections and update identifiers. @@ -236,12 +240,14 @@ class DatasetCollectionManager: element_identifiers=element_identifier['element_identifiers'], hide_source_items=hide_source_items, copy_elements=copy_elements, + copy_dataset_instance_attributes=copy_dataset_instance_attributes, history=history) if not new_collection: elements = self.__load_elements(trans=trans, element_identifiers=element_identifiers, hide_source_items=hide_source_items, copy_elements=copy_elements, + copy_dataset_instance_attributes=copy_dataset_instance_attributes, history=history) return elements @@ -299,7 +305,7 @@ class DatasetCollectionManager: changed = self._set_from_dict(trans, dataset_collection_instance, payload) return changed - def copy(self, trans, parent, source, encoded_source_id, copy_elements=False): + def copy(self, trans, parent, source, encoded_source_id, copy_elements=False, copy_dataset_instance_attributes=None): """ PRECONDITION: security checks on ability to add to parent occurred during load. @@ -308,7 +314,9 @@ class DatasetCollectionManager: source_hdca = self.__get_history_collection_instance(trans, encoded_source_id) copy_kwds = {} if copy_elements: - copy_kwds["element_destination"] = parent + copy_kwds["element_destination"] = parent # e.g. a history + if copy_dataset_instance_attributes is not None: + copy_kwds["copy_dataset_instance_attributes"] = copy_dataset_instance_attributes new_hdca = source_hdca.copy(**copy_kwds) new_hdca.copy_tags_from(target_user=trans.get_user(), source=source_hdca) if not copy_elements: @@ -409,17 +417,18 @@ class DatasetCollectionManager: new_elements[key] = collection elements.update(new_elements) - def __load_elements(self, trans, element_identifiers, hide_source_items=False, copy_elements=False, history=None): + def __load_elements(self, trans, element_identifiers, hide_source_items=False, copy_elements=False, copy_dataset_instance_attributes=None, history=None): elements = {} for element_identifier in element_identifiers: elements[element_identifier["name"]] = self.__load_element(trans, element_identifier=element_identifier, hide_source_items=hide_source_items, copy_elements=copy_elements, + copy_dataset_instance_attributes=copy_dataset_instance_attributes, history=history) return elements - def __load_element(self, trans, element_identifier, hide_source_items, copy_elements, history=None): + def __load_element(self, trans, element_identifier, hide_source_items, copy_elements, copy_dataset_instance_attributes=None, history=None): # if not isinstance( element_identifier, dict ): # # Is allowing this to just be the id of an hda too clever? Somewhat # # consistent with other API methods though. @@ -455,6 +464,10 @@ class DatasetCollectionManager: hda = self.hda_manager.get_accessible(decoded_id, trans.user) if copy_elements: element = self.hda_manager.copy(hda, history=history or trans.history, hide_copy=True) + if copy_dataset_instance_attributes is not None: + raise RequestParameterInvalidException("FOR TESTING, make sure this called - we want to be here...") + if 'dbkey' in copy_dataset_instance_attributes: + element.dbkey = copy_dataset_instance_attributes['dbkey'] else: element = hda if hide_source_items and self.hda_manager.get_owned(hda.id, user=trans.user, current_history=history or trans.history): diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 9cfba86966f..93a6d308b9a 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -4286,7 +4286,7 @@ class DatasetCollection(Dictifiable, UsesAnnotations, RepresentById): error_message = f"Dataset collection has no {get_by_attribute} with key {key}." raise KeyError(error_message) - def copy(self, destination=None, element_destination=None, flush=True): + def copy(self, destination=None, element_destination=None, copy_dataset_instance_attributes=None, flush=True): new_collection = DatasetCollection( collection_type=self.collection_type, element_count=self.element_count @@ -4296,6 +4296,7 @@ class DatasetCollection(Dictifiable, UsesAnnotations, RepresentById): new_collection, destination=destination, element_destination=element_destination, + copy_dataset_instance_attributes=copy_dataset_instance_attributes, flush=flush ) object_session(self).add(new_collection) @@ -4537,7 +4538,7 @@ class HistoryDatasetCollectionAssociation(DatasetCollectionInstance, break return matching_collection - def copy(self, element_destination=None): + def copy(self, element_destination=None, copy_dataset_instance_attributes=None): """ Create a copy of this history dataset collection association. Copy underlying collection. @@ -4558,6 +4559,7 @@ class HistoryDatasetCollectionAssociation(DatasetCollectionInstance, collection_copy = self.collection.copy( destination=hdca, element_destination=element_destination, + copy_dataset_instance_attributes=copy_dataset_instance_attributes, flush=False, ) hdca.collection = collection_copy @@ -4719,17 +4721,27 @@ class DatasetCollectionElement(Dictifiable, RepresentById): else: return [element_object] - def copy_to_collection(self, collection, destination=None, element_destination=None, flush=True): + def copy_to_collection(self, collection, destination=None, element_destination=None, copy_dataset_instance_attributes=None, flush=True): element_object = self.element_object if element_destination: if self.is_collection: element_object = element_object.copy( destination=destination, element_destination=element_destination, + copy_dataset_instance_attributes=copy_dataset_instance_attributes, flush=flush ) else: new_element_object = element_object.copy(flush=flush, copy_tags=element_object.tags) + if copy_dataset_instance_attributes is not None: + if 'dbkey' in copy_dataset_instance_attributes: + new_element_object.dbkey = copy_dataset_instance_attributes['dbkey'] + if 'ext' in copy_dataset_instance_attributes: + target_ext = copy_dataset_instance_attributes['ext'] + if new_element_object.extension != target_ext: + new_element_object.extension = target_ext + # TODO: recalculate metadata... + new_element_object.visible = False if destination is not None and element_object.hidden_beneath_collection_instance: new_element_object.hidden_beneath_collection_instance = destination diff --git a/lib/galaxy/tools/force_to_datatype.xml b/lib/galaxy/tools/force_to_datatype.xml new file mode 100644 index 00000000000..1fb20d0ee1e --- /dev/null +++ b/lib/galaxy/tools/force_to_datatype.xml @@ -0,0 +1,25 @@ + + + + + + + + + + + + + + + + + + + diff --git a/lib/galaxy/webapps/galaxy/api/history_contents.py b/lib/galaxy/webapps/galaxy/api/history_contents.py index 2f93ae00360..cb144ed425d 100644 --- a/lib/galaxy/webapps/galaxy/api/history_contents.py +++ b/lib/galaxy/webapps/galaxy/api/history_contents.py @@ -530,13 +530,23 @@ class HistoryContentsController(BaseGalaxyAPIController, UsesLibraryMixin, UsesL content = payload.get('content', None) if content is None: raise exceptions.RequestParameterMissingException("'content' id of target to copy is missing") - copy_elements = payload.get('copy_elements', False) + dbkey = payload.get("dbkey", None) + force_copy_elements = dbkey is not None + copy_elements = payload.get('copy_elements', force_copy_elements) + if not copy_elements and force_copy_elements: + raise exceptions.RequestParameterMissingException("copy_elements passed as 'false' but it is required to change specified attributes") + copy_dataset_instance_attributes = None + if force_copy_elements: + copy_dataset_instance_attributes = {} + if dbkey is not None: + copy_dataset_instance_attributes["dbkey"] = dbkey dataset_collection_instance = service.copy( trans=trans, parent=history, source="hdca", encoded_source_id=content, copy_elements=copy_elements, + copy_dataset_instance_attributes=copy_dataset_instance_attributes, ) else: message = "Invalid 'source' parameter in request %s" % source diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index da1908c5692..1787d160ebd 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -440,6 +440,49 @@ class HistoryContentsApiTestCase(ApiTestCase): self._assert_has_keys(new_forward, "history_id") assert new_forward["history_id"] == self.history_id + def test_hdca_copy_with_new_dbkey(self): + # sh run_tests.sh -api lib/galaxy_test/api/test_history_contents.py::HistoryContentsApiTestCase::test_hdca_copy_with_new_dbkey + hdca = self.dataset_collection_populator.create_pair_in_history(self.history_id).json() + hdca_id = hdca["id"] + print(hdca) + assert hdca["elements"][0]["object"]["metadata_dbkey"] == "?" + assert hdca["elements"][0]["object"]["genome_build"] == "?" + second_history_id = self.dataset_populator.new_history() + create_data = dict( + source='hdca', + content=hdca_id, + dbkey='hg19', + ) + assert len(self._get("histories/%s/contents/dataset_collections" % second_history_id).json()) == 0 + create_response = self._post("histories/%s/contents/dataset_collections" % second_history_id, create_data) + self.__check_create_collection_response(create_response) + contents = self._get("histories/%s/contents/dataset_collections" % second_history_id).json() + assert len(contents) == 1 + new_forward, _ = self.__get_paired_response_elements(contents[0]) + print(new_forward) + assert new_forward["metadata_dbkey"] == "hg19" + assert new_forward["genome_build"] == "hg19" + + def test_hdca_copy_with_new_extension(self): + # sh run_tests.sh -api lib/galaxy_test/api/test_history_contents.py::HistoryContentsApiTestCase::test_hdca_copy_with_new_dbkey + hdca = self.dataset_collection_populator.create_pair_in_history(self.history_id).json() + hdca_id = hdca["id"] + assert hdca["elements"][0]["object"]["file_ext"] == "txt" + second_history_id = self.dataset_populator.new_history() + create_data = dict( + source='hdca', + content=hdca_id, + ext='data', + ) + assert len(self._get("histories/%s/contents/dataset_collections" % second_history_id).json()) == 0 + create_response = self._post("histories/%s/contents/dataset_collections" % second_history_id, create_data) + self.__check_create_collection_response(create_response) + contents = self._get("histories/%s/contents/dataset_collections" % second_history_id).json() + assert len(contents) == 1 + new_forward, _ = self.__get_paired_response_elements(contents[0]) + print(new_forward) + assert new_forward["file_ext"] == "data" + def test_hdca_copy_and_elements(self): hdca = self.dataset_collection_populator.create_pair_in_history(self.history_id).json() hdca_id = hdca["id"] From 4ec5e7621e4ea4a6e6f84a9d27591afbd4cc1443 Mon Sep 17 00:00:00 2001 From: Assunta DeSanto Date: Tue, 23 Mar 2021 21:41:10 -0400 Subject: [PATCH 2/8] removed other path and anything referring to extension attribute --- lib/galaxy/managers/collections.py | 19 ++++----------- lib/galaxy/model/__init__.py | 5 ---- lib/galaxy/tools/force_to_datatype.xml | 25 -------------------- lib/galaxy_test/api/test_history_contents.py | 20 ---------------- 4 files changed, 4 insertions(+), 65 deletions(-) delete mode 100644 lib/galaxy/tools/force_to_datatype.xml diff --git a/lib/galaxy/managers/collections.py b/lib/galaxy/managers/collections.py index fa09788009e..c4999a7c6b3 100644 --- a/lib/galaxy/managers/collections.py +++ b/lib/galaxy/managers/collections.py @@ -105,8 +105,7 @@ class DatasetCollectionManager: def create(self, trans, parent, name, collection_type, element_identifiers=None, elements=None, implicit_collection_info=None, trusted_identifiers=None, hide_source_items=False, tags=None, copy_elements=False, history=None, - set_hid=True, flush=True, completed_job=None, output_name=None, - copy_dataset_instance_attributes=None): + set_hid=True, flush=True, completed_job=None, output_name=None): """ PRECONDITION: security checks on ability to add to parent occurred during load. @@ -129,7 +128,6 @@ class DatasetCollectionManager: elements=elements, hide_source_items=hide_source_items, copy_elements=copy_elements, - copy_dataset_instance_attributes=copy_dataset_instance_attributes, history=history, ) @@ -186,7 +184,7 @@ class DatasetCollectionManager: return self.__persist(dataset_collection_instance, flush=flush) def create_dataset_collection(self, trans, collection_type, element_identifiers=None, elements=None, - hide_source_items=None, copy_elements=False, history=None, copy_dataset_instance_attributes=None): + hide_source_items=None, copy_elements=False, history=None): # Make sure at least one of these is None. assert element_identifiers is None or elements is None @@ -205,7 +203,6 @@ class DatasetCollectionManager: element_identifiers=element_identifiers, hide_source_items=hide_source_items, copy_elements=copy_elements, - copy_dataset_instance_attributes=copy_dataset_instance_attributes, history=history) else: if has_subcollections: @@ -227,7 +224,6 @@ class DatasetCollectionManager: element_identifiers, hide_source_items=False, copy_elements=False, - copy_dataset_instance_attributes=None, history=None): if collection_type_description.has_subcollections(): # Nested collection - recursively create collections and update identifiers. @@ -240,14 +236,12 @@ class DatasetCollectionManager: element_identifiers=element_identifier['element_identifiers'], hide_source_items=hide_source_items, copy_elements=copy_elements, - copy_dataset_instance_attributes=copy_dataset_instance_attributes, history=history) if not new_collection: elements = self.__load_elements(trans=trans, element_identifiers=element_identifiers, hide_source_items=hide_source_items, copy_elements=copy_elements, - copy_dataset_instance_attributes=copy_dataset_instance_attributes, history=history) return elements @@ -417,18 +411,17 @@ class DatasetCollectionManager: new_elements[key] = collection elements.update(new_elements) - def __load_elements(self, trans, element_identifiers, hide_source_items=False, copy_elements=False, copy_dataset_instance_attributes=None, history=None): + def __load_elements(self, trans, element_identifiers, hide_source_items=False, copy_elements=False, history=None): elements = {} for element_identifier in element_identifiers: elements[element_identifier["name"]] = self.__load_element(trans, element_identifier=element_identifier, hide_source_items=hide_source_items, copy_elements=copy_elements, - copy_dataset_instance_attributes=copy_dataset_instance_attributes, history=history) return elements - def __load_element(self, trans, element_identifier, hide_source_items, copy_elements, copy_dataset_instance_attributes=None, history=None): + def __load_element(self, trans, element_identifier, hide_source_items, copy_elements, history=None): # if not isinstance( element_identifier, dict ): # # Is allowing this to just be the id of an hda too clever? Somewhat # # consistent with other API methods though. @@ -464,10 +457,6 @@ class DatasetCollectionManager: hda = self.hda_manager.get_accessible(decoded_id, trans.user) if copy_elements: element = self.hda_manager.copy(hda, history=history or trans.history, hide_copy=True) - if copy_dataset_instance_attributes is not None: - raise RequestParameterInvalidException("FOR TESTING, make sure this called - we want to be here...") - if 'dbkey' in copy_dataset_instance_attributes: - element.dbkey = copy_dataset_instance_attributes['dbkey'] else: element = hda if hide_source_items and self.hda_manager.get_owned(hda.id, user=trans.user, current_history=history or trans.history): diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 93a6d308b9a..f17332547ba 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -4736,11 +4736,6 @@ class DatasetCollectionElement(Dictifiable, RepresentById): if copy_dataset_instance_attributes is not None: if 'dbkey' in copy_dataset_instance_attributes: new_element_object.dbkey = copy_dataset_instance_attributes['dbkey'] - if 'ext' in copy_dataset_instance_attributes: - target_ext = copy_dataset_instance_attributes['ext'] - if new_element_object.extension != target_ext: - new_element_object.extension = target_ext - # TODO: recalculate metadata... new_element_object.visible = False if destination is not None and element_object.hidden_beneath_collection_instance: diff --git a/lib/galaxy/tools/force_to_datatype.xml b/lib/galaxy/tools/force_to_datatype.xml deleted file mode 100644 index 1fb20d0ee1e..00000000000 --- a/lib/galaxy/tools/force_to_datatype.xml +++ /dev/null @@ -1,25 +0,0 @@ - - - - - - - - - - - - - - - - - - - diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index 1787d160ebd..a59e7df8d18 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -463,26 +463,6 @@ class HistoryContentsApiTestCase(ApiTestCase): assert new_forward["metadata_dbkey"] == "hg19" assert new_forward["genome_build"] == "hg19" - def test_hdca_copy_with_new_extension(self): - # sh run_tests.sh -api lib/galaxy_test/api/test_history_contents.py::HistoryContentsApiTestCase::test_hdca_copy_with_new_dbkey - hdca = self.dataset_collection_populator.create_pair_in_history(self.history_id).json() - hdca_id = hdca["id"] - assert hdca["elements"][0]["object"]["file_ext"] == "txt" - second_history_id = self.dataset_populator.new_history() - create_data = dict( - source='hdca', - content=hdca_id, - ext='data', - ) - assert len(self._get("histories/%s/contents/dataset_collections" % second_history_id).json()) == 0 - create_response = self._post("histories/%s/contents/dataset_collections" % second_history_id, create_data) - self.__check_create_collection_response(create_response) - contents = self._get("histories/%s/contents/dataset_collections" % second_history_id).json() - assert len(contents) == 1 - new_forward, _ = self.__get_paired_response_elements(contents[0]) - print(new_forward) - assert new_forward["file_ext"] == "data" - def test_hdca_copy_and_elements(self): hdca = self.dataset_collection_populator.create_pair_in_history(self.history_id).json() hdca_id = hdca["id"] From ce6f553ffe3fb1b26f9ee22624d5d9d0897eed0e Mon Sep 17 00:00:00 2001 From: Assunta DeSanto Date: Thu, 25 Mar 2021 11:09:18 -0400 Subject: [PATCH 3/8] removing a comment --- lib/galaxy_test/api/test_history_contents.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index a59e7df8d18..bdfc4a2215c 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -441,7 +441,6 @@ class HistoryContentsApiTestCase(ApiTestCase): assert new_forward["history_id"] == self.history_id def test_hdca_copy_with_new_dbkey(self): - # sh run_tests.sh -api lib/galaxy_test/api/test_history_contents.py::HistoryContentsApiTestCase::test_hdca_copy_with_new_dbkey hdca = self.dataset_collection_populator.create_pair_in_history(self.history_id).json() hdca_id = hdca["id"] print(hdca) From 5d0cc34c775bc9c02f066467093fce06af0a4435 Mon Sep 17 00:00:00 2001 From: Assunta DeSanto Date: Thu, 25 Mar 2021 12:18:35 -0400 Subject: [PATCH 4/8] attempted fix for linting issue --- lib/galaxy/model/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 38397f94e98..3d39be2f115 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -4736,7 +4736,7 @@ class DatasetCollectionElement(Dictifiable, RepresentById): if copy_dataset_instance_attributes is not None: if 'dbkey' in copy_dataset_instance_attributes: new_element_object.dbkey = copy_dataset_instance_attributes['dbkey'] - + new_element_object.visible = False if destination is not None and element_object.hidden_beneath_collection_instance: new_element_object.hidden_beneath_collection_instance = destination From df71d9ac90e8ac82f5ecf82d7c67fd11e8b82141 Mon Sep 17 00:00:00 2001 From: Assunta DeSanto Date: Fri, 26 Mar 2021 13:31:57 -0400 Subject: [PATCH 5/8] suggested stylistic and simplification changes --- lib/galaxy/managers/collections.py | 6 +++--- lib/galaxy/model/__init__.py | 17 ++++++++--------- .../webapps/galaxy/api/history_contents.py | 18 ++++++++---------- lib/galaxy_test/api/test_history_contents.py | 18 ++++-------------- 4 files changed, 23 insertions(+), 36 deletions(-) diff --git a/lib/galaxy/managers/collections.py b/lib/galaxy/managers/collections.py index 0fd62368621..e3e62d33fef 100644 --- a/lib/galaxy/managers/collections.py +++ b/lib/galaxy/managers/collections.py @@ -287,7 +287,7 @@ class DatasetCollectionManager: changed = self._set_from_dict(trans, dataset_collection_instance, payload) return changed - def copy(self, trans, parent, source, encoded_source_id, copy_elements=False, copy_dataset_instance_attributes=None): + def copy(self, trans, parent, source, encoded_source_id, copy_elements=False, dataset_instance_attributes=None): """ PRECONDITION: security checks on ability to add to parent occurred during load. @@ -297,8 +297,8 @@ class DatasetCollectionManager: copy_kwds = {} if copy_elements: copy_kwds["element_destination"] = parent # e.g. a history - if copy_dataset_instance_attributes is not None: - copy_kwds["copy_dataset_instance_attributes"] = copy_dataset_instance_attributes + if dataset_instance_attributes is not None: + copy_kwds["dataset_instance_attributes"] = dataset_instance_attributes new_hdca = source_hdca.copy(**copy_kwds) new_hdca.copy_tags_from(target_user=trans.get_user(), source=source_hdca) if not copy_elements: diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 8d5662620b3..36649189715 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -4286,7 +4286,7 @@ class DatasetCollection(Dictifiable, UsesAnnotations, RepresentById): error_message = f"Dataset collection has no {get_by_attribute} with key {key}." raise KeyError(error_message) - def copy(self, destination=None, element_destination=None, copy_dataset_instance_attributes=None, flush=True): + def copy(self, destination=None, element_destination=None, dataset_instance_attributes=None, flush=True): new_collection = DatasetCollection( collection_type=self.collection_type, element_count=self.element_count @@ -4296,7 +4296,7 @@ class DatasetCollection(Dictifiable, UsesAnnotations, RepresentById): new_collection, destination=destination, element_destination=element_destination, - copy_dataset_instance_attributes=copy_dataset_instance_attributes, + dataset_instance_attributes=dataset_instance_attributes, flush=flush ) object_session(self).add(new_collection) @@ -4538,7 +4538,7 @@ class HistoryDatasetCollectionAssociation(DatasetCollectionInstance, break return matching_collection - def copy(self, element_destination=None, copy_dataset_instance_attributes=None): + def copy(self, element_destination=None, dataset_instance_attributes=None): """ Create a copy of this history dataset collection association. Copy underlying collection. @@ -4559,7 +4559,7 @@ class HistoryDatasetCollectionAssociation(DatasetCollectionInstance, collection_copy = self.collection.copy( destination=hdca, element_destination=element_destination, - copy_dataset_instance_attributes=copy_dataset_instance_attributes, + dataset_instance_attributes=dataset_instance_attributes, flush=False, ) hdca.collection = collection_copy @@ -4721,21 +4721,20 @@ class DatasetCollectionElement(Dictifiable, RepresentById): else: return [element_object] - def copy_to_collection(self, collection, destination=None, element_destination=None, copy_dataset_instance_attributes=None, flush=True): + def copy_to_collection(self, collection, destination=None, element_destination=None, dataset_instance_attributes=None, flush=True): element_object = self.element_object if element_destination: if self.is_collection: element_object = element_object.copy( destination=destination, element_destination=element_destination, - copy_dataset_instance_attributes=copy_dataset_instance_attributes, + dataset_instance_attributes=dataset_instance_attributes, flush=flush ) else: new_element_object = element_object.copy(flush=flush, copy_tags=element_object.tags) - if copy_dataset_instance_attributes is not None: - if 'dbkey' in copy_dataset_instance_attributes: - new_element_object.dbkey = copy_dataset_instance_attributes['dbkey'] + for attribute, value in dataset_instance_attributes.items(): + setattr(new_element_object, attribute, value) new_element_object.visible = False if destination is not None and element_object.hidden_beneath_collection_instance: diff --git a/lib/galaxy/webapps/galaxy/api/history_contents.py b/lib/galaxy/webapps/galaxy/api/history_contents.py index cb144ed425d..5bde1e5ea4e 100644 --- a/lib/galaxy/webapps/galaxy/api/history_contents.py +++ b/lib/galaxy/webapps/galaxy/api/history_contents.py @@ -530,23 +530,21 @@ class HistoryContentsController(BaseGalaxyAPIController, UsesLibraryMixin, UsesL content = payload.get('content', None) if content is None: raise exceptions.RequestParameterMissingException("'content' id of target to copy is missing") - dbkey = payload.get("dbkey", None) - force_copy_elements = dbkey is not None - copy_elements = payload.get('copy_elements', force_copy_elements) - if not copy_elements and force_copy_elements: + dbkey = payload.get("dbkey") + copy_required = dbkey is not None + copy_elements = payload.get('copy_elements', copy_required) + if not copy_required and not copy_elements: raise exceptions.RequestParameterMissingException("copy_elements passed as 'false' but it is required to change specified attributes") - copy_dataset_instance_attributes = None - if force_copy_elements: - copy_dataset_instance_attributes = {} - if dbkey is not None: - copy_dataset_instance_attributes["dbkey"] = dbkey + dataset_instance_attributes = {} + if dbkey is not None: + dataset_instance_attributes["dbkey"] = dbkey dataset_collection_instance = service.copy( trans=trans, parent=history, source="hdca", encoded_source_id=content, copy_elements=copy_elements, - copy_dataset_instance_attributes=copy_dataset_instance_attributes, + dataset_instance_attributes=dataset_instance_attributes, ) else: message = "Invalid 'source' parameter in request %s" % source diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index bdfc4a2215c..afa8ee74c03 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -443,22 +443,12 @@ class HistoryContentsApiTestCase(ApiTestCase): def test_hdca_copy_with_new_dbkey(self): hdca = self.dataset_collection_populator.create_pair_in_history(self.history_id).json() hdca_id = hdca["id"] - print(hdca) assert hdca["elements"][0]["object"]["metadata_dbkey"] == "?" assert hdca["elements"][0]["object"]["genome_build"] == "?" - second_history_id = self.dataset_populator.new_history() - create_data = dict( - source='hdca', - content=hdca_id, - dbkey='hg19', - ) - assert len(self._get("histories/%s/contents/dataset_collections" % second_history_id).json()) == 0 - create_response = self._post("histories/%s/contents/dataset_collections" % second_history_id, create_data) - self.__check_create_collection_response(create_response) - contents = self._get("histories/%s/contents/dataset_collections" % second_history_id).json() - assert len(contents) == 1 - new_forward, _ = self.__get_paired_response_elements(contents[0]) - print(new_forward) + create_data = {'source': 'hdca', 'content': hdca_id, 'dbkey': 'hg19'} + create_response = self._post(f"histories/{self.history_id}/contents/dataset_collections", create_data) + collection = self.__check_create_collection_response(create_response) + new_forward = collection['elements'][0]['object'] assert new_forward["metadata_dbkey"] == "hg19" assert new_forward["genome_build"] == "hg19" From fd7d077a3b22df2ef47530bcee2451cf7ddb6004 Mon Sep 17 00:00:00 2001 From: Assunta DeSanto Date: Fri, 26 Mar 2021 14:16:23 -0400 Subject: [PATCH 6/8] fixing Unit tests --- lib/galaxy/model/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 36649189715..b5cddd0c7cf 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -4733,8 +4733,9 @@ class DatasetCollectionElement(Dictifiable, RepresentById): ) else: new_element_object = element_object.copy(flush=flush, copy_tags=element_object.tags) - for attribute, value in dataset_instance_attributes.items(): - setattr(new_element_object, attribute, value) + if dataset_instance_attributes is not None: + for attribute, value in dataset_instance_attributes.items(): + setattr(new_element_object, attribute, value) new_element_object.visible = False if destination is not None and element_object.hidden_beneath_collection_instance: From bd7a00fa463e08b1e8fa3cff897441211bb68089 Mon Sep 17 00:00:00 2001 From: Assunta DeSanto Date: Fri, 26 Mar 2021 15:08:49 -0400 Subject: [PATCH 7/8] setting dataset_instance_attributes to the empty set --- lib/galaxy/model/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index b5cddd0c7cf..7d15d697787 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -4722,6 +4722,7 @@ class DatasetCollectionElement(Dictifiable, RepresentById): return [element_object] def copy_to_collection(self, collection, destination=None, element_destination=None, dataset_instance_attributes=None, flush=True): + dataset_instance_attributes = dataset_instance_attributes or {} element_object = self.element_object if element_destination: if self.is_collection: @@ -4733,9 +4734,8 @@ class DatasetCollectionElement(Dictifiable, RepresentById): ) else: new_element_object = element_object.copy(flush=flush, copy_tags=element_object.tags) - if dataset_instance_attributes is not None: - for attribute, value in dataset_instance_attributes.items(): - setattr(new_element_object, attribute, value) + for attribute, value in dataset_instance_attributes.items(): + setattr(new_element_object, attribute, value) new_element_object.visible = False if destination is not None and element_object.hidden_beneath_collection_instance: From c3e04c3eff5ac201b8443fffbbaa489c2a7da931 Mon Sep 17 00:00:00 2001 From: Marius van den Beek Date: Sat, 27 Mar 2021 10:48:51 +0100 Subject: [PATCH 8/8] Only fail if copy required and the collection is not getting copied --- lib/galaxy/webapps/galaxy/api/history_contents.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/api/history_contents.py b/lib/galaxy/webapps/galaxy/api/history_contents.py index 5bde1e5ea4e..dd3845e7f7a 100644 --- a/lib/galaxy/webapps/galaxy/api/history_contents.py +++ b/lib/galaxy/webapps/galaxy/api/history_contents.py @@ -533,7 +533,7 @@ class HistoryContentsController(BaseGalaxyAPIController, UsesLibraryMixin, UsesL dbkey = payload.get("dbkey") copy_required = dbkey is not None copy_elements = payload.get('copy_elements', copy_required) - if not copy_required and not copy_elements: + if copy_required and not copy_elements: raise exceptions.RequestParameterMissingException("copy_elements passed as 'false' but it is required to change specified attributes") dataset_instance_attributes = {} if dbkey is not None: