Thiemo Mättig (WMDE) has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/358525 )
Change subject: Allow empty EntityContent objects with no EntityHolder
......................................................................
Allow empty EntityContent objects with no EntityHolder
This is split from If0c9827.
This also adds a whole bunch of new tests. I wanted to avoid using the
abstract base class as much as I could.
Change-Id: Ifa9e84353a17a8de8cb827aa1522804412748a82
---
M repo/includes/Content/EntityContent.php
M repo/includes/Content/ItemContent.php
M repo/includes/Content/PropertyContent.php
M repo/tests/phpunit/includes/Content/EntityContentTest.php
M repo/tests/phpunit/includes/Content/ItemContentTest.php
M repo/tests/phpunit/includes/Content/PropertyContentTest.php
6 files changed, 152 insertions(+), 28 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/25/358525/1
diff --git a/repo/includes/Content/EntityContent.php
b/repo/includes/Content/EntityContent.php
index b6e5eda..dfe373b 100644
--- a/repo/includes/Content/EntityContent.php
+++ b/repo/includes/Content/EntityContent.php
@@ -104,29 +104,30 @@
* Returns a holder for the entity contained in this EntityContent
object.
*
* @throws MWException when it's a redirect (targets will never be
resolved)
- * @return EntityHolder
+ * @return EntityHolder|null
*/
abstract protected function getEntityHolder();
/**
- * Returns the ID of the entity represented by this EntityContent;
- *
- * @throws RuntimeException if no entity ID is set
+ * @throws RuntimeException if the content object is empty or no entity
ID is set
* @return EntityId
*/
public function getEntityId() {
if ( $this->isRedirect() ) {
return $this->getEntityRedirect()->getEntityId();
- } else {
- $id = $this->getEntityHolder()->getEntityId();
- if ( !$id ) {
- // @todo: Force an ID to be present; Entity
objects without an ID make sense,
- // EntityContent objects with no entity ID
don't.
- throw new RuntimeException( 'EntityContent was
constructed without an EntityId!' );
- }
-
- return $id;
}
+
+ $holder = $this->getEntityHolder();
+ if ( $holder !== null ) {
+ $id = $holder->getEntityId();
+ if ( $id !== null ) {
+ return $id;
+ }
+ }
+
+ // @todo: Force an ID to be present; Entity objects without an
ID make sense,
+ // EntityContent objects with no entity ID don't.
+ throw new RuntimeException( 'EntityContent was constructed
without an EntityId!' );
}
/**
@@ -503,8 +504,8 @@
return false;
}
- $thisId = $this->getEntityHolder()->getEntityId();
- $thatId = $that->getEntityHolder()->getEntityId();
+ $thisId = $this->getEntityHolder() ?
$this->getEntityHolder()->getEntityId() : null;
+ $thatId = $that->getEntityHolder() ?
$that->getEntityHolder()->getEntityId() : null;
if ( $thisId !== null && $thatId !== null
&& !$thisId->equals( $thatId )
@@ -627,22 +628,34 @@
* @return bool True if this is not a redirect and the page is empty.
*/
public function isEmpty() {
- return !$this->isRedirect() && parent::isEmpty();
+ if ( $this->isRedirect() ) {
+ return false;
+ }
+
+ $holder = $this->getEntityHolder();
+ return $holder === null || $holder->getEntity()->isEmpty();
}
/**
* @see Content::copy
*
- * @return ItemContent
+ * @return EntityContent
*/
public function copy() {
/* @var EntityHandler $handler */
$handler = $this->getContentHandler();
+
if ( $this->isRedirect() ) {
return $handler->makeEntityRedirectContent(
$this->getEntityRedirect() );
- } else {
- return $handler->makeEntityContent( new
DeferredCopyEntityHolder( $this->getEntityHolder() ) );
}
+
+ $holder = $this->getEntityHolder();
+ if ( $holder !== null ) {
+ return $handler->makeEntityContent( new
DeferredCopyEntityHolder( $holder ) );
+ }
+
+ // There is nothing mutable on an entirely empty content object.
+ return $this;
}
/**
diff --git a/repo/includes/Content/ItemContent.php
b/repo/includes/Content/ItemContent.php
index 5f9d42e..ce51a86 100644
--- a/repo/includes/Content/ItemContent.php
+++ b/repo/includes/Content/ItemContent.php
@@ -55,9 +55,8 @@
) {
parent::__construct( CONTENT_MODEL_WIKIBASE_ITEM );
- if ( is_null( $itemHolder ) === is_null( $entityRedirect ) ) {
- throw new InvalidArgumentException(
- 'Either $item or $entityRedirect and
$redirectTitle must be provided.' );
+ if ( $itemHolder !== null && $entityRedirect !== null ) {
+ throw new InvalidArgumentException( 'Can not be an item
and a redirect the same time' );
}
if ( $itemHolder !== null && $itemHolder->getEntityType() !==
Item::ENTITY_TYPE ) {
@@ -159,7 +158,7 @@
/**
* @see EntityContent::getEntityHolder
*
- * @return EntityHolder
+ * @return EntityHolder|null
*/
protected function getEntityHolder() {
return $this->itemHolder;
diff --git a/repo/includes/Content/PropertyContent.php
b/repo/includes/Content/PropertyContent.php
index 53d8f23..9af16ba 100644
--- a/repo/includes/Content/PropertyContent.php
+++ b/repo/includes/Content/PropertyContent.php
@@ -17,7 +17,7 @@
class PropertyContent extends EntityContent {
/**
- * @var EntityHolder
+ * @var EntityHolder|null
*/
private $propertyHolder;
@@ -30,13 +30,15 @@
*
* @protected
*
- * @param EntityHolder $propertyHolder
+ * @param EntityHolder|null $propertyHolder
* @throws InvalidArgumentException
*/
- public function __construct( EntityHolder $propertyHolder ) {
+ public function __construct( EntityHolder $propertyHolder = null ) {
parent::__construct( CONTENT_MODEL_WIKIBASE_PROPERTY );
- if ( $propertyHolder->getEntityType() !== Property::ENTITY_TYPE
) {
+ if ( $propertyHolder !== null
+ && $propertyHolder->getEntityType() !==
Property::ENTITY_TYPE
+ ) {
throw new InvalidArgumentException( '$propertyHolder
must contain a Property entity!' );
}
@@ -75,7 +77,7 @@
/**
* @see EntityContent::getEntityHolder
*
- * @return EntityHolder
+ * @return EntityHolder|null
*/
public function getEntityHolder() {
return $this->propertyHolder;
diff --git a/repo/tests/phpunit/includes/Content/EntityContentTest.php
b/repo/tests/phpunit/includes/Content/EntityContentTest.php
index 4b8eb52..e16835f 100644
--- a/repo/tests/phpunit/includes/Content/EntityContentTest.php
+++ b/repo/tests/phpunit/includes/Content/EntityContentTest.php
@@ -7,6 +7,7 @@
use Diff\Patcher\PatcherException;
use InvalidArgumentException;
use ParserOutput;
+use RuntimeException;
use Title;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\EntityId;
@@ -213,6 +214,16 @@
$this->assertEquals( $expected, $actual );
}
+ abstract public function provideContentObjectsWithoutId();
+
+ /**
+ * @dataProvider provideContentObjectsWithoutId
+ */
+ public function testGetEntityIdExceptions( EntityContent $content ) {
+ $this->setExpectedException( RuntimeException::class );
+ $content->getEntityId();
+ }
+
public function provideGetEntityPageProperties() {
$empty = $this->newEmpty();
diff --git a/repo/tests/phpunit/includes/Content/ItemContentTest.php
b/repo/tests/phpunit/includes/Content/ItemContentTest.php
index 86a357b..7526293 100644
--- a/repo/tests/phpunit/includes/Content/ItemContentTest.php
+++ b/repo/tests/phpunit/includes/Content/ItemContentTest.php
@@ -8,11 +8,13 @@
use Diff\DiffOp\DiffOpRemove;
use InvalidArgumentException;
use Title;
+use Wikibase\Content\EntityHolder;
use Wikibase\Content\EntityInstanceHolder;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityRedirect;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Entity\Property;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Services\Diff\EntityDiff;
use Wikibase\DataModel\Services\Lookup\InMemoryDataTypeLookup;
@@ -44,6 +46,66 @@
* @author Daniel Kinzler
*/
class ItemContentTest extends EntityContentTest {
+
+ public function provideValidConstructorArguments() {
+ return [
+ 'empty' => [ null, null, null ],
+ 'empty item' => [ new EntityInstanceHolder( new Item()
), null, null ],
+ 'redirect' => [
+ null,
+ new EntityRedirect( new ItemId( 'Q1' ), new
ItemId( 'Q2' ) ),
+ $this->getMock( Title::class )
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideValidConstructorArguments
+ */
+ public function testConstructor(
+ EntityHolder $holder = null,
+ EntityRedirect $redirect = null,
+ Title $title = null
+ ) {
+ $content = new ItemContent( $holder, $redirect, $title );
+ $this->assertInstanceOf( ItemContent::class, $content );
+ }
+
+ public function provideInvalidConstructorArguments() {
+ $holder = new EntityInstanceHolder( new Item() );
+ $redirect = new EntityRedirect( new ItemId( 'Q1' ), new ItemId(
'Q2' ) );
+ $title = $this->getMock( Title::class );
+
+ $propertyHolder = new EntityInstanceHolder(
Property::newFromType( 'string' ) );
+
+ $badTitle = $this->getMock( Title::class );
+ $badTitle->method( 'getContentModel' )
+ ->will( $this->returnValue( 'bad content model' ) );
+ $badTitle->method( 'exists' )
+ ->will( $this->returnValue( true ) );
+
+ return [
+ 'all' => [ $holder, $redirect, $title ],
+ 'holder and redirect' => [ $holder, $redirect, null ],
+ 'holder and title' => [ $holder, null, $title ],
+ 'redirect only' => [ null, $redirect, null ],
+ 'title only' => [ null, null, $title ],
+ 'bad entity type' => [ $propertyHolder, null, null ],
+ 'bad title' => [ null, $redirect, $badTitle ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideInvalidConstructorArguments
+ */
+ public function testConstructorExceptions(
+ EntityHolder $holder = null,
+ EntityRedirect $redirect = null,
+ Title $title = null
+ ) {
+ $this->setExpectedException( InvalidArgumentException::class );
+ new ItemContent( $holder, $redirect, $title );
+ }
/**
* @return ItemId
@@ -440,6 +502,13 @@
return $cases;
}
+ public function provideContentObjectsWithoutId() {
+ return [
+ 'no holder' => [ new ItemContent() ],
+ 'no ID' => [ new ItemContent( new EntityInstanceHolder(
new Item() ) ) ],
+ ];
+ }
+
public function entityRedirectProvider() {
$cases = parent::entityRedirectProvider();
diff --git a/repo/tests/phpunit/includes/Content/PropertyContentTest.php
b/repo/tests/phpunit/includes/Content/PropertyContentTest.php
index 6c9cc5e..156942c 100644
--- a/repo/tests/phpunit/includes/Content/PropertyContentTest.php
+++ b/repo/tests/phpunit/includes/Content/PropertyContentTest.php
@@ -3,8 +3,10 @@
namespace Wikibase\Repo\Tests\Content;
use InvalidArgumentException;
+use Wikibase\Content\EntityHolder;
use Wikibase\Content\EntityInstanceHolder;
use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\Property;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\PropertyContent;
@@ -22,6 +24,27 @@
* @author Jeroen De Dauw < jeroended...@gmail.com >
*/
class PropertyContentTest extends EntityContentTest {
+
+ public function provideValidConstructorArguments() {
+ return [
+ 'empty' => [ null ],
+ 'empty property' => [ new EntityInstanceHolder(
Property::newFromType( 'string' ) ) ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideValidConstructorArguments
+ */
+ public function testConstructor( EntityHolder $holder = null ) {
+ $content = new PropertyContent( $holder );
+ $this->assertInstanceOf( PropertyContent::class, $content );
+ }
+
+ public function testConstructorExceptions() {
+ $holder = new EntityInstanceHolder( new Item() );
+ $this->setExpectedException( InvalidArgumentException::class );
+ new PropertyContent( $holder );
+ }
/**
* @return PropertyId
@@ -61,6 +84,13 @@
);
}
+ public function provideContentObjectsWithoutId() {
+ return [
+ 'no holder' => [ new PropertyContent() ],
+ 'no ID' => [ new PropertyContent( new
EntityInstanceHolder( Property::newFromType( 'string' ) ) ) ],
+ ];
+ }
+
public function testIsEmpty_emptyProperty() {
$content = PropertyContent::newFromProperty(
Property::newFromType( 'foo' ) );
$this->assertTrue( $content->isEmpty() );
--
To view, visit https://gerrit.wikimedia.org/r/358525
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifa9e84353a17a8de8cb827aa1522804412748a82
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
