Merge branch 'better_ambiguous_ref_errors' into 'master'

UserCreateBranch: Improve "reference is ambiguous" errors

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4950

Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Co-authored-by: James Fargher <jfargher@gitlab.com>

4 files changed, 10 insertions, 10 deletions

diff --git a/internal/git/localrepo/refs.go b/internal/git/localrepo/refs.go
index f76975e28..0c8e4a29a 100644
--- a/internal/git/localrepo/refs.go
+++ b/internal/git/localrepo/refs.go
@@ -82,7 +82,7 @@ func (repo *Repo) GetReference(ctx context.Context, reference git.ReferenceName)
 		return git.Reference{}, git.ErrReferenceNotFound
 	}
 	if refs[0].Name != reference {
-		return git.Reference{}, git.ErrReferenceAmbiguous
+		return git.Reference{}, fmt.Errorf("%w: conflicts with %q", git.ErrReferenceAmbiguous, refs[0].Name)
 	}
 
 	return refs[0], nil
diff --git a/internal/git/localrepo/refs_test.go b/internal/git/localrepo/refs_test.go
index 47f64c3ab..7a9bafcc0 100644
--- a/internal/git/localrepo/refs_test.go
+++ b/internal/git/localrepo/refs_test.go
@@ -96,7 +96,7 @@ func TestRepo_GetReference(t *testing.T) {
 		{
 			desc:        "prefix returns an error",
 			ref:         "refs/heads",
-			expectedErr: git.ErrReferenceAmbiguous,
+			expectedErr: fmt.Errorf("%w: conflicts with %q", git.ErrReferenceAmbiguous, "refs/heads/master"),
 		},
 		{
 			desc:        "nonexistent branch",
diff --git a/internal/gitaly/service/operations/branches.go b/internal/gitaly/service/operations/branches.go
index 40cebfbc2..410835193 100644
--- a/internal/gitaly/service/operations/branches.go
+++ b/internal/gitaly/service/operations/branches.go
@@ -50,12 +50,6 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB
 	}
 
 	referenceName := git.NewReferenceNameFromBranchName(string(req.BranchName))
-	_, err = quarantineRepo.GetReference(ctx, referenceName)
-	if err == nil {
-		return nil, status.Errorf(codes.FailedPrecondition, "Could not update %s. Please refresh and try again.", req.BranchName)
-	} else if !errors.Is(err, git.ErrReferenceNotFound) {
-		return nil, status.Error(codes.Internal, err.Error())
-	}
 
 	if err := s.updateReferenceWithHooks(ctx, req.GetRepository(), req.User, quarantineDir, referenceName, startPointOID, git.ObjectHashSHA1.ZeroOID); err != nil {
 		var customHookErr updateref.CustomHookError
diff --git a/internal/gitaly/service/operations/branches_test.go b/internal/gitaly/service/operations/branches_test.go
index baa8920b9..e0bf721a0 100644
--- a/internal/gitaly/service/operations/branches_test.go
+++ b/internal/gitaly/service/operations/branches_test.go
@@ -368,13 +368,19 @@ func TestUserCreateBranch_Failure(t *testing.T) {
 			user:       gittest.TestUser,
 			err:        status.Errorf(codes.FailedPrecondition, "revspec '%s' not found", "i-dont-exist"),
 		},
-
 		{
 			desc:       "branch exists",
 			branchName: "master",
 			startPoint: "master",
 			user:       gittest.TestUser,
-			err:        status.Errorf(codes.FailedPrecondition, "Could not update %s. Please refresh and try again.", "master"),
+			err:        status.Errorf(codes.FailedPrecondition, "Could not update %s. Please refresh and try again.", "refs/heads/master"),
+		},
+		{
+			desc:       "conflicting with refs/heads/improve/awesome",
+			branchName: "improve",
+			startPoint: "master",
+			user:       gittest.TestUser,
+			err:        status.Errorf(codes.FailedPrecondition, "Could not update %s. Please refresh and try again.", "refs/heads/improve"),
 		},
 	}