Improved route registry module cleanup and guard ordering.

owlbot/registries/README.md

@@ -175,10 +175,10 @@ flowchart TD
     Match --> Result{"route match result"}
     Result -->|no path| NotFound["404"]
     Result -->|method not allowed| MethodNotAllowed["405 with Allow header"]
-    Result -->|handler found| Context["build RouteContext"]
+    Result -->|handler found| Guards{"session/auth/moderator guards pass?"}
-    Context --> Guards{"session/auth/moderator guards pass?"}
     Guards -->|no| Guidance["return 401 or 403 guidance page"]
-    Guards -->|yes| Streaming{"streaming route?"}
+    Guards -->|yes| Context["build RouteContext"]
+    Context --> Streaming{"streaming route?"}
     Streaming -->|yes| StreamTask["track streaming task for shutdown"]
     Streaming -->|no| HandlerTask["track non-streaming task for shutdown"]
     StreamTask --> NoTimeout["call handler without timeout"]
@@ -197,11 +197,12 @@ Multiple handlers can share the same path when their HTTP methods do not
 overlap. If a path matches but the method does not, dispatch returns `405` with
 an `Allow` header. If no path matches, dispatch returns `404`.
 
-Route guards run before the handler. `requires_session` needs any valid Owlbot
+Route guards run before `RouteContext` is built or the handler is called.
-browser session; `requires_authenticated` and `requires_moderator` require an
+`requires_session` needs any valid Owlbot browser session;
-authenticated user or moderator session. Any guard failure returns before the
+`requires_authenticated` and `requires_moderator` require an authenticated user
-handler is called. Missing or stale sessions return the shared connect guidance
+or moderator session. Any guard failure returns before module context lookup.
-page, while authentication and moderator failures return `403` guidance pages.
+Missing or stale sessions return the shared connect guidance page, while
+authentication and moderator failures return `403` guidance pages.
 
 Route handlers receive `RouteContext`, which contains the aiohttp request, the
 owning `ModuleContext`, captured path parameters, and the resolved browser

owlbot/registries/routes.py

@@ -74,8 +74,8 @@ class RouteRegistry:
         # Ordered list of route groups for pattern matching.
         # Each group represents a unique path pattern with one or more handlers.
         self._groups: list[_RouteGroup] = []
-        # Maps module_name -> list of full_paths (for cleanup).
+        # Maps module_name -> full_paths owned by that module (for cleanup).
-        self._module_routes: dict[str, list[str]] = defaultdict(list)
+        self._module_routes: defaultdict[str, set[str]] = defaultdict(set)
         logger.debug("RouteRegistry initialized.")
 
     def _find_group(self, full_path: str) -> _RouteGroup | None:
@@ -155,8 +155,7 @@ class RouteRegistry:
         else:
             group.handlers.append(info)
 
-        if full_path not in self._module_routes[module_name]:
+        self._module_routes[module_name].add(full_path)
-            self._module_routes[module_name].append(full_path)
 
         module_logger = logging.getLogger(f"owlbot.modules.{module_name}.routes")
         module_logger.debug(
@@ -186,12 +185,11 @@ class RouteRegistry:
                 # Remove entire group.
                 self._groups.pop(i)
                 for handler in group.handlers:
-                    if handler.module_name in self._module_routes:
+                    paths = self._module_routes.get(handler.module_name)
-                        self._module_routes[handler.module_name] = [
+                    if paths is not None:
-                            p
+                        paths.discard(full_path)
-                            for p in self._module_routes[handler.module_name]
+                        if not paths:
-                            if p != full_path
+                            self._module_routes.pop(handler.module_name, None)
-                        ]
                 module_names = {h.module_name for h in group.handlers}
                 for name in module_names:
                     module_logger = logging.getLogger(f"owlbot.modules.{name}.routes")
@@ -209,12 +207,12 @@ class RouteRegistry:
                     still_owns_path = any(
                         h.module_name == module_name for h in group.handlers
                     )
-                    if not still_owns_path and module_name in self._module_routes:
+                    if not still_owns_path:
-                        self._module_routes[module_name] = [
+                        paths = self._module_routes.get(module_name)
-                            p
+                        if paths is not None:
-                            for p in self._module_routes[module_name]
+                            paths.discard(full_path)
-                            if p != full_path
+                            if not paths:
-                        ]
+                                self._module_routes.pop(module_name, None)
                     # Remove the group if no handlers remain.
                     if not group.handlers:
                         self._groups.pop(i)
@@ -304,7 +302,9 @@ class RouteRegistry:
         :param module_name: The module name.
         :return: List of RouteInfo for that module.
         """
-        paths = set(self._module_routes.get(module_name, []))
+        paths = self._module_routes.get(module_name)
+        if not paths:
+            return []
         return [
             handler
             for group in self._groups
@@ -320,7 +320,7 @@ class RouteRegistry:
         :return: Number of route handlers removed.
         """
         module_logger = logging.getLogger(f"owlbot.modules.{module_name}.routes")
-        paths = set(self._module_routes.get(module_name, []))
+        paths = self._module_routes.get(module_name)
 
         if not paths:
             return 0
@@ -594,6 +594,13 @@ class RouteDispatcher:
         module_name = route_info.module_name
         mod_logger = logging.getLogger(f"owlbot.modules.{module_name}.routes")
 
+        guard_response = self._guard_response(
+            route_info,
+            session=session,
+        )
+        if guard_response is not None:
+            return guard_response
+
         module_ctx = self._get_module_context(module_name)
 
         ctx = RouteContext(
@@ -603,13 +610,6 @@ class RouteDispatcher:
             session=session,
         )
 
-        guard_response = self._guard_response(
-            route_info,
-            session=session,
-        )
-        if guard_response is not None:
-            return guard_response
-
         logger.debug(
             "Calling route handler: %s from module: %s",
             route_info.full_path,