Skip to content

Commit

Permalink
REST: Add property id filter to ValidatingRequestDeserializer
Browse files Browse the repository at this point in the history
This also modifies the GetItemStatements use case and related classes
to use the new validation.

Bug: T345715
Change-Id: I6b9098cc12c17b4e92ceb04fbc59181081ba9718
  • Loading branch information
Silvan-WMDE committed Sep 8, 2023
1 parent 3133cb6 commit fee17c0
Show file tree
Hide file tree
Showing 17 changed files with 251 additions and 133 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php declare( strict_types=1 );

namespace Wikibase\Repo\RestApi\Application\UseCases;

use Wikibase\DataModel\Entity\PropertyId;

/**
* @license GPL-2.0-or-later
*/
interface DeserializedPropertyIdFilterRequest {
public function getPropertyIdFilter(): ?PropertyId;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php declare( strict_types = 1 );

namespace Wikibase\Repo\RestApi\Application\UseCases\GetItemStatements;

use Wikibase\Repo\RestApi\Application\UseCases\DeserializedItemIdRequest;
use Wikibase\Repo\RestApi\Application\UseCases\DeserializedPropertyIdFilterRequest;

/**
* @license GPL-2.0-or-later
*/
interface DeserializedGetItemStatementsRequest extends DeserializedItemIdRequest, DeserializedPropertyIdFilterRequest {
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Wikibase\Repo\RestApi\Application\UseCases\GetItemStatements;

use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\NumericPropertyId;
use Wikibase\Repo\RestApi\Application\UseCases\GetLatestItemRevisionMetadata;
use Wikibase\Repo\RestApi\Application\UseCases\ItemRedirect;
use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
Expand Down Expand Up @@ -33,19 +31,14 @@ public function __construct(
* @throws UseCaseError
*/
public function execute( GetItemStatementsRequest $request ): GetItemStatementsResponse {
$this->validator->assertValidRequest( $request );

$itemId = new ItemId( $request->getItemId() );
$requestedFilterPropertyId = $request->getStatementPropertyId();
$filterPropertyId = $requestedFilterPropertyId
? new NumericPropertyId( $requestedFilterPropertyId )
: null;
$deserializedRequest = $this->validator->validateAndDeserialize( $request );
$itemId = $deserializedRequest->getItemId();

[ $revisionId, $lastModified ] = $this->getLatestRevisionMetadata->execute( $itemId );

return new GetItemStatementsResponse(
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable Item validated and exists
$this->statementsRetriever->getStatements( $itemId, $filterPropertyId ),
$this->statementsRetriever->getStatements( $itemId, $deserializedRequest->getPropertyIdFilter() ),
$lastModified,
$revisionId,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,29 @@

namespace Wikibase\Repo\RestApi\Application\UseCases\GetItemStatements;

use Wikibase\Repo\RestApi\Application\UseCases\ItemIdRequest;
use Wikibase\Repo\RestApi\Application\UseCases\PropertyIdFilterRequest;
use Wikibase\Repo\RestApi\Application\UseCases\UseCaseRequest;

/**
* @license GPL-2.0-or-later
*/
class GetItemStatementsRequest {
class GetItemStatementsRequest implements UseCaseRequest, ItemIdRequest, PropertyIdFilterRequest {

private string $itemId;
private ?string $statementPropertyId;
private ?string $propertyIdFilter;

public function __construct( string $itemId, ?string $statementPropertyId = null ) {
public function __construct( string $itemId, ?string $propertyIdFilter = null ) {
$this->itemId = $itemId;
$this->statementPropertyId = $statementPropertyId;
$this->propertyIdFilter = $propertyIdFilter;
}

public function getItemId(): string {
return $this->itemId;
}

public function getStatementPropertyId(): ?string {
return $this->statementPropertyId;
public function getPropertyIdFilter(): ?string {
return $this->propertyIdFilter;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,58 +2,28 @@

namespace Wikibase\Repo\RestApi\Application\UseCases\GetItemStatements;

use Wikibase\Repo\RestApi\Application\UseCases\RequestValidation\DeserializedRequestAdapter;
use Wikibase\Repo\RestApi\Application\UseCases\RequestValidation\ValidatingRequestDeserializer;
use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
use Wikibase\Repo\RestApi\Application\Validation\ItemIdValidator;
use Wikibase\Repo\RestApi\Application\Validation\PropertyIdValidator;

/**
* @license GPL-2.0-or-later
*/
class GetItemStatementsValidator {

private ItemIdValidator $itemIdValidator;
private PropertyIdValidator $propertyIdValidator;
private ValidatingRequestDeserializer $requestDeserializer;

public function __construct(
ItemIdValidator $itemIdValidator,
PropertyIdValidator $propertyIdValidator
) {
$this->itemIdValidator = $itemIdValidator;
$this->propertyIdValidator = $propertyIdValidator;
public function __construct( ValidatingRequestDeserializer $requestDeserializer ) {
$this->requestDeserializer = $requestDeserializer;
}

/**
* @throws UseCaseError
*/
public function assertValidRequest( GetItemStatementsRequest $request ): void {
$validationError = $this->itemIdValidator->validate( $request->getItemId() );
if ( $validationError ) {
throw new UseCaseError(
UseCaseError::INVALID_ITEM_ID,
"Not a valid item ID: {$validationError->getContext()[ItemIdValidator::CONTEXT_VALUE]}"
);
}

$this->validatePropertyId( $request->getStatementPropertyId() );
}

/**
* @throws UseCaseError
*/
private function validatePropertyId( ?string $propertyId ): void {
if ( $propertyId === null ) {
return;
}

$validationError = $this->propertyIdValidator->validate( $propertyId );
if ( $validationError ) {
$context = $validationError->getContext();
throw new UseCaseError(
UseCaseError::INVALID_PROPERTY_ID,
"Not a valid property ID: {$context[PropertyIdValidator::CONTEXT_VALUE]}",
[ UseCaseError::CONTEXT_PROPERTY_ID => $context[PropertyIdValidator::CONTEXT_VALUE] ]
);
}
public function validateAndDeserialize( GetItemStatementsRequest $request ): DeserializedGetItemStatementsRequest {
return new class( $this->requestDeserializer->validateAndDeserialize( $request ) )
extends DeserializedRequestAdapter implements DeserializedGetItemStatementsRequest {
};
}

}
10 changes: 10 additions & 0 deletions repo/rest-api/src/Application/UseCases/PropertyIdFilterRequest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php declare( strict_types=1 );

namespace Wikibase\Repo\RestApi\Application\UseCases;

/**
* @license GPL-2.0-or-later
*/
interface PropertyIdFilterRequest {
public function getPropertyIdFilter(): ?string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,22 @@
use LogicException;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\NumericPropertyId;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Statement\StatementGuid;
use Wikibase\Repo\RestApi\Application\UseCases\DeserializedItemIdRequest;
use Wikibase\Repo\RestApi\Application\UseCases\DeserializedPropertyIdFilterRequest;
use Wikibase\Repo\RestApi\Application\UseCases\DeserializedPropertyIdRequest;
use Wikibase\Repo\RestApi\Application\UseCases\DeserializedStatementIdRequest;

/**
* @license GPL-2.0-or-later
*/
class DeserializedRequestAdapter implements DeserializedItemIdRequest, DeserializedPropertyIdRequest, DeserializedStatementIdRequest {
class DeserializedRequestAdapter implements
DeserializedItemIdRequest,
DeserializedPropertyIdRequest,
DeserializedStatementIdRequest,
DeserializedPropertyIdFilterRequest
{
private array $deserializedRequest;

public function __construct( array $deserializedRequest ) {
Expand All @@ -32,11 +39,15 @@ public function getStatementId(): StatementGuid {
return $this->getRequestField( StatementIdRequestValidatingDeserializer::DESERIALIZED_VALUE );
}

public function getPropertyIdFilter(): ?PropertyId {
return $this->getRequestField( PropertyIdFilterRequestValidatingDeserializer::DESERIALIZED_VALUE );
}

/**
* @return mixed
*/
private function getRequestField( string $field ) {
if ( !isset( $this->deserializedRequest[$field] ) ) {
if ( !array_key_exists( $field, $this->deserializedRequest ) ) {
throw new LogicException( "'$field' is not part of the request" );
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php declare( strict_types=1 );

namespace Wikibase\Repo\RestApi\Application\UseCases\RequestValidation;

use Wikibase\DataModel\Entity\NumericPropertyId;
use Wikibase\Repo\RestApi\Application\UseCases\PropertyIdFilterRequest;
use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
use Wikibase\Repo\RestApi\Application\Validation\PropertyIdValidator;

/**
* @license GPL-2.0-or-later
*/
class PropertyIdFilterRequestValidatingDeserializer {
public const DESERIALIZED_VALUE = 'property-id-filter';
private PropertyIdValidator $propertyIdValidator;

public function __construct( PropertyIdValidator $validator ) {
$this->propertyIdValidator = $validator;
}

/**
* @throws UseCaseError
*/
public function validateAndDeserialize( PropertyIdFilterRequest $request ): array {
$filterPropertyId = $request->getPropertyIdFilter();
if ( $filterPropertyId === null ) {
return [ self::DESERIALIZED_VALUE => null ];
}

$validationError = $this->propertyIdValidator->validate( $filterPropertyId );
if ( $validationError ) {
$context = $validationError->getContext();
throw new UseCaseError(
UseCaseError::INVALID_PROPERTY_ID,
"Not a valid property ID: {$context[PropertyIdValidator::CONTEXT_VALUE]}",
[ UseCaseError::CONTEXT_PROPERTY_ID => $context[PropertyIdValidator::CONTEXT_VALUE] ]
);
}
return [ self::DESERIALIZED_VALUE => new NumericPropertyId( $filterPropertyId ) ];
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Wikibase\Repo\RestApi\Application\UseCases\RequestValidation;

use Wikibase\Repo\RestApi\Application\UseCases\ItemIdRequest;
use Wikibase\Repo\RestApi\Application\UseCases\PropertyIdFilterRequest;
use Wikibase\Repo\RestApi\Application\UseCases\PropertyIdRequest;
use Wikibase\Repo\RestApi\Application\UseCases\StatementIdRequest;
use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
Expand All @@ -27,6 +28,7 @@ public function validateAndDeserialize( UseCaseRequest $request ): array {
ItemIdRequest::class => [ $this->factory, 'newItemIdRequestValidatingDeserializer' ],
PropertyIdRequest::class => [ $this->factory, 'newPropertyIdRequestValidatingDeserializer' ],
StatementIdRequest::class => [ $this->factory, 'newStatementIdRequestValidatingDeserializer' ],
PropertyIdFilterRequest::class => [ $this->factory, 'newPropertyIdFilterRequestValidatingDeserializer' ],
];
$result = [];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,8 @@ public function newStatementIdRequestValidatingDeserializer(): StatementIdReques
);
}

public function newPropertyIdFilterRequestValidatingDeserializer(): PropertyIdFilterRequestValidatingDeserializer {
return new PropertyIdFilterRequestValidatingDeserializer( new PropertyIdValidator() );
}

}
2 changes: 1 addition & 1 deletion repo/rest-api/src/WbRestApi.ServiceWiring.php
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@

'WbRestApi.GetItemStatements' => function( MediaWikiServices $services ): GetItemStatements {
return new GetItemStatements(
new GetItemStatementsValidator( new ItemIdValidator(), new PropertyIdValidator() ),
new GetItemStatementsValidator( new ValidatingRequestDeserializer( new ValidatingRequestFieldDeserializerFactory() ) ),
WbRestApi::getItemDataRetriever( $services ),
WbRestApi::getGetLatestItemRevisionMetadata( $services )
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
use Wikibase\Repo\RestApi\Application\UseCases\GetItemStatements\GetItemStatementsRequest;
use Wikibase\Repo\RestApi\Application\UseCases\GetItemStatements\GetItemStatementsValidator;
use Wikibase\Repo\RestApi\Application\UseCases\GetLatestItemRevisionMetadata;
use Wikibase\Repo\RestApi\Application\UseCases\RequestValidation\ValidatingRequestDeserializer;
use Wikibase\Repo\RestApi\Application\UseCases\RequestValidation\ValidatingRequestFieldDeserializerFactory;
use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
use Wikibase\Repo\RestApi\Application\UseCases\UseCaseException;
use Wikibase\Repo\RestApi\Application\Validation\ItemIdValidator;
use Wikibase\Repo\RestApi\Application\Validation\PropertyIdValidator;
use Wikibase\Repo\RestApi\Domain\ReadModel\StatementList;
use Wikibase\Repo\RestApi\Domain\Services\ItemStatementsRetriever;
use Wikibase\Repo\Tests\RestApi\Domain\ReadModel\NewStatementReadModel;
Expand All @@ -26,12 +26,16 @@
*/
class GetItemStatementsTest extends TestCase {

private GetItemStatementsValidator $requestValidator;
private GetLatestItemRevisionMetadata $getRevisionMetadata;
private ItemStatementsRetriever $statementsRetriever;

protected function setUp(): void {
parent::setUp();

$this->requestValidator = new GetItemStatementsValidator(
new ValidatingRequestDeserializer( new ValidatingRequestFieldDeserializerFactory() )
);
$this->getRevisionMetadata = $this->createStub( GetLatestItemRevisionMetadata::class );
$this->statementsRetriever = $this->createStub( ItemStatementsRetriever::class );
}
Expand Down Expand Up @@ -89,14 +93,22 @@ public function testGivenFilterPropertyId_retrievesOnlyRequestedStatements(): vo
$this->assertSame( $expectedStatements, $response->getStatements() );
}

public function testGivenInvalidItemId_throwsException(): void {
public function testGivenInvalidRequest_throwsException(): void {
$request = $this->createStub( GetItemStatementsRequest::class );
$expectedError = $this->createStub( UseCaseError::class );

$this->requestValidator = $this->createMock( GetItemStatementsValidator::class );
$this->requestValidator->expects( $this->once() )
->method( 'validateAndDeserialize' )
->with( $request )
->willThrowException( $expectedError );

try {
$this->newUseCase()->execute( new GetItemStatementsRequest( 'X321' ) );
$this->newUseCase()->execute( $request );

$this->fail( 'this should not be reached' );
} catch ( UseCaseError $e ) {
$this->assertSame( UseCaseError::INVALID_ITEM_ID, $e->getErrorCode() );
$this->assertSame( 'Not a valid item ID: X321', $e->getErrorMessage() );
$this->assertSame( $expectedError, $e );
}
}

Expand All @@ -118,7 +130,7 @@ public function testGivenItemNotFoundOrRedirect_throws(): void {

private function newUseCase(): GetItemStatements {
return new GetItemStatements(
new GetItemStatementsValidator( new ItemIdValidator(), new PropertyIdValidator() ),
$this->requestValidator,
$this->statementsRetriever,
$this->getRevisionMetadata
);
Expand Down
Loading

0 comments on commit fee17c0

Please sign in to comment.