The request -> response path is not unit-testable
The high-value HTTP logic in SQLPage (Accept negotiation, error -> status mapping, the header/body streaming transition, and the 404 status decision) currently cannot be exercised without spinning up a full actix runtime and a real DB-backed AppState. That is why none of it has unit coverage today, and why subtle HTTP-correctness bugs (wrong status codes, wrong content type for a given Accept, 404-vs-200) can slip in unnoticed.
The root cause is that actix's &mut ServiceRequest is threaded all the way through routing, execution, and rendering instead of being confined to the edge.
Where the type flows
The actix request type is dragged through every layer of the core:
A plain RequestInfo data struct already exists, which is exactly the shape downstream code wants. But it is only ever constructed inside extract_request_info, after the actix extraction, so all the interesting logic stays locked behind the actix request type.
The decisions worth testing all live behind that wall:
Evidence of the gap
render_sql, process_sql_request, build_response_header_and_stream, and stream_response have no unit tests. The only tests in http.rs cover span-name string formatting (request_span_name, sql_execution_span_name).
$ grep -rn "ServiceRequest" src | wc -l
45
$ grep -n "fn .*ServiceRequest" src/webserver/http.rs
314:fn request_span_route(request: &ServiceRequest) -> Cow<'_, str> {
320:fn request_span_name(request: &ServiceRequest) -> String {
343:fn set_otel_parent(request: &ServiceRequest, span: &Span) {
355: fn on_request_start(request: &ServiceRequest) -> Span {
(render_sql, process_sql_request, and main_handler take the request as an async fn argument, so they do not show up in that one-line grep, but they all carry &mut ServiceRequest / ServiceRequest.)
Proposed change
Push the actix-touching code to a thin adapter at the very edge, e.g. fn from_actix(req) -> RequestInfo, and make everything downstream operate on the existing RequestInfo / ExecutionContext plus a Stream<DbItem>.
build_response_header_and_stream is already generic over S: Stream<Item = DbItem>, so once the actix request is gone from its inputs it can be driven by a hand-rolled in-memory stream in tests. "Given this request + this sequence of DbItems, what status / headers / body come out" then becomes a plain unit test, with no actix runtime and no live DB.
Risks and mitigations
- Touches many signatures. Mitigate by introducing the
RequestInfo boundary at the edge and migrating call sites incrementally. ExecutionContext already exists and already wraps RequestInfo, so most downstream code can switch to it with minimal churn.
- Behavior drift during the refactor. The extraction itself is mechanical; the move is from "actix type in, decision out" to "data struct in, decision out", with the actix-specific parsing isolated in the adapter.
Expected win
A unit-testable request -> response path, which unblocks fixing status-code and content-format bugs with proper regression tests instead of relying on full end-to-end runs.
The request -> response path is not unit-testable
The high-value HTTP logic in SQLPage (Accept negotiation, error -> status mapping, the header/body streaming transition, and the 404 status decision) currently cannot be exercised without spinning up a full actix runtime and a real DB-backed
AppState. That is why none of it has unit coverage today, and why subtle HTTP-correctness bugs (wrong status codes, wrong content type for a givenAccept, 404-vs-200) can slip in unnoticed.The root cause is that actix's
&mut ServiceRequestis threaded all the way through routing, execution, and rendering instead of being confined to the edge.Where the type flows
The actix request type is dragged through every layer of the core:
extract_request_info(req: &mut ServiceRequest, ...)render_sql(srv_req: &mut ServiceRequest, ...)process_sql_request(req: &mut ServiceRequest, ...)main_handler(mut service_request: ServiceRequest)A plain
RequestInfodata struct already exists, which is exactly the shape downstream code wants. But it is only ever constructed insideextract_request_info, after the actix extraction, so all the interesting logic stays locked behind the actix request type.The decisions worth testing all live behind that wall:
render_sqlbuild_response_header_and_streamandstream_responseAccept: html-> render404.sqland forceStatusCode::NOT_FOUND, otherwise plaintext) inmain_handlerEvidence of the gap
render_sql,process_sql_request,build_response_header_and_stream, andstream_responsehave no unit tests. The only tests inhttp.rscover span-name string formatting (request_span_name,sql_execution_span_name).(
render_sql,process_sql_request, andmain_handlertake the request as anasync fnargument, so they do not show up in that one-line grep, but they all carry&mut ServiceRequest/ServiceRequest.)Proposed change
Push the actix-touching code to a thin adapter at the very edge, e.g.
fn from_actix(req) -> RequestInfo, and make everything downstream operate on the existingRequestInfo/ExecutionContextplus aStream<DbItem>.build_response_header_and_streamis already generic overS: Stream<Item = DbItem>, so once the actix request is gone from its inputs it can be driven by a hand-rolled in-memory stream in tests. "Given this request + this sequence ofDbItems, what status / headers / body come out" then becomes a plain unit test, with no actix runtime and no live DB.Risks and mitigations
RequestInfoboundary at the edge and migrating call sites incrementally.ExecutionContextalready exists and already wrapsRequestInfo, so most downstream code can switch to it with minimal churn.Expected win
A unit-testable request -> response path, which unblocks fixing status-code and content-format bugs with proper regression tests instead of relying on full end-to-end runs.