chat handler cleanup (#5139)

container properties weren't used at all.
getItem didn't match the func behavior, since we
already had the item.
This commit is contained in:
Keepers 2024-02-28 09:27:42 -07:00 committed by GitHub
parent e1ed5275dc
commit 29c7235d92
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 44 additions and 68 deletions

View File

@ -14,7 +14,6 @@ import (
"github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/path"
"github.com/alcionai/corso/src/pkg/selectors" "github.com/alcionai/corso/src/pkg/selectors"
"github.com/alcionai/corso/src/pkg/services/m365/api"
"github.com/alcionai/corso/src/pkg/services/m365/api/graph" "github.com/alcionai/corso/src/pkg/services/m365/api/graph"
) )
@ -38,14 +37,7 @@ func CreateCollections[I chatsItemer](
} }
) )
cc := api.CallConfig{ container := bh.getContainer()
CanMakeDeltaQueries: false,
}
container, err := bh.getContainer(ctx, cc)
if err != nil {
return nil, false, clues.Stack(err)
}
counter.Add(count.Containers, 1) counter.Add(count.Containers, 1)
@ -149,7 +141,7 @@ func populateCollection[I chatsItemer](
data.NewBaseCollection( data.NewBaseCollection(
p, p,
p, p,
container.humanLocation.Builder(), &path.Builder{},
ctrlOpts, ctrlOpts,
false, false,
cl), cl),

View File

@ -51,15 +51,12 @@ type mockBackupHandler struct {
} }
func (bh mockBackupHandler) container() container[models.Chatable] { func (bh mockBackupHandler) container() container[models.Chatable] {
return chatContainer() return container[models.Chatable]{}
} }
//lint:ignore U1000 required for interface compliance //lint:ignore U1000 required for interface compliance
func (bh mockBackupHandler) getContainer( func (bh mockBackupHandler) getContainer() container[models.Chatable] {
context.Context, return container[models.Chatable]{}
api.CallConfig,
) (container[models.Chatable], error) {
return chatContainer(), nil
} }
func (bh mockBackupHandler) getItemIDs( func (bh mockBackupHandler) getItemIDs(
@ -85,7 +82,7 @@ func (bh mockBackupHandler) CanonicalPath() (path.Path, error) {
} }
//lint:ignore U1000 false linter issue due to generics //lint:ignore U1000 false linter issue due to generics
func (bh mockBackupHandler) getItem( func (bh mockBackupHandler) fillItem(
_ context.Context, _ context.Context,
chat models.Chatable, chat models.Chatable,
) (models.Chatable, *details.TeamsChatsInfo, error) { ) (models.Chatable, *details.TeamsChatsInfo, error) {

View File

@ -35,11 +35,8 @@ func NewUsersChatsBackupHandler(
// chats have no containers. Everything is stored at the root. // chats have no containers. Everything is stored at the root.
// //
//lint:ignore U1000 required for interface compliance //lint:ignore U1000 required for interface compliance
func (bh usersChatsBackupHandler) getContainer( func (bh usersChatsBackupHandler) getContainer() container[models.Chatable] {
ctx context.Context, return container[models.Chatable]{}
_ api.CallConfig,
) (container[models.Chatable], error) {
return chatContainer(), nil
} }
//lint:ignore U1000 required for interface compliance //lint:ignore U1000 required for interface compliance
@ -80,7 +77,7 @@ func (bh usersChatsBackupHandler) CanonicalPath() (path.Path, error) {
} }
//lint:ignore U1000 false linter issue due to generics //lint:ignore U1000 false linter issue due to generics
func (bh usersChatsBackupHandler) getItem( func (bh usersChatsBackupHandler) fillItem(
ctx context.Context, ctx context.Context,
chat models.Chatable, chat models.Chatable,
) (models.Chatable, *details.TeamsChatsInfo, error) { ) (models.Chatable, *details.TeamsChatsInfo, error) {
@ -106,10 +103,3 @@ func (bh usersChatsBackupHandler) getItem(
return chat, api.TeamsChatInfo(chat), nil return chat, api.TeamsChatInfo(chat), nil
} }
func chatContainer() container[models.Chatable] {
return container[models.Chatable]{
storageDirFolders: path.Elements{},
humanLocation: path.Elements{},
}
}

View File

@ -66,7 +66,7 @@ func updateStatus(
// or notMoved (if they match). // or notMoved (if they match).
func NewCollection[I chatsItemer]( func NewCollection[I chatsItemer](
baseCol data.BaseCollection, baseCol data.BaseCollection,
getter getItemer[I], filler fillItemer[I],
protectedResource string, protectedResource string,
items []I, items []I,
contains container[I], contains container[I],
@ -76,7 +76,7 @@ func NewCollection[I chatsItemer](
BaseCollection: baseCol, BaseCollection: baseCol,
items: items, items: items,
contains: contains, contains: contains,
getter: getter, filler: filler,
statusUpdater: statusUpdater, statusUpdater: statusUpdater,
stream: make(chan data.Item, collectionChannelBufferSize), stream: make(chan data.Item, collectionChannelBufferSize),
protectedResource: protectedResource, protectedResource: protectedResource,
@ -96,7 +96,7 @@ type lazyFetchCollection[I chatsItemer] struct {
items []I items []I
getter getItemer[I] filler fillItemer[I]
statusUpdater support.StatusUpdater statusUpdater support.StatusUpdater
} }
@ -166,9 +166,9 @@ func (col *lazyFetchCollection[I]) streamItems(ctx context.Context, errs *fault.
col.stream <- data.NewLazyItemWithInfo( col.stream <- data.NewLazyItemWithInfo(
ictx, ictx,
&lazyItemGetter[I]{ &lazyItemFiller[I]{
modTime: modTime, modTime: modTime,
getter: col.getter, filler: col.filler,
resourceID: col.protectedResource, resourceID: col.protectedResource,
item: item, item: item,
containerIDs: col.FullPath().Folders(), containerIDs: col.FullPath().Folders(),
@ -191,8 +191,8 @@ func (col *lazyFetchCollection[I]) streamItems(ctx context.Context, errs *fault.
wg.Wait() wg.Wait()
} }
type lazyItemGetter[I chatsItemer] struct { type lazyItemFiller[I chatsItemer] struct {
getter getItemer[I] filler fillItemer[I]
resourceID string resourceID string
item I item I
parentPath string parentPath string
@ -201,14 +201,14 @@ type lazyItemGetter[I chatsItemer] struct {
contains container[I] contains container[I]
} }
func (lig *lazyItemGetter[I]) GetData( func (lig *lazyItemFiller[I]) GetData(
ctx context.Context, ctx context.Context,
errs *fault.Bus, errs *fault.Bus,
) (io.ReadCloser, *details.ItemInfo, bool, error) { ) (io.ReadCloser, *details.ItemInfo, bool, error) {
writer := kjson.NewJsonSerializationWriter() writer := kjson.NewJsonSerializationWriter()
defer writer.Close() defer writer.Close()
item, info, err := lig.getter.getItem(ctx, lig.item) item, info, err := lig.filler.fillItem(ctx, lig.item)
if err != nil { if err != nil {
// For items that were deleted in flight, add the skip label so that // For items that were deleted in flight, add the skip label so that
// they don't lead to recoverable failures during backup. // they don't lead to recoverable failures during backup.

View File

@ -148,12 +148,12 @@ func (suite *CollectionUnitSuite) TestNewCollection_state() {
} }
} }
type getAndAugmentChat struct { type fillChat struct {
err error err error
} }
//lint:ignore U1000 false linter issue due to generics //lint:ignore U1000 false linter issue due to generics
func (m getAndAugmentChat) getItem( func (m fillChat) fillItem(
_ context.Context, _ context.Context,
chat models.Chatable, chat models.Chatable,
) (models.Chatable, *details.TeamsChatsInfo, error) { ) (models.Chatable, *details.TeamsChatsInfo, error) {
@ -207,7 +207,7 @@ func (suite *CollectionUnitSuite) TestLazyFetchCollection_Items_LazyFetch() {
ctx, flush := tester.NewContext(t) ctx, flush := tester.NewContext(t)
defer flush() defer flush()
getterAugmenter := &getAndAugmentChat{} filler := &fillChat{}
col := &lazyFetchCollection[models.Chatable]{ col := &lazyFetchCollection[models.Chatable]{
BaseCollection: data.NewBaseCollection( BaseCollection: data.NewBaseCollection(
@ -219,7 +219,7 @@ func (suite *CollectionUnitSuite) TestLazyFetchCollection_Items_LazyFetch() {
count.New()), count.New()),
items: test.items, items: test.items,
contains: container[models.Chatable]{}, contains: container[models.Chatable]{},
getter: getterAugmenter, filler: filler,
stream: make(chan data.Item), stream: make(chan data.Item),
statusUpdater: statusUpdater, statusUpdater: statusUpdater,
} }
@ -272,16 +272,16 @@ func (suite *CollectionUnitSuite) TestLazyItem_GetDataErrors() {
chat := testdata.StubChats(uuid.NewString())[0] chat := testdata.StubChats(uuid.NewString())[0]
m := getAndAugmentChat{ m := fillChat{
err: test.getErr, err: test.getErr,
} }
li := data.NewLazyItemWithInfo( li := data.NewLazyItemWithInfo(
ctx, ctx,
&lazyItemGetter[models.Chatable]{ &lazyItemFiller[models.Chatable]{
resourceID: "resourceID", resourceID: "resourceID",
item: chat, item: chat,
getter: &m, filler: &m,
modTime: now, modTime: now,
parentPath: parentPath, parentPath: parentPath,
}, },
@ -316,16 +316,16 @@ func (suite *CollectionUnitSuite) TestLazyItem_ReturnsEmptyReaderOnDeletedInFlig
chat := testdata.StubChats(uuid.NewString())[0] chat := testdata.StubChats(uuid.NewString())[0]
m := getAndAugmentChat{ m := fillChat{
err: core.ErrNotFound, err: core.ErrNotFound,
} }
li := data.NewLazyItemWithInfo( li := data.NewLazyItemWithInfo(
ctx, ctx,
&lazyItemGetter[models.Chatable]{ &lazyItemFiller[models.Chatable]{
resourceID: "resourceID", resourceID: "resourceID",
item: chat, item: chat,
getter: &m, filler: &m,
modTime: now, modTime: now,
parentPath: parentPath, parentPath: parentPath,
}, },
@ -357,14 +357,14 @@ func (suite *CollectionUnitSuite) TestLazyItem() {
defer flush() defer flush()
chat := testdata.StubChats(uuid.NewString())[0] chat := testdata.StubChats(uuid.NewString())[0]
m := getAndAugmentChat{} m := fillChat{}
li := data.NewLazyItemWithInfo( li := data.NewLazyItemWithInfo(
ctx, ctx,
&lazyItemGetter[models.Chatable]{ &lazyItemFiller[models.Chatable]{
resourceID: "resourceID", resourceID: "resourceID",
item: chat, item: chat,
getter: &m, filler: &m,
modTime: now, modTime: now,
parentPath: parentPath, parentPath: parentPath,
}, },

View File

@ -8,7 +8,6 @@ import (
"github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/backup/details"
"github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/path"
"github.com/alcionai/corso/src/pkg/selectors" "github.com/alcionai/corso/src/pkg/selectors"
"github.com/alcionai/corso/src/pkg/services/m365/api"
"github.com/alcionai/corso/src/pkg/services/m365/api/graph" "github.com/alcionai/corso/src/pkg/services/m365/api/graph"
) )
@ -22,8 +21,7 @@ type chatsItemer interface {
type backupHandler[I chatsItemer] interface { type backupHandler[I chatsItemer] interface {
getContainerer[I] getContainerer[I]
getItemer[I] fillItemer[I]
getItemer[I]
getItemIDser[I] getItemIDser[I]
includeItemer[I] includeItemer[I]
canonicalPather canonicalPather
@ -33,10 +31,7 @@ type backupHandler[I chatsItemer] interface {
// within this handler set, only one container (the root) // within this handler set, only one container (the root)
// is expected // is expected
type getContainerer[I chatsItemer] interface { type getContainerer[I chatsItemer] interface {
getContainer( getContainer() container[I]
ctx context.Context,
cc api.CallConfig,
) (container[I], error)
} }
// gets all item IDs in the container // gets all item IDs in the container
@ -46,8 +41,10 @@ type getItemIDser[I chatsItemer] interface {
) ([]I, error) ) ([]I, error)
} }
type getItemer[I chatsItemer] interface { // fillItemer takes a complete item and extends it with data that
getItem( // gets lazily populated during item streaming.
type fillItemer[I chatsItemer] interface {
fillItem(
ctx context.Context, ctx context.Context,
i I, i I,
) (I, *details.TeamsChatsInfo, error) ) (I, *details.TeamsChatsInfo, error)
@ -73,8 +70,4 @@ type canonicalPather interface {
// Container management // Container management
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
type container[I chatsItemer] struct { type container[I chatsItemer] struct{}
storageDirFolders path.Elements
humanLocation path.Elements
container I
}

View File

@ -145,9 +145,13 @@ func backupChats(
scope.Category().PathType().HumanString()) scope.Category().PathType().HumanString())
defer close(progressMessage) defer close(progressMessage)
qp := graph.QueryParams{
ProtectedResource: bc.producerConfig.ProtectedResource,
TenantID: bc.creds.AzureTenantID,
}
bh := teamschats.NewUsersChatsBackupHandler( bh := teamschats.NewUsersChatsBackupHandler(
bc.creds.AzureTenantID, qp,
bc.producerConfig.ProtectedResource.ID(),
bc.apiCli.Chats()) bc.apiCli.Chats())
// Always disable lazy reader for channels until #4321 support is added // Always disable lazy reader for channels until #4321 support is added