From e451b5e186af38b20125820480f46d5c7c5aba24 Mon Sep 17 00:00:00 2001
From: Rob Best <robertbest89@gmail.com>
Date: Mon, 6 Feb 2023 12:42:30 +0000
Subject: [PATCH] fix: pagination via `ForEach` does not progress past first
 page (#1)

Pagination was never progressing past the first page because the
itemsSeen was being set to the TotalCount, meaning that the loop would
always exit after the first page.

I've added some tests to cover this.

Signed-off-by: Rob Best <rob.best@jetstack.io>
---
 go.mod       |   1 +
 go.sum       |   2 +
 util.go      |   2 +-
 util_test.go | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 109 insertions(+), 1 deletion(-)
 create mode 100644 util_test.go

diff --git a/go.mod b/go.mod
index 19e3054..b76def1 100644
--- a/go.mod
+++ b/go.mod
@@ -10,6 +10,7 @@ require (
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/google/go-cmp v0.5.9 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
diff --git a/go.sum b/go.sum
index 9ce4eea..20f3d70 100644
--- a/go.sum
+++ b/go.sum
@@ -1,6 +1,8 @@
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
+github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
 github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/jarcoal/httpmock v1.2.0 h1:gSvTxxFR/MEMfsGrvRbdfpRUMBStovlSRLw0Ep1bwwc=
diff --git a/util.go b/util.go
index efca0d2..0b052bd 100644
--- a/util.go
+++ b/util.go
@@ -38,7 +38,7 @@ func ForEach[T any](pageFetchFunc func(po PageOptions) (Page[T], error), handler
 			}
 		}
 
-		itemsSeen += page.TotalCount
+		itemsSeen += len(page.Items)
 		if len(page.Items) == 0 || itemsSeen >= page.TotalCount {
 			break
 		}
diff --git a/util_test.go b/util_test.go
new file mode 100644
index 0000000..fc99775
--- /dev/null
+++ b/util_test.go
@@ -0,0 +1,105 @@
+package dtrack
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestFetchAll(t *testing.T) {
+	var wantItems []int
+	for i := 0; i < 468; i++ {
+		wantItems = append(wantItems, i)
+	}
+	gotItems, err := FetchAll(func(po PageOptions) (p Page[int], err error) {
+		for i := 0; i < po.PageSize; i++ {
+			idx := (po.PageSize * (po.PageNumber - 1)) + i
+			if idx >= len(wantItems) {
+				break
+			}
+			p.Items = append(p.Items, wantItems[idx])
+		}
+		p.TotalCount = len(wantItems)
+		return p, nil
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	if diff := cmp.Diff(wantItems, gotItems); diff != "" {
+		t.Errorf("unexpected items:\n%s", diff)
+	}
+}
+
+func TestFetchAll_PageFetchFuncErr(t *testing.T) {
+	var testErr = errors.New("test error")
+	if _, err := FetchAll(
+		func(po PageOptions) (p Page[int], err error) {
+			return p, testErr
+		},
+	); !errors.Is(err, testErr) {
+		t.Errorf("expected err but got nil")
+	}
+}
+
+func TestForEach(t *testing.T) {
+	var (
+		wantItems []int
+		gotItems  []int
+	)
+	for i := 0; i < 468; i++ {
+		wantItems = append(wantItems, i)
+	}
+	if err := ForEach(
+		func(po PageOptions) (p Page[int], err error) {
+			for i := 0; i < po.PageSize; i++ {
+				idx := (po.PageSize * (po.PageNumber - 1)) + i
+				if idx >= len(wantItems) {
+					break
+				}
+				p.Items = append(p.Items, wantItems[idx])
+			}
+			p.TotalCount = len(wantItems)
+			return p, nil
+		},
+		func(item int) error {
+			gotItems = append(gotItems, item)
+			return nil
+		},
+	); err != nil {
+		t.Errorf("unexpected error calling ForEach: %s", err)
+	}
+	if diff := cmp.Diff(wantItems, gotItems); diff != "" {
+		t.Errorf("unexpected items:\n%s", diff)
+	}
+}
+
+func TestForEach_PageFetchFuncErr(t *testing.T) {
+	var testErr = errors.New("test error")
+	if err := ForEach(
+		func(po PageOptions) (p Page[int], err error) {
+			return p, testErr
+		},
+		func(item int) error {
+			return nil
+		},
+	); !errors.Is(err, testErr) {
+		t.Errorf("expected error from pageFetchFunc but got nil")
+	}
+}
+
+func TestForEach_HandlerFuncErr(t *testing.T) {
+	var testErr = errors.New("test error")
+	if err := ForEach(
+		func(po PageOptions) (p Page[int], err error) {
+			p.Items = []int{0, 1, 2, 3}
+			p.TotalCount = len(p.Items)
+			return p, nil
+		},
+		func(item int) error {
+			return testErr
+		},
+	); !errors.Is(err, testErr) {
+		t.Errorf("expected error from handlerFunc but got nil")
+	}
+}