From 8175086be7ceebbede661d6438d144d8a8147581 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst-Satzkorn Date: Fri, 15 Dec 2023 17:21:25 -0800 Subject: [PATCH] internal/pagination: avoid import cycle The iam package is very foundational, and now depends on the pagination package. Thus, the pagination package must not import any packages that may import iam, such as the plugin package. Move the plugin helpers to a subdirectory to avoid this import cycle. --- .../{ => plugin}/pagination_plugin.go | 120 ++++++++++++++++-- .../{ => plugin}/pagination_plugin_test.go | 55 +++++++- .../{ => plugin}/pagination_plugins.go | 23 ++-- .../{ => plugin}/pagination_plugins_test.go | 15 ++- 4 files changed, 177 insertions(+), 36 deletions(-) rename internal/pagination/{ => plugin}/pagination_plugin.go (80%) rename internal/pagination/{ => plugin}/pagination_plugin_test.go (98%) rename internal/pagination/{ => plugin}/pagination_plugins.go (95%) rename internal/pagination/{ => plugin}/pagination_plugins_test.go (99%) diff --git a/internal/pagination/pagination_plugin.go b/internal/pagination/plugin/pagination_plugin.go similarity index 80% rename from internal/pagination/pagination_plugin.go rename to internal/pagination/plugin/pagination_plugin.go index 25933316e9..47d450b691 100644 --- a/internal/pagination/pagination_plugin.go +++ b/internal/pagination/plugin/pagination_plugin.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: BUSL-1.1 -package pagination +package plugin import ( "context" @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/boundary/internal/boundary" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/listtoken" + "github.com/hashicorp/boundary/internal/pagination" "github.com/hashicorp/boundary/internal/plugin" ) @@ -38,8 +39,8 @@ func ListPlugin[T boundary.Resource]( pageSize int, filterItemFn ListPluginFilterFunc[T], listItemsFn ListPluginItemsFunc[T], - estimatedCountFn EstimatedCountFunc, -) (*ListResponse[T], *plugin.Plugin, error) { + estimatedCountFn pagination.EstimatedCountFunc, +) (*pagination.ListResponse[T], *plugin.Plugin, error) { const op = "pagination.ListPlugin" switch { @@ -84,9 +85,9 @@ func ListPluginPage[T boundary.Resource]( pageSize int, filterItemFn ListPluginFilterFunc[T], listItemsFn ListPluginItemsFunc[T], - estimatedCountFn EstimatedCountFunc, + estimatedCountFn pagination.EstimatedCountFunc, tok *listtoken.Token, -) (*ListResponse[T], *plugin.Plugin, error) { +) (*pagination.ListResponse[T], *plugin.Plugin, error) { const op = "pagination.ListPluginPage" switch { @@ -137,10 +138,10 @@ func ListPluginRefresh[T boundary.Resource]( pageSize int, filterItemFn ListPluginFilterFunc[T], listItemsFn ListPluginItemsFunc[T], - estimatedCountFn EstimatedCountFunc, - listDeletedIDsFn ListDeletedIDsFunc, + estimatedCountFn pagination.EstimatedCountFunc, + listDeletedIDsFn pagination.ListDeletedIDsFunc, tok *listtoken.Token, -) (*ListResponse[T], *plugin.Plugin, error) { +) (*pagination.ListResponse[T], *plugin.Plugin, error) { const op = "pagination.ListPluginRefresh" switch { @@ -199,10 +200,10 @@ func ListPluginRefreshPage[T boundary.Resource]( pageSize int, filterItemFn ListPluginFilterFunc[T], listItemsFn ListPluginItemsFunc[T], - estimatedCountFn EstimatedCountFunc, - listDeletedIDsFn ListDeletedIDsFunc, + estimatedCountFn pagination.EstimatedCountFunc, + listDeletedIDsFn pagination.ListDeletedIDsFunc, tok *listtoken.Token, -) (*ListResponse[T], *plugin.Plugin, error) { +) (*pagination.ListResponse[T], *plugin.Plugin, error) { const op = "pagination.ListPluginRefreshPage" switch { @@ -305,3 +306,100 @@ dbLoop: return items, plg, completeListing, firstListTime, nil } + +func buildListResp[T boundary.Resource]( + ctx context.Context, + grantsHash []byte, + items []T, + completeListing bool, + listTime time.Time, + estimatedCountFn pagination.EstimatedCountFunc, +) (*pagination.ListResponse[T], error) { + resp := &pagination.ListResponse[T]{ + Items: items, + CompleteListing: completeListing, + EstimatedItemCount: len(items), + } + + var err error + if len(items) > 0 { + lastItem := items[len(items)-1] + + if completeListing { + // If this is the only page in the pagination, create a + // start refresh token so subsequent requests are informed + // that they need to start a new refresh phase. + resp.ListToken, err = listtoken.NewStartRefresh( + ctx, + listTime, // Use list time as the create time of the token + lastItem.GetResourceType(), + grantsHash, + listTime, // Use list time as the starting point for listing deleted ids + listTime, // Use list time as the lower bound for subsequent refresh + ) + if err != nil { + return nil, err + } + } else { + resp.ListToken, err = listtoken.NewPagination( + ctx, + listTime, // Use list time as the create time of the token + lastItem.GetResourceType(), + grantsHash, + lastItem.GetPublicId(), + lastItem.GetCreateTime().AsTime(), + ) + if err != nil { + return nil, err + } + } + } + if !completeListing { + // If this was not a complete listing, get an estimate + // of the total items from the DB. + var err error + resp.EstimatedItemCount, err = estimatedCountFn(ctx) + if err != nil { + return nil, err + } + } + return resp, err +} + +func buildListPageResp[T boundary.Resource]( + ctx context.Context, + completeListing bool, + deletedIds []string, + deletedIdsTime time.Time, + items []T, + listTime time.Time, + tok *listtoken.Token, + estimatedCountFn pagination.EstimatedCountFunc, +) (*pagination.ListResponse[T], error) { + resp := &pagination.ListResponse[T]{ + Items: items, + CompleteListing: completeListing, + ListToken: tok, + DeletedIds: deletedIds, + } + + var err error + resp.EstimatedItemCount, err = estimatedCountFn(ctx) + if err != nil { + return nil, err + } + var lastItem boundary.Resource + if len(items) > 0 { + lastItem = items[len(items)-1] + } + if err := resp.ListToken.Transition( + ctx, + completeListing, + lastItem, + deletedIdsTime, + listTime, + ); err != nil { + return nil, err + } + return resp, err +} diff --git a/internal/pagination/pagination_plugin_test.go b/internal/pagination/plugin/pagination_plugin_test.go similarity index 98% rename from internal/pagination/pagination_plugin_test.go rename to internal/pagination/plugin/pagination_plugin_test.go index 0e33b5c301..619d0adcb2 100644 --- a/internal/pagination/pagination_plugin_test.go +++ b/internal/pagination/plugin/pagination_plugin_test.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: BUSL-1.1 -package pagination +package plugin import ( "context" @@ -10,13 +10,54 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/hashicorp/boundary/internal/boundary" + "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/listtoken" + "github.com/hashicorp/boundary/internal/pagination" "github.com/hashicorp/boundary/internal/plugin" "github.com/hashicorp/boundary/internal/types/resource" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +var ( + // Some unique timestamps for tests + timeNow = time.Now() + fiveDaysAgo = timeNow.AddDate(0, 0, -5) + tokenCreateTime = timeNow.AddDate(0, 0, -10) + prevDeletedTime = fiveDaysAgo.Add(time.Hour) + lastItemCreateTime = fiveDaysAgo.Add(2 * time.Hour) + lastItemUpdateTime = fiveDaysAgo.Add(3 * time.Hour) + listReturnTime = timeNow.Add(-time.Second) + deletedIDsReturnTime = timeNow.Add(-2 * time.Second) + prevPhaseUpperBound = fiveDaysAgo.Add(2 * time.Second) + phaseLowerBound = fiveDaysAgo.Add(3 * time.Second) + phaseUpperBound = fiveDaysAgo.Add(4 * time.Second) +) + +type testType struct { + boundary.Resource + ID string + CreateTime time.Time + UpdateTime time.Time +} + +func (t *testType) GetResourceType() resource.Type { + return resource.Unknown +} + +func (t *testType) GetCreateTime() *timestamp.Timestamp { + return timestamp.New(t.CreateTime) +} + +func (t *testType) GetUpdateTime() *timestamp.Timestamp { + return timestamp.New(t.UpdateTime) +} + +func (t *testType) GetPublicId() string { + return t.ID +} + func Test_ListPlugin(t *testing.T) { t.Parallel() ctx := context.Background() @@ -113,7 +154,7 @@ func Test_ListPlugin(t *testing.T) { filterItemFn := func(ctx context.Context, item *testType, plg *plugin.Plugin) (bool, error) { return true, nil } - estimatedItemCountFn := EstimatedCountFunc(nil) + estimatedItemCountFn := pagination.EstimatedCountFunc(nil) grantsHash := []byte("some hash") _, _, err := ListPlugin(ctx, grantsHash, pageSize, filterItemFn, listItemsFn, estimatedItemCountFn) require.ErrorContains(t, err, "missing estimated count callback") @@ -815,7 +856,7 @@ func Test_ListPluginPage(t *testing.T) { filterItemFn := func(ctx context.Context, item *testType, plg *plugin.Plugin) (bool, error) { return true, nil } - estimatedItemCountFn := EstimatedCountFunc(nil) + estimatedItemCountFn := pagination.EstimatedCountFunc(nil) grantsHash := []byte("some hash") _, _, err = ListPluginPage(ctx, grantsHash, pageSize, filterItemFn, listItemsFn, estimatedItemCountFn, tok) require.ErrorContains(t, err, "missing estimated count callback") @@ -1624,7 +1665,7 @@ func Test_ListPluginRefresh(t *testing.T) { estimatedItemCountFn := func(ctx context.Context) (int, error) { return 10, nil } - deletedIDsFn := ListDeletedIDsFunc(nil) + deletedIDsFn := pagination.ListDeletedIDsFunc(nil) grantsHash := []byte("some hash") _, _, err = ListPluginRefresh(ctx, grantsHash, pageSize, filterItemFn, listItemsFn, estimatedItemCountFn, deletedIDsFn, tok) require.ErrorContains(t, err, "missing list deleted IDs callback") @@ -1697,7 +1738,7 @@ func Test_ListPluginRefresh(t *testing.T) { filterItemFn := func(ctx context.Context, item *testType, plg *plugin.Plugin) (bool, error) { return true, nil } - estimatedItemCountFn := EstimatedCountFunc(nil) + estimatedItemCountFn := pagination.EstimatedCountFunc(nil) deletedIDsFn := func(ctx context.Context, since time.Time) ([]string, time.Time, error) { return nil, deletedIDsReturnTime, nil } @@ -2611,7 +2652,7 @@ func Test_ListPluginRefreshPage(t *testing.T) { estimatedItemCountFn := func(ctx context.Context) (int, error) { return 10, nil } - deletedIDsFn := ListDeletedIDsFunc(nil) + deletedIDsFn := pagination.ListDeletedIDsFunc(nil) grantsHash := []byte("some hash") _, _, err = ListPluginRefreshPage(ctx, grantsHash, pageSize, filterItemFn, listItemsFn, estimatedItemCountFn, deletedIDsFn, tok) require.ErrorContains(t, err, "missing list deleted IDs callback") @@ -2687,7 +2728,7 @@ func Test_ListPluginRefreshPage(t *testing.T) { filterItemFn := func(ctx context.Context, item *testType, plg *plugin.Plugin) (bool, error) { return true, nil } - estimatedItemCountFn := EstimatedCountFunc(nil) + estimatedItemCountFn := pagination.EstimatedCountFunc(nil) deletedIDsFn := func(ctx context.Context, since time.Time) ([]string, time.Time, error) { return nil, deletedIDsReturnTime, nil } diff --git a/internal/pagination/pagination_plugins.go b/internal/pagination/plugin/pagination_plugins.go similarity index 95% rename from internal/pagination/pagination_plugins.go rename to internal/pagination/plugin/pagination_plugins.go index a53c22b44f..dd11f15613 100644 --- a/internal/pagination/pagination_plugins.go +++ b/internal/pagination/plugin/pagination_plugins.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: BUSL-1.1 -package pagination +package plugin import ( "context" @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/boundary/internal/boundary" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/listtoken" + "github.com/hashicorp/boundary/internal/pagination" "github.com/hashicorp/boundary/internal/plugin" ) @@ -38,8 +39,8 @@ func ListPlugins[T boundary.Resource]( pageSize int, filterItemFn ListPluginsFilterFunc[T], listItemsFn ListPluginsItemsFunc[T], - estimatedCountFn EstimatedCountFunc, -) (*ListResponse[T], map[string]*plugin.Plugin, error) { + estimatedCountFn pagination.EstimatedCountFunc, +) (*pagination.ListResponse[T], map[string]*plugin.Plugin, error) { const op = "pagination.ListsPlugin" switch { @@ -84,9 +85,9 @@ func ListPluginsPage[T boundary.Resource]( pageSize int, filterItemFn ListPluginsFilterFunc[T], listItemsFn ListPluginsItemsFunc[T], - estimatedCountFn EstimatedCountFunc, + estimatedCountFn pagination.EstimatedCountFunc, tok *listtoken.Token, -) (*ListResponse[T], map[string]*plugin.Plugin, error) { +) (*pagination.ListResponse[T], map[string]*plugin.Plugin, error) { const op = "pagination.ListPluginsPage" switch { @@ -137,10 +138,10 @@ func ListPluginsRefresh[T boundary.Resource]( pageSize int, filterItemFn ListPluginsFilterFunc[T], listItemsFn ListPluginsItemsFunc[T], - estimatedCountFn EstimatedCountFunc, - listDeletedIDsFn ListDeletedIDsFunc, + estimatedCountFn pagination.EstimatedCountFunc, + listDeletedIDsFn pagination.ListDeletedIDsFunc, tok *listtoken.Token, -) (*ListResponse[T], map[string]*plugin.Plugin, error) { +) (*pagination.ListResponse[T], map[string]*plugin.Plugin, error) { const op = "pagination.ListPluginsRefresh" switch { @@ -198,10 +199,10 @@ func ListPluginsRefreshPage[T boundary.Resource]( pageSize int, filterItemFn ListPluginsFilterFunc[T], listItemsFn ListPluginsItemsFunc[T], - estimatedCountFn EstimatedCountFunc, - listDeletedIDsFn ListDeletedIDsFunc, + estimatedCountFn pagination.EstimatedCountFunc, + listDeletedIDsFn pagination.ListDeletedIDsFunc, tok *listtoken.Token, -) (*ListResponse[T], map[string]*plugin.Plugin, error) { +) (*pagination.ListResponse[T], map[string]*plugin.Plugin, error) { const op = "pagination.ListPluginsRefreshPage" switch { diff --git a/internal/pagination/pagination_plugins_test.go b/internal/pagination/plugin/pagination_plugins_test.go similarity index 99% rename from internal/pagination/pagination_plugins_test.go rename to internal/pagination/plugin/pagination_plugins_test.go index 7a6a10d10e..d63ae6ebec 100644 --- a/internal/pagination/pagination_plugins_test.go +++ b/internal/pagination/plugin/pagination_plugins_test.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: BUSL-1.1 -package pagination +package plugin import ( "context" @@ -11,6 +11,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/boundary/internal/listtoken" + "github.com/hashicorp/boundary/internal/pagination" "github.com/hashicorp/boundary/internal/plugin" "github.com/hashicorp/boundary/internal/types/resource" "github.com/stretchr/testify/assert" @@ -113,7 +114,7 @@ func Test_ListPlugins(t *testing.T) { filterItemFn := func(ctx context.Context, item *testType, plgs map[string]*plugin.Plugin) (bool, error) { return true, nil } - estimatedItemCountFn := EstimatedCountFunc(nil) + estimatedItemCountFn := pagination.EstimatedCountFunc(nil) grantsHash := []byte("some hash") _, _, err := ListPlugins(ctx, grantsHash, pageSize, filterItemFn, listItemsFn, estimatedItemCountFn) require.ErrorContains(t, err, "missing estimated count callback") @@ -918,7 +919,7 @@ func Test_ListPluginsPage(t *testing.T) { filterItemFn := func(ctx context.Context, item *testType, plgs map[string]*plugin.Plugin) (bool, error) { return true, nil } - estimatedItemCountFn := EstimatedCountFunc(nil) + estimatedItemCountFn := pagination.EstimatedCountFunc(nil) grantsHash := []byte("some hash") _, _, err = ListPluginsPage(ctx, grantsHash, pageSize, filterItemFn, listItemsFn, estimatedItemCountFn, tok) require.ErrorContains(t, err, "missing estimated count callback") @@ -1830,7 +1831,7 @@ func Test_ListPluginsRefresh(t *testing.T) { estimatedItemCountFn := func(ctx context.Context) (int, error) { return 10, nil } - deletedIDsFn := ListDeletedIDsFunc(nil) + deletedIDsFn := pagination.ListDeletedIDsFunc(nil) grantsHash := []byte("some hash") _, _, err = ListPluginsRefresh(ctx, grantsHash, pageSize, filterItemFn, listItemsFn, estimatedItemCountFn, deletedIDsFn, tok) require.ErrorContains(t, err, "missing list deleted IDs callback") @@ -1903,7 +1904,7 @@ func Test_ListPluginsRefresh(t *testing.T) { filterItemFn := func(ctx context.Context, item *testType, plgs map[string]*plugin.Plugin) (bool, error) { return true, nil } - estimatedItemCountFn := EstimatedCountFunc(nil) + estimatedItemCountFn := pagination.EstimatedCountFunc(nil) deletedIDsFn := func(ctx context.Context, since time.Time) ([]string, time.Time, error) { return nil, deletedIDsReturnTime, nil } @@ -2923,7 +2924,7 @@ func Test_ListPluginsRefreshPage(t *testing.T) { estimatedItemCountFn := func(ctx context.Context) (int, error) { return 10, nil } - deletedIDsFn := ListDeletedIDsFunc(nil) + deletedIDsFn := pagination.ListDeletedIDsFunc(nil) grantsHash := []byte("some hash") _, _, err = ListPluginsRefreshPage(ctx, grantsHash, pageSize, filterItemFn, listItemsFn, estimatedItemCountFn, deletedIDsFn, tok) require.ErrorContains(t, err, "missing list deleted IDs callback") @@ -2999,7 +3000,7 @@ func Test_ListPluginsRefreshPage(t *testing.T) { filterItemFn := func(ctx context.Context, item *testType, plgs map[string]*plugin.Plugin) (bool, error) { return true, nil } - estimatedItemCountFn := EstimatedCountFunc(nil) + estimatedItemCountFn := pagination.EstimatedCountFunc(nil) deletedIDsFn := func(ctx context.Context, since time.Time) ([]string, time.Time, error) { return nil, deletedIDsReturnTime, nil }