diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2023-07-04 09:55:27 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2023-07-06 08:14:00 +0300 |
commit | feb70e4a7dab20cb6dfe7b7629047435360b17ef (patch) | |
tree | 83ea749f6393056924728ed1fb9f2edc9d8eaedc | |
parent | 514b8bf2ee9db51fb23f52efbcb93ef6decb3f75 (diff) |
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.
-rw-r--r-- | internal/git/updateref/updateref.go | 9 | ||||
-rw-r--r-- | internal/git/updateref/updateref_test.go | 9 | ||||
-rw-r--r-- | internal/gitaly/service/ref/delete_refs.go | 4 | ||||
-rw-r--r-- | internal/gitaly/storage/storagemgr/transaction_manager.go | 2 |
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. |