Skip to content

Commit 1ab9a89

Browse files
EyalDelareaEyalDelarea
authored
Remove Async query param & Fix rollback operation (#35)
1 parent 9765b4f commit 1ab9a89

File tree

5 files changed

+117
-60
lines changed

5 files changed

+117
-60
lines changed

‎apptrust/commands/flags.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ var commandFlags = map[string][]string{
133133
user,
134134
accessToken,
135135
serverId,
136+
SyncFlag,
136137
},
137138
VersionUpdate: {
138139
url,

‎apptrust/commands/version/rollback_app_version_cmd.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ type rollbackAppVersionCommand struct {
2323
version string
2424
requestPayload *model.RollbackAppVersionRequest
2525
fromStage string
26+
sync bool
2627
}
2728

2829
func (rv *rollbackAppVersionCommand) Run() error {
@@ -31,7 +32,7 @@ func (rv *rollbackAppVersionCommand) Run() error {
3132
return err
3233
}
3334

34-
return rv.versionService.RollbackAppVersion(ctx, rv.applicationKey, rv.version, rv.requestPayload)
35+
return rv.versionService.RollbackAppVersion(ctx, rv.applicationKey, rv.version, rv.requestPayload, rv.sync)
3536
}
3637

3738
func (rv *rollbackAppVersionCommand) ServerDetails() (*coreConfig.ServerDetails, error) {
@@ -51,6 +52,8 @@ func (rv *rollbackAppVersionCommand) prepareAndRunCommand(ctx *components.Contex
5152
rv.version = ctx.Arguments[1]
5253
rv.fromStage = ctx.Arguments[2]
5354

55+
rv.sync = ctx.GetBoolTFlagValue(commands.SyncFlag)
56+
5457
serverDetails, err := utils.ServerDetailsByFlags(ctx)
5558
if err != nil {
5659
return err

‎apptrust/commands/version/rollback_app_version_cmd_test.go‎

Lines changed: 66 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -13,57 +13,78 @@ import (
1313
)
1414

1515
func TestRollbackAppVersionCommand_Run(t *testing.T) {
16-
ctrl := gomock.NewController(t)
17-
defer ctrl.Finish()
18-
19-
serverDetails := &config.ServerDetails{Url: "https://example.com"}
20-
applicationKey := "video-encoder"
21-
version := "1.5.0"
22-
requestPayload := &model.RollbackAppVersionRequest{
23-
FromStage: "qa",
16+
tests := []struct {
17+
name string
18+
applicationKey string
19+
version string
20+
fromStage string
21+
sync bool
22+
mockError error
23+
expectedError bool
24+
}{
25+
{
26+
name: "successful rollback with sync=false",
27+
applicationKey: "video-encoder",
28+
version: "1.5.0",
29+
fromStage: "qa",
30+
sync: false,
31+
mockError: nil,
32+
expectedError: false,
33+
},
34+
{
35+
name: "successful rollback with sync=true",
36+
applicationKey: "test-app",
37+
version: "1.0.0",
38+
fromStage: "qa",
39+
sync: true,
40+
mockError: nil,
41+
expectedError: false,
42+
},
43+
{
44+
name: "failed rollback",
45+
applicationKey: "video-encoder",
46+
version: "1.5.0",
47+
fromStage: "qa",
48+
sync: false,
49+
mockError: errors.New("rollback service error occurred"),
50+
expectedError: true,
51+
},
2452
}
2553

26-
mockVersionService := mockversions.NewMockVersionService(ctrl)
27-
mockVersionService.EXPECT().RollbackAppVersion(gomock.Any(), applicationKey, version, requestPayload).
28-
Return(nil).Times(1)
29-
30-
cmd := &rollbackAppVersionCommand{
31-
versionService: mockVersionService,
32-
serverDetails: serverDetails,
33-
applicationKey: applicationKey,
34-
version: version,
35-
requestPayload: requestPayload,
36-
}
54+
for _, tt := range tests {
55+
t.Run(tt.name, func(t *testing.T) {
56+
ctrl := gomock.NewController(t)
57+
defer ctrl.Finish()
3758

38-
err := cmd.Run()
39-
assert.NoError(t, err)
40-
}
59+
serverDetails := &config.ServerDetails{Url: "https://example.com"}
60+
requestPayload := &model.RollbackAppVersionRequest{
61+
FromStage: tt.fromStage,
62+
}
4163

42-
func TestRollbackAppVersionCommand_Run_Error(t *testing.T) {
43-
ctrl := gomock.NewController(t)
44-
defer ctrl.Finish()
64+
mockVersionService := mockversions.NewMockVersionService(ctrl)
65+
mockVersionService.EXPECT().RollbackAppVersion(gomock.Any(), tt.applicationKey, tt.version, requestPayload, tt.sync).
66+
Return(tt.mockError).Times(1)
4567

46-
serverDetails := &config.ServerDetails{Url: "https://example.com"}
47-
applicationKey := "video-encoder"
48-
version := "1.5.0"
49-
requestPayload := &model.RollbackAppVersionRequest{
50-
FromStage: "qa",
51-
}
52-
expectedError := errors.New("rollback service error occurred")
68+
cmd := &rollbackAppVersionCommand{
69+
versionService: mockVersionService,
70+
serverDetails: serverDetails,
71+
applicationKey: tt.applicationKey,
72+
version: tt.version,
73+
requestPayload: requestPayload,
74+
fromStage: tt.fromStage,
75+
sync: tt.sync,
76+
}
5377

54-
mockVersionService := mockversions.NewMockVersionService(ctrl)
55-
mockVersionService.EXPECT().RollbackAppVersion(gomock.Any(), applicationKey, version, requestPayload).
56-
Return(expectedError).Times(1)
78+
err := cmd.Run()
5779

58-
cmd := &rollbackAppVersionCommand{
59-
versionService: mockVersionService,
60-
serverDetails: serverDetails,
61-
applicationKey: applicationKey,
62-
version: version,
63-
requestPayload: requestPayload,
80+
if tt.expectedError {
81+
assert.Error(t, err)
82+
if tt.mockError != nil {
83+
assert.Contains(t, err.Error(), tt.mockError.Error())
84+
}
85+
} else {
86+
assert.NoError(t, err)
87+
}
88+
})
6489
}
65-
66-
err := cmd.Run()
67-
assert.Error(t, err)
68-
assert.Contains(t, err.Error(), "rollback service error occurred")
6990
}

‎apptrust/service/versions/version_service.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ type VersionService interface {
1717
CreateAppVersion(ctx service.Context, request *model.CreateAppVersionRequest) error
1818
PromoteAppVersion(ctx service.Context, applicationKey string, version string, payload *model.PromoteAppVersionRequest, sync bool) error
1919
ReleaseAppVersion(ctx service.Context, applicationKey string, version string, request *model.ReleaseAppVersionRequest, sync bool) error
20-
RollbackAppVersion(ctx service.Context, applicationKey string, version string, request *model.RollbackAppVersionRequest) error
20+
RollbackAppVersion(ctx service.Context, applicationKey string, version string, request *model.RollbackAppVersionRequest, sync bool) error
2121
DeleteAppVersion(ctx service.Context, applicationKey string, version string) error
2222
UpdateAppVersion(ctx service.Context, applicationKey string, version string, request *model.UpdateAppVersionRequest) error
2323
}
@@ -76,19 +76,25 @@ func (vs *versionService) ReleaseAppVersion(ctx service.Context, applicationKey,
7676
return nil
7777
}
7878

79-
func (vs *versionService) RollbackAppVersion(ctx service.Context, applicationKey, version string, request *model.RollbackAppVersionRequest) error {
79+
func (vs *versionService) RollbackAppVersion(ctx service.Context, applicationKey, version string, request *model.RollbackAppVersionRequest, sync bool) error {
8080
endpoint := fmt.Sprintf("/v1/applications/%s/versions/%s/rollback", applicationKey, version)
81-
response, responseBody, err := ctx.GetHttpClient().Post(endpoint, request, map[string]string{})
81+
response, responseBody, err := ctx.GetHttpClient().Post(endpoint, request, map[string]string{"async": strconv.FormatBool(!sync)})
8282
if err != nil {
8383
return err
8484
}
8585

86-
if response.StatusCode != http.StatusOK && response.StatusCode != http.StatusAccepted {
86+
// Validate status code based on sync mode
87+
expectedStatusCode := http.StatusAccepted
88+
if sync {
89+
expectedStatusCode = http.StatusOK
90+
}
91+
92+
if response.StatusCode != expectedStatusCode {
8793
return fmt.Errorf("failed to rollback app version. Status code: %d. \n%s",
8894
response.StatusCode, responseBody)
8995
}
9096

91-
log.Output("Application version deleted successfully")
97+
log.Output(string(responseBody))
9298
return nil
9399
}
94100

@@ -114,7 +120,7 @@ func (vs *versionService) UpdateAppVersion(ctx service.Context, applicationKey s
114120
return err
115121
}
116122

117-
if response.StatusCode != http.StatusAccepted {
123+
if response.StatusCode != http.StatusOK {
118124
return fmt.Errorf("failed to update app version. Status code: %d. \n%s",
119125
response.StatusCode, responseBody)
120126
}

‎apptrust/service/versions/version_service_test.go‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ func TestUpdateAppVersion(t *testing.T) {
314314
request: &model.UpdateAppVersionRequest{
315315
Tag: "release/1.2.3",
316316
},
317-
mockResponse: &http.Response{StatusCode: http.StatusAccepted},
317+
mockResponse: &http.Response{StatusCode: http.StatusOK},
318318
mockResponseBody: "{}",
319319
mockError: nil,
320320
expectError: false,
@@ -327,7 +327,7 @@ func TestUpdateAppVersion(t *testing.T) {
327327
"status": {"rc", "validated"},
328328
},
329329
},
330-
mockResponse: &http.Response{StatusCode: http.StatusAccepted},
330+
mockResponse: &http.Response{StatusCode: http.StatusOK},
331331
mockResponseBody: "{}",
332332
mockError: nil,
333333
expectError: false,
@@ -338,7 +338,7 @@ func TestUpdateAppVersion(t *testing.T) {
338338
request: &model.UpdateAppVersionRequest{
339339
DeleteProperties: []string{"legacy_param", "toBeDeleted"},
340340
},
341-
mockResponse: &http.Response{StatusCode: http.StatusAccepted},
341+
mockResponse: &http.Response{StatusCode: http.StatusOK},
342342
mockResponseBody: "{}",
343343
mockError: nil,
344344
expectError: false,
@@ -353,7 +353,7 @@ func TestUpdateAppVersion(t *testing.T) {
353353
},
354354
DeleteProperties: []string{"old_param"},
355355
},
356-
mockResponse: &http.Response{StatusCode: http.StatusAccepted},
356+
mockResponse: &http.Response{StatusCode: http.StatusOK},
357357
mockResponseBody: "{}",
358358
mockError: nil,
359359
expectError: false,
@@ -422,26 +422,29 @@ func TestRollbackAppVersion(t *testing.T) {
422422
applicationKey string
423423
version string
424424
payload *model.RollbackAppVersionRequest
425+
sync bool
425426
expectedStatus int
426427
expectedError bool
427428
}{
428429
{
429-
name: "successful rollback with 200",
430+
name: "successful rollback with sync=true",
430431
applicationKey: "video-encoder",
431432
version: "1.5.0",
432433
payload: &model.RollbackAppVersionRequest{
433434
FromStage: "qa",
434435
},
436+
sync: true,
435437
expectedStatus: http.StatusOK,
436438
expectedError: false,
437439
},
438440
{
439-
name: "successful rollback with 204",
441+
name: "successful rollback with sync=false",
440442
applicationKey: "video-encoder",
441443
version: "1.5.0",
442444
payload: &model.RollbackAppVersionRequest{
443445
FromStage: "prod",
444446
},
447+
sync: false,
445448
expectedStatus: http.StatusAccepted,
446449
expectedError: false,
447450
},
@@ -452,9 +455,32 @@ func TestRollbackAppVersion(t *testing.T) {
452455
payload: &model.RollbackAppVersionRequest{
453456
FromStage: "nonexistent",
454457
},
458+
sync: true,
455459
expectedStatus: http.StatusBadRequest,
456460
expectedError: true,
457461
},
462+
{
463+
name: "failed rollback - sync=true but got 202",
464+
applicationKey: "video-encoder",
465+
version: "1.5.0",
466+
payload: &model.RollbackAppVersionRequest{
467+
FromStage: "qa",
468+
},
469+
sync: true,
470+
expectedStatus: http.StatusAccepted,
471+
expectedError: true,
472+
},
473+
{
474+
name: "failed rollback - sync=false but got 200",
475+
applicationKey: "video-encoder",
476+
version: "1.5.0",
477+
payload: &model.RollbackAppVersionRequest{
478+
FromStage: "prod",
479+
},
480+
sync: false,
481+
expectedStatus: http.StatusOK,
482+
expectedError: true,
483+
},
458484
}
459485

460486
for _, tt := range tests {
@@ -467,11 +493,11 @@ func TestRollbackAppVersion(t *testing.T) {
467493
mockCtx.EXPECT().GetHttpClient().Return(mockClient)
468494

469495
expectedEndpoint := "/v1/applications/" + tt.applicationKey + "/versions/" + tt.version + "/rollback"
470-
mockClient.EXPECT().Post(expectedEndpoint, tt.payload, map[string]string{}).
496+
mockClient.EXPECT().Post(expectedEndpoint, tt.payload, map[string]string{"async": strconv.FormatBool(!tt.sync)}).
471497
Return(&http.Response{StatusCode: tt.expectedStatus}, []byte(""), nil)
472498

473499
service := NewVersionService()
474-
err := service.RollbackAppVersion(mockCtx, tt.applicationKey, tt.version, tt.payload)
500+
err := service.RollbackAppVersion(mockCtx, tt.applicationKey, tt.version, tt.payload, tt.sync)
475501

476502
if tt.expectedError {
477503
assert.Error(t, err)

0 commit comments

Comments
 (0)