Changed cache key pattern, corrected docblocks

ibexa · Apr 19, 2024 · 935d176 · 935d176
1 parent 9c17daa
commit 935d176
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 83 deletions.
diff --git a/src/contracts/Persistence/Content/Handler.php b/src/contracts/Persistence/Content/Handler.php
@@ -283,7 +283,7 @@ 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.
+     * @deprecated 4.5.7 The "ContentService::loadRelations()" method is deprecated, will be removed in 5.0.
      *
      * @param mixed $sourceContentId Source Content ID
      * @param mixed|null $sourceContentVersionNo Source Content Version, null if not specified
@@ -297,7 +297,7 @@ public function removeRelation($relationId, $type, ?int $destinationContentId =
     public function loadRelations($sourceContentId, $sourceContentVersionNo = null, $type = null);
 
     /**
-     * Counts relations from $sourceContentId. Optionally, counts only those with $type and $sourceContentVersionNo.
+     * Counts all outgoing relations for the given version.
      */
     public function countRelations(
         int $sourceContentId,

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() instead.
+     * @deprecated 4.5.7 The "ContentService::loadRelations()" method is deprecated, will be removed in 5.0.
      *
      * @return \Ibexa\Contracts\Core\Repository\Values\Content\Relation[]
      */
@@ -409,7 +409,7 @@ 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
+     * If the user is not allowed to read specific version then a returned `RelationList` will contain `UnauthorizedRelationListItem`
      *
      * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
      * @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException

diff --git a/src/lib/Persistence/Cache/ContentHandler.php b/src/lib/Persistence/Cache/ContentHandler.php
@@ -31,15 +31,10 @@ class ContentHandler extends AbstractInMemoryPersistenceHandler implements Conte
     private const CONTENT_VERSION_LIST_IDENTIFIER = 'content_version_list';
     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_BY_TYPE_IDENTIFIER = 'content_relations_count_with_by_type_suffix';
-    private const CONTENT_RELATIONS_COUNT_WITH_BY_TYPE_VERSION_IDENTIFIER = 'content_relations_count_with_by_type_version_suffix';
-    private const CONTENT_RELATIONS_COUNT_WITH_BY_VERSION_IDENTIFIER = 'content_relations_count_with_by_version_suffix';
+    private const CONTENT_RELATIONS_COUNT_WITH_VERSION_TYPE_IDENTIFIER = 'content_relations_count_with_by_version_type_suffix';
     private const CONTENT_RELATION_IDENTIFIER = 'content_relation';
     private const CONTENT_RELATIONS_LIST_IDENTIFIER = 'content_relations_list';
-    private const CONTENT_RELATIONS_LIST_WITH_BY_TYPE_IDENTIFIER = 'content_relations_list_with_by_type_suffix';
-    private const CONTENT_RELATIONS_LIST_WITH_BY_TYPE_VERSION_IDENTIFIER = 'content_relations_list_with_by_type_version_suffix';
-    private const CONTENT_RELATIONS_LIST_WITH_BY_VERSION_IDENTIFIER = 'content_relations_list_with_by_version_suffix';
+    private const CONTENT_RELATIONS_LIST_WITH_VERSION_TYPE_IDENTIFIER = 'content_relations_list_with_by_version_type_suffix';
     private const CONTENT_REVERSE_RELATIONS_COUNT_IDENTIFIER = 'content_reverse_relations_count';
     private const RELATION_IDENTIFIER = 'relation';
 
@@ -528,24 +523,10 @@ 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_BY_TYPE_IDENTIFIER;
-            $values[] = $type;
-        } elseif ($sourceContentVersionNo !== null && $type === null) {
-            $patternName = self::CONTENT_RELATIONS_COUNT_WITH_BY_VERSION_IDENTIFIER;
-            $values[] = $sourceContentVersionNo;
-        } elseif ($sourceContentVersionNo !== null && $type !== null) {
-            $patternName = self::CONTENT_RELATIONS_COUNT_WITH_BY_TYPE_VERSION_IDENTIFIER;
-            $values = array_merge($values, [$type, $sourceContentVersionNo]);
-        } else {
-            $patternName = self::CONTENT_RELATIONS_COUNT_IDENTIFIER;
-        }
-
         $cacheItem = $this->cache->getItem(
             $this->cacheIdentifierGenerator->generateKey(
-                $patternName,
-                $values,
+                self::CONTENT_RELATIONS_COUNT_WITH_VERSION_TYPE_IDENTIFIER,
+                [$sourceContentId, $sourceContentVersionNo, $type],
                 true
             )
         );
@@ -583,25 +564,10 @@ public function loadRelationList(
         ?int $sourceContentVersionNo = null,
         ?int $type = null
     ): array {
-        $values = [$sourceContentId, $limit, $offset];
-
-        if ($sourceContentVersionNo === null && $type !== null) {
-            $patternName = self::CONTENT_RELATIONS_LIST_WITH_BY_TYPE_IDENTIFIER;
-            $values[] = $type;
-        } elseif ($sourceContentVersionNo !== null && $type === null) {
-            $patternName = self::CONTENT_RELATIONS_LIST_WITH_BY_VERSION_IDENTIFIER;
-            $values[] = $sourceContentVersionNo;
-        } elseif ($sourceContentVersionNo !== null && $type !== null) {
-            $patternName = self::CONTENT_RELATIONS_LIST_WITH_BY_TYPE_VERSION_IDENTIFIER;
-            $values = array_merge($values, [$type, $sourceContentVersionNo]);
-        } else {
-            $patternName = self::CONTENT_RELATIONS_LIST_IDENTIFIER;
-        }
-
         return $this->getListCacheValue(
             $this->cacheIdentifierGenerator->generateKey(
-                $patternName,
-                $values,
+                self::CONTENT_RELATIONS_LIST_WITH_VERSION_TYPE_IDENTIFIER,
+                [$sourceContentId, $limit, $offset, $sourceContentVersionNo, $type],
                 true
             ),
             function () use ($sourceContentId, $limit, $offset, $sourceContentVersionNo, $type): array {
@@ -642,7 +608,7 @@ function () use ($sourceContentId): array {
                     ),
                 ];
             },
-            $values
+            [$sourceContentId, $limit, $offset, $sourceContentVersionNo, $type]
         );
     }
 

diff --git a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php
@@ -1441,7 +1441,7 @@ private function prepareRelationQuery(
                     'c_to.status = :status'
                 )
             )
-            ->where(
+            ->andWhere(
                 'l.from_contentobject_id = :content_id'
             )
             ->setParameter(

diff --git a/src/lib/Repository/ContentService.php b/src/lib/Repository/ContentService.php
@@ -1991,15 +1991,6 @@ public function copyContent(ContentInfo $contentInfo, LocationCreateStruct $dest
         return $this->internalLoadContentById($content->id);
     }
 
-    /**
-     * Loads all outgoing relations for the given version.
-     *
-     * @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException if the user is not allowed to read this version
-     *
-     * @deprecated since 4.5, use loadRelationList().
-     *
-     * @return \Ibexa\Contracts\Core\Repository\Values\Content\Relation[]
-     */
     public function loadRelations(APIVersionInfo $versionInfo): iterable
     {
         if ($versionInfo->isPublished()) {

diff --git a/src/lib/Resources/settings/storage_engines/cache.yml b/src/lib/Resources/settings/storage_engines/cache.yml
@@ -119,14 +119,8 @@ parameters:
         content_info: 'ci-%s'
         content_info_by_remote_id: 'cibri-%s'
         content_locations: 'cl-%s'
-        content_relations_count: 'crc-%s'
-        content_relations_count_with_by_type_suffix: 'crc-%%s-t-%%s'
-        content_relations_count_with_by_type_version_suffix: 'crc-%%s-t-%%s-v-%%s'
-        content_relations_count_with_by_version_suffix: 'crc-%%s-v-%%s'
-        content_relations_list: 'crl-%%s-l-%%s-o-%%s'
-        content_relations_list_with_by_type_suffix: 'crl-%%s-l-%%s-o-%%s-t-%%s'
-        content_relations_list_with_by_type_version_suffix: 'crl-%%s-l-%%s-o-%%s-t-%%s-v-%%s'
-        content_relations_list_with_by_version_suffix: 'crl-%%s-l-%%s-o-%%s-v-%%s'
+        content_relations_count_with_by_version_type_suffix: 'crc-%%s-v-%%s-t-%%s'
+        content_relations_list_with_by_version_type_suffix: 'crl-%%s-l-%%s-o-%%s-v-%%s-t-%%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
@@ -94,14 +94,14 @@ public function providerForCachedLoadMethodsHit(): array
         // string $method, array $arguments, string $key, array? $tagGeneratingArguments, array? $tagGeneratingResults, array? $keyGeneratingArguments, array? $keyGeneratingResults, mixed? $data, bool $multi = false, array $additionalCalls
         return [
             ['countReverseRelations', [2], 'ibx-crrc-2', null, null, [['content_reverse_relations_count', [2], true]], ['ibx-crrc-2'], 10],
-            ['countRelations', [2], 'ibx-crc-2', null, null, [['content_relations_count', [2], true]], ['ibx-crc-2'], 10],
-            ['countRelations', [2, 2], 'ibx-crc-2-v-2', null, null, [['content_relations_count_with_by_version_suffix', [2, 2], true]], ['ibx-crc-2-v-2'], 10],
-            ['countRelations', [2, null, 1], 'ibx-crc-2-t-1', null, null, [['content_relations_count_with_by_type_suffix', [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_by_type_version_suffix', [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_by_version_suffix', [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_by_type_suffix', [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_by_type_version_suffix', [2, 1, 0, 1, 2], true]], ['ibx-crl-2-l-1-o-0-t-1-v-2'], $relationList],
+            ['countRelations', [2], 'ibx-crc-2-v--t-', null, null, [['content_relations_count_with_by_version_type_suffix', [2, null, null], true]], ['ibx-crc-2-v--t-'], 10],
+            ['countRelations', [2, 2], 'ibx-crc-2-v-2-t-', null, null, [['content_relations_count_with_by_version_type_suffix', [2, 2, null], true]], ['ibx-crc-2-v-2-t-'], 10],
+            ['countRelations', [2, null, 1], 'ibx-crc-2-v--t-1', null, null, [['content_relations_count_with_by_version_type_suffix', [2, null, 1], true]], ['ibx-crc-2-v--t-1'], 10],
+            ['countRelations', [2, 2, 1], 'ibx-crc-2-v-2-t-1', null, null, [['content_relations_count_with_by_version_type_suffix', [2, 2, 1], true]], ['ibx-crc-2-v-2-t-1'], 10],
+            ['loadRelationList', [2, 1, 0], 'ibx-crl-2-l-1-o-0-v--t-', null, null, [['content_relations_list_with_by_version_type_suffix', [2, 1, 0, null, null], true]], ['ibx-crl-2-l-1-o-0-v--t-'], $relationList],
+            ['loadRelationList', [2, 1, 0, 2], 'ibx-crl-2-l-1-o-0-v-2-t-', null, null, [['content_relations_list_with_by_version_type_suffix', [2, 1, 0, 2, null], true]], ['ibx-crl-2-l-1-o-0-v-2-t-'], $relationList],
+            ['loadRelationList', [2, 1, 0, null, 1], 'ibx-crl-2-l-1-o-0-v--t-1', null, null, [['content_relations_list_with_by_version_type_suffix', [2, 1, 0, null, 1], true]], ['ibx-crl-2-l-1-o-0-v--t-1'], $relationList],
+            ['loadRelationList', [2, 1, 0, 2, 1], 'ibx-crl-2-l-1-o-0-v-2-t-1', null, null, [['content_relations_list_with_by_version_type_suffix', [2, 1, 0, 2, 1], true]], ['ibx-crl-2-l-1-o-0-v-2-t-1'], $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],
@@ -150,57 +150,57 @@ public function providerForCachedLoadMethodsMiss(): array
             [
                 'countRelations',
                 [2],
-                'ibx-crc-2',
+                'ibx-crc-2-v--t-',
                 [
                     ['content', [2], false],
                 ],
                 ['c-2'],
                 [
-                    ['content_relations_count', [2], true],
+                    ['content_relations_count_with_by_version_type_suffix', [2, null, null], true],
                 ],
-                ['ibx-crc-2'],
+                ['ibx-crc-2-v--t-'],
                 10,
             ],
             [
                 'countRelations',
                 [2, 3],
-                'ibx-crc-2-v-3',
+                'ibx-crc-2-v-3-t-',
                 [
                     ['content', [2], false],
                 ],
                 ['c-2'],
                 [
-                    ['content_relations_count_with_by_version_suffix', [2, 3], true],
+                    ['content_relations_count_with_by_version_type_suffix', [2, 3, null], true],
                 ],
-                ['ibx-crc-2-v-3'],
+                ['ibx-crc-2-v-3-t-'],
                 10,
             ],
             [
                 'countRelations',
                 [2, null, 1],
-                'ibx-crc-2-t-1',
+                'ibx-crc-2-v--t-1',
                 [
                     ['content', [2], false],
                 ],
                 ['c-2'],
                 [
-                    ['content_relations_count_with_by_type_suffix', [2, 1], true],
+                    ['content_relations_count_with_by_version_type_suffix', [2, null, 1], true],
                 ],
-                ['ibx-crc-2-t-1'],
+                ['ibx-crc-2-v--t-1'],
                 10,
             ],
             [
                 'countRelations',
                 [2, 3, 1],
-                'ibx-crc-2-t-1-v-3',
+                'ibx-crc-2-v-3-t-',
                 [
                     ['content', [2], false],
                 ],
                 ['c-2'],
                 [
-                    ['content_relations_count_with_by_type_version_suffix', [2, 1, 3], true],
+                    ['content_relations_count_with_by_version_type_suffix', [2, 3, 1], true],
                 ],
-                ['ibx-crc-2-t-1-v-3'],
+                ['ibx-crc-2-v-3-t-'],
                 10,
             ],
             [