Skip to content

Commit

Permalink
fix(go): Handle more cursor pagination stopping conditions (#5759)
Browse files Browse the repository at this point in the history
  • Loading branch information
amckinney authored Jan 26, 2025
1 parent 90f8e46 commit 0a97268
Show file tree
Hide file tree
Showing 112 changed files with 5,560 additions and 100 deletions.
3 changes: 3 additions & 0 deletions fern/pages/changelogs/go-sdk/2025-01-26.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## 0.36.2
**`(fix):`** Fix cursor pagination stopping condition when the cursor types do not match (e.g. a `*string` cursor type with a `string` next cursor type).

9 changes: 9 additions & 0 deletions generators/go/internal/generator/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,7 @@ func (g *Generator) generate(ir *fernir.IntermediateRepresentation, mode Mode) (
if generatedPagination {
files = append(files, newPageFile(g.coordinator))
files = append(files, newPagerFile(g.coordinator, g.config.ImportPath))
files = append(files, newPagerTestFile(g.coordinator))
}
clientTestFile, err := newClientTestFile(g.config.ImportPath, rootPackageName, g.coordinator, g.config.PackageLayout, g.config.ClientName, g.config.ClientConstructorName)
if err != nil {
Expand Down Expand Up @@ -1289,6 +1290,14 @@ func newPagerFile(coordinator *coordinator.Client, baseImportPath string) *File
)
}

func newPagerTestFile(coordinator *coordinator.Client) *File {
return NewFile(
coordinator,
"internal/pager_test.go",
[]byte(pagerTestFile),
)
}

func newPageFile(coordinator *coordinator.Client) *File {
return NewFile(
coordinator,
Expand Down
6 changes: 6 additions & 0 deletions generators/go/internal/generator/sdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ var (
//go:embed sdk/internal/pager.go
pagerFile string

//go:embed sdk/internal/pager_test.go
pagerTestFile string

//go:embed sdk/utils/pointer.go
pointerFile string

Expand Down Expand Up @@ -1360,6 +1363,7 @@ func (f *fileWriter) WriteClient(
pagerConstructor = "internal.NewCursorPager"

f.P("readPageResponse := func(response ", endpoint.ResponseType, ") *internal.PageResponse[", endpoint.PaginationInfo.PageGoType, ",", endpoint.PaginationInfo.ResultsSingleGoType, "] {")
f.P("var zeroValue ", endpoint.PaginationInfo.NextCursorGoType)
if len(endpoint.PaginationInfo.NextCursor.PropertyPath) > 0 {
f.P("var next ", endpoint.PaginationInfo.NextCursorGoType)
f.P(endpoint.PaginationInfo.NextCursorNilCheck)
Expand All @@ -1381,6 +1385,7 @@ func (f *fileWriter) WriteClient(
f.P("if next == nil {")
f.P("return &internal.PageResponse[", endpoint.PaginationInfo.PageGoType, ",", endpoint.PaginationInfo.ResultsSingleGoType, "]{")
f.P("Results: results,")
f.P("Done: next == zeroValue,")
f.P("}")
f.P("}")
}
Expand All @@ -1396,6 +1401,7 @@ func (f *fileWriter) WriteClient(
f.P("Next: next,")
}
f.P("Results: results,")
f.P("Done: next == zeroValue,")
f.P("}")
f.P("}")
case "offset":
Expand Down
4 changes: 2 additions & 2 deletions generators/go/internal/generator/sdk/internal/pager.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type PageRequest[Cursor comparable] struct {
type PageResponse[Cursor comparable, Result any] struct {
Results []Result
Next Cursor
Done bool
}

// PageRequestFunc prepares the *CallParams from the given page request.
Expand Down Expand Up @@ -117,8 +118,7 @@ func (p *Pager[
return &core.Page[Results]{
Results: pageResponse.Results,
NextPageFunc: func(ctx context.Context) (*core.Page[Results], error) {
var next Cursor
if pageResponse.Next == next {
if pageResponse.Done {
return nil, core.ErrNoPages
}
return p.GetPage(ctx, pageResponse.Next)
Expand Down
162 changes: 162 additions & 0 deletions generators/go/internal/generator/sdk/internal/pager_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
package internal

import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

type TestPageResponse struct {
Items []TestPageItem `json:"items"`
Next string `json:"next"`
}

type TestPageItem struct {
ID string `json:"id"`
Name string `json:"name"`
}

func TestPager(t *testing.T) {
tests := []struct {
description string
givePages []TestPageResponse
giveCursor string
wantItems []TestPageItem
wantError error
}{
{
description: "handles multiple pages successfully",
givePages: []TestPageResponse{
{
Items: []TestPageItem{{ID: "1", Name: "First"}},
Next: "abc",
},
{
Items: []TestPageItem{{ID: "2", Name: "Second"}},
Next: "def",
},
{
Items: []TestPageItem{{ID: "3", Name: "Third"}},
Next: "",
},
},
wantItems: []TestPageItem{
{ID: "1", Name: "First"},
{ID: "2", Name: "Second"},
{ID: "3", Name: "Third"},
},
},
{
description: "handles empty response",
givePages: []TestPageResponse{
{
Items: []TestPageItem{},
Next: "",
},
},
wantItems: nil,
},
{
description: "handles single page",
givePages: []TestPageResponse{
{
Items: []TestPageItem{{ID: "1", Name: "Only"}},
Next: "",
},
},
wantItems: []TestPageItem{
{ID: "1", Name: "Only"},
},
},
{
description: "handles initial cursor",
giveCursor: "abc",
givePages: []TestPageResponse{
{
Items: []TestPageItem{{ID: "1", Name: "First"}},
Next: "",
},
},
wantItems: []TestPageItem{
{ID: "1", Name: "First"},
},
},
}

for _, tt := range tests {
t.Run(tt.description, func(t *testing.T) {
var pageIndex int
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if pageIndex >= len(tt.givePages) {
t.Fatal("requested more pages than available")
}
if pageIndex > 0 {
assert.Equal(t, tt.givePages[pageIndex-1].Next, r.URL.Query().Get("cursor"))
}
require.NoError(t, json.NewEncoder(w).Encode(tt.givePages[pageIndex]))
pageIndex++
}))
defer server.Close()

caller := NewCaller(
&CallerParams{
Client: server.Client(),
},
)
pager := NewCursorPager(
caller,
func(request *PageRequest[*string]) *CallParams {
url := server.URL
if request.Cursor != nil {
url += "?cursor=" + *request.Cursor
}
return &CallParams{
URL: url,
Method: http.MethodGet,
Response: request.Response,
}
},
func(response *TestPageResponse) *PageResponse[*string, *TestPageItem] {
var items []*TestPageItem
for _, item := range response.Items {
itemCopy := item
items = append(items, &itemCopy)
}
var next *string
if response.Next != "" {
next = &response.Next
}
return &PageResponse[*string, *TestPageItem]{
Results: items,
Next: next,
Done: response.Next == "",
}
},
)

page, err := pager.GetPage(context.Background(), &tt.giveCursor)
if tt.wantError != nil {
assert.Equal(t, tt.wantError, err)
return
}
require.NoError(t, err)

var items []TestPageItem
iter := page.Iterator()
for iter.Next(context.Background()) {
item := iter.Current()
items = append(items, TestPageItem{
ID: item.ID,
Name: item.Name,
})
}
require.NoError(t, iter.Err())
assert.Equal(t, tt.wantItems, items)
})
}
}
7 changes: 7 additions & 0 deletions generators/go/sdk/versions.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
- version: 0.36.2
changelogEntry:
- type: fix
summary: >-
Fix cursor pagination stopping condition when the cursor types do not
match (e.g. a `*string` cursor type with a `string` next cursor type).
irVersion: 53
- version: 0.36.1
changelogEntry:
- type: fix
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"type": "object",
"properties": {
"next": {
"type": "string"
},
"data": {
"type": "array",
"items": {
"$ref": "#/definitions/users.User"
}
}
},
"required": [
"next",
"data"
],
"additionalProperties": false,
"definitions": {
"users.User": {
"type": "object",
"properties": {
"name": {
"type": "string"
},
"id": {
"type": "integer"
}
},
"required": [
"name",
"id"
],
"additionalProperties": false
}
}
}
Loading

0 comments on commit 0a97268

Please sign in to comment.