updateref: Refactor `AlreadyLockedError` to be more consistent

The `AlreadyLockedError` is quite inconsistent with the other errors
declared in the `updateref` package as it abbreviates the embedded
reference name and is a pointer receiver. Refactor it to improve our
code hygiene.

4 files changed, 11 insertions, 13 deletions

diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go
index 9fa40dd75..16fe02e18 100644
--- a/internal/git/updateref/updateref.go
+++ b/internal/git/updateref/updateref.go
@@ -16,11 +16,12 @@ import (
 // AlreadyLockedError indicates a reference cannot be locked because another
 // process has already locked it.
 type AlreadyLockedError struct {
-	Ref string
+	// ReferenceName is the name of the reference that is already locked.
+	ReferenceName string
 }
 
-func (e *AlreadyLockedError) Error() string {
-	return fmt.Sprintf("reference is already locked: %q", e.Ref)
+func (e AlreadyLockedError) Error() string {
+	return fmt.Sprintf("reference is already locked: %q", e.ReferenceName)
 }
 
 // InvalidReferenceFormatError indicates a reference name was invalid.
@@ -381,7 +382,7 @@ func (u *Updater) handleIOError(fallbackErr error) error {
 
 	matches := refLockedRegex.FindSubmatch(stderr)
 	if len(matches) > 1 {
-		return &AlreadyLockedError{Ref: string(matches[1])}
+		return AlreadyLockedError{ReferenceName: string(matches[1])}
 	}
 
 	matches = refInvalidFormatRegex.FindSubmatch(stderr)
diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go
index 8f611b90b..fd005a6c9 100644
--- a/internal/git/updateref/updateref_test.go
+++ b/internal/git/updateref/updateref_test.go
@@ -427,12 +427,9 @@ func TestUpdater_concurrentLocking(t *testing.T) {
 
 	// Preparing this second updater should fail though because we notice that the reference is
 	// locked.
-	err = secondUpdater.Prepare()
-	var alreadyLockedErr *AlreadyLockedError
-	require.ErrorAs(t, err, &alreadyLockedErr)
-	require.Equal(t, err, &AlreadyLockedError{
-		Ref: "refs/heads/master",
-	})
+	require.Equal(t, AlreadyLockedError{
+		ReferenceName: "refs/heads/master",
+	}, secondUpdater.Prepare())
 
 	// Whereas committing the first transaction should succeed.
 	require.NoError(t, firstUpdater.Commit())
diff --git a/internal/gitaly/service/ref/delete_refs.go b/internal/gitaly/service/ref/delete_refs.go
index 390568494..538521f0e 100644
--- a/internal/gitaly/service/ref/delete_refs.go
+++ b/internal/gitaly/service/ref/delete_refs.go
@@ -78,13 +78,13 @@ func (s *server) DeleteRefs(ctx context.Context, in *gitalypb.DeleteRefsRequest)
 	}
 
 	if err := updater.Prepare(); err != nil {
-		var alreadyLockedErr *updateref.AlreadyLockedError
+		var alreadyLockedErr updateref.AlreadyLockedError
 		if errors.As(err, &alreadyLockedErr) {
 			return nil, structerr.NewFailedPrecondition("cannot lock references").WithDetail(
 				&gitalypb.DeleteRefsError{
 					Error: &gitalypb.DeleteRefsError_ReferencesLocked{
 						ReferencesLocked: &gitalypb.ReferencesLockedError{
-							Refs: [][]byte{[]byte(alreadyLockedErr.Ref)},
+							Refs: [][]byte{[]byte(alreadyLockedErr.ReferenceName)},
 						},
 					},
 				},
diff --git a/internal/gitaly/storage/storagemgr/transaction_manager.go b/internal/gitaly/storage/storagemgr/transaction_manager.go
index 5a4c5817f..d9bae6b2d 100644
--- a/internal/gitaly/storage/storagemgr/transaction_manager.go
+++ b/internal/gitaly/storage/storagemgr/transaction_manager.go
@@ -1313,7 +1313,7 @@ func (mgr *TransactionManager) prepareReferenceTransaction(ctx context.Context,
 
 	// If we get an error due to existing stale reference locks, we should clear it up
 	// and retry running git-update-ref(1).
-	var updateRefError *updateref.AlreadyLockedError
+	var updateRefError updateref.AlreadyLockedError
 	if errors.As(err, &updateRefError) {
 		// Before clearing stale reference locks, we add should ensure that housekeeping doesn't
 		// run git-pack-refs(1), which could create new reference locks. So we add an inhibitor.