Thiemo Mättig (WMDE) has uploaded a new change for review.
https://gerrit.wikimedia.org/r/179881
Change subject: Rework and cleanup ApiWikibase and subclasses
......................................................................
Rework and cleanup ApiWikibase and subclasses
Main changes are:
* Narrow interfaces from Entity to EntityId.
* Remove hard coded lists of entity types if possible.
* Make stuff private if possible.
* Fix docs.
Change-Id: Ic506adc1145d04e3f4af8f0aa5d4b65ea0d4de28
---
M repo/includes/api/ApiWikibase.php
M repo/includes/api/EditEntity.php
M repo/includes/api/MergeItems.php
M repo/includes/api/ModifyEntity.php
M repo/includes/api/ModifyTerm.php
M repo/includes/api/SetAliases.php
M repo/includes/api/SetSiteLink.php
7 files changed, 164 insertions(+), 171 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/81/179881/1
diff --git a/repo/includes/api/ApiWikibase.php
b/repo/includes/api/ApiWikibase.php
index 86f6b89..727cbb6 100644
--- a/repo/includes/api/ApiWikibase.php
+++ b/repo/includes/api/ApiWikibase.php
@@ -12,10 +12,10 @@
use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdParser;
+use Wikibase\DataModel\Entity\PropertyDataTypeLookup;
use Wikibase\EditEntity;
use Wikibase\EntityRevision;
use Wikibase\Lib\Localizer\ExceptionLocalizer;
-use Wikibase\DataModel\Entity\PropertyDataTypeLookup;
use Wikibase\Lib\Store\BadRevisionException;
use Wikibase\Lib\Store\EntityRevisionLookup;
use Wikibase\Lib\Store\EntityStore;
@@ -155,14 +155,17 @@
}
/**
- * @param Entity $entity
+ * @param EntityId|null $entityId
*
* @return bool
*/
- protected function entityExists( Entity $entity ) {
- $entityId = $entity->getId();
- $title = $entityId === null ? null :
$this->titleLookup->getTitleForId( $entityId );
- return ( $title !== null && $title->exists() );
+ protected function entityExists( EntityId $entityId = null ) {
+ if ( $entityId === null ) {
+ return false;
+ }
+
+ $title = $this->titleLookup->getTitleForId( $entityId );
+ return $title !== null && $title->exists();
}
/**
@@ -173,29 +176,36 @@
}
/**
- * @see ApiBase::getAllowedParams()
+ * @see ApiBase::getAllowedParams
+ *
+ * @return array Empty in this abstract base implementation.
*/
public function getAllowedParams() {
- return array(
- );
+ return array();
}
/**
- * @see ApiBase::needsToken()
+ * @see ApiBase::needsToken
+ *
+ * @return string|false
*/
public function needsToken() {
return $this->isWriteMode() ? 'csrf' : false;
}
/**
- * @see ApiBase::getTokenSalt()
+ * @see ApiBase::getTokenSalt
+ *
+ * @return string|false
*/
public function getTokenSalt() {
return $this->needsToken() ? '' : false;
}
/**
- * @see ApiBase::mustBePosted()
+ * @see ApiBase::mustBePosted
+ *
+ * @return bool
*/
public function mustBePosted() {
return $this->isWriteMode();
@@ -203,6 +213,8 @@
/**
* @see ApiBase::isReadMode
+ *
+ * @return bool Always true in this abstract base implementation.
*/
public function isReadMode() {
return true;
@@ -226,12 +238,11 @@
* Per default, this will include the 'read' permission if
$this->isReadMode() returns true,
* and the 'edit' permission if $this->isWriteMode() returns true,
*
- * @param Entity $entity The entity to check permissions for
- * @param array $params Arguments for the module, describing the
operation to be performed
+ * @param EntityId|null $entityId The entity to check permissions for
*
- * @return array A list of permissions
+ * @return string[] A list of permissions
*/
- protected function getRequiredPermissions( Entity $entity, array
$params ) {
+ protected function getRequiredPermissions( EntityId $entityId = null ) {
$permissions = array();
if ( $this->isReadMode() ) {
@@ -250,13 +261,12 @@
*
* @param $entity Entity the entity to check
* @param $user User doing the action
- * @param $params array of arguments for the module, passed for
ModifyItem
*
* @return Status the check's result
* @todo: use this also to check for read access in ApiGetEntities, etc
*/
- protected function checkPermissions( Entity $entity, User $user, array
$params ) {
- $permissions = $this->getRequiredPermissions( $entity, $params
);
+ protected function checkPermissions( Entity $entity, User $user ) {
+ $permissions = $this->getRequiredPermissions( $entity->getId()
);
$status = Status::newGood();
foreach ( array_unique( $permissions ) as $perm ) {
@@ -313,11 +323,11 @@
* @note: this function may or may not return normally, depending on
whether
* the status is fatal or not.
*
- * @see handleStatus().
+ * @see handleStatus
*
* @param Status $status The status to report
*/
- protected function handleSaveStatus( Status $status ) {
+ private function handleSaveStatus( Status $status ) {
$value = $status->getValue();
$errorCode = null;
@@ -393,7 +403,7 @@
*
* @throws LogicException if not in write mode
* @return Status the status of the save operation, as returned by
EditEntity::attemptSave()
- * @see EditEntity::attemptSave()
+ * @see EditEntity::attemptSave
*
* @todo: this could be factored out into a controller-like class, but
that controller
* would still need knowledge of the API module to be useful.
We'll put it here
@@ -406,7 +416,7 @@
}
if ( $summary instanceof Summary ) {
- $summary = $this->formatSummary( $summary );
+ $summary = $this->summaryFormatter->formatSummary(
$summary );
}
$params = $this->extractRequestParams();
@@ -444,40 +454,29 @@
/**
* @param array $params
*
- * @return false|null|string
+ * @return string|bool|null Token string, or false if not needed, or
null if not set.
*/
private function evaluateTokenParam( array $params ) {
if ( !$this->needsToken() ) {
- // false disabled the token check
- $token = false;
- } else {
- // null fails the token check
- $token = isset( $params['token'] ) ? $params['token'] :
null;
+ // False disables the token check.
+ return false;
}
- return $token;
+ // Null fails the token check.
+ return isset( $params['token'] ) ? $params['token'] : null;
}
/**
* @param array $params
*
- * @return null|false|int
+ * @return int|bool|null Revision id, or false if 0, or null if not set.
*/
private function evaluateBaseRevisionParam( array $params ) {
- $baseRevisionId = isset( $params['baserevid'] ) ? intval(
$params['baserevid'] ) : null;
- $baseRevisionId = $baseRevisionId > 0 ? $baseRevisionId : false;
+ if ( !isset( $params['baserevid'] ) ) {
+ return null;
+ }
- return $baseRevisionId;
- }
-
- /**
- * @param Summary $summary
- *
- * @return string
- */
- protected function formatSummary( Summary $summary ) {
- $formatter = $this->summaryFormatter;
- return $formatter->formatSummary( $summary );
+ return $params['baserevid'] ? (int)$params['baserevid'] : false;
}
/**
@@ -485,7 +484,7 @@
*
* @note: The returned Status will always be fatal, that is,
$status->isOk() will return false.
*
- * @see getExceptionMessage().
+ * @see getExceptionMessage
*
* @param Exception $error
*
@@ -514,7 +513,7 @@
}
/**
- * @see ApiErrorReporter::dieError()
+ * @see ApiErrorReporter::dieError
*
* @since 0.5
*
@@ -528,7 +527,7 @@
}
/**
- * @see ApiErrorReporter::dieException()
+ * @see ApiErrorReporter::dieException
*
* @since 0.5
*
@@ -542,7 +541,7 @@
}
/**
- * @see ApiErrorReporter::dieMessage()
+ * @see ApiErrorReporter::dieMessage
*
* @since 0.5
*
diff --git a/repo/includes/api/EditEntity.php b/repo/includes/api/EditEntity.php
index 40fdc2b..1926958 100644
--- a/repo/includes/api/EditEntity.php
+++ b/repo/includes/api/EditEntity.php
@@ -44,33 +44,24 @@
class EditEntity extends ModifyEntity {
/**
- * @since 0.4
- *
* @var string[]
*/
- protected $validLanguageCodes;
-
- /**
- * @since 0.5
- *
- * @var EntityRevisionLookup
- */
- protected $entityRevisionLookup;
+ private $validLanguageCodes;
/**
* @var FingerprintChangeOpFactory
*/
- protected $termChangeOpFactory;
+ private $termChangeOpFactory;
/**
* @var ClaimChangeOpFactory
*/
- protected $claimChangeOpFactory;
+ private $claimChangeOpFactory;
/**
* @var SiteLinkChangeOpFactory
*/
- protected $siteLinkChangeOpFactory;
+ private $siteLinkChangeOpFactory;
/**
* @param ApiMain $mainModule
@@ -93,18 +84,23 @@
/**
* @see ApiWikibase::getRequiredPermissions
*
- * @param Entity $entity
- * @param array $params
+ * @param EntityId|null $entityId
*
+ * @throws InvalidArgumentException
* @return string[]
*/
- protected function getRequiredPermissions( Entity $entity, array
$params ) {
- $permissions = parent::getRequiredPermissions( $entity, $params
);
+ protected function getRequiredPermissions( EntityId $entityId = null ) {
+ if ( $entityId === null ) {
+ throw new InvalidArgumentException( 'Can not edit
entity with no id' );
+ }
- if ( !$this->entityExists( $entity ) ) {
+ $permissions = parent::getRequiredPermissions( $entityId );
+
+ if ( !$this->entityExists( $entityId ) ) {
$permissions[] = 'createpage';
- if ( $entity->getType() === 'property' ) {
- $permissions[] = 'property-create';
+
+ if ( $entityId->getEntityType() !== 'item' ) {
+ $permissions[] = $entityId->getEntityType() .
'-create';
}
}
@@ -165,16 +161,16 @@
$this->validateDataProperties( $data, $entity, $baseRevId );
$revisionLookup = $this->getEntityRevisionLookup();
- $exists = $this->entityExists( $entity );
+ $exists = $this->entityExists( $entity->getId() );
if ( $params['clear'] ) {
- if( $params['baserevid'] && $exists ) {
+ if ( $params['baserevid'] && $exists ) {
$latestRevision =
$revisionLookup->getLatestRevisionId(
$entity->getId(),
EntityRevisionLookup::LATEST_FROM_MASTER
);
- if( !$baseRevId === $latestRevision ) {
+ if ( !$baseRevId === $latestRevision ) {
wfProfileOut( __METHOD__ );
$this->dieError(
'Tried to clear entity using
baserevid of entity not equal to current revision',
@@ -186,7 +182,7 @@
}
// if we create a new property, make sure we set the datatype
- if( !$exists && $entity instanceof Property ){
+ if ( !$exists && $entity instanceof Property ) {
if ( !isset( $data['datatype'] ) ) {
wfProfileOut( __METHOD__ );
$this->dieError( 'No datatype given',
'param-illegal' );
@@ -228,7 +224,7 @@
*
* @return ChangeOps
*/
- protected function getChangeOps( array $data, Entity $entity ) {
+ private function getChangeOps( array $data, Entity $entity ) {
$changeOps = new ChangeOps();
//FIXME: Use a ChangeOpBuilder so we can batch fingerprint ops
etc,
@@ -254,7 +250,7 @@
$changeOps->add( $this->getSiteLinksChangeOps(
$data['sitelinks'], $entity ) );
}
- if( array_key_exists( 'claims', $data ) ) {
+ if ( array_key_exists( 'claims', $data ) ) {
$changeOps->add(
$this->getClaimsChangeOps( $data['claims'] )
);
@@ -264,11 +260,11 @@
}
/**
- * @since 0.4
* @param array $labels
+ *
* @return ChangeOp[]
*/
- protected function getLabelChangeOps( $labels ) {
+ private function getLabelChangeOps( $labels ) {
$labelChangeOps = array();
if ( !is_array( $labels ) ) {
@@ -294,11 +290,11 @@
}
/**
- * @since 0.4
* @param array $descriptions
+ *
* @return ChangeOp[]
*/
- protected function getDescriptionChangeOps( $descriptions ) {
+ private function getDescriptionChangeOps( $descriptions ) {
$descriptionChangeOps = array();
if ( !is_array( $descriptions ) ) {
@@ -324,11 +320,11 @@
}
/**
- * @since 0.4
* @param array $aliases
+ *
* @return ChangeOp[]
*/
- protected function getAliasesChangeOps( $aliases ) {
+ private function getAliasesChangeOps( $aliases ) {
if ( !is_array( $aliases ) ) {
$this->dieError( "List of aliases must be an array",
'not-recognized-array' );
}
@@ -341,9 +337,10 @@
/**
* @param array $aliases
+ *
* @return array
*/
- protected function getIndexedAliases( array $aliases ) {
+ private function getIndexedAliases( array $aliases ) {
$indexedAliases = array();
foreach ( $aliases as $langCode => $arg ) {
@@ -359,9 +356,10 @@
/**
* @param array $indexedAliases
+ *
* @return ChangeOp[]
*/
- protected function getIndexedAliasesChangeOps( array $indexedAliases ) {
+ private function getIndexedAliasesChangeOps( array $indexedAliases ) {
$aliasesChangeOps = array();
foreach ( $indexedAliases as $langCode => $args ) {
$aliasesToSet = array();
@@ -391,14 +389,12 @@
}
/**
- * @since 0.4
- *
* @param array $siteLinks
- * @param Item $entity
+ * @param Item $item
*
* @return ChangeOp[]
*/
- protected function getSiteLinksChangeOps( $siteLinks, Item $item ) {
+ private function getSiteLinksChangeOps( $siteLinks, Item $item ) {
$siteLinksChangeOps = array();
if ( !is_array( $siteLinks ) ) {
@@ -455,20 +451,19 @@
}
/**
- * @since 0.5
- *
* @param array $claims
+ *
* @return ChangeOp[]
*/
- protected function getClaimsChangeOps( $claims ) {
+ private function getClaimsChangeOps( $claims ) {
if ( !is_array( $claims ) ) {
$this->dieError( "List of claims must be an array",
'not-recognized-array' );
}
$changeOps = array();
//check if the array is associative or in arrays by property
- if( array_keys( $claims ) !== range( 0, count( $claims ) - 1 )
){
- foreach( $claims as $subClaims ){
+ if ( array_keys( $claims ) !== range( 0, count( $claims ) - 1 )
) {
+ foreach ( $claims as $subClaims ) {
$changeOps = array_merge( $changeOps,
$this->getRemoveClaimsChangeOps(
$subClaims ),
$this->getModifyClaimsChangeOps(
$subClaims ) );
@@ -494,7 +489,7 @@
$unserializer = $serializerFactory->newUnserializerForClass(
'Wikibase\DataModel\Claim\Claim' );
foreach ( $claims as $claimArray ) {
- if( !array_key_exists( 'remove', $claimArray ) ){
+ if ( !array_key_exists( 'remove', $claimArray ) ) {
try {
$claim =
$unserializer->newFromSerialization( $claimArray );
@@ -523,8 +518,8 @@
private function getRemoveClaimsChangeOps( $claims ) {
$opsToReturn = array();
foreach ( $claims as $claimArray ) {
- if( array_key_exists( 'remove', $claimArray ) ){
- if( array_key_exists( 'id', $claimArray ) ){
+ if ( array_key_exists( 'remove', $claimArray ) ) {
+ if ( array_key_exists( 'id', $claimArray ) ) {
$opsToReturn[] =
$this->claimChangeOpFactory->newRemoveClaimOp( $claimArray['id'] );
} else {
$this->dieError( 'Cannot remove a claim
with no GUID', 'invalid-claim' );
@@ -537,7 +532,7 @@
/**
* @param Entity $entity
*/
- protected function buildResult( Entity $entity ) {
+ private function buildResult( Entity $entity ) {
$this->getResultBuilder()->addLabels( $entity->getLabels(),
'entity' );
$this->getResultBuilder()->addDescriptions(
$entity->getDescriptions(), 'entity' );
$this->getResultBuilder()->addAliases(
$entity->getAllAliases(), 'entity' );
@@ -560,13 +555,11 @@
}
/**
- * @since 0.4
- *
* @param mixed $data
* @param Entity $entity
* @param int $revId
*/
- protected function validateDataProperties( $data, Entity $entity,
$revId = 0 ) {
+ private function validateDataProperties( $data, Entity $entity, $revId
= 0 ) {
$entityId = $entity->getId();
$title = $entityId === null ? null :
$this->getTitleLookup()->getTitleForId( $entityId );
@@ -604,7 +597,7 @@
* @param array $data
* @param array $allowedProps
*/
- protected function checkValidJson( $data, array $allowedProps ) {
+ private function checkValidJson( $data, array $allowedProps ) {
if ( is_null( $data ) ) {
$this->dieError( 'Invalid json: The supplied JSON
structure could not be parsed or '
. 'recreated as a valid structure' ,
'invalid-json' );
@@ -630,7 +623,7 @@
* @param array $data
* @param Title|null $title
*/
- protected function checkPageIdProp( $data, $title ) {
+ private function checkPageIdProp( $data, $title ) {
if ( isset( $data['pageid'] )
&& ( is_object( $title ) ? $title->getArticleID() !==
$data['pageid'] : true ) ) {
$this->dieError(
@@ -644,7 +637,7 @@
* @param array $data
* @param Title|null $title
*/
- protected function checkNamespaceProp( $data, $title ) {
+ private function checkNamespaceProp( $data, $title ) {
// not completely convinced that we can use title to get the
namespace in this case
if ( isset( $data['ns'] )
&& ( is_object( $title ) ? $title->getNamespace() !==
$data['ns'] : true ) ) {
@@ -659,7 +652,7 @@
* @param array $data
* @param Title|null $title
*/
- protected function checkTitleProp( $data, $title ) {
+ private function checkTitleProp( $data, $title ) {
if ( isset( $data['title'] )
&& ( is_object( $title ) ? $title->getPrefixedText()
!== $data['title'] : true ) ) {
$this->dieError(
@@ -673,7 +666,7 @@
* @param array $data
* @param int|null $revisionId
*/
- protected function checkRevisionProp( $data, $revisionId ) {
+ private function checkRevisionProp( $data, $revisionId ) {
if ( isset( $data['lastrevid'] )
&& ( is_int( $revisionId ) ? $revisionId !==
$data['lastrevid'] : true ) ) {
$this->dieError(
@@ -693,7 +686,7 @@
}
$dataId = $this->getIdParser()->parse( $data['id'] );
- if( !$entityId->equals( $dataId ) ) {
+ if ( !$entityId->equals( $dataId ) ) {
$this->dieError(
'Invalid field used in call: "id", must
match id parameter',
'param-invalid'
@@ -703,8 +696,7 @@
}
private function checkEntityType( $data, Entity $entity ) {
- if ( isset( $data['type'] )
- && $entity->getType() !== $data['type'] ) {
+ if ( isset( $data['type'] ) && $data['type'] !==
$entity->getType() ) {
$this->dieError(
'Invalid field used in call: "type", must match
type associated with id',
'param-invalid'
@@ -737,7 +729,7 @@
}
/**
- * @see ApiBase:getExamplesMessages()
+ * @see ApiBase:getExamplesMessages
*
* @return array
*/
@@ -774,7 +766,7 @@
* @param array $arg The argument array to verify
* @param string $langCode The language code used in the value part
*/
- public function validateMultilangArgs( $arg, $langCode ) {
+ private function validateMultilangArgs( $arg, $langCode ) {
if ( !is_array( $arg ) ) {
$this->dieError(
"An array was expected, but not found in the
json for the langCode {$langCode}" ,
@@ -799,11 +791,12 @@
'inconsistent-language' );
}
}
- if ( isset( $this->validLanguageCodes ) && !array_key_exists(
$arg['language'], $this->validLanguageCodes ) ) {
- $this->dieError(
- "unknown language: {$arg['language']}",
- 'not-recognized-language' );
+ if ( !empty( $this->validLanguageCodes )
+ && !array_key_exists( $arg['language'],
$this->validLanguageCodes )
+ ) {
+ $this->dieError( 'Unknown language: ' .
$arg['language'], 'not-recognized-language' );
}
+
if ( !array_key_exists( 'remove', $arg ) && !is_string(
$arg['value'] ) ) {
$this->dieError(
"A string was expected, but not found in the
json for the langCode {$langCode} and argument 'value'" ,
@@ -816,9 +809,9 @@
*
* @param array $arg The argument array to verify
* @param string $siteCode The site code used in the argument
- * @param SiteList $sites The valid site codes as an assoc array
+ * @param SiteList|null $sites The valid sites.
*/
- public function checkSiteLinks( $arg, $siteCode, SiteList &$sites =
null ) {
+ private function checkSiteLinks( $arg, $siteCode, SiteList &$sites =
null ) {
if ( !is_array( $arg ) ) {
$this->dieError( 'An array was expected, but not found'
, 'not-recognized-array' );
}
@@ -830,8 +823,8 @@
$this->dieError( "inconsistent site:
{$siteCode} is not equal to {$arg['site']}", 'inconsistent-site' );
}
}
- if ( isset( $sites ) && !$sites->hasSite( $arg['site'] ) ) {
- $this->dieError( "unknown site: {$arg['site']}",
'not-recognized-site' );
+ if ( $sites !== null && !$sites->hasSite( $arg['site'] ) ) {
+ $this->dieError( 'Unknown site: ' . $arg['site'],
'not-recognized-site' );
}
if ( isset( $arg['title'] ) && !is_string( $arg['title'] ) ) {
$this->dieError( 'A string was expected, but not found'
, 'not-recognized-string' );
diff --git a/repo/includes/api/MergeItems.php b/repo/includes/api/MergeItems.php
index a94e6ab..1bd3982 100644
--- a/repo/includes/api/MergeItems.php
+++ b/repo/includes/api/MergeItems.php
@@ -4,6 +4,7 @@
use ApiBase;
use ApiMain;
+use InvalidArgumentException;
use LogicException;
use UsageException;
use Wikibase\DataModel\Entity\EntityIdParser;
@@ -101,14 +102,14 @@
try {
return new ItemId( $value );
- } catch ( \InvalidArgumentException $ex ) {
+ } catch ( InvalidArgumentException $ex ) {
$this->errorReporter->dieError( $ex->getMessage(),
'invalid-entity-id' );
- throw new \LogicException( 'ErrorReporter::dieError did
not throw an exception' );
+ throw new LogicException( 'ErrorReporter::dieError did
not throw an exception' );
}
}
/**
- * @see ApiBase::execute()
+ * @see ApiBase::execute
*/
public function execute() {
wfProfileIn( __METHOD__ );
@@ -204,7 +205,7 @@
}
/**
- * @see ApiBase::getExamplesMessages()
+ * @see ApiBase::getExamplesMessages
*
* @return array
*/
@@ -223,7 +224,8 @@
/**
* @see ApiBase::isWriteMode
- * @return bool true
+ *
+ * @return bool Always true.
*/
public function isWriteMode() {
return true;
diff --git a/repo/includes/api/ModifyEntity.php
b/repo/includes/api/ModifyEntity.php
index d0a07b3..63ebcbe 100644
--- a/repo/includes/api/ModifyEntity.php
+++ b/repo/includes/api/ModifyEntity.php
@@ -309,7 +309,7 @@
}
/**
- * @see ApiBase::execute()
+ * @see ApiBase::execute
*
* @since 0.1
*/
@@ -342,7 +342,7 @@
}
// At this point only change/edit rights should be checked
- $status = $this->checkPermissions( $entity, $user, $params );
+ $status = $this->checkPermissions( $entity, $user );
if ( !$status->isOK() ) {
wfProfileOut( __METHOD__ );
@@ -407,7 +407,9 @@
}
/**
- * @see ApiBase::isWriteMode()
+ * @see ApiBase::isWriteMode
+ *
+ * @return bool Always true.
*/
public function isWriteMode() {
return true;
@@ -468,4 +470,5 @@
'bot' => false,
);
}
+
}
diff --git a/repo/includes/api/ModifyTerm.php b/repo/includes/api/ModifyTerm.php
index 21930ef..ab26e2e 100644
--- a/repo/includes/api/ModifyTerm.php
+++ b/repo/includes/api/ModifyTerm.php
@@ -4,9 +4,7 @@
use ApiBase;
use InvalidArgumentException;
-use Wikibase\DataModel\Entity\Entity;
-use Wikibase\DataModel\Entity\Item;
-use Wikibase\DataModel\Entity\Property;
+use Wikibase\DataModel\Entity\EntityId;
use Wikibase\Summary;
use Wikibase\Utils;
@@ -44,29 +42,27 @@
}
/**
- * @see \Wikibase\Api\ModifyEntity::getRequiredPermissions()
+ * @see ApiWikibase::getRequiredPermissions
*
- * @param Entity $entity
- * @param array $params
+ * @param EntityId|null $entityId
*
- * @throws \InvalidArgumentException
- * @return array|\Status
+ * @throws InvalidArgumentException
+ * @return string[]
*/
- protected function getRequiredPermissions( Entity $entity, array
$params ) {
- $permissions = parent::getRequiredPermissions( $entity, $params
);
- if( $entity instanceof Item ) {
- $type = 'item';
- } else if ( $entity instanceof Property ) {
- $type = 'property';
- } else {
- throw new InvalidArgumentException( 'Unexpected Entity
type when checking special page term change permissions' );
+ protected function getRequiredPermissions( EntityId $entityId = null ) {
+ if ( $entityId === null ) {
+ throw new InvalidArgumentException( 'Can not modify
terms of entity with no id' );
}
- $permissions[] = $type . '-term';
+
+ $permissions = parent::getRequiredPermissions( $entityId );
+ $permissions[] = $entityId->getEntityType() . '-term';
return $permissions;
}
/**
- * @see \ApiBase::getAllowedParams()
+ * @see ApiWikibase::getAllowedParams
+ *
+ * @return array
*/
public function getAllowedParams() {
return array_merge(
@@ -85,4 +81,5 @@
)
);
}
+
}
diff --git a/repo/includes/api/SetAliases.php b/repo/includes/api/SetAliases.php
index 3ed903c..3262f93 100644
--- a/repo/includes/api/SetAliases.php
+++ b/repo/includes/api/SetAliases.php
@@ -10,8 +10,7 @@
use Wikibase\ChangeOp\ChangeOps;
use Wikibase\ChangeOp\FingerprintChangeOpFactory;
use Wikibase\DataModel\Entity\Entity;
-use Wikibase\DataModel\Entity\Item;
-use Wikibase\DataModel\Entity\Property;
+use Wikibase\DataModel\Entity\EntityId;
use Wikibase\Repo\WikibaseRepo;
use Wikibase\Utils;
@@ -32,7 +31,7 @@
/**
* @var FingerprintChangeOpFactory
*/
- protected $termChangeOpFactory;
+ private $termChangeOpFactory;
/**
* @param ApiMain $mainModule
@@ -47,29 +46,25 @@
}
/**
- * @see \Wikibase\Api\ModifyEntity::getRequiredPermissions()
+ * @see ApiWikibase::getRequiredPermissions
*
- * @param Entity $entity
- * @param array $params
+ * @param EntityId|null $entityId
*
- * @throws \InvalidArgumentException
- * @return array|\Status
+ * @throws InvalidArgumentException
+ * @return string[]
*/
- protected function getRequiredPermissions( Entity $entity, array
$params ) {
- $permissions = parent::getRequiredPermissions( $entity, $params
);
- if( $entity instanceof Item ) {
- $type = 'item';
- } else if ( $entity instanceof Property ) {
- $type = 'property';
- } else {
- throw new InvalidArgumentException( 'Unexpected Entity
type when checking special page term change permissions' );
+ protected function getRequiredPermissions( EntityId $entityId = null ) {
+ if ( $entityId === null ) {
+ throw new InvalidArgumentException( 'Can not set
aliases of entity with no id' );
}
- $permissions[] = $type . '-term';
+
+ $permissions = parent::getRequiredPermissions( $entityId );
+ $permissions[] = $entityId->getEntityType() . '-term';
return $permissions;
}
/**
- * @see \Wikibase\Api\ModifyEntity::validateParameters()
+ * @see ModifyEntity::validateParameters
*/
protected function validateParameters( array $params ) {
parent::validateParameters( $params );
@@ -80,14 +75,14 @@
}
/**
- * @see ApiModifyEntity::createEntity()
+ * @see ModifyEntity::createEntity
*/
protected function createEntity( array $params ) {
- $this->dieError( 'Could not find an existing entity' ,
'no-such-entity' );
+ $this->dieError( 'Could not find an existing entity',
'no-such-entity' );
}
/**
- * @see \Wikibase\Api\ModifyEntity::modifyEntity()
+ * @see ModifyEntity::modifyEntity
*/
protected function modifyEntity( Entity &$entity, array $params,
$baseRevId ) {
wfProfileIn( __METHOD__ );
@@ -123,7 +118,12 @@
return $summary;
}
- private function normalizeAliases( $aliases ) {
+ /**
+ * @param string[] $aliases
+ *
+ * @return string[]
+ */
+ private function normalizeAliases( array $aliases ) {
$stringNormalizer = $this->stringNormalizer; // hack for PHP
fail.
$aliases = array_map(
@@ -144,12 +144,11 @@
}
/**
- * @since 0.4
- *
* @param array $params
+ *
* @return ChangeOpAliases
*/
- protected function getChangeOps( array $params ) {
+ private function getChangeOps( array $params ) {
wfProfileIn( __METHOD__ );
$changeOps = array();
$language = $params['language'];
@@ -187,7 +186,7 @@
}
/**
- * @see ApiBase::getAllowedParams()
+ * @see ApiBase::getAllowedParams
*/
public function getAllowedParams() {
return array_merge(
@@ -217,7 +216,7 @@
}
/**
- * @see ApiBase::getExamplesMessages()
+ * @see ApiBase::getExamplesMessages
*
* @return array
*/
diff --git a/repo/includes/api/SetSiteLink.php
b/repo/includes/api/SetSiteLink.php
index d34d114..e595ca0 100644
--- a/repo/includes/api/SetSiteLink.php
+++ b/repo/includes/api/SetSiteLink.php
@@ -74,7 +74,7 @@
}
/**
- * @see ApiModifyEntity::modifyEntity()
+ * @see ModifyEntity::modifyEntity
*/
protected function modifyEntity( Entity &$entity, array $params,
$baseRevId ) {
wfProfileIn( __METHOD__ );
@@ -188,7 +188,7 @@
}
/**
- * @see ApiBase::getExamplesMessages()
+ * @see ApiBase::getExamplesMessages
*
* @return array
*/
--
To view, visit https://gerrit.wikimedia.org/r/179881
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic506adc1145d04e3f4af8f0aa5d4b65ea0d4de28
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
