From 411ef240240fc73284a7dfacba9dc1142d41ff7f Mon Sep 17 00:00:00 2001 From: Abhishek Pandey Date: Sun, 11 Feb 2024 14:13:20 -0800 Subject: [PATCH] Handle item attachments with missing name (#5209) Extending the earlier fix in https://github.com/alcionai/corso/pull/5199 to `itemAttachments`. Posts are not impacted here since they don't have attachment types like messages do. --- #### Does this PR need a docs update or release note? - [x] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [ ] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Issue(s) * # #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/converters/eml/eml.go | 11 +- src/internal/converters/eml/eml_test.go | 45 +++--- .../eml/testdata/email-within-email.json | 140 ++++++++++++++++++ 3 files changed, 176 insertions(+), 20 deletions(-) diff --git a/src/internal/converters/eml/eml.go b/src/internal/converters/eml/eml.go index 35c3519e5..29f7bf455 100644 --- a/src/internal/converters/eml/eml.go +++ b/src/internal/converters/eml/eml.go @@ -208,6 +208,15 @@ func getItemAttachment(ctx context.Context, attachment models.Attachmentable) (* With("attachment_id", ptr.Val(attachment.GetId())) } + name := ptr.Val(attachment.GetName()) + if len(name) == 0 { + // Graph as of now does not let us create any attachments + // without a name, but we have run into instances where we have + // see attachments without a name, possibly from old + // data. This is for those cases. + name = "Unnamed" + } + switch it := it.(type) { case *models.Message: cb, err := FromMessageable(ctx, it) @@ -217,7 +226,7 @@ func getItemAttachment(ctx context.Context, attachment models.Attachmentable) (* } return &mail.File{ - Name: ptr.Val(attachment.GetName()), + Name: name, MimeType: "message/rfc822", Data: []byte(cb), }, nil diff --git a/src/internal/converters/eml/eml_test.go b/src/internal/converters/eml/eml_test.go index a1c0fd5db..862842907 100644 --- a/src/internal/converters/eml/eml_test.go +++ b/src/internal/converters/eml/eml_test.go @@ -137,6 +137,11 @@ func (suite *EMLUnitSuite) TestConvert_messageble_to_eml() { } func (suite *EMLUnitSuite) TestConvert_edge_cases() { + bodies := []string{ + testdata.EmailWithAttachments, + testdata.EmailWithinEmail, + } + tests := []struct { name string transform func(models.Messageable) @@ -202,33 +207,35 @@ func (suite *EMLUnitSuite) TestConvert_edge_cases() { }, } - for _, test := range tests { - suite.Run(test.name, func() { - t := suite.T() + for _, b := range bodies { + for _, test := range tests { + suite.Run(test.name, func() { + t := suite.T() - ctx, flush := tester.NewContext(t) - defer flush() + ctx, flush := tester.NewContext(t) + defer flush() - body := []byte(testdata.EmailWithAttachments) + body := []byte(b) - msg, err := api.BytesToMessageable(body) - require.NoError(t, err, "creating message") + msg, err := api.BytesToMessageable(body) + require.NoError(t, err, "creating message") - test.transform(msg) + test.transform(msg) - writer := kjson.NewJsonSerializationWriter() + writer := kjson.NewJsonSerializationWriter() - defer writer.Close() + defer writer.Close() - err = writer.WriteObjectValue("", msg) - require.NoError(t, err, "serializing message") + err = writer.WriteObjectValue("", msg) + require.NoError(t, err, "serializing message") - nbody, err := writer.GetSerializedContent() - require.NoError(t, err, "getting serialized content") + nbody, err := writer.GetSerializedContent() + require.NoError(t, err, "getting serialized content") - _, err = FromJSON(ctx, nbody) - assert.NoError(t, err, "converting to eml") - }) + _, err = FromJSON(ctx, nbody) + assert.NoError(t, err, "converting to eml") + }) + } } } @@ -461,7 +468,7 @@ func (suite *EMLUnitSuite) TestConvert_message_in_messageble_to_eml() { assert.Equal(t, formatAddress(msg.GetFrom().GetEmailAddress()), eml.GetHeader("From")) attachments := eml.Attachments - assert.Equal(t, 1, len(attachments), "attachment count in parent email") + assert.Equal(t, 3, len(attachments), "attachment count in parent email") ieml, err := enmime.ReadEnvelope(strings.NewReader(string(attachments[0].Content))) require.NoError(t, err, "reading created eml") diff --git a/src/internal/converters/eml/testdata/email-within-email.json b/src/internal/converters/eml/testdata/email-within-email.json index 58263ae8d..aed67dc2b 100644 --- a/src/internal/converters/eml/testdata/email-within-email.json +++ b/src/internal/converters/eml/testdata/email-within-email.json @@ -77,6 +77,146 @@ ], "webLink": "https://outlook.office365.com/owa/?AttachmentItemID=AAMkAGJiZmE2NGU4LTQ4YjktNDI1Mi1iMWQzLTQ1MmMxODJkZmQyNABGAAAAAABFdiK7oifWRb4ADuqgSRcnBwBBFDg0JJk7TY1fmsJrh7tNAAAAAAEJAABBFDg0JJk7TY1fmsJrh7tNAAFnbV%2FqAAABEgAQAEUyH0VS3HJBgHDlZdWZl0k%3D&exvsurl=1&viewmodel=ItemAttachment" } + }, + { + "id": "AAMkAGJiZmE2NGU4LTQ4YjktNDI1Mi1iMWQzLTQ1MmMxODJkZmQyNABGAAAAAABFdiK7oifWRb4ADuqgSRcnBwBBFDg0JJk7TY1fmsJrh7tNAAAAAAEJAABBFDg0JJk7TY1fmsJrh7tNAAFnbV-qAAABEgAQAEUyH0VS3HJBgHDlZdWZl02=", + "@odata.type": "#microsoft.graph.itemAttachment", + "item@odata.navigationLink": "https://graph.microsoft.com/v1.0/users('7ceb8e03-bdc5-4509-a136-457526165ec0')/messages('')", + "item@odata.associationLink": "https://graph.microsoft.com/v1.0/users('7ceb8e03-bdc5-4509-a136-457526165ec0')/messages('')/$ref", + "isInline": false, + "lastModifiedDateTime": "2024-02-05T09:33:46Z", + "name": "Purpose of life part 2", + "size": 11840, + "item": { + "id": "", + "@odata.type": "#microsoft.graph.message", + "createdDateTime": "2024-02-05T09:33:24Z", + "lastModifiedDateTime": "2024-02-05T09:33:46Z", + "attachments": [ + { + "id": "AAMkAGJiZmE2NGU4LTQ4YjktNDI1Mi1iMWQzLTQ1MmMxODJkZmQyNABGAAAAAABFdiK7oifWRb4ADuqgSRcnBwBBFDg0JJk7TY1fmsJrh7tNAAAAAAEJAABBFDg0JJk7TY1fmsJrh7tNAAFnbV-qAAACEgAQAEUyH0VS3HJBgHDlZdWZl0kSABAAjBhd4-oQaUS969pTkS-gzA==", + "@odata.type": "#microsoft.graph.fileAttachment", + "@odata.mediaContentType": "text/calendar", + "contentType": "text/calendar", + "isInline": false, + "lastModifiedDateTime": "2024-02-05T09:33:46Z", + "name": "Abidjan.ics", + "size": 573, + "contentBytes": "QkVHSU46VkNBTEVOREFSDQpQUk9ESUQ6LS8vdHp1cmwub3JnLy9OT05TR01MIE9sc29uIDIwMjNkLy9FTg0KVkVSU0lPTjoyLjANCkJFR0lOOlZUSU1FWk9ORQ0KVFpJRDpBZnJpY2EvQWJpZGphbg0KTEFTVC1NT0RJRklFRDoyMDIzMTIyMlQyMzMzNThaDQpUWlVSTDpodHRwczovL3d3dy50enVybC5vcmcvem9uZWluZm8vQWZyaWNhL0FiaWRqYW4NClgtTElDLUxPQ0FUSU9OOkFmcmljYS9BYmlkamFuDQpYLVBST0xFUFRJQy1UWk5BTUU6TE1UDQpCRUdJTjpTVEFOREFSRA0KVFpOQU1FOkdNVA0KVFpPRkZTRVRGUk9NOi0wMDE2MDgNClRaT0ZGU0VUVE86KzAwMDANCkRUU1RBUlQ6MTkxMjAxMDFUMDAwMDAwDQpFTkQ6U1RBTkRBUkQNCkVORDpWVElNRVpPTkUNCkVORDpWQ0FMRU5EQVINCg==" + } + ], + "body": { + "content": "\r\n
I just realized the purpose of my life is to be a test case. Good to know.
", + "contentType": "html" + }, + "bodyPreview": "I just realized the purpose of my life is to be a test case. Good to know.", + "conversationId": "AAQkAGJiZmE2NGU4LTQ4YjktNDI1Mi1iMWQzLTQ1MmMxODJkZmQyNAAQAFEnxDqYmbJEm8d2l3qfS6A=", + "conversationIndex": "AQHaWBYiUSfEOpiZskSbx3aXep9LoA==", + "flag": { + "flagStatus": "notFlagged" + }, + "from": { + "emailAddress": { + "address": "JohannaL@10rqc2.onmicrosoft.com", + "name": "Johanna Lorenz" + } + }, + "hasAttachments": true, + "importance": "normal", + "internetMessageId": "", + "isDeliveryReceiptRequested": false, + "isDraft": false, + "isRead": true, + "isReadReceiptRequested": false, + "receivedDateTime": "2024-02-05T09:33:12Z", + "sender": { + "emailAddress": { + "address": "JohannaL@10rqc2.onmicrosoft.com", + "name": "Johanna Lorenz" + } + }, + "sentDateTime": "2024-02-05T09:33:11Z", + "subject": "Purpose of life", + "toRecipients": [ + { + "emailAddress": { + "address": "PradeepG@10rqc2.onmicrosoft.com", + "name": "Pradeep Gupta" + } + } + ], + "webLink": "https://outlook.office365.com/owa/?AttachmentItemID=AAMkAGJiZmE2NGU4LTQ4YjktNDI1Mi1iMWQzLTQ1MmMxODJkZmQyNABGAAAAAABFdiK7oifWRb4ADuqgSRcnBwBBFDg0JJk7TY1fmsJrh7tNAAAAAAEJAABBFDg0JJk7TY1fmsJrh7tNAAFnbV%2FqAAABEgAQAEUyH0VS3HJBgHDlZdWZl02%3D&exvsurl=1&viewmodel=ItemAttachment" + } + }, + { + "id": "AAMkAGJiZmE2NGU4LTQ4YjktNDI1Mi1iMWQzLTQ1MmMxODJkZmQyNABGAAAAAABFdiK7oifWRb4ADuqgSRcnBwBBFDg0JJk7TY1fmsJrh7tNAAAAAAEJAABBFDg0JJk7TY1fmsJrh7tNAAFnbV-qAAABEgAQAEUyH0VS3HJBgHDlZdWZl03=", + "@odata.type": "#microsoft.graph.itemAttachment", + "item@odata.navigationLink": "https://graph.microsoft.com/v1.0/users('7ceb8e03-bdc5-4509-a136-457526165ec0')/messages('')", + "item@odata.associationLink": "https://graph.microsoft.com/v1.0/users('7ceb8e03-bdc5-4509-a136-457526165ec0')/messages('')/$ref", + "isInline": false, + "lastModifiedDateTime": "2024-02-05T09:33:46Z", + "name": "Purpose of life part 3", + "size": 11840, + "item": { + "id": "", + "@odata.type": "#microsoft.graph.message", + "createdDateTime": "2024-02-05T09:33:24Z", + "lastModifiedDateTime": "2024-02-05T09:33:46Z", + "attachments": [ + { + "id": "AAMkAGJiZmE2NGU4LTQ4YjktNDI1Mi1iMWQzLTQ1MmMxODJkZmQyNABGAAAAAABFdiK7oifWRb4ADuqgSRcnBwBBFDg0JJk7TY1fmsJrh7tNAAAAAAEJAABBFDg0JJk7TY1fmsJrh7tNAAFnbV-qAAACEgAQAEUyH0VS3HJBgHDlZdWZl0kSABAAjBhd4-oQaUS969pTkS-gzA==", + "@odata.type": "#microsoft.graph.fileAttachment", + "@odata.mediaContentType": "text/calendar", + "contentType": "text/calendar", + "isInline": false, + "lastModifiedDateTime": "2024-02-05T09:33:46Z", + "name": "Abidjan.ics", + "size": 573, + "contentBytes": "QkVHSU46VkNBTEVOREFSDQpQUk9ESUQ6LS8vdHp1cmwub3JnLy9OT05TR01MIE9sc29uIDIwMjNkLy9FTg0KVkVSU0lPTjoyLjANCkJFR0lOOlZUSU1FWk9ORQ0KVFpJRDpBZnJpY2EvQWJpZGphbg0KTEFTVC1NT0RJRklFRDoyMDIzMTIyMlQyMzMzNThaDQpUWlVSTDpodHRwczovL3d3dy50enVybC5vcmcvem9uZWluZm8vQWZyaWNhL0FiaWRqYW4NClgtTElDLUxPQ0FUSU9OOkFmcmljYS9BYmlkamFuDQpYLVBST0xFUFRJQy1UWk5BTUU6TE1UDQpCRUdJTjpTVEFOREFSRA0KVFpOQU1FOkdNVA0KVFpPRkZTRVRGUk9NOi0wMDE2MDgNClRaT0ZGU0VUVE86KzAwMDANCkRUU1RBUlQ6MTkxMjAxMDFUMDAwMDAwDQpFTkQ6U1RBTkRBUkQNCkVORDpWVElNRVpPTkUNCkVORDpWQ0FMRU5EQVINCg==" + } + ], + "body": { + "content": "\r\n
I just realized the purpose of my life is to be a test case. Good to know.
", + "contentType": "html" + }, + "bodyPreview": "I just realized the purpose of my life is to be a test case. Good to know.", + "conversationId": "AAQkAGJiZmE2NGU4LTQ4YjktNDI1Mi1iMWQzLTQ1MmMxODJkZmQyNAAQAFEnxDqYmbJEm8d2l3qfS6A=", + "conversationIndex": "AQHaWBYiUSfEOpiZskSbx3aXep9LoA==", + "flag": { + "flagStatus": "notFlagged" + }, + "from": { + "emailAddress": { + "address": "JohannaL@10rqc2.onmicrosoft.com", + "name": "Johanna Lorenz" + } + }, + "hasAttachments": true, + "importance": "normal", + "internetMessageId": "", + "isDeliveryReceiptRequested": false, + "isDraft": false, + "isRead": true, + "isReadReceiptRequested": false, + "receivedDateTime": "2024-02-05T09:33:12Z", + "sender": { + "emailAddress": { + "address": "JohannaL@10rqc2.onmicrosoft.com", + "name": "Johanna Lorenz" + } + }, + "sentDateTime": "2024-02-05T09:33:11Z", + "subject": "Purpose of life", + "toRecipients": [ + { + "emailAddress": { + "address": "PradeepG@10rqc2.onmicrosoft.com", + "name": "Pradeep Gupta" + } + } + ], + "webLink": "https://outlook.office365.com/owa/?AttachmentItemID=AAMkAGJiZmE2NGU4LTQ4YjktNDI1Mi1iMWQzLTQ1MmMxODJkZmQyNABGAAAAAABFdiK7oifWRb4ADuqgSRcnBwBBFDg0JJk7TY1fmsJrh7tNAAAAAAEJAABBFDg0JJk7TY1fmsJrh7tNAAFnbV%2FqAAABEgAQAEUyH0VS3HJBgHDlZdWZl03%3D&exvsurl=1&viewmodel=ItemAttachment" + } } ], "bccRecipients": [],