From 46cf4f1e3010010e531340b99cbd59469529be0f Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 7 Mar 2015 04:31:08 -0800 Subject: [PATCH] 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 --- autoload.php | 1 - includes/db/DatabaseSqlite.php | 105 +++++++++++------- includes/installer/SqliteInstaller.php | 2 +- maintenance/sqlite.inc | 2 +- maintenance/sqlite.php | 6 +- maintenance/update.php | 2 +- .../includes/db/DatabaseSqliteTest.php | 30 ++--- 7 files changed, 84 insertions(+), 64 deletions(-) diff --git a/autoload.php b/autoload.php index faf8252481..835ed13474 100644 --- a/autoload.php +++ b/autoload.php @@ -289,7 +289,6 @@ $wgAutoloadLocalClasses = array( '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 0b51972ee1..ed86bab1b8 100644 --- a/includes/db/DatabaseSqlite.php +++ b/includes/db/DatabaseSqlite.php @@ -29,8 +29,11 @@ class DatabaseSqlite extends DatabaseBase { /** @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 @@ class DatabaseSqlite extends DatabaseBase { /** * 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 @@ class DatabaseSqlite extends DatabaseBase { * @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 @@ class DatabaseSqlite extends DatabaseBase { * @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", '', '', @@ -156,6 +188,14 @@ class DatabaseSqlite extends DatabaseBase { return false; } + /** + * @return string SQLite DB file path + * @since 1.25 + */ + public function getDbFilePath() { + return $this->dbPath; + } + /** * Does not actually close the connection, just destroys the reference for GC to do its work * @return bool @@ -206,8 +246,7 @@ class DatabaseSqlite extends DatabaseBase { $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 @@ class DatabaseSqlite extends DatabaseBase { * @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 @@ class DatabaseSqlite extends DatabaseBase { } 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\"." ); } } @@ -961,23 +997,6 @@ class DatabaseSqlite extends DatabaseBase { } } // 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 1e7e969848..f990ddf5c2 100644 --- a/includes/installer/SqliteInstaller.php +++ b/includes/installer/SqliteInstaller.php @@ -55,7 +55,7 @@ class SqliteInstaller extends DatabaseInstaller { 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.inc b/maintenance/sqlite.inc index 5c0fd07fbb..e17319087b 100644 --- a/maintenance/sqlite.inc +++ b/maintenance/sqlite.inc @@ -59,7 +59,7 @@ class Sqlite { 'blob', // NULL type is omitted intentionally ) ); - $db = new DatabaseSqliteStandalone( ':memory:' ); + $db = DatabaseSqlite::newStandaloneInstance( ':memory:' ); try { foreach ( $files as $file ) { $err = $db->sourceFile( $file ); diff --git a/maintenance/sqlite.php b/maintenance/sqlite.php index edc9e145ac..7e02a4b2bd 100644 --- a/maintenance/sqlite.php +++ b/maintenance/sqlite.php @@ -81,7 +81,7 @@ class SqliteMaintenance extends Maintenance { } 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 @@ class SqliteMaintenance extends Maintenance { $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 @@ class SqliteMaintenance extends Maintenance { 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 182a2c4660..6e93011bce 100755 --- a/maintenance/update.php +++ b/maintenance/update.php @@ -151,7 +151,7 @@ class UpdateMediaWiki extends Maintenance { $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 d32c1a6e14..645baf1f17 100644 --- a/tests/phpunit/includes/db/DatabaseSqliteTest.php +++ b/tests/phpunit/includes/db/DatabaseSqliteTest.php @@ -1,10 +1,12 @@ 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 @@ class DatabaseSqliteTest extends MediaWikiTestCase { */ 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 @@ class DatabaseSqliteTest extends MediaWikiTestCase { */ 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 @@ class DatabaseSqliteTest extends MediaWikiTestCase { * @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 @@ class DatabaseSqliteTest extends MediaWikiTestCase { * @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 @@ class DatabaseSqliteTest extends MediaWikiTestCase { * @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 @@ class DatabaseSqliteTest extends MediaWikiTestCase { '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 @@ class DatabaseSqliteTest extends MediaWikiTestCase { * @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 @@ class DatabaseSqliteTest extends MediaWikiTestCase { } 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 @@ class DatabaseSqliteTest extends MediaWikiTestCase { 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 @@ class DatabaseSqliteTest extends MediaWikiTestCase { * @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" ); -- 2.20.1