diff --git a/src/cli/backup/sharepoint.go b/src/cli/backup/sharepoint.go index fb4f7e766..4eb62dca0 100644 --- a/src/cli/backup/sharepoint.go +++ b/src/cli/backup/sharepoint.go @@ -13,7 +13,6 @@ import ( . "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/cli/utils" "github.com/alcionai/corso/src/internal/connector" - "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/kopia" "github.com/alcionai/corso/src/internal/model" "github.com/alcionai/corso/src/pkg/backup" @@ -210,7 +209,7 @@ func createSharePointCmd(cmd *cobra.Command, args []string) error { defer utils.CloseRepo(ctx, r) - gc, err := connector.NewGraphConnector(ctx, graph.LargeItemClient(), acct, connector.Sites) + gc, err := connector.NewGraphConnector(ctx, acct, connector.Sites) if err != nil { return Only(ctx, errors.Wrap(err, "Failed to connect to Microsoft APIs")) } diff --git a/src/cmd/factory/impl/common.go b/src/cmd/factory/impl/common.go index 585118442..369d80d20 100644 --- a/src/cmd/factory/impl/common.go +++ b/src/cmd/factory/impl/common.go @@ -12,7 +12,6 @@ import ( . "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/connector" - "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/mockconnector" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/pkg/account" @@ -112,7 +111,7 @@ func getGCAndVerifyUser(ctx context.Context, userID string) (*connector.GraphCon } // build a graph connector - gc, err := connector.NewGraphConnector(ctx, graph.LargeItemClient(), acct, connector.Users) + gc, err := connector.NewGraphConnector(ctx, acct, connector.Users) if err != nil { return nil, account.Account{}, errors.Wrap(err, "connecting to graph api") } diff --git a/src/cmd/getM365/getItem.go b/src/cmd/getM365/getItem.go index 24ce81d9a..25961a236 100644 --- a/src/cmd/getM365/getItem.go +++ b/src/cmd/getM365/getItem.go @@ -19,7 +19,6 @@ import ( "github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/connector" "github.com/alcionai/corso/src/internal/connector/exchange/api" - "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/credentials" @@ -178,7 +177,7 @@ func getGC(ctx context.Context) (*connector.GraphConnector, account.M365Config, return nil, m365Cfg, Only(ctx, errors.Wrap(err, "finding m365 account details")) } - gc, err := connector.NewGraphConnector(ctx, graph.LargeItemClient(), acct, connector.Users) + gc, err := connector.NewGraphConnector(ctx, acct, connector.Users) if err != nil { return nil, m365Cfg, Only(ctx, errors.Wrap(err, "connecting to graph API")) } diff --git a/src/cmd/purge/purge.go b/src/cmd/purge/purge.go index 32100772d..a59f7c2b8 100644 --- a/src/cmd/purge/purge.go +++ b/src/cmd/purge/purge.go @@ -255,7 +255,7 @@ func getGC(ctx context.Context) (*connector.GraphConnector, error) { } // build a graph connector - gc, err := connector.NewGraphConnector(ctx, graph.LargeItemClient(), acct, connector.Users) + gc, err := connector.NewGraphConnector(ctx, acct, connector.Users) if err != nil { return nil, Only(ctx, errors.Wrap(err, "connecting to graph api")) } diff --git a/src/internal/connector/data_collections.go b/src/internal/connector/data_collections.go index 0b6d20b27..7eaf1c517 100644 --- a/src/internal/connector/data_collections.go +++ b/src/internal/connector/data_collections.go @@ -87,7 +87,6 @@ func (gc *GraphConnector) DataCollections( case selectors.ServiceSharePoint: colls, err := sharepoint.DataCollections( ctx, - gc.itemClient, sels, gc.credentials.AzureTenantID, gc.Service, @@ -199,7 +198,6 @@ func (gc *GraphConnector) OneDriveDataCollections( logger.Ctx(ctx).With("user", user).Debug("Creating OneDrive collections") odcs, err := onedrive.NewCollections( - gc.itemClient, gc.credentials.AzureTenantID, user, onedrive.OneDriveSource, diff --git a/src/internal/connector/data_collections_test.go b/src/internal/connector/data_collections_test.go index 57332ce1a..e16aa4b51 100644 --- a/src/internal/connector/data_collections_test.go +++ b/src/internal/connector/data_collections_test.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/suite" "github.com/alcionai/corso/src/internal/connector/exchange" - "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/sharepoint" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/control" @@ -44,7 +43,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) SetupSuite() { tester.MustGetEnvVars(suite.T(), tester.M365AcctCredEnvs...) - suite.connector = loadConnector(ctx, suite.T(), graph.LargeItemClient(), AllResources) + suite.connector = loadConnector(ctx, suite.T(), AllResources) suite.user = tester.M365UserID(suite.T()) suite.site = tester.M365SiteID(suite.T()) @@ -63,7 +62,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestExchangeDataCollection selUsers := []string{suite.user} - connector := loadConnector(ctx, suite.T(), graph.LargeItemClient(), Users) + connector := loadConnector(ctx, suite.T(), Users) tests := []struct { name string getSelector func(t *testing.T) selectors.Selector @@ -139,7 +138,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestDataCollections_invali owners := []string{"snuffleupagus"} - connector := loadConnector(ctx, suite.T(), graph.LargeItemClient(), Users) + connector := loadConnector(ctx, suite.T(), Users) tests := []struct { name string getSelector func(t *testing.T) selectors.Selector @@ -215,7 +214,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestSharePointDataCollecti selSites := []string{suite.site} - connector := loadConnector(ctx, suite.T(), graph.LargeItemClient(), Sites) + connector := loadConnector(ctx, suite.T(), Sites) tests := []struct { name string expected int @@ -244,7 +243,6 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestSharePointDataCollecti suite.T().Run(test.name, func(t *testing.T) { collections, err := sharepoint.DataCollections( ctx, - graph.LargeItemClient(), test.getSelector(), connector.credentials.AzureTenantID, connector.Service, @@ -300,7 +298,7 @@ func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) SetupSuite() { tester.MustGetEnvSets(suite.T(), tester.M365AcctCredEnvs) - suite.connector = loadConnector(ctx, suite.T(), graph.LargeItemClient(), Sites) + suite.connector = loadConnector(ctx, suite.T(), Sites) suite.user = tester.M365UserID(suite.T()) tester.LogTimeOfTest(suite.T()) @@ -313,7 +311,7 @@ func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) TestCreateShar var ( t = suite.T() siteID = tester.M365SiteID(t) - gc = loadConnector(ctx, t, graph.LargeItemClient(), Sites) + gc = loadConnector(ctx, t, Sites) siteIDs = []string{siteID} ) @@ -337,7 +335,7 @@ func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) TestCreateShar var ( t = suite.T() siteID = tester.M365SiteID(t) - gc = loadConnector(ctx, t, graph.LargeItemClient(), Sites) + gc = loadConnector(ctx, t, Sites) siteIDs = []string{siteID} ) diff --git a/src/internal/connector/graph/service_helper.go b/src/internal/connector/graph/service_helper.go index a69183a79..b374933f8 100644 --- a/src/internal/connector/graph/service_helper.go +++ b/src/internal/connector/graph/service_helper.go @@ -55,31 +55,6 @@ func CreateHTTPClient() *http.Client { return httpClient } -// LargeItemClient generates a client that's configured to handle -// large file downloads. This client isn't suitable for other queries -// due to loose restrictions on timeouts and such. -// -// Re-use of http clients is critical, or else we leak os resources -// and consume relatively unbound socket connections. It is important -// to centralize this client to be passed downstream where api calls -// can utilize it on a per-download basis. -// -// TODO: this should get owned by an API client layer, not the GC itself. -func LargeItemClient() *http.Client { - httpClient := CreateHTTPClient() - httpClient.Timeout = 0 // infinite timeout for pulling large files - httpClient.Transport = &http.Transport{ - Proxy: http.ProxyFromEnvironment, - ForceAttemptHTTP2: true, - MaxIdleConns: 100, - IdleConnTimeout: 30 * time.Second, - TLSHandshakeTimeout: 10 * time.Second, - ExpectContinueTimeout: 1 * time.Second, - } - - return httpClient -} - // --------------------------------------------------------------------------- // Logging Middleware // --------------------------------------------------------------------------- @@ -110,10 +85,6 @@ func (handler *LoggingMiddleware) Intercept( logger.Ctx(ctx).Infow("graph api throttling", "method", req.Method, "url", req.URL) } - if resp.StatusCode != http.StatusTooManyRequests && (resp.StatusCode/100) != 2 { - logger.Ctx(ctx).Infow("graph api error", "method", req.Method, "url", req.URL) - } - if logger.DebugAPI || os.Getenv(logGraphRequestsEnvKey) != "" { respDump, _ := httputil.DumpResponse(resp, true) diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index 3dbc0e60c..68f03d048 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -4,7 +4,6 @@ package connector import ( "context" - "net/http" "runtime/trace" "strings" "sync" @@ -39,9 +38,8 @@ import ( // GraphRequestAdapter from the msgraph-sdk-go. Additional fields are for // bookkeeping and interfacing with other component. type GraphConnector struct { - Service graph.Servicer - Owners api.Client - itemClient *http.Client // configured to handle large item downloads + Service graph.Servicer + Owners api.Client tenant string Users map[string]string // key value @@ -66,19 +64,13 @@ const ( Sites ) -func NewGraphConnector( - ctx context.Context, - itemClient *http.Client, - acct account.Account, - r resource, -) (*GraphConnector, error) { +func NewGraphConnector(ctx context.Context, acct account.Account, r resource) (*GraphConnector, error) { m365, err := acct.M365Config() if err != nil { return nil, errors.Wrap(err, "retrieving m365 account configuration") } gc := GraphConnector{ - itemClient: itemClient, tenant: m365.AzureTenantID, Users: make(map[string]string, 0), wg: &sync.WaitGroup{}, diff --git a/src/internal/connector/graph_connector_disconnected_test.go b/src/internal/connector/graph_connector_disconnected_test.go index 711e55ff8..f7f583ebd 100644 --- a/src/internal/connector/graph_connector_disconnected_test.go +++ b/src/internal/connector/graph_connector_disconnected_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/account" @@ -66,7 +65,7 @@ func (suite *DisconnectedGraphConnectorSuite) TestBadConnection() { for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { - gc, err := NewGraphConnector(ctx, graph.LargeItemClient(), test.acct(t), Users) + gc, err := NewGraphConnector(ctx, test.acct(t), Users) assert.Nil(t, gc, test.name+" failed") assert.NotNil(t, err, test.name+"failed") }) diff --git a/src/internal/connector/graph_connector_helper_test.go b/src/internal/connector/graph_connector_helper_test.go index ce16a5a4d..c614df05d 100644 --- a/src/internal/connector/graph_connector_helper_test.go +++ b/src/internal/connector/graph_connector_helper_test.go @@ -3,7 +3,6 @@ package connector import ( "context" "io" - "net/http" "reflect" "testing" @@ -977,9 +976,9 @@ func getSelectorWith( } } -func loadConnector(ctx context.Context, t *testing.T, itemClient *http.Client, r resource) *GraphConnector { +func loadConnector(ctx context.Context, t *testing.T, r resource) *GraphConnector { a := tester.NewM365Account(t) - connector, err := NewGraphConnector(ctx, itemClient, a, r) + connector, err := NewGraphConnector(ctx, a, r) require.NoError(t, err) return connector diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 85ad3f45f..71eff095a 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -156,7 +156,7 @@ func (suite *GraphConnectorIntegrationSuite) SetupSuite() { tester.MustGetEnvSets(suite.T(), tester.M365AcctCredEnvs) - suite.connector = loadConnector(ctx, suite.T(), graph.LargeItemClient(), Users) + suite.connector = loadConnector(ctx, suite.T(), Users) suite.user = tester.M365UserID(suite.T()) suite.acct = tester.NewM365Account(suite.T()) @@ -380,7 +380,7 @@ func runRestoreBackupTest( start := time.Now() - restoreGC := loadConnector(ctx, t, graph.LargeItemClient(), test.resource) + restoreGC := loadConnector(ctx, t, test.resource) restoreSel := getSelectorWith(t, test.service, resourceOwners, true) deets, err := restoreGC.RestoreDataCollections( ctx, @@ -419,7 +419,7 @@ func runRestoreBackupTest( }) } - backupGC := loadConnector(ctx, t, graph.LargeItemClient(), test.resource) + backupGC := loadConnector(ctx, t, test.resource) backupSel := backupSelectorForExpected(t, test.service, expectedDests) t.Logf("Selective backup of %s\n", backupSel) @@ -870,7 +870,7 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames dest.ContainerName, ) - restoreGC := loadConnector(ctx, t, graph.LargeItemClient(), test.resource) + restoreGC := loadConnector(ctx, t, test.resource) deets, err := restoreGC.RestoreDataCollections(ctx, suite.acct, restoreSel, dest, collections) require.NoError(t, err) require.NotNil(t, deets) @@ -888,7 +888,7 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames // Run a backup and compare its output with what we put in. - backupGC := loadConnector(ctx, t, graph.LargeItemClient(), test.resource) + backupGC := loadConnector(ctx, t, test.resource) backupSel := backupSelectorForExpected(t, test.service, expectedDests) t.Log("Selective backup of", backupSel) diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go index ac0aa9fb3..4ea9ea9eb 100644 --- a/src/internal/connector/onedrive/collection.go +++ b/src/internal/connector/onedrive/collection.go @@ -4,13 +4,11 @@ package onedrive import ( "context" "io" - "net/http" "sync" "sync/atomic" "time" "github.com/microsoftgraph/msgraph-sdk-go/models" - "github.com/pkg/errors" "github.com/spatialcurrent/go-lazy/pkg/lazy" "github.com/alcionai/corso/src/internal/connector/graph" @@ -45,9 +43,6 @@ var ( // Collection represents a set of OneDrive objects retrieved from M365 type Collection struct { - // configured to handle large item downloads - itemClient *http.Client - // data is used to share data streams with the collection consumer data chan data.Stream // folderPath indicates what level in the hierarchy this collection @@ -69,13 +64,12 @@ type Collection struct { // itemReadFunc returns a reader for the specified item type itemReaderFunc func( - hc *http.Client, + ctx context.Context, item models.DriveItemable, ) (itemInfo details.ItemInfo, itemData io.ReadCloser, err error) // NewCollection creates a Collection func NewCollection( - itemClient *http.Client, folderPath path.Path, driveID string, service graph.Servicer, @@ -84,7 +78,6 @@ func NewCollection( ctrlOpts control.Options, ) *Collection { c := &Collection{ - itemClient: itemClient, folderPath: folderPath, driveItems: map[string]models.DriveItemable{}, driveID: driveID, @@ -205,16 +198,11 @@ func (oc *Collection) populateItems(ctx context.Context) { m.Unlock() } - for id, item := range oc.driveItems { + for _, item := range oc.driveItems { if oc.ctrl.FailFast && errs != nil { break } - if item == nil { - errUpdater(id, errors.New("nil item")) - continue - } - semaphoreCh <- struct{}{} wg.Add(1) @@ -231,9 +219,10 @@ func (oc *Collection) populateItems(ctx context.Context) { ) for i := 1; i <= maxRetries; i++ { - itemInfo, itemData, err = oc.itemReader(oc.itemClient, item) + itemInfo, itemData, err = oc.itemReader(ctx, item) + + // retry on Timeout type errors, break otherwise. if err == nil || graph.IsErrTimeout(err) == nil { - // retry on Timeout type errors, break otherwise. break } diff --git a/src/internal/connector/onedrive/collection_test.go b/src/internal/connector/onedrive/collection_test.go index a36db58c9..a19021ff7 100644 --- a/src/internal/connector/onedrive/collection_test.go +++ b/src/internal/connector/onedrive/collection_test.go @@ -2,8 +2,9 @@ package onedrive import ( "bytes" + "context" + "errors" "io" - "net/http" "sync" "testing" "time" @@ -14,7 +15,6 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/pkg/backup/details" @@ -73,7 +73,7 @@ func (suite *CollectionUnitTestSuite) TestCollection() { name: "oneDrive, no duplicates", numInstances: 1, source: OneDriveSource, - itemReader: func(*http.Client, models.DriveItemable) (details.ItemInfo, io.ReadCloser, error) { + itemReader: func(context.Context, models.DriveItemable) (details.ItemInfo, io.ReadCloser, error) { return details.ItemInfo{OneDrive: &details.OneDriveInfo{ItemName: testItemName, Modified: now}}, io.NopCloser(bytes.NewReader(testItemData)), nil @@ -87,7 +87,7 @@ func (suite *CollectionUnitTestSuite) TestCollection() { name: "oneDrive, duplicates", numInstances: 3, source: OneDriveSource, - itemReader: func(*http.Client, models.DriveItemable) (details.ItemInfo, io.ReadCloser, error) { + itemReader: func(context.Context, models.DriveItemable) (details.ItemInfo, io.ReadCloser, error) { return details.ItemInfo{OneDrive: &details.OneDriveInfo{ItemName: testItemName, Modified: now}}, io.NopCloser(bytes.NewReader(testItemData)), nil @@ -101,7 +101,7 @@ func (suite *CollectionUnitTestSuite) TestCollection() { name: "sharePoint, no duplicates", numInstances: 1, source: SharePointSource, - itemReader: func(*http.Client, models.DriveItemable) (details.ItemInfo, io.ReadCloser, error) { + itemReader: func(context.Context, models.DriveItemable) (details.ItemInfo, io.ReadCloser, error) { return details.ItemInfo{SharePoint: &details.SharePointInfo{ItemName: testItemName, Modified: now}}, io.NopCloser(bytes.NewReader(testItemData)), nil @@ -115,7 +115,7 @@ func (suite *CollectionUnitTestSuite) TestCollection() { name: "sharePoint, duplicates", numInstances: 3, source: SharePointSource, - itemReader: func(*http.Client, models.DriveItemable) (details.ItemInfo, io.ReadCloser, error) { + itemReader: func(context.Context, models.DriveItemable) (details.ItemInfo, io.ReadCloser, error) { return details.ItemInfo{SharePoint: &details.SharePointInfo{ItemName: testItemName, Modified: now}}, io.NopCloser(bytes.NewReader(testItemData)), nil @@ -140,7 +140,6 @@ func (suite *CollectionUnitTestSuite) TestCollection() { require.NoError(t, err) coll := NewCollection( - graph.LargeItemClient(), folderPath, "drive-id", suite, @@ -225,7 +224,6 @@ func (suite *CollectionUnitTestSuite) TestCollectionReadError() { require.NoError(t, err) coll := NewCollection( - graph.LargeItemClient(), folderPath, "fakeDriveID", suite, @@ -237,8 +235,10 @@ func (suite *CollectionUnitTestSuite) TestCollectionReadError() { mockItem.SetId(&testItemID) coll.Add(mockItem) - coll.itemReader = func(*http.Client, models.DriveItemable) (details.ItemInfo, io.ReadCloser, error) { - return details.ItemInfo{}, nil, assert.AnError + readError := errors.New("Test error") + + coll.itemReader = func(context.Context, models.DriveItemable) (details.ItemInfo, io.ReadCloser, error) { + return details.ItemInfo{}, nil, readError } coll.Items() diff --git a/src/internal/connector/onedrive/collections.go b/src/internal/connector/onedrive/collections.go index f446aa246..8f18aa780 100644 --- a/src/internal/connector/onedrive/collections.go +++ b/src/internal/connector/onedrive/collections.go @@ -3,7 +3,6 @@ package onedrive import ( "context" "fmt" - "net/http" "strings" "github.com/microsoftgraph/msgraph-sdk-go/models" @@ -47,9 +46,6 @@ type folderMatcher interface { // Collections is used to retrieve drive data for a // resource owner, which can be either a user or a sharepoint site. type Collections struct { - // configured to handle large item downloads - itemClient *http.Client - tenant string resourceOwner string source driveSource @@ -70,7 +66,6 @@ type Collections struct { } func NewCollections( - itemClient *http.Client, tenant string, resourceOwner string, source driveSource, @@ -80,7 +75,6 @@ func NewCollections( ctrlOpts control.Options, ) *Collections { return &Collections{ - itemClient: itemClient, tenant: tenant, resourceOwner: resourceOwner, source: source, @@ -271,13 +265,13 @@ func (c *Collections) UpdateCollections( // TODO(ashmrtn): Compare old and new path and set collection state // accordingly. col = NewCollection( - c.itemClient, collectionPath, driveID, c.service, c.statusUpdater, c.source, - c.ctrl) + c.ctrl, + ) c.CollectionMap[collectionPath.String()] = col c.NumContainers++ diff --git a/src/internal/connector/onedrive/collections_test.go b/src/internal/connector/onedrive/collections_test.go index 21ca061a9..485b9ed86 100644 --- a/src/internal/connector/onedrive/collections_test.go +++ b/src/internal/connector/onedrive/collections_test.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/suite" "golang.org/x/exp/maps" - "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/selectors" @@ -588,7 +587,6 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { outputFolderMap := map[string]string{} maps.Copy(outputFolderMap, tt.inputFolderMap) c := NewCollections( - graph.LargeItemClient(), tenant, user, OneDriveSource, diff --git a/src/internal/connector/onedrive/drive_test.go b/src/internal/connector/onedrive/drive_test.go index 755b7293b..7f6901621 100644 --- a/src/internal/connector/onedrive/drive_test.go +++ b/src/internal/connector/onedrive/drive_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/suite" "github.com/alcionai/corso/src/internal/common" - "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/logger" @@ -147,7 +146,6 @@ func (suite *OneDriveSuite) TestOneDriveNewCollections() { NewOneDriveBackup([]string{test.user}). AllData()[0] odcs, err := NewCollections( - graph.LargeItemClient(), creds.AzureTenantID, test.user, OneDriveSource, diff --git a/src/internal/connector/onedrive/item.go b/src/internal/connector/onedrive/item.go index 00ec1f630..73391033b 100644 --- a/src/internal/connector/onedrive/item.go +++ b/src/internal/connector/onedrive/item.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "io" - "net/http" "strings" msdrives "github.com/microsoftgraph/msgraph-sdk-go/drives" @@ -28,7 +27,7 @@ const ( // It crafts this by querying M365 for a download URL for the item // and using a http client to initialize a reader func sharePointItemReader( - hc *http.Client, + ctx context.Context, item models.DriveItemable, ) (details.ItemInfo, io.ReadCloser, error) { url, ok := item.GetAdditionalData()[downloadURLKey].(*string) @@ -36,7 +35,7 @@ func sharePointItemReader( return details.ItemInfo{}, nil, fmt.Errorf("failed to get url for %s", *item.GetName()) } - resp, err := hc.Get(*url) + rc, err := driveItemReader(ctx, *url) if err != nil { return details.ItemInfo{}, nil, err } @@ -45,14 +44,14 @@ func sharePointItemReader( SharePoint: sharePointItemInfo(item, *item.GetSize()), } - return dii, resp.Body, nil + return dii, rc, nil } // oneDriveItemReader will return a io.ReadCloser for the specified item // It crafts this by querying M365 for a download URL for the item // and using a http client to initialize a reader func oneDriveItemReader( - hc *http.Client, + ctx context.Context, item models.DriveItemable, ) (details.ItemInfo, io.ReadCloser, error) { url, ok := item.GetAdditionalData()[downloadURLKey].(*string) @@ -60,7 +59,7 @@ func oneDriveItemReader( return details.ItemInfo{}, nil, fmt.Errorf("failed to get url for %s", *item.GetName()) } - resp, err := hc.Get(*url) + rc, err := driveItemReader(ctx, *url) if err != nil { return details.ItemInfo{}, nil, err } @@ -69,7 +68,25 @@ func oneDriveItemReader( OneDrive: oneDriveItemInfo(item, *item.GetSize()), } - return dii, resp.Body, nil + return dii, rc, nil +} + +// driveItemReader will return a io.ReadCloser for the specified item +// It crafts this by querying M365 for a download URL for the item +// and using a http client to initialize a reader +func driveItemReader( + ctx context.Context, + url string, +) (io.ReadCloser, error) { + httpClient := graph.CreateHTTPClient() + httpClient.Timeout = 0 // infinite timeout for pulling large files + + resp, err := httpClient.Get(url) + if err != nil { + return nil, errors.Wrapf(err, "failed to download file from %s", url) + } + + return resp.Body, nil } // oneDriveItemInfo will populate a details.OneDriveInfo struct @@ -80,7 +97,7 @@ func oneDriveItemReader( func oneDriveItemInfo(di models.DriveItemable, itemSize int64) *details.OneDriveInfo { var email, parent string - if di.GetCreatedBy() != nil && di.GetCreatedBy().GetUser() != nil { + if di.GetCreatedBy().GetUser() != nil { // User is sometimes not available when created via some // external applications (like backup/restore solutions) ed, ok := di.GetCreatedBy().GetUser().GetAdditionalData()["email"] @@ -89,9 +106,11 @@ func oneDriveItemInfo(di models.DriveItemable, itemSize int64) *details.OneDrive } } - if di.GetParentReference() != nil && di.GetParentReference().GetName() != nil { - // EndPoint is not always populated from external apps - parent = *di.GetParentReference().GetName() + if di.GetParentReference() != nil { + if di.GetParentReference().GetName() != nil { + // EndPoint is not always populated from external apps + parent = *di.GetParentReference().GetName() + } } return &details.OneDriveInfo{ diff --git a/src/internal/connector/onedrive/item_test.go b/src/internal/connector/onedrive/item_test.go index 2b28f0910..b3f45ca56 100644 --- a/src/internal/connector/onedrive/item_test.go +++ b/src/internal/connector/onedrive/item_test.go @@ -126,7 +126,7 @@ func (suite *ItemIntegrationSuite) TestItemReader_oneDrive() { // Read data for the file - itemInfo, itemData, err := oneDriveItemReader(graph.LargeItemClient(), driveItem) + itemInfo, itemData, err := oneDriveItemReader(ctx, driveItem) require.NoError(suite.T(), err) require.NotNil(suite.T(), itemInfo.OneDrive) require.NotEmpty(suite.T(), itemInfo.OneDrive.ItemName) diff --git a/src/internal/connector/sharepoint/data_collections.go b/src/internal/connector/sharepoint/data_collections.go index 5950daede..d7c6547a0 100644 --- a/src/internal/connector/sharepoint/data_collections.go +++ b/src/internal/connector/sharepoint/data_collections.go @@ -2,7 +2,6 @@ package sharepoint import ( "context" - "net/http" "github.com/pkg/errors" @@ -25,7 +24,6 @@ type statusUpdater interface { // for the specified user func DataCollections( ctx context.Context, - itemClient *http.Client, selector selectors.Selector, tenantID string, serv graph.Servicer, @@ -68,7 +66,6 @@ func DataCollections( case path.LibrariesCategory: spcs, err = collectLibraries( ctx, - itemClient, serv, tenantID, site, @@ -127,7 +124,6 @@ func collectLists( // all the drives associated with the site. func collectLibraries( ctx context.Context, - itemClient *http.Client, serv graph.Servicer, tenantID, siteID string, scope selectors.SharePointScope, @@ -142,7 +138,6 @@ func collectLibraries( logger.Ctx(ctx).With("site", siteID).Debug("Creating SharePoint Library collections") colls := onedrive.NewCollections( - itemClient, tenantID, siteID, onedrive.SharePointSource, diff --git a/src/internal/connector/sharepoint/data_collections_test.go b/src/internal/connector/sharepoint/data_collections_test.go index 4daca0877..423336253 100644 --- a/src/internal/connector/sharepoint/data_collections_test.go +++ b/src/internal/connector/sharepoint/data_collections_test.go @@ -7,7 +7,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" - "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/onedrive" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/control" @@ -92,7 +91,6 @@ func (suite *SharePointLibrariesSuite) TestUpdateCollections() { newPaths := map[string]string{} excluded := map[string]struct{}{} c := onedrive.NewCollections( - graph.LargeItemClient(), tenant, site, onedrive.SharePointSource, diff --git a/src/internal/operations/backup_integration_test.go b/src/internal/operations/backup_integration_test.go index 83748baa2..3ee5c0230 100644 --- a/src/internal/operations/backup_integration_test.go +++ b/src/internal/operations/backup_integration_test.go @@ -655,7 +655,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { m365, err := acct.M365Config() require.NoError(t, err) - gc, err := connector.NewGraphConnector(ctx, graph.LargeItemClient(), acct, connector.Users) + gc, err := connector.NewGraphConnector(ctx, acct, connector.Users) require.NoError(t, err) ac, err := api.NewClient(m365) diff --git a/src/internal/operations/operation.go b/src/internal/operations/operation.go index e5382b022..30770bdf5 100644 --- a/src/internal/operations/operation.go +++ b/src/internal/operations/operation.go @@ -7,7 +7,6 @@ import ( "github.com/pkg/errors" "github.com/alcionai/corso/src/internal/connector" - "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/events" "github.com/alcionai/corso/src/internal/kopia" "github.com/alcionai/corso/src/internal/observe" @@ -108,7 +107,7 @@ func connectToM365( resource = connector.Sites } - gc, err := connector.NewGraphConnector(ctx, graph.LargeItemClient(), acct, resource) + gc, err := connector.NewGraphConnector(ctx, acct, resource) if err != nil { return nil, err } diff --git a/src/pkg/services/m365/m365.go b/src/pkg/services/m365/m365.go index e0dd75af9..ba9cc2a59 100644 --- a/src/pkg/services/m365/m365.go +++ b/src/pkg/services/m365/m365.go @@ -8,7 +8,6 @@ import ( "github.com/alcionai/corso/src/internal/connector" "github.com/alcionai/corso/src/internal/connector/discovery" - "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/pkg/account" ) @@ -21,7 +20,7 @@ type User struct { // Users returns a list of users in the specified M365 tenant // TODO: Implement paging support func Users(ctx context.Context, m365Account account.Account) ([]*User, error) { - gc, err := connector.NewGraphConnector(ctx, graph.LargeItemClient(), m365Account, connector.Users) + gc, err := connector.NewGraphConnector(ctx, m365Account, connector.Users) if err != nil { return nil, errors.Wrap(err, "could not initialize M365 graph connection") } @@ -77,7 +76,7 @@ func UserPNs(ctx context.Context, m365Account account.Account) ([]string, error) // SiteURLs returns a list of SharePoint site WebURLs in the specified M365 tenant func SiteURLs(ctx context.Context, m365Account account.Account) ([]string, error) { - gc, err := connector.NewGraphConnector(ctx, graph.LargeItemClient(), m365Account, connector.Sites) + gc, err := connector.NewGraphConnector(ctx, m365Account, connector.Sites) if err != nil { return nil, errors.Wrap(err, "could not initialize M365 graph connection") } @@ -87,7 +86,7 @@ func SiteURLs(ctx context.Context, m365Account account.Account) ([]string, error // SiteURLs returns a list of SharePoint sites IDs in the specified M365 tenant func SiteIDs(ctx context.Context, m365Account account.Account) ([]string, error) { - gc, err := connector.NewGraphConnector(ctx, graph.LargeItemClient(), m365Account, connector.Sites) + gc, err := connector.NewGraphConnector(ctx, m365Account, connector.Sites) if err != nil { return nil, errors.Wrap(err, "could not initialize M365 graph connection") }