Skip to content

Conversation

@raphael-goetz
Copy link
Member

Resolves: #132

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates the REST adapter to use hyper directly as the HTTP server foundation (addressing issue #132) and removes the now-redundant in-repo crates/http implementation.

Changes:

  • Replaced the custom http crate usage in the REST adapter with a hyper-based HTTP/1 server.
  • Introduced request routing via RequestRoute (method + URL) and extracted server config into a dedicated module.
  • Removed crates/http from the workspace and updated dependencies/lockfile accordingly.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
adapter/rest/src/main.rs Implements a hyper-based server loop, request handling, and flow execution response mapping.
adapter/rest/src/route.rs Adds method+URL-based flow identification and slug extraction helper.
adapter/rest/src/config.rs Defines HTTP server config and environment loading defaults.
adapter/rest/Cargo.toml Swaps http workspace crate dependency for hyper/hyper-util/http-body-util.
crates/base/src/store.rs Adds logging around flow execution requests (but currently introduces a move/compile issue).
Cargo.toml Removes crates/http from workspace members and workspace dependency overrides.
Cargo.lock Updates lockfile to reflect dependency changes and removal of the local http crate.
crates/http/* Removes the legacy custom HTTP implementation from the repository.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +48 to +54
fn json_error(status: StatusCode, msg: &str) -> Response<Full<Bytes>> {
let body = format!(r#"{{"error": "{}"}}"#, msg);
Response::builder()
.status(status)
.header("content-type", "application/json")
.body(Full::new(Bytes::from(body)))
.unwrap()
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

json_error interpolates msg directly into a JSON string without escaping, so quotes/newlines in msg can produce invalid JSON. Build the body via serde_json (or at least JSON-escape msg) before returning.

Copilot uses AI. Check for mistakes.
Comment on lines +215 to +217
let method = req.method().clone();
let path = req.uri().path().to_string();

Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path is taken from req.uri().path(), which drops the query string. Previously the request parser included ?query in path and also exposed parsed query params in the body structure, so route matching and flow inputs can change. If query strings matter for routing or inputs, use uri().path_and_query() and/or parse the query into a Value like before.

Copilot uses AI. Check for mistakes.
raphael-goetz and others added 8 commits January 30, 2026 13:58
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Raphael Götz <52959657+raphael-goetz@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Raphael Götz <52959657+raphael-goetz@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Raphael Götz <52959657+raphael-goetz@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Raphael Götz <52959657+raphael-goetz@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Raphael Götz <52959657+raphael-goetz@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Raphael Götz <52959657+raphael-goetz@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Raphael Götz <52959657+raphael-goetz@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 9 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +68 to +78
// Check if the request is matching
match regex::Regex::new(regex_str) {
Ok(regex) => {
log::debug!("Successfully compiled regex: {}", regex_str);
regex.is_match(&self.url)
}
Err(err) => {
log::error!("Failed to compile regex: {}", err);
false
}
}
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

identify compiles a new Regex for every candidate flow (Regex::new(regex_str)), which can become a hot path since get_possible_flow_match iterates over all matching keys/flows. Consider caching compiled regexes by pattern string (e.g., LRU/once_cell cache) or storing a precompiled regex in the flow representation if possible.

Copilot uses AI. Check for mistakes.
Comment on lines +52 to +58
fn json_error(status: StatusCode, msg: &str) -> Response<Full<Bytes>> {
let body = format!(r#"{{"error": "{}"}}"#, msg);
Response::builder()
.status(status)
.header("content-type", "application/json")
.body(Full::new(Bytes::from(body)))
.unwrap()
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

json_error interpolates msg directly into a JSON string. If msg contains quotes/backslashes/newlines, the response becomes invalid JSON (and can lead to confusing client behavior). Build the body via serde_json (or properly escape the string) instead of manual string formatting.

Copilot uses AI. Check for mistakes.
Comment on lines +146 to 158
// headers struct
let Value {
kind:
Some(StructValue(Struct {
fields: header_fields,
Some(Kind::ListValue(ListValue {
values: header_fields,
})),
} = headers
} = headers_val
else {
return None;
return json_error(
StatusCode::INTERNAL_SERVER_ERROR,
"headers was not a list of header entries",
);
};
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

headers_val is a &Value (from fields.get(...)), but this destructuring pattern matches against an owned Value. This will not compile. Match on a reference (e.g., &Value { .. }) or use headers_val.kind.as_ref() / if let Some(Kind::ListValue(...)) = headers_val.kind.as_ref() to avoid moving out of a borrowed value.

Copilot uses AI. Check for mistakes.
Comment on lines +204 to +210
// status_code number
let Some(Kind::NumberValue(code)) = status_code_val.kind else {
return json_error(
StatusCode::INTERNAL_SERVER_ERROR,
"status_code was not a number",
);
};
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

status_code_val is a &Value, so status_code_val.kind attempts to move the kind field out of a borrowed value and will not compile. Use status_code_val.kind.as_ref() (and then map/clone as needed) to read it without moving.

Copilot uses AI. Check for mistakes.
Comment on lines +232 to +234
let method = req.method().clone();
let path = req.uri().path().to_string();

Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handle_request uses req.uri().path(), which drops the query string. Previously the full request target (including ?query=...) could be used for matching and/or input construction. If flow matching or downstream logic depends on query parameters, consider using uri().path_and_query() and/or parsing the query into the Value input passed to validate_and_execute_flow.

Copilot uses AI. Check for mistakes.
let value: Option<Value> = if body.is_empty() {
None
} else {
match prost::Message::decode(body.as_slice()) {
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prost::Message::decode(...) cannot be called directly on the Message trait because the concrete Self type is unknown; this will not compile. Decode using the concrete type (e.g., Value::decode(...) or <Value as prost::Message>::decode(...)).

Suggested change
match prost::Message::decode(body.as_slice()) {
match Value::decode(body.as_slice()) {

Copilot uses AI. Check for mistakes.
}

let uuid = uuid::Uuid::new_v4().to_string();
let flow_id = flow.flow_id;
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flow_id is moved out of flow (let flow_id = flow.flow_id;) and then flow is moved into convert_validation_flow(flow, ...). Since flow_id is not Copy, this leaves flow partially moved and will not compile. Use let flow_id = flow.flow_id.clone(); (or borrow it via &flow.flow_id for logging) before passing flow onward.

Suggested change
let flow_id = flow.flow_id;
let flow_id = flow.flow_id.clone();

Copilot uses AI. Check for mistakes.
Comment on lines +99 to +112
let value: Option<Value> = if body.is_empty() {
None
} else {
match prost::Message::decode(body.as_slice()) {
Ok(v) => Some(v),
Err(e) => {
log::warn!("Failed to decode request body as protobuf Value: {}", e);
return json_error(
StatusCode::BAD_REQUEST,
"Failed to decode request body as protobuf Value",
);
}
}
};
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The request body is decoded as a protobuf-encoded tucana::shared::Value. For an HTTP/REST adapter this is a behavior change vs typical JSON payloads (and the prior implementation built Value from JSON). If clients send JSON, this will always return 400. Consider decoding based on Content-Type (e.g., JSON -> Value) and preserving the previous REST semantics.

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Raphael Götz <52959657+raphael-goetz@users.noreply.github.com>
@raphael-goetz raphael-goetz merged commit 9d625a8 into main Jan 30, 2026
4 checks passed
@raphael-goetz raphael-goetz deleted the 132-hyper-base branch January 30, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use Hyper as HTTP Base

2 participants