From b327de5801c650925121a05bd9a303ad674620e7 Mon Sep 17 00:00:00 2001 From: Abin Simon Date: Tue, 7 Feb 2023 10:55:22 +0530 Subject: [PATCH] Pass in prev delta to collectItems cont. (#2400) ## Description Forgot to push the review changes in https://github.com/alcionai/corso/pull/2371 . ## Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No ## Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup ## Issue(s) * # ## Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/connector/onedrive/api/drive.go | 6 ++++++ src/internal/connector/onedrive/collections_test.go | 12 ++++++------ src/internal/connector/onedrive/drive.go | 7 +++---- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/internal/connector/onedrive/api/drive.go b/src/internal/connector/onedrive/api/drive.go index ce246da85..391ea2632 100644 --- a/src/internal/connector/onedrive/api/drive.go +++ b/src/internal/connector/onedrive/api/drive.go @@ -30,6 +30,7 @@ const pageSize = int32(999) type driveItemPager struct { gs graph.Servicer + driveID string builder *msdrives.ItemRootDeltaRequestBuilder options *msdrives.ItemRootDeltaRequestBuilderGetRequestConfiguration } @@ -49,6 +50,7 @@ func NewItemPager( res := &driveItemPager{ gs: gs, + driveID: driveID, options: requestConfig, builder: gs.Client().DrivesById(driveID).Root().Delta(), } @@ -78,6 +80,10 @@ func (p *driveItemPager) SetNext(link string) { p.builder = msdrives.NewItemRootDeltaRequestBuilder(link, p.gs.Adapter()) } +func (p *driveItemPager) Reset() { + p.builder = p.gs.Client().DrivesById(p.driveID).Root().Delta() +} + func (p *driveItemPager) ValuesIn(l api.DeltaPageLinker) ([]models.DriveItemable, error) { return getValues[models.DriveItemable](l) } diff --git a/src/internal/connector/onedrive/collections_test.go b/src/internal/connector/onedrive/collections_test.go index 0b10d02bd..b172204c9 100644 --- a/src/internal/connector/onedrive/collections_test.go +++ b/src/internal/connector/onedrive/collections_test.go @@ -1048,6 +1048,7 @@ func (p *mockItemPager) GetPage(context.Context) (gapi.DeltaPageLinker, error) { } func (p *mockItemPager) SetNext(string) {} +func (p *mockItemPager) Reset() {} func (p *mockItemPager) ValuesIn(gapi.DeltaPageLinker) ([]models.DriveItemable, error) { idx := p.getIdx @@ -1497,7 +1498,6 @@ func (suite *OneDriveCollectionsSuite) TestCollectItems() { name string items []deltaPagerResult deltaURL string - prevDelta string prevDeltaSuccess bool err error }{ @@ -1522,7 +1522,7 @@ func (suite *OneDriveCollectionsSuite) TestCollectItems() { name: "invalid prev delta", deltaURL: delta, items: []deltaPagerResult{ - {nextLink: &next, err: deltaError}, + {err: deltaError}, {deltaLink: &delta}, // works on retry }, prevDeltaSuccess: false, @@ -1531,7 +1531,7 @@ func (suite *OneDriveCollectionsSuite) TestCollectItems() { name: "fail a normal delta query", items: []deltaPagerResult{ {nextLink: &next}, - {nextLink: &next, err: assert.AnError}, + {err: assert.AnError}, }, prevDeltaSuccess: true, err: assert.AnError, @@ -1566,9 +1566,9 @@ func (suite *OneDriveCollectionsSuite) TestCollectItems() { "", ) - require.ErrorIs(suite.T(), err, test.err) - require.Equal(suite.T(), test.deltaURL, delta.URL) - require.Equal(suite.T(), !test.prevDeltaSuccess, delta.Reset) + require.ErrorIs(suite.T(), err, test.err, "delta fetch err") + require.Equal(suite.T(), test.deltaURL, delta.URL, "delta url") + require.Equal(suite.T(), !test.prevDeltaSuccess, delta.Reset, "delta reset") }) } } diff --git a/src/internal/connector/onedrive/drive.go b/src/internal/connector/onedrive/drive.go index b06184884..a7585ffe3 100644 --- a/src/internal/connector/onedrive/drive.go +++ b/src/internal/connector/onedrive/drive.go @@ -148,6 +148,7 @@ type itemCollector func( type itemPager interface { GetPage(context.Context) (gapi.DeltaPageLinker, error) SetNext(nextLink string) + Reset() ValuesIn(gapi.DeltaPageLinker) ([]models.DriveItemable, error) } @@ -193,7 +194,6 @@ func collectItems( newPaths = map[string]string{} excluded = map[string]struct{}{} invalidPrevDelta = false - triedPrevDelta = false ) maps.Copy(newPaths, oldPaths) @@ -205,13 +205,12 @@ func collectItems( for { page, err := pager.GetPage(ctx) - if !triedPrevDelta && graph.IsErrInvalidDelta(err) { + if graph.IsErrInvalidDelta(err) { logger.Ctx(ctx).Infow("Invalid previous delta link", "link", prevDelta) - triedPrevDelta = true // TODO(meain): Do we need this check? invalidPrevDelta = true - pager.SetNext("") + pager.Reset() continue }