Skip to content

Commit

Permalink
fix(ui): unresponsive agent pool page (#701)
Browse files Browse the repository at this point in the history
Fixes #698
Fixes #661
  • Loading branch information
leg100 authored Oct 30, 2024
1 parent c8cdd4a commit c0853f5
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 0 deletions.
1 change: 1 addition & 0 deletions internal/agent/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ func (h *webHandlers) getAgentPool(w http.ResponseWriter, r *http.Request) {
// sets documented above.
allWorkspaces, err := resource.ListAll(func(opts resource.PageOptions) (*resource.Page[*workspacepkg.Workspace], error) {
return h.workspaces.List(r.Context(), workspacepkg.ListOptions{
PageOptions: opts,
Organization: &pool.Organization,
})
})
Expand Down
9 changes: 9 additions & 0 deletions internal/resource/pagination.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package resource

import (
"errors"
"math"

"github.com/jackc/pgx/v5/pgtype"
Expand Down Expand Up @@ -71,12 +72,16 @@ func NewPage[T any](resources []T, opts PageOptions, count *int64) *Page[T] {
}
}

var ErrInfinitePaginationDetected = errors.New("infinite pagination detected")

// ListAll is a helper for retrieving all pages. The provided fn should perform
// an operation that retrieves a page at a time.
func ListAll[T any](fn func(PageOptions) (*Page[T], error)) ([]T, error) {
var (
opts PageOptions
all []T
// keep track of the last NextPage to prevent an infinite loop.
lastNextPage int
)
for {
page, err := fn(opts)
Expand All @@ -91,7 +96,11 @@ func ListAll[T any](fn func(PageOptions) (*Page[T], error)) ([]T, error) {
if page.NextPage == nil {
break
}
if *page.NextPage == lastNextPage {
return nil, ErrInfinitePaginationDetected
}
opts.PageNumber = *page.NextPage
lastNextPage = *page.NextPage
}
return all, nil
}
Expand Down
39 changes: 39 additions & 0 deletions internal/resource/pagination_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/leg100/otf/internal"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestPagination(t *testing.T) {
Expand Down Expand Up @@ -154,3 +155,41 @@ func TestNewPage(t *testing.T) {
})
}
}

func TestListAll(t *testing.T) {
type foo int

var page int
got, err := ListAll(func(opts PageOptions) (*Page[foo], error) {
if opts.PageNumber == 10 {
return &Page[foo]{
Pagination: &Pagination{},
}, nil
}
page++
return &Page[foo]{
Items: []foo{foo(page)},
Pagination: &Pagination{
NextPage: internal.Int(page),
},
}, nil
})
require.NoError(t, err)
assert.Equal(t, []foo{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, got)
}

func TestListAll_infinite_pagination(t *testing.T) {
type foo int

// This demonstrates an incorrectly implemented func being passed to
// ListAll, which retrieves the same page ad infinitum.
_, err := ListAll(func(opts PageOptions) (*Page[foo], error) {
return &Page[foo]{
Items: []foo{0},
Pagination: &Pagination{
NextPage: internal.Int(1),
},
}, nil
})
assert.Equal(t, ErrInfinitePaginationDetected, err)
}

0 comments on commit c0853f5

Please sign in to comment.