jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/342451 )
Change subject: Move sitelink validation code to ChangeOpSiteLink::validate
......................................................................
Move sitelink validation code to ChangeOpSiteLink::validate
This also makes Api\SetSiteLink validate the ChangeOp related
to the requested change before applying it.
Note: this changes the error code used in the API error response.
Change-Id: I24d21605045355520675f79ff293392741051301
---
M repo/i18n/en.json
M repo/i18n/qqq.json
M repo/includes/Api/SetSiteLink.php
M repo/includes/ChangeOp/ChangeOpSiteLink.php
M repo/tests/phpunit/includes/Api/SetSiteLinkTest.php
M repo/tests/phpunit/includes/ChangeOp/ChangeOpSiteLinkTest.php
6 files changed, 39 insertions(+), 43 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/i18n/en.json b/repo/i18n/en.json
index 87c13a3..4cb4e3a 100644
--- a/repo/i18n/en.json
+++ b/repo/i18n/en.json
@@ -132,6 +132,7 @@
"wikibase-validator-illegal-file-chars": "File names are not allowed to
contain characters like colons or slashes. Only paste the file name after
\"File:\", please.",
"wikibase-validator-illegal-geo-shape-title": "Value must start with
\"Data:\" and end with \".map\". In addition title should not contain
characters like colon, hash or pipe.",
"wikibase-validator-no-such-media": "The file \"$1\" does not exist on
[https://commons.wikimedia.org/ Wikimedia Commons].",
+ "wikibase-validator-no-such-sitelink": "Could not find a sitelink to
\"$1\" when trying to edit badges.",
"wikibase-validator-page-not-exists": "The page \"$1\" does not exist.",
"wikibase-validator-not-a-language": "\"$1\" is not a known language
code.",
"wikibase-itemlink": "$1 $2",
diff --git a/repo/i18n/qqq.json b/repo/i18n/qqq.json
index 67d8370..1e4d5f2 100644
--- a/repo/i18n/qqq.json
+++ b/repo/i18n/qqq.json
@@ -164,6 +164,7 @@
"wikibase-validator-illegal-file-chars": "Validator error message shown
when a user tries to link to a Wikimedia Commons media file, but the file name
contains illegal special characters such as slashes or colons.",
"wikibase-validator-illegal-geo-shape-title": "Validator error message
shown when a user tries to link to a geo-shape page, but the page title
contains illegal special characters such as hash.",
"wikibase-validator-no-such-media": "Error message shown when a user
tries to add a link to a Wikimedia Commons media file which does not exist.
Parameters:\n* $1 is the name of the file.",
+ "wikibase-validator-no-such-sitelink": "Error message shown when a user
tries to edit a badges for the site on which the item has no sitelink.
Parameters:\n* $1 - the site id\n\nSee also:\n*
{{msg-mw|wikibase-api-no-such-sitelink}}",
"wikibase-validator-page-not-exists": "Error message shown when a user
tries to add a link to a Wiki page which does not exist. Parameters:\n* $1 is
the name of the page.",
"wikibase-validator-not-a-language": "Validation error message shown
when a user tries to use a language code that is not known to the system.\n* $1
- the invalid language code\nSee also:\n*
{{msg-mw|wikibase-wikibaserepopage-invalid-langcode}}",
"wikibase-itemlink": "{{optional}}\nused to generate the link text for
links to items. $1 is the items label, $2 the id given to
\"wikibase-itemlink-id-wrapper\". This message can be altered to change the
position of the single parts within the link. Params:\n* $1 is label on a html
form\n* $2 is id on a html form",
diff --git a/repo/includes/Api/SetSiteLink.php
b/repo/includes/Api/SetSiteLink.php
index 719372f..b2bf245 100644
--- a/repo/includes/Api/SetSiteLink.php
+++ b/repo/includes/Api/SetSiteLink.php
@@ -3,7 +3,9 @@
namespace Wikibase\Repo\Api;
use ApiMain;
+use Wikibase\ChangeOp\ChangeOpException;
use Wikibase\ChangeOp\ChangeOpSiteLink;
+use Wikibase\ChangeOp\ChangeOpValidationException;
use Wikibase\ChangeOp\SiteLinkChangeOpFactory;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\Item;
@@ -99,8 +101,14 @@
$resultBuilder->addRemovedSiteLinks( new
SiteLinkList( array( $siteLink ) ), 'entity' );
}
} else {
- if ( isset( $params['linktitle'] ) ||
$hasLinkWithSiteId ) {
+ try {
$changeOp = $this->getChangeOp( $params );
+
+ $result = $changeOp->validate( $entity );
+ if ( !$result->isValid() ) {
+ throw new ChangeOpValidationException(
$result );
+ }
+
$this->applyChangeOp( $changeOp, $entity,
$summary );
$link = $item->getSiteLinkList()->getBySiteId(
$linksite );
@@ -109,8 +117,8 @@
'entity',
true // always add the URL
);
- } else {
- $this->errorReporter->dieMessage(
'no-such-sitelink', $params['linktitle'] );
+ } catch ( ChangeOpException $ex ) {
+ $this->errorReporter->dieException( $ex,
'modification-failed' );
}
}
diff --git a/repo/includes/ChangeOp/ChangeOpSiteLink.php
b/repo/includes/ChangeOp/ChangeOpSiteLink.php
index 4cb0a0d..92f5928 100644
--- a/repo/includes/ChangeOp/ChangeOpSiteLink.php
+++ b/repo/includes/ChangeOp/ChangeOpSiteLink.php
@@ -3,6 +3,7 @@
namespace Wikibase\ChangeOp;
use InvalidArgumentException;
+use ValueValidators\Error;
use ValueValidators\Result;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\Item;
@@ -120,13 +121,6 @@
$commentArgs = array();
if ( $this->pageName === null ) {
- if ( !$siteLinks->hasLinkWithSiteId(
$this->siteId ) ) {
- // FIXME: Code in Api\EditEntity
processing the site link change request used to call
- // ApiErrorReporter::dieMessage(
'no-such-sitelink', $globalSiteId ) in this case.
- // Should the following exception
result in the similar behaviour?
- throw new InvalidArgumentException(
'The sitelink does not exist' );
- }
-
// If page name is not set (but badges are)
make sure that it remains intact
$pageName = $siteLinks->getBySiteId(
$this->siteId )->getPageName();
} else {
@@ -150,10 +144,23 @@
*
* @param EntityDocument $entity
*
- * @return Result Always successful.
+ * @throws InvalidArgumentException
+ *
+ * @return Result
*/
public function validate( EntityDocument $entity ) {
- //TODO: move validation logic from apply() here.
+ if ( !( $entity instanceof Item ) ) {
+ throw new InvalidArgumentException( 'ChangeOpSiteLink
can only be applied to Item instances' );
+ }
+
+ if ( $this->pageName === null && $this->badges !== null ) {
+ if ( !$entity->getSiteLinkList()->hasLinkWithSiteId(
$this->siteId ) ) {
+ return Result::newError( [
+ Error::newError( 'The sitelink does not
exist', null, 'no-such-sitelink', [ $this->siteId ] )
+ ] );
+ }
+ }
+
return Result::newSuccess();
}
diff --git a/repo/tests/phpunit/includes/Api/SetSiteLinkTest.php
b/repo/tests/phpunit/includes/Api/SetSiteLinkTest.php
index 4ad6c03..bc73890 100644
--- a/repo/tests/phpunit/includes/Api/SetSiteLinkTest.php
+++ b/repo/tests/phpunit/includes/Api/SetSiteLinkTest.php
@@ -317,7 +317,8 @@
),
'e' => array( 'exception' => array(
'type' => ApiUsageException::class,
- 'code' => 'no-such-sitelink'
+ 'code' => 'modification-failed',
+ 'message' => wfMessage(
'wikibase-validator-no-such-sitelink', 'svwiki' )->text(),
) )
),
);
diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpSiteLinkTest.php
b/repo/tests/phpunit/includes/ChangeOp/ChangeOpSiteLinkTest.php
index 5eaf923..34c3e4b 100644
--- a/repo/tests/phpunit/includes/ChangeOp/ChangeOpSiteLinkTest.php
+++ b/repo/tests/phpunit/includes/ChangeOp/ChangeOpSiteLinkTest.php
@@ -130,38 +130,16 @@
);
}
- public function invalidChangeOpSiteLinkProvider() {
- $deSiteLink = new SiteLink( 'dewiki', 'Berlin' );
- $plSiteLink = new SiteLink( 'plwiki', 'Berlin', array( new
ItemId( 'Q42' ) ) );
-
- $existingSitelinks = array(
- $deSiteLink,
- $plSiteLink
- );
-
- $args = array();
-
- // cannot change badges of non-existing sitelink
- $args[] = array(
- $existingSitelinks,
- new ChangeOpSiteLink( 'enwiki', null, array( new
ItemId( 'Q149' ) ) ),
- );
-
- return $args;
- }
-
- /**
- * @dataProvider invalidChangeOpSiteLinkProvider
- * @param SiteLink[] $existingSitelinks
- * @param ChangeOpSiteLink $changeOpSiteLink
- *
- * @expectedException InvalidArgumentException
- */
- public function testApplyWithInvalidData( array $existingSitelinks,
ChangeOpSiteLink $changeOpSiteLink ) {
+ public function
testGivenNoSitelinkOnSiteAndBadgeChangeRequested_validateReturnsError() {
$item = new Item();
- $item->setSiteLinkList( new SiteLinkList( $existingSitelinks )
);
+ $item->setSiteLinkList( new SiteLinkList( [ new SiteLink(
'plwiki', 'Berlin', [ new ItemId( 'Q42' ) ] ) ] ) );
- $changeOpSiteLink->apply( $item );
+ $changeOp = new ChangeOpSiteLink( 'enwiki', null, [ new ItemId(
'Q149' ) ] );
+
+ $result = $changeOp->validate( $item );
+
+ $this->assertFalse( $result->isValid() );
+ $this->assertEquals( 'no-such-sitelink',
$result->getErrors()[0]->getCode() );
}
public function summaryTestProvider() {
--
To view, visit https://gerrit.wikimedia.org/r/342451
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I24d21605045355520675f79ff293392741051301
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: WMDE-leszek <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
