Skip to content
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

Add implementation of Countable for CachingIterator #2450

Merged
merged 1 commit into from
Sep 26, 2022

Conversation

Branlute
Copy link
Contributor

Q A
Type bug
BC Break no
Fixed issues

Summary

Hi,

I currently use MongoDB with Doctrine, as well as ElasticSearch through FOSElascitaBundle.
When trying to use EnqueueBundle to speed up the populate command https://github.com/FriendsOfSymfony/FOSElasticaBundle/blob/master/doc/cookbook/speed-up-populate-command.md, I encountered an error.

The enqueue command for elastica makes a count of getCurrentPageResults:
https://github.com/php-enqueue/enqueue-elastica-bundle/blob/master/Queue/PopulateProcessor.php#L61

Unfortunately, Doctrine's CachingIterator does not implement countable unlike Mongo's https://github.com/mongodb/mongo-php-library/blob/master/src/Model/CachingIterator.php which generates an error.

So I added the implementation of countable in CachingIterator in order to be able to use Enqueue with FOSElastica and MongoDB via Doctrine.

Thank you very much for the library 😉

@malarzm
Copy link
Member

malarzm commented Jun 27, 2022

@Branlute thanks for the PR! Could you please add a test to https://github.com/doctrine/mongodb-odm/blob/2.4.x/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/CachingIteratorTest.php so the ability to call \count on an iterator won't be lost? Please squash the test addition into original commit.

Thank you very much for the library wink

🍻

@malarzm malarzm added this to the 2.5.0 milestone Jun 27, 2022
@Branlute Branlute force-pushed the caching_iterator_countable branch from 5224b08 to b9edc5e Compare July 2, 2022 17:02
@Branlute Branlute force-pushed the caching_iterator_countable branch from b9edc5e to 36c3767 Compare July 2, 2022 19:05
@malarzm
Copy link
Member

malarzm commented Jul 3, 2022

@Branlute thanks for the updates! We'll need to take care of CI first before merging as something is really off with it. I'll merge this whenever I'll have a chance :)

@malarzm
Copy link
Member

malarzm commented Sep 20, 2022

@Branlute we've managed to get checks back to shape (sorry for the long time wait :( ). May I ask you to rebase your PR atop newly created 2.5.x branch?

@Branlute Branlute force-pushed the caching_iterator_countable branch from 36c3767 to 3f22c10 Compare September 24, 2022 21:18
@Branlute Branlute changed the base branch from 2.4.x to 2.5.x September 24, 2022 21:19
@Branlute
Copy link
Contributor Author

@malarzm I rebase my branch with 2.5.x and change the target branch to 2.5.x

@Branlute Branlute force-pushed the caching_iterator_countable branch from 3f22c10 to d2fc01c Compare September 24, 2022 21:41
@IonBazan IonBazan merged commit ea15696 into doctrine:2.5.x Sep 26, 2022
@IonBazan
Copy link
Member

Thanks @Branlute! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants