From e05feba70855f1a8043cc422ea73166727c0afd4 Mon Sep 17 00:00:00 2001 From: Gregory Schier Date: Tue, 14 Jul 2026 07:43:14 -0700 Subject: [PATCH] Duplicate models from the DB instead of frontend snapshots (#505) --- .../yaak-app-client/src/models_ext.rs | 27 ++++++--- crates/yaak-models/guest-js/store.ts | 45 ++++----------- .../yaak-models/src/queries/duplicate_name.rs | 56 +++++++++++++++++++ .../yaak-models/src/queries/environments.rs | 7 +++ crates/yaak-models/src/queries/folders.rs | 22 +++++--- .../yaak-models/src/queries/grpc_requests.rs | 9 ++- .../yaak-models/src/queries/http_requests.rs | 9 ++- crates/yaak-models/src/queries/mod.rs | 2 + .../src/queries/websocket_requests.rs | 9 ++- 9 files changed, 133 insertions(+), 53 deletions(-) create mode 100644 crates/yaak-models/src/queries/duplicate_name.rs diff --git a/crates-tauri/yaak-app-client/src/models_ext.rs b/crates-tauri/yaak-app-client/src/models_ext.rs index 795af40f..e210cc97 100644 --- a/crates-tauri/yaak-app-client/src/models_ext.rs +++ b/crates-tauri/yaak-app-client/src/models_ext.rs @@ -194,20 +194,31 @@ pub(crate) fn models_delete( #[tauri::command] pub(crate) fn models_duplicate( window: WebviewWindow, - model: AnyModel, + model_type: String, + model_id: String, ) -> Result { use yaak_models::error::Error::GenericError; // Use transaction for duplications because it might recurse window.with_tx(|tx| { let source = &UpdateSource::from_window_label(window.label()); - let id = match model { - AnyModel::Environment(m) => tx.duplicate_environment(&m, source)?.id, - AnyModel::Folder(m) => tx.duplicate_folder(&m, source)?.id, - AnyModel::GrpcRequest(m) => tx.duplicate_grpc_request(&m, source)?.id, - AnyModel::HttpRequest(m) => tx.duplicate_http_request(&m, source)?.id, - AnyModel::WebsocketRequest(m) => tx.duplicate_websocket_request(&m, source)?.id, - a => return Err(GenericError(format!("Cannot duplicate AnyModel {a:?})"))), + // Fetch the model fresh from the DB so the duplicate doesn't come from + // a stale frontend snapshot + let id = match model_type.as_str() { + "environment" => { + tx.duplicate_environment(&tx.get_environment(&model_id)?, source)?.id + } + "folder" => tx.duplicate_folder(&tx.get_folder(&model_id)?, source)?.id, + "grpc_request" => { + tx.duplicate_grpc_request(&tx.get_grpc_request(&model_id)?, source)?.id + } + "http_request" => { + tx.duplicate_http_request(&tx.get_http_request(&model_id)?, source)?.id + } + "websocket_request" => { + tx.duplicate_websocket_request(&tx.get_websocket_request(&model_id)?, source)?.id + } + t => return Err(GenericError(format!("Cannot duplicate model type {t}"))), }; Ok(id) diff --git a/crates/yaak-models/guest-js/store.ts b/crates/yaak-models/guest-js/store.ts index c402b531..92b700e4 100644 --- a/crates/yaak-models/guest-js/store.ts +++ b/crates/yaak-models/guest-js/store.ts @@ -1,6 +1,5 @@ import { invoke } from "@tauri-apps/api/core"; import { getCurrentWebviewWindow } from "@tauri-apps/api/webviewWindow"; -import { resolvedModelName } from "@yaakapp/yaak-client/lib/resolvedModelName"; import { AnyModel, ModelPayload } from "../bindings/gen_models"; import { modelStoreDataAtom } from "./atoms"; import { ExtractModel, JotaiStore, ModelStoreData } from "./types"; @@ -156,44 +155,22 @@ export async function deleteModel("models_delete", { model })); } -export function duplicateModel>( - model: T | null, -) { +export async function duplicateModel< + M extends AnyModel["model"], + T extends ExtractModel, +>(model: T | null): Promise { if (model == null) { throw new Error("Failed to duplicate null model"); } - // If the model has an explicit (non-empty) name, try to duplicate it with a name that doesn't conflict. - // When the name is empty, keep it empty so the display falls back to the URL. - let name = "name" in model ? model.name : undefined; - if (name) { - const existingModels = listModels(model.model); - for (let i = 0; i < 100; i++) { - const hasConflict = existingModels.some((m) => { - if ("folderId" in m && "folderId" in model && model.folderId !== m.folderId) { - return false; - } else if (resolvedModelName(m) !== name) { - return false; - } - return true; - }); - if (!hasConflict) { - break; - } + // Flush pending writes first, since the backend duplicates from the DB (the passed-in + // model may be a stale snapshot, eg. from the memoized sidebar tree). Conflict-free + // naming ("Foo Copy 2") is also handled by the backend. + await flushAllModelWrites(); - // Name conflict. Try another one - const m: RegExpMatchArray | null = name.match(/ Copy( (?\d+))?$/); - if (m != null && m.groups?.n == null) { - name = name.substring(0, m.index) + " Copy 2"; - } else if (m != null && m.groups?.n != null) { - name = name.substring(0, m.index) + ` Copy ${parseInt(m.groups.n) + 1}`; - } else { - name = `${name} Copy`; - } - } - } - - return trackModelWrite(invoke("models_duplicate", { model: { ...model, name } })); + return trackModelWrite( + invoke("models_duplicate", { modelType: model.model, modelId: model.id }), + ); } export async function createGlobalModel>( diff --git a/crates/yaak-models/src/queries/duplicate_name.rs b/crates/yaak-models/src/queries/duplicate_name.rs new file mode 100644 index 00000000..87aedaf3 --- /dev/null +++ b/crates/yaak-models/src/queries/duplicate_name.rs @@ -0,0 +1,56 @@ +/// Compute a name for a duplicated model that doesn't conflict with any sibling +/// name, following the " Copy N" convention. Empty names are kept empty so the +/// display falls back to the URL. +pub(crate) fn conflict_free_name(name: &str, sibling_names: &[String]) -> String { + if name.is_empty() { + return String::new(); + } + + let mut name = name.to_string(); + for _ in 0..100 { + if !sibling_names.contains(&name) { + break; + } + name = next_copy_name(&name); + } + name +} + +fn next_copy_name(name: &str) -> String { + if let Some(base) = name.strip_suffix(" Copy") { + return format!("{base} Copy 2"); + } + + if let Some(idx) = name.rfind(" Copy ") { + let n = &name[idx + " Copy ".len()..]; + if !n.is_empty() && n.chars().all(|c| c.is_ascii_digit()) { + if let Ok(n) = n.parse::() { + return format!("{} Copy {}", &name[..idx], n + 1); + } + } + } + + format!("{name} Copy") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_next_copy_name() { + assert_eq!(next_copy_name("Foo"), "Foo Copy"); + assert_eq!(next_copy_name("Foo Copy"), "Foo Copy 2"); + assert_eq!(next_copy_name("Foo Copy 2"), "Foo Copy 3"); + assert_eq!(next_copy_name("Foo Copy 99"), "Foo Copy 100"); + assert_eq!(next_copy_name("Copy"), "Copy Copy"); + } + + #[test] + fn test_conflict_free_name() { + let siblings = vec!["Foo".to_string(), "Foo Copy".to_string(), "".to_string()]; + assert_eq!(conflict_free_name("Foo", &siblings), "Foo Copy 2"); + assert_eq!(conflict_free_name("Bar", &siblings), "Bar"); + assert_eq!(conflict_free_name("", &siblings), ""); + } +} diff --git a/crates/yaak-models/src/queries/environments.rs b/crates/yaak-models/src/queries/environments.rs index a0aaf2fb..9432d0f4 100644 --- a/crates/yaak-models/src/queries/environments.rs +++ b/crates/yaak-models/src/queries/environments.rs @@ -1,3 +1,4 @@ +use super::conflict_free_name; use crate::client_db::ClientDb; use crate::error::Error::{MissingBaseEnvironment, MultipleBaseEnvironments}; use crate::error::Result; @@ -88,6 +89,12 @@ impl<'a> ClientDb<'a> { ) -> Result { let mut environment = environment.clone(); environment.id = "".to_string(); + let sibling_names = self + .list_environments_dangerous(&environment.workspace_id)? + .into_iter() + .map(|e| e.name) + .collect::>(); + environment.name = conflict_free_name(&environment.name, &sibling_names); self.upsert_environment(&environment, source) } diff --git a/crates/yaak-models/src/queries/folders.rs b/crates/yaak-models/src/queries/folders.rs index 290bf0c1..a26e3379 100644 --- a/crates/yaak-models/src/queries/folders.rs +++ b/crates/yaak-models/src/queries/folders.rs @@ -1,3 +1,4 @@ +use super::conflict_free_name; use crate::client_db::ClientDb; use crate::connection_or_tx::ConnectionOrTx; use crate::error::Result; @@ -62,14 +63,19 @@ impl<'a> ClientDb<'a> { pub fn duplicate_folder(&self, src_folder: &Folder, source: &UpdateSource) -> Result { let fid = &src_folder.id; - let new_folder = self.upsert_folder( - &Folder { - id: "".into(), - sort_priority: src_folder.sort_priority + 0.001, - ..src_folder.clone() - }, - source, - )?; + let mut folder = Folder { + id: "".into(), + sort_priority: src_folder.sort_priority + 0.001, + ..src_folder.clone() + }; + let sibling_names = self + .list_folders(&folder.workspace_id)? + .into_iter() + .filter(|f| f.folder_id == folder.folder_id) + .map(|f| f.name) + .collect::>(); + folder.name = conflict_free_name(&folder.name, &sibling_names); + let new_folder = self.upsert_folder(&folder, source)?; for m in self.find_many::(HttpRequestIden::FolderId, fid, None)? { self.upsert_http_request( diff --git a/crates/yaak-models/src/queries/grpc_requests.rs b/crates/yaak-models/src/queries/grpc_requests.rs index a074f07b..f54635b4 100644 --- a/crates/yaak-models/src/queries/grpc_requests.rs +++ b/crates/yaak-models/src/queries/grpc_requests.rs @@ -1,4 +1,4 @@ -use super::dedupe_headers; +use super::{conflict_free_name, dedupe_headers}; use crate::client_db::ClientDb; use crate::error::Result; use crate::models::{ @@ -58,6 +58,13 @@ impl<'a> ClientDb<'a> { let mut request = grpc_request.clone(); request.id = "".to_string(); request.sort_priority = request.sort_priority + 0.001; + let sibling_names = self + .list_grpc_requests(&request.workspace_id)? + .into_iter() + .filter(|m| m.folder_id == request.folder_id) + .map(|m| m.name) + .collect::>(); + request.name = conflict_free_name(&request.name, &sibling_names); self.upsert(&request, source) } diff --git a/crates/yaak-models/src/queries/http_requests.rs b/crates/yaak-models/src/queries/http_requests.rs index 2c50e25d..6130ec67 100644 --- a/crates/yaak-models/src/queries/http_requests.rs +++ b/crates/yaak-models/src/queries/http_requests.rs @@ -1,4 +1,4 @@ -use super::dedupe_headers; +use super::{conflict_free_name, dedupe_headers}; use crate::client_db::ClientDb; use crate::error::Result; use crate::models::{ @@ -44,6 +44,13 @@ impl<'a> ClientDb<'a> { let mut http_request = http_request.clone(); http_request.id = "".to_string(); http_request.sort_priority = http_request.sort_priority + 0.001; + let sibling_names = self + .list_http_requests(&http_request.workspace_id)? + .into_iter() + .filter(|m| m.folder_id == http_request.folder_id) + .map(|m| m.name) + .collect::>(); + http_request.name = conflict_free_name(&http_request.name, &sibling_names); self.upsert(&http_request, source) } diff --git a/crates/yaak-models/src/queries/mod.rs b/crates/yaak-models/src/queries/mod.rs index a27361b4..2b6bdc43 100644 --- a/crates/yaak-models/src/queries/mod.rs +++ b/crates/yaak-models/src/queries/mod.rs @@ -1,6 +1,7 @@ pub mod any_request; mod batch; mod cookie_jars; +mod duplicate_name; mod environments; mod folders; mod graphql_introspections; @@ -22,6 +23,7 @@ mod websocket_requests; mod workspace_metas; pub mod workspaces; pub use model_changes::PersistedModelChange; +pub(crate) use duplicate_name::conflict_free_name; const MAX_HISTORY_ITEMS: usize = 20; diff --git a/crates/yaak-models/src/queries/websocket_requests.rs b/crates/yaak-models/src/queries/websocket_requests.rs index 296ea3c4..3ef4c16d 100644 --- a/crates/yaak-models/src/queries/websocket_requests.rs +++ b/crates/yaak-models/src/queries/websocket_requests.rs @@ -1,4 +1,4 @@ -use super::dedupe_headers; +use super::{conflict_free_name, dedupe_headers}; use crate::client_db::ClientDb; use crate::error::Result; use crate::models::{ @@ -60,6 +60,13 @@ impl<'a> ClientDb<'a> { let mut websocket_request = websocket_request.clone(); websocket_request.id = "".to_string(); websocket_request.sort_priority = websocket_request.sort_priority + 0.001; + let sibling_names = self + .list_websocket_requests(&websocket_request.workspace_id)? + .into_iter() + .filter(|m| m.folder_id == websocket_request.folder_id) + .map(|m| m.name) + .collect::>(); + websocket_request.name = conflict_free_name(&websocket_request.name, &sibling_names); self.upsert(&websocket_request, source) }