#21247: django.utils.method_decorator doesn't honour method binding properly
-------------------------------+--------------------
     Reporter:  grahamd        |      Owner:  nobody
         Type:  Uncategorized  |     Status:  new
    Component:  Uncategorized  |    Version:  1.5
     Severity:  Normal         |   Keywords:
 Triage Stage:  Unreviewed     |  Has patch:  0
Easy pickings:  0              |      UI/UX:  0
-------------------------------+--------------------
 The decorators created using {{{django.utils.method_decorator()}}} do not
 honour method binding properly.

 This becomes a problem when such a decorator is used in conjunction with
 decorators that are implemented as descriptors, implement the
 {{{__get__()}}} method and expect binding to be performed.

 Specifically, if a method decorator created using
 {{{django.utils.method_decorator()}}} is applied on top of a decorator or
 wrapper which is implemented as a descriptor then things will fail.

 {{{
 def my_wrapper_1(wrapped):
     def _wrapper(arg1, arg2):
         print('_wrapper', arg1, arg2)
         return wrapped(arg1, arg2)
     return _wrapper

 my_method_wrapper_1 = method_decorator(my_wrapper_1)

 class my_bound_wrapper_2(object):
     def __init__(self, wrapped):
         self.wrapped = wrapped
         self.__name__ = wrapped.__name__
     def __call__(self, arg1, arg2):
         print('my_bound_wrapper_2.__call__', arg1, arg2)
         return self.wrapped(arg1, arg2)
     def __get__(self, instance, owner):
         print('my_bound_wrapper_2.__get__', instance, owner)
         return self

 class my_desc_wrapper_2(object):
     def __init__(self, wrapped):
         self.wrapped = wrapped
         self.__name__ = wrapped.__name__
     def __get__(self, instance, owner):
         print('my_desc_wrapper_2.__get__', instance, owner)
         return my_bound_wrapper_2(self.wrapped.__get__(instance, owner))

 class Class(object):

     # Works
     @my_method_wrapper_1
     def method_1(self, arg1, arg2):
         print('Class.method_1', self, arg1, arg2)
         return arg1, arg2

     # Works
     @my_desc_wrapper_2
     def method_2(self, arg1, arg2):
         print('Class.method_2', arg1, arg2)
         return arg1, arg2

     # Fails
     @my_method_wrapper_1
     @my_desc_wrapper_2
     def method_3(self, arg1, arg2):
         print('Class.method_3', arg1, arg2)
         return arg1, arg2

 c = Class()

 c.method_1(1, 2) # Works.
 c.method_2(1, 2) # Works
 c.method_3(1, 2) # Fails.
 }}}

 The error one will see in this case is:

 {{{
 Traceback (most recent call last):
   File "runtest-1.py", line 90, in <module>
     c.method_3(1, 2)
   File "runtest-1.py", line 17, in _wrapper
     return bound_func(*args, **kwargs)
   File "runtest-1.py", line 37, in _wrapper
     return wrapped(arg1, arg2)
   File "runtest-1.py", line 13, in bound_func
     return func(self, *args2, **kwargs2)
 TypeError: 'my_desc_wrapper_2' object is not callable
 }}}

 This is because the decorator implemented as a decorator doesn't have a
 {{{__call__()}}} method. It instead expects the method to have been bound
 correctly to the instance. This would normally mean that {{{__get__()}}}
 is called to do the binding, which would see an instance of the bound
 wrapper returned. The {{{__call__()}}} of the bound wrapper would then
 normally be called.

 The {{{django.utils.method_decorator()}}} way of creating method
 decorators breaks this aspect of how instance method calling is meant to
 work for Python.

 The problem is the code in {{{django.utils.method_decorator()}}} of:

 {{{
         def _wrapper(self, *args, **kwargs):
             @decorator
             def bound_func(*args2, **kwargs2):
                 return func(self, *args2, **kwargs2)
             # bound_func has the signature that 'decorator' expects i.e.
 no
             # 'self' argument, but it is a closure over self so it can
 call
             # 'func' correctly.
             return bound_func(*args, **kwargs)
 }}}

 Although there are better ways of implementing decorators that can be used
 on both functions and instance methods, a fix for the way things are being
 done here is to use:

 {{{
         def _wrapper(self, *args, **kwargs):
             @decorator
             def bound_func(*args2, **kwargs2):
                 #return func(self, *args2, **kwargs2)
                 return func.__get__(self, type(self))(*args2, **kwargs2)
             # bound_func has the signature that 'decorator' expects i.e.
 no
             # 'self' argument, but it is a closure over self so it can
 call
             # 'func' correctly.
             return bound_func(*args, **kwargs)
 }}}

 That is, instead of simply calling the wrapped function (which is unbound
 at that point) and pass in the {{{self}}} argument explicitly:

 {{{
                 return func(self, *args2, **kwargs2)
 }}}

 it should bind the function to the instance prior to calling it.

 {{{

                 return func.__get__(self, type(self))(*args2, **kwargs2)
 }}}

 This results in it honouring correctly the binding requirements of
 instance methods and so it will not break in situations where the
 decorator is wrapped around another decorator which is implemented as a
 descriptor.

 Attached are {{{runtest-1.py}}}, which fails, and {{{runtest-2.py}}} which
 has the change and runs correctly.

-- 
Ticket URL: <https://code.djangoproject.com/ticket/21247>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/050.b27b42f161fec4ebac0a43a7242cc427%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to