Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #2429 (closed defect: fixed)

Opened 10 years ago

Last modified 9 years ago

paginate() breaks when calling a method with multiple keyword arguments

Reported by: pitrou Owned by: percious
Priority: normal Milestone: 2.1b1
Component: TurboGears Version: 2.0.3
Severity: normal Keywords:
Cc:

Description

Suppose you have the following declaration in own of your controllers:

  @expose()
  @paginate('foobar')
  def list(self, x=None, y=None, submit=None):
      # do stuff with self

Then it will break as soon as you call the method with an URL with more than one parameter such as .../list?x=1&y=2, because self will get replaced with one of the URL parameters. This is because of a bug of the parameter replacement logic in paginate() (whose purpose I don't exactly understand). It's a simple off-by-one error: the zeroth element of the argument specification of the decorated function is skipped, but the corresponding zeroth element of *args is not. The attached patch fixes the problem.

Attachments

decorators.patch Download (731 bytes) - added by pitrou 10 years ago.

Change History

Changed 10 years ago by pitrou

comment:1 Changed 9 years ago by percious

  • Owner set to percious
  • Milestone changed from __unclassified__ to 2.1b1

comment:2 Changed 9 years ago by percious

I'm not sure about this one.

Line 256 seems to do what the remainder of the patch does.

Suppose it should be argspec...[:2] instead of argspec...[:1] ?

comment:3 Changed 9 years ago by pitrou

Well, argspec..[:1] is fine: it removes the self from the method signature, shifting the remaining method parameters to the left:

>>> def f(self, x, y, z=1):
...   pass
... 
>>> inspect.getargspec(f)[0][1:]
['x', 'y', 'z']

However, similar left-shifting must be accomplished on the actual arguments when mapping them to the parameters from the method signature, which is what the patch does: when calling f(*args, **kwargs), args[0] has to contain the self object for the method, i.e. the controller instance. Without the patch, args[0] can be overwritten with the value of the first non-self argument (x in the example above).

If I look at  http://bitbucket.org/turbogears/tg-dev/src/tip/tg/decorators.py (is it the right repo?), the logic is totally different in 2.1 so the bug probably only applies to 2.0.

comment:4 Changed 9 years ago by percious

  • Status changed from new to closed
  • Resolution set to fixed

okay, im pretty certain this ticket is no longer valid, because the code that did this stuff has been removed in entirety. see:

 http://bitbucket.org/turbogears/tg-2_0/changeset/510df775d974/#chg-tg/decorators.py

and

 http://bitbucket.org/turbogears/tg-dev/src/tip/tg/decorators.py

I'm not sure what the code was supposed to do originaly, but the tests pass without it missing, so unless someone would like to provide a failing test in the TG2.1 codebase, I assume that the problem is fixed.

comment:5 Changed 9 years ago by pitrou

Indeed, the offending code (which seemed of dubious utility) has been removed. Thank you.

« removed undocumented and non-functional code by chris p. that broke pagination. added tests for pagination. »

Note: See TracTickets for help on using tickets.