Allow users to specify custom path segments for routing.

Change-Id: I3f998d62c2f188818a747f1aa91beb9439ed77a2

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
 -------------------------------
 

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'
 ]
 
 

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

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

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,8 +190,16 @@ def find_object(obj, remainder, notfound_handlers, request):
         if obj is None:
             raise PecanNotFound
         if iscontroller(obj):
+            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)
         try:
@@ -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__)

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())

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
+        )