From d067a7948326a42ade0b5d1d6f39d062f9dc5b78 Mon Sep 17 00:00:00 2001 From: Keepers Date: Wed, 3 May 2023 12:52:25 -0600 Subject: [PATCH] move permissions into metadata (#3216) Moves the UserPermissions struct into onedrive/metadata as the Permission struct, for better shared support across onedrive, sharepoint, and any other packages that reference it. --- #### Does this PR need a docs update or release note? - [x] :no_entry: No #### Type of change - [x] :broom: Tech Debt/Cleanup #### Issue(s) * #3135 #### Test Plan - [x] :zap: Unit test - [x] :green_heart: E2E --- src/cmd/factory/impl/common.go | 15 +- .../connector/graph/metadata/metadata.go | 1 + .../connector/graph_connector_helper_test.go | 8 +- .../graph_connector_onedrive_test.go | 27 ++- src/internal/connector/onedrive/collection.go | 28 --- .../connector/onedrive/collection_test.go | 40 ++-- src/internal/connector/onedrive/item.go | 19 +- src/internal/connector/onedrive/item_test.go | 25 +-- .../connector/onedrive/metadata/metadata.go | 12 ++ .../onedrive/metadata/permissions.go | 90 +++++++++ .../onedrive/metadata/permissions_test.go | 149 ++++++++++++++ src/internal/connector/onedrive/permission.go | 110 +++-------- .../connector/onedrive/permission_test.go | 184 +++--------------- src/internal/connector/onedrive/restore.go | 28 +-- src/internal/connector/sharepoint/restore.go | 3 +- .../operations/backup_integration_test.go | 73 +++---- 16 files changed, 424 insertions(+), 388 deletions(-) create mode 100644 src/internal/connector/onedrive/metadata/metadata.go create mode 100644 src/internal/connector/onedrive/metadata/permissions.go create mode 100644 src/internal/connector/onedrive/metadata/permissions_test.go diff --git a/src/cmd/factory/impl/common.go b/src/cmd/factory/impl/common.go index 096b842ae..f0ab0696c 100644 --- a/src/cmd/factory/impl/common.go +++ b/src/cmd/factory/impl/common.go @@ -20,7 +20,6 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector" exchMock "github.com/alcionai/corso/src/internal/connector/exchange/mock" - "github.com/alcionai/corso/src/internal/connector/onedrive" "github.com/alcionai/corso/src/internal/connector/onedrive/metadata" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/tester" @@ -204,7 +203,7 @@ type permData struct { user string // user is only for older versions entityID string roles []string - sharingMode onedrive.SharingMode + sharingMode metadata.SharingMode } type itemData struct { @@ -672,10 +671,10 @@ func onedriveMetadata( } } -func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metadata { +func getMetadata(fileName string, perm permData, permUseID bool) metadata.Metadata { if len(perm.user) == 0 || len(perm.roles) == 0 || - perm.sharingMode != onedrive.SharingModeCustom { - return onedrive.Metadata{ + perm.sharingMode != metadata.SharingModeCustom { + return metadata.Metadata{ FileName: fileName, SharingMode: perm.sharingMode, } @@ -685,7 +684,7 @@ func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metada // user/role combo unless deleted and readded, but we have to do // this as we only have two users of which one is already taken. id := uuid.NewString() - uperm := onedrive.UserPermission{ID: id, Roles: perm.roles} + uperm := metadata.Permission{ID: id, Roles: perm.roles} if permUseID { uperm.EntityID = perm.entityID @@ -693,9 +692,9 @@ func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metada uperm.Email = perm.user } - meta := onedrive.Metadata{ + meta := metadata.Metadata{ FileName: fileName, - Permissions: []onedrive.UserPermission{uperm}, + Permissions: []metadata.Permission{uperm}, } return meta diff --git a/src/internal/connector/graph/metadata/metadata.go b/src/internal/connector/graph/metadata/metadata.go index 11378f2ad..6aa0d5fa6 100644 --- a/src/internal/connector/graph/metadata/metadata.go +++ b/src/internal/connector/graph/metadata/metadata.go @@ -12,6 +12,7 @@ func IsMetadataFile(p path.Path) bool { case path.SharePointService: return p.Category() == path.LibrariesCategory && metadata.HasMetaSuffix(p.Item()) + default: return false } diff --git a/src/internal/connector/graph_connector_helper_test.go b/src/internal/connector/graph_connector_helper_test.go index 951b87104..628c68d36 100644 --- a/src/internal/connector/graph_connector_helper_test.go +++ b/src/internal/connector/graph_connector_helper_test.go @@ -693,7 +693,7 @@ func compareExchangeEvent( checkEvent(t, expectedEvent, itemEvent) } -func permissionEqual(expected onedrive.UserPermission, got onedrive.UserPermission) bool { +func permissionEqual(expected metadata.Permission, got metadata.Permission) bool { if !strings.EqualFold(expected.Email, got.Email) { return false } @@ -769,8 +769,8 @@ func compareOneDriveItem( if isMeta { var ( - itemMeta onedrive.Metadata - expectedMeta onedrive.Metadata + itemMeta metadata.Metadata + expectedMeta metadata.Metadata ) err = json.Unmarshal(buf, &itemMeta) @@ -812,7 +812,7 @@ func compareOneDriveItem( } // We cannot restore owner permissions, so skip checking them - itemPerms := []onedrive.UserPermission{} + itemPerms := []metadata.Permission{} for _, p := range itemMeta.Permissions { if p.Roles[0] != "owner" { diff --git a/src/internal/connector/graph_connector_onedrive_test.go b/src/internal/connector/graph_connector_onedrive_test.go index 2072d150e..0c4c40a47 100644 --- a/src/internal/connector/graph_connector_onedrive_test.go +++ b/src/internal/connector/graph_connector_onedrive_test.go @@ -16,7 +16,6 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/graph" - "github.com/alcionai/corso/src/internal/connector/onedrive" "github.com/alcionai/corso/src/internal/connector/onedrive/metadata" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/internal/version" @@ -29,10 +28,10 @@ import ( // permission instead of email const versionPermissionSwitchedToID = version.OneDrive4DirIncludesPermissions -func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metadata { +func getMetadata(fileName string, perm permData, permUseID bool) metadata.Metadata { if len(perm.user) == 0 || len(perm.roles) == 0 || - perm.sharingMode != onedrive.SharingModeCustom { - return onedrive.Metadata{ + perm.sharingMode != metadata.SharingModeCustom { + return metadata.Metadata{ FileName: fileName, SharingMode: perm.sharingMode, } @@ -42,7 +41,7 @@ func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metada // user/role combo unless deleted and readded, but we have to do // this as we only have two users of which one is already taken. id := uuid.NewString() - uperm := onedrive.UserPermission{ID: id, Roles: perm.roles} + uperm := metadata.Permission{ID: id, Roles: perm.roles} if permUseID { uperm.EntityID = perm.entityID @@ -50,9 +49,9 @@ func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metada uperm.Email = perm.user } - testMeta := onedrive.Metadata{ + testMeta := metadata.Metadata{ FileName: fileName, - Permissions: []onedrive.UserPermission{uperm}, + Permissions: []metadata.Permission{uperm}, } return testMeta @@ -278,7 +277,7 @@ type permData struct { user string // user is only for older versions entityID string roles []string - sharingMode onedrive.SharingMode + sharingMode metadata.SharingMode } type itemData struct { @@ -1114,21 +1113,21 @@ func testPermissionsInheritanceRestoreAndBackup(suite oneDriveSuite, startVersio user: secondaryUserName, entityID: secondaryUserID, roles: writePerm, - sharingMode: onedrive.SharingModeCustom, + sharingMode: metadata.SharingModeCustom, }, }, { name: "file-inherited", data: fileAData, perms: permData{ - sharingMode: onedrive.SharingModeInherited, + sharingMode: metadata.SharingModeInherited, }, }, { name: "file-empty", data: fileAData, perms: permData{ - sharingMode: onedrive.SharingModeCustom, + sharingMode: metadata.SharingModeCustom, }, }, } @@ -1180,21 +1179,21 @@ func testPermissionsInheritanceRestoreAndBackup(suite oneDriveSuite, startVersio user: tertiaryUserName, entityID: tertiaryUserID, roles: writePerm, - sharingMode: onedrive.SharingModeCustom, + sharingMode: metadata.SharingModeCustom, }, }, { pathElements: subfolderABPath, files: fileSet, perms: permData{ - sharingMode: onedrive.SharingModeInherited, + sharingMode: metadata.SharingModeInherited, }, }, { pathElements: subfolderACPath, files: fileSet, perms: permData{ - sharingMode: onedrive.SharingModeCustom, + sharingMode: metadata.SharingModeCustom, }, }, } diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go index f6b967826..2a827ce27 100644 --- a/src/internal/connector/onedrive/collection.go +++ b/src/internal/connector/onedrive/collection.go @@ -41,13 +41,6 @@ var ( _ data.StreamModTime = &MetadataItem{} ) -type SharingMode int - -const ( - SharingModeCustom = SharingMode(iota) - SharingModeInherited -) - // Collection represents a set of OneDrive objects retrieved from M365 type Collection struct { // configured to handle large item downloads @@ -306,27 +299,6 @@ func (oc Collection) DoNotMergeItems() bool { return oc.doNotMergeItems } -// FilePermission is used to store permissions of a specific user to a -// OneDrive item. -type UserPermission struct { - ID string `json:"id,omitempty"` - Roles []string `json:"role,omitempty"` - Email string `json:"email,omitempty"` // DEPRECATED: Replaced with UserID in newer backups - EntityID string `json:"entityId,omitempty"` - Expiration *time.Time `json:"expiration,omitempty"` -} - -// ItemMeta contains metadata about the Item. It gets stored in a -// separate file in kopia -type Metadata struct { - FileName string `json:"filename,omitempty"` - // SharingMode denotes what the current mode of sharing is for the object. - // - inherited: permissions same as parent permissions (no "shared" in delta) - // - custom: use Permissions to set correct permissions ("shared" has value in delta) - SharingMode SharingMode `json:"permissionMode,omitempty"` - Permissions []UserPermission `json:"permissions,omitempty"` -} - // Item represents a single item retrieved from OneDrive type Item struct { id string diff --git a/src/internal/connector/onedrive/collection_test.go b/src/internal/connector/onedrive/collection_test.go index 682033f07..48dcc0f67 100644 --- a/src/internal/connector/onedrive/collection_test.go +++ b/src/internal/connector/onedrive/collection_test.go @@ -68,14 +68,16 @@ func (suite *CollectionUnitTestSuite) TestCollection() { testItemName = "itemName" testItemData = []byte("testdata") now = time.Now() - testItemMeta = Metadata{Permissions: []UserPermission{ - { - ID: "testMetaID", - Roles: []string{"read", "write"}, - Email: "email@provider.com", - Expiration: &now, + testItemMeta = metadata.Metadata{ + Permissions: []metadata.Permission{ + { + ID: "testMetaID", + Roles: []string{"read", "write"}, + Email: "email@provider.com", + Expiration: &now, + }, }, - }} + } ) type nst struct { @@ -291,21 +293,19 @@ func (suite *CollectionUnitTestSuite) TestCollection() { assert.Equal(t, testItemName, name) assert.Equal(t, driveFolderPath, parentPath) - if test.source == OneDriveSource { - readItemMeta := readItems[1] + readItemMeta := readItems[1] - assert.Equal(t, testItemID+metadata.MetaFileSuffix, readItemMeta.UUID()) + assert.Equal(t, testItemID+metadata.MetaFileSuffix, readItemMeta.UUID()) - readMetaData, err := io.ReadAll(readItemMeta.ToReader()) - require.NoError(t, err, clues.ToCore(err)) + readMetaData, err := io.ReadAll(readItemMeta.ToReader()) + require.NoError(t, err, clues.ToCore(err)) - tm, err := json.Marshal(testItemMeta) - if err != nil { - t.Fatal("unable to marshall test permissions", err) - } - - assert.Equal(t, tm, readMetaData) + tm, err := json.Marshal(testItemMeta) + if err != nil { + t.Fatal("unable to marshall test permissions", err) } + + assert.Equal(t, tm, readMetaData) }) } } @@ -516,6 +516,10 @@ func (suite *CollectionUnitTestSuite) TestCollectionPermissionBackupLatestModTim name: "oneDrive", source: OneDriveSource, }, + { + name: "sharePoint", + source: SharePointSource, + }, } for _, test := range table { suite.Run(test.name, func() { diff --git a/src/internal/connector/onedrive/item.go b/src/internal/connector/onedrive/item.go index 5b9be865c..c7cebc8c1 100644 --- a/src/internal/connector/onedrive/item.go +++ b/src/internal/connector/onedrive/item.go @@ -15,6 +15,7 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/onedrive/api" + "github.com/alcionai/corso/src/internal/connector/onedrive/metadata" "github.com/alcionai/corso/src/internal/connector/uploadsession" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/logger" @@ -73,18 +74,18 @@ func baseItemMetaReader( item models.DriveItemable, ) (io.ReadCloser, int, error) { var ( - perms []UserPermission + perms []metadata.Permission err error - meta = Metadata{FileName: ptr.Val(item.GetName())} + meta = metadata.Metadata{FileName: ptr.Val(item.GetName())} ) if item.GetShared() == nil { - meta.SharingMode = SharingModeInherited + meta.SharingMode = metadata.SharingModeInherited } else { - meta.SharingMode = SharingModeCustom + meta.SharingMode = metadata.SharingModeCustom } - if meta.SharingMode == SharingModeCustom { + if meta.SharingMode == metadata.SharingModeCustom { perms, err = driveItemPermissionInfo(ctx, service, driveID, ptr.Val(item.GetId())) if err != nil { return nil, 0, err @@ -211,7 +212,7 @@ func driveItemPermissionInfo( service graph.Servicer, driveID string, itemID string, -) ([]UserPermission, error) { +) ([]metadata.Permission, error) { perm, err := api.GetItemPermission(ctx, service, driveID, itemID) if err != nil { return nil, err @@ -222,8 +223,8 @@ func driveItemPermissionInfo( return uperms, nil } -func filterUserPermissions(ctx context.Context, perms []models.Permissionable) []UserPermission { - up := []UserPermission{} +func filterUserPermissions(ctx context.Context, perms []models.Permissionable) []metadata.Permission { + up := []metadata.Permission{} for _, p := range perms { if p.GetGrantedToV2() == nil { @@ -268,7 +269,7 @@ func filterUserPermissions(ctx context.Context, perms []models.Permissionable) [ continue } - up = append(up, UserPermission{ + up = append(up, metadata.Permission{ ID: ptr.Val(p.GetId()), Roles: roles, EntityID: entityID, diff --git a/src/internal/connector/onedrive/item_test.go b/src/internal/connector/onedrive/item_test.go index 604ef10a4..65b69ede7 100644 --- a/src/internal/connector/onedrive/item_test.go +++ b/src/internal/connector/onedrive/item_test.go @@ -16,6 +16,7 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/onedrive/api" + "github.com/alcionai/corso/src/internal/connector/onedrive/metadata" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/fault" ) @@ -247,7 +248,7 @@ func (suite *ItemIntegrationSuite) TestDriveGetFolder() { } } -func getPermsUperms(permID, userID, entity string, scopes []string) (models.Permissionable, UserPermission) { +func getPermsUperms(permID, userID, entity string, scopes []string) (models.Permissionable, metadata.Permission) { identity := models.NewIdentity() identity.SetId(&userID) identity.SetAdditionalData(map[string]any{"email": &userID}) @@ -270,7 +271,7 @@ func getPermsUperms(permID, userID, entity string, scopes []string) (models.Perm perm.SetRoles([]string{"read"}) perm.SetGrantedToV2(sharepointIdentity) - uperm := UserPermission{ + uperm := metadata.Permission{ ID: permID, Roles: []string{"read"}, EntityID: userID, @@ -305,56 +306,56 @@ func (suite *ItemUnitTestSuite) TestOneDrivePermissionsFilter() { cases := []struct { name string graphPermissions []models.Permissionable - parsedPermissions []UserPermission + parsedPermissions []metadata.Permission }{ { name: "no perms", graphPermissions: []models.Permissionable{}, - parsedPermissions: []UserPermission{}, + parsedPermissions: []metadata.Permission{}, }, { name: "no user bound to perms", graphPermissions: []models.Permissionable{noPerm}, - parsedPermissions: []UserPermission{}, + parsedPermissions: []metadata.Permission{}, }, // user { name: "user with read permissions", graphPermissions: []models.Permissionable{userReadPerm}, - parsedPermissions: []UserPermission{userReadUperm}, + parsedPermissions: []metadata.Permission{userReadUperm}, }, { name: "user with owner permissions", graphPermissions: []models.Permissionable{userOwnerPerm}, - parsedPermissions: []UserPermission{userOwnerUperm}, + parsedPermissions: []metadata.Permission{userOwnerUperm}, }, { name: "user with read and write permissions", graphPermissions: []models.Permissionable{userReadWritePerm}, - parsedPermissions: []UserPermission{userReadWriteUperm}, + parsedPermissions: []metadata.Permission{userReadWriteUperm}, }, { name: "multiple users with separate permissions", graphPermissions: []models.Permissionable{userReadPerm, userReadWritePerm}, - parsedPermissions: []UserPermission{userReadUperm, userReadWriteUperm}, + parsedPermissions: []metadata.Permission{userReadUperm, userReadWriteUperm}, }, // group { name: "group with read permissions", graphPermissions: []models.Permissionable{groupReadPerm}, - parsedPermissions: []UserPermission{groupReadUperm}, + parsedPermissions: []metadata.Permission{groupReadUperm}, }, { name: "group with read and write permissions", graphPermissions: []models.Permissionable{groupReadWritePerm}, - parsedPermissions: []UserPermission{groupReadWriteUperm}, + parsedPermissions: []metadata.Permission{groupReadWriteUperm}, }, { name: "multiple groups with separate permissions", graphPermissions: []models.Permissionable{groupReadPerm, groupReadWritePerm}, - parsedPermissions: []UserPermission{groupReadUperm, groupReadWriteUperm}, + parsedPermissions: []metadata.Permission{groupReadUperm, groupReadWriteUperm}, }, } for _, tc := range cases { diff --git a/src/internal/connector/onedrive/metadata/metadata.go b/src/internal/connector/onedrive/metadata/metadata.go new file mode 100644 index 000000000..51e9105d5 --- /dev/null +++ b/src/internal/connector/onedrive/metadata/metadata.go @@ -0,0 +1,12 @@ +package metadata + +// ItemMeta contains metadata about the Item. It gets stored in a +// separate file in kopia +type Metadata struct { + FileName string `json:"filename,omitempty"` + // SharingMode denotes what the current mode of sharing is for the object. + // - inherited: permissions same as parent permissions (no "shared" in delta) + // - custom: use Permissions to set correct permissions ("shared" has value in delta) + SharingMode SharingMode `json:"permissionMode,omitempty"` + Permissions []Permission `json:"permissions,omitempty"` +} diff --git a/src/internal/connector/onedrive/metadata/permissions.go b/src/internal/connector/onedrive/metadata/permissions.go new file mode 100644 index 000000000..12c4a4b89 --- /dev/null +++ b/src/internal/connector/onedrive/metadata/permissions.go @@ -0,0 +1,90 @@ +package metadata + +import ( + "time" + + "golang.org/x/exp/slices" +) + +type SharingMode int + +const ( + SharingModeCustom = SharingMode(iota) + SharingModeInherited +) + +// FilePermission is used to store permissions of a specific resource owner +// to a drive item. +type Permission struct { + ID string `json:"id,omitempty"` + Roles []string `json:"role,omitempty"` + Email string `json:"email,omitempty"` // DEPRECATED: Replaced with EntityID in newer backups + EntityID string `json:"entityId,omitempty"` // this is the resource owner's ID + Expiration *time.Time `json:"expiration,omitempty"` +} + +// isSamePermission checks equality of two UserPermission objects +func (p Permission) Equals(other Permission) bool { + // EntityID can be empty for older backups and Email can be empty + // for newer ones. It is not possible for both to be empty. Also, + // if EntityID/Email for one is not empty then the other will also + // have EntityID/Email as we backup permissions for all the + // parents and children when we have a change in permissions. + if p.EntityID != "" && p.EntityID != other.EntityID { + return false + } + + if p.Email != "" && p.Email != other.Email { + return false + } + + p1r := p.Roles + p2r := other.Roles + + slices.Sort(p1r) + slices.Sort(p2r) + + return slices.Equal(p1r, p2r) +} + +// DiffPermissions compares the before and after set, returning +// the permissions that were added and removed (in that order) +// in the after set. +func DiffPermissions(before, after []Permission) ([]Permission, []Permission) { + var ( + added = []Permission{} + removed = []Permission{} + ) + + for _, cp := range after { + found := false + + for _, pp := range before { + if cp.Equals(pp) { + found = true + break + } + } + + if !found { + added = append(added, cp) + } + } + + for _, pp := range before { + found := false + + for _, cp := range after { + if cp.Equals(pp) { + found = true + break + } + } + + if !found { + removed = append(removed, pp) + } + } + + return added, removed +} diff --git a/src/internal/connector/onedrive/metadata/permissions_test.go b/src/internal/connector/onedrive/metadata/permissions_test.go new file mode 100644 index 000000000..046052f37 --- /dev/null +++ b/src/internal/connector/onedrive/metadata/permissions_test.go @@ -0,0 +1,149 @@ +package metadata + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" + + "github.com/alcionai/corso/src/internal/tester" +) + +type PermissionsUnitTestSuite struct { + tester.Suite +} + +func TestPermissionsUnitTestSuite(t *testing.T) { + suite.Run(t, &PermissionsUnitTestSuite{Suite: tester.NewUnitSuite(t)}) +} + +func (suite *PermissionsUnitTestSuite) TestDiffPermissions() { + perm1 := Permission{ + ID: "id1", + Roles: []string{"read"}, + EntityID: "user-id1", + } + + perm2 := Permission{ + ID: "id2", + Roles: []string{"write"}, + EntityID: "user-id2", + } + + perm3 := Permission{ + ID: "id3", + Roles: []string{"write"}, + EntityID: "user-id3", + } + + // The following two permissions have same id and user but + // different roles, this is a valid scenario for permissions. + sameidperm1 := Permission{ + ID: "id0", + Roles: []string{"write"}, + EntityID: "user-id0", + } + sameidperm2 := Permission{ + ID: "id0", + Roles: []string{"read"}, + EntityID: "user-id0", + } + + emailperm1 := Permission{ + ID: "id1", + Roles: []string{"read"}, + Email: "email1@provider.com", + } + + emailperm2 := Permission{ + ID: "id1", + Roles: []string{"read"}, + Email: "email2@provider.com", + } + + table := []struct { + name string + before []Permission + after []Permission + added []Permission + removed []Permission + }{ + { + name: "single permission added", + before: []Permission{}, + after: []Permission{perm1}, + added: []Permission{perm1}, + removed: []Permission{}, + }, + { + name: "single permission removed", + before: []Permission{perm1}, + after: []Permission{}, + added: []Permission{}, + removed: []Permission{perm1}, + }, + { + name: "multiple permission added", + before: []Permission{}, + after: []Permission{perm1, perm2}, + added: []Permission{perm1, perm2}, + removed: []Permission{}, + }, + { + name: "single permission removed", + before: []Permission{perm1, perm2}, + after: []Permission{}, + added: []Permission{}, + removed: []Permission{perm1, perm2}, + }, + { + name: "extra permissions", + before: []Permission{perm1, perm2}, + after: []Permission{perm1, perm2, perm3}, + added: []Permission{perm3}, + removed: []Permission{}, + }, + { + name: "less permissions", + before: []Permission{perm1, perm2, perm3}, + after: []Permission{perm1, perm2}, + added: []Permission{}, + removed: []Permission{perm3}, + }, + { + name: "same id different role", + before: []Permission{sameidperm1}, + after: []Permission{sameidperm2}, + added: []Permission{sameidperm2}, + removed: []Permission{sameidperm1}, + }, + { + name: "email based extra permissions", + before: []Permission{emailperm1}, + after: []Permission{emailperm1, emailperm2}, + added: []Permission{emailperm2}, + removed: []Permission{}, + }, + { + name: "email based less permissions", + before: []Permission{emailperm1, emailperm2}, + after: []Permission{emailperm1}, + added: []Permission{}, + removed: []Permission{emailperm2}, + }, + } + + for _, test := range table { + suite.Run(test.name, func() { + _, flush := tester.NewContext() + defer flush() + + t := suite.T() + + added, removed := DiffPermissions(test.before, test.after) + + assert.Equal(t, added, test.added, "added permissions") + assert.Equal(t, removed, test.removed, "removed permissions") + }) + } +} diff --git a/src/internal/connector/onedrive/permission.go b/src/internal/connector/onedrive/permission.go index deb5109be..101ae1ddf 100644 --- a/src/internal/connector/onedrive/permission.go +++ b/src/internal/connector/onedrive/permission.go @@ -6,7 +6,6 @@ import ( "github.com/alcionai/clues" msdrive "github.com/microsoftgraph/msgraph-sdk-go/drive" "github.com/microsoftgraph/msgraph-sdk-go/models" - "golang.org/x/exp/slices" "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/graph" @@ -19,20 +18,20 @@ import ( func getParentMetadata( parentPath path.Path, - metas map[string]Metadata, -) (Metadata, error) { + metas map[string]metadata.Metadata, +) (metadata.Metadata, error) { parentMeta, ok := metas[parentPath.String()] if !ok { onedrivePath, err := path.ToOneDrivePath(parentPath) if err != nil { - return Metadata{}, clues.Wrap(err, "invalid restore path") + return metadata.Metadata{}, clues.Wrap(err, "invalid restore path") } if len(onedrivePath.Folders) != 0 { - return Metadata{}, clues.Wrap(err, "computing item permissions") + return metadata.Metadata{}, clues.Wrap(err, "computing item permissions") } - parentMeta = Metadata{} + parentMeta = metadata.Metadata{} } return parentMeta, nil @@ -42,12 +41,12 @@ func getCollectionMetadata( ctx context.Context, drivePath *path.DrivePath, dc data.RestoreCollection, - metas map[string]Metadata, + metas map[string]metadata.Metadata, backupVersion int, restorePerms bool, -) (Metadata, error) { +) (metadata.Metadata, error) { if !restorePerms || backupVersion < version.OneDrive1DataAndMetaFiles { - return Metadata{}, nil + return metadata.Metadata{}, nil } var ( @@ -57,13 +56,13 @@ func getCollectionMetadata( if len(drivePath.Folders) == 0 { // No permissions for root folder - return Metadata{}, nil + return metadata.Metadata{}, nil } if backupVersion < version.OneDrive4DirIncludesPermissions { colMeta, err := getParentMetadata(collectionPath, metas) if err != nil { - return Metadata{}, clues.Wrap(err, "collection metadata") + return metadata.Metadata{}, clues.Wrap(err, "collection metadata") } return colMeta, nil @@ -79,83 +78,20 @@ func getCollectionMetadata( meta, err := fetchAndReadMetadata(ctx, dc, metaName) if err != nil { - return Metadata{}, clues.Wrap(err, "collection metadata") + return metadata.Metadata{}, clues.Wrap(err, "collection metadata") } return meta, nil } -// isSamePermission checks equality of two UserPermission objects -func isSamePermission(p1, p2 UserPermission) bool { - // EntityID can be empty for older backups and Email can be empty - // for newer ones. It is not possible for both to be empty. Also, - // if EntityID/Email for one is not empty then the other will also - // have EntityID/Email as we backup permissions for all the - // parents and children when we have a change in permissions. - if p1.EntityID != "" && p1.EntityID != p2.EntityID { - return false - } - - if p1.Email != "" && p1.Email != p2.Email { - return false - } - - p1r := p1.Roles - p2r := p2.Roles - - slices.Sort(p1r) - slices.Sort(p2r) - - return slices.Equal(p1r, p2r) -} - -func diffPermissions(before, after []UserPermission) ([]UserPermission, []UserPermission) { - var ( - added = []UserPermission{} - removed = []UserPermission{} - ) - - for _, cp := range after { - found := false - - for _, pp := range before { - if isSamePermission(cp, pp) { - found = true - break - } - } - - if !found { - added = append(added, cp) - } - } - - for _, pp := range before { - found := false - - for _, cp := range after { - if isSamePermission(cp, pp) { - found = true - break - } - } - - if !found { - removed = append(removed, pp) - } - } - - return added, removed -} - // computeParentPermissions computes the parent permissions by // traversing folderMetas and finding the first item with custom // permissions. folderMetas is expected to have all the parent // directory metas for this to work. -func computeParentPermissions(itemPath path.Path, folderMetas map[string]Metadata) (Metadata, error) { +func computeParentPermissions(itemPath path.Path, folderMetas map[string]metadata.Metadata) (metadata.Metadata, error) { var ( parent path.Path - meta Metadata + meta metadata.Metadata err error ok bool @@ -166,24 +102,24 @@ func computeParentPermissions(itemPath path.Path, folderMetas map[string]Metadat for { parent, err = parent.Dir() if err != nil { - return Metadata{}, clues.New("getting parent") + return metadata.Metadata{}, clues.New("getting parent") } onedrivePath, err := path.ToOneDrivePath(parent) if err != nil { - return Metadata{}, clues.New("get parent path") + return metadata.Metadata{}, clues.New("get parent path") } if len(onedrivePath.Folders) == 0 { - return Metadata{}, nil + return metadata.Metadata{}, nil } meta, ok = folderMetas[parent.String()] if !ok { - return Metadata{}, clues.New("no parent meta") + return metadata.Metadata{}, clues.New("no parent meta") } - if meta.SharingMode == SharingModeCustom { + if meta.SharingMode == metadata.SharingModeCustom { return meta, nil } } @@ -197,7 +133,7 @@ func UpdatePermissions( service graph.Servicer, driveID string, itemID string, - permAdded, permRemoved []UserPermission, + permAdded, permRemoved []metadata.Permission, permissionIDMappings map[string]string, ) error { // The ordering of the operations is important here. We first @@ -290,11 +226,11 @@ func RestorePermissions( driveID string, itemID string, itemPath path.Path, - meta Metadata, - folderMetas map[string]Metadata, + meta metadata.Metadata, + folderMetas map[string]metadata.Metadata, permissionIDMappings map[string]string, ) error { - if meta.SharingMode == SharingModeInherited { + if meta.SharingMode == metadata.SharingModeInherited { return nil } @@ -305,7 +241,7 @@ func RestorePermissions( return clues.Wrap(err, "parent permissions").WithClues(ctx) } - permAdded, permRemoved := diffPermissions(parentPermissions.Permissions, meta.Permissions) + permAdded, permRemoved := metadata.DiffPermissions(parentPermissions.Permissions, meta.Permissions) return UpdatePermissions(ctx, creds, service, driveID, itemID, permAdded, permRemoved, permissionIDMappings) } diff --git a/src/internal/connector/onedrive/permission_test.go b/src/internal/connector/onedrive/permission_test.go index 0483462b1..77c0d63ca 100644 --- a/src/internal/connector/onedrive/permission_test.go +++ b/src/internal/connector/onedrive/permission_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "github.com/alcionai/corso/src/internal/connector/onedrive/metadata" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/path" ) @@ -54,9 +55,9 @@ func (suite *PermissionsUnitTestSuite) TestComputeParentPermissions() { level0, err := level1.Dir() require.NoError(suite.T(), err, "level0 path") - metadata := Metadata{ - SharingMode: SharingModeCustom, - Permissions: []UserPermission{ + md := metadata.Metadata{ + SharingMode: metadata.SharingModeCustom, + Permissions: []metadata.Permission{ { Roles: []string{"write"}, EntityID: "user-id", @@ -64,9 +65,9 @@ func (suite *PermissionsUnitTestSuite) TestComputeParentPermissions() { }, } - metadata2 := Metadata{ - SharingMode: SharingModeCustom, - Permissions: []UserPermission{ + metadata2 := metadata.Metadata{ + SharingMode: metadata.SharingModeCustom, + Permissions: []metadata.Permission{ { Roles: []string{"read"}, EntityID: "user-id", @@ -74,52 +75,52 @@ func (suite *PermissionsUnitTestSuite) TestComputeParentPermissions() { }, } - inherited := Metadata{ - SharingMode: SharingModeInherited, - Permissions: []UserPermission{}, + inherited := metadata.Metadata{ + SharingMode: metadata.SharingModeInherited, + Permissions: []metadata.Permission{}, } table := []struct { name string item path.Path - meta Metadata - parentPerms map[string]Metadata + meta metadata.Metadata + parentPerms map[string]metadata.Metadata }{ { name: "root level entry", item: rootEntry, - meta: Metadata{}, - parentPerms: map[string]Metadata{}, + meta: metadata.Metadata{}, + parentPerms: map[string]metadata.Metadata{}, }, { name: "root level directory", item: level0, - meta: Metadata{}, - parentPerms: map[string]Metadata{}, + meta: metadata.Metadata{}, + parentPerms: map[string]metadata.Metadata{}, }, { name: "direct parent perms", item: entry, - meta: metadata, - parentPerms: map[string]Metadata{ - level2.String(): metadata, + meta: md, + parentPerms: map[string]metadata.Metadata{ + level2.String(): md, }, }, { name: "top level parent perms", item: entry, - meta: metadata, - parentPerms: map[string]Metadata{ + meta: md, + parentPerms: map[string]metadata.Metadata{ level2.String(): inherited, level1.String(): inherited, - level0.String(): metadata, + level0.String(): md, }, }, { name: "all inherited", item: entry, - meta: Metadata{}, - parentPerms: map[string]Metadata{ + meta: metadata.Metadata{}, + parentPerms: map[string]metadata.Metadata{ level2.String(): inherited, level1.String(): inherited, level0.String(): inherited, @@ -128,10 +129,10 @@ func (suite *PermissionsUnitTestSuite) TestComputeParentPermissions() { { name: "multiple custom permission", item: entry, - meta: metadata, - parentPerms: map[string]Metadata{ + meta: md, + parentPerms: map[string]metadata.Metadata{ level2.String(): inherited, - level1.String(): metadata, + level1.String(): md, level0.String(): metadata2, }, }, @@ -151,134 +152,3 @@ func (suite *PermissionsUnitTestSuite) TestComputeParentPermissions() { }) } } - -func (suite *PermissionsUnitTestSuite) TestDiffPermissions() { - perm1 := UserPermission{ - ID: "id1", - Roles: []string{"read"}, - EntityID: "user-id1", - } - - perm2 := UserPermission{ - ID: "id2", - Roles: []string{"write"}, - EntityID: "user-id2", - } - - perm3 := UserPermission{ - ID: "id3", - Roles: []string{"write"}, - EntityID: "user-id3", - } - - // The following two permissions have same id and user but - // different roles, this is a valid scenario for permissions. - sameidperm1 := UserPermission{ - ID: "id0", - Roles: []string{"write"}, - EntityID: "user-id0", - } - sameidperm2 := UserPermission{ - ID: "id0", - Roles: []string{"read"}, - EntityID: "user-id0", - } - - emailperm1 := UserPermission{ - ID: "id1", - Roles: []string{"read"}, - Email: "email1@provider.com", - } - - emailperm2 := UserPermission{ - ID: "id1", - Roles: []string{"read"}, - Email: "email2@provider.com", - } - - table := []struct { - name string - before []UserPermission - after []UserPermission - added []UserPermission - removed []UserPermission - }{ - { - name: "single permission added", - before: []UserPermission{}, - after: []UserPermission{perm1}, - added: []UserPermission{perm1}, - removed: []UserPermission{}, - }, - { - name: "single permission removed", - before: []UserPermission{perm1}, - after: []UserPermission{}, - added: []UserPermission{}, - removed: []UserPermission{perm1}, - }, - { - name: "multiple permission added", - before: []UserPermission{}, - after: []UserPermission{perm1, perm2}, - added: []UserPermission{perm1, perm2}, - removed: []UserPermission{}, - }, - { - name: "single permission removed", - before: []UserPermission{perm1, perm2}, - after: []UserPermission{}, - added: []UserPermission{}, - removed: []UserPermission{perm1, perm2}, - }, - { - name: "extra permissions", - before: []UserPermission{perm1, perm2}, - after: []UserPermission{perm1, perm2, perm3}, - added: []UserPermission{perm3}, - removed: []UserPermission{}, - }, - { - name: "less permissions", - before: []UserPermission{perm1, perm2, perm3}, - after: []UserPermission{perm1, perm2}, - added: []UserPermission{}, - removed: []UserPermission{perm3}, - }, - { - name: "same id different role", - before: []UserPermission{sameidperm1}, - after: []UserPermission{sameidperm2}, - added: []UserPermission{sameidperm2}, - removed: []UserPermission{sameidperm1}, - }, - { - name: "email based extra permissions", - before: []UserPermission{emailperm1}, - after: []UserPermission{emailperm1, emailperm2}, - added: []UserPermission{emailperm2}, - removed: []UserPermission{}, - }, - { - name: "email based less permissions", - before: []UserPermission{emailperm1, emailperm2}, - after: []UserPermission{emailperm1}, - added: []UserPermission{}, - removed: []UserPermission{emailperm2}, - }, - } - - for _, test := range table { - suite.Run(test.name, func() { - _, flush := tester.NewContext() - defer flush() - - t := suite.T() - - added, removed := diffPermissions(test.before, test.after) - - assert.Equal(t, added, test.added, "added permissions") - assert.Equal(t, removed, test.removed, "removed permissions") - }) - } -} diff --git a/src/internal/connector/onedrive/restore.go b/src/internal/connector/onedrive/restore.go index 3a3e137d4..5cfa27861 100644 --- a/src/internal/connector/onedrive/restore.go +++ b/src/internal/connector/onedrive/restore.go @@ -48,7 +48,7 @@ func RestoreCollections( var ( restoreMetrics support.CollectionMetrics metrics support.CollectionMetrics - folderMetas = map[string]Metadata{} + folderMetas = map[string]metadata.Metadata{} // permissionIDMappings is used to map between old and new id // of permissions as we restore them @@ -132,7 +132,7 @@ func RestoreCollection( backupVersion int, service graph.Servicer, dc data.RestoreCollection, - folderMetas map[string]Metadata, + folderMetas map[string]metadata.Metadata, permissionIDMappings map[string]string, fc *folderCache, rootIDCache map[string]string, // map of drive id -> root folder ID @@ -298,7 +298,7 @@ func restoreItem( drivePath *path.DrivePath, restoreFolderID string, copyBuffer []byte, - folderMetas map[string]Metadata, + folderMetas map[string]metadata.Metadata, permissionIDMappings map[string]string, restorePerms bool, itemData data.Stream, @@ -439,7 +439,7 @@ func restoreV1File( restoreFolderID string, copyBuffer []byte, restorePerms bool, - folderMetas map[string]Metadata, + folderMetas map[string]metadata.Metadata, permissionIDMappings map[string]string, itemPath path.Path, itemData data.Stream, @@ -500,7 +500,7 @@ func restoreV6File( restoreFolderID string, copyBuffer []byte, restorePerms bool, - folderMetas map[string]Metadata, + folderMetas map[string]metadata.Metadata, permissionIDMappings map[string]string, itemPath path.Path, itemData data.Stream, @@ -575,8 +575,8 @@ func createRestoreFoldersWithPermissions( driveRootID string, restoreFolders *path.Builder, folderPath path.Path, - folderMetadata Metadata, - folderMetas map[string]Metadata, + folderMetadata metadata.Metadata, + folderMetas map[string]metadata.Metadata, fc *folderCache, permissionIDMappings map[string]string, restorePerms bool, @@ -741,11 +741,11 @@ func fetchAndReadMetadata( ctx context.Context, fetcher fileFetcher, metaName string, -) (Metadata, error) { +) (metadata.Metadata, error) { metaFile, err := fetcher.Fetch(ctx, metaName) if err != nil { err = clues.Wrap(err, "getting item metadata").With("meta_file_name", metaName) - return Metadata{}, err + return metadata.Metadata{}, err } metaReader := metaFile.ToReader() @@ -754,25 +754,25 @@ func fetchAndReadMetadata( meta, err := getMetadata(metaReader) if err != nil { err = clues.Wrap(err, "deserializing item metadata").With("meta_file_name", metaName) - return Metadata{}, err + return metadata.Metadata{}, err } return meta, nil } // getMetadata read and parses the metadata info for an item -func getMetadata(metar io.ReadCloser) (Metadata, error) { - var meta Metadata +func getMetadata(metar io.ReadCloser) (metadata.Metadata, error) { + var meta metadata.Metadata // `metar` will be nil for the top level container folder if metar != nil { metaraw, err := io.ReadAll(metar) if err != nil { - return Metadata{}, err + return metadata.Metadata{}, err } err = json.Unmarshal(metaraw, &meta) if err != nil { - return Metadata{}, err + return metadata.Metadata{}, err } } diff --git a/src/internal/connector/sharepoint/restore.go b/src/internal/connector/sharepoint/restore.go index 9bd6a82ff..1c06e8ae3 100644 --- a/src/internal/connector/sharepoint/restore.go +++ b/src/internal/connector/sharepoint/restore.go @@ -12,6 +12,7 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/onedrive" + "github.com/alcionai/corso/src/internal/connector/onedrive/metadata" "github.com/alcionai/corso/src/internal/connector/sharepoint/api" "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" @@ -74,7 +75,7 @@ func RestoreCollections( backupVersion, service, dc, - map[string]onedrive.Metadata{}, // Currently permission data is not stored for sharepoint + map[string]metadata.Metadata{}, // Currently permission data is not stored for sharepoint map[string]string{}, driveFolderCache, nil, diff --git a/src/internal/operations/backup_integration_test.go b/src/internal/operations/backup_integration_test.go index ed2afb3d3..38a28ac86 100644 --- a/src/internal/operations/backup_integration_test.go +++ b/src/internal/operations/backup_integration_test.go @@ -27,6 +27,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/mock" "github.com/alcionai/corso/src/internal/connector/onedrive" + "github.com/alcionai/corso/src/internal/connector/onedrive/metadata" "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/events" @@ -1168,11 +1169,13 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { runDriveIncrementalTest( suite, suite.user, + suite.user, connector.Users, path.OneDriveService, path.FilesCategory, ic, - gtdi) + gtdi, + false) } func (suite *BackupOpIntegrationSuite) TestBackup_Run_sharePointIncrementals() { @@ -1205,21 +1208,24 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_sharePointIncrementals() { runDriveIncrementalTest( suite, suite.site, + suite.user, connector.Sites, path.SharePointService, path.LibrariesCategory, ic, - gtdi) + gtdi, + true) } func runDriveIncrementalTest( suite *BackupOpIntegrationSuite, - owner string, + owner, permissionsUser string, resource connector.Resource, service path.ServiceType, category path.CategoryType, includeContainers func([]string) selectors.Selector, getTestDriveID func(*testing.T, context.Context, graph.Servicer) string, + skipPermissionsTests bool, ) { ctx, flush := tester.NewContext() defer flush() @@ -1310,10 +1316,10 @@ func runDriveIncrementalTest( newFileName = "new_file.txt" permissionIDMappings = map[string]string{} - writePerm = onedrive.UserPermission{ + writePerm = metadata.Permission{ ID: "perm-id", Roles: []string{"write"}, - EntityID: owner, + EntityID: permissionsUser, } ) @@ -1348,14 +1354,14 @@ func runDriveIncrementalTest( driveID, targetContainer, driveItem) - require.NoError(t, err, "creating new file", clues.ToCore(err)) + require.NoErrorf(t, err, "creating new file %v", clues.ToCore(err)) }, itemsRead: 1, // .data file for newitem itemsWritten: 3, // .data and .meta for newitem, .dirmeta for parent }, { name: "add permission to new file", - skip: service == path.SharePointService, + skip: skipPermissionsTests, updateFiles: func(t *testing.T) { driveItem := models.NewDriveItem() driveItem.SetName(&newFileName) @@ -1366,18 +1372,17 @@ func runDriveIncrementalTest( gc.Service, driveID, *newFile.GetId(), - []onedrive.UserPermission{writePerm}, - []onedrive.UserPermission{}, - permissionIDMappings, - ) - require.NoErrorf(t, err, "add permission to file %v", clues.ToCore(err)) + []metadata.Permission{writePerm}, + []metadata.Permission{}, + permissionIDMappings) + require.NoErrorf(t, err, "adding permission to file %v", clues.ToCore(err)) }, itemsRead: 1, // .data file for newitem itemsWritten: 2, // .meta for newitem, .dirmeta for parent (.data is not written as it is not updated) }, { name: "remove permission from new file", - skip: service == path.SharePointService, + skip: skipPermissionsTests, updateFiles: func(t *testing.T) { driveItem := models.NewDriveItem() driveItem.SetName(&newFileName) @@ -1388,18 +1393,17 @@ func runDriveIncrementalTest( gc.Service, driveID, *newFile.GetId(), - []onedrive.UserPermission{}, - []onedrive.UserPermission{writePerm}, - permissionIDMappings, - ) - require.NoError(t, err, "add permission to file", clues.ToCore(err)) + []metadata.Permission{}, + []metadata.Permission{writePerm}, + permissionIDMappings) + require.NoErrorf(t, err, "adding permission to file %v", clues.ToCore(err)) }, itemsRead: 1, // .data file for newitem itemsWritten: 2, // .meta for newitem, .dirmeta for parent (.data is not written as it is not updated) }, { name: "add permission to container", - skip: service == path.SharePointService, + skip: skipPermissionsTests, updateFiles: func(t *testing.T) { targetContainer := containerIDs[container1] driveItem := models.NewDriveItem() @@ -1411,18 +1415,17 @@ func runDriveIncrementalTest( gc.Service, driveID, targetContainer, - []onedrive.UserPermission{writePerm}, - []onedrive.UserPermission{}, - permissionIDMappings, - ) - require.NoError(t, err, "add permission to file", clues.ToCore(err)) + []metadata.Permission{writePerm}, + []metadata.Permission{}, + permissionIDMappings) + require.NoErrorf(t, err, "adding permission to file %v", clues.ToCore(err)) }, itemsRead: 0, itemsWritten: 1, // .dirmeta for collection }, { name: "remove permission from container", - skip: service == path.SharePointService, + skip: skipPermissionsTests, updateFiles: func(t *testing.T) { targetContainer := containerIDs[container1] driveItem := models.NewDriveItem() @@ -1434,11 +1437,10 @@ func runDriveIncrementalTest( gc.Service, driveID, targetContainer, - []onedrive.UserPermission{}, - []onedrive.UserPermission{writePerm}, - permissionIDMappings, - ) - require.NoError(t, err, "add permission to file", clues.ToCore(err)) + []metadata.Permission{}, + []metadata.Permission{writePerm}, + permissionIDMappings) + require.NoErrorf(t, err, "adding permission to file %v", clues.ToCore(err)) }, itemsRead: 0, itemsWritten: 1, // .dirmeta for collection @@ -1452,7 +1454,7 @@ func runDriveIncrementalTest( ItemsById(ptr.Val(newFile.GetId())). Content(). Put(ctx, []byte("new content"), nil) - require.NoError(t, err, "updating file content") + require.NoErrorf(t, err, "updating file contents: %v", clues.ToCore(err)) }, itemsRead: 1, // .data file for newitem itemsWritten: 3, // .data and .meta for newitem, .dirmeta for parent @@ -1474,7 +1476,7 @@ func runDriveIncrementalTest( DrivesById(driveID). ItemsById(ptr.Val(newFile.GetId())). Patch(ctx, driveItem, nil) - require.NoError(t, err, "renaming file", clues.ToCore(err)) + require.NoError(t, err, "renaming file %v", clues.ToCore(err)) }, itemsRead: 1, // .data file for newitem itemsWritten: 3, // .data and .meta for newitem, .dirmeta for parent @@ -1495,7 +1497,7 @@ func runDriveIncrementalTest( DrivesById(driveID). ItemsById(ptr.Val(newFile.GetId())). Patch(ctx, driveItem, nil) - require.NoError(t, err, "moving file between folders", clues.ToCore(err)) + require.NoErrorf(t, err, "moving file between folders %v", clues.ToCore(err)) }, itemsRead: 1, // .data file for newitem itemsWritten: 3, // .data and .meta for newitem, .dirmeta for parent @@ -1510,7 +1512,7 @@ func runDriveIncrementalTest( DrivesById(driveID). ItemsById(ptr.Val(newFile.GetId())). Delete(ctx, nil) - require.NoError(t, err, "deleting file", clues.ToCore(err)) + require.NoErrorf(t, err, "deleting file %v", clues.ToCore(err)) }, itemsRead: 0, itemsWritten: 0, @@ -1609,9 +1611,8 @@ func runDriveIncrementalTest( } for _, test := range table { suite.Run(test.name, func() { - // TODO(rkeepers): remove when sharepoint supports permission. if test.skip { - return + suite.T().Skip("flagged to skip") } cleanGC, err := connector.NewGraphConnector(ctx, acct, resource)