Fix StaticResource routes not resolving

Author: BlindChickens

CHANGES/12255.bugfix.feature.rst

@@ -0,0 +1,6 @@
+Fixed :py:class:`~aiohttp.StaticResource` not resolving urls when adding them via the internal ``_routes`` property.
+You had to also add the method to the ``_allowed_methods`` property set, which is not ideal.
+
+Added :py:meth:`~aiohttp.StaticResource.add_route` and :py:meth:`~aiohttp.StaticResource.register_route`
+to :py:class:`~aiohttp.StaticResource` to handle this same :py:class:`~aiohttp.Resource`.
+-- by :user:`BlindChickens`.

CONTRIBUTORS.txt

@@ -182,6 +182,7 @@ Ivan Larin
 J. Nick Koston
 Jacob Champion
 Jacob Henner
+Jacques Combrink
 Jaesung Lee
 Jake Davis
 Jakob Ackermann

aiohttp/web_urldispatcher.py

@@ -101,6 +101,9 @@ class _InfoDict(TypedDict, total=False):
 class AbstractResource(Sized, Iterable["AbstractRoute"]):
     def __init__(self, *, name: str | None = None) -> None:
         self._name = name
+        self._routes: dict[str, ResourceRoute] = {}
+        self._any_route: ResourceRoute | None = None
+        self._allowed_methods: set[str] = set()
 
     @property
     def name(self) -> str | None:
@@ -119,6 +122,33 @@ def canonical(self) -> str:
     def url_for(self, **kwargs: str) -> URL:
         """Construct url for resource with additional params."""
 
+    def add_route(
+        self,
+        method: str,
+        handler: type[AbstractView] | Handler,
+        *,
+        expect_handler: _ExpectHandler | None = None,
+    ) -> "ResourceRoute":
+        if route := self._routes.get(method, self._any_route):
+            raise RuntimeError(
+                "Added route will never be executed, "
+                f"method {route.method} is already "
+                "registered"
+            )
+
+        route_obj = ResourceRoute(method, handler, self, expect_handler=expect_handler)
+        self.register_route(route_obj)
+        return route_obj
+
+    def register_route(self, route: "ResourceRoute") -> None:
+        assert isinstance(
+            route, ResourceRoute
+        ), f"Instance of Route class is required, got {route!r}"
+        if route.method == hdrs.METH_ANY:
+            self._any_route = route
+        self._allowed_methods.add(route.method)
+        self._routes[route.method] = route
+
     @abc.abstractmethod  # pragma: no branch
     async def resolve(self, request: Request) -> _Resolve:
         """Resolve resource.
@@ -308,36 +338,6 @@ async def _default_expect_handler(request: Request) -> None:
 class Resource(AbstractResource):
     def __init__(self, *, name: str | None = None) -> None:
         super().__init__(name=name)
-        self._routes: dict[str, ResourceRoute] = {}
-        self._any_route: ResourceRoute | None = None
-        self._allowed_methods: set[str] = set()
-
-    def add_route(
-        self,
-        method: str,
-        handler: type[AbstractView] | Handler,
-        *,
-        expect_handler: _ExpectHandler | None = None,
-    ) -> "ResourceRoute":
-        if route := self._routes.get(method, self._any_route):
-            raise RuntimeError(
-                "Added route will never be executed, "
-                f"method {route.method} is already "
-                "registered"
-            )
-
-        route_obj = ResourceRoute(method, handler, self, expect_handler=expect_handler)
-        self.register_route(route_obj)
-        return route_obj
-
-    def register_route(self, route: "ResourceRoute") -> None:
-        assert isinstance(
-            route, ResourceRoute
-        ), f"Instance of Route class is required, got {route!r}"
-        if route.method == hdrs.METH_ANY:
-            self._any_route = route
-        self._allowed_methods.add(route.method)
-        self._routes[route.method] = route
 
     async def resolve(self, request: Request) -> _Resolve:
         if (match_dict := self._match(request.rel_url.path_safe)) is None:
@@ -589,12 +589,7 @@ def get_info(self) -> _InfoDict:
         }
 
     def set_options_route(self, handler: Handler) -> None:
-        if "OPTIONS" in self._routes:
-            raise RuntimeError("OPTIONS route was set already")
-        self._routes["OPTIONS"] = ResourceRoute(
-            "OPTIONS", handler, self, expect_handler=self._expect_handler
-        )
-        self._allowed_methods.add("OPTIONS")
+        self.add_route("OPTIONS", handler)
 
     async def resolve(self, request: Request) -> _Resolve:
         path = request.rel_url.path_safe

docs/web_reference.rst

@@ -2098,6 +2098,26 @@ Resource classes hierarchy::
 
       .. versionadded:: 3.3
 
+   .. method:: add_route(method, handler, *, expect_handler=None)
+
+      Add a :term:`web-handler` to resource.
+
+      :param str method: HTTP method for route. Should be one of
+                         ``'GET'``, ``'POST'``, ``'PUT'``,
+                         ``'DELETE'``, ``'PATCH'``, ``'HEAD'``,
+                         ``'OPTIONS'`` or ``'*'`` for any method.
+
+                         The parameter is case-insensitive, e.g. you
+                         can push ``'get'`` as well as ``'GET'``.
+
+                         The method should be unique for resource.
+
+      :param collections.abc.Callable handler: route handler.
+
+      :param collections.abc.Coroutine expect_handler: optional *expect* header handler.
+
+      :returns: new :class:`ResourceRoute` instance.
+
    .. method:: resolve(request)
       :async:
 
@@ -2135,27 +2155,6 @@ Resource classes hierarchy::
    A base class for new-style resources, inherits :class:`AbstractResource`.
 
 
-   .. method:: add_route(method, handler, *, expect_handler=None)
-
-      Add a :term:`web-handler` to resource.
-
-      :param str method: HTTP method for route. Should be one of
-                         ``'GET'``, ``'POST'``, ``'PUT'``,
-                         ``'DELETE'``, ``'PATCH'``, ``'HEAD'``,
-                         ``'OPTIONS'`` or ``'*'`` for any method.
-
-                         The parameter is case-insensitive, e.g. you
-                         can push ``'get'`` as well as ``'GET'``.
-
-                         The method should be unique for resource.
-
-      :param collections.abc.Callable handler: route handler.
-
-      :param collections.abc.Coroutine expect_handler: optional *expect* header handler.
-
-      :returns: new :class:`ResourceRoute` instance.
-
-
 .. class:: PlainResource
    :canonical: aiohttp.web_urldispatcher.PlainResource
 
@@ -2206,7 +2205,7 @@ Resource classes hierarchy::
 .. class:: StaticResource
    :canonical: aiohttp.web_urldispatcher.StaticResource
 
-   A resource, inherited from :class:`Resource`.
+   A resource, inherited from :class:`PrefixResource`.
 
    The class corresponds to resources for :ref:`static file serving
    <aiohttp-web-static-file-handling>`.

tests/test_urldispatch.py

@@ -551,6 +551,19 @@ async def test_add_static_access_resources(router: web.UrlDispatcher) -> None:
     assert allowed_methods == {hdrs.METH_GET, hdrs.METH_OPTIONS, hdrs.METH_HEAD}
 
 
+async def test_add_static_access_resources_method(router: web.UrlDispatcher) -> None:
+    """Test adding a route via add_route to static resource."""
+    resource = router.add_static(
+        "/st", pathlib.Path(aiohttp.__file__).parent, name="static"
+    )
+    resource.add_route(hdrs.METH_POST, make_handler())
+    mapping, allowed_methods = await resource.resolve(
+        make_mocked_request("POST", "/st/path")
+    )
+    assert mapping is not None
+    assert allowed_methods == {hdrs.METH_GET, hdrs.METH_POST, hdrs.METH_HEAD}
+
+
 async def test_add_static_set_options_route(router: web.UrlDispatcher) -> None:
     """Ensure set_options_route works as expected."""
     resource = router.add_static(