From 6d5349c6214f215c0bb473d31cb31d53b8267092 Mon Sep 17 00:00:00 2001 From: arienandalibi Date: Fri, 11 Sep 2026 04:32:03 -0400 Subject: [PATCH 1/5] First pass. We add an ABAC cache for the generated GraphAccessFilters, making sure to cache denials as well (except for cache config errors). The key is carefully selected to only include what's relevant. Added tests for this. We also add a MutationListener to notify the authorization policy when changes to any graph are made so that we can clear its cache. We track which claims are accessed by each role/probe/filter. We track the WritableStore's generation as well, just an index which increments whenever we make a modification to it. --- raphtory-graphql/src/auth_policy.rs | 15 +++++++ raphtory-graphql/src/data.rs | 34 +++++++++++++-- raphtory-graphql/src/graph.rs | 43 ++++++++++++++++++- .../src/model/graph/mutable_graph.rs | 9 ++-- raphtory-graphql/src/model/mod.rs | 18 ++++---- 5 files changed, 103 insertions(+), 16 deletions(-) diff --git a/raphtory-graphql/src/auth_policy.rs b/raphtory-graphql/src/auth_policy.rs index 17b22677a8..2920f71412 100644 --- a/raphtory-graphql/src/auth_policy.rs +++ b/raphtory-graphql/src/auth_policy.rs @@ -177,6 +177,21 @@ pub trait AuthorizationPolicy: Send + Sync + 'static { .is_some_and(|p| p.level() >= PermissionLevel::Read)) } + /// Called after a graph on this server is successfully mutated, so a policy can discard + /// anything it derived from graph contents. + /// + /// Deliberately carries no argument. A policy may derive a caller's scope from *any* graph — an + /// ABAC probe reads whichever graph its query names, which need not be the one being read or + /// the one being written — so knowing which graph changed would not narrow what has to be + /// discarded without tracking that dependency. This says only "some graph changed"; the policy + /// decides what that invalidates. + /// + /// Only called when the mutation actually succeeded. A refused or failed write changes nothing, + /// so it must not discard anything either. + /// + /// Default no-op — only meaningful to a policy that caches. + fn on_graph_mutated(&self) {} + /// Called after a graph is successfully created to auto-grant `Write` for the creator's role. /// Returns an error if the grant cannot be persisted; the caller is responsible for rolling /// back the graph creation so the store and filesystem stay consistent. diff --git a/raphtory-graphql/src/data.rs b/raphtory-graphql/src/data.rs index 4d07aa7c75..a80473828d 100644 --- a/raphtory-graphql/src/data.rs +++ b/raphtory-graphql/src/data.rs @@ -3,7 +3,7 @@ use crate::{ auth_policy::{AuthorizationPolicy, GraphPermission, PermissionLevel}, cache::GraphCache, config::app_config::AppConfig, - graph::GraphWithVectors, + graph::{GraphWithVectors, MutationListener}, model::{ blocking_io, graph::{ @@ -324,6 +324,21 @@ impl Data { WorkDirWriteGuard { guard } } + /// The [`MutationListener`] handed to every graph this `Data` loads, so a successful write on + /// any of them reaches the authorization policy. + fn mutation_listener(&self) -> MutationListener { + MutationListener::new(self.auth_policy.clone()) + } + + /// Report a successful mutation that changed which graphs exist, rather than the contents of + /// one — a create, delete, replace or move. A policy may have derived a caller's scope from a + /// graph that has just appeared or gone, so it is told the same way an in-place write tells it. + fn notify_graph_mutated(&self) { + if let Some(policy) = &self.auth_policy { + policy.on_graph_mutated(); + } + } + pub(crate) fn set_auth_policy(&mut self, policy: Arc) { Arc::get_mut(&mut self.inner) .expect("Data is not uniquely owned when setting auth_policy") @@ -396,13 +411,15 @@ impl Data { let key = writeable_folder.local_path().to_owned(); let config = self.graph_conf.clone(); let read_only = self.read_only; + let listener = self.mutation_listener(); self.cache .insert_or_replace_with(&key, |old_graph| async { invalidate_graph(old_graph).await; blocking_compute(move || { let (is_dirty, new_graph) = writeable_folder.write_graph_data(graph, config)?; let folder = writeable_folder.finish()?; - let graph = GraphWithVectors::new(new_graph, None, folder.as_existing()?); + let graph = + GraphWithVectors::new(new_graph, None, folder.as_existing()?, listener); graph.set_dirty(is_dirty); Ok::<_, InsertionError>(if read_only { graph.into_read_only() @@ -413,6 +430,7 @@ impl Data { .await }) .await?; + self.notify_graph_mutated(); Ok(()) } @@ -433,6 +451,7 @@ impl Data { .await }) .await?; + self.notify_graph_mutated(); Ok(()) } @@ -462,6 +481,7 @@ impl Data { self.delete_graph_inner(graph_folder) .await .map_err(|err| DeletionError::from_inner(path, err))?; + self.notify_graph_mutated(); Ok(()) } @@ -496,6 +516,7 @@ impl Data { }) .await .map_err(|err| DeletionError::from_inner(path, err))?; + self.notify_graph_mutated(); Ok(()) } @@ -596,8 +617,12 @@ impl Data { self.cache .insert_or_replace_with(folder.local_path(), |old_graph| async { let current = old_graph.unwrap_or(fallback); - let updated = - GraphWithVectors::new(current.graph().clone(), Some(vectors), cloned_folder); + let updated = GraphWithVectors::new( + current.graph().clone(), + Some(vectors), + cloned_folder, + current.listener(), + ); updated.set_dirty(current.is_dirty()); Ok::<_, GQLError>(updated) }) @@ -630,6 +655,7 @@ impl Data { #[cfg(feature = "vectors")] &cache, config, + self.mutation_listener(), ) .await?; Ok(if self.read_only { diff --git a/raphtory-graphql/src/graph.rs b/raphtory-graphql/src/graph.rs index 5dceb0146f..de1e8de5c4 100644 --- a/raphtory-graphql/src/graph.rs +++ b/raphtory-graphql/src/graph.rs @@ -1,4 +1,5 @@ use crate::{ + auth_policy::AuthorizationPolicy, paths::{ExistingGraphFolder, UnlockedGraphFolder, ValidGraphPaths}, rayon::blocking_load, }; @@ -59,6 +60,33 @@ pub struct GraphWithVectorsInner { pub folder: UnlockedGraphFolder, pub is_dirty: AtomicBool, pub is_flushing: AtomicBool, + /// Told when this graph's contents change — see [`MutationListener`]. + listener: MutationListener, +} + +/// Whoever needs to know that a graph's contents changed. +/// +/// Today that is the authorization policy, which may hold scopes derived from graph data. It rides +/// on the graph handle rather than being threaded through the write resolvers because every +/// write-side handle — the graph, and the node and edge handles reached from it — already holds one +/// of these, so a mutation anywhere on that surface can report itself without a signature change. +/// +/// Empty when no policy is configured, which is the common case and costs a null check. +#[derive(Clone, Default)] +pub struct MutationListener(Option>); + +impl MutationListener { + pub fn new(policy: Option>) -> Self { + Self(policy) + } + + /// Report a mutation that **succeeded**. A refused or failed write must not call this: it + /// changed nothing, so anything derived from the graph is still current. + pub fn notify(&self) { + if let Some(policy) = &self.0 { + policy.on_graph_mutated(); + } + } } impl GraphWithVectors { @@ -66,6 +94,7 @@ impl GraphWithVectors { graph: MaterializedGraph, vectors: Option, folder: ExistingGraphFolder, + listener: MutationListener, ) -> Self { let inner = Arc::new(GraphWithVectorsInner { graph, @@ -73,6 +102,7 @@ impl GraphWithVectors { folder: folder.unlock(), is_dirty: AtomicBool::new(false), is_flushing: AtomicBool::new(false), + listener, }); Self { inner } } @@ -146,6 +176,16 @@ impl GraphWithVectors { Arc::strong_count(&self.inner) } + /// Report a mutation of this graph that succeeded. See [`MutationListener::notify`]. + pub fn notify_mutated(&self) { + self.inner.listener.notify(); + } + + /// This graph's listener, for handing to a handle rebuilt around the same graph. + pub(crate) fn listener(&self) -> MutationListener { + self.inner.listener.clone() + } + /// Flush in-memory writes to the storage engine and rewrite the on-disk /// metadata sidecar, so cache-miss namespace listings report accurate /// counts. The dirty flag is cleared up front so a mutation racing the @@ -198,6 +238,7 @@ impl GraphWithVectors { folder: &ExistingGraphFolder, #[cfg(feature = "vectors")] cache: &LazyDiskVectorCache, config: Config, + listener: MutationListener, ) -> Result { let folder_clone = folder.clone(); let graph_folder = folder.graph_folder(); @@ -234,7 +275,7 @@ impl GraphWithVectors { debug!("Graph loaded = {}", folder.local_path()); - Ok(Self::new(graph, vectors, folder.clone())) + Ok(Self::new(graph, vectors, folder.clone(), listener)) } } diff --git a/raphtory-graphql/src/model/graph/mutable_graph.rs b/raphtory-graphql/src/model/graph/mutable_graph.rs index 01595a382c..ba6df3501d 100644 --- a/raphtory-graphql/src/model/graph/mutable_graph.rs +++ b/raphtory-graphql/src/model/graph/mutable_graph.rs @@ -622,9 +622,10 @@ impl GqlMutableGraph { }) } - /// Post mutation operations. + /// Post mutation operations. Only reached once the mutation itself has succeeded. async fn post_mutation_ops(&self) { self.graph.set_dirty(true); + self.graph.notify_mutated(); } } @@ -753,9 +754,10 @@ impl GqlMutableNode { } impl GqlMutableNode { - /// Post mutation operations. + /// Post mutation operations. Only reached once the mutation itself has succeeded. async fn post_mutation_ops(&self) { self.node.graph.set_dirty(true); + self.node.graph.notify_mutated(); } } @@ -934,9 +936,10 @@ impl GqlMutableEdge { } impl GqlMutableEdge { - /// Post mutation operations. + /// Post mutation operations. Only reached once the mutation itself has succeeded. async fn post_mutation_ops(&self) { self.edge.graph.set_dirty(true); + self.edge.graph.notify_mutated(); } } diff --git a/raphtory-graphql/src/model/mod.rs b/raphtory-graphql/src/model/mod.rs index 4512dab0e6..4c6becd8cf 100644 --- a/raphtory-graphql/src/model/mod.rs +++ b/raphtory-graphql/src/model/mod.rs @@ -381,11 +381,11 @@ impl Mut { let data = ctx.data_unchecked::(); // src: require WRITE on graph // require_graph_write(ctx, &data.auth_policy, graph_path)?; - let graph = data + // The handle is kept, not just the graph it holds: reporting the write afterwards needs it. + let handle = data .get_graph_with_write_permission(ctx, &graph_path) - .await? - .graph() - .clone(); + .await?; + let graph = handle.graph().clone(); // NOTE: skipping shared metadata for now until we figure out parsing of types let properties_owned = properties.unwrap_or_default(); let properties: Vec<&str> = properties_owned.iter().map(String::as_str).collect(); @@ -423,6 +423,7 @@ impl Mut { true, arced_schema.clone(), )?; + handle.notify_mutated(); Ok(true) } @@ -456,11 +457,11 @@ impl Mut { let data = ctx.data_unchecked::(); // src: require WRITE on graph // require_graph_write(ctx, &data.auth_policy, graph_path)?; - let graph = data + // The handle is kept, not just the graph it holds: reporting the write afterwards needs it. + let handle = data .get_graph_with_write_permission(ctx, &graph_path) - .await? - .graph() - .clone(); + .await?; + let graph = handle.graph().clone(); // NOTE: skipping shared metadata for now until we figure out parsing of types let properties_owned = properties.unwrap_or_default(); let properties: Vec<&str> = properties_owned.iter().map(String::as_str).collect(); @@ -498,6 +499,7 @@ impl Mut { None, arced_schema.clone(), )?; + handle.notify_mutated(); Ok(true) } From cb4d1b38a2e1e130800b20e0f32be46364412c37 Mon Sep 17 00:00:00 2001 From: arienandalibi Date: Fri, 11 Sep 2026 05:38:51 -0400 Subject: [PATCH 2/5] Fix bug where a Claim was turned into a Var in lower_case_value --- raphtory-graphql/src/model/graph/namespace_filtering.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/raphtory-graphql/src/model/graph/namespace_filtering.rs b/raphtory-graphql/src/model/graph/namespace_filtering.rs index d5cde3f28a..ddd828660b 100644 --- a/raphtory-graphql/src/model/graph/namespace_filtering.rs +++ b/raphtory-graphql/src/model/graph/namespace_filtering.rs @@ -198,7 +198,7 @@ fn lower_case_value(value: &Value) -> Value { Value::NDTime(v) => Value::NDTime(v.clone()), Value::Decimal(v) => Value::Decimal(v.clone()), Value::Var(v) => Value::Var(v.clone()), - Value::Claim(v) => Value::Var(v.clone()), + Value::Claim(v) => Value::Claim(v.clone()), } } From 0e596824a8f33f1d209076083add0a0b3b7d70cc Mon Sep 17 00:00:00 2001 From: arienandalibi Date: Thu, 17 Sep 2026 02:32:08 -0400 Subject: [PATCH 3/5] Changing cache to return a CachedView instead of a filter. Better for performance when the source graph won't be mutated much. We now keep all roles as a set because we can't merge individual filters anymore. General cleanup. Update tests. Added static contributions (roles that don't need probes), we track their associated claims like the others. refine_permission now may return a cached graph. Fixed support for the static layer showing unlayered nodes in CachedView. --- raphtory-graphql/src/auth_policy.rs | 56 ++++++++++++- raphtory-graphql/src/data.rs | 79 +++++++++++++----- raphtory-graphql/src/lib.rs | 1 + raphtory/src/db/graph/views/cached_view.rs | 94 +++++++++++++++++++++- 4 files changed, 207 insertions(+), 23 deletions(-) diff --git a/raphtory-graphql/src/auth_policy.rs b/raphtory-graphql/src/auth_policy.rs index 2920f71412..38d95805ec 100644 --- a/raphtory-graphql/src/auth_policy.rs +++ b/raphtory-graphql/src/auth_policy.rs @@ -1,5 +1,8 @@ -use crate::model::graph::filtering::GraphAccessFilter; +use crate::{ + data::GqlGraphType, model::graph::filtering::GraphAccessFilter, paths::UnlockedGraphFolder, +}; use futures_util::future::BoxFuture; +use raphtory::db::api::view::DynamicGraph; /// Opaque error returned by [`AuthorizationPolicy::graph_permissions`] when access is entirely /// denied. The message is intended for logging only; callers must not surface it to end users. @@ -107,6 +110,42 @@ pub enum NamespacePermission { Write, } +/// A graph that has been loaded, read with the right semantics, and filtered — everything a read +/// needs, already done. +/// +/// Deliberately opaque. A policy that keeps these only ever stores one and hands it back; it has no +/// business reaching inside, and making it opaque means a policy crate needs no dependency on the +/// graph engine to hold one. Built by [`crate::data::Data::load_prepared`]. +#[derive(Clone)] +pub struct DynGraphWithFolder { + folder: UnlockedGraphFolder, + graph: DynamicGraph, +} + +impl DynGraphWithFolder { + pub(crate) fn new(folder: UnlockedGraphFolder, graph: DynamicGraph) -> Self { + Self { folder, graph } + } + + pub(crate) fn into_parts(self) -> (UnlockedGraphFolder, DynamicGraph) { + (self.folder, self.graph) + } +} + +/// What a refined read resolves to: either a filter still to be applied, or a graph the policy has +/// already prepared. +/// +/// Both arms say the same thing — what this caller may see — at different stages of being +/// materialised. A policy that caches prepared graphs returns `Cached` when it holds one and +/// `Filtered` when it does not; a policy that caches nothing only ever returns `Filtered`, which is +/// what the default refinement does. +pub enum MaybeCachedFilteredRead { + /// Apply this filter to the graph, as an unrefined read would. + Filtered(Option), + /// Nothing left to do: loaded and filtered already. + Cached(DynGraphWithFolder), +} + pub trait AuthorizationPolicy: Send + Sync + 'static { /// Resolves the effective permission level for a principal on a graph. /// @@ -154,13 +193,24 @@ pub trait AuthorizationPolicy: Send + Sync + 'static { /// /// The default returns `perm` unchanged: policies that need no refinement — and the no-policy /// case — are unaffected. Returning `Err` denies the request. + /// `graph_type` is passed because it is applied to the graph *before* the filter, so a policy + /// preparing a view has to prepare the right one — an event-semantics view and a + /// persistent-semantics view of one graph are different graphs to filter. fn refine_permission<'a>( &'a self, _ctx: &'a async_graphql::Context<'_>, _path: &'a str, + _graph_type: Option, perm: GraphPermission, - ) -> BoxFuture<'a, Result> { - Box::pin(std::future::ready(Ok(perm))) + ) -> BoxFuture<'a, Result> { + // Only a filtered read carries anything to refine; every other level reads unfiltered. + let filter = match perm { + GraphPermission::Read { filter } => filter, + _ => None, + }; + Box::pin(std::future::ready(Ok(MaybeCachedFilteredRead::Filtered( + filter, + )))) } /// Whether the principal has unfiltered read (`Write`, or `Read` with no filter) on the graph. diff --git a/raphtory-graphql/src/data.rs b/raphtory-graphql/src/data.rs index a80473828d..b2170d66a6 100644 --- a/raphtory-graphql/src/data.rs +++ b/raphtory-graphql/src/data.rs @@ -1,6 +1,9 @@ use crate::{ auth::ContextValidation, - auth_policy::{AuthorizationPolicy, GraphPermission, PermissionLevel}, + auth_policy::{ + AuthorizationPolicy, DynGraphWithFolder, GraphPermission, MaybeCachedFilteredRead, + PermissionLevel, + }, cache::GraphCache, config::app_config::AppConfig, graph::{GraphWithVectors, MutationListener}, @@ -750,7 +753,7 @@ impl PermissionError { } } -#[derive(Enum, Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Enum, Clone, Copy, Debug, PartialEq, Eq, Hash)] #[graphql(name = "GraphType")] pub enum GqlGraphType { /// Persistent. @@ -866,17 +869,22 @@ async fn refine( ctx: &Context<'_>, policy: &Option>, path: &str, + graph_type: Option, perm: GraphPermission, -) -> async_graphql::Result { +) -> async_graphql::Result { match policy { Some(policy) => policy - .refine_permission(ctx, path, perm) + .refine_permission(ctx, path, graph_type, perm) .await .map_err(|msg| { warn!(graph = path, "Access denied while refining permission"); msg.into() }), - None => Ok(perm), + // No policy: whatever the permission carried is what gets applied. + None => Ok(MaybeCachedFilteredRead::Filtered(match perm { + GraphPermission::Read { filter } => filter, + _ => None, + })), } } @@ -1004,11 +1012,16 @@ async fn apply_access_filter( impl Data { /// Loads and filters the graph using an already-verified permission. Private shared core. - async fn load_and_filter( + /// Load the graph at `path`, read it with `graph_type`'s semantics, and apply `filter`. + /// + /// The whole read path from a path and a filter to the graph a caller sees. Public because an + /// authorization policy that prepares views needs to produce exactly what an unprepared read + /// would, and the only way to guarantee that is for both to run this. + pub async fn load_filtered( &self, path: &str, - perm: GraphPermission, graph_type: Option, + filter: Option<&GraphAccessFilter>, ) -> async_graphql::Result<(UnlockedGraphFolder, DynamicGraph)> { let gwv = self.get_graph_unchecked(path).await?; let typed_graph = match graph_type { @@ -1029,17 +1042,47 @@ impl Data { None => gwv.graph().clone(), }; let raw = typed_graph.into_dynamic(); - let graph = if let GraphPermission::Read { - filter: Some(ref f), - } = perm - { - apply_access_filter(raw, f).await? - } else { - raw + let graph = match filter { + Some(f) => apply_access_filter(raw, f).await?, + None => raw, }; Ok((gwv.folder().clone(), graph)) } + /// As [`Self::load_filtered`], but with the filtered graph's node and edge membership + /// cached up front, so reading it costs a bitmap test per entity rather than re-checking + /// the filter's predicates on every visit. + /// + /// Worth it only for a graph that will be read more than once — the masks cost a pass over it + /// to build — which is why this is the policy's call to make and not the read path's. + pub async fn load_prepared( + &self, + path: &str, + graph_type: Option, + filter: Option<&GraphAccessFilter>, + ) -> async_graphql::Result { + let (folder, graph) = self.load_filtered(path, graph_type, filter).await?; + // A pass over the whole graph; it belongs on the compute pool, not the runtime. + let cached = blocking_compute(move || graph.cache_view().into_dynamic()).await; + Ok(DynGraphWithFolder::new(folder, cached)) + } + + /// The read a refinement resolved to. A prepared graph is already the answer; a filter still + /// has to be applied, which is [`Self::load_filtered`]'s job either way. + async fn load_refined( + &self, + path: &str, + refined: MaybeCachedFilteredRead, + graph_type: Option, + ) -> async_graphql::Result<(UnlockedGraphFolder, DynamicGraph)> { + match refined { + MaybeCachedFilteredRead::Cached(prepared) => Ok(prepared.into_parts()), + MaybeCachedFilteredRead::Filtered(filter) => { + self.load_filtered(path, graph_type, filter.as_ref()).await + } + } + } + /// For the `graph()` resolver: permission denial → `Ok(None)` (null to client, hides /// existence and access level). Load failure → `Err` (graph was deleted, etc.). pub async fn get_graph_with_read_permission( @@ -1049,8 +1092,8 @@ impl Data { graph_type: Option, ) -> async_graphql::Result> { match require_at_least_read(ctx, &self.auth_policy, path) { - Ok(perm) => match refine(ctx, &self.auth_policy, path, perm).await { - Ok(perm) => self.load_and_filter(path, perm, graph_type).await.map(Some), + Ok(perm) => match refine(ctx, &self.auth_policy, path, graph_type, perm).await { + Ok(refined) => self.load_refined(path, refined, graph_type).await.map(Some), // Refinement denied access — hide the graph, as with any other read denial. Err(_) => Ok(None), }, @@ -1069,8 +1112,8 @@ impl Data { graph_type: Option, ) -> async_graphql::Result<(UnlockedGraphFolder, DynamicGraph)> { let perm = require_at_least_read(ctx, &self.auth_policy, path)?; - let perm = refine(ctx, &self.auth_policy, path, perm).await?; - self.load_and_filter(path, perm, graph_type).await + let refined = refine(ctx, &self.auth_policy, path, graph_type, perm).await?; + self.load_refined(path, refined, graph_type).await } /// Checks read permission then returns the raw `GraphWithVectors` (unfiltered). diff --git a/raphtory-graphql/src/lib.rs b/raphtory-graphql/src/lib.rs index 8a19f47cc9..c8f56451ad 100644 --- a/raphtory-graphql/src/lib.rs +++ b/raphtory-graphql/src/lib.rs @@ -4,6 +4,7 @@ pub use crate::{ auth::{ Access, KeyResolver, ReadOnly, Roles, RolesMissing, StaticKeyResolver, TokenClaimValues, }, + auth_policy::{DynGraphWithFolder, MaybeCachedFilteredRead}, model::graph::{filtering::GraphAccessFilter, property::Value}, server::GraphServer, }; diff --git a/raphtory/src/db/graph/views/cached_view.rs b/raphtory/src/db/graph/views/cached_view.rs index 5847856f98..71415627a7 100644 --- a/raphtory/src/db/graph/views/cached_view.rs +++ b/raphtory/src/db/graph/views/cached_view.rs @@ -12,7 +12,7 @@ use crate::{ InternalNodeFilterOps, Static, }, }, - prelude::{GraphViewOps, LayerOps}, + prelude::{GraphViewOps, Layer, LayerOps}, storage::core_ops::InheritCoreGraphOps, }; use raphtory_api::{ @@ -41,6 +41,11 @@ use storage::EdgeEntryRef; pub struct CachedView { pub(crate) graph: G, pub(crate) global_nodes_mask: Arc, + /// Nodes a view with no layer selected still shows — the unlayered ones, which live in + /// `STATIC_GRAPH_LAYER_ID` and are visible in every view (see + /// `NodeStorageOps::tprop_iter_layers`). `unique_layers` never yields the no-layer case, so + /// this cannot fall out of the per-layer masks and is asked for separately. + pub(crate) unlayered_nodes_mask: Arc, pub(crate) layered_mask: Arc<[(RoaringTreemap, RoaringTreemap, Option)]>, } @@ -78,6 +83,14 @@ impl<'graph, G: GraphViewOps<'graph>> InheritEdgeHistoryFilter for CachedView impl<'graph, G: GraphViewOps<'graph>> CachedView { pub fn new(graph: G) -> Self { let mut layered_masks = vec![]; + // Derived by asking the graph rather than by reasoning about which nodes are unlayered, so + // it follows the same rules as every other mask here. + let unlayered_nodes_mask = Arc::new( + graph + .layers(Layer::None) + .map(|no_layers| no_layers.nodes().iter().map(|n| n.node.as_u64()).collect()) + .unwrap_or_default(), + ); let global_nodes_mask = Arc::new( graph .nodes() @@ -153,6 +166,7 @@ impl<'graph, G: GraphViewOps<'graph>> CachedView { Self { graph, global_nodes_mask, + unlayered_nodes_mask, layered_mask: layered_masks.into(), } } @@ -291,7 +305,10 @@ impl<'graph, G: GraphViewOps<'graph>> InternalNodeFilterOps for CachedView { #[inline] fn internal_filter_node(&self, node: NodeStorageRef, layer_ids: &LayerIds) -> bool { match layer_ids { - LayerIds::None => false, + // Not `false`, and not the global mask either: selecting no layers hides every + // *layered* node but still shows the unlayered ones. Answering anything else makes + // caching a view change what it contains, which is the one thing it must never do. + LayerIds::None => self.unlayered_nodes_mask.contains(node.vid().as_u64()), LayerIds::All => self.global_nodes_mask.contains(node.vid().as_u64()), LayerIds::One(id) => self .layered_mask @@ -311,3 +328,76 @@ impl<'graph, G: GraphViewOps<'graph>> InternalNodeFilterOps for CachedView { true } } + +#[cfg(test)] +mod tests { + use crate::prelude::*; + + fn fixture() -> Graph { + let graph = Graph::new(); + // Added without a layer, so it lives in the static layer and every view shows it. + graph + .add_node(0, "unlayered", NO_PROPS, None, None) + .unwrap(); + graph + .add_edge(0, "a", "b", NO_PROPS, Some("layer_a")) + .unwrap(); + graph + .add_edge(0, "c", "d", NO_PROPS, Some("layer_b")) + .unwrap(); + graph + } + + fn names<'a, G: GraphViewOps<'a>>(graph: &G) -> Vec { + let mut names: Vec = graph.nodes().name().into_iter().map(|(_, n)| n).collect(); + names.sort(); + names + } + + /// Caching a view must not change what it contains, and selecting no layers is the case that + /// separates "no nodes" from "the unlayered ones": layered nodes go, unlayered nodes stay. + #[test] + fn caching_a_no_layer_view_keeps_exactly_its_unlayered_nodes() { + let graph = fixture(); + let no_layers = graph.exclude_layers(["layer_a", "layer_b"]).unwrap(); + + let direct = names(&no_layers); + assert_eq!( + direct, + vec!["unlayered".to_string()], + "the engine's own answer" + ); + assert_eq!(names(&no_layers.cache_view()), direct); + assert_eq!( + no_layers.cache_view().edges().len(), + no_layers.edges().len() + ); + } + + /// The same, but with the layers excluded *after* caching rather than before. The masks are + /// then built over the whole graph, so an implementation that answered the no-layer case with + /// its global mask would leak every layered node here while looking correct above. + #[test] + fn excluding_every_layer_after_caching_also_keeps_only_unlayered_nodes() { + let graph = fixture(); + let cached = graph.cache_view(); + + let direct = names(&graph.exclude_layers(["layer_a", "layer_b"]).unwrap()); + let after = names(&cached.exclude_layers(["layer_a", "layer_b"]).unwrap()); + assert_eq!( + after, direct, + "caching must not change what excluding layers shows" + ); + } + + /// And the ordinary cases still agree, so the no-layer fix did not come at their expense. + #[test] + fn caching_agrees_on_all_layers_and_on_one() { + let graph = fixture(); + assert_eq!(names(&graph.cache_view()), names(&graph)); + + let one = graph.layers("layer_a").unwrap(); + assert_eq!(names(&one.cache_view()), names(&one)); + assert_eq!(one.cache_view().edges().len(), one.edges().len()); + } +} From 85e4838f9152bfa83b16ef838826442f30803a32 Mon Sep 17 00:00:00 2001 From: arienandalibi Date: Thu, 17 Sep 2026 03:26:46 -0400 Subject: [PATCH 4/5] Update documentation, add sanity tests, add caching for static grants that don't read any claims (or probes), add tests for default edge layer behaviour --- raphtory/src/db/graph/views/cached_view.rs | 87 +++++++++++++++++++++- 1 file changed, 84 insertions(+), 3 deletions(-) diff --git a/raphtory/src/db/graph/views/cached_view.rs b/raphtory/src/db/graph/views/cached_view.rs index 71415627a7..4df668237d 100644 --- a/raphtory/src/db/graph/views/cached_view.rs +++ b/raphtory/src/db/graph/views/cached_view.rs @@ -345,6 +345,9 @@ mod tests { graph .add_edge(0, "c", "d", NO_PROPS, Some("layer_b")) .unwrap(); + // No layer name, so this one lands in the default layer — an ordinary layer, unlike the + // static layer nodes get. + graph.add_edge(0, "e", "f", NO_PROPS, None).unwrap(); graph } @@ -354,12 +357,44 @@ mod tests { names } + /// Every edge as `src-dst@layer`, sorted — enough to catch an edge appearing in the wrong layer + /// as well as one appearing at all. + fn edges<'a, G: GraphViewOps<'a>>(graph: &G) -> Vec { + let mut edges: Vec = graph + .edges() + .iter() + .flat_map(|edge| { + let (src, dst) = (edge.src().name(), edge.dst().name()); + edge.layer_names() + .into_iter() + .map(|layer| format!("{src}-{dst}@{layer}")) + .collect::>() + }) + .collect(); + edges.sort(); + edges + } + + /// Caching a view must not change what it contains — nodes or edges. + fn assert_caching_changes_nothing<'a, G: GraphViewOps<'a> + Clone>(view: &G, what: &str) { + let cached = view.cache_view(); + assert_eq!(names(&cached), names(view), "nodes disagree for {what}"); + assert_eq!(edges(&cached), edges(view), "edges disagree for {what}"); + assert_eq!( + cached.count_edges(), + view.count_edges(), + "edge count disagrees for {what}" + ); + } + /// Caching a view must not change what it contains, and selecting no layers is the case that /// separates "no nodes" from "the unlayered ones": layered nodes go, unlayered nodes stay. #[test] fn caching_a_no_layer_view_keeps_exactly_its_unlayered_nodes() { let graph = fixture(); - let no_layers = graph.exclude_layers(["layer_a", "layer_b"]).unwrap(); + let no_layers = graph + .exclude_layers(["layer_a", "layer_b", "_default"]) + .unwrap(); let direct = names(&no_layers); assert_eq!( @@ -382,8 +417,9 @@ mod tests { let graph = fixture(); let cached = graph.cache_view(); - let direct = names(&graph.exclude_layers(["layer_a", "layer_b"]).unwrap()); - let after = names(&cached.exclude_layers(["layer_a", "layer_b"]).unwrap()); + let excluded = ["layer_a", "layer_b", "_default"]; + let direct = names(&graph.exclude_layers(excluded).unwrap()); + let after = names(&cached.exclude_layers(excluded).unwrap()); assert_eq!( after, direct, "caching must not change what excluding layers shows" @@ -400,4 +436,49 @@ mod tests { assert_eq!(names(&one.cache_view()), names(&one)); assert_eq!(one.cache_view().edges().len(), one.edges().len()); } + + /// Edges across every shape of layer selection. Unlike nodes, edges have no "visible in every + /// view" layer — an edge added without a layer name goes to the default layer, which is an + /// ordinary one — so selecting no layers really does mean no edges. + #[test] + fn caching_agrees_on_edges_for_every_layer_selection() { + let graph = fixture(); + + assert_caching_changes_nothing(&graph, "the whole graph"); + assert_caching_changes_nothing(&graph.layers("layer_a").unwrap(), "one named layer"); + assert_caching_changes_nothing(&graph.layers("_default").unwrap(), "the default layer"); + assert_caching_changes_nothing( + &graph.layers(vec!["layer_a", "layer_b"]).unwrap(), + "several named layers", + ); + assert_caching_changes_nothing( + &graph.exclude_layers("layer_a").unwrap(), + "one layer excluded", + ); + + let none = graph + .exclude_layers(vec!["layer_a", "layer_b", "_default"]) + .unwrap(); + assert!(edges(&none).is_empty(), "no layer selected means no edges"); + assert_caching_changes_nothing(&none, "every layer excluded"); + } + + /// The default layer is an ordinary layer, so excluding it hides its edges — the asymmetry with + /// nodes, whose static layer survives every exclusion. + #[test] + fn the_default_layer_is_not_exempt_the_way_the_static_node_layer_is() { + let graph = fixture(); + let without_default = graph.exclude_layers("_default").unwrap(); + + assert!( + !edges(&without_default) + .iter() + .any(|e| e.contains("@_default")), + "excluding the default layer must hide its edges, got {:?}", + edges(&without_default) + ); + // The unlayered node is still there, which is the rule edges do not share. + assert!(names(&without_default).contains(&"unlayered".to_string())); + assert_caching_changes_nothing(&without_default, "the default layer excluded"); + } } From a10af462a5d6c1b613ff983d9564d166dc334ef5 Mon Sep 17 00:00:00 2001 From: arienandalibi Date: Mon, 21 Sep 2026 22:11:57 -0400 Subject: [PATCH 5/5] Got rid of CachedView's unlayered_nodes_mask and added it in layered_mask[0]. --- raphtory/src/db/graph/views/cached_view.rs | 36 ++++++++++------------ 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/raphtory/src/db/graph/views/cached_view.rs b/raphtory/src/db/graph/views/cached_view.rs index 4df668237d..5d38aa0aa3 100644 --- a/raphtory/src/db/graph/views/cached_view.rs +++ b/raphtory/src/db/graph/views/cached_view.rs @@ -17,7 +17,7 @@ use crate::{ }; use raphtory_api::{ core::{ - entities::{LayerId, ELID}, + entities::{properties::meta::STATIC_GRAPH_LAYER_ID, LayerId, ELID}, storage::timeindex::{AsTime, EventTime}, }, inherit::Base, @@ -41,11 +41,6 @@ use storage::EdgeEntryRef; pub struct CachedView { pub(crate) graph: G, pub(crate) global_nodes_mask: Arc, - /// Nodes a view with no layer selected still shows — the unlayered ones, which live in - /// `STATIC_GRAPH_LAYER_ID` and are visible in every view (see - /// `NodeStorageOps::tprop_iter_layers`). `unique_layers` never yields the no-layer case, so - /// this cannot fall out of the per-layer masks and is asked for separately. - pub(crate) unlayered_nodes_mask: Arc, pub(crate) layered_mask: Arc<[(RoaringTreemap, RoaringTreemap, Option)]>, } @@ -82,15 +77,16 @@ impl<'graph, G: GraphViewOps<'graph>> InheritEdgeHistoryFilter for CachedView impl<'graph, G: GraphViewOps<'graph>> CachedView { pub fn new(graph: G) -> Self { - let mut layered_masks = vec![]; - // Derived by asking the graph rather than by reasoning about which nodes are unlayered, so - // it follows the same rules as every other mask here. - let unlayered_nodes_mask = Arc::new( - graph - .layers(Layer::None) - .map(|no_layers| no_layers.nodes().iter().map(|n| n.node.as_u64()).collect()) - .unwrap_or_default(), - ); + // Seeding slot 0 here is necessary because `STATIC_GRAPH_LAYER` is not returned by `unique_layers` + let static_layer_nodes: RoaringTreemap = graph + .layers(Layer::None) + .map(|no_layers| no_layers.nodes().iter().map(|n| n.node.as_u64()).collect()) + .unwrap_or_default(); + let mut layered_masks = vec![( + static_layer_nodes, + RoaringTreemap::new(), + Some(RoaringTreemap::new()), + )]; let global_nodes_mask = Arc::new( graph .nodes() @@ -166,7 +162,6 @@ impl<'graph, G: GraphViewOps<'graph>> CachedView { Self { graph, global_nodes_mask, - unlayered_nodes_mask, layered_mask: layered_masks.into(), } } @@ -305,10 +300,11 @@ impl<'graph, G: GraphViewOps<'graph>> InternalNodeFilterOps for CachedView { #[inline] fn internal_filter_node(&self, node: NodeStorageRef, layer_ids: &LayerIds) -> bool { match layer_ids { - // Not `false`, and not the global mask either: selecting no layers hides every - // *layered* node but still shows the unlayered ones. Answering anything else makes - // caching a view change what it contains, which is the one thing it must never do. - LayerIds::None => self.unlayered_nodes_mask.contains(node.vid().as_u64()), + // The unlayered nodes should still be returned when no layer is selected + LayerIds::None => self + .layered_mask + .get(STATIC_GRAPH_LAYER_ID.0) + .is_some_and(|(nodes, _, _)| nodes.contains(node.vid().as_u64())), LayerIds::All => self.global_nodes_mask.contains(node.vid().as_u64()), LayerIds::One(id) => self .layered_mask