Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions aws/request_pagination.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,29 @@ func (r *Request) nextPageTokens() []interface{} {
tokens := []interface{}{}
tokenAdded := false
for _, outToken := range r.Operation.OutputTokens {
v, _ := awsutil.ValuesAtPath(r.Data, outToken)
if len(v) > 0 {
tokens = append(tokens, v[0])
tokenAdded = true
} else {
vs, _ := awsutil.ValuesAtPath(r.Data, outToken)

if len(vs) == 0 {
tokens = append(tokens, nil)
continue
}
v := vs[0]

switch tv := v.(type) {
case *string:
if len(StringValue(tv)) == 0 {
tokens = append(tokens, nil)
continue
}
case string:
if len(tv) == 0 {
tokens = append(tokens, nil)
continue
}
}

tokenAdded = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if they are bunch of empy strings, wouldn't tokenAdded not be set to true? Meaning this would never return the tokens

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tokenAdded is present if a valid token is added. The tokens slice is a sparse array. Its index directly correspond to indexes in another slice.

If all of the values at the outTokens paths are empty strings there will be no valid tokens in the tokens slice. In that case returning nil would be the correct action. If there is at least one valid value in outTokens paths the tokenAdded will be set to true.

tokens = append(tokens, v)
}
if !tokenAdded {
return nil
Expand Down
92 changes: 84 additions & 8 deletions aws/request_pagination_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,15 +511,15 @@ func TestPaginationWithContextNilInput(t *testing.T) {
}
}

type testPageInput struct {
NextToken string
}
type testPageOutput struct {
Value string
NextToken *string
}

func TestPagination_Standalone(t *testing.T) {
type testPageInput struct {
NextToken string
}
type testPageOutput struct {
Value string
NextToken *string
}

expect := []struct {
Value, PrevToken, NextToken string
}{
Expand Down Expand Up @@ -586,6 +586,82 @@ func TestPagination_Standalone(t *testing.T) {
}
}

func TestPagination_Standalone_Pointers(t *testing.T) {
type testPageInput struct {
NextToken *string
}
type testPageOutput struct {
Value *string
NextToken *string
}

expect := []struct {
Value, PrevToken, NextToken *string
}{
{aws.String("FirstValue"), aws.String("InitalToken"), aws.String("FirstToken")},
{aws.String("SecondValue"), aws.String("FirstToken"), aws.String("SecondToken")},
{aws.String("ThirdValue"), aws.String("SecondToken"), nil},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not have a test to test for ""?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, TestPagination_Standalone is that testcase. The SDK already had this testcase but the way it worked prevented the bug from manifesting.

}
input := testPageInput{
NextToken: expect[0].PrevToken,
}

c := awstesting.NewClient(unit.Config())
i := 0
p := aws.Pagination{
NewRequest: func() (*aws.Request, error) {
r := c.NewRequest(
&aws.Operation{
Name: "Operation",
Paginator: &aws.Paginator{
InputTokens: []string{"NextToken"},
OutputTokens: []string{"NextToken"},
},
},
&input, &testPageOutput{},
)
// Setup handlers for testing
r.Handlers.Clear()
r.Handlers.Build.PushBack(func(req *aws.Request) {
in := req.Params.(*testPageInput)
if e, a := aws.StringValue(expect[i].PrevToken), aws.StringValue(in.NextToken); e != a {
t.Errorf("%d, expect NextToken input %q, got %q", i, e, a)
}
})
r.Handlers.Unmarshal.PushBack(func(req *aws.Request) {
out := &testPageOutput{
Value: expect[i].Value,
}
if expect[i].NextToken != nil {
next := *expect[i].NextToken
out.NextToken = aws.String(next)
}
req.Data = out
})
return r, nil
},
}

for p.Next() {
data := p.Page().(*testPageOutput)

if e, a := expect[i].Value, data.Value; e != a {
t.Errorf("%d, expect Value to be %q, got %q", i, e, a)
}
if e, a := aws.StringValue(expect[i].NextToken), aws.StringValue(data.NextToken); e != a {
t.Errorf("%d, expect NextToken to be %q, got %q", i, e, a)
}

i++
}
if e, a := len(expect), i; e != a {
t.Errorf("expected to process %d pages, did %d", e, a)
}
if err := p.Err(); err != nil {
t.Fatalf("%d, expected no error, got %v", i, err)
}
}

// Benchmarks
var benchResps = []dynamodb.ListTablesOutput{
{TableNames: []string{"TABLE", "NXT"}, LastEvaluatedTableName: aws.String("NXT")},
Expand Down