Skip to content

fix: update resource set page limit when changing record limit - #993

Open
pcbeingused333 wants to merge 1 commit into
twilio:mainfrom
pcbeingused333:fix/c2032-resource-set-limit
Open

pcbeingused333 wants to merge 1 commit into
twilio:mainfrom
pcbeingused333:fix/c2032-resource-set-limit

Conversation

@pcbeingused333

Copy link
Copy Markdown

Fixes ResourceSet.setLimit silently truncating iteration at the old page boundary when the limit is raised. No existing issue is associated with this fix.

ResourceSet computes pageLimit once in its constructor, and setLimit only updates the reader, so fetchNextPage keeps stopping at the old page count. For example, a set created with page size 2 and limit 2 still returns only two records after setLimit(4), even when another page is available.

Recalculate the page limit in setLimit, using the first page's size as the constructor does. Add a regression test with two pages that checks all four records are returned and the next page is fetched.

Testing and validation

mvn -q -Dtest=ResourceSetIterationTest test in maven:3-eclipse-temurin-17: before the fix, 3 tests ran with 1 regression failure (two records instead of four); after the fix, all 3 tests passed. ResourceSetResponseTest and ReaderTest also pass.

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file (not needed: bug fix, no API change)
  • I have added inline documentation to the code I modified (not needed: one-line fix, same calculation as the constructor)

AI assistance was used to prepare this change and its regression test.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant