From 6051164164d1835ae7ab76b60913aa245944937e Mon Sep 17 00:00:00 2001 From: Nick Gerace Date: Wed, 18 Dec 2024 18:12:58 -0500 Subject: [PATCH] WIP: move changes hash contents into approval Signed-off-by: Nick Gerace --- lib/dal/src/change_set/approval.rs | 52 ++++++++++++++++++- .../src/change_set/approval/changes_hash.rs | 27 ---------- lib/dal/src/workspace_snapshot.rs | 18 +++++++ .../integration_test/change_set/approval.rs | 10 ++-- 4 files changed, 74 insertions(+), 33 deletions(-) delete mode 100644 lib/dal/src/change_set/approval/changes_hash.rs diff --git a/lib/dal/src/change_set/approval.rs b/lib/dal/src/change_set/approval.rs index bea6796c59..08b6317412 100644 --- a/lib/dal/src/change_set/approval.rs +++ b/lib/dal/src/change_set/approval.rs @@ -1,6 +1,34 @@ +//! Provides the ability to approve change sets and calculate their approval status. + +#![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, + 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 chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use si_data_pg::{PgError, PgRow}; +use si_events::ChangesHash; use si_id::{ChangeSetApprovalId, ChangeSetId, UserPk}; use telemetry::prelude::*; use thiserror::Error; @@ -9,8 +37,7 @@ pub use si_events::ChangeSetApprovalStatus; use crate::{DalContext, HistoryActor, TransactionsError, WorkspaceSnapshotError}; -pub mod changes_hash; - +#[allow(missing_docs)] #[derive(Debug, Error)] pub enum ChangeSetApprovalError { #[error("invalid user for creating a change set approval")] @@ -27,6 +54,7 @@ pub enum ChangeSetApprovalError { type Result = std::result::Result; +/// An individual approval for applying a [`ChangeSet`](crate::ChangeSet). #[derive(Debug, Serialize, Deserialize)] pub struct ChangeSetApproval { id: ChangeSetApprovalId, @@ -57,6 +85,7 @@ impl TryFrom for ChangeSetApproval { } impl ChangeSetApproval { + /// Creates a new approval. #[instrument(name = "change_set.approval.new", level = "info", skip_all)] pub async fn new( ctx: &DalContext, @@ -80,22 +109,27 @@ impl ChangeSetApproval { Self::try_from(row) } + /// Returns the ID of the approval. pub fn id(&self) -> ChangeSetApprovalId { self.id } + /// Returns the status of the approval. pub fn status(&self) -> ChangeSetApprovalStatus { self.status } + /// Returns the ID of the approver. pub fn user_id(&self) -> UserPk { self.user_id } + /// Returns the checksum based on the changes compared to HEAD when the approval was performed. pub fn checksum(&self) -> &str { self.checksum.as_str() } + /// Lists all approvals in the [`ChangeSet`](crate::ChangeSet). #[instrument(name = "change_set.approval.list", level = "info", skip_all)] pub async fn list(ctx: &DalContext) -> Result> { let change_set_id = ctx.change_set_id(); @@ -114,4 +148,18 @@ impl ChangeSetApproval { } Ok(approvals) } + + /// Generates a checksum for changes in the current [`ChangeSet`](crate::ChangeSet). + pub async fn generate_checksum(ctx: &DalContext) -> Result { + let mut changes = ctx + .workspace_snapshot()? + .detect_changes_from_head(ctx) + .await?; + changes.sort_by_key(|c| c.id); + let mut hasher = ChangesHash::hasher(); + for change in changes { + hasher.update(change.merkle_tree_hash.as_bytes()); + } + Ok(hasher.finalize()) + } } diff --git a/lib/dal/src/change_set/approval/changes_hash.rs b/lib/dal/src/change_set/approval/changes_hash.rs deleted file mode 100644 index 9cc1e2de9a..0000000000 --- a/lib/dal/src/change_set/approval/changes_hash.rs +++ /dev/null @@ -1,27 +0,0 @@ -use si_events::ChangesHash; - -use crate::{workspace_snapshot::graph::detector::Change, DalContext, WorkspaceSnapshot}; - -use super::ChangeSetApprovalError; - -type Result = std::result::Result; - -pub async fn changes_hash(ctx: &DalContext) -> Result { - let changes = detect_changes_from_head(ctx).await?; - let mut hasher = ChangesHash::hasher(); - for change in changes { - hasher.update(change.merkle_tree_hash.as_bytes()); - } - Ok(hasher.finalize()) -} - -pub async fn detect_changes_from_head(ctx: &DalContext) -> Result> { - let head_change_set_id = ctx.get_workspace_default_change_set_id().await?; - let head_snapshot = WorkspaceSnapshot::find_for_change_set(&ctx, head_change_set_id).await?; - let mut changes = head_snapshot - .detect_changes(&ctx.workspace_snapshot()?.clone()) - .await?; - - changes.sort_by_key(|c| c.id); - Ok(changes) -} diff --git a/lib/dal/src/workspace_snapshot.rs b/lib/dal/src/workspace_snapshot.rs index 5ff8a54527..85e2c9ffcd 100644 --- a/lib/dal/src/workspace_snapshot.rs +++ b/lib/dal/src/workspace_snapshot.rs @@ -747,6 +747,24 @@ impl WorkspaceSnapshot { .await?) } + /// A wrapper around [`Self::detect_changes`](Self::detect_changes) where the "onto" snapshot is derived from the + /// workspace's default [`ChangeSet`](crate::ChangeSet). + #[instrument( + name = "workspace_snapshot.detect_changes_from_head", + level = "debug", + skip_all + )] + pub async fn detect_changes_from_head( + &self, + ctx: &DalContext, + ) -> WorkspaceSnapshotResult> { + let head_change_set_id = ctx.get_workspace_default_change_set_id().await?; + let head_snapshot = Self::find_for_change_set(&ctx, head_change_set_id).await?; + Ok(head_snapshot + .detect_changes(&ctx.workspace_snapshot()?.clone()) + .await?) + } + /// Gives the exact node index endpoints of an edge. pub async fn edge_endpoints( &self, diff --git a/lib/dal/tests/integration_test/change_set/approval.rs b/lib/dal/tests/integration_test/change_set/approval.rs index 29831a322a..ca47026bd0 100644 --- a/lib/dal/tests/integration_test/change_set/approval.rs +++ b/lib/dal/tests/integration_test/change_set/approval.rs @@ -1,6 +1,6 @@ use std::collections::HashSet; -use dal::change_set::approval::{self, ChangeSetApproval, ChangeSetApprovalStatus}; +use dal::change_set::approval::{ChangeSetApproval, ChangeSetApprovalStatus}; use dal::{DalContext, Ulid}; use dal_test::color_eyre::eyre::OptionExt; use dal_test::helpers::{ @@ -15,8 +15,7 @@ async fn new(ctx: &mut DalContext) -> Result<()> { ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; let status = ChangeSetApprovalStatus::Approved; - let hash = approval::changes_hash::changes_hash(ctx).await?; - let checksum = hash.to_string(); + let checksum = ChangeSetApproval::generate_checksum(ctx).await?.to_string(); let new_approval = ChangeSetApproval::new(ctx, status, checksum).await?; assert_eq!( @@ -45,7 +44,10 @@ async fn status(ctx: &mut DalContext) -> Result<()> { .await?; ChangeSetTestHelpers::commit_and_update_snapshot_to_visibility(ctx).await?; - let changes = approval::changes_hash::detect_changes_from_head(ctx).await?; + let changes = ctx + .workspace_snapshot()? + .detect_changes_from_head(ctx) + .await?; let seen: HashSet = HashSet::from_iter(changes.iter().map(|c| c.id)); assert_eq!( changes.len(), // expected