From 0fb03920f6a4d0251a45681817466d0b8af8fec7 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Fri, 27 May 2022 10:33:31 +0200 Subject: [PATCH 01/21] IBX-2823: Added loadRelation method https://issues.ibexa.co/browse/IBX-2823 --- src/contracts/Persistence/Content/Handler.php | 11 ++- src/lib/Persistence/Cache/ContentHandler.php | 79 ++++++++++++++++++- .../Persistence/Legacy/Content/Gateway.php | 8 ++ .../Content/Gateway/DoctrineDatabase.php | 14 ++++ .../Content/Gateway/ExceptionConversion.php | 9 +++ .../Persistence/Legacy/Content/Handler.php | 14 +++- src/lib/Repository/ContentService.php | 3 +- .../Repository/Helper/RelationProcessor.php | 6 +- .../settings/storage_engines/cache.yml | 3 + 9 files changed, 139 insertions(+), 8 deletions(-) diff --git a/src/contracts/Persistence/Content/Handler.php b/src/contracts/Persistence/Content/Handler.php index d32fd619aa..2e718d8dff 100644 --- a/src/contracts/Persistence/Content/Handler.php +++ b/src/contracts/Persistence/Content/Handler.php @@ -261,6 +261,15 @@ public function copy($contentId, $versionNo = null, $newOwnerId = null); */ public function addRelation(RelationCreateStruct $createStruct); + /** + * Load a relation by $relationId. + * + * @param int $relationId Relation ID + * + * @return \Ibexa\Contracts\Core\Persistence\Content\Relation + */ + public function loadRelation(int $relationId): Relation; + /** * Removes a relation by relation Id. * @@ -272,7 +281,7 @@ public function addRelation(RelationCreateStruct $createStruct); * \Ibexa\Contracts\Core\Repository\Values\Content\Relation::LINK, * \Ibexa\Contracts\Core\Repository\Values\Content\Relation::FIELD} */ - public function removeRelation($relationId, $type); + public function removeRelation($relationId, $type, ?int $destinationContentId = null); /** * Loads relations from $sourceContentId. Optionally, loads only those with $type and $sourceContentVersionNo. diff --git a/src/lib/Persistence/Cache/ContentHandler.php b/src/lib/Persistence/Cache/ContentHandler.php index dd18dd6f56..8658780398 100644 --- a/src/lib/Persistence/Cache/ContentHandler.php +++ b/src/lib/Persistence/Cache/ContentHandler.php @@ -11,6 +11,7 @@ use Ibexa\Contracts\Core\Persistence\Content\CreateStruct; use Ibexa\Contracts\Core\Persistence\Content\Handler as ContentHandlerInterface; use Ibexa\Contracts\Core\Persistence\Content\MetadataUpdateStruct; +use Ibexa\Contracts\Core\Persistence\Content\Relation; use Ibexa\Contracts\Core\Persistence\Content\Relation\CreateStruct as RelationCreateStruct; use Ibexa\Contracts\Core\Persistence\Content\UpdateStruct; use Ibexa\Contracts\Core\Persistence\Content\VersionInfo; @@ -30,6 +31,8 @@ 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_REVERSE_RELATIONS_COUNT_IDENTIFIER = 'content_reverse_relations_count'; + private const RELATION_IDENTIFIER = 'relation'; public const ALL_TRANSLATIONS_KEY = '0'; @@ -433,18 +436,67 @@ public function addRelation(RelationCreateStruct $relation) { $this->logger->logCall(__METHOD__, ['struct' => $relation]); + $this->cache->invalidateTags([ + $this->cacheIdentifierGenerator->generateTag( + self::CONTENT_IDENTIFIER, + [$relation->destinationContentId] + ), + ]); + return $this->persistenceHandler->contentHandler()->addRelation($relation); } /** * {@inheritdoc} */ - public function removeRelation($relationId, $type) + public function removeRelation($relationId, $type, ?int $destinationContentId = null) { $this->logger->logCall(__METHOD__, ['relation' => $relationId, 'type' => $type]); + + $contentId = $destinationContentId ?? $this->loadRelation($relationId)->destinationContentId; + + $this->cache->invalidateTags([ + $this->cacheIdentifierGenerator->generateTag( + self::CONTENT_IDENTIFIER, + [$contentId] + ), + $this->cacheIdentifierGenerator->generateTag( + self::RELATION_IDENTIFIER, + [$contentId] + ), + ]); + $this->persistenceHandler->contentHandler()->removeRelation($relationId, $type); } + public function loadRelation(int $relationId): Relation + { + $cacheItem = $this->cache->getItem( + $this->cacheIdentifierGenerator->generateKey( + self::RELATION_IDENTIFIER, + [$relationId] + ) + ); + + if ($cacheItem->isHit()) { + $this->logger->logCacheHit(['relationId' => $relationId]); + + return $cacheItem->get(); + } + + $this->logger->logCacheMiss(['relationId' => $relationId]); + $relation = $this->persistenceHandler->contentHandler()->loadRelation($relationId); + $cacheItem->set($relation); + $tags = [ + $this->cacheIdentifierGenerator->generateTag(self::RELATION_IDENTIFIER, [$relationId]), + ]; + + $cacheItem->tag($tags); + $this->cache->save($cacheItem); + + return $relation; + } + /** * {@inheritdoc} */ @@ -467,9 +519,30 @@ public function loadRelations($sourceContentId, $sourceContentVersionNo = null, */ public function countReverseRelations(int $destinationContentId, ?int $type = null): int { - $this->logger->logCall(__METHOD__, ['content' => $destinationContentId, 'type' => $type]); + $cacheItem = $this->cache->getItem( + $this->cacheIdentifierGenerator->generateKey( + self::CONTENT_REVERSE_RELATIONS_COUNT_IDENTIFIER, + [$destinationContentId] + ) + ); + + if ($cacheItem->isHit()) { + $this->logger->logCacheHit(['content' => $destinationContentId]); + + return $cacheItem->get(); + } + + $this->logger->logCacheMiss(['content' => $destinationContentId]); + $reverseRelationsCount = $this->persistenceHandler->contentHandler()->countReverseRelations($destinationContentId, $type); + $cacheItem->set($reverseRelationsCount); + $tags = [ + $this->cacheIdentifierGenerator->generateTag(self::CONTENT_IDENTIFIER, [$destinationContentId]), + ]; + + $cacheItem->tag($tags); + $this->cache->save($cacheItem); - return $this->persistenceHandler->contentHandler()->countReverseRelations($destinationContentId, $type); + return $reverseRelationsCount; } /** diff --git a/src/lib/Persistence/Legacy/Content/Gateway.php b/src/lib/Persistence/Legacy/Content/Gateway.php index 12c3b727af..c850219861 100644 --- a/src/lib/Persistence/Legacy/Content/Gateway.php +++ b/src/lib/Persistence/Legacy/Content/Gateway.php @@ -13,6 +13,7 @@ use Ibexa\Contracts\Core\Persistence\Content\Relation\CreateStruct as RelationCreateStruct; use Ibexa\Contracts\Core\Persistence\Content\UpdateStruct; use Ibexa\Contracts\Core\Persistence\Content\VersionInfo; +use Ibexa\Contracts\Core\Repository\Values\Content\Relation; /** * Base class for content gateways. @@ -395,6 +396,13 @@ abstract public function deleteRelation(int $relationId, int $type): void; */ abstract public function insertRelation(RelationCreateStruct $createStruct): int; + /** + * Load Relation object. + * + * @see \Ibexa\Contracts\Core\Persistence\Content\Relation + */ + abstract public function loadRelation(int $relationId): array; + /** * Return all Content IDs for the given $contentTypeId. * diff --git a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php index 4eb4f72a73..4e5bc4edd5 100644 --- a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php +++ b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php @@ -1609,6 +1609,20 @@ public function insertRelation(RelationCreateStruct $createStruct): int return (int)$this->connection->lastInsertId(self::CONTENT_RELATION_SEQ); } + public function loadRelation(int $relationId): array + { + $query = $this->queryBuilder->createRelationFindQueryBuilder(); + $expr = $query->expr(); + + $query + ->where( + $expr->eq('id', ':relationId') + ) + ->setParameter('relationId', $relationId, ParameterType::INTEGER); + + return $query->execute()->fetchAllAssociative(); + } + public function deleteRelation(int $relationId, int $type): void { // Legacy Storage stores COMMON, LINK and EMBED types using bitmask, therefore first load diff --git a/src/lib/Persistence/Legacy/Content/Gateway/ExceptionConversion.php b/src/lib/Persistence/Legacy/Content/Gateway/ExceptionConversion.php index 0fafc8df0f..afe797c317 100644 --- a/src/lib/Persistence/Legacy/Content/Gateway/ExceptionConversion.php +++ b/src/lib/Persistence/Legacy/Content/Gateway/ExceptionConversion.php @@ -446,6 +446,15 @@ public function insertRelation(RelationCreateStruct $struct): int } } + public function loadRelation(int $relationId): array + { + try { + return $this->innerGateway->loadRelation($relationId); + } catch (DBALException | PDOException $e) { + throw DatabaseException::wrap($e); + } + } + public function getContentIdsByContentTypeId($contentTypeId): array { try { diff --git a/src/lib/Persistence/Legacy/Content/Handler.php b/src/lib/Persistence/Legacy/Content/Handler.php index 700420ec62..c5c3881a94 100644 --- a/src/lib/Persistence/Legacy/Content/Handler.php +++ b/src/lib/Persistence/Legacy/Content/Handler.php @@ -12,6 +12,7 @@ use Ibexa\Contracts\Core\Persistence\Content\Field; use Ibexa\Contracts\Core\Persistence\Content\Handler as BaseContentHandler; use Ibexa\Contracts\Core\Persistence\Content\MetadataUpdateStruct; +use Ibexa\Contracts\Core\Persistence\Content\Relation; use Ibexa\Contracts\Core\Persistence\Content\Relation\CreateStruct as RelationCreateStruct; use Ibexa\Contracts\Core\Persistence\Content\Type\Handler as ContentTypeHandler; use Ibexa\Contracts\Core\Persistence\Content\UpdateStruct; @@ -776,6 +777,17 @@ public function addRelation(RelationCreateStruct $createStruct) return $relation; } + public function loadRelation(int $relationId): Relation + { + $rows = $this->contentGateway->loadRelation($relationId); + + if (empty($rows)) { + throw new NotFound('relation', "relationId: $relationId"); + } + + return current($this->mapper->extractRelationsFromRows($rows)); + } + /** * Removes a relation by relation Id. * @@ -787,7 +799,7 @@ public function addRelation(RelationCreateStruct $createStruct) * \Ibexa\Contracts\Core\Repository\Values\Content\Relation::LINK, * \Ibexa\Contracts\Core\Repository\Values\Content\Relation::FIELD} */ - public function removeRelation($relationId, $type) + public function removeRelation($relationId, $type, ?int $destinationContentId = null) { $this->contentGateway->deleteRelation($relationId, $type); } diff --git a/src/lib/Repository/ContentService.php b/src/lib/Repository/ContentService.php index 5c98b413a7..f9508c7800 100644 --- a/src/lib/Repository/ContentService.php +++ b/src/lib/Repository/ContentService.php @@ -2128,7 +2128,8 @@ public function deleteRelation(APIVersionInfo $sourceVersion, ContentInfo $desti if ($spiRelation->destinationContentId == $destinationContent->id) { $this->persistenceHandler->contentHandler()->removeRelation( $spiRelation->id, - APIRelation::COMMON + APIRelation::COMMON, + $spiRelation->destinationContentId ); } } diff --git a/src/lib/Repository/Helper/RelationProcessor.php b/src/lib/Repository/Helper/RelationProcessor.php index 647ed6904e..bcb4914ec7 100644 --- a/src/lib/Repository/Helper/RelationProcessor.php +++ b/src/lib/Repository/Helper/RelationProcessor.php @@ -192,7 +192,8 @@ public function processFieldRelations( foreach ($relationEntry as $relation) { $this->persistenceHandler->contentHandler()->removeRelation( $relation->id, - $relationType + $relationType, + $relation->destinationContentInfo->id ); } break; @@ -200,7 +201,8 @@ public function processFieldRelations( case Relation::EMBED: $this->persistenceHandler->contentHandler()->removeRelation( $relationEntry->id, - $relationType + $relationType, + $relationEntry->destinationContentInfo->id ); } } diff --git a/src/lib/Resources/settings/storage_engines/cache.yml b/src/lib/Resources/settings/storage_engines/cache.yml index 78a521dc46..ce15d6f28f 100644 --- a/src/lib/Resources/settings/storage_engines/cache.yml +++ b/src/lib/Resources/settings/storage_engines/cache.yml @@ -47,6 +47,7 @@ parameters: notification_count: 'nc-%s' notification_pending_count: 'npc-%s' policy: 'p-%s' + relation: 'rel-%s' role: 'r-%s' role_assignment: 'ra-%s' role_assignment_group_list: 'ragl-%s' @@ -114,6 +115,7 @@ parameters: content_info: 'ci-%s' content_info_by_remote_id: 'cibri-%s' content_locations: 'cl-%s' + content_reverse_relations_count: 'crrc-%s' content_version_info: 'cvi-%s' content_version_list: 'c-%s-vl' content_version: 'c-%%s-v-%%s' @@ -141,6 +143,7 @@ parameters: notification_count: 'nc-%s' notification_pending_count: 'npc-%s' policy: 'p-%s' + relation: 'rel-%s' role: 'r-%s' role_assignment: 'ra-%s' role_assignment_group_list: 'ragl-%s' From 92c53c4834b9a17da1cb3dc4a033c103407d79da Mon Sep 17 00:00:00 2001 From: ciastektk Date: Mon, 30 May 2022 13:15:22 +0200 Subject: [PATCH 02/21] Update src/lib/Resources/settings/storage_engines/cache.yml Co-authored-by: Konrad Oboza --- src/lib/Resources/settings/storage_engines/cache.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Resources/settings/storage_engines/cache.yml b/src/lib/Resources/settings/storage_engines/cache.yml index ce15d6f28f..3bab85a951 100644 --- a/src/lib/Resources/settings/storage_engines/cache.yml +++ b/src/lib/Resources/settings/storage_engines/cache.yml @@ -47,7 +47,7 @@ parameters: notification_count: 'nc-%s' notification_pending_count: 'npc-%s' policy: 'p-%s' - relation: 'rel-%s' + relation: 're-%s' role: 'r-%s' role_assignment: 'ra-%s' role_assignment_group_list: 'ragl-%s' From 80eee5a0527a54ede711725027ae0855553f59f1 Mon Sep 17 00:00:00 2001 From: ciastektk Date: Mon, 30 May 2022 13:15:27 +0200 Subject: [PATCH 03/21] Update src/lib/Resources/settings/storage_engines/cache.yml Co-authored-by: Konrad Oboza --- src/lib/Resources/settings/storage_engines/cache.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Resources/settings/storage_engines/cache.yml b/src/lib/Resources/settings/storage_engines/cache.yml index 3bab85a951..f8d285b1b5 100644 --- a/src/lib/Resources/settings/storage_engines/cache.yml +++ b/src/lib/Resources/settings/storage_engines/cache.yml @@ -143,7 +143,7 @@ parameters: notification_count: 'nc-%s' notification_pending_count: 'npc-%s' policy: 'p-%s' - relation: 'rel-%s' + relation: 're-%s' role: 'r-%s' role_assignment: 'ra-%s' role_assignment_group_list: 'ragl-%s' From 67cb972e459fa5db22e113f0cab44a4d4a86ee23 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Mon, 30 May 2022 13:21:35 +0200 Subject: [PATCH 04/21] Fixed invalidate tags in removeRelation --- src/lib/Persistence/Cache/ContentHandler.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib/Persistence/Cache/ContentHandler.php b/src/lib/Persistence/Cache/ContentHandler.php index 8658780398..ad56e681ed 100644 --- a/src/lib/Persistence/Cache/ContentHandler.php +++ b/src/lib/Persistence/Cache/ContentHandler.php @@ -453,20 +453,18 @@ public function removeRelation($relationId, $type, ?int $destinationContentId = { $this->logger->logCall(__METHOD__, ['relation' => $relationId, 'type' => $type]); - $contentId = $destinationContentId ?? $this->loadRelation($relationId)->destinationContentId; - $this->cache->invalidateTags([ $this->cacheIdentifierGenerator->generateTag( self::CONTENT_IDENTIFIER, - [$contentId] + [$destinationContentId ?? $this->loadRelation($relationId)->destinationContentId] ), $this->cacheIdentifierGenerator->generateTag( self::RELATION_IDENTIFIER, - [$contentId] + [$relationId] ), ]); - $this->persistenceHandler->contentHandler()->removeRelation($relationId, $type); + $this->persistenceHandler->contentHandler()->removeRelation($relationId, $type, $destinationContentId); } public function loadRelation(int $relationId): Relation @@ -474,7 +472,8 @@ public function loadRelation(int $relationId): Relation $cacheItem = $this->cache->getItem( $this->cacheIdentifierGenerator->generateKey( self::RELATION_IDENTIFIER, - [$relationId] + [$relationId], + true ) ); @@ -522,7 +521,8 @@ public function countReverseRelations(int $destinationContentId, ?int $type = nu $cacheItem = $this->cache->getItem( $this->cacheIdentifierGenerator->generateKey( self::CONTENT_REVERSE_RELATIONS_COUNT_IDENTIFIER, - [$destinationContentId] + [$destinationContentId], + true ) ); From 5cbc85b343087304460ef233c9bb392dda45fcf9 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Mon, 30 May 2022 13:21:47 +0200 Subject: [PATCH 05/21] Added tests --- .../Persistence/Cache/ContentHandlerTest.php | 34 +++++++++++++++++-- .../Legacy/Content/ContentHandlerTest.php | 32 +++++++++++++++++ .../Content/Gateway/DoctrineDatabaseTest.php | 17 ++++++++++ .../Service/Mock/RelationProcessorTest.php | 31 +++++++++-------- 4 files changed, 98 insertions(+), 16 deletions(-) diff --git a/tests/lib/Persistence/Cache/ContentHandlerTest.php b/tests/lib/Persistence/Cache/ContentHandlerTest.php index 4d92153fae..77cb5d5193 100644 --- a/tests/lib/Persistence/Cache/ContentHandlerTest.php +++ b/tests/lib/Persistence/Cache/ContentHandlerTest.php @@ -50,8 +50,8 @@ public function providerForUnCachedMethods(): array ['updateContent', [2, 1, new UpdateStruct()], [['content_version', [2, 1], false]], null, ['c-2-v-1']], //['deleteContent', [2]], own tests for relations complexity ['deleteVersion', [2, 1], [['content_version', [2, 1], false]], null, ['c-2-v-1']], - ['addRelation', [new RelationCreateStruct()]], - ['removeRelation', [66, APIRelation::COMMON]], + ['addRelation', [new RelationCreateStruct(['destinationContentId' => 2])], [['content', [2], false]], null, ['c-2']], + ['removeRelation', [66, APIRelation::COMMON, 2], [['content', [2], false], ['relation', [66], false]], null, ['c-2', 're-66']], ['loadRelations', [2, 1, 3]], ['loadReverseRelations', [2, 3]], ['publish', [2, 3, new MetadataUpdateStruct()], [['content', [2], false]], null, ['c-2']], @@ -85,6 +85,7 @@ 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], ['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], @@ -94,6 +95,7 @@ public function providerForCachedLoadMethodsHit(): array ['loadContentInfo', [2], 'ibx-ci-2', null, null, [['content_info', [], true]], ['ibx-ci'], $info], ['loadContentInfoList', [[2]], 'ibx-ci-2', null, null, [['content_info', [], true]], ['ibx-ci'], [2 => $info], true], ['loadContentInfoByRemoteId', ['3d8jrj'], 'ibx-cibri-3d8jrj', null, null, [['content_info_by_remote_id', [], true]], ['ibx-cibri'], $info], + ['loadRelation', [66], 'ibx-re-66', null, null, [['relation', [66], true]], ['ibx-re-66'], new SPIRelation()], ['loadVersionInfo', [2, 1], 'ibx-cvi-2-1', null, null, [['content_version_info', [2], true]], ['ibx-cvi-2'], $version], ['loadVersionInfo', [2], 'ibx-cvi-2', null, null, [['content_version_info', [2], true]], ['ibx-cvi-2'], $version], ['listVersions', [2], 'ibx-c-2-vl', null, null, [['content_version_list', [2], true]], ['ibx-c-2-vl'], [$version]], @@ -114,6 +116,20 @@ public function providerForCachedLoadMethodsMiss(): 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', + [ + ['content', [2], false], + ], + ['c-2'], + [ + ['content_reverse_relations_count', [2], true], + ], + ['ibx-crrc-2'], + 10, + ], [ 'load', [2, 1], @@ -300,6 +316,20 @@ public function providerForCachedLoadMethodsMiss(): array ['ibx-c-2-vl'], [$version], ], + [ + 'loadRelation', + [66], + 'ibx-re-66', + [ + ['relation', [66], false], + ], + ['re-66'], + [ + ['relation', [66], true], + ], + ['ibx-re-66'], + new SPIRelation(), + ], ]; } diff --git a/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php b/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php index 3f0bdcba1c..c9386ece4d 100644 --- a/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php +++ b/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php @@ -811,6 +811,37 @@ public function testUpdateMetadataUpdatesPathIdentificationString() ); } + /** + * @throws \Ibexa\Core\Base\Exceptions\NotFoundException + */ + public function testLoadRelation(): void + { + $handler = $this->getContentHandler(); + + $gatewayMock = $this->getGatewayMock(); + $mapperMock = $this->getMapperMock(); + $relationFixture = $this->getRelationFixture(); + + $gatewayMock + ->expects(self::once()) + ->method('loadRelation') + ->with(self::equalTo(1)) + ->willReturn([1]); + + $mapperMock + ->expects(self::once()) + ->method('extractRelationsFromRows') + ->with(self::equalTo([1])) + ->willReturn([$relationFixture]); + + $result = $handler->loadRelation(1); + + $this->assertEquals( + $result, + $relationFixture + ); + } + public function testLoadRelations() { $handler = $this->getContentHandler(); @@ -928,6 +959,7 @@ public function testRemoveRelation() protected function getRelationFixture() { $relation = new Relation(); + $relation->id = 1; $relation->sourceContentId = 23; $relation->sourceContentVersionNo = 1; $relation->destinationContentId = 69; diff --git a/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php b/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php index 3ba682dc13..1a153dafae 100644 --- a/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php +++ b/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php @@ -1186,6 +1186,23 @@ public function testDeleteContent() ); } + public function testLoadRelationById() + { + $this->insertRelationFixture(); + + $gateway = $this->getDatabaseGateway(); + + $relation = $gateway->loadRelation(2); + + $this->assertCount(1, $relation, 'Expecting one relation to be loaded'); + + $this->assertValuesInRows( + 'ezcontentobject_link_id', + [2], + $relation + ); + } + /** * @throws \Doctrine\DBAL\DBALException */ diff --git a/tests/lib/Repository/Service/Mock/RelationProcessorTest.php b/tests/lib/Repository/Service/Mock/RelationProcessorTest.php index bf51db889a..f9b98ac687 100644 --- a/tests/lib/Repository/Service/Mock/RelationProcessorTest.php +++ b/tests/lib/Repository/Service/Mock/RelationProcessorTest.php @@ -567,42 +567,45 @@ public function testProcessFieldRelationsRemovesRelations() $contentHandlerMock->expects($this->never())->method('addRelation'); - $contentTypeMock->expects($this->at(0)) + $contentTypeMock->expects(self::at(0)) ->method('getFieldDefinition') ->with($this->equalTo('identifier42')) ->will($this->returnValue(new FieldDefinition(['id' => 42]))); - $contentTypeMock->expects($this->at(1)) + $contentTypeMock->expects(self::at(1)) ->method('getFieldDefinition') ->with($this->equalTo('identifier44')) ->will($this->returnValue(new FieldDefinition(['id' => 44]))); - $contentHandlerMock->expects($this->at(0)) + $contentHandlerMock->expects(self::at(0)) ->method('removeRelation') ->with( - $this->equalTo(7), - $this->equalTo(Relation::EMBED) + self::equalTo(7), + self::equalTo(Relation::EMBED), + self::equalTo(16) ); - $contentHandlerMock->expects($this->at(1)) + $contentHandlerMock->expects(self::at(1)) ->method('removeRelation') ->with( - $this->equalTo(7), - $this->equalTo(Relation::LINK) + self::equalTo(7), + self::equalTo(Relation::LINK), + self::equalTo(16) ); - $contentHandlerMock->expects($this->at(2)) + $contentHandlerMock->expects(self::at(2)) ->method('removeRelation') ->with( - $this->equalTo(4), - $this->equalTo(Relation::FIELD) + self::equalTo(4), + self::equalTo(Relation::FIELD), + self::equalTo(13) ); - $contentHandlerMock->expects($this->at(3)) + $contentHandlerMock->expects(self::at(3)) ->method('removeRelation') ->with( - $this->equalTo(9), - $this->equalTo(Relation::FIELD) + self::equalTo(9), + self::equalTo(Relation::FIELD) ); $relationProcessor->processFieldRelations( From cba5903289227bdaca3a89cf7f4b737b74fab92e Mon Sep 17 00:00:00 2001 From: ciastektk Date: Thu, 2 Jun 2022 09:10:30 +0200 Subject: [PATCH 06/21] Update src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paweł Niedzielski --- src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php index 4e5bc4edd5..c35d28d601 100644 --- a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php +++ b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php @@ -1615,7 +1615,7 @@ public function loadRelation(int $relationId): array $expr = $query->expr(); $query - ->where( + ->andWhere( $expr->eq('id', ':relationId') ) ->setParameter('relationId', $relationId, ParameterType::INTEGER); From 75c2038b624a090865b3873625d422e9f57a2431 Mon Sep 17 00:00:00 2001 From: ciastektk Date: Thu, 2 Jun 2022 09:11:03 +0200 Subject: [PATCH 07/21] Update tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paweł Niedzielski --- tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php b/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php index c9386ece4d..a68861926d 100644 --- a/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php +++ b/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php @@ -825,7 +825,7 @@ public function testLoadRelation(): void $gatewayMock ->expects(self::once()) ->method('loadRelation') - ->with(self::equalTo(1)) + ->with(1) ->willReturn([1]); $mapperMock From d035fd11c71a031093a72abe235e56c504d6d4b6 Mon Sep 17 00:00:00 2001 From: ciastektk Date: Thu, 2 Jun 2022 09:11:08 +0200 Subject: [PATCH 08/21] Update tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paweł Niedzielski --- tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php b/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php index a68861926d..76f8d5f4f4 100644 --- a/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php +++ b/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php @@ -831,7 +831,7 @@ public function testLoadRelation(): void $mapperMock ->expects(self::once()) ->method('extractRelationsFromRows') - ->with(self::equalTo([1])) + ->with([1]) ->willReturn([$relationFixture]); $result = $handler->loadRelation(1); From 6ca3f5c86b8ca17938b91a10355fae1368517f69 Mon Sep 17 00:00:00 2001 From: ciastektk Date: Thu, 2 Jun 2022 09:11:14 +0200 Subject: [PATCH 09/21] Update tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paweł Niedzielski --- .../Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php b/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php index 1a153dafae..9339e9540a 100644 --- a/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php +++ b/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php @@ -1186,7 +1186,7 @@ public function testDeleteContent() ); } - public function testLoadRelationById() + public function testLoadRelationById(): void { $this->insertRelationFixture(); From b5612fe5ad3b9c7a2fae0d86f117369684179921 Mon Sep 17 00:00:00 2001 From: ciastektk Date: Thu, 2 Jun 2022 09:11:57 +0200 Subject: [PATCH 10/21] Update src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php Co-authored-by: Andrew Longosz --- .../Persistence/Legacy/Content/Gateway/DoctrineDatabase.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php index c35d28d601..2b3f904787 100644 --- a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php +++ b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php @@ -1609,6 +1609,10 @@ public function insertRelation(RelationCreateStruct $createStruct): int return (int)$this->connection->lastInsertId(self::CONTENT_RELATION_SEQ); } + /** + * @throws \Doctrine\DBAL\Driver\Exception + * @throws \Doctrine\DBAL\Exception + */ public function loadRelation(int $relationId): array { $query = $this->queryBuilder->createRelationFindQueryBuilder(); From 96354287b162d64bb6dfa4c1047ff95628e2c01a Mon Sep 17 00:00:00 2001 From: ciastektk Date: Thu, 2 Jun 2022 09:12:23 +0200 Subject: [PATCH 11/21] Update tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php Co-authored-by: Andrew Longosz --- tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php b/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php index 76f8d5f4f4..ab603be92c 100644 --- a/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php +++ b/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php @@ -812,7 +812,7 @@ public function testUpdateMetadataUpdatesPathIdentificationString() } /** - * @throws \Ibexa\Core\Base\Exceptions\NotFoundException + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException */ public function testLoadRelation(): void { From 8059fa05baf7b5e791dc4b613f86b43b7512074f Mon Sep 17 00:00:00 2001 From: ciastektk Date: Thu, 2 Jun 2022 09:12:32 +0200 Subject: [PATCH 12/21] Update tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php Co-authored-by: Andrew Longosz --- .../Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php b/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php index 9339e9540a..dafe262a79 100644 --- a/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php +++ b/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php @@ -1194,7 +1194,7 @@ public function testLoadRelationById(): void $relation = $gateway->loadRelation(2); - $this->assertCount(1, $relation, 'Expecting one relation to be loaded'); + self::assertCount(1, $relation, 'Expecting one relation to be loaded'); $this->assertValuesInRows( 'ezcontentobject_link_id', From eb8c399aa5e867ff60c59e6c40f9257c361669f8 Mon Sep 17 00:00:00 2001 From: ciastektk Date: Thu, 2 Jun 2022 09:12:41 +0200 Subject: [PATCH 13/21] Update src/lib/Persistence/Legacy/Content/Handler.php Co-authored-by: Andrew Longosz --- src/lib/Persistence/Legacy/Content/Handler.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib/Persistence/Legacy/Content/Handler.php b/src/lib/Persistence/Legacy/Content/Handler.php index c5c3881a94..83b9d9fc27 100644 --- a/src/lib/Persistence/Legacy/Content/Handler.php +++ b/src/lib/Persistence/Legacy/Content/Handler.php @@ -777,6 +777,9 @@ public function addRelation(RelationCreateStruct $createStruct) return $relation; } + /** + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException + */ public function loadRelation(int $relationId): Relation { $rows = $this->contentGateway->loadRelation($relationId); From 9ddfbaa2a31f09ababb9b038ec0bf5c7c52c5948 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Thu, 2 Jun 2022 11:09:27 +0200 Subject: [PATCH 14/21] Added deprecated info when destinationContentId is not passed --- src/lib/Persistence/Cache/ContentHandler.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lib/Persistence/Cache/ContentHandler.php b/src/lib/Persistence/Cache/ContentHandler.php index ad56e681ed..ff43c45de6 100644 --- a/src/lib/Persistence/Cache/ContentHandler.php +++ b/src/lib/Persistence/Cache/ContentHandler.php @@ -451,6 +451,10 @@ public function addRelation(RelationCreateStruct $relation) */ public function removeRelation($relationId, $type, ?int $destinationContentId = null) { + if (null === $destinationContentId) { + @trigger_error('Expecting to pass $destinationContentId argument since version 4.1.5', E_USER_DEPRECATED); + } + $this->logger->logCall(__METHOD__, ['relation' => $relationId, 'type' => $type]); $this->cache->invalidateTags([ From dc43e242abe8aacdf4ec0a8f9b7423a4dbd2943a Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Thu, 2 Jun 2022 11:11:26 +0200 Subject: [PATCH 15/21] Fixed phpdoc for removeRelation method --- src/contracts/Persistence/Content/Handler.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/contracts/Persistence/Content/Handler.php b/src/contracts/Persistence/Content/Handler.php index 2e718d8dff..f7909a1dc5 100644 --- a/src/contracts/Persistence/Content/Handler.php +++ b/src/contracts/Persistence/Content/Handler.php @@ -272,14 +272,14 @@ public function loadRelation(int $relationId): Relation; /** * Removes a relation by relation Id. - * * @todo Should the existence verifications happen here or is this supposed to be handled at a higher level? * - * @param mixed $relationId + * @param int $relationId * @param int $type {@see \Ibexa\Contracts\Core\Repository\Values\Content\Relation::COMMON, * \Ibexa\Contracts\Core\Repository\Values\Content\Relation::EMBED, * \Ibexa\Contracts\Core\Repository\Values\Content\Relation::LINK, * \Ibexa\Contracts\Core\Repository\Values\Content\Relation::FIELD} + * @param ?int $destinationContentId Content id to invalidate cache tag related to content reverse relations count */ public function removeRelation($relationId, $type, ?int $destinationContentId = null); From a7d3474298e21d54f0fbcd21c63fb25cde42e5a2 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Thu, 2 Jun 2022 11:11:45 +0200 Subject: [PATCH 16/21] Used const for relationId --- .../Legacy/Content/ContentHandlerTest.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php b/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php index ab603be92c..699cd85dcc 100644 --- a/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php +++ b/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php @@ -38,6 +38,8 @@ */ class ContentHandlerTest extends TestCase { + private const RELATION_ID = 1; + /** * Content handler to test. * @@ -825,16 +827,16 @@ public function testLoadRelation(): void $gatewayMock ->expects(self::once()) ->method('loadRelation') - ->with(1) - ->willReturn([1]); + ->with(self::RELATION_ID) + ->willReturn([self::RELATION_ID]); $mapperMock ->expects(self::once()) ->method('extractRelationsFromRows') - ->with([1]) + ->with([self::RELATION_ID]) ->willReturn([$relationFixture]); - $result = $handler->loadRelation(1); + $result = $handler->loadRelation(self::RELATION_ID); $this->assertEquals( $result, @@ -959,7 +961,7 @@ public function testRemoveRelation() protected function getRelationFixture() { $relation = new Relation(); - $relation->id = 1; + $relation->id = self::RELATION_ID; $relation->sourceContentId = 23; $relation->sourceContentVersionNo = 1; $relation->destinationContentId = 69; From 80a45c5921d8f518cf413c8387451616f388da89 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Thu, 2 Jun 2022 11:13:12 +0200 Subject: [PATCH 17/21] Fixed cs --- src/contracts/Persistence/Content/Handler.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/contracts/Persistence/Content/Handler.php b/src/contracts/Persistence/Content/Handler.php index f7909a1dc5..66942eff4d 100644 --- a/src/contracts/Persistence/Content/Handler.php +++ b/src/contracts/Persistence/Content/Handler.php @@ -272,6 +272,7 @@ public function loadRelation(int $relationId): Relation; /** * Removes a relation by relation Id. + * * @todo Should the existence verifications happen here or is this supposed to be handled at a higher level? * * @param int $relationId From bb9f76881a5c0ad7b7f102651cecf1aae77b4e43 Mon Sep 17 00:00:00 2001 From: ciastektk Date: Fri, 3 Jun 2022 12:34:43 +0200 Subject: [PATCH 18/21] Update src/lib/Persistence/Cache/ContentHandler.php Co-authored-by: Andrew Longosz --- src/lib/Persistence/Cache/ContentHandler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Persistence/Cache/ContentHandler.php b/src/lib/Persistence/Cache/ContentHandler.php index ff43c45de6..5e9b28eebb 100644 --- a/src/lib/Persistence/Cache/ContentHandler.php +++ b/src/lib/Persistence/Cache/ContentHandler.php @@ -449,7 +449,7 @@ public function addRelation(RelationCreateStruct $relation) /** * {@inheritdoc} */ - public function removeRelation($relationId, $type, ?int $destinationContentId = null) + public function removeRelation($relationId, $type, ?int $destinationContentId = null): void { if (null === $destinationContentId) { @trigger_error('Expecting to pass $destinationContentId argument since version 4.1.5', E_USER_DEPRECATED); From d7195d8289eb9ade83eb88c5ddcaa7406997fab4 Mon Sep 17 00:00:00 2001 From: ciastektk Date: Fri, 3 Jun 2022 12:34:49 +0200 Subject: [PATCH 19/21] Update src/lib/Persistence/Legacy/Content/Handler.php Co-authored-by: Andrew Longosz --- src/lib/Persistence/Legacy/Content/Handler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Persistence/Legacy/Content/Handler.php b/src/lib/Persistence/Legacy/Content/Handler.php index 83b9d9fc27..0a862a0dc6 100644 --- a/src/lib/Persistence/Legacy/Content/Handler.php +++ b/src/lib/Persistence/Legacy/Content/Handler.php @@ -802,7 +802,7 @@ public function loadRelation(int $relationId): Relation * \Ibexa\Contracts\Core\Repository\Values\Content\Relation::LINK, * \Ibexa\Contracts\Core\Repository\Values\Content\Relation::FIELD} */ - public function removeRelation($relationId, $type, ?int $destinationContentId = null) + public function removeRelation($relationId, $type, ?int $destinationContentId = null): void { $this->contentGateway->deleteRelation($relationId, $type); } From 79e6d11bfa568995f7af0da731435b903215466a Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Fri, 3 Jun 2022 13:43:38 +0200 Subject: [PATCH 20/21] Fixes after review --- src/contracts/Persistence/Content/Handler.php | 4 ---- .../Content/Gateway/DoctrineDatabase.php | 15 +++++++++++- .../Persistence/Legacy/Content/Handler.php | 10 +++----- src/lib/Persistence/Legacy/Content/Mapper.php | 2 +- .../Content/Gateway/DoctrineDatabaseTest.php | 24 +++++++++++++------ 5 files changed, 35 insertions(+), 20 deletions(-) diff --git a/src/contracts/Persistence/Content/Handler.php b/src/contracts/Persistence/Content/Handler.php index 66942eff4d..c8e09d3aa0 100644 --- a/src/contracts/Persistence/Content/Handler.php +++ b/src/contracts/Persistence/Content/Handler.php @@ -263,10 +263,6 @@ public function addRelation(RelationCreateStruct $createStruct); /** * Load a relation by $relationId. - * - * @param int $relationId Relation ID - * - * @return \Ibexa\Contracts\Core\Persistence\Content\Relation */ public function loadRelation(int $relationId): Relation; diff --git a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php index 2b3f904787..9723e130a4 100644 --- a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php +++ b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php @@ -25,12 +25,14 @@ use Ibexa\Contracts\Core\Repository\Values\Content\Relation; use Ibexa\Contracts\Core\Repository\Values\Content\VersionInfo as APIVersionInfo; use Ibexa\Core\Base\Exceptions\BadStateException; +use Ibexa\Core\Base\Exceptions\NotFoundException; use Ibexa\Core\Base\Exceptions\NotFoundException as NotFound; use Ibexa\Core\Persistence\Legacy\Content\Gateway; use Ibexa\Core\Persistence\Legacy\Content\Gateway\DoctrineDatabase\QueryBuilder; use Ibexa\Core\Persistence\Legacy\Content\Language\MaskGenerator as LanguageMaskGenerator; use Ibexa\Core\Persistence\Legacy\Content\StorageFieldValue; use Ibexa\Core\Persistence\Legacy\SharedGateway\Gateway as SharedGateway; +use LogicException; /** * Doctrine database based content gateway. @@ -1612,6 +1614,7 @@ public function insertRelation(RelationCreateStruct $createStruct): int /** * @throws \Doctrine\DBAL\Driver\Exception * @throws \Doctrine\DBAL\Exception + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException */ public function loadRelation(int $relationId): array { @@ -1624,7 +1627,17 @@ public function loadRelation(int $relationId): array ) ->setParameter('relationId', $relationId, ParameterType::INTEGER); - return $query->execute()->fetchAllAssociative(); + $result = $query->execute()->fetchAllAssociative(); + $resultCount = count($result); + if ($resultCount === 0) { + throw new NotFoundException('Relation', $relationId); + } + + if ($resultCount > 1) { + throw new LogicException('More then one row found for the relation id: ' . $relationId); + } + + return current($result); } public function deleteRelation(int $relationId, int $type): void diff --git a/src/lib/Persistence/Legacy/Content/Handler.php b/src/lib/Persistence/Legacy/Content/Handler.php index 0a862a0dc6..c8db9a38ab 100644 --- a/src/lib/Persistence/Legacy/Content/Handler.php +++ b/src/lib/Persistence/Legacy/Content/Handler.php @@ -782,13 +782,9 @@ public function addRelation(RelationCreateStruct $createStruct) */ public function loadRelation(int $relationId): Relation { - $rows = $this->contentGateway->loadRelation($relationId); - - if (empty($rows)) { - throw new NotFound('relation', "relationId: $relationId"); - } - - return current($this->mapper->extractRelationsFromRows($rows)); + return $this->mapper->extractRelationFromRow( + $this->contentGateway->loadRelation($relationId) + ); } /** diff --git a/src/lib/Persistence/Legacy/Content/Mapper.php b/src/lib/Persistence/Legacy/Content/Mapper.php index 59ce7dfa8f..b7cf7823be 100644 --- a/src/lib/Persistence/Legacy/Content/Mapper.php +++ b/src/lib/Persistence/Legacy/Content/Mapper.php @@ -542,7 +542,7 @@ public function extractRelationsFromRows(array $rows) * * @return \Ibexa\Contracts\Core\Persistence\Content\Relation */ - protected function extractRelationFromRow(array $row) + public function extractRelationFromRow(array $row) { $relation = new Relation(); $relation->id = (int)$row['ezcontentobject_link_id']; diff --git a/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php b/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php index dafe262a79..8d8a004560 100644 --- a/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php +++ b/tests/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabaseTest.php @@ -15,6 +15,7 @@ use Ibexa\Contracts\Core\Persistence\Content\Relation\CreateStruct as RelationCreateStruct; use Ibexa\Contracts\Core\Persistence\Content\UpdateStruct; use Ibexa\Contracts\Core\Persistence\Content\VersionInfo; +use Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException; use Ibexa\Contracts\Core\Repository\Values\Content\Relation as RelationValue; use Ibexa\Core\Persistence\Legacy\Content\Gateway\DoctrineDatabase; use Ibexa\Core\Persistence\Legacy\Content\StorageFieldValue; @@ -1186,20 +1187,29 @@ public function testDeleteContent() ); } + public function testLoadRelationThrowNotFoundExceptionWhenRelationNotFound(): void + { + $this->expectException(NotFoundException::class); + $this->expectExceptionMessage('Could not find \'Relation\' with identifier \'3\''); + + $this->insertRelationFixture(); + $gateway = $this->getDatabaseGateway(); + $gateway->loadRelation(3); + } + public function testLoadRelationById(): void { + $relationId = 2; + $this->insertRelationFixture(); $gateway = $this->getDatabaseGateway(); - $relation = $gateway->loadRelation(2); - - self::assertCount(1, $relation, 'Expecting one relation to be loaded'); + $relation = $gateway->loadRelation($relationId); - $this->assertValuesInRows( - 'ezcontentobject_link_id', - [2], - $relation + self::assertEquals( + $relationId, + $relation['ezcontentobject_link_id'] ); } From e6ad2c6d22f5d72f3da0cab9119ad6c8d31085ea Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Fri, 3 Jun 2022 13:55:04 +0200 Subject: [PATCH 21/21] Fixed ContentHandlerTest::testLoadRelation --- tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php b/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php index 699cd85dcc..2062a56c31 100644 --- a/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php +++ b/tests/lib/Persistence/Legacy/Content/ContentHandlerTest.php @@ -832,9 +832,9 @@ public function testLoadRelation(): void $mapperMock ->expects(self::once()) - ->method('extractRelationsFromRows') + ->method('extractRelationFromRow') ->with([self::RELATION_ID]) - ->willReturn([$relationFixture]); + ->willReturn($relationFixture); $result = $handler->loadRelation(self::RELATION_ID);