From 0545365b12fa28982c85a9dbb50a468fa0b70bcc Mon Sep 17 00:00:00 2001 From: Keepers Date: Fri, 22 Sep 2023 11:34:06 -0600 Subject: [PATCH] move container enum to pager pattern (#4298) moves exchange container enumeration funcs in the api to the pager pattern established with item enumeration. --- #### Does this PR need a docs update or release note? - [x] :no_entry: No #### Type of change - [x] :broom: Tech Debt/Cleanup #### Test Plan - [x] :zap: Unit test - [x] :green_heart: E2E --- .../m365/collection/exchange/backup_test.go | 3 +- .../exchange/contacts_container_cache.go | 8 +- .../collection/exchange/container_resolver.go | 15 ++- .../exchange/events_container_cache.go | 1 + .../exchange/mail_container_cache.go | 8 +- src/internal/m365/controller_test.go | 70 +++++----- src/internal/m365/graph/cache_container.go | 1 - src/pkg/services/m365/api/contacts_pager.go | 115 ++++++++++------- src/pkg/services/m365/api/events_pager.go | 120 ++++++++++-------- src/pkg/services/m365/api/events_test.go | 3 +- src/pkg/services/m365/api/mail_pager.go | 114 +++++++---------- 11 files changed, 246 insertions(+), 212 deletions(-) diff --git a/src/internal/m365/collection/exchange/backup_test.go b/src/internal/m365/collection/exchange/backup_test.go index fa8a300f9..4b046fd47 100644 --- a/src/internal/m365/collection/exchange/backup_test.go +++ b/src/internal/m365/collection/exchange/backup_test.go @@ -810,7 +810,8 @@ func (suite *BackupIntgSuite) TestEventsSerializationRegression() { err := suite.ac.Events().EnumerateContainers( ctx, suite.user, - api.DefaultCalendar, + "", + false, fn, fault.New(true)) require.NoError(t, err, clues.ToCore(err)) diff --git a/src/internal/m365/collection/exchange/contacts_container_cache.go b/src/internal/m365/collection/exchange/contacts_container_cache.go index aa9a45518..758271a71 100644 --- a/src/internal/m365/collection/exchange/contacts_container_cache.go +++ b/src/internal/m365/collection/exchange/contacts_container_cache.go @@ -77,7 +77,13 @@ func (cfc *contactContainerCache) Populate( return clues.Wrap(err, "initializing") } - err := cfc.enumer.EnumerateContainers(ctx, cfc.userID, baseID, cfc.addFolder, errs) + err := cfc.enumer.EnumerateContainers( + ctx, + cfc.userID, + baseID, + false, + cfc.addFolder, + errs) if err != nil { return clues.Wrap(err, "enumerating containers") } diff --git a/src/internal/m365/collection/exchange/container_resolver.go b/src/internal/m365/collection/exchange/container_resolver.go index fff528bae..6f068f91b 100644 --- a/src/internal/m365/collection/exchange/container_resolver.go +++ b/src/internal/m365/collection/exchange/container_resolver.go @@ -27,6 +27,7 @@ type containersEnumerator interface { EnumerateContainers( ctx context.Context, userID, baseDirID string, + immutableIDs bool, fn func(graph.CachedContainer) error, errs *fault.Bus, ) error @@ -332,15 +333,17 @@ func (cr *containerResolver) LocationInCache(pathString string) (string, bool) { // addFolder adds a folder to the cache with the given ID. If the item is // already in the cache does nothing. The path for the item is not modified. func (cr *containerResolver) addFolder(cf graph.CachedContainer) error { + var err error + // Only require a non-nil non-empty parent if the path isn't already populated. if cf.Path() != nil { - if err := checkIDAndName(cf); err != nil { - return clues.Wrap(err, "adding item to cache") - } + err = checkIDAndName(cf) } else { - if err := checkRequiredValues(cf); err != nil { - return clues.Wrap(err, "adding item to cache") - } + err = checkRequiredValues(cf) + } + + if err != nil { + return clues.Wrap(err, "validating container for cache") } if _, ok := cr.cache[ptr.Val(cf.GetId())]; ok { diff --git a/src/internal/m365/collection/exchange/events_container_cache.go b/src/internal/m365/collection/exchange/events_container_cache.go index eb8a70ae4..0bcb65170 100644 --- a/src/internal/m365/collection/exchange/events_container_cache.go +++ b/src/internal/m365/collection/exchange/events_container_cache.go @@ -74,6 +74,7 @@ func (ecc *eventContainerCache) Populate( ctx, ecc.userID, "", + false, ecc.addFolder, errs) if err != nil { diff --git a/src/internal/m365/collection/exchange/mail_container_cache.go b/src/internal/m365/collection/exchange/mail_container_cache.go index c499de40b..f06068be4 100644 --- a/src/internal/m365/collection/exchange/mail_container_cache.go +++ b/src/internal/m365/collection/exchange/mail_container_cache.go @@ -100,7 +100,13 @@ func (mc *mailContainerCache) Populate( return clues.Wrap(err, "initializing") } - err := mc.enumer.EnumerateContainers(ctx, mc.userID, "", mc.addFolder, errs) + err := mc.enumer.EnumerateContainers( + ctx, + mc.userID, + "", + false, + mc.addFolder, + errs) if err != nil { return clues.Wrap(err, "enumerating containers") } diff --git a/src/internal/m365/controller_test.go b/src/internal/m365/controller_test.go index 72d9e4478..3f535880a 100644 --- a/src/internal/m365/controller_test.go +++ b/src/internal/m365/controller_test.go @@ -933,25 +933,25 @@ func (suite *ControllerIntegrationSuite) TestRestoreAndBackup_core() { // { // name: "MultipleEventsSingleCalendar", // service: path.ExchangeService, - // collections: []colInfo{ + // collections: []stub.ColInfo{ // { - // pathElements: []string{"Work"}, - // category: path.EventsCategory, - // items: []itemInfo{ + // PathElements: []string{"Work"}, + // Category: path.EventsCategory, + // Items: []stub.ItemInfo{ // { - // name: "someencodeditemID", - // data: exchMock.EventWithSubjectBytes("Ghimley"), - // lookupKey: "Ghimley", + // Name: "someencodeditemID", + // Data: exchMock.EventWithSubjectBytes("Ghimley"), + // LookupKey: "Ghimley", // }, // { - // name: "someencodeditemID2", - // data: exchMock.EventWithSubjectBytes("Irgot"), - // lookupKey: "Irgot", + // Name: "someencodeditemID2", + // Data: exchMock.EventWithSubjectBytes("Irgot"), + // LookupKey: "Irgot", // }, // { - // name: "someencodeditemID3", - // data: exchMock.EventWithSubjectBytes("Jannes"), - // lookupKey: "Jannes", + // Name: "someencodeditemID3", + // Data: exchMock.EventWithSubjectBytes("Jannes"), + // LookupKey: "Jannes", // }, // }, // }, @@ -960,41 +960,41 @@ func (suite *ControllerIntegrationSuite) TestRestoreAndBackup_core() { // { // name: "MultipleEventsMultipleCalendars", // service: path.ExchangeService, - // collections: []colInfo{ + // collections: []stub.ColInfo{ // { - // pathElements: []string{"Work"}, - // category: path.EventsCategory, - // items: []itemInfo{ + // PathElements: []string{"Work"}, + // Category: path.EventsCategory, + // Items: []stub.ItemInfo{ // { - // name: "someencodeditemID", - // data: exchMock.EventWithSubjectBytes("Ghimley"), - // lookupKey: "Ghimley", + // Name: "someencodeditemID", + // Data: exchMock.EventWithSubjectBytes("Ghimley"), + // LookupKey: "Ghimley", // }, // { - // name: "someencodeditemID2", - // data: exchMock.EventWithSubjectBytes("Irgot"), - // lookupKey: "Irgot", + // Name: "someencodeditemID2", + // Data: exchMock.EventWithSubjectBytes("Irgot"), + // LookupKey: "Irgot", // }, // { - // name: "someencodeditemID3", - // data: exchMock.EventWithSubjectBytes("Jannes"), - // lookupKey: "Jannes", + // Name: "someencodeditemID3", + // Data: exchMock.EventWithSubjectBytes("Jannes"), + // LookupKey: "Jannes", // }, // }, // }, // { - // pathElements: []string{"Personal"}, - // category: path.EventsCategory, - // items: []itemInfo{ + // PathElements: []string{"Personal"}, + // Category: path.EventsCategory, + // Items: []stub.ItemInfo{ // { - // name: "someencodeditemID4", - // data: exchMock.EventWithSubjectBytes("Argon"), - // lookupKey: "Argon", + // Name: "someencodeditemID4", + // Data: exchMock.EventWithSubjectBytes("Argon"), + // LookupKey: "Argon", // }, // { - // name: "someencodeditemID5", - // data: exchMock.EventWithSubjectBytes("Bernard"), - // lookupKey: "Bernard", + // Name: "someencodeditemID5", + // Data: exchMock.EventWithSubjectBytes("Bernard"), + // LookupKey: "Bernard", // }, // }, // }, diff --git a/src/internal/m365/graph/cache_container.go b/src/internal/m365/graph/cache_container.go index d83b17d74..ba8e5b819 100644 --- a/src/internal/m365/graph/cache_container.go +++ b/src/internal/m365/graph/cache_container.go @@ -95,7 +95,6 @@ type CacheFolder struct { p *path.Builder } -// NewCacheFolder public constructor for struct func NewCacheFolder(c Container, pb, lpb *path.Builder) CacheFolder { cf := CacheFolder{ Container: c, diff --git a/src/pkg/services/m365/api/contacts_pager.go b/src/pkg/services/m365/api/contacts_pager.go index 6c8d90e74..5d43a5172 100644 --- a/src/pkg/services/m365/api/contacts_pager.go +++ b/src/pkg/services/m365/api/contacts_pager.go @@ -18,25 +18,29 @@ import ( // container pager // --------------------------------------------------------------------------- -// EnumerateContainers iterates through all of the users current -// contacts folders, converting each to a graph.CacheFolder, and calling -// fn(cf) on each one. -// Folder hierarchy is represented in its current state, and does -// not contain historical data. -// TODO: use enumerateItems for containers -func (c Contacts) EnumerateContainers( - ctx context.Context, +var _ Pager[models.ContactFolderable] = &contactsFoldersPageCtrl{} + +type contactsFoldersPageCtrl struct { + gs graph.Servicer + builder *users.ItemContactFoldersItemChildFoldersRequestBuilder + options *users.ItemContactFoldersItemChildFoldersRequestBuilderGetRequestConfiguration +} + +func (c Contacts) NewContactFoldersPager( userID, baseContainerID string, - fn func(graph.CachedContainer) error, - errs *fault.Bus, -) error { - config := &users.ItemContactFoldersItemChildFoldersRequestBuilderGetRequestConfiguration{ - QueryParameters: &users.ItemContactFoldersItemChildFoldersRequestBuilderGetQueryParameters{ - Select: idAnd(displayName, parentFolderID), - }, + immutableIDs bool, + selectProps ...string, +) Pager[models.ContactFolderable] { + options := &users.ItemContactFoldersItemChildFoldersRequestBuilderGetRequestConfiguration{ + Headers: newPreferHeaders(preferPageSize(maxNonDeltaPageSize), preferImmutableIDs(immutableIDs)), + QueryParameters: &users.ItemContactFoldersItemChildFoldersRequestBuilderGetQueryParameters{}, + // do NOT set Top. It limits the total items received. + } + + if len(selectProps) > 0 { + options.QueryParameters.Select = selectProps } - el := errs.Local() builder := c.Stable. Client(). Users(). @@ -45,44 +49,57 @@ func (c Contacts) EnumerateContainers( ByContactFolderId(baseContainerID). ChildFolders() - for { + return &contactsFoldersPageCtrl{c.Stable, builder, options} +} + +func (p *contactsFoldersPageCtrl) GetPage( + ctx context.Context, +) (NextLinkValuer[models.ContactFolderable], error) { + resp, err := p.builder.Get(ctx, p.options) + return resp, graph.Stack(ctx, err).OrNil() +} + +func (p *contactsFoldersPageCtrl) SetNextLink(nextLink string) { + p.builder = users.NewItemContactFoldersItemChildFoldersRequestBuilder(nextLink, p.gs.Adapter()) +} + +func (p *contactsFoldersPageCtrl) ValidModTimes() bool { + return true +} + +// EnumerateContainers iterates through all of the users current +// contacts folders, transforming each to a graph.CacheFolder, and calling +// fn(cf). +// Contact folders are represented in their current state, and do +// not contain historical data. +func (c Contacts) EnumerateContainers( + ctx context.Context, + userID, baseContainerID string, + immutableIDs bool, + fn func(graph.CachedContainer) error, + errs *fault.Bus, +) error { + var ( + el = errs.Local() + pgr = c.NewContactFoldersPager(userID, baseContainerID, immutableIDs) + ) + + containers, err := enumerateItems(ctx, pgr) + if err != nil { + return graph.Stack(ctx, err) + } + + for _, c := range containers { if el.Failure() != nil { break } - resp, err := builder.Get(ctx, config) - if err != nil { - return graph.Stack(ctx, err) + gncf := graph.NewCacheFolder(c, nil, nil) + + if err := fn(&gncf); err != nil { + errs.AddRecoverable(ctx, graph.Stack(ctx, err).Label(fault.LabelForceNoBackupCreation)) + continue } - - for _, fold := range resp.GetValue() { - if el.Failure() != nil { - return el.Failure() - } - - if err := graph.CheckIDNameAndParentFolderID(fold); err != nil { - errs.AddRecoverable(ctx, graph.Stack(ctx, err).Label(fault.LabelForceNoBackupCreation)) - continue - } - - fctx := clues.Add( - ctx, - "container_id", ptr.Val(fold.GetId()), - "container_display_name", ptr.Val(fold.GetDisplayName())) - - temp := graph.NewCacheFolder(fold, nil, nil) - if err := fn(&temp); err != nil { - errs.AddRecoverable(ctx, graph.Stack(fctx, err).Label(fault.LabelForceNoBackupCreation)) - continue - } - } - - link, ok := ptr.ValOK(resp.GetOdataNextLink()) - if !ok { - break - } - - builder = users.NewItemContactFoldersItemChildFoldersRequestBuilder(link, c.Stable.Adapter()) } return el.Failure() diff --git a/src/pkg/services/m365/api/events_pager.go b/src/pkg/services/m365/api/events_pager.go index b1ea1bedc..38c985770 100644 --- a/src/pkg/services/m365/api/events_pager.go +++ b/src/pkg/services/m365/api/events_pager.go @@ -21,73 +21,89 @@ const eventBetaDeltaURLTemplate = "https://graph.microsoft.com/beta/users/%s/cal // container pager // --------------------------------------------------------------------------- +var _ Pager[models.Calendarable] = &eventsCalendarsPageCtrl{} + +type eventsCalendarsPageCtrl struct { + gs graph.Servicer + builder *users.ItemCalendarsRequestBuilder + options *users.ItemCalendarsRequestBuilderGetRequestConfiguration +} + +func (c Events) NewEventCalendarsPager( + userID string, + immutableIDs bool, + selectProps ...string, +) Pager[models.Calendarable] { + options := &users.ItemCalendarsRequestBuilderGetRequestConfiguration{ + Headers: newPreferHeaders(preferPageSize(maxNonDeltaPageSize), preferImmutableIDs(immutableIDs)), + QueryParameters: &users.ItemCalendarsRequestBuilderGetQueryParameters{}, + // do NOT set Top. It limits the total items received. + } + + if len(selectProps) > 0 { + options.QueryParameters.Select = selectProps + } + + builder := c.Stable. + Client(). + Users(). + ByUserId(userID). + Calendars() + + return &eventsCalendarsPageCtrl{c.Stable, builder, options} +} + +func (p *eventsCalendarsPageCtrl) GetPage( + ctx context.Context, +) (NextLinkValuer[models.Calendarable], error) { + resp, err := p.builder.Get(ctx, p.options) + return resp, graph.Stack(ctx, err).OrNil() +} + +func (p *eventsCalendarsPageCtrl) SetNextLink(nextLink string) { + p.builder = users.NewItemCalendarsRequestBuilder(nextLink, p.gs.Adapter()) +} + +func (p *eventsCalendarsPageCtrl) ValidModTimes() bool { + return true +} + // EnumerateContainers iterates through all of the users current -// calendars, converting each to a graph.CacheFolder, and -// calling fn(cf) on each one. -// Folder hierarchy is represented in its current state, and does +// events calendars, transforming each to a graph.CacheFolder, and calling +// fn(cf). +// Calendars are represented in their current state, and do // not contain historical data. func (c Events) EnumerateContainers( ctx context.Context, - userID, baseContainerID string, + userID, _ string, // baseContainerID not needed + immutableIDs bool, fn func(graph.CachedContainer) error, errs *fault.Bus, ) error { var ( - el = errs.Local() - config = &users.ItemCalendarsRequestBuilderGetRequestConfiguration{ - QueryParameters: &users.ItemCalendarsRequestBuilderGetQueryParameters{ - Select: idAnd("name"), - }, - } - builder = c.Stable. - Client(). - Users(). - ByUserId(userID). - Calendars() + el = errs.Local() + pgr = c.NewEventCalendarsPager(userID, immutableIDs) ) - for { + containers, err := enumerateItems(ctx, pgr) + if err != nil { + return graph.Stack(ctx, err) + } + + for _, c := range containers { if el.Failure() != nil { break } - resp, err := builder.Get(ctx, config) - if err != nil { - return graph.Stack(ctx, err) + gncf := graph.NewCacheFolder( + CalendarDisplayable{Calendarable: c}, + path.Builder{}.Append(ptr.Val(c.GetId())), + path.Builder{}.Append(ptr.Val(c.GetName()))) + + if err := fn(&gncf); err != nil { + errs.AddRecoverable(ctx, graph.Stack(ctx, err).Label(fault.LabelForceNoBackupCreation)) + continue } - - for _, cal := range resp.GetValue() { - if el.Failure() != nil { - break - } - - cd := CalendarDisplayable{Calendarable: cal} - if err := graph.CheckIDAndName(cd); err != nil { - errs.AddRecoverable(ctx, graph.Stack(ctx, err).Label(fault.LabelForceNoBackupCreation)) - continue - } - - fctx := clues.Add( - ctx, - "container_id", ptr.Val(cal.GetId()), - "container_name", ptr.Val(cal.GetName())) - - temp := graph.NewCacheFolder( - cd, - path.Builder{}.Append(ptr.Val(cd.GetId())), // storage path - path.Builder{}.Append(ptr.Val(cd.GetDisplayName()))) // display location - if err := fn(&temp); err != nil { - errs.AddRecoverable(ctx, graph.Stack(fctx, err).Label(fault.LabelForceNoBackupCreation)) - continue - } - } - - link, ok := ptr.ValOK(resp.GetOdataNextLink()) - if !ok { - break - } - - builder = users.NewItemCalendarsRequestBuilder(link, c.Stable.Adapter()) } return el.Failure() diff --git a/src/pkg/services/m365/api/events_test.go b/src/pkg/services/m365/api/events_test.go index 6de81a9a6..f52f999e5 100644 --- a/src/pkg/services/m365/api/events_test.go +++ b/src/pkg/services/m365/api/events_test.go @@ -306,7 +306,8 @@ func (suite *EventsAPIIntgSuite) TestEvents_canFindNonStandardFolder() { err = ac.EnumerateContainers( ctx, suite.its.user.id, - "Calendar", + api.DefaultCalendar, + false, findContainer, fault.New(true)) require.NoError(t, err, clues.ToCore(err)) diff --git a/src/pkg/services/m365/api/mail_pager.go b/src/pkg/services/m365/api/mail_pager.go index 7d32e6ce2..1aa6c8414 100644 --- a/src/pkg/services/m365/api/mail_pager.go +++ b/src/pkg/services/m365/api/mail_pager.go @@ -19,100 +19,84 @@ import ( // container pager // --------------------------------------------------------------------------- -type mailFolderPager struct { - service graph.Servicer +var _ Pager[models.MailFolderable] = &mailFoldersPageCtrl{} + +type mailFoldersPageCtrl struct { + gs graph.Servicer builder *users.ItemMailFoldersRequestBuilder + options *users.ItemMailFoldersRequestBuilderGetRequestConfiguration } -func (c Mail) NewMailFolderPager(userID string) mailFolderPager { +func (c Mail) NewMailFoldersPager( + userID string, + immutableIDs bool, + selectProps ...string, +) Pager[models.MailFolderable] { + options := &users.ItemMailFoldersRequestBuilderGetRequestConfiguration{ + Headers: newPreferHeaders(preferPageSize(maxNonDeltaPageSize), preferImmutableIDs(immutableIDs)), + QueryParameters: &users.ItemMailFoldersRequestBuilderGetQueryParameters{}, + // do NOT set Top. It limits the total items received. + } + + if len(selectProps) > 0 { + options.QueryParameters.Select = selectProps + } + // v1.0 non delta /mailFolders endpoint does not return any of the nested folders rawURL := fmt.Sprintf(mailFoldersBetaURLTemplate, userID) builder := users.NewItemMailFoldersRequestBuilder(rawURL, c.Stable.Adapter()) - return mailFolderPager{c.Stable, builder} + return &mailFoldersPageCtrl{c.Stable, builder, options} } -func (p *mailFolderPager) getPage(ctx context.Context) (NextLinker, error) { - page, err := p.builder.Get(ctx, nil) - if err != nil { - return nil, graph.Stack(ctx, err) - } - - return page, nil +func (p *mailFoldersPageCtrl) GetPage( + ctx context.Context, +) (NextLinkValuer[models.MailFolderable], error) { + resp, err := p.builder.Get(ctx, p.options) + return resp, graph.Stack(ctx, err).OrNil() } -func (p *mailFolderPager) SetNextLink(nextLink string) { - p.builder = users.NewItemMailFoldersRequestBuilder(nextLink, p.service.Adapter()) +func (p *mailFoldersPageCtrl) SetNextLink(nextLink string) { + p.builder = users.NewItemMailFoldersRequestBuilder(nextLink, p.gs.Adapter()) } -func (p *mailFolderPager) valuesIn(pl NextLinker) ([]models.MailFolderable, error) { - // Ideally this should be `users.ItemMailFoldersResponseable`, but - // that is not a thing as stable returns different result - page, ok := pl.(models.MailFolderCollectionResponseable) - if !ok { - return nil, clues.New("converting to ItemMailFoldersResponseable") - } - - return page.GetValue(), nil +func (p *mailFoldersPageCtrl) ValidModTimes() bool { + return true } // EnumerateContainers iterates through all of the users current -// mail folders, converting each to a graph.CacheFolder, and calling -// fn(cf) on each one. +// mail folders, transforming each to a graph.CacheFolder, and calling +// fn(cf). // Folder hierarchy is represented in its current state, and does // not contain historical data. func (c Mail) EnumerateContainers( ctx context.Context, - userID, baseContainerID string, + userID, _ string, // baseContainerID not needed here + immutableIDs bool, fn func(graph.CachedContainer) error, errs *fault.Bus, ) error { - el := errs.Local() - pgr := c.NewMailFolderPager(userID) + var ( + el = errs.Local() + pgr = c.NewMailFoldersPager(userID, immutableIDs) + ) - for { + containers, err := enumerateItems(ctx, pgr) + if err != nil { + return graph.Stack(ctx, err) + } + + for _, c := range containers { if el.Failure() != nil { break } - page, err := pgr.getPage(ctx) - if err != nil { - return graph.Stack(ctx, err) + gncf := graph.NewCacheFolder(c, nil, nil) + + if err := fn(&gncf); err != nil { + errs.AddRecoverable(ctx, graph.Stack(ctx, err).Label(fault.LabelForceNoBackupCreation)) + continue } - - resp, err := pgr.valuesIn(page) - if err != nil { - return graph.Stack(ctx, err) - } - - for _, fold := range resp { - if el.Failure() != nil { - break - } - - if err := graph.CheckIDNameAndParentFolderID(fold); err != nil { - errs.AddRecoverable(ctx, graph.Stack(ctx, err).Label(fault.LabelForceNoBackupCreation)) - continue - } - - fctx := clues.Add( - ctx, - "container_id", ptr.Val(fold.GetId()), - "container_name", ptr.Val(fold.GetDisplayName())) - - temp := graph.NewCacheFolder(fold, nil, nil) - if err := fn(&temp); err != nil { - errs.AddRecoverable(ctx, graph.Stack(fctx, err).Label(fault.LabelForceNoBackupCreation)) - continue - } - } - - link, ok := ptr.ValOK(page.GetOdataNextLink()) - if !ok { - break - } - - pgr.SetNextLink(link) } return el.Failure()