Repository: falconry/falcon
Pull request #1540 (2019-05-08 16:28:34)

fix(custom-http-methods): Making sure we clean up the env var input string
Making sure we're strip and uppercase the custom methods specified from the environmental variable. Also, rewording docs a touch, and expanding the tests.

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

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

```diff
@@          Coverage Diff           @@
##           master   #1540   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          38      38           
  Lines        2567    2567           
  Branches      392     392           
======================================
  Hits         2567    2567
```


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

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1540?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/1540?src=pr&el=footer). Last update [7d6c06a...5092c29](https://codecov.io/gh/falconry/falcon/pull/1540?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 31159e7..7d7ca9c 100644
--- a/docs/api/routing.rst
+++ b/docs/api/routing.rst
@@ -317,24 +317,26 @@ be used by custom routing engines.
 Custom HTTP Methods
 -------------------
 
-Some applications need to support non-standard HTTP methods, such as FOO or BAR, 
-in addition to the standard HTTP methods like GET and PUT. To support custom
-HTTP methods, use one of the following methods:
+While not advised, some applications need to support non-standard HTTP methods,
+such as FOO or BAR, in addition to the standard HTTP methods like GET and PUT.
+To support custom HTTP methods, use one of the following methods:
 
-- define the FALCON_CUSTOM_HTTP_METHODS environment variable as a comma-delimited
-  list of custom methods, and make sure that environment variable is active in 
-  your API app's environment. For example::
+- Ideally, if you don't use hooks in your application, you can easily add the
+  custom methods in your application setup by overriding the value of
+  ``falcon.constants.COMBINED_METHODS``. For example::
 
-    $ export FALCON_CUSTOM_HTTP_METHODS=FOO,BAR
+    import falcon.constants
+    falcon.constants.COMBINED_METHODS += ['FOO', 'BAR']
 
-- in your app setup, override the value of ``falcon.constants.COMBINED_METHODS``
-  to include your custom methods. For example::
+- Due to the nature of hooks, if you do use them, you'll need to define the
+  FALCON_CUSTOM_HTTP_METHODS environment variable as a comma-delimited list
+  of custom methods. For example::
+
+    $ export FALCON_CUSTOM_HTTP_METHODS=FOO,BAR
 
-    import falcon.constants
-    falcon.constants += ['FOO', 'BAR']
 
-Once you have notified the framework that it should support these custom methods,
-you can define request methods like any other HTTP method, such as::
+Once you have used the appropriate method, your custom methods should be active.
+You then can define request methods like any other HTTP method, such as::
 
     def on_foo(self, req, resp):
         ...
diff --git a/falcon/constants.py b/falcon/constants.py
index 4ba0f40..f671f4d 100644
--- a/falcon/constants.py
+++ b/falcon/constants.py
@@ -26,9 +26,9 @@ WEBDAV_METHODS = [
 # if FALCON_CUSTOM_HTTP_METHODS is defined, treat it as a comma-
 # delimited string of additional supported methods in this env.
 FALCON_CUSTOM_HTTP_METHODS = [
-    method for method in
-    (os.environ.get('FALCON_CUSTOM_HTTP_METHODS') or '').split(',')
-    if method != ''
+    method.strip().upper()
+    for method in os.environ.get('FALCON_CUSTOM_HTTP_METHODS', '').split(',')
+    if method.strip() != ''
 ]
 
 COMBINED_METHODS = HTTP_METHODS + WEBDAV_METHODS + FALCON_CUSTOM_HTTP_METHODS
diff --git a/falcon/routing/util.py b/falcon/routing/util.py
index c9d244a..15a30e1 100644
--- a/falcon/routing/util.py
+++ b/falcon/routing/util.py
@@ -16,7 +16,7 @@
 
 import re
 
-from falcon import COMBINED_METHODS, responders
+from falcon import constants, responders
 
 
 class SuffixedMethodNotFoundError(Exception):
@@ -108,7 +108,7 @@ def map_http_methods(resource, suffix=None):
 
     method_map = {}
 
-    for method in COMBINED_METHODS:
+    for method in constants.COMBINED_METHODS:
         try:
             responder_name = 'on_' + method.lower()
             if suffix:
@@ -149,6 +149,6 @@ def set_default_responders(method_map):
 
     na_responder = responders.create_method_not_allowed(allowed_methods)
 
-    for method in COMBINED_METHODS:
+    for method in constants.COMBINED_METHODS:
         if method not in allowed_methods:
             method_map[method] = na_responder


Test patch:
diff --git a/tests/test_http_custom_method_routing.py b/tests/test_http_custom_method_routing.py
index 1a6f9ba..f8b9512 100644
--- a/tests/test_http_custom_method_routing.py
+++ b/tests/test_http_custom_method_routing.py
@@ -1,9 +1,17 @@
+import importlib
+import os
+
 import pytest
 
 import falcon
 from falcon import testing
 import falcon.constants
-from falcon.routing.util import map_http_methods
+from falcon.routing import util
+
+try:
+    import cython
+except ImportError:
+    cython = None
 
 FALCON_CUSTOM_HTTP_METHODS = ['FOO', 'BAR']
 
@@ -14,9 +22,25 @@ def resource_things():
 
 
 @pytest.fixture
-def client():
+def cleanup_constants():
+    # Forcing reload to make sure we used a module import and didn't import
+    # the list directly.
+    importlib.reload(falcon.constants)
+    yield
+    falcon.constants.COMBINED_METHODS = list(
+        set(falcon.constants.COMBINED_METHODS) - set(FALCON_CUSTOM_HTTP_METHODS)
+    )
+
+    if 'FALCON_CUSTOM_HTTP_METHODS' in os.environ:
+        del os.environ['FALCON_CUSTOM_HTTP_METHODS']
+
+
+@pytest.fixture
+def custom_http_client(cleanup_constants, resource_things):
+    falcon.constants.COMBINED_METHODS += FALCON_CUSTOM_HTTP_METHODS
+
     app = falcon.API()
-    app.add_route('/things', ThingsResource())
+    app.add_route('/things', resource_things)
     return testing.TestClient(app)
 
 
@@ -34,35 +58,51 @@ class ThingsResource:
         resp.status = falcon.HTTP_204
 
 
-def setup_module(module):
-    falcon.constants.COMBINED_METHODS += FALCON_CUSTOM_HTTP_METHODS
+def test_map_http_methods(custom_http_client, resource_things):
+    method_map = util.map_http_methods(resource_things)
 
+    assert 'FOO' in method_map
+    assert 'BAR' not in method_map
 
-def teardown_module(module):
-    for method in FALCON_CUSTOM_HTTP_METHODS:
-        index = falcon.constants.COMBINED_METHODS.index(method)
-        falcon.constants.COMBINED_METHODS.pop(index)
 
+@pytest.mark.skipif(cython, reason='Reloading modules on Cython does not work')
+@pytest.mark.parametrize('env_str,expected', [
+    ('foo', ['FOO']),
+    ('FOO', ['FOO']),
+    ('FOO,', ['FOO']),
+    ('FOO,BAR', ['FOO', 'BAR']),
+    ('FOO, BAR', ['FOO', 'BAR']),
+    (' foo , BAR ', ['FOO', 'BAR']),
+])
+def test_environment_override(cleanup_constants, resource_things, env_str, expected):
+    # Make sure we don't have anything in there
+    for method in expected:
+        assert method not in falcon.constants.COMBINED_METHODS
 
-def test_map_http_methods(resource_things, monkeypatch):
-    method_map = map_http_methods(resource_things)
-    assert 'FOO' in method_map
-    assert 'BAR' not in method_map
+    os.environ['FALCON_CUSTOM_HTTP_METHODS'] = env_str
+
+    # Reload module to pick up environment variable methods
+    importlib.reload(falcon.constants)
+
+    # Now it should be there
+    for method in expected:
+        assert method in falcon.constants.COMBINED_METHODS
+
+
+def test_foo(custom_http_client, resource_things):
+    """FOO is a supported method, so returns HTTP_204"""
+    custom_http_client.app.add_route('/things', resource_things)
+    response = custom_http_client.simulate_request(path='/things', method='FOO')
 
+    assert 'FOO' in falcon.constants.COMBINED_METHODS
+    assert response.status == falcon.HTTP_204
+    assert resource_things.called
 
-class TestHttpMethodRouting:
 
-    def test_foo(self, client, resource_things):
-        """FOO is a supported method, so returns HTTP_204"""
-        client.app.add_route('/things', resource_things)
-        response = client.simulate_request(path='/things', method='FOO')
-        assert 'FOO' in falcon.constants.COMBINED_METHODS
-        assert response.status == falcon.HTTP_204
-        assert resource_things.called
+def test_bar(custom_http_client, resource_things):
+    """BAR is not supported by ResourceThing"""
+    custom_http_client.app.add_route('/things', resource_things)
+    response = custom_http_client.simulate_request(path='/things', method='BAR')
 
-    def test_bar(self, client, resource_things):
-        """BAR is not supported by ResourceThing"""
-        client.app.add_route('/things', resource_things)
-        response = client.simulate_request(path='/things', method='BAR')
-        assert 'BAR' in falcon.constants.COMBINED_METHODS
-        assert response.status == falcon.HTTP_405
+    assert 'BAR' in falcon.constants.COMBINED_METHODS
+    assert response.status == falcon.HTTP_405