From cc2b9169d6bea98e42b028c35ee654d489a610e8 Mon Sep 17 00:00:00 2001 From: Ran Vaknin Date: Fri, 27 Jan 2023 19:38:28 -0800 Subject: [PATCH 01/11] added paginator for listObjectVersions --- service/s3/handwritten-paginators.go | 102 +++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 service/s3/handwritten-paginators.go diff --git a/service/s3/handwritten-paginators.go b/service/s3/handwritten-paginators.go new file mode 100644 index 00000000000..e32a12a102f --- /dev/null +++ b/service/s3/handwritten-paginators.go @@ -0,0 +1,102 @@ +package s3 + +import ( + "context" + "fmt" +) + +// ListObjectVersionsAPIClient is a client that implements the ListObjectVersions +// operation +type ListObjectVersionsAPIClient interface { + ListObjectVersions(context.Context, *ListObjectVersionsInput, ...func(*Options)) (*ListObjectVersionsOutput, error) +} + +var _ ListObjectVersionsAPIClient = (*Client)(nil) + +// ListObjectVersionsPaginatorOptions is the paginator options for ListObjectVersions +type ListObjectVersionsPaginatorOptions struct { + // (Optional) The maximum number of ResourceRecordSets that you want Amazon Route 53 to + // return. + Limit int32 + + // Set to true if pagination should stop if the service returns a pagination token + // that matches the most recent token provided to the service. + StopOnDuplicateToken bool +} + +// ListObjectVersionsPaginator is a paginator for ListObjectVersions +type ListObjectVersionsPaginator struct { + options ListObjectVersionsPaginatorOptions + client ListObjectVersionsAPIClient + params *ListObjectVersionsInput + firstPage bool + nextKeyMarker *string + nextVersionIdMarker *string +} + +// NewListObjectVersionsPaginator returns a new ListObjectVersionsPaginator +func NewListObjectVersionsPaginator(client ListObjectVersionsAPIClient, params *ListObjectVersionsInput, optFns ...func(*ListObjectVersionsPaginatorOptions)) *ListObjectVersionsPaginator { + if params == nil { + params = &ListObjectVersionsInput{} + } + + options := ListObjectVersionsPaginatorOptions{} + + options.Limit = params.MaxKeys + + for _, fn := range optFns { + fn(&options) + } + + return &ListObjectVersionsPaginator{ + options: options, + client: client, + params: params, + firstPage: true, + nextKeyMarker: params.KeyMarker, + nextVersionIdMarker: params.VersionIdMarker, + } +} + +// HasMorePages returns a boolean indicating whether more pages are available +func (p *ListObjectVersionsPaginator) HasMorePages() bool { + return p.firstPage || (p.nextKeyMarker != nil && len(*p.nextKeyMarker) != 0) +} + +// NextPage retrieves the next ListObjectVersions page. +func (p *ListObjectVersionsPaginator) NextPage(ctx context.Context, optFns ...func(*Options)) (*ListObjectVersionsOutput, error) { + if !p.HasMorePages() { + return nil, fmt.Errorf("no more pages available") + } + + params := *p.params + params.KeyMarker = p.nextKeyMarker + + params.VersionIdMarker = p.nextVersionIdMarker + + var limit *int32 + if p.options.Limit > 0 { + limit = &p.options.Limit + } + params.MaxKeys = *limit + + result, err := p.client.ListObjectVersions(ctx, ¶ms, optFns...) + if err != nil { + return nil, err + } + p.firstPage = false + + prevToken := p.nextKeyMarker + p.nextKeyMarker = result.NextKeyMarker + + p.nextVersionIdMarker = result.NextVersionIdMarker + + if p.options.StopOnDuplicateToken && + prevToken != nil && + p.nextKeyMarker != nil && + *prevToken == *p.nextKeyMarker { + p.nextKeyMarker = nil + } + + return result, nil +} From 2934130494793c4709a5b8cd05d8836d699b2659 Mon Sep 17 00:00:00 2001 From: Ran Vaknin Date: Sun, 29 Jan 2023 21:15:08 -0800 Subject: [PATCH 02/11] Fixed pagination on ListObjectVersionsPaginator, and added ListMultipartUploads paginator --- service/s3/handwritten-paginators.go | 142 ++++++++++++++++++++++----- 1 file changed, 117 insertions(+), 25 deletions(-) diff --git a/service/s3/handwritten-paginators.go b/service/s3/handwritten-paginators.go index e32a12a102f..e064a20132e 100644 --- a/service/s3/handwritten-paginators.go +++ b/service/s3/handwritten-paginators.go @@ -26,12 +26,12 @@ type ListObjectVersionsPaginatorOptions struct { // ListObjectVersionsPaginator is a paginator for ListObjectVersions type ListObjectVersionsPaginator struct { - options ListObjectVersionsPaginatorOptions - client ListObjectVersionsAPIClient - params *ListObjectVersionsInput - firstPage bool - nextKeyMarker *string - nextVersionIdMarker *string + options ListObjectVersionsPaginatorOptions + client ListObjectVersionsAPIClient + params *ListObjectVersionsInput + firstPage bool + KeyMarker *string + VersionIdMarker *string } // NewListObjectVersionsPaginator returns a new ListObjectVersionsPaginator @@ -41,7 +41,6 @@ func NewListObjectVersionsPaginator(client ListObjectVersionsAPIClient, params * } options := ListObjectVersionsPaginatorOptions{} - options.Limit = params.MaxKeys for _, fn := range optFns { @@ -49,18 +48,18 @@ func NewListObjectVersionsPaginator(client ListObjectVersionsAPIClient, params * } return &ListObjectVersionsPaginator{ - options: options, - client: client, - params: params, - firstPage: true, - nextKeyMarker: params.KeyMarker, - nextVersionIdMarker: params.VersionIdMarker, + options: options, + client: client, + params: params, + firstPage: true, + KeyMarker: params.KeyMarker, + VersionIdMarker: params.VersionIdMarker, } } // HasMorePages returns a boolean indicating whether more pages are available func (p *ListObjectVersionsPaginator) HasMorePages() bool { - return p.firstPage || (p.nextKeyMarker != nil && len(*p.nextKeyMarker) != 0) + return p.firstPage || (p.KeyMarker != nil && len(*p.KeyMarker) != 0) } // NextPage retrieves the next ListObjectVersions page. @@ -70,15 +69,15 @@ func (p *ListObjectVersionsPaginator) NextPage(ctx context.Context, optFns ...fu } params := *p.params - params.KeyMarker = p.nextKeyMarker + params.KeyMarker = p.KeyMarker - params.VersionIdMarker = p.nextVersionIdMarker + params.VersionIdMarker = p.VersionIdMarker - var limit *int32 + var limit int32 if p.options.Limit > 0 { - limit = &p.options.Limit + limit = p.options.Limit } - params.MaxKeys = *limit + params.MaxKeys = limit result, err := p.client.ListObjectVersions(ctx, ¶ms, optFns...) if err != nil { @@ -86,16 +85,109 @@ func (p *ListObjectVersionsPaginator) NextPage(ctx context.Context, optFns ...fu } p.firstPage = false - prevToken := p.nextKeyMarker - p.nextKeyMarker = result.NextKeyMarker + prevToken := p.KeyMarker + + p.KeyMarker = result.NextKeyMarker + p.VersionIdMarker = result.NextVersionIdMarker + + if p.options.StopOnDuplicateToken && + prevToken != nil && + p.KeyMarker != nil && + *prevToken == *p.KeyMarker { + p.KeyMarker = nil + } + + return result, nil +} + +// ListMultipartUploadsAPIClient is a client that implements the ListMultipartUploads +// operation +type ListMultipartUploadsAPIClient interface { + ListMultipartUploads(context.Context, *ListMultipartUploadsInput, ...func(*Options)) (*ListMultipartUploadsOutput, error) +} + +var _ ListMultipartUploadsAPIClient = (*Client)(nil) + +// ListMultipartUploadsPaginatorOptions is the paginator options for ListMultipartUploads +type ListMultipartUploadsPaginatorOptions struct { + // (Optional) The maximum number of ResourceRecordSets that you want Amazon Route 53 to + // return. + Limit int32 + + // Set to true if pagination should stop if the service returns a pagination token + // that matches the most recent token provided to the service. + StopOnDuplicateToken bool +} + +// ListMultipartUploadsPaginator is a paginator for ListMultipartUploads +type ListMultipartUploadsPaginator struct { + options ListMultipartUploadsPaginatorOptions + client ListMultipartUploadsAPIClient + params *ListMultipartUploadsInput + firstPage bool + KeyMarker *string + UploadIdMarker *string +} + +// NewListMultipartUploadsPaginator returns a new ListMultipartUploadsPaginator +func NewListMultipartUploadsPaginator(client ListMultipartUploadsAPIClient, params *ListMultipartUploadsInput, optFns ...func(*ListMultipartUploadsPaginatorOptions)) *ListMultipartUploadsPaginator { + if params == nil { + params = &ListMultipartUploadsInput{} + } + + options := ListMultipartUploadsPaginatorOptions{} + options.Limit = params.MaxUploads + + for _, fn := range optFns { + fn(&options) + } + + return &ListMultipartUploadsPaginator{ + options: options, + client: client, + params: params, + firstPage: true, + KeyMarker: params.KeyMarker, + UploadIdMarker: params.UploadIdMarker, + } +} + +// HasMorePages returns a boolean indicating whether more pages are available +func (p *ListMultipartUploadsPaginator) HasMorePages() bool { + return p.firstPage || (p.KeyMarker != nil && len(*p.KeyMarker) != 0) +} + +// NextPage retrieves the next ListMultipartUploads page. +func (p *ListMultipartUploadsPaginator) NextPage(ctx context.Context, optFns ...func(*Options)) (*ListMultipartUploadsOutput, error) { + if !p.HasMorePages() { + return nil, fmt.Errorf("no more pages available") + } + + params := *p.params + params.KeyMarker = p.KeyMarker + params.UploadIdMarker = p.UploadIdMarker + + var limit int32 + if p.options.Limit > 0 { + limit = p.options.Limit + } + params.MaxUploads = limit + + result, err := p.client.ListMultipartUploads(ctx, ¶ms, optFns...) + if err != nil { + return nil, err + } + p.firstPage = false - p.nextVersionIdMarker = result.NextVersionIdMarker + prevToken := p.KeyMarker + p.KeyMarker = result.NextKeyMarker + p.UploadIdMarker = result.NextUploadIdMarker if p.options.StopOnDuplicateToken && prevToken != nil && - p.nextKeyMarker != nil && - *prevToken == *p.nextKeyMarker { - p.nextKeyMarker = nil + p.KeyMarker != nil && + *prevToken == *p.KeyMarker { + p.KeyMarker = nil } return result, nil From 2ea49dacfd8e002a353dbc28c5856b1c17293491 Mon Sep 17 00:00:00 2001 From: Ran Vaknin Date: Sun, 29 Jan 2023 21:20:33 -0800 Subject: [PATCH 03/11] Added changelog --- .changelog/a7aeef887867446a8c517f39c2be006e.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changelog/a7aeef887867446a8c517f39c2be006e.json diff --git a/.changelog/a7aeef887867446a8c517f39c2be006e.json b/.changelog/a7aeef887867446a8c517f39c2be006e.json new file mode 100644 index 00000000000..e0469ccbe20 --- /dev/null +++ b/.changelog/a7aeef887867446a8c517f39c2be006e.json @@ -0,0 +1,8 @@ +{ + "id": "a7aeef88-7867-446a-8c51-7f39c2be006e", + "type": "feature", + "description": "added custom paginators for listMultipartUploads and ListObjectVersions", + "modules": [ + "service/s3" + ] +} \ No newline at end of file From 050d03ea308cb6e5cbeede867fc054881e889b80 Mon Sep 17 00:00:00 2001 From: Ran Vaknin Date: Sun, 29 Jan 2023 21:45:47 -0800 Subject: [PATCH 04/11] fixed documentation --- service/s3/handwritten-paginators.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/service/s3/handwritten-paginators.go b/service/s3/handwritten-paginators.go index e064a20132e..854f22d610c 100644 --- a/service/s3/handwritten-paginators.go +++ b/service/s3/handwritten-paginators.go @@ -15,7 +15,7 @@ var _ ListObjectVersionsAPIClient = (*Client)(nil) // ListObjectVersionsPaginatorOptions is the paginator options for ListObjectVersions type ListObjectVersionsPaginatorOptions struct { - // (Optional) The maximum number of ResourceRecordSets that you want Amazon Route 53 to + // (Optional) The maximum number of Object Versions that you want Amazon Route 53 to // return. Limit int32 @@ -70,7 +70,6 @@ func (p *ListObjectVersionsPaginator) NextPage(ctx context.Context, optFns ...fu params := *p.params params.KeyMarker = p.KeyMarker - params.VersionIdMarker = p.VersionIdMarker var limit int32 @@ -110,7 +109,7 @@ var _ ListMultipartUploadsAPIClient = (*Client)(nil) // ListMultipartUploadsPaginatorOptions is the paginator options for ListMultipartUploads type ListMultipartUploadsPaginatorOptions struct { - // (Optional) The maximum number of ResourceRecordSets that you want Amazon Route 53 to + // (Optional) The maximum number of Multipart Uploads that you want Amazon Route 53 to // return. Limit int32 From 298aafca69512e105ad9b685b834abf2eb0b18c9 Mon Sep 17 00:00:00 2001 From: Ran Vaknin Date: Wed, 15 Feb 2023 10:57:52 -0800 Subject: [PATCH 05/11] Fixed pr suggestions --- service/s3/handwritten-paginators.go | 66 ++++++++++++++++------------ 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/service/s3/handwritten-paginators.go b/service/s3/handwritten-paginators.go index 854f22d610c..84b20cacd81 100644 --- a/service/s3/handwritten-paginators.go +++ b/service/s3/handwritten-paginators.go @@ -15,7 +15,7 @@ var _ ListObjectVersionsAPIClient = (*Client)(nil) // ListObjectVersionsPaginatorOptions is the paginator options for ListObjectVersions type ListObjectVersionsPaginatorOptions struct { - // (Optional) The maximum number of Object Versions that you want Amazon Route 53 to + // (Optional) The maximum number of Object Versions that you want Amazon S3 to // return. Limit int32 @@ -30,8 +30,8 @@ type ListObjectVersionsPaginator struct { client ListObjectVersionsAPIClient params *ListObjectVersionsInput firstPage bool - KeyMarker *string - VersionIdMarker *string + keyMarker *string + versionIdMarker *string } // NewListObjectVersionsPaginator returns a new ListObjectVersionsPaginator @@ -52,14 +52,14 @@ func NewListObjectVersionsPaginator(client ListObjectVersionsAPIClient, params * client: client, params: params, firstPage: true, - KeyMarker: params.KeyMarker, - VersionIdMarker: params.VersionIdMarker, + keyMarker: params.KeyMarker, + versionIdMarker: params.VersionIdMarker, } } // HasMorePages returns a boolean indicating whether more pages are available func (p *ListObjectVersionsPaginator) HasMorePages() bool { - return p.firstPage || (p.KeyMarker != nil && len(*p.KeyMarker) != 0) + return p.firstPage || (p.keyMarker != nil && len(*p.keyMarker) != 0) && (p.versionIdMarker != nil && len(*p.versionIdMarker) != 0) } // NextPage retrieves the next ListObjectVersions page. @@ -69,8 +69,8 @@ func (p *ListObjectVersionsPaginator) NextPage(ctx context.Context, optFns ...fu } params := *p.params - params.KeyMarker = p.KeyMarker - params.VersionIdMarker = p.VersionIdMarker + params.KeyMarker = p.keyMarker + params.VersionIdMarker = p.versionIdMarker var limit int32 if p.options.Limit > 0 { @@ -84,16 +84,19 @@ func (p *ListObjectVersionsPaginator) NextPage(ctx context.Context, optFns ...fu } p.firstPage = false - prevToken := p.KeyMarker - - p.KeyMarker = result.NextKeyMarker - p.VersionIdMarker = result.NextVersionIdMarker + prevToken := p.keyMarker + p.keyMarker = nil + p.versionIdMarker = nil + if result.IsTruncated { + p.keyMarker = result.NextKeyMarker + p.versionIdMarker = result.NextVersionIdMarker + } if p.options.StopOnDuplicateToken && prevToken != nil && - p.KeyMarker != nil && - *prevToken == *p.KeyMarker { - p.KeyMarker = nil + p.keyMarker != nil && + *prevToken == *p.keyMarker { + p.keyMarker = nil } return result, nil @@ -109,7 +112,7 @@ var _ ListMultipartUploadsAPIClient = (*Client)(nil) // ListMultipartUploadsPaginatorOptions is the paginator options for ListMultipartUploads type ListMultipartUploadsPaginatorOptions struct { - // (Optional) The maximum number of Multipart Uploads that you want Amazon Route 53 to + // (Optional) The maximum number of Multipart Uploads that you want Amazon S3 to // return. Limit int32 @@ -124,8 +127,8 @@ type ListMultipartUploadsPaginator struct { client ListMultipartUploadsAPIClient params *ListMultipartUploadsInput firstPage bool - KeyMarker *string - UploadIdMarker *string + keyMarker *string + uploadIdMarker *string } // NewListMultipartUploadsPaginator returns a new ListMultipartUploadsPaginator @@ -146,14 +149,14 @@ func NewListMultipartUploadsPaginator(client ListMultipartUploadsAPIClient, para client: client, params: params, firstPage: true, - KeyMarker: params.KeyMarker, - UploadIdMarker: params.UploadIdMarker, + keyMarker: params.KeyMarker, + uploadIdMarker: params.UploadIdMarker, } } // HasMorePages returns a boolean indicating whether more pages are available func (p *ListMultipartUploadsPaginator) HasMorePages() bool { - return p.firstPage || (p.KeyMarker != nil && len(*p.KeyMarker) != 0) + return p.firstPage || (p.keyMarker != nil && len(*p.keyMarker) != 0) && (p.uploadIdMarker != nil && len(*p.uploadIdMarker) != 0) } // NextPage retrieves the next ListMultipartUploads page. @@ -163,8 +166,8 @@ func (p *ListMultipartUploadsPaginator) NextPage(ctx context.Context, optFns ... } params := *p.params - params.KeyMarker = p.KeyMarker - params.UploadIdMarker = p.UploadIdMarker + params.KeyMarker = p.keyMarker + params.UploadIdMarker = p.uploadIdMarker var limit int32 if p.options.Limit > 0 { @@ -178,15 +181,20 @@ func (p *ListMultipartUploadsPaginator) NextPage(ctx context.Context, optFns ... } p.firstPage = false - prevToken := p.KeyMarker - p.KeyMarker = result.NextKeyMarker - p.UploadIdMarker = result.NextUploadIdMarker + prevToken := p.keyMarker + + p.keyMarker = nil + p.uploadIdMarker = nil + if result.IsTruncated { + p.keyMarker = result.NextKeyMarker + p.uploadIdMarker = result.NextUploadIdMarker + } if p.options.StopOnDuplicateToken && prevToken != nil && - p.KeyMarker != nil && - *prevToken == *p.KeyMarker { - p.KeyMarker = nil + p.keyMarker != nil && + *prevToken == *p.keyMarker { + p.keyMarker = nil } return result, nil From 8440e48e6cb3d6805e1e78ac619df6bff958fbc9 Mon Sep 17 00:00:00 2001 From: Tianyi Wang Date: Wed, 12 Apr 2023 17:27:10 -0400 Subject: [PATCH 06/11] add unit test for handwritten paginators --- ...aginators.go => handwritten_paginators.go} | 0 service/s3/handwritten_paginators_test.go | 125 ++++++++++++++++++ 2 files changed, 125 insertions(+) rename service/s3/{handwritten-paginators.go => handwritten_paginators.go} (100%) create mode 100644 service/s3/handwritten_paginators_test.go diff --git a/service/s3/handwritten-paginators.go b/service/s3/handwritten_paginators.go similarity index 100% rename from service/s3/handwritten-paginators.go rename to service/s3/handwritten_paginators.go diff --git a/service/s3/handwritten_paginators_test.go b/service/s3/handwritten_paginators_test.go new file mode 100644 index 00000000000..c130b2c0228 --- /dev/null +++ b/service/s3/handwritten_paginators_test.go @@ -0,0 +1,125 @@ +package s3 + +import ( + "context" + "fmt" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/smithy-go/middleware" + "testing" +) + +var limit int32 + +type testListMPUMiddleware struct { + id int +} + +func (m *testListMPUMiddleware) ID() string { + return fmt.Sprintf("mock middleware %d", m.id) +} + +type testListOVMiddleware struct { + id int +} + +func (m *testListMPUMiddleware) HandleInitialize(ctx context.Context, input middleware.InitializeInput, next middleware.InitializeHandler) ( + output middleware.InitializeOutput, metadata middleware.Metadata, err error, +) { + limit = input.Parameters.(*ListMultipartUploadsInput).MaxUploads + return middleware.InitializeOutput{Result: &ListMultipartUploadsOutput{}}, metadata, nil +} + +func (m *testListOVMiddleware) ID() string { + return fmt.Sprintf("mock middleware %d", m.id) +} + +func (m *testListOVMiddleware) HandleInitialize(ctx context.Context, input middleware.InitializeInput, next middleware.InitializeHandler) ( + output middleware.InitializeOutput, metadata middleware.Metadata, err error, +) { + limit = input.Parameters.(*ListObjectVersionsInput).MaxKeys + return middleware.InitializeOutput{Result: &ListObjectVersionsOutput{}}, metadata, nil +} + +func TestListMultipartUploadsPaginator(t *testing.T) { + cases := map[string]struct { + limit int32 + }{ + "page limit 5": { + limit: 5, + }, + "page limit 10": { + limit: 10, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + client := NewFromConfig(aws.Config{}) + + paginator := NewListMultipartUploadsPaginator(client, &ListMultipartUploadsInput{ + Bucket: aws.String("test-bucket"), + }, func(options *ListMultipartUploadsPaginatorOptions) { + options.Limit = c.limit + }) + if !paginator.HasMorePages() { + t.Errorf("Expect paginator has more page, got not") + } + + paginator.NextPage(context.TODO(), initializeMiddlewareFn(&testListMPUMiddleware{1})) + + testNextPageResult(c.limit, paginator.keyMarker, paginator.uploadIdMarker, t) + }) + } +} + +func TestListObjectVersionsPaginator(t *testing.T) { + cases := map[string]struct { + limit int32 + }{ + "page limit 5": { + limit: 5, + }, + "page limit 10": { + limit: 10, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + client := NewFromConfig(aws.Config{}) + + paginator := NewListObjectVersionsPaginator(client, &ListObjectVersionsInput{ + Bucket: aws.String("test-bucket"), + }, func(options *ListObjectVersionsPaginatorOptions) { + options.Limit = c.limit + }) + if !paginator.HasMorePages() { + t.Errorf("Expect paginator has more page, got not") + } + + paginator.NextPage(context.TODO(), initializeMiddlewareFn(&testListOVMiddleware{1})) + + testNextPageResult(c.limit, paginator.keyMarker, paginator.versionIdMarker, t) + }) + } +} + +// insert middleware at the beginning of initialize step to see if page limit +// can be passed to API call's stack input +func initializeMiddlewareFn(initializeMiddleware middleware.InitializeMiddleware) func(*Options) { + return func(options *Options) { + options.APIOptions = append(options.APIOptions, func(stack *middleware.Stack) error { + return stack.Initialize.Add(initializeMiddleware, middleware.Before) + }) + } +} + +// unit test can not control client API call's output, so just check marker's default nil value +func testNextPageResult(expectLimit int32, keyMarker *string, idMarker *string, t *testing.T) { + if expectLimit != limit { + t.Errorf("Expect page limit to be %d, got %d", expectLimit, limit) + } + if keyMarker != nil || idMarker != nil { + t.Errorf("Expect paginator keyMarker and idMarker to be nil, got %s and %s", *keyMarker, *idMarker) + } +} From 5b730b4247fab448fb7f63e08fc87f8a7041cd33 Mon Sep 17 00:00:00 2001 From: Tianyi Wang Date: Wed, 12 Apr 2023 17:50:24 -0400 Subject: [PATCH 07/11] change variable name --- service/s3/handwritten_paginators.go | 24 +++++++++++------------ service/s3/handwritten_paginators_test.go | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/service/s3/handwritten_paginators.go b/service/s3/handwritten_paginators.go index 84b20cacd81..adef26ed384 100644 --- a/service/s3/handwritten_paginators.go +++ b/service/s3/handwritten_paginators.go @@ -31,7 +31,7 @@ type ListObjectVersionsPaginator struct { params *ListObjectVersionsInput firstPage bool keyMarker *string - versionIdMarker *string + versionIDMarker *string } // NewListObjectVersionsPaginator returns a new ListObjectVersionsPaginator @@ -53,13 +53,13 @@ func NewListObjectVersionsPaginator(client ListObjectVersionsAPIClient, params * params: params, firstPage: true, keyMarker: params.KeyMarker, - versionIdMarker: params.VersionIdMarker, + versionIDMarker: params.VersionIdMarker, } } // HasMorePages returns a boolean indicating whether more pages are available func (p *ListObjectVersionsPaginator) HasMorePages() bool { - return p.firstPage || (p.keyMarker != nil && len(*p.keyMarker) != 0) && (p.versionIdMarker != nil && len(*p.versionIdMarker) != 0) + return p.firstPage || (p.keyMarker != nil && len(*p.keyMarker) != 0) && (p.versionIDMarker != nil && len(*p.versionIDMarker) != 0) } // NextPage retrieves the next ListObjectVersions page. @@ -70,7 +70,7 @@ func (p *ListObjectVersionsPaginator) NextPage(ctx context.Context, optFns ...fu params := *p.params params.KeyMarker = p.keyMarker - params.VersionIdMarker = p.versionIdMarker + params.VersionIdMarker = p.versionIDMarker var limit int32 if p.options.Limit > 0 { @@ -86,10 +86,10 @@ func (p *ListObjectVersionsPaginator) NextPage(ctx context.Context, optFns ...fu prevToken := p.keyMarker p.keyMarker = nil - p.versionIdMarker = nil + p.versionIDMarker = nil if result.IsTruncated { p.keyMarker = result.NextKeyMarker - p.versionIdMarker = result.NextVersionIdMarker + p.versionIDMarker = result.NextVersionIdMarker } if p.options.StopOnDuplicateToken && @@ -128,7 +128,7 @@ type ListMultipartUploadsPaginator struct { params *ListMultipartUploadsInput firstPage bool keyMarker *string - uploadIdMarker *string + uploadIDMarker *string } // NewListMultipartUploadsPaginator returns a new ListMultipartUploadsPaginator @@ -150,13 +150,13 @@ func NewListMultipartUploadsPaginator(client ListMultipartUploadsAPIClient, para params: params, firstPage: true, keyMarker: params.KeyMarker, - uploadIdMarker: params.UploadIdMarker, + uploadIDMarker: params.UploadIdMarker, } } // HasMorePages returns a boolean indicating whether more pages are available func (p *ListMultipartUploadsPaginator) HasMorePages() bool { - return p.firstPage || (p.keyMarker != nil && len(*p.keyMarker) != 0) && (p.uploadIdMarker != nil && len(*p.uploadIdMarker) != 0) + return p.firstPage || (p.keyMarker != nil && len(*p.keyMarker) != 0) && (p.uploadIDMarker != nil && len(*p.uploadIDMarker) != 0) } // NextPage retrieves the next ListMultipartUploads page. @@ -167,7 +167,7 @@ func (p *ListMultipartUploadsPaginator) NextPage(ctx context.Context, optFns ... params := *p.params params.KeyMarker = p.keyMarker - params.UploadIdMarker = p.uploadIdMarker + params.UploadIdMarker = p.uploadIDMarker var limit int32 if p.options.Limit > 0 { @@ -184,10 +184,10 @@ func (p *ListMultipartUploadsPaginator) NextPage(ctx context.Context, optFns ... prevToken := p.keyMarker p.keyMarker = nil - p.uploadIdMarker = nil + p.uploadIDMarker = nil if result.IsTruncated { p.keyMarker = result.NextKeyMarker - p.uploadIdMarker = result.NextUploadIdMarker + p.uploadIDMarker = result.NextUploadIdMarker } if p.options.StopOnDuplicateToken && diff --git a/service/s3/handwritten_paginators_test.go b/service/s3/handwritten_paginators_test.go index c130b2c0228..6ed7a692977 100644 --- a/service/s3/handwritten_paginators_test.go +++ b/service/s3/handwritten_paginators_test.go @@ -67,7 +67,7 @@ func TestListMultipartUploadsPaginator(t *testing.T) { paginator.NextPage(context.TODO(), initializeMiddlewareFn(&testListMPUMiddleware{1})) - testNextPageResult(c.limit, paginator.keyMarker, paginator.uploadIdMarker, t) + testNextPageResult(c.limit, paginator.keyMarker, paginator.uploadIDMarker, t) }) } } @@ -99,7 +99,7 @@ func TestListObjectVersionsPaginator(t *testing.T) { paginator.NextPage(context.TODO(), initializeMiddlewareFn(&testListOVMiddleware{1})) - testNextPageResult(c.limit, paginator.keyMarker, paginator.versionIdMarker, t) + testNextPageResult(c.limit, paginator.keyMarker, paginator.versionIDMarker, t) }) } } From 83a015a26ec1609bd405635c12d6df31a765268a Mon Sep 17 00:00:00 2001 From: Tianyi Wang Date: Wed, 12 Apr 2023 23:15:35 -0400 Subject: [PATCH 08/11] add unit test feature for handwritten_paginators --- service/s3/handwritten_paginators_test.go | 91 ++++++++++++++++------- 1 file changed, 63 insertions(+), 28 deletions(-) diff --git a/service/s3/handwritten_paginators_test.go b/service/s3/handwritten_paginators_test.go index 6ed7a692977..a689e3b556f 100644 --- a/service/s3/handwritten_paginators_test.go +++ b/service/s3/handwritten_paginators_test.go @@ -8,24 +8,31 @@ import ( "testing" ) +var bucket *string var limit int32 +var keyMarker *string +var idMarker *string type testListMPUMiddleware struct { id int } -func (m *testListMPUMiddleware) ID() string { - return fmt.Sprintf("mock middleware %d", m.id) -} - type testListOVMiddleware struct { id int } +func (m *testListMPUMiddleware) ID() string { + return fmt.Sprintf("mock middleware %d", m.id) +} + func (m *testListMPUMiddleware) HandleInitialize(ctx context.Context, input middleware.InitializeInput, next middleware.InitializeHandler) ( output middleware.InitializeOutput, metadata middleware.Metadata, err error, ) { - limit = input.Parameters.(*ListMultipartUploadsInput).MaxUploads + params := input.Parameters.(*ListMultipartUploadsInput) + bucket = params.Bucket + limit = params.MaxUploads + keyMarker = params.KeyMarker + idMarker = params.UploadIdMarker return middleware.InitializeOutput{Result: &ListMultipartUploadsOutput{}}, metadata, nil } @@ -36,19 +43,32 @@ func (m *testListOVMiddleware) ID() string { func (m *testListOVMiddleware) HandleInitialize(ctx context.Context, input middleware.InitializeInput, next middleware.InitializeHandler) ( output middleware.InitializeOutput, metadata middleware.Metadata, err error, ) { - limit = input.Parameters.(*ListObjectVersionsInput).MaxKeys + params := input.Parameters.(*ListObjectVersionsInput) + bucket = params.Bucket + limit = params.MaxKeys + keyMarker = params.KeyMarker + idMarker = params.VersionIdMarker return middleware.InitializeOutput{Result: &ListObjectVersionsOutput{}}, metadata, nil } +type testCase struct { + bucket *string + limit int32 + keyMarker *string + idMarker *string +} + func TestListMultipartUploadsPaginator(t *testing.T) { - cases := map[string]struct { - limit int32 - }{ - "page limit 5": { - limit: 5, + cases := map[string]testCase{ + "page limit 5 without marker": { + bucket: aws.String("testBucket1"), + limit: 5, }, - "page limit 10": { - limit: 10, + "page limit 10 with marker": { + bucket: aws.String("testBucket2"), + limit: 10, + keyMarker: aws.String("testKey1"), + idMarker: aws.String("abc"), }, } @@ -57,7 +77,9 @@ func TestListMultipartUploadsPaginator(t *testing.T) { client := NewFromConfig(aws.Config{}) paginator := NewListMultipartUploadsPaginator(client, &ListMultipartUploadsInput{ - Bucket: aws.String("test-bucket"), + Bucket: c.bucket, + KeyMarker: c.keyMarker, + UploadIdMarker: c.idMarker, }, func(options *ListMultipartUploadsPaginatorOptions) { options.Limit = c.limit }) @@ -67,20 +89,22 @@ func TestListMultipartUploadsPaginator(t *testing.T) { paginator.NextPage(context.TODO(), initializeMiddlewareFn(&testListMPUMiddleware{1})) - testNextPageResult(c.limit, paginator.keyMarker, paginator.uploadIDMarker, t) + testNextPageResult(c, paginator.keyMarker, paginator.uploadIDMarker, t) }) } } func TestListObjectVersionsPaginator(t *testing.T) { - cases := map[string]struct { - limit int32 - }{ + cases := map[string]testCase{ "page limit 5": { - limit: 5, + bucket: aws.String("testBucket3"), + limit: 5, }, - "page limit 10": { - limit: 10, + "page limit 10 with marker": { + bucket: aws.String("testBucket4"), + limit: 10, + keyMarker: aws.String("testKey2"), + idMarker: aws.String("def"), }, } @@ -89,7 +113,9 @@ func TestListObjectVersionsPaginator(t *testing.T) { client := NewFromConfig(aws.Config{}) paginator := NewListObjectVersionsPaginator(client, &ListObjectVersionsInput{ - Bucket: aws.String("test-bucket"), + Bucket: c.bucket, + KeyMarker: c.keyMarker, + VersionIdMarker: c.idMarker, }, func(options *ListObjectVersionsPaginatorOptions) { options.Limit = c.limit }) @@ -99,7 +125,7 @@ func TestListObjectVersionsPaginator(t *testing.T) { paginator.NextPage(context.TODO(), initializeMiddlewareFn(&testListOVMiddleware{1})) - testNextPageResult(c.limit, paginator.keyMarker, paginator.versionIDMarker, t) + testNextPageResult(c, paginator.keyMarker, paginator.versionIDMarker, t) }) } } @@ -115,11 +141,20 @@ func initializeMiddlewareFn(initializeMiddleware middleware.InitializeMiddleware } // unit test can not control client API call's output, so just check marker's default nil value -func testNextPageResult(expectLimit int32, keyMarker *string, idMarker *string, t *testing.T) { - if expectLimit != limit { - t.Errorf("Expect page limit to be %d, got %d", expectLimit, limit) +func testNextPageResult(c testCase, pKeyMarker *string, pIdMarker *string, t *testing.T) { + if c.limit != limit { + t.Errorf("Expect page limit to be %d, got %d", c.limit, limit) + } + if *c.bucket != *bucket { + t.Errorf("Expect bucket to be %s, got %s", *c.bucket, *bucket) + } + if c.keyMarker != nil && *c.keyMarker != *keyMarker { + t.Errorf("Expect keyMarker to be %s, got %s", *c.keyMarker, *keyMarker) + } + if c.idMarker != nil && *c.idMarker != *idMarker { + t.Errorf("Expect idMarker to be %s, got %s", *c.idMarker, *idMarker) } - if keyMarker != nil || idMarker != nil { - t.Errorf("Expect paginator keyMarker and idMarker to be nil, got %s and %s", *keyMarker, *idMarker) + if pKeyMarker != nil || pIdMarker != nil { + t.Errorf("Expect paginator keyMarker and idMarker to be nil, got %s and %s", *pKeyMarker, *pIdMarker) } } From c7d2664f49d61120af59490367f1511ad5781d3a Mon Sep 17 00:00:00 2001 From: Tianyi Wang Date: Wed, 12 Apr 2023 23:33:13 -0400 Subject: [PATCH 09/11] add unit test feature for handwritten_paginators --- service/s3/handwritten_paginators_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/service/s3/handwritten_paginators_test.go b/service/s3/handwritten_paginators_test.go index a689e3b556f..9f8ee9066e1 100644 --- a/service/s3/handwritten_paginators_test.go +++ b/service/s3/handwritten_paginators_test.go @@ -141,7 +141,7 @@ func initializeMiddlewareFn(initializeMiddleware middleware.InitializeMiddleware } // unit test can not control client API call's output, so just check marker's default nil value -func testNextPageResult(c testCase, pKeyMarker *string, pIdMarker *string, t *testing.T) { +func testNextPageResult(c testCase, pKeyMarker *string, pIDMarker *string, t *testing.T) { if c.limit != limit { t.Errorf("Expect page limit to be %d, got %d", c.limit, limit) } @@ -154,7 +154,7 @@ func testNextPageResult(c testCase, pKeyMarker *string, pIdMarker *string, t *te if c.idMarker != nil && *c.idMarker != *idMarker { t.Errorf("Expect idMarker to be %s, got %s", *c.idMarker, *idMarker) } - if pKeyMarker != nil || pIdMarker != nil { - t.Errorf("Expect paginator keyMarker and idMarker to be nil, got %s and %s", *pKeyMarker, *pIdMarker) + if pKeyMarker != nil || pIDMarker != nil { + t.Errorf("Expect paginator keyMarker and idMarker to be nil, got %s and %s", *pKeyMarker, *pIDMarker) } } From 0d3f4910f964db0541bd73b9a96588618c7f34d5 Mon Sep 17 00:00:00 2001 From: Tianyi Wang Date: Tue, 18 Apr 2023 14:54:46 -0400 Subject: [PATCH 10/11] modify s3 paginator unit test logic --- service/s3/handwritten_paginators.go | 13 +- service/s3/handwritten_paginators_test.go | 320 +++++++++++++++------- 2 files changed, 224 insertions(+), 109 deletions(-) diff --git a/service/s3/handwritten_paginators.go b/service/s3/handwritten_paginators.go index adef26ed384..3d0b25100d8 100644 --- a/service/s3/handwritten_paginators.go +++ b/service/s3/handwritten_paginators.go @@ -32,6 +32,7 @@ type ListObjectVersionsPaginator struct { firstPage bool keyMarker *string versionIDMarker *string + isTruncated bool } // NewListObjectVersionsPaginator returns a new ListObjectVersionsPaginator @@ -59,7 +60,7 @@ func NewListObjectVersionsPaginator(client ListObjectVersionsAPIClient, params * // HasMorePages returns a boolean indicating whether more pages are available func (p *ListObjectVersionsPaginator) HasMorePages() bool { - return p.firstPage || (p.keyMarker != nil && len(*p.keyMarker) != 0) && (p.versionIDMarker != nil && len(*p.versionIDMarker) != 0) + return p.firstPage || p.isTruncated } // NextPage retrieves the next ListObjectVersions page. @@ -85,6 +86,7 @@ func (p *ListObjectVersionsPaginator) NextPage(ctx context.Context, optFns ...fu p.firstPage = false prevToken := p.keyMarker + p.isTruncated = result.IsTruncated p.keyMarker = nil p.versionIDMarker = nil if result.IsTruncated { @@ -96,7 +98,7 @@ func (p *ListObjectVersionsPaginator) NextPage(ctx context.Context, optFns ...fu prevToken != nil && p.keyMarker != nil && *prevToken == *p.keyMarker { - p.keyMarker = nil + p.isTruncated = false } return result, nil @@ -129,6 +131,7 @@ type ListMultipartUploadsPaginator struct { firstPage bool keyMarker *string uploadIDMarker *string + isTruncated bool } // NewListMultipartUploadsPaginator returns a new ListMultipartUploadsPaginator @@ -156,7 +159,7 @@ func NewListMultipartUploadsPaginator(client ListMultipartUploadsAPIClient, para // HasMorePages returns a boolean indicating whether more pages are available func (p *ListMultipartUploadsPaginator) HasMorePages() bool { - return p.firstPage || (p.keyMarker != nil && len(*p.keyMarker) != 0) && (p.uploadIDMarker != nil && len(*p.uploadIDMarker) != 0) + return p.firstPage || p.isTruncated } // NextPage retrieves the next ListMultipartUploads page. @@ -182,7 +185,7 @@ func (p *ListMultipartUploadsPaginator) NextPage(ctx context.Context, optFns ... p.firstPage = false prevToken := p.keyMarker - + p.isTruncated = result.IsTruncated p.keyMarker = nil p.uploadIDMarker = nil if result.IsTruncated { @@ -194,7 +197,7 @@ func (p *ListMultipartUploadsPaginator) NextPage(ctx context.Context, optFns ... prevToken != nil && p.keyMarker != nil && *prevToken == *p.keyMarker { - p.keyMarker = nil + p.isTruncated = false } return result, nil diff --git a/service/s3/handwritten_paginators_test.go b/service/s3/handwritten_paginators_test.go index 9f8ee9066e1..e61a7597b27 100644 --- a/service/s3/handwritten_paginators_test.go +++ b/service/s3/handwritten_paginators_test.go @@ -2,159 +2,271 @@ package s3 import ( "context" - "fmt" "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/smithy-go/middleware" "testing" ) -var bucket *string -var limit int32 -var keyMarker *string -var idMarker *string - -type testListMPUMiddleware struct { - id int +type mockListObjectVersionsClient struct { + outputs []*ListObjectVersionsOutput + inputs []*ListObjectVersionsInput + t *testing.T + limit int32 } -type testListOVMiddleware struct { - id int +type mockListMultipartUploadsClient struct { + outputs []*ListMultipartUploadsOutput + inputs []*ListMultipartUploadsInput + t *testing.T + limit int32 } -func (m *testListMPUMiddleware) ID() string { - return fmt.Sprintf("mock middleware %d", m.id) +func (c *mockListObjectVersionsClient) ListObjectVersions(ctx context.Context, input *ListObjectVersionsInput, optFns ...func(*Options)) (*ListObjectVersionsOutput, error) { + if input.MaxKeys != c.limit { + c.t.Errorf("Expect page limit to be %d, got %d", c.limit, input.MaxKeys) + } + c.inputs = append(c.inputs, input) + requestCnt := len(c.inputs) + testCurRequestCnt(len(c.outputs), requestCnt, c.t) + return c.outputs[requestCnt-1], nil } -func (m *testListMPUMiddleware) HandleInitialize(ctx context.Context, input middleware.InitializeInput, next middleware.InitializeHandler) ( - output middleware.InitializeOutput, metadata middleware.Metadata, err error, -) { - params := input.Parameters.(*ListMultipartUploadsInput) - bucket = params.Bucket - limit = params.MaxUploads - keyMarker = params.KeyMarker - idMarker = params.UploadIdMarker - return middleware.InitializeOutput{Result: &ListMultipartUploadsOutput{}}, metadata, nil +func (c *mockListMultipartUploadsClient) ListMultipartUploads(ctx context.Context, input *ListMultipartUploadsInput, optFns ...func(*Options)) (*ListMultipartUploadsOutput, error) { + if input.MaxUploads != c.limit { + c.t.Errorf("Expect page limit to be %d, got %d", c.limit, input.MaxUploads) + } + c.inputs = append(c.inputs, input) + requestCnt := len(c.inputs) + testCurRequestCnt(len(c.outputs), requestCnt, c.t) + return c.outputs[requestCnt-1], nil } -func (m *testListOVMiddleware) ID() string { - return fmt.Sprintf("mock middleware %d", m.id) +type testCase struct { + bucket *string + limit int32 + requestCnt int + stopOnDuplicationToken bool } -func (m *testListOVMiddleware) HandleInitialize(ctx context.Context, input middleware.InitializeInput, next middleware.InitializeHandler) ( - output middleware.InitializeOutput, metadata middleware.Metadata, err error, -) { - params := input.Parameters.(*ListObjectVersionsInput) - bucket = params.Bucket - limit = params.MaxKeys - keyMarker = params.KeyMarker - idMarker = params.VersionIdMarker - return middleware.InitializeOutput{Result: &ListObjectVersionsOutput{}}, metadata, nil +type listOVTestCase struct { + testCase + outputs []*ListObjectVersionsOutput } -type testCase struct { - bucket *string - limit int32 - keyMarker *string - idMarker *string +type listMPUTestCase struct { + testCase + outputs []*ListMultipartUploadsOutput } -func TestListMultipartUploadsPaginator(t *testing.T) { - cases := map[string]testCase{ - "page limit 5 without marker": { - bucket: aws.String("testBucket1"), - limit: 5, +func TestListObjectVersionsPaginator(t *testing.T) { + cases := map[string]listOVTestCase{ + "page limit 5": { + testCase: testCase{ + bucket: aws.String("testBucket1"), + limit: 5, + requestCnt: 3, + }, + outputs: []*ListObjectVersionsOutput{ + &ListObjectVersionsOutput{ + NextKeyMarker: aws.String("testKey1"), + NextVersionIdMarker: aws.String("testID1"), + IsTruncated: true, + }, + &ListObjectVersionsOutput{ + NextKeyMarker: aws.String("testKey2"), + NextVersionIdMarker: aws.String("testID2"), + IsTruncated: true, + }, + &ListObjectVersionsOutput{ + NextKeyMarker: aws.String("testKey3"), + NextVersionIdMarker: aws.String("testID3"), + IsTruncated: false, + }, + }, }, - "page limit 10 with marker": { - bucket: aws.String("testBucket2"), - limit: 10, - keyMarker: aws.String("testKey1"), - idMarker: aws.String("abc"), + "page limit 10 with duplicate marker": { + testCase: testCase{ + bucket: aws.String("testBucket2"), + limit: 10, + requestCnt: 3, + stopOnDuplicationToken: true, + }, + outputs: []*ListObjectVersionsOutput{ + &ListObjectVersionsOutput{ + NextKeyMarker: aws.String("testKey1"), + NextVersionIdMarker: aws.String("testID1"), + IsTruncated: true, + }, + &ListObjectVersionsOutput{ + NextKeyMarker: aws.String("testKey2"), + NextVersionIdMarker: aws.String("testID2"), + IsTruncated: true, + }, + &ListObjectVersionsOutput{ + NextKeyMarker: aws.String("testKey2"), + NextVersionIdMarker: aws.String("testID2"), + IsTruncated: true, + }, + &ListObjectVersionsOutput{ + NextKeyMarker: aws.String("testKey3"), + NextVersionIdMarker: aws.String("testID3"), + IsTruncated: false, + }, + }, }, } for name, c := range cases { t.Run(name, func(t *testing.T) { - client := NewFromConfig(aws.Config{}) - - paginator := NewListMultipartUploadsPaginator(client, &ListMultipartUploadsInput{ - Bucket: c.bucket, - KeyMarker: c.keyMarker, - UploadIdMarker: c.idMarker, - }, func(options *ListMultipartUploadsPaginatorOptions) { + client := mockListObjectVersionsClient{ + limit: c.limit, + t: t, + outputs: c.outputs, + inputs: []*ListObjectVersionsInput{}, + } + paginator := NewListObjectVersionsPaginator(&client, &ListObjectVersionsInput{ + Bucket: c.bucket, + }, func(options *ListObjectVersionsPaginatorOptions) { options.Limit = c.limit + options.StopOnDuplicateToken = c.stopOnDuplicationToken }) - if !paginator.HasMorePages() { - t.Errorf("Expect paginator has more page, got not") - } - paginator.NextPage(context.TODO(), initializeMiddlewareFn(&testListMPUMiddleware{1})) + for paginator.HasMorePages() { + _, err := paginator.NextPage(context.TODO()) + if err != nil { + t.Errorf("error: %v", err) + } + } - testNextPageResult(c, paginator.keyMarker, paginator.uploadIDMarker, t) + inputLen := len(client.inputs) + testTotalRequests(c.requestCnt, inputLen, t) + for i := 1; i < inputLen; i++ { + if *client.inputs[i].KeyMarker != *c.outputs[i-1].NextKeyMarker { + t.Errorf("Expect next input's KeyMarker to be eaqul to %s, got %s", + *c.outputs[i-1].NextKeyMarker, *client.inputs[i].KeyMarker) + } + if *client.inputs[i].VersionIdMarker != *c.outputs[i-1].NextVersionIdMarker { + t.Errorf("Expect next input's VersionIdMarker to be eaqul to %s, got %s", + *c.outputs[i-1].NextVersionIdMarker, *client.inputs[i].VersionIdMarker) + } + } }) } } -func TestListObjectVersionsPaginator(t *testing.T) { - cases := map[string]testCase{ +func TestListMultipartUploadsPaginator(t *testing.T) { + cases := map[string]listMPUTestCase{ "page limit 5": { - bucket: aws.String("testBucket3"), - limit: 5, + testCase: testCase{ + bucket: aws.String("testBucket1"), + limit: 5, + requestCnt: 4, + }, + outputs: []*ListMultipartUploadsOutput{ + &ListMultipartUploadsOutput{ + NextKeyMarker: aws.String("testKey1"), + NextUploadIdMarker: aws.String("testID1"), + IsTruncated: true, + }, + &ListMultipartUploadsOutput{ + NextKeyMarker: aws.String("testKey2"), + NextUploadIdMarker: aws.String("testID2"), + IsTruncated: true, + }, + &ListMultipartUploadsOutput{ + NextKeyMarker: aws.String("testKey3"), + NextUploadIdMarker: aws.String("testID3"), + IsTruncated: true, + }, + &ListMultipartUploadsOutput{ + NextKeyMarker: aws.String("testKey4"), + NextUploadIdMarker: aws.String("testID4"), + IsTruncated: false, + }, + }, }, - "page limit 10 with marker": { - bucket: aws.String("testBucket4"), - limit: 10, - keyMarker: aws.String("testKey2"), - idMarker: aws.String("def"), + "page limit 10 with duplicate marker": { + testCase: testCase{ + bucket: aws.String("testBucket2"), + limit: 10, + requestCnt: 3, + stopOnDuplicationToken: true, + }, + outputs: []*ListMultipartUploadsOutput{ + &ListMultipartUploadsOutput{ + NextKeyMarker: aws.String("testKey1"), + NextUploadIdMarker: aws.String("testID1"), + IsTruncated: true, + }, + &ListMultipartUploadsOutput{ + NextKeyMarker: aws.String("testKey2"), + NextUploadIdMarker: aws.String("testID2"), + IsTruncated: true, + }, + &ListMultipartUploadsOutput{ + NextKeyMarker: aws.String("testKey2"), + NextUploadIdMarker: aws.String("testID2"), + IsTruncated: true, + }, + &ListMultipartUploadsOutput{ + NextKeyMarker: aws.String("testKey4"), + NextUploadIdMarker: aws.String("testID4"), + IsTruncated: false, + }, + &ListMultipartUploadsOutput{ + NextKeyMarker: aws.String("testKey5"), + NextUploadIdMarker: aws.String("testID5"), + IsTruncated: false, + }, + }, }, } for name, c := range cases { t.Run(name, func(t *testing.T) { - client := NewFromConfig(aws.Config{}) - - paginator := NewListObjectVersionsPaginator(client, &ListObjectVersionsInput{ - Bucket: c.bucket, - KeyMarker: c.keyMarker, - VersionIdMarker: c.idMarker, - }, func(options *ListObjectVersionsPaginatorOptions) { + client := mockListMultipartUploadsClient{ + outputs: c.outputs, + inputs: []*ListMultipartUploadsInput{}, + t: t, + limit: c.limit, + } + paginator := NewListMultipartUploadsPaginator(&client, &ListMultipartUploadsInput{ + Bucket: c.bucket, + }, func(options *ListMultipartUploadsPaginatorOptions) { options.Limit = c.limit + options.StopOnDuplicateToken = c.stopOnDuplicationToken }) - if !paginator.HasMorePages() { - t.Errorf("Expect paginator has more page, got not") - } - paginator.NextPage(context.TODO(), initializeMiddlewareFn(&testListOVMiddleware{1})) + for paginator.HasMorePages() { + _, err := paginator.NextPage(context.TODO()) + if err != nil { + t.Errorf("error: %v", err) + } + } - testNextPageResult(c, paginator.keyMarker, paginator.versionIDMarker, t) + inputLen := len(client.inputs) + testTotalRequests(c.requestCnt, inputLen, t) + for i := 1; i < inputLen; i++ { + if *client.inputs[i].KeyMarker != *c.outputs[i-1].NextKeyMarker { + t.Errorf("Expect next input's KeyMarker to be eaqul to %s, got %s", + *c.outputs[i-1].NextKeyMarker, *client.inputs[i].KeyMarker) + } + if *client.inputs[i].UploadIdMarker != *c.outputs[i-1].NextUploadIdMarker { + t.Errorf("Expect next input's UploadIdMarker to be eaqul to %s, got %s", + *c.outputs[i-1].NextUploadIdMarker, *client.inputs[i].UploadIdMarker) + } + } }) } } -// insert middleware at the beginning of initialize step to see if page limit -// can be passed to API call's stack input -func initializeMiddlewareFn(initializeMiddleware middleware.InitializeMiddleware) func(*Options) { - return func(options *Options) { - options.APIOptions = append(options.APIOptions, func(stack *middleware.Stack) error { - return stack.Initialize.Add(initializeMiddleware, middleware.Before) - }) +func testTotalRequests(expect, actual int, t *testing.T) { + if actual != expect { + t.Errorf("Expect total request number to be %d, got %d", expect, actual) } } -// unit test can not control client API call's output, so just check marker's default nil value -func testNextPageResult(c testCase, pKeyMarker *string, pIDMarker *string, t *testing.T) { - if c.limit != limit { - t.Errorf("Expect page limit to be %d, got %d", c.limit, limit) - } - if *c.bucket != *bucket { - t.Errorf("Expect bucket to be %s, got %s", *c.bucket, *bucket) - } - if c.keyMarker != nil && *c.keyMarker != *keyMarker { - t.Errorf("Expect keyMarker to be %s, got %s", *c.keyMarker, *keyMarker) - } - if c.idMarker != nil && *c.idMarker != *idMarker { - t.Errorf("Expect idMarker to be %s, got %s", *c.idMarker, *idMarker) - } - if pKeyMarker != nil || pIDMarker != nil { - t.Errorf("Expect paginator keyMarker and idMarker to be nil, got %s and %s", *pKeyMarker, *pIDMarker) +func testCurRequestCnt(expectMax, actual int, t *testing.T) { + if actual > expectMax { + t.Errorf("Paginator calls client more than expected %d times", expectMax) } } From 252d004b5b86c5766c4b4cb8eb7682de7b9267f6 Mon Sep 17 00:00:00 2001 From: Tianyi Wang Date: Tue, 18 Apr 2023 21:47:52 -0400 Subject: [PATCH 11/11] modify s3 paginator unit test --- service/s3/handwritten_paginators_test.go | 39 ++++++++++++++--------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/service/s3/handwritten_paginators_test.go b/service/s3/handwritten_paginators_test.go index e61a7597b27..6ded64edd2d 100644 --- a/service/s3/handwritten_paginators_test.go +++ b/service/s3/handwritten_paginators_test.go @@ -10,33 +10,25 @@ type mockListObjectVersionsClient struct { outputs []*ListObjectVersionsOutput inputs []*ListObjectVersionsInput t *testing.T - limit int32 } type mockListMultipartUploadsClient struct { outputs []*ListMultipartUploadsOutput inputs []*ListMultipartUploadsInput t *testing.T - limit int32 } func (c *mockListObjectVersionsClient) ListObjectVersions(ctx context.Context, input *ListObjectVersionsInput, optFns ...func(*Options)) (*ListObjectVersionsOutput, error) { - if input.MaxKeys != c.limit { - c.t.Errorf("Expect page limit to be %d, got %d", c.limit, input.MaxKeys) - } c.inputs = append(c.inputs, input) requestCnt := len(c.inputs) - testCurRequestCnt(len(c.outputs), requestCnt, c.t) + testCurRequest(len(c.outputs), requestCnt, c.outputs[requestCnt-1].MaxKeys, input.MaxKeys, c.t) return c.outputs[requestCnt-1], nil } func (c *mockListMultipartUploadsClient) ListMultipartUploads(ctx context.Context, input *ListMultipartUploadsInput, optFns ...func(*Options)) (*ListMultipartUploadsOutput, error) { - if input.MaxUploads != c.limit { - c.t.Errorf("Expect page limit to be %d, got %d", c.limit, input.MaxUploads) - } c.inputs = append(c.inputs, input) requestCnt := len(c.inputs) - testCurRequestCnt(len(c.outputs), requestCnt, c.t) + testCurRequest(len(c.outputs), requestCnt, c.outputs[requestCnt-1].MaxUploads, input.MaxUploads, c.t) return c.outputs[requestCnt-1], nil } @@ -69,16 +61,19 @@ func TestListObjectVersionsPaginator(t *testing.T) { &ListObjectVersionsOutput{ NextKeyMarker: aws.String("testKey1"), NextVersionIdMarker: aws.String("testID1"), + MaxKeys: 5, IsTruncated: true, }, &ListObjectVersionsOutput{ NextKeyMarker: aws.String("testKey2"), NextVersionIdMarker: aws.String("testID2"), + MaxKeys: 5, IsTruncated: true, }, &ListObjectVersionsOutput{ NextKeyMarker: aws.String("testKey3"), NextVersionIdMarker: aws.String("testID3"), + MaxKeys: 5, IsTruncated: false, }, }, @@ -94,21 +89,25 @@ func TestListObjectVersionsPaginator(t *testing.T) { &ListObjectVersionsOutput{ NextKeyMarker: aws.String("testKey1"), NextVersionIdMarker: aws.String("testID1"), + MaxKeys: 10, IsTruncated: true, }, &ListObjectVersionsOutput{ NextKeyMarker: aws.String("testKey2"), NextVersionIdMarker: aws.String("testID2"), + MaxKeys: 10, IsTruncated: true, }, &ListObjectVersionsOutput{ NextKeyMarker: aws.String("testKey2"), NextVersionIdMarker: aws.String("testID2"), + MaxKeys: 10, IsTruncated: true, }, &ListObjectVersionsOutput{ NextKeyMarker: aws.String("testKey3"), NextVersionIdMarker: aws.String("testID3"), + MaxKeys: 10, IsTruncated: false, }, }, @@ -118,7 +117,6 @@ func TestListObjectVersionsPaginator(t *testing.T) { for name, c := range cases { t.Run(name, func(t *testing.T) { client := mockListObjectVersionsClient{ - limit: c.limit, t: t, outputs: c.outputs, inputs: []*ListObjectVersionsInput{}, @@ -165,21 +163,25 @@ func TestListMultipartUploadsPaginator(t *testing.T) { &ListMultipartUploadsOutput{ NextKeyMarker: aws.String("testKey1"), NextUploadIdMarker: aws.String("testID1"), + MaxUploads: 5, IsTruncated: true, }, &ListMultipartUploadsOutput{ NextKeyMarker: aws.String("testKey2"), NextUploadIdMarker: aws.String("testID2"), + MaxUploads: 5, IsTruncated: true, }, &ListMultipartUploadsOutput{ NextKeyMarker: aws.String("testKey3"), NextUploadIdMarker: aws.String("testID3"), + MaxUploads: 5, IsTruncated: true, }, &ListMultipartUploadsOutput{ NextKeyMarker: aws.String("testKey4"), NextUploadIdMarker: aws.String("testID4"), + MaxUploads: 5, IsTruncated: false, }, }, @@ -195,26 +197,31 @@ func TestListMultipartUploadsPaginator(t *testing.T) { &ListMultipartUploadsOutput{ NextKeyMarker: aws.String("testKey1"), NextUploadIdMarker: aws.String("testID1"), + MaxUploads: 10, IsTruncated: true, }, &ListMultipartUploadsOutput{ NextKeyMarker: aws.String("testKey2"), NextUploadIdMarker: aws.String("testID2"), + MaxUploads: 10, IsTruncated: true, }, &ListMultipartUploadsOutput{ NextKeyMarker: aws.String("testKey2"), NextUploadIdMarker: aws.String("testID2"), + MaxUploads: 10, IsTruncated: true, }, &ListMultipartUploadsOutput{ NextKeyMarker: aws.String("testKey4"), NextUploadIdMarker: aws.String("testID4"), + MaxUploads: 10, IsTruncated: false, }, &ListMultipartUploadsOutput{ NextKeyMarker: aws.String("testKey5"), NextUploadIdMarker: aws.String("testID5"), + MaxUploads: 10, IsTruncated: false, }, }, @@ -227,7 +234,6 @@ func TestListMultipartUploadsPaginator(t *testing.T) { outputs: c.outputs, inputs: []*ListMultipartUploadsInput{}, t: t, - limit: c.limit, } paginator := NewListMultipartUploadsPaginator(&client, &ListMultipartUploadsInput{ Bucket: c.bucket, @@ -265,8 +271,11 @@ func testTotalRequests(expect, actual int, t *testing.T) { } } -func testCurRequestCnt(expectMax, actual int, t *testing.T) { - if actual > expectMax { - t.Errorf("Paginator calls client more than expected %d times", expectMax) +func testCurRequest(maxReqCnt, actualReqCnt int, expectLimit, actualLimit int32, t *testing.T) { + if actualReqCnt > maxReqCnt { + t.Errorf("Paginator calls client more than expected %d times", maxReqCnt) + } + if expectLimit != actualLimit { + t.Errorf("Expect page limit to be %d, got %d", expectLimit, actualLimit) } }