From 500fa5b18dd797005b1a1e98389c0fe83b1326b1 Mon Sep 17 00:00:00 2001 From: Lena Garber Date: Wed, 26 Jun 2024 17:46:24 -0400 Subject: [PATCH 1/2] Create request object per-page in getPaginatedResults(...) --- request_helpers.go | 20 +++++++++++--------- request_helpers_test.go | 41 ++++++++++++++++++++++++----------------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/request_helpers.go b/request_helpers.go index 49b5dc401..747c8c4e3 100644 --- a/request_helpers.go +++ b/request_helpers.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "net/url" - "strconv" ) // paginatedResponse represents a single response from a paginated @@ -30,8 +29,6 @@ func getPaginatedResults[T any]( result := make([]T, 0) - req := client.R(ctx).SetResult(resultType) - if opts == nil { opts = &ListOptions{PageOptions: &PageOptions{Page: 0}} } @@ -40,15 +37,20 @@ func getPaginatedResults[T any]( opts.PageOptions = &PageOptions{Page: 0} } - // Apply all user-provided list options to the base request - if err := applyListOptionsToRequest(opts, req); err != nil { - return nil, err - } - // Makes a request to a particular page and // appends the response to the result handlePage := func(page int) error { - req.SetQueryParam("page", strconv.Itoa(page)) + // Override the page to be applied in applyListOptionsToRequest(...) + opts.Page = page + + // This request object cannot be reused for each page request + // because it can lead to possible data corruption + req := client.R(ctx).SetResult(resultType) + + // Apply all user-provided list options to the request + if err := applyListOptionsToRequest(opts, req); err != nil { + return err + } res, err := coupleAPIErrors(req.Get(endpoint)) if err != nil { diff --git a/request_helpers_test.go b/request_helpers_test.go index ced033c3f..bed2e7403 100644 --- a/request_helpers_test.go +++ b/request_helpers_test.go @@ -2,12 +2,15 @@ package linodego import ( "context" + "fmt" "math" "net/http" "reflect" "strconv" "testing" + "github.com/stretchr/testify/require" + "github.com/linode/linodego/internal/testutil" "github.com/google/go-cmp/cmp" @@ -21,6 +24,7 @@ type testResultNestedType struct { type testResultType struct { ID int `json:"id"` + Bar *string `json:"bar"` Foo string `json:"foo"` Cool testResultNestedType `json:"cool"` } @@ -161,6 +165,8 @@ func TestRequestHelpers_delete(t *testing.T) { } func TestRequestHelpers_paginateAll(t *testing.T) { + const totalResults = 4123 + client := testutil.CreateMockClient(t, NewClient) numRequests := 0 @@ -169,7 +175,7 @@ func TestRequestHelpers_paginateAll(t *testing.T) { "GET", testutil.MockRequestURL("/foo/bar"), mockPaginatedResponse( - buildPaginatedEntries(12), + buildPaginatedEntries(totalResults), &numRequests, ), ) @@ -179,27 +185,20 @@ func TestRequestHelpers_paginateAll(t *testing.T) { client, "/foo/bar", &ListOptions{ - PageSize: 4, + PageSize: 500, Filter: "{\"foo\": \"bar\"}", }, ) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) - if numRequests != 3 { - t.Fatalf("expected 3 requests, got %d", numRequests) - } + require.Equal(t, 9, numRequests) + require.Len(t, response, totalResults) - if len(response) != 12 { - t.Fatalf("expected 12 results, got %d", len(response)) - } - - for i := 0; i < 12; i++ { + for i := 0; i < totalResults; i++ { entry := response[i] - if entry.ID != i { - t.Fatalf("expected id %d, got %d", i, entry.ID) - } + + require.Equal(t, i, entry.ID) + require.Equal(t, fmt.Sprintf("test-%d", i), *entry.Bar) } } @@ -252,7 +251,9 @@ func buildPaginatedEntries(numEntries int) []testResultType { result := make([]testResultType, numEntries) for i := 0; i < numEntries; i++ { + bar := fmt.Sprintf("test-%d", i) result[i] = testResultType{ + Bar: &bar, Foo: "foo", ID: i, } @@ -282,13 +283,19 @@ func mockPaginatedResponse( } } + // Clamp the top index to prevent out of bounds issues + lastEntryIdx := pageSize * page + if lastEntryIdx > len(entries) { + lastEntryIdx = len(entries) + } + return httpmock.NewJsonResponse( 200, paginatedResponse[testResultType]{ Page: page, Pages: int(math.Ceil(float64(len(entries)) / float64(pageSize))), Results: pageSize, - Data: entries[pageSize*(page-1) : pageSize*page], + Data: entries[pageSize*(page-1) : lastEntryIdx], }, ) } From adf10bbdef6c0539b4cbe086466bdf7c2df702c6 Mon Sep 17 00:00:00 2001 From: Lena Garber Date: Thu, 27 Jun 2024 10:01:43 -0400 Subject: [PATCH 2/2] make tidy --- go.mod | 7 ++++++- go.sum | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index ae3f3c248..85774fe6e 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,12 @@ require ( gopkg.in/ini.v1 v1.66.6 ) -require github.com/stretchr/testify v1.9.0 // indirect +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/stretchr/testify v1.9.0 + gopkg.in/yaml.v3 v3.0.1 // indirect +) go 1.21 diff --git a/go.sum b/go.sum index b712de4dd..4744a7afb 100644 --- a/go.sum +++ b/go.sum @@ -64,6 +64,8 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/ini.v1 v1.66.6 h1:LATuAqN/shcYAOkv3wl2L4rkaKqkcgTBQjOyYDvcPKI= gopkg.in/ini.v1 v1.66.6/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=