Duplicate models from the DB instead of frontend snapshots (#505)

crates-tauri/yaak-app-client/src/models_ext.rs

@@ -194,20 +194,31 @@ pub(crate) fn models_delete<R: Runtime>(
 #[tauri::command]
 pub(crate) fn models_duplicate<R: Runtime>(
     window: WebviewWindow<R>,
-    model: AnyModel,
+    model_type: String,
+    model_id: String,
 ) -> Result<String> {
     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)

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<M extends AnyModel["model"], T extends Extract
   await trackModelWrite(invoke<string>("models_delete", { model }));
 }
 
-export function duplicateModel<M extends AnyModel["model"], T extends ExtractModel<AnyModel, M>>(
-  model: T | null,
-) {
+export async function duplicateModel<
+  M extends AnyModel["model"],
+  T extends ExtractModel<AnyModel, M>,
+>(model: T | null): Promise<string> {
   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( (?<n>\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<string>("models_duplicate", { model: { ...model, name } }));
+  return trackModelWrite(
+    invoke<string>("models_duplicate", { modelType: model.model, modelId: model.id }),
+  );
 }
 
 export async function createGlobalModel<T extends Exclude<AnyModel, { workspaceId: string }>>(

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::<u64>() {
+                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), "");
+    }
+}

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<Environment> {
         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::<Vec<_>>();
+        environment.name = conflict_free_name(&environment.name, &sibling_names);
         self.upsert_environment(&environment, source)
     }
 

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<Folder> {
         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::<Vec<_>>();
+        folder.name = conflict_free_name(&folder.name, &sibling_names);
+        let new_folder = self.upsert_folder(&folder, source)?;
 
         for m in self.find_many::<HttpRequest>(HttpRequestIden::FolderId, fid, None)? {
             self.upsert_http_request(

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::<Vec<_>>();
+        request.name = conflict_free_name(&request.name, &sibling_names);
         self.upsert(&request, source)
     }
 

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::<Vec<_>>();
+        http_request.name = conflict_free_name(&http_request.name, &sibling_names);
         self.upsert(&http_request, source)
     }
 

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;
 

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::<Vec<_>>();
+        websocket_request.name = conflict_free_name(&websocket_request.name, &sibling_names);
         self.upsert(&websocket_request, source)
     }