fix: Extracted replacing auth token to function to reduce cognitive c… (

#25) * fix: Extracted replacing auth token to function to reduce cognitive complexity * fix: plural tokens * fix: tried to label but didn't in the end * fix: add actually working tests this time * fix(style): remove comments --------- Co-authored-by: dnitsch <[email protected]>
dnitsch · Feb 22, 2023 · 2a52682 · 2a52682
1 parent 8384fe9
commit 2a52682
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 56 deletions.
diff --git a/seeder/cmd/strategyrestseeder/root.go b/seeder/cmd/strategyrestseeder/root.go
@@ -10,7 +10,7 @@ import (
 
 var (
 	verbose               bool
-	strategyrestseederCmd = &cobra.Command{
+	StrategyRestSeederCmd = &cobra.Command{
 		Use:     config.SELF_NAME,
 		Aliases: config.SHORT_NAME,
 		Short:   fmt.Sprintf("%s CLI provides an idempotent rest caller", config.SELF_NAME),
@@ -19,13 +19,13 @@ var (
 )
 
 func Execute() {
-	if err := strategyrestseederCmd.Execute(); err != nil {
+	if err := StrategyRestSeederCmd.Execute(); err != nil {
 		fmt.Errorf("cli error: %v", err)
 		os.Exit(1)
 	}
 	os.Exit(0)
 }
 
 func init() {
-	strategyrestseederCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "Verbosity level")
+	StrategyRestSeederCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "Verbosity level")
 }
diff --git a/seeder/cmd/strategyrestseeder/runseed.go b/seeder/cmd/strategyrestseeder/runseed.go
@@ -28,7 +28,7 @@ var (
 		RunE:    runExecute,
 		PreRunE: func(cmd *cobra.Command, args []string) error {
 			if len(path) < 1 {
-				return fmt.Errorf("must include input")
+				return fmt.Errorf("must include input for path")
 			}
 			return nil
 		},
@@ -40,7 +40,7 @@ func init() {
 	runCmd.PersistentFlags().BoolVarP(&enableConfigManager, "enable-config-manager", "c", false, "Enables config manager to replace placeholders for secret values")
 	runCmd.PersistentFlags().StringVarP(&cmTokenSeparator, "cm-token-separator", "t", "", `Config Manager token separator`)
 	runCmd.PersistentFlags().StringVarP(&cmKeySeparator, "cm-key-separator", "k", "", `Config Manager key separator`)
-	strategyrestseederCmd.AddCommand(runCmd)
+	StrategyRestSeederCmd.AddCommand(runCmd)
 }
 
 func runExecute(cmd *cobra.Command, args []string) error {

diff --git a/seeder/cmd/strategyrestseeder/runseed_test.go b/seeder/cmd/strategyrestseeder/runseed_test.go
@@ -9,19 +9,18 @@ import (
 	"testing"
 
 	"github.com/dnitsch/reststrategy/seeder"
+
 	"gopkg.in/yaml.v2"
 )
 
 func helperTestSeed(conf *seeder.StrategyConfig) string {
-	// originalArg := os.Args[0:1]
-	// os.Args = originalArg
-
 	b, _ := yaml.Marshal(conf)
 	dir, _ := os.MkdirTemp("", "seed-cli-test")
 	file := filepath.Join(dir, "seeder.yml")
 	_ = os.WriteFile(file, b, 0777)
 	return file
 }
+
 func TestMainRunSeed(t *testing.T) {
 	ttests := map[string]struct {
 		// path to file and delete file return
@@ -76,7 +75,6 @@ func TestMainRunSeed(t *testing.T) {
 				file := helperTestSeed(conf)
 				return []string{"run"}, func() {
 					os.Remove(file)
-					os.Args = os.Args[0:1]
 				}
 			},
 			"must include input",
@@ -86,39 +84,35 @@ func TestMainRunSeed(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			cmdArgs, cleanUp := tt.testInput(t, "")
 			defer cleanUp()
-			b := &bytes.Buffer{}
-			e := &bytes.Buffer{}
-			cmd := runCmd
-			// cmd.SetArgs did not work with this set up
-			os.Args = os.Args[0:1]
-			os.Args = append(os.Args, cmdArgs...)
-			cmd.SetOut(b)
-			cmd.SetErr(e)
+			b := new(bytes.Buffer)
+
+			cmd := StrategyRestSeederCmd
+
+			cmd.SetArgs(cmdArgs)
+			cmd.SetErr(b)
 			cmd.Execute()
 			out, err := io.ReadAll(b)
 			if err != nil {
 				t.Fatal(err)
 			}
-			errOut, err := io.ReadAll(e)
-			if err != nil {
-				t.Fatal(err)
+			//
+			if tt.expect == "" && len(out) > 0 {
+				t.Errorf(`%s 
+got: %v
+wanted: ""`, "expected empty buffer", string(out))
 			}
 
-			if len(out) > 0 {
-				if !strings.Contains(string(out), tt.expect) {
-					t.Errorf(`%s 
+			if tt.expect != "" && !strings.Contains(string(out), tt.expect) {
+				t.Errorf(`%s 
 got: %v
 want: %v`, "output comparison failed", string(out), tt.expect)
-				}
-			}
-			if len(errOut) > 0 {
-				if !strings.Contains(string(errOut), tt.expect) {
-					t.Errorf(`%s 
-got: %v
-want: %v`, "error output comparison failed", string(errOut), tt.expect)
-				}
 			}
 
+			cmd = nil
+			path = ""
+			cmKeySeparator = ""
+			cmTokenSeparator = ""
+			verbose = false
 		})
 	}
 }
diff --git a/seeder/cmd/strategyrestseeder/version.go b/seeder/cmd/strategyrestseeder/version.go
@@ -23,5 +23,5 @@ var (
 )
 
 func init() {
-	strategyrestseederCmd.AddCommand(versionCmd)
+	StrategyRestSeederCmd.AddCommand(versionCmd)
 }
diff --git a/seeder/seeder.go b/seeder/seeder.go
@@ -101,39 +101,40 @@ func (s *StrategyRestSeeder) WithConfigManager(configManager CMRetrieve) *Strate
 	return s
 }
 
+func (s *StrategyRestSeeder) replaceAuthTokens() error {
+	if s.configManager != nil && len(s.authInstructions) > 0 {
+		replacedAuthInstructions, err := configmanager.RetrieveMarshalledJson(&s.authInstructions, s.configManager, *s.configManagerOptions)
+		if err != nil {
+			return fmt.Errorf("Error while replacing secrets placeholders in authmap - %v", err)
+		}
+		s.rest.WithAuth(*replacedAuthInstructions)
+	}
+
+	return nil
+}
+
 // Execute the built actions list
 func (s *StrategyRestSeeder) Execute(ctx context.Context) error {
 	var errs []error
 	replacedActions := s.actions
 	// assign each action to method
 	s.log.Debugf("actions: %v", s.actions)
 	// configmanager the auth portion for any tokens
-	if s.configManager != nil && len(s.authInstructions) > 0 {
-		replacedAuthInstructions, err := configmanager.RetrieveMarshalledJson(&s.authInstructions, s.configManager, *s.configManagerOptions)
-		if err != nil {
-			return fmt.Errorf("Error while replacing secrets placeholders in authmap - %v", err)
-		}
-		s.rest.WithAuth(*replacedAuthInstructions)
+	if err := s.replaceAuthTokens(); err != nil {
+		return err
 	}
+
 	// do some ordering if exists
 	// send to fixed size channel goroutine
 	for _, action := range replacedActions {
 		if fn, ok := s.Strategy[StrategyType(action.Strategy)]; ok {
 			a := &action
-			// not the most efficient way of doing it
-			if s.configManager != nil {
-				if err := s.configManagerReplaceAction(a); err != nil {
-					errs = append(errs, err)
-				}
+			if err := s.performAction(ctx, a, fn, errs...); err != nil {
+				errs = append(errs, err...)
 			}
-			e := fn(ctx, a, s.rest)
-			if e != nil {
-				errs = append(errs, e)
-				continue
-			}
-			continue
+		} else {
+			s.log.Infof("unknown strategy")
 		}
-		s.log.Infof("unknown strategy")
 	}
 	if len(errs) > 0 {
 		finalErr := []string{}
@@ -145,6 +146,19 @@ func (s *StrategyRestSeeder) Execute(ctx context.Context) error {
 	return nil
 }
 
+func (s *StrategyRestSeeder) performAction(ctx context.Context, a *Action, fn StrategyFunc, errs ...error) []error {
+	// not the most efficient way of doing it
+	if s.configManager != nil {
+		if err := s.configManagerReplaceAction(a); err != nil {
+			errs = append(errs, err)
+		}
+	}
+	if err := fn(ctx, a, s.rest); err != nil {
+		errs = append(errs, err)
+	}
+	return errs
+}
+
 // configManagerReplaceAction replaces all occurences of ConfigManager Tokens
 // inside PayloadTemplates and Variables
 func (s *StrategyRestSeeder) configManagerReplaceAction(action *Action) error {

diff --git a/seeder/seeder_test.go b/seeder/seeder_test.go
@@ -239,6 +239,7 @@ func TestExecuteGetPutPost(t *testing.T) {
 			},
 		},
 	}
+	t.Parallel()
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			srs := seeder.New(&logger).WithRestClient(&http.Client{})
@@ -483,6 +484,7 @@ func TestExecuteFindPutPost(t *testing.T) {
 			},
 		},
 	}
+	t.Parallel()
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			srs := seeder.New(&logger).WithRestClient(&http.Client{})
@@ -589,6 +591,7 @@ func TestExecuteCustomTokenError(t *testing.T) {
 			},
 		},
 	}
+	t.Parallel()
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			srs := seeder.New(&logger).WithRestClient(&http.Client{})
@@ -910,6 +913,7 @@ func TestExecuteGetPost(t *testing.T) {
 			},
 		},
 	}
+	t.Parallel()
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			srs := seeder.New(&logger).WithRestClient(&http.Client{})
@@ -1130,6 +1134,8 @@ func TestExecutePutPost(t *testing.T) {
 			},
 		},
 	}
+
+	t.Parallel()
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			srs := seeder.New(&logger).WithRestClient(&http.Client{})
@@ -1357,6 +1363,7 @@ func TestExecuteFindPatchPost(t *testing.T) {
 			},
 		},
 	}
+	t.Parallel()
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			srs := seeder.New(&logger).WithRestClient(&http.Client{})
@@ -1698,6 +1705,7 @@ func TestExecuteFindDeletePost(t *testing.T) {
 			},
 		},
 	}
+	t.Parallel()
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			srs := seeder.New(&logger).WithRestClient(&http.Client{})
@@ -1837,6 +1845,7 @@ func TestExecutePut(t *testing.T) {
 			},
 		},
 	}
+	t.Parallel()
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			srs := seeder.New(&logger).WithRestClient(&http.Client{})
@@ -2116,6 +2125,7 @@ func TestExecuteFindPost(t *testing.T) {
 			},
 		},
 	}
+	t.Parallel()
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			srs := seeder.New(&logger).WithRestClient(&http.Client{})
@@ -2290,6 +2300,7 @@ func TestExecuteWithConfigManager(t *testing.T) {
 			},
 		},
 	}
+	t.Parallel()
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			srs := seeder.New(&logger).WithRestClient(&http.Client{})
@@ -2403,6 +2414,7 @@ func TestExecuteWithConfigManagerError(t *testing.T) {
 			},
 		},
 	}
+	t.Parallel()
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			srs := seeder.New(&logger).WithRestClient(&http.Client{})
@@ -2481,6 +2493,7 @@ func TestExecuteUnknownStrategy(t *testing.T) {
 			},
 		},
 	}
+	t.Parallel()
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			srs := seeder.New(&logger).WithRestClient(&http.Client{})

diff --git a/seeder/seederstrategies.go b/seeder/seederstrategies.go
@@ -2,6 +2,7 @@ package seeder
 
 import (
 	"context"
+	"errors"
 )
 
 // GetPost strategy calls a GET endpoint and if item ***FOUND it does NOT do a POST***
@@ -98,8 +99,9 @@ func (r *SeederImpl) FindDeletePost(ctx context.Context, action *Action) error {
 	action.templatedPayload = r.TemplatePayload(action.PayloadTemplate, action.Variables)
 	resp, err := r.get(ctx, action)
 	if err != nil {
-		if d, ok := err.(*Diagnostic); ok {
-			if !d.ProceedFallback {
+		var diag *Diagnostic
+		if errors.As(err, &diag) {
+			if !diag.ProceedFallback {
 				return err
 			}
 			// proceeding with request
@@ -134,8 +136,9 @@ func (r *SeederImpl) GetPutPost(ctx context.Context, action *Action) error {
 	action.templatedPayload = r.TemplatePayload(action.PayloadTemplate, action.Variables)
 	resp, err := r.get(ctx, action)
 	if err != nil {
-		if d, ok := err.(*Diagnostic); ok {
-			if !d.ProceedFallback {
+		var diag *Diagnostic
+		if errors.As(err, &diag) {
+			if !diag.ProceedFallback {
 				return err
 			}
 			// proceeding with request
@@ -176,8 +179,9 @@ func (r *SeederImpl) Put(ctx context.Context, action *Action) error {
 func (r *SeederImpl) PutPost(ctx context.Context, action *Action) error {
 	action.templatedPayload = r.TemplatePayload(action.PayloadTemplate, action.Variables)
 	if err := r.put(ctx, action); err != nil {
-		if d, ok := err.(*Diagnostic); ok {
-			if d.IsFatal || !d.ProceedFallback {
+		var diag *Diagnostic
+		if errors.As(err, &diag) {
+			if diag.IsFatal || !diag.ProceedFallback {
 				return err
 			}
 			r.log.Debug("falling back on POST")