Thiemo Mättig (WMDE) has submitted this change and it was merged.
Change subject: Minimize usage of EntityContent / ECFactory
......................................................................
Minimize usage of EntityContent / ECFactory
Change-Id: I33b0be329d0a490c6e049d44a6341607f95aaf9a
---
M repo/Wikibase.hooks.php
M repo/includes/EditEntity.php
M repo/includes/content/EntityContent.php
M repo/includes/content/EntityContentFactory.php
M repo/includes/content/ItemContent.php
M repo/includes/content/PropertyContent.php
M repo/includes/updates/EntityDeletionUpdate.php
M repo/includes/updates/ItemDeletionUpdate.php
M repo/tests/phpunit/includes/api/SetQualifierTest.php
M repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
M repo/tests/phpunit/includes/updates/ItemDeletionUpdateTest.php
M repo/tests/phpunit/includes/updates/ItemModificationUpdateTest.php
12 files changed, 72 insertions(+), 110 deletions(-)
Approvals:
WikidataJenkins: Verified
Thiemo Mättig (WMDE): Looks good to me, approved
diff --git a/repo/Wikibase.hooks.php b/repo/Wikibase.hooks.php
index b6d6eab..d93286b 100644
--- a/repo/Wikibase.hooks.php
+++ b/repo/Wikibase.hooks.php
@@ -344,39 +344,44 @@
// Bail out if we are not looking at an entity
if ( !$entityContentFactory->isEntityContentModel(
$title->getContentModel() ) ) {
+ wfProfileOut( __METHOD__ );
return true;
}
$revId = $title->getLatestRevID();
- $content = $entityContentFactory->getFromRevision( $revId );
+ $revision = Revision::newFromId( $revId );
+ $content = $revision ? $revision->getContent() : null;
- if ( $content ) {
- //XXX: EntityContent::save() also does this. Why are we
doing this twice?
-
StoreFactory::getStore()->newEntityPerPage()->addEntityPage(
- $content->getEntity()->getId(),
- $title->getArticleID()
- );
-
- $entity = $content->getEntity();
-
- $rev = Revision::newFromId( $revId );
-
- $userId = $rev->getUser();
- $change = EntityChange::newFromUpdate(
EntityChange::RESTORE, null, $entity, array(
- // TODO: Use timestamp of log entry, but needs
core change.
- // This hook is called before the log entry is
created.
- 'revision_id' => $revId,
- 'user_id' => $userId,
- 'object_id' =>
$entity->getId()->getPrefixedId(),
- 'time' => wfTimestamp( TS_MW, wfTimestampNow() )
- ) );
-
- $user = User::newFromId( $userId );
- $change->setMetadataFromUser( $user );
-
- $changeNotifier = new ChangeNotifier();
- $changeNotifier->handleChange( $change );
+ if ( !$content instanceof EntityContent ) {
+ wfProfileOut( __METHOD__ );
+ return true;
}
+
+ $entity = $content->getEntity();
+
+ //XXX: EntityContent::save() also does this. Why are we doing
this twice?
+ StoreFactory::getStore()->newEntityPerPage()->addEntityPage(
+ $entity->getId(),
+ $title->getArticleID()
+ );
+
+ $rev = Revision::newFromId( $revId );
+
+ $userId = $rev->getUser();
+ $change = EntityChange::newFromUpdate( EntityChange::RESTORE,
null, $entity, array(
+ // TODO: Use timestamp of log entry, but needs core
change.
+ // This hook is called before the log entry is created.
+ 'revision_id' => $revId,
+ 'user_id' => $userId,
+ 'object_id' => $entity->getId()->getPrefixedId(),
+ 'time' => wfTimestamp( TS_MW, wfTimestampNow() )
+ ) );
+
+ $user = User::newFromId( $userId );
+ $change->setMetadataFromUser( $user );
+
+ $changeNotifier = new ChangeNotifier();
+ $changeNotifier->handleChange( $change );
wfProfileOut( __METHOD__ );
return true;
@@ -1189,7 +1194,7 @@
$langCodes = Utils::getLanguageCodes() + array( $langCode =>
$fallbackChain );
$hookHandler = new MakeGlobalVariablesScriptHandler(
- $wikibaseRepo->getEntityContentFactory(),
+ $wikibaseRepo->getEntityRevisionLookup(),
$wikibaseRepo->getParserOutputJsConfigBuilder(
$langCode ),
$langCodes
);
diff --git a/repo/includes/EditEntity.php b/repo/includes/EditEntity.php
index 08a34c0..367e391 100644
--- a/repo/includes/EditEntity.php
+++ b/repo/includes/EditEntity.php
@@ -750,7 +750,7 @@
// Run edit filter hooks
$filterStatus = Status::newGood();
- $entityContent =
WikibaseRepo::getDefaultInstance()->getEntityContentFactory()->newFromEntity(
$this->newEntity );
+ $entityContent =
WikibaseRepo::getDefaultInstance()->getEntityContentFactory()->newContentFromEntity(
$this->newEntity );
if ( !wfRunHooks( 'EditFilterMergedContent',
array( $context, $entityContent, &$filterStatus,
$summary, $this->getUser(), false ) ) ) {
diff --git a/repo/includes/content/EntityContent.php
b/repo/includes/content/EntityContent.php
index dc54040..646abf0 100644
--- a/repo/includes/content/EntityContent.php
+++ b/repo/includes/content/EntityContent.php
@@ -80,12 +80,14 @@
* @since 0.1
* @deprecated use EntityTitleLookup instead
*
+ * @deprecated since 0.5, use EntityTitleLookup:.getTitleForId instead.
+ *
* @return Title|bool
*/
public function getTitle() {
$id = $this->getEntity()->getId();
- if ( $id === null ) {
+ if ( !$id ) {
return false;
}
diff --git a/repo/includes/content/EntityContentFactory.php
b/repo/includes/content/EntityContentFactory.php
index fa77d86..6eab483 100644
--- a/repo/includes/content/EntityContentFactory.php
+++ b/repo/includes/content/EntityContentFactory.php
@@ -3,7 +3,6 @@
namespace Wikibase;
use MWException;
-use InvalidArgumentException;
use OutOfBoundsException;
use Status;
use Title;
@@ -18,6 +17,7 @@
*
* @licence GNU GPL v2+
* @author Jeroen De Dauw < jeroended...@gmail.com >
+ * @author Daniel Kinzler
*/
class EntityContentFactory implements EntityTitleLookup,
EntityPermissionChecker {
@@ -81,7 +81,7 @@
foreach ( $entityIds as $entityId ) {
list( $type, $id ) = $entityId;
- $entity = self::getFromId( new EntityId( $type, $id ) );
+ $entity = $this->getFromId( new EntityId( $type, $id )
);
if ( $entity !== null ) {
$entities[] = $entity;
@@ -110,7 +110,7 @@
*
* @return EntityContent|null
*/
- public function getFromId( EntityId $id, $audience =
Revision::FOR_PUBLIC ) {
+ private function getFromId( EntityId $id, $audience =
Revision::FOR_PUBLIC ) {
// TODO: since we already did the trouble of getting a WikiPage
here,
// we probably want to keep a copy of it in the Content object.
$title = $this->getTitleForId( $id );
@@ -205,31 +205,6 @@
$handler = \ContentHandler::getForModelID(
$this->typeMap[$entity->getType()] );
return $handler->newContentFromEntity( $entity );
- }
-
- /**
- * Constructs a new EntityContent from a given type.
- *
- * @since 0.4
- *
- * @param string $type
- *
- * @return EntityContent
- *
- * @throws InvalidArgumentException
- */
- public function newFromType( $type ) {
- if ( !is_string( $type ) ) {
- throw new InvalidArgumentException( '$type needs to be
a string' );
- }
-
- if ( $type === Item::ENTITY_TYPE ) {
- return ItemContent::newEmpty();
- } elseif ( $type === Property::ENTITY_TYPE ) {
- return PropertyContent::newEmpty();
- } else {
- throw new InvalidArgumentException( 'unknown entity
type $type' );
- }
}
/**
diff --git a/repo/includes/content/ItemContent.php
b/repo/includes/content/ItemContent.php
index c4713ce..262ad7c 100644
--- a/repo/includes/content/ItemContent.php
+++ b/repo/includes/content/ItemContent.php
@@ -145,7 +145,7 @@
public function getDeletionUpdates( WikiPage $page, ParserOutput
$parserOutput = null ) {
return array_merge(
parent::getDeletionUpdates( $page, $parserOutput ),
- array( new ItemDeletionUpdate( $this ) )
+ array( new ItemDeletionUpdate( $this, $page->getTitle()
) )
);
}
diff --git a/repo/includes/content/PropertyContent.php
b/repo/includes/content/PropertyContent.php
index 62c5323..e64b454 100644
--- a/repo/includes/content/PropertyContent.php
+++ b/repo/includes/content/PropertyContent.php
@@ -138,7 +138,7 @@
return array_merge(
parent::getDeletionUpdates( $page, $parserOutput ),
array(
- new EntityDeletionUpdate( $this ),
+ new EntityDeletionUpdate( $this,
$page->getTitle() ),
new PropertyInfoDeletion(
$this->getProperty()->getId(), $infoStore ),
)
);
diff --git a/repo/includes/updates/EntityDeletionUpdate.php
b/repo/includes/updates/EntityDeletionUpdate.php
index 46a38ff..8128f7d 100644
--- a/repo/includes/updates/EntityDeletionUpdate.php
+++ b/repo/includes/updates/EntityDeletionUpdate.php
@@ -1,6 +1,7 @@
<?php
namespace Wikibase;
+use Title;
/**
* Deletion update to handle deletion of Wikibase entities.
@@ -17,7 +18,12 @@
*
* @var EntityContent
*/
- protected $content;
+ private $content;
+
+ /**
+ * @var Title
+ */
+ private $title;
/**
* Constructor.
@@ -25,9 +31,11 @@
* @since 0.1
*
* @param EntityContent $content
+ * @param Title $title
*/
- public function __construct( EntityContent $content ) {
+ public function __construct( EntityContent $content, Title $title ) {
$this->content = $content;
+ $this->title = $title;
}
/**
@@ -46,7 +54,7 @@
$store->newEntityPerPage()->deleteEntityPage(
$entity->getId(),
- $this->content->getTitle()->getArticleID()
+ $this->title->getArticleID()
);
/**
diff --git a/repo/includes/updates/ItemDeletionUpdate.php
b/repo/includes/updates/ItemDeletionUpdate.php
index 83e08db..81cd78a 100644
--- a/repo/includes/updates/ItemDeletionUpdate.php
+++ b/repo/includes/updates/ItemDeletionUpdate.php
@@ -1,6 +1,7 @@
<?php
namespace Wikibase;
+use Title;
/**
* Deletion update to handle deletion of Wikibase items.
@@ -19,8 +20,8 @@
*
* @param ItemContent $newContent
*/
- public function __construct( ItemContent $newContent ) {
- $this->content = $newContent;
+ public function __construct( ItemContent $newContent, Title $title ) {
+ parent::__construct( $newContent, $title );
}
/**
diff --git a/repo/tests/phpunit/includes/api/SetQualifierTest.php
b/repo/tests/phpunit/includes/api/SetQualifierTest.php
index 664a7d7..3a42ab6 100644
--- a/repo/tests/phpunit/includes/api/SetQualifierTest.php
+++ b/repo/tests/phpunit/includes/api/SetQualifierTest.php
@@ -93,18 +93,19 @@
if ( !$item ) {
$store =
WikibaseRepo::getDefaultInstance()->getEntityStore();
- $item = Item::newEmpty();
- $store->saveEntity( $item, '', $GLOBALS['wgUser'],
EDIT_NEW );
+ $newItem = Item::newEmpty();
+ $store->saveEntity( $newItem, '', $GLOBALS['wgUser'],
EDIT_NEW );
$prop = Property::newEmpty();
$propId = $this->makeProperty( $prop, 'string'
)->getId();
$claim = new Statement( new PropertyValueSnak( $propId,
new StringValue( '^_^' ) ) );
$guidGenerator = new ClaimGuidGenerator();
- $claim->setGuid( $guidGenerator->newGuid(
$item->getId() ) );
- $item->addClaim( $claim );
+ $claim->setGuid( $guidGenerator->newGuid(
$newItem->getId() ) );
+ $newItem->addClaim( $claim );
- $store->saveEntity( $item, '', $GLOBALS['wgUser'],
EDIT_UPDATE );
+ $store->saveEntity( $newItem, '', $GLOBALS['wgUser'],
EDIT_UPDATE );
+ $item = $newItem;
}
return $item;
diff --git a/repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
b/repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
index 8af5553..62d7f3f 100644
--- a/repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
+++ b/repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
@@ -80,42 +80,6 @@
$this->assertGreaterThanOrEqual( 0, $ns, 'namespace' );
}
- public function entityTypesProvider() {
- $argLists = array();
-
- $argLists[] = array( Item::ENTITY_TYPE );
- $argLists[] = array( Property::ENTITY_TYPE );
-
- return $argLists;
- }
-
- public function invalidEntityTypesProvider() {
- $argLists = array();
-
- $argLists[] = array( 42 );
- $argLists[] = array( 'foo' );
-
- return $argLists;
- }
-
- /**
- * @dataProvider entityTypesProvider
- */
- public function testNewFromType( $type ) {
- $entityContentFactory =
WikibaseRepo::getDefaultInstance()->getEntityContentFactory();
- $entityContent = $entityContentFactory->newFromType( $type );
- $this->assertEquals( $type,
$entityContent->getEntity()->getType() );
- }
-
- /**
- * @dataProvider invalidEntityTypesProvider
- * @expectedException InvalidArgumentException
- */
- public function testInvalidNewFromType( $type ) {
- $entityContentFactory =
WikibaseRepo::getDefaultInstance()->getEntityContentFactory();
- $entityContent = $entityContentFactory->newFromType( $type );
- }
-
public function provideGetPermissionStatusForEntity() {
return array(
'read allowed for non-existing entity' => array(
diff --git a/repo/tests/phpunit/includes/updates/ItemDeletionUpdateTest.php
b/repo/tests/phpunit/includes/updates/ItemDeletionUpdateTest.php
index 30701fe..4618f68 100644
--- a/repo/tests/phpunit/includes/updates/ItemDeletionUpdateTest.php
+++ b/repo/tests/phpunit/includes/updates/ItemDeletionUpdateTest.php
@@ -3,6 +3,7 @@
namespace Wikibase\Test;
use TestSites;
+use Title;
use Wikibase\ItemContent;
use Wikibase\ItemDeletionUpdate;
use Wikibase\Repo\WikibaseRepo;
@@ -33,7 +34,8 @@
}
public function testConstruct() {
- $update = new ItemDeletionUpdate( ItemContent::newEmpty() );
+ $title = Title::newFromText( 'ItemDeletionUpdateTest/Dummy' );
+ $update = new ItemDeletionUpdate( ItemContent::newEmpty(),
$title );
$this->assertInstanceOf( '\Wikibase\ItemDeletionUpdate',
$update );
$this->assertInstanceOf( '\Wikibase\EntityDeletionUpdate',
$update );
$this->assertInstanceOf( 'DataUpdate', $update );
@@ -58,7 +60,8 @@
$revision = $store->saveEntity( $itemContent->getEntity(),
"testing", $GLOBALS['wgUser'], EDIT_NEW );
$id = $revision->getEntity()->getId()->getNumericId();
- $update = new ItemDeletionUpdate( $itemContent );
+ $title = Title::newFromText( 'ItemDeletionUpdateTest/Dummy' );
+ $update = new ItemDeletionUpdate( $itemContent, $title );
$update->doUpdate();
$linkLookup = StoreFactory::getStore()->newSiteLinkCache();
diff --git a/repo/tests/phpunit/includes/updates/ItemModificationUpdateTest.php
b/repo/tests/phpunit/includes/updates/ItemModificationUpdateTest.php
index 8f19a64..9d39e58 100644
--- a/repo/tests/phpunit/includes/updates/ItemModificationUpdateTest.php
+++ b/repo/tests/phpunit/includes/updates/ItemModificationUpdateTest.php
@@ -51,9 +51,10 @@
$sitesTable->clear();
$sitesTable->saveSites( TestSites::getSites() );
- $linkLookup = StoreFactory::getStore()->newSiteLinkCache();
+ $linkLookup =
WikibaseRepo::getDefaultInstance()->getStore()->newSiteLinkCache();
$store = WikibaseRepo::getDefaultInstance()->getEntityStore();
+ $titleLookup =
WikibaseRepo::getDefaultInstance()->getEntityTitleLookup();
$revision = $store->saveEntity( $itemContent->getEntity(),
"testing", $GLOBALS['wgUser'], EDIT_NEW );
$id = $revision->getEntity()->getId()->getNumericId();
@@ -84,7 +85,9 @@
// TODO: verify terms
- $update = new ItemDeletionUpdate( $itemContent );
+ $title = $titleLookup->getTitleForId(
$itemContent->getEntity()->getId() );
+
+ $update = new ItemDeletionUpdate( $itemContent, $title );
$update->doUpdate();
}
--
To view, visit https://gerrit.wikimedia.org/r/131965
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I33b0be329d0a490c6e049d44a6341607f95aaf9a
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Adrian Lang <adrian.l...@wikimedia.de>
Gerrit-Reviewer: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Hoo man <h...@online.de>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: WikidataJenkins <wikidata-servi...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
