Merge pull request #4232 from systeminit/victor/bug-469-regenerating-…

…variant-with-auth-function-should-not-delete

fix: don't delete auth funcs when regenerating variants

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(())

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?;
 

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;

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));
+}