From ce609d816327b6c00668e95c4370ae7e5f22b328 Mon Sep 17 00:00:00 2001 From: Mark McDonnell Date: Wed, 19 Jul 2023 18:01:52 +0100 Subject: [PATCH] fix(flags): ensure ListServices call is paginated (#976) --- pkg/cmd/flags.go | 16 +++-- pkg/commands/backend/backend_test.go | 56 ++--------------- pkg/commands/service/service_test.go | 94 +++------------------------- pkg/testutil/paginator.go | 88 ++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 142 deletions(-) create mode 100644 pkg/testutil/paginator.go diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index ec191be49..f4231c8fa 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -12,12 +12,13 @@ import ( "strconv" "strings" + "github.com/fastly/go-fastly/v8/fastly" + "github.com/fastly/kingpin" + "github.com/fastly/cli/pkg/api" "github.com/fastly/cli/pkg/env" fsterr "github.com/fastly/cli/pkg/errors" "github.com/fastly/cli/pkg/text" - "github.com/fastly/go-fastly/v8/fastly" - "github.com/fastly/kingpin" ) var ( @@ -155,9 +156,14 @@ type OptionalServiceNameID struct { // Parse returns a service ID based off the given service name. func (sv *OptionalServiceNameID) Parse(client api.Interface) (serviceID string, err error) { - services, err := client.ListServices(&fastly.ListServicesInput{}) - if err != nil { - return serviceID, fmt.Errorf("error listing services: %w", err) + paginator := client.NewListServicesPaginator(&fastly.ListServicesInput{}) + var services []*fastly.Service + for paginator.HasNext() { + data, err := paginator.GetNext() + if err != nil { + return serviceID, fmt.Errorf("error listing services: %w", err) + } + services = append(services, data...) } for _, s := range services { if s.Name == sv.Value { diff --git a/pkg/commands/backend/backend_test.go b/pkg/commands/backend/backend_test.go index b9d5436fd..de161b2ff 100644 --- a/pkg/commands/backend/backend_test.go +++ b/pkg/commands/backend/backend_test.go @@ -6,11 +6,12 @@ import ( "strings" "testing" + "github.com/fastly/go-fastly/v8/fastly" + "github.com/fastly/cli/pkg/app" fsterr "github.com/fastly/cli/pkg/errors" "github.com/fastly/cli/pkg/mock" "github.com/fastly/cli/pkg/testutil" - "github.com/fastly/go-fastly/v8/fastly" ) func TestBackendCreate(t *testing.T) { @@ -81,8 +82,10 @@ func TestBackendCreate(t *testing.T) { { Args: args("backend create --service-name Foo --version 1 --address 127.0.0.1 --name www.test.com --autoclone"), API: mock.API{ - ListVersionsFn: testutil.ListVersions, - ListServicesFn: listServicesOK, + ListVersionsFn: testutil.ListVersions, + NewListServicesPaginatorFn: func(i *fastly.ListServicesInput) fastly.PaginatorServices { + return &testutil.ServicesPaginator{} + }, CloneVersionFn: testutil.CloneVersionResult(4), CreateBackendFn: createBackendOK, }, @@ -591,50 +594,3 @@ func deleteBackendOK(i *fastly.DeleteBackendInput) error { func deleteBackendError(i *fastly.DeleteBackendInput) error { return errTest } - -func listServicesOK(i *fastly.ListServicesInput) ([]*fastly.Service, error) { - return []*fastly.Service{ - { - ID: "123", - Name: "Foo", - Type: "wasm", - CustomerID: "mycustomerid", - ActiveVersion: 2, - UpdatedAt: testutil.MustParseTimeRFC3339("2010-11-15T19:01:02Z"), - Versions: []*fastly.Version{ - { - Number: 1, - Comment: "a", - ServiceID: "b", - CreatedAt: testutil.MustParseTimeRFC3339("2001-02-03T04:05:06Z"), - UpdatedAt: testutil.MustParseTimeRFC3339("2001-02-04T04:05:06Z"), - DeletedAt: testutil.MustParseTimeRFC3339("2001-02-05T04:05:06Z"), - }, - { - Number: 2, - Comment: "c", - ServiceID: "d", - Active: true, - Deployed: true, - CreatedAt: testutil.MustParseTimeRFC3339("2001-03-03T04:05:06Z"), - UpdatedAt: testutil.MustParseTimeRFC3339("2001-03-04T04:05:06Z"), - }, - }, - }, - { - ID: "456", - Name: "Bar", - Type: "wasm", - CustomerID: "mycustomerid", - ActiveVersion: 1, - UpdatedAt: testutil.MustParseTimeRFC3339("2015-03-14T12:59:59Z"), - }, - { - ID: "789", - Name: "Baz", - Type: "vcl", - CustomerID: "mycustomerid", - ActiveVersion: 1, - }, - }, nil -} diff --git a/pkg/commands/service/service_test.go b/pkg/commands/service/service_test.go index 9448d2ec2..ea43d81bc 100644 --- a/pkg/commands/service/service_test.go +++ b/pkg/commands/service/service_test.go @@ -9,11 +9,12 @@ import ( "strings" "testing" + "github.com/fastly/go-fastly/v8/fastly" + "github.com/fastly/cli/pkg/app" "github.com/fastly/cli/pkg/manifest" "github.com/fastly/cli/pkg/mock" "github.com/fastly/cli/pkg/testutil" - "github.com/fastly/go-fastly/v8/fastly" ) func TestServiceCreate(t *testing.T) { @@ -68,87 +69,6 @@ func TestServiceCreate(t *testing.T) { } } -type mockServicesPaginator struct { - count int - maxPages int - numOfPages int - requestedPage int - returnErr bool -} - -func (p *mockServicesPaginator) HasNext() bool { - if p.count > p.maxPages { - return false - } - p.count++ - return true -} - -func (p mockServicesPaginator) Remaining() int { - return 1 -} - -func (p *mockServicesPaginator) GetNext() (ss []*fastly.Service, err error) { - if p.returnErr { - err = testutil.Err - } - pageOne := fastly.Service{ - ID: "123", - Name: "Foo", - Type: "wasm", - CustomerID: "mycustomerid", - ActiveVersion: 2, - UpdatedAt: testutil.MustParseTimeRFC3339("2010-11-15T19:01:02Z"), - Versions: []*fastly.Version{ - { - Number: 1, - Comment: "a", - ServiceID: "b", - CreatedAt: testutil.MustParseTimeRFC3339("2001-02-03T04:05:06Z"), - UpdatedAt: testutil.MustParseTimeRFC3339("2001-02-04T04:05:06Z"), - DeletedAt: testutil.MustParseTimeRFC3339("2001-02-05T04:05:06Z"), - }, - { - Number: 2, - Comment: "c", - ServiceID: "d", - Active: true, - Deployed: true, - CreatedAt: testutil.MustParseTimeRFC3339("2001-03-03T04:05:06Z"), - UpdatedAt: testutil.MustParseTimeRFC3339("2001-03-04T04:05:06Z"), - }, - }, - } - pageTwo := fastly.Service{ - ID: "456", - Name: "Bar", - Type: "wasm", - CustomerID: "mycustomerid", - ActiveVersion: 1, - UpdatedAt: testutil.MustParseTimeRFC3339("2015-03-14T12:59:59Z"), - } - pageThree := fastly.Service{ - ID: "789", - Name: "Baz", - Type: "vcl", - CustomerID: "mycustomerid", - ActiveVersion: 1, - } - if p.count == 1 { - ss = append(ss, &pageOne) - } - if p.count == 2 { - ss = append(ss, &pageTwo) - } - if p.count == 3 { - ss = append(ss, &pageThree) - } - if p.requestedPage > 0 && p.numOfPages == 1 { - p.count = p.maxPages + 1 // forces only one result to be displayed - } - return ss, err -} - func TestServiceList(t *testing.T) { args := testutil.Args scenarios := []struct { @@ -160,7 +80,7 @@ func TestServiceList(t *testing.T) { { api: mock.API{ NewListServicesPaginatorFn: func(i *fastly.ListServicesInput) fastly.PaginatorServices { - return &mockServicesPaginator{returnErr: true} + return &testutil.ServicesPaginator{ReturnErr: true} }, }, args: args("service list"), @@ -171,7 +91,7 @@ func TestServiceList(t *testing.T) { { api: mock.API{ NewListServicesPaginatorFn: func(i *fastly.ListServicesInput) fastly.PaginatorServices { - return &mockServicesPaginator{numOfPages: i.PerPage, maxPages: 3} + return &testutil.ServicesPaginator{NumOfPages: i.PerPage, MaxPages: 3} }, }, args: args("service list --per-page 1"), @@ -182,7 +102,7 @@ func TestServiceList(t *testing.T) { { api: mock.API{ NewListServicesPaginatorFn: func(i *fastly.ListServicesInput) fastly.PaginatorServices { - return &mockServicesPaginator{count: i.Page - 1, requestedPage: i.Page, numOfPages: i.PerPage, maxPages: 3} + return &testutil.ServicesPaginator{Count: i.Page - 1, RequestedPage: i.Page, NumOfPages: i.PerPage, MaxPages: 3} }, }, args: args("service list --page 1 --per-page 1"), @@ -193,7 +113,7 @@ func TestServiceList(t *testing.T) { { api: mock.API{ NewListServicesPaginatorFn: func(i *fastly.ListServicesInput) fastly.PaginatorServices { - return &mockServicesPaginator{count: i.Page - 1, requestedPage: i.Page, numOfPages: i.PerPage, maxPages: 3} + return &testutil.ServicesPaginator{Count: i.Page - 1, RequestedPage: i.Page, NumOfPages: i.PerPage, MaxPages: 3} }, }, args: args("service list --page 2 --per-page 1"), @@ -202,7 +122,7 @@ func TestServiceList(t *testing.T) { { api: mock.API{ NewListServicesPaginatorFn: func(i *fastly.ListServicesInput) fastly.PaginatorServices { - return &mockServicesPaginator{maxPages: 3} + return &testutil.ServicesPaginator{MaxPages: 3} }, }, args: args("service list --verbose"), diff --git a/pkg/testutil/paginator.go b/pkg/testutil/paginator.go new file mode 100644 index 000000000..9281a2b4f --- /dev/null +++ b/pkg/testutil/paginator.go @@ -0,0 +1,88 @@ +package testutil + +import "github.com/fastly/go-fastly/v8/fastly" + +// ServicesPaginator mocks the behaviour of a paginator for services. +type ServicesPaginator struct { + Count int + MaxPages int + NumOfPages int + RequestedPage int + ReturnErr bool +} + +// HasNext indicates if there is another page of data. +func (p *ServicesPaginator) HasNext() bool { + if p.Count > p.MaxPages { + return false + } + p.Count++ + return true +} + +// Remaining returns the count of remaining pages. +func (p ServicesPaginator) Remaining() int { + return 1 +} + +// GetNext returns the next page of data. +func (p *ServicesPaginator) GetNext() (ss []*fastly.Service, err error) { + if p.ReturnErr { + err = Err + } + pageOne := fastly.Service{ + ID: "123", + Name: "Foo", + Type: "wasm", + CustomerID: "mycustomerid", + ActiveVersion: 2, + UpdatedAt: MustParseTimeRFC3339("2010-11-15T19:01:02Z"), + Versions: []*fastly.Version{ + { + Number: 1, + Comment: "a", + ServiceID: "b", + CreatedAt: MustParseTimeRFC3339("2001-02-03T04:05:06Z"), + UpdatedAt: MustParseTimeRFC3339("2001-02-04T04:05:06Z"), + DeletedAt: MustParseTimeRFC3339("2001-02-05T04:05:06Z"), + }, + { + Number: 2, + Comment: "c", + ServiceID: "d", + Active: true, + Deployed: true, + CreatedAt: MustParseTimeRFC3339("2001-03-03T04:05:06Z"), + UpdatedAt: MustParseTimeRFC3339("2001-03-04T04:05:06Z"), + }, + }, + } + pageTwo := fastly.Service{ + ID: "456", + Name: "Bar", + Type: "wasm", + CustomerID: "mycustomerid", + ActiveVersion: 1, + UpdatedAt: MustParseTimeRFC3339("2015-03-14T12:59:59Z"), + } + pageThree := fastly.Service{ + ID: "789", + Name: "Baz", + Type: "vcl", + CustomerID: "mycustomerid", + ActiveVersion: 1, + } + if p.Count == 1 { + ss = append(ss, &pageOne) + } + if p.Count == 2 { + ss = append(ss, &pageTwo) + } + if p.Count == 3 { + ss = append(ss, &pageThree) + } + if p.RequestedPage > 0 && p.NumOfPages == 1 { + p.Count = p.MaxPages + 1 // forces only one result to be displayed + } + return ss, err +}