diff --git a/CHANGES/12255.bugfix.rst b/CHANGES/12255.bugfix.rst new file mode 100644 index 00000000000..645bf728a39 --- /dev/null +++ b/CHANGES/12255.bugfix.rst @@ -0,0 +1,5 @@ +Fixed :py:class:`~aiohttp.web.StaticResource` not resolving urls when adding them via the internal ``_routes`` property. +You had to also add the method to the ``_allowed_methods`` property, which is not ideal. + +Added :py:meth:`~aiohttp.web.AbstractResource.add_route` to :py:class:`~aiohttp.web.AbstractResource`. +-- by :user:`BlindChickens`. diff --git a/CHANGES/12255.feature.rst b/CHANGES/12255.feature.rst new file mode 100644 index 00000000000..645bf728a39 --- /dev/null +++ b/CHANGES/12255.feature.rst @@ -0,0 +1,5 @@ +Fixed :py:class:`~aiohttp.web.StaticResource` not resolving urls when adding them via the internal ``_routes`` property. +You had to also add the method to the ``_allowed_methods`` property, which is not ideal. + +Added :py:meth:`~aiohttp.web.AbstractResource.add_route` to :py:class:`~aiohttp.web.AbstractResource`. +-- by :user:`BlindChickens`. diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index c3c16f82eee..f9dd1eb122d 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -182,6 +182,7 @@ Ivan Larin J. Nick Koston Jacob Champion Jacob Henner +Jacques Combrink Jaesung Lee Jake Davis Jakob Ackermann diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index b1d060bd0f3..1dcc8db396c 100644 --- a/aiohttp/web_urldispatcher.py +++ b/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 diff --git a/docs/web_reference.rst b/docs/web_reference.rst index 99318ec90ee..6519187fc6d 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -2052,7 +2052,7 @@ second one. User should never instantiate resource classes but give it by :meth:`UrlDispatcher.add_resource` call. -After that he may add a :term:`route` by calling :meth:`Resource.add_route`. +After that he may add a :term:`route` by calling :meth:`AbstractResource.add_route`. :meth:`UrlDispatcher.add_route` is just shortcut for:: @@ -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 @@ -2203,10 +2202,21 @@ Resource classes hierarchy:: be called as ``resource.url_for(to='val1', param='val2')`` +.. class:: PrefixResource + :canonical: aiohttp.web_urldispatcher.PrefixResource + + A resource, inherited from :class:`AbstractResource`. + + .. attribute:: canonical + + Read-only *canonical path* associate with the resource. Returns the prefix + used to create the PrefixResource. For example ``/prefix`` + + .. 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 `. diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index 6603389f16d..ec829691dec 100644 --- a/tests/test_urldispatch.py +++ b/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(