Skip to content

Commit

Permalink
IBX-7149: Refactored content type-based indexing to rely on a dedicat…
Browse files Browse the repository at this point in the history
…ed strategy (#296)

For more details see https://issues.ibexa.co/browse/IBX-7149 and #296

Key changes:

* Fixed fetching batches of content IDs by a content type

* Extracted content ID by content type fetching as a strategy

* Introduced ContentIdBatchList result for ContentIdListGeneratorStrategy

* Fixed ContentList iterator type hinting

* [Tests] Aligned ContentFilteringTest after ContentList type hinting fix

* [Tests] Added coverage for ContentType input generator strategy

* [Tests] Added coverage for ContentIdBatchList result wrapper object

---------

Co-authored-by: Andrew Longosz <alongosz@users.noreply.github.com>
Co-authored-by: Paweł Niedzielski <Steveb-p@users.noreply.github.com>
  • Loading branch information
3 people authored Mar 20, 2024
1 parent 40b0fd0 commit 1fd57b2
Show file tree
Hide file tree
Showing 12 changed files with 433 additions and 93 deletions.
5 changes: 0 additions & 5 deletions phpstan-baseline-7.4.neon
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ parameters:
count: 1
path: src/bundle/Core/Command/CleanupVersionsCommand.php

-
message: "#^Parameter \\#1 \\$var of function count expects array\\|Countable, iterable\\<Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\Content\\>&Traversable given\\.$#"
count: 1
path: src/bundle/Core/Command/ExpireUserPasswordsCommand.php

-
message: "#^Parameter \\#1 \\$fp of function fclose expects resource, resource\\|false given\\.$#"
count: 1
Expand Down
5 changes: 0 additions & 5 deletions phpstan-baseline-8.0.neon
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ parameters:
count: 1
path: src/bundle/Core/Command/CleanupVersionsCommand.php

-
message: "#^Parameter \\#1 \\$value of function count expects array\\|Countable, iterable\\<Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\Content\\>&Traversable given\\.$#"
count: 1
path: src/bundle/Core/Command/ExpireUserPasswordsCommand.php

-
message: "#^Parameter \\#1 \\$stream of function fclose expects resource, resource\\|false given\\.$#"
count: 1
Expand Down
32 changes: 1 addition & 31 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -255,11 +255,6 @@ parameters:
count: 1
path: src/bundle/Core/Command/RegenerateUrlAliasesCommand.php

-
message: "#^Access to an undefined property Ibexa\\\\Contracts\\\\Core\\\\Persistence\\\\Content\\\\Location\\\\Handler\\:\\:\\$pathString\\.$#"
count: 2
path: src/bundle/Core/Command/ReindexCommand.php

-
message: "#^Call to an undefined method Ibexa\\\\Core\\\\Search\\\\Common\\\\Indexer\\:\\:purge\\(\\)\\.$#"
count: 1
Expand All @@ -270,21 +265,11 @@ parameters:
count: 2
path: src/bundle/Core/Command/ReindexCommand.php

-
message: "#^Method Ibexa\\\\Bundle\\\\Core\\\\Command\\\\ReindexCommand\\:\\:__construct\\(\\) has parameter \\$searchIndexer with no type specified\\.$#"
count: 1
path: src/bundle/Core/Command/ReindexCommand.php

-
message: "#^Method Ibexa\\\\Bundle\\\\Core\\\\Command\\\\ReindexCommand\\:\\:configure\\(\\) has no return type specified\\.$#"
count: 1
path: src/bundle/Core/Command/ReindexCommand.php

-
message: "#^Method Ibexa\\\\Bundle\\\\Core\\\\Command\\\\ReindexCommand\\:\\:getPhpProcess\\(\\) has parameter \\$contentIds with no value type specified in iterable type array\\.$#"
count: 1
path: src/bundle/Core/Command/ReindexCommand.php

-
message: "#^Method Ibexa\\\\Bundle\\\\Core\\\\Command\\\\ReindexCommand\\:\\:initialize\\(\\) has no return type specified\\.$#"
count: 1
Expand Down Expand Up @@ -317,7 +302,7 @@ parameters:

-
message: "#^Property Ibexa\\\\Bundle\\\\Core\\\\Command\\\\ReindexCommand\\:\\:\\$phpPath \\(string\\) does not accept string\\|null\\.$#"
count: 2
count: 1
path: src/bundle/Core/Command/ReindexCommand.php

-
Expand Down Expand Up @@ -6930,11 +6915,6 @@ parameters:
count: 1
path: src/contracts/Repository/Iterator/BatchIteratorAdapter/ContentFilteringAdapter.php

-
message: "#^Method Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Iterator\\\\BatchIteratorAdapter\\\\ContentFilteringAdapter\\:\\:fetch\\(\\) should return Iterator but returns iterable\\<Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\Content\\>&Traversable\\.$#"
count: 1
path: src/contracts/Repository/Iterator/BatchIteratorAdapter/ContentFilteringAdapter.php

-
message: "#^Method Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Iterator\\\\BatchIteratorAdapter\\\\LocationFilteringAdapter\\:\\:__construct\\(\\) has parameter \\$languages with no value type specified in iterable type array\\.$#"
count: 1
Expand Down Expand Up @@ -7085,11 +7065,6 @@ parameters:
count: 1
path: src/contracts/Repository/Values/Content/ContentList.php

-
message: "#^Method Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\ContentList\\:\\:__construct\\(\\) has parameter \\$contentItems with no value type specified in iterable type array\\.$#"
count: 1
path: src/contracts/Repository/Values/Content/ContentList.php

-
message: "#^Method Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\DraftList\\\\Item\\\\UnauthorizedContentDraftListItem\\:\\:__construct\\(\\) has parameter \\$payload with no value type specified in iterable type array\\.$#"
count: 1
Expand Down Expand Up @@ -34345,11 +34320,6 @@ parameters:
count: 1
path: tests/integration/Core/Repository/Filtering/ContentFilteringTest.php

-
message: "#^Cannot access offset 0 on iterable\\<Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\Content\\>&Traversable\\.$#"
count: 1
path: tests/integration/Core/Repository/Filtering/ContentFilteringTest.php

-
message: "#^Cannot use array destructuring on iterable\\<Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\Location\\>\\.$#"
count: 1
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Bundle\Core\Command\Indexer\ContentIdList;

use Generator;
use Ibexa\Bundle\Core\Command\Indexer\ContentIdListGeneratorStrategyInterface;
use Ibexa\Contracts\Core\Repository\ContentService;
use Ibexa\Contracts\Core\Repository\Values\Content\ContentList;
use Ibexa\Contracts\Core\Repository\Values\Content\Query;
use Ibexa\Contracts\Core\Repository\Values\Filter\Filter;
use Ibexa\Core\Search\Indexer\ContentIdBatchList;
use Symfony\Component\Console\Input\InputInterface;

/**
* @internal
*/
final class ContentTypeInputGeneratorStrategy implements ContentIdListGeneratorStrategyInterface
{
private ContentService $contentService;

public function __construct(ContentService $contentService)
{
$this->contentService = $contentService;
}

/**
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException
*/
public function getBatchList(InputInterface $input, int $batchSize): ContentIdBatchList
{
$contentList = $this->getContentList($input->getOption('content-type'));

return new ContentIdBatchList(
$this->buildGenerator($contentList, $batchSize),
$contentList->getTotalCount(),
);
}

private function buildGenerator(ContentList $contentList, int $batchSize): Generator
{
$contentIds = [];
foreach ($contentList as $content) {
$contentIds[] = $content->getVersionInfo()->getContentInfo()->getId();
if (count($contentIds) >= $batchSize) {
yield $contentIds;
$contentIds = [];
}
}
if (!empty($contentIds)) {
yield $contentIds;
}
}

public function shouldPurgeIndex(InputInterface $input): bool
{
return false;
}

/**
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException
*/
private function getContentList(string $contentTypeIdentifier): ContentList
{
$filter = new Filter();
$filter
->withCriterion(
new Query\Criterion\ContentTypeIdentifier($contentTypeIdentifier)
)
;

return $this->contentService->find($filter);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Bundle\Core\Command\Indexer;

use Ibexa\Core\Search\Indexer\ContentIdBatchList;
use Symfony\Component\Console\Input\InputInterface;

/**
* @internal
*/
interface ContentIdListGeneratorStrategyInterface
{
public function shouldPurgeIndex(InputInterface $input): bool;

public function getBatchList(InputInterface $input, int $batchSize): ContentIdBatchList;
}
74 changes: 30 additions & 44 deletions src/bundle/Core/Command/ReindexCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,8 @@
use function count;
use DateTime;
use const DIRECTORY_SEPARATOR;
use Generator;
use Ibexa\Bundle\Core\Command\Indexer\ContentIdListGeneratorStrategyInterface;
use Ibexa\Contracts\Core\Persistence\Content\Location\Handler;
use Ibexa\Contracts\Core\Repository\ContentService as APIContentService;
use Ibexa\Contracts\Core\Repository\Values\Content\ContentList;
use Ibexa\Contracts\Core\Repository\Values\Content\Query;
use Ibexa\Contracts\Core\Repository\Values\Filter\Filter;
use Ibexa\Contracts\Core\Search\Content\IndexerGateway;
use Ibexa\Core\Base\Exceptions\InvalidArgumentException;
use Ibexa\Core\Search\Common\IncrementalIndexer;
Expand Down Expand Up @@ -59,18 +55,18 @@ class ReindexCommand extends Command implements BackwardCompatibleCommand
/** @var \Ibexa\Contracts\Core\Persistence\Content\Location\Handler */
private $locationHandler;

private APIContentService $contentService;
private ContentIdListGeneratorStrategyInterface $contentIdListGeneratorStrategy;

public function __construct(
$searchIndexer,
Indexer $searchIndexer,
Handler $locationHandler,
IndexerGateway $gateway,
LoggerInterface $logger,
string $siteaccess,
string $env,
bool $isDebug,
string $projectDir,
APIContentService $contentService,
ContentIdListGeneratorStrategyInterface $contentIdListGeneratorStrategy,
string $phpPath = null
) {
$this->gateway = $gateway;
Expand All @@ -82,8 +78,7 @@ public function __construct(
$this->env = $env;
$this->isDebug = $isDebug;
$this->projectDir = $projectDir;
$this->contentService = $contentService;
$this->phpPath = $phpPath;
$this->contentIdListGeneratorStrategy = $contentIdListGeneratorStrategy;

parent::__construct();
}
Expand Down Expand Up @@ -261,28 +256,20 @@ protected function indexIncrementally(

if ($since = $input->getOption('since')) {
$count = $this->gateway->countContentSince(new DateTime($since));
$generator = $this->gateway->getContentSince(new DateTime($since), $iterationCount);
$batchList = $this->gateway->getContentSince(new DateTime($since), $iterationCount);
$purge = false;
} elseif ($locationId = (int) $input->getOption('subtree')) {
/** @var \Ibexa\Contracts\Core\Persistence\Content\Location\Handler */
$location = $this->locationHandler->load($locationId);
$count = $this->gateway->countContentInSubtree($location->pathString);
$generator = $this->gateway->getContentInSubtree($location->pathString, $iterationCount);
$purge = false;
} elseif ($contentType = $input->getOption('content-type')) {
$filter = new Filter();
$filter
->withCriterion(
new Query\Criterion\ContentTypeIdentifier($contentType)
);

$contentList = $this->contentService->find($filter);
$count = $contentList->getTotalCount();
$generator = $this->fetchIterationFromContentList($contentList, $iterationCount);
$batchList = $this->gateway->getContentInSubtree($location->pathString, $iterationCount);
$purge = false;
} elseif (!empty($input->getOption('content-type'))) {
$batchList = $this->contentIdListGeneratorStrategy->getBatchList($input, $iterationCount);
$count = $batchList->getCount();
$purge = $this->contentIdListGeneratorStrategy->shouldPurgeIndex($input);
} else {
$count = $this->gateway->countAllContent();
$generator = $this->gateway->getAllContent($iterationCount);
$batchList = $this->gateway->getAllContent($iterationCount);
$purge = !$input->getOption('no-purge');
}

Expand Down Expand Up @@ -318,13 +305,13 @@ protected function indexIncrementally(
if ($processCount > 1) {
$this->runParallelProcess(
$progress,
$generator,
$batchList,
(int)$processCount,
$commit
);
} else {
// if we only have one process, or less iterations to warrant running several, we index it all inline
foreach ($generator as $contentIds) {
foreach ($batchList as $contentIds) {
$this->searchIndexer->updateSearchIndex($contentIds, $commit);
$progress->advance(1);
}
Expand All @@ -341,15 +328,28 @@ protected function indexIncrementally(
}

/**
* @param iterable<int, array<int>> $results
*
* @return \Generator<int, array<int>>
*/
private function buildGenerator(iterable $results): \Generator
{
yield from $results;
}

/**
* @param iterable<int, array<int>> $batchList
*
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
*/
private function runParallelProcess(
ProgressBar $progress,
Generator $generator,
iterable $batchList,
int $processCount,
bool $commit
): void {
/** @var \Symfony\Component\Process\Process[]|null[] */
$generator = $this->buildGenerator($batchList);
/** @var \Symfony\Component\Process\Process[]|null[] $processes */
$processes = array_fill(0, $processCount, null);
do {
/** @var \Symfony\Component\Process\Process $process */
Expand Down Expand Up @@ -390,7 +390,7 @@ private function runParallelProcess(
}

/**
* @param array $contentIds
* @param int[] $contentIds
*
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
*/
Expand Down Expand Up @@ -478,20 +478,6 @@ public function getDeprecatedAliases(): array
{
return ['ezplatform:reindex'];
}

private function fetchIterationFromContentList(ContentList $contentList, int $iterationCount): Generator
{
$i = 1;
$contentIds = [];
foreach ($contentList as $content) {
$contentIds[] = $content->id;
if ($iterationCount <= $i) {
break;
}
++$i;
}
yield $contentIds;
}
}

class_alias(ReindexCommand::class, 'eZ\Bundle\EzPublishCoreBundle\Command\ReindexCommand');
6 changes: 5 additions & 1 deletion src/bundle/Core/Resources/config/commands.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ services:
$env: '%kernel.environment%'
$projectDir: '%kernel.project_dir%'
$isDebug: '%kernel.debug%'
$contentService: '@Ibexa\Contracts\Core\Repository\ContentService'
$contentIdListGeneratorStrategy: '@Ibexa\Bundle\Core\Command\Indexer\ContentIdList\ContentTypeInputGeneratorStrategy'
tags:
- { name: console.command }

Expand All @@ -65,3 +65,7 @@ services:
Ibexa\Bundle\Core\Command\ExpireUserPasswordsCommand:
autowire: true
autoconfigure: true

# Dedicated services for commands
Ibexa\Bundle\Core\Command\Indexer\ContentIdList\ContentTypeInputGeneratorStrategy:
autowire: true
Loading

0 comments on commit 1fd57b2

Please sign in to comment.