fix message size report (#4493)

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

- [x] ✅ Yes, it's included

#### Type of change

- [x] 🐛 Bugfix

#### Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E

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
 

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)

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,
+						ItemID:       name,
-						Reader:   io.NopCloser(&bytes.Buffer{}),
+						Reader:       io.NopCloser(&bytes.Buffer{}),
-						ItemSize: 0,
+						ItemSize:     0,
-						ItemInfo: details.ItemInfo{OneDrive: &info},
+						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,
+					ItemID:       testFileName,
-					Reader:   io.NopCloser(&bytes.Buffer{}),
+					Reader:       io.NopCloser(&bytes.Buffer{}),
-					ItemSize: 0,
+					ItemSize:     0,
-					ItemInfo: details.ItemInfo{OneDrive: &info},
+					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,
+					ItemID:       testFileName,
-					Reader:   io.NopCloser(bytes.NewReader(testFileData)),
+					Reader:       io.NopCloser(bytes.NewReader(testFileData)),
-					ItemInfo: exchMock.StubMailInfo(),
+					ModifiedTime: info.Modified(),
+					ItemInfo:     info,
 				},
 				&dataMock.Item{
-					ItemID:   testFileName2,
+					ItemID:       testFileName2,
-					Reader:   io.NopCloser(bytes.NewReader(testFileData2)),
+					Reader:       io.NopCloser(bytes.NewReader(testFileData2)),
-					ItemInfo: exchMock.StubMailInfo(),
+					ModifiedTime: info.Modified(),
+					ItemInfo:     info,
 				},
 			},
 		},
@@ -1327,24 +1337,28 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_ReaderError() {
 			Loc:  loc2,
 			ItemData: []data.Item{
 				&dataMock.Item{
-					ItemID:   testFileName3,
+					ItemID:       testFileName3,
-					Reader:   io.NopCloser(bytes.NewReader(testFileData3)),
+					Reader:       io.NopCloser(bytes.NewReader(testFileData3)),
-					ItemInfo: exchMock.StubMailInfo(),
+					ModifiedTime: info.Modified(),
+					ItemInfo:     info,
 				},
 				&dataMock.Item{
-					ItemID:   testFileName4,
+					ItemID:       testFileName4,
-					ReadErr:  assert.AnError,
+					ReadErr:      assert.AnError,
-					ItemInfo: exchMock.StubMailInfo(),
+					ModifiedTime: info.Modified(),
+					ItemInfo:     info,
 				},
 				&dataMock.Item{
-					ItemID:   testFileName5,
+					ItemID:       testFileName5,
-					Reader:   io.NopCloser(bytes.NewReader(testFileData5)),
+					Reader:       io.NopCloser(bytes.NewReader(testFileData5)),
-					ItemInfo: exchMock.StubMailInfo(),
+					ModifiedTime: info.Modified(),
+					ItemInfo:     info,
 				},
 				&dataMock.Item{
-					ItemID:   testFileName6,
+					ItemID:       testFileName6,
-					Reader:   io.NopCloser(bytes.NewReader(testFileData6)),
+					Reader:       io.NopCloser(bytes.NewReader(testFileData6)),
-					ItemInfo: exchMock.StubMailInfo(),
+					ModifiedTime: info.Modified(),
+					ItemInfo:     info,
 				},
 			},
 		},

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")
 			},
 		},
 	}

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)

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()
+				email := models.NewMessage()
-				mitem.SetId(&mid)
+				email.SetId(&mid)
-				mitem.SetHasAttachments(ptr.To(true))
+				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)
+				size := int32(50)
-				aitem.SetSize(&asize)
+				attch.SetSize(&size)
-				atts.SetValue([]models.Attachmentable{aitem})
+				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
+				email := models.NewMessage()
-				mitem := models.NewMessage()
+				email.SetId(&mid)
-				mitem.SetId(&mid)
+				email.SetHasAttachments(ptr.To(true))
-				mitem.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()
+				attch := models.NewAttachment()
-				aitem.SetId(&aid)
+				attch.SetId(&aid)
 
-				asize := int32(200)
+				size := int32(200)
-				aitem.SetSize(&asize)
+				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()
+				email := models.NewMessage()
-				mitem.SetId(&mid)
+				email.SetId(&mid)
-				mitem.SetHasAttachments(&truthy)
+				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()
+				attch := models.NewAttachment()
-				aitem.SetId(&aid)
+				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 {
+	for _, test := range tests {
-		suite.Run(tt.name, func() {
+		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 {
+			if it.GetBody() != nil {
-				content := ptr.Val(mailBody.GetContent())
+				content := ptr.Val(it.GetBody().GetContent())
-				if len(content) > 0 {
+				size = int64(len(content))
-					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, test.attachmentCount, len(attachments), "attachment count")
-			assert.Equal(t, tt.size, size, "mail size")
+			assert.Equal(t, test.size, size, "mail size")
 			assert.True(t, gock.IsDone(), "made all requests")
 		})
 	}