From 5030afa6ab740e9bbe5e055a707b243da56661dc Mon Sep 17 00:00:00 2001 From: Fabian Murariu Date: Tue, 23 Jul 2024 15:58:11 +0100 Subject: [PATCH] fixes to Immutability, removing todos --- raphtory-cypher/src/lib.rs | 1 - raphtory-graphql/src/lib.rs | 1 + raphtory/src/core/entities/graph/mod.rs | 6 ++--- raphtory/src/db/api/mutation/addition_ops.rs | 2 +- raphtory/src/db/api/mutation/deletion_ops.rs | 2 +- raphtory/src/db/api/mutation/import_ops.rs | 4 +-- .../internal/internal_addition_ops.rs | 12 ++++----- raphtory/src/db/api/mutation/mod.rs | 4 +-- .../db/api/mutation/property_addition_ops.rs | 6 ++--- .../db/api/storage/storage_ops/additions.rs | 27 ++++++++++--------- raphtory/src/db/api/view/graph.rs | 2 +- raphtory/src/db/api/view/serialise.rs | 2 +- raphtory/src/db/graph/edge.rs | 2 +- raphtory/src/search/mod.rs | 6 ++--- 14 files changed, 40 insertions(+), 37 deletions(-) diff --git a/raphtory-cypher/src/lib.rs b/raphtory-cypher/src/lib.rs index e4f04bfcff..6661c86a67 100644 --- a/raphtory-cypher/src/lib.rs +++ b/raphtory-cypher/src/lib.rs @@ -31,7 +31,6 @@ mod cypher { use super::{ executor::{table_provider::edge::EdgeListTableProvider, ExecError}, - parser::ast::*, *, }; use raphtory::disk_graph::DiskGraphStorage; diff --git a/raphtory-graphql/src/lib.rs b/raphtory-graphql/src/lib.rs index 73498b28b3..40baeb2140 100644 --- a/raphtory-graphql/src/lib.rs +++ b/raphtory-graphql/src/lib.rs @@ -269,6 +269,7 @@ mod graphql_test { let req = Request::new(prop_has_key_filter); let res = schema.execute(req).await; + // let data = res.data.into_json().unwrap(); let expected = json!({ "graph": { "properties": { diff --git a/raphtory/src/core/entities/graph/mod.rs b/raphtory/src/core/entities/graph/mod.rs index 8cfce7595d..12034c2da9 100644 --- a/raphtory/src/core/entities/graph/mod.rs +++ b/raphtory/src/core/entities/graph/mod.rs @@ -14,9 +14,9 @@ mod test { #[test] fn test_neighbours_multiple_layers() { let g = GraphStorage::default(); - let l_btc = g.resolve_layer(Some("btc")); - let l_eth = g.resolve_layer(Some("eth")); - let l_tether = g.resolve_layer(Some("tether")); + let l_btc = g.resolve_layer(Some("btc")).unwrap(); + let l_eth = g.resolve_layer(Some("eth")).unwrap(); + let l_tether = g.resolve_layer(Some("tether")).unwrap(); let v1 = g.resolve_node(1).unwrap(); let v2 = g.resolve_node(2).unwrap(); let tx_sent_id = g diff --git a/raphtory/src/db/api/mutation/addition_ops.rs b/raphtory/src/db/api/mutation/addition_ops.rs index 364c7f505a..c5009d1e5b 100644 --- a/raphtory/src/db/api/mutation/addition_ops.rs +++ b/raphtory/src/db/api/mutation/addition_ops.rs @@ -130,7 +130,7 @@ impl AdditionOps for G { let ti = time_from_input(self, t)?; let src_id = self.resolve_node(src)?; let dst_id = self.resolve_node(dst)?; - let layer_id = self.resolve_layer(layer); + let layer_id = self.resolve_layer(layer)?; let properties: Vec<(usize, Prop)> = props.collect_properties( |name, dtype| self.resolve_edge_property(name, dtype, false), diff --git a/raphtory/src/db/api/mutation/deletion_ops.rs b/raphtory/src/db/api/mutation/deletion_ops.rs index 730606b2c5..007cdf4925 100644 --- a/raphtory/src/db/api/mutation/deletion_ops.rs +++ b/raphtory/src/db/api/mutation/deletion_ops.rs @@ -21,7 +21,7 @@ pub trait DeletionOps: InternalDeletionOps + InternalAdditionOps + Sized { let ti = time_from_input(self, t)?; let src_id = self.resolve_node(src)?; let dst_id = self.resolve_node(dst)?; - let layer = self.resolve_layer(layer); + let layer = self.resolve_layer(layer)?; self.internal_delete_edge(ti, src_id, dst_id, layer) } diff --git a/raphtory/src/db/api/mutation/import_ops.rs b/raphtory/src/db/api/mutation/import_ops.rs index 22e32488ea..a1a523cde8 100644 --- a/raphtory/src/db/api/mutation/import_ops.rs +++ b/raphtory/src/db/api/mutation/import_ops.rs @@ -188,7 +188,7 @@ impl< // make sure we preserve all layers even if they are empty // skip default layer for layer in edge.graph.unique_layers().skip(1) { - self.resolve_layer(Some(&layer)); + self.resolve_layer(Some(&layer))?; } if !force && self.has_edge(edge.src().name(), edge.dst().name()) { return Err(EdgeExistsError(edge.src().id(), edge.dst().id())); @@ -219,7 +219,7 @@ impl< let ti = time_from_input(self, t)?; let src_id = self.resolve_node(edge.src().id())?; let dst_id = self.resolve_node(edge.dst().id())?; - let layer = self.resolve_layer(layer_name); + let layer = self.resolve_layer(layer_name)?; self.internal_delete_edge(ti, src_id, dst_id, layer)?; } } diff --git a/raphtory/src/db/api/mutation/internal/internal_addition_ops.rs b/raphtory/src/db/api/mutation/internal/internal_addition_ops.rs index 45f82aa075..22be704cc6 100644 --- a/raphtory/src/db/api/mutation/internal/internal_addition_ops.rs +++ b/raphtory/src/db/api/mutation/internal/internal_addition_ops.rs @@ -12,10 +12,10 @@ use enum_dispatch::enum_dispatch; #[enum_dispatch] pub trait InternalAdditionOps { /// get the sequence id for the next event - fn next_event_id(&self) -> usize; + fn next_event_id(&self) -> Result; /// map layer name to id and allocate a new layer if needed - fn resolve_layer(&self, layer: Option<&str>) -> usize; + fn resolve_layer(&self, layer: Option<&str>) -> Result; fn resolve_node_type(&self, v_id: VID, node_type: Option<&str>) -> Result; @@ -23,7 +23,7 @@ pub trait InternalAdditionOps { fn resolve_node(&self, id: V) -> Result; /// map property key to internal id, allocating new property if needed - fn resolve_graph_property(&self, prop: &str, is_static: bool) -> usize; + fn resolve_graph_property(&self, prop: &str, is_static: bool) -> Result; /// map property key to internal id, allocating new property if needed and checking property type. /// returns `None` if the type does not match @@ -83,12 +83,12 @@ pub trait DelegateAdditionOps { impl InternalAdditionOps for G { #[inline(always)] - fn next_event_id(&self) -> usize { + fn next_event_id(&self) -> Result { self.graph().next_event_id() } #[inline] - fn resolve_layer(&self, layer: Option<&str>) -> usize { + fn resolve_layer(&self, layer: Option<&str>) -> Result { self.graph().resolve_layer(layer) } @@ -103,7 +103,7 @@ impl InternalAdditionOps for G { } #[inline] - fn resolve_graph_property(&self, prop: &str, is_static: bool) -> usize { + fn resolve_graph_property(&self, prop: &str, is_static: bool) -> Result { self.graph().resolve_graph_property(prop, is_static) } diff --git a/raphtory/src/db/api/mutation/mod.rs b/raphtory/src/db/api/mutation/mod.rs index e5494453e7..ecdf1c57c8 100644 --- a/raphtory/src/db/api/mutation/mod.rs +++ b/raphtory/src/db/api/mutation/mod.rs @@ -32,10 +32,10 @@ pub enum InputTime { pub fn time_from_input( g: &G, t: T, -) -> Result { +) -> Result { let t = t.try_into_input_time()?; Ok(match t { - InputTime::Simple(t) => TimeIndexEntry::new(t, g.next_event_id()), + InputTime::Simple(t) => TimeIndexEntry::new(t, g.next_event_id()?), InputTime::Indexed(t, s) => TimeIndexEntry::new(t, s), }) } diff --git a/raphtory/src/db/api/mutation/property_addition_ops.rs b/raphtory/src/db/api/mutation/property_addition_ops.rs index cd31b040dc..7e511e3599 100644 --- a/raphtory/src/db/api/mutation/property_addition_ops.rs +++ b/raphtory/src/db/api/mutation/property_addition_ops.rs @@ -30,7 +30,7 @@ impl PropertyAdditionOps f ) -> Result<(), GraphError> { let ti = time_from_input(self, t)?; let properties: Vec<_> = props.collect_properties( - |name, _| Ok(self.resolve_graph_property(name, false)), + |name, _| self.resolve_graph_property(name, false), |prop| self.process_prop_value(prop), )?; self.internal_add_properties(ti, properties) @@ -38,7 +38,7 @@ impl PropertyAdditionOps f fn add_constant_properties(&self, props: PI) -> Result<(), GraphError> { let properties: Vec<_> = props.collect_properties( - |name, _| Ok(self.resolve_graph_property(name, true)), + |name, _| self.resolve_graph_property(name, true), |prop| self.process_prop_value(prop), )?; self.internal_add_static_properties(properties) @@ -49,7 +49,7 @@ impl PropertyAdditionOps f props: PI, ) -> Result<(), GraphError> { let properties: Vec<_> = props.collect_properties( - |name, _| Ok(self.resolve_graph_property(name, true)), + |name, _| self.resolve_graph_property(name, true), |prop| self.process_prop_value(prop), )?; self.internal_update_static_properties(properties) diff --git a/raphtory/src/db/api/storage/storage_ops/additions.rs b/raphtory/src/db/api/storage/storage_ops/additions.rs index 643bc70aa9..bee5891801 100644 --- a/raphtory/src/db/api/storage/storage_ops/additions.rs +++ b/raphtory/src/db/api/storage/storage_ops/additions.rs @@ -13,23 +13,22 @@ use crate::{ use super::GraphStorage; -// FIXME: change the return type to Result<(), GraphError> where GraphError is AttemptToMutateImmutableGraph impl InternalAdditionOps for GraphStorage { - fn next_event_id(&self) -> usize { + fn next_event_id(&self) -> Result { match self { GraphStorage::Unlocked(storage) => { - storage.event_counter.fetch_add(1, Ordering::Relaxed) + Ok(storage.event_counter.fetch_add(1, Ordering::Relaxed)) } - _ => todo!(), + _ => Err(GraphError::AttemptToMutateImmutableGraph), } } - fn resolve_layer(&self, layer: Option<&str>) -> usize { + fn resolve_layer(&self, layer: Option<&str>) -> Result { match self { - GraphStorage::Unlocked(_) => layer + GraphStorage::Unlocked(_) => Ok(layer .map(|name| self.edge_meta().get_or_create_layer_id(name)) - .unwrap_or(0), - _ => todo!(), + .unwrap_or(0)), + _ => Err(GraphError::AttemptToMutateImmutableGraph), } } @@ -47,10 +46,10 @@ impl InternalAdditionOps for GraphStorage { } } - fn resolve_graph_property(&self, prop: &str, is_static: bool) -> usize { + fn resolve_graph_property(&self, prop: &str, is_static: bool) -> Result { match self { - GraphStorage::Unlocked(_) => self.graph_meta().resolve_property(prop, is_static), - _ => todo!(), + GraphStorage::Unlocked(_) => Ok(self.graph_meta().resolve_property(prop, is_static)), + _ => Err(GraphError::AttemptToMutateImmutableGraph), } } @@ -84,7 +83,11 @@ impl InternalAdditionOps for GraphStorage { Prop::Str(value) => Prop::Str(storage.resolve_str(value)), _ => prop, }, - _ => todo!(), + GraphStorage::Mem(storage) => match prop { + Prop::Str(value) => Prop::Str(storage.graph.resolve_str(value)), + _ => prop, + }, + _ => prop, } } diff --git a/raphtory/src/db/api/view/graph.rs b/raphtory/src/db/api/view/graph.rs index 87d0062377..577c143869 100644 --- a/raphtory/src/db/api/view/graph.rs +++ b/raphtory/src/db/api/view/graph.rs @@ -145,7 +145,7 @@ impl<'graph, G: BoxableGraphView + Sized + Clone + 'graph> GraphViewOps<'graph> // make sure we preserve all layers even if they are empty // skip default layer for layer in self.unique_layers().skip(1) { - g.resolve_layer(Some(&layer)); + g.resolve_layer(Some(&layer))?; } // Add edges first so we definitely have all associated nodes (important in case of persistent edges) for e in self.edges() { diff --git a/raphtory/src/db/api/view/serialise.rs b/raphtory/src/db/api/view/serialise.rs index 3793ff2b1c..09e4925289 100644 --- a/raphtory/src/db/api/view/serialise.rs +++ b/raphtory/src/db/api/view/serialise.rs @@ -421,7 +421,7 @@ impl< // alight the edge layers for (layer_id, layer) in g.layers.iter().enumerate() { - let l_id = graph.resolve_layer(Some(layer)); + let l_id = graph.resolve_layer(Some(layer))?; assert_eq!(l_id, layer_id); } diff --git a/raphtory/src/db/graph/edge.rs b/raphtory/src/db/graph/edge.rs index 06e5cc8a07..5fc21d49cd 100644 --- a/raphtory/src/db/graph/edge.rs +++ b/raphtory/src/db/graph/edge.rs @@ -183,7 +183,7 @@ impl }), None => { if create { - Ok(self.graph.resolve_layer(layer)) + self.graph.resolve_layer(layer) } else { self.graph .get_layer_id(name) diff --git a/raphtory/src/search/mod.rs b/raphtory/src/search/mod.rs index c178221233..3371c52e1e 100644 --- a/raphtory/src/search/mod.rs +++ b/raphtory/src/search/mod.rs @@ -761,11 +761,11 @@ impl<'graph, G: GraphViewOps<'graph>> IndexedGraph { impl InternalAdditionOps for IndexedGraph { #[inline] - fn next_event_id(&self) -> usize { + fn next_event_id(&self) -> Result { self.graph.next_event_id() } #[inline] - fn resolve_layer(&self, layer: Option<&str>) -> usize { + fn resolve_layer(&self, layer: Option<&str>) -> Result { self.graph.resolve_layer(layer) } @@ -780,7 +780,7 @@ impl InternalAdditionOps for Indexe } #[inline] - fn resolve_graph_property(&self, prop: &str, is_static: bool) -> usize { + fn resolve_graph_property(&self, prop: &str, is_static: bool) -> Result { self.graph.resolve_graph_property(prop, is_static) }