output: Use associative arrays to store modules

Why:

- OutputPage::addModules() and OutputPage::addModuleStyles() currently
  store added module names in two lists, mModules and mModuleStyles.
- The contents of these lists then get passed to array_unique() when
  OutputPage::getModules() is invoked, to filter out any duplicate
  module names.
- In I4f70ff15becbc4991c4f1b0307a14d5354c79dc1, we would like to modify
  UserLinkRenderer::userLink() to automatically add the requisite
  ResourceLoader modules to the output, instead of every page where
  userLink() is called needing to do so manually.
  This can cause these modules to be added thousands of times when a
  large list with many user links is rendered, e.g. on RecentChanges.
- Running `php maintenance/run.php benchmarks/benchmarkEval output.php`,
  where output.php holds:
  ```
  $out = ( new RequestContext() )->getOutput();
  for ( $j = 1; $j <= 2000; $j++ ) {
  	$out->addModules( 'ext.foo' );
  }

  $out->getModules();
  ```
  takes about 4-5ms on my system.
- Turning the properties into associative arrays and getting rid of the
  array_unique() call drops this to < 0.2ms.
- These properties are exposed via public property accessors that have
  been deprecated since MW 1.38 and have no usages in codesearch.[1]
  Since we're changing how the properties work, this is a good time to
  remove the deprecated public property accessors.

What:

- Use associative arrays to store module names in OutputPage.
- Document that addModules() and addModuleStyles() take a
  string|string[] rather than a string or an arbitrary array.
- Add unit tests for the behavior.
- Drop the deprecated public property accessors for mModules and
  mModuleStyles.

[1] https://codesearch.wmcloud.org/search/?q=mModule%5BSs%5D

Bug: T358469
Change-Id: I86c557a4ce7207359d100538c0d4b1ffa75fcbf9

1 files changed, 145 insertions, 0 deletions

diff --git a/tests/phpunit/unit/includes/Output/OutputPageUnitTest.php b/tests/phpunit/unit/includes/Output/OutputPageUnitTest.php
new file mode 100644
index 000000000000..46dd7dc93968
--- /dev/null
+++ b/tests/phpunit/unit/includes/Output/OutputPageUnitTest.php
@@ -0,0 +1,145 @@
+<?php
+namespace MediaWiki\Tests\Unit\Output;
+
+use MediaWiki\Output\OutputPage;
+use MediaWiki\ResourceLoader\Module;
+use MediaWiki\ResourceLoader\ResourceLoader;
+use MediaWikiUnitTestCase;
+
+/**
+ * @covers \MediaWiki\Output\OutputPage
+ */
+class OutputPageUnitTest extends MediaWikiUnitTestCase {
+	/**
+	 * @dataProvider provideGetModules
+	 *
+	 * @param string|string[] $addedModules Module name or list of module names to add to the output via
+	 * addModules()
+	 * @param string|string[] $addedStyleModules Module name or list of module names to add to the output via
+	 * addModuleStyles()
+	 * @param int|null $maxAllowedLevel Maximum allowed origin level to set via reduceAllowedModules(),
+	 * given as a Module::ORIGIN_* constant, or `null` for no restriction.
+	 * @param bool $filter Whether getModules() should filter the returned module list based on active origin
+	 * restrictions
+	 * @param string[] $expectedModules Expected list of module names to be returned by getModules()
+	 * @param string[] $expectedStyleModules Expected list of style module names to be returned by getModules()
+	 */
+	public function testGetModules(
+		$addedModules,
+		$addedStyleModules,
+		?int $maxAllowedLevel,
+		bool $filter,
+		array $expectedModules,
+		array $expectedStyleModules
+	): void {
+		$moduleMapping = [];
+
+		$moduleOrigins = [
+			'test.user.js' => Module::ORIGIN_USER_INDIVIDUAL,
+			'test.site.js' => Module::ORIGIN_USER_SITEWIDE,
+			'test.ext.js' => Module::ORIGIN_CORE_SITEWIDE,
+
+			'test.user.css' => Module::ORIGIN_USER_INDIVIDUAL,
+			'test.site.css' => Module::ORIGIN_USER_SITEWIDE,
+			'test.ext.css' => Module::ORIGIN_CORE_SITEWIDE,
+		];
+
+		foreach ( $moduleOrigins as $moduleName => $moduleOrigin ) {
+			$module = $this->createMock( Module::class );
+			$module->method( 'getOrigin' )
+				->willReturn( $moduleOrigin );
+
+			$moduleMapping[] = [ $moduleName, $module ];
+		}
+
+		$resourceLoader = $this->createMock( ResourceLoader::class );
+		$resourceLoader->method( 'getModule' )
+			->willReturnMap( $moduleMapping );
+
+		$outputPage = $this->getMockBuilder( OutputPage::class )
+			->onlyMethods( [ 'getResourceLoader' ] )
+			->disableOriginalConstructor()
+			->getMock();
+		$outputPage->method( 'getResourceLoader' )
+			->willReturn( $resourceLoader );
+
+		$outputPage->addModules( $addedModules );
+		$outputPage->addModuleStyles( $addedStyleModules );
+
+		if ( $maxAllowedLevel !== null ) {
+			$outputPage->reduceAllowedModules( Module::TYPE_STYLES, $maxAllowedLevel );
+			$outputPage->reduceAllowedModules( Module::TYPE_SCRIPTS, $maxAllowedLevel );
+		}
+
+		$modules = $outputPage->getModules( $filter );
+		$styleModules = $outputPage->getModules( $filter, null, 'mModuleStyles' );
+
+		$this->assertSame( $expectedModules, $modules );
+		$this->assertSame( $expectedStyleModules, $styleModules );
+	}
+
+	public static function provideGetModules(): iterable {
+		yield 'single module' => [
+			'test.ext.js',
+			[],
+			null,
+			false,
+			[ 'test.ext.js' ],
+			[]
+		];
+
+		yield 'single user-level module' => [
+			'test.user.js',
+			[],
+			null,
+			false,
+			[ 'test.user.js' ],
+			[]
+		];
+
+		yield 'duplicate additions of the same module' => [
+			[ 'test.ext.js', 'test.ext.js' ],
+			[],
+			null,
+			false,
+			[ 'test.ext.js' ],
+			[]
+		];
+
+		yield 'duplicate modules and style modules' => [
+			[ 'test.ext.js', 'test.ext.js', 'test.user.js' ],
+			[ 'test.ext.css', 'test.ext.css' ],
+			null,
+			false,
+			[ 'test.ext.js', 'test.user.js' ],
+			[ 'test.ext.css' ]
+		];
+
+		yield 'filtered module listing without module level restrictions' => [
+			[ 'test.user.js', 'test.site.js', 'test.ext.js' ],
+			[ 'test.user.css', 'test.site.css', 'test.ext.css' ],
+			null,
+			true,
+			[ 'test.user.js', 'test.site.js', 'test.ext.js' ],
+			[ 'test.user.css', 'test.site.css', 'test.ext.css' ],
+		];
+
+		yield 'filtered module listing with user-level modules restricted' => [
+			[ 'test.user.js', 'test.site.js', 'test.ext.js' ],
+			[ 'test.user.css', 'test.site.css', 'test.ext.css' ],
+			Module::ORIGIN_CORE_SITEWIDE,
+			true,
+			[ 'test.ext.js' ],
+			[ 'test.ext.css' ]
+		];
+
+		yield 'unfiltered module listing with user-level modules restricted' => [
+			[ 'test.user.js', 'test.site.js', 'test.ext.js' ],
+			[ 'test.user.css', 'test.site.css', 'test.ext.css' ],
+			Module::ORIGIN_CORE_SITEWIDE,
+			false,
+			[ 'test.user.js', 'test.site.js', 'test.ext.js' ],
+			[ 'test.user.css', 'test.site.css', 'test.ext.css' ],
+		];
+	}
+}