IBX-7959: Added ContentInfo::getSectionId strict getter (#348)

For more details see #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 <alongosz@users.noreply.github.com>
ibexa · Mar 19, 2024 · 1b54380 · 1b54380
1 parent 926bcc3
commit 1b54380
Show file tree

Hide file tree

Showing 10 changed files with 62 additions and 53 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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());
     }
 }