diff --git a/src/internal/connector/graph_connector_onedrive_test.go b/src/internal/connector/graph_connector_onedrive_test.go index 1614c59a8..e86042194 100644 --- a/src/internal/connector/graph_connector_onedrive_test.go +++ b/src/internal/connector/graph_connector_onedrive_test.go @@ -12,6 +12,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/onedrive" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/account" + "github.com/alcionai/corso/src/pkg/backup" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/path" ) @@ -83,10 +84,16 @@ var ( []byte("{}"), ) + fileAData = []byte(strings.Repeat("a", 33)) + fileBData = []byte(strings.Repeat("b", 65)) + fileCData = []byte(strings.Repeat("c", 129)) + fileDData = []byte(strings.Repeat("d", 257)) + fileEData = []byte(strings.Repeat("e", 257)) + fileAEmptyPerms = []itemInfo{ onedriveItemWithData( "test-file.txt"+onedrive.DataFileSuffix, - []byte(strings.Repeat("a", 33)), + fileAData, ), fileEmptyPerms, } @@ -94,7 +101,7 @@ var ( fileBEmptyPerms = []itemInfo{ onedriveItemWithData( "test-file.txt"+onedrive.DataFileSuffix, - []byte(strings.Repeat("b", 65)), + fileBData, ), fileEmptyPerms, } @@ -102,7 +109,7 @@ var ( fileCEmptyPerms = []itemInfo{ onedriveItemWithData( "test-file.txt"+onedrive.DataFileSuffix, - []byte(strings.Repeat("c", 129)), + fileCData, ), fileEmptyPerms, } @@ -110,7 +117,7 @@ var ( fileDEmptyPerms = []itemInfo{ onedriveItemWithData( "test-file.txt"+onedrive.DataFileSuffix, - []byte(strings.Repeat("d", 257)), + fileDData, ), fileEmptyPerms, } @@ -118,7 +125,7 @@ var ( fileEEmptyPerms = []itemInfo{ onedriveItemWithData( "test-file.txt"+onedrive.DataFileSuffix, - []byte(strings.Repeat("e", 257)), + fileEData, ), fileEmptyPerms, } @@ -169,7 +176,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestRestoreAndBackup() { items: withItems( onedriveFileWithMetadata( "test-file.txt", - []byte(strings.Repeat("a", 33)), + fileAData, getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"write"}), ), []itemInfo{onedriveItemWithData( @@ -194,7 +201,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestRestoreAndBackup() { category: path.FilesCategory, items: onedriveFileWithMetadata( "test-file.txt", - []byte(strings.Repeat("e", 66)), + fileEData, getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"read"}), ), auxItems: []itemInfo{ @@ -320,7 +327,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestRestoreAndBackup_Versio items: []itemInfo{ onedriveItemWithData( "test-file.txt", - []byte(strings.Repeat("a", 33)), + fileAData, ), }, }, @@ -335,7 +342,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestRestoreAndBackup_Versio items: []itemInfo{ onedriveItemWithData( "test-file.txt", - []byte(strings.Repeat("b", 65)), + fileBData, ), }, }, @@ -351,7 +358,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestRestoreAndBackup_Versio items: []itemInfo{ onedriveItemWithData( "test-file.txt", - []byte(strings.Repeat("c", 129)), + fileCData, ), }, }, @@ -368,7 +375,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestRestoreAndBackup_Versio items: []itemInfo{ onedriveItemWithData( "test-file.txt", - []byte(strings.Repeat("d", 257)), + fileDData, ), }, }, @@ -383,7 +390,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestRestoreAndBackup_Versio items: []itemInfo{ onedriveItemWithData( "test-file.txt", - []byte(strings.Repeat("e", 257)), + fileEData, ), }, }, @@ -435,13 +442,13 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndBa var ( fileAWritePerms = onedriveFileWithMetadata( "test-file.txt", - []byte(strings.Repeat("a", 33)), + fileAData, getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"write"}), ) fileEReadPerms = onedriveFileWithMetadata( "test-file.txt", - []byte(strings.Repeat("e", 66)), + fileEData, getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"read"}), ) @@ -638,7 +645,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsBackupAndNoR category: path.FilesCategory, items: onedriveFileWithMetadata( "test-file.txt", - []byte(strings.Repeat("a", 33)), + fileAData, getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"write"}), ), auxItems: []itemInfo{ @@ -668,3 +675,113 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsBackupAndNoR }) } } + +func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNoBackup() { + ctx, flush := tester.NewContext() + defer flush() + + t := suite.T() + + driveID := mustGetDefaultDriveID( + t, + ctx, + suite.connector.Service, + suite.user, + ) + + test := restoreBackupInfoMultiVersion{ + service: path.OneDriveService, + resource: Users, + backupVersion: backup.Version, + countMeta: false, + collectionsPrevious: []colInfo{ + { + pathElements: []string{ + "drives", + driveID, + "root:", + }, + category: path.FilesCategory, + items: withItems( + onedriveFileWithMetadata( + "test-file.txt", + fileAData, + getTestMetaJSON(t, suite.secondaryUser, []string{"write"}), + ), + []itemInfo{onedriveItemWithData( + "b"+onedrive.DirMetaFileSuffix, + getTestMetaJSON(t, suite.secondaryUser, []string{"read"}), + )}, + ), + auxItems: []itemInfo{ + onedriveItemWithData( + "test-file.txt"+onedrive.MetaFileSuffix, + getTestMetaJSON(t, suite.secondaryUser, []string{"write"}), + ), + }, + }, + { + pathElements: []string{ + "drives", + driveID, + "root:", + "b", + }, + category: path.FilesCategory, + items: onedriveFileWithMetadata( + "test-file.txt", + fileEData, + getTestMetaJSON(t, suite.secondaryUser, []string{"read"}), + ), + auxItems: []itemInfo{ + onedriveItemWithData( + "test-file.txt"+onedrive.MetaFileSuffix, + getTestMetaJSON(t, suite.secondaryUser, []string{"read"}), + ), + }, + }, + }, + collectionsLatest: []colInfo{ + { + pathElements: []string{ + "drives", + driveID, + "root:", + }, + category: path.FilesCategory, + items: withItems( + fileAEmptyPerms, + folderBEmptyPerms, + ), + auxItems: []itemInfo{ + fileEmptyPerms, + }, + }, + { + pathElements: []string{ + "drives", + driveID, + "root:", + "b", + }, + category: path.FilesCategory, + items: fileEEmptyPerms, + auxItems: []itemInfo{ + fileEmptyPerms, + }, + }, + }, + } + + runRestoreBackupTestVersions( + t, + suite.acct, + test, + suite.connector.tenant, + []string{suite.user}, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: false}, + }, + ) +} diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 49adbcb14..152e85331 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -480,10 +480,7 @@ func runBackupAndCompare( ctx, backupSel, nil, - control.Options{ - RestorePermissions: true, - ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, - }, + config.opts, fault.New(true)) require.NoError(t, err) // No excludes yet because this isn't an incremental backup. diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go index 5f5884599..078d704fb 100644 --- a/src/internal/connector/onedrive/collection.go +++ b/src/internal/connector/onedrive/collection.go @@ -5,11 +5,11 @@ import ( "context" "io" "net/http" - "strings" "sync" "sync/atomic" "time" + "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/pkg/errors" "github.com/spatialcurrent/go-lazy/pkg/lazy" @@ -91,6 +91,7 @@ type itemMetaReaderFunc func( service graph.Servicer, driveID string, item models.DriveItemable, + fetchPermissions bool, ) (io.ReadCloser, int, error) // NewCollection creates a Collection @@ -180,6 +181,7 @@ type UserPermission struct { // ItemMeta contains metadata about the Item. It gets stored in a // separate file in kopia type Metadata struct { + FileName string `json:"filename,omitempty"` Permissions []UserPermission `json:"permissions,omitempty"` } @@ -292,20 +294,16 @@ func (oc *Collection) populateItems(ctx context.Context) { if oc.source == OneDriveSource { // Fetch metadata for the file - if !oc.ctrl.ToggleFeatures.EnablePermissionsBackup { - // We are still writing the metadata file but with - // empty permissions as we don't have a way to - // signify that the permissions was explicitly - // not added. - itemMeta = io.NopCloser(strings.NewReader("{}")) - itemMetaSize = 2 - } else { - itemMeta, itemMetaSize, err = oc.itemMetaReader(ctx, oc.service, oc.driveID, item) + itemMeta, itemMetaSize, err = oc.itemMetaReader( + ctx, + oc.service, + oc.driveID, + item, + oc.ctrl.ToggleFeatures.EnablePermissionsBackup) - if err != nil { - errUpdater(*item.GetId(), errors.Wrap(err, "failed to get item permissions")) - return - } + if err != nil { + errUpdater(itemID, clues.Wrap(err, "getting item metadata")) + return } } diff --git a/src/internal/connector/onedrive/collection_test.go b/src/internal/connector/onedrive/collection_test.go index 5d843830f..d5305088a 100644 --- a/src/internal/connector/onedrive/collection_test.go +++ b/src/internal/connector/onedrive/collection_test.go @@ -197,6 +197,7 @@ func (suite *CollectionUnitTestSuite) TestCollection() { _ graph.Servicer, _ string, _ models.DriveItemable, + _ bool, ) (io.ReadCloser, int, error) { metaJSON, err := json.Marshal(testItemMeta) if err != nil { @@ -331,6 +332,7 @@ func (suite *CollectionUnitTestSuite) TestCollectionReadError() { _ graph.Servicer, _ string, _ models.DriveItemable, + _ bool, ) (io.ReadCloser, int, error) { return io.NopCloser(strings.NewReader(`{}`)), 2, nil } @@ -350,94 +352,6 @@ func (suite *CollectionUnitTestSuite) TestCollectionReadError() { } } -func (suite *CollectionUnitTestSuite) TestCollectionDisablePermissionsBackup() { - table := []struct { - name string - source driveSource - }{ - { - name: "oneDrive", - source: OneDriveSource, - }, - } - for _, test := range table { - suite.T().Run(test.name, func(t *testing.T) { - ctx, flush := tester.NewContext() - defer flush() - - var ( - testItemID = "fakeItemID" - testItemName = "Fake Item" - testItemSize = int64(10) - collStatus = support.ConnectorOperationStatus{} - wg = sync.WaitGroup{} - ) - - wg.Add(1) - - folderPath, err := GetCanonicalPath("drive/driveID1/root:/folderPath", "a-tenant", "a-user", test.source) - require.NoError(t, err) - - coll := NewCollection( - graph.HTTPClient(graph.NoTimeout()), - folderPath, - nil, - "fakeDriveID", - suite, - suite.testStatusUpdater(&wg, &collStatus), - test.source, - control.Options{ToggleFeatures: control.Toggles{}}, - true) - - now := time.Now() - mockItem := models.NewDriveItem() - mockItem.SetFile(models.NewFile()) - mockItem.SetId(&testItemID) - mockItem.SetName(&testItemName) - mockItem.SetSize(&testItemSize) - mockItem.SetCreatedDateTime(&now) - mockItem.SetLastModifiedDateTime(&now) - coll.Add(mockItem) - - coll.itemReader = func( - *http.Client, - models.DriveItemable, - ) (details.ItemInfo, io.ReadCloser, error) { - return details.ItemInfo{OneDrive: &details.OneDriveInfo{ItemName: "fakeName", Modified: time.Now()}}, - io.NopCloser(strings.NewReader("Fake Data!")), - nil - } - - coll.itemMetaReader = func(_ context.Context, - _ graph.Servicer, - _ string, - _ models.DriveItemable, - ) (io.ReadCloser, int, error) { - return io.NopCloser(strings.NewReader(`{"key": "value"}`)), 16, nil - } - - readItems := []data.Stream{} - for item := range coll.Items(ctx, fault.New(true)) { - readItems = append(readItems, item) - } - - wg.Wait() - - // Expect no items - require.Equal(t, 1, collStatus.ObjectCount) - require.Equal(t, 1, collStatus.Successful) - - for _, i := range readItems { - if strings.HasSuffix(i.UUID(), MetaFileSuffix) { - content, err := io.ReadAll(i.ToReader()) - require.NoError(t, err) - require.Equal(t, content, []byte("{}")) - } - } - }) - } -} - // TODO(meain): Remove this test once we start always backing up permissions func (suite *CollectionUnitTestSuite) TestCollectionPermissionBackupLatestModTime() { table := []struct { @@ -502,6 +416,7 @@ func (suite *CollectionUnitTestSuite) TestCollectionPermissionBackupLatestModTim _ graph.Servicer, _ string, _ models.DriveItemable, + _ bool, ) (io.ReadCloser, int, error) { return io.NopCloser(strings.NewReader(`{}`)), 16, nil } diff --git a/src/internal/connector/onedrive/item.go b/src/internal/connector/onedrive/item.go index c527ce09b..9ce1188ca 100644 --- a/src/internal/connector/onedrive/item.go +++ b/src/internal/connector/onedrive/item.go @@ -9,6 +9,7 @@ import ( "net/http" "strings" + "github.com/alcionai/clues" msdrives "github.com/microsoftgraph/msgraph-sdk-go/drives" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/pkg/errors" @@ -61,18 +62,40 @@ func oneDriveItemMetaReader( service graph.Servicer, driveID string, item models.DriveItemable, + fetchPermissions bool, ) (io.ReadCloser, int, error) { - meta, err := oneDriveItemMetaInfo(ctx, service, driveID, item) + meta := Metadata{ + FileName: *item.GetName(), + } + + perms, err := oneDriveItemPermissionInfo(ctx, service, driveID, item, fetchPermissions) if err != nil { + // Keep this in an if-block because if it's not then we have a weird issue + // of having no value in error but golang thinking it's non nil because of + // the way interfaces work. + err = clues.Wrap(err, "fetching item permissions") + } else { + meta.Permissions = perms + } + + metaJSON, serializeErr := json.Marshal(meta) + if serializeErr != nil { + serializeErr = clues.Wrap(serializeErr, "serializing item metadata") + + // Need to check if err was already non-nil since it doesn't filter nil + // values out in calls to Stack(). + if err != nil { + err = clues.Stack(err, serializeErr) + } else { + err = serializeErr + } + return nil, 0, err } - metaJSON, err := json.Marshal(meta) - if err != nil { - return nil, 0, err - } + r := io.NopCloser(bytes.NewReader(metaJSON)) - return io.NopCloser(bytes.NewReader(metaJSON)), len(metaJSON), nil + return r, len(metaJSON), err } // oneDriveItemReader will return a io.ReadCloser for the specified item @@ -180,23 +203,32 @@ func oneDriveItemInfo(di models.DriveItemable, itemSize int64) *details.OneDrive } } -// oneDriveItemMetaInfo will fetch the meta information for a drive -// item. As of now, it only adds the permissions applicable for a -// onedrive item. -func oneDriveItemMetaInfo( - ctx context.Context, service graph.Servicer, - driveID string, di models.DriveItemable, -) (Metadata, error) { - itemID := di.GetId() +// oneDriveItemPermissionInfo will fetch the permission information for a drive +// item. +func oneDriveItemPermissionInfo( + ctx context.Context, + service graph.Servicer, + driveID string, + di models.DriveItemable, + fetchPermissions bool, +) ([]UserPermission, error) { + if !fetchPermissions { + return nil, nil + } - perm, err := service.Client().DrivesById(driveID).ItemsById(*itemID).Permissions().Get(ctx, nil) + perm, err := service. + Client(). + DrivesById(driveID). + ItemsById(*di.GetId()). + Permissions(). + Get(ctx, nil) if err != nil { - return Metadata{}, err + return nil, err } uperms := filterUserPermissions(perm.GetValue()) - return Metadata{Permissions: uperms}, nil + return uperms, nil } func filterUserPermissions(perms []models.Permissionable) []UserPermission {