diff --git a/api/clients/automation/automation.go b/api/clients/automation/automation.go index 3f5939d..1c93ca8 100644 --- a/api/clients/automation/automation.go +++ b/api/clients/automation/automation.go @@ -171,27 +171,35 @@ func (a Client) Update(ctx context.Context, resourceType ResourceType, id string // - ListResponse: A ListResponse which is an api.PagedListResponse containing all objects fetched from the api // - error: An error if the HTTP call fails or another error happened. func (a Client) List(ctx context.Context, resourceType ResourceType) (ListResponse, error) { + // retVal are the collected paginated API responses var retVal ListResponse + + // result is the latest API response received var result listResponse - result.Count = 1 + result.Count = 1 // ensure starting condition is met, after the first API call this will be actual total count of objects - getCount := func(l ListResponse) int { - var count int - for _, r := range l { - count += len(r.Objects) - } - return count - } + retrieved := 0 + + adminAccess := true - for getCount(retVal) < result.Count { + for retrieved < result.Count { resp, err := a.client.GET(ctx, a.resources[resourceType].Path, rest.RequestOptions{ - QueryParams: url.Values{"offset": []string{strconv.Itoa(len(retVal))}}}, - ) + QueryParams: url.Values{ + "offset": []string{strconv.Itoa(retrieved)}, + "adminAccess": []string{strconv.FormatBool(adminAccess)}, + }, + }) if err != nil { return ListResponse{}, fmt.Errorf("failed to list buckets:%w", err) } + // if Workflow API rejected the initial request with admin permissions -> continue without + if resourceType == Workflows && resp.StatusCode == http.StatusForbidden { + adminAccess = false + continue + } + // if one of the response has code != 2xx return empty list with response info if !resp.IsSuccess() { return ListResponse{ @@ -210,6 +218,8 @@ func (a Client) List(ctx context.Context, resourceType ResourceType) (ListRespon return ListResponse{}, fmt.Errorf("failed to parse list response:%w", err) } + retrieved += len(result.Objects) + b := make([][]byte, len(result.Objects)) for i, v := range result.Objects { b[i], _ = v.MarshalJSON() // marshalling the JSON back to JSON will not fail @@ -281,39 +291,37 @@ func (a Client) Delete(ctx context.Context, resourceType ResourceType, id string return Response{}, fmt.Errorf("failed to create URL: %w", err) } - workflowsAdminAccess := resourceType == Workflows - - resp, err := a.client.DELETE(ctx, path, rest.RequestOptions{ - QueryParams: url.Values{"adminAccess": []string{strconv.FormatBool(workflowsAdminAccess)}}, + resp, err := a.makeRequestWithAdminAccess(resourceType, func(options rest.RequestOptions) (rest.Response, error) { + return a.client.DELETE(ctx, path, options) }) + if err != nil { return Response{}, fmt.Errorf("unable to delete object with ID %s: %w", id, err) } - if workflowsAdminAccess && resp.StatusCode == http.StatusForbidden { - resp, err = a.client.DELETE(ctx, path, rest.RequestOptions{}) - if err != nil { - return Response{}, fmt.Errorf("unable to delete object with ID %s: %w", id, err) - } - } return Response{api.Response{StatusCode: resp.StatusCode, Data: resp.Payload, Request: resp.RequestInfo}}, nil } func (a Client) create(ctx context.Context, data []byte, resourceType ResourceType) (Response, error) { - resp, err := a.client.POST(ctx, a.resources[resourceType].Path, bytes.NewReader(data), rest.RequestOptions{}) + resp, err := a.makeRequestWithAdminAccess(resourceType, func(options rest.RequestOptions) (rest.Response, error) { + return a.client.POST(ctx, a.resources[resourceType].Path, bytes.NewReader(data), options) + }) if err != nil { return Response{}, err } return Response{api.Response{StatusCode: resp.StatusCode, Data: resp.Payload, Request: resp.RequestInfo}}, nil } + func (a Client) createWithID(ctx context.Context, resourceType ResourceType, id string, data []byte) (Response, error) { // make sure actual "id" field is set in payload if err := setIDField(id, &data); err != nil { return Response{}, fmt.Errorf("unable to set the id field in order to crate object with id %s: %w", id, err) } - resp, err := a.client.POST(ctx, a.resources[resourceType].Path, bytes.NewReader(data), rest.RequestOptions{}) + resp, err := a.makeRequestWithAdminAccess(resourceType, func(options rest.RequestOptions) (rest.Response, error) { + return a.client.POST(ctx, a.resources[resourceType].Path, bytes.NewReader(data), options) + }) if err != nil { return Response{}, err } @@ -331,23 +339,30 @@ func (a Client) update(ctx context.Context, resourceType ResourceType, id string return Response{}, fmt.Errorf("failed to create URL: %w", err) } - workflowsAdminAccess := resourceType == Workflows - - resp, err := a.client.PUT(ctx, path, bytes.NewReader(data), rest.RequestOptions{ - QueryParams: url.Values{"adminAccess": []string{strconv.FormatBool(workflowsAdminAccess)}}, + resp, err := a.makeRequestWithAdminAccess(resourceType, func(options rest.RequestOptions) (rest.Response, error) { + return a.client.PUT(ctx, path, bytes.NewReader(data), options) }) + if err != nil { return Response{}, fmt.Errorf("unable to update object with ID %s: %w", id, err) } - if workflowsAdminAccess && resp.StatusCode == http.StatusForbidden { + return Response{api.Response{StatusCode: resp.StatusCode, Data: resp.Payload, Request: resp.RequestInfo}}, nil +} + +func (a Client) makeRequestWithAdminAccess(resourceType ResourceType, request func(options rest.RequestOptions) (rest.Response, error)) (rest.Response, error) { + opt := rest.RequestOptions{ + QueryParams: url.Values{"adminAccess": []string{"true"}}, + } + + resp, err := request(opt) - resp, err = a.client.PUT(ctx, path, bytes.NewReader(data), rest.RequestOptions{}) - if err != nil { - return Response{}, fmt.Errorf("unable to update object with ID %s: %w", id, err) - } + // if Workflow API rejected the initial request with admin permissions -> retry without + if resourceType == Workflows && resp.StatusCode == http.StatusForbidden { + return request(rest.RequestOptions{}) } - return Response{api.Response{StatusCode: resp.StatusCode, Data: resp.Payload, Request: resp.RequestInfo}}, nil + + return resp, err } func unmarshalJSONList(raw *rest.Response) (listResponse, error) { diff --git a/api/clients/automation/automation_test.go b/api/clients/automation/automation_test.go index 028e2c2..cba08b5 100644 --- a/api/clients/automation/automation_test.go +++ b/api/clients/automation/automation_test.go @@ -17,12 +17,18 @@ package automation_test import ( + "fmt" "github.com/dynatrace/dynatrace-configuration-as-code-core/api" "github.com/dynatrace/dynatrace-configuration-as-code-core/api/clients/automation" "github.com/dynatrace/dynatrace-configuration-as-code-core/api/rest" "github.com/dynatrace/dynatrace-configuration-as-code-core/internal/testutils" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "net/http" + "net/http/httptest" + "net/url" + "strconv" + "strings" "testing" ) @@ -295,7 +301,7 @@ func TestAutomationClient_Upsert(t *testing.T) { ValidateRequestFunc: func(request *http.Request) { adminAccessQP := request.URL.Query()["adminAccess"] assert.Len(t, adminAccessQP, 1) - assert.Equal(t, "false", adminAccessQP[0]) + assert.Equal(t, "true", adminAccessQP[0]) }, }, }} @@ -428,7 +434,7 @@ func TestAutomationClient_Delete(t *testing.T) { assert.NotNil(t, err) }) - t.Run("Delete - adminAccess query parameter set for workflows", func(t *testing.T) { + t.Run("Delete - adminAccess query parameter set", func(t *testing.T) { responses := []testutils.ServerResponses{{ http.MethodDelete: { ResponseCode: http.StatusOK, @@ -448,7 +454,7 @@ func TestAutomationClient_Delete(t *testing.T) { ValidateRequestFunc: func(request *http.Request) { adminAccessQP := request.URL.Query()["adminAccess"] assert.Len(t, adminAccessQP, 1) - assert.Equal(t, "false", adminAccessQP[0]) + assert.Equal(t, "true", adminAccessQP[0]) }, }, }} @@ -457,8 +463,8 @@ func TestAutomationClient_Delete(t *testing.T) { defer server.Close() client := automation.NewClient(rest.NewClient(server.URL(), server.Client())) - client.Delete(testutils.ContextWithLogger(t), automation.Workflows, "91cc8988-2223-404a-a3f5-5f1a839ecd45") - client.Delete(testutils.ContextWithLogger(t), automation.BusinessCalendars, "91cc8988-2223-404a-a3f5-5f1a839ecd45") + _, _ = client.Delete(testutils.ContextWithLogger(t), automation.Workflows, "91cc8988-2223-404a-a3f5-5f1a839ecd45") + _, _ = client.Delete(testutils.ContextWithLogger(t), automation.BusinessCalendars, "91cc8988-2223-404a-a3f5-5f1a839ecd45") }) t.Run("Delete - adminAccess forbidden", func(t *testing.T) { @@ -568,7 +574,7 @@ func TestAutomationClient_List(t *testing.T) { ResponseCode: http.StatusOK, ResponseBody: payloadePages[1], ValidateRequestFunc: func(request *http.Request) { - assert.Equal(t, []string{"1"}, request.URL.Query()["offset"]) + assert.Equal(t, []string{"2"}, request.URL.Query()["offset"]) }, }, }, @@ -585,6 +591,50 @@ func TestAutomationClient_List(t *testing.T) { assert.Nil(t, err) }) + t.Run("List - Paginated - With Admin Permissions Missing", func(t *testing.T) { + responses := []testutils.ServerResponses{ + { + http.MethodGet: { + ResponseCode: http.StatusForbidden, + ResponseBody: "{}", + ValidateRequestFunc: func(request *http.Request) { + assert.Equal(t, []string{"true"}, request.URL.Query()["adminAccess"]) + }, + }, + }, + { + http.MethodGet: { + ResponseCode: http.StatusOK, + ResponseBody: `{ "count": 2,"results": [ {"id": "82e7e7a4-dc69-4a7f-b0ad-7123f579ddf6","title": "Workflow1"} ] }`, + ValidateRequestFunc: func(request *http.Request) { + assert.Equal(t, []string{"false"}, request.URL.Query()["adminAccess"]) + assert.Equal(t, []string{"0"}, request.URL.Query()["offset"]) + }, + }, + }, + { + http.MethodGet: { + ResponseCode: http.StatusOK, + ResponseBody: `{ "count": 2,"results": [ {"id": "da105889-3817-435a-8b15-ec9777374b99","title": "Workflow2"} ] }`, + ValidateRequestFunc: func(request *http.Request) { + assert.Equal(t, []string{"false"}, request.URL.Query()["adminAccess"]) + assert.Equal(t, []string{"1"}, request.URL.Query()["offset"]) + }, + }, + }, + } + server := testutils.NewHTTPTestServer(t, responses) + defer server.Close() + + client := automation.NewClient(rest.NewClient(server.URL(), server.Client())) + ctx := testutils.ContextWithLogger(t) + resp, err := client.List(ctx, automation.Workflows) + assert.Len(t, resp, 2) + assert.Len(t, resp[0].Objects, 1) + assert.Len(t, resp[1].Objects, 1) + assert.Nil(t, err) + }) + t.Run("List - Paginated - Getting one page fails", func(t *testing.T) { responses := []testutils.ServerResponses{{ http.MethodGet: { @@ -600,7 +650,7 @@ func TestAutomationClient_List(t *testing.T) { ResponseCode: http.StatusInternalServerError, ResponseBody: "{}", ValidateRequestFunc: func(request *http.Request) { - assert.Equal(t, []string{"1"}, request.URL.Query()["offset"]) + assert.Equal(t, []string{"2"}, request.URL.Query()["offset"]) }, }, }, @@ -651,3 +701,63 @@ func TestAutomationClient_List(t *testing.T) { assert.NotNil(t, err) }) } + +func TestAutomationClient_List_PaginationLogic(t *testing.T) { + + // prepare test data + workflows := make([][]byte, 100) + for i := 0; i < 100; i++ { + u, err := uuid.NewRandom() + assert.NoError(t, err) + workflows[i] = []byte(fmt.Sprintf(`{"id": "%s","title": "Workflow%d"}`, u, i)) + } + + responseTmpl := `{ "count": %d,"results": [ %s ] }` + + getResponse := func(t *testing.T, pageSize int, offsetQuery []string, serverData [][]byte) string { + offset := 0 + if len(offsetQuery) > 0 { + assert.Len(t, offsetQuery, 1) + var err error + offset, err = strconv.Atoi(offsetQuery[0]) + if err != nil { + t.Fatalf("failed to parse query string: %v", err) + } + } + + end := offset + pageSize + if end > len(serverData) { + end = len(serverData) + } + + var s []string + for _, b := range serverData[offset:end] { + s = append(s, string(b)) + } + + return fmt.Sprintf(responseTmpl, len(serverData), strings.Join(s, ",")) + } + + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + res := getResponse(t, 15, req.URL.Query()["offset"], workflows) + _, _ = rw.Write([]byte(res)) + })) + defer server.Close() + + u, err := url.Parse(server.URL) + assert.NoError(t, err) + + client := automation.NewClient(rest.NewClient(u, server.Client())) + + ctx := testutils.ContextWithLogger(t) + resp, err := client.List(ctx, automation.Workflows) + assert.Nil(t, err) + + assert.Len(t, resp, 7) // expect 7 pages - 6x full size 15, 1x size 10 + for i := 0; i < 6; i++ { + assert.Len(t, resp[i].Objects, 15) + } + assert.Len(t, resp[6].Objects, 10) + + assert.ElementsMatch(t, workflows, resp.All()) +} diff --git a/api/response.go b/api/response.go index f8d59e2..ecaf2e7 100644 --- a/api/response.go +++ b/api/response.go @@ -30,7 +30,9 @@ type Response struct { // PagedListResponse is a list of ListResponse values. // It is used by return values of APIs that support pagination. // Each ListResponse entry possibly contains multiple objects of the fetched resource type. -// To get all response objects in a single slice you can call All() +// To get all response objects in a single slice of []byte you can call All(). +// +// In case of any individual API request being unsuccessful, PagedListResponse will contain only that failed ListResponse. type PagedListResponse []ListResponse // All returns all objects of a PagedListResponse in one slice @@ -89,6 +91,35 @@ func (r Response) AsAPIError() (APIError, bool) { return APIError{}, false } +// AsAPIError converts a PagedListResponse pointer to an APIError if it represents a 4xx or 5xx error. +// If the PagedListResponse does not represent an error, it returns an empty APIError and false. +// Unlike a normal Response, a PagedListResponse represents several individual API responses as a slice. +// In case of any response being unsuccessful, PagedListResponse will contain only that response by convention - this fact +// is used by this method. +// +// Parameters: +// - [ (PagedListResponse): The PagedListResponse object to convert to an APIError. +// +// Returns: +// - (APIError, bool): An APIError containing error information and a boolean indicating +// whether the conversion was successful (true for errors, false otherwise). +func (p PagedListResponse) AsAPIError() (APIError, bool) { + if len(p) != 1 { + return APIError{}, false + } + + r := []ListResponse(p)[0] + + if r.Is4xxError() || r.Is5xxError() { + return APIError{ + Body: r.Data, + StatusCode: r.StatusCode, + Request: r.Request, + }, true + } + return APIError{}, false +} + // APIError represents an error returned by an API with associated information. type APIError struct { StatusCode int `json:"statusCode"` // StatusCode is the HTTP response status code returned by the API. @@ -128,3 +159,19 @@ func DecodeJSONObjects[T any](r ListResponse) ([]T, error) { return res, nil } + +// DecodePaginatedJSONObjects unmarshalls all objects contained in the given PagedListResponse into a slice of T. +// Alternative ways to access data are to use PagedListResponse as a []ListResponse and decode each ListResponse or +// to access and decode the entries as []byte via PagedListResponse.All. +func DecodePaginatedJSONObjects[T any](p PagedListResponse) ([]T, error) { + var res []T + for _, r := range []ListResponse(p) { + ts, err := DecodeJSONObjects[T](r) + if err != nil { + return []T{}, err + } + res = append(res, ts...) + } + + return res, nil +} diff --git a/api/response_test.go b/api/response_test.go index fee984f..e626697 100644 --- a/api/response_test.go +++ b/api/response_test.go @@ -100,6 +100,47 @@ func TestDecodeJSONObjects(t *testing.T) { assert.Equal(t, "three", res[2].Key) } +func TestDecodePaginatedJSONObjects(t *testing.T) { + response := PagedListResponse{ + ListResponse{ + Objects: [][]byte{ + []byte(`{ "key": "one" }`), + []byte(`{ "key": "two" }`), + []byte(`{ "key": "three" }`), + }, + }, + ListResponse{ + Objects: [][]byte{ + []byte(`{ "key": "four" }`), + }, + }, + ListResponse{ + Objects: [][]byte{ + []byte(`{ "key": "five" }`), + }, + }, + ListResponse{ + Objects: [][]byte{ + []byte(`{ "key": "six" }`), + }, + }, + } + type val struct { + Key string `json:"key"` + } + + res, err := DecodePaginatedJSONObjects[val](response) + + assert.NoError(t, err) + assert.Len(t, res, 6) + assert.Equal(t, "one", res[0].Key) + assert.Equal(t, "two", res[1].Key) + assert.Equal(t, "three", res[2].Key) + assert.Equal(t, "four", res[3].Key) + assert.Equal(t, "five", res[4].Key) + assert.Equal(t, "six", res[5].Key) +} + func TestAsAPIError(t *testing.T) { testCases := []struct { name string @@ -184,3 +225,84 @@ func TestPagedListResponse(t *testing.T) { assert.Equal(t, [][]byte{{'1'}, {'2'}, {'3'}, {'4'}}, pr.All()) } + +func TestPagedListResponse_AsAPIError(t *testing.T) { + testCases := []struct { + name string + given PagedListResponse + expected APIError + expectedOK bool + }{ + { + "empty list is not an error", + PagedListResponse{}, + APIError{}, + false, + }, + { + "single entry 4xx is an error", + PagedListResponse{ + ListResponse{ + Response: Response{ + StatusCode: 403, + }, + }, + }, + APIError{ + StatusCode: 403, + }, + true, + }, + { + "single entry 5xx is an error", + PagedListResponse{ + ListResponse{ + Response: Response{ + StatusCode: 500, + }, + }, + }, + APIError{ + StatusCode: 500, + }, + true, + }, + { + "several entries is not an error", + PagedListResponse{ + ListResponse{ + Response: Response{ + StatusCode: 403, + }, + }, + ListResponse{ + Response: Response{ + StatusCode: 200, + }, + }, + }, + APIError{}, + false, + }, + { + "single entry 2xx is not an error", + PagedListResponse{ + ListResponse{ + Response: Response{ + StatusCode: 201, + }, + }, + }, + APIError{}, + false, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + got, gotOK := tt.given.AsAPIError() + assert.Equal(t, tt.expected, got) + assert.Equal(t, tt.expectedOK, gotOK) + }) + } +}