jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/331882 )
Change subject: Move ChangeOp initialization logic to entity type definitions
......................................................................
Move ChangeOp initialization logic to entity type definitions
Repo's entity type definitions now contain field
"changeop-deserializer-callback" defining a callback
turning a change request array into ChangeOp object
that can be applied on entities of the given type.
These callbacks are used by EntityChangeOpProvider
service.
Api\EditEntity no longer contains logic related to
creating ChangeOp object from the "serialized" change
request data.
As some of errors related to constructing and applying
ChangeOps have been reported in the API as translatable
messages, ChangeOpDeserializationExceptionLocalizer
has been introduced. Localizer uses exception's
"error code" to build a i18n message key, and if such
message exists, localized messaged is used. Otherwise
exception's descriptive message in English is used instead.
ChangeOpDeserializationException's constructor has been
extended to have optional argument allowing to pass in
i18n message parameters.
This solution is not ideal but it should keep previously
translated error messages localized also with the new code.
Bug: T154288
Change-Id: I2977607b4b02bc56183cfa5a9a9a0dc57432964d
---
M docs/extending-entities.wiki
M repo/WikibaseRepo.entitytypes.php
M repo/includes/Api/EditEntity.php
M repo/includes/ChangeOp/Deserialization/ChangeOpDeserializationException.php
M repo/includes/ChangeOp/Deserialization/SiteLinksChangeOpDeserializer.php
A repo/includes/Localizer/ChangeOpDeserializationExceptionLocalizer.php
M repo/includes/WikibaseRepo.php
M repo/tests/phpunit/includes/Api/EditEntityTest.php
M
repo/tests/phpunit/includes/ChangeOp/Deserialization/ChangeOpDeserializationExceptionTest.php
M
repo/tests/phpunit/includes/ChangeOp/Deserialization/SiteLinksChangeOpDeserializerTest.php
A
repo/tests/phpunit/includes/Localizer/ChangeOpDeserializationExceptionLocalizerTest.php
M repo/tests/phpunit/includes/WikibaseRepoTest.php
12 files changed, 225 insertions(+), 408 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/docs/extending-entities.wiki b/docs/extending-entities.wiki
index 71b04fa..df4f27d 100644
--- a/docs/extending-entities.wiki
+++ b/docs/extending-entities.wiki
@@ -12,7 +12,8 @@
* Add support to FooPatcher
* Add support to FooDiffer
* Add support in FooView (extends EntityView)
-* Add a ChangeOp for the new field, e.g. NewThingyChangeOp
+* Add a ChangeOp for the new field, e.g. NewThingyChangeOp, and the related
ChangeOpDeserializer,
+ e.g. NewThingyChangeOpDeserializer
* Add support in FooValidatorFactory
* Add support in FooChangeOpDeserializer
diff --git a/repo/WikibaseRepo.entitytypes.php
b/repo/WikibaseRepo.entitytypes.php
index 1fbe0d2..18bb53d 100644
--- a/repo/WikibaseRepo.entitytypes.php
+++ b/repo/WikibaseRepo.entitytypes.php
@@ -20,6 +20,8 @@
use Wikibase\DataModel\Entity\Property;
use Wikibase\DataModel\Services\Lookup\LabelDescriptionLookup;
use Wikibase\LanguageFallbackChain;
+use Wikibase\Repo\ChangeOp\Deserialization\ItemChangeOpDeserializer;
+use Wikibase\Repo\ChangeOp\Deserialization\PropertyChangeOpDeserializer;
use Wikibase\Repo\WikibaseRepo;
use Wikibase\View\EditSectionGenerator;
use Wikibase\View\EntityTermsView;
@@ -50,6 +52,11 @@
'entity-factory-callback' => function() {
return new Item();
},
+ 'changeop-deserializer-callback' => function() {
+ return new ItemChangeOpDeserializer(
+
WikibaseRepo::getDefaultInstance()->getChangeOpDeserializerFactory()
+ );
+ },
),
'property' => array(
'view-factory-callback' => function(
@@ -76,5 +83,10 @@
'entity-factory-callback' => function() {
return Property::newFromType( '' );
},
+ 'changeop-deserializer-callback' => function() {
+ return new PropertyChangeOpDeserializer(
+
WikibaseRepo::getDefaultInstance()->getChangeOpDeserializerFactory()
+ );
+ },
)
);
diff --git a/repo/includes/Api/EditEntity.php b/repo/includes/Api/EditEntity.php
index 730955d..02d63a6 100644
--- a/repo/includes/Api/EditEntity.php
+++ b/repo/includes/Api/EditEntity.php
@@ -4,14 +4,11 @@
use ApiMain;
use Deserializers\Deserializer;
-use Exception;
use InvalidArgumentException;
use MWException;
-use SiteList;
use Title;
use ApiUsageException;
use Wikibase\ChangeOp\ChangeOp;
-use Wikibase\ChangeOp\ChangeOps;
use Wikibase\ChangeOp\FingerprintChangeOpFactory;
use Wikibase\ChangeOp\SiteLinkChangeOpFactory;
use Wikibase\ChangeOp\StatementChangeOpFactory;
@@ -20,7 +17,6 @@
use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\Property;
-use Wikibase\DataModel\Statement\Statement;
use Wikibase\DataModel\Statement\StatementListProvider;
use Wikibase\DataModel\Term\AliasesProvider;
use Wikibase\DataModel\Term\DescriptionsProvider;
@@ -28,6 +24,8 @@
use Wikibase\EntityFactory;
use Wikibase\Lib\ContentLanguages;
use Wikibase\Lib\Store\EntityRevisionLookup;
+use Wikibase\Repo\ChangeOp\Deserialization\ChangeOpDeserializationException;
+use Wikibase\Repo\ChangeOp\EntityChangeOpProvider;
use Wikibase\Repo\WikibaseRepo;
use Wikibase\Summary;
@@ -80,6 +78,11 @@
private $entityFactory;
/**
+ * @var EntityChangeOpProvider
+ */
+ private $entityChangeOpProvider;
+
+ /**
* @var callable[]
*/
private $changeOpDeserializerCallbacks;
@@ -107,7 +110,7 @@
$this->termChangeOpFactory =
$changeOpFactoryProvider->getFingerprintChangeOpFactory();
$this->statementChangeOpFactory =
$changeOpFactoryProvider->getStatementChangeOpFactory();
$this->siteLinkChangeOpFactory =
$changeOpFactoryProvider->getSiteLinkChangeOpFactory();
- $this->changeOpDeserializerCallbacks =
$wikibaseRepo->getChangeOpDeserializerCallbacks();
+ $this->entityChangeOpProvider =
$wikibaseRepo->getEntityChangeOpProvider();
}
/**
@@ -289,299 +292,11 @@
* @return ChangeOp
*/
private function getChangeOp( array $changeRequest, EntityDocument
$entity ) {
- $type = $entity->getType();
- if ( isset( $this->changeOpDeserializerCallbacks[$type] ) ) {
- $changeOpDeserializer = call_user_func(
$this->changeOpDeserializerCallbacks[$type] );
- return $changeOpDeserializer->createEntityChangeOp(
$changeRequest );
+ try {
+ return
$this->entityChangeOpProvider->newEntityChangeOp( $entity->getType(),
$changeRequest );
+ } catch ( ChangeOpDeserializationException $exception ) {
+ $this->errorReporter->dieException( $exception,
$exception->getErrorCode() );
}
-
- $changeOps = new ChangeOps();
-
- //FIXME: Use a ChangeOpBuilder so we can batch fingerprint ops
etc,
- // for more efficient validation!
-
- if ( array_key_exists( 'labels', $changeRequest ) ) {
- if ( !( $entity instanceof LabelsProvider ) ) {
- $this->errorReporter->dieError( 'The given
entity cannot contain labels', 'not-supported' );
- }
- $this->assertArray( $changeRequest['labels'], 'List of
labels must be an array' );
- $changeOps->add( $this->getLabelChangeOps(
$changeRequest['labels'] ) );
- }
-
- if ( array_key_exists( 'descriptions', $changeRequest ) ) {
- if ( !( $entity instanceof DescriptionsProvider ) ) {
- $this->errorReporter->dieError( 'The given
entity cannot contain descriptions', 'not-supported' );
- }
- $this->assertArray( $changeRequest['descriptions'],
'List of descriptions must be an array' );
- $changeOps->add( $this->getDescriptionChangeOps(
$changeRequest['descriptions'] ) );
- }
-
- if ( array_key_exists( 'aliases', $changeRequest ) ) {
- if ( !( $entity instanceof AliasesProvider ) ) {
- $this->errorReporter->dieError( 'The given
entity cannot contain aliases', 'not-supported' );
- }
- $this->assertArray( $changeRequest['aliases'], 'List of
aliases must be an array' );
- $changeOps->add( $this->getAliasesChangeOps(
$changeRequest['aliases'] ) );
- }
-
- if ( array_key_exists( 'sitelinks', $changeRequest ) ) {
- if ( !( $entity instanceof Item ) ) {
- $this->errorReporter->dieError( 'Non Items
cannot have sitelinks', 'not-supported' );
- }
- $this->assertArray( $changeRequest['sitelinks'], 'List
of sitelinks must be an array' );
- $changeOps->add( $this->getSiteLinksChangeOps(
$changeRequest['sitelinks'], $entity ) );
- }
-
- if ( array_key_exists( 'claims', $changeRequest ) ) {
- if ( !( $entity instanceof StatementListProvider ) ) {
- $this->errorReporter->dieError( 'The given
entity cannot contain statements', 'not-supported' );
- }
- $this->assertArray( $changeRequest['claims'], 'List of
claims must be an array' );
- $changeOps->add( $this->getClaimsChangeOps(
$changeRequest['claims'] ) );
- }
-
- return $changeOps;
- }
-
- /**
- * @param array[] $labels
- *
- * @return ChangeOp[]
- */
- private function getLabelChangeOps( array $labels ) {
- $labelChangeOps = array();
-
- foreach ( $labels as $langCode => $arg ) {
- $this->validateMultilangArgs( $arg, $langCode );
-
- $language = $arg['language'];
- $newLabel = ( array_key_exists( 'remove', $arg ) ? '' :
- $this->stringNormalizer->trimToNFC(
$arg['value'] ) );
-
- if ( $newLabel === "" ) {
- $labelChangeOps[] =
$this->termChangeOpFactory->newRemoveLabelOp( $language );
- } else {
- $labelChangeOps[] =
$this->termChangeOpFactory->newSetLabelOp( $language, $newLabel );
- }
- }
-
- return $labelChangeOps;
- }
-
- /**
- * @param array[] $descriptions
- *
- * @return ChangeOp[]
- */
- private function getDescriptionChangeOps( array $descriptions ) {
- $descriptionChangeOps = array();
-
- foreach ( $descriptions as $langCode => $arg ) {
- $this->validateMultilangArgs( $arg, $langCode );
-
- $language = $arg['language'];
- $newDescription = ( array_key_exists( 'remove', $arg )
? '' :
- $this->stringNormalizer->trimToNFC(
$arg['value'] ) );
-
- if ( $newDescription === "" ) {
- $descriptionChangeOps[] =
$this->termChangeOpFactory->newRemoveDescriptionOp( $language );
- } else {
- $descriptionChangeOps[] =
$this->termChangeOpFactory->newSetDescriptionOp( $language, $newDescription );
- }
- }
-
- return $descriptionChangeOps;
- }
-
- /**
- * @param array[] $aliases
- *
- * @return ChangeOp[]
- */
- private function getAliasesChangeOps( array $aliases ) {
- $indexedAliases = $this->getIndexedAliases( $aliases );
- $aliasesChangeOps = $this->getIndexedAliasesChangeOps(
$indexedAliases );
-
- return $aliasesChangeOps;
- }
-
- /**
- * @param array[] $aliases
- *
- * @return array[]
- */
- private function getIndexedAliases( array $aliases ) {
- $indexedAliases = array();
-
- foreach ( $aliases as $langCode => $arg ) {
- if ( !is_string( $langCode ) ) {
- $indexedAliases[] = ( array_values( $arg ) ===
$arg ) ? $arg : array( $arg );
- } else {
- $indexedAliases[$langCode] = ( array_values(
$arg ) === $arg ) ? $arg : array( $arg );
- }
- }
-
- return $indexedAliases;
- }
-
- /**
- * @param array[] $indexedAliases
- *
- * @return ChangeOp[]
- */
- private function getIndexedAliasesChangeOps( array $indexedAliases ) {
- $aliasesChangeOps = array();
- foreach ( $indexedAliases as $langCode => $args ) {
- $aliasesToSet = array();
- $language = '';
-
- foreach ( $args as $arg ) {
- $this->validateMultilangArgs( $arg, $langCode );
-
- $alias = array(
$this->stringNormalizer->trimToNFC( $arg['value'] ) );
- $language = $arg['language'];
-
- if ( array_key_exists( 'remove', $arg ) ) {
- $aliasesChangeOps[] =
$this->termChangeOpFactory->newRemoveAliasesOp( $language, $alias );
- } elseif ( array_key_exists( 'add', $arg ) ) {
- $aliasesChangeOps[] =
$this->termChangeOpFactory->newAddAliasesOp( $language, $alias );
- } else {
- $aliasesToSet[] = $alias[0];
- }
- }
-
- if ( $aliasesToSet !== array() ) {
- $aliasesChangeOps[] =
$this->termChangeOpFactory->newSetAliasesOp( $language, $aliasesToSet );
- }
- }
-
- return $aliasesChangeOps;
- }
-
- /**
- * @param array[] $siteLinks
- * @param Item $item
- *
- * @return ChangeOp[]
- */
- private function getSiteLinksChangeOps( array $siteLinks, Item $item ) {
- $siteLinksChangeOps = array();
- $sites = $this->siteLinkTargetProvider->getSiteList(
$this->siteLinkGroups );
-
- foreach ( $siteLinks as $siteId => $arg ) {
- $this->checkSiteLinks( $arg, $siteId, $sites );
- $globalSiteId = $arg['site'];
-
- if ( !$sites->hasSite( $globalSiteId ) ) {
- $this->errorReporter->dieError( "There is no
site for global site id '$globalSiteId'", 'no-such-site' );
- }
-
- $linkSite = $sites->getSite( $globalSiteId );
- $shouldRemove = array_key_exists( 'remove', $arg )
- || ( !isset( $arg['title'] ) && !isset(
$arg['badges'] ) )
- || ( isset( $arg['title'] ) && $arg['title']
=== '' );
-
- if ( $shouldRemove ) {
- $siteLinksChangeOps[] =
$this->siteLinkChangeOpFactory->newRemoveSiteLinkOp( $globalSiteId );
- } else {
- $badges = ( isset( $arg['badges'] ) )
- ? $this->parseSiteLinkBadges(
$arg['badges'] )
- : null;
-
- if ( isset( $arg['title'] ) ) {
- $linkPage =
$linkSite->normalizePageName( $this->stringNormalizer->trimWhitespace(
$arg['title'] ) );
-
- if ( $linkPage === false ) {
-
$this->errorReporter->dieMessage(
- 'no-external-page',
- $globalSiteId,
- $arg['title']
- );
- }
- } else {
- $linkPage = null;
-
- if (
!$item->getSiteLinkList()->hasLinkWithSiteId( $globalSiteId ) ) {
-
$this->errorReporter->dieMessage( 'no-such-sitelink', $globalSiteId );
- }
- }
-
- $siteLinksChangeOps[] =
$this->siteLinkChangeOpFactory->newSetSiteLinkOp( $globalSiteId, $linkPage,
$badges );
- }
- }
-
- return $siteLinksChangeOps;
- }
-
- /**
- * @param array[] $claims
- *
- * @return ChangeOp[]
- */
- private function getClaimsChangeOps( array $claims ) {
- $changeOps = array();
-
- //check if the array is associative or in arrays by property
- if ( array_keys( $claims ) !== range( 0, count( $claims ) - 1 )
) {
- foreach ( $claims as $subClaims ) {
- $changeOps = array_merge( $changeOps,
- $this->getRemoveStatementChangeOps(
$subClaims ),
- $this->getModifyStatementChangeOps(
$subClaims ) );
- }
- } else {
- $changeOps = array_merge( $changeOps,
- $this->getRemoveStatementChangeOps( $claims ),
- $this->getModifyStatementChangeOps( $claims ) );
- }
-
- return $changeOps;
- }
-
- /**
- * @param array[] $statements array of serialized statements
- *
- * @return ChangeOp[]
- */
- private function getModifyStatementChangeOps( array $statements ) {
- $opsToReturn = array();
-
- foreach ( $statements as $statementArray ) {
- if ( !array_key_exists( 'remove', $statementArray ) ) {
- try {
- $statement =
$this->statementDeserializer->deserialize( $statementArray );
-
- if ( !( $statement instanceof Statement
) ) {
- throw new Exception( 'Statement
serialization did not contained a Statement.' );
- }
-
- $opsToReturn[] =
$this->statementChangeOpFactory->newSetStatementOp( $statement );
- } catch ( Exception $ex ) {
- $this->errorReporter->dieException(
$ex, 'invalid-claim' );
- }
- }
- }
- return $opsToReturn;
- }
-
- /**
- * Get changeops that remove all claims that have the 'remove' key in
the array
- *
- * @param array[] $claims array of serialized claims
- *
- * @return ChangeOp[]
- */
- private function getRemoveStatementChangeOps( array $claims ) {
- $opsToReturn = array();
- foreach ( $claims as $claimArray ) {
- if ( array_key_exists( 'remove', $claimArray ) ) {
- if ( array_key_exists( 'id', $claimArray ) ) {
- $opsToReturn[] =
$this->statementChangeOpFactory->newRemoveStatementOp( $claimArray['id'] );
- } else {
- $this->errorReporter->dieError( 'Cannot
remove a claim with no GUID', 'invalid-claim' );
- }
- }
- }
- return $opsToReturn;
}
/**
@@ -833,75 +548,6 @@
. '"type":"statement","rank":"normal"}]}'
=> 'apihelp-wbeditentity-example-10',
);
- }
-
- /**
- * Check some of the supplied data for multilang arg
- *
- * @param string[] $arg The argument array to verify
- * @param int|string $langCode The language code used in the value part
- */
- private function validateMultilangArgs( $arg, $langCode ) {
- $this->assertArray( $arg, 'An array was expected, but not found
in the json for the '
- . "langCode $langCode" );
-
- if ( !array_key_exists( 'language', $arg ) ) {
- $this->errorReporter->dieError(
- "'language' was not found in the label or
description json for $langCode",
- 'missing-language' );
- }
-
- $this->assertString( $arg['language'], 'A string was expected,
but not found in the json '
- . "for the langCode $langCode and argument 'language'"
);
- if ( !is_numeric( $langCode ) ) {
- if ( $langCode !== $arg['language'] ) {
- $this->errorReporter->dieError(
- "inconsistent language: $langCode is
not equal to {$arg['language']}",
- 'inconsistent-language' );
- }
- }
-
- if ( !$this->termsLanguages->hasLanguage( $arg['language'] ) ) {
- $this->errorReporter->dieError( 'Unknown language: ' .
$arg['language'], 'not-recognized-language' );
- }
-
- if ( !array_key_exists( 'remove', $arg ) ) {
- $this->assertString( $arg['value'], 'A string was
expected, but not found in the json '
- . "for the langCode $langCode and argument
'value'" );
- }
- }
-
- /**
- * Check some of the supplied data for sitelink arg
- *
- * @param array $arg The argument array to verify
- * @param string $siteCode The site code used in the argument
- * @param SiteList|null $sites The valid sites.
- */
- private function checkSiteLinks( $arg, $siteCode, SiteList &$sites =
null ) {
- $this->assertArray( $arg, 'An array was expected, but not
found' );
- $this->assertString( $arg['site'], 'A string was expected, but
not found' );
-
- if ( !is_numeric( $siteCode ) ) {
- if ( $siteCode !== $arg['site'] ) {
- $this->errorReporter->dieError( "inconsistent
site: $siteCode is not equal to {$arg['site']}", 'inconsistent-site' );
- }
- }
-
- if ( $sites !== null && !$sites->hasSite( $arg['site'] ) ) {
- $this->errorReporter->dieError( 'Unknown site: ' .
$arg['site'], 'not-recognized-site' );
- }
-
- if ( isset( $arg['title'] ) ) {
- $this->assertString( $arg['title'], 'A string was
expected, but not found' );
- }
-
- if ( isset( $arg['badges'] ) ) {
- $this->assertArray( $arg['badges'], 'Badges: an array
was expected, but not found' );
- foreach ( $arg['badges'] as $badge ) {
- $this->assertString( $badge, 'Badges: a string
was expected, but not found' );
- }
- }
}
/**
diff --git
a/repo/includes/ChangeOp/Deserialization/ChangeOpDeserializationException.php
b/repo/includes/ChangeOp/Deserialization/ChangeOpDeserializationException.php
index 3c56da7..4ff83ca 100644
---
a/repo/includes/ChangeOp/Deserialization/ChangeOpDeserializationException.php
+++
b/repo/includes/ChangeOp/Deserialization/ChangeOpDeserializationException.php
@@ -15,17 +15,28 @@
private $errorCode;
/**
+ * @var array
+ */
+ private $params;
+
+ /**
* @param string $message - descriptive error message to be used in logs
* @param string $errorCode - i18n code of the error message
+ * @param array $params - optional parameters (e.g. i18n message
arguments)
*/
- public function __construct( $message, $errorCode ) {
+ public function __construct( $message, $errorCode, array $params = [] )
{
parent::__construct( $message );
$this->errorCode = $errorCode;
+ $this->params = $params;
}
public function getErrorCode() {
return $this->errorCode;
}
+ public function getParams() {
+ return $this->params;
+ }
+
}
diff --git
a/repo/includes/ChangeOp/Deserialization/SiteLinksChangeOpDeserializer.php
b/repo/includes/ChangeOp/Deserialization/SiteLinksChangeOpDeserializer.php
index db341ad..6b1e633 100644
--- a/repo/includes/ChangeOp/Deserialization/SiteLinksChangeOpDeserializer.php
+++ b/repo/includes/ChangeOp/Deserialization/SiteLinksChangeOpDeserializer.php
@@ -110,14 +110,10 @@
$linkPage =
$linkSite->normalizePageName( $this->stringNormalizer->trimWhitespace(
$serialization['title'] ) );
if ( $linkPage === false ) {
- // FIXME: in this case API
class is supposed to print i18n-ed version of 'no-external-page' message,
- // using $globalSiteId and
$serialization['title'] as arguments. How this should be achieved
- // now with Api\EditEntity
using deserializers throwing ChangeOpDeserializationExceptions?
throw new
ChangeOpDeserializationException(
'A page "' .
$serialization['title'] . '" could not be found on "' . $globalSiteId . '"',
'no-external-page',
- $globalSiteId,
- $serialization['title']
+ [ $globalSiteId,
$serialization['title'] ]
);
}
} else {
diff --git
a/repo/includes/Localizer/ChangeOpDeserializationExceptionLocalizer.php
b/repo/includes/Localizer/ChangeOpDeserializationExceptionLocalizer.php
new file mode 100644
index 0000000..95cb301
--- /dev/null
+++ b/repo/includes/Localizer/ChangeOpDeserializationExceptionLocalizer.php
@@ -0,0 +1,53 @@
+<?php
+
+
+namespace Wikibase\Repo\Localizer;
+
+use Exception;
+use InvalidArgumentException;
+use Message;
+use Wikibase\Repo\ChangeOp\Deserialization\ChangeOpDeserializationException;
+
+/**
+ * Localizes ChangeOpDeserializationExceptions.
+ * NOTE: Only exceptions with error codes that prepended with "wikibase-api"
form the i18n message key are localized.
+ *
+ * @license GPL-2.0+
+ */
+class ChangeOpDeserializationExceptionLocalizer implements ExceptionLocalizer {
+
+ /**
+ * @see ExceptionLocalizer::hasExceptionMessage()
+ *
+ * @param Exception $exception
+ *
+ * @return bool
+ */
+ public function hasExceptionMessage( Exception $exception ) {
+ if ( !$exception instanceof ChangeOpDeserializationException ) {
+ return false;
+ }
+
+ $message = new Message( 'wikibase-api-' .
$exception->getErrorCode() );
+
+ return $message->exists();
+ }
+
+ /**
+ * @see ExceptionLocalizer::getExceptionMessage()
+ *
+ * @param Exception $exception
+ *
+ * @return Message
+ * @throws InvalidArgumentException
+ */
+ public function getExceptionMessage( Exception $exception ) {
+ if ( !$this->hasExceptionMessage( $exception ) ) {
+ throw new InvalidArgumentException( '$exception cannot
be localized.' );
+ }
+
+ /** @var ChangeOpDeserializationException $exception */
+ return new Message( 'wikibase-api-' .
$exception->getErrorCode(), $exception->getParams() );
+ }
+
+}
diff --git a/repo/includes/WikibaseRepo.php b/repo/includes/WikibaseRepo.php
index 6b7245f..99282f4 100644
--- a/repo/includes/WikibaseRepo.php
+++ b/repo/includes/WikibaseRepo.php
@@ -86,7 +86,11 @@
use Wikibase\Lib\Store\EntityStore;
use Wikibase\Lib\Store\EntityStoreWatcher;
use Wikibase\Rdf\EntityRdfBuilderFactory;
+use Wikibase\Repo\ChangeOp\Deserialization\ChangeOpDeserializerFactory;
+use
Wikibase\Repo\ChangeOp\Deserialization\SiteLinkBadgeChangeOpSerializationValidator;
+use Wikibase\Repo\ChangeOp\Deserialization\TermChangeOpSerializationValidator;
use Wikibase\Repo\ChangeOp\EntityChangeOpProvider;
+use Wikibase\Repo\Localizer\ChangeOpDeserializationExceptionLocalizer;
use Wikibase\Repo\Store\EntityTitleStoreLookup;
use Wikibase\Lib\Store\LanguageFallbackLabelDescriptionLookupFactory;
use Wikibase\Lib\Store\PrefetchingTermLookup;
@@ -853,6 +857,34 @@
}
/**
+ * TODO: this should be probably cached?
+ *
+ * @return ChangeOpDeserializerFactory
+ */
+ public function getChangeOpDeserializerFactory() {
+ $changeOpFactoryProvider = $this->getChangeOpFactoryProvider();
+
+ return new ChangeOpDeserializerFactory(
+
$changeOpFactoryProvider->getFingerprintChangeOpFactory(),
+ $changeOpFactoryProvider->getStatementChangeOpFactory(),
+ $changeOpFactoryProvider->getSiteLinkChangeOpFactory(),
+ new TermChangeOpSerializationValidator(
$this->getTermsLanguages() ),
+ new SiteLinkBadgeChangeOpSerializationValidator(
+ $this->getEntityTitleLookup(),
+ array_keys( $this->settings->getSetting(
'badgeItems' ) )
+ ),
+ $this->getExternalFormatStatementDeserializer(),
+ new SiteLinkTargetProvider(
+ $this->getSiteLookup(),
+ $this->settings->getSetting(
'specialSiteLinkGroups' )
+ ),
+ $this->getEntityIdParser(),
+ $this->getStringNormalizer(),
+ $this->settings->getSetting( 'siteLinkGroups' )
+ );
+ }
+
+ /**
* @return LanguageFallbackChainFactory
*/
public function getLanguageFallbackChainFactory() {
@@ -1076,6 +1108,7 @@
'MessageException' => new MessageExceptionLocalizer(),
'ParseException' => new ParseExceptionLocalizer(),
'ChangeOpValidationException' => new
ChangeOpValidationExceptionLocalizer( $formatter ),
+ 'ChangeOpDeserializationException' => new
ChangeOpDeserializationExceptionLocalizer(),
'Exception' => new GenericExceptionLocalizer()
);
}
@@ -1848,15 +1881,6 @@
return null;
}
return $storage;
- }
-
- /**
- * @see EntityTypeDefinitions::getChangeOpDeserializerCallbacks
- *
- * @return callable[]
- */
- public function getChangeOpDeserializerCallbacks() {
- return
$this->entityTypeDefinitions->getChangeOpDeserializerCallbacks();
}
/**
diff --git a/repo/tests/phpunit/includes/Api/EditEntityTest.php
b/repo/tests/phpunit/includes/Api/EditEntityTest.php
index 9b6e096..9b4d943 100644
--- a/repo/tests/phpunit/includes/Api/EditEntityTest.php
+++ b/repo/tests/phpunit/includes/Api/EditEntityTest.php
@@ -658,7 +658,7 @@
'e' => array( 'exception' => array(
'type' => ApiUsageException::class,
'code' => 'missing-language',
- 'message' => '\'language\' was not
found in the label or description json for de'
+ 'message' => '\'language\' was not
found in term serialization for de'
) )
),
'removing invalid claim fails' => array(
@@ -780,7 +780,8 @@
),
'e' => array( 'exception' => array(
'type' => ApiUsageException::class,
- 'code' => 'no-such-sitelink'
+ 'code' => 'modification-failed',
+ 'message' => wfMessage(
'wikibase-validator-no-such-sitelink', 'svwiki' )->text(),
) )
),
'bad id in serialization' => array(
@@ -846,7 +847,7 @@
'e' => array( 'exception' => array(
'type' => ApiUsageException::class,
'code' => 'not-supported',
- 'message' => 'Non Items cannot have
sitelinks'
+ 'message' => 'The requested feature is
not supported by the given entity'
) ) ),
'create mediainfo with automatic id' => array(
'p' => array( 'new' => 'mediainfo', 'data' =>
'{}' ),
diff --git
a/repo/tests/phpunit/includes/ChangeOp/Deserialization/ChangeOpDeserializationExceptionTest.php
b/repo/tests/phpunit/includes/ChangeOp/Deserialization/ChangeOpDeserializationExceptionTest.php
index 7eb6e12..2b2426e 100644
---
a/repo/tests/phpunit/includes/ChangeOp/Deserialization/ChangeOpDeserializationExceptionTest.php
+++
b/repo/tests/phpunit/includes/ChangeOp/Deserialization/ChangeOpDeserializationExceptionTest.php
@@ -39,4 +39,10 @@
return [ [ 'error-foo' ], [ 'error-bar' ] ];
}
+ public function testGetParams() {
+ $exception = new ChangeOpDeserializationException( 'foo',
'error-foo', [ 'bar', 'baz' ] );
+
+ $this->assertEquals( [ 'bar', 'baz' ], $exception->getParams()
);
+ }
+
}
diff --git
a/repo/tests/phpunit/includes/ChangeOp/Deserialization/SiteLinksChangeOpDeserializerTest.php
b/repo/tests/phpunit/includes/ChangeOp/Deserialization/SiteLinksChangeOpDeserializerTest.php
index 856f8f3..3b51c4a 100644
---
a/repo/tests/phpunit/includes/ChangeOp/Deserialization/SiteLinksChangeOpDeserializerTest.php
+++
b/repo/tests/phpunit/includes/ChangeOp/Deserialization/SiteLinksChangeOpDeserializerTest.php
@@ -2,6 +2,7 @@
namespace Wikibase\Repo\Tests\ChangeOp\Deserialization;
+use Exception;
use HashSiteStore;
use Site;
use Wikibase\ChangeOp\SiteLinkChangeOpFactory;
@@ -167,14 +168,19 @@
[ 'testwikis' ]
);
-
ChangeOpDeserializationAssert::assertThrowsChangeOpDeserializationException(
- function() use ( $deserializer ) {
- $deserializer->createEntityChangeOp(
- [ 'sitelinks' => [ self::SITE_ID => [
'site' => self::SITE_ID, 'title' => 'Some Random Page' ] ] ]
- );
- },
- 'no-external-page'
- );
+ $exception = null;
+
+ try {
+ $deserializer->createEntityChangeOp(
+ [ 'sitelinks' => [ self::SITE_ID => [ 'site' =>
self::SITE_ID, 'title' => 'Some Random Page' ] ] ]
+ );
+ } catch ( Exception $ex ) {
+ $exception = $ex;
+ }
+
+ $this->assertInstanceOf(
ChangeOpDeserializationException::class, $exception );
+ $this->assertSame( 'no-external-page',
$exception->getErrorCode() );
+ $this->assertEquals( [ 'somewiki', 'Some Random Page' ],
$exception->getParams() );
}
private function getItemWithoutSiteLinks() {
diff --git
a/repo/tests/phpunit/includes/Localizer/ChangeOpDeserializationExceptionLocalizerTest.php
b/repo/tests/phpunit/includes/Localizer/ChangeOpDeserializationExceptionLocalizerTest.php
new file mode 100644
index 0000000..a93d936
--- /dev/null
+++
b/repo/tests/phpunit/includes/Localizer/ChangeOpDeserializationExceptionLocalizerTest.php
@@ -0,0 +1,70 @@
+<?php
+
+namespace Wikibase\Repo\Tests\Localizer;
+
+use InvalidArgumentException;
+use Message;
+use Wikibase\Repo\ChangeOp\Deserialization\ChangeOpDeserializationException;
+use Wikibase\Repo\Localizer\ChangeOpDeserializationExceptionLocalizer;
+
+/**
+ * @covers Wikibase\Repo\Localizer\ChangeOpDeserializationExceptionLocalizer
+ *
+ * @group Wikibase
+ *
+ * @license GPL-2.0+
+ */
+class ChangeOpDeserializationExceptionLocalizerTest extends
\PHPUnit_Framework_TestCase {
+
+ public function
testGivenExceptionOfOtherType_hasExceptionMessageReturnsFalse() {
+ $localizer = new ChangeOpDeserializationExceptionLocalizer();
+
+ $this->assertFalse( $localizer->hasExceptionMessage( new
InvalidArgumentException( 'foo' ) ) );
+ }
+
+ public function
testGivenExceptionOfOtherType_getExceptionMessageThrowsException() {
+ $localizer = new ChangeOpDeserializationExceptionLocalizer();
+
+ $this->setExpectedException( InvalidArgumentException::class );
+
+ $localizer->getExceptionMessage( new InvalidArgumentException(
'foo' ) );
+ }
+
+ private function getExceptionWithoutLocalizableMessage() {
+ return new ChangeOpDeserializationException( 'Foooo',
'some-error' );
+ }
+
+ public function
testGivenExceptionAndNoLocalizableMessageExists_hasExceptionMessageReturnsFalse()
{
+ $localizer = new ChangeOpDeserializationExceptionLocalizer();
+
+ $this->assertFalse( $localizer->hasExceptionMessage(
$this->getExceptionWithoutLocalizableMessage() ) );
+ }
+
+ public function
testGivenExceptionAndNoLocalizableMessageExists_getExceptionMessageThrowsException()
{
+ $localizer = new ChangeOpDeserializationExceptionLocalizer();
+
+ $this->setExpectedException( InvalidArgumentException::class );
+
+ $localizer->getExceptionMessage(
$this->getExceptionWithoutLocalizableMessage() );
+ }
+
+ private function getExceptionWithLocalizableMessage() {
+ return new ChangeOpDeserializationException( 'Foooo',
'no-external-page', [ 'foowiki', 'Random page' ] );
+ }
+
+ public function
testGivenExceptionAndLocalizableMessageExists_hasExceptionMessageReturnsTrue() {
+ $localizer = new ChangeOpDeserializationExceptionLocalizer();
+
+ $this->assertTrue( $localizer->hasExceptionMessage(
$this->getExceptionWithLocalizableMessage() ) );
+ }
+
+ public function
testGivenExceptionAndLocalizableMessageExists_getExceptionMessageReturnsIt() {
+ $localizer = new ChangeOpDeserializationExceptionLocalizer();
+
+ $this->assertEquals(
+ new Message( 'wikibase-api-no-external-page', [
'foowiki', 'Random page' ] ),
+ $localizer->getExceptionMessage(
$this->getExceptionWithLocalizableMessage() )
+ );
+ }
+
+}
diff --git a/repo/tests/phpunit/includes/WikibaseRepoTest.php
b/repo/tests/phpunit/includes/WikibaseRepoTest.php
index 329180f..543da0a 100644
--- a/repo/tests/phpunit/includes/WikibaseRepoTest.php
+++ b/repo/tests/phpunit/includes/WikibaseRepoTest.php
@@ -58,6 +58,7 @@
use Wikibase\Repo\Api\ApiHelperFactory;
use Wikibase\Repo\BuilderBasedDataTypeValidatorFactory;
use Wikibase\Repo\CachingCommonsMediaFileNameLookup;
+use Wikibase\Repo\ChangeOp\Deserialization\ChangeOpDeserializerFactory;
use Wikibase\Repo\ChangeOp\EntityChangeOpProvider;
use Wikibase\Repo\Content\EntityContentFactory;
use Wikibase\Repo\Content\EntityHandler;
@@ -285,6 +286,11 @@
public function testGetEntityChangeOpProvider() {
$provider =
$this->getWikibaseRepo()->getEntityChangeOpProvider();
$this->assertInstanceOf( EntityChangeOpProvider::class,
$provider );
+ }
+
+ public function testGetChangeOpDeserializerFactory() {
+ $factory =
$this->getWikibaseRepo()->getChangeOpDeserializerFactory();
+ $this->assertInstanceOf( ChangeOpDeserializerFactory::class,
$factory );
}
public function testGetLanguageFallbackChainFactory() {
@@ -695,21 +701,6 @@
$expected = new EntityIdValue( new ItemId( 'Q13' ) );
$this->assertEquals( $expected, $deserialized );
- }
-
- public function testGetChangeOpDeserializerCallbacks() {
- $wikibaseRepo = $this->getWikibaseRepo(
- [
- 'foo' => [
- 'changeop-deserializer-callback' =>
'new-changeop-deserializer-callback'
- ]
- ]
- );
- $changeOpsCallbacks =
$wikibaseRepo->getChangeOpDeserializerCallbacks();
- $expected = [
- 'foo' => 'new-changeop-deserializer-callback'
- ];
- $this->assertSame( $expected, $changeOpsCallbacks );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/331882
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2977607b4b02bc56183cfa5a9a9a0dc57432964d
Gerrit-PatchSet: 18
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Jakob <[email protected]>
Gerrit-Reviewer: Aleksey Bekh-Ivanov (WMDE) <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Jakob <[email protected]>
Gerrit-Reviewer: Ladsgroup <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: WMDE-leszek <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
