Skip to content

Commit

Permalink
REST: Validate sitelinks in PatchItem use case
Browse files Browse the repository at this point in the history 
Bug: T365029

Change-Id: I5313459f196985f244f4aa35bb401f2efdcf3ed7
  • Loading branch information
dima koushha committed Jun 19, 2024
1 parent e0505e2 commit f7634d3
Show file tree
Hide file tree
Showing 11 changed files with 665 additions and 44 deletions.
9 changes: 9 additions & 0 deletions repo/rest-api/specs/global/examples.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,15 @@
}
}
},
"PatchedInvalidSitelinkTypeExample": {
"value": {
"code": "patched-invalid-sitelink-type'",
"message": "Not a valid sitelink type in patched sitelinks",
"context": {
"site-id": "{site_id}"
}
}
},
"PatchedSitelinkSiteIdInvalidExample": {
"value": {
"code": "patched-sitelink-invalid-site-id",
Expand Down
1 change: 1 addition & 0 deletions repo/rest-api/specs/global/responses.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1346,6 +1346,7 @@
"patched-alias-too-long": { "$ref": "./examples.json#/PatchedAliasTooLongExample" },
"patched-aliases-invalid-field": { "$ref": "./examples.json#/PatchedAliasesInvalidFieldExample" },
"patched-duplicate-alias": { "$ref": "./examples.json#/PatchedDuplicateAliasExample" },
"patched-invalid-sitelink-type": { "$ref": "./examples.json#/PatchedInvalidSitelinkTypeExample"},
"patched-sitelink-invalid-site-id": { "$ref": "./examples.json#/PatchedSitelinkSiteIdInvalidExample" },
"patched-sitelink-missing-title": { "$ref": "./examples.json#/PatchedSitelinkTitleMissingExample" },
"patched-sitelink-title-empty": { "$ref": "./examples.json#/PatchedSitelinkTitleEmptyExample" },
Expand Down
12 changes: 5 additions & 7 deletions repo/rest-api/src/Application/UseCases/PatchItem/PatchItem.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,19 +62,17 @@ public function execute( PatchItemRequest $request ): PatchItemResponse {
$this->assertItemExists->execute( $itemId );
$this->assertUserIsAuthorized->checkEditPermissions( $itemId, $providedMetadata->getUser() );

$item = $this->itemRetriever->getItem( $itemId );

$patchedItemSerialization = $this->patchJson->execute(
ConvertArrayObjectsToArray::execute(
$this->itemSerializer->serialize(
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable
$this->itemRetriever->getItem( $itemId )
)
),
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable
ConvertArrayObjectsToArray::execute( $this->itemSerializer->serialize( $item ) ),
$deserializedRequest->getPatch()
);

$originalItem = $this->itemWriteModelRetriever->getItemWriteModel( $itemId );
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable
$patchedItem = $this->patchedItemValidator->validateAndDeserialize( $patchedItemSerialization, $originalItem );
$patchedItem = $this->patchedItemValidator->validateAndDeserialize( $item, $patchedItemSerialization, $originalItem );

$itemRevision = $this->itemUpdater->update(
$patchedItem,
Expand Down
144 changes: 130 additions & 14 deletions repo/rest-api/src/Application/UseCases/PatchItem/PatchedItemValidator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@
use LogicException;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\SiteLinkList;
use Wikibase\DataModel\Statement\Statement;
use Wikibase\DataModel\Statement\StatementList;
use Wikibase\DataModel\Term\Fingerprint;
use Wikibase\DataModel\Term\Term;
use Wikibase\DataModel\Term\TermList;
use Wikibase\Repo\RestApi\Application\Serialization\SitelinkDeserializer;
use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
use Wikibase\Repo\RestApi\Application\Validation\AliasesInLanguageValidator;
use Wikibase\Repo\RestApi\Application\Validation\AliasesValidator;
Expand All @@ -22,8 +20,13 @@
use Wikibase\Repo\RestApi\Application\Validation\ItemLabelValidator;
use Wikibase\Repo\RestApi\Application\Validation\LabelsSyntaxValidator;
use Wikibase\Repo\RestApi\Application\Validation\LanguageCodeValidator;
use Wikibase\Repo\RestApi\Application\Validation\SiteIdValidator;
use Wikibase\Repo\RestApi\Application\Validation\SitelinksValidator;
use Wikibase\Repo\RestApi\Application\Validation\SitelinkValidator;
use Wikibase\Repo\RestApi\Application\Validation\StatementsValidator;
use Wikibase\Repo\RestApi\Application\Validation\ValidationError;
use Wikibase\Repo\RestApi\Domain\ReadModel\Item as ItemReadModel;
use Wikibase\Repo\RestApi\Domain\ReadModel\Sitelinks;

// disable because it forces comments for switch-cases that look like fall-throughs but aren't
// phpcs:disable PSR2.ControlStructures.SwitchDeclaration.TerminatingComment
Expand All @@ -38,7 +41,7 @@ class PatchedItemValidator {
private DescriptionsSyntaxValidator $descriptionsSyntaxValidator;
private ItemDescriptionsContentsValidator $descriptionsContentsValidator;
private AliasesValidator $aliasesValidator;
private SitelinkDeserializer $sitelinkDeserializer;
private SitelinksValidator $sitelinksValidator;
private StatementsValidator $statementsValidator;

public function __construct(
Expand All @@ -47,31 +50,32 @@ public function __construct(
DescriptionsSyntaxValidator $descriptionsSyntaxValidator,
ItemDescriptionsContentsValidator $descriptionsContentsValidator,
AliasesValidator $aliasesValidator,
SitelinkDeserializer $sitelinkDeserializer,
SitelinksValidator $sitelinksValidator,
StatementsValidator $statementsValidator
) {
$this->labelsSyntaxValidator = $labelsSyntaxValidator;
$this->labelsContentsValidator = $labelsContentsValidator;
$this->descriptionsSyntaxValidator = $descriptionsSyntaxValidator;
$this->descriptionsContentsValidator = $descriptionsContentsValidator;
$this->aliasesValidator = $aliasesValidator;
$this->sitelinkDeserializer = $sitelinkDeserializer;
$this->sitelinksValidator = $sitelinksValidator;
$this->statementsValidator = $statementsValidator;
}

/**
* @throws UseCaseError
*/
public function validateAndDeserialize( array $serialization, Item $originalItem ): Item {
if ( !isset( $serialization['id'] ) ) { // ignore ID removal
$serialization['id'] = $originalItem->getId()->getSerialization();
public function validateAndDeserialize( ItemReadModel $item, array $serialization, Item $originalItem ): Item {
if ( !isset( $serialization[ 'id' ] ) ) { // ignore ID removal
$serialization[ 'id' ] = $originalItem->getId()->getSerialization();
}

$this->assertNoIllegalModification( $serialization, $originalItem );
$this->assertNoUnexpectedFields( $serialization );
$this->assertValidFields( $serialization );
$this->assertValidLabelsAndDescriptions( $serialization, $originalItem );
$this->assertValidAliases( $serialization );
$this->assertValidSitelinks( $item, $serialization );
$this->assertValidStatements( $serialization, $originalItem );

return new Item(
Expand All @@ -81,7 +85,7 @@ public function validateAndDeserialize( array $serialization, Item $originalItem
$this->descriptionsContentsValidator->getValidatedDescriptions(),
$this->aliasesValidator->getValidatedAliases()
),
$this->deserializeSitelinks( $serialization[ 'sitelinks' ] ?? [] ),
$this->sitelinksValidator->getValidatedSitelinks(),
$this->statementsValidator->getValidatedStatements()
);
}
Expand Down Expand Up @@ -369,13 +373,125 @@ private function getModifiedLanguages( TermList $original, TermList $modified ):
) );
}

private function deserializeSitelinks( array $sitelinksSerialization ): SiteLinkList {
$sitelinkList = [];
foreach ( $sitelinksSerialization as $siteId => $sitelink ) {
$sitelinkList[] = $this->sitelinkDeserializer->deserialize( $siteId, $sitelink );
private function assertValidSitelinks( ItemReadModel $item, array $serialization ): void {
$itemId = $serialization['id'];
$sitelinksSerialization = $serialization['sitelinks'] ?? [];
$originalSitelinks = $item->getSitelinks();
$validationError = $this->sitelinksValidator->validate( $itemId, $sitelinksSerialization );

if ( $validationError ) {
$this->handleSitelinksValidationError( $validationError, $sitelinksSerialization );
}
$this->assertUrlsNotModified( $originalSitelinks, $sitelinksSerialization );
}

private function handleSitelinksValidationError( ValidationError $validationError, array $sitelinksSerialization ): void {
$context = $validationError->getContext();
$siteId = fn() => $context[ SitelinkValidator::CONTEXT_SITE_ID ];
switch ( $validationError->getCode() ) {
case SitelinksValidator::CODE_INVALID_SITELINK:
throw new UseCaseError(
UseCaseError::PATCHED_INVALID_SITELINK_TYPE,
'Not a valid sitelink type in patched sitelinks',
[ UseCaseError::CONTEXT_SITE_ID => $context[ SitelinksValidator::CONTEXT_SITE_ID ] ]
);
case SitelinksValidator::CODE_SITELINKS_NOT_ASSOCIATIVE:
$this->throwInvalidField( 'sitelinks', $sitelinksSerialization );
case SiteIdValidator::CODE_INVALID_SITE_ID:
throw new UseCaseError(
UseCaseError::PATCHED_SITELINK_INVALID_SITE_ID,
"Not a valid site ID '{$context[SiteIdValidator::CONTEXT_SITE_ID_VALUE]}' in patched sitelinks",
[ UseCaseError::CONTEXT_SITE_ID => $context[ SiteIdValidator::CONTEXT_SITE_ID_VALUE ] ]
);
case SitelinkValidator::CODE_TITLE_MISSING:
throw new UseCaseError(
UseCaseError::PATCHED_SITELINK_MISSING_TITLE,
"No sitelink title provided for site '{$siteId()}' in patched sitelinks",
[ UseCaseError::CONTEXT_SITE_ID => $siteId() ]
);
case SitelinkValidator::CODE_EMPTY_TITLE:
throw new UseCaseError(
UseCaseError::PATCHED_SITELINK_TITLE_EMPTY,
"Sitelink cannot be empty for site '{$siteId()}' in patched sitelinks",
[ UseCaseError::CONTEXT_SITE_ID => $siteId() ]
);
case SitelinkValidator::CODE_INVALID_TITLE:
case SitelinkValidator::CODE_INVALID_TITLE_TYPE:
$title = $sitelinksSerialization[ $siteId() ][ 'title' ];
throw new UseCaseError(
UseCaseError::PATCHED_SITELINK_INVALID_TITLE,
"Invalid sitelink title '$title' for site '{$siteId()}' in patched sitelinks",
[
UseCaseError::CONTEXT_SITE_ID => $siteId(),
UseCaseError::CONTEXT_TITLE => $title,
]
);
case SitelinkValidator::CODE_TITLE_NOT_FOUND:
$title = $sitelinksSerialization[ $siteId() ][ 'title' ];
throw new UseCaseError(
UseCaseError::PATCHED_SITELINK_TITLE_DOES_NOT_EXIST,
"Incorrect patched sitelinks. Page with title '$title' does not exist on site '{$siteId()}'",
[
UseCaseError::CONTEXT_SITE_ID => $siteId(),
UseCaseError::CONTEXT_TITLE => $title,
]
);
case SitelinkValidator::CODE_INVALID_BADGES_TYPE:
throw new UseCaseError(
UseCaseError::PATCHED_SITELINK_BADGES_FORMAT,
"Badges value for site '{$siteId()}' is not a list in patched sitelinks",
[
UseCaseError::CONTEXT_SITE_ID => $siteId(),
UseCaseError::CONTEXT_BADGES => $sitelinksSerialization[ $siteId() ][ 'badges' ],
]
);
case SitelinkValidator::CODE_INVALID_BADGE:
$badge = $context[ SitelinkValidator::CONTEXT_BADGE ];
throw new UseCaseError(
UseCaseError::PATCHED_SITELINK_INVALID_BADGE,
"Incorrect patched sitelinks. Badge value '$badge' for site '{$siteId()}' is not an item ID",
[
UseCaseError::CONTEXT_SITE_ID => $siteId(),
UseCaseError::CONTEXT_BADGE => $badge,
]
);
case SitelinkValidator::CODE_BADGE_NOT_ALLOWED:
$badge = (string)$context[ SitelinkValidator::CONTEXT_BADGE ];
throw new UseCaseError(
UseCaseError::PATCHED_SITELINK_ITEM_NOT_A_BADGE,
"Incorrect patched sitelinks. Item '$badge' used for site '{$siteId()}' is not allowed as a badge",
[
UseCaseError::CONTEXT_SITE_ID => $siteId(),
UseCaseError::CONTEXT_BADGE => $badge,
]
);
case SitelinkValidator::CODE_SITELINK_CONFLICT:
$matchingItemId = $context[ SitelinkValidator::CONTEXT_CONFLICT_ITEM_ID ];
throw new UseCaseError(
UseCaseError::PATCHED_SITELINK_CONFLICT,
"Site '{$siteId()}' is already being used on '$matchingItemId'",
[
UseCaseError::CONTEXT_MATCHING_ITEM_ID => "$matchingItemId",
UseCaseError::CONTEXT_SITE_ID => $siteId(),
]
);
}
}

return new SiteLinkList( $sitelinkList );
private function assertUrlsNotModified( Sitelinks $originalSitelinks, array $patchedSitelinkSerialization ): void {
foreach ( $patchedSitelinkSerialization as $siteId => $sitelink ) {
if (
isset( $sitelink[ 'url' ] ) &&
isset( $originalSitelinks[ $siteId ] ) &&
$originalSitelinks[ $siteId ]->getUrl() !== $sitelink[ 'url' ]
) {
throw new UseCaseError(
UseCaseError::PATCHED_SITELINK_URL_NOT_MODIFIABLE,
'URL of sitelink cannot be modified',
[ UseCaseError::CONTEXT_SITE_ID => $siteId ]
);
}
}
}

private function assertValidStatements( array $serialization, Item $originalItem ): void {
Expand Down
2 changes: 2 additions & 0 deletions repo/rest-api/src/Application/UseCases/UseCaseError.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class UseCaseError extends UseCaseException {
public const PATCHED_SITELINK_CONFLICT = 'patched-sitelink-conflict';
public const PATCHED_SITELINK_URL_NOT_MODIFIABLE = 'url-not-modifiable';
public const PATCHED_SITELINK_BADGES_FORMAT = 'patched-sitelink-badges-format';
public const PATCHED_INVALID_SITELINK_TYPE = 'patched-invalid-sitelink-type';
public const PATCH_TARGET_NOT_FOUND = 'patch-target-not-found';
public const PATCH_TEST_FAILED = 'patch-test-failed';
public const PERMISSION_DENIED = 'permission-denied';
Expand Down Expand Up @@ -217,6 +218,7 @@ class UseCaseError extends UseCaseException {
self::PATCHED_SITELINK_INVALID_BADGE => [ self::CONTEXT_SITE_ID, self::CONTEXT_BADGE ],
self::PATCHED_SITELINK_ITEM_NOT_A_BADGE => [ self::CONTEXT_SITE_ID, self::CONTEXT_BADGE ],
self::PATCHED_SITELINK_BADGES_FORMAT => [ self::CONTEXT_SITE_ID, self::CONTEXT_BADGES ],
self::PATCHED_INVALID_SITELINK_TYPE => [ self::CONTEXT_SITE_ID ],
self::PATCHED_SITELINK_CONFLICT => [ self::CONTEXT_MATCHING_ITEM_ID, self::CONTEXT_SITE_ID ],
self::PATCHED_SITELINK_URL_NOT_MODIFIABLE => [ self::CONTEXT_SITE_ID ],
self::PATCH_TARGET_NOT_FOUND => [ self::CONTEXT_OPERATION, self::CONTEXT_FIELD ],
Expand Down
1 change: 1 addition & 0 deletions repo/rest-api/src/RouteHandlers/ErrorResponseToHttpStatus.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class ErrorResponseToHttpStatus {
UseCaseError::PATCHED_STATEMENT_GROUP_PROPERTY_ID_MISMATCH => 422,
UseCaseError::STATEMENT_ID_NOT_MODIFIABLE => 422,
UseCaseError::PATCHED_STATEMENT_PROPERTY_NOT_MODIFIABLE => 422,
UseCaseError::PATCHED_INVALID_SITELINK_TYPE => 422,
];

public static function lookup( string $errorCode ): int {
Expand Down
12 changes: 12 additions & 0 deletions repo/rest-api/src/RouteHandlers/openapi.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5659,6 +5659,9 @@
"patched-duplicate-alias": {
"$ref": "#/components/examples/PatchedDuplicateAliasExample"
},
"patched-invalid-sitelink-type": {
"$ref": "#/components/examples/PatchedInvalidSitelinkTypeExample"
},
"patched-sitelink-invalid-site-id": {
"$ref": "#/components/examples/PatchedSitelinkSiteIdInvalidExample"
},
Expand Down Expand Up @@ -7912,6 +7915,15 @@
}
}
},
"PatchedInvalidSitelinkTypeExample": {
"value": {
"code": "patched-invalid-sitelink-type'",
"message": "Not a valid sitelink type in patched sitelinks",
"context": {
"site-id": "{site_id}"
}
}
},
"PatchedSitelinkSiteIdInvalidExample": {
"value": {
"code": "patched-sitelink-invalid-site-id",
Expand Down
10 changes: 9 additions & 1 deletion repo/rest-api/src/WbRestApi.ServiceWiring.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,15 @@ function( MediaWikiServices $services ): ItemSerializationRequestValidatingDeser
WbRestApi::getAliasLanguageCodeValidator( $services ),
new AliasesDeserializer()
),
WbRestApi::getSitelinkDeserializer(),
new SitelinksValidator(
new SiteIdValidator( WikibaseRepo::getSiteLinkGlobalIdentifiersProvider( $services )->getList(
WikibaseRepo::getSettings( $services )->getSetting( 'siteLinkGroups' )
) ),
new SiteLinkLookupSitelinkValidator(
WbRestApi::getSitelinkDeserializer( $services ),
WikibaseRepo::getStore( $services )->newSiteLinkStore()
),
),
new StatementsValidator( new StatementsDeserializer( WbRestApi::getStatementDeserializer() ) )
),
WbRestApi::getItemDataRetriever(),
Expand Down
Loading

0 comments on commit f7634d3

Please sign in to comment.