From 16135675bca2d53b8922ca23bf83ecf813f19fb0 Mon Sep 17 00:00:00 2001 From: Kiril Videlov Date: Mon, 21 Oct 2024 11:28:57 +0200 Subject: [PATCH] Reorder stack API stub --- .../gitbutler-branch-actions/src/actions.rs | 12 + crates/gitbutler-branch-actions/src/lib.rs | 12 +- .../gitbutler-branch-actions/src/reorder.rs | 345 ++++++++++++++++++ .../gitbutler-tauri/src/virtual_branches.rs | 16 +- 4 files changed, 379 insertions(+), 6 deletions(-) create mode 100644 crates/gitbutler-branch-actions/src/reorder.rs diff --git a/crates/gitbutler-branch-actions/src/actions.rs b/crates/gitbutler-branch-actions/src/actions.rs index b34dca6d1..3c43a48cc 100644 --- a/crates/gitbutler-branch-actions/src/actions.rs +++ b/crates/gitbutler-branch-actions/src/actions.rs @@ -1,6 +1,7 @@ use super::r#virtual as vbranch; use crate::branch_upstream_integration; use crate::move_commits; +use crate::reorder::{self, StackOrder}; use crate::reorder_commits; use crate::upstream_integration::{ self, BaseBranchResolution, BaseBranchResolutionApproach, BranchStatuses, Resolution, @@ -349,6 +350,17 @@ pub fn insert_blank_commit( vbranch::insert_blank_commit(&ctx, branch_id, commit_oid, offset).map_err(Into::into) } +pub fn reorder_stack(project: &Project, stack_id: StackId, stack_order: StackOrder) -> Result<()> { + let ctx = open_with_verify(project)?; + assure_open_workspace_mode(&ctx).context("Reordering a commit requires open workspace mode")?; + let mut guard = project.exclusive_worktree_access(); + let _ = ctx.project().create_snapshot( + SnapshotDetails::new(OperationKind::ReorderCommit), + guard.write_permission(), + ); + reorder::reorder_stack(&ctx, stack_id, stack_order, guard.write_permission()) +} + pub fn reorder_commit( project: &Project, branch_id: StackId, diff --git a/crates/gitbutler-branch-actions/src/lib.rs b/crates/gitbutler-branch-actions/src/lib.rs index 0322c532d..1977af38a 100644 --- a/crates/gitbutler-branch-actions/src/lib.rs +++ b/crates/gitbutler-branch-actions/src/lib.rs @@ -8,11 +8,11 @@ pub use actions::{ get_uncommited_files_reusable, insert_blank_commit, integrate_upstream, integrate_upstream_commits, list_local_branches, list_remote_commit_files, list_virtual_branches, list_virtual_branches_cached, move_commit, move_commit_file, - push_base_branch, push_virtual_branch, reorder_commit, reset_files, reset_virtual_branch, - resolve_upstream_integration, save_and_unapply_virutal_branch, set_base_branch, - set_target_push_remote, squash, unapply_ownership, unapply_without_saving_virtual_branch, - undo_commit, update_branch_order, update_commit_message, update_virtual_branch, - upstream_integration_statuses, + push_base_branch, push_virtual_branch, reorder_commit, reorder_stack, reset_files, + reset_virtual_branch, resolve_upstream_integration, save_and_unapply_virutal_branch, + set_base_branch, set_target_push_remote, squash, unapply_ownership, + unapply_without_saving_virtual_branch, undo_commit, update_branch_order, update_commit_message, + update_virtual_branch, upstream_integration_statuses, }; mod r#virtual; @@ -47,6 +47,8 @@ pub mod conflicts; pub mod branch_trees; pub mod branch_upstream_integration; mod move_commits; +mod reorder; +pub use reorder::{SeriesOrder, StackOrder}; mod reorder_commits; mod undo_commit; diff --git a/crates/gitbutler-branch-actions/src/reorder.rs b/crates/gitbutler-branch-actions/src/reorder.rs new file mode 100644 index 000000000..ed58df686 --- /dev/null +++ b/crates/gitbutler-branch-actions/src/reorder.rs @@ -0,0 +1,345 @@ +use anyhow::{bail, Result}; +use git2::Oid; +use gitbutler_command_context::CommandContext; +use gitbutler_project::access::WorktreeWritePermission; +use gitbutler_stack::{Series, StackId}; + +use itertools::Itertools; +use serde::Serialize; + +use crate::VirtualBranchesExt; + +/// This API allows the client to reorder commits in a stack. +/// Commits may be moved within the same series or between different series. +/// Moving of series is not permitted. +/// +/// # Errors +/// Errors out upon invalid stack order input. The following conditions are checked: +/// - The number of series in the order must match the number of series in the stack +/// - The series names in the reorder request must match the names in the stack +/// - The series themselves in the reorder request must be the same as the ones in the stack (this API is about moving commits, not series) +/// - The number of commits in the reorder request must match the number of commits in the stack +/// - The commit ids in the reorder request must be in the stack +pub fn reorder_stack( + ctx: &CommandContext, + branch_id: StackId, + stack_order: StackOrder, + _perm: &mut WorktreeWritePermission, +) -> Result<()> { + let state = ctx.project().virtual_branches(); + let stack = state.get_branch(branch_id)?; + let all_series = stack.list_series(ctx)?; + stack_order.validate(series_order(&all_series))?; + Ok(()) +} + +/// Represents the order of series (branches) and changes (commits) in a stack. +#[derive(Debug, PartialEq, Eq, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct StackOrder { + /// The series are ordered from newest to oldest (most recent stacks go first) + series: Vec, +} + +/// Represents the order of changes (commits) in a series (branch). +#[derive(Debug, PartialEq, Eq, Clone, Serialize)] +pub struct SeriesOrder { + /// Unique name of the series (branch). Must already exist in the stack. + name: String, + /// This is the desired commit order for the series. Because the commits will be rabased, + /// naturally, the the commit ids will be different afte updating. + /// The changes are ordered from newest to oldest (most recent changes go first) + #[serde(with = "gitbutler_serde::oid_vec")] + commit_ids: Vec, +} + +impl StackOrder { + fn validate(&self, current_order: StackOrder) -> Result<()> { + // Ensure the number of series is the same between the reorder update request and the stack + if self.series.len() != current_order.series.len() { + bail!( + "The number of series in the order ({}) does not match the number of series in the stack ({})", + self.series.len(), + current_order.series.len() + ); + } + // Ensure that the names in the reorder update request match the names in the stack + for series_order in &self.series { + if !current_order + .series + .iter() + .any(|s| s.name == series_order.name) + { + bail!("Series '{}' does not exist in the stack", series_order.name); + } + } + // Ensure that the series themselves in the updater request are the same as the ones in the stack (this API is about moving commits, not series) + for (new_order, current_order) in self.series.iter().zip(current_order.series.iter()) { + if new_order.name != current_order.name { + bail!( + "Series '{}' in the order does not match the series '{}' in the stack. Series can't be reordered with this API, it's only for commits", + new_order.name, + current_order.name + ); + } + } + + let new_order_commit_ids = self + .series + .iter() + .flat_map(|s| s.commit_ids.iter()) + .cloned() + .collect_vec(); + let current_order_commit_ids = current_order + .series + .iter() + .flat_map(|s| s.commit_ids.iter()) + .cloned() + .collect_vec(); + + // Ensure that the number of commits in the order is the same as the number of commits in the stack + if new_order_commit_ids.len() != current_order_commit_ids.len() { + bail!( + "The number of commits in the request order ({}) does not match the number of commits in the stack ({})", + new_order_commit_ids.len(), + current_order_commit_ids.len() + ); + } + // Ensure that every commit in the order is in the stack + for commit_id in &new_order_commit_ids { + if !current_order_commit_ids.contains(commit_id) { + bail!("Commit '{}' does not exist in the stack", commit_id); + } + } + // Ensure the new order is not a noop + if new_order_commit_ids == current_order_commit_ids { + bail!("The new order is the same as the current order"); + } + + Ok(()) + } +} + +fn series_order(all_series: &[Series<'_>]) -> StackOrder { + let series_order: Vec = all_series + .iter() + .map(|series| { + let commit_ids = series + .local_commits + .iter() + .map(|commit| commit.id()) + .collect(); + SeriesOrder { + name: series.head.name.clone(), + commit_ids, + } + }) + .collect(); + StackOrder { + series: series_order, + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn validation_ok() -> Result<()> { + let new_order = StackOrder { + series: vec![ + SeriesOrder { + name: "branch-2".to_string(), + commit_ids: vec![ + Oid::from_str("6").unwrap(), + Oid::from_str("5").unwrap(), + Oid::from_str("4").unwrap(), + ], + }, + SeriesOrder { + name: "branch-1".to_string(), + commit_ids: vec![ + Oid::from_str("3").unwrap(), + Oid::from_str("1").unwrap(), // swapped with below + Oid::from_str("2").unwrap(), + ], + }, + ], + }; + let result = new_order.validate(existing_order()); + assert!(result.is_ok()); + Ok(()) + } + + #[test] + fn noop_errors_out() -> Result<()> { + let result = existing_order().validate(existing_order()); + assert_eq!( + result.unwrap_err().to_string(), + "The new order is the same as the current order" + ); + Ok(()) + } + + #[test] + fn non_existing_id_errors_out() -> Result<()> { + let new_order = StackOrder { + series: vec![ + SeriesOrder { + name: "branch-2".to_string(), + commit_ids: vec![ + Oid::from_str("6").unwrap(), + Oid::from_str("5").unwrap(), + Oid::from_str("4").unwrap(), + ], + }, + SeriesOrder { + name: "branch-1".to_string(), + commit_ids: vec![ + Oid::from_str("3").unwrap(), + Oid::from_str("9").unwrap(), // does not exist + Oid::from_str("1").unwrap(), + ], + }, + ], + }; + let result = new_order.validate(existing_order()); + assert_eq!( + result.unwrap_err().to_string(), + "Commit '9000000000000000000000000000000000000000' does not exist in the stack" + ); + Ok(()) + } + + #[test] + fn number_of_commits_mismatch_errors_out() -> Result<()> { + let new_order = StackOrder { + series: vec![ + SeriesOrder { + name: "branch-2".to_string(), + commit_ids: vec![ + Oid::from_str("6").unwrap(), + Oid::from_str("5").unwrap(), + Oid::from_str("4").unwrap(), + ], + }, + SeriesOrder { + name: "branch-1".to_string(), + commit_ids: vec![ + Oid::from_str("3").unwrap(), // missing + Oid::from_str("1").unwrap(), + ], + }, + ], + }; + let result = new_order.validate(existing_order()); + assert_eq!( + result.unwrap_err().to_string(), + "The number of commits in the request order (5) does not match the number of commits in the stack (6)" + ); + Ok(()) + } + + #[test] + fn series_out_of_order_errors_out() -> Result<()> { + let new_order = StackOrder { + series: vec![ + SeriesOrder { + name: "branch-1".to_string(), // wrong order + commit_ids: vec![ + Oid::from_str("6").unwrap(), + Oid::from_str("5").unwrap(), + Oid::from_str("4").unwrap(), + ], + }, + SeriesOrder { + name: "branch-2".to_string(), // wrong order + commit_ids: vec![ + Oid::from_str("3").unwrap(), + Oid::from_str("2").unwrap(), + Oid::from_str("1").unwrap(), + ], + }, + ], + }; + let result = new_order.validate(existing_order()); + assert_eq!( + result.unwrap_err().to_string(), + "Series 'branch-1' in the order does not match the series 'branch-2' in the stack. Series can't be reordered with this API, it's only for commits" + ); + Ok(()) + } + + #[test] + fn different_series_name_errors_out() -> Result<()> { + let new_order = StackOrder { + series: vec![ + SeriesOrder { + name: "does-not-exist".to_string(), // invalid series name + commit_ids: vec![ + Oid::from_str("6").unwrap(), + Oid::from_str("5").unwrap(), + Oid::from_str("4").unwrap(), + ], + }, + SeriesOrder { + name: "branch-1".to_string(), + commit_ids: vec![ + Oid::from_str("3").unwrap(), + Oid::from_str("2").unwrap(), + Oid::from_str("1").unwrap(), + ], + }, + ], + }; + let result = new_order.validate(existing_order()); + assert_eq!( + result.unwrap_err().to_string(), + "Series 'does-not-exist' does not exist in the stack" + ); + Ok(()) + } + + #[test] + fn different_number_of_series_errors_out() -> Result<()> { + let new_order = StackOrder { + series: vec![SeriesOrder { + name: "branch-1".to_string(), + commit_ids: vec![ + Oid::from_str("3").unwrap(), + Oid::from_str("2").unwrap(), + Oid::from_str("1").unwrap(), + ], + }], + }; + let result = new_order.validate(existing_order()); + assert_eq!( + result.unwrap_err().to_string(), + "The number of series in the order (1) does not match the number of series in the stack (2)" + ); + Ok(()) + } + + fn existing_order() -> StackOrder { + StackOrder { + series: vec![ + SeriesOrder { + name: "branch-2".to_string(), + commit_ids: vec![ + Oid::from_str("6").unwrap(), + Oid::from_str("5").unwrap(), + Oid::from_str("4").unwrap(), + ], + }, + SeriesOrder { + name: "branch-1".to_string(), + commit_ids: vec![ + Oid::from_str("3").unwrap(), + Oid::from_str("2").unwrap(), + Oid::from_str("1").unwrap(), + ], + }, + ], + } + } +} diff --git a/crates/gitbutler-tauri/src/virtual_branches.rs b/crates/gitbutler-tauri/src/virtual_branches.rs index 06eb3ab79..9ad880915 100644 --- a/crates/gitbutler-tauri/src/virtual_branches.rs +++ b/crates/gitbutler-tauri/src/virtual_branches.rs @@ -7,7 +7,7 @@ pub mod commands { }; use gitbutler_branch_actions::{ BaseBranch, BranchListing, BranchListingDetails, BranchListingFilter, RemoteBranch, - RemoteBranchData, RemoteBranchFile, RemoteCommit, VirtualBranches, + RemoteBranchData, RemoteBranchFile, RemoteCommit, StackOrder, VirtualBranches, }; use gitbutler_command_context::CommandContext; use gitbutler_project as projects; @@ -397,6 +397,20 @@ pub mod commands { Ok(()) } + #[tauri::command(async)] + #[instrument(skip(projects, windows), err(Debug))] + pub fn reorder_stack( + windows: State<'_, WindowState>, + projects: State<'_, projects::Controller>, + project_id: ProjectId, + branch_id: StackId, + stack_order: StackOrder, + ) -> Result<(), Error> { + let project = projects.get(project_id)?; + gitbutler_branch_actions::reorder_stack(&project, branch_id, stack_order)?; + emit_vbranches(&windows, project_id); + Ok(()) + } #[tauri::command(async)] #[instrument(skip(projects, windows), err(Debug))] pub fn reorder_commit(