fix(automation): Correctly handle offset based pagination & adminAcce…

…ss query parameter (#35)

This fixes two issues accidentally introduced when the client was moved
from monaco.

It also adds some convenience methods for PaginatedListResponse, which
were found missing when re-integrating the client into monaco.

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) {

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())
+}