diff --git a/raphtory-graphql/src/auth_policy.rs b/raphtory-graphql/src/auth_policy.rs index 17b22677a8..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. @@ -177,6 +227,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 130ca9c8be..b2253fd3c6 100644 --- a/raphtory-graphql/src/data.rs +++ b/raphtory-graphql/src/data.rs @@ -1,9 +1,12 @@ use crate::{ auth::ContextValidation, - auth_policy::{AuthorizationPolicy, GraphPermission, PermissionLevel}, + auth_policy::{ + AuthorizationPolicy, DynGraphWithFolder, GraphPermission, MaybeCachedFilteredRead, + PermissionLevel, + }, cache::GraphCache, config::app_config::AppConfig, - graph::GraphWithVectors, + graph::{GraphWithVectors, MutationListener}, model::{ blocking_io, graph::{ @@ -323,6 +326,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") @@ -395,6 +413,7 @@ impl Data { let key = writeable_folder.local_path().to_owned(); let args = self.graph_args.clone(); let read_only = self.read_only; + let listener = self.mutation_listener(); self.cache .insert_or_replace_with(&key, |old_graph| async { @@ -402,7 +421,8 @@ impl Data { blocking_compute(move || { let (is_dirty, new_graph) = writeable_folder.write_graph_data(graph, args)?; 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 +433,7 @@ impl Data { .await }) .await?; + self.notify_graph_mutated(); Ok(()) } @@ -435,6 +456,7 @@ impl Data { .await }) .await?; + self.notify_graph_mutated(); Ok(()) } @@ -464,6 +486,7 @@ impl Data { self.delete_graph_inner(graph_folder) .await .map_err(|err| DeletionError::from_inner(path, err))?; + self.notify_graph_mutated(); Ok(()) } @@ -498,6 +521,7 @@ impl Data { }) .await .map_err(|err| DeletionError::from_inner(path, err))?; + self.notify_graph_mutated(); Ok(()) } @@ -598,8 +622,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) }) @@ -632,6 +660,7 @@ impl Data { #[cfg(feature = "vectors")] &cache, args, + self.mutation_listener(), ) .await?; Ok(if self.read_only { @@ -726,7 +755,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. @@ -842,17 +871,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, + })), } } @@ -980,11 +1014,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 { @@ -1005,17 +1044,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( @@ -1025,8 +1094,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), }, @@ -1045,8 +1114,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/graph.rs b/raphtory-graphql/src/graph.rs index a807408c95..c731fa5101 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, args: Args, + listener: MutationListener, ) -> Result { let folder_clone = folder.clone(); let graph_folder = folder.graph_folder(); @@ -236,7 +277,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/lib.rs b/raphtory-graphql/src/lib.rs index 2627ea1b6e..2d52f6498c 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-graphql/src/model/graph/mutable_graph.rs b/raphtory-graphql/src/model/graph/mutable_graph.rs index d24be05426..2d1f51cb35 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/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()), } } diff --git a/raphtory-graphql/src/model/mod.rs b/raphtory-graphql/src/model/mod.rs index 57a888e7a4..f646be032e 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) } diff --git a/raphtory/src/db/graph/views/cached_view.rs b/raphtory/src/db/graph/views/cached_view.rs index 5847856f98..5d38aa0aa3 100644 --- a/raphtory/src/db/graph/views/cached_view.rs +++ b/raphtory/src/db/graph/views/cached_view.rs @@ -12,12 +12,12 @@ use crate::{ InternalNodeFilterOps, Static, }, }, - prelude::{GraphViewOps, LayerOps}, + prelude::{GraphViewOps, Layer, LayerOps}, storage::core_ops::InheritCoreGraphOps, }; use raphtory_api::{ core::{ - entities::{LayerId, ELID}, + entities::{properties::meta::STATIC_GRAPH_LAYER_ID, LayerId, ELID}, storage::timeindex::{AsTime, EventTime}, }, inherit::Base, @@ -77,7 +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![]; + // 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() @@ -291,7 +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 { - LayerIds::None => false, + // 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 @@ -311,3 +324,157 @@ 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(); + // 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 + } + + 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 + } + + /// 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", "_default"]) + .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 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" + ); + } + + /// 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()); + } + + /// 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"); + } +}