Fix save_graph GQL mutation so it does not create multiple files for …

…the same graph name (#1419)

* add basic test for save graph and reloading

* better error message in case of multi-defined graph

* if graph with new name already exists, overwrite the file instead of creating a new one

* print error output and check for query errors

* use portable path manipulation in save

raphtory-graphql/src/data.rs

@@ -2,7 +2,7 @@ use parking_lot::RwLock;
 use raphtory::{
     core::Prop,
     db::api::view::MaterializedGraph,
-    prelude::{GraphViewOps, PropertyAdditionOps},
+    prelude::{GraphViewOps, PropUnwrap, PropertyAdditionOps},
     search::IndexedGraph,
     vectors::{document_template::DocumentTemplate, vectorised_graph::VectorisedGraph},
 };
@@ -70,58 +70,40 @@ impl Data {
     }
 
     pub fn load_from_file(path: &str) -> HashMap<String, IndexedGraph<MaterializedGraph>> {
-        let mut valid_paths = HashSet::<String>::new();
+        let valid_entries = WalkDir::new(path).into_iter().filter_map(|e| {
+            let entry = e.ok()?;
+            let path = entry.path();
+            let filename = path.file_name().and_then(|name| name.to_str())?;
+            (path.is_file() && !filename.starts_with('.')).then_some(entry)
+        });
+
+        let mut graphs: HashMap<String, IndexedGraph<MaterializedGraph>> = HashMap::default();
 
-        for entry in WalkDir::new(path).into_iter().filter_map(|e| e.ok()) {
+        for entry in valid_entries {
             let path = entry.path();
             let path_string = path.display().to_string();
-            let filename = path.file_name().and_then(|name| name.to_str());
-            if let Some(filename) = filename {
-                if path.is_file() && !filename.starts_with('.') {
-                    valid_paths.insert(path_string);
-                }
-            }
-        }
+            println!("loading graph from {path_string}");
+            let graph = MaterializedGraph::load_from_file(path).expect("Unable to load from graph");
+            let graph_name = graph
+                .properties()
+                .get("name")
+                .into_str()
+                .map(|v| v.to_string())
+                .unwrap_or_else(|| path.file_name().unwrap().to_str().unwrap().to_owned());
+            graph
+                .update_constant_properties([("path".to_string(), Prop::str(path_string.clone()))])
+                .expect("Failed to add static property");
 
-        let mut graphs_loaded: Vec<String> = vec![];
-        let mut is_graph_already_loaded = |graph_name: String| {
-            if graphs_loaded.contains(&graph_name) {
-                panic!("Graph by name {} is already loaded", graph_name);
-            } else {
-                graphs_loaded.push(graph_name);
+            if let Some(old_graph) = graphs.insert(
+                graph_name,
+                IndexedGraph::from_graph(&graph).expect("Unable to index graph"),
+            ) {
+                // insertion returns the old value if the entry already existed
+                let old_path = old_graph.properties().get("path").unwrap_str();
+                let name = old_graph.properties().get("name").unwrap_str();
+                panic!("Graph with name {name} defined multiple times, first file: {old_path}, second file: {path_string}")
             }
-        };
-
-        let graphs: HashMap<String, IndexedGraph<MaterializedGraph>> = valid_paths
-            .into_iter()
-            .map(|path| {
-                println!("loading graph from {path}");
-                let graph =
-                    MaterializedGraph::load_from_file(&path).expect("Unable to load from graph");
-                graph
-                    .update_constant_properties([("path".to_string(), Prop::str(path.clone()))])
-                    .expect("Failed to add static property");
-                let maybe_graph_name = graph.properties().get("name");
-
-                return match maybe_graph_name {
-                    None => {
-                        let graph_name = Path::new(&path).file_name().unwrap().to_str().unwrap();
-                        is_graph_already_loaded(graph_name.to_string());
-                        (graph_name.to_string(), graph)
-                    }
-                    Some(graph_name) => {
-                        is_graph_already_loaded(graph_name.to_string());
-                        (graph_name.to_string(), graph)
-                    }
-                };
-            })
-            .map(|(name, g)| {
-                (
-                    name,
-                    IndexedGraph::from_graph(&g).expect("Unable to index graph"),
-                )
-            })
-            .collect();
+        }
         graphs
     }
 }

raphtory-graphql/src/lib.rs

@@ -325,6 +325,21 @@ mod graphql_test {
             test_dir_path
         );
 
+        let save_graph = |parent_name: &str, nodes: &str| {
+            format!(
+                r#"mutation {{
+                  saveGraph(
+                    parentGraphName: "{parent_name}",
+                    graphName: "{parent_name}",
+                    newGraphName: "g2",
+                    props: "{{}}",
+                    isArchive: 0,
+                    graphNodes: {nodes},
+                  )
+              }}"#
+            )
+        };
+
         // only g0 which is empty
         let req = Request::new(load_all);
         let res = schema.execute(req).await;
@@ -376,6 +391,37 @@ mod graphql_test {
         let res = schema.execute(req).await;
         let res_json = res.data.into_json().unwrap();
         assert_eq!(res_json, json!({"graph": {"nodes": [{"id": "1"}]}}));
+
+        // test save graph
+        let req = Request::new(save_graph("g0", r#"["2"]"#));
+        let res = schema.execute(req).await;
+        println!("{:?}", res.errors);
+        assert!(res.errors.is_empty());
+        let req = Request::new(list_nodes("g2"));
+        let res = schema.execute(req).await;
+        let res_json = res.data.into_json().unwrap();
+        assert_eq!(res_json, json!({"graph": {"nodes": [{"id": "2"}]}}));
+
+        // test save graph overwrite
+        let req = Request::new(save_graph("g1", r#"["1"]"#));
+        let res = schema.execute(req).await;
+        println!("{:?}", res.errors);
+        assert!(res.errors.is_empty());
+        let req = Request::new(list_nodes("g2"));
+        let res = schema.execute(req).await;
+        println!("{:?}", res);
+        let res_json = res.data.into_json().unwrap();
+        assert_eq!(res_json, json!({"graph": {"nodes": [{"id": "1"}]}}));
+
+        // reload all graphs from folder
+        let req = Request::new(load_all);
+        schema.execute(req).await;
+        // g2 is still the last version
+        let req = Request::new(list_nodes("g2"));
+        let res = schema.execute(req).await;
+        println!("{:?}", res);
+        let res_json = res.data.into_json().unwrap();
+        assert_eq!(res_json, json!({"graph": {"nodes": [{"id": "1"}]}}));
     }
 
     #[tokio::test]

raphtory-graphql/src/model/mod.rs

@@ -21,15 +21,14 @@ use std::{
     error::Error,
     fmt::{Display, Formatter},
     io::BufReader,
+    path::Path,
 };
-use utils::path_prefix;
 use uuid::Uuid;
 
 pub mod algorithms;
 pub(crate) mod filters;
 pub(crate) mod graph;
 pub(crate) mod schema;
-pub(crate) mod utils;
 
 #[derive(Debug)]
 pub struct MissingGraph;
@@ -177,16 +176,29 @@ impl Mut {
         let mut data = ctx.data_unchecked::<Data>().graphs.write();
 
         let subgraph = data.get(&graph_name).ok_or("Graph not found")?;
-        let mut path = subgraph
-            .properties()
-            .constant()
-            .get("path")
-            .ok_or("Path is missing")?
-            .to_string();
 
-        if new_graph_name.ne(&graph_name) {
-            path = path_prefix(path)? + "/" + &Uuid::new_v4().hyphenated().to_string();
-        }
+        let path = match data.get(&new_graph_name) {
+            Some(new_graph) => new_graph
+                .properties()
+                .constant()
+                .get("path")
+                .ok_or("Path is missing")?
+                .to_string(),
+            None => {
+                let base_path = subgraph
+                    .properties()
+                    .constant()
+                    .get("path")
+                    .ok_or("Path is missing")?
+                    .to_string();
+                let path: &Path = Path::new(base_path.as_str());
+                path.with_file_name(Uuid::new_v4().hyphenated().to_string())
+                    .to_str()
+                    .ok_or("Invalid path")?
+                    .to_string()
+            }
+        };
+        println!("Saving graph to path {path}");
 
         let parent_graph = data.get(&parent_graph_name).ok_or("Graph not found")?;