refactor

Author: EyalDelarea

apptrust/commands/flags.go

@@ -90,13 +90,13 @@ var flagsMap = map[string]components.Flag{
 	ExcludeReposFlag:        components.NewStringFlag(ExcludeReposFlag, "Semicolon-separated list of repositories to exclude.", func(f *components.StringFlag) { f.Mandatory = false }),
 	IncludeReposFlag:        components.NewStringFlag(IncludeReposFlag, "Semicolon-separated list of repositories to include.", func(f *components.StringFlag) { f.Mandatory = false }),
 	PropsFlag:               components.NewStringFlag(PropsFlag, "Semicolon-separated list of properties in the form of 'key1=value1;key2=value2;...' to be added to each artifact.", func(f *components.StringFlag) { f.Mandatory = false }),
-	TagFlag:                 components.NewStringFlag(TagFlag, "A tag to associate with the version.", func(f *components.StringFlag) { f.Mandatory = false }),
+	TagFlag:                 components.NewStringFlag(TagFlag, "A tag to associate with the version. Must contain only alphanumeric characters, hyphens (-), underscores (_), and dots (.). Examples: 'release-1.2.3', 'v1.0.0', 'production_build'.", func(f *components.StringFlag) { f.Mandatory = false }),
 	BuildsFlag:              components.NewStringFlag(BuildsFlag, "List of builds in format 'name1:number1[:timestamp1];name2:number2[:timestamp2]'", func(f *components.StringFlag) { f.Mandatory = false }),
 	ReleaseBundlesFlag:      components.NewStringFlag(ReleaseBundlesFlag, "List of release bundles in format 'name1:version1;name2:version2'", func(f *components.StringFlag) { f.Mandatory = false }),
 	SourceVersionFlag:       components.NewStringFlag(SourceVersionFlag, "Source versions in format 'app1:version1;app2:version2'", func(f *components.StringFlag) { f.Mandatory = false }),
 	PackagesFlag:            components.NewStringFlag(PackagesFlag, "List of packages in format 'name1;name2'", func(f *components.StringFlag) { f.Mandatory = false }),
-	PropertiesFlag:          components.NewStringFlag(PropertiesFlag, "Sets or updates a custom property for the application version in format 'key=value1[,value2,...]'", func(f *components.StringFlag) { f.Mandatory = false }),
-	DeletePropertyFlag:      components.NewStringFlag(DeletePropertyFlag, "Completely removes the specified property key and all its associated values from the application version", func(f *components.StringFlag) { f.Mandatory = false }),
+	PropertiesFlag:          components.NewStringFlag(PropertiesFlag, "Set or update a property: 'key=val1[,val2,...]'", func(f *components.StringFlag) { f.Mandatory = false }),
+	DeletePropertyFlag:      components.NewStringFlag(DeletePropertyFlag, "Remove a property key and all its values", func(f *components.StringFlag) { f.Mandatory = false }),
 }
 
 var commandFlags = map[string][]string{

apptrust/commands/utils/utils.go

@@ -136,3 +136,45 @@ func ParseNameVersionPairs(input string) ([][2]string, error) {
 	}
 	return result, nil
 }
+
+// ParsePropertiesFlag parses a properties string into a map of keys to value slices.
+// Format: "key1=value1[,value2,...];key2=value3[,value4,...]"
+// Examples:
+//   - "status=rc" -> {"status": ["rc"]}
+//   - "status=rc,validated" -> {"status": ["rc", "validated"]}
+//   - "status=rc;deployed_to=staging" -> {"status": ["rc"], "deployed_to": ["staging"]}
+//   - "old_flag=" -> {"old_flag": []} (clears values)
+func ParsePropertiesFlag(propertiesStr string) (map[string][]string, error) {
+	if propertiesStr == "" {
+		return nil, nil
+	}
+
+	result := make(map[string][]string)
+	pairs := strings.Split(propertiesStr, ";")
+
+	for _, pair := range pairs {
+		keyValue := strings.SplitN(strings.TrimSpace(pair), "=", 2)
+		if len(keyValue) != 2 {
+			return nil, errorutils.CheckErrorf("invalid property format: \"%s\" (expected key=value1[,value2,...])", pair)
+		}
+
+		key := strings.TrimSpace(keyValue[0])
+		valuesStr := strings.TrimSpace(keyValue[1])
+
+		if key == "" {
+			return nil, errorutils.CheckErrorf("property key cannot be empty")
+		}
+
+		var values []string
+		if valuesStr != "" {
+			values = strings.Split(valuesStr, ",")
+			for i, v := range values {
+				values[i] = strings.TrimSpace(v)
+			}
+		}
+		// Always set the key, even with empty values (to clear values)
+		result[key] = values
+	}
+
+	return result, nil
+}

apptrust/commands/utils/utils_test.go

@@ -197,3 +197,82 @@ func TestParseNameVersionPairs(t *testing.T) {
 		})
 	}
 }
+
+func TestParsePropertiesFlag(t *testing.T) {
+	tests := []struct {
+		name      string
+		input     string
+		expected  map[string][]string
+		expectErr bool
+	}{
+		{
+			name:     "empty string",
+			input:    "",
+			expected: nil,
+		},
+		{
+			name:  "single property with single value",
+			input: "status=rc",
+			expected: map[string][]string{
+				"status": {"rc"},
+			},
+		},
+		{
+			name:  "single property with multiple values",
+			input: "status=rc,validated",
+			expected: map[string][]string{
+				"status": {"rc", "validated"},
+			},
+		},
+		{
+			name:  "multiple properties",
+			input: "status=rc,validated;deployed_to=staging-A,staging-B",
+			expected: map[string][]string{
+				"status":      {"rc", "validated"},
+				"deployed_to": {"staging-A", "staging-B"},
+			},
+		},
+		{
+			name:  "empty values (clears values)",
+			input: "old_feature_flag=",
+			expected: map[string][]string{
+				"old_feature_flag": nil,
+			},
+		},
+		{
+			name:  "with spaces",
+			input: " status = rc , validated ; deployed_to = staging-A , staging-B ",
+			expected: map[string][]string{
+				"status":      {"rc", "validated"},
+				"deployed_to": {"staging-A", "staging-B"},
+			},
+		},
+		{
+			name:      "invalid format - missing =",
+			input:     "invalid-format",
+			expectErr: true,
+		},
+		{
+			name:      "empty key",
+			input:     "=value",
+			expectErr: true,
+		},
+		{
+			name:      "empty key with spaces",
+			input:     " =value",
+			expectErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := ParsePropertiesFlag(tt.input)
+			if tt.expectErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}

apptrust/commands/version/update_app_version_cmd.go

@@ -3,8 +3,6 @@ package version
 //go:generate ${PROJECT_DIR}/scripts/mockgen.sh ${GOFILE}
 
 import (
-	"strings"
-
 	"github.com/jfrog/jfrog-cli-application/apptrust/app"
 	"github.com/jfrog/jfrog-cli-application/apptrust/commands"
 	"github.com/jfrog/jfrog-cli-application/apptrust/commands/utils"
@@ -17,6 +15,7 @@ import (
 	"github.com/jfrog/jfrog-cli-core/v2/plugins/components"
 	coreConfig "github.com/jfrog/jfrog-cli-core/v2/utils/config"
 	"github.com/jfrog/jfrog-client-go/utils/errorutils"
+	"github.com/jfrog/jfrog-client-go/utils/log"
 )
 
 type updateAppVersionCommand struct {
@@ -28,11 +27,22 @@ type updateAppVersionCommand struct {
 }
 
 func (uv *updateAppVersionCommand) Run() error {
+	log.Info("Updating application version:", uv.applicationKey, "version:", uv.version)
+
 	ctx, err := service.NewContext(*uv.serverDetails)
 	if err != nil {
+		log.Error("Failed to create service context:", err)
+		return err
+	}
+
+	err = uv.versionService.UpdateAppVersion(ctx, uv.requestPayload)
+	if err != nil {
+		log.Error("Failed to update application version:", err)
 		return err
 	}
-	return uv.versionService.UpdateAppVersion(ctx, uv.applicationKey, uv.version, uv.requestPayload)
+
+	log.Info("Successfully updated application version:", uv.applicationKey, "version:", uv.version)
+	return nil
 }
 
 func (uv *updateAppVersionCommand) ServerDetails() (*coreConfig.ServerDetails, error) {
@@ -48,15 +58,11 @@ func (uv *updateAppVersionCommand) prepareAndRunCommand(ctx *components.Context)
 		return pluginsCommon.WrongNumberOfArgumentsHandler(ctx)
 	}
 
-	uv.applicationKey = ctx.Arguments[0]
-	uv.version = ctx.Arguments[1]
-
-	serverDetails, err := utils.ServerDetailsByFlags(ctx)
-	if err != nil {
+	if err := uv.parseFlagsAndSetFields(ctx); err != nil {
 		return err
 	}
-	uv.serverDetails = serverDetails
 
+	var err error
 	uv.requestPayload, err = uv.buildRequestPayload(ctx)
 	if errorutils.CheckError(err) != nil {
 		return err
@@ -65,17 +71,32 @@ func (uv *updateAppVersionCommand) prepareAndRunCommand(ctx *components.Context)
 	return commonCLiCommands.Exec(uv)
 }
 
+// parseFlagsAndSetFields parses CLI flags and sets struct fields accordingly.
+func (uv *updateAppVersionCommand) parseFlagsAndSetFields(ctx *components.Context) error {
+	uv.applicationKey = ctx.Arguments[0]
+	uv.version = ctx.Arguments[1]
+
+	serverDetails, err := utils.ServerDetailsByFlags(ctx)
+	if err != nil {
+		return err
+	}
+	uv.serverDetails = serverDetails
+	return nil
+}
+
 func (uv *updateAppVersionCommand) buildRequestPayload(ctx *components.Context) (*model.UpdateAppVersionRequest, error) {
-	request := &model.UpdateAppVersionRequest{}
+	request := &model.UpdateAppVersionRequest{
+		ApplicationKey: uv.applicationKey,
+		Version:        uv.version,
+	}
 
-	// Handle tag - no validation, just pass through
 	if ctx.IsFlagSet(commands.TagFlag) {
 		request.Tag = ctx.GetStringFlagValue(commands.TagFlag)
 	}
 
-	// Handle properties - support multiple values per key
+	// Handle properties - use spec format: key=value1[,value2,...]
 	if ctx.IsFlagSet(commands.PropertiesFlag) {
-		properties, err := uv.parseProperties(ctx.GetStringFlagValue(commands.PropertiesFlag))
+		properties, err := utils.ParsePropertiesFlag(ctx.GetStringFlagValue(commands.PropertiesFlag))
 		if err != nil {
 			return nil, err
 		}
@@ -91,43 +112,6 @@ func (uv *updateAppVersionCommand) buildRequestPayload(ctx *components.Context)
 	return request, nil
 }
 
-func (uv *updateAppVersionCommand) parseProperties(propertiesStr string) (map[string][]string, error) {
-	// Format: "key1=value1[,value2,...];key2=value3[,value4,...]"
-	if propertiesStr == "" {
-		return nil, nil
-	}
-
-	result := make(map[string][]string)
-	pairs := strings.Split(propertiesStr, ";")
-
-	for _, pair := range pairs {
-		keyValue := strings.SplitN(strings.TrimSpace(pair), "=", 2)
-		if len(keyValue) != 2 {
-			return nil, errorutils.CheckErrorf("invalid property format: '%s' (expected key=value1[,value2,...])", pair)
-		}
-
-		key := strings.TrimSpace(keyValue[0])
-		valuesStr := strings.TrimSpace(keyValue[1])
-
-		if key == "" {
-			return nil, errorutils.CheckErrorf("property key cannot be empty")
-		}
-
-		var values []string
-		if valuesStr != "" {
-			values = strings.Split(valuesStr, ",")
-			for i, v := range values {
-				values[i] = strings.TrimSpace(v)
-			}
-		}
-		// Always set the key, even with empty values (to clear values)
-
-		result[key] = values
-	}
-
-	return result, nil
-}
-
 func GetUpdateAppVersionCommand(appContext app.Context) components.Command {
 	cmd := &updateAppVersionCommand{versionService: appContext.GetVersionService()}
 	return components.Command{