Merge pull request #1618 from carlfeberhard/managers.ratable

Implement the ratable mixin
This commit is contained in:
Martin Cech
2016-02-08 13:56:20 -05:00
11 changed files with 258 additions and 62 deletions
+46 -20
View File
@@ -24,7 +24,6 @@ attribute change to a model object.
# such as: a single flat class, serializers being singletons in the manager, etc.
# instead of the three separate classes. With no 'apparent' perfect scheme
# I'm opting to just keep them separate.
import sqlalchemy
import routes
@@ -470,7 +469,6 @@ class ModelManager( object ):
self.session().flush()
return item
# TODO: yagni?
def associate( self, associate_with, item, foreign_key_name=None ):
"""
Generically associate `item` with `associate_with` based on `foreign_key_name`.
@@ -479,12 +477,15 @@ class ModelManager( object ):
setattr( associate_with, foreign_key_name, item )
return item
def _foreign_key( self, associated_model_class, foreign_key_name=None ):
foreign_key_name = foreign_key_name or self.foreign_key_name
return getattr( associated_model_class, foreign_key_name )
def query_associated( self, associated_model_class, item, foreign_key_name=None ):
"""
Generically query other items that have been associated with this `item`.
"""
foreign_key_name = foreign_key_name or self.foreign_key_name
foreign_key = getattr( associated_model_class, foreign_key_name )
foreign_key = self._foreign_key( associated_model_class, foreign_key_name=foreign_key_name )
return self.session().query( associated_model_class ).filter( foreign_key == item )
# a rename of sql DELETE to differentiate from the Galaxy notion of mark_as_deleted
@@ -492,6 +493,34 @@ 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
# examples where this doesn't really work are ConfigurationSerializer (no manager)
# and contents (2 managers)
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 )
# this will error for unset model_manager_class'es
return self._manager
# ==== SERIALIZERS/to_dict,from_dict
class ModelSerializingError( exceptions.InternalServerError ):
"""Thrown when request model values can't be serialized"""
@@ -513,7 +542,7 @@ class SkipAttribute( Exception ):
pass
class ModelSerializer( object ):
class ModelSerializer( HasAModelManager ):
"""
Turns models into JSONable dicts.
@@ -533,10 +562,11 @@ class ModelSerializer( object ):
#: 'service' to use for getting urls - use class var to allow overriding when testing
url_for = staticmethod( routes.url_for )
def __init__( self, app ):
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
# a list of valid serializable keys that can use the default (string) serializer
@@ -681,20 +711,18 @@ 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 ):
def __init__( self, app, **kwargs ):
"""
Set up deserializers and validator.
"""
super( ModelDeserializer, self ).__init__( app, **kwargs )
self.app = app
self.deserializers = {}
@@ -703,11 +731,6 @@ class ModelDeserializer( object ):
# a sub object that can validate incoming values
self.validate = ModelValidator( self.app )
# create a generically accessible manager for the model this deserializer works with/for
self.manager = None
if self.model_manager_class:
self.manager = self.model_manager_class( self.app )
def add_deserializers( self ):
"""
Register a map of attribute keys -> functions that will deserialize data
@@ -773,7 +796,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
@@ -781,6 +804,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 ):
@@ -856,7 +880,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:
@@ -882,10 +906,11 @@ class ModelFilterParser( object ):
#: model class
model_class = None
def __init__( self, app ):
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
# dictionary containing parsing data for ORM/SQLAlchemy-based filters
@@ -917,6 +942,7 @@ class ModelFilterParser( object ):
"""
Parse string 3-tuples (attr, op, val) into orm or functional filters.
"""
# TODO: allow defining the default filter op in this class (and not 'eq' in base/controller.py)
parsed = []
for ( attr, op, val ) in filter_tuple_list:
filter_ = self.parse_filter( attr, op, val )
@@ -946,7 +972,7 @@ class ModelFilterParser( object ):
# 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, ValueError=str( val_err ) )
column=attr, operation=op, value=val, ValueError=str( val_err ) )
# 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
+2 -1
View File
@@ -16,6 +16,7 @@ log = logging.getLogger( __name__ )
# TODO: for lack of a manager file for the config. May well be better in config.py? Circ imports?
class ConfigSerializer( base.ModelSerializer ):
"""Configuration (galaxy.ini) settings viewable by all users"""
def __init__( self, app ):
super( ConfigSerializer, self ).__init__( app )
@@ -75,7 +76,7 @@ class ConfigSerializer( base.ModelSerializer ):
class AdminConfigSerializer( ConfigSerializer ):
# config attributes viewable by admin users
"""Configuration attributes viewable only by admin users"""
def add_serializers( self ):
super( AdminConfigSerializer, self ).add_serializers()
+4 -1
View File
@@ -133,10 +133,11 @@ class DatasetRBACPermissions( object ):
class DatasetSerializer( base.ModelSerializer, deletable.PurgableSerializerMixin ):
model_manager_class = DatasetManager
def __init__( self, app ):
super( DatasetSerializer, self ).__init__( app )
self.dataset_manager = DatasetManager( app )
self.dataset_manager = self.manager
# needed for admin test
self.user_manager = users.UserManager( app )
@@ -274,6 +275,8 @@ class DatasetAssociationManager( base.ModelManager,
# Instead, a dataset association HAS a dataset but contains metadata specific to a library (lda) or user (hda)
model_class = model.DatasetInstance
# NOTE: model_manager_class should be set in HDA/LDA subclasses
def __init__( self, app ):
super( DatasetAssociationManager, self ).__init__( app )
self.dataset_manager = DatasetManager( app )
+3 -3
View File
@@ -243,12 +243,11 @@ class HDASerializer( # datasets._UnflattenedMetadataDatasetAssociationSerialize
datasets.DatasetAssociationSerializer,
taggable.TaggableSerializerMixin,
annotatable.AnnotatableSerializerMixin ):
# TODO: inherit from datasets.DatasetAssociationSerializer
# TODO: move what makes sense into DatasetSerializer
model_manager_class = HDAManager
def __init__( self, app ):
super( HDASerializer, self ).__init__( app )
self.hda_manager = HDAManager( app )
self.hda_manager = self.manager
self.default_view = 'summary'
self.add_view( 'summary', [
@@ -463,6 +462,7 @@ class HDADeserializer( datasets.DatasetAssociationDeserializer,
class HDAFilterParser( datasets.DatasetAssociationFilterParser,
taggable.TaggableFilterMixin,
annotatable.AnnotatableFilterMixin ):
model_manager_class = HDAManager
model_class = model.HistoryDatasetAssociation
def _add_parsers( self ):
+10 -7
View File
@@ -33,7 +33,6 @@ class HistoryManager( sharable.SharableModelManager, deletable.PurgableManagerMi
def __init__( self, app, *args, **kwargs ):
super( HistoryManager, self ).__init__( app, *args, **kwargs )
self.hda_manager = hdas.HDAManager( app )
def copy( self, history, user, **kwargs ):
@@ -142,6 +141,7 @@ class HistoryManager( sharable.SharableModelManager, deletable.PurgableManagerMi
return desc( self.model_class.disk_size )
if order_by_string == 'size-asc':
return asc( self.model_class.disk_size )
# TODO: add functional/non-orm orders (such as rating)
if default:
return self.parse_order_by( default )
raise glx_exceptions.RequestParameterInvalidException( 'Unkown order_by', order_by=order_by_string,
@@ -152,12 +152,13 @@ class HistorySerializer( sharable.SharableModelSerializer, deletable.PurgableSer
"""
Interface/service object for serializing histories into dictionaries.
"""
model_manager_class = HistoryManager
SINGLE_CHAR_ABBR = 'h'
def __init__( self, app ):
super( HistorySerializer, self ).__init__( app )
def __init__( self, app, **kwargs ):
super( HistorySerializer, self ).__init__( app, **kwargs )
self.history_manager = HistoryManager( app )
self.history_manager = self.manager
self.hda_manager = hdas.HDAManager( app )
self.hda_serializer = hdas.HDASerializer( app )
@@ -189,8 +190,10 @@ class HistorySerializer( sharable.SharableModelSerializer, deletable.PurgableSer
'state',
'state_details',
'state_ids',
# in the Historys' case, each of these views includes the keys from the previous
# 'community_rating',
# 'user_rating',
], include_keys_from='summary' )
# in the Historys' case, each of these views includes the keys from the previous
# assumes: outgoing to json.dumps and sanitized
def add_serializers( self ):
@@ -337,9 +340,9 @@ class HistoryDeserializer( sharable.SharableModelDeserializer, deletable.Purgabl
})
class HistoryFilters( sharable.SharableModelFilters,
deletable.PurgableFiltersMixin ):
class HistoryFilters( sharable.SharableModelFilters, deletable.PurgableFiltersMixin ):
model_class = model.History
model_manager_class = HistoryManager
def _add_parsers( self ):
super( HistoryFilters, self )._add_parsers()
+91 -13
View File
@@ -2,19 +2,61 @@
Mixins for Ratable model managers and serializers.
"""
from sqlalchemy.sql.expression import func
from . import base
import logging
log = logging.getLogger( __name__ )
# TODO: stub
class RatableManagerMixin( object ):
#: class of RatingAssociation (e.g. HistoryRatingAssociation)
rating_assoc = None
# TODO: most of this seems to be covered by item_attrs.UsesItemRatings
def rating( self, item, user, as_int=True ):
"""Returns the integer rating given to this item by the user.
# def by_user( self, trans, user, **kwargs ):
# pass
Returns the full rating model if `as_int` is False.
"""
rating = self.query_associated( self.rating_assoc, item ).filter_by( user=user ).first()
# most common case is assumed to be 'get the number'
if not as_int:
return rating
# get the int value if there's a rating
return rating.rating if rating is not None else None
def ratings( self, 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 ):
"""Returns the average of all ratings given to this item."""
foreign_key = self._foreign_key( self.rating_assoc )
avg = self.session().query( func.avg( self.rating_assoc.rating ) ).filter( foreign_key == item ).scalar()
return avg or 0.0
def ratings_count( self, 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()
def rate( self, item, user, value, flush=True ):
"""Updates or creates a rating for this item and user. Returns the rating"""
# TODO?: possible generic update_or_create
# TODO?: update and create to RatingsManager (if not overkill)
rating = self.rating( item, user, as_int=False )
if not rating:
rating = self.rating_assoc( user=user )
self.associate( rating, item )
rating.rating = value
self.session().add( rating )
if flush:
self.session().flush()
return rating
# TODO?: all ratings for a user
class RatableSerializerMixin( object ):
@@ -24,22 +66,58 @@ class RatableSerializerMixin( object ):
self.serializers[ 'community_rating' ] = self.serialize_community_rating
def serialize_user_rating( self, item, key, user=None, **context ):
"""
"""
pass
"""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 )
def serialize_community_rating( self, item, key, **context ):
"""
Returns a dictionary containing:
`average` the (float) average of all ratings of this object
`count` the number of ratings
"""
pass
# ??: 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
return {
'average' : manager.ratings_avg( item ),
'count' : manager.ratings_count( item ),
}
class RatableDeserializerMixin( object ):
def add_deserializers( self ):
pass
# self.deserializers[ 'user_rating' ] = self.deserialize_rating
self.deserializers[ 'user_rating' ] = self.deserialize_rating
# def deserialize_rating( self, trans, item, key, val ):
# val = self.validate.int_range( key, val, 0, 5 )
# return self.set_rating...( trans, item, val, user=trans.user )
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' ) )
val = self.validate.int_range( key, val, 0, 5 )
return self.manager.rate( item, user, val, flush=False )
class RatableFilterMixin( object ):
def _ratings_avg_accessor( self, item ):
return self.manager.ratings_avg( item )
def _add_parsers( self ):
"""
Adds the following filters:
`community_rating`: filter
"""
self.fn_filter_parsers.update({
'community_rating': {
'op': {
'eq' : lambda i, v: self._ratings_avg_accessor( i ) == v,
# TODO: default to greater than (currently 'eq' due to base/controller.py)
'ge' : lambda i, v: self._ratings_avg_accessor( i ) >= v,
'le' : lambda i, v: self._ratings_avg_accessor( i ) <= v,
},
'val' : float
}
})
+6 -5
View File
@@ -122,12 +122,13 @@ class SharableModelManager( base.ModelManager, secured.OwnableManagerMixin, secu
filters = self._munge_filters( published_filter, filters )
return self.query( filters=filters, **kwargs )
def list_published( self, **kwargs ):
def list_published( self, filters=None, **kwargs ):
"""
Return a list of all published items.
"""
query = self._query_published( **kwargs )
return self.list( query=query, **kwargs )
published_filter = self.model_class.published == true()
filters = self._munge_filters( published_filter, filters )
return self.list( filters=filters, **kwargs )
# .... user sharing
# sharing is often done via a 3rd table btwn a User and an item -> a <Item>UserShareAssociation
@@ -410,13 +411,13 @@ class SharableModelDeserializer( base.ModelDeserializer,
class SharableModelFilters( base.ModelFilterParser,
taggable.TaggableFilterMixin,
annotatable.AnnotatableFilterMixin ):
taggable.TaggableFilterMixin, annotatable.AnnotatableFilterMixin, ratable.RatableFilterMixin ):
def _add_parsers( self ):
super( SharableModelFilters, self )._add_parsers()
taggable.TaggableFilterMixin._add_parsers( self )
annotatable.AnnotatableFilterMixin._add_parsers( self )
ratable.RatableFilterMixin._add_parsers( self )
self.orm_filter_parsers.update({
'importable' : { 'op': ( 'eq' ), 'val': self.parse_bool },
+4 -1
View File
@@ -239,13 +239,14 @@ class UserManager( base.ModelManager, deletable.PurgableManagerMixin ):
class UserSerializer( base.ModelSerializer, deletable.PurgableSerializerMixin ):
model_manager_class = UserManager
def __init__( self, app ):
"""
Convert a User and associated data to a dictionary representation.
"""
super( UserSerializer, self ).__init__( app )
self.user_manager = UserManager( app )
self.user_manager = self.manager
self.default_view = 'summary'
self.add_view( 'summary', [
@@ -288,6 +289,7 @@ class UserSerializer( base.ModelSerializer, deletable.PurgableSerializerMixin ):
class CurrentUserSerializer( UserSerializer ):
model_manager_class = UserManager
def serialize( self, user, keys, **kwargs ):
"""
@@ -324,6 +326,7 @@ class CurrentUserSerializer( UserSerializer ):
class AdminUserFilterParser( base.ModelFilterParser, deletable.PurgableFiltersMixin ):
model_manager_class = UserManager
model_class = model.User
def _add_parsers( self ):
+2 -1
View File
@@ -42,11 +42,12 @@ class VisualizationSerializer( sharable.SharableModelSerializer ):
"""
Interface/service object for serializing visualizations into dictionaries.
"""
model_manager_class = VisualizationManager
SINGLE_CHAR_ABBR = 'v'
def __init__( self, app ):
super( VisualizationSerializer, self ).__init__( app )
self.visualizations_manager = VisualizationManager( app )
self.visualization_manager = self.manager
self.default_view = 'summary'
self.add_view( 'summary', [] )
+1 -1
View File
@@ -419,7 +419,7 @@ class HistoriesController( BaseAPIController, ExportsHistoryMixin, ImportsHistor
self.history_deserializer.deserialize( history, payload, user=trans.user, trans=trans )
return self.history_serializer.serialize_to_view( history,
user=trans.user, trans=trans, **self._parse_serialization_params( kwd, 'detailed' ) )
user=trans.user, trans=trans, **self._parse_serialization_params( kwd, 'detailed' ) )
@expose_api
def archive_export( self, trans, id, **kwds ):
+89 -9
View File
@@ -17,8 +17,10 @@ from galaxy import exceptions
from base import BaseTestCase
from galaxy.managers import base
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
@@ -334,6 +336,40 @@ class HistoryManagerTestCase( BaseTestCase ):
self.assertEqual( self.history_manager.set_current_by_id( self.trans, history1.id ), history1 )
self.assertEqual( self.history_manager.get_current( self.trans ), history1 )
def test_rating( self ):
user2 = self.user_manager.create( **user2_data )
manager = self.history_manager
item = manager.create( name='history1', user=user2 )
self.log( "should properly handle no ratings" )
self.assertEqual( manager.rating( item, user2 ), None )
self.assertEqual( manager.ratings( item ), [] )
self.assertEqual( manager.ratings_avg( item ), 0 )
self.assertEqual( manager.ratings_count( item ), 0 )
self.log( "should allow rating by user" )
manager.rate( item, user2, 5 )
self.assertEqual( manager.rating( item, user2 ), 5 )
self.assertEqual( manager.ratings( item ), [ 5 ] )
self.assertEqual( manager.ratings_avg( item ), 5 )
self.assertEqual( manager.ratings_count( item ), 1 )
self.log( "should allow updating" )
manager.rate( item, user2, 4 )
self.assertEqual( manager.rating( item, user2 ), 4 )
self.assertEqual( manager.ratings( item ), [ 4 ] )
self.assertEqual( manager.ratings_avg( item ), 4 )
self.assertEqual( manager.ratings_count( item ), 1 )
self.log( "should reflect multiple reviews" )
user3 = self.user_manager.create( **user3_data )
self.assertEqual( manager.rating( item, user3 ), None )
manager.rate( item, user3, 1 )
self.assertEqual( manager.rating( item, user3 ), 1 )
self.assertEqual( manager.ratings( item ), [ 4, 1 ] )
self.assertEqual( manager.ratings_avg( item ), 2.5 )
self.assertEqual( manager.ratings_count( item ), 2 )
# =============================================================================
# web.url_for doesn't work well in the framework
@@ -426,7 +462,7 @@ class HistorySerializerTestCase( BaseTestCase ):
user2 = self.user_manager.create( **user2_data )
history1 = self.history_manager.create( name='history1', user=user2 )
all_keys = list( self.history_serializer.serializable_keyset )
serialized = self.history_serializer.serialize( history1, all_keys )
serialized = self.history_serializer.serialize( history1, all_keys, user=user2 )
self.log( 'everything serialized should be of the proper type' )
self.assertIsInstance( serialized[ 'size' ], int )
@@ -507,17 +543,57 @@ class HistorySerializerTestCase( BaseTestCase ):
self.log( 'serialized should jsonify well' )
self.assertIsJsonifyable( serialized )
def test_ratings( self ):
user2 = self.user_manager.create( **user2_data )
user3 = self.user_manager.create( **user3_data )
manager = self.history_manager
serializer = self.history_serializer
item = manager.create( name='history1', user=user2 )
# # =============================================================================
# class HistoryDeserializerTestCase( BaseTestCase ):
self.log( 'serialization should reflect no ratings' )
serialized = serializer.serialize( item, [ 'user_rating', 'community_rating' ], user=user2 )
self.assertEqual( serialized[ 'user_rating' ], None )
self.assertEqual( serialized[ 'community_rating' ][ 'count' ], 0 )
self.assertEqual( serialized[ 'community_rating' ][ 'average' ], 0.0 )
# def set_up_managers( self ):
# super( HistoryDeserializerTestCase, self ).set_up_managers()
# self.history_manager = HistoryManager( self.app )
# self.history_deserializer = HistoryDeserializer( self.app )
self.log( 'serialization should reflect ratings' )
manager.rate( item, user2, 1 )
manager.rate( item, user3, 4 )
serialized = serializer.serialize( item, [ 'user_rating', 'community_rating' ], user=user2 )
self.assertEqual( serialized[ 'user_rating' ], 1 )
self.assertEqual( serialized[ 'community_rating' ][ 'count' ], 2 )
self.assertEqual( serialized[ 'community_rating' ][ 'average' ], 2.5 )
self.assertIsJsonifyable( serialized )
# def test_base( self ):
# pass
self.log( 'serialization of user_rating without user should error' )
self.assertRaises( base.ModelSerializingError,
serializer.serialize, item, [ 'user_rating' ] )
# =============================================================================
class HistoryDeserializerTestCase( BaseTestCase ):
def set_up_managers( self ):
super( HistoryDeserializerTestCase, self ).set_up_managers()
self.history_manager = HistoryManager( self.app )
self.history_deserializer = HistoryDeserializer( self.app )
def test_ratings( self ):
user2 = self.user_manager.create( **user2_data )
manager = self.history_manager
deserializer = self.history_deserializer
item = manager.create( name='history1', user=user2 )
self.log( 'deserialization should allow ratings change' )
deserializer.deserialize( item, { 'user_rating' : 4 }, user=user2 )
self.assertEqual( manager.rating( item, user2 ), 4 )
self.assertEqual( manager.ratings( item ), [ 4 ] )
self.assertEqual( manager.ratings_avg( item ), 4 )
self.assertEqual( manager.ratings_count( item ), 1 )
self.log( 'deserialization should fail silently on community_rating' )
deserializer.deserialize( item, { 'community_rating' : 4 }, user=user2 )
self.assertEqual( manager.ratings_count( item ), 1 )
# =============================================================================
@@ -752,6 +828,10 @@ class HistoryFiltersTestCase( BaseTestCase ):
found = self.history_manager.list( filters=filters, offset=-1 )
self.assertEqual( found, deleted_and_annotated )
# TODO: eq, ge, le
# def test_ratings( self ):
# pass
# =============================================================================
if __name__ == '__main__':