From a3a6dfed2ea1f4df3320536e592acab180f02979 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 20 Jul 2019 12:42:14 -0700 Subject: [PATCH] rdbms: allow automatic PDO creation of SQLite database files Define missing DatabaseSqlite::doSelectDomain() method to handle attempts to change the database, prefix, and/or schema. Also add sanity check to serverIsReadOnly() to make sure open() was called Change-Id: I72c25bf4dab5e01def3fb9472217e7637aede1d4 --- includes/libs/rdbms/database/Database.php | 6 +++ .../libs/rdbms/database/DatabaseSqlite.php | 40 ++++++++++++++++++- includes/libs/rdbms/database/IDatabase.php | 7 ++-- tests/phpunit/includes/db/LBFactoryTest.php | 18 ++++----- 4 files changed, 58 insertions(+), 13 deletions(-) diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index e82c735f29..ffd40268d8 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/includes/libs/rdbms/database/Database.php @@ -2390,6 +2390,12 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware $this->doSelectDomain( DatabaseDomain::newFromId( $domain ) ); } + /** + * @param DatabaseDomain $domain + * @throws DBConnectionError + * @throws DBError + * @since 1.32 + */ protected function doSelectDomain( DatabaseDomain $domain ) { $this->currentDomain = $domain; } diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php index cb1b84230a..97c4c9f235 100644 --- a/includes/libs/rdbms/database/DatabaseSqlite.php +++ b/includes/libs/rdbms/database/DatabaseSqlite.php @@ -142,7 +142,12 @@ class DatabaseSqlite extends Database { throw $this->newExceptionAfterConnectError( "DB path or directory required" ); } - if ( !self::isProcessMemoryPath( $path ) && !is_readable( $path ) ) { + // Check if the database file already exists but is non-readable + if ( + !self::isProcessMemoryPath( $path ) && + file_exists( $path ) && + !is_readable( $path ) + ) { throw $this->newExceptionAfterConnectError( 'SQLite database file is not readable' ); } elseif ( !in_array( $this->trxMode, self::$VALID_TRX_MODES, true ) ) { throw $this->newExceptionAfterConnectError( "Got mode '{$this->trxMode}' for BEGIN" ); @@ -163,6 +168,7 @@ class DatabaseSqlite extends Database { } try { + // Open the database file, creating it if it does not yet exist $this->conn = new PDO( "sqlite:$path", null, null, $attributes ); } catch ( PDOException $e ) { throw $this->newExceptionAfterConnectError( $e->getMessage() ); @@ -451,6 +457,36 @@ class DatabaseSqlite extends Database { return false; } + protected function doSelectDomain( DatabaseDomain $domain ) { + if ( $domain->getSchema() !== null ) { + throw new DBExpectedError( + $this, + __CLASS__ . ": domain '{$domain->getId()}' has a schema component" + ); + } + + $database = $domain->getDatabase(); + // A null database means "don't care" so leave it as is and update the table prefix + if ( $database === null ) { + $this->currentDomain = new DatabaseDomain( + $this->currentDomain->getDatabase(), + null, + $domain->getTablePrefix() + ); + + return true; + } + + if ( $database !== $this->getDBname() ) { + throw new DBExpectedError( + $this, + __CLASS__ . ": cannot change database (got '$database')" + ); + } + + return true; + } + /** * Use MySQL's naming (accounts for prefix etc) but remove surrounding backticks * @@ -765,6 +801,8 @@ class DatabaseSqlite extends Database { } public function serverIsReadOnly() { + $this->assertHasConnectionHandle(); + $path = $this->getDbFilePath(); return ( !self::isProcessMemoryPath( $path ) && !is_writable( $path ) ); diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php index 7e542218f4..f66e327735 100644 --- a/includes/libs/rdbms/database/IDatabase.php +++ b/includes/libs/rdbms/database/IDatabase.php @@ -1119,8 +1119,8 @@ interface IDatabase { * * @param string $db * @return bool True unless an exception was thrown - * @throws DBConnectionError If databasesAreIndependent() is true and an error occurs - * @throws DBError + * @throws DBConnectionError If databasesAreIndependent() is true and connection change fails + * @throws DBError On query error or if database changes are disallowed * @deprecated Since 1.32 Use selectDomain() instead */ public function selectDB( $db ); @@ -1133,8 +1133,9 @@ interface IDatabase { * This should only be called by a load balancer or if the handle is not attached to one * * @param string|DatabaseDomain $domain + * @throws DBConnectionError If databasesAreIndependent() is true and connection change fails + * @throws DBError On query error, if domain changes are disallowed, or the domain is invalid * @since 1.32 - * @throws DBConnectionError */ public function selectDomain( $domain ); diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php index 424c64b094..1595cd241b 100644 --- a/tests/phpunit/includes/db/LBFactoryTest.php +++ b/tests/phpunit/includes/db/LBFactoryTest.php @@ -645,17 +645,18 @@ class LBFactoryTest extends MediaWikiTestCase { * @covers \Wikimedia\Rdbms\DatabasePostgres::selectDB * @expectedException \Wikimedia\Rdbms\DBConnectionError */ - public function testInvalidSelectDBIndependant() { + public function testInvalidSelectDBIndependent() { $dbname = 'unittest-domain'; // explodes if DB is selected $factory = $this->newLBFactoryMulti( [ 'localDomain' => ( new DatabaseDomain( $dbname, null, '' ) )->getId() ], [ - 'dbname' => 'do_not_select_me' // explodes if DB is selected + // Explodes with SQLite and Postgres during open/USE + 'dbname' => 'bad_dir/do_not_select_me' ] ); $lb = $factory->getMainLB(); - if ( !wfGetDB( DB_MASTER )->databasesAreIndependent() ) { + if ( !$lb->getConnection( DB_MASTER )->databasesAreIndependent() ) { $this->markTestSkipped( "Not applicable per databasesAreIndependent()" ); } @@ -666,26 +667,25 @@ class LBFactoryTest extends MediaWikiTestCase { /** * @covers \Wikimedia\Rdbms\DatabaseSqlite::selectDB * @covers \Wikimedia\Rdbms\DatabasePostgres::selectDB - * @expectedException \Wikimedia\Rdbms\DBConnectionError + * @expectedException \Wikimedia\Rdbms\DBExpectedError */ - public function testInvalidSelectDBIndependant2() { + public function testInvalidSelectDBIndependent2() { $dbname = 'unittest-domain'; // explodes if DB is selected $factory = $this->newLBFactoryMulti( [ 'localDomain' => ( new DatabaseDomain( $dbname, null, '' ) )->getId() ], [ - 'dbname' => 'do_not_select_me' // explodes if DB is selected + // Explodes with SQLite and Postgres during open/USE + 'dbname' => 'bad_dir/do_not_select_me' ] ); $lb = $factory->getMainLB(); - if ( !wfGetDB( DB_MASTER )->databasesAreIndependent() ) { + if ( !$lb->getConnection( DB_MASTER )->databasesAreIndependent() ) { $this->markTestSkipped( "Not applicable per databasesAreIndependent()" ); } $db = $lb->getConnection( DB_MASTER ); - \Wikimedia\suppressWarnings(); $db->selectDB( 'garbage-db' ); - \Wikimedia\restoreWarnings(); } /** -- 2.20.1