fixes to Immutability, removing todos

Pometry · Jul 23, 2024 · 5030afa · 5030afa
1 parent b19ff59
commit 5030afa
Show file tree

Hide file tree

Showing 14 changed files with 40 additions and 37 deletions.
diff --git 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
@@ -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
@@ -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
@@ -130,7 +130,7 @@ impl<G: InternalAdditionOps + StaticGraphViewOps> 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
@@ -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
@@ -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
@@ -12,18 +12,18 @@ 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<usize, GraphError>;
 
     /// 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<usize, GraphError>;
 
     fn resolve_node_type(&self, v_id: VID, node_type: Option<&str>) -> Result<usize, GraphError>;
 
     /// map external node id to internal id, allocating a new empty node if needed
     fn resolve_node<V: AsNodeRef>(&self, id: V) -> Result<VID, GraphError>;
 
     /// 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<usize, GraphError>;
 
     /// 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<G: DelegateAdditionOps> InternalAdditionOps for G {
     #[inline(always)]
-    fn next_event_id(&self) -> usize {
+    fn next_event_id(&self) -> Result<usize, GraphError> {
         self.graph().next_event_id()
     }
 
     #[inline]
-    fn resolve_layer(&self, layer: Option<&str>) -> usize {
+    fn resolve_layer(&self, layer: Option<&str>) -> Result<usize, GraphError> {
         self.graph().resolve_layer(layer)
     }
 
@@ -103,7 +103,7 @@ impl<G: DelegateAdditionOps> 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<usize, GraphError> {
         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
@@ -32,10 +32,10 @@ pub enum InputTime {
 pub fn time_from_input<G: InternalAdditionOps, T: TryIntoInputTime>(
     g: &G,
     t: T,
-) -> Result<TimeIndexEntry, ParseTimeError> {
+) -> Result<TimeIndexEntry, GraphError> {
     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
@@ -30,15 +30,15 @@ impl<G: InternalPropertyAdditionOps + InternalAdditionOps> 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)
     }
 
     fn add_constant_properties<PI: CollectProperties>(&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<G: InternalPropertyAdditionOps + InternalAdditionOps> 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
@@ -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<usize, GraphError> {
         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<usize, GraphError> {
         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<usize, GraphError> {
         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
@@ -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
@@ -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
@@ -183,7 +183,7 @@ impl<G: StaticGraphViewOps + InternalPropertyAdditionOps + InternalAdditionOps>
                     }),
                 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
@@ -761,11 +761,11 @@ impl<'graph, G: GraphViewOps<'graph>> IndexedGraph<G> {
 
 impl<G: StaticGraphViewOps + InternalAdditionOps> InternalAdditionOps for IndexedGraph<G> {
     #[inline]
-    fn next_event_id(&self) -> usize {
+    fn next_event_id(&self) -> Result<usize, GraphError> {
         self.graph.next_event_id()
     }
     #[inline]
-    fn resolve_layer(&self, layer: Option<&str>) -> usize {
+    fn resolve_layer(&self, layer: Option<&str>) -> Result<usize, GraphError> {
         self.graph.resolve_layer(layer)
     }
 
@@ -780,7 +780,7 @@ impl<G: StaticGraphViewOps + InternalAdditionOps> 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<usize, GraphError> {
         self.graph.resolve_graph_property(prop, is_static)
     }