From d76d92d3a0c8a053f17b7bdb9f988c4ab9be322a Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Tue, 10 May 2016 13:19:05 -0400 Subject: [PATCH 1/5] API, sharable: serialize users_shared_with - Changes sharable serializers so that, when requested with `keys=users_shared_with`, they return a list of encoded user ids for each user that the item has been shared with - Adds tests in test_HistoryManager --- lib/galaxy/managers/sharable.py | 8 ++++++-- test/unit/managers/test_HistoryManager.py | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/managers/sharable.py b/lib/galaxy/managers/sharable.py index 5826202ebf0..130dfab1ee9 100644 --- a/lib/galaxy/managers/sharable.py +++ b/lib/galaxy/managers/sharable.py @@ -335,7 +335,8 @@ class SharableModelSerializer( base.ModelSerializer, self.serializers.update({ 'user_id' : self.serialize_id, - 'username_and_slug' : self.serialize_username_and_slug + 'username_and_slug' : self.serialize_username_and_slug, + 'users_shared_with' : self.serialize_users_shared_with }) # these use the default serializer but must still be white-listed self.serializable_keyset.update([ @@ -357,9 +358,12 @@ class SharableModelSerializer( base.ModelSerializer, # the only ones that needs any fns: # user/user_id - # user_shares? # username_and_slug? + def serialize_users_shared_with( self, item, key, **context ): + share_assocs = self.manager.get_share_assocs( item ) + return [ self.serialize_id( share, 'user_id' ) for share in share_assocs ] + class SharableModelDeserializer( base.ModelDeserializer, taggable.TaggableDeserializerMixin, annotatable.AnnotatableDeserializerMixin, ratable.RatableDeserializerMixin ): diff --git a/test/unit/managers/test_HistoryManager.py b/test/unit/managers/test_HistoryManager.py index b98ec9f6809..231acfc6549 100644 --- a/test/unit/managers/test_HistoryManager.py +++ b/test/unit/managers/test_HistoryManager.py @@ -437,6 +437,14 @@ class HistorySerializerTestCase( BaseTestCase ): serialized = self.history_serializer.serialize( history1, sharable_attrs ) self.assertKeys( serialized, sharable_attrs ) + self.log( 'should return user_id for user with whom it\'s been shared' ) + non_owner = self.user_manager.create( **user3_data ) + self.history_manager.share_with( history1, non_owner ) + serialized = self.history_serializer.serialize( history1, [ 'users_shared_with' ] ) + self.assertKeys( serialized, [ 'users_shared_with' ] ) + self.assertIsInstance( serialized[ 'users_shared_with' ], list ) + self.assertEqual( serialized[ 'users_shared_with' ][0], self.app.security.encode_id( non_owner.id ) ) + def test_purgable( self ): user2 = self.user_manager.create( **user2_data ) history1 = self.history_manager.create( name='history1', user=user2 ) From dc1924bcdf780949df2614f6c721b48780da9d16 Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Tue, 10 May 2016 13:23:02 -0400 Subject: [PATCH 2/5] Testing: fixes galaxy_mock set_datatypes_registry - Add the datatypes_registry to the mock app using an empty registry --- test/unit/unittest_utils/galaxy_mock.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/unit/unittest_utils/galaxy_mock.py b/test/unit/unittest_utils/galaxy_mock.py index 48dbedb7f01..13ea156d5bc 100644 --- a/test/unit/unittest_utils/galaxy_mock.py +++ b/test/unit/unittest_utils/galaxy_mock.py @@ -7,6 +7,8 @@ import shutil from galaxy.web import security from galaxy import objectstore +from galaxy import model +from galaxy.datatypes import registry from galaxy.model import mapping from galaxy.util.bunch import Bunch @@ -55,6 +57,7 @@ class MockApp( object ): self.name = kwargs.get( 'name', 'galaxy' ) self.object_store = objectstore.build_object_store_from_config( self.config ) self.model = mapping.init( "/tmp", "sqlite:///:memory:", create_tables=True, object_store=self.object_store ) + model.set_datatypes_registry( registry.Registry() ) self.security_agent = self.model.security_agent self.visualizations_registry = MockVisualizationsRegistry() self.tag_handler = tags.GalaxyTagManager( self ) From 7ffa8aac676cf4f2ea8051a0ddd4056135935752 Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Tue, 10 May 2016 16:47:08 -0400 Subject: [PATCH 3/5] API, sharable: deserialize users_shared_with - Adds a deserializer for users_shared_with. PUT a list of user ids to update who an item is shared with. - Invalid users are skipped, bad ids error - (Only affects histories ATM) - Tests added to test_HistoryManager --- lib/galaxy/managers/sharable.py | 59 +++++++++++------------ test/unit/managers/test_HistoryManager.py | 36 ++++++++++++++ 2 files changed, 65 insertions(+), 30 deletions(-) diff --git a/lib/galaxy/managers/sharable.py b/lib/galaxy/managers/sharable.py index 130dfab1ee9..2208a7f10cc 100644 --- a/lib/galaxy/managers/sharable.py +++ b/lib/galaxy/managers/sharable.py @@ -147,6 +147,7 @@ class SharableModelManager( base.ModelManager, secured.OwnableManagerMixin, secu """ Get or create a share for the given user (or users if `user` is a list). """ + # precondition: user has been validated # allow user to be a list and call recursivly if isinstance( user, list ): return map( lambda user: self.share_with( item, user, flush=False ), user ) @@ -307,19 +308,7 @@ class SharableModelManager( base.ModelManager, secured.OwnableManagerMixin, secu self.session().flush() return item - # def by_slug( self, user, **kwargs ): - # """ - # """ - # pass - - # .... display - # def display_by_username_and_slug( self, username, slug ): - # """ Display item by username and slug. """ - - # def set_public_username( self, id, username, **kwargs ): - # """ Set user's public username and delegate to sharing() """ - # def sharing( self, id, **kwargs ): - # """ Handle item sharing. """ + # TODO: def by_slug( self, user, **kwargs ): class SharableModelSerializer( base.ModelSerializer, @@ -348,14 +337,6 @@ class SharableModelSerializer( base.ModelSerializer, return None return ( '/' ).join(( 'u', item.user.username, self.SINGLE_CHAR_ABBR, item.slug ) ) - # def serialize_username_and_slug( self, item, key, **context ): - # """ - # """ - # #TODO: replace the above with this - # url = self.url_for(controller='history', action="display_by_username_and_slug", - # username=item.user.username, slug=item.slug ) - # return url - # the only ones that needs any fns: # user/user_id # username_and_slug? @@ -375,8 +356,9 @@ class SharableModelDeserializer( base.ModelDeserializer, ratable.RatableDeserializerMixin.add_deserializers( self ) self.deserializers.update({ - 'published' : self.deserialize_published, - 'importable' : self.deserialize_importable, + 'published' : self.deserialize_published, + 'importable' : self.deserialize_importable, + 'users_shared_with' : self.deserialize_users_shared_with, }) def deserialize_published( self, item, key, val, **context ): @@ -403,15 +385,32 @@ class SharableModelDeserializer( base.ModelDeserializer, self.manager.make_importable( item, flush=False ) else: self.manager.make_non_importable( item, flush=False ) - return item.published + return item.importable - # def deserialize_slug( self, item, val, **context ): - # """ - # """ - # #TODO: call manager.set_slug - # pass + # TODO: def deserialize_slug( self, item, val, **context ): - # def deserialize_user_shares(): + def deserialize_users_shared_with( self, item, key, val, **context ): + """ + Accept a list of encoded user_ids, validate them as users, and then + add or remove user shares in order to update the users_shared_with to + match the given list finally returning the new list of shares. + """ + unencoded_ids = [ self.app.security.decode_id( id_ ) for id_ in val ] + new_users_shared_with = set( self.manager.user_manager.by_ids( unencoded_ids ) ) + current_shares = self.manager.get_share_assocs( item ) + currently_shared_with = set([ share.user for share in current_shares ]) + + needs_adding = new_users_shared_with - currently_shared_with + for user in needs_adding: + current_shares.append( self.manager.share_with( item, user, flush=False ) ) + + needs_removing = currently_shared_with - new_users_shared_with + for user in needs_removing: + current_shares.remove( self.manager.unshare_with( item, user, flush=False ) ) + + self.manager.session().flush() + # TODO: or should this return the list of ids? + return current_shares class SharableModelFilters( base.ModelFilterParser, diff --git a/test/unit/managers/test_HistoryManager.py b/test/unit/managers/test_HistoryManager.py index 231acfc6549..f8de101774b 100644 --- a/test/unit/managers/test_HistoryManager.py +++ b/test/unit/managers/test_HistoryManager.py @@ -603,6 +603,42 @@ class HistoryDeserializerTestCase( BaseTestCase ): deserializer.deserialize( item, { 'community_rating' : 4 }, user=user2 ) self.assertEqual( manager.ratings_count( item ), 1 ) + def test_sharable( self ): + manager = self.history_manager + deserializer = self.history_deserializer + + user2 = self.user_manager.create( **user2_data ) + item = manager.create( name='history1', user=user2 ) + non_owner = self.user_manager.create( **user3_data ) + + self.log( 'should allow adding a share by adding a user id to users_shared_with' ) + non_owner_id = self.app.security.encode_id( non_owner.id ) + deserializer.deserialize( item, { 'users_shared_with' : [ non_owner_id ] }, user=user2 ) + user_shares = manager.get_share_assocs( item ) + self.assertEqual( len( user_shares ), 1 ) + self.assertEqual( user_shares[0].user_id, non_owner.id ) + + self.log( 're-adding an existing user id should do nothing' ) + deserializer.deserialize( item, { 'users_shared_with' : [ non_owner_id, non_owner_id ] }, user=user2 ) + user_shares = manager.get_share_assocs( item ) + self.assertEqual( len( user_shares ), 1 ) + self.assertEqual( user_shares[0].user_id, non_owner.id ) + + self.log( 'should allow removing a share by not having it in users_shared_with' ) + deserializer.deserialize( item, { 'users_shared_with' : [] }, user=user2 ) + user_shares = manager.get_share_assocs( item ) + self.assertEqual( len( user_shares ), 0 ) + + self.log( 'adding a bad user id should error' ) + self.assertRaises( AttributeError, + deserializer.deserialize, item, { 'users_shared_with' : [ None ] }, user=user2 ) + + self.log( 'adding a non-existing user id should do nothing' ) + non_user_id = self.app.security.encode_id( 99 ) + deserializer.deserialize( item, { 'users_shared_with' : [ non_user_id ] }, user=user2 ) + user_shares = manager.get_share_assocs( item ) + self.assertEqual( len( user_shares ), 0 ) + # ============================================================================= class HistoryFiltersTestCase( BaseTestCase ): From 4baa3377ed7809278d224ee1af90577e1dbd7627 Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Wed, 11 May 2016 12:36:15 -0400 Subject: [PATCH 4/5] Tests: load_datatypes in galaxy_mock --- test/unit/unittest_utils/galaxy_mock.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/unit/unittest_utils/galaxy_mock.py b/test/unit/unittest_utils/galaxy_mock.py index 13ea156d5bc..bc64ab976d1 100644 --- a/test/unit/unittest_utils/galaxy_mock.py +++ b/test/unit/unittest_utils/galaxy_mock.py @@ -57,11 +57,16 @@ class MockApp( object ): self.name = kwargs.get( 'name', 'galaxy' ) self.object_store = objectstore.build_object_store_from_config( self.config ) self.model = mapping.init( "/tmp", "sqlite:///:memory:", create_tables=True, object_store=self.object_store ) - model.set_datatypes_registry( registry.Registry() ) self.security_agent = self.model.security_agent self.visualizations_registry = MockVisualizationsRegistry() self.tag_handler = tags.GalaxyTagManager( self ) self.quota_agent = quota.QuotaAgent( self.model ) + self.init_datatypes() + + def init_datatypes( self ): + datatypes_registry = registry.Registry() + datatypes_registry.load_datatypes() + model.set_datatypes_registry( datatypes_registry ) class MockAppConfig( Bunch ): From e2f34f7a25d3c6ee081dfa7126b15f2585803df9 Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Thu, 12 May 2016 10:43:59 -0400 Subject: [PATCH 5/5] API, history: skip users_shared_with for non-owner - If the requesting user is not the owner, don't return users_shared_with even when requested - add tests --- lib/galaxy/managers/sharable.py | 12 +++++++++++- test/unit/managers/test_HistoryManager.py | 8 ++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/managers/sharable.py b/lib/galaxy/managers/sharable.py index 2208a7f10cc..a091881fcaa 100644 --- a/lib/galaxy/managers/sharable.py +++ b/lib/galaxy/managers/sharable.py @@ -341,7 +341,17 @@ class SharableModelSerializer( base.ModelSerializer, # user/user_id # username_and_slug? - def serialize_users_shared_with( self, item, key, **context ): + def serialize_users_shared_with( self, item, key, user=None, **context ): + """ + Returns a list of encoded ids for users the item has been shared. + + Skipped if the requesting user is not the owner. + """ + # TODO: still an open question as to whether key removal based on user + # should be handled here or at a higher level (even if we didn't have to pass user (via thread context, etc.)) + if not self.manager.is_owner( item, user ): + self.skip() + share_assocs = self.manager.get_share_assocs( item ) return [ self.serialize_id( share, 'user_id' ) for share in share_assocs ] diff --git a/test/unit/managers/test_HistoryManager.py b/test/unit/managers/test_HistoryManager.py index f8de101774b..8c0b0046ff6 100644 --- a/test/unit/managers/test_HistoryManager.py +++ b/test/unit/managers/test_HistoryManager.py @@ -437,14 +437,18 @@ class HistorySerializerTestCase( BaseTestCase ): serialized = self.history_serializer.serialize( history1, sharable_attrs ) self.assertKeys( serialized, sharable_attrs ) - self.log( 'should return user_id for user with whom it\'s been shared' ) + self.log( 'should return user_id for user with whom it\'s been shared if the requester is the owner' ) non_owner = self.user_manager.create( **user3_data ) self.history_manager.share_with( history1, non_owner ) - serialized = self.history_serializer.serialize( history1, [ 'users_shared_with' ] ) + serialized = self.history_serializer.serialize( history1, [ 'users_shared_with' ], user=user2 ) self.assertKeys( serialized, [ 'users_shared_with' ] ) self.assertIsInstance( serialized[ 'users_shared_with' ], list ) self.assertEqual( serialized[ 'users_shared_with' ][0], self.app.security.encode_id( non_owner.id ) ) + self.log( 'should not return users_shared_with if the requester is not the owner' ) + serialized = self.history_serializer.serialize( history1, [ 'users_shared_with' ], user=non_owner ) + self.assertFalse( hasattr( serialized, 'users_shared_with' ) ) + def test_purgable( self ): user2 = self.user_manager.create( **user2_data ) history1 = self.history_manager.create( name='history1', user=user2 )