Repository: falconry/falcon
Pull request #1316 (2018-05-23 07:08:20)

Changed the default value of RequestOptions.keep_blank_qs_values to True
#1274 

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

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

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


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

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1316?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/1316?src=pr&el=footer). Last update [a560b2a...066fb93](https://codecov.io/gh/falconry/falcon/pull/1316?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index b333186..d3616c2 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -6,6 +6,14 @@ Breaking Changes
 
 - ``testing.Result.json`` now returns ``None`` when the response body is
   empty, rather than raising an error.
+- ``Request.get_param_as_bool()`` now defaults to treating valueless
+  parameters as truthy, rather than falsy. ``None`` is still returned
+  by default when the parameter is altogether missing.
+- ``Request.get_param_as_bool()`` no longer raises an error for a
+  valueless parameter when the ``blank_as_true`` keyword argument is ``False``.
+  Instead, ``False`` is simply returned in that case.
+- ``RequestOptions.keep_blank_qs_values`` now defaults to ``True`` instead of
+  ``False``.
 
 Changes to Supported Platforms
 ------------------------------
diff --git a/docs/changes/2.0.0.rst b/docs/changes/2.0.0.rst
index 42cb4fc..d8bed05 100644
--- a/docs/changes/2.0.0.rst
+++ b/docs/changes/2.0.0.rst
@@ -6,6 +6,14 @@ Breaking Changes
 
 - :attr:`falcon.testing.Result.json` now returns None when the response body is
   empty, rather than raising an error.
+- :meth:`~.Request.get_param_as_bool` now defaults to treating valueless
+  parameters as truthy, rather than falsy. ``None`` is still returned
+  by default when the parameter is altogether missing.
+- :meth:`~.Request.get_param_as_bool` no longer raises an error for a
+  valueless parameter when the ``blank_as_true`` keyword argument is ``False``.
+  Instead, ``False`` is simply returned in that case.
+- :attr:`~.RequestOptions.keep_blank_qs_values` now defaults to ``True``
+  instead of ``False``.
 
 Changes to Supported Platforms
 ------------------------------
diff --git a/falcon/request.py b/falcon/request.py
index b26cb46..40d0947 100644
--- a/falcon/request.py
+++ b/falcon/request.py
@@ -1367,8 +1367,14 @@ class Request(object):
         raise errors.HTTPMissingParam(name)
 
     def get_param_as_bool(self, name, required=False, store=None,
-                          blank_as_true=False, default=None):
-        """Return the value of a query string parameter as a boolean
+                          blank_as_true=True, default=None):
+        """Return the value of a query string parameter as a boolean.
+
+        This method treats valueless parameters as flags. By default, if no
+        value is provided for the parameter in the query string, ``True`` is
+        assumed and returned. If the parameter is missing altogether, ``None``
+        is returned as with other ``get_param_*()`` methods, which can be
+        easily treated as falsy by the caller as needed.
 
         The following boolean strings are supported::
 
@@ -1386,19 +1392,19 @@ class Request(object):
             store (dict): A ``dict``-like object in which to place
                 the value of the param, but only if the param is found (default
                 ``None``).
-            blank_as_true (bool): If ``True``, an empty string value will be
-                treated as ``True`` (default ``False``). Normally empty strings
-                are ignored; if you would like to recognize such parameters, you
-                must set the `keep_blank_qs_values` request option to ``True``.
-                Request options are set globally for each instance of
-                ``falcon.API`` through the `req_options` attribute.
-            default (any): If the param is not found returns the
-                given value instead of ``None``
+            blank_as_true (bool): Valueless query string parameters
+                are treated as flags, resulting in ``True`` being
+                returned when such a parameter is present, and ``False``
+                otherwise. To require the client to explicitly opt-in to a
+                truthy value, pass ``blank_as_true=False`` to return ``False``
+                when a value is not specified in the query string.
+            default (any): If the param is not found, return this
+                value instead of ``None``.
 
         Returns:
             bool: The value of the param if it is found and can be converted
             to a ``bool``. If the param is not found, returns ``None``
-            unless required is ``True``.
+            unless `required` is ``True``.
 
         Raises:
             HTTPBadRequest: A required param is missing from the request, or
@@ -1419,8 +1425,8 @@ class Request(object):
                 val = True
             elif val in FALSE_STRINGS:
                 val = False
-            elif blank_as_true and not val:
-                val = True
+            elif not val:
+                val = blank_as_true
             else:
                 msg = 'The value of the parameter must be "true" or "false".'
                 raise errors.HTTPInvalidParam(msg, name)
@@ -1743,8 +1749,8 @@ class RequestOptions(object):
     configuring certain :py:class:`~.Request` behaviors.
 
     Attributes:
-        keep_blank_qs_values (bool): Set to ``True`` to keep query string
-            fields even if they do not have a value (default ``False``).
+        keep_blank_qs_values (bool): Set to ``False`` to ignore query string
+            params that have missing or blank values (default ``True``).
             For comma-separated values, this option also determines
             whether or not empty elements in the parsed list are
             retained.
@@ -1813,7 +1819,7 @@ class RequestOptions(object):
     )
 
     def __init__(self):
-        self.keep_blank_qs_values = False
+        self.keep_blank_qs_values = True
         self.auto_parse_form_urlencoded = False
         self.auto_parse_qs_csv = True
         self.strip_url_path_trailing_slash = False


Test patch:
diff --git a/tests/test_middleware.py b/tests/test_middleware.py
index cb6eab0..e41380c 100644
--- a/tests/test_middleware.py
+++ b/tests/test_middleware.py
@@ -25,6 +25,12 @@ class CaptureResponseMiddleware(object):
         self.req_succeeded = req_succeeded
 
 
+class CaptureRequestMiddleware(object):
+
+    def process_request(self, req, resp):
+        self.req = req
+
+
 class RequestTimeMiddleware(object):
 
     def process_request(self, req, resp):
@@ -209,10 +215,20 @@ class TestTransactionIdMiddleware(TestMiddleware):
 
 
 class TestSeveralMiddlewares(TestMiddleware):
-    def test_generate_trans_id_and_time_with_request(self):
+    @pytest.mark.parametrize('independent_middleware', [True, False])
+    def test_generate_trans_id_and_time_with_request(self, independent_middleware):
+        # NOTE(kgriffs): We test both so that we can cover the code paths
+        # where only a single middleware method is implemented by a
+        # component.
+        creq = CaptureRequestMiddleware()
+        cresp = CaptureResponseMiddleware()
+
         global context
-        app = falcon.API(middleware=[TransactionIdMiddleware(),
-                                     RequestTimeMiddleware()])
+        app = falcon.API(independent_middleware=independent_middleware,
+                         middleware=[TransactionIdMiddleware(),
+                                     RequestTimeMiddleware(),
+                                     creq,
+                                     cresp])
 
         app.add_route(TEST_ROUTE, MiddlewareClassResource())
         client = testing.TestClient(app)
diff --git a/tests/test_options.py b/tests/test_options.py
index d8fdc0d..32b5257 100644
--- a/tests/test_options.py
+++ b/tests/test_options.py
@@ -8,7 +8,7 @@ class TestRequestOptions(object):
     def test_option_defaults(self):
         options = RequestOptions()
 
-        assert not options.keep_blank_qs_values
+        assert options.keep_blank_qs_values
         assert not options.auto_parse_form_urlencoded
         assert options.auto_parse_qs_csv
         assert not options.strip_url_path_trailing_slash
diff --git a/tests/test_query_params.py b/tests/test_query_params.py
index dc70160..8555c8f 100644
--- a/tests/test_query_params.py
+++ b/tests/test_query_params.py
@@ -111,6 +111,7 @@ class TestQueryParams(object):
     def test_blank(self, simulate_request, client, resource):
         query_string = 'marker='
         client.app.add_route('/', resource)
+        client.app.req_options.keep_blank_qs_values = False
         simulate_request(client=client, path='/', query_string=query_string)
 
         req = resource.captured_req
@@ -163,6 +164,7 @@ class TestQueryParams(object):
     def test_option_auto_parse_qs_csv_simple_true(self, simulate_request, client, resource):
         client.app.add_route('/', resource)
         client.app.req_options.auto_parse_qs_csv = True
+        client.app.req_options.keep_blank_qs_values = False
 
         query_string = 'id=23,42,,&id=2'
         simulate_request(client=client, path='/', query_string=query_string)
@@ -176,6 +178,7 @@ class TestQueryParams(object):
     def test_option_auto_parse_qs_csv_complex_false(self, simulate_request, client, resource):
         client.app.add_route('/', resource)
         client.app.req_options.auto_parse_qs_csv = False
+        client.app.req_options.keep_blank_qs_values = False
 
         encoded_json = '%7B%22msg%22:%22Testing%201,2,3...%22,%22code%22:857%7D'
         decoded_json = '{"msg":"Testing 1,2,3...","code":857}'
@@ -217,6 +220,7 @@ class TestQueryParams(object):
 
     def test_allowed_names(self, simulate_request, client, resource):
         client.app.add_route('/', resource)
+        client.app.req_options.keep_blank_qs_values = False
         query_string = ('p=0&p1=23&2p=foo&some-thing=that&blank=&'
                         'some_thing=x&-bogus=foo&more.things=blah&'
                         '_thing=42&_charset_=utf-8')
@@ -437,6 +441,7 @@ class TestQueryParams(object):
 
     def test_boolean(self, simulate_request, client, resource):
         client.app.add_route('/', resource)
+        client.app.req_options.keep_blank_qs_values = False
         query_string = ('echo=true&doit=false&bogus=bar&bogus2=foo&'
                         't1=True&f1=False&t2=yes&f2=no&blank&one=1&zero=0&'
                         'checkbox1=on&checkbox2=off')
@@ -476,22 +481,25 @@ class TestQueryParams(object):
 
     def test_boolean_blank(self, simulate_request, client, resource):
         client.app.add_route('/', resource)
-        client.app.req_options.keep_blank_qs_values = True
         simulate_request(client=client, path='/', query_string='blank&blank2=')
 
         req = resource.captured_req
         assert req.get_param('blank') == ''
         assert req.get_param('blank2') == ''
-        with pytest.raises(falcon.HTTPInvalidParam):
-            req.get_param_as_bool('blank')
 
-        with pytest.raises(falcon.HTTPInvalidParam):
-            req.get_param_as_bool('blank2')
-        assert req.get_param_as_bool('blank', blank_as_true=True)
-        assert req.get_param_as_bool('blank3', blank_as_true=True) is None
+        for param_name in ('blank', 'blank2'):
+            assert req.get_param_as_bool(param_name) is True
+            assert req.get_param_as_bool(param_name, blank_as_true=True) is True
+            assert req.get_param_as_bool(param_name, blank_as_true=False) is False
+
+        assert req.get_param_as_bool('nichts') is None
+        assert req.get_param_as_bool('nichts', default=None) is None
+        assert req.get_param_as_bool('nichts', default=False) is False
+        assert req.get_param_as_bool('nichts', default=True) is True
 
     def test_list_type(self, simulate_request, client, resource):
         client.app.add_route('/', resource)
+        client.app.req_options.keep_blank_qs_values = False
         query_string = ('colors=red,green,blue&limit=1'
                         '&list-ish1=f,,x&list-ish2=,0&list-ish3=a,,,b'
                         '&empty1=&empty2=,&empty3=,,'
@@ -581,6 +589,7 @@ class TestQueryParams(object):
 
     def test_list_transformer(self, simulate_request, client, resource):
         client.app.add_route('/', resource)
+        client.app.req_options.keep_blank_qs_values = False
         query_string = 'coord=1.4,13,15.1&limit=100&things=4,,1'
         simulate_request(client=client, path='/', query_string=query_string)