Skip to content

Commit

Permalink
Presentation: Fix paged request result accumulation (#3329) (#3331)
Browse files Browse the repository at this point in the history
* 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)

Co-authored-by: Saulius Skliutas <[email protected]>
Co-authored-by: Caleb Shafer <[email protected]>
Co-authored-by: Seamus Kirby <[email protected]>
Co-authored-by: Grigas <[email protected]>
  • Loading branch information
5 people authored Mar 14, 2022
1 parent 958dff2 commit afb64a4
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
Expand Up @@ -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++);
Expand All @@ -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++);
}
Expand Down
11 changes: 11 additions & 0 deletions presentation/frontend/src/test/PresentationManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [] });
});

});

});
Expand Down

0 comments on commit afb64a4

Please sign in to comment.