Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

Commit

Permalink
replace graph Wrap and Stack with clues (#5018)
Browse files Browse the repository at this point in the history
Now that graph errors are always transformed as part of the graph client wrapper or http_wrapper, we don't need to call graph.Stack or graph.Wrap outside of those inner helpers any longer. This PR swaps all those graph calls with equivalent clues funcs as a cleanup.

Should not contain any logical changes.

---

#### Does this PR need a docs update or release note?

- [x] ⛔ No

#### Type of change

- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)

* #4685 

#### Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E
  • Loading branch information
ryanfkeepers authored Feb 6, 2024
1 parent 53a0525 commit e86592f
Show file tree
Hide file tree
Showing 42 changed files with 260 additions and 374 deletions.
6 changes: 4 additions & 2 deletions src/internal/m365/collection/drive/item_collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ func (suite *ItemCollectorUnitSuite) TestDrives() {
{
Values: nil,
NextLink: nil,
Err: graph.Stack(ctx, mySiteURLNotFound),
// needs graph.Stack, not clues.Stack
Err: graph.Stack(ctx, mySiteURLNotFound),
},
},
expectedErr: assert.NoError,
Expand All @@ -165,7 +166,8 @@ func (suite *ItemCollectorUnitSuite) TestDrives() {
{
Values: nil,
NextLink: nil,
Err: graph.Stack(ctx, mySiteNotFound),
// needs graph.Stack, not clues.Stack
Err: graph.Stack(ctx, mySiteNotFound),
},
},
expectedErr: assert.NoError,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (cfc *contactContainerCache) Populate(
if err != nil {
errs.AddRecoverable(
ctx,
graph.Stack(ctx, err).Label(fault.LabelForceNoBackupCreation))
clues.StackWC(ctx, err).Label(fault.LabelForceNoBackupCreation))
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/internal/m365/collection/exchange/contacts_restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func restoreContact(
) (*details.ExchangeInfo, error) {
contact, err := api.BytesToContactable(body)
if err != nil {
return nil, graph.Wrap(ctx, err, "creating contact from bytes")
return nil, clues.WrapWC(ctx, err, "creating contact from bytes")
}

ctx = clues.Add(ctx, "item_id", ptr.Val(contact.GetId()))
Expand Down Expand Up @@ -148,7 +148,7 @@ func restoreContact(

item, err := cr.PostItem(ctx, userID, destinationID, contact)
if err != nil {
return nil, graph.Wrap(ctx, err, "restoring contact")
return nil, clues.Wrap(err, "restoring contact")
}

// contacts have no PUT request, and PATCH could retain data that's not
Expand All @@ -159,7 +159,7 @@ func restoreContact(
if shouldDeleteOriginal {
err := cr.DeleteItem(ctx, userID, collisionID)
if err != nil && !errors.Is(err, core.ErrNotFound) {
return nil, graph.Wrap(ctx, err, "deleting colliding contact")
return nil, clues.Wrap(err, "deleting colliding contact")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,8 @@ func (ecc *eventContainerCache) Populate(

err := ecc.addFolder(&cacheFolder)
if err != nil {
errs.AddRecoverable(
ctx,
graph.Stack(ctx, err).Label(fault.LabelForceNoBackupCreation))
err := clues.StackWC(ctx, err).Label(fault.LabelForceNoBackupCreation)
errs.AddRecoverable(ctx, err)
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/internal/m365/collection/exchange/events_restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func restoreEvent(

item, err := er.PostItem(ctx, userID, destinationID, event)
if err != nil {
return nil, graph.Wrap(ctx, err, "restoring event")
return nil, clues.Wrap(err, "restoring event")
}

// events have no PUT request, and PATCH could retain data that's not
Expand All @@ -169,7 +169,7 @@ func restoreEvent(
if shouldDeleteOriginal {
err := er.DeleteItem(ctx, userID, collisionID)
if err != nil && !errors.Is(err, core.ErrNotFound) {
return nil, graph.Wrap(ctx, err, "deleting colliding event")
return nil, clues.Wrap(err, "deleting colliding event")
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/internal/m365/collection/exchange/mail_container_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,8 @@ func (mc *mailContainerCache) Populate(

err := mc.addFolder(&cacheFolder)
if err != nil {
errs.AddRecoverable(
ctx,
graph.Stack(ctx, err).Label(fault.LabelForceNoBackupCreation))
err = clues.StackWC(ctx, err).Label(fault.LabelForceNoBackupCreation)
errs.AddRecoverable(ctx, err)
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/internal/m365/collection/exchange/mail_restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func restoreMail(

item, err := mr.PostItem(ctx, userID, destinationID, msg)
if err != nil {
return nil, graph.Wrap(ctx, err, "restoring mail message")
return nil, clues.Wrap(err, "restoring mail message")
}

// mails have no PUT request, and PATCH could retain data that's not
Expand All @@ -164,7 +164,7 @@ func restoreMail(
if shouldDeleteOriginal {
err := mr.DeleteItem(ctx, userID, collisionID)
if err != nil && !errors.Is(err, core.ErrNotFound) {
return nil, graph.Wrap(ctx, err, "deleting colliding mail message")
return nil, clues.Wrap(err, "deleting colliding mail message")
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/internal/m365/collection/site/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func CollectLibraries(

odcs, canUsePreviousBackup, err := colls.Get(ctx, bpc.MetadataCollections, ssmb, errs)
if err != nil {
return nil, false, graph.Wrap(ctx, err, "getting library")
return nil, false, clues.Wrap(err, "getting library")
}

return append(collections, odcs...), canUsePreviousBackup, nil
Expand Down
4 changes: 2 additions & 2 deletions src/internal/m365/collection/site/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -541,12 +541,12 @@ func serializeContent(

err := writer.WriteObjectValue("", obj)
if err != nil {
return nil, graph.Wrap(ctx, err, "writing to serializer").Label(fault.LabelForceNoBackupCreation)
return nil, clues.WrapWC(ctx, err, "writing to serializer").Label(fault.LabelForceNoBackupCreation)
}

byteArray, err := writer.GetSerializedContent()
if err != nil {
return nil, graph.Wrap(ctx, err, "getting content from writer").Label(fault.LabelForceNoBackupCreation)
return nil, clues.WrapWC(ctx, err, "getting content from writer").Label(fault.LabelForceNoBackupCreation)
}

return byteArray, nil
Expand Down
1 change: 0 additions & 1 deletion src/internal/m365/collection/site/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ func restoreListItem(
}

// Restore to List base to M365 back store

dii.SharePoint = api.ListToSPInfo(restoredList)

return dii, nil
Expand Down
3 changes: 1 addition & 2 deletions src/internal/m365/onedrive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/alcionai/corso/src/pkg/dttm"
"github.com/alcionai/corso/src/pkg/path"
"github.com/alcionai/corso/src/pkg/services/m365/api"
"github.com/alcionai/corso/src/pkg/services/m365/api/graph"
)

var (
Expand Down Expand Up @@ -67,7 +66,7 @@ func mustGetDefaultDriveID(
}

if err != nil {
err = graph.Wrap(ctx, err, "retrieving drive")
err = clues.Wrap(err, "retrieving drive")
}

require.NoError(t, err, clues.ToCore(err))
Expand Down
4 changes: 2 additions & 2 deletions src/internal/m365/service/exchange/enabled.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func GetMailboxInfo(
// First check whether the user is able to access their inbox.
inbox, err := gmb.GetMailInbox(ctx, userID)
if err != nil {
if err := api.EvaluateMailboxError(graph.Stack(ctx, err)); err != nil {
if err := api.EvaluateMailboxError(clues.Stack(err)); err != nil {
logger.CtxErr(ctx, err).Error("getting user's mail folder")

return mi, err
Expand All @@ -74,7 +74,7 @@ func GetMailboxInfo(
logger.CtxErr(ctx, err).Info("err getting user's mailbox settings")

if !graph.IsErrAccessDenied(err) {
return mi, graph.Wrap(ctx, err, "getting user's mailbox settings")
return mi, clues.Wrap(err, "getting user's mailbox settings")
}

logger.CtxErr(ctx, err).Info("mailbox settings access denied")
Expand Down
12 changes: 6 additions & 6 deletions src/internal/m365/service/exchange/enabled_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
odErr := graphTD.ODataErrWithMsg(string(graph.ResourceNotFound), "message")

return mockGMB{
mailboxErr: graph.Stack(ctx, odErr),
mailboxErr: clues.Stack(odErr),
}
},
expect: assert.False,
Expand All @@ -82,7 +82,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
odErr := graphTD.ODataErrWithMsg(string(graph.RequestResourceNotFound), "message")

return mockGMB{
mailboxErr: graph.Stack(ctx, odErr),
mailboxErr: clues.Stack(odErr),
}
},
expect: assert.False,
Expand All @@ -95,7 +95,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
err := clues.Stack(core.ErrNotFound, odErr)

return mockGMB{
mailboxErr: graph.Stack(ctx, err),
mailboxErr: clues.StackWC(ctx, err),
}
},
expect: assert.False,
Expand All @@ -107,7 +107,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
odErr := graphTD.ODataErrWithMsg("code", "message")

return mockGMB{
mailboxErr: graph.Stack(ctx, odErr),
mailboxErr: clues.Stack(odErr),
}
},
expect: assert.False,
Expand Down Expand Up @@ -158,7 +158,7 @@ func (suite *EnabledUnitSuite) TestGetMailboxInfo() {
err := graphTD.ODataErrWithMsg(string(graph.ResourceNotFound), "message")

return mockGMB{
mailboxErr: graph.Stack(ctx, err),
mailboxErr: clues.StackWC(ctx, err),
}
},
expectErr: func(t *testing.T, err error) {
Expand Down Expand Up @@ -219,7 +219,7 @@ func (suite *EnabledUnitSuite) TestGetMailboxInfo() {
return mockGMB{
mailbox: models.NewMailFolder(),
settings: mock.UserSettings(),
inboxMessageErr: graph.Stack(ctx, err),
inboxMessageErr: clues.StackWC(ctx, err),
}
},
expectErr: func(t *testing.T, err error) {
Expand Down
2 changes: 1 addition & 1 deletion src/internal/m365/service/groups/enabled_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
name: "group not found",
mock: func(ctx context.Context) api.GetByIDer[models.Groupable] {
return mockGBI{
err: graph.Stack(ctx, graphTD.ODataErrWithMsg(string(graph.RequestResourceNotFound), "message")),
err: clues.StackWC(ctx, graphTD.ODataErrWithMsg(string(graph.RequestResourceNotFound), "message")),
}
},
expect: assert.False,
Expand Down
9 changes: 6 additions & 3 deletions src/internal/m365/service/onedrive/enabled_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
name: "mysite not found",
mock: func(ctx context.Context) getDefaultDriver {
odErr := graphTD.ODataErrWithMsg("code", string(graph.MysiteNotFound))
// needs graph.Stack, not clues.Stack
return mockDGDD{nil, graph.Stack(ctx, odErr)}
},
expect: assert.False,
Expand All @@ -65,6 +66,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
name: "mysite URL not found",
mock: func(ctx context.Context) getDefaultDriver {
odErr := graphTD.ODataErrWithMsg("code", string(graph.MysiteURLNotFound))
// needs graph.Stack, not clues.Stack
return mockDGDD{nil, graph.Stack(ctx, odErr)}
},
expect: assert.False,
Expand All @@ -76,6 +78,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
name: "no sharepoint license",
mock: func(ctx context.Context) getDefaultDriver {
odErr := graphTD.ODataErrWithMsg("code", string(graph.NoSPLicense))
// needs graph.Stack, not clues.Stack
return mockDGDD{nil, graph.Stack(ctx, odErr)}
},
expect: assert.False,
Expand All @@ -87,7 +90,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
name: "user not found",
mock: func(ctx context.Context) getDefaultDriver {
odErr := graphTD.ODataErrWithMsg(string(graph.RequestResourceNotFound), "message")
return mockDGDD{nil, graph.Stack(ctx, odErr)}
return mockDGDD{nil, clues.StackWC(ctx, odErr)}
},
expect: assert.False,
expectErr: func(t *testing.T, err error) {
Expand All @@ -98,7 +101,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
name: "resource locked",
mock: func(ctx context.Context) getDefaultDriver {
odErr := graphTD.ODataErrWithMsg(string(graph.NotAllowed), "resource")
return mockDGDD{nil, graph.Stack(ctx, odErr)}
return mockDGDD{nil, clues.StackWC(ctx, odErr)}
},
expect: assert.False,
expectErr: func(t *testing.T, err error) {
Expand All @@ -109,7 +112,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
name: "arbitrary error",
mock: func(ctx context.Context) getDefaultDriver {
odErr := graphTD.ODataErrWithMsg("code", "message")
return mockDGDD{nil, graph.Stack(ctx, odErr)}
return mockDGDD{nil, clues.StackWC(ctx, odErr)}
},
expect: assert.False,
expectErr: func(t *testing.T, err error) {
Expand Down
29 changes: 20 additions & 9 deletions src/internal/m365/service/sharepoint/api/pages.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/alcionai/corso/src/internal/diagnostics"
"github.com/alcionai/corso/src/pkg/backup/details"
"github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/services/m365/api/graph"
betamodels "github.com/alcionai/corso/src/pkg/services/m365/api/graph/betasdk/models"
betasites "github.com/alcionai/corso/src/pkg/services/m365/api/graph/betasdk/sites"
)
Expand Down Expand Up @@ -68,9 +67,13 @@ func GetSitePages(
err error
)

page, err = serv.Client().SitesById(siteID).PagesById(pageID).Get(ctx, opts)
page, err = serv.
Client().
SitesById(siteID).
PagesById(pageID).
Get(ctx, opts)
if err != nil {
el.AddRecoverable(ctx, graph.Wrap(ctx, err, "fetching page"))
el.AddRecoverable(ctx, clues.Wrap(err, "fetching page"))
return
}

Expand All @@ -96,7 +99,7 @@ func FetchPages(ctx context.Context, bs *BetaService, siteID string) ([]NameID,
for {
resp, err = builder.Get(ctx, opts)
if err != nil {
return nil, graph.Wrap(ctx, err, "fetching site page")
return nil, clues.Wrap(err, "fetching site page")
}

for _, entry := range resp.GetValue() {
Expand Down Expand Up @@ -146,9 +149,13 @@ func DeleteSitePage(
serv *BetaService,
siteID, pageID string,
) error {
err := serv.Client().SitesById(siteID).PagesById(pageID).Delete(ctx, nil)
err := serv.
Client().
SitesById(siteID).
PagesById(pageID).
Delete(ctx, nil)
if err != nil {
return graph.Wrap(ctx, err, "deleting page")
return clues.Wrap(err, "deleting page")
}

return nil
Expand Down Expand Up @@ -206,9 +213,13 @@ func RestoreSitePage(
// 1. Create the Page on the site
// 2. Publish the site
// See: https://learn.microsoft.com/en-us/graph/api/sitepage-create?view=graph-rest-beta
restoredPage, err := service.Client().SitesById(siteID).Pages().Post(ctx, page, nil)
restoredPage, err := service.
Client().
SitesById(siteID).
Pages().
Post(ctx, page, nil)
if err != nil {
return dii, graph.Wrap(ctx, err, "creating page")
return dii, clues.Wrap(err, "creating page")
}

pageID = ptr.Val(restoredPage.GetId())
Expand All @@ -226,7 +237,7 @@ func RestoreSitePage(
Publish().
Post(ctx, nil)
if err != nil {
return dii, graph.Wrap(ctx, err, "publishing page")
return dii, clues.Wrap(err, "publishing page")
}

dii.SharePoint = PageInfo(restoredPage, int64(len(byteArray)))
Expand Down
5 changes: 2 additions & 3 deletions src/internal/m365/service/sharepoint/enabled_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
name: "no sharepoint license",
mock: func(ctx context.Context) getSiteRooter {
odErr := graphTD.ODataErrWithMsg("code", string(graph.NoSPLicense))

// needs graph.Stack, not clues.StackWC
return mockGSR{nil, graph.Stack(ctx, odErr)}
},
expect: assert.False,
Expand All @@ -70,8 +70,7 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
name: "arbitrary error",
mock: func(ctx context.Context) getSiteRooter {
odErr := graphTD.ODataErrWithMsg("code", "message")

return mockGSR{nil, graph.Stack(ctx, odErr)}
return mockGSR{nil, clues.StackWC(ctx, odErr)}
},
expect: assert.False,
expectErr: func(t *testing.T, err error) {
Expand Down
Loading

0 comments on commit e86592f

Please sign in to comment.