Avoid hitting DB in Category getters when they have the required info

Bug: T63045
Change-Id: Ib881ce263fa7a4a8256f627a56b774aaa815d3df

diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33
index c30c538..5efd2a2 100644 (file)
--- 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 3352c2c..ca16536 100644 (file)
--- 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 (file)
index 0000000..60fb144
--- /dev/null
+++ b/tests/phpunit/includes/CategoryTest.php
@@ -0,0 +1,287 @@
+<?php
+
+/**
+ * @group Database
+ * @group Category
+ */
+class CategoryTest extends MediaWikiTestCase {
+       protected function setUp() {
+               parent::setUp();
+
+               $this->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__ );
+       }
+}