Daniel Kinzler has uploaded a new change for review.
https://gerrit.wikimedia.org/r/269461
Change subject: Make ChangeOp::validate run against the current revision.
......................................................................
Make ChangeOp::validate run against the current revision.
The idea is that validation should happen against the current revision,
because modifications of the edit's base revision get patched into the
current revision later. If we perform validation against the base
revision, we may miss conflicts that arise only after patching.
This change also moves loadEntity() from SpecialWikibaseRepoPage to
SpecialModifyEntity, to allow access to the EntityRevisionLookup.
loadEntity() is not used outside of SpecialModifyEntit.
Bug: T121395
Change-Id: I0d49d676bc15efff4a96a3b06d3df26f3494c7a9
---
M repo/includes/api/ModifyEntity.php
M repo/includes/specials/SpecialModifyEntity.php
M repo/includes/specials/SpecialSetLabelDescriptionAliases.php
M repo/includes/specials/SpecialWikibaseRepoPage.php
4 files changed, 100 insertions(+), 60 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/61/269461/1
diff --git a/repo/includes/api/ModifyEntity.php
b/repo/includes/api/ModifyEntity.php
index eb41031..dcc9f1a 100644
--- a/repo/includes/api/ModifyEntity.php
+++ b/repo/includes/api/ModifyEntity.php
@@ -361,12 +361,15 @@
* @param ChangeOp $changeOp
* @param EntityDocument $entity
* @param Summary|null $summary The summary object to update with
information about the change.
- *
- * @throws UsageException
*/
protected function applyChangeOp( ChangeOp $changeOp, EntityDocument
$entity, Summary $summary = null ) {
try {
- $result = $changeOp->validate( $entity );
+ // NOTE: always validate modification against the
current revision, if it exists!
+ // TODO: this should be re-engineered, see T126231
+ $currentEntityRevision =
$this->revisionLookup->getEntityRevision( $entity->getId() );
+ $currentEntity = $currentEntityRevision ?
$currentEntityRevision->getEntity() : $entity;
+ $result = $changeOp->validate( $currentEntity );
+ ## $result = $changeOp->validate( $entity );
if ( !$result->isValid() ) {
throw new ChangeOpValidationException( $result
);
diff --git a/repo/includes/specials/SpecialModifyEntity.php
b/repo/includes/specials/SpecialModifyEntity.php
index c97b44e..46df66a 100644
--- a/repo/includes/specials/SpecialModifyEntity.php
+++ b/repo/includes/specials/SpecialModifyEntity.php
@@ -3,15 +3,24 @@
namespace Wikibase\Repo\Specials;
use Html;
+use SiteStore;
use Wikibase\ChangeOp\ChangeOp;
use Wikibase\ChangeOp\ChangeOpException;
use Wikibase\ChangeOp\ChangeOpValidationException;
use Wikibase\CopyrightMessageBuilder;
use Wikibase\DataModel\Entity\EntityDocument;
+use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\EditEntityFactory;
use Wikibase\EntityRevision;
+use Wikibase\Lib\MessageException;
+use Wikibase\Lib\Store\EntityRevisionLookup;
+use Wikibase\Lib\Store\EntityTitleLookup;
+use Wikibase\Lib\Store\RevisionedUnresolvedRedirectException;
+use Wikibase\Lib\Store\StorageException;
use Wikibase\Lib\UserInputException;
use Wikibase\Repo\WikibaseRepo;
use Wikibase\Summary;
+use Wikibase\SummaryFormatter;
/**
* Abstract special page for modifying Wikibase entity.
@@ -22,6 +31,11 @@
* @author Daniel Kinzler
*/
abstract class SpecialModifyEntity extends SpecialWikibaseRepoPage {
+
+ /**
+ * @var EntityRevisionLookup
+ */
+ private $entityRevisionLookup;
/**
* @since 0.5
@@ -49,10 +63,44 @@
public function __construct( $title, $restriction = 'edit' ) {
parent::__construct( $title, $restriction );
- $settings = WikibaseRepo::getDefaultInstance()->getSettings();
+ $wikibaseRepo = WikibaseRepo::getDefaultInstance();
+ $settings = $wikibaseRepo->getSettings();
$this->rightsUrl = $settings->getSetting( 'dataRightsUrl' );
$this->rightsText = $settings->getSetting( 'dataRightsText' );
+
+ $this->setSpecialModifyEntityServices(
+ $wikibaseRepo->getSummaryFormatter(),
+ $wikibaseRepo->getEntityRevisionLookup( 'uncached' ),
+ $wikibaseRepo->getEntityTitleLookup(),
+ $wikibaseRepo->getSiteStore(),
+ $wikibaseRepo->newEditEntityFactory(
$this->getContext() )
+ );
+ }
+
+ /**
+ * Override services (for testing).
+ *
+ * @param SummaryFormatter $summaryFormatter
+ * @param EntityRevisionLookup $entityRevisionLookup
+ * @param EntityTitleLookup $entityTitleLookup
+ * @param SiteStore $siteStore
+ * @param EditEntityFactory $editEntityFactory
+ */
+ public function setSpecialModifyEntityServices(
+ SummaryFormatter $summaryFormatter,
+ EntityRevisionLookup $entityRevisionLookup,
+ EntityTitleLookup $entityTitleLookup,
+ SiteStore $siteStore,
+ EditEntityFactory $editEntityFactory
+ ) {
+ $this->entityRevisionLookup = $entityRevisionLookup;
+ $this->setSpecialWikibaseRepoPageServices(
+ $summaryFormatter,
+ $entityTitleLookup,
+ $siteStore,
+ $editEntityFactory
+ );
}
public function doesWrites() {
@@ -129,6 +177,46 @@
$entityId = $this->parseEntityId( $idString );
$this->entityRevision = $this->loadEntity( $entityId );
+ }
+
+ /**
+ * Loads the entity for this entity id.
+ *
+ * @since 0.5
+ *
+ * @param EntityId $id
+ * @param int|string $revisionId
+ *
+ * @throws MessageException
+ * @throws UserInputException
+ * @return EntityRevision
+ */
+ protected function loadEntity( EntityId $id, $revisionId =
EntityRevisionLookup::LATEST_FROM_MASTER ) {
+ try {
+ $entity =
$this->entityRevisionLookup->getEntityRevision( $id, $revisionId );
+
+ if ( $entity === null ) {
+ throw new UserInputException(
+ 'wikibase-wikibaserepopage-invalid-id',
+ array( $id->getSerialization() ),
+ 'Entity id is unknown'
+ );
+ }
+ } catch ( RevisionedUnresolvedRedirectException $ex ) {
+ throw new UserInputException(
+ 'wikibase-wikibaserepopage-unresolved-redirect',
+ array( $id->getSerialization() ),
+ 'Entity id refers to a redirect'
+ );
+ } catch ( StorageException $ex ) {
+ throw new MessageException(
+ 'wikibase-wikibaserepopage-storage-exception',
+ array( $id->getSerialization(),
$ex->getMessage() ),
+ 'Entity could not be loaded'
+ );
+ }
+
+ return $entity;
}
/**
@@ -281,7 +369,10 @@
* @throws ChangeOpException
*/
protected function applyChangeOp( ChangeOp $changeOp, EntityDocument
$entity, Summary $summary = null ) {
- $result = $changeOp->validate( $entity );
+ // NOTE: always validate modification against the current
revision!
+ // TODO: this should be re-engineered, see T126231
+ $currentEntityRevision =
$this->entityRevisionLookup->getEntityRevision( $entity->getId() );
+ $result = $changeOp->validate(
$currentEntityRevision->getEntity() );
if ( !$result->isValid() ) {
throw new ChangeOpValidationException( $result );
diff --git a/repo/includes/specials/SpecialSetLabelDescriptionAliases.php
b/repo/includes/specials/SpecialSetLabelDescriptionAliases.php
index 3a12caa..4c07d5f 100644
--- a/repo/includes/specials/SpecialSetLabelDescriptionAliases.php
+++ b/repo/includes/specials/SpecialSetLabelDescriptionAliases.php
@@ -95,7 +95,7 @@
ContentLanguages $termsLanguages,
EditEntityFactory $editEntityFactory
) {
- $this->setSpecialWikibaseRepoPageServices(
+ $this->setSpecialModifyEntityServices(
$summaryFormatter,
$entityRevisionLookup,
$entityTitleLookup,
diff --git a/repo/includes/specials/SpecialWikibaseRepoPage.php
b/repo/includes/specials/SpecialWikibaseRepoPage.php
index aa98f1b..86607b0 100644
--- a/repo/includes/specials/SpecialWikibaseRepoPage.php
+++ b/repo/includes/specials/SpecialWikibaseRepoPage.php
@@ -11,12 +11,7 @@
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\EditEntityFactory;
-use Wikibase\EntityRevision;
-use Wikibase\Lib\MessageException;
-use Wikibase\Lib\Store\EntityRevisionLookup;
use Wikibase\Lib\Store\EntityTitleLookup;
-use Wikibase\Lib\Store\RevisionedUnresolvedRedirectException;
-use Wikibase\Lib\Store\StorageException;
use Wikibase\Lib\UserInputException;
use Wikibase\Repo\WikibaseRepo;
use Wikibase\Summary;
@@ -35,11 +30,6 @@
* @var SummaryFormatter
*/
protected $summaryFormatter;
-
- /**
- * @var EntityRevisionLookup
- */
- private $entityRevisionLookup;
/**
* @var EntityTitleLookup
@@ -68,7 +58,6 @@
$this->setSpecialWikibaseRepoPageServices(
$wikibaseRepo->getSummaryFormatter(),
- $wikibaseRepo->getEntityRevisionLookup( 'uncached' ),
$wikibaseRepo->getEntityTitleLookup(),
$wikibaseRepo->getSiteStore(),
$wikibaseRepo->newEditEntityFactory(
$this->getContext() )
@@ -79,20 +68,17 @@
* Override services (for testing).
*
* @param SummaryFormatter $summaryFormatter
- * @param EntityRevisionLookup $entityRevisionLookup
* @param EntityTitleLookup $entityTitleLookup
* @param SiteStore $siteStore
* @param EditEntityFactory $editEntityFactory
*/
public function setSpecialWikibaseRepoPageServices(
SummaryFormatter $summaryFormatter,
- EntityRevisionLookup $entityRevisionLookup,
EntityTitleLookup $entityTitleLookup,
SiteStore $siteStore,
EditEntityFactory $editEntityFactory
) {
$this->summaryFormatter = $summaryFormatter;
- $this->entityRevisionLookup = $entityRevisionLookup;
$this->entityTitleLookup = $entityTitleLookup;
$this->siteStore = $siteStore;
$this->editEntityFactory = $editEntityFactory;
@@ -141,46 +127,6 @@
);
}
return $id;
- }
-
- /**
- * Loads the entity for this entity id.
- *
- * @since 0.5
- *
- * @param EntityId $id
- * @param int|string $revisionId
- *
- * @throws MessageException
- * @throws UserInputException
- * @return EntityRevision
- */
- protected function loadEntity( EntityId $id, $revisionId =
EntityRevisionLookup::LATEST_FROM_MASTER ) {
- try {
- $entity =
$this->entityRevisionLookup->getEntityRevision( $id, $revisionId );
-
- if ( $entity === null ) {
- throw new UserInputException(
- 'wikibase-wikibaserepopage-invalid-id',
- array( $id->getSerialization() ),
- 'Entity id is unknown'
- );
- }
- } catch ( RevisionedUnresolvedRedirectException $ex ) {
- throw new UserInputException(
- 'wikibase-wikibaserepopage-unresolved-redirect',
- array( $id->getSerialization() ),
- 'Entity id refers to a redirect'
- );
- } catch ( StorageException $ex ) {
- throw new MessageException(
- 'wikibase-wikibaserepopage-storage-exception',
- array( $id->getSerialization(),
$ex->getMessage() ),
- 'Entity could not be loaded'
- );
- }
-
- return $entity;
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/269461
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0d49d676bc15efff4a96a3b06d3df26f3494c7a9
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
