From eba8dd58642bc4949371e8e13b1bccf20bc6a74f Mon Sep 17 00:00:00 2001 From: Gregory Schier Date: Tue, 10 Nov 2026 13:47:25 -0700 Subject: [PATCH] CLI: handle gRPC and WebSocket requests, keep IDs on Yaak import (#816) Co-authored-by: Claude Opus 5.5 --- crates-cli/yaak-cli/src/cli.rs | 8 +- .../yaak-cli/src/commands/import_export.rs | 20 +- crates-cli/yaak-cli/src/commands/mcp.rs | 38 +- crates-cli/yaak-cli/src/commands/request.rs | 169 ++- crates-cli/yaak-cli/src/commands/response.rs | 245 ++++- crates-cli/yaak-cli/src/commands/send.rs | 29 +- .../yaak-cli/tests/import_export_commands.rs | 248 ++++- crates-cli/yaak-cli/tests/mcp_command.rs | 47 +- crates-cli/yaak-cli/tests/request_commands.rs | 78 ++ .../yaak-cli/tests/response_commands.rs | 202 ++++ crates-cli/yaak-cli/tests/send_commands.rs | 59 +- crates/yaak-mcp/src/server.rs | 35 +- crates/yaak/src/import.rs | 966 ++++++++++++++++-- 13 files changed, 1947 insertions(+), 197 deletions(-) create mode 100644 crates-cli/yaak-cli/tests/response_commands.rs diff --git a/crates-cli/yaak-cli/src/cli.rs b/crates-cli/yaak-cli/src/cli.rs index c52ff72bf..a5f0fc5ce 100644 --- a/crates-cli/yaak-cli/src/cli.rs +++ b/crates-cli/yaak-cli/src/cli.rs @@ -241,8 +241,12 @@ pub struct ExportArgs { #[arg(long, conflicts_with = "workspace_ids")] pub all: bool, - /// Include private environments in the export - #[arg(long)] + /// Include private environments in the export. Environments made in the app are private + /// unless marked shareable, so they are left out by default, as the app's export does. + #[arg( + long = "include-private", + visible_alias = "include-private-environments" + )] pub include_private_environments: bool, } diff --git a/crates-cli/yaak-cli/src/commands/import_export.rs b/crates-cli/yaak-cli/src/commands/import_export.rs index 89f60a7b3..85bee0061 100644 --- a/crates-cli/yaak-cli/src/commands/import_export.rs +++ b/crates-cli/yaak-cli/src/commands/import_export.rs @@ -6,6 +6,7 @@ use yaak::import; use yaak_models::export::{self, ExportDataParams}; use yaak_models::util::{ BatchUpsertResult, ImportDestination, ImportOrigin, ImportPlanAction, ImportPlanItem, + ImportPlanWarning, }; use yaak_plugins::events::{ImportRequest, ImportResources, PluginContext}; @@ -13,11 +14,14 @@ type CommandResult = std::result::Result; pub async fn run_import(ctx: &CliContext, args: ImportArgs) -> i32 { match import(ctx, args).await { - Ok((result, items)) => { + Ok((result, items, warnings)) => { println!("Imported {}", format_counts(&result)); if let Some(skipped) = format_skipped(&items) { println!("Skipped {skipped}"); } + for warning in warnings { + eprintln!("Warning: {}: {}", warning.title, warning.detail); + } 0 } Err(error) => { @@ -43,7 +47,7 @@ pub fn run_export(ctx: &CliContext, args: ExportArgs) -> i32 { async fn import( ctx: &CliContext, args: ImportArgs, -) -> CommandResult<(BatchUpsertResult, Vec)> { +) -> CommandResult<(BatchUpsertResult, Vec, Vec)> { if let Some(workspace_id) = args.workspace_id.as_deref() { ctx.db() .get_workspace(workspace_id) @@ -67,6 +71,15 @@ async fn import( .to_string(), ); } + // Everything would land in the one workspace, merging separate workspaces into it + if workspace_id.is_some() && resources.workspaces.len() > 1 { + return Err(format!( + "{} contains {} workspaces, and --workspace-id imports into a single one. Run `yaak import {}` without --workspace-id to import each as its own workspace, or export one workspace at a time.", + args.file.display(), + resources.workspaces.len(), + args.file.display(), + )); + } let destination = match workspace_id { Some(workspace_id) => { ImportDestination::ExistingWorkspace { workspace_id, folder_id: None } @@ -83,9 +96,10 @@ async fn import( ) .map_err(|e| format!("Failed to plan import: {e}"))?; let items = plan.items.clone(); + let warnings = plan.warnings.clone(); let imported = import::commit_import_plan(ctx.query_manager(), plan) .map_err(|e| format!("Failed to import data: {e}"))?; - Ok((imported, items)) + Ok((imported, items, warnings)) } fn file_origin(path: &std::path::Path) -> ImportOrigin { diff --git a/crates-cli/yaak-cli/src/commands/mcp.rs b/crates-cli/yaak-cli/src/commands/mcp.rs index e5356bd43..97e064128 100644 --- a/crates-cli/yaak-cli/src/commands/mcp.rs +++ b/crates-cli/yaak-cli/src/commands/mcp.rs @@ -646,7 +646,43 @@ mod tests { fx.ok("delete_http_request", json!({ "id": id })).await, format!("Deleted: Created ({id})") ); - assert_eq!(fx.json("get_http_request", json!({ "id": id })).await, Value::Null); + assert_eq!( + fx.err("get_http_request", json!({ "id": id })).await, + format!("HTTP request with ID {id} not found") + ); + fx.finish().await; + } + + /// QA sweep bug 6: `get_http_request` with a `wr_` ID or `rq_bogus` answered `"null"` + /// with `isError: false`, and `delete_http_request` on a `wr_` ID failed obscurely. + #[tokio::test] + async fn http_request_tools_reject_unknown_and_non_http_ids() { + let fx = Fixture::new(None).await; + let websocket = fx + .query_manager + .with_tx(|db| { + db.upsert_websocket_request( + &yaak_models::models::WebsocketRequest { + workspace_id: fx.workspace_id.clone(), + name: "Socket".into(), + ..Default::default() + }, + &UpdateSource::Background, + ) + }) + .unwrap(); + + assert_eq!( + fx.err("get_http_request", json!({ "id": "rq_bogus" })).await, + "HTTP request with ID rq_bogus not found" + ); + let expected = format!( + "{} is a WebSocket request, and these tools only work with HTTP requests. Use the Yaak app or the `yaak request` CLI commands for it.", + websocket.id + ); + assert_eq!(fx.err("get_http_request", json!({ "id": websocket.id })).await, expected); + assert_eq!(fx.err("delete_http_request", json!({ "id": websocket.id })).await, expected); + assert!(fx.query_manager.connect().get_websocket_request(&websocket.id).is_ok()); fx.finish().await; } diff --git a/crates-cli/yaak-cli/src/commands/request.rs b/crates-cli/yaak-cli/src/commands/request.rs index 39070a735..622ab341d 100644 --- a/crates-cli/yaak-cli/src/commands/request.rs +++ b/crates-cli/yaak-cli/src/commands/request.rs @@ -67,18 +67,48 @@ pub async fn run( } } +/// Find a request of any type, with an error that names the ID rather than a table. +pub(crate) fn get_any_request(ctx: &CliContext, request_id: &str) -> CommandResult { + let db = ctx.db(); + if let Ok(request) = db.get_http_request(request_id) { + return Ok(AnyRequest::HttpRequest(request)); + } + if let Ok(request) = db.get_grpc_request(request_id) { + return Ok(AnyRequest::GrpcRequest(request)); + } + if let Ok(request) = db.get_websocket_request(request_id) { + return Ok(AnyRequest::WebsocketRequest(request)); + } + Err(format!("Request {request_id} not found")) +} + fn list(ctx: &CliContext, workspace_id: Option<&str>) -> CommandResult { let workspace_id = resolve_workspace_id(ctx, workspace_id, "request list")?; - let requests = ctx - .db() + let db = ctx.db(); + let http = db .list_http_requests(&workspace_id) - .map_err(|e| format!("Failed to list requests: {e}"))?; - if requests.is_empty() { + .map_err(|e| format!("Failed to list HTTP requests: {e}"))?; + let grpc = db + .list_grpc_requests(&workspace_id) + .map_err(|e| format!("Failed to list gRPC requests: {e}"))?; + let websocket = db + .list_websocket_requests(&workspace_id) + .map_err(|e| format!("Failed to list WebSocket requests: {e}"))?; + + if http.is_empty() && grpc.is_empty() && websocket.is_empty() { println!("No requests found in workspace {}", workspace_id); - } else { - for request in requests { - println!("{} - {} {}", request.id, request.method, request.name); - } + return Ok(()); + } + // HTTP lines keep their ` - ` shape; the other types put their type + // where the method would be, since they have no method of that kind. + for request in http { + println!("{} - {} {}", request.id, request.method, request.name); + } + for request in grpc { + println!("{} - gRPC {}", request.id, request.name); + } + for request in websocket { + println!("{} - WebSocket {}", request.id, request.name); } Ok(()) } @@ -112,20 +142,65 @@ async fn schema(ctx: &CliContext, request_type: RequestSchemaType, pretty: bool) /// HttpRequest with its unknown fields dropped, quietly producing a broken request /// (the gRPC method name lands in `method`, and `service` disappears). fn reject_non_http_payload(payload: &Value, context: &str) -> CommandResult { - let Some(object) = payload.as_object() else { - return Ok(()); - }; - - if let Some(model) = object.get("model").and_then(Value::as_str) + if let Some(model) = payload.get("model").and_then(Value::as_str) && !model.is_empty() && model != "http_request" { return Err(format!( - "{context} only supports HTTP requests, but the payload has \"model\": \"{model}\". The CLI cannot work with {model} requests yet; use the Yaak app instead." + "{context} only supports HTTP requests, but the payload has \"model\": \"{model}\". The CLI cannot create {model} requests yet; use the Yaak app instead." )); } - let known: std::collections::BTreeSet = serde_json::to_value(schema_for!(HttpRequest)) + reject_unknown_fields::(payload, context, "an HTTP request", "http").map_err( + |error| { + format!("{error} If this is a gRPC or WebSocket request, the CLI cannot create it yet.") + }, + ) +} + +/// An update is a merge patch onto the stored request, so it must use that request's +/// model and fields. Anything else would be dropped silently on save. +fn reject_mismatched_update(payload: &Value, request: &AnyRequest) -> CommandResult { + let (model, noun, schema_type) = match request { + AnyRequest::HttpRequest(_) => ("http_request", "an HTTP request", "http"), + AnyRequest::GrpcRequest(_) => ("grpc_request", "a gRPC request", "grpc"), + AnyRequest::WebsocketRequest(_) => { + ("websocket_request", "a WebSocket request", "websocket") + } + }; + let context = "request update"; + if let Some(given) = payload.get("model").and_then(Value::as_str) + && !given.is_empty() + && given != model + { + return Err(format!( + "{context} cannot change a request's type: the stored request is {noun} (\"{model}\"), but the payload has \"model\": \"{given}\"." + )); + } + match request { + AnyRequest::HttpRequest(_) => { + reject_unknown_fields::(payload, context, noun, schema_type) + } + AnyRequest::GrpcRequest(_) => { + reject_unknown_fields::(payload, context, noun, schema_type) + } + AnyRequest::WebsocketRequest(_) => { + reject_unknown_fields::(payload, context, noun, schema_type) + } + } +} + +fn reject_unknown_fields( + payload: &Value, + context: &str, + noun: &str, + schema_type: &str, +) -> CommandResult { + let Some(object) = payload.as_object() else { + return Ok(()); + }; + + let known: std::collections::BTreeSet = serde_json::to_value(schema_for!(T)) .ok() .and_then(|schema| schema.get("properties").and_then(Value::as_object).cloned()) .map(|properties| properties.keys().cloned().collect()) @@ -140,7 +215,7 @@ fn reject_non_http_payload(payload: &Value, context: &str) -> CommandResult { if !unknown.is_empty() { return Err(format!( - "{context} got fields that are not part of an HTTP request: {}. If this is a gRPC or WebSocket request, the CLI cannot work with it yet. Run `yaak request schema http` for the valid fields.", + "{context} got fields that are not part of {noun}: {}. Run `yaak request schema {schema_type}` for the valid fields.", unknown.join(", ") )); } @@ -454,43 +529,67 @@ fn create( fn update(ctx: &CliContext, json: Option, json_input: Option) -> CommandResult { let patch = parse_required_json(json, json_input, "request update")?; let id = require_id(&patch, "request update")?; - reject_non_http_payload(&patch, "request update")?; - let existing = ctx - .db() - .get_http_request(&id) - .map_err(|e| format!("Failed to get request for update: {e}"))?; - let updated = apply_merge_patch(&existing, &patch, &id, "request update")?; + let existing = + get_any_request(ctx, &id).map_err(|e| format!("Failed to get request for update: {e}"))?; + reject_mismatched_update(&patch, &existing)?; - let saved = ctx - .query_manager() - .with_tx(|tx| tx.upsert_http_request(&updated, &UpdateSource::Sync)) - .map_err(|e| format!("Failed to update request: {e}"))?; + let context = "request update"; + let source = &UpdateSource::Sync; + let saved_id = match existing { + AnyRequest::HttpRequest(existing) => { + let updated = apply_merge_patch(&existing, &patch, &id, context)?; + ctx.query_manager().with_tx(|tx| tx.upsert_http_request(&updated, source)).map(|r| r.id) + } + AnyRequest::GrpcRequest(existing) => { + let updated = apply_merge_patch(&existing, &patch, &id, context)?; + ctx.query_manager().with_tx(|tx| tx.upsert_grpc_request(&updated, source)).map(|r| r.id) + } + AnyRequest::WebsocketRequest(existing) => { + let updated = apply_merge_patch(&existing, &patch, &id, context)?; + ctx.query_manager() + .with_tx(|tx| tx.upsert_websocket_request(&updated, source)) + .map(|r| r.id) + } + } + .map_err(|e| format!("Failed to update request: {e}"))?; - println!("Updated request: {}", saved.id); + println!("Updated request: {saved_id}"); Ok(()) } fn show(ctx: &CliContext, request_id: &str) -> CommandResult { - let request = - ctx.db().get_http_request(request_id).map_err(|e| format!("Failed to get request: {e}"))?; - let output = serde_json::to_string_pretty(&request) - .map_err(|e| format!("Failed to serialize request: {e}"))?; + let request = get_any_request(ctx, request_id)?; + let output = match &request { + AnyRequest::HttpRequest(r) => serde_json::to_string_pretty(r), + AnyRequest::GrpcRequest(r) => serde_json::to_string_pretty(r), + AnyRequest::WebsocketRequest(r) => serde_json::to_string_pretty(r), + } + .map_err(|e| format!("Failed to serialize request: {e}"))?; println!("{output}"); Ok(()) } fn delete(ctx: &CliContext, request_id: &str, yes: bool) -> CommandResult { + // Resolved before asking, so an unknown ID fails without a prompt + let request = get_any_request(ctx, request_id)?; if !yes && !confirm_delete("request", request_id) { println!("Aborted"); return Ok(()); } - let deleted = ctx + let source = &UpdateSource::Sync; + let deleted_id = ctx .query_manager() - .with_tx(|tx| tx.delete_http_request_by_id(request_id, &UpdateSource::Sync)) + .with_tx(|tx| match &request { + AnyRequest::HttpRequest(r) => tx.delete_http_request_by_id(&r.id, source).map(|r| r.id), + AnyRequest::GrpcRequest(r) => tx.delete_grpc_request_by_id(&r.id, source).map(|r| r.id), + AnyRequest::WebsocketRequest(r) => { + tx.delete_websocket_request_by_id(&r.id, source).map(|r| r.id) + } + }) .map_err(|e| format!("Failed to delete request: {e}"))?; - println!("Deleted request: {}", deleted.id); + println!("Deleted request: {deleted_id}"); Ok(()) } @@ -503,7 +602,7 @@ pub async fn send_request_by_id( verbose: bool, ) -> Result<(), String> { let request = - ctx.db().get_any_request(request_id).map_err(|e| format!("Failed to get request: {e}"))?; + get_any_request(ctx, request_id).map_err(|e| format!("Failed to get request: {e}"))?; match request { AnyRequest::HttpRequest(http_request) => { send_http_request_by_id( diff --git a/crates-cli/yaak-cli/src/commands/response.rs b/crates-cli/yaak-cli/src/commands/response.rs index 57177a53e..71b6c1930 100644 --- a/crates-cli/yaak-cli/src/commands/response.rs +++ b/crates-cli/yaak-cli/src/commands/response.rs @@ -1,9 +1,13 @@ use crate::cli::{ResponseArgs, ResponseCommands}; +use crate::commands::request::get_any_request; use crate::context::CliContext; use crate::utils::confirm::confirm_delete; use crate::utils::workspace::resolve_workspace_id; use std::io::Write; -use yaak_models::models::HttpResponse; +use yaak_models::models::{ + GrpcConnection, GrpcEventType, HttpResponse, WebsocketConnection, WebsocketEventType, +}; +use yaak_models::queries::any_request::AnyRequest; use yaak_models::util::UpdateSource; type CommandResult = std::result::Result; @@ -25,37 +29,124 @@ pub fn run(ctx: &CliContext, args: ResponseArgs) -> i32 { } } -/// Accepts a response ID, or a request ID meaning "the latest response for that request". -fn resolve_response(ctx: &CliContext, id: &str) -> CommandResult { - if let Ok(response) = ctx.db().get_http_response(id) { - return Ok(response); +/// What a request leaves behind when sent: an HTTP response, or the connection a gRPC or +/// WebSocket request opened, whose messages are its events. +enum StoredResponse { + Http(HttpResponse), + Grpc(GrpcConnection), + Websocket(WebsocketConnection), +} + +impl StoredResponse { + fn id(&self) -> &str { + match self { + StoredResponse::Http(r) => &r.id, + StoredResponse::Grpc(c) => &c.id, + StoredResponse::Websocket(c) => &c.id, + } } - if ctx.db().get_http_request(id).is_ok() { - return ctx - .db() - .list_http_responses_for_request(id, Some(1)) - .map_err(|e| format!("Failed to list responses: {e}"))? + fn created_at(&self) -> impl Ord + use<> { + match self { + StoredResponse::Http(r) => r.created_at, + StoredResponse::Grpc(c) => c.created_at, + StoredResponse::Websocket(c) => c.created_at, + } + } +} + +/// Accepts a response or connection ID, or a request ID meaning "the latest response for that +/// request". +fn resolve_response(ctx: &CliContext, id: &str) -> CommandResult { + let db = ctx.db(); + if let Ok(response) = db.get_http_response(id) { + return Ok(StoredResponse::Http(response)); + } + if let Ok(connection) = db.get_grpc_connection(id) { + return Ok(StoredResponse::Grpc(connection)); + } + if let Ok(connection) = db.get_websocket_connection(id) { + return Ok(StoredResponse::Websocket(connection)); + } + + let Ok(request) = get_any_request(ctx, id) else { + return Err(format!("Could not resolve ID '{id}' as a response or request")); + }; + let latest = list_for_request(ctx, &request, Some(1))?.into_iter().next(); + latest.ok_or_else(|| format!("Request {id} has no stored responses yet")) +} + +fn list_for_request( + ctx: &CliContext, + request: &AnyRequest, + limit: Option, +) -> CommandResult> { + let db = ctx.db(); + let error = |e: yaak_models::error::Error| format!("Failed to list responses: {e}"); + Ok(match request { + AnyRequest::HttpRequest(r) => db + .list_http_responses_for_request(&r.id, limit) + .map_err(error)? .into_iter() - .next() - .ok_or_else(|| format!("Request {id} has no stored responses yet")); - } + .map(StoredResponse::Http) + .collect(), + AnyRequest::GrpcRequest(r) => db + .list_grpc_connections_for_request(&r.id, limit) + .map_err(error)? + .into_iter() + .map(StoredResponse::Grpc) + .collect(), + AnyRequest::WebsocketRequest(r) => { + let connections = db.list_websocket_connections_for_request(&r.id).map_err(error)?; + let limit = limit.map_or(usize::MAX, |n| usize::try_from(n).unwrap_or(usize::MAX)); + connections.into_iter().take(limit).map(StoredResponse::Websocket).collect() + } + }) +} - Err(format!("Could not resolve ID '{id}' as a response or request")) +/// Every kind of stored response in a workspace, newest first, with `limit` applied to the +/// merged history rather than to each kind. +fn list_for_workspace( + ctx: &CliContext, + workspace_id: &str, + limit: Option, +) -> CommandResult> { + let db = ctx.db(); + let error = |e: yaak_models::error::Error| format!("Failed to list responses: {e}"); + // Each kind comes back newest first, so the newest `limit` overall are among the newest + // `limit` of each kind. + let mut responses = db + .list_http_responses(workspace_id, limit) + .map_err(error)? + .into_iter() + .map(StoredResponse::Http) + .chain( + db.list_grpc_connections(workspace_id) + .map_err(error)? + .into_iter() + .map(StoredResponse::Grpc), + ) + .chain( + db.list_websocket_connections(workspace_id) + .map_err(error)? + .into_iter() + .map(StoredResponse::Websocket), + ) + .collect::>(); + responses.sort_by_key(|r| std::cmp::Reverse(r.created_at())); + if let Some(limit) = limit { + responses.truncate(usize::try_from(limit).unwrap_or(usize::MAX)); + } + Ok(responses) } fn list(ctx: &CliContext, id: Option<&str>, limit: Option) -> CommandResult { // An explicit request ID scopes to that request; anything else lists the workspace. - let responses = match id { - Some(id) if ctx.db().get_http_request(id).is_ok() => ctx - .db() - .list_http_responses_for_request(id, limit) - .map_err(|e| format!("Failed to list responses: {e}"))?, - other => { - let workspace_id = resolve_workspace_id(ctx, other, "response list")?; - ctx.db() - .list_http_responses(&workspace_id, limit) - .map_err(|e| format!("Failed to list responses: {e}"))? + let responses = match id.map(|id| get_any_request(ctx, id)) { + Some(Ok(request)) => list_for_request(ctx, &request, limit)?, + _ => { + let workspace_id = resolve_workspace_id(ctx, id, "response list")?; + list_for_workspace(ctx, &workspace_id, limit)? } }; @@ -65,7 +156,20 @@ fn list(ctx: &CliContext, id: Option<&str>, limit: Option) -> CommandResult } for response in responses { - let status = match response.error { + let (error, status, url, elapsed) = match &response { + StoredResponse::Http(r) => { + let reason = r.status_reason.clone().unwrap_or_default(); + let status = format!("{} {}", r.status, reason).trim_end().to_string(); + (r.error.as_deref(), status, r.url.as_str(), r.elapsed) + } + StoredResponse::Grpc(c) => { + (c.error.as_deref(), format!("gRPC status {}", c.status), c.url.as_str(), c.elapsed) + } + StoredResponse::Websocket(c) => { + (c.error.as_deref(), c.status.to_string(), c.url.as_str(), c.elapsed) + } + }; + let status = match error { // Errors can be a paragraph of nested debug output; `show` has the full text. Some(error) => { let first_line = error.lines().next().unwrap_or_default(); @@ -75,34 +179,70 @@ fn list(ctx: &CliContext, id: Option<&str>, limit: Option) -> CommandResult }; format!("ERROR {summary}") } - None => { - let reason = response.status_reason.unwrap_or_default(); - format!("{} {}", response.status, reason).trim_end().to_string() - } + None => status, }; - println!("{} - {} {} ({}ms)", response.id, status, response.url, response.elapsed); + println!("{} - {} {} ({}ms)", response.id(), status, url, elapsed); } Ok(()) } fn show(ctx: &CliContext, id: &str) -> CommandResult { - let response = resolve_response(ctx, id)?; - let output = serde_json::to_string_pretty(&response) - .map_err(|e| format!("Failed to serialize response: {e}"))?; + let output = match resolve_response(ctx, id)? { + StoredResponse::Http(r) => serde_json::to_string_pretty(&r), + StoredResponse::Grpc(c) => serde_json::to_string_pretty(&c), + StoredResponse::Websocket(c) => serde_json::to_string_pretty(&c), + } + .map_err(|e| format!("Failed to serialize response: {e}"))?; println!("{output}"); Ok(()) } +/// An HTTP body as stored; for a gRPC or WebSocket connection, each message the server sent, +/// oldest first, one per line. fn body(ctx: &CliContext, id: &str) -> CommandResult { - let response = resolve_response(ctx, id)?; - let Some(body_path) = response.body_path else { - return Err(format!("Response {} has no stored body", response.id)); + let bytes = match resolve_response(ctx, id)? { + StoredResponse::Http(response) => { + let Some(body_path) = response.body_path else { + return Err(format!("Response {} has no stored body", response.id)); + }; + std::fs::read(&body_path) + .map_err(|e| format!("Failed to read response body at {body_path}: {e}"))? + } + StoredResponse::Grpc(connection) => { + let mut events = ctx + .db() + .list_grpc_events(&connection.id) + .map_err(|e| format!("Failed to list connection events: {e}"))?; + events.sort_by_key(|e| e.created_at); + let mut bytes = Vec::new(); + for event in events.iter().filter(|e| e.event_type == GrpcEventType::ServerMessage) { + bytes.extend_from_slice(event.content.as_bytes()); + bytes.push(b'\n'); + } + bytes + } + StoredResponse::Websocket(connection) => { + let mut events = ctx + .db() + .list_websocket_events(&connection.id) + .map_err(|e| format!("Failed to list connection events: {e}"))?; + events.sort_by_key(|e| e.created_at); + let mut bytes = Vec::new(); + for event in events.iter().filter(|e| { + e.is_server + && matches!( + e.message_type, + WebsocketEventType::Text | WebsocketEventType::Binary + ) + }) { + bytes.extend_from_slice(&event.message); + bytes.push(b'\n'); + } + bytes + } }; - let bytes = std::fs::read(&body_path) - .map_err(|e| format!("Failed to read response body at {body_path}: {e}"))?; - let mut stdout = std::io::stdout(); stdout.write_all(&bytes).map_err(|e| format!("Failed to write response body: {e}"))?; stdout.flush().map_err(|e| format!("Failed to flush response body: {e}"))?; @@ -112,14 +252,29 @@ fn body(ctx: &CliContext, id: &str) -> CommandResult { fn delete(ctx: &CliContext, id: &str, yes: bool) -> CommandResult { // Deleting is scoped to a whole request or workspace, since a single stored // response is rarely the thing someone wants to remove. - if ctx.db().get_http_request(id).is_ok() { + if let Ok(request) = get_any_request(ctx, id) { if !yes && !confirm_delete("responses for request", id) { println!("Aborted"); return Ok(()); } + let source = &UpdateSource::Sync; let count = ctx .query_manager() - .with_tx(|tx| tx.delete_all_http_responses_for_request(id, &UpdateSource::Sync)) + .with_tx(|tx| match &request { + AnyRequest::HttpRequest(r) => { + tx.delete_all_http_responses_for_request(&r.id, source) + } + AnyRequest::GrpcRequest(r) => { + let count = tx.list_grpc_connections_for_request(&r.id, None)?.len(); + tx.delete_all_grpc_connections_for_request(&r.id, source)?; + Ok(count) + } + AnyRequest::WebsocketRequest(r) => { + let count = tx.list_websocket_connections_for_request(&r.id)?.len(); + tx.delete_all_websocket_connections_for_request(&r.id, source)?; + Ok(count) + } + }) .map_err(|e| format!("Failed to delete responses: {e}"))?; println!("Deleted {count} responses for request {id}"); return Ok(()); @@ -133,7 +288,13 @@ fn delete(ctx: &CliContext, id: &str, yes: bool) -> CommandResult { let count = ctx .query_manager() .with_tx(|tx| { - tx.delete_all_http_responses_for_workspace(&workspace_id, &UpdateSource::Sync) + let source = &UpdateSource::Sync; + let grpc = tx.list_grpc_connections(&workspace_id)?.len(); + let websocket = tx.list_websocket_connections(&workspace_id)?.len(); + let http = tx.delete_all_http_responses_for_workspace(&workspace_id, source)?; + tx.delete_all_grpc_connections_for_workspace(&workspace_id, source)?; + tx.delete_all_websocket_connections_for_workspace(&workspace_id, source)?; + Ok::<_, yaak_models::error::Error>(http + grpc + websocket) }) .map_err(|e| format!("Failed to delete responses: {e}"))?; println!("Deleted {count} responses for workspace {workspace_id}"); diff --git a/crates-cli/yaak-cli/src/commands/send.rs b/crates-cli/yaak-cli/src/commands/send.rs index ba96235c1..2c991ff14 100644 --- a/crates-cli/yaak-cli/src/commands/send.rs +++ b/crates-cli/yaak-cli/src/commands/send.rs @@ -179,6 +179,22 @@ async fn send_many( let mut success_count = 0usize; let mut failures: Vec<(String, String)> = Vec::new(); + // The CLI sends HTTP only, so the rest of a folder is reported, not counted as failing. + let mut skipped: Vec = Vec::new(); + let mut request_ids = request_ids; + request_ids.retain(|id| match request::get_any_request(ctx, id) { + Ok(AnyRequest::GrpcRequest(r)) => { + skipped.push(format!("{id} - gRPC {}", r.name)); + false + } + Ok(AnyRequest::WebsocketRequest(r)) => { + skipped.push(format!("{id} - WebSocket {}", r.name)); + false + } + // A request that has gone missing since it was listed fails when sent, as before + Ok(AnyRequest::HttpRequest(_)) | Err(_) => true, + }); + match mode { ExecutionMode::Sequential => { for request_id in request_ids { @@ -229,7 +245,18 @@ async fn send_many( } let failure_count = failures.len(); - println!("Send summary: {success_count} succeeded, {failure_count} failed"); + if skipped.is_empty() { + println!("Send summary: {success_count} succeeded, {failure_count} failed"); + } else { + println!( + "Send summary: {success_count} succeeded, {failure_count} failed, {} skipped", + skipped.len() + ); + println!("Skipped, since the CLI can only send HTTP requests so far:"); + for line in &skipped { + println!(" {line}"); + } + } if failure_count == 0 { return Ok(()); diff --git a/crates-cli/yaak-cli/tests/import_export_commands.rs b/crates-cli/yaak-cli/tests/import_export_commands.rs index 922e04d53..59302bec8 100644 --- a/crates-cli/yaak-cli/tests/import_export_commands.rs +++ b/crates-cli/yaak-cli/tests/import_export_commands.rs @@ -1,6 +1,6 @@ mod common; -use common::{cli_cmd, parse_created_id, query_manager, seed_request}; +use common::{cli_cmd, parse_created_id, query_manager, seed_folder, seed_request, seed_workspace}; use predicates::str::contains; use serde_json::Value; use tempfile::TempDir; @@ -322,3 +322,249 @@ fn re_import_leaves_deleted_resources_alone() { assert_eq!(requests.len(), 1, "a deleted request must not come back: {requests:?}"); assert_eq!(requests[0].name, "Request A"); } + +fn path_str(path: &std::path::Path) -> &str { + path.to_str().expect("path is utf-8") +} + +fn created_id(data_dir: &std::path::Path, args: &[&str]) -> String { + let assert = cli_cmd(data_dir).args(args).assert().success(); + parse_created_id(&assert.get_output().stdout, &args.join(" ")) +} + +/// A workspace made the way the skill describes: base and Staging environments and a request. +/// Returns the workspace, base environment, Staging environment, and request IDs. +fn cli_made_workspace(data_dir: &std::path::Path) -> (String, String, String, String) { + let workspace_id = created_id(data_dir, &["workspace", "create", "--name", "CI WS"]); + let list = cli_cmd(data_dir).args(["environment", "list", &workspace_id]).assert().success(); + let base_id = String::from_utf8_lossy(&list.get_output().stdout) + .lines() + .find(|l| l.ends_with("(workspace)")) + .and_then(|l| l.split_whitespace().next()) + .expect("base environment listed") + .to_string(); + let staging_id = + created_id(data_dir, &["environment", "create", &workspace_id, "--name", "Staging"]); + let request_id = created_id( + data_dir, + &[ + "request", + "create", + &workspace_id, + "--name", + "Health", + "--url", + "https://example.com", + ], + ); + (workspace_id, base_id, staging_id, request_id) +} + +fn read_json(path: &std::path::Path) -> Value { + serde_json::from_str(&std::fs::read_to_string(path).expect("read file")).expect("JSON") +} + +/// QA sweep bug 2: `yaak import req_in_existing_folder.json --workspace-id ` put requests +/// that name an existing folder of W at the workspace root. +#[test] +fn import_keeps_a_folder_id_that_names_an_existing_folder() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let data_dir = temp_dir.path(); + let import_path = temp_dir.path().join("req_in_existing_folder.json"); + seed_workspace(data_dir, "wk_target"); + seed_folder(data_dir, "wk_target", "fl_existing"); + + std::fs::write( + &import_path, + r#"{"yaakVersion":"test","yaakSchema":4,"resources":{"workspaces":[],"environments":[],"folders":[], +"httpRequests":[{"model":"http_request","id":"rq_newinfold1","workspaceId":"wk_target","folderId":"fl_existing","name":"Into existing","method":"GET","url":"http://127.0.0.1/x"}], +"grpcRequests":[{"model":"grpc_request","id":"gr_newinfold1","workspaceId":"wk_target","folderId":"fl_existing","name":"gRPC into existing","url":"127.0.0.1:50051"}], +"websocketRequests":[]}}"#, + ) + .expect("write fixture"); + + cli_cmd(data_dir) + .args([ + "import", + path_str(&import_path), + "--workspace-id", + "wk_target", + ]) + .assert() + .success() + .stdout(contains("Imported 1 HTTP request, 1 gRPC request")); + + let query_manager = query_manager(data_dir); + let db = query_manager.connect(); + let http = db.list_http_requests("wk_target").expect("list HTTP"); + let grpc = db.list_grpc_requests("wk_target").expect("list gRPC"); + assert_eq!(http[0].folder_id.as_deref(), Some("fl_existing"), "{http:?}"); + assert_eq!(grpc[0].folder_id.as_deref(), Some("fl_existing"), "{grpc:?}"); +} + +/// QA sweep bug 3: `yaak export e.json `, edit the file, `yaak import e.json --workspace-id +/// ` made copies with new IDs, a second base environment, and "(Imported)" environments. +#[test] +fn re_importing_an_edited_export_updates_the_workspace_in_place() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let data_dir = temp_dir.path(); + let export_path = temp_dir.path().join("exp.json"); + let (workspace_id, base_id, staging_id, request_id) = cli_made_workspace(data_dir); + + cli_cmd(data_dir) + .args([ + "export", + path_str(&export_path), + &workspace_id, + "--include-private", + ]) + .assert() + .success(); + let mut exported = read_json(&export_path); + exported["resources"]["httpRequests"][0]["name"] = "Health (edited)".into(); + std::fs::write(&export_path, exported.to_string()).expect("write edited export"); + + cli_cmd(data_dir) + .args([ + "import", + path_str(&export_path), + "--workspace-id", + &workspace_id, + ]) + .assert() + .success() + .stdout(contains("Imported 1 HTTP request")); + + let query_manager = query_manager(data_dir); + let db = query_manager.connect(); + let requests = db.list_http_requests(&workspace_id).expect("list requests"); + assert_eq!(requests.len(), 1, "no copies: {requests:?}"); + assert_eq!(requests[0].id, request_id); + assert_eq!(requests[0].name, "Health (edited)"); + + let mut environment_ids = + db.list_environments(&workspace_id).expect("list environments").into_iter().map(|e| e.id); + let mut expected = vec![base_id.clone(), staging_id]; + expected.sort(); + let mut actual = environment_ids.by_ref().collect::>(); + actual.sort(); + assert_eq!(actual, expected, "no second base environment and no Imported copies"); + assert_eq!(db.get_base_environment(&workspace_id).expect("base").id, base_id); +} + +/// QA sweep bug 7: the skill's CI recipe, `yaak export` then `yaak import` into a fresh +/// `--data-dir`, gave every model a new ID, so the IDs in the recipe no longer worked. +#[test] +fn import_into_a_fresh_data_dir_keeps_the_ids() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let data_dir = temp_dir.path().join("dev"); + let ci_dir = temp_dir.path().join("ci"); + let export_path = temp_dir.path().join("ci-export.json"); + let (workspace_id, base_id, staging_id, request_id) = cli_made_workspace(&data_dir); + + cli_cmd(&data_dir) + .args([ + "export", + path_str(&export_path), + &workspace_id, + "--include-private", + ]) + .assert() + .success(); + cli_cmd(&ci_dir).args(["import", path_str(&export_path)]).assert().success(); + + let query_manager = query_manager(&ci_dir); + let db = query_manager.connect(); + assert_eq!(db.get_workspace(&workspace_id).expect("workspace keeps its ID").name, "CI WS"); + assert_eq!(db.get_http_request(&request_id).expect("request keeps its ID").name, "Health"); + assert_eq!(db.get_base_environment(&workspace_id).expect("base").id, base_id); + assert_eq!(db.get_environment(&staging_id).expect("Staging keeps its ID").name, "Staging"); +} + +/// QA sweep bug 8: there was no `--include-private`, so the environments the CLI makes, which +/// are private like the app's, could not be exported without the long flag. +#[test] +fn export_includes_cli_made_environments_only_with_include_private() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let data_dir = temp_dir.path(); + let export_path = temp_dir.path().join("export.json"); + let (workspace_id, base_id, staging_id, _) = cli_made_workspace(data_dir); + + let exported_ids = |args: &[&str]| { + cli_cmd(data_dir).args(["export", path_str(&export_path)]).args(args).assert().success(); + let mut ids = read_json(&export_path)["resources"]["environments"] + .as_array() + .expect("environments") + .iter() + .map(|e| e["id"].as_str().expect("id").to_string()) + .collect::>(); + ids.sort(); + ids + }; + + assert!(exported_ids(&[&workspace_id]).is_empty(), "CLI-made environments are private"); + + let mut all = vec![base_id, staging_id]; + all.sort(); + assert_eq!(exported_ids(&[&workspace_id, "--include-private"]), all); + assert_eq!(exported_ids(&[&workspace_id, "--include-private-environments"]), all); +} + +/// QA sweep bug 15: the planner's warnings, such as IDs taken by another workspace, were +/// computed and then never printed. +#[test] +fn import_prints_the_plan_warnings_after_the_counts() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let data_dir = temp_dir.path(); + let export_path = temp_dir.path().join("e9.json"); + let (workspace_id, ..) = cli_made_workspace(data_dir); + let other_id = created_id(data_dir, &["workspace", "create", "--name", "Other WS"]); + cli_cmd(data_dir) + .args([ + "export", + path_str(&export_path), + &workspace_id, + "--include-private", + ]) + .assert() + .success(); + + cli_cmd(data_dir) + .args(["import", path_str(&export_path), "--workspace-id", &other_id]) + .assert() + .success() + .stdout(contains("Imported 2 environments, 1 HTTP request")) + .stderr(contains( + "Warning: New IDs assigned: 3 resources already exist in CI WS, so the imported copies get new IDs", + )) + .stderr(contains("Warning: Base environment kept separate:")); + + cli_cmd(data_dir) + .args(["import", path_str(&export_path)]) + .assert() + .success() + .stderr(contains("Warning: New IDs assigned: 4 resources already exist in CI WS")); +} + +/// QA sweep bug 16: `yaak import e_two.json --workspace-id ` merged both workspaces of the +/// file into W. +#[test] +fn import_with_workspace_id_refuses_a_file_with_several_workspaces() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let data_dir = temp_dir.path(); + let export_path = temp_dir.path().join("e_two.json"); + let first = created_id(data_dir, &["workspace", "create", "--name", "First"]); + let second = created_id(data_dir, &["workspace", "create", "--name", "Second"]); + seed_request(data_dir, &first, "rq_first"); + cli_cmd(data_dir).args(["export", path_str(&export_path), &first, &second]).assert().success(); + let target = created_id(data_dir, &["workspace", "create", "--name", "Target"]); + + cli_cmd(data_dir) + .args(["import", path_str(&export_path), "--workspace-id", &target]) + .assert() + .failure() + .stderr(contains("contains 2 workspaces")) + .stderr(contains("without --workspace-id to import each as its own workspace")); + let query_manager = query_manager(data_dir); + assert!(query_manager.connect().list_http_requests(&target).expect("list").is_empty()); +} diff --git a/crates-cli/yaak-cli/tests/mcp_command.rs b/crates-cli/yaak-cli/tests/mcp_command.rs index 7f892e08d..2916f5de0 100644 --- a/crates-cli/yaak-cli/tests/mcp_command.rs +++ b/crates-cli/yaak-cli/tests/mcp_command.rs @@ -223,6 +223,47 @@ async fn mcp_send_settings_reach_template_functions() { assert!(child.wait().await.unwrap().success()); } +/// QA sweep bug 6: the CLI has no window, so `show_toast` prints the toast to stderr (stdout is +/// the protocol channel) the way the CLI prints plugin toasts, and reports it as shown. +#[tokio::test] +async fn mcp_show_toast_prints_the_message_to_stderr() { + use tokio::io::AsyncReadExt; + + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let data_dir = temp_dir.path(); + seed_workspace(data_dir, "wk_test"); + let mut child = Command::new(env!("CARGO_BIN_EXE_yaak")) + .arg("--data-dir") + .arg(data_dir) + .arg("mcp") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true) + .spawn() + .unwrap(); + let mut stderr = child.stderr.take().unwrap(); + let io = (child.stdout.take().unwrap(), child.stdin.take().unwrap()); + let client = ().serve(io).await.unwrap(); + + let arguments = json!({ "message": "Build finished" }); + let Value::Object(arguments) = arguments else { + unreachable!() + }; + let result = client + .call_tool(CallToolRequestParams::new("show_toast").with_arguments(arguments)) + .await + .unwrap(); + assert_ne!(result.is_error, Some(true), "{result:?}"); + assert_eq!(result.content[0].as_text().unwrap().text, "✓ Toast shown: \"Build finished\""); + + client.cancel().await.unwrap(); + child.kill().await.unwrap(); + let mut output = String::new(); + stderr.read_to_string(&mut output).await.unwrap(); + assert!(output.lines().any(|l| l == "[mcp] Build finished"), "stderr: {output}"); +} + /// Spawns `yaak mcp --http` with `args` and returns the child, the URL it /// printed, and the token it printed or was given. async fn spawn_http(data_dir: &std::path::Path, args: &[&str]) -> (tokio::process::Child, String, String) { @@ -283,7 +324,9 @@ async fn post( .last() .map(str::trim) .unwrap_or(text.trim()); - (status, headers, serde_json::from_str(message).ok()) + // A plain-text body, such as the reason for a 500, comes back as a string to show + let body = serde_json::from_str(message).unwrap_or_else(|_| Value::String(text.clone())); + (status, headers, Some(body)) } #[tokio::test] @@ -309,7 +352,7 @@ async fn mcp_http_requires_the_token() { let auth = Some("Bearer s3cret"); let (status, headers, body) = post(&client, &url, auth, None, initialize).await; - assert_eq!(status, reqwest::StatusCode::OK); + assert_eq!(status, reqwest::StatusCode::OK, "initialize answered {body:?}"); let session = headers["mcp-session-id"].to_str().unwrap().to_string(); assert_eq!(body.unwrap()["result"]["serverInfo"]["name"], "yaak-mcp-server"); diff --git a/crates-cli/yaak-cli/tests/request_commands.rs b/crates-cli/yaak-cli/tests/request_commands.rs index cdb78c1fb..4b97318ce 100644 --- a/crates-cli/yaak-cli/tests/request_commands.rs +++ b/crates-cli/yaak-cli/tests/request_commands.rs @@ -289,3 +289,81 @@ fn request_send_websocket_returns_explicit_nyi_error() { .code(1) .stderr(contains("WebSocket request send is not implemented yet in yaak-cli")); } + +/// QA sweep bug 1: `request list`, `show`, `update` and `delete` were HTTP-only, so gRPC and +/// WebSocket requests were missing from the list and failed everywhere else. +#[test] +fn request_commands_handle_grpc_and_websocket_requests() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let data_dir = temp_dir.path(); + seed_workspace(data_dir, "wk_test"); + seed_request(data_dir, "wk_test", "rq_types"); + seed_grpc_request(data_dir, "wk_test", "gr_types"); + seed_websocket_request(data_dir, "wk_test", "wr_types"); + + cli_cmd(data_dir) + .args(["request", "list", "wk_test"]) + .assert() + .success() + .stdout(contains("rq_types - GET Seeded Request")) + .stdout(contains("gr_types - gRPC Seeded gRPC Request")) + .stdout(contains("wr_types - WebSocket Seeded WebSocket Request")); + + cli_cmd(data_dir) + .args(["request", "show", "gr_types"]) + .assert() + .success() + .stdout(contains("\"model\": \"grpc_request\"")); + cli_cmd(data_dir) + .args(["request", "show", "wr_types"]) + .assert() + .success() + .stdout(contains("\"model\": \"websocket_request\"")); + + cli_cmd(data_dir) + .args([ + "request", + "update", + r#"{"id":"wr_types","name":"Renamed Socket"}"#, + ]) + .assert() + .success() + .stdout(contains("Updated request: wr_types")); + cli_cmd(data_dir) + .args([ + "request", + "update", + r#"{"id":"gr_types","service":"pkg.Greeter"}"#, + ]) + .assert() + .success(); + cli_cmd(data_dir) + .args([ + "request", + "update", + r#"{"id":"gr_types","method":"GET","bodyType":"other"}"#, + ]) + .assert() + .failure() + .stderr(contains("not part of a gRPC request: bodyType")); + { + let query_manager = query_manager(data_dir); + let db = query_manager.connect(); + assert_eq!(db.get_websocket_request("wr_types").unwrap().name, "Renamed Socket"); + assert_eq!( + db.get_grpc_request("gr_types").unwrap().service.as_deref(), + Some("pkg.Greeter") + ); + } + + cli_cmd(data_dir).args(["request", "delete", "gr_types", "--yes"]).assert().success(); + cli_cmd(data_dir).args(["request", "delete", "wr_types", "--yes"]).assert().success(); + cli_cmd(data_dir) + .args(["request", "show", "wr_types"]) + .assert() + .failure() + .stderr(contains("Request wr_types not found")); + let query_manager = query_manager(data_dir); + assert!(query_manager.connect().get_grpc_request("gr_types").is_err()); + assert!(query_manager.connect().get_websocket_request("wr_types").is_err()); +} diff --git a/crates-cli/yaak-cli/tests/response_commands.rs b/crates-cli/yaak-cli/tests/response_commands.rs new file mode 100644 index 000000000..e09d7ea47 --- /dev/null +++ b/crates-cli/yaak-cli/tests/response_commands.rs @@ -0,0 +1,202 @@ +mod common; + +use common::{ + cli_cmd, query_manager, seed_grpc_request, seed_request, seed_websocket_request, seed_workspace, +}; +use predicates::prelude::PredicateBooleanExt; +use predicates::str::contains; +use serde_json::json; +use tempfile::TempDir; +use yaak_models::models::{ + GrpcConnection, GrpcEvent, GrpcEventType, HttpResponse, WebsocketConnection, WebsocketEvent, + WebsocketEventType, +}; +use yaak_models::util::UpdateSource; + +/// QA sweep bug 4: `response list/show/body/delete` given a `gr_` or `wr_` ID could not +/// find the request, so its connections were unreachable. +#[test] +fn response_commands_handle_grpc_and_websocket_connections() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let data_dir = temp_dir.path(); + seed_workspace(data_dir, "wk_test"); + seed_grpc_request(data_dir, "wk_test", "gr_conn"); + seed_websocket_request(data_dir, "wk_test", "wr_conn"); + + query_manager(data_dir) + .with_tx(|tx| { + let source = &UpdateSource::Sync; + let ws = tx.upsert_websocket_connection( + &WebsocketConnection { + id: "wc_conn".into(), + workspace_id: "wk_test".into(), + request_id: "wr_conn".into(), + url: "wss://example.com/socket".into(), + status: 101, + ..Default::default() + }, + source, + )?; + for (is_server, message) in [(false, "ping me"), (true, "hello"), (true, "world")] { + tx.upsert_websocket_event( + &WebsocketEvent { + workspace_id: "wk_test".into(), + request_id: "wr_conn".into(), + connection_id: ws.id.clone(), + is_server, + message: message.as_bytes().to_vec(), + message_type: WebsocketEventType::Text, + ..Default::default() + }, + source, + )?; + } + let grpc = tx.upsert_grpc_connection( + &GrpcConnection { + id: "gc_conn".into(), + workspace_id: "wk_test".into(), + request_id: "gr_conn".into(), + url: "https://example.com".into(), + ..Default::default() + }, + source, + )?; + tx.upsert_grpc_event( + &GrpcEvent { + workspace_id: "wk_test".into(), + request_id: "gr_conn".into(), + connection_id: grpc.id.clone(), + content: "{\"message\":\"hi\"}".into(), + event_type: GrpcEventType::ServerMessage, + ..Default::default() + }, + source, + ) + }) + .expect("seed connections"); + + cli_cmd(data_dir) + .args(["response", "list", "wr_conn"]) + .assert() + .success() + .stdout(contains("wc_conn - 101 wss://example.com/socket")); + cli_cmd(data_dir) + .args(["response", "list", "gr_conn"]) + .assert() + .success() + .stdout(contains("gc_conn - gRPC status 0 https://example.com")); + + cli_cmd(data_dir) + .args(["response", "show", "wr_conn"]) + .assert() + .success() + .stdout(contains("\"model\": \"websocket_connection\"")); + cli_cmd(data_dir) + .args(["response", "show", "gc_conn"]) + .assert() + .success() + .stdout(contains("\"model\": \"grpc_connection\"")); + + cli_cmd(data_dir) + .args(["response", "body", "wr_conn"]) + .assert() + .success() + .stdout("hello\nworld\n"); + cli_cmd(data_dir) + .args(["response", "body", "gr_conn"]) + .assert() + .success() + .stdout("{\"message\":\"hi\"}\n"); + + cli_cmd(data_dir) + .args(["response", "delete", "wr_conn", "--yes"]) + .assert() + .success() + .stdout(contains("Deleted 1 responses for request wr_conn")); + cli_cmd(data_dir) + .args(["response", "delete", "gr_conn", "--yes"]) + .assert() + .success() + .stdout(contains("Deleted 1 responses for request gr_conn")); + let query_manager = query_manager(data_dir); + assert!(query_manager.connect().get_websocket_connection("wc_conn").is_err()); + assert!(query_manager.connect().get_grpc_connection("gc_conn").is_err()); +} + +/// `response list ` and `response delete ` cover HTTP responses and +/// gRPC and WebSocket connections together, newest first, with `--limit` applied to the +/// merged history. +#[test] +fn workspace_response_commands_cover_every_kind() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let data_dir = temp_dir.path(); + seed_workspace(data_dir, "wk_test"); + seed_request(data_dir, "wk_test", "rq_mixed"); + seed_grpc_request(data_dir, "wk_test", "gr_mixed"); + seed_websocket_request(data_dir, "wk_test", "wr_mixed"); + + let at = |second: u32| format!("2026-01-01T00:00:{second:02}"); + let (seed_db, blob_manager, _rx) = + yaak_models::init_standalone(data_dir.join("db.sqlite"), data_dir.join("blobs.sqlite")) + .expect("open database"); + seed_db + .with_tx(|tx| { + let source = &UpdateSource::Sync; + tx.upsert_http_response( + &serde_json::from_value::(json!({ + "id": "rs_oldest", "workspaceId": "wk_test", "requestId": "rq_mixed", + "createdAt": at(1), "updatedAt": at(1), "status": 200, + "url": "https://example.com/oldest", + })) + .unwrap(), + source, + &blob_manager, + )?; + tx.upsert_websocket_connection( + &serde_json::from_value::(json!({ + "id": "wc_middle", "workspaceId": "wk_test", "requestId": "wr_mixed", + "createdAt": at(2), "updatedAt": at(2), "status": 101, + "url": "wss://example.com/socket", + })) + .unwrap(), + source, + )?; + tx.upsert_grpc_connection( + &serde_json::from_value::(json!({ + "id": "gc_newest", "workspaceId": "wk_test", "requestId": "gr_mixed", + "createdAt": at(3), "updatedAt": at(3), "url": "https://example.com", + })) + .unwrap(), + source, + )?; + Ok::<_, yaak_models::error::Error>(()) + }) + .expect("seed history"); + drop(seed_db); + + let listed = cli_cmd(data_dir).args(["response", "list", "wk_test"]).assert().success(); + let ids = String::from_utf8_lossy(&listed.get_output().stdout) + .lines() + .map(|l| l.split_whitespace().next().unwrap_or_default().to_string()) + .collect::>(); + assert_eq!(ids, ["gc_newest", "wc_middle", "rs_oldest"]); + + cli_cmd(data_dir) + .args(["response", "list", "wk_test", "--limit", "2"]) + .assert() + .success() + .stdout(contains("gc_newest")) + .stdout(contains("wc_middle")) + .stdout(contains("rs_oldest").not()); + + cli_cmd(data_dir) + .args(["response", "delete", "wk_test", "--yes"]) + .assert() + .success() + .stdout(contains("Deleted 3 responses for workspace wk_test")); + let query_manager = query_manager(data_dir); + let db = query_manager.connect(); + assert!(db.list_http_responses("wk_test", None).unwrap().is_empty()); + assert!(db.list_grpc_connections("wk_test").unwrap().is_empty()); + assert!(db.list_websocket_connections("wk_test").unwrap().is_empty()); +} diff --git a/crates-cli/yaak-cli/tests/send_commands.rs b/crates-cli/yaak-cli/tests/send_commands.rs index 2ff09b4bf..c2ab1d151 100644 --- a/crates-cli/yaak-cli/tests/send_commands.rs +++ b/crates-cli/yaak-cli/tests/send_commands.rs @@ -4,7 +4,7 @@ use common::http_server::TestHttpServer; use common::{cli_cmd, query_manager, seed_folder, seed_workspace}; use predicates::str::contains; use tempfile::TempDir; -use yaak_models::models::HttpRequest; +use yaak_models::models::{GrpcRequest, HttpRequest, WebsocketRequest}; use yaak_models::util::UpdateSource; #[test] @@ -48,3 +48,60 @@ fn top_level_send_unknown_id_fails_with_clear_error() { .code(1) .stderr(contains("Could not resolve ID 'does_not_exist' as request, folder, or workspace")); } + +/// QA sweep bug 5: `yaak send ` with a gRPC and a WebSocket request in the folder +/// reported them as failures and exited 1, though every HTTP request succeeded. +#[test] +fn folder_send_skips_grpc_and_websocket_requests() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let data_dir = temp_dir.path(); + seed_workspace(data_dir, "wk_test"); + seed_folder(data_dir, "wk_test", "fl_test"); + + let server = TestHttpServer::spawn_ok("mixed folder"); + query_manager(data_dir) + .with_tx(|tx| { + let source = &UpdateSource::Sync; + tx.upsert_http_request( + &HttpRequest { + id: "rq_mixed".to_string(), + workspace_id: "wk_test".to_string(), + folder_id: Some("fl_test".to_string()), + name: "HTTP One".to_string(), + url: server.url.clone(), + ..Default::default() + }, + source, + )?; + tx.upsert_grpc_request( + &GrpcRequest { + id: "gr_mixed".to_string(), + workspace_id: "wk_test".to_string(), + folder_id: Some("fl_test".to_string()), + name: "Greeter".to_string(), + ..Default::default() + }, + source, + )?; + tx.upsert_websocket_request( + &WebsocketRequest { + id: "wr_mixed".to_string(), + workspace_id: "wk_test".to_string(), + folder_id: Some("fl_test".to_string()), + name: "Socket".to_string(), + ..Default::default() + }, + source, + ) + }) + .expect("Failed to seed folder requests"); + + cli_cmd(data_dir) + .args(["send", "fl_test"]) + .assert() + .success() + .stdout(contains("mixed folder")) + .stdout(contains("Send summary: 1 succeeded, 0 failed, 2 skipped")) + .stdout(contains("gr_mixed - gRPC Greeter")) + .stdout(contains("wr_mixed - WebSocket Socket")); +} diff --git a/crates/yaak-mcp/src/server.rs b/crates/yaak-mcp/src/server.rs index 6501f89db..ab53559dd 100644 --- a/crates/yaak-mcp/src/server.rs +++ b/crates/yaak-mcp/src/server.rs @@ -66,6 +66,27 @@ impl YaakMcpServer { )) } + /// The HTTP request `id` names, or an error that says why there is none. These tools + /// handle HTTP requests only, so a gRPC or WebSocket ID is named as such. + fn require_http_request(&self, id: &str) -> HostResult { + if let Some(request) = self.host.get_http_request(id)? { + return Ok(request); + } + let other_type = if id.starts_with("gr_") { + Some("a gRPC request") + } else if id.starts_with("wr_") { + Some("a WebSocket request") + } else { + None + }; + Err(match other_type { + Some(kind) => format!( + "{id} is {kind}, and these tools only work with HTTP requests. Use the Yaak app or the `yaak request` CLI commands for it." + ), + None => format!("HTTP request with ID {id} not found"), + }) + } + fn find_folder(&self, workspace_id: &str, id: &str) -> HostResult> { Ok(self.host.list_folders(workspace_id)?.into_iter().find(|f| f.id == id)) } @@ -124,7 +145,7 @@ impl YaakMcpServer { Parameters(p): Parameters, ) -> ToolResult { self.resolve_workspace(p.workspace_id.as_deref())?; - to_json(&self.host.get_http_request(&p.id)?) + to_json(&self.require_http_request(&p.id)?) } #[tool( @@ -137,10 +158,7 @@ impl YaakMcpServer { Parameters(p): Parameters, ) -> ToolResult { let workspace = self.resolve_workspace(p.workspace_id.as_deref())?; - let request = self - .host - .get_http_request(&p.id)? - .ok_or_else(|| format!("HTTP request with ID {} not found", p.id))?; + let request = self.require_http_request(&p.id)?; let response = self.host.send_http_request(&workspace.id, request, p.environment_id).await?; @@ -182,10 +200,7 @@ impl YaakMcpServer { Parameters(p): Parameters, ) -> ToolResult { self.resolve_workspace(p.workspace_id.as_deref())?; - let existing = self - .host - .get_http_request(&p.id)? - .ok_or_else(|| format!("HTTP request with ID {} not found", p.id))?; + let existing = self.require_http_request(&p.id)?; let request: HttpRequest = merge(&existing, given_fields(&p))?; to_json(&self.host.upsert_http_request(&request)?) } @@ -199,6 +214,8 @@ impl YaakMcpServer { &self, Parameters(p): Parameters, ) -> ToolResult { + // Checked first so a gRPC or WebSocket ID gets an answer, not a missing-row error + self.require_http_request(&p.id)?; let request = self.host.delete_http_request(&p.id)?; Ok(format!("Deleted: {} ({})", request.name, request.id)) } diff --git a/crates/yaak/src/import.rs b/crates/yaak/src/import.rs index e32f3ee4e..dadde8ac2 100644 --- a/crates/yaak/src/import.rs +++ b/crates/yaak/src/import.rs @@ -194,10 +194,248 @@ pub fn plan_import_batch_resources( Ok(combined) } +/// Whether the importer's IDs are real Yaak IDs rather than placeholders. Older plugin runtimes +/// report the package name instead of the importer's display name. +fn is_yaak_importer(importer: &str) -> bool { + importer == "Yaak" || importer == "@yaak/importer-yaak" +} + +/// Decides which IDs an import keeps. +/// +/// Other importers' IDs are placeholders, so their models always get fresh ones. A Yaak export +/// carries real IDs: an ID the target workspace already has updates that model in place, an ID +/// nobody has is kept, and only an ID that belongs to another workspace is replaced. +struct ImportIds<'a> { + query_manager: &'a QueryManager, + keep: bool, + target_workspace_id: Option, + /// Kept IDs of models the target workspace already has, with their current content + matched: BTreeMap, + used: BTreeSet, + /// How many models were given new IDs because another workspace has theirs, by workspace + taken_elsewhere: BTreeMap, + /// The folder an import goes into and its ancestors. A folder from the document keeping one + /// of these IDs would end up inside itself. + reserved: BTreeSet, +} + +impl<'a> ImportIds<'a> { + fn new( + query_manager: &'a QueryManager, + importer: &str, + destination: &ImportDestination, + ) -> Self { + let (target_workspace_id, target_folder_id) = match destination { + ImportDestination::ExistingWorkspace { workspace_id, folder_id } => { + (Some(workspace_id.clone()), folder_id.clone()) + } + ImportDestination::NewWorkspace => (None, None), + }; + let mut reserved = BTreeSet::new(); + let mut next = target_folder_id; + while let Some(id) = next { + if !reserved.insert(id.clone()) { + break; + } + next = query_manager.connect().get_folder(&id).ok().and_then(|f| f.folder_id); + } + Self { + query_manager, + keep: is_yaak_importer(importer), + target_workspace_id, + matched: BTreeMap::new(), + used: BTreeSet::new(), + taken_elsewhere: BTreeMap::new(), + reserved, + } + } + + fn assign( + &mut self, + source_id: &str, + resource: ImportResourceType, + generate: fn() -> String, + ) -> Result { + let fresh = generate(); + if !self.keep + || !has_id_prefix_of(source_id, &fresh) + || self.used.contains(source_id) + || self.reserved.contains(source_id) + { + return Ok(fresh); + } + + let db = self.query_manager.connect(); + let existing = match resource { + ImportResourceType::Workspace => { + db.get_workspace(source_id).ok().map(|w| (w.id, Value::Null)) + } + _ => existing_model_json(&db, resource, source_id)?.map(|current| { + let workspace_id = + current.get("workspaceId").and_then(Value::as_str).unwrap_or_default(); + (workspace_id.to_string(), current) + }), + }; + + match existing { + None => {} + Some((workspace_id, current)) + if self.target_workspace_id.as_deref() == Some(workspace_id.as_str()) => + { + self.matched.insert(source_id.to_string(), current); + } + Some((workspace_id, _)) => { + *self.taken_elsewhere.entry(workspace_id).or_default() += 1; + return Ok(fresh); + } + } + self.used.insert(source_id.to_string()); + Ok(source_id.to_string()) + } + + fn is_matched(&self, id: &str) -> bool { + self.matched.contains_key(id) + } + + fn matched_parent_model(&self, id: &str) -> Option<&str> { + self.matched.get(id).and_then(|m| m.get("parentModel")).and_then(Value::as_str) + } + + fn warning(&self) -> Result> { + if self.taken_elsewhere.is_empty() { + return Ok(None); + } + let db = self.query_manager.connect(); + let mut names = BTreeSet::new(); + for workspace_id in self.taken_elsewhere.keys() { + names.insert( + db.get_workspace(workspace_id).map(|w| w.name).unwrap_or(workspace_id.clone()), + ); + } + let count = self.taken_elsewhere.values().sum::(); + let (noun, verb) = if count == 1 { ("resource", "exists") } else { ("resources", "exist") }; + let names = names.iter().map(String::as_str).collect::>(); + Ok(Some(ImportPlanWarning::warning( + "New IDs assigned", + format!( + "{count} {noun} already {verb} in {}, so the imported copies get new IDs", + display_list(&names) + ), + ))) + } +} + +/// Whether `id` looks like an ID of the same model as `generated`: the same prefix, then +/// something an ID can be made of. +fn has_id_prefix_of(id: &str, generated: &str) -> bool { + let Some((prefix, _)) = generated.split_once('_') else { + return false; + }; + id.strip_prefix(prefix).and_then(|rest| rest.strip_prefix('_')).is_some_and(|rest| { + !rest.is_empty() && rest.chars().all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') + }) +} + +/// A kept folder ID can name a folder the destination already has, so the document's parents +/// and the destination's can form a loop between them, such as a folder the document puts +/// under a folder the destination keeps inside it. Each planned folder found in a loop moves to +/// the top level, which cannot be part of one. Returns whether any did. +fn break_folder_cycles( + folders: &mut [Folder], + existing_parents: &BTreeMap>, +) -> bool { + let mut parents = existing_parents.clone(); + for folder in folders.iter() { + parents.insert(folder.id.clone(), folder.folder_id.clone()); + } + let mut moved = false; + for folder in folders.iter_mut() { + if folder_cycle_from(&parents, &folder.id) { + folder.folder_id = None; + parents.insert(folder.id.clone(), None); + moved = true; + } + } + moved +} + +/// Whether following parents up from `start` comes back to it. +fn folder_cycle_from(parents: &BTreeMap>, start: &str) -> bool { + let mut seen = BTreeSet::new(); + let mut next = parents.get(start).cloned().flatten(); + while let Some(id) = next { + if id == start { + return true; + } + if !seen.insert(id.clone()) { + return false; + } + next = parents.get(&id).cloned().flatten(); + } + false +} + +/// Models a Yaak export shares by ID with the destination are updates of those models, so they +/// are planned as updates (or as unchanged) rather than as new resources. +fn classify_matched_ids(plan: &mut ImportPlan, matched: &BTreeMap) -> Result<()> { + if matched.is_empty() { + return Ok(()); + } + let resources = &plan.resources; + let mut planned: BTreeMap = BTreeMap::new(); + for v in &resources.environments { + planned.insert(v.id.clone(), serde_json::to_value(v)?); + } + for v in &resources.folders { + planned.insert(v.id.clone(), serde_json::to_value(v)?); + } + for v in &resources.http_requests { + planned.insert(v.id.clone(), serde_json::to_value(v)?); + } + for v in &resources.grpc_requests { + planned.insert(v.id.clone(), serde_json::to_value(v)?); + } + for v in &resources.websocket_requests { + planned.insert(v.id.clone(), serde_json::to_value(v)?); + } + + for item in &mut plan.items { + if item.action != ImportPlanAction::Create { + continue; + } + let (Some(current), Some(incoming)) = + (matched.get(&item.model_id), planned.get(&item.model_id)) + else { + continue; + }; + // Only Yaak exports have IDs to match, and their order is the user's + let incoming = comparable_for(incoming.clone(), true); + let current = comparable_for(current.clone(), true); + if incoming == current { + item.action = ImportPlanAction::Unchanged; + item.selected = false; + } else { + item.action = ImportPlanAction::Update; + item.changed_fields = changed_fields(&incoming, ¤t); + } + } + + // A linked source may still map one of its old keys to a model the document now names by + // ID, and offer it as removed from the source. The document does have it, and the commit + // keeps one item per model, so that offer would cancel the update. + plan.items.retain(|i| { + !(i.action == ImportPlanAction::Delete + && matched.contains_key(&i.model_id) + && planned.contains_key(&i.model_id)) + }); + Ok(()) +} + /// Remap parsed importer resources into their selected destination. /// -/// Every imported model gets a fresh ID. This prevents an import from accidentally updating an -/// existing model and also makes the plan safe to inspect before it is committed. +/// Models from other importers get fresh IDs, which keeps an import from accidentally updating an +/// existing model. A Yaak export keeps its IDs where it can (see [`ImportIds`]). Either way the +/// plan is safe to inspect before it is committed. pub fn plan_import_resources( query_manager: &QueryManager, importer: String, @@ -213,11 +451,24 @@ pub fn plan_import_resources( // Source keys and merge decisions describe the document as the importer produced it, not as // this destination reshapes it, so keep the original around. let original = resources.clone(); + let mut ids = ImportIds::new(query_manager, &importer, &destination); + + // A request may name a folder the destination already has, rather than one in the document. + let existing_folder_parents = match &destination { + ImportDestination::ExistingWorkspace { workspace_id, .. } => query_manager + .connect() + .list_folders(workspace_id)? + .into_iter() + .map(|f| (f.id, f.folder_id)) + .collect::>(), + ImportDestination::NewWorkspace => BTreeMap::new(), + }; let source_folder_ids = resources.folders.iter().map(|v| v.id.clone()).collect::>(); let mut folder_ids = BTreeMap::new(); for folder in &resources.folders { - folder_ids.insert(folder.id.clone(), Folder::generate_id()); + let id = ids.assign(&folder.id, ImportResourceType::Folder, Folder::generate_id)?; + folder_ids.insert(folder.id.clone(), id); } let mut workspace_ids = BTreeMap::new(); @@ -226,7 +477,8 @@ pub fn plan_import_resources( ImportDestination::NewWorkspace => { for source in &resources.workspaces { let mut workspace = source.clone(); - workspace.id = Workspace::generate_id(); + workspace.id = + ids.assign(&source.id, ImportResourceType::Workspace, Workspace::generate_id)?; workspace_ids.insert(source.id.clone(), workspace.id.clone()); workspaces.push(workspace); } @@ -299,10 +551,11 @@ pub fn plan_import_resources( Some(source_id) if source_folder_ids.contains(&source_id) => { folder_ids.get(&source_id).cloned() } + Some(source_id) if existing_folder_parents.contains_key(&source_id) => Some(source_id), _ => target_folder_id.clone(), }; - let folders = resources + let mut folders: Vec = resources .folders .into_iter() .map(|mut folder| { @@ -312,42 +565,45 @@ pub fn plan_import_resources( folder }) .collect(); + if break_folder_cycles(&mut folders, &existing_folder_parents) { + warnings.push(ImportPlanWarning::warning( + "Folder moved to the top level", + "Its new place would have put it inside itself", + )); + } let mut request_ids = BTreeMap::new(); - let http_requests = resources - .http_requests - .into_iter() - .map(|mut request| { - let new_id = HttpRequest::generate_id(); - request_ids.insert(request.id.clone(), new_id.clone()); - request.id = new_id; - request.workspace_id = resolve_workspace_id(&request.workspace_id); - request.folder_id = resolve_folder_id(request.folder_id); - request - }) - .collect(); + let mut http_requests = Vec::new(); + for mut request in resources.http_requests { + let new_id = + ids.assign(&request.id, ImportResourceType::HttpRequest, HttpRequest::generate_id)?; + request_ids.insert(request.id.clone(), new_id.clone()); + request.id = new_id; + request.workspace_id = resolve_workspace_id(&request.workspace_id); + request.folder_id = resolve_folder_id(request.folder_id); + http_requests.push(request); + } - let grpc_requests = resources - .grpc_requests - .into_iter() - .map(|mut request| { - request.id = GrpcRequest::generate_id(); - request.workspace_id = resolve_workspace_id(&request.workspace_id); - request.folder_id = resolve_folder_id(request.folder_id); - request - }) - .collect(); + let mut grpc_requests = Vec::new(); + for mut request in resources.grpc_requests { + request.id = + ids.assign(&request.id, ImportResourceType::GrpcRequest, GrpcRequest::generate_id)?; + request.workspace_id = resolve_workspace_id(&request.workspace_id); + request.folder_id = resolve_folder_id(request.folder_id); + grpc_requests.push(request); + } - let websocket_requests = resources - .websocket_requests - .into_iter() - .map(|mut request| { - request.id = WebsocketRequest::generate_id(); - request.workspace_id = resolve_workspace_id(&request.workspace_id); - request.folder_id = resolve_folder_id(request.folder_id); - request - }) - .collect(); + let mut websocket_requests = Vec::new(); + for mut request in resources.websocket_requests { + request.id = ids.assign( + &request.id, + ImportResourceType::WebsocketRequest, + WebsocketRequest::generate_id, + )?; + request.workspace_id = resolve_workspace_id(&request.workspace_id); + request.folder_id = resolve_folder_id(request.folder_id); + websocket_requests.push(request); + } let importing_into_existing = matches!(destination, ImportDestination::ExistingWorkspace { .. }); @@ -356,61 +612,83 @@ pub fn plan_import_resources( let mut converted_duplicate_folder_environment = false; let mut base_environment_workspaces = BTreeSet::new(); let mut folder_environment_ids = BTreeSet::new(); - let environments = resources - .environments - .into_iter() - .map(|mut environment| { - environment.id = Environment::generate_id(); - environment.workspace_id = resolve_workspace_id(&environment.workspace_id); + let mut environments = Vec::new(); + for mut environment in resources.environments { + environment.id = + ids.assign(&environment.id, ImportResourceType::Environment, Environment::generate_id)?; + environment.workspace_id = resolve_workspace_id(&environment.workspace_id); - match (environment.parent_model.as_str(), environment.parent_id.clone()) { - ("workspace", _) if importing_into_existing => { + match (environment.parent_model.as_str(), environment.parent_id.clone()) { + // The destination's own base environment, exported and brought back + ("workspace", _) + if importing_into_existing + && ids.matched_parent_model(&environment.id) == Some("workspace") => + { + environment.parent_id = None; + } + ("workspace", _) if importing_into_existing => { + environment.parent_model = "environment".to_string(); + environment.parent_id = None; + let source_name = environment.name.clone(); + environment.name = format!("{} (Imported)", environment.name); + separated_base_environments.push(( + source_name, + environment.name.clone(), + environment.variables.len(), + )); + } + ("workspace", _) => { + environment.parent_id = None; + if !base_environment_workspaces.insert(environment.workspace_id.clone()) { environment.parent_model = "environment".to_string(); - environment.parent_id = None; - let source_name = environment.name.clone(); environment.name = format!("{} (Imported)", environment.name); - separated_base_environments.push(( - source_name, - environment.name.clone(), - environment.variables.len(), - )); - } - ("workspace", _) => { - environment.parent_id = None; - if !base_environment_workspaces.insert(environment.workspace_id.clone()) { - environment.parent_model = "environment".to_string(); - environment.name = format!("{} (Imported)", environment.name); - converted_duplicate_base_environment = true; - } - } - ("folder", Some(parent_id)) if source_folder_ids.contains(&parent_id) => { - environment.parent_id = folder_ids.get(&parent_id).cloned(); - if let Some(parent_id) = &environment.parent_id - && !folder_environment_ids.insert(parent_id.clone()) - { - environment.parent_model = "environment".to_string(); - environment.parent_id = None; - converted_duplicate_folder_environment = true; - } - } - ("folder", _) => { - // Never attach an imported folder environment to an existing folder: the model - // layer permits only one and would otherwise delete the destination's value. - environment.parent_model = "environment".to_string(); - environment.parent_id = None; - } - ("environment", _) => { - environment.parent_id = None; - } - _ => { - environment.parent_model = "environment".to_string(); - environment.parent_id = None; + converted_duplicate_base_environment = true; } } + ("folder", Some(parent_id)) if source_folder_ids.contains(&parent_id) => { + environment.parent_id = folder_ids.get(&parent_id).cloned(); + // A folder the destination already has may already have its own environment, + // which the model layer would delete to make room for this one. + let replaces_existing = match &environment.parent_id { + Some(parent_id) if ids.is_matched(parent_id) => query_manager + .connect() + .get_environment_by_folder_id(parent_id)? + .is_some_and(|e| e.id != environment.id), + _ => false, + }; + if let Some(parent_id) = &environment.parent_id + && (replaces_existing || !folder_environment_ids.insert(parent_id.clone())) + { + environment.parent_model = "environment".to_string(); + environment.parent_id = None; + converted_duplicate_folder_environment = true; + } + } + ("folder", _) => { + // Never attach an imported folder environment to an existing folder: the model + // layer permits only one and would otherwise delete the destination's value. + environment.parent_model = "environment".to_string(); + environment.parent_id = None; + } + ("environment", _) => { + environment.parent_id = None; + } + _ => { + environment.parent_model = "environment".to_string(); + environment.parent_id = None; + } + } - environment - }) - .collect(); + // Updating in place must not turn an existing environment into another kind, such as + // the destination's base environment into a sub-environment, so that one is a copy. + if ids.is_matched(&environment.id) + && ids.matched_parent_model(&environment.id) != Some(environment.parent_model.as_str()) + { + ids.matched.remove(&environment.id); + environment.id = Environment::generate_id(); + } + environments.push(environment); + } for (source_name, imported_name, variable_count) in separated_base_environments { let variables = if variable_count == 1 { "variable" } else { "variables" }; @@ -431,6 +709,9 @@ pub fn plan_import_resources( "Only the first remains attached to each folder", )); } + if let Some(warning) = ids.warning()? { + warnings.push(warning); + } let mut resources = BatchUpsertResult { workspaces, @@ -453,6 +734,7 @@ pub fn plan_import_resources( sources: Vec::new(), }; merge_with_linked_source(query_manager, &mut plan, &original)?; + classify_matched_ids(&mut plan, &ids.matched)?; warn_if_imported_elsewhere(&query_manager.connect(), &mut plan)?; Ok(plan) } @@ -560,6 +842,35 @@ fn commit_plan_in_tx(db: &WriteDb, plan: ImportPlan) -> Result>(); + if !applied_folders.is_empty() { + let mut parents = BTreeMap::new(); + let workspace_ids = + applied_folders.iter().map(|f| f.workspace_id.as_str()).collect::>(); + for workspace_id in workspace_ids { + for folder in db.list_folders(workspace_id)? { + parents.insert(folder.id, folder.folder_id); + } + } + for folder in &applied_folders { + parents.insert(folder.id.clone(), folder.folder_id.clone()); + } + if let Some(folder) = applied_folders.iter().find(|f| folder_cycle_from(&parents, &f.id)) { + return Err(yaak_models::error::Error::GenericError(format!( + "This selection would put the folder {} inside itself. Import the moves of the folders around it together, or leave them all out.", + folder.name + )) + .into()); + } + } + let upserted = db.batch_upsert( resources.workspaces.clone(), resources @@ -729,21 +1040,22 @@ fn record_import_plan_source( &UpdateSource::Import, )?; + let keep_order = is_yaak_importer(&source.importer); let mut committed: BTreeMap<&str, String> = BTreeMap::new(); for v in &upserted.environments { - committed.insert(&v.id, content_hash(serde_json::to_value(v)?)); + committed.insert(&v.id, content_hash(serde_json::to_value(v)?, keep_order)); } for v in &upserted.folders { - committed.insert(&v.id, content_hash(serde_json::to_value(v)?)); + committed.insert(&v.id, content_hash(serde_json::to_value(v)?, keep_order)); } for v in &upserted.http_requests { - committed.insert(&v.id, content_hash(serde_json::to_value(v)?)); + committed.insert(&v.id, content_hash(serde_json::to_value(v)?, keep_order)); } for v in &upserted.grpc_requests { - committed.insert(&v.id, content_hash(serde_json::to_value(v)?)); + committed.insert(&v.id, content_hash(serde_json::to_value(v)?, keep_order)); } for v in &upserted.websocket_requests { - committed.insert(&v.id, content_hash(serde_json::to_value(v)?)); + committed.insert(&v.id, content_hash(serde_json::to_value(v)?, keep_order)); } let write_row = |model_id: &str, @@ -761,7 +1073,9 @@ fn record_import_plan_source( ImportPlanAction::Unchanged | ImportPlanAction::KeepLocal | ImportPlanAction::Conflict, - ) => Some((Some(model_id.to_string()), Some(content_hash(incoming()?)))), + ) => { + Some((Some(model_id.to_string()), Some(content_hash(incoming()?, keep_order)))) + } // Turned down, so remember it as not wanted rather than offering it again Some(ImportPlanAction::Create | ImportPlanAction::Ignored) => Some((None, None)), Some(ImportPlanAction::Delete) if item.is_some_and(|i| i.selected) => { @@ -893,6 +1207,7 @@ fn merge_with_linked_source( original: &ImportResources, ) -> Result<()> { let db = query_manager.connect(); + let keep_order = is_yaak_importer(&plan.importer); let incoming_keys: BTreeSet = plan.source_keys.values().cloned().collect(); let linked = match (&plan.origin, &plan.destination) { @@ -1114,8 +1429,11 @@ fn merge_with_linked_source( KeyStatus::Wanted(row) => row, }; - let incoming = comparable(serde_json::to_value(&any)?); - let current = comparable(current_models.get(&planned_id).cloned().unwrap_or_default()); + let incoming = comparable_for(serde_json::to_value(&any)?, keep_order); + let current = comparable_for( + current_models.get(&planned_id).cloned().unwrap_or_default(), + keep_order, + ); let (source_changed, local_changed) = match recorded_hash(row, &incoming) { Some(hash) => { (hash_comparable(&incoming) != hash, hash_comparable(¤t) != hash) @@ -1321,16 +1639,38 @@ fn strip_ids(value: Value) -> Value { } } +/// [`comparable`], keeping `sortPriority` when `keep_order` is set. A Yaak export's sort +/// priorities are the sidebar order the user chose rather than numbering an importer made up, +/// so for Yaak imports a change of order is a change. +fn comparable_for(value: Value, keep_order: bool) -> Value { + let priority = if keep_order { value.get("sortPriority").cloned() } else { None }; + let mut value = comparable(value); + if let (Some(object), Some(priority)) = (value.as_object_mut(), priority) { + object.insert("sortPriority".to_string(), priority); + } + value +} + fn content_hash_version(value: &Value) -> &'static str { - // Only folder environments changed comparison semantics. Older hashes for these fall back - // to comparing the two sides until the next import records a hash without the derived name. - if value.get("parentModel").and_then(Value::as_str) == Some("folder") { "v2:" } else { "v1:" } + // Ordered comparisons are Yaak imports, whose older hashes left the order out; like the + // folder environments before them, those fall back to comparing the two sides until the + // next import records a hash in the new form. + if value.get("sortPriority").is_some() { + "v3:" + } else if value.get("parentModel").and_then(Value::as_str) == Some("folder") { + // Only folder environments changed comparison semantics. Older hashes for these fall + // back to comparing the two sides until the next import records one without the + // derived name. + "v2:" + } else { + "v1:" + } } /// Identifies a resource's content well enough to tell "changed since the last import" from /// "unchanged", without keeping a copy of every imported resource around. -fn content_hash(value: Value) -> String { - hash_comparable(&comparable(value)) +fn content_hash(value: Value, keep_order: bool) -> String { + hash_comparable(&comparable_for(value, keep_order)) } fn hash_comparable(value: &Value) -> String { @@ -2812,7 +3152,10 @@ mod tests { let current = existing_model_json(&db, resource, model_id) .expect("query current model") .expect("row target exists"); - assert_eq!(row.content_hash.as_deref(), Some(content_hash(current).as_str())); + assert_eq!( + row.content_hash.as_deref(), + Some(content_hash(current, false).as_str()) + ); } source }; @@ -3715,4 +4058,427 @@ mod tests { let plan = replan(&query_manager, &workspace_id, imported_resources()); assert_eq!(item_by_name(&plan, "Root Request").action, ImportPlanAction::Unchanged); } + + fn plan_yaak( + query_manager: &QueryManager, + destination: ImportDestination, + resources: ImportResources, + ) -> ImportPlan { + plan_import_resources( + query_manager, + "Yaak".to_string(), + destination, + resources, + None, + Some(ImportOrigin { origin: "/export.json".into(), label: "export.json".into() }), + ) + .expect("plan Yaak import") + } + + fn into_existing(workspace_id: &str) -> ImportDestination { + ImportDestination::ExistingWorkspace { workspace_id: workspace_id.into(), folder_id: None } + } + + /// QA sweep bug 2: `yaak import req_in_existing_folder.json --workspace-id `, where the + /// file's request names a folder that W already has. + #[test] + fn a_folder_id_naming_an_existing_folder_is_kept() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let destination = destination_workspace(); + query_manager + .with_tx(|tx| { + tx.upsert_workspace(&destination, &UpdateSource::Import)?; + tx.upsert_folder( + &Folder { + id: "fl_existing".into(), + workspace_id: destination.id.clone(), + name: "Existing".into(), + ..Default::default() + }, + &UpdateSource::Import, + ) + }) + .expect("seed destination"); + + let request = |id: &str, folder_id: &str| HttpRequest { + id: id.into(), + model: "http_request".into(), + workspace_id: destination.id.clone(), + folder_id: Some(folder_id.into()), + name: id.into(), + ..Default::default() + }; + let resources = ImportResources { + http_requests: vec![ + request("rq_into_existing", "fl_existing"), + request("rq_dangling", "fl_missing"), + ], + ..Default::default() + }; + let plan = plan_yaak(&query_manager, into_existing(&destination.id), resources); + let folder_of = |name: &str| { + plan.resources.http_requests.iter().find(|r| r.name == name).unwrap().folder_id.clone() + }; + assert_eq!(folder_of("rq_into_existing").as_deref(), Some("fl_existing")); + assert_eq!( + folder_of("rq_dangling"), + None, + "an unknown folder still falls back to the root" + ); + commit_import_plan(&query_manager, plan).expect("commit"); + let stored = query_manager.connect().get_http_request("rq_into_existing").expect("stored"); + assert_eq!(stored.folder_id.as_deref(), Some("fl_existing")); + } + + /// QA sweep bug 3: `yaak export e.json `, edit the file, then + /// `yaak import e.json --workspace-id `. + #[test] + fn importing_a_yaak_export_into_its_own_workspace_updates_in_place() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let workspace_id = "wk_ownexport1".to_string(); + let exported = query_manager + .with_tx(|tx| { + let source = &UpdateSource::Background; + let workspace = tx.upsert_workspace( + &Workspace { + id: workspace_id.clone(), + name: "Own".into(), + ..Default::default() + }, + source, + )?; + let base = tx.ensure_base_environment(&workspace.id)?; + let staging = tx.upsert_environment( + &Environment { + workspace_id: workspace.id.clone(), + name: "Staging".into(), + parent_model: "environment".into(), + ..Default::default() + }, + source, + )?; + let folder = tx.upsert_folder( + &Folder { + workspace_id: workspace.id.clone(), + name: "Folder".into(), + ..Default::default() + }, + source, + )?; + let request = tx.upsert_http_request( + &HttpRequest { + workspace_id: workspace.id.clone(), + folder_id: Some(folder.id.clone()), + name: "Request".into(), + url: "https://example.com/v1".into(), + ..Default::default() + }, + source, + )?; + Ok::<_, yaak_models::error::Error>(ImportResources { + workspaces: vec![workspace], + environments: vec![base, staging], + folders: vec![folder], + http_requests: vec![request], + ..Default::default() + }) + }) + .expect("seed workspace"); + + let mut edited = exported.clone(); + edited.http_requests[0].url = "https://example.com/v2".into(); + edited.environments[0].variables = vec![EnvironmentVariable { + enabled: true, + name: "host".into(), + value: "example.com".into(), + id: None, + }]; + + let plan = plan_yaak(&query_manager, into_existing(&workspace_id), edited); + assert!( + plan.items.iter().all(|i| i.action != ImportPlanAction::Create), + "nothing in the workspace's own export is new: {:?}", + plan.items + ); + commit_import_plan(&query_manager, plan).expect("commit"); + + let db = query_manager.connect(); + let requests = db.list_http_requests(&workspace_id).expect("list requests"); + assert_eq!(requests.len(), 1, "no copies: {requests:?}"); + assert_eq!(requests[0].id, exported.http_requests[0].id); + assert_eq!(requests[0].url, "https://example.com/v2"); + assert_eq!(db.list_folders(&workspace_id).expect("list folders").len(), 1); + + let environments = db.list_environments(&workspace_id).expect("list environments"); + assert_eq!(environments.len(), 2, "no second base or Imported copies: {environments:?}"); + assert!(environments.iter().all(|e| !e.name.contains("(Imported)")), "{environments:?}"); + let base = db.get_base_environment(&workspace_id).expect("one base environment"); + assert_eq!(base.id, exported.environments[0].id); + assert_eq!(base.variables.len(), 1); + } + + /// QA sweep bug 7: `yaak import ci-export.json` into a fresh `--data-dir`, then the same + /// file again next to the copy it made. + #[test] + fn a_yaak_export_keeps_its_ids_where_they_are_free() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let plan = plan_yaak(&query_manager, ImportDestination::NewWorkspace, imported_resources()); + let ids = |plan: &ImportPlan| { + let r = &plan.resources; + r.workspaces + .iter() + .map(|v| v.id.clone()) + .chain(r.environments.iter().map(|v| v.id.clone())) + .chain(r.folders.iter().map(|v| v.id.clone())) + .chain(r.http_requests.iter().map(|v| v.id.clone())) + .collect::>() + }; + let source_ids = BTreeSet::from( + [ + "wk_source", + "ev_source_base", + "fl_source", + "rq_root", + "rq_nested", + ] + .map(String::from), + ); + assert_eq!(ids(&plan), source_ids); + assert!(plan.warnings.iter().all(|w| w.title != "New IDs assigned"), "{:?}", plan.warnings); + commit_import_plan(&query_manager, plan).expect("commit"); + let nested = query_manager.connect().get_http_request("rq_nested").expect("kept ID"); + assert_eq!(nested.folder_id.as_deref(), Some("fl_source")); + + // The IDs now belong to another workspace, so a second copy gets fresh ones + let again = + plan_yaak(&query_manager, ImportDestination::NewWorkspace, imported_resources()); + assert!(ids(&again).is_disjoint(&source_ids), "{:?}", ids(&again)); + let warning = again.warnings.iter().find(|w| w.title == "New IDs assigned").expect("warns"); + assert_eq!( + warning.detail, + "5 resources already exist in Imported, so the imported copies get new IDs" + ); + + // Other importers' IDs are placeholders and are never kept + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let other = plan_import_resources( + &query_manager, + "OpenAPI".to_string(), + ImportDestination::NewWorkspace, + imported_resources(), + None, + None, + ) + .expect("plan"); + assert!(ids(&other).is_disjoint(&source_ids)); + } + + /// Keeping IDs must not let an import put a folder inside itself: neither when the import + /// goes into a folder the document also contains, nor when the document's parents and the + /// destination's form a loop. + #[test] + fn kept_folder_ids_never_form_a_cycle() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let destination = destination_workspace(); + let folder = |id: &str, parent: Option<&str>| Folder { + id: id.into(), + model: "folder".into(), + workspace_id: destination.id.clone(), + name: id.into(), + folder_id: parent.map(String::from), + ..Default::default() + }; + query_manager + .with_tx(|tx| { + tx.upsert_workspace(&destination, &UpdateSource::Import)?; + tx.upsert_folder(&folder("fl_outer", None), &UpdateSource::Import)?; + tx.upsert_folder(&folder("fl_inner", Some("fl_outer")), &UpdateSource::Import) + }) + .expect("seed destination"); + + // Into fl_outer, with fl_outer itself in the document + let resources = + ImportResources { folders: vec![folder("fl_outer", None)], ..Default::default() }; + let plan = plan_yaak( + &query_manager, + ImportDestination::ExistingWorkspace { + workspace_id: destination.id.clone(), + folder_id: Some("fl_outer".into()), + }, + resources, + ); + let planned = &plan.resources.folders[0]; + assert_ne!(planned.id, "fl_outer"); + assert_eq!(planned.folder_id.as_deref(), Some("fl_outer")); + + // The document puts fl_outer under fl_inner, which the destination keeps in fl_outer + let resources = ImportResources { + folders: vec![folder("fl_outer", Some("fl_inner"))], + ..Default::default() + }; + let plan = plan_yaak(&query_manager, into_existing(&destination.id), resources); + let planned = &plan.resources.folders[0]; + assert_eq!(planned.id, "fl_outer"); + assert_eq!(planned.folder_id, None, "the loop is broken at the top level"); + assert!(plan.warnings.iter().any(|w| w.title == "Folder moved to the top level")); + commit_import_plan(&query_manager, plan).expect("commit"); + } + + /// The preview can apply one folder's move and not another's, so the commit checks the moves + /// it actually makes. + #[test] + fn a_selection_that_would_loop_folders_is_refused() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let destination = destination_workspace(); + let folder = |id: &str, parent: Option<&str>| Folder { + id: id.into(), + model: "folder".into(), + workspace_id: destination.id.clone(), + name: id.into(), + folder_id: parent.map(String::from), + ..Default::default() + }; + query_manager + .with_tx(|tx| { + tx.upsert_workspace(&destination, &UpdateSource::Import)?; + tx.upsert_folder(&folder("fl_a", None), &UpdateSource::Import)?; + tx.upsert_folder(&folder("fl_b", Some("fl_a")), &UpdateSource::Import) + }) + .expect("seed destination"); + + let resources = ImportResources { + folders: vec![folder("fl_a", Some("fl_b")), folder("fl_b", None)], + ..Default::default() + }; + let mut plan = plan_yaak(&query_manager, into_existing(&destination.id), resources); + select(&mut plan, "fl_b", false); + let error = commit_import_plan(&query_manager, plan).expect_err("refused"); + assert!(error.to_string().contains("would put the folder fl_a inside itself"), "{error}"); + let stored = query_manager.connect().get_folder("fl_a").expect("unchanged"); + assert_eq!(stored.folder_id, None); + } + + /// Re-importing an export of a copy into that copy, from the file the copy was first + /// imported from: the linked source's old key and the document's ID name the same request, + /// and the stale removal offer must not cancel its update. + #[test] + fn an_id_match_is_not_cancelled_by_a_stale_removal_offer() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let original = Workspace { + id: "wk_original".into(), + model: "workspace".into(), + name: "Original".into(), + ..Default::default() + }; + let request = HttpRequest { + id: "rq_original".into(), + model: "http_request".into(), + workspace_id: original.id.clone(), + name: "Request".into(), + url: "https://example.com/v1".into(), + ..Default::default() + }; + query_manager + .with_tx(|tx| { + tx.upsert_workspace(&original, &UpdateSource::Import)?; + tx.upsert_http_request(&request, &UpdateSource::Import) + }) + .expect("seed original"); + let origin = || ImportOrigin { origin: "/export.json".into(), label: "export.json".into() }; + let keys = |ids: &[&str]| { + Some(ids.iter().map(|id| (id.to_string(), id.to_string())).collect::>()) + }; + + let copy = plan_import_resources( + &query_manager, + "Yaak".into(), + ImportDestination::NewWorkspace, + ImportResources { + workspaces: vec![original.clone()], + http_requests: vec![request.clone()], + ..Default::default() + }, + keys(&["wk_original", "rq_original"]), + Some(origin()), + ) + .expect("plan copy"); + let copy = commit_import_plan(&query_manager, copy).expect("commit copy"); + let (copy_workspace, copy_request) = (©.workspaces[0], ©.http_requests[0]); + assert_ne!(copy_request.id, request.id); + + let plan = plan_import_resources( + &query_manager, + "Yaak".into(), + into_existing(©_workspace.id), + ImportResources { + workspaces: vec![copy_workspace.clone()], + http_requests: vec![HttpRequest { + url: "https://example.com/v2".into(), + ..copy_request.clone() + }], + ..Default::default() + }, + keys(&[©_workspace.id, ©_request.id]), + Some(origin()), + ) + .expect("plan re-import"); + assert!( + plan.items.iter().all(|i| i.action != ImportPlanAction::Delete), + "{:?}", + plan.items + ); + commit_import_plan(&query_manager, plan).expect("commit re-import"); + let stored = query_manager.connect().get_http_request(©_request.id).expect("stored"); + assert_eq!(stored.url, "https://example.com/v2"); + } + + /// A Yaak export's sort priorities are the user's sidebar order, so changing only that is + /// an update, not an unchanged resource. + #[test] + fn a_sort_priority_change_in_a_yaak_export_is_applied() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let destination = destination_workspace(); + let request = HttpRequest { + id: "rq_ordered".into(), + model: "http_request".into(), + workspace_id: destination.id.clone(), + name: "Ordered".into(), + sort_priority: 1.0, + ..Default::default() + }; + query_manager + .with_tx(|tx| { + tx.upsert_workspace(&destination, &UpdateSource::Import)?; + tx.upsert_http_request(&request, &UpdateSource::Import) + }) + .expect("seed destination"); + + // The first import matches by ID; the second goes through the import source the first + // one linked, and is compared against the hashes it recorded. + for (round, priority) in [(1, 42.0), (2, 7.0)] { + let resources = ImportResources { + http_requests: vec![HttpRequest { sort_priority: priority, ..request.clone() }], + ..Default::default() + }; + let plan = plan_yaak(&query_manager, into_existing(&destination.id), resources); + if round == 2 { + let db = query_manager.connect(); + assert_eq!(db.list_import_sources(&destination.id).expect("sources").len(), 1); + } + let item = item_by_name(&plan, "Ordered"); + assert_eq!(item.action, ImportPlanAction::Update, "round {round}"); + assert_eq!(item.changed_fields, ["sortPriority"], "round {round}"); + commit_import_plan(&query_manager, plan).expect("commit"); + let stored = query_manager.connect().get_http_request("rq_ordered").expect("stored"); + assert_eq!(stored.sort_priority, priority, "round {round}"); + } + } }