From 89cb03a037d770c4c895d4e8f872de04d385ada9 Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Fri, 15 Apr 2016 14:47:45 -0400 Subject: [PATCH] History contents manager: improve contents query Serializers in the index_v2 of history_contents were re- querying the database for mapped model attributes. - eagerload collection on hdcas - undefer _metadata on hdas - eaglerload tags and annotations for both - rework annotatable manager mixin to cycle through annotations bypassing the get_item_annotation_str call (which would bypass item.annotations even if already loaded) Anecdotally provides significant speed up. --- lib/galaxy/managers/annotatable.py | 27 +++++++++++++--------- lib/galaxy/managers/history_contents.py | 30 +++++++++++++++++++++---- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/lib/galaxy/managers/annotatable.py b/lib/galaxy/managers/annotatable.py index 0b25a947db9..7a0aa653c08 100644 --- a/lib/galaxy/managers/annotatable.py +++ b/lib/galaxy/managers/annotatable.py @@ -6,6 +6,18 @@ import logging log = logging.getLogger( __name__ ) +# needed to extract this for use in manager *and* serializer, ideally, would use self.manager.annotation +# from serializer, but history_contents has no self.manager +# TODO: fix +def _match_by_user( item, user ): + if not user: + return None + for annotation in item.annotations: + if annotation.user_id == user.id: + return annotation.annotation + return None + + class AnnotatableManagerMixin( object ): #: class of AnnotationAssociation (e.g. HistoryAnnotationAssociation) annotation_assoc = None @@ -36,7 +48,7 @@ class AnnotatableManagerMixin( object ): return annotation_obj def _user_annotation( self, item, user ): - return item.get_item_annotation_str( self.session(), user, item ) + return _match_by_user( item, user ) def _delete_annotation( self, item, user, flush=True ): returned = item.delete_item_annotation( self.session(), user, item ) @@ -54,10 +66,8 @@ class AnnotatableSerializerMixin( object ): """ Get and serialize an `item`'s annotation. """ - # user = item.user - sa_session = self.app.model.context - returned = item.get_item_annotation_str( sa_session, user, item ) - return returned + annotation = _match_by_user( item, user ) + return annotation.strip() if annotation else None class AnnotatableDeserializerMixin( object ): @@ -81,12 +91,7 @@ class AnnotatableFilterMixin( object ): """ Get the annotation by the item's owner. """ - if not item.user: - return None - for annotation in item.annotations: - if annotation.user == item.user: - return annotation.annotation - return None + return _match_by_user( item, item.user ) def filter_annotation_contains( self, item, val ): """ diff --git a/lib/galaxy/managers/history_contents.py b/lib/galaxy/managers/history_contents.py index 7806e83385e..7a40e734ba8 100644 --- a/lib/galaxy/managers/history_contents.py +++ b/lib/galaxy/managers/history_contents.py @@ -8,6 +8,8 @@ from sqlalchemy import sql from sqlalchemy import asc, desc from sqlalchemy import true, false from sqlalchemy import func +from sqlalchemy.orm import eagerload +from sqlalchemy.orm import undefer from galaxy import model from galaxy import exceptions as glx_exceptions @@ -207,8 +209,10 @@ class HistoryContentsManager( containers.ContainerManagerMixin ): raise TypeError( 'Unknown contents type:', result_type ) # query 2 & 3: use the ids to query each component_class, returning an id->full component model map - id_map[ self.contained_class_type_name ] = self._by_ids( self.contained_class, id_map[ self.contained_class_type_name ] ) - id_map[ self.subcontainer_class_type_name ] = self._by_ids( self.subcontainer_class, id_map[ self.subcontainer_class_type_name ] ) + contained_ids = id_map[ self.contained_class_type_name ] + id_map[ self.contained_class_type_name ] = self._contained_id_map( contained_ids ) + subcontainer_ids = id_map[ self.subcontainer_class_type_name ] + id_map[ self.subcontainer_class_type_name ] = self._subcontainer_id_map( subcontainer_ids ) # cycle back over the union query to create an ordered list of the objects returned in queries 2 & 3 above contents = [] @@ -310,10 +314,28 @@ class HistoryContentsManager( containers.ContainerManagerMixin ): """Return the id for this row in the union results""" return union[ 2 ] - def _by_ids( self, component_class, id_list ): + def _contained_id_map( self, id_list ): + """Return an id to model map of all contained-type models in the id_list.""" if not id_list: return [] - query = self._session().query( component_class ).filter( component_class.id.in_( id_list ) ) + component_class = self.contained_class + query = ( self._session().query( component_class ) + .filter( component_class.id.in_( id_list ) ) + .options( undefer( '_metadata' ) ) + .options( eagerload( 'tags' ) ) + .options( eagerload( 'annotations' ) ) ) + return dict( ( row.id, row ) for row in query.all() ) + + def _subcontainer_id_map( self, id_list ): + """Return an id to model map of all subcontainer-type models in the id_list.""" + if not id_list: + return [] + component_class = self.subcontainer_class + query = ( self._session().query( component_class ) + .filter( component_class.id.in_( id_list ) ) + .options( eagerload( 'collection' ) ) + .options( eagerload( 'tags' ) ) + .options( eagerload( 'annotations' ) ) ) return dict( ( row.id, row ) for row in query.all() )