Removed unnecessary docblock, set deprecated for loadRelations functi…

…on, Corrected CS
ibexa · Apr 19, 2024 · 8c1e042 · 8c1e042
1 parent 6743a84
commit 8c1e042
Show file tree

Hide file tree

Showing 13 changed files with 127 additions and 174 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -16187,7 +16187,7 @@ parameters:
 
 		-
 			message: "#^Cannot call method fetchAll\\(\\) on Doctrine\\\\DBAL\\\\ForwardCompatibility\\\\Result\\|int\\|string\\.$#"
-			count: 17
+			count: 16
 			path: src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php
 
 		-

diff --git a/src/contracts/Persistence/Content/Handler.php b/src/contracts/Persistence/Content/Handler.php
@@ -308,8 +308,8 @@ public function countRelations(
      */
     public function loadRelationList(
         int $sourceContentId,
+        int $limit,
         int $offset = 0,
-        ?int $limit = null,
         ?int $sourceContentVersionNo = null,
         ?int $type = null
     ): array;

diff --git a/src/contracts/Repository/ContentService.php b/src/contracts/Repository/ContentService.php
@@ -30,6 +30,8 @@
  */
 interface ContentService
 {
+    public const DEFAULT_PAGE_SIZE = 25;
+
     /**
      * Loads a content info object.
      *
@@ -398,7 +400,7 @@ public function copyContent(ContentInfo $contentInfo, LocationCreateStruct $dest
      *
      * @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException if the user is not allowed to read this version
      *
-     * @param \Ibexa\Contracts\Core\Repository\Values\Content\VersionInfo $versionInfo
+     * @deprecated since 4.5, use loadRelationList().
      *
      * @return \Ibexa\Contracts\Core\Repository\Values\Content\Relation[]
      */
@@ -408,9 +410,14 @@ public function loadRelations(VersionInfo $versionInfo): iterable;
      * Loads all outgoing relations for the given version.
      *
      * If the user is not allowed to read specific version then UnauthorizedRelationListItem is returned
-     * {@link \Ibexa\Contracts\Core\Repository\Values\Content\RelationList\Item\UnauthorizedRelationListItem}
+     *
+     * @see \Ibexa\Contracts\Core\Repository\Values\Content\RelationList\Item\UnauthorizedRelationListItem
      */
-    public function loadRelationList(VersionInfo $versionInfo, int $offset = 0, ?int $limit = null): RelationList;
+    public function loadRelationList(
+        VersionInfo $versionInfo,
+        int $offset = 0,
+        int $limit = self::DEFAULT_PAGE_SIZE
+    ): RelationList;
 
     /**
      * Counts all outgoing relations for the given version.

diff --git a/src/contracts/Repository/Decorator/ContentServiceDecorator.php b/src/contracts/Repository/Decorator/ContentServiceDecorator.php
@@ -198,7 +198,7 @@ public function countRelations(VersionInfo $versionInfo): int
         return $this->innerService->countRelations($versionInfo);
     }
 
-    public function loadRelationList(VersionInfo $versionInfo, int $offset = 0, ?int $limit = null): RelationList
+    public function loadRelationList(VersionInfo $versionInfo, int $offset = 0, int $limit = self::DEFAULT_PAGE_SIZE): RelationList
     {
         return $this->innerService->loadRelationList($versionInfo, $offset, $limit);
     }

diff --git a/src/lib/Persistence/Cache/ContentHandler.php b/src/lib/Persistence/Cache/ContentHandler.php
@@ -32,6 +32,9 @@ class ContentHandler extends AbstractInMemoryPersistenceHandler implements Conte
     private const CONTENT_VERSION_INFO_IDENTIFIER = 'content_version_info';
     private const CONTENT_VERSION_IDENTIFIER = 'content_version';
     private const CONTENT_RELATIONS_COUNT_IDENTIFIER = 'content_relations_count';
+    private const CONTENT_RELATIONS_COUNT_WITH_TYPE_IDENTIFIER = 'content_relations_count_with_type';
+    private const CONTENT_RELATIONS_COUNT_WITH_TYPE_AND_VERSION_IDENTIFIER = 'content_relations_count_with_type_and_version';
+    private const CONTENT_RELATIONS_COUNT_WITH_VERSION_IDENTIFIER = 'content_relations_count_with_version';
     private const CONTENT_REVERSE_RELATIONS_COUNT_IDENTIFIER = 'content_reverse_relations_count';
     private const RELATION_IDENTIFIER = 'relation';
 
@@ -520,10 +523,24 @@ public function loadRelations($sourceContentId, $sourceContentVersionNo = null,
 
     public function countRelations(int $sourceContentId, ?int $sourceContentVersionNo = null, ?int $type = null): int
     {
+        $values[] = $sourceContentId;
+        if ($sourceContentVersionNo === null && $type !== null) {
+            $patternName = self::CONTENT_RELATIONS_COUNT_WITH_TYPE_IDENTIFIER;
+            $values[] = $type;
+        } elseif ($sourceContentVersionNo !== null && $type === null) {
+            $patternName = self::CONTENT_RELATIONS_COUNT_WITH_VERSION_IDENTIFIER;
+            $values[] = $sourceContentVersionNo;
+        } elseif ($sourceContentVersionNo !== null && $type !== null) {
+            $patternName = self::CONTENT_RELATIONS_COUNT_WITH_TYPE_AND_VERSION_IDENTIFIER;
+            $values = array_merge($values, [$type, $sourceContentVersionNo]);
+        } else {
+            $patternName = self::CONTENT_RELATIONS_COUNT_IDENTIFIER;
+        }
+
         $cacheItem = $this->cache->getItem(
             $this->cacheIdentifierGenerator->generateKey(
-                self::CONTENT_RELATIONS_COUNT_IDENTIFIER,
-                [$sourceContentId],
+                $patternName,
+                $values,
                 true
             )
         );
@@ -559,8 +576,8 @@ public function countRelations(int $sourceContentId, ?int $sourceContentVersionN
      */
     public function loadRelationList(
         int $sourceContentId,
+        int $limit,
         int $offset = 0,
-        ?int $limit = null,
         ?int $sourceContentVersionNo = null,
         ?int $type = null
     ): array {
@@ -574,8 +591,8 @@ public function loadRelationList(
 
         return $this->persistenceHandler->contentHandler()->loadRelationList(
             $sourceContentId,
-            $offset,
             $limit,
+            $offset,
             $sourceContentVersionNo,
             $type
         );

diff --git a/src/lib/Persistence/Legacy/Content/Gateway.php b/src/lib/Persistence/Legacy/Content/Gateway.php
@@ -376,8 +376,8 @@ abstract public function countRelations(
      */
     abstract public function listRelations(
         int $contentId,
+        int $limit,
         int $offset = 0,
-        ?int $limit = null,
         ?int $contentVersionNo = null,
         ?int $relationType = null
     ): array;

diff --git a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php
@@ -1387,62 +1387,9 @@ public function loadRelations(
         ?int $relationType = null
     ): array {
         $query = $this->queryBuilder->createRelationFindQueryBuilder();
-        $expr = $query->expr();
-        $query
-            ->innerJoin(
-                'l',
-                'ezcontentobject',
-                'ezcontentobject_to',
-                $expr->andX(
-                    'l.to_contentobject_id = ezcontentobject_to.id',
-                    'ezcontentobject_to.status = :status'
-                )
-            )
-            ->where(
-                'l.from_contentobject_id = :content_id'
-            )
-            ->setParameter(
-                'status',
-                ContentInfo::STATUS_PUBLISHED,
-                ParameterType::INTEGER
-            )
-            ->setParameter('content_id', $contentId, ParameterType::INTEGER);
-
-        // source version number
-        if (null !== $contentVersionNo) {
-            $query
-                ->andWhere('l.from_contentobject_version = :version_no')
-                ->setParameter('version_no', $contentVersionNo, ParameterType::INTEGER);
-        } else {
-            // from published version only
-            $query
-                ->innerJoin(
-                    'ezcontentobject_to',
-                    'ezcontentobject',
-                    'c',
-                    $expr->andX(
-                        'c.id = l.from_contentobject_id',
-                        'c.current_version = l.from_contentobject_version'
-                    )
-                );
-        }
-
-        // relation type
-        if (null !== $relationType) {
-            $query
-                ->andWhere(
-                    $expr->gt(
-                        $this->databasePlatform->getBitAndComparisonExpression(
-                            'l.relation_type',
-                            ':relation_type'
-                        ),
-                        0
-                    )
-                )
-                ->setParameter('relation_type', $relationType, ParameterType::INTEGER);
-        }
+        $query = $this->prepareRelationQuery($query, $contentId, $contentVersionNo, $relationType);
 
-        return $query->execute()->fetchAll(FetchMode::ASSOCIATIVE);
+        return $query->execute()->fetchAllAssociative();
     }
 
     public function countRelations(
@@ -1451,89 +1398,50 @@ public function countRelations(
         ?int $relationType = null
     ): int {
         $query = $this->connection->createQueryBuilder();
-        $expr = $query->expr();
-        $query
-            ->select($this->databasePlatform->getCountExpression('l.id'))
-            ->from(self::CONTENT_RELATION_TABLE, 'l')
-            ->innerJoin(
-                'l',
-                'ezcontentobject',
-                'ezcontentobject_to',
-                $expr->and(
-                    'l.to_contentobject_id = ezcontentobject_to.id',
-                    'ezcontentobject_to.status = :status'
-                )
-            )
-            ->andWhere(
-                'l.from_contentobject_id = :content_id'
-            )
-            ->setParameter(
-                'status',
-                ContentInfo::STATUS_PUBLISHED,
-                ParameterType::INTEGER
-            )
-            ->setParameter('content_id', $contentId, ParameterType::INTEGER);
-
-        // source version number
-        if (null !== $contentVersionNo) {
-            $query
-                ->andWhere('l.from_contentobject_version = :version_no')
-                ->setParameter('version_no', $contentVersionNo, ParameterType::INTEGER);
-        } else {
-            // from published version only
-            $query
-                ->innerJoin(
-                    'ezcontentobject_to',
-                    'ezcontentobject',
-                    'c',
-                    $expr->and(
-                        'c.id = l.from_contentobject_id',
-                        'c.current_version = l.from_contentobject_version'
-                    )
-                );
-        }
+        $query->select($this->databasePlatform->getCountExpression('l.id'))
+            ->from(self::CONTENT_RELATION_TABLE, 'l');
 
-        // relation type
-        if (null !== $relationType) {
-            $query
-                ->andWhere(
-                    $expr->gt(
-                        $this->databasePlatform->getBitAndComparisonExpression(
-                            'l.relation_type',
-                            ':relation_type'
-                        ),
-                        0
-                    )
-                )
-                ->setParameter('relation_type', $relationType, ParameterType::INTEGER);
-        }
+        $query = $this->prepareRelationQuery($query, $contentId, $contentVersionNo, $relationType);
 
         return (int)$query->execute()->fetchOne();
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public function listRelations(
         int $contentId,
+        int $limit,
         int $offset = 0,
-        ?int $limit = null,
         ?int $contentVersionNo = null,
         ?int $relationType = null
     ): array {
         $query = $this->queryBuilder->createRelationFindQueryBuilder();
+        $query = $this->prepareRelationQuery($query, $contentId, $contentVersionNo, $relationType);
+
+        $query->setFirstResult($offset)
+            ->setMaxResults($limit);
+
+        $query->orderBy('l.id', 'DESC');
+
+        return $query->execute()->fetchAllAssociative();
+    }
+
+    private function prepareRelationQuery(
+        DoctrineQueryBuilder $query,
+        int $contentId,
+        ?int $contentVersionNo = null,
+        ?int $relationType = null
+    ): DoctrineQueryBuilder {
         $expr = $query->expr();
         $query
             ->innerJoin(
                 'l',
-                'ezcontentobject',
+                self::CONTENT_ITEM_TABLE,
                 'ezcontentobject_to',
                 $expr->and(
-                    $expr->eq('l.to_contentobject_id', 'ezcontentobject_to.id'),
-                    $expr->eq('ezcontentobject_to.status', ':status'),
+                    'l.to_contentobject_id = ezcontentobject_to.id',
+                    'ezcontentobject_to.status = :status'
                 )
             )
-            ->andWhere(
+            ->where(
                 'l.from_contentobject_id = :content_id'
             )
             ->setParameter(
@@ -1544,7 +1452,7 @@ public function listRelations(
             ->setParameter('content_id', $contentId, ParameterType::INTEGER);
 
         // source version number
-        if (null !== $contentVersionNo) {
+        if ($contentVersionNo !== null) {
             $query
                 ->andWhere('l.from_contentobject_version = :version_no')
                 ->setParameter('version_no', $contentVersionNo, ParameterType::INTEGER);
@@ -1553,11 +1461,11 @@ public function listRelations(
             $query
                 ->innerJoin(
                     'ezcontentobject_to',
-                    'ezcontentobject',
+                    self::CONTENT_ITEM_TABLE,
                     'c',
                     $expr->and(
-                        $expr->eq('c.id', 'l.from_contentobject_id'),
-                        $expr->eq('c.current_version', 'l.from_contentobject_version'),
+                        'c.id = l.from_contentobject_id',
+                        'c.current_version = l.from_contentobject_version'
                     )
                 );
         }
@@ -1577,14 +1485,7 @@ public function listRelations(
                 ->setParameter('relation_type', $relationType, ParameterType::INTEGER);
         }
 
-        $query->setFirstResult($offset);
-        if ($limit !== null) {
-            $query->setMaxResults($limit);
-        }
-
-        $query->orderBy('l.id', 'DESC');
-
-        return $query->execute()->fetchAllAssociative();
+        return $query;
     }
 
     public function countReverseRelations(int $toContentId, ?int $relationType = null): int

diff --git a/src/lib/Persistence/Legacy/Content/Gateway/ExceptionConversion.php b/src/lib/Persistence/Legacy/Content/Gateway/ExceptionConversion.php
@@ -409,16 +409,16 @@ public function countRelations(
      */
     public function listRelations(
         int $contentId,
+        int $limit,
         int $offset = 0,
-        ?int $limit = null,
         ?int $contentVersionNo = null,
         ?int $relationType = null
     ): array {
         try {
             return $this->innerGateway->listRelations(
                 $contentId,
-                $offset,
                 $limit,
+                $offset,
                 $contentVersionNo,
                 $relationType
             );

diff --git a/src/lib/Persistence/Legacy/Content/Handler.php b/src/lib/Persistence/Legacy/Content/Handler.php
@@ -832,13 +832,13 @@ public function countRelations(int $sourceContentId, ?int $sourceContentVersionN
      */
     public function loadRelationList(
         int $sourceContentId,
+        int $limit,
         int $offset = 0,
-        ?int $limit = null,
         ?int $sourceContentVersionNo = null,
         ?int $type = null
     ): array {
         return $this->mapper->extractRelationsFromRows(
-            $this->contentGateway->listRelations($sourceContentId, $offset, $limit, $sourceContentVersionNo, $type)
+            $this->contentGateway->listRelations($sourceContentId, $limit, $offset, $sourceContentVersionNo, $type)
         );
     }