From 9d98cc1ae651c421265a23e09588b47f7b65c48e Mon Sep 17 00:00:00 2001 From: Umherirrender Date: Fri, 13 Jan 2023 00:26:06 +0100 Subject: [PATCH] Add a CategoryCache service for use on Special:TrackingCategories Replace the dynamic property on SpecialTrackingCategories object, this was used to store preloaded Category objects. Dynamic properties are deprecated in php8.2 The preload and storage is now done with the service. Adjust doc to LinkTarget (Id0cc2ca) Bug: T324897 Change-Id: I891ad79bc357d32585ef4d9206d398c5a75222aa --- extension.json | 8 +- includes/CategoryCache.php | 99 +++++++++++++++++++ includes/Hooks.php | 48 +++------ includes/ServiceWiring.php | 40 ++++++++ tests/phpunit/CategoryCacheTest.php | 86 ++++++++++++++++ .../phpunit/CategoryTreeServiceWiringTest.php | 28 ++++++ 6 files changed, 274 insertions(+), 35 deletions(-) create mode 100644 includes/CategoryCache.php create mode 100644 includes/ServiceWiring.php create mode 100644 tests/phpunit/CategoryCacheTest.php create mode 100644 tests/phpunit/CategoryTreeServiceWiringTest.php diff --git a/extension.json b/extension.json index 5752fabe..4344d72f 100644 --- a/extension.json +++ b/extension.json @@ -42,6 +42,9 @@ "AutoloadNamespaces": { "MediaWiki\\Extension\\CategoryTree\\": "includes/" }, + "TestAutoloadNamespaces": { + "MediaWiki\\Extension\\CategoryTree\\Tests\\": "tests/phpunit/" + }, "ResourceModules": { "ext.categoryTree": { "localBasePath": "modules/ext.categoryTree", @@ -89,7 +92,7 @@ "HookHandlers": { "default": { "class": "MediaWiki\\Extension\\CategoryTree\\Hooks", - "services": [ "DBLoadBalancer", "MainConfig" ] + "services": [ "CategoryTree.CategoryCache", "MainConfig" ] }, "config": { "class": "MediaWiki\\Extension\\CategoryTree\\ConfigHookHandler" @@ -183,5 +186,8 @@ } } }, + "ServiceWiringFiles": [ + "includes/ServiceWiring.php" + ], "manifest_version": 2 } diff --git a/includes/CategoryCache.php b/includes/CategoryCache.php new file mode 100644 index 00000000..a54620f0 --- /dev/null +++ b/includes/CategoryCache.php @@ -0,0 +1,99 @@ +loadBalancer = $loadBalancer; + } + + /** + * Get a preloaded Category object or null when the Category does not exists. Loaded the Category on demand, + * if not in cache, use self::doQuery when requesting a high number of category + * @param LinkTarget $categoryTarget + * @return ?Category + */ + public function getCategory( LinkTarget $categoryTarget ): ?Category { + if ( $categoryTarget->getNamespace() !== NS_CATEGORY ) { + return null; + } + $categoryDbKey = $categoryTarget->getDBkey(); + + if ( !array_key_exists( $categoryDbKey, $this->cache ) ) { + $this->doQuery( [ $categoryTarget ] ); + } + + return $this->cache[$categoryDbKey]; + } + + /** + * Preloads category counts in this cache + * @param LinkTarget[] $linkTargets + */ + public function doQuery( array $linkTargets ): void { + $categoryDbKeys = []; + foreach ( $linkTargets as $linkTarget ) { + if ( $linkTarget->getNamespace() !== NS_CATEGORY ) { + continue; + } + $categoryDbKey = $linkTarget->getDBkey(); + if ( !array_key_exists( $categoryDbKey, $this->cache ) ) { + $categoryDbKeys[] = $categoryDbKey; + // To cache db misses, also avoid duplicates in the db query + $this->cache[$categoryDbKey] = null; + } + } + if ( $categoryDbKeys === [] ) { + return; + } + + $rows = $this->loadBalancer->getConnection( ILoadBalancer::DB_REPLICA ) + ->newSelectQueryBuilder() + ->select( [ 'cat_id', 'cat_title', 'cat_pages', 'cat_subcats', 'cat_files' ] ) + ->from( 'category' ) + ->where( [ 'cat_title' => $categoryDbKeys ] ) + ->caller( __METHOD__ ) + ->fetchResultSet(); + + foreach ( $rows as $row ) { + $this->cache[$row->cat_title] = Category::newFromRow( $row ); + } + } + +} diff --git a/includes/Hooks.php b/includes/Hooks.php index 3772e596..4f0b2d70 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -25,7 +25,6 @@ namespace MediaWiki\Extension\CategoryTree; use Article; -use Category; use Config; use Html; use IContextSource; @@ -35,6 +34,7 @@ use MediaWiki\Hook\ParserFirstCallInitHook; use MediaWiki\Hook\SkinBuildSidebarHook; use MediaWiki\Hook\SpecialTrackingCategories__generateCatLinkHook; use MediaWiki\Hook\SpecialTrackingCategories__preprocessHook; +use MediaWiki\Linker\LinkTarget; use MediaWiki\Page\Hook\ArticleFromTitleHook; use OutputPage; use Parser; @@ -44,7 +44,6 @@ use Sanitizer; use Skin; use SpecialPage; use Title; -use Wikimedia\Rdbms\ILoadBalancer; /** * Hooks for the CategoryTree extension, an AJAX based gadget @@ -64,18 +63,18 @@ class Hooks implements private const EXTENSION_DATA_FLAG = 'CategoryTree'; - /** @var ILoadBalancer */ - private $loadBalancer; + /** @var CategoryCache */ + private $categoryCache; /** @var Config */ private $config; /** - * @param ILoadBalancer $loadBalancer + * @param CategoryCache $categoryCache * @param Config $config */ - public function __construct( ILoadBalancer $loadBalancer, Config $config ) { - $this->loadBalancer = $loadBalancer; + public function __construct( CategoryCache $categoryCache, Config $config ) { + $this->categoryCache = $categoryCache; $this->config = $config; } @@ -281,52 +280,33 @@ class Hooks implements /** * Hook handler for the SpecialTrackingCategories::preprocess hook - * @suppress PhanUndeclaredProperty SpecialPage->categoryTreeCategories * @param SpecialPage $specialPage SpecialTrackingCategories object - * @param array $trackingCategories [ 'msg' => Title, 'cats' => Title[] ] - * @phan-param array $trackingCategories + * @param array $trackingCategories [ 'msg' => LinkTarget, 'cats' => LinkTarget[] ] + * @phan-param array $trackingCategories */ public function onSpecialTrackingCategories__preprocess( $specialPage, $trackingCategories ) { - $categoryDbKeys = []; - foreach ( $trackingCategories as $catMsg => $data ) { + $categoryTargets = []; + foreach ( $trackingCategories as $data ) { foreach ( $data['cats'] as $catTitle ) { - $categoryDbKeys[] = $catTitle->getDbKey(); + $categoryTargets[] = $catTitle; } } - $categories = []; - if ( $categoryDbKeys ) { - $dbr = $this->loadBalancer->getConnectionRef( DB_REPLICA ); - $res = $dbr->select( - 'category', - [ 'cat_id', 'cat_title', 'cat_pages', 'cat_subcats', 'cat_files' ], - [ 'cat_title' => array_unique( $categoryDbKeys ) ], - __METHOD__ - ); - foreach ( $res as $row ) { - $categories[$row->cat_title] = Category::newFromRow( $row ); - } - } - $specialPage->categoryTreeCategories = $categories; + $this->categoryCache->doQuery( $categoryTargets ); } /** * Hook handler for the SpecialTrackingCategories::generateCatLink hook - * @suppress PhanUndeclaredProperty SpecialPage->categoryTreeCategories * @param SpecialPage $specialPage SpecialTrackingCategories object - * @param Title $catTitle Title object of the linked category + * @param LinkTarget $catTitle LinkTarget object of the linked category * @param string &$html Result html */ public function onSpecialTrackingCategories__generateCatLink( $specialPage, $catTitle, &$html ) { - if ( !isset( $specialPage->categoryTreeCategories ) ) { - return; - } - - $cat = $specialPage->categoryTreeCategories[$catTitle->getDBkey()] ?? null; + $cat = $this->categoryCache->getCategory( $catTitle ); $html .= CategoryTree::createCountString( $specialPage->getContext(), $cat, 0 ); } diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php new file mode 100644 index 00000000..da55b85b --- /dev/null +++ b/includes/ServiceWiring.php @@ -0,0 +1,40 @@ + static function ( MediaWikiServices $services ) { + return new CategoryCache( + $services->getDBLoadBalancer() + ); + }, +]; +// @codeCoverageIgnoreEnd diff --git a/tests/phpunit/CategoryCacheTest.php b/tests/phpunit/CategoryCacheTest.php new file mode 100644 index 00000000..f271f07b --- /dev/null +++ b/tests/phpunit/CategoryCacheTest.php @@ -0,0 +1,86 @@ +createMock( ILoadBalancer::class ) + ); + $this->addToAssertionCount( 1 ); + } + + public function testDoQuery() { + // Create a row in the category table + $this->editPage( + new TitleValue( NS_MAIN, 'CategoryTreeCategoryCacheTest' ), + '[[Category:Exists]]' + ); + + $categoryCache = TestingAccessWrapper::newFromObject( + $this->getServiceContainer()->get( 'CategoryTree.CategoryCache' ) + ); + + $categoryCache->doQuery( [ + new TitleValue( NS_CATEGORY, 'Exists' ), + new TitleValue( NS_CATEGORY, 'Missed' ), + ] ); + + $this->assertCount( 2, $categoryCache->cache ); + $this->assertArrayHasKey( 'Exists', $categoryCache->cache ); + $this->assertInstanceOf( Category::class, $categoryCache->cache['Exists'] ); + $this->assertArrayHasKey( 'Missed', $categoryCache->cache ); + $this->assertNull( $categoryCache->cache['Missed'] ); + } + + public function testGetCategory() { + // Create a row in the category table + $this->editPage( + new TitleValue( NS_MAIN, 'CategoryTreeCategoryCacheTest' ), + '[[Category:Exists]]' + ); + + $categoryCache = TestingAccessWrapper::newFromObject( + $this->getServiceContainer()->get( 'CategoryTree.CategoryCache' ) + ); + $title = new TitleValue( NS_CATEGORY, 'Exists' ); + + // Test for cache miss + $this->assertCount( 0, $categoryCache->cache ); + $this->assertInstanceOf( Category::class, $categoryCache->getCategory( $title ) ); + $this->assertCount( 1, $categoryCache->cache ); + + // Test normal access + $this->assertInstanceOf( Category::class, $categoryCache->getCategory( $title ) ); + } +} diff --git a/tests/phpunit/CategoryTreeServiceWiringTest.php b/tests/phpunit/CategoryTreeServiceWiringTest.php new file mode 100644 index 00000000..fdaf3aeb --- /dev/null +++ b/tests/phpunit/CategoryTreeServiceWiringTest.php @@ -0,0 +1,28 @@ +get( $name ); + $this->addToAssertionCount( 1 ); + } + + public function provideService() { + $wiring = require __DIR__ . '/../../includes/ServiceWiring.php'; + foreach ( $wiring as $name => $_ ) { + yield $name => [ $name ]; + } + } +}