From c15d6fb4d15ad37101ce34a88a2863897658a0d5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 15 Nov 2024 08:47:24 -0600 Subject: [PATCH] Refactor Resource.resolve to avoid linear search of methods (#9899) (cherry picked from commit 2249f2d59d30b7992e3a2ea25ab790c6f621b5cb) --- CHANGES/9899.misc.rst | 1 + aiohttp/web_urldispatcher.py | 45 ++++++++++++++++-------------------- 2 files changed, 21 insertions(+), 25 deletions(-) create mode 100644 CHANGES/9899.misc.rst diff --git a/CHANGES/9899.misc.rst b/CHANGES/9899.misc.rst new file mode 100644 index 0000000000..53243495d3 --- /dev/null +++ b/CHANGES/9899.misc.rst @@ -0,0 +1 @@ +Improved performance of resolving resources when multiple methods are registered for the same route -- by :user:`bdraco`. diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 5153e5b73b..03d7d7b15b 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -350,7 +350,9 @@ async def _default_expect_handler(request: Request) -> None: class Resource(AbstractResource): def __init__(self, *, name: Optional[str] = None) -> None: super().__init__(name=name) - self._routes: List[ResourceRoute] = [] + self._routes: Dict[str, ResourceRoute] = {} + self._any_route: Optional[ResourceRoute] = None + self._allowed_methods: Set[str] = set() def add_route( self, @@ -359,14 +361,12 @@ def add_route( *, expect_handler: Optional[_ExpectHandler] = None, ) -> "ResourceRoute": - - for route_obj in self._routes: - if route_obj.method == method or route_obj.method == hdrs.METH_ANY: - raise RuntimeError( - "Added route will never be executed, " - "method {route.method} is already " - "registered".format(route=route_obj) - ) + 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) @@ -376,23 +376,18 @@ def register_route(self, route: "ResourceRoute") -> None: assert isinstance( route, ResourceRoute ), f"Instance of Route class is required, got {route!r}" - self._routes.append(route) + if route.method == hdrs.METH_ANY: + self._any_route = route + else: + self._allowed_methods.add(route.method) + self._routes[route.method] = route async def resolve(self, request: Request) -> _Resolve: - allowed_methods: Set[str] = set() - - match_dict = self._match(request.rel_url.path_safe) - if match_dict is None: - return None, allowed_methods - - for route_obj in self._routes: - route_method = route_obj.method - allowed_methods.add(route_method) - - if route_method == request.method or route_method == hdrs.METH_ANY: - return (UrlMappingMatchInfo(match_dict, route_obj), allowed_methods) - else: - return None, allowed_methods + if (match_dict := self._match(request.rel_url.path_safe)) is None: + return None, set() + if route := self._routes.get(request.method, self._any_route): + return UrlMappingMatchInfo(match_dict, route), self._allowed_methods + return None, self._allowed_methods @abc.abstractmethod def _match(self, path: str) -> Optional[Dict[str, str]]: @@ -402,7 +397,7 @@ def __len__(self) -> int: return len(self._routes) def __iter__(self) -> Iterator["ResourceRoute"]: - return iter(self._routes) + return iter(self._routes.values()) # TODO: implement all abstract methods