diff --git a/CHANGELOG.md b/CHANGELOG.md index 862a58f08..0e8c12dfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Enables local or network-attached storage for Corso repositories. - Reduce backup runtime for OneDrive and SharePoint incremental backups that have no file changes. - Increase Exchange backup performance by lazily fetching data only for items whose content changed. +- Added `--backups` flag to delete multiple backups in `corso backup delete` command. + +## Fixed +- Teams Channels that cannot support delta tokens (those without messages) fall back to non-delta enumeration and no longer fail a backup. ## [v0.13.0] (beta) - 2023-09-18 diff --git a/src/cli/backup/backup.go b/src/cli/backup/backup.go index a79a8afb2..71cb4595c 100644 --- a/src/cli/backup/backup.go +++ b/src/cli/backup/backup.go @@ -252,8 +252,8 @@ func runBackups( func genericDeleteCommand( cmd *cobra.Command, pst path.ServiceType, - bID, designation string, - args []string, + designation string, + bID, args []string, ) error { if utils.HasNoFlagsAndShownHelp(cmd) { return nil @@ -265,21 +265,18 @@ func genericDeleteCommand( ctx := clues.Add(cmd.Context(), "delete_backup_id", bID) - r, _, _, _, err := utils.GetAccountAndConnectWithOverrides( - ctx, - cmd, - pst) + r, _, err := utils.GetAccountAndConnect(ctx, cmd, pst) if err != nil { return Only(ctx, err) } defer utils.CloseRepo(ctx, r) - if err := r.DeleteBackups(ctx, true, bID); err != nil { - return Only(ctx, clues.Wrap(err, "Deleting backup "+bID)) + if err := r.DeleteBackups(ctx, true, bID...); err != nil { + return Only(ctx, clues.Wrap(err, fmt.Sprintf("Deleting backup %v", bID))) } - Infof(ctx, "Deleted %s backup %s", designation, bID) + Infof(ctx, "Deleted %s backup %v", designation, bID) return nil } @@ -298,10 +295,7 @@ func genericListCommand( return nil } - r, _, _, _, err := utils.GetAccountAndConnectWithOverrides( - ctx, - cmd, - service) + r, _, err := utils.GetAccountAndConnect(ctx, cmd, service) if err != nil { return Only(ctx, err) } diff --git a/src/cli/backup/exchange.go b/src/cli/backup/exchange.go index 4a37c032b..d4f0d9534 100644 --- a/src/cli/backup/exchange.go +++ b/src/cli/backup/exchange.go @@ -32,7 +32,7 @@ const ( const ( exchangeServiceCommand = "exchange" exchangeServiceCommandCreateUseSuffix = "--mailbox | '" + flags.Wildcard + "'" - exchangeServiceCommandDeleteUseSuffix = "--backup " + exchangeServiceCommandDeleteUseSuffix = "--backups " exchangeServiceCommandDetailsUseSuffix = "--backup " ) @@ -46,8 +46,9 @@ corso backup create exchange --mailbox alice@example.com,bob@example.com --data # Backup all Exchange data for all M365 users corso backup create exchange --mailbox '*'` - exchangeServiceCommandDeleteExamples = `# Delete Exchange backup with ID 1234abcd-12ab-cd34-56de-1234abcd -corso backup delete exchange --backup 1234abcd-12ab-cd34-56de-1234abcd` + exchangeServiceCommandDeleteExamples = `# Delete Exchange backup with IDs 1234abcd-12ab-cd34-56de-1234abcd \ +and 1234abcd-12ab-cd34-56de-1234abce +corso backup delete exchange --backups 1234abcd-12ab-cd34-56de-1234abcd,1234abcd-12ab-cd34-56de-1234abce` exchangeServiceCommandDetailsExamples = `# Explore items in Alice's latest backup (1234abcd...) corso backup details exchange --backup 1234abcd-12ab-cd34-56de-1234abcd @@ -121,7 +122,8 @@ func addExchangeCommands(cmd *cobra.Command) *cobra.Command { c.Use = c.Use + " " + exchangeServiceCommandDeleteUseSuffix c.Example = exchangeServiceCommandDeleteExamples - flags.AddBackupIDFlag(c, true) + flags.AddMultipleBackupIDsFlag(c, false) + flags.AddBackupIDFlag(c, false) } return c @@ -273,17 +275,19 @@ func detailsExchangeCmd(cmd *cobra.Command, args []string) error { ctx := cmd.Context() opts := utils.MakeExchangeOpts(cmd) - r, _, _, ctrlOpts, err := utils.GetAccountAndConnectWithOverrides( - ctx, - cmd, - path.ExchangeService) + r, rdao, err := utils.GetAccountAndConnect(ctx, cmd, path.ExchangeService) if err != nil { return Only(ctx, err) } defer utils.CloseRepo(ctx, r) - ds, err := runDetailsExchangeCmd(ctx, r, flags.BackupIDFV, opts, ctrlOpts.SkipReduce) + ds, err := runDetailsExchangeCmd( + ctx, + r, + flags.BackupIDFV, + opts, + rdao.Opts.SkipReduce) if err != nil { return Only(ctx, err) } @@ -352,5 +356,15 @@ func exchangeDeleteCmd() *cobra.Command { // deletes an exchange service backup. func deleteExchangeCmd(cmd *cobra.Command, args []string) error { - return genericDeleteCommand(cmd, path.ExchangeService, flags.BackupIDFV, "Exchange", args) + var backupIDValue []string + + if len(flags.BackupIDsFV) > 0 { + backupIDValue = flags.BackupIDsFV + } else if len(flags.BackupIDFV) > 0 { + backupIDValue = append(backupIDValue, flags.BackupIDFV) + } else { + return clues.New("either --backup or --backups flag is required") + } + + return genericDeleteCommand(cmd, path.ExchangeService, "Exchange", backupIDValue, args) } diff --git a/src/cli/backup/exchange_e2e_test.go b/src/cli/backup/exchange_e2e_test.go index 992787997..2175a50e6 100644 --- a/src/cli/backup/exchange_e2e_test.go +++ b/src/cli/backup/exchange_e2e_test.go @@ -561,8 +561,8 @@ func runExchangeDetailsCmdTest(suite *PreparedBackupExchangeE2ESuite, category p type BackupDeleteExchangeE2ESuite struct { tester.Suite - dpnd dependencies - backupOp operations.BackupOperation + dpnd dependencies + backupOps [3]operations.BackupOperation } func TestBackupDeleteExchangeE2ESuite(t *testing.T) { @@ -588,13 +588,15 @@ func (suite *BackupDeleteExchangeE2ESuite) SetupSuite() { sel := selectors.NewExchangeBackup(users) sel.Include(sel.MailFolders([]string{api.MailInbox}, selectors.PrefixMatch())) - backupOp, err := suite.dpnd.repo.NewBackup(ctx, sel.Selector) - require.NoError(t, err, clues.ToCore(err)) + for i := 0; i < cap(suite.backupOps); i++ { + backupOp, err := suite.dpnd.repo.NewBackup(ctx, sel.Selector) + require.NoError(t, err, clues.ToCore(err)) - suite.backupOp = backupOp + suite.backupOps[i] = backupOp - err = suite.backupOp.Run(ctx) - require.NoError(t, err, clues.ToCore(err)) + err = suite.backupOps[i].Run(ctx) + require.NoError(t, err, clues.ToCore(err)) + } } func (suite *BackupDeleteExchangeE2ESuite) TestExchangeBackupDeleteCmd() { @@ -608,7 +610,10 @@ func (suite *BackupDeleteExchangeE2ESuite) TestExchangeBackupDeleteCmd() { cmd := cliTD.StubRootCmd( "backup", "delete", "exchange", "--config-file", suite.dpnd.configFilePath, - "--"+flags.BackupFN, string(suite.backupOp.Results.BackupID)) + "--"+flags.BackupIDsFN, + fmt.Sprintf("%s,%s", + string(suite.backupOps[0].Results.BackupID), + string(suite.backupOps[1].Results.BackupID))) cli.BuildCommandTree(cmd) // run the command @@ -619,7 +624,47 @@ func (suite *BackupDeleteExchangeE2ESuite) TestExchangeBackupDeleteCmd() { cmd = cliTD.StubRootCmd( "backup", "details", "exchange", "--config-file", suite.dpnd.configFilePath, - "--backup", string(suite.backupOp.Results.BackupID)) + "--backup", string(suite.backupOps[0].Results.BackupID)) + cli.BuildCommandTree(cmd) + + err = cmd.ExecuteContext(ctx) + require.Error(t, err, clues.ToCore(err)) + + // a follow-up details call should fail, due to the backup ID being deleted + cmd = cliTD.StubRootCmd( + "backup", "details", "exchange", + "--config-file", suite.dpnd.configFilePath, + "--backup", string(suite.backupOps[1].Results.BackupID)) + cli.BuildCommandTree(cmd) + + err = cmd.ExecuteContext(ctx) + require.Error(t, err, clues.ToCore(err)) +} + +func (suite *BackupDeleteExchangeE2ESuite) TestExchangeBackupDeleteCmd_SingleID() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + ctx = config.SetViper(ctx, suite.dpnd.vpr) + + defer flush() + + cmd := cliTD.StubRootCmd( + "backup", "delete", "exchange", + "--config-file", suite.dpnd.configFilePath, + "--"+flags.BackupFN, + string(suite.backupOps[2].Results.BackupID)) + cli.BuildCommandTree(cmd) + + // run the command + err := cmd.ExecuteContext(ctx) + require.NoError(t, err, clues.ToCore(err)) + + // a follow-up details call should fail, due to the backup ID being deleted + cmd = cliTD.StubRootCmd( + "backup", "details", "exchange", + "--config-file", suite.dpnd.configFilePath, + "--backup", string(suite.backupOps[2].Results.BackupID)) cli.BuildCommandTree(cmd) err = cmd.ExecuteContext(ctx) @@ -637,10 +682,28 @@ func (suite *BackupDeleteExchangeE2ESuite) TestExchangeBackupDeleteCmd_UnknownID cmd := cliTD.StubRootCmd( "backup", "delete", "exchange", "--config-file", suite.dpnd.configFilePath, - "--"+flags.BackupFN, uuid.NewString()) + "--"+flags.BackupIDsFN, uuid.NewString()) cli.BuildCommandTree(cmd) // unknown backupIDs should error since the modelStore can't find the backup err := cmd.ExecuteContext(ctx) require.Error(t, err, clues.ToCore(err)) } + +func (suite *BackupDeleteExchangeE2ESuite) TestExchangeBackupDeleteCmd_NoBackupID() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + ctx = config.SetViper(ctx, suite.dpnd.vpr) + + defer flush() + + cmd := cliTD.StubRootCmd( + "backup", "delete", "exchange", + "--config-file", suite.dpnd.configFilePath) + cli.BuildCommandTree(cmd) + + // empty backupIDs should error since no data provided + err := cmd.ExecuteContext(ctx) + require.Error(t, err, clues.ToCore(err)) +} diff --git a/src/cli/backup/groups.go b/src/cli/backup/groups.go index 7e3e041db..c8be220f3 100644 --- a/src/cli/backup/groups.go +++ b/src/cli/backup/groups.go @@ -32,7 +32,7 @@ const ( groupsServiceCommand = "groups" teamsServiceCommand = "teams" groupsServiceCommandCreateUseSuffix = "--group | '" + flags.Wildcard + "'" - groupsServiceCommandDeleteUseSuffix = "--backup " + groupsServiceCommandDeleteUseSuffix = "--backups " groupsServiceCommandDetailsUseSuffix = "--backup " ) @@ -46,8 +46,9 @@ corso backup create groups --group Marketing --data messages # Backup all Groups and Teams data for all groups corso backup create groups --group '*'` - groupsServiceCommandDeleteExamples = `# Delete Groups backup with ID 1234abcd-12ab-cd34-56de-1234abcd -corso backup delete groups --backup 1234abcd-12ab-cd34-56de-1234abcd` + groupsServiceCommandDeleteExamples = `# Delete Groups backup with ID 1234abcd-12ab-cd34-56de-1234abcd \ +and 1234abcd-12ab-cd34-56de-1234abce +corso backup delete groups --backups 1234abcd-12ab-cd34-56de-1234abcd,1234abcd-12ab-cd34-56de-1234abce` groupsServiceCommandDetailsExamples = `# Explore items in Marketing's latest backup (1234abcd...) corso backup details groups --backup 1234abcd-12ab-cd34-56de-1234abcd @@ -110,7 +111,8 @@ func addGroupsCommands(cmd *cobra.Command) *cobra.Command { c.Use = c.Use + " " + groupsServiceCommandDeleteUseSuffix c.Example = groupsServiceCommandDeleteExamples - flags.AddBackupIDFlag(c, true) + flags.AddMultipleBackupIDsFlag(c, false) + flags.AddBackupIDFlag(c, false) } return c @@ -226,17 +228,19 @@ func detailsGroupsCmd(cmd *cobra.Command, args []string) error { ctx := cmd.Context() opts := utils.MakeGroupsOpts(cmd) - r, _, _, ctrlOpts, err := utils.GetAccountAndConnectWithOverrides( - ctx, - cmd, - path.GroupsService) + r, rdao, err := utils.GetAccountAndConnect(ctx, cmd, path.GroupsService) if err != nil { return Only(ctx, err) } defer utils.CloseRepo(ctx, r) - ds, err := runDetailsGroupsCmd(ctx, r, flags.BackupIDFV, opts, ctrlOpts.SkipReduce) + ds, err := runDetailsGroupsCmd( + ctx, + r, + flags.BackupIDFV, + opts, + rdao.Opts.SkipReduce) if err != nil { return Only(ctx, err) } @@ -305,7 +309,17 @@ func groupsDeleteCmd() *cobra.Command { // deletes an groups service backup. func deleteGroupsCmd(cmd *cobra.Command, args []string) error { - return genericDeleteCommand(cmd, path.GroupsService, flags.BackupIDFV, "Groups", args) + backupIDValue := []string{} + + if len(flags.BackupIDsFV) > 0 { + backupIDValue = flags.BackupIDsFV + } else if len(flags.BackupIDFV) > 0 { + backupIDValue = append(backupIDValue, flags.BackupIDFV) + } else { + return clues.New("either --backup or --backups flag is required") + } + + return genericDeleteCommand(cmd, path.GroupsService, "Groups", backupIDValue, args) } // --------------------------------------------------------------------------- diff --git a/src/cli/backup/groups_e2e_test.go b/src/cli/backup/groups_e2e_test.go index 036b2cbb6..986979a4f 100644 --- a/src/cli/backup/groups_e2e_test.go +++ b/src/cli/backup/groups_e2e_test.go @@ -497,8 +497,8 @@ func runGroupsDetailsCmdTest(suite *PreparedBackupGroupsE2ESuite, category path. type BackupDeleteGroupsE2ESuite struct { tester.Suite - dpnd dependencies - backupOp operations.BackupOperation + dpnd dependencies + backupOps [3]operations.BackupOperation } func TestBackupDeleteGroupsE2ESuite(t *testing.T) { @@ -524,13 +524,15 @@ func (suite *BackupDeleteGroupsE2ESuite) SetupSuite() { sel := selectors.NewGroupsBackup(groups) sel.Include(selTD.GroupsBackupChannelScope(sel)) - backupOp, err := suite.dpnd.repo.NewBackup(ctx, sel.Selector) - require.NoError(t, err, clues.ToCore(err)) + for i := 0; i < cap(suite.backupOps); i++ { + backupOp, err := suite.dpnd.repo.NewBackup(ctx, sel.Selector) + require.NoError(t, err, clues.ToCore(err)) - suite.backupOp = backupOp + suite.backupOps[i] = backupOp - err = suite.backupOp.Run(ctx) - require.NoError(t, err, clues.ToCore(err)) + err = suite.backupOps[i].Run(ctx) + require.NoError(t, err, clues.ToCore(err)) + } } func (suite *BackupDeleteGroupsE2ESuite) TestGroupsBackupDeleteCmd() { @@ -544,7 +546,10 @@ func (suite *BackupDeleteGroupsE2ESuite) TestGroupsBackupDeleteCmd() { cmd := cliTD.StubRootCmd( "backup", "delete", "groups", "--config-file", suite.dpnd.configFilePath, - "--"+flags.BackupFN, string(suite.backupOp.Results.BackupID)) + "--"+flags.BackupIDsFN, + fmt.Sprintf("%s,%s", + string(suite.backupOps[0].Results.BackupID), + string(suite.backupOps[1].Results.BackupID))) cli.BuildCommandTree(cmd) // run the command @@ -555,7 +560,37 @@ func (suite *BackupDeleteGroupsE2ESuite) TestGroupsBackupDeleteCmd() { cmd = cliTD.StubRootCmd( "backup", "details", "groups", "--config-file", suite.dpnd.configFilePath, - "--backup", string(suite.backupOp.Results.BackupID)) + "--backups", string(suite.backupOps[0].Results.BackupID)) + cli.BuildCommandTree(cmd) + + err = cmd.ExecuteContext(ctx) + require.Error(t, err, clues.ToCore(err)) +} + +func (suite *BackupDeleteGroupsE2ESuite) TestGroupsBackupDeleteCmd_SingleID() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + ctx = config.SetViper(ctx, suite.dpnd.vpr) + + defer flush() + + cmd := cliTD.StubRootCmd( + "backup", "delete", "groups", + "--config-file", suite.dpnd.configFilePath, + "--"+flags.BackupFN, + string(suite.backupOps[2].Results.BackupID)) + cli.BuildCommandTree(cmd) + + // run the command + err := cmd.ExecuteContext(ctx) + require.NoError(t, err, clues.ToCore(err)) + + // a follow-up details call should fail, due to the backup ID being deleted + cmd = cliTD.StubRootCmd( + "backup", "details", "groups", + "--config-file", suite.dpnd.configFilePath, + "--backup", string(suite.backupOps[2].Results.BackupID)) cli.BuildCommandTree(cmd) err = cmd.ExecuteContext(ctx) @@ -573,7 +608,7 @@ func (suite *BackupDeleteGroupsE2ESuite) TestGroupsBackupDeleteCmd_UnknownID() { cmd := cliTD.StubRootCmd( "backup", "delete", "groups", "--config-file", suite.dpnd.configFilePath, - "--"+flags.BackupFN, uuid.NewString()) + "--"+flags.BackupIDsFN, uuid.NewString()) cli.BuildCommandTree(cmd) // unknown backupIDs should error since the modelStore can't find the backup @@ -581,6 +616,24 @@ func (suite *BackupDeleteGroupsE2ESuite) TestGroupsBackupDeleteCmd_UnknownID() { require.Error(t, err, clues.ToCore(err)) } +func (suite *BackupDeleteGroupsE2ESuite) TestGroupsBackupDeleteCmd_NoBackupID() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + ctx = config.SetViper(ctx, suite.dpnd.vpr) + + defer flush() + + cmd := cliTD.StubRootCmd( + "backup", "delete", "groups", + "--config-file", suite.dpnd.configFilePath) + cli.BuildCommandTree(cmd) + + // empty backupIDs should error since no data provided + err := cmd.ExecuteContext(ctx) + require.Error(t, err, clues.ToCore(err)) +} + // --------------------------------------------------------------------------- // helpers // --------------------------------------------------------------------------- diff --git a/src/cli/backup/onedrive.go b/src/cli/backup/onedrive.go index c870f84ee..fa8170f64 100644 --- a/src/cli/backup/onedrive.go +++ b/src/cli/backup/onedrive.go @@ -26,7 +26,7 @@ import ( const ( oneDriveServiceCommand = "onedrive" oneDriveServiceCommandCreateUseSuffix = "--user | '" + flags.Wildcard + "'" - oneDriveServiceCommandDeleteUseSuffix = "--backup " + oneDriveServiceCommandDeleteUseSuffix = "--backups " oneDriveServiceCommandDetailsUseSuffix = "--backup " ) @@ -40,8 +40,9 @@ corso backup create onedrive --user alice@example.com,bob@example.com # Backup all OneDrive data for all M365 users corso backup create onedrive --user '*'` - oneDriveServiceCommandDeleteExamples = `# Delete OneDrive backup with ID 1234abcd-12ab-cd34-56de-1234abcd -corso backup delete onedrive --backup 1234abcd-12ab-cd34-56de-1234abcd` + oneDriveServiceCommandDeleteExamples = `# Delete OneDrive backup with ID 1234abcd-12ab-cd34-56de-1234abcd \ +and 1234abcd-12ab-cd34-56de-1234abce +corso backup delete onedrive --backups 1234abcd-12ab-cd34-56de-1234abcd,1234abcd-12ab-cd34-56de-1234abce` oneDriveServiceCommandDetailsExamples = `# Explore items in Bob's latest backup (1234abcd...) corso backup details onedrive --backup 1234abcd-12ab-cd34-56de-1234abcd @@ -100,7 +101,8 @@ func addOneDriveCommands(cmd *cobra.Command) *cobra.Command { c.Use = c.Use + " " + oneDriveServiceCommandDeleteUseSuffix c.Example = oneDriveServiceCommandDeleteExamples - flags.AddBackupIDFlag(c, true) + flags.AddMultipleBackupIDsFlag(c, false) + flags.AddBackupIDFlag(c, false) } return c @@ -230,17 +232,19 @@ func detailsOneDriveCmd(cmd *cobra.Command, args []string) error { ctx := cmd.Context() opts := utils.MakeOneDriveOpts(cmd) - r, _, _, ctrlOpts, err := utils.GetAccountAndConnectWithOverrides( - ctx, - cmd, - path.OneDriveService) + r, rdao, err := utils.GetAccountAndConnect(ctx, cmd, path.OneDriveService) if err != nil { return Only(ctx, err) } defer utils.CloseRepo(ctx, r) - ds, err := runDetailsOneDriveCmd(ctx, r, flags.BackupIDFV, opts, ctrlOpts.SkipReduce) + ds, err := runDetailsOneDriveCmd( + ctx, + r, + flags.BackupIDFV, + opts, + rdao.Opts.SkipReduce) if err != nil { return Only(ctx, err) } @@ -306,5 +310,15 @@ func oneDriveDeleteCmd() *cobra.Command { // deletes a oneDrive service backup. func deleteOneDriveCmd(cmd *cobra.Command, args []string) error { - return genericDeleteCommand(cmd, path.OneDriveService, flags.BackupIDFV, "OneDrive", args) + backupIDValue := []string{} + + if len(flags.BackupIDsFV) > 0 { + backupIDValue = flags.BackupIDsFV + } else if len(flags.BackupIDFV) > 0 { + backupIDValue = append(backupIDValue, flags.BackupIDFV) + } else { + return clues.New("either --backup or --backups flag is required") + } + + return genericDeleteCommand(cmd, path.OneDriveService, "OneDrive", backupIDValue, args) } diff --git a/src/cli/backup/onedrive_e2e_test.go b/src/cli/backup/onedrive_e2e_test.go index ea5e808b4..f4b2c0bdc 100644 --- a/src/cli/backup/onedrive_e2e_test.go +++ b/src/cli/backup/onedrive_e2e_test.go @@ -121,8 +121,8 @@ func (suite *NoBackupOneDriveE2ESuite) TestOneDriveBackupCmd_userNotInTenant() { type BackupDeleteOneDriveE2ESuite struct { tester.Suite - dpnd dependencies - backupOp operations.BackupOperation + dpnd dependencies + backupOps [3]operations.BackupOperation } func TestBackupDeleteOneDriveE2ESuite(t *testing.T) { @@ -151,13 +151,15 @@ func (suite *BackupDeleteOneDriveE2ESuite) SetupSuite() { sel := selectors.NewOneDriveBackup(users) sel.Include(selTD.OneDriveBackupFolderScope(sel)) - backupOp, err := suite.dpnd.repo.NewBackupWithLookup(ctx, sel.Selector, ins) - require.NoError(t, err, clues.ToCore(err)) + for i := 0; i < cap(suite.backupOps); i++ { + backupOp, err := suite.dpnd.repo.NewBackupWithLookup(ctx, sel.Selector, ins) + require.NoError(t, err, clues.ToCore(err)) - suite.backupOp = backupOp + suite.backupOps[i] = backupOp - err = suite.backupOp.Run(ctx) - require.NoError(t, err, clues.ToCore(err)) + err = suite.backupOps[i].Run(ctx) + require.NoError(t, err, clues.ToCore(err)) + } } func (suite *BackupDeleteOneDriveE2ESuite) TestOneDriveBackupDeleteCmd() { @@ -173,7 +175,10 @@ func (suite *BackupDeleteOneDriveE2ESuite) TestOneDriveBackupDeleteCmd() { cmd := cliTD.StubRootCmd( "backup", "delete", "onedrive", "--config-file", suite.dpnd.configFilePath, - "--"+flags.BackupFN, string(suite.backupOp.Results.BackupID)) + "--"+flags.BackupIDsFN, + fmt.Sprintf("%s,%s", + string(suite.backupOps[0].Results.BackupID), + string(suite.backupOps[1].Results.BackupID))) cli.BuildCommandTree(cmd) cmd.SetErr(&suite.dpnd.recorder) @@ -187,13 +192,57 @@ func (suite *BackupDeleteOneDriveE2ESuite) TestOneDriveBackupDeleteCmd() { assert.True(t, strings.HasSuffix( result, - fmt.Sprintf("Deleted OneDrive backup %s\n", string(suite.backupOp.Results.BackupID)))) + fmt.Sprintf("Deleted OneDrive backup [%s %s]\n", + string(suite.backupOps[0].Results.BackupID), + string(suite.backupOps[1].Results.BackupID)))) // a follow-up details call should fail, due to the backup ID being deleted cmd = cliTD.StubRootCmd( "backup", "details", "onedrive", "--config-file", suite.dpnd.configFilePath, - "--backup", string(suite.backupOp.Results.BackupID)) + "--backups", string(suite.backupOps[0].Results.BackupID)) + cli.BuildCommandTree(cmd) + + err = cmd.ExecuteContext(ctx) + require.Error(t, err, clues.ToCore(err)) +} + +func (suite *BackupDeleteOneDriveE2ESuite) TestOneDriveBackupDeleteCmd_SingleID() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + ctx = config.SetViper(ctx, suite.dpnd.vpr) + + defer flush() + + suite.dpnd.recorder.Reset() + + cmd := cliTD.StubRootCmd( + "backup", "delete", "onedrive", + "--config-file", suite.dpnd.configFilePath, + "--"+flags.BackupFN, + string(suite.backupOps[2].Results.BackupID)) + cli.BuildCommandTree(cmd) + cmd.SetErr(&suite.dpnd.recorder) + + ctx = print.SetRootCmd(ctx, cmd) + + // run the command + err := cmd.ExecuteContext(ctx) + require.NoError(t, err, clues.ToCore(err)) + + result := suite.dpnd.recorder.String() + assert.True(t, + strings.HasSuffix( + result, + fmt.Sprintf("Deleted OneDrive backup [%s]\n", + string(suite.backupOps[2].Results.BackupID)))) + + // a follow-up details call should fail, due to the backup ID being deleted + cmd = cliTD.StubRootCmd( + "backup", "details", "onedrive", + "--config-file", suite.dpnd.configFilePath, + "--backup", string(suite.backupOps[0].Results.BackupID)) cli.BuildCommandTree(cmd) err = cmd.ExecuteContext(ctx) @@ -211,10 +260,28 @@ func (suite *BackupDeleteOneDriveE2ESuite) TestOneDriveBackupDeleteCmd_unknownID cmd := cliTD.StubRootCmd( "backup", "delete", "onedrive", "--config-file", suite.dpnd.configFilePath, - "--"+flags.BackupFN, uuid.NewString()) + "--"+flags.BackupIDsFN, uuid.NewString()) cli.BuildCommandTree(cmd) // unknown backupIDs should error since the modelStore can't find the backup err := cmd.ExecuteContext(ctx) require.Error(t, err, clues.ToCore(err)) } + +func (suite *BackupDeleteOneDriveE2ESuite) TestOneDriveBackupDeleteCmd_NoBackupID() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + ctx = config.SetViper(ctx, suite.dpnd.vpr) + + defer flush() + + cmd := cliTD.StubRootCmd( + "backup", "delete", "onedrive", + "--config-file", suite.dpnd.configFilePath) + cli.BuildCommandTree(cmd) + + // empty backupIDs should error since no data provided + err := cmd.ExecuteContext(ctx) + require.Error(t, err, clues.ToCore(err)) +} diff --git a/src/cli/backup/sharepoint.go b/src/cli/backup/sharepoint.go index 2b154573e..507a4a6d2 100644 --- a/src/cli/backup/sharepoint.go +++ b/src/cli/backup/sharepoint.go @@ -30,7 +30,7 @@ import ( const ( sharePointServiceCommand = "sharepoint" sharePointServiceCommandCreateUseSuffix = "--site | '" + flags.Wildcard + "'" - sharePointServiceCommandDeleteUseSuffix = "--backup " + sharePointServiceCommandDeleteUseSuffix = "--backups " sharePointServiceCommandDetailsUseSuffix = "--backup " ) @@ -44,8 +44,9 @@ corso backup create sharepoint --site https://example.com/hr,https://example.com # Backup all SharePoint data for all Sites corso backup create sharepoint --site '*'` - sharePointServiceCommandDeleteExamples = `# Delete SharePoint backup with ID 1234abcd-12ab-cd34-56de-1234abcd -corso backup delete sharepoint --backup 1234abcd-12ab-cd34-56de-1234abcd` + sharePointServiceCommandDeleteExamples = `# Delete SharePoint backup with ID 1234abcd-12ab-cd34-56de-1234abcd \ +and 1234abcd-12ab-cd34-56de-1234abce +corso backup delete sharepoint --backups 1234abcd-12ab-cd34-56de-1234abcd,1234abcd-12ab-cd34-56de-1234abce` sharePointServiceCommandDetailsExamples = `# Explore items in the HR site's latest backup (1234abcd...) corso backup details sharepoint --backup 1234abcd-12ab-cd34-56de-1234abcd @@ -111,7 +112,8 @@ func addSharePointCommands(cmd *cobra.Command) *cobra.Command { c.Use = c.Use + " " + sharePointServiceCommandDeleteUseSuffix c.Example = sharePointServiceCommandDeleteExamples - flags.AddBackupIDFlag(c, true) + flags.AddMultipleBackupIDsFlag(c, false) + flags.AddBackupIDFlag(c, false) } return c @@ -284,7 +286,17 @@ func sharePointDeleteCmd() *cobra.Command { // deletes a sharePoint service backup. func deleteSharePointCmd(cmd *cobra.Command, args []string) error { - return genericDeleteCommand(cmd, path.SharePointService, flags.BackupIDFV, "SharePoint", args) + backupIDValue := []string{} + + if len(flags.BackupIDsFV) > 0 { + backupIDValue = flags.BackupIDsFV + } else if len(flags.BackupIDFV) > 0 { + backupIDValue = append(backupIDValue, flags.BackupIDFV) + } else { + return clues.New("either --backup or --backups flag is required") + } + + return genericDeleteCommand(cmd, path.SharePointService, "SharePoint", backupIDValue, args) } // ------------------------------------------------------------------------------------------------ @@ -315,17 +327,19 @@ func detailsSharePointCmd(cmd *cobra.Command, args []string) error { ctx := cmd.Context() opts := utils.MakeSharePointOpts(cmd) - r, _, _, ctrlOpts, err := utils.GetAccountAndConnectWithOverrides( - ctx, - cmd, - path.SharePointService) + r, rdao, err := utils.GetAccountAndConnect(ctx, cmd, path.SharePointService) if err != nil { return Only(ctx, err) } defer utils.CloseRepo(ctx, r) - ds, err := runDetailsSharePointCmd(ctx, r, flags.BackupIDFV, opts, ctrlOpts.SkipReduce) + ds, err := runDetailsSharePointCmd( + ctx, + r, + flags.BackupIDFV, + opts, + rdao.Opts.SkipReduce) if err != nil { return Only(ctx, err) } diff --git a/src/cli/backup/sharepoint_e2e_test.go b/src/cli/backup/sharepoint_e2e_test.go index 138247850..bfb67f85a 100644 --- a/src/cli/backup/sharepoint_e2e_test.go +++ b/src/cli/backup/sharepoint_e2e_test.go @@ -84,8 +84,9 @@ func (suite *NoBackupSharePointE2ESuite) TestSharePointBackupListCmd_empty() { type BackupDeleteSharePointE2ESuite struct { tester.Suite - dpnd dependencies - backupOp operations.BackupOperation + dpnd dependencies + backupOp operations.BackupOperation + secondaryBackupOp operations.BackupOperation } func TestBackupDeleteSharePointE2ESuite(t *testing.T) { @@ -121,6 +122,15 @@ func (suite *BackupDeleteSharePointE2ESuite) SetupSuite() { err = suite.backupOp.Run(ctx) require.NoError(t, err, clues.ToCore(err)) + + // secondary backup + secondaryBackupOp, err := suite.dpnd.repo.NewBackupWithLookup(ctx, sel.Selector, ins) + require.NoError(t, err, clues.ToCore(err)) + + suite.secondaryBackupOp = secondaryBackupOp + + err = suite.secondaryBackupOp.Run(ctx) + require.NoError(t, err, clues.ToCore(err)) } func (suite *BackupDeleteSharePointE2ESuite) TestSharePointBackupDeleteCmd() { @@ -136,7 +146,10 @@ func (suite *BackupDeleteSharePointE2ESuite) TestSharePointBackupDeleteCmd() { cmd := cliTD.StubRootCmd( "backup", "delete", "sharepoint", "--config-file", suite.dpnd.configFilePath, - "--"+flags.BackupFN, string(suite.backupOp.Results.BackupID)) + "--"+flags.BackupIDsFN, + fmt.Sprintf("%s,%s", + string(suite.backupOp.Results.BackupID), + string(suite.secondaryBackupOp.Results.BackupID))) cli.BuildCommandTree(cmd) cmd.SetErr(&suite.dpnd.recorder) @@ -150,7 +163,9 @@ func (suite *BackupDeleteSharePointE2ESuite) TestSharePointBackupDeleteCmd() { assert.True(t, strings.HasSuffix( result, - fmt.Sprintf("Deleted SharePoint backup %s\n", string(suite.backupOp.Results.BackupID)))) + fmt.Sprintf("Deleted SharePoint backup [%s %s]\n", + string(suite.backupOp.Results.BackupID), + string(suite.secondaryBackupOp.Results.BackupID)))) } // moved out of the func above to make the linter happy @@ -175,10 +190,28 @@ func (suite *BackupDeleteSharePointE2ESuite) TestSharePointBackupDeleteCmd_unkno cmd := cliTD.StubRootCmd( "backup", "delete", "sharepoint", "--config-file", suite.dpnd.configFilePath, - "--"+flags.BackupFN, uuid.NewString()) + "--"+flags.BackupIDsFN, uuid.NewString()) cli.BuildCommandTree(cmd) // unknown backupIDs should error since the modelStore can't find the backup err := cmd.ExecuteContext(ctx) require.Error(t, err, clues.ToCore(err)) } + +func (suite *BackupDeleteSharePointE2ESuite) TestSharePointBackupDeleteCmd_NoBackupID() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + ctx = config.SetViper(ctx, suite.dpnd.vpr) + + defer flush() + + cmd := cliTD.StubRootCmd( + "backup", "delete", "groups", + "--config-file", suite.dpnd.configFilePath) + cli.BuildCommandTree(cmd) + + // empty backupIDs should error since no data provided + err := cmd.ExecuteContext(ctx) + require.Error(t, err, clues.ToCore(err)) +} diff --git a/src/cli/export/export.go b/src/cli/export/export.go index f106f87bc..db48f466a 100644 --- a/src/cli/export/export.go +++ b/src/cli/export/export.go @@ -67,10 +67,7 @@ func runExport( return Only(ctx, err) } - r, _, _, _, err := utils.GetAccountAndConnectWithOverrides( - ctx, - cmd, - sel.PathService()) + r, _, err := utils.GetAccountAndConnect(ctx, cmd, sel.PathService()) if err != nil { return Only(ctx, err) } diff --git a/src/cli/flags/repo.go b/src/cli/flags/repo.go index efac65b0c..2db4c23fc 100644 --- a/src/cli/flags/repo.go +++ b/src/cli/flags/repo.go @@ -6,6 +6,7 @@ import ( const ( BackupFN = "backup" + BackupIDsFN = "backups" AWSAccessKeyFN = "aws-access-key" AWSSecretAccessKeyFN = "aws-secret-access-key" AWSSessionTokenFN = "aws-session-token" @@ -18,6 +19,7 @@ const ( var ( BackupIDFV string + BackupIDsFV []string AWSAccessKeyFV string AWSSecretAccessKeyFV string AWSSessionTokenFV string @@ -26,6 +28,18 @@ var ( SucceedIfExistsFV bool ) +// AddMultipleBackupIDsFlag adds the --backups flag. +func AddMultipleBackupIDsFlag(cmd *cobra.Command, require bool) { + cmd.Flags().StringSliceVar( + &BackupIDsFV, + BackupIDsFN, nil, + "',' separated IDs of the backup to retrieve") + + if require { + cobra.CheckErr(cmd.MarkFlagRequired(BackupIDsFN)) + } +} + // AddBackupIDFlag adds the --backup flag. func AddBackupIDFlag(cmd *cobra.Command, require bool) { cmd.Flags().StringVar(&BackupIDFV, BackupFN, "", "ID of the backup to retrieve.") diff --git a/src/cli/restore/exchange_e2e_test.go b/src/cli/restore/exchange_e2e_test.go index 5e3d6181d..effbf14d8 100644 --- a/src/cli/restore/exchange_e2e_test.go +++ b/src/cli/restore/exchange_e2e_test.go @@ -86,7 +86,7 @@ func (suite *RestoreExchangeE2ESuite) SetupSuite() { ) // init the repo first - r, err := repository.New( + suite.repo, err = repository.New( ctx, suite.acct, suite.st, @@ -94,7 +94,7 @@ func (suite *RestoreExchangeE2ESuite) SetupSuite() { repository.NewRepoID) require.NoError(t, err, clues.ToCore(err)) - err = r.Initialize(ctx, ctrlRepo.Retention{}) + err = suite.repo.Initialize(ctx, ctrlRepo.Retention{}) require.NoError(t, err, clues.ToCore(err)) suite.backupOps = make(map[path.CategoryType]operations.BackupOperation) diff --git a/src/cli/restore/groups.go b/src/cli/restore/groups.go index 755d797e0..9e1f9cf5d 100644 --- a/src/cli/restore/groups.go +++ b/src/cli/restore/groups.go @@ -46,7 +46,7 @@ const ( corso restore groups --backup 1234abcd-12ab-cd34-56de-1234abcd --file 98765abcdef # Restore the file with ID 98765abcdef without its associated permissions -corso restore groups --backup 1234abcd-12ab-cd34-56de-1234abcd --file 98765abcdef --skip-permissions +corso restore groups --backup 1234abcd-12ab-cd34-56de-1234abcd --file 98765abcdef --no-permissions # Restore all files named "FY2021 Planning.xlsx" corso restore groups --backup 1234abcd-12ab-cd34-56de-1234abcd --file "FY2021 Planning.xlsx" diff --git a/src/cli/restore/restore.go b/src/cli/restore/restore.go index f9da46416..9dad4ca1c 100644 --- a/src/cli/restore/restore.go +++ b/src/cli/restore/restore.go @@ -100,10 +100,7 @@ func runRestore( return Only(ctx, err) } - r, _, _, _, err := utils.GetAccountAndConnectWithOverrides( - ctx, - cmd, - sel.PathService()) + r, _, err := utils.GetAccountAndConnect(ctx, cmd, sel.PathService()) if err != nil { return Only(ctx, err) } diff --git a/src/cli/utils/utils.go b/src/cli/utils/utils.go index e573bdb0a..2a4e3de34 100644 --- a/src/cli/utils/utils.go +++ b/src/cli/utils/utils.go @@ -22,30 +22,35 @@ import ( "github.com/alcionai/corso/src/pkg/storage" ) +type RepoDetailsAndOpts struct { + Repo config.RepoDetails + Opts control.Options +} + var ErrNotYetImplemented = clues.New("not yet implemented") -// GetAccountAndConnectWithOverrides is a wrapper for GetAccountAndConnect -// that also gets the storage provider and any storage provider specific +// GetAccountAndConnect is a wrapper for GetAccountAndConnectWithOverrides +// that automatically gets the storage provider and any storage provider specific // flag overrides from the command line. -func GetAccountAndConnectWithOverrides( +func GetAccountAndConnect( ctx context.Context, cmd *cobra.Command, pst path.ServiceType, -) (repository.Repositoryer, *storage.Storage, *account.Account, *control.Options, error) { +) (repository.Repositoryer, RepoDetailsAndOpts, error) { provider, overrides, err := GetStorageProviderAndOverrides(ctx, cmd) if err != nil { - return nil, nil, nil, nil, clues.Stack(err) + return nil, RepoDetailsAndOpts{}, clues.Stack(err) } - return GetAccountAndConnect(ctx, pst, provider, overrides) + return GetAccountAndConnectWithOverrides(ctx, pst, provider, overrides) } -func GetAccountAndConnect( +func GetAccountAndConnectWithOverrides( ctx context.Context, pst path.ServiceType, provider storage.ProviderType, overrides map[string]string, -) (repository.Repositoryer, *storage.Storage, *account.Account, *control.Options, error) { +) (repository.Repositoryer, RepoDetailsAndOpts, error) { cfg, err := config.GetConfigRepoDetails( ctx, provider, @@ -53,7 +58,7 @@ func GetAccountAndConnect( true, overrides) if err != nil { - return nil, nil, nil, nil, err + return nil, RepoDetailsAndOpts{}, err } repoID := cfg.RepoID @@ -70,20 +75,25 @@ func GetAccountAndConnect( opts, repoID) if err != nil { - return nil, nil, nil, nil, clues.Wrap(err, "creating a repository controller") + return nil, RepoDetailsAndOpts{}, clues.Wrap(err, "creating a repository controller") } if err := r.Connect(ctx); err != nil { - return nil, nil, nil, nil, clues.Wrap(err, "connecting to the "+cfg.Storage.Provider.String()+" repository") + return nil, RepoDetailsAndOpts{}, clues.Wrap(err, "connecting to the "+cfg.Storage.Provider.String()+" repository") } // this initializes our graph api client configurations, // including control options such as concurency limitations. if _, err := r.ConnectToM365(ctx, pst); err != nil { - return nil, nil, nil, nil, clues.Wrap(err, "connecting to m365") + return nil, RepoDetailsAndOpts{}, clues.Wrap(err, "connecting to m365") } - return r, &cfg.Storage, &cfg.Account, &opts, nil + rdao := RepoDetailsAndOpts{ + Repo: cfg, + Opts: opts, + } + + return r, rdao, nil } func AccountConnectAndWriteRepoConfig( @@ -91,22 +101,19 @@ func AccountConnectAndWriteRepoConfig( cmd *cobra.Command, pst path.ServiceType, ) (repository.Repositoryer, *account.Account, error) { - r, stg, acc, opts, err := GetAccountAndConnectWithOverrides( - ctx, - cmd, - pst) + r, rdao, err := GetAccountAndConnect(ctx, cmd, pst) if err != nil { logger.CtxErr(ctx, err).Info("getting and connecting account") return nil, nil, err } - sc, err := stg.StorageConfig() + sc, err := rdao.Repo.Storage.StorageConfig() if err != nil { logger.CtxErr(ctx, err).Info("getting storage configuration") return nil, nil, err } - m365Config, err := acc.M365Config() + m365Config, err := rdao.Repo.Account.M365Config() if err != nil { logger.CtxErr(ctx, err).Info("getting m365 configuration") return nil, nil, err @@ -114,13 +121,13 @@ func AccountConnectAndWriteRepoConfig( // repo config gets set during repo connect and init. // This call confirms we have the correct values. - err = config.WriteRepoConfig(ctx, sc, m365Config, opts.Repo, r.GetID()) + err = config.WriteRepoConfig(ctx, sc, m365Config, rdao.Opts.Repo, r.GetID()) if err != nil { logger.CtxErr(ctx, err).Info("writing to repository configuration") return nil, nil, err } - return r, acc, nil + return r, &rdao.Repo.Account, nil } // CloseRepo handles closing a repo. diff --git a/src/cmd/longevity_test/longevity.go b/src/cmd/longevity_test/longevity.go index 0b47ea518..ec7862191 100644 --- a/src/cmd/longevity_test/longevity.go +++ b/src/cmd/longevity_test/longevity.go @@ -31,11 +31,7 @@ func deleteBackups( ) ([]string, error) { ctx = clues.Add(ctx, "cutoff_days", deletionDays) - r, _, _, _, err := utils.GetAccountAndConnect( - ctx, - service, - storage.ProviderS3, - nil) + r, _, err := utils.GetAccountAndConnectWithOverrides(ctx, service, storage.ProviderS3, nil) if err != nil { return nil, clues.Wrap(err, "connecting to account").WithClues(ctx) } diff --git a/src/go.mod b/src/go.mod index d20785bf6..96d163731 100644 --- a/src/go.mod +++ b/src/go.mod @@ -21,7 +21,7 @@ require ( github.com/microsoftgraph/msgraph-sdk-go v1.19.0 github.com/microsoftgraph/msgraph-sdk-go-core v1.0.0 github.com/pkg/errors v0.9.1 - github.com/puzpuzpuz/xsync/v2 v2.5.0 + github.com/puzpuzpuz/xsync/v2 v2.5.1 github.com/rudderlabs/analytics-go v3.3.3+incompatible github.com/spatialcurrent/go-lazy v0.0.0-20211115014721-47315cc003d1 github.com/spf13/cast v1.5.1 diff --git a/src/go.sum b/src/go.sum index cda31894a..054c65071 100644 --- a/src/go.sum +++ b/src/go.sum @@ -360,8 +360,8 @@ github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsT github.com/prometheus/procfs v0.0.8/go.mod h1:7Qr8sr6344vo1JqZ6HhLceV9o3AJ1Ff+GxbHq6oeK9A= github.com/prometheus/procfs v0.11.1 h1:xRC8Iq1yyca5ypa9n1EZnWZkt7dwcoRPQwX/5gwaUuI= github.com/prometheus/procfs v0.11.1/go.mod h1:eesXgaPo1q7lBpVMoMy0ZOFTth9hBn4W/y0/p/ScXhY= -github.com/puzpuzpuz/xsync/v2 v2.5.0 h1:2k4qrO/orvmEXZ3hmtHqIy9XaQtPTwzMZk1+iErpE8c= -github.com/puzpuzpuz/xsync/v2 v2.5.0/go.mod h1:gD2H2krq/w52MfPLE+Uy64TzJDVY7lP2znR9qmR35kU= +github.com/puzpuzpuz/xsync/v2 v2.5.1 h1:mVGYAvzDSu52+zaGyNjC+24Xw2bQi3kTr4QJ6N9pIIU= +github.com/puzpuzpuz/xsync/v2 v2.5.1/go.mod h1:gD2H2krq/w52MfPLE+Uy64TzJDVY7lP2znR9qmR35kU= github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= github.com/rivo/uniseg v0.4.4 h1:8TfxU8dW6PdqD27gjM8MVNuicgxIjxpm4K7x4jp8sis= github.com/rivo/uniseg v0.4.4/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= diff --git a/src/internal/m365/graph/errors.go b/src/internal/m365/graph/errors.go index 91dc01bb0..f5c7824ab 100644 --- a/src/internal/m365/graph/errors.go +++ b/src/internal/m365/graph/errors.go @@ -95,6 +95,10 @@ var ( // https://learn.microsoft.com/en-us/graph/errors#code-property ErrInvalidDelta = clues.New("invalid delta token") + // Not all systems support delta queries. This must be handled separately + // from invalid delta token cases. + ErrDeltaNotSupported = clues.New("delta not supported") + // ErrItemAlreadyExistsConflict denotes that a post or put attempted to create // an item which already exists by some unique identifier. The identifier is // not always the id. For example, in onedrive, this error can be produced @@ -122,8 +126,8 @@ var ( ) func IsErrApplicationThrottled(err error) bool { - return hasErrorCode(err, applicationThrottled) || - errors.Is(err, ErrApplicationThrottled) + return errors.Is(err, ErrApplicationThrottled) || + hasErrorCode(err, applicationThrottled) } func IsErrAuthenticationError(err error) bool { @@ -151,9 +155,13 @@ func IsErrItemNotFound(err error) bool { } func IsErrInvalidDelta(err error) bool { - return hasErrorCode(err, syncStateNotFound, resyncRequired, syncStateInvalid) || - hasErrorMessage(err, parameterDeltaTokenNotSupported) || - errors.Is(err, ErrInvalidDelta) + return errors.Is(err, ErrInvalidDelta) || + hasErrorCode(err, syncStateNotFound, resyncRequired, syncStateInvalid) +} + +func IsErrDeltaNotSupported(err error) bool { + return errors.Is(err, ErrDeltaNotSupported) || + hasErrorMessage(err, parameterDeltaTokenNotSupported) } func IsErrQuotaExceeded(err error) bool { @@ -190,7 +198,8 @@ func IsErrCannotOpenFileAttachment(err error) bool { } func IsErrAccessDenied(err error) bool { - return hasErrorCode(err, ErrorAccessDenied) || clues.HasLabel(err, LabelStatus(http.StatusForbidden)) + return hasErrorCode(err, ErrorAccessDenied) || + clues.HasLabel(err, LabelStatus(http.StatusForbidden)) } func IsErrTimeout(err error) bool { @@ -218,8 +227,8 @@ func IsErrUnauthorized(err error) bool { } func IsErrItemAlreadyExistsConflict(err error) bool { - return hasErrorCode(err, nameAlreadyExists) || - errors.Is(err, ErrItemAlreadyExistsConflict) + return errors.Is(err, ErrItemAlreadyExistsConflict) || + hasErrorCode(err, nameAlreadyExists) } // LabelStatus transforms the provided statusCode into @@ -298,7 +307,7 @@ func hasErrorMessage(err error, msgs ...errorMessage) bool { cs[i] = string(c) } - return filters.Contains(cs).Compare(msg) + return filters.In(cs).Compare(msg) } // Wrap is a helper function that extracts ODataError metadata from diff --git a/src/internal/m365/graph/errors_test.go b/src/internal/m365/graph/errors_test.go index 606eb23a5..cf9f2f99d 100644 --- a/src/internal/m365/graph/errors_test.go +++ b/src/internal/m365/graph/errors_test.go @@ -247,11 +247,6 @@ func (suite *GraphErrorsUnitSuite) TestIsErrInvalidDelta() { err: odErr(string(syncStateInvalid)), expect: assert.True, }, - { - name: "deltatoken not supported oDataErrMsg", - err: odErrMsg("fnords", string(parameterDeltaTokenNotSupported)), - expect: assert.True, - }, // next two tests are to make sure the checks are case insensitive { name: "resync-required oDataErr camelcase", @@ -271,6 +266,55 @@ func (suite *GraphErrorsUnitSuite) TestIsErrInvalidDelta() { } } +func (suite *GraphErrorsUnitSuite) TestIsErrDeltaNotSupported() { + 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: "as", + err: ErrDeltaNotSupported, + expect: assert.True, + }, + { + name: "non-matching oDataErr", + err: odErr("fnords"), + expect: assert.False, + }, + { + name: "non-matching oDataErrMsg", + err: odErrMsg("fnords", "deltatoken not supported"), + expect: assert.False, + }, + { + name: "deltatoken not supported oDataErrMsg", + err: odErrMsg("fnords", string(parameterDeltaTokenNotSupported)), + expect: assert.True, + }, + { + name: "deltatoken not supported oDataErrMsg with punctuation", + err: odErrMsg("fnords", string(parameterDeltaTokenNotSupported)+"."), + expect: assert.True, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + test.expect(suite.T(), IsErrDeltaNotSupported(test.err)) + }) + } +} + func (suite *GraphErrorsUnitSuite) TestIsErrQuotaExceeded() { table := []struct { name string diff --git a/src/internal/m365/graph/http_wrapper.go b/src/internal/m365/graph/http_wrapper.go index fdffb583f..0948b9b9e 100644 --- a/src/internal/m365/graph/http_wrapper.go +++ b/src/internal/m365/graph/http_wrapper.go @@ -105,7 +105,7 @@ func (hw httpWrapper) Request( // retry in the event of a `stream error`, which is not // a common expectation. for i := 0; i < hw.config.maxConnectionRetries+1; i++ { - ictx := clues.Add(ctx, "request_retry_iter", i) + ctx = clues.Add(ctx, "request_retry_iter", i) resp, err = hw.client.Do(req) @@ -114,15 +114,15 @@ func (hw httpWrapper) Request( } if IsErrApplicationThrottled(err) { - return nil, Stack(ictx, clues.Stack(ErrApplicationThrottled, err)) + return nil, Stack(ctx, clues.Stack(ErrApplicationThrottled, err)) } var http2StreamErr http2.StreamError if !errors.As(err, &http2StreamErr) { - return nil, Stack(ictx, err) + return nil, Stack(ctx, err) } - logger.Ctx(ictx).Debug("http2 stream error") + logger.Ctx(ctx).Debug("http2 stream error") events.Inc(events.APICall, "streamerror") time.Sleep(3 * time.Second) @@ -132,6 +132,8 @@ func (hw httpWrapper) Request( return nil, Stack(ctx, err) } + logResp(ctx, resp) + return resp, nil } diff --git a/src/internal/m365/graph/logging.go b/src/internal/m365/graph/logging.go new file mode 100644 index 000000000..4f96ae926 --- /dev/null +++ b/src/internal/m365/graph/logging.go @@ -0,0 +1,72 @@ +package graph + +import ( + "context" + "net/http" + "net/http/httputil" + "os" + + "github.com/alcionai/corso/src/pkg/logger" +) + +const ( + // 1 MB + logMBLimit = 1 * 1024 * 1024 + logGraphRequestsEnvKey = "LOG_GRAPH_REQUESTS" + log2xxGraphRequestsEnvKey = "LOG_2XX_GRAPH_REQUESTS" + log2xxGraphResponseEnvKey = "LOG_2XX_GRAPH_RESPONSES" +) + +// special cases where we always dump the response body, since the response +// details might be critical to understanding the response when debugging. +func shouldLogRespBody(resp *http.Response) bool { + return logger.DebugAPIFV || + os.Getenv(logGraphRequestsEnvKey) != "" || + resp.StatusCode == http.StatusBadRequest || + resp.StatusCode == http.StatusForbidden || + resp.StatusCode == http.StatusConflict +} + +func logResp(ctx context.Context, resp *http.Response) { + var ( + log = logger.Ctx(ctx) + respClass = resp.StatusCode / 100 + + // special cases where we always dump the response body, since the response + // details might be critical to understanding the response when debugging. + logBody = shouldLogRespBody(resp) + ) + + // special case: always info-level status 429 logs + if resp.StatusCode == http.StatusTooManyRequests { + log.With("response", getRespDump(ctx, resp, logBody)). + Info("graph api throttling") + return + } + + // Log api calls according to api debugging configurations. + switch respClass { + case 2: + if logBody { + // only dump the body if it's under a size limit. We don't want to copy gigs into memory for a log. + dump := getRespDump(ctx, resp, os.Getenv(log2xxGraphResponseEnvKey) != "" && resp.ContentLength < logMBLimit) + log.Infow("2xx graph api resp", "response", dump) + } + case 3: + log.With("redirect_location", LoggableURL(resp.Header.Get(locationHeader))). + With("response", getRespDump(ctx, resp, false)). + Info("graph api redirect: " + resp.Status) + default: + log.With("response", getRespDump(ctx, resp, logBody)). + Error("graph api error: " + resp.Status) + } +} + +func getRespDump(ctx context.Context, resp *http.Response, getBody bool) string { + respDump, err := httputil.DumpResponse(resp, getBody) + if err != nil { + logger.CtxErr(ctx, err).Error("dumping http response") + } + + return string(respDump) +} diff --git a/src/internal/m365/graph/middleware.go b/src/internal/m365/graph/middleware.go index 63789ab23..d5935a5b0 100644 --- a/src/internal/m365/graph/middleware.go +++ b/src/internal/m365/graph/middleware.go @@ -4,8 +4,6 @@ import ( "context" "io" "net/http" - "net/http/httputil" - "os" "strconv" "strings" "time" @@ -115,9 +113,6 @@ func LoggableURL(url string) pii.SafeURL { } } -// 1 MB -const logMBLimit = 1 * 1048576 - func (mw *LoggingMiddleware) Intercept( pipeline khttp.Pipeline, middlewareIndex int, @@ -138,56 +133,11 @@ func (mw *LoggingMiddleware) Intercept( "resp_status_code", resp.StatusCode, "resp_content_len", resp.ContentLength) - var ( - log = logger.Ctx(ctx) - respClass = resp.StatusCode / 100 - - // special cases where we always dump the response body, since the response - // details might be critical to understanding the response when debugging. - logBody = logger.DebugAPIFV || - os.Getenv(logGraphRequestsEnvKey) != "" || - resp.StatusCode == http.StatusBadRequest || - resp.StatusCode == http.StatusForbidden || - resp.StatusCode == http.StatusConflict - ) - - // special case: always info-level status 429 logs - if resp.StatusCode == http.StatusTooManyRequests { - log.With("response", getRespDump(ctx, resp, logBody)). - Info("graph api throttling") - - return resp, err - } - - // Log api calls according to api debugging configurations. - switch respClass { - case 2: - if logBody { - // only dump the body if it's under a size limit. We don't want to copy gigs into memory for a log. - dump := getRespDump(ctx, resp, os.Getenv(log2xxGraphResponseEnvKey) != "" && resp.ContentLength < logMBLimit) - log.Infow("2xx graph api resp", "response", dump) - } - case 3: - log.With("redirect_location", LoggableURL(resp.Header.Get(locationHeader))). - With("response", getRespDump(ctx, resp, false)). - Info("graph api redirect: " + resp.Status) - default: - log.With("response", getRespDump(ctx, resp, logBody)). - Error("graph api error: " + resp.Status) - } + logResp(ctx, resp) return resp, err } -func getRespDump(ctx context.Context, resp *http.Response, getBody bool) string { - respDump, err := httputil.DumpResponse(resp, getBody) - if err != nil { - logger.CtxErr(ctx, err).Error("dumping http response") - } - - return string(respDump) -} - // --------------------------------------------------------------------------- // Retry & Backoff // --------------------------------------------------------------------------- diff --git a/src/internal/m365/graph/service.go b/src/internal/m365/graph/service.go index eedda99df..0b54b5589 100644 --- a/src/internal/m365/graph/service.go +++ b/src/internal/m365/graph/service.go @@ -23,18 +23,15 @@ import ( ) const ( - logGraphRequestsEnvKey = "LOG_GRAPH_REQUESTS" - log2xxGraphRequestsEnvKey = "LOG_2XX_GRAPH_REQUESTS" - log2xxGraphResponseEnvKey = "LOG_2XX_GRAPH_RESPONSES" - defaultMaxRetries = 3 - defaultDelay = 3 * time.Second - locationHeader = "Location" - rateLimitHeader = "RateLimit-Limit" - rateRemainingHeader = "RateLimit-Remaining" - rateResetHeader = "RateLimit-Reset" - retryAfterHeader = "Retry-After" - retryAttemptHeader = "Retry-Attempt" - defaultHTTPClientTimeout = 1 * time.Hour + defaultMaxRetries = 3 + defaultDelay = 3 * time.Second + locationHeader = "Location" + rateLimitHeader = "RateLimit-Limit" + rateRemainingHeader = "RateLimit-Remaining" + rateResetHeader = "RateLimit-Reset" + retryAfterHeader = "Retry-After" + retryAttemptHeader = "Retry-Attempt" + defaultHTTPClientTimeout = 1 * time.Hour ) type QueryParams struct { diff --git a/src/internal/m365/service/sharepoint/enabled.go b/src/internal/m365/service/sharepoint/enabled.go index d816cad7f..b5507af0f 100644 --- a/src/internal/m365/service/sharepoint/enabled.go +++ b/src/internal/m365/service/sharepoint/enabled.go @@ -7,10 +7,11 @@ import ( "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/alcionai/corso/src/internal/m365/graph" + "github.com/alcionai/corso/src/pkg/services/m365/api" ) type getSiteRooter interface { - GetRoot(ctx context.Context) (models.Siteable, error) + GetRoot(ctx context.Context, cc api.CallConfig) (models.Siteable, error) } func IsServiceEnabled( @@ -18,7 +19,7 @@ func IsServiceEnabled( gsr getSiteRooter, resource string, ) (bool, error) { - _, err := gsr.GetRoot(ctx) + _, err := gsr.GetRoot(ctx, api.CallConfig{}) if err != nil { if clues.HasLabel(err, graph.LabelsNoSharePointLicense) { return false, nil diff --git a/src/internal/m365/service/sharepoint/enabled_test.go b/src/internal/m365/service/sharepoint/enabled_test.go index af9fe01fb..49d59b909 100644 --- a/src/internal/m365/service/sharepoint/enabled_test.go +++ b/src/internal/m365/service/sharepoint/enabled_test.go @@ -12,6 +12,7 @@ import ( "github.com/alcionai/corso/src/internal/m365/graph" "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/services/m365/api" ) type EnabledUnitSuite struct { @@ -29,7 +30,10 @@ type mockGSR struct { err error } -func (m mockGSR) GetRoot(context.Context) (models.Siteable, error) { +func (m mockGSR) GetRoot( + context.Context, + api.CallConfig, +) (models.Siteable, error) { return m.response, m.err } diff --git a/src/pkg/services/m365/api/item_pager.go b/src/pkg/services/m365/api/item_pager.go index d8fed9116..5effcb7a6 100644 --- a/src/pkg/services/m365/api/item_pager.go +++ b/src/pkg/services/m365/api/item_pager.go @@ -138,6 +138,11 @@ func deltaEnumerateItems[T any]( // Loop through all pages returned by Graph API. for len(nextLink) > 0 { page, err := pager.GetPage(graph.ConsumeNTokens(ctx, graph.SingleGetOrDeltaLC)) + if graph.IsErrDeltaNotSupported(err) { + logger.Ctx(ctx).Infow("delta queries not supported") + return nil, DeltaUpdate{}, clues.Stack(graph.ErrDeltaNotSupported, err) + } + if graph.IsErrInvalidDelta(err) { logger.Ctx(ctx).Infow("invalid previous delta", "delta_link", prevDeltaLink) @@ -191,7 +196,7 @@ func getAddedAndRemovedItemIDs[T any]( ) (map[string]time.Time, bool, []string, DeltaUpdate, error) { if canMakeDeltaQueries { ts, du, err := deltaEnumerateItems[T](ctx, deltaPager, prevDeltaLink) - if err != nil && (!graph.IsErrInvalidDelta(err) || len(prevDeltaLink) == 0) { + if err != nil && !graph.IsErrInvalidDelta(err) && !graph.IsErrDeltaNotSupported(err) { return nil, false, nil, DeltaUpdate{}, graph.Stack(ctx, err) } @@ -284,7 +289,15 @@ func addedAndRemovedByDeletedDateTime[T any]( var modTime time.Time if mt, ok := giaddt.(getModTimer); ok { - modTime = ptr.Val(mt.GetLastModifiedDateTime()) + // Make sure to get a non-zero mod time if the item doesn't have one for + // some reason. Otherwise we can hit an issue where kopia has a + // different mod time for the file than the details does. This occurs + // due to a conversion kopia does on the time from + // time.Time -> nanoseconds for serialization. During incremental + // backups, kopia goes from nanoseconds -> time.Time but there's an + // overflow which yields a different timestamp. + // https://github.com/gohugoio/hugo/issues/6161#issuecomment-725915786 + modTime = ptr.OrNow(mt.GetLastModifiedDateTime()) } added[ptr.Val(giaddt.GetId())] = modTime diff --git a/src/pkg/services/m365/api/sites.go b/src/pkg/services/m365/api/sites.go index 2ad3751f1..0865a4f47 100644 --- a/src/pkg/services/m365/api/sites.go +++ b/src/pkg/services/m365/api/sites.go @@ -35,11 +35,16 @@ type Sites struct { // api calls // --------------------------------------------------------------------------- -func (c Sites) GetRoot(ctx context.Context) (models.Siteable, error) { +func (c Sites) GetRoot( + ctx context.Context, + cc CallConfig, +) (models.Siteable, error) { options := &sites.SiteItemRequestBuilderGetRequestConfiguration{ - QueryParameters: &sites.SiteItemRequestBuilderGetQueryParameters{ - Expand: []string{"drive"}, - }, + QueryParameters: &sites.SiteItemRequestBuilderGetQueryParameters{}, + } + + if len(cc.Expand) > 0 { + options.QueryParameters.Expand = cc.Expand } resp, err := c.Stable. diff --git a/src/pkg/services/m365/api/sites_test.go b/src/pkg/services/m365/api/sites_test.go index 64b0387f6..ada517c92 100644 --- a/src/pkg/services/m365/api/sites_test.go +++ b/src/pkg/services/m365/api/sites_test.go @@ -256,7 +256,7 @@ func (suite *SitesIntgSuite) TestGetRoot() { ctx, flush := tester.NewContext(t) defer flush() - result, err := suite.its.ac.Sites().GetRoot(ctx) + result, err := suite.its.ac.Sites().GetRoot(ctx, api.CallConfig{Expand: []string{"drive"}}) require.NoError(t, err) require.NotNil(t, result, "must find the root site") require.NotEmpty(t, ptr.Val(result.GetId()), "must have an id") diff --git a/src/pkg/services/m365/sites.go b/src/pkg/services/m365/sites.go index 439f53616..a70553208 100644 --- a/src/pkg/services/m365/sites.go +++ b/src/pkg/services/m365/sites.go @@ -114,22 +114,26 @@ func ParseSite(item models.Siteable) *Site { if item.GetDrive() != nil && item.GetDrive().GetOwner() != nil && - item.GetDrive().GetOwner().GetUser() != nil { + item.GetDrive().GetOwner().GetUser() != nil && + // some users might come back with a nil ID + // most likely in the case of deleted users + item.GetDrive().GetOwner().GetUser().GetId() != nil { s.OwnerType = SiteOwnerUser s.OwnerID = ptr.Val(item.GetDrive().GetOwner().GetUser().GetId()) - } + } else if item.GetDrive() != nil && item.GetDrive().GetOwner() != nil { + ownerItem := item.GetDrive().GetOwner() + if _, ok := ownerItem.GetAdditionalData()["group"]; ok { + s.OwnerType = SiteOwnerGroup - if _, ok := item.GetAdditionalData()["group"]; ok { - s.OwnerType = SiteOwnerGroup + group, err := tform.AnyValueToT[map[string]any]("group", ownerItem.GetAdditionalData()) + if err != nil { + return s + } - group, err := tform.AnyValueToT[map[string]any]("group", item.GetAdditionalData()) - if err != nil { - return s - } - - s.OwnerID, err = str.AnyValueToString("id", group) - if err != nil { - return s + s.OwnerID, err = str.AnyValueToString("id", group) + if err != nil { + return s + } } } diff --git a/src/pkg/services/m365/sites_test.go b/src/pkg/services/m365/sites_test.go index de67a6de2..b23d84226 100644 --- a/src/pkg/services/m365/sites_test.go +++ b/src/pkg/services/m365/sites_test.go @@ -61,6 +61,34 @@ func (suite *siteIntegrationSuite) TestSites() { } } +func (suite *siteIntegrationSuite) TestSites_GetByID() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + acct := tconfig.NewM365Account(t) + + sites, err := Sites(ctx, acct, fault.New(true)) + assert.NoError(t, err, clues.ToCore(err)) + assert.NotEmpty(t, sites) + + for _, s := range sites { + suite.Run("site_"+s.ID, func() { + t := suite.T() + site, err := SiteByID(ctx, acct, s.ID) + assert.NoError(t, err, clues.ToCore(err)) + assert.NotEmpty(t, site.WebURL) + assert.NotEmpty(t, site.ID) + assert.NotEmpty(t, site.DisplayName) + if site.OwnerType != SiteOwnerUnknown { + assert.NotEmpty(t, site.OwnerID) + assert.NotEmpty(t, site.OwnerType) + } + }) + } +} + func (suite *siteIntegrationSuite) TestSites_InvalidCredentials() { table := []struct { name string diff --git a/website/package-lock.json b/website/package-lock.json index 704e4c27c..267aa055c 100644 --- a/website/package-lock.json +++ b/website/package-lock.json @@ -24,7 +24,7 @@ "prism-react-renderer": "^1.3.5", "react": "^17.0.2", "react-dom": "^17.0.2", - "sass": "^1.67.0", + "sass": "^1.68.0", "tiny-slider": "^2.9.4", "tw-elements": "^1.0.0-alpha13", "wow.js": "^1.2.2" @@ -12658,9 +12658,9 @@ "license": "MIT" }, "node_modules/sass": { - "version": "1.67.0", - "resolved": "https://registry.npmjs.org/sass/-/sass-1.67.0.tgz", - "integrity": "sha512-SVrO9ZeX/QQyEGtuZYCVxoeAL5vGlYjJ9p4i4HFuekWl8y/LtJ7tJc10Z+ck1c8xOuoBm2MYzcLfTAffD0pl/A==", + "version": "1.68.0", + "resolved": "https://registry.npmjs.org/sass/-/sass-1.68.0.tgz", + "integrity": "sha512-Lmj9lM/fef0nQswm1J2HJcEsBUba4wgNx2fea6yJHODREoMFnwRpZydBnX/RjyXw2REIwdkbqE4hrTo4qfDBUA==", "dependencies": { "chokidar": ">=3.0.0 <4.0.0", "immutable": "^4.0.0", @@ -23971,9 +23971,9 @@ "integrity": "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==" }, "sass": { - "version": "1.67.0", - "resolved": "https://registry.npmjs.org/sass/-/sass-1.67.0.tgz", - "integrity": "sha512-SVrO9ZeX/QQyEGtuZYCVxoeAL5vGlYjJ9p4i4HFuekWl8y/LtJ7tJc10Z+ck1c8xOuoBm2MYzcLfTAffD0pl/A==", + "version": "1.68.0", + "resolved": "https://registry.npmjs.org/sass/-/sass-1.68.0.tgz", + "integrity": "sha512-Lmj9lM/fef0nQswm1J2HJcEsBUba4wgNx2fea6yJHODREoMFnwRpZydBnX/RjyXw2REIwdkbqE4hrTo4qfDBUA==", "requires": { "chokidar": ">=3.0.0 <4.0.0", "immutable": "^4.0.0", diff --git a/website/package.json b/website/package.json index f65d2dd2d..08ddd9305 100644 --- a/website/package.json +++ b/website/package.json @@ -30,7 +30,7 @@ "prism-react-renderer": "^1.3.5", "react": "^17.0.2", "react-dom": "^17.0.2", - "sass": "^1.67.0", + "sass": "^1.68.0", "tiny-slider": "^2.9.4", "tw-elements": "^1.0.0-alpha13", "wow.js": "^1.2.2"