Skip to content

Commit 0d69c47

Browse files
EyalDelareaEyalDelarea
committed
refactor
1 parent a68e6aa commit 0d69c47

File tree

8 files changed

+258
-132
lines changed

8 files changed

+258
-132
lines changed

‎apptrust/commands/utils/utils.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,3 +136,45 @@ func ParseNameVersionPairs(input string) ([][2]string, error) {
136136
}
137137
return result, nil
138138
}
139+
140+
// ParsePropertiesFlag parses a properties string into a map of keys to value slices.
141+
// Format: "key1=value1[,value2,...];key2=value3[,value4,...]"
142+
// Examples:
143+
// - "status=rc" -> {"status": ["rc"]}
144+
// - "status=rc,validated" -> {"status": ["rc", "validated"]}
145+
// - "status=rc;deployed_to=staging" -> {"status": ["rc"], "deployed_to": ["staging"]}
146+
// - "old_flag=" -> {"old_flag": []} (clears values)
147+
func ParsePropertiesFlag(propertiesStr string) (map[string][]string, error) {
148+
if propertiesStr == "" {
149+
return nil, nil
150+
}
151+
152+
result := make(map[string][]string)
153+
pairs := strings.Split(propertiesStr, ";")
154+
155+
for _, pair := range pairs {
156+
keyValue := strings.SplitN(strings.TrimSpace(pair), "=", 2)
157+
if len(keyValue) != 2 {
158+
return nil, errorutils.CheckErrorf("invalid property format: \"%s\" (expected key=value1[,value2,...])", pair)
159+
}
160+
161+
key := strings.TrimSpace(keyValue[0])
162+
valuesStr := strings.TrimSpace(keyValue[1])
163+
164+
if key == "" {
165+
return nil, errorutils.CheckErrorf("property key cannot be empty")
166+
}
167+
168+
var values []string
169+
if valuesStr != "" {
170+
values = strings.Split(valuesStr, ",")
171+
for i, v := range values {
172+
values[i] = strings.TrimSpace(v)
173+
}
174+
}
175+
// Always set the key, even with empty values (to clear values)
176+
result[key] = values
177+
}
178+
179+
return result, nil
180+
}

‎apptrust/commands/utils/utils_test.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,3 +197,82 @@ func TestParseNameVersionPairs(t *testing.T) {
197197
})
198198
}
199199
}
200+
201+
func TestParsePropertiesFlag(t *testing.T) {
202+
tests := []struct {
203+
name string
204+
input string
205+
expected map[string][]string
206+
expectErr bool
207+
}{
208+
{
209+
name: "empty string",
210+
input: "",
211+
expected: nil,
212+
},
213+
{
214+
name: "single property with single value",
215+
input: "status=rc",
216+
expected: map[string][]string{
217+
"status": {"rc"},
218+
},
219+
},
220+
{
221+
name: "single property with multiple values",
222+
input: "status=rc,validated",
223+
expected: map[string][]string{
224+
"status": {"rc", "validated"},
225+
},
226+
},
227+
{
228+
name: "multiple properties",
229+
input: "status=rc,validated;deployed_to=staging-A,staging-B",
230+
expected: map[string][]string{
231+
"status": {"rc", "validated"},
232+
"deployed_to": {"staging-A", "staging-B"},
233+
},
234+
},
235+
{
236+
name: "empty values (clears values)",
237+
input: "old_feature_flag=",
238+
expected: map[string][]string{
239+
"old_feature_flag": nil,
240+
},
241+
},
242+
{
243+
name: "with spaces",
244+
input: " status = rc , validated ; deployed_to = staging-A , staging-B ",
245+
expected: map[string][]string{
246+
"status": {"rc", "validated"},
247+
"deployed_to": {"staging-A", "staging-B"},
248+
},
249+
},
250+
{
251+
name: "invalid format - missing =",
252+
input: "invalid-format",
253+
expectErr: true,
254+
},
255+
{
256+
name: "empty key",
257+
input: "=value",
258+
expectErr: true,
259+
},
260+
{
261+
name: "empty key with spaces",
262+
input: " =value",
263+
expectErr: true,
264+
},
265+
}
266+
267+
for _, tt := range tests {
268+
t.Run(tt.name, func(t *testing.T) {
269+
result, err := ParsePropertiesFlag(tt.input)
270+
if tt.expectErr {
271+
assert.Error(t, err)
272+
} else {
273+
assert.NoError(t, err)
274+
assert.Equal(t, tt.expected, result)
275+
}
276+
})
277+
}
278+
}

‎apptrust/commands/version/update_app_version_cmd.go‎

Lines changed: 19 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package version
33
//go:generate ${PROJECT_DIR}/scripts/mockgen.sh ${GOFILE}
44

55
import (
6-
"strings"
7-
86
"github.com/jfrog/jfrog-cli-application/apptrust/app"
97
"github.com/jfrog/jfrog-cli-application/apptrust/commands"
108
"github.com/jfrog/jfrog-cli-application/apptrust/commands/utils"
@@ -17,6 +15,7 @@ import (
1715
"github.com/jfrog/jfrog-cli-core/v2/plugins/components"
1816
coreConfig "github.com/jfrog/jfrog-cli-core/v2/utils/config"
1917
"github.com/jfrog/jfrog-client-go/utils/errorutils"
18+
"github.com/jfrog/jfrog-client-go/utils/log"
2019
)
2120

2221
type updateAppVersionCommand struct {
@@ -28,11 +27,22 @@ type updateAppVersionCommand struct {
2827
}
2928

3029
func (uv *updateAppVersionCommand) Run() error {
30+
log.Info("Updating application version:", uv.applicationKey, "version:", uv.version)
31+
3132
ctx, err := service.NewContext(*uv.serverDetails)
3233
if err != nil {
34+
log.Error("Failed to create service context:", err)
35+
return err
36+
}
37+
38+
err = uv.versionService.UpdateAppVersion(ctx, uv.requestPayload)
39+
if err != nil {
40+
log.Error("Failed to update application version:", err)
3341
return err
3442
}
35-
return uv.versionService.UpdateAppVersion(ctx, uv.applicationKey, uv.version, uv.requestPayload)
43+
44+
log.Info("Successfully updated application version:", uv.applicationKey, "version:", uv.version)
45+
return nil
3646
}
3747

3848
func (uv *updateAppVersionCommand) ServerDetails() (*coreConfig.ServerDetails, error) {
@@ -66,16 +76,18 @@ func (uv *updateAppVersionCommand) prepareAndRunCommand(ctx *components.Context)
6676
}
6777

6878
func (uv *updateAppVersionCommand) buildRequestPayload(ctx *components.Context) (*model.UpdateAppVersionRequest, error) {
69-
request := &model.UpdateAppVersionRequest{}
79+
request := &model.UpdateAppVersionRequest{
80+
ApplicationKey: uv.applicationKey,
81+
Version: uv.version,
82+
}
7083

71-
// Handle tag - no validation, just pass through
7284
if ctx.IsFlagSet(commands.TagFlag) {
7385
request.Tag = ctx.GetStringFlagValue(commands.TagFlag)
7486
}
7587

76-
// Handle properties - support multiple values per key
88+
// Handle properties - use spec format: key=value1[,value2,...]
7789
if ctx.IsFlagSet(commands.PropertiesFlag) {
78-
properties, err := uv.parseProperties(ctx.GetStringFlagValue(commands.PropertiesFlag))
90+
properties, err := utils.ParsePropertiesFlag(ctx.GetStringFlagValue(commands.PropertiesFlag))
7991
if err != nil {
8092
return nil, err
8193
}
@@ -91,43 +103,6 @@ func (uv *updateAppVersionCommand) buildRequestPayload(ctx *components.Context)
91103
return request, nil
92104
}
93105

94-
func (uv *updateAppVersionCommand) parseProperties(propertiesStr string) (map[string][]string, error) {
95-
// Format: "key1=value1[,value2,...];key2=value3[,value4,...]"
96-
if propertiesStr == "" {
97-
return nil, nil
98-
}
99-
100-
result := make(map[string][]string)
101-
pairs := strings.Split(propertiesStr, ";")
102-
103-
for _, pair := range pairs {
104-
keyValue := strings.SplitN(strings.TrimSpace(pair), "=", 2)
105-
if len(keyValue) != 2 {
106-
return nil, errorutils.CheckErrorf("invalid property format: '%s' (expected key=value1[,value2,...])", pair)
107-
}
108-
109-
key := strings.TrimSpace(keyValue[0])
110-
valuesStr := strings.TrimSpace(keyValue[1])
111-
112-
if key == "" {
113-
return nil, errorutils.CheckErrorf("property key cannot be empty")
114-
}
115-
116-
var values []string
117-
if valuesStr != "" {
118-
values = strings.Split(valuesStr, ",")
119-
for i, v := range values {
120-
values[i] = strings.TrimSpace(v)
121-
}
122-
}
123-
// Always set the key, even with empty values (to clear values)
124-
125-
result[key] = values
126-
}
127-
128-
return result, nil
129-
}
130-
131106
func GetUpdateAppVersionCommand(appContext app.Context) components.Command {
132107
cmd := &updateAppVersionCommand{versionService: appContext.GetVersionService()}
133108
return components.Command{

‎apptrust/commands/version/update_app_version_cmd_test.go‎

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55
"testing"
66

7+
"github.com/jfrog/jfrog-cli-application/apptrust/commands/utils"
78
mockversions "github.com/jfrog/jfrog-cli-application/apptrust/service/versions/mocks"
89
"go.uber.org/mock/gomock"
910

@@ -24,15 +25,21 @@ func TestUpdateAppVersionCommand_Run(t *testing.T) {
2425
{
2526
name: "success",
2627
request: &model.UpdateAppVersionRequest{
27-
Tag: "release/1.2.3",
28+
ApplicationKey: "app-key",
29+
Version: "1.0.0",
30+
Tag: "release/1.2.3",
2831
Properties: map[string][]string{
2932
"status": {"rc", "validated"},
3033
},
3134
},
3235
},
3336
{
34-
name: "context error",
35-
request: &model.UpdateAppVersionRequest{Tag: "test-tag"},
37+
name: "context error",
38+
request: &model.UpdateAppVersionRequest{
39+
ApplicationKey: "app-key",
40+
Version: "1.0.0",
41+
Tag: "test-tag",
42+
},
3643
shouldError: true,
3744
errorMessage: "context error",
3845
},
@@ -45,10 +52,10 @@ func TestUpdateAppVersionCommand_Run(t *testing.T) {
4552

4653
mockVersionService := mockversions.NewMockVersionService(ctrl)
4754
if tt.shouldError {
48-
mockVersionService.EXPECT().UpdateAppVersion(gomock.Any(), "app-key", "1.0.0", tt.request).
55+
mockVersionService.EXPECT().UpdateAppVersion(gomock.Any(), gomock.Any()).
4956
Return(errors.New(tt.errorMessage)).Times(1)
5057
} else {
51-
mockVersionService.EXPECT().UpdateAppVersion(gomock.Any(), "app-key", "1.0.0", tt.request).
58+
mockVersionService.EXPECT().UpdateAppVersion(gomock.Any(), gomock.Any()).
5259
Return(nil).Times(1)
5360
}
5461

@@ -86,7 +93,9 @@ func TestUpdateAppVersionCommand_FlagsSuite(t *testing.T) {
8693
ctx.AddStringFlag(commands.TagFlag, "release/1.2.3")
8794
},
8895
expectsPayload: &model.UpdateAppVersionRequest{
89-
Tag: "release/1.2.3",
96+
ApplicationKey: "app-key",
97+
Version: "1.0.0",
98+
Tag: "release/1.2.3",
9099
},
91100
},
92101
{
@@ -96,6 +105,8 @@ func TestUpdateAppVersionCommand_FlagsSuite(t *testing.T) {
96105
ctx.AddStringFlag(commands.PropertiesFlag, "status=rc")
97106
},
98107
expectsPayload: &model.UpdateAppVersionRequest{
108+
ApplicationKey: "app-key",
109+
Version: "1.0.0",
99110
Properties: map[string][]string{
100111
"status": {"rc"},
101112
},
@@ -108,6 +119,8 @@ func TestUpdateAppVersionCommand_FlagsSuite(t *testing.T) {
108119
ctx.AddStringFlag(commands.PropertiesFlag, "status=rc,validated")
109120
},
110121
expectsPayload: &model.UpdateAppVersionRequest{
122+
ApplicationKey: "app-key",
123+
Version: "1.0.0",
111124
Properties: map[string][]string{
112125
"status": {"rc", "validated"},
113126
},
@@ -120,6 +133,8 @@ func TestUpdateAppVersionCommand_FlagsSuite(t *testing.T) {
120133
ctx.AddStringFlag(commands.PropertiesFlag, "status=rc,validated;deployed_to=staging-A,staging-B")
121134
},
122135
expectsPayload: &model.UpdateAppVersionRequest{
136+
ApplicationKey: "app-key",
137+
Version: "1.0.0",
123138
Properties: map[string][]string{
124139
"status": {"rc", "validated"},
125140
"deployed_to": {"staging-A", "staging-B"},
@@ -133,6 +148,8 @@ func TestUpdateAppVersionCommand_FlagsSuite(t *testing.T) {
133148
ctx.AddStringFlag(commands.DeletePropertyFlag, "legacy_param;toBeDeleted")
134149
},
135150
expectsPayload: &model.UpdateAppVersionRequest{
151+
ApplicationKey: "app-key",
152+
Version: "1.0.0",
136153
DeleteProperties: []string{"legacy_param", "toBeDeleted"},
137154
},
138155
},
@@ -143,6 +160,8 @@ func TestUpdateAppVersionCommand_FlagsSuite(t *testing.T) {
143160
ctx.AddStringFlag(commands.PropertiesFlag, "old_feature_flag=")
144161
},
145162
expectsPayload: &model.UpdateAppVersionRequest{
163+
ApplicationKey: "app-key",
164+
Version: "1.0.0",
146165
Properties: map[string][]string{
147166
"old_feature_flag": nil,
148167
},
@@ -157,7 +176,9 @@ func TestUpdateAppVersionCommand_FlagsSuite(t *testing.T) {
157176
ctx.AddStringFlag(commands.DeletePropertyFlag, "old_param")
158177
},
159178
expectsPayload: &model.UpdateAppVersionRequest{
160-
Tag: "release/1.2.3",
179+
ApplicationKey: "app-key",
180+
Version: "1.0.0",
181+
Tag: "release/1.2.3",
161182
Properties: map[string][]string{
162183
"status": {"rc", "validated"},
163184
},
@@ -171,7 +192,9 @@ func TestUpdateAppVersionCommand_FlagsSuite(t *testing.T) {
171192
ctx.AddStringFlag(commands.TagFlag, "")
172193
},
173194
expectsPayload: &model.UpdateAppVersionRequest{
174-
Tag: "",
195+
ApplicationKey: "app-key",
196+
Version: "1.0.0",
197+
Tag: "",
175198
},
176199
},
177200
{
@@ -206,8 +229,8 @@ func TestUpdateAppVersionCommand_FlagsSuite(t *testing.T) {
206229
var actualPayload *model.UpdateAppVersionRequest
207230
mockVersionService := mockversions.NewMockVersionService(ctrl)
208231
if !tt.expectsError {
209-
mockVersionService.EXPECT().UpdateAppVersion(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
210-
DoAndReturn(func(_ interface{}, _ string, _ string, req *model.UpdateAppVersionRequest) error {
232+
mockVersionService.EXPECT().UpdateAppVersion(gomock.Any(), gomock.Any()).
233+
DoAndReturn(func(_ interface{}, req *model.UpdateAppVersionRequest) error {
211234
actualPayload = req
212235
return nil
213236
}).Times(1)
@@ -232,8 +255,6 @@ func TestUpdateAppVersionCommand_FlagsSuite(t *testing.T) {
232255
}
233256

234257
func TestParseProperties(t *testing.T) {
235-
cmd := &updateAppVersionCommand{}
236-
237258
tests := []struct {
238259
name string
239260
input string
@@ -301,7 +322,7 @@ func TestParseProperties(t *testing.T) {
301322

302323
for _, tt := range tests {
303324
t.Run(tt.name, func(t *testing.T) {
304-
result, err := cmd.parseProperties(tt.input)
325+
result, err := utils.ParsePropertiesFlag(tt.input)
305326
if tt.expectErr {
306327
assert.Error(t, err)
307328
} else {

0 commit comments

Comments
 (0)