update create and update reference apis

No need to provide the change_id as it can be obtained from the commit itself
This commit is contained in:
Kiril Videlov 2024-08-23 14:32:31 +02:00
parent 7137fce76e
commit 4668a1ec3d
No known key found for this signature in database
GPG Key ID: A4C733025427C471
3 changed files with 30 additions and 33 deletions

View File

@ -117,7 +117,6 @@ fn update_existing_branch_references(
reference.branch_id,
reference.upstream.clone(),
new_commit.id(),
new_commit.change_id(),
)
.map(Some)
})

View File

@ -9,6 +9,7 @@ use gitbutler_branch::BranchReference;
use gitbutler_branch::VirtualBranchesHandle;
use gitbutler_branch::{Branch, BranchId};
use gitbutler_command_context::CommandContext;
use gitbutler_commit::commit_ext::CommitExt;
use gitbutler_reference::ReferenceName;
use itertools::Itertools;
@ -55,7 +56,6 @@ pub fn create_branch_reference(
branch_id: BranchId,
upstream: ReferenceName,
commit_id: git2::Oid,
change_id: Option<String>,
) -> Result<BranchReference> {
// The reference must be parseable as a remote reference
gitbutler_reference::RemoteRefname::from_str(&upstream)
@ -64,11 +64,18 @@ pub fn create_branch_reference(
// The branch must exist
let mut vbranch = handle.get_branch(branch_id)?;
// Enusre that the commit acutally exists
let commit = ctx
.repository()
.find_commit(commit_id)
.context(anyhow!("Commit {} does not exist", commit_id))?;
let branch_reference = BranchReference {
upstream,
branch_id,
commit_id,
change_id,
change_id: commit.change_id(),
};
let all_references = handle
.list_all_branches()?
@ -107,13 +114,11 @@ pub fn create_branch_reference(
/// - the reference exists, but the commit id is already associated with another reference
///
/// If the commit ID is the same as the current commit ID, the function is a no-op.
/// If the change ID is provided, it will be updated, otherwise it will be left unchanged.
pub fn update_branch_reference(
ctx: &CommandContext,
branch_id: BranchId,
upstream: ReferenceName,
new_commit_id: git2::Oid,
new_change_id: Option<String>,
) -> Result<BranchReference> {
// The reference must be parseable as a remote reference
gitbutler_reference::RemoteRefname::from_str(&upstream)
@ -122,6 +127,12 @@ pub fn update_branch_reference(
// The branch must exist
let mut vbranch = handle.get_branch(branch_id)?;
// Enusre that the commit acutally exists
let new_commit = ctx
.repository()
.find_commit(new_commit_id)
.context(anyhow!("Commit {} does not exist", new_commit_id))?;
// Fail early if the commit is not valid
validate_commit(&vbranch, new_commit_id, ctx, &handle)?;
@ -135,7 +146,7 @@ pub fn update_branch_reference(
branch_id
))?;
reference.commit_id = new_commit_id;
reference.change_id = new_change_id.or(reference.change_id.clone());
reference.change_id = new_commit.change_id();
let new_reference = reference.clone();
handle.set_branch(vbranch)?;
Ok(new_reference)
@ -171,7 +182,6 @@ pub fn push_branch_reference(
/// Validates a commit in the following ways:
/// - The reference does not already exists for any other branch
/// - There is no other reference already pointing to the commit
/// - The commit actually exists
/// - The commit is between the branch base and the branch head
fn validate_commit(
vbranch: &Branch,
@ -179,11 +189,6 @@ fn validate_commit(
ctx: &CommandContext,
handle: &VirtualBranchesHandle,
) -> Result<()> {
// Enusre that the commit acutally exists
ctx.repository()
.find_commit(commit_id)
.context(anyhow!("Commit {} does not exist", commit_id))?;
let target = handle.get_default_target()?;
let branch_commits = ctx
.log(vbranch.head, LogUntil::Commit(target.sha))?

View File

@ -1,6 +1,7 @@
use anyhow::Result;
use gitbutler_branch::VirtualBranchesHandle;
use gitbutler_command_context::CommandContext;
use gitbutler_commit::commit_ext::CommitExt;
use gitbutler_repo::{
create_branch_reference, credentials::Helper, list_branch_references, list_commit_references,
push_branch_reference, update_branch_reference, LogUntil, RepoActionsExt,
@ -16,12 +17,14 @@ fn create_success() -> Result<()> {
test_ctx.branch.id,
"refs/remotes/origin/success".into(),
test_ctx.commits.first().unwrap().id(),
Some("change-id".into()),
)?;
assert_eq!(reference.branch_id, test_ctx.branch.id);
assert_eq!(reference.upstream, "refs/remotes/origin/success".into());
assert_eq!(reference.commit_id, test_ctx.commits.first().unwrap().id());
assert_eq!(reference.change_id, Some("change-id".into()));
assert_eq!(
reference.change_id,
test_ctx.commits.first().unwrap().change_id()
);
Ok(())
}
@ -34,23 +37,24 @@ fn create_multiple() -> Result<()> {
test_ctx.branch.id,
"refs/remotes/origin/first".into(),
test_ctx.commits.first().unwrap().id(),
None,
)?;
assert_eq!(first.branch_id, test_ctx.branch.id);
assert_eq!(first.upstream, "refs/remotes/origin/first".into());
assert_eq!(first.commit_id, test_ctx.commits.first().unwrap().id());
assert_eq!(first.change_id, None);
assert_eq!(
first.change_id,
test_ctx.commits.first().unwrap().change_id()
);
let last = create_branch_reference(
&ctx,
test_ctx.branch.id,
"refs/remotes/origin/last".into(),
test_ctx.commits.last().unwrap().id(),
None,
)?;
assert_eq!(last.branch_id, test_ctx.branch.id);
assert_eq!(last.upstream, "refs/remotes/origin/last".into());
assert_eq!(last.commit_id, test_ctx.commits.last().unwrap().id());
assert_eq!(last.change_id, None);
assert_eq!(last.change_id, test_ctx.commits.last().unwrap().change_id());
Ok(())
}
@ -63,7 +67,6 @@ fn create_fails_with_non_remote_reference() -> Result<()> {
test_ctx.branch.id,
"foo".into(),
test_ctx.commits.first().unwrap().id(),
None,
);
assert_eq!(
result.unwrap_err().to_string(),
@ -81,14 +84,12 @@ fn create_fails_when_branch_reference_with_name_exists() -> Result<()> {
test_ctx.branch.id,
"refs/remotes/origin/taken".into(),
test_ctx.commits.first().unwrap().id(),
None,
)?;
let result = create_branch_reference(
&ctx,
test_ctx.branch.id,
"refs/remotes/origin/taken".into(),
test_ctx.commits.last().unwrap().id(),
None,
);
assert_eq!(
result.unwrap_err().to_string(),
@ -107,14 +108,12 @@ fn create_fails_when_commit_already_referenced() -> Result<()> {
test_ctx.branch.id,
"refs/remotes/origin/one".into(),
test_ctx.commits.first().unwrap().id(),
None,
)?;
let result = create_branch_reference(
&ctx,
test_ctx.branch.id,
"refs/remotes/origin/two".into(),
test_ctx.commits.first().unwrap().id(),
None,
);
assert_eq!(
result.unwrap_err().to_string(),
@ -137,7 +136,6 @@ fn create_fails_when_commit_in_anothe_branch() -> Result<()> {
test_ctx.branch.id,
"refs/remotes/origin/asdf".into(),
wrong_commit,
None,
);
assert_eq!(
result.unwrap_err().to_string(),
@ -158,7 +156,6 @@ fn create_fails_when_commit_is_the_base() -> Result<()> {
test_ctx.branch.id,
"refs/remotes/origin/baz".into(),
test_ctx.branch_base,
None,
);
assert_eq!(
result.unwrap_err().to_string(),
@ -179,14 +176,12 @@ fn list_success() -> Result<()> {
test_ctx.branch.id,
"refs/remotes/origin/first".into(),
test_ctx.commits.first().unwrap().id(),
Some("change-id".into()),
)?;
let second_ref = create_branch_reference(
&ctx,
test_ctx.branch.id,
"refs/remotes/origin/second".into(),
test_ctx.commits.last().unwrap().id(),
Some("change-id".into()),
)?;
let result = list_branch_references(&ctx, test_ctx.branch.id)?;
assert_eq!(result.len(), 2);
@ -204,16 +199,18 @@ fn update_success() -> Result<()> {
test_ctx.branch.id,
"refs/remotes/origin/first".into(),
test_ctx.commits.first().unwrap().id(),
Some("change-id".into()),
)?;
let updated = update_branch_reference(
&ctx,
test_ctx.branch.id,
"refs/remotes/origin/first".into(),
test_ctx.commits.last().unwrap().id(),
None,
)?;
assert_eq!(updated.commit_id, test_ctx.commits.last().unwrap().id());
assert_eq!(
updated.change_id,
test_ctx.commits.last().unwrap().change_id()
);
let list = list_branch_references(&ctx, test_ctx.branch.id)?;
assert_eq!(list.len(), 1);
assert_eq!(list[0].commit_id, test_ctx.commits.last().unwrap().id());
@ -229,7 +226,6 @@ fn push_success() -> Result<()> {
test_ctx.branch.id,
"refs/remotes/origin/first".into(),
test_ctx.commits.first().unwrap().id(),
Some("change-id".into()),
)?;
let result = push_branch_reference(
&ctx,
@ -251,21 +247,18 @@ fn list_by_commits_success() -> Result<()> {
test_ctx.branch.id,
"refs/remotes/origin/first".into(),
test_ctx.commits.first().unwrap().id(),
None,
)?;
let second = create_branch_reference(
&ctx,
test_ctx.branch.id,
"refs/remotes/origin/second".into(),
test_ctx.commits.last().unwrap().id(),
None,
)?;
let third = create_branch_reference(
&ctx,
test_ctx.other_branch.id,
"refs/remotes/origin/third".into(),
test_ctx.other_commits.first().unwrap().id(),
None,
)?;
let commits = vec![
test_ctx.commits.first().unwrap().id(),