From 873b404386229b0cccd4af594ba0a527f5906d33 Mon Sep 17 00:00:00 2001 From: ryanfkeepers Date: Thu, 16 Feb 2023 13:18:09 -0700 Subject: [PATCH] migrate deltaPaths to graph, add qp validator Adds a queryParam validation step. Migrates the DeltaPaths struct into graph so that it can have centralized usage. --- .../connector/exchange/data_collections.go | 43 ++----- .../exchange/data_collections_test.go | 48 ++++---- .../connector/exchange/service_iterators.go | 10 +- .../exchange/service_iterators_test.go | 108 +++++++++--------- src/internal/connector/graph/query_params.go | 66 +++++++++++ src/internal/connector/graph/service.go | 8 -- 6 files changed, 157 insertions(+), 126 deletions(-) create mode 100644 src/internal/connector/graph/query_params.go diff --git a/src/internal/connector/exchange/data_collections.go b/src/internal/connector/exchange/data_collections.go index 5becad190..e5e486147 100644 --- a/src/internal/connector/exchange/data_collections.go +++ b/src/internal/connector/exchange/data_collections.go @@ -30,44 +30,15 @@ func MetadataFileNames(cat path.CategoryType) []string { } } -type CatDeltaPaths map[path.CategoryType]DeltaPaths - -type DeltaPaths map[string]DeltaPath - -func (dps DeltaPaths) AddDelta(k, d string) { - dp, ok := dps[k] - if !ok { - dp = DeltaPath{} - } - - dp.delta = d - dps[k] = dp -} - -func (dps DeltaPaths) AddPath(k, p string) { - dp, ok := dps[k] - if !ok { - dp = DeltaPath{} - } - - dp.path = p - dps[k] = dp -} - -type DeltaPath struct { - delta string - path string -} - // ParseMetadataCollections produces a map of structs holding delta // and path lookup maps. func parseMetadataCollections( ctx context.Context, colls []data.RestoreCollection, errs *fault.Errors, -) (CatDeltaPaths, error) { +) (graph.CatDeltaPaths, error) { // cdp stores metadata - cdp := CatDeltaPaths{ + cdp := graph.CatDeltaPaths{ path.ContactsCategory: {}, path.EmailCategory: {}, path.EventsCategory: {}, @@ -147,7 +118,7 @@ func parseMetadataCollections( // complete backup on the next run. for _, dps := range cdp { for k, dp := range dps { - if len(dp.delta) == 0 || len(dp.path) == 0 { + if len(dp.Delta) == 0 || len(dp.Path) == 0 { delete(dps, k) } } @@ -231,7 +202,7 @@ func createCollections( creds account.M365Config, user string, scope selectors.ExchangeScope, - dps DeltaPaths, + dps graph.DeltaPaths, ctrlOpts control.Options, su support.StatusUpdater, errs *fault.Errors, @@ -256,6 +227,11 @@ func createCollections( Category: category, ResourceOwner: user, Credentials: creds, + DeltaPaths: dps, + } + + if err := qp.Validate(); err != nil { + return nil, clues.Wrap(err, "validating collection parameters").WithClues(ctx) } foldersComplete, closer := observe.MessageWithCompletion(ctx, observe.Bulletf( @@ -278,7 +254,6 @@ func createCollections( su, resolver, scope, - dps, ctrlOpts, errs) if err != nil { diff --git a/src/internal/connector/exchange/data_collections_test.go b/src/internal/connector/exchange/data_collections_test.go index c4920eef2..564a440ab 100644 --- a/src/internal/connector/exchange/data_collections_test.go +++ b/src/internal/connector/exchange/data_collections_test.go @@ -41,7 +41,7 @@ func (suite *DataCollectionsUnitSuite) TestParseMetadataCollections() { table := []struct { name string data []fileValues - expect map[string]DeltaPath + expect map[string]graph.DeltaPath expectError assert.ErrorAssertionFunc }{ { @@ -49,7 +49,7 @@ func (suite *DataCollectionsUnitSuite) TestParseMetadataCollections() { data: []fileValues{ {graph.DeltaURLsFileName, "delta-link"}, }, - expect: map[string]DeltaPath{}, + expect: map[string]graph.DeltaPath{}, expectError: assert.NoError, }, { @@ -65,7 +65,7 @@ func (suite *DataCollectionsUnitSuite) TestParseMetadataCollections() { data: []fileValues{ {graph.PreviousPathFileName, "prev-path"}, }, - expect: map[string]DeltaPath{}, + expect: map[string]graph.DeltaPath{}, expectError: assert.NoError, }, { @@ -82,10 +82,10 @@ func (suite *DataCollectionsUnitSuite) TestParseMetadataCollections() { {graph.DeltaURLsFileName, "delta-link"}, {graph.PreviousPathFileName, "prev-path"}, }, - expect: map[string]DeltaPath{ + expect: map[string]graph.DeltaPath{ "key": { - delta: "delta-link", - path: "prev-path", + Delta: "delta-link", + Path: "prev-path", }, }, expectError: assert.NoError, @@ -96,7 +96,7 @@ func (suite *DataCollectionsUnitSuite) TestParseMetadataCollections() { {graph.DeltaURLsFileName, "delta-link"}, {graph.PreviousPathFileName, ""}, }, - expect: map[string]DeltaPath{}, + expect: map[string]graph.DeltaPath{}, expectError: assert.NoError, }, { @@ -105,7 +105,7 @@ func (suite *DataCollectionsUnitSuite) TestParseMetadataCollections() { {graph.DeltaURLsFileName, ""}, {graph.PreviousPathFileName, "prev-path"}, }, - expect: map[string]DeltaPath{}, + expect: map[string]graph.DeltaPath{}, expectError: assert.NoError, }, { @@ -114,10 +114,10 @@ func (suite *DataCollectionsUnitSuite) TestParseMetadataCollections() { {graph.DeltaURLsFileName, "`!@#$%^&*()_[]{}/\"\\"}, {graph.PreviousPathFileName, "prev-path"}, }, - expect: map[string]DeltaPath{ + expect: map[string]graph.DeltaPath{ "key": { - delta: "`!@#$%^&*()_[]{}/\"\\", - path: "prev-path", + Delta: "`!@#$%^&*()_[]{}/\"\\", + Path: "prev-path", }, }, expectError: assert.NoError, @@ -128,10 +128,10 @@ func (suite *DataCollectionsUnitSuite) TestParseMetadataCollections() { {graph.DeltaURLsFileName, `\n\r\t\b\f\v\0\\`}, {graph.PreviousPathFileName, "prev-path"}, }, - expect: map[string]DeltaPath{ + expect: map[string]graph.DeltaPath{ "key": { - delta: "\\n\\r\\t\\b\\f\\v\\0\\\\", - path: "prev-path", + Delta: "\\n\\r\\t\\b\\f\\v\\0\\\\", + Path: "prev-path", }, }, expectError: assert.NoError, @@ -145,10 +145,10 @@ func (suite *DataCollectionsUnitSuite) TestParseMetadataCollections() { {graph.DeltaURLsFileName, string([]rune{rune(92), rune(110)})}, {graph.PreviousPathFileName, "prev-path"}, }, - expect: map[string]DeltaPath{ + expect: map[string]graph.DeltaPath{ "key": { - delta: "\\n", - path: "prev-path", + Delta: "\\n", + Path: "prev-path", }, }, expectError: assert.NoError, @@ -186,8 +186,8 @@ func (suite *DataCollectionsUnitSuite) TestParseMetadataCollections() { assert.Len(t, emails, len(test.expect)) for k, v := range emails { - assert.Equal(t, v.delta, emails[k].delta, "delta") - assert.Equal(t, v.path, emails[k].path, "path") + assert.Equal(t, v.Delta, emails[k].Delta, "delta") + assert.Equal(t, v.Path, emails[k].Path, "path") } }) } @@ -266,7 +266,7 @@ func (suite *DataCollectionsIntegrationSuite) TestMailFetch() { acct, userID, test.scope, - DeltaPaths{}, + graph.DeltaPaths{}, control.Options{}, func(status *support.ConnectorOperationStatus) {}, fault.New(true)) @@ -334,7 +334,7 @@ func (suite *DataCollectionsIntegrationSuite) TestDelta() { acct, userID, test.scope, - DeltaPaths{}, + graph.DeltaPaths{}, control.Options{}, func(status *support.ConnectorOperationStatus) {}, fault.New(true)) @@ -411,7 +411,7 @@ func (suite *DataCollectionsIntegrationSuite) TestMailSerializationRegression() acct, suite.user, sel.Scopes()[0], - DeltaPaths{}, + graph.DeltaPaths{}, control.Options{}, newStatusUpdater(t, &wg), fault.New(true)) @@ -479,7 +479,7 @@ func (suite *DataCollectionsIntegrationSuite) TestContactSerializationRegression acct, suite.user, test.scope, - DeltaPaths{}, + graph.DeltaPaths{}, control.Options{}, newStatusUpdater(t, &wg), fault.New(true)) @@ -587,7 +587,7 @@ func (suite *DataCollectionsIntegrationSuite) TestEventsSerializationRegression( acct, suite.user, test.scope, - DeltaPaths{}, + graph.DeltaPaths{}, control.Options{}, newStatusUpdater(t, &wg), fault.New(true)) diff --git a/src/internal/connector/exchange/service_iterators.go b/src/internal/connector/exchange/service_iterators.go index 49a167437..2171961ff 100644 --- a/src/internal/connector/exchange/service_iterators.go +++ b/src/internal/connector/exchange/service_iterators.go @@ -37,7 +37,6 @@ func filterContainersAndFillCollections( statusUpdater support.StatusUpdater, resolver graph.ContainerResolver, scope selectors.ExchangeScope, - dps DeltaPaths, ctrlOpts control.Options, errs *fault.Errors, ) error { @@ -45,6 +44,7 @@ func filterContainersAndFillCollections( // folder ID -> delta url or folder path lookups deltaURLs = map[string]string{} currPaths = map[string]string{} + dps = qp.DeltaPaths // copy of previousPaths. any folder found in the resolver get // deleted from this map, leaving only the deleted folders behind tombstones = makeTombstones(dps) @@ -79,8 +79,8 @@ func filterContainersAndFillCollections( var ( dp = dps[cID] - prevDelta = dp.delta - prevPathStr = dp.path + prevDelta = dp.Delta + prevPathStr = dp.Path prevPath path.Path ) @@ -209,11 +209,11 @@ func filterContainersAndFillCollections( // produces a set of id:path pairs from the deltapaths map. // Each entry in the set will, if not removed, produce a collection // that will delete the tombstone by path. -func makeTombstones(dps DeltaPaths) map[string]string { +func makeTombstones(dps graph.DeltaPaths) map[string]string { r := make(map[string]string, len(dps)) for id, v := range dps { - r[id] = v.path + r[id] = v.Path } return r diff --git a/src/internal/connector/exchange/service_iterators_test.go b/src/internal/connector/exchange/service_iterators_test.go index 70c9a5b37..50aff190b 100644 --- a/src/internal/connector/exchange/service_iterators_test.go +++ b/src/internal/connector/exchange/service_iterators_test.go @@ -121,10 +121,10 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections() { Category: path.EmailCategory, // doesn't matter which one we use. ResourceOwner: userID, Credentials: suite.creds, + DeltaPaths: graph.DeltaPaths{}, // incrementals are tested separately } statusUpdater = func(*support.ConnectorOperationStatus) {} allScope = selectors.NewExchangeBackup(nil).MailFolders(selectors.Any())[0] - dps = DeltaPaths{} // incrementals are tested separately commonResult = mockGetterResults{ added: []string{"a1", "a2", "a3"}, removed: []string{"r1", "r2", "r3"}, @@ -303,7 +303,6 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections() { statusUpdater, test.resolver, test.scope, - dps, control.Options{FailFast: test.failFast}, fault.New(test.failFast)) test.expectErr(t, err) @@ -434,10 +433,10 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_repea Category: path.EmailCategory, // doesn't matter which one we use. ResourceOwner: userID, Credentials: suite.creds, + DeltaPaths: graph.DeltaPaths{}, // incrementals are tested separately } statusUpdater = func(*support.ConnectorOperationStatus) {} allScope = selectors.NewExchangeBackup(nil).MailFolders(selectors.Any())[0] - dps = DeltaPaths{} // incrementals are tested separately container1 = mockContainer{ id: strPtr("1"), displayName: strPtr("display_name_1"), @@ -456,7 +455,6 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_repea statusUpdater, resolver, allScope, - dps, control.Options{FailFast: true}, fault.New(true)) require.NoError(t, err) @@ -548,7 +546,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre name string getter mockGetter resolver graph.ContainerResolver - dps DeltaPaths + dps graph.DeltaPaths expect map[string]endState }{ { @@ -561,7 +559,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre displayName: strPtr("new"), p: path.Builder{}.Append("1", "new"), }), - dps: DeltaPaths{}, + dps: graph.DeltaPaths{}, expect: map[string]endState{ "1": {data.NewState, false}, }, @@ -576,10 +574,10 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre displayName: strPtr("not_moved"), p: path.Builder{}.Append("1", "not_moved"), }), - dps: DeltaPaths{ - "1": DeltaPath{ - delta: "old_delta_url", - path: prevPath(suite.T(), "1", "not_moved").String(), + dps: graph.DeltaPaths{ + "1": graph.DeltaPath{ + Delta: "old_delta_url", + Path: prevPath(suite.T(), "1", "not_moved").String(), }, }, expect: map[string]endState{ @@ -596,10 +594,10 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre displayName: strPtr("moved"), p: path.Builder{}.Append("1", "moved"), }), - dps: DeltaPaths{ - "1": DeltaPath{ - delta: "old_delta_url", - path: prevPath(suite.T(), "1", "prev").String(), + dps: graph.DeltaPaths{ + "1": graph.DeltaPath{ + Delta: "old_delta_url", + Path: prevPath(suite.T(), "1", "prev").String(), }, }, expect: map[string]endState{ @@ -610,10 +608,10 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre name: "deleted container", getter: map[string]mockGetterResults{}, resolver: newMockResolver(), - dps: DeltaPaths{ - "1": DeltaPath{ - delta: "old_delta_url", - path: prevPath(suite.T(), "1", "deleted").String(), + dps: graph.DeltaPaths{ + "1": graph.DeltaPath{ + Delta: "old_delta_url", + Path: prevPath(suite.T(), "1", "deleted").String(), }, }, expect: map[string]endState{ @@ -630,10 +628,10 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre displayName: strPtr("new"), p: path.Builder{}.Append("2", "new"), }), - dps: DeltaPaths{ - "1": DeltaPath{ - delta: "old_delta_url", - path: prevPath(suite.T(), "1", "deleted").String(), + dps: graph.DeltaPaths{ + "1": graph.DeltaPath{ + Delta: "old_delta_url", + Path: prevPath(suite.T(), "1", "deleted").String(), }, }, expect: map[string]endState{ @@ -651,10 +649,10 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre displayName: strPtr("same"), p: path.Builder{}.Append("2", "same"), }), - dps: DeltaPaths{ - "1": DeltaPath{ - delta: "old_delta_url", - path: prevPath(suite.T(), "1", "same").String(), + dps: graph.DeltaPaths{ + "1": graph.DeltaPath{ + Delta: "old_delta_url", + Path: prevPath(suite.T(), "1", "same").String(), }, }, expect: map[string]endState{ @@ -680,10 +678,10 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre p: path.Builder{}.Append("2", "prev"), }, ), - dps: DeltaPaths{ - "1": DeltaPath{ - delta: "old_delta_url", - path: prevPath(suite.T(), "1", "prev").String(), + dps: graph.DeltaPaths{ + "1": graph.DeltaPath{ + Delta: "old_delta_url", + Path: prevPath(suite.T(), "1", "prev").String(), }, }, expect: map[string]endState{ @@ -701,14 +699,14 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre displayName: strPtr("not_moved"), p: path.Builder{}.Append("1", "not_moved"), }), - dps: DeltaPaths{ - "1": DeltaPath{ - delta: "old_delta_url", - path: "1/fnords/mc/smarfs", + dps: graph.DeltaPaths{ + "1": graph.DeltaPath{ + Delta: "old_delta_url", + Path: "1/fnords/mc/smarfs", }, - "2": DeltaPath{ - delta: "old_delta_url", - path: "2/fnords/mc/smarfs", + "2": graph.DeltaPath{ + Delta: "old_delta_url", + Path: "2/fnords/mc/smarfs", }, }, expect: map[string]endState{ @@ -725,10 +723,10 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre displayName: strPtr("same"), p: path.Builder{}.Append("1", "same"), }), - dps: DeltaPaths{ - "1": DeltaPath{ - delta: "old_delta_url", - path: prevPath(suite.T(), "1", "same").String(), + dps: graph.DeltaPaths{ + "1": graph.DeltaPath{ + Delta: "old_delta_url", + Path: prevPath(suite.T(), "1", "same").String(), }, }, expect: map[string]endState{ @@ -766,22 +764,22 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre p: path.Builder{}.Append("4", "moved"), }, ), - dps: DeltaPaths{ - "2": DeltaPath{ - delta: "old_delta_url", - path: prevPath(suite.T(), "2", "not_moved").String(), + dps: graph.DeltaPaths{ + "2": graph.DeltaPath{ + Delta: "old_delta_url", + Path: prevPath(suite.T(), "2", "not_moved").String(), }, - "3": DeltaPath{ - delta: "old_delta_url", - path: prevPath(suite.T(), "3", "prev").String(), + "3": graph.DeltaPath{ + Delta: "old_delta_url", + Path: prevPath(suite.T(), "3", "prev").String(), }, - "4": DeltaPath{ - delta: "old_delta_url", - path: prevPath(suite.T(), "4", "prev").String(), + "4": graph.DeltaPath{ + Delta: "old_delta_url", + Path: prevPath(suite.T(), "4", "prev").String(), }, - "5": DeltaPath{ - delta: "old_delta_url", - path: prevPath(suite.T(), "5", "deleted").String(), + "5": graph.DeltaPath{ + Delta: "old_delta_url", + Path: prevPath(suite.T(), "5", "deleted").String(), }, }, expect: map[string]endState{ @@ -799,6 +797,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre defer flush() collections := map[string]data.BackupCollection{} + qp.DeltaPaths = test.dps err := filterContainersAndFillCollections( ctx, @@ -808,7 +807,6 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre statusUpdater, test.resolver, allScope, - test.dps, control.Options{}, fault.New(true)) assert.NoError(t, err) diff --git a/src/internal/connector/graph/query_params.go b/src/internal/connector/graph/query_params.go new file mode 100644 index 000000000..32a473750 --- /dev/null +++ b/src/internal/connector/graph/query_params.go @@ -0,0 +1,66 @@ +package graph + +import ( + "github.com/alcionai/clues" + "github.com/alcionai/corso/src/pkg/account" + "github.com/alcionai/corso/src/pkg/path" +) + +// --------------------------------------------------------------------------- +// query parameter aggregation +// --------------------------------------------------------------------------- + +type QueryParams struct { + Category path.CategoryType + ResourceOwner string + Credentials account.M365Config + DeltaPaths DeltaPaths +} + +func (qp QueryParams) Validate() error { + if qp.Category == path.UnknownCategory { + return clues.New("unknown category") + } + + if len(qp.ResourceOwner) == 0 { + return clues.New("missing resource owner") + } + + return nil +} + +// --------------------------------------------------------------------------- +// delta path aggregation +// +// TODO: probably needs to be owned somewhere besides graph, but for now +// this allows the struct to be centralized and re-used through GC. +// --------------------------------------------------------------------------- + +type CatDeltaPaths map[path.CategoryType]DeltaPaths + +type DeltaPaths map[string]DeltaPath + +func (dps DeltaPaths) AddDelta(k, d string) { + dp, ok := dps[k] + if !ok { + dp = DeltaPath{} + } + + dp.Delta = d + dps[k] = dp +} + +func (dps DeltaPaths) AddPath(k, p string) { + dp, ok := dps[k] + if !ok { + dp = DeltaPath{} + } + + dp.Path = p + dps[k] = dp +} + +type DeltaPath struct { + Delta string + Path string +} diff --git a/src/internal/connector/graph/service.go b/src/internal/connector/graph/service.go index 3d2cacef9..7e5414de2 100644 --- a/src/internal/connector/graph/service.go +++ b/src/internal/connector/graph/service.go @@ -16,9 +16,7 @@ import ( "github.com/pkg/errors" "github.com/alcionai/corso/src/internal/connector/support" - "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/logger" - "github.com/alcionai/corso/src/pkg/path" ) const ( @@ -37,12 +35,6 @@ func AllMetadataFileNames() []string { return []string{DeltaURLsFileName, PreviousPathFileName} } -type QueryParams struct { - Category path.CategoryType - ResourceOwner string - Credentials account.M365Config -} - // --------------------------------------------------------------------------- // Service Handler // ---------------------------------------------------------------------------