jenkins-bot has submitted this change and it was merged.
Change subject: Refactor setclaim to use changeops
......................................................................
Refactor setclaim to use changeops
This is the final api modules that was not using
changeops! Now it does!
Change-Id: I80dce3af7fec3e4c922ada87646be581873267f9
---
M repo/includes/ChangeOp/ChangeOpClaim.php
M repo/includes/api/ModifyClaim.php
M repo/includes/api/SetClaim.php
M repo/tests/phpunit/includes/api/SetClaimTest.php
4 files changed, 115 insertions(+), 156 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/includes/ChangeOp/ChangeOpClaim.php
b/repo/includes/ChangeOp/ChangeOpClaim.php
index 6f3e540..0f4576e 100644
--- a/repo/includes/ChangeOp/ChangeOpClaim.php
+++ b/repo/includes/ChangeOp/ChangeOpClaim.php
@@ -4,6 +4,7 @@
use InvalidArgumentException;
use Wikibase\Claim;
+use Wikibase\Claims;
use Wikibase\Entity;
use Wikibase\Lib\ClaimGuidGenerator;
use Wikibase\Lib\ClaimGuidValidator;
@@ -65,8 +66,7 @@
if( $this->claim->getGuid() === null ){
$this->claim->setGuid( $this->guidGenerator->newGuid()
);
}
- $guid = $this->claim->getGuid();
- $guid = $guidParser->parse( $guid );
+ $guid = $guidParser->parse( $this->claim->getGuid() );
if ( $guidValidator->validate( $guid->getSerialization() ) ===
false ) {
throw new ChangeOpException( "Claim does not have a
valid GUID" );
@@ -74,8 +74,15 @@
throw new ChangeOpException( "Claim GUID invalid for
given entity" );
}
- $entity->addClaim( $this->claim );
- $this->updateSummary( $summary, 'add' );
+ $claims = new Claims( $entity->getClaims() );
+ if( $claims->hasClaimWithGuid( $guid->getSerialization() ) ){
+ $claims->removeClaimWithGuid( $guid->getSerialization()
);
+ $this->updateSummary( $summary, 'update' );
+ } else {
+ $this->updateSummary( $summary, 'create' );
+ }
+ $claims->addClaim( $this->claim );
+ $entity->setClaims( $claims );
return true;
}
diff --git a/repo/includes/api/ModifyClaim.php
b/repo/includes/api/ModifyClaim.php
index 3b83d6c..851cbc3 100644
--- a/repo/includes/api/ModifyClaim.php
+++ b/repo/includes/api/ModifyClaim.php
@@ -39,12 +39,19 @@
protected $claimGuidParser;
/**
+ * @since 0.5
+ *
+ * @var SnakValidationHelper
+ */
+ protected $snakValidation;
+
+ /**
* see ApiBase::__construct()
*/
public function __construct( ApiMain $mainModule, $moduleName,
$modulePrefix = '' ) {
parent::__construct( $mainModule, $moduleName, $modulePrefix );
- $snakValidation = new SnakValidationHelper(
+ $this->snakValidation = new SnakValidationHelper(
$this,
WikibaseRepo::getDefaultInstance()->getPropertyDataTypeLookup(),
WikibaseRepo::getDefaultInstance()->getDataTypeFactory(),
@@ -57,7 +64,7 @@
WikibaseRepo::getDefaultInstance()->getSnakConstructionService(),
WikibaseRepo::getDefaultInstance()->getEntityIdParser(),
WikibaseRepo::getDefaultInstance()->getClaimGuidValidator(),
- $snakValidation
+ $this->snakValidation
);
$this->claimGuidParser =
WikibaseRepo::getDefaultInstance()->getClaimGuidParser();
diff --git a/repo/includes/api/SetClaim.php b/repo/includes/api/SetClaim.php
index beb29ba..d29a737 100644
--- a/repo/includes/api/SetClaim.php
+++ b/repo/includes/api/SetClaim.php
@@ -2,62 +2,39 @@
namespace Wikibase\Api;
-use DataValues\IllegalValueException;
use ApiMain;
+use ApiBase;
+use MWException;
+use DataValues\IllegalValueException;
use Diff\Comparer\ComparableComparer;
use Diff\OrderedListDiffer;
-use MWException;
-use ApiBase;
+use FormatJson;
use Diff\ListDiffer;
use ValueFormatters\FormatterOptions;
-use ValueFormatters\ValueFormatter;
+use Wikibase\ChangeOp\ChangeOpClaim;
+use Wikibase\ChangeOp\ChangeOpException;
+use Wikibase\ClaimDiffer;
+use Wikibase\Claims;
+use Wikibase\ClaimSummaryBuilder;
use Wikibase\EntityContent;
use Wikibase\Claim;
use Wikibase\EntityContentFactory;
-use Wikibase\ClaimDiffer;
-use Wikibase\ClaimSaver;
-use Wikibase\ClaimSummaryBuilder;
+use Wikibase\Lib\ClaimGuidGenerator;
+use Wikibase\Lib\Serializers\SerializerFactory;
use Wikibase\Lib\SnakFormatter;
use Wikibase\Repo\WikibaseRepo;
use Wikibase\Summary;
-use Wikibase\Validators\ValidatorErrorLocalizer;
/**
* API module for creating or updating an entire Claim.
*
* @since 0.4
- *
- * @ingroup WikibaseRepo
- * @ingroup API
- *
* @licence GNU GPL v2+
* @author Jeroen De Dauw < [email protected] >
* @author Tobias Gritschacher < [email protected] >
+ * @author Adam Shorland
*/
-class SetClaim extends ApiWikibase {
-
- /**
- * @var SnakValidationHelper
- */
- protected $snakValidation;
-
- /**
- * see ApiBase::__construct()
- *
- * @param ApiMain $mainModule
- * @param string $moduleName
- * @param string $modulePrefix
- */
- public function __construct( ApiMain $mainModule, $moduleName,
$modulePrefix = '' ) {
- parent::__construct( $mainModule, $moduleName, $modulePrefix );
-
- $this->snakValidation = new SnakValidationHelper(
- $this,
-
WikibaseRepo::getDefaultInstance()->getPropertyDataTypeLookup(),
-
WikibaseRepo::getDefaultInstance()->getDataTypeFactory(),
- new ValidatorErrorLocalizer()
- );
- }
+class SetClaim extends ModifyClaim {
/**
* @see ApiBase::execute
@@ -65,95 +42,79 @@
* @since 0.4
*/
public function execute() {
- $claim = $this->getClaimFromRequest();
-
+ $params = $this->extractRequestParams();
+ $claim = $this->getClaimFromParams( $params );
$this->snakValidation->validateClaimSnaks( $claim );
- $claimDiffer = new ClaimDiffer( new OrderedListDiffer( new
ComparableComparer() ) );
+ $guid = $claim->getGuid();
+ if( $guid === null ){
+ $this->dieUsage( 'GUID must be set when setting a
claim', 'invalid-claim' );
+ }
+ $guid = $this->claimGuidParser->parse( $guid );
- $options = new FormatterOptions( array(
- //TODO: fallback chain
- ValueFormatter::OPT_LANG =>
$this->getContext()->getLanguage()->getCode()
- ) );
+ $entityId = $guid->getEntityId();
+ $entityContentFactory =
WikibaseRepo::getDefaultInstance()->getEntityContentFactory();
+ $entityContent = $entityContentFactory->getFromId( $entityId );
+ $entity = $entityContent->getEntity();
+ $summary = $this->getSummary( $params, $claim, $entityContent );
- $claimSummaryBuilder = new ClaimSummaryBuilder(
- $this->getModuleName(),
- $claimDiffer,
-
WikibaseRepo::getDefaultInstance()->getSnakFormatterFactory()->getSnakFormatter(
SnakFormatter::FORMAT_PLAIN, $options )
- );
- $claimSaver = new ClaimSaver();
-
- $params = $this->extractRequestParams();
-
- $baseRevisionId = isset( $params['baserevid'] ) ? intval(
$params['baserevid'] ) : null;
- $token = isset( $params['token'] ) ? $params['token'] : '';
-
- $user = $this->getUser();
- $flags = ( $user->isAllowed( 'bot' ) && $params['bot'] ) ?
EDIT_FORCE_BOT : 0;
-
- $newRevisionId = null;
-
- $status = $claimSaver->saveClaim(
- $claim,
- $baseRevisionId,
- $token,
- $user,
- $claimSummaryBuilder,
- $flags
- );
- $this->handleSaveStatus( $status ); // die on error, report
warnings, etc
-
- $statusValue = $status->getValue();
- $newRevisionId = isset( $statusValue['revision'] ) ?
$statusValue['revision']->getId() : null;
-
- if ( $newRevisionId !== null ) {
- $this->getResult()->addValue( null, 'success', 1 );
- $this->getResult()->addValue(
- 'pageinfo',
- 'lastrevid',
- $newRevisionId
- );
+ $changeop = new ChangeOpClaim( $claim , new ClaimGuidGenerator(
$guid->getEntityId() ) );
+ try{
+ $changeop->apply( $entity, $summary );
+ } catch( ChangeOpException $exception ){
+ $this->dieUsage( 'Failed to apply changeOp:' .
$exception->getMessage(), 'save-failed' );
}
- $this->outputClaim( $claim );
+ $this->saveChanges( $entityContent, $summary );
+ $this->claimModificationHelper->addClaimToApiResult( $claim );
+ }
+
+ /**
+ * @param array $params
+ * @param Claim $claim
+ * @param EntityContent $entityContent
+ * @return Summary
+ * @todo this summary builder is ugly and summary stuff needs to be
refactored
+ */
+ protected function getSummary( array $params, Claim $claim,
EntityContent $entityContent ){
+ $claimSummaryBuilder = new ClaimSummaryBuilder(
+ $this->getModuleName(),
+ new ClaimDiffer( new OrderedListDiffer( new
ComparableComparer() ) ),
+
WikibaseRepo::getDefaultInstance()->getSnakFormatterFactory()->getSnakFormatter(
+ SnakFormatter::FORMAT_PLAIN,
+ new FormatterOptions()
+ )
+ );
+ $summary = $claimSummaryBuilder->buildClaimSummary(
+ new Claims( $entityContent->getEntity()->getClaims() ),
+ $claim
+ );
+ if ( isset( $params['summary'] ) ) {
+ $summary->setUserSummary( $params['summary'] );
+ }
+ return $summary;
}
/**
* @since 0.4
- *
+ * @param array $params
* @return Claim
*/
- protected function getClaimFromRequest() {
- $serializerFactory = new
\Wikibase\Lib\Serializers\SerializerFactory();
+ protected function getClaimFromParams( array $params ) {
+ $serializerFactory = new SerializerFactory();
$unserializer = $serializerFactory->newUnserializerForClass(
'Wikibase\Claim' );
- $params = $this->extractRequestParams();
-
try {
- $claim = $unserializer->newFromSerialization(
\FormatJson::decode( $params['claim'], true ) );
-
- assert( $claim instanceof Claim );
+ $claim = $unserializer->newFromSerialization(
FormatJson::decode( $params['claim'], true ) );
+ if( !$claim instanceof Claim ) {
+ $this->dieUsage( 'Failed to get claim from
claim Serialization', 'invalid-claim' );
+ }
return $claim;
} catch ( IllegalValueException $illegalValueException ) {
$this->dieUsage( $illegalValueException->getMessage(),
'invalid-claim' );
+ } catch( MWException $mwException ) {
+ $this->dieUsage( 'Failed to get claim from claim
Serialization ' . $mwException->getMessage(), 'invalid-claim' );
}
- }
-
- /**
- * @since 0.4
- *
- * @param Claim $claim
- */
- protected function outputClaim( Claim $claim ) {
- $serializerFactory = new
\Wikibase\Lib\Serializers\SerializerFactory();
- $serializer = $serializerFactory->newSerializerForObject(
$claim );
- $serializer->getOptions()->setIndexTags(
$this->getResult()->getIsRawMode() );
-
- $this->getResult()->addValue(
- null,
- 'claim',
- $serializer->getSerialized( $claim )
- );
}
/**
@@ -164,16 +125,14 @@
* @return array
*/
public function getAllowedParams() {
- return array(
- 'claim' => array(
- ApiBase::PARAM_TYPE => 'string',
- ApiBase::PARAM_REQUIRED => true,
+ return array_merge(
+ array(
+ 'claim' => array(
+ ApiBase::PARAM_TYPE => 'string',
+ ApiBase::PARAM_REQUIRED => true
+ )
),
- 'token' => null,
- 'baserevid' => array(
- ApiBase::PARAM_TYPE => 'integer',
- ),
- 'bot' => false,
+ parent::getAllowedParams()
);
}
@@ -194,15 +153,11 @@
* @return array
*/
public function getParamDescription() {
- return array(
- 'claim' => 'Claim serialization',
- 'token' => 'An "edittoken" token previously obtained
through the token module (prop=info).',
- 'baserevid' => array( 'The numeric identifier for the
revision to base the modification on.',
- "This is used for detecting conflicts during
save."
- ),
- 'bot' => array( 'Mark this edit as bot',
- 'This URL flag will only be respected if the
user belongs to the group "bot".'
- ),
+ return array_merge(
+ parent::getParamDescription(),
+ array(
+ 'claim' => 'Claim serialization'
+ )
);
}
@@ -231,14 +186,6 @@
'api.php?action=wbsetclaim&claim={"id":"Q2$5627445f-43cb-ed6d-3adb-760e85bd17ee","type":"claim","mainsnak":{"snaktype":"value","property":"P1","datavalue":{"value":"City","type":"string"}}}'
=> 'Set the claim with the given id to property P1 with
a string value of "City',
);
- }
-
- /**
- * @see ApiBase::isWriteMode
- * @return bool true
- */
- public function isWriteMode() {
- return true;
}
}
diff --git a/repo/tests/phpunit/includes/api/SetClaimTest.php
b/repo/tests/phpunit/includes/api/SetClaimTest.php
index dd09006..383118f 100644
--- a/repo/tests/phpunit/includes/api/SetClaimTest.php
+++ b/repo/tests/phpunit/includes/api/SetClaimTest.php
@@ -2,6 +2,7 @@
namespace Wikibase\Test\Api;
+use FormatJson;
use Wikibase\Claim;
use Wikibase\Claims;
use Wikibase\DataModel\Entity\PropertyId;
@@ -126,7 +127,7 @@
public function testAddClaim( Claim $claim ) {
$item = Item::newEmpty();
$content = new ItemContent( $item );
- $content->save( '', null, EDIT_NEW );
+ $content->save( 'setclaimtest', null, EDIT_NEW );
$guidGenerator = new ClaimGuidGenerator( $item->getId() );
$guid = $guidGenerator->newGuid();
@@ -134,7 +135,7 @@
$claim->setGuid( $guid );
// Addition request
- $this->makeRequest( $claim, $item->getId(), 1 );
+ $this->makeRequest( $claim, $item->getId(), 1, 'addition
request' );
// Reorder qualifiers
if( count( $claim->getQualifiers() ) > 0 ) {
@@ -146,54 +147,51 @@
$serializedClaim = $serializer->getSerialized( $claim );
$firstPropertyId = array_shift(
$serializedClaim['qualifiers-order'] );
array_push( $serializedClaim['qualifiers-order'],
$firstPropertyId );
- $this->makeRequest( $serializedClaim, $item->getId(), 1
);
+ $this->makeRequest( $serializedClaim, $item->getId(),
1, 'reorder qualifiers' );
}
$claim = new Statement( new PropertyNoValueSnak( 9001 ) );
$claim->setGuid( $guid );
// Update request
- $this->makeRequest( $claim, $item->getId(), 1 );
+ $this->makeRequest( $claim, $item->getId(), 1, 'update request'
);
}
/**
* @param Claim|array $claim Native or serialized claim object.
* @param EntityId $entityId
* @param $claimCount
+ * @param $requestLabel string a label to identify requests that are
made in errors
*/
- protected function makeRequest( $claim, EntityId $entityId, $claimCount
) {
+ protected function makeRequest( $claim, EntityId $entityId,
$claimCount, $requestLabel ) {
$serializerFactory = new SerializerFactory();
if( is_a( $claim, '\Wikibase\Claim' ) ) {
- $serializer =
$serializerFactory->newSerializerForObject( $claim );
- $serializedClaim = $serializer->getSerialized( $claim );
+ $unserializer =
$serializerFactory->newSerializerForObject( $claim );
+ $serializedClaim = $unserializer->getSerialized( $claim
);
} else {
- $serializer =
$serializerFactory->newUnserializerForClass( 'Wikibase\Claim' );
+ $unserializer =
$serializerFactory->newUnserializerForClass( 'Wikibase\Claim' );
$serializedClaim = $claim;
- $claim = $serializer->newFromSerialization(
$serializedClaim );
+ $claim = $unserializer->newFromSerialization(
$serializedClaim );
}
- $params = array(
+ $this->makeValidRequest( array(
'action' => 'wbsetclaim',
- 'claim' => \FormatJson::encode( $serializedClaim ),
- );
-
- $this->makeValidRequest( $params );
+ 'claim' => FormatJson::encode( $serializedClaim ),
+ ) );
$content =
WikibaseRepo::getDefaultInstance()->getEntityContentFactory()->getFromId(
$entityId );
-
$this->assertInstanceOf( '\Wikibase\EntityContent', $content );
$claims = new Claims( $content->getEntity()->getClaims() );
-
- $this->assertTrue( $claims->hasClaim( $claim ) );
+ $this->assertTrue( $claims->hasClaim( $claim ), "Claims list
does not have claim after {$requestLabel}" );
$savedClaim = $claims->getClaimWithGuid( $claim->getGuid() );
if( count( $claim->getQualifiers() ) ) {
$this->assertArrayEquals(
$claim->getQualifiers()->toArray(), $savedClaim->getQualifiers()->toArray(),
true );
}
- $this->assertEquals( $claimCount, $claims->count() );
+ $this->assertEquals( $claimCount, $claims->count(), "Claims
count is wrong after {$requestLabel}" );
}
protected function makeValidRequest( array $params ) {
--
To view, visit https://gerrit.wikimedia.org/r/88017
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I80dce3af7fec3e4c922ada87646be581873267f9
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[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
