From ba9d51ccffc2bfb60c88ed59686fe4e2053ad46e Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Tue, 26 Oct 2021 11:52:04 -0400
Subject: [PATCH 1/8] Progress toward a stronger typed manager layer.

ModelManager still needs to use generics to really start providing a lot of benefit.
---
 lib/galaxy/managers/base.py     |  8 ++++----
 lib/galaxy/managers/datasets.py |  2 +-
 lib/galaxy/managers/secured.py  | 14 +++++++++++++-
 lib/galaxy/managers/sharable.py | 19 ++++++++-----------
 lib/galaxy/model/__init__.py    |  2 +-
 setup.cfg                       |  4 ----
 6 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/lib/galaxy/managers/base.py b/lib/galaxy/managers/base.py
index 256e880e337..d4fe53b80fb 100644
--- a/lib/galaxy/managers/base.py
+++ b/lib/galaxy/managers/base.py
@@ -180,7 +180,7 @@ class ModelManager:
     Provides common queries and CRUD operations as a (hopefully) light layer
     over the ORM.
     """
-    model_class: type = object
+    model_class: Type[model._HasTable]
     foreign_key_name: str
     app: BasicApp
 
@@ -309,12 +309,12 @@ class ModelManager:
             return None
 
     # NOTE: at this layer, all ids are expected to be decoded and in int form
-    def by_id(self, id, **kwargs):
+    def by_id(self, id: int):
         """
         Gets a model by primary id.
         """
-        id_filter = self.model_class.id == id
-        return self.one(filters=id_filter, **kwargs)
+        id_filter = self.model_class.table.c.id == id
+        return self.one(filters=id_filter)
 
     # .... multirow queries
     def list(self, filters=None, order_by=None, limit=None, offset=None, **kwargs):
diff --git a/lib/galaxy/managers/datasets.py b/lib/galaxy/managers/datasets.py
index ab5890dba7d..f44ef54a732 100644
--- a/lib/galaxy/managers/datasets.py
+++ b/lib/galaxy/managers/datasets.py
@@ -235,7 +235,7 @@ class DatasetAssociationManager(base.ModelManager,
     """
     # DA's were meant to be proxies - but were never fully implemented as them
     # Instead, a dataset association HAS a dataset but contains metadata specific to a library (lda) or user (hda)
-    model_class: Type[model.DatasetInstance] = model.DatasetInstance
+    model_class: Type[model.DatasetInstance]
     app: MinimalManagerApp
 
     # NOTE: model_manager_class should be set in HDA/LDA subclasses
diff --git a/lib/galaxy/managers/secured.py b/lib/galaxy/managers/secured.py
index 7e7081d628c..70f37c6f22b 100644
--- a/lib/galaxy/managers/secured.py
+++ b/lib/galaxy/managers/secured.py
@@ -3,8 +3,9 @@ Accessible models can be read and copied but not modified or deleted.
 
 Owned models can be modified and deleted.
 """
+from typing import Type
 
-from galaxy import exceptions
+from galaxy import exceptions, model
 
 
 class AccessibleManagerMixin:
@@ -14,6 +15,12 @@ class AccessibleManagerMixin:
     This can also be thought of as 'read but not modify' privileges.
     """
 
+    # declare what we are using from base ModelManager
+    model_class: Type[model._HasTable]
+
+    def by_id(self, id: int):
+        ...
+
     # don't want to override by_id since consumers will also want to fetch w/o any security checks
     def is_accessible(self, item, user, **kwargs):
         """
@@ -74,6 +81,11 @@ class OwnableManagerMixin:
 
     This can also be thought of as write/edit privileges.
     """
+    # declare what we are using from base ModelManager
+    model_class: Type[model._HasTable]
+
+    def by_id(self, id: int):
+        ...
 
     def is_owner(self, item, user, **kwargs):
         """
diff --git a/lib/galaxy/managers/sharable.py b/lib/galaxy/managers/sharable.py
index 92250dfbd8d..1f74fc2472c 100644
--- a/lib/galaxy/managers/sharable.py
+++ b/lib/galaxy/managers/sharable.py
@@ -18,6 +18,7 @@ from typing import (
 )
 
 from sqlalchemy import (
+    func,
     true,
 )
 
@@ -66,7 +67,7 @@ class SharableModelManager(base.ModelManager, secured.OwnableManagerMixin, secur
         Return list for all items (of model_class type) associated with the given
         `user`.
         """
-        user_filter = self.model_class.user_id == user.id
+        user_filter = self.model_class.table.c.user_id == user.id
         filters = self._munge_filters(user_filter, filters)
         return self.list(filters=filters, **kwargs)
 
@@ -137,7 +138,7 @@ class SharableModelManager(base.ModelManager, secured.OwnableManagerMixin, secur
         """
         Return a query for all published items.
         """
-        published_filter = self.model_class.published == true()
+        published_filter = self.model_class.table.c.published == true()
         filters = self._munge_filters(published_filter, filters)
         return self.query(filters=filters, **kwargs)
 
@@ -145,7 +146,7 @@ class SharableModelManager(base.ModelManager, secured.OwnableManagerMixin, secur
         """
         Return a list of all published items.
         """
-        published_filter = self.model_class.published == true()
+        published_filter = self.model_class.table.c.published == true()
         filters = self._munge_filters(published_filter, filters)
         return self.list(filters=filters, **kwargs)
 
@@ -289,15 +290,11 @@ class SharableModelManager(base.ModelManager, secured.OwnableManagerMixin, secur
         VALID_SLUG_RE = re.compile(r"^[a-z0-9\-]+$")
         return VALID_SLUG_RE.match(slug)
 
-    def _existing_set_of_slugs(self, user):
-        query = (self.session().query(self.model_class.slug)
-                 .filter_by(user=user))
-        return list(set(query.all()))
-
     def _slug_exists(self, user, slug):
-        query = (self.session().query(self.model_class.slug)
-                 .filter_by(user=user, slug=slug))
-        return query.count() != 0
+        query = (self.session().query(self.model_class)
+                 .filter_by(user_id=user.id, slug=slug)
+                 .with_entities(func.count()))
+        return query.scalar() != 0
 
     def _slugify(self, start_with):
         # Replace whitespace with '-'
diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py
index 5668888c80d..0dd60773322 100644
--- a/lib/galaxy/model/__init__.py
+++ b/lib/galaxy/model/__init__.py
@@ -3463,7 +3463,7 @@ def datatype_for_extension(extension, datatypes_registry=None):
     return ret
 
 
-class DatasetInstance:
+class DatasetInstance(_HasTable):
     """A base class for all 'dataset instances', HDAs, LDAs, etc"""
     states = Dataset.states
     conversion_messages = Dataset.conversion_messages
diff --git a/setup.cfg b/setup.cfg
index 3bda14a42a1..fa666d6d9cf 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -305,8 +305,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.model.dataset_collections.matching]
 check_untyped_defs = False
-[mypy-galaxy.managers.secured]
-check_untyped_defs = False
 [mypy-galaxy.job_metrics.instrumenters.env]
 check_untyped_defs = False
 [mypy-galaxy.job_metrics.instrumenters.collectl]
@@ -579,8 +577,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.tools.actions.model_operations]
 check_untyped_defs = False
-[mypy-galaxy.managers.sharable]
-check_untyped_defs = False
 [mypy-galaxy.job_execution.output_collect]
 check_untyped_defs = False
 [mypy-galaxy.actions.library]

From 74a6d159135a89cfb869eb7d299ffe34dd33c15d Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Tue, 26 Oct 2021 12:08:03 -0400
Subject: [PATCH 2/8] Enable stricter type checking of galaxy.model.tags.

---
 lib/galaxy/model/__init__.py | 1 +
 lib/galaxy/model/tags.py     | 6 +++---
 setup.cfg                    | 2 --
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py
index 0dd60773322..79a445f754d 100644
--- a/lib/galaxy/model/__init__.py
+++ b/lib/galaxy/model/__init__.py
@@ -8116,6 +8116,7 @@ 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
 
     def __init_subclass__(cls, **kwargs):
         super().__init_subclass__(**kwargs)
diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py
index 0a4f4b81816..499d44ee06d 100644
--- a/lib/galaxy/model/tags.py
+++ b/lib/galaxy/model/tags.py
@@ -1,6 +1,6 @@
 import logging
 import re
-from typing import Dict
+from typing import Dict, List, Optional
 
 from sqlalchemy.orm.scoping import scoped_session
 from sqlalchemy.sql import select
@@ -341,13 +341,13 @@ class TagHandler:
             scrubbed_tag_list.append(self._scrub_tag_name(tag))
         return scrubbed_tag_list
 
-    def _get_name_value_pair(self, tag_str):
+    def _get_name_value_pair(self, tag_str) -> List[Optional[str]]:
         """Get name, value pair from a tag string."""
         # Use regular expression to parse name, value.
         if tag_str.startswith('#'):
             tag_str = f"name:{tag_str[1:]}"
         reg_exp = re.compile(f"[{self.key_value_separators}]")
-        name_value_pair = reg_exp.split(tag_str, 1)
+        name_value_pair: List[Optional[str]] = list(reg_exp.split(tag_str, 1))
         # Add empty slot if tag does not have value.
         if len(name_value_pair) < 2:
             name_value_pair.append(None)
diff --git a/setup.cfg b/setup.cfg
index fa666d6d9cf..a78f7bb2111 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -361,8 +361,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.model.tool_shed_install]
 check_untyped_defs = False
-[mypy-galaxy.model.tags]
-check_untyped_defs = False
 [mypy-galaxy.model.metadata]
 check_untyped_defs = False
 [mypy-galaxy.model]

From eb9b3e4485fe02c8f57999f665a0f414a6094001 Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Tue, 26 Oct 2021 12:52:08 -0400
Subject: [PATCH 3/8] Cleanup manager base stuff toward strong typing.

That raise didn't need to be a class method, it doesn't use self. And validate is always just a ModelValidator so type it as such.
---
 lib/galaxy/managers/base.py             |  9 ++++++---
 lib/galaxy/managers/history_contents.py | 13 +++++++------
 lib/galaxy/managers/taggable.py         |  3 ++-
 lib/galaxy/managers/tools.py            |  4 ++--
 4 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/lib/galaxy/managers/base.py b/lib/galaxy/managers/base.py
index d4fe53b80fb..7ddeadf5cbd 100644
--- a/lib/galaxy/managers/base.py
+++ b/lib/galaxy/managers/base.py
@@ -739,9 +739,11 @@ class ModelDeserializer(HasAModelManager):
     An object that converts an incoming serialized dict into values that can be
     directly assigned to an item's attributes and assigns them.
     """
+    validate: ModelValidator
+
     # TODO:?? a larger question is: which should be first? Deserialize then validate - or - validate then deserialize?
 
-    def __init__(self, app: MinimalManagerApp, validator=None, **kwargs):
+    def __init__(self, app: MinimalManagerApp, validator: Optional[ModelValidator] = None, **kwargs):
         """
         Set up deserializers and validator.
         """
@@ -1198,8 +1200,9 @@ class ModelFilterParser(HasAModelManager):
             return date_string
         raise ValueError('datetime strings must be in the ISO 8601 format and in the UTC')
 
-    def raise_filter_err(self, attr, op, val, msg):
-        raise exceptions.RequestParameterInvalidException(msg, column=attr, operation=op, val=val)
+
+def raise_filter_err(attr, op, val, msg):
+    raise exceptions.RequestParameterInvalidException(msg, column=attr, operation=op, val=val)
 
 
 def is_valid_slug(slug):
diff --git a/lib/galaxy/managers/history_contents.py b/lib/galaxy/managers/history_contents.py
index 1281655e7d1..323d6f6ae68 100644
--- a/lib/galaxy/managers/history_contents.py
+++ b/lib/galaxy/managers/history_contents.py
@@ -33,6 +33,7 @@ from galaxy.managers import (
     tools
 )
 from galaxy.structured_app import MinimalManagerApp
+from .base import raise_filter_err
 
 log = logging.getLogger(__name__)
 
@@ -483,14 +484,14 @@ class HistoryContentsFilters(base.ModelFilterParser,
             if attr == 'history_content_type' and op == 'eq':
                 if val in ('dataset', 'dataset_collection'):
                     return sql.column('history_content_type') == val
-                self.raise_filter_err(attr, op, val, 'bad op in filter')
+                raise_filter_err(attr, op, val, 'bad op in filter')
 
             if attr == 'type_id':
                 if op == 'eq':
                     return sql.column('type_id') == val
                 if op == 'in':
                     return sql.column('type_id').in_(self.parse_type_id_list(val))
-                self.raise_filter_err(attr, op, val, 'bad op in filter')
+                raise_filter_err(attr, op, val, 'bad op in filter')
 
             if attr in ('update_time', 'create_time'):
                 if op == 'ge':
@@ -501,21 +502,21 @@ class HistoryContentsFilters(base.ModelFilterParser,
                     return sql.column(attr) > self.parse_date(val)
                 if op == 'lt':
                     return sql.column(attr) < self.parse_date(val)
-                self.raise_filter_err(attr, op, val, 'bad op in filter')
+                raise_filter_err(attr, op, val, 'bad op in filter')
 
             if attr == 'state':
                 valid_states = model.Dataset.states.values()
                 if op == 'eq':
                     if val not in valid_states:
-                        self.raise_filter_err(attr, op, val, 'invalid state in filter')
+                        raise_filter_err(attr, op, val, 'invalid state in filter')
                     return sql.column('state') == val
                 if op == 'in':
                     states = [s for s in val.split(',') if s]
                     for state in states:
                         if state not in valid_states:
-                            self.raise_filter_err(attr, op, state, 'invalid state in filter')
+                            raise_filter_err(attr, op, state, 'invalid state in filter')
                     return sql.column('state').in_(states)
-                self.raise_filter_err(attr, op, val, 'bad op in filter')
+                raise_filter_err(attr, op, val, 'bad op in filter')
 
         column_filter = get_filter(attr, op, val)
         if column_filter is not None:
diff --git a/lib/galaxy/managers/taggable.py b/lib/galaxy/managers/taggable.py
index c4b8e0f1509..1c5be68e528 100644
--- a/lib/galaxy/managers/taggable.py
+++ b/lib/galaxy/managers/taggable.py
@@ -11,6 +11,7 @@ from sqlalchemy import sql
 
 from galaxy import model
 from galaxy.util import unicodify
+from .base import ModelValidator, raise_filter_err
 
 log = logging.getLogger(__name__)
 
@@ -102,7 +103,7 @@ class TaggableFilterMixin:
 
         def _create_tag_filter(model_class=None):
             if op not in TaggableFilterMixin.valid_ops:
-                self.raise_filter_err(attr, op, val, 'bad op in filter')
+                raise_filter_err(attr, op, val, 'bad op in filter')
             if model_class is None:
                 return True
             class_name = model_class.__name__
diff --git a/lib/galaxy/managers/tools.py b/lib/galaxy/managers/tools.py
index c320a726445..d99e8c6af89 100644
--- a/lib/galaxy/managers/tools.py
+++ b/lib/galaxy/managers/tools.py
@@ -6,7 +6,7 @@ from sqlalchemy import sql
 from galaxy import exceptions
 from galaxy import model
 from galaxy.exceptions import DuplicatedIdentifierException
-from .base import ModelManager
+from .base import ModelManager, raise_filter_err
 from .executables import artifact_class
 
 log = logging.getLogger(__name__)
@@ -112,7 +112,7 @@ class ToolFilterMixin:
             elif op == 'contains':
                 cond = model.Job.table.c.tool_id.contains(val, autoescape=True)
             else:
-                self.raise_filter_err(attr, op, val, 'bad op in filter')
+                raise_filter_err(attr, op, val, 'bad op in filter')
             if model_class is model.HistoryDatasetAssociation:
                 return sql.expression.and_(
                     model.Job.table.c.id == model.JobToOutputDatasetAssociation.table.c.job_id,

From d2d717bf5fb3beaf6440242f4b55e52e95b97f65 Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Tue, 26 Oct 2021 12:53:53 -0400
Subject: [PATCH 4/8] Strong typing around tagging in manager layer, less
 dependence on app.

---
 lib/galaxy/managers/hdas.py     |  5 ++++-
 lib/galaxy/managers/hdcas.py    | 10 +++++++++-
 lib/galaxy/managers/sharable.py |  6 ++++++
 lib/galaxy/managers/taggable.py |  8 ++++++--
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/lib/galaxy/managers/hdas.py b/lib/galaxy/managers/hdas.py
index dc2c27b0210..fd7d2867e5b 100644
--- a/lib/galaxy/managers/hdas.py
+++ b/lib/galaxy/managers/hdas.py
@@ -23,6 +23,7 @@ from galaxy.managers import (
     taggable,
     users,
 )
+from galaxy.model.tags import GalaxyTagHandler
 from galaxy.structured_app import MinimalManagerApp, StructuredApp
 
 log = logging.getLogger(__name__)
@@ -49,12 +50,13 @@ class HDAManager(datasets.DatasetAssociationManager,
     # TODO: move what makes sense into DatasetManager
     # TODO: which of these are common with LDDAs and can be pushed down into DatasetAssociationManager?
 
-    def __init__(self, app: MinimalManagerApp, user_manager: users.UserManager):
+    def __init__(self, app: MinimalManagerApp, user_manager: users.UserManager, 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
 
     def get_owned_ids(self, object_ids, history=None):
         """Get owned IDs.
@@ -534,6 +536,7 @@ class HDADeserializer(datasets.DatasetAssociationDeserializer,
     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/hdcas.py b/lib/galaxy/managers/hdcas.py
index cb991cfa88d..e72aa46add3 100644
--- a/lib/galaxy/managers/hdcas.py
+++ b/lib/galaxy/managers/hdcas.py
@@ -17,7 +17,8 @@ from galaxy.managers import (
     taggable
 )
 from galaxy.managers.collections_util import get_hda_and_element_identifiers
-from galaxy.structured_app import StructuredApp
+from galaxy.model.tags import GalaxyTagHandler
+from galaxy.structured_app import MinimalManagerApp, StructuredApp
 from galaxy.util.zipstream import ZipstreamWrapper
 
 
@@ -62,6 +63,13 @@ class HDCAManager(
     tag_assoc = model.HistoryDatasetCollectionTagAssociation
     annotation_assoc = model.HistoryDatasetCollectionAssociationAnnotationAssociation
 
+    def __init__(self, app: MinimalManagerApp):
+        """
+        Set up and initialize other managers needed by hdas.
+        """
+        super().__init__(app)
+        self.tag_handler = app[GalaxyTagHandler]
+
     def map_datasets(self, content, fn, *parents):
         """
         Iterate over the datasets of a given collection, recursing into collections, and
diff --git a/lib/galaxy/managers/sharable.py b/lib/galaxy/managers/sharable.py
index 1f74fc2472c..506627c32b4 100644
--- a/lib/galaxy/managers/sharable.py
+++ b/lib/galaxy/managers/sharable.py
@@ -35,6 +35,7 @@ from galaxy.model import (
     User,
     UserShareAssociation,
 )
+from galaxy.model.tags import GalaxyTagHandler
 from galaxy.schema.schema import (
     ShareWithExtra,
     SharingOptions,
@@ -60,6 +61,7 @@ class SharableModelManager(base.ModelManager, secured.OwnableManagerMixin, secur
         super().__init__(app)
         # user manager is needed to check access/ownership/admin
         self.user_manager = users.UserManager(app)
+        self.tag_handler = app[GalaxyTagHandler]
 
     # .... has a user
     def by_user(self, user, filters=None, **kwargs):
@@ -417,6 +419,10 @@ class SharableModelSerializer(base.ModelSerializer,
 class SharableModelDeserializer(base.ModelDeserializer,
         taggable.TaggableDeserializerMixin, annotatable.AnnotatableDeserializerMixin, ratable.RatableDeserializerMixin):
 
+    def __init__(self, app: MinimalManagerApp, **kwargs):
+        super().__init__(app, **kwargs)
+        self.tag_handler = app.tag_handler
+
     def add_deserializers(self):
         super().add_deserializers()
         taggable.TaggableDeserializerMixin.add_deserializers(self)
diff --git a/lib/galaxy/managers/taggable.py b/lib/galaxy/managers/taggable.py
index 1c5be68e528..233fc417c9d 100644
--- a/lib/galaxy/managers/taggable.py
+++ b/lib/galaxy/managers/taggable.py
@@ -10,6 +10,7 @@ from typing import Type
 from sqlalchemy import sql
 
 from galaxy import model
+from galaxy.model.tags import GalaxyTagHandler
 from galaxy.util import unicodify
 from .base import ModelValidator, raise_filter_err
 
@@ -48,6 +49,7 @@ def _tags_from_strings(item, tag_handler, new_tags_list, user=None):
 
 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):
@@ -60,7 +62,7 @@ class TaggableManagerMixin:
         """
         Set an `item`'s tags from a list of strings.
         """
-        return _tags_from_strings(item, self.app.tag_handler, new_tags, user=user)
+        return _tags_from_strings(item, self.tag_handler, new_tags, user=user)
 
     # def tags_by_user( self, user, **kwargs ):
     # TODO: here or GalaxyTagHandler
@@ -80,6 +82,8 @@ class TaggableSerializerMixin:
 
 
 class TaggableDeserializerMixin:
+    tag_handler: GalaxyTagHandler
+    validate: ModelValidator
 
     def add_deserializers(self):
         self.deserializers['tags'] = self.deserialize_tags
@@ -91,7 +95,7 @@ class TaggableDeserializerMixin:
         Note: this will erase any previous tags.
         """
         new_tags_list = self.validate.basestring_list(key, val)
-        _tags_from_strings(item, self.app.tag_handler, new_tags_list, user=user)
+        _tags_from_strings(item, self.tag_handler, new_tags_list, user=user)
         return item.tags
 
 

From 6231f61a63706b5ae95bc933ee2d741b704e6370 Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Tue, 26 Oct 2021 13:28:33 -0400
Subject: [PATCH 5/8] More typing for managers...

---
 lib/galaxy/managers/base.py       | 220 +++++++++++++++---------------
 lib/galaxy/managers/containers.py |   6 +-
 2 files changed, 112 insertions(+), 114 deletions(-)

diff --git a/lib/galaxy/managers/base.py b/lib/galaxy/managers/base.py
index 7ddeadf5cbd..6ebe38ede71 100644
--- a/lib/galaxy/managers/base.py
+++ b/lib/galaxy/managers/base.py
@@ -41,6 +41,7 @@ from typing import (
 )
 
 import sqlalchemy
+from sqlalchemy.orm import Query
 from sqlalchemy.orm.scoping import scoped_session
 from typing_extensions import Protocol
 
@@ -202,7 +203,7 @@ class ModelManager:
         return item
 
     # .... query foundation wrapper
-    def query(self, eagerloads=True, **kwargs):
+    def query(self, eagerloads: bool = True, filters=None, order_by=None, limit: Optional[int] = None, offset: Optional[int] = None) -> Query:
         """
         Return a basic query from model_class, filters, order_by, and limit and offset.
 
@@ -212,9 +213,9 @@ class ModelManager:
         # joined table loading
         if eagerloads is False:
             query = query.enable_eagerloads(False)
-        return self._filter_and_order_query(query, **kwargs)
+        return self._filter_and_order_query(query, filters=filters, order_by=order_by, limit=limit, offset=offset)
 
-    def _filter_and_order_query(self, query, filters=None, order_by=None, limit=None, offset=None, **kwargs):
+    def _filter_and_order_query(self, query: Query, filters=None, order_by=None, limit: Optional[int] = None, offset: Optional[int] = None) -> Query:
         # TODO: not a lot of functional cohesion here
         query = self._apply_orm_filters(query, filters)
         query = self._apply_order_by(query, order_by)
@@ -222,7 +223,7 @@ class ModelManager:
         return query
 
     # .... filters
-    def _apply_orm_filters(self, query, filters):
+    def _apply_orm_filters(self, query: Query, filters) -> Query:
         """
         Add any filters to the given query.
         """
@@ -245,7 +246,7 @@ class ModelManager:
         return munge_lists(filtersA, filtersB)
 
     # .... order, limit, and offset
-    def _apply_order_by(self, query, order_by):
+    def _apply_order_by(self, query: Query, order_by) -> Query:
         """
         Return the query after adding the order_by clauses.
 
@@ -262,9 +263,9 @@ class ModelManager:
         """
         Returns a tuple of columns for the default order when getting multiple models.
         """
-        return (self.model_class.create_time, )
+        return (self.model_class.table.c.create_time, )
 
-    def _apply_orm_limit_offset(self, query, limit, offset):
+    def _apply_orm_limit_offset(self, query: Query, limit: Optional[int], offset: Optional[int]) -> Query:
         """
         Return the query after applying the given limit and offset (if not None).
         """
@@ -345,7 +346,8 @@ class ModelManager:
             a list of filters to be added to the SQL query
         and a list of functional filters to be applied after the SQL query.
         """
-        orm_filters, fn_filters = ([], [])
+        orm_filters: list = []
+        fn_filters: list = []
         if filters is None:
             return (orm_filters, fn_filters)
         if not isinstance(filters, list):
@@ -406,7 +408,7 @@ class ModelManager:
         """
         if not ids:
             return []
-        ids_filter = parsed_filter("orm", self.model_class.id.in_(ids))
+        ids_filter = parsed_filter("orm", self.model_class.table.c.id.in_(ids))
         found = self.list(filters=self._munge_filters(ids_filter, filters), **kwargs)
         # TODO: this does not order by the original 'ids' array
 
@@ -506,12 +508,14 @@ class HasAModelManager:
     """
 
     #: the class used to create this serializer's generically accessible model_manager
-    model_manager_class: Type[object]
+    model_manager_class: Type[object]  # ideally this would be Type[ModelManager] but HistoryContentsManager cannot be a ModelManager
     # examples where this doesn't really work are ConfigurationSerializer (no manager)
     # and contents (2 managers)
+    app: MinimalManagerApp
 
     def __init__(self, app: MinimalManagerApp, manager=None, **kwargs):
         self._manager = manager
+        self.app = app
 
     @property
     def manager(self):
@@ -569,7 +573,6 @@ class ModelSerializer(HasAModelManager):
         Set up serializer map, any additional serializable keys, and views here.
         """
         super().__init__(app, **kwargs)
-        self.app = app
 
         # a list of valid serializable keys that can use the default (string) serializer
         #   this allows us to: 'mention' the key without adding the default serializer
@@ -728,98 +731,6 @@ class ModelSerializer(HasAModelManager):
         return self.views[view][:]
 
 
-class Deserializer(Protocol):
-
-    def __call__(self, item: Any, key: Any, val: Any, **kwargs) -> Any:
-        ...
-
-
-class ModelDeserializer(HasAModelManager):
-    """
-    An object that converts an incoming serialized dict into values that can be
-    directly assigned to an item's attributes and assigns them.
-    """
-    validate: ModelValidator
-
-    # TODO:?? a larger question is: which should be first? Deserialize then validate - or - validate then deserialize?
-
-    def __init__(self, app: MinimalManagerApp, validator: Optional[ModelValidator] = None, **kwargs):
-        """
-        Set up deserializers and validator.
-        """
-        super().__init__(app, **kwargs)
-        self.app = app
-
-        self.deserializers: Dict[str, Deserializer] = {}
-        self.deserializable_keyset: Set[str] = set()
-        self.add_deserializers()
-        # a sub object that can validate incoming values
-        self.validate = validator or ModelValidator(self.app)
-
-    def add_deserializers(self):
-        """
-        Register a map of attribute keys -> functions that will deserialize data
-        into attributes to be assigned to the item.
-        """
-        # to be overridden in subclasses
-
-    def deserialize(self, item, data, flush=True, **context):
-        """
-        Convert an incoming serialized dict into values that can be
-        directly assigned to an item's attributes and assign them
-        """
-        # TODO: constrain context to current_user/whos_asking when that's all we need (trans)
-        sa_session = self.app.model.context
-        new_dict = {}
-        for key, val in data.items():
-            if key in self.deserializers:
-                new_dict[key] = self.deserializers[key](item, key, val, **context)
-            # !important: don't error on unreg. keys -- many clients will add weird ass keys onto the model
-
-        # TODO:?? add and flush here or in manager?
-        if flush and len(new_dict):
-            sa_session.add(item)
-            sa_session.flush()
-
-        return new_dict
-
-    # ... common deserializers for primitives
-    def default_deserializer(self, item, key, val, **context):
-        """
-        If the incoming `val` is different than the `item` value change it
-        and, in either case, return the value.
-        """
-        # TODO: sets the item attribute to value (this may not work in all instances)
-
-        # only do the following if val == getattr( item, key )
-        if hasattr(item, key) and getattr(item, key) != val:
-            setattr(item, key, val)
-        return val
-
-    def deserialize_basestring(self, item, key, val, convert_none_to_empty=False, **context):
-        val = '' if (convert_none_to_empty and val is None) else self.validate.basestring(key, val)
-        return self.default_deserializer(item, key, val, **context)
-
-    def deserialize_bool(self, item, key, val, **context):
-        val = self.validate.bool(key, val)
-        return self.default_deserializer(item, key, val, **context)
-
-    def deserialize_int(self, item, key, val, min=None, max=None, **context):
-        val = self.validate.int_range(key, val, min, max)
-        return self.default_deserializer(item, key, val, **context)
-
-    # def deserialize_date( self, item, key, val ):
-    #    #TODO: parse isoformat date into date object
-
-    # ... common deserializers for Galaxy
-    def deserialize_genome_build(self, item, key, val, **context):
-        """
-        Make sure `val` is a valid dbkey and assign it.
-        """
-        val = self.validate.genome_build(key, val)
-        return self.default_deserializer(item, key, val, **context)
-
-
 class ModelValidator(HasAModelManager):
     """
     An object that inspects a dictionary (generally meant to be a set of
@@ -827,10 +738,6 @@ class ModelValidator(HasAModelManager):
     not acceptable.
     """
 
-    def __init__(self, app, *args, **kwargs):
-        super().__init__(app, **kwargs)
-        self.app = app
-
     def type(self, key, val, types):
         """
         Check `val` against the type (or tuple of types) in `types`.
@@ -903,6 +810,98 @@ class ModelValidator(HasAModelManager):
     #    pass
 
 
+class Deserializer(Protocol):
+
+    def __call__(self, item: Any, key: Any, val: Any, **kwargs) -> Any:
+        ...
+
+
+class ModelDeserializer(HasAModelManager):
+    """
+    An object that converts an incoming serialized dict into values that can be
+    directly assigned to an item's attributes and assigns them.
+    """
+    validate: ModelValidator
+    app: MinimalManagerApp
+
+    # TODO:?? a larger question is: which should be first? Deserialize then validate - or - validate then deserialize?
+
+    def __init__(self, app: MinimalManagerApp, validator: Optional[ModelValidator] = None, **kwargs):
+        """
+        Set up deserializers and validator.
+        """
+        super().__init__(app, **kwargs)
+
+        self.deserializers: Dict[str, Deserializer] = {}
+        self.deserializable_keyset: Set[str] = set()
+        self.add_deserializers()
+        # a sub object that can validate incoming values
+        self.validate = validator or ModelValidator(self.app)
+
+    def add_deserializers(self):
+        """
+        Register a map of attribute keys -> functions that will deserialize data
+        into attributes to be assigned to the item.
+        """
+        # to be overridden in subclasses
+
+    def deserialize(self, item, data, flush=True, **context):
+        """
+        Convert an incoming serialized dict into values that can be
+        directly assigned to an item's attributes and assign them
+        """
+        # TODO: constrain context to current_user/whos_asking when that's all we need (trans)
+        sa_session = self.app.model.context
+        new_dict = {}
+        for key, val in data.items():
+            if key in self.deserializers:
+                new_dict[key] = self.deserializers[key](item, key, val, **context)
+            # !important: don't error on unreg. keys -- many clients will add weird ass keys onto the model
+
+        # TODO:?? add and flush here or in manager?
+        if flush and len(new_dict):
+            sa_session.add(item)
+            sa_session.flush()
+
+        return new_dict
+
+    # ... common deserializers for primitives
+    def default_deserializer(self, item, key, val, **context):
+        """
+        If the incoming `val` is different than the `item` value change it
+        and, in either case, return the value.
+        """
+        # TODO: sets the item attribute to value (this may not work in all instances)
+
+        # only do the following if val == getattr( item, key )
+        if hasattr(item, key) and getattr(item, key) != val:
+            setattr(item, key, val)
+        return val
+
+    def deserialize_basestring(self, item, key, val, convert_none_to_empty=False, **context):
+        val = '' if (convert_none_to_empty and val is None) else self.validate.basestring(key, val)
+        return self.default_deserializer(item, key, val, **context)
+
+    def deserialize_bool(self, item, key, val, **context):
+        val = self.validate.bool(key, val)
+        return self.default_deserializer(item, key, val, **context)
+
+    def deserialize_int(self, item, key, val, min=None, max=None, **context):
+        val = self.validate.int_range(key, val, min, max)
+        return self.default_deserializer(item, key, val, **context)
+
+    # def deserialize_date( self, item, key, val ):
+    #    #TODO: parse isoformat date into date object
+
+    # ... common deserializers for Galaxy
+    def deserialize_genome_build(self, item, key, val, **context):
+        """
+        Make sure `val` is a valid dbkey and assign it.
+        """
+        val = self.validate.genome_build(key, val)
+        return self.default_deserializer(item, key, val, **context)
+
+
 # ==== Building query filters based on model data
 class ModelFilterParser(HasAModelManager):
     """
@@ -929,7 +928,7 @@ class ModelFilterParser(HasAModelManager):
     # (as the model informs how the filter params are parsed)
     # I have no great idea where this 'belongs', so it's here for now
 
-    model_class: type
+    model_class: Type[model._HasTable]
     parsed_filter = parsed_filter
     orm_filter_parsers: Dict[str, Dict]
     fn_filter_parsers: Dict[str, Dict]
@@ -939,7 +938,6 @@ class ModelFilterParser(HasAModelManager):
         Set up serializer map, any additional serializable keys, and views here.
         """
         super().__init__(app, **kwargs)
-        self.app = app
 
         #: regex for testing/dicing iso8601 date strings, with optional time and ms, but allowing only UTC timezone
         self.date_string_re = re.compile(r'^(\d{4}\-\d{2}\-\d{2})[T| ]{0,1}(\d{2}:\d{2}:\d{2}(?:\.\d{1,6}){0,1}){0,1}Z{0,1}$')
@@ -1064,7 +1062,7 @@ class ModelFilterParser(HasAModelManager):
         attr_map = self.fn_filter_parsers.get(attr, None)
         if not attr_map:
             return None
-        allowed_ops = attr_map.get('op')
+        allowed_ops = attr_map['op']
         # allowed ops is a map here, op => fn
         filter_fn = allowed_ops.get(op, None)
         if not filter_fn:
@@ -1104,7 +1102,7 @@ class ModelFilterParser(HasAModelManager):
             return None
 
         # op must be allowlisted: contained in the list orm_filter_list[ attr ][ 'op' ]
-        allowed_ops = column_map.get('op')
+        allowed_ops = column_map['op']
         if op not in allowed_ops:
             return None
         op = self._convert_op_string_to_fn(column, op)
diff --git a/lib/galaxy/managers/containers.py b/lib/galaxy/managers/containers.py
index 3c105da8bd9..860fdcf4d23 100644
--- a/lib/galaxy/managers/containers.py
+++ b/lib/galaxy/managers/containers.py
@@ -8,7 +8,7 @@ and other (nested) containers.
 # Libraries should be DatasetCollections.
 import logging
 import operator
-from typing import Optional
+from typing import Optional, Type
 
 import galaxy.exceptions
 import galaxy.util
@@ -31,8 +31,8 @@ class ContainerManagerMixin:
     # TODO: terminology is getting a bit convoluted and silly at this point: rename three public below?
     # TODO: this should be an open mapping (not just 2)
     #: the classes that can be contained
-    contained_class: type
-    subcontainer_class: type
+    contained_class: Type[model._HasTable]
+    subcontainer_class: Type[model._HasTable]
     #: how any contents lists produced are ordered - (string) attribute name to sort on or tuple of attribute names
     default_order_by: Optional[str] = None
 

From 789d70845dd057560c328238684709ec0a665179 Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Tue, 26 Oct 2021 15:09:59 -0400
Subject: [PATCH 6/8] Simplify design of ModelValidator in managers base.

Didn't use ``app`` or any class methods so just made it a utility class of staticmethods. Added types.
---
 lib/galaxy/managers/base.py     | 57 +++++++++++++++++----------------
 lib/galaxy/managers/datasets.py |  2 +-
 2 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/lib/galaxy/managers/base.py b/lib/galaxy/managers/base.py
index 6ebe38ede71..61ebb5f162d 100644
--- a/lib/galaxy/managers/base.py
+++ b/lib/galaxy/managers/base.py
@@ -38,6 +38,7 @@ from typing import (
     Set,
     Tuple,
     Type,
+    Union,
 )
 
 import sqlalchemy
@@ -731,14 +732,15 @@ class ModelSerializer(HasAModelManager):
         return self.views[view][:]
 
 
-class ModelValidator(HasAModelManager):
+class ModelValidator:
     """
     An object that inspects a dictionary (generally meant to be a set of
     new/updated values for the model) and raises an error if a value is
     not acceptable.
     """
 
-    def type(self, key, val, types):
+    @staticmethod
+    def matches_type(key: str, val: Any, types: Union[type, Tuple[Union[type, Tuple[Any, ...]], ...]]):
         """
         Check `val` against the type (or tuple of types) in `types`.
 
@@ -750,43 +752,46 @@ class ModelValidator(HasAModelManager):
         return val
 
     # validators for primitives and compounds of primitives
-    def basestring(self, key, val):
-        return self.type(key, val, (str,))
+    @staticmethod
+    def basestring(key: str, val: Any) -> str:
+        return ModelValidator.matches_type(key, val, (str,))
 
-    def bool(self, key, val):
-        return self.type(key, val, bool)
+    @staticmethod
+    def bool(key: str, val: Any) -> bool:
+        return ModelValidator.matches_type(key, val, bool)
 
-    def int(self, key, val):
-        return self.type(key, val, int)
-
-    def nullable_basestring(self, key, val):
+    @staticmethod
+    def nullable_basestring(key: str, val: Any) -> str:
         """
         Must be a basestring or None.
         """
-        return self.type(key, val, ((str,), type(None)))
+        return ModelValidator.matches_type(key, val, ((str,), type(None)))
 
-    def int_range(self, key, val, min=None, max=None):
+    @staticmethod
+    def int_range(key: str, val: Any, min: Optional[int] = None, max: Optional[int] = None) -> int:
         """
         Must be a int between min and max.
         """
-        val = self.type(key, val, int)
-        if min is not None and val < min:
-            raise exceptions.RequestParameterInvalidException("less than minimum", key=key, val=val, min=min)
-        if max is not None and val > max:
-            raise exceptions.RequestParameterInvalidException("greater than maximum", key=key, val=val, max=max)
-        return val
+        val_ = ModelValidator.matches_type(key, val, int)
+        if min is not None and val_ < min:
+            raise exceptions.RequestParameterInvalidException("less than minimum", key=key, val=val_, min=min)
+        if max is not None and val_ > max:
+            raise exceptions.RequestParameterInvalidException("greater than maximum", key=key, val=val_, max=max)
+        return val_
 
-    def basestring_list(self, key, val):
+    @staticmethod
+    def basestring_list(key: str, val: Any) -> List[str]:
         """
         Must be a list of basestrings.
         """
         # TODO: Here's where compound types start becoming a nightmare. Any more or more complex
         #   and should find a different way.
-        val = self.type(key, val, list)
-        return [self.basestring(key, elem) for elem in val]
+        val_ = ModelValidator.matches_type(key, val, list)
+        return [ModelValidator.basestring(key, elem) for elem in val_]
 
     # validators for Galaxy
-    def genome_build(self, key, val):
+    @staticmethod
+    def genome_build(key: str, val: Any) -> str:
         """
         Must be a valid base_string.
 
@@ -803,7 +808,7 @@ class ModelValidator(HasAModelManager):
         #         return val
         # raise exceptions.RequestParameterInvalidException( "invalid reference", key=key, val=val )
         # IOW: fallback to string validation
-        return self.basestring(key, val)
+        return ModelValidator.basestring(key, val)
 
     # def slug( self, item, key, val ):
     #    """validate slug"""
@@ -821,12 +826,12 @@ class ModelDeserializer(HasAModelManager):
     An object that converts an incoming serialized dict into values that can be
     directly assigned to an item's attributes and assigns them.
     """
-    validate: ModelValidator
+    validate = ModelValidator()
     app: MinimalManagerApp
 
     # TODO:?? a larger question is: which should be first? Deserialize then validate - or - validate then deserialize?
 
-    def __init__(self, app: MinimalManagerApp, validator: Optional[ModelValidator] = None, **kwargs):
+    def __init__(self, app: MinimalManagerApp, **kwargs):
         """
         Set up deserializers and validator.
         """
@@ -835,8 +840,6 @@ class ModelDeserializer(HasAModelManager):
         self.deserializers: Dict[str, Deserializer] = {}
         self.deserializable_keyset: Set[str] = set()
         self.add_deserializers()
-        # a sub object that can validate incoming values
-        self.validate = validator or ModelValidator(self.app)
 
     def add_deserializers(self):
         """
diff --git a/lib/galaxy/managers/datasets.py b/lib/galaxy/managers/datasets.py
index f44ef54a732..7dc7d1d4505 100644
--- a/lib/galaxy/managers/datasets.py
+++ b/lib/galaxy/managers/datasets.py
@@ -661,7 +661,7 @@ class DatasetAssociationDeserializer(base.ModelDeserializer, deletable.PurgableD
     def deserialize_metadata(self, dataset_assoc, metadata_key, metadata_dict, **context):
         """
         """
-        self.validate.type(metadata_key, metadata_dict, dict)
+        self.validate.matches_type(metadata_key, metadata_dict, dict)
         returned = {}
         for key, val in metadata_dict.items():
             returned[key] = self.deserialize_metadatum(dataset_assoc, key, val, **context)

From c3ea408eeb1e25dcbcf0eb3d938c0f6c76f65b36 Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Tue, 26 Oct 2021 16:22:38 -0400
Subject: [PATCH 7/8] Define protocol for serializers - mypy check all the
 serialiers in manager layer.

---
 lib/galaxy/managers/base.py             | 15 +++--
 lib/galaxy/managers/cloudauthzs.py      | 26 +++++----
 lib/galaxy/managers/configuration.py    | 30 +++++-----
 lib/galaxy/managers/datasets.py         | 75 ++++++++++++++-----------
 lib/galaxy/managers/hdas.py             | 46 ++++++++-------
 lib/galaxy/managers/hdcas.py            | 31 +++++-----
 lib/galaxy/managers/history_contents.py | 12 ++--
 setup.cfg                               |  2 -
 8 files changed, 128 insertions(+), 109 deletions(-)

diff --git a/lib/galaxy/managers/base.py b/lib/galaxy/managers/base.py
index 61ebb5f162d..baf6be8e8e8 100644
--- a/lib/galaxy/managers/base.py
+++ b/lib/galaxy/managers/base.py
@@ -30,7 +30,6 @@ import logging
 import re
 from typing import (
     Any,
-    Callable,
     Dict,
     List,
     NamedTuple,
@@ -547,6 +546,12 @@ class SkipAttribute(Exception):
     """
 
 
+class Serializer(Protocol):
+
+    def __call__(self, item: Any, key: str, **context) -> Any:
+        ...
+
+
 class ModelSerializer(HasAModelManager):
     """
     Turns models into JSONable dicts.
@@ -581,7 +586,7 @@ class ModelSerializer(HasAModelManager):
         #   that is in neither serializable_keyset or serializers
         self.serializable_keyset: Set[str] = set()
         # a map of dictionary keys to the functions (often lambdas) that create the values for those keys
-        self.serializers: Dict[str, Callable] = {}
+        self.serializers: Dict[str, Serializer] = {}
         # add subclass serializers defined there
         self.add_serializers()
         # update the keyset by the serializers (removing the responsibility from subclasses)
@@ -662,14 +667,14 @@ class ModelSerializer(HasAModelManager):
         return getattr(item, key)
 
     # serializers for common galaxy objects
-    def serialize_date(self, item, key, **context):
+    def serialize_date(self, item: Any, key: str, **context):
         """
         Serialize a date attribute of `item`.
         """
         date = getattr(item, key)
         return date.isoformat() if date is not None else None
 
-    def serialize_id(self, item, key, **context):
+    def serialize_id(self, item: Any, key: str, **context):
         """
         Serialize an id attribute of `item`.
         """
@@ -677,7 +682,7 @@ class ModelSerializer(HasAModelManager):
         # Note: it may not be best to encode the id at this layer
         return self.app.security.encode_id(id) if id is not None else None
 
-    def serialize_type_id(self, item, key, **context):
+    def serialize_type_id(self, item: Any, key: str, **context):
         """
         Serialize an type-id for `item`.
         """
diff --git a/lib/galaxy/managers/cloudauthzs.py b/lib/galaxy/managers/cloudauthzs.py
index e568fe62924..005fbe780eb 100644
--- a/lib/galaxy/managers/cloudauthzs.py
+++ b/lib/galaxy/managers/cloudauthzs.py
@@ -3,6 +3,7 @@ Manager and (de)serializer for cloud authorizations (cloudauthzs).
 """
 
 import logging
+from typing import Dict
 
 from galaxy import model
 from galaxy.exceptions import (
@@ -51,18 +52,19 @@ class CloudAuthzsSerializer(base.ModelSerializer):
         # i  : an instance of galaxy.model.CloudAuthz.
         # k  : serialized dictionary key (e.g., 'model_class', 'provider').
         # **c: a dictionary containing 'trans' and 'user' objects.
-        self.serializers.update({
-            'id': lambda i, k, **c: self.app.security.encode_id(i.id),
-            'model_class': lambda *a, **c: 'CloudAuthz',
-            'user_id': lambda i, k, **c: self.app.security.encode_id(i.user_id),
-            'provider': lambda i, k, **c: str(i.provider),
-            'config': lambda i, k, **c: i.config,
-            'authn_id': lambda i, k, **c: self.app.security.encode_id(i.authn_id) if i.authn_id else None,
-            'last_update': lambda i, k, **c: str(i.last_update),
-            'last_activity': lambda i, k, **c: str(i.last_activity),
-            'create_time': lambda i, k, **c: str(i.create_time),
-            'description': lambda i, k, **c: str(i.description)
-        })
+        serializers: Dict[str, base.Serializer] = {
+            'id': lambda item, key, **context: self.app.security.encode_id(item.id),
+            'model_class': lambda item, key, **context: 'CloudAuthz',
+            'user_id': lambda item, key, **context: self.app.security.encode_id(item.user_id),
+            'provider': lambda item, key, **context: str(item.provider),
+            'config': lambda item, key, **context: item.config,
+            'authn_id': lambda item, key, **context: self.app.security.encode_id(item.authn_id) if item.authn_id else None,
+            'last_update': lambda item, key, **context: str(item.last_update),
+            'last_activity': lambda item, key, **context: str(item.last_activity),
+            'create_time': lambda item, key, **context: str(item.create_time),
+            'description': lambda item, key, **context: str(item.description)
+        }
+        self.serializers.update(serializers)
 
 
 class CloudAuthzsDeserializer(base.ModelDeserializer):
diff --git a/lib/galaxy/managers/configuration.py b/lib/galaxy/managers/configuration.py
index c3f77ff8594..27427d89606 100644
--- a/lib/galaxy/managers/configuration.py
+++ b/lib/galaxy/managers/configuration.py
@@ -116,18 +116,18 @@ class ConfigSerializer(base.ModelSerializer):
 
     def add_serializers(self):
 
-        def _defaults_to(default):
-            return lambda config, key, **context: getattr(config, key, default)
+        def _defaults_to(default) -> base.Serializer:
+            return lambda item, key, **context: getattr(item, key, default)
 
-        def _use_config(config, key, **context):
+        def _use_config(item, key: str, **context):
             """Let config object determine the value for key"""
-            assert hasattr(config, key)
-            return config.config_value_for_host(key, context.get("host"))
+            assert hasattr(item, key)
+            return item.config_value_for_host(key, context.get("host"))
 
-        def _config_is_truthy(config, key, **context):
-            return True if config.get(key) else False
+        def _config_is_truthy(item, key, **context):
+            return True if item.get(key) else False
 
-        self.serializers = {
+        self.serializers: Dict[str, base.Serializer] = {
             # TODO: this is available from user data, remove
             'is_admin_user': lambda *a, **c: False,
             'brand': _use_config,
@@ -167,7 +167,7 @@ class ConfigSerializer(base.ModelSerializer):
             'ga_code': _use_config,
             'plausible_server': _use_config,
             'plausible_domain': _use_config,
-            'markdown_to_pdf_available': lambda config, key, **context: weasyprint_available(),
+            'markdown_to_pdf_available': lambda item, key, **context: weasyprint_available(),
             'matomo_server': _use_config,
             'matomo_site_id': _use_config,
             'enable_unique_workflow_defaults': _use_config,
@@ -180,7 +180,7 @@ class ConfigSerializer(base.ModelSerializer):
             # TODO: is there no 'correct' way to get an api url? controller='api', action='tools' is a hack
             # at any rate: the following works with path_prefix but is still brittle
             # TODO: change this to (more generic) upload_path and incorporate config.nginx_upload_path into building it
-            'nginx_upload_path': lambda config, key, **context: getattr(config, key, False),
+            'nginx_upload_path': lambda item, key, **context: getattr(item, key, False),
             'chunk_upload_size': _use_config,
             'ftp_upload_site': _use_config,
             'version_major': _defaults_to(None),
@@ -193,22 +193,22 @@ class ConfigSerializer(base.ModelSerializer):
             'message_box_content': _use_config,
             'message_box_visible': _use_config,
             'message_box_class': _use_config,
-            'server_startttime': lambda config, key, **context: server_starttime,
+            'server_startttime': lambda item, key, **context: server_starttime,
             'mailing_join_addr': _defaults_to('galaxy-announce-join@bx.psu.edu'),  # should this be the schema default?
-            'server_mail_configured': lambda config, key, **context: bool(config.smtp_server),
+            'server_mail_configured': lambda item, key, **context: bool(item.smtp_server),
             'registration_warning_message': _use_config,
             'welcome_url': _use_config,
             'show_welcome_with_login': _defaults_to(True),  # schema default is False
             'cookie_domain': _use_config,
             'python': _defaults_to((sys.version_info.major, sys.version_info.minor)),
             'select_type_workflow_threshold': _use_config,
-            'file_sources_configured': lambda config, key, **context: self.app.file_sources.custom_sources_configured,
-            'panel_views': lambda config, key, **content: self.app.toolbox.panel_view_dicts(),
+            'file_sources_configured': lambda item, key, **context: self.app.file_sources.custom_sources_configured,
+            'panel_views': lambda item, key, **context: self.app.toolbox.panel_view_dicts(),
             'default_panel_view': _use_config,
             'upload_from_form_button': _use_config,
             'release_doc_base_url': _use_config,
             'expose_user_email': _use_config,
-            'user_library_import_dir_available': lambda config, key, **context: bool(config.get('user_library_import_dir')),
+            'user_library_import_dir_available': lambda item, key, **context: bool(item.get('user_library_import_dir')),
             'welcome_directory': _use_config,
         }
 
diff --git a/lib/galaxy/managers/datasets.py b/lib/galaxy/managers/datasets.py
index 7dc7d1d4505..528e149efaf 100644
--- a/lib/galaxy/managers/datasets.py
+++ b/lib/galaxy/managers/datasets.py
@@ -4,7 +4,7 @@ Manager and Serializer for Datasets.
 import glob
 import logging
 import os
-from typing import List, Type
+from typing import Dict, List, Type
 
 from galaxy import (
     exceptions,
@@ -171,25 +171,26 @@ class DatasetSerializer(base.ModelSerializer, deletable.PurgableSerializerMixin)
     def add_serializers(self):
         super().add_serializers()
         deletable.PurgableSerializerMixin.add_serializers(self)
-
-        self.serializers.update({
+        serializers: Dict[str, base.Serializer] = {
             'create_time': self.serialize_date,
             'update_time': self.serialize_date,
 
-            'uuid': lambda i, k, **c: str(i.uuid) if i.uuid else None,
+            'uuid': lambda item, key, **context: str(item.uuid) if item.uuid else None,
             'file_name': self.serialize_file_name,
             'extra_files_path': self.serialize_extra_files_path,
             'permissions': self.serialize_permissions,
 
-            'total_size': lambda i, k, **c: int(i.get_total_size()),
-            'file_size': lambda i, k, **c: int(i.get_size())
-        })
+            'total_size': lambda item, key, **context: int(item.get_total_size()),
+            'file_size': lambda item, key, **context: int(item.get_size())
+        }
+        self.serializers.update(serializers)
 
-    def serialize_file_name(self, dataset, key, user=None, **context):
+    def serialize_file_name(self, item, key, user=None, **context):
         """
         If the config allows or the user is admin, return the file name
         of the file that contains this dataset's data.
         """
+        dataset = item
         is_admin = self.user_manager.is_admin(user, trans=context.get("trans"))
         # expensive: allow config option due to cost of operation
         if is_admin or self.app.config.expose_dataset_path:
@@ -197,10 +198,11 @@ class DatasetSerializer(base.ModelSerializer, deletable.PurgableSerializerMixin)
                 return dataset.file_name
         self.skip()
 
-    def serialize_extra_files_path(self, dataset, key, user=None, **context):
+    def serialize_extra_files_path(self, item, key, user=None, **context):
         """
         If the config allows or the user is admin, return the file path.
         """
+        dataset = item
         is_admin = self.user_manager.is_admin(user, trans=context.get("trans"))
         # expensive: allow config option due to cost of operation
         if is_admin or self.app.config.expose_dataset_path:
@@ -208,9 +210,10 @@ class DatasetSerializer(base.ModelSerializer, deletable.PurgableSerializerMixin)
                 return dataset.extra_files_path
         self.skip()
 
-    def serialize_permissions(self, dataset, key, user=None, **context):
+    def serialize_permissions(self, item, key, user=None, **context):
         """
         """
+        dataset = item
         trans = context.get("trans")
         if not self.dataset_manager.permissions.manage.is_permitted(dataset, user, trans=trans):
             self.skip()
@@ -451,30 +454,30 @@ class _UnflattenedMetadataDatasetAssociationSerializer(base.ModelSerializer,
         super().add_serializers()
         deletable.PurgableSerializerMixin.add_serializers(self)
 
-        self.serializers.update({
+        serializers: Dict[str, base.Serializer] = {
             'create_time': self.serialize_date,
             'update_time': self.serialize_date,
 
             # underlying dataset
-            'dataset': lambda i, k, **c: self.dataset_serializer.serialize_to_view(i.dataset, view='summary', **c),
-            'dataset_id': self._proxy_to_dataset(key='id'),
+            'dataset': lambda item, key, **context: self.dataset_serializer.serialize_to_view(item.dataset, view='summary', **context),
+            'dataset_id': self._proxy_to_dataset(proxy_key='id'),
             # TODO: why is this named uuid!? The da doesn't have a uuid - it's the underlying dataset's uuid!
-            'uuid': self._proxy_to_dataset(key='uuid'),
+            'uuid': self._proxy_to_dataset(proxy_key='uuid'),
             # 'dataset_uuid': self._proxy_to_dataset( key='uuid' ),
             'file_name': self._proxy_to_dataset(serializer=self.dataset_serializer.serialize_file_name),
             'extra_files_path': self._proxy_to_dataset(serializer=self.dataset_serializer.serialize_extra_files_path),
             'permissions': self._proxy_to_dataset(serializer=self.dataset_serializer.serialize_permissions),
             # TODO: do the sizes proxy accurately/in the same way?
-            'size': lambda i, k, **c: int(i.get_size()),
-            'file_size': lambda i, k, **c: self.serializers['size'](i, k, **c),
-            'nice_size': lambda i, k, **c: i.get_size(nice_size=True),
+            'size': lambda item, key, **context: int(item.get_size()),
+            'file_size': lambda item, key, **context: self.serializers['size'](item, key, **context),
+            'nice_size': lambda item, key, **context: item.get_size(nice_size=True),
 
             # common to lddas and hdas - from mapping.py
             'copied_from_history_dataset_association_id': self.serialize_id,
             'copied_from_library_dataset_dataset_association_id': self.serialize_id,
-            'info': lambda i, k, **c: i.info.strip() if isinstance(i.info, str) else i.info,
-            'blurb': lambda i, k, **c: i.blurb,
-            'peek': lambda i, k, **c: i.display_peek() if i.peek and i.peek != 'no peek' else None,
+            'info': lambda item, key, **context: item.info.strip() if isinstance(item.info, str) else item.info,
+            'blurb': lambda item, key, **context: item.blurb,
+            'peek': lambda item, key, **context: item.display_peek() if item.peek and item.peek != 'no peek' else None,
 
             'meta_files': self.serialize_meta_files,
             'metadata': self.serialize_metadata,
@@ -483,38 +486,40 @@ class _UnflattenedMetadataDatasetAssociationSerializer(base.ModelSerializer,
             'rerunnable': self.serialize_rerunnable,
 
             'parent_id': self.serialize_id,
-            'designation': lambda i, k, **c: i.designation,
+            'designation': lambda item, key, **context: item.designation,
 
             # 'extended_metadata': self.serialize_extended_metadata,
             # 'extended_metadata_id': self.serialize_id,
 
             # remapped
-            'genome_build': lambda i, k, **c: i.dbkey,
+            'genome_build': lambda item, key, **context: item.dbkey,
 
             # derived (not mapped) attributes
-            'data_type': lambda i, k, **c: f"{i.datatype.__class__.__module__}.{i.datatype.__class__.__name__}",
+            'data_type': lambda item, key, **context: f"{item.datatype.__class__.__module__}.{item.datatype.__class__.__name__}",
 
             'converted': self.serialize_converted_datasets,
             # TODO: metadata/extra files
-        })
+        }
+        self.serializers.update(serializers)
         # this an abstract superclass, so no views created
         # because of that: we need to add a few keys that will use the default serializer
         self.serializable_keyset.update(['name', 'state', 'tool_version', 'extension', 'visible', 'dbkey'])
 
-    def _proxy_to_dataset(self, serializer=None, key=None):
+    def _proxy_to_dataset(self, serializer: base.Serializer = None, proxy_key=None):
         # dataset associations are (rough) proxies to datasets - access their serializer using this remapping fn
         # remapping done by either kwarg key: IOW dataset attr key (e.g. uuid)
         # or by kwarg serializer: a function that's passed in (e.g. permissions)
-        if key:
-            serializer = self.dataset_serializer.serializers.get(key)
+        if proxy_key:
+            serializer = self.dataset_serializer.serializers.get(proxy_key)
         if serializer:
-            return lambda i, k, **c: serializer(i.dataset, key or k, **c)
+            return lambda item, key, **context: serializer(item.dataset, proxy_key or key, **context)
         raise TypeError('kwarg serializer or key needed')
 
-    def serialize_meta_files(self, dataset_assoc, key, **context):
+    def serialize_meta_files(self, item, key, **context):
         """
         Cycle through meta files and return them as a list of dictionaries.
         """
+        dataset_assoc = item
         meta_files = []
         for meta_type in dataset_assoc.metadata_file_types:
             if getattr(dataset_assoc.metadata, meta_type, None):
@@ -526,10 +531,11 @@ class _UnflattenedMetadataDatasetAssociationSerializer(base.ModelSerializer,
                                                    metadata_file=meta_type)))
         return meta_files
 
-    def serialize_metadata(self, dataset_assoc, key, excluded=None, **context):
+    def serialize_metadata(self, item, key, excluded=None, **context):
         """
         Cycle through metadata and return as dictionary.
         """
+        dataset_assoc = item
         # dbkey is a repeat actually (metadata_dbkey == genome_build)
         # excluded = [ 'dbkey' ] if excluded is None else excluded
         excluded = [] if excluded is None else excluded
@@ -555,34 +561,37 @@ class _UnflattenedMetadataDatasetAssociationSerializer(base.ModelSerializer,
 
         return metadata
 
-    def serialize_creating_job(self, dataset, key, **context):
+    def serialize_creating_job(self, item, key, **context):
         """
         Return the id of the Job that created this dataset (or its original)
         or None if no `creating_job` is found.
         """
+        dataset = item
         if dataset.creating_job:
             return self.serialize_id(dataset.creating_job, 'id')
         else:
             return None
 
-    def serialize_rerunnable(self, dataset, key, **context):
+    def serialize_rerunnable(self, item, key, **context):
         """
         Return False if this tool that created this dataset can't be re-run
         (e.g. upload).
         """
+        dataset = item
         if dataset.creating_job:
             tool = self.app.toolbox.get_tool(dataset.creating_job.tool_id, dataset.creating_job.tool_version)
             if tool and tool.is_workflow_compatible:
                 return True
         return False
 
-    def serialize_converted_datasets(self, dataset_assoc, key, **context):
+    def serialize_converted_datasets(self, item, key, **context):
         """
         Return a file extension -> converted dataset encoded id map with all
         the existing converted datasets associated with this instance.
 
         This filters out deleted associations.
         """
+        dataset_assoc = item
         id_map = {}
         for converted in dataset_assoc.implicitly_converted_datasets:
             if not converted.deleted and converted.dataset:
diff --git a/lib/galaxy/managers/hdas.py b/lib/galaxy/managers/hdas.py
index fd7d2867e5b..4d19b41281a 100644
--- a/lib/galaxy/managers/hdas.py
+++ b/lib/galaxy/managers/hdas.py
@@ -18,6 +18,7 @@ from galaxy import (
 )
 from galaxy.managers import (
     annotatable,
+    base,
     datasets,
     secured,
     taggable,
@@ -398,10 +399,10 @@ class HDASerializer(  # datasets._UnflattenedMetadataDatasetAssociationSerialize
         taggable.TaggableSerializerMixin.add_serializers(self)
         annotatable.AnnotatableSerializerMixin.add_serializers(self)
 
-        self.serializers.update({
-            'model_class': lambda *a, **c: 'HistoryDatasetAssociation',
-            'history_content_type': lambda *a, **c: 'dataset',
-            'hda_ldda': lambda *a, **c: 'hda',
+        serializers: Dict[str, base.Serializer] = {
+            'model_class': lambda item, key, **context: 'HistoryDatasetAssociation',
+            'history_content_type': lambda item, key, **context: 'dataset',
+            'hda_ldda': lambda item, key, **context: 'hda',
             'type_id': self.serialize_type_id,
             'copied_from_ldda_id': self.serialize_copied_from_ldda_id,
             'history_id': self.serialize_id,
@@ -411,7 +412,7 @@ class HDASerializer(  # datasets._UnflattenedMetadataDatasetAssociationSerialize
             'misc_blurb': self._remap_from('blurb'),
             'file_ext': self._remap_from('extension'),
             'file_path': self._remap_from('file_name'),
-            'resubmitted': lambda i, k, **c: self.hda_manager.has_been_resubmitted(i),
+            'resubmitted': lambda item, key, **context: self.hda_manager.has_been_resubmitted(item),
             'display_apps': self.serialize_display_apps,
             'display_types': self.serialize_old_display_applications,
             'visualizations': self.serialize_visualization_links,
@@ -420,22 +421,23 @@ class HDASerializer(  # datasets._UnflattenedMetadataDatasetAssociationSerialize
             # TODO: this intermittently causes a routes.GenerationException - temp use the legacy route to prevent this
             #   see also: https://trello.com/c/5d6j4X5y
             #   see also: https://sentry.galaxyproject.org/galaxy/galaxy-main/group/20769/events/9352883/
-            'url': lambda i, k, **c: self.url_for('history_content',
-                                                  history_id=self.app.security.encode_id(i.history_id),
-                                                  id=self.app.security.encode_id(i.id)),
+            'url': lambda item, key, **context: self.url_for('history_content',
+                                                             history_id=self.app.security.encode_id(item.history_id),
+                                                             id=self.app.security.encode_id(item.id)),
             'urls': self.serialize_urls,
 
             # TODO: backwards compat: need to go away
-            'download_url': lambda i, k, **c: self.url_for('history_contents_display',
-                                                           history_id=self.app.security.encode_id(i.history.id),
-                                                           history_content_id=self.app.security.encode_id(i.id)),
+            'download_url': lambda item, key, **context: self.url_for('history_contents_display',
+                                                                      history_id=self.app.security.encode_id(item.history.id),
+                                                                      history_content_id=self.app.security.encode_id(item.id)),
             'parent_id': self.serialize_id,
             # TODO: to DatasetAssociationSerializer
-            'accessible': lambda i, k, user=None, **c: self.manager.is_accessible(i, user, **c),
-            'api_type': lambda *a, **c: 'file',
-            'type': lambda *a, **c: 'file',
-            'created_from_basename': lambda i, k, **c: i.created_from_basename,
-        })
+            'accessible': lambda item, key, user=None, **c: self.manager.is_accessible(item, user, **c),
+            'api_type': lambda item, key, **context: 'file',
+            'type': lambda item, key, **context: 'file',
+            'created_from_basename': lambda item, key, **context: item.created_from_basename,
+        }
+        self.serializers.update(serializers)
 
     def serialize(self, hda, keys, user=None, **context):
         """
@@ -446,10 +448,11 @@ class HDASerializer(  # datasets._UnflattenedMetadataDatasetAssociationSerialize
             keys = self._view_to_keys('inaccessible')
         return super().serialize(hda, keys, user=user, **context)
 
-    def serialize_display_apps(self, hda, key, trans=None, **context):
+    def serialize_display_apps(self, item, key, trans=None, **context):
         """
         Return dictionary containing new-style display app urls.
         """
+        hda = item
         display_apps: List[Dict[str, Any]] = []
         for display_app in hda.get_display_applications(trans).values():
 
@@ -465,10 +468,11 @@ class HDASerializer(  # datasets._UnflattenedMetadataDatasetAssociationSerialize
 
         return display_apps
 
-    def serialize_old_display_applications(self, hda, key, trans=None, **context):
+    def serialize_old_display_applications(self, item, key, trans=None, **context):
         """
         Return dictionary containing old-style display app urls.
         """
+        hda = item
         display_apps: List[Dict[str, Any]] = []
         if not self.app.config.enable_old_display_applications:
             return display_apps
@@ -492,20 +496,22 @@ class HDASerializer(  # datasets._UnflattenedMetadataDatasetAssociationSerialize
 
         return display_apps
 
-    def serialize_visualization_links(self, hda, key, trans=None, **context):
+    def serialize_visualization_links(self, item, key, trans=None, **context):
         """
         Return a list of dictionaries with links to visualization pages
         for those visualizations that apply to this hda.
         """
+        hda = item
         # use older system if registry is off in the config
         if not self.app.visualizations_registry:
             return hda.get_visualizations()
         return self.app.visualizations_registry.get_visualizations(trans, hda)
 
-    def serialize_urls(self, hda, key, **context):
+    def serialize_urls(self, item, key, **context):
         """
         Return web controller urls useful for this HDA.
         """
+        hda = item
         url_for = self.url_for
         encoded_id = self.app.security.encode_id(hda.id)
         urls = {
diff --git a/lib/galaxy/managers/hdcas.py b/lib/galaxy/managers/hdcas.py
index e72aa46add3..0c21479bafc 100644
--- a/lib/galaxy/managers/hdcas.py
+++ b/lib/galaxy/managers/hdcas.py
@@ -300,34 +300,33 @@ class HDCASerializer(
         super().add_serializers()
         taggable.TaggableSerializerMixin.add_serializers(self)
         annotatable.AnnotatableSerializerMixin.add_serializers(self)
-
-        self.serializers.update({
-            'model_class': lambda *a, **c: self.hdca_manager.model_class.__class__.__name__,
+        serializers: Dict[str, base.Serializer] = {
+            'model_class': lambda item, key, **context: self.hdca_manager.model_class.__class__.__name__,
             # TODO: remove
-            'type': lambda *a, **c: 'collection',
+            'type': lambda item, key, **context: 'collection',
             # part of a history and container
             'history_id': self.serialize_id,
-            'history_content_type': lambda *a, **c: self.hdca_manager.model_class.content_type,
+            'history_content_type': lambda item, key, **context: self.hdca_manager.model_class.content_type,
             'type_id': self.serialize_type_id,
             'job_source_id': self.serialize_id,
-
-            'url': lambda i, k, **c: self.url_for('history_content_typed',
-                                                  history_id=self.app.security.encode_id(i.history_id),
-                                                  id=self.app.security.encode_id(i.id),
-                                                  type=self.hdca_manager.model_class.content_type),
+            'url': lambda item, key, **context: self.url_for('history_content_typed',
+                                                             history_id=self.app.security.encode_id(item.history_id),
+                                                             id=self.app.security.encode_id(item.id),
+                                                             type=self.hdca_manager.model_class.content_type),
             'contents_url': self.generate_contents_url,
             'job_state_summary': self.serialize_job_state_summary
-        })
+        }
+        self.serializers.update(serializers)
 
-    def generate_contents_url(self, hdca, key, **context):
+    def generate_contents_url(self, item, key, **context):
         encode_id = self.app.security.encode_id
         contents_url = self.url_for('contents_dataset_collection',
-            hdca_id=encode_id(hdca.id),
-            parent_id=encode_id(hdca.collection_id))
+            hdca_id=encode_id(item.id),
+            parent_id=encode_id(item.collection_id))
         return contents_url
 
-    def serialize_job_state_summary(self, hdca, key, **context):
-        states = hdca.job_state_summary.__dict__.copy()
+    def serialize_job_state_summary(self, item, key, **context):
+        states = item.job_state_summary.__dict__.copy()
         del states['_sa_instance_state']
         del states['hdca_id']
         return states
diff --git a/lib/galaxy/managers/history_contents.py b/lib/galaxy/managers/history_contents.py
index 323d6f6ae68..3c0dcbc2072 100644
--- a/lib/galaxy/managers/history_contents.py
+++ b/lib/galaxy/managers/history_contents.py
@@ -450,19 +450,19 @@ class HistoryContentsSerializer(base.ModelSerializer, deletable.PurgableSerializ
     def add_serializers(self):
         super().add_serializers()
         deletable.PurgableSerializerMixin.add_serializers(self)
-
-        self.serializers.update({
+        serializers: Dict[str, Serializer] = {
             'type_id': self.serialize_type_id,
             'history_id': self.serialize_id,
             'dataset_id': self.serialize_id_or_skip,
             'collection_id': self.serialize_id_or_skip,
-        })
+        }
+        self.serializers.update(serializers)
 
-    def serialize_id_or_skip(self, content, key, **context):
+    def serialize_id_or_skip(self, item: Any, key: str, **context):
         """Serialize id or skip if attribute with `key` is not present."""
-        if not hasattr(content, key):
+        if not hasattr(item, key):
             raise base.SkipAttribute('no such attribute')
-        return self.serialize_id(content, key, **context)
+        return self.serialize_id(item, key, **context)
 
 
 class HistoryContentsFilters(base.ModelFilterParser,
diff --git a/setup.cfg b/setup.cfg
index a78f7bb2111..e5339897287 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -605,8 +605,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.tools.evaluation]
 check_untyped_defs = False
-[mypy-galaxy.managers.history_contents]
-check_untyped_defs = False
 [mypy-galaxy.jobs.runners]
 check_untyped_defs = False
 [mypy-galaxy.jobs]

From e9db538fc1e5ab771ba75ca09dd65fca27837be3 Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Tue, 26 Oct 2021 16:23:18 -0400
Subject: [PATCH 8/8] Remove unused manager containers code.

---
 lib/galaxy/managers/containers.py       | 114 ------------------------
 lib/galaxy/managers/history_contents.py |  14 +--
 setup.cfg                               |   2 -
 3 files changed, 7 insertions(+), 123 deletions(-)
 delete mode 100644 lib/galaxy/managers/containers.py

diff --git a/lib/galaxy/managers/containers.py b/lib/galaxy/managers/containers.py
deleted file mode 100644
index 860fdcf4d23..00000000000
--- a/lib/galaxy/managers/containers.py
+++ /dev/null
@@ -1,114 +0,0 @@
-"""
-Manager mixins to unify the interface into things that can contain: Datasets
-and other (nested) containers.
-
-(e.g. DatasetCollections, Histories, LibraryFolders)
-"""
-# Histories should be DatasetCollections.
-# Libraries should be DatasetCollections.
-import logging
-import operator
-from typing import Optional, Type
-
-import galaxy.exceptions
-import galaxy.util
-from galaxy import model
-
-log = logging.getLogger(__name__)
-
-
-# ====
-class ContainerManagerMixin:
-    """
-    A class that tracks/contains two types of items:
-        1) some non-container object (such as datasets)
-        2) other sub-containers nested within this one
-
-    Levels of nesting are not considered here; In other words,
-    each of the methods below only work on the first level of
-    nesting.
-    """
-    # TODO: terminology is getting a bit convoluted and silly at this point: rename three public below?
-    # TODO: this should be an open mapping (not just 2)
-    #: the classes that can be contained
-    contained_class: Type[model._HasTable]
-    subcontainer_class: Type[model._HasTable]
-    #: how any contents lists produced are ordered - (string) attribute name to sort on or tuple of attribute names
-    default_order_by: Optional[str] = None
-
-    # ---- interface
-    def contents(self, container):
-        """
-        Returns both types of contents: filtered and in some order.
-        """
-        iters = []
-        iters.append(self.contained(container))
-        iters.append(self.subcontainers(container))
-        return galaxy.util.merge_sorted_iterables(self.order_contents_on, *iters)
-
-    def contained(self, container, **kwargs):
-        """
-        Returns non-container objects.
-        """
-        return self._filter_contents(container, self.contained_class, **kwargs)
-
-    def subcontainers(self, container, **kwargs):
-        """
-        Returns only the containers within this one.
-        """
-        return self._filter_contents(container, self.subcontainer_class, **kwargs)
-
-    # ---- private
-    def _filter_contents(self, container, content_class, **kwargs):
-        # TODO: use list (or by_history etc.)
-        container_filter = self._filter_to_contained(container, content_class)
-        query = self.session().query(content_class).filter(container_filter)
-        return query
-
-    def _get_filter_for_contained(self, container, content_class):
-        raise galaxy.exceptions.NotImplemented('Abstract class')
-
-    def _content_manager(self, content):
-        raise galaxy.exceptions.NotImplemented('Abstract class')
-
-
-class LibraryFolderAsContainerManagerMixin(ContainerManagerMixin):
-    # can contain two types of subcontainer: LibraryFolder, LibraryDatasetCollectionAssociation
-    # has as the top level container: Library
-
-    contained_class = model.LibraryDataset
-    subcontainer_class = model.LibraryFolder
-    # subcontainer_class = model.LibraryDatasetCollectionAssociation
-    order_contents_on = operator.attrgetter('create_time')
-
-    def _get_filter_for_contained(self, container, content_class):
-        if content_class == self.subcontainer_class:
-            return self.subcontainer_class.parent == container
-        return self.contained_class.folder == container
-
-    def _content_manager(self, content):
-        # type snifffing is inevitable
-        if isinstance(content, model.LibraryDataset):
-            return self.lda_manager
-        elif isinstance(content, model.LibraryFolder):
-            return self.folder_manager
-        raise TypeError(f"Unknown contents class: {str(content)}")
-
-
-class DatasetCollectionAsContainerManagerMixin(ContainerManagerMixin):
-
-    # (note: unlike the other collections, dc's wrap both contained and subcontainers in this class)
-    contained_class = model.DatasetCollectionElement
-    subcontainer_class = model.DatasetCollection
-    order_contents_on = operator.attrgetter('element_index')
-
-    def _get_filter_for_contained(self, container, content_class):
-        return content_class.collection == container
-
-    def _content_manager(self, content):
-        # type snifffing is inevitable
-        if isinstance(content, model.DatasetCollectionElement):
-            return self.collection_manager
-        elif isinstance(content, model.DatasetCollection):
-            return self.collection_manager
-        raise TypeError(f"Unknown contents class: {str(content)}")
diff --git a/lib/galaxy/managers/history_contents.py b/lib/galaxy/managers/history_contents.py
index 3c0dcbc2072..7766e35d905 100644
--- a/lib/galaxy/managers/history_contents.py
+++ b/lib/galaxy/managers/history_contents.py
@@ -3,6 +3,7 @@ Heterogenous lists/contents are difficult to query properly since unions are
 not easily made.
 """
 import logging
+from typing import Any, Dict, List
 
 from sqlalchemy import (
     asc,
@@ -25,7 +26,6 @@ from galaxy import (
 from galaxy.managers import (
     annotatable,
     base,
-    containers,
     deletable,
     hdas,
     hdcas,
@@ -33,14 +33,14 @@ from galaxy.managers import (
     tools
 )
 from galaxy.structured_app import MinimalManagerApp
-from .base import raise_filter_err
+from .base import raise_filter_err, Serializer
 
 log = logging.getLogger(__name__)
 
 
 # into its own class to have it's own filters, etc.
 # TODO: but can't inherit from model manager (which assumes only one model)
-class HistoryContentsManager(containers.ContainerManagerMixin, base.SortableManager):
+class HistoryContentsManager(base.SortableManager):
 
     root_container_class = model.History
 
@@ -225,7 +225,7 @@ class HistoryContentsManager(containers.ContainerManagerMixin, base.SortableMana
             return contents_results
 
         # partition ids into a map of { component_class names -> list of ids } from the above union query
-        id_map = dict(((self.contained_class_type_name, []), (self.subcontainer_class_type_name, [])))
+        id_map: Dict[str, List[int]] = dict([(self.contained_class_type_name, []), (self.subcontainer_class_type_name, [])])
         for result in contents_results:
             result_type = self._get_union_type(result)
             contents_id = self._get_union_id(result)
@@ -344,14 +344,14 @@ class HistoryContentsManager(containers.ContainerManagerMixin, base.SortableMana
         )
         subquery = self._session().query(*columns)
         # for the HDA's we need to join the Dataset since it has an actual state column
-        subquery = subquery.join(model.Dataset, model.Dataset.id == component_class.dataset_id)
+        subquery = subquery.join(model.Dataset, model.Dataset.id == component_class.table.c.dataset_id)
         if history_id:
-            subquery = subquery.filter(component_class.history_id == history_id)
+            subquery = subquery.filter(component_class.table.c.history_id == history_id)
         else:
             # Make sure we only return items that are user-accessible by checking that they are in a history
             # owned by the current user.
             # TODO: move into filter mixin, and implement accessible logic as SQL query
-            subquery = subquery.filter(component_class.history_id == model.History.table.c.id,
+            subquery = subquery.filter(component_class.table.c.history_id == model.History.table.c.id,
                                        model.History.table.c.user_id == user_id)
         return subquery
 
diff --git a/setup.cfg b/setup.cfg
index e5339897287..23ce19b9a29 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -403,8 +403,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.managers.taggable]
 check_untyped_defs = False
-[mypy-galaxy.managers.containers]
-check_untyped_defs = False
 [mypy-galaxy.managers.collections_util]
 check_untyped_defs = False
 [mypy-galaxy.jobs.splitters.multi]