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
