From 1b543804f95d8e1b23dc05e76b7bb12e4063ab87 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Tue, 19 Mar 2024 15:15:28 +0100 Subject: [PATCH] IBX-7959: Added `ContentInfo::getSectionId` strict getter (#348) For more details see https://github.com/ibexa/core/pull/348 Key changes: * Added `ContentInfo::getSectionId` strict getter * Deprecated `ContentInfo::$sectionId` property-read * Added strict type for `ContentInfo::$sectionId` property * Replaced usages of property getter `sectionId` with strict getter --------- Co-authored-by: Andrew Longosz --- phpstan-baseline.neon | 10 --- .../Repository/Values/Content/ContentInfo.php | 9 ++- src/lib/Limitation/SectionLimitationType.php | 8 ++- .../Matcher/ContentBased/Id/Section.php | 6 +- .../ContentBased/Identifier/Section.php | 6 +- src/lib/Repository/ContentService.php | 4 +- .../Core/Repository/ContentServiceTest.php | 4 +- .../Repository/SearchEngineIndexingTest.php | 5 +- .../Limitation/NewSectionLimitationTest.php | 2 +- .../Values/Content/ContentInfoTest.php | 61 +++++++++++-------- 10 files changed, 62 insertions(+), 53 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index a63ae6ce76..14c6dec3f1 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -36530,11 +36530,6 @@ parameters: count: 1 path: tests/integration/Core/Repository/SearchEngineIndexingTest.php - - - message: "#^Access to an undefined property Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\ValueObject\\:\\:\\$sectionId\\.$#" - count: 1 - path: tests/integration/Core/Repository/SearchEngineIndexingTest.php - - message: "#^Access to an undefined property Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\ValueObject\\:\\:\\$versionInfo\\.$#" count: 2 @@ -61480,11 +61475,6 @@ parameters: count: 1 path: tests/lib/Repository/Validator/TargetContentValidatorTest.php - - - message: "#^Method Ibexa\\\\Tests\\\\Core\\\\Repository\\\\Values\\\\Content\\\\ContentInfoTest\\:\\:testObjectProperties\\(\\) has no return type specified\\.$#" - count: 1 - path: tests/lib/Repository/Values/Content/ContentInfoTest.php - - message: "#^Access to an undefined property Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\Language\\:\\:\\$notDefined\\.$#" count: 1 diff --git a/src/contracts/Repository/Values/Content/ContentInfo.php b/src/contracts/Repository/Values/Content/ContentInfo.php index 163ae822e7..6a7a4e13c9 100644 --- a/src/contracts/Repository/Values/Content/ContentInfo.php +++ b/src/contracts/Repository/Values/Content/ContentInfo.php @@ -18,7 +18,7 @@ * @property-read int $id @deprecated Use {@see ContentInfo::getId} instead. The unique id of the Content object * @property-read int $contentTypeId The unique id of the content type item the Content object is an instance of * @property-read string $name the computed name (via name schema) in the main language of the Content object - * @property-read int $sectionId the section to which the Content object is assigned + * @property-read int $sectionId @deprecated 4.6.2 Use {@see ContentInfo::getSectionId} instead. The section to which the Content object is assigned * @property-read int $currentVersionNo Current Version number is the version number of the published version or the version number of a newly created draft (which is 1). * @property-read bool $published true if there exists a published version false otherwise * @property-read int $ownerId the user id of the owner of the Content object @@ -65,7 +65,7 @@ class ContentInfo extends ValueObject * * @var int */ - protected $sectionId; + protected int $sectionId; /** * Current Version number is the version number of the published version or the version number of @@ -195,6 +195,11 @@ public function getSection(): Section return $this->section; } + public function getSectionId(): int + { + return $this->sectionId; + } + public function getMainLanguage(): Language { return $this->mainLanguage; diff --git a/src/lib/Limitation/SectionLimitationType.php b/src/lib/Limitation/SectionLimitationType.php index 5b8588d9f3..99769e3958 100644 --- a/src/lib/Limitation/SectionLimitationType.php +++ b/src/lib/Limitation/SectionLimitationType.php @@ -139,9 +139,13 @@ public function evaluate(APILimitationValue $value, APIUserReference $currentUse * We ignore Targets here, they are only interesting in NewState limitation as we on this one is more interested * the section already assigned to object. * - * @var $object ContentInfo|ContentCreateStruct + * We can't use strict comparison because limitationValues is an array of string */ - return in_array($object->sectionId, $value->limitationValues); + if ($object instanceof ContentCreateStruct) { + return in_array($object->sectionId, $value->limitationValues); + } + + return in_array($object->getSectionId(), $value->limitationValues); } /** diff --git a/src/lib/MVC/Symfony/Matcher/ContentBased/Id/Section.php b/src/lib/MVC/Symfony/Matcher/ContentBased/Id/Section.php index d0118196b5..6573b56ea7 100644 --- a/src/lib/MVC/Symfony/Matcher/ContentBased/Id/Section.php +++ b/src/lib/MVC/Symfony/Matcher/ContentBased/Id/Section.php @@ -23,7 +23,7 @@ class Section extends MultipleValued */ public function matchLocation(APILocation $location) { - return isset($this->values[$location->getContentInfo()->sectionId]); + return isset($this->values[$location->getContentInfo()->getSectionId()]); } /** @@ -35,7 +35,7 @@ public function matchLocation(APILocation $location) */ public function matchContentInfo(ContentInfo $contentInfo) { - return isset($this->values[$contentInfo->sectionId]); + return isset($this->values[$contentInfo->getSectionId()]); } public function match(View $view) @@ -44,7 +44,7 @@ public function match(View $view) return false; } - return isset($this->values[$view->getContent()->contentInfo->sectionId]); + return isset($this->values[$view->getContent()->contentInfo->getSectionId()]); } } diff --git a/src/lib/MVC/Symfony/Matcher/ContentBased/Identifier/Section.php b/src/lib/MVC/Symfony/Matcher/ContentBased/Identifier/Section.php index 1525723d10..97cff3130d 100644 --- a/src/lib/MVC/Symfony/Matcher/ContentBased/Identifier/Section.php +++ b/src/lib/MVC/Symfony/Matcher/ContentBased/Identifier/Section.php @@ -27,7 +27,7 @@ public function matchLocation(Location $location) $section = $this->repository->sudo( static function (Repository $repository) use ($location) { return $repository->getSectionService()->loadSection( - $location->getContentInfo()->sectionId + $location->getContentInfo()->getSectionId() ); } ); @@ -47,7 +47,7 @@ public function matchContentInfo(ContentInfo $contentInfo) $section = $this->repository->sudo( static function (Repository $repository) use ($contentInfo) { return $repository->getSectionService()->loadSection( - $contentInfo->sectionId + $contentInfo->getSectionId() ); } ); @@ -65,7 +65,7 @@ public function match(View $view) $section = $this->repository->sudo( static function (Repository $repository) use ($contentInfo) { return $repository->getSectionService()->loadSection( - $contentInfo->sectionId + $contentInfo->getSectionId() ); } ); diff --git a/src/lib/Repository/ContentService.php b/src/lib/Repository/ContentService.php index b02bc75a8f..e355bc343f 100644 --- a/src/lib/Repository/ContentService.php +++ b/src/lib/Repository/ContentService.php @@ -632,7 +632,7 @@ public function createContent(APIContentCreateStruct $contentCreateStruct, array $location = $this->repository->getLocationService()->loadLocation( $locationCreateStructs[0]->parentLocationId ); - $contentCreateStruct->sectionId = $location->contentInfo->sectionId; + $contentCreateStruct->sectionId = $location->getContentInfo()->getSectionId(); } else { $contentCreateStruct->sectionId = 1; } @@ -1945,7 +1945,7 @@ public function copyContent(ContentInfo $contentInfo, LocationCreateStruct $dest 'create', [ 'parentLocationId' => $destinationLocationCreateStruct->parentLocationId, - 'sectionId' => $contentInfo->sectionId, + 'sectionId' => $contentInfo->getSectionId(), ] ); } diff --git a/tests/integration/Core/Repository/ContentServiceTest.php b/tests/integration/Core/Repository/ContentServiceTest.php index ad720a89b9..b7eb36778c 100644 --- a/tests/integration/Core/Repository/ContentServiceTest.php +++ b/tests/integration/Core/Repository/ContentServiceTest.php @@ -1365,7 +1365,7 @@ public function testCreateContentDraftSetsContentInfo($draft) $contentInfo->mainLanguageCode, $contentInfo->ownerId, $contentInfo->remoteId, - $contentInfo->sectionId, + $contentInfo->getSectionId(), ] ); } @@ -2071,7 +2071,7 @@ public function testUpdateContentMetadataSetsExpectedProperties($content) ], [ 'remoteId' => $contentInfo->remoteId, - 'sectionId' => $contentInfo->sectionId, + 'sectionId' => $contentInfo->getSectionId(), 'alwaysAvailable' => $contentInfo->alwaysAvailable, 'currentVersionNo' => $contentInfo->currentVersionNo, 'mainLanguageCode' => $contentInfo->mainLanguageCode, diff --git a/tests/integration/Core/Repository/SearchEngineIndexingTest.php b/tests/integration/Core/Repository/SearchEngineIndexingTest.php index fc71962f8b..8fe3092741 100644 --- a/tests/integration/Core/Repository/SearchEngineIndexingTest.php +++ b/tests/integration/Core/Repository/SearchEngineIndexingTest.php @@ -1112,7 +1112,10 @@ public function testAssignSection() $criterion = new Criterion\ContentId($content->id); $query = new Query(['filter' => $criterion]); $results = $searchService->findContentInfo($query); - $this->assertEquals($section->id, $results->searchHits[0]->valueObject->sectionId); + + /** @var \Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo $contentInfo */ + $contentInfo = $results->searchHits[0]->valueObject; + self::assertEquals($section->id, $contentInfo->getSectionId()); } /** diff --git a/tests/integration/Core/Repository/Values/User/Limitation/NewSectionLimitationTest.php b/tests/integration/Core/Repository/Values/User/Limitation/NewSectionLimitationTest.php index 94bc19ed91..e5f4d1e6a4 100644 --- a/tests/integration/Core/Repository/Values/User/Limitation/NewSectionLimitationTest.php +++ b/tests/integration/Core/Repository/Values/User/Limitation/NewSectionLimitationTest.php @@ -63,7 +63,7 @@ public function testNewSectionLimitationAllow() $this->assertSame( $sectionId, - $contentService->loadContentInfo($contentId)->sectionId + $contentService->loadContentInfo($contentId)->getSectionId() ); } diff --git a/tests/lib/Repository/Values/Content/ContentInfoTest.php b/tests/lib/Repository/Values/Content/ContentInfoTest.php index 29ca2f9bb1..18fecfb249 100644 --- a/tests/lib/Repository/Values/Content/ContentInfoTest.php +++ b/tests/lib/Repository/Values/Content/ContentInfoTest.php @@ -6,6 +6,7 @@ */ namespace Ibexa\Tests\Core\Repository\Values\Content; +use DateTimeImmutable; use Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo; use PHPUnit\Framework\TestCase; @@ -14,35 +15,41 @@ */ class ContentInfoTest extends TestCase { - public function testObjectProperties() + public function testCreateObject(): void { - $object = new ContentInfo(); - $properties = $object->attributes(); - self::assertNotContains('internalFields', $properties, 'Internal property found '); - self::assertContains('contentTypeId', $properties, 'Property not found'); - self::assertContains('id', $properties, 'Property not found'); - self::assertContains('name', $properties, 'Property not found'); - self::assertContains('sectionId', $properties, 'Property not found'); - self::assertContains('currentVersionNo', $properties, 'Property not found'); - self::assertContains('published', $properties, 'Property not found'); - self::assertContains('ownerId', $properties, 'Property not found'); - self::assertContains('modificationDate', $properties, 'Property not found'); - self::assertContains('publishedDate', $properties, 'Property not found'); - self::assertContains('alwaysAvailable', $properties, 'Property not found'); - self::assertContains('remoteId', $properties, 'Property not found'); - self::assertContains('mainLanguageCode', $properties, 'Property not found'); - self::assertContains('mainLocationId', $properties, 'Property not found'); + $dateTime = new DateTimeImmutable(); + $contentInfo = new ContentInfo( + [ + 'id' => 1, + 'contentTypeId' => 2, + 'name' => 'foo', + 'sectionId' => 1, + 'currentVersionNo' => 1, + 'status' => 1, + 'ownerId' => 10, + 'modificationDate' => $dateTime, + 'publishedDate' => $dateTime, + 'alwaysAvailable' => false, + 'remoteId' => '1qaz2wsx', + 'mainLanguageCode' => 'eng-GB', + 'mainLocationId' => 2, + ] + ); - // check for duplicates and double check existence of property - $propertiesHash = []; - foreach ($properties as $property) { - if (isset($propertiesHash[$property])) { - self::fail("Property '{$property}' exists several times in properties list"); - } elseif (!isset($object->$property)) { - self::fail("Property '{$property}' does not exist on object, even though it was hinted to be there"); - } - $propertiesHash[$property] = 1; - } + $dateFormatted = $dateTime->format('c'); + self::assertSame(1, $contentInfo->getId()); + self::assertSame(2, $contentInfo->contentTypeId); + self::assertSame('foo', $contentInfo->name); + self::assertSame(1, $contentInfo->getSectionId()); + self::assertSame(1, $contentInfo->currentVersionNo); + self::assertTrue($contentInfo->isPublished()); + self::assertSame(10, $contentInfo->ownerId); + self::assertSame($dateFormatted, $contentInfo->modificationDate->format('c')); + self::assertSame($dateFormatted, $contentInfo->publishedDate->format('c')); + self::assertFalse($contentInfo->alwaysAvailable); + self::assertSame('1qaz2wsx', $contentInfo->remoteId); + self::assertSame('eng-GB', $contentInfo->getMainLanguageCode()); + self::assertSame(2, $contentInfo->getMainLocationId()); } }