remove --all flag support, minor cleanup (#1047)

## Description

Removes the -all flag from the exchange cli.  The
same functionality can be gained with the flag
--user *.

Additionally does some minor tidying of other
comments and outputs throughout the cli.

## Type of change

- [x] 🐹 Trivial/Minor

## Issue(s)

* #1032

## Test Plan

- [x]  Unit test
This commit is contained in:
Keepers 2022-10-05 18:10:56 -06:00 committed by GitHub
parent cba065ec94
commit 78632f56e7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 54 additions and 80 deletions

View File

@ -27,7 +27,6 @@ import (
// exchange bucket info from flags // exchange bucket info from flags
var ( var (
backupID string backupID string
exchangeAll bool
exchangeData []string exchangeData []string
user []string user []string
@ -59,9 +58,9 @@ const (
const ( const (
exchangeServiceCommand = "exchange" exchangeServiceCommand = "exchange"
exchangeServiceCommandCreateUseSuffix = " --user <userId or email> | '" + utils.Wildcard + "'" exchangeServiceCommandCreateUseSuffix = "--user <userId or email> | '" + utils.Wildcard + "'"
exchangeServiceCommandDeleteUseSuffix = " --backup <backupId>" exchangeServiceCommandDeleteUseSuffix = "--backup <backupId>"
exchangeServiceCommandDetailsUseSuffix = " --backup <backupId>" exchangeServiceCommandDetailsUseSuffix = "--backup <backupId>"
) )
const ( const (
@ -104,14 +103,11 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
case createCommand: case createCommand:
c, fs = utils.AddCommand(parent, exchangeCreateCmd()) c, fs = utils.AddCommand(parent, exchangeCreateCmd())
c.Use = c.Use + exchangeServiceCommandCreateUseSuffix c.Use = c.Use + " " + exchangeServiceCommandCreateUseSuffix
c.Example = exchangeServiceCommandCreateExamples c.Example = exchangeServiceCommandCreateExamples
// Flags addition ordering should follow the order we want them to appear in help and docs: // Flags addition ordering should follow the order we want them to appear in help and docs:
// More generic (ex: --all) and more frequently used flags take precedence. // More generic (ex: --user) and more frequently used flags take precedence.
fs.BoolVar(&exchangeAll,
"all", false,
"Backup all Exchange data for all users")
fs.StringSliceVar( fs.StringSliceVar(
&user, &user,
"user", nil, "user", nil,
@ -128,11 +124,11 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
case detailsCommand: case detailsCommand:
c, fs = utils.AddCommand(parent, exchangeDetailsCmd()) c, fs = utils.AddCommand(parent, exchangeDetailsCmd())
c.Use = c.Use + exchangeServiceCommandDetailsUseSuffix c.Use = c.Use + " " + exchangeServiceCommandDetailsUseSuffix
c.Example = exchangeServiceCommandDetailsExamples c.Example = exchangeServiceCommandDetailsExamples
// Flags addition ordering should follow the order we want them to appear in help and docs: // Flags addition ordering should follow the order we want them to appear in help and docs:
// More generic (ex: --all) and more frequently used flags take precedence. // More generic (ex: --user) and more frequently used flags take precedence.
fs.StringVar(&backupID, fs.StringVar(&backupID,
"backup", "", "backup", "",
"ID of the backup to explore. (required)") "ID of the backup to explore. (required)")
@ -216,7 +212,7 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
case deleteCommand: case deleteCommand:
c, fs = utils.AddCommand(parent, exchangeDeleteCmd()) c, fs = utils.AddCommand(parent, exchangeDeleteCmd())
c.Use = c.Use + exchangeServiceCommandDeleteUseSuffix c.Use = c.Use + " " + exchangeServiceCommandDeleteUseSuffix
c.Example = exchangeServiceCommandDeleteExamples c.Example = exchangeServiceCommandDeleteExamples
fs.StringVar(&backupID, "backup", "", "ID of the backup to delete. (required)") fs.StringVar(&backupID, "backup", "", "ID of the backup to delete. (required)")
@ -248,7 +244,7 @@ func createExchangeCmd(cmd *cobra.Command, args []string) error {
return nil return nil
} }
if err := validateExchangeBackupCreateFlags(exchangeAll, user, exchangeData); err != nil { if err := validateExchangeBackupCreateFlags(user, exchangeData); err != nil {
return err return err
} }
@ -264,7 +260,7 @@ func createExchangeCmd(cmd *cobra.Command, args []string) error {
defer utils.CloseRepo(ctx, r) defer utils.CloseRepo(ctx, r)
sel := exchangeBackupCreateSelectors(exchangeAll, user, exchangeData) sel := exchangeBackupCreateSelectors(user, exchangeData)
bo, err := r.NewBackup(ctx, sel) bo, err := r.NewBackup(ctx, sel)
if err != nil { if err != nil {
@ -286,40 +282,32 @@ func createExchangeCmd(cmd *cobra.Command, args []string) error {
return nil return nil
} }
func exchangeBackupCreateSelectors(all bool, users, data []string) selectors.Selector { func exchangeBackupCreateSelectors(userIDs, data []string) selectors.Selector {
sel := selectors.NewExchangeBackup() sel := selectors.NewExchangeBackup()
if all {
sel.Include(sel.Users(selectors.Any()))
return sel.Selector
}
if len(data) == 0 { if len(data) == 0 {
sel.Include(sel.ContactFolders(user, selectors.Any())) sel.Include(sel.ContactFolders(userIDs, selectors.Any()))
sel.Include(sel.MailFolders(user, selectors.Any())) sel.Include(sel.MailFolders(userIDs, selectors.Any()))
sel.Include(sel.EventCalendars(user, selectors.Any())) sel.Include(sel.EventCalendars(userIDs, selectors.Any()))
} }
for _, d := range data { for _, d := range data {
switch d { switch d {
case dataContacts: case dataContacts:
sel.Include(sel.ContactFolders(users, selectors.Any())) sel.Include(sel.ContactFolders(userIDs, selectors.Any()))
case dataEmail: case dataEmail:
sel.Include(sel.MailFolders(users, selectors.Any())) sel.Include(sel.MailFolders(userIDs, selectors.Any()))
case dataEvents: case dataEvents:
sel.Include(sel.EventCalendars(users, selectors.Any())) sel.Include(sel.EventCalendars(userIDs, selectors.Any()))
} }
} }
return sel.Selector return sel.Selector
} }
func validateExchangeBackupCreateFlags(all bool, users, data []string) error { func validateExchangeBackupCreateFlags(userIDs, data []string) error {
if len(users) == 0 && !all { if len(userIDs) == 0 {
return errors.New("requires one or more --user ids, the wildcard --user *, or the --all flag") return errors.New("--user requires one or more ids or the wildcard *")
}
if len(data) > 0 && all {
return errors.New("--all does a backup on all data, and cannot be reduced with --data")
} }
for _, d := range data { for _, d := range data {

View File

@ -33,7 +33,7 @@ func (suite *ExchangeSuite) TestAddExchangeCommands() {
expectRunE func(*cobra.Command, []string) error expectRunE func(*cobra.Command, []string) error
}{ }{
{ {
"create exchange", createCommand, expectUse + exchangeServiceCommandCreateUseSuffix, "create exchange", createCommand, expectUse + " " + exchangeServiceCommandCreateUseSuffix,
exchangeCreateCmd().Short, createExchangeCmd, exchangeCreateCmd().Short, createExchangeCmd,
}, },
{ {
@ -41,11 +41,11 @@ func (suite *ExchangeSuite) TestAddExchangeCommands() {
exchangeListCmd().Short, listExchangeCmd, exchangeListCmd().Short, listExchangeCmd,
}, },
{ {
"details exchange", detailsCommand, expectUse + exchangeServiceCommandDetailsUseSuffix, "details exchange", detailsCommand, expectUse + " " + exchangeServiceCommandDetailsUseSuffix,
exchangeDetailsCmd().Short, detailsExchangeCmd, exchangeDetailsCmd().Short, detailsExchangeCmd,
}, },
{ {
"delete exchange", deleteCommand, expectUse + exchangeServiceCommandDeleteUseSuffix, "delete exchange", deleteCommand, expectUse + " " + exchangeServiceCommandDeleteUseSuffix,
exchangeDeleteCmd().Short, deleteExchangeCmd, exchangeDeleteCmd().Short, deleteExchangeCmd,
}, },
} }
@ -70,46 +70,33 @@ func (suite *ExchangeSuite) TestAddExchangeCommands() {
func (suite *ExchangeSuite) TestValidateBackupCreateFlags() { func (suite *ExchangeSuite) TestValidateBackupCreateFlags() {
table := []struct { table := []struct {
name string name string
a bool
user, data []string user, data []string
expect assert.ErrorAssertionFunc expect assert.ErrorAssertionFunc
}{ }{
{ {
name: "no users, not any", name: "no users or data",
expect: assert.Error, expect: assert.Error,
}, },
{ {
name: "any and data", name: "no users only data",
a: true,
data: []string{dataEmail}, data: []string{dataEmail},
expect: assert.Error, expect: assert.Error,
}, },
{ {
name: "unrecognized data", name: "unrecognized data category",
user: []string{"fnord"}, user: []string{"fnord"},
data: []string{"smurfs"}, data: []string{"smurfs"},
expect: assert.Error, expect: assert.Error,
}, },
{ {
name: "users, not any", name: "only users no data",
user: []string{"fnord"},
expect: assert.NoError,
},
{
name: "no users, any",
a: true,
expect: assert.NoError,
},
{
name: "users, any",
a: true,
user: []string{"fnord"}, user: []string{"fnord"},
expect: assert.NoError, expect: assert.NoError,
}, },
} }
for _, test := range table { for _, test := range table {
suite.T().Run(test.name, func(t *testing.T) { suite.T().Run(test.name, func(t *testing.T) {
test.expect(t, validateExchangeBackupCreateFlags(test.a, test.user, test.data)) test.expect(t, validateExchangeBackupCreateFlags(test.user, test.data))
}) })
} }
} }
@ -117,13 +104,11 @@ func (suite *ExchangeSuite) TestValidateBackupCreateFlags() {
func (suite *ExchangeSuite) TestExchangeBackupCreateSelectors() { func (suite *ExchangeSuite) TestExchangeBackupCreateSelectors() {
table := []struct { table := []struct {
name string name string
a bool
user, data []string user, data []string
expectIncludeLen int expectIncludeLen int
}{ }{
{ {
name: "any", name: "default: one of each category, all None() matchers",
a: true,
expectIncludeLen: 3, expectIncludeLen: 3,
}, },
{ {
@ -223,7 +208,7 @@ func (suite *ExchangeSuite) TestExchangeBackupCreateSelectors() {
} }
for _, test := range table { for _, test := range table {
suite.T().Run(test.name, func(t *testing.T) { suite.T().Run(test.name, func(t *testing.T) {
sel := exchangeBackupCreateSelectors(test.a, test.user, test.data) sel := exchangeBackupCreateSelectors(test.user, test.data)
assert.Equal(t, test.expectIncludeLen, len(sel.Includes)) assert.Equal(t, test.expectIncludeLen, len(sel.Includes))
}) })
} }

View File

@ -25,9 +25,9 @@ import (
const ( const (
oneDriveServiceCommand = "onedrive" oneDriveServiceCommand = "onedrive"
oneDriveServiceCommandCreateUseSuffix = " --user <userId or email> | '" + utils.Wildcard + "'" oneDriveServiceCommandCreateUseSuffix = "--user <userId or email> | '" + utils.Wildcard + "'"
oneDriveServiceCommandDeleteUseSuffix = " --backup <backupId>" oneDriveServiceCommandDeleteUseSuffix = "--backup <backupId>"
oneDriveServiceCommandDetailsUseSuffix = " --backup <backupId>" oneDriveServiceCommandDetailsUseSuffix = "--backup <backupId>"
) )
const ( const (
@ -76,7 +76,7 @@ func addOneDriveCommands(parent *cobra.Command) *cobra.Command {
case createCommand: case createCommand:
c, fs = utils.AddCommand(parent, oneDriveCreateCmd()) c, fs = utils.AddCommand(parent, oneDriveCreateCmd())
c.Use = c.Use + oneDriveServiceCommandCreateUseSuffix c.Use = c.Use + " " + oneDriveServiceCommandCreateUseSuffix
c.Example = oneDriveServiceCommandCreateExamples c.Example = oneDriveServiceCommandCreateExamples
fs.StringArrayVar(&user, "user", nil, fs.StringArrayVar(&user, "user", nil,
@ -89,7 +89,7 @@ func addOneDriveCommands(parent *cobra.Command) *cobra.Command {
case detailsCommand: case detailsCommand:
c, fs = utils.AddCommand(parent, oneDriveDetailsCmd()) c, fs = utils.AddCommand(parent, oneDriveDetailsCmd())
c.Use = c.Use + oneDriveServiceCommandDetailsUseSuffix c.Use = c.Use + " " + oneDriveServiceCommandDetailsUseSuffix
c.Example = oneDriveServiceCommandDetailsExamples c.Example = oneDriveServiceCommandDetailsExamples
fs.StringVar(&backupID, "backup", "", "ID of the backup to explore. (required)") fs.StringVar(&backupID, "backup", "", "ID of the backup to explore. (required)")
@ -130,7 +130,7 @@ func addOneDriveCommands(parent *cobra.Command) *cobra.Command {
case deleteCommand: case deleteCommand:
c, fs = utils.AddCommand(parent, oneDriveDeleteCmd()) c, fs = utils.AddCommand(parent, oneDriveDeleteCmd())
c.Use = c.Use + oneDriveServiceCommandDeleteUseSuffix c.Use = c.Use + " " + oneDriveServiceCommandDeleteUseSuffix
c.Example = oneDriveServiceCommandDeleteExamples c.Example = oneDriveServiceCommandDeleteExamples
fs.StringVar(&backupID, "backup", "", "ID of the backup to delete. (required)") fs.StringVar(&backupID, "backup", "", "ID of the backup to delete. (required)")

View File

@ -30,7 +30,7 @@ func (suite *OneDriveSuite) TestAddOneDriveCommands() {
expectRunE func(*cobra.Command, []string) error expectRunE func(*cobra.Command, []string) error
}{ }{
{ {
"create onedrive", createCommand, expectUse + oneDriveServiceCommandCreateUseSuffix, "create onedrive", createCommand, expectUse + " " + oneDriveServiceCommandCreateUseSuffix,
oneDriveCreateCmd().Short, createOneDriveCmd, oneDriveCreateCmd().Short, createOneDriveCmd,
}, },
{ {
@ -38,11 +38,11 @@ func (suite *OneDriveSuite) TestAddOneDriveCommands() {
oneDriveListCmd().Short, listOneDriveCmd, oneDriveListCmd().Short, listOneDriveCmd,
}, },
{ {
"details onedrive", detailsCommand, expectUse + oneDriveServiceCommandDetailsUseSuffix, "details onedrive", detailsCommand, expectUse + " " + oneDriveServiceCommandDetailsUseSuffix,
oneDriveDetailsCmd().Short, detailsOneDriveCmd, oneDriveDetailsCmd().Short, detailsOneDriveCmd,
}, },
{ {
"delete onedrive", deleteCommand, expectUse + oneDriveServiceCommandDeleteUseSuffix, "delete onedrive", deleteCommand, expectUse + " " + oneDriveServiceCommandDeleteUseSuffix,
oneDriveDeleteCmd().Short, deleteOneDriveCmd, oneDriveDeleteCmd().Short, deleteOneDriveCmd,
}, },
} }

View File

@ -37,18 +37,19 @@ func addS3Commands(parent *cobra.Command) *cobra.Command {
c, fs = utils.AddCommand(parent, s3ConnectCmd()) c, fs = utils.AddCommand(parent, s3ConnectCmd())
} }
c.Use = c.Use + s3ProviderCommandUseSuffix c.Use = c.Use + " " + s3ProviderCommandUseSuffix
c.SetUsageTemplate(parent.UsageTemplate()) c.SetUsageTemplate(parent.UsageTemplate())
// Flags addition ordering should follow the order we want them to appear in help and docs: // Flags addition ordering should follow the order we want them to appear in help and docs:
// More generic (ex: --all) and more frequently used flags take precedence. // More generic and more frequently used flags take precedence.
fs.StringVar(&bucket, "bucket", "", "Name of S3 bucket for repo. (required)") fs.StringVar(&bucket, "bucket", "", "Name of S3 bucket for repo. (required)")
cobra.CheckErr(c.MarkFlagRequired("bucket")) cobra.CheckErr(c.MarkFlagRequired("bucket"))
fs.StringVar(&prefix, "prefix", "", "Repo prefix within bucket.") fs.StringVar(&prefix, "prefix", "", "Repo prefix within bucket.")
fs.StringVar(&endpoint, "endpoint", "s3.amazonaws.com", "S3 service endpoint.") fs.StringVar(&endpoint, "endpoint", "s3.amazonaws.com", "S3 service endpoint.")
fs.BoolVar(&succeedIfExists, "succeed-if-exists", false, "Exit with success if the repo has already been initialized.")
// In general, we don't want to expose this flag to users and have them mistake it // In general, we don't want to expose this flag to users and have them mistake it
// for a broad-scale idempotency solution. We can un-hide it later the need arises. // for a broad-scale idempotency solution. We can un-hide it later the need arises.
fs.BoolVar(&succeedIfExists, "succeed-if-exists", false, "Exit with success if the repo has already been initialized.")
cobra.CheckErr(fs.MarkHidden("succeed-if-exists")) cobra.CheckErr(fs.MarkHidden("succeed-if-exists"))
return c return c
@ -56,7 +57,7 @@ func addS3Commands(parent *cobra.Command) *cobra.Command {
const ( const (
s3ProviderCommand = "s3" s3ProviderCommand = "s3"
s3ProviderCommandUseSuffix = " --bucket <bucket>" s3ProviderCommandUseSuffix = "--bucket <bucket>"
) )
const ( const (

View File

@ -20,7 +20,7 @@ func TestS3Suite(t *testing.T) {
} }
func (suite *S3Suite) TestAddS3Commands() { func (suite *S3Suite) TestAddS3Commands() {
expectUse := s3ProviderCommand + s3ProviderCommandUseSuffix expectUse := s3ProviderCommand + " " + s3ProviderCommandUseSuffix
table := []struct { table := []struct {
name string name string

View File

@ -51,10 +51,10 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
case restoreCommand: case restoreCommand:
c, fs = utils.AddCommand(parent, exchangeRestoreCmd()) c, fs = utils.AddCommand(parent, exchangeRestoreCmd())
c.Use = c.Use + exchangeServiceCommandUseSuffix c.Use = c.Use + " " + exchangeServiceCommandUseSuffix
// Flags addition ordering should follow the order we want them to appear in help and docs: // Flags addition ordering should follow the order we want them to appear in help and docs:
// More generic (ex: --all) and more frequently used flags take precedence. // More generic (ex: --user) and more frequently used flags take precedence.
// general flags // general flags
fs.StringVar(&backupID, "backup", "", "ID of the backup to restore. (required)") fs.StringVar(&backupID, "backup", "", "ID of the backup to restore. (required)")
cobra.CheckErr(c.MarkFlagRequired("backup")) cobra.CheckErr(c.MarkFlagRequired("backup"))
@ -140,7 +140,7 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
const ( const (
exchangeServiceCommand = "exchange" exchangeServiceCommand = "exchange"
exchangeServiceCommandUseSuffix = " --backup <backupId>" exchangeServiceCommandUseSuffix = "--backup <backupId>"
exchangeServiceCommandRestoreExamples = `# Restore emails with ID 98765abcdef and 12345abcdef from a specific backup exchangeServiceCommandRestoreExamples = `# Restore emails with ID 98765abcdef and 12345abcdef from a specific backup
corso restore exchange --backup 1234abcd-12ab-cd34-56de-1234abcd --email 98765abcdef,12345abcdef corso restore exchange --backup 1234abcd-12ab-cd34-56de-1234abcd --email 98765abcdef,12345abcdef

View File

@ -20,7 +20,7 @@ func TestExchangeSuite(t *testing.T) {
} }
func (suite *ExchangeSuite) TestAddExchangeCommands() { func (suite *ExchangeSuite) TestAddExchangeCommands() {
expectUse := exchangeServiceCommand + exchangeServiceCommandUseSuffix expectUse := exchangeServiceCommand + " " + exchangeServiceCommandUseSuffix
table := []struct { table := []struct {
name string name string

View File

@ -36,10 +36,10 @@ func addOneDriveCommands(parent *cobra.Command) *cobra.Command {
case restoreCommand: case restoreCommand:
c, fs = utils.AddCommand(parent, oneDriveRestoreCmd()) c, fs = utils.AddCommand(parent, oneDriveRestoreCmd())
c.Use = c.Use + oneDriveServiceCommandUseSuffix c.Use = c.Use + " " + oneDriveServiceCommandUseSuffix
// Flags addition ordering should follow the order we want them to appear in help and docs: // Flags addition ordering should follow the order we want them to appear in help and docs:
// More generic (ex: --all) and more frequently used flags take precedence. // More generic (ex: --user) and more frequently used flags take precedence.
fs.SortFlags = false fs.SortFlags = false
fs.StringVar(&backupID, "backup", "", "ID of the backup to restore. (required)") fs.StringVar(&backupID, "backup", "", "ID of the backup to restore. (required)")
@ -90,7 +90,7 @@ func addOneDriveCommands(parent *cobra.Command) *cobra.Command {
const ( const (
oneDriveServiceCommand = "onedrive" oneDriveServiceCommand = "onedrive"
oneDriveServiceCommandUseSuffix = " --backup <backupId>" oneDriveServiceCommandUseSuffix = "--backup <backupId>"
oneDriveServiceCommandRestoreExamples = `# Restore file with ID 98765abcdef oneDriveServiceCommandRestoreExamples = `# Restore file with ID 98765abcdef
corso restore onedrive --backup 1234abcd-12ab-cd34-56de-1234abcd --file 98765abcdef corso restore onedrive --backup 1234abcd-12ab-cd34-56de-1234abcd --file 98765abcdef

View File

@ -20,7 +20,7 @@ func TestOneDriveSuite(t *testing.T) {
} }
func (suite *OneDriveSuite) TestAddOneDriveCommands() { func (suite *OneDriveSuite) TestAddOneDriveCommands() {
expectUse := oneDriveServiceCommand + oneDriveServiceCommandUseSuffix expectUse := oneDriveServiceCommand + " " + oneDriveServiceCommandUseSuffix
table := []struct { table := []struct {
name string name string

View File

@ -279,7 +279,7 @@ func (suite *GraphConnectorIntegrationSuite) TestEventsSerializationRegression()
} }
// TestAccessOfInboxAllUsers verifies that GraphConnector can // TestAccessOfInboxAllUsers verifies that GraphConnector can
// support `--all-users` for backup operations. Selector.DiscreteScopes // support `--users *` for backup operations. Selector.DiscreteScopes
// returns all of the users within one scope. Only users who have // returns all of the users within one scope. Only users who have
// messages in their inbox will have a collection returned. // messages in their inbox will have a collection returned.
// The final test insures that more than a 75% of the user collections are // The final test insures that more than a 75% of the user collections are