Skip to content

Commit fc6afde

Browse files
jenkins-botGerrit Code Review
jenkins-bot
authored and
Gerrit Code Review
committed
Merge "REST: Implement fallback in GetPropertyDescriptionWithFallback"
2 parents 542283e + 6daad64 commit fc6afde

File tree

12 files changed

+171
-25
lines changed

12 files changed

+171
-25
lines changed

‎repo/rest-api/specs/resources/descriptions/description-with-fallback-for-property.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"responses": {
1717
"200": { "$ref": "../../global/responses.json#/Description" },
1818
"304": { "$ref": "../../global/responses.json#/NotModified" },
19+
"307": { "$ref": "../../global/responses.json#/MovedTemporarily" },
1920
"400": { "$ref": "../../global/responses.json#/InvalidTermByLanguageInput" },
2021
"404": { "$ref": "../../global/responses.json#/ResourceNotFound" },
2122
"412": { "$ref": "../../global/responses.json#/PreconditionFailedError" },

‎repo/rest-api/src/Application/UseCases/GetPropertyDescriptionWithFallback/GetPropertyDescriptionWithFallback.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
use Wikibase\Repo\RestApi\Application\UseCases\GetLatestPropertyRevisionMetadata;
66
use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
7-
use Wikibase\Repo\RestApi\Domain\Services\PropertyDescriptionRetriever;
7+
use Wikibase\Repo\RestApi\Domain\Services\PropertyDescriptionWithFallbackRetriever;
88

99
/**
1010
* @license GPL-2.0-or-later
@@ -13,12 +13,12 @@ class GetPropertyDescriptionWithFallback {
1313

1414
private GetPropertyDescriptionWithFallbackValidator $validator;
1515
private GetLatestPropertyRevisionMetadata $getRevisionMetadata;
16-
private PropertyDescriptionRetriever $descriptionRetriever;
16+
private PropertyDescriptionWithFallbackRetriever $descriptionRetriever;
1717

1818
public function __construct(
1919
GetPropertyDescriptionWithFallbackValidator $validator,
2020
GetLatestPropertyRevisionMetadata $getRevisionMetadata,
21-
PropertyDescriptionRetriever $descriptionRetriever
21+
PropertyDescriptionWithFallbackRetriever $descriptionRetriever
2222
) {
2323
$this->validator = $validator;
2424
$this->getRevisionMetadata = $getRevisionMetadata;

‎repo/rest-api/src/Domain/Services/PropertyDescriptionWithFallbackRetriever.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php declare( strict_types=1 );
2+
3+
namespace Wikibase\Repo\RestApi\Domain\Services;
4+
5+
use Wikibase\DataModel\Entity\PropertyId;
6+
use Wikibase\Repo\RestApi\Domain\ReadModel\Description;
7+
8+
/**
9+
* @license GPL-2.0-or-later
10+
*/
11+
interface PropertyDescriptionWithFallbackRetriever {
12+
13+
public function getDescription( PropertyId $propertyId, string $languageCode ): ?Description;
14+
15+
}

‎repo/rest-api/src/Infrastructure/DataAccess/FallbackLookupFactoryTermsRetriever.php‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,23 @@
44

55
use MediaWiki\Languages\LanguageFactory;
66
use Wikibase\DataModel\Entity\EntityId;
7+
use Wikibase\DataModel\Entity\PropertyId;
78
use Wikibase\DataModel\Services\Lookup\LabelDescriptionLookupException;
89
use Wikibase\Lib\Store\FallbackLabelDescriptionLookupFactory;
10+
use Wikibase\Repo\RestApi\Domain\ReadModel\Description;
911
use Wikibase\Repo\RestApi\Domain\ReadModel\Label;
1012
use Wikibase\Repo\RestApi\Domain\Services\ItemLabelWithFallbackRetriever;
13+
use Wikibase\Repo\RestApi\Domain\Services\PropertyDescriptionWithFallbackRetriever;
1114
use Wikibase\Repo\RestApi\Domain\Services\PropertyLabelWithFallbackRetriever;
1215

1316
/**
1417
* @license GPL-2.0-or-later
1518
*/
16-
class FallbackLookupFactoryTermsRetriever implements ItemLabelWithFallbackRetriever, PropertyLabelWithFallbackRetriever {
19+
class FallbackLookupFactoryTermsRetriever implements
20+
ItemLabelWithFallbackRetriever,
21+
PropertyLabelWithFallbackRetriever,
22+
PropertyDescriptionWithFallbackRetriever
23+
{
1724

1825
private FallbackLabelDescriptionLookupFactory $lookupFactory;
1926
private LanguageFactory $languageFactory;
@@ -40,4 +47,20 @@ public function getLabel( EntityId $entityId, string $languageCode ): ?Label {
4047
new Label( $labelFallback->getActualLanguageCode(), $labelFallback->getText() ) :
4148
null;
4249
}
50+
51+
public function getDescription( PropertyId $propertyId, string $languageCode ): ?Description {
52+
try {
53+
$descriptionFallback = $this->lookupFactory
54+
->newLabelDescriptionLookup( $this->languageFactory->getLanguage( $languageCode ) )
55+
->getDescription( $propertyId );
56+
} catch ( LabelDescriptionLookupException $e ) {
57+
// this probably means that the entity does not exist, which should be checked prior to calling this method
58+
return null;
59+
}
60+
61+
return $descriptionFallback !== null ?
62+
new Description( $descriptionFallback->getActualLanguageCode(), $descriptionFallback->getText() ) :
63+
null;
64+
}
65+
4366
}

‎repo/rest-api/src/RouteHandlers/GetPropertyDescriptionRouteHandler.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ class GetPropertyDescriptionRouteHandler extends SimpleHandler {
2525
private const PROPERTY_ID_PATH_PARAM = 'property_id';
2626
private const LANGUAGE_CODE_PATH_PARAM = 'language_code';
2727

28+
public const ROUTE = '/wikibase/v0/entities/properties/{property_id}/descriptions/{language_code}';
29+
2830
private GetPropertyDescription $useCase;
2931
private MiddlewareHandler $middlewareHandler;
3032
private ResponseFactory $responseFactory;

‎repo/rest-api/src/RouteHandlers/GetPropertyDescriptionWithFallbackRouteHandler.php‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,11 @@ public function run( ...$args ): Response {
6767

6868
public function runUseCase( string $propertyId, string $languageCode ): Response {
6969
try {
70-
return $this->newSuccessHttpResponse(
71-
$this->useCase->execute( new GetPropertyDescriptionWithFallbackRequest( $propertyId, $languageCode ) )
72-
);
70+
$response = $this->useCase->execute( new GetPropertyDescriptionWithFallbackRequest( $propertyId, $languageCode ) );
71+
72+
return $response->getDescription()->getLanguageCode() === $languageCode
73+
? $this->newSuccessHttpResponse( $response )
74+
: $this->newLanguageFallbackResponse( $propertyId, $response->getDescription()->getLanguageCode() );
7375
} catch ( UseCaseError $e ) {
7476
return $this->responseFactory->newErrorResponseFromException( $e );
7577
}
@@ -111,4 +113,21 @@ public function checkPreconditions(): ?ResponseInterface {
111113
return null;
112114
}
113115

116+
private function newLanguageFallbackResponse( string $propertyId, string $fallbackLanguageCode ): Response {
117+
$httpResponse = $this->getResponseFactory()->create();
118+
$httpResponse->setHeader(
119+
'Location',
120+
$this->getRouter()->getRouteUrl(
121+
GetPropertyDescriptionRouteHandler::ROUTE,
122+
[
123+
self::PROPERTY_ID_PATH_PARAM => $propertyId,
124+
self::LANGUAGE_CODE_PATH_PARAM => $fallbackLanguageCode,
125+
]
126+
)
127+
);
128+
$httpResponse->setStatus( 307 );
129+
130+
return $httpResponse;
131+
}
132+
114133
}

‎repo/rest-api/src/RouteHandlers/openapi.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,6 +1479,9 @@
14791479
"304": {
14801480
"$ref": "#/components/responses/NotModified"
14811481
},
1482+
"307": {
1483+
"$ref": "#/components/responses/MovedTemporarily"
1484+
},
14821485
"400": {
14831486
"$ref": "#/components/responses/InvalidTermByLanguageInput"
14841487
},

‎repo/rest-api/src/WbRestApi.ServiceWiring.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,10 @@ function( MediaWikiServices $services ): ItemSerializationRequestValidatingDeser
685685
return new GetPropertyDescriptionWithFallback(
686686
WbRestApi::getValidatingRequestDeserializer( $services ),
687687
WbRestApi::getGetLatestPropertyRevisionMetadata( $services ),
688-
WbRestApi::getTermLookupEntityTermsRetriever( $services )
688+
new FallbackLookupFactoryTermsRetriever(
689+
$services->getLanguageFactory(),
690+
WikibaseRepo::getFallbackLabelDescriptionLookupFactory( $services )
691+
)
689692
);
690693
},
691694

‎repo/rest-api/tests/mocha/api-testing/GetPropertyDescriptionwithFallbackTest.js‎ renamed to ‎repo/rest-api/tests/mocha/api-testing/GetPropertyDescriptionWithFallbackTest.js‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,33 +9,46 @@ const { assertValidError } = require( '../helpers/responseValidator' );
99

1010
describe( newGetPropertyDescriptionWithFallbackRequestBuilder().getRouteDescription(), () => {
1111
let propertyId;
12-
const propertyEnDescription = `string-property-description-${utils.uniq()}`;
12+
const fallbackLanguageWithDescription = 'de';
13+
const propertyDeDescription = `string-property-description-${utils.uniq()}`;
1314

1415
before( async () => {
1516
const testProperty = await createEntity( 'property', {
1617
labels: { en: { language: 'en', value: `string-property-${utils.uniq()}` } },
17-
descriptions: {
18-
en: { language: 'en', value: propertyEnDescription }
19-
},
18+
descriptions: [ { language: fallbackLanguageWithDescription, value: propertyDeDescription } ],
2019
datatype: 'string'
2120
} );
2221

2322
propertyId = testProperty.entity.id;
2423
} );
2524

26-
it( 'can get a language specific description of a property', async () => {
25+
it( '200 - can get a language specific description of a property', async () => {
2726
const testPropertyCreationMetadata = await getLatestEditMetadata( propertyId );
28-
const response = await newGetPropertyDescriptionWithFallbackRequestBuilder( propertyId, 'en' )
27+
const response = await newGetPropertyDescriptionWithFallbackRequestBuilder( propertyId, 'de' )
2928
.assertValidRequest().makeRequest();
3029

3130
expect( response ).to.have.status( 200 );
32-
assert.strictEqual( response.body, propertyEnDescription );
31+
assert.strictEqual( response.body, propertyDeDescription );
3332
assert.strictEqual( response.header.etag, `"${testPropertyCreationMetadata.revid}"` );
3433
assert.strictEqual( response.header[ 'last-modified' ], testPropertyCreationMetadata.timestamp );
3534
} );
3635

36+
it( '307 - language fallback redirect', async () => {
37+
const response = await newGetPropertyDescriptionWithFallbackRequestBuilder( propertyId, 'bar' )
38+
.assertValidRequest()
39+
.makeRequest();
40+
41+
expect( response ).to.have.status( 307 );
42+
assert.isTrue(
43+
new URL( response.headers.location ).pathname
44+
.endsWith(
45+
`rest.php/wikibase/v0/entities/properties/${propertyId}/descriptions/${fallbackLanguageWithDescription}`
46+
)
47+
);
48+
} );
49+
3750
it( '400 - invalid property ID', async () => {
38-
const response = await newGetPropertyDescriptionWithFallbackRequestBuilder( 'X123', 'en' )
51+
const response = await newGetPropertyDescriptionWithFallbackRequestBuilder( 'X123', 'de' )
3952
.assertInvalidRequest()
4053
.makeRequest();
4154

@@ -60,17 +73,17 @@ describe( newGetPropertyDescriptionWithFallbackRequestBuilder().getRouteDescript
6073
);
6174
} );
6275

63-
it( 'responds 404 in case the property does not exist', async () => {
76+
it( '404 - in case the property does not exist', async () => {
6477
const nonExistentProperty = 'P99999999';
65-
const response = await newGetPropertyDescriptionWithFallbackRequestBuilder( nonExistentProperty, 'en' )
78+
const response = await newGetPropertyDescriptionWithFallbackRequestBuilder( nonExistentProperty, 'de' )
6679
.assertValidRequest()
6780
.makeRequest();
6881

6982
assertValidError( response, 404, 'resource-not-found', { resource_type: 'property' } );
7083
assert.strictEqual( response.body.message, 'The requested resource does not exist' );
7184
} );
7285

73-
it( 'responds 404 in case the property has no description in the requested language', async () => {
86+
it( '404 - in case the property has no description in the requested language', async () => {
7487
const languageCode = 'ko';
7588
const response = await newGetPropertyDescriptionWithFallbackRequestBuilder( propertyId, languageCode )
7689
.assertValidRequest()

‎repo/rest-api/tests/mocha/openapi-validation/GetPropertyDescriptionWithFallbackTest.js‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ describe( newGetPropertyDescriptionWithFallbackRequestBuilder().getRouteDescript
99

1010
let propertyId;
1111
let lastRevisionId;
12-
const languageCode = 'en';
12+
const languageCode = 'de';
1313

1414
before( async () => {
1515
const createPropertyResponse = await createEntity( 'property', {
16-
descriptions: [ { language: languageCode, value: 'an-English-description-' + utils.uniq() } ],
16+
descriptions: [ { language: languageCode, value: 'a-German-description-' + utils.uniq() } ],
1717
datatype: 'string'
1818
} );
1919

@@ -37,6 +37,17 @@ describe( newGetPropertyDescriptionWithFallbackRequestBuilder().getRouteDescript
3737
expect( response ).to.satisfyApiSpec;
3838
} );
3939

40+
it( '307 Temporary Redirect response is valid for a description with language fallback', async () => {
41+
const languageCodeWithFallback = 'bar';
42+
const response = await newGetPropertyDescriptionWithFallbackRequestBuilder(
43+
propertyId,
44+
languageCodeWithFallback
45+
).makeRequest();
46+
47+
expect( response ).to.have.status( 307 );
48+
expect( response ).to.satisfyApiSpec;
49+
} );
50+
4051
it( '400 Bad Request response is valid for an invalid property ID', async () => {
4152
const response = await newGetPropertyDescriptionWithFallbackRequestBuilder( 'X123', languageCode ).makeRequest();
4253

0 commit comments

Comments
 (0)