Deprecate ContentHandler::makeParserOptions()

Having a different ParserOptions for each content model isn't feasible
in an MCR world. And the only thing using this was Wikibase, which has
been fixed to do what it needs in a different way.

Bug: T194263
Change-Id: I01373b29ee25fa9346c6b0317155be4ccdc8c515

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index 25ea4f6..98bb03f 100644 (file)
--- a/RELEASE-NOTES-1.32
+++ b/RELEASE-NOTES-1.32
@@ -248,6 +248,8 @@ because of Phabricator reports.
   OutputPage::showFileCopyError() and OutputPage::showUnexpectedValueError().
 * The Replacer, DoubleReplacer, HashtableReplacer, and RegexlikeReplacer
   classes are now deprecated. Use a Closure instead.
+* (T194263) ContentHandler::makeParserOptions() is deprecated. Use
+  WikiPage::makeParserOptions() or ParserOptions::newCanonical() instead.
 
 === Other changes in 1.32 ===
 * (T198811) The following tables have had their UNIQUE indexes turned into proper

diff --git a/includes/content/AbstractContent.php b/includes/content/AbstractContent.php
index 284cab2..b6211b0 100644 (file)
--- a/includes/content/AbstractContent.php
+++ b/includes/content/AbstractContent.php
@@ -501,7 +501,7 @@ abstract class AbstractContent implements Content {
                ParserOptions $options = null, $generateHtml = true
        ) {
                if ( $options === null ) {
-                       $options = $this->getContentHandler()->makeParserOptions( 'canonical' );
+                       $options = ParserOptions::newCanonical( 'canonical' );
                }
 
                $po = new ParserOutput();

diff --git a/includes/content/ContentHandler.php b/includes/content/ContentHandler.php
index 3c5ee26..e628aef 100644 (file)
--- a/includes/content/ContentHandler.php
+++ b/includes/content/ContentHandler.php
@@ -1114,6 +1114,8 @@ abstract class ContentHandler {
        /**
         * Get parser options suitable for rendering and caching the article
         *
+        * @deprecated since 1.32, use WikiPage::makeParserOptions() or
+        *  ParserOptions::newCanonical() instead.
         * @param IContextSource|User|string $context One of the following:
         *        - IContextSource: Use the User and the Language of the provided
         *                                            context
@@ -1126,22 +1128,7 @@ abstract class ContentHandler {
         * @return ParserOptions
         */
        public function makeParserOptions( $context ) {
-               global $wgContLang;
-
-               if ( $context instanceof IContextSource ) {
-                       $user = $context->getUser();
-                       $lang = $context->getLanguage();
-               } elseif ( $context instanceof User ) { // settings per user (even anons)
-                       $user = $context;
-                       $lang = null;
-               } elseif ( $context === 'canonical' ) { // canonical settings
-                       $user = new User;
-                       $lang = $wgContLang;
-               } else {
-                       throw new MWException( "Bad context for parser options: $context" );
-               }
-
-               return ParserOptions::newCanonical( $user, $lang );
+               return ParserOptions::newCanonical( $context );
        }
 
        /**

diff --git a/includes/jobqueue/jobs/CategoryMembershipChangeJob.php b/includes/jobqueue/jobs/CategoryMembershipChangeJob.php
index 16640be..a0c70ab 100644 (file)
--- a/includes/jobqueue/jobs/CategoryMembershipChangeJob.php
+++ b/includes/jobqueue/jobs/CategoryMembershipChangeJob.php
@@ -173,7 +173,7 @@ class CategoryMembershipChangeJob extends Job {
                }
 
                // Parse the new revision and get the categories
-               $categoryChanges = $this->getExplicitCategoriesChanges( $title, $newRev, $oldRev );
+               $categoryChanges = $this->getExplicitCategoriesChanges( $page, $newRev, $oldRev );
                list( $categoryInserts, $categoryDeletes ) = $categoryChanges;
                if ( !$categoryInserts && !$categoryDeletes ) {
                        return; // nothing to do
@@ -203,7 +203,7 @@ class CategoryMembershipChangeJob extends Job {
        }
 
        private function getExplicitCategoriesChanges(
-               Title $title, Revision $newRev, Revision $oldRev = null
+               WikiPage $page, Revision $newRev, Revision $oldRev = null
        ) {
                // Inject the same timestamp for both revision parses to avoid seeing category changes
                // due to time-based parser functions. Inject the same page title for the parses too.
@@ -213,10 +213,10 @@ class CategoryMembershipChangeJob extends Job {
                // assumes these updates are perfectly FIFO and that link tables are always
                // up to date, neither of which are true.
                $oldCategories = $oldRev
-                       ? $this->getCategoriesAtRev( $title, $oldRev, $parseTimestamp )
+                       ? $this->getCategoriesAtRev( $page, $oldRev, $parseTimestamp )
                        : [];
                // Parse the new revision and get the categories
-               $newCategories = $this->getCategoriesAtRev( $title, $newRev, $parseTimestamp );
+               $newCategories = $this->getCategoriesAtRev( $page, $newRev, $parseTimestamp );
 
                $categoryInserts = array_values( array_diff( $newCategories, $oldCategories ) );
                $categoryDeletes = array_values( array_diff( $oldCategories, $newCategories ) );
@@ -225,19 +225,19 @@ class CategoryMembershipChangeJob extends Job {
        }
 
        /**
-        * @param Title $title
+        * @param WikiPage $page
         * @param Revision $rev
         * @param string $parseTimestamp TS_MW
         *
         * @return string[] category names
         */
-       private function getCategoriesAtRev( Title $title, Revision $rev, $parseTimestamp ) {
+       private function getCategoriesAtRev( WikiPage $page, Revision $rev, $parseTimestamp ) {
                $content = $rev->getContent();
-               $options = $content->getContentHandler()->makeParserOptions( 'canonical' );
+               $options = $page->makeParserOptions( 'canonical' );
                $options->setTimestamp( $parseTimestamp );
                // This could possibly use the parser cache if it checked the revision ID,
                // but that's more complicated than it's worth.
-               $output = $content->getParserOutput( $title, $rev->getId(), $options );
+               $output = $content->getParserOutput( $page->getTitle(), $rev->getId(), $options );
 
                // array keys will cast numeric category names to ints
                // so we need to cast them back to strings to avoid breaking things!

diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php
index f34e894..7cc25bd 100644 (file)
--- a/includes/page/WikiPage.php
+++ b/includes/page/WikiPage.php
@@ -1870,7 +1870,7 @@ class WikiPage implements Page, IDBAccessObject {
        /**
         * Get parser options suitable for rendering the primary article wikitext
         *
-        * @see ContentHandler::makeParserOptions
+        * @see ParserOptions::newCanonical
         *
         * @param IContextSource|User|string $context One of the following:
         *        - IContextSource: Use the User and the Language of the provided
@@ -1882,7 +1882,7 @@ class WikiPage implements Page, IDBAccessObject {
         * @return ParserOptions
         */
        public function makeParserOptions( $context ) {
-               $options = $this->getContentHandler()->makeParserOptions( $context );
+               $options = ParserOptions::newCanonical( $context );
 
                if ( $this->getTitle()->isConversionTable() ) {
                        // @todo ConversionTable should become a separate content model, so

diff --git a/includes/parser/ParserOptions.php b/includes/parser/ParserOptions.php
index 3a7a1d6..c8e68b2 100644 (file)
--- a/includes/parser/ParserOptions.php
+++ b/includes/parser/ParserOptions.php
@@ -41,6 +41,13 @@ use Wikimedia\ScopedCallback;
  */
 class ParserOptions {
 
+       /**
+        * Flag indicating that newCanonical() accepts an IContextSource or the string 'canonical', for
+        * back-compat checks from extensions.
+        * @since 1.32
+        */
+       const HAS_NEWCANONICAL_FROM_CONTEXT = 1;
+
        /**
         * Default values for all options that are relevant for caching.
         * @see self::getDefaults()
@@ -930,8 +937,7 @@ class ParserOptions {
 
        /**
         * @warning For interaction with the parser cache, use
-        *  WikiPage::makeParserOptions(), ContentHandler::makeParserOptions(), or
-        *  ParserOptions::newCanonical() instead.
+        *  WikiPage::makeParserOptions() or ParserOptions::newCanonical() instead.
         * @param User|null $user
         * @param Language|null $lang
         */
@@ -957,8 +963,7 @@ class ParserOptions {
        /**
         * Get a ParserOptions object for an anonymous user
         * @warning For interaction with the parser cache, use
-        *  WikiPage::makeParserOptions(), ContentHandler::makeParserOptions(), or
-        *  ParserOptions::newCanonical() instead.
+        *  WikiPage::makeParserOptions() or ParserOptions::newCanonical() instead.
         * @since 1.27
         * @return ParserOptions
         */
@@ -972,8 +977,7 @@ class ParserOptions {
         * Language will be taken from $wgLang.
         *
         * @warning For interaction with the parser cache, use
-        *  WikiPage::makeParserOptions(), ContentHandler::makeParserOptions(), or
-        *  ParserOptions::newCanonical() instead.
+        *  WikiPage::makeParserOptions() or ParserOptions::newCanonical() instead.
         * @param User $user
         * @return ParserOptions
         */
@@ -985,8 +989,7 @@ class ParserOptions {
         * Get a ParserOptions object from a given user and language
         *
         * @warning For interaction with the parser cache, use
-        *  WikiPage::makeParserOptions(), ContentHandler::makeParserOptions(), or
-        *  ParserOptions::newCanonical() instead.
+        *  WikiPage::makeParserOptions() or ParserOptions::newCanonical() instead.
         * @param User $user
         * @param Language $lang
         * @return ParserOptions
@@ -999,8 +1002,7 @@ class ParserOptions {
         * Get a ParserOptions object from a IContextSource object
         *
         * @warning For interaction with the parser cache, use
-        *  WikiPage::makeParserOptions(), ContentHandler::makeParserOptions(), or
-        *  ParserOptions::newCanonical() instead.
+        *  WikiPage::makeParserOptions() or ParserOptions::newCanonical() instead.
         * @param IContextSource $context
         * @return ParserOptions
         */
@@ -1015,12 +1017,29 @@ class ParserOptions {
         * different from the canonical values used for caching.
         *
         * @since 1.30
-        * @param User|null $user
-        * @param Language|StubObject|null $lang
+        * @since 1.32 Added string and IContextSource as options for the first parameter
+        * @param IContextSource|string|User|null $context
+        *  - If an IContextSource, the options are initialized based on the source's User and Language.
+        *  - If the string 'canonical', the options are initialized with an anonymous user and
+        *    $wgContLang.
+        *  - If a User or null, the options are initialized for that User (or $wgUser if null).
+        *    'userlang' is taken from the $userLang parameter, defaulting to $wgLang if that is null.
+        * @param Language|StubObject|null $userLang (see above)
         * @return ParserOptions
         */
-       public static function newCanonical( User $user = null, $lang = null ) {
-               $ret = new ParserOptions( $user, $lang );
+       public static function newCanonical( $context = null, $userLang = null ) {
+               if ( $context instanceof IContextSource ) {
+                       $ret = self::newFromContext( $context );
+               } elseif ( $context === 'canonical' ) {
+                       $ret = self::newFromAnon();
+               } elseif ( $context instanceof User || $context === null ) {
+                       $ret = new self( $context, $userLang );
+               } else {
+                       throw new InvalidArgumentException(
+                               '$context must be an IContextSource, the string "canonical", a User, or null'
+                       );
+               }
+
                foreach ( self::getCanonicalOverrides() as $k => $v ) {
                        $ret->setOption( $k, $v );
                }

diff --git a/tests/phpunit/includes/content/WikitextContentTest.php b/tests/phpunit/includes/content/WikitextContentTest.php
index 1db6aab..c78bc5b 100644 (file)
--- a/tests/phpunit/includes/content/WikitextContentTest.php
+++ b/tests/phpunit/includes/content/WikitextContentTest.php
@@ -377,7 +377,7 @@ just a test"
                $wikitext = false;
                $redirectTarget = false;
                $content = $this->newContent( 'hello world.' );
-               $options = $content->getContentHandler()->makeParserOptions( 'canonical' );
+               $options = ParserOptions::newCanonical( 'canonical' );
                $options->setRedirectTarget( $title );
                $content->getParserOutput( $title, null, $options );
                $this->assertEquals( 'hello world.', $wikitext,
@@ -394,7 +394,7 @@ just a test"
                $content = $this->newContent(
                        "#REDIRECT [[TestRedirectParserOption/redir]]\nhello redirect."
                );
-               $options = $content->getContentHandler()->makeParserOptions( 'canonical' );
+               $options = ParserOptions::newCanonical( 'canonical' );
                $content->getParserOutput( $title, null, $options );
                $this->assertEquals(
                        'hello redirect.',

diff --git a/tests/phpunit/includes/parser/ParserOptionsTest.php b/tests/phpunit/includes/parser/ParserOptionsTest.php
index 8c61b03..48205f4 100644 (file)
--- a/tests/phpunit/includes/parser/ParserOptionsTest.php
+++ b/tests/phpunit/includes/parser/ParserOptionsTest.php
@@ -42,6 +42,68 @@ class ParserOptionsTest extends MediaWikiTestCase {
                parent::tearDown();
        }
 
+       public function testNewCanonical() {
+               $wgUser = $this->getMutableTestUser()->getUser();
+               $wgLang = Language::factory( 'fr' );
+               $wgContLang = Language::factory( 'qqx' );
+
+               $this->setMwGlobals( [
+                       'wgUser' => $wgUser,
+                       'wgLang' => $wgLang,
+                       'wgContLang' => $wgContLang,
+               ] );
+
+               $user = $this->getMutableTestUser()->getUser();
+               $lang = Language::factory( 'de' );
+               $lang2 = Language::factory( 'bug' );
+               $context = new DerivativeContext( RequestContext::getMain() );
+               $context->setUser( $user );
+               $context->setLanguage( $lang );
+
+               // No parameters picks up $wgUser and $wgLang
+               $popt = ParserOptions::newCanonical();
+               $this->assertSame( $wgUser, $popt->getUser() );
+               $this->assertSame( $wgLang, $popt->getUserLangObj() );
+
+               // Just a user uses $wgLang
+               $popt = ParserOptions::newCanonical( $user );
+               $this->assertSame( $user, $popt->getUser() );
+               $this->assertSame( $wgLang, $popt->getUserLangObj() );
+
+               // Just a language uses $wgUser
+               $popt = ParserOptions::newCanonical( null, $lang );
+               $this->assertSame( $wgUser, $popt->getUser() );
+               $this->assertSame( $lang, $popt->getUserLangObj() );
+
+               // Passing both works
+               $popt = ParserOptions::newCanonical( $user, $lang );
+               $this->assertSame( $user, $popt->getUser() );
+               $this->assertSame( $lang, $popt->getUserLangObj() );
+
+               // Passing 'canonical' uses an anon and $wgContLang, and ignores
+               // any passed $userLang
+               $popt = ParserOptions::newCanonical( 'canonical' );
+               $this->assertTrue( $popt->getUser()->isAnon() );
+               $this->assertSame( $wgContLang, $popt->getUserLangObj() );
+               $popt = ParserOptions::newCanonical( 'canonical', $lang2 );
+               $this->assertSame( $wgContLang, $popt->getUserLangObj() );
+
+               // Passing an IContextSource uses the user and lang from it, and ignores
+               // any passed $userLang
+               $popt = ParserOptions::newCanonical( $context );
+               $this->assertSame( $user, $popt->getUser() );
+               $this->assertSame( $lang, $popt->getUserLangObj() );
+               $popt = ParserOptions::newCanonical( $context, $lang2 );
+               $this->assertSame( $lang, $popt->getUserLangObj() );
+
+               // Passing something else raises an exception
+               try {
+                       $popt = ParserOptions::newCanonical( 'bogus' );
+                       $this->fail( 'Excpected exception not thrown' );
+               } catch ( InvalidArgumentException $ex ) {
+               }
+       }
+
        /**
         * @dataProvider provideIsSafeToCache
         * @param bool $expect Expected value