larryh/corso
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Handle previous paths for group mailbox (#5154)

Browse Source 
<!-- PR description-->

* Tombstone collections weren't being added because we were not processing prev paths for group mailbox category.
* Hence deleted conversations were being carried forward even if `--disable-incrementals` was used.

---

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

- [ ] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x] ⛔ 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
This commit is contained in:
Abhishek Pandey 2024-02-05 15:10:09 -08:00 committed by GitHub
parent de22131b23
commit 53a0525bfd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 551 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

521
src/internal/m365/collection/groups/backup_test.go
View File

@ -39,22 +39,24 @@ import (
// mocks
// ---------------------------------------------------------------------------
var _ backupHandler[models.Channelable, models.ChatMessageable] = &mockBackupHandler{}
var _ backupHandler[models.Channelable, models.ChatMessageable] = &mockChannelsBH{}
//lint:ignore U1000 false linter issue due to generics
type mockBackupHandler struct {
type mockChannelsBH struct {
channels []models.Channelable
conversations []models.Conversationable
messageIDs []string
deletedMsgIDs []string
messagesErr error
messages map[string]models.ChatMessageable
posts map[string]models.Postable
info map[string]*details.GroupsInfo
getMessageErr map[string]error
doNotInclude bool
}
//lint:ignore U1000 false linter issue due to generics
func (bh mockBackupHandler) augmentItemInfo(
func (bh mockChannelsBH) augmentItemInfo(
*details.GroupsInfo,
models.Channelable,
) {
@ -62,15 +64,15 @@ func (bh mockBackupHandler) augmentItemInfo(
}
//lint:ignore U1000 false linter issue due to generics
func (bh mockBackupHandler) supportsItemMetadata() bool {
func (bh mockChannelsBH) supportsItemMetadata() bool {
return false
}
func (bh mockBackupHandler) canMakeDeltaQueries() bool {
func (bh mockChannelsBH) canMakeDeltaQueries() bool {
return true
}
func (bh mockBackupHandler) containers() []container[models.Channelable] {
func (bh mockChannelsBH) containers() []container[models.Channelable] {
containers := make([]container[models.Channelable], 0, len(bh.channels))
for _, ch := range bh.channels {
@ -81,14 +83,14 @@ func (bh mockBackupHandler) containers() []container[models.Channelable] {
}
//lint:ignore U1000 required for interface compliance
func (bh mockBackupHandler) getContainers(
func (bh mockChannelsBH) getContainers(
context.Context,
api.CallConfig,
) ([]container[models.Channelable], error) {
return bh.containers(), nil
}
func (bh mockBackupHandler) getContainerItemIDs(
func (bh mockChannelsBH) getContainerItemIDs(
_ context.Context,
_ path.Elements,
_ string,
@ -111,14 +113,14 @@ func (bh mockBackupHandler) getContainerItemIDs(
}
//lint:ignore U1000 required for interface compliance
func (bh mockBackupHandler) includeContainer(
func (bh mockChannelsBH) includeContainer(
models.Channelable,
selectors.GroupsScope,
) bool {
return !bh.doNotInclude
}
func (bh mockBackupHandler) canonicalPath(
func (bh mockChannelsBH) canonicalPath(
storageDirFolders path.Elements,
tenantID string,
) (path.Path, error) {
@ -133,7 +135,7 @@ func (bh mockBackupHandler) canonicalPath(
}
//lint:ignore U1000 false linter issue due to generics
func (bh mockBackupHandler) getItem(
func (bh mockChannelsBH) getItem(
_ context.Context,
_ string,
_ path.Elements,
@ -142,7 +144,7 @@ func (bh mockBackupHandler) getItem(
return bh.messages[itemID], bh.info[itemID], bh.getMessageErr[itemID]
}
func (bh mockBackupHandler) getItemMetadata(
func (bh mockChannelsBH) getItemMetadata(
_ context.Context,
_ models.Channelable,
) (io.ReadCloser, int, error) {
@ -150,7 +152,7 @@ func (bh mockBackupHandler) getItemMetadata(
}
//lint:ignore U1000 false linter issue due to generics
func (bh mockBackupHandler) makeTombstones(
func (bh mockChannelsBH) makeTombstones(
dps metadata.DeltaPaths,
) (map[string]string, error) {
return makeTombstones(dps), nil
@ -176,6 +178,10 @@ func (suite *BackupUnitSuite) SetupSuite() {
suite.creds = m365
}
// ---------------------------------------------------------------------------
// Channels tests
// ---------------------------------------------------------------------------
func (suite *BackupUnitSuite) TestPopulateCollections() {
var (
qp = graph.QueryParams{
@ -188,7 +194,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections() {
table := []struct {
name string
mock mockBackupHandler
mock mockChannelsBH
expectErr require.ErrorAssertionFunc
expectColls int
expectNewColls int
@ -196,7 +202,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections() {
}{
{
name: "happy path, one container",
mock: mockBackupHandler{
mock: mockChannelsBH{
channels: testdata.StubChannels("one"),
messageIDs: []string{"msg-one"},
},
@ -207,7 +213,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections() {
},
{
name: "happy path, one container, only deleted messages",
mock: mockBackupHandler{
mock: mockChannelsBH{
channels: testdata.StubChannels("one"),
deletedMsgIDs: []string{"msg-one"},
},
@ -218,7 +224,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections() {
},
{
name: "happy path, many containers",
mock: mockBackupHandler{
mock: mockChannelsBH{
channels: testdata.StubChannels("one", "two"),
messageIDs: []string{"msg-one"},
},
@ -229,7 +235,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections() {
},
{
name: "no containers pass scope",
mock: mockBackupHandler{
mock: mockChannelsBH{
channels: testdata.StubChannels("one"),
doNotInclude: true,
},
@ -240,7 +246,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections() {
},
{
name: "no channels",
mock: mockBackupHandler{},
mock: mockChannelsBH{},
expectErr: require.NoError,
expectColls: 1,
expectNewColls: 0,
@ -248,7 +254,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections() {
},
{
name: "no channel messages",
mock: mockBackupHandler{
mock: mockChannelsBH{
channels: testdata.StubChannels("one"),
},
expectErr: require.NoError,
@ -258,7 +264,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections() {
},
{
name: "err: deleted in flight",
mock: mockBackupHandler{
mock: mockChannelsBH{
channels: testdata.StubChannels("one"),
messagesErr: core.ErrNotFound,
},
@ -269,7 +275,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections() {
},
{
name: "err: other error",
mock: mockBackupHandler{
mock: mockChannelsBH{
channels: testdata.StubChannels("one"),
messagesErr: assert.AnError,
},
@ -348,7 +354,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections_incremental() {
table := []struct {
name string
mock mockBackupHandler
mock mockChannelsBH
deltaPaths metadata.DeltaPaths
expectErr require.ErrorAssertionFunc
expectColls int
@ -358,7 +364,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections_incremental() {
}{
{
name: "non incremental",
mock: mockBackupHandler{
mock: mockChannelsBH{
channels: testdata.StubChannels("chan"),
messageIDs: []string{"msg"},
},
@ -371,7 +377,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections_incremental() {
},
{
name: "incremental",
mock: mockBackupHandler{
mock: mockChannelsBH{
channels: testdata.StubChannels("chan"),
deletedMsgIDs: []string{"msg"},
},
@ -389,7 +395,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections_incremental() {
},
{
name: "incremental no new messages",
mock: mockBackupHandler{
mock: mockChannelsBH{
channels: testdata.StubChannels("chan"),
},
deltaPaths: metadata.DeltaPaths{
@ -406,7 +412,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections_incremental() {
},
{
name: "incremental deleted channel",
mock: mockBackupHandler{
mock: mockChannelsBH{
channels: testdata.StubChannels(),
},
deltaPaths: metadata.DeltaPaths{
@ -423,7 +429,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections_incremental() {
},
{
name: "incremental new and deleted channel",
mock: mockBackupHandler{
mock: mockChannelsBH{
channels: testdata.StubChannels("chan2"),
messageIDs: []string{"msg"},
},
@ -493,6 +499,465 @@ func (suite *BackupUnitSuite) TestPopulateCollections_incremental() {
}
}
// ---------------------------------------------------------------------------
// Conversations tests
// ---------------------------------------------------------------------------
var _ backupHandler[models.Conversationable, models.Postable] = &mockConversationsBH{}
//lint:ignore U1000 false linter issue due to generics
type mockConversationsBH struct {
conversations []models.Conversationable
// Assume all conversations have the same thread object under them for simplicty.
// It doesn't impact the tests.
thread models.ConversationThreadable
postIDs []string
deletedPostIDs []string
PostsErr error
Posts map[string]models.Postable
info map[string]*details.GroupsInfo
getPostErr map[string]error
doNotInclude bool
}
//lint:ignore U1000 false linter issue due to generics
func (bh mockConversationsBH) augmentItemInfo(
*details.GroupsInfo,
models.Conversationable,
) {
// no-op
}
func (bh mockConversationsBH) canMakeDeltaQueries() bool {
return false
}
func (bh mockConversationsBH) containers() []container[models.Conversationable] {
containers := make([]container[models.Conversationable], 0, len(bh.conversations))
for _, ch := range bh.conversations {
containers = append(containers, conversationThreadContainer(ch, bh.thread))
}
return containers
}
//lint:ignore U1000 required for interface compliance
func (bh mockConversationsBH) getContainers(
context.Context,
api.CallConfig,
) ([]container[models.Conversationable], error) {
return bh.containers(), nil
}
func (bh mockConversationsBH) getContainerItemIDs(
_ context.Context,
_ path.Elements,
_ string,
_ api.CallConfig,
) (pagers.AddedAndRemoved, error) {
idRes := make(map[string]time.Time, len(bh.postIDs))
for _, id := range bh.postIDs {
idRes[id] = time.Time{}
}
aar := pagers.AddedAndRemoved{
Added: idRes,
Removed: bh.deletedPostIDs,
ValidModTimes: true,
DU: pagers.DeltaUpdate{},
}
return aar, bh.PostsErr
}
//lint:ignore U1000 required for interface compliance
func (bh mockConversationsBH) includeContainer(
models.Conversationable,
selectors.GroupsScope,
) bool {
return !bh.doNotInclude
}
func (bh mockConversationsBH) canonicalPath(
storageDirFolders path.Elements,
tenantID string,
) (path.Path, error) {
return storageDirFolders.
Builder().
ToDataLayerPath(
tenantID,
"protectedResource",
path.GroupsService,
path.ConversationPostsCategory,
false)
}
//lint:ignore U1000 false linter issue due to generics
func (bh mockConversationsBH) getItem(
_ context.Context,
_ string,
_ path.Elements,
itemID string,
) (models.Postable, *details.GroupsInfo, error) {
return bh.Posts[itemID], bh.info[itemID], bh.getPostErr[itemID]
}
//lint:ignore U1000 false linter issue due to generics
func (bh mockConversationsBH) supportsItemMetadata() bool {
return true
}
func (bh mockConversationsBH) getItemMetadata(
_ context.Context,
_ models.Conversationable,
) (io.ReadCloser, int, error) {
return nil, 0, nil
}
//lint:ignore U1000 false linter issue due to generics
func (bh mockConversationsBH) makeTombstones(
dps metadata.DeltaPaths,
) (map[string]string, error) {
r := make(map[string]string, len(dps))
for id, v := range dps {
elems := path.Split(id)
if len(elems) != 2 {
return nil, clues.New("invalid prev path")
}
r[elems[0]] = v.Path
}
return r, nil
}
func (suite *BackupUnitSuite) TestPopulateCollections_Conversations() {
var (
qp = graph.QueryParams{
Category: path.ConversationPostsCategory, // doesn't matter which one we use.
ProtectedResource: inMock.NewProvider("group_id", "user_name"),
TenantID: suite.creds.AzureTenantID,
}
statusUpdater = func(*support.ControllerOperationStatus) {}
)
table := []struct {
name string
mock mockConversationsBH
expectErr require.ErrorAssertionFunc
expectColls int
expectNewColls int
expectMetadataColls int
}{
{
name: "happy path, one container",
mock: mockConversationsBH{
conversations: testdata.StubConversations("one"),
thread: testdata.StubConversationThreads("t-one")[0],
postIDs: []string{"msg-one"},
},
expectErr: require.NoError,
expectColls: 2,
expectNewColls: 1,
expectMetadataColls: 1,
},
{
name: "happy path, one container, only deleted messages",
mock: mockConversationsBH{
conversations: testdata.StubConversations("one"),
thread: testdata.StubConversationThreads("t-one")[0],
deletedPostIDs: []string{"msg-one"},
},
expectErr: require.NoError,
expectColls: 2,
expectNewColls: 1,
expectMetadataColls: 1,
},
{
name: "happy path, many containers",
mock: mockConversationsBH{
conversations: testdata.StubConversations("one", "two"),
thread: testdata.StubConversationThreads("t-one")[0],
postIDs: []string{"msg-one"},
},
expectErr: require.NoError,
expectColls: 3,
expectNewColls: 2,
expectMetadataColls: 1,
},
{
name: "no containers pass scope",
mock: mockConversationsBH{
conversations: testdata.StubConversations("one"),
thread: testdata.StubConversationThreads("t-one")[0],
doNotInclude: true,
},
expectErr: require.NoError,
expectColls: 1,
expectNewColls: 0,
expectMetadataColls: 1,
},
{
name: "no conversations",
mock: mockConversationsBH{},
expectErr: require.NoError,
expectColls: 1,
expectNewColls: 0,
expectMetadataColls: 1,
},
{
name: "no conv posts",
mock: mockConversationsBH{
conversations: testdata.StubConversations("one"),
thread: testdata.StubConversationThreads("t-one")[0],
},
expectErr: require.NoError,
expectColls: 2,
expectNewColls: 1,
expectMetadataColls: 1,
},
{
name: "err: deleted in flight",
mock: mockConversationsBH{
conversations: testdata.StubConversations("one"),
thread: testdata.StubConversationThreads("t-one")[0],
PostsErr: core.ErrNotFound,
},
expectErr: require.Error,
expectColls: 1,
expectNewColls: 0,
expectMetadataColls: 1,
},
{
name: "err: other error",
mock: mockConversationsBH{
conversations: testdata.StubConversations("one"),
thread: testdata.StubConversationThreads("t-one")[0],
PostsErr: assert.AnError,
},
expectErr: require.Error,
expectColls: 1,
expectNewColls: 0,
expectMetadataColls: 1,
},
}
for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()
ctx, flush := tester.NewContext(t)
defer flush()
ctrlOpts := control.Options{FailureHandling: control.FailFast}
collections, err := populateCollections(
ctx,
qp,
test.mock,
statusUpdater,
test.mock.containers(),
selectors.NewGroupsBackup(nil).Channels(selectors.Any())[0],
nil,
false,
ctrlOpts,
count.New(),
fault.New(true))
test.expectErr(t, err, clues.ToCore(err))
assert.Len(t, collections, test.expectColls, "number of collections")
// collection assertions
deleteds, news, metadatas, doNotMerges := 0, 0, 0, 0
for _, c := range collections {
if c.FullPath().Service() == path.GroupsMetadataService {
metadatas++
continue
}
if c.State() == data.DeletedState {
deleteds++
}
if c.State() == data.NewState {
news++
}
if c.DoNotMergeItems() {
doNotMerges++
}
}
assert.Zero(t, deleteds, "deleted collections")
assert.Equal(t, test.expectNewColls, news, "new collections")
assert.Equal(t, test.expectMetadataColls, metadatas, "metadata collections")
})
}
}
func (suite *BackupUnitSuite) TestPopulateCollections_ConversationsIncremental() {
var (
qp = graph.QueryParams{
Category: path.ConversationPostsCategory, // doesn't matter which one we use.
ProtectedResource: inMock.NewProvider("group_id", "user_name"),
TenantID: suite.creds.AzureTenantID,
}
statusUpdater = func(*support.ControllerOperationStatus) {}
allScope = selectors.NewGroupsBackup(nil).Conversation(selectors.Any())[0]
)
convPath, err := path.Build("t", "g", path.GroupsService, path.ConversationPostsCategory, false, "conv0", "thread0")
require.NoError(suite.T(), err, clues.ToCore(err))
table := []struct {
name string
mock mockConversationsBH
deltaPaths metadata.DeltaPaths
expectErr require.ErrorAssertionFunc
expectColls int
expectNewColls int
expectTombstoneCols int
expectMetadataColls int
}{
{
name: "non incremental",
mock: mockConversationsBH{
conversations: testdata.StubConversations("conv0"),
thread: testdata.StubConversationThreads("t0")[0],
postIDs: []string{"msg"},
},
deltaPaths: metadata.DeltaPaths{},
expectErr: require.NoError,
expectColls: 2,
expectNewColls: 1,
expectTombstoneCols: 0,
expectMetadataColls: 1,
},
{
name: "incremental",
mock: mockConversationsBH{
conversations: testdata.StubConversations("conv0"),
thread: testdata.StubConversationThreads("t0")[0],
postIDs: []string{"msg"},
},
deltaPaths: metadata.DeltaPaths{
"conv0/thread0": {
Path: convPath.String(),
},
},
expectErr: require.NoError,
expectColls: 2,
expectNewColls: 1, // No delta support
expectTombstoneCols: 0,
expectMetadataColls: 1,
},
{
name: "incremental no new posts",
mock: mockConversationsBH{
conversations: testdata.StubConversations("conv0"),
thread: testdata.StubConversationThreads("t0")[0],
},
deltaPaths: metadata.DeltaPaths{
"conv0/thread0": {
Path: convPath.String(),
},
},
expectErr: require.NoError,
expectColls: 2,
expectNewColls: 1, // No delta support
expectTombstoneCols: 0,
expectMetadataColls: 1,
},
{
name: "incremental deleted conversation",
mock: mockConversationsBH{
conversations: testdata.StubConversations(),
},
deltaPaths: metadata.DeltaPaths{
"conv0/thread0": {
Path: convPath.String(),
},
},
expectErr: require.NoError,
expectColls: 2,
expectNewColls: 0,
expectTombstoneCols: 1,
expectMetadataColls: 1,
},
{
name: "incremental new and deleted conversations",
mock: mockConversationsBH{
conversations: testdata.StubConversations("conv1"),
thread: testdata.StubConversationThreads("t1")[0],
postIDs: []string{"msg"},
},
deltaPaths: metadata.DeltaPaths{
"conv0/thread0": {
Path: convPath.String(),
},
},
expectErr: require.NoError,
expectColls: 3,
expectNewColls: 1,
expectTombstoneCols: 1,
expectMetadataColls: 1,
},
}
for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()
ctx, flush := tester.NewContext(t)
defer flush()
ctrlOpts := control.Options{FailureHandling: control.FailFast}
collections, err := populateCollections(
ctx,
qp,
test.mock,
statusUpdater,
test.mock.containers(),
allScope,
test.deltaPaths,
false,
ctrlOpts,
count.New(),
fault.New(true))
test.expectErr(t, err, clues.ToCore(err))
assert.Len(t, collections, test.expectColls, "number of collections")
// collection assertions
tombstones, news, metadatas, doNotMerges := 0, 0, 0, 0
for _, c := range collections {
if c.FullPath() != nil && c.FullPath().Service() == path.GroupsMetadataService {
metadatas++
continue
}
if c.State() == data.DeletedState {
tombstones++
}
if c.State() == data.NewState {
news++
}
if c.DoNotMergeItems() {
doNotMerges++
}
}
assert.Equal(t, test.expectNewColls, news, "new collections")
assert.Equal(t, test.expectTombstoneCols, tombstones, "tombstone collections")
assert.Equal(t, test.expectMetadataColls, metadatas, "metadata collections")
})
}
}
// ---------------------------------------------------------------------------
// Integration tests
// ---------------------------------------------------------------------------

3
src/internal/m365/collection/groups/metadata.go
View File

@ -22,12 +22,14 @@ func parseMetadataCollections(
// cdp stores metadata
cdp := metadata.CatDeltaPaths{
path.ChannelMessagesCategory: {},
path.ConversationPostsCategory: {},
}
// found tracks the metadata we've loaded, to make sure we don't
// fetch overlapping copies.
found := map[path.CategoryType]map[string]struct{}{
path.ChannelMessagesCategory: {},
path.ConversationPostsCategory: {},
}
// errors from metadata items should not stop the backup,
@ -105,6 +107,7 @@ func parseMetadataCollections(
return metadata.CatDeltaPaths{
path.ChannelMessagesCategory: {},
path.ConversationPostsCategory: {},
}, false, nil
}

52
src/internal/m365/collection/groups/testdata/conversations.go vendored Normal file
View File

@ -0,0 +1,52 @@
package testdata
import (
"github.com/google/uuid"
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/alcionai/corso/src/internal/common/ptr"
)
func StubConversations(ids ...string) []models.Conversationable {
sl := make([]models.Conversationable, 0, len(ids))
for _, id := range ids {
c := models.NewConversation()
c.SetId(ptr.To(id))
sl = append(sl, c)
}
return sl
}
func StubConversationThreads(ids ...string) []models.ConversationThreadable {
sl := make([]models.ConversationThreadable, 0, len(ids))
for _, id := range ids {
ct := models.NewConversationThread()
ct.SetId(ptr.To(id))
sl = append(sl, ct)
}
return sl
}
func StubPosts(ids ...string) []models.Postable {
sl := make([]models.Postable, 0, len(ids))
for _, id := range ids {
p := models.NewPost()
p.SetId(ptr.To(uuid.NewString()))
body := models.NewItemBody()
body.SetContent(ptr.To(id))
p.SetBody(body)
sl = append(sl, p)
}
return sl
}