-
Notifications
You must be signed in to change notification settings - Fork 32
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
IBX-719: Introduced BatchIterator #211
Conversation
cc55031
to
26341fc
Compare
PR updated according to code review suggestions. |
eZ/Publish/API/Repository/Tests/Iterator/BatchIteratorAdapter/AbstractSearchAdapterTest.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added language phpdocs based on usage found in PR.
👍
eZ/Publish/API/Repository/Iterator/BatchIteratorAdapter/LocationFilteringAdapter.php
Outdated
Show resolved
Hide resolved
eZ/Publish/API/Repository/Iterator/BatchIteratorAdapter/ContentFilteringAdapter.php
Outdated
Show resolved
Hide resolved
eZ/Publish/API/Repository/Iterator/BatchIteratorAdapter/AbstractSearchAdapter.php
Outdated
Show resolved
Hide resolved
I think sonar code style checks can be discarded: it complains that a local variable has the same name as class property. |
|
||
final class BatchIterator implements Iterator | ||
{ | ||
public const DEFAULT_BATCH_SIZE = 25; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it need to be public?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, but there is valid use case to keep it public. See https://github.com/ibexa/installer/pull/30/commits/50841b962bc2e666cf0036b11738a5503ad44d1e
eZ/Publish/API/Repository/Tests/Iterator/BatchIteratorAdapter/ContentFilteringAdapterTest.php
Outdated
Show resolved
Hide resolved
eZ/Publish/API/Repository/Tests/Iterator/BatchIteratorAdapter/LocationFilteringAdapterTest.php
Outdated
Show resolved
Hide resolved
…ert number of fetches)
…onFilteringAdapter.php Co-authored-by: Paweł Niedzielski <pawel.niedzielski@ibexa.co>
Co-authored-by: Paweł Niedzielski <pawel.niedzielski@ibexa.co>
Co-authored-by: Paweł Niedzielski <pawel.niedzielski@ibexa.co>
477a2df
to
4d2d76c
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Both code smells detected by SonarCloud should probably be marked as "won't fix", since they are about variable that shares the name with property that they are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA Approved on Ibexa Experience 3.3.6-dev with branch.
v3.3
BatchIterator
allows iterate over result sets (e.g. content objects, locations) which cannot be loaded into memory at once.Examples from existing code base:
ezplatform-kernel/eZ/Bundle/EzPublishCoreBundle/Command/CheckURLsCommand.php
Lines 90 to 108 in 4c84074
ezplatform-kernel/eZ/Bundle/EzPublishCoreBundle/Command/ResizeOriginalImagesCommand.php
Lines 210 to 220 in 4c84074
Usage
Available adapters
All built-in adapters are places under
\eZ\Publish\API\Repository\Iterator\BatchIteratorAdapter\
namespace.ContentFilteringAdapter
\eZ\Publish\API\Repository\ContentService::find
ContentInfoSearchAdapter
\eZ\Publish\API\Repository\SearchService::findContentInfo
ContentSearchAdapter
\eZ\Publish\API\Repository\SearchService::findContent
LocationFilteringAdapter
\eZ\Publish\API\Repository\LocationService::find
LocationSearchAdapter
\eZ\Publish\API\Repository\SearchService::findLocations
Checklist:
$ composer fix-cs
).@ezsystems/php-dev-team
).