From 9358e59af80d9c5f88d5713ca7d1e648bfad1403 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Tue, 23 Sep 2014 18:17:48 -0400 Subject: Improve argspec detection and leniency for wrapped controllers. Pecan makes abundant use of `inspect.getargspec`, but unless you're very meticulous in the decorators you wrap your controllers with, the original argspec is not persisted (and pecan functionality can break in various ways). When a controller is decorated in a way that breaks argspec, we should instead attempt to locate the *actual* argspec for the method (not the wrapped function) and use it. Additionally, when controllers are missing **kwargs in the method signature to map optional GET and POST arguments, we shouldn't consider that a non-routable offense (an HTTP 400); instead, we should just *not* pass extraneous arguments to the function. Change-Id: I47fe0496ff6aa105359ee8e5b99f6c80476cc2e9 --- pecan/core.py | 11 +++++++- pecan/decorators.py | 4 +-- pecan/rest.py | 4 +-- pecan/routing.py | 3 +-- pecan/tests/test_util.py | 68 ++++++++++++++++++++++++++++++++++++++++++++++++ pecan/util.py | 28 ++++++++++++++++++++ 6 files changed, 111 insertions(+), 7 deletions(-) create mode 100644 pecan/tests/test_util.py diff --git a/pecan/core.py b/pecan/core.py index 9174c50..3a6f684 100644 --- a/pecan/core.py +++ b/pecan/core.py @@ -19,7 +19,7 @@ from .compat import urlparse, unquote_plus, izip from .secure import handle_security from .templating import RendererFactory from .routing import lookup_controller, NonCanonicalPath -from .util import _cfg, encode_if_needed +from .util import _cfg, encode_if_needed, getargspec from .middleware.recursive import ForwardRequestException @@ -527,6 +527,15 @@ class PecanBase(object): resp = state.response pecan_state = req.pecan + # If a keyword is supplied via HTTP GET or POST arguments, but the + # function signature does not allow it, just drop it (rather than + # generating a TypeError). + argspec = getargspec(controller) + keys = kwargs.keys() + for key in keys: + if key not in argspec.args and not argspec.keywords: + kwargs.pop(key) + # get the result from the controller result = controller(*args, **kwargs) diff --git a/pecan/decorators.py b/pecan/decorators.py index 45ad635..ed054ec 100644 --- a/pecan/decorators.py +++ b/pecan/decorators.py @@ -1,8 +1,8 @@ -from inspect import getargspec, getmembers, isclass, ismethod, isfunction +from inspect import getmembers, isclass, ismethod, isfunction import six -from .util import _cfg +from .util import _cfg, getargspec __all__ = [ 'expose', 'transactional', 'accept_noncanonical', 'after_commit', diff --git a/pecan/rest.py b/pecan/rest.py index 48ca1f2..9406ad4 100644 --- a/pecan/rest.py +++ b/pecan/rest.py @@ -1,4 +1,4 @@ -from inspect import getargspec, ismethod +from inspect import ismethod import warnings from webob import exc @@ -7,7 +7,7 @@ import six from .core import abort from .decorators import expose from .routing import lookup_controller, handle_lookup_traversal -from .util import iscontroller +from .util import iscontroller, getargspec class RestController(object): diff --git a/pecan/routing.py b/pecan/routing.py index c6abb2e..7acff76 100644 --- a/pecan/routing.py +++ b/pecan/routing.py @@ -1,10 +1,9 @@ import warnings -from inspect import getargspec from webob import exc from .secure import handle_security, cross_boundary -from .util import iscontroller +from .util import iscontroller, getargspec __all__ = ['lookup_controller', 'find_object'] diff --git a/pecan/tests/test_util.py b/pecan/tests/test_util.py new file mode 100644 index 0000000..c1cdfbd --- /dev/null +++ b/pecan/tests/test_util.py @@ -0,0 +1,68 @@ +import functools +import inspect +import unittest + +from pecan import expose +from pecan import util + + +class TestArgSpec(unittest.TestCase): + + @property + def controller(self): + + class RootController(object): + + @expose() + def index(self, a, b, c=1, *args, **kwargs): + return 'Hello, World!' + + return RootController() + + def test_no_decorator(self): + expected = inspect.getargspec(self.controller.index.__func__) + actual = util.getargspec(self.controller.index.__func__) + assert expected == actual + + def test_simple_decorator(self): + def dec(f): + return f + + expected = inspect.getargspec(self.controller.index.__func__) + actual = util.getargspec(dec(self.controller.index.__func__)) + assert expected == actual + + def test_simple_wrapper(self): + def dec(f): + @functools.wraps(f) + def wrapped(*a, **kw): + return f(*a, **kw) + return wrapped + + expected = inspect.getargspec(self.controller.index.__func__) + actual = util.getargspec(dec(self.controller.index.__func__)) + assert expected == actual + + def test_multiple_decorators(self): + def dec(f): + @functools.wraps(f) + def wrapped(*a, **kw): + return f(*a, **kw) + return wrapped + + expected = inspect.getargspec(self.controller.index.__func__) + actual = util.getargspec(dec(dec(dec(self.controller.index.__func__)))) + assert expected == actual + + def test_decorator_with_args(self): + def dec(flag): + def inner(f): + @functools.wraps(f) + def wrapped(*a, **kw): + return f(*a, **kw) + return wrapped + return inner + + expected = inspect.getargspec(self.controller.index.__func__) + actual = util.getargspec(dec(True)(self.controller.index.__func__)) + assert expected == actual diff --git a/pecan/util.py b/pecan/util.py index aa2e683..168ff66 100644 --- a/pecan/util.py +++ b/pecan/util.py @@ -1,10 +1,38 @@ +import inspect import sys +import six + def iscontroller(obj): return getattr(obj, 'exposed', False) +def getargspec(method): + """ + Drill through layers of decorators attempting to locate the actual argspec + for a method. + """ + + argspec = inspect.getargspec(method) + args = argspec[0] + if args and args[0] == 'self': + return argspec + if hasattr(method, '__func__'): + method = method.__func__ + + func_closure = six.get_function_closure(method) + + closure = next( + ( + c for c in func_closure if six.callable(c.cell_contents) + ), + None + ) + method = closure.cell_contents + return getargspec(method) + + def _cfg(f): if not hasattr(f, '_pecan'): f._pecan = {} -- cgit v1.2.1