Repository: falconry/falcon

Issue:
Routes added to add_sink override static routes added with add_static_route
Hi Falcon team,

Thanks for all your hard work. I ran into an issue today when I tried to make use of the `add_static_route` method. 

Our project uses `add_sink` to implement a custom handler for requests with no corresponding handler. However, that `add_sink` function would gobble up any requests that we intended to be handled by the static route:

```py
api.add_static_route('/static', valid_static_file_path) # Would never get called
api.add_sink(unmatched_route_handler) # Would always swallow calls to assets at `/static/WHATEVER`
```
The case was the same whether the static route was added before or after the sink.

I would expect that if a static route is added, that resource path is excluded for any sink handlers.

My workaround for now was to basically use a regex on the sink to exclude the static path:
```
api.add_sink(unmatched_route_auth_wrapper, '^((?!static).)*$')
```


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1769?src=pr&el=h1) Report
> Merging [#1769](https://codecov.io/gh/falconry/falcon/pull/1769?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/e7c139a3ebfa60652c64b7363636146d605a8803?el=desc) will **decrease** coverage by `12.78%`.
> The diff coverage is `100.00%`.

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

```diff
@@             Coverage Diff              @@
##            master    #1769       +/-   ##
============================================
- Coverage   100.00%   87.21%   -12.79%     
============================================
  Files           52       47        -5     
  Lines         4295     3472      -823     
  Branches       690      517      -173     
============================================
- Hits          4295     3028     -1267     
- Misses           0      435      +435     
- Partials         0        9        +9     
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1769?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/inspect.py](https://codecov.io/gh/falconry/falcon/pull/1769/diff?src=pr&el=tree#diff-ZmFsY29uL2luc3BlY3QucHk=) | `100.00% <100.00%> (ø)` | |
| [falcon/asgi/response.py](https://codecov.io/gh/falconry/falcon/pull/1769/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVzcG9uc2UucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/\_request\_helpers.py](https://codecov.io/gh/falconry/falcon/pull/1769/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvX3JlcXVlc3RfaGVscGVycy5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/testing/helpers.py](https://codecov.io/gh/falconry/falcon/pull/1769/diff?src=pr&el=tree#diff-ZmFsY29uL3Rlc3RpbmcvaGVscGVycy5weQ==) | `47.75% <0.00%> (-52.25%)` | :arrow_down: |
| [falcon/responders.py](https://codecov.io/gh/falconry/falcon/pull/1769/diff?src=pr&el=tree#diff-ZmFsY29uL3Jlc3BvbmRlcnMucHk=) | `64.70% <0.00%> (-35.30%)` | :arrow_down: |
| [falcon/testing/client.py](https://codecov.io/gh/falconry/falcon/pull/1769/diff?src=pr&el=tree#diff-ZmFsY29uL3Rlc3RpbmcvY2xpZW50LnB5) | `66.34% <0.00%> (-33.66%)` | :arrow_down: |
| [falcon/media/json.py](https://codecov.io/gh/falconry/falcon/pull/1769/diff?src=pr&el=tree#diff-ZmFsY29uL21lZGlhL2pzb24ucHk=) | `68.96% <0.00%> (-31.04%)` | :arrow_down: |
| [falcon/media/validators/jsonschema.py](https://codecov.io/gh/falconry/falcon/pull/1769/diff?src=pr&el=tree#diff-ZmFsY29uL21lZGlhL3ZhbGlkYXRvcnMvanNvbnNjaGVtYS5weQ==) | `69.04% <0.00%> (-30.96%)` | :arrow_down: |
| [falcon/media/msgpack.py](https://codecov.io/gh/falconry/falcon/pull/1769/diff?src=pr&el=tree#diff-ZmFsY29uL21lZGlhL21zZ3BhY2sucHk=) | `73.91% <0.00%> (-26.09%)` | :arrow_down: |
| [falcon/util/sync.py](https://codecov.io/gh/falconry/falcon/pull/1769/diff?src=pr&el=tree#diff-ZmFsY29uL3V0aWwvc3luYy5weQ==) | `76.19% <0.00%> (-23.81%)` | :arrow_down: |
| ... and [23 more](https://codecov.io/gh/falconry/falcon/pull/1769/diff?src=pr&el=tree-more) | |

------

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

CaselIT: @kgriffs I've updated the docs 
vytas7: Another round of cosmetic nitpicks. This otherwise LGTM code wise :+1: 
CaselIT: Regarding issue 1789 I think it's best to address it after this is merged in another pr
CaselIT: Thanks!

Patch:
diff --git a/docs/_newsfragments/1372.newandimproved.rst b/docs/_newsfragments/1372.newandimproved.rst
new file mode 100644
index 0000000..4ab8999
--- /dev/null
+++ b/docs/_newsfragments/1372.newandimproved.rst
@@ -0,0 +1,4 @@
+The :class:`falcon.App` class initializer now supports a new argument
+``sink_before_static_route`` (default ``True``, maintaining 2.0 behavior) to
+specify if :meth:`sinks <falcon.App.add_sink>` should be handled before or
+after :meth:`static routes <falcon.App.add_static_route>`.
diff --git a/falcon/app.py b/falcon/app.py
index ac09dd7..351aeb3 100644
--- a/falcon/app.py
+++ b/falcon/app.py
@@ -160,6 +160,10 @@ class App:
             (default ``False``).
             (See also: :ref:`CORS <cors>`)
 
+        sink_before_static_route (bool): Indicates if the sinks should be processed
+            before (when ``True``) or after (when ``False``) the static routes.
+            This has an effect only if no route was matched. (default ``True``)
+
     Attributes:
         req_options: A set of behavioral options related to incoming
             requests. (See also: :py:class:`~.RequestOptions`)
@@ -192,22 +196,43 @@ class App:
     _default_responder_bad_request = falcon.responders.bad_request
     _default_responder_path_not_found = falcon.responders.path_not_found
 
-    __slots__ = ('_request_type', '_response_type',
-                 '_error_handlers', '_router', '_sinks',
-                 '_serialize_error', 'req_options', 'resp_options',
-                 '_middleware', '_independent_middleware', '_router_search',
-                 '_static_routes', '_cors_enable', '_unprepared_middleware',
-
-                 # NOTE(kgriffs): WebSocket is currently only supported for
-                 #   ASGI apps, but we may add support for WSGI at some point.
-                 '_middleware_ws')
-
-    def __init__(self, media_type=falcon.constants.DEFAULT_MEDIA_TYPE,
-                 request_type=Request, response_type=Response,
-                 middleware=None, router=None,
-                 independent_middleware=True, cors_enable=False):
+    __slots__ = (
+        '_cors_enable',
+        '_error_handlers',
+        '_independent_middleware',
+        '_middleware',
+        # NOTE(kgriffs): WebSocket is currently only supported for
+        #   ASGI apps, but we may add support for WSGI at some point.
+        '_middleware_ws',
+        '_request_type',
+        '_response_type',
+        '_router_search',
+        '_router',
+        '_serialize_error',
+        '_sink_and_static_routes',
+        '_sink_before_static_route',
+        '_sinks',
+        '_static_routes',
+        '_unprepared_middleware',
+        'req_options',
+        'resp_options',
+    )
+
+    def __init__(
+        self,
+        media_type=falcon.constants.DEFAULT_MEDIA_TYPE,
+        request_type=Request,
+        response_type=Response,
+        middleware=None,
+        router=None,
+        independent_middleware=True,
+        cors_enable=False,
+        sink_before_static_route=True,
+    ):
+        self._sink_before_static_route = sink_before_static_route
         self._sinks = []
         self._static_routes = []
+        self._sink_and_static_routes = ()
 
         if cors_enable:
             cm = CORSMiddleware()
@@ -570,11 +595,11 @@ class App:
 
         """
 
-        self._static_routes.insert(
-            0,
-            self._STATIC_ROUTE_TYPE(prefix, directory, downloadable=downloadable,
-                                    fallback_filename=fallback_filename)
+        sr = self._STATIC_ROUTE_TYPE(
+            prefix, directory, downloadable=downloadable, fallback_filename=fallback_filename
         )
+        self._static_routes.insert(0, (sr, sr, False))
+        self._update_sink_and_static_routes()
 
     def add_sink(self, sink, prefix=r'/'):
         """Register a sink method for the App.
@@ -589,7 +614,10 @@ class App:
         proxy that forwards requests to one or more backend services.
 
         Args:
-            sink (callable): A callable taking the form ``func(req, resp)``.
+            sink (callable): A callable taking the form ``func(req, resp, **kwargs)``.
+
+                Note:
+                    When using an async version of the ``App``, this must be a coroutine.
 
             prefix (str): A regex string, typically starting with '/', which
                 will trigger the sink if it matches the path portion of the
@@ -616,7 +644,8 @@ class App:
         # NOTE(kgriffs): Insert at the head of the list such that
         # in the case of a duplicate prefix, the last one added
         # is preferred.
-        self._sinks.insert(0, (prefix, sink))
+        self._sinks.insert(0, (prefix, sink, True))
+        self._update_sink_and_static_routes()
 
     def add_error_handler(self, exception, handler=None):
         """Register a handler for one or more exception types.
@@ -859,21 +888,16 @@ class App:
         else:
             params = {}
 
-            for pattern, sink in self._sinks:
-                m = pattern.match(path)
+            for matcher, obj, is_sink in self._sink_and_static_routes:
+                m = matcher.match(path)
                 if m:
-                    params = m.groupdict()
-                    responder = sink
+                    if is_sink:
+                        params = m.groupdict()
+                    responder = obj
 
                     break
             else:
-
-                for sr in self._static_routes:
-                    if sr.match(path):
-                        responder = sr
-                        break
-                else:
-                    responder = self.__class__._default_responder_path_not_found
+                responder = self.__class__._default_responder_path_not_found
 
         return (responder, params, resource, uri_template)
 
@@ -1017,6 +1041,12 @@ class App:
 
         return [], 0
 
+    def _update_sink_and_static_routes(self):
+        if self._sink_before_static_route:
+            self._sink_and_static_routes = tuple(self._sinks + self._static_routes)
+        else:
+            self._sink_and_static_routes = tuple(self._static_routes + self._sinks)
+
 
 # TODO(myusko): This class is a compatibility alias, and should be removed
 # in the next major release (4.0).
diff --git a/falcon/asgi/app.py b/falcon/asgi/app.py
index 387f2e0..c7fdecd 100644
--- a/falcon/asgi/app.py
+++ b/falcon/asgi/app.py
@@ -233,6 +233,10 @@ class App(falcon.app.App):
             (default ``False``).
             (See also: :ref:`CORS <cors>`)
 
+        sink_before_static_route (bool): Indicates if the sinks should be processed
+            before (when ``True``) or after (when ``False``) the static routes.
+            This has an effect only if no route was matched. (default ``True``)
+
     Attributes:
         req_options: A set of behavioral options related to incoming
             requests. (See also: :py:class:`~.RequestOptions`)
@@ -258,6 +262,10 @@ class App(falcon.app.App):
     _default_responder_bad_request = falcon.responders.bad_request_async
     _default_responder_path_not_found = falcon.responders.path_not_found_async
 
+    __slots__ = (
+        'ws_options',
+    )
+
     def __init__(self, *args, request_type=Request, response_type=Response, **kwargs):
         super().__init__(*args, request_type=request_type, response_type=response_type, **kwargs)
 
diff --git a/falcon/inspect.py b/falcon/inspect.py
index b513951..7901bc9 100644
--- a/falcon/inspect.py
+++ b/falcon/inspect.py
@@ -109,7 +109,7 @@ def inspect_static_routes(app: App) -> 'List[StaticRouteInfo]':
         been added to the application.
     """
     routes = []
-    for sr in app._static_routes:
+    for sr, _, _ in app._static_routes:
         info = StaticRouteInfo(sr._prefix, sr._directory, sr._fallback_filename)
         routes.append(info)
     return routes
@@ -126,7 +126,7 @@ def inspect_sinks(app: App) -> 'List[SinkInfo]':
         List[SinkInfo]: A list of sinks used by the application.
     """
     sinks = []
-    for prefix, sink in app._sinks:
+    for prefix, sink, _ in app._sinks:
         source_info, name = _get_source_info_and_name(sink)
         info = SinkInfo(prefix.pattern, name, source_info)
         sinks.append(info)


Test patch:
diff --git a/tests/test_inspect.py b/tests/test_inspect.py
index 3158738..92b396e 100644
--- a/tests/test_inspect.py
+++ b/tests/test_inspect.py
@@ -140,7 +140,7 @@ class TestInspectApp:
         assert routes[-2].directory == os.path.abspath('tests')
         assert routes[-2].fallback_filename.endswith('conftest.py')
 
-    def test_sync(self, asgi):
+    def test_sink(self, asgi):
         sinks = inspect.inspect_sinks(make_app_async() if asgi else make_app())
 
         assert all(isinstance(s, inspect.SinkInfo) for s in sinks)
diff --git a/tests/test_sink_and_static.py b/tests/test_sink_and_static.py
new file mode 100644
index 0000000..2065085
--- /dev/null
+++ b/tests/test_sink_and_static.py
@@ -0,0 +1,48 @@
+import pytest
+
+import falcon
+from falcon import testing
+
+from _util import create_app  # NOQA
+
+
+def sink(req, resp, **kw):
+    resp.body = 'sink'
+
+
+async def sink_async(req, resp, **kw):
+    resp.body = 'sink'
+
+
+@pytest.fixture
+def client(asgi, tmp_path):
+    file = tmp_path / 'file.txt'
+    file.write_text('foo bar')
+
+    def make(sink_before_static_route):
+        app = create_app(asgi=asgi, sink_before_static_route=sink_before_static_route)
+        app.add_sink(sink_async if asgi else sink, '/sink')
+        app.add_static_route('/sink/static', str(tmp_path))
+
+        return testing.TestClient(app)
+    return make
+
+
+def test_sink_before_static_route(client):
+    cl = client(True)
+    res = cl.simulate_get('/sink/foo')
+    assert res.text == 'sink'
+    res = cl.simulate_get('/sink/static/file.txt')
+    assert res.text == 'sink'
+    res = cl.simulate_get('/sink/static/')
+    assert res.text == 'sink'
+
+
+def test_sink_after_static_route(client):
+    cl = client(False)
+    res = cl.simulate_get('/sink/foo')
+    assert res.text == 'sink'
+    res = cl.simulate_get('/sink/static/file.txt')
+    assert res.text == 'foo bar'
+    res = cl.simulate_get('/sink/static/')
+    assert res.status == falcon.HTTP_404