Skip to content

Commit

Permalink
Merge "REST: Add $basePath param to AliasesValidator"
Browse files Browse the repository at this point in the history
  • Loading branch information
jenkins-bot authored and Gerrit Code Review committed Sep 4, 2024
2 parents 4be9678 + 53d55c6 commit 50a1d24
Show file tree
Hide file tree
Showing 7 changed files with 159 additions and 103 deletions.
2 changes: 1 addition & 1 deletion ...c/Application/UseCaseRequestValidation/ItemSerializationRequestValidatingDeserializer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ private function handleAliasesValidationErrors( ValidationError $validationError
$context = $validationError->getContext();
switch ( $validationError->getCode() ) {
case AliasesValidator::CODE_INVALID_VALUE:
throw UseCaseError::newInvalidValue( "/item/aliases{$context[AliasesValidator::CONTEXT_PATH]}" );
throw UseCaseError::newInvalidValue( $context[AliasesValidator::CONTEXT_PATH] );
case AliasesValidator::CODE_INVALID_ALIASES:
throw UseCaseError::newInvalidValue( '/item/aliases' );
case AliasesInLanguageValidator::CODE_TOO_LONG:
Expand Down
4 changes: 2 additions & 2 deletions repo/rest-api/src/Application/UseCases/PatchItem/PatchedItemValidator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,14 +231,14 @@ private function handleDescriptionsValidationError( ValidationError $validationE

private function assertValidAliases( array $serialization ): void {
$aliasesSerialization = $serialization[ 'aliases' ] ?? [];
$validationError = $this->aliasesValidator->validate( $aliasesSerialization );
$validationError = $this->aliasesValidator->validate( $aliasesSerialization, '/aliases' );
if ( $validationError ) {
$errorCode = $validationError->getCode();
$context = $validationError->getContext();
switch ( $errorCode ) {
case AliasesValidator::CODE_INVALID_VALUE:
throw UseCaseError::newPatchResultInvalidValue(
"/aliases{$context[AliasesValidator::CONTEXT_PATH]}",
$context[AliasesValidator::CONTEXT_PATH],
$context[AliasesValidator::CONTEXT_VALUE]
);
case AliasesValidator::CODE_INVALID_ALIASES:
Expand Down
4 changes: 2 additions & 2 deletions repo/rest-api/src/Application/UseCases/PatchProperty/PatchedPropertyValidator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ private function handleDescriptionsValidationError( ValidationError $validationE
}

private function assertValidAliases( array $aliasesSerialization ): void {
$validationError = $this->aliasesValidator->validate( $aliasesSerialization );
$validationError = $this->aliasesValidator->validate( $aliasesSerialization, '/aliases' );
if ( $validationError ) {
$errorCode = $validationError->getCode();
$context = $validationError->getContext();
Expand All @@ -244,7 +244,7 @@ private function assertValidAliases( array $aliasesSerialization ): void {
throw UseCaseError::newPatchResultInvalidKey( '/aliases', $context[LanguageCodeValidator::CONTEXT_LANGUAGE_CODE] );
case AliasesValidator::CODE_INVALID_VALUE:
throw UseCaseError::newPatchResultInvalidValue(
"/aliases{$context[AliasesValidator::CONTEXT_PATH]}",
$context[AliasesValidator::CONTEXT_PATH],
$context[AliasesValidator::CONTEXT_VALUE]
);
case AliasesValidator::CODE_INVALID_ALIASES:
Expand Down
10 changes: 5 additions & 5 deletions repo/rest-api/src/Application/Validation/AliasesValidator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function __construct(
$this->languageCodeValidator = $languageCodeValidator;
}

public function validate( array $aliases ): ?ValidationError {
public function validate( array $aliases, string $basePath ): ?ValidationError {
if ( count( $aliases ) === 0 ) {
$this->deserializedAliases = new AliasGroupList();
return null;
Expand Down Expand Up @@ -66,12 +66,12 @@ public function validate( array $aliases ): ?ValidationError {
}
}

return $this->deserializeAliases( $aliases ) ?? $this->validateAliasesInLanguage( $this->deserializedAliases );
return $this->deserializeAliases( $aliases, $basePath ) ?? $this->validateAliases( $this->deserializedAliases );
}

private function deserializeAliases( array $aliases ): ?ValidationError {
private function deserializeAliases( array $aliases, string $basePath ): ?ValidationError {
try {
$this->deserializedAliases = $this->aliasesDeserializer->deserialize( $aliases, '' );
$this->deserializedAliases = $this->aliasesDeserializer->deserialize( $aliases, $basePath );
} catch ( InvalidFieldException $e ) {
return new ValidationError(
self::CODE_INVALID_VALUE,
Expand All @@ -82,7 +82,7 @@ private function deserializeAliases( array $aliases ): ?ValidationError {
return null;
}

private function validateAliasesInLanguage( AliasGroupList $aliases ): ?ValidationError {
private function validateAliases( AliasGroupList $aliases ): ?ValidationError {
foreach ( $aliases as $aliasesInLanguage ) {
$aliasesInLanguageValidationError = $this->aliasesInLanguageValidator->validate( $aliasesInLanguage );
if ( $aliasesInLanguageValidationError ) {
Expand Down
10 changes: 5 additions & 5 deletions repo/rest-api/src/Application/Validation/ItemValidator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class ItemValidator {
private ItemLabelsContentsValidator $labelsContentsValidator;
private DescriptionsSyntaxValidator $descriptionsSyntaxValidator;
private ItemDescriptionsContentsValidator $descriptionsContentsValidator;
private AliasesValidator $itemAliasesValidator;
private AliasesValidator $aliasesValidator;
private StatementsValidator $itemStatementsValidator;
private SitelinksValidator $sitelinksValidator;

Expand All @@ -30,15 +30,15 @@ public function __construct(
ItemLabelsContentsValidator $labelsContentsValidator,
DescriptionsSyntaxValidator $descriptionsSyntaxValidator,
ItemDescriptionsContentsValidator $descriptionsContentsValidator,
AliasesValidator $itemAliasesValidator,
AliasesValidator $aliasesValidator,
StatementsValidator $itemStatementsValidator,
SitelinksValidator $sitelinksValidator
) {
$this->labelsSyntaxValidator = $labelsSyntaxValidator;
$this->labelsContentsValidator = $labelsContentsValidator;
$this->descriptionsSyntaxValidator = $descriptionsSyntaxValidator;
$this->descriptionsContentsValidator = $descriptionsContentsValidator;
$this->itemAliasesValidator = $itemAliasesValidator;
$this->aliasesValidator = $aliasesValidator;
$this->itemStatementsValidator = $itemStatementsValidator;
$this->sitelinksValidator = $sitelinksValidator;
}
Expand All @@ -59,7 +59,7 @@ public function validate( array $serialization, string $basePath = '' ): ?Valida
}

$validationError = $this->validateLabelsAndDescriptions( $serialization ) ??
$this->itemAliasesValidator->validate( $serialization['aliases'] ) ??
$this->aliasesValidator->validate( $serialization['aliases'], "$basePath/aliases" ) ??
$this->itemStatementsValidator->validate( $serialization['statements'], "$basePath/statements" ) ??
$this->sitelinksValidator->validate( null, $serialization['sitelinks'], null, "$basePath/sitelinks" );
if ( $validationError ) {
Expand All @@ -71,7 +71,7 @@ public function validate( array $serialization, string $basePath = '' ): ?Valida
new Fingerprint(
$this->labelsContentsValidator->getValidatedLabels(),
$this->descriptionsContentsValidator->getValidatedDescriptions(),
$this->itemAliasesValidator->getValidatedAliases()
$this->aliasesValidator->getValidatedAliases()
),
$this->sitelinksValidator->getValidatedSitelinks(),
$this->itemStatementsValidator->getValidatedStatements()
Expand Down
200 changes: 121 additions & 79 deletions ...plication/UseCaseRequestValidation/ItemSerializationRequestValidatingDeserializerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,22 @@
namespace Wikibase\Repo\Tests\RestApi\Application\UseCaseRequestValidation;

use Generator;
use MediaWiki\Languages\LanguageNameUtils;
use PHPUnit\Framework\TestCase;
use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Services\Lookup\TermLookup;
use Wikibase\DataModel\Tests\NewItem;
use Wikibase\Repo\RestApi\Application\Serialization\AliasesDeserializer;
use Wikibase\Repo\RestApi\Application\Serialization\AliasesInLanguageDeserializer;
use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\ItemSerializationRequest;
use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\ItemSerializationRequestValidatingDeserializer;
use Wikibase\Repo\RestApi\Application\UseCases\CreateItem\CreateItemRequest;
use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
use Wikibase\Repo\RestApi\Application\Validation\AliasesInLanguageValidator;
use Wikibase\Repo\RestApi\Application\Validation\AliasesValidator;
use Wikibase\Repo\RestApi\Application\Validation\DescriptionsSyntaxValidator;
use Wikibase\Repo\RestApi\Application\Validation\ItemDescriptionsContentsValidator;
use Wikibase\Repo\RestApi\Application\Validation\ItemDescriptionValidator;
use Wikibase\Repo\RestApi\Application\Validation\ItemLabelsContentsValidator;
use Wikibase\Repo\RestApi\Application\Validation\ItemLabelValidator;
use Wikibase\Repo\RestApi\Application\Validation\ItemValidator;
use Wikibase\Repo\RestApi\Application\Validation\LabelsSyntaxValidator;
Expand All @@ -22,6 +29,11 @@
use Wikibase\Repo\RestApi\Application\Validation\StatementsValidator;
use Wikibase\Repo\RestApi\Application\Validation\StatementValidator;
use Wikibase\Repo\RestApi\Application\Validation\ValidationError;
use Wikibase\Repo\RestApi\Infrastructure\TermValidatorFactoryAliasesInLanguageValidator;
use Wikibase\Repo\RestApi\Infrastructure\ValueValidatorLanguageCodeValidator;
use Wikibase\Repo\Store\TermsCollisionDetectorFactory;
use Wikibase\Repo\Validators\MembershipValidator;
use Wikibase\Repo\Validators\TermValidatorFactory;

/**
* @covers \Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\ItemSerializationRequestValidatingDeserializer
Expand All @@ -47,11 +59,118 @@ public function testGivenValidRequest_returnsItem(): void {
);
}

/**
* In contrast to the solitary testGivenInvalidRequest_throws() method, this is a sociable test.
* It is more thorough at testing the behaviour of the SUT and is expected to be more resilient to refactoring of the SUT.
* I suggest that if other parts of the SUT are touched, we also refactor the related tests to this more sociable style.
*
* @dataProvider provideInvalidAliases
*/
public function testGivenInvalidRequest_throwsUseCaseError( UseCaseError $expectedError, array $serialization ): void {
try {
$this->newRequestValidatingDeserializer()->validateAndDeserialize(
new CreateItemRequest( $serialization, [], false, null, null )
);
$this->fail( 'Expected exception was not thrown' );
} catch ( UseCaseError $e ) {
$this->assertEquals( $expectedError, $e );
}
}

public function provideInvalidAliases(): Generator {
yield 'invalid serialization - string' => [
UseCaseError::newInvalidValue( '/item/aliases' ),
[ 'aliases' => 'not an array' ],
];

yield 'invalid serialization - sequential array' => [
UseCaseError::newInvalidValue( '/item/aliases' ),
[ 'aliases' => [ 'not', 'an', 'associative', 'array' ] ],
];

yield 'invalid language code - int' => [
UseCaseError::newInvalidKey( '/item/aliases', '3912' ),
[ 'aliases' => [ 3912 => [ 'alias 1' ] ] ],
];

yield 'invalid language code - not an allowed language code' => [
UseCaseError::newInvalidKey( '/item/aliases', 'xyz' ),
[ 'aliases' => [ 'xyz' => [ 'alias 1' ] ] ],
];

yield 'invalid aliases in language - string' => [
UseCaseError::newInvalidValue( '/item/aliases/en' ),
[ 'aliases' => [ 'en' => 'not a list' ] ],
];

yield 'invalid aliases in language - associative array' => [
UseCaseError::newInvalidValue( '/item/aliases/en' ),
[ 'aliases' => [ 'en' => [ 'not' => 'a', 'sequential' => 'array' ] ] ],
];

yield 'invalid aliases in language - empty array' => [
UseCaseError::newInvalidValue( '/item/aliases/en' ),
[ 'aliases' => [ 'en' => [] ] ],
];

yield 'invalid alias - integer' => [
UseCaseError::newInvalidValue( '/item/aliases/en/0' ),
[ 'aliases' => [ 'en' => [ 7940, 'alias 2' ] ] ],
];

yield 'invalid alias - zero length string' => [
UseCaseError::newInvalidValue( '/item/aliases/en/1' ),
[ 'aliases' => [ 'en' => [ 'alias 1', '' ] ] ],
];

yield 'invalid alias - whitespace only' => [
UseCaseError::newInvalidValue( '/item/aliases/en/0' ),
[ 'aliases' => [ 'en' => [ " \t ", 'alias 2' ] ] ],
];

yield 'invalid alias - invalid characters' => [
UseCaseError::newInvalidValue( '/item/aliases/en/1' ),
[ 'aliases' => [ 'en' => [ 'alias 1', "alias \t with \t tabs" ] ] ],
];

yield 'invalid alias - too long' => [
UseCaseError::newValueTooLong( '/item/aliases/en/0', self::MAX_LENGTH ),
[ 'aliases' => [ 'en' => [ str_repeat( 'A', self::MAX_LENGTH + 1 ) ] ] ],
];
}

private function newRequestValidatingDeserializer(): ItemSerializationRequestValidatingDeserializer {
$validLanguageCodes = [ 'ar', 'de', 'en', 'fr' ];
return new ItemSerializationRequestValidatingDeserializer(
new ItemValidator(
$this->createStub( LabelsSyntaxValidator::class ),
$this->createStub( ItemLabelsContentsValidator::class ),
$this->createStub( DescriptionsSyntaxValidator::class ),
$this->createStub( ItemDescriptionsContentsValidator::class ),
new AliasesValidator(
new TermValidatorFactoryAliasesInLanguageValidator(
new TermValidatorFactory(
self::MAX_LENGTH,
$validLanguageCodes,
$this->createStub( EntityIdParser::class ),
$this->createStub( TermsCollisionDetectorFactory::class ),
$this->createStub( TermLookup::class ),
$this->createStub( LanguageNameUtils::class )
)
),
new ValueValidatorLanguageCodeValidator( new MembershipValidator( $validLanguageCodes ) ),
new AliasesDeserializer( new AliasesInLanguageDeserializer() )
),
$this->createStub( StatementsValidator::class ),
$this->createStub( SitelinksValidator::class ),
)
);
}

/**
* @dataProvider itemValidationErrorProvider
* @dataProvider itemLabelsValidationErrorProvider
* @dataProvider itemDescriptionsValidationErrorProvider
* @dataProvider itemAliasesValidationErrorProvider
* @dataProvider itemStatementsValidationErrorProvider
* @dataProvider sitelinksValidationErrorProvider
*/
Expand Down Expand Up @@ -304,83 +423,6 @@ public function itemDescriptionsValidationErrorProvider(): Generator {
];
}

public function itemAliasesValidationErrorProvider(): Generator {
yield 'invalid value' => [
new ValidationError(
AliasesValidator::CODE_INVALID_VALUE,
[ AliasesValidator::CONTEXT_PATH => '/en/1', AliasesValidator::CONTEXT_VALUE => '' ]
),
UseCaseError::newInvalidValue( '/item/aliases/en/1' ),
];

yield 'empty aliases in language list' => [
new ValidationError(
AliasesValidator::CODE_INVALID_ALIAS_LIST,
[ AliasesValidator::CONTEXT_LANGUAGE => 'en' ]
),
UseCaseError::newInvalidValue( '/item/aliases/en' ),
];

$invalidAliases = [ 'not a valid aliases array' ];
yield 'invalid aliases' => [
new ValidationError(
AliasesValidator::CODE_INVALID_ALIASES,
[ AliasesValidator::CONTEXT_ALIASES => $invalidAliases ]
),
new UseCaseError(
UseCaseError::INVALID_VALUE,
"Invalid value at '/item/aliases'",
[ UseCaseError::CONTEXT_PATH => '/item/aliases' ]
),
];

yield 'invalid aliases in language list' => [
new ValidationError(
AliasesValidator::CODE_INVALID_ALIAS_LIST,
[ AliasesValidator::CONTEXT_LANGUAGE => 'en' ]
),
UseCaseError::newInvalidValue( '/item/aliases/en' ),
];

$tooLongAlias = str_repeat( 'a', self::MAX_LENGTH + 1 );
yield 'alias too long' => [
new ValidationError(
AliasesInLanguageValidator::CODE_TOO_LONG,
[
AliasesInLanguageValidator::CONTEXT_VALUE => $tooLongAlias,
AliasesInLanguageValidator::CONTEXT_LANGUAGE => 'en',
AliasesInLanguageValidator::CONTEXT_LIMIT => self::MAX_LENGTH,
]
),
UseCaseError::newValueTooLong( '/item/aliases/en/0', self::MAX_LENGTH ),
[ 'aliases' => [ 'en' => [ $tooLongAlias ] ] ],
];

yield 'invalid alias' => [
new ValidationError(
AliasesInLanguageValidator::CODE_INVALID,
[
AliasesInLanguageValidator::CONTEXT_VALUE => "alias \t with \t tabs",
AliasesInLanguageValidator::CONTEXT_LANGUAGE => 'en',
AliasesInLanguageValidator::CONTEXT_PATH => 'en/1',
]
),
UseCaseError::newInvalidValue( '/item/aliases/en/1' ),
[ 'aliases' => [ 'en' => [ 'valid alias', "alias \t with \t tabs" ] ] ],
];

yield 'invalid aliases language code' => [
new ValidationError(
LanguageCodeValidator::CODE_INVALID_LANGUAGE_CODE,
[
LanguageCodeValidator::CONTEXT_FIELD => 'aliases',
LanguageCodeValidator::CONTEXT_LANGUAGE_CODE => 'e2',
]
),
UseCaseError::newInvalidKey( '/item/aliases', 'e2' ),
];
}

public function itemStatementsValidationErrorProvider(): Generator {
$invalidStatements = [ 'not valid statements' ];
yield 'invalid statements array' => [
Expand Down
Loading

0 comments on commit 50a1d24

Please sign in to comment.