r89821 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r89820‎ | r89821 | r89822 >
Date:11:32, 10 June 2011
Author:tstarling
Status:ok (Comments)
Tags:
Comment:
PostgreSQL install fixes:
* Made PG throw a DBQueryError when it gets a query error, instead of DBUnexpectedError. Apparently this mistake goes back to r14625, when exceptions were first introduced. Did it by removing reportQueryError(), the DatabaseBase version works fine.
* Fixed several places where there was an attempt to check for a query error by checking if the result of query() was false. This never worked. Used try/catch instead.
* Made the DBConnectionError messages go on one line so that they don't mess up the formatting in the installer.
* In DatabasePostgres::selectDB(), only disconnect and reconnect if the DB name is actually changing.
* Made DatabasePostgres::schemaExists() less weird and scary.
* Added DatabasePostgres::roleExists() for use by the installer.
* Removed the PostgreSQL-specific hack to make _InstallUser have a default other than "root". Made _InstallUser into a proper DBMS-specific internal variable instead, since every DBMS we support so far needs a different default.
* Removed the $dbName parameters from openConnection/getConnection, and got rid of $this->useAdmin. Implemented a more sophisticated caching scheme instead. Partial revert of r89389 and r81440.
* When connecting as the install user before DB creation, and when testing the web user's credentials, try a few different database names and use whichever one works.
* Instead of connecting as the web user to create tables, I used SET ROLE. It seems cleaner and more like what the other DBMSes do during installation. "SET ROLE wikiuser" requires the same privileges as "CREATE SCHEMA ... AUTHORIZATION wikiuser", so it's unlikely to break anything.
* In the area of web account creation, fixed various minor logic errors and introduced more informative error messages at the submit stage, pre-install. Show a helpful error message if the web user exists already and the install user can't do the relevant SET ROLE.
* Split schema creation out to a separate install step.
* When creating an account as a non-superuser, add the administrative account to the new account's group. This is necessary to avoid a fatal error during installation (bug 28845).
* Removed code which alters an existing web user to have appropriate search paths and permissions. This may break other apps and is not necessary. As in other DBMSes, If the web user exists, it is the responsibility of the sysadmin to ensure that it has appropriate permissions.
* Rewrote setupPLpgSQL() to use the query builder functions.
Modified paths:
  • /trunk/phase3/includes/db/DatabasePostgres.php (modified) (history)
  • /trunk/phase3/includes/installer/DatabaseInstaller.php (modified) (history)
  • /trunk/phase3/includes/installer/Ibm_db2Installer.php (modified) (history)
  • /trunk/phase3/includes/installer/Installer.i18n.php (modified) (history)
  • /trunk/phase3/includes/installer/Installer.php (modified) (history)
  • /trunk/phase3/includes/installer/MysqlInstaller.php (modified) (history)
  • /trunk/phase3/includes/installer/OracleInstaller.php (modified) (history)
  • /trunk/phase3/includes/installer/PostgresInstaller.php (modified) (history)
  • /trunk/phase3/includes/installer/SqliteInstaller.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/db/DatabasePostgres.php
@@ -186,7 +186,7 @@
187187 wfDebug( "DB connection error\n" );
188188 wfDebug( "Server: $server, Database: $dbName, User: $user, Password: " . substr( $password, 0, 3 ) . "...\n" );
189189 wfDebug( $this->lastError() . "\n" );
190 - throw new DBConnectionError( $this, $phpError );
 190+ throw new DBConnectionError( $this, str_replace( "\n", ' ', $phpError ) );
191191 }
192192
193193 $this->mOpened = true;
@@ -218,7 +218,11 @@
219219 * @return
220220 */
221221 function selectDB( $db ) {
222 - return (bool)$this->open( $this->mServer, $this->mUser, $this->mPassword, $db );
 222+ if ( $this->mDBname !== $db ) {
 223+ return (bool)$this->open( $this->mServer, $this->mUser, $this->mPassword, $db );
 224+ } else {
 225+ return true;
 226+ }
223227 }
224228
225229 function makeConnectionString( $vars ) {
@@ -762,23 +766,6 @@
763767 return $valuedata;
764768 }
765769
766 - function reportQueryError( $error, $errno, $sql, $fname, $tempIgnore = false ) {
767 - // Ignore errors during error handling to avoid infinite recursion
768 - $ignore = $this->ignoreErrors( true );
769 - $this->mErrorCount++;
770 -
771 - if ( $ignore || $tempIgnore ) {
772 - wfDebug( "SQL ERROR (ignored): $error\n" );
773 - $this->ignoreErrors( $ignore );
774 - } else {
775 - $message = "A database error has occurred. Did you forget to run maintenance/update.php after upgrading? See: http://www.mediawiki.org/wiki/Manual:Upgrading#Run_the_update_script\n" .
776 - "Query: $sql\n" .
777 - "Function: $fname\n" .
778 - "Error: $errno $error\n";
779 - throw new DBUnexpectedError( $this, $message );
780 - }
781 - }
782 -
783770 /**
784771 * @return string wikitext of a link to the server software's web site
785772 */
@@ -894,22 +881,23 @@
895882 }
896883
897884 /**
898 - * Query whether a given schema exists. Returns the name of the owner
 885+ * Query whether a given schema exists. Returns true if it does, false if it doesn't.
899886 */
900887 function schemaExists( $schema ) {
901 - $eschema = str_replace( "'", "''", $schema );
902 - $SQL = "SELECT rolname FROM pg_catalog.pg_namespace n, pg_catalog.pg_roles r "
903 - ."WHERE n.nspowner=r.oid AND n.nspname = '$eschema'";
904 - $res = $this->query( $SQL );
905 - if ( $res && $res->numRows() ) {
906 - $row = $res->fetchObject();
907 - $owner = $row->rolname;
908 - } else {
909 - $owner = false;
910 - }
911 - return $owner;
 888+ $exists = $this->selectField( '"pg_catalog"."pg_namespace"', 1,
 889+ array( 'nspname' => $schema ), __METHOD__ );
 890+ return (bool)$exists;
912891 }
913892
 893+ /**
 894+ * Returns true if a given role (i.e. user) exists, false otherwise.
 895+ */
 896+ function roleExists( $roleName ) {
 897+ $exists = $this->selectField( '"pg_catalog"."pg_roles"', 1,
 898+ array( 'rolname' => $roleName ), __METHOD__ );
 899+ return (bool)$exists;
 900+ }
 901+
914902 function fieldInfo( $table, $field ) {
915903 return PostgresField::fromText( $this, $table, $field );
916904 }
Index: trunk/phase3/includes/installer/Installer.php
@@ -161,7 +161,6 @@
162162 '_UpgradeDone' => false,
163163 '_InstallDone' => false,
164164 '_Caches' => array(),
165 - '_InstallUser' => 'root',
166165 '_InstallPassword' => '',
167166 '_SameAccount' => true,
168167 '_CreateDBAccount' => false,
Index: trunk/phase3/includes/installer/Installer.i18n.php
@@ -211,6 +211,7 @@
212212 'config-db-schema' => 'Schema for MediaWiki',
213213 'config-db-schema-help' => 'This schema will usually be fine.
214214 Only change it if you know you need to.',
 215+ 'config-pg-test-error' => "Cannot connect to database '''$1''': $2",
215216 'config-sqlite-dir' => 'SQLite data directory:',
216217 'config-sqlite-dir-help' => "SQLite stores all data in a single file.
217218
@@ -486,6 +487,7 @@
487488 'config-install-step-failed' => 'failed',
488489 'config-install-extensions' => 'Including extensions',
489490 'config-install-database' => 'Setting up database',
 491+ 'config-install-schema' => 'Creating schema',
490492 'config-install-pg-schema-not-exist' => 'PostgreSQL schema does not exist.',
491493 'config-install-pg-schema-failed' => 'Tables creation failed.
492494 Make sure that the user "$1" can write to the schema "$2".',
@@ -493,10 +495,17 @@
494496 'config-install-pg-plpgsql' => 'Checking for language PL/pgSQL',
495497 'config-pg-no-plpgsql' => 'You need to install the language PL/pgSQL in the database $1',
496498 'config-pg-no-create-privs' => 'The account you specified for installation does not have enough privileges to create an account.',
 499+ 'config-pg-not-in-role' => 'The account you specified for the web user already exists.
 500+The account you specified for installation is not a superuser and is not a member of the web user\'s role, so it is unable to create objects owned by the web user.
 501+
 502+MediaWiki currently requires that the tables be owned by the web user. Please specify another web account name, or click "back" and specify a suitably privileged install user.',
497503 'config-install-user' => 'Creating database user',
498504 'config-install-user-alreadyexists' => 'User "$1" already exists',
499505 'config-install-user-create-failed' => 'Creating user "$1" failed: $2',
500506 'config-install-user-grant-failed' => 'Granting permission to user "$1" failed: $2',
 507+ 'config-install-user-missing' => 'The specified user "$1" does not exist.',
 508+ 'config-install-user-missing-create' => 'The specified user "$1" does not exist.
 509+Please click the "create account" checkbox below if you want to create it.',
501510 'config-install-tables' => 'Creating tables',
502511 'config-install-tables-exist' => "'''Warning''': MediaWiki tables seem to already exist.
503512 Skipping creation.",
Index: trunk/phase3/includes/installer/Ibm_db2Installer.php
@@ -24,6 +24,10 @@
2525 'wgDBmwschema',
2626 );
2727
 28+ protected $internalDefaults = array(
 29+ '_InstallUser' => 'db2admin'
 30+ );
 31+
2832 /**
2933 * Get the DB2 database extension name
3034 * @return string
@@ -113,7 +117,7 @@
114118 * Open a DB2 database connection
115119 * @return Status
116120 */
117 - public function openConnection( $dbName = null ) {
 121+ public function openConnection() {
118122 $status = Status::newGood();
119123 try {
120124 $db = new DatabaseIbm_db2(
@@ -244,4 +248,4 @@
245249 public function __construct($parent) {
246250 parent::__construct($parent);
247251 }
248 -}
\ No newline at end of file
 252+}
Index: trunk/phase3/includes/installer/SqliteInstaller.php
@@ -122,7 +122,7 @@
123123 /**
124124 * @return Status
125125 */
126 - public function openConnection( $dbName = null ) {
 126+ public function openConnection() {
127127 global $wgSQLiteDataDir;
128128
129129 $status = Status::newGood();
Index: trunk/phase3/includes/installer/DatabaseInstaller.php
@@ -102,7 +102,7 @@
103103 *
104104 * @return Status
105105 */
106 - public abstract function openConnection( $dbName = null );
 106+ public abstract function openConnection();
107107
108108 /**
109109 * Create the database and return a Status object indicating success or
@@ -121,14 +121,12 @@
122122 *
123123 * @return Status
124124 */
125 - public function getConnection( $dbName = null ) {
126 - if ( isset($this->db) && $this->db ) { /* Weirdly get E_STRICT
127 - * errors without the
128 - * isset */
 125+ public function getConnection() {
 126+ if ( $this->db ) {
129127 return Status::newGood( $this->db );
130128 }
131129
132 - $status = $this->openConnection( $dbName );
 130+ $status = $this->openConnection();
133131 if ( $status->isOK() ) {
134132 $this->db = $status->value;
135133 // Enable autocommit
Index: trunk/phase3/includes/installer/MysqlInstaller.php
@@ -27,6 +27,7 @@
2828 protected $internalDefaults = array(
2929 '_MysqlEngine' => 'InnoDB',
3030 '_MysqlCharset' => 'binary',
 31+ '_InstallUser' => 'root',
3132 );
3233
3334 public $supportedEngines = array( 'InnoDB', 'MyISAM' );
@@ -126,7 +127,7 @@
127128 /**
128129 * @return Status
129130 */
130 - public function openConnection( $dbName = null ) {
 131+ public function openConnection() {
131132 $status = Status::newGood();
132133 try {
133134 $db = new DatabaseMysql(
Index: trunk/phase3/includes/installer/OracleInstaller.php
@@ -24,7 +24,8 @@
2525
2626 protected $internalDefaults = array(
2727 '_OracleDefTS' => 'USERS',
28 - '_OracleTempTS' => 'TEMP'
 28+ '_OracleTempTS' => 'TEMP',
 29+ '_InstallUser' => 'SYSDBA',
2930 );
3031
3132 public $minimumVersion = '9.0.1'; // 9iR1
@@ -127,7 +128,7 @@
128129 return $status;
129130 }
130131
131 - public function openConnection( $dbName = null ) {
 132+ public function openConnection() {
132133 $status = Status::newGood();
133134 try {
134135 $db = new DatabaseOracle(
Index: trunk/phase3/includes/installer/PostgresInstaller.php
@@ -23,9 +23,15 @@
2424 'wgDBmwschema',
2525 );
2626
 27+ protected $internalDefaults = array(
 28+ '_InstallUser' => 'postgres',
 29+ );
 30+
2731 var $minimumVersion = '8.3';
28 - private $useAdmin = false;
 32+ var $maxRoleSearchDepth = 5;
2933
 34+ protected $pgConns = array();
 35+
3036 function getName() {
3137 return 'postgres';
3238 }
@@ -35,11 +41,6 @@
3642 }
3743
3844 function getConnectForm() {
39 - // If this is our first time here, switch the default user presented in the form
40 - if ( ! $this->getVar('_switchedInstallUser') ) {
41 - $this->setVar('_InstallUser', 'postgres');
42 - $this->setVar('_switchedInstallUser', true);
43 - }
4445 return
4546 $this->getTextBox( 'wgDBserver', 'config-db-host', array(), $this->parent->getHelpBox( 'config-db-host-help' ) ) .
4647 $this->getTextBox( 'wgDBport', 'config-db-port' ) .
@@ -75,87 +76,185 @@
7677 return $status;
7778 }
7879
79 - $this->useAdmin = true;
80 - // Try to connect
81 - $status->merge( $this->getConnection() );
 80+ $status = $this->getPgConnection( 'create-db' );
8281 if ( !$status->isOK() ) {
8382 return $status;
8483 }
 84+ $conn = $status->value;
8585
86 - //Make sure install user can create
87 - if( !$this->canCreateAccounts() ) {
88 - $status->fatal( 'config-pg-no-create-privs' );
89 - }
90 - if ( !$status->isOK() ) {
91 - return $status;
92 - }
93 -
9486 // Check version
95 - $version = $this->db->getServerVersion();
 87+ $version = $conn->getServerVersion();
9688 if ( version_compare( $version, $this->minimumVersion ) < 0 ) {
9789 return Status::newFatal( 'config-postgres-old', $this->minimumVersion, $version );
9890 }
9991
10092 $this->setVar( 'wgDBuser', $this->getVar( '_InstallUser' ) );
10193 $this->setVar( 'wgDBpassword', $this->getVar( '_InstallPassword' ) );
 94+ return Status::newGood();
 95+ }
 96+
 97+ public function getConnection() {
 98+ $status = $this->getPgConnection( 'create-tables' );
 99+ if ( $status->isOK() ) {
 100+ $this->db = $status->value;
 101+ }
102102 return $status;
103103 }
104104
105 - public function openConnection( $dbName = null ) {
 105+ public function openConnection() {
 106+ return $this->openPgConnection( 'create-tables' );
 107+ }
 108+
 109+ /**
 110+ * Open a PG connection with given parameters
 111+ * @param $user User name
 112+ * @param $password Password
 113+ * @param $dbName Database name
 114+ * @return Status
 115+ */
 116+ protected function openConnectionWithParams( $user, $password, $dbName ) {
106117 $status = Status::newGood();
107118 try {
108 - if ( $this->useAdmin ) {
109 - if ( $dbName === null ) $dbName = 'postgres';
 119+ $db = new DatabasePostgres(
 120+ $this->getVar( 'wgDBserver' ),
 121+ $user,
 122+ $password,
 123+ $dbName);
 124+ $status->value = $db;
 125+ } catch ( DBConnectionError $e ) {
 126+ $status->fatal( 'config-connection-error', $e->getMessage() );
 127+ }
 128+ return $status;
 129+ }
110130
111 - $db = new DatabasePostgres(
112 - $this->getVar( 'wgDBserver' ),
 131+ /**
 132+ * Get a special type of connection
 133+ * @param $type See openPgConnection() for details.
 134+ * @return Status
 135+ */
 136+ protected function getPgConnection( $type ) {
 137+ if ( isset( $this->pgConns[$type] ) ) {
 138+ return Status::newGood( $this->pgConns[$type] );
 139+ }
 140+ $status = $this->openPgConnection( $type );
 141+
 142+ if ( $status->isOK() ) {
 143+ $conn = $status->value;
 144+ $conn->clearFlag( DBO_TRX );
 145+ $conn->commit();
 146+ $this->pgConns[$type] = $conn;
 147+ }
 148+ return $status;
 149+ }
 150+
 151+ /**
 152+ * Get a connection of a specific PostgreSQL-specific type. Connections
 153+ * of a given type are cached.
 154+ *
 155+ * PostgreSQL lacks cross-database operations, so after the new database is
 156+ * created, you need to make a separate connection to connect to that
 157+ * database and add tables to it.
 158+ *
 159+ * New tables are owned by the user that creates them, and MediaWiki's
 160+ * PostgreSQL support has always assumed that the table owner will be
 161+ * $wgDBuser. So before we create new tables, we either need to either
 162+ * connect as the other user or to execute a SET ROLE command. Using a
 163+ * separate connection for this allows us to avoid accidental cross-module
 164+ * dependencies.
 165+ *
 166+ * @param $type The type of connection to get:
 167+ * - create-db: A connection for creating DBs, suitable for pre-
 168+ * installation.
 169+ * - create-schema: A connection to the new DB, for creating schemas and
 170+ * other similar objects in the new DB.
 171+ * - create-tables: A connection with a role suitable for creating tables.
 172+ *
 173+ * @return A Status object. On success, a connection object will be in the
 174+ * value member.
 175+ */
 176+ protected function openPgConnection( $type ) {
 177+ switch ( $type ) {
 178+ case 'create-db':
 179+ return $this->openConnectionToAnyDB(
 180+ $this->getVar( '_InstallUser' ),
 181+ $this->getVar( '_InstallPassword' ) );
 182+ case 'create-schema':
 183+ return $this->openConnectionWithParams(
113184 $this->getVar( '_InstallUser' ),
114185 $this->getVar( '_InstallPassword' ),
115 - $dbName );
116 - } else {
117 - if ( $dbName === null ) $dbName = $this->getVar( 'wgDBname' );
 186+ $this->getVar( 'wgDBname' ) );
 187+ case 'create-tables':
 188+ $status = $this->openPgConnection( 'create-schema' );
 189+ if ( $status->isOK() ) {
 190+ $conn = $status->value;
 191+ $safeRole = $conn->addIdentifierQuotes( $this->getVar( 'wgDBuser' ) );
 192+ $conn->query( "SET ROLE $safeRole" );
 193+ }
 194+ return $status;
 195+ default:
 196+ throw new MWException( "Invalid special connection type: \"$type\"" );
 197+ }
 198+ }
118199
119 - $db = new DatabasePostgres(
 200+ public function openConnectionToAnyDB( $user, $password ) {
 201+ $dbs = array(
 202+ 'template1',
 203+ 'postgres',
 204+ );
 205+ if ( !in_array( $this->getVar( 'wgDBname' ), $dbs ) ) {
 206+ array_unshift( $dbs, $this->getVar( 'wgDBname' ) );
 207+ }
 208+ $status = Status::newGood();
 209+ foreach ( $dbs as $db ) {
 210+ try {
 211+ $conn = new DatabasePostgres(
120212 $this->getVar( 'wgDBserver' ),
121 - $this->getVar( 'wgDBuser' ),
122 - $this->getVar( 'wgDBpassword' ),
123 - $dbName );
 213+ $user,
 214+ $password,
 215+ $db );
 216+ } catch ( DBConnectionError $error ) {
 217+ $conn = false;
 218+ $status->fatal( 'config-pg-test-error', $db,
 219+ $error->getMessage() );
124220 }
125 -
126 - if( $db === null ) throw new DBConnectionError("Unknown problem while connecting.");
127 - $safeschema = $db->addIdentifierQuotes( $this->getVar( 'wgDBmwschema' ) );
128 - if( $db->schemaExists( $this->getVar( 'wgDBmwschema' ) ) ) $db->query( "SET search_path = $safeschema" );
129 -
130 - $status->value = $db;
131 - } catch ( DBConnectionError $e ) {
132 - $status->fatal( 'config-connection-error', $e->getMessage() );
 221+ if ( $conn !== false ) {
 222+ break;
 223+ }
133224 }
134 - return $status;
 225+ if ( $conn !== false ) {
 226+ return Status::newGood( $conn );
 227+ } else {
 228+ return $status;
 229+ }
135230 }
136231
137 - protected function canCreateAccounts() {
138 - $this->useAdmin = true;
139 - $status = $this->getConnection();
 232+ protected function getInstallUserPermissions() {
 233+ $status = $this->getPgConnection( 'create-db' );
140234 if ( !$status->isOK() ) {
141235 return false;
142236 }
143237 $conn = $status->value;
144 -
145238 $superuser = $this->getVar( '_InstallUser' );
146239
147 - $rights = $conn->selectField( 'pg_catalog.pg_roles',
148 - 'CASE WHEN rolsuper then 1
149 - WHEN rolcreatedb then 2
150 - ELSE 3
151 - END as rights',
152 - array( 'rolname' => $superuser ), __METHOD__
153 - );
 240+ $row = $conn->selectRow( '"pg_catalog"."pg_roles"', '*',
 241+ array( 'rolname' => $superuser ), __METHOD__ );
 242+ return $row;
 243+ }
154244
155 - if( !$rights || $rights == 3 ) {
 245+ protected function canCreateAccounts() {
 246+ $perms = $this->getInstallUserPermissions();
 247+ if ( !$perms ) {
156248 return false;
157249 }
 250+ return $perms->rolsuper === 't' || $perms->rolcreaterole === 't';
 251+ }
158252
159 - return true;
 253+ protected function isSuperUser() {
 254+ $perms = $this->getInstallUserPermissions();
 255+ if ( !$perms ) {
 256+ return false;
 257+ }
 258+ return $perms->rolsuper === 't';
160259 }
161260
162261 public function getSettingsForm() {
@@ -175,28 +274,112 @@
176275 return $status;
177276 }
178277
 278+ $same = $this->getVar( 'wgDBuser' ) === $this->getVar( '_InstallUser' );
 279+
 280+ if ( !$same ) {
 281+ // Check if the web user exists
 282+ // Connect to the database with the install user
 283+ $status = $this->getPgConnection( 'create-db' );
 284+ if ( !$status->isOK() ) {
 285+ return $status;
 286+ }
 287+ $exists = $status->value->roleExists( $this->getVar( 'wgDBuser' ) );
 288+ }
 289+
179290 // Validate the create checkbox
180 - if ( !$this->canCreateAccounts() ) {
 291+ if ( $this->canCreateAccounts() && !$same && !$exists ) {
 292+ $create = $this->getVar( '_CreateDBAccount' );
 293+ } else {
181294 $this->setVar( '_CreateDBAccount', false );
182295 $create = false;
183 - } else {
184 - $create = $this->getVar( '_CreateDBAccount' );
185296 }
186297
187 - // Don't test the web account if it is the same as the admin.
188 - if ( !$create && $this->getVar( 'wgDBuser' ) != $this->getVar( '_InstallUser' ) ) {
189 - // Test the web account
190 - try {
191 - $this->useAdmin = false;
192 - return $this->openConnection();
193 - } catch ( DBConnectionError $e ) {
194 - return Status::newFatal( 'config-connection-error', $e->getMessage() );
 298+ if ( !$create && !$exists ) {
 299+ if ( $this->canCreateAccounts() ) {
 300+ $msg = 'config-install-user-missing-create';
 301+ } else {
 302+ $msg = 'config-install-user-missing';
195303 }
 304+ return Status::newFatal( $msg, $this->getVar( 'wgDBuser' ) );
196305 }
197306
198 - return Status::newGood();
 307+ if ( !$exists ) {
 308+ // No more checks to do
 309+ return Status::newGood();
 310+ }
 311+
 312+ // Existing web account. Test the connection.
 313+ $status = $this->openConnectionToAnyDB(
 314+ $this->getVar( 'wgDBuser' ),
 315+ $this->getVar( 'wgDBpassword' ) );
 316+ if ( !$status->isOK() ) {
 317+ return $status;
 318+ }
 319+
 320+ // The web user is conventionally the table owner in PostgreSQL
 321+ // installations. Make sure the install user is able to create
 322+ // objects on behalf of the web user.
 323+ if ( $this->canCreateObjectsForWebUser() ) {
 324+ return Status::newGood();
 325+ } else {
 326+ return Status::newFatal( 'config-pg-not-in-role' );
 327+ }
199328 }
200329
 330+ /**
 331+ * Returns true if the install user is able to create objects owned
 332+ * by the web user, false otherwise.
 333+ */
 334+ protected function canCreateObjectsForWebUser() {
 335+ if ( $this->isSuperUser() ) {
 336+ return true;
 337+ }
 338+
 339+ $status = $this->getPgConnection( 'create-db' );
 340+ if ( !$status->isOK() ) {
 341+ return false;
 342+ }
 343+ $conn = $status->value;
 344+ $installerId = $conn->selectField( '"pg_catalog"."pg_roles"', 'oid',
 345+ array( 'rolname' => $this->getVar( '_InstallUser' ) ), __METHOD__ );
 346+ $webId = $conn->selectField( '"pg_catalog"."pg_roles"', 'oid',
 347+ array( 'rolname' => $this->getVar( 'wgDBuser' ) ), __METHOD__ );
 348+
 349+ return $this->isRoleMember( $conn, $installerId, $webId, $this->maxRoleSearchDepth );
 350+ }
 351+
 352+ /**
 353+ * Recursive helper for canCreateObjectsForWebUser().
 354+ * @param $conn Database object
 355+ * @param $targetMember Role ID of the member to look for
 356+ * @param $group Role ID of the group to look for
 357+ * @param $maxDepth Maximum recursive search depth
 358+ */
 359+ protected function isRoleMember( $conn, $targetMember, $group, $maxDepth ) {
 360+ if ( $targetMember === $group ) {
 361+ // A role is always a member of itself
 362+ return true;
 363+ }
 364+ // Get all members of the given group
 365+ $res = $conn->select( '"pg_catalog"."pg_auth_members"', array( 'member' ),
 366+ array( 'roleid' => $group ), __METHOD__ );
 367+ foreach ( $res as $row ) {
 368+ if ( $row->member == $targetMember ) {
 369+ // Found target member
 370+ return true;
 371+ }
 372+ // Recursively search each member of the group to see if the target
 373+ // is a member of it, up to the given maximum depth.
 374+ if ( $maxDepth > 0 ) {
 375+ if ( $this->isRoleMember( $conn, $targetMember, $row->member, $maxDepth - 1 ) ) {
 376+ // Found member of member
 377+ return true;
 378+ }
 379+ }
 380+ }
 381+ return false;
 382+ }
 383+
201384 public function preInstall() {
202385 $commitCB = array(
203386 'name' => 'pg-commit',
@@ -206,8 +389,13 @@
207390 'name' => 'pg-plpgsql',
208391 'callback' => array( $this, 'setupPLpgSQL' ),
209392 );
 393+ $schemaCB = array(
 394+ 'name' => 'schema',
 395+ 'callback' => array( $this, 'setupSchema' )
 396+ );
210397 $this->parent->addInstallStep( $commitCB, 'interwiki' );
211398 $this->parent->addInstallStep( $plpgCB, 'database' );
 399+ $this->parent->addInstallStep( $schemaCB, 'database' );
212400 if( $this->getVar( '_CreateDBAccount' ) ) {
213401 $this->parent->addInstallStep( array(
214402 'name' => 'user',
@@ -217,12 +405,10 @@
218406 }
219407
220408 function setupDatabase() {
221 - $this->useAdmin = true;
222 - $status = $this->getConnection();
 409+ $status = $this->getPgConnection( 'create-db' );
223410 if ( !$status->isOK() ) {
224411 return $status;
225412 }
226 - $this->setupSchemaVars();
227413 $conn = $status->value;
228414
229415 $dbName = $this->getVar( 'wgDBname' );
@@ -231,46 +417,45 @@
232418 $safeschema = $conn->addIdentifierQuotes( $schema );
233419 $safeuser = $conn->addIdentifierQuotes( $user );
234420
235 - $SQL = "SELECT 1 FROM pg_catalog.pg_database WHERE datname = " . $conn->addQuotes( $dbName );
236 - $rows = $conn->numRows( $conn->query( $SQL ) );
237 - $safedb = $conn->addIdentifierQuotes( $dbName );
238 - if( !$rows ) {
 421+ $exists = $conn->selectField( '"pg_catalog"."pg_database"', '1',
 422+ array( 'datname' => $dbName ), __METHOD__ );
 423+ if ( !$exists ) {
 424+ $safedb = $conn->addIdentifierQuotes( $dbName );
239425 $conn->query( "CREATE DATABASE $safedb", __METHOD__ );
240 - $conn->query( "GRANT ALL ON DATABASE $safedb to $safeuser", __METHOD__ );
241 - } else {
242 - $conn->query( "GRANT ALL ON DATABASE $safedb TO $safeuser", __METHOD__ );
243426 }
 427+ return Status::newGood();
 428+ }
244429
245 - // Now that we've established the real database exists, connect to it
246 - // Because we do not want the same connection, forcibly expire the existing conn
247 - $this->db = null;
248 - $this->useAdmin = false;
249 - $status = $this->getConnection();
 430+ function setupSchema() {
 431+ // Get a connection to the target database
 432+ $status = $this->getPgConnection( 'create-schema' );
250433 if ( !$status->isOK() ) {
251434 return $status;
252435 }
253436 $conn = $status->value;
254437
 438+ // Create the schema if necessary
 439+ $schema = $this->getVar( 'wgDBmwschema' );
 440+ $safeschema = $conn->addIdentifierQuotes( $schema );
 441+ $safeuser = $conn->addIdentifierQuotes( $this->getVar( 'wgDBuser' ) );
255442 if( !$conn->schemaExists( $schema ) ) {
256 - $result = $conn->query( "CREATE SCHEMA $safeschema AUTHORIZATION $safeuser" );
257 - if( !$result ) {
258 - $status->fatal( 'config-install-pg-schema-failed', $user, $schema );
 443+ try {
 444+ $conn->query( "CREATE SCHEMA $safeschema AUTHORIZATION $safeuser" );
 445+ } catch ( DBQueryError $e ) {
 446+ return Status::newFatal( 'config-install-pg-schema-failed',
 447+ $this->getVar( '_InstallUser' ), $schema );
259448 }
260 - } else {
261 - $safeschema2 = $conn->addQuotes( $schema );
262 - $SQL = "SELECT 'GRANT ALL ON '||pg_catalog.quote_ident(relname)||' TO $safeuser;'\n".
263 - "FROM pg_catalog.pg_class p, pg_catalog.pg_namespace n\n" .
264 - "WHERE relnamespace = n.oid AND n.nspname = $safeschema2\n" .
265 - "AND p.relkind IN ('r','S','v')\n";
266 - $SQL .= "UNION\n";
267 - $SQL .= "SELECT 'GRANT ALL ON FUNCTION '||pg_catalog.quote_ident(proname)||'('||\n".
268 - "pg_catalog.oidvectortypes(p.proargtypes)||') TO $safeuser;'\n" .
269 - "FROM pg_catalog.pg_proc p, pg_catalog.pg_namespace n\n" .
270 - "WHERE p.pronamespace = n.oid AND n.nspname = $safeschema2";
271 - $conn->query( "SET search_path = $safeschema" );
272 - $res = $conn->query( $SQL );
273449 }
274 - return $status;
 450+
 451+ // If we created a user, alter it now to search the new schema by default
 452+ if ( $this->getVar( '_CreateDBAccount' ) ) {
 453+ $conn->query( "ALTER ROLE $safeuser SET search_path = $safeschema, public",
 454+ __METHOD__ );
 455+ }
 456+
 457+ // Select the new schema in the current connection
 458+ $conn->query( "SET search_path = $safeschema" );
 459+ return Status::newGood();
275460 }
276461
277462 function commitChanges() {
@@ -283,34 +468,39 @@
284469 return Status::newGood();
285470 }
286471
287 - $this->useAdmin = true;
288 - $status = $this->getConnection();
289 -
 472+ $status = $this->getPgConnection( 'create-db' );
290473 if ( !$status->isOK() ) {
291474 return $status;
292475 }
 476+ $conn = $status->value;
293477
294478 $schema = $this->getVar( 'wgDBmwschema' );
295 - $safeuser = $this->db->addIdentifierQuotes( $this->getVar( 'wgDBuser' ) );
296 - $safeusercheck = $this->db->addQuotes( $this->getVar( 'wgDBuser' ) );
297 - $safepass = $this->db->addQuotes( $this->getVar( 'wgDBpassword' ) );
298 - $safeschema = $this->db->addIdentifierQuotes( $schema );
 479+ $safeuser = $conn->addIdentifierQuotes( $this->getVar( 'wgDBuser' ) );
 480+ $safepass = $conn->addQuotes( $this->getVar( 'wgDBpassword' ) );
 481+ $safeschema = $conn->addIdentifierQuotes( $schema );
299482
300 - $rows = $this->db->numRows(
301 - $this->db->query( "SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = $safeusercheck" )
302 - );
303 - if ( $rows < 1 ) {
304 - $res = $this->db->query( "CREATE ROLE $safeuser NOCREATEDB LOGIN PASSWORD $safepass", __METHOD__ );
305 - if ( $res !== true && !( $res instanceOf ResultWrapper ) ) {
306 - $status->fatal( 'config-install-user-failed', $this->getVar( 'wgDBuser' ), $res );
 483+ // Check if the user already exists
 484+ $userExists = $conn->roleExists( $this->getVar( 'wgDBuser' ) );
 485+ if ( !$userExists ) {
 486+ // Create the user
 487+ try {
 488+ $sql = "CREATE ROLE $safeuser NOCREATEDB LOGIN PASSWORD $safepass";
 489+
 490+ // If the install user is not a superuser, we need to make the install
 491+ // user a member of the new user's group, so that the install user will
 492+ // be able to create a schema and other objects on behalf of the new user.
 493+ if ( !$this->isSuperUser() ) {
 494+ $sql .= ' ROLE' . $conn->addIdentifierQuotes( $this->getVar( '_InstallUser' ) );
 495+ }
 496+
 497+ $conn->query( $sql, __METHOD__ );
 498+ } catch ( DBQueryError $e ) {
 499+ return Status::newFatal( 'config-install-user-create-failed',
 500+ $this->getVar( 'wgDBuser' ), $e->getMessage() );
307501 }
308 - if( $status->isOK() ) {
309 - $this->db->query("ALTER ROLE $safeuser LOGIN");
310 - }
311502 }
312 - $this->db->query("ALTER ROLE $safeuser SET search_path = $safeschema, public");
313503
314 - return $status;
 504+ return Status::newGood();
315505 }
316506
317507 function getLocalSettings() {
@@ -334,32 +524,30 @@
335525 public function createTables() {
336526 $schema = $this->getVar( 'wgDBmwschema' );
337527
338 - $this->db = null;
339 - $this->useAdmin = false;
340528 $status = $this->getConnection();
341529 if ( !$status->isOK() ) {
342530 return $status;
343531 }
 532+ $conn = $status->value;
344533
345 - if( $this->db->tableExists( 'user' ) ) {
 534+ if( $conn->tableExists( 'user' ) ) {
346535 $status->warning( 'config-install-tables-exist' );
347536 return $status;
348537 }
349538
350 - $this->db->begin( __METHOD__ );
 539+ $conn->begin( __METHOD__ );
351540
352 - // getConnection() should have already selected the schema if it exists
353 - if( !$this->db->schemaExists( $schema ) ) {
354 - $status->error( 'config-install-pg-schema-not-exist' );
 541+ if( !$conn->schemaExists( $schema ) ) {
 542+ $status->fatal( 'config-install-pg-schema-not-exist' );
355543 return $status;
356544 }
357 - $error = $this->db->sourceFile( $this->db->getSchema() );
 545+ $error = $conn->sourceFile( $conn->getSchema() );
358546 if( $error !== true ) {
359 - $this->db->reportQueryError( $error, 0, '', __METHOD__ );
360 - $this->db->rollback( __METHOD__ );
 547+ $conn->reportQueryError( $error, 0, '', __METHOD__ );
 548+ $conn->rollback( __METHOD__ );
361549 $status->fatal( 'config-install-tables-failed', $error );
362550 } else {
363 - $this->db->commit( __METHOD__ );
 551+ $conn->commit( __METHOD__ );
364552 }
365553 // Resume normal operations
366554 if( $status->isOk() ) {
@@ -369,34 +557,40 @@
370558 }
371559
372560 public function setupPLpgSQL() {
373 - $this->db = null;
374 - $this->useAdmin = true;
375 - $dbName = $this->getVar( 'wgDBname' );
376 - $status = $this->getConnection( $dbName );
 561+ // Connect as the install user, since it owns the database and so is
 562+ // the user that needs to run "CREATE LANGAUGE"
 563+ $status = $this->getPgConnection( 'create-schema' );
377564 if ( !$status->isOK() ) {
378565 return $status;
379566 }
380 - $this->db = $status->value;
 567+ $conn = $status->value;
381568
382 - /* Admin user has to be connected to the db it just
383 - created to satisfy ownership requirements for
384 - "CREATE LANGAUGE" */
385 - $rows = $this->db->numRows(
386 - $this->db->query( "SELECT 1 FROM pg_catalog.pg_language WHERE lanname = 'plpgsql'" )
387 - );
388 - if ( $rows < 1 ) {
389 - // plpgsql is not installed, but if we have a pg_pltemplate table, we should be able to create it
390 - $SQL = "SELECT 1 FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n ON (n.oid = c.relnamespace) ".
391 - "WHERE relname = 'pg_pltemplate' AND nspname='pg_catalog'";
392 - $rows = $this->db->numRows( $this->db->query( $SQL ) );
393 - if ( $rows >= 1 ) {
394 - $result = $this->db->query( 'CREATE LANGUAGE plpgsql' );
395 - if ( !$result ) {
396 - return Status::newFatal( 'config-pg-no-plpgsql', $dbName );
397 - }
398 - } else {
399 - return Status::newFatal( 'config-pg-no-plpgsql', $dbName );
 569+ $exists = $conn->selectField( '"pg_catalog"."pg_language"', 1,
 570+ array( 'lanname' => 'plpgsql' ), __METHOD__ );
 571+ if ( $exists ) {
 572+ // Already exists, nothing to do
 573+ return Status::newGood();
 574+ }
 575+
 576+ // plpgsql is not installed, but if we have a pg_pltemplate table, we
 577+ // should be able to create it
 578+ $exists = $conn->selectField(
 579+ array( '"pg_catalog"."pg_class"', '"pg_catalog"."pg_namespace"' ),
 580+ 1,
 581+ array(
 582+ 'pg_namespace.oid=relnamespace',
 583+ 'nspname' => 'pg_catalog',
 584+ 'relname' => 'pg_pltemplate',
 585+ ),
 586+ __METHOD__ );
 587+ if ( $exists ) {
 588+ try {
 589+ $conn->query( 'CREATE LANGUAGE plpgsql' );
 590+ } catch ( DBQueryError $e ) {
 591+ return Status::newFatal( 'config-pg-no-plpgsql', $this->getVar( 'wgDBname' ) );
400592 }
 593+ } else {
 594+ return Status::newFatal( 'config-pg-no-plpgsql', $this->getVar( 'wgDBname' ) );
401595 }
402596 return Status::newGood();
403597 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r89822Revert r89807, i.e. reapply r88929, since it has some useful changes and is t...tstarling11:55, 10 June 2011
r89823MFT r89821: PostgreSQL installer fixes.tstarling12:16, 10 June 2011
r89839Make Pg installer work in the “common” case: follow up r89821 so that $ex...mah18:48, 10 June 2011
r89855Fix for r89821, r89839: we can skip certain tests if the web user is the same...tstarling22:33, 10 June 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r14625Some initial work on exceptions. More to follow.tstarling23:07, 6 June 2006
r81440Follow-up to r81439, works up to installing the admin user then dies with:...overlordq05:17, 3 February 2011
r88929Commiting here, because other problems exist on trunk. This needs to...mah20:34, 26 May 2011
r89389forward port r88929mah03:41, 3 June 2011

Comments

#Comment by Tim Starling (talk | contribs)   13:01, 10 June 2011

If you revert r89807 in 1.17 then the changes here to PostgresInstaller.php merge cleanly.

Status & tagging log