Daniel Kinzler has uploaded a new change for review.
https://gerrit.wikimedia.org/r/123872
Change subject: (bug 62644) snak validation in ChangeOpReference
......................................................................
(bug 62644) snak validation in ChangeOpReference
Change-Id: I181379984027f081d7e40c0f792d58cece656dd7
---
M repo/includes/ChangeOp/ChangeOpFactory.php
M repo/includes/ChangeOp/ChangeOpMainSnak.php
M repo/includes/ChangeOp/ChangeOpQualifier.php
M repo/includes/ChangeOp/ChangeOpReference.php
M repo/includes/api/SetReference.php
M repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierTest.php
M repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceTest.php
M repo/tests/phpunit/includes/api/SetQualifierTest.php
M repo/tests/phpunit/includes/api/SetReferenceTest.php
9 files changed, 328 insertions(+), 83 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/72/123872/1
diff --git a/repo/includes/ChangeOp/ChangeOpFactory.php
b/repo/includes/ChangeOp/ChangeOpFactory.php
index 6fa44e2..28e5191 100644
--- a/repo/includes/ChangeOp/ChangeOpFactory.php
+++ b/repo/includes/ChangeOp/ChangeOpFactory.php
@@ -242,7 +242,7 @@
*/
public function newSetQualifierOp( $claimGuid, Snak $snak, $snakHash ) {
//XXX: index??
- return new ChangeOpQualifier( $claimGuid, $snak, $snakHash );
+ return new ChangeOpQualifier( $claimGuid, $snak, $snakHash,
$this->snakValidator );
}
/**
@@ -266,7 +266,7 @@
* @return ChangeOp
*/
public function newSetReferenceOp( $claimGuid, Reference $reference,
$referenceHash, $index = null ) {
- return new ChangeOpReference( $claimGuid, $reference,
$referenceHash );
+ return new ChangeOpReference( $claimGuid, $reference,
$referenceHash, $this->snakValidator );
}
/**
diff --git a/repo/includes/ChangeOp/ChangeOpMainSnak.php
b/repo/includes/ChangeOp/ChangeOpMainSnak.php
index 83a0982..94a942a 100644
--- a/repo/includes/ChangeOp/ChangeOpMainSnak.php
+++ b/repo/includes/ChangeOp/ChangeOpMainSnak.php
@@ -30,7 +30,7 @@
/**
* @since 0.4
*
- * @var Snak|null
+ * @var Snak
*/
protected $snak;
@@ -45,7 +45,7 @@
* @since 0.4
*
* @param string $claimGuid
- * @param Snak|null $snak
+ * @param Snak $snak
* @param ClaimGuidGenerator $guidGenerator
* @param SnakValidator $snakValidator
*
@@ -53,16 +53,12 @@
*/
public function __construct(
$claimGuid,
- $snak,
+ Snak $snak,
ClaimGuidGenerator $guidGenerator,
SnakValidator $snakValidator
) {
if ( !is_string( $claimGuid ) ) {
throw new InvalidArgumentException( '$claimGuid needs
to be a string' );
- }
-
- if ( !( $snak instanceof Snak ) ) {
- throw new InvalidArgumentException( '$snak needs to be
an instance of Snak' );
}
$this->claimGuid = $claimGuid;
diff --git a/repo/includes/ChangeOp/ChangeOpQualifier.php
b/repo/includes/ChangeOp/ChangeOpQualifier.php
index 9b8fb98..4e5bf3c 100644
--- a/repo/includes/ChangeOp/ChangeOpQualifier.php
+++ b/repo/includes/ChangeOp/ChangeOpQualifier.php
@@ -8,6 +8,7 @@
use Wikibase\DataModel\Snak\Snak;
use Wikibase\DataModel\Snak\Snaks;
use Wikibase\Summary;
+use Wikibase\Validators\SnakValidator;
/**
* Class for qualifier change operation
@@ -15,6 +16,7 @@
* @since 0.4
* @licence GNU GPL v2+
* @author Tobias Gritschacher < [email protected] >
+ * @author Daniel Kinzler
*/
class ChangeOpQualifier extends ChangeOpBase {
@@ -28,7 +30,7 @@
/**
* @since 0.4
*
- * @var Snak|null
+ * @var Snak
*/
protected $snak;
@@ -40,17 +42,23 @@
protected $snakHash;
/**
+ * @var SnakValidator
+ */
+ private $snakValidator;
+
+ /**
* Constructs a new qualifier change operation
*
* @since 0.4
*
* @param string $claimGuid
- * @param Snak|null $snak
+ * @param Snak $snak
* @param string $snakHash
+ * @param SnakValidator $snakValidator
*
* @throws InvalidArgumentException
*/
- public function __construct( $claimGuid, $snak, $snakHash ) {
+ public function __construct( $claimGuid, Snak $snak, $snakHash,
SnakValidator $snakValidator ) {
if ( !is_string( $claimGuid ) || $claimGuid === '' ) {
throw new InvalidArgumentException( '$claimGuid needs
to be a string and must not be empty' );
}
@@ -59,13 +67,10 @@
throw new InvalidArgumentException( '$snakHash needs to
be a string' );
}
- if ( !( $snak instanceof Snak ) ) {
- throw new InvalidArgumentException( '$snak needs to be
an instance of Snak' );
- }
-
$this->claimGuid = $claimGuid;
$this->snak = $snak;
$this->snakHash = $snakHash;
+ $this->snakValidator = $snakValidator;
}
/**
@@ -74,6 +79,8 @@
* - the qualifier gets set to $snak when $snakHash and $snak are set
*/
public function apply( Entity $entity, Summary $summary = null ) {
+ $this->validate();
+
$claims = new Claims( $entity->getClaims() );
if( !$claims->hasClaimWithGuid( $this->claimGuid ) ) {
@@ -143,4 +150,17 @@
$propertyId = $snak->getPropertyId();
return array( array( $propertyId->getPrefixedId() => $snak ) );
}
+
+ /**
+ * @since 0.5
+ *
+ * @throws ChangeOpException
+ */
+ protected function validate() {
+ $result = $this->snakValidator->validate( $this->snak );
+
+ if ( !$result->isValid() ) {
+ throw new ChangeOpValidationException( $result );
+ }
+ }
}
diff --git a/repo/includes/ChangeOp/ChangeOpReference.php
b/repo/includes/ChangeOp/ChangeOpReference.php
index 1c2edf5..26737cd 100644
--- a/repo/includes/ChangeOp/ChangeOpReference.php
+++ b/repo/includes/ChangeOp/ChangeOpReference.php
@@ -10,6 +10,7 @@
use Wikibase\DataModel\References;
use Wikibase\DataModel\Snak\Snak;
use Wikibase\Summary;
+use Wikibase\Validators\SnakValidator;
/**
* Class for reference change operation
@@ -17,6 +18,7 @@
* @since 0.4
* @licence GNU GPL v2+
* @author Tobias Gritschacher < [email protected] >
+ * @author Daniel Kinzler
*/
class ChangeOpReference extends ChangeOpBase {
@@ -30,7 +32,7 @@
/**
* @since 0.4
*
- * @var Reference|null
+ * @var Reference
*/
protected $reference;
@@ -49,18 +51,24 @@
protected $index;
/**
+ * @var SnakValidator
+ */
+ private $snakValidator;
+
+ /**
* Constructs a new reference change operation
*
* @since 0.4
*
* @param string $claimGuid
- * @param Reference|null $reference
+ * @param Reference $reference
* @param string $referenceHash (if empty '' a new reference will be
created)
+ * @param SnakValidator $snakValidator
* @param int|null $index
*
* @throws InvalidArgumentException
*/
- public function __construct( $claimGuid, $reference, $referenceHash,
$index = null ) {
+ public function __construct( $claimGuid, Reference $reference,
$referenceHash, SnakValidator $snakValidator, $index = null ) {
if ( !is_string( $claimGuid ) || $claimGuid === '' ) {
throw new InvalidArgumentException( '$claimGuid needs
to be a string and must not be empty' );
}
@@ -81,6 +89,7 @@
$this->reference = $reference;
$this->referenceHash = $referenceHash;
$this->index = $index;
+ $this->snakValidator = $snakValidator;
}
/**
@@ -89,6 +98,8 @@
* - the reference gets set to $reference when $referenceHash and
$reference are set
*/
public function apply( Entity $entity, Summary $summary = null ) {
+ $this->validate();
+
$claims = new Claims( $entity->getClaims() );
if( !$claims->hasClaimWithGuid( $this->claimGuid ) ) {
@@ -177,4 +188,17 @@
return array( array( $propertyId->getPrefixedId() => $snak ) );
}
+
+ /**
+ * @since 0.5
+ *
+ * @throws ChangeOpException
+ */
+ protected function validate() {
+ $result = $this->snakValidator->validateReference(
$this->reference );
+
+ if ( !$result->isValid() ) {
+ throw new ChangeOpValidationException( $result );
+ }
+ }
}
diff --git a/repo/includes/api/SetReference.php
b/repo/includes/api/SetReference.php
index be7753b..897a800 100644
--- a/repo/includes/api/SetReference.php
+++ b/repo/includes/api/SetReference.php
@@ -70,6 +70,7 @@
try {
$changeOp->apply( $entity, $summary );
} catch ( ChangeOpException $e ) {
+ //FIXME: specific, localized error message!
$this->dieUsage( $e->getMessage(), 'failed-save' );
}
diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierTest.php
b/repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierTest.php
index bd0ec08..daf8991 100644
--- a/repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierTest.php
+++ b/repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierTest.php
@@ -2,12 +2,20 @@
namespace Wikibase\Test;
+use DataValues\NumberValue;
use DataValues\StringValue;
use Wikibase\ChangeOp\ChangeOpQualifier;
use Wikibase\DataModel\Claim\Claim;
use Wikibase\DataModel\Claim\Claims;
+use Wikibase\DataModel\Claim\Statement;
use Wikibase\DataModel\Entity\Entity;
+use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Snak\PropertyNoValueSnak;
use Wikibase\DataModel\Snak\PropertyValueSnak;
+use Wikibase\DataModel\Snak\Snak;
+use Wikibase\DataModel\Snak\SnakList;
+use Wikibase\Item;
use Wikibase\ItemContent;
use InvalidArgumentException;
use Wikibase\Lib\ClaimGuidGenerator;
@@ -21,8 +29,25 @@
*
* @licence GNU GPL v2+
* @author Tobias Gritschacher < [email protected] >
+ * @author Daniel Kinzler
*/
class ChangeOpQualifierTest extends \PHPUnit_Framework_TestCase {
+
+ /**
+ * @var ClaimTestHelper
+ */
+ protected $helper;
+
+ /**
+ * @param null $name
+ * @param array $data
+ * @param string $dataName
+ */
+ public function __construct( $name = null, array $data = array(),
$dataName = '' ) {
+ parent::__construct( $name, $data, $dataName );
+
+ $this->helper = new ClaimTestHelper();
+ }
public function invalidArgumentProvider() {
$item = ItemContent::newFromArray( array( 'entity' => 'q42' )
)->getEntity();
@@ -34,10 +59,6 @@
$args = array();
$args[] = array( 123, $validSnak, $validSnakHash );
$args[] = array( '', $validSnak, $validSnakHash );
- $args[] = array( 123, null, $validSnakHash );
- $args[] = array( $validClaimGuid, 'notASnak', $validSnakHash );
- $args[] = array( $validClaimGuid, 'notASnak', '' );
- $args[] = array( $validClaimGuid, null, '' );
$args[] = array( $validClaimGuid, $validSnak, 123 );
return $args;
@@ -49,7 +70,7 @@
* @expectedException InvalidArgumentException
*/
public function testInvalidConstruct( $claimGuid, $snak, $snakHash ) {
- new ChangeOpQualifier( $claimGuid, $snak, $snakHash );
+ new ChangeOpQualifier( $claimGuid, $snak, $snakHash,
$this->helper->getMockSnakValidator() );
}
public function changeOpAddProvider() {
@@ -61,7 +82,7 @@
$claim = reset( $claims );
$claimGuid = $claim->getGuid();
$newQualifier = new PropertyValueSnak( 78462378, new
StringValue( 'newQualifier' ) );
- $changeOp = new ChangeOpQualifier( $claimGuid, $newQualifier,
'' );
+ $changeOp = new ChangeOpQualifier( $claimGuid, $newQualifier,
'', $this->helper->getMockSnakValidator() );
$snakHash = $newQualifier->getHash();
$args[] = array ( $item, $changeOp, $snakHash );
@@ -100,7 +121,7 @@
$item->setClaims( new Claims( $claims ) );
$snakHash = $newQualifier->getHash();
$changedQualifier = new PropertyValueSnak( 78462378, new
StringValue( 'changedQualifier' ) );
- $changeOp = new ChangeOpQualifier( $claimGuid,
$changedQualifier, $snakHash );
+ $changeOp = new ChangeOpQualifier( $claimGuid,
$changedQualifier, $snakHash, $this->helper->getMockSnakValidator() );
$args[] = array ( $item, $changeOp,
$changedQualifier->getHash() );
return $args;
@@ -131,4 +152,55 @@
$entity->setClaims( $claims );
return $entity;
}
+
+ public function provideApplyInvalid() {
+ $p11 = new PropertyId( 'P11' );
+ $q17 = new ItemId( 'Q17' );
+
+ $item = Item::newEmpty();
+ $item->setId( $q17 );
+ $claimGuid = $this->helper->getGuidGenerator()->newGuid( $q17 );
+ $badGuid = $this->helper->getGuidGenerator()->newGuid( $q17 );
+
+ $oldSnak = new PropertyValueSnak( $p11, new StringValue( "old
qualifier" ) );
+
+ $claim = new Statement( new PropertyNoValueSnak( $p11 ), new
SnakList( array( $oldSnak ) ) );
+ $claim->setGuid( $claimGuid );
+ $item->addClaim( $claim );
+
+ //NOTE: the mock validator will consider the string "INVALID"
to be invalid.
+ $goodSnak = new PropertyValueSnak( $p11, new StringValue(
'good' ) );
+ $badSnak = new PropertyValueSnak( $p11, new StringValue(
'INVALID' ) );
+ $brokenSnak = new PropertyValueSnak( $p11, new NumberValue( 23
) );
+
+ $snakHash = $oldSnak->getHash();
+ $badSnakHash = $badSnak->getHash();
+
+ $cases = array();
+ $cases['malformed claim guid'] = array( $item, 'NotAGuid',
$goodSnak, '' );
+ $cases['unknown claim guid'] = array( $item, $badGuid,
$goodSnak, $snakHash );
+ $cases['unknown snak hash'] = array( $item, $claimGuid,
$goodSnak, $badSnakHash );
+
+ $cases['invalid snak value'] = array( $item, $claimGuid,
$badSnak, '' );
+ $cases['invalid snak value type'] = array( $item, $claimGuid,
$brokenSnak, $snakHash );
+
+ return $cases;
+ }
+
+ /**
+ * @dataProvider provideApplyInvalid
+ */
+ public function testApplyInvalid( Entity $entity, $claimGuid, Snak
$snak, $snakHash = '' ) {
+ $this->setExpectedException(
'Wikibase\ChangeOp\ChangeOpException' );
+
+ $changeOpClaim = new ChangeOpQualifier(
+ $claimGuid,
+ $snak,
+ $snakHash,
+ $this->helper->getMockSnakValidator()
+ );
+
+ $entity = $entity->copy();
+ $changeOpClaim->apply( $entity );
+ }
}
diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceTest.php
b/repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceTest.php
index 0a7b399..6203741 100644
--- a/repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceTest.php
+++ b/repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceTest.php
@@ -2,12 +2,16 @@
namespace Wikibase\Test;
+use DataValues\NumberValue;
use DataValues\StringValue;
use LogicException;
use Wikibase\ChangeOp\ChangeOpReference;
use Wikibase\DataModel\Claim\Claims;
use Wikibase\DataModel\Claim\Statement;
use Wikibase\DataModel\Entity\Entity;
+use Wikibase\DataModel\Entity\Item;
+use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Reference;
use Wikibase\DataModel\Snak\PropertyNoValueSnak;
use Wikibase\DataModel\Snak\PropertyValueSnak;
@@ -25,8 +29,25 @@
*
* @licence GNU GPL v2+
* @author Tobias Gritschacher < [email protected] >
+ * @author Daniel Kinzler
*/
class ChangeOpReferenceTest extends \PHPUnit_Framework_TestCase {
+
+ /**
+ * @var ClaimTestHelper
+ */
+ protected $helper;
+
+ /**
+ * @param null $name
+ * @param array $data
+ * @param string $dataName
+ */
+ public function __construct( $name = null, array $data = array(),
$dataName = '' ) {
+ parent::__construct( $name, $data, $dataName );
+
+ $this->helper = new ClaimTestHelper();
+ }
public function invalidArgumentProvider() {
$item = ItemContent::newFromArray( array( 'entity' => 'q42' )
)->getEntity();
@@ -40,11 +61,7 @@
$args = array();
$args[] = array( 123, $validReference, $validReferenceHash );
$args[] = array( '', $validReference, $validReferenceHash );
- $args[] = array( '', null, $validReferenceHash );
$args[] = array( $validClaimGuid, $validReference, 123 );
- $args[] = array( $validClaimGuid, 'notAReference',
$validReferenceHash );
- $args[] = array( $validClaimGuid, 'notAReference', '' );
- $args[] = array( $validClaimGuid, null, '' );
$args[] = array( $validClaimGuid, $validReference,
$validReferenceHash, 'string' );
return $args;
@@ -56,7 +73,7 @@
* @expectedException InvalidArgumentException
*/
public function testInvalidConstruct( $claimGuid, $reference,
$referenceHash, $index = null ) {
- new ChangeOpReference( $claimGuid, $reference, $referenceHash,
$index );
+ new ChangeOpReference( $claimGuid, $reference, $referenceHash,
$this->helper->getMockSnakValidator(), $index );
}
public function changeOpAddProvider() {
@@ -70,7 +87,7 @@
$snaks = new SnakList();
$snaks[] = new PropertyValueSnak( 78462378, new StringValue(
'newQualifier' ) );
$newReference = new Reference( $snaks );
- $changeOp = new ChangeOpReference( $claimGuid, $newReference,
'' );
+ $changeOp = new ChangeOpReference( $claimGuid, $newReference,
'', $this->helper->getMockSnakValidator() );
$referenceHash = $newReference->getHash();
$args[] = array ( $item, $changeOp, $referenceHash );
@@ -122,6 +139,7 @@
$claimGuid,
$newReference,
'',
+ $this->helper->getMockSnakValidator(),
$newReferenceIndex
);
@@ -174,7 +192,7 @@
$snaks = new SnakList();
$snaks[] = new PropertyValueSnak( 78462378, new StringValue(
'changedQualifier' ) );
$changedReference = new Reference( $snaks );
- $changeOp = new ChangeOpReference( $claimGuid,
$changedReference, $referenceHash );
+ $changeOp = new ChangeOpReference( $claimGuid,
$changedReference, $referenceHash, $this->helper->getMockSnakValidator() );
$args[] = array ( $item, $changeOp,
$changedReference->getHash() );
// Just change a reference's index:
@@ -198,6 +216,7 @@
$claim->getGuid(),
$references[1],
$references[1]->getHash(),
+ $this->helper->getMockSnakValidator(),
0
);
$args[] = array ( $item, $changeOp, $references[1]->getHash() );
@@ -231,4 +250,62 @@
return $entity;
}
+
+
+ public function provideApplyInvalid() {
+ $p11 = new PropertyId( 'P11' );
+ $q17 = new ItemId( 'Q17' );
+
+ $item = Item::newEmpty();
+ $item->setId( $q17 );
+ $claimGuid = $this->helper->getGuidGenerator()->newGuid( $q17 );
+ $badGuid = $this->helper->getGuidGenerator()->newGuid( $q17 );
+
+ $oldSnak = new PropertyValueSnak( $p11, new StringValue( "old
reference" ) );
+ $oldReference = new Reference( new SnakList( array( $oldSnak )
) );
+
+ $claim = new Statement( new PropertyNoValueSnak( $p11 ), new
SnakList( array( $oldSnak ) ) );
+ $claim->setGuid( $claimGuid );
+ $item->addClaim( $claim );
+
+ //NOTE: the mock validator will consider the string "INVALID"
to be invalid.
+ $goodSnak = new PropertyValueSnak( $p11, new StringValue(
'good' ) );
+ $badSnak = new PropertyValueSnak( $p11, new StringValue(
'INVALID' ) );
+ $brokenSnak = new PropertyValueSnak( $p11, new NumberValue( 23
) );
+
+ $goodReference = new Reference( new SnakList( array( $goodSnak
) ) );
+ $badReference = new Reference( new SnakList( array( $badSnak )
) );
+ $brokenReference = new Reference( new SnakList( array(
$brokenSnak ) ) );
+
+ $refHash = $oldReference->getHash();
+ $badRefHash = $badReference->getHash();
+
+ $cases = array();
+ $cases['malformed claim guid'] = array( $item, 'NotAGuid',
$goodReference, '' );
+ $cases['unknown claim guid'] = array( $item, $badGuid,
$goodReference, $refHash );
+ $cases['unknown reference hash'] = array( $item, $claimGuid,
$goodReference, $badRefHash );
+
+ $cases['invalid snak value'] = array( $item, $claimGuid,
$badReference, '' );
+ $cases['invalid snak value type'] = array( $item, $claimGuid,
$brokenReference, $refHash );
+
+ return $cases;
+ }
+
+ /**
+ * @dataProvider provideApplyInvalid
+ */
+ public function testApplyInvalid( Entity $entity, $claimGuid, Reference
$reference, $referenceHash = '', $index = null ) {
+ $this->setExpectedException(
'Wikibase\ChangeOp\ChangeOpException' );
+
+ $changeOpClaim = new ChangeOpReference(
+ $claimGuid,
+ $reference,
+ $referenceHash,
+ $this->helper->getMockSnakValidator(),
+ $index
+ );
+
+ $entity = $entity->copy();
+ $changeOpClaim->apply( $entity );
+ }
}
diff --git a/repo/tests/phpunit/includes/api/SetQualifierTest.php
b/repo/tests/phpunit/includes/api/SetQualifierTest.php
index c35be8a..529ae35 100644
--- a/repo/tests/phpunit/includes/api/SetQualifierTest.php
+++ b/repo/tests/phpunit/includes/api/SetQualifierTest.php
@@ -5,6 +5,7 @@
use DataValues\StringValue;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\Item;
+use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\Property;
use Wikibase\DataModel\Snak\PropertyValueSnak;
use Wikibase\Repo\WikibaseRepo;
@@ -34,6 +35,17 @@
* @author Marius Hoch < [email protected] >
*/
class SetQualifierTest extends WikibaseApiTestCase {
+
+ public function setUp() {
+ parent::setUp();
+
+ static $hasEntities = false;
+
+ if ( !$hasEntities ) {
+ $this->initTestEntities( array( 'StringProp', 'Berlin'
) );
+ $hasEntities = true;
+ }
+ }
/**
* Creates a Snak of the given type with the given data.
@@ -189,29 +201,49 @@
}
/**
- * @dataProvider invalidClaimProvider
+ * @dataProvider invalidRequestProvider
*/
- public function testInvalidClaimGuid( $claimGuid ) {
+ public function testInvalidRequest( $itemHandle, $claimGuid,
$propertyHande, $snakType, $value, $error ) {
+ $itemId = new ItemId( EntityTestHelper::getId( $itemHandle ) );
+ $item =
WikibaseRepo::getDefaultInstance()->getEntityLookup()->getEntity( $itemId );
+
+ $propertyId = EntityTestHelper::getId( $propertyHande );
+
+ if ( $claimGuid === null ) {
+ $claims = $item->getClaims();
+
+ /* @var Claim $claim */
+ $claim = reset( $claims );
+ $claimGuid = $claim->getGuid();
+ }
+
+ if ( !is_string( $value ) ) {
+ $value = json_encode( $value );
+ }
+
$params = array(
'action' => 'wbsetqualifier',
'claim' => $claimGuid,
- 'property' => 7,
- 'snaktype' => 'value',
- 'value' => 'abc',
+ 'property' => $propertyId,
+ 'snaktype' => $snakType,
+ 'value' => $value,
);
try {
$this->doApiRequestWithToken( $params );
- $this->fail( 'Invalid claim guid did not throw an
error' );
- } catch ( UsageException $e ) {
- $this->assertEquals( 'invalid-guid',
$e->getCodeString(), 'Invalid claim guid raised correct error' );
+ $this->fail( 'Invalid request did not raise an error' );
+ } catch ( \UsageException $e ) {
+ $this->assertEquals( $error, $e->getCodeString(),
'Invalid claim guid raised correct error' );
}
}
- public function invalidClaimProvider() {
+ public function invalidRequestProvider() {
return array(
- array( 'xyz' ),
- array( 'x$y$z' )
+ 'bad guid 1' => array( 'Berlin', 'xyz', 'StringProp',
'value', 'abc', 'invalid-guid' ),
+ 'bad guid 2' => array( 'Berlin', 'x$y$z', 'StringProp',
'value', 'abc', 'invalid-guid' ),
+ 'bad guid 3' => array( 'Berlin',
'i1813$358fa2a0-4345-82b6-12a4-7b0fee494a5f', 'StringProp', 'value', 'abc',
'invalid-guid' ),
+ 'bad snak type' => array( 'Berlin', null, 'StringProp',
'alksdjf', 'abc', 'unknown_snaktype' ),
+ 'bad snak value' => array( 'Berlin', null,
'StringProp', 'value', '" "', 'invalid-snak-value' ),
);
}
diff --git a/repo/tests/phpunit/includes/api/SetReferenceTest.php
b/repo/tests/phpunit/includes/api/SetReferenceTest.php
index acdd1ad..7ae52e8 100644
--- a/repo/tests/phpunit/includes/api/SetReferenceTest.php
+++ b/repo/tests/phpunit/includes/api/SetReferenceTest.php
@@ -5,6 +5,8 @@
use DataValues\StringValue;
use FormatJson;
use UsageException;
+use Wikibase\DataModel\Claim\Claim;
+use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\Property;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Entity\Item;
@@ -44,13 +46,17 @@
$store = WikibaseRepo::getDefaultInstance()->getEntityStore();
- self::$propertyIds = array();
+ if ( !self::$propertyIds ) {
+ self::$propertyIds = array();
- for ( $i = 0; $i < 4; $i++ ) {
- $property = Property::newFromType( 'string' );
- $store->saveEntity( $property, '', $GLOBALS['wgUser'],
EDIT_NEW );
+ for ( $i = 0; $i < 4; $i++ ) {
+ $property = Property::newFromType( 'string' );
+ $store->saveEntity( $property, '',
$GLOBALS['wgUser'], EDIT_NEW );
- self::$propertyIds[] = $property->getId();
+ self::$propertyIds[] = $property->getId();
+ }
+
+ $this->initTestEntities( array( 'StringProp', 'Berlin'
) );
}
}
@@ -304,40 +310,6 @@
}
/**
- * @dataProvider invalidClaimProvider
- */
- public function testInvalidClaimGuid( $claimGuid, $snakHash, $refHash,
$expectedError ) {
- $params = array(
- 'action' => 'wbsetreference',
- 'statement' => $claimGuid,
- 'snaks' => $snakHash,
- 'reference' => $refHash,
- );
-
- try {
- $this->doApiRequestWithToken( $params );
- $this->fail( "Exception with code $expectedError
expected" );
- } catch ( UsageException $e ) {
- $this->assertEquals( $expectedError,
$e->getCodeString(), 'Error code' );
- }
- }
-
- public function invalidClaimProvider() {
- $invalidId = new PropertyId( 'P347894353458984658' );
-
- $snak = new PropertyValueSnak( $invalidId, new StringValue(
'abc') );
- $snakHash = $snak->getHash();
-
- $reference = new PropertyValueSnak( $invalidId, new
StringValue( 'def' ) );
- $refHash = $reference->getHash();
-
- return array(
- array( 'xyz', $snakHash, $refHash, 'invalid-guid' ),
- array( 'x$y$z', $snakHash, $refHash, 'invalid-guid' )
- );
- }
-
- /**
* Currently tests bad calender model
* @todo test more bad serializations...
*/
@@ -369,4 +341,55 @@
$this->doApiRequestWithToken( $params );
}
+
+ /**
+ * @dataProvider invalidRequestProvider
+ */
+ public function testInvalidRequest( $itemHandle, $claimGuid,
$referenceValue, $referenceHash, $error ) {
+ $itemId = new ItemId( EntityTestHelper::getId( $itemHandle ) );
+ $item =
WikibaseRepo::getDefaultInstance()->getEntityLookup()->getEntity( $itemId );
+
+ if ( $claimGuid === null ) {
+ $claims = $item->getClaims();
+
+ /* @var Claim $claim */
+ $claim = reset( $claims );
+ $claimGuid = $claim->getGuid();
+ }
+
+ $prop = new PropertyId( EntityTestHelper::getId( 'StringProp' )
);
+ $snak = new PropertyValueSnak( $prop, new StringValue(
$referenceValue ) );
+ $reference = new Reference( new SnakList( array( $snak ) ) );
+
+ $serializedReference = $this->serializeReference( $reference );
+
+ $params = array(
+ 'action' => 'wbsetreference',
+ 'statement' => $claimGuid,
+ 'snaks' => FormatJson::encode(
$serializedReference['snaks'] ),
+ 'snaks-order' => FormatJson::encode(
$serializedReference['snaks-order'] ),
+ );
+
+ if ( $referenceHash ) {
+ $params['reference'] = $referenceHash;
+ }
+
+ try {
+ $this->doApiRequestWithToken( $params );
+ $this->fail( 'Invalid request did not raise an error' );
+ } catch ( \UsageException $e ) {
+ $this->assertEquals( $error, $e->getCodeString(),
'Invalid claim guid raised correct error' );
+ }
+ }
+
+ public function invalidRequestProvider() {
+ return array(
+ 'bad guid 1' => array( 'Berlin', 'xyz', 'good', '',
'invalid-guid' ),
+ 'bad guid 2' => array( 'Berlin', 'x$y$z', 'good', '',
'invalid-guid' ),
+ 'bad guid 3' => array( 'Berlin',
'i1813$358fa2a0-4345-82b6-12a4-7b0fee494a5f', 'good', '', 'invalid-guid' ),
+ 'bad snak value' => array( 'Berlin', null, ' ', '',
'invalid-snak-value' ),
+ );
+ }
+
+
}
--
To view, visit https://gerrit.wikimedia.org/r/123872
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I181379984027f081d7e40c0f792d58cece656dd7
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
