From 38e0c741b181cdaeb883a5c0cc33f1bc61a81505 Mon Sep 17 00:00:00 2001 From: Louis Chan Date: Fri, 8 May 2026 17:32:39 +0100 Subject: [PATCH 1/6] Add namespace creation/deletion to graphql Add TestSetup struct, setup_with_graphs, run_mutation, and assert_is_namespace_dir helpers to mod graphql_test in raphtory-graphql/src/lib.rs for use by namespace tests in later tasks. Previously called validate_path_for_insert which created a graph-folder skeleton + dirty marker on disk and leaked them, so the new namespace appeared as a MetaGraph. Now uses validate_path_for_namespace_create plus fs::create_dir_all. test: createNamespace creates nested directories test: createNamespace rejects path of existing graph test: createNamespace rejects path of existing namespace test: createNamespace rejects invalid paths test: tighten createNamespace existing-namespace error check test: add FakePolicy and setup_with_policy helpers test: createNamespace denied without parent write test: tighten FakePolicy docs and silence dead-code warning test: deleteNamespace removes empty namespace test: deleteNamespace removes namespace with children test: deleteNamespace rejects empty path test: deleteNamespace rejects non-existent path test: deleteNamespace denied when descendant graph unwritable test: deleteNamespace invalidates cached graphs test: clarify deleteNamespace denied-test comments feat(graphql): deleteNamespace infrastructure - auth.rs: add is_exclusive_write so deleteNamespace acquires the exclusive write lock alongside updateGraph - namespace.rs: expose current_dir() and relative_path() accessors used by Mut::delete_namespace and the data layer --- raphtory-graphql/schema.graphql | 32 +- raphtory-graphql/src/auth.rs | 12 +- raphtory-graphql/src/auth_policy.rs | 57 +++ raphtory-graphql/src/data.rs | 43 +++ raphtory-graphql/src/lib.rs | 344 ++++++++++++++++++ raphtory-graphql/src/model/graph/namespace.rs | 10 +- raphtory-graphql/src/model/mod.rs | 66 +++- raphtory-graphql/src/paths.rs | 31 ++ raphtory-graphql/src/test_support.rs | 95 +++++ 9 files changed, 683 insertions(+), 7 deletions(-) create mode 100644 raphtory-graphql/src/test_support.rs diff --git a/raphtory-graphql/schema.graphql b/raphtory-graphql/schema.graphql index 14c9295b18..692957f058 100644 --- a/raphtory-graphql/schema.graphql +++ b/raphtory-graphql/schema.graphql @@ -2724,6 +2724,37 @@ type MutRoot { overwrite: Boolean! ): String! """ + Create an empty namespace at `path`. + + Creates any missing parent namespaces along the way. Requires WRITE + permission on the parent namespace. Rejects paths that already host a + graph or an existing namespace, and paths that fail validation. + + Returns:: the path of the created namespace + """ + createNamespace( + """ + Destination path relative to the root namespace. + """ + path: String! + ): String! + """ + Delete a namespace and all of its descendants (graphs and sub-namespaces). + + Requires WRITE permission on the parent namespace, on the namespace + itself, and on every descendant graph and sub-namespace. Cached graphs + at any deleted path are invalidated. Rejects empty and non-existent + paths. + + Returns:: true on success + """ + deleteNamespace( + """ + Path to delete relative to the root namespace. + """ + path: String! + ): Boolean! + """ Returns a subgraph given a set of nodes from an existing graph in the server. Returns:: @@ -5583,4 +5614,3 @@ schema { query: QueryRoot mutation: MutRoot } - diff --git a/raphtory-graphql/src/auth.rs b/raphtory-graphql/src/auth.rs index 1d11fcabfe..4fe421c4fb 100644 --- a/raphtory-graphql/src/auth.rs +++ b/raphtory-graphql/src/auth.rs @@ -183,10 +183,10 @@ where let req = batch_req.data(access).data(role); let contains_update = match &req { - BatchRequest::Single(request) => request.query.contains("updateGraph"), + BatchRequest::Single(request) => is_exclusive_write(&request.query), BatchRequest::Batch(requests) => requests .iter() - .any(|request| request.query.contains("updateGraph")), + .any(|request| is_exclusive_write(&request.query)), }; if contains_update { if let Some(lock) = &self.lock { @@ -207,6 +207,14 @@ where } } +/// Operations that must run with exclusive access to the graph store, blocking +/// all other concurrent requests. `deleteNamespace` is included because it +/// removes a whole subtree and would race with concurrent inserts/deletes +/// inside that subtree. +fn is_exclusive_write(query: &str) -> bool { + query.contains("updateGraph") || query.contains("deleteNamespace") +} + fn is_query_heavy(query: &str) -> bool { query.contains("outComponent") || query.contains("inComponent") diff --git a/raphtory-graphql/src/auth_policy.rs b/raphtory-graphql/src/auth_policy.rs index e6b78327fe..0e876c4f04 100644 --- a/raphtory-graphql/src/auth_policy.rs +++ b/raphtory-graphql/src/auth_policy.rs @@ -136,3 +136,60 @@ pub trait AuthorizationPolicy: Send + Sync + 'static { path: &str, ) -> NamespacePermission; } + +#[cfg(test)] +pub(crate) mod auth_policy_tests { + use super::{ + AuthPolicyError, AuthorizationPolicy, GraphPermission, NamespacePermission, + }; + use std::collections::HashMap; + + /// Test-only authorization policy: every path must be configured explicitly via + /// [`Self::with_namespace`] / [`Self::with_graph`]. Unknown namespaces default + /// to `NamespacePermission::Denied` and unknown graphs return `Err`. This is + /// stricter than the production policy's fail-open contract — that's + /// intentional, so a missing `with_*` call in a test surfaces as an obvious + /// failure rather than as a silent allow. + #[derive(Default)] + pub(crate) struct FakePolicy { + namespaces: HashMap, + graphs: HashMap, + } + + #[allow(dead_code)] + impl FakePolicy { + pub(crate) fn with_namespace(mut self, path: &str, perm: NamespacePermission) -> Self { + self.namespaces.insert(path.to_string(), perm); + self + } + pub(crate) fn with_graph(mut self, path: &str, perm: GraphPermission) -> Self { + self.graphs.insert(path.to_string(), perm); + self + } + } + + impl AuthorizationPolicy for FakePolicy { + fn graph_permissions( + &self, + _ctx: &async_graphql::Context<'_>, + path: &str, + ) -> Result { + match self.graphs.get(path) { + Some(p) => Ok(p.clone()), + None => Err(AuthPolicyError::new(format!( + "no permission for graph {path}" + ))), + } + } + fn namespace_permissions( + &self, + _ctx: &async_graphql::Context<'_>, + path: &str, + ) -> NamespacePermission { + self.namespaces + .get(path) + .cloned() + .unwrap_or(NamespacePermission::Denied) + } + } +} diff --git a/raphtory-graphql/src/data.rs b/raphtory-graphql/src/data.rs index 989a706eaf..c6f0fd91bc 100644 --- a/raphtory-graphql/src/data.rs +++ b/raphtory-graphql/src/data.rs @@ -7,6 +7,8 @@ use crate::{ blocking_io, graph::{ filtering::{GraphAccessFilter, GraphRowFilter, HiddenKeys}, + namespace::Namespace, + namespaced_item::NamespacedItem, vectorised_graph::GqlVectorisedGraph, }, }, @@ -323,6 +325,47 @@ impl Data { Ok(()) } + pub async fn delete_namespace(&self, path: &str) -> Result<(), DeletionError> { + if path.is_empty() { + return Err(DeletionError::PathValidation( + PathValidationError::NamespaceDoesNotExist(path.to_string()), + )); + } + let namespace = Namespace::try_new(self.work_dir.clone(), path.to_string())?; + for item in namespace.get_all_children() { + if let NamespacedItem::MetaGraph(g) = item { + self.invalidate(g.local_path()).await; + self.cache.remove(g.local_path()).await; + } + } + let root = namespace.current_dir().to_path_buf(); + let dirty_file = mark_dirty(&root).map_err(|err| { + DeletionError::from_inner(path, MutationErrorInner::InvalidInternal(err)) + })?; + blocking_io(move || { + fs::remove_dir_all(&root)?; + fs::remove_file(dirty_file)?; + Ok::<_, MutationErrorInner>(()) + }) + .await + .map_err(|err| DeletionError::from_inner(path, err))?; + Ok(()) + } + + pub async fn create_namespace(&self, path: &str) -> Result<(), InsertionError> { + let target = crate::paths::validate_path_for_namespace_create( + self.work_dir.clone(), + path, + )?; + blocking_io(move || { + fs::create_dir_all(&target)?; + Ok::<_, MutationErrorInner>(()) + }) + .await + .map_err(|err| InsertionError::from_inner(path, err))?; + Ok(()) + } + async fn vectorise_with_template( &self, graph: MaterializedGraph, diff --git a/raphtory-graphql/src/lib.rs b/raphtory-graphql/src/lib.rs index 5a4c7b0cf6..8053647f5f 100644 --- a/raphtory-graphql/src/lib.rs +++ b/raphtory-graphql/src/lib.rs @@ -20,6 +20,9 @@ mod routes; pub mod server; pub mod url_encode; +#[cfg(test)] +pub(crate) mod test_support; + pub mod cli; pub mod config; #[cfg(feature = "python")] @@ -47,11 +50,17 @@ mod graphql_test { use crate::config::app_config::AppConfigBuilder; use crate::{ auth::Access, + auth_policy::auth_policy_tests::FakePolicy, config::app_config::AppConfig, data::{data_tests::save_graphs_to_work_dir, Data}, model::App, + test_support::{ + assert_is_namespace_dir, run_mutation, run_mutation_as_user, + setup_with_graphs, setup_with_policy, + }, url_encode::{url_decode_graph_at, url_encode_graph}, }; + use crate::auth_policy::{GraphPermission, NamespacePermission}; use async_graphql::{dynamic::Schema, UploadValue}; use dynamic_graphql::{Request, Variables}; use itertools::Itertools; @@ -72,6 +81,7 @@ mod graphql_test { use std::{ collections::{HashMap, HashSet}, fs, + sync::Arc, }; use tempfile::tempdir; @@ -1927,4 +1937,338 @@ mod graphql_test { "node types returned by GraphQL should match those set on ingest" ); } + + #[tokio::test] + async fn test_create_namespace_at_root() { + let setup = setup_with_graphs(&[]).await; + + let res = run_mutation( + &setup.schema, + r#"mutation { createNamespace(path: "foo") }"#, + ) + .await; + + assert_eq!(res.errors, vec![]); + assert_eq!( + res.data.into_json().unwrap(), + json!({ "createNamespace": "foo" }), + ); + + let foo = setup.tmp.path().join("foo"); + assert_is_namespace_dir(&foo); + + let req = Request::new( + r#"{ namespace(path: "") { items { list { __typename ... on Namespace { path } ... on MetaGraph { path } } } } }"#, + ); + let res = setup.schema.execute(req).await; + assert_eq!(res.errors, vec![]); + assert_eq!( + res.data.into_json().unwrap(), + json!({ + "namespace": { + "items": { + "list": [ + { "__typename": "Namespace", "path": "foo" } + ] + } + } + }), + ); + } + + #[tokio::test] + async fn test_create_namespace_nested() { + let setup = setup_with_graphs(&[]).await; + + let res = run_mutation( + &setup.schema, + r#"mutation { createNamespace(path: "a/b/c") }"#, + ) + .await; + assert_eq!(res.errors, vec![]); + assert_eq!( + res.data.into_json().unwrap(), + json!({ "createNamespace": "a/b/c" }), + ); + + for rel in ["a", "a/b", "a/b/c"] { + let p = setup.tmp.path().join(rel); + assert_is_namespace_dir(&p); + } + + let req = Request::new( + r#"{ namespace(path: "a/b") { children { list { path } } } }"#, + ); + let res = setup.schema.execute(req).await; + assert_eq!(res.errors, vec![]); + assert_eq!( + res.data.into_json().unwrap(), + json!({ + "namespace": { + "children": { "list": [ { "path": "a/b/c" } ] } + } + }), + ); + } + + #[tokio::test] + async fn test_create_namespace_rejects_existing_graph() { + let g = Graph::new(); + g.add_node(0, 1, NO_PROPS, None, None).unwrap(); + let g: MaterializedGraph = g.into(); + let setup = setup_with_graphs(&[("g", g)]).await; + + let res = run_mutation( + &setup.schema, + r#"mutation { createNamespace(path: "g") }"#, + ) + .await; + assert!(!res.errors.is_empty(), "expected error, got {:?}", res); + + assert!(setup.data.get_graph_for_test("g").await.is_ok()); + } + + #[tokio::test] + async fn test_create_namespace_rejects_existing_namespace() { + let setup = setup_with_graphs(&[]).await; + + let res = run_mutation( + &setup.schema, + r#"mutation { createNamespace(path: "ns") }"#, + ) + .await; + assert_eq!(res.errors, vec![]); + + let res = run_mutation( + &setup.schema, + r#"mutation { createNamespace(path: "ns") }"#, + ) + .await; + assert!(!res.errors.is_empty(), "expected error, got {:?}", res); + assert!( + res.errors[0].message.contains("Namespace"), + "unexpected error message: {}", + res.errors[0].message, + ); + } + + #[tokio::test] + async fn test_create_namespace_rejects_invalid_paths() { + let setup = setup_with_graphs(&[]).await; + + let cases = [ + "", + ".hidden/x", + "x/.hidden", + "../escape", + "a//b", + ]; + + let snapshot_before = std::fs::read_dir(setup.tmp.path()) + .unwrap() + .map(|e| e.unwrap().file_name()) + .collect::>(); + + for path in cases { + let query = format!( + r#"mutation {{ createNamespace(path: "{}") }}"#, + path.replace('"', r#"\""#), + ); + let res = run_mutation(&setup.schema, &query).await; + assert!( + !res.errors.is_empty(), + "expected error for path {:?}, got {:?}", + path, + res, + ); + } + + let snapshot_after = std::fs::read_dir(setup.tmp.path()) + .unwrap() + .map(|e| e.unwrap().file_name()) + .collect::>(); + assert_eq!( + snapshot_before, snapshot_after, + "work_dir contents changed after rejected creates", + ); + } + + #[tokio::test] + async fn test_create_namespace_denied_without_parent_write() { + let policy = Arc::new( + FakePolicy::default() + .with_namespace("", NamespacePermission::Read), + ); + let setup = setup_with_policy(&[], policy).await; + + let res = run_mutation_as_user( + &setup.schema, + r#"mutation { createNamespace(path: "foo") }"#, + ) + .await; + assert!(!res.errors.is_empty(), "expected error, got {:?}", res); + assert!( + res.errors[0].message.contains("WRITE required on namespace"), + "unexpected error message: {}", + res.errors[0].message, + ); + + assert!(!setup.tmp.path().join("foo").exists()); + } + + #[tokio::test] + async fn test_delete_namespace_empty() { + let setup = setup_with_graphs(&[]).await; + + let res = run_mutation( + &setup.schema, + r#"mutation { createNamespace(path: "ns") }"#, + ) + .await; + assert_eq!(res.errors, vec![]); + assert!(setup.tmp.path().join("ns").is_dir()); + + let res = run_mutation( + &setup.schema, + r#"mutation { deleteNamespace(path: "ns") }"#, + ) + .await; + assert_eq!(res.errors, vec![]); + assert_eq!( + res.data.into_json().unwrap(), + json!({ "deleteNamespace": true }), + ); + assert!(!setup.tmp.path().join("ns").exists()); + assert_is_namespace_dir(setup.tmp.path()); + } + + #[tokio::test] + async fn test_delete_namespace_with_children() { + let g1 = Graph::new(); + g1.add_node(0, 1, NO_PROPS, None, None).unwrap(); + let g1: MaterializedGraph = g1.into(); + let g2 = Graph::new(); + g2.add_node(0, 2, NO_PROPS, None, None).unwrap(); + let g2: MaterializedGraph = g2.into(); + let setup = setup_with_graphs(&[("ns/g1", g1), ("ns/sub/g2", g2)]).await; + + let res = run_mutation( + &setup.schema, + r#"mutation { createNamespace(path: "ns/empty") }"#, + ) + .await; + assert_eq!(res.errors, vec![]); + assert!(setup.tmp.path().join("ns/empty").is_dir()); + + let res = run_mutation( + &setup.schema, + r#"mutation { deleteNamespace(path: "ns") }"#, + ) + .await; + assert_eq!(res.errors, vec![]); + assert_eq!( + res.data.into_json().unwrap(), + json!({ "deleteNamespace": true }), + ); + assert!(!setup.tmp.path().join("ns").exists()); + + let req = Request::new( + r#"{ namespace(path: "") { children { list { path } } } }"#, + ); + let res = setup.schema.execute(req).await; + assert_eq!(res.errors, vec![]); + assert_eq!( + res.data.into_json().unwrap(), + json!({ "namespace": { "children": { "list": [] } } }), + ); + } + + #[tokio::test] + async fn test_delete_namespace_rejects_empty_path() { + let setup = setup_with_graphs(&[]).await; + + let res = run_mutation( + &setup.schema, + r#"mutation { deleteNamespace(path: "") }"#, + ) + .await; + assert!(!res.errors.is_empty(), "expected error, got {:?}", res); + } + + #[tokio::test] + async fn test_delete_namespace_rejects_nonexistent() { + let setup = setup_with_graphs(&[]).await; + + let res = run_mutation( + &setup.schema, + r#"mutation { deleteNamespace(path: "noexist") }"#, + ) + .await; + assert!(!res.errors.is_empty(), "expected error, got {:?}", res); + } + + #[tokio::test] + async fn test_delete_namespace_denied_when_descendant_unwritable() { + let g1 = Graph::new(); + g1.add_node(0, 1, NO_PROPS, None, None).unwrap(); + let g1: MaterializedGraph = g1.into(); + let g2 = Graph::new(); + g2.add_node(0, 2, NO_PROPS, None, None).unwrap(); + let g2: MaterializedGraph = g2.into(); + + let policy = Arc::new( + FakePolicy::default() + .with_namespace("", NamespacePermission::Write) + .with_namespace("ns", NamespacePermission::Write) + .with_graph("ns/g1", GraphPermission::Write) + .with_graph( + "ns/g2", + GraphPermission::Read { filter: None }, + ), + ); + let setup = setup_with_policy(&[("ns/g1", g1), ("ns/g2", g2)], policy).await; + + let res = run_mutation_as_user( + &setup.schema, + r#"mutation { deleteNamespace(path: "ns") }"#, + ) + .await; + assert!(!res.errors.is_empty(), "expected error, got {:?}", res); + // Substring is from `require_graph_write` in raphtory-graphql/src/model/mod.rs. + assert!( + res.errors[0] + .message + .contains("WRITE permission required for graph"), + "unexpected error message: {}", + res.errors[0].message, + ); + + assert!(setup.tmp.path().join("ns").is_dir()); + assert!(setup.data.get_graph_for_test("ns/g1").await.is_ok()); + assert!(setup.data.get_graph_for_test("ns/g2").await.is_ok()); + } + + #[tokio::test] + async fn test_delete_namespace_invalidates_cache() { + let g = Graph::new(); + g.add_node(0, 1, NO_PROPS, None, None).unwrap(); + let g: MaterializedGraph = g.into(); + let setup = setup_with_graphs(&[("ns/g", g)]).await; + + setup.data.get_graph_for_test("ns/g").await.unwrap(); + assert!(setup.data.get_cached_graph("ns/g").await.is_some()); + + let res = run_mutation( + &setup.schema, + r#"mutation { deleteNamespace(path: "ns") }"#, + ) + .await; + assert_eq!(res.errors, vec![]); + assert_eq!( + res.data.into_json().unwrap(), + json!({ "deleteNamespace": true }), + ); + + assert!(setup.data.get_cached_graph("ns/g").await.is_none()); + } } diff --git a/raphtory-graphql/src/model/graph/namespace.rs b/raphtory-graphql/src/model/graph/namespace.rs index cdf13fb9bf..b0c9a2f702 100644 --- a/raphtory-graphql/src/model/graph/namespace.rs +++ b/raphtory-graphql/src/model/graph/namespace.rs @@ -135,10 +135,18 @@ impl Namespace { /// Recursively list all children pub fn get_all_children(&self) -> impl Iterator { - let it = WalkDir::new(&self.current_dir).into_iter(); + let it = WalkDir::new(&self.current_dir).min_depth(1).into_iter(); let root = self.clone(); NamespaceIter { it, root } } + + pub fn current_dir(&self) -> &std::path::Path { + &self.current_dir + } + + pub fn relative_path(&self) -> &str { + &self.relative_path + } } fn is_graph_visible( diff --git a/raphtory-graphql/src/model/mod.rs b/raphtory-graphql/src/model/mod.rs index 41b846552a..fa795b657c 100644 --- a/raphtory-graphql/src/model/mod.rs +++ b/raphtory-graphql/src/model/mod.rs @@ -282,11 +282,11 @@ impl QueryRoot { let data = ctx.data_unchecked::(); let root = Namespace::root(data.work_dir.clone()); let list = blocking_compute(move || { - root.get_all_children() - .filter_map(|child| match child { + std::iter::once(root.clone()) + .chain(root.get_all_children().filter_map(|child| match child { NamespacedItem::Namespace(item) => Some(item), NamespacedItem::MetaGraph(_) => None, - }) + })) .sorted() .collect() }) @@ -493,6 +493,66 @@ impl Mut { Ok(path.to_owned()) } + /// Create an empty namespace at `path`. + /// + /// Creates any missing parent namespaces along the way. Requires WRITE + /// permission on the parent namespace. Rejects paths that already host a + /// graph or an existing namespace, and paths that fail validation. + /// + /// Returns:: the path of the created namespace + async fn create_namespace<'a>( + ctx: &Context<'a>, + #[graphql(desc = "Destination path relative to the root namespace.")] path: &str, + ) -> Result { + let data = ctx.data_unchecked::(); + let ns = parent_namespace(path); + require_namespace_write(ctx, &data.auth_policy, ns, path, "create")?; + data.create_namespace(path).await?; + Ok(path.to_string()) + } + + /// Delete a namespace and all of its descendants (graphs and sub-namespaces). + /// + /// Requires WRITE permission on the parent namespace, on the namespace + /// itself, and on every descendant graph and sub-namespace. Cached graphs + /// at any deleted path are invalidated. Rejects empty and non-existent + /// paths. + /// + /// Returns:: true on success + async fn delete_namespace<'a>( + ctx: &Context<'a>, + #[graphql(desc = "Path to delete relative to the root namespace.")] path: &str, + ) -> Result { + let data = ctx.data_unchecked::(); + let parent_ns = parent_namespace(path); + require_namespace_write(ctx, &data.auth_policy, parent_ns, path, "delete")?; + require_namespace_write(ctx, &data.auth_policy, path, path, "delete")?; + + let namespace = Namespace::try_new(data.work_dir.clone(), path.to_string())?; + let ns_clone = namespace.clone(); + let descendants: Vec = + blocking_compute(move || ns_clone.get_all_children().collect()).await; + for item in &descendants { + match item { + NamespacedItem::Namespace(n) => { + require_namespace_write( + ctx, + &data.auth_policy, + n.relative_path(), + path, + "delete", + )?; + } + NamespacedItem::MetaGraph(g) => { + require_graph_write(ctx, &data.auth_policy, g.local_path())?; + } + } + } + + data.delete_namespace(path).await?; + Ok(true) + } + /// Returns a subgraph given a set of nodes from an existing graph in the server. /// /// Returns:: diff --git a/raphtory-graphql/src/paths.rs b/raphtory-graphql/src/paths.rs index b3f8093663..1a6fd18917 100644 --- a/raphtory-graphql/src/paths.rs +++ b/raphtory-graphql/src/paths.rs @@ -84,6 +84,33 @@ impl ValidPath { } } +/// Validate a path for use as a *new* namespace directory. +/// +/// Returns the absolute path that should be created. Does not create the +/// target directory. Errors when the path is empty, contains invalid +/// components, or already exists as either a graph folder or a namespace +/// directory. +pub fn validate_path_for_namespace_create( + base_path: PathBuf, + relative_path: &str, +) -> Result { + if relative_path.is_empty() { + return Err(PathValidationError::EmptyPath); + } + let valid = ValidPath::try_new(base_path, relative_path)?; + if valid.is_graph() { + return Err(PathValidationError::GraphExistsError( + relative_path.to_string(), + )); + } + if valid.is_namespace() { + return Err(PathValidationError::NamespaceExistsError( + relative_path.to_string(), + )); + } + Ok(valid.into_path()) +} + #[derive(Clone, Debug, PartialOrd, PartialEq, Ord, Eq)] pub struct ExistingGraphFolder(pub(crate) ValidGraphFolder); @@ -459,6 +486,10 @@ impl From for InternalPathValidationError { #[derive(thiserror::Error, Debug)] pub enum PathValidationError { + #[error("Path is empty")] + EmptyPath, + #[error("Namespace '{0}' already exists")] + NamespaceExistsError(String), #[error("Graph '{0}' already exists")] GraphExistsError(String), #[error("Graph '{0}' does not exist")] diff --git a/raphtory-graphql/src/test_support.rs b/raphtory-graphql/src/test_support.rs new file mode 100644 index 0000000000..e9995191e3 --- /dev/null +++ b/raphtory-graphql/src/test_support.rs @@ -0,0 +1,95 @@ +//! Shared GraphQL test scaffolding: schema/fixture builders and single-mutation +//! runners used by the integration-style tests in `lib.rs`. Gated by `#[cfg(test)]` +//! and only intended for in-crate consumers. + +#![allow(dead_code)] + +use crate::{ + auth::Access, + auth_policy::AuthorizationPolicy, + config::app_config::AppConfig, + data::{Data, DIRTY_PATH}, + model::App, +}; +use async_graphql::dynamic::Schema; +use dynamic_graphql::Request; +use raphtory::{ + db::api::{storage::storage::Config, view::MaterializedGraph}, + serialise::ROOT_META_PATH, +}; +use std::{path::Path, sync::Arc}; +use tempfile::{tempdir, TempDir}; + +pub(crate) struct TestSetup { + pub(crate) tmp: TempDir, + pub(crate) data: Data, + pub(crate) schema: Schema, +} + +pub(crate) async fn setup_with_graphs( + graphs: &[(&str, MaterializedGraph)], +) -> TestSetup { + let tmp = tempdir().unwrap(); + let data = Data::new(tmp.path(), &AppConfig::default(), Config::default()); + for (path, graph) in graphs { + let folder = data.validate_path_for_insert(path, false).unwrap(); + data.insert_graph(folder, graph.clone()).await.unwrap(); + } + let schema = App::create_schema().data(data.clone()).finish().unwrap(); + TestSetup { tmp, data, schema } +} + +pub(crate) async fn setup_with_policy( + graphs: &[(&str, MaterializedGraph)], + policy: Arc, +) -> TestSetup { + let tmp = tempdir().unwrap(); + let mut data = Data::new(tmp.path(), &AppConfig::default(), Config::default()); + for (path, graph) in graphs { + let folder = data.validate_path_for_insert(path, false).unwrap(); + data.insert_graph(folder, graph.clone()).await.unwrap(); + } + data.set_auth_policy(policy); + let schema = App::create_schema().data(data.clone()).finish().unwrap(); + TestSetup { tmp, data, schema } +} + +pub(crate) async fn run_mutation( + schema: &Schema, + query: &str, +) -> async_graphql::Response { + let req = Request::new(query).data(Access::Rw); + schema.execute(req).await +} + +pub(crate) async fn run_mutation_as_user( + schema: &Schema, + query: &str, +) -> async_graphql::Response { + // No `Access::Rw` injected, so the policy decides allow/deny. + // A role (`Option`) is injected because `write_denied` in + // `model/mod.rs` returns the specific policy error message only when a + // role is present; with `None` it returns the generic + // `AuthError::RequireWrite` string, which would fail tests that assert on + // the policy-specific text. + let req = Request::new(query).data(Some("test-user".to_string())); + schema.execute(req).await +} + +/// Assert that `path` is an existing directory that is NOT a graph folder +/// (no `ROOT_META_PATH`) and contains no leftover `DIRTY_PATH` marker. +pub(crate) fn assert_is_namespace_dir(path: &Path) { + assert!(path.is_dir(), "expected directory at {:?}", path); + assert!( + !path.join(ROOT_META_PATH).exists(), + "{:?} contains a graph metadata file ({}); expected a plain namespace directory", + path, + ROOT_META_PATH, + ); + assert!( + !path.join(DIRTY_PATH).exists(), + "{:?} contains a dirty marker ({}); expected a clean namespace directory", + path, + DIRTY_PATH, + ); +} From 6f8a5bfa3a89c612d0f897698c9c067bfa2a8458 Mon Sep 17 00:00:00 2001 From: Louis Chan Date: Thu, 14 May 2026 16:42:50 +0100 Subject: [PATCH 2/6] Mark paths dirty before cache eviction and in create_namespace --- raphtory-graphql/src/data.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/raphtory-graphql/src/data.rs b/raphtory-graphql/src/data.rs index c6f0fd91bc..a47e2cc1db 100644 --- a/raphtory-graphql/src/data.rs +++ b/raphtory-graphql/src/data.rs @@ -332,16 +332,16 @@ impl Data { )); } let namespace = Namespace::try_new(self.work_dir.clone(), path.to_string())?; + let root = namespace.current_dir().to_path_buf(); + let dirty_file = mark_dirty(&root).map_err(|err| { + DeletionError::from_inner(path, MutationErrorInner::InvalidInternal(err)) + })?; for item in namespace.get_all_children() { if let NamespacedItem::MetaGraph(g) = item { self.invalidate(g.local_path()).await; self.cache.remove(g.local_path()).await; } } - let root = namespace.current_dir().to_path_buf(); - let dirty_file = mark_dirty(&root).map_err(|err| { - DeletionError::from_inner(path, MutationErrorInner::InvalidInternal(err)) - })?; blocking_io(move || { fs::remove_dir_all(&root)?; fs::remove_file(dirty_file)?; @@ -357,8 +357,19 @@ impl Data { self.work_dir.clone(), path, )?; + let mut cleanup_root = target.as_path(); + while let Some(parent) = cleanup_root.parent() { + if parent.is_dir() { + break; + } + cleanup_root = parent; + } + let dirty_file = mark_dirty(cleanup_root).map_err(|err| { + InsertionError::from_inner(path, MutationErrorInner::InvalidInternal(err)) + })?; blocking_io(move || { fs::create_dir_all(&target)?; + fs::remove_file(dirty_file)?; Ok::<_, MutationErrorInner>(()) }) .await From 3a67bceb47c03aecd8fb8fe6bf201b3f1f1df9d2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 14 May 2026 19:52:50 +0000 Subject: [PATCH 3/6] chore: apply tidy-public auto-fixes --- docs/reference/graphql/graphql_API.md | 49 ++++++++++++++ raphtory-graphql/schema.graphql | 1 + raphtory-graphql/src/auth_policy.rs | 4 +- raphtory-graphql/src/data.rs | 5 +- raphtory-graphql/src/lib.rs | 93 +++++++-------------------- raphtory-graphql/src/test_support.rs | 14 +--- 6 files changed, 77 insertions(+), 89 deletions(-) diff --git a/docs/reference/graphql/graphql_API.md b/docs/reference/graphql/graphql_API.md index c888579afa..82ff44d718 100644 --- a/docs/reference/graphql/graphql_API.md +++ b/docs/reference/graphql/graphql_API.md @@ -449,6 +449,55 @@ Base64-encoded bincode of the serialised graph. If true, replace any graph already at `path`. + + + +createNamespace +String! + + +Create an empty namespace at `path`. + +Creates any missing parent namespaces along the way. Requires WRITE +permission on the parent namespace. Rejects paths that already host a +graph or an existing namespace, and paths that fail validation. + +Returns:: the path of the created namespace + + + + +path +String! + + +Destination path relative to the root namespace. + + + + +deleteNamespace +Boolean! + + +Delete a namespace and all of its descendants (graphs and sub-namespaces). + +Requires WRITE permission on the parent namespace, on the namespace +itself, and on every descendant graph and sub-namespace. Cached graphs +at any deleted path are invalidated. Rejects empty and non-existent +paths. + +Returns:: true on success + + + + +path +String! + + +Path to delete relative to the root namespace. + diff --git a/raphtory-graphql/schema.graphql b/raphtory-graphql/schema.graphql index 692957f058..acf7bc2581 100644 --- a/raphtory-graphql/schema.graphql +++ b/raphtory-graphql/schema.graphql @@ -5614,3 +5614,4 @@ schema { query: QueryRoot mutation: MutRoot } + diff --git a/raphtory-graphql/src/auth_policy.rs b/raphtory-graphql/src/auth_policy.rs index 0e876c4f04..ecc53d11b1 100644 --- a/raphtory-graphql/src/auth_policy.rs +++ b/raphtory-graphql/src/auth_policy.rs @@ -139,9 +139,7 @@ pub trait AuthorizationPolicy: Send + Sync + 'static { #[cfg(test)] pub(crate) mod auth_policy_tests { - use super::{ - AuthPolicyError, AuthorizationPolicy, GraphPermission, NamespacePermission, - }; + use super::{AuthPolicyError, AuthorizationPolicy, GraphPermission, NamespacePermission}; use std::collections::HashMap; /// Test-only authorization policy: every path must be configured explicitly via diff --git a/raphtory-graphql/src/data.rs b/raphtory-graphql/src/data.rs index a47e2cc1db..29a41511ef 100644 --- a/raphtory-graphql/src/data.rs +++ b/raphtory-graphql/src/data.rs @@ -353,10 +353,7 @@ impl Data { } pub async fn create_namespace(&self, path: &str) -> Result<(), InsertionError> { - let target = crate::paths::validate_path_for_namespace_create( - self.work_dir.clone(), - path, - )?; + let target = crate::paths::validate_path_for_namespace_create(self.work_dir.clone(), path)?; let mut cleanup_root = target.as_path(); while let Some(parent) = cleanup_root.parent() { if parent.is_dir() { diff --git a/raphtory-graphql/src/lib.rs b/raphtory-graphql/src/lib.rs index 8053647f5f..c2828308b4 100644 --- a/raphtory-graphql/src/lib.rs +++ b/raphtory-graphql/src/lib.rs @@ -50,17 +50,16 @@ mod graphql_test { use crate::config::app_config::AppConfigBuilder; use crate::{ auth::Access, - auth_policy::auth_policy_tests::FakePolicy, + auth_policy::{auth_policy_tests::FakePolicy, GraphPermission, NamespacePermission}, config::app_config::AppConfig, data::{data_tests::save_graphs_to_work_dir, Data}, model::App, test_support::{ - assert_is_namespace_dir, run_mutation, run_mutation_as_user, - setup_with_graphs, setup_with_policy, + assert_is_namespace_dir, run_mutation, run_mutation_as_user, setup_with_graphs, + setup_with_policy, }, url_encode::{url_decode_graph_at, url_encode_graph}, }; - use crate::auth_policy::{GraphPermission, NamespacePermission}; use async_graphql::{dynamic::Schema, UploadValue}; use dynamic_graphql::{Request, Variables}; use itertools::Itertools; @@ -1996,9 +1995,7 @@ mod graphql_test { assert_is_namespace_dir(&p); } - let req = Request::new( - r#"{ namespace(path: "a/b") { children { list { path } } } }"#, - ); + let req = Request::new(r#"{ namespace(path: "a/b") { children { list { path } } } }"#); let res = setup.schema.execute(req).await; assert_eq!(res.errors, vec![]); assert_eq!( @@ -2018,11 +2015,7 @@ mod graphql_test { let g: MaterializedGraph = g.into(); let setup = setup_with_graphs(&[("g", g)]).await; - let res = run_mutation( - &setup.schema, - r#"mutation { createNamespace(path: "g") }"#, - ) - .await; + let res = run_mutation(&setup.schema, r#"mutation { createNamespace(path: "g") }"#).await; assert!(!res.errors.is_empty(), "expected error, got {:?}", res); assert!(setup.data.get_graph_for_test("g").await.is_ok()); @@ -2032,18 +2025,10 @@ mod graphql_test { async fn test_create_namespace_rejects_existing_namespace() { let setup = setup_with_graphs(&[]).await; - let res = run_mutation( - &setup.schema, - r#"mutation { createNamespace(path: "ns") }"#, - ) - .await; + let res = run_mutation(&setup.schema, r#"mutation { createNamespace(path: "ns") }"#).await; assert_eq!(res.errors, vec![]); - let res = run_mutation( - &setup.schema, - r#"mutation { createNamespace(path: "ns") }"#, - ) - .await; + let res = run_mutation(&setup.schema, r#"mutation { createNamespace(path: "ns") }"#).await; assert!(!res.errors.is_empty(), "expected error, got {:?}", res); assert!( res.errors[0].message.contains("Namespace"), @@ -2056,13 +2041,7 @@ mod graphql_test { async fn test_create_namespace_rejects_invalid_paths() { let setup = setup_with_graphs(&[]).await; - let cases = [ - "", - ".hidden/x", - "x/.hidden", - "../escape", - "a//b", - ]; + let cases = ["", ".hidden/x", "x/.hidden", "../escape", "a//b"]; let snapshot_before = std::fs::read_dir(setup.tmp.path()) .unwrap() @@ -2095,10 +2074,7 @@ mod graphql_test { #[tokio::test] async fn test_create_namespace_denied_without_parent_write() { - let policy = Arc::new( - FakePolicy::default() - .with_namespace("", NamespacePermission::Read), - ); + let policy = Arc::new(FakePolicy::default().with_namespace("", NamespacePermission::Read)); let setup = setup_with_policy(&[], policy).await; let res = run_mutation_as_user( @@ -2108,7 +2084,9 @@ mod graphql_test { .await; assert!(!res.errors.is_empty(), "expected error, got {:?}", res); assert!( - res.errors[0].message.contains("WRITE required on namespace"), + res.errors[0] + .message + .contains("WRITE required on namespace"), "unexpected error message: {}", res.errors[0].message, ); @@ -2120,19 +2098,11 @@ mod graphql_test { async fn test_delete_namespace_empty() { let setup = setup_with_graphs(&[]).await; - let res = run_mutation( - &setup.schema, - r#"mutation { createNamespace(path: "ns") }"#, - ) - .await; + let res = run_mutation(&setup.schema, r#"mutation { createNamespace(path: "ns") }"#).await; assert_eq!(res.errors, vec![]); assert!(setup.tmp.path().join("ns").is_dir()); - let res = run_mutation( - &setup.schema, - r#"mutation { deleteNamespace(path: "ns") }"#, - ) - .await; + let res = run_mutation(&setup.schema, r#"mutation { deleteNamespace(path: "ns") }"#).await; assert_eq!(res.errors, vec![]); assert_eq!( res.data.into_json().unwrap(), @@ -2160,11 +2130,7 @@ mod graphql_test { assert_eq!(res.errors, vec![]); assert!(setup.tmp.path().join("ns/empty").is_dir()); - let res = run_mutation( - &setup.schema, - r#"mutation { deleteNamespace(path: "ns") }"#, - ) - .await; + let res = run_mutation(&setup.schema, r#"mutation { deleteNamespace(path: "ns") }"#).await; assert_eq!(res.errors, vec![]); assert_eq!( res.data.into_json().unwrap(), @@ -2172,9 +2138,7 @@ mod graphql_test { ); assert!(!setup.tmp.path().join("ns").exists()); - let req = Request::new( - r#"{ namespace(path: "") { children { list { path } } } }"#, - ); + let req = Request::new(r#"{ namespace(path: "") { children { list { path } } } }"#); let res = setup.schema.execute(req).await; assert_eq!(res.errors, vec![]); assert_eq!( @@ -2187,11 +2151,7 @@ mod graphql_test { async fn test_delete_namespace_rejects_empty_path() { let setup = setup_with_graphs(&[]).await; - let res = run_mutation( - &setup.schema, - r#"mutation { deleteNamespace(path: "") }"#, - ) - .await; + let res = run_mutation(&setup.schema, r#"mutation { deleteNamespace(path: "") }"#).await; assert!(!res.errors.is_empty(), "expected error, got {:?}", res); } @@ -2221,18 +2181,13 @@ mod graphql_test { .with_namespace("", NamespacePermission::Write) .with_namespace("ns", NamespacePermission::Write) .with_graph("ns/g1", GraphPermission::Write) - .with_graph( - "ns/g2", - GraphPermission::Read { filter: None }, - ), + .with_graph("ns/g2", GraphPermission::Read { filter: None }), ); let setup = setup_with_policy(&[("ns/g1", g1), ("ns/g2", g2)], policy).await; - let res = run_mutation_as_user( - &setup.schema, - r#"mutation { deleteNamespace(path: "ns") }"#, - ) - .await; + let res = + run_mutation_as_user(&setup.schema, r#"mutation { deleteNamespace(path: "ns") }"#) + .await; assert!(!res.errors.is_empty(), "expected error, got {:?}", res); // Substring is from `require_graph_write` in raphtory-graphql/src/model/mod.rs. assert!( @@ -2258,11 +2213,7 @@ mod graphql_test { setup.data.get_graph_for_test("ns/g").await.unwrap(); assert!(setup.data.get_cached_graph("ns/g").await.is_some()); - let res = run_mutation( - &setup.schema, - r#"mutation { deleteNamespace(path: "ns") }"#, - ) - .await; + let res = run_mutation(&setup.schema, r#"mutation { deleteNamespace(path: "ns") }"#).await; assert_eq!(res.errors, vec![]); assert_eq!( res.data.into_json().unwrap(), diff --git a/raphtory-graphql/src/test_support.rs b/raphtory-graphql/src/test_support.rs index e9995191e3..1509b1c433 100644 --- a/raphtory-graphql/src/test_support.rs +++ b/raphtory-graphql/src/test_support.rs @@ -26,9 +26,7 @@ pub(crate) struct TestSetup { pub(crate) schema: Schema, } -pub(crate) async fn setup_with_graphs( - graphs: &[(&str, MaterializedGraph)], -) -> TestSetup { +pub(crate) async fn setup_with_graphs(graphs: &[(&str, MaterializedGraph)]) -> TestSetup { let tmp = tempdir().unwrap(); let data = Data::new(tmp.path(), &AppConfig::default(), Config::default()); for (path, graph) in graphs { @@ -54,18 +52,12 @@ pub(crate) async fn setup_with_policy( TestSetup { tmp, data, schema } } -pub(crate) async fn run_mutation( - schema: &Schema, - query: &str, -) -> async_graphql::Response { +pub(crate) async fn run_mutation(schema: &Schema, query: &str) -> async_graphql::Response { let req = Request::new(query).data(Access::Rw); schema.execute(req).await } -pub(crate) async fn run_mutation_as_user( - schema: &Schema, - query: &str, -) -> async_graphql::Response { +pub(crate) async fn run_mutation_as_user(schema: &Schema, query: &str) -> async_graphql::Response { // No `Access::Rw` injected, so the policy decides allow/deny. // A role (`Option`) is injected because `write_denied` in // `model/mod.rs` returns the specific policy error message only when a From 8efcd716afa73e05918ea13203acf203a978f76e Mon Sep 17 00:00:00 2001 From: Louis Chan Date: Mon, 18 May 2026 15:54:26 +0100 Subject: [PATCH 4/6] Fix race condition in create_namespace --- raphtory-graphql/src/auth.rs | 12 +++++++----- raphtory-graphql/src/data.rs | 11 +++++++---- raphtory-graphql/src/model/graph/namespace.rs | 6 ++++++ raphtory-graphql/src/model/mod.rs | 8 ++++---- 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/raphtory-graphql/src/auth.rs b/raphtory-graphql/src/auth.rs index 4fe421c4fb..99bbb6787b 100644 --- a/raphtory-graphql/src/auth.rs +++ b/raphtory-graphql/src/auth.rs @@ -207,12 +207,14 @@ where } } -/// Operations that must run with exclusive access to the graph store, blocking -/// all other concurrent requests. `deleteNamespace` is included because it -/// removes a whole subtree and would race with concurrent inserts/deletes -/// inside that subtree. fn is_exclusive_write(query: &str) -> bool { - query.contains("updateGraph") || query.contains("deleteNamespace") + is_operation(query, "updateGraph") || is_operation(query, "deleteNamespace") +} + +fn is_operation(query: &str, op: &str) -> bool { + query + .split(|c: char| !c.is_alphanumeric() && c != '_') + .any(|token| token == op) } fn is_query_heavy(query: &str) -> bool { diff --git a/raphtory-graphql/src/data.rs b/raphtory-graphql/src/data.rs index 29a41511ef..0b3ec0f841 100644 --- a/raphtory-graphql/src/data.rs +++ b/raphtory-graphql/src/data.rs @@ -325,10 +325,10 @@ impl Data { Ok(()) } - pub async fn delete_namespace(&self, path: &str) -> Result<(), DeletionError> { + pub async fn delete_namespace(&self, path: &str, descendants: &Vec) -> Result<(), DeletionError> { if path.is_empty() { return Err(DeletionError::PathValidation( - PathValidationError::NamespaceDoesNotExist(path.to_string()), + PathValidationError::EmptyPath, )); } let namespace = Namespace::try_new(self.work_dir.clone(), path.to_string())?; @@ -336,7 +336,7 @@ impl Data { let dirty_file = mark_dirty(&root).map_err(|err| { DeletionError::from_inner(path, MutationErrorInner::InvalidInternal(err)) })?; - for item in namespace.get_all_children() { + for item in descendants { if let NamespacedItem::MetaGraph(g) = item { self.invalidate(g.local_path()).await; self.cache.remove(g.local_path()).await; @@ -365,7 +365,10 @@ impl Data { InsertionError::from_inner(path, MutationErrorInner::InvalidInternal(err)) })?; blocking_io(move || { - fs::create_dir_all(&target)?; + if let Some(parent) = target.parent() { + fs::create_dir_all(parent)?; + } + fs::create_dir(&target)?; fs::remove_file(dirty_file)?; Ok::<_, MutationErrorInner>(()) }) diff --git a/raphtory-graphql/src/model/graph/namespace.rs b/raphtory-graphql/src/model/graph/namespace.rs index b0c9a2f702..e9fa8b4da0 100644 --- a/raphtory-graphql/src/model/graph/namespace.rs +++ b/raphtory-graphql/src/model/graph/namespace.rs @@ -140,6 +140,12 @@ impl Namespace { NamespaceIter { it, root } } + /// Recursively list self and all children. + pub fn self_and_all_children(&self) -> impl Iterator { + std::iter::once(NamespacedItem::Namespace(self.clone())) + .chain(self.get_all_children()) + } + pub fn current_dir(&self) -> &std::path::Path { &self.current_dir } diff --git a/raphtory-graphql/src/model/mod.rs b/raphtory-graphql/src/model/mod.rs index fa795b657c..16208b5028 100644 --- a/raphtory-graphql/src/model/mod.rs +++ b/raphtory-graphql/src/model/mod.rs @@ -282,11 +282,11 @@ impl QueryRoot { let data = ctx.data_unchecked::(); let root = Namespace::root(data.work_dir.clone()); let list = blocking_compute(move || { - std::iter::once(root.clone()) - .chain(root.get_all_children().filter_map(|child| match child { + root.self_and_all_children() + .filter_map(|child| match child { NamespacedItem::Namespace(item) => Some(item), NamespacedItem::MetaGraph(_) => None, - })) + }) .sorted() .collect() }) @@ -549,7 +549,7 @@ impl Mut { } } - data.delete_namespace(path).await?; + data.delete_namespace(path, &descendants).await?; Ok(true) } From ca91c8e4fe9dabef99fe79a8873e36dccef75a30 Mon Sep 17 00:00:00 2001 From: Louis Chan Date: Mon, 18 May 2026 16:10:42 +0100 Subject: [PATCH 5/6] Add tests asserting failure due to lack of permissions --- raphtory-graphql/src/lib.rs | 98 +++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/raphtory-graphql/src/lib.rs b/raphtory-graphql/src/lib.rs index c2828308b4..ce43f5db13 100644 --- a/raphtory-graphql/src/lib.rs +++ b/raphtory-graphql/src/lib.rs @@ -2203,6 +2203,104 @@ mod graphql_test { assert!(setup.data.get_graph_for_test("ns/g2").await.is_ok()); } + #[tokio::test] + async fn test_delete_namespace_denied_without_parent_write() { + let g = Graph::new(); + g.add_node(0, 1, NO_PROPS, None, None).unwrap(); + let g: MaterializedGraph = g.into(); + + let policy = Arc::new( + FakePolicy::default() + .with_namespace("", NamespacePermission::Read) + .with_namespace("ns", NamespacePermission::Write) + .with_graph("ns/g", GraphPermission::Write), + ); + let setup = setup_with_policy(&[("ns/g", g)], policy).await; + + let res = run_mutation_as_user( + &setup.schema, + r#"mutation { deleteNamespace(path: "ns") }"#, + ) + .await; + assert!(!res.errors.is_empty(), "expected error, got {:?}", res); + assert!( + res.errors[0] + .message + .contains("WRITE required on namespace"), + "unexpected error message: {}", + res.errors[0].message, + ); + + assert!(setup.tmp.path().join("ns").is_dir()); + assert!(setup.data.get_graph_for_test("ns/g").await.is_ok()); + } + + #[tokio::test] + async fn test_delete_namespace_denied_without_own_write() { + let g = Graph::new(); + g.add_node(0, 1, NO_PROPS, None, None).unwrap(); + let g: MaterializedGraph = g.into(); + + let policy = Arc::new( + FakePolicy::default() + .with_namespace("", NamespacePermission::Write) + .with_namespace("ns", NamespacePermission::Read) + .with_graph("ns/g", GraphPermission::Write), + ); + let setup = setup_with_policy(&[("ns/g", g)], policy).await; + + let res = run_mutation_as_user( + &setup.schema, + r#"mutation { deleteNamespace(path: "ns") }"#, + ) + .await; + assert!(!res.errors.is_empty(), "expected error, got {:?}", res); + assert!( + res.errors[0] + .message + .contains("WRITE required on namespace"), + "unexpected error message: {}", + res.errors[0].message, + ); + + assert!(setup.tmp.path().join("ns").is_dir()); + assert!(setup.data.get_graph_for_test("ns/g").await.is_ok()); + } + + #[tokio::test] + async fn test_delete_namespace_denied_without_descendant_namespace_write() { + let g = Graph::new(); + g.add_node(0, 1, NO_PROPS, None, None).unwrap(); + let g: MaterializedGraph = g.into(); + + let policy = Arc::new( + FakePolicy::default() + .with_namespace("", NamespacePermission::Write) + .with_namespace("ns", NamespacePermission::Write) + .with_namespace("ns/sub", NamespacePermission::Read) + .with_graph("ns/sub/g", GraphPermission::Write), + ); + let setup = setup_with_policy(&[("ns/sub/g", g)], policy).await; + + let res = run_mutation_as_user( + &setup.schema, + r#"mutation { deleteNamespace(path: "ns") }"#, + ) + .await; + assert!(!res.errors.is_empty(), "expected error, got {:?}", res); + assert!( + res.errors[0] + .message + .contains("WRITE required on namespace"), + "unexpected error message: {}", + res.errors[0].message, + ); + + assert!(setup.tmp.path().join("ns").is_dir()); + assert!(setup.tmp.path().join("ns/sub").is_dir()); + assert!(setup.data.get_graph_for_test("ns/sub/g").await.is_ok()); + } + #[tokio::test] async fn test_delete_namespace_invalidates_cache() { let g = Graph::new(); From 431300ccd9ac165fd027e3aeb5c98f820ff62caf Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 18 May 2026 20:34:55 +0000 Subject: [PATCH 6/6] chore: apply tidy-public auto-fixes --- raphtory-graphql/src/data.rs | 6 ++++- raphtory-graphql/src/lib.rs | 24 +++++++------------ raphtory-graphql/src/model/graph/namespace.rs | 3 +-- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/raphtory-graphql/src/data.rs b/raphtory-graphql/src/data.rs index 0b3ec0f841..9d76e336ba 100644 --- a/raphtory-graphql/src/data.rs +++ b/raphtory-graphql/src/data.rs @@ -325,7 +325,11 @@ impl Data { Ok(()) } - pub async fn delete_namespace(&self, path: &str, descendants: &Vec) -> Result<(), DeletionError> { + pub async fn delete_namespace( + &self, + path: &str, + descendants: &Vec, + ) -> Result<(), DeletionError> { if path.is_empty() { return Err(DeletionError::PathValidation( PathValidationError::EmptyPath, diff --git a/raphtory-graphql/src/lib.rs b/raphtory-graphql/src/lib.rs index ce43f5db13..4309eec66b 100644 --- a/raphtory-graphql/src/lib.rs +++ b/raphtory-graphql/src/lib.rs @@ -2217,11 +2217,9 @@ mod graphql_test { ); let setup = setup_with_policy(&[("ns/g", g)], policy).await; - let res = run_mutation_as_user( - &setup.schema, - r#"mutation { deleteNamespace(path: "ns") }"#, - ) - .await; + let res = + run_mutation_as_user(&setup.schema, r#"mutation { deleteNamespace(path: "ns") }"#) + .await; assert!(!res.errors.is_empty(), "expected error, got {:?}", res); assert!( res.errors[0] @@ -2249,11 +2247,9 @@ mod graphql_test { ); let setup = setup_with_policy(&[("ns/g", g)], policy).await; - let res = run_mutation_as_user( - &setup.schema, - r#"mutation { deleteNamespace(path: "ns") }"#, - ) - .await; + let res = + run_mutation_as_user(&setup.schema, r#"mutation { deleteNamespace(path: "ns") }"#) + .await; assert!(!res.errors.is_empty(), "expected error, got {:?}", res); assert!( res.errors[0] @@ -2282,11 +2278,9 @@ mod graphql_test { ); let setup = setup_with_policy(&[("ns/sub/g", g)], policy).await; - let res = run_mutation_as_user( - &setup.schema, - r#"mutation { deleteNamespace(path: "ns") }"#, - ) - .await; + let res = + run_mutation_as_user(&setup.schema, r#"mutation { deleteNamespace(path: "ns") }"#) + .await; assert!(!res.errors.is_empty(), "expected error, got {:?}", res); assert!( res.errors[0] diff --git a/raphtory-graphql/src/model/graph/namespace.rs b/raphtory-graphql/src/model/graph/namespace.rs index e9fa8b4da0..52a06e2f27 100644 --- a/raphtory-graphql/src/model/graph/namespace.rs +++ b/raphtory-graphql/src/model/graph/namespace.rs @@ -142,8 +142,7 @@ impl Namespace { /// Recursively list self and all children. pub fn self_and_all_children(&self) -> impl Iterator { - std::iter::once(NamespacedItem::Namespace(self.clone())) - .chain(self.get_all_children()) + std::iter::once(NamespacedItem::Namespace(self.clone())).chain(self.get_all_children()) } pub fn current_dir(&self) -> &std::path::Path {