From dfd016a7a4750cabef8677405a6a307357f0d760 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 14 Mar 2019 09:25:06 -0400 Subject: [PATCH] Refactor TagManager (back) to TagHandler. The original TagHandler used trans and app and was what I think we convinced of as a "Manager" - something that provides business logic and ties app and trans to the model (and other lower layers of Galaxy). Since 7c86c5480bc03db4897e64da1abd4c41ea6056c3 the tag manager has not needed app or trans to do any of its work. It is a clean model manipulation utility. It also doesn't use or check security, it isn't a "Manager" in our sense. It is a model utility and so it belongs in galaxy.model I believe. This also "fixes" the problem of galaxy.managers dependending on galaxy.model and then galaxy.model depending back on galaxy.managers. Hopefully it is clear the dependency chain should be one direction and that is galaxy.managers depends on galaxy.model and not vice versa. In addition to allowing us to properly create a package out the models, this also fixes something needling me in the history import/export and model store branch - the tag manager is/was the only manager I needed to import/export histories and such - that was good evidence I think that it wasn't a manager and should have been in models all along. --- lib/galaxy/app.py | 4 +-- lib/galaxy/managers/collections.py | 16 ++++----- lib/galaxy/managers/library_datasets.py | 6 ++-- lib/galaxy/managers/taggable.py | 4 +-- lib/galaxy/model/__init__.py | 6 ++-- lib/galaxy/{managers => model}/tags.py | 10 +++--- lib/galaxy/tools/__init__.py | 4 +-- lib/galaxy/tools/actions/upload_common.py | 4 +-- lib/galaxy/web/base/controller.py | 8 ++--- lib/galaxy/webapps/tool_shed/app.py | 4 +-- ...{test_TagManager.py => test_TagHandler.py} | 36 +++++++++---------- test/unit/unittest_utils/galaxy_mock.py | 3 +- 12 files changed, 52 insertions(+), 53 deletions(-) rename lib/galaxy/{managers => model}/tags.py (98%) rename test/unit/managers/{test_TagManager.py => test_TagHandler.py} (75%) diff --git a/lib/galaxy/app.py b/lib/galaxy/app.py index 555873824b6..1e48845314d 100644 --- a/lib/galaxy/app.py +++ b/lib/galaxy/app.py @@ -16,7 +16,7 @@ from galaxy.managers.collections import DatasetCollectionManager from galaxy.managers.folders import FolderManager from galaxy.managers.histories import HistoryManager from galaxy.managers.libraries import LibraryManager -from galaxy.managers.tags import GalaxyTagManager +from galaxy.model.tags import GalaxyTagHandler from galaxy.queue_worker import GalaxyQueueWorker from galaxy.tools.cache import ( ToolCache, @@ -94,7 +94,7 @@ class UniverseApplication(config.ConfiguresGalaxyMixin): # Security helper self._configure_security() # Tag handler - self.tag_handler = GalaxyTagManager(self.model.context) + self.tag_handler = GalaxyTagHandler(self.model.context) self.dataset_collections_service = DatasetCollectionManager(self) self.history_manager = HistoryManager(self) self.dependency_resolvers_view = DependencyResolversView(self) diff --git a/lib/galaxy/managers/collections.py b/lib/galaxy/managers/collections.py index 4aa717461f6..01eb50aec4e 100644 --- a/lib/galaxy/managers/collections.py +++ b/lib/galaxy/managers/collections.py @@ -14,9 +14,9 @@ from galaxy.managers import ( hdas, histories, lddas, - tags ) from galaxy.managers.collections_util import validate_input_element_identifiers +from galaxy.model import tags from galaxy.util import ( odict, validation @@ -43,7 +43,7 @@ class DatasetCollectionManager(object): self.hda_manager = hdas.HDAManager(app) self.history_manager = histories.HistoryManager(app) - self.tag_manager = tags.GalaxyTagManager(app.model.context) + self.tag_handler = tags.GalaxyTagHandler(app.model.context) self.ldda_manager = lddas.LDDAManager(app) def precreate_dataset_collection_instance(self, trans, parent, name, structure, implicit_inputs=None, implicit_output_name=None, tags=None): @@ -154,7 +154,7 @@ class DatasetCollectionManager(object): # values. if isinstance(tags, list): assert implicit_inputs is None, implicit_inputs - tags = self.tag_manager.add_tags_from_list(trans.user, dataset_collection_instance, tags) + tags = self.tag_handler.add_tags_from_list(trans.user, dataset_collection_instance, tags) else: tags = self._append_tags(dataset_collection_instance, implicit_inputs, tags) return self.__persist(dataset_collection_instance, flush=flush) @@ -284,8 +284,8 @@ class DatasetCollectionManager(object): if copy_elements: copy_kwds["element_destination"] = parent new_hdca = source_hdca.copy(**copy_kwds) - tags_str = self.tag_manager.get_tags_str(source_hdca.tags) - self.tag_manager.apply_item_tags(trans.get_user(), new_hdca, tags_str) + tags_str = self.tag_handler.get_tags_str(source_hdca.tags) + self.tag_handler.apply_item_tags(trans.get_user(), new_hdca, tags_str) parent.add_dataset_collection(new_hdca) trans.sa_session.add(new_hdca) trans.sa_session.flush() @@ -300,7 +300,7 @@ class DatasetCollectionManager(object): changed['annotation'] = new_data['annotation'] if 'tags' in new_data.keys() and trans.get_user(): # set_tags_from_list will flush on its own, no need to add to 'changed' here and incur a second flush. - self.tag_manager.set_tags_from_list(trans.get_user(), dataset_collection_instance, new_data['tags']) + self.tag_handler.set_tags_from_list(trans.get_user(), dataset_collection_instance, new_data['tags']) if changed.keys(): trans.sa_session.flush() @@ -431,11 +431,11 @@ class DatasetCollectionManager(object): element = hda if hide_source_items and self.hda_manager.get_owned(hda.id, user=trans.user, current_history=trans.history): hda.visible = False - self.tag_manager.apply_item_tags(user=trans.user, item=element, tags_str=tag_str) + self.tag_handler.apply_item_tags(user=trans.user, item=element, tags_str=tag_str) elif src_type == 'ldda': element = self.ldda_manager.get(trans, encoded_id, check_accessible=True) element = element.to_history_dataset_association(trans.history, add_to_history=True) - self.tag_manager.apply_item_tags(user=trans.user, item=element, tags_str=tag_str) + self.tag_handler.apply_item_tags(user=trans.user, item=element, tags_str=tag_str) elif src_type == 'hdca': # TODO: Option to copy? Force copy? Copy or allow if not owned? element = self.__get_history_collection_instance(trans, encoded_id).collection diff --git a/lib/galaxy/managers/library_datasets.py b/lib/galaxy/managers/library_datasets.py index 4b740c16f93..6d5eacf2567 100644 --- a/lib/galaxy/managers/library_datasets.py +++ b/lib/galaxy/managers/library_datasets.py @@ -10,8 +10,8 @@ from galaxy.exceptions import ( ) from galaxy.managers import ( datasets, - tags ) +from galaxy.model import tags from galaxy.util import validation log = logging.getLogger(__name__) @@ -22,7 +22,7 @@ class LibraryDatasetsManager(datasets.DatasetAssociationManager): def __init__(self, app): self.app = app - self.tag_manager = tags.GalaxyTagManager(app.model.context) + self.tag_handler = tags.GalaxyTagHandler(app.model.context) def get(self, trans, decoded_library_dataset_id, check_accessible=True): """ @@ -219,7 +219,7 @@ class LibraryDatasetsManager(datasets.DatasetAssociationManager): rval['date_uploaded'] = ldda.create_time.strftime("%Y-%m-%d %I:%M %p") rval['can_user_modify'] = trans.user_is_admin or trans.app.security_agent.can_modify_library_item(current_user_roles, ld) rval['is_unrestricted'] = trans.app.security_agent.dataset_is_public(ldda.dataset) - rval['tags'] = self.tag_manager.get_tags_str(ldda.tags) + rval['tags'] = self.tag_handler.get_tags_str(ldda.tags) # Manage dataset permission is always attached to the dataset itself, not the the ld or ldda to maintain consistency rval['can_user_manage'] = trans.user_is_admin or trans.app.security_agent.can_manage_dataset(current_user_roles, ldda.dataset) diff --git a/lib/galaxy/managers/taggable.py b/lib/galaxy/managers/taggable.py index dc4508abefa..794ab41008d 100644 --- a/lib/galaxy/managers/taggable.py +++ b/lib/galaxy/managers/taggable.py @@ -45,7 +45,7 @@ class TaggableManagerMixin(object): #: class of TagAssociation (e.g. HistoryTagAssociation) tag_assoc = None - # TODO: most of this can be done by delegating to the TagManager? + # TODO: most of this can be done by delegating to the GalaxyTagHandler? def get_tags(self, item): """ Return a list of tag strings. @@ -59,7 +59,7 @@ class TaggableManagerMixin(object): return _tags_from_strings(item, self.app.tag_handler, new_tags, user=user) # def tags_by_user( self, user, **kwargs ): - # TODO: here or TagManager + # TODO: here or GalaxyTagHandler # pass diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index db541be9788..ca328599033 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -43,9 +43,9 @@ from sqlalchemy.schema import UniqueConstraint import galaxy.model.metadata import galaxy.model.orm.now +import galaxy.model.tags import galaxy.security.passwords import galaxy.util -from galaxy.managers import tags from galaxy.model.item_attrs import UsesAnnotations from galaxy.model.util import pgcalc from galaxy.security import get_permitted_actions @@ -3145,7 +3145,7 @@ class LibraryDatasetDatasetAssociation(DatasetInstance, HasName, RepresentById): copied_from_library_dataset_dataset_association=self, history=target_history) - tag_manager = tags.GalaxyTagManager(sa_session) + tag_manager = galaxy.model.tags.GalaxyTagHandler(sa_session) src_ldda_tags = tag_manager.get_tags_str(self.tags) tag_manager.apply_item_tags(user=self.user, item=hda, tags_str=src_ldda_tags) @@ -3175,7 +3175,7 @@ class LibraryDatasetDatasetAssociation(DatasetInstance, HasName, RepresentById): copied_from_library_dataset_dataset_association=self, folder=target_folder) - tag_manager = tags.GalaxyTagManager(sa_session) + tag_manager = galaxy.model.tags.GalaxyTagHandler(sa_session) src_ldda_tags = tag_manager.get_tags_str(self.tags) tag_manager.apply_item_tags(user=self.user, item=ldda, tags_str=src_ldda_tags) diff --git a/lib/galaxy/managers/tags.py b/lib/galaxy/model/tags.py similarity index 98% rename from lib/galaxy/managers/tags.py rename to lib/galaxy/model/tags.py index 36b58bfe16e..952b21652c2 100644 --- a/lib/galaxy/managers/tags.py +++ b/lib/galaxy/model/tags.py @@ -22,7 +22,7 @@ class ItemTagAssocInfo(object): self.item_id_col = item_id_col -class TagManager(object): +class TagHandler(object): """ Manages CRUD operations related to tagging objects. """ @@ -319,10 +319,10 @@ class TagManager(object): return name_value_pair -class GalaxyTagManager(TagManager): +class GalaxyTagHandler(TagHandler): def __init__(self, sa_session): from galaxy import model - TagManager.__init__(self, sa_session) + TagHandler.__init__(self, sa_session) self.item_tag_assoc_info["History"] = ItemTagAssocInfo(model.History, model.HistoryTagAssociation, model.HistoryTagAssociation.table.c.history_id) @@ -349,6 +349,6 @@ class GalaxyTagManager(TagManager): model.VisualizationTagAssociation.table.c.visualization_id) -class CommunityTagManager(TagManager): +class CommunityTagHandler(TagHandler): def __init__(self, sa_session): - TagManager.__init__(self, sa_session) + TagHandler.__init__(self, sa_session) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 30cc55ed3f4..39802d3deff 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -26,8 +26,8 @@ from galaxy import ( model ) from galaxy.managers.jobs import JobSearch -from galaxy.managers.tags import GalaxyTagManager from galaxy.metadata import get_metadata_compute_strategy +from galaxy.model.tags import GalaxyTagHandler from galaxy.queue_worker import send_control_task from galaxy.tools.actions import DefaultToolAction from galaxy.tools.actions.data_manager import DataManagerToolAction @@ -2824,7 +2824,7 @@ class TagFromFileTool(DatabaseOperationTool): how = incoming['how'] new_tags_dataset_assoc = incoming["tags"] new_elements = odict() - tags_manager = GalaxyTagManager(trans.app.model.context) + tags_manager = GalaxyTagHandler(trans.app.model.context) new_datasets = [] def add_copied_value_to_new_elements(new_tags_dict, dce): diff --git a/lib/galaxy/tools/actions/upload_common.py b/lib/galaxy/tools/actions/upload_common.py index 839c5a42f73..1604d9ac4a4 100644 --- a/lib/galaxy/tools/actions/upload_common.py +++ b/lib/galaxy/tools/actions/upload_common.py @@ -14,7 +14,7 @@ from webob.compat import cgi_FieldStorage from galaxy import datatypes, util from galaxy.exceptions import ConfigDoesNotAllowException, ObjectInvalid -from galaxy.managers import tags +from galaxy.model import tags from galaxy.util import unicodify from galaxy.util.odict import odict @@ -220,7 +220,7 @@ def __new_library_upload(trans, cntrller, uploaded_dataset, library_bunch, state sa_session=trans.sa_session) if uploaded_dataset.get('tag_using_filenames', False): tag_from_filename = os.path.splitext(os.path.basename(uploaded_dataset.name))[0] - tag_manager = tags.GalaxyTagManager(trans.sa_session) + tag_manager = tags.GalaxyTagHandler(trans.sa_session) tag_manager.apply_item_tag(item=ldda, user=trans.user, name='name', value=tag_from_filename) trans.sa_session.add(ldda) diff --git a/lib/galaxy/web/base/controller.py b/lib/galaxy/web/base/controller.py index 6640ca154cc..5dcddd403f2 100644 --- a/lib/galaxy/web/base/controller.py +++ b/lib/galaxy/web/base/controller.py @@ -24,7 +24,6 @@ from galaxy.managers import ( api_keys, base as managers_base, configuration, - tags, users, workflows ) @@ -32,7 +31,8 @@ from galaxy.model import ( ExtendedMetadata, ExtendedMetadataIndex, HistoryDatasetAssociation, - LibraryDatasetDatasetAssociation + LibraryDatasetDatasetAssociation, + tags, ) from galaxy.model.item_attrs import UsesAnnotations from galaxy.util.dictifiable import Dictifiable @@ -1551,8 +1551,8 @@ class UsesTagsMixin(SharableItemSecurityMixin): return self.get_tag_handler(trans)._get_item_tag_assoc(user, tagged_item, tag_name) def set_tags_from_list(self, trans, item, new_tags_list, user=None): - tags_manager = tags.GalaxyTagManager(trans.app.model.context) - return tags_manager.set_tags_from_list(user, item, new_tags_list) + tag_handler = tags.GalaxyTagHandler(trans.app.model.context) + return tag_handler.set_tags_from_list(user, item, new_tags_list) def get_user_tags_used(self, trans, user=None): """ diff --git a/lib/galaxy/webapps/tool_shed/app.py b/lib/galaxy/webapps/tool_shed/app.py index 587b86965f8..332dc82ea64 100644 --- a/lib/galaxy/webapps/tool_shed/app.py +++ b/lib/galaxy/webapps/tool_shed/app.py @@ -10,7 +10,7 @@ import tool_shed.repository_registry import tool_shed.repository_types.registry from galaxy import tools from galaxy.config import configure_logging -from galaxy.managers.tags import CommunityTagManager +from galaxy.model.tags import CommunityTagHandler from galaxy.util.dbkeys import GenomeBuilds from galaxy.web import security from galaxy.web.stack import application_stack_instance @@ -54,7 +54,7 @@ class UniverseApplication(object): # Initialize the Tool Shed security helper. self.security = security.SecurityHelper(id_secret=self.config.id_secret) # initialize the Tool Shed tag handler. - self.tag_handler = CommunityTagManager(self) + self.tag_handler = CommunityTagHandler(self) # Initialize the Tool Shed tool data tables. Never pass a configuration file here # because the Tool Shed should always have an empty dictionary! self.tool_data_tables = galaxy.tools.data.ToolDataTableManager(self.config.tool_data_path) diff --git a/test/unit/managers/test_TagManager.py b/test/unit/managers/test_TagHandler.py similarity index 75% rename from test/unit/managers/test_TagManager.py rename to test/unit/managers/test_TagHandler.py index a11d7b0dd75..31c158d2a00 100644 --- a/test/unit/managers/test_TagManager.py +++ b/test/unit/managers/test_TagHandler.py @@ -2,7 +2,7 @@ from galaxy.managers import hdas from galaxy.managers.datasets import DatasetManager from galaxy.managers.histories import HistoryManager -from galaxy.managers.tags import GalaxyTagManager +from galaxy.model.tags import GalaxyTagHandler from galaxy.util import unicodify from .base import BaseTestCase @@ -12,14 +12,14 @@ user2_data = dict(email='user2@user2.user2', username='user2', password=default_ # ============================================================================= -class TagManagerTestCase(BaseTestCase): +class TagHandlerTestCase(BaseTestCase): def set_up_managers(self): - super(TagManagerTestCase, self).set_up_managers() + super(TagHandlerTestCase, self).set_up_managers() self.hda_manager = hdas.HDAManager(self.app) self.history_manager = HistoryManager(self.app) self.dataset_manager = DatasetManager(self.app) - self.tag_manager = GalaxyTagManager(self.trans.sa_session) + self.tag_handler = GalaxyTagHandler(self.trans.sa_session) self.user = self.user_manager.create(**user2_data) def _create_vanilla_hda(self, user=None): @@ -59,49 +59,49 @@ class TagManagerTestCase(BaseTestCase): ] for tag_string, expected_tag in zip(tag_strings, expected_tags): hda = self._create_vanilla_hda() - self.tag_manager.apply_item_tags(user=self.user, item=hda, tags_str=tag_string) + self.tag_handler.apply_item_tags(user=self.user, item=hda, tags_str=tag_string) self._check_tag_list(hda.tags, expected_tag) def test_set_tag_from_list(self): hda = self._create_vanilla_hda() tags = ['tag1', 'tag2'] - self.tag_manager.set_tags_from_list(self.user, hda, tags) + self.tag_handler.set_tags_from_list(self.user, hda, tags) self._check_tag_list(hda.tags, tags) # Setting tags should erase previous tags - self.tag_manager.set_tags_from_list(self.user, hda, ['tag1']) + self.tag_handler.set_tags_from_list(self.user, hda, ['tag1']) self._check_tag_list(hda.tags, expected_tags=['tag1']) def test_add_tag_from_list(self): hda = self._create_vanilla_hda() tags = ['tag1', 'tag2'] - self.tag_manager.add_tags_from_list(self.user, hda, tags) + self.tag_handler.add_tags_from_list(self.user, hda, tags) self._check_tag_list(tags=hda.tags, expected_tags=tags) # Adding tags should keep previous tags - self.tag_manager.add_tags_from_list(self.user, hda, ['tag3']) + self.tag_handler.add_tags_from_list(self.user, hda, ['tag3']) self._check_tag_list(hda.tags, expected_tags=['tag1', 'tag2', 'tag3']) def test_remove_tag_from_list(self): hda = self._create_vanilla_hda() tags = ['tag1', 'tag2'] - self.tag_manager.set_tags_from_list(self.user, hda, tags) + self.tag_handler.set_tags_from_list(self.user, hda, tags) self._check_tag_list(hda.tags, tags) - self.tag_manager.remove_tags_from_list(self.user, hda, ['tag1']) + self.tag_handler.remove_tags_from_list(self.user, hda, ['tag1']) self._check_tag_list(hda.tags, ['tag2']) def test_delete_item_tags(self): hda = self._create_vanilla_hda() tags = ['tag1'] - self.tag_manager.set_tags_from_list(self.user, hda, tags) - self.tag_manager.delete_item_tags(user=self.user, item=hda) + self.tag_handler.set_tags_from_list(self.user, hda, tags) + self.tag_handler.delete_item_tags(user=self.user, item=hda) self.assertEqual(hda.tags, []) def test_item_has_tag(self): hda = self._create_vanilla_hda() tags = ['tag1'] - self.tag_manager.set_tags_from_list(self.user, hda, tags) - self.assertTrue(self.tag_manager.item_has_tag(self.user, item=hda, tag='tag1')) + self.tag_handler.set_tags_from_list(self.user, hda, tags) + self.assertTrue(self.tag_handler.item_has_tag(self.user, item=hda, tag='tag1')) # ItemTagAssociation - self.assertTrue(self.tag_manager.item_has_tag(self.user, item=hda, tag=hda.tags[0])) + self.assertTrue(self.tag_handler.item_has_tag(self.user, item=hda, tag=hda.tags[0])) # Tag - self.assertTrue(self.tag_manager.item_has_tag(self.user, item=hda, tag=hda.tags[0].tag)) - self.assertFalse(self.tag_manager.item_has_tag(self.user, item=hda, tag='tag2')) + self.assertTrue(self.tag_handler.item_has_tag(self.user, item=hda, tag=hda.tags[0].tag)) + self.assertFalse(self.tag_handler.item_has_tag(self.user, item=hda, tag='tag2')) diff --git a/test/unit/unittest_utils/galaxy_mock.py b/test/unit/unittest_utils/galaxy_mock.py index 92ff960b8f7..04ee766188f 100644 --- a/test/unit/unittest_utils/galaxy_mock.py +++ b/test/unit/unittest_utils/galaxy_mock.py @@ -13,8 +13,7 @@ from galaxy import ( from galaxy.auth import AuthManager from galaxy.datatypes import registry from galaxy.jobs.manager import NoopManager -from galaxy.managers import tags -from galaxy.model import mapping +from galaxy.model import mapping, tags from galaxy.tools.deps.containers import NullContainerFinder from galaxy.util.bunch import Bunch from galaxy.util.dbkeys import GenomeBuilds