merge: #3468

3468: Update Components to reflect (un)deletion of Components r=jhelwig a=jhelwig

# Update Component inputs/outputs to reflect when a Component is (un)marked as to_delete.

Data should be able to flow between components following the rules below:

| From Component `to_delete` | To Component `to_delete` | Data Propagates |
| -------------------------- | ------------------------ | --------------- |
| False                      | False                    | Yes             |
| False                      | True                     | Yes             |
| True                       | False                    | No              |
| True                       | True                     | Yes             |

Whenever a Component is marked (or un-marked) as `to_delete`, we need to make sure that both its inputs, and all downstream Components' inputs are updated to reflect the rules above. This allows us to both show the effect of "deleting" a Component on the rest of the workspace, and keep data that may be necessary to perform the delete action in Components that are marked for deletion.

# Enable delete_connection API endpoint

Removing an AttributePrototypeArgument will now cause the AttributeValue(s) that use that Prototype to automatically be updated to reflect the new function inputs.


Co-authored-by: Jacob Helwig <jacob@technosorcery.net>
Co-authored-by: John Obelenus <jobelenus@systeminit.com>

Author: 3 people

app/web/src/components/ModelingView/ModelingRightClickMenu.vue

@@ -85,7 +85,7 @@ const rightClickMenuItems = computed(() => {
     });
 
     // single selected component
-    if (selectedComponent.value.changeStatus === "deleted") {
+    if (selectedComponent.value.toDelete) {
       items.push({
         label: `Restore ${typeDisplayName()} "${
           selectedComponent.value.displayName

app/web/src/store/components.store.ts

@@ -1096,25 +1096,6 @@ export const useComponentsStore = (forceChangeSetId?: ChangeSetId) => {
               },
             });
           },
-          async RESTORE_COMPONENT(componentId: ComponentId) {
-            if (changeSetsStore.creatingChangeSet)
-              throw new Error("race, wait until the change set is created");
-            if (changeSetId === changeSetsStore.headChangeSetId)
-              changeSetsStore.creatingChangeSet = true;
-
-            return new ApiRequest({
-              method: "post",
-              url: "diagram/restore_component",
-              keyRequestStatusBy: componentId,
-              params: {
-                componentId,
-                ...visibilityParams,
-              },
-              onSuccess: (response) => {
-                // this.componentDiffsById[componentId] = response.componentDiff;
-              },
-            });
-          },
 
           async PASTE_COMPONENTS(
             componentIds: ComponentId[],
@@ -1214,7 +1195,7 @@ export const useComponentsStore = (forceChangeSetId?: ChangeSetId) => {
 
             return new ApiRequest({
               method: "post",
-              url: "diagram/restore_components",
+              url: "diagram/remove_delete_intent",
               keyRequestStatusBy: componentIds,
               params: {
                 componentIds,

lib/dal/src/attribute/prototype.rs

@@ -11,6 +11,7 @@
 
 use std::sync::Arc;
 
+use content_node_weight::ContentNodeWeight;
 use petgraph::Direction;
 use serde::{Deserialize, Serialize};
 use si_events::ContentHash;
@@ -27,7 +28,7 @@ use crate::workspace_snapshot::edge_weight::{
     EdgeWeight, EdgeWeightError, EdgeWeightKind, EdgeWeightKindDiscriminants,
 };
 use crate::workspace_snapshot::node_weight::{
-    NodeWeight, NodeWeightDiscriminants, NodeWeightError,
+    content_node_weight, NodeWeight, NodeWeightDiscriminants, NodeWeightError,
 };
 use crate::workspace_snapshot::WorkspaceSnapshotError;
 use crate::{
@@ -50,8 +51,12 @@ pub enum AttributePrototypeError {
     LayerDb(#[from] LayerDbError),
     #[error("attribute prototype {0} is missing a function edge")]
     MissingFunction(AttributePrototypeId),
+    #[error("No attribute values for: {0}")]
+    NoAttributeValues(AttributePrototypeId),
     #[error("node weight error: {0}")]
     NodeWeight(#[from] NodeWeightError),
+    #[error("Attribute Prototype not found: {0}")]
+    NotFound(AttributePrototypeId),
     #[error("transactions error: {0}")]
     Transactions(#[from] TransactionsError),
     #[error("could not acquire lock: {0}")]
@@ -220,6 +225,40 @@ impl AttributePrototype {
         Ok(None)
     }
 
+    pub async fn get_by_id(
+        ctx: &DalContext,
+        prototype_id: AttributePrototypeId,
+    ) -> AttributePrototypeResult<Option<Self>> {
+        let (_node_weight, content) = Self::get_node_weight_and_content(ctx, prototype_id).await?;
+        Ok(Some(Self::assemble(prototype_id, &content)))
+    }
+
+    async fn get_node_weight_and_content(
+        ctx: &DalContext,
+        prototype_id: AttributePrototypeId,
+    ) -> AttributePrototypeResult<(ContentNodeWeight, AttributePrototypeContentV1)> {
+        let content_weight = ctx
+            .workspace_snapshot()?
+            .get_node_weight_by_id(prototype_id)
+            .await?;
+        let prototype_node_weight = content_weight
+            .get_content_node_weight_of_kind(ContentAddressDiscriminants::AttributePrototype)?;
+
+        let content: AttributePrototypeContent = ctx
+            .layer_db()
+            .cas()
+            .try_read_as(&prototype_node_weight.content_hash())
+            .await?
+            .ok_or(WorkspaceSnapshotError::MissingContentFromStore(
+                prototype_id.into(),
+            ))?;
+
+        // Do "upgrading" of the storage format from old versions to the latest here.
+        let AttributePrototypeContent::V1(inner) = content;
+
+        Ok((prototype_node_weight, inner))
+    }
+
     pub async fn update_func_by_id(
         ctx: &DalContext,
         attribute_prototype_id: AttributePrototypeId,

lib/dal/src/attribute/prototype/argument.rs

@@ -3,6 +3,8 @@
 //! value. It defines source of the value for the function argument in the
 //! context of the prototype.
 
+use std::collections::HashSet;
+
 use serde::{Deserialize, Serialize};
 use telemetry::prelude::*;
 use thiserror::Error;
@@ -22,8 +24,8 @@ use crate::{
         },
         WorkspaceSnapshotError,
     },
-    AttributePrototype, AttributePrototypeId, ComponentId, DalContext, OutputSocketId, PropId,
-    Timestamp, TransactionsError,
+    AttributePrototype, AttributePrototypeId, AttributeValue, ComponentId, DalContext,
+    OutputSocketId, PropId, Timestamp, TransactionsError,
 };
 
 use self::{
@@ -45,6 +47,8 @@ pk!(AttributePrototypeArgumentId);
 pub enum AttributePrototypeArgumentError {
     #[error("attribute prototype error: {0}")]
     AttributePrototype(#[from] AttributePrototypeError),
+    #[error("attribute value error: {0}")]
+    AttributeValue(String),
     #[error("change set error: {0}")]
     ChangeSet(#[from] ChangeSetPointerError),
     #[error("edge weight error: {0}")]
@@ -59,8 +63,12 @@ pub enum AttributePrototypeArgumentError {
     LayerDb(#[from] si_layer_cache::LayerDbError),
     #[error("attribute prototype argument {0} has no func argument")]
     MissingFuncArgument(AttributePrototypeArgumentId),
+    #[error("attribute prototype argument {0} has no value source")]
+    MissingSource(AttributePrototypeArgumentId),
     #[error("node weight error: {0}")]
     NodeWeight(#[from] NodeWeightError),
+    #[error("no targets for prototype argument: {0}")]
+    NoTargets(AttributePrototypeArgumentId),
     #[error("serde json error: {0}")]
     Serde(#[from] serde_json::Error),
     #[error("transactions error: {0}")]
@@ -525,9 +533,37 @@ impl AttributePrototypeArgument {
         ctx: &DalContext,
         apa_id: AttributePrototypeArgumentId,
     ) -> AttributePrototypeArgumentResult<()> {
+        let apa = Self::get_by_id(ctx, apa_id).await?;
+        let prototype_id = apa.prototype_id(ctx).await?;
+        // Find all of the "destination" attribute values.
+        let mut avs_to_update = AttributePrototype::attribute_value_ids(ctx, prototype_id).await?;
+        // If the argument has targets, then we only care about AVs that are for the same
+        // destination component.
+        if let Some(targets) = apa.targets() {
+            let mut av_ids_to_keep = HashSet::new();
+            for av_id in &avs_to_update {
+                let component_id = AttributeValue::component_id(ctx, *av_id)
+                    .await
+                    .map_err(|e| AttributePrototypeArgumentError::AttributeValue(e.to_string()))?;
+                if component_id == targets.destination_component_id {
+                    av_ids_to_keep.insert(*av_id);
+                }
+            }
+            avs_to_update.retain(|av_id| av_ids_to_keep.contains(av_id));
+        }
+
+        // Remove the argument
         ctx.workspace_snapshot()?
             .remove_node_by_id(ctx.change_set_pointer()?, apa_id)
             .await?;
+        // Update the destination attribute values
+        for av_id_to_update in &avs_to_update {
+            AttributeValue::update_from_prototype_function(ctx, *av_id_to_update)
+                .await
+                .map_err(|e| AttributePrototypeArgumentError::AttributeValue(e.to_string()))?;
+        }
+        // Enqueue a dependent values update with the destination attribute values
+        ctx.enqueue_dependent_values_update(avs_to_update).await?;
 
         Ok(())
     }

lib/dal/src/component.rs

@@ -1199,30 +1199,33 @@ impl Component {
     where
         L: FnOnce(&mut Self) -> ComponentResult<()>,
     {
+        let original_component = self.clone();
         let mut component = self;
 
         let before = ComponentContentV1::from(component.clone());
         lambda(&mut component)?;
 
-        let workspace_snapshot = ctx.workspace_snapshot()?;
-        let component_idx = workspace_snapshot
-            .get_node_index_by_id(component.id())
-            .await?;
-        let component_node_weight = workspace_snapshot
-            .get_node_weight(component_idx)
-            .await?
-            .get_component_node_weight()?;
-
         // The `to_delete` lives on the node itself, not in the content, so we need to be a little
         // more manual when updating that field.
-        if component.to_delete != component_node_weight.to_delete() {
+        if component.to_delete != original_component.to_delete {
+            let component_idx = ctx
+                .workspace_snapshot()?
+                .get_node_index_by_id(original_component.id)
+                .await?;
+            let component_node_weight = ctx
+                .workspace_snapshot()?
+                .get_node_weight(component_idx)
+                .await?
+                .get_component_node_weight()?;
             let mut new_component_node_weight = component_node_weight
                 .new_with_incremented_vector_clock(ctx.change_set_pointer()?)?;
             new_component_node_weight.set_to_delete(component.to_delete);
-            workspace_snapshot
+            ctx.workspace_snapshot()?
                 .add_node(NodeWeight::Component(new_component_node_weight))
                 .await?;
-            workspace_snapshot.replace_references(component_idx).await?;
+            ctx.workspace_snapshot()?
+                .replace_references(component_idx)
+                .await?;
         }
 
         let updated = ComponentContentV1::from(component.clone());
@@ -1237,31 +1240,105 @@ impl Component {
                     ctx.events_actor(),
                 )
                 .await?;
-            workspace_snapshot
+            ctx.workspace_snapshot()?
                 .update_content(ctx.change_set_pointer()?, component.id.into(), hash)
                 .await?;
         }
 
+        let component_node_weight = ctx
+            .workspace_snapshot()?
+            .get_node_weight_by_id(original_component.id)
+            .await?
+            .get_component_node_weight()?;
+
         Ok(Component::assemble(&component_node_weight, updated))
     }
 
     pub async fn delete(self, ctx: &DalContext) -> ComponentResult<Self> {
-        self.modify(ctx, |component| {
-            component.to_delete = true;
-            Ok(())
-        })
-        .await
-
-        // TODO: Trigger DependentValuesUpdate for all Components that get data from this
-        // Component.
+        self.set_to_delete(ctx, true).await
     }
 
     pub async fn set_to_delete(self, ctx: &DalContext, to_delete: bool) -> ComponentResult<Self> {
-        self.modify(ctx, |component| {
-            component.to_delete = to_delete;
-            Ok(())
-        })
-        .await
+        let modified = self
+            .modify(ctx, |component| {
+                component.to_delete = to_delete;
+                Ok(())
+            })
+            .await?;
+
+        // If we're clearing the `to_delete` flag, we need to make sure our inputs are updated
+        // appropriately, as we may have an input connected to a still `to_delete` component, and
+        // we should not be using it for input as long as it's still marked `to_delete`.
+        //
+        // If we're setting the `to_delete` flag, then we may need to pick up inputs from other
+        // `to_delete` Components that we were ignoring before.
+        //
+        // This will update more than is strictly necessary, but it will ensure that everything is
+        // correct.
+        let input_av_ids: Vec<AttributeValueId> = modified
+            .input_socket_attribute_values(ctx)
+            .await?
+            .values()
+            .copied()
+            .collect();
+        for av_id in &input_av_ids {
+            AttributeValue::update_from_prototype_function(ctx, *av_id).await?;
+        }
+        ctx.enqueue_dependent_values_update(input_av_ids).await?;
+
+        // We always want to make sure that everything "downstream" of us reacts appropriately
+        // regardless of whether we're setting, or clearing the `to_delete` flag.
+        //
+        // We can't use self.output_socket_attribute_values here, and just enqueue a dependent
+        // values update for those IDs, as the DVU explicitly *does not* update a not-to_delete AV,
+        // using a source from a to_delete AV, and we want the not-to_delete AVs to be updated to
+        // reflect that they're not getting data from this to_delete Component any more.
+        let downstream_av_ids = modified.downstream_attribute_value_ids(ctx).await?;
+        for av_id in &downstream_av_ids {
+            AttributeValue::update_from_prototype_function(ctx, *av_id).await?;
+        }
+        ctx.enqueue_dependent_values_update(downstream_av_ids)
+            .await?;
+
+        Ok(modified)
+    }
+
+    /// `AttributeValueId`s of all input sockets connected to any output socket of this component.
+    async fn downstream_attribute_value_ids(
+        &self,
+        ctx: &DalContext,
+    ) -> ComponentResult<Vec<AttributeValueId>> {
+        let mut results = Vec::new();
+
+        let output_sockets: Vec<OutputSocketId> = self
+            .output_socket_attribute_values(ctx)
+            .await?
+            .keys()
+            .copied()
+            .collect();
+        for output_socket_id in output_sockets {
+            let output_socket = OutputSocket::get_by_id(ctx, output_socket_id).await?;
+            for argument_using_id in output_socket.prototype_arguments_using(ctx).await? {
+                let argument_using =
+                    AttributePrototypeArgument::get_by_id(ctx, argument_using_id).await?;
+                if let Some(targets) = argument_using.targets() {
+                    if targets.source_component_id == self.id() {
+                        let prototype_id = argument_using.prototype_id(ctx).await?;
+                        for maybe_downstream_av_id in
+                            AttributePrototype::attribute_value_ids(ctx, prototype_id).await?
+                        {
+                            if AttributeValue::component_id(ctx, maybe_downstream_av_id).await?
+                                == targets.destination_component_id
+                            {
+                                results.push(maybe_downstream_av_id);
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
+        Ok(results)
     }
 }