Aaron Schulz has uploaded a new change for review.
https://gerrit.wikimedia.org/r/195033
Change subject: [WIP] Clean up $wgSQLiteDataDir handling and removed standalone
sqlite class
......................................................................
[WIP] Clean up $wgSQLiteDataDir handling and removed standalone sqlite class
* The data directory can now be set via the construction params
* A standalone factory method now replaces the subclass
* Also made mDatabaseFile protected
Change-Id: I1791fd4f630e5c121fa7f68f473411a7c12d0c97
---
M autoload.php
M includes/db/DatabaseSqlite.php
M includes/installer/SqliteInstaller.php
M maintenance/sqlite.php
M maintenance/update.php
M tests/phpunit/includes/db/DatabaseSqliteTest.php
6 files changed, 83 insertions(+), 63 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/33/195033/1
diff --git a/autoload.php b/autoload.php
index faf8252..835ed13 100644
--- a/autoload.php
+++ b/autoload.php
@@ -289,7 +289,6 @@
'DatabaseOracle' => __DIR__ . '/includes/db/DatabaseOracle.php',
'DatabasePostgres' => __DIR__ . '/includes/db/DatabasePostgres.php',
'DatabaseSqlite' => __DIR__ . '/includes/db/DatabaseSqlite.php',
- 'DatabaseSqliteStandalone' => __DIR__ .
'/includes/db/DatabaseSqlite.php',
'DatabaseUpdater' => __DIR__ .
'/includes/installer/DatabaseUpdater.php',
'DateFormats' => __DIR__ . '/maintenance/language/date-formats.php',
'DateFormatter' => __DIR__ . '/includes/parser/DateFormatter.php',
diff --git a/includes/db/DatabaseSqlite.php b/includes/db/DatabaseSqlite.php
index 0b51972..ed86bab 100644
--- a/includes/db/DatabaseSqlite.php
+++ b/includes/db/DatabaseSqlite.php
@@ -29,8 +29,11 @@
/** @var bool Whether full text is enabled */
private static $fulltextEnabled = null;
+ /** @var string Directory */
+ protected $dbDir;
+
/** @var string File name for SQLite database file */
- public $mDatabaseFile;
+ protected $dbPath;
/** @var string Transaction mode */
protected $trxMode;
@@ -49,30 +52,61 @@
/**
* Additional params include:
- * - trxMode : one of (deferred, immediate, exclusive)
+ * - dbDirectory : directory containing the DB and the lock file
directory
+ * [defaults to $wgSQLiteDataDir]
+ * - dbFilePath : use this to force the path of the DB file
+ * - trxMode : one of (deferred, immediate, exclusive)
* @param array $p
*/
function __construct( array $p ) {
global $wgSharedDB, $wgSQLiteDataDir;
- $this->mDBname = $p['dbname'];
- parent::__construct( $p );
- // parent doesn't open when $user is false, but we can work
with $dbName
- if ( $p['dbname'] && !$this->isOpen() ) {
- if ( $this->open( $p['host'], $p['user'],
$p['password'], $p['dbname'] ) ) {
- if ( $wgSharedDB ) {
- $this->attachDatabase( $wgSharedDB );
+ $this->dbDir = isset( $p['dbDirectory'] ) ? $p['dbDirectory'] :
$wgSQLiteDataDir;
+
+ if ( isset( $p['dbFilePath'] ) ) {
+ $this->mFlags = isset( $p['flags'] ) ? $p['flags'] : 0;
+ // Standalone .sqlite file mode
+ $this->openFile( $p['dbFilePath'] );
+ // @FIXME: clean up base constructor so this can call
super instead
+ $this->mTrxAtomicLevels = new SplStack;
+ } else {
+ $this->mDBname = $p['dbname'];
+ // Stock wiki mode using standard file names per DB
+ parent::__construct( $p );
+ // parent doesn't open when $user is false, but we can
work with $dbName
+ if ( $p['dbname'] && !$this->isOpen() ) {
+ if ( $this->open( $p['host'], $p['user'],
$p['password'], $p['dbname'] ) ) {
+ if ( $wgSharedDB ) {
+ $this->attachDatabase(
$wgSharedDB );
+ }
}
}
}
$this->trxMode = isset( $p['trxMode'] ) ? strtoupper(
$p['trxMode'] ) : null;
- if ( $this->trxMode && !in_array( $this->trxMode, array(
'IMMEDIATE', 'EXCLUSIVE' ) ) ) {
+ if ( $this->trxMode &&
+ !in_array( $this->trxMode, array( 'DEFERRED',
'IMMEDIATE', 'EXCLUSIVE' ) )
+ ) {
$this->trxMode = null;
wfWarn( "Invalid SQLite transaction mode provided." );
}
- $this->lockMgr = new FSLockManager( array( 'lockDirectory' =>
"$wgSQLiteDataDir/locks" ) );
+ $this->lockMgr = new FSLockManager( array( 'lockDirectory' =>
"{$this->dbDir}/locks" ) );
+ }
+
+ /**
+ * @param string $filename
+ * @param array $p Options map; supports:
+ * - flags : (same as __construct counterpart)
+ * - trxMode : (same as __construct counterpart)
+ * - dbDirectory : (same as __construct counterpart)
+ * @return DatabaseSqlite
+ * @since 1.25
+ */
+ public static function newStandaloneInstance( $filename, array $p =
array() ) {
+ $p['dbFilePath'] = $filename;
+
+ return new self( $p );
}
/**
@@ -103,10 +137,8 @@
* @return PDO
*/
function open( $server, $user, $pass, $dbName ) {
- global $wgSQLiteDataDir;
-
$this->close();
- $fileName = self::generateFileName( $wgSQLiteDataDir, $dbName );
+ $fileName = self::generateFileName( $this->dbDir, $dbName );
if ( !is_readable( $fileName ) ) {
$this->mConn = false;
throw new DBConnectionError( $this, "SQLite database
not accessible" );
@@ -123,10 +155,10 @@
* @throws DBConnectionError
* @return PDO|bool SQL connection or false if failed
*/
- function openFile( $fileName ) {
+ protected function openFile( $fileName ) {
$err = false;
- $this->mDatabaseFile = $fileName;
+ $this->dbPath = $fileName;
try {
if ( $this->mFlags & DBO_PERSISTENT ) {
$this->mConn = new PDO( "sqlite:$fileName", '',
'',
@@ -154,6 +186,14 @@
}
return false;
+ }
+
+ /**
+ * @return string SQLite DB file path
+ * @since 1.25
+ */
+ public function getDbFilePath() {
+ return $this->dbPath;
}
/**
@@ -206,8 +246,7 @@
$cachedResult = false;
$table = 'dummy_search_test';
- $db = new DatabaseSqliteStandalone( ':memory:' );
-
+ $db = self::newStandaloneInstance( ':memory:' );
if ( $db->query( "CREATE VIRTUAL TABLE $table USING
FTS3(dummy_field)", __METHOD__, true ) ) {
$cachedResult = 'FTS3';
}
@@ -223,14 +262,13 @@
* @param string $name Database name to be used in queries like
* SELECT foo FROM dbname.table
* @param bool|string $file Database file name. If omitted, will be
generated
- * using $name and $wgSQLiteDataDir
+ * using $name and configured data directory
* @param string $fname Calling function name
* @return ResultWrapper
*/
function attachDatabase( $name, $file = false, $fname = __METHOD__ ) {
- global $wgSQLiteDataDir;
if ( !$file ) {
- $file = self::generateFileName( $wgSQLiteDataDir, $name
);
+ $file = self::generateFileName( $this->dbDir, $name );
}
$file = $this->addQuotes( $file );
@@ -863,11 +901,9 @@
}
public function lock( $lockName, $method, $timeout = 5 ) {
- global $wgSQLiteDataDir;
-
- if ( !is_dir( "$wgSQLiteDataDir/locks" ) ) { // create dir as
needed
- if ( !is_writable( $wgSQLiteDataDir ) || !mkdir(
"$wgSQLiteDataDir/locks" ) ) {
- throw new DBError( "Cannot create directory
\"$wgSQLiteDataDir/locks\"." );
+ if ( !is_dir( "{$this->dbDir}/locks" ) ) { // create dir as
needed
+ if ( !is_writable( $this->dbDir ) || !mkdir(
"{$this->dbDir}/locks" ) ) {
+ throw new DBError( "Cannot create directory
\"{$this->dbDir}/locks\"." );
}
}
@@ -960,23 +996,6 @@
return $endArray;
}
} // end DatabaseSqlite class
-
-/**
- * This class allows simple acccess to a SQLite database independently from
main database settings
- * @ingroup Database
- */
-class DatabaseSqliteStandalone extends DatabaseSqlite {
- public function __construct( $fileName, $flags = 0 ) {
- global $wgSQLiteDataDir;
-
- $this->mTrxAtomicLevels = new SplStack;
- $this->lockMgr = new FSLockManager( array( 'lockDirectory' =>
"$wgSQLiteDataDir/locks" ) );
-
- $this->mFlags = $flags;
- $this->tablePrefix( null );
- $this->openFile( $fileName );
- }
-}
/**
* @ingroup Database
diff --git a/includes/installer/SqliteInstaller.php
b/includes/installer/SqliteInstaller.php
index 1e7e969..f990ddf 100644
--- a/includes/installer/SqliteInstaller.php
+++ b/includes/installer/SqliteInstaller.php
@@ -55,7 +55,7 @@
public function checkPrerequisites() {
$result = Status::newGood();
// Bail out if SQLite is too old
- $db = new DatabaseSqliteStandalone( ':memory:' );
+ $db = DatabaseSqlite::newStandaloneInstance( ':memory:' );
if ( version_compare( $db->getServerVersion(),
self::MINIMUM_VERSION, '<' ) ) {
$result->fatal( 'config-outdated-sqlite',
$db->getServerVersion(), self::MINIMUM_VERSION );
}
diff --git a/maintenance/sqlite.php b/maintenance/sqlite.php
index edc9e14..7e02a4b 100644
--- a/maintenance/sqlite.php
+++ b/maintenance/sqlite.php
@@ -81,7 +81,7 @@
}
private function vacuum() {
- $prevSize = filesize( $this->db->mDatabaseFile );
+ $prevSize = filesize( $this->db->getDbFilePath() );
if ( $prevSize == 0 ) {
$this->error( "Can't vacuum an empty database.\n", true
);
}
@@ -89,7 +89,7 @@
$this->output( 'VACUUM: ' );
if ( $this->db->query( 'VACUUM' ) ) {
clearstatcache();
- $newSize = filesize( $this->db->mDatabaseFile );
+ $newSize = filesize( $this->db->getDbFilePath() );
$this->output( sprintf( "Database size was %d, now %d
(%.1f%% reduction).\n",
$prevSize, $newSize, ( $prevSize - $newSize ) *
100.0 / $prevSize ) );
} else {
@@ -115,7 +115,7 @@
private function backup( $fileName ) {
$this->output( "Backing up database:\n Locking..." );
$this->db->query( 'BEGIN IMMEDIATE TRANSACTION', __METHOD__ );
- $ourFile = $this->db->mDatabaseFile;
+ $ourFile = $this->db->getDbFilePath();
$this->output( " Copying database file $ourFile to
$fileName... " );
wfSuppressWarnings( false );
if ( !copy( $ourFile, $fileName ) ) {
diff --git a/maintenance/update.php b/maintenance/update.php
index 182a2c4..6e93011 100755
--- a/maintenance/update.php
+++ b/maintenance/update.php
@@ -151,7 +151,7 @@
$this->output( "Going to run database updates for " .
wfWikiID() . "\n" );
if ( $db->getType() === 'sqlite' ) {
- $this->output( "Using SQLite file:
'{$db->mDatabaseFile}'\n" );
+ $this->output( "Using SQLite file:
'{$db->getDbFilePath()}'\n" );
}
$this->output( "Depending on the size of your database this may
take a while!\n" );
diff --git a/tests/phpunit/includes/db/DatabaseSqliteTest.php
b/tests/phpunit/includes/db/DatabaseSqliteTest.php
index d32c1a6..645baf1 100644
--- a/tests/phpunit/includes/db/DatabaseSqliteTest.php
+++ b/tests/phpunit/includes/db/DatabaseSqliteTest.php
@@ -1,10 +1,12 @@
<?php
-class MockDatabaseSqlite extends DatabaseSqliteStandalone {
+class MockDatabaseSqlite extends DatabaseSqlite {
private $lastQuery;
- function __construct() {
- parent::__construct( ':memory:' );
+ public static function newInstance( array $p = array() ) {
+ $p['dbFilePath'] = ':memory:';
+
+ return new self( $p );
}
function query( $sql, $fname = '', $tempIgnore = false ) {
@@ -36,7 +38,7 @@
if ( !Sqlite::isPresent() ) {
$this->markTestSkipped( 'No SQLite support detected' );
}
- $this->db = new MockDatabaseSqlite();
+ $this->db = MockDatabaseSqlite::newInstance();
if ( version_compare( $this->db->getServerVersion(), '3.6.0',
'<' ) ) {
$this->markTestSkipped( "SQLite at least 3.6 required,
{$this->db->getServerVersion()} found" );
}
@@ -89,7 +91,7 @@
*/
public function testAddQuotes( $value, $expected ) {
// check quoting
- $db = new DatabaseSqliteStandalone( ':memory:' );
+ $db = DatabaseSqlite::newStandaloneInstance( ':memory:' );
$this->assertEquals( $expected, $db->addQuotes( $value ),
'string not quoted as expected' );
// ok, quoting works as expected, now try a round trip.
@@ -172,7 +174,7 @@
*/
public function testTableName() {
// @todo Moar!
- $db = new DatabaseSqliteStandalone( ':memory:' );
+ $db = DatabaseSqlite::newStandaloneInstance( ':memory:' );
$this->assertEquals( 'foo', $db->tableName( 'foo' ) );
$this->assertEquals( 'sqlite_master', $db->tableName(
'sqlite_master' ) );
$db->tablePrefix( 'foo' );
@@ -184,7 +186,7 @@
* @covers DatabaseSqlite::duplicateTableStructure
*/
public function testDuplicateTableStructure() {
- $db = new DatabaseSqliteStandalone( ':memory:' );
+ $db = DatabaseSqlite::newStandaloneInstance( ':memory:' );
$db->query( 'CREATE TABLE foo(foo, barfoo)' );
$db->duplicateTableStructure( 'foo', 'bar' );
@@ -208,7 +210,7 @@
* @covers DatabaseSqlite::duplicateTableStructure
*/
public function testDuplicateTableStructureVirtual() {
- $db = new DatabaseSqliteStandalone( ':memory:' );
+ $db = DatabaseSqlite::newStandaloneInstance( ':memory:' );
if ( $db->getFulltextSearchModule() != 'FTS3' ) {
$this->markTestSkipped( 'FTS3 not supported, cannot
create virtual tables' );
}
@@ -231,7 +233,7 @@
* @covers DatabaseSqlite::deleteJoin
*/
public function testDeleteJoin() {
- $db = new DatabaseSqliteStandalone( ':memory:' );
+ $db = DatabaseSqlite::newStandaloneInstance( ':memory:' );
$db->query( 'CREATE TABLE a (a_1)', __METHOD__ );
$db->query( 'CREATE TABLE b (b_1, b_2)', __METHOD__ );
$db->insert( 'a', array(
@@ -289,7 +291,7 @@
'user_newtalk.user_last_timestamp', // r84185
);
- $currentDB = new DatabaseSqliteStandalone( ':memory:' );
+ $currentDB = DatabaseSqlite::newStandaloneInstance( ':memory:'
);
$currentDB->sourceFile( "$IP/maintenance/tables.sql" );
$profileToDb = false;
@@ -357,7 +359,7 @@
* @covers DatabaseSqlite::insertId
*/
public function testInsertIdType() {
- $db = new DatabaseSqliteStandalone( ':memory:' );
+ $db = DatabaseSqlite::newStandaloneInstance( ':memory:' );
$databaseCreation = $db->query( 'CREATE TABLE a ( a_1 )',
__METHOD__ );
$this->assertInstanceOf( 'ResultWrapper', $databaseCreation,
"Database creation" );
@@ -377,7 +379,7 @@
}
global $IP;
- $db = new DatabaseSqliteStandalone( ':memory:' );
+ $db = DatabaseSqlite::newStandaloneInstance( ':memory:' );
$db->sourceFile(
"$IP/tests/phpunit/data/db/sqlite/tables-$version.sql" );
$updater = DatabaseUpdater::newForDB( $db, false, $maint );
$updater->doUpdates( array( 'core' ) );
@@ -440,7 +442,7 @@
public function testCaseInsensitiveLike() {
// TODO: Test this for all databases
- $db = new DatabaseSqliteStandalone( ':memory:' );
+ $db = DatabaseSqlite::newStandaloneInstance( ':memory:' );
$res = $db->query( 'SELECT "a" LIKE "A" AS a' );
$row = $res->fetchRow();
$this->assertFalse( (bool)$row['a'] );
@@ -450,7 +452,7 @@
* @covers DatabaseSqlite::numFields
*/
public function testNumFields() {
- $db = new DatabaseSqliteStandalone( ':memory:' );
+ $db = DatabaseSqlite::newStandaloneInstance( ':memory:' );
$databaseCreation = $db->query( 'CREATE TABLE a ( a_1 )',
__METHOD__ );
$this->assertInstanceOf( 'ResultWrapper', $databaseCreation,
"Failed to create table a" );
--
To view, visit https://gerrit.wikimedia.org/r/195033
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1791fd4f630e5c121fa7f68f473411a7c12d0c97
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits