feat(sdf-server, dal): Exec variant def creates new variant, copies funcs

Instead of deleting the old schema variant and creating a new one, then attempting to migrate components, we just create a new schema variant on exec_variant_def and mark it as the default variant. Existing components are unchanged.

Component upgrades are coming next.

This is controlled by the "multiVariantEditing" flag, which is not yet in posthog (but can be easily turned on in the feature flags store).

Author: zacharyhamm

app/web/src/components/AssetDetailsPanel.vue

@@ -182,6 +182,7 @@ import { FuncVariant } from "@/api/sdf/dal/func";
 import { useAssetStore } from "@/store/asset.store";
 import { FuncId } from "@/store/func/funcs.store";
 import { ComponentType } from "@/components/ModelingDiagram/diagram_types";
+import { useFeatureFlagsStore } from "@/store/feature_flags.store";
 import ColorPicker from "./ColorPicker.vue";
 import AssetFuncAttachModal from "./AssetFuncAttachModal.vue";
 
@@ -196,6 +197,8 @@ const loadAssetReqStatus = assetStore.getRequestStatus(
 );
 const executeAssetModalRef = ref();
 
+const featureFlagStore = useFeatureFlagsStore();
+
 const openAttachModal = (warning: {
   variant?: FuncVariant;
   funcId?: FuncId;
@@ -244,10 +247,16 @@ const updateAsset = async () => {
   }
 };
 
-const disabled = computed(() => !!(editingAsset.value?.hasComponents ?? false));
+const disabled = computed(() => {
+  if (featureFlagStore.MULTI_VARIANT_EDITING) {
+    return false;
+  }
+
+  return !!(editingAsset.value?.hasComponents ?? false);
+});
 
 const disabledWarning = computed(() => {
-  if (editingAsset.value?.hasComponents) {
+  if (disabled.value) {
     return `This asset cannot be edited because it is in use by components.`;
   }
 

app/web/src/components/AssetEditor.vue

@@ -52,19 +52,26 @@ import {
 } from "@si/vue-lib/design-system";
 import { useAssetStore, assetDisplayName } from "@/store/asset.store";
 import SiChip from "@/components/SiChip.vue";
+import { useFeatureFlagsStore } from "@/store/feature_flags.store";
 import CodeEditor from "./CodeEditor.vue";
 import NodeSkeleton from "./NodeSkeleton.vue";
 
 const props = defineProps<{
   assetId?: string;
 }>();
 
+const featureFlagStore = useFeatureFlagsStore();
 const assetStore = useAssetStore();
 const selectedAsset = computed(() =>
   props.assetId ? assetStore.assetsById[props.assetId] : undefined,
 );
 
-const isReadOnly = computed(() => !!selectedAsset.value?.hasComponents);
+const isReadOnly = computed(() => {
+  if (featureFlagStore.MULTI_VARIANT_EDITING) {
+    return false;
+  }
+  return !!selectedAsset.value?.hasComponents;
+});
 
 const editingAsset = ref<string>(selectedAsset.value?.code ?? "");
 

app/web/src/store/asset.store.ts

@@ -96,10 +96,15 @@ export type Asset = VariantDef;
 export type AssetListEntry = ListedVariantDef;
 export type AssetSaveRequest = Visibility & {
   overrideBuiltinSchemaFeatureFlag: boolean;
+  multiVariantEditingFlag: boolean;
 } & Omit<Asset, "createdAt" | "updatedAt" | "variantExists" | "hasComponents">;
 export type AssetCreateRequest = Omit<
   AssetSaveRequest,
-  "id" | "definition" | "variantExists" | "overrideBuiltinSchemaFeatureFlag"
+  | "id"
+  | "definition"
+  | "variantExists"
+  | "overrideBuiltinSchemaFeatureFlag"
+  | "multiVariantEditingFlag"
 >;
 export type AssetCloneRequest = Visibility & { id: AssetId };
 
@@ -350,6 +355,7 @@ export const useAssetStore = () => {
             params: {
               overrideBuiltinSchemaFeatureFlag:
                 featureFlagsStore.OVERRIDE_SCHEMA,
+              multiVariantEditingFlag: featureFlagsStore.MULTI_VARIANT_EDITING,
               ...visibility,
               ..._.omit(asset, [
                 "schemaVariantId",
@@ -414,6 +420,7 @@ export const useAssetStore = () => {
             params: {
               overrideBuiltinSchemaFeatureFlag:
                 featureFlagsStore.OVERRIDE_SCHEMA,
+              multiVariantEditingFlag: featureFlagsStore.MULTI_VARIANT_EDITING,
               ...visibility,
               ..._.omit(asset, [
                 "schemaVariantId",

app/web/src/store/feature_flags.store.ts

@@ -6,17 +6,19 @@ import { posthog } from "@/utils/posthog";
 // translation from store key to posthog feature flag name
 const FLAG_MAPPING = {
   // STORE_FLAG_NAME: "posthogFlagName",
-  MODULES_TAB: "modules_tab",
-  SECRETS: "secrets",
-  INVITE_USER: "invite_user",
-  WORKSPACE_BACKUPS: "workspaceBackups",
+  // STORE_FLAG_NAME: "posthogFlagName",
   COPY_PASTE: "copy_paste",
   DONT_BLOCK_ON_ACTIONS: "dont_block_on_actions",
-  OVERRIDE_SCHEMA: "override_schema",
+  INVITE_USER: "invite_user",
   JOI_VALIDATIONS: "joi_validations",
-  SHOW_EDGES_ON_SELECT: "show_edges_on_select",
+  MODULES_TAB: "modules_tab",
+  MULTI_VARIANT_EDITING: "multiVariantEditing",
+  OVERRIDE_SCHEMA: "override_schema",
   REMOVE_COMPONENT_ICONS: "remove_component_icons",
   RESIZABLE_PANEL_UPGRADE: "resizable-panel-upgrade",
+  SECRETS: "secrets",
+  SHOW_EDGES_ON_SELECT: "show_edges_on_select",
+  WORKSPACE_BACKUPS: "workspaceBackups",
 };
 
 type FeatureFlags = keyof typeof FLAG_MAPPING;
@@ -42,6 +44,8 @@ export function useFeatureFlagsStore() {
         });
         // You can override feature flags while working on a feature by setting them to true here
 
+        // this.MULTI_VARIANT_EDITING = true;
+
         // Make sure to remove the override before committing your code!
       },
     }),

lib/dal/src/func.rs

@@ -165,14 +165,20 @@ impl Func {
 
     /// Creates a new [`Func`] from [`self`](Func). All relevant fields are duplicated, but rows
     /// existing on relationship tables (e.g. "belongs_to" or "many_to_many") are not.
-    pub async fn duplicate(&self, ctx: &DalContext) -> FuncResult<Self> {
-        // Generate a unique name and make sure it's not in use
+    pub async fn duplicate(&self, ctx: &DalContext, new_name: Option<String>) -> FuncResult<Self> {
+        // Generate a unique name and make sure it's not in use, unless we were
+        // passed in a name, in which case just use that
         let mut new_unique_name;
-        loop {
-            new_unique_name = format!("{}{}", self.name(), generate_unique_id(4));
-            if Self::find_by_name(ctx, &new_unique_name).await?.is_none() {
-                break;
-            };
+        match new_name {
+            Some(new_name) => {
+                new_unique_name = new_name;
+            }
+            None => loop {
+                new_unique_name = format!("{}{}", self.name(), generate_unique_id(4));
+                if Self::find_by_name(ctx, &new_unique_name).await?.is_none() {
+                    break;
+                };
+            },
         }
 
         let mut new_func = Self::new(

lib/dal/src/pkg.rs

@@ -33,8 +33,8 @@ use crate::{
     WsEvent, WsEventResult, WsPayload,
 };
 
-mod export;
-mod import;
+pub mod export;
+pub mod import;
 
 #[remain::sorted]
 #[derive(Debug, Error)]

lib/dal/src/pkg/export.rs

@@ -115,6 +115,10 @@ impl PkgExporter {
         }
     }
 
+    fn new_standalone_variant_exporter() -> Self {
+        Self::new_module_exporter("", "", None::<String>, "", vec![])
+    }
+
     pub async fn export_as_bytes(&mut self, ctx: &DalContext) -> PkgResult<Vec<u8>> {
         match self.kind {
             SiPkgKind::Module => info!("Building module package"),
@@ -145,40 +149,19 @@ impl PkgExporter {
         }
 
         let in_change_set = std_model_change_set_matches(change_set_pk, schema);
-        let is_deleted = schema.visibility().is_deleted();
+        let schema_is_deleted = schema.visibility().is_deleted();
 
         let default_variant_id = schema.default_schema_variant_id().copied();
         let mut default_variant_unique_id = None;
 
         for variant in &variants {
-            let related_funcs = SchemaVariant::all_funcs(ctx, *variant.id()).await?;
-
-            for func in &related_funcs {
-                if change_set_pk.is_some()
-                    && change_set_pk.as_ref().expect("some is ensured") != &ChangeSetPk::NONE
-                    && self.func_map.get(ChangeSetPk::NONE, func.id()).is_none()
-                    && func.visibility().change_set_pk == ChangeSetPk::NONE
-                {
-                    let (func_spec, _) =
-                        self.export_func(ctx, Some(ChangeSetPk::NONE), func).await?;
-                    self.func_map
-                        .insert(ChangeSetPk::NONE, *func.id(), func_spec.to_owned());
-                    head_funcs.push(func_spec);
-                } else {
-                    let (func_spec, include) = self.export_func(ctx, change_set_pk, func).await?;
-                    self.func_map.insert(
-                        change_set_pk.unwrap_or(ChangeSetPk::NONE),
-                        *func.id(),
-                        func_spec.to_owned(),
-                    );
-
-                    if include {
-                        funcs.push(func_spec);
-                    }
-                }
-            }
+            let (variant_funcs, variant_head_funcs) = self
+                .export_funcs_for_variant(ctx, change_set_pk, *variant.id())
+                .await?;
+            funcs.extend(variant_funcs);
+            head_funcs.extend(variant_head_funcs);
 
-            if !is_deleted {
+            if !schema_is_deleted {
                 let variant_spec = self.export_variant(ctx, change_set_pk, variant).await?;
                 self.variant_map.insert(
                     change_set_pk.unwrap_or(ChangeSetPk::NONE),
@@ -196,7 +179,7 @@ impl PkgExporter {
             }
         }
 
-        if in_change_set && is_deleted {
+        if in_change_set && schema_is_deleted {
             schema_spec_builder.deleted(true);
         } else if in_change_set {
             let mut data_builder = SchemaSpecData::builder();
@@ -222,6 +205,23 @@ impl PkgExporter {
         Ok((schema_spec, funcs, head_funcs))
     }
 
+    /// Exports just a single schema variant and the functions connected to it.
+    /// Visiblity is taken from the context, so this will export the schema
+    /// variant according to the normal rules of visibility.
+    pub async fn export_variant_standalone(
+        ctx: &DalContext,
+        variant: &SchemaVariant,
+    ) -> PkgResult<(SchemaVariantSpec, Vec<FuncSpec>)> {
+        let mut exporter = Self::new_standalone_variant_exporter();
+        let (funcs, _) = exporter
+            .export_funcs_for_variant(ctx, None, *variant.id())
+            .await?;
+
+        let variant_spec = exporter.export_variant(ctx, None, variant).await?;
+
+        Ok((variant_spec, funcs))
+    }
+
     async fn export_variant(
         &self,
         ctx: &DalContext,
@@ -1672,6 +1672,43 @@ impl PkgExporter {
 
         Ok(pkg)
     }
+
+    async fn export_funcs_for_variant(
+        &mut self,
+        ctx: &DalContext,
+        change_set_pk: Option<ChangeSetPk>,
+        schema_variant_id: SchemaVariantId,
+    ) -> PkgResult<(Vec<FuncSpec>, Vec<FuncSpec>)> {
+        let related_funcs = SchemaVariant::all_funcs(ctx, schema_variant_id).await?;
+        let mut head_funcs = vec![];
+        let mut funcs = vec![];
+
+        for func in &related_funcs {
+            if change_set_pk.is_some()
+                && change_set_pk.as_ref().expect("some is ensured") != &ChangeSetPk::NONE
+                && self.func_map.get(ChangeSetPk::NONE, func.id()).is_none()
+                && func.visibility().change_set_pk == ChangeSetPk::NONE
+            {
+                let (func_spec, _) = self.export_func(ctx, Some(ChangeSetPk::NONE), func).await?;
+                self.func_map
+                    .insert(ChangeSetPk::NONE, *func.id(), func_spec.to_owned());
+                head_funcs.push(func_spec);
+            } else {
+                let (func_spec, include) = self.export_func(ctx, change_set_pk, func).await?;
+                self.func_map.insert(
+                    change_set_pk.unwrap_or(ChangeSetPk::NONE),
+                    *func.id(),
+                    func_spec.to_owned(),
+                );
+
+                if include {
+                    funcs.push(func_spec);
+                }
+            }
+        }
+
+        Ok((funcs, head_funcs))
+    }
 }
 
 fn remove_duplicate_func_specs(func_specs: &[FuncSpec]) -> Vec<FuncSpec> {

lib/dal/src/pkg/import.rs

@@ -55,7 +55,7 @@ use crate::{
 use super::{PkgError, PkgResult};
 
 #[derive(Clone, Debug)]
-enum Thing {
+pub enum Thing {
     ActionPrototype(ActionPrototype),
     AuthPrototype(AuthenticationPrototype),
     AttributePrototypeArgument(AttributePrototypeArgument),
@@ -68,7 +68,7 @@ enum Thing {
     Socket(Box<(Socket, Option<InternalProvider>, Option<ExternalProvider>)>),
 }
 
-type ThingMap = super::ChangeSetThingMap<String, Thing>;
+pub type ThingMap = super::ChangeSetThingMap<String, Thing>;
 
 #[derive(Clone, Debug, Default)]
 pub struct ImportOptions {
@@ -82,6 +82,8 @@ pub struct ImportOptions {
     pub is_builtin: bool,
 }
 
+const SPECIAL_CASE_FUNCS: [&str; 2] = ["si:resourcePayloadToValue", "si:normalizeToArray"];
+
 #[allow(clippy::too_many_arguments)]
 async fn import_change_set(
     ctx: &DalContext,
@@ -104,9 +106,8 @@ async fn import_change_set(
         // This is a hack because the hash of the intrinsics has changed from the version in the
         // packages. We also apply this to si:resourcePayloadToValue since it should be an
         // intrinsic but is only in our packages
-        let special_case_funcs = ["si:resourcePayloadToValue", "si:normalizeToArray"];
         if func::is_intrinsic(func_spec.name())
-            || special_case_funcs.contains(&func_spec.name())
+            || SPECIAL_CASE_FUNCS.contains(&func_spec.name())
             || func_spec.is_from_builtin().unwrap_or(false)
         {
             let hash = func_spec.hash();
@@ -1485,7 +1486,7 @@ async fn update_func(
     Ok(())
 }
 
-async fn import_func(
+pub async fn import_func(
     ctx: &DalContext,
     change_set_pk: ChangeSetPk,
     func_spec: &FuncSpec,
@@ -2459,7 +2460,29 @@ async fn update_schema_variant(
     Ok(())
 }
 
-async fn import_schema_variant(
+/// Duplicate all the functions, and return a thing_map with them included, so
+/// that we can import a standalone schema variant.
+pub async fn clone_and_import_funcs(ctx: &DalContext, funcs: Vec<FuncSpec>) -> PkgResult<ThingMap> {
+    let mut thing_map = ThingMap::new();
+
+    for func_spec in funcs {
+        let func = if func::is_intrinsic(&func_spec.name)
+            || SPECIAL_CASE_FUNCS.contains(&func_spec.name.as_str())
+        {
+            Func::find_by_name(ctx, &func_spec.name)
+                .await?
+                .ok_or(PkgError::MissingIntrinsicFunc(func_spec.name.to_owned()))?
+        } else {
+            create_func(ctx, &func_spec).await?
+        };
+
+        thing_map.insert(ChangeSetPk::NONE, func_spec.unique_id, Thing::Func(func));
+    }
+
+    Ok(thing_map)
+}
+
+pub async fn import_schema_variant(
     ctx: &DalContext,
     change_set_pk: ChangeSetPk,
     schema: &mut Schema,

lib/dal/src/schema/variant/definition.rs

@@ -499,9 +499,9 @@ impl SchemaVariantDefinitionJson {
         metadata: SchemaVariantDefinitionMetadataJson,
         identity_func_unique_id: &str,
         asset_func_spec_unique_id: &str,
+        name: &str,
     ) -> SchemaVariantDefinitionResult<SchemaVariantSpec> {
         let mut builder = SchemaVariantSpec::builder();
-        let name = "v0";
         builder.name(name);
 
         let mut data_builder = SchemaVariantSpecData::builder();