From 582466f77a87615218fe9b58c663e188777aa315 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Thu, 29 Jun 2023 16:27:45 +0200
Subject: [PATCH 01/41] Fix TaggableItemClass

The ItemTagAssociation.associated_item_names were not properly converted due to a difference in class naming convention.
Also there were more classes inheriting from ItemTagAssociation that are not handled by the GalaxyTagHandler.
So I decided to make the TaggableItemClass enum explicit and not dynamically generated.

- The TagHandler.item_tag_assoc_info is now initialized just once instead of every time the TagHandler is instantiated.
---
 client/src/schema/schema.ts  |  7 +---
 lib/galaxy/managers/tags.py  | 14 ++++---
 lib/galaxy/model/__init__.py |  2 -
 lib/galaxy/model/tags.py     | 71 +++++++++++++++++++++---------------
 4 files changed, 52 insertions(+), 42 deletions(-)

diff --git a/client/src/schema/schema.ts b/client/src/schema/schema.ts
index 1f59e67a0faa..5d736f8e7a69 100644
--- a/client/src/schema/schema.ts
+++ b/client/src/schema/schema.ts
@@ -7957,14 +7957,11 @@ export interface components {
         TaggableItemClass:
             | "History"
             | "HistoryDatasetAssociation"
+            | "HistoryDatasetCollectionAssociation"
             | "LibraryDatasetDatasetAssociation"
             | "Page"
-            | "WorkflowStep"
             | "StoredWorkflow"
-            | "Visualization"
-            | "HistoryDatasetCollection"
-            | "LibraryDatasetCollection"
-            | "Tool";
+            | "Visualization";
         /** ToolDataDetails */
         ToolDataDetails: {
             /**
diff --git a/lib/galaxy/managers/tags.py b/lib/galaxy/managers/tags.py
index 5eec0e6b53e7..db5a3a3d657c 100644
--- a/lib/galaxy/managers/tags.py
+++ b/lib/galaxy/managers/tags.py
@@ -4,7 +4,6 @@
 from pydantic import Field
 
 from galaxy.managers.context import ProvidesUserContext
-from galaxy.model import ItemTagAssociation
 from galaxy.model.base import transaction
 from galaxy.model.tags import GalaxyTagHandlerSession
 from galaxy.schema.fields import DecodedDatabaseIdField
@@ -13,10 +12,15 @@
     TagCollection,
 )
 
-taggable_item_names = {item: item for item in ItemTagAssociation.associated_item_names}
-# This Enum is generated dynamically and mypy can not statically infer its real type,
-# so it should be ignored. See: https://github.com/python/mypy/issues/4865#issuecomment-592560696
-TaggableItemClass = Enum("TaggableItemClass", taggable_item_names)  # type: ignore[misc]
+
+class TaggableItemClass(Enum):
+    History = "History"
+    HistoryDatasetAssociation = "HistoryDatasetAssociation"
+    HistoryDatasetCollectionAssociation = "HistoryDatasetCollectionAssociation"
+    LibraryDatasetDatasetAssociation = "LibraryDatasetDatasetAssociation"
+    Page = "Page"
+    StoredWorkflow = "StoredWorkflow"
+    Visualization = "Visualization"
 
 
 class ItemTagsPayload(Model):
diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py
index 99e41676757e..44d031ed14c4 100644
--- a/lib/galaxy/model/__init__.py
+++ b/lib/galaxy/model/__init__.py
@@ -9829,13 +9829,11 @@ def __str__(self):
 class ItemTagAssociation(Dictifiable):
     dict_collection_visible_keys = ["id", "user_tname", "user_value"]
     dict_element_visible_keys = dict_collection_visible_keys
-    associated_item_names: List[str] = []
     user_tname: Column
     user_value = Column(TrimmedString(255), index=True)
 
     def __init_subclass__(cls, **kwargs):
         super().__init_subclass__(**kwargs)
-        cls.associated_item_names.append(cls.__name__.replace("TagAssociation", ""))
 
     def copy(self, cls=None):
         if cls:
diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py
index 6b0c1b9f9923..d82e23e136d2 100644
--- a/lib/galaxy/model/tags.py
+++ b/lib/galaxy/model/tags.py
@@ -381,39 +381,50 @@ def _get_name_value_pair(self, tag_str) -> List[Optional[str]]:
 
 
 class GalaxyTagHandler(TagHandler):
+    _item_tag_assoc_info: Dict[str, ItemTagAssocInfo] = {}
+
     def __init__(self, sa_session: galaxy_scoped_session):
+        TagHandler.__init__(self, sa_session)
+        if not GalaxyTagHandler._item_tag_assoc_info:
+            GalaxyTagHandler.init_tag_associations()
+        self.item_tag_assoc_info = GalaxyTagHandler._item_tag_assoc_info
+
+    @classmethod
+    def init_tag_associations(cls):
         from galaxy import model
 
-        TagHandler.__init__(self, sa_session)
-        self.item_tag_assoc_info["History"] = ItemTagAssocInfo(
-            model.History, model.HistoryTagAssociation, model.HistoryTagAssociation.history_id
-        )
-        self.item_tag_assoc_info["HistoryDatasetAssociation"] = ItemTagAssocInfo(
-            model.HistoryDatasetAssociation,
-            model.HistoryDatasetAssociationTagAssociation,
-            model.HistoryDatasetAssociationTagAssociation.history_dataset_association_id,
-        )
-        self.item_tag_assoc_info["HistoryDatasetCollectionAssociation"] = ItemTagAssocInfo(
-            model.HistoryDatasetCollectionAssociation,
-            model.HistoryDatasetCollectionTagAssociation,
-            model.HistoryDatasetCollectionTagAssociation.history_dataset_collection_id,
-        )
-        self.item_tag_assoc_info["LibraryDatasetDatasetAssociation"] = ItemTagAssocInfo(
-            model.LibraryDatasetDatasetAssociation,
-            model.LibraryDatasetDatasetAssociationTagAssociation,
-            model.LibraryDatasetDatasetAssociationTagAssociation.library_dataset_dataset_association_id,
-        )
-        self.item_tag_assoc_info["Page"] = ItemTagAssocInfo(
-            model.Page, model.PageTagAssociation, model.PageTagAssociation.page_id
-        )
-        self.item_tag_assoc_info["StoredWorkflow"] = ItemTagAssocInfo(
-            model.StoredWorkflow,
-            model.StoredWorkflowTagAssociation,
-            model.StoredWorkflowTagAssociation.stored_workflow_id,
-        )
-        self.item_tag_assoc_info["Visualization"] = ItemTagAssocInfo(
-            model.Visualization, model.VisualizationTagAssociation, model.VisualizationTagAssociation.visualization_id
-        )
+        cls._item_tag_assoc_info = {
+            "History": ItemTagAssocInfo(
+                model.History, model.HistoryTagAssociation, model.HistoryTagAssociation.history_id
+            ),
+            "HistoryDatasetAssociation": ItemTagAssocInfo(
+                model.HistoryDatasetAssociation,
+                model.HistoryDatasetAssociationTagAssociation,
+                model.HistoryDatasetAssociationTagAssociation.history_dataset_association_id,
+            ),
+            "HistoryDatasetCollectionAssociation": ItemTagAssocInfo(
+                model.HistoryDatasetCollectionAssociation,
+                model.HistoryDatasetCollectionTagAssociation,
+                model.HistoryDatasetCollectionTagAssociation.history_dataset_collection_id,
+            ),
+            "LibraryDatasetDatasetAssociation": ItemTagAssocInfo(
+                model.LibraryDatasetDatasetAssociation,
+                model.LibraryDatasetDatasetAssociationTagAssociation,
+                model.LibraryDatasetDatasetAssociationTagAssociation.library_dataset_dataset_association_id,
+            ),
+            "Page": ItemTagAssocInfo(model.Page, model.PageTagAssociation, model.PageTagAssociation.page_id),
+            "StoredWorkflow": ItemTagAssocInfo(
+                model.StoredWorkflow,
+                model.StoredWorkflowTagAssociation,
+                model.StoredWorkflowTagAssociation.stored_workflow_id,
+            ),
+            "Visualization": ItemTagAssocInfo(
+                model.Visualization,
+                model.VisualizationTagAssociation,
+                model.VisualizationTagAssociation.visualization_id,
+            ),
+        }
+        return cls._item_tag_assoc_info
 
 
 class GalaxyTagHandlerSession(GalaxyTagHandler):

From 5057b4054d3b885e7ae4be2b04a08868fbbf5983 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Thu, 29 Jun 2023 16:29:03 +0200
Subject: [PATCH 02/41] Ensure item is owned before handling tags

---
 lib/galaxy/model/tags.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py
index d82e23e136d2..55d5ff8027ec 100644
--- a/lib/galaxy/model/tags.py
+++ b/lib/galaxy/model/tags.py
@@ -13,6 +13,10 @@
 from sqlalchemy.sql.expression import func
 
 import galaxy.model
+from galaxy.exceptions import (
+    AuthenticationRequired,
+    ItemOwnershipException,
+)
 from galaxy.model.base import transaction
 from galaxy.model.scoped_session import galaxy_scoped_session
 from galaxy.util import (
@@ -131,6 +135,7 @@ def get_tool_tags(self):
 
     def remove_item_tag(self, user, item, tag_name):
         """Remove a tag from an item."""
+        self._ensure_user_owns_item(user, item)
         # Get item tag association.
         item_tag_assoc = self._get_item_tag_assoc(user, item, tag_name)
         # Remove association.
@@ -143,6 +148,7 @@ def remove_item_tag(self, user, item, tag_name):
 
     def delete_item_tags(self, user, item):
         """Delete tags from an item."""
+        self._ensure_user_owns_item(user, item)
         # Delete item-tag associations.
         for tag in item.tags:
             if tag.id:
@@ -151,6 +157,18 @@ def delete_item_tags(self, user, item):
         # Delete tags from item.
         del item.tags[:]
 
+    def _ensure_user_owns_item(self, user, item):
+        """Raises exception if user does not own item.
+        Notice that even admin users cannot directly modify tags on items they do not own.
+        To modify tags on items they don't own, admin users must impersonate the item's owner.
+        """
+        if user is None:
+            raise AuthenticationRequired("Must be logged in to manage tags.")
+        history = getattr(item, "history", None)
+        is_owner = getattr(item, "user", None) == user or getattr(history, "user", None) == user
+        if not is_owner:
+            raise ItemOwnershipException("User does not own item.")
+
     def item_has_tag(self, user, item, tag):
         """Returns true if item is has a given tag."""
         # Get tag name.
@@ -167,6 +185,7 @@ def item_has_tag(self, user, item, tag):
         return False
 
     def apply_item_tag(self, user, item, name, value=None, flush=True):
+        self._ensure_user_owns_item(user, item)
         # Use lowercase name for searching/creating tag.
         if name is None:
             return
@@ -203,6 +222,7 @@ def apply_item_tag(self, user, item, name, value=None, flush=True):
 
     def apply_item_tags(self, user, item, tags_str, flush=True):
         """Apply tags to an item."""
+        self._ensure_user_owns_item(user, item)
         # Parse tags.
         parsed_tags = self.parse_tags(tags_str)
         # Apply each tag.

From c5d664177b93e4bb6abe3d2408325afba9b7ce69 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Thu, 29 Jun 2023 16:29:36 +0200
Subject: [PATCH 03/41] Add test coverage for all taggable items

---
 lib/galaxy_test/api/test_tags.py | 191 +++++++++++++++++++++++++++++++
 1 file changed, 191 insertions(+)
 create mode 100644 lib/galaxy_test/api/test_tags.py

diff --git a/lib/galaxy_test/api/test_tags.py b/lib/galaxy_test/api/test_tags.py
new file mode 100644
index 000000000000..4521f7cf42ab
--- /dev/null
+++ b/lib/galaxy_test/api/test_tags.py
@@ -0,0 +1,191 @@
+import json
+from typing import List
+from unittest import SkipTest
+from uuid import uuid4
+
+from galaxy_test.base.populators import (
+    DatasetCollectionPopulator,
+    DatasetPopulator,
+    LibraryPopulator,
+    WorkflowPopulator,
+)
+from ._framework import ApiTestCase
+
+
+class TagsApiTests(ApiTestCase):
+    api_name: str
+    item_class: str
+    publishable: bool
+
+    def setUp(self):
+        super().setUp()
+        self.dataset_populator = DatasetPopulator(self.galaxy_interactor)
+
+    def create_item(self) -> str:
+        """Creates a taggable item and returns it's ID."""
+        raise SkipTest("Abstract")
+
+    def test_tags_on_item(self):
+        item_id = self.create_item()
+
+        self._assert_tags_in_item(item_id, [])
+
+        tag_name = "mytagname"
+        tag_value = "mytagvalue"
+        create_tag_response = self._create_named_tag_on_item(item_id, tag_name, tag_value)
+        self._assert_status_code_is(create_tag_response, 200)
+        item_tag_assoc = create_tag_response.json()
+        assert item_tag_assoc["user_tname"] == tag_name
+        assert item_tag_assoc["user_value"] == tag_value
+
+        new_tags = ["UpdatedTag"]
+        self._update_tags_using_item_update(item_id, new_tags)
+        self._assert_tags_in_item(item_id, new_tags)
+
+        new_tags = ["APITag"]
+        update_history_tags_response = self._update_tags_using_tags_api(item_id, new_tags)
+        self._assert_status_code_is(update_history_tags_response, 204)
+        self._assert_tags_in_item(item_id, new_tags)
+
+        # other users can't create or update tags
+        with self._different_user():
+            create_tag_response = self._create_named_tag_on_item(
+                item_id, tag_name="othertagname", tag_value="othertagvalue"
+            )
+            self._assert_status_code_is(create_tag_response, 403)
+
+            new_tags = ["otherAPITag"]
+            update_item_response = self._update_tags_using_item_update(item_id, new_tags)
+            self._assert_status_code_is(update_item_response, 403)
+
+            new_tags = ["otherAPITag"]
+            update_history_tags_response = self._update_tags_using_tags_api(item_id, new_tags)
+            self._assert_status_code_is(update_history_tags_response, 403)
+
+    def _create_named_tag_on_item(self, item_id, tag_name, tag_value):
+        create_tag_response = self._post(
+            f"{self.api_name}/{item_id}/tags/{tag_name}", data={"value": tag_value}, json=True
+        )
+
+        return create_tag_response
+
+    def _update_tags_using_item_update(self, item_id, new_tags):
+        update_item_response = self._put(f"{self.api_name}/{item_id}", data={"tags": new_tags}, json=True)
+        return update_item_response
+
+    def _update_tags_using_tags_api(self, item_id, new_tags):
+        payload = {"item_class": self.item_class, "item_id": item_id, "item_tags": new_tags}
+        update_history_tags_response = self._put("tags", data=payload, json=True)
+        return update_history_tags_response
+
+    def _assert_tags_in_item(self, item_id, expected_tags: List[str]):
+        item_response = self._get(f"{self.api_name}/{item_id}")
+        self._assert_status_code_is(item_response, 200)
+        item = item_response.json()
+        assert "tags" in item
+        assert item["tags"] == expected_tags
+
+
+class TestHistoryTags(TagsApiTests):
+    api_name = "histories"
+    item_class = "History"
+
+    def create_item(self) -> str:
+        item_id = self.dataset_populator.new_history(name=f"history-{uuid4()}")
+        return item_id
+
+
+class TestDatasetTags(TagsApiTests):
+    api_name = "datasets"
+    item_class = "HistoryDatasetAssociation"
+
+    def create_item(self) -> str:
+        history_id = self.dataset_populator.new_history(name=f"history-{uuid4()}")
+        item_id = self.dataset_populator.new_dataset(history_id=history_id)
+        return item_id["id"]
+
+
+class TestDatasetCollectionTags(TagsApiTests):
+    api_name = "dataset_collections"
+    item_class = "HistoryDatasetCollectionAssociation"
+
+    def setUp(self):
+        super().setUp()
+        self.dataset_collection_populator = DatasetCollectionPopulator(self.galaxy_interactor)
+
+    def create_item(self) -> str:
+        history_id = self.dataset_populator.new_history(name=f"history-{uuid4()}")
+        response = self.dataset_collection_populator.create_list_in_history(history_id)
+        self._assert_status_code_is(response, 200)
+        hdca = response.json()["outputs"][0]
+        return hdca["id"]
+
+
+class TestLibraryDatasetTags(TagsApiTests):
+    api_name = "libraries"
+    item_class = "LibraryDatasetDatasetAssociation"
+
+    def setUp(self):
+        super().setUp()
+        self.library_populator = LibraryPopulator(self.galaxy_interactor)
+
+    def create_item(self) -> str:
+        ld = self.library_populator.new_library_dataset(name=f"test-library-dataset-{uuid4()}")
+        return ld["id"]
+
+
+class TestPageTags(TagsApiTests):
+    api_name = "pages"
+    item_class = "Page"
+
+    def create_item(self) -> str:
+        page = self.dataset_populator.new_page(slug=f"page-{uuid4()}")
+        return page["id"]
+
+
+class TestWorkflowTags(TagsApiTests):
+    api_name = "workflows"
+    item_class = "StoredWorkflow"
+
+    def setUp(self):
+        super().setUp()
+        self.workflow_populator = WorkflowPopulator(self.galaxy_interactor)
+
+    def create_item(self) -> str:
+        workflow = self.workflow_populator.load_workflow(name=f"workflow-{uuid4()}")
+        data = dict(
+            workflow=json.dumps(workflow),
+        )
+        upload_response = self._post("workflows", data=data)
+
+        self._assert_status_code_is(upload_response, 200)
+        return upload_response.json()["id"]
+
+
+class TestVisualizationTags(TagsApiTests):
+    api_name = "visualizations"
+    item_class = "Visualization"
+
+    def create_item(self) -> str:
+        uuid_str = str(uuid4())
+
+        title = f"Test Visualization {uuid_str}"
+        slug = f"test-visualization-{uuid_str}"
+        config = json.dumps(
+            {
+                "x": 10,
+                "y": 12,
+            }
+        )
+        create_payload = {
+            "title": title,
+            "slug": slug,
+            "type": "example",
+            "dbkey": "hg17",
+            "annotation": "this is a test visualization for tags",
+            "config": config,
+        }
+        response = self._post("visualizations", data=create_payload)
+        self._assert_status_code_is(response, 200)
+        viz = response.json()
+        return viz["id"]

From 5b9c5c53fbe81b3b4d0d79ed7a8bb9174f599a0d Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Fri, 30 Jun 2023 12:26:00 +0200
Subject: [PATCH 04/41] Fix user session in manager unit tests

Most unit tests were creating histories with user ID=2 while adding datasets and collections to those histories using the default `trans.user` which was user ID=1 (admin).
This resulted in `ItemOwnershipException: User does not own item.` which I think makes sense.
---
 test/unit/app/managers/test_CollectionManager.py      |  2 ++
 test/unit/app/managers/test_HDCAManager.py            |  1 +
 test/unit/app/managers/test_HistoryContentsManager.py | 10 ++++++++++
 3 files changed, 13 insertions(+)

diff --git a/test/unit/app/managers/test_CollectionManager.py b/test/unit/app/managers/test_CollectionManager.py
index feb7da6e9cfb..3e7e17a8bbcb 100644
--- a/test/unit/app/managers/test_CollectionManager.py
+++ b/test/unit/app/managers/test_CollectionManager.py
@@ -28,6 +28,7 @@ def set_up_managers(self):
 
     def test_create_simple_list(self):
         owner = self.user_manager.create(**user2_data)
+        self.trans.set_user(owner)
 
         history = self.history_manager.create(name="history1", user=owner)
 
@@ -80,6 +81,7 @@ def test_create_simple_list(self):
 
     def test_update_from_dict(self):
         owner = self.user_manager.create(**user2_data)
+        self.trans.set_user(owner)
 
         history = self.history_manager.create(name="history1", user=owner)
 
diff --git a/test/unit/app/managers/test_HDCAManager.py b/test/unit/app/managers/test_HDCAManager.py
index 1388953372d0..23ea050b2def 100644
--- a/test/unit/app/managers/test_HDCAManager.py
+++ b/test/unit/app/managers/test_HDCAManager.py
@@ -31,6 +31,7 @@ def set_up_managers(self):
     def _create_history(self, user_data=None, **kwargs):
         user_data = user_data or user2_data
         owner = self.user_manager.create(**user_data)
+        self.trans.set_user(owner)
         return self.history_manager.create(user=owner, **kwargs)
 
     def _create_hda(self, history, dataset=None, **kwargs):
diff --git a/test/unit/app/managers/test_HistoryContentsManager.py b/test/unit/app/managers/test_HistoryContentsManager.py
index 9c20987d115b..367d80241388 100644
--- a/test/unit/app/managers/test_HistoryContentsManager.py
+++ b/test/unit/app/managers/test_HistoryContentsManager.py
@@ -56,6 +56,7 @@ def add_list_collection_to_history(self, history, hdas, name="test collection",
 class TestHistoryAsContainer(HistoryAsContainerBaseTestCase):
     def test_contents(self):
         user2 = self.user_manager.create(**user2_data)
+        self.trans.set_user(user2)
         history = self.history_manager.create(name="history", user=user2)
 
         self.log("calling contents on an empty history should return an empty list")
@@ -74,6 +75,7 @@ def test_contents(self):
 
     def test_contained(self):
         user2 = self.user_manager.create(**user2_data)
+        self.trans.set_user(user2)
         history = self.history_manager.create(name="history", user=user2)
 
         self.log("calling contained on an empty history should return an empty list")
@@ -86,6 +88,7 @@ def test_contained(self):
 
     def test_copy_elements_on_collection_creation(self):
         user2 = self.user_manager.create(**user2_data)
+        self.trans.set_user(user2)
         history = self.history_manager.create(name="history", user=user2)
         hdas = [self.add_hda_to_history(history, name=("hda-" + str(x))) for x in range(3)]
         hdca = self.add_list_collection_to_history(history, hdas)
@@ -96,6 +99,7 @@ def test_copy_elements_on_collection_creation(self):
 
     def test_subcontainers(self):
         user2 = self.user_manager.create(**user2_data)
+        self.trans.set_user(user2)
         history = self.history_manager.create(name="history", user=user2)
 
         self.log("calling subcontainers on an empty history should return an empty list")
@@ -109,6 +113,7 @@ def test_subcontainers(self):
 
     def test_limit_and_offset(self):
         user2 = self.user_manager.create(**user2_data)
+        self.trans.set_user(user2)
         history = self.history_manager.create(name="history", user=user2)
         contents = []
         contents.extend([self.add_hda_to_history(history, name=("hda-" + str(x))) for x in range(3)])
@@ -130,6 +135,7 @@ def test_limit_and_offset(self):
     def test_orm_filtering(self):
         parse_filter = self.history_contents_filters.parse_filter
         user2 = self.user_manager.create(**user2_data)
+        self.trans.set_user(user2)
         history = self.history_manager.create(name="history", user=user2)
         contents = []
         contents.extend([self.add_hda_to_history(history, name=("hda-" + str(x))) for x in range(3)])
@@ -228,6 +234,7 @@ def test_orm_filtering(self):
 
     def test_order_by(self):
         user2 = self.user_manager.create(**user2_data)
+        self.trans.set_user(user2)
         history = self.history_manager.create(name="history", user=user2)
         contents = []
         contents.extend([self.add_hda_to_history(history, name=("hda-" + str(x))) for x in range(3)])
@@ -263,6 +270,7 @@ def get_create_time(item):
 
     def test_update_time_filter(self):
         user2 = self.user_manager.create(**user2_data)
+        self.trans.set_user(user2)
         history = self.history_manager.create(name="history", user=user2)
         contents = []
         contents.extend([self.add_hda_to_history(history, name=("hda-" + str(x))) for x in range(3)])
@@ -292,6 +300,7 @@ def get_update_time(item):
     def test_filtered_counting(self):
         parse_filter = self.history_contents_filters.parse_filter
         user2 = self.user_manager.create(**user2_data)
+        self.trans.set_user(user2)
         history = self.history_manager.create(name="history", user=user2)
         contents = []
         contents.extend([self.add_hda_to_history(history, name=("hda-" + str(x))) for x in range(3)])
@@ -324,6 +333,7 @@ def test_filtered_counting(self):
 
     def test_type_id(self):
         user2 = self.user_manager.create(**user2_data)
+        self.trans.set_user(user2)
         history = self.history_manager.create(name="history", user=user2)
         contents = []
         contents.extend([self.add_hda_to_history(history, name=("hda-" + str(x))) for x in range(3)])

From 32efc0dee702aa0f27a98d017d31f3582d0db6a7 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Fri, 30 Jun 2023 14:08:15 +0200
Subject: [PATCH 05/41] Do not check ownership on copied items

If the item is not yet persisted we allow to alter the tags as this item is likely being copied from another and does not have an associated user or history yet.
---
 lib/galaxy/model/tags.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py
index 55d5ff8027ec..46d1f2f3c2f9 100644
--- a/lib/galaxy/model/tags.py
+++ b/lib/galaxy/model/tags.py
@@ -162,6 +162,10 @@ def _ensure_user_owns_item(self, user, item):
         Notice that even admin users cannot directly modify tags on items they do not own.
         To modify tags on items they don't own, admin users must impersonate the item's owner.
         """
+        if getattr(item, "id", None) is None:
+            # Item is not persisted, likely it is being copied from an existing, so no need
+            # to check ownership at this point.
+            return
         if user is None:
             raise AuthenticationRequired("Must be logged in to manage tags.")
         history = getattr(item, "history", None)

From 22f819ef168fdb893bbb08a94bfc0ee17b8264da Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 3 Jul 2023 12:49:54 +0200
Subject: [PATCH 06/41] Fix TestLibraryDatasetTags to use the correct endpoint

---
 lib/galaxy_test/api/test_tags.py | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lib/galaxy_test/api/test_tags.py b/lib/galaxy_test/api/test_tags.py
index 4521f7cf42ab..5ca9b7782416 100644
--- a/lib/galaxy_test/api/test_tags.py
+++ b/lib/galaxy_test/api/test_tags.py
@@ -78,10 +78,14 @@ def _update_tags_using_tags_api(self, item_id, new_tags):
         update_history_tags_response = self._put("tags", data=payload, json=True)
         return update_history_tags_response
 
-    def _assert_tags_in_item(self, item_id, expected_tags: List[str]):
+    def _get_item(self, item_id: str):
         item_response = self._get(f"{self.api_name}/{item_id}")
         self._assert_status_code_is(item_response, 200)
         item = item_response.json()
+        return item
+
+    def _assert_tags_in_item(self, item_id, expected_tags: List[str]):
+        item = self._get_item(item_id)
         assert "tags" in item
         assert item["tags"] == expected_tags
 
@@ -131,8 +135,16 @@ def setUp(self):
 
     def create_item(self) -> str:
         ld = self.library_populator.new_library_dataset(name=f"test-library-dataset-{uuid4()}")
+        self.library_id = ld["parent_library_id"]
         return ld["id"]
 
+    def _get_item(self, item_id: str):
+        assert self.library_id is not None, "Library ID not set"
+        item_response = self._get(f"{self.api_name}/{self.library_id}/contents/{item_id}")
+        self._assert_status_code_is(item_response, 200)
+        item = item_response.json()
+        return item
+
 
 class TestPageTags(TagsApiTests):
     api_name = "pages"

From 8475caacca9e83a919d32b919f0242e5836d00a2 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 3 Jul 2023 12:53:05 +0200
Subject: [PATCH 07/41] Temporary drop tags API consistency tests

Those tests can be added back along with the improved API changes in a PR targeting the next version
---
 lib/galaxy_test/api/test_tags.py | 33 --------------------------------
 1 file changed, 33 deletions(-)

diff --git a/lib/galaxy_test/api/test_tags.py b/lib/galaxy_test/api/test_tags.py
index 5ca9b7782416..f8908968f3b8 100644
--- a/lib/galaxy_test/api/test_tags.py
+++ b/lib/galaxy_test/api/test_tags.py
@@ -27,21 +27,8 @@ def create_item(self) -> str:
 
     def test_tags_on_item(self):
         item_id = self.create_item()
-
         self._assert_tags_in_item(item_id, [])
 
-        tag_name = "mytagname"
-        tag_value = "mytagvalue"
-        create_tag_response = self._create_named_tag_on_item(item_id, tag_name, tag_value)
-        self._assert_status_code_is(create_tag_response, 200)
-        item_tag_assoc = create_tag_response.json()
-        assert item_tag_assoc["user_tname"] == tag_name
-        assert item_tag_assoc["user_value"] == tag_value
-
-        new_tags = ["UpdatedTag"]
-        self._update_tags_using_item_update(item_id, new_tags)
-        self._assert_tags_in_item(item_id, new_tags)
-
         new_tags = ["APITag"]
         update_history_tags_response = self._update_tags_using_tags_api(item_id, new_tags)
         self._assert_status_code_is(update_history_tags_response, 204)
@@ -49,30 +36,10 @@ def test_tags_on_item(self):
 
         # other users can't create or update tags
         with self._different_user():
-            create_tag_response = self._create_named_tag_on_item(
-                item_id, tag_name="othertagname", tag_value="othertagvalue"
-            )
-            self._assert_status_code_is(create_tag_response, 403)
-
-            new_tags = ["otherAPITag"]
-            update_item_response = self._update_tags_using_item_update(item_id, new_tags)
-            self._assert_status_code_is(update_item_response, 403)
-
             new_tags = ["otherAPITag"]
             update_history_tags_response = self._update_tags_using_tags_api(item_id, new_tags)
             self._assert_status_code_is(update_history_tags_response, 403)
 
-    def _create_named_tag_on_item(self, item_id, tag_name, tag_value):
-        create_tag_response = self._post(
-            f"{self.api_name}/{item_id}/tags/{tag_name}", data={"value": tag_value}, json=True
-        )
-
-        return create_tag_response
-
-    def _update_tags_using_item_update(self, item_id, new_tags):
-        update_item_response = self._put(f"{self.api_name}/{item_id}", data={"tags": new_tags}, json=True)
-        return update_item_response
-
     def _update_tags_using_tags_api(self, item_id, new_tags):
         payload = {"item_class": self.item_class, "item_id": item_id, "item_tags": new_tags}
         update_history_tags_response = self._put("tags", data=payload, json=True)

From 177703b63750efcfd9fb565c5e7634cf22727812 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 3 Jul 2023 15:26:20 +0200
Subject: [PATCH 08/41] Remove required user for handling tags

Otherwise this would break anonymous usage of tagging in general for legitimate anonymous items.
The check for the history owner user matching seems fine for the anonymous case as both would be None and anonymous users will not be able to tag histories with real owners.
---
 lib/galaxy/model/tags.py | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py
index 46d1f2f3c2f9..ab3db277e97a 100644
--- a/lib/galaxy/model/tags.py
+++ b/lib/galaxy/model/tags.py
@@ -13,10 +13,7 @@
 from sqlalchemy.sql.expression import func
 
 import galaxy.model
-from galaxy.exceptions import (
-    AuthenticationRequired,
-    ItemOwnershipException,
-)
+from galaxy.exceptions import ItemOwnershipException
 from galaxy.model.base import transaction
 from galaxy.model.scoped_session import galaxy_scoped_session
 from galaxy.util import (
@@ -166,8 +163,6 @@ def _ensure_user_owns_item(self, user, item):
             # Item is not persisted, likely it is being copied from an existing, so no need
             # to check ownership at this point.
             return
-        if user is None:
-            raise AuthenticationRequired("Must be logged in to manage tags.")
         history = getattr(item, "history", None)
         is_owner = getattr(item, "user", None) == user or getattr(history, "user", None) == user
         if not is_owner:

From 6043c8d23c25bc2ca0f9c6eacb119ec2a74a0bb8 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Thu, 27 Jul 2023 11:22:26 +0200
Subject: [PATCH 09/41] Drop TaggableManagerMixin that is only used in unit
 tests

---
 lib/galaxy/managers/hdas.py                   |  1 -
 lib/galaxy/managers/hdcas.py                  |  1 -
 lib/galaxy/managers/sharable.py               |  1 -
 lib/galaxy/managers/taggable.py               | 33 ++-----------------
 test/unit/app/managers/test_HDAManager.py     | 18 ++--------
 test/unit/app/managers/test_HistoryManager.py |  7 ++--
 6 files changed, 8 insertions(+), 53 deletions(-)

diff --git a/lib/galaxy/managers/hdas.py b/lib/galaxy/managers/hdas.py
index 449357d38cac..3ca251d1327d 100644
--- a/lib/galaxy/managers/hdas.py
+++ b/lib/galaxy/managers/hdas.py
@@ -71,7 +71,6 @@ class HistoryDatasetAssociationNoHistoryException(Exception):
 class HDAManager(
     datasets.DatasetAssociationManager,
     secured.OwnableManagerMixin,
-    taggable.TaggableManagerMixin,
     annotatable.AnnotatableManagerMixin,
 ):
     """
diff --git a/lib/galaxy/managers/hdcas.py b/lib/galaxy/managers/hdcas.py
index d30663009d20..c3136f03d941 100644
--- a/lib/galaxy/managers/hdcas.py
+++ b/lib/galaxy/managers/hdcas.py
@@ -59,7 +59,6 @@ class HDCAManager(
     secured.AccessibleManagerMixin,
     secured.OwnableManagerMixin,
     deletable.PurgableManagerMixin,
-    taggable.TaggableManagerMixin,
     annotatable.AnnotatableManagerMixin,
 ):
     """
diff --git a/lib/galaxy/managers/sharable.py b/lib/galaxy/managers/sharable.py
index 3c1f908da134..92499743b3d0 100644
--- a/lib/galaxy/managers/sharable.py
+++ b/lib/galaxy/managers/sharable.py
@@ -58,7 +58,6 @@ class SharableModelManager(
     base.ModelManager,
     secured.OwnableManagerMixin,
     secured.AccessibleManagerMixin,
-    taggable.TaggableManagerMixin,
     annotatable.AnnotatableManagerMixin,
     ratable.RatableManagerMixin,
 ):
diff --git a/lib/galaxy/managers/taggable.py b/lib/galaxy/managers/taggable.py
index bcd9e7388190..8f1aa76e0a48 100644
--- a/lib/galaxy/managers/taggable.py
+++ b/lib/galaxy/managers/taggable.py
@@ -42,41 +42,14 @@ def _tags_to_strings(item):
     return sorted(tag_list, key=lambda str: re.sub("^name:", "#", str))
 
 
-def _tags_from_strings(item, tag_handler, new_tags_list, user=None):
-    # TODO: have to assume trans.user here...
-    if not user:
-        # raise galaxy_exceptions.RequestParameterMissingException( 'User required for tags on ' + str( item ) )
-        # TODO: this becomes a 'silent failure' - no tags are set. This is a questionable approach but
-        # I haven't found a better one for anon users copying items with tags
-        return
+def _tags_from_strings(item, tag_handler, new_tags_list, user: Optional[User], galaxy_session: Optional[GalaxySession] = None):
     # TODO: duped from tags manager - de-dupe when moved to taggable mixin
-    tag_handler.delete_item_tags(user, item)
+    tag_handler.delete_item_tags(user, item, galaxy_session=galaxy_session)
     new_tags_str = ",".join(new_tags_list)
-    tag_handler.apply_item_tags(user, item, unicodify(new_tags_str, "utf-8"))
+    tag_handler.apply_item_tags(user, item, unicodify(new_tags_str, "utf-8"), galaxy_session=galaxy_session)
     # TODO:!! does the creation of new_tags_list mean there are now more and more unused tag rows in the db?
 
 
-class TaggableManagerMixin:
-    tag_assoc: Type[model.ItemTagAssociation]
-    tag_handler: GalaxyTagHandler
-
-    # TODO: most of this can be done by delegating to the GalaxyTagHandler?
-    def get_tags(self, item):
-        """
-        Return a list of tag strings.
-        """
-        return _tags_to_strings(item)
-
-    def set_tags(self, item, new_tags, user=None):
-        """
-        Set an `item`'s tags from a list of strings.
-        """
-        return _tags_from_strings(item, self.tag_handler, new_tags, user=user)
-
-    # def tags_by_user( self, user, **kwargs ):
-    # TODO: here or GalaxyTagHandler
-    #    pass
-
 
 class TaggableSerializerMixin:
     def add_serializers(self):
diff --git a/test/unit/app/managers/test_HDAManager.py b/test/unit/app/managers/test_HDAManager.py
index 94ebfbb0b4ac..466093806390 100644
--- a/test/unit/app/managers/test_HDAManager.py
+++ b/test/unit/app/managers/test_HDAManager.py
@@ -89,18 +89,6 @@ def test_create(self):
         assert hda3.history is None, "history will be None"
         assert hda3.hid is None, "should allow setting hid to None (or any other value)"
 
-    def test_hda_tags(self):
-        owner = self.user_manager.create(**user2_data)
-        history1 = self.history_manager.create(name="history1", user=owner)
-        dataset1 = self.dataset_manager.create()
-        hda1 = self.hda_manager.create(history=history1, dataset=dataset1)
-
-        self.log("should be able to set tags on an hda")
-        tags_to_set = ["tag-one", "tag-two"]
-        self.hda_manager.set_tags(hda1, tags_to_set, user=owner)
-        tag_str_array = self.hda_manager.get_tags(hda1)
-        assert sorted(tags_to_set) == sorted(tag_str_array)
-
     def test_hda_annotation(self):
         owner = self.user_manager.create(**user2_data)
         history1 = self.history_manager.create(name="history1", user=owner)
@@ -130,11 +118,9 @@ def test_copy_from_hda(self):
         self.log("tags should be copied between HDAs")
         tagged = self.hda_manager.create(history=history1, dataset=self.dataset_manager.create())
         tags_to_set = ["tag-one", "tag-two"]
-        self.hda_manager.set_tags(tagged, tags_to_set, user=owner)
-
+        self.hda_manager.tag_handler.add_tags_from_list(owner, tagged, tags_to_set)
         hda2 = self.hda_manager.copy(tagged, history=history1)
-        tag_str_array = self.hda_manager.get_tags(hda2)
-        assert sorted(tags_to_set) == sorted(tag_str_array)
+        assert tagged.make_tag_string_list() == hda2.make_tag_string_list()
 
         self.log("annotations should be copied between HDAs")
         annotated = self.hda_manager.create(history=history1, dataset=self.dataset_manager.create())
diff --git a/test/unit/app/managers/test_HistoryManager.py b/test/unit/app/managers/test_HistoryManager.py
index 32be275d4db1..0cc5376472a2 100644
--- a/test/unit/app/managers/test_HistoryManager.py
+++ b/test/unit/app/managers/test_HistoryManager.py
@@ -86,13 +86,13 @@ def test_copy(self):
         history1 = self.history_manager.create(name="history1", user=user2)
         tags = ["tag-one"]
         annotation = "history annotation"
-        self.history_manager.set_tags(history1, tags, user=user2)
+        self.history_manager.tag_handler.set_tags_from_list(user=user2, item=history1, new_tags_list=tags)
         self.history_manager.annotate(history1, annotation, user=user2)
 
         hda = self.add_hda_to_history(history1, name="wat")
         hda_tags = ["tag-one", "tag-two"]
         hda_annotation = "annotation"
-        self.hda_manager.set_tags(hda, hda_tags, user=user2)
+        self.hda_manager.tag_handler.set_tags_from_list(user=user2, item=hda, new_tags_list=hda_tags)
         self.hda_manager.annotate(hda, hda_annotation, user=user2)
 
         history2 = self.history_manager.copy(history1, user=user3)
@@ -103,8 +103,7 @@ def test_copy(self):
         assert history2 != history1
 
         copied_hda = history2.datasets[0]
-        copied_hda_tags = self.hda_manager.get_tags(copied_hda)
-        assert sorted(hda_tags) == sorted(copied_hda_tags)
+        assert hda.make_tag_string_list() == copied_hda.make_tag_string_list()
         copied_hda_annotation = self.hda_manager.annotation(copied_hda)
         assert hda_annotation == copied_hda_annotation
 

From ad6bc9b278b28127091ba192898c0428bc3ff596 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Fri, 28 Jul 2023 12:28:30 +0200
Subject: [PATCH 10/41] Check tag ownership for anon users via galaxy session

---
 lib/galaxy/job_execution/actions/post.py     |  6 +-
 lib/galaxy/job_execution/output_collect.py   |  4 +-
 lib/galaxy/jobs/__init__.py                  |  4 +-
 lib/galaxy/managers/collections.py           | 73 ++++++++++-----
 lib/galaxy/managers/context.py               | 17 ++++
 lib/galaxy/managers/hdas.py                  |  4 -
 lib/galaxy/managers/library_datasets.py      | 14 +--
 lib/galaxy/managers/taggable.py              | 21 ++---
 lib/galaxy/managers/tags.py                  |  6 +-
 lib/galaxy/managers/workflows.py             |  4 +-
 lib/galaxy/model/store/__init__.py           |  2 +-
 lib/galaxy/model/store/discover.py           | 16 +++-
 lib/galaxy/model/tags.py                     | 99 +++++++++++++++-----
 lib/galaxy/tools/__init__.py                 | 10 +-
 lib/galaxy/tools/actions/__init__.py         |  2 +-
 lib/galaxy/tools/actions/model_operations.py | 10 +-
 lib/galaxy/tools/actions/upload_common.py    |  7 +-
 lib/galaxy/tools/evaluation.py               |  4 +-
 lib/galaxy/tools/imp_exp/__init__.py         |  5 +-
 lib/galaxy/webapps/base/controller.py        | 16 ++--
 lib/galaxy/webapps/galaxy/api/jobs.py        |  4 +-
 lib/galaxy/webapps/galaxy/api/workflows.py   |  5 +-
 lib/galaxy/webapps/galaxy/controllers/tag.py | 16 ++--
 lib/galaxy/work/context.py                   | 30 ++++--
 24 files changed, 254 insertions(+), 125 deletions(-)

diff --git a/lib/galaxy/job_execution/actions/post.py b/lib/galaxy/job_execution/actions/post.py
index f8edd4704774..b151d7ba6e3e 100644
--- a/lib/galaxy/job_execution/actions/post.py
+++ b/lib/galaxy/job_execution/actions/post.py
@@ -448,13 +448,13 @@ class TagDatasetAction(DefaultJobAction):
     def execute_on_mapped_over(
         cls, trans, sa_session, action, step_inputs, step_outputs, replacement_dict, final_job_state=None
     ):
-        tag_handler = trans.app.tag_handler.create_tag_handler_session()
         if action.action_arguments:
             tags = [
                 t.replace("#", "name:") if t.startswith("#") else t
                 for t in [t.strip() for t in action.action_arguments.get("tags", "").split(",") if t.strip()]
             ]
-            if tags:
+            if tags and step_outputs:
+                tag_handler = trans.tag_handler
                 for name, step_output in step_outputs.items():
                     if action.output_name == "" or name == action.output_name:
                         cls._execute(tag_handler, trans.user, step_output, tags)
@@ -462,12 +462,12 @@ def execute_on_mapped_over(
     @classmethod
     def execute(cls, app, sa_session, action, job, replacement_dict, final_job_state=None):
         if action.action_arguments:
-            tag_handler = app.tag_handler.create_tag_handler_session()
             tags = [
                 t.replace("#", "name:") if t.startswith("#") else t
                 for t in [t.strip() for t in action.action_arguments.get("tags", "").split(",") if t.strip()]
             ]
             if tags:
+                tag_handler = app.tag_handler.create_tag_handler_session(job.galaxy_session)
                 for dataset_assoc in job.output_datasets:
                     if action.output_name == "" or dataset_assoc.name == action.output_name:
                         cls._execute(tag_handler, job.user, dataset_assoc.dataset, tags)
diff --git a/lib/galaxy/job_execution/output_collect.py b/lib/galaxy/job_execution/output_collect.py
index 826218c42b50..98f5dad4c1f3 100644
--- a/lib/galaxy/job_execution/output_collect.py
+++ b/lib/galaxy/job_execution/output_collect.py
@@ -244,14 +244,14 @@ def __init__(
     @property
     def tag_handler(self):
         if self._tag_handler is None:
-            self._tag_handler = self.app.tag_handler.create_tag_handler_session()
+            self._tag_handler = self.app.tag_handler.create_tag_handler_session(self.job.galaxy_session)
         return self._tag_handler
 
     @property
     def work_context(self):
         from galaxy.work.context import WorkRequestContext
 
-        return WorkRequestContext(self.app, user=self.user)
+        return WorkRequestContext(self.app, user=self.user, galaxy_session=self.job.galaxy_session)
 
     @property
     def sa_session(self) -> ScopedSession:
diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py
index 786947e08dce..318a410d4221 100644
--- a/lib/galaxy/jobs/__init__.py
+++ b/lib/galaxy/jobs/__init__.py
@@ -1058,7 +1058,7 @@ def tool_directory(self):
     def job_io(self):
         if self._job_io is None:
             job = self.get_job()
-            work_request = WorkRequestContext(self.app, user=job.user)
+            work_request = WorkRequestContext(self.app, user=job.user, galaxy_session=job.galaxy_session)
             user_context = ProvidesUserFileSourcesUserContext(work_request)
             tool_source = self.tool and self.tool.tool_source.to_string()
             self._job_io = JobIO(
@@ -1913,7 +1913,7 @@ def fail(message=job.info, exception=None):
                     app=self.app,
                     import_options=import_options,
                     user=job.user,
-                    tag_handler=self.app.tag_handler.create_tag_handler_session(),
+                    tag_handler=self.app.tag_handler.create_tag_handler_session(job.galaxy_session),
                 )
                 import_model_store.perform_import(history=job.history, job=job)
                 if job.state == job.states.ERROR:
diff --git a/lib/galaxy/managers/collections.py b/lib/galaxy/managers/collections.py
index a86ab1fded67..1d92e23f141a 100644
--- a/lib/galaxy/managers/collections.py
+++ b/lib/galaxy/managers/collections.py
@@ -22,6 +22,11 @@
     RequestParameterInvalidException,
 )
 from galaxy.managers.collections_util import validate_input_element_identifiers
+from galaxy.managers.context import (
+    ProvidesAppContext,
+    ProvidesHistoryContext,
+    ProvidesUserContext,
+)
 from galaxy.managers.hdas import (
     HDAManager,
     HistoryDatasetAssociationNoHistoryException,
@@ -35,7 +40,6 @@
 from galaxy.model.dataset_collections.registry import DATASET_COLLECTION_TYPES_REGISTRY
 from galaxy.model.dataset_collections.type_description import COLLECTION_TYPE_DESCRIPTION_FACTORY
 from galaxy.model.mapping import GalaxyModelMapping
-from galaxy.model.tags import GalaxyTagHandler
 from galaxy.schema.schema import DatasetCollectionInstanceType
 from galaxy.schema.tasks import PrepareDatasetCollectionDownload
 from galaxy.security.idencoding import IdEncodingHelper
@@ -65,7 +69,6 @@ def __init__(
         security: IdEncodingHelper,
         hda_manager: HDAManager,
         history_manager: HistoryManager,
-        tag_handler: GalaxyTagHandler,
         ldda_manager: LDDAManager,
         short_term_storage_monitor: ShortTermStorageMonitor,
     ):
@@ -74,15 +77,13 @@ def __init__(
         self.model = model
         self.security = security
         self.short_term_storage_monitor = short_term_storage_monitor
-
         self.hda_manager = hda_manager
         self.history_manager = history_manager
-        self.tag_handler = tag_handler.create_tag_handler_session()
         self.ldda_manager = ldda_manager
 
     def precreate_dataset_collection_instance(
         self,
-        trans,
+        trans: ProvidesHistoryContext,
         parent,
         name,
         structure,
@@ -158,7 +159,7 @@ def precreate_dataset_collection(
 
     def create(
         self,
-        trans,
+        trans: ProvidesHistoryContext,
         parent,
         name,
         collection_type,
@@ -222,7 +223,7 @@ def create(
 
     def _create_instance_for_collection(
         self,
-        trans,
+        trans: ProvidesHistoryContext,
         parent,
         name,
         dataset_collection,
@@ -270,14 +271,16 @@ def _create_instance_for_collection(
         # values.
         if isinstance(tags, list):
             assert implicit_inputs is None, implicit_inputs
-            tags = self.tag_handler.add_tags_from_list(trans.user, dataset_collection_instance, tags, flush=False)
+            tags = trans.tag_handler.add_tags_from_list(
+                trans.user, dataset_collection_instance, tags, flush=False, galaxy_session=trans.galaxy_session
+            )
         else:
             tags = self._append_tags(dataset_collection_instance, implicit_inputs, tags)
         return self.__persist(dataset_collection_instance, flush=flush)
 
     def create_dataset_collection(
         self,
-        trans,
+        trans: ProvidesHistoryContext,
         collection_type,
         element_identifiers=None,
         elements=None,
@@ -324,7 +327,9 @@ def create_dataset_collection(
         dataset_collection.collection_type = collection_type
         return dataset_collection
 
-    def get_converters_for_collection(self, trans, id, datatypes_registry: Registry, instance_type="history"):
+    def get_converters_for_collection(
+        self, trans: ProvidesHistoryContext, id, datatypes_registry: Registry, instance_type="history"
+    ):
         dataset_collection_instance = self.get_dataset_collection_instance(
             trans, id=id, instance_type=instance_type, check_ownership=True
         )
@@ -360,7 +365,7 @@ def get_converters_for_collection(self, trans, id, datatypes_registry: Registry,
 
     def _element_identifiers_to_elements(
         self,
-        trans,
+        trans: ProvidesHistoryContext,
         collection_type_description,
         element_identifiers,
         hide_source_items=False,
@@ -405,7 +410,7 @@ def _append_tags(self, dataset_collection_instance, implicit_inputs=None, tags=N
     def collection_builder_for(self, dataset_collection):
         return builder.BoundCollectionBuilder(dataset_collection)
 
-    def delete(self, trans, instance_type, id, recursive=False, purge=False):
+    def delete(self, trans: ProvidesHistoryContext, instance_type, id, recursive=False, purge=False):
         dataset_collection_instance = self.get_dataset_collection_instance(
             trans, instance_type, id, check_ownership=True
         )
@@ -431,7 +436,7 @@ def delete(self, trans, instance_type, id, recursive=False, purge=False):
             trans.sa_session.commit()
         return async_result
 
-    def update(self, trans, instance_type, id, payload):
+    def update(self, trans: ProvidesHistoryContext, instance_type, id, payload):
         dataset_collection_instance = self.get_dataset_collection_instance(
             trans, instance_type, id, check_ownership=True
         )
@@ -447,7 +452,15 @@ def update(self, trans, instance_type, id, payload):
         changed = self._set_from_dict(trans, dataset_collection_instance, payload)
         return changed
 
-    def copy(self, trans, parent, source, encoded_source_id, copy_elements=False, dataset_instance_attributes=None):
+    def copy(
+        self,
+        trans: ProvidesHistoryContext,
+        parent,
+        source,
+        encoded_source_id,
+        copy_elements=False,
+        dataset_instance_attributes=None,
+    ):
         """
         PRECONDITION: security checks on ability to add to parent occurred
         during load.
@@ -467,7 +480,7 @@ def copy(self, trans, parent, source, encoded_source_id, copy_elements=False, da
             trans.sa_session.commit()
         return new_hdca
 
-    def _set_from_dict(self, trans, dataset_collection_instance, new_data):
+    def _set_from_dict(self, trans: ProvidesUserContext, dataset_collection_instance, new_data):
         # send what we can down into the model
         changed = dataset_collection_instance.set_from_dict(new_data)
 
@@ -481,10 +494,13 @@ def _set_from_dict(self, trans, dataset_collection_instance, new_data):
         # the api promises a list of changed fields, but tags are not marked as changed to avoid the
         # flush, so we must handle changed tag responses manually
         new_tags = None
-        if "tags" in new_data.keys() and trans.get_user():
+        if "tags" in new_data.keys():
             # set_tags_from_list will flush on its own, no need to add to 'changed' here and incur a second flush.
-            new_tags = self.tag_handler.set_tags_from_list(
-                trans.get_user(), dataset_collection_instance, new_data["tags"]
+            new_tags = trans.tag_handler.set_tags_from_list(
+                trans.user,
+                dataset_collection_instance,
+                new_data["tags"],
+                galaxy_session=trans.galaxy_session,
             )
 
         if changed.keys():
@@ -634,14 +650,18 @@ def __load_element(self, trans, element_identifier, hide_source_items, copy_elem
                 hda.id, user=trans.user, current_history=history or trans.history
             ):
                 hda.visible = False
-            self.tag_handler.apply_item_tags(user=trans.user, item=element, tags_str=tag_str, flush=False)
+            trans.tag_handler.apply_item_tags(
+                user=trans.user, item=element, tags_str=tag_str, flush=False, galaxy_session=trans.galaxy_session
+            )
             return element
         elif src_type == "ldda":
             element2 = self.ldda_manager.get(trans, element_id, check_accessible=True)
             element3 = element2.to_history_dataset_association(
                 history or trans.history, add_to_history=True, visible=not hide_source_items
             )
-            self.tag_handler.apply_item_tags(user=trans.user, item=element3, tags_str=tag_str, flush=False)
+            trans.tag_handler.apply_item_tags(
+                user=trans.user, item=element3, tags_str=tag_str, flush=False, galaxy_session=trans.galaxy_session
+            )
             return element3
         elif src_type == "hdca":
             # TODO: Option to copy? Force copy? Copy or allow if not owned?
@@ -658,18 +678,18 @@ def match_collections(self, collections_to_match):
 
     @overload
     def get_dataset_collection_instance(
-        self, trans, instance_type: Literal["history"], id, **kwds: Any
+        self, trans: ProvidesHistoryContext, instance_type: Literal["history"], id, **kwds: Any
     ) -> model.HistoryDatasetCollectionAssociation:
         ...
 
     @overload
     def get_dataset_collection_instance(
-        self, trans, instance_type: Literal["library"], id, **kwds: Any
+        self, trans: ProvidesHistoryContext, instance_type: Literal["library"], id, **kwds: Any
     ) -> model.LibraryDatasetCollectionAssociation:
         ...
 
     def get_dataset_collection_instance(
-        self, trans, instance_type: DatasetCollectionInstanceType, id, **kwds: Any
+        self, trans: ProvidesHistoryContext, instance_type: DatasetCollectionInstanceType, id, **kwds: Any
     ) -> Union[model.HistoryDatasetCollectionAssociation, model.LibraryDatasetCollectionAssociation]:
         """ """
         if instance_type == "history":
@@ -784,7 +804,7 @@ def __init_rule_data(self, elements, collection_type_description, parent_identif
         return data, sources
 
     def __get_history_collection_instance(
-        self, trans, id, check_ownership=False, check_accessible=True
+        self, trans: ProvidesHistoryContext, id, check_ownership=False, check_accessible=True
     ) -> model.HistoryDatasetCollectionAssociation:
         instance_id = trans.app.security.decode_id(id) if isinstance(id, str) else id
         collection_instance = trans.sa_session.query(trans.app.model.HistoryDatasetCollectionAssociation).get(
@@ -792,6 +812,7 @@ def __get_history_collection_instance(
         )
         if not collection_instance:
             raise RequestParameterInvalidException("History dataset collection association not found")
+        # TODO: that sure looks like a bug, we can't check ownership using the history of the object we're checking ownership for ...
         history = getattr(trans, "history", collection_instance.history)
         if check_ownership:
             self.history_manager.error_unless_owner(collection_instance.history, trans.user, current_history=history)
@@ -802,7 +823,7 @@ def __get_history_collection_instance(
         return collection_instance
 
     def __get_library_collection_instance(
-        self, trans, id, check_ownership=False, check_accessible=True
+        self, trans: ProvidesHistoryContext, id, check_ownership=False, check_accessible=True
     ) -> model.LibraryDatasetCollectionAssociation:
         if check_ownership:
             raise NotImplementedError(
@@ -823,7 +844,7 @@ def __get_library_collection_instance(
                 )
         return collection_instance
 
-    def get_collection_contents(self, trans, parent_id, limit=None, offset=None):
+    def get_collection_contents(self, trans: ProvidesAppContext, parent_id, limit=None, offset=None):
         """Find first level of collection contents by containing collection parent_id"""
         contents_qry = self._get_collection_contents_qry(parent_id, limit=limit, offset=offset)
         contents = contents_qry.with_session(trans.sa_session()).all()
diff --git a/lib/galaxy/managers/context.py b/lib/galaxy/managers/context.py
index 4ee1a1337cf4..2a19c85ef533 100644
--- a/lib/galaxy/managers/context.py
+++ b/lib/galaxy/managers/context.py
@@ -39,6 +39,7 @@
 from json import dumps
 from typing import (
     Callable,
+    cast,
     List,
     Optional,
 )
@@ -49,15 +50,18 @@
 )
 from galaxy.model import (
     Dataset,
+    GalaxySession,
     History,
     HistoryDatasetAssociation,
     Role,
+    User,
 )
 from galaxy.model.base import (
     ModelMapping,
     transaction,
 )
 from galaxy.model.scoped_session import galaxy_scoped_session
+from galaxy.model.tags import GalaxyTagHandlerSession
 from galaxy.schema.tasks import RequestUser
 from galaxy.security.idencoding import IdEncodingHelper
 from galaxy.security.vault import UserVaultWrapper
@@ -206,6 +210,15 @@ class ProvidesUserContext(ProvidesAppContext):
     properties.
     """
 
+    galaxy_session: Optional[GalaxySession] = None
+    _tag_handler: Optional[GalaxyTagHandlerSession] = None
+
+    @property
+    def tag_handler(self):
+        if self._tag_handler is None:
+            self._tag_handler = self.app.tag_handler.create_tag_handler_session(self.galaxy_session)
+        return self._tag_handler
+
     @property
     def async_request_user(self) -> RequestUser:
         if self.user is None:
@@ -221,6 +234,10 @@ def user_vault(self):
         """Provide access to a user's personal vault."""
         return UserVaultWrapper(self.app.vault, self.user)
 
+    def get_user(self) -> Optional[User]:
+        user = cast(Optional[User], self.user or self.galaxy_session and self.galaxy_session.user)
+        return user
+
     @property
     def anonymous(self) -> bool:
         return self.user is None
diff --git a/lib/galaxy/managers/hdas.py b/lib/galaxy/managers/hdas.py
index 3ca251d1327d..17abe5063337 100644
--- a/lib/galaxy/managers/hdas.py
+++ b/lib/galaxy/managers/hdas.py
@@ -42,7 +42,6 @@
 )
 from galaxy.model.base import transaction
 from galaxy.model.deferred import materializer_factory
-from galaxy.model.tags import GalaxyTagHandler
 from galaxy.schema.schema import DatasetSourceType
 from galaxy.schema.storage_cleaner import (
     CleanableItemsSummary,
@@ -92,14 +91,12 @@ def __init__(
         app: MinimalManagerApp,
         user_manager: users.UserManager,
         ldda_manager: lddas.LDDAManager,
-        tag_handler: GalaxyTagHandler,
     ):
         """
         Set up and initialize other managers needed by hdas.
         """
         super().__init__(app)
         self.user_manager = user_manager
-        self.tag_handler = tag_handler
         self.ldda_manager = ldda_manager
 
     def get_owned_ids(self, object_ids, history=None):
@@ -708,7 +705,6 @@ class HDADeserializer(
     def __init__(self, app: MinimalManagerApp):
         super().__init__(app)
         self.hda_manager = self.manager
-        self.tag_handler = app.tag_handler
 
     def add_deserializers(self):
         super().add_deserializers()
diff --git a/lib/galaxy/managers/library_datasets.py b/lib/galaxy/managers/library_datasets.py
index 1f06a6cfb4f3..76cfc97a656c 100644
--- a/lib/galaxy/managers/library_datasets.py
+++ b/lib/galaxy/managers/library_datasets.py
@@ -12,6 +12,7 @@
     RequestParameterInvalidException,
 )
 from galaxy.managers import datasets
+from galaxy.managers.context import ProvidesUserContext
 from galaxy.model import tags
 from galaxy.model.base import transaction
 from galaxy.structured_app import MinimalManagerApp
@@ -27,7 +28,6 @@ class LibraryDatasetsManager(datasets.DatasetAssociationManager):
 
     def __init__(self, app: MinimalManagerApp):
         self.app = app
-        self.tag_handler = tags.GalaxyTagHandler(app.model.context)
 
     def get(self, trans, decoded_library_dataset_id, check_accessible=True):
         """
@@ -82,7 +82,7 @@ def update(self, trans, ld, payload):
         self._set_from_dict(trans, ldda, payload)
         return ld
 
-    def _set_from_dict(self, trans, ldda, new_data):
+    def _set_from_dict(self, trans: ProvidesUserContext, ldda, new_data):
         changed = False
         new_name = new_data.get("name", None)
         if new_name is not None and new_name != ldda.name:
@@ -109,10 +109,12 @@ def _set_from_dict(self, trans, ldda, new_data):
             changed = True
         new_tags = new_data.get("tags", None)
         if new_tags is not None and new_tags != ldda.tags:
-            self.tag_handler.delete_item_tags(item=ldda, user=trans.user)
-            tag_list = self.tag_handler.parse_tags_list(new_tags)
+            trans.tag_handler.delete_item_tags(item=ldda, user=trans.user, galaxy_session=trans.galaxy_session)
+            tag_list = trans.tag_handler.parse_tags_list(new_tags)
             for tag in tag_list:
-                self.tag_handler.apply_item_tag(item=ldda, user=trans.user, name=tag[0], value=tag[1])
+                trans.tag_handler.apply_item_tag(
+                    item=ldda, user=trans.user, name=tag[0], value=tag[1], galaxy_session=trans.galaxy_session
+                )
             changed = True
         if changed:
             ldda.update_parent_folder_update_times()
@@ -251,7 +253,7 @@ def serialize(self, trans, ld):
             current_user_roles, ld
         )
         rval["is_unrestricted"] = trans.app.security_agent.dataset_is_public(ldda.dataset)
-        rval["tags"] = self.tag_handler.get_tags_str(ldda.tags)
+        rval["tags"] = trans.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(
diff --git a/lib/galaxy/managers/taggable.py b/lib/galaxy/managers/taggable.py
index 8f1aa76e0a48..249cf31696a0 100644
--- a/lib/galaxy/managers/taggable.py
+++ b/lib/galaxy/managers/taggable.py
@@ -6,7 +6,10 @@
 
 import logging
 import re
-from typing import Type
+from typing import (
+    Optional,
+    Type,
+)
 
 from sqlalchemy import (
     func,
@@ -14,6 +17,7 @@
 )
 
 from galaxy import model
+from galaxy.managers.context import ProvidesUserContext
 from galaxy.model.tags import GalaxyTagHandler
 from galaxy.util import unicodify
 from .base import (
@@ -42,15 +46,6 @@ def _tags_to_strings(item):
     return sorted(tag_list, key=lambda str: re.sub("^name:", "#", str))
 
 
-def _tags_from_strings(item, tag_handler, new_tags_list, user: Optional[User], galaxy_session: Optional[GalaxySession] = None):
-    # TODO: duped from tags manager - de-dupe when moved to taggable mixin
-    tag_handler.delete_item_tags(user, item, galaxy_session=galaxy_session)
-    new_tags_str = ",".join(new_tags_list)
-    tag_handler.apply_item_tags(user, item, unicodify(new_tags_str, "utf-8"), galaxy_session=galaxy_session)
-    # TODO:!! does the creation of new_tags_list mean there are now more and more unused tag rows in the db?
-
-
-
 class TaggableSerializerMixin:
     def add_serializers(self):
         self.serializers["tags"] = self.serialize_tags
@@ -69,14 +64,16 @@ class TaggableDeserializerMixin:
     def add_deserializers(self):
         self.deserializers["tags"] = self.deserialize_tags
 
-    def deserialize_tags(self, item, key, val, user=None, **context):
+    def deserialize_tags(
+        self, item, key, val, *, user: Optional[model.User] = None, trans: ProvidesUserContext, **context
+    ):
         """
         Make sure `val` is a valid list of tag strings and assign them.
 
         Note: this will erase any previous tags.
         """
         new_tags_list = self.validate.basestring_list(key, val)
-        _tags_from_strings(item, self.tag_handler, new_tags_list, user=user)
+        trans.tag_handler.set_tags_from_list(user=user, item=item, new_tags_list=new_tags_list)
         return item.tags
 
 
diff --git a/lib/galaxy/managers/tags.py b/lib/galaxy/managers/tags.py
index db5a3a3d657c..043a85ace394 100644
--- a/lib/galaxy/managers/tags.py
+++ b/lib/galaxy/managers/tags.py
@@ -48,13 +48,13 @@ def update(self, trans: ProvidesUserContext, payload: ItemTagsPayload) -> None:
         """Apply a new set of tags to an item; previous tags are deleted."""
         sa_session = trans.sa_session
         user = trans.user
-        tag_handler = GalaxyTagHandlerSession(sa_session)
+        tag_handler = GalaxyTagHandlerSession(sa_session, trans.galaxy_session)
         new_tags: Optional[str] = None
         if payload.item_tags and len(payload.item_tags.__root__) > 0:
             new_tags = ",".join(payload.item_tags.__root__)
         item = self._get_item(tag_handler, payload)
-        tag_handler.delete_item_tags(user, item)
-        tag_handler.apply_item_tags(user, item, new_tags)
+        tag_handler.delete_item_tags(user, item, galaxy_session=trans.galaxy_session)
+        tag_handler.apply_item_tags(user, item, new_tags, galaxy_session=trans.galaxy_session)
         with transaction(sa_session):
             sa_session.commit()
 
diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py
index 077165451be7..305b2a10e196 100644
--- a/lib/galaxy/managers/workflows.py
+++ b/lib/galaxy/managers/workflows.py
@@ -639,7 +639,9 @@ def build_workflow_from_raw_description(
             annotation = sanitize_html(data["annotation"])
             self.add_item_annotation(trans.sa_session, stored.user, stored, annotation)
         workflow_tags = data.get("tags", [])
-        trans.app.tag_handler.set_tags_from_list(user=trans.user, item=stored, new_tags_list=workflow_tags)
+        trans.app.tag_handler.set_tags_from_list(
+            user=trans.user, item=stored, new_tags_list=workflow_tags, galaxy_session=trans.galaxy_session
+        )
 
         # Persist
         trans.sa_session.add(stored)
diff --git a/lib/galaxy/model/store/__init__.py b/lib/galaxy/model/store/__init__.py
index 7363e4154479..e6195d07d0f5 100644
--- a/lib/galaxy/model/store/__init__.py
+++ b/lib/galaxy/model/store/__init__.py
@@ -2936,7 +2936,7 @@ def source_to_import_store(
     else:
         source_uri: str = str(source)
         delete = False
-        tag_handler = app.tag_handler.create_tag_handler_session()
+        tag_handler = app.tag_handler.create_tag_handler_session(galaxy_session=None)
         if source_uri.startswith("file://"):
             source_uri = source_uri[len("file://") :]
         if "://" in source_uri:
diff --git a/lib/galaxy/model/store/discover.py b/lib/galaxy/model/store/discover.py
index 19a4c1e84194..3cdf14bfc6bd 100644
--- a/lib/galaxy/model/store/discover.py
+++ b/lib/galaxy/model/store/discover.py
@@ -60,6 +60,9 @@ class ModelPersistenceContext(metaclass=abc.ABCMeta):
     max_discovered_files = float("inf")
     discovered_file_count: int
 
+    def get_job(self) -> Optional[galaxy.model.Job]:
+        return getattr(self, "job", None)
+
     def create_dataset(
         self,
         ext,
@@ -161,8 +164,10 @@ def create_dataset(
             primary_data.created_from_basename = created_from_basename
 
         if tag_list:
-            job = getattr(self, "job", None)
-            self.tag_handler.add_tags_from_list(job and job.user, primary_data, tag_list, flush=False)
+            job = self.get_job()
+            self.tag_handler.add_tags_from_list(
+                job and job.user, primary_data, tag_list, flush=False, galaxy_session=job and job.galaxy_session
+            )
 
         # If match specified a name use otherwise generate one from
         # designation.
@@ -392,9 +397,12 @@ def _populate_elements(self, chunk, name, root_collection_builder, metadata_sour
         self.set_datasets_metadata(datasets=element_datasets["datasets"])
 
     def add_tags_to_datasets(self, datasets, tag_lists):
+        job = self.get_job()
         if any(tag_lists):
             for dataset, tags in zip(datasets, tag_lists):
-                self.tag_handler.add_tags_from_list(self.user, dataset, tags, flush=False)
+                self.tag_handler.add_tags_from_list(
+                    self.user, dataset, tags, flush=False, galaxy_session=job and job.galaxy_session
+                )
 
     def update_object_store_with_datasets(self, datasets, paths, extra_files, output_name):
         for dataset, path, extra_file in zip(datasets, paths, extra_files):
@@ -573,7 +581,7 @@ def __init__(self, object_store, export_store: "ModelExportStore", working_direc
 
     @property
     def tag_handler(self):
-        return GalaxySessionlessTagHandler(self.sa_session)
+        return GalaxySessionlessTagHandler(self.sa_session, galaxy_session=None)
 
     @property
     def sa_session(self):
diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py
index ab3db277e97a..d5b3609a0eeb 100644
--- a/lib/galaxy/model/tags.py
+++ b/lib/galaxy/model/tags.py
@@ -5,6 +5,7 @@
     List,
     Optional,
     Tuple,
+    TYPE_CHECKING,
 )
 
 from sqlalchemy.exc import IntegrityError
@@ -21,6 +22,13 @@
     unicodify,
 )
 
+if TYPE_CHECKING:
+    from galaxy.model import (
+        GalaxySession,
+        Tag,
+        User,
+    )
+
 log = logging.getLogger(__name__)
 
 
@@ -37,7 +45,7 @@ class TagHandler:
     Manages CRUD operations related to tagging objects.
     """
 
-    def __init__(self, sa_session: galaxy_scoped_session) -> None:
+    def __init__(self, sa_session: galaxy_scoped_session, galaxy_session=None) -> None:
         self.sa_session = sa_session
         # Minimum tag length.
         self.min_tag_len = 1
@@ -51,30 +59,39 @@ def __init__(self, sa_session: galaxy_scoped_session) -> None:
         self.key_value_separators = "=:"
         # Initialize with known classes - add to this in subclasses.
         self.item_tag_assoc_info: Dict[str, ItemTagAssocInfo] = {}
+        # Can't include type annotation in signature, because lagom will attempt to look up
+        # GalaxySession, but can't find it due to the circular import
+        self.galaxy_session: Optional["GalaxySession"] = galaxy_session
 
-    def create_tag_handler_session(self):
+    def create_tag_handler_session(self, galaxy_session: Optional["GalaxySession"]):
         # Creates a transient tag handler that avoids repeated flushes
-        return GalaxyTagHandlerSession(self.sa_session)
+        return GalaxyTagHandlerSession(self.sa_session, galaxy_session=galaxy_session)
 
-    def add_tags_from_list(self, user, item, new_tags_list, flush=True):
+    def add_tags_from_list(
+        self, user, item, new_tags_list, flush=True, galaxy_session: Optional["GalaxySession"] = None
+    ):
         new_tags_set = set(new_tags_list)
         if item.tags:
             new_tags_set.update(self.get_tags_str(item.tags).split(","))
-        return self.set_tags_from_list(user, item, new_tags_set, flush=flush)
+        return self.set_tags_from_list(user, item, new_tags_set, flush=flush, galaxy_session=galaxy_session)
 
-    def remove_tags_from_list(self, user, item, tag_to_remove_list, flush=True):
+    def remove_tags_from_list(
+        self, user, item, tag_to_remove_list, flush=True, galaxy_session: Optional["GalaxySession"] = None
+    ):
         tag_to_remove_set = set(tag_to_remove_list)
         tags_set = {_.strip() for _ in self.get_tags_str(item.tags).split(",")}
         if item.tags:
             tags_set -= tag_to_remove_set
-        return self.set_tags_from_list(user, item, tags_set, flush=flush)
+        return self.set_tags_from_list(user, item, tags_set, flush=flush, galaxy_session=galaxy_session)
 
-    def set_tags_from_list(self, user, item, new_tags_list, flush=True):
+    def set_tags_from_list(
+        self, user, item, new_tags_list, flush=True, galaxy_session: Optional["GalaxySession"] = None
+    ):
         # precondition: item is already security checked against user
         # precondition: incoming tags is a list of sanitized/formatted strings
-        self.delete_item_tags(user, item)
+        self.delete_item_tags(user, item, galaxy_session=galaxy_session)
         new_tags_str = ",".join(new_tags_list)
-        self.apply_item_tags(user, item, unicodify(new_tags_str, "utf-8"), flush=flush)
+        self.apply_item_tags(user, item, unicodify(new_tags_str, "utf-8"), flush=flush, galaxy_session=galaxy_session)
         if flush:
             with transaction(self.sa_session):
                 self.sa_session.commit()
@@ -130,9 +147,9 @@ def get_tool_tags(self):
             tags.append(self.get_tag_by_id(tag_id))
         return tags
 
-    def remove_item_tag(self, user, item, tag_name):
+    def remove_item_tag(self, user: "User", item, tag_name: str, galaxy_session: Optional["GalaxySession"] = None):
         """Remove a tag from an item."""
-        self._ensure_user_owns_item(user, item)
+        self._ensure_user_owns_item(user, item, galaxy_session)
         # Get item tag association.
         item_tag_assoc = self._get_item_tag_assoc(user, item, tag_name)
         # Remove association.
@@ -143,9 +160,9 @@ def remove_item_tag(self, user, item, tag_name):
             return True
         return False
 
-    def delete_item_tags(self, user, item):
+    def delete_item_tags(self, user: Optional["User"], item, galaxy_session: Optional["GalaxySession"] = None):
         """Delete tags from an item."""
-        self._ensure_user_owns_item(user, item)
+        self._ensure_user_owns_item(user, item, galaxy_session)
         # Delete item-tag associations.
         for tag in item.tags:
             if tag.id:
@@ -154,7 +171,7 @@ def delete_item_tags(self, user, item):
         # Delete tags from item.
         del item.tags[:]
 
-    def _ensure_user_owns_item(self, user, item):
+    def _ensure_user_owns_item(self, user: Optional["User"], item, galaxy_session: Optional["GalaxySession"] = None):
         """Raises exception if user does not own item.
         Notice that even admin users cannot directly modify tags on items they do not own.
         To modify tags on items they don't own, admin users must impersonate the item's owner.
@@ -163,8 +180,19 @@ def _ensure_user_owns_item(self, user, item):
             # Item is not persisted, likely it is being copied from an existing, so no need
             # to check ownership at this point.
             return
-        history = getattr(item, "history", None)
-        is_owner = getattr(item, "user", None) == user or getattr(history, "user", None) == user
+        # Prefer checking ownership via history (or associated history).
+        # When checking multiple items in batch this should save a few lazy-loads
+        is_owner = False
+        history = item if isinstance(item, galaxy.model.History) else getattr(item, "history", None)
+        if not user:
+            if self.galaxy_session and history:
+                # anon users can only tag histories and history items,
+                # and should only have a single history
+                if history == self.galaxy_session.current_history:
+                    return
+            raise ItemOwnershipException("User does not own item.")
+        user_id = history.user_id if history else getattr(item, "user_id", None)
+        is_owner = user_id == user.id
         if not is_owner:
             raise ItemOwnershipException("User does not own item.")
 
@@ -183,8 +211,16 @@ def item_has_tag(self, user, item, tag):
             return True
         return False
 
-    def apply_item_tag(self, user, item, name, value=None, flush=True):
-        self._ensure_user_owns_item(user, item)
+    def apply_item_tag(
+        self,
+        user: Optional["User"],
+        item,
+        name,
+        value=None,
+        flush=True,
+        galaxy_session: Optional["GalaxySession"] = None,
+    ):
+        self._ensure_user_owns_item(user, item, galaxy_session=galaxy_session)
         # Use lowercase name for searching/creating tag.
         if name is None:
             return
@@ -219,9 +255,16 @@ def apply_item_tag(self, user, item, name, value=None, flush=True):
                 self.sa_session.commit()
         return item_tag_assoc
 
-    def apply_item_tags(self, user, item, tags_str, flush=True):
+    def apply_item_tags(
+        self,
+        user: Optional["User"],
+        item,
+        tags_str: Optional[str],
+        flush=True,
+        galaxy_session: Optional["GalaxySession"] = None,
+    ):
         """Apply tags to an item."""
-        self._ensure_user_owns_item(user, item)
+        self._ensure_user_owns_item(user, item, galaxy_session=galaxy_session)
         # Parse tags.
         parsed_tags = self.parse_tags(tags_str)
         # Apply each tag.
@@ -402,8 +445,8 @@ def _get_name_value_pair(self, tag_str) -> List[Optional[str]]:
 class GalaxyTagHandler(TagHandler):
     _item_tag_assoc_info: Dict[str, ItemTagAssocInfo] = {}
 
-    def __init__(self, sa_session: galaxy_scoped_session):
-        TagHandler.__init__(self, sa_session)
+    def __init__(self, sa_session: galaxy_scoped_session, galaxy_session=None):
+        TagHandler.__init__(self, sa_session, galaxy_session=galaxy_session)
         if not GalaxyTagHandler._item_tag_assoc_info:
             GalaxyTagHandler.init_tag_associations()
         self.item_tag_assoc_info = GalaxyTagHandler._item_tag_assoc_info
@@ -449,9 +492,9 @@ def init_tag_associations(cls):
 class GalaxyTagHandlerSession(GalaxyTagHandler):
     """Like GalaxyTagHandler, but avoids one flush per created tag."""
 
-    def __init__(self, sa_session):
-        super().__init__(sa_session)
-        self.created_tags = {}
+    def __init__(self, sa_session, galaxy_session: Optional["GalaxySession"]):
+        super().__init__(sa_session, galaxy_session)
+        self.created_tags: Dict[str, "Tag"] = {}
 
     def _get_tag(self, tag_name):
         """Get tag from cache or database."""
@@ -466,6 +509,10 @@ def _create_tag_instance(self, tag_name):
 
 
 class GalaxySessionlessTagHandler(GalaxyTagHandlerSession):
+    def _ensure_user_owns_item(self, user: Optional["User"], item, galaxy_session: Optional["GalaxySession"] = None):
+        # In sessionless mode we don't need to check ownership, we're only exporting
+        pass
+
     def _get_tag(self, tag_name):
         """Get tag from cache or database."""
         # Short-circuit session access
diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py
index 943c62c1f3e5..d6e7836153a6 100644
--- a/lib/galaxy/tools/__init__.py
+++ b/lib/galaxy/tools/__init__.py
@@ -3724,7 +3724,9 @@ def produce_outputs(self, trans, out_data, output_collections, incoming, history
         def copy_dataset(dataset, tags):
             copied_dataset = dataset.copy(copy_tags=dataset.tags, flush=False)
             if tags is not None:
-                tag_handler.set_tags_from_list(trans.get_user(), copied_dataset, tags, flush=False)
+                tag_handler.set_tags_from_list(
+                    trans.get_user(), copied_dataset, tags, flush=False, galaxy_session=trans.galaxy_session
+                )
             copied_dataset.history_id = history.id
             copied_datasets.append(copied_dataset)
             return copied_dataset
@@ -3772,7 +3774,11 @@ def add_copied_value_to_new_elements(new_tags_dict, dce):
                             old_tags = old_tags - set(new_tags)
                         new_tags = old_tags
                     tag_handler.add_tags_from_list(
-                        user=history.user, item=copied_value, new_tags_list=new_tags, flush=False
+                        user=history.user,
+                        item=copied_value,
+                        new_tags_list=new_tags,
+                        flush=False,
+                        galaxy_session=history.galaxy_sessions,
                     )
             else:
                 # We have a collection, and we copy the elements so that we don't manipulate the original tags
diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py
index 404f623edf76..1233502600b5 100644
--- a/lib/galaxy/tools/actions/__init__.py
+++ b/lib/galaxy/tools/actions/__init__.py
@@ -977,7 +977,7 @@ def __init__(
         hdca_tags,
     ):
         self.trans = trans
-        self.tag_handler = trans.app.tag_handler.create_tag_handler_session()
+        self.tag_handler = trans.tag_handler
         self.history = history
         self.tool = tool
         self.tool_action = tool_action
diff --git a/lib/galaxy/tools/actions/model_operations.py b/lib/galaxy/tools/actions/model_operations.py
index f963321283cc..6b6dac0af782 100644
--- a/lib/galaxy/tools/actions/model_operations.py
+++ b/lib/galaxy/tools/actions/model_operations.py
@@ -1,4 +1,5 @@
 import logging
+from typing import TYPE_CHECKING
 
 from galaxy.tools.actions import (
     DefaultToolAction,
@@ -6,6 +7,9 @@
     ToolExecutionCache,
 )
 
+if TYPE_CHECKING:
+    from galaxy.managers.context import ProvidesUserContext
+
 log = logging.getLogger(__name__)
 
 
@@ -103,8 +107,10 @@ def execute(
         log.info(f"Calling produce_outputs, tool is {tool}")
         return job, out_data, history
 
-    def _produce_outputs(self, trans, tool, out_data, output_collections, incoming, history, tags, hdca_tags):
-        tag_handler = trans.app.tag_handler.create_tag_handler_session()
+    def _produce_outputs(
+        self, trans: "ProvidesUserContext", tool, out_data, output_collections, incoming, history, tags, hdca_tags
+    ):
+        tag_handler = trans.tag_handler
         tool.produce_outputs(
             trans,
             out_data,
diff --git a/lib/galaxy/tools/actions/upload_common.py b/lib/galaxy/tools/actions/upload_common.py
index d0859f7ba72b..2f1c0938621e 100644
--- a/lib/galaxy/tools/actions/upload_common.py
+++ b/lib/galaxy/tools/actions/upload_common.py
@@ -22,6 +22,7 @@
     stream_to_file,
     validate_non_local,
 )
+from galaxy.managers.context import ProvidesUserContext
 from galaxy.model import (
     FormDefinition,
     LibraryDataset,
@@ -261,8 +262,10 @@ def __new_library_upload(trans, cntrller, uploaded_dataset, library_bunch, tag_h
     return ldda
 
 
-def new_upload(trans, cntrller, uploaded_dataset, library_bunch=None, history=None, state=None, tag_list=None):
-    tag_handler = tags.GalaxyTagHandlerSession(trans.sa_session)
+def new_upload(
+    trans: ProvidesUserContext, cntrller, uploaded_dataset, library_bunch=None, history=None, state=None, tag_list=None
+):
+    tag_handler = trans.tag_handler
     if library_bunch:
         upload_target_dataset_instance = __new_library_upload(
             trans, cntrller, uploaded_dataset, library_bunch, tag_handler, state
diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py
index c0be8794f9e9..b53bc48f51c8 100644
--- a/lib/galaxy/tools/evaluation.py
+++ b/lib/galaxy/tools/evaluation.py
@@ -268,7 +268,9 @@ def replace_deferred(input, value, context, prefixed_name=None, **kwargs):
         visit_input_values(self.tool.inputs, incoming, replace_deferred)
 
     def _validate_incoming(self, incoming: dict):
-        request_context = WorkRequestContext(app=self.app, user=self._user, history=self._history)
+        request_context = WorkRequestContext(
+            app=self.app, user=self._user, history=self._history, galaxy_session=self.job.galaxy_session
+        )
 
         def validate_inputs(input, value, context, **kwargs):
             value = input.from_json(value, request_context, context)
diff --git a/lib/galaxy/tools/imp_exp/__init__.py b/lib/galaxy/tools/imp_exp/__init__.py
index 589dfbcda2bc..0634ab913e7a 100644
--- a/lib/galaxy/tools/imp_exp/__init__.py
+++ b/lib/galaxy/tools/imp_exp/__init__.py
@@ -64,7 +64,10 @@ def cleanup_after_job(self):
                     "history import archive directory",
                 )
             model_store = store.get_import_model_store_for_directory(
-                archive_dir, app=self.app, user=user, tag_handler=self.app.tag_handler.create_tag_handler_session()
+                archive_dir,
+                app=self.app,
+                user=user,
+                tag_handler=self.app.tag_handler.create_tag_handler_session(jiha.job.galaxy_session),
             )
             job = jiha.job
             with model_store.target_history(default_history=job.history) as new_history:
diff --git a/lib/galaxy/webapps/base/controller.py b/lib/galaxy/webapps/base/controller.py
index 1bec1ae6f993..4112c158a47b 100644
--- a/lib/galaxy/webapps/base/controller.py
+++ b/lib/galaxy/webapps/base/controller.py
@@ -1389,9 +1389,6 @@ def get_item(self, trans, id):
 
 
 class UsesTagsMixin(SharableItemSecurityMixin):
-    def get_tag_handler(self, trans) -> tags.GalaxyTagHandler:
-        return trans.app.tag_handler
-
     def _get_user_tags(self, trans, item_class_name, id):
         user = trans.user
         tagged_item = self._get_tagged_item(trans, item_class_name, id)
@@ -1407,7 +1404,9 @@ def _remove_items_tag(self, trans, item_class_name, id, tag_name):
         """Remove a tag from an item."""
         user = trans.user
         tagged_item = self._get_tagged_item(trans, item_class_name, id)
-        deleted = tagged_item and self.get_tag_handler(trans).remove_item_tag(trans, user, tagged_item, tag_name)
+        deleted = tagged_item and trans.tag_handler.remove_item_tag(
+            user, tagged_item, tag_name, galaxy_session=trans.galaxy_session
+        )
         with transaction(trans.sa_session):
             trans.sa_session.commit()
         return deleted
@@ -1415,7 +1414,7 @@ def _remove_items_tag(self, trans, item_class_name, id, tag_name):
     def _apply_item_tag(self, trans, item_class_name, id, tag_name, tag_value=None):
         user = trans.user
         tagged_item = self._get_tagged_item(trans, item_class_name, id)
-        tag_assoc = self.get_tag_handler(trans).apply_item_tag(user, tagged_item, tag_name, tag_value)
+        tag_assoc = trans.tag_handler.apply_item_tag(user, tagged_item, tag_name, tag_value)
         with transaction(trans.sa_session):
             trans.sa_session.commit()
         return tag_assoc
@@ -1424,11 +1423,10 @@ def _get_item_tag_assoc(self, trans, item_class_name, id, tag_name):
         user = trans.user
         tagged_item = self._get_tagged_item(trans, item_class_name, id)
         log.debug(f"In get_item_tag_assoc with tagged_item {tagged_item}")
-        return self.get_tag_handler(trans)._get_item_tag_assoc(user, tagged_item, tag_name)
+        return trans.tag_handler._get_item_tag_assoc(user, tagged_item, tag_name)
 
     def set_tags_from_list(self, trans, item, new_tags_list, user=None):
-        tag_handler = tags.GalaxyTagHandler(trans.app.model.context)
-        return tag_handler.set_tags_from_list(user, item, new_tags_list)
+        return trans.tag_handler.set_tags_from_list(user, item, new_tags_list, galaxy_session=trans.galaxy_session)
 
     def get_user_tags_used(self, trans, user=None):
         """
@@ -1444,7 +1442,7 @@ def get_user_tags_used(self, trans, user=None):
             return []
 
         # get all the taggable model TagAssociations
-        tag_models = [v.tag_assoc_class for v in trans.app.tag_handler.item_tag_assoc_info.values()]
+        tag_models = [v.tag_assoc_class for v in trans.tag_handler.item_tag_assoc_info.values()]
         # create a union of subqueries for each for this user - getting only the tname and user_value
         all_tags_query = None
         for tag_model in tag_models:
diff --git a/lib/galaxy/webapps/galaxy/api/jobs.py b/lib/galaxy/webapps/galaxy/api/jobs.py
index 72a565b05ccd..1c5a3598b004 100644
--- a/lib/galaxy/webapps/galaxy/api/jobs.py
+++ b/lib/galaxy/webapps/galaxy/api/jobs.py
@@ -487,7 +487,9 @@ def search(self, trans: ProvidesHistoryContext, payload: dict, **kwd):
         for k, v in payload.items():
             if k.startswith("files_") or k.startswith("__files_"):
                 inputs[k] = v
-        request_context = WorkRequestContext(app=trans.app, user=trans.user, history=trans.history)
+        request_context = WorkRequestContext(
+            app=trans.app, user=trans.user, history=trans.history, galaxy_session=trans.galaxy_session
+        )
         all_params, all_errors, _, _ = tool.expand_incoming(
             trans=trans, incoming=inputs, request_context=request_context
         )
diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py
index 8c99c6cb983e..7c5637e60bb8 100644
--- a/lib/galaxy/webapps/galaxy/api/workflows.py
+++ b/lib/galaxy/webapps/galaxy/api/workflows.py
@@ -525,7 +525,10 @@ def update(self, trans: GalaxyWebTransaction, id, payload, **kwds):
             # set tags
             if "tags" in workflow_dict:
                 trans.app.tag_handler.set_tags_from_list(
-                    user=trans.user, item=stored_workflow, new_tags_list=workflow_dict["tags"]
+                    user=trans.user,
+                    item=stored_workflow,
+                    new_tags_list=workflow_dict["tags"],
+                    galaxy_session=trans.galaxy_session,
                 )
 
             if require_flush:
diff --git a/lib/galaxy/webapps/galaxy/controllers/tag.py b/lib/galaxy/webapps/galaxy/controllers/tag.py
index 32a8fd4c4ffa..5c690c5347c1 100644
--- a/lib/galaxy/webapps/galaxy/controllers/tag.py
+++ b/lib/galaxy/webapps/galaxy/controllers/tag.py
@@ -30,7 +30,7 @@ def add_tag_async(self, trans, item_id=None, item_class=None, new_tag=None, cont
         # Apply tag.
         item = self._get_item(trans, item_class, trans.security.decode_id(item_id))
         user = trans.user
-        self.get_tag_handler(trans).apply_item_tags(user, item, new_tag)
+        trans.tag_handler.apply_item_tags(user, item, new_tag)
         with transaction(trans.sa_session):
             trans.sa_session.commit()
         # Log.
@@ -46,7 +46,7 @@ def remove_tag_async(self, trans, item_id=None, item_class=None, tag_name=None,
         # Remove tag.
         item = self._get_item(trans, item_class, trans.security.decode_id(item_id))
         user = trans.user
-        self.get_tag_handler(trans).remove_item_tag(user, item, tag_name)
+        trans.tag_handler.remove_item_tag(user, item, tag_name, galaxy_session=trans.galaxy_session)
         with transaction(trans.sa_session):
             trans.sa_session.commit()
         # Log.
@@ -82,7 +82,7 @@ def _get_tag_autocomplete_names(self, trans, q, limit, timestamp, user=None, ite
             raise RuntimeError("Both item and item_class cannot be None")
         elif item is not None:
             item_class = item.__class__
-        item_tag_assoc_class = self.get_tag_handler(trans).get_tag_assoc_class(item_class)
+        item_tag_assoc_class = trans.tag_handler.get_tag_assoc_class(item_class)
         # Build select statement.
         cols_to_select = [item_tag_assoc_class.table.c.tag_id, func.count("*")]
         from_obj = item_tag_assoc_class.table.join(item_class.table).join(trans.app.model.Tag.table)
@@ -104,9 +104,9 @@ def _get_tag_autocomplete_names(self, trans, q, limit, timestamp, user=None, ite
         # Create and return autocomplete data.
         ac_data = "#Header|Your Tags\n"
         for row in result_set:
-            tag = self.get_tag_handler(trans).get_tag_by_id(row[0])
+            tag = trans.tag_handler.get_tag_by_id(row[0])
             # Exclude tags that are already applied to the item.
-            if (item is not None) and (self.get_tag_handler(trans).item_has_tag(trans.user, item, tag)):
+            if (item is not None) and (trans.tag_handler.item_has_tag(trans.user, item, tag)):
                 continue
             # Add tag to autocomplete data. Use the most frequent name that user
             # has employed for the tag.
@@ -122,7 +122,7 @@ def _get_tag_autocomplete_values(self, trans, q, limit, timestamp, user=None, it
         tag_name_and_value = q.split(":")
         tag_name = tag_name_and_value[0]
         tag_value = tag_name_and_value[1]
-        tag = self.get_tag_handler(trans).get_tag_by_name(tag_name)
+        tag = trans.tag_handler.get_tag_by_name(tag_name)
         # Don't autocomplete if tag doesn't exist.
         if tag is None:
             return ""
@@ -131,7 +131,7 @@ def _get_tag_autocomplete_values(self, trans, q, limit, timestamp, user=None, it
             raise RuntimeError("Both item and item_class cannot be None")
         elif item is not None:
             item_class = item.__class__
-        item_tag_assoc_class = self.get_tag_handler(trans).get_tag_assoc_class(item_class)
+        item_tag_assoc_class = trans.tag_handler.get_tag_assoc_class(item_class)
         # Build select statement.
         cols_to_select = [item_tag_assoc_class.table.c.value, func.count("*")]
         from_obj = item_tag_assoc_class.table.join(item_class.table).join(trans.app.model.Tag.table)
@@ -183,6 +183,6 @@ def _get_item(self, trans, item_class_name, id):
         """
         Get an item based on type and id.
         """
-        item_class = self.get_tag_handler(trans).item_tag_assoc_info[item_class_name].item_class
+        item_class = trans.tag_handler.item_tag_assoc_info[item_class_name].item_class
         item = trans.sa_session.query(item_class).filter(item_class.id == id)[0]
         return item
diff --git a/lib/galaxy/work/context.py b/lib/galaxy/work/context.py
index f78db6bf28cd..310779e504e3 100644
--- a/lib/galaxy/work/context.py
+++ b/lib/galaxy/work/context.py
@@ -1,8 +1,15 @@
 import abc
-from typing import Optional
+from typing import (
+    List,
+    Optional,
+)
 
 from galaxy.managers.context import ProvidesHistoryContext
-from galaxy.model import History
+from galaxy.model import (
+    GalaxySession,
+    History,
+    Role,
+)
 from galaxy.model.base import transaction
 
 
@@ -19,13 +26,22 @@ class WorkRequestContext(ProvidesHistoryContext):
     objects.
     """
 
-    def __init__(self, app, user=None, history=None, workflow_building_mode=False, url_builder=None):
+    def __init__(
+        self,
+        app,
+        user=None,
+        history=None,
+        workflow_building_mode=False,
+        url_builder=None,
+        galaxy_session: Optional[GalaxySession] = None,
+    ):
         self._app = app
         self.__user = user
-        self.__user_current_roles = None
+        self.__user_current_roles: Optional[List[Role]] = None
         self.__history = history
         self._url_builder = url_builder
         self.workflow_building_mode = workflow_building_mode
+        self.galaxy_session = galaxy_session
 
     @property
     def app(self):
@@ -88,10 +104,9 @@ def get_content_type(self):
 
 
 class SessionRequestContext(WorkRequestContext):
-    """Like WorkRequestContext, but provides access to galaxy session and request."""
+    """Like WorkRequestContext, but provides access to request."""
 
     def __init__(self, **kwargs):
-        self.galaxy_session = kwargs.pop("galaxy_session", None)
         self.__request: GalaxyAbstractRequest = kwargs.pop("request")
         self.__response: GalaxyAbstractResponse = kwargs.pop("response")
         super().__init__(**kwargs)
@@ -112,7 +127,7 @@ def get_galaxy_session(self):
         return self.galaxy_session
 
     def set_history(self, history):
-        if history and not history.deleted:
+        if history and not history.deleted and self.galaxy_session:
             self.galaxy_session.current_history = history
         self.sa_session.add(self.galaxy_session)
         with transaction(self.sa_session):
@@ -134,4 +149,5 @@ def proxy_work_context_for_history(
         history=history or trans.history,
         url_builder=trans.url_builder,
         workflow_building_mode=workflow_building_mode,
+        galaxy_session=trans.galaxy_session,
     )

From 7329575bf482187d815d191f73ae409bcacead8f Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Fri, 28 Jul 2023 18:35:22 +0200
Subject: [PATCH 11/41] Unit test fixes

---
 lib/galaxy/app_unittest_utils/galaxy_mock.py  | 4 ++++
 test/unit/app/managers/base.py                | 8 ++++++--
 test/unit/app/managers/test_HDAManager.py     | 2 +-
 test/unit/app/managers/test_HistoryManager.py | 2 +-
 test/unit/app/managers/test_UserManager.py    | 4 ++--
 test/unit/app/tools/test_actions.py           | 4 ++++
 test/unit/app/tools/test_execution.py         | 1 +
 test/unit/data/model/test_model_discovery.py  | 6 +++++-
 8 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/lib/galaxy/app_unittest_utils/galaxy_mock.py b/lib/galaxy/app_unittest_utils/galaxy_mock.py
index 049cad5908ca..fe7f5ba04f55 100644
--- a/lib/galaxy/app_unittest_utils/galaxy_mock.py
+++ b/lib/galaxy/app_unittest_utils/galaxy_mock.py
@@ -296,6 +296,10 @@ def __init__(self, app=None, user=None, history=None, **kwargs):
         self.request: Any = Bunch(headers={}, body=None)
         self.response: Any = Bunch(headers={}, set_content_type=lambda i: None)
 
+    @property
+    def tag_handler(self):
+        return self.app.tag_handler
+
     def check_csrf_token(self, payload):
         pass
 
diff --git a/test/unit/app/managers/base.py b/test/unit/app/managers/base.py
index 5c0aa7b414d9..5251fc06730a 100644
--- a/test/unit/app/managers/base.py
+++ b/test/unit/app/managers/base.py
@@ -2,12 +2,14 @@
 """
 
 import json
+from typing import cast
 
 import sqlalchemy
 
 from galaxy.app_unittest_utils import galaxy_mock
 from galaxy.managers.users import UserManager
 from galaxy.util.unittest import TestCase
+from galaxy.work.context import SessionRequestContext
 
 # =============================================================================
 admin_email = "admin@admin.admin"
@@ -33,7 +35,9 @@ def setUp(self):
 
     def set_up_mocks(self):
         admin_users_list = [u for u in admin_users.split(",") if u]
-        self.trans = galaxy_mock.MockTrans(admin_users=admin_users, admin_users_list=admin_users_list)
+        self.trans = cast(
+            SessionRequestContext, galaxy_mock.MockTrans(admin_users=admin_users, admin_users_list=admin_users_list)
+        )
         self.app = self.trans.app
 
     def set_up_managers(self):
@@ -99,7 +103,7 @@ def assertIsJsonifyable(self, item):
 
 
 class CreatesCollectionsMixin:
-    trans: galaxy_mock.MockTrans
+    trans: SessionRequestContext
 
     def build_element_identifiers(self, elements):
         identifier_list = []
diff --git a/test/unit/app/managers/test_HDAManager.py b/test/unit/app/managers/test_HDAManager.py
index 466093806390..548711bc5eb8 100644
--- a/test/unit/app/managers/test_HDAManager.py
+++ b/test/unit/app/managers/test_HDAManager.py
@@ -118,7 +118,7 @@ def test_copy_from_hda(self):
         self.log("tags should be copied between HDAs")
         tagged = self.hda_manager.create(history=history1, dataset=self.dataset_manager.create())
         tags_to_set = ["tag-one", "tag-two"]
-        self.hda_manager.tag_handler.add_tags_from_list(owner, tagged, tags_to_set)
+        self.app.tag_handler.add_tags_from_list(owner, tagged, tags_to_set)
         hda2 = self.hda_manager.copy(tagged, history=history1)
         assert tagged.make_tag_string_list() == hda2.make_tag_string_list()
 
diff --git a/test/unit/app/managers/test_HistoryManager.py b/test/unit/app/managers/test_HistoryManager.py
index 0cc5376472a2..67d1665d4802 100644
--- a/test/unit/app/managers/test_HistoryManager.py
+++ b/test/unit/app/managers/test_HistoryManager.py
@@ -92,7 +92,7 @@ def test_copy(self):
         hda = self.add_hda_to_history(history1, name="wat")
         hda_tags = ["tag-one", "tag-two"]
         hda_annotation = "annotation"
-        self.hda_manager.tag_handler.set_tags_from_list(user=user2, item=hda, new_tags_list=hda_tags)
+        self.app.tag_handler.set_tags_from_list(user=user2, item=hda, new_tags_list=hda_tags)
         self.hda_manager.annotate(hda, hda_annotation, user=user2)
 
         history2 = self.history_manager.copy(history1, user=user3)
diff --git a/test/unit/app/managers/test_UserManager.py b/test/unit/app/managers/test_UserManager.py
index 00152b51ca97..abefe20ee828 100644
--- a/test/unit/app/managers/test_UserManager.py
+++ b/test/unit/app/managers/test_UserManager.py
@@ -195,7 +195,7 @@ def testable_url_for(*a, **k):
     def test_activation_email(self):
         self.log("should produce the activation email")
         self.user_manager.create(email="user@nopassword.com", username="nopassword")
-        self.trans.request.host = "request.host"
+        setattr(self.trans.request, "host", "request.host")
 
         def validate_send_email(frm, to, subject, body, config, html=None):
             assert frm == "email_from"
@@ -219,7 +219,7 @@ def validate_send_email(frm, to, subject, body, config, html=None):
     def test_reset_email(self):
         self.log("should produce the password reset email")
         self.user_manager.create(email="user@nopassword.com", username="nopassword")
-        self.trans.request.host = "request.host"
+        setattr(self.trans.request, "host", "request.host")
 
         def validate_send_email(frm, to, subject, body, config, html=None):
             assert frm == "email_from"
diff --git a/test/unit/app/tools/test_actions.py b/test/unit/app/tools/test_actions.py
index e5f6b1990286..85cc8ca63702 100644
--- a/test/unit/app/tools/test_actions.py
+++ b/test/unit/app/tools/test_actions.py
@@ -259,6 +259,10 @@ def __init__(self, app, history, user=None):
         self._user_is_active = True
         self.user_is_admin = False
 
+    @property
+    def tag_handler(self):
+        return self.app.tag_handler
+
     def get_user_is_active(self):
         # NOTE: the real user_is_active also checks whether activation is enabled in the config
         return self._user_is_active
diff --git a/test/unit/app/tools/test_execution.py b/test/unit/app/tools/test_execution.py
index 97b4c4920ee4..881562d3b839 100644
--- a/test/unit/app/tools/test_execution.py
+++ b/test/unit/app/tools/test_execution.py
@@ -205,6 +205,7 @@ def __init__(self, app, history):
         self.webapp = Bunch(name="galaxy")
         self.sa_session = self.app.model.context
         self.url_builder = None
+        self.galaxy_session = None
 
     def get_history(self, **kwargs):
         return self.history
diff --git a/test/unit/data/model/test_model_discovery.py b/test/unit/data/model/test_model_discovery.py
index 751fb41ec813..838896bffbbd 100644
--- a/test/unit/data/model/test_model_discovery.py
+++ b/test/unit/data/model/test_model_discovery.py
@@ -250,7 +250,11 @@ def _import_directory_to_history(app, target, work_directory):
 
     import_options = store.ImportOptions(allow_dataset_object_edit=True)
     import_model_store = store.get_import_model_store_for_directory(
-        target, app=app, user=u, import_options=import_options, tag_handler=app.tag_handler.create_tag_handler_session()
+        target,
+        app=app,
+        user=u,
+        import_options=import_options,
+        tag_handler=app.tag_handler.create_tag_handler_session(None),
     )
     with import_model_store.target_history(default_history=import_history):
         import_model_store.perform_import(import_history)

From d6f7351e85e1b0e348dd836808cbda18f9faea62 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Fri, 28 Jul 2023 18:57:13 +0200
Subject: [PATCH 12/41] Rely on trans for galaxy_session

---
 lib/galaxy/managers/collections.py           | 13 +++------
 lib/galaxy/managers/library_datasets.py      |  6 ++---
 lib/galaxy/managers/tags.py                  | 12 ++++-----
 lib/galaxy/managers/workflows.py             |  4 +--
 lib/galaxy/model/tags.py                     | 28 +++++++++++---------
 lib/galaxy/tools/__init__.py                 |  6 +++--
 lib/galaxy/webapps/base/controller.py        |  4 +--
 lib/galaxy/webapps/galaxy/api/jobs.py        |  4 +--
 lib/galaxy/webapps/galaxy/api/workflows.py   |  3 +--
 lib/galaxy/webapps/galaxy/controllers/tag.py |  2 +-
 10 files changed, 34 insertions(+), 48 deletions(-)

diff --git a/lib/galaxy/managers/collections.py b/lib/galaxy/managers/collections.py
index 1d92e23f141a..e941c69d680c 100644
--- a/lib/galaxy/managers/collections.py
+++ b/lib/galaxy/managers/collections.py
@@ -271,9 +271,7 @@ def _create_instance_for_collection(
         # values.
         if isinstance(tags, list):
             assert implicit_inputs is None, implicit_inputs
-            tags = trans.tag_handler.add_tags_from_list(
-                trans.user, dataset_collection_instance, tags, flush=False, galaxy_session=trans.galaxy_session
-            )
+            tags = trans.tag_handler.add_tags_from_list(trans.user, dataset_collection_instance, tags, flush=False)
         else:
             tags = self._append_tags(dataset_collection_instance, implicit_inputs, tags)
         return self.__persist(dataset_collection_instance, flush=flush)
@@ -500,7 +498,6 @@ def _set_from_dict(self, trans: ProvidesUserContext, dataset_collection_instance
                 trans.user,
                 dataset_collection_instance,
                 new_data["tags"],
-                galaxy_session=trans.galaxy_session,
             )
 
         if changed.keys():
@@ -650,18 +647,14 @@ def __load_element(self, trans, element_identifier, hide_source_items, copy_elem
                 hda.id, user=trans.user, current_history=history or trans.history
             ):
                 hda.visible = False
-            trans.tag_handler.apply_item_tags(
-                user=trans.user, item=element, tags_str=tag_str, flush=False, galaxy_session=trans.galaxy_session
-            )
+            trans.tag_handler.apply_item_tags(user=trans.user, item=element, tags_str=tag_str, flush=False)
             return element
         elif src_type == "ldda":
             element2 = self.ldda_manager.get(trans, element_id, check_accessible=True)
             element3 = element2.to_history_dataset_association(
                 history or trans.history, add_to_history=True, visible=not hide_source_items
             )
-            trans.tag_handler.apply_item_tags(
-                user=trans.user, item=element3, tags_str=tag_str, flush=False, galaxy_session=trans.galaxy_session
-            )
+            trans.tag_handler.apply_item_tags(user=trans.user, item=element3, tags_str=tag_str, flush=False)
             return element3
         elif src_type == "hdca":
             # TODO: Option to copy? Force copy? Copy or allow if not owned?
diff --git a/lib/galaxy/managers/library_datasets.py b/lib/galaxy/managers/library_datasets.py
index 76cfc97a656c..176a298929ca 100644
--- a/lib/galaxy/managers/library_datasets.py
+++ b/lib/galaxy/managers/library_datasets.py
@@ -109,12 +109,10 @@ def _set_from_dict(self, trans: ProvidesUserContext, ldda, new_data):
             changed = True
         new_tags = new_data.get("tags", None)
         if new_tags is not None and new_tags != ldda.tags:
-            trans.tag_handler.delete_item_tags(item=ldda, user=trans.user, galaxy_session=trans.galaxy_session)
+            trans.tag_handler.delete_item_tags(item=ldda, user=trans.user)
             tag_list = trans.tag_handler.parse_tags_list(new_tags)
             for tag in tag_list:
-                trans.tag_handler.apply_item_tag(
-                    item=ldda, user=trans.user, name=tag[0], value=tag[1], galaxy_session=trans.galaxy_session
-                )
+                trans.tag_handler.apply_item_tag(item=ldda, user=trans.user, name=tag[0], value=tag[1])
             changed = True
         if changed:
             ldda.update_parent_folder_update_times()
diff --git a/lib/galaxy/managers/tags.py b/lib/galaxy/managers/tags.py
index 043a85ace394..1bf1ec953392 100644
--- a/lib/galaxy/managers/tags.py
+++ b/lib/galaxy/managers/tags.py
@@ -46,17 +46,15 @@ class TagsManager:
 
     def update(self, trans: ProvidesUserContext, payload: ItemTagsPayload) -> None:
         """Apply a new set of tags to an item; previous tags are deleted."""
-        sa_session = trans.sa_session
         user = trans.user
-        tag_handler = GalaxyTagHandlerSession(sa_session, trans.galaxy_session)
         new_tags: Optional[str] = None
         if payload.item_tags and len(payload.item_tags.__root__) > 0:
             new_tags = ",".join(payload.item_tags.__root__)
-        item = self._get_item(tag_handler, payload)
-        tag_handler.delete_item_tags(user, item, galaxy_session=trans.galaxy_session)
-        tag_handler.apply_item_tags(user, item, new_tags, galaxy_session=trans.galaxy_session)
-        with transaction(sa_session):
-            sa_session.commit()
+        item = self._get_item(trans.tag_handler, payload)
+        trans.tag_handler.delete_item_tags(user, item)
+        trans.tag_handler.apply_item_tags(user, item, new_tags)
+        with transaction(trans.sa_session):
+            trans.sa_session.commit()
 
     def _get_item(self, tag_handler: GalaxyTagHandlerSession, payload: ItemTagsPayload):
         """
diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py
index 305b2a10e196..a268c4ce73c4 100644
--- a/lib/galaxy/managers/workflows.py
+++ b/lib/galaxy/managers/workflows.py
@@ -639,9 +639,7 @@ def build_workflow_from_raw_description(
             annotation = sanitize_html(data["annotation"])
             self.add_item_annotation(trans.sa_session, stored.user, stored, annotation)
         workflow_tags = data.get("tags", [])
-        trans.app.tag_handler.set_tags_from_list(
-            user=trans.user, item=stored, new_tags_list=workflow_tags, galaxy_session=trans.galaxy_session
-        )
+        trans.tag_handler.set_tags_from_list(user=trans.user, item=stored, new_tags_list=workflow_tags)
 
         # Persist
         trans.sa_session.add(stored)
diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py
index d5b3609a0eeb..08eab6c50448 100644
--- a/lib/galaxy/model/tags.py
+++ b/lib/galaxy/model/tags.py
@@ -85,13 +85,17 @@ def remove_tags_from_list(
         return self.set_tags_from_list(user, item, tags_set, flush=flush, galaxy_session=galaxy_session)
 
     def set_tags_from_list(
-        self, user, item, new_tags_list, flush=True, galaxy_session: Optional["GalaxySession"] = None
+        self,
+        user,
+        item,
+        new_tags_list,
+        flush=True,
     ):
         # precondition: item is already security checked against user
         # precondition: incoming tags is a list of sanitized/formatted strings
-        self.delete_item_tags(user, item, galaxy_session=galaxy_session)
+        self.delete_item_tags(user, item)
         new_tags_str = ",".join(new_tags_list)
-        self.apply_item_tags(user, item, unicodify(new_tags_str, "utf-8"), flush=flush, galaxy_session=galaxy_session)
+        self.apply_item_tags(user, item, unicodify(new_tags_str, "utf-8"), flush=flush)
         if flush:
             with transaction(self.sa_session):
                 self.sa_session.commit()
@@ -147,9 +151,9 @@ def get_tool_tags(self):
             tags.append(self.get_tag_by_id(tag_id))
         return tags
 
-    def remove_item_tag(self, user: "User", item, tag_name: str, galaxy_session: Optional["GalaxySession"] = None):
+    def remove_item_tag(self, user: "User", item, tag_name: str):
         """Remove a tag from an item."""
-        self._ensure_user_owns_item(user, item, galaxy_session)
+        self._ensure_user_owns_item(user, item)
         # Get item tag association.
         item_tag_assoc = self._get_item_tag_assoc(user, item, tag_name)
         # Remove association.
@@ -160,9 +164,9 @@ def remove_item_tag(self, user: "User", item, tag_name: str, galaxy_session: Opt
             return True
         return False
 
-    def delete_item_tags(self, user: Optional["User"], item, galaxy_session: Optional["GalaxySession"] = None):
+    def delete_item_tags(self, user: Optional["User"], item):
         """Delete tags from an item."""
-        self._ensure_user_owns_item(user, item, galaxy_session)
+        self._ensure_user_owns_item(user, item)
         # Delete item-tag associations.
         for tag in item.tags:
             if tag.id:
@@ -171,7 +175,7 @@ def delete_item_tags(self, user: Optional["User"], item, galaxy_session: Optiona
         # Delete tags from item.
         del item.tags[:]
 
-    def _ensure_user_owns_item(self, user: Optional["User"], item, galaxy_session: Optional["GalaxySession"] = None):
+    def _ensure_user_owns_item(self, user: Optional["User"], item):
         """Raises exception if user does not own item.
         Notice that even admin users cannot directly modify tags on items they do not own.
         To modify tags on items they don't own, admin users must impersonate the item's owner.
@@ -218,9 +222,8 @@ def apply_item_tag(
         name,
         value=None,
         flush=True,
-        galaxy_session: Optional["GalaxySession"] = None,
     ):
-        self._ensure_user_owns_item(user, item, galaxy_session=galaxy_session)
+        self._ensure_user_owns_item(user, item)
         # Use lowercase name for searching/creating tag.
         if name is None:
             return
@@ -261,10 +264,9 @@ def apply_item_tags(
         item,
         tags_str: Optional[str],
         flush=True,
-        galaxy_session: Optional["GalaxySession"] = None,
     ):
         """Apply tags to an item."""
-        self._ensure_user_owns_item(user, item, galaxy_session=galaxy_session)
+        self._ensure_user_owns_item(user, item)
         # Parse tags.
         parsed_tags = self.parse_tags(tags_str)
         # Apply each tag.
@@ -509,7 +511,7 @@ def _create_tag_instance(self, tag_name):
 
 
 class GalaxySessionlessTagHandler(GalaxyTagHandlerSession):
-    def _ensure_user_owns_item(self, user: Optional["User"], item, galaxy_session: Optional["GalaxySession"] = None):
+    def _ensure_user_owns_item(self, user: Optional["User"], item):
         # In sessionless mode we don't need to check ownership, we're only exporting
         pass
 
diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py
index d6e7836153a6..53e8f1e0c4cb 100644
--- a/lib/galaxy/tools/__init__.py
+++ b/lib/galaxy/tools/__init__.py
@@ -3725,7 +3725,10 @@ def copy_dataset(dataset, tags):
             copied_dataset = dataset.copy(copy_tags=dataset.tags, flush=False)
             if tags is not None:
                 tag_handler.set_tags_from_list(
-                    trans.get_user(), copied_dataset, tags, flush=False, galaxy_session=trans.galaxy_session
+                    trans.get_user(),
+                    copied_dataset,
+                    tags,
+                    flush=False,
                 )
             copied_dataset.history_id = history.id
             copied_datasets.append(copied_dataset)
@@ -3778,7 +3781,6 @@ def add_copied_value_to_new_elements(new_tags_dict, dce):
                         item=copied_value,
                         new_tags_list=new_tags,
                         flush=False,
-                        galaxy_session=history.galaxy_sessions,
                     )
             else:
                 # We have a collection, and we copy the elements so that we don't manipulate the original tags
diff --git a/lib/galaxy/webapps/base/controller.py b/lib/galaxy/webapps/base/controller.py
index 4112c158a47b..45ba80c29b70 100644
--- a/lib/galaxy/webapps/base/controller.py
+++ b/lib/galaxy/webapps/base/controller.py
@@ -1404,9 +1404,7 @@ def _remove_items_tag(self, trans, item_class_name, id, tag_name):
         """Remove a tag from an item."""
         user = trans.user
         tagged_item = self._get_tagged_item(trans, item_class_name, id)
-        deleted = tagged_item and trans.tag_handler.remove_item_tag(
-            user, tagged_item, tag_name, galaxy_session=trans.galaxy_session
-        )
+        deleted = tagged_item and trans.tag_handler.remove_item_tag(user, tagged_item, tag_name)
         with transaction(trans.sa_session):
             trans.sa_session.commit()
         return deleted
diff --git a/lib/galaxy/webapps/galaxy/api/jobs.py b/lib/galaxy/webapps/galaxy/api/jobs.py
index 1c5a3598b004..72a565b05ccd 100644
--- a/lib/galaxy/webapps/galaxy/api/jobs.py
+++ b/lib/galaxy/webapps/galaxy/api/jobs.py
@@ -487,9 +487,7 @@ def search(self, trans: ProvidesHistoryContext, payload: dict, **kwd):
         for k, v in payload.items():
             if k.startswith("files_") or k.startswith("__files_"):
                 inputs[k] = v
-        request_context = WorkRequestContext(
-            app=trans.app, user=trans.user, history=trans.history, galaxy_session=trans.galaxy_session
-        )
+        request_context = WorkRequestContext(app=trans.app, user=trans.user, history=trans.history)
         all_params, all_errors, _, _ = tool.expand_incoming(
             trans=trans, incoming=inputs, request_context=request_context
         )
diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py
index 7c5637e60bb8..dc3713100660 100644
--- a/lib/galaxy/webapps/galaxy/api/workflows.py
+++ b/lib/galaxy/webapps/galaxy/api/workflows.py
@@ -524,11 +524,10 @@ def update(self, trans: GalaxyWebTransaction, id, payload, **kwds):
                         require_flush = True
             # set tags
             if "tags" in workflow_dict:
-                trans.app.tag_handler.set_tags_from_list(
+                trans.tag_handler.set_tags_from_list(
                     user=trans.user,
                     item=stored_workflow,
                     new_tags_list=workflow_dict["tags"],
-                    galaxy_session=trans.galaxy_session,
                 )
 
             if require_flush:
diff --git a/lib/galaxy/webapps/galaxy/controllers/tag.py b/lib/galaxy/webapps/galaxy/controllers/tag.py
index 5c690c5347c1..e8d5bf820d50 100644
--- a/lib/galaxy/webapps/galaxy/controllers/tag.py
+++ b/lib/galaxy/webapps/galaxy/controllers/tag.py
@@ -46,7 +46,7 @@ def remove_tag_async(self, trans, item_id=None, item_class=None, tag_name=None,
         # Remove tag.
         item = self._get_item(trans, item_class, trans.security.decode_id(item_id))
         user = trans.user
-        trans.tag_handler.remove_item_tag(user, item, tag_name, galaxy_session=trans.galaxy_session)
+        trans.tag_handler.remove_item_tag(user, item, tag_name)
         with transaction(trans.sa_session):
             trans.sa_session.commit()
         # Log.

From a33f66517e3c5d8d7590b65f0cc7457c4ad339c7 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Fri, 28 Jul 2023 20:01:13 +0200
Subject: [PATCH 13/41] Test anon tag update and delete

---
 lib/galaxy_test/api/test_datasets.py | 34 ++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/lib/galaxy_test/api/test_datasets.py b/lib/galaxy_test/api/test_datasets.py
index 9fedb5116ccf..fdd139d78640 100644
--- a/lib/galaxy_test/api/test_datasets.py
+++ b/lib/galaxy_test/api/test_datasets.py
@@ -1,4 +1,5 @@
 import textwrap
+import urllib
 import zipfile
 from io import BytesIO
 from typing import (
@@ -530,6 +531,39 @@ def test_tag_change(self, history_id):
         assert "name:tag_d" in updated_hda["tags"]
         assert "name:tag_e" in updated_hda["tags"]
 
+    def test_anon_tag_permissions(self):
+        with self._different_user(anon=True):
+            history_id = self._get(urllib.parse.urljoin(self.url, "history/current_history_json")).json()["id"]
+            hda_id = self.dataset_populator.new_dataset(history_id, content="abc", wait=True)["id"]
+            payload = {
+                "item_id": hda_id,
+                "item_class": "HistoryDatasetAssociation",
+                "item_tags": ["cool:tag_a", "cool:tag_b", "tag_c", "name:tag_d", "#tag_e"],
+            }
+            put_response = self._put("tags", data=payload, json=True)
+            updated_hda = self._get(f"histories/{history_id}/contents/{hda_id}").json()
+            assert len(updated_hda["tags"]) == 5
+            # ensure we can remove these tags again
+            payload = {
+                "item_id": hda_id,
+                "item_class": "HistoryDatasetAssociation",
+                "item_tags": [],
+            }
+            put_response = self._put("tags", data=payload, json=True)
+            put_response.raise_for_status()
+            updated_hda = self._get(f"histories/{history_id}/contents/{hda_id}").json()
+            assert len(updated_hda["tags"]) == 0
+        with self._different_user(anon=True):
+            # another anon user can't modify tags
+            payload = {
+                "item_id": hda_id,
+                "item_class": "HistoryDatasetAssociation",
+                "item_tags": ["cool:tag_a", "cool:tag_b", "tag_c", "name:tag_d", "#tag_e"],
+            }
+            put_response = self._put("tags", data=payload, json=True)
+            updated_hda = self._get(f"histories/{history_id}/contents/{hda_id}").json()
+            assert len(updated_hda["tags"]) == 0
+
     @skip_without_tool("cat_data_and_sleep")
     def test_update_datatype(self, history_id):
         hda_id = self.dataset_populator.new_dataset(history_id)["id"]

From da68ee18ea9d3ddf56fe643cb11bfdcd4b9976f5 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Sun, 30 Jul 2023 14:46:54 +0200
Subject: [PATCH 14/41] Fix linting

---
 lib/galaxy/managers/library_datasets.py    | 1 -
 lib/galaxy/managers/taggable.py            | 6 +-----
 lib/galaxy/tools/actions/upload_common.py  | 1 -
 lib/galaxy/webapps/base/controller.py      | 1 -
 test/unit/app/managers/test_UserManager.py | 4 ++--
 5 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/lib/galaxy/managers/library_datasets.py b/lib/galaxy/managers/library_datasets.py
index 176a298929ca..f1296c454df0 100644
--- a/lib/galaxy/managers/library_datasets.py
+++ b/lib/galaxy/managers/library_datasets.py
@@ -13,7 +13,6 @@
 )
 from galaxy.managers import datasets
 from galaxy.managers.context import ProvidesUserContext
-from galaxy.model import tags
 from galaxy.model.base import transaction
 from galaxy.structured_app import MinimalManagerApp
 from galaxy.util import validation
diff --git a/lib/galaxy/managers/taggable.py b/lib/galaxy/managers/taggable.py
index 249cf31696a0..fae4b6440185 100644
--- a/lib/galaxy/managers/taggable.py
+++ b/lib/galaxy/managers/taggable.py
@@ -6,10 +6,7 @@
 
 import logging
 import re
-from typing import (
-    Optional,
-    Type,
-)
+from typing import Optional
 
 from sqlalchemy import (
     func,
@@ -19,7 +16,6 @@
 from galaxy import model
 from galaxy.managers.context import ProvidesUserContext
 from galaxy.model.tags import GalaxyTagHandler
-from galaxy.util import unicodify
 from .base import (
     ModelValidator,
     raise_filter_err,
diff --git a/lib/galaxy/tools/actions/upload_common.py b/lib/galaxy/tools/actions/upload_common.py
index 2f1c0938621e..14e4861a9ad8 100644
--- a/lib/galaxy/tools/actions/upload_common.py
+++ b/lib/galaxy/tools/actions/upload_common.py
@@ -28,7 +28,6 @@
     LibraryDataset,
     LibraryFolder,
     Role,
-    tags,
 )
 from galaxy.model.base import transaction
 from galaxy.util import is_url
diff --git a/lib/galaxy/webapps/base/controller.py b/lib/galaxy/webapps/base/controller.py
index 45ba80c29b70..ab5f03b9d9d8 100644
--- a/lib/galaxy/webapps/base/controller.py
+++ b/lib/galaxy/webapps/base/controller.py
@@ -35,7 +35,6 @@
     ExtendedMetadataIndex,
     HistoryDatasetAssociation,
     LibraryDatasetDatasetAssociation,
-    tags,
 )
 from galaxy.model.base import transaction
 from galaxy.model.item_attrs import UsesAnnotations
diff --git a/test/unit/app/managers/test_UserManager.py b/test/unit/app/managers/test_UserManager.py
index abefe20ee828..00152b51ca97 100644
--- a/test/unit/app/managers/test_UserManager.py
+++ b/test/unit/app/managers/test_UserManager.py
@@ -195,7 +195,7 @@ def testable_url_for(*a, **k):
     def test_activation_email(self):
         self.log("should produce the activation email")
         self.user_manager.create(email="user@nopassword.com", username="nopassword")
-        setattr(self.trans.request, "host", "request.host")
+        self.trans.request.host = "request.host"
 
         def validate_send_email(frm, to, subject, body, config, html=None):
             assert frm == "email_from"
@@ -219,7 +219,7 @@ def validate_send_email(frm, to, subject, body, config, html=None):
     def test_reset_email(self):
         self.log("should produce the password reset email")
         self.user_manager.create(email="user@nopassword.com", username="nopassword")
-        setattr(self.trans.request, "host", "request.host")
+        self.trans.request.host = "request.host"
 
         def validate_send_email(frm, to, subject, body, config, html=None):
             assert frm == "email_from"

From a0b365b315b840da2a507b5d8217e2a82355340f Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Sun, 30 Jul 2023 15:13:19 +0200
Subject: [PATCH 15/41] Add request.host to MockTrans

---
 lib/galaxy/app_unittest_utils/galaxy_mock.py | 2 +-
 test/unit/app/managers/test_UserManager.py   | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/galaxy/app_unittest_utils/galaxy_mock.py b/lib/galaxy/app_unittest_utils/galaxy_mock.py
index fe7f5ba04f55..462bf7094512 100644
--- a/lib/galaxy/app_unittest_utils/galaxy_mock.py
+++ b/lib/galaxy/app_unittest_utils/galaxy_mock.py
@@ -293,7 +293,7 @@ def __init__(self, app=None, user=None, history=None, **kwargs):
         self.security = self.app.security
         self.history = history
 
-        self.request: Any = Bunch(headers={}, body=None)
+        self.request: Any = Bunch(headers={}, body=None, host="request.host")
         self.response: Any = Bunch(headers={}, set_content_type=lambda i: None)
 
     @property
diff --git a/test/unit/app/managers/test_UserManager.py b/test/unit/app/managers/test_UserManager.py
index 00152b51ca97..250d87e8adee 100644
--- a/test/unit/app/managers/test_UserManager.py
+++ b/test/unit/app/managers/test_UserManager.py
@@ -195,7 +195,6 @@ def testable_url_for(*a, **k):
     def test_activation_email(self):
         self.log("should produce the activation email")
         self.user_manager.create(email="user@nopassword.com", username="nopassword")
-        self.trans.request.host = "request.host"
 
         def validate_send_email(frm, to, subject, body, config, html=None):
             assert frm == "email_from"
@@ -219,7 +218,6 @@ def validate_send_email(frm, to, subject, body, config, html=None):
     def test_reset_email(self):
         self.log("should produce the password reset email")
         self.user_manager.create(email="user@nopassword.com", username="nopassword")
-        self.trans.request.host = "request.host"
 
         def validate_send_email(frm, to, subject, body, config, html=None):
             assert frm == "email_from"

From 818634af9541f9af657594eebe836229d2df444d Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Sun, 30 Jul 2023 15:25:41 +0200
Subject: [PATCH 16/41] Override session when passed in explicitly

---
 lib/galaxy/model/tags.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py
index 08eab6c50448..dbfb702f285b 100644
--- a/lib/galaxy/model/tags.py
+++ b/lib/galaxy/model/tags.py
@@ -90,7 +90,10 @@ def set_tags_from_list(
         item,
         new_tags_list,
         flush=True,
+        galaxy_session: Optional["GalaxySession"] = None,
     ):
+        # Override galaxy_session if passed in
+        self.galaxy_session = self.galaxy_session or galaxy_session
         # precondition: item is already security checked against user
         # precondition: incoming tags is a list of sanitized/formatted strings
         self.delete_item_tags(user, item)

From f6b2faddda08169039de32182dc5811eae3918fc Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Sun, 30 Jul 2023 15:56:28 +0200
Subject: [PATCH 17/41] Unify tags API for LibraryDatasets

It should return a list of tags as all the other kind of items instead of a comma-separated string.
---
 lib/galaxy/managers/library_datasets.py       |  2 +-
 lib/galaxy/model/tags.py                      | 22 ++++++++++++-------
 lib/galaxy/schema/schema.py                   |  2 +-
 .../webapps/galaxy/api/library_contents.py    |  2 +-
 .../services/library_folder_contents.py       |  2 +-
 5 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/lib/galaxy/managers/library_datasets.py b/lib/galaxy/managers/library_datasets.py
index f1296c454df0..091946714041 100644
--- a/lib/galaxy/managers/library_datasets.py
+++ b/lib/galaxy/managers/library_datasets.py
@@ -250,7 +250,7 @@ def serialize(self, trans, ld):
             current_user_roles, ld
         )
         rval["is_unrestricted"] = trans.app.security_agent.dataset_is_public(ldda.dataset)
-        rval["tags"] = trans.tag_handler.get_tags_str(ldda.tags)
+        rval["tags"] = trans.tag_handler.get_tags_list(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(
diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py
index dbfb702f285b..0e24dccaf4d3 100644
--- a/lib/galaxy/model/tags.py
+++ b/lib/galaxy/model/tags.py
@@ -276,19 +276,25 @@ def apply_item_tags(
         for name, value in parsed_tags:
             self.apply_item_tag(user, item, name, value, flush=flush)
 
-    def get_tags_str(self, tags):
-        """Build a string from an item's tags."""
-        # Return empty string if there are no tags.
+    def get_tags_list(self, tags) -> List[str]:
+        """Build a list of tags from an item's tags."""
+        # Return empty list if there are no tags.
         if not tags:
-            return ""
-        # Create string of tags.
-        tags_str_list = list()
+            return []
+        # Create list of tags.
+        tags_list: List[str] = []
         for tag in tags:
             tag_str = tag.user_tname
             if tag.value is not None:
                 tag_str += f":{tag.user_value}"
-            tags_str_list.append(tag_str)
-        return ", ".join(tags_str_list)
+            tags_list.append(tag_str)
+        return tags_list
+
+    def get_tags_str(self, tags):
+        """Build a string from an item's tags."""
+        tags_list = self.get_tags_list(tags)
+        # Return empty string if there are no tags.
+        return ", ".join(tags_list)
 
     def get_tag_by_id(self, tag_id):
         """Get a Tag object from a tag id."""
diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py
index 863f4543f149..ca0d2b03210a 100644
--- a/lib/galaxy/schema/schema.py
+++ b/lib/galaxy/schema/schema.py
@@ -2912,7 +2912,7 @@ class FileLibraryFolderItem(LibraryFolderItemBase):
     file_size: str
     raw_size: int
     ldda_id: EncodedDatabaseIdField
-    tags: str
+    tags: TagCollection
     message: Optional[str]
 
 
diff --git a/lib/galaxy/webapps/galaxy/api/library_contents.py b/lib/galaxy/webapps/galaxy/api/library_contents.py
index c152759b66c4..e3c5ecce3ee1 100644
--- a/lib/galaxy/webapps/galaxy/api/library_contents.py
+++ b/lib/galaxy/webapps/galaxy/api/library_contents.py
@@ -183,7 +183,7 @@ def show(self, trans, id, library_id, **kwd):
             rval["parent_library_id"] = trans.security.encode_id(rval["parent_library_id"])
 
             tag_manager = tags.GalaxyTagHandler(trans.sa_session)
-            rval["tags"] = tag_manager.get_tags_str(content.library_dataset_dataset_association.tags)
+            rval["tags"] = tag_manager.get_tags_list(content.library_dataset_dataset_association.tags)
         return rval
 
     @expose_api
diff --git a/lib/galaxy/webapps/galaxy/services/library_folder_contents.py b/lib/galaxy/webapps/galaxy/services/library_folder_contents.py
index 1b601991efd3..6629559140fa 100644
--- a/lib/galaxy/webapps/galaxy/services/library_folder_contents.py
+++ b/lib/galaxy/webapps/galaxy/services/library_folder_contents.py
@@ -183,7 +183,7 @@ def _serialize_library_dataset(
             file_size=util.nice_size(raw_size),
             raw_size=raw_size,
             ldda_id=ldda.id,
-            tags=tag_manager.get_tags_str(ldda.tags),
+            tags=tag_manager.get_tags_list(ldda.tags),
             message=ldda.message or ldda.info,
         )
         return dataset_item

From 33262303ef93c5ba4c565b5ecbda03bd798be8db Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Mon, 31 Jul 2023 10:58:03 +0200
Subject: [PATCH 18/41] Drop explicit galaxy_session passing

Forgot to remove this from a previous attempt.
---
 lib/galaxy/model/store/discover.py    |  2 +-
 lib/galaxy/model/tags.py              | 11 ++++-------
 lib/galaxy/webapps/base/controller.py |  2 +-
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/lib/galaxy/model/store/discover.py b/lib/galaxy/model/store/discover.py
index 3cdf14bfc6bd..a27fe815ca69 100644
--- a/lib/galaxy/model/store/discover.py
+++ b/lib/galaxy/model/store/discover.py
@@ -166,7 +166,7 @@ def create_dataset(
         if tag_list:
             job = self.get_job()
             self.tag_handler.add_tags_from_list(
-                job and job.user, primary_data, tag_list, flush=False, galaxy_session=job and job.galaxy_session
+                job and job.user, primary_data, tag_list, flush=False,
             )
 
         # If match specified a name use otherwise generate one from
diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py
index 0e24dccaf4d3..09e39edd47a9 100644
--- a/lib/galaxy/model/tags.py
+++ b/lib/galaxy/model/tags.py
@@ -68,21 +68,21 @@ def create_tag_handler_session(self, galaxy_session: Optional["GalaxySession"]):
         return GalaxyTagHandlerSession(self.sa_session, galaxy_session=galaxy_session)
 
     def add_tags_from_list(
-        self, user, item, new_tags_list, flush=True, galaxy_session: Optional["GalaxySession"] = None
+        self, user, item, new_tags_list, flush=True,
     ):
         new_tags_set = set(new_tags_list)
         if item.tags:
             new_tags_set.update(self.get_tags_str(item.tags).split(","))
-        return self.set_tags_from_list(user, item, new_tags_set, flush=flush, galaxy_session=galaxy_session)
+        return self.set_tags_from_list(user, item, new_tags_set, flush=flush)
 
     def remove_tags_from_list(
-        self, user, item, tag_to_remove_list, flush=True, galaxy_session: Optional["GalaxySession"] = None
+        self, user, item, tag_to_remove_list, flush=True,
     ):
         tag_to_remove_set = set(tag_to_remove_list)
         tags_set = {_.strip() for _ in self.get_tags_str(item.tags).split(",")}
         if item.tags:
             tags_set -= tag_to_remove_set
-        return self.set_tags_from_list(user, item, tags_set, flush=flush, galaxy_session=galaxy_session)
+        return self.set_tags_from_list(user, item, tags_set, flush=flush)
 
     def set_tags_from_list(
         self,
@@ -90,10 +90,7 @@ def set_tags_from_list(
         item,
         new_tags_list,
         flush=True,
-        galaxy_session: Optional["GalaxySession"] = None,
     ):
-        # Override galaxy_session if passed in
-        self.galaxy_session = self.galaxy_session or galaxy_session
         # precondition: item is already security checked against user
         # precondition: incoming tags is a list of sanitized/formatted strings
         self.delete_item_tags(user, item)
diff --git a/lib/galaxy/webapps/base/controller.py b/lib/galaxy/webapps/base/controller.py
index ab5f03b9d9d8..ad579fc01595 100644
--- a/lib/galaxy/webapps/base/controller.py
+++ b/lib/galaxy/webapps/base/controller.py
@@ -1423,7 +1423,7 @@ def _get_item_tag_assoc(self, trans, item_class_name, id, tag_name):
         return trans.tag_handler._get_item_tag_assoc(user, tagged_item, tag_name)
 
     def set_tags_from_list(self, trans, item, new_tags_list, user=None):
-        return trans.tag_handler.set_tags_from_list(user, item, new_tags_list, galaxy_session=trans.galaxy_session)
+        return trans.tag_handler.set_tags_from_list(user, item, new_tags_list)
 
     def get_user_tags_used(self, trans, user=None):
         """

From c1efbdfa6c94fcbccbd4c029301f8468c449514f Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Mon, 31 Jul 2023 10:58:40 +0200
Subject: [PATCH 19/41] Fix integration tests

---
 test/integration/test_workflow_refactoring.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/test/integration/test_workflow_refactoring.py b/test/integration/test_workflow_refactoring.py
index 476996174117..4c12dae60f02 100644
--- a/test/integration/test_workflow_refactoring.py
+++ b/test/integration/test_workflow_refactoring.py
@@ -893,6 +893,10 @@ def __init__(self, app, user):
         self.history = None
         self.workflow_building_mode = workflow_building_modes.ENABLED
 
+    @property
+    def galaxy_session(self):
+        return None
+
     @property
     def app(self):
         return self._app

From e9cccd6d769947212ae011a49cc3099705ef9f07 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 31 Jul 2023 12:48:31 +0200
Subject: [PATCH 20/41] Fix formatting

---
 lib/galaxy/model/store/discover.py | 4 +---
 lib/galaxy/model/tags.py           | 8 ++------
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/lib/galaxy/model/store/discover.py b/lib/galaxy/model/store/discover.py
index a27fe815ca69..a56d00fd3c33 100644
--- a/lib/galaxy/model/store/discover.py
+++ b/lib/galaxy/model/store/discover.py
@@ -165,9 +165,7 @@ def create_dataset(
 
         if tag_list:
             job = self.get_job()
-            self.tag_handler.add_tags_from_list(
-                job and job.user, primary_data, tag_list, flush=False,
-            )
+            self.tag_handler.add_tags_from_list(job and job.user, primary_data, tag_list, flush=False)
 
         # If match specified a name use otherwise generate one from
         # designation.
diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py
index 09e39edd47a9..5d2035d33891 100644
--- a/lib/galaxy/model/tags.py
+++ b/lib/galaxy/model/tags.py
@@ -67,17 +67,13 @@ def create_tag_handler_session(self, galaxy_session: Optional["GalaxySession"]):
         # Creates a transient tag handler that avoids repeated flushes
         return GalaxyTagHandlerSession(self.sa_session, galaxy_session=galaxy_session)
 
-    def add_tags_from_list(
-        self, user, item, new_tags_list, flush=True,
-    ):
+    def add_tags_from_list(self, user, item, new_tags_list, flush=True):
         new_tags_set = set(new_tags_list)
         if item.tags:
             new_tags_set.update(self.get_tags_str(item.tags).split(","))
         return self.set_tags_from_list(user, item, new_tags_set, flush=flush)
 
-    def remove_tags_from_list(
-        self, user, item, tag_to_remove_list, flush=True,
-    ):
+    def remove_tags_from_list(self, user, item, tag_to_remove_list, flush=True):
         tag_to_remove_set = set(tag_to_remove_list)
         tags_set = {_.strip() for _ in self.get_tags_str(item.tags).split(",")}
         if item.tags:

From 451b1e3609ffc07c8dd6cc3307e918c04517823d Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 31 Jul 2023 14:03:23 +0200
Subject: [PATCH 21/41] Remove left over galaxy_session param

---
 lib/galaxy/model/store/discover.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/galaxy/model/store/discover.py b/lib/galaxy/model/store/discover.py
index a56d00fd3c33..832357acb45b 100644
--- a/lib/galaxy/model/store/discover.py
+++ b/lib/galaxy/model/store/discover.py
@@ -395,12 +395,9 @@ def _populate_elements(self, chunk, name, root_collection_builder, metadata_sour
         self.set_datasets_metadata(datasets=element_datasets["datasets"])
 
     def add_tags_to_datasets(self, datasets, tag_lists):
-        job = self.get_job()
         if any(tag_lists):
             for dataset, tags in zip(datasets, tag_lists):
-                self.tag_handler.add_tags_from_list(
-                    self.user, dataset, tags, flush=False, galaxy_session=job and job.galaxy_session
-                )
+                self.tag_handler.add_tags_from_list(self.user, dataset, tags, flush=False)
 
     def update_object_store_with_datasets(self, datasets, paths, extra_files, output_name):
         for dataset, path, extra_file in zip(datasets, paths, extra_files):

From a4b5f012bc287ef84cbc471f7325111eec09d99a Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 31 Jul 2023 14:07:50 +0200
Subject: [PATCH 22/41] Update client API schema

---
 client/src/schema/schema.ts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/client/src/schema/schema.ts b/client/src/schema/schema.ts
index 5d736f8e7a69..d932c01f96e5 100644
--- a/client/src/schema/schema.ts
+++ b/client/src/schema/schema.ts
@@ -3802,8 +3802,7 @@ export interface components {
              * @description The current state of this dataset.
              */
             state: components["schemas"]["DatasetState"];
-            /** Tags */
-            tags: string;
+            tags: components["schemas"]["TagCollection"];
             /**
              * Type
              * @enum {string}

From 3b4e7fb5ee367e4b38a0f47e5ffaaccfbcfee031 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Mon, 31 Jul 2023 15:48:46 +0200
Subject: [PATCH 23/41] Drop final use of hda_manager.tag_handler

---
 .../galaxy/services/history_contents.py       | 21 +++++++++----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/lib/galaxy/webapps/galaxy/services/history_contents.py b/lib/galaxy/webapps/galaxy/services/history_contents.py
index 0232c7ff5556..0254f92001f7 100644
--- a/lib/galaxy/webapps/galaxy/services/history_contents.py
+++ b/lib/galaxy/webapps/galaxy/services/history_contents.py
@@ -43,7 +43,10 @@
     api_payload_to_create_params,
     dictify_dataset_collection_instance,
 )
-from galaxy.managers.context import ProvidesHistoryContext
+from galaxy.managers.context import (
+    ProvidesHistoryContext,
+    ProvidesUserContext,
+)
 from galaxy.managers.genomes import GenomesManager
 from galaxy.managers.history_contents import (
     HistoryContentsFilters,
@@ -1389,10 +1392,8 @@ def __init__(
                 item, params, trans
             ),
             HistoryContentItemOperation.change_dbkey: lambda item, params, trans: self._change_dbkey(item, params),
-            HistoryContentItemOperation.add_tags: lambda item, params, trans: self._add_tags(item, trans.user, params),
-            HistoryContentItemOperation.remove_tags: lambda item, params, trans: self._remove_tags(
-                item, trans.user, params
-            ),
+            HistoryContentItemOperation.add_tags: lambda item, params, trans: self._add_tags(trans, item, params),
+            HistoryContentItemOperation.remove_tags: lambda item, params, trans: self._remove_tags(trans, item, params),
         }
 
     def apply(
@@ -1478,10 +1479,8 @@ def _change_dbkey(self, item: HistoryItemModel, params: ChangeDbkeyOperationPara
             for dataset_instance in item.dataset_instances:
                 dataset_instance.set_dbkey(params.dbkey)
 
-    def _add_tags(self, item: HistoryItemModel, user: User, params: TagOperationParams):
-        manager = self._get_item_manager(item)
-        manager.tag_handler.add_tags_from_list(user, item, params.tags, flush=self.flush)
+    def _add_tags(self, trans: ProvidesUserContext, item: HistoryItemModel, params: TagOperationParams):
+        trans.tag_handler.add_tags_from_list(trans.user, item, params.tags, flush=self.flush)
 
-    def _remove_tags(self, item: HistoryItemModel, user: User, params: TagOperationParams):
-        manager = self._get_item_manager(item)
-        manager.tag_handler.remove_tags_from_list(user, item, params.tags, flush=self.flush)
+    def _remove_tags(self, trans: ProvidesUserContext, item: HistoryItemModel, params: TagOperationParams):
+        trans.tag_handler.remove_tags_from_list(trans.user, item, params.tags, flush=self.flush)

From 2d3a822583965b27213e569f45ea0a5aea3154ea Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Mon, 31 Jul 2023 15:48:58 +0200
Subject: [PATCH 24/41] Update test assertion

---
 lib/galaxy_test/api/test_libraries.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy_test/api/test_libraries.py b/lib/galaxy_test/api/test_libraries.py
index 816efff4cadd..cd1c67926d29 100644
--- a/lib/galaxy_test/api/test_libraries.py
+++ b/lib/galaxy_test/api/test_libraries.py
@@ -458,7 +458,7 @@ def test_update_dataset_tags(self):
         data = {"tags": ["#Lancelot", "name:Holy Grail", "blue"]}
         ld_updated = self._patch_library_dataset(ld["id"], data)
         self._assert_has_keys(ld_updated, "tags")
-        assert ld_updated["tags"] == "name:Lancelot, name:HolyGrail, blue"
+        assert ld_updated["tags"] == ["name:Lancelot", "name:HolyGrail", "blue"]
 
     @requires_new_library
     def test_invalid_update_dataset_in_folder(self):

From 73a0edd06dedc1d73560c722fa314bfcc3c74c80 Mon Sep 17 00:00:00 2001
From: Dannon Baker <dannon.baker@gmail.com>
Date: Tue, 1 Aug 2023 17:17:12 -0400
Subject: [PATCH 25/41] Fix history item states help display to apply same
 styling attributes and resolution as the history does.  This fixes new/queued
 display colors.

That said, we should formalize this so we aren't relying on a mix of
data-state css rules and alert-<status> bindings.
---
 client/src/components/History/Content/model/StatesInfo.vue | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/client/src/components/History/Content/model/StatesInfo.vue b/client/src/components/History/Content/model/StatesInfo.vue
index 2463ae5e1d0f..4e0add202a67 100644
--- a/client/src/components/History/Content/model/StatesInfo.vue
+++ b/client/src/components/History/Content/model/StatesInfo.vue
@@ -45,7 +45,7 @@ function onFilter(value: string) {
         <p>Here are all available item states in Galaxy:</p>
         <p><i>(Note that the colors for each state correspond to content item state colors in the history)</i></p>
         <dl v-for="(state, key, index) in states" :key="index">
-            <b-alert :variant="state.status || 'success'" show>
+            <div :class="['alert', 'content-item', 'alert-' + state.status]" :data-state="key">
                 <dt>
                     <a class="text-decoration-none" href="javascript:void(0)" @click="onFilter(key)"
                         ><code>{{ key }}</code></a
@@ -53,7 +53,7 @@ function onFilter(value: string) {
                     <icon v-if="state.icon" :icon="state.icon" />
                 </dt>
                 <dd>{{ helpText[key] || state.text }}</dd>
-            </b-alert>
+            </div>
         </dl>
     </b-modal>
 </template>

From 214d38f5f33dc4d6721bf39467a57d8dd9119ff4 Mon Sep 17 00:00:00 2001
From: Ahmed Awan <qe66653@umbc.edu>
Date: Wed, 2 Aug 2023 13:03:42 -0500
Subject: [PATCH 26/41] Fix `ToolBoxWorkflow` search delay bug `queryFilter`
 prop for `ToolSection`s is set to null for invalid queries. This prevents the
 Watcher from being triggered for every letter entered for a search query.

---
 .../src/components/Panels/ToolBoxWorkflow.vue | 29 +++++++++++++++----
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/client/src/components/Panels/ToolBoxWorkflow.vue b/client/src/components/Panels/ToolBoxWorkflow.vue
index 0cf02296ccf5..c237adf00051 100644
--- a/client/src/components/Panels/ToolBoxWorkflow.vue
+++ b/client/src/components/Panels/ToolBoxWorkflow.vue
@@ -22,9 +22,19 @@
                 placeholder="search tools"
                 :toolbox="workflowTools"
                 :query="query"
+                :query-pending="queryPending"
                 @onQuery="onQuery"
                 @onResults="onResults" />
-            <div v-if="queryTooShort" class="pb-2">
+            <div v-if="closestTerm" class="pb-2">
+                <b-badge class="alert-danger w-100">
+                    Did you mean:
+                    <i>
+                        <a href="javascript:void(0)" @click="onQuery(closestTerm)">{{ closestTerm }}</a>
+                    </i>
+                    ?
+                </b-badge>
+            </div>
+            <div v-else-if="queryTooShort" class="pb-2">
                 <b-badge class="alert-danger w-100">Search string too short!</b-badge>
             </div>
             <div v-else-if="noResults" class="pb-2">
@@ -40,21 +50,21 @@
                     :category="category"
                     tool-key="name"
                     :section-name="category.name"
-                    :query-filter="query"
+                    :query-filter="queryFilter"
                     :disable-filter="true"
                     @onClick="onInsertModule" />
                 <tool-section
                     v-if="hasDataManagerSection"
                     :key="dataManagerSection.id"
                     :category="dataManagerSection"
-                    :query-filter="query"
+                    :query-filter="queryFilter"
                     :disable-filter="true"
                     @onClick="onInsertTool" />
                 <tool-section
                     v-for="section in sections"
                     :key="section.id"
                     :category="section"
-                    :query-filter="query"
+                    :query-filter="queryFilter"
                     @onClick="onInsertTool" />
                 <tool-section
                     v-if="hasWorkflowSection"
@@ -64,7 +74,7 @@
                     :sort-items="false"
                     operation-icon="fa fa-files-o"
                     operation-title="Insert individual steps."
-                    :query-filter="query"
+                    :query-filter="queryFilter"
                     :disable-filter="true"
                     @onClick="onInsertWorkflow"
                     @onOperation="onInsertWorkflowSteps" />
@@ -113,7 +123,10 @@ export default {
     },
     data() {
         return {
+            closestTerm: null,
             query: null,
+            queryPending: false,
+            queryFilter: null,
             results: null,
         };
     },
@@ -166,9 +179,13 @@ export default {
     methods: {
         onQuery(query) {
             this.query = query;
+            this.queryPending = true;
         },
-        onResults(results) {
+        onResults(results, closestTerm = null) {
             this.results = results;
+            this.closestTerm = closestTerm;
+            this.queryFilter = !this.noResults ? this.query : null;
+            this.queryPending = false;
         },
         onInsertTool(tool, evt) {
             evt.preventDefault();

From 34b7243acfbdfa258988a0f5c7d031370d6e3de4 Mon Sep 17 00:00:00 2001
From: Ahmed Awan <qe66653@umbc.edu>
Date: Wed, 2 Aug 2023 13:27:18 -0500
Subject: [PATCH 27/41] Make v-b-tooltip.hover directives noninteractive

---
 client/src/components/Common/DelayedInput.vue            | 4 ++--
 client/src/components/Panels/Buttons/FavoritesButton.vue | 2 +-
 client/src/components/Panels/Buttons/PanelViewButton.vue | 2 +-
 client/src/components/Panels/Common/ToolPanelLabel.vue   | 2 +-
 client/src/components/Panels/Common/ToolSection.vue      | 2 +-
 client/src/components/Upload/UploadButton.vue            | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/client/src/components/Common/DelayedInput.vue b/client/src/components/Common/DelayedInput.vue
index bb7691bbea1c..bb36d5f8fb92 100644
--- a/client/src/components/Common/DelayedInput.vue
+++ b/client/src/components/Common/DelayedInput.vue
@@ -13,7 +13,7 @@
         <b-input-group-append>
             <b-button
                 v-if="enableAdvanced"
-                v-b-tooltip.hover
+                v-b-tooltip.hover.noninteractive
                 aria-haspopup="true"
                 size="sm"
                 :pressed="showAdvanced"
@@ -25,7 +25,7 @@
                 <icon v-else fixed-width icon="angle-double-down" />
             </b-button>
             <b-button
-                v-b-tooltip.hover
+                v-b-tooltip.hover.noninteractive
                 aria-haspopup="true"
                 class="search-clear"
                 size="sm"
diff --git a/client/src/components/Panels/Buttons/FavoritesButton.vue b/client/src/components/Panels/Buttons/FavoritesButton.vue
index 63f0dd4c6173..18b4136ffe58 100644
--- a/client/src/components/Panels/Buttons/FavoritesButton.vue
+++ b/client/src/components/Panels/Buttons/FavoritesButton.vue
@@ -1,6 +1,6 @@
 <template>
     <b-button
-        v-b-tooltip.hover
+        v-b-tooltip.hover.noninteractive
         class="panel-header-button-toolbox"
         size="sm"
         variant="link"
diff --git a/client/src/components/Panels/Buttons/PanelViewButton.vue b/client/src/components/Panels/Buttons/PanelViewButton.vue
index 71809a564960..e68a2709b106 100644
--- a/client/src/components/Panels/Buttons/PanelViewButton.vue
+++ b/client/src/components/Panels/Buttons/PanelViewButton.vue
@@ -1,6 +1,6 @@
 <template>
     <b-dropdown
-        v-b-tooltip.hover
+        v-b-tooltip.hover.noninteractive
         right
         title="Show panel options"
         variant="link"
diff --git a/client/src/components/Panels/Common/ToolPanelLabel.vue b/client/src/components/Panels/Common/ToolPanelLabel.vue
index 791925ac8d75..13dc23970c47 100644
--- a/client/src/components/Panels/Common/ToolPanelLabel.vue
+++ b/client/src/components/Panels/Common/ToolPanelLabel.vue
@@ -1,5 +1,5 @@
 <template>
-    <div v-b-tooltip.topright.hover class="tool-panel-label" tabindex="0" :title="description">
+    <div v-b-tooltip.topright.hover.noninteractive class="tool-panel-label" tabindex="0" :title="description">
         {{ definition.text }}
         <ToolPanelLinks :links="definition.links" />
     </div>
diff --git a/client/src/components/Panels/Common/ToolSection.vue b/client/src/components/Panels/Common/ToolSection.vue
index 0d0c076c9571..1bad08e65d1d 100644
--- a/client/src/components/Panels/Common/ToolSection.vue
+++ b/client/src/components/Panels/Common/ToolSection.vue
@@ -1,7 +1,7 @@
 <template>
     <div v-if="isSection && hasElements" class="tool-panel-section">
         <div
-            v-b-tooltip.topright.hover
+            v-b-tooltip.topright.hover.noninteractive
             :class="['toolSectionTitle', `tool-menu-section-${sectionName}`]"
             :title="title">
             <a class="title-link" href="javascript:void(0)" @click="toggleMenu()">
diff --git a/client/src/components/Upload/UploadButton.vue b/client/src/components/Upload/UploadButton.vue
index 74b23bd5f2f0..a63e440c2029 100644
--- a/client/src/components/Upload/UploadButton.vue
+++ b/client/src/components/Upload/UploadButton.vue
@@ -1,7 +1,7 @@
 <template>
     <b-button
         id="activity-upload"
-        v-b-tooltip.hover.bottom
+        v-b-tooltip.hover.noninteractive.bottom
         :aria-label="title | localize"
         :title="title | localize"
         class="upload-button"

From 779ee9a5a8804ac568f4420591d535f75b26836d Mon Sep 17 00:00:00 2001
From: Dannon Baker <dannon.baker@gmail.com>
Date: Wed, 2 Aug 2023 15:09:27 -0400
Subject: [PATCH 28/41] Upgrade h5 to v8.

---
 config/plugins/visualizations/h5web/package.json | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/config/plugins/visualizations/h5web/package.json b/config/plugins/visualizations/h5web/package.json
index 145aa121d70a..96c3b5ea1363 100644
--- a/config/plugins/visualizations/h5web/package.json
+++ b/config/plugins/visualizations/h5web/package.json
@@ -1,6 +1,6 @@
 {
     "name": "visualization",
-    "version": "0.1.0",
+    "version": "0.2.0",
     "keywords": [
         "galaxy",
         "visualization"
@@ -8,7 +8,7 @@
     "license": "AFL-3.0",
     "dependencies": {
         "@galaxyproject/charts": "^0.0.6",
-        "@h5web/app": "^7.1.0",
+        "@h5web/app": "^8.0.0",
         "backbone": "^1.3.3",
         "normalize.css": "^8.0.1",
         "react": "17.0.2",

From 49d67b3e366b05ee988af243aee309f940708e97 Mon Sep 17 00:00:00 2001
From: Dannon Baker <dannon.baker@gmail.com>
Date: Wed, 2 Aug 2023 20:57:50 -0400
Subject: [PATCH 29/41] Upgrade parcel for h5web, drop a couple unused deps.

---
 config/plugins/visualizations/h5web/package.json  | 15 +++++----------
 config/plugins/visualizations/h5web/src/script.js |  2 +-
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/config/plugins/visualizations/h5web/package.json b/config/plugins/visualizations/h5web/package.json
index 96c3b5ea1363..b8cdef990ec4 100644
--- a/config/plugins/visualizations/h5web/package.json
+++ b/config/plugins/visualizations/h5web/package.json
@@ -3,25 +3,20 @@
     "version": "0.2.0",
     "keywords": [
         "galaxy",
-        "visualization"
+        "visualization",
+        "h5web"
     ],
     "license": "AFL-3.0",
     "dependencies": {
-        "@galaxyproject/charts": "^0.0.6",
         "@h5web/app": "^8.0.0",
-        "backbone": "^1.3.3",
         "normalize.css": "^8.0.1",
-        "react": "17.0.2",
-        "react-dom": "17.0.2"
+        "react": "18.2.0",
+        "react-dom": "18.2.0"
     },
     "scripts": {
         "build": "parcel build src/script.js --dist-dir static"
     },
     "devDependencies": {
-        "@types/react": "^17.0.52",
-        "@types/react-dom": "^17.0.18",
-        "buffer": "^5.7.1",
-        "parcel": "^2.0.0",
-        "typescript": "4.9.3"
+        "parcel": "^2.9.3"
     }
 }
diff --git a/config/plugins/visualizations/h5web/src/script.js b/config/plugins/visualizations/h5web/src/script.js
index db416ab2e35a..f57b8fea214a 100644
--- a/config/plugins/visualizations/h5web/src/script.js
+++ b/config/plugins/visualizations/h5web/src/script.js
@@ -9,7 +9,7 @@
  */
 
 import './styles.css';
-import React, { StrictMode } from 'react'
+import React from 'react'
 import {render as reactRender} from 'react-dom'
 import {App, H5GroveProvider} from '@h5web/app'
 

From 86caa8e18c8cc7c38d58aa33c22fd72c3227433f Mon Sep 17 00:00:00 2001
From: Dannon Baker <dannon.baker@gmail.com>
Date: Wed, 2 Aug 2023 23:09:16 -0400
Subject: [PATCH 30/41] Swap react to use createRoot in v18 style, formatting.

---
 .../visualizations/h5web/src/script.js        | 43 +++++++------------
 1 file changed, 16 insertions(+), 27 deletions(-)

diff --git a/config/plugins/visualizations/h5web/src/script.js b/config/plugins/visualizations/h5web/src/script.js
index f57b8fea214a..474cb6086fa2 100644
--- a/config/plugins/visualizations/h5web/src/script.js
+++ b/config/plugins/visualizations/h5web/src/script.js
@@ -1,46 +1,35 @@
 /**
  * Visualizer interface for h5web (https://github.com/silx-kit/h5web)
- * 
+ *
  * This relies on Galaxy being able to serve files using the
- * h5grove protocol (https://silx-kit.github.io/h5grove/). 
+ * h5grove protocol (https://silx-kit.github.io/h5grove/).
  * This provides efficient access to the contents of the
  * HDF5 file and avoids having to read the whole file at any
  * point.
  */
 
-import './styles.css';
-import React from 'react'
-import {render as reactRender} from 'react-dom'
-import {App, H5GroveProvider} from '@h5web/app'
+import "./styles.css";
+import React from "react";
+import { createRoot } from "react-dom/client";
+import { App, H5GroveProvider } from "@h5web/app";
 
 function MyApp(props) {
     return (
-          <H5GroveProvider
-            url={props.url}
-            filepath={props.name}
-            axiosConfig={{ params: { file: props.name } }}
-          >
+        <H5GroveProvider url={props.url} filepath={props.name} axiosConfig={{ params: { file: props.name } }}>
             <App explorerOpen={props.explorer} />
-          </H5GroveProvider>
-      );
+        </H5GroveProvider>
+    );
 }
 
 window.bundleEntries = window.bundleEntries || {};
 window.bundleEntries.load = function (options) {
-    var dataset = options.dataset;
-    var settings = options.chart.settings;
-    var explorer = settings.get('explorer');
-    var url = window.location.origin + "/api/datasets/" + dataset.id + "/content";
-    reactRender(
-      <MyApp 
-        url={url} 
-        name={dataset.name} 
-        filepath={dataset.file_name} 
-        explorer={explorer}
-      />,
-      document.getElementById(options.target)
-    )
-    options.chart.state('ok', 'Chart drawn.');
+    const dataset = options.dataset;
+    const settings = options.chart.settings;
+    const explorer = settings.get("explorer");
+    const url = window.location.origin + "/api/datasets/" + dataset.id + "/content";
+    const root = createRoot(document.getElementById(options.target));
+    root.render(<MyApp url={url} name={dataset.name} filepath={dataset.file_name} explorer={explorer} />);
+    options.chart.state("ok", "Chart drawn.");
     options.process.resolve();
 };
 

From a6cd891a2307cb1b2a9ba183bcad41ddfbdf828e Mon Sep 17 00:00:00 2001
From: Dannon Baker <dannon.baker@gmail.com>
Date: Thu, 3 Aug 2023 08:42:34 -0400
Subject: [PATCH 31/41] Add explicit dependency for buffer polyfill

---
 config/plugins/visualizations/h5web/package.json | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config/plugins/visualizations/h5web/package.json b/config/plugins/visualizations/h5web/package.json
index b8cdef990ec4..bf378c84849c 100644
--- a/config/plugins/visualizations/h5web/package.json
+++ b/config/plugins/visualizations/h5web/package.json
@@ -9,6 +9,7 @@
     "license": "AFL-3.0",
     "dependencies": {
         "@h5web/app": "^8.0.0",
+        "buffer": "^6.0.3",
         "normalize.css": "^8.0.1",
         "react": "18.2.0",
         "react-dom": "18.2.0"

From 3f036b378cd0fe3367401db2f2aa87d160dd72d5 Mon Sep 17 00:00:00 2001
From: hujambo-dunia <michelle.savage.1@gmail.com>
Date: Thu, 3 Aug 2023 15:23:00 -0400
Subject: [PATCH 32/41] For Chrome browsers, specify font-smoothing to ensure
 font-weight parameter is distinct from normal text. Especially visible in the
 Tool Form pages (Tools) where <P>aragraph and <Strong> tags are widely used.

---
 client/src/style/scss/base.scss | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/client/src/style/scss/base.scss b/client/src/style/scss/base.scss
index 2a3cb110d1cc..3d4cecc08463 100644
--- a/client/src/style/scss/base.scss
+++ b/client/src/style/scss/base.scss
@@ -1,3 +1,5 @@
+* {-webkit-font-smoothing: antialiased;}
+
 // Bootstrap variables and core framework
 @import "~bootstrap/scss/_functions.scss";
 @import "theme/blue.scss";

From 4868e595f954f6d2a2140ba11a717f22a2c1e2e9 Mon Sep 17 00:00:00 2001
From: hujambo-dunia <michelle.savage.1@gmail.com>
Date: Thu, 3 Aug 2023 16:08:25 -0400
Subject: [PATCH 33/41] For Chrome browsers, specify font-smoothing to ensure
 font-weight parameter is distinct from normal text. Especially visible in the
 Tool Form pages (Tools) where <P>aragraph and <Strong> tags are widely used.
 Plus linting.

---
 client/src/style/scss/base.scss | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/client/src/style/scss/base.scss b/client/src/style/scss/base.scss
index 3d4cecc08463..9f45f509ba6a 100644
--- a/client/src/style/scss/base.scss
+++ b/client/src/style/scss/base.scss
@@ -1,4 +1,6 @@
-* {-webkit-font-smoothing: antialiased;}
+* {
+    -webkit-font-smoothing: antialiased;
+}
 
 // Bootstrap variables and core framework
 @import "~bootstrap/scss/_functions.scss";

From e839b63b07ae437427e16fae69739d6e43c5f6b3 Mon Sep 17 00:00:00 2001
From: hujambo-dunia <michelle.savage.1@gmail.com>
Date: Fri, 4 Aug 2023 15:45:29 -0400
Subject: [PATCH 34/41] Fix for Accessibility, on Workflows List page Tooltip
 when hover over Workflow Title; Fix previously in place but likely remoed
 from bad merge.

---
 .../components/Workflow/WorkflowDropdown.vue  | 28 +++++++++----------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/client/src/components/Workflow/WorkflowDropdown.vue b/client/src/components/Workflow/WorkflowDropdown.vue
index 5698d0e1fb95..6eadac556402 100644
--- a/client/src/components/Workflow/WorkflowDropdown.vue
+++ b/client/src/components/Workflow/WorkflowDropdown.vue
@@ -10,21 +10,21 @@
             @dragend="onDragEnd">
             <Icon icon="caret-down" class="fa-lg" />
             <span class="workflow-dropdown-name">{{ workflow.name }}</span>
+            <span
+                v-if="sourceType.includes('trs')"
+                v-b-tooltip.hover
+                aria-haspopup="true"
+                :title="getWorkflowTooltip(sourceType, workflow)">
+                <Icon fixed-width icon="check" class="mr-1 workflow-trs-icon" />
+            </span>
+            <span
+                v-if="sourceType == 'url'"
+                v-b-tooltip.hover
+                aria-haspopup="true"
+                :title="getWorkflowTooltip(sourceType, workflow)">
+                <Icon fixed-width icon="link" class="mr-1 workflow-external-link" />
+            </span>
         </b-link>
-        <span
-            v-if="sourceType.includes('trs')"
-            v-b-tooltip.hover
-            aria-haspopup="true"
-            :title="getWorkflowTooltip(sourceType, workflow)">
-            <Icon fixed-width icon="check" class="mr-1 workflow-trs-icon" />
-        </span>
-        <span
-            v-if="sourceType == 'url'"
-            v-b-tooltip.hover
-            aria-haspopup="true"
-            :title="getWorkflowTooltip(sourceType, workflow)">
-            <Icon fixed-width icon="link" class="mr-1 workflow-external-link" />
-        </span>
         <p v-if="workflow.description" class="workflow-dropdown-description">
             <TextSummary :description="workflow.description" :show-details.sync="showDetails" />
         </p>

From 30983d50d656b70cc77aaa3a5cb0b16a5bcdc0c2 Mon Sep 17 00:00:00 2001
From: hujambo-dunia <michelle.savage.1@gmail.com>
Date: Sun, 6 Aug 2023 18:59:03 -0400
Subject: [PATCH 35/41] Reference added to apply the Bold/700 font-weight for
 Atkinson-Hyperlegible to website

---
 client/src/components/Masthead/Masthead.vue | 1 +
 client/src/style/scss/base.scss             | 4 ----
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/client/src/components/Masthead/Masthead.vue b/client/src/components/Masthead/Masthead.vue
index 8c8e86ac2cdc..eab1fd57ad46 100644
--- a/client/src/components/Masthead/Masthead.vue
+++ b/client/src/components/Masthead/Masthead.vue
@@ -148,6 +148,7 @@ onMounted(() => {
 
 <style scoped lang="scss">
 @import "theme/blue.scss";
+@import "@fontsource/atkinson-hyperlegible/700.css";
 
 #masthead {
     padding: 0;
diff --git a/client/src/style/scss/base.scss b/client/src/style/scss/base.scss
index 9f45f509ba6a..2a3cb110d1cc 100644
--- a/client/src/style/scss/base.scss
+++ b/client/src/style/scss/base.scss
@@ -1,7 +1,3 @@
-* {
-    -webkit-font-smoothing: antialiased;
-}
-
 // Bootstrap variables and core framework
 @import "~bootstrap/scss/_functions.scss";
 @import "theme/blue.scss";

From 13a3c36e1081ba68564efe5de7fd171b486fdfc8 Mon Sep 17 00:00:00 2001
From: hujambo-dunia <michelle.savage.1@gmail.com>
Date: Mon, 7 Aug 2023 16:06:39 -0400
Subject: [PATCH 36/41] Relocation of Font style reference to parent-file

---
 client/src/components/Masthead/Masthead.vue | 1 -
 client/src/entry/analysis/App.vue           | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/client/src/components/Masthead/Masthead.vue b/client/src/components/Masthead/Masthead.vue
index eab1fd57ad46..8c8e86ac2cdc 100644
--- a/client/src/components/Masthead/Masthead.vue
+++ b/client/src/components/Masthead/Masthead.vue
@@ -148,7 +148,6 @@ onMounted(() => {
 
 <style scoped lang="scss">
 @import "theme/blue.scss";
-@import "@fontsource/atkinson-hyperlegible/700.css";
 
 #masthead {
     padding: 0;
diff --git a/client/src/entry/analysis/App.vue b/client/src/entry/analysis/App.vue
index 5d9d818f0d75..a35dfa0d3d1a 100644
--- a/client/src/entry/analysis/App.vue
+++ b/client/src/entry/analysis/App.vue
@@ -173,5 +173,6 @@ export default {
 </script>
 
 <style lang="scss">
+@import "@fontsource/atkinson-hyperlegible/700.css";
 @import "custom_theme_variables.scss";
 </style>

From 0fd3df419a89d49cd279f0568b023f9b4798ed85 Mon Sep 17 00:00:00 2001
From: hujambo-dunia <michelle.savage.1@gmail.com>
Date: Tue, 8 Aug 2023 09:14:52 -0400
Subject: [PATCH 37/41] Relocation of Font style reference to parent-file which
 also follows conventions

---
 client/src/entry/analysis/App.vue | 1 -
 client/src/onload/index.js        | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/client/src/entry/analysis/App.vue b/client/src/entry/analysis/App.vue
index a35dfa0d3d1a..5d9d818f0d75 100644
--- a/client/src/entry/analysis/App.vue
+++ b/client/src/entry/analysis/App.vue
@@ -173,6 +173,5 @@ export default {
 </script>
 
 <style lang="scss">
-@import "@fontsource/atkinson-hyperlegible/700.css";
 @import "custom_theme_variables.scss";
 </style>
diff --git a/client/src/onload/index.js b/client/src/onload/index.js
index e47fb8113168..ce8da61e9526 100644
--- a/client/src/onload/index.js
+++ b/client/src/onload/index.js
@@ -11,6 +11,7 @@ import "./publicPath";
 
 // Default Font
 import "@fontsource/atkinson-hyperlegible";
+import "@fontsource/atkinson-hyperlegible/700.css";
 
 import { overrideProductionConsole } from "./console";
 

From 91d4d886ef4da9fd83873dddfcb79c8d15ce7d35 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 8 Aug 2023 21:25:58 +0200
Subject: [PATCH 38/41] Fix double-encoding notification content

---
 lib/galaxy/managers/notification.py |  2 +-
 lib/galaxy/model/__init__.py        |  5 ++++-
 lib/galaxy/model/custom_types.py    | 11 +++++++++++
 lib/galaxy/schema/notifications.py  | 19 -------------------
 4 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/lib/galaxy/managers/notification.py b/lib/galaxy/managers/notification.py
index 87b75db17139..1013f2d05d86 100644
--- a/lib/galaxy/managers/notification.py
+++ b/lib/galaxy/managers/notification.py
@@ -323,7 +323,7 @@ def _create_notification_model(self, payload: NotificationCreateData):
             payload.source,
             payload.category,
             payload.variant,
-            payload.content.json(),
+            payload.content.dict(),
         )
         notification.publication_time = payload.publication_time
         notification.expiration_time = payload.expiration_time
diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py
index 44d031ed14c4..52e5c48faea3 100644
--- a/lib/galaxy/model/__init__.py
+++ b/lib/galaxy/model/__init__.py
@@ -119,6 +119,7 @@
 import galaxy.util
 from galaxy.model.base import transaction
 from galaxy.model.custom_types import (
+    DoubleEncodedJsonType,
     JSONType,
     MetadataType,
     MutableJSONType,
@@ -2788,7 +2789,9 @@ class Notification(Base, Dictifiable, RepresentById):
     variant = Column(
         String(16), index=True
     )  # Defines the 'importance' of the notification ('info', 'warning', 'urgent', etc.). Used for filtering, highlight rendering, etc
-    content = Column(JSONType)  # Structured content in JSON. Depending on the category of the notification
+    # A bug in early 23.1 led to values being stored as json string, so we use this special type to process the result value twice.
+    # content should always be a dict
+    content = Column(DoubleEncodedJsonType)
 
     user_notification_associations = relationship("UserNotificationAssociation", back_populates="notification")
 
diff --git a/lib/galaxy/model/custom_types.py b/lib/galaxy/model/custom_types.py
index 8f292a792a43..8152802adff8 100644
--- a/lib/galaxy/model/custom_types.py
+++ b/lib/galaxy/model/custom_types.py
@@ -116,6 +116,17 @@ def compare_values(self, x, y):
         return x == y
 
 
+class DoubleEncodedJsonType(JSONType):
+    def process_result_value(self, value, dialect):
+        value = super().process_result_value(value, dialect)
+        if isinstance(value, str):
+            try:
+                return json.loads(value)
+            except ValueError:
+                return value
+        return value
+
+
 class MutableJSONType(JSONType):
     """Associated with MutationObj"""
 
diff --git a/lib/galaxy/schema/notifications.py b/lib/galaxy/schema/notifications.py
index 3b5c422592cb..30bb8c1aa1a9 100644
--- a/lib/galaxy/schema/notifications.py
+++ b/lib/galaxy/schema/notifications.py
@@ -1,8 +1,6 @@
-import json
 from datetime import datetime
 from enum import Enum
 from typing import (
-    Any,
     Dict,
     List,
     Optional,
@@ -14,7 +12,6 @@
     Field,
     Required,
 )
-from pydantic.utils import GetterDict
 from typing_extensions import (
     Annotated,
     Literal,
@@ -125,21 +122,6 @@ class NewSharedItemNotificationContent(Model):
     ),
 ]
 
-
-class NotificationGetter(GetterDict):
-    """Helper to convert a Notification ORM model into a NotificationResponse.
-    For more information: https://docs.pydantic.dev/usage/models/#data-binding
-    """
-
-    def get(self, key: Any, default: Any = None) -> Any:
-        # The `content` column of the ORM model is a JSON string that needs to be passed
-        # as a dictionary to the `from_orm` constructor to build the correct `AnyNotificationContent`.
-        if key in {"content"} and isinstance(self._obj.content, str):
-            return json.loads(self._obj.content)
-
-        return super().get(key, default)
-
-
 NotificationIdField = Field(
     Required,
     title="ID",
@@ -204,7 +186,6 @@ class NotificationResponse(Model):
 
     class Config:
         orm_mode = True
-        getter_dict = NotificationGetter
 
 
 class UserNotificationResponse(NotificationResponse):

From c88458d081d50fc486d6564846f04a7679911441 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Wed, 9 Aug 2023 08:59:55 +0200
Subject: [PATCH 39/41] Adjust notification unit tests

---
 .../unit/app/managers/test_NotificationManager.py | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/test/unit/app/managers/test_NotificationManager.py b/test/unit/app/managers/test_NotificationManager.py
index 19403545c431..99d493b3ef3b 100644
--- a/test/unit/app/managers/test_NotificationManager.py
+++ b/test/unit/app/managers/test_NotificationManager.py
@@ -1,4 +1,3 @@
-import json
 from datetime import (
     datetime,
     timedelta,
@@ -109,7 +108,7 @@ def _assert_notification_expected(self, actual_notification: Any, expected_notif
             assert actual_notification.expiration_time is not None
 
         assert actual_notification.content
-        user_notification_content = json.loads(actual_notification.content)
+        user_notification_content = actual_notification.content
         assert user_notification_content["category"] == expected_notification["content"]["category"]
         assert user_notification_content["subject"] == expected_notification["content"]["subject"]
         assert user_notification_content["message"] == expected_notification["content"]["message"]
@@ -122,7 +121,7 @@ def test_create_notification_broadcast(self):
 
         assert actual_notification.id
         assert actual_notification.category == "broadcast"
-        notification_content = json.loads(actual_notification.content)
+        notification_content = actual_notification.content
         assert notification_content["category"] == "broadcast"
         assert notification_content["subject"] == "Testing Broadcast Subject"
         assert notification_content["message"] == "Testing Broadcast Message"
@@ -161,16 +160,16 @@ def test_get_all_broadcasted_notifications(self):
 
         notifications = self.notification_manager.get_all_broadcasted_notifications()
         assert len(notifications) == 1
-        assert json.loads(notifications[0].content)["subject"] == "Recent Notification"
+        assert notifications[0].content["subject"] == "Recent Notification"
 
         notifications = self.notification_manager.get_all_broadcasted_notifications(since=next_week, active_only=False)
         assert len(notifications) == 2
-        assert json.loads(notifications[0].content)["subject"] == "Scheduled Next Week Notification"
-        assert json.loads(notifications[1].content)["subject"] == "Scheduled Next Month Notification"
+        assert notifications[0].content["subject"] == "Scheduled Next Week Notification"
+        assert notifications[1].content["subject"] == "Scheduled Next Month Notification"
 
         notifications = self.notification_manager.get_all_broadcasted_notifications(since=next_month, active_only=False)
         assert len(notifications) == 1
-        assert json.loads(notifications[0].content)["subject"] == "Scheduled Next Month Notification"
+        assert notifications[0].content["subject"] == "Scheduled Next Month Notification"
 
     def test_update_broadcasted_notification(self):
         next_month = datetime.utcnow() + timedelta(days=30)
@@ -195,7 +194,7 @@ def test_update_broadcasted_notification(self):
         assert updated_notification.source == update_request.source
         assert updated_notification.variant == update_request.variant
         assert updated_notification.publication_time == update_request.publication_time
-        content = json.loads(updated_notification.content)
+        content = updated_notification.content
         assert content["subject"] == expected_content.subject
         assert content["message"] == expected_content.message
 

From 35e47388e76769b78488dbff28b61af692fe0043 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Wed, 9 Aug 2023 09:12:02 +0200
Subject: [PATCH 40/41] Exclude selenium version with broken type hints

---
 packages/selenium/setup.cfg      | 2 +-
 packages/test_selenium/setup.cfg | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/selenium/setup.cfg b/packages/selenium/setup.cfg
index 8432f9e7fad1..a95b3a449dfa 100644
--- a/packages/selenium/setup.cfg
+++ b/packages/selenium/setup.cfg
@@ -37,7 +37,7 @@ install_requires =
     axe-selenium-python
     PyYAML
     requests
-    selenium
+    selenium!=4.11.0,!=4.11.1,!=4.11.2
 packages = find:
 python_requires = >=3.7
 
diff --git a/packages/test_selenium/setup.cfg b/packages/test_selenium/setup.cfg
index 086d0243571a..dca41568d040 100644
--- a/packages/test_selenium/setup.cfg
+++ b/packages/test_selenium/setup.cfg
@@ -41,7 +41,7 @@ install_requires =
     pytest
     PyYAML
     requests
-    selenium
+    selenium!=4.11.0,!=4.11.1,!=4.11.2
 packages = find:
 python_requires = >=3.7
 

From 130d2b078e31acbaf86d843aa16d5a415235856d Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Wed, 9 Aug 2023 16:50:10 +0200
Subject: [PATCH 41/41] Update and apply ruff refactorings

ruff 0.280.0 has a bug that would turn the imports guarded by `if
TYPE_CHECKING` into their non-string variant, which would break the
runtime. So we can't merge 23.1 into dev without this.
---
 lib/galaxy/datatypes/sequence.py              |  4 +--
 .../dependencies/pinned-lint-requirements.txt |  2 +-
 lib/galaxy/managers/citations.py              |  8 +++---
 lib/galaxy/managers/cloud.py                  | 10 +++----
 lib/galaxy/objectstore/pithos.py              | 10 ++-----
 lib/galaxy/queues.py                          |  3 +--
 lib/galaxy/tool_util/deps/brew_exts.py        |  8 +++---
 lib/galaxy/util/__init__.py                   |  2 +-
 lib/galaxy/util/commands.py                   |  8 +++---
 lib/galaxy/webapps/base/webapp.py             |  2 +-
 lib/galaxy/webapps/galaxy/api/cloudauthz.py   |  3 +--
 .../webapps/galaxy/api/library_contents.py    |  4 +--
 lib/galaxy/webapps/galaxy/api/users.py        |  2 +-
 .../webapps/galaxy/controllers/authnz.py      |  4 +--
 lib/galaxy_test/api/test_drs.py               |  2 +-
 scripts/api/common.py                         |  2 +-
 test/integration/test_cli_runners.py          |  2 +-
 test/unit/config/test_load_config.py          | 26 +++++++++----------
 test/unit/config/test_schema.py               |  8 +++---
 19 files changed, 50 insertions(+), 60 deletions(-)

diff --git a/lib/galaxy/datatypes/sequence.py b/lib/galaxy/datatypes/sequence.py
index 5fe01c38f9fc..89746eec5c3e 100644
--- a/lib/galaxy/datatypes/sequence.py
+++ b/lib/galaxy/datatypes/sequence.py
@@ -175,7 +175,7 @@ def do_slow_split(cls, input_datasets, subdir_generator_function, split_params):
     def do_fast_split(cls, input_datasets, toc_file_datasets, subdir_generator_function, split_params):
         data = json.load(open(toc_file_datasets[0].file_name))
         sections = data["sections"]
-        total_sequences = int(0)
+        total_sequences = 0
         for section in sections:
             total_sequences += int(section["sequences"])
         sequences_per_file = cls.get_sequences_per_file(total_sequences, split_params)
@@ -247,7 +247,7 @@ def get_split_commands_with_toc(
         sections = toc_file["sections"]
         result = []
 
-        current_sequence = int(0)
+        current_sequence = 0
         i = 0
         # skip to the section that contains my starting sequence
         while i < len(sections) and start_sequence >= current_sequence + int(sections[i]["sequences"]):
diff --git a/lib/galaxy/dependencies/pinned-lint-requirements.txt b/lib/galaxy/dependencies/pinned-lint-requirements.txt
index 0685b383b397..4349574508b8 100644
--- a/lib/galaxy/dependencies/pinned-lint-requirements.txt
+++ b/lib/galaxy/dependencies/pinned-lint-requirements.txt
@@ -5,6 +5,6 @@ importlib-metadata==4.2.0
 mccabe==0.7.0
 pycodestyle==2.9.1
 pyflakes==2.5.0
-ruff==0.0.280
+ruff==0.0.283
 typing_extensions==4.7.1
 zipp==3.15.0
diff --git a/lib/galaxy/managers/citations.py b/lib/galaxy/managers/citations.py
index 64e8ca91754d..09a4e137cbee 100644
--- a/lib/galaxy/managers/citations.py
+++ b/lib/galaxy/managers/citations.py
@@ -152,12 +152,10 @@ def to_bibtex(self):
                 log.exception("Failed to fetch bibtex for DOI %s", self.__doi)
 
         if self.raw_bibtex is DoiCitation.BIBTEX_UNSET:
-            return """@MISC{{{doi},
-                DOI = {{{doi}}},
+            return f"""@MISC{{{self.__doi},
+                DOI = {{{self.__doi}}},
                 note = {{Failed to fetch BibTeX for DOI.}}
-            }}""".format(
-                doi=self.__doi
-            )
+            }}"""
         else:
             return self.raw_bibtex
 
diff --git a/lib/galaxy/managers/cloud.py b/lib/galaxy/managers/cloud.py
index 671ec216989e..2a26640c9cee 100644
--- a/lib/galaxy/managers/cloud.py
+++ b/lib/galaxy/managers/cloud.py
@@ -86,7 +86,7 @@ def configure_provider(provider, credentials):
             if len(missing_credentials) > 0:
                 raise RequestParameterMissingException(
                     "The following required key(s) are missing from the provided "
-                    "credentials object: {}".format(missing_credentials)
+                    f"credentials object: {missing_credentials}"
                 )
             session_token = credentials.get("SessionToken")
             config = {"aws_access_key": access, "aws_secret_key": secret, "aws_session_token": session_token}
@@ -107,7 +107,7 @@ def configure_provider(provider, credentials):
             if len(missing_credentials) > 0:
                 raise RequestParameterMissingException(
                     "The following required key(s) are missing from the provided "
-                    "credentials object: {}".format(missing_credentials)
+                    f"credentials object: {missing_credentials}"
                 )
 
             config = {
@@ -145,7 +145,7 @@ def configure_provider(provider, credentials):
             if len(missing_credentials) > 0:
                 raise RequestParameterMissingException(
                     "The following required key(s) are missing from the provided "
-                    "credentials object: {}".format(missing_credentials)
+                    f"credentials object: {missing_credentials}"
                 )
             config = {
                 "os_username": username,
@@ -161,8 +161,8 @@ def configure_provider(provider, credentials):
             connection = CloudProviderFactory().create_provider(ProviderList.GCP, config)
         else:
             raise RequestParameterInvalidException(
-                "Unrecognized provider '{}'; the following are the supported "
-                "providers: {}.".format(provider, SUPPORTED_PROVIDERS.keys())
+                f"Unrecognized provider '{provider}'; the following are the supported "
+                f"providers: {SUPPORTED_PROVIDERS.keys()}."
             )
 
         # The authorization-assertion mechanism of Cloudbridge assumes a user has an elevated privileges,
diff --git a/lib/galaxy/objectstore/pithos.py b/lib/galaxy/objectstore/pithos.py
index 63b81bbc2815..6dfa71b239fb 100644
--- a/lib/galaxy/objectstore/pithos.py
+++ b/lib/galaxy/objectstore/pithos.py
@@ -320,10 +320,7 @@ def _size(self, obj, **kwargs) -> int:
             try:
                 return os.path.getsize(self._get_cache_path(path))
             except OSError as ex:
-                log.warning(
-                    "Could not get size of file {path} in local cache,"
-                    "will try Pithos. Error: {err}".format(path=path, err=ex)
-                )
+                log.warning(f"Could not get size of file {path} in local cache," f"will try Pithos. Error: {ex}")
         try:
             file = self.pithos.get_object_info(path)
         except ClientError as ce:
@@ -406,10 +403,7 @@ def _update_from_file(self, obj, **kwargs):
         if kwargs.get("create"):
             self._create(obj, **kwargs)
         if not self._exists(obj, **kwargs):
-            raise ObjectNotFound(
-                "objectstore.update_from_file, object does not exist: {obj}, "
-                "kwargs: {kwargs}".format(obj=obj, kwargs=kwargs)
-            )
+            raise ObjectNotFound(f"objectstore.update_from_file, object does not exist: {obj}, " f"kwargs: {kwargs}")
 
         path = self._construct_path(obj, **kwargs)
         cache_path = self._get_cache_path(path)
diff --git a/lib/galaxy/queues.py b/lib/galaxy/queues.py
index 75fd4fa5fb53..d68012beb5c7 100644
--- a/lib/galaxy/queues.py
+++ b/lib/galaxy/queues.py
@@ -23,8 +23,7 @@ def all_control_queues_for_declare(application_stack):
     """
     # Get all active processes and construct queues for each process
     process_names = (
-        "{p.server_name}@{p.hostname}".format(p=p)
-        for p in application_stack.app.database_heartbeat.get_active_processes()
+        f"{p.server_name}@{p.hostname}" for p in application_stack.app.database_heartbeat.get_active_processes()
     )
     return [Queue(f"control.{server_name}", galaxy_exchange, routing_key="control.*") for server_name in process_names]
 
diff --git a/lib/galaxy/tool_util/deps/brew_exts.py b/lib/galaxy/tool_util/deps/brew_exts.py
index 7e8b67991678..852d7425a908 100755
--- a/lib/galaxy/tool_util/deps/brew_exts.py
+++ b/lib/galaxy/tool_util/deps/brew_exts.py
@@ -160,14 +160,14 @@ def __init__(self, command, stdout, stderr):
         self.stdout = stdout
         self.stderr = stderr
         self.message = (
-            "Failed to execute command-line {}, stderr was:\n"
+            f"Failed to execute command-line {command}, stderr was:\n"
             "-------->>begin stderr<<--------\n"
-            "{}\n"
+            f"{stderr}\n"
             "-------->>end stderr<<--------\n"
             "-------->>begin stdout<<--------\n"
-            "{}\n"
+            f"{stdout}\n"
             "-------->>end stdout<<--------\n"
-        ).format(command, stderr, stdout)
+        )
 
     def __str__(self):
         return self.message
diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py
index 7d3df0d4ded1..680df532d56c 100644
--- a/lib/galaxy/util/__init__.py
+++ b/lib/galaxy/util/__init__.py
@@ -544,7 +544,7 @@ def pretty_print_time_interval(time=False, precise=False, utc=False):
         now = datetime.utcnow()
     else:
         now = datetime.now()
-    if type(time) is int:
+    if isinstance(time, (int, float)):
         diff = now - datetime.fromtimestamp(time)
     elif isinstance(time, datetime):
         diff = now - time
diff --git a/lib/galaxy/util/commands.py b/lib/galaxy/util/commands.py
index 679aba33f4f4..321dd71f2fad 100644
--- a/lib/galaxy/util/commands.py
+++ b/lib/galaxy/util/commands.py
@@ -155,14 +155,14 @@ def __init__(self, command, stdout, stderr, returncode):
         self.stderr = stderr
         self.returncode = returncode
         self.message = (
-            "Failed to execute command-line {}, stderr was:\n"
+            f"Failed to execute command-line {command}, stderr was:\n"
             "-------->>begin stderr<<--------\n"
-            "{}\n"
+            f"{stderr}\n"
             "-------->>end stderr<<--------\n"
             "-------->>begin stdout<<--------\n"
-            "{}\n"
+            f"{stdout}\n"
             "-------->>end stdout<<--------\n"
-        ).format(command, stderr, stdout)
+        )
 
     def __str__(self):
         """Return a verbose error message indicating the command problem."""
diff --git a/lib/galaxy/webapps/base/webapp.py b/lib/galaxy/webapps/base/webapp.py
index 4e2cac6e5086..93ca937e0484 100644
--- a/lib/galaxy/webapps/base/webapp.py
+++ b/lib/galaxy/webapps/base/webapp.py
@@ -147,7 +147,7 @@ def build_apispec(self):
             methods = []
             if rule.conditions:
                 m = rule.conditions.get("method", [])
-                methods = type(m) is str and [m] or m
+                methods = [m] if isinstance(m, str) else m
             # Find the controller class
             if controller not in self.api_controllers:
                 # Only happens when removing a controller after porting to FastAPI.
diff --git a/lib/galaxy/webapps/galaxy/api/cloudauthz.py b/lib/galaxy/webapps/galaxy/api/cloudauthz.py
index 36616b095afc..fb0227f1f906 100644
--- a/lib/galaxy/webapps/galaxy/api/cloudauthz.py
+++ b/lib/galaxy/webapps/galaxy/api/cloudauthz.py
@@ -124,8 +124,7 @@ def create(self, trans, payload, **kwargs):
         if not isinstance(config, dict):
             log.debug(msg_template.format(f"invalid config type `{type(config)}`, expect `dict`"))
             raise RequestParameterInvalidException(
-                "Invalid type for the required `config` variable; expect `dict` "
-                "but received `{}`.".format(type(config))
+                "Invalid type for the required `config` variable; expect `dict` " f"but received `{type(config)}`."
             )
         if authn_id:
             try:
diff --git a/lib/galaxy/webapps/galaxy/api/library_contents.py b/lib/galaxy/webapps/galaxy/api/library_contents.py
index e3c5ecce3ee1..a0e21e1ad6d6 100644
--- a/lib/galaxy/webapps/galaxy/api/library_contents.py
+++ b/lib/galaxy/webapps/galaxy/api/library_contents.py
@@ -371,9 +371,9 @@ def _upload_library_dataset(self, trans, folder_id: int, **kwd):
                 trans, folder_id=folder.id, replace_dataset=replace_dataset, **kwd
             )
             if created_outputs_dict:
-                if type(created_outputs_dict) == str:
+                if isinstance(created_outputs_dict, str):
                     return 400, created_outputs_dict
-                elif type(created_outputs_dict) == tuple:
+                elif isinstance(created_outputs_dict, tuple):
                     return created_outputs_dict[0], created_outputs_dict[1]
                 return 200, created_outputs_dict
             else:
diff --git a/lib/galaxy/webapps/galaxy/api/users.py b/lib/galaxy/webapps/galaxy/api/users.py
index cd2768f5e13c..3cbbb57751c2 100644
--- a/lib/galaxy/webapps/galaxy/api/users.py
+++ b/lib/galaxy/webapps/galaxy/api/users.py
@@ -1087,7 +1087,7 @@ def set_toolbox_filters(self, trans, id, payload=None, **kwd):
             for prefixed_name in payload:
                 if prefixed_name.startswith(filter_type):
                     filter_selection = payload.get(prefixed_name)
-                    if type(filter_selection) != bool:
+                    if not isinstance(filter_selection, bool):
                         raise exceptions.RequestParameterInvalidException(
                             "Please specify the filter selection as boolean value."
                         )
diff --git a/lib/galaxy/webapps/galaxy/controllers/authnz.py b/lib/galaxy/webapps/galaxy/controllers/authnz.py
index 71bd0fbcf7e4..88af61d8fa11 100644
--- a/lib/galaxy/webapps/galaxy/controllers/authnz.py
+++ b/lib/galaxy/webapps/galaxy/controllers/authnz.py
@@ -101,9 +101,9 @@ def callback(self, trans, provider, idphint=None, **kwargs):
                 " Error message: {}".format(provider, user, kwargs.get("error", "None"))
             )
             return trans.show_error_message(
-                "Failed to handle authentication callback from {}. "
+                f"Failed to handle authentication callback from {provider}. "
                 "Please try again, and if the problem persists, contact "
-                "the Galaxy instance admin".format(provider)
+                "the Galaxy instance admin"
             )
         try:
             success, message, (redirect_url, user) = trans.app.authnz_manager.callback(
diff --git a/lib/galaxy_test/api/test_drs.py b/lib/galaxy_test/api/test_drs.py
index c221e486a03c..00921aca35fe 100644
--- a/lib/galaxy_test/api/test_drs.py
+++ b/lib/galaxy_test/api/test_drs.py
@@ -130,7 +130,7 @@ def test_exception_handling_schema(self):
         for method in HTTP_METHODS:
             api_url = self._url_join(f"ga4gh/drs/v1/objects/{drs_id}/access/fakeid")
             error_response = method(api_url)
-            assert type(error_response.status_code) == int
+            assert isinstance(error_response.status_code, int)
             assert error_response.status_code == 404
             error_as_dict = error_response.json()
             assert "status_code" in error_as_dict
diff --git a/scripts/api/common.py b/scripts/api/common.py
index 1a99d4af6c32..1b5a09e44070 100644
--- a/scripts/api/common.py
+++ b/scripts/api/common.py
@@ -109,7 +109,7 @@ def display(api_key, url, return_formatted=True):
         print("------------------")
         for k, v in r.items():
             print(f"{k}: {v}")
-    elif type(r) == str:
+    elif isinstance(r, str):
         print(r)
     else:
         print("response is unknown type: %s" % type(r))
diff --git a/test/integration/test_cli_runners.py b/test/integration/test_cli_runners.py
index 3dd4ed613792..b240ebaf7cc3 100644
--- a/test/integration/test_cli_runners.py
+++ b/test/integration/test_cli_runners.py
@@ -43,7 +43,7 @@ def start_ssh_docker(container_name, jobs_directory, port=10022, image="agaveapi
         "--rm",
         "--privileged",  # for torque
         "-v",
-        "{jobs_directory}:{jobs_directory}".format(jobs_directory=jobs_directory),
+        f"{jobs_directory}:{jobs_directory}",
         "-v",
         f"{ssh_keys.public_key_file}:/home/testuser/.ssh/authorized_keys",
         "--ulimit",
diff --git a/test/unit/config/test_load_config.py b/test/unit/config/test_load_config.py
index 13e4cd8754d3..7ab70339437f 100644
--- a/test/unit/config/test_load_config.py
+++ b/test/unit/config/test_load_config.py
@@ -47,10 +47,10 @@ def test_load_config_from_schema(mock_init):
     assert config.property5 is None
     assert config.property6 is None
 
-    assert type(config.property1) is str
-    assert type(config.property2) is int
-    assert type(config.property3) is float
-    assert type(config.property4) is bool
+    assert isinstance(config.property1, str)
+    assert isinstance(config.property2, int)
+    assert isinstance(config.property3, float)
+    assert isinstance(config.property4, bool)
 
 
 def test_update_raw_config_from_kwargs(mock_init):
@@ -64,10 +64,10 @@ def test_update_raw_config_from_kwargs(mock_init):
     assert config._raw_config["property5"] is None  # no change
     assert config._raw_config["property6"] is None  # no change
 
-    assert type(config._raw_config["property1"]) is str
-    assert type(config._raw_config["property2"]) is int
-    assert type(config._raw_config["property3"]) is float
-    assert type(config._raw_config["property4"]) is bool
+    assert isinstance(config._raw_config["property1"], str)
+    assert isinstance(config._raw_config["property2"], int)
+    assert isinstance(config._raw_config["property3"], float)
+    assert isinstance(config._raw_config["property4"], bool)
 
 
 def test_update_raw_config_from_string_kwargs(mock_init):
@@ -80,10 +80,10 @@ def test_update_raw_config_from_string_kwargs(mock_init):
     assert config._raw_config["property3"] == 2.0  # updated
     assert config._raw_config["property4"] is False  # updated
 
-    assert type(config._raw_config["property1"]) is str
-    assert type(config._raw_config["property2"]) is int
-    assert type(config._raw_config["property3"]) is float
-    assert type(config._raw_config["property4"]) is bool
+    assert isinstance(config._raw_config["property1"], str)
+    assert isinstance(config._raw_config["property2"], int)
+    assert isinstance(config._raw_config["property3"], float)
+    assert isinstance(config._raw_config["property4"], bool)
 
 
 def test_update_raw_config_from_kwargs_with_none(mock_init):
@@ -111,7 +111,7 @@ def test_update_raw_config_from_kwargs_falsy_not_none(mock_init):
     config = BaseAppConfiguration(property1=0)
 
     assert config._raw_config["property1"] == "0"  # updated
-    assert type(config._raw_config["property1"]) is str  # and converted to str
+    assert isinstance(config._raw_config["property1"], str)  # and converted to str
 
 
 def test_unset_renamed_option_set_by_old_option(mock_init, monkeypatch):
diff --git a/test/unit/config/test_schema.py b/test/unit/config/test_schema.py
index 1b6b11b4c86a..820f74300e25 100644
--- a/test/unit/config/test_schema.py
+++ b/test/unit/config/test_schema.py
@@ -60,10 +60,10 @@ def mock_init(self, stream):
     assert schema.defaults["property5"] is None
     assert schema.defaults["property6"] is None
 
-    assert type(schema.defaults["property1"]) is str
-    assert type(schema.defaults["property2"]) is int
-    assert type(schema.defaults["property3"]) is float
-    assert type(schema.defaults["property4"]) is bool
+    assert isinstance(schema.defaults["property1"], str)
+    assert isinstance(schema.defaults["property2"], int)
+    assert isinstance(schema.defaults["property3"], float)
+    assert isinstance(schema.defaults["property4"], bool)
 
     assert len(schema.reloadable_options) == 2
     assert "property2" in schema.reloadable_options