From 0a2ca8d984fa4dd2c059f8757f836616972b2e56 Mon Sep 17 00:00:00 2001 From: Silvan Date: Tue, 10 Oct 2023 10:46:50 +0200 Subject: [PATCH] REST: Create SetPropertyLabel happy path * Add new use case with deserialization * Differentiate between 200 OK and 201 CREATED * Add RouteHandler and e2e-test * Does not include any request validation Bug: T342979 Change-Id: I24e0714b07c7a3fab4f9c18b6e7e7e58fb03b398 --- repo/rest-api/routes.dev.json | 5 + .../DeserializedItemLabelEditRequest.php | 2 +- .../DeserializedPropertyLabelEditRequest.php | 12 ++ .../DeserializedRequestAdapter.php | 10 +- .../PropertyLabelEditRequest.php | 10 + ...LabelEditRequestValidatingDeserializer.php | 17 ++ .../UseCases/SetItemLabel/SetItemLabel.php | 2 +- .../DeserializedSetPropertyLabelRequest.php | 12 ++ .../SetPropertyLabel/SetPropertyLabel.php | 56 ++++++ .../SetPropertyLabelRequest.php | 68 +++++++ .../SetPropertyLabelResponse.php | 40 ++++ .../SetPropertyLabelValidator.php | 12 ++ .../ValidatingRequestDeserializer.php | 7 +- .../SetPropertyLabelRouteHandler.php | 123 ++++++++++++ repo/rest-api/src/WbRestApi.ServiceWiring.php | 17 +- repo/rest-api/src/WbRestApi.php | 6 + .../mocha/api-testing/SetPropertyLabelTest.js | 181 ++++++++++++++++++ .../mocha/helpers/RequestBuilderFactory.js | 8 + .../DeserializedRequestAdapterTest.php | 20 +- ...lEditRequestValidatingDeserializerTest.php | 31 +++ .../SetPropertyLabelRequestTest.php | 45 +++++ .../SetPropertyLabel/SetPropertyLabelTest.php | 136 +++++++++++++ .../ValidatingRequestDeserializerTest.php | 20 +- 23 files changed, 829 insertions(+), 11 deletions(-) create mode 100644 repo/rest-api/src/Application/UseCaseRequestValidation/DeserializedPropertyLabelEditRequest.php create mode 100644 repo/rest-api/src/Application/UseCaseRequestValidation/PropertyLabelEditRequest.php create mode 100644 repo/rest-api/src/Application/UseCaseRequestValidation/PropertyLabelEditRequestValidatingDeserializer.php create mode 100644 repo/rest-api/src/Application/UseCases/SetPropertyLabel/DeserializedSetPropertyLabelRequest.php create mode 100644 repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabel.php create mode 100644 repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabelRequest.php create mode 100644 repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabelResponse.php create mode 100644 repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabelValidator.php create mode 100644 repo/rest-api/src/RouteHandlers/SetPropertyLabelRouteHandler.php create mode 100644 repo/rest-api/tests/mocha/api-testing/SetPropertyLabelTest.js create mode 100644 repo/rest-api/tests/phpunit/Application/UseCaseRequestValidation/PropertyLabelEditRequestValidatingDeserializerTest.php create mode 100644 repo/rest-api/tests/phpunit/Application/UseCases/SetPropertyLabel/SetPropertyLabelRequestTest.php create mode 100644 repo/rest-api/tests/phpunit/Application/UseCases/SetPropertyLabel/SetPropertyLabelTest.php diff --git a/repo/rest-api/routes.dev.json b/repo/rest-api/routes.dev.json index 3ffbf2b2c85..0645e59389f 100644 --- a/repo/rest-api/routes.dev.json +++ b/repo/rest-api/routes.dev.json @@ -9,6 +9,11 @@ "method": "PATCH", "factory": "Wikibase\\Repo\\RestApi\\RouteHandlers\\PatchItemAliasesRouteHandler::factory" }, + { + "path": "/wikibase/v0/entities/properties/{property_id}/labels/{language_code}", + "method": "PUT", + "factory": "Wikibase\\Repo\\RestApi\\RouteHandlers\\SetPropertyLabelRouteHandler::factory" + }, { "path": "/wikibase/v0/entities/properties/{property_id}/aliases", "method": "PATCH", diff --git a/repo/rest-api/src/Application/UseCaseRequestValidation/DeserializedItemLabelEditRequest.php b/repo/rest-api/src/Application/UseCaseRequestValidation/DeserializedItemLabelEditRequest.php index 7623b77902c..e03be79bef4 100644 --- a/repo/rest-api/src/Application/UseCaseRequestValidation/DeserializedItemLabelEditRequest.php +++ b/repo/rest-api/src/Application/UseCaseRequestValidation/DeserializedItemLabelEditRequest.php @@ -8,5 +8,5 @@ * @license GPL-2.0-or-later */ interface DeserializedItemLabelEditRequest extends DeserializedItemIdRequest { - public function getLabel(): Term; + public function getItemLabel(): Term; } diff --git a/repo/rest-api/src/Application/UseCaseRequestValidation/DeserializedPropertyLabelEditRequest.php b/repo/rest-api/src/Application/UseCaseRequestValidation/DeserializedPropertyLabelEditRequest.php new file mode 100644 index 00000000000..38dce54b530 --- /dev/null +++ b/repo/rest-api/src/Application/UseCaseRequestValidation/DeserializedPropertyLabelEditRequest.php @@ -0,0 +1,12 @@ +getRequestField( PatchRequest::class ); } - public function getLabel(): Term { + public function getItemLabel(): Term { return $this->getRequestField( ItemLabelEditRequest::class ); } @@ -136,6 +138,10 @@ public function getItemDescription(): Term { return $this->getRequestField( ItemDescriptionEditRequest::class ); } + public function getPropertyLabel(): Term { + return $this->getRequestField( PropertyLabelEditRequest::class ); + } + public function getPropertyDescription(): Term { return $this->getRequestField( PropertyDescriptionEditRequest::class ); } diff --git a/repo/rest-api/src/Application/UseCaseRequestValidation/PropertyLabelEditRequest.php b/repo/rest-api/src/Application/UseCaseRequestValidation/PropertyLabelEditRequest.php new file mode 100644 index 00000000000..71eb9e94357 --- /dev/null +++ b/repo/rest-api/src/Application/UseCaseRequestValidation/PropertyLabelEditRequest.php @@ -0,0 +1,10 @@ +getLanguageCode(), $request->getLabel() ); + } + +} diff --git a/repo/rest-api/src/Application/UseCases/SetItemLabel/SetItemLabel.php b/repo/rest-api/src/Application/UseCases/SetItemLabel/SetItemLabel.php index a0c352b601f..6cefe5e72e2 100644 --- a/repo/rest-api/src/Application/UseCases/SetItemLabel/SetItemLabel.php +++ b/repo/rest-api/src/Application/UseCases/SetItemLabel/SetItemLabel.php @@ -43,7 +43,7 @@ public function __construct( public function execute( SetItemLabelRequest $request ): SetItemLabelResponse { $deserializedRequest = $this->validator->validateAndDeserialize( $request ); $itemId = $deserializedRequest->getItemId(); - $label = $deserializedRequest->getLabel(); + $label = $deserializedRequest->getItemLabel(); $this->assertItemExists->execute( $itemId ); diff --git a/repo/rest-api/src/Application/UseCases/SetPropertyLabel/DeserializedSetPropertyLabelRequest.php b/repo/rest-api/src/Application/UseCases/SetPropertyLabel/DeserializedSetPropertyLabelRequest.php new file mode 100644 index 00000000000..389f8bc19a2 --- /dev/null +++ b/repo/rest-api/src/Application/UseCases/SetPropertyLabel/DeserializedSetPropertyLabelRequest.php @@ -0,0 +1,12 @@ +validator = $validator; + $this->propertyRetriever = $propertyRetriever; + $this->propertyUpdater = $propertyUpdater; + } + + public function execute( SetPropertyLabelRequest $request ): SetPropertyLabelResponse { + $deserializedRequest = $this->validator->validateAndDeserialize( $request ); + $propertyId = $deserializedRequest->getPropertyId(); + $label = $deserializedRequest->getPropertyLabel(); + $editMetadata = $deserializedRequest->getEditMetadata(); + + $property = $this->propertyRetriever->getProperty( $propertyId ); + $labelExists = $property->getLabels()->hasTermForLanguage( $label->getLanguageCode() ); + $property->getLabels()->setTerm( $label ); + + $editSummary = $labelExists + ? LabelEditSummary::newReplaceSummary( $editMetadata->getComment(), $label ) + : LabelEditSummary::newAddSummary( $editMetadata->getComment(), $label ); + + $newRevision = $this->propertyUpdater->update( + $property, // @phan-suppress-current-line PhanTypeMismatchArgumentNullable + new EditMetadata( $editMetadata->getTags(), $editMetadata->isBot(), $editSummary ) + ); + + return new SetPropertyLabelResponse( + $newRevision->getProperty()->getLabels()[$label->getLanguageCode()], + $newRevision->getLastModified(), + $newRevision->getRevisionId(), + $labelExists + ); + } + +} diff --git a/repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabelRequest.php b/repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabelRequest.php new file mode 100644 index 00000000000..d61604d4cb6 --- /dev/null +++ b/repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabelRequest.php @@ -0,0 +1,68 @@ +propertyId = $propertyId; + $this->languageCode = $languageCode; + $this->label = trim( $label ); + $this->editTags = $editTags; + $this->isBot = $isBot; + $this->comment = $comment; + $this->username = $username; + } + + public function getPropertyId(): string { + return $this->propertyId; + } + + public function getLanguageCode(): string { + return $this->languageCode; + } + + public function getLabel(): string { + return $this->label; + } + + public function getEditTags(): array { + return $this->editTags; + } + + public function isBot(): bool { + return $this->isBot; + } + + public function getComment(): ?string { + return $this->comment; + } + + public function getUsername(): ?string { + return $this->username; + } + +} diff --git a/repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabelResponse.php b/repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabelResponse.php new file mode 100644 index 00000000000..d6ffdea9b46 --- /dev/null +++ b/repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabelResponse.php @@ -0,0 +1,40 @@ +label = $label; + $this->lastModified = $lastModified; + $this->revisionId = $revisionId; + $this->replaced = $replaced; + } + + public function getLabel(): Label { + return $this->label; + } + + public function getLastModified(): string { + return $this->lastModified; + } + + public function getRevisionId(): int { + return $this->revisionId; + } + + public function wasReplaced(): bool { + return $this->replaced; + } + +} diff --git a/repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabelValidator.php b/repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabelValidator.php new file mode 100644 index 00000000000..040dcdb29e4 --- /dev/null +++ b/repo/rest-api/src/Application/UseCases/SetPropertyLabel/SetPropertyLabelValidator.php @@ -0,0 +1,12 @@ + self::PATCH_REQUEST_VALIDATING_DESERIALIZER, ItemLabelEditRequest::class => self::ITEM_LABEL_EDIT_REQUEST_VALIDATING_DESERIALIZER, ItemDescriptionEditRequest::class => self::ITEM_DESCRIPTION_EDIT_REQUEST_VALIDATING_DESERIALIZER, + PropertyLabelEditRequest::class => self::PROPERTY_LABEL_EDIT_REQUEST_VALIDATING_DESERIALIZER, PropertyDescriptionEditRequest::class => self::PROPERTY_DESCRIPTION_EDIT_REQUEST_VALIDATING_DESERIALIZER, ]; $result = []; diff --git a/repo/rest-api/src/RouteHandlers/SetPropertyLabelRouteHandler.php b/repo/rest-api/src/RouteHandlers/SetPropertyLabelRouteHandler.php new file mode 100644 index 00000000000..34e9dfabf3b --- /dev/null +++ b/repo/rest-api/src/RouteHandlers/SetPropertyLabelRouteHandler.php @@ -0,0 +1,123 @@ +setPropertyLabel = $setPropertyLabel; + } + + public static function factory(): Handler { + return new self( WbRestApi::getSetPropertyLabel() ); + } + + public function run( string $propertyId, string $languageCode ): Response { + $jsonBody = $this->getValidatedBody(); + $useCaseResponse = $this->setPropertyLabel->execute( + new SetPropertyLabelRequest( + $propertyId, + $languageCode, + $jsonBody[self::LABEL_BODY_PARAM], + $jsonBody[self::TAGS_BODY_PARAM], + $jsonBody[self::BOT_BODY_PARAM], + $jsonBody[self::COMMENT_BODY_PARAM], + $this->getUsername() + ) + ); + return $this->newSuccessHttpResponse( $useCaseResponse ); + } + + /** + * @inheritDoc + */ + public function getParamSettings(): array { + return [ + self::PROPERTY_ID_PATH_PARAM => [ + self::PARAM_SOURCE => 'path', + ParamValidator::PARAM_TYPE => 'string', + ParamValidator::PARAM_REQUIRED => true, + ], + self::LANGUAGE_CODE_PATH_PARAM => [ + self::PARAM_SOURCE => 'path', + ParamValidator::PARAM_TYPE => 'string', + ParamValidator::PARAM_REQUIRED => true, + ], + ]; + } + + /** + * @inheritDoc + */ + public function getBodyValidator( $contentType ): BodyValidator { + return $contentType === 'application/json' ? + new TypeValidatingJsonBodyValidator( [ + self::LABEL_BODY_PARAM => [ + self::PARAM_SOURCE => 'body', + ParamValidator::PARAM_TYPE => 'string', + ParamValidator::PARAM_REQUIRED => true, + ], + self::TAGS_BODY_PARAM => [ + self::PARAM_SOURCE => 'body', + ParamValidator::PARAM_TYPE => 'array', + ParamValidator::PARAM_REQUIRED => false, + ParamValidator::PARAM_DEFAULT => [], + ], + self::BOT_BODY_PARAM => [ + self::PARAM_SOURCE => 'body', + ParamValidator::PARAM_TYPE => 'boolean', + ParamValidator::PARAM_REQUIRED => false, + ParamValidator::PARAM_DEFAULT => false, + ], + self::COMMENT_BODY_PARAM => [ + self::PARAM_SOURCE => 'body', + ParamValidator::PARAM_TYPE => 'string', + ParamValidator::PARAM_REQUIRED => false, + ], + ] ) : parent::getBodyValidator( $contentType ); + } + + private function newSuccessHttpResponse( SetPropertyLabelResponse $useCaseResponse ): Response { + $httpResponse = $this->getResponseFactory()->create(); + $httpResponse->setStatus( $useCaseResponse->wasReplaced() ? 200 : 201 ); + $httpResponse->setHeader( 'Content-Type', 'application/json' ); + $httpResponse->setHeader( 'ETag', "\"{$useCaseResponse->getRevisionId()}\"" ); + $httpResponse->setHeader( + 'Last-Modified', + wfTimestamp( TS_RFC2822, $useCaseResponse->getLastModified() ) + ); + $httpResponse->setBody( new StringStream( json_encode( $useCaseResponse->getLabel()->getText() ) ) ); + + return $httpResponse; + } + + private function getUsername(): ?string { + $mwUser = $this->getAuthority()->getUser(); + return $mwUser->isRegistered() ? $mwUser->getName() : null; + } + +} diff --git a/repo/rest-api/src/WbRestApi.ServiceWiring.php b/repo/rest-api/src/WbRestApi.ServiceWiring.php index b4dd0a36b61..136038b2472 100644 --- a/repo/rest-api/src/WbRestApi.ServiceWiring.php +++ b/repo/rest-api/src/WbRestApi.ServiceWiring.php @@ -31,6 +31,7 @@ use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyIdFilterRequest; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyIdRequest; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyIdValidatingDeserializer; +use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyLabelEditRequestValidatingDeserializer; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\StatementIdRequestValidatingDeserializer; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\StatementSerializationRequestValidatingDeserializer; use Wikibase\Repo\RestApi\Application\UseCases\AddItemStatement\AddItemStatement; @@ -80,6 +81,7 @@ use Wikibase\Repo\RestApi\Application\UseCases\SetItemDescription\SetItemDescription; use Wikibase\Repo\RestApi\Application\UseCases\SetItemLabel\SetItemLabel; use Wikibase\Repo\RestApi\Application\UseCases\SetPropertyDescription\SetPropertyDescription; +use Wikibase\Repo\RestApi\Application\UseCases\SetPropertyLabel\SetPropertyLabel; use Wikibase\Repo\RestApi\Application\Validation\EditMetadataValidator; use Wikibase\Repo\RestApi\Application\Validation\ItemIdValidator; use Wikibase\Repo\RestApi\Application\Validation\LanguageCodeValidator; @@ -224,6 +226,11 @@ function ( MediaWikiServices $services ): ItemDescriptionEditRequestValidatingDe ); }, + VRD::PROPERTY_LABEL_EDIT_REQUEST_VALIDATING_DESERIALIZER => + function (): PropertyLabelEditRequestValidatingDeserializer { + return new PropertyLabelEditRequestValidatingDeserializer(); + }, + VRD::PROPERTY_DESCRIPTION_EDIT_REQUEST_VALIDATING_DESERIALIZER => function ( MediaWikiServices $services ): PropertyDescriptionEditRequestValidatingDeserializer { return new PropertyDescriptionEditRequestValidatingDeserializer( @@ -260,7 +267,7 @@ function ( MediaWikiServices $services ): PropertyDescriptionEditRequestValidati $statementReadModelConverter ), new GuidGenerator(), - new EntityUpdaterPropertyUpdater( WbRestApi::getEntityUpdater(), $statementReadModelConverter ), + WbRestApi::getPropertyUpdater( $services ), new AssertUserIsAuthorized( new WikibaseEntityPermissionChecker( WikibaseRepo::getEntityPermissionChecker( $services ), @@ -714,6 +721,14 @@ function ( MediaWikiServices $services ): PropertyDescriptionEditRequestValidati ); }, + 'WbRestApi.SetPropertyLabel' => function( MediaWikiServices $services ): SetPropertyLabel { + return new SetPropertyLabel( + WbRestApi::getValidatingRequestDeserializer( $services ), + WbRestApi::getPropertyDataRetriever( $services ), + WbRestApi::getPropertyUpdater( $services ), + ); + }, + 'WbRestApi.StatementDeserializer' => function( MediaWikiServices $services ): StatementDeserializer { $entityIdParser = WikibaseRepo::getEntityIdParser( $services ); $propertyValuePairDeserializer = new PropertyValuePairDeserializer( diff --git a/repo/rest-api/src/WbRestApi.php b/repo/rest-api/src/WbRestApi.php index 07c0dac033b..a78f72ce25f 100644 --- a/repo/rest-api/src/WbRestApi.php +++ b/repo/rest-api/src/WbRestApi.php @@ -49,6 +49,7 @@ use Wikibase\Repo\RestApi\Application\UseCases\SetItemDescription\SetItemDescription; use Wikibase\Repo\RestApi\Application\UseCases\SetItemLabel\SetItemLabel; use Wikibase\Repo\RestApi\Application\UseCases\SetPropertyDescription\SetPropertyDescription; +use Wikibase\Repo\RestApi\Application\UseCases\SetPropertyLabel\SetPropertyLabel; use Wikibase\Repo\RestApi\Domain\Services\ItemUpdater; use Wikibase\Repo\RestApi\Domain\Services\PropertyUpdater; use Wikibase\Repo\RestApi\Domain\Services\StatementRemover; @@ -107,6 +108,11 @@ public static function getSetItemLabel( ContainerInterface $services = null ): S ->get( 'WbRestApi.SetItemLabel' ); } + public static function getSetPropertyLabel( ContainerInterface $services = null ): SetPropertyLabel { + return ( $services ?: MediaWikiServices::getInstance() ) + ->get( 'WbRestApi.SetPropertyLabel' ); + } + public static function getSetItemDescription( ContainerInterface $services = null ): SetItemDescription { return ( $services ?: MediaWikiServices::getInstance() ) ->get( 'WbRestApi.SetItemDescription' ); diff --git a/repo/rest-api/tests/mocha/api-testing/SetPropertyLabelTest.js b/repo/rest-api/tests/mocha/api-testing/SetPropertyLabelTest.js new file mode 100644 index 00000000000..7e109305453 --- /dev/null +++ b/repo/rest-api/tests/mocha/api-testing/SetPropertyLabelTest.js @@ -0,0 +1,181 @@ +'use strict'; + +const { assert, action, utils } = require( 'api-testing' ); +const { expect } = require( '../helpers/chaiHelper' ); +const entityHelper = require( '../helpers/entityHelper' ); +const { newSetPropertyLabelRequestBuilder } = require( '../helpers/RequestBuilderFactory' ); +const { formatTermEditSummary } = require( '../helpers/formatEditSummaries' ); +const { makeEtag } = require( '../helpers/httpHelper' ); + +describe( newSetPropertyLabelRequestBuilder().getRouteDescription(), () => { + let testPropertyId; + let originalLastModified; + let originalRevisionId; + + function assertValidResponse( response, labelText ) { + assert.strictEqual( response.header[ 'content-type' ], 'application/json' ); + assert.isAbove( new Date( response.header[ 'last-modified' ] ), originalLastModified ); + assert.notStrictEqual( response.header.etag, makeEtag( originalRevisionId ) ); + assert.strictEqual( response.body, labelText ); + } + + function assertValid200Response( response, labelText ) { + expect( response ).to.have.status( 200 ); + assertValidResponse( response, labelText ); + } + + function assertValid201Response( response, labelText ) { + expect( response ).to.have.status( 201 ); + assertValidResponse( response, labelText ); + } + + before( async () => { + const createEntityResponse = await entityHelper.createEntity( 'property', { + labels: { en: { language: 'en', value: `english label ${utils.uniq()}` } }, + datatype: 'string' + } ); + testPropertyId = createEntityResponse.entity.id; + + const testPropertyCreationMetadata = await entityHelper.getLatestEditMetadata( testPropertyId ); + originalLastModified = new Date( testPropertyCreationMetadata.timestamp ); + originalRevisionId = testPropertyCreationMetadata.revid; + + // wait 1s before modifying labels to verify the last-modified timestamps are different + await new Promise( ( resolve ) => { + setTimeout( resolve, 1000 ); + } ); + } ); + + describe( '20x success response ', () => { + it( 'can add a label with edit metadata omitted', async () => { + const languageCode = 'de'; + const newLabel = `neues deutsches Label ${utils.uniq()}`; + const comment = 'omg look, i added a new label'; + const response = await newSetPropertyLabelRequestBuilder( testPropertyId, languageCode, newLabel ) + .withJsonBodyParam( 'comment', comment ) + .assertValidRequest() + .makeRequest(); + + assertValid201Response( response, newLabel ); + + const editMetadata = await entityHelper.getLatestEditMetadata( testPropertyId ); + assert.strictEqual( + editMetadata.comment, + formatTermEditSummary( + 'wbsetlabel', + 'add', + languageCode, + newLabel, + comment + ) + ); + } ); + + it( 'can add a label with edit metadata provided', async () => { + const languageCode = 'en-us'; + const newLabel = `new us-english label ${utils.uniq()}`; + const user = await action.robby(); // robby is a bot + const tag = await action.makeTag( 'e2e test tag', 'Created during e2e test' ); + const comment = 'omg look, an edit i made'; + const response = await newSetPropertyLabelRequestBuilder( testPropertyId, languageCode, newLabel ) + .withJsonBodyParam( 'tags', [ tag ] ) + .withJsonBodyParam( 'bot', true ) + .withJsonBodyParam( 'comment', comment ) + .withUser( user ) + .assertValidRequest() + .makeRequest(); + + assertValid201Response( response, newLabel ); + + const editMetadata = await entityHelper.getLatestEditMetadata( testPropertyId ); + assert.deepEqual( editMetadata.tags, [ tag ] ); + assert.property( editMetadata, 'bot' ); + assert.strictEqual( + editMetadata.comment, + formatTermEditSummary( + 'wbsetlabel', + 'add', + languageCode, + newLabel, + comment + ) + ); + assert.strictEqual( editMetadata.user, user.username ); + } ); + + it( 'can replace a label with edit metadata omitted', async () => { + const languageCode = 'en'; + const newLabel = `new label ${utils.uniq()}`; + const comment = 'omg look, i replaced a new label'; + const response = await newSetPropertyLabelRequestBuilder( testPropertyId, languageCode, newLabel ) + .withJsonBodyParam( 'comment', comment ) + .assertValidRequest() + .makeRequest(); + + assertValid200Response( response, newLabel ); + + const editMetadata = await entityHelper.getLatestEditMetadata( testPropertyId ); + assert.strictEqual( + editMetadata.comment, + formatTermEditSummary( + 'wbsetlabel', + 'set', + languageCode, + newLabel, + comment + ) + ); + } ); + + it( 'can replace a label with edit metadata provided', async () => { + const languageCode = 'en'; + const newLabel = `new english label ${utils.uniq()}`; + const user = await action.robby(); // robby is a bot + const tag = await action.makeTag( 'e2e test tag', 'Created during e2e test' ); + const comment = 'omg look, an edit i made'; + const response = await newSetPropertyLabelRequestBuilder( testPropertyId, languageCode, newLabel ) + .withJsonBodyParam( 'tags', [ tag ] ) + .withJsonBodyParam( 'bot', true ) + .withJsonBodyParam( 'comment', comment ) + .withUser( user ) + .assertValidRequest() + .makeRequest(); + + assertValid200Response( response, newLabel ); + + const editMetadata = await entityHelper.getLatestEditMetadata( testPropertyId ); + assert.deepEqual( editMetadata.tags, [ tag ] ); + assert.property( editMetadata, 'bot' ); + assert.strictEqual( + editMetadata.comment, + formatTermEditSummary( + 'wbsetlabel', + 'set', + languageCode, + newLabel, + comment + ) + ); + assert.strictEqual( editMetadata.user, user.username ); + } ); + + it( 'idempotency check: can set the same label twice', async () => { + const languageCode = 'en'; + const newLabel = `new English Label ${utils.uniq()}`; + const comment = 'omg look, i can set a new label'; + let response = await newSetPropertyLabelRequestBuilder( testPropertyId, languageCode, newLabel ) + .withJsonBodyParam( 'comment', comment ) + .assertValidRequest() + .makeRequest(); + + assertValid200Response( response, newLabel ); + + response = await newSetPropertyLabelRequestBuilder( testPropertyId, languageCode, newLabel ) + .withJsonBodyParam( 'comment', 'omg look, i can set the same label again' ) + .assertValidRequest() + .makeRequest(); + + assertValid200Response( response, newLabel ); + } ); + } ); +} ); diff --git a/repo/rest-api/tests/mocha/helpers/RequestBuilderFactory.js b/repo/rest-api/tests/mocha/helpers/RequestBuilderFactory.js index 6ca435c0e40..a9806995b6e 100644 --- a/repo/rest-api/tests/mocha/helpers/RequestBuilderFactory.js +++ b/repo/rest-api/tests/mocha/helpers/RequestBuilderFactory.js @@ -129,6 +129,14 @@ module.exports = { .withJsonBodyParam( 'label', label ); }, + newSetPropertyLabelRequestBuilder( propertyId, languageCode, label ) { + return new RequestBuilder() + .withRoute( 'PUT', '/entities/properties/{property_id}/labels/{language_code}' ) + .withPathParam( 'property_id', propertyId ) + .withPathParam( 'language_code', languageCode ) + .withJsonBodyParam( 'label', label ); + }, + newSetItemDescriptionRequestBuilder( itemId, languageCode, description ) { return new RequestBuilder() .withRoute( 'PUT', '/entities/items/{item_id}/descriptions/{language_code}' ) diff --git a/repo/rest-api/tests/phpunit/Application/UseCaseRequestValidation/DeserializedRequestAdapterTest.php b/repo/rest-api/tests/phpunit/Application/UseCaseRequestValidation/DeserializedRequestAdapterTest.php index 26e701b78b9..deaa3b66ff8 100644 --- a/repo/rest-api/tests/phpunit/Application/UseCaseRequestValidation/DeserializedRequestAdapterTest.php +++ b/repo/rest-api/tests/phpunit/Application/UseCaseRequestValidation/DeserializedRequestAdapterTest.php @@ -21,6 +21,7 @@ use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyFieldsRequest; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyIdFilterRequest; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyIdRequest; +use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyLabelEditRequest; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\StatementIdRequest; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\StatementSerializationRequest; use Wikibase\Repo\RestApi\Domain\Model\UserProvidedEditMetadata; @@ -156,15 +157,26 @@ public function testGivenNoPatch_getPatchThrows(): void { ( new DeserializedRequestAdapter( [] ) )->getPatch(); } - public function testGetLabel(): void { + public function testGetItemLabel(): void { $label = new Term( 'en', 'potato' ); $requestAdapter = new DeserializedRequestAdapter( [ ItemLabelEditRequest::class => $label ] ); - $this->assertSame( $label, $requestAdapter->getLabel() ); + $this->assertSame( $label, $requestAdapter->getItemLabel() ); } - public function testGivenNoLabel_getLabelThrows(): void { + public function testGivenNoLabel_getItemLabelThrows(): void { $this->expectException( LogicException::class ); - ( new DeserializedRequestAdapter( [] ) )->getLabel(); + ( new DeserializedRequestAdapter( [] ) )->getItemLabel(); + } + + public function testGetPropertyLabel(): void { + $label = new Term( 'en', 'potato' ); + $requestAdapter = new DeserializedRequestAdapter( [ PropertyLabelEditRequest::class => $label ] ); + $this->assertSame( $label, $requestAdapter->getPropertyLabel() ); + } + + public function testGivenNoLabel_getPropertyLabelThrows(): void { + $this->expectException( LogicException::class ); + ( new DeserializedRequestAdapter( [] ) )->getPropertyLabel(); } public function testGetItemDescription(): void { diff --git a/repo/rest-api/tests/phpunit/Application/UseCaseRequestValidation/PropertyLabelEditRequestValidatingDeserializerTest.php b/repo/rest-api/tests/phpunit/Application/UseCaseRequestValidation/PropertyLabelEditRequestValidatingDeserializerTest.php new file mode 100644 index 00000000000..c10d02cb96f --- /dev/null +++ b/repo/rest-api/tests/phpunit/Application/UseCaseRequestValidation/PropertyLabelEditRequestValidatingDeserializerTest.php @@ -0,0 +1,31 @@ +createStub( PropertyLabelEditRequest::class ); + $request->method( 'getPropertyId' )->willReturn( 'P1' ); + $request->method( 'getLanguageCode' )->willReturn( 'en' ); + $request->method( 'getLabel' )->willReturn( 'some-property-label' ); + + $this->assertEquals( + new Term( 'en', 'some-property-label' ), + ( new PropertyLabelEditRequestValidatingDeserializer() )->validateAndDeserialize( $request ) + ); + } + +} diff --git a/repo/rest-api/tests/phpunit/Application/UseCases/SetPropertyLabel/SetPropertyLabelRequestTest.php b/repo/rest-api/tests/phpunit/Application/UseCases/SetPropertyLabel/SetPropertyLabelRequestTest.php new file mode 100644 index 00000000000..c0cb0ed0050 --- /dev/null +++ b/repo/rest-api/tests/phpunit/Application/UseCases/SetPropertyLabel/SetPropertyLabelRequestTest.php @@ -0,0 +1,45 @@ +assertEquals( $propertyId, $request->getPropertyId() ); + $this->assertEquals( $langCode, $request->getLanguageCode() ); + $this->assertEquals( $newLabelText, $request->getLabel() ); + $this->assertEquals( $editTags, $request->getEditTags() ); + $this->assertFalse( $request->isBot() ); + } + + public function testNewWithTrimmedLabel(): void { + $propertyId = 'P123'; + $langCode = 'en'; + $spaceyLabelText = ' New label '; + $editTags = [ 'some', 'tags' ]; + $isBot = false; + + $request = new SetPropertyLabelRequest( $propertyId, $langCode, $spaceyLabelText, $editTags, $isBot, null, null ); + $this->assertEquals( $propertyId, $request->getPropertyId() ); + $this->assertEquals( $langCode, $request->getLanguageCode() ); + $this->assertEquals( 'New label', $request->getLabel() ); + $this->assertEquals( $editTags, $request->getEditTags() ); + $this->assertFalse( $request->isBot() ); + } + +} diff --git a/repo/rest-api/tests/phpunit/Application/UseCases/SetPropertyLabel/SetPropertyLabelTest.php b/repo/rest-api/tests/phpunit/Application/UseCases/SetPropertyLabel/SetPropertyLabelTest.php new file mode 100644 index 00000000000..91791bf327d --- /dev/null +++ b/repo/rest-api/tests/phpunit/Application/UseCases/SetPropertyLabel/SetPropertyLabelTest.php @@ -0,0 +1,136 @@ +validator = new TestValidatingRequestDeserializer(); + $this->propertyRetriever = $this->createStub( PropertyRetriever::class ); + $this->propertyUpdater = $this->createStub( PropertyUpdater::class ); + } + + public function testAddLabel(): void { + $propertyId = 'P1'; + $langCode = 'en'; + $newLabelText = 'New label'; + $editTags = TestValidatingRequestDeserializer::ALLOWED_TAGS; + $isBot = false; + $comment = "{$this->getName()} Comment"; + $revisionId = 432; + $lastModified = '20231006040506'; + $property = new DataModelProperty( new NumericPropertyId( $propertyId ), null, 'string' ); + + $this->propertyRetriever = $this->createMock( PropertyRetriever::class ); + $this->propertyRetriever->expects( $this->once() )->method( 'getProperty' )->with( $propertyId )->willReturn( $property ); + + $updatedProperty = new Property( + new Labels( new Label( $langCode, $newLabelText ) ), + new Descriptions(), + new Aliases(), + new StatementList() + ); + $this->propertyUpdater = $this->createMock( PropertyUpdater::class ); + $this->propertyUpdater->expects( $this->once() )->method( 'update' ) + ->with( + $this->callback( + fn( DataModelProperty $property ) => $property->getLabels()->toTextArray() === [ $langCode => $newLabelText ] + ), + $this->expectEquivalentMetadata( $editTags, $isBot, $comment, EditSummary::ADD_ACTION ) + ) + ->willReturn( new PropertyRevision( $updatedProperty, $lastModified, $revisionId ) ); + + $request = new SetPropertyLabelRequest( $propertyId, $langCode, $newLabelText, $editTags, $isBot, $comment, null ); + $response = $this->newUseCase()->execute( $request ); + + $this->assertEquals( new Label( $langCode, $newLabelText ), $response->getLabel() ); + $this->assertSame( $revisionId, $response->getRevisionId() ); + $this->assertSame( $lastModified, $response->getLastModified() ); + } + + public function testReplaceLabel(): void { + $propertyId = 'P1'; + $langCode = 'en'; + $updatedLabelText = 'Replaced label'; + $editTags = TestValidatingRequestDeserializer::ALLOWED_TAGS; + $isBot = false; + $comment = "{$this->getName()} Comment"; + $revisionId = 432; + $lastModified = '20231006040506'; + $property = new DataModelProperty( + new NumericPropertyId( $propertyId ), + new Fingerprint( new TermList( [ new Term( 'en', 'Label to replace' ) ] ) ), + 'string' + ); + + $this->propertyRetriever = $this->createMock( PropertyRetriever::class ); + $this->propertyRetriever->expects( $this->once() )->method( 'getProperty' )->with( $propertyId )->willReturn( $property ); + + $updatedProperty = new Property( + new Labels( new Label( $langCode, $updatedLabelText ) ), + new Descriptions(), + new Aliases(), + new StatementList() + ); + $this->propertyUpdater = $this->createMock( PropertyUpdater::class ); + $this->propertyUpdater->expects( $this->once() )->method( 'update' ) + ->with( + $this->callback( + fn( DataModelProperty $property ) => $property->getLabels()->toTextArray() === [ $langCode => $updatedLabelText ] + ), + $this->expectEquivalentMetadata( $editTags, $isBot, $comment, EditSummary::REPLACE_ACTION ) + ) + ->willReturn( new PropertyRevision( $updatedProperty, $lastModified, $revisionId ) ); + + $request = new SetPropertyLabelRequest( $propertyId, $langCode, $updatedLabelText, $editTags, $isBot, $comment, null ); + $response = $this->newUseCase()->execute( $request ); + + $this->assertEquals( new Label( $langCode, $updatedLabelText ), $response->getLabel() ); + $this->assertSame( $revisionId, $response->getRevisionId() ); + $this->assertSame( $lastModified, $response->getLastModified() ); + } + + private function newUseCase(): SetPropertyLabel { + return new SetPropertyLabel( + $this->validator, + $this->propertyRetriever, + $this->propertyUpdater + ); + } + +} diff --git a/repo/rest-api/tests/phpunit/Infrastructure/ValidatingRequestDeserializerTest.php b/repo/rest-api/tests/phpunit/Infrastructure/ValidatingRequestDeserializerTest.php index efba55686b4..4234c299e1e 100644 --- a/repo/rest-api/tests/phpunit/Infrastructure/ValidatingRequestDeserializerTest.php +++ b/repo/rest-api/tests/phpunit/Infrastructure/ValidatingRequestDeserializerTest.php @@ -30,6 +30,8 @@ use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyFieldsRequest; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyIdFilterRequest; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyIdRequest; +use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyLabelEditRequest; +use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\PropertyLabelEditRequestValidatingDeserializer; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\StatementIdRequest; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\StatementIdRequestValidatingDeserializer; use Wikibase\Repo\RestApi\Application\UseCaseRequestValidation\StatementSerializationRequest; @@ -157,7 +159,17 @@ public function testGivenValidItemLabelEditRequest_returnsLabel(): void { $request->method( 'getLabel' )->willReturn( 'potato' ); $result = $this->newRequestDeserializer()->validateAndDeserialize( $request ); - $this->assertEquals( new Term( 'en', 'potato' ), $result->getLabel() ); + $this->assertEquals( new Term( 'en', 'potato' ), $result->getItemLabel() ); + } + + public function testGivenValidPropertyLabelEditRequest_returnsLabel(): void { + $request = $this->createStub( PropertyLabelEditUseCaseRequest::class ); + $request->method( 'getPropertyId' )->willReturn( 'P123' ); + $request->method( 'getLanguageCode' )->willReturn( 'en' ); + $request->method( 'getLabel' )->willReturn( 'instance of' ); + + $result = $this->newRequestDeserializer()->validateAndDeserialize( $request ); + $this->assertEquals( new Term( 'en', 'instance of' ), $result->getPropertyLabel() ); } public function testGivenValidItemDescriptionEditRequest_returnsDescription(): void { @@ -290,6 +302,11 @@ public function invalidRequestProvider(): Generator { ItemLabelEditRequestValidatingDeserializer::class, ValidatingRequestDeserializer::ITEM_LABEL_EDIT_REQUEST_VALIDATING_DESERIALIZER, ]; + yield [ + PropertyLabelEditUseCaseRequest::class, + PropertyLabelEditRequestValidatingDeserializer::class, + ValidatingRequestDeserializer::PROPERTY_LABEL_EDIT_REQUEST_VALIDATING_DESERIALIZER, + ]; yield [ ItemDescriptionEditUseCaseRequest::class, ItemDescriptionEditRequestValidatingDeserializer::class, @@ -322,6 +339,7 @@ interface StatementSerializationUseCaseRequest extends UseCaseRequest, Statement interface EditMetadataUseCaseRequest extends UseCaseRequest, EditMetadataRequest {} interface PatchUseCaseRequest extends UseCaseRequest, PatchRequest {} interface ItemLabelEditUseCaseRequest extends UseCaseRequest, ItemLabelEditRequest {} +interface PropertyLabelEditUseCaseRequest extends UseCaseRequest, PropertyLabelEditRequest {} interface ItemDescriptionEditUseCaseRequest extends UseCaseRequest, ItemDescriptionEditRequest {} interface PropertyDescriptionEditUseCaseRequest extends UseCaseRequest, PropertyDescriptionEditRequest {} interface PropertyFieldsUseCaseRequest extends UseCaseRequest, PropertyFieldsRequest {}