From 03cb90a3a81aa8ce3e9e23f522e9193647430bac Mon Sep 17 00:00:00 2001 From: Allan Clements Date: Thu, 14 Nov 2024 16:27:53 -0600 Subject: [PATCH] Populate edge properties in GraphSONV2/V3 & GraphBinary. Also implemented deserialization of Property in GraphBinary --- gremlin-client/src/io/graph_binary_v1.rs | 46 +++++++++-- gremlin-client/src/io/serializer_v2.rs | 67 ++++++++++------ gremlin-client/src/io/serializer_v3.rs | 59 +++++++++----- gremlin-client/src/structure/macros.rs | 2 +- gremlin-client/tests/custom_vertex_ids.rs | 95 +++++++++++++++++------ 5 files changed, 196 insertions(+), 73 deletions(-) diff --git a/gremlin-client/src/io/graph_binary_v1.rs b/gremlin-client/src/io/graph_binary_v1.rs index 30d72167..db7cdd9c 100644 --- a/gremlin-client/src/io/graph_binary_v1.rs +++ b/gremlin-client/src/io/graph_binary_v1.rs @@ -9,7 +9,7 @@ use crate::{ message::{ReponseStatus, Response, ResponseResult}, process::traversal::{Instruction, Order, Scope}, structure::{Column, Direction, Merge, Pop, TextP, Traverser, T}, - Cardinality, Edge, GKey, GValue, GremlinError, GremlinResult, Metric, Path, ToGValue, + Cardinality, Edge, GKey, GValue, GremlinError, GremlinResult, Metric, Path, Property, ToGValue, TraversalMetrics, Vertex, VertexProperty, GID, }; @@ -34,7 +34,7 @@ const SET: u8 = 0x0B; const UUID: u8 = 0x0C; const EDGE: u8 = 0x0D; const PATH: u8 = 0x0E; -// const PROPERTY: u8 = 0x0F; +const PROPERTY: u8 = 0x0F; // const TINKERGRAPH: u8 = 0x10; const VERTEX: u8 = 0x11; const VERTEX_PROPERTY: u8 = 0x12; @@ -671,6 +671,10 @@ impl GraphBinaryV1Deser for GValue { Some(value) => GValue::Path(value), None => GValue::Null, }), + PROPERTY => Ok(match Property::from_be_bytes_nullable(bytes)? { + Some(value) => GValue::Property(value), + None => GValue::Null, + }), VERTEX => Ok(match Vertex::from_be_bytes_nullable(bytes)? { Some(value) => GValue::Vertex(value), None => GValue::Null, @@ -963,8 +967,23 @@ impl GraphBinaryV1Deser for Edge { //{parent} is a fully qualified typed value composed of {type_code}{type_info}{value_flag}{value} which contains the parent Vertex. Note that as TinkerPop currently send "references" only, this value will always be null. consume_expected_null_reference_bytes(bytes, "Parent")?; - //{properties} is a fully qualified typed value composed of {type_code}{type_info}{value_flag}{value} which contains the properties for the edge. Note that as TinkerPop currently send "references" only this value will always be null. - consume_expected_null_reference_bytes(bytes, "Properties")?; + //{properties} is a fully qualified typed value composed of {type_code}{type_info}{value_flag}{value} which contains the properties for the edge. + let properties = match GValue::from_be_bytes(bytes)? { + GValue::Null => HashMap::new(), + GValue::List(raw_properties) => raw_properties + .into_iter() + .map(|property| { + property + .take::() + .map(|converted| (converted.label().clone(), converted)) + }) + .collect::>()?, + _ => { + return Err(GremlinError::Cast(format!( + "Edge properties should either be Null or List" + ))) + } + }; Ok(Edge::new( id.try_into()?, label, @@ -972,11 +991,28 @@ impl GraphBinaryV1Deser for Edge { in_v_label, out_v_id.try_into()?, out_v_label, - HashMap::new(), + properties, )) } } +impl GraphBinaryV1Deser for Property { + fn from_be_bytes<'a, S: Iterator>(bytes: &mut S) -> GremlinResult { + //Format: {key}{value}{parent} + + //{key} is a String value + let key: String = GraphBinaryV1Deser::from_be_bytes(bytes)?; + + //{value} is a fully qualified typed value composed of {type_code}{type_info}{value_flag}{value} + let value = GValue::from_be_bytes(bytes)?; + + //{parent} is a fully qualified typed value composed of {type_code}{type_info}{value_flag}{value} which is either an Edge or VertexProperty. + //Note that as TinkerPop currently sends "references" only this value will always be null. + consume_expected_null_reference_bytes(bytes, "Parent")?; + Ok(Property::new(key, value)) + } +} + impl GraphBinaryV1Deser for Path { fn from_be_bytes<'a, S: Iterator>(bytes: &mut S) -> GremlinResult { let labels = GValue::from_be_bytes(bytes)?; diff --git a/gremlin-client/src/io/serializer_v2.rs b/gremlin-client/src/io/serializer_v2.rs index a67b2fae..1042f68c 100644 --- a/gremlin-client/src/io/serializer_v2.rs +++ b/gremlin-client/src/io/serializer_v2.rs @@ -177,6 +177,26 @@ where let out_v_id = deserialize_id(reader, &val["outV"])?; let out_v_label = get_value!(&val["outVLabel"], Value::String)?.clone(); + let json_properties = &val["properties"]; + let properties = if json_properties.is_object() { + get_value!(&json_properties, Value::Object)? + .into_iter() + .map(|(key, value)| { + deserializer_v2(value) + .map(|deserialized| { + //if the given value is a property, unfurl it + match deserialized { + GValue::Property(property) => property.value().clone(), + other => other, + } + }) + .map(|property| (key.clone(), Property::new(key, property))) + }) + .collect::>()? + } else { + HashMap::new() + }; + Ok(Edge::new( id, label, @@ -184,7 +204,7 @@ where in_v_label, out_v_id, out_v_label, - HashMap::new(), + properties, ) .into()) } @@ -431,7 +451,7 @@ mod tests { use super::deserializer_v2; use serde_json::json; - use crate::{edge, vertex}; + use crate::{edge, vertex, Edge}; use crate::structure::{GValue, Map, Path, Property, Token, Vertex, VertexProperty, GID}; use chrono::offset::TimeZone; @@ -593,29 +613,32 @@ mod tests { #[test] fn test_edge() { - let value = json!({"@type":"g:Edge","@value":{"id":{"@type":"g:Int32","@value":13},"label":"develops","inVLabel":"software","outVLabel":"person","inV":{"@type":"g:Int32","@value":10},"outV":{"@type":"g:Int32","@value":1},"properties":{"since":{"@type":"g:Property","@value":{"key":"since","value":{"@type":"g:Int32","@value":2009}}}}}}); + let value = json!({"@type":"g:Edge","@value":{"id":{"@type":"g:Int32","@value":13},"label":"develops","inVLabel":"software","outVLabel":"person","inV":{"@type":"g:Int32","@value":10},"outV":{"@type":"g:Int32","@value":1},"properties":{"since":{"@type":"g:Int32","@value":2009}}}}); let result = deserializer_v2(&value).expect("Failed to deserialize an Edge"); + let actual: Edge = result.take().expect("Should have deserialized"); + + let expected = edge!({ + id => 13, + label=> "develops", + inV => { + id => 10, + label => "software" + }, + outV => { + id => 1, + label => "person" + }, + properties => { + "since" => 2009i32 + } + }); - assert_eq!( - result, - edge!({ - id => 13, - label=> "develops", - inV => { - id => 10, - label => "software" - }, - outV => { - id => 1, - label => "person" - }, - properties => { - - } - }) - .into() - ); + assert_eq!(actual, expected); + //Now make sure the properties deserialized correctly + let expected_properties: Vec<(String, Property)> = expected.into_iter().collect(); + let actual_properites: Vec<(String, Property)> = actual.into_iter().collect(); + assert_eq!(actual_properites, expected_properties); } #[test] diff --git a/gremlin-client/src/io/serializer_v3.rs b/gremlin-client/src/io/serializer_v3.rs index ec96fc7b..ea4c6134 100644 --- a/gremlin-client/src/io/serializer_v3.rs +++ b/gremlin-client/src/io/serializer_v3.rs @@ -170,6 +170,20 @@ where let out_v_id = deserialize_id(reader, &val["outV"])?; let out_v_label = get_value!(&val["outVLabel"], Value::String)?.clone(); + let json_properties = &val["properties"]; + let properties = if json_properties.is_object() { + get_value!(&json_properties, Value::Object)? + .values() + .map(|value| { + deserialize_property(reader, &value["@value"]) + .and_then(|property| property.take::()) + .map(|property| (property.label().clone(), property)) + }) + .collect::>()? + } else { + HashMap::new() + }; + Ok(Edge::new( id, label, @@ -177,7 +191,7 @@ where in_v_label, out_v_id, out_v_label, - HashMap::new(), + properties, ) .into()) } @@ -439,7 +453,7 @@ mod tests { use super::deserializer_v3; use serde_json::json; - use crate::{edge, vertex}; + use crate::{edge, vertex, Edge}; use crate::structure::{ GValue, Map, Metric, Path, Property, Token, TraversalMetrics, Vertex, VertexProperty, GID, @@ -626,26 +640,29 @@ mod tests { let value = json!({"@type":"g:Edge","@value":{"id":{"@type":"g:Int32","@value":13},"label":"develops","inVLabel":"software","outVLabel":"person","inV":{"@type":"g:Int32","@value":10},"outV":{"@type":"g:Int32","@value":1},"properties":{"since":{"@type":"g:Property","@value":{"key":"since","value":{"@type":"g:Int32","@value":2009}}}}}}); let result = deserializer_v3(&value).expect("Failed to deserialize an Edge"); + let actual: Edge = result.take().expect("Should have deserialized"); + + let expected = edge!({ + id => 13, + label=> "develops", + inV => { + id => 10, + label => "software" + }, + outV => { + id => 1, + label => "person" + }, + properties => { + "since" => 2009i32 + } + }); - assert_eq!( - result, - edge!({ - id => 13, - label=> "develops", - inV => { - id => 10, - label => "software" - }, - outV => { - id => 1, - label => "person" - }, - properties => { - - } - }) - .into() - ); + assert_eq!(actual, expected); + //Now make sure the properties deserialized correctly + let expected_properties: Vec<(String, Property)> = expected.into_iter().collect(); + let actual_properites: Vec<(String, Property)> = actual.into_iter().collect(); + assert_eq!(actual_properites, expected_properties); } #[test] diff --git a/gremlin-client/src/structure/macros.rs b/gremlin-client/src/structure/macros.rs index 5b675765..43b557af 100644 --- a/gremlin-client/src/structure/macros.rs +++ b/gremlin-client/src/structure/macros.rs @@ -31,7 +31,7 @@ macro_rules! edge { #[allow(unused_mut)] let mut properties = ::std::collections::HashMap::::new(); $( - let p = Property::new($key.into(),$value.into()); + let p = Property::new($key,$value); properties.insert($key.into(),p); )* $crate::Edge::new($id.into(), $label, $inVId.into(),$inVLabel,$outVId.into(),$outVLabel,properties) diff --git a/gremlin-client/tests/custom_vertex_ids.rs b/gremlin-client/tests/custom_vertex_ids.rs index 802cbda4..4f378530 100644 --- a/gremlin-client/tests/custom_vertex_ids.rs +++ b/gremlin-client/tests/custom_vertex_ids.rs @@ -1,15 +1,16 @@ use std::collections::HashMap; -use common::io::{drop_vertices, expect_janusgraph_client}; +use common::io::expect_janusgraph_client; use gremlin_client::{ process::traversal::{traversal, __}, + structure::Edge, structure::T, - GKey, GValue, IoProtocol, + GKey, GValue, IoProtocol, Property, }; use rstest::*; use rstest_reuse::apply; -use serial_test::serial; +use uuid::Uuid; mod common; @@ -17,7 +18,6 @@ mod common; //https://docs.janusgraph.org/advanced-topics/custom-vertex-id/ #[apply(common::serializers)] -#[serial(test_mapping_custom_vertex_id)] #[cfg(feature = "derive")] fn test_mapping_custom_vertex_id(protocol: IoProtocol) { if protocol == IoProtocol::GraphSONV2 { @@ -31,14 +31,13 @@ fn test_mapping_custom_vertex_id(protocol: IoProtocol) { use gremlin_client::process::traversal::{Bytecode, TraversalBuilder}; use std::convert::TryFrom; - drop_vertices(&client, "test_mapping_custom_vertex_id").unwrap(); - let g = traversal().with_remote(client); let uuid = uuid::Uuid::new_v4(); + let mark_id = create_novel_vertex_id(); let mark = g .add_v("test_mapping_custom_vertex_id") - .property(T::Id, "test_mapping_custom_id") + .property(T::Id, mark_id.as_str()) .property("name", "Mark") .property("age", 22) .property("time", 22 as i64) @@ -66,7 +65,7 @@ fn test_mapping_custom_vertex_id(protocol: IoProtocol) { assert_eq!( Person { - id: String::from("test_mapping_custom_id"), + id: mark_id, label: String::from("test_mapping_custom_vertex_id"), name: String::from("Mark"), age: 22, @@ -80,20 +79,64 @@ fn test_mapping_custom_vertex_id(protocol: IoProtocol) { } #[apply(common::serializers)] -#[serial(test_merge_v_custom_id)] +fn test_jg_add_e(protocol: IoProtocol) { + //It seems JanusGraph differs from the standard Tinkerpop Gremlin Server in how it returns edges + //JG returns edge properties in the same call if the edge is the terminal type + let client = expect_janusgraph_client(protocol); + let g = traversal().with_remote(client.clone()); + let v1 = g + .add_v("test_jg_add_e") + .property(T::Id, create_novel_vertex_id()) + .property("name", "foo") + .next() + .expect("Should to get response") + .expect("Should have gotten a vertex"); + + let v2 = g + .add_v("test_jg_add_e") + .property(T::Id, create_novel_vertex_id()) + .next() + .expect("Should get response") + .expect("Should have gotten a vertex"); + + let created_edge = g + .v(v1.id().clone()) + .out_e("knows") + .where_(__.in_v().id().is(v2.id().clone())) + .fold() + .coalesce::([ + __.unfold(), + __.add_e("knows") + .from(__.v(v1.id().clone())) + .to(__.v(v2.id().clone())), + ]) + .property("someKey", "someValue") + .next() + .expect("Should get response") + .expect("Should get edge"); + let actual_property = created_edge + .property("someKey") + .expect("Should have had property"); + let actual_property = actual_property + .get::() + .expect("Should have had property with expected type"); + assert_eq!(actual_property, "someValue"); +} + +#[apply(common::serializers)] fn test_merge_v_custom_id(protocol: IoProtocol) { if protocol == IoProtocol::GraphSONV2 { //GraphSONV2 doesn't support the non-string key of the merge step, //so skip it in testing return; } + let client = expect_janusgraph_client(protocol); - let expected_label = "test_merge_v_custom_id"; - drop_vertices(&client, expected_label).expect("Failed to drop vertices"); let g = traversal().with_remote(client); - let expected_id = "test_merge_v_custom_id"; + let expected_id = create_novel_vertex_id(); + let expected_label = "test_merge_v_custom_id"; let mut start_step_map: HashMap = HashMap::new(); - start_step_map.insert(T::Id.into(), expected_id.into()); + start_step_map.insert(T::Id.into(), expected_id.clone().into()); start_step_map.insert(T::Label.into(), expected_label.into()); let actual_vertex = g .merge_v(start_step_map) @@ -101,19 +144,19 @@ fn test_merge_v_custom_id(protocol: IoProtocol) { .expect("Should get a response") .expect("Should return a vertex"); match actual_vertex.id() { - gremlin_client::GID::String(actual) => assert_eq!(expected_id, actual), + gremlin_client::GID::String(actual) => assert_eq!(&expected_id, actual), other => panic!("Didn't get expected id type {:?}", other), } assert_eq!(expected_label, actual_vertex.label()); //Now try it as a mid-traversal step (inject is the start step) - let expected_id = "foo"; + let expected_id = create_novel_vertex_id(); let expected_property = "propValue"; let mut map_to_inject: HashMap = HashMap::new(); let mut lookup_map: HashMap = HashMap::new(); - lookup_map.insert(T::Id.into(), expected_id.into()); + lookup_map.insert(T::Id.into(), expected_id.clone().into()); lookup_map.insert(T::Label.into(), "myvertexlabel".into()); let mut property_map: HashMap = HashMap::new(); property_map.insert("propertyKey".into(), expected_property.into()); @@ -136,7 +179,7 @@ fn test_merge_v_custom_id(protocol: IoProtocol) { .expect("Should have returned a vertex"); match actual_vertex.id() { - gremlin_client::GID::String(actual) => assert_eq!(expected_id, actual), + gremlin_client::GID::String(actual) => assert_eq!(&expected_id, actual), other => panic!("Didn't get expected id type {:?}", other), } @@ -149,21 +192,25 @@ fn test_merge_v_custom_id(protocol: IoProtocol) { } #[apply(common::serializers)] -#[serial(test_merge_v_custom_id)] fn test_add_v_custom_id(protocol: IoProtocol) { let client = expect_janusgraph_client(protocol); - let expected_id = "test_add_v_custom_id"; - let test_vertex_label = "test_add_v_custom_id"; - drop_vertices(&client, test_vertex_label).expect("Failed to drop vertices"); + let expected_id = create_novel_vertex_id(); let g = traversal().with_remote(client); let actual_vertex = g - .add_v(test_vertex_label) - .property(T::Id, expected_id) + .add_v("test_add_v_custom_id") + .property(T::Id, &expected_id) .next() .expect("Should get a response") .expect("Should return a vertex"); match actual_vertex.id() { - gremlin_client::GID::String(actual) => assert_eq!(expected_id, actual), + gremlin_client::GID::String(actual) => assert_eq!(&expected_id, actual), other => panic!("Didn't get expected id type {:?}", other), } } + +fn create_novel_vertex_id() -> String { + //JanusGraph by default treats "-" as a reserved character + //we can override it, but to stay closer to the default nature of JG just map the character to "_" + //in our generated vertex ids + Uuid::new_v4().to_string().replace("-", "_") +}