From f33dbc63518c784f40315590fe655af50901fb85 Mon Sep 17 00:00:00 2001 From: Keepers Date: Fri, 10 Feb 2023 17:17:42 -0700 Subject: [PATCH] deprecate adder interface for only fault.Errors (#2378) ## Does this PR need a docs update or release note? - [x] :no_entry: No ## Type of change - [x] :broom: Tech Debt/Cleanup ## Issue(s) * #1970 ## Test Plan - [x] :zap: Unit test - [x] :green_heart: E2E --- src/internal/operations/manifests.go | 8 ++++++-- src/internal/operations/manifests_test.go | 5 ++--- src/pkg/fault/fault.go | 5 ----- src/pkg/fault/mock/mock.go | 22 ---------------------- src/pkg/selectors/exchange.go | 2 +- src/pkg/selectors/exchange_test.go | 7 ++----- src/pkg/selectors/onedrive.go | 2 +- src/pkg/selectors/onedrive_test.go | 7 ++----- src/pkg/selectors/scopes.go | 2 +- src/pkg/selectors/scopes_test.go | 6 +++--- src/pkg/selectors/selectors.go | 4 ++-- src/pkg/selectors/selectors_reduce_test.go | 7 ++----- src/pkg/selectors/sharepoint.go | 2 +- src/pkg/selectors/sharepoint_test.go | 7 ++----- 14 files changed, 25 insertions(+), 61 deletions(-) delete mode 100644 src/pkg/fault/mock/mock.go diff --git a/src/internal/operations/manifests.go b/src/internal/operations/manifests.go index a1d7997ed..38ef60e0c 100644 --- a/src/internal/operations/manifests.go +++ b/src/internal/operations/manifests.go @@ -135,13 +135,17 @@ func produceManifestsAndMetadata( // of manifests, that each manifest's Reason (owner, service, category) is only // included once. If a reason is duplicated by any two manifests, an error is // returned. -func verifyDistinctBases(ctx context.Context, mans []*kopia.ManifestEntry, errs fault.Adder) error { +func verifyDistinctBases(ctx context.Context, mans []*kopia.ManifestEntry, errs *fault.Errors) error { var ( failed bool reasons = map[string]manifest.ID{} ) for _, man := range mans { + if errs.Failed() { + break + } + // Incomplete snapshots are used only for kopia-assisted incrementals. The // fact that we need this check here makes it seem like this should live in // the kopia code. However, keeping it here allows for better debugging as @@ -173,7 +177,7 @@ func verifyDistinctBases(ctx context.Context, mans []*kopia.ManifestEntry, errs return clues.New("multiple base snapshots qualify").WithClues(ctx) } - return nil + return errs.Err() } // collectMetadata retrieves all metadata files associated with the manifest. diff --git a/src/internal/operations/manifests_test.go b/src/internal/operations/manifests_test.go index 24c948320..a234b81e8 100644 --- a/src/internal/operations/manifests_test.go +++ b/src/internal/operations/manifests_test.go @@ -15,7 +15,6 @@ import ( "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/backup" "github.com/alcionai/corso/src/pkg/fault" - "github.com/alcionai/corso/src/pkg/fault/mock" "github.com/alcionai/corso/src/pkg/path" ) @@ -392,7 +391,7 @@ func (suite *OperationsManifestsUnitSuite) TestVerifyDistinctBases() { ctx, flush := tester.NewContext() defer flush() - err := verifyDistinctBases(ctx, test.mans, mock.NewAdder()) + err := verifyDistinctBases(ctx, test.mans, fault.New(true)) test.expect(t, err) }) } @@ -834,7 +833,7 @@ func (suite *BackupManifestSuite) TestBackupOperation_VerifyDistinctBases() { ctx, flush := tester.NewContext() defer flush() - test.errCheck(t, verifyDistinctBases(ctx, test.input, mock.NewAdder())) + test.errCheck(t, verifyDistinctBases(ctx, test.input, fault.New(true))) }) } } diff --git a/src/pkg/fault/fault.go b/src/pkg/fault/fault.go index d060e969b..ee560965b 100644 --- a/src/pkg/fault/fault.go +++ b/src/pkg/fault/fault.go @@ -102,11 +102,6 @@ func (e *Errors) setErr(err error) *Errors { return e } -type Adder interface { - Add(err error) *Errors - Failed() bool -} - // Add appends the error to the slice of recoverable and // iterated errors (ie: errors.errs). If failFast is true, // the first Added error will get copied to errors.err, diff --git a/src/pkg/fault/mock/mock.go b/src/pkg/fault/mock/mock.go deleted file mode 100644 index 7076f134c..000000000 --- a/src/pkg/fault/mock/mock.go +++ /dev/null @@ -1,22 +0,0 @@ -package mock - -import "github.com/alcionai/corso/src/pkg/fault" - -// Adder mocks an adder interface for testing. -type Adder struct { - FailFast bool - Errs []error -} - -func NewAdder() *Adder { - return &Adder{Errs: []error{}} -} - -func (ma *Adder) Add(err error) *fault.Errors { - ma.Errs = append(ma.Errs, err) - return fault.New(true) -} - -func (ma *Adder) Failed() bool { - return ma.FailFast && len(ma.Errs) > 0 -} diff --git a/src/pkg/selectors/exchange.go b/src/pkg/selectors/exchange.go index a5547749f..eb974a6f1 100644 --- a/src/pkg/selectors/exchange.go +++ b/src/pkg/selectors/exchange.go @@ -708,7 +708,7 @@ func (s ExchangeScope) setDefaults() { func (s exchange) Reduce( ctx context.Context, deets *details.Details, - errs fault.Adder, + errs *fault.Errors, ) *details.Details { return reduce[ExchangeScope]( ctx, diff --git a/src/pkg/selectors/exchange_test.go b/src/pkg/selectors/exchange_test.go index 830cde0c0..b801722f0 100644 --- a/src/pkg/selectors/exchange_test.go +++ b/src/pkg/selectors/exchange_test.go @@ -11,7 +11,7 @@ import ( "github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/backup/details" - "github.com/alcionai/corso/src/pkg/fault/mock" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/filters" "github.com/alcionai/corso/src/pkg/path" ) @@ -1030,13 +1030,10 @@ func (suite *ExchangeSelectorSuite) TestExchangeRestore_Reduce() { ctx, flush := tester.NewContext() defer flush() - errs := mock.NewAdder() - sel := test.makeSelector() - results := sel.Reduce(ctx, test.deets, errs) + results := sel.Reduce(ctx, test.deets, fault.New(true)) paths := results.Paths() assert.Equal(t, test.expect, paths) - assert.Empty(t, errs.Errs) }) } } diff --git a/src/pkg/selectors/onedrive.go b/src/pkg/selectors/onedrive.go index f4d924a3b..b6d7c3dd7 100644 --- a/src/pkg/selectors/onedrive.go +++ b/src/pkg/selectors/onedrive.go @@ -487,7 +487,7 @@ func (s OneDriveScope) DiscreteCopy(user string) OneDriveScope { func (s oneDrive) Reduce( ctx context.Context, deets *details.Details, - errs fault.Adder, + errs *fault.Errors, ) *details.Details { return reduce[OneDriveScope]( ctx, diff --git a/src/pkg/selectors/onedrive_test.go b/src/pkg/selectors/onedrive_test.go index 273019519..63a21b3b0 100644 --- a/src/pkg/selectors/onedrive_test.go +++ b/src/pkg/selectors/onedrive_test.go @@ -11,7 +11,7 @@ import ( "github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/backup/details" - "github.com/alcionai/corso/src/pkg/fault/mock" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" ) @@ -242,13 +242,10 @@ func (suite *OneDriveSelectorSuite) TestOneDriveRestore_Reduce() { ctx, flush := tester.NewContext() defer flush() - errs := mock.NewAdder() - sel := test.makeSelector() - results := sel.Reduce(ctx, test.deets, errs) + results := sel.Reduce(ctx, test.deets, fault.New(true)) paths := results.Paths() assert.Equal(t, test.expect, paths) - assert.Empty(t, errs.Errs) }) } } diff --git a/src/pkg/selectors/scopes.go b/src/pkg/selectors/scopes.go index 5fc05e789..458759265 100644 --- a/src/pkg/selectors/scopes.go +++ b/src/pkg/selectors/scopes.go @@ -287,7 +287,7 @@ func reduce[T scopeT, C categoryT]( deets *details.Details, s Selector, dataCategories map[path.CategoryType]C, - errs fault.Adder, + errs *fault.Errors, ) *details.Details { ctx, end := D.Span(ctx, "selectors:reduce") defer end() diff --git a/src/pkg/selectors/scopes_test.go b/src/pkg/selectors/scopes_test.go index 848d55767..273b1b774 100644 --- a/src/pkg/selectors/scopes_test.go +++ b/src/pkg/selectors/scopes_test.go @@ -9,7 +9,7 @@ import ( "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/backup/details" - "github.com/alcionai/corso/src/pkg/fault/mock" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/filters" "github.com/alcionai/corso/src/pkg/path" ) @@ -274,7 +274,7 @@ func (suite *SelectorScopesSuite) TestReduce() { ctx, flush := tester.NewContext() defer flush() - errs := mock.NewAdder() + errs := fault.New(true) ds := deets() result := reduce[mockScope]( @@ -284,7 +284,7 @@ func (suite *SelectorScopesSuite) TestReduce() { dataCats, errs) require.NotNil(t, result) - require.Empty(t, errs.Errs, "iteration errors") + require.Empty(t, errs.Errs(), "recoverable errors") assert.Len(t, result.Entries, test.expectLen) }) } diff --git a/src/pkg/selectors/selectors.go b/src/pkg/selectors/selectors.go index 8a9c02337..41bfecbcd 100644 --- a/src/pkg/selectors/selectors.go +++ b/src/pkg/selectors/selectors.go @@ -70,7 +70,7 @@ var ( const All = "All" type Reducer interface { - Reduce(context.Context, *details.Details, fault.Adder) *details.Details + Reduce(context.Context, *details.Details, *fault.Errors) *details.Details } // selectorResourceOwners aggregates all discrete path category types described @@ -240,7 +240,7 @@ func (s Selector) PathService() path.ServiceType { func (s Selector) Reduce( ctx context.Context, deets *details.Details, - errs fault.Adder, + errs *fault.Errors, ) (*details.Details, error) { r, err := selectorAsIface[Reducer](s) if err != nil { diff --git a/src/pkg/selectors/selectors_reduce_test.go b/src/pkg/selectors/selectors_reduce_test.go index 3748f793a..62ef4242b 100644 --- a/src/pkg/selectors/selectors_reduce_test.go +++ b/src/pkg/selectors/selectors_reduce_test.go @@ -10,7 +10,7 @@ import ( "github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/backup/details" - "github.com/alcionai/corso/src/pkg/fault/mock" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/selectors" "github.com/alcionai/corso/src/pkg/selectors/testdata" ) @@ -265,11 +265,8 @@ func (suite *SelectorReduceSuite) TestReduce() { for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { - errs := mock.NewAdder() - - output := test.selFunc().Reduce(ctx, allDetails, errs) + output := test.selFunc().Reduce(ctx, allDetails, fault.New(true)) assert.ElementsMatch(t, test.expected, output.Entries) - assert.Empty(t, errs.Errs) }) } } diff --git a/src/pkg/selectors/sharepoint.go b/src/pkg/selectors/sharepoint.go index 1df132e93..17f90dea7 100644 --- a/src/pkg/selectors/sharepoint.go +++ b/src/pkg/selectors/sharepoint.go @@ -559,7 +559,7 @@ func (s SharePointScope) DiscreteCopy(site string) SharePointScope { func (s sharePoint) Reduce( ctx context.Context, deets *details.Details, - errs fault.Adder, + errs *fault.Errors, ) *details.Details { return reduce[SharePointScope]( ctx, diff --git a/src/pkg/selectors/sharepoint_test.go b/src/pkg/selectors/sharepoint_test.go index 2bf3f585c..6e0444a48 100644 --- a/src/pkg/selectors/sharepoint_test.go +++ b/src/pkg/selectors/sharepoint_test.go @@ -9,7 +9,7 @@ import ( "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/backup/details" - "github.com/alcionai/corso/src/pkg/fault/mock" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" ) @@ -306,13 +306,10 @@ func (suite *SharePointSelectorSuite) TestSharePointRestore_Reduce() { ctx, flush := tester.NewContext() defer flush() - errs := mock.NewAdder() - sel := test.makeSelector() - results := sel.Reduce(ctx, test.deets, errs) + results := sel.Reduce(ctx, test.deets, fault.New(true)) paths := results.Paths() assert.Equal(t, test.expect, paths) - assert.Empty(t, errs.Errs) }) } }