jenkins-bot has submitted this change and it was merged.
Change subject: Allow id and type params in wbeditentity data!
......................................................................
Allow id and type params in wbeditentity data!
Bug: 54146
Change-Id: I379b280bdf398c4cf35284469bbbfa9ee7877084
---
M repo/includes/api/EditEntity.php
M repo/tests/phpunit/includes/api/EditEntityTest.php
2 files changed, 88 insertions(+), 29 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/includes/api/EditEntity.php b/repo/includes/api/EditEntity.php
index 73e7da7..72ba60d 100644
--- a/repo/includes/api/EditEntity.php
+++ b/repo/includes/api/EditEntity.php
@@ -7,8 +7,8 @@
use InvalidArgumentException;
use MWException;
use Revision;
+use Site;
use SiteList;
-use SiteSQLStore;
use Title;
use Wikibase\ChangeOp\ChangeOp;
use Wikibase\ChangeOp\ChangeOpAliases;
@@ -20,6 +20,7 @@
use Wikibase\ChangeOp\ChangeOpSiteLink;
use Wikibase\ChangeOp\ChangeOps;
use Wikibase\Claim;
+use Wikibase\DataModel\Entity\EntityId;
use Wikibase\Entity;
use Wikibase\EntityContent;
use Wikibase\Item;
@@ -124,7 +125,7 @@
$entity = $entityContent->getEntity();
$this->validateDataParameter( $params );
$data = json_decode( $params['data'], true );
- $this->validateDataProperties( $data,
$entityContent->getWikiPage() );
+ $this->validateDataProperties( $data, $entityContent );
if ( $params['clear'] ) {
$entity->clear();
@@ -171,6 +172,12 @@
return $summary;
}
+ /**
+ * @param array $data
+ * @param Entity $entity
+ *
+ * @return ChangeOps
+ */
protected function getChangeOps( array $data, Entity $entity ) {
$changeOps = new ChangeOps();
@@ -306,6 +313,7 @@
$aliasesChangeOps = array();
foreach ( $indexedAliases as $langCode => $args ) {
$aliasesToSet = array();
+ $language = '';
foreach ( $args as $arg ) {
$this->validateMultilangArgs( $arg, $langCode );
@@ -334,7 +342,7 @@
* @since 0.4
*
* @param array $siteLinks
- * @param Entity $entity
+ * @param Entity|Item $entity
*
* @return ChangeOpSiteLink[]
*/
@@ -360,6 +368,7 @@
} else {
$this->dieUsage( "There is no site for global
site id '$globalSiteId'", 'no-such-site' );
}
+ /** @var Site $linkSite */
if ( $shouldRemove ) {
$siteLinksChangeOps[] = new ChangeOpSiteLink(
$globalSiteId );
@@ -373,7 +382,7 @@
if ( $linkPage === false ) {
$this->dieUsage(
- "The external client
site did not provide page information for site '{$globalSiteId}' and title
'{$pageTitle}'",
+ "The external client
site did not provide page information for site '{$globalSiteId}'",
'no-external-page' );
}
} else {
@@ -508,26 +517,33 @@
/**
* @since 0.4
+ *
* @param array $data
- * @param WikiPage|bool $page
+ * @param EntityContent $entityContent
*/
- protected function validateDataProperties( $data, $page ) {
+ protected function validateDataProperties( $data, $entityContent ) {
$title = null;
$revision = null;
- if ( $page ) {
- $title = $page->getTitle();
- $revision = $page->getRevision();
+ if ( $entityContent ) {
+ $wikiPage = $entityContent->getWikiPage();
+ $title = $wikiPage->getTitle();
+ $revision = $wikiPage->getTitle();
}
$allowedProps = array(
+ // ignored props
'length',
'count',
'touched',
+ // checked props
+ 'id',
+ 'type',
'pageid',
'ns',
'title',
'lastrevid',
+ // useful props
'labels',
'descriptions',
'aliases',
@@ -537,7 +553,9 @@
);
$this->checkValidJson( $data, $allowedProps );
- $this->checkPageIdProp( $data, $page );
+ $this->checkEntityId( $data,
$entityContent->getEntity()->getId() );
+ $this->checkEntityType( $data, $entityContent );
+ $this->checkPageIdProp( $data, $entityContent );
$this->checkNamespaceProp( $data, $title );
$this->checkTitleProp( $data, $title );
$this->checkRevisionProp( $data, $revision );
@@ -576,7 +594,10 @@
protected function checkPageIdProp( $data, $page ) {
if ( isset( $data['pageid'] )
&& ( is_object( $page ) ? $page->getId() !==
$data['pageid'] : true ) ) {
- $this->dieUsage( 'Illegal field used in call, "pageid",
must either be correct or not given', 'param-illegal' );
+ $this->dieUsage(
+ 'Illegal field used in call, "pageid", must
either be correct or not given',
+ 'param-illegal'
+ );
}
}
@@ -588,7 +609,10 @@
// 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 ) ) {
- $this->dieUsage( 'Illegal field used in call:
"namespace", must either be correct or not given', 'param-illegal' );
+ $this->dieUsage(
+ 'Illegal field used in call: "namespace", must
either be correct or not given',
+ 'param-illegal'
+ );
}
}
@@ -599,7 +623,10 @@
protected function checkTitleProp( $data, $title ) {
if ( isset( $data['title'] )
&& ( is_object( $title ) ? $title->getPrefixedText()
!== $data['title'] : true ) ) {
- $this->dieUsage( 'Illegal field used in call: "title",
must either be correct or not given', 'param-illegal' );
+ $this->dieUsage(
+ 'Illegal field used in call: "title", must
either be correct or not given',
+ 'param-illegal'
+ );
}
}
@@ -610,7 +637,33 @@
protected function checkRevisionProp( $data, $revision ) {
if ( isset( $data['lastrevid'] )
&& ( is_object( $revision ) ? $revision->getId() !==
$data['lastrevid'] : true ) ) {
- $this->dieUsage( 'Illegal field used in call:
"lastrevid", must either be correct or not given', 'param-illegal' );
+ $this->dieUsage(
+ 'Illegal field used in call: "lastrevid", must
either be correct or not given',
+ 'param-illegal'
+ );
+ }
+ }
+
+ private function checkEntityId( $data, EntityId $entityId ) {
+ if ( isset( $data['id'] ) ) {
+ $entityIdParser =
WikibaseRepo::getDefaultInstance()->getEntityIdParser();
+ $dataId = $entityIdParser->parse( $data['id'] );
+ if( !$entityId->equals( $dataId ) ) {
+ $this->dieUsage(
+ 'Invalid field used in call: "id", must
match id parameter',
+ 'param-invalid'
+ );
+ }
+ }
+ }
+
+ private function checkEntityType( $data, EntityContent $entityContent )
{
+ if ( isset( $data['type'] )
+ && $entityContent->getEntity()->getType() !==
$data['type'] ) {
+ $this->dieUsage(
+ 'Invalid field used in call: "type", must match
type associated with id',
+ 'param-invalid'
+ );
}
}
@@ -632,6 +685,7 @@
array( 'code' => 'not-recognized-string', 'info' =>
$this->msg( 'wikibase-api-not-recognized-string' )->text() ),
array( 'code' => 'param-illegal', 'info' => $this->msg(
'wikibase-api-param-illegal' )->text() ),
array( 'code' => 'param-missing', 'info' => $this->msg(
'wikibase-api-param-missing' )->text() ),
+ array( 'code' => 'param-invalid', 'info' => $this->msg(
'wikibase-api-param-invalid' )->text() ),
array( 'code' => 'inconsistent-language', 'info' =>
$this->msg( 'wikibase-api-inconsistent-language' )->text() ),
array( 'code' => 'not-recognised-language', 'info' =>
$this->msg( 'wikibase-not-recognised-language' )->text() ),
array( 'code' => 'inconsistent-site', 'info' =>
$this->msg( 'wikibase-api-inconsistent-site' )->text() ),
diff --git a/repo/tests/phpunit/includes/api/EditEntityTest.php
b/repo/tests/phpunit/includes/api/EditEntityTest.php
index 7a34c43..855add1 100644
--- a/repo/tests/phpunit/includes/api/EditEntityTest.php
+++ b/repo/tests/phpunit/includes/api/EditEntityTest.php
@@ -2,9 +2,6 @@
namespace Wikibase\Test\Api;
-use ApiTestCase;
-use Wikibase\DataModel\Entity\ItemId;
-use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\ItemContent;
use Wikibase\PropertyContent;
@@ -15,22 +12,15 @@
*
* @licence GNU GPL v2+
* @author Adam Shorland
- * @author Michał Łazowik
+ * @author Michal Lazowik
*
* @group API
* @group Wikibase
* @group WikibaseAPI
* @group EditEntityTest
* @group BreakingTheSlownessBarrier
- *
- * The database group has as a side effect that temporal database tables are
created. This makes
- * it possible to test without poisoning a production database.
* @group Database
- *
- * Some of the tests takes more time, and needs therefor longer time before
they can be aborted
- * as non-functional. The reason why tests are aborted is assumed to be set up
of temporal databases
- * that hold the first tests in a pending state awaiting access to the
database.
- * @group large
+ * @group medium
*/
class EditEntityTest extends WikibaseApiTestCase {
@@ -42,6 +32,7 @@
if( !isset( self::$hasSetup ) ){
$this->initTestEntities( array( 'Berlin' ) );
+ self::$idMap['%Berlin%'] = EntityTestHelper::getId(
'Berlin' );
$prop = PropertyContent::newEmpty();
$prop->getEntity()->setDataTypeId( 'string' );
@@ -64,19 +55,23 @@
self::$hasSetup = true;
}
+ /**
+ * Provide data for a sequence of requests that will work when run in
order
+ * @return array
+ */
public static function provideData() {
return array(
'new item' => array( // new item
'p' => array( 'new' => 'item', 'data' => '{}' ),
'e' => array( 'type' => 'item' ) ),
- 'new property' => array( // new property
+ 'new property' => array( // new property (also make
sure if we pass in a valid type it is accepted)
'p' => array( 'new' => 'property', 'data' =>
'{"datatype":"string"}' ),
'e' => array( 'type' => 'property' ) ),
'new property (this is our current example in the api
doc)' => array( // new property (this is our current example in the api doc)
'p' => array( 'new' => 'property', 'data' =>
'{"labels":{"en-gb":{"language":"en-gb","value":"Propertylabel"}},'.
'"descriptions":{"en-gb":{"language":"en-gb","value":"Propertydescription"}},"datatype":"string"}'
),
'e' => array( 'type' => 'property' ) ),
- 'add a sitelink..' => array( // add a sitelink..
+ 'add a sitelink..' => array( // add a sitelink.. (also
makes sure if we pass in a valid id it is accepted)
'p' => array( 'data' =>
'{"sitelinks":{"dewiki":{"site":"dewiki","title":"TestPage!","badges":["%Q42%","%Q149%"]}}}'
),
'e' => array(
'sitelinks' => array(
@@ -358,7 +353,7 @@
/**
* @dataProvider provideData
*/
- function testEditEntity( $params, $expected ) {
+ public function testEditEntity( $params, $expected ) {
$this->injectIds( $params );
$this->injectIds( $expected );
@@ -413,6 +408,10 @@
}
}
+ /**
+ * Provide data for requests that will fail with a set exception, code
and message
+ * @return array
+ */
public static function provideExceptionData() {
return array(
'no entity id given' => array( // no entity id given
@@ -494,6 +493,12 @@
'no sitelink - cannot change badges' => array( // no
sitelink - cannot change badges
'p' => array( 'site' => 'enwiki', 'title' =>
'Berlin', 'data' =>
'{"sitelinks":{"svwiki":{"site":"svwiki","badges":["%Q42%","%Q149%"]}}}' ),
'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'no-such-sitelink' ) ) ),
+ 'bad id in serialization' => array( // no entity id
given
+ 'p' => array( 'id' => '%Berlin%', 'data' =>
'{"id":"Q13244"}'),
+ 'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'param-invalid', 'message' => 'Invalid field used
in call: "id", must match id parameter' ) ) ),
+ 'bad type in serialization' => array( // no entity id
given
+ 'p' => array( 'id' => '%Berlin%', 'data' =>
'{"id":"%Berlin%","type":"foobar"}'),
+ 'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'param-invalid', 'message' => 'Invalid field used
in call: "type", must match type associated with id' ) ) ),
);
}
--
To view, visit https://gerrit.wikimedia.org/r/94775
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I379b280bdf398c4cf35284469bbbfa9ee7877084
Gerrit-PatchSet: 16
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Tobias Gritschacher <tobias.gritschac...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
