Fix iteration over empty and unsorted records. (#971)

.changeset/fix-empty-iters.md

@@ -0,0 +1,5 @@
+---
+"github.com/livekit/protocol": patch
+---
+
+Fix iteration over empty and unsorted records.

livekit/types.go

@@ -184,16 +184,17 @@ func (it *listPageIter[T, Req, Resp]) NextPage(ctx context.Context) ([]T, error)
 		// No pagination set - returns all items.
 		// We have to do this to support legacy implementations.
 		it.done = true
-		return page, nil
+		return page, err
 	}
 	// Advance pagination cursor.
+	hasID := false
 	for i := len(page) - 1; i >= 0; i-- {
-		if id := page[i].ID(); id != "" {
+		if id := page[i].ID(); id > opts.AfterId {
 			opts.AfterId = id
-			break
+			hasID = true
 		}
 	}
-	if err == nil && len(page) == 0 {
+	if err == nil && (len(page) == 0 || !hasID) {
 		err = io.EOF
 		it.done = true
 	}

livekit/types_test.go

@@ -165,7 +165,8 @@ func (t testPageItem) ID() string {
 }
 
 type testPageReq struct {
-	Page *Pagination
+	Page        *Pagination
+	ReturnEmpty bool
 }
 
 func (t *testPageReq) GetPage() *Pagination {
@@ -183,31 +184,38 @@ func (t testPageResp) GetItems() []testPageItem {
 }
 
 func TestListPageIter(t *testing.T) {
-	testList := func(t testing.TB, req *testPageReq) {
-		const page = 3
-		var exp []testPageItem
-		for i := 0; i < 10; i++ {
-			exp = append(exp, testPageItem(fmt.Sprintf("%03d", i)))
-		}
-		it := ListPageIter(func(ctx context.Context, req *testPageReq) (testPageResp, error) {
-			limit := -1
-			if req.Page != nil {
-				limit = int(req.Page.Limit)
-				if limit == 0 {
-					limit = page
-				}
+	const page = 3
+	var (
+		all []testPageItem
+	)
+	for i := 0; i < 10; i++ {
+		all = append(all, testPageItem(fmt.Sprintf("%03d", i)))
+	}
+	pageFunc := func(ctx context.Context, req *testPageReq) (testPageResp, error) {
+		limit := -1
+		if req.Page != nil {
+			limit = int(req.Page.Limit)
+			if limit == 0 {
+				limit = page
 			}
-			var out testPageResp
-			for _, v := range exp {
-				if req.Filter(v) && req.Page.Filter(v) {
-					out = append(out, v)
-					if limit > 0 && len(out) >= limit {
-						break
-					}
+		}
+		if req.ReturnEmpty {
+			return make(testPageResp, page), nil
+		}
+		var out testPageResp
+		for _, v := range all {
+			if req.Filter(v) && req.Page.Filter(v) {
+				out = append(out, v)
+				if limit > 0 && len(out) >= limit {
+					break
 				}
 			}
-			return out, nil
-		}, req)
+		}
+		return out, nil
+	}
+	exp := all
+	testList := func(t testing.TB, req *testPageReq) {
+		it := ListPageIter(pageFunc, req)
 
 		got, err := iters.AllPages(context.Background(), it)
 		require.NoError(t, err)
@@ -222,4 +230,11 @@ func TestListPageIter(t *testing.T) {
 	t.Run("limit", func(t *testing.T) {
 		testList(t, &testPageReq{Page: &Pagination{Limit: 5}})
 	})
+	t.Run("no ids", func(t *testing.T) {
+		it := ListPageIter(pageFunc, &testPageReq{Page: &Pagination{}, ReturnEmpty: true})
+
+		got, err := iters.AllPages(context.Background(), it)
+		require.NoError(t, err)
+		require.Equal(t, []testPageItem(nil), got)
+	})
 }