Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring API client operation methods for better usability #438

Closed
jasdel opened this issue Nov 22, 2019 · 4 comments
Closed

Refactoring API client operation methods for better usability #438

jasdel opened this issue Nov 22, 2019 · 4 comments
Labels
breaking-change Issue requires a breaking change to remediate. refactor

Comments

@jasdel
Copy link
Contributor

jasdel commented Nov 22, 2019

This issue details how the v2 AWS SDK for Go's API client operations can be updated to reduce the difficulty of mocking the v2 SDK's API clients in your applications. The proposed changes in this design seek to improve the v2 SDK by reducing the complexity of the API client's operation methods, paginators and waiters.

We'd like to hear your feedback on this proposed refactor to the v2 SDK!

Background

Issues such as #70, highlight the difficulty and pain writing unit test for applications which use the v2 AWS SDK for Go API clients. Specifically due to the v2 SDK API client operation methods returning a concrete structure with a method that must be invoked before a response is returned.

req := client.GetObjectRequest(params)

result, err := req.Send(context.Background())

Since the API operation request's build, and send behavior are split into two statements, the application's tests must mock both the API client's operation method, and the request struct, (e.g. GetObjectRequest). Returning an interfaces instead of a concrete type would not significantly improved the issue, since the application tests would still need to mock out the interface type in addition to the client operation method as well.

Proposed API Client 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 changes.

The refactor would replace all of the API client operation Request builder methods, (e.g. GetObjectRequest) with methods that invoke the operation directly. This pattern is similar to the v1 SDK's WithContext operation methods, (e.g. GetObjectWithContext). The new method would initialize the operation's underlying behavior, apply the application's optional customizations, and invoke the operation. The response of the operation would be returned, or error.

The following example invokes the Amazon S3 GetObject operation with the operation's GetObjectResponse or error returned via the resp, or err variables. The GetObjectOutput parameters can be

client := s3.New(cfg)

resp, err := client.GetObject(context.Background(), params)

This pattern enables request customization via functional options. Functional options are an idiomatic Go pattern for creating APIs with optional configuration. Common options can be defined by the SDK for applications to customize operation call, (e.g. add HTTP headers).

The following example is similar to the above GetObject operation call, with the addition of a custom HTTP header x-custom-header with some value on the request.

client := s3.New(cfg)

resp, err := client.GetObject(context.Background(), params,
	aws.WithHTTPHeader("x-custom-header","some value"))

The following is an example of the SDK's generated S3 API client GetObject and ListObjects method signatures.

// Client provides the API client for making operation calls to the Amazon S3 API.
type Client struct {}

// NewClient returns a client initialized for making API operation calls. 
func NewClient(cfg) *Client {
	// implementation
}

// GetObject invokes the Amazon S3 API operation GetObject with the provided
// input parameters, returning the operation's response. An error is returned
// if the operation invocation fails.
func (c *Client) GetObject(ctx context.Context, input *GetObjectInput, opts ...func(*aws.Request)) (*GetObjectResponse, error) {
	// implementation
}

// ListObjects invokes the Amazon S3 API operation ListObjects with the provided
// input parameters, returning the operation's response. An error is returned
// if the operation invocation fails.
func (c *Client) ListObjects(ctx context.Context, input *ListObjectsInput, opts ...func(*aws.Request)) (*ListObjectsResponse, error) {
	// implementation
}

SDK provided API client interfaces

The v2 SDK client all are generated with sub packages with iface suffix, (e.g. s3iface). These packages contain a interface for the API client and all of its methods. Making it easier to use an interface fro the client in your application. This refactor would rename these packages to have a api, (e.g. s3api) instead. The rename is to improve communicating the purpose of the interface package.

The generated interfaces in the api sub package would also be generated per API operation. Instead of only as a single large interface. In addition, an omnibus interface should also be created containing all operation interfaces. Applications should prefer the individual, or a composed interface of a handful the interfaces. The SDK will still generate an omibus interface for all of the API client's operations for the use cases where applications prefer that style.

The following describes the S3 client, GetObject and ListObjects interfaces. These interfaces would be generated into the s3api package, with the import path of github.com/aws/aws-sdk-go-v2/service/s3/s3api.

// Client provides an interface for all Amazon S3 API operations.
type Client interface {
	GetObjectClient
	ListObjectsClient
}

// GetObjectClient provides the interface for the Amazon S3 GetObject operation.
type GetObjectClient interface {
	GetObject(context.Context, *s3.GetObjectInput, ...func(*aws.Request)) (*s3.GetObjectResponse, error)
}

// ListObjectsClient provides the interface for the Amazon S3 ListObjects operation.
type ListObjectsClient {
	ListObjects(context.Context, *s3.ListObjectsInput, ...func(*aws.Request)) (*s3.ListObjectsResponse, error)
}

Using API client interfaces

The following example uses the S3 client's s3api.GetObjectClient interface instead of the concrete s3.Client. This pattern allows application unit test to mock out the API operation, so the test can focus on the application's behavior without any knowledge of the SDK's internal behavior.

func getS3Object(ctx context.Context, client s3api.GetObjectClient, bucket, key string) (io.ReadCloser, error) {
	result, err := client.GetObject(ctx, &s3.GetObjectInput{
		Bucket: &bucket,
		Key: &key,
	})
	if err != nil {
		return err
	}

	return result.Body, nil
}
@cadedaniel
Copy link

Thanks for this work! This will really improve the usability+testability of code that uses this SDK. I have a question about the generated code in the section Proposed API Client Refactor:

// Comments stripped for brevity.
type Client struct {}

func NewClient(cfg) *Client {
	// implementation
}

func (c *Client) GetObject(ctx context.Context, input *GetObjectInput, opts ...func(*aws.Request)) (*GetObjectResponse, error) {
	// implementation
}

func (c *Client) ListObjects(ctx context.Context, input *ListObjectsInput, opts ...func(*aws.Request)) (*ListObjectsResponse, error) {
	// implementation
}

Why is this client type Client struct exported for consumption? Why not just export the interface, such that func NewClient(cfg) *Client becomes NewClient(cfg) api.Client?

@jasdel
Copy link
Contributor Author

jasdel commented May 5, 2020

Thanks a lot for the feedback on this issue. We recently created PR #530 that takes the idea of this design proposal, tweaks it and, clarifies some of the expectations around modifying/decorating per operation behavior.

Most notably the PR implementation improves this design by removing the aws.Request, and old request handler implementations. The PR takes advantage of our new middleware.Stack.

@jasdel
Copy link
Contributor Author

jasdel commented May 5, 2020

One of the changes made in PR #530 vs this design is that the api or s3iface package and its interface is no longer generated. Instead when helper functionality like Pagination and Waiters are generated they will also trigger operation specific client interfaces to be generated.

type GetSessionAPIClient interface {
    GetSession(context.Context, *lexruntimeservice.GetSessionInput,
          ...func(*Options))
}

This interface would the be used by the relevant helper utility also generated in the package.

@jasdel
Copy link
Contributor Author

jasdel commented Oct 12, 2020

V2 SDK generated API operations are now generated with the pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Issue requires a breaking change to remediate. refactor
Projects
None yet
Development

No branches or pull requests

2 participants