Managers: extract manager() code and move into class with property accessor

lib/galaxy/managers/annotatable.py

@@ -71,7 +71,7 @@ class AnnotatableDeserializerMixin( object ):
         if `val` is None.
         """
         val = self.validate.nullable_basestring( key, val )
-        return self.manager().annotate( item, val, user=user, flush=False )
+        return self.manager.annotate( item, val, user=user, flush=False )
 
 
 # TODO: I'm not entirely convinced this (or tags) are a good idea for filters since they involve a/the user

lib/galaxy/managers/base.py

@@ -493,6 +493,31 @@ class ModelManager( object ):
     #    return item
 
 
+# ---- code for classes that use one *main* model manager
+# TODO: this may become unecessary if we can access managers some other way (class var, app, etc.)
+class HasAModelManager( object ):
+    """
+    Mixin used where serializers, deserializers, filter parsers, etc.
+    need some functionality around the model they're mainly concerned with
+    and would perform that functionality with a manager.
+    """
+
+    #: the class used to create this serializer's generically accessible model_manager
+    model_manager_class = None
+
+    def __init__( self, app, manager=None, **kwargs ):
+        self._manager = manager
+
+    @property
+    def manager( self ):
+        """Return an appropriate manager if it exists, instantiate if not."""
+        # PRECONDITION: assumes self.app is assigned elsewhere
+        if not self._manager:
+            # TODO: pass this serializer to it
+            self._manager = self.model_manager_class( self.app )
+        return self._manager
+
+
 # ==== SERIALIZERS/to_dict,from_dict
 class ModelSerializingError( exceptions.InternalServerError ):
     """Thrown when request model values can't be serialized"""
@@ -514,7 +539,7 @@ class SkipAttribute( Exception ):
     pass
 
 
-class ModelSerializer( object ):
+class ModelSerializer( HasAModelManager ):
     """
     Turns models into JSONable dicts.
 
@@ -531,17 +556,15 @@ class ModelSerializer( object ):
         keys_to_serialize = [ 'id', 'name', 'attr1', 'attr2', ... ]
         item_dict = MySerializer.serialize( my_item, keys_to_serialize )
     """
-    #: the class used to create this serializer's generically accessible model_manager
-    model_manager_class = None
     #: 'service' to use for getting urls - use class var to allow overriding when testing
     url_for = staticmethod( routes.url_for )
 
-    def __init__( self, app, manager=None ):
+    def __init__( self, app, **kwargs ):
         """
         Set up serializer map, any additional serializable keys, and views here.
         """
+        super( ModelSerializer, self ).__init__( app, **kwargs )
         self.app = app
-        self._manager = manager
 
         # 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
@@ -560,13 +583,6 @@ class ModelSerializer( object ):
         self.views = {}
         self.default_view = None
 
-    def manager( self ):
-        """Return an appropriate manager if it exists, instantiate if not."""
-        if not self._manager:
-            # TODO: pass this serializer to it
-            self._manager = self.model_manager_class( self.app )
-        return self._manager
-
     def add_serializers( self ):
         """
         Register a map of attribute keys -> serializing functions that will serialize
@@ -692,22 +708,19 @@ class ModelSerializer( object ):
         return self.views[ view ][:]
 
 
-class ModelDeserializer( object ):
+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.
     """
-    #: the class used to create this deserializer's generically accessible model_manager
-    model_manager_class = None
-
     # TODO:?? a larger question is: which should be first? Deserialize then validate - or - validate then deserialize?
 
-    def __init__( self, app, manager=None ):
+    def __init__( self, app, **kwargs ):
         """
         Set up deserializers and validator.
         """
+        super( ModelDeserializer, self ).__init__( app, **kwargs )
         self.app = app
-        self._manager = None
 
         self.deserializers = {}
         self.deserializable_keyset = set([])
@@ -715,13 +728,6 @@ class ModelDeserializer( object ):
         # a sub object that can validate incoming values
         self.validate = ModelValidator( self.app )
 
-    def manager( self ):
-        """Return an appropriate manager if it exists, instantiate if not."""
-        if not self._manager:
-            # TODO: pass this deserializer to it
-            self._manager = self.model_manager_class( self.app )
-        return self._manager
-
     def add_deserializers( self ):
         """
         Register a map of attribute keys -> functions that will deserialize data
@@ -787,7 +793,7 @@ class ModelDeserializer( object ):
         return self.default_deserializer( item, key, val, **context )
 
 
-class ModelValidator( object ):
+class ModelValidator( HasAModelManager ):
     """
     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
@@ -795,6 +801,7 @@ class ModelValidator( object ):
     """
 
     def __init__( self, app, *args, **kwargs ):
+        super( ModelValidator, self ).__init__( app, **kwargs )
         self.app = app
 
     def type( self, key, val, types ):
@@ -870,7 +877,7 @@ class ModelValidator( object ):
 
 
 # ==== Building query filters based on model data
-class ModelFilterParser( object ):
+class ModelFilterParser( HasAModelManager ):
     """
     Converts string tuples (partially converted query string params) of
     attr, op, val into either:
@@ -889,9 +896,6 @@ class ModelFilterParser( object ):
     These might be safely be replaced in the future by creating SQLAlchemy
     hybrid properties or more thoroughly mapping derived values.
     """
-    #: the class used to create this deserializer's generically accessible model_manager
-    model_manager_class = None
-
     # ??: this class kindof 'lives' in both the world of the controllers/param-parsing and to models/orm
     # (as the model informs how the filter params are parsed)
     # I have no great idea where this 'belongs', so it's here for now
@@ -899,12 +903,12 @@ class ModelFilterParser( object ):
     #: model class
     model_class = None
 
-    def __init__( self, app, manager=None ):
+    def __init__( self, app, **kwargs ):
         """
         Set up serializer map, any additional serializable keys, and views here.
         """
+        super( ModelFilterParser, self ).__init__( app, **kwargs )
         self.app = app
-        self._manager = manager
 
         # dictionary containing parsing data for ORM/SQLAlchemy-based filters
         # ..note: although kind of a pain in the ass and verbose, opt-in/whitelisting allows more control
@@ -917,13 +921,6 @@ class ModelFilterParser( object ):
         # set up both of the above
         self._add_parsers()
 
-    def manager( self ):
-        """Return an appropriate manager if it exists, instantiate if not."""
-        if not self._manager:
-            # TODO: pass this parser to it
-            self._manager = self.model_manager_class( self.app )
-        return self._manager
-
     def _add_parsers( self ):
         """
         Set up, extend, or alter `orm_filter_parsers` and `fn_filter_parsers`.

lib/galaxy/managers/datasets.py

@@ -234,11 +234,11 @@ class DatasetDeserializer( base.ModelDeserializer, deletable.PurgableDeserialize
         `permissions` dictionary, where `permissions` is in the form:
             { 'manage': [ <role id 1>, ... ], 'access': [ <role id 2>, ... ] }
         """
-        self.manager().permissions.manage.error_unless_permitted( dataset, user )
+        self.manager.permissions.manage.error_unless_permitted( dataset, user )
         self._validate_permissions( permissions, **context )
         manage = self._list_of_roles_from_ids( permissions[ 'manage' ] )
         access = self._list_of_roles_from_ids( permissions[ 'access' ] )
-        self.manager().permissions.set( dataset, manage, access, flush=False )
+        self.manager.permissions.set( dataset, manage, access, flush=False )
         return permissions
 
     def _validate_permissions( self, permissions, **context ):

lib/galaxy/managers/deletable.py

@@ -52,9 +52,9 @@ class DeletableDeserializerMixin( object ):
             return item.deleted
         # TODO:?? flush=False?
         if new_deleted:
-            self.manager().delete( item, flush=False )
+            self.manager.delete( item, flush=False )
         else:
-            self.manager().undelete( item, flush=False )
+            self.manager.undelete( item, flush=False )
         return item.deleted
 
 
@@ -103,7 +103,7 @@ class PurgableDeserializerMixin( DeletableDeserializerMixin ):
             return item.purged
         # do we want to error if something attempts to 'unpurge'?
         if new_purged:
-            self.manager().purge( item, flush=False )
+            self.manager.purge( item, flush=False )
         return item.purged
 
 

lib/galaxy/managers/hdas.py

@@ -443,7 +443,7 @@ class HDADeserializer( datasets.DatasetAssociationDeserializer,
 
     def __init__( self, app ):
         super( HDADeserializer, self ).__init__( app )
-        self.hda_manager = self.manager()
+        self.hda_manager = self.manager
 
     def add_deserializers( self ):
         super( HDADeserializer, self ).add_deserializers()

lib/galaxy/managers/histories.py

@@ -158,7 +158,7 @@ class HistorySerializer( sharable.SharableModelSerializer, deletable.PurgableSer
     def __init__( self, app, **kwargs ):
         super( HistorySerializer, self ).__init__( app, **kwargs )
 
-        self.history_manager = self.manager()
+        self.history_manager = self.manager
         self.hda_manager = hdas.HDAManager( app )
         self.hda_serializer = hdas.HDASerializer( app )
 
@@ -328,7 +328,7 @@ class HistoryDeserializer( sharable.SharableModelDeserializer, deletable.Purgabl
 
     def __init__( self, app ):
         super( HistoryDeserializer, self ).__init__( app )
-        self.history_manager = self.manager()
+        self.history_manager = self.manager
 
     def add_deserializers( self ):
         super( HistoryDeserializer, self ).add_deserializers()

lib/galaxy/managers/pages.py

@@ -65,7 +65,7 @@ class PageDeserializer( sharable.SharableModelDeserializer ):
 
     def __init__( self, app ):
         super( PageDeserializer, self ).__init__( app )
-        self.page_manager = self.manager()
+        self.page_manager = self.manager
 
     def add_deserializers( self ):
         super( PageDeserializer, self ).add_deserializers()

lib/galaxy/managers/ratable.py

@@ -27,7 +27,7 @@ class RatableManagerMixin( object ):
         return rating.rating if rating is not None else None
 
     def ratings( self, item ):
-        """Returns all ratings given to this item."""
+        """Returns a list of all rating values given to this item."""
         return [ r.rating for r in item.ratings ]
 
     def ratings_avg( self, item ):
@@ -37,7 +37,7 @@ class RatableManagerMixin( object ):
         return avg or 0.0
 
     def ratings_count( self, item ):
-        """Returns the average of all ratings given to this item."""
+        """Returns the number of ratings given to this item."""
         foreign_key = self._foreign_key( self.rating_assoc )
         return self.session().query( func.count( self.rating_assoc.rating ) ).filter( foreign_key == item ).scalar()
 
@@ -69,8 +69,8 @@ class RatableSerializerMixin( object ):
         """Returns the integer rating given to this item by the user."""
         if not user:
             raise base.ModelSerializingError( 'user_rating requires a user',
-                model_class=self.manager().model_class, id=self.serialize_id( item, 'id' ) )
-        return self.manager().rating( item, user )
+                model_class=self.manager.model_class, id=self.serialize_id( item, 'id' ) )
+        return self.manager.rating( item, user )
 
     def serialize_community_rating( self, item, key, **context ):
         """
@@ -80,7 +80,7 @@ class RatableSerializerMixin( object ):
         """
         # ??: seems like two queries (albeit in-sql functions) would slower
         # than getting the rows and calc'ing both here with one query
-        manager = self.manager()
+        manager = self.manager
         return {
             'average' : manager.ratings_avg( item ),
             'count'   : manager.ratings_count( item ),
@@ -95,15 +95,15 @@ class RatableDeserializerMixin( object ):
     def deserialize_rating( self, item, key, val, user=None, **context ):
         if not user:
             raise base.ModelDeserializingError( 'user_rating requires a user',
-                model_class=self.manager().model_class, id=self.serialize_id( item, 'id' ) )
+                model_class=self.manager.model_class, id=self.serialize_id( item, 'id' ) )
         val = self.validate.int_range( key, val, 0, 5 )
-        return self.manager().rate( item, user, val, flush=False )
+        return self.manager.rate( item, user, val, flush=False )
 
 
 class RatableFilterMixin( object ):
 
     def _ratings_avg_accessor( self, item ):
-        return self.manager().ratings_avg( item )
+        return self.manager.ratings_avg( item )
 
     def _add_parsers( self ):
         """

lib/galaxy/managers/sharable.py

@@ -383,9 +383,9 @@ class SharableModelDeserializer( base.ModelDeserializer,
             return val
 
         if val:
-            self.manager().publish( item, flush=False )
+            self.manager.publish( item, flush=False )
         else:
-            self.manager().unpublish( item, flush=False )
+            self.manager.unpublish( item, flush=False )
         return item.published
 
     def deserialize_importable( self, item, key, val, **context ):
@@ -396,9 +396,9 @@ class SharableModelDeserializer( base.ModelDeserializer,
             return val
 
         if val:
-            self.manager().make_importable( item, flush=False )
+            self.manager.make_importable( item, flush=False )
         else:
-            self.manager().make_non_importable( item, flush=False )
+            self.manager.make_non_importable( item, flush=False )
         return item.published
 
     # def deserialize_slug( self, item, val, **context ):

lib/galaxy/managers/visualizations.py

@@ -67,7 +67,7 @@ class VisualizationDeserializer( sharable.SharableModelDeserializer ):
 
     def __init__( self, app ):
         super( VisualizationDeserializer, self ).__init__( app )
-        self.visualization_manager = self.manager()
+        self.visualization_manager = self.manager
 
     def add_deserializers( self ):
         super( VisualizationDeserializer, self ).add_deserializers()