Skip to content

PHPLIB-608 Fix wrong validity check in CachingIterator #803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 25, 2021

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Jan 21, 2021

PHPLIB-608

Note that this does not affect 1.8, as CachingIterator always wraps the given traversable in an IteratorIterator, which corrects this wrong behaviour in underlying iterators. In the upcoming 1.9 release, this is no longer the case for any traversable that implements the Iterator interface, so this wrong behaviour can lead to extra elements given (hence the test failures).

@alcaeus alcaeus requested a review from jmikola January 21, 2021 13:39
@alcaeus alcaeus self-assigned this Jan 21, 2021
Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

Suggested refactoring but LGTM.

Side note: reviewing this made me realize that CachingIterator would break if the wrapped Traversable had non-unique keys. I opened PHPLIB-609 to track that.

*/
public function testWithWrongIterator()
{
$nestedIterator = new class implements Iterator {
Copy link
Member

Choose a reason for hiding this comment

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

What a lovely use of anonymous classes!

@alcaeus alcaeus merged commit ae3dc59 into mongodb:master Jan 25, 2021
@alcaeus alcaeus deleted the phplib-608 branch January 25, 2021 11:43
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.

2 participants