From e3156ba75c9357f5244364d610db9753b881e54f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 26 May 2024 14:34:45 +0200 Subject: [PATCH] remove single-use traits while leaving the structure as is --- crates/gitbutler-cli/src/main.rs | 2 +- crates/gitbutler-core/src/ops/oplog.rs | 111 ++++++++---------- crates/gitbutler-core/src/ops/snapshot.rs | 39 ++---- crates/gitbutler-core/src/synchronize/mod.rs | 1 - .../src/virtual_branches/controller.rs | 6 +- .../src/virtual_branches/virtual.rs | 1 - .../tests/suite/virtual_branches/oplog.rs | 2 - crates/gitbutler-tauri/src/undo.rs | 2 +- crates/gitbutler-watcher/src/handler.rs | 1 - 9 files changed, 65 insertions(+), 100 deletions(-) diff --git a/crates/gitbutler-cli/src/main.rs b/crates/gitbutler-cli/src/main.rs index 0f418b459..5397fb92b 100644 --- a/crates/gitbutler-cli/src/main.rs +++ b/crates/gitbutler-cli/src/main.rs @@ -1,5 +1,5 @@ use anyhow::Result; -use gitbutler_core::{ops::oplog::Oplog, projects::Project}; +use gitbutler_core::projects::Project; use clap::{arg, Command}; #[cfg(not(windows))] diff --git a/crates/gitbutler-core/src/ops/oplog.rs b/crates/gitbutler-core/src/ops/oplog.rs index 554bc8822..ea36c2fe0 100644 --- a/crates/gitbutler-core/src/ops/oplog.rs +++ b/crates/gitbutler-core/src/ops/oplog.rs @@ -21,7 +21,7 @@ use super::{ const SNAPSHOT_FILE_LIMIT_BYTES: u64 = 32 * 1024 * 1024; -/// The Oplog trait allows for crating snapshots of the current state of the project as well as restoring to a previous snapshot. +/// The Oplog allows for crating snapshots of the current state of the project as well as restoring to a previous snapshot. /// Snapshots include the state of the working directory as well as all additional GitButler state (e.g virtual branches, conflict state). /// The data is stored as git trees in the following shape: /// . @@ -34,60 +34,11 @@ const SNAPSHOT_FILE_LIMIT_BYTES: u64 = 32 * 1024 * 1024; /// │ ├── commit-message.txt /// │ └── tree (subtree) /// └── virtual_branches.toml -pub trait Oplog { +impl Project { /// Prepares a snapshot of the current state of the working directory as well as GitButler data. /// Returns a tree sha of the snapshot. The snapshot is not discoverable until it is comitted with `commit_snapshot` - /// If there are files that are untracked and larger than SNAPSHOT_FILE_LIMIT_BYTES, they are excluded from snapshot creation and restoring. - fn prepare_snapshot(&self) -> Result; - /// Commits the snapshot tree that is created with the `prepare_snapshot` method. - /// Committing it makes the snapshot discoverable in `list_snapshots` as well as restorable with `restore_snapshot`. - /// Returns the sha of the created snapshot commit or None if snapshots are disabled. - fn commit_snapshot( - &self, - snapshot_tree_sha: String, - details: SnapshotDetails, - ) -> Result>; - /// Creates a snapshot of the current state of the working directory as well as GitButler data. - /// This is a convinience method that combines `prepare_snapshot` and `commit_snapshot`. - fn create_snapshot(&self, details: SnapshotDetails) -> Result>; - /// Lists the snapshots that have been created for the given repository, up to the given limit. - /// An alternative way of retrieving the snapshots would be to manually the oplog head `git log ` available in `.git/gitbutler/operations-log.toml`. - /// - /// If there are no snapshots, an empty list is returned. - fn list_snapshots(&self, limit: usize, sha: Option) -> Result>; - /// Reverts to a previous state of the working directory, virtual branches and commits. - /// The provided sha must refer to a valid snapshot commit. - /// Upon success, a new snapshot is created. - /// - /// This will restore the following: - /// - The state of the working directory is checked out from the subtree `workdir` in the snapshot. - /// - The state of virtual branches is restored from the blob `virtual_branches.toml` in the snapshot. - /// - The state of conflicts (.git/base_merge_parent and .git/conflicts) is restored from the subtree `conflicts` in the snapshot (if not present, existing files are deleted). - /// - /// If there are files that are untracked and larger than SNAPSHOT_FILE_LIMIT_BYTES, they are excluded from snapshot creation and restoring. - /// Returns the sha of the created revert snapshot commit or None if snapshots are disabled. - fn restore_snapshot(&self, sha: String) -> Result>; - /// Determines if a new snapshot should be created due to file changes being created since the last snapshot. - /// The needs for the automatic snapshotting are: - /// - It needs to facilitate backup of work in progress code - /// - The snapshots should not be too frequent or small - both for UX and performance reasons - /// - Checking if an automatic snapshot is needed should be fast and efficient since it is called on filesystem events - /// - /// This implementation works as follows: - /// - If it's been more than 5 minutes since the last snapshot, - /// check the sum of added and removed lines since the last snapshot, otherwise return false. - /// - If the sum of added and removed lines is greater than a configured threshold, return true, otherwise return false. - fn should_auto_snapshot(&self) -> Result; - /// Returns the diff of the snapshot and it's parent. It only includes the workdir changes. - /// - /// This is useful to show what has changed in this particular snapshot - fn snapshot_diff(&self, sha: String) -> Result>; - /// Gets the sha of the last snapshot commit if present. - fn oplog_head(&self) -> Result>; -} - -impl Oplog for Project { - fn prepare_snapshot(&self) -> Result { + /// If there are files that are untracked and larger than `SNAPSHOT_FILE_LIMIT_BYTES`, they are excluded from snapshot creation and restoring. + pub(crate) fn prepare_snapshot(&self) -> Result { let repo_path = self.path.as_path(); let repo = git2::Repository::init(repo_path)?; @@ -248,7 +199,14 @@ impl Oplog for Project { Ok(tree_id.to_string()) } - fn commit_snapshot(&self, tree_id: String, details: SnapshotDetails) -> Result> { + /// Commits the snapshot tree that is created with the `prepare_snapshot` method. + /// Committing it makes the snapshot discoverable in `list_snapshots` as well as restorable with `restore_snapshot`. + /// Returns the sha of the created snapshot commit or None if snapshots are disabled. + pub(crate) fn commit_snapshot( + &self, + tree_id: String, + details: SnapshotDetails, + ) -> Result> { let repo_path = self.path.as_path(); let repo = git2::Repository::init(repo_path)?; @@ -306,14 +264,21 @@ impl Oplog for Project { Ok(Some(new_commit_oid.to_string())) } + /// Creates a snapshot of the current state of the working directory as well as GitButler data. + /// This is a convinience method that combines `prepare_snapshot` and `commit_snapshot`. + /// /// Note that errors in snapshot creation is typically ignored, so we want to learn about them. #[instrument(skip(details), err(Debug))] - fn create_snapshot(&self, details: SnapshotDetails) -> Result> { + pub fn create_snapshot(&self, details: SnapshotDetails) -> Result> { let tree_id = self.prepare_snapshot()?; self.commit_snapshot(tree_id, details) } - fn list_snapshots(&self, limit: usize, sha: Option) -> Result> { + /// Lists the snapshots that have been created for the given repository, up to the given limit. + /// An alternative way of retrieving the snapshots would be to manually the oplog head `git log ` available in `.git/gitbutler/operations-log.toml`. + /// + /// If there are no snapshots, an empty list is returned. + pub fn list_snapshots(&self, limit: usize, sha: Option) -> Result> { let repo_path = self.path.as_path(); let repo = git2::Repository::init(repo_path)?; @@ -408,7 +373,18 @@ impl Oplog for Project { Ok(snapshots) } - fn restore_snapshot(&self, sha: String) -> Result> { + /// Reverts to a previous state of the working directory, virtual branches and commits. + /// The provided sha must refer to a valid snapshot commit. + /// Upon success, a new snapshot is created. + /// + /// This will restore the following: + /// - The state of the working directory is checked out from the subtree `workdir` in the snapshot. + /// - The state of virtual branches is restored from the blob `virtual_branches.toml` in the snapshot. + /// - The state of conflicts (.git/base_merge_parent and .git/conflicts) is restored from the subtree `conflicts` in the snapshot (if not present, existing files are deleted). + /// + /// If there are files that are untracked and larger than `SNAPSHOT_FILE_LIMIT_BYTES`, they are excluded from snapshot creation and restoring. + /// Returns the sha of the created revert snapshot commit or None if snapshots are disabled. + pub fn restore_snapshot(&self, sha: String) -> Result> { let repo_path = self.path.as_path(); let repo = git2::Repository::init(repo_path)?; @@ -577,7 +553,17 @@ impl Oplog for Project { snapshot_tree.and_then(|snapshot_tree| self.commit_snapshot(snapshot_tree, details)) } - fn should_auto_snapshot(&self) -> Result { + /// Determines if a new snapshot should be created due to file changes being created since the last snapshot. + /// The needs for the automatic snapshotting are: + /// - It needs to facilitate backup of work in progress code + /// - The snapshots should not be too frequent or small - both for UX and performance reasons + /// - Checking if an automatic snapshot is needed should be fast and efficient since it is called on filesystem events + /// + /// This implementation works as follows: + /// - If it's been more than 5 minutes since the last snapshot, + /// check the sum of added and removed lines since the last snapshot, otherwise return false. + /// - If the sum of added and removed lines is greater than a configured threshold, return true, otherwise return false. + pub fn should_auto_snapshot(&self) -> Result { let oplog_state = OplogHandle::new(&self.gb_dir()); let last_snapshot_time = oplog_state.get_modified_at()?; if last_snapshot_time.elapsed()? > Duration::from_secs(300) { @@ -591,7 +577,10 @@ impl Oplog for Project { Ok(false) } - fn snapshot_diff(&self, sha: String) -> Result> { + /// Returns the diff of the snapshot and it's parent. It only includes the workdir changes. + /// + /// This is useful to show what has changed in this particular snapshot + pub fn snapshot_diff(&self, sha: String) -> Result> { let repo_path = self.path.as_path(); let repo = git2::Repository::init(repo_path)?; @@ -630,7 +619,9 @@ impl Oplog for Project { let hunks = hunks_by_filepath(None, &diff)?; Ok(hunks) } - fn oplog_head(&self) -> Result> { + + /// Gets the sha of the last snapshot commit if present. + pub fn oplog_head(&self) -> Result> { let oplog_state = OplogHandle::new(&self.gb_dir()); oplog_state.get_oplog_head() } diff --git a/crates/gitbutler-core/src/ops/snapshot.rs b/crates/gitbutler-core/src/ops/snapshot.rs index 437c4c76d..d3795a10b 100644 --- a/crates/gitbutler-core/src/ops/snapshot.rs +++ b/crates/gitbutler-core/src/ops/snapshot.rs @@ -1,33 +1,16 @@ use std::vec; +use crate::projects::Project; use crate::{ ops::entry::{OperationKind, SnapshotDetails}, virtual_branches::{branch::BranchUpdateRequest, Branch}, }; -use super::{entry::Trailer, oplog::Oplog}; +use super::entry::Trailer; -pub trait Snapshot { - fn snapshot_branch_creation(&self, branch_name: String) -> anyhow::Result<()>; - fn snapshot_branch_deletion(&self, branch_name: String) -> anyhow::Result<()>; - fn snapshot_branch_applied(&self, branch_name: String) -> anyhow::Result<()>; - fn snapshot_branch_unapplied(&self, branch_name: String) -> anyhow::Result<()>; - fn snapshot_branch_update( - &self, - old_branch: &Branch, - update: &BranchUpdateRequest, - ) -> anyhow::Result<()>; - fn snapshot_commit_creation( - &self, - snapshot_tree: String, - commit_message: String, - sha: Option, - ) -> anyhow::Result<()>; - fn snapshot_commit_undo(&self, commit_sha: String) -> anyhow::Result<()>; -} - -impl Snapshot for T { - fn snapshot_branch_applied(&self, branch_name: String) -> anyhow::Result<()> { +/// Snapshot functionality +impl Project { + pub(crate) fn snapshot_branch_applied(&self, branch_name: String) -> anyhow::Result<()> { let details = SnapshotDetails::new(OperationKind::ApplyBranch).with_trailers(vec![Trailer { key: "name".to_string(), @@ -36,7 +19,7 @@ impl Snapshot for T { self.create_snapshot(details)?; Ok(()) } - fn snapshot_branch_unapplied(&self, branch_name: String) -> anyhow::Result<()> { + pub(crate) fn snapshot_branch_unapplied(&self, branch_name: String) -> anyhow::Result<()> { let details = SnapshotDetails::new(OperationKind::UnapplyBranch).with_trailers(vec![Trailer { key: "name".to_string(), @@ -45,7 +28,7 @@ impl Snapshot for T { self.create_snapshot(details)?; Ok(()) } - fn snapshot_commit_undo(&self, commit_sha: String) -> anyhow::Result<()> { + pub(crate) fn snapshot_commit_undo(&self, commit_sha: String) -> anyhow::Result<()> { let details = SnapshotDetails::new(OperationKind::UndoCommit).with_trailers(vec![Trailer { key: "sha".to_string(), @@ -54,7 +37,7 @@ impl Snapshot for T { self.create_snapshot(details)?; Ok(()) } - fn snapshot_commit_creation( + pub(crate) fn snapshot_commit_creation( &self, snapshot_tree: String, commit_message: String, @@ -73,7 +56,7 @@ impl Snapshot for T { self.commit_snapshot(snapshot_tree, details)?; Ok(()) } - fn snapshot_branch_creation(&self, branch_name: String) -> anyhow::Result<()> { + pub(crate) fn snapshot_branch_creation(&self, branch_name: String) -> anyhow::Result<()> { let details = SnapshotDetails::new(OperationKind::CreateBranch).with_trailers(vec![Trailer { key: "name".to_string(), @@ -82,7 +65,7 @@ impl Snapshot for T { self.create_snapshot(details)?; Ok(()) } - fn snapshot_branch_deletion(&self, branch_name: String) -> anyhow::Result<()> { + pub(crate) fn snapshot_branch_deletion(&self, branch_name: String) -> anyhow::Result<()> { let details = SnapshotDetails::new(OperationKind::DeleteBranch).with_trailers(vec![Trailer { key: "name".to_string(), @@ -92,7 +75,7 @@ impl Snapshot for T { self.create_snapshot(details)?; Ok(()) } - fn snapshot_branch_update( + pub(crate) fn snapshot_branch_update( &self, old_branch: &Branch, update: &BranchUpdateRequest, diff --git a/crates/gitbutler-core/src/synchronize/mod.rs b/crates/gitbutler-core/src/synchronize/mod.rs index 104464837..ba79e84ca 100644 --- a/crates/gitbutler-core/src/synchronize/mod.rs +++ b/crates/gitbutler-core/src/synchronize/mod.rs @@ -1,7 +1,6 @@ use std::time; use crate::id::Id; -use crate::ops::oplog::Oplog; use crate::{ git::{self, Oid}, project_repository, diff --git a/crates/gitbutler-core/src/virtual_branches/controller.rs b/crates/gitbutler-core/src/virtual_branches/controller.rs index 31088d701..cc5f9a3b8 100644 --- a/crates/gitbutler-core/src/virtual_branches/controller.rs +++ b/crates/gitbutler-core/src/virtual_branches/controller.rs @@ -1,10 +1,6 @@ use crate::{ error::Error, - ops::{ - entry::{OperationKind, SnapshotDetails}, - oplog::Oplog, - snapshot::Snapshot, - }, + ops::entry::{OperationKind, SnapshotDetails}, }; use std::{collections::HashMap, path::Path, sync::Arc}; diff --git a/crates/gitbutler-core/src/virtual_branches/virtual.rs b/crates/gitbutler-core/src/virtual_branches/virtual.rs index 557b1bde5..a217abbd7 100644 --- a/crates/gitbutler-core/src/virtual_branches/virtual.rs +++ b/crates/gitbutler-core/src/virtual_branches/virtual.rs @@ -26,7 +26,6 @@ use super::{ }; use crate::error::{self, AnyhowContextExt, Code}; use crate::git::diff::{diff_files_into_hunks, trees, FileDiff}; -use crate::ops::snapshot::Snapshot; use crate::time::now_since_unix_epoch_ms; use crate::virtual_branches::branch::HunkHash; use crate::{ diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/oplog.rs b/crates/gitbutler-core/tests/suite/virtual_branches/oplog.rs index b81357034..d472dd8f1 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/oplog.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/oplog.rs @@ -1,7 +1,5 @@ use std::io::Write; -use gitbutler_core::ops::oplog::Oplog; - use super::*; #[tokio::test] diff --git a/crates/gitbutler-tauri/src/undo.rs b/crates/gitbutler-tauri/src/undo.rs index 6679b93d2..b9b35a91c 100644 --- a/crates/gitbutler-tauri/src/undo.rs +++ b/crates/gitbutler-tauri/src/undo.rs @@ -2,7 +2,7 @@ use crate::error::Error; use anyhow::Context; use gitbutler_core::git::diff::FileDiff; use gitbutler_core::{ - ops::{entry::Snapshot, oplog::Oplog}, + ops::entry::Snapshot, projects::{self, ProjectId}, }; use std::collections::HashMap; diff --git a/crates/gitbutler-watcher/src/handler.rs b/crates/gitbutler-watcher/src/handler.rs index faf60a962..5be2c930c 100644 --- a/crates/gitbutler-watcher/src/handler.rs +++ b/crates/gitbutler-watcher/src/handler.rs @@ -3,7 +3,6 @@ use std::sync::Arc; use anyhow::{Context, Result}; use gitbutler_core::ops::entry::{OperationKind, SnapshotDetails}; -use gitbutler_core::ops::oplog::Oplog; use gitbutler_core::projects::ProjectId; use gitbutler_core::synchronize::sync_with_gitbutler; use gitbutler_core::virtual_branches::VirtualBranches;