Addshore has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/403693 )
Change subject: Make Revision::__construct work with bad page ID
......................................................................
Make Revision::__construct work with bad page ID
For backwards-copatibility, we need to be able to construct a Revision
object even for bad page IDs.
Bug: T184689
Change-Id: I18c823d7b72504447982364d581b34e98924b67f
---
M includes/Revision.php
M includes/Storage/RevisionStore.php
M tests/phpunit/includes/RevisionTest.php
3 files changed, 72 insertions(+), 3 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/93/403693/1
diff --git a/includes/Revision.php b/includes/Revision.php
index 0844e89..8849697 100644
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -495,13 +495,13 @@
$this->mRecord =
self::getRevisionStore()->newMutableRevisionFromArray(
$row,
$queryFlags,
- $title
+ $this->ensureTitle( $row, $queryFlags, $title )
);
} elseif ( is_object( $row ) ) {
$this->mRecord =
self::getRevisionStore()->newRevisionFromRow(
$row,
$queryFlags,
- $title
+ $this->ensureTitle( $row, $queryFlags, $title )
);
} else {
throw new InvalidArgumentException(
@@ -511,6 +511,51 @@
}
/**
+ * Make sure we have *some* Title object for use by the constructor.
+ * For B/C, the constructor shouldn't fail even for a bad page ID or
bad revision ID.
+ *
+ * @param array|object $row
+ * @param int $queryFlags
+ * @param Title|null $title
+ *
+ * @return Title $title if not null, or a Title constructed from
information in $row.
+ */
+ private function ensureTitle( $row, $queryFlags, $title = null ) {
+ if ( $title ) {
+ return $title;
+ }
+
+ if ( is_array( $row ) ) {
+ if ( isset( $row['title'] ) ) {
+ if ( !( $row['title'] instanceof Title ) ) {
+ throw new MWException( 'title field
must contain a Title object.' );
+ }
+
+ return $row['title'];
+ }
+
+ $pageId = isset( $row['page'] ) ? $row['page'] : 0;
+ $revId = isset( $row['id'] ) ? $row['id'] : 0;
+ } else {
+ $pageId = isset( $row->rev_page ) ? $row->rev_page : 0;
+ $revId = isset( $row->rev_id ) ? $row->rev_id : 0;
+ }
+
+ try {
+ $title = self::getRevisionStore()->getTitle( $pageId,
$revId, $queryFlags );
+ } catch ( RevisionAccessException $ex ) {
+ // construct a dummy title!
+ wfLogWarning( __METHOD__ . ': ' . $ex->getMessage() );
+
+ // NOTE: this Title will only be used inside
RevisionRecord
+ $title = Title::makeTitleSafe( NS_SPECIAL,
"Badtitle/ID=$pageId" );
+ $title->resetArticleID( $pageId );
+ }
+
+ return $title;
+ }
+
+ /**
* @return RevisionRecord
*/
public function getRevisionRecord() {
diff --git a/includes/Storage/RevisionStore.php
b/includes/Storage/RevisionStore.php
index 2429e1c..2efaa9d 100644
--- a/includes/Storage/RevisionStore.php
+++ b/includes/Storage/RevisionStore.php
@@ -178,6 +178,8 @@
*
* MCR migration note: this corresponds to Revision::getTitle
*
+ * @note this method should be private, external use should be avoided!
+ *
* @param int|null $pageId
* @param int|null $revId
* @param int $queryFlags
@@ -185,7 +187,7 @@
* @return Title
* @throws RevisionAccessException
*/
- private function getTitle( $pageId, $revId, $queryFlags = 0 ) {
+ public function getTitle( $pageId, $revId, $queryFlags = 0 ) {
if ( !$pageId && !$revId ) {
throw new InvalidArgumentException( '$pageId and $revId
cannot both be 0 or null' );
}
diff --git a/tests/phpunit/includes/RevisionTest.php
b/tests/phpunit/includes/RevisionTest.php
index 0db76ff..7ce43ed 100644
--- a/tests/phpunit/includes/RevisionTest.php
+++ b/tests/phpunit/includes/RevisionTest.php
@@ -76,6 +76,17 @@
$this->assertNull( $rev->getContent(), 'no content object
should be available' );
}
+ /**
+ * @covers Revision::__construct
+ * @covers \MediaWiki\Storage\RevisionStore::newMutableRevisionFromArray
+ */
+ public function testConstructFromArrayWithBadPageId() {
+ MediaWiki\suppressWarnings();
+ $rev = new Revision( [ 'page' => 77777777 ] );
+ $this->assertSame( 77777777, $rev->getPage() );
+ MediaWiki\restoreWarnings();
+ }
+
public function provideConstructFromArray_userSetAsExpected() {
yield 'no user defaults to wgUser' => [
[
@@ -297,6 +308,17 @@
$assertions( $this, $rev );
}
+ /**
+ * @covers Revision::__construct
+ * @covers \MediaWiki\Storage\RevisionStore::newMutableRevisionFromArray
+ */
+ public function testConstructFromRowWithBadPageId() {
+ MediaWiki\suppressWarnings();
+ $rev = new Revision( (object)[ 'rev_page' => 77777777 ] );
+ $this->assertSame( 77777777, $rev->getPage() );
+ MediaWiki\restoreWarnings();
+ }
+
public function provideGetRevisionText() {
yield 'Generic test' => [
'This is a goat of revision text.',
--
To view, visit https://gerrit.wikimedia.org/r/403693
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I18c823d7b72504447982364d581b34e98924b67f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: wmf/1.31.0-wmf.16
Gerrit-Owner: Addshore <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
