From 41dd5eb524b661f32687b0e786f7cd30825de17e Mon Sep 17 00:00:00 2001 From: Victor Bustamante Date: Thu, 25 Jul 2024 16:27:38 -0300 Subject: [PATCH] fix: don't delete auth funcs when regenerating variants Signed-off-by: Victor Bustamante --- lib/dal/src/schema/variant.rs | 46 ++++++++-- lib/dal/src/schema/variant/authoring.rs | 14 ++-- .../schema/variant/authoring.rs | 1 + .../schema/variant/authoring/regenerate.rs | 84 +++++++++++++++++++ 4 files changed, 132 insertions(+), 13 deletions(-) create mode 100644 lib/dal/tests/integration_test/schema/variant/authoring/regenerate.rs diff --git a/lib/dal/src/schema/variant.rs b/lib/dal/src/schema/variant.rs index 58cbc73a6b..5af4158f5a 100644 --- a/lib/dal/src/schema/variant.rs +++ b/lib/dal/src/schema/variant.rs @@ -1993,18 +1993,48 @@ impl SchemaVariant { Ok(pairs) } - pub async fn remove_direct_connected_edges(&self, ctx: &DalContext) -> SchemaVariantResult<()> { + /// Remove all direct nodes connected to the schema variant, while being careful to not + /// change anything outside the variant + /// TODO: Implement and actual "delete schema variant function" + pub async fn remove_external_connections(&self, ctx: &DalContext) -> SchemaVariantResult<()> { let workspace_snapshot = ctx.workspace_snapshot()?; let maybe_schema_indices = workspace_snapshot.edges_directed(self.id, Outgoing).await?; - for (_edge_weight, _source_index, target_index) in maybe_schema_indices { - workspace_snapshot - .remove_node_by_id( - ctx.vector_clock_id()?, - workspace_snapshot.get_node_weight(target_index).await?.id(), - ) - .await?; + for (edge_weight, source_index, target_index) in maybe_schema_indices { + let kind = EdgeWeightKindDiscriminants::from(edge_weight.kind()); + match kind { + EdgeWeightKindDiscriminants::Use + | EdgeWeightKindDiscriminants::Socket + | EdgeWeightKindDiscriminants::ActionPrototype => { + workspace_snapshot + .remove_all_edges( + ctx.vector_clock_id()?, + workspace_snapshot.get_node_weight(target_index).await?.id(), + ) + .await?; + } + + EdgeWeightKindDiscriminants::AuthenticationPrototype => { + workspace_snapshot + .remove_edge(ctx.vector_clock_id()?, source_index, target_index, kind) + .await?; + } + + EdgeWeightKindDiscriminants::Action + | EdgeWeightKindDiscriminants::Contain + | EdgeWeightKindDiscriminants::FrameContains + | EdgeWeightKindDiscriminants::Ordering + | EdgeWeightKindDiscriminants::Ordinal + | EdgeWeightKindDiscriminants::Prop + | EdgeWeightKindDiscriminants::Prototype + | EdgeWeightKindDiscriminants::PrototypeArgument + | EdgeWeightKindDiscriminants::PrototypeArgumentValue + | EdgeWeightKindDiscriminants::Proxy + | EdgeWeightKindDiscriminants::Root + | EdgeWeightKindDiscriminants::SocketValue + | EdgeWeightKindDiscriminants::ValidationOutput => {} + } } Ok(()) diff --git a/lib/dal/src/schema/variant/authoring.rs b/lib/dal/src/schema/variant/authoring.rs index 96b5ff9c4e..b645e7b577 100644 --- a/lib/dal/src/schema/variant/authoring.rs +++ b/lib/dal/src/schema/variant/authoring.rs @@ -354,11 +354,11 @@ impl VariantAuthoringClient { component_type: ComponentType, ) -> VariantAuthoringResult<()> { // Ok we need to delete the first level of outgoing children for the schema variant - let schema_variant = + let current_schema_variant = SchemaVariant::get_by_id_or_error(ctx, current_schema_variant_id).await?; // then we can build the package and reimport ALL but the schema variant itself - let asset_func = schema_variant.get_asset_func(ctx).await?; + let asset_func = current_schema_variant.get_asset_func(ctx).await?; let display_name = display_name.into(); let category = category.into(); @@ -427,8 +427,12 @@ impl VariantAuthoringClient { .await?; // We need to clean up the old graph before we re-import the new parts! - schema_variant.remove_direct_connected_edges(ctx).await?; - schema_variant.rebuild_variant_root_prop(ctx).await?; + current_schema_variant + .remove_external_connections(ctx) + .await?; + current_schema_variant + .rebuild_variant_root_prop(ctx) + .await?; // Now we can reimport all parts of the schema variant in place! let mut thing_map = import_only_new_funcs(ctx, pkg.funcs()?).await?; @@ -439,7 +443,7 @@ impl VariantAuthoringClient { variant_pkg_spec, None, &mut thing_map, - Some(schema_variant), + Some(current_schema_variant), ) .await?; diff --git a/lib/dal/tests/integration_test/schema/variant/authoring.rs b/lib/dal/tests/integration_test/schema/variant/authoring.rs index 4a0932ed61..469f95d056 100644 --- a/lib/dal/tests/integration_test/schema/variant/authoring.rs +++ b/lib/dal/tests/integration_test/schema/variant/authoring.rs @@ -1,6 +1,7 @@ mod clone_variant; mod create_variant; mod delete_unlocked_variant; +mod regenerate; mod save_variant; mod unlock_and_edit_variant; mod update_variant; diff --git a/lib/dal/tests/integration_test/schema/variant/authoring/regenerate.rs b/lib/dal/tests/integration_test/schema/variant/authoring/regenerate.rs new file mode 100644 index 0000000000..7865506650 --- /dev/null +++ b/lib/dal/tests/integration_test/schema/variant/authoring/regenerate.rs @@ -0,0 +1,84 @@ +use dal::func::summary::FuncSummary; +use dal::schema::variant::authoring::VariantAuthoringClient; +use dal::{DalContext, Func, Schema, SchemaVariant}; + +use dal_test::helpers::ChangeSetTestHelpers; +use dal_test::test; + +#[test] +async fn regenerate_variant(ctx: &mut DalContext) { + ChangeSetTestHelpers::fork_from_head_change_set(ctx) + .await + .expect("could not fork head"); + // find the variant we know is default and attached to this func already + let schema = Schema::find_by_name(ctx, "dummy-secret") + .await + .expect("unable to find by name") + .expect("no schema found"); + + let schema_variant_id = SchemaVariant::get_default_id_for_schema(ctx, schema.id()) + .await + .expect("unable to get default schema variant"); + // Cache the total number of funcs before continuing. + let funcs = FuncSummary::list_for_schema_variant_id(ctx, schema_variant_id) + .await + .expect("unable to get the funcs for a schema variant"); + + // Get the Auth Func + let fn_name = "test:setDummySecretString"; + let func_id = Func::find_id_by_name(ctx, fn_name) + .await + .expect("found auth func") + .expect("has a func"); + + // ensure the func is attached + assert!(funcs.into_iter().any(|func| func.id == func_id)); + + // unlock schema variant + let unlocked_schema_variant = + VariantAuthoringClient::create_unlocked_variant_copy(ctx, schema_variant_id) + .await + .expect("could not unlock variant"); + + // ensure func is attached to new variant + let funcs_for_unlocked = + FuncSummary::list_for_schema_variant_id(ctx, unlocked_schema_variant.id) + .await + .expect("unable to get the funcs for a schema variant"); + + // ensure the func is attached + assert!(funcs_for_unlocked + .into_iter() + .any(|func| func.id == func_id)); + + // get the existing default variant and ensure the auth func is still attached to it + let funcs_for_default = FuncSummary::list_for_schema_variant_id(ctx, schema_variant_id) + .await + .expect("unable to get the funcs for a schema variant"); + // ensure the func is attached + assert!(funcs_for_default.into_iter().any(|func| func.id == func_id)); + + // regenerate variant + VariantAuthoringClient::regenerate_variant(ctx, unlocked_schema_variant.id) + .await + .expect("could not regenerate variant"); + + // ensure funcs are attached to regenerated AND the existing default + // ensure func is attached to new variant + let funcs_for_unlocked = + FuncSummary::list_for_schema_variant_id(ctx, unlocked_schema_variant.id) + .await + .expect("unable to get the funcs for a schema variant"); + + // ensure the func is attached + assert!(funcs_for_unlocked + .into_iter() + .any(|func| func.id == func_id)); + + // get the existing default variant and ensure the auth func is still attached to it + let funcs_for_default = FuncSummary::list_for_schema_variant_id(ctx, schema_variant_id) + .await + .expect("unable to get the funcs for a schema variant"); + // ensure the func is attached + assert!(funcs_for_default.into_iter().any(|func| func.id == func_id)); +}