Repository: falconry/falcon
Pull request #1329 (2018-06-27 17:58:09)

feat(api): Facilitate the overriding of the default method mapping logic
This is a rebased version of #1252 plus some doc updates and method signature tweaks.

codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1329?src=pr&el=h1) Report
> Merging [#1329](https://codecov.io/gh/falconry/falcon/pull/1329?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/4afb296c34c68fae99854e2217b07021d60eec73?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1329/graphs/tree.svg?height=150&width=650&token=zOtkfNQ3h6&src=pr)](https://codecov.io/gh/falconry/falcon/pull/1329?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           master   #1329   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          38      38           
  Lines        2516    2521    +5     
  Branches      385     385           
======================================
+ Hits         2516    2521    +5
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1329?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/routing/compiled.py](https://codecov.io/gh/falconry/falcon/pull/1329/diff?src=pr&el=tree#diff-ZmFsY29uL3JvdXRpbmcvY29tcGlsZWQucHk=) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1329?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1329?src=pr&el=footer). Last update [4afb296...1459928](https://codecov.io/gh/falconry/falcon/pull/1329?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/docs/api/routing.rst b/docs/api/routing.rst
index d13cd39..1e000b5 100644
--- a/docs/api/routing.rst
+++ b/docs/api/routing.rst
@@ -239,23 +239,32 @@ A custom router is any class that implements the following interface:
 .. code:: python
 
     class MyRouter(object):
-        def add_route(self, uri_template, method_map, resource, **kwargs):
+        def add_route(self, uri_template, resource, **kwargs):
             """Adds a route between URI path template and resource.
 
             Args:
-                uri_template (str): The URI template to add.
-                method_map (dict): A method map obtained by calling
-                    falcon.routing.create_http_method_map.
-                resource (object): Instance of the resource class that
-                    will handle requests for the given URI.
-
-            Keyword Arguments:
-
-                **kwargs (dict): Accepts any additional keyword arguments that
-                    were originally passed to the falcon.API.add_route() method.
-                    These arguments MUST be accepted via the double-star
-                    variadic pattern (**kwargs), and ignore any unrecognized
-                    or unsupported arguments.
+                uri_template (str): A URI template to use for the route
+                resource (object): The resource instance to associate with
+                    the URI template.
+
+            Keyword Args:
+                suffix (str): Optional responder name suffix for this
+                    route. If a suffix is provided, Falcon will map GET
+                    requests to ``on_get_{suffix}()``, POST requests to
+                    ``on_post_{suffix}()``, etc. In this way, multiple
+                    closely-related routes can be mapped to the same
+                    resource. For example, a single resource class can
+                    use suffixed responders to distinguish requests for
+                    a single item vs. a collection of those same items.
+                    Another class might use a suffixed responder to handle
+                    a shortlink route in addition to the regular route for
+                    the resource.
+
+                **kwargs (dict): Accepts any additional keyword arguments
+                    that were originally passed to the falcon.API.add_route()
+                    method. These arguments MUST be accepted via the
+                    double-star variadic pattern (**kwargs), and ignore any
+                    unrecognized or unsupported arguments.
             """
 
         def find(self, uri, req=None):
@@ -285,6 +294,13 @@ A custom router is any class that implements the following interface:
 
             """
 
+Default Router
+--------------
+
+.. autoclass:: falcon.routing.CompiledRouter
+    :members:
+
+
 Routing Utilities
 -----------------
 
diff --git a/docs/changes/0.3.0.rst b/docs/changes/0.3.0.rst
index edfa6f1..2a53ecc 100644
--- a/docs/changes/0.3.0.rst
+++ b/docs/changes/0.3.0.rst
@@ -5,6 +5,10 @@ Breaking Changes
 ----------------
 - Date headers are now returned as :py:class:`datetime.datetime` objects
   instead of strings.
+- The expected signature for the ``add_route()`` method of custom routers no
+  longer includes a `method_map` parameter. Custom routers should, instead,
+  call the :py:meth:`falcon.routing.util.map_http_methods` function directly
+  from their ``add_route()`` method if they require this mapping.
 
 New & Improved
 --------------
diff --git a/falcon/api.py b/falcon/api.py
index 3979748..7fdd650 100644
--- a/falcon/api.py
+++ b/falcon/api.py
@@ -301,7 +301,7 @@ class API(object):
     def router_options(self):
         return self._router.options
 
-    def add_route(self, uri_template, resource, suffix=None, **kwargs):
+    def add_route(self, uri_template, resource, **kwargs):
         """Associate a templatized URI path with a resource.
 
         Falcon routes incoming requests to resources based on a set of
@@ -313,6 +313,10 @@ class API(object):
         default responder that simply raises an instance of
         :class:`~.HTTPNotFound`.
 
+        This method delegates to the configured router's ``add_route()``
+        method. To override the default behavior, pass a custom router
+        object to the :class:`~.API` initializer.
+
         (See also: :ref:`Routing <routing>`)
 
         Args:
@@ -362,9 +366,7 @@ class API(object):
         if '//' in uri_template:
             raise ValueError("uri_template may not contain '//'")
 
-        method_map = routing.map_http_methods(resource, suffix=suffix)
-        routing.set_default_responders(method_map)
-        self._router.add_route(uri_template, method_map, resource, **kwargs)
+        self._router.add_route(uri_template, resource, **kwargs)
 
     def add_static_route(self, prefix, directory, downloadable=False, fallback_filename=None):
         """Add a route to a directory of static files.
diff --git a/falcon/routing/compiled.py b/falcon/routing/compiled.py
index 4ac6622..da61c54 100644
--- a/falcon/routing/compiled.py
+++ b/falcon/routing/compiled.py
@@ -21,6 +21,7 @@ import textwrap
 from six.moves import UserDict
 
 from falcon.routing import converters
+from falcon.routing.util import map_http_methods, set_default_responders
 
 
 _TAB_STR = ' ' * 4
@@ -89,17 +90,61 @@ class CompiledRouter(object):
     def finder_src(self):
         return self._finder_src
 
-    def add_route(self, uri_template, method_map, resource, **kwargs):
+    def map_http_methods(self, resource, **kwargs):
+        """Map HTTP methods (e.g., GET, POST) to methods of a resource object.
+
+        This method is called from :meth:`~.add_route` and may be overridden to
+        provide a custom mapping strategy.
+
+        Args:
+            resource (instance): Object which represents a REST resource.
+                The default maps the HTTP method ``GET`` to ``on_get()``,
+                ``POST`` to ``on_post()``, etc. If any HTTP methods are not
+                supported by your resource, simply don't define the
+                corresponding request handlers, and Falcon will do the right
+                thing.
+
+        Keyword Args:
+            suffix (str): Optional responder name suffix for this route. If
+                a suffix is provided, Falcon will map GET requests to
+                ``on_get_{suffix}()``, POST requests to ``on_post_{suffix}()``,
+                etc. In this way, multiple closely-related routes can be
+                mapped to the same resource. For example, a single resource
+                class can use suffixed responders to distinguish requests
+                for a single item vs. a collection of those same items.
+                Another class might use a suffixed responder to handle
+                a shortlink route in addition to the regular route for the
+                resource.
+        """
+
+        return map_http_methods(resource, suffix=kwargs.get('suffix', None))
+
+    def add_route(self, uri_template, resource, **kwargs):
         """Adds a route between a URI path template and a resource.
 
+        This method may be overridden to customize how a route is added.
+
         Args:
             uri_template (str): A URI template to use for the route
-            method_map (dict): A mapping of HTTP methods (e.g., 'GET',
-                'POST') to methods of a resource object.
             resource (object): The resource instance to associate with
                 the URI template.
+
+        Keyword Args:
+            suffix (str): Optional responder name suffix for this route. If
+                a suffix is provided, Falcon will map GET requests to
+                ``on_get_{suffix}()``, POST requests to ``on_post_{suffix}()``,
+                etc. In this way, multiple closely-related routes can be
+                mapped to the same resource. For example, a single resource
+                class can use suffixed responders to distinguish requests
+                for a single item vs. a collection of those same items.
+                Another class might use a suffixed responder to handle
+                a shortlink route in addition to the regular route for the
+                resource.
         """
 
+        method_map = self.map_http_methods(resource, **kwargs)
+        set_default_responders(method_map)
+
         # NOTE(kgriffs): Fields may have whitespace in them, so sub
         # those before checking the rest of the URI template.
         if re.search(r'\s', _FIELD_PATTERN.sub('{FIELD}', uri_template)):


Test patch:
diff --git a/tests/test_custom_router.py b/tests/test_custom_router.py
index a0ddea7..3f6db6e 100644
--- a/tests/test_custom_router.py
+++ b/tests/test_custom_router.py
@@ -72,7 +72,7 @@ def test_can_pass_additional_params_to_add_route():
     check = []
 
     class CustomRouter(object):
-        def add_route(self, uri_template, method_map, resource, **kwargs):
+        def add_route(self, uri_template, resource, **kwargs):
             name = kwargs['name']
             self._index = {name: uri_template}
             check.append(name)
diff --git a/tests/test_default_router.py b/tests/test_default_router.py
index 4ce72df..9170ac3 100644
--- a/tests/test_default_router.py
+++ b/tests/test_default_router.py
@@ -17,99 +17,95 @@ def router():
     router = DefaultRouter()
 
     router.add_route(
-        '/repos', {}, ResourceWithId(1))
+        '/repos', ResourceWithId(1))
     router.add_route(
-        '/repos/{org}', {}, ResourceWithId(2))
+        '/repos/{org}', ResourceWithId(2))
     router.add_route(
-        '/repos/{org}/{repo}', {}, ResourceWithId(3))
+        '/repos/{org}/{repo}', ResourceWithId(3))
     router.add_route(
-        '/repos/{org}/{repo}/commits', {}, ResourceWithId(4))
+        '/repos/{org}/{repo}/commits', ResourceWithId(4))
     router.add_route(
         u'/repos/{org}/{repo}/compare/{usr0}:{branch0}...{usr1}:{branch1}',
-        {}, ResourceWithId(5))
+        ResourceWithId(5))
 
     router.add_route(
-        '/teams/{id}', {}, ResourceWithId(6))
+        '/teams/{id}', ResourceWithId(6))
     router.add_route(
-        '/teams/{id}/members', {}, ResourceWithId(7))
+        '/teams/{id}/members', ResourceWithId(7))
 
     router.add_route(
-        '/teams/default', {}, ResourceWithId(19))
+        '/teams/default', ResourceWithId(19))
     router.add_route(
-        '/teams/default/members/thing', {}, ResourceWithId(19))
+        '/teams/default/members/thing', ResourceWithId(19))
 
     router.add_route(
-        '/user/memberships', {}, ResourceWithId(8))
+        '/user/memberships', ResourceWithId(8))
     router.add_route(
-        '/emojis', {}, ResourceWithId(9))
+        '/emojis', ResourceWithId(9))
     router.add_route(
         '/repos/{org}/{repo}/compare/{usr0}:{branch0}...{usr1}:{branch1}/full',
-        {}, ResourceWithId(10))
+        ResourceWithId(10))
     router.add_route(
-        '/repos/{org}/{repo}/compare/all', {}, ResourceWithId(11))
+        '/repos/{org}/{repo}/compare/all', ResourceWithId(11))
 
     # NOTE(kgriffs): The ordering of these calls is significant; we
     # need to test that the {id} field does not match the other routes,
     # regardless of the order they are added.
     router.add_route(
-        '/emojis/signs/0', {}, ResourceWithId(12))
+        '/emojis/signs/0', ResourceWithId(12))
     router.add_route(
-        '/emojis/signs/{id}', {}, ResourceWithId(13))
+        '/emojis/signs/{id}', ResourceWithId(13))
     router.add_route(
-        '/emojis/signs/42', {}, ResourceWithId(14))
+        '/emojis/signs/42', ResourceWithId(14))
     router.add_route(
-        '/emojis/signs/42/small.jpg', {}, ResourceWithId(23))
+        '/emojis/signs/42/small.jpg', ResourceWithId(23))
     router.add_route(
-        '/emojis/signs/78/small.png', {}, ResourceWithId(24))
+        '/emojis/signs/78/small.png', ResourceWithId(24))
 
     # Test some more special chars
     router.add_route(
-        '/emojis/signs/78/small(png)', {}, ResourceWithId(25))
+        '/emojis/signs/78/small(png)', ResourceWithId(25))
     router.add_route(
-        '/emojis/signs/78/small_png', {}, ResourceWithId(26))
-    router.add_route('/images/{id}.gif', {}, ResourceWithId(27))
+        '/emojis/signs/78/small_png', ResourceWithId(26))
+    router.add_route('/images/{id}.gif', ResourceWithId(27))
 
     router.add_route(
         '/repos/{org}/{repo}/compare/{usr0}:{branch0}...{usr1}:{branch1}/part',
-        {}, ResourceWithId(15))
+        ResourceWithId(15))
     router.add_route(
-        '/repos/{org}/{repo}/compare/{usr0}:{branch0}',
-        {}, ResourceWithId(16))
+        '/repos/{org}/{repo}/compare/{usr0}:{branch0}', ResourceWithId(16))
     router.add_route(
-        '/repos/{org}/{repo}/compare/{usr0}:{branch0}/full',
-        {}, ResourceWithId(17))
+        '/repos/{org}/{repo}/compare/{usr0}:{branch0}/full', ResourceWithId(17))
 
     router.add_route(
-        '/gists/{id}/{representation}', {}, ResourceWithId(21))
+        '/gists/{id}/{representation}', ResourceWithId(21))
     router.add_route(
-        '/gists/{id}/raw', {}, ResourceWithId(18))
+        '/gists/{id}/raw', ResourceWithId(18))
     router.add_route(
-        '/gists/first', {}, ResourceWithId(20))
+        '/gists/first', ResourceWithId(20))
 
-    router.add_route('/item/{q}', {}, ResourceWithId(28))
+    router.add_route('/item/{q}', ResourceWithId(28))
 
     # ----------------------------------------------------------------
     # Routes with field converters
     # ----------------------------------------------------------------
 
     router.add_route(
-        '/cvt/teams/{id:int(min=7)}', {}, ResourceWithId(29))
+        '/cvt/teams/{id:int(min=7)}', ResourceWithId(29))
     router.add_route(
-        '/cvt/teams/{id:int(min=7)}/members', {}, ResourceWithId(30))
+        '/cvt/teams/{id:int(min=7)}/members', ResourceWithId(30))
     router.add_route(
-        '/cvt/teams/default', {}, ResourceWithId(31))
+        '/cvt/teams/default', ResourceWithId(31))
     router.add_route(
-        '/cvt/teams/default/members/{id:int}-{tenure:int}', {}, ResourceWithId(32))
+        '/cvt/teams/default/members/{id:int}-{tenure:int}', ResourceWithId(32))
 
     router.add_route(
         '/cvt/repos/{org}/{repo}/compare/{usr0}:{branch0:int}...{usr1}:{branch1:int}/part',
-        {}, ResourceWithId(33))
+        ResourceWithId(33))
     router.add_route(
-        '/cvt/repos/{org}/{repo}/compare/{usr0}:{branch0:int}',
-        {}, ResourceWithId(34))
+        '/cvt/repos/{org}/{repo}/compare/{usr0}:{branch0:int}', ResourceWithId(34))
     router.add_route(
-        '/cvt/repos/{org}/{repo}/compare/{usr0}:{branch0:int}/full',
-        {}, ResourceWithId(35))
+        '/cvt/repos/{org}/{repo}/compare/{usr0}:{branch0:int}/full', ResourceWithId(35))
 
     return router
 
@@ -145,12 +141,12 @@ class SpamConverter(object):
 
 def test_user_regression_versioned_url():
     router = DefaultRouter()
-    router.add_route('/{version}/messages', {}, ResourceWithId(2))
+    router.add_route('/{version}/messages', ResourceWithId(2))
 
     resource, __, __, __ = router.find('/v2/messages')
     assert resource.resource_id == 2
 
-    router.add_route('/v2', {}, ResourceWithId(1))
+    router.add_route('/v2', ResourceWithId(1))
 
     resource, __, __, __ = router.find('/v2')
     assert resource.resource_id == 1
@@ -169,12 +165,10 @@ def test_user_regression_recipes():
     router = DefaultRouter()
     router.add_route(
         '/recipes/{activity}/{type_id}',
-        {},
         ResourceWithId(1)
     )
     router.add_route(
         '/recipes/baking',
-        {},
         ResourceWithId(2)
     )
 
@@ -235,7 +229,7 @@ def test_user_regression_recipes():
 def test_user_regression_special_chars(uri_template, path, expected_params):
     router = DefaultRouter()
 
-    router.add_route(uri_template, {}, ResourceWithId(1))
+    router.add_route(uri_template, ResourceWithId(1))
 
     route = router.find(path)
     assert route is not None
@@ -263,7 +257,7 @@ def test_not_str(uri_template):
 
 def test_root_path():
     router = DefaultRouter()
-    router.add_route('/', {}, ResourceWithId(42))
+    router.add_route('/', ResourceWithId(42))
 
     resource, __, __, __ = router.find('/')
     assert resource.resource_id == 42
@@ -292,7 +286,7 @@ def test_root_path():
 def test_duplicate_field_names(uri_template):
     router = DefaultRouter()
     with pytest.raises(ValueError):
-        router.add_route(uri_template, {}, ResourceWithId(1))
+        router.add_route(uri_template, ResourceWithId(1))
 
 
 @pytest.mark.parametrize('uri_template,path', [
@@ -304,7 +298,7 @@ def test_duplicate_field_names(uri_template):
 def test_match_entire_path(uri_template, path):
     router = DefaultRouter()
 
-    router.add_route(uri_template, {}, ResourceWithId(1))
+    router.add_route(uri_template, ResourceWithId(1))
 
     route = router.find(path)
     assert route is None
@@ -318,7 +312,7 @@ def test_match_entire_path(uri_template, path):
 ])
 def test_conflict(router, uri_template):
     with pytest.raises(ValueError):
-        router.add_route(uri_template, {}, ResourceWithId(-1))
+        router.add_route(uri_template, ResourceWithId(-1))
 
 
 @pytest.mark.parametrize('uri_template', [
@@ -327,7 +321,7 @@ def test_conflict(router, uri_template):
     '/repos/{org}/{repo}/compare/{complex}:{vs}...{complex2}/full',
 ])
 def test_non_conflict(router, uri_template):
-    router.add_route(uri_template, {}, ResourceWithId(-1))
+    router.add_route(uri_template, ResourceWithId(-1))
 
 
 @pytest.mark.parametrize('uri_template', [
@@ -365,7 +359,7 @@ def test_non_conflict(router, uri_template):
 ])
 def test_invalid_field_name(router, uri_template):
     with pytest.raises(ValueError):
-        router.add_route(uri_template, {}, ResourceWithId(-1))
+        router.add_route(uri_template, ResourceWithId(-1))
 
 
 def test_print_src(router):
@@ -379,7 +373,7 @@ def test_print_src(router):
 
 
 def test_override(router):
-    router.add_route('/emojis/signs/0', {}, ResourceWithId(-1))
+    router.add_route('/emojis/signs/0', ResourceWithId(-1))
 
     resource, __, __, __ = router.find('/emojis/signs/0')
     assert resource.resource_id == -1
@@ -467,7 +461,7 @@ def test_converters_with_invalid_options(router, uri_template):
     # when calling add_route(). Additional checks can be found
     # in test_uri_converters.py
     with pytest.raises(ValueError):
-        router.add_route(uri_template, {}, ResourceWithId(1))
+        router.add_route(uri_template, ResourceWithId(1))
 
 
 @pytest.mark.parametrize('uri_template', [
@@ -476,7 +470,7 @@ def test_converters_with_invalid_options(router, uri_template):
 ])
 def test_converters_malformed_specification(router, uri_template):
     with pytest.raises(ValueError):
-        router.add_route(uri_template, {}, ResourceWithId(1))
+        router.add_route(uri_template, ResourceWithId(1))
 
 
 def test_variable(router):
@@ -613,7 +607,7 @@ def test_complex_alt(router, url_postfix, resource_id, expected_template):
 def test_options_converters_set(router):
     router.options.converters['spam'] = SpamConverter
 
-    router.add_route('/{food:spam(3, eggs=True)}', {}, ResourceWithId(1))
+    router.add_route('/{food:spam(3, eggs=True)}', ResourceWithId(1))
     resource, __, params, __ = router.find('/spam')
 
     assert params == {'food': 'spam&eggs, spam&eggs, spam&eggs'}
@@ -630,7 +624,7 @@ def test_options_converters_update(router, converter_name):
     })
 
     template = '/{food:' + converter_name + '(3, eggs=True)}'
-    router.add_route(template, {}, ResourceWithId(1))
+    router.add_route(template, ResourceWithId(1))
     resource, __, params, __ = router.find('/spam')
 
     assert params == {'food': 'spam&eggs, spam&eggs, spam&eggs'}