diff --git a/lib/dal/src/approval_requirement.rs b/lib/dal/src/approval_requirement.rs new file mode 100644 index 0000000000..9ba3a625f4 --- /dev/null +++ b/lib/dal/src/approval_requirement.rs @@ -0,0 +1,144 @@ +//! This module provides functionality for creating, updating, deleting, getting and listing +//! approval requirements when applying a change set. + +#![warn( + bad_style, + clippy::missing_panics_doc, + clippy::panic, + clippy::panic_in_result_fn, + clippy::unwrap_in_result, + clippy::unwrap_used, + dead_code, + improper_ctypes, + missing_debug_implementations, + // TODO(nick): restore this and clean up the mess. + // missing_docs, + no_mangle_generic_items, + non_shorthand_field_patterns, + overflowing_literals, + path_statements, + patterns_in_fns_without_body, + unconditional_recursion, + unreachable_pub, + unused, + unused_allocation, + unused_comparisons, + unused_parens, + while_true +)] + +use std::collections::{HashMap, HashSet}; + +use si_events::merkle_tree_hash::MerkleTreeHash; +use si_id::{ulid::Ulid, ApprovalRequirementDefinitionId, EntityId, UserPk}; +use telemetry::prelude::*; +use thiserror::Error; + +use crate::{ + workspace_snapshot::{ + graph::detector::Change, traits::approval_requirement::ApprovalRequirementExt, + }, + DalContext, WorkspaceSnapshotError, +}; + +pub use crate::workspace_snapshot::traits::approval_requirement::{ + ApprovalRequirementApprover, ApprovalRequirementRule, +}; + +#[allow(missing_docs)] +#[derive(Debug, Error)] +pub enum ApprovalRequirementError { + #[error("workspace snapshot error: {0}")] + WorkspaceSnapshot(#[from] WorkspaceSnapshotError), +} + +type Result = std::result::Result; + +#[derive(Debug)] +pub struct ApprovalRequirementExplicit { + pub id: ApprovalRequirementDefinitionId, + pub rule: ApprovalRequirementRule, +} + +#[derive(Debug)] +pub enum ApprovalRequirement { + Explicit(ApprovalRequirementExplicit), + Virtual(ApprovalRequirementRule), +} + +impl ApprovalRequirement { + #[instrument( + name = "approval_requirement.new_definition", + level = "debug", + skip_all + )] + pub async fn new_definition( + ctx: &DalContext, + entity_id: impl Into, + minimum_approvers_count: usize, + approvers: HashSet, + ) -> Result { + ctx.workspace_snapshot()? + .new_definition(ctx, entity_id.into(), minimum_approvers_count, approvers) + .await + .map_err(Into::into) + } + + #[instrument( + name = "approval_requirement.remove_definition", + level = "debug", + skip_all + )] + pub async fn remove_definition( + ctx: &DalContext, + id: ApprovalRequirementDefinitionId, + ) -> Result<()> { + ctx.workspace_snapshot()? + .remove_definition(id) + .await + .map_err(Into::into) + } + + #[instrument( + name = "approval_requirement.add_individual_approver_for_definition", + level = "debug", + skip_all + )] + pub async fn add_individual_approver_for_definition( + ctx: &DalContext, + id: ApprovalRequirementDefinitionId, + user_id: UserPk, + ) -> Result<()> { + ctx.workspace_snapshot()? + .add_individual_approver_for_definition(ctx, id, user_id) + .await + .map_err(Into::into) + } + + #[instrument( + name = "approval_requirement.remove_individual_approver_for_definition", + level = "debug", + skip_all + )] + pub async fn remove_individual_approver_for_definition( + ctx: &DalContext, + id: ApprovalRequirementDefinitionId, + user_id: UserPk, + ) -> Result<()> { + ctx.workspace_snapshot()? + .remove_individual_approver_for_definition(ctx, id, user_id) + .await + .map_err(Into::into) + } + + #[instrument(name = "approval_requirement.list", level = "debug", skip_all)] + pub async fn list( + ctx: &DalContext, + changes: &[Change], + ) -> Result<(Vec, HashMap)> { + ctx.workspace_snapshot()? + .approval_requirements_for_changes(ctx, changes) + .await + .map_err(Into::into) + } +} diff --git a/lib/dal/src/change_set/approval.rs b/lib/dal/src/change_set/approval.rs index e0c4da3587..7d577822ba 100644 --- a/lib/dal/src/change_set/approval.rs +++ b/lib/dal/src/change_set/approval.rs @@ -28,6 +28,7 @@ use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use si_data_pg::{PgError, PgRow}; +use si_events::merkle_tree_hash::MerkleTreeHash; use si_id::{ChangeSetApprovalId, ChangeSetId, EntityId, UserPk}; use telemetry::prelude::*; use thiserror::Error; @@ -88,11 +89,11 @@ impl ChangeSetApproval { pub async fn new( ctx: &DalContext, status: ChangeSetApprovalStatus, - ids: Vec, + ids_with_hashes: Vec<(EntityId, MerkleTreeHash)>, ) -> Result { let checksum = ctx .workspace_snapshot()? - .calculate_checksum(ctx, ids) + .calculate_checksum(ctx, ids_with_hashes) .await?; let change_set_id = ctx.change_set_id(); diff --git a/lib/dal/src/diagram.rs b/lib/dal/src/diagram.rs index 60cf3e47fd..f1cf9dfba4 100644 --- a/lib/dal/src/diagram.rs +++ b/lib/dal/src/diagram.rs @@ -13,6 +13,7 @@ use std::{ use telemetry::prelude::*; use thiserror::Error; +use crate::approval_requirement::ApprovalRequirementError; use crate::workspace_snapshot::node_weight::NodeWeight; use crate::{ attribute::{ @@ -46,6 +47,8 @@ use si_layer_cache::LayerDbError; #[remain::sorted] #[derive(Error, Debug)] pub enum DiagramError { + #[error("approval requirement error: {0}")] + ApprovalRequirement(#[from] ApprovalRequirementError), #[error("attribute prototype argument error: {0}")] AttributePrototypeArgument(#[from] AttributePrototypeArgumentError), #[error("attribute prototype not found")] diff --git a/lib/dal/src/diagram/geometry.rs b/lib/dal/src/diagram/geometry.rs index a25291a767..2b8c4d2823 100644 --- a/lib/dal/src/diagram/geometry.rs +++ b/lib/dal/src/diagram/geometry.rs @@ -220,7 +220,8 @@ impl Geometry { | NodeWeight::SchemaVariant(_) | NodeWeight::ManagementPrototype(_) | NodeWeight::Geometry(_) - | NodeWeight::View(_) => { + | NodeWeight::View(_) + | NodeWeight::ApprovalRequirementDefinition(_) => { return Err(DiagramError::GeometryCannotRepresentNodeWeight( node_weight.into(), )) diff --git a/lib/dal/src/layer_db_types/content_types.rs b/lib/dal/src/layer_db_types/content_types.rs index e09020890b..f09d400f4d 100644 --- a/lib/dal/src/layer_db_types/content_types.rs +++ b/lib/dal/src/layer_db_types/content_types.rs @@ -8,6 +8,7 @@ use strum::EnumDiscriminants; use thiserror::Error; use crate::action::prototype::ActionKind; +use crate::approval_requirement::ApprovalRequirementApprover; use crate::validation::ValidationStatus; use crate::{ action::ActionCompletionStatus, func::argument::FuncArgumentKind, prop::WidgetOptions, @@ -60,6 +61,7 @@ pub enum ContentTypes { ManagementPrototype(ManagementPrototypeContent), Geometry(GeometryContent), View(ViewContent), + ApprovalRequirementDefinition(ApprovalRequirementDefinitionContent), } macro_rules! impl_into_content_types { @@ -117,6 +119,7 @@ impl_into_content_types!(Validation); impl_into_content_types!(ManagementPrototype); impl_into_content_types!(Geometry); impl_into_content_types!(View); +impl_into_content_types!(ApprovalRequirementDefinition); // Here we've broken the Foo, FooContent convention so we need to implement // these traits manually @@ -683,3 +686,14 @@ pub struct ManagementPrototypeContentV1 { pub managed_schemas: Option>, pub description: Option, } + +#[derive(Debug, Clone, EnumDiscriminants, Serialize, Deserialize, PartialEq)] +pub enum ApprovalRequirementDefinitionContent { + V1(ApprovalRequirementDefinitionContentV1), +} + +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] +pub struct ApprovalRequirementDefinitionContentV1 { + pub minimum: usize, + pub approvers: HashSet, +} diff --git a/lib/dal/src/lib.rs b/lib/dal/src/lib.rs index d7ae03d8c6..e1c7e3c079 100644 --- a/lib/dal/src/lib.rs +++ b/lib/dal/src/lib.rs @@ -21,6 +21,7 @@ use tokio::time::Instant; pub mod action; pub mod actor_view; +pub mod approval_requirement; pub mod attribute; pub mod audit_logging; pub mod authentication_prototype; diff --git a/lib/dal/src/schema/variant.rs b/lib/dal/src/schema/variant.rs index cf257b4e6d..ec72d87197 100644 --- a/lib/dal/src/schema/variant.rs +++ b/lib/dal/src/schema/variant.rs @@ -2296,7 +2296,8 @@ impl SchemaVariant { | EdgeWeightKindDiscriminants::SocketValue | EdgeWeightKindDiscriminants::ValidationOutput | EdgeWeightKindDiscriminants::Manages - | EdgeWeightKindDiscriminants::DiagramObject => {} + | EdgeWeightKindDiscriminants::DiagramObject + | EdgeWeightKindDiscriminants::HasApprovalRequirement => {} } } diff --git a/lib/dal/src/workspace_snapshot.rs b/lib/dal/src/workspace_snapshot.rs index 4d1ed559d4..73a988a27f 100644 --- a/lib/dal/src/workspace_snapshot.rs +++ b/lib/dal/src/workspace_snapshot.rs @@ -7,7 +7,6 @@ use std::collections::{HashMap, HashSet}; use std::sync::atomic::AtomicBool; use std::sync::Arc; -use graph::approval::ApprovalRequirement; use graph::correct_transforms::correct_transforms; use graph::detector::{Change, Update}; use graph::{RebaseBatch, WorkspaceSnapshotGraph}; @@ -15,9 +14,10 @@ use node_weight::traits::CorrectTransformsError; use petgraph::prelude::*; use serde::{Deserialize, Serialize}; use si_data_pg::PgError; +use si_events::merkle_tree_hash::MerkleTreeHash; use si_events::workspace_snapshot::Checksum; use si_events::{ulid::Ulid, ContentHash, WorkspaceSnapshotAddress}; -use si_id::{EntityId, WorkspacePk}; +use si_id::{ApprovalRequirementDefinitionId, EntityId}; use si_layer_cache::LayerDbError; use telemetry::prelude::*; use thiserror::Error; @@ -112,6 +112,10 @@ pub enum WorkspaceSnapshotError { Join(#[from] JoinError), #[error("layer db error: {0}")] LayerDb(#[from] si_layer_cache::LayerDbError), + #[error( + "missing content from content map for hash ({0}) and approval requirement definition ({1})" + )] + MissingContentFromContentMap(ContentHash, ApprovalRequirementDefinitionId), #[error("missing content from store for id: {0}")] MissingContentFromStore(Ulid), #[error("could not find a max vector clock for change set id {0}")] @@ -727,7 +731,7 @@ impl WorkspaceSnapshot { .await .detect_changes(&*onto_clone.working_copy().await) })? - .await?) + .await??) } /// A wrapper around [`Self::detect_changes`](Self::detect_changes) where the "onto" snapshot is derived from the @@ -748,24 +752,7 @@ impl WorkspaceSnapshot { .await } - /// Collects all the [`ApprovalRequirements`](ApprovalRequirement) based on the changes passed in. - #[instrument( - name = "workspace_snapshot.approval_requirements_for_changes", - level = "debug", - skip_all - )] - pub async fn approval_requirements_for_changes( - &self, - workspace_id: WorkspacePk, - changes: &[Change], - ) -> WorkspaceSnapshotResult> { - Ok(self - .working_copy() - .await - .approval_requirements_for_changes(workspace_id, changes)?) - } - - /// Calculates the checksum based on a list of IDs passed in. + /// Calculates the checksum based on a list of IDs with hashes passed in. #[instrument( name = "workspace_snapshot.calculate_checksum", level = "debug", @@ -774,35 +761,31 @@ impl WorkspaceSnapshot { pub async fn calculate_checksum( &self, ctx: &DalContext, - mut ids: Vec, + mut ids_with_hashes: Vec<(EntityId, MerkleTreeHash)>, ) -> WorkspaceSnapshotResult { - // If an empty list of IDs were passed in, then we use the root node's ID as our sole ID so - // that algorithms using the checksum can "invalidate" as needed. - if ids.is_empty() { + // If an empty list of IDs with hashes wass passed in, then we use the root node's ID and + // merkle tree hash as our sole ID and hash so that algorithms using the checksum can + // "invalidate" as needed. + if ids_with_hashes.is_empty() { let root_node_index = ctx.workspace_snapshot()?.root().await?; - ids.push( - ctx.workspace_snapshot()? - .get_node_weight(root_node_index) - .await? - .id() - .into(), - ); + let root_node_weight = ctx + .workspace_snapshot()? + .get_node_weight(root_node_index) + .await?; + ids_with_hashes.push(( + root_node_weight.id().into(), + root_node_weight.merkle_tree_hash(), + )); } // We MUST sort IDs before creating the checksum. - ids.sort(); + ids_with_hashes.sort_by_key(|(_, hash)| *hash); - // Now that we have strictly ordered IDs and there's at least one present, we can create - // the checksum. - let snapshot = self.working_copy().await; + // Now that we have strictly ordered IDs with hasesh and there's at least one group + // present, we can create the checksum. let mut hasher = Checksum::hasher(); - for id in ids { - hasher.update( - snapshot - .get_node_weight_by_id(id)? - .merkle_tree_hash() - .as_bytes(), - ); + for (_, hash) in ids_with_hashes { + hasher.update(hash.as_bytes()); } Ok(hasher.finalize()) } @@ -1678,6 +1661,7 @@ impl WorkspaceSnapshot { } ContentAddressDiscriminants::ActionPrototype + | ContentAddressDiscriminants::ApprovalRequirementDefinition | ContentAddressDiscriminants::Component | ContentAddressDiscriminants::DeprecatedAction | ContentAddressDiscriminants::DeprecatedActionBatch @@ -1687,6 +1671,7 @@ impl WorkspaceSnapshot { | ContentAddressDiscriminants::Geometry | ContentAddressDiscriminants::InputSocket | ContentAddressDiscriminants::JsonValue + | ContentAddressDiscriminants::ManagementPrototype | ContentAddressDiscriminants::Module | ContentAddressDiscriminants::OutputSocket | ContentAddressDiscriminants::Prop @@ -1695,12 +1680,12 @@ impl WorkspaceSnapshot { | ContentAddressDiscriminants::SchemaVariant | ContentAddressDiscriminants::Secret | ContentAddressDiscriminants::ValidationPrototype - | ContentAddressDiscriminants::View - | ContentAddressDiscriminants::ManagementPrototype => None, + | ContentAddressDiscriminants::View => None, }, NodeWeight::Action(_) | NodeWeight::ActionPrototype(_) + | NodeWeight::ApprovalRequirementDefinition(_) | NodeWeight::Category(_) | NodeWeight::Component(_) | NodeWeight::DependentValueRoot(_) @@ -1709,12 +1694,12 @@ impl WorkspaceSnapshot { | NodeWeight::Func(_) | NodeWeight::FuncArgument(_) | NodeWeight::Geometry(_) - | NodeWeight::View(_) | NodeWeight::InputSocket(_) + | NodeWeight::ManagementPrototype(_) | NodeWeight::Prop(_) | NodeWeight::SchemaVariant(_) - | NodeWeight::ManagementPrototype(_) - | NodeWeight::Secret(_) => None, + | NodeWeight::Secret(_) + | NodeWeight::View(_) => None, } { let next_node_idxs = self .incoming_sources_for_edge_weight_kind(this_node_weight.id(), edge_kind) diff --git a/lib/dal/src/workspace_snapshot/content_address.rs b/lib/dal/src/workspace_snapshot/content_address.rs index d1c0cde0c2..ef8e8c38f8 100644 --- a/lib/dal/src/workspace_snapshot/content_address.rs +++ b/lib/dal/src/workspace_snapshot/content_address.rs @@ -38,6 +38,7 @@ pub enum ContentAddress { ManagementPrototype(ContentHash), Geometry(ContentHash), View(ContentHash), + ApprovalRequirementDefinition(ContentHash), } impl ContentAddress { @@ -65,7 +66,8 @@ impl ContentAddress { | ContentAddress::ValidationPrototype(id) | ContentAddress::ValidationOutput(id) | ContentAddress::View(id) - | ContentAddress::ManagementPrototype(id) => Some(*id), + | ContentAddress::ManagementPrototype(id) + | ContentAddress::ApprovalRequirementDefinition(id) => Some(*id), } .unwrap_or_default() } diff --git a/lib/dal/src/workspace_snapshot/edge_weight.rs b/lib/dal/src/workspace_snapshot/edge_weight.rs index d2ca81a4ea..6cdd17c3d2 100644 --- a/lib/dal/src/workspace_snapshot/edge_weight.rs +++ b/lib/dal/src/workspace_snapshot/edge_weight.rs @@ -69,6 +69,8 @@ pub enum EdgeWeightKind { Manages, /// From a view node to a diagram object node, to which geometries can be connected. DiagramObject, + /// Indicates if the node has a corresponding approval requirement. + HasApprovalRequirement, } impl EdgeWeightKind { diff --git a/lib/dal/src/workspace_snapshot/graph.rs b/lib/dal/src/workspace_snapshot/graph.rs index ea3489dd0f..3b608d5a0a 100644 --- a/lib/dal/src/workspace_snapshot/graph.rs +++ b/lib/dal/src/workspace_snapshot/graph.rs @@ -5,6 +5,7 @@ use detector::Update; use petgraph::prelude::*; use serde::{Deserialize, Serialize}; use si_events::{merkle_tree_hash::MerkleTreeHash, ulid::Ulid}; +use si_id::EntityId; use si_layer_cache::db::serialize; use si_layer_cache::LayerDbError; use strum::{EnumDiscriminants, EnumIter, EnumString, IntoEnumIterator}; @@ -20,7 +21,6 @@ use crate::{ ComponentError, EdgeWeightKindDiscriminants, SchemaVariantError, }; -pub mod approval; pub mod correct_transforms; pub mod deprecated; pub mod detector; @@ -30,7 +30,10 @@ pub mod v2; pub mod v3; pub mod v4; -pub use traits::{schema::variant::SchemaVariantExt, socket::input::InputSocketExt}; +pub use traits::{ + approval_requirement::ApprovalRequirementExt, schema::variant::SchemaVariantExt, + socket::input::InputSocketExt, +}; pub use v2::WorkspaceSnapshotGraphV2; pub use v3::WorkspaceSnapshotGraphV3; pub use v4::WorkspaceSnapshotGraphV4; @@ -70,6 +73,8 @@ pub enum WorkspaceSnapshotGraphError { LayerDb(#[from] LayerDbError), #[error("monotonic error: {0}")] Monotonic(#[from] ulid::MonotonicError), + #[error("multiple merkle tree hashes found for entity {0} (at least two found, including {1} and {2})")] + MultipleMerkleTreeHashesForEntity(EntityId, MerkleTreeHash, MerkleTreeHash), #[error("mutex poisoning: {0}")] MutexPoison(String), #[error("NodeWeight error: {0}")] diff --git a/lib/dal/src/workspace_snapshot/graph/approval.rs b/lib/dal/src/workspace_snapshot/graph/approval.rs deleted file mode 100644 index 402c54549c..0000000000 --- a/lib/dal/src/workspace_snapshot/graph/approval.rs +++ /dev/null @@ -1,19 +0,0 @@ -//! This module contains graph-specific functionality related to approvals. - -use si_events::workspace_snapshot::EntityKind; -use si_id::EntityId; - -#[derive(Debug)] -pub struct ApprovalRequirement { - pub entity_id: EntityId, - pub entity_kind: EntityKind, - pub number: usize, - pub lookup_groups: Vec, -} - -#[derive(Debug, Hash, PartialEq, Eq)] -pub struct ApprovalRequirementLookupGroup { - pub object_type: String, - pub object_id: String, - pub permission: String, -} diff --git a/lib/dal/src/workspace_snapshot/graph/detector.rs b/lib/dal/src/workspace_snapshot/graph/detector.rs index 255096c494..c39c09607c 100644 --- a/lib/dal/src/workspace_snapshot/graph/detector.rs +++ b/lib/dal/src/workspace_snapshot/graph/detector.rs @@ -5,16 +5,20 @@ use petgraph::{ visit::{Control, DfsEvent}, }; use serde::{Deserialize, Serialize}; -use si_events::{merkle_tree_hash::MerkleTreeHash, ulid::Ulid}; +use si_events::{merkle_tree_hash::MerkleTreeHash, ulid::Ulid, workspace_snapshot::EntityKind}; +use si_id::EntityId; use strum::EnumDiscriminants; -use telemetry::prelude::*; use crate::{ workspace_snapshot::{node_weight::NodeWeight, NodeInformation}, EdgeWeight, EdgeWeightKind, EdgeWeightKindDiscriminants, }; -use super::WorkspaceSnapshotGraphVCurrent; +use super::{ + traits::entity_kind::EntityKindExt, WorkspaceSnapshotGraphError, WorkspaceSnapshotGraphVCurrent, +}; + +type Result = std::result::Result; #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, EnumDiscriminants)] pub enum Update { @@ -38,7 +42,8 @@ pub enum Update { #[derive(Debug)] pub struct Change { - pub id: Ulid, + pub entity_id: EntityId, + pub entity_kind: EntityKind, pub merkle_tree_hash: MerkleTreeHash, } @@ -88,16 +93,34 @@ impl<'a, 'b> Detector<'a, 'b> { /// /// This assumes that all graphs involved to not have any "garbage" laying around. If in doubt, perform "cleanup" /// on both graphs before creating the [`Detector`]. - pub fn detect_changes(&self) -> Vec { + pub fn detect_changes(&self) -> Result> { let mut changes = Vec::new(); petgraph::visit::depth_first_search( self.updated_graph.graph(), Some(self.updated_graph.root()), |event| self.calculate_changes_dfs_event(event, &mut changes), - ); + )?; + + // Add changes for IDs that only exist in the base graph because our DFS walk of the + // updated graph will not include them. + for base_graph_id in self + .base_graph + .all_node_ids() + .difference(&self.updated_graph.all_node_ids()) + { + let entity_id = base_graph_id.inner().into(); + changes.push(Change { + entity_id, + entity_kind: self.base_graph.get_entity_kind_for_id(entity_id)?, + merkle_tree_hash: self + .base_graph + .get_node_weight_by_id(entity_id)? + .merkle_tree_hash(), + }) + } - changes + Ok(changes) } fn node_diff_from_base_graph( @@ -375,26 +398,29 @@ impl<'a, 'b> Detector<'a, 'b> { &self, event: DfsEvent, changes: &mut Vec, - ) -> Control<()> { + ) -> Result> { if let DfsEvent::Discover(updated_graph_index, _) = event { - match self.updated_graph.get_node_weight(updated_graph_index) { - Ok(updated_node_weight) => { - if let Some(original_node_weight) = self.base_graph.get_node_weight_by_id_opt(updated_node_weight.id()) { - if original_node_weight.merkle_tree_hash() == updated_node_weight.merkle_tree_hash() { - return Control::Prune; - } - } + let updated_node_weight = self.updated_graph.get_node_weight(updated_graph_index)?; - // If either the original node weight was not found or it was found the merkle tree hashes differ, - // then we have information that needs to be collected! - changes.push(Change { - id: updated_node_weight.id(), - merkle_tree_hash: updated_node_weight.merkle_tree_hash(), - }); + if let Some(original_node_weight) = self + .base_graph + .get_node_weight_by_id_opt(updated_node_weight.id()) + { + if original_node_weight.merkle_tree_hash() == updated_node_weight.merkle_tree_hash() + { + return Ok(Control::Prune); } - Err(err) => error!(?err, "heat death of the universe error: updated node weight not found by updated node index from the same graph"), } + + // If either the original node weight was not found or it was found the merkle tree hashes differ, + // then we have information that needs to be collected! + let entity_id = updated_node_weight.id().into(); + changes.push(Change { + entity_id, + entity_kind: self.updated_graph.get_entity_kind_for_id(entity_id)?, + merkle_tree_hash: updated_node_weight.merkle_tree_hash(), + }); } - Control::Continue + Ok(Control::Continue) } } diff --git a/lib/dal/src/workspace_snapshot/graph/tests/detect_changes.rs b/lib/dal/src/workspace_snapshot/graph/tests/detect_changes.rs index ac524b629c..d7cda89290 100644 --- a/lib/dal/src/workspace_snapshot/graph/tests/detect_changes.rs +++ b/lib/dal/src/workspace_snapshot/graph/tests/detect_changes.rs @@ -10,7 +10,9 @@ mod test { assert!(base_graph.is_acyclic_directed()); assert!(updated_graph.is_acyclic_directed()); - let changes = base_graph.detect_changes(&updated_graph); + let changes = base_graph + .detect_changes(&updated_graph) + .expect("could not detect changes"); assert!(changes.is_empty()); } } diff --git a/lib/dal/src/workspace_snapshot/graph/traits.rs b/lib/dal/src/workspace_snapshot/graph/traits.rs index 2328bb46b3..0863d5f40a 100644 --- a/lib/dal/src/workspace_snapshot/graph/traits.rs +++ b/lib/dal/src/workspace_snapshot/graph/traits.rs @@ -1,3 +1,4 @@ +pub mod approval_requirement; pub mod diagram; pub mod entity_kind; pub mod schema; diff --git a/lib/dal/src/workspace_snapshot/graph/traits/approval_requirement.rs b/lib/dal/src/workspace_snapshot/graph/traits/approval_requirement.rs new file mode 100644 index 0000000000..0c10e84d46 --- /dev/null +++ b/lib/dal/src/workspace_snapshot/graph/traits/approval_requirement.rs @@ -0,0 +1,47 @@ +use std::collections::{HashMap, HashSet}; + +use serde::{Deserialize, Serialize}; +use si_events::{merkle_tree_hash::MerkleTreeHash, workspace_snapshot::EntityKind}; +use si_id::{ApprovalRequirementDefinitionId, EntityId, UserPk, WorkspacePk}; + +use crate::workspace_snapshot::graph::{detector::Change, WorkspaceSnapshotGraphResult}; + +#[derive(Debug, Clone, Hash, PartialEq, Eq, Serialize, Deserialize)] +pub struct ApprovalRequirementPermissionLookup { + pub object_type: String, + pub object_id: String, + pub permission: String, +} + +#[derive(Debug, Clone, Hash, PartialEq, Eq, Serialize, Deserialize)] +pub enum ApprovalRequirementApprover { + User(UserPk), + PermissionLookup(ApprovalRequirementPermissionLookup), +} + +#[derive(Debug, Clone)] +pub struct ApprovalRequirementRule { + pub entity_id: EntityId, + pub entity_kind: EntityKind, + pub minimum: usize, + pub approvers: HashSet, +} + +#[derive(Debug)] +pub struct ApprovalRequirementsBag { + pub entity_id: EntityId, + pub entity_kind: EntityKind, + pub explicit_approval_requirement_definition_ids: Vec, + pub virtual_approval_requirement_rules: Vec, +} + +pub trait ApprovalRequirementExt { + fn approval_requirements_for_changes( + &self, + workspace_id: WorkspacePk, + changes: &[Change], + ) -> WorkspaceSnapshotGraphResult<( + Vec, + HashMap, + )>; +} diff --git a/lib/dal/src/workspace_snapshot/graph/v2.rs b/lib/dal/src/workspace_snapshot/graph/v2.rs index 25eef8dfa0..3145d49c12 100644 --- a/lib/dal/src/workspace_snapshot/graph/v2.rs +++ b/lib/dal/src/workspace_snapshot/graph/v2.rs @@ -1235,7 +1235,8 @@ impl WorkspaceSnapshotGraphV2 { | EdgeWeightKind::ManagementPrototype | EdgeWeightKind::ValidationOutput | EdgeWeightKind::Manages - | EdgeWeightKind::DiagramObject => {} + | EdgeWeightKind::DiagramObject + | EdgeWeightKind::HasApprovalRequirement => {} } } } diff --git a/lib/dal/src/workspace_snapshot/graph/v3.rs b/lib/dal/src/workspace_snapshot/graph/v3.rs index f3d2827e25..a7b33937ed 100644 --- a/lib/dal/src/workspace_snapshot/graph/v3.rs +++ b/lib/dal/src/workspace_snapshot/graph/v3.rs @@ -734,23 +734,24 @@ impl WorkspaceSnapshotGraphV3 { EdgeWeightKindDiscriminants::ActionPrototype => "black", EdgeWeightKindDiscriminants::AuthenticationPrototype => "black", EdgeWeightKindDiscriminants::Contain => "blue", + EdgeWeightKindDiscriminants::DiagramObject => "black", EdgeWeightKindDiscriminants::FrameContains => "black", - EdgeWeightKindDiscriminants::Represents => "black", + EdgeWeightKindDiscriminants::HasApprovalRequirement => "black", + EdgeWeightKindDiscriminants::ManagementPrototype => "pink", + EdgeWeightKindDiscriminants::Manages => "pink", EdgeWeightKindDiscriminants::Ordering => "gray", EdgeWeightKindDiscriminants::Ordinal => "gray", EdgeWeightKindDiscriminants::Prop => "orange", EdgeWeightKindDiscriminants::Prototype => "green", EdgeWeightKindDiscriminants::PrototypeArgument => "green", EdgeWeightKindDiscriminants::PrototypeArgumentValue => "green", - EdgeWeightKindDiscriminants::Socket => "red", - EdgeWeightKindDiscriminants::SocketValue => "purple", EdgeWeightKindDiscriminants::Proxy => "gray", + EdgeWeightKindDiscriminants::Represents => "black", EdgeWeightKindDiscriminants::Root => "black", + EdgeWeightKindDiscriminants::Socket => "red", + EdgeWeightKindDiscriminants::SocketValue => "purple", EdgeWeightKindDiscriminants::Use => "black", EdgeWeightKindDiscriminants::ValidationOutput => "darkcyan", - EdgeWeightKindDiscriminants::ManagementPrototype => "pink", - EdgeWeightKindDiscriminants::Manages => "pink", - EdgeWeightKindDiscriminants::DiagramObject => "black", }; match edgeref.weight().kind() { @@ -776,27 +777,28 @@ impl WorkspaceSnapshotGraphV3 { // Some of these should never happen as they have their own top-level // NodeWeight variant. ContentAddressDiscriminants::ActionPrototype => "green", + ContentAddressDiscriminants::ApprovalRequirementDefinition => "black", ContentAddressDiscriminants::AttributePrototype => "green", ContentAddressDiscriminants::Component => "black", ContentAddressDiscriminants::DeprecatedAction => "green", ContentAddressDiscriminants::DeprecatedActionBatch => "green", ContentAddressDiscriminants::DeprecatedActionRunner => "green", - ContentAddressDiscriminants::OutputSocket => "red", ContentAddressDiscriminants::Func => "black", ContentAddressDiscriminants::FuncArg => "black", ContentAddressDiscriminants::Geometry => "black", ContentAddressDiscriminants::InputSocket => "red", ContentAddressDiscriminants::JsonValue => "fuchsia", + ContentAddressDiscriminants::ManagementPrototype => "black", ContentAddressDiscriminants::Module => "yellow", + ContentAddressDiscriminants::OutputSocket => "red", ContentAddressDiscriminants::Prop => "orange", ContentAddressDiscriminants::Root => "black", ContentAddressDiscriminants::Schema => "black", ContentAddressDiscriminants::SchemaVariant => "black", ContentAddressDiscriminants::Secret => "black", ContentAddressDiscriminants::StaticArgumentValue => "green", - ContentAddressDiscriminants::ValidationPrototype => "black", ContentAddressDiscriminants::ValidationOutput => "darkcyan", - ContentAddressDiscriminants::ManagementPrototype => "black", + ContentAddressDiscriminants::ValidationPrototype => "black", ContentAddressDiscriminants::View => "black", }; (discrim.to_string(), color) @@ -876,6 +878,9 @@ impl WorkspaceSnapshotGraphV3 { ("ManagementPrototype".to_string(), "black") } NodeWeight::DiagramObject(_) => ("DiagramObject".to_string(), "black"), + NodeWeight::ApprovalRequirementDefinition(_) => { + ("ApprovalRequirement".to_string(), "black") + } }; let color = color.to_string(); let id = node_weight.id(); @@ -1414,7 +1419,8 @@ impl WorkspaceSnapshotGraphV3 { | EdgeWeightKind::ValidationOutput | EdgeWeightKind::ManagementPrototype | EdgeWeightKind::Manages - | EdgeWeightKind::DiagramObject => {} + | EdgeWeightKind::DiagramObject + | EdgeWeightKind::HasApprovalRequirement => {} } } } diff --git a/lib/dal/src/workspace_snapshot/graph/v4.rs b/lib/dal/src/workspace_snapshot/graph/v4.rs index 9803dea356..668ea598ff 100644 --- a/lib/dal/src/workspace_snapshot/graph/v4.rs +++ b/lib/dal/src/workspace_snapshot/graph/v4.rs @@ -12,8 +12,7 @@ use petgraph::{ visit::DfsEvent, }; use serde::{Deserialize, Serialize}; -use si_events::{ulid::Ulid, workspace_snapshot::EntityKind, ContentHash}; -use si_id::{EntityId, WorkspacePk}; +use si_events::{ulid::Ulid, ContentHash}; use si_layer_cache::db::serialize; use strum::IntoEnumIterator; use telemetry::prelude::*; @@ -34,12 +33,9 @@ use crate::{ Timestamp, }; -use super::{ - approval::{ApprovalRequirement, ApprovalRequirementLookupGroup}, - detector::Change, - traits::entity_kind::EntityKindExt, -}; +use super::detector::Change; +pub mod approval_requirement; pub mod component; pub mod diagram; pub mod entity_kind; @@ -228,6 +224,10 @@ impl WorkspaceSnapshotGraphV4 { .into()) } + pub fn all_node_ids(&self) -> HashSet { + HashSet::from_iter(self.node_index_by_id.keys().copied()) + } + pub fn update_node_id( &mut self, current_idx: NodeIndex, @@ -697,37 +697,11 @@ impl WorkspaceSnapshotGraphV4 { Detector::new(self, updated_graph).detect_updates() } - pub fn detect_changes(&self, updated_graph: &Self) -> Vec { - Detector::new(self, updated_graph).detect_changes() - } - - pub fn approval_requirements_for_changes( + pub fn detect_changes( &self, - workspace_id: WorkspacePk, - changes: &[Change], - ) -> WorkspaceSnapshotGraphResult> { - let mut requirements = Vec::new(); - for change in changes { - let entity_id: EntityId = change.id.into(); - - // TODO(nick,jacob): handle more than schema variants. - if let EntityKind::SchemaVariant = self.get_entity_kind_for_id(entity_id)? { - requirements.push(ApprovalRequirement { - // TODO(nick,jacob): handle more than schema variants. - entity_kind: EntityKind::SchemaVariant, - entity_id, - // TODO(nick,jacob): remove hardcoded number requirement. - number: 1, - // TODO(nick,jacob): replace hardcoded relations. - lookup_groups: vec![ApprovalRequirementLookupGroup { - object_type: "workspace".to_string(), - object_id: workspace_id.to_string(), - permission: "approve".to_string(), - }], - }); - } - } - Ok(requirements) + updated_graph: &Self, + ) -> WorkspaceSnapshotGraphResult> { + Detector::new(self, updated_graph).detect_changes() } #[allow(dead_code)] @@ -899,23 +873,24 @@ impl WorkspaceSnapshotGraphV4 { EdgeWeightKindDiscriminants::ActionPrototype => "black", EdgeWeightKindDiscriminants::AuthenticationPrototype => "black", EdgeWeightKindDiscriminants::Contain => "blue", + EdgeWeightKindDiscriminants::DiagramObject => "black", EdgeWeightKindDiscriminants::FrameContains => "black", - EdgeWeightKindDiscriminants::Represents => "black", + EdgeWeightKindDiscriminants::HasApprovalRequirement => "black", + EdgeWeightKindDiscriminants::ManagementPrototype => "pink", + EdgeWeightKindDiscriminants::Manages => "pink", EdgeWeightKindDiscriminants::Ordering => "gray", EdgeWeightKindDiscriminants::Ordinal => "gray", EdgeWeightKindDiscriminants::Prop => "orange", EdgeWeightKindDiscriminants::Prototype => "green", EdgeWeightKindDiscriminants::PrototypeArgument => "green", EdgeWeightKindDiscriminants::PrototypeArgumentValue => "green", - EdgeWeightKindDiscriminants::Socket => "red", - EdgeWeightKindDiscriminants::SocketValue => "purple", EdgeWeightKindDiscriminants::Proxy => "gray", + EdgeWeightKindDiscriminants::Represents => "black", EdgeWeightKindDiscriminants::Root => "black", + EdgeWeightKindDiscriminants::Socket => "red", + EdgeWeightKindDiscriminants::SocketValue => "purple", EdgeWeightKindDiscriminants::Use => "black", EdgeWeightKindDiscriminants::ValidationOutput => "darkcyan", - EdgeWeightKindDiscriminants::ManagementPrototype => "pink", - EdgeWeightKindDiscriminants::Manages => "pink", - EdgeWeightKindDiscriminants::DiagramObject => "black", }; match edgeref.weight().kind() { @@ -941,27 +916,28 @@ impl WorkspaceSnapshotGraphV4 { // Some of these should never happen as they have their own top-level // NodeWeight variant. ContentAddressDiscriminants::ActionPrototype => "green", + ContentAddressDiscriminants::ApprovalRequirementDefinition => "black", ContentAddressDiscriminants::AttributePrototype => "green", ContentAddressDiscriminants::Component => "black", ContentAddressDiscriminants::DeprecatedAction => "green", ContentAddressDiscriminants::DeprecatedActionBatch => "green", ContentAddressDiscriminants::DeprecatedActionRunner => "green", - ContentAddressDiscriminants::OutputSocket => "red", ContentAddressDiscriminants::Func => "black", ContentAddressDiscriminants::FuncArg => "black", ContentAddressDiscriminants::Geometry => "black", ContentAddressDiscriminants::InputSocket => "red", ContentAddressDiscriminants::JsonValue => "fuchsia", + ContentAddressDiscriminants::ManagementPrototype => "black", ContentAddressDiscriminants::Module => "yellow", + ContentAddressDiscriminants::OutputSocket => "red", ContentAddressDiscriminants::Prop => "orange", ContentAddressDiscriminants::Root => "black", ContentAddressDiscriminants::Schema => "black", ContentAddressDiscriminants::SchemaVariant => "black", ContentAddressDiscriminants::Secret => "black", ContentAddressDiscriminants::StaticArgumentValue => "green", - ContentAddressDiscriminants::ValidationPrototype => "black", ContentAddressDiscriminants::ValidationOutput => "darkcyan", - ContentAddressDiscriminants::ManagementPrototype => "black", + ContentAddressDiscriminants::ValidationPrototype => "black", ContentAddressDiscriminants::View => "black", }; (discrim.to_string(), color) @@ -1041,6 +1017,9 @@ impl WorkspaceSnapshotGraphV4 { ("ManagementPrototype".to_string(), "black") } NodeWeight::DiagramObject(_) => ("DiagramObject".to_string(), "black"), + NodeWeight::ApprovalRequirementDefinition(_) => { + ("ApprovalRequirement".to_string(), "black") + } }; let color = color.to_string(); let id = node_weight.id(); @@ -1601,7 +1580,8 @@ impl WorkspaceSnapshotGraphV4 { | EdgeWeightKind::ValidationOutput | EdgeWeightKind::ManagementPrototype | EdgeWeightKind::Manages - | EdgeWeightKind::DiagramObject => {} + | EdgeWeightKind::DiagramObject + | EdgeWeightKind::HasApprovalRequirement => {} } } } diff --git a/lib/dal/src/workspace_snapshot/graph/v4/approval_requirement.rs b/lib/dal/src/workspace_snapshot/graph/v4/approval_requirement.rs new file mode 100644 index 0000000000..08477d4ad3 --- /dev/null +++ b/lib/dal/src/workspace_snapshot/graph/v4/approval_requirement.rs @@ -0,0 +1,146 @@ +use std::collections::{HashMap, HashSet}; + +use petgraph::Direction; +use si_events::{merkle_tree_hash::MerkleTreeHash, workspace_snapshot::EntityKind}; +use si_id::{EntityId, WorkspacePk}; + +use crate::{ + workspace_snapshot::{ + graph::{ + detector::Change, + traits::approval_requirement::{ + ApprovalRequirementApprover, ApprovalRequirementExt, + ApprovalRequirementPermissionLookup, ApprovalRequirementRule, + ApprovalRequirementsBag, + }, + WorkspaceSnapshotGraphError, WorkspaceSnapshotGraphResult, + }, + node_weight::traits::SiNodeWeight, + }, + EdgeWeightKindDiscriminants, +}; + +use super::WorkspaceSnapshotGraphV4; + +impl ApprovalRequirementExt for WorkspaceSnapshotGraphV4 { + fn approval_requirements_for_changes( + &self, + workspace_id: WorkspacePk, + changes: &[Change], + ) -> WorkspaceSnapshotGraphResult<( + Vec, + HashMap, + )> { + let mut requirements = Vec::new(); + let mut ids_with_hashes_for_deleted_nodes = HashMap::new(); + + for change in changes { + let mut explicit_approval_requirement_definition_ids = Vec::new(); + let mut virtual_approval_requirement_rules = Vec::new(); + + // Check if the node exists in the current graph. If it does, we are working with an + // addition or a modification. If it does not, we are working with a removal. + if let Some(entity_node_index) = self.get_node_index_by_id_opt(change.entity_id) { + for (_, _, requirement_node_index) in self.edges_directed_for_edge_weight_kind( + entity_node_index, + Direction::Outgoing, + EdgeWeightKindDiscriminants::HasApprovalRequirement, + ) { + let requirement_node_weight = self + .get_node_weight(requirement_node_index)? + .get_approval_requirement_definition_node_weight()?; + explicit_approval_requirement_definition_ids + .push(requirement_node_weight.id().into()); + } + + // After processing all explicit approval requirements from approval requirement + // definitions, let's check if we need to add virtual requirements. + match change.entity_kind { + // TODO(nick,jacob): replace this hard-coded virtual rule with an explicit definition + // the schema variant category. + EntityKind::SchemaVariant + if explicit_approval_requirement_definition_ids.is_empty() => + { + virtual_approval_requirement_rules.push(ApprovalRequirementRule { + entity_id: change.entity_id, + entity_kind: change.entity_kind, + minimum: 1, + approvers: HashSet::from([ + ApprovalRequirementApprover::PermissionLookup( + ApprovalRequirementPermissionLookup { + object_type: "workspace".to_string(), + object_id: workspace_id.to_string(), + permission: "approve".to_string(), + }, + ), + ]), + }); + } + // For any changes to explicit approval requirements, we need approvals from + // workspace approvers. + EntityKind::ApprovalRequirementDefinition => { + virtual_approval_requirement_rules.push(ApprovalRequirementRule { + entity_id: change.entity_id, + entity_kind: change.entity_kind, + minimum: 1, + approvers: HashSet::from([ + ApprovalRequirementApprover::PermissionLookup( + ApprovalRequirementPermissionLookup { + object_type: "workspace".to_string(), + object_id: workspace_id.to_string(), + permission: "approve".to_string(), + }, + ), + ]), + }); + } + _ => {} + } + } else { + // If the node does not exist on the current graph, then we know it was deleted. + if let Some(existing_merkle_tree_hash) = ids_with_hashes_for_deleted_nodes + .insert(change.entity_id, change.merkle_tree_hash) + { + // NOTE(nick): this is one of those "heat death of the universe" errors, but + // both you and I do not want to be paged because of a hidden map insertion, + // now do we? Didn't think so! + return Err( + WorkspaceSnapshotGraphError::MultipleMerkleTreeHashesForEntity( + change.entity_id, + change.merkle_tree_hash, + existing_merkle_tree_hash, + ), + ); + }; + + // If the node does not exist on the current graph and it is an approval + // requirement definition node, then we know that the approval requirement + // definition node was deleted. We will need a virtual requirement for this + // removal. + if let EntityKind::ApprovalRequirementDefinition = change.entity_kind { + virtual_approval_requirement_rules.push(ApprovalRequirementRule { + entity_id: change.entity_id, + entity_kind: change.entity_kind, + minimum: 1, + approvers: HashSet::from([ApprovalRequirementApprover::PermissionLookup( + ApprovalRequirementPermissionLookup { + object_type: "workspace".to_string(), + object_id: workspace_id.to_string(), + permission: "approve".to_string(), + }, + )]), + }); + } + } + + requirements.push(ApprovalRequirementsBag { + entity_id: change.entity_id, + entity_kind: change.entity_kind, + explicit_approval_requirement_definition_ids, + virtual_approval_requirement_rules, + }); + } + + Ok((requirements, ids_with_hashes_for_deleted_nodes)) + } +} diff --git a/lib/dal/src/workspace_snapshot/graph/v4/entity_kind.rs b/lib/dal/src/workspace_snapshot/graph/v4/entity_kind.rs index 3fad4b9b14..fb68c45d0c 100644 --- a/lib/dal/src/workspace_snapshot/graph/v4/entity_kind.rs +++ b/lib/dal/src/workspace_snapshot/graph/v4/entity_kind.rs @@ -21,6 +21,9 @@ impl EntityKindExt for WorkspaceSnapshotGraphV4 { Ok(match node_weight.into() { NodeWeightDiscriminants::Action => EntityKind::Action, NodeWeightDiscriminants::ActionPrototype => EntityKind::ActionPrototype, + NodeWeightDiscriminants::ApprovalRequirementDefinition => { + EntityKind::ApprovalRequirementDefinition + } NodeWeightDiscriminants::AttributePrototypeArgument => { EntityKind::AttributePrototypeArgument } diff --git a/lib/dal/src/workspace_snapshot/node_weight.rs b/lib/dal/src/workspace_snapshot/node_weight.rs index 75ae6dab5b..33e1dc36db 100644 --- a/lib/dal/src/workspace_snapshot/node_weight.rs +++ b/lib/dal/src/workspace_snapshot/node_weight.rs @@ -8,6 +8,13 @@ use strum::{EnumDiscriminants, EnumIter}; use thiserror::Error; use traits::{CorrectExclusiveOutgoingEdge, CorrectTransforms, CorrectTransformsResult}; +use crate::layer_db_types::ComponentContentDiscriminants; +use crate::workspace_snapshot::node_weight::diagram_object_node_weight::{ + DiagramObjectKind, DiagramObjectNodeWeight, +}; +use crate::workspace_snapshot::node_weight::geometry_node_weight::GeometryNodeWeight; +use crate::workspace_snapshot::node_weight::traits::SiVersionedNodeWeight; +use crate::workspace_snapshot::node_weight::view_node_weight::ViewNodeWeight; use crate::{ action::prototype::ActionKind, func::FuncKind, @@ -22,19 +29,15 @@ use crate::{ }; use self::{ + approval_requirement_definition_node_weight::ApprovalRequirementDefinitionNodeWeight, input_socket_node_weight::InputSocketNodeWeightError, schema_variant_node_weight::SchemaVariantNodeWeightError, }; + use super::graph::{ deprecated::v1::DeprecatedNodeWeightV1, detector::Update, WorkspaceSnapshotGraphError, }; -use crate::layer_db_types::ComponentContentDiscriminants; -use crate::workspace_snapshot::node_weight::diagram_object_node_weight::{ - DiagramObjectKind, DiagramObjectNodeWeight, -}; -use crate::workspace_snapshot::node_weight::geometry_node_weight::GeometryNodeWeight; -use crate::workspace_snapshot::node_weight::traits::SiVersionedNodeWeight; -use crate::workspace_snapshot::node_weight::view_node_weight::ViewNodeWeight; + pub use action_node_weight::ActionNodeWeight; pub use action_prototype_node_weight::ActionPrototypeNodeWeight; pub use attribute_prototype_argument_node_weight::{ @@ -55,6 +58,7 @@ pub use schema_variant_node_weight::SchemaVariantNodeWeight; pub mod action_node_weight; pub mod action_prototype_node_weight; +pub mod approval_requirement_definition_node_weight; pub mod attribute_prototype_argument_node_weight; pub mod attribute_value_node_weight; pub mod category_node_weight; @@ -146,6 +150,7 @@ pub enum NodeWeight { Geometry(GeometryNodeWeight), View(ViewNodeWeight), DiagramObject(DiagramObjectNodeWeight), + ApprovalRequirementDefinition(ApprovalRequirementDefinitionNodeWeight), } impl NodeWeight { @@ -168,9 +173,10 @@ impl NodeWeight { NodeWeight::InputSocket(weight) => weight.content_hash(), NodeWeight::SchemaVariant(weight) => weight.content_hash(), NodeWeight::ManagementPrototype(weight) => weight.content_hash(), - NodeWeight::Geometry(w) => w.content_hash(), - NodeWeight::View(w) => w.content_hash(), - NodeWeight::DiagramObject(w) => w.content_hash(), + NodeWeight::Geometry(weight) => weight.content_hash(), + NodeWeight::View(weight) => weight.content_hash(), + NodeWeight::DiagramObject(weight) => weight.content_hash(), + NodeWeight::ApprovalRequirementDefinition(weight) => weight.content_hash(), } } @@ -195,7 +201,8 @@ impl NodeWeight { NodeWeight::ManagementPrototype(weight) => weight.content_store_hashes(), NodeWeight::Geometry(weight) => weight.content_store_hashes(), NodeWeight::View(weight) => weight.content_store_hashes(), - NodeWeight::DiagramObject(w) => w.content_store_hashes(), + NodeWeight::DiagramObject(weight) => weight.content_store_hashes(), + NodeWeight::ApprovalRequirementDefinition(weight) => weight.content_store_hashes(), } } @@ -220,7 +227,8 @@ impl NodeWeight { | NodeWeight::ManagementPrototype(_) | NodeWeight::View(_) | NodeWeight::SchemaVariant(_) - | NodeWeight::DiagramObject(_) => None, + | NodeWeight::DiagramObject(_) + | NodeWeight::ApprovalRequirementDefinition(_) => None, } } @@ -246,6 +254,7 @@ impl NodeWeight { NodeWeight::Geometry(weight) => weight.id(), NodeWeight::View(weight) => weight.id(), NodeWeight::DiagramObject(weight) => weight.id(), + NodeWeight::ApprovalRequirementDefinition(weight) => weight.id(), } } @@ -271,6 +280,7 @@ impl NodeWeight { NodeWeight::Geometry(weight) => weight.lineage_id(), NodeWeight::View(weight) => weight.lineage_id(), NodeWeight::DiagramObject(weight) => weight.lineage_id(), + NodeWeight::ApprovalRequirementDefinition(weight) => weight.lineage_id(), } } @@ -356,6 +366,10 @@ impl NodeWeight { weight.set_id(id.into()); weight.set_lineage_id(lineage_id); } + NodeWeight::ApprovalRequirementDefinition(weight) => { + weight.set_id(id.into()); + weight.set_lineage_id(lineage_id); + } } } @@ -378,9 +392,10 @@ impl NodeWeight { NodeWeight::InputSocket(weight) => weight.merkle_tree_hash(), NodeWeight::SchemaVariant(weight) => weight.merkle_tree_hash(), NodeWeight::ManagementPrototype(weight) => weight.merkle_tree_hash(), - NodeWeight::Geometry(w) => w.merkle_tree_hash(), - NodeWeight::View(w) => w.merkle_tree_hash(), - NodeWeight::DiagramObject(w) => w.merkle_tree_hash(), + NodeWeight::Geometry(weight) => weight.merkle_tree_hash(), + NodeWeight::View(weight) => weight.merkle_tree_hash(), + NodeWeight::DiagramObject(weight) => weight.merkle_tree_hash(), + NodeWeight::ApprovalRequirementDefinition(weight) => weight.merkle_tree_hash(), } } @@ -421,6 +436,10 @@ impl NodeWeight { traits::SiVersionedNodeWeight::inner_mut(w).new_content_hash(content_hash); Ok(()) } + NodeWeight::ApprovalRequirementDefinition(w) => { + traits::SiVersionedNodeWeight::inner_mut(w).new_content_hash(content_hash); + Ok(()) + } } } @@ -449,6 +468,7 @@ impl NodeWeight { NodeWeight::Geometry(weight) => weight.node_hash(), NodeWeight::View(weight) => weight.node_hash(), NodeWeight::DiagramObject(weight) => weight.node_hash(), + NodeWeight::ApprovalRequirementDefinition(weight) => weight.node_hash(), } } @@ -474,6 +494,9 @@ impl NodeWeight { NodeWeight::Geometry(weight) => weight.set_merkle_tree_hash(new_hash), NodeWeight::View(weight) => weight.set_merkle_tree_hash(new_hash), NodeWeight::DiagramObject(weight) => weight.set_merkle_tree_hash(new_hash), + NodeWeight::ApprovalRequirementDefinition(weight) => { + weight.set_merkle_tree_hash(new_hash) + } } } @@ -501,7 +524,10 @@ impl NodeWeight { | NodeWeight::InputSocket(_) | NodeWeight::ManagementPrototype(_) | NodeWeight::DiagramObject(_) - | NodeWeight::SchemaVariant(_) => Err(NodeWeightError::CannotSetOrderOnKind), + | NodeWeight::SchemaVariant(_) + | NodeWeight::ApprovalRequirementDefinition(_) => { + Err(NodeWeightError::CannotSetOrderOnKind) + } } } @@ -537,6 +563,7 @@ impl NodeWeight { NodeWeight::SchemaVariant(weight) => weight.exclusive_outgoing_edges(), NodeWeight::ManagementPrototype(weight) => weight.exclusive_outgoing_edges(), NodeWeight::DiagramObject(weight) => weight.exclusive_outgoing_edges(), + NodeWeight::ApprovalRequirementDefinition(weight) => weight.exclusive_outgoing_edges(), } } @@ -638,6 +665,18 @@ impl NodeWeight { } } + pub fn get_approval_requirement_definition_node_weight( + &self, + ) -> NodeWeightResult { + match self { + NodeWeight::ApprovalRequirementDefinition(inner) => Ok(inner.to_owned()), + other => Err(NodeWeightError::UnexpectedNodeWeightVariant( + NodeWeightDiscriminants::ApprovalRequirementDefinition, + other.into(), + )), + } + } + pub fn get_prop_node_weight(&self) -> NodeWeightResult { match self { NodeWeight::Prop(inner) => Ok(inner.to_owned()), @@ -860,6 +899,18 @@ impl NodeWeight { NodeWeight::DiagramObject(DiagramObjectNodeWeight::new(id, lineage_id, object_kind)) } + pub fn new_approval_requirement_definition( + approval_requirement_definition_id: Ulid, + lineage_id: Ulid, + content_hash: ContentHash, + ) -> Self { + NodeWeight::ApprovalRequirementDefinition(ApprovalRequirementDefinitionNodeWeight::new( + approval_requirement_definition_id, + lineage_id, + content_hash, + )) + } + pub fn new_prop( prop_id: Ulid, lineage_id: Ulid, @@ -1093,6 +1144,11 @@ impl CorrectTransforms for NodeWeight { updates, from_different_change_set, ), + NodeWeight::ApprovalRequirementDefinition(weight) => weight.correct_transforms( + workspace_snapshot_graph, + updates, + from_different_change_set, + ), }?; Ok(self.correct_exclusive_outgoing_edges(workspace_snapshot_graph, updates)) @@ -1122,6 +1178,7 @@ impl CorrectExclusiveOutgoingEdge for NodeWeight { NodeWeight::Geometry(weight) => weight.exclusive_outgoing_edges(), NodeWeight::View(weight) => weight.exclusive_outgoing_edges(), NodeWeight::DiagramObject(weight) => weight.exclusive_outgoing_edges(), + NodeWeight::ApprovalRequirementDefinition(weight) => weight.exclusive_outgoing_edges(), } } } diff --git a/lib/dal/src/workspace_snapshot/node_weight/approval_requirement_definition_node_weight.rs b/lib/dal/src/workspace_snapshot/node_weight/approval_requirement_definition_node_weight.rs new file mode 100644 index 0000000000..b469664ab2 --- /dev/null +++ b/lib/dal/src/workspace_snapshot/node_weight/approval_requirement_definition_node_weight.rs @@ -0,0 +1,26 @@ +use serde::{Deserialize, Serialize}; +use si_events::{ulid::Ulid, ContentHash}; + +use super::traits::SiVersionedNodeWeight; + +pub mod v1; + +pub use v1::ApprovalRequirementDefinitionNodeWeightV1; + +#[derive( + Debug, Clone, Serialize, Deserialize, PartialEq, Eq, dal_macros::SiVersionedNodeWeight, +)] +pub enum ApprovalRequirementDefinitionNodeWeight { + #[si_versioned_node_weight(current)] + V1(ApprovalRequirementDefinitionNodeWeightV1), +} + +impl ApprovalRequirementDefinitionNodeWeight { + pub fn new(id: Ulid, lineage_id: Ulid, content_hash: ContentHash) -> Self { + Self::V1(ApprovalRequirementDefinitionNodeWeightV1::new( + id, + lineage_id, + content_hash, + )) + } +} diff --git a/lib/dal/src/workspace_snapshot/node_weight/approval_requirement_definition_node_weight/v1.rs b/lib/dal/src/workspace_snapshot/node_weight/approval_requirement_definition_node_weight/v1.rs new file mode 100644 index 0000000000..18ed947d78 --- /dev/null +++ b/lib/dal/src/workspace_snapshot/node_weight/approval_requirement_definition_node_weight/v1.rs @@ -0,0 +1,46 @@ +use serde::{Deserialize, Serialize}; +use si_events::{merkle_tree_hash::MerkleTreeHash, ulid::Ulid, ContentHash}; + +use crate::{ + workspace_snapshot::{ + content_address::ContentAddress, + graph::LineageId, + node_weight::traits::{CorrectExclusiveOutgoingEdge, CorrectTransforms, SiNodeWeight}, + }, + NodeWeightDiscriminants, Timestamp, +}; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, dal_macros::SiNodeWeight)] +#[si_node_weight(discriminant = NodeWeightDiscriminants::ApprovalRequirementDefinition)] +pub struct ApprovalRequirementDefinitionNodeWeightV1 { + pub id: Ulid, + pub lineage_id: LineageId, + merkle_tree_hash: MerkleTreeHash, + #[si_node_weight(node_hash = "self.content_address.content_hash().as_bytes()")] + content_address: ContentAddress, + timestamp: Timestamp, +} + +impl ApprovalRequirementDefinitionNodeWeightV1 { + pub fn new(id: Ulid, lineage_id: Ulid, content_hash: ContentHash) -> Self { + Self { + id, + lineage_id, + content_address: ContentAddress::ApprovalRequirementDefinition(content_hash), + merkle_tree_hash: MerkleTreeHash::default(), + timestamp: Timestamp::now(), + } + } + + pub fn new_content_hash(&mut self, new_content_hash: ContentHash) { + self.content_address = ContentAddress::ApprovalRequirementDefinition(new_content_hash); + } +} + +impl CorrectTransforms for ApprovalRequirementDefinitionNodeWeightV1 {} + +impl CorrectExclusiveOutgoingEdge for ApprovalRequirementDefinitionNodeWeightV1 { + fn exclusive_outgoing_edges(&self) -> &[crate::EdgeWeightKindDiscriminants] { + &[] + } +} diff --git a/lib/dal/src/workspace_snapshot/node_weight/content_node_weight.rs b/lib/dal/src/workspace_snapshot/node_weight/content_node_weight.rs index a732d1bcf6..0431a1f4b4 100644 --- a/lib/dal/src/workspace_snapshot/node_weight/content_node_weight.rs +++ b/lib/dal/src/workspace_snapshot/node_weight/content_node_weight.rs @@ -96,6 +96,12 @@ impl ContentNodeWeight { ContentAddress::DeprecatedActionRunner(content_hash) } ContentAddress::ActionPrototype(_) => ContentAddress::ActionPrototype(content_hash), + ContentAddress::ApprovalRequirementDefinition(_) => { + return Err(NodeWeightError::InvalidContentAddressForWeightKind( + "ApprovalRequirement".to_string(), + "Content".to_string(), + )); + } ContentAddress::AttributePrototype(_) => { ContentAddress::AttributePrototype(content_hash) } diff --git a/lib/dal/src/workspace_snapshot/traits.rs b/lib/dal/src/workspace_snapshot/traits.rs index d367527109..6006519d4f 100644 --- a/lib/dal/src/workspace_snapshot/traits.rs +++ b/lib/dal/src/workspace_snapshot/traits.rs @@ -2,6 +2,7 @@ //! occur. Using these traits should not require any knowledge of how the underlying graph is //! implemented. +pub mod approval_requirement; pub mod diagram; pub mod entity_kind; pub mod schema; diff --git a/lib/dal/src/workspace_snapshot/traits/approval_requirement.rs b/lib/dal/src/workspace_snapshot/traits/approval_requirement.rs new file mode 100644 index 0000000000..f37296a70d --- /dev/null +++ b/lib/dal/src/workspace_snapshot/traits/approval_requirement.rs @@ -0,0 +1,260 @@ +use std::{ + collections::{HashMap, HashSet}, + sync::Arc, +}; + +use async_trait::async_trait; +use si_events::{merkle_tree_hash::MerkleTreeHash, ContentHash}; +use si_id::{ulid::Ulid, ApprovalRequirementDefinitionId, EntityId, UserPk}; + +use crate::{ + approval_requirement::{ApprovalRequirement, ApprovalRequirementExplicit}, + layer_db_types::{ + ApprovalRequirementDefinitionContent, ApprovalRequirementDefinitionContentV1, + }, + workspace_snapshot::{ + graph::{ + detector::Change, + traits::approval_requirement::ApprovalRequirementExt as ApprovalRequirementExtGraph, + }, + node_weight::{traits::SiNodeWeight, NodeWeight}, + WorkspaceSnapshotResult, + }, + DalContext, EdgeWeight, EdgeWeightKind, WorkspaceSnapshot, WorkspaceSnapshotError, +}; + +pub use crate::workspace_snapshot::graph::traits::approval_requirement::{ + ApprovalRequirementApprover, ApprovalRequirementRule, +}; + +#[async_trait] +pub trait ApprovalRequirementExt { + async fn new_definition( + &self, + ctx: &DalContext, + entity_id: Ulid, + minimum_approvers_count: usize, + approvers: HashSet, + ) -> WorkspaceSnapshotResult; + + async fn remove_definition( + &self, + approval_requirement_definition_id: ApprovalRequirementDefinitionId, + ) -> WorkspaceSnapshotResult<()>; + + async fn add_individual_approver_for_definition( + &self, + ctx: &DalContext, + id: ApprovalRequirementDefinitionId, + user_id: UserPk, + ) -> WorkspaceSnapshotResult<()>; + + async fn remove_individual_approver_for_definition( + &self, + ctx: &DalContext, + id: ApprovalRequirementDefinitionId, + user_id: UserPk, + ) -> WorkspaceSnapshotResult<()>; + + async fn approval_requirements_for_changes( + &self, + ctx: &DalContext, + changes: &[Change], + ) -> WorkspaceSnapshotResult<(Vec, HashMap)>; +} + +#[async_trait] +impl ApprovalRequirementExt for WorkspaceSnapshot { + async fn new_definition( + &self, + ctx: &DalContext, + entity_id: Ulid, + minimum_approvers_count: usize, + approvers: HashSet, + ) -> WorkspaceSnapshotResult { + let content = ApprovalRequirementDefinitionContentV1 { + minimum: minimum_approvers_count, + approvers, + }; + + let (hash, _) = ctx.layer_db().cas().write( + Arc::new(ApprovalRequirementDefinitionContent::V1(content.clone()).into()), + None, + ctx.events_tenancy(), + ctx.events_actor(), + )?; + + let id = self.generate_ulid().await?; + let lineage_id = self.generate_ulid().await?; + let node_weight = NodeWeight::new_approval_requirement_definition(id, lineage_id, hash); + self.add_or_replace_node(node_weight).await?; + + self.add_edge( + entity_id, + EdgeWeight::new(EdgeWeightKind::HasApprovalRequirement), + id, + ) + .await?; + + Ok(id.into()) + } + + async fn remove_definition( + &self, + approval_requirement_definition_id: ApprovalRequirementDefinitionId, + ) -> WorkspaceSnapshotResult<()> { + self.remove_node_by_id(approval_requirement_definition_id) + .await + } + + async fn add_individual_approver_for_definition( + &self, + ctx: &DalContext, + id: ApprovalRequirementDefinitionId, + user_id: UserPk, + ) -> WorkspaceSnapshotResult<()> { + let node_weight = self.get_node_weight_by_id(id).await?; + let content: ApprovalRequirementDefinitionContent = ctx + .layer_db() + .cas() + .try_read_as(&node_weight.content_hash()) + .await? + .ok_or(WorkspaceSnapshotError::MissingContentFromStore(id.into()))?; + + // This should always expect the newest version since we migrate the world when we perform graph migrations. + let ApprovalRequirementDefinitionContent::V1(mut inner) = content; + + // Only update the content store and node if the approver wasn't already in the set. + if inner + .approvers + .insert(ApprovalRequirementApprover::User(user_id)) + { + let (hash, _) = ctx.layer_db().cas().write( + Arc::new(ApprovalRequirementDefinitionContent::V1(inner).into()), + None, + ctx.events_tenancy(), + ctx.events_actor(), + )?; + + ctx.workspace_snapshot()? + .update_content(id.into(), hash) + .await?; + } + + Ok(()) + } + + async fn remove_individual_approver_for_definition( + &self, + ctx: &DalContext, + id: ApprovalRequirementDefinitionId, + user_id: UserPk, + ) -> WorkspaceSnapshotResult<()> { + let node_weight = self.get_node_weight_by_id(id).await?; + let content: ApprovalRequirementDefinitionContent = ctx + .layer_db() + .cas() + .try_read_as(&node_weight.content_hash()) + .await? + .ok_or(WorkspaceSnapshotError::MissingContentFromStore(id.into()))?; + + // This should always expect the newest version since we migrate the world when we perform graph migrations. + let ApprovalRequirementDefinitionContent::V1(mut inner) = content; + + // Only update the content store and node if the approver already existed in the set. + if inner + .approvers + .remove(&ApprovalRequirementApprover::User(user_id)) + { + let (hash, _) = ctx.layer_db().cas().write( + Arc::new(ApprovalRequirementDefinitionContent::V1(inner).into()), + None, + ctx.events_tenancy(), + ctx.events_actor(), + )?; + + ctx.workspace_snapshot()? + .update_content(id.into(), hash) + .await?; + } + + Ok(()) + } + + async fn approval_requirements_for_changes( + &self, + ctx: &DalContext, + changes: &[Change], + ) -> WorkspaceSnapshotResult<(Vec, HashMap)> + { + let mut results = Vec::new(); + + let workspace_id = ctx.workspace_pk()?; + let (bags, ids_with_hashes_for_deleted_nodes) = self + .working_copy() + .await + .approval_requirements_for_changes(workspace_id, changes)?; + + let mut cache = HashMap::new(); + for bag in bags { + // For the explicit requirements, build a cache of hashes. + for approval_requirement_definition_id in + bag.explicit_approval_requirement_definition_ids + { + let requirement_node_weight = self + .working_copy() + .await + .get_node_weight_by_id(approval_requirement_definition_id)? + .get_approval_requirement_definition_node_weight()?; + let hash = requirement_node_weight.content_hash(); + cache.insert( + hash, + ( + approval_requirement_definition_id, + bag.entity_id, + bag.entity_kind, + ), + ); + } + + // For the virtual requirements, add them directly to our requirements list. + results.extend( + bag.virtual_approval_requirement_rules + .iter() + .cloned() + .map(ApprovalRequirement::Virtual), + ); + } + + // From the cache of hashes, perform one bulk retrieval from CAS. + let hashes: Vec = Vec::from_iter(cache.keys().cloned()); + let content_map: HashMap = + ctx.layer_db().cas().try_read_many_as(&hashes).await?; + + // With both the content map and cache in hand, we can assemble all of the explicit + // requirements. + for (hash, (approval_requirement_definition_id, entity_id, entity_kind)) in cache { + if let Some(content) = content_map.get(&hash) { + // NOTE(nick): if we had a v2, then there would be migration logic here. + let ApprovalRequirementDefinitionContent::V1(inner) = content; + + results.push(ApprovalRequirement::Explicit(ApprovalRequirementExplicit { + id: approval_requirement_definition_id, + rule: ApprovalRequirementRule { + entity_id, + entity_kind, + minimum: inner.minimum, + approvers: inner.approvers.to_owned(), + }, + })); + } else { + return Err(WorkspaceSnapshotError::MissingContentFromContentMap( + hash, + approval_requirement_definition_id, + )); + } + } + + Ok((results, ids_with_hashes_for_deleted_nodes)) + } +} diff --git a/lib/dal/tests/integration_test/change_set/approval.rs b/lib/dal/tests/integration_test/change_set/approval.rs index c0631e36f5..7ef812a8cc 100644 --- a/lib/dal/tests/integration_test/change_set/approval.rs +++ b/lib/dal/tests/integration_test/change_set/approval.rs @@ -7,18 +7,30 @@ use pretty_assertions_sorted::assert_eq; #[test] async fn new_and_list_latest(ctx: &mut DalContext) -> Result<()> { + // Create a component and commit. let component = create_component_for_default_schema_name_in_default_view(ctx, "fallout", "soken").await?; ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; + // Cache information we need. let status = ChangeSetApprovalStatus::Approved; - let raw_id: si_id::ulid::Ulid = component.id().into(); - let new_approval = ChangeSetApproval::new(ctx, status, vec![raw_id.into()]).await?; + let component_node_weight = ctx + .workspace_snapshot()? + .get_node_weight_by_id(component.id()) + .await?; + let approving_ids_with_hashes = vec![( + component_node_weight.id().into(), + component_node_weight.merkle_tree_hash(), + )]; + + // Create an approval. + let new_approval = ChangeSetApproval::new(ctx, status, approving_ids_with_hashes).await?; assert_eq!( status, // expectd new_approval.status() // actual ); + // List latest approvals. let mut approvals = ChangeSetApproval::list_latest(ctx).await?; let approval = approvals.pop().ok_or_eyre("unexpected empty approvals")?; assert!(approvals.is_empty()); diff --git a/lib/dal/tests/integration_test/deserialize/mod.rs b/lib/dal/tests/integration_test/deserialize/mod.rs index 5be83d6b5f..359f9ff6c1 100644 --- a/lib/dal/tests/integration_test/deserialize/mod.rs +++ b/lib/dal/tests/integration_test/deserialize/mod.rs @@ -140,13 +140,20 @@ fn make_me_one_with_everything(graph: &mut WorkspaceSnapshotGraphVCurrent) { NodeWeightDiscriminants::View => NodeWeight::new_view( Ulid::new(), Ulid::new(), - ContentHash::new("geometry".as_bytes()), + ContentHash::new("view".as_bytes()), ), NodeWeightDiscriminants::DiagramObject => NodeWeight::new_diagram_object( Ulid::new(), Ulid::new(), DiagramObjectKind::View(Ulid::new().into()), ), + NodeWeightDiscriminants::ApprovalRequirementDefinition => { + NodeWeight::new_approval_requirement_definition( + Ulid::new(), + Ulid::new(), + ContentHash::new("stellaris hurts my brain".as_bytes()), + ) + } }; let idx = graph.add_or_replace_node(weight).expect("add node"); @@ -197,6 +204,9 @@ fn make_me_one_with_everything(graph: &mut WorkspaceSnapshotGraphVCurrent) { EdgeWeightKindDiscriminants::Represents => EdgeWeightKind::Represents, EdgeWeightKindDiscriminants::Manages => EdgeWeightKind::Manages, EdgeWeightKindDiscriminants::DiagramObject => EdgeWeightKind::DiagramObject, + EdgeWeightKindDiscriminants::HasApprovalRequirement => { + EdgeWeightKind::HasApprovalRequirement + } }; let edge_weight = EdgeWeight::new(edge_weight_kind); @@ -290,6 +300,7 @@ async fn graph_can_be_deserialized(_ctx: &DalContext) { NodeWeight::Geometry(_) => {} NodeWeight::View(_) => {} NodeWeight::DiagramObject(_) => {} + NodeWeight::ApprovalRequirementDefinition(_) => {} } } } diff --git a/lib/sdf-server/src/dal_wrapper.rs b/lib/sdf-server/src/dal_wrapper.rs index c092bf4604..2388711de2 100644 --- a/lib/sdf-server/src/dal_wrapper.rs +++ b/lib/sdf-server/src/dal_wrapper.rs @@ -29,15 +29,15 @@ while_true )] -use std::collections::HashMap; +use std::{collections::HashMap, str::FromStr}; use dal::{ + approval_requirement::{ApprovalRequirement, ApprovalRequirementApprover}, change_set::approval::ChangeSetApproval, - workspace_snapshot::{graph::approval::ApprovalRequirement, EntityKindExt}, - DalContext, HistoryActor, WorkspacePk, + DalContext, HistoryActor, UserPk, WorkspacePk, }; use permissions::{Permission, PermissionBuilder}; -use si_events::ChangeSetApprovalStatus; +use si_events::{merkle_tree_hash::MerkleTreeHash, ChangeSetApprovalStatus}; use si_id::{ChangeSetApprovalId, EntityId}; use thiserror::Error; @@ -45,6 +45,8 @@ use thiserror::Error; #[remain::sorted] #[derive(Debug, Error)] pub enum DalWrapperError { + #[error("approval requirement error: {0}")] + ApprovalRequirement(#[from] dal::approval_requirement::ApprovalRequirementError), #[error("change set approval error")] ChangeSetApproval(#[from] dal::change_set::approval::ChangeSetApprovalError), #[error("invalid user found")] @@ -57,6 +59,8 @@ pub enum DalWrapperError { SpiceDBLookupSubjects(#[source] si_data_spicedb::Error), #[error("transactions error: {0}")] Transactions(#[from] dal::TransactionsError), + #[error("ulid decode error: {0}")] + UlidDecode(#[from] ulid::DecodeError), #[error("workspace snapshot error: {0}")] WorkspaceSnapshot(#[from] dal::WorkspaceSnapshotError), } @@ -83,10 +87,16 @@ impl ChangeSetApprovalCalculator { .workspace_snapshot()? .detect_changes_from_head(ctx) .await?; - let requirements = ctx - .workspace_snapshot()? - .approval_requirements_for_changes(workspace_id, &changes) - .await?; + let (requirements, ids_with_hashes_for_deleted_nodes) = + ApprovalRequirement::list(ctx, &changes).await?; + let approving_requirement_ids_with_hashes = determine_approving_ids_with_hashes_inner( + ctx, + spicedb_client, + workspace_id, + &requirements, + &ids_with_hashes_for_deleted_nodes, + ) + .await?; let latest_approvals = ChangeSetApproval::list_latest(ctx).await?; // Initialize what we will eventual use to construct ourself. @@ -96,12 +106,9 @@ impl ChangeSetApprovalCalculator { // Go through each approval, determine its validity, and populate the cache. for approval in latest_approvals { - let approved_requirement_ids = - determine_approving_ids_inner(ctx, spicedb_client, workspace_id, &requirements) - .await?; - for approved_requirement_id in &approved_requirement_ids { + for (approving_requirement_id, _) in &approving_requirement_ids_with_hashes { requirements_to_approvals_cache - .entry(*approved_requirement_id) + .entry(*approving_requirement_id) .and_modify(|a| a.push(approval.id())) .or_insert_with(|| vec![approval.id()]); } @@ -109,7 +116,7 @@ impl ChangeSetApprovalCalculator { // Based on the approving IDs, get the checksum. let checksum = ctx .workspace_snapshot()? - .calculate_checksum(ctx, approved_requirement_ids) + .calculate_checksum(ctx, approving_requirement_ids_with_hashes.to_owned()) .await? .to_string(); @@ -140,25 +147,28 @@ impl ChangeSetApprovalCalculator { /// Returns the requirments for frontend use, which includes whether or not they are satisfied. pub async fn frontend_requirements( &self, - ctx: &DalContext, spicedb_client: &mut si_data_spicedb::Client, ) -> Result> { let mut frontend_requirements = Vec::with_capacity(self.requirements.len()); - let mut global_approving_groups_cache: HashMap> = HashMap::new(); + let mut global_approving_groups_cache: HashMap> = HashMap::new(); // For each requirement, check if it has been satisfied, what approvals are applicable for // it, and what groups and users can approve it. for requirement in &self.requirements { + let rule = match requirement { + ApprovalRequirement::Explicit(inner) => &inner.rule, + ApprovalRequirement::Virtual(inner) => inner, + }; + // First, reset the satisfication and helper variables. - let required_count = requirement.number; + let required_count = rule.minimum; let mut satisfying_approval_count = 0; let mut is_satisfied = false; // If we have applicable approvals, then any that are valid and "approved" will // contribute to the count. If we hit the count, break and rejoice. - let applicable_approval_ids = if let Some(applicable_approval_ids) = self - .requirements_to_approvals_cache - .get(&requirement.entity_id) + let applicable_approval_ids = if let Some(applicable_approval_ids) = + self.requirements_to_approvals_cache.get(&rule.entity_id) { for applicable_approval_id in applicable_approval_ids { let applicable_approval = self @@ -183,50 +193,65 @@ impl ChangeSetApprovalCalculator { }; // We know what requirements have been satisfied, but we need to determine what groups - // can fulfill those requirements as well as who belongs to them. - let mut approving_groups = HashMap::new(); - for lookup_group in &requirement.lookup_groups { - let lookup_group_key = format!( + // and/or individuals can fulfill those requirements. + let mut approver_groups = HashMap::new(); + let mut approver_individuals = Vec::new(); + for approver in &rule.approvers { + let permission_lookup = match approver { + ApprovalRequirementApprover::User(user_id) => { + approver_individuals.push(*user_id); + continue; + } + ApprovalRequirementApprover::PermissionLookup(permission_lookup) => { + permission_lookup + } + }; + + let permisssion_lookup_key = format!( "{}#{}#{}", - lookup_group.object_type, lookup_group.object_id, lookup_group.permission + permission_lookup.object_type, + permission_lookup.object_id, + permission_lookup.permission ); // Check the global cache to reduce calls to SpiceDB. - let member_ids: Vec = - match global_approving_groups_cache.get(&lookup_group_key) { + let member_ids: Vec = + match global_approving_groups_cache.get(&permisssion_lookup_key) { Some(member_ids) => member_ids.to_owned(), None => { // TODO(nick): uh... do what Brit said in her original comment to this. - let member_ids = spicedb_client + let raw_member_ids = spicedb_client .lookup_subjects( - lookup_group.object_type.to_owned(), - lookup_group.object_id.to_owned(), - lookup_group.permission.to_owned(), + permission_lookup.object_type.to_owned(), + permission_lookup.object_id.to_owned(), + permission_lookup.permission.to_owned(), "user".to_owned(), ) .await .map_err(DalWrapperError::SpiceDBLookupSubjects)?; + let mut member_ids = Vec::with_capacity(raw_member_ids.len()); + for raw_member_id in raw_member_ids { + member_ids.push(UserPk::from_str(raw_member_id.as_str())?); + } global_approving_groups_cache - .insert(lookup_group_key.to_owned(), member_ids.to_owned()); + .insert(permisssion_lookup_key.to_owned(), member_ids.to_owned()); member_ids } }; - approving_groups.insert(lookup_group_key, member_ids); + approver_groups.insert(permisssion_lookup_key, member_ids); } - // With both the satisfaction and approving groups information in hand, we can assemble - // the frontend requirement. + // With both the satisfaction and approvers information in hand, we can assemble the + // frontend requirement. frontend_requirements.push(si_frontend_types::ChangeSetApprovalRequirement { - entity_id: requirement.entity_id, - entity_kind: ctx - .workspace_snapshot()? - .get_entity_kind_for_id(requirement.entity_id) - .await?, + entity_id: rule.entity_id, + entity_kind: rule.entity_kind, required_count, is_satisfied, applicable_approval_ids, - approving_groups, + approver_groups, + approver_individuals, }) } @@ -234,55 +259,101 @@ impl ChangeSetApprovalCalculator { } } -/// Determines which IDs corresponding to nodes on the graph that the user can approve changes for. -pub async fn determine_approving_ids( +/// Determines which IDs (with hashes) correspond to nodes on the graph that the user can approve +/// changes for. +pub async fn determine_approving_ids_with_hashes( ctx: &DalContext, spicedb_client: &mut si_data_spicedb::Client, -) -> Result> { +) -> Result> { let workspace_id = ctx.workspace_pk()?; let changes = ctx .workspace_snapshot()? .detect_changes_from_head(ctx) .await?; - let requirements = ctx - .workspace_snapshot()? - .approval_requirements_for_changes(workspace_id, &changes) - .await?; - determine_approving_ids_inner(ctx, spicedb_client, workspace_id, &requirements).await + let (requirements, ids_with_hashes_for_deleted_nodes) = + ApprovalRequirement::list(ctx, &changes).await?; + determine_approving_ids_with_hashes_inner( + ctx, + spicedb_client, + workspace_id, + &requirements, + &ids_with_hashes_for_deleted_nodes, + ) + .await } -async fn determine_approving_ids_inner( +async fn determine_approving_ids_with_hashes_inner( ctx: &DalContext, spicedb_client: &mut si_data_spicedb::Client, workspace_id: WorkspacePk, requirements: &[ApprovalRequirement], -) -> Result> { - let mut approving_ids = Vec::new(); + ids_with_hashes_for_deleted_nodes: &HashMap, +) -> Result> { + let user_id = match ctx.history_actor() { + HistoryActor::SystemInit => return Err(DalWrapperError::InvalidUser), + HistoryActor::User(user_id) => *user_id, + }; + + let mut approving_ids_with_hashes = Vec::new(); let mut cache = HashMap::new(); for requirement in requirements { + let rule = match requirement { + ApprovalRequirement::Explicit(inner) => &inner.rule, + ApprovalRequirement::Virtual(inner) => inner, + }; + // For each requirement, we need to see if we have permission to fulfill it. - for lookup_group in &requirement.lookup_groups { - let has_permission_for_requirement = - if let Some(has_permission) = cache.get(&lookup_group) { - *has_permission + for approver in &rule.approvers { + let has_permission_for_requirement = if let Some(has_permission) = cache.get(&approver) + { + *has_permission + } else { + // If the permission is not in our cache, we need to find it and cache it for later. + match approver { + ApprovalRequirementApprover::User(approver_user_id) => { + let has_permission = *approver_user_id == user_id; + cache.insert(approver, has_permission); + has_permission + } + ApprovalRequirementApprover::PermissionLookup(_) => { + // TODO(nick): use the actual lookup group rather than this hardcoded check. + let has_permission = PermissionBuilder::new() + .workspace_object(workspace_id) + .permission(Permission::Approve) + .user_subject(match ctx.history_actor() { + HistoryActor::SystemInit => { + return Err(DalWrapperError::InvalidUser) + } + HistoryActor::User(user_id) => *user_id, + }) + .has_permission(spicedb_client) + .await?; + cache.insert(approver, has_permission); + has_permission + } + } + }; + + if has_permission_for_requirement { + // NOTE(nick): okay, this is where things get weird. What if a virtual requirement + // was generated for a deleted node? Well then ya can't get the merkle tree hash + // ya know? Therefore, we must first consult the map pertaining to deleted nodes + // to see if our hash is in there first. This algorithm assumes that the map is + // assembled correctly, so it's best to be sure it's correct or you will perish. + let merkle_tree_hash = if let Some(merkle_tree_hash_for_deleted_node) = + ids_with_hashes_for_deleted_nodes.get(&rule.entity_id) + { + *merkle_tree_hash_for_deleted_node } else { - // TODO(nick,jacob): use the actual lookup group rather than this hardcoded check. - let has_permission = PermissionBuilder::new() - .workspace_object(workspace_id) - .permission(Permission::Approve) - .user_subject(match ctx.history_actor() { - HistoryActor::SystemInit => return Err(DalWrapperError::InvalidUser), - HistoryActor::User(user_id) => *user_id, - }) - .has_permission(spicedb_client) - .await?; - cache.insert(lookup_group, has_permission); - has_permission + ctx.workspace_snapshot()? + .get_node_weight_by_id(rule.entity_id) + .await? + .merkle_tree_hash() }; - if has_permission_for_requirement { - approving_ids.push(requirement.entity_id); + // Now that we have the hash, we can push! + approving_ids_with_hashes.push((rule.entity_id, merkle_tree_hash)); // If we found that we have permission for the requirement, we do not need to continue // going through lookup groups for permissions. @@ -291,5 +362,5 @@ async fn determine_approving_ids_inner( } } - Ok(approving_ids) + Ok(approving_ids_with_hashes) } diff --git a/lib/sdf-server/src/service/v2.rs b/lib/sdf-server/src/service/v2.rs index 5d7de6c78f..230a03e2d9 100644 --- a/lib/sdf-server/src/service/v2.rs +++ b/lib/sdf-server/src/service/v2.rs @@ -12,6 +12,7 @@ use crate::{ }; pub mod admin; +pub mod approval_requirement_definition; pub mod audit_log; pub mod change_set; pub mod fs; @@ -41,7 +42,11 @@ fn workspace_routes(state: AppState) -> Router { .nest("/modules", module::v2_routes()) .nest("/schema-variants", variant::v2_routes()) .nest("/management", management::v2_routes()) - .nest("/views", view::v2_routes()), + .nest("/views", view::v2_routes()) + .nest( + "/approval-requirement-definitions", + approval_requirement_definition::v2_routes(), + ), ) .nest("/integrations", integrations::v2_routes()) .nest("/fs", fs::fs_routes()) diff --git a/lib/sdf-server/src/service/v2/approval_requirement_definition.rs b/lib/sdf-server/src/service/v2/approval_requirement_definition.rs new file mode 100644 index 0000000000..34e67871b9 --- /dev/null +++ b/lib/sdf-server/src/service/v2/approval_requirement_definition.rs @@ -0,0 +1,48 @@ +use axum::{ + response::{IntoResponse, Response}, + routing::{delete, put}, + Router, +}; +use thiserror::Error; + +use crate::{service::ApiError, AppState}; + +mod add_individual_approver; +mod new; +mod remove; +mod remove_individual_approver; + +#[remain::sorted] +#[derive(Debug, Error)] +pub enum ApprovalRequirementDefinitionError { + #[error("dal approval requirement error: {0}")] + DalApprovalRequirement(#[from] dal::approval_requirement::ApprovalRequirementError), + #[error("dal change set error: {0}")] + DalChangeSet(#[from] dal::ChangeSetError), + #[error("dal transactions error: {0}")] + DalTransactions(#[from] dal::TransactionsError), +} + +impl IntoResponse for ApprovalRequirementDefinitionError { + fn into_response(self) -> Response { + let err_string = self.to_string(); + + #[allow(clippy::match_single_binding)] + let (status_code, maybe_message) = match self { + _ => (ApiError::DEFAULT_ERROR_STATUS_CODE, None), + }; + + ApiError::new(status_code, maybe_message.unwrap_or(err_string)).into_response() + } +} + +pub fn v2_routes() -> Router { + Router::new() + .route("/", put(new::new)) + .route("/:id", delete(remove::remove)) + .route( + "/:id/individual-approver/:user-id", + put(add_individual_approver::add_individual_approver) + .delete(remove_individual_approver::remove_individual_approver), + ) +} diff --git a/lib/sdf-server/src/service/v2/approval_requirement_definition/add_individual_approver.rs b/lib/sdf-server/src/service/v2/approval_requirement_definition/add_individual_approver.rs new file mode 100644 index 0000000000..90e400b77d --- /dev/null +++ b/lib/sdf-server/src/service/v2/approval_requirement_definition/add_individual_approver.rs @@ -0,0 +1,38 @@ +use axum::extract::Path; +use dal::{approval_requirement::ApprovalRequirement, ChangeSet, ChangeSetId, UserPk, WorkspacePk}; +use si_id::ApprovalRequirementDefinitionId; + +use crate::{ + extract::HandlerContext, service::force_change_set_response::ForceChangeSetResponse, + service::v2::AccessBuilder, +}; + +use super::ApprovalRequirementDefinitionError; + +pub async fn add_individual_approver( + HandlerContext(builder): HandlerContext, + AccessBuilder(access_builder): AccessBuilder, + Path((_workspace_pk, change_set_id, approval_requirement_definition_id, user_id)): Path<( + WorkspacePk, + ChangeSetId, + ApprovalRequirementDefinitionId, + UserPk, + )>, +) -> Result, ApprovalRequirementDefinitionError> { + let mut ctx = builder + .build(access_builder.build(change_set_id.into())) + .await?; + let force_change_set_id = ChangeSet::force_new(&mut ctx).await?; + + // TODO(nick): add audit logs, posthog tracking and WsEvent(s). + ApprovalRequirement::add_individual_approver_for_definition( + &ctx, + approval_requirement_definition_id, + user_id, + ) + .await?; + + ctx.commit().await?; + + Ok(ForceChangeSetResponse::empty(force_change_set_id)) +} diff --git a/lib/sdf-server/src/service/v2/approval_requirement_definition/new.rs b/lib/sdf-server/src/service/v2/approval_requirement_definition/new.rs new file mode 100644 index 0000000000..97feb6cbdf --- /dev/null +++ b/lib/sdf-server/src/service/v2/approval_requirement_definition/new.rs @@ -0,0 +1,44 @@ +use std::collections::HashSet; + +use axum::{extract::Path, Json}; +use dal::{approval_requirement::ApprovalRequirement, ChangeSet, ChangeSetId, WorkspacePk}; +use serde::Deserialize; +use si_id::EntityId; + +use crate::{ + extract::HandlerContext, service::force_change_set_response::ForceChangeSetResponse, + service::v2::AccessBuilder, +}; + +use super::ApprovalRequirementDefinitionError; + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct Request { + entity_id: EntityId, +} + +pub async fn new( + HandlerContext(builder): HandlerContext, + AccessBuilder(access_builder): AccessBuilder, + Path((_workspace_pk, change_set_id)): Path<(WorkspacePk, ChangeSetId)>, + Json(request): Json, +) -> Result, ApprovalRequirementDefinitionError> { + let mut ctx = builder + .build(access_builder.build(change_set_id.into())) + .await?; + let force_change_set_id = ChangeSet::force_new(&mut ctx).await?; + + // TODO(nick): add audit logs, posthog tracking and WsEvent(s). + ApprovalRequirement::new_definition( + &ctx, + request.entity_id, + 1, // TODO(nick): allow users to change the minimum approvers count + HashSet::new(), // TODO(nick): allow users to send in an initial set of approvers + ) + .await?; + + ctx.commit().await?; + + Ok(ForceChangeSetResponse::empty(force_change_set_id)) +} diff --git a/lib/sdf-server/src/service/v2/approval_requirement_definition/remove.rs b/lib/sdf-server/src/service/v2/approval_requirement_definition/remove.rs new file mode 100644 index 0000000000..175e36de88 --- /dev/null +++ b/lib/sdf-server/src/service/v2/approval_requirement_definition/remove.rs @@ -0,0 +1,32 @@ +use axum::extract::Path; +use dal::{approval_requirement::ApprovalRequirement, ChangeSet, ChangeSetId, WorkspacePk}; +use si_id::ApprovalRequirementDefinitionId; + +use crate::{ + extract::HandlerContext, service::force_change_set_response::ForceChangeSetResponse, + service::v2::AccessBuilder, +}; + +use super::ApprovalRequirementDefinitionError; + +pub async fn remove( + HandlerContext(builder): HandlerContext, + AccessBuilder(access_builder): AccessBuilder, + Path((_workspace_pk, change_set_id, approval_requirement_definition_id)): Path<( + WorkspacePk, + ChangeSetId, + ApprovalRequirementDefinitionId, + )>, +) -> Result, ApprovalRequirementDefinitionError> { + let mut ctx = builder + .build(access_builder.build(change_set_id.into())) + .await?; + let force_change_set_id = ChangeSet::force_new(&mut ctx).await?; + + // TODO(nick): add audit logs, posthog tracking and WsEvent(s). + ApprovalRequirement::remove_definition(&ctx, approval_requirement_definition_id).await?; + + ctx.commit().await?; + + Ok(ForceChangeSetResponse::empty(force_change_set_id)) +} diff --git a/lib/sdf-server/src/service/v2/approval_requirement_definition/remove_individual_approver.rs b/lib/sdf-server/src/service/v2/approval_requirement_definition/remove_individual_approver.rs new file mode 100644 index 0000000000..f65e54eb89 --- /dev/null +++ b/lib/sdf-server/src/service/v2/approval_requirement_definition/remove_individual_approver.rs @@ -0,0 +1,38 @@ +use axum::extract::Path; +use dal::{approval_requirement::ApprovalRequirement, ChangeSet, ChangeSetId, UserPk, WorkspacePk}; +use si_id::ApprovalRequirementDefinitionId; + +use crate::{ + extract::HandlerContext, service::force_change_set_response::ForceChangeSetResponse, + service::v2::AccessBuilder, +}; + +use super::ApprovalRequirementDefinitionError; + +pub async fn remove_individual_approver( + HandlerContext(builder): HandlerContext, + AccessBuilder(access_builder): AccessBuilder, + Path((_workspace_pk, change_set_id, approval_requirement_definition_id, user_id)): Path<( + WorkspacePk, + ChangeSetId, + ApprovalRequirementDefinitionId, + UserPk, + )>, +) -> Result, ApprovalRequirementDefinitionError> { + let mut ctx = builder + .build(access_builder.build(change_set_id.into())) + .await?; + let force_change_set_id = ChangeSet::force_new(&mut ctx).await?; + + // TODO(nick): add audit logs, posthog tracking and WsEvent(s). + ApprovalRequirement::remove_individual_approver_for_definition( + &ctx, + approval_requirement_definition_id, + user_id, + ) + .await?; + + ctx.commit().await?; + + Ok(ForceChangeSetResponse::empty(force_change_set_id)) +} diff --git a/lib/sdf-server/src/service/v2/change_set/approval_status.rs b/lib/sdf-server/src/service/v2/change_set/approval_status.rs index ce70c05f40..9cf4bfce19 100644 --- a/lib/sdf-server/src/service/v2/change_set/approval_status.rs +++ b/lib/sdf-server/src/service/v2/change_set/approval_status.rs @@ -28,9 +28,7 @@ pub async fn approval_status( let calculator = ChangeSetApprovalCalculator::new(&ctx, spicedb_client).await?; Ok(Json(si_frontend_types::ChangeSetApprovals { - requirements: calculator - .frontend_requirements(&ctx, spicedb_client) - .await?, + requirements: calculator.frontend_requirements(spicedb_client).await?, latest_approvals: calculator.frontend_latest_approvals(), })) } diff --git a/lib/sdf-server/src/service/v2/change_set/approve_v2.rs b/lib/sdf-server/src/service/v2/change_set/approve_v2.rs index 2bd135df1e..2cffa03a46 100644 --- a/lib/sdf-server/src/service/v2/change_set/approve_v2.rs +++ b/lib/sdf-server/src/service/v2/change_set/approve_v2.rs @@ -59,7 +59,7 @@ pub async fn approve( let spicedb_client = state .spicedb_client() .ok_or(ChangeSetAPIError::SpiceDBClientNotFound)?; - let approving_ids = dal_wrapper::determine_approving_ids(&ctx, spicedb_client).await?; + let approving_ids = dal_wrapper::determine_approving_ids_with_hashes(&ctx, spicedb_client).await?; ChangeSetApproval::new(&ctx, request.status, approving_ids).await?; WsEvent::change_set_approval_status_changed(&ctx, ctx.change_set_id()) diff --git a/lib/sdf-server/tests/service_tests/change_set_approval.rs b/lib/sdf-server/tests/service_tests/change_set_approval.rs index fff1ddedc4..78c610ce07 100644 --- a/lib/sdf-server/tests/service_tests/change_set_approval.rs +++ b/lib/sdf-server/tests/service_tests/change_set_approval.rs @@ -1,6 +1,10 @@ use std::collections::HashMap; +use std::collections::HashSet; +use dal::approval_requirement::ApprovalRequirement; +use dal::approval_requirement::ApprovalRequirementApprover; use dal::change_set::approval::ChangeSetApproval; +use dal::diagram::view::View; use dal::ComponentType; use dal::DalContext; use dal::HistoryActor; @@ -61,11 +65,11 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( // Cache hardcoded values. This should eventually become dynamic as the feature evolves. let entity_kind = EntityKind::SchemaVariant; let required_count = 1; - let lookup_group_key = format!("workspace#{workspace_id}#approve"); - let approving_groups_without_relation = - HashMap::from_iter(vec![(lookup_group_key.to_owned(), Vec::new())]); - let approving_groups_with_relation = - HashMap::from_iter(vec![(lookup_group_key, vec![user_id.to_string()])]); + let permission_lookup_key = format!("workspace#{workspace_id}#approve"); + let approvers_groups_without_relation = + HashMap::from_iter(vec![(permission_lookup_key.to_owned(), Vec::new())]); + let approvers_groups_with_relation = + HashMap::from_iter(vec![(permission_lookup_key, vec![user_id])]); // Scenario 1: we start without any relations in SpiceDB. First, create a component, create a // schema variant and then approve. Second, create a second component and then approve. Both of @@ -100,7 +104,8 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( }; ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; - let approving_ids = dal_wrapper::determine_approving_ids(ctx, &mut spicedb_client).await?; + let approving_ids = + dal_wrapper::determine_approving_ids_with_hashes(ctx, &mut spicedb_client).await?; let first_approval = ChangeSetApproval::new(ctx, ChangeSetApprovalStatus::Approved, approving_ids).await?; ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; @@ -113,7 +118,8 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( .await?; ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; - let approving_ids = dal_wrapper::determine_approving_ids(ctx, &mut spicedb_client).await?; + let approving_ids = + dal_wrapper::determine_approving_ids_with_hashes(ctx, &mut spicedb_client).await?; let second_approval = ChangeSetApproval::new(ctx, ChangeSetApprovalStatus::Approved, approving_ids).await?; ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; @@ -138,15 +144,16 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( ); let frontend_requirements = calculator - .frontend_requirements(ctx, &mut spicedb_client) + .frontend_requirements(&mut spicedb_client) .await?; let expected_requirements = vec![si_frontend_types::ChangeSetApprovalRequirement { entity_id, entity_kind, required_count, is_satisfied: false, - applicable_approval_ids: vec![], - approving_groups: approving_groups_without_relation.to_owned(), + applicable_approval_ids: Vec::new(), + approver_groups: approvers_groups_without_relation.to_owned(), + approver_individuals: Vec::new(), }]; assert_eq!( expected_requirements, // expected @@ -185,7 +192,7 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( ); let mut frontend_requirements = calculator - .frontend_requirements(ctx, &mut spicedb_client) + .frontend_requirements(&mut spicedb_client) .await?; frontend_requirements .iter_mut() @@ -196,7 +203,8 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( required_count, is_satisfied: false, applicable_approval_ids: vec![first_approval_id, second_approval_id], - approving_groups: approving_groups_with_relation.to_owned(), + approver_groups: approvers_groups_with_relation.to_owned(), + approver_individuals: Vec::new(), }]; assert_eq!( expected_requirements, // expected @@ -209,7 +217,8 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( // Scenario 3: create an approval with our relation intact. Our new approval should satisfy the // requirements. let third_approval_id = { - let approving_ids = dal_wrapper::determine_approving_ids(ctx, &mut spicedb_client).await?; + let approving_ids = + dal_wrapper::determine_approving_ids_with_hashes(ctx, &mut spicedb_client).await?; let third_approval = ChangeSetApproval::new(ctx, ChangeSetApprovalStatus::Approved, approving_ids).await?; ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; @@ -235,7 +244,7 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( ); let mut frontend_requirements = calculator - .frontend_requirements(ctx, &mut spicedb_client) + .frontend_requirements(&mut spicedb_client) .await?; frontend_requirements .iter_mut() @@ -250,7 +259,8 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( second_approval_id, third_approval.id(), ], - approving_groups: approving_groups_with_relation.to_owned(), + approver_groups: approvers_groups_with_relation.to_owned(), + approver_individuals: Vec::new(), }]; assert_eq!( expected_requirements, // expected @@ -287,15 +297,16 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( ); let frontend_requirements = calculator - .frontend_requirements(ctx, &mut spicedb_client) + .frontend_requirements(&mut spicedb_client) .await?; let expected_requirements = vec![si_frontend_types::ChangeSetApprovalRequirement { entity_id, entity_kind, required_count, is_satisfied: false, - applicable_approval_ids: vec![], - approving_groups: approving_groups_without_relation.to_owned(), + applicable_approval_ids: Vec::new(), + approver_groups: approvers_groups_without_relation.to_owned(), + approver_individuals: Vec::new(), }]; assert_eq!( expected_requirements, // expected @@ -329,7 +340,7 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( ); let mut frontend_requirements = calculator - .frontend_requirements(ctx, &mut spicedb_client) + .frontend_requirements(&mut spicedb_client) .await?; frontend_requirements .iter_mut() @@ -340,7 +351,8 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( required_count, is_satisfied: true, applicable_approval_ids: vec![first_approval_id, second_approval_id, third_approval_id], - approving_groups: approving_groups_with_relation, + approver_groups: approvers_groups_with_relation, + approver_individuals: Vec::new(), }]; assert_eq!( expected_requirements, // expected @@ -350,3 +362,368 @@ async fn single_user_relation_existence_and_checksum_validility_permutations( Ok(()) } + +// NOTE(nick): this is an integration test and not a service test, but given that "sdf_test" is in +// a weird, unused place at the time of writing, this test will live here. +#[sdf_test] +async fn individual_approver_for_view( + ctx: &mut DalContext, + spicedb_client: SpiceDbClient, +) -> Result<()> { + let mut spicedb_client = spicedb_client; + + // FIXME(nick,jacob): see the comment attached to this function. + write_schema(&mut spicedb_client).await?; + + // Cache the IDs we need. + let workspace_id = ctx.workspace_pk()?; + let user_id = match ctx.history_actor() { + HistoryActor::SystemInit => return Err(eyre!("invalid user")), + HistoryActor::User(user_id) => *user_id, + }; + let view_id = View::get_id_for_default(ctx).await?; + + // Scenario 1: see all approvals and requirements with an "empty" workspace. + { + let calculator = ChangeSetApprovalCalculator::new(ctx, &mut spicedb_client).await?; + let frontend_latest_approvals = calculator.frontend_latest_approvals(); + let frontend_requirements = calculator + .frontend_requirements(&mut spicedb_client) + .await?; + + assert!(frontend_latest_approvals.is_empty()); + assert!(frontend_requirements.is_empty()); + } + + // Scenario 2: add the approval requirement to the default view with ourself as the individual approver. + let ( + view_entity_id, + approval_requirement_definition_entity_id, + approval_requirement_definition_id, + ) = { + let approval_requirement_definition_id = ApprovalRequirement::new_definition( + ctx, + view_id, + 1, + HashSet::from([ApprovalRequirementApprover::User(user_id)]), + ) + .await?; + let (view_entity_id, approval_requirement_definition_entity_id) = ( + view_id.into_inner().into(), + approval_requirement_definition_id.into_inner().into(), + ); + ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; + + let calculator = ChangeSetApprovalCalculator::new(ctx, &mut spicedb_client).await?; + let frontend_latest_approvals = calculator.frontend_latest_approvals(); + let mut frontend_requirements = calculator + .frontend_requirements(&mut spicedb_client) + .await?; + frontend_requirements.sort_by_key(|r| r.entity_id); + + assert!(frontend_latest_approvals.is_empty()); + assert_eq!( + vec![ + si_frontend_types::ChangeSetApprovalRequirement { + entity_id: view_entity_id, + entity_kind: EntityKind::View, + required_count: 1, + is_satisfied: false, + applicable_approval_ids: Vec::new(), + approver_groups: HashMap::new(), + approver_individuals: vec![user_id] + }, + si_frontend_types::ChangeSetApprovalRequirement { + entity_id: approval_requirement_definition_entity_id, + entity_kind: EntityKind::ApprovalRequirementDefinition, + required_count: 1, + is_satisfied: false, + applicable_approval_ids: Vec::new(), + approver_groups: HashMap::from_iter(vec![( + format!("workspace#{workspace_id}#approve"), + Vec::new() + )]), + approver_individuals: Vec::new(), + } + ], // expected + frontend_requirements // actual + ); + + ( + view_entity_id, + approval_requirement_definition_entity_id, + approval_requirement_definition_id, + ) + }; + + // Scenario 3: create an approval that will satisfy the view requirement, but not the + // definition requirement. + let first_approval_id = { + let first_approval_id = { + let approving_ids = + dal_wrapper::determine_approving_ids_with_hashes(ctx, &mut spicedb_client).await?; + let first_approval = + ChangeSetApproval::new(ctx, ChangeSetApprovalStatus::Approved, approving_ids) + .await?; + first_approval.id() + }; + ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; + + let calculator = ChangeSetApprovalCalculator::new(ctx, &mut spicedb_client).await?; + let frontend_latest_approvals = calculator.frontend_latest_approvals(); + let mut frontend_requirements = calculator + .frontend_requirements(&mut spicedb_client) + .await?; + frontend_requirements.sort_by_key(|r| r.entity_id); + + assert_eq!( + vec![si_frontend_types::ChangeSetApproval { + id: first_approval_id, + user_id, + status: ChangeSetApprovalStatus::Approved, + is_valid: true, + }], // expected + frontend_latest_approvals // actual + ); + assert_eq!( + vec![ + si_frontend_types::ChangeSetApprovalRequirement { + entity_id: view_entity_id, + entity_kind: EntityKind::View, + required_count: 1, + is_satisfied: true, + applicable_approval_ids: vec![first_approval_id], + approver_groups: HashMap::new(), + approver_individuals: vec![user_id] + }, + si_frontend_types::ChangeSetApprovalRequirement { + entity_id: approval_requirement_definition_entity_id, + entity_kind: EntityKind::ApprovalRequirementDefinition, + required_count: 1, + is_satisfied: false, + applicable_approval_ids: Vec::new(), + approver_groups: HashMap::from_iter(vec![( + format!("workspace#{workspace_id}#approve"), + Vec::new() + )]), + approver_individuals: Vec::new(), + } + ], // expected + frontend_requirements // actual + ); + + first_approval_id + }; + + // Scenario 4: add ourself as a workspace approver. The original approval should no longer + // satisfy the view requirement because the IDs we are approving has changed. It would also + // not satisfy the definition requirement for the same reason. + { + let relation = RelationBuilder::new() + .object(ObjectType::Workspace, workspace_id) + .relation(Relation::Approver) + .subject(ObjectType::User, user_id); + relation.create(&mut spicedb_client).await?; + + let calculator = ChangeSetApprovalCalculator::new(ctx, &mut spicedb_client).await?; + let frontend_latest_approvals = calculator.frontend_latest_approvals(); + let mut frontend_requirements = calculator + .frontend_requirements(&mut spicedb_client) + .await?; + frontend_requirements.sort_by_key(|r| r.entity_id); + + assert_eq!( + vec![si_frontend_types::ChangeSetApproval { + id: first_approval_id, + user_id, + status: ChangeSetApprovalStatus::Approved, + is_valid: false, + }], // expected + frontend_latest_approvals // actual + ); + assert_eq!( + vec![ + si_frontend_types::ChangeSetApprovalRequirement { + entity_id: view_entity_id, + entity_kind: EntityKind::View, + required_count: 1, + is_satisfied: false, + applicable_approval_ids: vec![first_approval_id], + approver_groups: HashMap::new(), + approver_individuals: vec![user_id] + }, + si_frontend_types::ChangeSetApprovalRequirement { + entity_id: approval_requirement_definition_entity_id, + entity_kind: EntityKind::ApprovalRequirementDefinition, + required_count: 1, + is_satisfied: false, + applicable_approval_ids: vec![first_approval_id], + approver_groups: HashMap::from_iter(vec![( + format!("workspace#{workspace_id}#approve"), + vec![user_id], + )]), + approver_individuals: Vec::new(), + } + ], // expected + frontend_requirements // actual + ); + }; + + // Scenario 5: create an approval that will satisfy both requirements. + { + let second_approval_id = { + let approving_ids = + dal_wrapper::determine_approving_ids_with_hashes(ctx, &mut spicedb_client).await?; + let second_approval = + ChangeSetApproval::new(ctx, ChangeSetApprovalStatus::Approved, approving_ids) + .await?; + second_approval.id() + }; + ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; + + let calculator = ChangeSetApprovalCalculator::new(ctx, &mut spicedb_client).await?; + let mut frontend_latest_approvals = calculator.frontend_latest_approvals(); + let mut frontend_requirements = calculator + .frontend_requirements(&mut spicedb_client) + .await?; + frontend_latest_approvals.sort_by_key(|a| a.id); + frontend_requirements.sort_by_key(|r| r.entity_id); + frontend_requirements + .iter_mut() + .for_each(|r| r.applicable_approval_ids.sort()); + + assert_eq!( + vec![ + si_frontend_types::ChangeSetApproval { + id: first_approval_id, + user_id, + status: ChangeSetApprovalStatus::Approved, + is_valid: false, + }, + si_frontend_types::ChangeSetApproval { + id: second_approval_id, + user_id, + status: ChangeSetApprovalStatus::Approved, + is_valid: true, + } + ], // expected + frontend_latest_approvals // actual + ); + assert_eq!( + vec![ + si_frontend_types::ChangeSetApprovalRequirement { + entity_id: view_entity_id, + entity_kind: EntityKind::View, + required_count: 1, + is_satisfied: true, + applicable_approval_ids: vec![first_approval_id, second_approval_id], + approver_groups: HashMap::new(), + approver_individuals: vec![user_id] + }, + si_frontend_types::ChangeSetApprovalRequirement { + entity_id: approval_requirement_definition_entity_id, + entity_kind: EntityKind::ApprovalRequirementDefinition, + required_count: 1, + is_satisfied: true, + applicable_approval_ids: vec![first_approval_id, second_approval_id], + approver_groups: HashMap::from_iter(vec![( + format!("workspace#{workspace_id}#approve"), + vec![user_id], + )]), + approver_individuals: Vec::new(), + } + ], // expected + frontend_requirements // actual + ); + } + + // Scenario 6: apply the change set, create a new change set and observe that no approvals nor requirements exist. + { + ChangeSetTestHelpers::apply_change_set_to_base(ctx).await?; + ChangeSetTestHelpers::fork_from_head_change_set(ctx).await?; + + let calculator = ChangeSetApprovalCalculator::new(ctx, &mut spicedb_client).await?; + let frontend_latest_approvals = calculator.frontend_latest_approvals(); + let frontend_requirements = calculator + .frontend_requirements(&mut spicedb_client) + .await?; + + assert!(frontend_latest_approvals.is_empty()); + assert!(frontend_requirements.is_empty()); + } + + // Scenario 7: remove the definiton from the view. + { + ApprovalRequirement::remove_definition(ctx, approval_requirement_definition_id).await?; + ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; + + let calculator = ChangeSetApprovalCalculator::new(ctx, &mut spicedb_client).await?; + let frontend_latest_approvals = calculator.frontend_latest_approvals(); + let frontend_requirements = calculator + .frontend_requirements(&mut spicedb_client) + .await?; + + assert!(frontend_latest_approvals.is_empty()); + assert_eq!( + vec![si_frontend_types::ChangeSetApprovalRequirement { + entity_id: approval_requirement_definition_entity_id, + entity_kind: EntityKind::ApprovalRequirementDefinition, + required_count: 1, + is_satisfied: false, + applicable_approval_ids: Vec::new(), + approver_groups: HashMap::from_iter(vec![( + format!("workspace#{workspace_id}#approve"), + vec![user_id], + )]), + approver_individuals: Vec::new(), + }], // expected + frontend_requirements // actual + ); + } + + // Scenario 8: approve the removal. + { + let third_approval_id = { + let approving_ids = + dal_wrapper::determine_approving_ids_with_hashes(ctx, &mut spicedb_client).await?; + let third_approval = + ChangeSetApproval::new(ctx, ChangeSetApprovalStatus::Approved, approving_ids) + .await?; + third_approval.id() + }; + ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; + + let calculator = ChangeSetApprovalCalculator::new(ctx, &mut spicedb_client).await?; + let frontend_latest_approvals = calculator.frontend_latest_approvals(); + let frontend_requirements = calculator + .frontend_requirements(&mut spicedb_client) + .await?; + + assert_eq!( + vec![si_frontend_types::ChangeSetApproval { + id: third_approval_id, + user_id, + status: ChangeSetApprovalStatus::Approved, + is_valid: true, + },], // expected + frontend_latest_approvals // actual + ); + assert_eq!( + vec![si_frontend_types::ChangeSetApprovalRequirement { + entity_id: approval_requirement_definition_entity_id, + entity_kind: EntityKind::ApprovalRequirementDefinition, + required_count: 1, + is_satisfied: true, + applicable_approval_ids: vec![third_approval_id], + approver_groups: HashMap::from_iter(vec![( + format!("workspace#{workspace_id}#approve"), + vec![user_id], + )]), + approver_individuals: Vec::new(), + }], // expected + frontend_requirements // actual + ); + } + + Ok(()) +} diff --git a/lib/si-events-rs/src/workspace_snapshot.rs b/lib/si-events-rs/src/workspace_snapshot.rs index fba816387e..d0b9ec6970 100644 --- a/lib/si-events-rs/src/workspace_snapshot.rs +++ b/lib/si-events-rs/src/workspace_snapshot.rs @@ -10,6 +10,7 @@ create_xxhash_type!(Checksum); pub enum EntityKind { Action, ActionPrototype, + ApprovalRequirementDefinition, AttributePrototype, AttributePrototypeArgument, AttributeValue, diff --git a/lib/si-frontend-types-rs/src/change_set.rs b/lib/si-frontend-types-rs/src/change_set.rs index 8fe9c2c0ce..f66d61ea1b 100644 --- a/lib/si-frontend-types-rs/src/change_set.rs +++ b/lib/si-frontend-types-rs/src/change_set.rs @@ -58,11 +58,12 @@ pub struct ChangeSetApprovalRequirement { // Which approvals are for this requirement? pub applicable_approval_ids: Vec, // What groups can approve this? - // NOTE(nick,jacob): this is almost certainly not the right place to send up the users as well. - pub approving_groups: HashMap>, + pub approver_groups: HashMap>, + // What individuals can approve this? + pub approver_individuals: Vec, } -#[derive(Debug, Clone, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] #[serde(rename_all = "camelCase")] pub struct ChangeSetApproval { // What approval is this? diff --git a/lib/si-id/src/lib.rs b/lib/si-id/src/lib.rs index 23c481ef7d..4eacce9303 100644 --- a/lib/si-id/src/lib.rs +++ b/lib/si-id/src/lib.rs @@ -35,6 +35,7 @@ pub use ::ulid as ulid_upstream; // Please keep these alphabetically sorted! id!(ActionPrototypeId); id!(ActivityId); +id!(ApprovalRequirementDefinitionId); id!(AttributePrototypeArgumentId); id!(AttributePrototypeId); id!(AuthenticationPrototypeId);