Daniel Kinzler has submitted this change and it was merged.
Change subject: Split up switch-case into separate methods
......................................................................
Split up switch-case into separate methods
- part of EditEntity API refactoring
- get rid of huge switch-case and split it into methods
- remove the EXCLUDE parameter from the EditEntity API
- move aliases operations into seperate ChangeOpAliases class
- move sitelink operations into separate ChangeOpSiteLink class
Bug: 48137
Change-Id: Ic27774f67f8728acbb48cbf945fda063791096cf
---
M repo/Wikibase.classes.php
M repo/Wikibase.hooks.php
M repo/includes/api/EditEntity.php
A repo/includes/changeop/ChangeOpAliases.php
M repo/includes/changeop/ChangeOpDescription.php
M repo/includes/changeop/ChangeOpLabel.php
A repo/includes/changeop/ChangeOpSiteLink.php
M repo/includes/changeop/ChangeOps.php
M repo/tests/phpunit/includes/api/EditEntityTest.php
A repo/tests/phpunit/includes/changeop/ChangeOpAliasesTest.php
A repo/tests/phpunit/includes/changeop/ChangeOpSiteLinkTest.php
M repo/tests/phpunit/includes/changeop/ChangeOpsTest.php
12 files changed, 769 insertions(+), 292 deletions(-)
Approvals:
Daniel Kinzler: Verified; Looks good to me, approved
diff --git a/repo/Wikibase.classes.php b/repo/Wikibase.classes.php
index 1fa252b..6eb7d60 100644
--- a/repo/Wikibase.classes.php
+++ b/repo/Wikibase.classes.php
@@ -49,10 +49,14 @@
'Wikibase\PropertyView' => 'includes/PropertyView.php',
'Wikibase\Summary' => 'includes/Summary.php',
'Wikibase\Repo\WikibaseRepo' => 'includes/WikibaseRepo.php',
+
+ // includes/changeop
'Wikibase\ChangeOps' => 'includes/changeop/ChangeOps.php',
'Wikibase\ChangeOp' => 'includes/changeop/ChangeOp.php',
'Wikibase\ChangeOpLabel' =>
'includes/changeop/ChangeOpLabel.php',
'Wikibase\ChangeOpDescription' =>
'includes/changeop/ChangeOpDescription.php',
+ 'Wikibase\ChangeOpAliases' =>
'includes/changeop/ChangeOpAliases.php',
+ 'Wikibase\ChangeOpSiteLink' =>
'includes/changeop/ChangeOpSiteLink.php',
// includes/actions
'Wikibase\HistoryEntityAction' =>
'includes/actions/HistoryEntityAction.php',
diff --git a/repo/Wikibase.hooks.php b/repo/Wikibase.hooks.php
index 359bfcb..ebd10b3 100755
--- a/repo/Wikibase.hooks.php
+++ b/repo/Wikibase.hooks.php
@@ -190,6 +190,8 @@
'changeop/ChangeOps',
'changeop/ChangeOpLabel',
'changeop/ChangeOpDescription',
+ 'changeop/ChangeOpAliases',
+ 'changeop/ChangeOpSiteLink',
'content/EntityContentFactory',
'content/EntityHandler',
diff --git a/repo/includes/api/EditEntity.php b/repo/includes/api/EditEntity.php
index 65049e6..466cb98 100644
--- a/repo/includes/api/EditEntity.php
+++ b/repo/includes/api/EditEntity.php
@@ -3,9 +3,10 @@
namespace Wikibase\Api;
use Wikibase\ChangeOps;
-
use Wikibase\ChangeOpLabel;
use Wikibase\ChangeOpDescription;
+use Wikibase\ChangeOpAliases;
+use Wikibase\ChangeOpSiteLink;
use ApiBase, User, Status, SiteList;
use Wikibase\SiteLink;
use Wikibase\Entity;
@@ -31,7 +32,23 @@
class EditEntity extends ModifyEntity {
/**
- * @see \Wikibase\Api\Api::getRequiredPermissions()
+ * @since 0.4
+ *
+ * @var string[]
+ */
+ protected $validLanguageCodes;
+
+ /**
+ * @see ApiBase::_construct()
+ */
+ public function __construct( $mainModule, $moduleName ) {
+ parent::__construct( $mainModule, $moduleName );
+
+ $this->validLanguageCodes = array_flip(
Utils::getLanguageCodes() );
+ }
+
+ /**
+ * @see \Wikibase\Api\Api::getRequiredPermissions()
*/
protected function getRequiredPermissions( Entity $entity, array
$params ) {
$permissions = parent::getRequiredPermissions( $entity, $params
);
@@ -43,7 +60,7 @@
}
/**
- * @see \Wikibase\Api\ModifyEntity::findEntity()
+ * @see \Wikibase\Api\ModifyEntity::findEntity()
*/
protected function findEntity( array $params ) {
$entityContent = parent::findEntity( $params );
@@ -96,10 +113,10 @@
*/
protected function modifyEntity( EntityContent &$entityContent, array
$params ) {
wfProfileIn( __METHOD__ );
- $status = Status::newGood();
$summary = $this->createSummary( $params );
$entity = $entityContent->getEntity();
$changeOps = new ChangeOps();
+ $status = Status::newGood();
if ( isset( $params['id'] ) XOR ( isset( $params['site'] ) &&
isset( $params['title'] ) ) ) {
$summary->setAction( $params['clear'] === false ?
'update' : 'override' );
@@ -111,246 +128,48 @@
//TODO: Construct a nice and meaningful summary from the
changes that get applied!
// Perhaps that could be based on the resulting diff?
- if ( isset( $params['data'] ) ) {
- $data = json_decode( $params['data'], true );
- if ( is_null( $data ) ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( $this->msg(
'wikibase-api-json-invalid' )->text(), 'json-invalid' );
- }
- if ( !is_array( $data ) ) { // NOTE: json_decode will
decode any JS literal or structure, not just objects!
- wfProfileOut( __METHOD__ );
- $this->dieUsage( 'Top level structure must be a
JSON object', 'not-recognized-array' );
- }
- $languages = array_flip( Utils::getLanguageCodes() );
-
- if ( isset( $params['clear'] ) && $params['clear'] ) {
- $entityContent->getEntity()->clear();
- }
-
- $title = null;
- $revision = null;
- $page = $entityContent->getWikiPage();
- if ( $page ) {
- $title = $page->getTitle();
- $revision = $page->getRevision();
- }
-
- foreach ( $data as $props => $list ) {
- if ( !is_string( $props ) ) { // NOTE: catch
json_decode returning an indexed array (list)
- $this->dieUsage( 'Top level structure
must be a JSON object', 'not-recognized-string' );
- }
- // unconditional no-ops
- if ( in_array( $props, array( 'length',
'count', 'touched' ) ) ) {
- continue;
- }
- // conditional no-ops
- if ( isset( $params['exclude'] ) && in_array(
$props, $params['exclude'] ) ) {
- continue;
- }
-
- switch ($props) {
-
- // conditional processing
- case 'pageid':
- if ( isset( $data[$props] ) && (
is_object( $page ) ? $page->getId() !== $data[$props] : true ) ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( $this->msg(
'wikibase-api-illegal-field', 'pageid' )->text(), 'illegal-field' );
- }
- break;
- case 'ns':
- // not completely convinced that we can
use title to get the namespace in this case
- if ( isset( $data[$props] ) && (
is_object( $title ) ? $title->getNamespace() !== $data[$props] : true ) ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( $this->msg(
'wikibase-api-illegal-field', 'namespace' )->text(), 'illegal-field' );
- }
- break;
- case 'title':
- if ( isset( $data[$props] ) && (
is_object( $title ) ? $title->getPrefixedText() !== $data[$props] : true ) ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( $this->msg(
'wikibase-api-illegal-field', 'title' )->text(), 'illegal-field' );
- }
- break;
- case 'lastrevid':
- if ( isset( $data[$props] ) && (
is_object( $revision ) ? $revision->getId() !== $data[$props] : true ) ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( $this->msg(
'wikibase-api-illegal-field', 'lastrevid' )->text(), 'illegal-field' );
- }
- break;
-
- // ordinary entries
- case 'labels':
- if ( !is_array( $list ) ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( "Key 'labels'
must refer to an array", 'not-recognized-array' );
- }
-
- foreach ( $list as $langCode => $arg ) {
- $status->merge(
$this->checkMultilangArgs( $arg, $langCode, $languages ) );
-
- $language = $arg['language'];
- $newLabel = Utils::trimToNFC(
$arg['value'] );
- $oldLabel = $entity->getLabel(
$language );
-
- if ( array_key_exists(
'remove', $arg ) || $newLabel === "" ) {
- $changeOps->add( new
ChangeOpLabel( $language, null ) );
- }
- else {
- $changeOps->add( new
ChangeOpLabel( $language, $newLabel ) );
- }
- }
-
- if ( !$status->isOk() ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( "Contained
status: $1", $status->getWikiText() );
- }
-
- break;
-
- case 'descriptions':
- if ( !is_array( $list ) ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( "Key
'descriptions' must refer to an array", 'not-recognized-array' );
- }
-
- foreach ( $list as $langCode => $arg ) {
- $status->merge(
$this->checkMultilangArgs( $arg, $langCode, $languages ) );
-
- $language = $arg['language'];
- $newDescription =
Utils::trimToNFC( $arg['value'] );
- $oldDescription =
$entity->getDescription( $language );
-
- if ( array_key_exists(
'remove', $arg ) || $newDescription === "" ) {
- $changeOps->add( new
ChangeOpDescription( $language, null ) );
- }
- else {
- $changeOps->add( new
ChangeOpDescription( $language, $newDescription ) );
- }
- }
-
- if ( !$status->isOk() ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( "Contained
status: $1", $status->getWikiText() );
- }
-
- break;
-
- case 'aliases':
- if ( !is_array( $list ) ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( "Key 'aliases'
must refer to an array", 'not-recognized-array' );
- }
-
- $aliases = array();
-
- foreach ( $list as $langCode => $arg ) {
- if ( intval( $langCode ) ) {
- $aliases[] = (
array_values($arg) === $arg ) ? $arg : array( $arg );
- } else {
- $aliases[$langCode] = (
array_values($arg) === $arg ) ? $arg : array( $arg );
- }
- }
-
- $setAliases = array();
- $addAliases = array();
- $remAliases = array();
-
- foreach ( $aliases as $langCode =>
$args ) {
- foreach ( $args as $arg ) {
- $status->merge(
$this->checkMultilangArgs( $arg, $langCode, $languages ) );
- if ( array_key_exists(
'remove', $arg ) ) {
-
$remAliases[$arg['language']][] = Utils::trimToNFC( $arg['value'] );
- }
- elseif (
array_key_exists( 'add', $arg ) ) {
-
$addAliases[$arg['language']][] = Utils::trimToNFC( $arg['value'] );
- }
- else {
-
$setAliases[$arg['language']][] = Utils::trimToNFC( $arg['value'] );
- }
- }
- }
- foreach ( $setAliases as $langCode =>
$strings ) {
-
$entityContent->getEntity()->setAliases( $langCode, $strings );
- }
- foreach ( $remAliases as $langCode =>
$strings ) {
-
$entityContent->getEntity()->removeAliases( $langCode, $strings );
- }
- foreach ( $addAliases as $langCode =>
$strings ) {
-
$entityContent->getEntity()->addAliases( $langCode, $strings );
- }
-
- if ( !$status->isOk() ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( "Contained
status: $1", $status->getWikiText() );
- }
-
- unset( $aliases );
- unset( $setAliases );
- unset( $addAliases );
- unset( $remAliases );
-
- break;
-
- case 'sitelinks':
- // TODO: This is a temporary fix that
should be handled properly with an
- // injector/inputter class that is
specific for the given entity
- if (
$entityContent->getEntity()->getType() !== Item::ENTITY_TYPE ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( "key can't be
handled: $props", 'not-recognized' );
- }
-
- if ( !is_array( $list ) ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( "Key
'sitelinks' must refer to an array", 'not-recognized-array' );
- }
-
- $sites =
$this->getSiteLinkTargetSites();
-
- foreach ( $list as $siteId => $arg ) {
- $status->merge(
$this->checkSiteLinks( $arg, $siteId, $sites ) );
- if ( array_key_exists(
'remove', $arg ) || $arg['title'] === "" ) {
-
$entityContent->getEntity()->removeSiteLink( $arg['site'] );
- }
- else {
- $linkSite =
$sites->getSite( $arg['site'] );
- $linkPage =
$linkSite->normalizePageName( Utils::trimWhitespace( $arg['title'] ) );
-
- if ( $linkPage ===
false ) {
- wfProfileOut(
__METHOD__ );
-
$this->dieUsage( $this->msg( 'wikibase-api-no-external-page' )->text(),
'add-sitelink-failed' );
- }
-
- $link = new SiteLink(
$linkSite, $linkPage );
- $ret =
$entityContent->getEntity()->addSiteLink( $link, 'set' );
-
- if ( $ret === false ) {
- wfProfileOut(
__METHOD__ );
-
$this->dieUsage( $this->msg( 'wikibase-api-add-sitelink-failed' )->text(),
'add-sitelink-failed' );
- }
-
- unset( $linkSite );
- unset( $linkPage );
- unset( $link );
- unset( $ret );
- }
- }
-
- if ( !$status->isOk() ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( "Contained
status: $1", $status->getWikiText() );
- }
-
- unset( $sites );
-
- break;
-
- default:
- wfProfileOut( __METHOD__ );
- $this->dieUsage( "unknown key: $props",
'not-recognized' );
- }
- }
+ if ( !isset( $params['data'] ) ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( $this->msg( 'wikibase-api-no-data'
)->text(), 'no-data' );
}
- $changeOps->apply( $entity );
+ $data = json_decode( $params['data'], true );
+ $status->merge( $this->checkDataProperties( $data,
$entityContent->getWikiPage() ) );
+
+ if ( $params['clear'] ) {
+ $entityContent->getEntity()->clear();
+ }
+
+ if ( array_key_exists( 'labels', $data ) ) {
+ $changeOps->add( $this->getLabelChangeOps(
$data['labels'], $status ) );
+ }
+
+ if ( array_key_exists( 'descriptions', $data ) ) {
+ $changeOps->add( $this->getDescriptionChangeOps(
$data['descriptions'], $status ) );
+ }
+
+ if ( array_key_exists( 'aliases', $data ) ) {
+ $changeOps->add( $this->getAliasesChangeOps(
$data['aliases'], $status ) );
+ }
+
+ if ( array_key_exists( 'sitelinks', $data ) ) {
+ if ( $entity->getType() !== Item::ENTITY_TYPE ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( "key can't be handled:
$props", 'not-recognized' );
+ }
+
+ $changeOps->add( $this->getSiteLinksChangeOps(
$data['sitelinks'], $status ) );
+ }
+
+ if ( !$status->isOk() ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( "Contained status: $1",
$status->getWikiText() );
+ }
+
+ if ( $changeOps->apply( $entity ) === false ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( 'Change could not be applied to
entity', 'edit-entity-apply-failed' );
+ }
// This is already done in createEntity
if ( $entityContent->isNew() ) {
@@ -371,6 +190,240 @@
wfProfileOut( __METHOD__ );
return $summary;
+ }
+
+ /**
+ * @since 0.4
+ *
+ * @param array $labels
+ *
+ * @return ChangeOpLabel[]
+ */
+ protected function getLabelChangeOps( $labels, $status ) {
+ $labelChangeOps = array();
+
+ if ( !is_array( $labels ) ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( "List of labels must be an array",
'not-recognized-array' );
+ }
+
+ foreach ( $labels as $langCode => $arg ) {
+ $status->merge( $this->checkMultilangArgs( $arg,
$langCode ) );
+
+ $language = $arg['language'];
+ $newLabel = Utils::trimToNFC( $arg['value'] );
+
+ if ( array_key_exists( 'remove', $arg ) || $newLabel
=== "" ) {
+ $labelChangeOps[] = new ChangeOpLabel(
$language, null );
+ }
+ else {
+ $labelChangeOps[] = new ChangeOpLabel(
$language, $newLabel );
+ }
+ }
+
+ return $labelChangeOps;
+ }
+
+ /**
+ * @since 0.4
+ *
+ * @param array $descriptions
+ *
+ * @return ChangeOpdescription[]
+ */
+ protected function getDescriptionChangeOps( $descriptions, $status ) {
+ $descriptionChangeOps = array();
+
+ if ( !is_array( $descriptions ) ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( "List of descriptions must be an
array", 'not-recognized-array' );
+ }
+
+ foreach ( $descriptions as $langCode => $arg ) {
+ $status->merge( $this->checkMultilangArgs( $arg,
$langCode ) );
+
+ $language = $arg['language'];
+ $newDescription = Utils::trimToNFC( $arg['value'] );
+
+ if ( array_key_exists( 'remove', $arg ) ||
$newDescription === "" ) {
+ $descriptionChangeOps[] = new
ChangeOpDescription( $language, null );
+ }
+ else {
+ $descriptionChangeOps[] = new
ChangeOpDescription( $language, $newDescription );
+ }
+ }
+
+ return $descriptionChangeOps;
+ }
+
+ /**
+ * @since 0.4
+ *
+ * @param array $aliases
+ *
+ * @return ChangeOpAliases[]
+ */
+ protected function getAliasesChangeOps( $aliases, $status ) {
+ $aliasesChangeOps = array();
+
+ if ( !is_array( $aliases ) ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( "List of aliases must be an array",
'not-recognized-array' );
+ }
+
+ $indexedAliases = array();
+
+ foreach ( $aliases as $langCode => $arg ) {
+ if ( intval( $langCode ) ) {
+ $indexedAliases[] = ( array_values($arg) ===
$arg ) ? $arg : array( $arg );
+ } else {
+ $indexedAliases[$langCode] = (
array_values($arg) === $arg ) ? $arg : array( $arg );
+ }
+ }
+
+ foreach ( $indexedAliases as $langCode => $args ) {
+ foreach ( $args as $arg ) {
+ $status->merge( $this->checkMultilangArgs(
$arg, $langCode ) );
+
+ $alias = array( Utils::trimToNFC( $arg['value']
) );
+ $language = $arg['language'];
+
+ if ( array_key_exists( 'remove', $arg ) ) {
+ $aliasesChangeOps[] = new
ChangeOpAliases( $language, $alias, 'remove' );
+ }
+ elseif ( array_key_exists( 'add', $arg ) ) {
+ $aliasesChangeOps[] = new
ChangeOpAliases( $language, $alias, 'add' );
+ }
+ else {
+ $aliasesChangeOps[] = new
ChangeOpAliases( $language, $alias, 'set' );
+ }
+ }
+ }
+
+ if ( !$status->isOk() ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( "Contained status: $1",
$status->getWikiText() );
+ }
+
+ return $aliasesChangeOps;
+ }
+
+ /**
+ * @since 0.4
+ *
+ * @param array $siteLinks
+ *
+ * @return ChangeOpSiteLink[]
+ */
+ protected function getSitelinksChangeOps( $siteLinks, $status ) {
+ $siteLinksChangeOps = array();
+
+ if ( !is_array( $siteLinks ) ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( "List of sitelinks must be an array",
'not-recognized-array' );
+ }
+
+ $sites = $this->getSiteLinkTargetSites();
+
+ foreach ( $siteLinks as $siteId => $arg ) {
+ $status->merge( $this->checkSiteLinks( $arg, $siteId,
$sites ) );
+ $globalSiteId = $arg['site'];
+
+ if ( $sites->hasSite( $globalSiteId ) ) {
+ $linkSite = $sites->getSite( $globalSiteId );
+ } else {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( "There is no site for global
site id '$globalSiteId'", 'site-not-found' );
+ }
+
+ if ( array_key_exists( 'remove', $arg ) ||
$arg['title'] === "" ) {
+ $siteLinksChangeOps[] = new ChangeOpSiteLink(
$linkSite, null );
+ } else {
+ $linkPage = $linkSite->normalizePageName(
Utils::trimWhitespace( $arg['title'] ) );
+
+ if ( $linkPage === false ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( $this->msg(
'wikibase-api-no-external-page' )->text(), 'add-sitelink-failed' );
+ }
+
+ $siteLinksChangeOps[] = new ChangeOpSiteLink(
$linkSite, $linkPage );
+ }
+ }
+
+ return $siteLinksChangeOps;
+ }
+
+ /**
+ * @since 0.4
+ *
+ * @param array $data
+ * @param WikiPage|bool $page
+ *
+ * @return Status
+ */
+ protected function checkDataProperties( $data, $page ) {
+ $status = Status::newGood();
+
+ $title = null;
+ $revision = null;
+ if ( $page ) {
+ $title = $page->getTitle();
+ $revision = $page->getRevision();
+ }
+
+ $allowedProps = array(
+ 'length',
+ 'count',
+ 'touched',
+ 'pageid',
+ 'ns',
+ 'title',
+ 'lastrevid',
+ 'labels',
+ 'descriptions',
+ 'aliases',
+ 'sitelinks' );
+
+ if ( is_null( $data ) ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( $this->msg(
'wikibase-api-json-invalid' )->text(), 'json-invalid' );
+ }
+
+ if ( !is_array( $data ) ) { // NOTE: json_decode will decode
any JS literal or structure, not just objects!
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( 'Top level structure must be a JSON
object', 'not-recognized-array' );
+ }
+
+ foreach ( $data as $prop => $args ) {
+ if ( !is_string( $prop ) ) { // NOTE: catch json_decode
returning an indexed array (list)
+ $this->dieUsage( 'Top level structure must be a
JSON object', 'not-recognized-string' );
+ }
+
+ if ( !in_array( $prop, $allowedProps ) ) {
+ $this->dieUsage( "unknown key: $prop",
'not-recognized' );
+ }
+ }
+
+ // conditional processing
+ if ( isset( $data['pageid'] ) && ( is_object( $page ) ?
$page->getId() !== $data['pageid'] : true ) ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( $this->msg(
'wikibase-api-illegal-field', 'pageid' )->text(), 'illegal-field' );
+ }
+ // 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 ) ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( $this->msg(
'wikibase-api-illegal-field', 'namespace' )->text(), 'illegal-field' );
+ }
+ if ( isset( $data['title'] ) && ( is_object( $title ) ?
$title->getPrefixedText() !== $data['title'] : true ) ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( $this->msg(
'wikibase-api-illegal-field', 'title' )->text(), 'illegal-field' );
+ }
+ if ( isset( $data['lastrevid'] ) && ( is_object( $revision ) ?
$revision->getId() !== $data['lastrevid'] : true ) ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( $this->msg(
'wikibase-api-illegal-field', 'lastrevid' )->text(), 'illegal-field' );
+ }
+
+ return $status;
}
/**
@@ -469,11 +522,10 @@
*
* @param $arg Array: The argument array to verify
* @param $langCode string: The language code used in the value part
- * @param &$languages array: The valid language codes as an assoc array
*
* @return Status: The result from the comparison (always true)
*/
- public function checkMultilangArgs( $arg, $langCode, &$languages = null
) {
+ public function checkMultilangArgs( $arg, $langCode ) {
$status = Status::newGood();
if ( !is_array( $arg ) ) {
$this->dieUsage( $this->msg(
'wikibase-api-not-recognized-array' )->text(), 'not-recognized-array' );
@@ -486,7 +538,7 @@
$this->dieUsage( "inconsistent language:
{$langCode} is not equal to {$arg['language']}", 'inconsistent-language' );
}
}
- if ( isset( $languages ) && !array_key_exists(
$arg['language'], $languages ) ) {
+ if ( isset( $this->validLanguageCodes ) && !array_key_exists(
$arg['language'], $this->validLanguageCodes ) ) {
$this->dieUsage( "unknown language:
{$arg['language']}", 'not-recognized-language' );
}
if ( !is_string( $arg['value'] ) ) {
diff --git a/repo/includes/changeop/ChangeOpAliases.php
b/repo/includes/changeop/ChangeOpAliases.php
new file mode 100644
index 0000000..8333f80
--- /dev/null
+++ b/repo/includes/changeop/ChangeOpAliases.php
@@ -0,0 +1,103 @@
+<?php
+
+namespace Wikibase;
+
+use InvalidArgumentException;
+use MWExceptiontion;
+
+/**
+ * Class for aliases change operation
+ *
+ * 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
+ *
+ * @licence GNU GPL v2+
+ * @author Tobias Gritschacher < [email protected] >
+ */
+class ChangeOpAliases implements ChangeOp {
+
+ /**
+ * @since 0.4
+ *
+ * @var string
+ */
+ protected $language;
+
+ /**
+ * @since 0.4
+ *
+ * @var string[]
+ */
+ protected $aliases;
+
+ /**
+ * @since 0.4
+ *
+ * @var array
+ */
+ protected $action;
+
+ /**
+ * @since 0.4
+ *
+ * @param string $language
+ * @param string[] $aliases
+ * @param string $action
+ *
+ * @throws InvalidArgumentException
+ */
+ public function __construct( $language, array $aliases, $action ) {
+ if ( !is_string( $language ) ) {
+ throw new InvalidArgumentException( '$language needs to
be a string' );
+ }
+
+ if ( !is_string( $action ) ) {
+ throw new InvalidArgumentException( '$action needs to
be a string' );
+ }
+
+ $this->language = $language;
+ $this->aliases = $aliases;
+ $this->action = $action;
+ }
+
+ /**
+ * Applies the change to the given entity
+ *
+ * @since 0.4
+ *
+ * @param Entity $entity
+ *
+ * @return bool
+ *
+ * @throws MWException
+ */
+ public function apply( Entity $entity ) {
+ if ( $this->action === "" || $this->action === "set" ) {
+ $entity->setAliases( $this->language, $this->aliases );
+ } elseif ( $this->action === "add" ) {
+ $entity->addAliases( $this->language, $this->aliases );
+ } elseif ( $this->action === "remove" ) {
+ $entity->removeAliases( $this->language, $this->aliases
);
+ } else {
+ throw new \MWException( "Unknown action: $this->action"
);
+ }
+ return true;
+ }
+
+}
diff --git a/repo/includes/changeop/ChangeOpDescription.php
b/repo/includes/changeop/ChangeOpDescription.php
index e02ad9c..fe86075 100644
--- a/repo/includes/changeop/ChangeOpDescription.php
+++ b/repo/includes/changeop/ChangeOpDescription.php
@@ -68,6 +68,8 @@
* @since 0.4
*
* @param Entity $entity
+ *
+ * @return bool
*/
public function apply( Entity $entity ) {
if ( $this->description === null ) {
@@ -75,6 +77,7 @@
} else {
$entity->setDescription( $this->language,
$this->description );
}
+ return true;
}
}
diff --git a/repo/includes/changeop/ChangeOpLabel.php
b/repo/includes/changeop/ChangeOpLabel.php
index 200d955..0a19a44 100644
--- a/repo/includes/changeop/ChangeOpLabel.php
+++ b/repo/includes/changeop/ChangeOpLabel.php
@@ -68,6 +68,8 @@
* @since 0.4
*
* @param Entity $entity
+ *
+ * @return bool
*/
public function apply( Entity $entity ) {
if ( $this->label === null ) {
@@ -75,6 +77,7 @@
} else {
$entity->setLabel( $this->language, $this->label );
}
+ return true;
}
}
diff --git a/repo/includes/changeop/ChangeOpSiteLink.php
b/repo/includes/changeop/ChangeOpSiteLink.php
new file mode 100644
index 0000000..b6dbe9b
--- /dev/null
+++ b/repo/includes/changeop/ChangeOpSiteLink.php
@@ -0,0 +1,85 @@
+<?php
+
+namespace Wikibase;
+
+use InvalidArgumentException;
+use Site;
+
+/**
+ * Class for sitelink change operation
+ *
+ * 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
+ *
+ * @licence GNU GPL v2+
+ * @author Tobias Gritschacher < [email protected] >
+ */
+class ChangeOpSiteLink implements ChangeOp {
+
+ /**
+ * @since 0.4
+ *
+ * @var Site
+ */
+ protected $linkSite;
+
+ /**
+ * @since 0.4
+ *
+ * @var string|null
+ */
+ protected $linkPage;
+
+ /**
+ * @since 0.4
+ *
+ * @param Site $linkSite
+ * @param string|null $linkPage
+ *
+ * @throws InvalidArgumentException
+ */
+ public function __construct( Site $linkSite, $linkPage ) {
+ if ( !is_string( $linkPage ) && $linkPage !==null ) {
+ throw new InvalidArgumentException( '$linkPage needs to
be a string|null' );
+ }
+
+ $this->linkSite = $linkSite;
+ $this->linkPage = $linkPage;
+ }
+
+ /**
+ * Applies the change to the given entity
+ *
+ * @since 0.4
+ *
+ * @param Entity $entity
+ *
+ * @return SiteLink|bool
+ */
+ public function apply( Entity $entity ) {
+ if ( $this->linkPage === null ) {
+ $entity->removeSiteLink( $this->linkSite->getGlobalId()
);
+ return true; // don't show an error when the sitelink
we try to remove does not exist
+ } else {
+ $siteLink = new SiteLink( $this->linkSite,
$this->linkPage );
+ return $entity->addSiteLink( $siteLink,'set' );
+ }
+ }
+
+}
diff --git a/repo/includes/changeop/ChangeOps.php
b/repo/includes/changeop/ChangeOps.php
index 47c1591..68091b9 100644
--- a/repo/includes/changeop/ChangeOps.php
+++ b/repo/includes/changeop/ChangeOps.php
@@ -2,6 +2,8 @@
namespace Wikibase;
+use InvalidArgumentException;
+
/**
* Class for holding a batch of change operations
*
@@ -49,10 +51,26 @@
*
* @since 0.4
*
- * @param ChangeOp $changeOp
+ * @param ChangeOp|ChangeOp[] $changeOp
+ *
+ * @throws InvalidArgumentException
*/
- public function add( ChangeOp $changeOp ) {
- $this->ops[] = $changeOp;
+ public function add( $changeOp ) {
+ if ( !is_array( $changeOp ) && !( $changeOp instanceof ChangeOp
) ) {
+ throw new InvalidArgumentException( '$changeOp needs to
be an instance of ChangeOp or an array of ChangeOps' );
+ }
+
+ if ( $changeOp instanceof ChangeOp ) {
+ $this->ops[] = $changeOp;
+ } else {
+ foreach ( $changeOp as $op ) {
+ if ( $op instanceof ChangeOp ) {
+ $this->ops[] = $op;
+ } else {
+ throw new InvalidArgumentException(
'array $changeOp must contain ChangeOps only' );
+ }
+ }
+ }
}
/**
@@ -71,11 +89,17 @@
* @since 0.4
*
* @param Entity $entity
+ *
+ * @return bool
*/
public function apply( Entity $entity ) {
foreach ( $this->ops as $op ) {
- $op->apply( $entity );
+ if ( $op->apply( $entity ) === false ) {
+ return false;
+ }
}
+
+ return true;
}
}
diff --git a/repo/tests/phpunit/includes/api/EditEntityTest.php
b/repo/tests/phpunit/includes/api/EditEntityTest.php
index 03cfd53..a03609c 100644
--- a/repo/tests/phpunit/includes/api/EditEntityTest.php
+++ b/repo/tests/phpunit/includes/api/EditEntityTest.php
@@ -212,46 +212,6 @@
}
/**
- * Check success when the entity is set again, with fields in the json
that should be ignored
- */
- function testEditEntityWithIgnoredData() {
- $token = $this->getItemToken();
-
- // these sets of failing data must be merged with an existing
entity
- $ignoredData = array(
- array( 'length' => 999999 ), // always ignored
- array( 'count' => 999999 ), // always ignored
- array( 'touched' => '2000-01-01T18:05:01Z' ), // always
ignored
- array( 'pageid' => 999999 ),
- array( 'ns' => 200 ),
- array( 'title' => 'does-not-exist' ),
- array( 'lastrevid' => 99999999 ),
- );
- foreach ( $ignoredData as $data ) {
- try {
- list($res,,) = $this->doApiRequest(
- array(
- 'action' => 'wbeditentity',
- 'reason' => 'Some reason',
- 'data' => json_encode(
array_merge( self::$entity, $data ) ),
- 'token' => $token,
- 'id' => self::$id, // will now
use default type
- 'exclude' =>
'pageid|ns|title|lastrevid'
- ),
- null,
- false,
- self::$users['wbeditor']->user
- );
- $this->assertSuccess( $res, 'entity', 'id' );
- $this->assertItemEquals( self::$expect,
$res['entity'] );
- }
- catch ( \UsageException $e ) {
- $this->fail( "Got unexpected exception: $e" );
- }
- }
- }
-
- /**
* Check failure to set the same entity again, with illegal field
values in the json
*/
function testEditEntityWithIllegalData() {
@@ -273,7 +233,6 @@
'data' => json_encode(
array_merge( self::$entity, $data ) ),
'token' => $token,
'id' => self::$id,
- //'exclude' => '' // make sure
all critical values are checked per default
),
null,
false,
@@ -324,7 +283,6 @@
'data' => json_encode(
array_merge( $data, self::$entity ) ),
'token' => $token,
'id' => self::$id,
- //'exclude' => '' // make sure
all critical values are checked per default
),
null,
false,
@@ -354,7 +312,6 @@
'token' => $token,
'id' => self::$id,
'clear' => true,
- //'exclude' => '' // make sure all
critical values are checked per default
),
null,
false,
diff --git a/repo/tests/phpunit/includes/changeop/ChangeOpAliasesTest.php
b/repo/tests/phpunit/includes/changeop/ChangeOpAliasesTest.php
new file mode 100644
index 0000000..bef4658
--- /dev/null
+++ b/repo/tests/phpunit/includes/changeop/ChangeOpAliasesTest.php
@@ -0,0 +1,100 @@
+<?php
+
+namespace Wikibase\Test;
+
+use Wikibase\ChangeOpAliases;
+use Wikibase\ItemContent;
+use InvalidArgumentException;
+use MWException;
+
+/**
+ * @covers Wikibase\ChangeOpAliases
+ *
+ * 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
+ *
+ * @file
+ * @since 0.4
+ *
+ * @ingroup Wikibase
+ * @ingroup Test
+ *
+ * @group Wikibase
+ * @group WikibaseRepo
+ * @group ChangeOp
+ *
+ * @licence GNU GPL v2+
+ * @author Tobias Gritschacher < [email protected] >
+ */
+class ChangeOpAliasesTest extends \PHPUnit_Framework_TestCase {
+
+ public function invalidConstructorProvider() {
+ $args = array();
+ $args[] = array( 42, array( 'myNewAlias' ), 'add' );
+ $args[] = array( 'en', array( 'myNewAlias' ), 1234 );
+
+ return $args;
+ }
+
+ /**
+ * @dataProvider invalidConstructorProvider
+ * @expectedException InvalidArgumentException
+ *
+ * @param string $language
+ * @param string[] $aliases
+ * @param string $action
+ */
+ public function testInvalidConstruct( $language, $aliases, $action ) {
+ $changeOpLabel = new ChangeOpAliases( $language, $aliases,
$action );
+ }
+
+ public function changeOpAliasesProvider() {
+ $enAliases = array( 'en-alias1', 'en-alias2', 'en-alias3' );
+ $existingEnAliases = array ( 'en-existingAlias1',
'en-existingAlias2' );
+ $item = ItemContent::newEmpty();
+ $entity = $item->getEntity();
+ $entity->setAliases( 'en', $existingEnAliases );
+
+ $args = array();
+ $args[] = array ( clone $entity, new ChangeOpAliases( 'en',
$enAliases, 'add' ), array_merge( $existingEnAliases, $enAliases ) );
+ $args[] = array ( clone $entity, new ChangeOpAliases( 'en',
$enAliases, 'set' ), $enAliases );
+ $args[] = array ( clone $entity, new ChangeOpAliases( 'en',
$enAliases, '' ), $enAliases );
+ $args[] = array ( clone $entity, new ChangeOpAliases( 'en',
$existingEnAliases, 'remove' ), array() );
+
+ return $args;
+ }
+
+ /**
+ * @dataProvider changeOpAliasesProvider
+ *
+ * @param Entity $entity
+ * @param ChangeOpAliases $changeOpAliases
+ * @param string $expectedAliases
+ */
+ public function testApply( $entity, $changeOpAliases, $expectedAliases
) {
+ $changeOpAliases->apply( $entity );
+ $this->assertEquals( $expectedAliases, $entity->getAliases(
'en' ) );
+ }
+
+ /**
+ * @expectedException MWException
+ */
+ public function testApplyWithInvalidAction() {
+ $item = ItemContent::newEmpty();
+ $entity = $item->getEntity();
+ $changeOpAliases = new ChangeOpAliases( 'en', array( 'test' ),
'invalidAction' );
+ $changeOpAliases->apply( $entity );
+ }
+
+}
diff --git a/repo/tests/phpunit/includes/changeop/ChangeOpSiteLinkTest.php
b/repo/tests/phpunit/includes/changeop/ChangeOpSiteLinkTest.php
new file mode 100644
index 0000000..31467fe
--- /dev/null
+++ b/repo/tests/phpunit/includes/changeop/ChangeOpSiteLinkTest.php
@@ -0,0 +1,101 @@
+<?php
+
+namespace Wikibase\Test;
+
+use Wikibase\ChangeOpSiteLink;
+use Wikibase\ItemContent;
+use InvalidArgumentException;
+use Wikibase\SiteLink;
+
+/**
+ * @covers Wikibase\ChangeOpSiteLink
+ *
+ * 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
+ *
+ * @file
+ * @since 0.4
+ *
+ * @ingroup Wikibase
+ * @ingroup Test
+ *
+ * @group Wikibase
+ * @group WikibaseRepo
+ * @group ChangeOp
+ *
+ * @licence GNU GPL v2+
+ * @author Tobias Gritschacher < [email protected] >
+ */
+class ChangeOpSiteLinkTest extends \PHPUnit_Framework_TestCase {
+
+ public function invalidConstructorProvider() {
+ $args = array();
+ $args[] = array( $this->getSite( 'enwiki' ), 1234 );
+
+ return $args;
+ }
+
+ /**
+ * @dataProvider invalidConstructorProvider
+ * @expectedException InvalidArgumentException
+ *
+ * @param Site $linkSite
+ * @param string|null $aliases
+ */
+ public function testInvalidConstruct( $linkSite, $linkPage ) {
+ $changeOpSiteLink = new ChangeOpSiteLink( $linkSite, $linkPage
);
+ }
+
+ public function changeOpSiteLinkProvider() {
+ $enWiki = $this->getSite( 'enwiki' );
+ $deWiki = $this->getSite( 'dewiki' );
+ $existingSiteLinks = array( new SiteLink( $deWiki, 'Berlin' ) );
+ $enSiteLink = new SiteLink( $enWiki, 'Berlin' );
+
+ $item = ItemContent::newEmpty();
+ $entity = $item->getEntity();
+ foreach ( $existingSiteLinks as $siteLink ) {
+ $entity->addSiteLink( $siteLink );
+ }
+
+ $args = array();
+ $args[] = array ( clone $entity, new ChangeOpSiteLink( $enWiki,
'Berlin' ), array_merge( $existingSiteLinks, array ( $enSiteLink ) ) );
+ $args[] = array ( clone $entity, new ChangeOpSiteLink( $deWiki,
null ), array() );
+
+ return $args;
+ }
+
+ /**
+ * @dataProvider changeOpSiteLinkProvider
+ *
+ * @param Entity $entity
+ * @param ChangeOpSiteLink $changeOpSiteLink
+ * @param string $expectedSiteLinks
+ */
+ public function testApply( $entity, $changeOpSiteLink,
$expectedSiteLinks ) {
+ $changeOpSiteLink->apply( $entity );
+ $this->assertEquals(
+ SiteLink::siteLinksToArray( $expectedSiteLinks ),
+ SiteLink::siteLinksToArray( $entity->getSiteLinks() )
+ );
+ }
+
+ protected function getSite( $globalId ) {
+ $site = new \MediaWikiSite();
+ $site->setGlobalId( $globalId );
+
+ return $site;
+ }
+
+}
diff --git a/repo/tests/phpunit/includes/changeop/ChangeOpsTest.php
b/repo/tests/phpunit/includes/changeop/ChangeOpsTest.php
index c0f4016..6e15e9e 100644
--- a/repo/tests/phpunit/includes/changeop/ChangeOpsTest.php
+++ b/repo/tests/phpunit/includes/changeop/ChangeOpsTest.php
@@ -67,6 +67,49 @@
$this->assertEquals( array( $changeOp ),
$changeOps->getChangeOps() );
}
+ public function changeOpArrayProvider() {
+ $ops = array();
+ $ops[] = array (
+ array(
+ new ChangeOpLabel( 'en',
'enLabel' ),
+ new ChangeOpLabel( 'de',
'deLabel' ),
+ new ChangeOpDescription( 'en',
'enDescr' ),
+ )
+ );
+
+ return $ops;
+ }
+
+ /**
+ * @dataProvider changeOpArrayProvider
+ *
+ * @param ChangeOp[] $changeOp
+ */
+ public function testAddArray( $changeOpArray ) {
+ $changeOps = new ChangeOps();
+ $changeOps->add( $changeOpArray );
+ $this->assertEquals( $changeOpArray, $changeOps->getChangeOps()
);
+ }
+
+ public function invalidChangeOpProvider() {
+ $ops = array();
+ $ops[] = array ( 1234 );
+ $ops[] = array ( array( new ChangeOpLabel( 'en', 'test' ), 123
) );
+
+ return $ops;
+ }
+
+ /**
+ * @dataProvider invalidChangeOpProvider
+ * @expectedException InvalidArgumentException
+ *
+ * @param $invalidChangeOp
+ */
+ public function testInvalidAdd( $invalidChangeOp ) {
+ $changeOps = new ChangeOps();
+ $changeOps->add( $invalidChangeOp );
+ }
+
public function changeOpsProvider() {
$args = array();
--
To view, visit https://gerrit.wikimedia.org/r/66981
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic27774f67f8728acbb48cbf945fda063791096cf
Gerrit-PatchSet: 14
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Tobias Gritschacher <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Tobias Gritschacher <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
