Merge pull request #3 from javrrr/hotfix/pagination-yield-and-next-page-url

Fix pagination iteration and next-page handling

Author: javrrr

src/core/pagination.ts

@@ -44,13 +44,35 @@ export interface PaginateOptions<T> extends PaginationParams {
   extractItems?: (raw: unknown) => T[];
 }
 
+function normalizeNextPagePath(nextPageUrl: string): string {
+  const trimmed = nextPageUrl.trim();
+  let pathWithQuery = trimmed;
+
+  try {
+    // API may return a full absolute URL; HttpClient expects a path.
+    const parsed = new URL(trimmed);
+    pathWithQuery = `${parsed.pathname}${parsed.search}`;
+  } catch {
+    // Not an absolute URL, keep as-is.
+  }
+
+  // API may return "/services/data/vXX.X/..." while the client baseUrl already
+  // includes "/services/data/vXX.X".
+  const withVersionPrefix = pathWithQuery.match(/^\/services\/data\/v\d+(?:\.\d+)?(\/.*)$/);
+  if (withVersionPrefix) {
+    return withVersionPrefix[1];
+  }
+
+  return pathWithQuery;
+}
+
 /**
  * Async generator that yields pages of items from a paginated endpoint.
  * Supports both offset-based and nextPageUrl-based pagination.
  */
 export async function* paginate<T>(
   options: PaginateOptions<T>,
-): AsyncGenerator<T[], void, undefined> {
+): AsyncGenerator<T, void, undefined> {
   const {
     httpClient,
     path,
@@ -65,6 +87,7 @@ export async function* paginate<T>(
 
   let currentOffset = startOffset;
   let nextUrl: string | undefined;
+  let totalSize: number | undefined;
 
   while (true) {
     const paginationQuery: Record<string, string | number | boolean | undefined> = {
@@ -87,22 +110,31 @@ export async function* paginate<T>(
     });
 
     const items = customExtractor ? customExtractor(raw) : extractItems(raw);
+    if (typeof raw.totalSize === "number" && Number.isFinite(raw.totalSize) && raw.totalSize >= 0) {
+      totalSize = raw.totalSize;
+    }
 
     if (items.length === 0) {
       break;
     }
 
-    yield items;
+    for (const item of items) {
+      yield item;
+    }
+    currentOffset += items.length;
+
+    // Safety net: stop when we have already yielded all advertised rows.
+    if (totalSize !== undefined && currentOffset >= totalSize) {
+      break;
+    }
 
     // Check for next page
     if (raw.nextPageUrl) {
-      nextUrl = raw.nextPageUrl;
-      currentOffset += items.length;
+      nextUrl = normalizeNextPagePath(raw.nextPageUrl);
     } else if (items.length < batchSize) {
       // No more pages
       break;
     } else {
-      currentOffset += items.length;
       nextUrl = undefined;
     }
   }
@@ -115,8 +147,8 @@ export async function collectAll<T>(
   options: PaginateOptions<T>,
 ): Promise<T[]> {
   const all: T[] = [];
-  for await (const page of paginate(options)) {
-    all.push(...page);
+  for await (const item of paginate(options)) {
+    all.push(item);
   }
   return all;
 }

src/resources/base-resource.ts

@@ -40,7 +40,7 @@ export abstract class BaseResource {
     path: string,
     params?: PaginationParams & { query?: Record<string, string | number | boolean | undefined> },
     requestOptions?: RequestOptions,
-  ): AsyncGenerator<T[], void, undefined> {
+  ): AsyncGenerator<T, void, undefined> {
     const { batchSize, offset, orderBy, pageSizeParam, query, ...rest } = params ?? {};
     return paginate<T>({
       httpClient: this.httpClient,

tests/core/pagination.test.ts

@@ -17,24 +17,22 @@ function createMockHttpClient(responses: unknown[]): HttpClient {
 }
 
 describe("pagination", () => {
-  it("yields pages of items using offset-based pagination", async () => {
+  it("yields individual items using offset-based pagination", async () => {
     const httpClient = createMockHttpClient([
       { data: [{ id: 1 }, { id: 2 }] },
       { data: [{ id: 3 }] },
     ]);
 
-    const pages: unknown[][] = [];
-    for await (const page of paginate({
+    const items: unknown[] = [];
+    for await (const item of paginate({
       httpClient,
       path: "/ssot/test",
       batchSize: 2,
     })) {
-      pages.push(page);
+      items.push(item);
     }
 
-    expect(pages).toHaveLength(2);
-    expect(pages[0]).toEqual([{ id: 1 }, { id: 2 }]);
-    expect(pages[1]).toEqual([{ id: 3 }]);
+    expect(items).toEqual([{ id: 1 }, { id: 2 }, { id: 3 }]);
   });
 
   it("stops when empty page is returned", async () => {
@@ -43,16 +41,16 @@ describe("pagination", () => {
       { data: [] },
     ]);
 
-    const pages: unknown[][] = [];
-    for await (const page of paginate({
+    const items: unknown[] = [];
+    for await (const item of paginate({
       httpClient,
       path: "/ssot/test",
       batchSize: 10,
     })) {
-      pages.push(page);
+      items.push(item);
     }
 
-    expect(pages).toHaveLength(1);
+    expect(items).toEqual([{ id: 1 }]);
   });
 
   it("collectAll gathers all items", async () => {
@@ -120,14 +118,14 @@ describe("pagination", () => {
       { data: [{ id: 1 }] },
     ]);
 
-    const pages: unknown[][] = [];
-    for await (const page of paginate({
+    const items: unknown[] = [];
+    for await (const item of paginate({
       httpClient,
       path: "/ssot/test",
       batchSize: 5,
       pageSizeParam: "limit",
     })) {
-      pages.push(page);
+      items.push(item);
     }
 
     expect(httpClient.get).toHaveBeenCalledWith("/ssot/test", {
@@ -140,13 +138,13 @@ describe("pagination", () => {
       { data: [{ id: 1 }] },
     ]);
 
-    const pages: unknown[][] = [];
-    for await (const page of paginate({
+    const items: unknown[] = [];
+    for await (const item of paginate({
       httpClient,
       path: "/ssot/test",
       batchSize: 5,
     })) {
-      pages.push(page);
+      items.push(item);
     }
 
     expect(httpClient.get).toHaveBeenCalledWith("/ssot/test", {
@@ -169,4 +167,55 @@ describe("pagination", () => {
 
     expect(all).toEqual([{ name: "a" }]);
   });
+
+  it("strips API base prefix from absolute nextPageUrl", async () => {
+    const httpClient = createMockHttpClient([
+      {
+        data: [{ id: 1 }],
+        nextPageUrl: "https://instance.my.salesforce.com/services/data/v66.0/ssot/test?offset=1&batchSize=1",
+      },
+      { data: [{ id: 2 }] },
+    ]);
+
+    const all = await collectAll({
+      httpClient,
+      path: "/ssot/test",
+      batchSize: 1,
+    });
+
+    expect(all).toEqual([{ id: 1 }, { id: 2 }]);
+    expect(httpClient.get).toHaveBeenNthCalledWith(2, "/ssot/test?offset=1&batchSize=1", {
+      query: undefined,
+    });
+  });
+
+  it("uses totalSize to avoid extra offset-based request", async () => {
+    const httpClient = createMockHttpClient([
+      { data: [{ id: 1 }, { id: 2 }], totalSize: 2 },
+    ]);
+
+    const all = await collectAll({
+      httpClient,
+      path: "/ssot/test",
+      batchSize: 2,
+    });
+
+    expect(all).toEqual([{ id: 1 }, { id: 2 }]);
+    expect(httpClient.get).toHaveBeenCalledTimes(1);
+  });
+
+  it("uses totalSize guard even when nextPageUrl is present", async () => {
+    const httpClient = createMockHttpClient([
+      { data: [{ id: 1 }], totalSize: 1, nextPageUrl: "/ssot/test?offset=1&batchSize=1" },
+    ]);
+
+    const all = await collectAll({
+      httpClient,
+      path: "/ssot/test",
+      batchSize: 1,
+    });
+
+    expect(all).toEqual([{ id: 1 }]);
+    expect(httpClient.get).toHaveBeenCalledTimes(1);
+  });
 });