Refactoring API client Paginators for better refactored API Client

[jasdel]

This issue details how the v2 AWS SDK for Go's API client Paginators can be updated based on the API client refactor proposed in #438.

Since, the API client operation methods no longer return a built Request, (e.g. GetObjectRequest), the paginator needs be refactored.

Proposed API Client Paginator Refactor

The proposed refactor of the SDK's clients is split into four parts, API Client, Paginators, Waiters, and Request Presigner helpers. This proposal covers the API client Paginator changes.

The following is an example of the existing S3 ListObjects Paginator.

req := client.GetObjectRequest(params)

p := s3.NewListObjectsPaginator(req)

The paginator's constructor would be updated to take an interface for the paginated operation client, in addition to the operation's input parameters. The constructor would continue to return the operation paginator type, (e.g. ListObjectsPaginator). The paginator's NextPage) method would be updated to take functional options for modifying the request per pages, in addition to a Context value.

The following is an example of the proposed construction of a S3 ListObjects Paginator.

p := s3.NewListObjectsPaginator(client, params)

The following is an example of the updated ListObjectsPaginator's constructor and method signatures. The client parameter is a unnamed interface to prevent additional types defined for each operation paginator and circular package imports with the api package where the ListObjectClient interface is defined.

// NewListObjectsPaginator returns a paginator for the ListObjects operation
// with the input parameters specified.
func NewListObjectsPaginator(input *ListObjectsInput,
	client interface{
		ListObjects(context.Context, *ListObjectsInput, ...func(*aws.Request)) (*ListObjectsResponse, error)
	},
) *ListObjectsPaginator { 
	// implementation
}

type ListObjectsPaginator struct {
}

func (p *ListObjectsPaginator) NextPage(context.Context, ...func(*aws.Request)) bool {
	// implementation
}

func (p *ListObjectsPaginator) CurrentPage() *ListObjectsResponse {
	// implementation
}

func (p *ListObjectsPaginator) Err() error {
	// implementation
}

Using the refactored paginator

The application creates a paginator for an operation by calling the paginator's constructor, passing in the client and input parameters. Once the application has created a paginator, (e.g. ListObjectsPaginator), the application can iterate over the response pages using the operation paginator's NextPage, CurrentPage, and Err methods.

The NextPage method must be called to iterate through the operation response pages. The method takes a context.Context and optional aws.Request functional option. The request functional options provide you the ability to customize the operation call for each page, such as adding headers, or tracing.

The following example shows the ListObjectsPaginator being created, and pages iterated over. CurrentPage returns the ListObjectsResponse for each page. If an error occurs, NextPage will return false and the for loop will break out. The application code using the paginator should check for an error via the paginator's Err method.

p := s3.NewListObjectsPaginator(input, client)

for p.NextPage(ctx) {
	tp := p.CurrentPage()
}

if err := p.Err(); err != nil {
	return fmt.Errorf("failed to list objects, %v", err)
}

Mocking the paginator

To mock out an operation paginator, the application's test should provide a mocked API client implementation for the operation being paginated. The operation specific API client should return the mocked operation responses for each page, or error behavior of the application logic being tested.

This experience could further be improved if the SDK were to generate helper mocked API Clients for the paginated operations. The mocks could take a list operation responses for the mock client provide to the paginator.

The following example of application code that takes a ListObjectsClient interface. Using the client interface allows the application's test code to provide mocked behavior for paginating ListObject operation.

func listEmptyObjects(client s3api.ListObjectsClient) ([]s3.Object, error) {
	p := s3.NewListObjectsPaginator(client)

	var objects []s3.Object
	for p.NextPage(ctx) {
		for _, obj := range p.Contents {
			if aws.Int64Value(obj.Size) == 0 {
				objects = append(objects, obj)
			})
		}
	}

	if err := p.Err(); err != nil {
		return nil, fmt.Errorf("failed to list objects, %v", err)
	}

	return objects, nil
}

The following example defines a mock type that implements the API client's ListObjects operation method. The application will pass this mocked API client to the paginator's constructor.

type mockListObjectsClient struct {
	resps []*s3.ListObjectsResponse 
}

func (p *mockListObjectsClient) ListObjects(context.Context, *s3.ListObjectsInput, ...func(*aws.Request)) (*s3.ListObjectsResponse, error) {
	resp := p.resps[0]
	p.resps = p.resps[1:]
	if len(p.resps) != 0 {
		resp.NextToken = aws.String("abc123")
	} else {
		resp.NextToken = nil
	}
	return resp
}

Using the mock ListObjectsClient, the application's unit test can replace the behavior of the API client so that the ListObjectsPaginator used by listEmptyObjects behavior can be mocked.

func TestListEmptyObjects(t *testing.T) {
	client := mockListObjectClient{resps: mockListObjectResponsePages}

	objects, err := listEmptyObjects(client)
	if err != nil {
		t.Fatalf("expect no errors, got %v", err)
	}

	// Assert objects contains correct values.
}

[cadedaniel]

Thanks for this work! This will really improve the testability of code that uses this SDK.

I have a few questions / comments.

Proposed API Client Paginator Refactor

The client parameter is a unnamed interface to prevent additional types defined for each operation paginator and circular package imports with the api package where the ListObjectClient interface is defined.

Why exactly can't this use the granular github.com/aws/aws-sdk-go-v2/service/*/api interface discussed in #438?

Using the refactored paginator

I know we discussed offline a few different ways to do pagination. I acknowledge that the variety of AWS service APIs/behaviors constrains how much the SDK can do in the area of pagination (which makes a cleaner Kubernetes-like API difficult or impossible).

I have one more suggestion on the pagination API. Consider this example from the proposal:

p := s3.NewListObjectsPaginator(input, client)

for p.NextPage(ctx) {
	tp := p.CurrentPage()
}

if err := p.Err(); err != nil {
	return fmt.Errorf("failed to list objects, %v", err)
}

I think there is a design smell here: Why can NextPage or CurrentPage fail, without returning the error message? The code does not force the user to check for err, which is an invitation for bugs. I understand from reading documentation that, in case of failure, I need to ignore the result of CurrentPage + understand that NextPage will return false; to obtain the error, use Err. I think the API would become more self-documenting + prevent people from forgetting to check Err if, instead of a separate Err function, the errors were returned in the call where they were made.

To do this, I think the cleanest way is the following:

p := s3.NewListObjectsPaginator(input, client)

for p.HasNextPage() {
     tp, err := p.NextPage(ctx)
     if err != nil {
         return fmt.Errorf("failed to list objects, %v", err)
     }
}

HasNextPage would presumably check to see if the token is empty (like how the s3 cpp sdk suggests), or if the first request has not yet been made.

Do you agree that there is a code smell / that this resolves the code smell? Is my proposal possible given the wide variety of API behaviors that the sdk supports?

Mocking the paginator

The request functional options provide you the ability to customize the operation call for each page, such as adding headers, or tracing.

Is it / will it be possible for there to be paginator-specific options? I am concerned with the mixing of production code in unit tests (the return value from New*Paginator). If the paginator modifies or consumes any option (such that it does not forward the option to the client), then it prevents customers from verifying that options are set correctly. If not, then this comment is a noop. If so, we should try to think of a mockable paginator so that customers can verify all options are set correctly.

[jasdel]

Thanks for the feedback and suggestion #530 PR provides a prototype based on these designs, and clarifies some of the behavior. We took your suggestion for paginators and simplified their methods. Instead of having a a Next method that returns a bool, a get current page, and an error method, There is just the two methods, HasMorePages which returns if their could be more pages, but doesn't do any requests, and NextPage which will retrieve the next page each time its called.

    // Use the paginator to iterate over pages returned by the API. HasMorePages
    // will return false when there are no more pages, or NextPage failed.
    for p.HasMorePages() {
        o, err := p.NextPage(context.TODO())
        if err != nil {
            log.Fatalf("failed to get next page, %v", err)
        }
        fmt.Println("Page:", o)
    }

[jasdel]

Example of a hand written paginator that the SDK code generation should use

// ListObjectsV2APIClient provides interface for the Amazon S3 API client
// ListObjectsV2 operation call.
type ListObjectsV2APIClient interface {
    ListObjectsV2(context.Context, *ListObjectsV2Input, ...func(*Options)) (*ListObjectsV2Output, error)
}

// ListObjectsV2PaginatorOptions provides the options for configuring the
// ListObjectsV2Paginator.
type ListObjectsV2PaginatorOptions struct {
    // The maximum number of keys to return per page.
    Limit int32
}

// ListObjectsV2Paginator provides the paginator to paginate S3 ListObjectsV2
// response pages.
type ListObjectsV2Paginator struct {
    options ListObjectsV2PaginatorOptions

    client ListObjectsV2APIClient
    params ListObjectsV2Input

    nextToken *string
    firstPage bool
}

// NewListObjectsV2Paginator initializes a new S3 ListObjectsV2 Paginator for
// paginating the ListObjectsV2 respones.
func NewListObjectsV2Paginator(client ListObjectsV2APIClient, params *ListObjectsV2Input, optFns ...func(*ListObjectsV2PaginatorOptions)) istObjectsV2Paginator {
    var options ListObjectsV2PaginatorOptions
    for _, fn := range optFns {
        fn(&options)
    }
    p := &S3ListObjectsV2Paginator{
        options:   options,
        client:    client,
        firstPage: true,
    }
    if params != nil {
        p.params = *params
    }
    return p
}

// HasMorePages returns true if there are more pages or if the first page has
// not been retrieved yet.
func (p *ListObjectsV2Paginator) HasMorePages() bool {
    return p.firstPage || (p.nextToken != nil && len(*p.nextToken) != 0)
}

// NextPage attempts to retrieve the next page, or returns error if unable to.
func (p *ListObjectsV2Paginator) NextPage(ctx context.Context) (
    *ListObjectsV2Output, error,
) {
    if !p.HasMorePages() {
        return nil, fmt.Errorf("no more pages available")
    }

    params := p.params
    if v := p.options.Limit; v != 0 {
        params.MaxKeys = &v
    }
    result, err := p.client.ListObjectsV2(ctx, &params)
    if err != nil {
        return nil, err
    }

    p.firstPage = false
    if result.IsTruncated != nil && *result.IsTruncated == false {
        p.nextToken = nil
    } else {
        p.nextToken = result.NextContinuationToken
    }
    p.params.ContinuationToken = p.nextToken

    return result, nil
}

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.