RevisionStore, fix loadSlotContent with no $blobFlags

This includes tests that were previously created in:
I6dcfc0497bfce6605fa5517c9f91faf7131f4334

Bug: T184749
Change-Id: Ieb02ac593fc6b42af1692d03d9d578a76417eb54

diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php
index 6c8cac9..b0c193b 100644 (file)
--- a/includes/Storage/RevisionStore.php
+++ b/includes/Storage/RevisionStore.php
@@ -689,7 +689,7 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
 
                $content = null;
                $blobData = null;
-               $blobFlags = '';
+               $blobFlags = null;
 
                if ( is_object( $row ) ) {
                        // archive row
@@ -706,7 +706,11 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
                        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 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
                        $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 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
         *
         * @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 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
        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 (file)
--- a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
+++ b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
@@ -6,8 +6,10 @@ use CommentStoreComment;
 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 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
         */
        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 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
                        $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 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
         */
        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 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
                        $page->getTitle()
                );
                $this->assertRevisionRecordMatchesRevision( $rev, $record );
+               $this->assertSame( $text, $rev->getContent()->serialize() );
        }
 
        /**
@@ -656,9 +687,41 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
        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( $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() );
+       }
+
+       /**
+        * @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( __METHOD__ ), __METHOD__ )
+               $orig = $page->doEditContent( new WikitextContent( $text ), __METHOD__ )
                        ->value['revision'];
                $page->doDeleteArticle( __METHOD__ );
 
@@ -675,6 +738,7 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
                $record = $store->newRevisionFromArchiveRow( $row );
 
                $this->assertRevisionRecordMatchesRevision( $orig, $record );
+               $this->assertSame( $text, $record->getContent( 'main' )->serialize() );
        }
 
        /**
@@ -1026,6 +1090,39 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
                                '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 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
                        $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 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
                }
        }
 
+       /**
+        * @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 );
+       }
+
 }