Skip to content

Commit

Permalink
Wrapped iterables used as arrays with iterator_to_array (#372)
Browse files Browse the repository at this point in the history
For more details see #372

Key changes:

* Wrapped `iterable`s used as arrays with `iterator_to_array`

* [Tests] Wrapped `iterable`s used as arrays with `iterator_to_array`

* [PHPStan] Aligned baseline with the changes
  • Loading branch information
alongosz authored May 17, 2024
1 parent 721ca78 commit 3278641
Show file tree
Hide file tree
Showing 25 changed files with 105 additions and 330 deletions.
20 changes: 0 additions & 20 deletions phpstan-baseline-gte-8.0.neon
Original file line number Diff line number Diff line change
Expand Up @@ -355,21 +355,11 @@ parameters:
count: 4
path: tests/integration/Core/Repository/ContentServiceAuthorizationTest.php

-
message: "#^Parameter \\#1 \\$array of function array_keys expects array, iterable\\<Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\ContentInfo\\> given\\.$#"
count: 1
path: tests/integration/Core/Repository/ContentServiceTest.php

-
message: "#^Parameter \\#1 \\$array of function array_values expects array, iterable\\<Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\Location\\> given\\.$#"
count: 1
path: tests/integration/Core/Repository/ContentServiceTest.php

-
message: "#^Parameter \\#1 \\$array of function usort expects TArray of array, iterable\\<Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\Relation\\> given\\.$#"
count: 2
path: tests/integration/Core/Repository/ContentServiceTest.php

-
message: "#^Parameter \\#1 \\$string of function md5 expects string, float given\\.$#"
count: 3
Expand Down Expand Up @@ -405,16 +395,6 @@ parameters:
count: 2
path: tests/integration/Core/Repository/LanguageServiceTest.php

-
message: "#^Parameter \\#1 \\$array of function array_filter expects array, iterable\\<Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\URLAlias\\> given\\.$#"
count: 1
path: tests/integration/Core/Repository/LocationServiceTest.php

-
message: "#^Parameter \\#1 \\$array of function array_keys expects array, iterable\\<Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\Location\\> given\\.$#"
count: 3
path: tests/integration/Core/Repository/LocationServiceTest.php

-
message: "#^Parameter \\#1 \\$array of function array_filter expects array, iterable\\<Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\User\\\\Policy\\> given\\.$#"
count: 1
Expand Down
225 changes: 0 additions & 225 deletions phpstan-baseline.neon

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion src/bundle/Core/Command/RegenerateUrlAliasesCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ protected function getProgressBar($maxSteps, OutputInterface $output)
* @param \Ibexa\Contracts\Core\Repository\Values\Content\Location[] $locations
* @param \Symfony\Component\Console\Helper\ProgressBar $progressBar
*/
private function processLocations(array $locations, ProgressBar $progressBar)
private function processLocations(array $locations, ProgressBar $progressBar): void
{
$contentList = $this->repository->sudo(
static function (Repository $repository) use ($locations) {
Expand All @@ -204,6 +204,7 @@ static function (Location $location) {
);
}
);
$contentList = iterator_to_array($contentList);
foreach ($locations as $location) {
try {
// ignore missing Content items
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/Repository/LanguageService.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public function loadLanguageListByCode(array $languageCodes): iterable;
*
* @param int[] $languageIds
*
* @return \Ibexa\Contracts\Core\Repository\Values\Content\Language[] list of Languages with id as keys
* @return iterable<int, \Ibexa\Contracts\Core\Repository\Values\Content\Language> list of Languages with id as keys
*/
public function loadLanguageListById(array $languageIds): iterable;

Expand Down
2 changes: 1 addition & 1 deletion src/contracts/Repository/Values/Content/Content.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ abstract public function getFields(): iterable;
*
* @param string|null $languageCode
*
* @return \Ibexa\Contracts\Core\Repository\Values\Content\Field[] An array of {@link Field} with field identifier as keys
* @return iterable<string, \Ibexa\Contracts\Core\Repository\Values\Content\Field> An array of {@link Field} with field identifier as keys
*/
abstract public function getFieldsByLanguage(?string $languageCode = null): iterable;

Expand Down
7 changes: 6 additions & 1 deletion src/contracts/Repository/Values/User/RoleAssignment.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ abstract class RoleAssignment extends ValueObject
*
* @var int
*/
protected $id;
protected int $id;

public function getId(): int
{
return $this->id;
}

/**
* Returns the limitation of the role assignment.
Expand Down
6 changes: 4 additions & 2 deletions src/lib/MVC/Symfony/Routing/Generator/UrlAliasGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,14 @@ private function createPathString(Location $location, ?string $siteAccess = null
if ($siteAccess) {
// We generate for a different SiteAccess, so potentially in a different language.
$languages = $this->configResolver->getParameter('languages', null, $siteAccess);
$urlAliases = $urlAliasService->listLocationAliases($location, false, null, null, $languages);
$urlAliases = iterator_to_array(
$urlAliasService->listLocationAliases($location, false, null, null, $languages)
);
// Use the target SiteAccess root location
$rootLocationId = $this->configResolver->getParameter('content.tree_root.location_id', null, $siteAccess);
} else {
$languages = null;
$urlAliases = $urlAliasService->listLocationAliases($location, false);
$urlAliases = iterator_to_array($urlAliasService->listLocationAliases($location, false));
$rootLocationId = $this->rootLocationId;
}

Expand Down
4 changes: 2 additions & 2 deletions src/lib/Persistence/Legacy/Content/ObjectState/Mapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public function createObjectStateFromData(array $data)
foreach ($data as $stateTranslation) {
$languageIds[] = (int)$stateTranslation['ezcobj_state_language_language_id'] & ~1;
}
$languages = $this->languageHandler->loadList($languageIds);
$languages = iterator_to_array($this->languageHandler->loadList($languageIds));

$objectState->id = (int)$data[0]['ezcobj_state_id'];
$objectState->groupId = (int)$data[0]['ezcobj_state_group_id'];
Expand Down Expand Up @@ -104,7 +104,7 @@ public function createObjectStateGroupFromData(array $data)
foreach ($data as $groupTranslation) {
$languageIds[] = (int)$groupTranslation['ezcobj_state_group_language_real_language_id'];
}
$languages = $this->languageHandler->loadList($languageIds);
$languages = iterator_to_array($this->languageHandler->loadList($languageIds));

$objectStateGroup->id = (int)$data[0]['ezcobj_state_group_id'];
$objectStateGroup->identifier = $data[0]['ezcobj_state_group_identifier'];
Expand Down
12 changes: 8 additions & 4 deletions src/lib/Repository/ContentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -556,9 +556,11 @@ public function loadContentListByContentInfo(
$contentIds,
$translations
);
$contentTypeList = $this->repository->getContentTypeService()->loadContentTypeList(
array_unique($contentTypeIds),
$languages
$contentTypeList = iterator_to_array(
$this->repository->getContentTypeService()->loadContentTypeList(
array_unique($contentTypeIds),
$languages
)
);
foreach ($spiContentList as $contentId => $spiContent) {
$contentInfo = $spiContent->versionInfo->contentInfo;
Expand Down Expand Up @@ -1522,7 +1524,9 @@ protected function copyNonTranslatableFieldsFromPublishedVersion(APIContent $cur
}

$mainLanguageCode = $publishedContent->getVersionInfo()->getContentInfo()->getMainLanguageCode();
$publishedContentFieldsInMainLanguage = $publishedContent->getFieldsByLanguage($mainLanguageCode);
$publishedContentFieldsInMainLanguage = iterator_to_array(
$publishedContent->getFieldsByLanguage($mainLanguageCode)
);

$fieldValues = [];
$persistenceFields = [];
Expand Down
4 changes: 3 additions & 1 deletion src/lib/Repository/Mapper/ContentDomainMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,9 @@ public function buildContentDomainObjectsOnSearchResult(SearchResult $result, ar
}

$missingContentList = [];
$contentList = $this->contentHandler->loadContentList($contentIds, array_unique($translations));
$contentList = iterator_to_array(
$this->contentHandler->loadContentList($contentIds, array_unique($translations))
);
$contentTypeList = $this->contentTypeHandler->loadContentTypeList(array_unique($contentTypeIds));
foreach ($result->searchHits as $key => $hit) {
if (isset($contentList[$hit->valueObject->id])) {
Expand Down
14 changes: 6 additions & 8 deletions src/lib/Variation/VariationHandlerRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,18 @@

use Ibexa\Contracts\Core\Exception\InvalidArgumentException;
use Ibexa\Contracts\Core\Variation\VariationHandler;
use Traversable;

final class VariationHandlerRegistry
{
/** @var iterable<string, \Ibexa\Contracts\Core\Variation\VariationHandler> */
private iterable $variationHandlers;
/** @var array<string, \Ibexa\Contracts\Core\Variation\VariationHandler> */
private array $variationHandlers;

/**
* @param iterable<string, \Ibexa\Contracts\Core\Variation\VariationHandler> $variationHandlers
*/
public function __construct(iterable $variationHandlers)
{
$handlers = $variationHandlers instanceof Traversable
? iterator_to_array($variationHandlers)
: $variationHandlers;

foreach ($handlers as $identifier => $handler) {
foreach ($variationHandlers as $identifier => $handler) {
$this->setVariationHandler($identifier, $handler);
}
}
Expand Down
38 changes: 20 additions & 18 deletions tests/integration/Core/Repository/ContentServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ public function testLoadContentInfoThrowsNotFoundException()
public function testLoadContentInfoList()
{
$mediaFolderId = $this->generateId('object', self::MEDIA_CONTENT_ID);
$list = $this->contentService->loadContentInfoList([$mediaFolderId]);
$list = iterator_to_array($this->contentService->loadContentInfoList([$mediaFolderId]));

self::assertCount(1, $list);
self::assertEquals([$mediaFolderId], array_keys($list), 'Array key was not content id');
Expand Down Expand Up @@ -867,7 +867,7 @@ public function testLoadVersionInfoByIdGetInitialLanguage(VersionInfo $versionIn
*/
public function testLoadVersionInfoByIdGetLanguages(VersionInfo $versionInfo): void
{
$actualLanguages = $versionInfo->getLanguages();
$actualLanguages = iterator_to_array($versionInfo->getLanguages());

$expectedLanguages = ['eng-US'];
foreach ($expectedLanguages as $i => $expectedLanguage) {
Expand Down Expand Up @@ -2000,12 +2000,12 @@ public function testPublishVersionFromOldContentDraftArchivesNewerVersionNo()
*
* @depends testPublishVersionFromContentDraft
*/
public function testPublishVersionNotCreatingUnlimitedArchives()
public function testPublishVersionNotCreatingUnlimitedArchives(): void
{
$content = $this->createContentVersion1();

// load first to make sure list gets updated also (cache)
$versionInfoList = $this->contentService->loadVersions($content->contentInfo);
$versionInfoList = iterator_to_array($this->contentService->loadVersions($content->contentInfo));
self::assertCount(1, $versionInfoList);
self::assertEquals(1, $versionInfoList[0]->versionNo);

Expand Down Expand Up @@ -2033,7 +2033,7 @@ public function testPublishVersionNotCreatingUnlimitedArchives()
$draftedContentVersion = $this->contentService->createContentDraft($content->contentInfo);
$this->contentService->publishVersion($draftedContentVersion->getVersionInfo());

$versionInfoList = $this->contentService->loadVersions($content->contentInfo);
$versionInfoList = iterator_to_array($this->contentService->loadVersions($content->contentInfo));

self::assertCount(6, $versionInfoList);
self::assertEquals(2, $versionInfoList[0]->versionNo);
Expand Down Expand Up @@ -2433,7 +2433,7 @@ public function testLoadContentDrafts()
$this->contentService->createContentDraft($demoDesignContentInfo);

// Now $contentDrafts should contain two drafted versions
$draftedVersions = $this->contentService->loadContentDrafts();
$draftedVersions = iterator_to_array($this->contentService->loadContentDrafts());

$actual = [
$draftedVersions[0]->status,
Expand Down Expand Up @@ -2479,8 +2479,8 @@ public function testLoadContentDraftsWithFirstParameter()
$this->permissionResolver->setCurrentUserReference($oldCurrentUser);

// Now $contentDrafts for the previous current user and the new user
$newCurrentUserDrafts = $this->contentService->loadContentDrafts($user);
$oldCurrentUserDrafts = $this->contentService->loadContentDrafts();
$newCurrentUserDrafts = iterator_to_array($this->contentService->loadContentDrafts($user));
$oldCurrentUserDrafts = iterator_to_array($this->contentService->loadContentDrafts());

self::assertSame([], $oldCurrentUserDrafts);

Expand Down Expand Up @@ -3178,7 +3178,9 @@ public function testDeleteVersion()
// Delete the previously created draft
$this->contentService->deleteVersion($draft->getVersionInfo());

$versions = $this->contentService->loadVersions($content->getVersionInfo()->getContentInfo());
$versions = iterator_to_array(
$this->contentService->loadVersions($content->getVersionInfo()->getContentInfo())
);

self::assertCount(1, $versions);
self::assertEquals(
Expand Down Expand Up @@ -3659,7 +3661,7 @@ public function testLoadRelations()
{
$draft = $this->createContentWithRelations();

$relations = $this->contentService->loadRelations($draft->getVersionInfo());
$relations = iterator_to_array($this->contentService->loadRelations($draft->getVersionInfo()));

usort(
$relations,
Expand Down Expand Up @@ -3732,7 +3734,7 @@ public function testLoadRelationsSkipsArchivedContent()
$trashService->trash($demoDesignLocation);

// Load all relations
$relations = $this->contentService->loadRelations($draft->getVersionInfo());
$relations = iterator_to_array($this->contentService->loadRelations($draft->getVersionInfo()));

self::assertCount(1, $relations);
self::assertEquals(
Expand Down Expand Up @@ -3783,7 +3785,7 @@ public function testLoadRelationsSkipsDraftContent()
$demoDesign
);

$relations = $this->contentService->loadRelations($mediaDraft->getVersionInfo());
$relations = iterator_to_array($this->contentService->loadRelations($mediaDraft->getVersionInfo()));

self::assertCount(1, $relations);
self::assertEquals(
Expand Down Expand Up @@ -3970,7 +3972,7 @@ public function testLoadReverseRelations()
$this->contentService->publishVersion($demoDesignDraft->getVersionInfo());

$relations = $this->contentService->loadRelations($versionInfo);
$reverseRelations = $this->contentService->loadReverseRelations($contentInfo);
$reverseRelations = iterator_to_array($this->contentService->loadReverseRelations($contentInfo));

self::assertEquals($contentInfo->id, $relation1->getDestinationContentInfo()->id);
self::assertEquals($mediaDraft->id, $relation1->getSourceContentInfo()->id);
Expand Down Expand Up @@ -4062,7 +4064,7 @@ public function testLoadReverseRelationsSkipsArchivedContent()

// Load all relations
$relations = $this->contentService->loadRelations($versionInfo);
$reverseRelations = $this->contentService->loadReverseRelations($contentInfo);
$reverseRelations = iterator_to_array($this->contentService->loadReverseRelations($contentInfo));

self::assertEquals($contentInfo->id, $relation1->getDestinationContentInfo()->id);
self::assertEquals($mediaDraft->id, $relation1->getSourceContentInfo()->id);
Expand Down Expand Up @@ -4126,7 +4128,7 @@ public function testLoadReverseRelationsSkipsDraftContent()
// will not be loaded as reverse relation for "Media" page

$relations = $this->contentService->loadRelations($media->versionInfo);
$reverseRelations = $this->contentService->loadReverseRelations($media->contentInfo);
$reverseRelations = iterator_to_array($this->contentService->loadReverseRelations($media->contentInfo));

self::assertEquals($media->contentInfo->id, $relation1->getDestinationContentInfo()->id);
self::assertEquals($newDraftVersionInfo->contentInfo->id, $relation1->getSourceContentInfo()->id);
Expand Down Expand Up @@ -5998,7 +6000,7 @@ public function testLoadVersionsAfterDeletingTwoDrafts()
$this->updateFolder($content, [self::ENG_GB => 'Foo3']);
$this->updateFolder($content, [self::ENG_GB => 'Foo4']);

$versions = $this->contentService->loadVersions($content->contentInfo);
$versions = iterator_to_array($this->contentService->loadVersions($content->contentInfo));

foreach ($versions as $key => $version) {
if ($version->isDraft()) {
Expand Down Expand Up @@ -6518,7 +6520,7 @@ public function testHideContentWithParentLocation()

$this->contentService->hideContent($publishedContent->contentInfo);

$locations = $this->locationService->loadLocations($publishedContent->contentInfo);
$locations = iterator_to_array($this->locationService->loadLocations($publishedContent->contentInfo));

$childContentCreate = $this->contentService->newContentCreateStruct($contentType, self::ENG_US);
$childContentCreate->setField('name', 'Child');
Expand All @@ -6534,7 +6536,7 @@ public function testHideContentWithParentLocation()

$publishedChildContent = $this->contentService->publishVersion($childContent->versionInfo);

$childLocations = $this->locationService->loadLocations($publishedChildContent->contentInfo);
$childLocations = iterator_to_array($this->locationService->loadLocations($publishedChildContent->contentInfo));

self::assertTrue($locations[0]->hidden);
self::assertTrue($locations[0]->invisible);
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/Core/Repository/LanguageServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public function testLoadLanguageById()
$language
);

$languages = $languageService->loadLanguageListById([$languageId]);
$languages = iterator_to_array($languageService->loadLanguageListById([$languageId]));

self::assertIsIterable($languages);
self::assertCount(1, $languages);
Expand Down Expand Up @@ -377,7 +377,7 @@ public function testLoadLanguage()
$language
);

$languages = $languageService->loadLanguageListByCode(['eng-NZ']);
$languages = iterator_to_array($languageService->loadLanguageListByCode(['eng-NZ']));

self::assertIsIterable($languages);
self::assertCount(1, $languages);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ public function testLoadLocationsNoAccess()
$editorGroupContentInfo = $repository->getContentService()->loadContentInfo($editorsGroupId);

// this should return one location for admin
$locations = $locationService->loadLocations($editorGroupContentInfo);
$locations = iterator_to_array($locationService->loadLocations($editorGroupContentInfo));
self::assertCount(1, $locations);
self::assertInstanceOf(Location::class, $locations[0]);

Expand Down
Loading

0 comments on commit 3278641

Please sign in to comment.