From f28120274df05726754adcbfbcbb6c217a699cff Mon Sep 17 00:00:00 2001 From: Carl Eberhard Date: Mon, 2 Feb 2015 11:07:17 -0500 Subject: [PATCH] Managers: wire url_for in serializers under a class-level variable for overriding in tests, more test coverage (serializers) --- lib/galaxy/managers/base.py | 4 + lib/galaxy/managers/hdas.py | 7 +- lib/galaxy/managers/histories.py | 5 +- lib/galaxy/managers/sharable.py | 11 +- test/unit/managers/mock.py | 4 + test/unit/managers/test_HistoryManager.py | 163 ++++++++++++++++++---- 6 files changed, 155 insertions(+), 39 deletions(-) diff --git a/lib/galaxy/managers/base.py b/lib/galaxy/managers/base.py index b17e2c669a0..3258e236a57 100644 --- a/lib/galaxy/managers/base.py +++ b/lib/galaxy/managers/base.py @@ -31,6 +31,7 @@ import sqlalchemy from galaxy import exceptions from galaxy import model +from galaxy import web from galaxy.model import tool_shed_install import logging @@ -496,6 +497,8 @@ class ModelSerializer( object ): item_dict = MySerializer.serialize( trans, my_item, keys_to_serialize ) # if a key to serialize is not listed in the Serializer.serializable_keys or serializers, it will not be added """ + #: 'service' to use for getting urls - use class var to allow overriding when testing + url_for = web.url_for def __init__( self, app ): """ @@ -578,6 +581,7 @@ class ModelSerializer( object ): no `view` or `keys`: use the `default_view` if any `view` and `keys`: combine both into one list of keys """ +#TODO: default view + view makes no sense outside the API.index context - move default view there all_keys = [] keys = keys or [] # chose explicit over concise here diff --git a/lib/galaxy/managers/hdas.py b/lib/galaxy/managers/hdas.py index e9f90f471a5..0eaa05dc63e 100644 --- a/lib/galaxy/managers/hdas.py +++ b/lib/galaxy/managers/hdas.py @@ -11,7 +11,6 @@ import gettext from galaxy import model from galaxy import exceptions from galaxy import datatypes -import galaxy.web import galaxy.datatypes.metadata from galaxy import objectstore @@ -436,11 +435,11 @@ class HDASerializer( datasets.DatasetAssociationSerializer, #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 t, i, k: galaxy.web.url_for( 'history_content', + 'url' : lambda t, i, k: self.url_for( 'history_content', history_id=t.security.encode_id( i.history_id ), id=t.security.encode_id( i.id ) ), 'urls' : self.serialize_urls, - 'download_url' : lambda t, i, k: galaxy.web.url_for( 'history_contents_display', + 'download_url' : lambda t, i, k: self.url_for( 'history_contents_display', history_id=t.security.encode_id( i.history.id ), history_content_id=t.security.encode_id( i.id ) ), @@ -548,7 +547,7 @@ class HDASerializer( datasets.DatasetAssociationSerializer, """ Return web controller urls useful for this HDA. """ - url_for = galaxy.web.url_for + url_for = self.url_for encoded_id = self.security.encode_id( hda.id ) urls = { 'purge' : url_for( controller='dataset', action='purge_async', dataset_id=encoded_id ), diff --git a/lib/galaxy/managers/histories.py b/lib/galaxy/managers/histories.py index f74abccfd33..4e53e1010ab 100644 --- a/lib/galaxy/managers/histories.py +++ b/lib/galaxy/managers/histories.py @@ -5,7 +5,6 @@ Histories are containers for datasets or dataset collections created (or copied) by users over the course of an analysis. """ -import galaxy.web from galaxy import model from galaxy.managers import base from galaxy.managers import sharable @@ -307,9 +306,9 @@ class HistorySerializer( sharable.SharableModelSerializer, deletable.PurgableSer 'size' : lambda t, i, k: int( i.get_disk_size() ), 'nice_size' : lambda t, i, k: i.get_disk_size( nice_size=True ), - 'url' : lambda t, i, k: galaxy.web.url_for( 'history', id=t.security.encode_id( i.id ) ), + 'url' : lambda t, i, k: self.url_for( 'history', id=t.security.encode_id( i.id ) ), 'contents_url' : lambda t, i, k: - galaxy.web.url_for( 'history_contents', history_id=t.security.encode_id( i.id ) ), + self.url_for( 'history_contents', history_id=t.security.encode_id( i.id ) ), 'empty' : lambda t, i, k: len( i.datasets ) <= 0, 'hdas' : lambda t, i, k: [ t.security.encode_id( hda.id ) for hda in i.datasets ], diff --git a/lib/galaxy/managers/sharable.py b/lib/galaxy/managers/sharable.py index bb84a296c8a..eb56a6e033a 100644 --- a/lib/galaxy/managers/sharable.py +++ b/lib/galaxy/managers/sharable.py @@ -326,13 +326,9 @@ class SharableModelSerializer( base.ModelSerializer, def serialize_username_and_slug( self, trans, item, key ): if not ( item.user and item.slug and self.SINGLE_CHAR_ABBR ): return None +#TODO: self.url_for return ( '/' ).join(( 'u', item.user.username, self.SINGLE_CHAR_ABBR, item.slug ) ) - # the only ones that needs any fns: - # user/user_id - # user_shares? - # username_and_slug? - #def get_name_and_link_async( self, trans, id=None ): #def published_url( self, trans, item, key ): # """ # """ @@ -340,6 +336,11 @@ class SharableModelSerializer( base.ModelSerializer, # username=item.user.username, slug=item.slug ) # return url + # the only ones that needs any fns: + # user/user_id + # user_shares? + # username_and_slug? + class SharableModelDeserializer( base.ModelDeserializer, taggable.TaggableDeserializerMixin, annotatable.AnnotatableDeserializerMixin, ratable.RatableDeserializerMixin ): diff --git a/test/unit/managers/mock.py b/test/unit/managers/mock.py index 4412e5ac659..d630e857950 100644 --- a/test/unit/managers/mock.py +++ b/test/unit/managers/mock.py @@ -21,6 +21,7 @@ from galaxy.util.bunch import Bunch class OpenObject( object ): pass + class MockAppConfig( Bunch ): def __init__( self, **kwargs ): Bunch.__init__( self, **kwargs ) @@ -36,7 +37,10 @@ class MockAppConfig( Bunch ): self.user_activation_on = False self.new_user_dataset_access_role_default_private = False + self.expose_dataset_path = True self.allow_user_dataset_purge = True + self.enable_old_display_applications = True + class MockApp( object ): def __init__( self, **kwargs ): diff --git a/test/unit/managers/test_HistoryManager.py b/test/unit/managers/test_HistoryManager.py index ecb5e62a723..cf1d473781a 100644 --- a/test/unit/managers/test_HistoryManager.py +++ b/test/unit/managers/test_HistoryManager.py @@ -19,8 +19,12 @@ from galaxy.util.bunch import Bunch import mock from test_ModelManager import BaseTestCase + from galaxy.managers.histories import HistoryManager +from galaxy.managers.histories import HistorySerializer +from galaxy.managers.histories import HistoryDeserializer from galaxy.managers.histories import HistoryFilters +from galaxy.managers import hdas # ============================================================================= @@ -305,10 +309,122 @@ class HistoryManagerTestCase( BaseTestCase ): self.assertEqual( self.history_mgr.get_current( self.trans ), history1 ) +# ============================================================================= +# web.url_for doesn't work well in the framework +testable_url_for = lambda *a, **k: '(fake url): %s, %s' % ( a, k ) +HistorySerializer.url_for = testable_url_for +hdas.HDASerializer.url_for = testable_url_for + + +class HistorySerializerTestCase( BaseTestCase ): + + def assertHasKeys( self, obj, key_list ): + self.assertEqual( sorted( obj.keys() ), sorted( key_list ) ) + + def set_up_managers( self ): + super( HistorySerializerTestCase, self ).set_up_managers() + self.history_mgr = HistoryManager( self.app ) + self.hda_mgr = hdas.HDAManager( self.app ) + self.history_serializer = HistorySerializer( self.app ) + + def test_views( self ): + user2 = self.user_mgr.create( self.trans, **user2_data ) + history1 = self.history_mgr.create( self.trans, name='history1', user=user2 ) + + self.log( 'should have a summary view' ) + summary_view = self.history_serializer.serialize_to_view( self.trans, history1, view='summary' ) + self.assertHasKeys( summary_view, self.history_serializer.views[ 'summary' ] ) + + self.log( 'should have a detailed view' ) + detailed_view = self.history_serializer.serialize_to_view( self.trans, history1, view='detailed' ) + self.assertHasKeys( detailed_view, self.history_serializer.views[ 'detailed' ] ) + + self.log( 'should have a extended view' ) + extended_view = self.history_serializer.serialize_to_view( self.trans, history1, view='extended' ) + self.assertHasKeys( extended_view, self.history_serializer.views[ 'extended' ] ) + + self.log( 'should have the summary view as default view' ) + default_view = self.history_serializer.serialize_to_view( self.trans, history1, default_view='summary' ) + self.assertHasKeys( summary_view, self.history_serializer.views[ 'summary' ] ) + + self.log( 'should have a serializer for all serializable keys' ) + need_no_serializers = ( basestring, bool, type( None ) ) + for key in self.history_serializer.serializable_keys: + instantiated_attribute = getattr( history1, key, None ) + if not ( ( key in self.history_serializer.serializers ) + or ( isinstance( instantiated_attribute, need_no_serializers ) ) ): + self.fail( 'no serializer for: %s (%s)' % ( key, instantiated_attribute ) ) + else: + self.assertTrue( True, 'all serializable keys have a serializer' ) + + def test_views_and_keys( self ): + user2 = self.user_mgr.create( self.trans, **user2_data ) + history1 = self.history_mgr.create( self.trans, name='history1', user=user2 ) + + self.log( 'should be able to use keys with views' ) + serialized = self.history_serializer.serialize_to_view( self.trans, history1, + view='summary', keys=[ 'state_ids', 'user_id' ] ) + self.assertHasKeys( serialized, + self.history_serializer.views[ 'summary' ] + [ 'state_ids', 'user_id' ] ) + + self.log( 'should be able to use keys on their own' ) + serialized = self.history_serializer.serialize_to_view( self.trans, history1, + keys=[ 'state_ids', 'user_id' ] ) + self.assertHasKeys( serialized, [ 'state_ids', 'user_id' ] ) + + def test_serializers( self ): + # size + # nice size + pass + + def test_contents( self ): + user2 = self.user_mgr.create( self.trans, **user2_data ) + history1 = self.history_mgr.create( self.trans, name='history1', user=user2 ) + + self.log( 'a history with no contents should be properly reflected in empty, etc.' ) + keys = [ 'empty', 'count', 'state_ids', 'state_details', 'state' ] + serialized = self.history_serializer.serialize( self.trans, history1, keys ) + self.assertEqual( serialized[ 'state' ], 'new' ) + self.assertEqual( serialized[ 'empty' ], True ) + self.assertEqual( serialized[ 'count' ], 0 ) + self.assertEqual( sum( serialized[ 'state_details' ].values() ), 0 ) + self.assertEqual( serialized[ 'state_ids' ][ 'ok' ], [] ) + + hda1 = self.hda_mgr.create( self.trans, history=history1, hid=1 ) + self.hda_mgr.update( self.trans, hda1, dict( state='ok' ) ) + + serialized = self.history_serializer.serialize( self.trans, history1, keys ) + self.assertEqual( serialized[ 'state' ], 'ok' ) + self.assertEqual( serialized[ 'empty' ], False ) + self.assertEqual( serialized[ 'count' ], 1 ) + self.assertEqual( serialized[ 'state_details' ][ 'ok' ], 1 ) + self.assertIsInstance( serialized[ 'state_ids' ][ 'ok' ], list ) + + #pprint.pprint( self.history_serializer.serialize( self.trans, history1, [ 'contents' ] ) ) + +# ============================================================================= +class HistoryDeserializerTestCase( BaseTestCase ): + + def set_up_managers( self ): + super( HistoryDeserializerTestCase, self ).set_up_managers() + self.history_mgr = HistoryManager( self.app ) + self.history_deserializer = HistoryDeserializer( self.app ) + + def test_base( self ): + pass + + +# ============================================================================= +class HistoryFiltersTestCase( BaseTestCase ): + + def set_up_managers( self ): + super( HistoryFiltersTestCase, self ).set_up_managers() + self.history_mgr = HistoryManager( self.app ) + self.filter_parser = HistoryFilters( self.app ) + # ---- functional and orm filter splitting and resolution def test_parse_filters( self ): - filter_parser = HistoryFilters( self.app ) - filters = filter_parser.parse_filters([ + filters = self.filter_parser.parse_filters([ ( 'name', 'eq', 'wot' ), ( 'deleted', 'eq', 'True' ), ( 'annotation', 'has', 'hrrmm' ) @@ -320,36 +436,34 @@ class HistoryManagerTestCase( BaseTestCase ): 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, [ + self.assertRaises( exceptions.RequestParameterInvalidException, self.filter_parser.parse_filters, [ ( 'merp', 'eq', 'wot' ), ]) self.log( 'should error on non-whitelisted attr') - self.assertRaises( exceptions.RequestParameterInvalidException, filter_parser.parse_filters, [ + self.assertRaises( exceptions.RequestParameterInvalidException, self.filter_parser.parse_filters, [ ( 'user_id', 'eq', 'wot' ), ]) self.log( 'should error on non-whitelisted op') - self.assertRaises( exceptions.RequestParameterInvalidException, filter_parser.parse_filters, [ + self.assertRaises( exceptions.RequestParameterInvalidException, self.filter_parser.parse_filters, [ ( 'name', 'lt', 'wot' ), ]) self.log( 'should error on non-listed fn op') - self.assertRaises( exceptions.RequestParameterInvalidException, filter_parser.parse_filters, [ + self.assertRaises( exceptions.RequestParameterInvalidException, self.filter_parser.parse_filters, [ ( 'annotation', 'like', 'wot' ), ]) self.log( 'should error on val parsing error') - self.assertRaises( exceptions.RequestParameterInvalidException, filter_parser.parse_filters, [ + self.assertRaises( exceptions.RequestParameterInvalidException, self.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([ + filters = self.filter_parser.parse_filters([ ( 'name', 'like', 'history%' ), ]) histories = self.history_mgr.list( self.trans, filters=filters ) @@ -357,23 +471,23 @@ class HistoryManagerTestCase( BaseTestCase ): # print h.name self.assertEqual( histories, [ history1, history2, history3 ]) - filters = filter_parser.parse_filters([ ( 'name', 'like', '%2' ), ]) + filters = self.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' ), ]) + filters = self.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' ), ]) + filters = self.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' ), ]) + filters = self.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([ + filters = self.filter_parser.parse_filters([ ( 'deleted', 'eq', 'True' ), ( 'importable', 'eq', 'True' ), ]) @@ -381,13 +495,12 @@ class HistoryManagerTestCase( BaseTestCase ): 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', 'has', 'no play' ), ]) + filters = self.filter_parser.parse_filters([ ( 'annotation', 'has', 'no play' ), ]) anno_filter = filters[0] history3.add_item_annotation( self.trans.sa_session, user2, history3, "All work and no play" ) @@ -404,20 +517,19 @@ class HistoryManagerTestCase( BaseTestCase ): 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([ + shining_examples = self.history_mgr.list( self.trans, filters=self.filter_parser.parse_filters([ ( 'importable', 'eq', 'True' ), ( 'annotation', 'has', 'no play' ), ])) self.assertEqual( shining_examples, [ history3 ]) def test_fn_filter_currying( self ): - filter_parser = HistoryFilters( self.app ) - filter_parser.fn_filter_parsers = { + self.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([ + self.assertEqual( len( self.filter_parser.fn_filter_parsers ), 1 ) + filters = self.filter_parser.parse_filters([ ( 'name_len', 'lt', '4' ) ]) self.log( 'should have parsed out a single filter' ) @@ -436,7 +548,6 @@ class HistoryManagerTestCase( BaseTestCase ): """ 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 ) @@ -491,7 +602,7 @@ class HistoryManagerTestCase( BaseTestCase ): found = self.history_mgr.list( self.trans, filters=filters, offset=1, limit=1 ) self.assertEqual( found, [ history2 ] ) - filters = filter_parser.parse_filters([ ( 'annotation', 'has', test_annotation ) ]) + filters = self.filter_parser.parse_filters([ ( 'annotation', 'has', 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 ] ) @@ -505,7 +616,7 @@ class HistoryManagerTestCase( BaseTestCase ): found = self.history_mgr.list( self.trans, filters=filters, offset=1, limit=1 ) self.assertEqual( found, [ history3 ] ) - filters = filter_parser.parse_filters([ + filters = self.filter_parser.parse_filters([ ( 'deleted', 'eq', 'True' ), ( 'annotation', 'has', test_annotation ) ]) @@ -536,8 +647,6 @@ class HistoryManagerTestCase( BaseTestCase ): self.assertEqual( found, deleted_and_annotated ) - - # ============================================================================= if __name__ == '__main__': # or more generally, nosetests test_resourcemanagers.py -s -v