From d19a6fe542f6b7a334cac47c044d3d4319b6d0ae Mon Sep 17 00:00:00 2001 From: Agabi10 Date: Wed, 7 Nov 2018 16:15:06 +0000 Subject: [PATCH] Avoid hitting DB in Category getters when they have the required info Bug: T63045 Change-Id: Ib881ce263fa7a4a8256f627a56b774aaa815d3df --- RELEASE-NOTES-1.33 | 2 + includes/Category.php | 4 +- tests/phpunit/includes/CategoryTest.php | 287 ++++++++++++++++++++++++ 3 files changed, 291 insertions(+), 2 deletions(-) create mode 100644 tests/phpunit/includes/CategoryTest.php diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33 index c30c538ea2..5efd2a26eb 100644 --- a/RELEASE-NOTES-1.33 +++ b/RELEASE-NOTES-1.33 @@ -432,6 +432,8 @@ because of Phabricator reports. to page updates: LinksUpdateConstructed, LinksUpdate, LinksUpdateComplete. LinksUpdateAfterInsert is not triggered since deletions do not cause insertions into links tables. +* Category::newFromID( $id )->getID() will now return $id without any + validation, to avoid a mostly unnecessary DB query. == Compatibility == MediaWiki 1.33 requires PHP 7.0.13 or later. Although HHVM 3.18.5 or later is diff --git a/includes/Category.php b/includes/Category.php index 3352c2c2b0..ca16536c88 100644 --- a/includes/Category.php +++ b/includes/Category.php @@ -299,10 +299,10 @@ class Category { /** * Generic accessor * @param string $key - * @return bool + * @return mixed */ private function getX( $key ) { - if ( !$this->initialize( self::LAZY_INIT_ROW ) ) { + if ( $this->{$key} === null && !$this->initialize( self::LAZY_INIT_ROW ) ) { return false; } return $this->{$key}; diff --git a/tests/phpunit/includes/CategoryTest.php b/tests/phpunit/includes/CategoryTest.php new file mode 100644 index 0000000000..60fb144a44 --- /dev/null +++ b/tests/phpunit/includes/CategoryTest.php @@ -0,0 +1,287 @@ +setMwGlobals( [ + 'wgAllowUserJs' => false, + 'wgDefaultLanguageVariant' => false, + 'wgMetaNamespace' => 'Project', + ] ); + $this->setUserLang( 'en' ); + $this->setContentLang( 'en' ); + } + + /** + * @covers Category::initialize() + */ + public function testInitialize_idNotExist() { + $category = Category::newFromID( -1 ); + $this->assertFalse( $category->getName() ); + } + + public function provideInitializeVariants() { + return [ + // Existing title + [ 'newFromName', 'Example', 'getID', 1 ], + [ 'newFromName', 'Example', 'getName', 'Example' ], + [ 'newFromName', 'Example', 'getPageCount', 3 ], + [ 'newFromName', 'Example', 'getSubcatCount', 4 ], + [ 'newFromName', 'Example', 'getFileCount', 5 ], + + // Non-existing title + [ 'newFromName', 'NoExample', 'getID', 0 ], + [ 'newFromName', 'NoExample', 'getName', 'NoExample' ], + [ 'newFromName', 'NoExample', 'getPageCount', 0 ], + [ 'newFromName', 'NoExample', 'getSubcatCount', 0 ], + [ 'newFromName', 'NoExample', 'getFileCount', 0 ], + + // Existing ID + [ 'newFromID', 1, 'getID', 1 ], + [ 'newFromID', 1, 'getName', 'Example' ], + [ 'newFromID', 1, 'getPageCount', 3 ], + [ 'newFromID', 1, 'getSubcatCount', 4 ], + [ 'newFromID', 1, 'getFileCount', 5 ] + ]; + } + + /** + * @covers Category::initialize() + * @dataProvider provideInitializeVariants + */ + public function testInitialize( $createFunction, $createParam, $testFunction, $expected ) { + $dbw = wfGetDB( DB_MASTER ); + $dbw->insert( 'category', + [ + [ + 'cat_id' => 1, + 'cat_title' => 'Example', + 'cat_pages' => 3, + 'cat_subcats' => 4, + 'cat_files' => 5 + ] + ], + __METHOD__, + [ 'IGNORE' ] + ); + + $category = Category::{$createFunction}( $createParam ); + $this->assertEquals( $expected, $category->{$testFunction}() ); + + $dbw->delete( 'category', '*', __METHOD__ ); + } + + /** + * @covers Category::newFromName() + * @covers Category::getName() + */ + public function testNewFromName_validTitle() { + $category = Category::newFromName( 'Example' ); + $this->assertSame( 'Example', $category->getName() ); + } + + /** + * @covers Category::newFromName() + */ + public function testNewFromName_invalidTitle() { + $this->assertFalse( Category::newFromName( '#' ) ); + } + + /** + * @covers Category::newFromTitle() + */ + public function testNewFromTitle() { + $title = Title::newFromText( 'Category:Example' ); + $category = Category::newFromTitle( $title ); + $this->assertSame( 'Example', $category->getName() ); + } + + /** + * @covers Category::newFromID() + * @covers Category::getID() + */ + public function testNewFromID() { + $category = Category::newFromID( 5 ); + $this->assertSame( 5, $category->getID() ); + } + + /** + * @covers Category::newFromRow() + */ + public function testNewFromRow_found() { + $dbw = wfGetDB( DB_MASTER ); + $dbw->insert( 'category', + [ + [ + 'cat_id' => 1, + 'cat_title' => 'Example', + 'cat_pages' => 3, + 'cat_subcats' => 4, + 'cat_files' => 5 + ] + ], + __METHOD__, + [ 'IGNORE' ] + ); + + $category = Category::newFromRow( $dbw->selectRow( + 'category', + [ 'cat_id', 'cat_title', 'cat_pages', 'cat_subcats', 'cat_files' ], + [ 'cat_id' => 1 ], + __METHOD__ + ) ); + + $this->assertEquals( 1, $category->getID() ); + + $dbw->delete( 'category', '*', __METHOD__ ); + } + + /** + * @covers Category::newFromRow() + */ + public function testNewFromRow_notFoundWithoutTitle() { + $dbw = wfGetDB( DB_MASTER ); + $dbw->insert( 'category', + [ + [ + 'cat_id' => 1, + 'cat_title' => 'Example', + 'cat_pages' => 3, + 'cat_subcats' => 4, + 'cat_files' => 5 + ] + ], + __METHOD__, + [ 'IGNORE' ] + ); + + $row = $dbw->selectRow( + 'category', + [ 'cat_id', 'cat_title', 'cat_pages', 'cat_subcats', 'cat_files' ], + [ 'cat_id' => 1 ], + __METHOD__ + ); + $row->cat_title = null; + + $this->assertFalse( Category::newFromRow( $row ) ); + + $dbw->delete( 'category', '*', __METHOD__ ); + } + + /** + * @covers Category::newFromRow() + */ + public function testNewFromRow_notFoundWithTitle() { + $dbw = wfGetDB( DB_MASTER ); + $dbw->insert( 'category', + [ + [ + 'cat_id' => 1, + 'cat_title' => 'Example', + 'cat_pages' => 3, + 'cat_subcats' => 4, + 'cat_files' => 5 + ] + ], + __METHOD__, + [ 'IGNORE' ] + ); + + $row = $dbw->selectRow( + 'category', + [ 'cat_id', 'cat_title', 'cat_pages', 'cat_subcats', 'cat_files' ], + [ 'cat_id' => 1 ], + __METHOD__ + ); + $row->cat_title = null; + + $category = Category::newFromRow( + $row, + Title::newFromText( NS_CATEGORY, 'Example' ) + ); + + $this->assertFalse( $category->getID() ); + + $dbw->delete( 'category', '*', __METHOD__ ); + } + + /** + * @covers Category::getPageCount() + */ + public function testGetPageCount() { + $dbw = wfGetDB( DB_MASTER ); + $dbw->insert( 'category', + [ + [ + 'cat_id' => 1, + 'cat_title' => 'Example', + 'cat_pages' => 3, + 'cat_subcats' => 4, + 'cat_files' => 5 + ] + ], + __METHOD__, + [ 'IGNORE' ] + ); + + $category = Category::newFromID( 1 ); + $this->assertEquals( 3, $category->getPageCount() ); + + $dbw->delete( 'category', '*', __METHOD__ ); + } + + /** + * @covers Category::getSubcatCount() + */ + public function testGetSubcatCount() { + $dbw = wfGetDB( DB_MASTER ); + $dbw->insert( 'category', + [ + [ + 'cat_id' => 1, + 'cat_title' => 'Example', + 'cat_pages' => 3, + 'cat_subcats' => 4, + 'cat_files' => 5 + ] + ], + __METHOD__, + [ 'IGNORE' ] + ); + + $category = Category::newFromID( 1 ); + $this->assertEquals( 4, $category->getSubcatCount() ); + + $dbw->delete( 'category', '*', __METHOD__ ); + } + + /** + * @covers Category::getFileCount() + */ + public function testGetFileCount() { + $dbw = wfGetDB( DB_MASTER ); + $dbw->insert( 'category', + [ + [ + 'cat_id' => 1, + 'cat_title' => 'Example', + 'cat_pages' => 3, + 'cat_subcats' => 4, + 'cat_files' => 5 + ] + ], + __METHOD__, + [ 'IGNORE' ] + ); + + $category = Category::newFromID( 1 ); + $this->assertEquals( 5, $category->getFileCount() ); + + $dbw->delete( 'category', '*', __METHOD__ ); + } +} -- 2.20.1