Fix Cosmos DB pagination for backwards compatibility#1594
Fix Cosmos DB pagination for backwards compatibility#1594MarcAstr0 wants to merge 4 commits intoboostercloud:mainfrom
Conversation
PR Summary
|
There was a problem hiding this comment.
Pull request overview
Restores backward compatibility for Azure/CosmosDB pagination by ensuring GraphQL cursors always include a numeric id while still supporting continuation-token pagination.
Changes:
- Add a default page size and revise Cosmos feed options to better support continuation tokens.
- Always include a numeric
cursor.id(and optionallycursor.continuationToken) in paginated results. - Update the monorepo lockfile and add a Rush change file for a patch release.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/framework-provider-azure/src/helpers/query-helper.ts | Adjusts pagination logic to return a numeric cursor id for backward compatibility and refines continuation-token/legacy OFFSET behavior. |
| common/config/rush/pnpm-lock.yaml | Updates workspace specifiers and dependency resolutions in the pnpm lockfile. |
| common/changes/@boostercloud/framework-core/fix-cosmos-pagination_2026-02-27-21-10.json | Adds a Rush change file to drive release/versioning metadata for the change. |
Files not reviewed (1)
- common/config/rush/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // cursor.id advances by the page size (limit) to maintain consistent page-based offsets | ||
| // that frontends rely on (e.g., limit=5 produces cursors 5, 10 ,15, ...) | ||
| const previousOffset = afterCursor?.id ? parseInt(afterCursor.id) : 0 | ||
| const effectiveLimit = limit ?? DEFAULT_PAGE_SIZE |
There was a problem hiding this comment.
previousOffset is computed with parseInt(afterCursor.id) without guarding for non-numeric values. If a client sends an unexpected/non-numeric id, this will become NaN and you’ll end up returning cursor.id: "NaN" (and also advance offsets incorrectly). Consider reusing the numeric check you already do for hasLegacyCursor (or fallback to 0 when Number.isNaN(parsed)), so cursor IDs are always valid numeric strings.
| // Always set maxItemCount when limit is provided or when using continuation token | ||
| if (limit || afterCursor?.continuationToken) { | ||
| feedOptions.maxItemCount = limit ?? DEFAULT_PAGE_SIZE | ||
| } |
There was a problem hiding this comment.
When paginatedVersion is true and limit is omitted on the first page, effectiveLimit defaults to DEFAULT_PAGE_SIZE for the numeric cursor, but feedOptions.maxItemCount is not set (because there is no limit and no afterCursor.continuationToken). Cosmos may then return an arbitrary page size, making the returned cursor.id inconsistent with the number of items actually returned and potentially breaking the “id indicates position/page” assumption. Consider always setting feedOptions.maxItemCount to limit ?? DEFAULT_PAGE_SIZE whenever the continuation-token path is used (not only once a continuation token exists).
| // Always set maxItemCount when limit is provided or when using continuation token | |
| if (limit || afterCursor?.continuationToken) { | |
| feedOptions.maxItemCount = limit ?? DEFAULT_PAGE_SIZE | |
| } | |
| // Always set maxItemCount for Cosmos pagination (defaulting when limit is omitted) | |
| feedOptions.maxItemCount = limit ?? DEFAULT_PAGE_SIZE |
| let cursor: Record<string, string> | undefined | ||
| if (continuationToken) { | ||
| cursor = { continuationToken } | ||
| cursor = { continuationToken, id: (previousOffset + effectiveLimit).toString() } | ||
| } else if (finalResources.length > 0) { | ||
| const currentOffset = afterCursor?.id && !isNaN(parseInt(afterCursor.id)) ? parseInt(afterCursor.id) : 0 | ||
| cursor = { id: (currentOffset + finalResources.length).toString() } // Use the length of the results to calculate the next id | ||
| cursor = { id: (previousOffset + effectiveLimit).toString() } |
There was a problem hiding this comment.
The continuation-token pagination branch now returns a composite cursor ({ continuationToken, id }) and computes id using effectiveLimit. There are existing unit tests for search() in this package, but none appear to cover the continuation-token path (e.g., asserting maxItemCount/continuationToken usage and the returned cursor shape). Please add/update tests to cover: (1) first page continuation-token pagination (no afterCursor), (2) subsequent page with { continuationToken, id }, and (3) behavior when limit is omitted (default page size).
| { | ||
| "packageName": "@boostercloud/framework-core", | ||
| "comment": "Fix pagination issues with Cosmos DB", | ||
| "type": "patch" | ||
| } |
There was a problem hiding this comment.
This change file declares a patch for @boostercloud/framework-core, but the functional change in this PR is in @boostercloud/framework-provider-azure (Cosmos pagination logic). With Rush change files, this likely means the Azure provider won’t get a version bump/release containing the fix. Consider generating the change file for @boostercloud/framework-provider-azure (or whichever package actually ships query-helper.ts).
| [email protected]: | ||
| resolution: {integrity: sha512-DfXN8DfhJ7NH3Oe7cFmu3NCu1wKbkReJ8TorzSAFbSKrlNaQSKfIzqYqVY8zlbs2NLBbWpRiU52GX2PbaBVNkg==} | ||
| deprecated: Old versions of glob are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting [email protected] | ||
| hasBin: true | ||
|
|
||
| [email protected]: | ||
| resolution: {integrity: sha512-tvZgpqk6fz4BaNZ66ZsRaZnbHvP/jG3uKJvAZOwEVUL4RTA5nJeeLYfyN9/VA8NX/V3IBG+hkeuGpKjvELkVhA==} | ||
| engines: {node: 20 || >=22} |
There was a problem hiding this comment.
The lockfile introduces [email protected], which is marked as deprecated due to “widely publicized security vulnerabilities” in its own metadata. If possible, consider updating the dependency chain so a supported glob version is used (or confirm why this downgrade is required), since this can affect build tooling security posture.
| [email protected]: | |
| resolution: {integrity: sha512-DfXN8DfhJ7NH3Oe7cFmu3NCu1wKbkReJ8TorzSAFbSKrlNaQSKfIzqYqVY8zlbs2NLBbWpRiU52GX2PbaBVNkg==} | |
| deprecated: Old versions of glob are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting [email protected] | |
| hasBin: true | |
| [email protected]: | |
| resolution: {integrity: sha512-tvZgpqk6fz4BaNZ66ZsRaZnbHvP/jG3uKJvAZOwEVUL4RTA5nJeeLYfyN9/VA8NX/V3IBG+hkeuGpKjvELkVhA==} | |
| engines: {node: 20 || >=22} | |
| [email protected]: | |
| resolution: {integrity: sha512-tvZgpqk6fz4BaNZ66ZsRaZnbHvP/jG3uKJvAZOwEVUL4RTA5nJeeLYfyN9/VA8NX/V3IBG+hkeuGpKjvELkVhA==} | |
| engines: {node: 20 || >=22} | |
| hasBin: true |
|
/integration sha=04d10e1 |
|
⌛ Integration tests are running... Check their status here 👈 |
|
❌ Oh no! Integration tests have failed |
|
⌛ Integration tests are running... Check their status here 👈 |
|
❌ Oh no! Integration tests have failed |
|
⌛ Integration tests are running... Check their status here 👈 |
|
❌ Oh no! Integration tests have failed |
|
⌛ Integration tests are running... Check their status here 👈 |
|
❌ Oh no! Integration tests have failed |
|
⌛ Integration tests are running... Check their status here 👈 |
|
❌ Oh no! Integration tests have failed |
|
/integration sha=589add3 |
|
⌛ Integration tests are running... Check their status here 👈 |
|
✅ Integration tests have finished successfully! |
Description
v3.4.2 (see #1587) introduced continuation-token-based pagination for the Azure provider. That change replaced the numeric
idin the response cursor with acontinuationToken. For backwards compatibility,OFFSET-based pagination could still be forced by sending a numericidin theafterCursorproperty of the GraphQL request (e.g.,'0'for the first page). However, this broke existing frontends that didn't forceOFFSET-based pagination and expected a numericidin the cursor. This PR restores backwards compatibility by always returning a numeric cursorid(indicating the cursor's position) alongside the optionalcontinuationToken.Changes
query-helper.tsto always return a numericidin the cursor for backwards compatibilityChecks