From 61331a4ca3591cac5983c6333f255a828fde8bf6 Mon Sep 17 00:00:00 2001 From: Adrien CABARBAYE Date: Sat, 21 Jan 2023 23:51:36 +0000 Subject: [PATCH] :bug: `[pagination]` Fix panic in contructor --- changes/20230121234948.bugfix | 1 + .../collection/pagination/pagination_test.go | 113 ++++++++++++++++++ utils/collection/pagination/stream.go | 3 + 3 files changed, 117 insertions(+) create mode 100644 changes/20230121234948.bugfix diff --git a/changes/20230121234948.bugfix b/changes/20230121234948.bugfix new file mode 100644 index 0000000000..598d47fea5 --- /dev/null +++ b/changes/20230121234948.bugfix @@ -0,0 +1 @@ +:bug:`[pagination]` Fix panic in contructor diff --git a/utils/collection/pagination/pagination_test.go b/utils/collection/pagination/pagination_test.go index 6a52a7c62c..b21a374d22 100644 --- a/utils/collection/pagination/pagination_test.go +++ b/utils/collection/pagination/pagination_test.go @@ -161,6 +161,119 @@ func TestPaginator(t *testing.T) { } } +// TestPaginator_InitialisationError tests whether errors are correctly handled if API returns some error +func TestPaginator_InitialisationError(t *testing.T) { + tests := []struct { + paginator func(context.Context, IStaticPageStream) (IGenericPaginator, error) + name string + expectInitialisationError bool + }{ + { + paginator: func(ctx context.Context, collection IStaticPageStream) (IGenericPaginator, error) { + return NewAbstractPaginator(ctx, collection, func(fCtx context.Context, current IStaticPage) (IStaticPage, error) { + return nil, commonerrors.ErrUnexpected + }) + }, + name: "Abstract paginator", + expectInitialisationError: false, + }, + { + paginator: func(ctx context.Context, collection IStaticPageStream) (IGenericPaginator, error) { + return NewStaticPagePaginator(ctx, func(context.Context) (IStaticPage, error) { + return nil, commonerrors.ErrUnexpected + }, func(fCtx context.Context, current IStaticPage) (IStaticPage, error) { + return nil, commonerrors.ErrUnexpected + }) + }, + name: "Static page paginator", + expectInitialisationError: true, + }, + { + paginator: func(ctx context.Context, collection IStaticPageStream) (IGenericPaginator, error) { + return NewCollectionPaginator(ctx, func(context.Context) (IPage, error) { + return nil, commonerrors.ErrUnexpected + }) + }, + name: "paginator over a collection of dynamic pages", + expectInitialisationError: true, + }, + { + paginator: func(ctx context.Context, collection IStaticPageStream) (IGenericPaginator, error) { + return NewStaticPageStreamPaginator(ctx, time.Second, 10*time.Millisecond, func(context.Context) (IStaticPageStream, error) { + return nil, commonerrors.ErrUnexpected + }, func(fCtx context.Context, current IStaticPage) (IStaticPage, error) { + return nil, commonerrors.ErrUnexpected + }, func(fCtx context.Context, current IStaticPageStream) (IStaticPageStream, error) { + return nil, commonerrors.ErrUnexpected + }) + }, + name: "stream paginator over a collection of static pages", + expectInitialisationError: true, + }, + { + paginator: func(ctx context.Context, collection IStaticPageStream) (IGenericPaginator, error) { + return NewStreamPaginator(ctx, time.Second, 10*time.Millisecond, func(context.Context) (IStream, error) { + return nil, commonerrors.ErrUnexpected + }) + }, + name: "stream paginator over a collection of dynamic pages", + expectInitialisationError: true, + }, + { + paginator: func(ctx context.Context, collection IStaticPageStream) (IGenericPaginator, error) { + paginator, err := NewStaticPageStreamPaginator(ctx, time.Second, 10*time.Millisecond, func(context.Context) (IStaticPageStream, error) { + return nil, commonerrors.ErrUnexpected + }, func(fCtx context.Context, current IStaticPage) (IStaticPage, error) { + return nil, commonerrors.ErrUnexpected + }, func(fCtx context.Context, current IStaticPageStream) (IStaticPageStream, error) { + return nil, commonerrors.ErrUnexpected + }) + if paginator != nil { + // Indicate the stream will run out. + err = paginator.DryUp() + } + return paginator, err + }, + name: "stream paginator over a running dry stream of static pages", + expectInitialisationError: true, + }, + { + paginator: func(ctx context.Context, collection IStaticPageStream) (IGenericPaginator, error) { + paginator, err := NewStreamPaginator(ctx, 50*time.Millisecond, 10*time.Millisecond, func(context.Context) (IStream, error) { + return nil, commonerrors.ErrUnexpected + }) + if paginator != nil { + // Indicate the stream will run out. + err = paginator.DryUp() + } + return paginator, err + }, + name: "stream paginator over a running dry stream of dynamic pages", + expectInitialisationError: true, + }, + } + + for te := range tests { + test := tests[te] + for i := 0; i < 50; i++ { + var mockPages IStream + t.Run(fmt.Sprintf("%v-#%v", test.name, i), func(t *testing.T) { + paginator, err := test.paginator(context.TODO(), mockPages) + if test.expectInitialisationError { + assert.Error(t, err) + assert.Nil(t, paginator) + } else { + assert.NoError(t, err) + assert.NotNil(t, paginator) + assert.False(t, paginator.HasNext()) + require.NoError(t, paginator.Close()) + } + + }) + } + } +} + func TestPaginator_stop(t *testing.T) { tests := []struct { paginator func(context.Context, IStaticPageStream) (IGenericPaginator, error) diff --git a/utils/collection/pagination/stream.go b/utils/collection/pagination/stream.go index ee9bdce87c..ca0e130054 100644 --- a/utils/collection/pagination/stream.go +++ b/utils/collection/pagination/stream.go @@ -191,6 +191,9 @@ func NewStaticPageStreamPaginator(ctx context.Context, runOutTimeOut, backoff ti parent, err := newAbstractStreamPaginator(ctx, runOutTimeOut, backoff, func(fCtx context.Context) (IStaticPage, error) { return fetchFirstPageFunc(fCtx) }, fetchNextPageFunc, fetchFutureFunc) + if err != nil { + return + } paginator = &StaticPageStreamPaginator{ AbstractStreamPaginator: *parent, }