jenkins-bot has submitted this change and it was merged. Change subject: Hardcode category ids ......................................................................
Hardcode category ids Instead of having a complex auto-increment category manager that had additional caching, just hardcode the (currently 6) ids for each category, which allows us to simplify a lot of code. If Parsoid sends a lint error in a category that we don't know about, it is silently dropped. Bug: T151287 Change-Id: Ice6edf1b7985390aa0c1c410d357bc565bb69108 --- M includes/ApiQueryLintErrors.php M includes/ApiRecordLint.php M includes/CategoryManager.php M includes/Database.php M includes/Hooks.php M includes/LintErrorsPager.php M includes/SpecialLintErrors.php D lint_categories.sql M linter.sql 9 files changed, 49 insertions(+), 133 deletions(-) Approvals: Arlolra: Looks good to me, approved jenkins-bot: Verified diff --git a/includes/ApiQueryLintErrors.php b/includes/ApiQueryLintErrors.php index 89dfde4..795a614 100644 --- a/includes/ApiQueryLintErrors.php +++ b/includes/ApiQueryLintErrors.php @@ -33,7 +33,7 @@ public function execute() { $params = $this->extractRequestParams(); - $categoryMgr = CategoryManager::getInstance(); + $categoryMgr = new CategoryManager(); $this->addTables( 'linter' ); if ( $params['category'] !== null ) { @@ -41,7 +41,7 @@ } else { // Limit only to enabled categories (there might be others in the DB) $this->addWhereFld( 'linter_cat', array_values( $categoryMgr->getCategoryIds( - $categoryMgr->getCategories() + $categoryMgr->getVisibleCategories() ) ) ); } $db = $this->getDB(); @@ -96,7 +96,7 @@ public function getAllowedParams() { return [ 'category' => [ - ApiBase::PARAM_TYPE => CategoryManager::getInstance()->getCategories(), + ApiBase::PARAM_TYPE => ( new CategoryManager() )->getVisibleCategories(), ApiBase::PARAM_ISMULTI => true, ], 'limit' => [ diff --git a/includes/ApiRecordLint.php b/includes/ApiRecordLint.php index 99e7fff..c2255fe 100644 --- a/includes/ApiRecordLint.php +++ b/includes/ApiRecordLint.php @@ -57,7 +57,11 @@ ) { $this->dieUsage( 'Invalid, non-existent, or outdated title', 'invalid-title' ); } + $categoryMgr = new CategoryManager(); foreach ( $data as $info ) { + if ( !$categoryMgr->isKnownCategory( $info['type'] ) ) { + continue; + } $info['params']['location'] = array_slice( $info['dsr'], 0, 2 ); if ( isset( $info['templateInfo'] ) && $info['templateInfo'] ) { $info['params']['templateInfo'] = $info['templateInfo']; diff --git a/includes/CategoryManager.php b/includes/CategoryManager.php index 6045e01..97c30c9 100644 --- a/includes/CategoryManager.php +++ b/includes/CategoryManager.php @@ -20,73 +20,54 @@ namespace MediaWiki\Linter; -use MediaWiki\MediaWikiServices; - /** * Functions for lint error categories */ class CategoryManager { /** - * @var array|bool + * Map of category names to their hardcoded + * numerical ids for use in the database + * + * @var int[] */ - private $map; + private $categoryIds = [ + 'fostered' => 1, + 'obsolete-tag' => 2, + 'bogus-image-options' => 3, + 'missing-end-tag' => 4, + 'stripped-tag' => 5, + 'self-closed-tag' => 6, + ]; + /** - * @var \BagOStuff + * Categories that are configure to be displayed to users + * + * @return string[] */ - private $cache; - /** - * @var string - */ - private $cacheKey; - - private function __construct() { - $this->cache = MediaWikiServices::getInstance()->getLocalServerObjectCache(); - $this->cacheKey = $this->cache->makeKey( 'linter', 'categories' ); - } - - public static function getInstance() { - static $self; - if ( !$self ) { - $self = new self(); - } - - return $self; - } - - public function getCategories() { + public function getVisibleCategories() { global $wgLinterCategories; return array_keys( array_filter( $wgLinterCategories ) ); } /** - * @see getCategoryId + * Whether this category has a hardcoded id and can be + * inserted into the database + * * @param string $name - * @return bool|int + * @return bool */ - public function getAndMaybeCreateCategoryId( $name ) { - return $this->getCategoryId( $name, true ); + public function isKnownCategory( $name ) { + return isset( $this->categoryIds[$name] ); } /** - * @param string $id - * @throws \RuntimeException - * @return int + * @param int $id + * @throws \RuntimeException if we can't find the name for the id + * @return string */ public function getCategoryName( $id ) { - if ( !$this->map ) { - $this->loadMapFromCache(); - } - - if ( $this->map ) { - $flip = array_flip( $this->map ); - if ( isset( $flip[$id] ) ) { - return $flip[$id]; - } - } - - $this->loadMapFromDB(); - $flip = array_flip( $this->map ); + $flip = array_flip( $this->categoryIds ); if ( isset( $flip[$id] ) ) { return $flip[$id]; } @@ -94,6 +75,10 @@ throw new \RuntimeException( "Could not find name for id $id" ); } + /** + * @param string[] $names + * @return int[] + */ public function getCategoryIds( array $names ) { $result = []; foreach ( $names as $name ) { @@ -103,79 +88,18 @@ return $result; } - private function loadMapFromCache() { - $this->map = $this->cache->get( $this->cacheKey ); - } - - private function saveMapToCache() { - $this->cache->set( $this->cacheKey, $this->map ); - } - /** * Get the int id for the category in lint_categories table * * @param string $name - * @param bool $create Whether to create an id if missing - * @return int|bool + * @return int + * @throws \RuntimeException if we can't find the id for the name */ - public function getCategoryId( $name, $create = false ) { - // Check static cache - if ( !$this->map ) { - $this->loadMapFromCache(); + public function getCategoryId( $name ) { + if ( isset( $this->categoryIds[$name] ) ) { + return $this->categoryIds[$name]; } - if ( $this->map && isset( $this->map[$name] ) ) { - return $this->map[$name]; - } - - // Cache miss, hit the DB (replica) - $this->loadMapFromDB(); - if ( isset( $this->map[$name] ) ) { - $this->saveMapToCache(); - return $this->map[$name]; - } - - if ( !$create ) { - return false; - } - - // Not in DB, create a new ID - $dbw = wfGetDB( DB_MASTER ); - $dbw->insert( - 'lint_categories', - [ 'lc_name' => $name ], - __METHOD__, - [ 'IGNORE' ] - ); - if ( $dbw->affectedRows() ) { - $id = $dbw->insertId(); - } else { - // Raced out, get the inserted id - $id = $dbw->selectField( - 'lint_categories', - 'lc_id', - [ 'lc_name' => $name ], - __METHOD__, - [ 'LOCK IN SHARE MODE' ] - ); - } - - $this->map[$name] = (int)$id; - $this->saveMapToCache(); - return $this->map[$name]; + throw new \RuntimeException( "Cannot find id for '$name'" ); } - - private function loadMapFromDB() { - $rows = wfGetDB( DB_REPLICA )->select( - 'lint_categories', - [ 'lc_id', 'lc_name' ], - [], - __METHOD__ - ); - $this->map = []; - foreach ( $rows as $row ) { - $this->map[$row->lc_name] = (int)$row->lc_id; - } - } - } diff --git a/includes/Database.php b/includes/Database.php index 64fe85a..a9ee972 100644 --- a/includes/Database.php +++ b/includes/Database.php @@ -67,9 +67,8 @@ * @return LintError */ public static function makeLintError( $row ) { - $categoryMgr = CategoryManager::getInstance(); return new LintError( - $categoryMgr->getCategoryName( $row->linter_cat ), + ( new CategoryManager() )->getCategoryName( $row->linter_cat ), $row->linter_params, (int)$row->linter_id ); @@ -104,10 +103,9 @@ * @return array */ private function serializeError( LintError $error ) { - $categoryMgr = CategoryManager::getInstance(); return [ 'linter_page' => $this->pageId, - 'linter_cat' => $categoryMgr->getAndMaybeCreateCategoryId( $error->category ), + 'linter_cat' => ( new CategoryManager() )->getCategoryId( $error->category ), 'linter_params' => FormatJson::encode( $error->params, false, FormatJson::ALL_OK ), ]; } diff --git a/includes/Hooks.php b/includes/Hooks.php index dd7c303..5c94acb 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -30,7 +30,6 @@ public static function onLoadExtensionSchemaUpdates( DatabaseUpdater $updater ) { $dir = dirname( __DIR__ ); $updater->addExtensionTable( 'linter', "$dir/linter.sql" ); - $updater->addExtensionTable( 'lint_categories', "$dir/lint_categories.sql" ); } /** diff --git a/includes/LintErrorsPager.php b/includes/LintErrorsPager.php index a9992f0..4c0fd86 100644 --- a/includes/LintErrorsPager.php +++ b/includes/LintErrorsPager.php @@ -43,7 +43,7 @@ public function __construct( IContextSource $context, $category, LinkRenderer $linkRenderer ) { $this->category = $category; - $this->categoryId = CategoryManager::getInstance()->getCategoryId( $this->category ); + $this->categoryId = ( new CategoryManager() )->getCategoryId( $this->category ); $this->linkRenderer = $linkRenderer; parent::__construct( $context ); } diff --git a/includes/SpecialLintErrors.php b/includes/SpecialLintErrors.php index 6dd9324..16c289a 100644 --- a/includes/SpecialLintErrors.php +++ b/includes/SpecialLintErrors.php @@ -34,7 +34,7 @@ public function execute( $par ) { $this->setHeaders(); $this->outputHeader(); - $cats = CategoryManager::getInstance()->getCategories(); + $cats = ( new CategoryManager() )->getVisibleCategories(); if ( in_array( $par, $cats ) ) { $this->category = $par; } diff --git a/lint_categories.sql b/lint_categories.sql deleted file mode 100644 index 21b60ee..0000000 --- a/lint_categories.sql +++ /dev/null @@ -1,9 +0,0 @@ -CREATE TABLE /*_*/lint_categories ( - -- primary key - lc_id int UNSIGNED PRIMARY KEY not null AUTO_INCREMENT, - -- category name - lc_name VARCHAR(30) not null -) /*$wgDBTableOptions*/; - --- Query by name -CREATE UNIQUE INDEX /*i*/lc_name ON /*_*/lint_categories(lc_name); diff --git a/linter.sql b/linter.sql index aa0f158..1f2a962 100644 --- a/linter.sql +++ b/linter.sql @@ -3,7 +3,7 @@ linter_id int UNSIGNED PRIMARY KEY not null AUTO_INCREMENT, -- page id linter_page int UNSIGNED not null, - -- error category (lint_categories.lc_id) + -- error category (see CategoryManager::$categoryIds) linter_cat int UNSIGNED not null, -- extra parameters about the error, JSON encoded linter_params blob NOT NULL -- To view, visit https://gerrit.wikimedia.org/r/322840 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ice6edf1b7985390aa0c1c410d357bc565bb69108 Gerrit-PatchSet: 3 Gerrit-Project: mediawiki/extensions/Linter Gerrit-Branch: master Gerrit-Owner: Legoktm <lego...@member.fsf.org> Gerrit-Reviewer: Arlolra <abrea...@wikimedia.org> Gerrit-Reviewer: Brian Wolff <bawolff...@gmail.com> Gerrit-Reviewer: Legoktm <lego...@member.fsf.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits