Presentation: Fix paged request result accumulation (#3329)

* Avoid infinite loop when building paged requests response * Rush change * Update presentation/frontend/src/test/PresentationManager.test.ts Co-authored-by: Grigas <[email protected]> Co-authored-by: Grigas <[email protected]> (cherry picked from commit 3dbccf5)
iTwin · Mar 10, 2022 · 859ef4d · 859ef4d
1 parent 08d7fa4
commit 859ef4d
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 10 deletions.
diff --git a/...itwin/presentation-frontend/presentation-fix_paged_request_response_2022-03-10-14-40.json b/...itwin/presentation-frontend/presentation-fix_paged_request_response_2022-03-10-14-40.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@itwin/presentation-frontend",
+      "comment": "Fix paged requests result accumulation resulting in infinite loop.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@itwin/presentation-frontend"
+}
diff --git a/presentation/frontend/src/presentation-frontend/PresentationManager.ts b/presentation/frontend/src/presentation-frontend/PresentationManager.ts
@@ -481,11 +481,8 @@ interface PagedGeneratorCreateProps<TPagedResponseItem> {
   get: (pageStart: Required<PageOptions>, requestIndex: number) => Promise<{ total: number, items: TPagedResponseItem[] }>;
 }
 async function createPagedGeneratorResponse<TPagedResponseItem>(props: PagedGeneratorCreateProps<TPagedResponseItem>) {
-  const requestedPageStart = props.page?.start ?? 0;
-  const requestedPageSize = props.page?.size ?? 0;
-  let pageStart = requestedPageStart;
-  let pageSize = requestedPageSize;
-  let receivedItemsCount = 0;
+  let pageStart = props.page?.start ?? 0;
+  let pageSize = props.page?.size ?? 0;
   let requestIndex = 0;
 
   const firstPage = await props.get({ start: pageStart, size: pageSize }, requestIndex++);
@@ -496,21 +493,21 @@ async function createPagedGeneratorResponse<TPagedResponseItem>(props: PagedGene
       while (true) {
         for (const item of partialResult.items) {
           yield item;
-          ++receivedItemsCount;
         }
 
+        const receivedItemsCount = partialResult.items.length;
         if (partialResult.total !== 0 && receivedItemsCount === 0) {
           if (pageStart >= partialResult.total)
             throw new Error(`Requested page with start index ${pageStart} is out of bounds. Total number of items: ${partialResult.total}`);
           throw new Error("Paged request returned non zero total count but no items");
         }
 
-        if (requestedPageSize !== 0 && receivedItemsCount >= requestedPageSize || receivedItemsCount >= (partialResult.total - requestedPageStart))
+        if (pageSize !== 0 && receivedItemsCount >= pageSize || receivedItemsCount >= (partialResult.total - pageStart))
           break;
 
-        if (requestedPageSize !== 0)
-          pageSize = requestedPageSize - receivedItemsCount;
-        pageStart = requestedPageStart + receivedItemsCount;
+        if (pageSize !== 0)
+          pageSize -= receivedItemsCount;
+        pageStart += receivedItemsCount;
 
         partialResult = await props.get({ start: pageStart, size: pageSize }, requestIndex++);
       }

diff --git a/presentation/frontend/src/test/PresentationManager.test.ts b/presentation/frontend/src/test/PresentationManager.test.ts
@@ -1343,6 +1343,17 @@ describe("PresentationManager", () => {
       expect(result).to.deep.eq({ total: 0, items: [] });
     });
 
+    it("returns zero response when partial request returns less items than requested", async () => {
+      const getter = sinon.stub();
+      getter.onFirstCall().resolves({ total: 5, items: [2, 3] });
+      getter.onSecondCall().resolves({ total: 5, items: [] });
+      const result = await buildPagedArrayResponse({ start: 1 }, getter);
+      expect(getter).to.be.calledTwice;
+      expect(getter.firstCall).to.be.calledWith({ start: 1, size: 0 });
+      expect(getter.secondCall).to.be.calledWith({ start: 3, size: 0 });
+      expect(result).to.deep.eq({ total: 0, items: [] });
+    });
+
   });
 
 });