From 5c7a9a739ad3c5b2a1a2cb0f181a836e274398a4 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 15 Oct 2022 16:33:15 +0200 Subject: [PATCH] backend: Split RetryBackend into own package The RetryBackend tests depend on the mock backend. When the Backend interface is eventually split from the restic package, this will lead to a dependency cycle between backend and backend/mock. Thus split the RetryBackend into a separate package to avoid this problem. --- cmd/restic/global.go | 3 +- cmd/restic/integration_helpers_test.go | 4 +-- internal/backend/{ => retry}/backend_retry.go | 28 +++++++++---------- .../backend/{ => retry}/backend_retry_test.go | 16 +++++------ internal/backend/{ => retry}/testing.go | 2 +- 5 files changed, 27 insertions(+), 26 deletions(-) rename internal/backend/{ => retry}/backend_retry.go (82%) rename internal/backend/{ => retry}/backend_retry_test.go (95%) rename internal/backend/{ => retry}/testing.go (90%) diff --git a/cmd/restic/global.go b/cmd/restic/global.go index fb14934f8..25dae55f2 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -22,6 +22,7 @@ import ( "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/backend/rclone" "github.com/restic/restic/internal/backend/rest" + "github.com/restic/restic/internal/backend/retry" "github.com/restic/restic/internal/backend/s3" "github.com/restic/restic/internal/backend/sftp" "github.com/restic/restic/internal/backend/swift" @@ -445,7 +446,7 @@ func OpenRepository(ctx context.Context, opts GlobalOptions) (*repository.Reposi success := func(msg string, retries int) { Warnf("%v operation successful after %d retries\n", msg, retries) } - be = backend.NewRetryBackend(be, 10, report, success) + be = retry.New(be, 10, report, success) // wrap backend if a test specified a hook if opts.backendTestHook != nil { diff --git a/cmd/restic/integration_helpers_test.go b/cmd/restic/integration_helpers_test.go index 6a4d064a8..1acde5bc2 100644 --- a/cmd/restic/integration_helpers_test.go +++ b/cmd/restic/integration_helpers_test.go @@ -9,7 +9,7 @@ import ( "runtime" "testing" - "github.com/restic/restic/internal/backend" + "github.com/restic/restic/internal/backend/retry" "github.com/restic/restic/internal/options" "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" @@ -172,7 +172,7 @@ func withTestEnvironment(t testing.TB) (env *testEnvironment, cleanup func()) { repository.TestUseLowSecurityKDFParameters(t) restic.TestDisableCheckPolynomial(t) - backend.TestFastRetries(t) + retry.TestFastRetries(t) tempdir, err := ioutil.TempDir(rtest.TestTempDir, "restic-test-") rtest.OK(t, err) diff --git a/internal/backend/backend_retry.go b/internal/backend/retry/backend_retry.go similarity index 82% rename from internal/backend/backend_retry.go rename to internal/backend/retry/backend_retry.go index e9a22d75f..334fe4216 100644 --- a/internal/backend/backend_retry.go +++ b/internal/backend/retry/backend_retry.go @@ -1,4 +1,4 @@ -package backend +package retry import ( "context" @@ -11,9 +11,9 @@ import ( "github.com/restic/restic/internal/restic" ) -// RetryBackend retries operations on the backend in case of an error with a +// Backend retries operations on the backend in case of an error with a // backoff. -type RetryBackend struct { +type Backend struct { restic.Backend MaxTries int Report func(string, error, time.Duration) @@ -21,14 +21,14 @@ type RetryBackend struct { } // statically ensure that RetryBackend implements restic.Backend. -var _ restic.Backend = &RetryBackend{} +var _ restic.Backend = &Backend{} -// NewRetryBackend wraps be with a backend that retries operations after a +// New wraps be with a backend that retries operations after a // backoff. report is called with a description and the error, if one occurred. // success is called with the number of retries before a successful operation // (it is not called if it succeeded on the first try) -func NewRetryBackend(be restic.Backend, maxTries int, report func(string, error, time.Duration), success func(string, int)) *RetryBackend { - return &RetryBackend{ +func New(be restic.Backend, maxTries int, report func(string, error, time.Duration), success func(string, int)) *Backend { + return &Backend{ Backend: be, MaxTries: maxTries, Report: report, @@ -57,7 +57,7 @@ func retryNotifyErrorWithSuccess(operation backoff.Operation, b backoff.BackOff, var fastRetries = false -func (be *RetryBackend) retry(ctx context.Context, msg string, f func() error) error { +func (be *Backend) retry(ctx context.Context, msg string, f func() error) error { // Don't do anything when called with an already cancelled context. There would be // no retries in that case either, so be consistent and abort always. // This enforces a strict contract for backend methods: Using a cancelled context @@ -92,7 +92,7 @@ func (be *RetryBackend) retry(ctx context.Context, msg string, f func() error) e } // Save stores the data in the backend under the given handle. -func (be *RetryBackend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error { +func (be *Backend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error { return be.retry(ctx, fmt.Sprintf("Save(%v)", h), func() error { err := rd.Rewind() if err != nil { @@ -125,7 +125,7 @@ func (be *RetryBackend) Save(ctx context.Context, h restic.Handle, rd restic.Rew // given offset. If length is larger than zero, only a portion of the file // is returned. rd must be closed after use. If an error is returned, the // ReadCloser must be nil. -func (be *RetryBackend) Load(ctx context.Context, h restic.Handle, length int, offset int64, consumer func(rd io.Reader) error) (err error) { +func (be *Backend) Load(ctx context.Context, h restic.Handle, length int, offset int64, consumer func(rd io.Reader) error) (err error) { return be.retry(ctx, fmt.Sprintf("Load(%v, %v, %v)", h, length, offset), func() error { return be.Backend.Load(ctx, h, length, offset, consumer) @@ -133,7 +133,7 @@ func (be *RetryBackend) Load(ctx context.Context, h restic.Handle, length int, o } // Stat returns information about the File identified by h. -func (be *RetryBackend) Stat(ctx context.Context, h restic.Handle) (fi restic.FileInfo, err error) { +func (be *Backend) Stat(ctx context.Context, h restic.Handle) (fi restic.FileInfo, err error) { err = be.retry(ctx, fmt.Sprintf("Stat(%v)", h), func() error { var innerError error @@ -145,14 +145,14 @@ func (be *RetryBackend) Stat(ctx context.Context, h restic.Handle) (fi restic.Fi } // Remove removes a File with type t and name. -func (be *RetryBackend) Remove(ctx context.Context, h restic.Handle) (err error) { +func (be *Backend) Remove(ctx context.Context, h restic.Handle) (err error) { return be.retry(ctx, fmt.Sprintf("Remove(%v)", h), func() error { return be.Backend.Remove(ctx, h) }) } // Test a boolean value whether a File with the name and type exists. -func (be *RetryBackend) Test(ctx context.Context, h restic.Handle) (exists bool, err error) { +func (be *Backend) Test(ctx context.Context, h restic.Handle) (exists bool, err error) { err = be.retry(ctx, fmt.Sprintf("Test(%v)", h), func() error { var innerError error exists, innerError = be.Backend.Test(ctx, h) @@ -166,7 +166,7 @@ func (be *RetryBackend) Test(ctx context.Context, h restic.Handle) (exists bool, // error is returned by the underlying backend, the request is retried. When fn // returns an error, the operation is aborted and the error is returned to the // caller. -func (be *RetryBackend) List(ctx context.Context, t restic.FileType, fn func(restic.FileInfo) error) error { +func (be *Backend) List(ctx context.Context, t restic.FileType, fn func(restic.FileInfo) error) error { // create a new context that we can cancel when fn returns an error, so // that listing is aborted listCtx, cancel := context.WithCancel(ctx) diff --git a/internal/backend/backend_retry_test.go b/internal/backend/retry/backend_retry_test.go similarity index 95% rename from internal/backend/backend_retry_test.go rename to internal/backend/retry/backend_retry_test.go index aca30f61d..b8f2dafca 100644 --- a/internal/backend/backend_retry_test.go +++ b/internal/backend/retry/backend_retry_test.go @@ -1,4 +1,4 @@ -package backend +package retry import ( "bytes" @@ -36,7 +36,7 @@ func TestBackendSaveRetry(t *testing.T) { } TestFastRetries(t) - retryBackend := NewRetryBackend(be, 10, nil, nil) + retryBackend := New(be, 10, nil, nil) data := test.Random(23, 5*1024*1024+11241) err := retryBackend.Save(context.TODO(), restic.Handle{}, restic.NewByteReader(data, be.Hasher())) @@ -72,7 +72,7 @@ func TestBackendSaveRetryAtomic(t *testing.T) { } TestFastRetries(t) - retryBackend := NewRetryBackend(be, 10, nil, nil) + retryBackend := New(be, 10, nil, nil) data := test.Random(23, 5*1024*1024+11241) err := retryBackend.Save(context.TODO(), restic.Handle{}, restic.NewByteReader(data, be.Hasher())) @@ -106,7 +106,7 @@ func TestBackendListRetry(t *testing.T) { } TestFastRetries(t) - retryBackend := NewRetryBackend(be, 10, nil, nil) + retryBackend := New(be, 10, nil, nil) var listed []string err := retryBackend.List(context.TODO(), restic.PackFile, func(fi restic.FileInfo) error { @@ -136,7 +136,7 @@ func TestBackendListRetryErrorFn(t *testing.T) { } TestFastRetries(t) - retryBackend := NewRetryBackend(be, 10, nil, nil) + retryBackend := New(be, 10, nil, nil) var ErrTest = errors.New("test error") @@ -193,7 +193,7 @@ func TestBackendListRetryErrorBackend(t *testing.T) { TestFastRetries(t) const maxRetries = 2 - retryBackend := NewRetryBackend(be, maxRetries, nil, nil) + retryBackend := New(be, maxRetries, nil, nil) var listed []string err := retryBackend.List(context.TODO(), restic.PackFile, func(fi restic.FileInfo) error { @@ -263,7 +263,7 @@ func TestBackendLoadRetry(t *testing.T) { } TestFastRetries(t) - retryBackend := NewRetryBackend(be, 10, nil, nil) + retryBackend := New(be, 10, nil, nil) var buf []byte err := retryBackend.Load(context.TODO(), restic.Handle{}, 0, 0, func(rd io.Reader) (err error) { @@ -283,7 +283,7 @@ func TestBackendCanceledContext(t *testing.T) { // unimplemented mock backend functions return an error by default // check that we received the expected context canceled error instead TestFastRetries(t) - retryBackend := NewRetryBackend(mock.NewBackend(), 2, nil, nil) + retryBackend := New(mock.NewBackend(), 2, nil, nil) h := restic.Handle{Type: restic.PackFile, Name: restic.NewRandomID().String()} // create an already canceled context diff --git a/internal/backend/testing.go b/internal/backend/retry/testing.go similarity index 90% rename from internal/backend/testing.go rename to internal/backend/retry/testing.go index 0d7fa1d76..797573b03 100644 --- a/internal/backend/testing.go +++ b/internal/backend/retry/testing.go @@ -1,4 +1,4 @@ -package backend +package retry import "testing"