From b5527e9ef2bd2abcc4bb2da2627171bb4ad68f13 Mon Sep 17 00:00:00 2001 From: ryanfkeepers Date: Thu, 23 Mar 2023 15:37:53 -0600 Subject: [PATCH] fix circular ref from prior change --- src/cmd/factory/impl/common.go | 2 +- .../connector/data_collections_test.go | 4 +- src/internal/connector/graph_connector.go | 13 ++++-- .../connector/graph_connector_test.go | 40 +++++++++---------- .../mockconnector/mock_data_connector.go | 7 ++-- src/internal/data/metrics.go | 16 ++++++++ src/internal/operations/backup.go | 11 +++-- src/internal/operations/backup_test.go | 11 ++--- src/internal/operations/restore.go | 11 +++-- src/internal/operations/restore_test.go | 15 +++---- 10 files changed, 72 insertions(+), 58 deletions(-) create mode 100644 src/internal/data/metrics.go diff --git a/src/cmd/factory/impl/common.go b/src/cmd/factory/impl/common.go index 6215b58d8..4a8ee1b52 100644 --- a/src/cmd/factory/impl/common.go +++ b/src/cmd/factory/impl/common.go @@ -96,7 +96,7 @@ func generateAndRestoreItems( print.Infof(ctx, "Generating %d %s items in %s\n", howMany, cat, Destination) - return gc.RestoreDataCollections(ctx, version.Backup, acct, sel, dest, opts, dataColls, errs) + return gc.ConsumeRestoreCollections(ctx, version.Backup, acct, sel, dest, opts, dataColls, errs) } // ------------------------------------------------------------------------------------------ diff --git a/src/internal/connector/data_collections_test.go b/src/internal/connector/data_collections_test.go index e17001153..00d16c882 100644 --- a/src/internal/connector/data_collections_test.go +++ b/src/internal/connector/data_collections_test.go @@ -130,7 +130,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestExchangeDataCollection } status := connector.Wait() - assert.NotZero(t, status.Metrics.Successes) + assert.NotZero(t, status.Successes) t.Log(status.String()) }) } @@ -288,7 +288,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestSharePointDataCollecti } status := connector.Wait() - assert.NotZero(t, status.Metrics.Successes) + assert.NotZero(t, status.Successes) t.Log(status.String()) }) } diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index 1fa7a62a7..0e2205496 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -22,6 +22,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/sharepoint" "github.com/alcionai/corso/src/internal/connector/support" + "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/diagnostics" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/fault" @@ -223,7 +224,7 @@ func (gc *GraphConnector) UnionSiteIDsAndWebURLs( } // AwaitStatus waits for all gc tasks to complete and then returns status -func (gc *GraphConnector) Wait() *support.ConnectorOperationStatus { +func (gc *GraphConnector) Wait() data.CollectionStats { defer func() { if gc.region != nil { gc.region.End() @@ -233,12 +234,18 @@ func (gc *GraphConnector) Wait() *support.ConnectorOperationStatus { gc.wg.Wait() // clean up and reset statefulness - status := gc.status + dcs := data.CollectionStats{ + Folders: gc.status.Folders, + Objects: gc.status.Metrics.Objects, + Successes: gc.status.Metrics.Successes, + Bytes: gc.status.Metrics.Bytes, + Details: gc.status.String(), + } gc.wg = &sync.WaitGroup{} gc.status = support.ConnectorOperationStatus{} - return &status + return dcs } // UpdateStatus is used by gc initiated tasks to indicate completion diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 61825c87c..e2b2a9d22 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -241,7 +241,7 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreFailsBadService() { } ) - deets, err := suite.connector.RestoreDataCollections( + deets, err := suite.connector.ConsumeRestoreCollections( ctx, version.Backup, acct, @@ -257,9 +257,9 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreFailsBadService() { assert.NotNil(t, deets) status := suite.connector.Wait() - assert.Equal(t, 0, status.Metrics.Objects) + assert.Equal(t, 0, status.Objects) assert.Equal(t, 0, status.Folders) - assert.Equal(t, 0, status.Metrics.Successes) + assert.Equal(t, 0, status.Successes) } func (suite *GraphConnectorIntegrationSuite) TestEmptyCollections() { @@ -320,7 +320,7 @@ func (suite *GraphConnectorIntegrationSuite) TestEmptyCollections() { ctx, flush := tester.NewContext() defer flush() - deets, err := suite.connector.RestoreDataCollections( + deets, err := suite.connector.ConsumeRestoreCollections( ctx, version.Backup, suite.acct, @@ -336,9 +336,9 @@ func (suite *GraphConnectorIntegrationSuite) TestEmptyCollections() { assert.NotNil(t, deets) stats := suite.connector.Wait() - assert.Zero(t, stats.Metrics.Objects) + assert.Zero(t, stats.Objects) assert.Zero(t, stats.Folders) - assert.Zero(t, stats.Metrics.Successes) + assert.Zero(t, stats.Successes) }) } } @@ -400,7 +400,7 @@ func runRestore( restoreGC := loadConnector(ctx, t, graph.HTTPClient(graph.NoTimeout()), config.resource) restoreSel := getSelectorWith(t, config.service, config.resourceOwners, true) - deets, err := restoreGC.RestoreDataCollections( + deets, err := restoreGC.ConsumeRestoreCollections( ctx, backupVersion, config.acct, @@ -415,8 +415,8 @@ func runRestore( status := restoreGC.Wait() runTime := time.Since(start) - assert.Equal(t, numRestoreItems, status.Metrics.Objects, "restored status.Metrics.Objects") - assert.Equal(t, numRestoreItems, status.Metrics.Successes, "restored status.Metrics.Successes") + assert.Equal(t, numRestoreItems, status.Objects, "restored status.Objects") + assert.Equal(t, numRestoreItems, status.Successes, "restored status.Successes") assert.Len( t, deets.Entries, @@ -484,10 +484,10 @@ func runBackupAndCompare( status := backupGC.Wait() - assert.Equalf(t, totalItems+skipped, status.Metrics.Objects, - "backup status.Metrics.Objects; wanted %d items + %d skipped", totalItems, skipped) - assert.Equalf(t, totalItems+skipped, status.Metrics.Successes, - "backup status.Metrics.Successes; wanted %d items + %d skipped", totalItems, skipped) + assert.Equalf(t, totalItems+skipped, status.Objects, + "backup status.Objects; wanted %d items + %d skipped", totalItems, skipped) + assert.Equalf(t, totalItems+skipped, status.Successes, + "backup status.Successes; wanted %d items + %d skipped", totalItems, skipped) } func runRestoreBackupTest( @@ -966,7 +966,7 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames ) restoreGC := loadConnector(ctx, t, graph.HTTPClient(graph.NoTimeout()), test.resource) - deets, err := restoreGC.RestoreDataCollections( + deets, err := restoreGC.ConsumeRestoreCollections( ctx, version.Backup, suite.acct, @@ -983,10 +983,10 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames status := restoreGC.Wait() // Always just 1 because it's just 1 collection. - assert.Equal(t, totalItems, status.Metrics.Objects, "status.Metrics.Objects") - assert.Equal(t, totalItems, status.Metrics.Successes, "status.Metrics.Successes") - assert.Len( - t, deets.Entries, totalItems, + assert.Equal(t, totalItems, status.Objects, "status.Objects") + assert.Equal(t, totalItems, status.Successes, "status.Successes") + assert.Equal( + t, totalItems, len(deets.Entries), "details entries contains same item count as total successful items restored") t.Log("Restore complete") @@ -1028,8 +1028,8 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames true) status := backupGC.Wait() - assert.Equal(t, allItems+skipped, status.Metrics.Objects, "status.Metrics.Objects") - assert.Equal(t, allItems+skipped, status.Metrics.Successes, "status.Metrics.Successes") + assert.Equal(t, allItems+skipped, status.Objects, "status.Objects") + assert.Equal(t, allItems+skipped, status.Successes, "status.Successes") }) } } diff --git a/src/internal/connector/mockconnector/mock_data_connector.go b/src/internal/connector/mockconnector/mock_data_connector.go index 26b99a5a7..583dd8e32 100644 --- a/src/internal/connector/mockconnector/mock_data_connector.go +++ b/src/internal/connector/mockconnector/mock_data_connector.go @@ -3,7 +3,6 @@ package mockconnector import ( "context" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/backup/details" @@ -20,7 +19,7 @@ type GraphConnector struct { Err error - Status *support.ConnectorOperationStatus + Stats data.CollectionStats } func (gc GraphConnector) ProduceBackupCollections( @@ -38,8 +37,8 @@ func (gc GraphConnector) ProduceBackupCollections( return gc.Collections, gc.Exclude, gc.Err } -func (gc GraphConnector) Wait() *support.ConnectorOperationStatus { - return gc.Status +func (gc GraphConnector) Wait() data.CollectionStats { + return gc.Stats } func (gc GraphConnector) ConsumeRestoreCollections( diff --git a/src/internal/data/metrics.go b/src/internal/data/metrics.go new file mode 100644 index 000000000..ab2547299 --- /dev/null +++ b/src/internal/data/metrics.go @@ -0,0 +1,16 @@ +package data + +type CollectionStats struct { + Folders int + Objects, Successes int + Bytes int64 + Details string +} + +func (cs CollectionStats) IsZero() bool { + return cs.Folders+cs.Objects+cs.Successes+int(cs.Bytes) == 0 +} + +func (cs CollectionStats) String() string { + return cs.Details +} diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index edef54f26..de2988bc4 100644 --- a/src/internal/operations/backup.go +++ b/src/internal/operations/backup.go @@ -9,7 +9,6 @@ import ( "github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/common/crash" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/diagnostics" "github.com/alcionai/corso/src/internal/events" @@ -105,7 +104,7 @@ func (op BackupOperation) validate() error { // get populated asynchronously. type backupStats struct { k *kopia.BackupStats - gc *support.ConnectorOperationStatus + gc data.CollectionStats resourceCount int } @@ -332,7 +331,7 @@ type BackupProducer interface { ) ([]data.BackupCollection, map[string]map[string]struct{}, error) // TODO: ConnectorOperationStatus should be replaced with something // more generic. - Wait() *support.ConnectorOperationStatus + Wait() data.CollectionStats } // calls the producer to generate collections of data to backup @@ -685,16 +684,16 @@ func (op *BackupOperation) persistResults( op.Results.ItemsWritten = opStats.k.TotalFileCount op.Results.ResourceOwners = opStats.resourceCount - if opStats.gc == nil { + if opStats.gc.IsZero() { op.Status = Failed return clues.New("backup population never completed") } - if op.Status != Failed && opStats.gc.Metrics.Successes == 0 { + if op.Status != Failed && opStats.gc.Successes == 0 { op.Status = NoData } - op.Results.ItemsRead = opStats.gc.Metrics.Successes + op.Results.ItemsRead = opStats.gc.Successes return op.Errors.Failure() } diff --git a/src/internal/operations/backup_test.go b/src/internal/operations/backup_test.go index 85b900588..14c087130 100644 --- a/src/internal/operations/backup_test.go +++ b/src/internal/operations/backup_test.go @@ -15,7 +15,6 @@ import ( "github.com/stretchr/testify/suite" "github.com/alcionai/corso/src/internal/connector/mockconnector" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" evmock "github.com/alcionai/corso/src/internal/events/mock" "github.com/alcionai/corso/src/internal/kopia" @@ -381,9 +380,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_PersistResults() { TotalHashedBytes: 1, TotalUploadedBytes: 1, }, - gc: &support.ConnectorOperationStatus{ - Metrics: support.CollectionMetrics{Successes: 1}, - }, + gc: data.CollectionStats{Successes: 1}, }, }, { @@ -392,7 +389,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_PersistResults() { fail: assert.AnError, stats: backupStats{ k: &kopia.BackupStats{}, - gc: &support.ConnectorOperationStatus{}, + gc: data.CollectionStats{}, }, }, { @@ -400,7 +397,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_PersistResults() { expectErr: assert.NoError, stats: backupStats{ k: &kopia.BackupStats{}, - gc: &support.ConnectorOperationStatus{}, + gc: data.CollectionStats{}, }, }, } @@ -427,7 +424,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_PersistResults() { test.expectErr(t, op.persistResults(now, &test.stats)) assert.Equal(t, test.expectStatus.String(), op.Status.String(), "status") - assert.Equal(t, test.stats.gc.Metrics.Successes, op.Results.ItemsRead, "items read") + assert.Equal(t, test.stats.gc.Successes, op.Results.ItemsRead, "items read") assert.Equal(t, test.stats.k.TotalFileCount, op.Results.ItemsWritten, "items written") assert.Equal(t, test.stats.k.TotalHashedBytes, op.Results.BytesRead, "bytes read") assert.Equal(t, test.stats.k.TotalUploadedBytes, op.Results.BytesUploaded, "bytes written") diff --git a/src/internal/operations/restore.go b/src/internal/operations/restore.go index 36f84ffc9..4def27c82 100644 --- a/src/internal/operations/restore.go +++ b/src/internal/operations/restore.go @@ -12,7 +12,6 @@ import ( "github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/common/crash" "github.com/alcionai/corso/src/internal/connector/onedrive" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/diagnostics" "github.com/alcionai/corso/src/internal/events" @@ -94,7 +93,7 @@ func (op RestoreOperation) validate() error { // get populated asynchronously. type restoreStats struct { cs []data.RestoreCollection - gc *support.ConnectorOperationStatus + gc data.CollectionStats bytesRead *stats.ByteCounter resourceCount int @@ -281,16 +280,16 @@ func (op *RestoreOperation) persistResults( op.Results.ItemsRead = len(opStats.cs) // TODO: file count, not collection count op.Results.ResourceOwners = opStats.resourceCount - if opStats.gc == nil { + if opStats.gc.IsZero() { op.Status = Failed return clues.New("restoration never completed") } - if op.Status != Failed && opStats.gc.Metrics.Successes == 0 { + if op.Status != Failed && opStats.gc.Successes == 0 { op.Status = NoData } - op.Results.ItemsWritten = opStats.gc.Metrics.Successes + op.Results.ItemsWritten = opStats.gc.Successes op.bus.Event( ctx, @@ -330,7 +329,7 @@ type RestoreConsumer interface { ) (*details.Details, error) // TODO: ConnectorOperationStatus should be replaced with something // more generic. - Wait() *support.ConnectorOperationStatus + Wait() data.CollectionStats } func consumeRestoreCollections( diff --git a/src/internal/operations/restore_test.go b/src/internal/operations/restore_test.go index 32aee31d8..97f2715f2 100644 --- a/src/internal/operations/restore_test.go +++ b/src/internal/operations/restore_test.go @@ -16,7 +16,6 @@ import ( "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/mockconnector" "github.com/alcionai/corso/src/internal/connector/onedrive/api" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/events" evmock "github.com/alcionai/corso/src/internal/events/mock" @@ -75,11 +74,9 @@ func (suite *RestoreOpSuite) TestRestoreOperation_PersistResults() { Collection: &mockconnector.MockExchangeDataCollection{}, }, }, - gc: &support.ConnectorOperationStatus{ - Metrics: support.CollectionMetrics{ - Objects: 1, - Successes: 1, - }, + gc: data.CollectionStats{ + Objects: 1, + Successes: 1, }, }, }, @@ -89,7 +86,7 @@ func (suite *RestoreOpSuite) TestRestoreOperation_PersistResults() { fail: assert.AnError, stats: restoreStats{ bytesRead: &stats.ByteCounter{}, - gc: &support.ConnectorOperationStatus{}, + gc: data.CollectionStats{}, }, }, { @@ -98,7 +95,7 @@ func (suite *RestoreOpSuite) TestRestoreOperation_PersistResults() { stats: restoreStats{ bytesRead: &stats.ByteCounter{}, cs: []data.RestoreCollection{}, - gc: &support.ConnectorOperationStatus{}, + gc: data.CollectionStats{}, }, }, } @@ -126,7 +123,7 @@ func (suite *RestoreOpSuite) TestRestoreOperation_PersistResults() { assert.Equal(t, test.expectStatus.String(), op.Status.String(), "status") assert.Equal(t, len(test.stats.cs), op.Results.ItemsRead, "items read") - assert.Equal(t, test.stats.gc.Metrics.Successes, op.Results.ItemsWritten, "items written") + assert.Equal(t, test.stats.gc.Successes, op.Results.ItemsWritten, "items written") assert.Equal(t, test.stats.bytesRead.NumBytes, op.Results.BytesRead, "resource owners") assert.Equal(t, test.stats.resourceCount, op.Results.ResourceOwners, "resource owners") assert.Equal(t, now, op.Results.StartedAt, "started at")