Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 5, 2025

The ResourceSet class implements Iterable but had a flawed implementation that only allowed iterating through its elements once. Subsequent calls to iterator() would return an iterator that immediately reported no elements available. Additionally, the fix introduced code duplication between the two ResourceSet implementations.

Problem

The issue was in the ResourceSet's iterator implementation. While iterator() created a new ResourceSetIterator instance, the iterator relied on shared state variables in the ResourceSet instance:

  • processed - count of elements already iterated
  • iterator - the current page's iterator
  • pages - current page number

After the first iteration completed, these variables remained in their exhausted state, causing subsequent iterations to fail.

Solution

Multiple Iteration Fix

Modified both ResourceSet.java and bearertoken/ResourceSet.java to reset state when a new iterator is requested:

  1. Store a reference to the first page (firstPage) during construction
  2. Reset state variables in iterator() method before creating new iterator:
    • processed = 0 - reset element count
    • pages = 1 - reset to first page
    • page = firstPage - reset current page reference
    • iterator = firstPage.getRecords().iterator() - reset page iterator

Code Duplication Reduction

Created ResourceSetIterationHelper to eliminate duplication between the two ResourceSet classes:

  • Extracted common iteration state management to a shared helper class
  • Both ResourceSet classes now use the same logic for state reset and iteration
  • Eliminated ~50 lines of duplicated code while preserving all functionality
  • Maintained type safety using generics to work with both Resource hierarchies

Example

ResourceSet<Alert> alerts = Alert.reader().limit(5).read();

// First iteration - works
for(Alert alert : alerts) {
    System.out.println("First: " + alert.getSid());
}

// Second iteration - now works too!  
for(Alert alert : alerts) {
    System.out.println("Second: " + alert.getSid());
}

Testing

Added comprehensive tests to verify:

  • Multiple iterations return identical results
  • Limit functionality is preserved across iterations
  • Both enhanced for-loop and explicit iterator usage work
  • All existing functionality remains unchanged

Fixes #684.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] ResourceSet will only iterate its elements once Fix ResourceSet iterator to allow multiple iterations over the same dataset Aug 5, 2025
@Copilot Copilot AI requested a review from tiwarishubham635 August 5, 2025 06:58
Copilot finished work on behalf of tiwarishubham635 August 5, 2025 06:58
@tiwarishubham635 tiwarishubham635 changed the title Fix ResourceSet iterator to allow multiple iterations over the same dataset chore: Fix ResourceSet iterator to allow multiple iterations over the same dataset Aug 5, 2025
@tiwarishubham635
Copy link
Contributor

Add inline comments highlighting why the firstPage attribute is added

Copy link
Contributor Author

Copilot AI commented Aug 5, 2025

Add inline comments highlighting why the firstPage attribute is added

Added inline comments explaining why the firstPage attribute was added and how it enables multiple iterations. The comments clarify its purpose in both the field declaration and where it's used in the constructor and iterator() method. (686053a)

Copilot finished work on behalf of tiwarishubham635 August 5, 2025 07:33
Copy link
Contributor

@tiwarishubham635 tiwarishubham635 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@tiwarishubham635 tiwarishubham635 marked this pull request as ready for review August 5, 2025 08:19
Copy link
Contributor

@manisha1997 manisha1997 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code has duplication. Reduce duplication.

Copy link
Contributor Author

Copilot AI commented Aug 5, 2025

Code has duplication. Reduce duplication.

Reduced code duplication by extracting the common iteration logic to a shared ResourceSetIterationHelper class. Both ResourceSet classes now use the same helper for state management, eliminating ~50 lines of duplicated code while preserving all functionality and type safety. (cedb4fe)

@Copilot Copilot AI changed the title chore: Fix ResourceSet iterator to allow multiple iterations over the same dataset chore: Fix ResourceSet iterator to allow multiple iterations and reduce code duplication Aug 5, 2025
@Copilot Copilot AI requested a review from manisha1997 August 5, 2025 08:58
Copilot finished work on behalf of manisha1997 August 5, 2025 08:58
@shrutiburman
Copy link
Contributor

Sonarcloud reported code duplication, improve.
Increase testcases for more coverage.

Copy link

sonarqubecloud bot commented Aug 6, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
8.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@tiwarishubham635 tiwarishubham635 merged commit 2ae7c6b into main Aug 6, 2025
14 of 15 checks passed
@tiwarishubham635 tiwarishubham635 deleted the copilot/fix-684 branch August 6, 2025 07:57
tiwarishubham635 added a commit that referenced this pull request Sep 9, 2025
* chore: updating maven repository link in release notes (#876)

* chore: Fix ResourceSet iterator to allow multiple iterations and reduce code duplication (#883)

---------

Co-authored-by: Copilot <[email protected]>
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.

ResourceSet will only iterate its elements once
4 participants