diff --git a/CHANGELOG.md b/CHANGELOG.md index 746944f02..14a11c924 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Catch and report cases where a protected resource is locked out of access. SDK consumers have a new errs sentinel that allows them to check for this case. - Fix a case where missing item LastModifiedTimes could cause incremental backups to fail. +- Email size metadata was incorrectly set to the size of the last attachment. Emails will now correctly report the size of the mail content plus the size of all attachments. ## [v0.14.0] (beta) - 2023-10-09 diff --git a/src/internal/kopia/upload.go b/src/internal/kopia/upload.go index 6030ec838..d99fe9cd3 100644 --- a/src/internal/kopia/upload.go +++ b/src/internal/kopia/upload.go @@ -148,6 +148,10 @@ func (cp *corsoProgress) FinishedFile(relativePath string, err error) { Label(fault.LabelForceNoBackupCreation)) return + } else if info.Modified().IsZero() { + cp.errs.AddRecoverable(ctx, clues.New("zero-valued mod time"). + WithClues(ctx). + Label(fault.LabelForceNoBackupCreation)) } err = cp.deets.Add(d.repoPath, d.locationPath, info) diff --git a/src/internal/kopia/wrapper_test.go b/src/internal/kopia/wrapper_test.go index 2aad0c3dc..b1578d746 100644 --- a/src/internal/kopia/wrapper_test.go +++ b/src/internal/kopia/wrapper_test.go @@ -1067,6 +1067,7 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_NoDetailsForMeta() { DriveID: "drive-id", DriveName: "drive-name", ItemName: "item", + Modified: time.Now(), } // tags that are supplied by the caller. This includes basic tags to support @@ -1124,10 +1125,11 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_NoDetailsForMeta() { info.ItemName = name ms := &dataMock.Item{ - ItemID: name, - Reader: io.NopCloser(&bytes.Buffer{}), - ItemSize: 0, - ItemInfo: details.ItemInfo{OneDrive: &info}, + ItemID: name, + Reader: io.NopCloser(&bytes.Buffer{}), + ItemSize: 0, + ItemInfo: details.ItemInfo{OneDrive: &info}, + ModifiedTime: info.Modified, } streams = append(streams, ms) @@ -1152,12 +1154,15 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_NoDetailsForMeta() { cols: func() []data.BackupCollection { info := baseOneDriveItemInfo info.ItemName = testFileName + // Update the mod time so it's not counted as cached. + info.Modified = info.Modified.Add(time.Hour) ms := &dataMock.Item{ - ItemID: testFileName, - Reader: io.NopCloser(&bytes.Buffer{}), - ItemSize: 0, - ItemInfo: details.ItemInfo{OneDrive: &info}, + ItemID: testFileName, + Reader: io.NopCloser(&bytes.Buffer{}), + ItemSize: 0, + ItemInfo: details.ItemInfo{OneDrive: &info}, + ModifiedTime: info.Modified, } mc := &dataMock.Collection{ @@ -1305,20 +1310,25 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_ReaderError() { loc2 := path.Builder{}.Append(suite.storePath2.Folders()...) r := identity.NewReason(testTenant, testUser, path.ExchangeService, path.EmailCategory) + info := exchMock.StubMailInfo() + info.Exchange.Modified = time.Now() + collections := []data.BackupCollection{ &dataMock.Collection{ Path: suite.storePath1, Loc: loc1, ItemData: []data.Item{ &dataMock.Item{ - ItemID: testFileName, - Reader: io.NopCloser(bytes.NewReader(testFileData)), - ItemInfo: exchMock.StubMailInfo(), + ItemID: testFileName, + Reader: io.NopCloser(bytes.NewReader(testFileData)), + ModifiedTime: info.Modified(), + ItemInfo: info, }, &dataMock.Item{ - ItemID: testFileName2, - Reader: io.NopCloser(bytes.NewReader(testFileData2)), - ItemInfo: exchMock.StubMailInfo(), + ItemID: testFileName2, + Reader: io.NopCloser(bytes.NewReader(testFileData2)), + ModifiedTime: info.Modified(), + ItemInfo: info, }, }, }, @@ -1327,24 +1337,28 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_ReaderError() { Loc: loc2, ItemData: []data.Item{ &dataMock.Item{ - ItemID: testFileName3, - Reader: io.NopCloser(bytes.NewReader(testFileData3)), - ItemInfo: exchMock.StubMailInfo(), + ItemID: testFileName3, + Reader: io.NopCloser(bytes.NewReader(testFileData3)), + ModifiedTime: info.Modified(), + ItemInfo: info, }, &dataMock.Item{ - ItemID: testFileName4, - ReadErr: assert.AnError, - ItemInfo: exchMock.StubMailInfo(), + ItemID: testFileName4, + ReadErr: assert.AnError, + ModifiedTime: info.Modified(), + ItemInfo: info, }, &dataMock.Item{ - ItemID: testFileName5, - Reader: io.NopCloser(bytes.NewReader(testFileData5)), - ItemInfo: exchMock.StubMailInfo(), + ItemID: testFileName5, + Reader: io.NopCloser(bytes.NewReader(testFileData5)), + ModifiedTime: info.Modified(), + ItemInfo: info, }, &dataMock.Item{ - ItemID: testFileName6, - Reader: io.NopCloser(bytes.NewReader(testFileData6)), - ItemInfo: exchMock.StubMailInfo(), + ItemID: testFileName6, + Reader: io.NopCloser(bytes.NewReader(testFileData6)), + ModifiedTime: info.Modified(), + ItemInfo: info, }, }, }, diff --git a/src/pkg/services/m365/api/drive_test.go b/src/pkg/services/m365/api/drive_test.go index 74a4b6239..8a61a299c 100644 --- a/src/pkg/services/m365/api/drive_test.go +++ b/src/pkg/services/m365/api/drive_test.go @@ -3,6 +3,7 @@ package api_test import ( "fmt" "testing" + "time" "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" @@ -89,6 +90,9 @@ func (suite *DriveAPIIntgSuite) TestDrives_PostItemInContainer() { control.Copy) require.NoError(t, err, clues.ToCore(err)) + // ensure we don't bucket the mod time within a second + time.Sleep(2 * time.Second) + updatedFile := models.NewDriveItem() updatedFile.SetAdditionalData(origFile.GetAdditionalData()) updatedFile.SetCreatedBy(origFile.GetCreatedBy()) @@ -222,7 +226,7 @@ func (suite *DriveAPIIntgSuite) TestDrives_PostItemInContainer() { assert.True( t, ptr.Val(origFile.GetLastModifiedDateTime()).Before(ptr.Val(i.GetLastModifiedDateTime())), - "replaced item should have the same mod time") + "replaced item should have a later mod time") }, }, } diff --git a/src/pkg/services/m365/api/mail.go b/src/pkg/services/m365/api/mail.go index 59ad150ac..5c0be3b91 100644 --- a/src/pkg/services/m365/api/mail.go +++ b/src/pkg/services/m365/api/mail.go @@ -304,7 +304,7 @@ func (c Mail) GetItem( if err == nil { for _, a := range attached.GetValue() { attachSize := ptr.Val(a.GetSize()) - size = +int64(attachSize) + size += int64(attachSize) } mail.SetAttachments(attached.GetValue()) @@ -380,7 +380,7 @@ func (c Mail) GetItem( atts = append(atts, att) attachSize := ptr.Val(a.GetSize()) - size = +int64(attachSize) + size += int64(attachSize) } mail.SetAttachments(atts) diff --git a/src/pkg/services/m365/api/mail_test.go b/src/pkg/services/m365/api/mail_test.go index 5e0a7f968..67554126b 100644 --- a/src/pkg/services/m365/api/mail_test.go +++ b/src/pkg/services/m365/api/mail_test.go @@ -231,20 +231,20 @@ func (suite *MailAPIIntgSuite) TestHugeAttachmentListDownload() { { name: "fetch with attachment", setupf: func() { - mitem := models.NewMessage() - mitem.SetId(&mid) - mitem.SetHasAttachments(ptr.To(true)) + email := models.NewMessage() + email.SetId(&mid) + email.SetHasAttachments(ptr.To(true)) interceptV1Path("users", "user", "messages", mid). Reply(200). - JSON(parseableToMap(suite.T(), mitem)) + JSON(parseableToMap(suite.T(), email)) atts := models.NewAttachmentCollectionResponse() - aitem := models.NewAttachment() + attch := models.NewAttachment() - asize := int32(50) - aitem.SetSize(&asize) - atts.SetValue([]models.Attachmentable{aitem}) + size := int32(50) + attch.SetSize(&size) + atts.SetValue([]models.Attachmentable{attch}) interceptV1Path("users", "user", "messages", mid, "attachments"). Reply(200). @@ -257,23 +257,22 @@ func (suite *MailAPIIntgSuite) TestHugeAttachmentListDownload() { { name: "fetch individual attachment", setupf: func() { - truthy := true - mitem := models.NewMessage() - mitem.SetId(&mid) - mitem.SetHasAttachments(&truthy) + email := models.NewMessage() + email.SetId(&mid) + email.SetHasAttachments(ptr.To(true)) interceptV1Path("users", "user", "messages", mid). Reply(200). - JSON(parseableToMap(suite.T(), mitem)) + JSON(parseableToMap(suite.T(), email)) atts := models.NewAttachmentCollectionResponse() - aitem := models.NewAttachment() - aitem.SetId(&aid) + attch := models.NewAttachment() + attch.SetId(&aid) - asize := int32(200) - aitem.SetSize(&asize) + size := int32(200) + attch.SetSize(&size) - atts.SetValue([]models.Attachmentable{aitem}) + atts.SetValue([]models.Attachmentable{attch}) interceptV1Path("users", "user", "messages", mid, "attachments"). Reply(503) @@ -284,7 +283,7 @@ func (suite *MailAPIIntgSuite) TestHugeAttachmentListDownload() { interceptV1Path("users", "user", "messages", mid, "attachments", aid). Reply(200). - JSON(parseableToMap(suite.T(), aitem)) + JSON(parseableToMap(suite.T(), attch)) }, attachmentCount: 1, size: 200, @@ -294,22 +293,22 @@ func (suite *MailAPIIntgSuite) TestHugeAttachmentListDownload() { name: "fetch multiple individual attachments", setupf: func() { truthy := true - mitem := models.NewMessage() - mitem.SetId(&mid) - mitem.SetHasAttachments(&truthy) + email := models.NewMessage() + email.SetId(&mid) + email.SetHasAttachments(&truthy) interceptV1Path("users", "user", "messages", mid). Reply(200). - JSON(parseableToMap(suite.T(), mitem)) + JSON(parseableToMap(suite.T(), email)) atts := models.NewAttachmentCollectionResponse() - aitem := models.NewAttachment() - aitem.SetId(&aid) + attch := models.NewAttachment() + attch.SetId(&aid) asize := int32(200) - aitem.SetSize(&asize) + attch.SetSize(&asize) - atts.SetValue([]models.Attachmentable{aitem, aitem, aitem, aitem, aitem}) + atts.SetValue([]models.Attachmentable{attch, attch, attch, attch, attch}) interceptV1Path("users", "user", "messages", mid, "attachments"). Reply(503) @@ -321,24 +320,24 @@ func (suite *MailAPIIntgSuite) TestHugeAttachmentListDownload() { for i := 0; i < 5; i++ { interceptV1Path("users", "user", "messages", mid, "attachments", aid). Reply(200). - JSON(parseableToMap(suite.T(), aitem)) + JSON(parseableToMap(suite.T(), attch)) } }, attachmentCount: 5, - size: 200, + size: 1000, expect: assert.NoError, }, } - for _, tt := range tests { - suite.Run(tt.name, func() { + for _, test := range tests { + suite.Run(test.name, func() { t := suite.T() ctx, flush := tester.NewContext(t) defer flush() defer gock.Off() - tt.setupf() + test.setupf() item, _, err := suite.its.gockAC.Mail().GetItem( ctx, @@ -346,28 +345,26 @@ func (suite *MailAPIIntgSuite) TestHugeAttachmentListDownload() { mid, false, fault.New(true)) - tt.expect(t, err) + test.expect(t, err) it, ok := item.(models.Messageable) require.True(t, ok, "convert to messageable") var size int64 - mailBody := it.GetBody() - if mailBody != nil { - content := ptr.Val(mailBody.GetContent()) - if len(content) > 0 { - size = int64(len(content)) - } + + if it.GetBody() != nil { + content := ptr.Val(it.GetBody().GetContent()) + size = int64(len(content)) } attachments := it.GetAttachments() for _, attachment := range attachments { - size = +int64(*attachment.GetSize()) + size += int64(*attachment.GetSize()) } assert.Equal(t, *it.GetId(), mid) - assert.Equal(t, tt.attachmentCount, len(attachments), "attachment count") - assert.Equal(t, tt.size, size, "mail size") + assert.Equal(t, test.attachmentCount, len(attachments), "attachment count") + assert.Equal(t, test.size, size, "mail size") assert.True(t, gock.IsDone(), "made all requests") }) }