From baf80afa5eda4928eea2349c5cc494b09783ddd2 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Fri, 16 Jan 2015 13:30:45 -0500 Subject: Allow users to specify custom path segments for routing. Change-Id: I3f998d62c2f188818a747f1aa91beb9439ed77a2 --- docs/source/routing.rst | 58 +++++++++++ pecan/__init__.py | 9 +- pecan/decorators.py | 23 ++++- pecan/rest.py | 31 +++++- pecan/routing.py | 157 +++++++++++++++++++++++++++- pecan/tests/test_base.py | 263 ++++++++++++++++++++++++++++++++++++++++++++++- pecan/tests/test_rest.py | 16 +++ 7 files changed, 549 insertions(+), 8 deletions(-) diff --git a/docs/source/routing.rst b/docs/source/routing.rst index a2bf378..f099787 100644 --- a/docs/source/routing.rst +++ b/docs/source/routing.rst @@ -136,6 +136,64 @@ use the ``text/html`` content type by default. * :ref:`pecan_decorators` +Specifying Explicit Path Segments +--------------------------------- + +Occasionally, you may want to use a path segment in your routing that doesn't +work with Pecan's declarative approach to routing because of restrictions in +Python's syntax. For example, if you wanted to route for a path that includes +dashes, such as ``/some-path/``, the following is *not* valid Python:: + + + class RootController(object): + + @pecan.expose() + def some-path(self): + return dict() + +To work around this, pecan allows you to specify an explicit path segment in +the :func:`~pecan.decorators.expose` decorator:: + + class RootController(object): + + @pecan.expose(route='some-path') + def some_path(self): + return dict() + +In this example, the pecan application will reply with an ``HTTP 200`` for +requests made to ``/some-path/``, but requests made to ``/some_path/`` will +yield an ``HTTP 404``. + +:func:`~pecan.routing.route` can also be used explicitly as an alternative to +the ``route`` argument in :func:`~pecan.decorators.expose`:: + + class RootController(object): + + @pecan.expose() + def some_path(self): + return dict() + + pecan.route('some-path', RootController.some_path) + +Routing to child controllers can be handled simliarly by utilizing +:func:`~pecan.routing.route`:: + + + class ChildController(object): + + @pecan.expose() + def child(self): + return dict() + + class RootController(object): + pass + + pecan.route(RootController, 'child-path', ChildController()) + +In this example, the pecan application will reply with an ``HTTP 200`` for +requests made to ``/child-path/child/``. + + Routing Based on Request Method ------------------------------- diff --git a/pecan/__init__.py b/pecan/__init__.py index 83ff059..3987892 100644 --- a/pecan/__init__.py +++ b/pecan/__init__.py @@ -4,6 +4,13 @@ from .core import ( ) from .decorators import expose from .hooks import RequestViewerHook + +from .middleware.debug import DebugMiddleware +from .middleware.errordocument import ErrorDocumentMiddleware +from .middleware.recursive import RecursiveMiddleware +from .middleware.static import StaticFileMiddleware +from .routing import route + from .configuration import set_config, Config from .configuration import _runtime_conf as conf from . import middleware @@ -20,7 +27,7 @@ import warnings __all__ = [ 'make_app', 'load_app', 'Pecan', 'Request', 'Response', 'request', 'response', 'override_template', 'expose', 'conf', 'set_config', 'render', - 'abort', 'redirect' + 'abort', 'redirect', 'route' ] diff --git a/pecan/decorators.py b/pecan/decorators.py index 36d33fa..68195a9 100644 --- a/pecan/decorators.py +++ b/pecan/decorators.py @@ -13,10 +13,10 @@ __all__ = [ def when_for(controller): def when(method=None, **kw): def decorate(f): - expose(**kw)(f) _cfg(f)['generic_handler'] = True controller._pecan['generic_handlers'][method.upper()] = f controller._pecan['allowed_methods'].append(method.upper()) + expose(**kw)(f) return f return decorate return when @@ -24,7 +24,8 @@ def when_for(controller): def expose(template=None, content_type='text/html', - generic=False): + generic=False, + route=None): ''' Decorator used to flag controller methods as being "exposed" for @@ -38,6 +39,11 @@ def expose(template=None, ``functools.singledispatch`` generic functions. Allows you to split a single controller into multiple paths based upon HTTP method. + :param route: The name of the path segment to match (excluding + separator characters, like `/`). Defaults to the name of + the function itself, but this can be used to resolve paths + which are not valid Python function names, e.g., if you + wanted to route a function to `some-special-path'. ''' if template == 'json': @@ -47,8 +53,19 @@ def expose(template=None, # flag the method as exposed f.exposed = True - # set a "pecan" attribute, where we will store details cfg = _cfg(f) + + if route: + # This import is here to avoid a circular import issue + from pecan import routing + if cfg.get('generic_handler'): + raise ValueError( + 'Path segments cannot be overridden for generic ' + 'controllers.' + ) + routing.route(route, f) + + # set a "pecan" attribute, where we will store details cfg['content_type'] = content_type cfg.setdefault('template', []).append(template) cfg.setdefault('content_types', {})[content_type] = template diff --git a/pecan/rest.py b/pecan/rest.py index ef5d753..adcb5f5 100644 --- a/pecan/rest.py +++ b/pecan/rest.py @@ -1,4 +1,4 @@ -from inspect import ismethod +from inspect import ismethod, getmembers import warnings from webob import exc @@ -27,6 +27,35 @@ class RestController(object): ''' _custom_actions = {} + def __new__(cls, *args, **kwargs): + """ + RestController does not support the `route` argument to + :func:`~pecan.decorators.expose` + + Implement this with __new__ rather than a metaclass, because it's very + common for pecan users to mixin RestController (with other bases that + have their own metaclasses). + """ + for name, value in getmembers(cls): + if iscontroller(value) and getattr(value, 'custom_route', None): + raise ValueError( + 'Path segments cannot be used in combination with ' + 'pecan.rest.RestController. Remove the `route` argument ' + 'to @pecan.expose on %s.%s.%s' % ( + cls.__module__, cls.__name__, value.__name__ + ) + ) + + # object.__new__ will error if called with extra arguments, and either + # __new__ is overridden or __init__ is not overridden; + # https://hg.python.org/cpython/file/78d36d54391c/Objects/typeobject.c#l3034 + # In PY3, this is actually a TypeError (in PY2, it just raises + # a DeprecationWarning) + new = super(RestController, cls).__new__ + if new is object.__new__: + return new(cls) + return new(cls, *args, **kwargs) + def _get_args_for_controller(self, controller): """ Retrieve the arguments we actually care about. For Pecan applications diff --git a/pecan/routing.py b/pecan/routing.py index 1d534aa..8633220 100644 --- a/pecan/routing.py +++ b/pecan/routing.py @@ -1,11 +1,99 @@ +import re import warnings +from inspect import getmembers, ismethod from webob import exc +import six from .secure import handle_security, cross_boundary from .util import iscontroller, getargspec, _cfg -__all__ = ['lookup_controller', 'find_object'] +__all__ = ['lookup_controller', 'find_object', 'route'] +__observed_controllers__ = set() +__custom_routes__ = {} + + +def route(*args): + """ + This function is used to define an explicit route for a path segment. + + You generally only want to use this in situations where your desired path + segment is not a valid Python variable/function name. + + For example, if you wanted to be able to route to: + + /path/with-dashes/ + + ...the following is invalid Python syntax:: + + class Controller(object): + + with-dashes = SubController() + + ...so you would instead define the route explicitly:: + + class Controller(object): + pass + + pecan.route(Controller, 'with-dashes', SubController()) + """ + + def _validate_route(route): + if not isinstance(route, six.string_types): + raise TypeError('%s must be a string' % route) + + if not re.match('^[0-9a-zA-Z-_$\(\),;:]+$', route): + raise ValueError( + '%s must be a valid path segment. Keep in mind ' + 'that path segments should not contain path separators ' + '(e.g., /) ' % route + ) + + if len(args) == 2: + # The handler in this situation is a @pecan.expose'd callable, + # and is generally only used by the @expose() decorator itself. + # + # This sets a special attribute, `custom_route` on the callable, which + # pecan's routing logic knows how to make use of (as a special case) + route, handler = args + if ismethod(handler): + handler = handler.__func__ + if not iscontroller(handler): + raise TypeError( + '%s must be a callable decorated with @pecan.expose' % handler + ) + obj, attr, value = handler, 'custom_route', route + + if handler.__name__ in ('_lookup', '_default', '_route'): + raise ValueError( + '%s is a special method in pecan and cannot be used in ' + 'combination with custom path segments.' % handler.__name__ + ) + elif len(args) == 3: + # This is really just a setattr on the parent controller (with some + # additional validation for the path segment itself) + _, route, handler = args + obj, attr, value = args + + if hasattr(obj, attr): + raise RuntimeError( + ( + "%(module)s.%(class)s already has an " + "existing attribute named \"%(route)s\"." % { + 'module': obj.__module__, + 'class': obj.__name__, + 'route': attr + } + ), + ) + else: + raise TypeError( + 'pecan.route should be called in the format ' + 'route(ParentController, "path-segment", SubController())' + ) + + _validate_route(route) + setattr(obj, attr, value) class PecanNotFound(Exception): @@ -102,7 +190,15 @@ def find_object(obj, remainder, notfound_handlers, request): if obj is None: raise PecanNotFound if iscontroller(obj): - return obj, remainder + if getattr(obj, 'custom_route', None) is None: + return obj, remainder + + _detect_custom_path_segments(obj) + + if remainder: + custom_route = __custom_routes__.get((obj.__class__, remainder[0])) + if custom_route: + return getattr(obj, custom_route), remainder[1:] # are we traversing to another controller cross_boundary(prev_obj, obj) @@ -165,3 +261,60 @@ def find_object(obj, remainder, notfound_handlers, request): if request.method in _cfg(prev_obj.index).get('generic_handlers', {}): return prev_obj.index, prev_remainder + + +def _detect_custom_path_segments(obj): + # Detect custom controller routes (on the initial traversal) + if obj.__class__.__module__ == '__builtin__': + return + + attrs = set(dir(obj)) + + if obj.__class__ not in __observed_controllers__: + for key, val in getmembers(obj): + if iscontroller(val) and isinstance( + getattr(val, 'custom_route', None), + six.string_types + ): + route = val.custom_route + + # Detect class attribute name conflicts + for conflict in attrs.intersection(set((route,))): + raise RuntimeError( + ( + "%(module)s.%(class)s.%(function)s has " + "a custom path segment, \"%(route)s\", " + "but %(module)s.%(class)s already has an " + "existing attribute named \"%(route)s\"." % { + 'module': obj.__class__.__module__, + 'class': obj.__class__.__name__, + 'function': val.__name__, + 'route': conflict + } + ), + ) + + existing = __custom_routes__.get( + (obj.__class__, route) + ) + if existing: + # Detect custom path conflicts between functions + raise RuntimeError( + ( + "%(module)s.%(class)s.%(function)s and " + "%(module)s.%(class)s.%(other)s have a " + "conflicting custom path segment, " + "\"%(route)s\"." % { + 'module': obj.__class__.__module__, + 'class': obj.__class__.__name__, + 'function': val.__name__, + 'other': existing, + 'route': route + } + ), + ) + + __custom_routes__[ + (obj.__class__, route) + ] = key + __observed_controllers__.add(obj.__class__) diff --git a/pecan/tests/test_base.py b/pecan/tests/test_base.py index e9f6af2..e6adf0e 100644 --- a/pecan/tests/test_base.py +++ b/pecan/tests/test_base.py @@ -16,7 +16,7 @@ from six.moves import cStringIO as StringIO from pecan import ( Pecan, Request, Response, expose, request, response, redirect, - abort, make_app, override_template, render + abort, make_app, override_template, render, route ) from pecan.templating import ( _builtin_renderers as builtin_renderers, error_formatters @@ -1965,3 +1965,264 @@ class TestDeprecatedRouteMethod(PecanTestCase): r = self.app_.get('/foo/bar/') assert r.status_int == 200 assert b_('foo, bar') in r.body + + +class TestExplicitRoute(PecanTestCase): + + def test_alternate_route(self): + + class RootController(object): + + @expose(route='some-path') + def some_path(self): + return 'Hello, World!' + + app = TestApp(Pecan(RootController())) + + r = app.get('/some-path/') + assert r.status_int == 200 + assert r.body == b_('Hello, World!') + + r = app.get('/some_path/', expect_errors=True) + assert r.status_int == 404 + + def test_manual_route(self): + + class SubController(object): + + @expose(route='some-path') + def some_path(self): + return 'Hello, World!' + + class RootController(object): + pass + + route(RootController, 'some-controller', SubController()) + + app = TestApp(Pecan(RootController())) + + r = app.get('/some-controller/some-path/') + assert r.status_int == 200 + assert r.body == b_('Hello, World!') + + r = app.get('/some-controller/some_path/', expect_errors=True) + assert r.status_int == 404 + + def test_manual_route_conflict(self): + + class SubController(object): + pass + + class RootController(object): + + @expose() + def hello(self): + return 'Hello, World!' + + self.assertRaises( + RuntimeError, + route, + RootController, + 'hello', + SubController() + ) + + def test_custom_route_on_index(self): + + class RootController(object): + + @expose(route='some-path') + def index(self): + return 'Hello, World!' + + app = TestApp(Pecan(RootController())) + + r = app.get('/some-path/') + assert r.status_int == 200 + assert r.body == b_('Hello, World!') + + r = app.get('/') + assert r.status_int == 200 + assert r.body == b_('Hello, World!') + + r = app.get('/index/', expect_errors=True) + assert r.status_int == 404 + + def test_custom_route_with_attribute_conflict(self): + + class RootController(object): + + @expose(route='mock') + def greet(self): + return 'Hello, World!' + + @expose() + def mock(self): + return 'You are not worthy!' + + app = TestApp(Pecan(RootController())) + + self.assertRaises( + RuntimeError, + app.get, + '/mock/' + ) + + def test_conflicting_custom_routes(self): + + class RootController(object): + + @expose(route='testing') + def foo(self): + return 'Foo!' + + @expose(route='testing') + def bar(self): + return 'Bar!' + + app = TestApp(Pecan(RootController())) + + self.assertRaises( + RuntimeError, + app.get, + '/testing/' + ) + + def test_conflicting_custom_routes_in_subclass(self): + + class BaseController(object): + + @expose(route='testing') + def foo(self): + return request.path + + class ChildController(BaseController): + pass + + class RootController(BaseController): + child = ChildController() + + app = TestApp(Pecan(RootController())) + + r = app.get('/testing/') + assert r.body == b_('/testing/') + + r = app.get('/child/testing/') + assert r.body == b_('/child/testing/') + + def test_custom_route_prohibited_on_lookup(self): + try: + class RootController(object): + + @expose(route='some-path') + def _lookup(self): + return 'Hello, World!' + except ValueError: + pass + else: + raise AssertionError( + '_lookup cannot be used with a custom path segment' + ) + + def test_custom_route_prohibited_on_default(self): + try: + class RootController(object): + + @expose(route='some-path') + def _default(self): + return 'Hello, World!' + except ValueError: + pass + else: + raise AssertionError( + '_default cannot be used with a custom path segment' + ) + + def test_custom_route_prohibited_on_route(self): + try: + class RootController(object): + + @expose(route='some-path') + def _route(self): + return 'Hello, World!' + except ValueError: + pass + else: + raise AssertionError( + '_route cannot be used with a custom path segment' + ) + + def test_custom_route_with_generic_controllers(self): + + class RootController(object): + + @expose(route='some-path', generic=True) + def foo(self): + return 'Hello, World!' + + @foo.when(method='POST') + def handle_post(self): + return 'POST!' + + app = TestApp(Pecan(RootController())) + + r = app.get('/some-path/') + assert r.status_int == 200 + assert r.body == b_('Hello, World!') + + r = app.get('/foo/', expect_errors=True) + assert r.status_int == 404 + + r = app.post('/some-path/') + assert r.status_int == 200 + assert r.body == b_('POST!') + + r = app.post('/foo/', expect_errors=True) + assert r.status_int == 404 + + def test_custom_route_prohibited_on_generic_controllers(self): + try: + class RootController(object): + + @expose(generic=True) + def foo(self): + return 'Hello, World!' + + @foo.when(method='POST', route='some-path') + def handle_post(self): + return 'POST!' + except ValueError: + pass + else: + raise AssertionError( + 'generic controllers cannot be used with a custom path segment' + ) + + def test_invalid_route_arguments(self): + class C(object): + + def secret(self): + return {} + + self.assertRaises(TypeError, route) + self.assertRaises(TypeError, route, 'some-path', lambda x: x) + self.assertRaises(TypeError, route, 'some-path', C.secret) + self.assertRaises(TypeError, route, C, {}, C()) + + for path in ( + 'VARIED-case-PATH', + 'this,custom,path', + '123-path', + 'path(with-parens)', + 'path;with;semicolons', + 'path:with:colons', + ): + handler = C() + route(C, path, handler) + assert getattr(C, path, handler) + + self.assertRaises(ValueError, route, C, '/path/', C()) + self.assertRaises(ValueError, route, C, '.', C()) + self.assertRaises(ValueError, route, C, '..', C()) + self.assertRaises(ValueError, route, C, 'path?', C()) + self.assertRaises(ValueError, route, C, 'percent%20encoded', C()) diff --git a/pecan/tests/test_rest.py b/pecan/tests/test_rest.py index 433e424..3ae4786 100644 --- a/pecan/tests/test_rest.py +++ b/pecan/tests/test_rest.py @@ -1554,3 +1554,19 @@ class TestRestController(PecanTestCase): r = app.get('/foo/%F0%9F%8C%B0/') assert r.status_int == 200 assert r.body == b'Hello, World!' + + +class TestExplicitRoute(PecanTestCase): + + def test_alternate_route(self): + + class RootController(RestController): + + @expose(route='some-path') + def get_all(self): + return "Hello, World!" + + self.assertRaises( + ValueError, + RootController + ) -- cgit v1.2.1