fix(pagination): iterator paged whichever array the spec declared first

internal/analyzer/pagination.go

@@ -66,8 +66,10 @@ func (a *Analyzer) detectCursorPagination(op *ir.OperationDef, pkg *ir.Package)
 		return nil
 	}
 
-	// Find the items field.
-	items := findItemsField(respType)
+	items := findItemsField(ir.TypesByName(pkg.Types), respType)
+	if items.name == "" {
+		return nil
+	}
 
 	return &ir.PaginationDef{
 		Style:       ir.PaginationStyleCursor,
@@ -120,9 +122,12 @@ func (a *Analyzer) detectOffsetPagination(op *ir.OperationDef, pkg *ir.Package)
 // buildOffsetPagination builds an offset-style PaginationDef.
 func (a *Analyzer) buildOffsetPagination(op *ir.OperationDef, pkg *ir.Package, offsetParam, limitParam string) *ir.PaginationDef {
 	respType := a.findSuccessResponseType(op, pkg)
-	var items itemsFieldInfo
-	if respType != nil {
-		items = findItemsField(respType)
+	if respType == nil {
+		return nil
+	}
+	items := findItemsField(ir.TypesByName(pkg.Types), respType)
+	if items.name == "" {
+		return nil
 	}
 
 	return &ir.PaginationDef{
@@ -154,22 +159,45 @@ type itemsFieldInfo struct {
 }
 
 // findItemsField finds the name and element type of the array-typed field in a
-// response type that contains the paginated items.
-func findItemsField(td *ir.TypeDef) itemsFieldInfo {
+// response type that contains the paginated items. It returns the zero value
+// when nothing identifies one, which leaves the operation without an iterator
+// rather than paging over whichever array the spec happens to declare first.
+func findItemsField(byName map[string]*ir.TypeDef, td *ir.TypeDef) itemsFieldInfo {
+	var arrays []*ir.Field
 	for _, f := range td.Fields {
-		if containsCI(itemsFieldNames, f.JSONName) && strings.HasPrefix(f.Type, "[]") {
-			return itemsFieldInfo{name: f.JSONName, elemType: f.Type[2:]}
+		if !strings.HasPrefix(f.Type, "[]") {
+			continue
 		}
-	}
-	// Fallback: find any array-typed field.
-	for _, f := range td.Fields {
-		if strings.HasPrefix(f.Type, "[]") {
+		if containsCI(itemsFieldNames, f.JSONName) {
 			return itemsFieldInfo{name: f.JSONName, elemType: f.Type[2:]}
 		}
+		arrays = append(arrays, f)
+	}
+	// One array is the page by elimination.
+	if len(arrays) == 1 {
+		return itemsFieldInfo{name: arrays[0].JSONName, elemType: arrays[0].Type[2:]}
+	}
+	// A page holds records, so an array of a declared type beside arrays of
+	// scalars is still unambiguous. Two arrays of records are not, and guessing
+	// there returns the wrong data instead of failing.
+	if records := recordArrays(byName, arrays); len(records) == 1 {
+		return itemsFieldInfo{name: records[0].JSONName, elemType: records[0].Type[2:]}
 	}
 	return itemsFieldInfo{}
 }
 
+// recordArrays returns the array fields whose element type is a type the spec
+// declares rather than a builtin.
+func recordArrays(byName map[string]*ir.TypeDef, arrays []*ir.Field) []*ir.Field {
+	var records []*ir.Field
+	for _, f := range arrays {
+		if byName[ir.NamedType(f.Type[2:])] != nil {
+			records = append(records, f)
+		}
+	}
+	return records
+}
+
 // containsCI checks if any element in the list matches the target (case-insensitive).
 func containsCI(list []string, target string) bool {
 	lower := strings.ToLower(target)

internal/analyzer/pagination_test.go

@@ -200,3 +200,92 @@ func TestContainsCI(t *testing.T) {
 		}
 	}
 }
+
+// itemsSpec builds a cursor-paginated operation whose page type declares the
+// given properties, so each case differs only in what the response holds.
+func itemsSpec(properties string) string {
+	return `openapi: 3.1.0
+info: { title: t, version: "1" }
+paths:
+  /events:
+    get:
+      operationId: listEvents
+      parameters:
+        - { name: cursor, in: query, schema: { type: string } }
+      responses:
+        "200":
+          description: ok
+          content:
+            application/json:
+              schema: { $ref: "#/components/schemas/EventPage" }
+components:
+  schemas:
+    EventPage:
+      type: object
+      properties:
+        nextCursor: { type: string }
+` + properties + `
+    Event:
+      type: object
+      properties: { id: { type: string } }
+`
+}
+
+func paginationOf(t *testing.T, spec string) *ir.PaginationDef {
+	t.Helper()
+	pkg, _ := analyzeSpec(t, spec)
+	for _, op := range pkg.Operations {
+		if op.Name == "ListEvents" {
+			return op.Pagination
+		}
+	}
+	t.Fatal("ListEvents operation not found")
+	return nil
+}
+
+func TestPaginationItems_ConventionalNameWins(t *testing.T) {
+	pd := paginationOf(t, itemsSpec(`        data: { type: array, items: { $ref: "#/components/schemas/Event" } }
+        warnings: { type: array, items: { type: string } }`))
+
+	if pd == nil || pd.ItemsField != "data" {
+		t.Errorf("items field = %+v, want data", pd)
+	}
+}
+
+func TestPaginationItems_LoneArrayIsThePage(t *testing.T) {
+	pd := paginationOf(t, itemsSpec(`        events: { type: array, items: { $ref: "#/components/schemas/Event" } }`))
+
+	if pd == nil || pd.ItemsField != "events" || pd.ItemsType != "Event" {
+		t.Errorf("items field = %+v, want events of Event", pd)
+	}
+}
+
+// A page holds records, so an array of a declared type beside an array of
+// scalars still identifies itself.
+func TestPaginationItems_RecordArrayBeatsScalarArrays(t *testing.T) {
+	pd := paginationOf(t, itemsSpec(`        warnings: { type: array, items: { type: string } }
+        events: { type: array, items: { $ref: "#/components/schemas/Event" } }`))
+
+	if pd == nil || pd.ItemsField != "events" {
+		t.Errorf("items field = %+v, want events rather than the first array declared", pd)
+	}
+}
+
+// Two arrays of records identify nothing. Paging over whichever the spec
+// declares first returns the wrong data, so the operation gets no iterator.
+func TestPaginationItems_AmbiguousArraysGetNoIterator(t *testing.T) {
+	pd := paginationOf(t, itemsSpec(`        warnings: { type: array, items: { $ref: "#/components/schemas/Event" } }
+        events: { type: array, items: { $ref: "#/components/schemas/Event" } }`))
+
+	if pd != nil {
+		t.Errorf("pagination = %+v, want none: neither array identifies the page", pd)
+	}
+}
+
+func TestPaginationItems_NoArrayGetsNoIterator(t *testing.T) {
+	pd := paginationOf(t, itemsSpec(`        total: { type: integer }`))
+
+	if pd != nil {
+		t.Errorf("pagination = %+v, want none: the response holds no page", pd)
+	}
+}

internal/generator/e2e_pagination_test.go

@@ -0,0 +1,90 @@
+package generator
+
+import "testing"
+
+const cursorPageSpec = `openapi: 3.1.0
+info: { title: events, version: "1" }
+paths:
+  /events:
+    get:
+      operationId: listEvents
+      parameters:
+        - { name: cursor, in: query, schema: { type: string } }
+      responses:
+        "200":
+          description: ok
+          content:
+            application/json:
+              schema: { $ref: "#/components/schemas/EventPage" }
+components:
+  schemas:
+    EventPage:
+      type: object
+      properties:
+        cursor: { type: string }
+        events: { type: array, items: { $ref: "#/components/schemas/Event" } }
+    Event:
+      type: object
+      properties:
+        id: { type: string }
+`
+
+// TestE2E_IteratorStopsOnANonAdvancingCursor covers a server that echoes the
+// cursor it was given, which a response field named `cursor` commonly is. The
+// iterator stopped only on an empty cursor, so All() ran identical requests
+// forever.
+func TestE2E_IteratorStopsOnANonAdvancingCursor(t *testing.T) {
+	files, _ := generateFromSpec(t, cursorPageSpec, "eventsapi")
+
+	runGeneratedWireTest(t, files, "cursorpage", `package eventsapi
+
+import "testing"
+
+func TestEchoedCursorTerminates(t *testing.T) {
+	calls := 0
+	it := &PageIterator[Event]{
+		fetch: func(cursor string) ([]Event, string, error) {
+			calls++
+			if calls > 100 {
+				t.Fatal("iterator never stopped on a cursor that does not advance")
+			}
+			return []Event{{}}, "same-cursor", nil
+		},
+	}
+
+	items, err := it.All()
+	if err != nil {
+		t.Fatalf("All: %v", err)
+	}
+	if calls != 2 {
+		t.Errorf("fetched %d pages, want 2: one page, then the repeat that ends it", calls)
+	}
+	if len(items) != 2 {
+		t.Errorf("items = %d, want the 2 pages it did fetch", len(items))
+	}
+}
+
+func TestAdvancingCursorPagesToTheEnd(t *testing.T) {
+	pages := map[string]string{"": "b", "b": "c", "c": ""}
+	var seen []string
+
+	it := &PageIterator[Event]{
+		fetch: func(cursor string) ([]Event, string, error) {
+			seen = append(seen, cursor)
+			return []Event{{}}, pages[cursor], nil
+		},
+	}
+
+	items, err := it.All()
+	if err != nil {
+		t.Fatalf("All: %v", err)
+	}
+	if len(items) != 3 {
+		t.Errorf("items = %d, want 3", len(items))
+	}
+	if len(seen) != 3 || seen[1] != "b" || seen[2] != "c" {
+		t.Errorf("cursors sent = %v, want the empty first page then b and c", seen)
+	}
+}
+`)
+}

internal/templates/pagination.go.tmpl

@@ -21,7 +21,10 @@ func (it *PageIterator[T]) Next() ([]T, error) {
 	if err != nil {
 		return nil, err
 	}
-	if next == "" {
+	// A cursor that comes back unchanged means the page did not advance, which
+	// an echoed request cursor would otherwise turn into an endless run of
+	// identical requests.
+	if next == "" || next == it.nextCursor {
 		it.done = true
 	}
 	it.nextCursor = next