Merge "REST: Implement fallback in GetPropertyLabelWithFallback"

jenkins-bot · Gerrit Code Review · commit f1a6045334fa · 2024-10-02T12:13:02.000Z
diff --git a/repo/rest-api/specs/resources/labels/label-with-fallback-for-property.json b/repo/rest-api/specs/resources/labels/label-with-fallback-for-property.json
@@ -16,6 +16,7 @@
 		"responses": {
 			"200": { "$ref": "../../global/responses.json#/Label" },
 			"304": { "$ref": "../../global/responses.json#/NotModified" },
+			"307": { "$ref": "../../global/responses.json#/MovedTemporarily" },
 			"400": { "$ref": "../../global/responses.json#/InvalidTermByLanguageInput" },
 			"404": { "$ref": "../../global/responses.json#/ResourceNotFound" },
 			"412": { "$ref": "../../global/responses.json#/PreconditionFailedError" },
diff --git a/repo/rest-api/src/Application/UseCases/GetPropertyLabelWithFallback/GetPropertyLabelWithFallback.php b/repo/rest-api/src/Application/UseCases/GetPropertyLabelWithFallback/GetPropertyLabelWithFallback.php
@@ -4,21 +4,21 @@
 
 use Wikibase\Repo\RestApi\Application\UseCases\GetLatestPropertyRevisionMetadata;
 use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
-use Wikibase\Repo\RestApi\Domain\Services\PropertyLabelRetriever;
+use Wikibase\Repo\RestApi\Domain\Services\PropertyLabelWithFallbackRetriever;
 
 /**
  * @license GPL-2.0-or-later
  */
 class GetPropertyLabelWithFallback {
 
-	private PropertyLabelRetriever $labelRetriever;
+	private PropertyLabelWithFallbackRetriever $labelRetriever;
 	private GetLatestPropertyRevisionMetadata $getRevisionMetadata;
 	private GetPropertyLabelWithFallbackValidator $validator;
 
 	public function __construct(
 		GetPropertyLabelWithFallbackValidator $validator,
 		GetLatestPropertyRevisionMetadata $getRevisionMetadata,
-		PropertyLabelRetriever $labelRetriever
+		PropertyLabelWithFallbackRetriever $labelRetriever
 	) {
 		$this->labelRetriever = $labelRetriever;
 		$this->getRevisionMetadata = $getRevisionMetadata;
diff --git a/repo/rest-api/src/Domain/Services/PropertyLabelWithFallbackRetriever.php b/repo/rest-api/src/Domain/Services/PropertyLabelWithFallbackRetriever.php
@@ -0,0 +1,15 @@
+<?php declare( strict_types=1 );
+
+namespace Wikibase\Repo\RestApi\Domain\Services;
+
+use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\Repo\RestApi\Domain\ReadModel\Label;
+
+/**
+ * @license GPL-2.0-or-later
+ */
+interface PropertyLabelWithFallbackRetriever {
+
+	public function getLabel( PropertyId $propertyId, string $languageCode ): ?Label;
+
+}
diff --git a/repo/rest-api/src/Infrastructure/DataAccess/FallbackLookupFactoryTermsRetriever.php b/repo/rest-api/src/Infrastructure/DataAccess/FallbackLookupFactoryTermsRetriever.php
@@ -8,11 +8,12 @@
 use Wikibase\Lib\Store\FallbackLabelDescriptionLookupFactory;
 use Wikibase\Repo\RestApi\Domain\ReadModel\Label;
 use Wikibase\Repo\RestApi\Domain\Services\ItemLabelWithFallbackRetriever;
+use Wikibase\Repo\RestApi\Domain\Services\PropertyLabelWithFallbackRetriever;
 
 /**
  * @license GPL-2.0-or-later
  */
-class FallbackLookupFactoryTermsRetriever implements ItemLabelWithFallbackRetriever {
+class FallbackLookupFactoryTermsRetriever implements ItemLabelWithFallbackRetriever, PropertyLabelWithFallbackRetriever {
 
 	private FallbackLabelDescriptionLookupFactory $lookupFactory;
 	private LanguageFactory $languageFactory;
diff --git a/repo/rest-api/src/RouteHandlers/GetPropertyLabelRouteHandler.php b/repo/rest-api/src/RouteHandlers/GetPropertyLabelRouteHandler.php
@@ -25,6 +25,7 @@ class GetPropertyLabelRouteHandler extends SimpleHandler {
 
 	private const PROPERTY_ID_PATH_PARAM = 'property_id';
 	private const LANGUAGE_CODE_PATH_PARAM = 'language_code';
+	public const ROUTE = '/wikibase/v0/entities/properties/{property_id}/labels/{language_code}';
 
 	private GetPropertyLabel $useCase;
 	private MiddlewareHandler $middlewareHandler;
diff --git a/repo/rest-api/src/RouteHandlers/GetPropertyLabelWithFallbackRouteHandler.php b/repo/rest-api/src/RouteHandlers/GetPropertyLabelWithFallbackRouteHandler.php
@@ -65,8 +65,11 @@ public function run( ...$args ): Response {
 
 	public function runUseCase( string $propertyId, string $languageCode ): Response {
 		try {
-			$useCaseResponse = $this->useCase->execute( new GetPropertyLabelWithFallbackRequest( $propertyId, $languageCode ) );
-			return $this->newSuccessResponse( $useCaseResponse );
+			$response = $this->useCase->execute( new GetPropertyLabelWithFallbackRequest( $propertyId, $languageCode ) );
+
+			return $response->getLabel()->getLanguageCode() === $languageCode ?
+				$this->newSuccessResponse( $response ) :
+				$this->newLanguageFallbackResponse( $propertyId, $response->getLabel()->getLanguageCode() );
 		} catch ( UseCaseError $e ) {
 			return $this->responseFactory->newErrorResponseFromException( $e );
 		}
@@ -113,4 +116,21 @@ public function checkPreconditions(): ?ResponseInterface {
 		return null;
 	}
 
+	private function newLanguageFallbackResponse( string $propertyId, string $fallbackLanguageCode ): Response {
+		$httpResponse = $this->getResponseFactory()->create();
+		$httpResponse->setHeader(
+			'Location',
+			$this->getRouter()->getRouteUrl(
+				GetPropertyLabelRouteHandler::ROUTE,
+				[
+					self::PROPERTY_ID_PATH_PARAM => $propertyId,
+					self::LANGUAGE_CODE_PATH_PARAM => $fallbackLanguageCode,
+				]
+			)
+		);
+		$httpResponse->setStatus( 307 );
+
+		return $httpResponse;
+	}
+
 }
diff --git a/repo/rest-api/src/RouteHandlers/openapi.json b/repo/rest-api/src/RouteHandlers/openapi.json
@@ -2593,6 +2593,9 @@
           "304": {
             "$ref": "#/components/responses/NotModified"
           },
+          "307": {
+            "$ref": "#/components/responses/MovedTemporarily"
+          },
           "400": {
             "$ref": "#/components/responses/InvalidTermByLanguageInput"
           },
diff --git a/repo/rest-api/src/WbRestApi.ServiceWiring.php b/repo/rest-api/src/WbRestApi.ServiceWiring.php
@@ -691,7 +691,10 @@ function( MediaWikiServices $services ): ItemSerializationRequestValidatingDeser
 		return new GetPropertyLabelWithFallback(
 			WbRestApi::getValidatingRequestDeserializer( $services ),
 			WbRestApi::getGetLatestPropertyRevisionMetadata( $services ),
-			WbRestApi::getTermLookupEntityTermsRetriever( $services )
+			new FallbackLookupFactoryTermsRetriever(
+				$services->getLanguageFactory(),
+				WikibaseRepo::getFallbackLabelDescriptionLookupFactory( $services )
+			)
 		);
 	},
 
diff --git a/repo/rest-api/tests/mocha/api-testing/GetPropertyLabelWithFallbackTest.js b/repo/rest-api/tests/mocha/api-testing/GetPropertyLabelWithFallbackTest.js
@@ -9,10 +9,11 @@ const { assertValidError } = require( '../helpers/responseValidator' );
 describe( newGetPropertyLabelWithFallbackRequestBuilder().getRouteDescription(), () => {
 	let propertyId;
 	const propertyEnLabel = `en-label-${utils.uniq()}`;
+	const fallbackLanguageWithExistingLabel = 'en';
 
 	before( async () => {
 		const testProperty = await createEntity( 'property', {
-			labels: [ { language: 'en', value: propertyEnLabel } ],
+			labels: [ { language: fallbackLanguageWithExistingLabel, value: propertyEnLabel } ],
 			datatype: 'string'
 		} );
 		propertyId = testProperty.entity.id;
@@ -41,16 +42,35 @@ describe( newGetPropertyLabelWithFallbackRequestBuilder().getRouteDescription(),
 		assert.strictEqual( response.body.message, 'The requested resource does not exist' );
 	} );
 
-	it( 'responds 404 if the label does not exist', async () => {
-		const languageCodeWithNoDefinedLabel = 'ko';
-		const response = await newGetPropertyLabelWithFallbackRequestBuilder( propertyId, languageCodeWithNoDefinedLabel )
-			.assertValidRequest()
-			.makeRequest();
+	it( 'responds 404 if the label does not exist in the requested or any fallback languages', async () => {
+		const propertyWithoutFallback = await createEntity( 'property', {
+			labels: { de: { language: 'de', value: `de-label-${utils.uniq()}` } },
+			datatype: 'string'
+		} );
+
+		const response = await newGetPropertyLabelWithFallbackRequestBuilder(
+			propertyWithoutFallback.entity.id,
+			'ko'
+		).assertValidRequest().makeRequest();
 
 		assertValidError( response, 404, 'resource-not-found', { resource_type: 'label' } );
 		assert.strictEqual( response.body.message, 'The requested resource does not exist' );
 	} );
 
+	it( '307 - language fallback redirect', async () => {
+		const languageCodeWithFallback = 'en-ca';
+
+		const response = await newGetPropertyLabelWithFallbackRequestBuilder( propertyId, languageCodeWithFallback )
+			.assertValidRequest()
+			.makeRequest();
+
+		expect( response ).to.have.status( 307 );
+
+		assert.isTrue( new URL( response.headers.location ).pathname.endsWith(
+			`rest.php/wikibase/v0/entities/properties/${propertyId}/labels/${fallbackLanguageWithExistingLabel}`
+		) );
+	} );
+
 	it( '400 - invalid property ID', async () => {
 		const response = await newGetPropertyLabelWithFallbackRequestBuilder( 'X123', 'en' )
 			.assertInvalidRequest()
diff --git a/repo/rest-api/tests/mocha/openapi-validation/GetPropertyLabelWithFallbackTest.js b/repo/rest-api/tests/mocha/openapi-validation/GetPropertyLabelWithFallbackTest.js
@@ -38,28 +38,41 @@ describe( newGetPropertyLabelWithFallbackRequestBuilder().getRouteDescription(),
 		expect( response ).to.satisfyApiSpec;
 	} );
 
-	it( '400 Bad Request response is valid for an invalid property ID', async () => {
-		const response = await newGetPropertyLabelWithFallbackRequestBuilder( 'X123', languageCode )
-			.makeRequest();
-
-		expect( response ).to.have.status( 400 );
+	it( '307 Temporary Redirect response is valid for a label with language fallback', async () => {
+		const languageCodeWithFallback = 'en-ca';
+		const response = await newGetPropertyLabelWithFallbackRequestBuilder(
+			propertyId,
+			languageCodeWithFallback
+		).makeRequest();
+
+		expect( response ).to.have.status( 307 );
 		expect( response ).to.satisfyApiSpec;
 	} );
 
-	it( '404 Not Found response is valid for a non-existing property', async () => {
-		const response = await newGetPropertyLabelWithFallbackRequestBuilder( 'P99999', languageCode )
+	it( '400 Bad Request response is valid for an invalid property ID', async () => {
+		const response = await newGetPropertyLabelWithFallbackRequestBuilder( 'X123', languageCode )
 			.makeRequest();
 
-		expect( response ).to.have.status( 404 );
+		expect( response ).to.have.status( 400 );
 		expect( response ).to.satisfyApiSpec;
 	} );
 
-	it( '404 Not Found response is valid if there is no label in the requested language', async () => {
-		const response = await newGetPropertyLabelWithFallbackRequestBuilder( propertyId, 'ko' )
-			.makeRequest();
-
-		expect( response ).to.have.status( 404 );
-		expect( response ).to.satisfyApiSpec;
-	} );
+	it(
+		'404 Not Found response is valid if there is no label in the requested or any fallback language',
+		async () => {
+			const propertyWithoutFallback = await createEntity( 'property', {
+				labels: { de: { language: 'de', value: `de-label-${utils.uniq()}` } },
+				datatype: 'string'
+			} );
+
+			const response = await newGetPropertyLabelWithFallbackRequestBuilder(
+				propertyWithoutFallback.entity.id,
+				'ko'
+			).makeRequest();
+
+			expect( response ).to.have.status( 404 );
+			expect( response ).to.satisfyApiSpec;
+		}
+	);
 
 } );
diff --git a/repo/rest-api/tests/phpunit/Application/UseCases/GetPropertyLabelWithFallback/GetPropertyLabelWithFallbackTest.php b/repo/rest-api/tests/phpunit/Application/UseCases/GetPropertyLabelWithFallback/GetPropertyLabelWithFallbackTest.php
@@ -10,7 +10,7 @@
 use Wikibase\Repo\RestApi\Application\UseCases\GetPropertyLabelWithFallback\GetPropertyLabelWithFallbackResponse;
 use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
 use Wikibase\Repo\RestApi\Domain\ReadModel\Label;
-use Wikibase\Repo\RestApi\Domain\Services\PropertyLabelRetriever;
+use Wikibase\Repo\RestApi\Domain\Services\PropertyLabelWithFallbackRetriever;
 use Wikibase\Repo\Tests\RestApi\Application\UseCaseRequestValidation\TestValidatingRequestDeserializer;
 
 /**
@@ -22,14 +22,14 @@
  */
 class GetPropertyLabelWithFallbackTest extends TestCase {
 
-	private PropertyLabelRetriever $labelRetriever;
+	private PropertyLabelWithFallbackRetriever $labelRetriever;
 	private GetLatestPropertyRevisionMetadata $getRevisionMetadata;
 
 	protected function setUp(): void {
 		parent::setUp();
 
 		$this->getRevisionMetadata = $this->createStub( GetLatestPropertyRevisionMetadata::class );
-		$this->labelRetriever = $this->createStub( PropertyLabelRetriever::class );
+		$this->labelRetriever = $this->createStub( PropertyLabelWithFallbackRetriever::class );
 	}
 
 	public function testSuccess(): void {
@@ -38,7 +38,7 @@ public function testSuccess(): void {
 		$lastModified = '20230922070707';
 		$revisionId = 432;
 
-		$this->labelRetriever = $this->createMock( PropertyLabelRetriever::class );
+		$this->labelRetriever = $this->createMock( PropertyLabelWithFallbackRetriever::class );
 		$this->labelRetriever->expects( $this->once() )
 			->method( 'getLabel' )
 			->with( $propertyId, 'en' )
@@ -57,6 +57,31 @@ public function testSuccess(): void {
 		);
 	}
 
+	public function testSuccessWithFallback(): void {
+		$label = new Label( 'en', 'instance of' );
+		$propertyId = new NumericPropertyId( 'P31' );
+		$lastModified = '20230922070707';
+		$revisionId = 432;
+
+		$this->labelRetriever = $this->createMock( PropertyLabelWithFallbackRetriever::class );
+		$this->labelRetriever->expects( $this->once() )
+			->method( 'getLabel' )
+			->with( $propertyId, 'en-ca' )
+			->willReturn( $label );
+
+		$this->getRevisionMetadata = $this->createStub( GetLatestPropertyRevisionMetadata::class );
+		$this->getRevisionMetadata->method( 'execute' )->willReturn( [ $revisionId, $lastModified ] );
+
+		$response = $this->newUseCase()->execute(
+			new GetPropertyLabelWithFallbackRequest( "$propertyId", 'en-ca' )
+		);
+
+		$this->assertEquals(
+			new GetPropertyLabelWithFallbackResponse( $label, $lastModified, $revisionId ),
+			$response
+		);
+	}
+
 	public function testGivenPropertyDoesNotExist_throws(): void {
 		$expectedException = $this->createStub( UseCaseError::class );
 		$this->getRevisionMetadata = $this->createStub( GetLatestPropertyRevisionMetadata::class );
diff --git a/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/FallbackLookupFactoryTermsRetrieverTest.php b/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/FallbackLookupFactoryTermsRetrieverTest.php
@@ -2,10 +2,12 @@
 
 namespace Wikibase\Repo\Tests\RestApi\Infrastructure\DataAccess;
 
+use Generator;
 use MediaWiki\MediaWikiServices;
 use PHPUnit\Framework\TestCase;
 use Wikibase\DataModel\Entity\EntityId;
 use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Entity\NumericPropertyId;
 use Wikibase\DataModel\Services\Lookup\LabelDescriptionLookupException;
 use Wikibase\DataModel\Term\TermFallback;
 use Wikibase\Lib\Store\FallbackLabelDescriptionLookup;
@@ -22,8 +24,10 @@
  */
 class FallbackLookupFactoryTermsRetrieverTest extends TestCase {
 
-	public function testLabelsLookupThrowsLookupException_returnsNull(): void {
-		$entityId = $this->createStub( EntityId::class );
+	/**
+	 * @dataProvider entityIdGenerator
+	 */
+	public function testLabelsLookupThrowsLookupException_returnsNull( EntityId $entityId ): void {
 		$languageCode = 'en';
 
 		$lookup = $this->createStub( FallbackLabelDescriptionLookup::class );
@@ -33,8 +37,10 @@ public function testLabelsLookupThrowsLookupException_returnsNull(): void {
 		$this->assertNull( $this->newTermsRetriever( $lookup )->getLabel( $entityId, $languageCode ) );
 	}
 
-	public function testGetLabel(): void {
-		$entityId = $this->createStub( EntityId::class );
+	/**
+	 * @dataProvider entityIdGenerator
+	 */
+	public function testGetLabel( EntityId $entityId ): void {
 		$languageCode = 'en';
 		$labelText = 'some label';
 
@@ -70,13 +76,21 @@ public function testGetLabelWithFallbackLanguage(): void {
 		);
 	}
 
-	public function testGivenNoLabelInRequestedLanguage_getLabelReturnsNull(): void {
+	/**
+	 * @dataProvider entityIdGenerator
+	 */
+	public function testGivenNoLabelInRequestedLanguage_getLabelReturnsNull( EntityId $entityId ): void {
 		$this->assertNull(
 			( $this->newTermsRetriever( $this->createStub( FallbackLabelDescriptionLookup::class ) ) )
-				->getLabel( new ItemId( 'Q321' ), 'ko' )
+				->getLabel( $entityId, 'ko' )
 		);
 	}
 
+	public function entityIdGenerator(): Generator {
+		yield 'item_id' => [ new ItemId( 'Q321' ) ];
+		yield 'property_id' => [ new NumericPropertyId( 'P123' ) ];
+	}
+
 	private function newTermsRetriever( FallbackLabelDescriptionLookup $lookup ): FallbackLookupFactoryTermsRetriever {
 		$languageFactory = MediaWikiServices::getInstance()->getLanguageFactory();
 
