clan-app: Cleaned up http code

pkgs/clan-app/clan_app/deps/http/http_bridge.py

@@ -36,46 +36,151 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler):
         openapi_file: Path | None = None,
         swagger_dist: Path | None = None,
     ) -> None:
-        # Initialize the API bridge fields
+        # Initialize API bridge fields
         self.api = api
-        self.openapi_file = openapi_file
-        self.swagger_dist = swagger_dist
         self.middleware_chain = middleware_chain
         self.threads: dict[str, WebThread] = {}
 
-        # Initialize the HTTP handler
+        # Initialize OpenAPI/Swagger fields
+        self.openapi_file = openapi_file
+        self.swagger_dist = swagger_dist
+
+        # Initialize HTTP handler
         super(BaseHTTPRequestHandler, self).__init__(request, client_address, server)
 
+    def _send_cors_headers(self) -> None:
+        """Send CORS headers for cross-origin requests."""
+        self.send_header("Access-Control-Allow-Origin", "*")
+        self.send_header("Access-Control-Allow-Methods", "GET, POST, OPTIONS")
+        self.send_header("Access-Control-Allow-Headers", "Content-Type")
+
+    def _send_json_response_with_status(
+        self, data: dict[str, Any], status_code: int = 200
+    ) -> None:
+        """Send a JSON response with the given status code."""
+        try:
+            self.send_response_only(status_code)
+            self.send_header("Content-Type", "application/json")
+            self._send_cors_headers()
+            self.end_headers()
+
+            response_data = json.dumps(data, indent=2, ensure_ascii=False)
+            self.wfile.write(response_data.encode("utf-8"))
+        except BrokenPipeError as e:
+            log.warning(f"Client disconnected before we could send a response: {e!s}")
+
     def send_api_response(self, response: BackendResponse) -> None:
         """Send HTTP response directly to the client."""
+        response_dict = dataclass_to_dict(response)
+        self._send_json_response_with_status(response_dict, 200)
+        log.debug(
+            f"HTTP response for {response._op_key}: {json.dumps(response_dict, indent=2)}"
+        )  # noqa: SLF001
+
+    def _create_success_response(
+        self, op_key: str, data: dict[str, Any]
+    ) -> BackendResponse:
+        """Create a successful API response."""
+        return BackendResponse(
+            body=SuccessDataClass(op_key=op_key, status="success", data=data),
+            header={},
+            _op_key=op_key,
+        )
+
+    def _send_info_response(self) -> None:
+        """Send server information response."""
+        response = self._create_success_response(
+            "info", {"message": "Clan API Server", "version": "1.0.0"}
+        )
+        self.send_api_response(response)
+
+    def _send_methods_response(self) -> None:
+        """Send available API methods response."""
+        response = self._create_success_response(
+            "methods", {"methods": list(self.api.functions.keys())}
+        )
+        self.send_api_response(response)
+
+    def _handle_swagger_request(self, parsed_url: Any) -> None:
+        """Handle Swagger UI related requests."""
+        if not self.swagger_dist or not self.swagger_dist.exists():
+            self.send_error(404, "Swagger file not found")
+            return
+
+        rel_path = parsed_url.path[len("/api/swagger") :].lstrip("/")
+
+        # Redirect /api/swagger to /api/swagger/index.html
+        if rel_path == "":
+            self.send_response(302)
+            self.send_header("Location", "/api/swagger/index.html")
+            self.end_headers()
+            return
+
+        self._serve_swagger_file(rel_path)
+
+    def _serve_swagger_file(self, rel_path: str) -> None:
+        """Serve a specific Swagger UI file."""
+        file_path = self._get_swagger_file_path(rel_path)
+
+        if not file_path.exists() or not file_path.is_file():
+            self.send_error(404, "Swagger file not found")
+            return
 
         try:
-            # Send HTTP response
-            self.send_response_only(200)
-            self.send_header("Content-Type", "application/json")
-            self.send_header("Access-Control-Allow-Origin", "*")
-            self.send_header("Access-Control-Allow-Methods", "GET, POST, OPTIONS")
-            self.send_header("Access-Control-Allow-Headers", "Content-Type")
+            content_type = self._get_content_type(file_path)
+            file_data = self._read_and_process_file(file_path, rel_path)
+
+            self.send_response(200)
+            self.send_header("Content-Type", content_type)
             self.end_headers()
+            self.wfile.write(file_data)
+        except Exception as e:
+            log.error(f"Error reading Swagger file: {e!s}")
+            self.send_error(500, "Internal Server Error")
 
-            # Write response data
-            response_data = json.dumps(
-                dataclass_to_dict(response), indent=2, ensure_ascii=False
-            )
-            self.wfile.write(response_data.encode("utf-8"))
+    def _get_swagger_file_path(self, rel_path: str) -> Path:
+        """Get the file path for a Swagger resource."""
+        if rel_path == "index.html":
+            return Path(__file__).parent / "swagger.html"
+        if rel_path == "openapi.json":
+            if not self.openapi_file:
+                return Path("/nonexistent")  # Will fail exists() check
+            return self.openapi_file
+        return (
+            self.swagger_dist / rel_path if self.swagger_dist else Path("/nonexistent")
+        )
 
-            # Log the response for debugging
-            log.debug(f"HTTP response for {response._op_key}: {response_data}")  # noqa: SLF001
-        except BrokenPipeError as e:
-            # Handle broken pipe errors gracefully
-            log.warning(f"Client disconnected before we could send a response: {e!s}")
+    def _get_content_type(self, file_path: Path) -> str:
+        """Get the content type for a file based on its extension."""
+        content_types = {
+            ".html": "text/html",
+            ".js": "application/javascript",
+            ".css": "text/css",
+            ".json": "application/json",
+            ".png": "image/png",
+            ".svg": "image/svg+xml",
+        }
+        return content_types.get(file_path.suffix, "application/octet-stream")
+
+    def _read_and_process_file(self, file_path: Path, rel_path: str) -> bytes:
+        """Read and optionally process a file (e.g., inject server URL into openapi.json)."""
+        with file_path.open("rb") as f:
+            file_data = f.read()
+
+        if rel_path == "openapi.json":
+            json_data = json.loads(file_data.decode("utf-8"))
+            server_address = getattr(self.server, "server_address", ("localhost", 80))
+            json_data["servers"] = [
+                {"url": f"http://{server_address[0]}:{server_address[1]}/api/v1/"}
+            ]
+            file_data = json.dumps(json_data, indent=2).encode("utf-8")
+
+        return file_data
 
     def do_OPTIONS(self) -> None:  # noqa: N802
         """Handle CORS preflight requests."""
         self.send_response_only(200)
-        self.send_header("Access-Control-Allow-Origin", "*")
-        self.send_header("Access-Control-Allow-Methods", "GET, POST, OPTIONS")
-        self.send_header("Access-Control-Allow-Headers", "Content-Type")
+        self._send_cors_headers()
         self.end_headers()
 
     def do_GET(self) -> None:  # noqa: N802
@@ -84,102 +189,28 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler):
         path = parsed_url.path
 
         if path == "/":
-            response = BackendResponse(
-                body=SuccessDataClass(
-                    op_key="info",
-                    status="success",
-                    data={"message": "Clan API Server", "version": "1.0.0"},
-                ),
-                header={},
-                _op_key="info",
-            )
-            self.send_api_response(response)
+            self._send_info_response()
         elif path.startswith("/api/swagger"):
-            if self.swagger_dist and self.swagger_dist.exists():
-                # Serve static files from swagger_dist
-                rel_path = parsed_url.path[len("/api/swagger") :].lstrip("/")
-                # Redirect /api/swagger (no trailing slash or file) to /api/swagger/index.html
-                if rel_path == "":
-                    self.send_response(302)
-                    self.send_header("Location", "/api/swagger/index.html")
-                    self.end_headers()
-                    return
-                file_path = self.swagger_dist / rel_path
-                if rel_path == "index.html":
-                    file_path = Path(__file__).parent / "swagger.html"
-                elif rel_path == "openapi.json":
-                    if not self.openapi_file:
-                        self.send_error(404, "OpenAPI file not found")
-                        return
-                    file_path = self.openapi_file
-
-                if file_path.exists() and file_path.is_file():
-                    try:
-                        # Guess content type
-                        if file_path.suffix == ".html":
-                            content_type = "text/html"
-                        elif file_path.suffix == ".js":
-                            content_type = "application/javascript"
-                        elif file_path.suffix == ".css":
-                            content_type = "text/css"
-                        elif file_path.suffix == ".json":
-                            content_type = "application/json"
-                        elif file_path.suffix == ".png":
-                            content_type = "image/png"
-                        elif file_path.suffix == ".svg":
-                            content_type = "image/svg+xml"
-                        else:
-                            content_type = "application/octet-stream"
-                        with file_path.open("rb") as f:
-                            file_data = f.read()
-                        if rel_path == "openapi.json":
-                            json_data = json.loads(file_data.decode("utf-8"))
-                            json_data["servers"] = [
-                                {
-                                    "url": f"http://{getattr(self.server, 'server_address', ('localhost', 80))[0]}:{getattr(self.server, 'server_address', ('localhost', 80))[1]}/api/v1/"
-                                }
-                            ]
-                            file_data = json.dumps(json_data, indent=2).encode("utf-8")
-                        self.send_response(200)
-                        self.send_header("Content-Type", content_type)
-                        self.end_headers()
-
-                        self.wfile.write(file_data)
-                    except Exception as e:
-                        log.error(f"Error reading Swagger file: {e!s}")
-                        self.send_error(500, "Internal Server Error")
-                else:
-                    self.send_error(404, "Swagger file not found")
-            else:
-                self.send_error(404, "Swagger file not found")
-
+            self._handle_swagger_request(parsed_url)
         elif path == "/api/methods":
-            response = BackendResponse(
-                body=SuccessDataClass(
-                    op_key="methods",
-                    status="success",
-                    data={"methods": list(self.api.functions.keys())},
-                ),
-                header={},
-                _op_key="methods",
-            )
-            self.send_api_response(response)
+            self._send_methods_response()
+        else:
+            self.send_api_error_response("info", "Not Found", ["http_bridge", "GET"])
 
     def do_POST(self) -> None:  # noqa: N802
         """Handle POST requests."""
         parsed_url = urlparse(self.path)
         path = parsed_url.path
 
-        # Check if this is an API call
+        # Validate API path
         if not path.startswith("/api/v1/"):
             self.send_api_error_response(
-                "post", f"Path not found  {path}", ["http_bridge", "POST"]
+                "post", f"Path not found: {path}", ["http_bridge", "POST"]
             )
             return
 
-        # Extract method name from path
+        # Extract and validate method name
         method_name = path[len("/api/v1/") :]
-
         if not method_name:
             self.send_api_error_response(
                 "post", "Method name required", ["http_bridge", "POST"]
@@ -194,33 +225,13 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler):
             )
             return
 
-        # Read request body
-        try:
-            content_length = int(self.headers.get("Content-Length", 0))
-            if content_length > 0:
-                body = self.rfile.read(content_length)
-                request_data = json.loads(body.decode("utf-8"))
-            else:
-                request_data = {}
-        except json.JSONDecodeError:
-            self.send_api_error_response(
-                "post",
-                "Invalid JSON in request body",
-                ["http_bridge", "POST", method_name],
-            )
-            return
-        except Exception as e:
-            self.send_api_error_response(
-                "post",
-                f"Error reading request: {e!s}",
-                ["http_bridge", "POST", method_name],
-            )
-            return
+        # Read and parse request body
+        request_data = self._read_request_body(method_name)
+        if request_data is None:
+            return  # Error already sent
 
-        # Generate a unique operation key
+        # Generate operation key and handle request
         gen_op_key = str(uuid.uuid4())
-
-        # Handle the API request
         try:
             self._handle_api_request(method_name, request_data, gen_op_key)
         except Exception as e:
@@ -231,6 +242,29 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler):
                 ["http_bridge", "POST", method_name],
             )
 
+    def _read_request_body(self, method_name: str) -> dict[str, Any] | None:
+        """Read and parse the request body. Returns None if there was an error."""
+        try:
+            content_length = int(self.headers.get("Content-Length", 0))
+            if content_length > 0:
+                body = self.rfile.read(content_length)
+                return json.loads(body.decode("utf-8"))
+            return {}
+        except json.JSONDecodeError:
+            self.send_api_error_response(
+                "post",
+                "Invalid JSON in request body",
+                ["http_bridge", "POST", method_name],
+            )
+            return None
+        except Exception as e:
+            self.send_api_error_response(
+                "post",
+                f"Error reading request: {e!s}",
+                ["http_bridge", "POST", method_name],
+            )
+            return None
+
     def _handle_api_request(
         self,
         method_name: str,
@@ -239,32 +273,11 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler):
     ) -> None:
         """Handle an API request by processing it through middleware."""
         try:
-            # Parse the HTTP request format
-            header = request_data.get("header", {})
-            if not isinstance(header, dict):
-                msg = f"Expected header to be a dict, got {type(header)}"
-                raise TypeError(msg)
+            # Validate and parse request data
+            header, body, op_key = self._parse_request_data(request_data, gen_op_key)
 
-            body = request_data.get("body", {})
-            if not isinstance(body, dict):
-                msg = f"Expected body to be a dict, got {type(body)}"
-                raise TypeError(msg)
-
-            op_key = header.get("op_key", gen_op_key)
-            if not isinstance(op_key, str):
-                msg = f"Expected op_key to be a string, got {type(op_key)}"
-                raise TypeError(msg)
-
-            # Check if op_key is a valid UUID
-            try:
-                uuid.UUID(op_key)
-            except ValueError as e:
-                msg = f"op_key '{op_key}' is not a valid UUID"
-                raise TypeError(msg) from e
-
-            if op_key in self.threads:
-                msg = f"Operation key '{op_key}' is already in use. Please try again."
-                raise ValueError(msg)
+            # Validate operation key
+            self._validate_operation_key(op_key)
 
             # Create API request
             api_request = BackendRequest(
@@ -272,14 +285,54 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler):
             )
 
         except Exception as e:
-            # Create error response directly
-            self.send_api_error_response(op_key, str(e), ["http_bridge", method_name])
+            self.send_api_error_response(
+                gen_op_key, str(e), ["http_bridge", method_name]
+            )
             return
 
-        # Process in a separate thread
+        self._process_api_request_in_thread(api_request, method_name)
+
+    def _parse_request_data(
+        self, request_data: dict[str, Any], gen_op_key: str
+    ) -> tuple[dict[str, Any], dict[str, Any], str]:
+        """Parse and validate request data components."""
+        header = request_data.get("header", {})
+        if not isinstance(header, dict):
+            msg = f"Expected header to be a dict, got {type(header)}"
+            raise TypeError(msg)
+
+        body = request_data.get("body", {})
+        if not isinstance(body, dict):
+            msg = f"Expected body to be a dict, got {type(body)}"
+            raise TypeError(msg)
+
+        op_key = header.get("op_key", gen_op_key)
+        if not isinstance(op_key, str):
+            msg = f"Expected op_key to be a string, got {type(op_key)}"
+            raise TypeError(msg)
+
+        return header, body, op_key
+
+    def _validate_operation_key(self, op_key: str) -> None:
+        """Validate that the operation key is valid and not in use."""
+        try:
+            uuid.UUID(op_key)
+        except ValueError as e:
+            msg = f"op_key '{op_key}' is not a valid UUID"
+            raise TypeError(msg) from e
+
+        if op_key in self.threads:
+            msg = f"Operation key '{op_key}' is already in use. Please try again."
+            raise ValueError(msg)
+
+    def _process_api_request_in_thread(
+        self, api_request: BackendRequest, method_name: str
+    ) -> None:
+        """Process the API request in a separate thread."""
+        op_key = api_request.op_key or "unknown"
+
         def thread_task(stop_event: threading.Event) -> None:
             set_should_cancel(lambda: stop_event.is_set())
-
             try:
                 self.process_request(api_request)
             finally:
@@ -295,7 +348,7 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler):
         # Wait for the thread to complete (this blocks until response is sent)
         thread.join(timeout=60.0)
 
-        # If thread is still alive, it timed out
+        # Handle timeout
         if thread.is_alive():
             stop_event.set()  # Cancel the thread
             self.send_api_error_response(

pkgs/clan-app/clan_app/deps/http/test_http_api.py

@@ -147,7 +147,7 @@ class TestHttpApiServer:
             # Test API call endpoint
             request_data: dict = {"header": {}, "body": {"message": "World"}}
             req: Request = Request(
-                "http://127.0.0.1:8081/api/call/test_method",
+                "http://127.0.0.1:8081/api/v1/test_method",
                 data=json.dumps(request_data).encode(),
                 headers={"Content-Type": "application/json"},
             )
@@ -182,7 +182,7 @@ class TestHttpApiServer:
             # Test method not found
             request_data: dict = {"header": {}, "body": {}}
             req: Request = Request(
-                "http://127.0.0.1:8081/api/call/nonexistent_method",
+                "http://127.0.0.1:8081/api/v1/nonexistent_method",
                 data=json.dumps(request_data).encode(),
                 headers={"Content-Type": "application/json"},
             )
@@ -194,7 +194,7 @@ class TestHttpApiServer:
 
             # Test invalid JSON
             req = Request(
-                "http://127.0.0.1:8081/api/call/test_method",
+                "http://127.0.0.1:8081/api/v1/test_method",
                 data=b"invalid json",
                 headers={"Content-Type": "application/json"},
             )
@@ -264,7 +264,7 @@ class TestIntegration:
                 "body": {"message": "Integration"},
             }
             req: Request = Request(
-                "http://127.0.0.1:8082/api/call/test_method",
+                "http://127.0.0.1:8082/api/v1/test_method",
                 data=json.dumps(request_data).encode(),
                 headers={"Content-Type": "application/json"},
             )

pyproject.toml

@@ -50,6 +50,7 @@ lint.ignore = [
   # We might actually want to fix this.
   "A005",
   "TRY301",
+  "TRY300",
   "ANN401",
   "RUF100",
   "TRY400",