From 33b354de9165b1a0bfdd6caa4399385d192a85e1 Mon Sep 17 00:00:00 2001 From: Muhammad Jaziraly Date: Mon, 7 Aug 2023 17:42:24 +0200 Subject: [PATCH] REST: Extract getStatementSubject to a new helper class * As the function is now needed in multiple places, I thought having it in a seperate class would be a good idea. Change-Id: I7677486e57b68e5c186e530c5e723272e79ed92e --- ...EntityRevisionLookupStatementRetriever.php | 31 +--- .../DataAccess/StatementSubjectRetriever.php | 41 ++++++ repo/rest-api/src/WbRestApi.ServiceWiring.php | 3 +- ...tyRevisionLookupStatementRetrieverTest.php | 33 +---- .../StatementSubjectRetrieverTest.php | 134 ++++++++++++++++++ 5 files changed, 183 insertions(+), 59 deletions(-) create mode 100644 repo/rest-api/src/Infrastructure/DataAccess/StatementSubjectRetriever.php create mode 100644 repo/rest-api/tests/phpunit/Infrastructure/DataAccess/StatementSubjectRetrieverTest.php diff --git a/repo/rest-api/src/Infrastructure/DataAccess/EntityRevisionLookupStatementRetriever.php b/repo/rest-api/src/Infrastructure/DataAccess/EntityRevisionLookupStatementRetriever.php index a2970cc14ae..dbc1d802e96 100644 --- a/repo/rest-api/src/Infrastructure/DataAccess/EntityRevisionLookupStatementRetriever.php +++ b/repo/rest-api/src/Infrastructure/DataAccess/EntityRevisionLookupStatementRetriever.php @@ -2,11 +2,7 @@ namespace Wikibase\Repo\RestApi\Infrastructure\DataAccess; -use Wikibase\DataModel\Entity\EntityId; -use Wikibase\DataModel\Entity\StatementListProvidingEntity; use Wikibase\DataModel\Statement\StatementGuid; -use Wikibase\Lib\Store\EntityRevisionLookup; -use Wikibase\Lib\Store\RevisionedUnresolvedRedirectException; use Wikibase\Repo\RestApi\Domain\ReadModel\Statement; use Wikibase\Repo\RestApi\Domain\Services\StatementReadModelConverter; use Wikibase\Repo\RestApi\Domain\Services\StatementRetriever; @@ -16,20 +12,20 @@ */ class EntityRevisionLookupStatementRetriever implements StatementRetriever { - private EntityRevisionLookup $entityRevisionLookup; + private StatementSubjectRetriever $statementSubjectRetriever; private StatementReadModelConverter $statementReadModelConverter; public function __construct( - EntityRevisionLookup $entityRevisionLookup, + StatementSubjectRetriever $statementSubjectRetriever, StatementReadModelConverter $statementReadModelConverter ) { - $this->entityRevisionLookup = $entityRevisionLookup; + $this->statementSubjectRetriever = $statementSubjectRetriever; $this->statementReadModelConverter = $statementReadModelConverter; } public function getStatement( StatementGuid $statementId ): ?Statement { $subjectId = $statementId->getEntityId(); - $subject = $this->getStatementSubject( $subjectId ); + $subject = $this->statementSubjectRetriever->getStatementSubject( $subjectId ); if ( $subject === null ) { return null; @@ -39,23 +35,4 @@ public function getStatement( StatementGuid $statementId ): ?Statement { return $statement ? $this->statementReadModelConverter->convert( $statement ) : null; } - private function getStatementSubject( EntityId $subjectId ): ?StatementListProvidingEntity { - try { - $entityRevision = $this->entityRevisionLookup->getEntityRevision( $subjectId ); - } catch ( RevisionedUnresolvedRedirectException $e ) { - return null; - } - - if ( !$entityRevision ) { - return null; - } - - $subject = $entityRevision->getEntity(); - if ( !$subject instanceof StatementListProvidingEntity ) { - return null; - } - - return $subject; - } - } diff --git a/repo/rest-api/src/Infrastructure/DataAccess/StatementSubjectRetriever.php b/repo/rest-api/src/Infrastructure/DataAccess/StatementSubjectRetriever.php new file mode 100644 index 00000000000..a8a0085b74e --- /dev/null +++ b/repo/rest-api/src/Infrastructure/DataAccess/StatementSubjectRetriever.php @@ -0,0 +1,41 @@ +entityRevisionLookup = $entityRevisionLookup; + } + + public function getStatementSubject( EntityId $subjectId ): ?StatementListProvidingEntity { + try { + $entityRevision = $this->entityRevisionLookup->getEntityRevision( $subjectId ); + } catch ( RevisionedUnresolvedRedirectException $e ) { + return null; + } + + if ( !$entityRevision ) { + return null; + } + + $subject = $entityRevision->getEntity(); + if ( !$subject instanceof StatementListProvidingEntity ) { + throw new LogicException( 'Entity is not a ' . StatementListProvidingEntity::class ); + } + + return $subject; + } + +} diff --git a/repo/rest-api/src/WbRestApi.ServiceWiring.php b/repo/rest-api/src/WbRestApi.ServiceWiring.php index d7daac4e25c..3472c633cae 100644 --- a/repo/rest-api/src/WbRestApi.ServiceWiring.php +++ b/repo/rest-api/src/WbRestApi.ServiceWiring.php @@ -69,6 +69,7 @@ use Wikibase\Repo\RestApi\Infrastructure\DataAccess\EntityRevisionLookupStatementRetriever; use Wikibase\Repo\RestApi\Infrastructure\DataAccess\MediaWikiEditEntityFactoryItemUpdater; use Wikibase\Repo\RestApi\Infrastructure\DataAccess\PrefetchingTermLookupAliasesRetriever; +use Wikibase\Repo\RestApi\Infrastructure\DataAccess\StatementSubjectRetriever; use Wikibase\Repo\RestApi\Infrastructure\DataAccess\TermLookupItemDataRetriever; use Wikibase\Repo\RestApi\Infrastructure\DataAccess\WikibaseEntityPermissionChecker; use Wikibase\Repo\RestApi\Infrastructure\DataAccess\WikibaseEntityRevisionLookupItemRevisionMetadataRetriever; @@ -473,7 +474,7 @@ 'WbRestApi.StatementRetriever' => function( MediaWikiServices $services ): EntityRevisionLookupStatementRetriever { return new EntityRevisionLookupStatementRetriever( - WikibaseRepo::getEntityRevisionLookup( $services ), + new StatementSubjectRetriever( WikibaseRepo::getEntityRevisionLookup( $services ) ), new StatementReadModelConverter( WikibaseRepo::getStatementGuidParser( $services ), WikibaseRepo::getPropertyDataTypeLookup() diff --git a/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/EntityRevisionLookupStatementRetrieverTest.php b/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/EntityRevisionLookupStatementRetrieverTest.php index c2b6387b503..2a0e22fd4dd 100644 --- a/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/EntityRevisionLookupStatementRetrieverTest.php +++ b/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/EntityRevisionLookupStatementRetrieverTest.php @@ -10,8 +10,6 @@ use Wikibase\DataModel\Entity\NumericPropertyId; use Wikibase\DataModel\Entity\Property; use Wikibase\DataModel\Entity\StatementListProvidingEntity as StatementSubject; -use Wikibase\DataModel\Fixtures\CustomEntityId; -use Wikibase\DataModel\Services\Fixtures\FakeEntityDocument; use Wikibase\DataModel\Statement\StatementGuid; use Wikibase\DataModel\Statement\StatementList; use Wikibase\DataModel\Term\Fingerprint; @@ -19,9 +17,9 @@ use Wikibase\DataModel\Tests\NewStatement; use Wikibase\Lib\Store\EntityRevision; use Wikibase\Lib\Store\EntityRevisionLookup; -use Wikibase\Lib\Store\RevisionedUnresolvedRedirectException; use Wikibase\Repo\RestApi\Domain\ReadModel\Statement; use Wikibase\Repo\RestApi\Infrastructure\DataAccess\EntityRevisionLookupStatementRetriever; +use Wikibase\Repo\RestApi\Infrastructure\DataAccess\StatementSubjectRetriever; /** * @covers \Wikibase\Repo\RestApi\Infrastructure\DataAccess\EntityRevisionLookupStatementRetriever @@ -93,14 +91,6 @@ public function provideSubjectId(): Generator { yield 'Property ID' => [ new NumericPropertyId( 'P123' ) ]; } - public function testGivenItemRedirected_getStatementReturnsNull(): void { - $itemId = new ItemId( 'Q321' ); - $statementId = new StatementGuid( $itemId, 'c48c32c3-42b5-498f-9586-84608b88747c' ); - $this->entityRevisionLookup = $this->newLookupForIdWithRedirect( $itemId ); - - $this->assertNull( $this->newRetriever()->getStatement( $statementId ) ); - } - /** * @dataProvider provideStatementSubjectWithoutStatement */ @@ -121,15 +111,6 @@ public function provideStatementSubjectWithoutStatement(): Generator { ]; } - public function testGivenEntityIsNotStatementSubject_getStatementReturnsNull(): void { - $subjectId = new CustomEntityId( 'A123' ); - $subject = new FakeEntityDocument( $subjectId ); - $statementId = new StatementGuid( $subjectId, '69460e7f-4c45-d417-9420-9431a47969a8' ); - $this->entityRevisionLookup = $this->newLookupForIdWithReturnValue( $subjectId, $subject ); - - $this->assertNull( $this->newRetriever()->getStatement( $statementId ) ); - } - private function newLookupForIdWithReturnValue( EntityId $id, ?EntityDocument $returnValue ): EntityRevisionLookup { $entityRevisionLookup = $this->createMock( EntityRevisionLookup::class ); $entityRevisionLookup->expects( $this->once() ) @@ -140,19 +121,9 @@ private function newLookupForIdWithReturnValue( EntityId $id, ?EntityDocument $r return $entityRevisionLookup; } - private function newLookupForIdWithRedirect( EntityId $id ): EntityRevisionLookup { - $entityRevisionLookup = $this->createMock( EntityRevisionLookup::class ); - $entityRevisionLookup->expects( $this->once() ) - ->method( 'getEntityRevision' ) - ->with( $id ) - ->willThrowException( $this->createStub( RevisionedUnresolvedRedirectException::class ) ); - - return $entityRevisionLookup; - } - private function newRetriever(): EntityRevisionLookupStatementRetriever { return new EntityRevisionLookupStatementRetriever( - $this->entityRevisionLookup, + new StatementSubjectRetriever( $this->entityRevisionLookup ), $this->newStatementReadModelConverter() ); } diff --git a/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/StatementSubjectRetrieverTest.php b/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/StatementSubjectRetrieverTest.php new file mode 100644 index 00000000000..fc3cc669fe5 --- /dev/null +++ b/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/StatementSubjectRetrieverTest.php @@ -0,0 +1,134 @@ +entityRevisionLookup = $this->createStub( EntityRevisionLookup::class ); + } + + /** + * @dataProvider provideSubjectAndSubjectId + */ + public function testGetStatementSubject( EntityId $subjectId, EntityDocument $expectedSubject ): void { + $this->entityRevisionLookup = $this->newLookupForIdWithReturnValue( $subjectId, $expectedSubject ); + $subject = $this->newStatementSubjectRetriever()->getStatementSubject( $subjectId ); + + $this->assertEquals( $expectedSubject, $subject ); + } + + /** + * @dataProvider provideSubjectId + */ + public function testGivenSubjectDoesNotExist_getStatementSubjectReturnsNull( EntityId $subjectId ): void { + $this->entityRevisionLookup = $this->newLookupForIdWithReturnValue( $subjectId, null ); + + $this->assertNull( $this->newStatementSubjectRetriever()->getStatementSubject( $subjectId ) ); + } + + public function testGivenEntityIsNotStatementSubject_getStatementSubjectThrows(): void { + $subjectId = new CustomEntityId( 'A123' ); + $subject = new FakeEntityDocument( $subjectId ); + $this->entityRevisionLookup = $this->newLookupForIdWithReturnValue( $subjectId, $subject ); + + try { + $this->newStatementSubjectRetriever()->getStatementSubject( $subjectId ); + $this->fail( 'Expected exception not thrown' ); + } catch ( LogicException $e ) { + $this->assertEquals( 'Entity is not a ' . StatementListProvidingEntity::class, $e->getMessage() ); + } + } + + public function testGivenItemRedirected_getStatementSubjectReturnsNull(): void { + $itemId = new ItemId( 'Q321' ); + $this->entityRevisionLookup = $this->newLookupForIdWithRedirect( $itemId ); + + $this->assertNull( $this->newStatementSubjectRetriever()->getStatementSubject( $itemId ) ); + } + + public function provideSubjectAndSubjectId(): Generator { + $itemId = new ItemId( 'Q123' ); + $statementId = new StatementGuid( $itemId, 'c48c32c3-42b5-498f-9586-84608b88747c' ); + $statement = NewStatement::forProperty( 'P123' ) + ->withValue( 'potato' ) + ->withGuid( $statementId ) + ->build(); + + yield 'Item' => [ $itemId, NewItem::withId( $itemId )->andStatement( $statement )->build() ]; + + $propertyId = new NumericPropertyId( 'P567' ); + $statementId = new StatementGuid( $propertyId, 'c48c32c3-42b5-498f-9586-84608b88747c' ); + $statement = NewStatement::forProperty( 'P123' ) + ->withValue( 'potato' ) + ->withGuid( $statementId ) + ->build(); + + yield 'Property' => [ + $propertyId, + new Property( $propertyId, new Fingerprint(), 'string', new StatementList( $statement ) ), + ]; + } + + public function provideSubjectId(): Generator { + yield 'Item ID' => [ new ItemId( 'Q123' ) ]; + yield 'Property ID' => [ new NumericPropertyId( 'P123' ) ]; + } + + private function newLookupForIdWithRedirect( EntityId $id ): EntityRevisionLookup { + $entityRevisionLookup = $this->createMock( EntityRevisionLookup::class ); + $entityRevisionLookup->expects( $this->once() ) + ->method( 'getEntityRevision' ) + ->with( $id ) + ->willThrowException( $this->createStub( RevisionedUnresolvedRedirectException::class ) ); + + return $entityRevisionLookup; + } + + private function newLookupForIdWithReturnValue( EntityId $id, ?EntityDocument $returnValue ): EntityRevisionLookup { + $entityRevisionLookup = $this->createMock( EntityRevisionLookup::class ); + $entityRevisionLookup->expects( $this->once() ) + ->method( 'getEntityRevision' ) + ->with( $id ) + ->willReturn( $returnValue ? new EntityRevision( $returnValue ) : null ); + + return $entityRevisionLookup; + } + + private function newStatementSubjectRetriever(): StatementSubjectRetriever { + return new StatementSubjectRetriever( $this->entityRevisionLookup ); + } + +}