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

@@ -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):

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

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):
         """

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 '-'

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

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]