[Kimchi-devel] [PATCH V2] Issue #737: Fix to remove twice calls of resource lookup on GET OPERATION

[Aline Manera]

On 05/11/2015 10:41, Archana Singh wrote:
> I think as you mention remove lookup() from get() and in update() add 
> call to lookup() and then get().

Yeap! I think that can work!

>
> On 11/5/2015 5:56 PM, Archana Singh wrote:
>> I have tried that option but it does not help as Resource.get() gets 
>> called from update place which needs lookup to be run again to get 
>> the updated value.
>>
>> On 11/5/2015 5:54 PM, Aline Manera wrote:
>>>
>>> I think the only way to reduce the lookup() calls for a GET request 
>>> is removing it from Resource.get() as the lookup() was already 
>>> called in index() and everything keep as it is today.
>>>
>>> On 03/11/2015 16:54, archus at linux.vnet.ibm.com wrote:
>>>> From: Archana Singh <archus at linux.vnet.ibm.com>
>>>>
>>>> Incase of GET lookup was called twice.
>>>> Once lookup() before is_authorized() and then in self.get().
>>>> This added overhead to system when lookup() is called
>>>> for each value in list from get_list() of Collection.
>>>> So to avoid this overhead, lookup() should not be called
>>>> before self.get().
>>>>
>>>> 1) Added lookup() call from Resource's update(), delete().
>>>> 2) Removed lookup() call from Resource's index().
>>>> 2) As is_authorized() calls self.data which calls self.info.
>>>> Added check to make sure that self.data only get called
>>>> if self.info is not None. And intialized self.info as None
>>>> in __init__. As its value is getting assigned in lookup().
>>>> 3) In _generate_action_handler_base(), lookup() was getting
>>>> called before is_authorized(), move its call after is_authorized().
>>>> ---
>>>>   src/wok/control/base.py | 16 ++++++++++------
>>>>   1 file changed, 10 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/src/wok/control/base.py b/src/wok/control/base.py
>>>> index 638e196..4f25ac9 100644
>>>> --- a/src/wok/control/base.py
>>>> +++ b/src/wok/control/base.py
>>>> @@ -58,6 +58,7 @@ class Resource(object):
>>>>           self.model_args = (ident,)
>>>>           self.role_key = None
>>>>           self.admin_methods = []
>>>> +        self.info = None
>>>>
>>>>       def _redirect(self, action_result, code=303):
>>>>           if isinstance(action_result, list):
>>>> @@ -102,10 +103,9 @@ class Resource(object):
>>>>           def wrapper(*args, **kwargs):
>>>>               validate_method(('POST'), self.role_key, 
>>>> self.admin_methods)
>>>>               try:
>>>> -                self.lookup()
>>>>                   if not self.is_authorized():
>>>>                       raise UnauthorizedError('WOKAPI0009E')
>>>> -
>>>> +                self.lookup()
>>>>                   model_args = list(self.model_args)
>>>>                   if action_args is not None:
>>>>                       request = parse_request()
>>>> @@ -145,6 +145,7 @@ class Resource(object):
>>>>
>>>>       def delete(self):
>>>>           try:
>>>> +            self.lookup()
>>>>               fn = getattr(self.model, model_fn(self, 'delete'))
>>>>               fn(*self.model_args)
>>>>               cherrypy.response.status = 204
>>>> @@ -163,10 +164,8 @@ class Resource(object):
>>>>                                    self.role_key, self.admin_methods)
>>>>
>>>>           try:
>>>> -            self.lookup()
>>>>               if not self.is_authorized():
>>>>                   raise UnauthorizedError('WOKAPI0009E')
>>>> -
>>>>               return {'GET': self.get,
>>>>                       'DELETE': self.delete,
>>>>                       'PUT': self.update}[method](*args, **kargs)
>>>> @@ -188,8 +187,12 @@ class Resource(object):
>>>>           user_groups = cherrypy.session.get(USER_GROUPS, [])
>>>>           user_role = cherrypy.session.get(USER_ROLES, 
>>>> {}).get(self.role_key)
>>>>
>>>> -        users = self.data.get("users", None)
>>>> -        groups = self.data.get("groups", None)
>>>> +        users = None
>>>> +        groups = None
>>>> +
>>>> +        if self.info:
>>>> +            users = self.data.get("users", None)
>>>> +            groups = self.data.get("groups", None)
>>>>
>>>>           if (users is None and groups is None) or user_role == 
>>>> 'admin':
>>>>               return True
>>>> @@ -198,6 +201,7 @@ class Resource(object):
>>>>
>>>>       def update(self, *args, **kargs):
>>>>           try:
>>>> +            self.lookup()
>>>>               update = getattr(self.model, model_fn(self, 'update'))
>>>>           except AttributeError:
>>>>               e = InvalidOperation('WOKAPI0003E', {'resource':
>>>
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>
>