refactor: adopt cobra's error handling and properly validate inputs

cmd/cmd.go

@@ -27,9 +27,9 @@ func NewCommad() *cobra.Command {
 	var clientOpts connectors.ClientOptions
 
 	command := &cobra.Command{
-		Use:          "microcks",
-		Short:        "A CLI tool for Microcks",
-		SilenceUsage: true,
+		Use:           "microcks",
+		Short:         "A CLI tool for Microcks",
+		SilenceErrors: true,
 		Run: func(cmd *cobra.Command, args []string) {
 			cmd.HelpFunc()(cmd, args)
 		},

cmd/cmd_test.go

@@ -0,0 +1,88 @@
+package cmd
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/require"
+)
+
+func TestStartCommandPortValidation(t *testing.T) {
+	tests := []struct {
+		name    string
+		port    string
+		wantErr bool
+	}{
+		{name: "lowest valid port", port: "1", wantErr: false},
+		{name: "default port", port: "8585", wantErr: false},
+		{name: "highest valid port", port: "65535", wantErr: false},
+		{name: "non numeric port", port: "abc", wantErr: true},
+		{name: "zero port", port: "0", wantErr: true},
+		{name: "negative port", port: "-1", wantErr: true},
+		{name: "port above range", port: "65536", wantErr: true},
+		{name: "empty port", port: "", wantErr: true},
+		{name: "port with spaces", port: " 8080", wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd := NewStartCommand(nil)
+			cmd.SetArgs([]string{"--port", tt.port})
+
+			executed := false
+			cmd.RunE = func(cmd *cobra.Command, args []string) error {
+				executed = true
+				return nil
+			}
+
+			err := cmd.Execute()
+			if tt.wantErr {
+				require.Error(t, err)
+				require.False(t, executed, "RunE should not execute when validation fails")
+			} else {
+				require.NoError(t, err)
+				require.True(t, executed, "RunE should execute when validation passes")
+			}
+		})
+	}
+}
+
+func TestLoginCommandSsoPortValidation(t *testing.T) {
+	tests := []struct {
+		name    string
+		ssoPort int
+		wantErr bool
+	}{
+		{name: "lowest valid port", ssoPort: 1, wantErr: false},
+		{name: "default port", ssoPort: 58085, wantErr: false},
+		{name: "highest valid port", ssoPort: 65535, wantErr: false},
+		{name: "zero port", ssoPort: 0, wantErr: true},
+		{name: "negative port", ssoPort: -1, wantErr: true},
+		{name: "port above range", ssoPort: 65536, wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd := NewLoginCommand(nil)
+			cmd.SetArgs([]string{"--sso-port", fmt.Sprintf("%d", tt.ssoPort), "http://localhost:8080"})
+
+			executed := false
+			originalRun := cmd.Run
+			cmd.RunE = func(cmd *cobra.Command, args []string) error {
+				executed = true
+				return nil
+			}
+
+			err := cmd.Execute()
+			if tt.wantErr {
+				require.Error(t, err)
+				require.False(t, executed, "RunE should not execute when validation fails")
+			} else {
+				require.NoError(t, err)
+				require.True(t, executed, "RunE should execute when validation passes")
+			}
+			_ = originalRun
+		})
+	}
+}

cmd/context.go

@@ -27,29 +27,28 @@ microcks context http://localhost:8080
 
 # Delete Microcks context
 microcks context http://localhost:8080 --delete`,
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			configPath := globalClientOpts.ConfigPath
 			localCfg, err := config.ReadLocalConfig(configPath)
 			errors.CheckError(err)
 			if delete {
 				if len(args) == 0 {
-					cmd.HelpFunc()(cmd, args)
-					os.Exit(1)
+					return fmt.Errorf("context name is required when using --delete")
 				}
 				err := deleteContext(args[0], configPath)
 				errors.CheckError(err)
-				return
+				return nil
 			}
 
 			if len(args) == 0 {
 				printMicrocksContexts(configPath)
-				return
+				return nil
 			}
 
 			ctxName := args[0]
 			if localCfg.CurrentContext == ctxName {
 				fmt.Printf("Already at context '%s'\n", localCfg.CurrentContext)
-				return
+				return nil
 			}
 			if _, err = localCfg.ResolveContext(ctxName); err != nil {
 				log.Fatal(err)
@@ -58,6 +57,7 @@ microcks context http://localhost:8080 --delete`,
 			err = config.WriteLocalConfig(*localCfg, configPath)
 			errors.CheckError(err)
 			fmt.Printf("Switched to context '%s'\n", localCfg.CurrentContext)
+			return nil
 		},
 	}
 

cmd/import.go

@@ -17,7 +17,8 @@ package cmd
 
 import (
 	"fmt"
-	"os"
+	"path/filepath"
+	"runtime"
 	"strconv"
 	"strings"
 
@@ -28,37 +29,45 @@ import (
 	"github.com/spf13/cobra"
 )
 
+func parsePathSuffix(arg string) (string, bool) {
+	colonIdx := strings.LastIndex(arg, ":")
+	if colonIdx < 0 {
+		return arg, true
+	}
+	if runtime.GOOS == "windows" && colonIdx == 1 && len(arg) > 2 {
+		if c := arg[0]; (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') {
+			return arg, true
+		}
+	}
+	pathPart := arg[:colonIdx]
+	suffixPart := arg[colonIdx+1:]
+	val, err := strconv.ParseBool(suffixPart)
+	if err != nil {
+		return arg, true
+	}
+	return pathPart, val
+}
+
 func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command {
 	var watch bool
 
 	var importCmd = &cobra.Command{
-		Use:   "import",
+		Use:   "import <specificationFile1[:primary],specificationFile2[:primary]>",
 		Short: "import API artifacts on Microcks server",
 		Long:  `import API artifacts on Microcks server`,
-		Args:  cobra.MaximumNArgs(1),
-		Run: func(cmd *cobra.Command, args []string) {
-			// Parse subcommand args first.
-			if len(args) == 0 {
-				fmt.Println("import command require <specificationFile1[:primary],specificationFile2[:primary]> args")
-				cmd.HelpFunc()(cmd, args)
-				os.Exit(1)
-			}
-
+		Args:  cobra.ExactArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
 			specificationFiles := args[0]
 
-			// Initialize config from command options.
 			config.InsecureTLS = globalClientOpts.InsecureTLS
 			config.CaCertPaths = globalClientOpts.CaCertPaths
 			config.Verbose = globalClientOpts.Verbose
 
-			// Read local config file in case we need some context info.
 			localConfig, err := config.ReadLocalConfig(globalClientOpts.ConfigPath)
 			if err != nil {
-				fmt.Println(err)
-				return
+				return err
 			}
 
-			// Prepare Microcks client.
 			var mc connectors.MicrocksClient
 
 			if globalClientOpts.ServerAddr != "" && globalClientOpts.ClientId != "" && globalClientOpts.ClientSecret != "" {
@@ -67,8 +76,7 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command
 
 				keycloakURL, err := mc.GetKeycloakURL()
 				if err != nil {
-					fmt.Printf("Got error when invoking Microcks client retrieving config: %s", err)
-					os.Exit(1)
+					return fmt.Errorf("got error when invoking Microcks client retrieving config: %s", err)
 				}
 
 				var oauthToken string = "unauthenticated-token"
@@ -78,10 +86,8 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command
 
 					oauthToken, err = kc.ConnectAndGetToken()
 					if err != nil {
-						fmt.Printf("Got error when invoking Keycloak client: %s", err)
-						os.Exit(1)
+						return fmt.Errorf("got error when invoking Keycloak client: %s", err)
 					}
-					//fmt.Printf("Retrieve OAuthToken: %s", oauthToken)
 				}
 
 				// Set Auth token.
@@ -100,8 +106,7 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command
 			} else {
 				// Create client from config file and using the current or provided context.
 				if localConfig == nil {
-					fmt.Println("Please login to perform operation...")
-					return
+					return fmt.Errorf("please login to perform operation")
 				}
 
 				if globalClientOpts.Context == "" {
@@ -110,8 +115,7 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command
 
 				mc, err = connectors.NewClient(*globalClientOpts)
 				if err != nil {
-					fmt.Printf("error %v", err)
-					return
+					return err
 				}
 			}
 
@@ -121,21 +125,12 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command
 				mainArtifact := true
 				var err error
 
-				// Check if mainArtifact flag is provided.
-				if strings.Contains(f, ":") {
-					pathAndMainArtifact := strings.Split(f, ":")
-					f = pathAndMainArtifact[0]
-					mainArtifact, err = strconv.ParseBool(pathAndMainArtifact[1])
-					if err != nil {
-						fmt.Printf("Cannot parse '%s' as Bool, default to true\n", pathAndMainArtifact[1])
-					}
-				}
+				f, mainArtifact = parsePathSuffix(f)
 
 				// Try uploading this artifact.
 				msg, err := mc.UploadArtifact(f, mainArtifact)
 				if err != nil {
-					fmt.Printf("Got error when invoking Microcks client importing Artifact: %s", err)
-					os.Exit(1)
+					return fmt.Errorf("got error when invoking Microcks client importing Artifact: %s", err)
 				}
 				action := "discovered"
 				if !mainArtifact {
@@ -155,9 +150,9 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command
 					}
 
 					// Normalize file path to match the watcher fsnotify events format.
-					if strings.HasPrefix(f, "./") {
-						f = strings.TrimPrefix(f, "./")
-					}
+					f = strings.TrimPrefix(f, "./")
+					f = strings.TrimPrefix(f, ".\\")
+					f = filepath.Clean(f)
 
 					// Upsert entry.
 					watchCfg.UpsertEntry(config.WatchEntry{
@@ -183,6 +178,7 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command
 				fmt.Println("Watch mode enabled - microcks-watcher started...")
 				wm.Run()
 			}
+			return nil
 		},
 	}
 

cmd/importDir.go

@@ -115,13 +115,8 @@ func NewImportDirCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm
 			microcks import-dir ./api-specs --recursive
 			microcks import-dir ./api-specs --pattern "*.yaml"
 			microcks import-dir ./api-specs --recursive --pattern "openapi.*"`,
-		Run: func(cmd *cobra.Command, args []string) {
-			if len(args) == 0 {
-				fmt.Println("import-dir command requires a directory path")
-				cmd.HelpFunc()(cmd, args)
-				os.Exit(1)
-			}
-
+		Args: cobra.ExactArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
 			dirPath := args[0]
 
 			config.InsecureTLS = globalClientOpts.InsecureTLS
@@ -130,13 +125,11 @@ func NewImportDirCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm
 
 			localConfig, err := config.ReadLocalConfig(globalClientOpts.ConfigPath)
 			if err != nil {
-				fmt.Println(err)
-				return
+				return err
 			}
 
 			if localConfig == nil {
-				fmt.Println("Please login to perform operation...")
-				return
+				return fmt.Errorf("please login to perform operation")
 			}
 
 			if globalClientOpts.Context == "" {
@@ -146,8 +139,7 @@ func NewImportDirCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm
 			// Create client
 			mc, err := connectors.NewClient(*globalClientOpts)
 			if err != nil {
-				fmt.Printf("error %v", err)
-				return
+				return err
 			}
 
 			// Set up business logic dependencies
@@ -161,12 +153,7 @@ func NewImportDirCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm
 			// Execute business logic
 			result, err := ImportDirectory(mc, fs, dirPath, importConfig)
 			if err != nil {
-				if validationErr, ok := err.(*ValidationError); ok {
-					fmt.Println(validationErr.Message)
-					return
-				}
-				fmt.Printf("Error: %v\n", err)
-				os.Exit(1)
+				return err
 			}
 
 			// Display results
@@ -197,6 +184,7 @@ func NewImportDirCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm
 			}
 
 			fmt.Printf("\nImport completed: %d/%d files imported successfully\n", result.SuccessCount, result.TotalFiles)
+			return nil
 		},
 	}