jenkins-bot has submitted this change and it was merged.
Change subject: Use narrower interfaces in term ChangeOps
......................................................................
Use narrower interfaces in term ChangeOps
Change-Id: I3f42423332f52905b9c23b7ba0b1fd0a9503bfb2
---
M repo/includes/ChangeOp/ChangeOpAliases.php
M repo/includes/ChangeOp/ChangeOpDescription.php
M repo/includes/ChangeOp/ChangeOpLabel.php
M repo/tests/phpunit/includes/ChangeOp/ChangeOpAliasesTest.php
M repo/tests/phpunit/includes/ChangeOp/ChangeOpDescriptionTest.php
M repo/tests/phpunit/includes/ChangeOp/ChangeOpLabelTest.php
6 files changed, 93 insertions(+), 56 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/includes/ChangeOp/ChangeOpAliases.php
b/repo/includes/ChangeOp/ChangeOpAliases.php
index 5f6fb82..2e049b8 100644
--- a/repo/includes/ChangeOp/ChangeOpAliases.php
+++ b/repo/includes/ChangeOp/ChangeOpAliases.php
@@ -5,6 +5,8 @@
use InvalidArgumentException;
use ValueValidators\Result;
use Wikibase\DataModel\Entity\EntityDocument;
+use Wikibase\DataModel\Term\AliasesProvider;
+use Wikibase\DataModel\Term\AliasGroupList;
use Wikibase\DataModel\Term\Fingerprint;
use Wikibase\DataModel\Term\FingerprintProvider;
use Wikibase\Repo\Validators\TermValidatorFactory;
@@ -72,15 +74,15 @@
}
/**
- * Applies the change to the fingerprint
+ * Applies the change to the aliases
*
- * @param Fingerprint $fingerprint
+ * @param AliasGroupList $aliases
*
* @throws ChangeOpException
*/
- private function updateFingerprint( Fingerprint $fingerprint ) {
- if ( $fingerprint->getAliasGroups()->hasGroupForLanguage(
$this->languageCode ) ) {
- $oldAliases = $fingerprint->getAliasGroup(
$this->languageCode )->getAliases();
+ private function updateAliases( AliasGroupList $aliases ) {
+ if ( $aliases->hasGroupForLanguage( $this->languageCode ) ) {
+ $oldAliases = $aliases->getByLanguage(
$this->languageCode )->getAliases();
} else {
$oldAliases = array();
}
@@ -95,22 +97,20 @@
throw new ChangeOpException( 'Bad action: ' .
$this->action );
}
- $fingerprint->getAliasGroups()->setAliasesForLanguage(
$this->languageCode, $newAliases );
+ $aliases->setAliasesForLanguage( $this->languageCode,
$newAliases );
}
/**
* @see ChangeOp::apply()
*/
public function apply( EntityDocument $entity, Summary $summary = null
) {
- if ( !( $entity instanceof FingerprintProvider ) ) {
- throw new InvalidArgumentException( '$entity must be a
FingerprintProvider' );
+ if ( !( $entity instanceof AliasesProvider ) ) {
+ throw new InvalidArgumentException( '$entity must be a
AliasesProvider' );
}
-
- $fingerprint = $entity->getFingerprint();
$this->updateSummary( $summary, $this->action,
$this->languageCode, $this->aliases );
- $this->updateFingerprint( $fingerprint );
+ $this->updateAliases( $entity->getAliasGroups() );
}
/**
diff --git a/repo/includes/ChangeOp/ChangeOpDescription.php
b/repo/includes/ChangeOp/ChangeOpDescription.php
index 611bcc0..4f7c211 100644
--- a/repo/includes/ChangeOp/ChangeOpDescription.php
+++ b/repo/includes/ChangeOp/ChangeOpDescription.php
@@ -5,8 +5,10 @@
use InvalidArgumentException;
use ValueValidators\Result;
use Wikibase\DataModel\Entity\EntityDocument;
+use Wikibase\DataModel\Term\DescriptionsProvider;
use Wikibase\DataModel\Term\Fingerprint;
use Wikibase\DataModel\Term\FingerprintProvider;
+use Wikibase\DataModel\Term\TermList;
use Wikibase\Repo\Validators\TermValidatorFactory;
use Wikibase\Summary;
@@ -61,15 +63,15 @@
}
/**
- * Applies the change to the fingerprint
+ * Applies the change to the descriptions
*
- * @param Fingerprint $fingerprint
+ * @param TermList $descriptions
*/
- private function updateFingerprint( Fingerprint $fingerprint ) {
+ private function updateDescriptions( TermList $descriptions ) {
if ( $this->description === null ) {
- $fingerprint->removeDescription( $this->languageCode );
+ $descriptions->removeByLanguage( $this->languageCode );
} else {
- $fingerprint->getDescriptions()->setTextForLanguage(
$this->languageCode, $this->description );
+ $descriptions->setTextForLanguage( $this->languageCode,
$this->description );
}
}
@@ -77,15 +79,15 @@
* @see ChangeOp::apply()
*/
public function apply( EntityDocument $entity, Summary $summary = null
) {
- if ( !( $entity instanceof FingerprintProvider ) ) {
- throw new InvalidArgumentException( '$entity must be a
FingerprintProvider' );
+ if ( !( $entity instanceof DescriptionsProvider ) ) {
+ throw new InvalidArgumentException( '$entity must be a
DescriptionsProvider' );
}
- $fingerprint = $entity->getFingerprint();
+ $descriptions = $entity->getDescriptions();
- if ( $fingerprint->getDescriptions()->hasTermForLanguage(
$this->languageCode ) ) {
+ if ( $descriptions->hasTermForLanguage( $this->languageCode ) )
{
if ( $this->description === null ) {
- $removedDescription =
$fingerprint->getDescription( $this->languageCode )->getText();
+ $removedDescription =
$descriptions->getByLanguage( $this->languageCode )->getText();
$this->updateSummary( $summary, 'remove',
$this->languageCode, $removedDescription );
} else {
$this->updateSummary( $summary, 'set',
$this->languageCode, $this->description );
@@ -94,7 +96,7 @@
$this->updateSummary( $summary, 'add',
$this->languageCode, $this->description );
}
- $this->updateFingerprint( $fingerprint );
+ $this->updateDescriptions( $descriptions );
}
/**
@@ -106,13 +108,12 @@
* @return Result
*/
public function validate( EntityDocument $entity ) {
- if ( !( $entity instanceof FingerprintProvider ) ) {
- throw new InvalidArgumentException( '$entity must be a
FingerprintProvider' );
+ if ( !( $entity instanceof DescriptionsProvider ) ) {
+ throw new InvalidArgumentException( '$entity must be a
DescriptionsProvider' );
}
$languageValidator =
$this->termValidatorFactory->getLanguageValidator();
$termValidator =
$this->termValidatorFactory->getDescriptionValidator();
- $fingerprintValidator =
$this->termValidatorFactory->getFingerprintValidator( $entity->getType() );
// check that the language is valid
$result = $languageValidator->validate( $this->languageCode );
@@ -126,16 +127,21 @@
return $result;
}
- // Check if the new fingerprint of the entity is valid (e.g. if
the combination
- // of label and description is still unique)
- $fingerprint = unserialize( serialize(
$entity->getFingerprint() ) );
- $this->updateFingerprint( $fingerprint );
+ // TODO: Don't bind against Fingerprint here, rather use
general builders for validators
+ if ( $entity instanceof FingerprintProvider ) {
+ $fingerprintValidator =
$this->termValidatorFactory->getFingerprintValidator( $entity->getType() );
- $result = $fingerprintValidator->validateFingerprint(
- $fingerprint,
- $entity->getId(),
- array( $this->languageCode )
- );
+ // Check if the new fingerprint of the entity is valid
(e.g. if the combination
+ // of label and description is still unique)
+ $fingerprint = clone $entity->getFingerprint();
+ $this->updateDescriptions(
$fingerprint->getDescriptions() );
+
+ $result = $fingerprintValidator->validateFingerprint(
+ $fingerprint,
+ $entity->getId(),
+ array( $this->languageCode )
+ );
+ }
return $result;
}
diff --git a/repo/includes/ChangeOp/ChangeOpLabel.php
b/repo/includes/ChangeOp/ChangeOpLabel.php
index da5faed..7905e20 100644
--- a/repo/includes/ChangeOp/ChangeOpLabel.php
+++ b/repo/includes/ChangeOp/ChangeOpLabel.php
@@ -7,6 +7,8 @@
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Term\Fingerprint;
use Wikibase\DataModel\Term\FingerprintProvider;
+use Wikibase\DataModel\Term\LabelsProvider;
+use Wikibase\DataModel\Term\TermList;
use Wikibase\Repo\Validators\TermValidatorFactory;
use Wikibase\Summary;
@@ -61,15 +63,15 @@
}
/**
- * Applies the change to the fingerprint
+ * Applies the change to the labels
*
- * @param Fingerprint $fingerprint
+ * @param TermList $labels
*/
- private function updateFingerprint( Fingerprint $fingerprint ) {
+ private function updateLabels( TermList $labels ) {
if ( $this->label === null ) {
- $fingerprint->removeLabel( $this->languageCode );
+ $labels->removeByLanguage( $this->languageCode );
} else {
- $fingerprint->getLabels()->setTextForLanguage(
$this->languageCode, $this->label );
+ $labels->setTextForLanguage( $this->languageCode,
$this->label );
}
}
@@ -82,15 +84,15 @@
* @throws InvalidArgumentException
*/
public function apply( EntityDocument $entity, Summary $summary = null
) {
- if ( !( $entity instanceof FingerprintProvider ) ) {
- throw new InvalidArgumentException( '$entity must be a
FingerprintProvider' );
+ if ( !( $entity instanceof LabelsProvider ) ) {
+ throw new InvalidArgumentException( '$entity must be a
LabelsProvider' );
}
- $fingerprint = $entity->getFingerprint();
+ $labels = $entity->getLabels();
- if ( $fingerprint->getLabels()->hasTermForLanguage(
$this->languageCode ) ) {
+ if ( $labels->hasTermForLanguage( $this->languageCode ) ) {
if ( $this->label === null ) {
- $oldLabel = $fingerprint->getLabel(
$this->languageCode )->getText();
+ $oldLabel = $labels->getByLanguage(
$this->languageCode )->getText();
$this->updateSummary( $summary, 'remove',
$this->languageCode, $oldLabel );
} else {
$this->updateSummary( $summary, 'set',
$this->languageCode, $this->label );
@@ -99,7 +101,7 @@
$this->updateSummary( $summary, 'add',
$this->languageCode, $this->label );
}
- $this->updateFingerprint( $fingerprint );
+ $this->updateLabels( $labels );
}
/**
@@ -111,13 +113,12 @@
* @return Result
*/
public function validate( EntityDocument $entity ) {
- if ( !( $entity instanceof FingerprintProvider ) ) {
- throw new InvalidArgumentException( '$entity must be a
FingerprintProvider' );
+ if ( !( $entity instanceof LabelsProvider ) ) {
+ throw new InvalidArgumentException( '$entity must be a
LabelsProvider' );
}
$languageValidator =
$this->termValidatorFactory->getLanguageValidator();
$termValidator =
$this->termValidatorFactory->getLabelValidator( $entity->getType() );
- $fingerprintValidator =
$this->termValidatorFactory->getFingerprintValidator( $entity->getType() );
// check that the language is valid
$result = $languageValidator->validate( $this->languageCode );
@@ -131,15 +132,20 @@
return $result;
}
- // Check if the new fingerprint of the entity is valid (e.g. if
the label is unique)
- $fingerprint = unserialize( serialize(
$entity->getFingerprint() ) );
- $this->updateFingerprint( $fingerprint );
+ // TODO: Don't bind against Fingerprint here, rather use
general builders for validators
+ if ( $entity instanceof FingerprintProvider ) {
+ $fingerprintValidator =
$this->termValidatorFactory->getFingerprintValidator( $entity->getType() );
- $result = $fingerprintValidator->validateFingerprint(
- $fingerprint,
- $entity->getId(),
- array( $this->languageCode )
- );
+ // Check if the new fingerprint of the entity is valid
(e.g. if the label is unique)
+ $fingerprint = clone $entity->getFingerprint();
+ $this->updateLabels( $fingerprint->getLabels() );
+
+ $result = $fingerprintValidator->validateFingerprint(
+ $fingerprint,
+ $entity->getId(),
+ array( $this->languageCode )
+ );
+ }
return $result;
}
diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpAliasesTest.php
b/repo/tests/phpunit/includes/ChangeOp/ChangeOpAliasesTest.php
index b4654cf..601cb4a 100644
--- a/repo/tests/phpunit/includes/ChangeOp/ChangeOpAliasesTest.php
+++ b/repo/tests/phpunit/includes/ChangeOp/ChangeOpAliasesTest.php
@@ -5,6 +5,7 @@
use InvalidArgumentException;
use Wikibase\ChangeOp\ChangeOpAliases;
use Wikibase\ChangeOp\ChangeOpException;
+use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\Item;
use Wikibase\ItemContent;
@@ -154,4 +155,12 @@
$changeOpAliases->apply( $entity );
}
+ public function testApplyNoAliasesProvider() {
+ $changeOp = new ChangeOpAliases( 'en', array( 'Foo' ), 'set',
$this->getTermValidatorFactory() );
+ $entity = $this->getMock( EntityDocument::class );
+
+ $this->setExpectedException( InvalidArgumentException::class );
+ $changeOp->apply( $entity );
+ }
+
}
diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpDescriptionTest.php
b/repo/tests/phpunit/includes/ChangeOp/ChangeOpDescriptionTest.php
index 093333b..871f8ba 100644
--- a/repo/tests/phpunit/includes/ChangeOp/ChangeOpDescriptionTest.php
+++ b/repo/tests/phpunit/includes/ChangeOp/ChangeOpDescriptionTest.php
@@ -150,4 +150,12 @@
$this->assertEquals( $summaryExpectedLanguage,
$summary->getLanguageCode() );
}
+ public function testApplyNoDescriptionsProvider() {
+ $changeOp = new ChangeOpDescription( 'en', 'Foo',
$this->getTermValidatorFactory() );
+ $entity = $this->getMock( EntityDocument::class );
+
+ $this->setExpectedException( InvalidArgumentException::class );
+ $changeOp->apply( $entity );
+ }
+
}
diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpLabelTest.php
b/repo/tests/phpunit/includes/ChangeOp/ChangeOpLabelTest.php
index f09fdea..12c7cc9 100644
--- a/repo/tests/phpunit/includes/ChangeOp/ChangeOpLabelTest.php
+++ b/repo/tests/phpunit/includes/ChangeOp/ChangeOpLabelTest.php
@@ -152,4 +152,12 @@
$this->assertEquals( $summaryExpectedLanguage,
$summary->getLanguageCode() );
}
+ public function testApplyNoLabelsProvider() {
+ $changeOp = new ChangeOpLabel( 'en', 'Foo',
$this->getTermValidatorFactory() );
+ $entity = $this->getMock( EntityDocument::class );
+
+ $this->setExpectedException( InvalidArgumentException::class );
+ $changeOp->apply( $entity );
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/276719
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3f42423332f52905b9c23b7ba0b1fd0a9503bfb2
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Bene <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Bene <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Jonas Kress (WMDE) <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
