CLI connect refactor (#3213)

Move code for connecting to a repo into a common package so that backup and restore CLI code can both use it This will also make it easier for maintenance code in the future as it can reuse the same helper There are no logic changes in this PR, only code movement --- #### Does this PR need a docs update or release note? - [ ] ✅ Yes, it's included - [ ] 🕐 Yes, but in a later PR - [x] ⛔ No #### Type of change - [ ] 🌻 Feature - [ ] 🐛 Bugfix - [ ] 🗺️ Documentation - [ ] 🤖 Supportability/Tests - [ ] 💻 CI/Deployment - [x] 🧹 Tech Debt/Cleanup #### Issue(s) tangentially related to * #3077 #### Test Plan - [ ] 💪 Manual - [x] ⚡ Unit test - [ ] 💚 E2E
2023-04-25 11:35:50 -07:00 · 2023-04-25 11:35:50 -07:00 · 8a2e63dcad
commit 8a2e63dcad
parent 51809cf6eb
13 changed files with 59 additions and 83 deletions
--- a/src/cli/backup/backup.go
+++ b/src/cli/backup/backup.go
@ -9,13 +9,10 @@ import (
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"

-	"github.com/alcionai/corso/src/cli/config"
-	"github.com/alcionai/corso/src/cli/options"
 	. "github.com/alcionai/corso/src/cli/print"
 	"github.com/alcionai/corso/src/cli/utils"
 	"github.com/alcionai/corso/src/internal/common/idname"
 	"github.com/alcionai/corso/src/internal/data"
-	"github.com/alcionai/corso/src/pkg/account"
 	"github.com/alcionai/corso/src/pkg/backup"
 	"github.com/alcionai/corso/src/pkg/logger"
 	"github.com/alcionai/corso/src/pkg/path"
@ -270,7 +267,7 @@ func genericDeleteCommand(cmd *cobra.Command, bID, designation string, args []st

 	ctx := clues.Add(cmd.Context(), "delete_backup_id", bID)

-	r, _, err := getAccountAndConnect(ctx)
+	r, _, err := utils.GetAccountAndConnect(ctx)
 	if err != nil {
 		return Only(ctx, err)
 	}
@ -291,7 +288,7 @@ func genericDeleteCommand(cmd *cobra.Command, bID, designation string, args []st
 func genericListCommand(cmd *cobra.Command, bID string, service path.ServiceType, args []string) error {
 	ctx := cmd.Context()

-	r, _, err := getAccountAndConnect(ctx)
+	r, _, err := utils.GetAccountAndConnect(ctx)
 	if err != nil {
 		return Only(ctx, err)
 	}
@ -324,20 +321,6 @@ func genericListCommand(cmd *cobra.Command, bID string, service path.ServiceType
 	return nil
 }

-func getAccountAndConnect(ctx context.Context) (repository.Repository, *account.Account, error) {
-	cfg, err := config.GetConfigRepoDetails(ctx, true, nil)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	r, err := repository.Connect(ctx, cfg.Account, cfg.Storage, options.Control())
-	if err != nil {
-		return nil, nil, clues.Wrap(err, "Failed to connect to the "+cfg.Storage.Provider.String()+" repository")
-	}
-
-	return r, &cfg.Account, nil
-}
-
 func ifShow(flag string) bool {
 	return strings.ToLower(strings.TrimSpace(flag)) == "show"
 }
--- a/src/cli/backup/exchange.go
+++ b/src/cli/backup/exchange.go
@ -153,7 +153,7 @@ func createExchangeCmd(cmd *cobra.Command, args []string) error {
 		return err
 	}

-	r, acct, err := getAccountAndConnect(ctx)
+	r, acct, err := utils.GetAccountAndConnect(ctx)
 	if err != nil {
 		return Only(ctx, err)
 	}
@ -265,7 +265,7 @@ func detailsExchangeCmd(cmd *cobra.Command, args []string) error {
 	ctx := cmd.Context()
 	opts := utils.MakeExchangeOpts(cmd)

-	r, _, err := getAccountAndConnect(ctx)
+	r, _, err := utils.GetAccountAndConnect(ctx)
 	if err != nil {
 		return Only(ctx, err)
 	}
--- a/src/cli/backup/onedrive.go
+++ b/src/cli/backup/onedrive.go
@ -135,7 +135,7 @@ func createOneDriveCmd(cmd *cobra.Command, args []string) error {
 		return err
 	}

-	r, acct, err := getAccountAndConnect(ctx)
+	r, acct, err := utils.GetAccountAndConnect(ctx)
 	if err != nil {
 		return Only(ctx, err)
 	}
@ -224,7 +224,7 @@ func detailsOneDriveCmd(cmd *cobra.Command, args []string) error {
 	ctx := cmd.Context()
 	opts := utils.MakeOneDriveOpts(cmd)

-	r, _, err := getAccountAndConnect(ctx)
+	r, _, err := utils.GetAccountAndConnect(ctx)
 	if err != nil {
 		return Only(ctx, err)
 	}
--- a/src/cli/backup/sharepoint.go
+++ b/src/cli/backup/sharepoint.go
@ -146,7 +146,7 @@ func createSharePointCmd(cmd *cobra.Command, args []string) error {
 		return err
 	}

-	r, acct, err := getAccountAndConnect(ctx)
+	r, acct, err := utils.GetAccountAndConnect(ctx)
 	if err != nil {
 		return Only(ctx, err)
 	}
@ -308,7 +308,7 @@ func detailsSharePointCmd(cmd *cobra.Command, args []string) error {
 	ctx := cmd.Context()
 	opts := utils.MakeSharePointOpts(cmd)

-	r, _, err := getAccountAndConnect(ctx)
+	r, _, err := utils.GetAccountAndConnect(ctx)
 	if err != nil {
 		return Only(ctx, err)
 	}
--- a/src/cli/config/account.go
+++ b/src/cli/config/account.go
@ -6,7 +6,6 @@ import (
 	"github.com/alcionai/clues"
 	"github.com/spf13/viper"

-	"github.com/alcionai/corso/src/cli/utils"
 	"github.com/alcionai/corso/src/internal/common"
 	"github.com/alcionai/corso/src/pkg/account"
 	"github.com/alcionai/corso/src/pkg/credentials"
@ -72,7 +71,7 @@ func configureAccount(
 	}

 	// ensure required properties are present
-	if err := utils.RequireProps(map[string]string{
+	if err := requireProps(map[string]string{
 		credentials.AzureClientID:     m365Cfg.AzureClientID,
 		credentials.AzureClientSecret: m365Cfg.AzureClientSecret,
 		account.AzureTenantID:         m365Cfg.AzureTenantID,
--- a/src/cli/config/config.go
+++ b/src/cli/config/config.go
@ -321,3 +321,15 @@ func mustMatchConfig(vpr *viper.Viper, m map[string]string) error {

 	return nil
 }
+
+// requireProps validates the existence of the properties
+// in the map.  Expects the format map[propName]propVal.
+func requireProps(props map[string]string) error {
+	for name, val := range props {
+		if len(val) == 0 {
+			return clues.New(name + " is required to perform this command")
+		}
+	}
+
+	return nil
+}
--- a/src/cli/config/config_test.go
+++ b/src/cli/config/config_test.go
@ -39,6 +39,27 @@ func TestConfigSuite(t *testing.T) {
 	suite.Run(t, &ConfigSuite{Suite: tester.NewUnitSuite(t)})
 }

+func (suite *ConfigSuite) TestRequireProps() {
+	table := []struct {
+		name     string
+		props    map[string]string
+		errCheck assert.ErrorAssertionFunc
+	}{
+		{
+			props:    map[string]string{"exists": "I have seen the fnords!"},
+			errCheck: assert.NoError,
+		},
+		{
+			props:    map[string]string{"not-exists": ""},
+			errCheck: assert.Error,
+		},
+	}
+	for _, test := range table {
+		err := requireProps(test.props)
+		test.errCheck(suite.T(), err, clues.ToCore(err))
+	}
+}
+
 func (suite *ConfigSuite) TestReadRepoConfigBasic() {
 	var (
 		t   = suite.T()
--- a/src/cli/config/storage.go
+++ b/src/cli/config/storage.go
@ -9,7 +9,6 @@ import (
 	"github.com/aws/aws-sdk-go/aws/defaults"
 	"github.com/spf13/viper"

-	"github.com/alcionai/corso/src/cli/utils"
 	"github.com/alcionai/corso/src/internal/common"
 	"github.com/alcionai/corso/src/pkg/credentials"
 	"github.com/alcionai/corso/src/pkg/storage"
@ -112,7 +111,7 @@ func configureStorage(
 	}

 	// ensure required properties are present
-	if err := utils.RequireProps(map[string]string{
+	if err := requireProps(map[string]string{
 		storage.Bucket:              s3Cfg.Bucket,
 		credentials.CorsoPassphrase: corso.CorsoPassphrase,
 	}); err != nil {
--- a/src/cli/restore/exchange.go
+++ b/src/cli/restore/exchange.go
@ -6,14 +6,12 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"

-	"github.com/alcionai/corso/src/cli/config"
 	"github.com/alcionai/corso/src/cli/options"
 	. "github.com/alcionai/corso/src/cli/print"
 	"github.com/alcionai/corso/src/cli/utils"
 	"github.com/alcionai/corso/src/internal/common"
 	"github.com/alcionai/corso/src/internal/data"
 	"github.com/alcionai/corso/src/pkg/control"
-	"github.com/alcionai/corso/src/pkg/repository"
 )

 // called by restore.go to map subcommands to provider-specific handling.
@ -90,16 +88,11 @@ func restoreExchangeCmd(cmd *cobra.Command, args []string) error {
 		return err
 	}

-	cfg, err := config.GetConfigRepoDetails(ctx, true, nil)
+	r, _, err := utils.GetAccountAndConnect(ctx)
 	if err != nil {
 		return Only(ctx, err)
 	}

-	r, err := repository.Connect(ctx, cfg.Account, cfg.Storage, options.Control())
-	if err != nil {
-		return Only(ctx, clues.Wrap(err, "Failed to connect to the "+cfg.Storage.Provider.String()+" repository"))
-	}
-
 	defer utils.CloseRepo(ctx, r)

 	dest := control.DefaultRestoreDestination(common.SimpleDateTime)
--- a/src/cli/restore/onedrive.go
+++ b/src/cli/restore/onedrive.go
@ -6,14 +6,12 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"

-	"github.com/alcionai/corso/src/cli/config"
 	"github.com/alcionai/corso/src/cli/options"
 	. "github.com/alcionai/corso/src/cli/print"
 	"github.com/alcionai/corso/src/cli/utils"
 	"github.com/alcionai/corso/src/internal/common"
 	"github.com/alcionai/corso/src/internal/data"
 	"github.com/alcionai/corso/src/pkg/control"
-	"github.com/alcionai/corso/src/pkg/repository"
 )

 // called by restore.go to map subcommands to provider-specific handling.
@ -92,16 +90,11 @@ func restoreOneDriveCmd(cmd *cobra.Command, args []string) error {
 		return err
 	}

-	cfg, err := config.GetConfigRepoDetails(ctx, true, nil)
+	r, _, err := utils.GetAccountAndConnect(ctx)
 	if err != nil {
 		return Only(ctx, err)
 	}

-	r, err := repository.Connect(ctx, cfg.Account, cfg.Storage, options.Control())
-	if err != nil {
-		return Only(ctx, clues.Wrap(err, "Failed to connect to the "+cfg.Storage.Provider.String()+" repository"))
-	}
-
 	defer utils.CloseRepo(ctx, r)

 	dest := control.DefaultRestoreDestination(common.SimpleDateTimeOneDrive)
--- a/src/cli/restore/sharepoint.go
+++ b/src/cli/restore/sharepoint.go
@ -6,14 +6,12 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"

-	"github.com/alcionai/corso/src/cli/config"
 	"github.com/alcionai/corso/src/cli/options"
 	. "github.com/alcionai/corso/src/cli/print"
 	"github.com/alcionai/corso/src/cli/utils"
 	"github.com/alcionai/corso/src/internal/common"
 	"github.com/alcionai/corso/src/internal/data"
 	"github.com/alcionai/corso/src/pkg/control"
-	"github.com/alcionai/corso/src/pkg/repository"
 )

 // called by restore.go to map subcommands to provider-specific handling.
@ -91,16 +89,11 @@ func restoreSharePointCmd(cmd *cobra.Command, args []string) error {
 		return err
 	}

-	cfg, err := config.GetConfigRepoDetails(ctx, true, nil)
+	r, _, err := utils.GetAccountAndConnect(ctx)
 	if err != nil {
 		return Only(ctx, err)
 	}

-	r, err := repository.Connect(ctx, cfg.Account, cfg.Storage, options.Control())
-	if err != nil {
-		return Only(ctx, clues.Wrap(err, "Failed to connect to the "+cfg.Storage.Provider.String()+" repository"))
-	}
-
 	defer utils.CloseRepo(ctx, r)

 	dest := control.DefaultRestoreDestination(common.SimpleDateTimeOneDrive)
--- a/src/cli/utils/utils.go
+++ b/src/cli/utils/utils.go
@ -8,7 +8,10 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"

+	"github.com/alcionai/corso/src/cli/config"
+	"github.com/alcionai/corso/src/cli/options"
 	"github.com/alcionai/corso/src/internal/events"
+	"github.com/alcionai/corso/src/pkg/account"
 	"github.com/alcionai/corso/src/pkg/control"
 	"github.com/alcionai/corso/src/pkg/logger"
 	"github.com/alcionai/corso/src/pkg/path"
@ -21,16 +24,18 @@ const (
 	Wildcard = "*"
 )

-// RequireProps validates the existence of the properties
-// in the map.  Expects the format map[propName]propVal.
-func RequireProps(props map[string]string) error {
-	for name, val := range props {
-		if len(val) == 0 {
-			return clues.New(name + " is required to perform this command")
-		}
+func GetAccountAndConnect(ctx context.Context) (repository.Repository, *account.Account, error) {
+	cfg, err := config.GetConfigRepoDetails(ctx, true, nil)
+	if err != nil {
+		return nil, nil, err
 	}

-	return nil
+	r, err := repository.Connect(ctx, cfg.Account, cfg.Storage, options.Control())
+	if err != nil {
+		return nil, nil, clues.Wrap(err, "Failed to connect to the "+cfg.Storage.Provider.String()+" repository")
+	}
+
+	return r, &cfg.Account, nil
 }

 // CloseRepo handles closing a repo.
--- a/src/cli/utils/utils_test.go
+++ b/src/cli/utils/utils_test.go
@ -3,7 +3,6 @@ package utils
 import (
 	"testing"

-	"github.com/alcionai/clues"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"

@ -19,27 +18,6 @@ func TestCliUtilsSuite(t *testing.T) {
 	suite.Run(t, &CliUtilsSuite{Suite: tester.NewUnitSuite(t)})
 }

-func (suite *CliUtilsSuite) TestRequireProps() {
-	table := []struct {
-		name     string
-		props    map[string]string
-		errCheck assert.ErrorAssertionFunc
-	}{
-		{
-			props:    map[string]string{"exists": "I have seen the fnords!"},
-			errCheck: assert.NoError,
-		},
-		{
-			props:    map[string]string{"not-exists": ""},
-			errCheck: assert.Error,
-		},
-	}
-	for _, test := range table {
-		err := RequireProps(test.props)
-		test.errCheck(suite.T(), err, clues.ToCore(err))
-	}
-}
-
 func (suite *CliUtilsSuite) TestSplitFoldersIntoContainsAndPrefix() {
 	table := []struct {
 		name    string