Daniel Werner has submitted this change and it was merged. Change subject: Revert introduction of ApiModifyClaim ......................................................................
Revert introduction of ApiModifyClaim ApiModifyClaim and how it's used is abuse of inheritance that introduces many problems This partially reverts commit e60f6cad52ce2568be01214c32fa959a8c6d5a98 Change-Id: Iac13cf92a5160e996e146afc6d7c5834944340d4 --- M repo/Wikibase.php M repo/includes/api/ApiCreateClaim.php D repo/includes/api/ApiModifyClaim.php M repo/includes/api/ApiRemoveClaims.php M repo/includes/api/ApiSetClaimValue.php 5 files changed, 244 insertions(+), 257 deletions(-) Approvals: Daniel Werner: Verified; Looks good to me, approved diff --git a/repo/Wikibase.php b/repo/Wikibase.php index 2111cee..26e8bed 100644 --- a/repo/Wikibase.php +++ b/repo/Wikibase.php @@ -133,7 +133,6 @@ $wgAutoloadClasses['Wikibase\ApiEditEntity'] = $dir . 'includes/api/ApiEditEntity.php'; $wgAutoloadClasses['Wikibase\ApiGetEntities'] = $dir . 'includes/api/ApiGetEntities.php'; $wgAutoloadClasses['Wikibase\ApiLinkTitles'] = $dir . 'includes/api/ApiLinkTitles.php'; -$wgAutoloadClasses['Wikibase\ApiModifyClaim'] = $dir . 'includes/api/ApiModifyClaim.php'; $wgAutoloadClasses['Wikibase\ApiModifyEntity'] = $dir . 'includes/api/ApiModifyEntity.php'; $wgAutoloadClasses['Wikibase\ApiModifyLangAttribute'] = $dir . 'includes/api/ApiModifyLangAttribute.php'; $wgAutoloadClasses['Wikibase\ApiSearchEntities'] = $dir . 'includes/api/ApiSearchEntities.php'; diff --git a/repo/includes/api/ApiCreateClaim.php b/repo/includes/api/ApiCreateClaim.php index 9661965..4c661a8 100644 --- a/repo/includes/api/ApiCreateClaim.php +++ b/repo/includes/api/ApiCreateClaim.php @@ -28,9 +28,8 @@ * * @licence GNU GPL v2+ * @author Jeroen De Dauw < [email protected] > - * @author Daniel Kinzler */ -class ApiCreateClaim extends ApiModifyClaim { +class ApiCreateClaim extends Api implements ApiAutocomment { // TODO: automcomment // TODO: rights @@ -91,24 +90,40 @@ } /** - * @since 0.2 + * @since 0.3 * - * @return EntityContent + * @param EntityContent $content */ - protected function getEntityContent() { + protected function saveChanges( EntityContent $content ) { $params = $this->extractRequestParams(); + $user = $this->getUser(); + $flags = 0; $baseRevisionId = isset( $params['baserevid'] ) ? intval( $params['baserevid'] ) : null; + $baseRevisionId = $baseRevisionId > 0 ? $baseRevisionId : false; + $flags |= ( $user->isAllowed( 'bot' ) && $params['bot'] ) ? EDIT_FORCE_BOT : 0; + $flags |= EDIT_UPDATE; + $editEntity = new EditEntity( $content, $user, $baseRevisionId, $this->getContext() ); - $entityId = EntityId::newFromPrefixedId( $params['entity'] ); - $entityTitle = $entityId ? EntityContentFactory::singleton()->getTitleForId( $entityId ) : null; - $entityContent = $entityTitle === null ? null : $this->loadEntityContent( $entityTitle, $baseRevisionId ); + $status = $editEntity->attemptSave( + '', // TODO: autocomment + $flags, + isset( $params['token'] ) ? $params['token'] : '' + ); - if ( $entityContent === null ) { - $this->dieUsage( 'Entity ' . $params['entity'] . ' not found', 'entity-not-found' ); + if ( !$status->isOK() ) { + $this->dieUsage( 'Failed to save the change', 'createclaim-save-failed' ); } - return $entityContent; + $statusValue = $status->getValue(); + + if ( isset( $statusValue['revision'] ) ) { + $this->getResult()->addValue( + 'pageinfo', + 'lastrevid', + (int)$statusValue['revision']->getId() + ); + } } /** @@ -135,6 +150,70 @@ } /** + * @since 0.2 + * + * @return EntityContent + */ + protected function getEntityContent() { + $params = $this->extractRequestParams(); + + $baseRevisionId = isset( $params['baserevid'] ) ? intval( $params['baserevid'] ) : null; + + $entityId = EntityId::newFromPrefixedId( $params['entity'] ); + $entityTitle = $entityId ? EntityContentFactory::singleton()->getTitleForId( $entityId ) : null; + $entityContent = $entityTitle === null ? null : $this->loadEntityContent( $entityTitle, $baseRevisionId ); + + if ( $entityContent === null ) { + $this->dieUsage( 'Entity not found, snak not created', 'entity-not-found' ); + } + + return $entityContent; + } + + /** + * @since 0.2 + * + * @return Snak + * @throws MWException + */ + protected function getSnakInstance() { + $params = $this->extractRequestParams(); + + $factory = new SnakFactory(); + + $libRegistry = new LibRegistry( Settings::singleton() ); + $parseResult = $libRegistry->getEntityIdParser()->parse( $params['property'] ); + + if ( !$parseResult->isValid() ) { + throw new MWException( $parseResult->getError()->getText() ); + } + + return $factory->newSnak( + $parseResult->getValue(), + $params['snaktype'], + isset( $params['value'] ) ? \FormatJson::decode( $params['value'], true ) : null + ); + } + + /** + * @since 0.3 + * + * @param Claim $claim + */ + protected function outputClaim( Claim $claim ) { + $serializerFactory = new \Wikibase\Lib\Serializers\SerializerFactory(); + + $serializer = $serializerFactory->newSerializerForObject( $claim ); + $serializer->getOptions()->setIndexTags( $this->getResult()->getIsRawMode() ); + + $this->getResult()->addValue( + null, + 'claim', + $serializer->getSerialized( $claim ) + ); + } + + /** * @see ApiBase::getAllowedParams * * @since 0.2 @@ -142,9 +221,13 @@ * @return array */ public function getAllowedParams() { - return array_merge( parent::getAllowedParams(), array( + return array( 'entity' => array( ApiBase::PARAM_TYPE => 'string', + ApiBase::PARAM_REQUIRED => true, + ), + 'snaktype' => array( + ApiBase::PARAM_TYPE => array( 'value', 'novalue', 'somevalue' ), ApiBase::PARAM_REQUIRED => true, ), 'property' => array( @@ -155,11 +238,12 @@ ApiBase::PARAM_TYPE => 'string', ApiBase::PARAM_REQUIRED => false, ), - 'snaktype' => array( - ApiBase::PARAM_TYPE => array( 'value', 'novalue', 'somevalue' ), - ApiBase::PARAM_REQUIRED => true, + 'token' => null, + 'baserevid' => array( + ApiBase::PARAM_TYPE => 'integer', ), - ) ); + 'bot' => null, + ); } /** @@ -170,12 +254,19 @@ * @return array */ public function getParamDescription() { - return array_merge( parent::getParamDescription(), array( + return array( 'entity' => 'Id of the entity you are adding the claim to', 'property' => 'Id of the snaks property', 'value' => 'Value of the snak when creating a claim with a snak that has a value', 'snaktype' => 'The type of the snak', - ) ); + 'token' => 'An "edittoken" token previously obtained through the token module (prop=info).', + 'baserevid' => array( 'The numeric identifier for the revision to base the modification on.', + "This is used for detecting conflicts during save." + ), + 'bot' => array( 'Mark this edit as bot', + 'This URL flag will only be respected if the user belongs to the group "bot".' + ), + ); } /** @@ -226,4 +317,5 @@ Autocomment::pickValuesFromParams( $params, 'property' ) ); } + } diff --git a/repo/includes/api/ApiModifyClaim.php b/repo/includes/api/ApiModifyClaim.php deleted file mode 100644 index 1ed6fd5..0000000 --- a/repo/includes/api/ApiModifyClaim.php +++ /dev/null @@ -1,221 +0,0 @@ -<?php - -namespace Wikibase; -use ApiBase, MWException; - -/** - * Base module for handling claims. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - * http://www.gnu.org/copyleft/gpl.html - * - * @since 0.4 - * - * @ingroup WikibaseRepo - * @ingroup API - * - * @licence GNU GPL v2+ - * @author Jeroen De Dauw < [email protected] > - * @author Daniel Kinzler - */ -abstract class ApiModifyClaim extends Api implements ApiAutocomment { - - /** - * @since 0.4 - * - * @param EntityContent $content The content to save - * @param int $flags Edit flags, e.g. EDIT_NEW - * @param string|null $summary The summary to set. If null, the summary will be auto-generated. - * - * @return void - */ - protected function saveChanges( EntityContent $content, $summary = null ) { - $params = $this->extractRequestParams(); - - if ( $summary === null ) { - $summary = Autocomment::buildApiSummary( $this, $params, $content ); - } - - $user = $this->getUser(); - $baseRevisionId = isset( $params['baserevid'] ) ? intval( $params['baserevid'] ) : null; - $baseRevisionId = $baseRevisionId > 0 ? $baseRevisionId : false; - - $flags = EDIT_UPDATE; - $flags |= ( $user->isAllowed( 'bot' ) && $params['bot'] ) ? EDIT_FORCE_BOT : 0; - - $editEntity = new EditEntity( $content, $user, $baseRevisionId, $this->getContext() ); - - $status = $editEntity->attemptSave( - $summary, - $flags, - isset( $params['token'] ) ? $params['token'] : '' - ); - - if ( !$status->isOK() ) { - $this->dieUsage( $status->getHTML( 'wikibase-api-save-failed' ), 'save-failed' ); - } - - $statusValue = $status->getValue(); - - if ( isset( $statusValue['revision'] ) ) { - $this->getResult()->addValue( - 'pageinfo', - 'lastrevid', - (int)$statusValue['revision']->getId() - ); - } - } - - /** - * Checks if the required parameters are set and are valid and consistent. - * - * @since 0.2 - */ - protected function checkParameterRequirements() { - // noop - } - - /** - * @since 0.2 - * - * @return Snak - * @throws MWException - */ - protected function getSnakInstance() { - $params = $this->extractRequestParams(); - - $factory = new SnakFactory(); - - return $factory->newSnak( - $this->getPropertyId(), - $params['snaktype'], - isset( $params['value'] ) ? \FormatJson::decode( $params['value'], true ) : null - ); - } - - /** - * @return EntityId - */ - protected function getPropertyId() { - $params = $this->extractRequestParams(); - - $libRegistry = new LibRegistry( Settings::singleton() ); - $parseResult = $libRegistry->getEntityIdParser()->parse( $params['property'] ); - - if ( !$parseResult->isValid() ) { - $this->dieUsage( $parseResult->getError()->getText(), 'illegal-property-id' ); - } - - return $parseResult->getValue(); - } - - /** - * @since 0.3 - * - * @param Claim $claim - */ - protected function outputClaim( Claim $claim ) { - $serializerFactory = new \Wikibase\Lib\Serializers\SerializerFactory(); - - $serializer = $serializerFactory->newSerializerForObject( $claim ); - $serializer->getOptions()->setIndexTags( $this->getResult()->getIsRawMode() ); - - $this->getResult()->addValue( - null, - 'claim', - $serializer->getSerialized( $claim ) - ); - } - - /** - * @see ApiBase::getAllowedParams - * - * @since 0.2 - * - * @return array - */ - public function getAllowedParams() { - return array( - 'token' => null, - 'baserevid' => array( - ApiBase::PARAM_TYPE => 'integer', - ), - 'bot' => null, - ); - } - - /** - * @see ApiBase::getParamDescription - * - * @since 0.2 - * - * @return array - */ - public function getParamDescription() { - return array( - 'token' => 'An "edittoken" token previously obtained through the token module (prop=info).', - 'baserevid' => array( 'The numeric identifier for the revision to base the modification on.', - "This is used for detecting conflicts during save." - ), - 'bot' => array( 'Mark this edit as bot', - 'This URL flag will only be respected if the user belongs to the group "bot".' - ), - ); - } - - /** - * @see ApiBase::getHelpUrls - * - * @since 0.2 - * - * @return string - */ - public function getHelpUrls() { - return 'https://www.mediawiki.org/wiki/Extension:Wikibase/API#' . $this->getModuleName(); - } - - /** - * @see ApiBase::getVersion - * - * @since 0.2 - * - * @return string - */ - public function getVersion() { - return get_class( $this ) . '-' . WB_VERSION; - } - - /** - * @see \ApiBase::needsToken() - */ - public function needsToken() { - return Settings::get( 'apiInDebug' ) ? Settings::get( 'apiDebugWithTokens' ) : true; - } - - /** - * @see \ApiBase::mustBePosted() - */ - public function mustBePosted() { - return Settings::get( 'apiInDebug' ) ? Settings::get( 'apiDebugWithPost' ) : true; - } - - /** - * @see \ApiBase::isWriteMode() - */ - public function isWriteMode() { - return true; - } - -} diff --git a/repo/includes/api/ApiRemoveClaims.php b/repo/includes/api/ApiRemoveClaims.php index 2e9e8d6..4430f56 100644 --- a/repo/includes/api/ApiRemoveClaims.php +++ b/repo/includes/api/ApiRemoveClaims.php @@ -28,9 +28,8 @@ * * @licence GNU GPL v2+ * @author Jeroen De Dauw < [email protected] > - * @author Daniel Kinzler */ -class ApiRemoveClaims extends ApiModifyClaim { +class ApiRemoveClaims extends Api { // TODO: example // TODO: rights @@ -108,6 +107,7 @@ ); $entity->setClaims( $claims ); + $this->saveChanges( $entityContent ); } @@ -191,33 +191,82 @@ } /** + * @since 0.3 + * + * @param EntityContent $content + */ + protected function saveChanges( EntityContent $content ) { + $params = $this->extractRequestParams(); + + $user = $this->getUser(); + $flags = 0; + $baseRevisionId = isset( $params['baserevid'] ) ? intval( $params['baserevid'] ) : null; + $baseRevisionId = $baseRevisionId > 0 ? $baseRevisionId : false; + $flags |= ( $user->isAllowed( 'bot' ) && $params['bot'] ) ? EDIT_FORCE_BOT : 0; + $flags |= EDIT_UPDATE; + $editEntity = new EditEntity( $content, $user, $baseRevisionId, $this->getContext() ); + + $status = $editEntity->attemptSave( + '', // TODO: automcomment + $flags, + isset( $params['token'] ) ? $params['token'] : '' + ); + + if ( !$status->isGood() ) { + $this->dieUsage( 'Failed to save the change', 'save-failed' ); + } + + $statusValue = $status->getValue(); + + if ( isset( $statusValue['revision'] ) ) { + $this->getResult()->addValue( + 'pageinfo', + 'lastrevid', + (int)$statusValue['revision']->getId() + ); + } + } + + /** * @see ApiBase::getAllowedParams * - * @since 0.2 + * @since 0.3 * * @return array */ public function getAllowedParams() { - return array_merge( parent::getAllowedParams(), array( + return array( 'claim' => array( ApiBase::PARAM_TYPE => 'string', ApiBase::PARAM_ISMULTI => true, ApiBase::PARAM_REQUIRED => true, ), - ) ); + 'token' => null, + 'baserevid' => array( + ApiBase::PARAM_TYPE => 'integer', + ), + 'bot' => null, + ); } /** * @see ApiBase::getParamDescription * - * @since 0.2 + * @since 0.3 * * @return array */ public function getParamDescription() { - return array_merge( parent::getParamDescription(), array( + return array( 'claim' => 'A GUID identifying the claim', - ) ); + 'token' => 'An "edittoken" token previously obtained through the token module (prop=info).', + 'baserevid' => array( 'The numeric identifier for the revision to base the modification on.', + "This is used for detecting conflicts during save." + ), + 'bot' => array( 'Mark this edit as bot', + 'This URL flag will only be respected if the user belongs to the group "bot".' + ), + ); } /** @@ -269,4 +318,5 @@ Autocomment::pickValuesFromParams( $params, 'claim' ) ); } + } diff --git a/repo/includes/api/ApiSetClaimValue.php b/repo/includes/api/ApiSetClaimValue.php index f592fd6..3bc36f8 100644 --- a/repo/includes/api/ApiSetClaimValue.php +++ b/repo/includes/api/ApiSetClaimValue.php @@ -28,9 +28,8 @@ * * @licence GNU GPL v2+ * @author Jeroen De Dauw < [email protected] > - * @author Daniel Kinzler */ -class ApiSetClaimValue extends ApiModifyClaim { +class ApiSetClaimValue extends Api { // TODO: example // TODO: rights @@ -131,14 +130,69 @@ } /** + * @since 0.3 + * + * @param EntityContent $content + */ + protected function saveChanges( EntityContent $content ) { + $params = $this->extractRequestParams(); + + $user = $this->getUser(); + $flags = 0; + $baseRevisionId = isset( $params['baserevid'] ) ? intval( $params['baserevid'] ) : null; + $baseRevisionId = $baseRevisionId > 0 ? $baseRevisionId : false; + $flags |= ( $user->isAllowed( 'bot' ) && $params['bot'] ) ? EDIT_FORCE_BOT : 0; + $flags |= EDIT_UPDATE; + $editEntity = new EditEntity( $content, $user, $baseRevisionId, $this->getContext() ); + + $status = $editEntity->attemptSave( + '', // TODO: automcomment + $flags, + isset( $params['token'] ) ? $params['token'] : '' + ); + + if ( !$status->isGood() ) { + $this->dieUsage( 'Failed to save the change', 'setclaimvalue-save-failed' ); + } + + $statusValue = $status->getValue(); + + if ( isset( $statusValue['revision'] ) ) { + $this->getResult()->addValue( + 'pageinfo', + 'lastrevid', + (int)$statusValue['revision']->getId() + ); + } + } + + /** + * @since 0.3 + * + * @param Claim $claim + */ + protected function outputClaim( Claim $claim ) { + $serializerFactory = new \Wikibase\Lib\Serializers\SerializerFactory(); + $serializer = $serializerFactory->newSerializerForObject( $claim ); + + $serializer->getOptions()->setIndexTags( $this->getResult()->getIsRawMode() ); + + $this->getResult()->addValue( + null, + 'claim', + $serializer->getSerialized( $claim ) + ); + } + + /** * @see ApiBase::getAllowedParams * - * @since 0.2 + * @since 0.3 * * @return array */ public function getAllowedParams() { - return array_merge( parent::getAllowedParams(), array( + return array( 'claim' => array( ApiBase::PARAM_TYPE => 'string', ApiBase::PARAM_REQUIRED => true, @@ -151,22 +205,35 @@ ApiBase::PARAM_TYPE => array( 'value', 'novalue', 'somevalue' ), ApiBase::PARAM_REQUIRED => true, ), - ) ); + 'token' => null, + 'baserevid' => array( + ApiBase::PARAM_TYPE => 'integer', + ), + 'bot' => null, + ); } /** * @see ApiBase::getParamDescription * - * @since 0.2 + * @since 0.3 * * @return array */ public function getParamDescription() { - return array_merge( parent::getParamDescription(), array( + return array( 'claim' => 'A GUID identifying the claim', 'snaktype' => 'The type of the snak', 'value' => 'The value to set the datavalue of the the main snak of the claim to', - ) ); + 'token' => 'An "edittoken" token previously obtained through the token module (prop=info).', + 'baserevid' => array( 'The numeric identifier for the revision to base the modification on.', + "This is used for detecting conflicts during save." + ), + 'bot' => array( 'Mark this edit as bot', + 'This URL flag will only be respected if the user belongs to the group "bot".' + ), + + ); } /** @@ -196,7 +263,6 @@ ); } - /** * @see ApiAutocomment::getTextForComment() */ @@ -217,4 +283,5 @@ Autocomment::pickValuesFromParams( $params, 'claim' ) ); } + } -- To view, visit https://gerrit.wikimedia.org/r/50494 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iac13cf92a5160e996e146afc6d7c5834944340d4 Gerrit-PatchSet: 4 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Jeroen De Dauw <[email protected]> Gerrit-Reviewer: Daniel Kinzler <[email protected]> Gerrit-Reviewer: Daniel Werner <[email protected]> Gerrit-Reviewer: Jeroen De Dauw <[email protected]> Gerrit-Reviewer: John Erling Blad <[email protected]> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
