Dependency injection and raw error refactor

.github/instructions/mediawiki-extensions-clean-code.instructions.md

@@ -0,0 +1,157 @@
+---
+applyTo: "*"
+---
+
+# MediaWiki Extension Clean Code Best Practices
+
+## Service container pattern
+
+Use the service container pattern (based on MediaWikiServices) to define how key objects are constructed and wired together.
+
+Absolutely no MediaWikiServices references should be in the code because everything should be injected.
+
+## Dependency injection
+
+Use this example from PluggableAuth to implement dependency injection.
+
+Objects should only be instantiated in `ServiceWiring.php`, not in any other file.
+
+Below is an `includes/ServiceWiring.php` file.
+
+```php
+return [
+	'PluggableAuthFactory' =>
+		static function ( MediaWikiServices $services ): PluggableAuthFactory {
+			return new PluggableAuthFactory(
+				new ServiceOptions( PluggableAuthFactory::CONSTRUCTOR_OPTIONS, $services->getMainConfig() ),
+				ExtensionRegistry::getInstance(),
+				$services->getAuthManager(),
+				LoggerFactory::getInstance( 'PluggableAuth' ),
+				$services->getObjectFactory()
+			);
+		},
+	'PluggableAuthService' =>
+		static function ( MediaWikiServices $services ): PluggableAuthService {
+			return new PluggableAuthService(
+				new ServiceOptions( PluggableAuthService::CONSTRUCTOR_OPTIONS, $services->getMainConfig() ),
+				ExtensionRegistry::getInstance(),
+				$services->getUserFactory(),
+				$services->get( 'PluggableAuthFactory' ),
+				$services->get( 'PluggableAuth.GroupProcessorRunner' ),
+				$services->getPermissionManager(),
+				$services->getAuthManager(),
+				LoggerFactory::getInstance( 'PluggableAuth' ),
+				$services->getUrlUtils()
+			);
+		},
+	'PluggableAuth.GroupProcessorFactory' =>
+		static function ( MediaWikiServices $services ): GroupProcessorFactory {
+			$factory = new GroupProcessorFactory(
+				ExtensionRegistry::getInstance()->getAttribute( 'PluggableAuthGroupSyncs' ),
+				$services->getObjectFactory()
+			);
+			$factory->setLogger( LoggerFactory::getInstance( 'PluggableAuth' ) );
+			return $factory;
+		},
+	'PluggableAuth.GroupProcessorRunner' =>
+		static function ( MediaWikiServices $services ): GroupProcessorRunner {
+			$factory = new GroupProcessorRunner(
+				$services->getService( 'PluggableAuth.GroupProcessorFactory' )
+			);
+			$factory->setLogger( LoggerFactory::getInstance( 'PluggableAuth' ) );
+			return $factory;
+		},
+];
+```
+
+The ServiceWiring.php file returns an associative array where:
+
+- Keys are service names (e.g., 'PluggableAuthFactory', 'PluggableAuth.GroupProcessorRunner')
+- Values are factory closures that receive the MediaWikiServices container and return a fully constructed service instance
+- When any code in the extension (or elsewhere) calls $services->get('PluggableAuthFactory'), MediaWiki invokes the corresponding closure, injects all dependencies, and returns the object. Services are typically lazily instantiated and cached (singleton per request).
+
+## Loading LocalSettings.php config values into the extension
+
+All configs should be loaded with dependency injection.
+
+## Consuming services
+
+### 1. Declaratively via extension.json
+
+#### Hook handlers
+
+MediaWiki's ObjectFactory reads service names from `extension.json` and automatically injects them as constructor arguments.
+
+```json
+"HookHandlers": {
+    "main": {
+        "class": "MediaWiki\\Extension\\PluggableAuth\\PluggableAuthHooks",
+        "services": [
+            "PluggableAuthService",
+            "UrlUtils"
+        ]
+    }
+},
+```
+
+### 2. Internally between services
+
+Services reference each other via `$services->get(...)`.
+
+For example, in the MediaWiki PluggableAuth extension:
+
+- PluggableAuthService depends on PluggableAuthFactory and PluggableAuth.GroupProcessorRunner
+- PluggableAuth.GroupProcessorRunner depends on PluggableAuth.GroupProcessorFactory
+
+## Testing services
+
+Dependency injection means code is easily testable. Below is an example of a test file for services.
+
+```php
+class PluggableAuthServiceTest extends MediaWikiIntegrationTestCase {
+
+	/**
+	 * @param array $links
+	 * @param array $expectedLinks
+	 * @param array $options
+	 * @param bool $shouldOverrideDefaultLogout
+	 * @param string $msg
+	 * @return void
+	 * @throws \PHPUnit\Framework\MockObject\Exception
+	 * @covers       MediaWiki\Extension\PluggableAuth\PluggableAuthService::modifyLogoutLink
+	 * @dataProvider provideTestModifyLogoutLinkData
+	 */
+	public function testModifyLogoutLink( $links, $expectedLinks, $options, $shouldOverrideDefaultLogout, $msg ) {
+		$serviceOptions = new ServiceOptions(
+			PluggableAuthService::CONSTRUCTOR_OPTIONS,
+			$options
+		);
+		$extensionRegistry = $this->createMock( ExtensionRegistry::class );
+		$userFactory = $this->createMock( UserFactory::class );
+		$pluggableAuthPlugin = $this->createMock( PluggableAuthPlugin::class );
+		$pluggableAuthPlugin->method( 'shouldOverrideDefaultLogout' )->willReturn( $shouldOverrideDefaultLogout );
+		$pluggableAuthFactory = $this->createMock( PluggableAuthFactory::class );
+		$pluggableAuthFactory->method( 'getInstance' )->willReturn( $pluggableAuthPlugin );
+		$groupProcessorRunner = $this->createMock( GroupProcessorRunner::class );
+		$permissionManager = $this->createMock( PermissionManager::class );
+		$authManager = $this->createMock( AuthManager::class );
+		$logger = $this->createMock( LoggerInterface::class );
+		$service = new PluggableAuthService(
+			$serviceOptions,
+			$extensionRegistry,
+			$userFactory,
+			$pluggableAuthFactory,
+			$groupProcessorRunner,
+			$permissionManager,
+			$authManager,
+			$logger,
+			$this->getServiceContainer()->getUrlUtils()
+		);
+
+		$service->modifyLogoutLink( $links );
+
+		$this->assertEquals( $expectedLinks, $links, $msg );
+	}
+
+}
+```

.github/workflows/ci.yml

@@ -25,9 +25,6 @@ jobs:
         php: [ '8.2', '8.3', '8.4' ]
         mediawiki: [ REL1_43 ]
         include:
-          - os: ubuntu-latest
-            php: '7.4'
-            mediawiki: REL1_35
           - os: ubuntu-latest
             php: '7.4'
             mediawiki: REL1_39
@@ -52,8 +49,9 @@ jobs:
         with:
           path: extensions/${{ env.EXTNAME }}
       - name: Setup Composer
+        # Don't block insecure packages because MediaWiki might be using outdated dependencies, which shouldn't block the extension's pipelines from running
         run: |
-          echo '{"extra":{"merge-plugin":{"include":["extensions/*/composer.json","skins/*/composer.json"]}}}' > composer.local.json
+          echo '{"extra":{"merge-plugin":{"include":["extensions/*/composer.json","skins/*/composer.json"]}},"config":{"audit":{"abandoned":"report","block-insecure":false}}}' > composer.local.json
           composer update
           composer update
       - name: Lint
@@ -72,9 +70,6 @@ jobs:
         php: [ '8.2', '8.3' ]
         mediawiki: [ REL1_43 ]
         include:
-          - os: ubuntu-latest
-            php: '7.4'
-            mediawiki: REL1_35
           - os: ubuntu-latest
             php: '7.4'
             mediawiki: REL1_39
@@ -99,8 +94,9 @@ jobs:
         with:
           path: extensions/${{ env.EXTNAME }}
       - name: Setup Composer
+        # Don't block insecure packages because MediaWiki might be using outdated dependencies, which shouldn't block the extension's pipelines from running
         run: |
-          echo '{"extra":{"merge-plugin":{"include":["extensions/*/composer.json","skins/*/composer.json"]}}}' > composer.local.json
+          echo '{"extra":{"merge-plugin":{"include":["extensions/*/composer.json","skins/*/composer.json"]}},"config":{"audit":{"abandoned":"report","block-insecure":false}}}' > composer.local.json
           composer update
           composer update
       - name: Phan
@@ -115,9 +111,6 @@ jobs:
         php: [ '8.2', '8.3', '8.4' ]
         mediawiki: [ REL1_43 ]
         include:
-          - os: ubuntu-latest
-            php: '7.4'
-            mediawiki: REL1_35
           - os: ubuntu-latest
             php: '7.4'
             mediawiki: REL1_39
@@ -142,8 +135,9 @@ jobs:
         with:
           path: extensions/${{ env.EXTNAME }}
       - name: Setup Composer
+        # Don't block insecure packages because MediaWiki might be using outdated dependencies, which shouldn't block the extension's pipelines from running
         run: |
-          echo '{"extra":{"merge-plugin":{"include":["extensions/*/composer.json","skins/*/composer.json"]}}}' > composer.local.json
+          echo '{"extra":{"merge-plugin":{"include":["extensions/*/composer.json","skins/*/composer.json"]}},"config":{"audit":{"abandoned":"report","block-insecure":false}}}' > composer.local.json
           composer update
           composer update
       - name: Install MediaWiki

extension.json

@@ -1,19 +1,21 @@
 {
     "name": "CrawlerProtection",
     "author": "MyWikis LLC",
-    "version": "1.1.0",
+    "version": "1.2.0",
     "description": "Suite of protective measures to protect wikis from crawlers.",
     "type": "hook",
     "requires": {
-        "MediaWiki": ">= 1.35"
+        "MediaWiki": ">= 1.39.4"
     },
     "AutoloadNamespaces": {
         "MediaWiki\\Extension\\CrawlerProtection\\": "includes/"
     },
     "HookHandlers": {
         "main": {
             "class": "MediaWiki\\Extension\\CrawlerProtection\\Hooks",
-            "services": []
+            "services": [
+                "CrawlerProtection.CrawlerProtectionService"
+            ]
         }
     },
     "Hooks": {
@@ -28,10 +30,22 @@
                 "whatlinkshere"
             ]
         },
+        "CrawlerProtectionRawDenial": {
+            "value": false
+        },
         "CrawlerProtectionUse418": {
             "value": false
+        },
+        "CrawlerProtectionRawDenialHeader": {
+            "value": "HTTP/1.0 403 Forbidden"
+        },
+        "CrawlerProtectionRawDenialText": {
+            "value": "403 Forbidden. You must be logged in to view this page."
         }
     },
+    "ServiceWiringFiles": [
+        "includes/ServiceWiring.php"
+    ],
     "license-name": "MIT",
     "Tests": {
         "phpunit": "tests/phpunit"