Fix several bugs for RestController

Resolve several bugs in RestController that occur when the URL ends in
a trailing slash.

Change-Id: I6439ee9e45b715b0bea57e9af9124675376d0c9d
Fixes-bug: 1250566

2 files changed, 99 insertions, 19 deletions

diff --git a/pecan/rest.py b/pecan/rest.py
index ce111a8..9fc18ab 100644
--- a/pecan/rest.py
+++ b/pecan/rest.py
@@ -54,7 +54,11 @@ class RestController(object):
             return result
 
         # handle the request
-        handler = getattr(self, '_handle_%s' % method, self._handle_custom)
+        handler = getattr(
+            self,
+            '_handle_%s' % method,
+            self._handle_unknown_method
+        )
 
         try:
             result = handler(method, args)
@@ -140,9 +144,9 @@ class RestController(object):
                     remainder[fixed_args + 1:]
                 )
 
-    def _handle_custom(self, method, remainder):
+    def _handle_unknown_method(self, method, remainder):
         '''
-        Routes ``_custom`` actions to the appropriate controller.
+        Routes undefined actions (like RESET) to the appropriate controller.
         '''
         # try finding a post_{custom} or {custom} method first
         controller = self._find_controller('post_%s' % method, method)
@@ -180,14 +184,10 @@ class RestController(object):
             if controller:
                 return controller, remainder[:-1]
 
-        # check for custom GET requests
-        if method.upper() in self._custom_actions.get(method_name, []):
-            controller = self._find_controller(
-                'get_%s' % method_name,
-                method_name
-            )
-            if controller:
-                return controller, remainder[:-1]
+        match = self._handle_custom_action(method, remainder)
+        if match:
+            return match
+
         controller = getattr(self, remainder[0], None)
         if controller and not ismethod(controller):
             return lookup_controller(controller, remainder[1:])
@@ -204,6 +204,10 @@ class RestController(object):
         Routes ``DELETE`` actions to the appropriate controller.
         '''
         if remainder:
+            match = self._handle_custom_action(method, remainder)
+            if match:
+                return match
+
             controller = getattr(self, remainder[0], None)
             if controller and not ismethod(controller):
                 return lookup_controller(controller, remainder[1:])
@@ -230,14 +234,10 @@ class RestController(object):
         '''
         # check for custom POST/PUT requests
         if remainder:
-            method_name = remainder[-1]
-            if method.upper() in self._custom_actions.get(method_name, []):
-                controller = self._find_controller(
-                    '%s_%s' % (method, method_name),
-                    method_name
-                )
-                if controller:
-                    return controller, remainder[:-1]
+            match = self._handle_custom_action(method, remainder)
+            if match:
+                return match
+
             controller = getattr(self, remainder[0], None)
             if controller and not ismethod(controller):
                 return lookup_controller(controller, remainder[1:])
@@ -252,6 +252,25 @@ class RestController(object):
     def _handle_put(self, method, remainder):
         return self._handle_post(method, remainder)
 
+    def _handle_custom_action(self, method, remainder):
+        remainder = [r for r in remainder if r]
+        if remainder:
+            if method in ('put', 'delete'):
+                # For PUT and DELETE, additional arguments are supplied, e.g.,
+                # DELETE /foo/XYZ
+                method_name = remainder[0]
+                remainder = remainder[1:]
+            else:
+                method_name = remainder[-1]
+                remainder = remainder[:-1]
+            if method.upper() in self._custom_actions.get(method_name, []):
+                controller = self._find_controller(
+                    '%s_%s' % (method, method_name),
+                    method_name
+                )
+                if controller:
+                    return controller, remainder
+
     def _set_routing_args(self, args):
         '''
         Sets default routing arguments.
diff --git a/pecan/tests/test_rest.py b/pecan/tests/test_rest.py
index f53030a..6975fc2 100644
--- a/pecan/tests/test_rest.py
+++ b/pecan/tests/test_rest.py
@@ -670,6 +670,67 @@ class TestRestController(PecanTestCase):
             r = app.request('/things', method='RESET', status=404)
             assert r.status_int == 404
 
+    def test_custom_with_trailing_slash(self):
+
+        class CustomController(RestController):
+
+            _custom_actions = {
+                'detail': ['GET'],
+                'create': ['POST'],
+                'update': ['PUT'],
+                'remove': ['DELETE'],
+            }
+
+            @expose()
+            def detail(self):
+                return 'DETAIL'
+
+            @expose()
+            def create(self):
+                return 'CREATE'
+
+            @expose()
+            def update(self, id):
+                return id
+
+            @expose()
+            def remove(self, id):
+                return id
+
+        app = TestApp(make_app(CustomController()))
+
+        r = app.get('/detail')
+        assert r.status_int == 200
+        assert r.body == b_('DETAIL')
+
+        r = app.get('/detail/')
+        assert r.status_int == 200
+        assert r.body == b_('DETAIL')
+
+        r = app.post('/create')
+        assert r.status_int == 200
+        assert r.body == b_('CREATE')
+
+        r = app.post('/create/')
+        assert r.status_int == 200
+        assert r.body == b_('CREATE')
+
+        r = app.put('/update/123')
+        assert r.status_int == 200
+        assert r.body == b_('123')
+
+        r = app.put('/update/123/')
+        assert r.status_int == 200
+        assert r.body == b_('123')
+
+        r = app.delete('/remove/456')
+        assert r.status_int == 200
+        assert r.body == b_('456')
+
+        r = app.delete('/remove/456/')
+        assert r.status_int == 200
+        assert r.body == b_('456')
+
     def test_custom_delete(self):
 
         class OthersController(object):