Skip to content

Commit 68c48fd

Browse files
jenkins-botGerrit Code Review
jenkins-bot
authored and
Gerrit Code Review
committed
Merge "REST: Modify GetItem to use exceptions"
2 parents 370af68 + 6e83ac3 commit 68c48fd

File tree

9 files changed

+116
-180
lines changed

9 files changed

+116
-180
lines changed

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

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
namespace Wikibase\Repo\RestApi\RouteHandlers;
44

5-
use LogicException;
65
use MediaWiki\Rest\Handler;
76
use MediaWiki\Rest\RequestInterface;
87
use MediaWiki\Rest\Response;
@@ -16,10 +15,10 @@
1615
use Wikibase\Repo\RestApi\RouteHandlers\Middleware\UserAgentCheckMiddleware;
1716
use Wikibase\Repo\RestApi\Serialization\ItemDataSerializer;
1817
use Wikibase\Repo\RestApi\UseCases\GetItem\GetItem;
19-
use Wikibase\Repo\RestApi\UseCases\GetItem\GetItemErrorResponse;
2018
use Wikibase\Repo\RestApi\UseCases\GetItem\GetItemRequest;
21-
use Wikibase\Repo\RestApi\UseCases\GetItem\GetItemSuccessResponse;
22-
use Wikibase\Repo\RestApi\UseCases\ItemRedirectResponse;
19+
use Wikibase\Repo\RestApi\UseCases\GetItem\GetItemResponse;
20+
use Wikibase\Repo\RestApi\UseCases\ItemRedirectException;
21+
use Wikibase\Repo\RestApi\UseCases\UseCaseException;
2322
use Wikibase\Repo\RestApi\WbRestApi;
2423
use Wikimedia\ParamValidator\ParamValidator;
2524

@@ -78,20 +77,19 @@ public function run( ...$args ): Response {
7877

7978
public function runUseCase( string $id ): Response {
8079
$fields = explode( ',', $this->getValidatedParams()[self::FIELDS_QUERY_PARAM] );
81-
$useCaseResponse = $this->getItem->execute( new GetItemRequest( $id, $fields ) );
82-
83-
if ( $useCaseResponse instanceof GetItemSuccessResponse ) {
84-
return $this->newSuccessHttpResponse( $useCaseResponse );
85-
} elseif ( $useCaseResponse instanceof ItemRedirectResponse ) {
86-
return $this->newRedirectHttpResponse( $useCaseResponse );
87-
} elseif ( $useCaseResponse instanceof GetItemErrorResponse ) {
88-
return $this->responseFactory->newErrorResponse( $useCaseResponse );
89-
} else {
90-
throw new LogicException( 'Received an unexpected use case result in ' . __CLASS__ );
80+
81+
try {
82+
return $this->newSuccessHttpResponse(
83+
$this->getItem->execute( new GetItemRequest( $id, $fields ) )
84+
);
85+
} catch ( ItemRedirectException $e ) {
86+
return $this->newRedirectHttpResponse( $e );
87+
} catch ( UseCaseException $e ) {
88+
return $this->responseFactory->newErrorResponseFromException( $e );
9189
}
9290
}
9391

94-
private function newSuccessHttpResponse( GetItemSuccessResponse $useCaseResponse ): Response {
92+
private function newSuccessHttpResponse( GetItemResponse $useCaseResponse ): Response {
9593
$httpResponse = $this->getResponseFactory()->create();
9694
$httpResponse->setHeader( 'Content-Type', 'application/json' );
9795
$httpResponse->setHeader( 'Last-Modified', wfTimestamp( TS_RFC2822, $useCaseResponse->getLastModified() ) );
@@ -103,12 +101,12 @@ private function newSuccessHttpResponse( GetItemSuccessResponse $useCaseResponse
103101
return $httpResponse;
104102
}
105103

106-
private function newRedirectHttpResponse( ItemRedirectResponse $useCaseResponse ): Response {
104+
private function newRedirectHttpResponse( ItemRedirectException $e ): Response {
107105
$httpResponse = $this->getResponseFactory()->create();
108106
$httpResponse->setHeader(
109107
'Location',
110108
$this->getRouteUrl(
111-
[ self::ITEM_ID_PATH_PARAM => $useCaseResponse->getRedirectTargetId() ],
109+
[ self::ITEM_ID_PATH_PARAM => $e->getRedirectTargetId() ],
112110
$this->getRequest()->getQueryParams()
113111
)
114112
);

‎repo/rest-api/src/UseCases/GetItem/GetItem.php‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
use Wikibase\Repo\RestApi\Domain\Services\ItemDataRetriever;
77
use Wikibase\Repo\RestApi\Domain\Services\ItemRevisionMetadataRetriever;
88
use Wikibase\Repo\RestApi\UseCases\ErrorResponse;
9-
use Wikibase\Repo\RestApi\UseCases\ItemRedirectResponse;
9+
use Wikibase\Repo\RestApi\UseCases\ItemRedirectException;
10+
use Wikibase\Repo\RestApi\UseCases\UseCaseException;
1011

1112
/**
1213
* @license GPL-2.0-or-later
@@ -28,28 +29,29 @@ public function __construct(
2829
}
2930

3031
/**
31-
* @return GetItemSuccessResponse|GetItemErrorResponse|ItemRedirectResponse
32+
* @throws UseCaseException
33+
* @throws ItemRedirectException
3234
*/
33-
public function execute( GetItemRequest $itemRequest ) {
34-
$validationError = $this->validator->validate( $itemRequest );
35-
36-
if ( $validationError ) {
37-
return GetItemErrorResponse::newFromValidationError( $validationError );
38-
}
35+
public function execute( GetItemRequest $itemRequest ): GetItemResponse {
36+
$this->validator->assertValidRequest( $itemRequest );
3937

4038
$itemId = new ItemId( $itemRequest->getItemId() );
4139
$latestRevisionMetadata = $this->revisionMetadataRetriever->getLatestRevisionMetadata( $itemId );
4240

4341
if ( !$latestRevisionMetadata->itemExists() ) {
44-
return new GetItemErrorResponse(
42+
throw new UseCaseException(
4543
ErrorResponse::ITEM_NOT_FOUND,
4644
"Could not find an item with the ID: {$itemRequest->getItemId()}"
4745
);
48-
} elseif ( $latestRevisionMetadata->isRedirect() ) {
49-
return new ItemRedirectResponse( $latestRevisionMetadata->getRedirectTarget()->getSerialization() );
5046
}
5147

52-
return new GetItemSuccessResponse(
48+
if ( $latestRevisionMetadata->isRedirect() ) {
49+
throw new ItemRedirectException(
50+
$latestRevisionMetadata->getRedirectTarget()->getSerialization()
51+
);
52+
}
53+
54+
return new GetItemResponse(
5355
$this->itemDataRetriever->getItemData( $itemId, $itemRequest->getFields() ),
5456
$latestRevisionMetadata->getRevisionTimestamp(),
5557
$latestRevisionMetadata->getRevisionId()

‎repo/rest-api/src/UseCases/GetItem/GetItemErrorResponse.php‎

Lines changed: 0 additions & 34 deletions
This file was deleted.

‎repo/rest-api/src/UseCases/GetItem/GetItemSuccessResponse.php‎ renamed to ‎repo/rest-api/src/UseCases/GetItem/GetItemResponse.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
/**
88
* @license GPL-2.0-or-later
99
*/
10-
class GetItemSuccessResponse {
10+
class GetItemResponse {
1111

1212
private ItemData $itemData;
1313

‎repo/rest-api/src/UseCases/GetItem/GetItemValidator.php‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,34 +3,50 @@
33
namespace Wikibase\Repo\RestApi\UseCases\GetItem;
44

55
use Wikibase\Repo\RestApi\Domain\ReadModel\ItemData;
6+
use Wikibase\Repo\RestApi\UseCases\ErrorResponse;
7+
use Wikibase\Repo\RestApi\UseCases\UseCaseException;
68
use Wikibase\Repo\RestApi\Validation\ItemIdValidator;
7-
use Wikibase\Repo\RestApi\Validation\ValidationError;
89

910
/**
1011
* @license GPL-2.0-or-later
1112
*/
1213
class GetItemValidator {
13-
public const CODE_INVALID_FIELD = 'invalid-field';
1414

15-
public const CONTEXT_FIELD_VALUE = 'field-value';
15+
public const CODE_INVALID_FIELD = 'invalid-field';
1616

1717
private ItemIdValidator $itemIdValidator;
1818

1919
public function __construct( ItemIdValidator $itemIdValidator ) {
2020
$this->itemIdValidator = $itemIdValidator;
2121
}
2222

23-
public function validate( GetItemRequest $request ): ?ValidationError {
24-
return $this->itemIdValidator->validate( $request->getItemId() )
25-
?: $this->validateFields( $request->getFields() );
23+
/**
24+
* @throws UseCaseException
25+
*/
26+
public function assertValidRequest( GetItemRequest $request ): void {
27+
$validationError = $this->itemIdValidator->validate( $request->getItemId() );
28+
29+
if ( $validationError ) {
30+
throw new UseCaseException(
31+
ErrorResponse::INVALID_ITEM_ID,
32+
'Not a valid item ID: ' . $validationError->getContext()[ItemIdValidator::CONTEXT_VALUE]
33+
);
34+
}
35+
36+
$this->validateFields( $request->getFields() );
2637
}
2738

28-
private function validateFields( array $fields ): ?ValidationError {
39+
/**
40+
* @throws UseCaseException
41+
*/
42+
private function validateFields( array $fields ): void {
2943
foreach ( $fields as $field ) {
3044
if ( !in_array( $field, ItemData::VALID_FIELDS ) ) {
31-
return new ValidationError( self::CODE_INVALID_FIELD, [ self::CONTEXT_FIELD_VALUE => $field ] );
45+
throw new UseCaseException(
46+
ErrorResponse::INVALID_FIELD,
47+
'Not a valid field: ' . $field
48+
);
3249
}
3350
}
34-
return null;
3551
}
3652
}

‎repo/rest-api/tests/phpunit/Presentation/Presenters/ErrorJsonPresenterTest.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
use PHPUnit\Framework\TestCase;
66
use Wikibase\Repo\RestApi\Presentation\Presenters\ErrorJsonPresenter;
77
use Wikibase\Repo\RestApi\UseCases\ErrorResponse;
8-
use Wikibase\Repo\RestApi\UseCases\GetItem\GetItemErrorResponse;
98

109
/**
1110
* @covers \Wikibase\Repo\RestApi\Presentation\Presenters\ErrorJsonPresenter
@@ -17,7 +16,7 @@
1716
class ErrorJsonPresenterTest extends TestCase {
1817

1918
public function testGetJson_withoutContext(): void {
20-
$error = new GetItemErrorResponse( ErrorResponse::ITEM_NOT_FOUND, 'Could not find an item with the ID Q123' );
19+
$error = new ErrorResponse( ErrorResponse::ITEM_NOT_FOUND, 'Could not find an item with the ID Q123' );
2120

2221
$presenter = new ErrorJsonPresenter();
2322

‎repo/rest-api/tests/phpunit/UseCases/GetItem/GetItemErrorResponseTest.php‎

Lines changed: 0 additions & 61 deletions
This file was deleted.

0 commit comments

Comments
 (0)