From b46a33638a8c8f0115f1fd4307e7f6f1772e4329 Mon Sep 17 00:00:00 2001 From: Zachary Hamm Date: Mon, 14 Oct 2024 13:39:34 -0500 Subject: [PATCH] chore: add a test to prevent breaking graph deserializations Commits a small snapshot to the repository that has a graph with one instance of every node weight and one of every edge weight kind, then ensures we can deserialize it. This is a first pass at ensuring code cannot be merged that will break graph deserialization. Another test is necessary that writes one of every content object somewhere and attempts to deserialize all of them. This test graph can be generated at any time by running buck2 run //lib/dal:test-integration -- write_deserialization_data --ignored Then you just have to update the constant in lib/dal/tests/integration_test/deserialize/mod.rs to point to the new graph. --- lib/dal/BUCK | 1 + lib/dal/examples/rebase/main.rs | 11 +- lib/dal/src/layer_db_types/content_types.rs | 1 + lib/dal/src/prop.rs | 1 - lib/dal/src/workspace_snapshot/edge_weight.rs | 2 +- lib/dal/src/workspace_snapshot/node_weight.rs | 4 +- .../tests/integration_test/deserialize/mod.rs | 276 ++++++++++++++++++ lib/dal/tests/integration_test/mod.rs | 1 + ...erialization-test-data-2024-10-14.snapshot | Bin 0 -> 1474 bytes 9 files changed, 283 insertions(+), 14 deletions(-) create mode 100644 lib/dal/tests/integration_test/deserialize/mod.rs create mode 100644 lib/dal/tests/serialization-test-data-2024-10-14.snapshot diff --git a/lib/dal/BUCK b/lib/dal/BUCK index 92cdd02636..be2eabb6e9 100644 --- a/lib/dal/BUCK +++ b/lib/dal/BUCK @@ -93,6 +93,7 @@ rust_test( "//lib/rebaser-server:rebaser-server", "//lib/si-events-rs:si-events", "//lib/si-frontend-types-rs:si-frontend-types", + "//lib/si-layer-cache:si-layer-cache", "//lib/si-pkg:si-pkg", "//lib/veritech-client:veritech-client", "//third-party/rust:chrono", diff --git a/lib/dal/examples/rebase/main.rs b/lib/dal/examples/rebase/main.rs index ccb5de2bb0..5721047bf5 100644 --- a/lib/dal/examples/rebase/main.rs +++ b/lib/dal/examples/rebase/main.rs @@ -1,17 +1,10 @@ use std::{env, fs::File, io::prelude::*}; use petgraph::{visit::EdgeRef, Direction::Incoming}; -use regex::bytes; use serde::de::DeserializeOwned; use si_layer_cache::db::serialize; -use dal::{ - workspace_snapshot::{ - graph::{correct_transforms::correct_transforms, RebaseBatch}, - node_weight::NodeWeight, - }, - WorkspaceSnapshotGraph, -}; +use dal::{workspace_snapshot::node_weight::NodeWeight, WorkspaceSnapshotGraph}; type Result = std::result::Result>; @@ -73,5 +66,3 @@ fn main() -> Result<()> { Ok(()) } - -// 01J2F7HKZFMTN6GVKXE73044AT diff --git a/lib/dal/src/layer_db_types/content_types.rs b/lib/dal/src/layer_db_types/content_types.rs index a72fdb90a1..5c086b5db6 100644 --- a/lib/dal/src/layer_db_types/content_types.rs +++ b/lib/dal/src/layer_db_types/content_types.rs @@ -37,6 +37,7 @@ pub type ContentTypeResult = Result; /// with postcard, DO *NOT* add new types to this list in alphabetical order. /// Add them to the *END* of the enum *ONLY*. #[derive(EnumDiscriminants, Serialize, Deserialize, Clone, PartialEq, Debug)] +#[strum_discriminants(derive(strum::EnumIter))] pub enum ContentTypes { Any(CasValue), AttributePrototype(AttributePrototypeContent), diff --git a/lib/dal/src/prop.rs b/lib/dal/src/prop.rs index 0dd224b081..fbfd5b4fec 100644 --- a/lib/dal/src/prop.rs +++ b/lib/dal/src/prop.rs @@ -255,7 +255,6 @@ impl From for PropPath { } } -#[remain::sorted] #[derive( AsRefStr, Clone, diff --git a/lib/dal/src/workspace_snapshot/edge_weight.rs b/lib/dal/src/workspace_snapshot/edge_weight.rs index a963bcce56..c4fc73d8b5 100644 --- a/lib/dal/src/workspace_snapshot/edge_weight.rs +++ b/lib/dal/src/workspace_snapshot/edge_weight.rs @@ -8,7 +8,7 @@ pub mod deprecated; /// This type is postcard serialized and new enum variants *MUST* be added to the end *ONLY*. #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash, EnumDiscriminants)] -#[strum_discriminants(derive(Hash, Serialize, Deserialize))] +#[strum_discriminants(derive(Hash, Serialize, Deserialize, strum::EnumIter))] pub enum EdgeWeightKind { Action, /// A function used by a [`SchemaVariant`] to perform an action that affects its resource diff --git a/lib/dal/src/workspace_snapshot/node_weight.rs b/lib/dal/src/workspace_snapshot/node_weight.rs index f53232c373..45f69ad26b 100644 --- a/lib/dal/src/workspace_snapshot/node_weight.rs +++ b/lib/dal/src/workspace_snapshot/node_weight.rs @@ -4,7 +4,7 @@ use finished_dependent_value_root_node_weight::FinishedDependentValueRootNodeWei use serde::{Deserialize, Serialize}; use si_events::{merkle_tree_hash::MerkleTreeHash, ulid::Ulid, ContentHash, EncryptedSecretKey}; use si_layer_cache::LayerDbError; -use strum::EnumDiscriminants; +use strum::{EnumDiscriminants, EnumIter}; use thiserror::Error; use traits::{CorrectExclusiveOutgoingEdge, CorrectTransforms, CorrectTransformsResult}; @@ -111,7 +111,7 @@ pub type NodeWeightResult = Result; /// **WARNING**: the order of this enum is important! Do not re-order elements. /// New variants must go at the end, even if it's not in lexical order! #[derive(Debug, Serialize, Deserialize, Clone, EnumDiscriminants, PartialEq, Eq)] -#[strum_discriminants(derive(strum::Display, Hash, Serialize, Deserialize))] +#[strum_discriminants(derive(strum::Display, Hash, Serialize, Deserialize, EnumIter))] pub enum NodeWeight { Action(ActionNodeWeight), ActionPrototype(ActionPrototypeNodeWeight), diff --git a/lib/dal/tests/integration_test/deserialize/mod.rs b/lib/dal/tests/integration_test/deserialize/mod.rs new file mode 100644 index 0000000000..f5e453beb9 --- /dev/null +++ b/lib/dal/tests/integration_test/deserialize/mod.rs @@ -0,0 +1,276 @@ +use std::io::Read; + +use dal::action::prototype::ActionKind; +use dal::func::FuncKind; +use dal::workspace_snapshot::content_address::ContentAddress; +use dal::workspace_snapshot::node_weight::category_node_weight::CategoryNodeKind; +use dal::workspace_snapshot::node_weight::traits::SiVersionedNodeWeight; +use dal::workspace_snapshot::node_weight::{ + ArgumentTargets, CategoryNodeWeight, NodeWeight, OrderingNodeWeight, +}; +use dal::{ + ContentHash, DalContext, EdgeWeight, EdgeWeightKind, EdgeWeightKindDiscriminants, + NodeWeightDiscriminants, SocketArity, WorkspaceSnapshotGraph, WorkspaceSnapshotGraphVCurrent, +}; +use dal::{PropKind, Ulid}; +use dal_test::test; +use si_events::EncryptedSecretKey; +use si_layer_cache::db::serialize; +use strum::IntoEnumIterator; + +const CURRENT_SERIALIZED_GRAPH_DIR_PATH: &str = "./lib/dal/tests"; +const CURRENT_SERIALIZED_GRAPH_FILENAME: &str = "serialization-test-data-2024-10-14.snapshot"; + +// If you're modifying this, you probably just added a new node or edge weight. Before you replace +// the snapshot with one that includes the new weights, ensure that your current code passes the +// deserialization test against the snapshot with the *old* weights. If it does, update the +// validation snapshot file. +#[allow(unused)] +fn make_me_one_with_everything(graph: &mut WorkspaceSnapshotGraphVCurrent) { + let mut node_indexes = vec![]; + + // For every enum that goes into a node weight, try to pick the last variant + // in the enum, so on the verification pass we will break if anything is + // added out of order + for node_kind in NodeWeightDiscriminants::iter() { + let weight = match node_kind { + NodeWeightDiscriminants::Action => { + NodeWeight::new_action(Ulid::new().into(), Ulid::new(), Ulid::new()) + } + NodeWeightDiscriminants::ActionPrototype => NodeWeight::new_action_prototype( + Ulid::new(), + Ulid::new(), + ActionKind::Update, + "foo".into(), + None, + ), + NodeWeightDiscriminants::AttributePrototypeArgument => { + NodeWeight::new_attribute_prototype_argument( + Ulid::new(), + Ulid::new(), + Some(ArgumentTargets { + source_component_id: Ulid::new().into(), + destination_component_id: Ulid::new().into(), + }), + ) + } + NodeWeightDiscriminants::AttributeValue => NodeWeight::new_attribute_value( + Ulid::new(), + Ulid::new(), + Some(ContentAddress::JsonValue(ContentHash::new( + "foo".as_bytes(), + ))), + Some(ContentAddress::JsonValue(ContentHash::new( + "foo".as_bytes(), + ))), + ), + NodeWeightDiscriminants::Category => NodeWeight::Category(CategoryNodeWeight::new( + Ulid::new(), + Ulid::new(), + CategoryNodeKind::DependentValueRoots, + )), + NodeWeightDiscriminants::Component => NodeWeight::new_component( + Ulid::new(), + Ulid::new(), + ContentHash::new("bar".as_bytes()), + ), + NodeWeightDiscriminants::Content => NodeWeight::new_content( + Ulid::new(), + Ulid::new(), + ContentAddress::ManagementPrototype(ContentHash::new("baz".as_bytes())), + ), + NodeWeightDiscriminants::DependentValueRoot => { + NodeWeight::new_dependent_value_root(Ulid::new(), Ulid::new(), Ulid::new()) + } + NodeWeightDiscriminants::Func => NodeWeight::new_func( + Ulid::new(), + Ulid::new(), + "foo", + FuncKind::Management, + ContentHash::new("quux".as_bytes()), + ), + NodeWeightDiscriminants::FuncArgument => NodeWeight::new_func_argument( + Ulid::new(), + Ulid::new(), + "arg", + ContentHash::new("the attitude era".as_bytes()), + ), + NodeWeightDiscriminants::Ordering => { + NodeWeight::Ordering(OrderingNodeWeight::new(Ulid::new(), Ulid::new())) + } + NodeWeightDiscriminants::Prop => NodeWeight::new_prop( + Ulid::new(), + Ulid::new(), + PropKind::String, + "foo", + ContentHash::new("bar".as_bytes()), + ), + NodeWeightDiscriminants::Secret => NodeWeight::new_secret( + Ulid::new(), + Ulid::new(), + EncryptedSecretKey::new("shhh".as_bytes()), + ContentHash::new("content".as_bytes()), + ), + NodeWeightDiscriminants::FinishedDependentValueRoot => { + NodeWeight::new_finished_dependent_value_root(Ulid::new(), Ulid::new(), Ulid::new()) + } + NodeWeightDiscriminants::InputSocket => NodeWeight::new_input_socket( + Ulid::new(), + Ulid::new(), + SocketArity::Many, + ContentHash::new("bar".as_bytes()), + ), + NodeWeightDiscriminants::SchemaVariant => NodeWeight::new_schema_variant( + Ulid::new(), + Ulid::new(), + false, + ContentHash::new("variant".as_bytes()), + ), + NodeWeightDiscriminants::ManagementPrototype => NodeWeight::new_management_prototype( + Ulid::new(), + Ulid::new(), + ContentHash::new("management".as_bytes()), + ), + }; + + let idx = graph.add_or_replace_node(weight).expect("add node"); + // Attach to root + graph + .add_edge( + graph.root(), + EdgeWeight::new(EdgeWeightKind::new_use()), + idx, + ) + .expect("add edge"); + node_indexes.push(idx); + } + + let mut last_node = 0; + + for edge_kind in EdgeWeightKindDiscriminants::iter() { + if last_node + 1 == node_indexes.len() { + last_node = 0; + }; + let edge_weight_kind = match edge_kind { + EdgeWeightKindDiscriminants::Action => EdgeWeightKind::Action, + EdgeWeightKindDiscriminants::ActionPrototype => EdgeWeightKind::ActionPrototype, + EdgeWeightKindDiscriminants::AuthenticationPrototype => { + EdgeWeightKind::AuthenticationPrototype + } + EdgeWeightKindDiscriminants::Contain => { + EdgeWeightKind::Contain(Some("foo".to_string())) + } + EdgeWeightKindDiscriminants::FrameContains => EdgeWeightKind::FrameContains, + EdgeWeightKindDiscriminants::Ordering => EdgeWeightKind::Ordering, + EdgeWeightKindDiscriminants::Ordinal => EdgeWeightKind::Ordinal, + EdgeWeightKindDiscriminants::Prop => EdgeWeightKind::Prop, + EdgeWeightKindDiscriminants::Prototype => { + EdgeWeightKind::Prototype(Some("bar".to_string())) + } + EdgeWeightKindDiscriminants::PrototypeArgument => EdgeWeightKind::PrototypeArgument, + EdgeWeightKindDiscriminants::PrototypeArgumentValue => { + EdgeWeightKind::PrototypeArgumentValue + } + EdgeWeightKindDiscriminants::Proxy => EdgeWeightKind::Proxy, + EdgeWeightKindDiscriminants::Root => EdgeWeightKind::Root, + EdgeWeightKindDiscriminants::Socket => EdgeWeightKind::Socket, + EdgeWeightKindDiscriminants::SocketValue => EdgeWeightKind::SocketValue, + EdgeWeightKindDiscriminants::Use => EdgeWeightKind::new_use(), + EdgeWeightKindDiscriminants::ValidationOutput => EdgeWeightKind::ValidationOutput, + EdgeWeightKindDiscriminants::ManagementPrototype => EdgeWeightKind::ManagementPrototype, + }; + + let edge_weight = EdgeWeight::new(edge_weight_kind); + + graph + .add_edge( + node_indexes[last_node], + edge_weight, + node_indexes[last_node + 1], + ) + .expect("add edge"); + + last_node += 1; + } +} + +// Run this test to produce a serialized version of the graph. Do this any time +// a breaking change in serialization occurs. +// +// cd to the root of "si" +// then run: +// +// $ buck2 run //lib/dal:test-integration -- write_deserialization_data --ignored +// +// Then delete the old copy of the graph, and replace the constant +// `CURRENT_SERIALIZED_GRAPH_FILENAME` with the filename of the new graph. +#[test] +#[ignore = "only run this when you want to produce a new serialized graph"] +async fn write_deserialization_data(_ctx: &DalContext) { + let mut graph = WorkspaceSnapshotGraphVCurrent::new().expect("make new"); + + make_me_one_with_everything(&mut graph); + + graph.cleanup_and_merkle_tree_hash().expect("hash it"); + + let real_graph = WorkspaceSnapshotGraph::V3(graph); + let serialized = serialize::to_vec(&real_graph).expect("serialize"); + + let date = chrono::Utc::now().format("%Y-%m-%d").to_string(); + + let mut file = std::fs::File::create(format!( + "{CURRENT_SERIALIZED_GRAPH_DIR_PATH}/serialization-test-data-{date}.snapshot" + )) + .expect("create file"); + file.write_all(&serialized).expect("write file"); +} + +#[test] +async fn graph_can_be_deserialized(_ctx: &DalContext) { + let mut file = std::fs::File::open(format!( + "{CURRENT_SERIALIZED_GRAPH_DIR_PATH}/{CURRENT_SERIALIZED_GRAPH_FILENAME}" + )) + .expect("open file"); + let mut bytes = vec![]; + file.read_to_end(&mut bytes).expect("able to read bytes"); + + let graph: WorkspaceSnapshotGraph = serialize::from_bytes(&bytes).expect("deserialize"); + + assert_eq!(18, graph.node_count()); + + // Where we can, verify that the enums on the node weights match what we expect + for (node_weight, _) in graph.nodes() { + match node_weight { + NodeWeight::Action(_) => {} + NodeWeight::ActionPrototype(action_prototype_node_weight) => { + assert_eq!(ActionKind::Update, action_prototype_node_weight.kind()); + } + NodeWeight::AttributePrototypeArgument(_) => {} + NodeWeight::AttributeValue(_) => {} + NodeWeight::Category(category_node_weight) => { + assert_eq!( + CategoryNodeKind::DependentValueRoots, + category_node_weight.kind() + ); + } + NodeWeight::Component(_) => {} + NodeWeight::Content(_) => {} + NodeWeight::DependentValueRoot(_) => {} + NodeWeight::Func(func_node_weight) => { + assert_eq!(FuncKind::Management, func_node_weight.func_kind()); + } + NodeWeight::FuncArgument(_) => {} + NodeWeight::Ordering(_) => {} + NodeWeight::Prop(prop_node_weight) => { + assert_eq!(PropKind::String, prop_node_weight.kind()); + } + NodeWeight::Secret(_) => {} + NodeWeight::FinishedDependentValueRoot(_) => {} + NodeWeight::InputSocket(input_socket_node_weight) => { + assert_eq!(SocketArity::Many, input_socket_node_weight.inner().arity()); + } + NodeWeight::SchemaVariant(_) => {} + NodeWeight::ManagementPrototype(_) => {} + } + } +} diff --git a/lib/dal/tests/integration_test/mod.rs b/lib/dal/tests/integration_test/mod.rs index 333574779f..ba754492bd 100644 --- a/lib/dal/tests/integration_test/mod.rs +++ b/lib/dal/tests/integration_test/mod.rs @@ -6,6 +6,7 @@ mod component; mod connection; mod cycle_check_guard; mod dependent_values_update; +mod deserialize; mod diagram; mod frame; mod func; diff --git a/lib/dal/tests/serialization-test-data-2024-10-14.snapshot b/lib/dal/tests/serialization-test-data-2024-10-14.snapshot new file mode 100644 index 0000000000000000000000000000000000000000..520bdad4f84da75e60dbb87c0d6da3586fc05ffc GIT binary patch literal 1474 zcmV;z1wHzOR(mv*X&nCExqWk=m(y0O4uxX1tQm7L%GM;8)w)G)XDGUG*v6i)ooZEd zN{SXyY;;*lizr7bCF@jHtxD3Ou(h>i*-Vs5S=LwY=k09k>_5*R&-?z~dA{H68Kxlz zg3MH(_I{83>jvz@3?;1{|0X&`u*3UxCO?Fc)pfOOzOLzTBtf7{b@eAJ7DdJy-PSRU zY^ClRrNYkJ#Vycjv6H7twon%2qzDKI@JE;LdOG+XZP`3LkP91%`(kAS&NWp;^K1jJ z&PnpxXi;PBuRj;(WCCXP7Q&XLpUU%vL71mfBiN<$h9o2RoI6t;$9&m)fb73aK>s ze~y_*SN6g=nt280!rG~bh`GLvnBRRyWfKf%t`3Y$f``S`i*H_Pxlxo1cyMRJCUWI6 zUCB(1^H!Uovi1K#vCu<0Fg*9w?d{8mW<;3WuYM43yKCN*l_Wft8lG-bYQ8TkB=Du~ z(?Km8-N{`r=e+5<=IBM!5pi05xLsu;iw!x=fZ?}fhn6?2Xpx$LiaVchv!Q9nISsp1 zS`jr;{PNoTl34`ZJ_=Wpg6~_i7prX0Gi*1bkrZExt;w4w!C5IWb|BW+s>+;U`6Bx9 zFubB!ni`at{3I*CeMoCOFPI=vlv_^W=-%9(Ot~Hrn%T{LPwMI}Tpfie7kCO}_d?bZQ*ccU&Hdd|1D#)!WNQ-*Fic)02KAbL*Q! z^Jb7REutsgv2--C(6Q2`vZ>1tMtUpK%KO6Hf?fn#C)PPeGQ-wJ@KK3K=n7-G8QDY)gB{p+1EWqR@nc<$b?ajQ*8z3lReCv8eQ}4!b2n z2vhyrcbHjksK~DKvWPR>lmWLqW@;I|qEN`mHr~Rn*Ta(6BJYP*KA)`&U*R#is4GK+ zUSW4hv&g~F?Hv~op?|ZZ@8osK?Qh&@Li4x|h`YOtc;MX78L?k~$2nb0 z4>;+&cj&p)+%)G53fg_PVX0N4*~gcUL-Do*-LeB-2dutK&bz>NJV9_j`1FZJPpMR^ zA4sx<+d z!vY+V4M;8raCs!)@wtE*9<~HeQT+BwQ|rMJS<|wVA~}SgZ+Vp z7`$6a#Ncs}AO<&$0b;N<1H|Cv%B7Qtjg>0UTh-V%0i86wKgZk$yuBqkM{h4crv$qV zpeuss2S6kOZ!kbt06Pw#6NQ%upfh#Xlm7Pr=nujh4A2$Fdt