Skip to content

Commit

Permalink
PLATFORM-9062 | Replace use of PRIMARY_DB connection in CargoUtils, C…
Browse files Browse the repository at this point in the history
…argoAttach, CargoDeclare and CargoPageValues

Some of the actions are still using PRIMARY DB connection during the Read requests, and therefore we should replace them with use of the proper REPLICA DB. All of those cases are simple and shouldn't yield any errors when PRIMARY DB is not used to query the results.
Also, mark CargoRecreateTablesAPI as writing API to minimise noise from transaction profile.
  • Loading branch information
t-tomalak committed Feb 23, 2024
1 parent 0667936 commit e4fada1
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 19 deletions.
3 changes: 2 additions & 1 deletion includes/CargoRecreateDataAction.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?php

use MediaWiki\MediaWikiServices;
use MediaWiki\Permissions\PermissionManager;

/**
* Handles the 'recreatedata' action.
Expand Down Expand Up @@ -58,7 +59,7 @@ public static function displayTab( $obj, &$links ) {

$user = $obj->getUser();
$permissionManager = MediaWikiServices::getInstance()->getPermissionManager();
if ( !$permissionManager->userCan( 'recreatecargodata', $user, $title ) ) {
if ( !$permissionManager->userCan( 'recreatecargodata', $user, $title, PermissionManager::RIGOR_QUICK ) ) {
return true;
}

Expand Down
3 changes: 1 addition & 2 deletions includes/CargoUtils.php
Original file line number Diff line number Diff line change
Expand Up @@ -620,8 +620,7 @@ public static function tableFullyExists( $tableName ) {
return false;
}

$cdb = self::getDB();
return $cdb->tableExists( $tableName );
return CargoServices::getCargoConnectionProvider()->getConnection( DB_REPLICA )->tableExists( $tableName );
}

public static function fieldTypeToSQLType( $fieldType, $dbType, $size = null ) {
Expand Down
5 changes: 4 additions & 1 deletion includes/api/CargoRecreateTablesAPI.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ protected function getDescription() {

protected function getExamples() {
return [
'api.php?action=cargorecreatetables&template=City'
'api.php?action=cargorecreatetables&template=City',
];
}

Expand All @@ -76,4 +76,7 @@ public function needsToken() {
return 'csrf';
}

public function isWriteMode() {
return true;
}
}
9 changes: 7 additions & 2 deletions includes/parserfunctions/CargoAttach.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?php

use MediaWiki\MediaWikiServices;

/**
* Class for the #cargo_attach parser function.
*
Expand Down Expand Up @@ -41,8 +44,10 @@ public static function run( Parser $parser ) {
return CargoUtils::formatError( wfMessage( "cargo-notable" )->parse() );
}

$dbw = wfGetDB( DB_MASTER );
$res = $dbw->select( 'cargo_tables', 'COUNT(*) AS total', [ 'main_table' => $tableName ] );
$res = MediaWikiServices::getInstance()
->getDBLoadBalancer()
->getConnection( DB_REPLICA )
->select( 'cargo_tables', 'COUNT(*) AS total', [ 'main_table' => $tableName ] );
$row = $res->fetchRow();
if ( !empty( $row ) && $row['total'] == 0 ) {
return CargoUtils::formatError( "Error: The specified table, \"$tableName\", does not exist." );
Expand Down
4 changes: 1 addition & 3 deletions includes/parserfunctions/CargoDeclare.php
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,7 @@ public static function run( Parser $parser ) {
}

// Validate table name.

$cdb = CargoUtils::getDB();
$cdb = CargoServices::getCargoConnectionProvider()->getConnection( DB_REPLICA );

foreach ( $parentTables as $extraParams ) {
$parentTableName = $extraParams['Name'];
Expand Down Expand Up @@ -359,7 +358,6 @@ public static function run( Parser $parser ) {
// exists already - otherwise, explain that it needs to be
// created.
$text = wfMessage( 'cargo-definestable', $tableName )->text();
$cdb = CargoUtils::getDB();
if ( $cdb->tableExists( $tableName ) ) {
$ct = SpecialPage::getTitleFor( 'CargoTables' );
$pageName = $ct->getPrefixedText() . "/$tableName";
Expand Down
5 changes: 2 additions & 3 deletions includes/specials/CargoPageValues.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,6 @@ private function getInfoForAllFields( $tableName ) {
}

public function getRowsForPageInTable( $tableName ) {
$cdb = CargoUtils::getDB();

$sqlQuery = new CargoSQLQuery();
$sqlQuery->mAliasedTableNames = [ $tableName => $tableName ];

Expand Down Expand Up @@ -197,7 +195,8 @@ public function getRowsForPageInTable( $tableName ) {
$sqlQuery->mOrigAliasedFieldNames = $aliasedFieldNames;
$sqlQuery->setDescriptionsAndTableNamesForFields();
$sqlQuery->handleDateFields();
$sqlQuery->mWhereStr = $cdb->addIdentifierQuotes( '_pageID' ) . " = " .
$sqlQuery->mWhereStr = CargoServices::getCargoConnectionProvider()->getConnection( DB_REPLICA )
->addIdentifierQuotes( '_pageID' ) . " = " .
$this->mTitle->getArticleID();

$queryResults = $sqlQuery->run();
Expand Down
14 changes: 7 additions & 7 deletions includes/specials/SpecialCargoRecreateData.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public function execute( $query = null ) {
$out->addModules( 'ext.cargo.recreatedata' );

$templateData = [];
$dbw = wfGetDB( DB_MASTER );
$dbr = MediaWikiServices::getInstance()->getDBLoadBalancer()->getConnection( DB_REPLICA );
if ( $this->mTemplateTitle === null ) {
if ( $this->mTableName == '_pageData' ) {
$conds = null;
Expand All @@ -75,18 +75,18 @@ public function execute( $query = null ) {
} else { // if ( $this->mTableName == '_ganttData' ) {
$conds = 'page_namespace = ' . FD_NS_GANTT;
}
$numTotalPages = $dbw->selectField( 'page', 'COUNT(*)', $conds );
$numTotalPages = $dbr->selectField( 'page', 'COUNT(*)', $conds );
} else {
$numTotalPages = null;
$templateData[] = [
'name' => $this->mTemplateTitle->getText(),
'numPages' => $this->getNumPagesThatCallTemplate( $dbw, $this->mTemplateTitle )
'numPages' => $this->getNumPagesThatCallTemplate( $dbr, $this->mTemplateTitle )
];
}

if ( $this->mIsDeclared ) {
// Get all attached templates.
$res = $dbw->select( 'page_props',
$res = $dbr->select( 'page_props',
[
'pp_page'
],
Expand All @@ -98,7 +98,7 @@ public function execute( $query = null ) {
foreach ( $res as $row ) {
$templateID = $row->pp_page;
$attachedTemplateTitle = Title::newFromID( $templateID );
$numPages = $this->getNumPagesThatCallTemplate( $dbw, $attachedTemplateTitle );
$numPages = $this->getNumPagesThatCallTemplate( $dbr, $attachedTemplateTitle );
$attachedTemplateName = $attachedTemplateTitle->getText();
$templateData[] = [
'name' => $attachedTemplateName,
Expand Down Expand Up @@ -156,7 +156,7 @@ public function execute( $query = null ) {
return true;
}

public function getNumPagesThatCallTemplate( IDatabase $dbw, LinkTarget $templateTitle ) {
public function getNumPagesThatCallTemplate( IDatabase $dbr, LinkTarget $templateTitle ) {
$conds = [ "tl_from=page_id" ];

// MW 1.38+ - use the normalized link target ID for fetching incoming links to this template.
Expand All @@ -170,7 +170,7 @@ public function getNumPagesThatCallTemplate( IDatabase $dbw, LinkTarget $templat
$conds['tl_title'] = $templateTitle->getDBkey();
}

$res = $dbw->select(
$res = $dbr->select(
[ 'page', 'templatelinks' ],
'COUNT(*) AS total',
$conds,
Expand Down

0 comments on commit e4fada1

Please sign in to comment.