1 new commit in galaxy-central: https://bitbucket.org/galaxy/galaxy-central/commits/68f0ed37e089/ Changeset: 68f0ed37e089 User: carlfeberhard Date: 2015-01-29 15:42:54+00:00 Summary: History API: allow filtering, limit, and offset in index params; Implement managers/base.FilterParser to configure how models can be filtered; Change ModelManager.list to allow limit, offset, and post-query (functional) filters; Add user filter, limit, and offset parsers to BaseController; Add tests for previous; Remove some UserManager functions; Continue filling out Annotatable mixin Affected #: 13 files diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 lib/galaxy/managers/annotatable.py --- a/lib/galaxy/managers/annotatable.py +++ b/lib/galaxy/managers/annotatable.py @@ -11,6 +11,26 @@ annotation_assoc = None # TODO: most of this seems to be covered by item_attrs.UsesAnnotations + # TODO: use these below (serializer/deserializer) + def user_annotation( self, trans, item, user ): + return item.get_item_annotation_str( self.app.model.context, user, item ) + + def owner_annotation( self, trans, item ): + return self.user_annotation( trans, item, item.user ) + + def delete_annotation( self, trans, item, user ): + return item.delete_item_annotation( self.app.model.context, user, item ) + + def annotate( self, trans, item, user, annotation ): + if annotation is None: + self.delete_annotation( self, trans, item, user ) + return None + + annotation_obj = item.add_item_annotation( self.app.model.context, user, item, annotation ) + return annotation_obj.annotation + + #def by_user( self, trans, user, **kwargs ): + # pass class AnnotatableSerializer( object ): diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 lib/galaxy/managers/base.py --- a/lib/galaxy/managers/base.py +++ b/lib/galaxy/managers/base.py @@ -32,6 +32,7 @@ from galaxy import exceptions from galaxy import model from galaxy.model import tool_shed_install +from galaxy.managers import filters as filter_parser import logging log = logging.getLogger( __name__ ) @@ -159,12 +160,7 @@ def __init__( self, app ): self.app = app - def _default_order_by( self ): - """ - Returns a tuple of columns for the default order when getting multiple models. - """ - return ( self.model_class.create_time, ) - + # .... query foundation wrapper def query( self, trans, eagerloads=True, filters=None, order_by=None, limit=None, offset=None, **kwargs ): """ Return a basic query from model_class, filters, order_by, and limit and offset. @@ -172,17 +168,17 @@ Set eagerloads to False to disable them for this query. """ query = trans.sa_session.query( self.model_class ) - # joined table loading if eagerloads is False: query = query.enable_eagerloads( False ) - # TODO: if non-orm filters are the only option, here is where they'd go - query = self._apply_filters( query, filters ) - query = self._apply_order_by_limit_offset( query, order_by, limit, offset ) + query = self._apply_orm_filters( query, filters ) + query = self._apply_order_by( query, order_by ) + query = self._apply_orm_limit_offset( query, limit, offset ) return query - def _apply_filters( self, query, filters ): + # .... filters + def _apply_orm_filters( self, query, filters ): """ Add any filters to the given query. """ @@ -213,13 +209,7 @@ filtersB = [ filtersB ] return filtersA + filtersB - def _apply_order_by_limit_offset( self, query, order_by, limit, offset ): - """ - Return the query after adding the order_by, limit, and offset clauses. - """ - query = self._apply_order_by( query, order_by ) - return self._apply_limit_offset( query, limit, offset ) - + # .... order, limit, and offset def _apply_order_by( self, query, order_by ): """ Return the query after adding the order_by clauses. @@ -233,7 +223,13 @@ return query.order_by( *order_by ) return query.order_by( order_by ) - def _apply_limit_offset( self, query, limit, offset ): + def _default_order_by( self ): + """ + Returns a tuple of columns for the default order when getting multiple models. + """ + return ( self.model_class.create_time, ) + + def _apply_orm_limit_offset( self, query, limit, offset ): """ Return the query after applying the given limit and offset (if not None). """ @@ -243,6 +239,7 @@ query = query.offset( offset ) return query + # .... query resolution def one( self, trans, **kwargs ): """ Sends kwargs to build the query and returns one and only one model. @@ -284,25 +281,94 @@ id_filter = self.model_class.id == id return self.one( trans, filters=id_filter, **kwargs ) - def list( self, trans, query=None, **kwargs ): + # .... multirow queries + def _orm_list( self, trans, query=None, **kwargs ): """ Sends kwargs to build the query return all models found. """ query = query or self.query( trans, **kwargs ) return query.all() - def _query_by_ids( self, trans, ids, filters=None, **kwargs ): + #def list( self, trans, query=None, filters=None, order_by=None, limit=None, offset=None, **kwargs ): + def list( self, trans, filters=None, order_by=None, limit=None, offset=None, **kwargs ): """ - Builds a query to find a list of models with the given list of `ids`. + Returns all objects matching the given filters """ - ids_filter = self.model_class.id.in_( ids ) - return self.query( trans, filters=self._munge_filters( ids_filter, filters ), **kwargs ) + orm_filters, fn_filters = self._split_filters( filters ) + if not fn_filters: + # if no fn_filtering required, we can use the 'all orm' version with limit offset + return self._orm_list( trans, filters=orm_filters, order_by=order_by, limit=limit, offset=offset, **kwargs ) - def by_ids( self, trans, ids, **kwargs ): + # fn filters will change the number of items returnable by limit/offset - remove them here from the orm query + query = self.query( trans, filters=orm_filters, order_by=order_by, limit=None, offset=None, **kwargs ) + items = query.all() + + # apply limit, offset after SQL filtering + items = self._apply_fn_filters_gen( items, fn_filters ) + return list( self._apply_fn_limit_offset_gen( items, limit, offset ) ) + + def _split_filters( self, filters ): + """ + Splits `filters` into a tuple of two lists: + 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 = ( [], [] ) + if filters is None: + return ( orm_filters, fn_filters ) + if not isinstance( filters, list ): + filters = [ filters ] + for filter_ in filters: + if self._is_fn_filter( filter_ ): + fn_filters.append( filter_ ) + else: + orm_filters.append( filter_ ) + return ( orm_filters, fn_filters ) + + def _is_fn_filter( self, filter_ ): + """ + Returns True if `filter_` is a functional filter to be applied after the SQL query. + """ + return callable( filter_ ) + + def _apply_fn_filters_gen( self, items, filters ): + """ + If all the filter functions in `filters` return True for an item in `items`, + yield that item. + """ + #cpu-expensive + for item in items: + filter_results = map( lambda f: f( item ), filters ) + if all( filter_results ): + yield item + + def _apply_fn_limit_offset_gen( self, items, limit, offset ): + """ + Iterate over `items` and begin yielding items after + `offset` number of items and stop when we've yielded + `limit` number of items. + """ + # change negative limit, offset to None + if limit is not None and limit < 0: + limit = None + if offset is not None and offset < 0: + offset = None + + yielded = 0 + for i, item in enumerate( items ): + if offset is not None and i < offset: + continue + if limit is not None and yielded >= limit: + break + yield item + yielded += 1 + + def by_ids( self, trans, ids, filters=None, **kwargs ): """ Returns an in-order list of models with the matching ids in `ids`. """ - found = self._query_by_ids( trans, ids, **kwargs ).all() + ids_filter = self.model_class.id.in_( ids ) + found = self.list( trans, filters=self._munge_filters( ids_filter, filters ), **kwargs ) # TODO: this does not order by the original 'ids' array # ...could use get (supposedly since found are in the session, the db won't be hit twice) @@ -607,6 +673,9 @@ val = self.validate.int_range( key, val, min, max ) return self.default_deserializer( trans, item, key, val ) + #def deserialize_date( self, trans, item, key, val ): + # #TODO: parse isoformat date into date object + # ... common deserializers for Galaxy def deserialize_genome_build( self, trans, item, key, val ): """ @@ -691,6 +760,229 @@ # pass +# ==== Building query filters based on model data +class FilterParser( object ): + """ + Converts string tuples (partially converted query string params) of + attr, op, val into either: + - ORM based filters (filters that can be applied by the ORM at the SQL + level) or + - functional filters (filters that use derived values or values not + within the SQL tables) + These filters can then be applied to queries. + + This abstraction allows 'smarter' application of limit and offset at either the + SQL level or the generator/list level based on the presence of functional + filters. In other words, if no functional filters are present, limit and offset + may be applied at the SQL level. If functional filters are present, limit and + offset need to applied at the list level. + + These might be safely be replaced in the future by creating SQLAlchemy + hybrid properties or more thoroughly mapping derived values. + """ + #??: 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 + + #: model class + model_class = None + + def __init__( self, app ): + """ + Set up serializer map, any additional serializable keys, and views here. + """ + self.app = app + + #: 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 + #: over potentially expensive queries + self.orm_filter_parsers = {} + + #: dictionary containing parsing data for functional filters - applied after a query is made + self.fn_filter_parsers = {} + + # set up both of the above + self._add_parsers() + + def _add_parsers( self ): + """ + Set up, extend, or alter `orm_filter_parsers` and `fn_filter_parsers`. + """ + pass + + def parse_filters( self, filter_tuple_list ): + """ + Parse string 3-tuples (attr, op, val) into orm or functional filters. + """ + parsed = [] + for ( attr, op, val ) in filter_tuple_list: + filter_ = self.parse_filter( attr, op, val ) + parsed.append( filter_ ) + return parsed + + def parse_filter( self, attr, op, val ): + """ + Attempt to parse filter as a custom/fn filter, then an orm filter, and + if neither work - raise an error. + + :raises exceptions.RequestParameterInvalidException: if no functional or orm + filter can be parsed. + """ + try: + # check for a custom filter + fn_filter = self._parse_fn_filter( attr, op, val ) + if fn_filter is not None: + return fn_filter + + # if no custom filter found, try to make an ORM filter + #note: have to use explicit is None here, bool( sqlalx.filter ) == False + orm_filter = self._parse_orm_filter( attr, op, val ) + if orm_filter is not None: + return orm_filter + + # by convention, assume most val parsers raise ValueError + except ValueError, val_err: + raise exceptions.RequestParameterInvalidException( 'unparsable value for filter', + column=attr, operation=op, value=val ) + + # if neither of the above work, raise an error with how-to info + #TODO: send back all valid filter keys in exception for added user help + raise exceptions.RequestParameterInvalidException( 'bad filter', column=attr, operation=op ) + + # ---- fn filters + def _parse_fn_filter( self, attr, op, val ): + """ + """ + # fn_filter_list is a dict: fn_filter_list[ attr ] = { 'opname1' : opfn1, 'opname2' : opfn2, etc. } + + # attr, op is a nested dictionary pointing to the filter fn + attr_map = self.fn_filter_parsers.get( attr, None ) + if not attr_map: + return None + allowed_ops = attr_map.get( 'op' ) + # allowed ops is a map here, op => fn + filter_fn = allowed_ops.get( op, None ) + if not filter_fn: + return None + # parse the val from string using the 'val' parser if present (otherwise, leave as string) + val_parser = attr_map.get( 'val', None ) + if val_parser: + val = val_parser( val ) + + # curry/partial and fold the val in there now + return lambda i: filter_fn( i, val ) + + # ---- ORM filters + def _parse_orm_filter( self, attr, op, val ): + """ + """ + # orm_filter_list is a dict: orm_filter_list[ attr ] = <list of allowed ops> + # attr must be a whitelisted column + column = self.model_class.table.columns.get( attr ) + column_map = self.orm_filter_parsers.get( attr, None ) + if column is None or not column_map: + return None + # op must be whitelisted: contained in the list orm_filter_list[ attr ][ 'op' ] + allowed_ops = column_map.get( 'op' ) + if op not in allowed_ops: + return None + op = self._convert_op_string_to_fn( column, op ) + # parse the val from string using the 'val' parser if present (otherwise, leave as string) + val_parser = column_map.get( 'val', None ) + if val_parser: + val = val_parser( val ) + + orm_filter = op( val ) + return orm_filter + + #: these are the easier/shorter string equivalents to the python operator fn names that need '__' around them + UNDERSCORED_OPS = ( 'lt', 'le', 'eq', 'ne', 'ge', 'gt' ) + #UNCHANGED_OPS = ( 'like' ) + def _convert_op_string_to_fn( self, column, op_string ): + """ + """ + # correct op_string to usable function key + fn_name = op_string + if op_string in self.UNDERSCORED_OPS: + fn_name = '__' + op_string + '__' + elif op_string == 'in': + fn_name = 'in_' + + # get the column fn using the op_string and error if not a callable attr + #TODO: special case 'not in' - or disallow? + op_fn = getattr( column, fn_name, None ) + if not op_fn or not callable( op_fn ): + return None + return op_fn + + # --- more parsers! yay! +#TODO: These should go somewhere central - we've got ~6 parser modules/sections now + #TODO: to annotatable + def _owner_annotation( self, item ): + """ + 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 + + def filter_annotation_contains( self, item, val ): + """ + Test whether `val` is in the owner's annotation. + """ + owner_annotation = self._owner_annotation( item ) + if owner_annotation is None: + return False + return val in owner_annotation + + #TODO: to taggable + def _filter_tags( self, item, val, fn_name='__eq__' ): + """ + Test whether the string version of any tag `fn_name`s (__eq__, contains) + `val`. + """ + #TODO: which user is this? all? + for tag in item.tags: + tag_str = tag.user_tname + if tag.value is not None: + tag_str += ":" + tag.user_value + if tag_str[ fn_name ]( val ): + return True + return False + + def filter_has_partial_tag( self, item, val ): + """ + Return True if any tag partially contains `val`. + """ + return self._filter_tags( item, val, fn_name='contains' ) + + def filter_has_tag( self, item, val ): + """ + Return True if any tag exactly equals `val`. + """ + return self._filter_tags( item, val, fn_name='__eq__' ) + + def parse_bool( self, bool_string ): + """ + Parse a boolean from a string. + """ + #Be strict here to remove complexity of options. + if bool_string in ( 'True', True ): + return True + if bool_string in ( 'False', False ): + return False + raise ValueError( 'invalid boolean: ' + str( bool_string ) ) + + def parse_id_list( self, id_list_string, sep=',' ): + """ + Split `id_list_string` at `sep`. + """ + return id_list_string.split( sep ) + + # ==== Security Mixins class AccessibleModelInterface( object ): """ @@ -794,8 +1086,9 @@ :raises exceptions.ItemAccessibilityException: """ + raise exceptions.NotImplemented( "Abstract Interface Method" ) # just alias to by_user (easier/same thing) - return self.by_user( trans, user, **kwargs ) + #return self.by_user( trans, user, **kwargs ) def filter_owned( self, trans, user, **kwargs ): """ diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 lib/galaxy/managers/histories.py --- a/lib/galaxy/managers/histories.py +++ b/lib/galaxy/managers/histories.py @@ -63,7 +63,7 @@ return super( HistoryManager, self ).is_owner( trans, history, user ) #TODO: possibly to sharable - def most_recent( self, trans, user, **kwargs ): + def most_recent( self, trans, user, filters=None, **kwargs ): """ Return the most recently update history for the user. """ @@ -71,7 +71,8 @@ if not user: return None if trans.history.deleted else trans.history desc_update_time = self.model_class.table.c.update_time - return self._query_by_user( trans, user, order_by=desc_update_time, limit=1, **kwargs ).first() + filters = self._munge_filters( filters, self.model_class.user_id == user.id ) + return self.query( trans, filters=filters, order_by=desc_update_time, limit=1, **kwargs ).first() # .... purgable def purge( self, trans, history, flush=True, **kwargs ): @@ -109,7 +110,7 @@ return self.set_current( trans, self.by_id( trans, history_id ) ) # .... serialization - #TODO: move to serializer (i.e. history with contents attr) +#TODO: move to serializer (i.e. history with contents attr) def _get_history_data( self, trans, history ): """ Returns a dictionary containing ``history`` and ``contents``, serialized @@ -342,7 +343,6 @@ super( HistoryDeserializer, self ).__init__( app ) self.history_manager = self.manager - #assumes: incoming from json.loads and sanitized def add_deserializers( self ): super( HistoryDeserializer, self ).add_deserializers() base.PurgableModelDeserializer.add_deserializers( self ) @@ -351,3 +351,45 @@ 'name' : self.deserialize_basestring, 'genome_build' : self.deserialize_genome_build, }) + + +class HistoryFilters( base.FilterParser ): + model_class = model.History + + def _add_parsers( self ): + super( HistoryFilters, self )._add_parsers() + self.orm_filter_parsers.update({ + #TODO: these three are (prob.) applicable to all models + 'id' : { 'op': ( 'in' ), 'val': self.parse_id_list }, + # dates can be directly passed through the orm into a filter (no need to parse into datetime object) + 'create_time' : { 'op': ( 'le', 'ge' ) }, + 'update_time' : { 'op': ( 'le', 'ge' ) }, + + # history specific + 'name' : { 'op': ( 'eq', 'contains', 'like' ) }, + 'genome_build' : { 'op': ( 'eq', 'contains', 'like' ) }, + + #TODO: purgable + 'deleted' : { 'op': ( 'eq' ), 'val': self.parse_bool }, + 'purged' : { 'op': ( 'eq' ), 'val': self.parse_bool }, + + #TODO: sharable + 'importable' : { 'op': ( 'eq' ), 'val': self.parse_bool }, + 'published' : { 'op': ( 'eq' ), 'val': self.parse_bool }, + 'slug' : { 'op': ( 'eq', 'contains', 'like' ) }, + # chose by user should prob. only be available for admin? (most often we'll only need trans.user) + #'user' : { 'op': ( 'eq' ), 'val': self.parse_id_list }, + }) + + #TODO: I'm not entirely convinced this (or tags) are a good idea for filters since they involve a/the user + self.fn_filter_parsers.update({ + #TODO: add this in annotatable mixin + 'annotation' : { 'op': { 'in' : self.filter_annotation_contains, } }, + #TODO: add this in taggable mixin + 'tag' : { + 'op': { + 'eq' : self.filter_has_tag, + 'in' : self.filter_has_partial_tag, + } + } + }) diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 lib/galaxy/managers/ratable.py --- a/lib/galaxy/managers/ratable.py +++ b/lib/galaxy/managers/ratable.py @@ -16,6 +16,9 @@ #TODO: most of this seems to be covered by item_attrs.UsesItemRatings + #def by_user( self, trans, user, **kwargs ): + # pass + class RatableSerializer( object ): @@ -33,6 +36,7 @@ """ pass + class RatableDeserializer( object ): def add_deserializers( self ): diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 lib/galaxy/managers/sharable.py --- a/lib/galaxy/managers/sharable.py +++ b/lib/galaxy/managers/sharable.py @@ -41,22 +41,14 @@ self.user_manager = users.UserManager( app ) # .... has a user - def _query_by_user( self, trans, user, filters=None, **kwargs ): + def by_user( self, trans, user, filters=None, **kwargs ): """ - Return query for all items (of model_class type) associated with the given + Return list for all items (of model_class type) associated with the given `user`. """ user_filter = self.model_class.user_id == user.id filters=self._munge_filters( user_filter, filters ) - return self.query( trans, filters=filters, **kwargs ) - - def by_user( self, trans, user, **kwargs ): - """ - Return list for all items (of model_class type) associated with the given - `user`. - """ - query = self._query_by_user( trans, user, **kwargs ) - return self.list( trans, query=query, **kwargs ) + return self.list( trans, filters=filters, **kwargs ) # .... owned model interface def is_owner( self, trans, item, user ): diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 lib/galaxy/managers/taggable.py --- a/lib/galaxy/managers/taggable.py +++ b/lib/galaxy/managers/taggable.py @@ -15,6 +15,9 @@ #TODO: most of this can be done by delegating to the TagManager? + #def by_user( self, trans, user, **kwargs ): + # pass + class TaggableSerializer( object ): diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 lib/galaxy/managers/users.py --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -145,37 +145,9 @@ return user # ---- current - def is_current_user( self, trans, user ): - """ - Return True if this user is the trans' current user. - """ + def current_user( self, trans ): # define here for single point of change and make more readable - return user == trans.user - - def is_current_user_anonymous( self, trans ): - """ - Return True if the current user is anonymous. - """ - return self.is_anonymous( trans.user ) - - def is_current_user_admin( self, trans ): - """ - Return True if the current user is admin. - """ - return self.is_admin( trans, trans.user ) - - # ---- User-created notes/metadata on other models - #def tags( self, trans, user, **kwargs ): - # """ - # Return all tags created by this user. - # """ - # pass - - #def annotations( self, trans, user, **kwargs ): - # """ - # Return all annotations created by this user. - # """ - # pass + return trans.user # ---- api keys def create_api_key( self, trans, user ): diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 lib/galaxy/web/base/controller.py --- a/lib/galaxy/web/base/controller.py +++ b/lib/galaxy/web/base/controller.py @@ -100,6 +100,7 @@ def get_role( self, trans, id, check_ownership=False, check_accessible=False, deleted=None ): return self.get_object( trans, id, 'Role', check_ownership=False, check_accessible=False, deleted=deleted ) + # ---- parsing query params def decode_id( self, id ): try: return self.app.security.decode_id( id ) @@ -115,6 +116,53 @@ """ return trans.security.encode_all_ids( rval, recursive=recursive ) + def parse_filter_params( self, qdict, filter_attr_key='q', filter_value_key='qv', attr_op_split_char='-' ): + """ + """ + #TODO: import DEFAULT_OP from FilterParser + DEFAULT_OP = 'eq' + if filter_attr_key not in qdict: + return [] + #precondition: attrs/value pairs are in-order in the qstring + attrs = qdict.get( filter_attr_key ) + if not isinstance( attrs, list ): + attrs = [ attrs ] + # ops are strings placed after the attr strings and separated by a split char (e.g. 'create_time-lt') + # ops are optional and default to 'eq' + reparsed_attrs = [] + ops = [] + for attr in attrs: + op = DEFAULT_OP + if attr_op_split_char in attr: + #note: only split the last (e.g. q=community-tags-in&qv=rna yields ( 'community-tags', 'in', 'rna' ) + attr, op = attr.rsplit( attr_op_split_char, 1 ) + ops.append( op ) + reparsed_attrs.append( attr ) + attrs = reparsed_attrs + + values = qdict.get( filter_value_key, [] ) + if not isinstance( values, list ): + values = [ values ] + #TODO: it may be more helpful to the consumer if we error on incomplete 3-tuples + # (instead of relying on zip to shorten) + return zip( attrs, ops, values ) + + def parse_limit_offset( self, qdict ): + """ + """ + def _parse_pos_int( i ): + try: + new_val = int( i ) + if new_val >= 0: + return new_val + except ( TypeError, ValueError ): + pass + return None + + limit = _parse_pos_int( qdict.get( 'limit', None ) ) + offset = _parse_pos_int( qdict.get( 'offset', None ) ) + return ( limit, offset ) + Root = BaseController diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 lib/galaxy/webapps/galaxy/api/histories.py --- a/lib/galaxy/webapps/galaxy/api/histories.py +++ b/lib/galaxy/webapps/galaxy/api/histories.py @@ -19,7 +19,7 @@ from galaxy.web.base.controller import ExportsHistoryMixin from galaxy.web.base.controller import ImportsHistoryMixin -from galaxy.managers import histories, citations +from galaxy.managers import histories, citations, users from galaxy import util from galaxy.util import string_as_bool @@ -35,9 +35,11 @@ def __init__( self, app ): super( HistoriesController, self ).__init__( app ) self.citations_manager = citations.CitationsManager( app ) + self.user_manager = users.UserManager( app ) self.history_manager = histories.HistoryManager( app ) self.history_serializer = histories.HistorySerializer( app ) self.history_deserializer = histories.HistoryDeserializer( app ) + self.history_filters = histories.HistoryFilters( app ) @expose_api_anonymous def index( self, trans, deleted='False', **kwd ): @@ -55,21 +57,58 @@ :rtype: list :returns: list of dictionaries containing summary history information """ + serialization_params = self._parse_serialization_params( kwd, 'summary' ) + limit, offset = self.parse_limit_offset( kwd ) + filter_params = self.parse_filter_params( kwd ) + + # bail early with current history if user is anonymous + current_user = self.user_manager.current_user( trans ) + if self.user_manager.is_anonymous( current_user ): + current_history = self.history_manager.get_current( trans ) + #note: ignores filters, limit, offset + return [ self.history_serializer.serialize_to_view( trans, current_history, **serialization_params ) ] + + filters = [] + # support the old default of not-returning/filtering-out deleted histories + filters += self._get_deleted_filter( deleted, filter_params ) + # users are limited to requesting only their own histories (here) + filters += [ self.app.model.History.user == current_user ] + # and any sent in from the query string + filters += self.history_filters.parse_filters( filter_params ) + + #TODO: eventually make order_by a param as well + order_by = sqlalchemy.desc( self.app.model.History.create_time ) + histories = self.history_manager.list( trans, filters=filters, order_by=order_by, limit=limit, offset=offset ) + rval = [] - serialization_params = self._parse_serialization_params( kwd, 'summary' ) - - deleted_filter = ( self.app.model.History.deleted == False ) - if string_as_bool( deleted ): - deleted_filter = ( self.app.model.History.deleted == True ) - - #TODO: create time? is this right? - order_by = sqlalchemy.desc( self.app.model.History.create_time ) - histories = self.history_manager.by_user( trans, user=trans.user, filters=deleted_filter, order_by=order_by ) for history in histories: history_dict = self.history_serializer.serialize_to_view( trans, history, **serialization_params ) rval.append( history_dict ) + return rval - return rval + def _get_deleted_filter( self, deleted, filter_params ): + #TODO: this should all be removed (along with the default) in v2 + # support the old default of not-returning/filtering-out deleted histories + try: + # the consumer must explicitly ask for both deleted and non-deleted + # but pull it from the parsed params (as the filter system will error on None) + deleted_filter_index = filter_params.index( ( 'deleted', 'eq', 'None' ) ) + filter_params.pop( deleted_filter_index ) + return [] + except ValueError: + pass + + # the deleted string bool was also used as an 'include deleted' flag + if deleted in ( 'True', 'true' ): + return [ self.app.model.History.deleted == True ] + + # the third option not handled here is 'return only deleted' + # if this is passed in (in the form below), simply return and let the filter system handle it + if ( 'deleted', 'eq', 'True' ) in filter_params: + return [] + + # otherwise, do the default filter of removing the deleted histories + return [ self.app.model.History.deleted == False ] @expose_api_anonymous def show( self, trans, id, deleted='False', **kwd ): diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 test/unit/managers/mock.py --- a/test/unit/managers/mock.py +++ b/test/unit/managers/mock.py @@ -18,6 +18,9 @@ # ============================================================================= +class OpenObject( object ): + pass + class MockAppConfig( Bunch ): def __init__( self, **kwargs ): Bunch.__init__( self, **kwargs ) diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 test/unit/managers/test_HistoryManager.py --- a/test/unit/managers/test_HistoryManager.py +++ b/test/unit/managers/test_HistoryManager.py @@ -20,6 +20,7 @@ import mock from test_ModelManager import BaseTestCase from galaxy.managers.histories import HistoryManager +from galaxy.managers.histories import HistoryFilters # ============================================================================= @@ -92,9 +93,6 @@ user_histories = self.history_mgr.by_user( self.trans, owner ) self.assertEqual( user_histories, [ item1, item2 ] ) - query = self.history_mgr._query_by_user( self.trans, owner ) - self.assertEqual( query.all(), user_histories ) - def test_ownable( self ): owner = self.user_mgr.create( self.trans, **user2_data ) non_owner = self.user_mgr.create( self.trans, **user3_data ) @@ -307,6 +305,239 @@ self.assertEqual( self.history_mgr.get_current( self.trans ), history1 ) + # ---- functional and orm filter splitting and resolution + def test_parse_filters( self ): + filter_parser = HistoryFilters( self.app ) + filters = filter_parser.parse_filters([ + ( 'name', 'eq', 'wot' ), + ( 'deleted', 'eq', 'True' ), + ( 'annotation', 'in', 'hrrmm' ) + ]) + self.log( 'both orm and fn filters should be parsed and returned' ) + self.assertEqual( len( filters ), 3 ) + + self.log( 'values should be parsed' ) + self.assertEqual( filters[1].right.value, True ) + + def test_parse_filters_invalid_filters( self ): + filter_parser = HistoryFilters( self.app ) + self.log( 'should error on non-column attr') + self.assertRaises( exceptions.RequestParameterInvalidException, filter_parser.parse_filters, [ + ( 'merp', 'eq', 'wot' ), + ]) + self.log( 'should error on non-whitelisted attr') + self.assertRaises( exceptions.RequestParameterInvalidException, filter_parser.parse_filters, [ + ( 'user_id', 'eq', 'wot' ), + ]) + self.log( 'should error on non-whitelisted op') + self.assertRaises( exceptions.RequestParameterInvalidException, filter_parser.parse_filters, [ + ( 'name', 'lt', 'wot' ), + ]) + self.log( 'should error on non-listed fn op') + self.assertRaises( exceptions.RequestParameterInvalidException, filter_parser.parse_filters, [ + ( 'annotation', 'like', 'wot' ), + ]) + self.log( 'should error on val parsing error') + self.assertRaises( exceptions.RequestParameterInvalidException, filter_parser.parse_filters, [ + ( 'deleted', 'eq', 'true' ), + ]) + + def test_orm_filter_parsing( self ): + filter_parser = HistoryFilters( self.app ) + user2 = self.user_mgr.create( self.trans, **user2_data ) + history1 = self.history_mgr.create( self.trans, name='history1', user=user2 ) + history2 = self.history_mgr.create( self.trans, name='history2', user=user2 ) + history3 = self.history_mgr.create( self.trans, name='history3', user=user2 ) + + filters = filter_parser.parse_filters([ + ( 'name', 'like', 'history%' ), + ]) + histories = self.history_mgr.list( self.trans, filters=filters ) + #for h in histories: + # print h.name + self.assertEqual( histories, [ history1, history2, history3 ]) + + filters = filter_parser.parse_filters([ ( 'name', 'like', '%2' ), ]) + self.assertEqual( self.history_mgr.list( self.trans, filters=filters ), [ history2 ]) + + filters = filter_parser.parse_filters([ ( 'name', 'eq', 'history2' ), ]) + self.assertEqual( self.history_mgr.list( self.trans, filters=filters ), [ history2 ]) + + self.history_mgr.update( self.trans, history1, dict( deleted=True ) ) + filters = filter_parser.parse_filters([ ( 'deleted', 'eq', 'True' ), ]) + self.assertEqual( self.history_mgr.list( self.trans, filters=filters ), [ history1 ]) + filters = filter_parser.parse_filters([ ( 'deleted', 'eq', 'False' ), ]) + self.assertEqual( self.history_mgr.list( self.trans, filters=filters ), [ history2, history3 ]) + self.assertEqual( self.history_mgr.list( self.trans ), [ history1, history2, history3 ]) + + self.history_mgr.update( self.trans, history3, dict( deleted=True ) ) + self.history_mgr.update( self.trans, history1, dict( importable=True ) ) + self.history_mgr.update( self.trans, history2, dict( importable=True ) ) + filters = filter_parser.parse_filters([ + ( 'deleted', 'eq', 'True' ), + ( 'importable', 'eq', 'True' ), + ]) + self.assertEqual( self.history_mgr.list( self.trans, filters=filters ), [ history1 ]) + self.assertEqual( self.history_mgr.list( self.trans ), [ history1, history2, history3 ]) + + def test_fn_filter_parsing( self ): + filter_parser = HistoryFilters( self.app ) + user2 = self.user_mgr.create( self.trans, **user2_data ) + history1 = self.history_mgr.create( self.trans, name='history1', user=user2 ) + history2 = self.history_mgr.create( self.trans, name='history2', user=user2 ) + history3 = self.history_mgr.create( self.trans, name='history3', user=user2 ) + + filters = filter_parser.parse_filters([ ( 'annotation', 'in', 'no play' ), ]) + anno_filter = filters[0] + + history3.add_item_annotation( self.trans.sa_session, user2, history3, "All work and no play" ) + self.trans.sa_session.flush() + + self.assertTrue( anno_filter( history3 ) ) + self.assertFalse( anno_filter( history2 ) ) + + self.assertEqual( self.history_mgr.list( self.trans, filters=filters ), [ history3 ]) + + self.log( 'should allow combinations of orm and fn filters' ) + self.history_mgr.update( self.trans, history3, dict( importable=True ) ) + self.history_mgr.update( self.trans, history2, dict( importable=True ) ) + history1.add_item_annotation( self.trans.sa_session, user2, history1, "All work and no play" ) + self.trans.sa_session.flush() + + shining_examples = self.history_mgr.list( self.trans, filters=filter_parser.parse_filters([ + ( 'importable', 'eq', 'True' ), + ( 'annotation', 'in', 'no play' ), + ])) + self.assertEqual( shining_examples, [ history3 ]) + + def test_fn_filter_currying( self ): + filter_parser = HistoryFilters( self.app ) + filter_parser.fn_filter_parsers = { + 'name_len' : { 'op': { 'lt' : lambda i, v: len( i.name ) < v }, 'val': int } + } + self.log( 'should be 2 filters now' ) + self.assertEqual( len( filter_parser.fn_filter_parsers ), 1 ) + filters = filter_parser.parse_filters([ + ( 'name_len', 'lt', '4' ) + ]) + self.log( 'should have parsed out a single filter' ) + self.assertEqual( len( filters ), 1 ) + + filter_ = filters[0] + fake = mock.OpenObject() + fake.name = '123' + self.log( '123 should return true through the filter' ) + self.assertTrue( filter_( fake ) ) + fake.name = '1234' + self.log( '1234 should return false through the filter' ) + self.assertFalse( filter_( fake ) ) + + def test_list( self ): + """ + Test limit and offset in conjunction with both orm and fn filtering. + """ + filter_parser = HistoryFilters( self.app ) + user2 = self.user_mgr.create( self.trans, **user2_data ) + history1 = self.history_mgr.create( self.trans, name='history1', user=user2 ) + history2 = self.history_mgr.create( self.trans, name='history2', user=user2 ) + history3 = self.history_mgr.create( self.trans, name='history3', user=user2 ) + history4 = self.history_mgr.create( self.trans, name='history4', user=user2 ) + + self.history_mgr.delete( self.trans, history1 ) + self.history_mgr.delete( self.trans, history2 ) + self.history_mgr.delete( self.trans, history3 ) + + test_annotation = "testing" + history2.add_item_annotation( self.trans.sa_session, user2, history2, test_annotation ) + self.trans.sa_session.flush() + history3.add_item_annotation( self.trans.sa_session, user2, history3, test_annotation ) + self.trans.sa_session.flush() + history3.add_item_annotation( self.trans.sa_session, user2, history4, test_annotation ) + self.trans.sa_session.flush() + + all_histories = [ history1, history2, history3, history4 ] + deleted_and_annotated = [ history2, history3 ] + + self.log( "no offset, no limit should work" ) + self.assertEqual( self.history_mgr.list( self.trans, offset=None, limit=None ), all_histories ) + self.assertEqual( self.history_mgr.list( self.trans ), all_histories ) + self.log( "no offset, limit should work" ) + self.assertEqual( self.history_mgr.list( self.trans, limit=2 ), [ history1, history2 ] ) + self.log( "offset, no limit should work" ) + self.assertEqual( self.history_mgr.list( self.trans, offset=1 ), [ history2, history3, history4 ] ) + self.log( "offset, limit should work" ) + self.assertEqual( self.history_mgr.list( self.trans, offset=1, limit=1 ), [ history2 ] ) + + self.log( "zero limit should return empty list" ) + self.assertEqual( self.history_mgr.list( self.trans, limit=0 ), [] ) + self.log( "past len offset should return empty list" ) + self.assertEqual( self.history_mgr.list( self.trans, offset=len( all_histories ) ), [] ) + self.log( "negative limit should return full list" ) + self.assertEqual( self.history_mgr.list( self.trans, limit=-1 ), all_histories ) + self.log( "negative offset should return full list" ) + self.assertEqual( self.history_mgr.list( self.trans, offset=-1 ), all_histories ) + + filters = [ model.History.deleted == True ] + self.log( "orm filtered, no offset, no limit should work" ) + found = self.history_mgr.list( self.trans, filters=filters ) + self.assertEqual( found, [ history1, history2, history3 ] ) + self.log( "orm filtered, no offset, limit should work" ) + found = self.history_mgr.list( self.trans, filters=filters, limit=2 ) + self.assertEqual( found, [ history1, history2 ] ) + self.log( "orm filtered, offset, no limit should work" ) + found = self.history_mgr.list( self.trans, filters=filters, offset=1 ) + self.assertEqual( found, [ history2, history3 ] ) + self.log( "orm filtered, offset, limit should work" ) + found = self.history_mgr.list( self.trans, filters=filters, offset=1, limit=1 ) + self.assertEqual( found, [ history2 ] ) + + filters = filter_parser.parse_filters([ ( 'annotation', 'in', test_annotation ) ]) + self.log( "fn filtered, no offset, no limit should work" ) + found = self.history_mgr.list( self.trans, filters=filters ) + self.assertEqual( found, [ history2, history3, history4 ] ) + self.log( "fn filtered, no offset, limit should work" ) + found = self.history_mgr.list( self.trans, filters=filters, limit=2 ) + self.assertEqual( found, [ history2, history3 ] ) + self.log( "fn filtered, offset, no limit should work" ) + found = self.history_mgr.list( self.trans, filters=filters, offset=1 ) + self.assertEqual( found, [ history3, history4 ] ) + self.log( "fn filtered, offset, limit should work" ) + found = self.history_mgr.list( self.trans, filters=filters, offset=1, limit=1 ) + self.assertEqual( found, [ history3 ] ) + + filters = filter_parser.parse_filters([ + ( 'deleted', 'eq', 'True' ), + ( 'annotation', 'in', test_annotation ) + ]) + self.log( "orm and fn filtered, no offset, no limit should work" ) + found = self.history_mgr.list( self.trans, filters=filters ) + self.assertEqual( found, [ history2, history3 ] ) + self.log( "orm and fn filtered, no offset, limit should work" ) + found = self.history_mgr.list( self.trans, filters=filters, limit=1 ) + self.assertEqual( found, [ history2 ] ) + self.log( "orm and fn filtered, offset, no limit should work" ) + found = self.history_mgr.list( self.trans, filters=filters, offset=1 ) + self.assertEqual( found, [ history3 ] ) + self.log( "orm and fn filtered, offset, limit should work" ) + found = self.history_mgr.list( self.trans, filters=filters, offset=1, limit=1 ) + self.assertEqual( found, [ history3 ] ) + + self.log( "orm and fn filtered, zero limit should return empty list" ) + found = self.history_mgr.list( self.trans, filters=filters, limit=0 ) + self.assertEqual( found, [] ) + self.log( "orm and fn filtered, past len offset should return empty list" ) + found = self.history_mgr.list( self.trans, filters=filters, offset=len( deleted_and_annotated ) ) + self.assertEqual( found, [] ) + self.log( "orm and fn filtered, negative limit should return full list" ) + found = self.history_mgr.list( self.trans, filters=filters, limit=-1 ) + self.assertEqual( found, deleted_and_annotated ) + self.log( "orm and fn filtered, negative offset should return full list" ) + found = self.history_mgr.list( self.trans, filters=filters, offset=-1 ) + self.assertEqual( found, deleted_and_annotated ) + + + + # ============================================================================= if __name__ == '__main__': # or more generally, nosetests test_resourcemanagers.py -s -v diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 test/unit/managers/test_ModelManager.py --- a/test/unit/managers/test_ModelManager.py +++ b/test/unit/managers/test_ModelManager.py @@ -22,6 +22,7 @@ import mock from galaxy.managers.users import UserManager +from galaxy.managers import base # ============================================================================= admin_email = 'admin@admin.admin' diff -r 8a234cd86e8a972a43f0e13617c9f09ae1216fee -r 68f0ed37e08942acb1bb84b63911b22d7d4b6f65 test/unit/managers/test_UserManager.py --- a/test/unit/managers/test_UserManager.py +++ b/test/unit/managers/test_UserManager.py @@ -110,8 +110,8 @@ user3 = self.user_mgr.create( self.trans, **user3_data ) self.log( "should be able to tell if a user is the current (trans) user" ) - self.assertTrue( self.user_mgr.is_current_user( self.trans, self.admin_user ) ) - self.assertFalse( self.user_mgr.is_current_user( self.trans, user2 ) ) + self.assertEqual( self.user_mgr.current_user( self.trans ), self.admin_user ) + self.assertNotEqual( self.user_mgr.current_user( self.trans ), user2 ) def test_api_keys( self ): user2 = self.user_mgr.create( self.trans, **user2_data ) Repository URL: https://bitbucket.org/galaxy/galaxy-central/ -- This is a commit notification from bitbucket.org. You are receiving this because you have the service enabled, addressing the recipient of this email.