jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/403909 )
Change subject: RevisionStore, fix loadSlotContent with no $blobFlags
......................................................................
RevisionStore, fix loadSlotContent with no $blobFlags
This includes tests that were previously created in:
I6dcfc0497bfce6605fa5517c9f91faf7131f4334
Bug: T184749
Change-Id: Ieb02ac593fc6b42af1692d03d9d578a76417eb54
---
M includes/Storage/RevisionStore.php
M tests/phpunit/includes/Storage/RevisionStoreDbTest.php
2 files changed, 150 insertions(+), 15 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/Storage/RevisionStore.php
b/includes/Storage/RevisionStore.php
index 6c8cac9..b0c193b 100644
--- a/includes/Storage/RevisionStore.php
+++ b/includes/Storage/RevisionStore.php
@@ -689,7 +689,7 @@
$content = null;
$blobData = null;
- $blobFlags = '';
+ $blobFlags = null;
if ( is_object( $row ) ) {
// archive row
@@ -706,7 +706,11 @@
if ( isset( $row->old_text ) ) {
// this happens when the text-table gets joined
directly, in the pre-1.30 schema
$blobData = isset( $row->old_text ) ? strval(
$row->old_text ) : null;
- $blobFlags = isset( $row->old_flags ) ? strval(
$row->old_flags ) : '';
+ // Check against selects that might have not
included old_flags
+ if ( !property_exists( $row, 'old_flags' ) ) {
+ throw new InvalidArgumentException(
'old_flags was not set in $row' );
+ }
+ $blobFlags = ( $row->old_flags === null ) ? ''
: $row->old_flags;
}
$mainSlotRow->slot_revision = intval( $row->rev_id );
@@ -735,7 +739,9 @@
$mainSlotRow->format_name = isset(
$row['content_format'] )
? strval( $row['content_format'] ) : null;
$blobData = isset( $row['text'] ) ? rtrim( strval(
$row['text'] ) ) : null;
- $blobFlags = isset( $row['flags'] ) ? trim( strval(
$row['flags'] ) ) : '';
+ // XXX: If the flags field is not set then $blobFlags
should be null so that no
+ // decoding will happen. An empty string will result in
default decodings.
+ $blobFlags = isset( $row['flags'] ) ? trim( strval(
$row['flags'] ) ) : null;
// if we have a Content object, override mText and
mContentModel
if ( !empty( $row['content'] ) ) {
@@ -797,7 +803,8 @@
*
* @param SlotRecord $slot The SlotRecord to load content for
* @param string|null $blobData The content blob, in the form indicated
by $blobFlags
- * @param string $blobFlags Flags indicating how $blobData needs to be
processed
+ * @param string|null $blobFlags Flags indicating how $blobData needs
to be processed.
+ * null if no processing should happen.
* @param string|null $blobFormat MIME type indicating how $dataBlob is
encoded
* @param int $queryFlags
*
@@ -807,23 +814,27 @@
private function loadSlotContent(
SlotRecord $slot,
$blobData = null,
- $blobFlags = '',
+ $blobFlags = null,
$blobFormat = null,
$queryFlags = 0
) {
if ( $blobData !== null ) {
Assert::parameterType( 'string', $blobData, '$blobData'
);
- Assert::parameterType( 'string', $blobFlags,
'$blobFlags' );
+ Assert::parameterType( 'string|null', $blobFlags,
'$blobFlags' );
$cacheKey = $slot->hasAddress() ? $slot->getAddress() :
null;
- $data = $this->blobStore->expandBlob( $blobData,
$blobFlags, $cacheKey );
-
- if ( $data === false ) {
- throw new RevisionAccessException(
- "Failed to expand blob data using flags
$blobFlags (key: $cacheKey)"
- );
+ if ( $blobFlags === null ) {
+ $data = $blobData;
+ } else {
+ $data = $this->blobStore->expandBlob(
$blobData, $blobFlags, $cacheKey );
+ if ( $data === false ) {
+ throw new RevisionAccessException(
+ "Failed to expand blob data
using flags $blobFlags (key: $cacheKey)"
+ );
+ }
}
+
} else {
$address = $slot->getAddress();
try {
diff --git a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
index 15f173a..e33de20 100644
--- a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
+++ b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
@@ -6,8 +6,10 @@
use Exception;
use HashBagOStuff;
use InvalidArgumentException;
+use Language;
use MediaWiki\Linker\LinkTarget;
use MediaWiki\MediaWikiServices;
+use MediaWiki\Storage\BlobStoreFactory;
use MediaWiki\Storage\IncompleteRevisionException;
use MediaWiki\Storage\MutableRevisionRecord;
use MediaWiki\Storage\RevisionRecord;
@@ -611,9 +613,10 @@
*/
public function testNewRevisionFromRow_anonEdit() {
$page = WikiPage::factory( Title::newFromText( 'UTPage' ) );
+ $text = __METHOD__ . 'a-ä';
/** @var Revision $rev */
$rev = $page->doEditContent(
- new WikitextContent( __METHOD__. 'a' ),
+ new WikitextContent( $text ),
__METHOD__. 'a'
)->value['revision'];
@@ -624,6 +627,32 @@
$page->getTitle()
);
$this->assertRevisionRecordMatchesRevision( $rev, $record );
+ $this->assertSame( $text, $rev->getContent()->serialize() );
+ }
+
+ /**
+ * @covers \MediaWiki\Storage\RevisionStore::newRevisionFromRow
+ * @covers \MediaWiki\Storage\RevisionStore::newRevisionFromRow_1_29
+ */
+ public function testNewRevisionFromRow_anonEdit_legacyEncoding() {
+ $this->setMwGlobals( 'wgLegacyEncoding', 'windows-1252' );
+ $this->overrideMwServices();
+ $page = WikiPage::factory( Title::newFromText( 'UTPage' ) );
+ $text = __METHOD__ . 'a-ä';
+ /** @var Revision $rev */
+ $rev = $page->doEditContent(
+ new WikitextContent( $text ),
+ __METHOD__. 'a'
+ )->value['revision'];
+
+ $store = MediaWikiServices::getInstance()->getRevisionStore();
+ $record = $store->newRevisionFromRow(
+ $this->revisionToRow( $rev ),
+ [],
+ $page->getTitle()
+ );
+ $this->assertRevisionRecordMatchesRevision( $rev, $record );
+ $this->assertSame( $text, $rev->getContent()->serialize() );
}
/**
@@ -632,9 +661,10 @@
*/
public function testNewRevisionFromRow_userEdit() {
$page = WikiPage::factory( Title::newFromText( 'UTPage' ) );
+ $text = __METHOD__ . 'b-ä';
/** @var Revision $rev */
$rev = $page->doEditContent(
- new WikitextContent( __METHOD__. 'b' ),
+ new WikitextContent( $text ),
__METHOD__ . 'b',
0,
false,
@@ -648,6 +678,7 @@
$page->getTitle()
);
$this->assertRevisionRecordMatchesRevision( $rev, $record );
+ $this->assertSame( $text, $rev->getContent()->serialize() );
}
/**
@@ -656,9 +687,10 @@
public function testNewRevisionFromArchiveRow() {
$store = MediaWikiServices::getInstance()->getRevisionStore();
$title = Title::newFromText( __METHOD__ );
+ $text = __METHOD__ . '-bä';
$page = WikiPage::factory( $title );
/** @var Revision $orig */
- $orig = $page->doEditContent( new WikitextContent( __METHOD__
), __METHOD__ )
+ $orig = $page->doEditContent( new WikitextContent( $text ),
__METHOD__ )
->value['revision'];
$page->doDeleteArticle( __METHOD__ );
@@ -675,6 +707,38 @@
$record = $store->newRevisionFromArchiveRow( $row );
$this->assertRevisionRecordMatchesRevision( $orig, $record );
+ $this->assertSame( $text, $record->getContent( 'main'
)->serialize() );
+ }
+
+ /**
+ * @covers \MediaWiki\Storage\RevisionStore::newRevisionFromArchiveRow
+ */
+ public function testNewRevisionFromArchiveRow_legacyEncoding() {
+ $this->setMwGlobals( 'wgLegacyEncoding', 'windows-1252' );
+ $this->overrideMwServices();
+ $store = MediaWikiServices::getInstance()->getRevisionStore();
+ $title = Title::newFromText( __METHOD__ );
+ $text = __METHOD__ . '-bä';
+ $page = WikiPage::factory( $title );
+ /** @var Revision $orig */
+ $orig = $page->doEditContent( new WikitextContent( $text ),
__METHOD__ )
+ ->value['revision'];
+ $page->doDeleteArticle( __METHOD__ );
+
+ $db = wfGetDB( DB_MASTER );
+ $arQuery = $store->getArchiveQueryInfo();
+ $res = $db->select(
+ $arQuery['tables'], $arQuery['fields'], [ 'ar_rev_id'
=> $orig->getId() ],
+ __METHOD__, [], $arQuery['joins']
+ );
+ $this->assertTrue( is_object( $res ), 'query failed' );
+
+ $row = $res->fetchObject();
+ $res->free();
+ $record = $store->newRevisionFromArchiveRow( $row );
+
+ $this->assertRevisionRecordMatchesRevision( $orig, $record );
+ $this->assertSame( $text, $record->getContent( 'main'
)->serialize() );
}
/**
@@ -1026,6 +1090,39 @@
'content' => new WikitextContent( 'Some
Content' ),
]
];
+ yield 'Basic array, serialized text' => [
+ [
+ 'id' => 2,
+ 'page' => 1,
+ 'timestamp' => '20171017114835',
+ 'user_text' => '111.0.1.2',
+ 'user' => 0,
+ 'minor_edit' => false,
+ 'deleted' => 0,
+ 'len' => 46,
+ 'parent_id' => 1,
+ 'sha1' => 'rdqbbzs3pkhihgbs8qf2q9jsvheag5z',
+ 'comment' => 'Goat Comment!',
+ 'text' => ( new WikitextContent( 'Söme Content'
) )->serialize(),
+ ]
+ ];
+ yield 'Basic array, serialized text, utf-8 flags' => [
+ [
+ 'id' => 2,
+ 'page' => 1,
+ 'timestamp' => '20171017114835',
+ 'user_text' => '111.0.1.2',
+ 'user' => 0,
+ 'minor_edit' => false,
+ 'deleted' => 0,
+ 'len' => 46,
+ 'parent_id' => 1,
+ 'sha1' => 'rdqbbzs3pkhihgbs8qf2q9jsvheag5z',
+ 'comment' => 'Goat Comment!',
+ 'text' => ( new WikitextContent( 'Söme Content'
) )->serialize(),
+ 'flags' => 'utf-8',
+ ]
+ ];
yield 'Basic array, with title' => [
[
'title' => Title::newFromText( 'SomeText' ),
@@ -1092,6 +1189,8 @@
$this->assertTrue(
$result->getSlot( 'main'
)->getContent()->equals( $array['content'] )
);
+ } elseif ( isset( $array['text'] ) ) {
+ $this->assertSame( $array['text'], $result->getSlot(
'main' )->getContent()->serialize() );
} else {
$this->assertSame(
$array['content_format'],
@@ -1101,4 +1200,29 @@
}
}
+ /**
+ * @dataProvider provideNewMutableRevisionFromArray
+ * @covers \MediaWiki\Storage\RevisionStore::newMutableRevisionFromArray
+ */
+ public function testNewMutableRevisionFromArray_legacyEncoding( array
$array ) {
+ $cache = new WANObjectCache( [ 'cache' => new HashBagOStuff() ]
);
+ $blobStore = new SqlBlobStore( wfGetLB(), $cache );
+ $blobStore->setLegacyEncoding( 'windows-1252',
Language::factory( 'en' ) );
+
+ $factory = $this->getMockBuilder( BlobStoreFactory::class )
+ ->setMethods( [ 'newBlobStore', 'newSqlBlobStore' ] )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $factory->expects( $this->any() )
+ ->method( 'newBlobStore' )
+ ->willReturn( $blobStore );
+ $factory->expects( $this->any() )
+ ->method( 'newSqlBlobStore' )
+ ->willReturn( $blobStore );
+
+ $this->setService( 'BlobStoreFactory', $factory );
+
+ $this->testNewMutableRevisionFromArray( $array );
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/403909
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ieb02ac593fc6b42af1692d03d9d578a76417eb54
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Lucas Werkmeister (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
