Merge "Return stderr from Shell\Command"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 12 Oct 2017 09:21:08 +0000 (09:21 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 12 Oct 2017 09:21:08 +0000 (09:21 +0000)
includes/CategoryViewer.php
includes/CommentStore.php
includes/GlobalFunctions.php
includes/installer/PostgresUpdater.php
maintenance/postgres/archives/patch-add-3d.sql [deleted file]
tests/phpunit/includes/CommentStoreTest.php
tests/phpunit/includes/RevisionStorageTest.php
tests/phpunit/includes/RevisionStorageTestContentHandlerUseDB.php [deleted file]

index 9d692d7..f36c758 100644 (file)
@@ -629,7 +629,7 @@ class CategoryViewer extends ContextSource {
         * @return string HTML
         */
        private function pagingLinks( $first, $last, $type = '' ) {
-               $prevLink = $this->msg( 'prev-page' )->text();
+               $prevLink = $this->msg( 'prev-page' )->escaped();
 
                $linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer();
                if ( $first != '' ) {
@@ -638,13 +638,13 @@ class CategoryViewer extends ContextSource {
                        unset( $prevQuery["{$type}from"] );
                        $prevLink = $linkRenderer->makeKnownLink(
                                $this->addFragmentToTitle( $this->title, $type ),
-                               $prevLink,
+                               new HtmlArmor( $prevLink ),
                                [],
                                $prevQuery
                        );
                }
 
-               $nextLink = $this->msg( 'next-page' )->text();
+               $nextLink = $this->msg( 'next-page' )->escaped();
 
                if ( $last != '' ) {
                        $lastQuery = $this->query;
@@ -652,7 +652,7 @@ class CategoryViewer extends ContextSource {
                        unset( $lastQuery["{$type}until"] );
                        $nextLink = $linkRenderer->makeKnownLink(
                                $this->addFragmentToTitle( $this->title, $type ),
-                               $nextLink,
+                               new HtmlArmor( $nextLink ),
                                [],
                                $lastQuery
                        );
index b8a31e6..0d679d3 100644 (file)
@@ -29,10 +29,24 @@ use Wikimedia\Rdbms\IDatabase;
  */
 class CommentStore {
 
-       /** Maximum length of a comment. Longer comments will be truncated. */
+       /**
+        * Maximum length of a comment in UTF-8 characters. Longer comments will be truncated.
+        * @note This must be at least 255 and not greater than floor( MAX_COMMENT_LENGTH / 4 ).
+        */
+       const COMMENT_CHARACTER_LIMIT = 1000;
+
+       /**
+        * Maximum length of a comment in bytes. Longer comments will be truncated.
+        * @note This value is determined by the size of the underlying database field,
+        *  currently BLOB in MySQL/MariaDB.
+        */
        const MAX_COMMENT_LENGTH = 65535;
 
-       /** Maximum length of serialized data. Longer data will result in an exception. */
+       /**
+        * Maximum length of serialized data in bytes. Longer data will result in an exception.
+        * @note This value is determined by the size of the underlying database field,
+        *  currently BLOB in MySQL/MariaDB.
+        */
        const MAX_DATA_LENGTH = 65535;
 
        /**
@@ -371,6 +385,15 @@ class CommentStore {
 
                # Truncate comment in a Unicode-sensitive manner
                $comment->text = $this->lang->truncate( $comment->text, self::MAX_COMMENT_LENGTH );
+               if ( mb_strlen( $comment->text, 'UTF-8' ) > self::COMMENT_CHARACTER_LIMIT ) {
+                       $ellipsis = wfMessage( 'ellipsis' )->inLanguage( $this->lang )->escaped();
+                       if ( mb_strlen( $ellipsis ) >= self::COMMENT_CHARACTER_LIMIT ) {
+                               // WTF?
+                               $ellipsis = '...';
+                       }
+                       $maxLength = self::COMMENT_CHARACTER_LIMIT - mb_strlen( $ellipsis, 'UTF-8' );
+                       $comment->text = mb_substr( $comment->text, 0, $maxLength, 'UTF-8' ) . $ellipsis;
+               }
 
                if ( $this->stage > MIGRATION_OLD && !$comment->id ) {
                        $dbData = $comment->data;
index 069e1be..d53e98d 100644 (file)
@@ -2259,6 +2259,7 @@ function wfEscapeShellArg( /*...*/ ) {
  * @deprecated since 1.30 use MediaWiki\Shell::isDisabled()
  */
 function wfShellExecDisabled() {
+       wfDeprecated( __FUNCTION__, '1.30' );
        return Shell::isDisabled() ? 'disabled' : false;
 }
 
index 07aeb13..0475fe4 100644 (file)
@@ -454,7 +454,7 @@ class PostgresUpdater extends DatabaseUpdater {
                        [ 'addPgIndex', 'user_groups', 'user_groups_expiry', '( ug_expiry )' ],
 
                        // 1.30
-                       [ 'modifyField', 'image', 'img_media_type', 'patch-add-3d.sql' ],
+                       [ 'addPgEnumValue', 'media_type', '3D' ],
                        [ 'setDefault', 'revision', 'rev_comment', '' ],
                        [ 'changeNullableField', 'revision', 'rev_comment', 'NOT NULL', true ],
                        [ 'setDefault', 'archive', 'ar_comment', '' ],
@@ -838,6 +838,46 @@ END;
                }
        }
 
+       /**
+        * Add a value to an existing PostgreSQL enum type
+        * @since 1.31
+        * @param string $type Type name. Must be in the core schema.
+        * @param string $value Value to add.
+        */
+       public function addPgEnumValue( $type, $value ) {
+               $row = $this->db->selectRow(
+                       [
+                               't' => 'pg_catalog.pg_type',
+                               'n' => 'pg_catalog.pg_namespace',
+                               'e' => 'pg_catalog.pg_enum',
+                       ],
+                       [ 't.typname', 't.typtype', 'e.enumlabel' ],
+                       [
+                               't.typname' => $type,
+                               'n.nspname' => $this->db->getCoreSchema(),
+                       ],
+                       __METHOD__,
+                       [],
+                       [
+                               'n' => [ 'JOIN', 't.typnamespace = n.oid' ],
+                               'e' => [ 'LEFT JOIN', [ 'e.enumtypid = t.oid', 'e.enumlabel' => $value ] ],
+                       ]
+               );
+
+               if ( !$row ) {
+                       $this->output( "...Type $type does not exist, skipping modify enum.\n" );
+               } elseif ( $row->typtype !== 'e' ) {
+                       $this->output( "...Type $type does not seem to be an enum, skipping modify enum.\n" );
+               } elseif ( $row->enumlabel === $value ) {
+                       $this->output( "...Enum type $type already contains value '$value'.\n" );
+               } else {
+                       $this->output( "...Adding value '$value' to enum type $type.\n" );
+                       $etype = $this->db->addIdentifierQuotes( $type );
+                       $evalue = $this->db->addQuotes( $value );
+                       $this->db->query( "ALTER TYPE $etype ADD VALUE $evalue" );
+               }
+       }
+
        protected function dropFkey( $table, $field ) {
                $fi = $this->db->fieldInfo( $table, $field );
                if ( is_null( $fi ) ) {
diff --git a/maintenance/postgres/archives/patch-add-3d.sql b/maintenance/postgres/archives/patch-add-3d.sql
deleted file mode 100644 (file)
index f892755..0000000
+++ /dev/null
@@ -1 +0,0 @@
-ALTER TYPE media_type ADD VALUE '3D';
index b0f7678..9369f30 100644 (file)
@@ -618,7 +618,7 @@ class CommentStoreTest extends MediaWikiLangTestCase {
        public function testInsertTruncation() {
                $comment = str_repeat( '💣', 16400 );
                $truncated1 = str_repeat( '💣', 63 ) . '...';
-               $truncated2 = str_repeat( '💣', 16383 ) . '...';
+               $truncated2 = str_repeat( '💣', CommentStore::COMMENT_CHARACTER_LIMIT - 3 ) . '...';
 
                $store = $this->makeStore( MIGRATION_WRITE_BOTH, 'ipb_reason' );
                $fields = $store->insert( $this->db, $comment );
index 08f128a..a15b9b4 100644 (file)
@@ -39,15 +39,31 @@ class RevisionStorageTest extends MediaWikiTestCase {
        }
 
        protected function setUp() {
-               global $wgExtraNamespaces, $wgNamespaceContentModels, $wgContentHandlers, $wgContLang;
+               global $wgContLang;
 
                parent::setUp();
 
-               $wgExtraNamespaces[12312] = 'Dummy';
-               $wgExtraNamespaces[12313] = 'Dummy_talk';
+               $this->mergeMwGlobalArrayValue(
+                       'wgExtraNamespaces',
+                       [
+                               12312 => 'Dummy',
+                               12313 => 'Dummy_talk',
+                       ]
+               );
 
-               $wgNamespaceContentModels[12312] = 'DUMMY';
-               $wgContentHandlers['DUMMY'] = 'DummyContentHandlerForTesting';
+               $this->mergeMwGlobalArrayValue(
+                       'wgNamespaceContentModels',
+                       [
+                               12312 => 'DUMMY',
+                       ]
+               );
+
+               $this->mergeMwGlobalArrayValue(
+                       'wgContentHandlers',
+                       [
+                               'DUMMY' => 'DummyContentHandlerForTesting',
+                       ]
+               );
 
                MWNamespace::clearCaches();
                // Reset namespace cache
@@ -64,16 +80,10 @@ class RevisionStorageTest extends MediaWikiTestCase {
        }
 
        protected function tearDown() {
-               global $wgExtraNamespaces, $wgNamespaceContentModels, $wgContentHandlers, $wgContLang;
+               global $wgContLang;
 
                parent::tearDown();
 
-               unset( $wgExtraNamespaces[12312] );
-               unset( $wgExtraNamespaces[12313] );
-
-               unset( $wgNamespaceContentModels[12312] );
-               unset( $wgContentHandlers['DUMMY'] );
-
                MWNamespace::clearCaches();
                // Reset namespace cache
                $wgContLang->resetNamespaces();
@@ -104,30 +114,33 @@ class RevisionStorageTest extends MediaWikiTestCase {
                return $rev;
        }
 
-       protected function createPage( $page, $text, $model = null ) {
-               if ( is_string( $page ) ) {
-                       if ( !preg_match( '/:/', $page ) &&
-                               ( $model === null || $model === CONTENT_MODEL_WIKITEXT )
-                       ) {
-                               $ns = $this->getDefaultWikitextNS();
-                               $page = MWNamespace::getCanonicalName( $ns ) . ':' . $page;
-                       }
-
-                       $page = Title::newFromText( $page );
+       /**
+        * @param string $titleString
+        * @param string $text
+        * @param string|null $model
+        *
+        * @return WikiPage
+        */
+       protected function createPage( $titleString, $text, $model = null ) {
+               if ( !preg_match( '/:/', $titleString ) &&
+                       ( $model === null || $model === CONTENT_MODEL_WIKITEXT )
+               ) {
+                       $ns = $this->getDefaultWikitextNS();
+                       $titleString = MWNamespace::getCanonicalName( $ns ) . ':' . $titleString;
                }
 
-               if ( $page instanceof Title ) {
-                       $page = new WikiPage( $page );
-               }
+               $title = Title::newFromText( $titleString );
+               $wikipage = new WikiPage( $title );
 
-               if ( $page->exists() ) {
-                       $page->doDeleteArticle( "done" );
+               // Delete the article if it already exists
+               if ( $wikipage->exists() ) {
+                       $wikipage->doDeleteArticle( "done" );
                }
 
-               $content = ContentHandler::makeContent( $text, $page->getTitle(), $model );
-               $page->doEditContent( $content, "testing", EDIT_NEW );
+               $content = ContentHandler::makeContent( $text, $title, $model );
+               $wikipage->doEditContent( $content, __METHOD__, EDIT_NEW );
 
-               return $page;
+               return $wikipage;
        }
 
        protected function assertRevEquals( Revision $orig, Revision $rev = null ) {
@@ -160,6 +173,56 @@ class RevisionStorageTest extends MediaWikiTestCase {
                $this->assertRevEquals( $orig, $rev );
        }
 
+       /**
+        * @covers Revision::newFromTitle
+        */
+       public function testNewFromTitle_withoutId() {
+               $page = $this->createPage(
+                       __METHOD__,
+                       'GOAT',
+                       CONTENT_MODEL_WIKITEXT
+               );
+               $latestRevId = $page->getLatest();
+
+               $rev = Revision::newFromTitle( $page->getTitle() );
+
+               $this->assertTrue( $page->getTitle()->equals( $rev->getTitle() ) );
+               $this->assertEquals( $latestRevId, $rev->getId() );
+       }
+
+       /**
+        * @covers Revision::newFromTitle
+        */
+       public function testNewFromTitle_withId() {
+               $page = $this->createPage(
+                       __METHOD__,
+                       'GOAT',
+                       CONTENT_MODEL_WIKITEXT
+               );
+               $latestRevId = $page->getLatest();
+
+               $rev = Revision::newFromTitle( $page->getTitle(), $latestRevId );
+
+               $this->assertTrue( $page->getTitle()->equals( $rev->getTitle() ) );
+               $this->assertEquals( $latestRevId, $rev->getId() );
+       }
+
+       /**
+        * @covers Revision::newFromTitle
+        */
+       public function testNewFromTitle_withBadId() {
+               $page = $this->createPage(
+                       __METHOD__,
+                       'GOAT',
+                       CONTENT_MODEL_WIKITEXT
+               );
+               $latestRevId = $page->getLatest();
+
+               $rev = Revision::newFromTitle( $page->getTitle(), $latestRevId + 1 );
+
+               $this->assertNull( $rev );
+       }
+
        /**
         * @covers Revision::newFromRow
         */
diff --git a/tests/phpunit/includes/RevisionStorageTestContentHandlerUseDB.php b/tests/phpunit/includes/RevisionStorageTestContentHandlerUseDB.php
deleted file mode 100644 (file)
index 9e667f2..0000000
+++ /dev/null
@@ -1,89 +0,0 @@
-<?php
-
-/**
- * @group ContentHandler
- * @group Database
- * ^--- important, causes temporary tables to be used instead of the real database
- */
-class RevisionTestContentHandlerUseDB extends RevisionStorageTest {
-
-       protected function setUp() {
-               $this->setMwGlobals( 'wgContentHandlerUseDB', false );
-
-               $dbw = wfGetDB( DB_MASTER );
-
-               $page_table = $dbw->tableName( 'page' );
-               $revision_table = $dbw->tableName( 'revision' );
-               $archive_table = $dbw->tableName( 'archive' );
-
-               if ( $dbw->fieldExists( $page_table, 'page_content_model' ) ) {
-                       $dbw->query( "alter table $page_table drop column page_content_model" );
-                       $dbw->query( "alter table $revision_table drop column rev_content_model" );
-                       $dbw->query( "alter table $revision_table drop column rev_content_format" );
-                       $dbw->query( "alter table $archive_table drop column ar_content_model" );
-                       $dbw->query( "alter table $archive_table drop column ar_content_format" );
-               }
-
-               parent::setUp();
-       }
-
-       /**
-        * @covers Revision::selectFields
-        */
-       public function testSelectFields() {
-               $fields = Revision::selectFields();
-
-               $this->assertTrue( in_array( 'rev_id', $fields ), 'missing rev_id in list of fields' );
-               $this->assertTrue( in_array( 'rev_page', $fields ), 'missing rev_page in list of fields' );
-               $this->assertTrue(
-                       in_array( 'rev_timestamp', $fields ),
-                       'missing rev_timestamp in list of fields'
-               );
-               $this->assertTrue( in_array( 'rev_user', $fields ), 'missing rev_user in list of fields' );
-
-               $this->assertFalse(
-                       in_array( 'rev_content_model', $fields ),
-                       'missing rev_content_model in list of fields'
-               );
-               $this->assertFalse(
-                       in_array( 'rev_content_format', $fields ),
-                       'missing rev_content_format in list of fields'
-               );
-       }
-
-       /**
-        * @covers Revision::getContentModel
-        */
-       public function testGetContentModel() {
-               try {
-                       $this->makeRevision( [ 'text' => 'hello hello.',
-                               'content_model' => CONTENT_MODEL_JAVASCRIPT ] );
-
-                       $this->fail( "Creating JavaScript content on a wikitext page should fail with "
-                               . "\$wgContentHandlerUseDB disabled" );
-               } catch ( MWException $ex ) {
-                       $this->assertTrue( true ); // ok
-               }
-       }
-
-       /**
-        * @covers Revision::getContentFormat
-        */
-       public function testGetContentFormat() {
-               try {
-                       // @todo change this to test failure on using a non-standard (but supported) format
-                       //       for a content model supported in the given location. As of 1.21, there are
-                       //       no alternative formats for any of the standard content models that could be
-                       //       used for this though.
-
-                       $this->makeRevision( [ 'text' => 'hello hello.',
-                               'content_model' => CONTENT_MODEL_JAVASCRIPT,
-                               'content_format' => 'text/javascript' ] );
-
-                       $this->fail( "Creating JavaScript content on a wikitext page should fail with "
-                               . "\$wgContentHandlerUseDB disabled" );
-               } catch ( MWException $ex ) {
-                       $this->assertTrue( true ); // ok
-               }
-       }
-}