From 0e6ae32fc3f0bdde7aaeb476302c20cd32d3afe2 Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Tue, 13 Sep 2022 14:48:03 -0700 Subject: [PATCH] Add OneDrive path base structs (#833) * Constants for OneDrive stuff * Tests and constructor for OneDrive paths * Populate onedrive path struct in data collection (#835) * Helper function to make path structs for onedrive * Use path struct in onedrive data collection Does not change the external API at all, just the internals of how FullPath functions and what is stored for the path. * Wire up making data collections with path struct Requires addition of tenant as input to Collections(). * Fixup onedrive Collections tests * Wire up call to onedrive.NewCollections() Just requires adding the tenant ID to the call. --- src/internal/connector/graph_connector.go | 7 +- src/internal/connector/onedrive/collection.go | 17 ++-- .../connector/onedrive/collection_test.go | 45 +++++++---- .../connector/onedrive/collections.go | 64 ++++++++++++--- .../connector/onedrive/collections_test.go | 78 ++++++++++++++----- src/internal/path/categorytype_string.go | 5 +- src/internal/path/path.go | 21 +++++ src/internal/path/resource_path.go | 9 +++ src/internal/path/resource_path_test.go | 33 ++++++-- src/internal/path/service_category_test.go | 8 ++ src/internal/path/servicetype_string.go | 5 +- 11 files changed, 228 insertions(+), 64 deletions(-) diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index 6cef311a3..cab564563 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -463,7 +463,12 @@ func (gc *GraphConnector) OneDriveDataCollections( for _, user := range scope.Get(selectors.OneDriveUser) { logger.Ctx(ctx).With("user", user).Debug("Creating OneDrive collections") - odcs, err := onedrive.NewCollections(user, &gc.graphService, gc.UpdateStatus).Get(ctx) + odcs, err := onedrive.NewCollections( + gc.credentials.TenantID, + user, + &gc.graphService, + gc.UpdateStatus, + ).Get(ctx) if err != nil { return nil, support.WrapAndAppend(user, err, errs) } diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go index 2c3488f05..fa9b8871e 100644 --- a/src/internal/connector/onedrive/collection.go +++ b/src/internal/connector/onedrive/collection.go @@ -9,6 +9,7 @@ import ( "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/internal/path" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/logger" ) @@ -30,7 +31,7 @@ type Collection struct { data chan data.Stream // folderPath indicates what level in the hierarchy this collection // represents - folderPath string + folderPath path.Path // M365 IDs of file items within this collection driveItemIDs []string // M365 ID of the drive this collection was created from @@ -48,7 +49,10 @@ type itemReaderFunc func( ) (name string, itemData io.ReadCloser, err error) // NewCollection creates a Collection -func NewCollection(folderPath, driveID string, service graph.Service, +func NewCollection( + folderPath path.Path, + driveID string, + service graph.Service, statusUpdater support.StatusUpdater, ) *Collection { c := &Collection{ @@ -77,10 +81,9 @@ func (oc *Collection) Items() <-chan data.Stream { } func (oc *Collection) FullPath() []string { - path := oc.folderPath - // Remove leading `/` if any so that Split - // doesn't return a "" - return strings.Split(strings.TrimPrefix(path, "/"), "/") + // TODO(ashmrtn): Update this when data.Collection.FullPath has support for + // path.Path. + return strings.Split(oc.folderPath.String(), "/") } // Item represents a single item retrieved from OneDrive @@ -125,7 +128,7 @@ func (oc *Collection) populateItems(ctx context.Context) { info: &details.OneDriveInfo{ ItemType: details.OneDriveItem, ItemName: itemName, - ParentPath: oc.folderPath, + ParentPath: oc.folderPath.String(), }, } } diff --git a/src/internal/connector/onedrive/collection_test.go b/src/internal/connector/onedrive/collection_test.go index 8cb0a64bb..8c9b2c55a 100644 --- a/src/internal/connector/onedrive/collection_test.go +++ b/src/internal/connector/onedrive/collection_test.go @@ -5,6 +5,7 @@ import ( "context" "errors" "io" + "strings" "sync" "testing" @@ -54,12 +55,20 @@ func (suite *OneDriveCollectionSuite) testStatusUpdater( } func (suite *OneDriveCollectionSuite) TestOneDriveCollection() { + t := suite.T() wg := sync.WaitGroup{} collStatus := support.ConnectorOperationStatus{} - folderPath := "dir1/dir2/dir3" + + folderPath, err := getCanonicalPath("dir1/dir2/dir3", "a-tenant", "a-user") + require.NoError(t, err) + coll := NewCollection(folderPath, "fakeDriveID", suite, suite.testStatusUpdater(&wg, &collStatus)) - require.NotNil(suite.T(), coll) - assert.Equal(suite.T(), []string{"dir1", "dir2", "dir3"}, coll.FullPath()) + require.NotNil(t, coll) + assert.Equal( + t, + strings.Split(folderPath.String(), "/"), + coll.FullPath(), + ) testItemID := "fakeItemID" testItemName := "itemName" @@ -80,31 +89,35 @@ func (suite *OneDriveCollectionSuite) TestOneDriveCollection() { } wg.Wait() // Expect only 1 item - require.Len(suite.T(), readItems, 1) - require.Equal(suite.T(), 1, collStatus.ObjectCount) - require.Equal(suite.T(), 1, collStatus.Successful) + require.Len(t, readItems, 1) + require.Equal(t, 1, collStatus.ObjectCount) + require.Equal(t, 1, collStatus.Successful) // Validate item info and data readItem := readItems[0] readItemInfo := readItem.(data.StreamInfo) - assert.Equal(suite.T(), testItemID, readItem.UUID()) + assert.Equal(t, testItemID, readItem.UUID()) readData, err := io.ReadAll(readItem.ToReader()) - require.NoError(suite.T(), err) + require.NoError(t, err) - assert.Equal(suite.T(), testItemData, readData) - require.NotNil(suite.T(), readItemInfo.Info()) - require.NotNil(suite.T(), readItemInfo.Info().OneDrive) - assert.Equal(suite.T(), testItemName, readItemInfo.Info().OneDrive.ItemName) - assert.Equal(suite.T(), folderPath, readItemInfo.Info().OneDrive.ParentPath) + assert.Equal(t, testItemData, readData) + require.NotNil(t, readItemInfo.Info()) + require.NotNil(t, readItemInfo.Info().OneDrive) + assert.Equal(t, testItemName, readItemInfo.Info().OneDrive.ItemName) + assert.Equal(t, folderPath.String(), readItemInfo.Info().OneDrive.ParentPath) } func (suite *OneDriveCollectionSuite) TestOneDriveCollectionReadError() { + t := suite.T() wg := sync.WaitGroup{} collStatus := support.ConnectorOperationStatus{} wg.Add(1) - coll := NewCollection("folderPath", "fakeDriveID", suite, suite.testStatusUpdater(&wg, &collStatus)) + folderPath, err := getCanonicalPath("folderPath", "a-tenant", "a-user") + require.NoError(t, err) + + coll := NewCollection(folderPath, "fakeDriveID", suite, suite.testStatusUpdater(&wg, &collStatus)) coll.Add("testItemID") readError := errors.New("Test error") @@ -116,6 +129,6 @@ func (suite *OneDriveCollectionSuite) TestOneDriveCollectionReadError() { coll.Items() wg.Wait() // Expect no items - require.Equal(suite.T(), 1, collStatus.ObjectCount) - require.Equal(suite.T(), 0, collStatus.Successful) + require.Equal(t, 1, collStatus.ObjectCount) + require.Equal(t, 0, collStatus.Successful) } diff --git a/src/internal/connector/onedrive/collections.go b/src/internal/connector/onedrive/collections.go index 166da2ea1..398051078 100644 --- a/src/internal/connector/onedrive/collections.go +++ b/src/internal/connector/onedrive/collections.go @@ -2,7 +2,8 @@ package onedrive import ( "context" - "path" + stdpath "path" + "strings" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/pkg/errors" @@ -10,12 +11,14 @@ import ( "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/internal/path" ) // Collections is used to retrieve OneDrive data for a // specified user type Collections struct { - user string + tenant string + user string // collectionMap allows lookup of the data.Collection // for a OneDrive folder collectionMap map[string]data.Collection @@ -30,11 +33,13 @@ type Collections struct { } func NewCollections( + tenant string, user string, service graph.Service, statusUpdater support.StatusUpdater, ) *Collections { return &Collections{ + tenant: tenant, user: user, collectionMap: map[string]data.Collection{}, service: service, @@ -66,6 +71,17 @@ func (c *Collections) Get(ctx context.Context) ([]data.Collection, error) { return collections, nil } +func getCanonicalPath(p, tenant, user string) (path.Path, error) { + pathBuilder := path.Builder{}.Append(strings.Split(p, "/")...) + + res, err := pathBuilder.ToDataLayerOneDrivePath(tenant, user, false) + if err != nil { + return nil, errors.Wrap(err, "converting to canonical path") + } + + return res, nil +} + // updateCollections initializes and adds the provided OneDrive items to Collections // A new collection is created for every OneDrive folder (or package) func (c *Collections) updateCollections(ctx context.Context, driveID string, items []models.DriveItemable) error { @@ -81,22 +97,52 @@ func (c *Collections) updateCollections(ctx context.Context, driveID string, ite if item.GetParentReference() == nil || item.GetParentReference().GetPath() == nil { return errors.Errorf("item does not have a parent reference. item name : %s", *item.GetName()) } + // Create a collection for the parent of this item - collectionPath := *item.GetParentReference().GetPath() - if _, found := c.collectionMap[collectionPath]; !found { - c.collectionMap[collectionPath] = NewCollection(collectionPath, driveID, c.service, c.statusUpdater) + collectionPath, err := getCanonicalPath( + *item.GetParentReference().GetPath(), + c.tenant, + c.user, + ) + if err != nil { + return err + } + + if _, found := c.collectionMap[collectionPath.String()]; !found { + c.collectionMap[collectionPath.String()] = NewCollection( + collectionPath, + driveID, + c.service, + c.statusUpdater, + ) } switch { case item.GetFolder() != nil, item.GetPackage() != nil: // For folders and packages we also create a collection to represent those // TODO: This is where we might create a "special file" to represent these in the backup repository // e.g. a ".folderMetadataFile" - itemPath := path.Join(*item.GetParentReference().GetPath(), *item.GetName()) - if _, found := c.collectionMap[itemPath]; !found { - c.collectionMap[itemPath] = NewCollection(itemPath, driveID, c.service, c.statusUpdater) + itemPath, err := getCanonicalPath( + stdpath.Join( + *item.GetParentReference().GetPath(), + *item.GetName(), + ), + c.tenant, + c.user, + ) + if err != nil { + return err + } + + if _, found := c.collectionMap[itemPath.String()]; !found { + c.collectionMap[itemPath.String()] = NewCollection( + itemPath, + driveID, + c.service, + c.statusUpdater, + ) } case item.GetFile() != nil: - collection := c.collectionMap[collectionPath].(*Collection) + collection := c.collectionMap[collectionPath.String()].(*Collection) collection.Add(*item.GetId()) default: return errors.Errorf("item type not supported. item name : %s", *item.GetName()) diff --git a/src/internal/connector/onedrive/collections_test.go b/src/internal/connector/onedrive/collections_test.go index 9cf02f6a8..b8d4c94b2 100644 --- a/src/internal/connector/onedrive/collections_test.go +++ b/src/internal/connector/onedrive/collections_test.go @@ -7,9 +7,23 @@ import ( msgraphsdk "github.com/microsoftgraph/msgraph-sdk-go" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) +func expectedPathAsSlice(t *testing.T, tenant, user string, rest ...string) []string { + res := make([]string, 0, len(rest)) + + for _, r := range rest { + p, err := getCanonicalPath(r, tenant, user) + require.NoError(t, err) + + res = append(res, p.String()) + } + + return res +} + type OneDriveCollectionsSuite struct { suite.Suite } @@ -19,6 +33,8 @@ func TestOneDriveCollectionsSuite(t *testing.T) { } func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { + tenant := "tenant" + user := "user" tests := []struct { testCase string items []models.DriveItemable @@ -41,30 +57,47 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { items: []models.DriveItemable{ driveItem("file", "/root", true, false, false), }, - expect: assert.NoError, - expectedCollectionPaths: []string{"/root"}, - expectedItemCount: 1, - expectedFileCount: 1, + expect: assert.NoError, + expectedCollectionPaths: expectedPathAsSlice( + suite.T(), + tenant, + user, + "root", + ), + expectedItemCount: 1, + expectedFileCount: 1, }, { testCase: "Single Folder", items: []models.DriveItemable{ driveItem("folder", "/root", false, true, false), }, - expect: assert.NoError, - expectedCollectionPaths: []string{"/root", "/root/folder"}, - expectedItemCount: 1, - expectedFolderCount: 1, + expect: assert.NoError, + expectedCollectionPaths: expectedPathAsSlice( + suite.T(), + tenant, + user, + "/root", + "/root/folder", + ), + expectedItemCount: 1, + expectedFolderCount: 1, }, { testCase: "Single Package", items: []models.DriveItemable{ driveItem("package", "/root", false, false, true), }, - expect: assert.NoError, - expectedCollectionPaths: []string{"/root", "/root/package"}, - expectedItemCount: 1, - expectedPackageCount: 1, + expect: assert.NoError, + expectedCollectionPaths: expectedPathAsSlice( + suite.T(), + tenant, + user, + "/root", + "/root/package", + ), + expectedItemCount: 1, + expectedPackageCount: 1, }, { testCase: "1 root file, 1 folder, 1 package, 2 files, 3 collections", @@ -75,17 +108,24 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { driveItem("fileInFolder", "/root/folder", true, false, false), driveItem("fileInPackage", "/root/package", true, false, false), }, - expect: assert.NoError, - expectedCollectionPaths: []string{"/root", "/root/folder", "/root/package"}, - expectedItemCount: 5, - expectedFileCount: 3, - expectedFolderCount: 1, - expectedPackageCount: 1, + expect: assert.NoError, + expectedCollectionPaths: expectedPathAsSlice( + suite.T(), + tenant, + user, + "/root", + "/root/folder", + "/root/package", + ), + expectedItemCount: 5, + expectedFileCount: 3, + expectedFolderCount: 1, + expectedPackageCount: 1, }, } for _, tt := range tests { suite.T().Run(tt.testCase, func(t *testing.T) { - c := NewCollections("user", &MockGraphService{}, nil) + c := NewCollections(tenant, user, &MockGraphService{}, nil) err := c.updateCollections(context.Background(), "driveID", tt.items) tt.expect(t, err) assert.Equal(t, len(tt.expectedCollectionPaths), len(c.collectionMap)) diff --git a/src/internal/path/categorytype_string.go b/src/internal/path/categorytype_string.go index b612e19e4..5ac1c3b21 100644 --- a/src/internal/path/categorytype_string.go +++ b/src/internal/path/categorytype_string.go @@ -12,11 +12,12 @@ func _() { _ = x[EmailCategory-1] _ = x[ContactsCategory-2] _ = x[EventsCategory-3] + _ = x[FilesCategory-4] } -const _CategoryType_name = "UnknownCategoryemailcontactsevents" +const _CategoryType_name = "UnknownCategoryemailcontactseventsfiles" -var _CategoryType_index = [...]uint8{0, 15, 20, 28, 34} +var _CategoryType_index = [...]uint8{0, 15, 20, 28, 34, 39} func (i CategoryType) String() string { if i < 0 || i >= CategoryType(len(_CategoryType_index)-1) { diff --git a/src/internal/path/path.go b/src/internal/path/path.go index cb7d1e854..03ea92682 100644 --- a/src/internal/path/path.go +++ b/src/internal/path/path.go @@ -241,6 +241,27 @@ func (pb Builder) ToDataLayerExchangePathForCategory( }, nil } +func (pb Builder) ToDataLayerOneDrivePath( + tenant, user string, + isItem bool, +) (Path, error) { + if err := pb.verifyPrefix(tenant, user); err != nil { + return nil, err + } + + return &dataLayerResourcePath{ + Builder: *pb.withPrefix( + tenant, + OneDriveService.String(), + user, + FilesCategory.String(), + ), + service: OneDriveService, + category: FilesCategory, + hasItem: isItem, + }, nil +} + // FromDataLayerPath parses the escaped path p, validates the elements in p // match a resource-specific path format, and returns a Path struct for that // resource-specific type. If p does not match any resource-specific paths or diff --git a/src/internal/path/resource_path.go b/src/internal/path/resource_path.go index 478d242ed..31300692c 100644 --- a/src/internal/path/resource_path.go +++ b/src/internal/path/resource_path.go @@ -12,12 +12,15 @@ type ServiceType int const ( UnknownService ServiceType = iota ExchangeService // exchange + OneDriveService // onedrive ) func toServiceType(service string) ServiceType { switch service { case ExchangeService.String(): return ExchangeService + case OneDriveService.String(): + return OneDriveService default: return UnknownService } @@ -33,6 +36,7 @@ const ( EmailCategory // email ContactsCategory // contacts EventsCategory // events + FilesCategory // files ) func ToCategoryType(category string) CategoryType { @@ -43,6 +47,8 @@ func ToCategoryType(category string) CategoryType { return ContactsCategory case EventsCategory.String(): return EventsCategory + case FilesCategory.String(): + return FilesCategory default: return UnknownCategory } @@ -55,6 +61,9 @@ var serviceCategories = map[ServiceType]map[CategoryType]struct{}{ ContactsCategory: {}, EventsCategory: {}, }, + OneDriveService: { + FilesCategory: {}, + }, } func validateServiceAndCategoryStrings(s, c string) (ServiceType, CategoryType, error) { diff --git a/src/internal/path/resource_path_test.go b/src/internal/path/resource_path_test.go index 001700ba2..37e3dacd5 100644 --- a/src/internal/path/resource_path_test.go +++ b/src/internal/path/resource_path_test.go @@ -68,22 +68,39 @@ var ( }, } - // Set of acceptable service/category mixtures for exchange. - exchangeServiceCategories = []struct { + // Set of acceptable service/category mixtures. + serviceCategories = []struct { service path.ServiceType category path.CategoryType + pathFunc func(pb *path.Builder, tenant, user string, isItem bool) (path.Path, error) }{ { service: path.ExchangeService, category: path.EmailCategory, + pathFunc: func(pb *path.Builder, tenant, user string, isItem bool) (path.Path, error) { + return pb.ToDataLayerExchangePathForCategory(tenant, user, path.EmailCategory, isItem) + }, }, { service: path.ExchangeService, category: path.ContactsCategory, + pathFunc: func(pb *path.Builder, tenant, user string, isItem bool) (path.Path, error) { + return pb.ToDataLayerExchangePathForCategory(tenant, user, path.ContactsCategory, isItem) + }, }, { service: path.ExchangeService, category: path.EventsCategory, + pathFunc: func(pb *path.Builder, tenant, user string, isItem bool) (path.Path, error) { + return pb.ToDataLayerExchangePathForCategory(tenant, user, path.EventsCategory, isItem) + }, + }, + { + service: path.OneDriveService, + category: path.FilesCategory, + pathFunc: func(pb *path.Builder, tenant, user string, isItem bool) (path.Path, error) { + return pb.ToDataLayerOneDrivePath(tenant, user, isItem) + }, }, } ) @@ -97,7 +114,7 @@ func TestDataLayerResourcePath(t *testing.T) { } func (suite *DataLayerResourcePath) TestMissingInfoErrors() { - for _, types := range exchangeServiceCategories { + for _, types := range serviceCategories { suite.T().Run(types.service.String()+types.category.String(), func(t1 *testing.T) { for _, m := range modes { t1.Run(m.name, func(t2 *testing.T) { @@ -105,10 +122,10 @@ func (suite *DataLayerResourcePath) TestMissingInfoErrors() { t2.Run(test.name, func(t *testing.T) { b := path.Builder{}.Append(test.rest...) - _, err := b.ToDataLayerExchangePathForCategory( + _, err := types.pathFunc( + b, test.tenant, test.user, - types.category, m.isItem, ) assert.Error(t, err) @@ -124,12 +141,12 @@ func (suite *DataLayerResourcePath) TestMailItemNoFolder() { item := "item" b := path.Builder{}.Append(item) - for _, types := range exchangeServiceCategories { + for _, types := range serviceCategories { suite.T().Run(types.service.String()+types.category.String(), func(t *testing.T) { - p, err := b.ToDataLayerExchangePathForCategory( + p, err := types.pathFunc( + b, testTenant, testUser, - types.category, true, ) require.NoError(t, err) diff --git a/src/internal/path/service_category_test.go b/src/internal/path/service_category_test.go index 6862bdb32..d453d145d 100644 --- a/src/internal/path/service_category_test.go +++ b/src/internal/path/service_category_test.go @@ -97,6 +97,14 @@ func (suite *ServiceCategoryUnitSuite) TestValidateServiceAndCategory() { expectedCategory: EventsCategory, check: assert.NoError, }, + { + name: "OneDriveFiles", + service: OneDriveService.String(), + category: FilesCategory.String(), + expectedService: OneDriveService, + expectedCategory: FilesCategory, + check: assert.NoError, + }, } for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { diff --git a/src/internal/path/servicetype_string.go b/src/internal/path/servicetype_string.go index 208bba5aa..a7d561ebd 100644 --- a/src/internal/path/servicetype_string.go +++ b/src/internal/path/servicetype_string.go @@ -10,11 +10,12 @@ func _() { var x [1]struct{} _ = x[UnknownService-0] _ = x[ExchangeService-1] + _ = x[OneDriveService-2] } -const _ServiceType_name = "UnknownServiceexchange" +const _ServiceType_name = "UnknownServiceexchangeonedrive" -var _ServiceType_index = [...]uint8{0, 14, 22} +var _ServiceType_index = [...]uint8{0, 14, 22, 30} func (i ServiceType) String() string { if i < 0 || i >= ServiceType(len(_ServiceType_index)-1) {