Merge "Add caching to looking up totals"

extension.json

@@ -16,7 +16,8 @@
 		"MediaWiki\\Linter\\ApiQueryLintErrors": "includes/ApiQueryLintErrors.php",
 		"MediaWiki\\Linter\\RecordLintJob": "includes/RecordLintJob.php",
 		"MediaWiki\\Linter\\SpecialLintErrors": "includes/SpecialLintErrors.php",
-		"MediaWiki\\Linter\\LintErrorsPager": "includes/LintErrorsPager.php"
+		"MediaWiki\\Linter\\LintErrorsPager": "includes/LintErrorsPager.php",
+		"MediaWiki\\Linter\\TotalsLookup": "includes/TotalsLookup.php"
 	},
 	"MessagesDirs": {
 		"Linter": "i18n"

includes/Database.php

@@ -129,18 +129,35 @@ class Database {
 		];
 	}
 
+	/**
+	 * @param LintError[] $errors
+	 * @return array
+	 */
+	private function countByCat( array $errors ) {
+		$count = [];
+		foreach ( $errors as $error ) {
+			if ( !isset( $count[$error->category] ) ) {
+				$count[$error->category] = 1;
+			} else {
+				$count[$error->category] += 1;
+			}
+		}
+
+		return $count;
+	}
+
 	/**
 	 * Save the specified lint errors in the
 	 * database
 	 *
 	 * @param LintError[] $errors
-	 * @return array [ 'deleted' => int, 'added' => int ]
+	 * @return array [ 'deleted' => [ cat => count ], 'added' => [ cat => count ] ]
 	 */
 	public function setForPage( $errors ) {
 		$previous = $this->getForPage();
 		$dbw = wfGetDB( DB_MASTER );
 		if ( !$previous && !$errors ) {
-			return [ 'deleted' => 0, 'added' => 0 ];
+			return [ 'deleted' => [], 'added' => [] ];
 		} elseif ( !$previous && $errors ) {
 			$toInsert = array_values( $errors );
 			$toDelete = [];
@@ -150,7 +167,7 @@ class Database {
 				[ 'linter_page' => $this->pageId ],
 				__METHOD__
 			);
-			return [ 'deleted' => count( $previous ), 'added' => 0 ];
+			return [ 'deleted' => $this->countByCat( $previous ), 'added' => [] ];
 		} else {
 			$toInsert = [];
 			$toDelete = $previous;
@@ -191,8 +208,8 @@ class Database {
 		}
 
 		return [
-			'deleted' => count( $toDelete ),
-			'added' => count( $toInsert ),
+			'deleted' => $this->countByCat( $toDelete ),
+			'added' => $this->countByCat( $toInsert ),
 		];
 	}
 

includes/RecordLintJob.php

@@ -62,22 +62,46 @@ class RecordLintJob extends Job {
 			$toSet = array_merge( $toSet, $catErrors );
 		}
 
-		$lintDb->setForPage( $toSet );
-		$this->updateStats( $lintDb );
+		$changes = $lintDb->setForPage( $toSet );
+		$this->updateStats( $lintDb, $changes );
 
 		return true;
 	}
 
 	/**
-	 * Send stats to statsd
+	 * Send stats to statsd and update totals cache
 	 *
 	 * @param Database $lintDb
+	 * @param array $changes
 	 */
-	protected function updateStats( Database $lintDb ) {
+	protected function updateStats( Database $lintDb, array $changes ) {
 		global $wgLinterStatsdSampleFactor;
 
+		$mwServices = MediaWikiServices::getInstance();
+
+		$totalsLookup = new TotalsLookup(
+			new CategoryManager(),
+			$mwServices->getMainWANObjectCache()
+		);
+
 		if ( $wgLinterStatsdSampleFactor === false ) {
-			// Not enabled at all
+			// Don't send to statsd, but update cache with $changes
+			$raw = $changes['added'];
+			foreach ( $changes['deleted'] as $cat => $count ) {
+				if ( isset( $raw[$cat] ) ) {
+					$raw[$cat] -= $count;
+				} else {
+					// Negative value
+					$raw[$cat] = 0 - $count;
+				}
+			}
+
+			foreach ( $raw as $cat => $count ) {
+				if ( $count != 0 ) {
+					// There was a change in counts, invalidate the cache
+					$totalsLookup->touchCategoryCache( $cat );
+				}
+			}
 			return;
 		} elseif ( mt_rand( 1, $wgLinterStatsdSampleFactor ) != 1 ) {
 			return;
@@ -86,12 +110,14 @@ class RecordLintJob extends Job {
 		$totals = $lintDb->getTotals();
 		$wiki = wfWikiID();
 
-		$stats = MediaWikiServices::getInstance()->getStatsdDataFactory();
+		$stats = $mwServices->getStatsdDataFactory();
 		foreach ( $totals as $name => $count ) {
 			$stats->gauge( "linter.category.$name.$wiki", $count );
 		}
 
 		$stats->gauge( "linter.totals.$wiki", array_sum( $totals ) );
+
+		$totalsLookup->touchAllCategoriesCache();
 	}
 
 }

includes/SpecialLintErrors.php

@@ -22,6 +22,8 @@ namespace MediaWiki\Linter;
 
 use HTMLForm;
 use Html;
+use MediaWiki\MediaWikiServices;
+use ObjectCache;
 use SpecialPage;
 
 class SpecialLintErrors extends SpecialPage {
@@ -94,7 +96,11 @@ class SpecialLintErrors extends SpecialPage {
 	}
 
 	private function showCategoryListings( CategoryManager $catManager ) {
-		$totals = ( new Database( 0 ) )->getTotals();
+		$lookup = new TotalsLookup(
+			$catManager,
+			MediaWikiServices::getInstance()->getMainWANObjectCache()
+		);
+		$totals = $lookup->getTotals();
 
 		// Display lint issues by priority
 		$this->displayList( 'high', $totals, $catManager->getHighPriority() );

includes/TotalsLookup.php

@@ -0,0 +1,104 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki\Linter;
+
+use WANObjectCache;
+use Wikimedia\Rdbms\Database as MWDatabase;
+
+/**
+ * Lookup to find and cache the total amount of
+ * lint errors in each category
+ */
+class TotalsLookup {
+
+	/**
+	 * @var WANObjectCache
+	 */
+	private $cache;
+
+	/**
+	 * @var CategoryManager
+	 */
+	private $catManager;
+
+	public function __construct( CategoryManager $catManager, WANObjectCache $cache ) {
+		$this->cache = $cache;
+		$this->catManager = $catManager;
+	}
+
+	/**
+	 * @param string $cat
+	 * @return string
+	 */
+	private function makeKey( $cat ) {
+		return $this->cache->makeKey( 'linter', 'total', $cat );
+	}
+
+	/**
+	 * Get the totals for every category in the database
+	 *
+	 * @return array
+	 */
+	public function getTotals() {
+		$cats = $this->catManager->getVisibleCategories();
+		$fetchedTotals = false;
+		$totals = [];
+		foreach ( $cats as $cat ) {
+			$totals[$cat] = $this->cache->getWithSetCallback(
+				$this->makeKey( $cat ),
+				WANObjectCache::TTL_INDEFINITE,
+				function( $oldValue, &$ttl, &$setOpts, $oldAsOf ) use ( $cat, &$fetchedTotals ) {
+					$setOpts += MWDatabase::getCacheSetOptions( wfGetDB( DB_REPLICA ) );
+					if ( $fetchedTotals === false ) {
+						$fetchedTotals = ( new Database( 0 ) )->getTotals();
+					}
+
+					return $fetchedTotals[$cat];
+				},
+				[
+					'checkKeys' => [
+						$this->cache->makeKey( 'linter', 'totals' ),
+						$this->makeKey( $cat ),
+					],
+					'lockTSE' => 30,
+				]
+			);
+		}
+
+		return $totals;
+	}
+
+	/**
+	 * Have a single category be recalculated
+	 *
+	 * @param string $cat category name
+	 */
+	public function touchCategoryCache( $cat ) {
+		$this->cache->touchCheckKey( $this->makeKey( $cat ) );
+	}
+
+	/**
+	 * Have all categories be recalculated
+	 */
+	public function touchAllCategoriesCache() {
+		$this->cache->touchCheckKey( $this->cache->makeKey( 'linter', 'totals' ) );
+	}
+}

tests/phpunit/DatabaseTest.php

@@ -66,22 +66,22 @@ class DatabaseTest extends MediaWikiTestCase {
 		$lintDb = new Database( 5 );
 		$dummyErrors = $this->getDummyLintErrors();
 		$result = $lintDb->setForPage( $dummyErrors );
-		$this->assertSetForPageResult( $result, 0, 2 );
+		$this->assertSetForPageResult( $result, [], [ 'fostered' => 1, 'obsolete-tag' => 1 ] );
 		$this->assertLintErrorsEqual( $dummyErrors, $lintDb->getForPage() );
 
 		// Should delete the second error
 		$result2 = $lintDb->setForPage( [ $dummyErrors[0] ] );
-		$this->assertSetForPageResult( $result2, 1, 0 );
+		$this->assertSetForPageResult( $result2, [ 'obsolete-tag' => 1 ], [] );
 		$this->assertLintErrorsEqual( [ $dummyErrors[0] ], $lintDb->getForPage() );
 
 		// Insert the second error, delete the first
 		$result3 = $lintDb->setForPage( [ $dummyErrors[1] ] );
-		$this->assertSetForPageResult( $result3, 1, 1 );
+		$this->assertSetForPageResult( $result3, [ 'fostered' => 1 ], [ 'obsolete-tag' => 1 ] );
 		$this->assertLintErrorsEqual( [ $dummyErrors[1] ], $lintDb->getForPage() );
 
 		// Delete the second (only) error
 		$result4 = $lintDb->setForPage( [] );
-		$this->assertSetForPageResult( $result4, 1, 0 );
+		$this->assertSetForPageResult( $result4, [ 'obsolete-tag' => 1 ], [] );
 		$this->assertLintErrorsEqual( [], $lintDb->getForPage() );
 	}