Resolve used lazy options in ParserOptions::optionsHash()

If a lazy option is passed to ParserOptions::optionsHash(), we should
resolve the option so the hash can incorporate the proper value instead
of omitting it.

Also, completely unrelatedly, refactor the hook overriding in the unit
test because people won't stop whining about it in code review.

Change-Id: I2df78ed90875c229090b503b65f20fbbbba7f237

diff --git a/includes/parser/ParserOptions.php b/includes/parser/ParserOptions.php
index 8fb9857..20bd599 100644 (file)
--- a/includes/parser/ParserOptions.php
+++ b/includes/parser/ParserOptions.php
@@ -1275,9 +1275,17 @@ class ParserOptions {
        public function optionsHash( $forOptions, $title = null ) {
                global $wgRenderHashAppend;
 
+               $inCacheKey = self::allCacheVaryingOptions();
+
+               // Resolve any lazy options
+               foreach ( array_intersect( $forOptions, $inCacheKey, array_keys( self::$lazyOptions ) ) as $k ) {
+                       if ( $this->options[$k] === null ) {
+                               $this->options[$k] = call_user_func( self::$lazyOptions[$k], $this, $k );
+                       }
+               }
+
                $options = $this->options;
                $defaults = self::getCanonicalOverrides() + self::getDefaults();
-               $inCacheKey = self::$inCacheKey;
 
                // We only include used options with non-canonical values in the key
                // so adding a new option doesn't invalidate the entire parser cache.
@@ -1285,13 +1293,11 @@ class ParserOptions {
                // requires manual invalidation of existing cache entries, as mentioned
                // in the docs on the relevant methods and hooks.
                $values = [];
-               foreach ( $inCacheKey as $option => $include ) {
-                       if ( $include && in_array( $option, $forOptions, true ) ) {
-                               $v = $this->optionToString( $options[$option] );
-                               $d = $this->optionToString( $defaults[$option] );
-                               if ( $v !== $d ) {
-                                       $values[] = "$option=$v";
-                               }
+               foreach ( array_intersect( $inCacheKey, $forOptions ) as $option ) {
+                       $v = $this->optionToString( $options[$option] );
+                       $d = $this->optionToString( $defaults[$option] );
+                       if ( $v !== $d ) {
+                               $values[] = "$option=$v";
                        }
                }
 

diff --git a/tests/phpunit/includes/parser/ParserOptionsTest.php b/tests/phpunit/includes/parser/ParserOptionsTest.php
index e2ed1d5..8c61b03 100644 (file)
--- a/tests/phpunit/includes/parser/ParserOptionsTest.php
+++ b/tests/phpunit/includes/parser/ParserOptionsTest.php
@@ -25,17 +25,16 @@ class ParserOptionsTest extends MediaWikiTestCase {
        }
 
        protected function setUp() {
-               global $wgHooks;
-
                parent::setUp();
                self::clearCache();
 
                $this->setMwGlobals( [
                        'wgRenderHashAppend' => '',
-                       'wgHooks' => [
-                               'PageRenderingHash' => [],
-                       ] + $wgHooks,
                ] );
+
+               // This is crazy, but registering false, null, or other falsey values
+               // as a hook callback "works".
+               $this->setTemporaryHook( 'PageRenderingHash', null );
        }
 
        protected function tearDown() {
@@ -84,17 +83,13 @@ class ParserOptionsTest extends MediaWikiTestCase {
         * @param string $expect Expected value
         * @param array $options Options to set
         * @param array $globals Globals to set
+        * @param callable|null $hookFunc PageRenderingHash hook function
         */
-       public function testOptionsHash( $usedOptions, $expect, $options, $globals = [] ) {
-               global $wgHooks;
-
-               $globals += [
-                       'wgHooks' => [],
-               ];
-               $globals['wgHooks'] += [
-                       'PageRenderingHash' => [],
-               ] + $wgHooks;
+       public function testOptionsHash(
+               $usedOptions, $expect, $options, $globals = [], $hookFunc = null
+       ) {
                $this->setMwGlobals( $globals );
+               $this->setTemporaryHook( 'PageRenderingHash', $hookFunc );
 
                $popt = ParserOptions::newCanonical();
                foreach ( $options as $name => $value ) {
@@ -129,14 +124,50 @@ class ParserOptionsTest extends MediaWikiTestCase {
                                [],
                                'canonical!wgRenderHashAppend!onPageRenderingHash',
                                [],
-                               [
-                                       'wgRenderHashAppend' => '!wgRenderHashAppend',
-                                       'wgHooks' => [ 'PageRenderingHash' => [ [ __CLASS__ . '::onPageRenderingHash' ] ] ],
-                               ]
+                               [ 'wgRenderHashAppend' => '!wgRenderHashAppend' ],
+                               [ __CLASS__ . '::onPageRenderingHash' ],
                        ],
                ];
        }
 
+       public function testUsedLazyOptionsInHash() {
+               $this->setTemporaryHook( 'ParserOptionsRegister',
+                       function ( &$defaults, &$inCacheKey, &$lazyOptions ) {
+                               $lazyFuncs = $this->getMockBuilder( stdClass::class )
+                                       ->setMethods( [ 'neverCalled', 'calledOnce' ] )
+                                       ->getMock();
+                               $lazyFuncs->expects( $this->never() )->method( 'neverCalled' );
+                               $lazyFuncs->expects( $this->once() )->method( 'calledOnce' )->willReturn( 'value' );
+
+                               $defaults += [
+                                       'opt1' => null,
+                                       'opt2' => null,
+                                       'opt3' => null,
+                               ];
+                               $inCacheKey += [
+                                       'opt1' => true,
+                                       'opt2' => true,
+                               ];
+                               $lazyOptions += [
+                                       'opt1' => [ $lazyFuncs, 'calledOnce' ],
+                                       'opt2' => [ $lazyFuncs, 'neverCalled' ],
+                                       'opt3' => [ $lazyFuncs, 'neverCalled' ],
+                               ];
+                       }
+               );
+
+               self::clearCache();
+
+               $popt = ParserOptions::newCanonical();
+               $popt->registerWatcher( function () {
+                       $this->fail( 'Watcher should not have been called' );
+               } );
+               $this->assertSame( 'opt1=value', $popt->optionsHash( [ 'opt1', 'opt3' ] ) );
+
+               // Second call to see that opt1 isn't resolved a second time
+               $this->assertSame( 'opt1=value', $popt->optionsHash( [ 'opt1', 'opt3' ] ) );
+       }
+
        public static function onPageRenderingHash( &$confstr ) {
                $confstr .= '!onPageRenderingHash';
        }
@@ -192,10 +223,7 @@ class ParserOptionsTest extends MediaWikiTestCase {
        }
 
        public function testAllCacheVaryingOptions() {
-               global $wgHooks;
-
-               // $wgHooks is already saved in self::setUp(), so we can modify it freely here
-               $wgHooks['ParserOptionsRegister'] = [];
+               $this->setTemporaryHook( 'ParserOptionsRegister', null );
                $this->assertSame( [
                        'dateformat', 'numberheadings', 'printable', 'stubthreshold',
                        'thumbsize', 'userlang'
@@ -203,7 +231,7 @@ class ParserOptionsTest extends MediaWikiTestCase {
 
                self::clearCache();
 
-               $wgHooks['ParserOptionsRegister'][] = function ( &$defaults, &$inCacheKey ) {
+               $this->setTemporaryHook( 'ParserOptionsRegister', function ( &$defaults, &$inCacheKey ) {
                        $defaults += [
                                'foo' => 'foo',
                                'bar' => 'bar',
@@ -213,7 +241,7 @@ class ParserOptionsTest extends MediaWikiTestCase {
                                'foo' => true,
                                'bar' => false,
                        ];
-               };
+               } );
                $this->assertSame( [
                        'dateformat', 'foo', 'numberheadings', 'printable', 'stubthreshold',
                        'thumbsize', 'userlang'