diff --git a/src/internal/m365/collection/drive/permission.go b/src/internal/m365/collection/drive/permission.go index 4125231c6..78ac06793 100644 --- a/src/internal/m365/collection/drive/permission.go +++ b/src/internal/m365/collection/drive/permission.go @@ -12,7 +12,9 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/m365/collection/drive/metadata" + "github.com/alcionai/corso/src/internal/m365/graph" "github.com/alcionai/corso/src/internal/version" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" ) @@ -193,7 +195,10 @@ func UpdatePermissions( itemID string, permAdded, permRemoved []metadata.Permission, oldPermIDToNewID *xsync.MapOf[string, string], + errs *fault.Bus, ) error { + el := errs.Local() + // The ordering of the operations is important here. We first // remove all the removed permissions and then add the added ones. for _, p := range permRemoved { @@ -223,6 +228,10 @@ func UpdatePermissions( } for _, p := range permAdded { + if el.Failure() != nil { + break + } + ictx := clues.Add( ctx, "permission_entity_type", p.EntityType, @@ -267,14 +276,20 @@ func UpdatePermissions( pbody.SetRecipients([]models.DriveRecipientable{rec}) newPerm, err := udip.PostItemPermissionUpdate(ictx, driveID, itemID, pbody) + if graph.IsErrUsersCannotBeResolved(err) { + logger.CtxErr(ictx, err).Info("Unable to restore link share") + continue + } + if err != nil { - return clues.Stack(err) + el.AddRecoverable(ictx, clues.Stack(err)) + continue } oldPermIDToNewID.Store(p.ID, ptr.Val(newPerm.GetValue()[0].GetId())) } - return nil + return el.Failure() } type updateDeleteItemLinkSharer interface { @@ -289,14 +304,20 @@ func UpdateLinkShares( itemID string, lsAdded, lsRemoved []metadata.LinkShare, oldLinkShareIDToNewID *xsync.MapOf[string, string], + errs *fault.Bus, ) (bool, error) { // You can only delete inherited sharing links the first time you // create a sharing link which is done using // `retainInheritedPermissions`. We cannot separately delete any // inherited link shares via DELETE API call like for permissions. alreadyDeleted := false + el := errs.Local() for _, ls := range lsAdded { + if el.Failure() != nil { + break + } + ictx := clues.Add(ctx, "link_share_id", ls.ID) // Links with password are not shared with a specific user @@ -363,13 +384,23 @@ func UpdateLinkShares( } newLS, err := upils.PostItemLinkShareUpdate(ictx, driveID, itemID, lsbody) + if graph.IsErrUsersCannotBeResolved(err) { + logger.CtxErr(ictx, err).Info("Unable to restore link share") + continue + } + if err != nil { - return alreadyDeleted, clues.Stack(err) + el.AddRecoverable(ctx, clues.Stack(err)) + continue } oldLinkShareIDToNewID.Store(ls.ID, ptr.Val(newLS.GetId())) } + if el.Failure() != nil { + return alreadyDeleted, el.Failure() + } + // It is possible to have empty link shares even though we should // have inherited one if the user creates a link using // `retainInheritedPermissions` as false, but then deleted it. We @@ -411,6 +442,7 @@ func RestorePermissions( itemPath path.Path, current metadata.Metadata, caches *restoreCaches, + errs *fault.Bus, ) error { if current.SharingMode == metadata.SharingModeInherited { return nil @@ -435,7 +467,8 @@ func RestorePermissions( itemID, lsAdded, lsRemoved, - caches.OldLinkShareIDToNewID) + caches.OldLinkShareIDToNewID, + errs) if err != nil { return clues.Wrap(err, "updating link shares") } @@ -464,7 +497,8 @@ func RestorePermissions( itemID, permAdded, permRemoved, - caches.OldPermIDToNewID) + caches.OldPermIDToNewID, + errs) if err != nil { return clues.Wrap(err, "updating permissions") } diff --git a/src/internal/m365/collection/drive/restore.go b/src/internal/m365/collection/drive/restore.go index 544808d2e..7a9017744 100644 --- a/src/internal/m365/collection/drive/restore.go +++ b/src/internal/m365/collection/drive/restore.go @@ -130,7 +130,8 @@ func RestoreCollection( dc.FullPath(), colMeta, caches, - rcc.RestoreConfig.IncludePermissions) + rcc.RestoreConfig.IncludePermissions, + errs) if err != nil { return metrics, clues.Wrap(err, "creating folders for restore") } @@ -211,7 +212,8 @@ func RestoreCollection( caches, itemData, itemPath, - ctr) + ctr, + errs) // skipped items don't get counted, but they can error if !skipped { @@ -260,6 +262,7 @@ func restoreItem( itemData data.Item, itemPath path.Path, ctr *count.Bus, + errs *fault.Bus, ) (details.ItemInfo, bool, error) { itemUUID := itemData.ID() ctx = clues.Add(ctx, "item_id", itemUUID) @@ -332,7 +335,8 @@ func restoreItem( caches, itemPath, itemData, - ctr) + ctr, + errs) if err != nil { if errors.Is(err, graph.ErrItemAlreadyExistsConflict) && rcc.RestoreConfig.OnCollision == control.Skip { return details.ItemInfo{}, true, nil @@ -357,7 +361,8 @@ func restoreItem( caches, itemPath, itemData, - ctr) + ctr, + errs) if err != nil { if errors.Is(err, graph.ErrItemAlreadyExistsConflict) && rcc.RestoreConfig.OnCollision == control.Skip { return details.ItemInfo{}, true, nil @@ -412,6 +417,7 @@ func restoreV1File( itemPath path.Path, itemData data.Item, ctr *count.Bus, + errs *fault.Bus, ) (details.ItemInfo, error) { trimmedName := strings.TrimSuffix(itemData.ID(), metadata.DataFileSuffix) @@ -452,7 +458,8 @@ func restoreV1File( itemID, itemPath, meta, - caches) + caches, + errs) if err != nil { return details.ItemInfo{}, clues.Wrap(err, "restoring item permissions") } @@ -472,6 +479,7 @@ func restoreV6File( itemPath path.Path, itemData data.Item, ctr *count.Bus, + errs *fault.Bus, ) (details.ItemInfo, error) { trimmedName := strings.TrimSuffix(itemData.ID(), metadata.DataFileSuffix) @@ -528,7 +536,8 @@ func restoreV6File( itemID, itemPath, meta, - caches) + caches, + errs) if err != nil { return details.ItemInfo{}, clues.Wrap(err, "restoring item permissions") } @@ -550,6 +559,7 @@ func CreateRestoreFolders( folderMetadata metadata.Metadata, caches *restoreCaches, restorePerms bool, + errs *fault.Bus, ) (string, error) { id, err := createRestoreFolders( ctx, @@ -577,7 +587,8 @@ func CreateRestoreFolders( id, folderPath, folderMetadata, - caches) + caches, + errs) return id, err } diff --git a/src/internal/m365/collection/drive/restore_test.go b/src/internal/m365/collection/drive/restore_test.go index 731ccef77..00aff96e0 100644 --- a/src/internal/m365/collection/drive/restore_test.go +++ b/src/internal/m365/collection/drive/restore_test.go @@ -23,6 +23,7 @@ import ( "github.com/alcionai/corso/src/internal/version" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" apiMock "github.com/alcionai/corso/src/pkg/services/m365/api/mock" @@ -246,7 +247,8 @@ func (suite *RestoreUnitSuite) TestRestoreItem_collisionHandling() { ItemInfo: odStub.DriveItemInfo(), }, nil, - ctr) + ctr, + fault.New(true)) require.NoError(t, err, clues.ToCore(err)) test.expectSkipped(t, skip) diff --git a/src/internal/m365/graph/errors.go b/src/internal/m365/graph/errors.go index 958ecf639..e67198011 100644 --- a/src/internal/m365/graph/errors.go +++ b/src/internal/m365/graph/errors.go @@ -49,6 +49,7 @@ const ( // nameAlreadyExists occurs when a request with // @microsoft.graph.conflictBehavior=fail finds a conflicting file. nameAlreadyExists errorCode = "nameAlreadyExists" + noResolvedUsers errorCode = "noResolvedUsers" QuotaExceeded errorCode = "ErrorQuotaExceeded" RequestResourceNotFound errorCode = "Request_ResourceNotFound" // Returned when we try to get the inbox of a user that doesn't exist. @@ -62,10 +63,11 @@ const ( type errorMessage string const ( - IOErrDuringRead errorMessage = "IO error during request payload read" - MysiteURLNotFound errorMessage = "unable to retrieve user's mysite url" - MysiteNotFound errorMessage = "user's mysite not found" - NoSPLicense errorMessage = "Tenant does not have a SPO license" + IOErrDuringRead errorMessage = "IO error during request payload read" + MysiteURLNotFound errorMessage = "unable to retrieve user's mysite url" + MysiteNotFound errorMessage = "user's mysite not found" + NoSPLicense errorMessage = "Tenant does not have a SPO license" + usersCannotBeResolved errorMessage = "One or more users could not be resolved" ) const ( @@ -225,6 +227,10 @@ func IsErrFolderExists(err error) bool { return hasErrorCode(err, folderExists) } +func IsErrUsersCannotBeResolved(err error) bool { + return hasErrorCode(err, noResolvedUsers) || hasErrorMessage(err, usersCannotBeResolved) +} + // --------------------------------------------------------------------------- // error parsers // --------------------------------------------------------------------------- @@ -252,6 +258,30 @@ func hasErrorCode(err error, codes ...errorCode) bool { return filters.Equal(cs).Compare(code) } +// only use this as a last resort. Prefer the code or statuscode if possible. +func hasErrorMessage(err error, msgs ...errorMessage) bool { + if err == nil { + return false + } + + var oDataError odataerrors.ODataErrorable + if !errors.As(err, &oDataError) { + return false + } + + msg, ok := ptr.ValOK(oDataError.GetErrorEscaped().GetMessage()) + if !ok { + return false + } + + cs := make([]string, len(msgs)) + for i, c := range msgs { + cs[i] = string(c) + } + + return filters.Equal(cs).Compare(msg) +} + // Wrap is a helper function that extracts ODataError metadata from // the error. If the error is not an ODataError type, returns the error. func Wrap(ctx context.Context, e error, msg string) *clues.Err { diff --git a/src/internal/m365/graph/errors_test.go b/src/internal/m365/graph/errors_test.go index d3690f798..8f043074b 100644 --- a/src/internal/m365/graph/errors_test.go +++ b/src/internal/m365/graph/errors_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "net/http" + "strings" "syscall" "testing" @@ -466,7 +467,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrFolderExists() { expect: assert.False, }, { - name: "matching oDataErr", + name: "matching oDataErr msg", err: odErr(string(folderExists)), expect: assert.True, }, @@ -489,6 +490,56 @@ func (suite *GraphErrorsUnitSuite) TestIsErrFolderExists() { } } +func (suite *GraphErrorsUnitSuite) TestIsErrUsersCannotBeResolved() { + table := []struct { + name string + err error + expect assert.BoolAssertionFunc + }{ + { + name: "nil", + err: nil, + expect: assert.False, + }, + { + name: "non-matching", + err: assert.AnError, + expect: assert.False, + }, + { + name: "non-matching oDataErr", + err: odErrMsg("InvalidRequest", "cant resolve users"), + expect: assert.False, + }, + { + name: "matching oDataErr code", + err: odErrMsg(string(noResolvedUsers), "usersCannotBeResolved"), + expect: assert.True, + }, + { + name: "matching oDataErr msg", + err: odErrMsg("InvalidRequest", string(usersCannotBeResolved)), + expect: assert.True, + }, + // next two tests are to make sure the checks are case insensitive + { + name: "oDataErr uppercase", + err: odErrMsg("InvalidRequest", strings.ToUpper(string(usersCannotBeResolved))), + expect: assert.True, + }, + { + name: "oDataErr lowercase", + err: odErrMsg("InvalidRequest", strings.ToLower(string(usersCannotBeResolved))), + expect: assert.True, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + test.expect(suite.T(), IsErrUsersCannotBeResolved(test.err)) + }) + } +} + func (suite *GraphErrorsUnitSuite) TestIsErrCannotOpenFileAttachment() { table := []struct { name string diff --git a/src/internal/operations/test/onedrive_test.go b/src/internal/operations/test/onedrive_test.go index ba32fd485..db058f5af 100644 --- a/src/internal/operations/test/onedrive_test.go +++ b/src/internal/operations/test/onedrive_test.go @@ -393,7 +393,8 @@ func runDriveIncrementalTest( ptr.Val(newFile.GetId()), []metadata.Permission{writePerm}, []metadata.Permission{}, - permissionIDMappings) + permissionIDMappings, + fault.New(true)) require.NoErrorf(t, err, "adding permission to file %v", clues.ToCore(err)) // no expectedDeets: metadata isn't tracked }, @@ -411,7 +412,8 @@ func runDriveIncrementalTest( *newFile.GetId(), []metadata.Permission{}, []metadata.Permission{writePerm}, - permissionIDMappings) + permissionIDMappings, + fault.New(true)) require.NoErrorf(t, err, "removing permission from file %v", clues.ToCore(err)) // no expectedDeets: metadata isn't tracked }, @@ -430,7 +432,8 @@ func runDriveIncrementalTest( targetContainer, []metadata.Permission{writePerm}, []metadata.Permission{}, - permissionIDMappings) + permissionIDMappings, + fault.New(true)) require.NoErrorf(t, err, "adding permission to container %v", clues.ToCore(err)) // no expectedDeets: metadata isn't tracked }, @@ -449,7 +452,8 @@ func runDriveIncrementalTest( targetContainer, []metadata.Permission{}, []metadata.Permission{writePerm}, - permissionIDMappings) + permissionIDMappings, + fault.New(true)) require.NoErrorf(t, err, "removing permission from container %v", clues.ToCore(err)) // no expectedDeets: metadata isn't tracked },