From 03642c517f69862d82d00851dfffbf029ac192c3 Mon Sep 17 00:00:00 2001 From: Keepers Date: Mon, 30 Jan 2023 12:24:44 -0700 Subject: [PATCH] standardize deleteContainer func in exch api (#2246) ## Description Minor refactor that came up while hunting bugs. For the life of me, I cannot find the reason for read/write counts being off in event incrementals. ## Does this PR need a docs update or release note? - [x] :no_entry: No ## Type of change - [x] :broom: Tech Debt/Cleanup ## Issue(s) * #2022 ## Test Plan - [x] :muscle: Manual - [x] :green_heart: E2E --- .../connector/exchange/api/contacts.go | 5 +- src/internal/connector/exchange/api/events.go | 4 +- src/internal/connector/exchange/api/mail.go | 4 +- .../connector/exchange/restore_test.go | 75 +++++++++---------- src/internal/kopia/upload.go | 16 ++++ .../operations/backup_integration_test.go | 21 +++--- src/internal/tester/storage.go | 5 +- 7 files changed, 74 insertions(+), 56 deletions(-) diff --git a/src/internal/connector/exchange/api/contacts.go b/src/internal/connector/exchange/api/contacts.go index 2e9014235..0db1e964c 100644 --- a/src/internal/connector/exchange/api/contacts.go +++ b/src/internal/connector/exchange/api/contacts.go @@ -48,9 +48,8 @@ func (c Contacts) CreateContactFolder( return c.stable.Client().UsersById(user).ContactFolders().Post(ctx, requestBody, nil) } -// DeleteContactFolder deletes the ContactFolder associated with the M365 ID if permissions are valid. -// Errors returned if the function call was not successful. -func (c Contacts) DeleteContactFolder( +// DeleteContainer deletes the ContactFolder associated with the M365 ID if permissions are valid. +func (c Contacts) DeleteContainer( ctx context.Context, user, folderID string, ) error { diff --git a/src/internal/connector/exchange/api/events.go b/src/internal/connector/exchange/api/events.go index b3e6f9467..e643c1f89 100644 --- a/src/internal/connector/exchange/api/events.go +++ b/src/internal/connector/exchange/api/events.go @@ -50,9 +50,9 @@ func (c Events) CreateCalendar( return c.stable.Client().UsersById(user).Calendars().Post(ctx, requestbody, nil) } -// DeleteCalendar removes calendar from user's M365 account +// DeleteContainer removes a calendar from user's M365 account // Reference: https://docs.microsoft.com/en-us/graph/api/calendar-delete?view=graph-rest-1.0&tabs=go -func (c Events) DeleteCalendar( +func (c Events) DeleteContainer( ctx context.Context, user, calendarID string, ) error { diff --git a/src/internal/connector/exchange/api/mail.go b/src/internal/connector/exchange/api/mail.go index 4c9d564f0..bbac48a66 100644 --- a/src/internal/connector/exchange/api/mail.go +++ b/src/internal/connector/exchange/api/mail.go @@ -72,9 +72,9 @@ func (c Mail) CreateMailFolderWithParent( Post(ctx, requestBody, nil) } -// DeleteMailFolder removes a mail folder with the corresponding M365 ID from the user's M365 Exchange account +// DeleteContainer removes a mail folder with the corresponding M365 ID from the user's M365 Exchange account // Reference: https://docs.microsoft.com/en-us/graph/api/mailfolder-delete?view=graph-rest-1.0&tabs=http -func (c Mail) DeleteMailFolder( +func (c Mail) DeleteContainer( ctx context.Context, user, folderID string, ) error { diff --git a/src/internal/connector/exchange/restore_test.go b/src/internal/connector/exchange/restore_test.go index f439ea3ad..9c32fd530 100644 --- a/src/internal/connector/exchange/restore_test.go +++ b/src/internal/connector/exchange/restore_test.go @@ -76,7 +76,7 @@ func (suite *ExchangeRestoreSuite) TestRestoreContact() { defer func() { // Remove the folder containing contact prior to exiting test - err = suite.ac.Contacts().DeleteContactFolder(ctx, userID, folderID) + err = suite.ac.Contacts().DeleteContainer(ctx, userID, folderID) assert.NoError(t, err) }() @@ -110,7 +110,7 @@ func (suite *ExchangeRestoreSuite) TestRestoreEvent() { defer func() { // Removes calendar containing events created during the test - err = suite.ac.Events().DeleteCalendar(ctx, userID, calendarID) + err = suite.ac.Events().DeleteContainer(ctx, userID, calendarID) assert.NoError(t, err) }() @@ -124,6 +124,10 @@ func (suite *ExchangeRestoreSuite) TestRestoreEvent() { assert.NotNil(t, info, "event item info") } +type containerDeleter interface { + DeleteContainer(context.Context, string, string) error +} + // TestRestoreExchangeObject verifies path.Category usage for restored objects func (suite *ExchangeRestoreSuite) TestRestoreExchangeObject() { a := tester.NewM365Account(suite.T()) @@ -133,20 +137,24 @@ func (suite *ExchangeRestoreSuite) TestRestoreExchangeObject() { service, err := createService(m365) require.NoError(suite.T(), err) + deleters := map[path.CategoryType]containerDeleter{ + path.EmailCategory: suite.ac.Mail(), + path.ContactsCategory: suite.ac.Contacts(), + path.EventsCategory: suite.ac.Events(), + } + userID := tester.M365UserID(suite.T()) now := time.Now() tests := []struct { name string bytes []byte category path.CategoryType - cleanupFunc func(context.Context, string, string) error destination func(*testing.T, context.Context) string }{ { - name: "Test Mail", - bytes: mockconnector.GetMockMessageBytes("Restore Exchange Object"), - category: path.EmailCategory, - cleanupFunc: suite.ac.Mail().DeleteMailFolder, + name: "Test Mail", + bytes: mockconnector.GetMockMessageBytes("Restore Exchange Object"), + category: path.EmailCategory, destination: func(t *testing.T, ctx context.Context) string { folderName := "TestRestoreMailObject: " + common.FormatSimpleDateTime(now) folder, err := suite.ac.Mail().CreateMailFolder(ctx, userID, folderName) @@ -156,10 +164,9 @@ func (suite *ExchangeRestoreSuite) TestRestoreExchangeObject() { }, }, { - name: "Test Mail: One Direct Attachment", - bytes: mockconnector.GetMockMessageWithDirectAttachment("Restore 1 Attachment"), - category: path.EmailCategory, - cleanupFunc: suite.ac.Mail().DeleteMailFolder, + name: "Test Mail: One Direct Attachment", + bytes: mockconnector.GetMockMessageWithDirectAttachment("Restore 1 Attachment"), + category: path.EmailCategory, destination: func(t *testing.T, ctx context.Context) string { folderName := "TestRestoreMailwithAttachment: " + common.FormatSimpleDateTime(now) folder, err := suite.ac.Mail().CreateMailFolder(ctx, userID, folderName) @@ -169,10 +176,9 @@ func (suite *ExchangeRestoreSuite) TestRestoreExchangeObject() { }, }, { - name: "Test Mail: One Large Attachment", - bytes: mockconnector.GetMockMessageWithLargeAttachment("Restore Large Attachment"), - category: path.EmailCategory, - cleanupFunc: suite.ac.Mail().DeleteMailFolder, + name: "Test Mail: One Large Attachment", + bytes: mockconnector.GetMockMessageWithLargeAttachment("Restore Large Attachment"), + category: path.EmailCategory, destination: func(t *testing.T, ctx context.Context) string { folderName := "TestRestoreMailwithLargeAttachment: " + common.FormatSimpleDateTime(now) folder, err := suite.ac.Mail().CreateMailFolder(ctx, userID, folderName) @@ -182,10 +188,9 @@ func (suite *ExchangeRestoreSuite) TestRestoreExchangeObject() { }, }, { - name: "Test Mail: Two Attachments", - bytes: mockconnector.GetMockMessageWithTwoAttachments("Restore 2 Attachments"), - category: path.EmailCategory, - cleanupFunc: suite.ac.Mail().DeleteMailFolder, + name: "Test Mail: Two Attachments", + bytes: mockconnector.GetMockMessageWithTwoAttachments("Restore 2 Attachments"), + category: path.EmailCategory, destination: func(t *testing.T, ctx context.Context) string { folderName := "TestRestoreMailwithAttachments: " + common.FormatSimpleDateTime(now) folder, err := suite.ac.Mail().CreateMailFolder(ctx, userID, folderName) @@ -195,10 +200,9 @@ func (suite *ExchangeRestoreSuite) TestRestoreExchangeObject() { }, }, { - name: "Test Mail: Reference(OneDrive) Attachment", - bytes: mockconnector.GetMessageWithOneDriveAttachment("Restore Reference(OneDrive) Attachment"), - category: path.EmailCategory, - cleanupFunc: suite.ac.Mail().DeleteMailFolder, + name: "Test Mail: Reference(OneDrive) Attachment", + bytes: mockconnector.GetMessageWithOneDriveAttachment("Restore Reference(OneDrive) Attachment"), + category: path.EmailCategory, destination: func(t *testing.T, ctx context.Context) string { folderName := "TestRestoreMailwithReferenceAttachment: " + common.FormatSimpleDateTime(now) folder, err := suite.ac.Mail().CreateMailFolder(ctx, userID, folderName) @@ -209,10 +213,9 @@ func (suite *ExchangeRestoreSuite) TestRestoreExchangeObject() { }, // TODO: #884 - reinstate when able to specify root folder by name { - name: "Test Contact", - bytes: mockconnector.GetMockContactBytes("Test_Omega"), - category: path.ContactsCategory, - cleanupFunc: suite.ac.Contacts().DeleteContactFolder, + name: "Test Contact", + bytes: mockconnector.GetMockContactBytes("Test_Omega"), + category: path.ContactsCategory, destination: func(t *testing.T, ctx context.Context) string { folderName := "TestRestoreContactObject: " + common.FormatSimpleDateTime(now) folder, err := suite.ac.Contacts().CreateContactFolder(ctx, userID, folderName) @@ -222,10 +225,9 @@ func (suite *ExchangeRestoreSuite) TestRestoreExchangeObject() { }, }, { - name: "Test Events", - bytes: mockconnector.GetDefaultMockEventBytes("Restored Event Object"), - category: path.EventsCategory, - cleanupFunc: suite.ac.Events().DeleteCalendar, + name: "Test Events", + bytes: mockconnector.GetDefaultMockEventBytes("Restored Event Object"), + category: path.EventsCategory, destination: func(t *testing.T, ctx context.Context) string { calendarName := "TestRestoreEventObject: " + common.FormatSimpleDateTime(now) calendar, err := suite.ac.Events().CreateCalendar(ctx, userID, calendarName) @@ -235,10 +237,9 @@ func (suite *ExchangeRestoreSuite) TestRestoreExchangeObject() { }, }, { - name: "Test Event with Attachment", - bytes: mockconnector.GetMockEventWithAttachment("Restored Event Attachment"), - category: path.EventsCategory, - cleanupFunc: suite.ac.Events().DeleteCalendar, + name: "Test Event with Attachment", + bytes: mockconnector.GetMockEventWithAttachment("Restored Event Attachment"), + category: path.EventsCategory, destination: func(t *testing.T, ctx context.Context) string { calendarName := "TestRestoreEventObject_" + common.FormatSimpleDateTime(now) calendar, err := suite.ac.Events().CreateCalendar(ctx, userID, calendarName) @@ -266,9 +267,7 @@ func (suite *ExchangeRestoreSuite) TestRestoreExchangeObject() { ) assert.NoError(t, err, support.ConnectorStackErrorTrace(err)) assert.NotNil(t, info, "item info is populated") - - cleanupError := test.cleanupFunc(ctx, userID, destination) - assert.NoError(t, cleanupError) + assert.NoError(t, deleters[test.category].DeleteContainer(ctx, userID, destination)) }) } } diff --git a/src/internal/kopia/upload.go b/src/internal/kopia/upload.go index 5301e6872..8ddb46978 100644 --- a/src/internal/kopia/upload.go +++ b/src/internal/kopia/upload.go @@ -3,10 +3,13 @@ package kopia import ( "bytes" "context" + "encoding/base64" "encoding/binary" + "fmt" "io" "os" "runtime/trace" + "strings" "sync" "sync/atomic" "time" @@ -204,6 +207,19 @@ func (cp *corsoProgress) FinishedHashingFile(fname string, bs int64) { // Pass the call through as well so we don't break expected functionality. defer cp.UploadProgress.FinishedHashingFile(fname, bs) + sl := strings.Split(fname, "/") + + for i := range sl { + rdt, err := base64.StdEncoding.DecodeString(sl[i]) + if err != nil { + fmt.Println("f did not decode") + } + + sl[i] = string(rdt) + } + + logger.Ctx(context.Background()).Debugw("finished hashing file", "path", sl[2:]) + atomic.AddInt64(&cp.totalBytes, bs) } diff --git a/src/internal/operations/backup_integration_test.go b/src/internal/operations/backup_integration_test.go index f6e63b022..c28159c1a 100644 --- a/src/internal/operations/backup_integration_test.go +++ b/src/internal/operations/backup_integration_test.go @@ -633,6 +633,8 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { ctx, flush := tester.NewContext() defer flush() + tester.LogTimeOfTest(suite.T()) + var ( t = suite.T() acct = tester.NewM365Account(t) @@ -803,7 +805,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { { name: "move an email folder to a subfolder", updateUserData: func(t *testing.T) { - // contacts cannot be sufoldered; this is an email-only change + // contacts and events cannot be sufoldered; this is an email-only change toContainer := dataset[path.EmailCategory].dests[container1].containerID fromContainer := dataset[path.EmailCategory].dests[container2].containerID @@ -826,23 +828,22 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { updateUserData: func(t *testing.T) { for category, d := range dataset { containerID := d.dests[container2].containerID - cli := gc.Service.Client().UsersById(suite.user) switch category { case path.EmailCategory: require.NoError( t, - cli.MailFoldersById(containerID).Delete(ctx, nil), + ac.Mail().DeleteContainer(ctx, suite.user, containerID), "deleting an email folder") case path.ContactsCategory: require.NoError( t, - cli.ContactFoldersById(containerID).Delete(ctx, nil), + ac.Contacts().DeleteContainer(ctx, suite.user, containerID), "deleting a contacts folder") case path.EventsCategory: require.NoError( t, - cli.CalendarsById(containerID).Delete(ctx, nil), + ac.Events().DeleteContainer(ctx, suite.user, containerID), "deleting a calendar") } } @@ -923,19 +924,19 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { require.NoError(t, err, "updating contact folder name") case path.EventsCategory: - ccf := cli.CalendarsById(containerID) + cbi := cli.CalendarsById(containerID) - body, err := ccf.Get(ctx, nil) + body, err := cbi.Get(ctx, nil) require.NoError(t, err, "getting calendar") body.SetName(&containerRename) - _, err = ccf.Patch(ctx, body, nil) + _, err = cbi.Patch(ctx, body, nil) require.NoError(t, err, "updating calendar name") } } }, - itemsRead: 0, - itemsWritten: 4, + itemsRead: 0, // containers are not counted as reads + itemsWritten: 4, // two items per category }, { name: "add a new item", diff --git a/src/internal/tester/storage.go b/src/internal/tester/storage.go index 0cfabba20..3e16ce05e 100644 --- a/src/internal/tester/storage.go +++ b/src/internal/tester/storage.go @@ -29,11 +29,14 @@ func NewPrefixedS3Storage(t *testing.T) storage.Storage { cfg, err := readTestConfig() require.NoError(t, err, "configuring storage from test file") + prefix := testRepoRootPrefix + t.Name() + "-" + now + t.Logf("testing at s3 bucket [%s] prefix [%s]", cfg[TestCfgBucket], prefix) + st, err := storage.NewStorage( storage.ProviderS3, storage.S3Config{ Bucket: cfg[TestCfgBucket], - Prefix: testRepoRootPrefix + t.Name() + "-" + now, + Prefix: prefix, }, storage.CommonConfig{ Corso: credentials.GetCorso(),