jenkins-bot has submitted this change and it was merged.
Change subject: Fix EditEntity::isNew for new entities with an Id set
......................................................................
Fix EditEntity::isNew for new entities with an Id set
In some places (eg. the EditEntity api) we set an id on new
entities before calling out to EditEntity (because several
things require that).
This broke watching new entities that were created via the api,
as we always assumed that they already existed.
Bug: T85454
Change-Id: Ib915e928ee65a9db027326fb8e4aec62ea88b362
---
M repo/includes/EditEntity.php
M repo/tests/phpunit/includes/EditEntityTest.php
2 files changed, 40 insertions(+), 13 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, but someone else must approve
Thiemo Mättig (WMDE): Looks good to me, but someone else must approve
Jeroen De Dauw: Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/includes/EditEntity.php b/repo/includes/EditEntity.php
index ef03d5c..3f5d532 100644
--- a/repo/includes/EditEntity.php
+++ b/repo/includes/EditEntity.php
@@ -74,9 +74,9 @@
protected $latestRev = null;
/**
- * @var int|null
+ * @var int
*/
- protected $latestRevId = null;
+ protected $latestRevId = 0;
/**
* @var Status|null
@@ -246,7 +246,7 @@
* @return Title|null
*/
public function getTitle() {
- if ( $this->isNew() ) {
+ if ( $this->newEntity->getId() === null ) {
return null;
}
@@ -263,7 +263,7 @@
* @return EntityRevision|null
*/
public function getLatestRevision() {
- if ( $this->isNew() ) {
+ if ( $this->newEntity->getId() === null ) {
return null;
}
@@ -281,18 +281,21 @@
/**
* Returns the latest revision ID.
*
- * @return int
+ * @return int 0 if the entity doesn't exist
*/
public function getLatestRevisionId() {
- if ( $this->isNew() ) {
+ if ( $this->newEntity->getId() === null ) {
return 0;
}
- if ( $this->latestRevId === null ) {
+ wfProfileIn( __METHOD__ );
+ // Don't do negative caching: We call this to see whether the
entity yet exists
+ // before creating.
+ if ( $this->latestRevId === 0 ) {
if ( $this->latestRev !== null ) {
$this->latestRevId =
$this->latestRev->getRevisionId();
} else {
- $this->latestRevId =
$this->entityRevisionLookup->getLatestRevisionId(
+ $this->latestRevId =
(int)$this->entityRevisionLookup->getLatestRevisionId(
$this->getEntityId(),
EntityRevisionLookup::LATEST_FROM_MASTER
);
@@ -312,10 +315,14 @@
}
/**
- * Returns whether the new content is new, that is, does not have an ID
yet and thus no title, page or revisions.
+ * Is the entity new?
+ * An entity is new in case it either doesn't have an id or the Title
belonging
+ * to it doesn't (yet) exist.
+ *
+ * @return bool
*/
- public function isNew() {
- return $this->newEntity->getId() === null;
+ private function isNew() {
+ return $this->newEntity->getId() === null ||
$this->getLatestRevisionId() === 0;
}
/**
@@ -454,7 +461,7 @@
return false;
}
- if ( !is_int( $this->getBaseRevisionId() ) ||
$this->getBaseRevisionId() == $this->getLatestRevisionId() ) {
+ if ( $this->getBaseRevisionId() == $this->getLatestRevisionId()
) {
return false;
}
@@ -768,7 +775,7 @@
* @return IContextSource
*/
private function getContextForEditFilter( EntityDocument $entity ) {
- if ( !$this->isNew() ) {
+ if ( $this->getTitle() ) {
$context = clone $this->context;
$title = $this->getTitle();
} else {
diff --git a/repo/tests/phpunit/includes/EditEntityTest.php
b/repo/tests/phpunit/includes/EditEntityTest.php
index 13e1a9c..4239bae 100644
--- a/repo/tests/phpunit/includes/EditEntityTest.php
+++ b/repo/tests/phpunit/includes/EditEntityTest.php
@@ -5,6 +5,7 @@
use FauxRequest;
use HashBagOStuff;
use IContextSource;
+use ReflectionMethod;
use RequestContext;
use Status;
use Title;
@@ -768,4 +769,23 @@
$this->assertEquals( $expected, $repo->isWatching( $user,
$item->getId() ), "watched" );
}
+ public function testIsNew() {
+ $repo = $this->getMockRepository();
+ $titleLookup = $this->getEntityTitleLookup();
+ $item = new Item();
+
+ $isNew = new ReflectionMethod( 'Wikibase\EditEntity', 'isNew' );
+ $isNew->setAccessible( true );
+
+ $edit = $this->makeEditEntity( $repo, $item, $titleLookup );
+ $this->assertTrue( $isNew->invoke( $edit ), 'New entity: No id'
);
+
+ $repo->assignFreshId( $item );
+ $edit = $this->makeEditEntity( $repo, $item, $titleLookup );
+ $this->assertTrue( $isNew->invoke( $edit ), "New entity: Has an
id, but doesn't exist, yet" );
+
+ $repo->saveEntity( $item, 'testIsNew', $this->getUser(
'EditEntityTestUser1' ) );
+ $edit = $this->makeEditEntity( $repo, $item, $titleLookup );
+ $this->assertFalse( $isNew->invoke( $edit ), "Entity exists" );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/190657
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib915e928ee65a9db027326fb8e4aec62ea88b362
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Hoo man <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[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
