IBX-7911: Used strict getters for content tree loading code paths (#347)

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

Key changes:

* Introduced dedicated strict getters and deprecated magic ones for Content, ContentInfo, Location, VersionInfo, ContentType\FieldDefinition, User\User, Security\User, and SimplifiedRequest Value Object properties.

* Replaced usages of magic getters with strict ones relevant to the issue

* [Tests] Added coverage for strict getters

* [Tests] Aligned tests with the changes

* [Tests] Introduced symfony/phpunit-bridge deprecations helper

* [Tests] Added coverage for the new deprecation message

* [PHPStan] Aligned baseline with the changes

* [PHPStan] Updated baseline after PHPStan release

---------

Co-authored-by: Paweł Niedzielski <[email protected]>
Co-authored-by: Konrad Oboza <[email protected]>

Author: 3 people

composer.json

@@ -69,7 +69,7 @@
         "ibexa/code-style": "^1.0",
         "phpunit/phpunit": "^8.2",
         "matthiasnoback/symfony-dependency-injection-test": "^4.1",
-        "symfony/phpunit-bridge": "^5.1",
+        "symfony/phpunit-bridge": "^5.4",
         "symfony/proxy-manager-bridge": "^5.3",
         "symfony/runtime": "^5.3.0",
         "phpstan/phpstan": "^1.2",

phpstan-baseline.neon

@@ -21,6 +21,7 @@
         <env name="KERNEL_CLASS" value="Ibexa\Contracts\Solr\Test\IbexaSolrTestKernel"/>
         <env name="SEARCH_ENGINE" value="solr"/>
         <env name="DATABASE_URL" value="sqlite://:memory:" />
+        <env name="SYMFONY_DEPRECATIONS_HELPER" value="disabled"/>
     </php>
     <testsuites>
         <!-- Search service is used all over the place, so we must run entire integration test suite -->

phpunit-integration-legacy-solr.xml

@@ -19,6 +19,7 @@
         <env name="DATABASE_URL" value="sqlite://:memory:" />
         <env name="KERNEL_CLASS" value="Ibexa\Contracts\Core\Test\IbexaTestKernel"/>
         <env name="SEARCH_ENGINE" value="legacy"/>
+        <env name="SYMFONY_DEPRECATIONS_HELPER" value="disabled"/>
     </php>
     <testsuites>
         <testsuite name="integration_core">

phpunit-integration-legacy.xml

@@ -11,7 +11,7 @@
   >
   <php>
     <ini name="error_reporting" value="-1" />
-    <env name="SYMFONY_DEPRECATIONS_HELPER" value="disabled"/>
+    <env name="SYMFONY_DEPRECATIONS_HELPER" value="max[total]=576&amp;verbose=0"/>
   </php>
   <testsuites>
     <testsuite name="unit_core">
@@ -33,4 +33,7 @@
       <directory>tests/bundle/LegacySearchEngineBundle</directory>
     </testsuite>
   </testsuites>
+  <listeners>
+    <listener class="Symfony\Bridge\PhpUnit\SymfonyTestsListener"/>
+  </listeners>
 </phpunit>

phpunit.xml

@@ -15,14 +15,24 @@
 /**
  * this class represents a content object in a specific version.
  *
- * @property-read \Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo $contentInfo convenience getter for getVersionInfo()->getContentInfo()
- * @property-read int $id convenience getter for retrieving the contentId: $versionInfo->contentInfo->id
+ * @property-read \Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo $contentInfo @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Content::getContentInfo()} instead.
+ * @property-read int $id @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Content::getId()} instead.
  * @property-read \Ibexa\Contracts\Core\Repository\Values\Content\VersionInfo $versionInfo calls getVersionInfo()
  * @property-read \Ibexa\Contracts\Core\Repository\Values\Content\Field[] $fields access fields, calls getFields()
  * @property-read \Ibexa\Contracts\Core\Repository\Values\Content\Thumbnail|null $thumbnail calls getThumbnail()
  */
 abstract class Content extends ValueObject
 {
+    public function getId(): int
+    {
+        return $this->getContentInfo()->getId();
+    }
+
+    public function getContentInfo(): ContentInfo
+    {
+        return $this->getVersionInfo()->getContentInfo();
+    }
+
     /**
      * Returns the VersionInfo for this version.
      *

src/contracts/Repository/Values/Content/Content.php

@@ -15,9 +15,9 @@
 /**
  * This class provides all version independent information of the Content object.
  *
- * @property-read int $id @deprecated Use {@see ContentInfo::getId} instead. The unique id of the Content object
+ * @property-read int $id @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see ContentInfo::getId()} instead.
  * @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 string $name @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see ContentInfo::getName()} instead.
  * @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
@@ -29,7 +29,7 @@
  * @property-read string $mainLanguageCode The main language code of the Content object. If the available flag is set to true the Content is shown in this language if the requested language does not exist.
  * @property-read int|null $mainLocationId @deprecated Use {@see ContentInfo::getMainLocationId} instead
  * @property-read int $status status of the Content object
- * @property-read bool $isHidden status of the Content object
+ * @property-read bool $isHidden @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see ContentInfo::isHidden()} instead.
  */
 class ContentInfo extends ValueObject
 {
@@ -185,6 +185,11 @@ public function isTrashed(): bool
         return $this->status === self::STATUS_TRASHED;
     }
 
+    public function isHidden(): bool
+    {
+        return $this->isHidden;
+    }
+
     public function getContentType(): ContentType
     {
         return $this->contentType;
@@ -229,6 +234,11 @@ public function getId(): int
     {
         return $this->id;
     }
+
+    public function getName(): string
+    {
+        return $this->name;
+    }
 }
 
 class_alias(ContentInfo::class, 'eZ\Publish\API\Repository\Values\Content\ContentInfo');

src/contracts/Repository/Values/Content/ContentInfo.php

@@ -16,17 +16,17 @@
  * This class represents a location in the repository.
  *
  * @property-read \Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo $contentInfo calls getContentInfo()
- * @property-read int $contentId calls getContentInfo()->id
- * @property-read int $id the id of the location
+ * @property-read int $contentId @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getContentId()} instead.
+ * @property-read int $id @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getId()} instead.
  * @property-read int $priority Position of the Location among its siblings when sorted using priority
- * @property-read bool $hidden Indicates that the Location entity is hidden (explicitly or hidden by content).
- * @property-read bool $invisible Indicates that the Location is not visible, being either marked as hidden itself, or implicitly hidden by its Content or an ancestor Location
+ * @property-read bool $hidden @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::isHidden()} instead.
+ * @property-read bool $invisible @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::isInvisible()} instead.
  * @property-read bool $explicitlyHidden Indicates that the Location entity has been explicitly marked as hidden.
  * @property-read string $remoteId a global unique id of the content object
  * @property-read int $parentLocationId the id of the parent location
- * @property-read string $pathString @deprecated use {@see Location::getPathString()} instead.
- * @property-read array $path Same as $pathString but as array, e.g. [ 1, 2, 4, 23 ]
- * @property-read int $depth Depth location has in the location tree
+ * @property-read string $pathString @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getPathString()} instead.
+ * @property-read array $path @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getPath()} instead.
+ * @property-read int $depth @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getDepth()} instead.
  * @property-read int $sortField Specifies which property the child locations should be sorted on. Valid values are found at {@link Location::SORT_FIELD_*}
  * @property-read int $sortOrder Specifies whether the sort order should be ascending or descending. Valid values are {@link Location::SORT_ORDER_*}
  */
@@ -158,6 +158,13 @@ abstract class Location extends ValueObject
      */
     protected $pathString;
 
+    /**
+     * Same as {@see Location::$pathString} but as array, e.g.: <code>[ '1', '2', '4', '23' ]</code>.
+     *
+     * @var string[]
+     */
+    protected array $path;
+
     /**
      * Depth location has in the location tree.
      *
@@ -194,12 +201,22 @@ abstract class Location extends ValueObject
     abstract public function getContentInfo(): ContentInfo;
 
     /**
-     * Return the parent location of of this location.
+     * Return the parent location of this location.
      *
      * @return \Ibexa\Contracts\Core\Repository\Values\Content\Location|null
      */
     abstract public function getParentLocation(): ?Location;
 
+    public function getId(): int
+    {
+        return $this->id;
+    }
+
+    public function getContentId(): int
+    {
+        return $this->getContentInfo()->getId();
+    }
+
     /**
      * Returns true if current location is a draft.
      *
@@ -248,6 +265,62 @@ public function getPathString(): string
     {
         return $this->pathString;
     }
+
+    /**
+     * Same as {@see Location::getPathString()} but as array, e.g.: <code>[ '1', '2', '4', '23' ]</code>.
+     *
+     * @return string[]
+     */
+    public function getPath(): array
+    {
+        if (isset($this->path)) {
+            return $this->path;
+        }
+
+        $pathString = trim($this->pathString ?? '', '/');
+
+        return $this->path = !empty($pathString) ? explode('/', $pathString) : [];
+    }
+
+    /**
+     * Indicates that the Location is not visible, being either marked as hidden itself, or implicitly hidden by
+     * its Content or an ancestor Location.
+     */
+    public function isInvisible(): bool
+    {
+        return $this->invisible;
+    }
+
+    /**
+     * Indicates that the Location is hidden either explicitly or by content.
+     */
+    public function isHidden(): bool
+    {
+        return $this->hidden;
+    }
+
+    public function getDepth(): int
+    {
+        return $this->depth;
+    }
+
+    public function __isset($property)
+    {
+        if ($property === 'path') {
+            return true;
+        }
+
+        return parent::__isset($property);
+    }
+
+    public function __get($property)
+    {
+        if ($property === 'path') {
+            return $this->getPath();
+        }
+
+        return parent::__get($property);
+    }
 }
 
 class_alias(Location::class, 'eZ\Publish\API\Repository\Values\Content\Location');

src/contracts/Repository/Values/Content/Location.php

@@ -18,7 +18,7 @@
  *
  * @property-read \Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo $contentInfo calls getContentInfo()
  * @property-read mixed $id the internal id of the version
- * @property-read int $versionNo the version number of this version (which only increments in scope of a single Content object)
+ * @property-read int $versionNo @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see VersionInfo::getVersionNo()} instead.
  * @property-read \DateTime $modificationDate the last modified date of this version
  * @property-read \DateTime $creationDate the creation date of this version
  * @property-read mixed $creatorId the user id of the user which created this version
@@ -115,6 +115,11 @@ public function getLanguageCodes(): array
         return $this->languageCodes;
     }
 
+    public function getVersionNo(): int
+    {
+        return $this->versionNo;
+    }
+
     /**
      * Returns true if version is a draft.
      *

src/contracts/Repository/Values/Content/VersionInfo.php

@@ -19,7 +19,7 @@
  * @property-read \Ibexa\Contracts\Core\Repository\Values\ContentType\FieldDefinitionCollection $fieldDefinitions calls getFieldDefinitions() or on access getFieldDefinition($fieldDefIdentifier)
  * @property-read mixed $id the id of the content type
  * @property-read int $status the status of the content type. One of ContentType::STATUS_DEFINED|ContentType::STATUS_DRAFT|ContentType::STATUS_MODIFIED
- * @property-read string $identifier the identifier of the content type
+ * @property-read string $identifier @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see ContentType::getIdentifier()} instead.
  * @property-read \DateTime $creationDate the date of the creation of this content type
  * @property-read \DateTime $modificationDate the date of the last modification of this content type
  * @property-read mixed $creatorId the user id of the creator of this content type
@@ -176,6 +176,11 @@ abstract public function getContentTypeGroups();
      */
     abstract public function getFieldDefinitions(): FieldDefinitionCollection;
 
+    public function getIdentifier(): string
+    {
+        return $this->identifier;
+    }
+
     /**
      * This method returns the field definition for the given identifier.
      *