Daniel Kinzler has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/402903 )
Change subject: Specify encoding expected by DatabaseDomain::newFromId.
......................................................................
Specify encoding expected by DatabaseDomain::newFromId.
Alternative to I45a151a332
Bug: T183914
Change-Id: I647e7bd6dcb3f63c912c8ebb0fd187ff4fd37b45
---
M includes/Storage/RevisionStore.php
M includes/libs/rdbms/database/DatabaseDomain.php
M includes/libs/rdbms/loadbalancer/ILoadBalancer.php
M tests/phpunit/includes/Storage/RevisionStoreDbTest.php
4 files changed, 57 insertions(+), 35 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/03/402903/1
diff --git a/includes/Storage/RevisionStore.php
b/includes/Storage/RevisionStore.php
index 3101aea..484d1aa 100644
--- a/includes/Storage/RevisionStore.php
+++ b/includes/Storage/RevisionStore.php
@@ -74,6 +74,11 @@
private $wikiId;
/**
+ * @var null|string
+ */
+ private $dbDomainId;
+
+ /**
* @var boolean
*/
private $contentHandlerUseDB = true;
@@ -94,7 +99,7 @@
* @param LoadBalancer $loadBalancer
* @param SqlBlobStore $blobStore
* @param WANObjectCache $cache
- * @param bool|string $wikiId
+ * @param bool|string $wikiId A wiki database domain, as expected by
LoadBalancer::getConnection.
*/
public function __construct(
LoadBalancer $loadBalancer,
@@ -268,7 +273,7 @@
*/
public function insertRevisionOn( RevisionRecord $rev, IDatabase $dbw )
{
// TODO: pass in a DBTransactionContext instead of a database
connection.
- $this->checkDatabaseWikiId( $dbw );
+ $this->checkDatabaseDomain( $dbw );
if ( !$rev->getSlotRoles() ) {
throw new InvalidArgumentException( 'At least one slot
needs to be defined!' );
@@ -507,7 +512,7 @@
$minor,
User $user
) {
- $this->checkDatabaseWikiId( $dbw );
+ $this->checkDatabaseDomain( $dbw );
$fields = [ 'page_latest', 'page_namespace', 'page_title',
'rev_id', 'rev_text_id', 'rev_len', 'rev_sha1' ];
@@ -1462,31 +1467,27 @@
}
/**
+ * Determines the database domain ID this RevisionStore is bound to.
+ *
+ * @return string
+ */
+ private function getDomainID() {
+ if ( !$this->dbDomainId ) {
+ $this->dbDomainId = $this->getDBConnectionRef(
DB_REPLICA )->getDomainID();
+ }
+ return $this->dbDomainId;
+ }
+
+ /**
* Throws an exception if the given database connection does not belong
to the wiki this
* RevisionStore is bound to.
*
* @param IDatabase $db
* @throws MWException
*/
- private function checkDatabaseWikiId( IDatabase $db ) {
- $storeWiki = $this->wikiId;
+ private function checkDatabaseDomain( IDatabase $db ) {
+ $storeWiki = $this->getDomainID();
$dbWiki = $db->getDomainID();
-
- if ( $dbWiki === $storeWiki ) {
- return;
- }
-
- // XXX: we really want the default database ID...
- $storeWiki = $storeWiki ?: wfWikiID();
- $dbWiki = $dbWiki ?: wfWikiID();
-
- if ( $dbWiki === $storeWiki ) {
- return;
- }
-
- // HACK: counteract encoding imposed by DatabaseDomain
- $storeWiki = str_replace( '?h', '-', $storeWiki );
- $dbWiki = str_replace( '?h', '-', $dbWiki );
if ( $dbWiki === $storeWiki ) {
return;
@@ -1509,7 +1510,7 @@
* @return object|false data row as a raw object
*/
private function fetchRevisionRowFromConds( IDatabase $db, $conditions,
$flags = 0 ) {
- $this->checkDatabaseWikiId( $db );
+ $this->checkDatabaseDomain( $db );
$revQuery = self::getQueryInfo( [ 'page', 'user' ] );
$options = [];
@@ -1680,7 +1681,7 @@
* of the corresponding revision.
*/
public function listRevisionSizes( IDatabase $db, array $revIds ) {
- $this->checkDatabaseWikiId( $db );
+ $this->checkDatabaseDomain( $db );
$revLens = [];
if ( !$revIds ) {
@@ -1749,7 +1750,7 @@
* @return int
*/
private function getPreviousRevisionId( IDatabase $db, RevisionRecord
$rev ) {
- $this->checkDatabaseWikiId( $db );
+ $this->checkDatabaseDomain( $db );
if ( $rev->getPageId() === null ) {
return 0;
@@ -1805,7 +1806,7 @@
* @return int
*/
public function countRevisionsByPageId( IDatabase $db, $id ) {
- $this->checkDatabaseWikiId( $db );
+ $this->checkDatabaseDomain( $db );
$row = $db->selectRow( 'revision',
[ 'revCount' => 'COUNT(*)' ],
@@ -1854,7 +1855,7 @@
* @return bool True if the given user was the only one to edit since
the given timestamp
*/
public function userWasLastToEdit( IDatabase $db, $pageId, $userId,
$since ) {
- $this->checkDatabaseWikiId( $db );
+ $this->checkDatabaseDomain( $db );
if ( !$userId ) {
return false;
diff --git a/includes/libs/rdbms/database/DatabaseDomain.php
b/includes/libs/rdbms/database/DatabaseDomain.php
index ef6600b..b84f4b2 100644
--- a/includes/libs/rdbms/database/DatabaseDomain.php
+++ b/includes/libs/rdbms/database/DatabaseDomain.php
@@ -57,7 +57,18 @@
}
/**
- * @param DatabaseDomain|string $domain Result of
DatabaseDomain::toString()
+ * Constructs a DatabaseDomain by decoding a symbolic domain name.
+ * The database domain can be given as one, two, or three parts,
separated by a dash:
+ * <dbname> or <dbname>-<prefix> or <dbname>-<schema>-<prefix>.
+ * In each part, the dash itself must be encoded as '?h' and the
question mark as '??'.
+ *
+ * @todo clarify the relationship between this and getWikiId().
+ * @todo clarify the relationship between this and Site::getGlobalId().
+ * @todo clarify the relationship between this and WikiMap::getWiki(
$wikiID ).
+ *
+ * This method is round-trip-compatible with DatabaseDomain::toString().
+ *
+ * @param DatabaseDomain|string $domain
* @return DatabaseDomain
*/
public static function newFromId( $domain ) {
diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
index e246b79..eb9c017 100644
--- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
@@ -176,7 +176,8 @@
*
* @param int $i Server index or DB_MASTER/DB_REPLICA
* @param array|string|bool $groups Query group(s), or false for the
generic reader
- * @param string|bool $domain Domain ID, or false for the current domain
+ * @param string|bool $domain Domain ID, or false for the local domain.
+ * See DatabaseDomain::newFromId for details.
* @param int $flags Bitfield of CONN_* class constants
*
* @throws DBError
@@ -206,7 +207,8 @@
*
* @param int $i Server index or DB_MASTER/DB_REPLICA
* @param array|string|bool $groups Query group(s), or false for the
generic reader
- * @param string|bool $domain Domain ID, or false for the current domain
+ * @param string|bool $domain Domain ID, or false for the local domain.
+ * See DatabaseDomain::newFromId for details.
* @param int $flags Bitfield of CONN_* class constants (e.g.
CONN_TRX_AUTO)
* @return DBConnRef
*/
@@ -223,7 +225,8 @@
*
* @param int $i Server index or DB_MASTER/DB_REPLICA
* @param array|string|bool $groups Query group(s), or false for the
generic reader
- * @param string|bool $domain Domain ID, or false for the current domain
+ * @param string|bool $domain Domain ID, or false for the local domain.
+ * See DatabaseDomain::newFromId for details.
* @param int $flags Bitfield of CONN_* class constants (e.g.
CONN_TRX_AUTO)
* @return DBConnRef
*/
@@ -240,7 +243,8 @@
*
* @param int $db Server index or DB_MASTER/DB_REPLICA
* @param array|string|bool $groups Query group(s), or false for the
generic reader
- * @param string|bool $domain Domain ID, or false for the current domain
+ * @param string|bool $domain Domain ID, or false for the local domain.
+ * See DatabaseDomain::newFromId for details.
* @param int $flags Bitfield of CONN_* class constants (e.g.
CONN_TRX_AUTO)
* @return MaintainableDBConnRef
*/
@@ -257,7 +261,8 @@
* @note If disable() was called on this LoadBalancer, this method will
throw a DBAccessError.
*
* @param int $i Server index (does not support DB_MASTER/DB_REPLICA)
- * @param string|bool $domain Domain ID, or false for the current domain
+ * @param string|bool $domain Domain ID, or false for the local domain.
+ * See DatabaseDomain::newFromId for details.
* @param int $flags Bitfield of CONN_* class constants (e.g.
CONN_TRX_AUTO)
* @return Database|bool Returns false on errors
* @throws DBAccessError
diff --git a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
index 15f173a..79edf43 100644
--- a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
+++ b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
@@ -43,7 +43,12 @@
$lb->method( 'reallyOpenConnection' )->willReturnCallback(
function ( array $server, $dbNameOverride = false ) {
- return $this->getDatabaseMock( $server );
+ // match behavior of
LoadBalancer::reallyOpenConnection
+ if ( $dbNameOverride !== false ) {
+ $server['dbname'] = $dbNameOverride;
+ }
+
+ return $this->getDatabaseMock( $server,
$dbNameOverride );
}
);
@@ -73,7 +78,7 @@
yield [ 'test-foo_', 'test', 'foo_' ];
yield [ false, 'dash-test', '' ];
- yield [ 'dash-test', 'dash-test', '' ];
+ yield [ 'dash?htest', 'dash-test', '' ];
yield [ false, 'underscore_test', 'foo_' ];
yield [ 'underscore_test-foo_', 'underscore_test', 'foo_' ];
@@ -81,7 +86,7 @@
/**
* @dataProvider provideDomainCheck
- * @covers \MediaWiki\Storage\RevisionStore::checkDatabaseWikiId
+ * @covers \MediaWiki\Storage\RevisionStore::checkDatabaseDomain
*/
public function testDomainCheck( $wikiId, $dbName, $dbPrefix ) {
$this->setMwGlobals(
--
To view, visit https://gerrit.wikimedia.org/r/402903
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I647e7bd6dcb3f63c912c8ebb0fd187ff4fd37b45
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <daniel.kinz...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
