Skip to content

Commit

Permalink
Merge pull request #5170 from systeminit/jhelwig/bug-722-unable-to-de…
Browse files Browse the repository at this point in the history
…lete-views-that-have-components-that-are-also-in

Allow removal of Views contining Components also in other Views when applying a set of Updates
  • Loading branch information
jhelwig authored Dec 19, 2024
2 parents 522958b + 6b36854 commit d58215a
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 1 deletion.
34 changes: 33 additions & 1 deletion lib/dal/src/workspace_snapshot/graph/v4/component.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use petgraph::prelude::*;
use si_id::ViewId;

use crate::{
component::ComponentResult,
workspace_snapshot::{
edge_weight::EdgeWeightKindDiscriminants, graph::WorkspaceSnapshotGraphV4,
edge_weight::EdgeWeightKindDiscriminants,
graph::{WorkspaceSnapshotGraphResult, WorkspaceSnapshotGraphV4},
node_weight::NodeWeightDiscriminants,
},
ComponentError, ComponentId, SchemaVariantId, WorkspaceSnapshotError,
Expand Down Expand Up @@ -63,4 +65,34 @@ impl WorkspaceSnapshotGraphV4 {

Err(ComponentError::SchemaVariantNotFound(component_id))
}

pub fn component_contained_in_views(
&self,
component_id: ComponentId,
) -> WorkspaceSnapshotGraphResult<Vec<ViewId>> {
// Component <--Represents-- Geometry <--Use-- View
let component_node_index = *self
.node_index_by_id
.get(&component_id.into())
.ok_or(ComponentError::NotFound(component_id))
.map_err(Box::new)?;
let mut results = Vec::new();
for (_edge_weight, geometry_node_index, _component_node_index) in self
.edges_directed_for_edge_weight_kind(
component_node_index,
Incoming,
EdgeWeightKindDiscriminants::Represents,
)
{
let view_node_index = self.get_edge_weight_kind_target_idx(
geometry_node_index,
Incoming,
EdgeWeightKindDiscriminants::Use,
)?;
let view_id = self.get_node_weight(view_node_index)?.id();
results.push(view_id.into());
}

Ok(results)
}
}
12 changes: 12 additions & 0 deletions lib/dal/src/workspace_snapshot/node_weight/view_node_weight/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use dal_macros::SiNodeWeight;
use jwt_simple::prelude::{Deserialize, Serialize};
use petgraph::prelude::*;
use si_events::{merkle_tree_hash::MerkleTreeHash, ulid::Ulid, ContentHash};
use si_id::ViewId;

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, SiNodeWeight)]
#[si_node_weight(discriminant = NodeWeightDiscriminants::View)]
Expand Down Expand Up @@ -136,6 +137,17 @@ impl CorrectTransforms for ViewNodeWeightV1 {
// Geometry, so we need to check if the Component itself is being removed.
continue;
}

// If the Component isn't being removed, we need to make sure that it still
// exists in at least one other View to make sure we're not orphaning it.
let mut appears_in_views = workspace_snapshot_graph
.component_contained_in_views(component.id().into())?;
let my_id: ViewId = self.id().into();
appears_in_views.retain(|&view_id| my_id != view_id);

if !appears_in_views.is_empty() {
continue;
}
}

updates.remove(view_removal_update_idx);
Expand Down
65 changes: 65 additions & 0 deletions lib/dal/tests/integration_test/node_weight/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use dal_test::{
test,
};
use pretty_assertions_sorted::assert_eq;
use si_frontend_types::RawGeometry;

#[test]
async fn correct_transforms_remove_view_all_geometries_removed(ctx: &mut DalContext) {
Expand Down Expand Up @@ -244,3 +245,67 @@ async fn correct_transforms_remove_view_no_other_views(ctx: &mut DalContext) {
.len(),
);
}

#[test]
async fn correct_transforms_remove_view_with_component_in_another_view(ctx: &mut DalContext) {
let default_view_id = ExpectView::get_id_for_default(ctx).await;
let new_view = ExpectView::create(ctx).await;
let component = create_component_for_default_schema_name(
ctx,
"swifty",
generate_fake_name(),
default_view_id,
)
.await
.expect("Unable to create Component in default View");
Component::add_to_view(ctx, component.id(), new_view.id(), RawGeometry::default())
.await
.expect("Unable to add Component to new View");
expected::commit_and_update_snapshot_to_visibility(ctx).await;
expected::apply_change_set_to_base(ctx).await;

assert_eq!(
2,
View::list(ctx).await.expect("Unable to list Views").len(),
);
assert_eq!(
2,
Geometry::list_ids_by_component(ctx, component.id())
.await
.expect("Unable to get Geometries for Component")
.len()
);

expected::fork_from_head_change_set(ctx).await;

View::remove(ctx, new_view.id())
.await
.expect("Unable to remove new View");
expected::commit_and_update_snapshot_to_visibility(ctx).await;

assert_eq!(
1,
View::list(ctx).await.expect("Unable to list Views").len(),
);
assert_eq!(
1,
Geometry::list_ids_by_component(ctx, component.id())
.await
.expect("Unable to get Geometries for Component")
.len()
);

expected::apply_change_set_to_base(ctx).await;

assert_eq!(
1,
View::list(ctx).await.expect("Unable to list Views").len(),
);
assert_eq!(
1,
Geometry::list_ids_by_component(ctx, component.id())
.await
.expect("Unable to get Geometries for Component")
.len()
);
}

0 comments on commit d58215a

Please sign in to comment.