From 3cc58813f088fac3b9d008372052694ae1f75914 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Sat, 8 Sep 2018 11:12:03 +0200 Subject: [PATCH] backend/remote: use a search query and use pagination To prevent making unnecessary heavy calls to the backend, we should use a search query to limit the result. But even if we use a search query, we should still use the pagination details to make sure we retrieved all items. --- backend/remote/backend.go | 52 +++++++++++++------ backend/remote/backend_mock.go | 29 +++++++++-- .../test-fixtures/plan-scaleout/main.tf | 10 ---- 3 files changed, 62 insertions(+), 29 deletions(-) delete mode 100644 backend/remote/test-fixtures/plan-scaleout/main.tf diff --git a/backend/remote/backend.go b/backend/remote/backend.go index 5c00edd2f..e36ca8ef0 100644 --- a/backend/remote/backend.go +++ b/backend/remote/backend.go @@ -251,9 +251,10 @@ func (b *Remote) State(workspace string) (state.State, error) { } // Configure the remote workspace name. - if workspace == backend.DefaultStateName { + switch { + case workspace == backend.DefaultStateName: workspace = b.workspace - } else if b.prefix != "" && !strings.HasPrefix(workspace, b.prefix) { + case b.prefix != "" && !strings.HasPrefix(workspace, b.prefix): workspace = b.prefix + workspace } @@ -293,9 +294,10 @@ func (b *Remote) DeleteState(workspace string) error { } // Configure the remote workspace name. - if workspace == backend.DefaultStateName { + switch { + case workspace == backend.DefaultStateName: workspace = b.workspace - } else if b.prefix != "" && !strings.HasPrefix(workspace, b.prefix) { + case b.prefix != "" && !strings.HasPrefix(workspace, b.prefix): workspace = b.prefix + workspace } @@ -336,20 +338,39 @@ func (b *Remote) states() ([]string, error) { } options := tfe.WorkspaceListOptions{} - wl, err := b.client.Workspaces.List(context.Background(), b.organization, options) - if err != nil { - return nil, err + switch { + case b.workspace != "": + options.Search = tfe.String(b.workspace) + case b.prefix != "": + options.Search = tfe.String(b.prefix) } + // Create a slice to contain all the names. var names []string - for _, w := range wl.Items { - if b.workspace != "" && w.Name == b.workspace { - names = append(names, backend.DefaultStateName) - continue + + for { + wl, err := b.client.Workspaces.List(context.Background(), b.organization, options) + if err != nil { + return nil, err } - if b.prefix != "" && strings.HasPrefix(w.Name, b.prefix) { - names = append(names, strings.TrimPrefix(w.Name, b.prefix)) + + for _, w := range wl.Items { + if b.workspace != "" && w.Name == b.workspace { + names = append(names, backend.DefaultStateName) + continue + } + if b.prefix != "" && strings.HasPrefix(w.Name, b.prefix) { + names = append(names, strings.TrimPrefix(w.Name, b.prefix)) + } } + + // Exit the loop when we've seen all pages. + if wl.CurrentPage == wl.TotalPages { + break + } + + // Update the page number to get the next page. + options.PageNumber = wl.NextPage } // Sort the result so we have consistent output. @@ -361,9 +382,10 @@ func (b *Remote) states() ([]string, error) { // Operation implements backend.Enhanced func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend.RunningOperation, error) { // Configure the remote workspace name. - if op.Workspace == backend.DefaultStateName { + switch { + case op.Workspace == backend.DefaultStateName: op.Workspace = b.workspace - } else if b.prefix != "" && !strings.HasPrefix(op.Workspace, b.prefix) { + case b.prefix != "" && !strings.HasPrefix(op.Workspace, b.prefix): op.Workspace = b.prefix + op.Workspace } diff --git a/backend/remote/backend_mock.go b/backend/remote/backend_mock.go index 673b5c81c..c680eb770 100644 --- a/backend/remote/backend_mock.go +++ b/backend/remote/backend_mock.go @@ -413,17 +413,38 @@ func newMockWorkspaces(client *mockClient) *mockWorkspaces { } func (m *mockWorkspaces) List(ctx context.Context, organization string, options tfe.WorkspaceListOptions) (*tfe.WorkspaceList, error) { + dummyWorkspaces := 10 wl := &tfe.WorkspaceList{} + + // We return dummy workspaces for the first page to test pagination. + if options.PageNumber <= 1 { + for i := 0; i < dummyWorkspaces; i++ { + wl.Items = append(wl.Items, &tfe.Workspace{ + ID: generateID("ws-"), + Name: fmt.Sprintf("dummy-workspace-%d", i), + }) + } + + wl.Pagination = &tfe.Pagination{ + CurrentPage: 1, + NextPage: 2, + TotalPages: 2, + TotalCount: len(wl.Items) + len(m.workspaceIDs), + } + + return wl, nil + } + + // The second page will return any actual results. for _, w := range m.workspaceIDs { wl.Items = append(wl.Items, w) } wl.Pagination = &tfe.Pagination{ - CurrentPage: 1, - NextPage: 1, + CurrentPage: 2, PreviousPage: 1, - TotalPages: 1, - TotalCount: len(wl.Items), + TotalPages: 2, + TotalCount: len(wl.Items) + dummyWorkspaces, } return wl, nil diff --git a/backend/remote/test-fixtures/plan-scaleout/main.tf b/backend/remote/test-fixtures/plan-scaleout/main.tf deleted file mode 100644 index 4067af592..000000000 --- a/backend/remote/test-fixtures/plan-scaleout/main.tf +++ /dev/null @@ -1,10 +0,0 @@ -resource "test_instance" "foo" { - count = 3 - ami = "bar" - - # This is here because at some point it caused a test failure - network_interface { - device_index = 0 - description = "Main network interface" - } -}