Skip to content

Commit f3fc4e2

Browse files
malte-ehrlenmalte-ehrlen
committed
review comments
1 parent b4f5448 commit f3fc4e2

File tree

3 files changed

+114
-141
lines changed

3 files changed

+114
-141
lines changed

‎services/cdn/go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module github.com/stackitcloud/stackit-sdk-go/services/cdn
33
go 1.21
44

55
require (
6+
github.com/google/go-cmp v0.7.0
67
github.com/google/uuid v1.6.0
78
github.com/stackitcloud/stackit-sdk-go/core v0.16.2
89
)

‎services/cdn/wait/wait.go‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,14 @@ func UpdateDistributionWaitHandler(ctx context.Context, api APIClientInterface,
7474
switch *distribution.Distribution.Status {
7575
case DistributionStatusActive:
7676
return true, distribution, err
77+
case DistributionStatusUpdating:
78+
return false, nil, nil
79+
case DistributionStatusDeleting:
80+
return true, nil, fmt.Errorf("updating CDN distribution failed")
81+
case DistributionStatusError:
82+
return true, nil, fmt.Errorf("updating CDN distribution failed")
7783
default:
78-
return false, distribution, err
84+
return true, nil, fmt.Errorf("UpdateDistributionWaitHandler: unexpected status %s", *distribution.Distribution.Status)
7985
}
8086
}
8187

@@ -88,16 +94,19 @@ func UpdateDistributionWaitHandler(ctx context.Context, api APIClientInterface,
8894

8995
func DeleteDistributionWaitHandler(ctx context.Context, api APIClientInterface, projectId, distributionId string) *wait.AsyncActionHandler[cdn.GetDistributionResponse] {
9096
handler := wait.New(func() (waitFinished bool, distribution *cdn.GetDistributionResponse, err error) {
91-
distribution, err = api.GetDistributionExecute(ctx, projectId, distributionId)
97+
_, err = api.GetDistributionExecute(ctx, projectId, distributionId)
9298

9399
// the distribution is still gettable, e.g. not deleted
94100
if err == nil {
95101
return false, nil, nil
96102
}
97-
oapiErr, ok := err.(*oapierror.GenericOpenAPIError) //nolint:errorlint //complaining that error.As should be used to catch wrapped errors, but this error should not be wrapped
98-
if ok && oapiErr.StatusCode == http.StatusNotFound {
99-
return true, nil, nil
103+
var oapiError *oapierror.GenericOpenAPIError
104+
if errors.As(err, &oapiError) {
105+
if statusCode := oapiError.StatusCode; statusCode == http.StatusNotFound || statusCode == http.StatusGone {
106+
return true, nil, nil
107+
}
100108
}
109+
101110
return false, nil, err
102111
})
103112
handler.SetTimeout(30 * time.Second)
@@ -139,9 +148,11 @@ func DeleteCDNCustomDomainWaitHandler(ctx context.Context, a APIClientInterface,
139148
if err == nil {
140149
return false, nil, nil
141150
}
142-
oapiErr, ok := err.(*oapierror.GenericOpenAPIError) //nolint:errorlint //complaining that error.As should be used to catch wrapped errors, but this error should not be wrapped
143-
if ok && oapiErr.StatusCode == http.StatusNotFound {
144-
return true, nil, nil
151+
var oapiError *oapierror.GenericOpenAPIError
152+
if errors.As(err, &oapiError) {
153+
if statusCode := oapiError.StatusCode; statusCode == http.StatusNotFound || statusCode == http.StatusGone {
154+
return true, nil, nil
155+
}
145156
}
146157
return false, nil, err
147158
})

‎services/cdn/wait/wait_test.go‎

Lines changed: 94 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -307,155 +307,116 @@ func TestDeleteDistributionWaitHandler(t *testing.T) {
307307
}
308308
}
309309

310-
type response struct {
311-
state string
312-
status int
313-
}
314-
315-
var (
316-
testProjectId = "testProjectId"
317-
testDistributionId = "testDistributionId"
318-
testCustomDomain = "testCustomDomain"
319-
)
320-
321-
type repeatableClientMock struct {
322-
responseIndex int
323-
responses []response
324-
}
310+
func TestUpdateDistributionWaitHandler(t *testing.T) {
311+
projectId := "test-project-id"
312+
distributionId := "test-distribution-id"
313+
statusActive := DistributionStatusActive
314+
statusUpdating := DistributionStatusUpdating
315+
statusCreating := DistributionStatusCreating
316+
statusError := DistributionStatusError
317+
statusDeleting := DistributionStatusDeleting
325318

326-
func (mock *repeatableClientMock) GetDistributionExecute(_ context.Context, _, _ string) (*cdn.GetDistributionResponse, error) {
327-
response := mock.responses[mock.responseIndex]
328-
mock.responseIndex++
329-
mock.responseIndex %= len(mock.responses)
330-
if response.status >= http.StatusBadRequest {
331-
return nil, &oapierror.GenericOpenAPIError{
332-
StatusCode: response.status,
333-
ErrorMessage: "simulated error",
319+
mockClientFixture := func(patches ...func(tc *mockApiClient)) *mockApiClient {
320+
client := &mockApiClient{
321+
distributions: []*cdn.Distribution{
322+
{
323+
Id: &distributionId,
324+
ProjectId: &projectId,
325+
Status: &statusActive,
326+
},
327+
},
328+
getDistributionError: nil,
334329
}
335-
}
336-
return &cdn.GetDistributionResponse{
337-
Distribution: &cdn.Distribution{
338-
Id: &testDistributionId,
339-
Status: &response.state,
340-
},
341-
}, nil
342-
}
343-
344-
func (mock *repeatableClientMock) GetCustomDomainExecute(_ context.Context, _, _, _ string) (*cdn.GetCustomDomainResponse, error) {
345-
response := mock.responses[mock.responseIndex]
346-
mock.responseIndex++
347-
mock.responseIndex %= len(mock.responses)
348-
if response.status >= http.StatusBadRequest {
349-
return nil, &oapierror.GenericOpenAPIError{
350-
StatusCode: response.status,
351-
ErrorMessage: "simulated error",
330+
for _, p := range patches {
331+
p(client)
352332
}
333+
return client
353334
}
354-
return &cdn.GetCustomDomainResponse{
355-
CustomDomain: &cdn.CustomDomain{
356-
Name: &testCustomDomain,
357-
Status: cdn.DOMAINSTATUS_ACTIVE.Ptr(),
358-
},
359-
}, nil
360-
}
361-
362-
func TestUpdateDistributionWaitHandler(t *testing.T) {
363-
364-
type args struct {
365-
projectId string
366-
distributionId string
367-
}
368-
type fields struct {
369-
responses []response
370-
}
371-
tests := []struct {
372-
name string
373-
args args
374-
fields fields
375-
wantStatus string
376-
wantErr bool
335+
tests := map[string]struct {
336+
apiClient *mockApiClient
337+
projectId string
338+
distributionId string
339+
expectedDistribution *cdn.GetDistributionResponse
340+
errCheck func(*testing.T, error)
377341
}{
378-
{
379-
"success immediately",
380-
args{
381-
projectId: testProjectId,
382-
distributionId: testDistributionId,
383-
},
384-
fields{
385-
responses: []response{
386-
{DistributionStatusActive, 0},
342+
"happyPath": {
343+
apiClient: mockClientFixture(),
344+
projectId: projectId,
345+
distributionId: distributionId,
346+
expectedDistribution: &cdn.GetDistributionResponse{
347+
Distribution: &cdn.Distribution{
348+
Id: &distributionId,
349+
ProjectId: &projectId,
350+
Status: &statusActive,
387351
},
388352
},
389-
DistributionStatusActive,
390-
false,
353+
errCheck: isNil,
391354
},
392-
{
393-
"success delayed",
394-
args{
395-
projectId: testProjectId,
396-
distributionId: testDistributionId,
397-
},
398-
fields{
399-
responses: []response{
400-
{DistributionStatusUpdating, 0},
401-
{DistributionStatusUpdating, 0},
402-
{DistributionStatusActive, http.StatusOK},
403-
},
355+
"statusUpdating": {
356+
apiClient: mockClientFixture(func(tc *mockApiClient) {
357+
tc.distributions[0].Status = &statusUpdating
358+
}),
359+
projectId: projectId,
360+
distributionId: distributionId,
361+
expectedDistribution: nil,
362+
errCheck: func(t *testing.T, err error) {
363+
if err.Error() == "WaitWithContext() has timed out" {
364+
return
365+
}
366+
t.Fatalf("Unexpected error: %v", err)
404367
},
405-
DistributionStatusActive,
406-
false,
407368
},
408-
{
409-
"request fails",
410-
args{
411-
projectId: testProjectId,
412-
distributionId: testDistributionId,
413-
},
414-
fields{
415-
responses: []response{
416-
{DistributionStatusUpdating, 0},
417-
{DistributionStatusUpdating, 0},
418-
{DistributionStatusUpdating, http.StatusInternalServerError},
419-
},
369+
"statusCreating": {
370+
apiClient: mockClientFixture(func(tc *mockApiClient) {
371+
tc.distributions[0].Status = &statusCreating
372+
}),
373+
projectId: projectId,
374+
distributionId: distributionId,
375+
expectedDistribution: nil,
376+
errCheck: func(t *testing.T, err error) {
377+
if strings.Contains(err.Error(), "unexpected status CREATING") {
378+
return
379+
}
380+
t.Fatalf("Unexpected error: %v", err)
420381
},
421-
"",
422-
true,
423382
},
424-
{
425-
"timeout",
426-
args{
427-
projectId: testProjectId,
428-
distributionId: testDistributionId,
383+
"statusDeleting": {
384+
apiClient: mockClientFixture(func(tc *mockApiClient) {
385+
tc.distributions[0].Status = &statusDeleting
386+
}),
387+
projectId: projectId,
388+
distributionId: distributionId,
389+
expectedDistribution: nil,
390+
errCheck: func(t *testing.T, err error) {
391+
if strings.Contains(err.Error(), "updating CDN distribution failed") {
392+
return
393+
}
394+
t.Fatalf("Unexpected error: %v", err)
429395
},
430-
fields{
431-
responses: []response{
432-
{DistributionStatusUpdating, 0},
433-
},
396+
},
397+
"statusError": {
398+
apiClient: mockClientFixture(func(tc *mockApiClient) {
399+
tc.distributions[0].Status = &statusError
400+
}),
401+
projectId: projectId,
402+
distributionId: distributionId,
403+
expectedDistribution: nil,
404+
errCheck: func(t *testing.T, err error) {
405+
if strings.Contains(err.Error(), "updating CDN distribution failed") {
406+
return
407+
}
408+
t.Fatalf("Unexpected error: %v", err)
434409
},
435-
DistributionStatusUpdating,
436-
true,
437410
},
438411
}
439-
for _, tt := range tests {
440-
t.Run(tt.name, func(t *testing.T) {
441-
apiClientMock := repeatableClientMock{
442-
responses: tt.fields.responses,
443-
}
444-
ctx := context.Background()
445-
handler := UpdateDistributionWaitHandler(ctx, &apiClientMock, tt.args.projectId, tt.args.distributionId)
446-
handler.SetTimeout(2 * time.Second)
447-
handler.SetSleepBeforeWait(100 * time.Millisecond)
448-
handler.SetThrottle(50 * time.Millisecond)
449-
response, err := handler.WaitWithContext(ctx)
450-
if (err != nil) != tt.wantErr {
451-
t.Fatalf("handler error %s, wantErr %v", err, tt.wantErr)
452-
}
453-
var actualStatus string
454-
if response != nil {
455-
actualStatus = *response.Distribution.Status
456-
}
457-
if tt.wantStatus != actualStatus {
458-
t.Errorf("wrong state want %q but got %q", tt.wantStatus, actualStatus)
412+
413+
for name, tc := range tests {
414+
t.Run(name, func(t *testing.T) {
415+
handler := UpdateDistributionWaitHandler(context.Background(), tc.apiClient, tc.projectId, tc.distributionId)
416+
dist, err := handler.SetTimeout(10 * time.Millisecond).WaitWithContext(context.Background())
417+
tc.errCheck(t, err)
418+
if diff := cmp.Diff(tc.expectedDistribution, dist); diff != "" {
419+
t.Fatalf("Unexpected response (-want, +got):\n%s", diff)
459420
}
460421
})
461422
}

0 commit comments

Comments
 (0)