Added cache for RelationList, Added deprecated and @throws for loadRelations in contracts

mateuszdebinski · mateuszdebinski · commit 309b952e40a7 · 2024-03-29T15:18:36.000+01:00
diff --git a/src/contracts/Persistence/Content/Handler.php b/src/contracts/Persistence/Content/Handler.php
@@ -283,6 +283,8 @@ public function removeRelation($relationId, $type, ?int $destinationContentId =
     /**
      * Loads relations from $sourceContentId. Optionally, loads only those with $type and $sourceContentVersionNo.
      *
+     * @deprecated since 4.5, use loadRelationList() instead.
+     *
      * @param mixed $sourceContentId Source Content ID
      * @param mixed|null $sourceContentVersionNo Source Content Version, null if not specified
      * @param int|null $type {@see \Ibexa\Contracts\Core\Repository\Values\Content\Relation::COMMON,
diff --git a/src/contracts/Repository/ContentService.php b/src/contracts/Repository/ContentService.php
@@ -400,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
      *
-     * @deprecated since 4.5, use loadRelationList().
+     * @deprecated since 4.5, use loadRelationList() instead.
      *
      * @return \Ibexa\Contracts\Core\Repository\Values\Content\Relation[]
      */
@@ -411,6 +411,9 @@ public function loadRelations(VersionInfo $versionInfo): iterable;
      *
      * If the user is not allowed to read specific version then UnauthorizedRelationListItem is returned
      *
+     * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
+     * @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException
+     *
      * @see \Ibexa\Contracts\Core\Repository\Values\Content\RelationList\Item\UnauthorizedRelationListItem
      */
     public function loadRelationList(
@@ -421,6 +424,9 @@ public function loadRelationList(
 
     /**
      * Counts all outgoing relations for the given version.
+     *
+     * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
+     * @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException
      */
     public function countRelations(VersionInfo $versionInfo): int;
 
diff --git a/src/lib/Persistence/Cache/ContentHandler.php b/src/lib/Persistence/Cache/ContentHandler.php
@@ -35,6 +35,10 @@ class ContentHandler extends AbstractInMemoryPersistenceHandler implements Conte
     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_RELATIONS_LIST_IDENTIFIER = 'content_relations_list';
+    private const CONTENT_RELATIONS_LIST_WITH_TYPE_IDENTIFIER = 'content_relations_list_with_type';
+    private const CONTENT_RELATIONS_LIST_WITH_TYPE_AND_VERSION_IDENTIFIER = 'content_relations_list_with_type_and_version';
+    private const CONTENT_RELATIONS_LIST_WITH_VERSION_IDENTIFIER = 'content_relations_list_with_version';
     private const CONTENT_REVERSE_RELATIONS_COUNT_IDENTIFIER = 'content_reverse_relations_count';
     private const RELATION_IDENTIFIER = 'relation';
 
@@ -546,12 +550,12 @@ public function countRelations(int $sourceContentId, ?int $sourceContentVersionN
         );
 
         if ($cacheItem->isHit()) {
-            $this->logger->logCacheHit(['content' => $sourceContentId, 'version' => $sourceContentVersionNo]);
+            $this->logger->logCacheHit(['content' => $sourceContentId, 'version' => $sourceContentVersionNo, 'type' => $type]);
 
             return $cacheItem->get();
         }
 
-        $this->logger->logCacheMiss(['content' => $sourceContentId, 'version' => $sourceContentVersionNo]);
+        $this->logger->logCacheMiss(['content' => $sourceContentId, 'version' => $sourceContentVersionNo, 'type' => $type]);
         $relationsCount = $this->persistenceHandler->contentHandler()->countRelations(
             $sourceContentId,
             $sourceContentVersionNo,
@@ -571,31 +575,72 @@ public function countRelations(int $sourceContentId, ?int $sourceContentVersionN
         return $relationsCount;
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public function loadRelationList(
         int $sourceContentId,
         int $limit,
         int $offset = 0,
         ?int $sourceContentVersionNo = null,
         ?int $type = null
     ): array {
-        $this->logger->logCall(__METHOD__, [
+        $values = [$sourceContentId, $limit, $offset];
+
+        if ($sourceContentVersionNo === null && $type !== null) {
+            $patternName = self::CONTENT_RELATIONS_LIST_WITH_TYPE_IDENTIFIER;
+            $values[] = $type;
+        } elseif ($sourceContentVersionNo !== null && $type === null) {
+            $patternName = self::CONTENT_RELATIONS_LIST_WITH_VERSION_IDENTIFIER;
+            $values[] = $sourceContentVersionNo;
+        } elseif ($sourceContentVersionNo !== null && $type !== null) {
+            $patternName = self::CONTENT_RELATIONS_LIST_WITH_TYPE_AND_VERSION_IDENTIFIER;
+            $values = array_merge($values, [$type, $sourceContentVersionNo]);
+        } else {
+            $patternName = self::CONTENT_RELATIONS_LIST_IDENTIFIER;
+        }
+
+        $cacheItem = $this->cache->getItem(
+            $this->cacheIdentifierGenerator->generateKey(
+                $patternName,
+                $values,
+                true
+            )
+        );
+
+        $logCacheArguments = [
             'content' => $sourceContentId,
             'version' => $sourceContentVersionNo,
-            'offset' => $offset,
-            'limit' => $limit,
             'type' => $type,
-        ]);
+            'limit' => $limit,
+            'offset' => $offset,
+        ];
+
+        if ($cacheItem->isHit()) {
+            $this->logger->logCacheHit($logCacheArguments);
+
+            return $cacheItem->get();
+        }
+
+        $this->logger->logCacheMiss($logCacheArguments);
 
-        return $this->persistenceHandler->contentHandler()->loadRelationList(
+        $relationList = $this->persistenceHandler->contentHandler()->loadRelationList(
             $sourceContentId,
             $limit,
             $offset,
             $sourceContentVersionNo,
             $type
         );
+
+        $cacheItem->set($relationList);
+        $tags = [
+            $this->cacheIdentifierGenerator->generateTag(
+                self::CONTENT_IDENTIFIER,
+                [$sourceContentId]
+            ),
+        ];
+
+        $cacheItem->tag($tags);
+        $this->cache->save($cacheItem);
+
+        return $relationList;
     }
 
     /**
diff --git a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php
@@ -1435,10 +1435,10 @@ private function prepareRelationQuery(
             ->innerJoin(
                 'l',
                 self::CONTENT_ITEM_TABLE,
-                'ezcontentobject_to',
+                'c_to',
                 $expr->and(
-                    'l.to_contentobject_id = ezcontentobject_to.id',
-                    'ezcontentobject_to.status = :status'
+                    'l.to_contentobject_id = c_to.id',
+                    'c_to.status = :status'
                 )
             )
             ->where(
@@ -1460,7 +1460,7 @@ private function prepareRelationQuery(
             // from published version only
             $query
                 ->innerJoin(
-                    'ezcontentobject_to',
+                    'c_to',
                     self::CONTENT_ITEM_TABLE,
                     'c',
                     $expr->and(
diff --git a/src/lib/Persistence/Legacy/Content/Gateway/ExceptionConversion.php b/src/lib/Persistence/Legacy/Content/Gateway/ExceptionConversion.php
@@ -404,9 +404,6 @@ public function countRelations(
         }
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public function listRelations(
         int $contentId,
         int $limit,
diff --git a/src/lib/Persistence/Legacy/Content/Handler.php b/src/lib/Persistence/Legacy/Content/Handler.php
@@ -827,9 +827,6 @@ public function countRelations(int $sourceContentId, ?int $sourceContentVersionN
         return $this->contentGateway->countRelations($sourceContentId, $sourceContentVersionNo, $type);
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public function loadRelationList(
         int $sourceContentId,
         int $limit,
diff --git a/src/lib/Resources/settings/storage_engines/cache.yml b/src/lib/Resources/settings/storage_engines/cache.yml
@@ -121,6 +121,10 @@ parameters:
         content_relations_count_with_type: 'crc-%%s-t-%%s'
         content_relations_count_with_type_and_version: 'crc-%%s-t-%%s-v-%%s'
         content_relations_count_with_version: 'crc-%%s-v-%%s'
+        content_relations_list: 'crl-%%s-l-%%s-o-%%s'
+        content_relations_list_with_type: 'crl-%%s-l-%%s-o-%%s-t-%%s'
+        content_relations_list_with_type_and_version: 'crl-%%s-l-%%s-o-%%s-t-%%s-v-%%s'
+        content_relations_list_with_version: 'crl-%%s-l-%%s-o-%%s-v-%%s'
         content_reverse_relations_count: 'crrc-%s'
         content_version_info: 'cvi-%s'
         content_version_list: 'c-%s-vl'
diff --git a/tests/lib/Persistence/Cache/ContentHandlerTest.php b/tests/lib/Persistence/Cache/ContentHandlerTest.php
@@ -11,6 +11,7 @@
 use Ibexa\Contracts\Core\Persistence\Content\CreateStruct;
 use Ibexa\Contracts\Core\Persistence\Content\Handler as SPIContentHandler;
 use Ibexa\Contracts\Core\Persistence\Content\MetadataUpdateStruct;
+use Ibexa\Contracts\Core\Persistence\Content\Relation;
 use Ibexa\Contracts\Core\Persistence\Content\Relation as SPIRelation;
 use Ibexa\Contracts\Core\Persistence\Content\Relation\CreateStruct as RelationCreateStruct;
 use Ibexa\Contracts\Core\Persistence\Content\UpdateStruct;
@@ -82,6 +83,13 @@ public function providerForCachedLoadMethodsHit(): array
         $info = new ContentInfo(['id' => 2]);
         $version = new VersionInfo(['versionNo' => 1, 'contentInfo' => $info]);
         $content = new Content(['fields' => [], 'versionInfo' => $version]);
+        $relation = new Relation();
+        $relation->id = 1;
+        $relation->sourceContentId = 2;
+        $relation->sourceContentVersionNo = 2;
+        $relation->destinationContentId = 1;
+        $relation->type = 1;
+        $relationList[1] = $relation;
 
         // string $method, array $arguments, string $key, array? $tagGeneratingArguments, array? $tagGeneratingResults, array? $keyGeneratingArguments, array? $keyGeneratingResults, mixed? $data, bool $multi = false, array $additionalCalls
         return [
@@ -90,6 +98,10 @@ public function providerForCachedLoadMethodsHit(): array
             ['countRelations', [2, 2], 'ibx-crc-2-v-2', null, null, [['content_relations_count_with_version', [2, 2], true]], ['ibx-crc-2-v-2'], 10],
             ['countRelations', [2, null, 1], 'ibx-crc-2-t-1', null, null, [['content_relations_count_with_type', [2, 1], true]], ['ibx-crc-2-t-1'], 10],
             ['countRelations', [2, 2, 1], 'ibx-crc-2-t-1-v-2', null, null, [['content_relations_count_with_type_and_version', [2, 1, 2], true]], ['ibx-crc-2-t-1-v-2'], 10],
+            ['loadRelationList', [2, 1, 0], 'ibx-crl-2-l-1-o-0', null, null, [['content_relations_list', [2, 1, 0], true]], ['ibx-crl-2-l-1-o-0'], $relationList],
+            ['loadRelationList', [2, 1, 0, 2], 'ibx-crl-2-l-1-o-0-v-2', null, null, [['content_relations_list_with_version', [2, 1, 0, 2], true]], ['ibx-crl-2-l-1-o-0-v-2'], $relationList],
+            ['loadRelationList', [2, 1, 0, null, 1], 'ibx-crl-2-l-1-o-0-t-1', null, null, [['content_relations_list_with_type', [2, 1, 0, 1], true]], ['ibx-crl-2-l-1-o-0-t-1'], $relationList],
+            ['loadRelationList', [2, 1, 0, 2, 1], 'ibx-crl-2-l-1-o-0-t-1-v-2', null, null, [['content_relations_list_with_type_and_version', [2, 1, 0, 1, 2], true]], ['ibx-crl-2-l-1-o-0-t-1-v-2'], $relationList],
             ['load', [2, 1], 'ibx-c-2-1-' . ContentHandler::ALL_TRANSLATIONS_KEY, null, null, [['content', [], true]], ['ibx-c'], $content],
             ['load', [2, 1, ['eng-GB', 'eng-US']], 'ibx-c-2-1-eng-GB|eng-US', null, null, [['content', [], true]], ['ibx-c'], $content],
             ['load', [2], 'ibx-c-2-' . ContentHandler::ALL_TRANSLATIONS_KEY, null, null, [['content', [], true]], ['ibx-c'], $content],

Original file line number	Diff line number	Diff line change
`@@ -283,6 +283,8 @@ public function removeRelation($relationId, $type, ?int $destinationContentId =`
`283`	`283`	`/**`
`284`	`284`	`* Loads relations from $sourceContentId. Optionally, loads only those with $type and $sourceContentVersionNo.`
`285`	`285`	`*`
	`286`	`+ * @deprecated since 4.5, use loadRelationList() instead.`
	`287`	`+ *`
`286`	`288`	`* @param mixed $sourceContentId Source Content ID`
`287`	`289`	`* @param mixed\|null $sourceContentVersionNo Source Content Version, null if not specified`
`288`	`290`	`* @param int\|null $type {@see \Ibexa\Contracts\Core\Repository\Values\Content\Relation::COMMON,`
Original file line number	Diff line number	Diff line change
`@@ -400,7 +400,7 @@ public function copyContent(ContentInfo $contentInfo, LocationCreateStruct $dest`
`400`	`400`	`*`
`401`	`401`	`* @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException if the user is not allowed to read this version`
`402`	`402`	`*`
`403`		`- * @deprecated since 4.5, use loadRelationList().`
	`403`	`+ * @deprecated since 4.5, use loadRelationList() instead.`
`404`	`404`	`*`
`405`	`405`	`* @return \Ibexa\Contracts\Core\Repository\Values\Content\Relation[]`
`406`	`406`	`*/`
`@@ -411,6 +411,9 @@ public function loadRelations(VersionInfo $versionInfo): iterable;`
`411`	`411`	`*`
`412`	`412`	`* If the user is not allowed to read specific version then UnauthorizedRelationListItem is returned`
`413`	`413`	`*`
	`414`	`+ * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException`
	`415`	`+ * @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException`
	`416`	`+ *`
`414`	`417`	`* @see \Ibexa\Contracts\Core\Repository\Values\Content\RelationList\Item\UnauthorizedRelationListItem`
`415`	`418`	`*/`
`416`	`419`	`public function loadRelationList(`
`@@ -421,6 +424,9 @@ public function loadRelationList(`
`421`	`424`
`422`	`425`	`/**`
`423`	`426`	`* Counts all outgoing relations for the given version.`
	`427`	`+ *`
	`428`	`+ * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException`
	`429`	`+ * @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException`
`424`	`430`	`*/`
`425`	`431`	`public function countRelations(VersionInfo $versionInfo): int;`
`426`	`432`
Original file line number	Diff line number	Diff line change
`@@ -404,9 +404,6 @@ public function countRelations(`
`404`	`404`	`}`
`405`	`405`	`}`
`406`	`406`
`407`		`- /**`
`408`		`- * {@inheritdoc}`
`409`		`- */`
`410`	`407`	`public function listRelations(`
`411`	`408`	`int $contentId,`
`412`	`409`	`int $limit,`
Original file line number	Diff line number	Diff line change
`@@ -827,9 +827,6 @@ public function countRelations(int $sourceContentId, ?int $sourceContentVersionN`
`827`	`827`	`return $this->contentGateway->countRelations($sourceContentId, $sourceContentVersionNo, $type);`
`828`	`828`	`}`
`829`	`829`
`830`		`- /**`
`831`		`- * {@inheritdoc}`
`832`		`- */`
`833`	`830`	`public function loadRelationList(`
`834`	`831`	`int $sourceContentId,`
`835`	`832`	`int $limit,`