Thiemo Mättig (WMDE) has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/358530 )
Change subject: Inline duplicate getItem/Property in favor of
EntityContent::getEntity
......................................................................
Inline duplicate getItem/Property in favor of EntityContent::getEntity
I feel these methods don't help making the code better readable. The only
benefit I can see is when the two have different type hints: the generic
getEntity returns an EntityDocument, the more specific ones return specific
entity types. But this is not the case. The specific implementations do
have specific type hints, and this just works fine.
Change-Id: Ib539ef73cd9d0b7b6183e86f08ca841b948c3168
---
M repo/includes/Content/ItemContent.php
M repo/includes/Content/PropertyContent.php
M repo/includes/Content/PropertyHandler.php
M repo/tests/phpunit/includes/Actions/ActionTestCase.php
M repo/tests/phpunit/includes/Content/ItemContentTest.php
M repo/tests/phpunit/includes/Content/ItemHandlerTest.php
M repo/tests/phpunit/includes/Content/PropertyContentTest.php
7 files changed, 23 insertions(+), 40 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/30/358530/1
diff --git a/repo/includes/Content/ItemContent.php
b/repo/includes/Content/ItemContent.php
index 78b1ce9..4ac077b 100644
--- a/repo/includes/Content/ItemContent.php
+++ b/repo/includes/Content/ItemContent.php
@@ -125,11 +125,13 @@
}
/**
+ * @see EntityContent::getEntity
+ *
* @throws MWException when it's a redirect (targets will never be
resolved)
* @throws LogicException if the content object is empty and does not
contain an entity.
* @return Item
*/
- public function getItem() {
+ public function getEntity() {
$redirect = $this->getRedirectTarget();
if ( $redirect ) {
@@ -141,17 +143,6 @@
}
return $this->itemHolder->getEntity( Item::class );
- }
-
- /**
- * @see EntityContent::getEntity
- *
- * @throws MWException when it's a redirect (targets will never be
resolved)
- * @throws LogicException if the content object is empty and does not
contain an entity.
- * @return Item
- */
- public function getEntity() {
- return $this->getItem();
}
/**
@@ -174,7 +165,7 @@
// TODO: Refactor ItemSearchTextGenerator to share an interface
with
// FingerprintSearchTextGenerator, so we don't have to
re-implement getTextForSearchIndex() here.
$searchTextGenerator = new ItemSearchTextGenerator();
- $text = $searchTextGenerator->generate( $this->getItem() );
+ $text = $searchTextGenerator->generate( $this->getEntity() );
if ( !Hooks::run( 'WikibaseTextForSearchIndex', array( $this,
&$text ) ) ) {
return '';
@@ -191,7 +182,7 @@
* @return bool True if this is not a redirect and the item is not
empty.
*/
public function isCountable( $hasLinks = null ) {
- return !$this->isRedirect() && !$this->getItem()->isEmpty();
+ return !$this->isRedirect() && !$this->getEntity()->isEmpty();
}
/**
@@ -200,7 +191,7 @@
* @return bool True if this is not a redirect and the item is empty.
*/
public function isEmpty() {
- return !$this->isRedirect() && $this->getItem()->isEmpty();
+ return !$this->isRedirect() && $this->getEntity()->isEmpty();
}
/**
@@ -215,7 +206,7 @@
$properties = parent::getEntityPageProperties();
if ( !$this->isRedirect() ) {
- $item = $this->getItem();
+ $item = $this->getEntity();
$properties['wb-claims'] =
$item->getStatements()->count();
$properties['wb-sitelinks'] =
$item->getSiteLinkList()->count();
$properties['wb-identifiers'] =
$this->getContentHandler()
diff --git a/repo/includes/Content/PropertyContent.php
b/repo/includes/Content/PropertyContent.php
index 6732997..2732ffc 100644
--- a/repo/includes/Content/PropertyContent.php
+++ b/repo/includes/Content/PropertyContent.php
@@ -58,25 +58,17 @@
}
/**
- * @throws LogicException if the content object is empty and does not
contain an entity.
- * @return Property
- */
- public function getProperty() {
- if ( !$this->propertyHolder ) {
- throw new LogicException( 'This content object is
empty' );
- }
-
- return $this->propertyHolder->getEntity( Property::class );
- }
-
- /**
* @see EntityContent::getEntity
*
* @throws LogicException if the content object is empty and does not
contain an entity.
* @return Property
*/
public function getEntity() {
- return $this->getProperty();
+ if ( !$this->propertyHolder ) {
+ throw new LogicException( 'This content object is
empty' );
+ }
+
+ return $this->propertyHolder->getEntity( Property::class );
}
/**
@@ -98,7 +90,7 @@
public function getEntityPageProperties() {
$properties = parent::getEntityPageProperties();
- $properties['wb-claims'] =
$this->getProperty()->getStatements()->count();
+ $properties['wb-claims'] =
$this->getEntity()->getStatements()->count();
return $properties;
}
@@ -112,7 +104,7 @@
*/
public function isValid() {
//TODO: provide a way to get the data type from the holder
directly!
- return parent::isValid() &&
$this->getProperty()->getDataTypeId() !== '';
+ return parent::isValid() && $this->getEntity()->getDataTypeId()
!== '';
}
/**
@@ -123,7 +115,7 @@
* @return bool True if this is not a redirect and the property is not
empty.
*/
public function isCountable( $hasLinks = null ) {
- return !$this->isRedirect() && !$this->getProperty()->isEmpty();
+ return !$this->isRedirect() && !$this->getEntity()->isEmpty();
}
/**
@@ -132,7 +124,7 @@
* @return bool True if this is not a redirect and the property is
empty.
*/
public function isEmpty() {
- return !$this->isRedirect() && $this->getProperty()->isEmpty();
+ return !$this->isRedirect() && $this->getEntity()->isEmpty();
}
}
diff --git a/repo/includes/Content/PropertyHandler.php
b/repo/includes/Content/PropertyHandler.php
index bdb1ffd..84ebba2 100644
--- a/repo/includes/Content/PropertyHandler.php
+++ b/repo/includes/Content/PropertyHandler.php
@@ -178,7 +178,7 @@
/** @var PropertyContent $content */
$updates = array();
- $info = $this->propertyInfoBuilder->buildPropertyInfo(
$content->getProperty() );
+ $info = $this->propertyInfoBuilder->buildPropertyInfo(
$content->getEntity() );
$updates[] = new DataUpdateAdapter(
array( $this->infoStore, 'setPropertyInfo' ),
diff --git a/repo/tests/phpunit/includes/Actions/ActionTestCase.php
b/repo/tests/phpunit/includes/Actions/ActionTestCase.php
index 0fb4731..25114a5 100644
--- a/repo/tests/phpunit/includes/Actions/ActionTestCase.php
+++ b/repo/tests/phpunit/includes/Actions/ActionTestCase.php
@@ -386,7 +386,7 @@
/** @var ItemContent $content */
$content = $page->getContent();
- return $content->getItem();
+ return $content->getEntity();
}
/**
diff --git a/repo/tests/phpunit/includes/Content/ItemContentTest.php
b/repo/tests/phpunit/includes/Content/ItemContentTest.php
index 7526293..2cf061a 100644
--- a/repo/tests/phpunit/includes/Content/ItemContentTest.php
+++ b/repo/tests/phpunit/includes/Content/ItemContentTest.php
@@ -131,7 +131,7 @@
$empty = new ItemContent( new EntityInstanceHolder( new Item()
) );
if ( $itemId !== null ) {
- $empty->getItem()->setId( $itemId );
+ $empty->getEntity()->setId( $itemId );
}
return $empty;
@@ -214,7 +214,7 @@
*/
private function getItemContentWithClaim() {
$itemContent = $this->newEmpty();
- $item = $itemContent->getItem();
+ $item = $itemContent->getEntity();
$item->getStatements()->addNewStatement(
new PropertyNoValueSnak( new PropertyId( 'P11' ) ),
@@ -289,7 +289,7 @@
*/
private function getItemContentWithSiteLink() {
$itemContent = $this->newEmpty();
- $item = $itemContent->getItem();
+ $item = $itemContent->getEntity();
$item->setSiteLinkList( new SiteLinkList( array(
new SiteLink( 'enwiki', 'Foo' )
diff --git a/repo/tests/phpunit/includes/Content/ItemHandlerTest.php
b/repo/tests/phpunit/includes/Content/ItemHandlerTest.php
index 2e2d843..283acb2 100644
--- a/repo/tests/phpunit/includes/Content/ItemHandlerTest.php
+++ b/repo/tests/phpunit/includes/Content/ItemHandlerTest.php
@@ -68,7 +68,7 @@
$contents[] = [ $content ];
$content = $content->copy();
- $content->getItem()->getSiteLinkList()->addNewSiteLink(
'enwiki', 'Foobar' );
+ $content->getEntity()->getSiteLinkList()->addNewSiteLink(
'enwiki', 'Foobar' );
$contents[] = [ $content ];
return $contents;
diff --git a/repo/tests/phpunit/includes/Content/PropertyContentTest.php
b/repo/tests/phpunit/includes/Content/PropertyContentTest.php
index 156942c..c954390 100644
--- a/repo/tests/phpunit/includes/Content/PropertyContentTest.php
+++ b/repo/tests/phpunit/includes/Content/PropertyContentTest.php
@@ -70,7 +70,7 @@
$empty = new PropertyContent( new EntityInstanceHolder(
Property::newFromType( 'string' ) ) );
if ( $propertyId !== null ) {
- $empty->getProperty()->setId( $propertyId );
+ $empty->getEntity()->setId( $propertyId );
}
return $empty;
--
To view, visit https://gerrit.wikimedia.org/r/358530
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib539ef73cd9d0b7b6183e86f08ca841b948c3168
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
