diff --git a/src/cli/backup/groups_e2e_test.go b/src/cli/backup/groups_e2e_test.go index c6c4f456b..d23ed6d6b 100644 --- a/src/cli/backup/groups_e2e_test.go +++ b/src/cli/backup/groups_e2e_test.go @@ -464,10 +464,6 @@ func runGroupsDetailsCmdTest(suite *PreparedBackupGroupsE2ESuite, category path. t := suite.T() - if category == path.ConversationPostsCategory { - t.Skip("skipping conversation details test, see issue #4780") - } - ctx, flush := tester.NewContext(t) ctx = config.SetViper(ctx, suite.dpnd.vpr) diff --git a/src/cli/utils/groups_test.go b/src/cli/utils/groups_test.go index 333210287..bfdf18a6a 100644 --- a/src/cli/utils/groups_test.go +++ b/src/cli/utils/groups_test.go @@ -40,34 +40,30 @@ func (suite *GroupsUtilsSuite) TestIncludeGroupsRestoreDataSelectors() { }{ // resource { - name: "no inputs", - opts: utils.GroupsOpts{}, - // TODO: bump to 3 when we release conversations - expectIncludeLen: 2, + name: "no inputs", + opts: utils.GroupsOpts{}, + expectIncludeLen: 3, }, { name: "empty", opts: utils.GroupsOpts{ Groups: empty, }, - // TODO: bump to 3 when we release conversations - expectIncludeLen: 2, + expectIncludeLen: 3, }, { name: "single inputs", opts: utils.GroupsOpts{ Groups: single, }, - // TODO: bump to 3 when we release conversations - expectIncludeLen: 2, + expectIncludeLen: 3, }, { name: "multi inputs", opts: utils.GroupsOpts{ Groups: multi, }, - // TODO: bump to 3 when we release conversations - expectIncludeLen: 2, + expectIncludeLen: 3, }, // sharepoint { @@ -421,7 +417,7 @@ func (suite *GroupsUtilsSuite) TestAddGroupsCategories() { { name: "none", cats: []string{}, - expectScopeLen: 2, + expectScopeLen: 3, }, { name: "libraries", @@ -443,10 +439,9 @@ func (suite *GroupsUtilsSuite) TestAddGroupsCategories() { cats: []string{ flags.DataLibraries, flags.DataMessages, - // flags.DataConversations, + flags.DataConversations, }, - // TODO: bump to 3 when we include conversations in all data - expectScopeLen: 2, + expectScopeLen: 3, }, { name: "bad inputs", diff --git a/src/internal/m365/collection/groups/backup.go b/src/internal/m365/collection/groups/backup.go index 5fc7dd94d..f5f283ac3 100644 --- a/src/internal/m365/collection/groups/backup.go +++ b/src/internal/m365/collection/groups/backup.go @@ -69,7 +69,7 @@ func CreateCollections[C graph.GetIDer, I groupsItemer]( counter.Add(count.Channels, int64(len(containers))) - collections, err := populateCollections( + collections, err := populateCollections[C, I]( ctx, qp, bh, @@ -213,6 +213,7 @@ func populateCollections[C graph.GetIDer, I groupsItemer]( qp.ProtectedResource.ID(), added, removed, + c, statusUpdater) collections[c.storageDirFolders.String()] = &edc diff --git a/src/internal/m365/collection/groups/backup_test.go b/src/internal/m365/collection/groups/backup_test.go index 1e2502aee..3b61d4e12 100644 --- a/src/internal/m365/collection/groups/backup_test.go +++ b/src/internal/m365/collection/groups/backup_test.go @@ -39,6 +39,7 @@ import ( var _ backupHandler[models.Channelable, models.ChatMessageable] = &mockBackupHandler{} +//lint:ignore U1000 false linter issue due to generics type mockBackupHandler struct { channels []models.Channelable messageIDs []string @@ -50,6 +51,14 @@ type mockBackupHandler struct { doNotInclude bool } +//lint:ignore U1000 false linter issue due to generics +func (bh mockBackupHandler) augmentItemInfo( + *details.GroupsInfo, + models.Channelable, +) { + // no-op +} + func (bh mockBackupHandler) canMakeDeltaQueries() bool { return true } @@ -116,7 +125,8 @@ func (bh mockBackupHandler) canonicalPath( false) } -func (bh mockBackupHandler) GetItem( +//lint:ignore U1000 false linter issue due to generics +func (bh mockBackupHandler) getItem( _ context.Context, _ string, _ path.Elements, diff --git a/src/internal/m365/collection/groups/channel_handler.go b/src/internal/m365/collection/groups/channel_handler.go index 6a5eb7574..59e97d2ee 100644 --- a/src/internal/m365/collection/groups/channel_handler.go +++ b/src/internal/m365/collection/groups/channel_handler.go @@ -95,7 +95,8 @@ func (bh channelsBackupHandler) PathPrefix(tenantID string) (path.Path, error) { false) } -func (bh channelsBackupHandler) GetItem( +//lint:ignore U1000 false linter issue due to generics +func (bh channelsBackupHandler) getItem( ctx context.Context, groupID string, containerIDs path.Elements, @@ -104,6 +105,14 @@ func (bh channelsBackupHandler) GetItem( return bh.ac.GetChannelMessage(ctx, groupID, containerIDs[0], messageID) } +//lint:ignore U1000 false linter issue due to generics +func (bh channelsBackupHandler) augmentItemInfo( + dgi *details.GroupsInfo, + c models.Channelable, +) { + // no-op +} + func channelContainer(ch models.Channelable) container[models.Channelable] { return container[models.Channelable]{ storageDirFolders: path.Elements{ptr.Val(ch.GetId())}, diff --git a/src/internal/m365/collection/groups/collection.go b/src/internal/m365/collection/groups/collection.go index 41f125942..a3a72479d 100644 --- a/src/internal/m365/collection/groups/collection.go +++ b/src/internal/m365/collection/groups/collection.go @@ -17,26 +17,29 @@ import ( "github.com/alcionai/corso/src/pkg/count" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" + "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) -var _ data.BackupCollection = &Collection[groupsItemer]{} +var _ data.BackupCollection = &Collection[graph.GetIDer, groupsItemer]{} const ( collectionChannelBufferSize = 1000 numberOfRetries = 4 ) -type Collection[I groupsItemer] struct { +type Collection[C graph.GetIDer, I groupsItemer] struct { data.BaseCollection protectedResource string stream chan data.Item + contains container[C] + // added is a list of existing item IDs that were added to a container added map[string]struct{} // removed is a list of item IDs that were deleted from, or moved out, of a container removed map[string]struct{} - getter getItemer[I] + getAndAugment getItemAndAugmentInfoer[C, I] statusUpdater support.StatusUpdater } @@ -47,18 +50,20 @@ type Collection[I groupsItemer] struct { // to be deleted. If the prev path is nil, it is assumed newly created. // If both are populated, then state is either moved (if they differ), // or notMoved (if they match). -func NewCollection[I groupsItemer]( +func NewCollection[C graph.GetIDer, I groupsItemer]( baseCol data.BaseCollection, - getter getItemer[I], + getAndAugment getItemAndAugmentInfoer[C, I], protectedResource string, added map[string]struct{}, removed map[string]struct{}, + contains container[C], statusUpdater support.StatusUpdater, -) Collection[I] { - collection := Collection[I]{ +) Collection[C, I] { + collection := Collection[C, I]{ BaseCollection: baseCol, added: added, - getter: getter, + contains: contains, + getAndAugment: getAndAugment, removed: removed, statusUpdater: statusUpdater, stream: make(chan data.Item, collectionChannelBufferSize), @@ -70,7 +75,7 @@ func NewCollection[I groupsItemer]( // Items utility function to asynchronously execute process to fill data channel with // M365 exchange objects and returns the data channel -func (col *Collection[I]) Items(ctx context.Context, errs *fault.Bus) <-chan data.Item { +func (col *Collection[C, I]) Items(ctx context.Context, errs *fault.Bus) <-chan data.Item { go col.streamItems(ctx, errs) return col.stream } @@ -79,7 +84,7 @@ func (col *Collection[I]) Items(ctx context.Context, errs *fault.Bus) <-chan dat // items() production // --------------------------------------------------------------------------- -func (col *Collection[I]) streamItems(ctx context.Context, errs *fault.Bus) { +func (col *Collection[C, I]) streamItems(ctx context.Context, errs *fault.Bus) { var ( streamedItems int64 totalBytes int64 @@ -145,7 +150,7 @@ func (col *Collection[I]) streamItems(ctx context.Context, errs *fault.Bus) { writer := kjson.NewJsonSerializationWriter() defer writer.Close() - item, info, err := col.getter.GetItem( + item, info, err := col.getAndAugment.getItem( ctx, col.protectedResource, col.FullPath().Folders(), @@ -157,6 +162,8 @@ func (col *Collection[I]) streamItems(ctx context.Context, errs *fault.Bus) { return } + col.getAndAugment.augmentItemInfo(info, col.contains.container) + if err := writer.WriteObjectValue("", item); err != nil { err = clues.Wrap(err, "writing channel message to serializer").Label(fault.LabelForceNoBackupCreation) el.AddRecoverable(ctx, err) @@ -207,7 +214,7 @@ func (col *Collection[I]) streamItems(ctx context.Context, errs *fault.Bus) { // finishPopulation is a utility function used to close a Collection's data channel // and to send the status update through the channel. -func (col *Collection[I]) finishPopulation( +func (col *Collection[C, I]) finishPopulation( ctx context.Context, streamedItems, totalBytes int64, err error, diff --git a/src/internal/m365/collection/groups/collection_test.go b/src/internal/m365/collection/groups/collection_test.go index c33602f09..c84724916 100644 --- a/src/internal/m365/collection/groups/collection_test.go +++ b/src/internal/m365/collection/groups/collection_test.go @@ -2,6 +2,7 @@ package groups import ( "bytes" + "context" "io" "testing" "time" @@ -12,9 +13,9 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/common/readers" "github.com/alcionai/corso/src/internal/data" - "github.com/alcionai/corso/src/internal/m365/collection/groups/mock" "github.com/alcionai/corso/src/internal/m365/support" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/backup/details" @@ -117,7 +118,7 @@ func (suite *CollectionUnitSuite) TestNewCollection_state() { suite.Run(test.name, func() { t := suite.T() - c := NewCollection[models.ChatMessageable]( + c := NewCollection[models.Channelable, models.ChatMessageable]( data.NewBaseCollection( test.curr, test.prev, @@ -127,7 +128,9 @@ func (suite *CollectionUnitSuite) TestNewCollection_state() { count.New()), nil, "g", - nil, nil, + nil, + nil, + container[models.Channelable]{}, nil) assert.Equal(t, test.expect, c.State(), "collection state") assert.Equal(t, test.curr, c.FullPath(), "full path") @@ -137,6 +140,28 @@ func (suite *CollectionUnitSuite) TestNewCollection_state() { } } +type getAndAugmentChannelMessage struct { + Err error +} + +//lint:ignore U1000 false linter issue due to generics +func (m getAndAugmentChannelMessage) getItem( + _ context.Context, + _ string, + _ path.Elements, + itemID string, +) (models.ChatMessageable, *details.GroupsInfo, error) { + msg := models.NewChatMessage() + msg.SetId(ptr.To(itemID)) + + return msg, &details.GroupsInfo{}, m.Err +} + +//lint:ignore U1000 false linter issue due to generics +func (getAndAugmentChannelMessage) augmentItemInfo(*details.GroupsInfo, models.Channelable) { + // no-op +} + func (suite *CollectionUnitSuite) TestCollection_streamItems() { var ( t = suite.T() @@ -199,7 +224,7 @@ func (suite *CollectionUnitSuite) TestCollection_streamItems() { ctx, flush := tester.NewContext(t) defer flush() - col := &Collection[models.ChatMessageable]{ + col := &Collection[models.Channelable, models.ChatMessageable]{ BaseCollection: data.NewBaseCollection( fullPath, nil, @@ -208,8 +233,9 @@ func (suite *CollectionUnitSuite) TestCollection_streamItems() { false, count.New()), added: test.added, + contains: container[models.Channelable]{}, removed: test.removed, - getter: mock.GetChannelMessage{}, + getAndAugment: getAndAugmentChannelMessage{}, stream: make(chan data.Item), statusUpdater: statusUpdater, } diff --git a/src/internal/m365/collection/groups/conversation_handler.go b/src/internal/m365/collection/groups/conversation_handler.go index dcfe76894..2fb02f09e 100644 --- a/src/internal/m365/collection/groups/conversation_handler.go +++ b/src/internal/m365/collection/groups/conversation_handler.go @@ -113,7 +113,8 @@ func (bh conversationsBackupHandler) PathPrefix(tenantID string) (path.Path, err false) } -func (bh conversationsBackupHandler) GetItem( +//lint:ignore U1000 false linter issue due to generics +func (bh conversationsBackupHandler) getItem( ctx context.Context, groupID string, containerIDs path.Elements, // expects: [conversationID, threadID] @@ -128,6 +129,14 @@ func (bh conversationsBackupHandler) GetItem( api.CallConfig{}) } +//lint:ignore U1000 false linter issue due to generics +func (bh conversationsBackupHandler) augmentItemInfo( + dgi *details.GroupsInfo, + c models.Conversationable, +) { + dgi.Post.Topic = ptr.Val(c.GetTopic()) +} + func conversationThreadContainer( c models.Conversationable, t models.ConversationThreadable, @@ -136,9 +145,8 @@ func conversationThreadContainer( storageDirFolders: path.Elements{ptr.Val(c.GetId()), ptr.Val(t.GetId())}, // microsoft UX doesn't display any sort of container name that would make a reasonable // "location" for the posts in the conversation. We may need to revisit this, perhaps - // the subject is sufficiently acceptable. But at this time it's left empty so that - // we don't populate it with problematic data. - humanLocation: path.Elements{}, + // the subject (aka topic) is sufficiently acceptable. + humanLocation: path.Elements{ptr.Val(c.GetTopic())}, canMakeDeltaQueries: false, container: c, } diff --git a/src/internal/m365/collection/groups/handlers.go b/src/internal/m365/collection/groups/handlers.go index 06361fcc2..dfb224d56 100644 --- a/src/internal/m365/collection/groups/handlers.go +++ b/src/internal/m365/collection/groups/handlers.go @@ -25,13 +25,25 @@ type backupHandler[C graph.GetIDer, I groupsItemer] interface { getItemer[I] getContainerser[C] getContainerItemIDser + getItemAndAugmentInfoer[C, I] includeContainerer[C] canonicalPather canMakeDeltaQuerieser } +type getItemAndAugmentInfoer[C graph.GetIDer, I groupsItemer] interface { + getItemer[I] + augmentItemInfoer[C] +} + +type augmentItemInfoer[C graph.GetIDer] interface { + // augmentItemInfo completes the groupInfo population with any data + // owned by the container and not accessible to the item. + augmentItemInfo(*details.GroupsInfo, C) +} + type getItemer[I groupsItemer] interface { - GetItem( + getItem( ctx context.Context, protectedResource string, containerIDs path.Elements, diff --git a/src/internal/m365/collection/groups/mock/getter.go b/src/internal/m365/collection/groups/mock/getter.go deleted file mode 100644 index b9b13af3a..000000000 --- a/src/internal/m365/collection/groups/mock/getter.go +++ /dev/null @@ -1,27 +0,0 @@ -package mock - -import ( - "context" - - "github.com/microsoftgraph/msgraph-sdk-go/models" - - "github.com/alcionai/corso/src/internal/common/ptr" - "github.com/alcionai/corso/src/pkg/backup/details" - "github.com/alcionai/corso/src/pkg/path" -) - -type GetChannelMessage struct { - Err error -} - -func (m GetChannelMessage) GetItem( - _ context.Context, - _ string, - _ path.Elements, - itemID string, -) (models.ChatMessageable, *details.GroupsInfo, error) { - msg := models.NewChatMessage() - msg.SetId(ptr.To(itemID)) - - return msg, &details.GroupsInfo{}, m.Err -} diff --git a/src/pkg/backup/details/groups.go b/src/pkg/backup/details/groups.go index e7edb46b0..99829f05a 100644 --- a/src/pkg/backup/details/groups.go +++ b/src/pkg/backup/details/groups.go @@ -65,7 +65,7 @@ type ConversationPostInfo struct { Creator string `json:"creator,omitempty"` Preview string `json:"preview,omitempty"` Size int64 `json:"size,omitempty"` - Subject string `json:"subject,omitempty"` + Topic string `json:"topic,omitempty"` } type ChannelMessageInfo struct { @@ -86,6 +86,8 @@ func (i GroupsInfo) Headers() []string { return []string{"ItemName", "Library", "ParentPath", "Size", "Owner", "Created", "Modified"} case GroupsChannelMessage: return []string{"Message", "Channel", "Subject", "Replies", "Creator", "Created", "Last Reply"} + case GroupsConversationPost: + return []string{"Post", "Conversation", "Sender", "Created"} } return []string{} @@ -112,7 +114,7 @@ func (i GroupsInfo) Values() []string { } return []string{ - // html parsing may produce newlijnes, which we'll want to avoid + // html parsing may produce newlines, which we'll want to avoid strings.ReplaceAll(i.Message.Preview, "\n", "\\n"), i.ParentPath, i.Message.Subject, @@ -121,6 +123,14 @@ func (i GroupsInfo) Values() []string { dttm.FormatToTabularDisplay(i.Message.CreatedAt), lastReply, } + case GroupsConversationPost: + return []string{ + // html parsing may produce newlines, which we'll want to avoid + strings.ReplaceAll(i.Post.Preview, "\n", "\\n"), + i.Post.Topic, + i.Post.Creator, + dttm.FormatToTabularDisplay(i.Post.CreatedAt), + } } return []string{} diff --git a/src/pkg/backup/details/groups_test.go b/src/pkg/backup/details/groups_test.go index 3cf756743..d9874ea5b 100644 --- a/src/pkg/backup/details/groups_test.go +++ b/src/pkg/backup/details/groups_test.go @@ -21,38 +21,95 @@ func TestGroupsUnitSuite(t *testing.T) { } func (suite *GroupsUnitSuite) TestGroupsPrintable() { - t := suite.T() now := time.Now() then := now.Add(time.Minute) - gi := details.GroupsInfo{ - ItemType: details.GroupsChannelMessage, - ParentPath: "parentPath", - Message: details.ChannelMessageInfo{ - Preview: "preview", - ReplyCount: 1, - Creator: "creator", - CreatedAt: now, - Subject: "subject", + table := []struct { + name string + info details.GroupsInfo + expectHs []string + expectVs []string + }{ + { + name: "channel message", + info: details.GroupsInfo{ + ItemType: details.GroupsChannelMessage, + ParentPath: "parentpath", + Message: details.ChannelMessageInfo{ + Preview: "preview", + ReplyCount: 1, + Creator: "creator", + CreatedAt: now, + Subject: "subject", + }, + LastReply: details.ChannelMessageInfo{ + CreatedAt: then, + }, + }, + expectHs: []string{"Message", "Channel", "Subject", "Replies", "Creator", "Created", "Last Reply"}, + expectVs: []string{ + "preview", + "parentpath", + "subject", + "1", + "creator", + dttm.FormatToTabularDisplay(now), + dttm.FormatToTabularDisplay(then), + }, }, - LastReply: details.ChannelMessageInfo{ - CreatedAt: then, + { + name: "conversation post", + info: details.GroupsInfo{ + ItemType: details.GroupsConversationPost, + Post: details.ConversationPostInfo{ + Preview: "preview", + Creator: "creator", + CreatedAt: now, + Topic: "topic", + }, + }, + expectHs: []string{"Post", "Conversation", "Sender", "Created"}, + expectVs: []string{ + "preview", + "topic", + "creator", + dttm.FormatToTabularDisplay(now), + }, + }, + { + name: "sharepoint library", + info: details.GroupsInfo{ + ItemType: details.SharePointLibrary, + ParentPath: "parentPath", + Created: now, + Modified: then, + DriveName: "librarydrive", + ItemName: "item", + Size: 42, + Owner: "user", + }, + expectHs: []string{"ItemName", "Library", "ParentPath", "Size", "Owner", "Created", "Modified"}, + expectVs: []string{ + "item", + "librarydrive", + "parentPath", + "42 B", + "user", + dttm.FormatToTabularDisplay(now), + dttm.FormatToTabularDisplay(then), + }, }, } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() - expectVs := []string{ - "preview", - "parentPath", - "subject", - "1", - "creator", - dttm.FormatToTabularDisplay(now), - dttm.FormatToTabularDisplay(then), + hs := test.info.Headers() + vs := test.info.Values() + + assert.Equal(t, len(hs), len(vs)) + assert.Equal(t, test.expectHs, hs) + assert.Equal(t, test.expectVs, vs) + }) } - - hs := gi.Headers() - vs := gi.Values() - - assert.Equal(t, len(hs), len(vs)) - assert.Equal(t, expectVs, vs) } diff --git a/src/pkg/selectors/groups.go b/src/pkg/selectors/groups.go index 33e302e82..cd33d4934 100644 --- a/src/pkg/selectors/groups.go +++ b/src/pkg/selectors/groups.go @@ -217,9 +217,8 @@ func (s *groups) AllData() []GroupsScope { scopes = append( scopes, makeScope[GroupsScope](GroupsLibraryFolder, Any()), - makeScope[GroupsScope](GroupsChannel, Any())) - // TODO: enable conversations in all-data backups - // makeScope[GroupsScope](GroupsConversation, Any())) + makeScope[GroupsScope](GroupsChannel, Any()), + makeScope[GroupsScope](GroupsConversation, Any())) return scopes } diff --git a/src/pkg/selectors/groups_test.go b/src/pkg/selectors/groups_test.go index 616e584a3..6c1cf962a 100644 --- a/src/pkg/selectors/groups_test.go +++ b/src/pkg/selectors/groups_test.go @@ -245,9 +245,8 @@ func (suite *GroupsSelectorSuite) TestGroupsRestore_Reduce() { }, expect: arr( libItem, libItem2, libItem3, - chanItem, chanItem2, chanItem3), - // TODO: re-add when we release conversations - // convItem, convItem2, convItem3), + chanItem, chanItem2, chanItem3, + convItem, convItem2, convItem3), }, { name: "only match library item", diff --git a/src/pkg/services/m365/api/conversations_test.go b/src/pkg/services/m365/api/conversations_test.go index 0599d0908..da63b9b8d 100644 --- a/src/pkg/services/m365/api/conversations_test.go +++ b/src/pkg/services/m365/api/conversations_test.go @@ -96,7 +96,7 @@ func (suite *ConversationsAPIUnitSuite) TestConversationPostInfo() { Preview: "", Size: 0, // TODO: feed the subject in from the conversation - Subject: "", + Topic: "", }, }