Download attachments for group mailbox posts (#4992)

<!-- PR description-->

* Attachment download needed an expand operation. Added that. 
* Added test coverage with gock. Tested with manual backup of posts which contain attachments(embedded/non-embedded). We can't add e2e tests with attachments, since API to create new conversations requires delegated access.
* Note that https://github.com/alcionai/corso/issues/4991 is still an open item. We don't have a resolution for it right now, since attachment endpoint requires delegated token. Defaulting to let the backup fail for "too many attachments" error case. We don't know yet if we'd see that with group mailboxes, and whether it'd be the same error that we saw with exchange (recurring 503s).
---

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

- [x] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [ ] ⛔ No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* #<issue>

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [x] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E

CHANGELOG.md

@@ -9,10 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 - Retry transient 400 "invalidRequest" errors during onedrive & sharepoint backup.
+- Backup attachments associated with group mailbox items.
 
 ### Changed
 - When running `backup details` on an empty backup returns a more helpful error message.
 
+### Known issues
+- Backing up a group mailbox item may fail if it has a very large number of attachments (500+).
+
 ## [v0.18.0] (beta) - 2024-01-02
 
 ### Fixed

src/pkg/services/m365/api/conversations.go

@@ -11,6 +11,7 @@ import (
 	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/common/str"
 	"github.com/alcionai/corso/src/pkg/backup/details"
+	"github.com/alcionai/corso/src/pkg/logger"
 	"github.com/alcionai/corso/src/pkg/services/m365/api/graph"
 )
 
@@ -63,7 +64,38 @@ func (c Conversations) GetConversationPost(
 		return nil, nil, graph.Stack(ctx, err)
 	}
 
-	return post, conversationPostInfo(post), graph.Stack(ctx, err).OrNil()
+	preview, contentLen, err := getConversationPostContentPreview(post)
+	if err != nil {
+		preview = "malformed or unparseable content body: " + preview
+	}
+
+	if !ptr.Val(post.GetHasAttachments()) && !HasAttachments(post.GetBody()) {
+		return post, conversationPostInfo(post, contentLen, preview), nil
+	}
+
+	attachments, totalSize, err := c.getAttachments(
+		ctx,
+		groupID,
+		conversationID,
+		threadID,
+		postID)
+	if err != nil {
+		// Similar to exchange, a failure can happen if a post has a lot of attachments.
+		// We don't have a fallback option here to fetch attachments one by one. See
+		// issue #4991.
+		//
+		// Resort to failing the post backup for now since we don't know yet how this
+		// error might manifest itself for posts.
+		logger.CtxErr(ctx, err).Info("failed to get post attachments")
+
+		return nil, nil, clues.Stack(err)
+	}
+
+	contentLen += totalSize
+
+	post.SetAttachments(attachments)
+
+	return post, conversationPostInfo(post, contentLen, preview), graph.Stack(ctx, err).OrNil()
 }
 
 // ---------------------------------------------------------------------------
@@ -72,27 +104,18 @@ func (c Conversations) GetConversationPost(
 
 func conversationPostInfo(
 	post models.Postable,
+	size int64,
+	preview string,
 ) *details.GroupsInfo {
 	if post == nil {
 		return nil
 	}
 
-	preview, contentLen, err := getConversationPostContentPreview(post)
-	if err != nil {
-		preview = "malformed or unparseable html" + preview
-	}
-
 	var sender string
 	if post.GetSender() != nil && post.GetSender().GetEmailAddress() != nil {
 		sender = ptr.Val(post.GetSender().GetEmailAddress().GetAddress())
 	}
 
-	size := contentLen
-
-	for _, a := range post.GetAttachments() {
-		size += int64(ptr.Val(a.GetSize()))
-	}
-
 	cpi := details.ConversationPostInfo{
 		CreatedAt: ptr.Val(post.GetCreatedDateTime()),
 		Creator:   sender,
@@ -128,3 +151,44 @@ func stripConversationPostHTML(post models.Postable) (string, int64, error) {
 
 	return content, origSize, clues.Stack(err).OrNil()
 }
+
+// getAttachments attempts to get all attachments, including their content, in a singe query.
+func (c Conversations) getAttachments(
+	ctx context.Context,
+	groupID, conversationID, threadID, postID string,
+) ([]models.Attachmentable, int64, error) {
+	var (
+		result    = []models.Attachmentable{}
+		totalSize int64
+	)
+
+	cfg := &groups.ItemConversationsItemThreadsItemPostsPostItemRequestBuilderGetRequestConfiguration{
+		QueryParameters: &groups.ItemConversationsItemThreadsItemPostsPostItemRequestBuilderGetQueryParameters{
+			Expand: []string{"attachments"},
+		},
+	}
+
+	post, err := c.LargeItem.
+		Client().
+		Groups().
+		ByGroupId(groupID).
+		Conversations().
+		ByConversationId(conversationID).
+		Threads().
+		ByConversationThreadId(threadID).
+		Posts().
+		ByPostId(postID).
+		Get(ctx, cfg)
+	if err != nil {
+		return nil, 0, graph.Stack(ctx, err)
+	}
+
+	attachments := post.GetAttachments()
+
+	for _, a := range attachments {
+		totalSize += int64(ptr.Val(a.GetSize()))
+		result = append(result, a)
+	}
+
+	return result, totalSize, nil
+}

src/pkg/services/m365/api/conversations_test.go

@@ -108,8 +108,8 @@ func (suite *ConversationsAPIUnitSuite) TestConversationPostInfo() {
 		suite.Run(test.name, func() {
 			t := suite.T()
 
-			chMsg, expected := test.postAndInfo()
+			post, expected := test.postAndInfo()
-			result := conversationPostInfo(chMsg)
+			result := conversationPostInfo(post, 0, "")
 
 			assert.Equal(t, expected, result)
 		})
@@ -138,6 +138,7 @@ func (suite *ConversationAPIIntgSuite) SetupSuite() {
 func (suite *ConversationAPIIntgSuite) TestConversations_attachmentListDownload() {
 	pid := "fake-post-id"
 	aid := "fake-attachment-id"
+	contentWithAttachment := "<html><body><img src=\"cid:fake-attach-id\"/></body></html>"
 
 	tests := []struct {
 		name            string
@@ -173,6 +174,19 @@ func (suite *ConversationAPIIntgSuite) TestConversations_attachmentListDownload(
 				itm.SetId(&pid)
 				itm.SetHasAttachments(ptr.To(true))
 
+				// First call to get the post will not expand attachments.
+				interceptV1Path(
+					"groups",
+					"group",
+					"conversations",
+					"conv",
+					"threads",
+					"thread",
+					"posts",
+					pid).
+					Reply(200).
+					JSON(graphTD.ParseableToMap(suite.T(), itm))
+
 				attch := models.NewAttachment()
 				attch.SetSize(ptr.To[int32](50))
 
@@ -187,6 +201,7 @@ func (suite *ConversationAPIIntgSuite) TestConversations_attachmentListDownload(
 					"thread",
 					"posts",
 					pid).
+					MatchParam("$expand", "attachments").
 					Reply(200).
 					JSON(graphTD.ParseableToMap(suite.T(), itm))
 			},
@@ -197,10 +212,22 @@ func (suite *ConversationAPIIntgSuite) TestConversations_attachmentListDownload(
 		{
 			name: "fetch multiple individual attachments",
 			setupf: func() {
-				truthy := true
 				itm := models.NewPost()
+
 				itm.SetId(&pid)
-				itm.SetHasAttachments(&truthy)
+				itm.SetHasAttachments(ptr.To(true))
+
+				interceptV1Path(
+					"groups",
+					"group",
+					"conversations",
+					"conv",
+					"threads",
+					"thread",
+					"posts",
+					pid).
+					Reply(200).
+					JSON(graphTD.ParseableToMap(suite.T(), itm))
 
 				attch := models.NewAttachment()
 				attch.SetId(&aid)
@@ -217,6 +244,7 @@ func (suite *ConversationAPIIntgSuite) TestConversations_attachmentListDownload(
 					"thread",
 					"posts",
 					pid).
+					MatchParam("$expand", "attachments").
 					Reply(200).
 					JSON(graphTD.ParseableToMap(suite.T(), itm))
 			},
@@ -224,6 +252,54 @@ func (suite *ConversationAPIIntgSuite) TestConversations_attachmentListDownload(
 			size:            1000,
 			expect:          assert.NoError,
 		},
+		{
+			name: "embedded attachment",
+			setupf: func() {
+				itm := models.NewPost()
+				itm.SetId(&pid)
+
+				body := models.NewItemBody()
+				body.SetContentType(ptr.To(models.HTML_BODYTYPE))
+
+				// Test html content with embedded attachment.
+
+				body.SetContent(ptr.To(contentWithAttachment))
+
+				itm.SetBody(body)
+
+				interceptV1Path(
+					"groups",
+					"group",
+					"conversations",
+					"conv",
+					"threads",
+					"thread",
+					"posts",
+					pid).
+					Reply(200).
+					JSON(graphTD.ParseableToMap(suite.T(), itm))
+
+				attch := models.NewAttachment()
+				attch.SetSize(ptr.To[int32](50))
+				itm.SetAttachments([]models.Attachmentable{attch})
+
+				interceptV1Path(
+					"groups",
+					"group",
+					"conversations",
+					"conv",
+					"threads",
+					"thread",
+					"posts",
+					pid).
+					MatchParam("$expand", "attachments").
+					Reply(200).
+					JSON(graphTD.ParseableToMap(suite.T(), itm))
+			},
+			attachmentCount: 1,
+			size:            50 + int64(len(contentWithAttachment)),
+			expect:          assert.NoError,
+		},
 	}
 
 	for _, test := range tests {

website/docs/support/known-issues.md

@@ -34,4 +34,6 @@ Below is a list of known Corso issues and limitations:
 
 * Groups and Teams support is available in an early-access status, and may be subject to breaking changes.
 
-* Restoring the data into a different Group from the one it was backed up from isn't currently supported
+* Restoring the data into a different Group from the one it was backed up from isn't currently supported.
+
+* Backing up a group mailbox item may fail if it has a large number of attachments (500+).